Skip to content

Refact: Did nit fixes in entire project code and updated header also added missing headers #252

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 5 commits into from
Jan 7, 2019
Merged

Refact: Did nit fixes in entire project code and updated header also added missing headers #252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c9fd47d
Refact: Did nit fixes in entire project code and updated header also …
mnoman09 Jan 2, 2019
6b05688
Indentation in the code comment fix.
msohailhussain Jan 2, 2019
be7600e
Nit fixes
mnoman09 Jan 2, 2019
fbe4a8f
userprofile comment refact
mnoman09 Jan 2, 2019
1578759
Merge branch 'master' into mnoman/codeNitFixes
mikeproeng37 Jan 7, 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
397 changes: 211 additions & 186 deletions core-api/src/main/java/com/optimizely/ab/Optimizely.java
View file
Open in desktop

Large diffs are not rendered by default.

3 changes: 2 additions & 1 deletion core-api/src/main/java/com/optimizely/ab/OptimizelyRuntimeException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@
*/
public class OptimizelyRuntimeException extends RuntimeException {

public OptimizelyRuntimeException() { }
public OptimizelyRuntimeException() {
}

public OptimizelyRuntimeException(Exception exception) {
super(exception);
Expand Down
24 changes: 13 additions & 11 deletions core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2016-2017, Optimizely and contributors
* Copyright 2016-2017, 2019, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -113,7 +113,7 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
Variation bucketedVariation = experiment.getVariationIdToVariationMap().get(bucketedVariationId);
String variationKey = bucketedVariation.getKey();
logger.info("User with bucketingId \"{}\" is in variation \"{}\" of experiment \"{}\".", bucketingId, variationKey,
experimentKey);
experimentKey);

return bucketedVariation;
}
Expand All @@ -125,12 +125,14 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,

/**
* Assign a {@link Variation} of an {@link Experiment} to a user based on hashed value from murmurhash3.
* @param experiment The Experiment in which the user is to be bucketed.
*
* @param experiment The Experiment in which the user is to be bucketed.
* @param bucketingId string A customer-assigned value used to create the key for the murmur hash.
* @return Variation the user is bucketed into or null.
*/
public @Nullable Variation bucket(@Nonnull Experiment experiment,
@Nonnull String bucketingId) {
@Nullable
public Variation bucket(@Nonnull Experiment experiment,
@Nonnull String bucketingId) {
// ---------- Bucket User ----------
String groupId = experiment.getGroupId();
// check whether the experiment belongs to a group
Expand All @@ -142,20 +144,19 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,
if (bucketedExperiment == null) {
logger.info("User with bucketingId \"{}\" is not in any experiment of group {}.", bucketingId, experimentGroup.getId());
return null;
}
else {
} else {

}
// if the experiment a user is bucketed in within a group isn't the same as the experiment provided,
// don't perform further bucketing within the experiment
if (!bucketedExperiment.getId().equals(experiment.getId())) {
logger.info("User with bucketingId \"{}\" is not in experiment \"{}\" of group {}.", bucketingId, experiment.getKey(),
experimentGroup.getId());
experimentGroup.getId());
return null;
}

logger.info("User with bucketingId \"{}\" is in experiment \"{}\" of group {}.", bucketingId, experiment.getKey(),
experimentGroup.getId());
experimentGroup.getId());
}
}

