Keep offline data when connectUser fails

stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt

@@ -656,7 +656,8 @@ internal constructor(
                 )
             }
         }.onErrorSuspend {
-            disconnectSuspend(flushPersistence = true)
+            // A failed connection is not a logout: keep the offline data and stored credentials.
+            disconnectSuspend(flushPersistence = false)
         }
     }
 
@@ -1611,7 +1612,7 @@ internal constructor(
      * This method should only be used whenever the user logouts from the main app.
      * You shouldn't call this method, if the user will continue using the Chat in the future.
      *
-     * @param flushPersistence if true will clear user data.
+     * @param flushPersistence if true will clear user data, also when no user is connected.
      * @param deleteDevice If set to true, will attempt to delete the registered device from Stream backend. For
      * backwards compatibility, by default it's set to the value of [flushPersistence].
      *
@@ -1634,7 +1635,10 @@ internal constructor(
                     Result.Success(Unit)
                 }
 
-                false -> {
+                false -> if (flushPersistence) {
+                    // The data of a previous session can outlive it, e.g. after a failed connectUser.
+                    clearPersistence().await()
+                } else {
                     logger.i { "[disconnect] cannot disconnect as the user wasn't connected" }
                     Result.Failure(
                         Error.GenericError(

stream-chat-android-client/src/test/java/io/getstream/chat/android/client/ChatClientConnectionTests.kt

@@ -93,6 +93,7 @@ internal class ChatClientConnectionTests {
     private val streamDateFormatter = StreamDateFormatter()
     private val config = mock<ChatApiConfig>()
     private val userCredentialStorage = mock<UserCredentialStorage>()
+    private lateinit var networkStateProvider: NetworkStateProvider
 
     @BeforeEach
     fun setup() {
@@ -106,7 +107,7 @@ internal class ChatClientConnectionTests {
         val userScope = UserTestScope(clientScope)
         val lifecycleObserver = StreamLifecycleObserver(userScope, lifecycleOwner.lifecycle)
         val tokenManager = FakeTokenManager("")
-        val networkStateProvider: NetworkStateProvider = mock()
+        networkStateProvider = mock()
         whenever(networkStateProvider.isConnected()) doReturn true
         fakeChatSocket = FakeChatSocket(
             userScope = userScope,
@@ -244,6 +245,22 @@ internal class ChatClientConnectionTests {
         (result as Result.Failure).value.message `should be equal to` messageError
     }
 
+    @Test
+    fun `When the server rejects the connection, local data should be kept`() = runCancellableTest {
+        val error = Error.NetworkError(
+            message = randomString(),
+            serverErrorCode = ChatErrorCode.API_KEY_NOT_FOUND.code,
+        )
+        val event = ErrorEvent(EventType.CONNECTION_ERROR, Date(), streamDateFormatter.format(Date()), error)
+
+        val deferred = (testCoroutines.scope + Job()).async { client.connectUser(user, jwt).await() }
+        fakeChatSocket.mockEventReceived(event)
+        val result = deferred.await()
+
+        result.shouldBeInstanceOf(Result.Failure::class)
+        verify(userCredentialStorage, never()).clear()
+    }
+
     @Test
     fun `When there is an ongoing connection with the same user, an error should be propagated`() = runTest {
         userStateService.onSetUser(user, false)
@@ -262,6 +279,36 @@ internal class ChatClientConnectionTests {
         (result as Result.Failure).value.message `should be equal to` "Connection wasn't established in 1ms"
     }
 
+    @Test
+    fun `When connection times out, local data should be kept`() = runTest {
+        val result = client.connectUser(user, jwt, 1).await()
+
+        result.shouldBeInstanceOf(Result.Failure::class)
+        verify(userCredentialStorage, never()).clear()
+    }
+
+    @Test
+    fun `When the token is blank, local data should be kept`() = runTest {
+        val result = client.connectUser(user, "").await()
+
+        result.shouldBeInstanceOf(Result.Failure::class)
+        verify(userCredentialStorage, never()).clear()
+    }
+
+    @Test
+    fun `When offline without a timeout, connecting should wait for the network and keep local data`() = runCancellableTest {
+        whenever(networkStateProvider.isConnected()) doReturn false
+
+        val deferred = (testCoroutines.scope + Job()).async { client.connectUser(user, jwt).await() }
+        testScheduler.advanceUntilIdle()
+
+        deferred.isCompleted `should be equal to` false
+        verify(userCredentialStorage, never()).clear()
+        mutableClientState.initializationState.value `should be equal to` InitializationState.COMPLETE
+        client.getCurrentUser() `should be equal to` user
+        deferred.cancel()
+    }
+
     @Test
     fun `When there is an user connected and try to connect a different user, an error should be propagated`() =
         runTest {
@@ -342,14 +389,32 @@ internal class ChatClientConnectionTests {
 
     @Test
     fun `Given no connected user, calling disconnect should return error`() = runCancellableTest {
-        val flushPersistence = randomBoolean()
-        val result = client.disconnect(flushPersistence).await()
+        val result = client.disconnect(flushPersistence = false).await()
 
         result.shouldBeInstanceOf(Result.Failure::class)
         (result as Result.Failure).value.message `should be equal to`
             "ChatClient can't be disconnected because user wasn't connected previously"
     }
 
+    @Test
+    fun `Given no connected user, calling disconnect with flushPersistence should clear local data`() = runCancellableTest {
+        val result = client.disconnect(flushPersistence = true).await()
+
+        result.shouldBeInstanceOf(Result.Success::class)
+        verify(userCredentialStorage).clear()
+    }
+
+    @Test
+    fun `Given a failed connection, calling disconnect with flushPersistence should clear local data`() = runCancellableTest {
+        client.connectUser(user, jwt, 1).await()
+        verify(userCredentialStorage, never()).clear()
+
+        val result = client.disconnect(flushPersistence = true).await()
+
+        result.shouldBeInstanceOf(Result.Success::class)
+        verify(userCredentialStorage).clear()
+    }
+
     @Test
     fun `Given connected user, calling disconnect with flushPersistence should return success and clear local data`() = runCancellableTest {
         val connectionId = randomString()