[WOOMOB-2734] Fix duplicated device registration API calls on login

RELEASE-NOTES.txt

@@ -4,6 +4,7 @@
 
 25.5
 -----
+- [Internal] Fixed duplicated device registration API calls fired concurrently during login [https://github.com/woocommerce/woocommerce-android/pull/16354]
 - [*] The Pay In Person toggle on the Payments screen no longer looks turned off when its status could not be loaded
 - [Internal] Woo POS: every POS analytics event now carries device_type (phone/tablet) and entry_point, so POS usage can be split by form factor and attributed to where POS was opened from [https://github.com/woocommerce/woocommerce-android/pull/16414]
 - [*] Woo POS: Remote Tap to Pay failures now explain what went wrong - phone not eligible for Tap to Pay, NFC turned off, or a payment service error - instead of a generic message or raw error text [https://github.com/woocommerce/woocommerce-android/pull/16384]

WooCommerce/src/main/kotlin/com/woocommerce/android/notifications/push/RegisterDevice.kt

@@ -21,7 +21,9 @@ import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import org.wordpress.android.fluxc.store.AccountStore
 import javax.inject.Inject
+import javax.inject.Singleton
 
+@Singleton
 class RegisterDevice @Inject constructor(
     private val appPrefsWrapper: AppPrefsWrapper,
     private val accountStore: AccountStore,
@@ -106,7 +108,7 @@ class RegisterDevice @Inject constructor(
                             pushNotificationRepository.registerPushTokenInWooCoreSystem(
                                 token = token,
                                 selectedSite = site,
-                                allowWpComFallback = trigger != Trigger.SITE_SWITCH
+                                allowWpComFallback = false
                             )
                         }
                     }

WooCommerce/src/test/kotlin/com/woocommerce/android/notifications/push/RegisterDeviceTest.kt

@@ -38,6 +38,7 @@ import org.mockito.kotlin.whenever
 import org.wordpress.android.fluxc.model.SiteModel
 import org.wordpress.android.fluxc.store.AccountStore
 import org.wordpress.android.fluxc.store.WpComPushNotificationStore
+import javax.inject.Singleton
 import kotlin.time.Duration.Companion.milliseconds
 
 @ExperimentalCoroutinesApi
@@ -115,7 +116,7 @@ class RegisterDeviceTest : BaseUnitTest(StandardTestDispatcher()) {
         // THEN
         verify(selectedSite).getIfExists()
         verify(pushNotificationRepository).shouldRegisterWooPushForSite(TEST_TOKEN, SELECTED_SITE_ID)
-        verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel)
+        verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, false)
         verify(pushNotificationRepository, never()).shouldRegisterWooPushForSite(TEST_TOKEN, SITE_ID_ONE)
         verify(pushNotificationRepository, never()).shouldRegisterWooPushForSite(TEST_TOKEN, SITE_ID_TWO)
         verify(getWooVisibleSites, never()).invoke()
@@ -151,8 +152,8 @@ class RegisterDeviceTest : BaseUnitTest(StandardTestDispatcher()) {
             // THEN
             verify(pushNotificationRepository, never()).shouldRegisterWooPushForSite(TEST_TOKEN, SITE_ID_ONE)
             verify(pushNotificationRepository, never()).shouldRegisterWooPushForSite(TEST_TOKEN, SITE_ID_TWO)
-            verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteOne)
-            verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteTwo)
+            verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteOne, false)
+            verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteTwo, false)
             verify(pushNotificationRepository).registerPushTokenInWpComSystem(TEST_TOKEN)
         }
 
@@ -219,7 +220,7 @@ class RegisterDeviceTest : BaseUnitTest(StandardTestDispatcher()) {
             val siteOneOrder = inOrder(pushNotificationRepository)
             siteOneOrder.verify(pushNotificationRepository)
                 .clearWooPushRegistrationForStaleToken(SITE_ID_ONE, TEST_TOKEN)
-            siteOneOrder.verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteOne)
+            siteOneOrder.verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteOne, false)
             verify(pushNotificationRepository).clearWooPushRegistrationForStaleToken(SITE_ID_TWO, TEST_TOKEN)
         }
 
@@ -422,7 +423,8 @@ class RegisterDeviceTest : BaseUnitTest(StandardTestDispatcher()) {
             sut(APP_FOREGROUND)
 
             // THEN
-            verify(pushNotificationRepository, never()).registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel)
+            verify(pushNotificationRepository, never())
+                .registerPushTokenInWooCoreSystem(eq(TEST_TOKEN), eq(selectedSiteModel), any())
         }
 
     @Test