Expand All @@ -167,14 +168,15 @@ private Variation bucketToVariation(@Nonnull Experiment experiment,

/**
* Map the given 32-bit hashcode into the range [0, {@link #MAX_TRAFFIC_VALUE}).
*
* @param hashCode the provided hashcode
* @return a value in the range closed-open range, [0, {@link #MAX_TRAFFIC_VALUE})
*/
@VisibleForTesting
int generateBucketValue(int hashCode) {
// map the hashCode into the range [0, BucketAlgorithm.MAX_TRAFFIC_VALUE)
double ratio = (double)(hashCode & 0xFFFFFFFFL) / Math.pow(2, 32);
return (int)Math.floor(MAX_TRAFFIC_VALUE * ratio);
double ratio = (double) (hashCode & 0xFFFFFFFFL) / Math.pow(2, 32);
return (int) Math.floor(MAX_TRAFFIC_VALUE * ratio);
}


Expand Down
10 changes: 7 additions & 3 deletions core-api/src/main/java/com/optimizely/ab/bucketing/Decision.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/****************************************************************************
* Copyright 2017, Optimizely, Inc. and contributors *
* Copyright 2017, 2019, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
Expand All @@ -24,11 +24,15 @@
*/
public class Decision {

/** The ID of the {@link com.optimizely.ab.config.Variation} the user was bucketed into. */
@Nonnull public String variationId;
/**
* The ID of the {@link com.optimizely.ab.config.Variation} the user was bucketed into.
*/
@Nonnull
public String variationId;

/**
* Initialize a Decision object.
*
* @param variationId The ID of the variation the user was bucketed into.
*/
public Decision(@Nonnull String variationId) {
Expand Down
119 changes: 64 additions & 55 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
@@ -1,5 +1,5 @@
/****************************************************************************
* Copyright 2017-2018, Optimizely, Inc. and contributors *
* Copyright 2017-2019, Optimizely, Inc. and contributors *
* *
* Licensed under the Apache License, Version 2.0 (the "License"); *
* you may not use this file except in compliance with the License. *
Expand Down Expand Up @@ -55,9 +55,10 @@ public class DecisionService {

/**
* Initialize a decision service for the Optimizely client.
* @param bucketer Base bucketer to allocate new users to an experiment.
* @param errorHandler The error handler of the Optimizely client.
* @param projectConfig Optimizely Project Config representing the datafile.
*
* @param bucketer Base bucketer to allocate new users to an experiment.
* @param errorHandler The error handler of the Optimizely client.
* @param projectConfig Optimizely Project Config representing the datafile.
* @param userProfileService UserProfileService implementation for storing user info.
*/
public DecisionService(@Nonnull Bucketer bucketer,
Expand All @@ -73,14 +74,15 @@ public DecisionService(@Nonnull Bucketer bucketer,
/**
* Get a {@link Variation} of an {@link Experiment} for a user to be allocated into.
*
* @param experiment The Experiment the user will be bucketed into.
* @param userId The userId of the user.
* @param experiment The Experiment the user will be bucketed into.
* @param userId The userId of the user.
* @param filteredAttributes The user's attributes. This should be filtered to just attributes in the Datafile.
* @return The {@link Variation} the user is allocated into.
*/
public @Nullable Variation getVariation(@Nonnull Experiment experiment,
@Nonnull String userId,
@Nonnull Map<String, ?> filteredAttributes) {
@Nullable
public Variation getVariation(@Nonnull Experiment experiment,
@Nonnull String userId,
@Nonnull Map<String, ?> filteredAttributes) {

if (!ExperimentUtils.isExperimentActive(experiment)) {
return null;
Expand All @@ -100,7 +102,7 @@ public DecisionService(@Nonnull Bucketer bucketer,

// fetch the user profile map from the user profile service
UserProfile userProfile = null;

if (userProfileService != null) {
try {
Map<String, Object> userProfileMap = userProfileService.lookup(userId);
Expand Down Expand Up @@ -149,21 +151,23 @@ public DecisionService(@Nonnull Bucketer bucketer,

/**
* Get the variation the user is bucketed into for the FeatureFlag
* @param featureFlag The feature flag the user wants to access.
* @param userId User Identifier
*
* @param featureFlag The feature flag the user wants to access.
* @param userId User Identifier
* @param filteredAttributes A map of filtered attributes.
* @return {@link FeatureDecision}
*/
public @Nonnull FeatureDecision getVariationForFeature(@Nonnull FeatureFlag featureFlag,
@Nonnull String userId,
@Nonnull Map<String, ?> filteredAttributes) {
@Nonnull
public FeatureDecision getVariationForFeature(@Nonnull FeatureFlag featureFlag,
@Nonnull String userId,
@Nonnull Map<String, ?> filteredAttributes) {
if (!featureFlag.getExperimentIds().isEmpty()) {
for (String experimentId : featureFlag.getExperimentIds()) {
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);
Variation variation = this.getVariation(experiment, userId, filteredAttributes);
if (variation != null) {
return new FeatureDecision(experiment, variation,
FeatureDecision.DecisionSource.EXPERIMENT);
FeatureDecision.DecisionSource.EXPERIMENT);
}
}
} else {
Expand All @@ -173,10 +177,10 @@ public DecisionService(@Nonnull Bucketer bucketer,
FeatureDecision featureDecision = getVariationForFeatureInRollout(featureFlag, userId, filteredAttributes);
if (featureDecision.variation == null) {
logger.info("The user \"{}\" was not bucketed into a rollout for feature flag \"{}\".",
userId, featureFlag.getKey());
userId, featureFlag.getKey());
} else {
logger.info("The user \"{}\" was bucketed into a rollout for feature flag \"{}\".",
userId, featureFlag.getKey());
userId, featureFlag.getKey());
}
return featureDecision;
}
Expand All @@ -185,14 +189,16 @@ public DecisionService(@Nonnull Bucketer bucketer,
* Try to bucket the user into a rollout rule.
* Evaluate the user for rules in priority order by seeing if the user satisfies the audience.
* Fall back onto the everyone else rule if the user is ever excluded from a rule due to traffic allocation.
* @param featureFlag The feature flag the user wants to access.
* @param userId User Identifier
*
* @param featureFlag The feature flag the user wants to access.
* @param userId User Identifier
* @param filteredAttributes A map of filtered attributes.
* @return {@link FeatureDecision}
*/
@Nonnull FeatureDecision getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag,
@Nonnull String userId,
@Nonnull Map<String, ?> filteredAttributes) {
@Nonnull
FeatureDecision getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag,
@Nonnull String userId,
@Nonnull Map<String, ?> filteredAttributes) {
// use rollout to get variation for feature
if (featureFlag.getRolloutId().isEmpty()) {
logger.info("The feature flag \"{}\" is not used in a rollout.", featureFlag.getKey());
Expand All @@ -201,7 +207,7 @@ public DecisionService(@Nonnull Bucketer bucketer,
Rollout rollout = projectConfig.getRolloutIdMapping().get(featureFlag.getRolloutId());
if (rollout == null) {
logger.error("The rollout with id \"{}\" was not found in the datafile for feature flag \"{}\".",
featureFlag.getRolloutId(), featureFlag.getKey());
featureFlag.getRolloutId(), featureFlag.getKey());
return new FeatureDecision(null, null, null);
}

Expand All @@ -218,11 +224,10 @@ public DecisionService(@Nonnull Bucketer bucketer,
break;
}
return new FeatureDecision(rolloutRule, variation,
FeatureDecision.DecisionSource.ROLLOUT);
}
else {
FeatureDecision.DecisionSource.ROLLOUT);
} else {
logger.debug("User \"{}\" did not meet the conditions to be in rollout rule for audience \"{}\".",
userId, audience.getName());
userId, audience.getName());
}
}

Expand All @@ -232,20 +237,22 @@ public DecisionService(@Nonnull Bucketer bucketer,
variation = bucketer.bucket(finalRule, bucketingId);
if (variation != null) {
return new FeatureDecision(finalRule, variation,
FeatureDecision.DecisionSource.ROLLOUT);
FeatureDecision.DecisionSource.ROLLOUT);
}
}
return new FeatureDecision(null, null, null);
}

/**
* Get the variation the user has been whitelisted into.
*
* @param experiment {@link Experiment} in which user is to be bucketed.
* @param userId User Identifier
* @param userId User Identifier
* @return null if the user is not whitelisted into any variation
* {@link Variation} the user is bucketed into if the user has a specified whitelisted variation.
* {@link Variation} the user is bucketed into if the user has a specified whitelisted variation.
*/
@Nullable Variation getWhitelistedVariation(@Nonnull Experiment experiment, @Nonnull String userId) {
@Nullable
Variation getWhitelistedVariation(@Nonnull Experiment experiment, @Nonnull String userId) {
// if a user has a forced variation mapping, return the respective variation
Map<String, String> userIdToVariationKeyMap = experiment.getUserIdToVariationKeyMap();
if (userIdToVariationKeyMap.containsKey(userId)) {
Expand All @@ -255,7 +262,7 @@ public DecisionService(@Nonnull Bucketer bucketer,
logger.info("User \"{}\" is forced in variation \"{}\".", userId, forcedVariationKey);
} else {
logger.error("Variation \"{}\" is not in the datafile. Not activating user \"{}\".",
forcedVariationKey, userId);
forcedVariationKey, userId);
}
return forcedVariation;
}
Expand All @@ -264,13 +271,15 @@ public DecisionService(@Nonnull Bucketer bucketer,

/**
* Get the {@link Variation} that has been stored for the user in the {@link UserProfileService} implementation.
* @param experiment {@link Experiment} in which the user was bucketed.
*
* @param experiment {@link Experiment} in which the user was bucketed.
* @param userProfile {@link UserProfile} of the user.
* @return null if the {@link UserProfileService} implementation is null or the user was not previously bucketed.
* else return the {@link Variation} the user was previously bucketed into.
* else return the {@link Variation} the user was previously bucketed into.
*/
@Nullable Variation getStoredVariation(@Nonnull Experiment experiment,
@Nonnull UserProfile userProfile) {
@Nullable
Variation getStoredVariation(@Nonnull Experiment experiment,
@Nonnull UserProfile userProfile) {
// ---------- Check User Profile for Sticky Bucketing ----------
// If a user profile instance is present then check it for a saved variation
String experimentId = experiment.getId();
Expand All @@ -279,35 +288,35 @@ public DecisionService(@Nonnull Bucketer bucketer,
if (decision != null) {
String variationId = decision.variationId;
Variation savedVariation = projectConfig
.getExperimentIdMapping()
.get(experimentId)
.getVariationIdToVariationMap()
.get(variationId);
.getExperimentIdMapping()
.get(experimentId)
.getVariationIdToVariationMap()
.get(variationId);
if (savedVariation != null) {
logger.info("Returning previously activated variation \"{}\" of experiment \"{}\" " +
"for user \"{}\" from user profile.",
savedVariation.getKey(), experimentKey, userProfile.userId);
"for user \"{}\" from user profile.",
savedVariation.getKey(), experimentKey, userProfile.userId);
// A variation is stored for this combined bucket id
return savedVariation;
} else {
logger.info("User \"{}\" was previously bucketed into variation with ID \"{}\" for experiment \"{}\", " +
"but no matching variation was found for that user. We will re-bucket the user.",
userProfile.userId, variationId, experimentKey);
"but no matching variation was found for that user. We will re-bucket the user.",
userProfile.userId, variationId, experimentKey);
return null;
}
} else {
logger.info("No previously activated variation of experiment \"{}\" " +
"for user \"{}\" found in user profile.",
experimentKey, userProfile.userId);
"for user \"{}\" found in user profile.",
experimentKey, userProfile.userId);
return null;
}
}

/**
* Save a {@link Variation} of an {@link Experiment} for a user in the {@link UserProfileService}.
*
* @param experiment The experiment the user was buck
* @param variation The Variation to save.
* @param experiment The experiment the user was buck
* @param variation The Variation to save.
* @param userProfile A {@link UserProfile} instance of the user information.
*/
void saveVariation(@Nonnull Experiment experiment,
Expand All @@ -332,18 +341,19 @@ void saveVariation(@Nonnull Experiment experiment,
variationId, experimentId, userProfile.userId);
} catch (Exception exception) {
logger.warn("Failed to save variation \"{}\" of experiment \"{}\" for user \"{}\".",
variationId, experimentId, userProfile.userId);
variationId, experimentId, userProfile.userId);
errorHandler.handleError(new OptimizelyRuntimeException(exception));
}
}
}

/**
* Get the bucketingId of a user if a bucketingId exists in attributes, or else default to userId.
* @param userId The userId of the user.
*
* @param userId The userId of the user.
* @param filteredAttributes The user's attributes. This should be filtered to just attributes in the Datafile.
* @return bucketingId if it is a String type in attributes.
* else return userId
* else return userId
*/
String getBucketingId(@Nonnull String userId,
@Nonnull Map<String, ?> filteredAttributes) {
Expand All @@ -352,10 +362,9 @@ String getBucketingId(@Nonnull String userId,
if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) {
bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString());
logger.debug("BucketingId is valid: \"{}\"", bucketingId);
}
else {
} else {
logger.warn("BucketingID attribute is not a string. Defaulted to userId");
}
}
}
return bucketingId;
}
Expand Down
Loading