improve code base from review comments

Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>

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<Response> = 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"
+    }
 }

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

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
+                    loadList(state, mimeTypeSelectionFilter)
-                    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()
                 }
                 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()

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 {
+) : 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)
-}

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()
     }

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<RemoteFileBrowserItem> {
+        private val alphanumComparator = AlphanumComparator<RemoteFileBrowserItem>()
 
         override fun compare(left: RemoteFileBrowserItem, right: RemoteFileBrowserItem): Int {
             return if (!left.isFile && !right.isFile) {
-                return multiplier * AlphanumComparator<RemoteFileBrowserItem>()
+                return multiplier * alphanumComparator.compare(left.path, right.path)
-                    .compareRemoteFileBrowserItem(left, right)
             } else if (!left.isFile) {
                 -1
             } else if (!right.isFile) {
                 1
             } else {
-                multiplier * AlphanumComparator<RemoteFileBrowserItem>().compareRemoteFileBrowserItem(left, right)
+                multiplier * alphanumComparator.compare(left.path, right.path)
             }
         }
     }

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<T> implements Comparator<T>, 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());
     }

detekt.yml

@@ -1,5 +1,5 @@
 build:
-  maxIssues: 89
+  maxIssues: 88
   weights:
     # complexity: 2
     # LongParameterList: 1