Merge branch 'refs/heads/tushar-khandelwal/rc-custom-targeting' into tushar-khandelwal/rename-configmetadataclient

# Conflicts:
#	firebase-config/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfig.java
#	firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigSharedPrefsClient.java
#	firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/ConfigSharedPrefsClientTest.java

Author: tusharkhandelwal8

firebase-config/api.txt

@@ -16,6 +16,17 @@ package com.google.firebase.remoteconfig {
     method public void remove();
   }
 
+  public class CustomSignals {
+  }
+
+  public static class CustomSignals.Builder {
+    ctor public CustomSignals.Builder();
+    method @NonNull public com.google.firebase.remoteconfig.CustomSignals build();
+    method @NonNull public com.google.firebase.remoteconfig.CustomSignals.Builder put(@NonNull String, @Nullable String);
+    method @NonNull public com.google.firebase.remoteconfig.CustomSignals.Builder put(@NonNull String, long);
+    method @NonNull public com.google.firebase.remoteconfig.CustomSignals.Builder put(@NonNull String, double);
+  }
+
   public class FirebaseRemoteConfig {
     method @NonNull public com.google.android.gms.tasks.Task<java.lang.Boolean> activate();
     method @NonNull public com.google.firebase.remoteconfig.ConfigUpdateListenerRegistration addOnConfigUpdateListener(@NonNull com.google.firebase.remoteconfig.ConfigUpdateListener);
@@ -35,7 +46,7 @@ package com.google.firebase.remoteconfig {
     method @NonNull public com.google.firebase.remoteconfig.FirebaseRemoteConfigValue getValue(@NonNull String);
     method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> reset();
     method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setConfigSettingsAsync(@NonNull com.google.firebase.remoteconfig.FirebaseRemoteConfigSettings);
-    method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setCustomSignals(@NonNull java.util.Map<java.lang.String,java.lang.Object>);
+    method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setCustomSignals(@NonNull com.google.firebase.remoteconfig.CustomSignals);
     method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setDefaultsAsync(@NonNull java.util.Map<java.lang.String,java.lang.Object>);
     method @NonNull public com.google.android.gms.tasks.Task<java.lang.Void> setDefaultsAsync(@XmlRes int);
     field public static final boolean DEFAULT_VALUE_FOR_BOOLEAN = false;

firebase-config/src/main/java/com/google/firebase/remoteconfig/CustomSignals.java

@@ -0,0 +1,62 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.firebase.remoteconfig;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
+import java.util.HashMap;
+import java.util.Map;
+
+/**
+ * Helper class which handles the storage and conversion to strings of key/value pairs with
+ * heterogeneous value types for custom signals.
+ */
+public class CustomSignals {
+
+  final Map<String, String> customSignals;
+
+  public static class Builder {
+    // Holds the converted pairs of custom keys and values.
+    private Map<String, String> customSignals = new HashMap<String, String>();
+
+    // Methods to accept keys and values and convert values to strings.
+    @NonNull
+    public Builder put(@NonNull String key, @Nullable String value) {
+      customSignals.put(key, value);
+      return this;
+    }
+
+    @NonNull
+    public Builder put(@NonNull String key, long value) {
+      customSignals.put(key, Long.toString(value));
+      return this;
+    }
+
+    @NonNull
+    public Builder put(@NonNull String key, double value) {
+      customSignals.put(key, Double.toString(value));
+      return this;
+    }
+
+    @NonNull
+    public CustomSignals build() {
+      return new CustomSignals(this);
+    }
+  }
+
+  CustomSignals(@NonNull Builder builder) {
+    this.customSignals = builder.customSignals;
+  }
+}

firebase-config/src/main/java/com/google/firebase/remoteconfig/FirebaseRemoteConfig.java

@@ -655,21 +655,18 @@ private Task<Void> setDefaultsWithStringsMapAsync(Map<String, String> defaultsSt
   /**
    * Asynchronously changes the custom signals for this {@link FirebaseRemoteConfig} instance.
    *
-   * <p>The values in {@code customSignals} must be one of the following types:
+   * <p>The {@code customSignals} parameter should be an instance of {@link CustomSignals}, which
+   * enforces the allowed types for custom signal values (String, Long or Double).
    *
-   * <ul>
-   *   <li><code>Long</code>
-   *   <li><code>String</code>
-   * </ul>
-   *
-   * @param customSignals Map (key, value) of the custom signals to be set for the app instance
+   * @param customSignalsMap A dictionary of keys and the values of the custom signals to be set for
+   *                         the app instance
    */
   @NonNull
-  public Task<Void> setCustomSignals(@NonNull Map<String, Object> customSignals) {
+  public Task<Void> setCustomSignals(@NonNull CustomSignals customSignalsMap) {
     return Tasks.call(
         executor,
         () -> {
-          frcSharedPrefs.setCustomSignals(customSignals);
+          frcSharedPrefs.setCustomSignals(customSignalsMap.customSignals);
           return null;
         });
   }

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

@@ -94,7 +94,7 @@ public class ConfigFetchHttpClient {
   private final String apiKey;
   private final String projectNumber;
   private final String namespace;
-  Map<String, Object> customSignalMap;
+  Map<String, String> customSignalsMap;
   private final long connectTimeoutInSeconds;
   private final long readTimeoutInSeconds;
 
@@ -109,15 +109,15 @@ public ConfigFetchHttpClient(
       String namespace,
       long connectTimeoutInSeconds,
       long readTimeoutInSeconds,
-      Map<String, Object> customSignalMap) {
+      Map<String, String> customSignalsMap) {
     this.context = context;
     this.appId = appId;
     this.apiKey = apiKey;
     this.projectNumber = extractProjectNumberFromAppId(appId);
     this.namespace = namespace;
     this.connectTimeoutInSeconds = connectTimeoutInSeconds;
     this.readTimeoutInSeconds = readTimeoutInSeconds;
-    this.customSignalMap = customSignalMap;
+    this.customSignalsMap = customSignalsMap;
   }
 
   /** Used to verify that the timeout is being set correctly. */
@@ -351,7 +351,9 @@ private JSONObject createFetchRequestBody(
 
     requestBodyMap.put(ANALYTICS_USER_PROPERTIES, new JSONObject(analyticsUserProperties));
 
-    requestBodyMap.put(CUSTOM_SIGNALS, new JSONObject(customSignalMap));
+    if (!customSignalsMap.isEmpty()) {
+      requestBodyMap.put(CUSTOM_SIGNALS, new JSONObject(customSignalsMap));
+    }
 
     if (firstOpenTime != null) {
       requestBodyMap.put(FIRST_OPEN_TIME, convertToISOString(firstOpenTime));

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

@@ -18,12 +18,14 @@
 import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.LAST_FETCH_STATUS_NO_FETCH_YET;
 import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.LAST_FETCH_STATUS_SUCCESS;
 import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.LAST_FETCH_STATUS_THROTTLED;
+import static com.google.firebase.remoteconfig.FirebaseRemoteConfig.TAG;
 import static com.google.firebase.remoteconfig.RemoteConfigComponent.CONNECTION_TIMEOUT_IN_SECONDS;
 import static com.google.firebase.remoteconfig.RemoteConfigConstants.RequestFieldKey.CUSTOM_SIGNALS;
 import static com.google.firebase.remoteconfig.internal.ConfigFetchHandler.DEFAULT_MINIMUM_FETCH_INTERVAL_IN_SECONDS;
 import static java.lang.annotation.RetentionPolicy.SOURCE;
 
 import android.content.SharedPreferences;
+import android.util.Log;
 import androidx.annotation.IntDef;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
@@ -81,6 +83,13 @@ public class ConfigSharedPrefsClient {
   private static final String REALTIME_BACKOFF_END_TIME_IN_MILLIS_KEY =
       "realtime_backoff_end_time_in_millis";
 
+  /** Constants for custom signal limits.*/
+  private static final int CUSTOM_SIGNALS_MAX_KEY_LENGTH = 250;
+
+  private static final int CUSTOM_SIGNALS_MAX_STRING_VALUE_LENGTH = 500;
+
+  private static final int CUSTOM_SIGNALS_MAX_COUNT = 100;
+
   private final SharedPreferences frcSharedPrefs;
 
   private final Object frcInfoLock;
@@ -259,22 +268,24 @@ void setBackoffMetadata(int numFailedFetches, Date backoffEndTime) {
     }
   }
 
-  public void setCustomSignals(Map<String, Object> newCustomSignals) {
+  public void setCustomSignals(Map<String, String> newCustomSignals) {
     synchronized (customSignalsLock) {
       // Retrieve existing custom signals
-      Map<String, Object> existingCustomSignals = getCustomSignals();
+      Map<String, String> existingCustomSignals = getCustomSignals();
 
-      for (Map.Entry<String, Object> entry : newCustomSignals.entrySet()) {
+      for (Map.Entry<String, String> entry : newCustomSignals.entrySet()) {
         String key = entry.getKey();
-        Object value = entry.getValue();
-
-        // Validate value type, and key and value length
-        if (value != null && !(value instanceof String || value instanceof Long)) {
-          throw new IllegalArgumentException("Custom signal values must be of type String or Long");
-        }
-        if (key.length() > 250 || (value instanceof String && ((String) value).length() > 500)) {
-          throw new IllegalArgumentException(
-              "Custom signal keys must be 250 characters or less, and string values must be 500 characters or less.");
+        String value = entry.getValue();
+
+        // Validate key and value length
+        if (key.length() > CUSTOM_SIGNALS_MAX_KEY_LENGTH
+            || (value != null && value.length() > CUSTOM_SIGNALS_MAX_STRING_VALUE_LENGTH)) {
+          Log.w(
+              TAG,
+              String.format(
+                  "Invalid custom signal: Custom signal keys must be %d characters or less, and values must be %d characters or less.",
+                  CUSTOM_SIGNALS_MAX_KEY_LENGTH, CUSTOM_SIGNALS_MAX_STRING_VALUE_LENGTH));
+          return;
         }
 
         // Merge new signals with existing ones, overwriting existing keys.
@@ -290,9 +301,13 @@ public void setCustomSignals(Map<String, Object> newCustomSignals) {
       if (existingCustomSignals.equals(getCustomSignals())) {
         return;
       }
-      if (existingCustomSignals.size() > 100) {
-        throw new IllegalArgumentException(
-            "Too many custom signals provided. The maximum allowed is 100.");
+      if (existingCustomSignals.size() > CUSTOM_SIGNALS_MAX_COUNT) {
+        Log.w(
+            TAG,
+            String.format(
+                "Invalid custom signal: Too many custom signals provided. The maximum allowed is %d.",
+                CUSTOM_SIGNALS_MAX_COUNT));
+        return;
       }
 
       frcSharedPrefs
@@ -302,18 +317,15 @@ public void setCustomSignals(Map<String, Object> newCustomSignals) {
     }
   }
 
-  public Map<String, Object> getCustomSignals() {
+  public Map<String, String> getCustomSignals() {
     String jsonString = frcSharedPrefs.getString(CUSTOM_SIGNALS, "{}");
     try {
       JSONObject existingCustomSignalsJson = new JSONObject(jsonString);
-      Map<String, Object> custom_signals = new HashMap<>();
+      Map<String, String> custom_signals = new HashMap<>();
       Iterator<String> keys = existingCustomSignalsJson.keys();
       while (keys.hasNext()) {
         String key = keys.next();
-        Object value = existingCustomSignalsJson.get(key);
-        if (value instanceof Integer) {
-          value = existingCustomSignalsJson.getLong(key);
-        }
+        String value = existingCustomSignalsJson.optString(key);
         custom_signals.put(key, value);
       }
       return custom_signals;

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

@@ -86,7 +86,7 @@ public class ConfigFetchHttpClientTest {
       "etag-" + PROJECT_NUMBER + "-" + DEFAULT_NAMESPACE + "-fetch-%d";
   private static final String FIRST_ETAG = String.format(ETAG_FORMAT, 1);
   private static final String SECOND_ETAG = String.format(ETAG_FORMAT, 2);
-  private static final Map<String, Object> SAMPLE_CUSTOM_SIGNALS =
+  private static final Map<String, String> SAMPLE_CUSTOM_SIGNALS =
       ImmutableMap.of(
           "subscription", "premium",
           "age", "20");

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

@@ -25,7 +25,6 @@
 import static com.google.firebase.remoteconfig.internal.ConfigSharedPrefsClient.LAST_FETCH_TIME_NO_FETCH_YET;
 import static com.google.firebase.remoteconfig.internal.ConfigSharedPrefsClient.NO_BACKOFF_TIME;
 import static com.google.firebase.remoteconfig.internal.ConfigSharedPrefsClient.NO_FAILED_FETCHES;
-import static com.google.firebase.remoteconfig.testutil.Assert.assertThrows;
 
 import android.content.Context;
 import android.content.SharedPreferences;
@@ -364,7 +363,7 @@ public void getCustomSignals_isNotSet_returnsEmptyMap() {
 
   @Test
   public void getCustomSignals_isSet_returnsCustomSignals() {
-    Map<String, Object> SAMPLE_CUSTOM_SIGNALS =
+    Map<String, String> SAMPLE_CUSTOM_SIGNALS =
         ImmutableMap.of(
             "subscription", "premium",
             "age", "20");
@@ -374,40 +373,22 @@ public void getCustomSignals_isSet_returnsCustomSignals() {
 
   @Test
   public void setCustomSignals_multipleTimes_addsNewSignals() {
-    Map<String, Object> signals1 = ImmutableMap.of("subscription", "premium");
-    Map<String, Object> signals2 = ImmutableMap.of("age", 20L);
+    Map<String, String> signals1 = ImmutableMap.of("subscription", "premium");
+    Map<String, String> signals2 = ImmutableMap.of("age", "20");
     sharedPrefsClient.setCustomSignals(signals1);
     sharedPrefsClient.setCustomSignals(signals2);
-    Map<String, Object> expectedSignals = ImmutableMap.of("subscription", "premium", "age", 20L);
+    Map<String, String> expectedSignals = ImmutableMap.of("subscription", "premium", "age", "20");
     assertThat(sharedPrefsClient.getCustomSignals()).isEqualTo(expectedSignals);
   }
 
   @Test
   public void setCustomSignals_nullValue_removesSignal() {
-    Map<String, Object> signals1 = ImmutableMap.of("subscription", "premium", "age", 20L);
+    Map<String, String> signals1 = ImmutableMap.of("subscription", "premium", "age", "20");
     sharedPrefsClient.setCustomSignals(signals1);
-    Map<String, Object> signals2 = new HashMap<>();
+    Map<String, String> signals2 = new HashMap<>();
     signals2.put("age", null);
     sharedPrefsClient.setCustomSignals(signals2);
     Map<String, Object> expectedSignals = ImmutableMap.of("subscription", "premium");
     assertThat(sharedPrefsClient.getCustomSignals()).isEqualTo(expectedSignals);
   }
-
-  @Test
-  public void setCustomSignals_invalidInput_throwsException() {
-    Map<String, Object> invalidSignals1 = new HashMap<>();
-    invalidSignals1.put("name", true);
-    assertThrows(
-        IllegalArgumentException.class, () -> sharedPrefsClient.setCustomSignals(invalidSignals1));
-
-    Map<String, Object> invalidSignals2 = new HashMap<>();
-    invalidSignals2.put("a".repeat(251), "value");
-    assertThrows(
-        IllegalArgumentException.class, () -> sharedPrefsClient.setCustomSignals(invalidSignals2));
-
-    Map<String, Object> invalidSignals3 = new HashMap<>();
-    invalidSignals3.put("key", "a".repeat(501));
-    assertThrows(
-        IllegalArgumentException.class, () -> sharedPrefsClient.setCustomSignals(invalidSignals3));
-  }
 }