Merge branch 'mrschmidt/bundle/reader' into mrschmidt/bundle/localstore

Author: schmidt-sebastian

buildSrc/build.gradle

@@ -39,7 +39,7 @@ dependencies {
     runtime 'io.opencensus:opencensus-impl:0.18.0'
     implementation group: 'com.google.code.gson', name: 'gson', version: '2.8.6'
     implementation 'org.jetbrains.dokka:dokka-android-gradle-plugin:0.9.17-g013-9b8280a'
-    implementation 'com.android.tools.build:gradle:3.4.2'
+    implementation 'com.android.tools.build:gradle:3.4.3'
 
     testImplementation 'junit:junit:4.13-rc-1'
     testImplementation group: 'com.google.code.gson', name: 'gson', version: '2.8.6'

firebase-components/src/main/java/com/google/firebase/components/Dependency.java

@@ -47,30 +47,82 @@ private Dependency(Class<?> anInterface, @Type int type, @Injection int injectio
     this.injection = injection;
   }
 
+  /**
+   * Declares an optional dependency.
+   *
+   * <p>Optional dependencies can be missing at runtime(being {@code null}) and dependents must be
+   * ready to handle that.
+   *
+   * @deprecated Optional dependencies are not safe to use in the context of Play's dynamic feature
+   *     delivery. Use {@link #optionalProvider(Class) optional provider} instead.
+   */
+  @Deprecated
   public static Dependency optional(Class<?> anInterface) {
     return new Dependency(anInterface, Type.OPTIONAL, Injection.DIRECT);
   }
 
+  /**
+   * Declares a deferred dependency.
+   *
+   * <p>Such dependencies are optional and may not be present by default. But they can become
+   * available if a dynamic module that contains them is installed.
+   */
   public static Dependency deferred(Class<?> anInterface) {
     return new Dependency(anInterface, Type.OPTIONAL, Injection.DEFERRED);
   }
 
+  /**
+   * Declares a required dependency.
+   *
+   * <p>Such dependencies must be present in order for the dependent component to function. Any
+   * component with a required dependency should also declare a Maven dependency on an SDK that
+   * provides it. Failing to do so will result in a {@link MissingDependencyException} to be thrown
+   * at runtime.
+   */
   public static Dependency required(Class<?> anInterface) {
     return new Dependency(anInterface, Type.REQUIRED, Injection.DIRECT);
   }
 
+  /**
+   * Declares a Set multi-binding dependency.
+   *
+   * <p>Such dependencies provide access to a {@code Set<Foo>} to dependent components. Note that
+   * the set is only filled with components that explicitly declare the intent to be a "set"
+   * dependency via {@link Component#intoSet(Object, Class)}.
+   */
   public static Dependency setOf(Class<?> anInterface) {
     return new Dependency(anInterface, Type.SET, Injection.DIRECT);
   }
 
+  /**
+   * Declares an optional dependency.
+   *
+   * <p>Optional dependencies can be missing at runtime(being {@code null}) and dependents must be
+   * ready to handle that.
+   */
   public static Dependency optionalProvider(Class<?> anInterface) {
     return new Dependency(anInterface, Type.OPTIONAL, Injection.PROVIDER);
   }
 
+  /**
+   * Declares a required dependency.
+   *
+   * <p>Such dependencies must be present in order for the dependent component to function. Any
+   * component with a required dependency should also declare a Maven dependency on an SDK that
+   * provides it. Failing to do so will result in a {@link MissingDependencyException} to be thrown
+   * at runtime.
+   */
   public static Dependency requiredProvider(Class<?> anInterface) {
     return new Dependency(anInterface, Type.REQUIRED, Injection.PROVIDER);
   }
 
+  /**
+   * Declares a Set multi-binding dependency.
+   *
+   * <p>Such dependencies provide access to a {@code Set<Foo>} to dependent components. Note that
+   * the set is only filled with components that explicitly declare the intent to be a "set"
+   * dependency via {@link Component#intoSet(Object, Class)}.
+   */
   public static Dependency setOfProvider(Class<?> anInterface) {
     return new Dependency(anInterface, Type.SET, Injection.PROVIDER);
   }

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

@@ -17,18 +17,36 @@
 import android.os.Bundle;
 import androidx.annotation.NonNull;
 import com.google.firebase.analytics.connector.AnalyticsConnector;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 import org.json.JSONObject;
 
 public class Personalization {
   public static final String ANALYTICS_ORIGIN_PERSONALIZATION = "fp";
-  public static final String ANALYTICS_PULL_EVENT = "_fpc";
-  public static final String ARM_KEY = "_fpid";
-  public static final String ARM_VALUE = "_fpct";
-  static final String PERSONALIZATION_ID = "personalizationId";
+
+  // The PARAM suffix identifies log keys sent to Google Analytics.
+  public static final String EXTERNAL_EVENT = "personalization_assignment";
+  public static final String EXTERNAL_RC_PARAMETER_PARAM = "arm_key";
+  public static final String EXTERNAL_ARM_VALUE_PARAM = "arm_value";
+  public static final String PERSONALIZATION_ID = "personalizationId";
+  public static final String EXTERNAL_PERSONALIZATION_ID_PARAM = "personalization_id";
+  public static final String ARM_INDEX = "armIndex";
+  public static final String EXTERNAL_ARM_INDEX_PARAM = "arm_index";
+  public static final String GROUP = "group";
+  public static final String EXTERNAL_GROUP_PARAM = "group";
+
+  public static final String INTERNAL_EVENT = "_fpc";
+  public static final String CHOICE_ID = "choiceId";
+  public static final String INTERNAL_CHOICE_ID_PARAM = "_fpid";
 
   /** The app's Firebase Analytics client. */
   private final AnalyticsConnector analyticsConnector;
 
+  /** Remote Config parameter key and choice ID pairs that have already been logged to Analytics. */
+  private final Map<String, String> loggedChoiceIds =
+      Collections.synchronizedMap(new HashMap<String, String>());
+
   /** Creates an instance of {@code Personalization}. */
   public Personalization(@NonNull AnalyticsConnector analyticsConnector) {
     this.analyticsConnector = analyticsConnector;
@@ -38,11 +56,11 @@ public Personalization(@NonNull AnalyticsConnector analyticsConnector) {
    * Called when a Personalization parameter value (an arm) is retrieved, and uses Google Analytics
    * for Firebase to log metadata if it's a Personalization parameter.
    *
-   * @param key Remote Config parameter
+   * @param rcParameter Remote Config parameter
    * @param configContainer {@link ConfigContainer} containing Personalization metadata for {@code
    *     key}
    */
-  public void logArmActive(@NonNull String key, @NonNull ConfigContainer configContainer) {
+  public void logArmActive(@NonNull String rcParameter, @NonNull ConfigContainer configContainer) {
     JSONObject ids = configContainer.getPersonalizationMetadata();
     if (ids.length() < 1) {
       return;
@@ -53,14 +71,34 @@ public void logArmActive(@NonNull String key, @NonNull ConfigContainer configCon
       return;
     }
 
-    JSONObject metadata = ids.optJSONObject(key);
+    JSONObject metadata = ids.optJSONObject(rcParameter);
     if (metadata == null) {
       return;
     }
 
-    Bundle params = new Bundle();
-    params.putString(ARM_KEY, metadata.optString(PERSONALIZATION_ID));
-    params.putString(ARM_VALUE, values.optString(key));
-    analyticsConnector.logEvent(ANALYTICS_ORIGIN_PERSONALIZATION, ANALYTICS_PULL_EVENT, params);
+    String choiceId = metadata.optString(CHOICE_ID);
+    if (choiceId.isEmpty()) {
+      return;
+    }
+
+    synchronized (loggedChoiceIds) {
+      if (choiceId.equals(loggedChoiceIds.get(rcParameter))) {
+        return;
+      }
+      loggedChoiceIds.put(rcParameter, choiceId);
+    }
+
+    Bundle logParams = new Bundle();
+    logParams.putString(EXTERNAL_RC_PARAMETER_PARAM, rcParameter);
+    logParams.putString(EXTERNAL_ARM_VALUE_PARAM, values.optString(rcParameter));
+    logParams.putString(EXTERNAL_PERSONALIZATION_ID_PARAM, metadata.optString(PERSONALIZATION_ID));
+    logParams.putInt(EXTERNAL_ARM_INDEX_PARAM, metadata.optInt(ARM_INDEX, -1));
+    logParams.putString(EXTERNAL_GROUP_PARAM, metadata.optString(GROUP));
+    analyticsConnector.logEvent(ANALYTICS_ORIGIN_PERSONALIZATION, EXTERNAL_EVENT, logParams);
+
+    Bundle internalLogParams = new Bundle();
+    internalLogParams.putString(INTERNAL_CHOICE_ID_PARAM, choiceId);
+    analyticsConnector.logEvent(
+        ANALYTICS_ORIGIN_PERSONALIZATION, INTERNAL_EVENT, internalLogParams);
   }
 }

firebase-config/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java

@@ -958,7 +958,7 @@ public void personalization_hasMetadata_successful() throws Exception {
         .when(mockAnalyticsConnector)
         .logEvent(
             eq(Personalization.ANALYTICS_ORIGIN_PERSONALIZATION),
-            eq(Personalization.ANALYTICS_PULL_EVENT),
+            eq(Personalization.EXTERNAL_EVENT),
             any(Bundle.class));
 
     ConfigContainer configContainer =
@@ -986,18 +986,18 @@ public void personalization_hasMetadata_successful() throws Exception {
               verify(mockAnalyticsConnector, times(2))
                   .logEvent(
                       eq(Personalization.ANALYTICS_ORIGIN_PERSONALIZATION),
-                      eq(Personalization.ANALYTICS_PULL_EVENT),
+                      eq(Personalization.EXTERNAL_EVENT),
                       any(Bundle.class));
               assertThat(fakeLogs).hasSize(2);
 
               Bundle params1 = new Bundle();
-              params1.putString(Personalization.ARM_KEY, "id1");
-              params1.putString(Personalization.ARM_VALUE, "value1");
+              params1.putString(Personalization.EXTERNAL_RC_PARAMETER_PARAM, "id1");
+              params1.putString(Personalization.EXTERNAL_ARM_VALUE_PARAM, "value1");
               assertThat(fakeLogs.get(0).toString()).isEqualTo(params1.toString());
 
               Bundle params2 = new Bundle();
-              params2.putString(Personalization.ARM_KEY, "id2");
-              params2.putString(Personalization.ARM_VALUE, "value2");
+              params2.putString(Personalization.EXTERNAL_RC_PARAMETER_PARAM, "id2");
+              params2.putString(Personalization.EXTERNAL_ARM_VALUE_PARAM, "value2");
               assertThat(fakeLogs.get(1).toString()).isEqualTo(params2.toString());
             });
   }

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

@@ -90,7 +90,9 @@ public void read_validContainerWithPersonalization_returnsContainer() throws Exc
     ConfigContainer configWithPersonalization =
         ConfigContainer.newBuilder(configContainer)
             .withPersonalizationMetadata(
-                new JSONObject(ImmutableMap.of(Personalization.ARM_KEY, "arm_value")))
+                new JSONObject(
+                    "{long_param: {personalizationId: 'id1'}, "
+                        + "string_param: {personalizationId: 'id2'}}"))
             .build();
     storageClient.write(configWithPersonalization);
     Preconditions.checkArgument(getFileAsString().equals(configWithPersonalization.toString()));

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

@@ -16,17 +16,25 @@
 
 import static com.google.common.truth.Truth.assertThat;
 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.ARM_KEY;
-import static com.google.firebase.remoteconfig.internal.Personalization.ARM_VALUE;
+import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_ARM_INDEX_PARAM;
+import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_ARM_VALUE_PARAM;
+import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_EVENT;
+import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_GROUP_PARAM;
+import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_PERSONALIZATION_ID_PARAM;
+import static com.google.firebase.remoteconfig.internal.Personalization.EXTERNAL_RC_PARAMETER_PARAM;
+import static com.google.firebase.remoteconfig.internal.Personalization.INTERNAL_CHOICE_ID_PARAM;
+import static com.google.firebase.remoteconfig.internal.Personalization.INTERNAL_EVENT;
+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;
 import static org.mockito.Mockito.verify;
 import static org.mockito.MockitoAnnotations.initMocks;
 
 import android.os.Bundle;
+import com.google.common.truth.Correspondence;
 import com.google.firebase.analytics.connector.AnalyticsConnector;
 import java.util.ArrayList;
 import java.util.Date;
@@ -45,6 +53,10 @@
 @Config(manifest = Config.NONE)
 public class PersonalizationTest {
   private static final ConfigContainer CONFIG_CONTAINER;
+  private static final Bundle LOG_PARAMS_1 = new Bundle();
+  private static final Bundle LOG_PARAMS_2 = new Bundle();
+  private static final Bundle INTERNAL_LOG_PARAMS_1 = new Bundle();
+  private static final Bundle INTERNAL_LOG_PARAMS_2 = new Bundle();
 
   static {
     try {
@@ -55,15 +67,37 @@ public class PersonalizationTest {
               .withFetchTime(new Date(1))
               .withPersonalizationMetadata(
                   new JSONObject(
-                      "{key1: {personalizationId: 'id1'}, key2: {personalizationId: 'id2'}}"))
+                      "{key1: {personalizationId: 'p13n1', armIndex: 0,"
+                          + " choiceId: 'id1', group: 'BASELINE'},"
+                          + " key2: {personalizationId: 'p13n2', armIndex: 1,"
+                          + " choiceId: 'id2', group: 'P13N'}}"))
               .build();
     } catch (JSONException e) {
       throw new RuntimeException(e);
     }
+
+    LOG_PARAMS_1.putString(EXTERNAL_RC_PARAMETER_PARAM, "key1");
+    LOG_PARAMS_1.putString(EXTERNAL_ARM_VALUE_PARAM, "value1");
+    LOG_PARAMS_1.putString(EXTERNAL_PERSONALIZATION_ID_PARAM, "p13n1");
+    LOG_PARAMS_1.putInt(EXTERNAL_ARM_INDEX_PARAM, 0);
+    LOG_PARAMS_1.putString(EXTERNAL_GROUP_PARAM, "BASELINE");
+
+    LOG_PARAMS_2.putString(EXTERNAL_RC_PARAMETER_PARAM, "key2");
+    LOG_PARAMS_2.putString(EXTERNAL_ARM_VALUE_PARAM, "value2");
+    LOG_PARAMS_2.putString(EXTERNAL_PERSONALIZATION_ID_PARAM, "p13n2");
+    LOG_PARAMS_2.putInt(EXTERNAL_ARM_INDEX_PARAM, 1);
+    LOG_PARAMS_2.putString(EXTERNAL_GROUP_PARAM, "P13N");
+
+    INTERNAL_LOG_PARAMS_1.putString(INTERNAL_CHOICE_ID_PARAM, "id1");
+
+    INTERNAL_LOG_PARAMS_2.putString(INTERNAL_CHOICE_ID_PARAM, "id2");
   }
 
   private static final List<Bundle> FAKE_LOGS = new ArrayList<>();
 
+  private static final Correspondence<Bundle, String> TO_STRING =
+      Correspondence.transforming(Bundle::toString, "as String is");
+
   private Personalization personalization;
 
   @Mock private AnalyticsConnector mockAnalyticsConnector;
@@ -74,8 +108,7 @@ public void setUp() {
 
     doAnswer(invocation -> FAKE_LOGS.add(invocation.getArgument(2)))
         .when(mockAnalyticsConnector)
-        .logEvent(
-            eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(ANALYTICS_PULL_EVENT), any(Bundle.class));
+        .logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), anyString(), any(Bundle.class));
 
     personalization = new Personalization(mockAnalyticsConnector);
 
@@ -88,43 +121,46 @@ public void logArmActive_nonPersonalizationKey_notLogged() {
 
     verify(mockAnalyticsConnector, times(0))
         .logEvent(
-            eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(ANALYTICS_PULL_EVENT), any(Bundle.class));
+            eq(ANALYTICS_ORIGIN_PERSONALIZATION),
+            or(eq(EXTERNAL_EVENT), eq(INTERNAL_EVENT)),
+            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))
-        .logEvent(
-            eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(ANALYTICS_PULL_EVENT), any(Bundle.class));
-    assertThat(FAKE_LOGS).hasSize(1);
+        .logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(EXTERNAL_EVENT), any(Bundle.class));
+    verify(mockAnalyticsConnector, times(1))
+        .logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(INTERNAL_EVENT), any(Bundle.class));
+    assertThat(FAKE_LOGS).hasSize(2);
 
