From 5183832e350ba7f16d013eef4c11fddaf6ff5af2 Mon Sep 17 00:00:00 2001 From: David Perez Date: Thu, 20 Jun 2024 16:03:42 -0500 Subject: [PATCH] trackEvent should not suspend (#3331) --- .../manager/event/OrganizationEventManager.kt | 2 +- .../event/OrganizationEventManagerImpl.kt | 37 ++++--- .../event/OrganizationEventManagerTest.kt | 104 +++++++++--------- 3 files changed, 72 insertions(+), 71 deletions(-) diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManager.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManager.kt index 17036b55d..91fa9d8e4 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManager.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManager.kt @@ -9,5 +9,5 @@ interface OrganizationEventManager { /** * Tracks a specific event to be uploaded at a different time. */ - suspend fun trackEvent(eventType: OrganizationEventType, cipherId: String? = null) + fun trackEvent(eventType: OrganizationEventType, cipherId: String? = null) } diff --git a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManagerImpl.kt b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManagerImpl.kt index 787f04f9d..b1bf0c4cd 100644 --- a/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManagerImpl.kt +++ b/app/src/main/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManagerImpl.kt @@ -58,28 +58,31 @@ class OrganizationEventManagerImpl( } @Suppress("ReturnCount") - override suspend fun trackEvent(eventType: OrganizationEventType, cipherId: String?) { + override fun trackEvent(eventType: OrganizationEventType, cipherId: String?) { val userId = authRepository.activeUserId ?: return if (authRepository.authStateFlow.value !is AuthState.Authenticated) return val organizations = authRepository.organizations.filter { it.shouldUseEvents } if (organizations.none()) return - cipherId?.let { id -> - val cipherOrganizationId = vaultRepository - .getVaultItemStateFlow(itemId = id) - .first { it.data != null } - .data - ?.organizationId - ?: return - if (organizations.none { it.id == cipherOrganizationId }) return + + ioScope.launch { + cipherId?.let { id -> + val cipherOrganizationId = vaultRepository + .getVaultItemStateFlow(itemId = id) + .first { it.data != null } + .data + ?.organizationId + ?: return@launch + if (organizations.none { it.id == cipherOrganizationId }) return@launch + } + eventDiskSource.addOrganizationEvent( + userId = userId, + event = OrganizationEvent( + type = eventType, + cipherId = cipherId, + date = ZonedDateTime.now(clock), + ), + ) } - eventDiskSource.addOrganizationEvent( - userId = userId, - event = OrganizationEvent( - type = eventType, - cipherId = cipherId, - date = ZonedDateTime.now(clock), - ), - ) } private suspend fun uploadEvents() { diff --git a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManagerTest.kt b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManagerTest.kt index cbed10e51..9f094250d 100644 --- a/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManagerTest.kt +++ b/app/src/test/java/com/x8bit/bitwarden/data/platform/manager/event/OrganizationEventManagerTest.kt @@ -121,7 +121,7 @@ class OrganizationEventManagerTest { } @Test - fun `trackEvent should do nothing if there is no active user`() = runTest { + fun `trackEvent should do nothing if there is no active user`() { every { authRepository.activeUserId } returns null organizationEventManager.trackEvent( @@ -135,7 +135,7 @@ class OrganizationEventManagerTest { } @Test - fun `trackEvent should do nothing if the active user is not authenticated`() = runTest { + fun `trackEvent should do nothing if the active user is not authenticated`() { organizationEventManager.trackEvent( eventType = OrganizationEventType.CIPHER_UPDATED, cipherId = CIPHER_ID, @@ -147,71 +147,69 @@ class OrganizationEventManagerTest { } @Test - fun `trackEvent should do nothing if the active user has no organizations that use events`() = - runTest { - mutableAuthStateFlow.value = AuthState.Authenticated(accessToken = "access-token") - val organization = createMockOrganization(number = 1) - every { authRepository.organizations } returns listOf(organization) + fun `trackEvent should do nothing if the active user has no organizations that use events`() { + mutableAuthStateFlow.value = AuthState.Authenticated(accessToken = "access-token") + val organization = createMockOrganization(number = 1) + every { authRepository.organizations } returns listOf(organization) - organizationEventManager.trackEvent( - eventType = OrganizationEventType.CIPHER_UPDATED, - cipherId = CIPHER_ID, - ) + organizationEventManager.trackEvent( + eventType = OrganizationEventType.CIPHER_UPDATED, + cipherId = CIPHER_ID, + ) - coVerify(exactly = 0) { - eventDiskSource.addOrganizationEvent(userId = any(), event = any()) - } + coVerify(exactly = 0) { + eventDiskSource.addOrganizationEvent(userId = any(), event = any()) } + } @Suppress("MaxLineLength") @Test - fun `trackEvent should do nothing if the cipher does not belong to an organization that uses events`() = - runTest { - mutableAuthStateFlow.value = AuthState.Authenticated(accessToken = "access-token") - val organization = createMockOrganization(number = 1).copy(shouldUseEvents = true) - every { authRepository.organizations } returns listOf(organization) - val cipherView = createMockCipherView(number = 1) - mutableVaultItemStateFlow.value = DataState.Loaded(data = cipherView) + fun `trackEvent should do nothing if the cipher does not belong to an organization that uses events`() { + mutableAuthStateFlow.value = AuthState.Authenticated(accessToken = "access-token") + val organization = createMockOrganization(number = 1).copy(shouldUseEvents = true) + every { authRepository.organizations } returns listOf(organization) + val cipherView = createMockCipherView(number = 1) + mutableVaultItemStateFlow.value = DataState.Loaded(data = cipherView) - organizationEventManager.trackEvent( - eventType = OrganizationEventType.CIPHER_UPDATED, - cipherId = CIPHER_ID, - ) + organizationEventManager.trackEvent( + eventType = OrganizationEventType.CIPHER_UPDATED, + cipherId = CIPHER_ID, + ) - coVerify(exactly = 0) { - eventDiskSource.addOrganizationEvent(userId = any(), event = any()) - } + coVerify(exactly = 0) { + eventDiskSource.addOrganizationEvent(userId = any(), event = any()) } + } @Test - fun `trackEvent should add the event to disk if the ciphers organization allows it`() = - runTest { - mutableAuthStateFlow.value = AuthState.Authenticated(accessToken = "access-token") - val organization = createMockOrganization(number = 1).copy( - id = "mockOrganizationId-1", - shouldUseEvents = true, - ) - every { authRepository.organizations } returns listOf(organization) - val cipherView = createMockCipherView(number = 1) - mutableVaultItemStateFlow.value = DataState.Loaded(data = cipherView) - val eventType = OrganizationEventType.CIPHER_UPDATED + fun `trackEvent should add the event to disk if the ciphers organization allows it`() { + mutableAuthStateFlow.value = AuthState.Authenticated(accessToken = "access-token") + val organization = createMockOrganization(number = 1).copy( + id = "mockOrganizationId-1", + shouldUseEvents = true, + ) + every { authRepository.organizations } returns listOf(organization) + val cipherView = createMockCipherView(number = 1) + mutableVaultItemStateFlow.value = DataState.Loaded(data = cipherView) + val eventType = OrganizationEventType.CIPHER_UPDATED - organizationEventManager.trackEvent( - eventType = eventType, - cipherId = CIPHER_ID, - ) + organizationEventManager.trackEvent( + eventType = eventType, + cipherId = CIPHER_ID, + ) - coVerify(exactly = 1) { - eventDiskSource.addOrganizationEvent( - userId = USER_ID, - event = OrganizationEvent( - type = eventType, - cipherId = CIPHER_ID, - date = ZonedDateTime.now(fixedClock), - ), - ) - } + dispatcher.scheduler.runCurrent() + coVerify(exactly = 1) { + eventDiskSource.addOrganizationEvent( + userId = USER_ID, + event = OrganizationEvent( + type = eventType, + cipherId = CIPHER_ID, + date = ZonedDateTime.now(fixedClock), + ), + ) } + } } private const val CIPHER_ID: String = "mockId-1"