Skip to content

[AI-FSSDK] (DO NOT REVIEW) [FSSDK-12275] Skip unsupported experiment type during flag decision #599

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
raju-opti wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[AI-FSSDK] (DO NOT REVIEW) [FSSDK-12275] Skip unsupported experiment type during flag decision #599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,17 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
for (String experimentId : featureFlag.getExperimentIds()) {
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);

// Skip experiments with unsupported types.
// If the experiment type is null (not set in datafile), we still evaluate it.
// If the experiment type is set but not in the supported list, we skip it.
if (experiment != null && experiment.getType() != null && !Experiment.SUPPORTED_TYPES.contains(experiment.getType())) {
String skipMessage = reasons.addInfo(
"Skipping experiment \"%s\" with unsupported type \"%s\" for feature \"%s\".",
experiment.getKey(), experiment.getType(), featureFlag.getKey());
logger.debug(skipMessage);
continue;
}

DecisionResponse<Variation> decisionVariation =
getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker, decisionPath);
reasons.merge(decisionVariation.getReasons());
Expand Down
43 changes: 37 additions & 6 deletions core-api/src/main/java/com/optimizely/ab/config/Experiment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,12 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;

/**
* Represents the Optimizely Experiment configuration.
Expand All @@ -36,8 +39,18 @@
@JsonIgnoreProperties(ignoreUnknown = true)
public class Experiment implements ExperimentCore {

/**
* The set of experiment types supported by this SDK.
* Experiments with a type not in this set will be skipped during flag decisions.
* If an experiment has no type (null), it is still evaluated.
*/
public static final Set<String> SUPPORTED_TYPES = Collections.unmodifiableSet(
new HashSet<>(Arrays.asList("a/b", "mab", "cmab", "feature_rollouts"))
);

