Improve search logic and UI (#703)

* Don't list the current directory in search results

* Scroll to top result when search term is changed

* Match relative path in StrictDomain filter mode

* Improve and document DirectoryStructure null handling

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>

2 files changed, 74 insertions, 18 deletions

diff --git a/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/AutofillPreferences.kt b/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/AutofillPreferences.kt
index bedb551d..39e1dbb5 100644
--- a/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/AutofillPreferences.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/AutofillPreferences.kt
@@ -19,32 +19,72 @@ enum class DirectoryStructure(val value: String) {
     FileBased("file"),
     DirectoryBased("directory");
 
-    fun getUsernameFor(file: File) = when (this) {
+    /**
+     * Returns the username associated to [file], following the convention of the current
+     * [DirectoryStructure].
+     *
+     * Examples:
+     *   - work/example.org/john@doe.org.gpg --> john@doe.org (FileBased)
+     *   - work/example.org/john@doe.org/password.gpg --> john@doe.org (DirectoryBased)
+     *   - Temporary PIN.gpg --> Temporary PIN (DirectoryBased, fallback)
+     */
+    fun getUsernameFor(file: File): String = when (this) {
         FileBased -> file.nameWithoutExtension
         DirectoryBased -> file.parentFile?.name ?: file.nameWithoutExtension
     }
 
-    fun getIdentifierFor(file: File) = when (this) {
-        FileBased -> file.parentFile.name
-        DirectoryBased -> file.parentFile.parentFile?.name
+    /**
+     * Returns the origin identifier associated to [file], following the convention of the current
+     * [DirectoryStructure].
+     *
+     * At least one of [DirectoryStructure.getIdentifierFor] and
+     * [DirectoryStructure.getAccountPartFor] will always return a non-null result.
+     *
+     * Examples:
+     *   - work/example.org/john@doe.org.gpg --> example.org (FileBased)
+     *   - example.org.gpg --> example.org (FileBased, fallback)
+     *   - work/example.org/john@doe.org/password.gpg --> example.org (DirectoryBased)
+     *   - Temporary PIN.gpg --> null (DirectoryBased)
+     */
+    fun getIdentifierFor(file: File): String? = when (this) {
+        FileBased -> file.parentFile?.name ?: file.nameWithoutExtension
+        DirectoryBased -> file.parentFile?.parent
     }
 
     /**
      * Returns the path components of [file] until right before the component that contains the
      * origin identifier according to the current [DirectoryStructure].
      *
+     * At least one of [DirectoryStructure.getIdentifierFor] and
+     * [DirectoryStructure.getAccountPartFor] will always return a non-null result.
+     *
      * Examples:
-     *   - /work/example.org/john@doe.org --> /work (FileBased)
-     *   - /work/example.org/john@doe.org/password --> /work (DirectoryBased)
+     *   - work/example.org/john@doe.org.gpg --> work (FileBased)
+     *   - example.org/john@doe.org.gpg --> null (FileBased)
+     *   - john@doe.org.gpg --> null (FileBased)
+     *   - work/example.org/john@doe.org/password.gpg --> work (DirectoryBased)
+     *   - example.org/john@doe.org/password.gpg --> null (DirectoryBased)
      */
-    fun getPathToIdentifierFor(file: File) = when (this) {
-        FileBased -> file.parentFile.parent
-        DirectoryBased -> file.parentFile.parentFile?.parent
+    fun getPathToIdentifierFor(file: File): String? = when (this) {
+        FileBased -> file.parentFile?.parent
+        DirectoryBased -> file.parentFile?.parentFile?.parent
     }
 
-    fun getAccountPartFor(file: File) = when (this) {
-        FileBased -> file.nameWithoutExtension
-        DirectoryBased -> "${file.parentFile.name}/${file.nameWithoutExtension}"
+    /**
+     * Returns the path component of [file] following the origin identifier according to the current
+     * [DirectoryStructure] (without file extension).
+     *
+     * Examples:
+     *   - work/example.org/john@doe.org.gpg --> john@doe.org (FileBased)
+     *   - example.org.gpg --> null (FileBased, fallback)
+     *   - work/example.org/john@doe.org/password.gpg --> john@doe.org/password (DirectoryBased)
+     *   - Temporary PIN.gpg --> Temporary PIN (DirectoryBased, fallback)
+     */
+    fun getAccountPartFor(file: File): String? = when (this) {
+        FileBased -> file.nameWithoutExtension.takeIf { file.parentFile != null }
+        DirectoryBased -> file.parentFile?.let { parentFile ->
+            "${parentFile.name}/${file.nameWithoutExtension}"
+        } ?: file.nameWithoutExtension
     }
 
     @RequiresApi(Build.VERSION_CODES.O)
diff --git a/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillFilterActivity.kt b/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillFilterActivity.kt
index aee7f1a9..0901f8d9 100644
--- a/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillFilterActivity.kt
+++ b/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillFilterActivity.kt
@@ -11,6 +11,7 @@ import android.content.Intent
 import android.content.IntentSender
 import android.os.Build
 import android.os.Bundle
+import android.view.View
 import android.view.autofill.AutofillManager
 import android.widget.TextView
 import androidx.activity.viewModels
@@ -119,13 +120,26 @@ class AutofillFilterView : AppCompatActivity() {
             ::PasswordViewHolder) { item ->
             val file = item.file.relativeTo(item.rootDir)
             val pathToIdentifier = directoryStructure.getPathToIdentifierFor(file)
-            val identifier = directoryStructure.getIdentifierFor(file) ?: "INVALID"
+            val identifier = directoryStructure.getIdentifierFor(file)
             val accountPart = directoryStructure.getAccountPartFor(file)
-            title.text = buildSpannedString {
-                pathToIdentifier?.let { append("$it/") }
-                bold { underline { append(identifier) } }
+            check(identifier != null || accountPart != null) { "At least one of identifier and accountPart should always be non-null" }
+            title.text = if (identifier != null) {
+                buildSpannedString {
+                    if (pathToIdentifier != null)
+                        append("$pathToIdentifier/")
+                    bold { underline { append(identifier) } }
+                }
+            } else {
+                accountPart
+            }
+            subtitle.apply {
+                if (identifier != null && accountPart != null) {
+                    text = accountPart
+                    visibility = View.VISIBLE
+                } else {
+                    visibility = View.GONE
+                }
             }
-            subtitle.text = accountPart
         }.onItemClicked { _, item ->
             decryptAndFill(item)
         }
@@ -154,7 +168,9 @@ class AutofillFilterView : AppCompatActivity() {
         }
         model.searchResult.observe(this) { result ->
             val list = result.passwordItems
-            recyclerAdapter.submitList(list)
+            recyclerAdapter.submitList(list) {
+                binding.rvPassword.scrollToPosition(0)
+            }
             // Switch RecyclerView out for a "no results" message if the new list is empty and
             // the message is not yet shown (and vice versa).
             if ((list.isEmpty() && binding.rvPasswordSwitcher.nextView.id == binding.rvPasswordEmpty.id) ||