Check cache for choice ID and add more asserts in tests.

Author: vic-flair

firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/Personalization.java

@@ -25,29 +25,31 @@
 public class Personalization {
   public static final String ANALYTICS_ORIGIN_PERSONALIZATION = "fp";
 
+  // Constants with LOG_KEY suffix are how the corresponding ones without it are identified on
+  // Google Analytics.
   public static final String ANALYTICS_PULL_EVENT = "personalization_assignment";
   public static final String ARM_KEY = "arm_key";
   public static final String ARM_VALUE = "arm_value";
   public static final String PERSONALIZATION_ID = "personalizationId";
-  public static final String PERSONALIZATION_ID_KEY = "personalization_id";
+  public static final String PERSONALIZATION_ID_LOG_KEY = "personalization_id";
   public static final String ARM_INDEX = "armIndex";
-  public static final String ARM_INDEX_KEY = "arm_index";
+  public static final String ARM_INDEX_LOG_KEY = "arm_index";
   public static final String GROUP = "group";
 
   public static final String ANALYTICS_PULL_EVENT_INTERNAL = "_fpc";
   public static final String CHOICE_ID = "choiceId";
-  public static final String CHOICE_ID_KEY = "_fpid";
+  public static final String CHOICE_ID_LOG_KEY = "_fpid";
 
   /** The app's Firebase Analytics client. */
   private final AnalyticsConnector analyticsConnector;
 
-  /** A map of Remote Config parameter key to Personalization ID. */
-  private final Map<String, String> armsCache;
+  /** A map of Remote Config parameter key to choice ID. */
+  private final Map<String, String> armsCache =
+      Collections.synchronizedMap(new HashMap<String, String>());
 
   /** Creates an instance of {@code Personalization}. */
   public Personalization(@NonNull AnalyticsConnector analyticsConnector) {
     this.analyticsConnector = analyticsConnector;
-    armsCache = Collections.synchronizedMap(new HashMap<String, String>());
   }
 
   /**
@@ -74,29 +76,29 @@ public void logArmActive(@NonNull String key, @NonNull ConfigContainer configCon
       return;
     }
 
-    String personalizationId = metadata.optString(PERSONALIZATION_ID);
-    if (personalizationId.isEmpty()) {
+    String choiceId = metadata.optString(CHOICE_ID);
+    if (choiceId.isEmpty()) {
       return;
     }
 
     synchronized (armsCache) {
-      if (armsCache.get(key) == personalizationId) {
+      if (choiceId.equals(armsCache.get(key))) {
         return;
       }
-      armsCache.put(key, personalizationId);
+      armsCache.put(key, choiceId);
     }
 
-    Bundle params = new Bundle();
-    params.putString(ARM_KEY, key);
-    params.putString(ARM_VALUE, values.optString(key));
-    params.putString(PERSONALIZATION_ID_KEY, personalizationId);
-    params.putInt(ARM_INDEX_KEY, metadata.optInt(ARM_INDEX, -1));
-    params.putString(GROUP, metadata.optString(GROUP));
-    analyticsConnector.logEvent(ANALYTICS_ORIGIN_PERSONALIZATION, ANALYTICS_PULL_EVENT, params);
+    Bundle logParams = new Bundle();
+    logParams.putString(ARM_KEY, key);
+    logParams.putString(ARM_VALUE, values.optString(key));
+    logParams.putString(PERSONALIZATION_ID_LOG_KEY, metadata.optString(PERSONALIZATION_ID));
+    logParams.putInt(ARM_INDEX_LOG_KEY, metadata.optInt(ARM_INDEX, -1));
+    logParams.putString(GROUP, metadata.optString(GROUP));
+    analyticsConnector.logEvent(ANALYTICS_ORIGIN_PERSONALIZATION, ANALYTICS_PULL_EVENT, logParams);
 
-    Bundle paramsInternal = new Bundle();
-    paramsInternal.putString(CHOICE_ID_KEY, metadata.optString(CHOICE_ID));
+    Bundle internalLogParams = new Bundle();
+    internalLogParams.putString(CHOICE_ID_LOG_KEY, choiceId);
     analyticsConnector.logEvent(
-        ANALYTICS_ORIGIN_PERSONALIZATION, ANALYTICS_PULL_EVENT_INTERNAL, paramsInternal);
+        ANALYTICS_ORIGIN_PERSONALIZATION, ANALYTICS_PULL_EVENT_INTERNAL, internalLogParams);
   }
 }

firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/PersonalizationTest.java

@@ -18,9 +18,14 @@
 import static com.google.firebase.remoteconfig.internal.Personalization.ANALYTICS_ORIGIN_PERSONALIZATION;
 import static com.google.firebase.remoteconfig.internal.Personalization.ANALYTICS_PULL_EVENT;
 import static com.google.firebase.remoteconfig.internal.Personalization.ANALYTICS_PULL_EVENT_INTERNAL;
-import static com.google.firebase.remoteconfig.internal.Personalization.CHOICE_ID_KEY;
+import static com.google.firebase.remoteconfig.internal.Personalization.ARM_INDEX_LOG_KEY;
+import static com.google.firebase.remoteconfig.internal.Personalization.ARM_KEY;
+import static com.google.firebase.remoteconfig.internal.Personalization.ARM_VALUE;
+import static com.google.firebase.remoteconfig.internal.Personalization.CHOICE_ID_LOG_KEY;
+import static com.google.firebase.remoteconfig.internal.Personalization.GROUP;
+import static com.google.firebase.remoteconfig.internal.Personalization.PERSONALIZATION_ID_LOG_KEY;
+import static org.mockito.AdditionalMatchers.or;
 import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.times;
@@ -82,7 +87,10 @@ public void setUp() {
 
     doAnswer(invocation -> FAKE_LOGS.add(invocation.getArgument(2)))
         .when(mockAnalyticsConnector)
-        .logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), anyString(), any(Bundle.class));
+        .logEvent(
+            eq(ANALYTICS_ORIGIN_PERSONALIZATION),
+            or(eq(ANALYTICS_PULL_EVENT), eq(ANALYTICS_PULL_EVENT_INTERNAL)),
+            any(Bundle.class));
 
     personalization = new Personalization(mockAnalyticsConnector);
 
@@ -94,12 +102,15 @@ public void logArmActive_nonPersonalizationKey_notLogged() {
     personalization.logArmActive("key3", CONFIG_CONTAINER);
 
     verify(mockAnalyticsConnector, times(0))
-        .logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), anyString(), any(Bundle.class));
+        .logEvent(
+            eq(ANALYTICS_ORIGIN_PERSONALIZATION),
+            or(eq(ANALYTICS_PULL_EVENT), eq(ANALYTICS_PULL_EVENT_INTERNAL)),
+            any(Bundle.class));
     assertThat(FAKE_LOGS).isEmpty();
   }
 
   @Test
-  public void logArmActive_singlePersonalizationKey_loggedOnce() {
+  public void logArmActive_singlePersonalizationKey_loggedInternallyAndExternally() {
     personalization.logArmActive("key1", CONFIG_CONTAINER);
 
     verify(mockAnalyticsConnector, times(1))
@@ -112,15 +123,24 @@ public void logArmActive_singlePersonalizationKey_loggedOnce() {
             any(Bundle.class));
     assertThat(FAKE_LOGS).hasSize(2);
 
-    Bundle params = new Bundle();
-    params.putString(CHOICE_ID_KEY, "id1");
-    assertThat(FAKE_LOGS).comparingElementsUsing(TO_STRING).contains(params.toString());
+    Bundle logParams = new Bundle();
+    logParams.putString(ARM_KEY, "key1");
+    logParams.putString(ARM_VALUE, "value1");
+    logParams.putString(PERSONALIZATION_ID_LOG_KEY, "p13n1");
+    logParams.putInt(ARM_INDEX_LOG_KEY, 0);
+    logParams.putString(GROUP, "BASELINE");
+    Bundle internalLogParams = new Bundle();
+    internalLogParams.putString(CHOICE_ID_LOG_KEY, "id1");
+    assertThat(FAKE_LOGS)
+        .comparingElementsUsing(TO_STRING)
+        .containsExactly(logParams.toString(), internalLogParams.toString());
   }
 
   @Test
   public void logArmActive_multiplePersonalizationKeys_loggedMultiple() {
     personalization.logArmActive("key1", CONFIG_CONTAINER);
     personalization.logArmActive("key2", CONFIG_CONTAINER);
+    personalization.logArmActive("key1", CONFIG_CONTAINER);
 
     verify(mockAnalyticsConnector, times(2))
         .logEvent(
@@ -132,13 +152,30 @@ public void logArmActive_multiplePersonalizationKeys_loggedMultiple() {
             any(Bundle.class));
     assertThat(FAKE_LOGS).hasSize(4);
 
-    Bundle params1 = new Bundle();
-    params1.putString(CHOICE_ID_KEY, "id1");
-    Bundle params2 = new Bundle();
-    params2.putString(CHOICE_ID_KEY, "id2");
+    Bundle logParams1 = new Bundle();
+    logParams1.putString(ARM_KEY, "key1");
+    logParams1.putString(ARM_VALUE, "value1");
+    logParams1.putString(PERSONALIZATION_ID_LOG_KEY, "p13n1");
+    logParams1.putInt(ARM_INDEX_LOG_KEY, 0);
+    logParams1.putString(GROUP, "BASELINE");
+    Bundle logParams2 = new Bundle();
+    logParams2.putString(ARM_KEY, "key2");
+    logParams2.putString(ARM_VALUE, "value2");
+    logParams2.putString(PERSONALIZATION_ID_LOG_KEY, "p13n2");
+    logParams2.putInt(ARM_INDEX_LOG_KEY, 1);
+    logParams2.putString(GROUP, "P13N");
+    assertThat(FAKE_LOGS)
+        .comparingElementsUsing(TO_STRING)
+        .containsAtLeast(logParams1.toString(), logParams2.toString())
+        .inOrder();
+
+    Bundle internalLogParams1 = new Bundle();
+    internalLogParams1.putString(CHOICE_ID_LOG_KEY, "id1");
+    Bundle internalLogParams2 = new Bundle();
+    internalLogParams2.putString(CHOICE_ID_LOG_KEY, "id2");
     assertThat(FAKE_LOGS)
         .comparingElementsUsing(TO_STRING)
-        .containsAtLeast(params1.toString(), params2.toString())
+        .containsAtLeast(internalLogParams1.toString(), internalLogParams2.toString())
         .inOrder();
   }
 }