[Reliability] Keep crash-report user identity current

Simplenote/src/main/java/com/automattic/simplenote/utils/crashlogging/SimplenoteCrashLoggingDataProvider.kt

@@ -1,5 +1,6 @@
 package com.automattic.simplenote.utils.crashlogging
 
+import android.content.SharedPreferences
 import android.util.Log
 import com.automattic.android.tracks.crashlogging.CrashLoggingDataProvider
 import com.automattic.android.tracks.crashlogging.CrashLoggingUser
@@ -11,11 +12,16 @@ import com.automattic.simplenote.BuildConfig
 import com.automattic.simplenote.Simplenote
 import com.automattic.simplenote.repositories.PreferencesRepository
 import com.automattic.simplenote.utils.locale.LocaleProvider
-import com.simperium.client.User
+import com.simperium.android.AndroidClient
+import com.simperium.android.AsyncAuthClient.USER_ACCESS_TOKEN_PREFERENCE
+import com.simperium.android.AsyncAuthClient.USER_EMAIL_PREFERENCE
+import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.catch
+import kotlinx.coroutines.flow.callbackFlow
+import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.emptyFlow
-import kotlinx.coroutines.flow.flow
+import kotlinx.coroutines.flow.map
 import java.util.Locale
 import javax.inject.Inject
 import javax.inject.Provider
@@ -83,21 +89,32 @@ class SimplenoteCrashLoggingDataProvider @Inject constructor(
     }
 
     private fun provideUser(): Flow<CrashLoggingUser?> =
