Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
126 changes: 67 additions & 59 deletions src/main/java/com/google/firebase/remoteconfig/ParameterValue.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,12 @@
import com.google.firebase.remoteconfig.internal.TemplateResponse.ParameterValueResponse;
import com.google.firebase.remoteconfig.internal.TemplateResponse.PersonalizationValueResponse;
import com.google.firebase.remoteconfig.internal.TemplateResponse.RolloutValueResponse;

import java.util.ArrayList;
import java.util.List;
import java.util.Objects;

/**
* Represents a Remote Config parameter value that can be used in a {@link Template}.
*/
/** Represents a Remote Config parameter value
* that can be used in a
* {@link Template}. */
public abstract class ParameterValue {

/**
Expand Down Expand Up @@ -84,15 +82,15 @@ public static PersonalizationValue ofPersonalization(String personalizationId) {
* @param variantValues The list of experiment variant values.
* @return A {@link ParameterValue.ExperimentValue} instance.
*/
public static ExperimentValue ofExperiment(String experimentId,
List<ExperimentVariantValue> variantValues) {
return new ExperimentValue(experimentId, variantValues);
public static ExperimentValue ofExperiment(
String experimentId, List<ExperimentVariantValue> variantValues, double exposurePercent) {
return new ExperimentValue(experimentId, variantValues, exposurePercent);
}

abstract ParameterValueResponse toParameterValueResponse();

static ParameterValue fromParameterValueResponse(
@NonNull ParameterValueResponse parameterValueResponse) {
@NonNull ParameterValueResponse parameterValueResponse) {
checkNotNull(parameterValueResponse);
if (parameterValueResponse.isUseInAppDefault()) {
return ParameterValue.inAppDefault();
Expand All @@ -102,7 +100,7 @@ static ParameterValue fromParameterValueResponse(
// Protobuf serialization does not set values for fields on the wire when
// they are equal to the default value for the field type. When deserializing,
// can appear as the value not being set. Explicitly handle default value for
// the percent field since 0 is a valid value.
// the percent field since 0 is a valid value.
double percent = 0;
if (rv.getPercent() != null) {
percent = rv.getPercent();
Expand All @@ -115,18 +113,21 @@ static ParameterValue fromParameterValueResponse(
}
if (parameterValueResponse.getExperimentValue() != null) {
ExperimentValueResponse ev = parameterValueResponse.getExperimentValue();
List<ExperimentVariantValue> variantValues = ev.getExperimentVariantValues().stream()
.map(evv -> new ExperimentVariantValue(
evv.getVariantId(), evv.getValue(), evv.getNoChange()))
.collect(toList());
return ParameterValue.ofExperiment(ev.getExperimentId(), variantValues);
List<ExperimentVariantValue> variantValues =
ev.getExperimentVariantValues().stream()
.map(
evv ->
new ExperimentVariantValue(
evv.getVariantId(), evv.getValue(), evv.getNoChange()))
.collect(toList());
return ParameterValue.ofExperiment(
ev.getExperimentId(), variantValues, ev.getExposurePercent());
}
return ParameterValue.of(parameterValueResponse.getValue());
}

/**
* Represents an explicit Remote Config parameter value with a value that the
* parameter is set to.
* Represents an explicit Remote Config parameter value with a value that the parameter is set to.
*/
public static final class Explicit extends ParameterValue {

Expand All @@ -147,8 +148,7 @@ public String getValue() {

@Override
ParameterValueResponse toParameterValueResponse() {
return new ParameterValueResponse()
.setValue(this.value);
return new ParameterValueResponse().setValue(this.value);
}

@Override
Expand All @@ -169,9 +169,7 @@ public int hashCode() {
}
}

/**
* Represents an in app default parameter value.
*/
/** Represents an in app default parameter value. */
public static final class InAppDefault extends ParameterValue {

@Override
Expand All @@ -191,9 +189,7 @@ public boolean equals(Object o) {
}
}

/**
* Represents a Rollout value.
*/
/** Represents a Rollout value. */
public static final class RolloutValue extends ParameterValue {
private final String rolloutId;
private final String value;
Expand Down Expand Up @@ -224,8 +220,8 @@ public String getValue() {
}

/**
* Gets the rollout percentage representing the exposure of rollout value
* in the target audience.
* Gets the rollout percentage representing the exposure of rollout value in the target
* audience.
*
* @return Percentage of audience exposed to the rollout
*/
Expand All @@ -235,11 +231,12 @@ public double getPercent() {

@Override
ParameterValueResponse toParameterValueResponse() {
return new ParameterValueResponse().setRolloutValue(
return new ParameterValueResponse()
.setRolloutValue(
new RolloutValueResponse()
.setRolloutId(this.rolloutId)
.setValue(this.value)
.setPercent(this.percent));
.setRolloutId(this.rolloutId)
.setValue(this.value)
.setPercent(this.percent));
}

@Override
Expand All @@ -252,8 +249,8 @@ public boolean equals(Object o) {
}
RolloutValue that = (RolloutValue) o;
return Double.compare(that.percent, percent) == 0
&& Objects.equals(rolloutId, that.rolloutId)
&& Objects.equals(value, that.value);
&& Objects.equals(rolloutId, that.rolloutId)
&& Objects.equals(value, that.value);
}

@Override
Expand All @@ -262,9 +259,7 @@ public int hashCode() {
}
}

/**
* Represents a Personalization value.
*/
/** Represents a Personalization value. */
public static final class PersonalizationValue extends ParameterValue {
private final String personalizationId;

Expand All @@ -283,9 +278,9 @@ public String getPersonalizationId() {

@Override
ParameterValueResponse toParameterValueResponse() {
return new ParameterValueResponse().setPersonalizationValue(
new PersonalizationValueResponse()
.setPersonalizationId(this.personalizationId));
return new ParameterValueResponse()
.setPersonalizationValue(
new PersonalizationValueResponse().setPersonalizationId(this.personalizationId));
}

@Override
Expand All @@ -306,9 +301,7 @@ public int hashCode() {
}
}

/**
* Represents a specific variant within an Experiment.
*/
/** Represents a specific variant within an Experiment. */
public static final class ExperimentVariantValue {
private final String variantId;
private final String value;
Expand Down Expand Up @@ -384,8 +377,8 @@ public boolean equals(Object o) {
}
ExperimentVariantValue that = (ExperimentVariantValue) o;
return noChange == that.noChange
&& Objects.equals(variantId, that.variantId)
&& Objects.equals(value, that.value);
&& Objects.equals(variantId, that.variantId)
&& Objects.equals(value, that.value);
}

@Override
Expand All @@ -394,16 +387,17 @@ public int hashCode() {
}
}

/**
* Represents an Experiment value.
*/
/** Represents an Experiment value. */
public static final class ExperimentValue extends ParameterValue {
private final String experimentId;
private final List<ExperimentVariantValue> variantValues;
private final double exposurePercent;

private ExperimentValue(String experimentId, List<ExperimentVariantValue> variantValues) {
private ExperimentValue(
String experimentId, List<ExperimentVariantValue> variantValues, double exposurePercent) {
this.experimentId = experimentId;
this.variantValues = variantValues;
this.exposurePercent = exposurePercent;
}

/**
Expand All @@ -415,6 +409,15 @@ public String getExperimentId() {
return experimentId;
}

/**
* Gets the exposure percentage of the experiment linked to this value.
*
* @return Exposure percentage of the experiment linked to this value.
*/
public double getExposurePercent() {
return exposurePercent;
}

/**
* Gets a collection of variant values served by the experiment.
*
Expand All @@ -426,16 +429,20 @@ public List<ExperimentVariantValue> getExperimentVariantValues() {

@Override
ParameterValueResponse toParameterValueResponse() {
List<ExperimentVariantValueResponse> variantValueResponses = variantValues.stream()
.map(variantValue -> new ExperimentVariantValueResponse()
.setVariantId(variantValue.getVariantId())
.setValue(variantValue.getValue())
.setNoChange(variantValue.getNoChange()))
.collect(toList());
return new ParameterValueResponse().setExperimentValue(
List<ExperimentVariantValueResponse> variantValueResponses =
variantValues.stream()
.map(
variantValue ->
new ExperimentVariantValueResponse()
.setVariantId(variantValue.getVariantId())
.setValue(variantValue.getValue())
.setNoChange(variantValue.getNoChange()))
.collect(toList());
return new ParameterValueResponse()
.setExperimentValue(
new ExperimentValueResponse()
.setExperimentId(this.experimentId)
.setExperimentVariantValues(variantValueResponses));
.setExperimentId(this.experimentId)
.setExperimentVariantValues(variantValueResponses));
}

@Override
Expand All @@ -448,12 +455,13 @@ public boolean equals(Object o) {
}
ExperimentValue that = (ExperimentValue) o;
return Objects.equals(experimentId, that.experimentId)
&& Objects.equals(variantValues, that.variantValues);
&& Objects.equals(variantValues, that.variantValues)
&& Double.compare(that.exposurePercent, exposurePercent) == 0;
}

@Override
public int hashCode() {
return Objects.hash(experimentId, variantValues);
return Objects.hash(experimentId, variantValues, exposurePercent);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@ public static final class ExperimentValueResponse {
@Key("variantValue")
private List<ExperimentVariantValueResponse> experimentVariantValues;

@Key("exposurePercent")
private Double exposurePercent;

public String getExperimentId() {
return experimentId;
}
Expand All @@ -296,6 +299,11 @@ public List<ExperimentVariantValueResponse> getExperimentVariantValues() {
return experimentVariantValues;
}

public Double getExposurePercent() {
return exposurePercent;
}


public ExperimentValueResponse setExperimentId(String experimentId) {
this.experimentId = experimentId;
return this;
Expand All @@ -306,6 +314,11 @@ public ExperimentValueResponse setExperimentVariantValues(
this.experimentVariantValues = experimentVariantValues;
return this;
}

public ExperimentValueResponse setExposurePercent(Double exposurePercent) {
this.exposurePercent = exposurePercent;
return this;
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertTrue;

import com.google.common.collect.ImmutableList;
import com.google.firebase.remoteconfig.ParameterValue.ExperimentVariantValue;
Expand Down Expand Up @@ -64,7 +63,11 @@ public void testCreateExperimentValue() {
ParameterValue.ofExperiment("experiment_1", ImmutableList.of(
ExperimentVariantValue.of("variant_1", "value_1"),
ExperimentVariantValue.ofNoChange("variant_2")
));
), 10.0);

assertEquals("experiment_1", parameterValue.getExperimentId());
assertEquals(2, parameterValue.getExperimentVariantValues().size());
assertEquals(10.0, parameterValue.getExposurePercent(), 0.0);

assertEquals("experiment_1", parameterValue.getExperimentId());
assertEquals(2, parameterValue.getExperimentVariantValues().size());
Expand All @@ -77,6 +80,8 @@ public void testCreateExperimentValue() {
assertEquals("variant_2", variant2.getVariantId());
assertEquals(null, variant2.getValue());
assertEquals(true, variant2.isNoChange());
assertEquals(10.0, parameterValue.getExposurePercent(), 0.0);

}

@Test
Expand Down Expand Up @@ -116,19 +121,19 @@ public void testEquality() {
ParameterValue.ExperimentValue experimentValueOne =
ParameterValue.ofExperiment("experiment_1", ImmutableList.of(
ExperimentVariantValue.of("variant_1", "value_1")
));
), 10.0);
ParameterValue.ExperimentValue experimentValueTwo =
ParameterValue.ofExperiment("experiment_1", ImmutableList.of(
ExperimentVariantValue.of("variant_1", "value_1")
));
), 10.0);
ParameterValue.ExperimentValue experimentValueThree =
ParameterValue.ofExperiment("experiment_2", ImmutableList.of(
ExperimentVariantValue.of("variant_1", "value_1")
));
), 10.0);
ParameterValue.ExperimentValue experimentValueFour =
ParameterValue.ofExperiment("experiment_1", ImmutableList.of(
ExperimentVariantValue.of("variant_2", "value_2")
));
), 10.0);
Comment on lines +124 to +136

Choose a reason for hiding this comment

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

high

The testEquality method for ExperimentValue is updated to include exposurePercent in the constructor calls, but the equals and hashCode methods in ParameterValue.ExperimentValue are not updated to consider exposurePercent. This means the equality checks in this test might not be fully accurate or could pass incorrectly if exposurePercent values differ. This is a high-severity correctness issue.


assertEquals(experimentValueOne, experimentValueTwo);
assertNotEquals(experimentValueOne, experimentValueThree);
Expand Down
Loading