Merge pull request #10191 from nextcloud/fix/some-detekt

Fix a bunch of Detekt issues

app/detekt.yml

@@ -1,5 +1,5 @@
 build:
-  maxIssues: 11
+  maxIssues: 3
   weights:
     # complexity: 2
     # LongParameterList: 1

app/src/main/java/com/nextcloud/ui/SetStatusDialogFragment.kt

@@ -81,6 +81,9 @@ private const val LAST_HOUR_OF_DAY = 23
 private const val LAST_MINUTE_OF_HOUR = 59
 private const val LAST_SECOND_OF_MINUTE = 59
 
+private const val CLEAR_AT_TYPE_PERIOD = "period"
+private const val CLEAR_AT_TYPE_END_OF = "end-of"
+
 class SetStatusDialogFragment :
     DialogFragment(),
     PredefinedStatusClickListener,
@@ -147,26 +150,7 @@ class SetStatusDialogFragment :
         accountManager = (activity as BaseActivity).userAccountManager
 
         currentStatus?.let {
-            binding.emoji.setText(it.icon)
-            binding.customStatusInput.text?.clear()
-            binding.customStatusInput.setText(it.message)
-            visualizeStatus(it.status)
-
-            if (it.clearAt > 0) {
-                binding.clearStatusAfterSpinner.visibility = View.GONE
-                binding.remainingClearTime.apply {
-                    binding.clearStatusMessageTextView.text = getString(R.string.clear_status_message)
-                    visibility = View.VISIBLE
-                    text = DisplayUtils.getRelativeTimestamp(context, it.clearAt * ONE_SECOND_IN_MILLIS, true)
-                        .toString()
-                        .decapitalize(Locale.getDefault())
-                    setOnClickListener {
-                        visibility = View.GONE
-                        binding.clearStatusAfterSpinner.visibility = View.VISIBLE
-                        binding.clearStatusMessageTextView.text = getString(R.string.clear_status_message_after)
-                    }
-                }
-            }
+            updateCurrentStatusViews(it)
         }
 
         adapter = PredefinedStatusListAdapter(this, requireContext())
@@ -183,7 +167,7 @@ class SetStatusDialogFragment :
 
         binding.clearStatus.setOnClickListener { clearStatus() }
         binding.setStatus.setOnClickListener { setStatusMessage() }
-        binding.emoji.setOnClickListener { openEmojiPopup() }
+        binding.emoji.setOnClickListener { popup.show() }
 
         popup = EmojiPopup.Builder
             .fromRootView(view)
@@ -229,83 +213,84 @@ class SetStatusDialogFragment :
         )
     }
 
