From 3e99dc065bca0be2867f3c17dd2901facedcbd37 Mon Sep 17 00:00:00 2001 From: Andy Scherzinger Date: Thu, 9 Jun 2022 12:02:15 +0200 Subject: [PATCH] improve code base from review comments Signed-off-by: Andy Scherzinger --- .../webdav/ReadFolderListingOperation.kt | 52 ++++++++------- .../talk/controllers/ProfileController.kt | 7 +- .../activities/RemoteFileBrowserActivity.kt | 64 +++++++++++-------- .../model/RemoteFileBrowserItem.kt | 7 +- .../RemoteFileBrowserItemsViewModel.kt | 2 +- .../talk/utils/FileSortOrderByName.kt | 6 +- .../daveKoeller/AlphanumComparator.java | 9 --- detekt.yml | 2 +- 8 files changed, 76 insertions(+), 73 deletions(-) diff --git a/app/src/main/java/com/nextcloud/talk/components/filebrowser/webdav/ReadFolderListingOperation.kt b/app/src/main/java/com/nextcloud/talk/components/filebrowser/webdav/ReadFolderListingOperation.kt index 0a511607b..b14035ca2 100644 --- a/app/src/main/java/com/nextcloud/talk/components/filebrowser/webdav/ReadFolderListingOperation.kt +++ b/app/src/main/java/com/nextcloud/talk/components/filebrowser/webdav/ReadFolderListingOperation.kt @@ -54,6 +54,26 @@ class ReadFolderListingOperation(okHttpClient: OkHttpClient, currentUser: UserEn private val url: String private val depth: Int private val basePath: String + + init { + val okHttpClientBuilder: OkHttpClient.Builder = okHttpClient.newBuilder() + okHttpClientBuilder.followRedirects(false) + okHttpClientBuilder.followSslRedirects(false) + okHttpClientBuilder.authenticator( + MagicAuthenticator( + ApiUtils.getCredentials( + currentUser.username, + currentUser.token + ), + "Authorization" + ) + ) + this.okHttpClient = okHttpClientBuilder.build() + basePath = currentUser.baseUrl + DavUtils.DAV_PATH + currentUser.userId + url = basePath + path + this.depth = depth + } + fun readRemotePath(): DavResponse { val davResponse = DavResponse() val memberElements: MutableList = ArrayList() @@ -96,29 +116,6 @@ class ReadFolderListingOperation(okHttpClient: OkHttpClient, currentUser: UserEn return davResponse } - companion object { - private const val TAG = "ReadFilesystemOperation" - } - - init { - val okHttpClientBuilder: OkHttpClient.Builder = okHttpClient.newBuilder() - okHttpClientBuilder.followRedirects(false) - okHttpClientBuilder.followSslRedirects(false) - okHttpClientBuilder.authenticator( - MagicAuthenticator( - ApiUtils.getCredentials( - currentUser.username, - currentUser.token - ), - "Authorization" - ) - ) - this.okHttpClient = okHttpClientBuilder.build() - basePath = currentUser.baseUrl + DavUtils.DAV_PATH + currentUser.userId - url = basePath + path - this.depth = depth - } - private fun getModelFromResponse(response: Response, remotePath: String): RemoteFileBrowserItem { val remoteFileBrowserItem = RemoteFileBrowserItem() remoteFileBrowserItem.path = Uri.decode(remotePath) @@ -127,7 +124,9 @@ class ReadFolderListingOperation(okHttpClient: OkHttpClient, currentUser: UserEn for (property in properties) { mapPropertyToBrowserFile(property, remoteFileBrowserItem) } - if (remoteFileBrowserItem.permissions != null && remoteFileBrowserItem.permissions!!.contains("R")) { + if (remoteFileBrowserItem.permissions != null && + remoteFileBrowserItem.permissions!!.contains(READ_PERMISSION) + ) { remoteFileBrowserItem.isAllowedToReShare = true } if (TextUtils.isEmpty(remoteFileBrowserItem.mimeType) && !remoteFileBrowserItem.isFile) { @@ -172,4 +171,9 @@ class ReadFolderListingOperation(okHttpClient: OkHttpClient, currentUser: UserEn } } } + + companion object { + private const val TAG = "ReadFilesystemOperation" + private const val READ_PERMISSION = "R" + } } diff --git a/app/src/main/java/com/nextcloud/talk/controllers/ProfileController.kt b/app/src/main/java/com/nextcloud/talk/controllers/ProfileController.kt index 3616efee1..fa604250e 100644 --- a/app/src/main/java/com/nextcloud/talk/controllers/ProfileController.kt +++ b/app/src/main/java/com/nextcloud/talk/controllers/ProfileController.kt @@ -514,11 +514,11 @@ class ProfileController : NewBaseController(R.layout.controller_profile) { try { FileUtils.removeTempCacheFile( this.context!!, - "photos/avatar.png" + AVATAR_PATH ) file = FileUtils.getTempCacheFile( this.context!!, - "photos/avatar.png" + AVATAR_PATH ) try { FileOutputStream(file).use { out -> bitmap.compress(Bitmap.CompressFormat.PNG, FULL_QUALITY, out) } @@ -558,7 +558,7 @@ class ProfileController : NewBaseController(R.layout.controller_profile) { } else if (resultCode == ImagePicker.RESULT_ERROR) { Toast.makeText(activity, getError(intent), Toast.LENGTH_SHORT).show() } else { - Toast.makeText(activity, "Task Cancelled", Toast.LENGTH_SHORT).show() + Log.i(TAG, "Task Cancelled") } } @@ -808,6 +808,7 @@ class ProfileController : NewBaseController(R.layout.controller_profile) { companion object { private const val TAG: String = "ProfileController" + private const val AVATAR_PATH = "photos/avatar.png" private const val REQUEST_CODE_SELECT_REMOTE_FILES = 22 private const val DEFAULT_CACHE_SIZE: Int = 20 private const val DEFAULT_RETRIES: Long = 3 diff --git a/app/src/main/java/com/nextcloud/talk/remotefilebrowser/activities/RemoteFileBrowserActivity.kt b/app/src/main/java/com/nextcloud/talk/remotefilebrowser/activities/RemoteFileBrowserActivity.kt index 53a6372e9..3e3caf29e 100644 --- a/app/src/main/java/com/nextcloud/talk/remotefilebrowser/activities/RemoteFileBrowserActivity.kt +++ b/app/src/main/java/com/nextcloud/talk/remotefilebrowser/activities/RemoteFileBrowserActivity.kt @@ -113,32 +113,7 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe showEmpty() } is RemoteFileBrowserItemsViewModel.LoadedState -> { - val remoteFileBrowserItems = state.items - Log.d(TAG, "Items received: $remoteFileBrowserItems") - - // TODO make showGrid based on preferences (when available) - val showGrid = false - val layoutManager = if (showGrid) { - GridLayoutManager(this, SPAN_COUNT) - } else { - LinearLayoutManager(this, LinearLayoutManager.VERTICAL, false) - } - - // TODO do not needlessly recreate adapter if it can be reused - val adapter = RemoteFileBrowserItemsAdapter( - showGrid = showGrid, - mimeTypeSelectionFilter = mimeTypeSelectionFilter, - userEntity = currentUserProvider.currentUser!!, - selectionInterface = this, - onItemClicked = viewModel::onItemClicked - ) - adapter.items = remoteFileBrowserItems - - binding.recyclerView.adapter = adapter - binding.recyclerView.layoutManager = layoutManager - binding.recyclerView.setHasFixedSize(true) - - showList() + loadList(state, mimeTypeSelectionFilter) } is RemoteFileBrowserItemsViewModel.FinishState -> { finishWithResult(state.selectedPaths) @@ -164,6 +139,38 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe } } + private fun loadList( + state: RemoteFileBrowserItemsViewModel.LoadedState, + mimeTypeSelectionFilter: String? + ) { + val remoteFileBrowserItems = state.items + Log.d(TAG, "Items received: $remoteFileBrowserItems") + + // TODO make showGrid based on preferences (when available) + val showGrid = false + val layoutManager = if (showGrid) { + GridLayoutManager(this, SPAN_COUNT) + } else { + LinearLayoutManager(this, LinearLayoutManager.VERTICAL, false) + } + + // TODO do not needlessly recreate adapter if it can be reused + val adapter = RemoteFileBrowserItemsAdapter( + showGrid = showGrid, + mimeTypeSelectionFilter = mimeTypeSelectionFilter, + userEntity = currentUserProvider.currentUser!!, + selectionInterface = this, + onItemClicked = viewModel::onItemClicked + ) + adapter.items = remoteFileBrowserItems + + binding.recyclerView.adapter = adapter + binding.recyclerView.layoutManager = layoutManager + binding.recyclerView.setHasFixedSize(true) + + showList() + } + override fun onCreateOptionsMenu(menu: Menu?): Boolean { super.onCreateOptionsMenu(menu) menuInflater.inflate(R.menu.menu_share_files, menu) @@ -171,6 +178,11 @@ class RemoteFileBrowserActivity : AppCompatActivity(), SelectionInterface, Swipe return true } + override fun onBackPressed() { + setResult(Activity.RESULT_CANCELED) + super.onBackPressed() + } + override fun onResume() { super.onResume() refreshCurrentPath() diff --git a/app/src/main/java/com/nextcloud/talk/remotefilebrowser/model/RemoteFileBrowserItem.kt b/app/src/main/java/com/nextcloud/talk/remotefilebrowser/model/RemoteFileBrowserItem.kt index 3904cd18c..e4eb2c044 100644 --- a/app/src/main/java/com/nextcloud/talk/remotefilebrowser/model/RemoteFileBrowserItem.kt +++ b/app/src/main/java/com/nextcloud/talk/remotefilebrowser/model/RemoteFileBrowserItem.kt @@ -23,11 +23,9 @@ package com.nextcloud.talk.remotefilebrowser.model import android.os.Parcelable -import com.bluelinelabs.logansquare.annotation.JsonObject import kotlinx.android.parcel.Parcelize @Parcelize -@JsonObject data class RemoteFileBrowserItem( var path: String? = null, var displayName: String? = null, @@ -43,7 +41,4 @@ data class RemoteFileBrowserItem( var isEncrypted: Boolean = false, var permissions: String? = null, var isAllowedToReShare: Boolean = false -) : Parcelable { - // This constructor is added to work with the 'com.bluelinelabs.logansquare.annotation.JsonObject' - constructor() : this(null, null, null, 0, 0, false, null, false, false, false, null, false) -} +) : Parcelable diff --git a/app/src/main/java/com/nextcloud/talk/remotefilebrowser/viewmodels/RemoteFileBrowserItemsViewModel.kt b/app/src/main/java/com/nextcloud/talk/remotefilebrowser/viewmodels/RemoteFileBrowserItemsViewModel.kt index ef45577bd..3c0bb4cbe 100644 --- a/app/src/main/java/com/nextcloud/talk/remotefilebrowser/viewmodels/RemoteFileBrowserItemsViewModel.kt +++ b/app/src/main/java/com/nextcloud/talk/remotefilebrowser/viewmodels/RemoteFileBrowserItemsViewModel.kt @@ -150,7 +150,7 @@ class RemoteFileBrowserItemsViewModel @Inject constructor( } } - fun changePath(path: String) { + private fun changePath(path: String) { _currentPath.value = path loadItems() } diff --git a/app/src/main/java/com/nextcloud/talk/utils/FileSortOrderByName.kt b/app/src/main/java/com/nextcloud/talk/utils/FileSortOrderByName.kt index 4c5535938..2b4f3524a 100644 --- a/app/src/main/java/com/nextcloud/talk/utils/FileSortOrderByName.kt +++ b/app/src/main/java/com/nextcloud/talk/utils/FileSortOrderByName.kt @@ -40,17 +40,17 @@ class FileSortOrderByName internal constructor(name: String, ascending: Boolean) * Comparator for RemoteFileBrowserItems, sorts by name. */ class RemoteFileBrowserItemNameComparator(private val multiplier: Int) : Comparator { + private val alphanumComparator = AlphanumComparator() override fun compare(left: RemoteFileBrowserItem, right: RemoteFileBrowserItem): Int { return if (!left.isFile && !right.isFile) { - return multiplier * AlphanumComparator() - .compareRemoteFileBrowserItem(left, right) + return multiplier * alphanumComparator.compare(left.path, right.path) } else if (!left.isFile) { -1 } else if (!right.isFile) { 1 } else { - multiplier * AlphanumComparator().compareRemoteFileBrowserItem(left, right) + multiplier * alphanumComparator.compare(left.path, right.path) } } } diff --git a/app/src/main/java/third_parties/daveKoeller/AlphanumComparator.java b/app/src/main/java/third_parties/daveKoeller/AlphanumComparator.java index 9f7025136..35cd76177 100644 --- a/app/src/main/java/third_parties/daveKoeller/AlphanumComparator.java +++ b/app/src/main/java/third_parties/daveKoeller/AlphanumComparator.java @@ -24,8 +24,6 @@ package third_parties.daveKoeller; -import com.nextcloud.talk.remotefilebrowser.model.RemoteFileBrowserItem; - import java.io.Serializable; import java.math.BigInteger; import java.text.Collator; @@ -86,13 +84,6 @@ public class AlphanumComparator implements Comparator, Serializable { return chunk.toString(); } - public int compareRemoteFileBrowserItem(RemoteFileBrowserItem f1, RemoteFileBrowserItem f2) { - String s1 = f1.getPath(); - String s2 = f2.getPath(); - - return compare(s1, s2); - } - public int compare(T t1, T t2) { return compare(t1.toString(), t2.toString()); } diff --git a/detekt.yml b/detekt.yml index d4cc80a27..a5e45ac8e 100644 --- a/detekt.yml +++ b/detekt.yml @@ -1,5 +1,5 @@ build: - maxIssues: 89 + maxIssues: 88 weights: # complexity: 2 # LongParameterList: 1