From 8ffd14c2fba6abdf7f932c671e40bd80bceb7b7d Mon Sep 17 00:00:00 2001 From: Dave Severns <149429124+dseverns-livefront@users.noreply.github.com> Date: Wed, 24 Jul 2024 16:17:58 -0400 Subject: [PATCH] [PM-9927] Sort order update (#3625) --- .../util/SpecialCharacterStringComparator.kt | 38 +++++++++++++++++++ .../vault/repository/VaultRepositoryImpl.kt | 2 +- .../util/VaultSdkCipherExtensions.kt | 14 +++++-- .../util/VaultSdkCollectionExtensions.kt | 11 ++++-- .../util/VaultSdkFolderExtensions.kt | 21 ++++++---- .../repository/util/VaultSdkSendExtensions.kt | 14 +++++++ .../util/VaultSdkCipherExtensionsTest.kt | 11 ++++++ .../util/VaultSdkCollectionExtensionsTest.kt | 4 ++ .../util/VaultSdkFolderExtensionsTest.kt | 4 ++ .../util/VaultSdkSendExtensionsTest.kt | 30 +++++++++++++++ 10 files changed, 133 insertions(+), 16 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/platform/util/SpecialCharacterStringComparator.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/util/SpecialCharacterStringComparator.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/util/SpecialCharacterStringComparator.kt new file mode 100644 index 0000000000..d3bcd171b3 --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/util/SpecialCharacterStringComparator.kt @@ -0,0 +1,38 @@ +package com.x8bit.bitwarden.data.platform.util + +import java.util.Locale + +/** + * Compare two characters, where a special character is considered with higher precedence over + * letters and numbers. If both characters are a letter or a digit use the default + * [Char.compareTo]. + */ +private fun compareCharsSpecialCharsWithPrecedence(c1: Char, c2: Char): Int { + return when { + c1.isLetterOrDigit() && !c2.isLetterOrDigit() -> 1 + !c1.isLetterOrDigit() && c2.isLetterOrDigit() -> -1 + else -> c1.compareTo(c2) + } +} + +/** + * String [Comparator] where the characters are compared giving precedence to + * special characters. + */ +object CompareStringSpecialCharWithPrecedence : Comparator { + override fun compare(str1: String, str2: String): Int { + val uppercaseStr1 = str1.uppercase(Locale.getDefault()) + val uppercaseStr2 = str2.uppercase(Locale.getDefault()) + val minLength = minOf(uppercaseStr1.length, uppercaseStr2.length) + for (i in 0 until minLength) { + val char1 = uppercaseStr1[i] + val char2 = uppercaseStr2[i] + val compareResult = compareCharsSpecialCharsWithPrecedence(char1, char2) + if (compareResult != 0) { + return compareResult + } + } + // If all compared chars are the same give precedence to the shorter String. + return uppercaseStr1.length - uppercaseStr2.length + } +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt index 9f153e9c7a..ec9aebaf70 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/VaultRepositoryImpl.kt @@ -1085,7 +1085,7 @@ class VaultRepositoryImpl( sendList = it.toEncryptedSdkSendList(), ) .fold( - onSuccess = { sends -> DataState.Loaded(sends) }, + onSuccess = { sends -> DataState.Loaded(sends.sortAlphabetically()) }, onFailure = { throwable -> DataState.Error(throwable) }, ) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCipherExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCipherExtensions.kt index 184bba9c1c..ca62d83276 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCipherExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCipherExtensions.kt @@ -18,6 +18,7 @@ import com.bitwarden.vault.PasswordHistory import com.bitwarden.vault.SecureNote import com.bitwarden.vault.SecureNoteType import com.bitwarden.vault.UriMatchType +import com.x8bit.bitwarden.data.platform.util.CompareStringSpecialCharWithPrecedence import com.x8bit.bitwarden.data.vault.datasource.network.model.AttachmentJsonRequest import com.x8bit.bitwarden.data.vault.datasource.network.model.CipherJsonRequest import com.x8bit.bitwarden.data.vault.datasource.network.model.CipherRepromptTypeJson @@ -29,7 +30,6 @@ import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson import com.x8bit.bitwarden.data.vault.datasource.network.model.UriMatchTypeJson import java.time.ZoneOffset import java.time.ZonedDateTime -import java.util.Locale /** * Converts a Bitwarden SDK [Cipher] object to a corresponding @@ -553,8 +553,14 @@ fun FieldTypeJson.toSdkFieldType(): FieldType = } /** - * Sorts the data in alphabetical order by name. + * Sorts the data in alphabetical order by name. Using lexicographical sorting but giving + * precedence to special characters over letters and digits. */ @JvmName("toAlphabeticallySortedCipherList") -fun List.sortAlphabetically(): List = - this.sortedBy { it.name.uppercase(Locale.getDefault()) } +fun List.sortAlphabetically(): List { + return this.sortedWith( + comparator = { cipher1, cipher2 -> + CompareStringSpecialCharWithPrecedence.compare(cipher1.name, cipher2.name) + }, + ) +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCollectionExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCollectionExtensions.kt index e27e9071f2..b3a32725a8 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCollectionExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCollectionExtensions.kt @@ -2,8 +2,8 @@ package com.x8bit.bitwarden.data.vault.repository.util import com.bitwarden.vault.Collection import com.bitwarden.vault.CollectionView +import com.x8bit.bitwarden.data.platform.util.CompareStringSpecialCharWithPrecedence import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson -import java.util.Locale /** * Converts a [SyncResponseJson.Collection] object to a corresponding Bitwarden SDK [Collection] @@ -30,5 +30,10 @@ fun List.toEncryptedSdkCollectionList(): List.sortAlphabetically(): List = - this.sortedBy { it.name.uppercase(Locale.getDefault()) } +fun List.sortAlphabetically(): List { + return this.sortedWith( + comparator = { collection1, collection2 -> + CompareStringSpecialCharWithPrecedence.compare(collection1.name, collection2.name) + }, + ) +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkFolderExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkFolderExtensions.kt index 5cada62fcb..c461da34f7 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkFolderExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkFolderExtensions.kt @@ -2,9 +2,9 @@ package com.x8bit.bitwarden.data.vault.repository.util import com.bitwarden.vault.Folder import com.bitwarden.vault.FolderView +import com.x8bit.bitwarden.data.platform.util.CompareStringSpecialCharWithPrecedence import com.x8bit.bitwarden.data.vault.datasource.network.model.FolderJsonRequest import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson -import java.util.Locale /** * Converts a list of [SyncResponseJson.Folder] objects to a list of corresponding @@ -24,16 +24,21 @@ fun SyncResponseJson.Folder.toEncryptedSdkFolder(): Folder = revisionDate = revisionDate.toInstant(), ) -/** - * Sorts the data in alphabetical order by name. - */ -@JvmName("toAlphabeticallySortedFolderList") -fun List.sortAlphabetically(): List = - this.sortedBy { it.name.uppercase(Locale.getDefault()) } - /** * Converts a Bitwarden SDK [Folder] objects to a corresponding * [SyncResponseJson.Folder] object. */ fun Folder.toEncryptedNetworkFolder(): FolderJsonRequest = FolderJsonRequest(name = name) + +/** + * Sorts the data in alphabetical order by name. + */ +@JvmName("toAlphabeticallySortedFolderList") +fun List.sortAlphabetically(): List { + return this.sortedWith( + comparator = { folder1, folder2 -> + CompareStringSpecialCharWithPrecedence.compare(folder1.name, folder2.name) + }, + ) +} diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensions.kt index bbd778b190..3770e7330c 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensions.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensions.kt @@ -4,6 +4,8 @@ import com.bitwarden.send.Send import com.bitwarden.send.SendFile import com.bitwarden.send.SendText import com.bitwarden.send.SendType +import com.bitwarden.send.SendView +import com.x8bit.bitwarden.data.platform.util.CompareStringSpecialCharWithPrecedence import com.x8bit.bitwarden.data.vault.datasource.network.model.SendJsonRequest import com.x8bit.bitwarden.data.vault.datasource.network.model.SendTypeJson import com.x8bit.bitwarden.data.vault.datasource.network.model.SyncResponseJson @@ -123,3 +125,15 @@ private fun SendTypeJson.toSdkSendType(): SendType = SendTypeJson.TEXT -> SendType.TEXT SendTypeJson.FILE -> SendType.FILE } + +/** + * Sorts the data in alphabetical order by name. + */ +@JvmName("toAlphabeticallySortedSendList") +fun List.sortAlphabetically(): List { + return this.sortedWith( + comparator = { send1, send2 -> + CompareStringSpecialCharWithPrecedence.compare(send1.name, send2.name) + }, + ) +} diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCipherExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCipherExtensionsTest.kt index 54fc5bdf38..13738b890b 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCipherExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCipherExtensionsTest.kt @@ -304,12 +304,23 @@ class VaultSdkCipherExtensionsTest { createMockCipherView(1).copy(name = "c"), createMockCipherView(1).copy(name = "B"), createMockCipherView(1).copy(name = "z"), + createMockCipherView(1).copy(name = "8"), + createMockCipherView(1).copy(name = "7"), + createMockCipherView(1).copy(name = "_"), createMockCipherView(1).copy(name = "A"), createMockCipherView(1).copy(name = "D"), + createMockCipherView(1).copy(name = "AbA"), + createMockCipherView(1).copy(name = "aAb"), + ) val expected = listOf( + createMockCipherView(1).copy(name = "_"), + createMockCipherView(1).copy(name = "7"), + createMockCipherView(1).copy(name = "8"), createMockCipherView(1).copy(name = "A"), + createMockCipherView(1).copy(name = "aAb"), + createMockCipherView(1).copy(name = "AbA"), createMockCipherView(1).copy(name = "B"), createMockCipherView(1).copy(name = "c"), createMockCipherView(1).copy(name = "D"), diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCollectionExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCollectionExtensionsTest.kt index f65a2d0862..4e1458a2bf 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCollectionExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkCollectionExtensionsTest.kt @@ -65,11 +65,15 @@ class VaultSdkCollectionExtensionsTest { createMockCollectionView(1).copy(name = "c"), createMockCollectionView(1).copy(name = "B"), createMockCollectionView(1).copy(name = "z"), + createMockCollectionView(1).copy(name = "4"), createMockCollectionView(1).copy(name = "A"), + createMockCollectionView(1).copy(name = "#"), createMockCollectionView(1).copy(name = "D"), ) val expected = listOf( + createMockCollectionView(1).copy(name = "#"), + createMockCollectionView(1).copy(name = "4"), createMockCollectionView(1).copy(name = "A"), createMockCollectionView(1).copy(name = "B"), createMockCollectionView(1).copy(name = "c"), diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkFolderExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkFolderExtensionsTest.kt index 2b9646800e..7792bef22f 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkFolderExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkFolderExtensionsTest.kt @@ -41,12 +41,16 @@ class VaultSdkFolderExtensionsTest { val list = listOf( createMockFolderView(1).copy(name = "c"), createMockFolderView(1).copy(name = "D"), + createMockFolderView(1).copy(name = "_"), + createMockFolderView(1).copy(name = "4"), createMockFolderView(1).copy(name = "B"), createMockFolderView(1).copy(name = "A"), createMockFolderView(1).copy(name = "z"), ) val expected = listOf( + createMockFolderView(1).copy(name = "_"), + createMockFolderView(1).copy(name = "4"), createMockFolderView(1).copy(name = "A"), createMockFolderView(1).copy(name = "B"), createMockFolderView(1).copy(name = "c"), diff --git a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensionsTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensionsTest.kt index 6d621bc88e..83143049e7 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensionsTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/vault/repository/util/VaultSdkSendExtensionsTest.kt @@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.vault.repository.util import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockSend import com.x8bit.bitwarden.data.vault.datasource.network.model.createMockSendJsonRequest +import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSendView import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSdkSend import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test @@ -50,4 +51,33 @@ class VaultSdkSendExtensionsTest { sdkSends, ) } + + @Suppress("MaxLineLength") + @Test + fun `toSortAlphabetically should sort SendView by name`() { + val list = listOf( + createMockSendView(1).copy(name = "c"), + createMockSendView(1).copy(name = "B"), + createMockSendView(1).copy(name = "z"), + createMockSendView(1).copy(name = "4"), + createMockSendView(1).copy(name = "A"), + createMockSendView(1).copy(name = "#"), + createMockSendView(1).copy(name = "D"), + ) + + val expected = listOf( + createMockSendView(1).copy(name = "#"), + createMockSendView(1).copy(name = "4"), + createMockSendView(1).copy(name = "A"), + createMockSendView(1).copy(name = "B"), + createMockSendView(1).copy(name = "c"), + createMockSendView(1).copy(name = "D"), + createMockSendView(1).copy(name = "z"), + ) + + assertEquals( + expected, + list.sortAlphabetically(), + ) + } }