@@ -459,7 +461,7 @@ class RegisterDeviceTest : BaseUnitTest(StandardTestDispatcher()) {
             // GIVEN
             runBlocking {
                 whenever(
-                    pushNotificationRepository.registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, true)
+                    pushNotificationRepository.registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, false)
                 ).doSuspendableAnswer {
                     delay(1000.milliseconds)
                     Result.success(Unit)
@@ -479,10 +481,6 @@ class RegisterDeviceTest : BaseUnitTest(StandardTestDispatcher()) {
             verify(
                 pushNotificationRepository,
                 times(1)
-            ).registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, true)
-            verify(
-                pushNotificationRepository,
-                never()
             ).registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, false)
 
             // WHEN
@@ -493,7 +491,7 @@ class RegisterDeviceTest : BaseUnitTest(StandardTestDispatcher()) {
             // THEN
             verify(
                 pushNotificationRepository,
-                times(1)
+                times(2)
             ).registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, false)
         }
 
@@ -504,7 +502,7 @@ class RegisterDeviceTest : BaseUnitTest(StandardTestDispatcher()) {
             var isFirstCall = true
             runBlocking {
                 whenever(
-                    pushNotificationRepository.registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, true)
+                    pushNotificationRepository.registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, false)
                 ).doSuspendableAnswer {
                     if (isFirstCall) {
                         isFirstCall = false
@@ -528,12 +526,59 @@ class RegisterDeviceTest : BaseUnitTest(StandardTestDispatcher()) {
             verify(
                 pushNotificationRepository,
                 times(1)
-            ).registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, true)
-            verify(pushNotificationRepository, atLeast(1)).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteOne, true)
-            verify(pushNotificationRepository, atLeast(1)).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteTwo, true)
+            ).registerPushTokenInWooCoreSystem(TEST_TOKEN, selectedSiteModel, false)
+            verify(pushNotificationRepository, atLeast(1)).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteOne, false)
+            verify(pushNotificationRepository, atLeast(1)).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteTwo, false)
+            verify(pushNotificationRepository, times(1)).registerPushTokenInWpComSystem(TEST_TOKEN)
+        }
+
+    @Test
+    fun `given multiple sites failing woo registration, when login trigger runs, then wpcom is registered exactly once`() =
+        testBlocking {
+            // GIVEN
+            runBlocking {
+                whenever(pushNotificationRepository.isWpComPushRegistered()).thenReturn(false)
+                whenever(pushNotificationRepository.registerPushTokenInWooCoreSystem(any(), any(), any()))
+                    .thenReturn(Result.failure(Exception("registration failed")))
+            }
+
+            // WHEN
+            sut(LOGIN_SUCCESS)
+
+            // THEN
+            verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteOne, false)
+            verify(pushNotificationRepository).registerPushTokenInWooCoreSystem(TEST_TOKEN, siteTwo, false)
             verify(pushNotificationRepository, times(1)).registerPushTokenInWpComSystem(TEST_TOKEN)
         }
 
+    @Test
+    fun `given login registration in progress, when a second login trigger runs, then wpcom is registered only once`() =
+        testBlocking {
+            // GIVEN
+            var isWpComRegistered = false
+            whenever(pushNotificationRepository.isWpComPushRegistered()).thenAnswer { isWpComRegistered }
+            runBlocking {
+                whenever(pushNotificationRepository.registerPushTokenInWpComSystem(any())).doSuspendableAnswer {
+                    delay(1000.milliseconds)
+                    isWpComRegistered = true
+                    WpComPushNotificationStore.RegisterDeviceResponsePayload(deviceId = "device-id-123")
+                }
+            }
+
+            // WHEN
+            sut.kickoff(LOGIN_SUCCESS)
+            sut.kickoff(LOGIN_SUCCESS)
+            advanceUntilIdle()
+
+            // THEN
+            verify(pushNotificationRepository, times(1)).registerPushTokenInWpComSystem(TEST_TOKEN)
+        }
+
+    @Test
+    fun `given instance level orchestration state, when register device is resolved by di, then it is singleton scoped`() {
+        assertThat(RegisterDevice::class.java.isAnnotationPresent(Singleton::class.java)).isTrue()
+    }
+
     private fun stubJetpackConnection(site: SiteModel) {
         whenever(site.origin).thenReturn(SiteModel.ORIGIN_WPCOM_REST)
         whenever(site.isJetpackConnected).thenReturn(true)