-    @Suppress("ComplexMethod")
-    private fun setClearStatusAfterValue(item: Int) {
-        when (item) {
-            POS_DONT_CLEAR -> {
-                // don't clear
-                clearAt = null
+    private fun updateCurrentStatusViews(it: Status) {
+        binding.emoji.setText(it.icon)
+        binding.customStatusInput.text?.clear()
+        binding.customStatusInput.setText(it.message)
+        visualizeStatus(it.status)
+
+        if (it.clearAt > 0) {
+            binding.clearStatusAfterSpinner.visibility = View.GONE
+            binding.remainingClearTime.apply {
+                binding.clearStatusMessageTextView.text = getString(R.string.clear_status_message)
+                visibility = View.VISIBLE
+                text = DisplayUtils.getRelativeTimestamp(context, it.clearAt * ONE_SECOND_IN_MILLIS, true)
+                    .toString()
+                    .replaceFirstChar { it.lowercase(Locale.getDefault()) }
+                setOnClickListener {
+                    visibility = View.GONE
+                    binding.clearStatusAfterSpinner.visibility = View.VISIBLE
+                    binding.clearStatusMessageTextView.text = getString(R.string.clear_status_message_after)
+                }
             }
+        }
+    }
 
+    private fun setClearStatusAfterValue(item: Int) {
+        clearAt = when (item) {
+            POS_DONT_CLEAR -> null // don't clear
             POS_HALF_AN_HOUR -> {
                 // 30 minutes
-                clearAt = System.currentTimeMillis() / ONE_SECOND_IN_MILLIS + THIRTY_MINUTES * ONE_MINUTE_IN_SECONDS
+                System.currentTimeMillis() / ONE_SECOND_IN_MILLIS + THIRTY_MINUTES * ONE_MINUTE_IN_SECONDS
             }
-
             POS_AN_HOUR -> {
                 // one hour
-                clearAt =
-                    System.currentTimeMillis() / ONE_SECOND_IN_MILLIS + ONE_MINUTE_IN_SECONDS * ONE_MINUTE_IN_SECONDS
+                System.currentTimeMillis() / ONE_SECOND_IN_MILLIS + ONE_MINUTE_IN_SECONDS * ONE_MINUTE_IN_SECONDS
             }
-
             POS_FOUR_HOURS -> {
                 // four hours
-                clearAt =
-                    System.currentTimeMillis() / ONE_SECOND_IN_MILLIS
-                +FOUR_HOURS * ONE_MINUTE_IN_SECONDS * ONE_MINUTE_IN_SECONDS
+                System.currentTimeMillis() / ONE_SECOND_IN_MILLIS +
+                    FOUR_HOURS * ONE_MINUTE_IN_SECONDS * ONE_MINUTE_IN_SECONDS
             }
-
             POS_TODAY -> {
                 // today
-                val date = Calendar.getInstance().apply {
-                    set(Calendar.HOUR_OF_DAY, LAST_HOUR_OF_DAY)
-                    set(Calendar.MINUTE, LAST_MINUTE_OF_HOUR)
-                    set(Calendar.SECOND, LAST_SECOND_OF_MINUTE)
-                }
-                clearAt = date.timeInMillis / ONE_SECOND_IN_MILLIS
+                val date = getLastSecondOfToday()
+                dateToSeconds(date)
             }
-
             POS_END_OF_WEEK -> {
                 // end of week
-                val date = Calendar.getInstance().apply {
-                    set(Calendar.HOUR_OF_DAY, LAST_HOUR_OF_DAY)
-                    set(Calendar.MINUTE, LAST_MINUTE_OF_HOUR)
-                    set(Calendar.SECOND, LAST_SECOND_OF_MINUTE)
-                }
-
+                val date = getLastSecondOfToday()
                 while (date.get(Calendar.DAY_OF_WEEK) != Calendar.SUNDAY) {
                     date.add(Calendar.DAY_OF_YEAR, 1)
                 }
-
-                clearAt = date.timeInMillis / ONE_SECOND_IN_MILLIS
+                dateToSeconds(date)
             }
+            else -> clearAt
         }
     }
 
-    @Suppress("ReturnCount")
-    private fun clearAtToUnixTime(clearAt: ClearAt?): Long {
-        if (clearAt != null) {
-            if (clearAt.type.equals("period")) {
-                return System.currentTimeMillis() / ONE_SECOND_IN_MILLIS + clearAt.time.toLong()
-            } else if (clearAt.type.equals("end-of")) {
-                if (clearAt.time.equals("day")) {
-                    val date = Calendar.getInstance().apply {
-                        set(Calendar.HOUR_OF_DAY, LAST_HOUR_OF_DAY)
-                        set(Calendar.MINUTE, LAST_MINUTE_OF_HOUR)
-                        set(Calendar.SECOND, LAST_SECOND_OF_MINUTE)
-                    }
-                    return date.timeInMillis / ONE_SECOND_IN_MILLIS
-                }
-            }
+    private fun clearAtToUnixTime(clearAt: ClearAt?): Long = when {
+        clearAt?.type == CLEAR_AT_TYPE_PERIOD -> {
+            System.currentTimeMillis() / ONE_SECOND_IN_MILLIS + clearAt.time.toLong()
         }
-
-        return -1
+        clearAt?.type == CLEAR_AT_TYPE_END_OF && clearAt.time == "day" -> {
+            val date = getLastSecondOfToday()
+            dateToSeconds(date)
+        }
+        else -> -1
     }
 
-    private fun openEmojiPopup() {
-        popup.show()
+    private fun getLastSecondOfToday(): Calendar {
+        val date = Calendar.getInstance().apply {
+            set(Calendar.HOUR_OF_DAY, LAST_HOUR_OF_DAY)
+            set(Calendar.MINUTE, LAST_MINUTE_OF_HOUR)
+            set(Calendar.SECOND, LAST_SECOND_OF_MINUTE)
+        }
+        return date
     }
 
+    private fun dateToSeconds(date: Calendar) = date.timeInMillis / ONE_SECOND_IN_MILLIS
+
     private fun clearStatus() {
         asyncRunner.postQuickTask(
             ClearStatusTask(accountManager.currentOwnCloudAccount?.savedAccount, context),
@@ -425,28 +410,35 @@ class SetStatusDialogFragment :
         binding.clearStatusAfterSpinner.visibility = View.VISIBLE
         binding.clearStatusMessageTextView.text = getString(R.string.clear_status_message_after)
 
-        if (predefinedStatus.clearAt == null) {
+        val clearAt = predefinedStatus.clearAt
+        if (clearAt == null) {
             binding.clearStatusAfterSpinner.setSelection(0)
         } else {
-            val clearAt = predefinedStatus.clearAt!!
-            if (clearAt.type.equals("period")) {
-                when (clearAt.time) {
-                    "1800" -> binding.clearStatusAfterSpinner.setSelection(POS_HALF_AN_HOUR)
-                    "3600" -> binding.clearStatusAfterSpinner.setSelection(POS_AN_HOUR)
-                    "14400" -> binding.clearStatusAfterSpinner.setSelection(POS_FOUR_HOURS)
-                    else -> binding.clearStatusAfterSpinner.setSelection(POS_DONT_CLEAR)
-                }
-            } else if (clearAt.type.equals("end-of")) {
-                when (clearAt.time) {
-                    "day" -> binding.clearStatusAfterSpinner.setSelection(POS_TODAY)
-                    "week" -> binding.clearStatusAfterSpinner.setSelection(POS_END_OF_WEEK)
-                    else -> binding.clearStatusAfterSpinner.setSelection(POS_DONT_CLEAR)
-                }
+            when (clearAt.type) {
+                CLEAR_AT_TYPE_PERIOD -> updateClearAtViewsForPeriod(clearAt)
+                CLEAR_AT_TYPE_END_OF -> updateClearAtViewsForEndOf(clearAt)
             }
         }
         setClearStatusAfterValue(binding.clearStatusAfterSpinner.selectedItemPosition)
     }
 
+    private fun updateClearAtViewsForPeriod(clearAt: ClearAt) {
+        when (clearAt.time) {
+            "1800" -> binding.clearStatusAfterSpinner.setSelection(POS_HALF_AN_HOUR)
+            "3600" -> binding.clearStatusAfterSpinner.setSelection(POS_AN_HOUR)
+            "14400" -> binding.clearStatusAfterSpinner.setSelection(POS_FOUR_HOURS)
+            else -> binding.clearStatusAfterSpinner.setSelection(POS_DONT_CLEAR)
+        }
+    }
+
+    private fun updateClearAtViewsForEndOf(clearAt: ClearAt) {
+        when (clearAt.time) {
+            "day" -> binding.clearStatusAfterSpinner.setSelection(POS_TODAY)
+            "week" -> binding.clearStatusAfterSpinner.setSelection(POS_END_OF_WEEK)
+            else -> binding.clearStatusAfterSpinner.setSelection(POS_DONT_CLEAR)
+        }
+    }
+
     @VisibleForTesting
     fun setPredefinedStatus(predefinedStatus: ArrayList<PredefinedStatus>) {
         adapter.list = predefinedStatus

app/src/main/java/com/owncloud/android/ui/activity/SyncedFoldersActivity.kt

@@ -344,6 +344,7 @@ class SyncedFoldersActivity :
      * @param mediaFolders  the media folders
      * @return the merged list of SyncedFolderItems
      */
+    @Suppress("NestedBlockDepth") // legacy code
     private fun mergeFolderData(
         syncedFolders: List<SyncedFolder>,
         mediaFolders: List<MediaFolder>

app/src/main/java/com/owncloud/android/utils/UriUtils.kt

@@ -50,41 +50,42 @@ object UriUtils {
         return displayName?.replace("/".toRegex(), "-")
     }
 
+    @Suppress("TooGenericExceptionCaught")
     private fun getDisplayNameFromContentResolver(uri: Uri, context: Context): String? {
+        val mimeType = context.contentResolver.getType(uri) ?: return null
+        val displayNameColumn: String = getDisplayNameColumnForMimeType(mimeType)
         var displayName: String? = null
-        val mimeType = context.contentResolver.getType(uri)
-        if (mimeType != null) {
-            val displayNameColumn: String = when {
-                MimeTypeUtil.isImage(mimeType) -> {
-                    MediaStore.Images.ImageColumns.DISPLAY_NAME
-                }
-                MimeTypeUtil.isVideo(mimeType) -> {
-                    MediaStore.Video.VideoColumns.DISPLAY_NAME
-                }
-                MimeTypeUtil.isAudio(mimeType) -> {
-                    MediaStore.Audio.AudioColumns.DISPLAY_NAME
-                }
-                else -> {
-                    MediaStore.Files.FileColumns.DISPLAY_NAME
+        try {
+            context.contentResolver.query(
+                uri, arrayOf(displayNameColumn),
+                null,
+                null,
+                null
+            ).use { cursor ->
+                if (cursor != null) {
+                    cursor.moveToFirst()
+                    displayName = cursor.getString(cursor.getColumnIndexOrThrow(displayNameColumn))
                 }
             }
-            try {
-                context.contentResolver.query(
-                    uri, arrayOf(displayNameColumn),
-                    null,
-                    null,
-                    null
-                ).use { cursor ->
-                    if (cursor != null) {
-                        cursor.moveToFirst()
-                        displayName = cursor.getString(cursor.getColumnIndexOrThrow(displayNameColumn))
-                    }
-                }
-            } catch (e: Exception) {
-                Log_OC.e(TAG, "Could not retrieve display name for $uri")
-                // nothing else, displayName keeps null
-            }
+        } catch (e: Exception) {
+            Log_OC.e(TAG, "Could not retrieve display name for $uri")
+            // nothing else, displayName keeps null
         }
         return displayName
     }
+
+    private fun getDisplayNameColumnForMimeType(mimeType: String?) = when {
+        MimeTypeUtil.isImage(mimeType) -> {
+            MediaStore.Images.ImageColumns.DISPLAY_NAME
+        }
+        MimeTypeUtil.isVideo(mimeType) -> {
+            MediaStore.Video.VideoColumns.DISPLAY_NAME
+        }
+        MimeTypeUtil.isAudio(mimeType) -> {
+            MediaStore.Audio.AudioColumns.DISPLAY_NAME
+        }
+        else -> {
+            MediaStore.Files.FileColumns.DISPLAY_NAME
+        }
+    }
 }

app/src/main/java/com/owncloud/android/utils/glide/HttpStreamFetcher.kt

@@ -54,14 +54,7 @@ class HttpStreamFetcher internal constructor(
                 get.setRequestHeader(RemoteOperation.OCS_API_HEADER, RemoteOperation.OCS_API_HEADER_VALUE)
                 val status = client.executeMethod(get)
                 if (status == HttpStatus.SC_OK) {
-                    val byteOutputStream = ByteArrayOutputStream()
-                    get.responseBodyAsStream.use { input ->
-                        byteOutputStream.use { output ->
-                            input.copyTo(output)
-                        }
-                    }
-
-                    return ByteArrayInputStream(byteOutputStream.toByteArray())
+                    return getResponseAsInputStream(get)
                 } else {
                     client.exhaustResponse(get.responseBodyAsStream)
                 }
@@ -74,6 +67,17 @@ class HttpStreamFetcher internal constructor(
         return null
     }
 
+    private fun getResponseAsInputStream(getMethod: GetMethod): ByteArrayInputStream {
+        val byteOutputStream = ByteArrayOutputStream()
+        getMethod.responseBodyAsStream.use { input ->
+            byteOutputStream.use { output ->
+                input.copyTo(output)
+            }
+        }
+
+        return ByteArrayInputStream(byteOutputStream.toByteArray())
+    }
+
     override fun cleanup() {
         Log_OC.i(TAG, "Cleanup")
     }

app/src/test/java/com/owncloud/android/ui/adapter/ShareeListAdapterTest.kt

@@ -48,54 +48,43 @@ class ShareeListAdapterTest {
     @Mock
     private lateinit var themeAvatarUtils: ThemeAvatarUtils
 
+    private val orderedShares = listOf(
+        OCShare("/1").apply {
+            shareType = ShareType.EMAIL
+            sharedDate = 1004
+        },
+        OCShare("/2").apply {
+            shareType = ShareType.PUBLIC_LINK
+            sharedDate = 1003
+        },
+        OCShare("/3").apply {
+            shareType = ShareType.PUBLIC_LINK
+            sharedDate = 1001
+        },
+        OCShare("/4").apply {
+            shareType = ShareType.EMAIL
+            sharedDate = 1000
+        },
+        OCShare("/5").apply {
+            shareType = ShareType.USER
+            sharedDate = 80
+        },
+        OCShare("/6").apply {
+            shareType = ShareType.CIRCLE
+            sharedDate = 20
+        }
+    )
+
     @Test
-    @Suppress("LongMethod")
     fun testSorting() {
         MockitoAnnotations.openMocks(this)
         val resources = Mockito.mock(Resources::class.java)
         Mockito.`when`(context!!.resources).thenReturn(resources)
         Mockito.`when`(fileActivity!!.resources).thenReturn(resources)
-        val expectedSortOrder: MutableList<OCShare?> = ArrayList()
-        expectedSortOrder.add(
-            OCShare("/1").apply {
-                shareType = ShareType.EMAIL
-                sharedDate = 1004
-            }
-        )
-        expectedSortOrder.add(
-            OCShare("/2").apply {
-                shareType = ShareType.PUBLIC_LINK
-                sharedDate = 1003
-            }
-        )
-        expectedSortOrder.add(
-            OCShare("/3").apply {
-                shareType = ShareType.PUBLIC_LINK
-                sharedDate = 1001
-            }
-        )
-        expectedSortOrder.add(
-            OCShare("/4").apply {
-                shareType = ShareType.EMAIL
-                sharedDate = 1000
-            }
-        )
-        expectedSortOrder.add(
-            OCShare("/5").apply {
-                shareType = ShareType.USER
-                sharedDate = 80
-            }
-        )
-        expectedSortOrder.add(
-            OCShare("/6").apply {
-                shareType = ShareType.CIRCLE
-                sharedDate = 20
-            }
-        )
 
-        val randomOrder: MutableList<OCShare?> = ArrayList(expectedSortOrder)
-        randomOrder.shuffle()
+        val randomOrder = orderedShares.shuffled()
         val user = AnonymousUser("nextcloud")
+
         val sut = ShareeListAdapter(
             fileActivity,
             randomOrder,
@@ -108,20 +97,24 @@ class ShareeListAdapterTest {
         sut.sortShares()
 
         // compare
+        assertSort(sut.shares)
+    }
+
+    private fun assertSort(shares: MutableList<OCShare>) {
         var compare = true
         var i = 0
-        while (i < expectedSortOrder.size && compare) {
-            compare = expectedSortOrder[i] === sut.shares[i]
+        while (i < orderedShares.size && compare) {
+            compare = orderedShares[i] === shares[i]
             i++
         }
         if (!compare) {
             println("Expected:")
-            for (item in expectedSortOrder) {
-                println(item!!.path + " " + item.shareType + " " + item.sharedDate)
+            for (item in orderedShares) {
+                println(item.path + " " + item.shareType + " " + item.sharedDate)
             }
             println()
             println("Actual:")
-            for (item in sut.shares) {
+            for (item in shares) {
                 println(item.path + " " + item.shareType + " " + item.sharedDate)
             }
         }