Merge pull request #11092 from nextcloud/fix/copy-move-same-path

FolderPickerActivity: disable copy/move to the same folder

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

@@ -53,6 +53,7 @@ import com.owncloud.android.utils.DataHolderUtil
 import com.owncloud.android.utils.DisplayUtils
 import com.owncloud.android.utils.ErrorMessageAdapter
 import com.owncloud.android.utils.FileSortOrder
+import com.owncloud.android.utils.PathUtils
 import java.io.File
 import javax.inject.Inject
 
@@ -74,6 +75,9 @@ open class FolderPickerActivity :
     private var mChooseBtn: MaterialButton? = null
     private var caption: String? = null
 
+    private var mAction: String? = null
+    private var mTargetFilePaths: ArrayList<String>? = null
+
     @Inject
     lateinit var localBroadcastManager: LocalBroadcastManager
 
@@ -95,8 +99,9 @@ open class FolderPickerActivity :
             View.VISIBLE
         findViewById<View>(R.id.switch_grid_view_button).visibility =
             View.GONE
-        if (intent.getStringExtra(EXTRA_ACTION) != null) {
-            when (intent.getStringExtra(EXTRA_ACTION)) {
+        mAction = intent.getStringExtra(EXTRA_ACTION)
+        if (mAction != null) {
+            when (mAction) {
                 MOVE -> {
                     caption = resources.getText(R.string.move_to).toString()
                     mSearchOnlyFolders = true
@@ -118,9 +123,8 @@ open class FolderPickerActivity :
         } else {
             caption = themeUtils.getDefaultDisplayNameForRootFolder(this)
         }
-        if (intent.getParcelableExtra<Parcelable?>(EXTRA_CURRENT_FOLDER) != null) {
-            file = intent.getParcelableExtra(EXTRA_CURRENT_FOLDER)
-        }
+        mTargetFilePaths = intent.getStringArrayListExtra(EXTRA_FILE_PATHS)
+
         if (savedInstanceState == null) {
             createFragments()
         }
@@ -146,7 +150,7 @@ open class FolderPickerActivity :
             val listOfFolders = listOfFilesFragment
             listOfFolders!!.listDirectory(folder, false, false)
             startSyncFolderOperation(folder, false)
-            updateNavigationElementsInActionBar()
+            updateUiElements()
         }
     }
 
@@ -205,7 +209,7 @@ open class FolderPickerActivity :
      */
     override fun onBrowsedDownTo(directory: OCFile) {
         file = directory
-        updateNavigationElementsInActionBar()
+        updateUiElements()
         // Sync Folder
         startSyncFolderOperation(directory, false)
     }
@@ -241,6 +245,9 @@ open class FolderPickerActivity :
         // refresh list of files
         refreshListOfFilesFragment(false)
 
+        file = listOfFilesFragment?.currentFile
+        updateUiElements()
+
         // Listen for sync messages
         val syncIntentFilter = IntentFilter(FileSyncAdapter.EVENT_FULL_SYNC_START)
         syncIntentFilter.addAction(FileSyncAdapter.EVENT_FULL_SYNC_END)
@@ -317,7 +324,7 @@ open class FolderPickerActivity :
             val root = storageManager.getFileByPath(OCFile.ROOT_PATH)
             listOfFiles.listDirectory(root, false, false)
             file = listOfFiles.currentFile
-            updateNavigationElementsInActionBar()
+            updateUiElements()
             startSyncFolderOperation(root, false)
         }
     }
@@ -331,7 +338,30 @@ open class FolderPickerActivity :
                 return
             }
             file = listOfFiles.currentFile
-            updateNavigationElementsInActionBar()
+            updateUiElements()
+        }
+    }
+
+    private fun updateUiElements() {
+        toggleChooseEnabled()
+        updateNavigationElementsInActionBar()
+    }
+
+    private fun toggleChooseEnabled() {
+        mChooseBtn?.isEnabled = checkFolderSelectable()
+    }
+
+    // for copy and move, disable selecting parent folder of target files
+    private fun checkFolderSelectable(): Boolean {
+        return when {
+            mAction != COPY && mAction != MOVE -> true
+            mTargetFilePaths.isNullOrEmpty() -> true
+            file?.isFolder != true -> true
+            // all of the target files are already in the selected directory
+            mTargetFilePaths!!.all { PathUtils.isDirectParent(file.remotePath, it) } -> false
+            // some of the target files are parents of the selected folder
+            mTargetFilePaths!!.any { PathUtils.isAncestor(it, file.remotePath) } -> false
+            else -> true
         }
     }
 
@@ -378,9 +408,8 @@ open class FolderPickerActivity :
             if (targetFiles != null) {
                 resultData.putParcelableArrayListExtra(EXTRA_FILES, targetFiles)
             }
-            val targetFilePaths = i.getStringArrayListExtra(EXTRA_FILE_PATHS)
-            if (targetFilePaths != null) {
-                resultData.putStringArrayListExtra(EXTRA_FILE_PATHS, targetFilePaths)
+            mTargetFilePaths.let {
+                resultData.putStringArrayListExtra(EXTRA_FILE_PATHS, it)
             }
             setResult(RESULT_OK, resultData)
             finish()
@@ -560,9 +589,6 @@ open class FolderPickerActivity :
         @JvmField
         val EXTRA_ACTION = FolderPickerActivity::class.java.canonicalName?.plus(".EXTRA_ACTION")
 
-        @JvmField
-        val EXTRA_CURRENT_FOLDER = FolderPickerActivity::class.java.canonicalName?.plus(".EXTRA_CURRENT_FOLDER")
-
         const val MOVE = "MOVE"
         const val COPY = "COPY"
         const val CHOOSE_LOCATION = "CHOOSE_LOCATION"

app/src/main/java/com/owncloud/android/ui/fragment/OCFileListFragment.java

@@ -1227,7 +1227,6 @@ public class OCFileListFragment extends ExtendedListFragment implements
             paths.add(file.getRemotePath());
         }
         action.putStringArrayListExtra(FolderPickerActivity.EXTRA_FILE_PATHS, paths);
