Fix onAdditionalDetails being called multiple times

RELEASE_NOTES.md

@@ -13,6 +13,7 @@
 - Overriding some of the XML styles without specifying a parent style no longer causes a build error.
 - Not defining `?android:attr/textColor` in your own theme will no longer crash.
 - The build output should no longer contain warnings about multiple substitutions specified in non-positional format in string resources.
+- In some edge cases `onAdditionalDetails` was triggered multiple times, this no longer happens.
 
 ## Removed
 - The functions to get specific configurations from `CheckoutConfiguration` (such as `CheckoutConfiguration.getDropInConfiguration()` or `CheckoutConfiguration.getCardConfiguration()`) are no longer accessible. Pass the `CheckoutConfiguration` object as it is when starting Drop-in or Components.

components-core/src/main/java/com/adyen/checkout/components/core/internal/data/api/StatusRepository.kt

@@ -9,25 +9,34 @@
 package com.adyen.checkout.components.core.internal.data.api
 
 import androidx.annotation.RestrictTo
+import androidx.annotation.VisibleForTesting
 import com.adyen.checkout.components.core.internal.data.model.StatusRequest
 import com.adyen.checkout.components.core.internal.data.model.StatusResponse
 import com.adyen.checkout.components.core.internal.util.StatusResponseUtils
+import com.adyen.checkout.components.core.internal.util.bufferedChannel
 import com.adyen.checkout.core.AdyenLogLevel
 import com.adyen.checkout.core.internal.util.adyenLog
 import com.adyen.checkout.core.internal.util.runSuspendCatching
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.cancel
 import kotlinx.coroutines.currentCoroutineContext
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.debounce
 import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.merge
+import kotlinx.coroutines.flow.onEach
+import kotlinx.coroutines.flow.receiveAsFlow
+import kotlinx.coroutines.flow.transform
 import kotlinx.coroutines.isActive
 import kotlinx.coroutines.withContext
