From fc156a72bc46337ecc0163fbba72624f28e4fa24 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Mon, 26 Aug 2019 13:31:33 +0200 Subject: [PATCH 1/4] codacy: A class which only has private constructors should be final codacy: Avoid throwing raw exception types. Signed-off-by: Andy Scherzinger --- .../android/operations/UploadException.java | 31 +++++++++++++++++++ .../operations/UploadFileOperation.java | 10 +++--- .../data/activities/ActivityRepositories.java | 6 ++-- .../data/files/FileRepositories.java | 4 +-- 4 files changed, 40 insertions(+), 11 deletions(-) create mode 100644 src/main/java/com/owncloud/android/operations/UploadException.java diff --git a/src/main/java/com/owncloud/android/operations/UploadException.java b/src/main/java/com/owncloud/android/operations/UploadException.java new file mode 100644 index 0000000000..49feb5f03c --- /dev/null +++ b/src/main/java/com/owncloud/android/operations/UploadException.java @@ -0,0 +1,31 @@ +/* + * Nextcloud Android client application + * + * @author Andy Scherzinger + * Copyright (C) 2019 Andy Scherzinger + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package com.owncloud.android.operations; + +class UploadException extends Exception { + UploadException() { + super(); + } + + UploadException(String message) { + super(message); + } +} diff --git a/src/main/java/com/owncloud/android/operations/UploadFileOperation.java b/src/main/java/com/owncloud/android/operations/UploadFileOperation.java index 3d8f181686..1ffe1dcecc 100644 --- a/src/main/java/com/owncloud/android/operations/UploadFileOperation.java +++ b/src/main/java/com/owncloud/android/operations/UploadFileOperation.java @@ -464,9 +464,9 @@ public class UploadFileOperation extends SyncOperation { mUpload.setFolderUnlockToken(token); uploadsStorageManager.updateUpload(mUpload); } else if (lockFileOperationResult.getHttpCode() == HttpStatus.SC_FORBIDDEN) { - throw new Exception("Forbidden! Please try again later.)"); + throw new UploadException("Forbidden! Please try again later.)"); } else { - throw new Exception("Unknown error!"); + throw new UploadException("Unknown error!"); } // Update metadata @@ -498,10 +498,10 @@ public class UploadFileOperation extends SyncOperation { metadata.getMetadata().getMetadataKeys().put(0, encryptedMetadataKey); } else { // TODO error - throw new Exception("something wrong"); + throw new UploadException("something wrong"); } - /***** E2E *****/ + /**** E2E *****/ // check name collision checkNameCollision(client, metadata, parentFile.isEncrypted()); @@ -646,7 +646,7 @@ public class UploadFileOperation extends SyncOperation { } if (!uploadMetadataOperationResult.isSuccess()) { - throw new Exception(); + throw new UploadException(); } } } catch (FileNotFoundException e) { diff --git a/src/main/java/com/owncloud/android/ui/activities/data/activities/ActivityRepositories.java b/src/main/java/com/owncloud/android/ui/activities/data/activities/ActivityRepositories.java index 8949b17196..c341875ae1 100644 --- a/src/main/java/com/owncloud/android/ui/activities/data/activities/ActivityRepositories.java +++ b/src/main/java/com/owncloud/android/ui/activities/data/activities/ActivityRepositories.java @@ -1,4 +1,4 @@ -/** +/* * Nextcloud Android client application * * Copyright (C) 2018 Edvard Holst @@ -20,7 +20,7 @@ package com.owncloud.android.ui.activities.data.activities; import androidx.annotation.NonNull; -public class ActivityRepositories { +public final class ActivityRepositories { private ActivityRepositories() { // No instance @@ -29,6 +29,4 @@ public class ActivityRepositories { public static synchronized ActivitiesRepository getRepository(@NonNull ActivitiesServiceApi activitiesServiceApi) { return new RemoteActivitiesRepository(activitiesServiceApi); } - } - diff --git a/src/main/java/com/owncloud/android/ui/activities/data/files/FileRepositories.java b/src/main/java/com/owncloud/android/ui/activities/data/files/FileRepositories.java index 5c6d51ca87..073559d99a 100644 --- a/src/main/java/com/owncloud/android/ui/activities/data/files/FileRepositories.java +++ b/src/main/java/com/owncloud/android/ui/activities/data/files/FileRepositories.java @@ -1,4 +1,4 @@ -/** +/* * Nextcloud Android client application * * Copyright (C) 2018 Edvard Holst @@ -20,7 +20,7 @@ package com.owncloud.android.ui.activities.data.files; import androidx.annotation.NonNull; -public class FileRepositories { +public final class FileRepositories { private FileRepositories() { // No instance From dfc1a5e89f0d01ee4932f124c599ae4a1f84eb04 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 27 Aug 2019 12:20:34 +0200 Subject: [PATCH 2/4] codacy: Avoid instantiating new objects inside loops Signed-off-by: Andy Scherzinger --- .../datamodel/FileDataStorageManager.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java b/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java index fc2e46e8f6..444ddf7aa4 100644 --- a/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java +++ b/src/main/java/com/owncloud/android/datamodel/FileDataStorageManager.java @@ -683,6 +683,7 @@ public class FileDataStorageManager { if (c.moveToFirst()) { int lengthOfOldPath = file.getRemotePath().length(); int lengthOfOldStoragePath = defaultSavePath.length() + lengthOfOldPath; + String[] fileId = new String[1]; do { ContentValues cv = new ContentValues(); // keep construction in the loop OCFile child = createFileInstance(c); @@ -704,13 +705,11 @@ public class FileDataStorageManager { if (child.getRemotePath().equals(file.getRemotePath())) { cv.put(ProviderTableMeta.FILE_PARENT, targetParent.getFileId()); } + fileId[0] = String.valueOf(child.getFileId()); operations.add( ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI). withValues(cv). - withSelection( - ProviderTableMeta._ID + "=?", - new String[]{String.valueOf(child.getFileId())} - ) + withSelection(ProviderTableMeta._ID + "=?", fileId) .build()); } while (c.moveToNext()); @@ -802,9 +801,10 @@ public class FileDataStorageManager { ArrayList operations = new ArrayList<>(cursor.getCount()); if (cursor.moveToFirst()) { + String[] fileId = new String[1]; do { ContentValues cv = new ContentValues(); - long fileId = cursor.getLong(cursor.getColumnIndex(ProviderTableMeta._ID)); + fileId[0] = String.valueOf(cursor.getLong(cursor.getColumnIndex(ProviderTableMeta._ID))); String oldFileStoragePath = cursor.getString(cursor.getColumnIndex(ProviderTableMeta.FILE_STORAGE_PATH)); @@ -815,7 +815,7 @@ public class FileDataStorageManager { operations.add( ContentProviderOperation.newUpdate(ProviderTableMeta.CONTENT_URI). withValues(cv). - withSelection(ProviderTableMeta._ID + "=?", new String[]{String.valueOf(fileId)}) + withSelection(ProviderTableMeta._ID + "=?", fileId) .build()); } @@ -1850,6 +1850,7 @@ public class FileDataStorageManager { parentPath = parentPath.substring(0, parentPath.lastIndexOf(OCFile.PATH_SEPARATOR) + 1); Log_OC.d(TAG, "checking parents to remove conflict; STARTING with " + parentPath); + String[] projection = new String[]{ProviderTableMeta._ID}; while (parentPath.length() > 0) { String whereForDescencentsInConflict = @@ -1861,7 +1862,7 @@ public class FileDataStorageManager { if (getContentResolver() != null) { descendentsInConflict = getContentResolver().query( ProviderTableMeta.CONTENT_URI_FILE, - new String[]{ProviderTableMeta._ID}, + projection, whereForDescencentsInConflict, new String[]{account.name, parentPath + "%"}, null @@ -1870,7 +1871,7 @@ public class FileDataStorageManager { try { descendentsInConflict = getContentProviderClient().query( ProviderTableMeta.CONTENT_URI_FILE, - new String[]{ProviderTableMeta._ID}, + projection, whereForDescencentsInConflict, new String[]{account.name, parentPath + "%"}, null From 56e74d2e8a32d6c3cb9bdcc352bdae75465966a6 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 27 Aug 2019 12:38:17 +0200 Subject: [PATCH 3/4] codacy: Avoid really long methods. refactor updatePublicShareSection method Signed-off-by: Andy Scherzinger --- .../ui/fragment/ShareFileFragment.java | 265 ++++++++++-------- 1 file changed, 151 insertions(+), 114 deletions(-) diff --git a/src/main/java/com/owncloud/android/ui/fragment/ShareFileFragment.java b/src/main/java/com/owncloud/android/ui/fragment/ShareFileFragment.java index 860ab879f3..db6c95e13a 100644 --- a/src/main/java/com/owncloud/android/ui/fragment/ShareFileFragment.java +++ b/src/main/java/com/owncloud/android/ui/fragment/ShareFileFragment.java @@ -67,13 +67,12 @@ import androidx.fragment.app.Fragment; /** * Fragment for Sharing a file with sharees (users or groups) or creating * a public link. - *

