Merge pull request #2237 from element-hq/feature/bma/longUserId

Be robust with long userId

libraries/pushstore/api/src/main/kotlin/io/element/android/libraries/pushstore/api/clientsecret/PushClientSecret.kt

@@ -29,9 +29,4 @@ interface PushClientSecret {
      * Return null if not found.
      */
     suspend fun getUserIdFromSecret(clientSecret: String): SessionId?
-
-    /**
-     * To call when the user signs out.
-     */
-    suspend fun resetSecretForUser(userId: SessionId)
 }

libraries/pushstore/impl/build.gradle.kts

@@ -34,6 +34,7 @@ anvil {
 dependencies {
     implementation(libs.dagger)
     implementation(projects.libraries.architecture)
+    implementation(projects.libraries.androidutils)
     implementation(projects.libraries.core)
     implementation(projects.libraries.matrix.api)
     implementation(projects.libraries.pushstore.api)
@@ -48,6 +49,7 @@ dependencies {
     testImplementation(libs.coroutines.test)
     testImplementation(projects.libraries.matrix.test)
     testImplementation(projects.services.appnavstate.test)
+    testImplementation(projects.libraries.sessionStorage.test)
 
     androidTestImplementation(libs.coroutines.test)
     androidTestImplementation(libs.test.core)

libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/UserPushStoreDataStore.kt

@@ -23,12 +23,15 @@ import androidx.datastore.preferences.core.booleanPreferencesKey
 import androidx.datastore.preferences.core.edit
 import androidx.datastore.preferences.core.stringPreferencesKey
 import androidx.datastore.preferences.preferencesDataStore
+import androidx.datastore.preferences.preferencesDataStoreFile
+import io.element.android.libraries.androidutils.hash.hash
 import io.element.android.libraries.core.bool.orTrue
 import io.element.android.libraries.matrix.api.core.SessionId
 import io.element.android.libraries.pushstore.api.UserPushStore
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.map
+import timber.log.Timber
 
 /**
  * Store data related to push about a user.
@@ -37,7 +40,24 @@ class UserPushStoreDataStore(
     private val context: Context,
     userId: SessionId,
 ) : UserPushStore {
-    private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = "push_store_$userId")
+    // Hash the sessionId to get rid of exotic chars and take only the first 16 chars.
+    // The risk of collision is not high.
+    private val preferenceName = "push_store_${userId.value.hash().take(16)}"
+
+    init {
+        // Migrate legacy data. Previous file can be too long if the userId is too long. The userId can be up to 255 chars.
+        // Example of long file path, with `averylonguserid` replacing a very longer name
+        // /data/user/0/io.element.android.x.debug/files/datastore/push_store_@averylonguserid:example.org.preferences_pb
+        val legacyFile = context.preferencesDataStoreFile("push_store_$userId")
+        if (legacyFile.exists()) {
+            Timber.d("Migrating legacy push data store for $userId")
+            if (!legacyFile.renameTo(context.preferencesDataStoreFile(preferenceName))) {
+                Timber.w("Failed to migrate legacy push data store for $userId")
+            }
+        }
+    }
+
+    private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = preferenceName)
     private val pushProviderName = stringPreferencesKey("pushProviderName")
     private val currentPushKey = stringPreferencesKey("currentPushKey")
     private val notificationEnabled = booleanPreferencesKey("notificationEnabled")
@@ -80,5 +100,7 @@ class UserPushStoreDataStore(
         context.dataStore.edit {
             it.clear()
         }
+        // Also delete the file
+        context.preferencesDataStoreFile(preferenceName).delete()
     }
 }

libraries/pushstore/impl/src/main/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImpl.kt

@@ -18,17 +18,26 @@ 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
 
-@ContributesBinding(AppScope::class)
+@SingleIn(AppScope::class)
+@ContributesBinding(AppScope::class, boundType = PushClientSecret::class)
 class PushClientSecretImpl @Inject constructor(
     private val pushClientSecretFactory: PushClientSecretFactory,
     private val pushClientSecretStore: PushClientSecretStore,
-) : PushClientSecret {
+    private val sessionObserver: SessionObserver,
+) : PushClientSecret, SessionListener {
+    init {
+        observeSessions()
+    }
+
     override suspend fun getSecretForUser(userId: SessionId): String {
         val existingSecret = pushClientSecretStore.getSecret(userId)
         if (existingSecret != null) {
@@ -43,7 +52,16 @@ class PushClientSecretImpl @Inject constructor(
         return pushClientSecretStore.getUserIdFromSecret(clientSecret)
     }
 
-    override suspend fun resetSecretForUser(userId: SessionId) {
-        pushClientSecretStore.resetSecret(userId)
+    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))
     }
 }

libraries/pushstore/impl/src/test/kotlin/io/element/android/libraries/pushstore/impl/clientsecret/PushClientSecretImplTest.kt

@@ -18,6 +18,7 @@ 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.sessionstorage.test.observer.NoOpSessionObserver
 import kotlinx.coroutines.test.runTest
 import org.junit.Test
 
@@ -31,7 +32,7 @@ internal class PushClientSecretImplTest {
     fun test() = runTest {
         val factory = FakePushClientSecretFactory()
         val store = InMemoryPushClientSecretStore()
-        val sut = PushClientSecretImpl(factory, store)
+        val sut = PushClientSecretImpl(factory, store, NoOpSessionObserver())
 
         val secret0 = factory.getSecretForUser(0)
         val secret1 = factory.getSecretForUser(1)
@@ -56,7 +57,7 @@ internal class PushClientSecretImplTest {
         assertThat(sut.getUserIdFromSecret(A_UNKNOWN_SECRET)).isNull()
 
         // User signs out
-        sut.resetSecretForUser(A_USER_ID_0)
+        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)