Refactor primary data model (#19)

bitwarden · Apr 9, 2024 · 32b0c90 · 32b0c90
1 parent cd992a6
commit 32b0c90
Show file tree

Hide file tree

Showing 9 changed files with 69 additions and 68 deletions.
diff --git a/...en.authenticator.data.authenticator.datasource.disk.database.AuthenticatorDatabase/1.json b/...en.authenticator.data.authenticator.datasource.disk.database.AuthenticatorDatabase/1.json
@@ -2,18 +2,30 @@
   "formatVersion": 1,
   "database": {
     "version": 1,
-    "identityHash": "3cc9605b629947b2b4148a40ff0d955d",
+    "identityHash": "8724b95439edde85bd15e0bd2e02195e",
     "entities": [
       {
         "tableName": "items",
-        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `type` TEXT NOT NULL, `algorithm` TEXT NOT NULL, `period` INTEGER NOT NULL, `digits` INTEGER NOT NULL, `key` TEXT NOT NULL, `issuer` TEXT, `userId` TEXT, `username` TEXT, PRIMARY KEY(`id`))",
+        "createSql": "CREATE TABLE IF NOT EXISTS `${TABLE_NAME}` (`id` TEXT NOT NULL, `key` TEXT NOT NULL, `accountName` TEXT NOT NULL, `type` TEXT NOT NULL, `algorithm` TEXT NOT NULL, `period` INTEGER NOT NULL, `digits` INTEGER NOT NULL, `issuer` TEXT, `userId` TEXT, PRIMARY KEY(`id`))",
         "fields": [
           {
             "fieldPath": "id",
             "columnName": "id",
             "affinity": "TEXT",
             "notNull": true
           },
+          {
+            "fieldPath": "key",
+            "columnName": "key",
+            "affinity": "TEXT",
+            "notNull": true
+          },
+          {
+            "fieldPath": "accountName",
+            "columnName": "accountName",
+            "affinity": "TEXT",
+            "notNull": true
+          },
           {
             "fieldPath": "type",
             "columnName": "type",
@@ -38,12 +50,6 @@
             "affinity": "INTEGER",
             "notNull": true
           },
-          {
-            "fieldPath": "key",
-            "columnName": "key",
-            "affinity": "TEXT",
-            "notNull": true
-          },
           {
             "fieldPath": "issuer",
             "columnName": "issuer",
@@ -55,12 +61,6 @@
             "columnName": "userId",
             "affinity": "TEXT",
             "notNull": false
-          },
-          {
-            "fieldPath": "username",
-            "columnName": "username",
-            "affinity": "TEXT",
-            "notNull": false
           }
         ],
         "primaryKey": {
@@ -76,7 +76,7 @@
     "views": [],
     "setupQueries": [
       "CREATE TABLE IF NOT EXISTS room_master_table (id INTEGER PRIMARY KEY,identity_hash TEXT)",
-      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '3cc9605b629947b2b4148a40ff0d955d')"
+      "INSERT OR REPLACE INTO room_master_table (id,identity_hash) VALUES(42, '8724b95439edde85bd15e0bd2e02195e')"
     ]
   }
-}
+}
diff --git a/...warden/authenticator/data/authenticator/datasource/disk/entity/AuthenticatorItemEntity.kt b/...warden/authenticator/data/authenticator/datasource/disk/entity/AuthenticatorItemEntity.kt
@@ -13,6 +13,12 @@ data class AuthenticatorItemEntity(
     @ColumnInfo(name = "id")
     val id: String,
 
+    @ColumnInfo(name = "key")
+    val key: String,
+
+    @ColumnInfo(name = "accountName")
+    val accountName: String,
+
     @ColumnInfo(name = "type")
     val type: AuthenticatorItemType = AuthenticatorItemType.TOTP,
 
@@ -25,15 +31,9 @@ data class AuthenticatorItemEntity(
     @ColumnInfo(name = "digits")
     val digits: Int = 6,
 
-    @ColumnInfo(name = "key")
-    val key: String,
-
     @ColumnInfo(name = "issuer")
-    val issuer: String?,
+    val issuer: String? = null,
 
     @ColumnInfo(name = "userId")
-    val userId: String?,
-
-    @ColumnInfo(name = "username")
-    val username: String?,
+    val userId: String? = null,
 )
diff --git a/...otlin/com/x8bit/bitwarden/authenticator/data/authenticator/manager/TotpCodeManagerImpl.kt b/...otlin/com/x8bit/bitwarden/authenticator/data/authenticator/manager/TotpCodeManagerImpl.kt
@@ -96,7 +96,7 @@ class TotpCodeManagerImpl @Inject constructor(
                                         time % response.period.toInt(),
                                     issueTime = clock.millis(),
                                     id = cipherId,
-                                    username = itemEntity.username,
+                                    username = itemEntity.accountName,
                                     issuer = itemEntity.issuer,
                                 )
                             }

