From c401466160b34b895bc86e598fd923ea70949900 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 16 Aug 2018 11:38:38 -0700 Subject: [PATCH 01/18] wildcard replaces string; implement type checking --- .../java/com/optimizely/ab/Optimizely.java | 44 +++++++++++++------ .../ab/bucketing/DecisionService.java | 10 ++--- .../ab/config/audience/AndCondition.java | 2 +- .../ab/config/audience/Condition.java | 2 +- .../ab/config/audience/NotCondition.java | 2 +- .../ab/config/audience/OrCondition.java | 2 +- .../ab/config/audience/UserAttribute.java | 4 +- .../ab/event/internal/EventFactory.java | 8 ++-- .../ab/internal/ExperimentUtils.java | 2 +- 9 files changed, 47 insertions(+), 29 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 51f5dadbd..7bc013c65 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -120,7 +120,7 @@ Variation activate(@Nonnull String experimentKey, public @Nullable Variation activate(@Nonnull String experimentKey, @Nonnull String userId, - @Nonnull Map attributes) throws UnknownExperimentException { + @Nonnull Map attributes) throws UnknownExperimentException { if (experimentKey == null) { logger.error("The experimentKey parameter must be nonnull."); @@ -153,7 +153,7 @@ Variation activate(@Nonnull Experiment experiment, public @Nullable Variation activate(@Nonnull Experiment experiment, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { ProjectConfig currentConfig = getProjectConfig(); @@ -164,7 +164,7 @@ Variation activate(@Nonnull Experiment experiment, Variation activate(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment experiment, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { if (!validateUserId(userId)){ logger.info("Not activating user \"{}\" for experiment \"{}\".", userId, experiment.getKey()); @@ -172,7 +172,7 @@ Variation activate(@Nonnull ProjectConfig projectConfig, } // determine whether all the given attributes are present in the project config. If not, filter out the unknown // attributes. - Map filteredAttributes = filterAttributes(projectConfig, attributes); + Map filteredAttributes = filterAttributes(projectConfig, attributes); // bucket the user to the given experiment and dispatch an impression event Variation variation = decisionService.getVariation(experiment, userId, filteredAttributes); @@ -189,7 +189,7 @@ Variation activate(@Nonnull ProjectConfig projectConfig, private void sendImpression(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment experiment, @Nonnull String userId, - @Nonnull Map filteredAttributes, + @Nonnull Map filteredAttributes, @Nonnull Variation variation) { if (experiment.isRunning()) { LogEvent impressionEvent = eventFactory.createImpressionEvent( @@ -259,7 +259,7 @@ public void track(@Nonnull String eventName, // determine whether all the given attributes are present in the project config. If not, filter out the unknown // attributes. - Map filteredAttributes = filterAttributes(currentConfig, attributes); + Map filteredAttributes = filterAttributes(currentConfig, attributes); if (eventTags == null) { logger.warn("Event tags is null when non-null was expected. Defaulting to an empty event tags map."); @@ -359,7 +359,7 @@ else if (userId == null) { return false; } - Map filteredAttributes = filterAttributes(projectConfig, attributes); + Map filteredAttributes = filterAttributes(projectConfig, attributes); FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, filteredAttributes); if (featureDecision.variation != null) { @@ -644,7 +644,7 @@ Variation getVariation(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes) throws UnknownExperimentException { - Map filteredAttributes = filterAttributes(projectConfig, attributes); + Map filteredAttributes = filterAttributes(projectConfig, attributes); return decisionService.getVariation(experiment, userId, filteredAttributes); } @@ -677,7 +677,7 @@ Variation getVariation(@Nonnull String experimentKey, return null; } - Map filteredAttributes = filterAttributes(projectConfig, attributes); + Map filteredAttributes = filterAttributes(projectConfig, attributes); return decisionService.getVariation(experiment,userId,filteredAttributes); } @@ -763,17 +763,18 @@ public UserProfileService getUserProfileService() { * @return the filtered attributes map (containing only attributes that are present in the project config) or an * empty map if a null attributes object is passed in */ - private Map filterAttributes(@Nonnull ProjectConfig projectConfig, - @Nonnull Map attributes) { + private Map filterAttributes(@Nonnull ProjectConfig projectConfig, + @Nonnull Map attributes) { if (attributes == null) { logger.warn("Attributes is null when non-null was expected. Defaulting to an empty attributes map."); return Collections.emptyMap(); } List unknownAttributes = null; + List invalidTypeAttributes = null; Map attributeKeyMapping = projectConfig.getAttributeKeyMapping(); - for (Map.Entry attribute : attributes.entrySet()) { + for (Map.Entry attribute : attributes.entrySet()) { if (!attributeKeyMapping.containsKey(attribute.getKey()) && !attribute.getKey().startsWith(ProjectConfig.RESERVED_ATTRIBUTE_PREFIX)) { if (unknownAttributes == null) { @@ -781,17 +782,34 @@ private Map filterAttributes(@Nonnull ProjectConfig projectConfi } unknownAttributes.add(attribute.getKey()); } + else if (!Boolean.class.isInstance(attribute.getValue()) && + !Number.class.isInstance(attribute.getValue()) && + !String.class.isInstance(attribute.getValue())) { + if (invalidTypeAttributes == null) { + invalidTypeAttributes = null; + } + invalidTypeAttributes.add(attribute.getKey()); + } } if (unknownAttributes != null) { logger.warn("Attribute(s) {} not in the datafile.", unknownAttributes); // make a copy of the passed through attributes, then remove the unknown list - attributes = new HashMap(attributes); + attributes = new HashMap(attributes); for (String unknownAttribute : unknownAttributes) { attributes.remove(unknownAttribute); } } + if (invalidTypeAttributes != null) { + //TODO: logger message + attributes = new HashMap(attributes); + Remove invalid attributes? dont do anything with them? + for (String invalidTypeAttribute: invalidTypeAttributes) { + attributes.remove(invalidTypeAttribute); + } + } + return attributes; } diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 0dc895feb..2a9c204aa 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -80,7 +80,7 @@ public DecisionService(@Nonnull Bucketer bucketer, */ public @Nullable Variation getVariation(@Nonnull Experiment experiment, @Nonnull String userId, - @Nonnull Map filteredAttributes) { + @Nonnull Map filteredAttributes) { if (!ExperimentUtils.isExperimentActive(experiment)) { return null; @@ -131,7 +131,7 @@ public DecisionService(@Nonnull Bucketer bucketer, if (ExperimentUtils.isUserInExperiment(projectConfig, experiment, filteredAttributes)) { String bucketingId = userId; if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { - bucketingId = filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); + bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); } variation = bucketer.bucket(experiment, bucketingId); @@ -159,7 +159,7 @@ public DecisionService(@Nonnull Bucketer bucketer, */ public @Nonnull FeatureDecision getVariationForFeature(@Nonnull FeatureFlag featureFlag, @Nonnull String userId, - @Nonnull Map filteredAttributes) { + @Nonnull Map filteredAttributes) { if (!featureFlag.getExperimentIds().isEmpty()) { for (String experimentId : featureFlag.getExperimentIds()) { Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId); @@ -195,7 +195,7 @@ public DecisionService(@Nonnull Bucketer bucketer, */ @Nonnull FeatureDecision getVariationForFeatureInRollout(@Nonnull FeatureFlag featureFlag, @Nonnull String userId, - @Nonnull Map filteredAttributes) { + @Nonnull Map 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()); @@ -212,7 +212,7 @@ public DecisionService(@Nonnull Bucketer bucketer, int rolloutRulesLength = rollout.getExperiments().size(); String bucketingId = userId; if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { - bucketingId = filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); + bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); } Variation variation; for (int i = 0; i < rolloutRulesLength - 1; i++) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java index 2e485b281..300d40973 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java @@ -36,7 +36,7 @@ public List getConditions() { return conditions; } - public boolean evaluate(Map attributes) { + public boolean evaluate(Map attributes) { for (Condition condition : conditions) { if (!condition.evaluate(attributes)) return false; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java index 3a47453e4..319217c3b 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java @@ -23,5 +23,5 @@ */ public interface Condition { - boolean evaluate(Map attributes); + boolean evaluate(Map attributes); } \ No newline at end of file diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java index 7e269c6f3..f4e9f3c0e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java @@ -37,7 +37,7 @@ public Condition getCondition() { return condition; } - public boolean evaluate(Map attributes) { + public boolean evaluate(Map attributes) { return !condition.evaluate(attributes); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java index 6e80a43e5..13bc25e36 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java @@ -36,7 +36,7 @@ public List getConditions() { return conditions; } - public boolean evaluate(Map attributes) { + public boolean evaluate(Map attributes) { for (Condition condition : conditions) { if (condition.evaluate(attributes)) return true; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index 3827291bd..e57065fb1 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -49,8 +49,8 @@ public String getValue() { return value; } - public boolean evaluate(Map attributes) { - String userAttributeValue = attributes.get(name); + public boolean evaluate(Map attributes) { + Object userAttributeValue = attributes.get(name); if (value != null) { // if there is a value in the condition // check user attribute value is equal diff --git a/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java b/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java index a7aedda50..ba347e7af 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java +++ b/core-api/src/main/java/com/optimizely/ab/event/internal/EventFactory.java @@ -62,7 +62,7 @@ public LogEvent createImpressionEvent(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment activatedExperiment, @Nonnull Variation variation, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { Decision decision = new Decision.Builder() .setCampaignId(activatedExperiment.getLayerId()) @@ -108,7 +108,7 @@ public LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig, @Nonnull String userId, @Nonnull String eventId, // Why is this not used? @Nonnull String eventName, - @Nonnull Map attributes, + @Nonnull Map attributes, @Nonnull Map eventTags) { if (experimentVariationMap.isEmpty()) { @@ -164,10 +164,10 @@ public LogEvent createConversionEvent(@Nonnull ProjectConfig projectConfig, return new LogEvent(LogEvent.RequestMethod.POST, EVENT_ENDPOINT, Collections.emptyMap(), eventBatch); } - private List buildAttributeList(ProjectConfig projectConfig, Map attributes) { + private List buildAttributeList(ProjectConfig projectConfig, Map attributes) { List attributesList = new ArrayList(); - for (Map.Entry entry : attributes.entrySet()) { + for (Map.Entry entry : attributes.entrySet()) { String attributeId = projectConfig.getAttributeId(projectConfig, entry.getKey()); if(attributeId == null) { continue; diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java index a0ba4b99b..0355a9f09 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java @@ -58,7 +58,7 @@ public static boolean isExperimentActive(@Nonnull Experiment experiment) { */ public static boolean isUserInExperiment(@Nonnull ProjectConfig projectConfig, @Nonnull Experiment experiment, - @Nonnull Map attributes) { + @Nonnull Map attributes) { List experimentAudienceIds = experiment.getAudienceIds(); // if there are no audiences, ALL users should be part of the experiment From d73f4ebc80f7e5cfb7f4c0c1c46525c012b11369 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 16 Aug 2018 14:46:19 -0700 Subject: [PATCH 02/18] deleting invalid types for filtered attributes --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 7bc013c65..28342dddf 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -770,6 +770,7 @@ public UserProfileService getUserProfileService() { return Collections.emptyMap(); } + // List of attribute keys List unknownAttributes = null; List invalidTypeAttributes = null; @@ -786,7 +787,7 @@ else if (!Boolean.class.isInstance(attribute.getValue()) && !Number.class.isInstance(attribute.getValue()) && !String.class.isInstance(attribute.getValue())) { if (invalidTypeAttributes == null) { - invalidTypeAttributes = null; + invalidTypeAttributes = new ArrayList(); } invalidTypeAttributes.add(attribute.getKey()); } @@ -802,10 +803,10 @@ else if (!Boolean.class.isInstance(attribute.getValue()) && } if (invalidTypeAttributes != null) { - //TODO: logger message + // TODO: logger message attributes = new HashMap(attributes); - Remove invalid attributes? dont do anything with them? - for (String invalidTypeAttribute: invalidTypeAttributes) { + // Remove invalid attributes? dont do anything with them? + for (String invalidTypeAttribute : invalidTypeAttributes) { attributes.remove(invalidTypeAttribute); } } From 6173eee4f540fa754fd027c08b4324fed49e55df Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 16 Aug 2018 15:35:55 -0700 Subject: [PATCH 03/18] attribute with null values are filtered out --- core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index b121624c6..fb626dcbf 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -725,7 +725,7 @@ public void activateWithNullAttributeValues() throws Exception { eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); Map actualValue = attributeCaptor.getValue(); - assertThat(actualValue, hasEntry(attribute.getKey(), null)); + assertThat(actualValue, not(hasEntry(attribute.getKey(), null))); // verify that dispatchEvent was called with the correct LogEvent object verify(mockEventHandler).dispatchEvent(logEventToDispatch); From be4de1cded5d48d12ae0c437522120feb9d22dc2 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Fri, 17 Aug 2018 17:00:33 -0700 Subject: [PATCH 04/18] resolved issues with null --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 7 ++++--- .../src/test/java/com/optimizely/ab/OptimizelyTest.java | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 28342dddf..f55f410a0 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -783,9 +783,10 @@ public UserProfileService getUserProfileService() { } unknownAttributes.add(attribute.getKey()); } - else if (!Boolean.class.isInstance(attribute.getValue()) && - !Number.class.isInstance(attribute.getValue()) && - !String.class.isInstance(attribute.getValue())) { + else if (attribute.getValue() != null && + !Boolean.class.isInstance(attribute.getValue()) && + !Number.class.isInstance(attribute.getValue()) && + !String.class.isInstance(attribute.getValue())) { if (invalidTypeAttributes == null) { invalidTypeAttributes = new ArrayList(); } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index fb626dcbf..b121624c6 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -725,7 +725,7 @@ public void activateWithNullAttributeValues() throws Exception { eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); Map actualValue = attributeCaptor.getValue(); - assertThat(actualValue, not(hasEntry(attribute.getKey(), null))); + assertThat(actualValue, hasEntry(attribute.getKey(), null)); // verify that dispatchEvent was called with the correct LogEvent object verify(mockEventHandler).dispatchEvent(logEventToDispatch); From 0b238273f14e359ba8bf769e0e786fc32bb598ed Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Tue, 21 Aug 2018 15:13:30 -0700 Subject: [PATCH 05/18] paramater/return changes --- .../com/optimizely/ab/config/audience/UserAttribute.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index e57065fb1..0ae651a47 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -29,9 +29,9 @@ public class UserAttribute implements Condition { private final String name; private final String type; - private final String value; + private final Object value; - public UserAttribute(@Nonnull String name, @Nonnull String type, @Nullable String value) { + public UserAttribute(@Nonnull String name, @Nonnull String type, @Nullable Object value) { this.name = name; this.type = type; this.value = value; @@ -45,7 +45,7 @@ public String getType() { return type; } - public String getValue() { + public Object getValue() { return value; } From cc9a5a643d089e2b17a7b418e155d9a46e2fd6b9 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Tue, 21 Aug 2018 15:15:53 -0700 Subject: [PATCH 06/18] remove filtering invalid types --- .../java/com/optimizely/ab/Optimizely.java | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index f55f410a0..83be8f0d1 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -772,7 +772,6 @@ public UserProfileService getUserProfileService() { // List of attribute keys List unknownAttributes = null; - List invalidTypeAttributes = null; Map attributeKeyMapping = projectConfig.getAttributeKeyMapping(); for (Map.Entry attribute : attributes.entrySet()) { @@ -783,15 +782,6 @@ public UserProfileService getUserProfileService() { } unknownAttributes.add(attribute.getKey()); } - else if (attribute.getValue() != null && - !Boolean.class.isInstance(attribute.getValue()) && - !Number.class.isInstance(attribute.getValue()) && - !String.class.isInstance(attribute.getValue())) { - if (invalidTypeAttributes == null) { - invalidTypeAttributes = new ArrayList(); - } - invalidTypeAttributes.add(attribute.getKey()); - } } if (unknownAttributes != null) { @@ -803,15 +793,6 @@ else if (attribute.getValue() != null && } } - if (invalidTypeAttributes != null) { - // TODO: logger message - attributes = new HashMap(attributes); - // Remove invalid attributes? dont do anything with them? - for (String invalidTypeAttribute : invalidTypeAttributes) { - attributes.remove(invalidTypeAttribute); - } - } - return attributes; } From 881774f291c0a3bf58bfb4ccc5988d8c1b7d4c3f Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Wed, 22 Aug 2018 16:01:13 -0700 Subject: [PATCH 07/18] added from suggestions + unit tests --- .../ab/bucketing/DecisionService.java | 8 +- .../ab/config/audience/UserAttribute.java | 1 + .../com/optimizely/ab/OptimizelyTest.java | 76 ++++++++++++++++++- .../ab/config/ValidProjectConfigV4.java | 15 ++++ .../config/valid-project-config-v4.json | 15 +++- 5 files changed, 107 insertions(+), 8 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 2a9c204aa..f4ce3eee7 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -131,7 +131,9 @@ public DecisionService(@Nonnull Bucketer bucketer, if (ExperimentUtils.isUserInExperiment(projectConfig, experiment, filteredAttributes)) { String bucketingId = userId; if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { - bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); + if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) { + bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); + } } variation = bucketer.bucket(experiment, bucketingId); @@ -212,7 +214,9 @@ public DecisionService(@Nonnull Bucketer bucketer, int rolloutRulesLength = rollout.getExperiments().size(); String bucketingId = userId; if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { - bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); + if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) { + bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); + } } Variation variation; for (int i = 0; i < rolloutRulesLength - 1; i++) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index 0ae651a47..077915ceb 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -50,6 +50,7 @@ public Object getValue() { } public boolean evaluate(Map attributes) { + // Valid for primative types, but needs to change when a value is an object or an array Object userAttributeValue = attributes.get(name); if (value != null) { // if there is a value in the condition diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index b121624c6..254a64063 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -524,7 +524,7 @@ public void activateWithAttributes() throws Exception { Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); Variation bucketedVariation = activatedExperiment.getVariations().get(0); Variation userIdBucketedVariation = activatedExperiment.getVariations().get(1); - Attribute attribute = validProjectConfig.getAttributes().get(0); + Attribute attributeString = validProjectConfig.getAttributes().get(0); // setup a mock event builder to return expected impression params EventFactory mockEventFactory = mock(EventFactory.class); @@ -546,8 +546,9 @@ public void activateWithAttributes() throws Exception { when(mockBucketer.bucket(activatedExperiment, testBucketingId)) .thenReturn(bucketedVariation); - Map attr = new HashMap(); - attr.put(attribute.getKey(), "attributeValue"); + Map attr = new HashMap(); + + attr.put(attributeString.getKey(), "attributeValue"); attr.put(testBucketingIdKey, testBucketingId); // activate the experiment @@ -564,12 +565,79 @@ public void activateWithAttributes() throws Exception { eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); Map actualValue = attributeCaptor.getValue(); - assertThat(actualValue, hasEntry(attribute.getKey(), "attributeValue")); + assertThat(actualValue, hasEntry(attributeString.getKey(), "attributeValue")); // verify that dispatchEvent was called with the correct LogEvent object verify(mockEventHandler).dispatchEvent(logEventToDispatch); } + public void activateWithTypedAttributes() throws Exception { + if (datafileVersion >= 4) { + Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + Variation userIdBucketedVariation = activatedExperiment.getVariations().get(1); + Attribute attributeString = validProjectConfig.getAttributes().get(0); + Attribute attributeBoolean = validProjectConfig.getAttributes().get(3); + Attribute attributeInteger = validProjectConfig.getAttributes().get(4); + Attribute attributeDouble = validProjectConfig.getAttributes().get(5); + + + // setup a mock event builder to return expected impression params + EventFactory mockEventFactory = mock(EventFactory.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventFactory) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + when(mockEventFactory.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(bucketedVariation), + eq(testUserId), anyMapOf(String.class, String.class))) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, testUserId)) + .thenReturn(userIdBucketedVariation); + + when(mockBucketer.bucket(activatedExperiment, testBucketingId)) + .thenReturn(bucketedVariation); + + Map attr = new HashMap(); + + attr.put(attributeString.getKey(), "attributeValue"); + attr.put(attributeBoolean.getKey(), true); + attr.put(attributeInteger.getKey(), 3); + attr.put(attributeDouble.getKey(), 3.123); + attr.put(testBucketingIdKey, testBucketingId); + + // activate the experiment + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, + attr); + + // verify that the bucketing algorithm was called correctly + verify(mockBucketer).bucket(activatedExperiment, testBucketingId); + assertThat(actualVariation, is(bucketedVariation)); + + // setup the attribute map captor (so we can verify its content) + ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); + verify(mockEventFactory).createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), + eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); + + Map actualValue = attributeCaptor.getValue(); + + assertThat(actualValue, hasKey(attributeString.getKey())); + if (String.class.isInstance(actualValue.get(attributeString.getKey()))){ + assertThat((String) actualValue.get(attributeString.getKey()), is("attributeValue")); + } + // assertThat(actualValue, hasEntry(attributeBoolean.getKey(), true)); + // assertThat(actualValue, hasEntry(attributeInteger.getKey(), 3)); + // assertThat(actualValue, hasEntry(attributeDouble.getKey(), 3.1415)); + + // verify that dispatchEvent was called with the correct LogEvent object + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + } + } + /** * Verify that {@link Optimizely#activate(String, String, Map)} handles the case * where an unknown attribute (i.e., not in the config) is passed through. diff --git a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java index 19a65dcba..19b3a4ae1 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java @@ -49,6 +49,18 @@ public class ValidProjectConfigV4 { public static final String ATTRIBUTE_OPT_KEY = "$opt_test"; private static final Attribute ATTRIBUTE_OPT = new Attribute(ATTRIBUTE_OPT_ID, ATTRIBUTE_OPT_KEY); + private static final String ATTRIBUTE_BOOLEAN_ID = "323434545"; + public static final String ATTRIBUTE_BOOLEAN_KEY = "booleanKey"; + private static final Attribute ATTRIBUTE_BOOLEAN = new Attribute(ATTRIBUTE_BOOLEAN_ID, ATTRIBUTE_BOOLEAN_KEY); + + private static final String ATTRIBUTE_INTEGER_ID = "616727838"; + public static final String ATTRIBUTE_INTEGER_KEY = "integerKey"; + private static final Attribute ATTRIBUTE_INTEGER = new Attribute(ATTRIBUTE_INTEGER_ID, ATTRIBUTE_INTEGER_KEY); + + private static final String ATTRIBUTE_DOUBLE_ID = "808797686"; + public static final String ATTRIBUTE_DOUBLE_KEY = "doubleKey"; + private static final Attribute ATTRIBUTE_DOUBLE = new Attribute(ATTRIBUTE_DOUBLE_ID, ATTRIBUTE_DOUBLE_KEY); + // audiences private static final String CUSTOM_DIMENSION_TYPE = "custom_dimension"; private static final String AUDIENCE_GRYFFINDOR_ID = "3468206642"; @@ -1033,6 +1045,9 @@ public static ProjectConfig generateValidProjectConfigV4() { attributes.add(ATTRIBUTE_HOUSE); attributes.add(ATTRIBUTE_NATIONALITY); attributes.add(ATTRIBUTE_OPT); + attributes.add(ATTRIBUTE_BOOLEAN); + attributes.add(ATTRIBUTE_INTEGER); + attributes.add(ATTRIBUTE_DOUBLE); // list audiences List audiences = new ArrayList(); diff --git a/core-api/src/test/resources/config/valid-project-config-v4.json b/core-api/src/test/resources/config/valid-project-config-v4.json index db961012e..d5cc0720d 100644 --- a/core-api/src/test/resources/config/valid-project-config-v4.json +++ b/core-api/src/test/resources/config/valid-project-config-v4.json @@ -35,10 +35,21 @@ { "id": "58339410", "key": "nationality" - }, - { + }, { "id": "583394100", "key": "$opt_test" + }, + { + "id": "323434545", + "key": "booleanKey" + }, + { + "id": "616727838", + "key": "integerKey" + }, + { + "id": "808797686", + "key": "doubleKey" } ], "events": [ From a47f8af51b6511784adf14bf7742fba546eafcd2 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Wed, 22 Aug 2018 18:10:39 -0700 Subject: [PATCH 08/18] logger, work-around for unit test --- .../ab/bucketing/DecisionService.java | 12 +++++++++-- .../com/optimizely/ab/OptimizelyTest.java | 21 +++++++++++++------ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index f4ce3eee7..9ddadfbf8 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -133,7 +133,11 @@ public DecisionService(@Nonnull Bucketer bucketer, if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) { bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); - } + logger.info("bucketingId has a valid type"); + } + else { + logger.warn("bucketingID has type mismatch, defaulted to userId "); + } } variation = bucketer.bucket(experiment, bucketingId); @@ -216,7 +220,11 @@ public DecisionService(@Nonnull Bucketer bucketer, if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) { bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); - } + logger.info("bucketingId has a valid type"); + } + else { + logger.warn("bucketingID has type mismatch, defaulted to userId "); + } } Variation variation; for (int i = 0; i < rolloutRulesLength - 1; i++) { diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 254a64063..7ee8cd3a6 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -76,6 +76,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; @@ -626,12 +627,20 @@ public void activateWithTypedAttributes() throws Exception { Map actualValue = attributeCaptor.getValue(); assertThat(actualValue, hasKey(attributeString.getKey())); - if (String.class.isInstance(actualValue.get(attributeString.getKey()))){ - assertThat((String) actualValue.get(attributeString.getKey()), is("attributeValue")); - } - // assertThat(actualValue, hasEntry(attributeBoolean.getKey(), true)); - // assertThat(actualValue, hasEntry(attributeInteger.getKey(), 3)); - // assertThat(actualValue, hasEntry(attributeDouble.getKey(), 3.1415)); + assertThat("attributeValue", sameInstance(actualValue.get(attributeString.getKey()))); + assertThat((String) actualValue.get(attributeString.getKey()), is("attributeValue")); + + assertThat(actualValue, hasKey(attributeBoolean.getKey())); + assertThat(true, sameInstance(actualValue.get(attributeBoolean.getKey()))); + assertThat((Boolean) actualValue.get(attributeBoolean.getKey()), is(true)); + + assertThat(actualValue, hasKey(attributeInteger.getKey())); + assertThat(3, sameInstance(actualValue.get(attributeInteger.getKey()))); + assertThat((Integer) actualValue.get(attributeInteger.getKey()), is(3)); + + assertThat(actualValue, hasKey(attributeDouble.getKey())); + assertThat(3.123, sameInstance(actualValue.get(attributeDouble.getKey()))); + assertThat((Double) actualValue.get(attributeDouble.getKey()), is(3.123)); // verify that dispatchEvent was called with the correct LogEvent object verify(mockEventHandler).dispatchEvent(logEventToDispatch); From e49c473d8dd678354a1a748ff6d05ed86ae489a3 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 23 Aug 2018 11:09:55 -0700 Subject: [PATCH 09/18] diamond notation for hashmap --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 2 +- .../java/com/optimizely/ab/bucketing/DecisionService.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index eeb39e107..f9cb5cb71 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -766,7 +766,7 @@ public UserProfileService getUserProfileService() { if (unknownAttributes != null) { logger.warn("Attribute(s) {} not in the datafile.", unknownAttributes); // make a copy of the passed through attributes, then remove the unknown list - attributes = new HashMap(attributes); + attributes = new HashMap<>(attributes); for (String unknownAttribute : unknownAttributes) { attributes.remove(unknownAttribute); } diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 9ddadfbf8..222adc288 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -133,7 +133,7 @@ public DecisionService(@Nonnull Bucketer bucketer, if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) { bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); - logger.info("bucketingId has a valid type"); + logger.info("bucketingId is valid"); } else { logger.warn("bucketingID has type mismatch, defaulted to userId "); @@ -220,7 +220,7 @@ public DecisionService(@Nonnull Bucketer bucketer, if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) { bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); - logger.info("bucketingId has a valid type"); + logger.info("bucketingId is valid"); } else { logger.warn("bucketingID has type mismatch, defaulted to userId "); From 7c868cb33330a8edf3f1169eeaa762f626cd30c5 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Thu, 23 Aug 2018 13:26:56 -0700 Subject: [PATCH 10/18] funky casting to get Map working --- .../src/test/java/com/optimizely/ab/OptimizelyTest.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 7ee8cd3a6..0153b5cbb 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -43,6 +43,7 @@ import com.optimizely.ab.notification.NotificationCenter; import com.optimizely.ab.notification.TrackNotificationListener; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import org.hamcrest.Matcher; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -594,7 +595,7 @@ public void activateWithTypedAttributes() throws Exception { .build(); when(mockEventFactory.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(bucketedVariation), - eq(testUserId), anyMapOf(String.class, String.class))) + eq(testUserId), anyMapOf(String.class, Object.class))) .thenReturn(logEventToDispatch); when(mockBucketer.bucket(activatedExperiment, testUserId)) @@ -603,7 +604,7 @@ public void activateWithTypedAttributes() throws Exception { when(mockBucketer.bucket(activatedExperiment, testBucketingId)) .thenReturn(bucketedVariation); - Map attr = new HashMap(); + Map attr = new HashMap<>(); attr.put(attributeString.getKey(), "attributeValue"); attr.put(attributeBoolean.getKey(), true); @@ -624,8 +625,9 @@ public void activateWithTypedAttributes() throws Exception { verify(mockEventFactory).createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); - Map actualValue = attributeCaptor.getValue(); + Map actualValue = attributeCaptor.getValue(); + assertThat((Map)actualValue, hasEntry(attributeString.getKey(), "attributeValue")); assertThat(actualValue, hasKey(attributeString.getKey())); assertThat("attributeValue", sameInstance(actualValue.get(attributeString.getKey()))); assertThat((String) actualValue.get(attributeString.getKey()), is("attributeValue")); From 9f0593fd8966a63629369530b81540e64f6fe997 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 23 Aug 2018 13:34:16 -0700 Subject: [PATCH 11/18] fixed unit tests --- .../com/optimizely/ab/OptimizelyTest.java | 133 ++++++++---------- 1 file changed, 61 insertions(+), 72 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 0153b5cbb..da6d4f8f1 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -574,79 +574,68 @@ public void activateWithAttributes() throws Exception { } public void activateWithTypedAttributes() throws Exception { - if (datafileVersion >= 4) { - Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); - Variation bucketedVariation = activatedExperiment.getVariations().get(0); - Variation userIdBucketedVariation = activatedExperiment.getVariations().get(1); - Attribute attributeString = validProjectConfig.getAttributes().get(0); - Attribute attributeBoolean = validProjectConfig.getAttributes().get(3); - Attribute attributeInteger = validProjectConfig.getAttributes().get(4); - Attribute attributeDouble = validProjectConfig.getAttributes().get(5); - - - // setup a mock event builder to return expected impression params - EventFactory mockEventFactory = mock(EventFactory.class); - - Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) - .withBucketing(mockBucketer) - .withEventBuilder(mockEventFactory) - .withConfig(validProjectConfig) - .withErrorHandler(mockErrorHandler) - .build(); - - when(mockEventFactory.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(bucketedVariation), - eq(testUserId), anyMapOf(String.class, Object.class))) - .thenReturn(logEventToDispatch); - - when(mockBucketer.bucket(activatedExperiment, testUserId)) - .thenReturn(userIdBucketedVariation); - - when(mockBucketer.bucket(activatedExperiment, testBucketingId)) - .thenReturn(bucketedVariation); - - Map attr = new HashMap<>(); - - attr.put(attributeString.getKey(), "attributeValue"); - attr.put(attributeBoolean.getKey(), true); - attr.put(attributeInteger.getKey(), 3); - attr.put(attributeDouble.getKey(), 3.123); - attr.put(testBucketingIdKey, testBucketingId); - - // activate the experiment - Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, - attr); - - // verify that the bucketing algorithm was called correctly - verify(mockBucketer).bucket(activatedExperiment, testBucketingId); - assertThat(actualVariation, is(bucketedVariation)); - - // setup the attribute map captor (so we can verify its content) - ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); - verify(mockEventFactory).createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), - eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); - - Map actualValue = attributeCaptor.getValue(); - - assertThat((Map)actualValue, hasEntry(attributeString.getKey(), "attributeValue")); - assertThat(actualValue, hasKey(attributeString.getKey())); - assertThat("attributeValue", sameInstance(actualValue.get(attributeString.getKey()))); - assertThat((String) actualValue.get(attributeString.getKey()), is("attributeValue")); - - assertThat(actualValue, hasKey(attributeBoolean.getKey())); - assertThat(true, sameInstance(actualValue.get(attributeBoolean.getKey()))); - assertThat((Boolean) actualValue.get(attributeBoolean.getKey()), is(true)); - - assertThat(actualValue, hasKey(attributeInteger.getKey())); - assertThat(3, sameInstance(actualValue.get(attributeInteger.getKey()))); - assertThat((Integer) actualValue.get(attributeInteger.getKey()), is(3)); - - assertThat(actualValue, hasKey(attributeDouble.getKey())); - assertThat(3.123, sameInstance(actualValue.get(attributeDouble.getKey()))); - assertThat((Double) actualValue.get(attributeDouble.getKey()), is(3.123)); - - // verify that dispatchEvent was called with the correct LogEvent object - verify(mockEventHandler).dispatchEvent(logEventToDispatch); + if (datafileVersion < 4) { + return; } + + Experiment activatedExperiment = validProjectConfig.getExperiments().get(0); + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + Variation userIdBucketedVariation = activatedExperiment.getVariations().get(1); + Attribute attributeString = validProjectConfig.getAttributes().get(0); + Attribute attributeBoolean = validProjectConfig.getAttributes().get(3); + Attribute attributeInteger = validProjectConfig.getAttributes().get(4); + Attribute attributeDouble = validProjectConfig.getAttributes().get(5); + + // setup a mock event builder to return expected impression params + EventFactory mockEventFactory = mock(EventFactory.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventFactory) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + when(mockEventFactory.createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), eq(bucketedVariation), + eq(testUserId), anyMapOf(String.class, Object.class))) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, testUserId)) + .thenReturn(userIdBucketedVariation); + + when(mockBucketer.bucket(activatedExperiment, testBucketingId)) + .thenReturn(bucketedVariation); + + Map attr = new HashMap<>(); + + attr.put(attributeString.getKey(), "attributeValue"); + attr.put(attributeBoolean.getKey(), true); + attr.put(attributeInteger.getKey(), 3); + attr.put(attributeDouble.getKey(), 3.123); + attr.put(testBucketingIdKey, testBucketingId); + + // activate the experiment + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), testUserId, + attr); + + // verify that the bucketing algorithm was called correctly + verify(mockBucketer).bucket(activatedExperiment, testBucketingId); + assertThat(actualVariation, is(bucketedVariation)); + + // setup the attribute map captor (so we can verify its content) + ArgumentCaptor attributeCaptor = ArgumentCaptor.forClass(Map.class); + verify(mockEventFactory).createImpressionEvent(eq(validProjectConfig), eq(activatedExperiment), + eq(bucketedVariation), eq(testUserId), attributeCaptor.capture()); + + Map actualValue = attributeCaptor.getValue(); + + assertThat((Map)actualValue, hasEntry(attributeString.getKey(), "attributeValue")); + assertThat((Map)actualValue, hasEntry(attributeBoolean.getKey(), true)); + assertThat((Map)actualValue, hasEntry(attributeInteger.getKey(), 3)); + assertThat((Map)actualValue, hasEntry(attributeDouble.getKey(), 3.123)); + + // verify that dispatchEvent was called with the correct LogEvent object + verify(mockEventHandler).dispatchEvent(logEventToDispatch); } /** From b4849607c0852c96f17bfbe5c2fa372077c7be86 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 23 Aug 2018 14:04:49 -0700 Subject: [PATCH 12/18] notification listener, features function signature change --- .../java/com/optimizely/ab/Optimizely.java | 22 +++++++++---------- .../ActivateNotificationListener.java | 4 ++-- ...ActivateNotificationListenerInterface.java | 2 +- .../ab/notification/NotificationCenter.java | 4 ++-- .../TrackNotificationListener.java | 4 ++-- .../TrackNotificationListenerInterface.java | 2 +- .../com/optimizely/ab/OptimizelyTest.java | 1 - 7 files changed, 19 insertions(+), 20 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index f9cb5cb71..02be1399a 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -228,13 +228,13 @@ public void track(@Nonnull String eventName, public void track(@Nonnull String eventName, @Nonnull String userId, - @Nonnull Map attributes) throws UnknownEventTypeException { + @Nonnull Map attributes) throws UnknownEventTypeException { track(eventName, userId, attributes, Collections.emptyMap()); } public void track(@Nonnull String eventName, @Nonnull String userId, - @Nonnull Map attributes, + @Nonnull Map attributes, @Nonnull Map eventTags) throws UnknownEventTypeException { if (!validateUserId(userId)) { @@ -344,7 +344,7 @@ public void track(@Nonnull String eventName, */ public @Nonnull Boolean isFeatureEnabled(@Nonnull String featureKey, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { if (featureKey == null) { logger.warn("The featureKey parameter must be nonnull."); return false; @@ -410,7 +410,7 @@ else if (userId == null) { public @Nullable Boolean getFeatureVariableBoolean(@Nonnull String featureKey, @Nonnull String variableKey, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { String variableValue = getFeatureVariableValueForType( featureKey, variableKey, @@ -450,7 +450,7 @@ else if (userId == null) { public @Nullable Double getFeatureVariableDouble(@Nonnull String featureKey, @Nonnull String variableKey, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { String variableValue = getFeatureVariableValueForType( featureKey, variableKey, @@ -495,7 +495,7 @@ else if (userId == null) { public @Nullable Integer getFeatureVariableInteger(@Nonnull String featureKey, @Nonnull String variableKey, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { String variableValue = getFeatureVariableValueForType( featureKey, variableKey, @@ -540,7 +540,7 @@ else if (userId == null) { public @Nullable String getFeatureVariableString(@Nonnull String featureKey, @Nonnull String variableKey, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { return getFeatureVariableValueForType( featureKey, variableKey, @@ -553,7 +553,7 @@ else if (userId == null) { String getFeatureVariableValueForType(@Nonnull String featureKey, @Nonnull String variableKey, @Nonnull String userId, - @Nonnull Map attributes, + @Nonnull Map attributes, @Nonnull LiveVariable.VariableType variableType) { if (featureKey == null) { logger.warn("The featureKey parameter must be nonnull."); @@ -614,7 +614,7 @@ else if (userId == null) { * @return List of the feature keys that are enabled for the user if the userId is empty it will * return Empty List. */ - public List getEnabledFeatures(@Nonnull String userId, @Nonnull Map attributes) { + public List getEnabledFeatures(@Nonnull String userId, @Nonnull Map attributes) { List enabledFeaturesList = new ArrayList(); if (!validateUserId(userId)){ @@ -642,7 +642,7 @@ Variation getVariation(@Nonnull Experiment experiment, public @Nullable Variation getVariation(@Nonnull Experiment experiment, @Nonnull String userId, - @Nonnull Map attributes) throws UnknownExperimentException { + @Nonnull Map attributes) throws UnknownExperimentException { Map filteredAttributes = filterAttributes(projectConfig, attributes); @@ -659,7 +659,7 @@ Variation getVariation(@Nonnull String experimentKey, public @Nullable Variation getVariation(@Nonnull String experimentKey, @Nonnull String userId, - @Nonnull Map attributes) { + @Nonnull Map attributes) { if (!validateUserId(userId)) { return null; } diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java index 883bb6963..bb003cd25 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java @@ -38,7 +38,7 @@ public final void notify(Object... args) { assert(args[1] instanceof String); String userId = (String) args[1]; assert(args[2] instanceof java.util.Map); - Map attributes = (Map) args[2]; + Map attributes = (Map) args[2]; assert(args[3] instanceof Variation); Variation variation = (Variation) args[3]; assert(args[4] instanceof LogEvent); @@ -57,7 +57,7 @@ public final void notify(Object... args) { */ public abstract void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, - @Nonnull Map attributes, + @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) ; diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java index fdae1fc5d..866fa5a80 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListenerInterface.java @@ -18,7 +18,7 @@ public interface ActivateNotificationListenerInterface { */ public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, - @Nonnull Map attributes, + @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) ; diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java index 9b68c75ba..495ebac68 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationCenter.java @@ -94,7 +94,7 @@ public int addActivateNotificationListener(final ActivateNotificationListenerInt else { return addNotificationListener(NotificationType.Activate, new ActivateNotificationListener() { @Override - public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { activateNotificationListenerInterface.onActivate(experiment, userId, attributes, variation, event); } }); @@ -113,7 +113,7 @@ public int addTrackNotificationListener(final TrackNotificationListenerInterface else { return addNotificationListener(NotificationType.Track, new TrackNotificationListener() { @Override - public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { trackNotificationListenerInterface.onTrack(eventKey, userId, attributes, eventTags, event); } }); diff --git a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java index 16809f716..e52398acb 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java @@ -36,7 +36,7 @@ public final void notify(Object... args) { assert(args[1] instanceof String); String userId = (String) args[1]; assert(args[2] instanceof java.util.Map); - Map attributes = (Map) args[2]; + Map attributes = (Map) args[2]; assert(args[3] instanceof java.util.Map); Map eventTags = (Map) args[3]; assert(args[4] instanceof LogEvent); @@ -55,7 +55,7 @@ public final void notify(Object... args) { */ public abstract void onTrack(@Nonnull String eventKey, @Nonnull String userId, - @Nonnull Map attributes, + @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) ; } diff --git a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListenerInterface.java b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListenerInterface.java index 74df611dd..7a7f22c4e 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListenerInterface.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListenerInterface.java @@ -16,7 +16,7 @@ public interface TrackNotificationListenerInterface { */ public void onTrack(@Nonnull String eventKey, @Nonnull String userId, - @Nonnull Map attributes, + @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) ; diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index da6d4f8f1..83576954f 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -77,7 +77,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; -import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; From 4aa66fed9ee5893b7dc5443b49ff10bdb002b778 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 23 Aug 2018 15:33:51 -0700 Subject: [PATCH 13/18] notif. listener tests function signature change --- .../test/java/com/optimizely/ab/OptimizelyTest.java | 10 +++++----- .../ab/notification/NotificationCenterTest.java | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index 83576954f..eb648d83d 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -2389,11 +2389,11 @@ public void activateWithListener() throws Exception { ActivateNotificationListener activateNotification = new ActivateNotificationListener() { @Override - public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { assertEquals(experiment.getKey(), activatedExperiment.getKey()); assertEquals(bucketedVariation.getKey(), variation.getKey()); assertEquals(userId, testUserId); - for (Map.Entry entry : attributes.entrySet()) { + for (Map.Entry entry : attributes.entrySet()) { assertEquals(testUserAttributes.get(entry.getKey()), entry.getValue()); } @@ -2451,7 +2451,7 @@ public void activateWithListenerNullAttributes() throws Exception { ActivateNotificationListener activateNotification = new ActivateNotificationListener() { @Override - public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { assertEquals(experiment.getKey(), activatedExperiment.getKey()); assertEquals(bucketedVariation.getKey(), variation.getKey()); assertEquals(userId, testUserId); @@ -2786,7 +2786,7 @@ public void trackEventWithListenerAttributes() throws Exception { TrackNotificationListener trackNotification = new TrackNotificationListener() { @Override - public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map _attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map _attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { assertEquals(eventType.getKey(), eventKey); assertEquals(genericUserId, userId); assertEquals(attributes, _attributes); @@ -2870,7 +2870,7 @@ public void trackEventWithListenerNullAttributes() throws Exception { TrackNotificationListener trackNotification = new TrackNotificationListener() { @Override - public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { assertEquals(eventType.getKey(), eventKey); assertEquals(genericUserId, userId); assertTrue(attributes.isEmpty()); diff --git a/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java b/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java index 08958be64..cffa01094 100644 --- a/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java +++ b/core-api/src/test/java/com/optimizely/ab/notification/NotificationCenterTest.java @@ -54,7 +54,7 @@ public void testAddWrongActivateNotificationListener() { public void testAddActivateNotificationTwice() { ActivateNotificationListener listener = new ActivateNotificationListener() { @Override - public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { } }; @@ -70,7 +70,7 @@ public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @ public void testAddActivateNotification() { int notificationId = notificationCenter.addActivateNotificationListener(new ActivateNotificationListenerInterface() { @Override - public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { } }); @@ -83,7 +83,7 @@ public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @ public void testAddTrackNotification() { int notificationId = notificationCenter.addTrackNotificationListener(new TrackNotificationListenerInterface() { @Override - public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { } }); @@ -103,7 +103,7 @@ public void testNotificationTypeClasses() { public void testAddTrackNotificationInterface() { int notificationId = notificationCenter.addTrackNotificationListener(new TrackNotificationListenerInterface() { @Override - public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { + public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Map eventTags, @Nonnull LogEvent event) { } }); @@ -116,7 +116,7 @@ public void onTrack(@Nonnull String eventKey, @Nonnull String userId, @Nonnull M public void testAddActivateNotificationInterface() { int notificationId = notificationCenter.addActivateNotificationListener(new ActivateNotificationListenerInterface() { @Override - public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { + public void onActivate(@Nonnull Experiment experiment, @Nonnull String userId, @Nonnull Map attributes, @Nonnull Variation variation, @Nonnull LogEvent event) { } }); From 7e1dd58ab5c0e980fb66ac284dc9ac4332909605 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Wed, 29 Aug 2018 13:37:13 -0700 Subject: [PATCH 14/18] code review changes --- .../ab/bucketing/DecisionService.java | 43 ++++++++++--------- .../ab/config/audience/AndCondition.java | 2 +- .../ab/config/audience/Audience.java | 2 +- .../ab/config/audience/Condition.java | 2 +- .../ab/config/audience/NotCondition.java | 2 +- .../ab/config/audience/OrCondition.java | 2 +- .../ab/config/audience/UserAttribute.java | 4 +- .../ab/internal/ExperimentUtils.java | 2 +- .../ActivateNotificationListener.java | 2 +- .../ab/notification/NotificationListener.java | 2 +- .../TrackNotificationListener.java | 2 +- .../com/optimizely/ab/OptimizelyTest.java | 1 - .../AudienceConditionEvaluationTest.java | 24 +++++++++++ 13 files changed, 58 insertions(+), 32 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index 222adc288..ca01048a5 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -129,16 +129,7 @@ public DecisionService(@Nonnull Bucketer bucketer, } if (ExperimentUtils.isUserInExperiment(projectConfig, experiment, filteredAttributes)) { - String bucketingId = userId; - if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { - if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) { - bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); - logger.info("bucketingId is valid"); - } - else { - logger.warn("bucketingID has type mismatch, defaulted to userId "); - } - } + String bucketingId = getBucketingId(userId,filteredAttributes); variation = bucketer.bucket(experiment, bucketingId); if (variation != null) { @@ -216,16 +207,7 @@ public DecisionService(@Nonnull Bucketer bucketer, // for all rules before the everyone else rule int rolloutRulesLength = rollout.getExperiments().size(); - String bucketingId = userId; - if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { - if (String.class.isInstance(filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()))) { - bucketingId = (String) filteredAttributes.get(ControlAttribute.BUCKETING_ATTRIBUTE.toString()); - logger.info("bucketingId is valid"); - } - else { - logger.warn("bucketingID has type mismatch, defaulted to userId "); - } - } + String bucketingId = getBucketingId(userId, filteredAttributes); Variation variation; for (int i = 0; i < rolloutRulesLength - 1; i++) { Experiment rolloutRule = rollout.getExperiments().get(i); @@ -355,4 +337,25 @@ void saveVariation(@Nonnull Experiment experiment, } } } + + /** + * @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 + */ + String getBucketingId(@Nonnull String userId, + @Nonnull Map filteredAttributes) { + String bucketingId = userId; + if (filteredAttributes.containsKey(ControlAttribute.BUCKETING_ATTRIBUTE.toString())) { + 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 { + logger.warn("BucketingID attribute is not a string. Defaulted to userId"); + } + } + return bucketingId; + } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java index 300d40973..375205e42 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, Optimizely and contributors + * Copyright 2016-2018, 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. diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/Audience.java b/core-api/src/main/java/com/optimizely/ab/config/audience/Audience.java index 40bbbbe28..4a208bc4c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/Audience.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/Audience.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, Optimizely and contributors + * Copyright 2016-2018, 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. diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java index 319217c3b..c84f39aef 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, Optimizely and contributors + * Copyright 2016-2018, 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. diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java index f4e9f3c0e..84b8c010e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, Optimizely and contributors + * Copyright 2016-2018, 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. diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java index 13bc25e36..52a53c952 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, Optimizely and contributors + * Copyright 2016-2018, 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. diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index 077915ceb..c8627a4a1 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, Optimizely and contributors + * Copyright 2016-2018, 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. @@ -70,7 +70,7 @@ else if (userAttributeValue != null) { // if the datafile value is null but user public String toString() { return "{name='" + name + "\'" + ", type='" + type + "\'" + - ", value='" + value + "\'" + + ", value='" + value.toString() + "\'" + "}"; } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java index 0355a9f09..5ca27449e 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java @@ -1,6 +1,6 @@ /** * - * Copyright 2017, Optimizely and contributors + * Copyright 2017-2018, 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. diff --git a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java index bb003cd25..973c5d1ec 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/ActivateNotificationListener.java @@ -1,6 +1,6 @@ /** * - * Copyright 2017, Optimizely and contributors + * Copyright 2017-2018, 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. diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java index 687870979..be8f3b59e 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, Optimizely and contributors + * Copyright 2016-2018, 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. diff --git a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java index e52398acb..c45759efc 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/TrackNotificationListener.java @@ -1,6 +1,6 @@ /** * - * Copyright 2017, Optimizely and contributors + * Copyright 2017-2018, 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. diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index eb648d83d..a29a1fa12 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -43,7 +43,6 @@ import com.optimizely.ab.notification.NotificationCenter; import com.optimizely.ab.notification.TrackNotificationListener; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import org.hamcrest.Matcher; import org.junit.Before; import org.junit.Rule; import org.junit.Test; diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index d15bd70aa..74d4024e3 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -43,12 +43,19 @@ public class AudienceConditionEvaluationTest { Map testUserAttributes; + Map testTypedUserAttributes; @Before public void initialize() { testUserAttributes = new HashMap(); testUserAttributes.put("browser_type", "chrome"); testUserAttributes.put("device_type", "Android"); + + testTypedUserAttributes = new HashMap(); + testTypedUserAttributes.put("is_firefox", true); + testTypedUserAttributes.put("num_counts", 3.55); + testTypedUserAttributes.put("num_size", 3); + testTypedUserAttributes.put("meta_data", testUserAttributes); } /** @@ -60,6 +67,23 @@ public void userAttributeEvaluateTrue() throws Exception { assertTrue(testInstance.evaluate(testUserAttributes)); } + + /** + * Verify that UserAttribute.evaluate returns true on exact-matching visitor attribute data. + */ + @Test + public void typedUserAttributeEvaluateTrue() throws Exception { + UserAttribute testInstance = new UserAttribute("meta_data", "custom_dimension", testUserAttributes); + UserAttribute testInstance2 = new UserAttribute("is_firefox", "custom_dimension", true); + UserAttribute testInstance3 = new UserAttribute("num_counts", "custom_dimension", 3.55); + UserAttribute testInstance4 = new UserAttribute("num_size", "custom_dimension", 3); + + assertTrue(testInstance.evaluate(testTypedUserAttributes)); + assertTrue(testInstance2.evaluate(testTypedUserAttributes)); + assertTrue(testInstance3.evaluate(testTypedUserAttributes)); + assertTrue(testInstance4.evaluate(testTypedUserAttributes)); + } + /** * Verify that UserAttribute.evaluate returns false on non-exact-matching visitor attribute data. */ From a17bdbd8de849afb4fa2401525a9990e76a26fc9 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Wed, 29 Aug 2018 15:28:31 -0700 Subject: [PATCH 15/18] Update valid-project-config-v4.json --- .../src/test/resources/config/valid-project-config-v4.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core-api/src/test/resources/config/valid-project-config-v4.json b/core-api/src/test/resources/config/valid-project-config-v4.json index d5cc0720d..761fd97cc 100644 --- a/core-api/src/test/resources/config/valid-project-config-v4.json +++ b/core-api/src/test/resources/config/valid-project-config-v4.json @@ -35,7 +35,8 @@ { "id": "58339410", "key": "nationality" - }, { + }, + { "id": "583394100", "key": "$opt_test" }, From f90b466c2f0bb1d6e859383db724f3194ed13843 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Wed, 29 Aug 2018 15:29:04 -0700 Subject: [PATCH 16/18] Update valid-project-config-v4.json --- core-api/src/test/resources/config/valid-project-config-v4.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/test/resources/config/valid-project-config-v4.json b/core-api/src/test/resources/config/valid-project-config-v4.json index 761fd97cc..e2feb6e91 100644 --- a/core-api/src/test/resources/config/valid-project-config-v4.json +++ b/core-api/src/test/resources/config/valid-project-config-v4.json @@ -35,7 +35,7 @@ { "id": "58339410", "key": "nationality" - }, + }, { "id": "583394100", "key": "$opt_test" From d9d23e2e10ece635abfa9450e11000bc9ae17682 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 30 Aug 2018 10:36:05 -0700 Subject: [PATCH 17/18] small changes --- .../main/java/com/optimizely/ab/bucketing/DecisionService.java | 3 ++- .../main/java/com/optimizely/ab/config/audience/Audience.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index ca01048a5..5083f580b 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -129,7 +129,7 @@ public DecisionService(@Nonnull Bucketer bucketer, } if (ExperimentUtils.isUserInExperiment(projectConfig, experiment, filteredAttributes)) { - String bucketingId = getBucketingId(userId,filteredAttributes); + String bucketingId = getBucketingId(userId, filteredAttributes); variation = bucketer.bucket(experiment, bucketingId); if (variation != null) { @@ -339,6 +339,7 @@ void saveVariation(@Nonnull Experiment experiment, } /** + * 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 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. diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/Audience.java b/core-api/src/main/java/com/optimizely/ab/config/audience/Audience.java index 4a208bc4c..40bbbbe28 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/Audience.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/Audience.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2018, Optimizely and contributors + * Copyright 2016-2017, 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. From 182c4fec857b38ca62d6fc1b27c14801fe3653be Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 30 Aug 2018 13:11:08 -0700 Subject: [PATCH 18/18] Update NotificationListener.java --- .../com/optimizely/ab/notification/NotificationListener.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java b/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java index be8f3b59e..687870979 100644 --- a/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java +++ b/core-api/src/main/java/com/optimizely/ab/notification/NotificationListener.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2018, Optimizely and contributors + * Copyright 2016-2017, 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.