Skip to content

(feat): Feature Management isFeatureEnabled Listener #271

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

Merged
merged 19 commits into from
Apr 22, 2019
Merged

(feat): Feature Management isFeatureEnabled Listener #271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d5de529
Added Listener In isFeatureEnabled
mnoman09 Mar 8, 2019
bbb38ba
Added getEnabledFeatures unitTests
mnoman09 Mar 8, 2019
d9b42d4
bug fix
mnoman09 Mar 8, 2019
f634e9f
Changed OnDecision enum to Decision
mnoman09 Mar 12, 2019
8250e95
Updated testcases and passed attributes empty instead of null in deci…
mnoman09 Mar 13, 2019
76f4e62
Did naming changes
mnoman09 Mar 14, 2019
c461797
Merge branch 'master' into mnoman/isFeatEnabledNotif
mfahadahmed Mar 18, 2019
0c8e099
nit changes
mnoman09 Mar 20, 2019
0d841b6
Nit fix and added check of is listener Called
mnoman09 Mar 20, 2019
d3099aa
Merge branch 'master' into mnoman/isFeatEnabledNotif
mnoman09 Apr 2, 2019
a790882
Added builder pattern and refactored code
mnoman09 Apr 2, 2019
221cb90
Merge branch 'master' into mnoman/isFeatEnabledNotif
aliabbasrizvi Apr 5, 2019
0593274
Merge branch 'master' into mnoman/isFeatEnabledNotif
mnoman09 Apr 8, 2019
8f07155
Merge branch 'master' into mnoman/isFeatEnabledNotif
mnoman09 Apr 11, 2019
e8b4500
Changed DecisionSource experiment to feature-test and added source_in…
mnoman09 Apr 16, 2019
5035ff0
Merge remote-tracking branch 'origin/master' into mnoman/isFeatENotif…
mnoman09 Apr 18, 2019
6f08403
Nit fix
mnoman09 Apr 19, 2019
887ba63
Changed naming convention of decision Notification keys
mnoman09 Apr 19, 2019
d95af12
Merge branch 'master' into mnoman/isFeatEnabledNotif
aliabbasrizvi Apr 22, 2019
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
24 changes: 21 additions & 3 deletions core-api/src/main/java/com/optimizely/ab/Optimizely.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,12 @@ public Boolean isFeatureEnabled(@Nonnull String featureKey,
}

Map<String, ?> copiedAttributes = copyAttributes(attributes);
FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes);
FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT;
String sourceExperimentKey = null;
String sourceVariationKey = null;

FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, copiedAttributes);
Boolean featureEnabled = false;
if (featureDecision.variation != null) {
if (featureDecision.decisionSource.equals(FeatureDecision.DecisionSource.FEATURE_TEST)) {
sendImpression(
Expand All @@ -397,18 +401,32 @@ public Boolean isFeatureEnabled(@Nonnull String featureKey,
userId,
copiedAttributes,
featureDecision.variation);
decisionSource = featureDecision.decisionSource;
sourceVariationKey = featureDecision.variation.getKey();
sourceExperimentKey = featureDecision.experiment.getKey();
} else {
logger.info("The user \"{}\" is not included in an experiment for feature \"{}\".",
userId, featureKey);
}
if (featureDecision.variation.getFeatureEnabled()) {
logger.info("Feature \"{}\" is enabled for user \"{}\".", featureKey, userId);
return true;
featureEnabled = true;
}
}

DecisionNotification decisionNotification = DecisionNotification.newFeatureDecisionNotificationBuilder()
.withUserId(userId)
.withAttributes(copiedAttributes)
.withFeatureKey(featureKey)
.withFeatureEnabled(featureEnabled)
.withSource(decisionSource)
.withExperimentKey(sourceExperimentKey)
.withVariationKey(sourceVariationKey)
Copy link
Contributor

Choose a reason for hiding this comment

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

May be remove these 2 build options and replace with withSourceInfo option which takes SourceInfo object.

You can then define 2 implementations SourceInfo. 1 for rollout and 1 for feature test and then use a get method on them to get the source info that you build in the builder here. That may be a better way to structure this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you suggesting to make methods of get source info in optimizely or in DecisionNotification.java?
By the way I think current implementation is much simpler and fully utilizing builder pattern.

Copy link
Contributor

Choose a reason for hiding this comment

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

I am suggesting something like:

public interface SourceInfo {
  HashMap<String, Object> get()
}

public class FeatureTestSourceInfo implements SourceInfo {
}

public class RolloutSourceInfo implements SourceInfo {
}

This may allow us to hopefully add more SourceInfo options in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

Discussed offline. This will happen in a follow up.

.build();
notificationCenter.sendNotifications(decisionNotification);

logger.info("Feature \"{}\" is not enabled for user \"{}\".", featureKey, userId);
return false;
return featureEnabled;
}

