Release 17.8.0 (#1390)

Author: jyaganeh

CHANGELOG.md

@@ -2,6 +2,12 @@
 
 [Migration Guides](https://github.com/urbanairship/android-library/tree/main/documentation/migration)
 
+## Version 17.8.0, April 11, 2024
+Minor release that fixes potential crashes when evaluating experiments before a Channel ID has been created. Apps that make use of experiments or holdout groups should update to this version or later.
+
+### Changes
+- Avoid NPE in `ExperimentManager` when evaluating experiments before a Channel ID has been created.
+
 ## Version 17.7.4, April 5, 2024
 Patch release that fixes a potential crash on Android 13 (API 33) channel ID creation delay after enabling a feature when none was enabled. The SDK will new create the channel ID without having to relaunch the app. Apps that have no features enabled at launch should update to this version or later.
 

README.md

@@ -18,7 +18,7 @@ Airship SDK for Android.
 
 ```
    dependencies {
-     def airshipVersion = "17.7.4"
+     def airshipVersion = "17.8.0"
 
      // FCM push provider
      implementation "com.urbanairship.android:urbanairship-fcm:$airshipVersion"

build.gradle

@@ -1,7 +1,7 @@
 buildscript {
     ext {
         // Airship Version - major.minor.patch
-        airshipVersion = '17.7.4'
+        airshipVersion = '17.8.0'
 
         // Airship Version Qualifier beta, release, etc...
         // airshipVersionQualifier = "beta"

urbanairship-automation/src/main/java/com/urbanairship/automation/InAppAutomation.java

@@ -51,6 +51,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.UUID;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.Executor;
@@ -64,6 +65,8 @@
 import androidx.annotation.VisibleForTesting;
 import androidx.annotation.WorkerThread;
 
+import static com.urbanairship.util.Checks.checkNotNull;
+
 /**
  * In-app automation.
  */
@@ -648,6 +651,10 @@ private void onPrepareSchedule(final @NonNull Schedule<? extends ScheduleData> s
         PendingResult<ExperimentResult> experimentResults = new PendingResult<>();
         RetryingExecutor.Operation evaluateExperiments = () -> {
             try {
+                // Ensure we have a channel ID available. Otherwise, throw so that we'll retry.
+                Objects.requireNonNull(infoProvider.getChannelId(),
+                        "Channel ID must be available to evaluate experiments.");
+
                 ExperimentResult result = evaluateExperiments(schedule);
                 experimentResults.setResult(result);
                 return RetryingExecutor.finishedResult();

urbanairship-automation/src/test/java/com/urbanairship/automation/InAppAutomationTest.java

@@ -10,6 +10,7 @@
 import com.urbanairship.PrivacyManager;
 import com.urbanairship.ShadowAirshipExecutorsLegacy;
 import com.urbanairship.TestApplication;
+import com.urbanairship.TestDeviceInfoProvider;
 import com.urbanairship.TestRequestSession;
 import com.urbanairship.UAirship;
 import com.urbanairship.analytics.CustomEvent;
@@ -79,6 +80,7 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.ArgumentMatchers.nullable;
 import static org.mockito.Mockito.atLeastOnce;
@@ -139,6 +141,9 @@ public class InAppAutomationTest {
     @Before
     public void setup() {
         when(mockRuntimeConfig.getConfigOptions()).thenAnswer((Answer<AirshipConfigOptions>) invocation -> config);
+
+        when(mockInfoProvider.getChannelId()).thenReturn("channel-id");
+
         mockChannel = mock(AirshipChannel.class);
         mockIamManager = mock(InAppMessageManager.class);
         mockObserver = mock(InAppRemoteDataObserver.class);
@@ -324,6 +329,38 @@ public void testPrepareSchedule() {
         verify(callback).onFinish(AutomationDriver.PREPARE_RESULT_CONTINUE);
     }
 
+
+    @Test
+    public void testPrepareScheduleNoChannelId() {
+        when(mockInfoProvider.getChannelId()).thenReturn(null);
+
+        InAppMessage message = InAppMessage.newBuilder()
+                                           .setDisplayContent(new CustomDisplayContent(JsonValue.NULL))
+                                           .build();
+
+        Schedule<InAppMessage> schedule = Schedule.newBuilder(message)
+                                                  .addTrigger(Triggers.newAppInitTriggerBuilder().setGoal(1).build())
+                                                  .setBypassHoldoutGroups(true)
+                                                  .build();
+
+        when(mockObserver.requiresRefresh(eq(schedule))).thenReturn(false);
+        when(mockObserver.bestEffortRefresh(eq(schedule))).thenReturn(true);
+
+        AutomationDriver.PrepareScheduleCallback callback = mock(AutomationDriver.PrepareScheduleCallback.class);
+        driver.onPrepareSchedule(schedule, null, callback);
+        verify(mockMessageScheduleDelegate, never()).onPrepareSchedule(eq(schedule), eq(schedule.getData()), any(), any());
+        verify(callback, never()).onFinish(anyInt());
+
+        // Sanity check: we should be able to prepare a schedule once we do have a channel ID.
+        when(mockInfoProvider.getChannelId()).thenReturn("channel-id");
+
+        driver.onPrepareSchedule(schedule, null, callback);
+        ArgumentCaptor<AutomationDriver.PrepareScheduleCallback> argumentCaptor = ArgumentCaptor.forClass(AutomationDriver.PrepareScheduleCallback.class);
+        verify(mockMessageScheduleDelegate).onPrepareSchedule(eq(schedule), eq(schedule.getData()), any(), argumentCaptor.capture());
+        argumentCaptor.getValue().onFinish(AutomationDriver.PREPARE_RESULT_CONTINUE);
+        verify(callback).onFinish(AutomationDriver.PREPARE_RESULT_CONTINUE);
+    }
+
     @Test
     public void testPrepareScheduleRequiresRefresh() {
         InAppMessage message = InAppMessage.newBuilder()

urbanairship-core/src/main/java/com/urbanairship/experiment/Experiment.kt

@@ -12,6 +12,7 @@ import com.urbanairship.json.optionalField
 import com.urbanairship.json.optionalFieldConverted
 import com.urbanairship.json.requireField
 import com.urbanairship.util.DateUtils
+import java.text.ParseException
 
 internal enum class ExperimentType(val jsonValue: String) {
     HOLDOUT_GROUP("holdout");
@@ -179,6 +180,9 @@ internal data class Experiment(
             } catch (ex: JsonException) {
                 UALog.e { "failed to parse Experiment from json $json" }
                 return null
+            } catch (ex: ParseException) {
+                UALog.e { "failed to parse Experiment from json $json" }
+                return null
             }
         }
     }

urbanairship-core/src/main/java/com/urbanairship/experiment/ExperimentManager.kt

@@ -16,7 +16,6 @@ import com.urbanairship.json.JsonException
 import com.urbanairship.json.JsonMap
 import com.urbanairship.remotedata.RemoteData
 import com.urbanairship.util.Clock
-import kotlin.jvm.Throws
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.launch
@@ -47,6 +46,7 @@ public class ExperimentManager internal constructor(
     /**
      * Returns an optional Experiment with the given [id].
      *
+     * @param messageInfo The message info.
      * @param id The ID of the Experiment.
      */
     internal suspend fun getExperimentWithId(messageInfo: MessageInfo, id: String): Experiment? {
@@ -56,11 +56,12 @@ public class ExperimentManager internal constructor(
 
     /**
      * Checks if the channel and/or contact is part of a global holdout or not.
+     * @hide
+     *
+     * @param messageInfo The message info.
      * @param contactId The contact ID. If not provided, the stable contact ID will be used.
      * @return The experiments result. If no experiment matches, null is returned.
      */
-    /** @hide */
-    @Throws(NullPointerException::class)
     @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
     public suspend fun evaluateExperiments(messageInfo: MessageInfo, contactId: String? = null): ExperimentResult? {
 
@@ -70,7 +71,10 @@ public class ExperimentManager internal constructor(
         }
 
         val channelId = infoProvider.channelId
-            ?: throw NullPointerException("Channel ID missing, unable to evaluate hold out groups.")
+        if (channelId == null) {
+            UALog.d("Channel ID not available, unable to evaluate hold out groups.")
+            return null
+        }
 
         val evaluationContactId = contactId ?: infoProvider.getStableContactId()
 

urbanairship-core/src/test/java/com/urbanairship/experiment/ExperimentManagerTest.kt

@@ -1,3 +1,5 @@
+@file:OptIn(ExperimentalCoroutinesApi::class)
+
 package com.urbanairship.experiment
 
 import android.content.Context
@@ -20,6 +22,7 @@ import io.mockk.every
 import io.mockk.mockk
 import java.util.Date
 import junit.framework.Assert.assertTrue
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.TestResult
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
@@ -184,6 +187,28 @@ public class ExperimentManagerTest {
         assert(result.allEvaluatedExperimentsMetadata.contains(extractReportingMetadata(experimentJson)))
     }
 
+    @Test
+    public fun testHoldoutGroupEvaluationNoChannelId(): TestResult = runTest {
+        channelId = null
+        contactId = "some-contact-id"
+
+        val experimentJson = generateExperimentsPayload(
+            id = "fake-id",
+            hashIdentifier = "channel")
+            .build()
+
+        val data = RemoteDataPayload(
+            type = PAYLOAD_TYPE,
+            timestamp = 1L,
+            data = jsonMapOf(PAYLOAD_TYPE to jsonListOf(experimentJson))
+        )
+
+        coEvery { remoteData.payloads(PAYLOAD_TYPE) } returns listOf(data)
+
+        val result = subject.evaluateExperiments(messageInfo)
+        assertNull(result)
+    }
+
     @Test
     public fun testHoldoutGroupEvaluationRespectHashBuckets(): TestResult = runTest {
         channelId = "channel-id"

urbanairship-feature-flag/src/main/java/com/urbanairship/featureflag/FeatureFlag.kt

@@ -2,6 +2,7 @@
 
 package com.urbanairship.featureflag
 
+import com.urbanairship.json.JsonException
 import com.urbanairship.json.JsonMap
 import com.urbanairship.json.JsonSerializable
 import com.urbanairship.json.JsonValue
@@ -123,15 +124,15 @@ class FeatureFlag private constructor(
          * Parses a `JsonValue` as a `FeatureFlag`.
          * @throws `JsonException`
          */
-        @Throws
         @JvmStatic
+        @Throws(JsonException::class)
         public fun fromJson(json: JsonValue): FeatureFlag {
             return FeatureFlag(
                 name = json.requireMap().requireField(KEY_NAME),
                 isEligible = json.requireMap().requireField(KEY_IS_ELIGIBLE),
                 exists = json.requireMap().requireField(KEY_EXISTS),
                 reportingInfo = json.requireMap().get(KEY_REPORTING_INFO)?.let {
-                    ReportingInfo.fromJson(it)
+                        ReportingInfo.fromJson(it)
                 },
                 variables = json.requireMap().optionalField(KEY_VARIABLES),
             )
@@ -149,7 +150,7 @@ class FeatureFlag private constructor(
             private const val KEY_CHANNEL_ID = "channel_id"
             private const val KEY_CONTACT_ID = "contact_id"
 
-            @Throws
+            @Throws(JsonException::class)
             fun fromJson(json: JsonValue): ReportingInfo {
                 return ReportingInfo(
                     reportingMetadata = json.requireMap().requireField(KEY_REPORTING_METADATA),

urbanairship-feature-flag/src/main/java/com/urbanairship/featureflag/FeatureFlagInfo.kt

@@ -18,6 +18,7 @@ import com.urbanairship.json.jsonMapOf
 import com.urbanairship.json.optionalField
 import com.urbanairship.json.requireField
 import com.urbanairship.util.DateUtils
+import java.text.ParseException
 import kotlin.jvm.Throws
 
 internal enum class FeatureFlagVariablesType(val jsonValue: String) {
@@ -211,6 +212,9 @@ internal class FeatureFlagInfo(
             } catch (ex: JsonException) {
                 UALog.e { "failed to parse FeatureFlagInfo from json $json" }
                 return null
+            } catch (ex: ParseException) {
+                UALog.e { "failed to parse FeatureFlagInfo from json $json" }
+                return null
             }
         }
 
@@ -233,6 +237,7 @@ internal class DeferredPayload(
     companion object {
         private const val KEY_URL = "url"
 
+        @Throws(JsonException::class)
         fun fromJson(json: JsonMap): DeferredPayload {
 
             return DeferredPayload(
@@ -256,11 +261,12 @@ internal class StaticPayload(
 ) : FeatureFlagPayload {
 
     companion object {
+
+        @Throws(JsonException::class)
         fun fromJson(json: JsonMap): StaticPayload {
             if (json.isEmpty) {
                 val variables = FeatureFlagVariables(
-                    type = FeatureFlagVariablesType.FIXED,
-                    variants = listOf(VariablesVariant.empty)
+                    type = FeatureFlagVariablesType.FIXED, variants = listOf(VariablesVariant.empty)
                 )
                 return StaticPayload(variables)
             }

urbanairship-feature-flag/src/main/java/com/urbanairship/featureflag/FeatureFlagInteractionEvent.kt

@@ -3,14 +3,15 @@
 package com.urbanairship.featureflag
 
 import com.urbanairship.analytics.Event
+import com.urbanairship.json.JsonException
 import com.urbanairship.json.JsonMap
 import com.urbanairship.json.jsonMapOf
 
 internal class FeatureFlagInteractionEvent private constructor(
     val data: JsonMap
 ) : Event() {
 
-    @Throws
+    @Throws(JsonException::class)
     internal constructor(flag: FeatureFlag) : this(
         jsonMapOf(
             "flag_name" to flag.name,

urbanairship-feature-flag/src/main/java/com/urbanairship/featureflag/FeatureFlagManager.kt

@@ -72,7 +72,6 @@ public class FeatureFlagManager
      * @param name The flag name
      * @return an instance of `PendingResult<FeatureFlag>`.
      */
-    @Throws(FeatureFlagException::class)
     fun flagAsPendingResult(name: String): PendingResult<FeatureFlag> {
         val result = PendingResult<FeatureFlag>()
         pendingResultScope.launch {
@@ -92,7 +91,7 @@ public class FeatureFlagManager
 
     private suspend fun flag(name: String, allowRefresh: Boolean): Result<FeatureFlag> {
         if (!isComponentEnabled) {
-            throw FeatureFlagException.FailedToFetch()
+            return Result.failure(FeatureFlagException.FailedToFetch())
         }
 
         val remoteDataInfo = fetchFlagRemoteInfo(name)
@@ -123,6 +122,7 @@ public class FeatureFlagManager
                     Result.failure(FeatureFlagException.FailedToFetch())
                 }
             }
+
             else -> Result.failure(FeatureFlagException.FailedToFetch())
         }
     }