Ssrc bugbash fix (#1117)

* Handle empty context

* fix issue related to update time

* fix string equality

* fix textcase

* Fix lint errors

* Add unit tests

* fix for [438426692](getDouble() logs a malformed warning on type conversion failure)

Using getDouble on a string parameter value, returns the appropriate default static value but logs a warning which looks incorrect ("%s" in the warning message?).

* Update ServerTemplateResponse.java to fix b/438607881

In the server template builder flow using cached template, evaluation using custom signals is not working as intended.

* Update getServerRemoteConfig.json to fix b/438607881

* Update getServerTemplateData.json to fix b/438607881

* fix for bugs

* Resolve comment related to revert of ServerVersion Class

* remove serverVersion

* Resolve comments related to Evaluator

* fix indentation

* fix indentation

* fix indentations

* fix multi line indent

* fix multi line indents

* Update ConditionEvaluator.java

* Update ConditionEvaluator.java

---------

Co-authored-by: Athira M <[email protected]>
Co-authored-by: Varun Rathore <[email protected]>

Author: 3 people

src/main/java/com/google/firebase/remoteconfig/ConditionEvaluator.java

@@ -18,11 +18,12 @@
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.ImmutableMap.toImmutableMap;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.firebase.internal.NonNull;
 import com.google.firebase.internal.Nullable;
-
 import java.math.BigInteger;
 import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
@@ -35,44 +36,46 @@
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
 import java.util.stream.Collectors;
-
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
- 
+
 final class ConditionEvaluator {
   private static final int MAX_CONDITION_RECURSION_DEPTH = 10;
   private static final Logger logger = LoggerFactory.getLogger(ConditionEvaluator.class);
   private static final BigInteger MICRO_PERCENT_MODULO = BigInteger.valueOf(100_000_000L);
 
   /**
    * Evaluates server conditions and assigns a boolean value to each condition.
-   * 
+   *
    * @param conditions List of conditions which are to be evaluated.
-   * @param context A map with additional metadata used during evaluation. 
+   * @param context A map with additional metadata used during evaluation.
    * @return A map of condition to evaluated value.
    */
   @NonNull
   Map<String, Boolean> evaluateConditions(
-      @NonNull List<ServerCondition> conditions,
-      @Nullable KeysAndValues context) {
+      @NonNull List<ServerCondition> conditions, @Nullable KeysAndValues context) {
     checkNotNull(conditions, "List of conditions must not be null.");
     checkArgument(!conditions.isEmpty(), "List of conditions must not be empty.");
-    KeysAndValues evaluationContext = context != null 
-        ? context 
-        : new KeysAndValues.Builder().build();
-
-    Map<String, Boolean> evaluatedConditions = conditions.stream()
-        .collect(Collectors.toMap(
-            ServerCondition::getName,
-            condition -> 
-                evaluateCondition(condition.getCondition(), evaluationContext, /* nestingLevel= */0)
-        ));
+    if (context == null || conditions.isEmpty()) {
+      return ImmutableMap.of();
+    }
+    KeysAndValues evaluationContext =
+        context != null ? context : new KeysAndValues.Builder().build();
+
+    Map<String, Boolean> evaluatedConditions =
+        conditions.stream()
+            .collect(
+                toImmutableMap(
+                    ServerCondition::getName,
+                    condition ->
+                        evaluateCondition(
+                            condition.getCondition(), evaluationContext, /* nestingLevel= */ 0)));
 
     return evaluatedConditions;
   }
 
-  private boolean evaluateCondition(OneOfCondition condition, KeysAndValues context,
-      int nestingLevel) {
+  private boolean evaluateCondition(
+      OneOfCondition condition, KeysAndValues context, int nestingLevel) {
     if (nestingLevel > MAX_CONDITION_RECURSION_DEPTH) {
       logger.warn("Maximum condition recursion depth exceeded.");
       return false;
@@ -95,30 +98,30 @@ private boolean evaluateCondition(OneOfCondition condition, KeysAndValues contex
     return false;
   }
 
-
-  private boolean evaluateOrCondition(OrCondition condition, KeysAndValues context,
-      int nestingLevel) {
+  private boolean evaluateOrCondition(
+      OrCondition condition, KeysAndValues context, int nestingLevel) {
     return condition.getConditions().stream()
         .anyMatch(subCondition -> evaluateCondition(subCondition, context, nestingLevel + 1));
   }
 
-  private boolean evaluateAndCondition(AndCondition condition, KeysAndValues context,
-      int nestingLevel) {
+  private boolean evaluateAndCondition(
+      AndCondition condition, KeysAndValues context, int nestingLevel) {
     return condition.getConditions().stream()
         .allMatch(subCondition -> evaluateCondition(subCondition, context, nestingLevel + 1));
   }
 
-  private boolean evaluateCustomSignalCondition(CustomSignalCondition condition,
-      KeysAndValues context) {
+  private boolean evaluateCustomSignalCondition(
+      CustomSignalCondition condition, KeysAndValues context) {
     CustomSignalOperator customSignalOperator = condition.getCustomSignalOperator();
     String customSignalKey = condition.getCustomSignalKey();
-    ImmutableList<String> targetCustomSignalValues = ImmutableList.copyOf(
-        condition.getTargetCustomSignalValues());
+    ImmutableList<String> targetCustomSignalValues =
+        ImmutableList.copyOf(condition.getTargetCustomSignalValues());
 
     if (targetCustomSignalValues.isEmpty()) {
-      logger.warn(String.format(
-          "Values must be assigned to all custom signal fields. Operator:%s, Key:%s, Values:%s",
-          customSignalOperator, customSignalKey, targetCustomSignalValues));
+      logger.warn(
+          String.format(
+              "Values must be assigned to all custom signal fields. Operator:%s, Key:%s, Values:%s",
+              customSignalOperator, customSignalKey, targetCustomSignalValues));
       return false;
     }
 
@@ -130,64 +133,65 @@ private boolean evaluateCustomSignalCondition(CustomSignalCondition condition,
     switch (customSignalOperator) {
       // String operations.
       case STRING_CONTAINS:
-        return compareStrings(targetCustomSignalValues, customSignalValue,
+        return compareStrings(
+            targetCustomSignalValues,
+            customSignalValue,
             (customSignal, targetSignal) -> customSignal.contains(targetSignal));
       case STRING_DOES_NOT_CONTAIN:
-        return !compareStrings(targetCustomSignalValues, customSignalValue,
+        return !compareStrings(
+            targetCustomSignalValues,
+            customSignalValue,
             (customSignal, targetSignal) -> customSignal.contains(targetSignal));
       case STRING_EXACTLY_MATCHES:
-        return compareStrings(targetCustomSignalValues, customSignalValue,
+        return compareStrings(
+            targetCustomSignalValues,
+            customSignalValue,
             (customSignal, targetSignal) -> customSignal.equals(targetSignal));
       case STRING_CONTAINS_REGEX:
-        return compareStrings(targetCustomSignalValues, customSignalValue,
+        return compareStrings(
+            targetCustomSignalValues,
+            customSignalValue,
             (customSignal, targetSignal) -> compareStringRegex(customSignal, targetSignal));
 
       // Numeric operations.
       case NUMERIC_LESS_THAN:
-        return compareNumbers(targetCustomSignalValues, customSignalValue,
-            (result) -> result < 0);
+        return compareNumbers(targetCustomSignalValues, customSignalValue, (result) -> result < 0);
       case NUMERIC_LESS_EQUAL:
-        return compareNumbers(targetCustomSignalValues, customSignalValue,
-            (result) -> result <= 0);
+        return compareNumbers(targetCustomSignalValues, customSignalValue, (result) -> result <= 0);
       case NUMERIC_EQUAL:
-        return compareNumbers(targetCustomSignalValues, customSignalValue,
-            (result) -> result == 0);
+        return compareNumbers(targetCustomSignalValues, customSignalValue, (result) -> result == 0);
       case NUMERIC_NOT_EQUAL:
-        return compareNumbers(targetCustomSignalValues, customSignalValue,
-            (result) -> result != 0);
+        return compareNumbers(targetCustomSignalValues, customSignalValue, (result) -> result != 0);
       case NUMERIC_GREATER_THAN:
-        return compareNumbers(targetCustomSignalValues, customSignalValue,
-            (result) -> result > 0);
+        return compareNumbers(targetCustomSignalValues, customSignalValue, (result) -> result > 0);
       case NUMERIC_GREATER_EQUAL:
-        return compareNumbers(targetCustomSignalValues, customSignalValue,
-            (result) -> result >= 0);
+        return compareNumbers(targetCustomSignalValues, customSignalValue, (result) -> result >= 0);
 
       // Semantic operations.
       case SEMANTIC_VERSION_EQUAL:
-        return compareSemanticVersions(targetCustomSignalValues, customSignalValue,
-            (result) -> result == 0);
+        return compareSemanticVersions(
+            targetCustomSignalValues, customSignalValue, (result) -> result == 0);
       case SEMANTIC_VERSION_GREATER_EQUAL:
-        return compareSemanticVersions(targetCustomSignalValues, customSignalValue,
-            (result) -> result >= 0);
+        return compareSemanticVersions(
+            targetCustomSignalValues, customSignalValue, (result) -> result >= 0);
       case SEMANTIC_VERSION_GREATER_THAN:
-        return compareSemanticVersions(targetCustomSignalValues, customSignalValue,
-            (result) -> result > 0);
+        return compareSemanticVersions(
+            targetCustomSignalValues, customSignalValue, (result) -> result > 0);
       case SEMANTIC_VERSION_LESS_EQUAL:
-        return compareSemanticVersions(targetCustomSignalValues, customSignalValue,
-            (result) -> result <= 0);
+        return compareSemanticVersions(
+            targetCustomSignalValues, customSignalValue, (result) -> result <= 0);
       case SEMANTIC_VERSION_LESS_THAN:
-        return compareSemanticVersions(targetCustomSignalValues, customSignalValue,
-            (result) -> result < 0);
+        return compareSemanticVersions(
+            targetCustomSignalValues, customSignalValue, (result) -> result < 0);
       case SEMANTIC_VERSION_NOT_EQUAL:
-        return compareSemanticVersions(targetCustomSignalValues, customSignalValue,
-            (result) -> result != 0);
+        return compareSemanticVersions(
+            targetCustomSignalValues, customSignalValue, (result) -> result != 0);
       default:
         return false;
     }
   }
 
-  private boolean evaluatePercentCondition(PercentCondition condition,
-      KeysAndValues context) {
+  private boolean evaluatePercentCondition(PercentCondition condition, KeysAndValues context) {
     if (!context.containsKey("randomizationId")) {
       logger.warn("Percentage operation must not be performed without randomizationId");
       return false;
@@ -197,18 +201,16 @@ private boolean evaluatePercentCondition(PercentCondition condition,
 
     // The micro-percent interval to be used with the BETWEEN operator.
     MicroPercentRange microPercentRange = condition.getMicroPercentRange();
-    int microPercentUpperBound = microPercentRange != null
-        ? microPercentRange.getMicroPercentUpperBound()
-        : 0;
-    int microPercentLowerBound = microPercentRange != null
-        ? microPercentRange.getMicroPercentLowerBound()
-        : 0;
+    int microPercentUpperBound =
+        microPercentRange != null ? microPercentRange.getMicroPercentUpperBound() : 0;
+    int microPercentLowerBound =
+        microPercentRange != null ? microPercentRange.getMicroPercentLowerBound() : 0;
     // The limit of percentiles to target in micro-percents when using the
     // LESS_OR_EQUAL and GREATER_THAN operators. The value must be in the range [0
     // and 100000000].
     int microPercent = condition.getMicroPercent();
-    BigInteger microPercentile = getMicroPercentile(condition.getSeed(),
-        context.get("randomizationId"));
+    BigInteger microPercentile =
+        getMicroPercentile(condition.getSeed(), context.get("randomizationId"));
     switch (operator) {
       case LESS_OR_EQUAL:
         return microPercentile.compareTo(BigInteger.valueOf(microPercent)) <= 0;
@@ -246,10 +248,12 @@ private BigInteger hashSeededRandomizationId(String seededRandomizationId) {
     }
   }
 
-  private boolean compareStrings(ImmutableList<String> targetValues, String customSignal,
-                              BiPredicate<String, String> compareFunction) {
-    return targetValues.stream().anyMatch(targetValue -> 
-              compareFunction.test(customSignal, targetValue));
+  private boolean compareStrings(
+      ImmutableList<String> targetValues,
+      String customSignal,
+      BiPredicate<String, String> compareFunction) {
+    return targetValues.stream()
+        .anyMatch(targetValue -> compareFunction.test(customSignal, targetValue));
   }
 
   private boolean compareStringRegex(String customSignal, String targetSignal) {
@@ -260,12 +264,13 @@ private boolean compareStringRegex(String customSignal, String targetSignal) {
     }
   }
 
-  private boolean compareNumbers(ImmutableList<String> targetValues, String customSignal,
-                            IntPredicate compareFunction) {
+  private boolean compareNumbers(
+      ImmutableList<String> targetValues, String customSignal, IntPredicate compareFunction) {
     if (targetValues.size() != 1) {
-      logger.warn(String.format(
-          "Target values must contain 1 element for numeric operations. Target Value: %s",
-          targetValues));
+      logger.warn(
+          String.format(
+              "Target values must contain 1 element for numeric operations. Target Value: %s",
+              targetValues));
       return false;
     }
 
@@ -275,23 +280,22 @@ private boolean compareNumbers(ImmutableList<String> targetValues, String custom
       int comparisonResult = Double.compare(customSignalDouble, targetValue);
       return compareFunction.test(comparisonResult);
     } catch (NumberFormatException e) {
-      logger.warn("Error parsing numeric values: customSignal=%s, targetValue=%s",
+      logger.warn(
+          "Error parsing numeric values: customSignal=%s, targetValue=%s",
           customSignal, targetValues.get(0), e);
       return false;
     }
   }
 
-  private boolean compareSemanticVersions(ImmutableList<String> targetValues,
-                                      String customSignal,
-                                      IntPredicate compareFunction) {
+  private boolean compareSemanticVersions(
+      ImmutableList<String> targetValues, String customSignal, IntPredicate compareFunction) {
     if (targetValues.size() != 1) {
       logger.warn(String.format("Target values must contain 1 element for semantic operation."));
       return false;
     }
 
     String targetValueString = targetValues.get(0);
-    if (!validateSemanticVersion(targetValueString)
-        || !validateSemanticVersion(customSignal)) {
+    if (!validateSemanticVersion(targetValueString) || !validateSemanticVersion(customSignal)) {
       return false;
     }
 
@@ -300,7 +304,8 @@ private boolean compareSemanticVersions(ImmutableList<String> targetValues,
 
     int maxLength = 5;
     if (targetVersion.size() > maxLength || customSignalVersion.size() > maxLength) {
-      logger.warn("Semantic version max length(%s) exceeded. Target: %s, Custom Signal: %s",
+      logger.warn(
+          "Semantic version max length(%s) exceeded. Target: %s, Custom Signal: %s",
           maxLength, targetValueString, customSignal);
       return false;
     }
@@ -316,8 +321,8 @@ private int compareSemanticVersions(List<Integer> version1, List<Integer> versio
 
     for (int i = 0; i < maxLength; i++) {
       // Default to 0 if segment is missing
-      int v1 =  i < version1Size ? version1.get(i) : 0;
-      int v2 =  i < version2Size ? version2.get(i) : 0;
+      int v1 = i < version1Size ? version1.get(i) : 0;
+      int v2 = i < version2Size ? version2.get(i) : 0;
 
       int comparison = Integer.compare(v1, v2);
       if (comparison != 0) {
@@ -330,8 +335,8 @@ private int compareSemanticVersions(List<Integer> version1, List<Integer> versio
 
   private List<Integer> parseSemanticVersion(String versionString) {
     return Arrays.stream(versionString.split("\\."))
-          .map(Integer::parseInt)
-          .collect(Collectors.toList());
+        .map(Integer::parseInt)
+        .collect(Collectors.toList());
   }
 
   private boolean validateSemanticVersion(String version) {

src/main/java/com/google/firebase/remoteconfig/ServerTemplateImpl.java

@@ -20,9 +20,8 @@
 import com.google.api.core.ApiFutures;
 import com.google.common.collect.ImmutableMap;
 import com.google.firebase.ErrorCode;
+import com.google.firebase.internal.Nullable;
 import com.google.firebase.remoteconfig.internal.TemplateResponse.ParameterValueResponse;
-import com.google.gson.Gson;
-import com.google.gson.GsonBuilder;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -43,7 +42,7 @@ public static class Builder implements ServerTemplate.Builder {
     private KeysAndValues defaultConfig;
     private String cachedTemplate;
     private FirebaseRemoteConfigClient client;
-  
+
     Builder(FirebaseRemoteConfigClient remoteConfigClient) {
       this.client = remoteConfigClient;
     }
@@ -78,7 +77,8 @@ private ServerTemplateImpl(Builder builder) {
   }
 
   @Override
-  public ServerConfig evaluate(KeysAndValues context) throws FirebaseRemoteConfigException {
+  public ServerConfig evaluate(@Nullable KeysAndValues context)
+      throws FirebaseRemoteConfigException {
     if (this.cache == null) {
       throw new FirebaseRemoteConfigException(ErrorCode.FAILED_PRECONDITION,
           "No Remote Config Server template in cache. Call load() before calling evaluate().");
@@ -103,8 +103,7 @@ public ServerConfig evaluate(KeysAndValues context) throws FirebaseRemoteConfigE
 
   @Override
   public ServerConfig evaluate() throws FirebaseRemoteConfigException {
-    KeysAndValues context = new KeysAndValues.Builder().build();
-    return evaluate(context);
+    return evaluate(null);
   }
 
   @Override
@@ -126,8 +125,7 @@ public String getCachedTemplate() {
 
   @Override
   public String toJson() {
-    Gson gson = new GsonBuilder().setPrettyPrinting().create();
-    return gson.toJson(this.cache);
+    return this.cache.toJSON();
   }
 
   private void mergeDerivedConfigValues(ImmutableMap<String, Boolean> evaluatedCondition,