diff --git a/firebase-config/CHANGELOG.md b/firebase-config/CHANGELOG.md index 79122fb5556..5165a5c7a42 100644 --- a/firebase-config/CHANGELOG.md +++ b/firebase-config/CHANGELOG.md @@ -1,5 +1,7 @@ # Unreleased +- [fixed] Realtime support for A/B test updates + # 23.0.1 - [changed] Bumped internal dependencies. diff --git a/firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigConstants.java b/firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigConstants.java index 410306afd9c..66e8bf61d52 100644 --- a/firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigConstants.java +++ b/firebase-config/src/main/java/com/google/firebase/remoteconfig/RemoteConfigConstants.java @@ -97,12 +97,14 @@ public final class RemoteConfigConstants { */ @StringDef({ ExperimentDescriptionFieldKey.EXPERIMENT_ID, - ExperimentDescriptionFieldKey.VARIANT_ID + ExperimentDescriptionFieldKey.VARIANT_ID, + ExperimentDescriptionFieldKey.AFFECTED_PARAMETER_KEYS }) @Retention(RetentionPolicy.SOURCE) public @interface ExperimentDescriptionFieldKey { String EXPERIMENT_ID = "experimentId"; String VARIANT_ID = "variantId"; + String AFFECTED_PARAMETER_KEYS = "affectedParameterKeys"; } private RemoteConfigConstants() {} diff --git a/firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigContainer.java b/firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigContainer.java index 161139339bc..32420de9998 100644 --- a/firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigContainer.java +++ b/firebase-config/src/main/java/com/google/firebase/remoteconfig/internal/ConfigContainer.java @@ -14,6 +14,9 @@ package com.google.firebase.remoteconfig.internal; +import static com.google.firebase.remoteconfig.RemoteConfigConstants.ExperimentDescriptionFieldKey.AFFECTED_PARAMETER_KEYS; +import static com.google.firebase.remoteconfig.RemoteConfigConstants.ExperimentDescriptionFieldKey.EXPERIMENT_ID; + import com.google.errorprone.annotations.CanIgnoreReturnValue; import java.util.Date; import java.util.HashMap; @@ -37,6 +40,7 @@ public class ConfigContainer { static final String ABT_EXPERIMENTS_KEY = "abt_experiments_key"; static final String PERSONALIZATION_METADATA_KEY = "personalization_metadata_key"; static final String TEMPLATE_VERSION_NUMBER_KEY = "template_version_number_key"; + static final String ROLLOUT_ID_PREFIX = "rollout"; static final String ROLLOUT_METADATA_KEY = "rollout_metadata_key"; public static final String ROLLOUT_METADATA_AFFECTED_KEYS = "affectedParameterKeys"; public static final String ROLLOUT_METADATA_ID = "rolloutId"; @@ -220,6 +224,31 @@ private Map> createRolloutParameterKeyMap() throws J return rolloutMetadataMap; } + /** Creates a map where the key is the config key and the value is the experiment description. */ + private Map createExperimentsMap() throws JSONException { + Map experimentsMap = new HashMap<>(); + JSONArray abtExperiments = this.getAbtExperiments(); + // Iterate through all experiments to check if it has the `affectedParameterKeys` field. + for (int i = 0; i < abtExperiments.length(); i++) { + JSONObject experiment = abtExperiments.getJSONObject(i); + if (!experiment.has(AFFECTED_PARAMETER_KEYS) + || experiment.getString(EXPERIMENT_ID).startsWith(ROLLOUT_ID_PREFIX)) { + continue; + } + + // Since a config key can only have one experiment associated with it, map the key to the + // experiment. + JSONArray affectedKeys = experiment.getJSONArray(AFFECTED_PARAMETER_KEYS); + for (int j = 0; j < affectedKeys.length(); j++) { + String key = affectedKeys.getString(j); + JSONObject experimentsCopy = new JSONObject(experiment.toString()); + experimentsMap.put(key, experimentsCopy); + } + } + + return experimentsMap; + } + /** * @param other The other {@link ConfigContainer} against which to compute the diff * @return The set of config keys that have changed between the this config and {@code other} @@ -233,6 +262,10 @@ public Set getChangedParams(ConfigContainer other) throws JSONException Map> rolloutMetadataMap = this.createRolloutParameterKeyMap(); Map> otherRolloutMetadataMap = other.createRolloutParameterKeyMap(); + // Config key to experiments map. + Map experimentsMap = this.createExperimentsMap(); + Map otherExperimentsMap = other.createExperimentsMap(); + Set changed = new HashSet<>(); Iterator keys = this.getConfigs().keys(); while (keys.hasNext()) { @@ -284,6 +317,20 @@ public Set getChangedParams(ConfigContainer other) throws JSONException continue; } + // If one and only one of the experiments map contains the key, add it to changed. + if (experimentsMap.containsKey(key) != otherExperimentsMap.containsKey(key)) { + changed.add(key); + continue; + } + + // If both experiment maps contains the key, compare the experiments to see if it's different. + if (otherExperimentsMap.containsKey(key) + && experimentsMap.containsKey(key) + && !otherExperimentsMap.get(key).toString().equals(experimentsMap.get(key).toString())) { + changed.add(key); + continue; + } + // Since the key is the same in both configs, remove it from otherConfig otherConfig.remove(key); } diff --git a/firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/ConfigContainerTest.java b/firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/ConfigContainerTest.java index 0dd1e399c35..8483a055e10 100644 --- a/firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/ConfigContainerTest.java +++ b/firebase-config/src/test/java/com/google/firebase/remoteconfig/internal/ConfigContainerTest.java @@ -15,6 +15,7 @@ package com.google.firebase.remoteconfig.internal; import static com.google.common.truth.Truth.assertThat; +import static com.google.firebase.remoteconfig.RemoteConfigConstants.ExperimentDescriptionFieldKey.AFFECTED_PARAMETER_KEYS; import static com.google.firebase.remoteconfig.RemoteConfigConstants.ExperimentDescriptionFieldKey.EXPERIMENT_ID; import static com.google.firebase.remoteconfig.RemoteConfigConstants.ExperimentDescriptionFieldKey.VARIANT_ID; import static com.google.firebase.remoteconfig.internal.Personalization.ARM_INDEX; @@ -138,16 +139,20 @@ public void getChangedParams_sameP13nMetadata_returnsEmptySet() throws Exception } @Test - public void getChangedParams_changedExperimentsMetadata_returnsNoParamKeys() throws Exception { + public void getChangedParams_sameExperimentsMetadata_returnsEmptySet() throws Exception { + JSONArray activeExperiments = generateAbtExperiments(1); + JSONArray fetchedExperiments = generateAbtExperiments(1); + ConfigContainer config = ConfigContainer.newBuilder() - .replaceConfigsWith(ImmutableMap.of("string_param", "value_1")) + .replaceConfigsWith(ImmutableMap.of("abt_test_key_1", "value_1")) + .withAbtExperiments(activeExperiments) .build(); ConfigContainer other = ConfigContainer.newBuilder() - .replaceConfigsWith(ImmutableMap.of("string_param", "value_1")) - .withAbtExperiments(generateAbtExperiments(1)) + .replaceConfigsWith(ImmutableMap.of("abt_test_key_1", "value_1")) + .withAbtExperiments(fetchedExperiments) .build(); Set changedParams = config.getChangedParams(other); @@ -155,6 +160,81 @@ public void getChangedParams_changedExperimentsMetadata_returnsNoParamKeys() thr assertThat(changedParams).isEmpty(); } + @Test + public void getChangedParams_changedExperimentsMetadata_returnsUpdatedKey() throws Exception { + JSONArray activeExperiments = generateAbtExperiments(1); + JSONArray fetchedExperiments = generateAbtExperiments(1); + + activeExperiments.getJSONObject(0).put(VARIANT_ID, "32"); + + ConfigContainer config = + ConfigContainer.newBuilder() + .replaceConfigsWith(ImmutableMap.of("abt_test_key_1", "value_1")) + .withAbtExperiments(activeExperiments) + .build(); + + ConfigContainer other = + ConfigContainer.newBuilder() + .replaceConfigsWith(ImmutableMap.of("abt_test_key_1", "value_1")) + .withAbtExperiments(fetchedExperiments) + .build(); + + Set changedParams = config.getChangedParams(other); + + assertThat(changedParams).containsExactly("abt_test_key_1"); + } + + @Test + public void getChangedParams_deletedExperiment_returnsUpdatedKey() throws Exception { + JSONArray activeExperiments = generateAbtExperiments(1); + JSONArray fetchedExperiments = new JSONArray(); + + ConfigContainer config = + ConfigContainer.newBuilder() + .replaceConfigsWith(ImmutableMap.of("abt_test_key_1", "value_1")) + .withAbtExperiments(activeExperiments) + .build(); + + ConfigContainer other = + ConfigContainer.newBuilder() + .replaceConfigsWith(ImmutableMap.of("abt_test_key_1", "value_1")) + .withAbtExperiments(fetchedExperiments) + .build(); + + Set changedParams = config.getChangedParams(other); + + assertThat(changedParams).containsExactly("abt_test_key_1"); + } + + @Test + public void getChangedParams_changedExperimentsKeys_returnsUpdatedKey() throws Exception { + JSONArray activeExperiments = generateAbtExperiments(1); + JSONArray fetchedExperiments = generateAbtExperiments(1); + + fetchedExperiments + .getJSONObject(0) + .getJSONArray(AFFECTED_PARAMETER_KEYS) + .put(0, "abt_test_key_2"); + + ConfigContainer config = + ConfigContainer.newBuilder() + .replaceConfigsWith( + ImmutableMap.of("abt_test_key_1", "value_1", "abt_test_key_2", "value_2")) + .withAbtExperiments(activeExperiments) + .build(); + + ConfigContainer other = + ConfigContainer.newBuilder() + .replaceConfigsWith( + ImmutableMap.of("abt_test_key_1", "value_1", "abt_test_key_2", "value_2")) + .withAbtExperiments(fetchedExperiments) + .build(); + + Set changedParams = config.getChangedParams(other); + + assertThat(changedParams).containsExactly("abt_test_key_1", "abt_test_key_2"); + } + @Test public void getChangedParams_noChanges_returnsEmptySet() throws Exception { ConfigContainer config = @@ -452,9 +532,14 @@ public void getChangedParams_unchangedRolloutMetadata_returnsNoKey() throws Exce private static JSONArray generateAbtExperiments(int numExperiments) throws JSONException { JSONArray experiments = new JSONArray(); + JSONArray experimentKeys = new JSONArray(); + experimentKeys.put("abt_test_key_1"); for (int experimentNum = 1; experimentNum <= numExperiments; experimentNum++) { experiments.put( - new JSONObject().put(EXPERIMENT_ID, "exp" + experimentNum).put(VARIANT_ID, "var1")); + new JSONObject() + .put(EXPERIMENT_ID, "exp_" + experimentNum) + .put(VARIANT_ID, "var1") + .put(AFFECTED_PARAMETER_KEYS, experimentKeys)); } return experiments; }