private final String id;
private final String key;
private final String type;
private final String status;
private final String layerId;
private final String groupId;
Expand Down Expand Up @@ -72,30 +85,40 @@ public String toString() {

@VisibleForTesting
public Experiment(String id, String key, String layerId) {
this(id, key, null, layerId, Collections.emptyList(), null, Collections.emptyList(), Collections.emptyMap(), Collections.emptyList(), "", null);
this(id, key, null, null, layerId, Collections.emptyList(), null, Collections.emptyList(), Collections.emptyMap(), Collections.emptyList(), "", null);
}

@VisibleForTesting
public Experiment(String id, String key, String status, String layerId,
List<String> audienceIds, Condition audienceConditions,
List<Variation> variations, Map<String, String> userIdToVariationKeyMap,
List<TrafficAllocation> trafficAllocation, String groupId) {
this(id, key, status, layerId, audienceIds, audienceConditions, variations,
userIdToVariationKeyMap, trafficAllocation, groupId, null); // Default cmab=null
this(id, key, null, status, layerId, audienceIds, audienceConditions, variations,
userIdToVariationKeyMap, trafficAllocation, groupId, null); // Default type=null, cmab=null
}

@VisibleForTesting
public Experiment(String id, String key, String status, String layerId,
List<String> audienceIds, Condition audienceConditions,
List<Variation> variations, Map<String, String> userIdToVariationKeyMap,
List<TrafficAllocation> trafficAllocation) {
this(id, key, status, layerId, audienceIds, audienceConditions, variations,
userIdToVariationKeyMap, trafficAllocation, "", null); // Default groupId="" and cmab=null
this(id, key, null, status, layerId, audienceIds, audienceConditions, variations,
userIdToVariationKeyMap, trafficAllocation, "", null); // Default type=null, groupId="" and cmab=null
}

@VisibleForTesting
public Experiment(String id, String key, String status, String layerId,
List<String> audienceIds, Condition audienceConditions,
List<Variation> variations, Map<String, String> userIdToVariationKeyMap,
List<TrafficAllocation> trafficAllocation, Cmab cmab) {
this(id, key, null, status, layerId, audienceIds, audienceConditions, variations,
userIdToVariationKeyMap, trafficAllocation, "", cmab); // Default type=null, groupId=""
}

@JsonCreator
public Experiment(@JsonProperty("id") String id,
@JsonProperty("key") String key,
@JsonProperty("type") String type,
@JsonProperty("status") String status,
@JsonProperty("layerId") String layerId,
@JsonProperty("audienceIds") List<String> audienceIds,
Expand All @@ -104,11 +127,12 @@ public Experiment(@JsonProperty("id") String id,
@JsonProperty("forcedVariations") Map<String, String> userIdToVariationKeyMap,
@JsonProperty("trafficAllocation") List<TrafficAllocation> trafficAllocation,
@JsonProperty("cmab") Cmab cmab) {
this(id, key, status, layerId, audienceIds, audienceConditions, variations, userIdToVariationKeyMap, trafficAllocation, "", cmab);
this(id, key, type, status, layerId, audienceIds, audienceConditions, variations, userIdToVariationKeyMap, trafficAllocation, "", cmab);
}

public Experiment(@Nonnull String id,
@Nonnull String key,
@Nullable String type,
@Nullable String status,
@Nullable String layerId,
@Nonnull List<String> audienceIds,
Expand All @@ -120,6 +144,7 @@ public Experiment(@Nonnull String id,
@Nullable Cmab cmab) {
this.id = id;
this.key = key;
this.type = type;
this.status = status == null ? ExperimentStatus.NOT_STARTED.toString() : status;
this.layerId = layerId;
this.audienceIds = Collections.unmodifiableList(audienceIds);
Expand All @@ -141,6 +166,11 @@ public String getKey() {
return key;
}

@Nullable
public String getType() {
return type;
}

public String getStatus() {
return status;
}
Expand Down Expand Up @@ -203,6 +233,7 @@ public String toString() {
return "Experiment{" +
"id='" + id + '\'' +
", key='" + key + '\'' +
", type='" + type + '\'' +
", groupId='" + groupId + '\'' +
", status='" + status + '\'' +
", audienceIds=" + audienceIds +
Expand Down
1 change: 1 addition & 0 deletions core-api/src/main/java/com/optimizely/ab/config/Group.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public Group(@JsonProperty("id") String id,
experiment = new Experiment(
experiment.getId(),
experiment.getKey(),
experiment.getType(),
experiment.getStatus(),
experiment.getLayerId(),
experiment.getAudienceIds(),
Expand Down
4 changes: 3 additions & 1 deletion core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,8 @@ static Cmab parseCmab(JsonObject cmabJson, JsonDeserializationContext context) {
static Experiment parseExperiment(JsonObject experimentJson, String groupId, JsonDeserializationContext context) {
String id = experimentJson.get("id").getAsString();
String key = experimentJson.get("key").getAsString();
String type = experimentJson.has("type") && !experimentJson.get("type").isJsonNull()
? experimentJson.get("type").getAsString() : null;
JsonElement experimentStatusJson = experimentJson.get("status");
String status = experimentStatusJson.isJsonNull() ?
ExperimentStatus.NOT_STARTED.toString() : experimentStatusJson.getAsString();
Expand Down Expand Up @@ -168,7 +170,7 @@ static Experiment parseExperiment(JsonObject experimentJson, String groupId, Jso
}
}

return new Experiment(id, key, status, layerId, audienceIds, conditions, variations, userIdToVariationKeyMap,
return new Experiment(id, key, type, status, layerId, audienceIds, conditions, variations, userIdToVariationKeyMap,
trafficAllocations, groupId, cmab);
}

Expand Down
4 changes: 3 additions & 1 deletion core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,8 @@ private List<Experiment> parseExperiments(JSONArray experimentJson, String group
JSONObject experimentObject = (JSONObject) obj;
String id = experimentObject.getString("id");
String key = experimentObject.getString("key");
String type = experimentObject.has("type") && !experimentObject.isNull("type")
? experimentObject.getString("type") : null;
String status = experimentObject.isNull("status") ?
ExperimentStatus.NOT_STARTED.toString() : experimentObject.getString("status");
String layerId = experimentObject.has("layerId") ? experimentObject.getString("layerId") : null;
Expand Down Expand Up @@ -179,7 +181,7 @@ private List<Experiment> parseExperiments(JSONArray experimentJson, String group
cmab = parseCmab(cmabObject);
}

experiments.add(new Experiment(id, key, status, layerId, audienceIds, conditions, variations, userIdToVariationKeyMap,
experiments.add(new Experiment(id, key, type, status, layerId, audienceIds, conditions, variations, userIdToVariationKeyMap,
trafficAllocations, groupId, cmab));
}

Expand Down
4 changes: 3 additions & 1 deletion core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ private List<Experiment> parseExperiments(JSONArray experimentJson, String group
JSONObject experimentObject = (JSONObject) obj;
String id = (String) experimentObject.get("id");
String key = (String) experimentObject.get("key");
Object typeObj = experimentObject.get("type");
String type = typeObj != null ? (String) typeObj : null;
Object statusJson = experimentObject.get("status");
String status = statusJson == null ? ExperimentStatus.NOT_STARTED.toString() :
(String) experimentObject.get("status");
Expand Down Expand Up @@ -189,7 +191,7 @@ private List<Experiment> parseExperiments(JSONArray experimentJson, String group
}
}

experiments.add(new Experiment(id, key, status, layerId, audienceIds, conditions, variations,
experiments.add(new Experiment(id, key, type, status, layerId, audienceIds, conditions, variations,
userIdToVariationKeyMap, trafficAllocations, groupId, cmab));
}

Expand Down