Skip to content

[FSSDK-11448] Java Implementation: Add Experiment ID and Variation ID to Decision Notification #566

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.

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

Merged
6 changes: 6 additions & 0 deletions core-api/src/main/java/com/optimizely/ab/Optimizely.java
Original file line number Diff line number Diff line change
Expand Up @@ -1303,6 +1303,8 @@ private OptimizelyDecision createOptimizelyDecision(
ProjectConfig projectConfig
) {
String userId = user.getUserId();
String experimentId = null;
String variationId = null;

Boolean flagEnabled = false;
if (flagDecision.variation != null) {
Expand Down Expand Up @@ -1336,6 +1338,8 @@ private OptimizelyDecision createOptimizelyDecision(


Boolean decisionEventDispatched = false;
experimentId = flagDecision.experiment != null ? flagDecision.experiment.getId() : null;
variationId = flagDecision.variation != null ? flagDecision.variation.getId() : null;

Map<String, Object> attributes = user.getAttributes();
Map<String, ?> copiedAttributes = new HashMap<>(attributes);
Expand All @@ -1362,6 +1366,8 @@ private OptimizelyDecision createOptimizelyDecision(
.withRuleKey(ruleKey)
.withReasons(reasonsToReport)
.withDecisionEventDispatched(decisionEventDispatched)
.withExperimentId(experimentId)
.withVariationId(variationId)
.build();
notificationCenter.send(decisionNotification);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,9 +97,13 @@ public static ExperimentDecisionNotificationBuilder newExperimentDecisionNotific
public static class ExperimentDecisionNotificationBuilder {
public final static String EXPERIMENT_KEY = "experimentKey";
public final static String VARIATION_KEY = "variationKey";
public final static String EXPERIMENT_ID = "experimentId";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this class is responsible for activate api and other stuff that are failing in FSC. The createOptimizelyDecision method uses FlagDecisionNotificationBuilder. This class needs to stay unchanged.

public final static String VARIATION_ID = "variationId";

private String type;
private String experimentKey;
private String experimentId;
private String variationId;
Copy link
Contributor

Choose a reason for hiding this comment

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

these also are not needed

private Variation variation;
private String userId;
private Map<String, ?> attributes;
Expand Down Expand Up @@ -130,6 +134,16 @@ public ExperimentDecisionNotificationBuilder withVariation(Variation variation)
return this;
}

public ExperimentDecisionNotificationBuilder withExperimentId(String experimentId) {
this.experimentId = experimentId;
return this;
}

public ExperimentDecisionNotificationBuilder withVariationId(String variationId) {
this.variationId = variationId;
return this;
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Probably why FSC is getting null values as experiment_id and variation_id. Not needed.

public DecisionNotification build() {
if (type == null) {
throw new OptimizelyRuntimeException("type not set");
Expand All @@ -141,7 +155,9 @@ public DecisionNotification build() {

decisionInfo = new HashMap<>();
decisionInfo.put(EXPERIMENT_KEY, experimentKey);
decisionInfo.put(EXPERIMENT_ID, experimentId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed.

decisionInfo.put(VARIATION_KEY, variation != null ? variation.getKey() : null);
decisionInfo.put(VARIATION_ID, variationId);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not needed.


return new DecisionNotification(
type,
Expand Down Expand Up @@ -364,6 +380,8 @@ public static class FlagDecisionNotificationBuilder {
public final static String RULE_KEY = "ruleKey";
public final static String REASONS = "reasons";
public final static String DECISION_EVENT_DISPATCHED = "decisionEventDispatched";
public final static String EXPERIMENT_ID = "experimentId";
public final static String VARIATION_ID = "variationId";

private String flagKey;
private Boolean enabled;
Expand All @@ -374,6 +392,8 @@ public static class FlagDecisionNotificationBuilder {
private String ruleKey;
private List<String> reasons;
private Boolean decisionEventDispatched;
private String experimentId;
private String variationId;

private Map<String, Object> decisionInfo;

Expand Down Expand Up @@ -422,6 +442,16 @@ public FlagDecisionNotificationBuilder withDecisionEventDispatched(Boolean dispa
return this;
}

public FlagDecisionNotificationBuilder withExperimentId(String experimentId) {
this.experimentId = experimentId;
return this;
}

public FlagDecisionNotificationBuilder withVariationId(String variationId) {
this.variationId = variationId;
return this;
}

public DecisionNotification build() {
if (flagKey == null) {
throw new OptimizelyRuntimeException("flagKey not set");
Expand All @@ -439,6 +469,8 @@ public DecisionNotification build() {
put(RULE_KEY, ruleKey);
put(REASONS, reasons);
put(DECISION_EVENT_DISPATCHED, decisionEventDispatched);
put(EXPERIMENT_ID, experimentId);
put(VARIATION_ID, variationId);
}};

return new DecisionNotification(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,8 @@ public void decisionNotification() {
OptimizelyJSON variables = optimizely.getAllFeatureVariables(flagKey, userId);
String ruleKey = "exp_no_audience";
List<String> reasons = Collections.emptyList();
String experimentId = "10420810910";
String variationId = "10418551353";

final Map<String, Object> testDecisionInfoMap = new HashMap<>();
testDecisionInfoMap.put(FLAG_KEY, flagKey);
Expand All @@ -715,6 +717,8 @@ public void decisionNotification() {
testDecisionInfoMap.put(VARIABLES, variables.toMap());
testDecisionInfoMap.put(RULE_KEY, ruleKey);
testDecisionInfoMap.put(REASONS, reasons);
testDecisionInfoMap.put(EXPERIMENT_ID, experimentId);
testDecisionInfoMap.put(VARIATION_ID, variationId);

Map<String, Object> attributes = Collections.singletonMap("gender", "f");
OptimizelyUserContext user = optimizely.createUserContext(userId, attributes);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,15 @@ public class DecisionNotificationTest {

private static final Boolean FEATURE_ENABLED = Boolean.FALSE;
private static final String EXPERIMENT_KEY = "experimentKey";
private static final String EXPERIMENT_ID = "1234567";
private static final String FEATURE_KEY = "featureKey";
private static final String FEATURE_VARIABLE_KEY = "featureVariableKey";
private static final String FEATURE_TEST = "featureTest";
private static final String FEATURE_TEST_VARIATION = "featureTestVariation";
private static final String USER_ID = "userID";
private static final Map<String, String> USER_ATTRIBUTES = Collections.singletonMap("user", "attr");
private static final Variation VARIATION = mock(Variation.class);
private static final String VARIATION_ID = "1234567";

private FeatureTestSourceInfo featureTestSourceInfo;
private RolloutSourceInfo rolloutSourceInfo;
Expand All @@ -58,6 +60,8 @@ public void setUp() {
.withAttributes(USER_ATTRIBUTES)
.withExperimentKey(EXPERIMENT_KEY)
.withVariation(VARIATION)
.withExperimentId(EXPERIMENT_ID)
.withVariationId(VARIATION_ID)
.withType(NotificationCenter.DecisionNotificationType.AB_TEST.toString())
.build();
featureTestSourceInfo = new FeatureTestSourceInfo(FEATURE_TEST, FEATURE_TEST_VARIATION);
Expand Down Expand Up @@ -107,6 +111,8 @@ public void testGetDecisionInfo() {
HashMap<String, String> expectedExperimentDecisionInfo = new HashMap<>();
expectedExperimentDecisionInfo.put(DecisionNotification.ExperimentDecisionNotificationBuilder.EXPERIMENT_KEY, EXPERIMENT_KEY);
expectedExperimentDecisionInfo.put(DecisionNotification.ExperimentDecisionNotificationBuilder.VARIATION_KEY, VARIATION.getKey());
expectedExperimentDecisionInfo.put(DecisionNotification.ExperimentDecisionNotificationBuilder.EXPERIMENT_ID, EXPERIMENT_ID);
expectedExperimentDecisionInfo.put(DecisionNotification.ExperimentDecisionNotificationBuilder.VARIATION_ID, VARIATION_ID);
assertEquals(expectedExperimentDecisionInfo, experimentDecisionNotification.getDecisionInfo());

// Assert for Feature's DecisionInfo
Expand All @@ -128,7 +134,7 @@ public void testGetDecisionInfo() {

@Test
public void testToString() {
assertEquals("DecisionNotification{type='ab-test', userId='userID', attributes={user=attr}, decisionInfo={experimentKey=experimentKey, variationKey=null}}", experimentDecisionNotification.toString());
assertEquals("DecisionNotification{type='ab-test', userId='userID', attributes={user=attr}, decisionInfo={experimentKey=experimentKey, variationKey=null, experimentId=null, variationId=null}}", experimentDecisionNotification.toString());
assertEquals("DecisionNotification{type='feature', userId='userID', attributes={user=attr}, decisionInfo={featureEnabled=false, sourceInfo={experimentKey=featureTest, variationKey=featureTestVariation}, source=feature-test, featureKey=featureKey}}", featureDecisionNotification.toString());
assertEquals("DecisionNotification{type='feature-variable', userId='userID', attributes={user=attr}, decisionInfo={variableType=string, featureEnabled=true, sourceInfo={}, variableValue=null, variableKey=featureVariableKey, source=rollout, featureKey=featureKey}}", featureVariableDecisionNotification.toString());
}
Expand Down
Loading