diff --git a/...om/x8bit/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepository.kt b/...om/x8bit/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepository.kt
@@ -64,7 +64,7 @@ interface AuthenticatorRepository {
     /**
      * Attempt to create a cipher.
      */
-    suspend fun createItem(code: String, issuer: String): CreateItemResult
+    suspend fun createItem(item: AuthenticatorItemEntity): CreateItemResult
 
     /**
      * Attempt to delete a cipher.

diff --git a/...8bit/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryImpl.kt b/...8bit/bitwarden/authenticator/data/authenticator/repository/AuthenticatorRepositoryImpl.kt
@@ -2,7 +2,6 @@ package com.x8bit.bitwarden.authenticator.data.authenticator.repository
 
 import com.x8bit.bitwarden.authenticator.data.authenticator.datasource.disk.AuthenticatorDiskSource
 import com.x8bit.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemEntity
-import com.x8bit.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemType
 import com.x8bit.bitwarden.authenticator.data.authenticator.manager.TotpCodeManager
 import com.x8bit.bitwarden.authenticator.data.authenticator.manager.model.VerificationCodeItem
 import com.x8bit.bitwarden.authenticator.data.authenticator.repository.model.AuthenticatorData
@@ -31,7 +30,6 @@ import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.flow.onStart
 import kotlinx.coroutines.flow.stateIn
-import java.util.UUID
 import javax.inject.Inject
 
 /**
@@ -63,7 +61,7 @@ class AuthenticatorRepositoryImpl @Inject constructor(
                 is DataState.Error -> {
                     DataState.Error(
                         cipherDataState.error,
-                        AuthenticatorData(cipherDataState.data.orEmpty())
+                        AuthenticatorData(cipherDataState.data.orEmpty()),
                     )
                 }
 
@@ -86,7 +84,7 @@ class AuthenticatorRepositoryImpl @Inject constructor(
         }.stateIn(
             scope = unconfinedScope,
             started = SharingStarted.WhileSubscribed(stopTimeoutMillis = STOP_TIMEOUT_DELAY_MS),
-            initialValue = DataState.Loading
+            initialValue = DataState.Loading,
         )
 
     override val ciphersStateFlow: StateFlow<DataState<List<AuthenticatorItemEntity>>>
@@ -125,8 +123,8 @@ class AuthenticatorRepositoryImpl @Inject constructor(
             .flatMapLatest { cipherDataState ->
                 val cipher = cipherDataState.data
                     ?: return@flatMapLatest flowOf(DataState.Loaded(null))
-                totpCodeManager
-                    .getTotpCodeStateFlow(item = cipher)
+
+                totpCodeManager.getTotpCodeStateFlow(item = cipher)
                     .map { totpCodeDataState ->
                         combineDataStates(
                             totpCodeDataState,
@@ -153,10 +151,7 @@ class AuthenticatorRepositoryImpl @Inject constructor(
             }
             .flatMapLatest { cipherDataState ->
                 val cipherList = cipherDataState.data ?: emptyList()
-                totpCodeManager
-                    .getTotpCodesStateFlow(
-                        itemList = cipherList,
-                    )
+                totpCodeManager.getTotpCodesStateFlow(itemList = cipherList)
                     .map { verificationCodeDataStates ->
                         combineDataStates(
                             verificationCodeDataStates,
@@ -179,20 +174,9 @@ class AuthenticatorRepositoryImpl @Inject constructor(
         mutableTotpCodeResultFlow.tryEmit(totpCodeResult)
     }
 
-    override suspend fun createItem(code: String, issuer: String): CreateItemResult {
+    override suspend fun createItem(item: AuthenticatorItemEntity): CreateItemResult {
         return try {
-            authenticatorDiskSource.saveItem(
-                AuthenticatorItemEntity(
-                    id = UUID.randomUUID().toString(),
-                    type = AuthenticatorItemType.TOTP,
-                    period = 30,
-                    digits = 6,
-                    key = code,
-                    issuer = issuer,
-                    userId = null,
-                    username = null,
-                )
-            )
+            authenticatorDiskSource.saveItem(item)
             CreateItemResult.Success
         } catch (e: Exception) {
             CreateItemResult.Error
@@ -216,14 +200,14 @@ class AuthenticatorRepositoryImpl @Inject constructor(
             authenticatorDiskSource.saveItem(
                 AuthenticatorItemEntity(
                     id = itemId,
+                    key = updateItemRequest.key,
+                    accountName = updateItemRequest.accountName,
                     type = updateItemRequest.type,
                     period = updateItemRequest.period,
                     digits = updateItemRequest.digits,
-                    key = updateItemRequest.key,
                     issuer = updateItemRequest.issuer,
                     userId = null,
-                    username = null,
-                )
+                ),
             )
             UpdateItemResult.Success
         } catch (e: Exception) {

diff --git a/...om/x8bit/bitwarden/authenticator/data/authenticator/repository/model/UpdateItemRequest.kt b/...om/x8bit/bitwarden/authenticator/data/authenticator/repository/model/UpdateItemRequest.kt
@@ -6,34 +6,32 @@ import com.x8bit.bitwarden.authenticator.data.authenticator.datasource.disk.enti
 /**
  * Models a request to modify an existing authenticator item.
  *
+ * @property key Key used to generate verification codes for the authenticator item.
+ * @property accountName Required account or username associated with the item.
  * @property type Type of authenticator item.
  * @property algorithm Hashing algorithm applied to the authenticator item verification code.
- * @property period Time, in seconds, the authenticator item verification code is valid. Default is
- * 30 seconds.
+ * @property period Time, in seconds, the authenticator item verification code is valid.
  * @property digits Number of digits contained in the verification code for this authenticator item.
- * Default is 6 digits.
- * @property key Key used to generate verification codes for the authenticator item.
  * @property issuer Entity that provided the authenticator item.
- * @property username Optional username associated with .
  */
 data class UpdateItemRequest(
-    val type: AuthenticatorItemType,
-    val algorithm: AuthenticatorItemAlgorithm = AuthenticatorItemAlgorithm.SHA1,
-    val period: Int = 30,
-    val digits: Int = 6,
     val key: String,
+    val accountName: String,
+    val type: AuthenticatorItemType,
+    val algorithm: AuthenticatorItemAlgorithm,
+    val period: Int,
+    val digits: Int,
     val issuer: String?,
-    val username: String?,
 ) {
     /**
-     * The composite label of the authenticator item.
+     * The composite label of the authenticator item. Derived from combining [issuer] and [accountName]
      *  ```
-     *  label = issuer (“:” / “%3A”) *”%20” username
+     *  label = accountName /issuer (“:” / “%3A”) *”%20” accountName
      *  ```
      */
     val label = if (issuer != null) {
-        issuer + username?.let { ":$it" }.orEmpty()
+        "$issuer:$accountName"
     } else {
-        username.orEmpty()
+        accountName
     }
 }
diff --git a/...in/com/x8bit/bitwarden/authenticator/data/platform/repository/util/DataStateExtensions.kt b/...in/com/x8bit/bitwarden/authenticator/data/platform/repository/util/DataStateExtensions.kt
@@ -1,6 +1,8 @@
 package com.x8bit.bitwarden.authenticator.data.platform.repository.util
 
 import com.x8bit.bitwarden.authenticator.data.platform.repository.model.DataState
+import kotlinx.coroutines.flow.Flow
+import kotlinx.coroutines.flow.transformWhile
 
 /**
  * Maps the data inside a [DataState] with the given [transform].
@@ -15,6 +17,14 @@ inline fun <T : Any?, R : Any?> DataState<T>.map(
     is DataState.NoNetwork -> DataState.NoNetwork(data?.let(transform))
 }
 
+/**
+ * Emits all values of a [DataState] [Flow] until it emits a [DataState.Loaded].
+ */
+fun <T : Any?> Flow<DataState<T>>.takeUntilLoaded(): Flow<DataState<T>> = transformWhile {
+    emit(it)
+    it !is DataState.Loaded
+}
+
 /**
  * Combines the [dataState1] and [dataState2] [DataState]s together using the provided [transform].
  *

diff --git a/...n/kotlin/com/x8bit/bitwarden/authenticator/ui/authenticator/feature/item/ItemViewModel.kt b/...n/kotlin/com/x8bit/bitwarden/authenticator/ui/authenticator/feature/item/ItemViewModel.kt
@@ -58,7 +58,7 @@ class ItemViewModel @Inject constructor(
 
                     TotpCodeItemData(
                         type = item.type,
-                        username = item.username?.asText(),
+                        username = item.accountName.asText(),
                         issuer = item.issuer.orEmpty().asText(),
                         periodSeconds = authCode.periodSeconds,
                         timeLeftSeconds = authCode.timeLeftSeconds,

diff --git a/...warden/authenticator/ui/authenticator/feature/manualcodeentry/ManualCodeEntryViewModel.kt b/...warden/authenticator/ui/authenticator/feature/manualcodeentry/ManualCodeEntryViewModel.kt
@@ -3,13 +3,15 @@ package com.x8bit.bitwarden.authenticator.ui.authenticator.feature.manualcodeent
 import android.os.Parcelable
 import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.viewModelScope
+import com.x8bit.bitwarden.authenticator.data.authenticator.datasource.disk.entity.AuthenticatorItemEntity
 import com.x8bit.bitwarden.authenticator.data.authenticator.repository.AuthenticatorRepository
 import com.x8bit.bitwarden.authenticator.ui.platform.base.BaseViewModel
 import com.x8bit.bitwarden.authenticator.ui.platform.base.util.Text
 import dagger.hilt.android.lifecycle.HiltViewModel
 import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.launch
 import kotlinx.parcelize.Parcelize
+import java.util.UUID
 import javax.inject.Inject
 
 private const val KEY_STATE = "state"
@@ -55,7 +57,14 @@ class ManualCodeEntryViewModel @Inject constructor(
 
     private fun handleCodeSubmit() {
         viewModelScope.launch {
-            authenticatorRepository.createItem(state.code, state.issuer)
+            authenticatorRepository.createItem(
+                AuthenticatorItemEntity(
+                    id = UUID.randomUUID().toString(),
+                    key = state.code,
+                    accountName = "",
+                    issuer = state.issuer,
+                )
+            )
         }
         sendEvent(ManualCodeEntryEvent.NavigateBack)
     }