Skip to content

Ssrc bugbash fix #1117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 21 commits into
base: ssrc
Choose a base branch
from
Open

Ssrc bugbash fix #1117

Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
486e841
Handle empty context
Aug 7, 2025
91acd60
fix issue related to update time
Aug 8, 2025
422d227
fix string equality
Aug 8, 2025
38f658d
fix textcase
Aug 8, 2025
ee44298
Fix lint errors
Aug 11, 2025
23b1b95
Add unit tests
Aug 12, 2025
7a28b89
fix for [438426692](getDouble() logs a malformed warning on type conv…
rathovarun1032 Aug 13, 2025
ce4caea
Update ServerTemplateResponse.java to fix b/438607881
rathovarun1032 Aug 14, 2025
6135680
Update getServerRemoteConfig.json to fix b/438607881
rathovarun1032 Aug 14, 2025
28166e0
Update getServerTemplateData.json to fix b/438607881
rathovarun1032 Aug 14, 2025
8932e7e
fix for bugs
Aug 14, 2025
dfecfc6
Resolve comment related to revert of ServerVersion Class
Aug 18, 2025
a0b71bd
remove serverVersion
Aug 18, 2025
8fc3cb7
Resolve comments related to Evaluator
Aug 18, 2025
95910ad
fix indentation
Aug 18, 2025
3257dc5
fix indentation
Aug 18, 2025
d75cfb2
fix indentations
Aug 18, 2025
9fabd60
fix multi line indent
Aug 18, 2025
db3fc82
fix multi line indents
Aug 18, 2025
4dd8b1d
Update ConditionEvaluator.java
rathovarun1032 Aug 22, 2025
1a8006b
Update ConditionEvaluator.java
rathovarun1032 Aug 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 93 additions & 89 deletions src/main/java/com/google/firebase/remoteconfig/ConditionEvaluator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
import static com.google.common.base.Preconditions.checkNotNull;

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;
Expand All @@ -35,44 +35,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(
ImmutableMap.toImmutableMap(
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider using a static import.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

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;
Expand All @@ -95,30 +97,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;
}

Expand All @@ -130,64 +132,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;
Expand All @@ -197,18 +200,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;
Expand Down Expand Up @@ -246,10 +247,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) {
Expand All @@ -260,12 +263,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;
}

Expand All @@ -275,23 +279,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;
}

Expand All @@ -300,7 +303,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;
}
Expand All @@ -316,8 +320,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) {
Expand All @@ -330,8 +334,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) {
Expand Down
14 changes: 6 additions & 8 deletions src/main/java/com/google/firebase/remoteconfig/ServerTemplateImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
}
Expand Down Expand Up @@ -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().");
Expand All @@ -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
Expand All @@ -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,
Expand Down
Loading