From 7a7b5d2dd0395e02b0c9a5c3cbbf4013fa26af41 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 15 Nov 2024 12:31:23 +0100 Subject: [PATCH] Properly unregister from the ntfy app when the user logs out. --- libraries/push/impl/build.gradle.kts | 2 + .../libraries/push/impl/DefaultPushService.kt | 42 ++++++++++++++++++- .../push/impl/DefaultPushServiceTest.kt | 8 ++++ .../pushproviders/api/PushProvider.kt | 6 +++ .../firebase/FirebasePushProvider.kt | 6 +++ .../pushproviders/test/FakePushProvider.kt | 6 +++ .../unifiedpush/UnifiedPushProvider.kt | 8 +++- .../UnregisterUnifiedPushUseCase.kt | 14 +++++-- ...DefaultUnregisterUnifiedPushUseCaseTest.kt | 8 ++-- .../FakeUnregisterUnifiedPushUseCase.kt | 11 +++-- .../unifiedpush/UnifiedPushProviderTest.kt | 12 +++--- libraries/pushstore/impl/build.gradle.kts | 2 - .../impl/DefaultUserPushStoreFactory.kt | 24 +---------- .../clientsecret/DefaultPushClientSecret.kt | 26 +----------- .../DefaultPushClientSecretTest.kt | 12 +----- 15 files changed, 110 insertions(+), 77 deletions(-) diff --git a/libraries/push/impl/build.gradle.kts b/libraries/push/impl/build.gradle.kts index 74d6ba3182..6489f26dec 100644 --- a/libraries/push/impl/build.gradle.kts +++ b/libraries/push/impl/build.gradle.kts @@ -43,6 +43,7 @@ dependencies { implementation(projects.libraries.matrix.api) implementation(projects.libraries.matrixui) implementation(projects.libraries.preferences.api) + implementation(projects.libraries.sessionStorage.api) implementation(projects.libraries.uiStrings) implementation(projects.libraries.troubleshoot.api) implementation(projects.features.call.api) @@ -64,6 +65,7 @@ dependencies { testImplementation(libs.coroutines.test) testImplementation(projects.libraries.matrix.test) testImplementation(projects.libraries.preferences.test) + testImplementation(projects.libraries.sessionStorage.test) testImplementation(projects.libraries.push.test) testImplementation(projects.libraries.pushproviders.test) testImplementation(projects.libraries.pushstore.test) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt index 789b8e1f4e..ca192ea886 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt +++ b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/DefaultPushService.kt @@ -9,6 +9,7 @@ package io.element.android.libraries.push.impl import com.squareup.anvil.annotations.ContributesBinding import io.element.android.libraries.di.AppScope +import io.element.android.libraries.di.SingleIn import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.push.api.GetCurrentPushProvider @@ -17,17 +18,27 @@ import io.element.android.libraries.push.impl.test.TestPush import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider import io.element.android.libraries.pushstore.api.UserPushStoreFactory +import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretStore +import io.element.android.libraries.sessionstorage.api.observer.SessionListener +import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import kotlinx.coroutines.flow.Flow import timber.log.Timber import javax.inject.Inject -@ContributesBinding(AppScope::class) +@ContributesBinding(AppScope::class, boundType = PushService::class) +@SingleIn(AppScope::class) class DefaultPushService @Inject constructor( private val testPush: TestPush, private val userPushStoreFactory: UserPushStoreFactory, private val pushProviders: Set<@JvmSuppressWildcards PushProvider>, private val getCurrentPushProvider: GetCurrentPushProvider, -) : PushService { + private val sessionObserver: SessionObserver, + private val pushClientSecretStore: PushClientSecretStore, +) : PushService, SessionListener { + init { + observeSessions() + } + override suspend fun getCurrentPushProvider(): PushProvider? { val currentPushProvider = getCurrentPushProvider.getCurrentPushProvider() return pushProviders.find { it.name == currentPushProvider } @@ -87,4 +98,31 @@ class DefaultPushService @Inject constructor( testPush.execute(config) return true } + + private fun observeSessions() { + sessionObserver.addListener(this) + } + + override suspend fun onSessionCreated(userId: String) { + // Nothing to do + } + + /** + * The session has been deleted. + * In this case, this is not necessary to unregister the pusher from the homeserver, + * but we need to do some cleanup locally. + * The current push provider may want to take action, and we need to + * cleanup the stores. + */ + override suspend fun onSessionDeleted(userId: String) { + val sessionId = SessionId(userId) + val userPushStore = userPushStoreFactory.getOrCreate(sessionId) + val currentPushProviderName = userPushStore.getPushProviderName() + val currentPushProvider = pushProviders.find { it.name == currentPushProviderName } + // Cleanup the current push provider. They may need the client secret, so delete the secret after. + currentPushProvider?.onSessionDeleted(sessionId) + // Now we can safely reset the stores. + pushClientSecretStore.resetSecret(sessionId) + userPushStore.reset() + } } diff --git a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt index f2b029da49..d42d1f7ffd 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/DefaultPushServiceTest.kt @@ -22,8 +22,12 @@ import io.element.android.libraries.pushproviders.api.PushProvider import io.element.android.libraries.pushproviders.test.FakePushProvider import io.element.android.libraries.pushproviders.test.aCurrentUserPushConfig import io.element.android.libraries.pushstore.api.UserPushStoreFactory +import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretStore import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushStore import io.element.android.libraries.pushstore.test.userpushstore.FakeUserPushStoreFactory +import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.InMemoryPushClientSecretStore +import io.element.android.libraries.sessionstorage.api.observer.SessionObserver +import io.element.android.libraries.sessionstorage.test.observer.NoOpSessionObserver import io.element.android.tests.testutils.lambda.lambdaRecorder import io.element.android.tests.testutils.lambda.value import kotlinx.coroutines.flow.first @@ -215,12 +219,16 @@ class DefaultPushServiceTest { userPushStoreFactory: UserPushStoreFactory = FakeUserPushStoreFactory(), pushProviders: Set<@JvmSuppressWildcards PushProvider> = emptySet(), getCurrentPushProvider: GetCurrentPushProvider = FakeGetCurrentPushProvider(currentPushProvider = null), + sessionObserver: SessionObserver = NoOpSessionObserver(), + pushClientSecretStore: PushClientSecretStore = InMemoryPushClientSecretStore(), ): DefaultPushService { return DefaultPushService( testPush = testPush, userPushStoreFactory = userPushStoreFactory, pushProviders = pushProviders, getCurrentPushProvider = getCurrentPushProvider, + sessionObserver = sessionObserver, + pushClientSecretStore = pushClientSecretStore, ) } } diff --git a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt index 90b4cb0465..c5a436985e 100644 --- a/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt +++ b/libraries/pushproviders/api/src/main/kotlin/io/element/android/libraries/pushproviders/api/PushProvider.kt @@ -8,6 +8,7 @@ package io.element.android.libraries.pushproviders.api import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId /** * This is the main API for this module. @@ -43,6 +44,11 @@ interface PushProvider { */ suspend fun unregister(matrixClient: MatrixClient): Result + /** + * To invoke when the session is deleted. + */ + suspend fun onSessionDeleted(sessionId: SessionId) + suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? fun canRotateToken(): Boolean diff --git a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt index b1e31a2303..6f3d256d88 100644 --- a/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt +++ b/libraries/pushproviders/firebase/src/main/kotlin/io/element/android/libraries/pushproviders/firebase/FirebasePushProvider.kt @@ -11,6 +11,7 @@ import com.squareup.anvil.annotations.ContributesMultibinding import io.element.android.libraries.core.log.logger.LoggerTag import io.element.android.libraries.di.AppScope import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushproviders.api.CurrentUserPushConfig import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider @@ -63,6 +64,11 @@ class FirebasePushProvider @Inject constructor( } } + /** + * Nothing to clean up here. + */ + override suspend fun onSessionDeleted(sessionId: SessionId) = Unit + override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { return firebaseStore.getFcmToken()?.let { fcmToken -> CurrentUserPushConfig( diff --git a/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt b/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt index 6f5bdda5b7..076d89076f 100644 --- a/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt +++ b/libraries/pushproviders/test/src/main/kotlin/io/element/android/libraries/pushproviders/test/FakePushProvider.kt @@ -8,6 +8,7 @@ package io.element.android.libraries.pushproviders.test import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushproviders.api.CurrentUserPushConfig import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider @@ -21,6 +22,7 @@ class FakePushProvider( private val currentUserPushConfig: CurrentUserPushConfig? = null, private val registerWithResult: (MatrixClient, Distributor) -> Result = { _, _ -> lambdaError() }, private val unregisterWithResult: (MatrixClient) -> Result = { lambdaError() }, + private val onSessionDeletedLambda: (SessionId) -> Unit = { lambdaError() }, private val canRotateTokenResult: () -> Boolean = { lambdaError() }, private val rotateTokenLambda: () -> Result = { lambdaError() }, ) : PushProvider { @@ -38,6 +40,10 @@ class FakePushProvider( return unregisterWithResult(matrixClient) } + override suspend fun onSessionDeleted(sessionId: SessionId) { + onSessionDeletedLambda(sessionId) + } + override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { return currentUserPushConfig } diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt index 1ce929a75b..329232231c 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProvider.kt @@ -10,6 +10,7 @@ package io.element.android.libraries.pushproviders.unifiedpush import com.squareup.anvil.annotations.ContributesMultibinding import io.element.android.libraries.di.AppScope import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushproviders.api.CurrentUserPushConfig import io.element.android.libraries.pushproviders.api.Distributor import io.element.android.libraries.pushproviders.api.PushProvider @@ -47,7 +48,12 @@ class UnifiedPushProvider @Inject constructor( override suspend fun unregister(matrixClient: MatrixClient): Result { val clientSecret = pushClientSecret.getSecretForUser(matrixClient.sessionId) - return unRegisterUnifiedPushUseCase.execute(matrixClient, clientSecret) + return unRegisterUnifiedPushUseCase.unregister(matrixClient, clientSecret) + } + + override suspend fun onSessionDeleted(sessionId: SessionId) { + val clientSecret = pushClientSecret.getSecretForUser(sessionId) + unRegisterUnifiedPushUseCase.cleanup(clientSecret) } override suspend fun getCurrentUserPushConfig(): CurrentUserPushConfig? { diff --git a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt index 3bdec33859..2c2613901c 100644 --- a/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt +++ b/libraries/pushproviders/unifiedpush/src/main/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnregisterUnifiedPushUseCase.kt @@ -18,7 +18,15 @@ import timber.log.Timber import javax.inject.Inject interface UnregisterUnifiedPushUseCase { - suspend fun execute(matrixClient: MatrixClient, clientSecret: String): Result + /** + * Unregister the app from the homeserver, then from UnifiedPush. + */ + suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result + + /** + * Cleanup any remaining data for the given client secret and unregister the app from UnifiedPush. + */ + fun cleanup(clientSecret: String) } @ContributesBinding(AppScope::class) @@ -27,7 +35,7 @@ class DefaultUnregisterUnifiedPushUseCase @Inject constructor( private val unifiedPushStore: UnifiedPushStore, private val pusherSubscriber: PusherSubscriber, ) : UnregisterUnifiedPushUseCase { - override suspend fun execute(matrixClient: MatrixClient, clientSecret: String): Result { + override suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result { val endpoint = unifiedPushStore.getEndpoint(clientSecret) val gateway = unifiedPushStore.getPushGateway(clientSecret) if (endpoint == null || gateway == null) { @@ -42,7 +50,7 @@ class DefaultUnregisterUnifiedPushUseCase @Inject constructor( } } - private fun cleanup(clientSecret: String) { + override fun cleanup(clientSecret: String) { unifiedPushStore.storeUpEndpoint(clientSecret, null) unifiedPushStore.storePushGateway(clientSecret, null) UnifiedPush.unregisterApp(context, clientSecret) diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt index 24f95d94c2..0c05748b03 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/DefaultUnregisterUnifiedPushUseCaseTest.kt @@ -41,7 +41,7 @@ class DefaultUnregisterUnifiedPushUseCaseTest { unregisterPusherResult = lambda ) ) - val result = useCase.execute(matrixClient, A_SECRET) + val result = useCase.unregister(matrixClient, A_SECRET) assertThat(result.isSuccess).isTrue() lambda.assertions() .isCalledOnce() @@ -67,7 +67,7 @@ class DefaultUnregisterUnifiedPushUseCaseTest { storePushGatewayResult = storePushGatewayResult, ), ) - val result = useCase.execute(matrixClient, A_SECRET) + val result = useCase.unregister(matrixClient, A_SECRET) assertThat(result.isSuccess).isTrue() storeUpEndpointResult.assertions() .isCalledOnce() @@ -90,7 +90,7 @@ class DefaultUnregisterUnifiedPushUseCaseTest { storePushGatewayResult = storePushGatewayResult, ), ) - val result = useCase.execute(matrixClient, A_SECRET) + val result = useCase.unregister(matrixClient, A_SECRET) assertThat(result.isSuccess).isTrue() storeUpEndpointResult.assertions() .isCalledOnce() @@ -112,7 +112,7 @@ class DefaultUnregisterUnifiedPushUseCaseTest { unregisterPusherResult = { _, _, _ -> Result.failure(AN_EXCEPTION) } ) ) - val result = useCase.execute(matrixClient, A_SECRET) + val result = useCase.unregister(matrixClient, A_SECRET) assertThat(result.isFailure).isTrue() } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt index 145681de8c..d04ef25bfb 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/FakeUnregisterUnifiedPushUseCase.kt @@ -11,9 +11,14 @@ import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.tests.testutils.lambda.lambdaError class FakeUnregisterUnifiedPushUseCase( - private val result: (MatrixClient, String) -> Result = { _, _ -> lambdaError() } + private val unregisterLambda: (MatrixClient, String) -> Result = { _, _ -> lambdaError() }, + private val cleanupLambda: (String) -> Unit = { lambdaError() }, ) : UnregisterUnifiedPushUseCase { - override suspend fun execute(matrixClient: MatrixClient, clientSecret: String): Result { - return result(matrixClient, clientSecret) + override suspend fun unregister(matrixClient: MatrixClient, clientSecret: String): Result { + return unregisterLambda(matrixClient, clientSecret) + } + + override fun cleanup(clientSecret: String) { + cleanupLambda(clientSecret) } } diff --git a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt index 93e6a106e9..6f6919947d 100644 --- a/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt +++ b/libraries/pushproviders/unifiedpush/src/test/kotlin/io/element/android/libraries/pushproviders/unifiedpush/UnifiedPushProviderTest.kt @@ -117,13 +117,13 @@ class UnifiedPushProviderTest { fun `unregister ok`() = runTest { val matrixClient = FakeMatrixClient() val getSecretForUserResultLambda = lambdaRecorder { A_SECRET } - val executeLambda = lambdaRecorder> { _, _ -> Result.success(Unit) } + val unregisterLambda = lambdaRecorder> { _, _ -> Result.success(Unit) } val unifiedPushProvider = createUnifiedPushProvider( pushClientSecret = FakePushClientSecret( getSecretForUserResult = getSecretForUserResultLambda, ), unRegisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( - result = executeLambda, + unregisterLambda = unregisterLambda, ), ) val result = unifiedPushProvider.unregister(matrixClient) @@ -131,7 +131,7 @@ class UnifiedPushProviderTest { getSecretForUserResultLambda.assertions() .isCalledOnce() .with(value(A_SESSION_ID)) - executeLambda.assertions() + unregisterLambda.assertions() .isCalledOnce() .with(value(matrixClient), value(A_SECRET)) } @@ -140,13 +140,13 @@ class UnifiedPushProviderTest { fun `unregister ko`() = runTest { val matrixClient = FakeMatrixClient() val getSecretForUserResultLambda = lambdaRecorder { A_SECRET } - val executeLambda = lambdaRecorder> { _, _ -> Result.failure(AN_EXCEPTION) } + val unregisterLambda = lambdaRecorder> { _, _ -> Result.failure(AN_EXCEPTION) } val unifiedPushProvider = createUnifiedPushProvider( pushClientSecret = FakePushClientSecret( getSecretForUserResult = getSecretForUserResultLambda, ), unRegisterUnifiedPushUseCase = FakeUnregisterUnifiedPushUseCase( - result = executeLambda, + unregisterLambda = unregisterLambda, ), ) val result = unifiedPushProvider.unregister(matrixClient) @@ -154,7 +154,7 @@ class UnifiedPushProviderTest { getSecretForUserResultLambda.assertions() .isCalledOnce() .with(value(A_SESSION_ID)) - executeLambda.assertions() + unregisterLambda.assertions() .isCalledOnce() .with(value(matrixClient), value(A_SECRET)) } diff --git a/libraries/pushstore/impl/build.gradle.kts b/libraries/pushstore/impl/build.gradle.kts index cca5f3445a..c4eafd896c 100644 --- a/libraries/pushstore/impl/build.gradle.kts +++ b/libraries/pushstore/impl/build.gradle.kts @@ -28,7 +28,6 @@ dependencies { implementation(projects.libraries.core) implementation(projects.libraries.matrix.api) implementation(projects.libraries.pushstore.api) - implementation(projects.libraries.sessionStorage.api) implementation(libs.androidx.corektx) implementation(libs.androidx.datastore.preferences) @@ -41,7 +40,6 @@ dependencies { testImplementation(projects.libraries.matrix.test) testImplementation(projects.services.appnavstate.test) testImplementation(projects.libraries.pushstore.test) - testImplementation(projects.libraries.sessionStorage.test) androidTestImplementation(libs.coroutines.test) androidTestImplementation(libs.test.core) diff --git a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt index 714d8bece7..f4cb77f01e 100644 --- a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt +++ b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/DefaultUserPushStoreFactory.kt @@ -15,21 +15,14 @@ import io.element.android.libraries.di.SingleIn import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushstore.api.UserPushStore import io.element.android.libraries.pushstore.api.UserPushStoreFactory -import io.element.android.libraries.sessionstorage.api.observer.SessionListener -import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import java.util.concurrent.ConcurrentHashMap import javax.inject.Inject @SingleIn(AppScope::class) -@ContributesBinding(AppScope::class, boundType = UserPushStoreFactory::class) +@ContributesBinding(AppScope::class) class DefaultUserPushStoreFactory @Inject constructor( @ApplicationContext private val context: Context, - private val sessionObserver: SessionObserver, -) : UserPushStoreFactory, SessionListener { - init { - observeSessions() - } - +) : UserPushStoreFactory { // We can have only one class accessing a single data store, so keep a cache of them. private val cache = ConcurrentHashMap() override fun getOrCreate(userId: SessionId): UserPushStore { @@ -40,17 +33,4 @@ class DefaultUserPushStoreFactory @Inject constructor( ) } } - - private fun observeSessions() { - sessionObserver.addListener(this) - } - - override suspend fun onSessionCreated(userId: String) { - // Nothing to do - } - - override suspend fun onSessionDeleted(userId: String) { - // Delete the store - getOrCreate(SessionId(userId)).reset() - } } diff --git a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecret.kt b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecret.kt index c4404c7c21..d488b69213 100644 --- a/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecret.kt +++ b/libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecret.kt @@ -9,26 +9,17 @@ package io.element.android.libraries.pushstore.impl.clientsecret import com.squareup.anvil.annotations.ContributesBinding import io.element.android.libraries.di.AppScope -import io.element.android.libraries.di.SingleIn import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecret import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretFactory import io.element.android.libraries.pushstore.api.clientsecret.PushClientSecretStore -import io.element.android.libraries.sessionstorage.api.observer.SessionListener -import io.element.android.libraries.sessionstorage.api.observer.SessionObserver import javax.inject.Inject -@SingleIn(AppScope::class) -@ContributesBinding(AppScope::class, boundType = PushClientSecret::class) +@ContributesBinding(AppScope::class) class DefaultPushClientSecret @Inject constructor( private val pushClientSecretFactory: PushClientSecretFactory, private val pushClientSecretStore: PushClientSecretStore, - private val sessionObserver: SessionObserver, -) : PushClientSecret, SessionListener { - init { - observeSessions() - } - +) : PushClientSecret { override suspend fun getSecretForUser(userId: SessionId): String { val existingSecret = pushClientSecretStore.getSecret(userId) if (existingSecret != null) { @@ -42,17 +33,4 @@ class DefaultPushClientSecret @Inject constructor( override suspend fun getUserIdFromSecret(clientSecret: String): SessionId? { return pushClientSecretStore.getUserIdFromSecret(clientSecret) } - - private fun observeSessions() { - sessionObserver.addListener(this) - } - - override suspend fun onSessionCreated(userId: String) { - // Nothing to do - } - - override suspend fun onSessionDeleted(userId: String) { - // Delete the secret - pushClientSecretStore.resetSecret(SessionId(userId)) - } } diff --git a/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecretTest.kt b/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecretTest.kt index 6c02ec2eb0..f4569468d7 100644 --- a/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecretTest.kt +++ b/libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/DefaultPushClientSecretTest.kt @@ -10,7 +10,6 @@ package io.element.android.libraries.pushstore.impl.clientsecret import com.google.common.truth.Truth.assertThat import io.element.android.libraries.matrix.api.core.SessionId import io.element.android.libraries.pushstore.test.userpushstore.clientsecret.InMemoryPushClientSecretStore -import io.element.android.libraries.sessionstorage.test.observer.NoOpSessionObserver import kotlinx.coroutines.test.runTest import org.junit.Test @@ -24,11 +23,10 @@ internal class DefaultPushClientSecretTest { fun test() = runTest { val factory = FakePushClientSecretFactory() val store = InMemoryPushClientSecretStore() - val sut = DefaultPushClientSecret(factory, store, NoOpSessionObserver()) + val sut = DefaultPushClientSecret(factory, store) val secret0 = factory.getSecretForUser(0) val secret1 = factory.getSecretForUser(1) - val secret2 = factory.getSecretForUser(2) assertThat(store.getSecrets()).isEmpty() assertThat(sut.getUserIdFromSecret(secret0)).isNull() @@ -48,16 +46,10 @@ internal class DefaultPushClientSecretTest { // Unknown secret assertThat(sut.getUserIdFromSecret(A_UNKNOWN_SECRET)).isNull() - // User signs out - sut.onSessionDeleted(A_USER_ID_0.value) - assertThat(store.getSecrets()).hasSize(1) - // Create a new secret after reset - assertThat(sut.getSecretForUser(A_USER_ID_0)).isEqualTo(secret2) - // Check the store content assertThat(store.getSecrets()).isEqualTo( mapOf( - A_USER_ID_0 to secret2, + A_USER_ID_0 to secret0, A_USER_ID_1 to secret1, ) )