diff --git a/src/main/java/com/google/firebase/remoteconfig/ConditionEvaluator.java b/src/main/java/com/google/firebase/remoteconfig/ConditionEvaluator.java index 2bd828e2..8361a143 100644 --- a/src/main/java/com/google/firebase/remoteconfig/ConditionEvaluator.java +++ b/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,10 +36,9 @@ 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); @@ -46,33 +46,36 @@ final class ConditionEvaluator { /** * 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 evaluateConditions( - @NonNull List conditions, - @Nullable KeysAndValues context) { + @NonNull List 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 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 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 targetCustomSignalValues = ImmutableList.copyOf( - condition.getTargetCustomSignalValues()); + ImmutableList 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 targetValues, String customSignal, - BiPredicate compareFunction) { - return targetValues.stream().anyMatch(targetValue -> - compareFunction.test(customSignal, targetValue)); + private boolean compareStrings( + ImmutableList targetValues, + String customSignal, + BiPredicate 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 targetValues, String customSignal, - IntPredicate compareFunction) { + private boolean compareNumbers( + ImmutableList 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 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 targetValues, - String customSignal, - IntPredicate compareFunction) { + private boolean compareSemanticVersions( + ImmutableList 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 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 version1, List 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 version1, List versio private List 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) { diff --git a/src/main/java/com/google/firebase/remoteconfig/ServerTemplateImpl.java b/src/main/java/com/google/firebase/remoteconfig/ServerTemplateImpl.java index c85fa160..d38ac51c 100644 --- a/src/main/java/com/google/firebase/remoteconfig/ServerTemplateImpl.java +++ b/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 evaluatedCondition, diff --git a/src/main/java/com/google/firebase/remoteconfig/Value.java b/src/main/java/com/google/firebase/remoteconfig/Value.java index 7f04aefe..7935e849 100644 --- a/src/main/java/com/google/firebase/remoteconfig/Value.java +++ b/src/main/java/com/google/firebase/remoteconfig/Value.java @@ -100,7 +100,7 @@ long asLong() { try { return Long.parseLong(value); } catch (NumberFormatException e) { - logger.warn("Unable to convert %s to long type.", value); + logger.warn("Unable to convert {} to long type.", value); return DEFAULT_VALUE_FOR_LONG; } } @@ -118,7 +118,7 @@ long asLong() { try { return Double.parseDouble(this.value); } catch (NumberFormatException e) { - logger.warn("Unable to convert %s to double type.", value); + logger.warn("Unable to convert {} to double type.", value); return DEFAULT_VALUE_FOR_DOUBLE; } } @@ -133,4 +133,3 @@ ValueSource getSource() { return source; } } - diff --git a/src/main/java/com/google/firebase/remoteconfig/internal/ServerTemplateResponse.java b/src/main/java/com/google/firebase/remoteconfig/internal/ServerTemplateResponse.java index 87f624a7..c58faa94 100644 --- a/src/main/java/com/google/firebase/remoteconfig/internal/ServerTemplateResponse.java +++ b/src/main/java/com/google/firebase/remoteconfig/internal/ServerTemplateResponse.java @@ -25,14 +25,15 @@ import java.util.Map; /** - * The Data Transfer Object for parsing Remote Config template responses from the Remote Config + * The Data Transfer Object for parsing Remote Config template responses from + * the Remote Config * service. */ public final class ServerTemplateResponse { @Key("parameters") private Map parameters; - @Key("conditions") + @Key("serverConditions") private List serverConditions; @Key("parameterGroups") @@ -94,7 +95,8 @@ public ServerTemplateResponse setEtag(String etag) { } /** - * The Data Transfer Object for parsing Remote Config condition responses from the Remote Config + * The Data Transfer Object for parsing Remote Config condition responses from + * the Remote Config * service. */ public static final class ServerConditionResponse { @@ -102,7 +104,7 @@ public static final class ServerConditionResponse { @Key("name") private String name; - @Key("condition") + @Key("serverCondition") private OneOfConditionResponse condition; public String getName() { @@ -164,7 +166,7 @@ public OneOfConditionResponse setAndCondition(AndConditionResponse andCondition) } public OneOfConditionResponse setCustomSignalCondition( - CustomSignalConditionResponse customSignalCondition) { + CustomSignalConditionResponse customSignalCondition) { this.customSignalCondition = customSignalCondition; return this; } @@ -248,7 +250,7 @@ public static final class PercentConditionResponse { @Key("microPercentRange") private MicroPercentRangeResponse microPercentRange; - @Key("percentOperator") + @Key("percentConditionOperator") private String percentOperator; @Key("seed") @@ -276,7 +278,7 @@ public PercentConditionResponse setMicroPercent(int microPercent) { } public PercentConditionResponse setMicroPercentRange( - MicroPercentRangeResponse microPercentRange) { + MicroPercentRangeResponse microPercentRange) { this.microPercentRange = microPercentRange; return this; } @@ -318,4 +320,3 @@ public MicroPercentRangeResponse setMicroPercentUpperBound(int microPercentUpper } } } - diff --git a/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java index d820dda7..78a5c276 100644 --- a/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java +++ b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigClientImplTest.java @@ -68,36 +68,37 @@ public class FirebaseRemoteConfigClientImplTest { private static final List HTTP_STATUS_CODES = ImmutableList.of(401, 404, 500); - private static final Map HTTP_STATUS_TO_ERROR_CODE = ImmutableMap.of( + private static final Map HTTP_STATUS_TO_ERROR_CODE = + ImmutableMap.of( 401, ErrorCode.UNAUTHENTICATED, 404, ErrorCode.NOT_FOUND, 500, ErrorCode.INTERNAL); - private static final String MOCK_TEMPLATE_RESPONSE = TestUtils - .loadResource("getRemoteConfig.json"); - - private static final String MOCK_SERVER_TEMPLATE_RESPONSE = TestUtils - .loadResource("getServerRemoteConfig.json"); + private static final String MOCK_TEMPLATE_RESPONSE = + TestUtils.loadResource("getRemoteConfig.json"); + + private static final String MOCK_SERVER_TEMPLATE_RESPONSE = + TestUtils.loadResource("getServerRemoteConfig.json"); - private static final String MOCK_LIST_VERSIONS_RESPONSE = TestUtils - .loadResource("listRemoteConfigVersions.json"); + private static final String MOCK_LIST_VERSIONS_RESPONSE = + TestUtils.loadResource("listRemoteConfigVersions.json"); private static final String TEST_ETAG = "etag-123456789012-1"; private static final Map EXPECTED_PARAMETERS = ImmutableMap.of( "welcome_message_text", - new Parameter() - .setDefaultValue(ParameterValue.of("welcome to app")) - .setConditionalValues( - ImmutableMap.of( - "ios_en", ParameterValue.of("welcome to app en"))) - .setDescription("text for welcome message!") - .setValueType(ParameterValueType.STRING), + new Parameter() + .setDefaultValue(ParameterValue.of("welcome to app")) + .setConditionalValues( + ImmutableMap.of( + "ios_en", ParameterValue.of("welcome to app en"))) + .setDescription("text for welcome message!") + .setValueType(ParameterValueType.STRING), "header_text", - new Parameter() - .setDefaultValue(ParameterValue.inAppDefault()) - .setValueType(ParameterValueType.STRING)); + new Parameter() + .setDefaultValue(ParameterValue.inAppDefault()) + .setValueType(ParameterValueType.STRING)); private static final Map EXPECTED_PARAMETER_GROUPS = ImmutableMap.of( @@ -167,28 +168,27 @@ public class FirebaseRemoteConfigClientImplTest { ImmutableList.of( new OneOfCondition() .setCustomSignal( - new CustomSignalCondition( - "users", - CustomSignalOperator - .NUMERIC_LESS_THAN, - new ArrayList<>( - ImmutableList.of("100")))), + new CustomSignalCondition( + "users", + CustomSignalOperator + .NUMERIC_LESS_THAN, + new ArrayList<>( + ImmutableList.of("100")))), new OneOfCondition() .setCustomSignal( - new CustomSignalCondition( - "premium users", - CustomSignalOperator - .NUMERIC_GREATER_THAN, - new ArrayList<>( - ImmutableList.of("20")))), + new CustomSignalCondition( + "premium users", + CustomSignalOperator + .NUMERIC_GREATER_THAN, + new ArrayList<>( + ImmutableList.of("20")))), new OneOfCondition() - .setPercent( - new PercentCondition( - new MicroPercentRange( - 25000000, 100000000), - PercentConditionOperator.BETWEEN, - "cla24qoibb61")) - )))))))); + .setPercent( + new PercentCondition( + new MicroPercentRange( + 25000000, 100000000), + PercentConditionOperator.BETWEEN, + "cla24qoibb61")))))))))); private static final Version EXPECTED_VERSION = new Version( @@ -394,15 +394,17 @@ public void testGetTemplateErrorWithMalformedResponse() { @Test public void testGetTemplateErrorWithDetails() { for (int code : HTTP_STATUS_CODES) { - response.setStatusCode(code).setContent( + response + .setStatusCode(code) + .setContent( "{\"error\": {\"status\": \"INVALID_ARGUMENT\", \"message\": \"test error\"}}"); try { client.getTemplate(); fail("No error thrown for HTTP error"); } catch (FirebaseRemoteConfigException error) { - checkExceptionFromHttpResponse(error, ErrorCode.INVALID_ARGUMENT, null, "test error", - HttpMethods.GET); + checkExceptionFromHttpResponse( + error, ErrorCode.INVALID_ARGUMENT, null, "test error", HttpMethods.GET); } checkGetRequestHeader(interceptor.getLastRequest()); } @@ -411,17 +413,22 @@ public void testGetTemplateErrorWithDetails() { @Test public void testGetTemplateErrorWithRcError() { for (int code : HTTP_STATUS_CODES) { - response.setStatusCode(code).setContent( + response + .setStatusCode(code) + .setContent( "{\"error\": {\"status\": \"INVALID_ARGUMENT\", " - + "\"message\": \"[INVALID_ARGUMENT]: test error\"}}"); + + "\"message\": \"[INVALID_ARGUMENT]: test error\"}}"); try { client.getTemplate(); fail("No error thrown for HTTP error"); } catch (FirebaseRemoteConfigException error) { - checkExceptionFromHttpResponse(error, ErrorCode.INVALID_ARGUMENT, - RemoteConfigErrorCode.INVALID_ARGUMENT, "[INVALID_ARGUMENT]: test error", - HttpMethods.GET); + checkExceptionFromHttpResponse( + error, + ErrorCode.INVALID_ARGUMENT, + RemoteConfigErrorCode.INVALID_ARGUMENT, + "[INVALID_ARGUMENT]: test error", + HttpMethods.GET); } checkGetRequestHeader(interceptor.getLastRequest()); } @@ -680,7 +687,8 @@ public void testPublishTemplateWithValidTemplateAndValidateOnlyTrue() throws Exc Template validatedTemplate = client.publishTemplate(expectedTemplate, true, false); - // check if the etag matches the input template's etag and not the etag from the server response + // check if the etag matches the input template's etag and not the etag from the + // server response assertNotEquals(TEST_ETAG, validatedTemplate.getETag()); assertEquals("etag-123456789012-45", validatedTemplate.getETag()); assertEquals(expectedTemplate, validatedTemplate); @@ -1470,7 +1478,6 @@ public void testGetServerTemplateWithTimestampUpToNanosecondPrecision() throws E String receivedTemplate = client.getServerTemplate(); ServerTemplateData serverTemplateData = ServerTemplateData.fromJSON(receivedTemplate); - assertEquals(TEST_ETAG, serverTemplateData.getETag()); assertEquals("17", serverTemplateData.getVersion().getVersionNumber()); assertEquals(1605423446000L, serverTemplateData.getVersion().getUpdateTime()); diff --git a/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java index f34035f5..8b416b67 100644 --- a/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java +++ b/src/test/java/com/google/firebase/remoteconfig/FirebaseRemoteConfigTest.java @@ -28,11 +28,14 @@ import com.google.firebase.TestOnlyImplFirebaseTrampolines; import com.google.firebase.auth.MockGoogleCredentials; import com.google.firebase.remoteconfig.internal.TemplateResponse; +import com.google.gson.JsonElement; +import com.google.gson.JsonParser; import java.util.concurrent.ExecutionException; import org.junit.After; import org.junit.Test; -/** Unit tests + +/** Tests * for {@link FirebaseRemoteConfig}. * */ public class FirebaseRemoteConfigTest { @@ -42,13 +45,6 @@ public class FirebaseRemoteConfigTest { .setCredentials(new MockGoogleCredentials("test-token")) .setProjectId("test-project") .build(); - private static final String TEST_SERVER_TEMPLATE = - "{\n" - + " \"etag\": \"etag-123456789012-1\",\n" - + " \"parameters\": {},\n" - + " \"serverConditions\": [],\n" - + " \"parameterGroups\": {}\n" - + "}"; private static final FirebaseRemoteConfigException TEST_EXCEPTION = new FirebaseRemoteConfigException(ErrorCode.INTERNAL, "Test error message"); @@ -636,7 +632,11 @@ public void testGetServerTemplate() throws FirebaseRemoteConfigException { ServerTemplate template = remoteConfig.getServerTemplate(); String templateData = template.toJson(); - assertEquals(TEST_SERVER_TEMPLATE, templateData); + JsonElement expectedJson = + JsonParser.parseString(new ServerTemplateData().setETag(TEST_ETAG).toJSON()); + JsonElement actualJson = JsonParser.parseString(templateData); + + assertEquals(expectedJson, actualJson); } @Test @@ -660,7 +660,11 @@ public void testGetServerTemplateAsync() throws Exception { ServerTemplate template = remoteConfig.getServerTemplateAsync().get(); String templateData = template.toJson(); - assertEquals(TEST_SERVER_TEMPLATE, templateData); + JsonElement expectedJson = + JsonParser.parseString(new ServerTemplateData().setETag(TEST_ETAG).toJSON()); + JsonElement actualJson = JsonParser.parseString(templateData); + + assertEquals(expectedJson, actualJson); } @Test diff --git a/src/test/java/com/google/firebase/remoteconfig/ServerTemplateImplTest.java b/src/test/java/com/google/firebase/remoteconfig/ServerTemplateImplTest.java index ae70d236..bafe8433 100644 --- a/src/test/java/com/google/firebase/remoteconfig/ServerTemplateImplTest.java +++ b/src/test/java/com/google/firebase/remoteconfig/ServerTemplateImplTest.java @@ -24,12 +24,14 @@ import com.google.firebase.FirebaseOptions; import com.google.firebase.auth.MockGoogleCredentials; import com.google.firebase.testing.TestUtils; +import com.google.gson.JsonElement; +import com.google.gson.JsonParser; import org.junit.BeforeClass; import org.junit.Test; -/** -* Tests for {@link ServerTemplateImpl}. -*/ +/** Tests +* for {@link ServerTemplateImpl}. +* */ public class ServerTemplateImplTest { private static final FirebaseOptions TEST_OPTIONS = @@ -37,13 +39,6 @@ public class ServerTemplateImplTest { .setCredentials(new MockGoogleCredentials("test-token")) .setProjectId("test-project") .build(); - private static final String TEST_SERVER_TEMPLATE = - "{\n" - + " \"etag\": \"etag-123456789012-1\",\n" - + " \"parameters\": {},\n" - + " \"serverConditions\": [],\n" - + " \"parameterGroups\": {}\n" - + "}"; private static String cacheTemplate; @@ -57,12 +52,28 @@ public void testServerTemplateWithoutCacheValueThrowsException() throws FirebaseRemoteConfigException { KeysAndValues defaultConfig = new KeysAndValues.Builder().build(); - IllegalArgumentException error = assertThrows(IllegalArgumentException.class, - () -> new ServerTemplateImpl.Builder(null).defaultConfig(defaultConfig).build()); + IllegalArgumentException error = + assertThrows( + IllegalArgumentException.class, + () -> new ServerTemplateImpl.Builder(null).defaultConfig(defaultConfig).build()); assertEquals("JSON String must not be null or empty.", error.getMessage()); } + @Test + public void testEvaluateWithoutContextReturnsDefaultValue() throws FirebaseRemoteConfigException { + KeysAndValues defaultConfig = new KeysAndValues.Builder().build(); + ServerTemplate template = + new ServerTemplateImpl.Builder(null) + .defaultConfig(defaultConfig) + .cachedTemplate(cacheTemplate) + .build(); + + ServerConfig evaluatedConfig = template.evaluate(); + + assertEquals("Default value", evaluatedConfig.getString("Custom")); + } + @Test public void testEvaluateCustomSignalReturnsDefaultValue() throws FirebaseRemoteConfigException { KeysAndValues defaultConfig = new KeysAndValues.Builder().build(); @@ -185,11 +196,12 @@ public void testEvaluateWithInvalidCacheValueThrowsException() .cachedTemplate(invalidJsonString) .build(); - FirebaseRemoteConfigException error = assertThrows(FirebaseRemoteConfigException.class, - () -> template.evaluate(context)); + FirebaseRemoteConfigException error = + assertThrows(FirebaseRemoteConfigException.class, () -> template.evaluate(context)); - assertEquals("No Remote Config Server template in cache. Call load() before " - + "calling evaluate().", error.getMessage()); + assertEquals( + "No Remote Config Server template in cache. Call load() before " + "calling evaluate().", + error.getMessage()); } @Test @@ -240,10 +252,12 @@ public void testEvaluateWithMultipleConditionReturnsConditionalValue() throws Ex @Test public void testEvaluateWithChainedAndConditionReturnsDefaultValue() throws Exception { KeysAndValues defaultConfig = new KeysAndValues.Builder().build(); - KeysAndValues context = new KeysAndValues.Builder().put("users", "100") - .put("premium users", 20) - .put("randomizationId", "user") - .build(); + KeysAndValues context = + new KeysAndValues.Builder() + .put("users", "100") + .put("premium users", 20) + .put("randomizationId", "user") + .build(); ServerTemplate template = new ServerTemplateImpl.Builder(null) .defaultConfig(defaultConfig) @@ -341,25 +355,67 @@ private FirebaseRemoteConfig getRemoteConfig(FirebaseRemoteConfigClient client) @Test public void testLoad() throws Exception { - KeysAndValues defaultConfig = - new KeysAndValues.Builder().put("Unset default config", "abc").build(); + // 1. Define the template data that the mock client will return. + // This is the EXPECTED state after `load()` is called. + final String expectedTemplateJsonAfterLoad = + new ServerTemplateData().setETag(TEST_ETAG).toJSON(); - // Mock the HTTP client to return a predefined response + // 2. Mock the HTTP client to return the predefined response. MockRemoteConfigClient client = - MockRemoteConfigClient.fromServerTemplate( - new ServerTemplateData().setETag(TEST_ETAG).toJSON()); + MockRemoteConfigClient.fromServerTemplate(expectedTemplateJsonAfterLoad); FirebaseRemoteConfig remoteConfig = getRemoteConfig(client); - ServerTemplate template1 = + + // 3. Build the template instance. + // It's initialized with a complex `cacheTemplate` to ensure `load()` properly + // overwrites it. + KeysAndValues defaultConfig = + new KeysAndValues.Builder().put("Unset default config", "abc").build(); + ServerTemplate template = remoteConfig .serverTemplateBuilder() .defaultConfig(defaultConfig) - .cachedTemplate(cacheTemplate) + .cachedTemplate(cacheTemplate) // This is the initial state before `load()` .build(); - // Call the load method - ApiFuture loadFuture = template1.load(); - loadFuture.get(); - String cachedTemplate = template1.toJson(); - assertEquals(TEST_SERVER_TEMPLATE, cachedTemplate); + // 4. Call the load method, which fetches the new template from the mock client. + ApiFuture loadFuture = template.load(); + loadFuture.get(); // Wait for the async operation to complete. + + // 5. Get the ACTUAL state of the template after `load()` has executed. + String actualJsonAfterLoad = template.toJson(); + + // 6. Assert that the template's state has been updated to match what the mock + // client returned. + // Parsing to JsonElement performs a deep, order-insensitive comparison. + JsonElement expectedJson = JsonParser.parseString(expectedTemplateJsonAfterLoad); + JsonElement actualJson = JsonParser.parseString(actualJsonAfterLoad); + + assertEquals(expectedJson, actualJson); + } + + @Test + public void testBuilderParsesCachedTemplateCorrectly() throws FirebaseRemoteConfigException { + // Arrange: + // 1. Create a canonical JSON string by parsing the input file and then + // re-serializing it. This gives us the precise expected output format, + // accounting for any formatting or default value differences. + ServerTemplateData canonicalData = ServerTemplateData.fromJSON(cacheTemplate); + String expectedJsonString = canonicalData.toJSON(); + + // Act: + // 2. Build a ServerTemplate instance from the original cached JSON string, + // which triggers the parsing logic we want to test. + ServerTemplate template = + new ServerTemplateImpl.Builder(null).cachedTemplate(cacheTemplate).build(); + + // Assert: + // 3. Compare the JSON from the newly built template against the canonical + // version. + // This verifies that the internal state was parsed and stored correctly. + // Using JsonElement ensures the comparison is not affected by key order. + JsonElement expectedJsonTree = JsonParser.parseString(expectedJsonString); + JsonElement actualJsonTree = JsonParser.parseString(template.toJson()); + + assertEquals(expectedJsonTree, actualJsonTree); } } diff --git a/src/test/resources/getServerRemoteConfig.json b/src/test/resources/getServerRemoteConfig.json index 2e3c3556..19967e95 100644 --- a/src/test/resources/getServerRemoteConfig.json +++ b/src/test/resources/getServerRemoteConfig.json @@ -1,8 +1,8 @@ { - "conditions": [ + "serverConditions": [ { "name": "custom_signal", - "condition": { + "serverCondition": { "orCondition": { "conditions": [ { @@ -26,7 +26,7 @@ }, { "name": "percent", - "condition": { + "serverCondition": { "orCondition": { "conditions": [ { @@ -34,7 +34,7 @@ "conditions": [ { "percent": { - "percentOperator": "BETWEEN", + "percentConditionOperator": "BETWEEN", "seed": "3maarirs9xzs", "microPercentRange": { "microPercentLowerBound": 12000000, @@ -51,7 +51,7 @@ }, { "name": "chained_conditions", - "condition": { + "serverCondition": { "orCondition": { "conditions": [ { @@ -77,7 +77,7 @@ }, { "percent": { - "percentOperator": "BETWEEN", + "percentConditionOperator": "BETWEEN", "seed": "cla24qoibb61", "microPercentRange": { "microPercentLowerBound": 25000000, @@ -139,4 +139,4 @@ "updateTime": "2020-11-15T06:57:26.342763941Z", "description": "promo config" } -} +} \ No newline at end of file diff --git a/src/test/resources/getServerTemplateData.json b/src/test/resources/getServerTemplateData.json index 27e3507d..91e3971e 100644 --- a/src/test/resources/getServerTemplateData.json +++ b/src/test/resources/getServerTemplateData.json @@ -1,8 +1,8 @@ { - "conditions": [ + "serverConditions": [ { "name": "custom_signal", - "condition": { + "serverCondition": { "orCondition": { "conditions": [ { @@ -26,7 +26,7 @@ }, { "name": "percent", - "condition": { + "serverCondition": { "orCondition": { "conditions": [ { @@ -34,7 +34,7 @@ "conditions": [ { "percent": { - "percentOperator": "BETWEEN", + "percentConditionOperator": "BETWEEN", "seed": "3maarirs9xzs", "microPercentRange": { "microPercentLowerBound": 12000000, @@ -51,7 +51,7 @@ }, { "name": "chained_conditions", - "condition": { + "serverCondition": { "orCondition": { "conditions": [ { @@ -75,7 +75,7 @@ ] }, "percent": { - "percentOperator": "BETWEEN", + "percentConditionOperator": "BETWEEN", "seed": "cla24qoibb61", "microPercentRange": { "microPercentLowerBound": 25000000, @@ -132,7 +132,7 @@ } } }, - "Derived in-app default": { + "Derived in-app default": { "defaultValue": { "value": "Default value" }, @@ -167,7 +167,15 @@ } }, "version": { - "versionNumber": "27", - "isLegacy": true + "versionNumber": "17", + "updateOrigin": "ADMIN_SDK_NODE", + "updateType": "INCREMENTAL_UPDATE", + "updateUser": { + "email": "firebase-user@account.com", + "name": "dev-admin", + "imageUrl": "http://image.jpg" + }, + "updateTime": "2020-11-15T06:57:26.342763941Z", + "description": "promo config" } -} +} \ No newline at end of file