-        flow {
-            emit(app.simperium?.user?.toCrashLoggingUser())
-        }.catch { e ->
+        callbackFlow {
+            val preferences = AndroidClient.sharedPreferences(app)
+            val listener = SharedPreferences.OnSharedPreferenceChangeListener { _, key ->
+                if (key == null || key == USER_ACCESS_TOKEN_PREFERENCE || key == USER_EMAIL_PREFERENCE) {
+                    trySend(preferences)
+                }
+            }
+            preferences.registerOnSharedPreferenceChangeListener(listener)
+            try {
+                trySend(preferences)
+                awaitClose()
+            } finally {
+                preferences.unregisterOnSharedPreferenceChangeListener(listener)
+            }
+        }.map { it.toCrashLoggingUser() }.distinctUntilChanged().catch { e ->
             Log.e(TAG, "Exception getting the user", e)
             emit(null)
         }
 
-    private fun User.toCrashLoggingUser(): CrashLoggingUser? {
-        if (userId.isNullOrEmpty()) return null
+    private fun SharedPreferences.toCrashLoggingUser(): CrashLoggingUser? {
+        val snapshot = all
+        val accessToken = snapshot[USER_ACCESS_TOKEN_PREFERENCE] as? String
+        val email = snapshot[USER_EMAIL_PREFERENCE] as? String
+        if (accessToken.isNullOrBlank() || email.isNullOrBlank()) return null
 
-        return CrashLoggingUser(
-            userID = userId,
-            email = email.orEmpty(),
-            username = ""
-        )
+        return CrashLoggingUser(email = email)
     }
 
     companion object {

Simplenote/src/test/java/com/automattic/simplenote/utils/crashlogging/SimplenoteCrashLoggingDataProviderTest.kt

@@ -1,18 +1,37 @@
 package com.automattic.simplenote.utils.crashlogging
 
+import android.content.Context
+import android.content.SharedPreferences
+import com.automattic.android.tracks.crashlogging.CrashLoggingUser
 import com.automattic.simplenote.Simplenote
 import com.automattic.simplenote.models.Preferences
 import com.automattic.simplenote.repositories.PreferencesRepository
 import com.automattic.simplenote.utils.locale.LocaleProvider
+import com.simperium.android.AndroidClient
+import com.simperium.android.AsyncAuthClient.USER_ACCESS_TOKEN_PREFERENCE
+import com.simperium.android.AsyncAuthClient.USER_EMAIL_PREFERENCE
 import com.simperium.client.Bucket
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.cancelAndJoin
+import kotlinx.coroutines.flow.first
+import kotlinx.coroutines.flow.take
+import kotlinx.coroutines.flow.toList
+import kotlinx.coroutines.launch
+import kotlinx.coroutines.test.runCurrent
+import kotlinx.coroutines.test.runTest
+import org.junit.Assert.assertEquals
 import org.junit.Assert.assertFalse
+import org.junit.Assert.assertNull
 import org.junit.Assert.assertTrue
 import org.junit.Test
+import org.mockito.kotlin.any
+import org.mockito.kotlin.argumentCaptor
 import org.mockito.kotlin.mock
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 import javax.inject.Provider
 
+@OptIn(ExperimentalCoroutinesApi::class)
 class SimplenoteCrashLoggingDataProviderTest {
     private val localeProvider = mock<LocaleProvider>()
 
@@ -41,4 +60,143 @@ class SimplenoteCrashLoggingDataProviderTest {
         assertFalse(provider.analyticsEnabledForCrashLogging())
         verify(repository).analyticsEnabledSnapshot()
     }
+
+    @Test
+    fun userRestoresThePersistedAuthenticatedSession() = runTest {
+        val app = mock<Simplenote>()
+        val sharedPreferences = mock<SharedPreferences>()
+        val email = "person@example.com"
+        val provider = userProvider(app, sharedPreferences, { "token" }, { email })
+
+        assertEquals(CrashLoggingUser(email = email), provider.user.first())
+
+        verify(sharedPreferences).registerOnSharedPreferenceChangeListener(any())
+        verify(sharedPreferences).unregisterOnSharedPreferenceChangeListener(any())
+    }
+
+    @Test
+    fun userTracksLoginAndLogoutPreferenceChanges() = runTest {
+        val app = mock<Simplenote>()
+        val sharedPreferences = mock<SharedPreferences>()
+        var accessToken: String? = null
+        var email: String? = null
+        val provider = userProvider(app, sharedPreferences, { accessToken }, { email })
+        val users = mutableListOf<CrashLoggingUser?>()
+        val collection = launch {
+            provider.user.take(3).toList(users)
+        }
+        runCurrent()
+
+        val listener = argumentCaptor<SharedPreferences.OnSharedPreferenceChangeListener>().run {
+            verify(sharedPreferences).registerOnSharedPreferenceChangeListener(capture())
+            firstValue
+        }
+        assertEquals(listOf<CrashLoggingUser?>(null), users)
+
+        accessToken = "token"
+        email = "person@example.com"
+        listener.onSharedPreferenceChanged(sharedPreferences, USER_ACCESS_TOKEN_PREFERENCE)
+        listener.onSharedPreferenceChanged(sharedPreferences, USER_EMAIL_PREFERENCE)
+        runCurrent()
+        assertEquals(listOf(null, CrashLoggingUser(email = "person@example.com")), users)
+
+        accessToken = null
+        email = null
+        listener.onSharedPreferenceChanged(sharedPreferences, null)
+        runCurrent()
+        collection.join()
+
+        assertEquals(
+            listOf(null, CrashLoggingUser(email = "person@example.com"), null),
+            users
+        )
+        verify(sharedPreferences).unregisterOnSharedPreferenceChangeListener(listener)
+    }
+
+    @Test
+    fun userDoesNotRestoreAnOlderSnapshotAfterConcurrentLogin() = runTest {
+        val app = mock<Simplenote>()
+        val sharedPreferences = mock<SharedPreferences>()
+        var listener: SharedPreferences.OnSharedPreferenceChangeListener? = null
+        var loggedIn = false
+        whenever(
+            app.getSharedPreferences(AndroidClient.SHARED_PREFERENCES_NAME, Context.MODE_PRIVATE)
+        ).thenReturn(sharedPreferences)
+        whenever(sharedPreferences.registerOnSharedPreferenceChangeListener(any())).thenAnswer {
+            listener = it.getArgument(0)
+            null
+        }
+        whenever(sharedPreferences.all).thenAnswer {
+            if (!loggedIn) {
+                loggedIn = true
+                listener?.onSharedPreferenceChanged(sharedPreferences, USER_ACCESS_TOKEN_PREFERENCE)
+            }
+            credentialSnapshot("token", "person@example.com")
+        }
+        whenever(sharedPreferences.getString(USER_ACCESS_TOKEN_PREFERENCE, null)).thenAnswer {
+            if (!loggedIn) {
+                loggedIn = true
+                listener?.onSharedPreferenceChanged(sharedPreferences, USER_ACCESS_TOKEN_PREFERENCE)
+                null
+            } else {
+                "token"
+            }
+        }
+        whenever(sharedPreferences.getString(USER_EMAIL_PREFERENCE, null)).thenAnswer {
+            if (loggedIn) "person@example.com" else null
+        }
+        val provider = SimplenoteCrashLoggingDataProvider(
+            app,
+            localeProvider,
+            Provider { mock<PreferencesRepository>() }
+        )
+        val users = mutableListOf<CrashLoggingUser?>()
+        val collection = launch {
+            provider.user.toList(users)
+        }
+
+        runCurrent()
+
+        assertEquals(listOf(CrashLoggingUser(email = "person@example.com")), users)
+        collection.cancelAndJoin()
+        verify(sharedPreferences).unregisterOnSharedPreferenceChangeListener(listener)
+    }
+
+    @Test
+    fun userStaysAnonymousWithoutCompleteCredentials() = runTest {
+        val app = mock<Simplenote>()
+        val sharedPreferences = mock<SharedPreferences>()
+        var accessToken = " "
+        var email = "person@example.com"
+        val provider = userProvider(app, sharedPreferences, { accessToken }, { email })
+
+        assertNull(provider.user.first())
+
+        accessToken = "token"
+        email = "\t"
+        assertNull(provider.user.first())
+    }
+
+    private fun userProvider(
+        app: Simplenote,
+        sharedPreferences: SharedPreferences,
+        accessToken: () -> String?,
+        email: () -> String?,
+    ): SimplenoteCrashLoggingDataProvider {
+        whenever(
+            app.getSharedPreferences(AndroidClient.SHARED_PREFERENCES_NAME, Context.MODE_PRIVATE)
+        ).thenReturn(sharedPreferences)
+        whenever(sharedPreferences.all).thenAnswer { credentialSnapshot(accessToken(), email()) }
+        return SimplenoteCrashLoggingDataProvider(
+            app,
+            localeProvider,
+            Provider { mock<PreferencesRepository>() }
+        )
+    }
+
+    private fun credentialSnapshot(accessToken: String?, email: String?): Map<String, String> =
+        buildMap {
+            accessToken?.let { put(USER_ACCESS_TOKEN_PREFERENCE, it) }
+            email?.let { put(USER_EMAIL_PREFERENCE, it) }
+        }
 }