-import java.util.concurrent.TimeUnit
+import kotlin.time.Duration.Companion.milliseconds
+import kotlin.time.Duration.Companion.seconds
+import kotlin.time.TimeMark
+import kotlin.time.TimeSource
 
 @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
 interface StatusRepository {
@@ -41,38 +50,51 @@ interface StatusRepository {
 class DefaultStatusRepository(
     private val statusService: StatusService,
     private val clientKey: String,
+    private val timeSource: TimeSource = TimeSource.Monotonic,
     private val coroutineDispatcher: CoroutineDispatcher = Dispatchers.IO,
 ) : StatusRepository {
 
     private var delay: Long = 0
 
-    private val refreshFlow: MutableSharedFlow<String> = MutableSharedFlow(extraBufferCapacity = 1)
+    private val refreshFlow = bufferedChannel<String>()
 
+    @OptIn(FlowPreview::class)
     override fun poll(paymentData: String, maxPollingDuration: Long): Flow<Result<StatusResponse>> {
-        val startTime = System.currentTimeMillis()
+        val startTime = timeSource.markNow()
+
+        updateDelay(startTime, maxPollingDuration)
 
         val pollingFlow = flow {
             while (currentCoroutineContext().isActive) {
-                val result = fetchStatus(paymentData)
+                emit(paymentData)
+                delay(delay)
+            }
+        }
+
+        return merge(
+            pollingFlow,
+            refreshFlow.receiveAsFlow(),
+        )
+            .debounce(DEBOUNCE_TIME)
+            .map {
+                fetchStatus(it)
+            }
+            .transform { result ->
                 emit(result)
 
                 if (result.isSuccess && StatusResponseUtils.isFinalResult(result.getOrThrow())) {
                     currentCoroutineContext().cancel()
                 }
 
                 if (!updateDelay(startTime, maxPollingDuration)) {
-                    emit(Result.failure(IllegalStateException("Max polling time has been exceeded.")))
+                    adyenLog(AdyenLogLevel.DEBUG) { "Max polling time exceeded" }
+                    emit(Result.failure(IllegalStateException("Max polling time exceeded.")))
                     currentCoroutineContext().cancel()
                 }
-
-                delay(delay)
             }
-        }
-
-        return merge(
-            pollingFlow,
-            refreshFlow.map { fetchStatus(it) },
-        )
+            .onEach {
+                adyenLog(AdyenLogLevel.DEBUG) { "Emitting status: ${it.getOrNull()?.resultCode}" }
+            }
     }
 
     private suspend fun fetchStatus(paymentData: String) = withContext(coroutineDispatcher) {
@@ -84,8 +106,8 @@ class DefaultStatusRepository(
     /**
      * @return Returns if the delay time was updated. If not, that means the max polling time has been exceeded.
      */
-    private fun updateDelay(startTime: Long, maxPollingDuration: Long): Boolean {
-        val elapsedTime = System.currentTimeMillis() - startTime
+    private fun updateDelay(startTime: TimeMark, maxPollingDuration: Long): Boolean {
+        val elapsedTime = startTime.elapsedNow().inWholeMilliseconds
         return when {
             elapsedTime <= POLLING_THRESHOLD -> {
                 delay = POLLING_DELAY_FAST
@@ -103,12 +125,15 @@ class DefaultStatusRepository(
 
     override fun refreshStatus(paymentData: String) {
         adyenLog(AdyenLogLevel.VERBOSE) { "refreshStatus" }
-        refreshFlow.tryEmit(paymentData)
+        refreshFlow.trySend(paymentData)
     }
 
     companion object {
-        private val POLLING_DELAY_FAST = TimeUnit.SECONDS.toMillis(2)
-        private val POLLING_DELAY_SLOW = TimeUnit.SECONDS.toMillis(10)
-        private val POLLING_THRESHOLD = TimeUnit.SECONDS.toMillis(60)
+        private val POLLING_DELAY_FAST = 2.seconds.inWholeMilliseconds
+        private val POLLING_DELAY_SLOW = 10.seconds.inWholeMilliseconds
+        private val POLLING_THRESHOLD = 60.seconds.inWholeMilliseconds
+
+        @VisibleForTesting
+        internal val DEBOUNCE_TIME = 100.milliseconds.inWholeMilliseconds
     }
 }

components-core/src/test/java/com/adyen/checkout/components/core/internal/data/api/DefaultStatusRepositoryTest.kt

@@ -8,72 +8,147 @@
 
 package com.adyen.checkout.components.core.internal.data.api
 
-import app.cash.turbine.test
 import com.adyen.checkout.components.core.internal.data.model.StatusResponse
+import com.adyen.checkout.test.LoggingExtension
 import com.adyen.checkout.test.TestDispatcherExtension
+import com.adyen.checkout.test.extensions.test
+import kotlinx.coroutines.CancellationException
 import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.cancel
+import kotlinx.coroutines.test.TestCoroutineScheduler
+import kotlinx.coroutines.test.UnconfinedTestDispatcher
+import kotlinx.coroutines.test.advanceUntilIdle
 import kotlinx.coroutines.test.runTest
+import kotlinx.coroutines.test.testTimeSource
 import org.junit.jupiter.api.Assertions.assertEquals
-import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Assertions.assertInstanceOf
+import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test
 import org.junit.jupiter.api.extension.ExtendWith
 import org.mockito.Mock
 import org.mockito.junit.jupiter.MockitoExtension
 import org.mockito.kotlin.any
 import org.mockito.kotlin.doReturn
+import org.mockito.kotlin.times
+import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
-import java.util.concurrent.TimeUnit
+import kotlin.time.Duration.Companion.minutes
+import kotlin.time.ExperimentalTime
+import kotlin.time.TimeSource
 
-@OptIn(ExperimentalCoroutinesApi::class)
-@ExtendWith(MockitoExtension::class, TestDispatcherExtension::class)
+@OptIn(ExperimentalCoroutinesApi::class, ExperimentalTime::class)
+@ExtendWith(MockitoExtension::class, TestDispatcherExtension::class, LoggingExtension::class)
 internal class DefaultStatusRepositoryTest(
     @Mock private val statusService: StatusService
 ) {
 
     private lateinit var statusRepository: DefaultStatusRepository
 
-    @BeforeEach
-    fun beforeEach() {
-        statusRepository = DefaultStatusRepository(statusService, "someclientkey")
-    }
-
     @Test
     fun `when receiving the final result, then it should be emitted and the flow should end`() = runTest {
+        statusRepository = createRepository(testScheduler, testTimeSource)
         val response = StatusResponse(resultCode = "final")
         whenever(statusService.checkStatus(any(), any())) doReturn response
 
-        statusRepository
-            .poll("paymentData", DEFAULT_MAX_POLLING_DURATION)
-            .test {
-                val expected = Result.success(response)
-                assertEquals(expected, awaitItem())
+        val statusFlow = statusRepository
+            .poll("paymentData", MAX_POLLING_DURATION)
+            .test(testScheduler)
+
+        advanceUntilIdle()
+
+        val expected = Result.success(response)
+        assertEquals(expected, statusFlow.latestValue)
 
-                cancelAndIgnoreRemainingEvents()
-            }
+        statusFlow.cancel()
     }
 
     @Test
     fun `when refreshing the status, then the result is emitted immediately`() = runTest {
+        statusRepository = createRepository(testScheduler, testTimeSource)
         val refreshResponse = StatusResponse(resultCode = "refresh")
         whenever(statusService.checkStatus(any(), any()))
-            // return final result first, so polling stops
-            .doReturn(StatusResponse(resultCode = "final"), refreshResponse)
+            .doReturn(StatusResponse(resultCode = "pending"), refreshResponse)
 
-        statusRepository
-            .poll("paymentData", DEFAULT_MAX_POLLING_DURATION)
-            .test {
-                skipItems(1)
+        val statusFlow = statusRepository
+            .poll("paymentData", MAX_POLLING_DURATION)
+            .test(testScheduler)
 
-                statusRepository.refreshStatus("test")
+        statusRepository.refreshStatus("test")
+
+        advanceUntilIdle()
+
+        val expected = Result.success(refreshResponse)
+        assertEquals(expected, statusFlow.latestValue)
+
+        statusFlow.cancel()
+    }
 
-                val expected = Result.success(refreshResponse)
-                assertEquals(expected, awaitItem())
+    @Test
+    fun `when fetching the status multiple times at the same time, then it is only fetched once`() = runTest {
+        statusRepository = createRepository(testScheduler, testTimeSource)
+        whenever(statusService.checkStatus(any(), any())) doReturn StatusResponse(resultCode = "pending")
+
+        val statusFlow = statusRepository
+            .poll("paymentData", MAX_POLLING_DURATION)
+            .test(testScheduler)
+
+        statusRepository.refreshStatus("test")
+        statusRepository.refreshStatus("test")
+        statusRepository.refreshStatus("test")
+
+        testScheduler.advanceTimeBy(DefaultStatusRepository.DEBOUNCE_TIME + 1)
+
+        verify(statusService, times(1)).checkStatus(any(), any())
+
+        statusFlow.cancel()
+    }
+
+    @Test
+    fun `when polling result is final, then the flow is cancelled`() = runTest {
+        statusRepository = createRepository(testScheduler, testTimeSource)
+        whenever(statusService.checkStatus(any(), any())) doReturn StatusResponse(resultCode = "authorised")
+
+        val statusFlow = statusRepository
+            .poll("paymentData", MAX_POLLING_DURATION)
+            .test(testScheduler)
+
+        testScheduler.advanceTimeBy(DefaultStatusRepository.DEBOUNCE_TIME + 1)
+
+        assertInstanceOf(CancellationException::class.java, statusFlow.completionThrowable)
+
+        statusFlow.cancel()
+    }
+
+    @Test
+    fun `when max polling time is exceeded, then a value is emitted and the flow is cancelled`() = runTest {
+        statusRepository = createRepository(testScheduler, testTimeSource)
+        whenever(statusService.checkStatus(any(), any())) doReturn StatusResponse(resultCode = "pending")
+
+        val statusFlow = statusRepository
+            .poll("paymentData", MAX_POLLING_DURATION)
+            .test(testScheduler)
+
+        testScheduler.advanceTimeBy(MAX_POLLING_DURATION + 2100)
+
+        assertTrue(statusFlow.latestValue.isFailure)
+        assertInstanceOf(CancellationException::class.java, statusFlow.completionThrowable)
+
+        statusFlow.cancel()
+    }
 
-                cancelAndIgnoreRemainingEvents()
-            }
+    private fun createRepository(
+        testScheduler: TestCoroutineScheduler,
+        testTimeSource: TimeSource
+    ): DefaultStatusRepository {
+        return DefaultStatusRepository(
+            statusService = statusService,
+            clientKey = "someclientkey",
+            timeSource = testTimeSource,
+            coroutineDispatcher = UnconfinedTestDispatcher(testScheduler),
+        )
     }
 
     companion object {
-        private val DEFAULT_MAX_POLLING_DURATION = TimeUnit.MINUTES.toMillis(10)
+        private val MAX_POLLING_DURATION = 1.minutes.inWholeMilliseconds
     }
 }

test-core/src/main/java/com/adyen/checkout/test/extensions/TestFlow.kt

@@ -13,6 +13,7 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.launchIn
+import kotlinx.coroutines.flow.onCompletion
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.test.TestCoroutineScheduler
 import kotlinx.coroutines.test.UnconfinedTestDispatcher
@@ -35,9 +36,13 @@ class TestFlow<T> internal constructor(flow: Flow<T>, testScheduler: TestCorouti
 
     val latestValue: T get() = values.last()
 
+    var completionThrowable: Throwable? = null
+        private set
+
     init {
         flow
             .onEach { _values.add(it) }
+            .onCompletion { completionThrowable = it }
             .launchIn(this)
     }
 }