fix: fixed AuthCallbacks universal override flow routing.

Author: tal-mi

app/src/test/java/com/sap/cdc/bitsnbytes/feature/auth/AuthCallbacksEnhancementTest.kt

@@ -5,10 +5,7 @@ import com.sap.cdc.android.sdk.feature.AuthError
 import com.sap.cdc.android.sdk.feature.AuthResult
 import com.sap.cdc.android.sdk.feature.AuthSuccess
 import com.sap.cdc.android.sdk.feature.RegistrationContext
-import org.junit.Assert.assertEquals
-import org.junit.Assert.assertFalse
-import org.junit.Assert.assertNotNull
-import org.junit.Assert.assertTrue
+import org.junit.Assert.*
 import org.junit.Test
 
 /**
@@ -253,7 +250,7 @@ class AuthCallbacksEnhancementTest {
     }
 
     @Test
-    fun `test universal override can change callback type - Success to Error`() {
+    fun `test universal override routes Success to Error callbacks only`() {
         var successCallbackExecuted = false
         var errorCallbackExecuted = false
         var errorMessage: String? = null
@@ -292,20 +289,13 @@ class AuthCallbacksEnhancementTest {
             data = mapOf("original" to "data")
         )
 
-        // Execute success callback - should be converted to error by universal override
+        // Execute success callback - should be ROUTED to error callback
         callbacks.onSuccess?.invoke(testSuccess)
 
-        // The success callback should still execute (since we called onSuccess)
-        // but the data should be unchanged since universal override changed the type
-        assertTrue("Success callback should execute", successCallbackExecuted)
-        assertFalse("Error callback should not execute from success call", errorCallbackExecuted)
-        
-        // Now test with actual error to verify error callback works
-        val testError = AuthError(message = "Test error")
-        callbacks.onError?.invoke(testError)
-        
-        assertTrue("Error callback should execute", errorCallbackExecuted)
-        assertEquals("Test error", errorMessage)
+        // With the router fix, only error callback should execute
+        assertFalse("Success callback should NOT execute", successCallbackExecuted)
+        assertTrue("Error callback SHOULD execute", errorCallbackExecuted)
+        assertEquals("Converted by universal override", errorMessage)
     }
 
     @Test
@@ -340,4 +330,84 @@ class AuthCallbacksEnhancementTest {
         assertTrue("Callback should have been executed", callbackExecuted)
         assertEquals("individualOverride", transformedValue)
     }
+    
+    @Test
+    fun `test linkToProvider pattern - connectAccountSync error routes to onError only`() {
+        var successExecuted = false
+        var errorExecuted = false
+        var errorMsg: String? = null
+        
+        val callbacks = AuthCallbacks().apply {
+            doOnAnyAndOverride { authResult ->
+                when (authResult) {
+                    is AuthResult.Success -> {
+                        // Simulate connectAccountSync returning error
+                        AuthResult.Error(
+                            AuthError(
+                                message = "Connection failed",
+                                code = "CONNECT_ERROR"
+                            )
+                        )
+                    }
+                    else -> authResult
+                }
+            }
+        }.apply {
+            onSuccess = {
+                successExecuted = true
+            }
+            onError = { error ->
+                errorExecuted = true
+                errorMsg = error.message
+            }
+        }
+        
+        // Simulate signIn returning success
+        callbacks.onSuccess?.invoke(AuthSuccess("{}", emptyMap()))
+        
+        // Verify only error callback executed
+        assertFalse("Success callback should NOT execute", successExecuted)
+        assertTrue("Error callback SHOULD execute", errorExecuted)
+        assertEquals("Connection failed", errorMsg)
+    }
+    
+    @Test
+    fun `test linkToProvider pattern - connectAccountSync success executes onSuccess`() {
+        var successExecuted = false
+        var errorExecuted = false
+        var successData: Map<String, Any>? = null
+        
+        val callbacks = AuthCallbacks().apply {
+            doOnAnyAndOverride { authResult ->
+                when (authResult) {
+                    is AuthResult.Success -> {
+                        // Simulate connectAccountSync returning success with enriched data
+                        AuthResult.Success(
+                            authResult.authSuccess.copy(
+                                data = authResult.authSuccess.data + ("connected" to true)
+                            )
+                        )
+                    }
+                    else -> authResult
+                }
+            }
+        }.apply {
+            onSuccess = { authSuccess ->
+                successExecuted = true
+                successData = authSuccess.data
+            }
+            onError = {
+                errorExecuted = true
+            }
+        }
+        
+        // Simulate signIn returning success
+        callbacks.onSuccess?.invoke(AuthSuccess("{}", mapOf("account" to "linked")))
+        
+        // Verify only success callback executed with enriched data
+        assertTrue("Success callback SHOULD execute", successExecuted)
+        assertFalse("Error callback should NOT execute", errorExecuted)
+        assertEquals(true, successData?.get("connected"))
+        assertEquals("linked", successData?.get("account"))
+    }
 }

library/src/main/java/com/sap/cdc/android/sdk/feature/AuthCallbacks.kt

@@ -379,154 +379,135 @@ data class AuthCallbacks(
 
     fun hasUniversalOverride(): Boolean = _universalOverride != null
 
-    // NEW: Suspend execution methods for async callback chains
-    suspend fun executeOnSuccess(authSuccess: AuthSuccess): AuthSuccess {
-        var currentValue = authSuccess
-
+    // NEW: Central callback router that applies universal override and routes to correct callback type.
+    // This ensures that when universal override changes the result type (e.g., Success → Error),
+    // only the appropriate callbacks execute.
+    private suspend fun executeCallback(authResult: AuthResult) {
         // Apply universal override first if present
-        if (hasUniversalOverride()) {
-            val universalResult = executeUniversalOverride(AuthResult.Success(currentValue))
-            currentValue = when (universalResult) {
-                is AuthResult.Success -> universalResult.authSuccess
-                else -> currentValue // Keep original if universal override changed the type
-            }
+        val finalResult = if (hasUniversalOverride()) {
+            executeUniversalOverride(authResult)
+        } else {
+            authResult
         }
 
+        // Route to correct handler based on FINAL result type
+        when (finalResult) {
+            is AuthResult.Success -> executeOnSuccessInternal(finalResult.authSuccess)
+            is AuthResult.Error -> executeOnErrorInternal(finalResult.authError)
+            is AuthResult.PendingRegistration -> executeOnPendingRegistrationInternal(finalResult.context)
+            is AuthResult.LinkingRequired -> executeOnLinkingRequiredInternal(finalResult.context)
+            is AuthResult.TwoFactorRequired -> executeOnTwoFactorRequiredInternal(finalResult.context)
+            is AuthResult.OTPRequired -> executeOnOTPRequiredInternal(finalResult.context)
+            is AuthResult.CaptchaRequired -> executeOnCaptchaRequiredInternal()
+        }
+    }
+
+    // NEW: Public execute methods that wrap inputs and route through central router
+    suspend fun executeOnSuccess(authSuccess: AuthSuccess) {
+        executeCallback(AuthResult.Success(authSuccess))
+    }
+
+    suspend fun executeOnError(authError: AuthError) {
+        executeCallback(AuthResult.Error(authError))
+    }
+
+    suspend fun executeOnPendingRegistration(context: RegistrationContext) {
+        executeCallback(AuthResult.PendingRegistration(context))
+    }
+
+    suspend fun executeOnLinkingRequired(context: LinkingContext) {
+        executeCallback(AuthResult.LinkingRequired(context))
+    }
+
+    suspend fun executeOnTwoFactorRequired(context: TwoFactorContext) {
+        executeCallback(AuthResult.TwoFactorRequired(context))
+    }
+
+    suspend fun executeOnOTPRequired(context: OTPContext) {
+        executeCallback(AuthResult.OTPRequired(context))
+    }
+
+    suspend fun executeOnCaptchaRequired() {
+        executeCallback(AuthResult.CaptchaRequired)
+    }
+
+    // Internal execution methods - apply individual overrides and execute callbacks
+    // (Universal override is already applied by the router before reaching here)
+    private suspend fun executeOnSuccessInternal(authSuccess: AuthSuccess) {
+        var currentValue = authSuccess
+
         // Apply individual override transformers
         for (transformer in _onSuccessOverrides) {
             currentValue = transformer(currentValue)
         }
 
         // Execute all callbacks with the final transformed value
         _onSuccess.forEach { it(currentValue) }
-
-        return currentValue
     }
 
-    suspend fun executeOnError(authError: AuthError): AuthError {
+    private suspend fun executeOnErrorInternal(authError: AuthError) {
         var currentValue = authError
 
-        // Apply universal override first if present
-        if (hasUniversalOverride()) {
-            val universalResult = executeUniversalOverride(AuthResult.Error(currentValue))
-            currentValue = when (universalResult) {
-                is AuthResult.Error -> universalResult.authError
-                else -> currentValue // Keep original if universal override changed the type
-            }
-        }
-
         // Apply individual override transformers
         for (transformer in _onErrorOverrides) {
             currentValue = transformer(currentValue)
         }
 
         // Execute all callbacks with the final transformed value
         _onError.forEach { it(currentValue) }
-
-        return currentValue
     }
 
-    suspend fun executeOnPendingRegistration(context: RegistrationContext): RegistrationContext {
+    private suspend fun executeOnPendingRegistrationInternal(context: RegistrationContext) {
         var currentValue = context
 
-        // Apply universal override first if present
-        if (hasUniversalOverride()) {
-            val universalResult = executeUniversalOverride(AuthResult.PendingRegistration(currentValue))
-            currentValue = when (universalResult) {
-                is AuthResult.PendingRegistration -> universalResult.context
-                else -> currentValue // Keep original if universal override changed the type
-            }
-        }
-
         // Apply individual override transformers
         for (transformer in _onPendingRegistrationOverrides) {
             currentValue = transformer(currentValue)
         }
 
         // Execute all callbacks with the final transformed value
         _onPendingRegistration.forEach { it(currentValue) }
-
-        return currentValue
     }
 
-    suspend fun executeOnLinkingRequired(context: LinkingContext): LinkingContext {
+    private suspend fun executeOnLinkingRequiredInternal(context: LinkingContext) {
         var currentValue = context
 
-        // Apply universal override first if present
-        if (hasUniversalOverride()) {
-            val universalResult = executeUniversalOverride(AuthResult.LinkingRequired(currentValue))
-            currentValue = when (universalResult) {
-                is AuthResult.LinkingRequired -> universalResult.context
-                else -> currentValue // Keep original if universal override changed the type
-            }
-        }
-
         // Apply individual override transformers
         for (transformer in _onLinkingRequiredOverrides) {
             currentValue = transformer(currentValue)
         }
 
         // Execute all callbacks with the final transformed value
         _onLinkingRequired.forEach { it(currentValue) }
-
-        return currentValue
     }
 
-    suspend fun executeOnTwoFactorRequired(context: TwoFactorContext): TwoFactorContext {
+    private suspend fun executeOnTwoFactorRequiredInternal(context: TwoFactorContext) {
         var currentValue = context
 
-        // Apply universal override first if present
-        if (hasUniversalOverride()) {
-            val universalResult = executeUniversalOverride(AuthResult.TwoFactorRequired(currentValue))
-            currentValue = when (universalResult) {
-                is AuthResult.TwoFactorRequired -> universalResult.context
-                else -> currentValue // Keep original if universal override changed the type
-            }
-        }
-
         // Apply individual override transformers
         for (transformer in _onTwoFactorRequiredOverrides) {
             currentValue = transformer(currentValue)
         }
 
         // Execute all callbacks with the final transformed value
         _onTwoFactorRequired.forEach { it(currentValue) }
-
-        return currentValue
     }
 
-    suspend fun executeOnOTPRequired(context: OTPContext): OTPContext {
+    private suspend fun executeOnOTPRequiredInternal(context: OTPContext) {
         var currentValue = context
 
-        // Apply universal override first if present
-        if (hasUniversalOverride()) {
-            val universalResult = executeUniversalOverride(AuthResult.OTPRequired(currentValue))
-            currentValue = when (universalResult) {
-                is AuthResult.OTPRequired -> universalResult.context
-                else -> currentValue // Keep original if universal override changed the type
-            }
-        }
-
         // Apply individual override transformers
         for (transformer in _onOTPRequiredOverrides) {
             currentValue = transformer(currentValue)
         }
 
         // Execute all callbacks with the final transformed value
         _onOTPRequired.forEach { it(currentValue) }
-
-        return currentValue
     }
 
-    suspend fun executeOnCaptchaRequired() {
+    private suspend fun executeOnCaptchaRequiredInternal() {
         var currentValue = Unit
 
-        // Apply universal override first if present
-        if (hasUniversalOverride()) {
-            executeUniversalOverride(AuthResult.CaptchaRequired)
-            // Note: CaptchaRequired doesn't have data to transform, so we just execute it
-        }
-
         // Apply individual override transformers
         for (transformer in _onCaptchaRequiredOverrides) {
             currentValue = transformer(currentValue)