-    Bundle params = new Bundle();
-    params.putString(ARM_KEY, "id1");
-    params.putString(ARM_VALUE, "value1");
-    assertThat(FAKE_LOGS.get(0).toString()).isEqualTo(params.toString());
+    assertThat(FAKE_LOGS)
+        .comparingElementsUsing(TO_STRING)
+        .containsExactly(LOG_PARAMS_1.toString(), INTERNAL_LOG_PARAMS_1.toString());
   }
 
   @Test
-  public void logArmActive_multiplePersonalizationKeys_loggedMultiple() {
+  public void logArmActive_multiplePersonalizationKeys_loggedInternallyAndExternally() {
     personalization.logArmActive("key1", CONFIG_CONTAINER);
     personalization.logArmActive("key2", CONFIG_CONTAINER);
+    personalization.logArmActive("key1", CONFIG_CONTAINER);
 
     verify(mockAnalyticsConnector, times(2))
-        .logEvent(
-            eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(ANALYTICS_PULL_EVENT), any(Bundle.class));
-    assertThat(FAKE_LOGS).hasSize(2);
-
-    Bundle params1 = new Bundle();
-    params1.putString(ARM_KEY, "id1");
-    params1.putString(ARM_VALUE, "value1");
-    assertThat(FAKE_LOGS.get(0).toString()).isEqualTo(params1.toString());
-
-    Bundle params2 = new Bundle();
-    params2.putString(ARM_KEY, "id2");
-    params2.putString(ARM_VALUE, "value2");
-    assertThat(FAKE_LOGS.get(1).toString()).isEqualTo(params2.toString());
+        .logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(EXTERNAL_EVENT), any(Bundle.class));
+    verify(mockAnalyticsConnector, times(2))
+        .logEvent(eq(ANALYTICS_ORIGIN_PERSONALIZATION), eq(INTERNAL_EVENT), any(Bundle.class));
+    assertThat(FAKE_LOGS).hasSize(4);
+
+    assertThat(FAKE_LOGS)
+        .comparingElementsUsing(TO_STRING)
+        .containsAtLeast(LOG_PARAMS_1.toString(), LOG_PARAMS_2.toString())
+        .inOrder();
+    assertThat(FAKE_LOGS)
+        .comparingElementsUsing(TO_STRING)
+        .containsAtLeast(INTERNAL_LOG_PARAMS_1.toString(), INTERNAL_LOG_PARAMS_2.toString())
+        .inOrder();
   }
 }

firebase-crashlytics/firebase-crashlytics.gradle

@@ -49,7 +49,6 @@ android {
 }
 
 thirdPartyLicenses {
-    add 'Protocol Buffers', "file://${projectDir}/third_party_licenses/protocol_buffers/LICENSE"
     add 'Tape', "file://${projectDir}/third_party_licenses/tape/LICENSE"
 }