From bf9845d7a06b767a675300c494eb65280d5c150f Mon Sep 17 00:00:00 2001 From: David Perez Date: Tue, 19 Dec 2023 14:27:03 -0600 Subject: [PATCH] Ensure VaultDiskSource emits when replace operation does not actually change any data (#412) --- .../repository/util/SharedFlowExtensions.kt | 9 ++ .../vault/datasource/disk/VaultDiskSource.kt | 3 + .../datasource/disk/VaultDiskSourceImpl.kt | 96 ++++++++++++------- .../vault/datasource/disk/dao/CiphersDao.kt | 13 ++- .../datasource/disk/dao/CollectionsDao.kt | 18 +++- .../vault/datasource/disk/dao/FoldersDao.kt | 13 ++- 6 files changed, 100 insertions(+), 52 deletions(-) create mode 100644 app/src/main/java/com/x8bit/bitwarden/data/platform/repository/util/SharedFlowExtensions.kt diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/util/SharedFlowExtensions.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/util/SharedFlowExtensions.kt new file mode 100644 index 0000000000..6aa968b44e --- /dev/null +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/repository/util/SharedFlowExtensions.kt @@ -0,0 +1,9 @@ +package com.x8bit.bitwarden.data.platform.repository.util + +import kotlinx.coroutines.flow.MutableSharedFlow + +/** + * Creates a [MutableSharedFlow] with a buffer of [Int.MAX_VALUE]. + */ +fun bufferedMutableSharedFlow(): MutableSharedFlow = + MutableSharedFlow(extraBufferCapacity = Int.MAX_VALUE) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSource.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSource.kt index 5cc5ee2344..955619bbda 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSource.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSource.kt @@ -25,6 +25,9 @@ interface VaultDiskSource { /** * Replaces all [vault] data for a given [userId] with the new `vault`. + * + * This will always cause the [getCiphers], [getCollections], and [getFolders] functions to + * re-emit even if the underlying data has not changed. */ suspend fun replaceVaultData(userId: String, vault: SyncResponseJson) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceImpl.kt index adb1bc54e3..396d7f4328 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/VaultDiskSourceImpl.kt @@ -1,5 +1,6 @@ package com.x8bit.bitwarden.data.vault.datasource.disk +import com.x8bit.bitwarden.data.platform.repository.util.bufferedMutableSharedFlow import com.x8bit.bitwarden.data.vault.datasource.disk.dao.CiphersDao import com.x8bit.bitwarden.data.vault.datasource.disk.dao.CollectionsDao import com.x8bit.bitwarden.data.vault.datasource.disk.dao.FoldersDao @@ -12,6 +13,7 @@ import kotlinx.coroutines.awaitAll import kotlinx.coroutines.coroutineScope import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.merge import kotlinx.serialization.encodeToString import kotlinx.serialization.json.Json @@ -25,49 +27,63 @@ class VaultDiskSourceImpl( private val json: Json, ) : VaultDiskSource { + private val forceCiphersFlow = bufferedMutableSharedFlow>() + private val forceCollectionsFlow = + bufferedMutableSharedFlow>() + private val forceFolderFlow = bufferedMutableSharedFlow>() + override fun getCiphers( userId: String, ): Flow> = - ciphersDao - .getAllCiphers(userId = userId) - .map { entities -> - entities.map { entity -> - json.decodeFromString(entity.cipherJson) - } - } + merge( + forceCiphersFlow, + ciphersDao + .getAllCiphers(userId = userId) + .map { entities -> + entities.map { entity -> + json.decodeFromString(entity.cipherJson) + } + }, + ) override fun getCollections( userId: String, ): Flow> = - collectionsDao - .getAllCollections(userId = userId) - .map { entities -> - entities.map { entity -> - SyncResponseJson.Collection( - id = entity.id, - name = entity.name, - organizationId = entity.organizationId, - shouldHidePasswords = entity.shouldHidePasswords, - externalId = entity.externalId, - isReadOnly = entity.isReadOnly, - ) - } - } + merge( + forceCollectionsFlow, + collectionsDao + .getAllCollections(userId = userId) + .map { entities -> + entities.map { entity -> + SyncResponseJson.Collection( + id = entity.id, + name = entity.name, + organizationId = entity.organizationId, + shouldHidePasswords = entity.shouldHidePasswords, + externalId = entity.externalId, + isReadOnly = entity.isReadOnly, + ) + } + }, + ) override fun getFolders( userId: String, ): Flow> = - foldersDao - .getAllFolders(userId = userId) - .map { entities -> - entities.map { entity -> - SyncResponseJson.Folder( - id = entity.id, - name = entity.name, - revisionDate = entity.revisionDate, - ) - } - } + merge( + forceFolderFlow, + foldersDao + .getAllFolders(userId = userId) + .map { entities -> + entities.map { entity -> + SyncResponseJson.Folder( + id = entity.id, + name = entity.name, + revisionDate = entity.revisionDate, + ) + } + }, + ) override suspend fun replaceVaultData( userId: String, @@ -116,11 +132,17 @@ class VaultDiskSourceImpl( }, ) } - awaitAll( - deferredCiphers, - deferredCollections, - deferredFolders, - ) + // When going from 0 items to 0 items, the respective dao flow will not re-emit + // So we use this to give it a little push. + if (!deferredCiphers.await()) { + forceCiphersFlow.tryEmit(emptyList()) + } + if (!deferredCollections.await()) { + forceCollectionsFlow.tryEmit(emptyList()) + } + if (!deferredFolders.await()) { + forceFolderFlow.tryEmit(emptyList()) + } } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CiphersDao.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CiphersDao.kt index afe560ff24..c238501009 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CiphersDao.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CiphersDao.kt @@ -30,18 +30,21 @@ interface CiphersDao { ): Flow> /** - * Deletes all the stored ciphers associated with the given [userId]. + * Deletes all the stored ciphers associated with the given [userId]. This will return the + * number of rows deleted by this query. */ @Query("DELETE FROM ciphers WHERE user_id = :userId") - suspend fun deleteAllCiphers(userId: String) + suspend fun deleteAllCiphers(userId: String): Int /** * Deletes all the stored ciphers associated with the given [userId] and then add all new - * [ciphers] to the database. + * [ciphers] to the database. This will return `true` if any changes were made to the database + * and `false` otherwise. */ @Transaction - suspend fun replaceAllCiphers(userId: String, ciphers: List) { - deleteAllCiphers(userId) + suspend fun replaceAllCiphers(userId: String, ciphers: List): Boolean { + val deletedCiphersCount = deleteAllCiphers(userId) insertCiphers(ciphers) + return deletedCiphersCount > 0 || ciphers.isNotEmpty() } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CollectionsDao.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CollectionsDao.kt index 6a0f4f569d..7575a5cdc6 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CollectionsDao.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/CollectionsDao.kt @@ -34,10 +34,11 @@ interface CollectionsDao { fun getAllCollections(userId: String): Flow> /** - * Deletes all the stored collections associated with the given [userId]. + * Deletes all the stored collections associated with the given [userId]. This will return the + * number of rows deleted by this query. */ @Query("DELETE FROM collections WHERE user_id = :userId") - suspend fun deleteAllCollections(userId: String) + suspend fun deleteAllCollections(userId: String): Int /** * Deletes the stored collection associated with the given [userId] that matches the @@ -48,11 +49,18 @@ interface CollectionsDao { /** * Deletes all the stored [collections] associated with the given [userId] and then add all new - * `collections` to the database. + * `collections` to the database. This will return `true` if any changes were made to the + * database and `false` otherwise. + * + * @return `true` if any changes were made to the database. */ @Transaction - suspend fun replaceAllCollections(userId: String, collections: List) { - deleteAllCollections(userId) + suspend fun replaceAllCollections( + userId: String, + collections: List, + ): Boolean { + val deletedCollectionsCount = deleteAllCollections(userId) insertCollections(collections) + return deletedCollectionsCount > 0 || collections.isNotEmpty() } } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/FoldersDao.kt b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/FoldersDao.kt index fe7ed96d69..8ddd8bff14 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/FoldersDao.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/vault/datasource/disk/dao/FoldersDao.kt @@ -36,10 +36,11 @@ interface FoldersDao { ): Flow> /** - * Deletes all the stored folders associated with the given [userId]. + * Deletes all the stored folders associated with the given [userId]. This will return the + * number of rows deleted by this query. */ @Query("DELETE FROM folders WHERE user_id = :userId") - suspend fun deleteAllFolders(userId: String) + suspend fun deleteAllFolders(userId: String): Int /** * Deletes the stored folder associated with the given [userId] that matches the [folderId]. @@ -49,11 +50,13 @@ interface FoldersDao { /** * Deletes all the stored [folders] associated with the given [userId] and then add all new - * `folders` to the database. + * `folders` to the database. This will return `true` if any changes were made to the database + * and `false` otherwise. */ @Transaction - suspend fun replaceAllFolders(userId: String, folders: List) { - deleteAllFolders(userId) + suspend fun replaceAllFolders(userId: String, folders: List): Boolean { + val deletedFoldersCount = deleteAllFolders(userId) insertFolders(folders) + return deletedFoldersCount > 0 || folders.isNotEmpty() } }