Skip to content

Ensure that we have only one single instance of SeenInviteStore per session #4577

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 11, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/

package io.element.android.features.invite.api

import io.element.android.libraries.matrix.api.core.SessionId
import kotlinx.coroutines.CoroutineScope

fun interface SeenInvitesStoreFactory {
fun getOrCreate(
sessionId: SessionId,
sessionCoroutineScope: CoroutineScope,
): SeenInvitesStore
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,36 +12,25 @@
import androidx.datastore.preferences.core.edit
import androidx.datastore.preferences.core.stringSetPreferencesKey
import androidx.datastore.preferences.preferencesDataStoreFile
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.features.invite.api.SeenInvitesStore
import io.element.android.libraries.androidutils.file.safeDelete
import io.element.android.libraries.androidutils.hash.hash
import io.element.android.libraries.di.ApplicationContext
import io.element.android.libraries.di.SessionScope
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.di.annotations.SessionCoroutineScope
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.matrix.api.user.CurrentSessionIdHolder
import io.element.android.libraries.sessionstorage.api.observer.SessionListener
import io.element.android.libraries.sessionstorage.api.observer.SessionObserver
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.map
import javax.inject.Inject

private val seenInvitesKey = stringSetPreferencesKey("seenInvites")

@SingleIn(SessionScope::class)
@ContributesBinding(SessionScope::class)
class DefaultSeenInvitesStore @Inject constructor(
@ApplicationContext context: Context,
currentSessionIdHolder: CurrentSessionIdHolder,
@SessionCoroutineScope sessionCoroutineScope: CoroutineScope,
class DefaultSeenInvitesStore(

Check warning on line 28 in features/invite/impl/src/main/kotlin/io/element/android/features/invite/impl/DefaultSeenInvitesStore.kt

View check run for this annotation

Codecov / codecov/patch

features/invite/impl/src/main/kotlin/io/element/android/features/invite/impl/DefaultSeenInvitesStore.kt#L28

Added line #L28 was not covered by tests
context: Context,
sessionId: SessionId,
sessionCoroutineScope: CoroutineScope,
sessionObserver: SessionObserver,
) : SeenInvitesStore {
private val sessionId: SessionId = currentSessionIdHolder.current

init {
sessionObserver.addListener(object : SessionListener {
override suspend fun onSessionCreated(userId: String) = Unit
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright 2025 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial
* Please see LICENSE files in the repository root for full details.
*/

package io.element.android.features.invite.impl

import android.content.Context
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.features.invite.api.SeenInvitesStore
import io.element.android.features.invite.api.SeenInvitesStoreFactory
import io.element.android.libraries.di.AppScope
import io.element.android.libraries.di.ApplicationContext
import io.element.android.libraries.di.SingleIn
import io.element.android.libraries.matrix.api.core.SessionId
import io.element.android.libraries.sessionstorage.api.observer.SessionObserver
import kotlinx.coroutines.CoroutineScope
import java.util.concurrent.ConcurrentHashMap
import javax.inject.Inject

@SingleIn(AppScope::class)
@ContributesBinding(AppScope::class)
class DefaultSeenInvitesStoreFactory @Inject constructor(
@ApplicationContext private val context: Context,
private val sessionObserver: SessionObserver,

Check warning on line 27 in features/invite/impl/src/main/kotlin/io/element/android/features/invite/impl/DefaultSeenInvitesStoreFactory.kt

View check run for this annotation

Codecov / codecov/patch

features/invite/impl/src/main/kotlin/io/element/android/features/invite/impl/DefaultSeenInvitesStoreFactory.kt#L25-L27

Added lines #L25 - L27 were not covered by tests
) : SeenInvitesStoreFactory {
// We can have only one class accessing a single data store, so keep a cache of them.
private val cache = ConcurrentHashMap<SessionId, SeenInvitesStore>()

Check warning on line 30 in features/invite/impl/src/main/kotlin/io/element/android/features/invite/impl/DefaultSeenInvitesStoreFactory.kt

View check run for this annotation

Codecov / codecov/patch

features/invite/impl/src/main/kotlin/io/element/android/features/invite/impl/DefaultSeenInvitesStoreFactory.kt#L30

Added line #L30 was not covered by tests

override fun getOrCreate(
sessionId: SessionId,
sessionCoroutineScope: CoroutineScope,
): SeenInvitesStore {
return cache.getOrPut(sessionId) {
DefaultSeenInvitesStore(
context = context,
sessionId = sessionId,
sessionCoroutineScope = sessionCoroutineScope,
sessionObserver = sessionObserver,
)

Check warning on line 42 in features/invite/impl/src/main/kotlin/io/element/android/features/invite/impl/DefaultSeenInvitesStoreFactory.kt

View check run for this annotation

Codecov / codecov/patch

features/invite/impl/src/main/kotlin/io/element/android/features/invite/impl/DefaultSeenInvitesStoreFactory.kt#L36-L42

Added lines #L36 - L42 were not covered by tests
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import androidx.compose.runtime.mutableStateOf
import androidx.compose.runtime.remember
import androidx.compose.runtime.rememberCoroutineScope
import im.vector.app.features.analytics.plan.JoinedRoom
import io.element.android.features.invite.api.SeenInvitesStore
import io.element.android.features.invite.api.SeenInvitesStoreFactory
import io.element.android.features.invite.api.response.AcceptDeclineInviteEvents
import io.element.android.features.invite.api.response.AcceptDeclineInviteState
import io.element.android.features.invite.api.response.ConfirmingDeclineInvite
Expand All @@ -35,8 +35,13 @@ class AcceptDeclineInvitePresenter @Inject constructor(
private val client: MatrixClient,
private val joinRoom: JoinRoom,
private val notificationCleaner: NotificationCleaner,
private val seenInvitesStore: SeenInvitesStore,
seenInvitesStoreFactory: SeenInvitesStoreFactory,
) : Presenter<AcceptDeclineInviteState> {
private val seenInvitesStore = seenInvitesStoreFactory.getOrCreate(
sessionId = client.sessionId,
sessionCoroutineScope = client.sessionCoroutineScope,
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't mind using this, but I think you could also create a @Provides method for SessionScope that would return the cached store if needed:

@Provides
fun providesSeenInvitesStore(factory: SeenInvitesStoreFactory, sessionId: SessionId, @SessionCoroutineScope sessionCoroutineScope: CoroutineScope) {
    return factory.getOrCreate(sessionId, sesionCoroutineScope)
 }

And then you can just inject the result in the session scope.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah that's a bit cleaner, will give it a try

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.


@Composable
override fun present(): AcceptDeclineInviteState {
val localCoroutineScope = rememberCoroutineScope()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ class AcceptDeclineInvitePresenterTest {
client = client,
joinRoom = FakeJoinRoom(joinRoomLambda),
notificationCleaner = notificationCleaner,
seenInvitesStore = seenInvitesStore,
seenInvitesStoreFactory = { _, _ -> seenInvitesStore },
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import androidx.compose.runtime.setValue
import dagger.assisted.Assisted
import dagger.assisted.AssistedInject
import im.vector.app.features.analytics.plan.JoinedRoom
import io.element.android.features.invite.api.SeenInvitesStore
import io.element.android.features.invite.api.SeenInvitesStoreFactory
import io.element.android.features.invite.api.response.AcceptDeclineInviteEvents
import io.element.android.features.invite.api.response.AcceptDeclineInviteState
import io.element.android.features.invite.api.response.InviteData
Expand Down Expand Up @@ -69,7 +69,7 @@ class JoinRoomPresenter @AssistedInject constructor(
private val forgetRoom: ForgetRoom,
private val acceptDeclineInvitePresenter: Presenter<AcceptDeclineInviteState>,
private val buildMeta: BuildMeta,
private val seenInvitesStore: SeenInvitesStore,
seenInvitesStoreFactory: SeenInvitesStoreFactory,
) : Presenter<JoinRoomState> {
interface Factory {
fun create(
Expand All @@ -81,6 +81,11 @@ class JoinRoomPresenter @AssistedInject constructor(
): JoinRoomPresenter
}

private val seenInvitesStore = seenInvitesStoreFactory.getOrCreate(
matrixClient.sessionId,
matrixClient.sessionCoroutineScope,
)

@Composable
override fun present(): JoinRoomState {
val coroutineScope = rememberCoroutineScope()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import dagger.Module
import dagger.Provides
import im.vector.app.features.analytics.plan.JoinedRoom
import io.element.android.features.invite.api.SeenInvitesStore
import io.element.android.features.invite.api.SeenInvitesStoreFactory
import io.element.android.features.invite.api.response.AcceptDeclineInviteState
import io.element.android.features.joinroom.impl.JoinRoomPresenter
import io.element.android.features.roomdirectory.api.RoomDescription
Expand All @@ -36,7 +36,7 @@
forgetRoom: ForgetRoom,
acceptDeclineInvitePresenter: Presenter<AcceptDeclineInviteState>,
buildMeta: BuildMeta,
seenInvitesStore: SeenInvitesStore,
seenInvitesStoreFactory: SeenInvitesStoreFactory,
): JoinRoomPresenter.Factory {
return object : JoinRoomPresenter.Factory {
override fun create(
Expand All @@ -59,7 +59,7 @@
cancelKnockRoom = cancelKnockRoom,
acceptDeclineInvitePresenter = acceptDeclineInvitePresenter,
buildMeta = buildMeta,
seenInvitesStore = seenInvitesStore,
seenInvitesStoreFactory = seenInvitesStoreFactory,

Check warning on line 62 in features/joinroom/impl/src/main/kotlin/io/element/android/features/joinroom/impl/di/JoinRoomModule.kt

View check run for this annotation

Codecov / codecov/patch

features/joinroom/impl/src/main/kotlin/io/element/android/features/joinroom/impl/di/JoinRoomModule.kt#L62

Added line #L62 was not covered by tests
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,7 @@ class JoinRoomPresenterTest {
forgetRoom = forgetRoom,
buildMeta = buildMeta,
acceptDeclineInvitePresenter = acceptDeclineInvitePresenter,
seenInvitesStore = seenInvitesStore,
seenInvitesStoreFactory = { _, _ -> seenInvitesStore },
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import android.content.Context
import coil3.SingletonImageLoader
import com.squareup.anvil.annotations.ContributesBinding
import io.element.android.features.ftue.api.state.FtueService
import io.element.android.features.invite.api.SeenInvitesStore
import io.element.android.features.invite.api.SeenInvitesStoreFactory
import io.element.android.features.preferences.impl.DefaultCacheService
import io.element.android.libraries.core.coroutine.CoroutineDispatchers
import io.element.android.libraries.di.ApplicationContext
Expand All @@ -36,8 +36,13 @@ class DefaultClearCacheUseCase @Inject constructor(
private val okHttpClient: Provider<OkHttpClient>,
private val ftueService: FtueService,
private val pushService: PushService,
private val seenInvitesStore: SeenInvitesStore,
seenInvitesStoreFactory: SeenInvitesStoreFactory,
) : ClearCacheUseCase {
private val seenInvitesStore = seenInvitesStoreFactory.getOrCreate(
matrixClient.sessionId,
matrixClient.sessionCoroutineScope,
)

override suspend fun invoke() = withContext(coroutineDispatchers.io) {
// Clear Matrix cache
matrixClient.clearCache()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class DefaultClearCacheUseCaseTest {
okHttpClient = { OkHttpClient.Builder().build() },
ftueService = ftueService,
pushService = pushService,
seenInvitesStore = seenInvitesStore,
seenInvitesStoreFactory = { _, _ -> seenInvitesStore },
)
defaultCacheService.clearedCacheEventFlow.test {
sut.invoke()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.runtime.snapshotFlow
import im.vector.app.features.analytics.plan.Interaction
import io.element.android.features.invite.api.SeenInvitesStore
import io.element.android.features.invite.api.SeenInvitesStoreFactory
import io.element.android.features.invite.api.response.AcceptDeclineInviteEvents
import io.element.android.features.invite.api.response.AcceptDeclineInviteState
import io.element.android.features.invite.api.response.InviteData
Expand Down Expand Up @@ -94,10 +94,15 @@ class RoomListPresenter @Inject constructor(
private val logoutPresenter: Presenter<DirectLogoutState>,
private val appPreferencesStore: AppPreferencesStore,
private val rageshakeFeatureAvailability: RageshakeFeatureAvailability,
private val seenInvitesStore: SeenInvitesStore,
seenInvitesStoreFactory: SeenInvitesStoreFactory,
) : Presenter<RoomListState> {
private val encryptionService: EncryptionService = client.encryptionService()

private val seenInvitesStore = seenInvitesStoreFactory.getOrCreate(
client.sessionId,
client.sessionCoroutineScope,
)

@Composable
override fun present(): RoomListState {
val coroutineScope = rememberCoroutineScope()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -716,7 +716,7 @@ class RoomListPresenterTest {
logoutPresenter = { aDirectLogoutState() },
appPreferencesStore = appPreferencesStore,
rageshakeFeatureAvailability = rageshakeFeatureAvailability,
seenInvitesStore = seenInvitesStore,
seenInvitesStoreFactory = { _, _ -> seenInvitesStore },
)
}

Expand Down
Loading