Make ABT Dynamic-Module compliant. (#2524)

* Make ABT Dynamic-Module compliant.

Changes the dependency on analytics to an optional Provider.

* Rename and improve lint test that verifies fall positives are gone.

* Address review comments.

Author: vkryachko

firebase-abt/src/main/java/com/google/firebase/abt/FirebaseABTesting.java

@@ -23,6 +23,7 @@
 import androidx.annotation.WorkerThread;
 import com.google.firebase.analytics.connector.AnalyticsConnector;
 import com.google.firebase.analytics.connector.AnalyticsConnector.ConditionalUserProperty;
+import com.google.firebase.inject.Provider;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 import java.util.ArrayDeque;
@@ -56,7 +57,7 @@ public class FirebaseABTesting {
   static final String ORIGIN_LAST_KNOWN_START_TIME_KEY_FORMAT = "%s_lastKnownExperimentStartTime";
 
   /** The App's Firebase Analytics client. */
-  private final AnalyticsConnector analyticsConnector;
+  private final Provider<AnalyticsConnector> analyticsConnector;
 
   /** The name of an ABT client. */
   private final String originService;
@@ -89,7 +90,7 @@ public class FirebaseABTesting {
    */
   public FirebaseABTesting(
       Context unusedAppContext,
-      AnalyticsConnector analyticsConnector,
+      Provider<AnalyticsConnector> analyticsConnector,
       @OriginService String originService) {
     this.analyticsConnector = analyticsConnector;
     this.originService = originService;
@@ -333,11 +334,11 @@ private static List<AbtExperimentInfo> convertMapsToExperimentInfos(
   }
 
   private void addExperimentToAnalytics(ConditionalUserProperty experiment) {
-    analyticsConnector.setConditionalUserProperty(experiment);
+    analyticsConnector.get().setConditionalUserProperty(experiment);
   }
 
   private void throwAbtExceptionIfAnalyticsIsNull() throws AbtException {
-    if (analyticsConnector == null) {
+    if (analyticsConnector.get() == null) {
       throw new AbtException(
           "The Analytics SDK is not available. "
               + "Please check that the Analytics SDK is included in your app dependencies.");
@@ -350,14 +351,16 @@ private void throwAbtExceptionIfAnalyticsIsNull() throws AbtException {
    * breaking, or if the underlying Analytics clear method is failing.
    */
   private void removeExperimentFromAnalytics(String experimentId) {
-    analyticsConnector.clearConditionalUserProperty(
-        experimentId, /*clearEventName=*/ null, /*clearEventParams=*/ null);
+    analyticsConnector
+        .get()
+        .clearConditionalUserProperty(
+            experimentId, /*clearEventName=*/ null, /*clearEventParams=*/ null);
   }
 
   @WorkerThread
   private int getMaxUserPropertiesInAnalytics() {
     if (maxUserProperties == null) {
-      maxUserProperties = analyticsConnector.getMaxUserProperties(originService);
+      maxUserProperties = analyticsConnector.get().getMaxUserProperties(originService);
     }
     return maxUserProperties;
   }
@@ -370,7 +373,8 @@ private int getMaxUserPropertiesInAnalytics() {
    */
   @WorkerThread
   private List<ConditionalUserProperty> getAllExperimentsInAnalytics() {
-    return analyticsConnector.getConditionalUserProperties(
-        originService, /*propertyNamePrefix=*/ "");
+    return analyticsConnector
+        .get()
+        .getConditionalUserProperties(originService, /*propertyNamePrefix=*/ "");
   }
 }

firebase-abt/src/main/java/com/google/firebase/abt/component/AbtComponent.java

@@ -20,6 +20,7 @@
 import com.google.firebase.abt.FirebaseABTesting;
 import com.google.firebase.abt.FirebaseABTesting.OriginService;
 import com.google.firebase.analytics.connector.AnalyticsConnector;
+import com.google.firebase.inject.Provider;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -37,11 +38,11 @@ public class AbtComponent {
   private final Map<String, FirebaseABTesting> abtOriginInstances = new HashMap<>();
 
   private final Context appContext;
-  private final AnalyticsConnector analyticsConnector;
+  private final Provider<AnalyticsConnector> analyticsConnector;
 
   /** Firebase ABT Component constructor. */
   @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE)
-  protected AbtComponent(Context appContext, AnalyticsConnector analyticsConnector) {
+  protected AbtComponent(Context appContext, Provider<AnalyticsConnector> analyticsConnector) {
     this.appContext = appContext;
     this.analyticsConnector = analyticsConnector;
   }

firebase-abt/src/main/java/com/google/firebase/abt/component/AbtRegistrar.java

@@ -38,11 +38,12 @@ public List<Component<?>> getComponents() {
     return Arrays.asList(
         Component.builder(AbtComponent.class)
             .add(Dependency.required(Context.class))
-            .add(Dependency.optional(AnalyticsConnector.class))
+            .add(Dependency.optionalProvider(AnalyticsConnector.class))
             .factory(
                 container ->
                     new AbtComponent(
-                        container.get(Context.class), container.get(AnalyticsConnector.class)))
+                        container.get(Context.class),
+                        container.getProvider(AnalyticsConnector.class)))
             .build(),
         LibraryVersionComponent.create("fire-abt", BuildConfig.VERSION_NAME));
   }

firebase-abt/src/test/java/com/google/firebase/abt/FirebaseABTestingTest.java

@@ -76,7 +76,7 @@ public void setUp() {
     firebaseAbt =
         new FirebaseABTesting(
             /*unusedAppContext=*/ null,
-            /*analyticsConnector=*/ mockAnalyticsConnector,
+            /*analyticsConnector=*/ () -> mockAnalyticsConnector,
             ORIGIN_SERVICE);
 
     when(mockAnalyticsConnector.getMaxUserProperties(ORIGIN_SERVICE))
@@ -189,7 +189,7 @@ public void replaceAllExperiments_totalExperimentsExceedsAnalyticsLimit_oldExper
   public void replaceAllExperiments_analyticsSdkUnavailable_throwsAbtException() {
     firebaseAbt =
         new FirebaseABTesting(
-            /*unusedAppContext=*/ null, /*analyticsConnector=*/ null, ORIGIN_SERVICE);
+            /*unusedAppContext=*/ null, /*analyticsConnector=*/ () -> null, ORIGIN_SERVICE);
 
     AbtException actualException =
         assertThrows(
@@ -243,7 +243,7 @@ public void removeAllExperiments_existExperimentsInAnalytics_experimentsClearedF
   public void removeAllExperiments_analyticsSdkUnavailable_throwsAbtException() {
     firebaseAbt =
         new FirebaseABTesting(
-            /*unusedAppContext=*/ null, /*analyticsConnector=*/ null, ORIGIN_SERVICE);
+            /*unusedAppContext=*/ null, /*analyticsConnector=*/ () -> null, ORIGIN_SERVICE);
 
     AbtException actualException =
         assertThrows(AbtException.class, () -> firebaseAbt.removeAllExperiments());
@@ -281,7 +281,7 @@ public void getAllExperiments_existExperimentsInAnalytics_returnAllExperiments()
   public void getAllExperiments_analyticsSdkUnavailable_throwsAbtException() {
     firebaseAbt =
         new FirebaseABTesting(
-            /*unusedAppContext=*/ null, /*analyticsConnector=*/ null, ORIGIN_SERVICE);
+            /*unusedAppContext=*/ null, /*analyticsConnector=*/ () -> null, ORIGIN_SERVICE);
 
     AbtException actualException =
         assertThrows(AbtException.class, () -> firebaseAbt.getAllExperiments());

tools/lint/src/main/kotlin/ProviderAssignmentDetector.kt

@@ -39,9 +39,17 @@ class ProviderAssignmentDetector : Detector(), SourceCodeScanner {
         if (!isProviderGet(method)) {
             return
         }
-        val assignmentTarget = node.getParentOfType<JavaUAssignmentExpression>(
-                JavaUAssignmentExpression::class.java,
-                true)?.leftOperand as? UReferenceExpression ?: return
+        val assignmentExpression = node
+                .getParentOfType<JavaUAssignmentExpression>(
+                    JavaUAssignmentExpression::class.java, true)
+        val assignmentTarget = assignmentExpression?.leftOperand as? UReferenceExpression ?: return
+
+        // This would only be true if assigning the result of get(),
+        // in cases like foo = p.get().someMethod() there would be an intermediate parent
+        // and we don't want to trigger in such cases.
+        if (assignmentExpression != node.uastParent?.uastParent) {
+            return
+        }
         assignmentTarget.resolve()?.let {
             if (it is PsiField) {
                 context.report(

tools/lint/src/test/kotlin/ProviderAssignmentDetectorTests.kt

@@ -87,4 +87,21 @@ class ProviderAssignmentDetectorTests : LintDetectorTest() {
                 .run()
                 .expectClean()
     }
+
+    fun test_assignmentOfAPropertyOrMethodOfTheProvidedObject_shouldSucceed() {
+        lint().files(java(providerSource()), java("""
+            import com.google.firebase.inject.Provider;
+
+            class Foo {
+              private final int length;
+              private final String s;
+              Foo(Provider<String> p) {
+                length = p.get().length;
+                s = p.get().toString();
+              }
+            }
+        """.trimIndent()))
+                .run()
+                .expectClean()
+    }
 }