/**
Expand Down
83 changes: 79 additions & 4 deletions core-api/src/main/java/com/optimizely/ab/notification/DecisionNotification.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@

package com.optimizely.ab.notification;


import com.optimizely.ab.bucketing.FeatureDecision;
import com.optimizely.ab.config.FeatureVariable;
import com.optimizely.ab.config.Variation;

import javax.annotation.Nonnull;
Expand Down Expand Up @@ -69,8 +67,8 @@ public static ExperimentDecisionNotificationBuilder newExperimentDecisionNotific
}

public static class ExperimentDecisionNotificationBuilder {
public final static String EXPERIMENT_KEY = "experiment_key";
public final static String VARIATION_KEY = "variation_key";
public final static String EXPERIMENT_KEY = "experimentKey";
public final static String VARIATION_KEY = "variationKey";

private String type;
private String experimentKey;
Expand Down Expand Up @@ -116,4 +114,81 @@ public DecisionNotification build() {
decisionInfo);
}
}

public static FeatureDecisionNotificationBuilder newFeatureDecisionNotificationBuilder() {
return new FeatureDecisionNotificationBuilder();
}

public static class FeatureDecisionNotificationBuilder {
public final static String FEATURE_KEY = "featureKey";
public final static String FEATURE_ENABLED = "featureEnabled";
public final static String SOURCE = "source";
public final static String SOURCE_INFO = "sourceInfo";
public final static String EXPERIMENT_KEY = "experimentKey";
public final static String VARIATION_KEY = "variationKey";

private String featureKey;
private Boolean featureEnabled;
private String experimentKey;
private String variationKey;
private FeatureDecision.DecisionSource source;
private String userId;
private Map<String, ?> attributes;
private Map<String, Object> decisionInfo;

public FeatureDecisionNotificationBuilder withUserId(String userId) {
this.userId = userId;
return this;
}

public FeatureDecisionNotificationBuilder withAttributes(Map<String, ?> attributes) {
this.attributes = attributes;
return this;
}

public FeatureDecisionNotificationBuilder withExperimentKey(String experimentKey) {
this.experimentKey = experimentKey;
return this;
}

public FeatureDecisionNotificationBuilder withVariationKey(String variationKey) {
this.variationKey = variationKey;
return this;
}

public FeatureDecisionNotificationBuilder withSource(FeatureDecision.DecisionSource source) {
this.source = source;
return this;
}

public FeatureDecisionNotificationBuilder withFeatureKey(String featureKey) {
this.featureKey = featureKey;
return this;
}

public FeatureDecisionNotificationBuilder withFeatureEnabled(Boolean featureEnabled) {
this.featureEnabled = featureEnabled;
return this;
}

public DecisionNotification build() {
decisionInfo = new HashMap<>();
decisionInfo.put(FEATURE_KEY, featureKey);
decisionInfo.put(FEATURE_ENABLED, featureEnabled);
decisionInfo.put(SOURCE, source);

Map<String, String> sourceInfo = new HashMap<>();
if (source.equals(FeatureDecision.DecisionSource.FEATURE_TEST)) {
sourceInfo.put(EXPERIMENT_KEY, experimentKey);
sourceInfo.put(VARIATION_KEY, variationKey);
}
decisionInfo.put(SOURCE_INFO, sourceInfo);

return new DecisionNotification(
NotificationCenter.DecisionNotificationType.FEATURE.toString(),
userId,
attributes,
decisionInfo);
}
}
}
Loading