+ * * A simple {@link Fragment} subclass. - *

+ * * Activities that contain this fragment must implement the - * {@link ShareFragmentListener} interface - * to handle interaction events. - *

+ * {@link ShareFragmentListener} interface to handle interaction events. + * * Use the {@link ShareFileFragment#newInstance} factory method to * create an instance of this fragment. */ @@ -694,16 +693,9 @@ public class ShareFileFragment extends Fragment implements ShareUserListAdapter. */ private void updatePublicShareSection() { if (mPublicShare != null && ShareType.PUBLIC_LINK.equals(mPublicShare.getShareType())) { - /// public share bound -> expand section - SwitchCompat shareViaLinkSwitch = getShareViaLinkSwitch(); - if (!shareViaLinkSwitch.isChecked()) { - // set null listener before setChecked() to prevent infinite loop of calls - shareViaLinkSwitch.setOnCheckedChangeListener(null); - shareViaLinkSwitch.setChecked(true); - shareViaLinkSwitch.setOnCheckedChangeListener( - mOnShareViaLinkSwitchCheckedChangeListener - ); - } + // public share bound -> expand section + updateShareViaLinkSwitch(mOnShareViaLinkSwitchCheckedChangeListener); + getExpirationDateSection().setVisibility(View.VISIBLE); getPasswordSection().setVisibility(View.VISIBLE); if (mFile.isFolder() && !mCapabilities.getFilesSharingPublicUpload().isFalse()) { @@ -713,116 +705,161 @@ public class ShareFileFragment extends Fragment implements ShareUserListAdapter. getEditPermissionSection().setVisibility(View.GONE); } - // GetLink button - MaterialButton getLinkButton = getGetLinkButton(); - getLinkButton.getBackground().setColorFilter(ThemeUtils.primaryColor(getContext()), - PorterDuff.Mode.SRC_ATOP); - getLinkButton.setVisibility(View.VISIBLE); - getLinkButton.setOnClickListener(new View.OnClickListener() { - @Override - public void onClick(View v) { - //GetLink from the server and show ShareLinkToDialog - ((FileActivity) getActivity()).getFileOperationsHelper(). - getFileWithLink(mFile); + // init link button + initLinkButton(); - } - }); + /// update state of expiration date switch + updateExpirationDateSwitch(mPublicShare.getExpirationDate(), mOnExpirationDateInteractionListener); - /// update state of expiration date switch and message depending on expiration date - SwitchCompat expirationDateSwitch = getExpirationDateSwitch(); - // set null listener before setChecked() to prevent infinite loop of calls - expirationDateSwitch.setOnCheckedChangeListener(null); - long expirationDate = mPublicShare.getExpirationDate(); - if (expirationDate > 0) { - if (!expirationDateSwitch.isChecked()) { - expirationDateSwitch.toggle(); - } - String formattedDate = - SimpleDateFormat.getDateInstance().format( - new Date(expirationDate) - ); - getExpirationDateValue().setText(formattedDate); - } else { - if (expirationDateSwitch.isChecked()) { - expirationDateSwitch.toggle(); - } - getExpirationDateValue().setText(R.string.empty); - } - - // recover listener - expirationDateSwitch.setOnCheckedChangeListener(mOnExpirationDateInteractionListener); - - /// update state of password switch and message depending on password protection - SwitchCompat passwordSwitch = getPasswordSwitch(); - // set null listener before setChecked() to prevent infinite loop of calls - passwordSwitch.setOnCheckedChangeListener(null); - if (mPublicShare.isPasswordProtected()) { - if (!passwordSwitch.isChecked()) { - passwordSwitch.toggle(); - } - getPasswordValue().setVisibility(View.VISIBLE); - } else { - if (passwordSwitch.isChecked()) { - passwordSwitch.toggle(); - } - getPasswordValue().setVisibility(View.INVISIBLE); - } - - // recover listener - passwordSwitch.setOnCheckedChangeListener(mOnPasswordInteractionListener); + /// update state of password switch + updatePasswordSwitch(mPublicShare.isPasswordProtected(), mOnPasswordInteractionListener); /// update state of the edit permission switch - SwitchCompat editPermissionSwitch = getEditPermissionSwitch(); - - // set null listener before setChecked() to prevent infinite loop of calls - editPermissionSwitch.setOnCheckedChangeListener(null); - if (mPublicShare.getPermissions() > OCShare.READ_PERMISSION_FLAG) { - if (!editPermissionSwitch.isChecked()) { - editPermissionSwitch.toggle(); - } - getHideFileListingPermissionSection().setVisibility(View.VISIBLE); - } else { - if (editPermissionSwitch.isChecked()) { - editPermissionSwitch.toggle(); - } - getHideFileListingPermissionSection().setVisibility(View.GONE); - } - - // recover listener - editPermissionSwitch.setOnCheckedChangeListener(mOnEditPermissionInteractionListener); + updatePermissionSwitch(mPublicShare.getPermissions(), mOnEditPermissionInteractionListener); /// update state of the hide file listing permission switch - SwitchCompat hideFileListingPermissionSwitch = getHideFileListingPermissionSwitch(); - - // set null listener before setChecked() to prevent infinite loop of calls - hideFileListingPermissionSwitch.setOnCheckedChangeListener(null); - - boolean readOnly = (mPublicShare.getPermissions() & OCShare.READ_PERMISSION_FLAG) != 0; - hideFileListingPermissionSwitch.setChecked(!readOnly); - - // recover listener - hideFileListingPermissionSwitch.setOnCheckedChangeListener( - mOnHideFileListingPermissionInteractionListener - ); + updateHideFileListingPermissionSwitch(mPublicShare.getPermissions(), + mOnHideFileListingPermissionInteractionListener); } else { - /// no public share -> collapse section - SwitchCompat shareViaLinkSwitch = getShareViaLinkSwitch(); - if (shareViaLinkSwitch.isChecked()) { - shareViaLinkSwitch.setOnCheckedChangeListener(null); - getShareViaLinkSwitch().setChecked(false); - shareViaLinkSwitch.setOnCheckedChangeListener( - mOnShareViaLinkSwitchCheckedChangeListener - ); - } - getExpirationDateSection().setVisibility(View.GONE); - getPasswordSection().setVisibility(View.GONE); - getEditPermissionSection().setVisibility(View.GONE); - getHideFileListingPermissionSection().setVisibility(View.GONE); - getGetLinkButton().setVisibility(View.GONE); + // no public share -> collapse section + collapsePublicShareSection(); } } + private void updateShareViaLinkSwitch( + CompoundButton.OnCheckedChangeListener onShareViaLinkSwitchCheckedChangeListener) { + SwitchCompat shareViaLinkSwitch = getShareViaLinkSwitch(); + if (!shareViaLinkSwitch.isChecked()) { + // set null listener before setChecked() to prevent infinite loop of calls + shareViaLinkSwitch.setOnCheckedChangeListener(null); + shareViaLinkSwitch.setChecked(true); + shareViaLinkSwitch.setOnCheckedChangeListener(onShareViaLinkSwitchCheckedChangeListener); + } + } + + private void updateHideFileListingPermissionSwitch( + int permissions, + OnHideFileListingPermissionInteractionListener onHideFileListingPermissionInteractionListener) + { + SwitchCompat hideFileListingPermissionSwitch = getHideFileListingPermissionSwitch(); + + // set null listener before setChecked() to prevent infinite loop of calls + hideFileListingPermissionSwitch.setOnCheckedChangeListener(null); + + boolean readOnly = (permissions & OCShare.READ_PERMISSION_FLAG) != 0; + hideFileListingPermissionSwitch.setChecked(!readOnly); + + // recover listener + hideFileListingPermissionSwitch.setOnCheckedChangeListener(onHideFileListingPermissionInteractionListener); + } + + private void updatePermissionSwitch( + int permissions, + OnEditPermissionInteractionListener onEditPermissionInteractionListener) + { + SwitchCompat editPermissionSwitch = getEditPermissionSwitch(); + + // set null listener before setChecked() to prevent infinite loop of calls + editPermissionSwitch.setOnCheckedChangeListener(null); + + if (permissions > OCShare.READ_PERMISSION_FLAG) { + if (!editPermissionSwitch.isChecked()) { + editPermissionSwitch.toggle(); + } + getHideFileListingPermissionSection().setVisibility(View.VISIBLE); + } else { + if (editPermissionSwitch.isChecked()) { + editPermissionSwitch.toggle(); + } + getHideFileListingPermissionSection().setVisibility(View.GONE); + } + + // recover listener + editPermissionSwitch.setOnCheckedChangeListener(onEditPermissionInteractionListener); + } + + private void updatePasswordSwitch( + boolean isPasswordProtected, + OnPasswordInteractionListener onPasswordInteractionListener) + { + SwitchCompat passwordSwitch = getPasswordSwitch(); + // set null listener before setChecked() to prevent infinite loop of calls + passwordSwitch.setOnCheckedChangeListener(null); + + if (isPasswordProtected) { + if (!passwordSwitch.isChecked()) { + passwordSwitch.toggle(); + } + getPasswordValue().setVisibility(View.VISIBLE); + } else { + if (passwordSwitch.isChecked()) { + passwordSwitch.toggle(); + } + getPasswordValue().setVisibility(View.INVISIBLE); + } + + // recover listener + passwordSwitch.setOnCheckedChangeListener(onPasswordInteractionListener); + } + + private void updateExpirationDateSwitch( + long expirationDate, + OnExpirationDateInteractionListener onExpirationDateInteractionListener) + { + SwitchCompat expirationDateSwitch = getExpirationDateSwitch(); + // set null listener before setChecked() to prevent infinite loop of calls + expirationDateSwitch.setOnCheckedChangeListener(null); + + if (expirationDate > 0) { + if (!expirationDateSwitch.isChecked()) { + expirationDateSwitch.toggle(); + } + String formattedDate = SimpleDateFormat.getDateInstance().format(new Date(expirationDate)); + getExpirationDateValue().setText(formattedDate); + } else { + if (expirationDateSwitch.isChecked()) { + expirationDateSwitch.toggle(); + } + getExpirationDateValue().setText(R.string.empty); + } + + // recover listener + expirationDateSwitch.setOnCheckedChangeListener(onExpirationDateInteractionListener); + } + + private void initLinkButton() { + MaterialButton getLinkButton = getGetLinkButton(); + getLinkButton.getBackground().setColorFilter(ThemeUtils.primaryColor(getContext()), + PorterDuff.Mode.SRC_ATOP); + getLinkButton.setVisibility(View.VISIBLE); + getLinkButton.setOnClickListener(new View.OnClickListener() { + @Override + public void onClick(View v) { + //GetLink from the server and show ShareLinkToDialog + ((FileActivity) getActivity()).getFileOperationsHelper(). + getFileWithLink(mFile); + + } + }); + } + + private void collapsePublicShareSection() { + SwitchCompat shareViaLinkSwitch = getShareViaLinkSwitch(); + if (shareViaLinkSwitch.isChecked()) { + shareViaLinkSwitch.setOnCheckedChangeListener(null); + getShareViaLinkSwitch().setChecked(false); + shareViaLinkSwitch.setOnCheckedChangeListener( + mOnShareViaLinkSwitchCheckedChangeListener + ); + } + getExpirationDateSection().setVisibility(View.GONE); + getPasswordSection().setVisibility(View.GONE); + getEditPermissionSection().setVisibility(View.GONE); + getHideFileListingPermissionSection().setVisibility(View.GONE); + getGetLinkButton().setVisibility(View.GONE); + } + // BEWARE: following methods will fail with NullPointerException if called before onCreateView() finishes From cf02e861b870ec561607aae0b331fc87db897b62 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Tue, 27 Aug 2019 12:45:14 +0200 Subject: [PATCH 4/4] codacy: Classes implementing Serializable should set a serialVersionUID Signed-off-by: Andy Scherzinger --- .../java/com/owncloud/android/operations/UploadException.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/com/owncloud/android/operations/UploadException.java b/src/main/java/com/owncloud/android/operations/UploadException.java index 49feb5f03c..c104459f9a 100644 --- a/src/main/java/com/owncloud/android/operations/UploadException.java +++ b/src/main/java/com/owncloud/android/operations/UploadException.java @@ -21,6 +21,8 @@ package com.owncloud.android.operations; class UploadException extends Exception { + private static final long serialVersionUID = 5931153844211429915L; + UploadException() { super(); }