improve: abort further processing/retries when config URL is empty (S…

…DKCF-6736)
rakutentech · Aug 29, 2023 · c13187b · c13187b
1 parent e247fc4
commit c13187b
Show file tree

Hide file tree

Showing 5 changed files with 46 additions and 19 deletions.
diff --git a/inappmessaging/USERGUIDE.md b/inappmessaging/USERGUIDE.md
@@ -501,6 +501,7 @@ Documents targeting Product Managers:
   - Dokka to `1.8.10`
   - Robolectric to `4.10.3`
   - Dependency Check to `8.2.1`
+* SDKCF-6736: Improved worker to abort further processing when config URL is empty.
 * SDKCF-6547: Fixed impression is not incremented when tooltip campaign is displayed.
 * SDKCF-6518: Fixed and suppressed some SonarCloud code smells.
 * Updates for RMC SDK:

diff --git a/...n/java/com/rakuten/tech/mobile/inappmessaging/runtime/workmanager/workers/ConfigWorker.kt b/...n/java/com/rakuten/tech/mobile/inappmessaging/runtime/workmanager/workers/ConfigWorker.kt
@@ -54,36 +54,38 @@ internal class ConfigWorker(
     @SuppressWarnings("TooGenericExceptionCaught")
     override fun doWork(): Result {
         InAppLogger(TAG).debug(hostRepo.getConfigUrl())
-        val hostAppId = hostRepo.getPackageName()
-        val hostAppVersion = hostRepo.getVersion()
-        val subscriptionId = hostRepo.getSubscriptionKey()
-        // Terminate request if any of the following values are empty: appId, appVersion or subscription key.
-        if (hostAppId.isEmpty() || hostAppVersion.isEmpty() || subscriptionId.isEmpty()) {
+        // Terminate request if any of the following values are empty
+        if (!isConfigValid()) {
             return Result.failure()
         }
 
         return try {
             // Executing the API network call.
-            onResponse(setupCall(hostAppId, hostAppVersion, subscriptionId).execute())
+            onResponse(setupCall().execute())
         } catch (e: Exception) {
             InAppLogger(TAG).error(e.message)
             // RETRY by default has exponential backoff baked in.
             Result.retry()
         }
     }
 
-    private fun setupCall(hostAppId: String, hostAppVersion: String, subscriptionId: String): Call<ConfigResponse> {
+    private fun isConfigValid() = hostRepo.getConfigUrl().isNotEmpty() &&
+        hostRepo.getPackageName().isNotEmpty() &&
+        hostRepo.getVersion().isNotEmpty() &&
+        hostRepo.getSubscriptionKey().isNotEmpty()
+
+    private fun setupCall(): Call<ConfigResponse> {
         val params = ConfigQueryParamsBuilder(
-            appId = hostAppId,
+            appId = hostRepo.getPackageName(),
             locale = hostRepo.getDeviceLocale(),
-            appVersion = hostAppVersion,
+            appVersion = hostRepo.getVersion(),
             sdkVersion = BuildConfig.VERSION_NAME,
             rmcSdkVersion = hostRepo.getRmcSdkVersion(),
         ).queryParams
         return RuntimeUtil.getRetrofit()
             .create(ConfigRetrofitService::class.java)
             .getConfigService(
-                url = hostRepo.getConfigUrl(), subscriptionId = subscriptionId,
+                url = hostRepo.getConfigUrl(), subscriptionId = hostRepo.getSubscriptionKey(),
                 deviceId = hostRepo.getDeviceId(), parameters = params,
             )
     }

diff --git a/...c/test/java/com/rakuten/tech/mobile/inappmessaging/runtime/InAppMessagingTestConstants.kt b/...c/test/java/com/rakuten/tech/mobile/inappmessaging/runtime/InAppMessagingTestConstants.kt
@@ -6,6 +6,7 @@ object InAppMessagingTestConstants {
     const val APP_ID = "com.rakuten.test"
     val LOCALE = Locale.getDefault()
     const val APP_VERSION = "0.0.1"
+    const val CONFIG_URL = "https://config"
     const val SUB_KEY = "12345-abcde"
     const val DEVICE_ID = "3a57f343769516eb"
     const val ACC = 1

diff --git a/...c/test/java/com/rakuten/tech/mobile/inappmessaging/runtime/integration/IntegrationSpec.kt b/...c/test/java/com/rakuten/tech/mobile/inappmessaging/runtime/integration/IntegrationSpec.kt
@@ -51,8 +51,8 @@ class IntegrationSpec {
             context, workerParameters, HostAppInfoRepository.instance(),
             ConfigResponseRepository.instance(), mockMessageScheduler,
         )
-        val expected = if (HostAppInfoRepository.instance().getConfigUrl().isNullOrEmpty()) {
-            ListenableWorker.Result.retry()
+        val expected = if (HostAppInfoRepository.instance().getConfigUrl().isEmpty()) {
+            ListenableWorker.Result.failure()
         } else {
             ListenableWorker.Result.success()
         }
@@ -81,7 +81,7 @@ class IntegrationSpec {
             CampaignRepository.instance().lastSyncMillis?.shouldBeGreaterThan(0)
             CampaignRepository.instance().messages.shouldBeEmpty()
         } else {
-            result shouldBeEqualTo ListenableWorker.Result.retry()
+            result shouldBeEqualTo ListenableWorker.Result.failure()
         }
     }
 }
diff --git a/...va/com/rakuten/tech/mobile/inappmessaging/runtime/workmanager/workers/ConfigWorkerSpec.kt b/...va/com/rakuten/tech/mobile/inappmessaging/runtime/workmanager/workers/ConfigWorkerSpec.kt
@@ -10,6 +10,7 @@ import androidx.work.testing.WorkManagerTestInitHelper
 import com.nhaarman.mockitokotlin2.anyOrNull
 import com.nhaarman.mockitokotlin2.eq
 import com.rakuten.tech.mobile.inappmessaging.runtime.BaseTest
+import com.rakuten.tech.mobile.inappmessaging.runtime.BuildConfig
 import com.rakuten.tech.mobile.inappmessaging.runtime.InApp
 import com.rakuten.tech.mobile.inappmessaging.runtime.InAppMessaging
 import com.rakuten.tech.mobile.inappmessaging.runtime.InAppMessagingTestConstants
@@ -57,6 +58,11 @@ open class ConfigWorkerSpec : BaseTest() {
         ConfigScheduler.currDelay = RetryDelayUtil.INITIAL_BACKOFF_DELAY
         ConfigResponseRepository.resetInstance()
         ConfigWorker.serverErrorCounter.set(0)
+
+        `when`(mockHostRepo.getConfigUrl()).thenReturn(InAppMessagingTestConstants.CONFIG_URL)
+        `when`(mockHostRepo.getPackageName()).thenReturn(InAppMessagingTestConstants.APP_ID)
+        `when`(mockHostRepo.getVersion()).thenReturn(BuildConfig.VERSION_NAME)
+        `when`(mockHostRepo.getSubscriptionKey()).thenReturn(InAppMessagingTestConstants.SUB_KEY)
     }
 
     internal fun initializeInstance() {
@@ -111,7 +117,7 @@ class ConfigWorkerSuccessSpec : ConfigWorkerSpec() {
         `when`(mockHostRepo.getSubscriptionKey()).thenReturn(bundle.getString(SUB_KEY, ""))
         val worker = ConfigWorker(this.ctx, workParam, mockHostRepo, mockConfigRepo, mockMsgSched)
         val expected = if (bundle.getString(CONFIG_KEY, "").isNullOrEmpty()) {
-            ListenableWorker.Result.retry()
+            ListenableWorker.Result.failure()
         } else {
             ListenableWorker.Result.success()
         }
@@ -206,6 +212,16 @@ class ConfigWorkerSuccessSpec : ConfigWorkerSpec() {
     }
 }
 
+class ConfigWorkerRetrySpec : ConfigWorkerSpec() {
+    @Test
+    fun `should return retry when exception is encountered`() {
+        `when`(mockHostRepo.getDeviceId()).thenThrow(IllegalStateException())
+
+        val worker = ConfigWorker(this.ctx, workParam, mockHostRepo, mockConfigRepo, mockMsgSched)
+        worker.doWork() shouldBeEqualTo ListenableWorker.Result.retry()
+    }
+}
+
 class ConfigWorkerFailSpec : ConfigWorkerSpec() {
     @Test
     fun `should fail if server fail more than 3 times`() {
@@ -244,7 +260,17 @@ class ConfigWorkerFailSpec : ConfigWorkerSpec() {
     }
 
     @Test
-    fun `should fail if hostapp id is empty`() {
+    fun `should fail if config URL is empty`() {
+        `when`(mockHostRepo.getConfigUrl()).thenReturn("")
+        val worker = ConfigWorker(
+            ctx, workParam, mockHostRepo, ConfigResponseRepository.instance(),
+            MessageMixerPingScheduler.instance(),
+        )
+        worker.doWork() shouldBeEqualTo ListenableWorker.Result.failure()
+    }
+
+    @Test
+    fun `should fail if host app id is empty`() {
         `when`(mockHostRepo.getPackageName()).thenReturn("")
         val worker = ConfigWorker(
             ctx, workParam, mockHostRepo, ConfigResponseRepository.instance(),
@@ -254,8 +280,7 @@ class ConfigWorkerFailSpec : ConfigWorkerSpec() {
     }
 
     @Test
-    fun `should fail if hostapp version is empty`() {
-        `when`(mockHostRepo.getPackageName()).thenReturn("valid.package.name")
+    fun `should fail if host app version is empty`() {
         `when`(mockHostRepo.getVersion()).thenReturn("")
         val worker = ConfigWorker(
             ctx, workParam, mockHostRepo, ConfigResponseRepository.instance(),
@@ -266,8 +291,6 @@ class ConfigWorkerFailSpec : ConfigWorkerSpec() {
 
     @Test
     fun `should fail if subscription key is empty`() {
-        `when`(mockHostRepo.getPackageName()).thenReturn("valid.package.name")
-        `when`(mockHostRepo.getVersion()).thenReturn("valid.version")
         `when`(mockHostRepo.getSubscriptionKey()).thenReturn("")
         val worker = ConfigWorker(
             ctx, workParam, mockHostRepo, ConfigResponseRepository.instance(),