-        action.putExtra(FolderPickerActivity.EXTRA_CURRENT_FOLDER, mFile);
         action.putExtra(FolderPickerActivity.EXTRA_ACTION, extraAction);
         getActivity().startActivityForResult(action, requestCode);
     }

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

@@ -0,0 +1,49 @@
+/*
+ * Nextcloud Android client application
+ *
+ *  @author Álvaro Brey
+ *  Copyright (C) 2022 Álvaro Brey
+ *  Copyright (C) 2022 Nextcloud GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or 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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package com.owncloud.android.utils
+
+import com.owncloud.android.datamodel.OCFile
+import java.io.File
+
+object PathUtils {
+    /**
+     * Returns `true` if [folderPath] is a direct parent of [filePath], `false` otherwise
+     */
+    fun isDirectParent(folderPath: String, filePath: String): Boolean {
+        return File(folderPath).path == File(filePath).parent
+    }
+
+    /**
+     * Returns `true` if [folderPath] is an ancestor of [filePath], `false` otherwise
+     *
+     * If [isDirectParent] is `true` for the same arguments, this function should return `true` as well
+     */
+    fun isAncestor(folderPath: String, filePath: String): Boolean {
+        if (folderPath.isEmpty() || filePath.isEmpty()) {
+            return false
+        }
+        val folderPathWithSlash =
+            if (folderPath.endsWith(OCFile.PATH_SEPARATOR)) folderPath else folderPath + OCFile.PATH_SEPARATOR
+        return filePath.startsWith(folderPathWithSlash)
+    }
+}

app/src/test/java/com/owncloud/android/utils/PathUtilsTest.kt

@@ -0,0 +1,114 @@
+/*
+ * Nextcloud Android client application
+ *
+ *  @author Álvaro Brey
+ *  Copyright (C) 2022 Álvaro Brey
+ *  Copyright (C) 2022 Nextcloud GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or 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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+package com.owncloud.android.utils
+
+import org.junit.Assert.assertEquals
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+import org.junit.runners.Suite
+
+private val directParents: Array<Array<Any>> = arrayOf(
+    arrayOf("/bar", "/bar/foo.tgz", true),
+    arrayOf("/bar/", "/bar/foo.tgz", true),
+    arrayOf("/bar/", "/bar/foo/", true),
+    arrayOf("/bar/", "/bar/foo", true),
+    arrayOf("/", "/bar/", true),
+    arrayOf("/bar/", "/foo/bar", false)
+)
+
+private val nonAncestors: Array<Array<Any>> = arrayOf(
+    arrayOf("/bar/", "/", false),
+    arrayOf("/bar/", "", false),
+    arrayOf("/", "", false),
+    arrayOf("", "", false),
+    arrayOf("", "/", false)
+)
+
+/**
+ * These should return `false` for [PathUtils.isDirectParent] but `true` for [PathUtils.isAncestor]
+ */
+private val indirectAncestors: List<Pair<String, String>> = listOf(
+    Pair("/bar", "/bar/foo/baz.tgz"),
+    Pair("/bar/", "/bar/foo/baz.tgz"),
+    Pair("/bar/", "/bar/foo/baz/"),
+    Pair("/bar/", "/bar/foo/baz")
+)
+
+@RunWith(Suite::class)
+@Suite.SuiteClasses(
+    PathUtilsTest.IsDirectParent::class,
+    PathUtilsTest.IsAncestor::class
+)
+class PathUtilsTest {
+
+    @RunWith(Parameterized::class)
+    internal class IsDirectParent(
+        private val folderPath: String,
+        private val filePath: String,
+        private val isParent: Boolean
+    ) {
+
+        @Test
+        fun testIsParent() {
+            assertEquals("Wrong isParentPath result", isParent, PathUtils.isDirectParent(folderPath, filePath))
+        }
+
+        companion object {
+            @Parameterized.Parameters(name = "{0}, {1} => {2}")
+            @JvmStatic
+            fun urls(): Array<Array<Any>> {
+                val otherAncestors: Array<Array<Any>> = indirectAncestors.map {
+                    @Suppress("UNCHECKED_CAST")
+                    arrayOf(it.first, it.second, false) as Array<Any>
+                }.toTypedArray()
+                return directParents + nonAncestors + otherAncestors
+            }
+        }
+    }
+
+    @RunWith(Parameterized::class)
+    internal class IsAncestor(
+        private val folderPath: String,
+        private val filePath: String,
+        private val isAscendant: Boolean
+    ) {
+
+        @Test
+        fun testIsAncestor() {
+            assertEquals("Wrong isParentPath result", isAscendant, PathUtils.isAncestor(folderPath, filePath))
+        }
+
+        companion object {
+            @Parameterized.Parameters(name = "{0}, {1} => {2}")
+            @JvmStatic
+            fun urls(): Array<Array<Any>> {
+                val otherAncestors: Array<Array<Any>> = indirectAncestors.map {
+                    @Suppress("UNCHECKED_CAST")
+                    arrayOf(it.first, it.second, true) as Array<Any>
+                }.toTypedArray()
+                return directParents + nonAncestors + otherAncestors
+            }
+        }
+    }
+}