From b7fb1e68f6c0b347e1a7a983ead1d828c7c2b0fd Mon Sep 17 00:00:00 2001 From: tuancoltech Date: Mon, 18 Mar 2024 12:43:40 +0700 Subject: [PATCH 1/2] Prohibit creating nested roots: https://github.com/ARK-Builders/ark-android/issues/68 --- .../components/filepicker/ArkFilePickerFragment.kt | 4 ++++ .../components/filepicker/ArkFilePickerViewModel.kt | 12 ++++++++++++ filepicker/src/main/res/values/strings.xml | 1 + 3 files changed, 17 insertions(+) diff --git a/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerFragment.kt b/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerFragment.kt index ac21c3f..27166dd 100644 --- a/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerFragment.kt +++ b/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerFragment.kt @@ -306,6 +306,10 @@ open class ArkFilePickerFragment : FilePickerSideEffect.CannotPinFile -> { activity?.toast(R.string.ark_file_picker_pin_folder_only) } + + FilePickerSideEffect.NestedRootProhibited -> { + activity?.toast(R.string.ark_file_nested_root_inside) + } } diff --git a/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerViewModel.kt b/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerViewModel.kt index 19d4b1b..cd01b1a 100644 --- a/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerViewModel.kt +++ b/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerViewModel.kt @@ -44,6 +44,7 @@ internal sealed class FilePickerSideEffect { data object AlreadyFavorite : FilePickerSideEffect() data object PinAsFirstRoot : FilePickerSideEffect() data object CannotPinFile : FilePickerSideEffect() + data object NestedRootProhibited : FilePickerSideEffect() } @@ -190,6 +191,17 @@ internal class ArkFilePickerViewModel( val roots = rootsWithFavorites.keys val root = roots.find { root -> file.startsWith(root) } val favorites = rootsWithFavorites[root]?.flatten() + + val hasNestedRoot = roots.contains(file) + || (roots.indexOfFirst { path -> + val index = path.toString().indexOf(file.toString()) + (index >= 0) && (path.toString()[index] == '/') } >= 0) + + if (hasNestedRoot) { + postSideEffect(FilePickerSideEffect.NestedRootProhibited) + return@intent + } + val haveRoot = haveRoot() root?.let { diff --git a/filepicker/src/main/res/values/strings.xml b/filepicker/src/main/res/values/strings.xml index e7f016e..20116db 100644 --- a/filepicker/src/main/res/values/strings.xml +++ b/filepicker/src/main/res/values/strings.xml @@ -16,6 +16,7 @@ Already be a Favorite folder! Already be a Root folder! Only folder can be pinned. + There\'s already nested root(s) inside. Pin %d item From 0ae7b9977f97d9e244cc4e70fbe318907b645ca3 Mon Sep 17 00:00:00 2001 From: tuancoltech Date: Wed, 6 Nov 2024 22:02:11 +0700 Subject: [PATCH 2/2] Extract API for nested/parental root checking to utils module --- .../filepicker/ArkFilePickerViewModel.kt | 6 +- .../arkbuilders/components/utils/PathExt.kt | 10 ++ .../components/utils/PathExtTest.kt | 132 ++++++++++++++++++ 3 files changed, 144 insertions(+), 4 deletions(-) create mode 100644 utils/src/main/java/dev/arkbuilders/components/utils/PathExt.kt create mode 100644 utils/src/test/java/dev/arkbuilders/components/utils/PathExtTest.kt diff --git a/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerViewModel.kt b/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerViewModel.kt index cd01b1a..cafe146 100644 --- a/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerViewModel.kt +++ b/filepicker/src/main/java/dev/arkbuilders/components/filepicker/ArkFilePickerViewModel.kt @@ -14,6 +14,7 @@ import org.orbitmvi.orbit.viewmodel.container import dev.arkbuilders.arklib.data.folders.FoldersRepo import dev.arkbuilders.arklib.utils.DeviceStorageUtils import dev.arkbuilders.arklib.utils.listChildren +import dev.arkbuilders.components.utils.hasNestedOrParentalRoot import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import java.nio.file.Path @@ -192,10 +193,7 @@ internal class ArkFilePickerViewModel( val root = roots.find { root -> file.startsWith(root) } val favorites = rootsWithFavorites[root]?.flatten() - val hasNestedRoot = roots.contains(file) - || (roots.indexOfFirst { path -> - val index = path.toString().indexOf(file.toString()) - (index >= 0) && (path.toString()[index] == '/') } >= 0) + val hasNestedRoot = file.hasNestedOrParentalRoot(roots) if (hasNestedRoot) { postSideEffect(FilePickerSideEffect.NestedRootProhibited) diff --git a/utils/src/main/java/dev/arkbuilders/components/utils/PathExt.kt b/utils/src/main/java/dev/arkbuilders/components/utils/PathExt.kt new file mode 100644 index 0000000..df0d9a9 --- /dev/null +++ b/utils/src/main/java/dev/arkbuilders/components/utils/PathExt.kt @@ -0,0 +1,10 @@ +package dev.arkbuilders.components.utils + +import java.nio.file.Path + +fun Path.hasNestedOrParentalRoot(roots: Iterable): Boolean { + val hasNestedRoot = roots.any { path -> + this.startsWith(path) || path.startsWith(this) + } + return hasNestedRoot +} \ No newline at end of file diff --git a/utils/src/test/java/dev/arkbuilders/components/utils/PathExtTest.kt b/utils/src/test/java/dev/arkbuilders/components/utils/PathExtTest.kt new file mode 100644 index 0000000..09a74a3 --- /dev/null +++ b/utils/src/test/java/dev/arkbuilders/components/utils/PathExtTest.kt @@ -0,0 +1,132 @@ +package dev.arkbuilders.components.utils + +import junit.framework.TestCase.assertEquals +import org.junit.Test +import java.nio.file.Path +import java.nio.file.Paths + +class PathExtTest { + + // --- Tests with Set for roots parameter --- + + @Test + fun `hasNestedOrParentalRoot should return true when there is an exact match in roots`() { + val path = Paths.get("/parent/child") + val roots = setOf(Paths.get("/parent/child")) + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return true when there is a direct parent in roots`() { + val path = Paths.get("/parent/child") + val roots = setOf(Paths.get("/parent")) + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return true when there is a direct child in roots`() { + val path = Paths.get("/parent") + val roots = setOf(Paths.get("/parent/child")) + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return true when there is a nested parent in roots`() { + val path = Paths.get("/parent/child/grandchild") + val roots = setOf(Paths.get("/parent")) + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return true when there is a nested child in roots`() { + val path = Paths.get("/parent") + val roots = setOf(Paths.get("/parent/child/grandchild")) + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return false when there are no nested or parental roots`() { + val path = Paths.get("/parent/child") + val roots = setOf(Paths.get("/unrelated"), Paths.get("/another")) + assertEquals(path.hasNestedOrParentalRoot(roots), false) + } + + @Test + fun `hasNestedOrParentalRoot should return false when roots set is empty`() { + val path = Paths.get("/parent/child") + val roots = emptySet() + assertEquals(path.hasNestedOrParentalRoot(roots), false) + } + + // --- Tests with List for roots parameter --- + + @Test + fun `hasNestedOrParentalRoot should return true when an exact match exists in roots (List)`() { + val path = Paths.get("/parent/child") + val roots = listOf(Paths.get("/parent/child")) + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return true when a direct parent exists in roots (List)`() { + val path = Paths.get("/parent/child") + val roots = listOf(Paths.get("/parent")) + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return true when a direct child exists in roots (List)`() { + val path = Paths.get("/parent") + val roots = listOf(Paths.get("/parent/child")) + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return true when a nested parent exists in roots (List)`() { + val path = Paths.get("/parent/child/grandchild") + val roots = listOf(Paths.get("/parent")) + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return false when there are no nested or parental roots (List)`() { + val path = Paths.get("/parent/child") + val roots = listOf(Paths.get("/unrelated"), Paths.get("/another")) + assertEquals(path.hasNestedOrParentalRoot(roots), false) + } + + @Test + fun `hasNestedOrParentalRoot should return false when roots is empty`() { + val path = Paths.get("/parent/child") + val roots = emptyList() + assertEquals(path.hasNestedOrParentalRoot(roots), false) + } + + @Test + fun `hasNestedOrParentalRoot should return true when duplicates of an exact match exist in roots (List)`() { + val path = Paths.get("/parent/child") + val roots = listOf(Paths.get("/parent/child"), Paths.get("/parent/child")) // Duplicate exact match + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return true when duplicates of a direct parent exist in roots (List)`() { + val path = Paths.get("/parent/child") + val roots = listOf(Paths.get("/parent"), Paths.get("/parent")) // Duplicate direct parent + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return true when duplicates of a nested parent exist in roots (List)`() { + val path = Paths.get("/parent/child/grandchild") + val roots = listOf(Paths.get("/parent"), Paths.get("/parent")) // Duplicate nested parent + assertEquals(path.hasNestedOrParentalRoot(roots), true) + } + + @Test + fun `hasNestedOrParentalRoot should return false when duplicates of unrelated paths exist in roots (List)`() { + val path = Paths.get("/parent/child") + val roots = listOf(Paths.get("/unrelated"), Paths.get("/unrelated")) // Duplicate unrelated paths + assertEquals(path.hasNestedOrParentalRoot(roots), false) + } +}