Skip to content

feat: Add ODP Segments support in Audience Evaluation #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Jul 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2ba5621
OASIS-8309 Added qualified segments to User Context, updated evaluate…
genemitelmanopti May 23, 2022
6d1924a
OASIS-8309 Updated UserAttribute evaluate to check qualified segments.
genemitelmanopti May 24, 2022
0d9d3b8
Added unit tests and fixed issues
NomanShoaib Jun 16, 2022
a8d0c3f
Updated header
NomanShoaib Jun 16, 2022
b461f82
resolved comments
NomanShoaib Jun 21, 2022
8ccd995
Resolved spotbugs error
NomanShoaib Jun 21, 2022
f449dda
removed unnecessary null check
NomanShoaib Jun 21, 2022
a9c8d05
Merge branch 'master' into gene/ats
mnoman09 Jun 27, 2022
315a9c9
Comment added
NomanShoaib Jun 28, 2022
0469623
1. Implemented parsing of integrations in all four Datafile Parsers.
zashraf1985 Jul 16, 2022
15bfe93
Update core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext…
mnoman09 Jul 18, 2022
b8eaae1
Resolved comment and fixed
NomanShoaib Jul 18, 2022
eb2e8c8
trying to fix a test
zashraf1985 Jul 18, 2022
1ba1b3e
trying ignoing some tests
zashraf1985 Jul 18, 2022
9910be8
IGNORED the correct test
zashraf1985 Jul 18, 2022
6af28aa
Merge branch 'master' into gene/ats
msohailhussain Jul 18, 2022
226d3ae
Enabled ignored tests
zashraf1985 Jul 19, 2022
24396e9
test
mnoman09 Jul 19, 2022
12d92ee
test
mnoman09 Jul 19, 2022
889e405
Reverting assertEqual to assertThat
mnoman09 Jul 19, 2022
29750c4
test
mnoman09 Jul 19, 2022
cc4ee31
test
mnoman09 Jul 19, 2022
4f61e54
using macos as in git action there is some problem with ubuntu 18.04
mnoman09 Jul 19, 2022
99a7f0a
change os version to macos latest for unit test
mnoman09 Jul 19, 2022
aad5d0b
added integrations to parsing tests
zashraf1985 Jul 19, 2022
c0f0284
Incorporate review feedback
zashraf1985 Jul 20, 2022
d6ec898
Added unit tests for integrations parsing
zashraf1985 Jul 21, 2022
5df10d4
Added some more unit tests
zashraf1985 Jul 21, 2022
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
2 changes: 1 addition & 1 deletion .github/workflows/java.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:

test:
if: startsWith(github.ref, 'refs/tags/') != true
runs-on: ubuntu-18.04
runs-on: macos-latest
Copy link
Contributor

Choose a reason for hiding this comment

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

why macos?

Copy link
Contributor

Choose a reason for hiding this comment

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

Because some tests were failing for no apparent reason on linux. we have changes it to macox for now. will fix it later as part of a separate ticket

strategy:
fail-fast: false
matrix:
Expand Down
35 changes: 25 additions & 10 deletions core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ public class OptimizelyUserContext {
@Nonnull
private final Map<String, Object> attributes;

private List<String> qualifiedSegments;

@Nonnull
private final Optimizely optimizely;

Expand All @@ -44,19 +46,14 @@ public class OptimizelyUserContext {
public OptimizelyUserContext(@Nonnull Optimizely optimizely,
@Nonnull String userId,
@Nonnull Map<String, ?> attributes) {
this.optimizely = optimizely;
this.userId = userId;
if (attributes != null) {
this.attributes = Collections.synchronizedMap(new HashMap<>(attributes));
} else {
this.attributes = Collections.synchronizedMap(new HashMap<>());
}
this(optimizely, userId, attributes, Collections.EMPTY_MAP, null);
}

public OptimizelyUserContext(@Nonnull Optimizely optimizely,
@Nonnull String userId,
@Nonnull Map<String, ?> attributes,
@Nullable Map<String, OptimizelyForcedDecision> forcedDecisionsMap) {
@Nullable Map<String, OptimizelyForcedDecision> forcedDecisionsMap,
@Nullable List<String> qualifiedSegments) {
this.optimizely = optimizely;
this.userId = userId;
if (attributes != null) {
Expand All @@ -65,8 +62,10 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely,
this.attributes = Collections.synchronizedMap(new HashMap<>());
}
if (forcedDecisionsMap != null) {
this.forcedDecisionsMap = new ConcurrentHashMap<>(forcedDecisionsMap);
this.forcedDecisionsMap = new ConcurrentHashMap<>(forcedDecisionsMap);
}

this.qualifiedSegments = Collections.synchronizedList( qualifiedSegments == null ? new LinkedList<>(): qualifiedSegments);
}

public OptimizelyUserContext(@Nonnull Optimizely optimizely, @Nonnull String userId) {
Expand All @@ -86,7 +85,16 @@ public Optimizely getOptimizely() {
}

public OptimizelyUserContext copy() {
return new OptimizelyUserContext(optimizely, userId, attributes, forcedDecisionsMap);
return new OptimizelyUserContext(optimizely, userId, attributes, forcedDecisionsMap, qualifiedSegments);
}

/**
* Returns true if the user is qualified for the given segment name
* @param segment A String segment key which will be checked in the qualified segments list that if it exists then user is qualified.
* @return boolean Is user qualified for a segment.
*/
public boolean isQualifiedFor(@Nonnull String segment) {
return qualifiedSegments.contains(segment);
}

/**
Expand Down Expand Up @@ -265,7 +273,14 @@ public boolean removeAllForcedDecisions() {
return true;
}

public List<String> getQualifiedSegments() {
return qualifiedSegments;
}

public void setQualifiedSegments(List<String> qualifiedSegments) {
this.qualifiedSegments.clear();
this.qualifiedSegments.addAll(qualifiedSegments);
}

// Utils

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
}
}

DecisionResponse<Boolean> decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, experiment, user.getAttributes(), EXPERIMENT, experiment.getKey());
DecisionResponse<Boolean> decisionMeetAudience = ExperimentUtils.doesUserMeetAudienceConditions(projectConfig, experiment, user, EXPERIMENT, experiment.getKey());
reasons.merge(decisionMeetAudience.getReasons());
if (decisionMeetAudience.getResult()) {
String bucketingId = getBucketingId(user.getUserId(), user.getAttributes());
Expand Down Expand Up @@ -693,7 +693,7 @@ DecisionResponse<AbstractMap.SimpleEntry> getVariationFromDeliveryRule(@Nonnull
DecisionResponse<Boolean> audienceDecisionResponse = ExperimentUtils.doesUserMeetAudienceConditions(
projectConfig,
rule,
user.getAttributes(),
user,
RULE,
String.valueOf(ruleIndex + 1)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ public class DatafileProjectConfig implements ProjectConfig {
private final boolean anonymizeIP;
private final boolean sendFlagDecisions;
private final Boolean botFiltering;
private final String hostForODP;
private final String publicKeyForODP;
Comment on lines +66 to +67

Choose a reason for hiding this comment

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

For acronyms in names, are we using ODP or Odp ?

private final List<Attribute> attributes;
private final List<Audience> audiences;
private final List<Audience> typedAudiences;
Expand All @@ -71,6 +73,7 @@ public class DatafileProjectConfig implements ProjectConfig {
private final List<FeatureFlag> featureFlags;
private final List<Group> groups;
private final List<Rollout> rollouts;
private final List<Integration> integrations;

// key to entity mappings
private final Map<String, Attribute> attributeKeyMapping;
Expand Down Expand Up @@ -121,6 +124,7 @@ public DatafileProjectConfig(String accountId, String projectId, String version,
experiments,
null,
groups,
null,
null
);
}
Expand All @@ -142,8 +146,8 @@ public DatafileProjectConfig(String accountId,
List<Experiment> experiments,
List<FeatureFlag> featureFlags,
List<Group> groups,
List<Rollout> rollouts) {

List<Rollout> rollouts,
List<Integration> integrations) {
this.accountId = accountId;
this.projectId = projectId;
this.version = version;
Expand Down Expand Up @@ -182,6 +186,24 @@ public DatafileProjectConfig(String accountId,
allExperiments.addAll(aggregateGroupExperiments(groups));
this.experiments = Collections.unmodifiableList(allExperiments);

String publicKeyForODP = "";
String hostForODP = "";
if (integrations == null) {
this.integrations = Collections.emptyList();
} else {
this.integrations = Collections.unmodifiableList(integrations);
for (Integration integration: this.integrations) {
if (integration.getKey().equals("odp")) {
hostForODP = integration.getHost();
publicKeyForODP = integration.getPublicKey();
break;
}
}
}

this.publicKeyForODP = publicKeyForODP;
this.hostForODP = hostForODP;

Map<String, Experiment> variationIdToExperimentMap = new HashMap<String, Experiment>();
for (Experiment experiment : this.experiments) {
for (Variation variation : experiment.getVariations()) {
Expand Down Expand Up @@ -448,6 +470,11 @@ public List<Audience> getTypedAudiences() {
return typedAudiences;
}

@Override
public List<Integration> getIntegrations() {
return integrations;
}

@Override
public Audience getAudience(String audienceId) {
return audienceIdMapping.get(audienceId);
Expand Down Expand Up @@ -524,6 +551,16 @@ public Variation getFlagVariationByKey(String flagKey, String variationKey) {
return null;
}

@Override
public String getHostForODP() {
return hostForODP;
}

@Override
public String getPublicKeyForODP() {
return publicKeyForODP;
}

@Override
public String toString() {
return "ProjectConfig{" +
Expand Down
67 changes: 67 additions & 0 deletions core-api/src/main/java/com/optimizely/ab/config/Integration.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/**
*
* Copyright 2022, 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.optimizely.ab.config;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable;

/**
* Represents the Optimizely Integration configuration.
*
* @see <a href="http://developers.optimizely.com/server/reference/index.html#json">Project JSON</a>
*/
@Immutable
@JsonIgnoreProperties(ignoreUnknown = true)
public class Integration {
private final String key;
private final String host;
private final String publicKey;

@JsonCreator
public Integration(@JsonProperty("key") String key,
@JsonProperty("host") String host,
@JsonProperty("publicKey") String publicKey) {
this.key = key;
this.host = host;
this.publicKey = publicKey;
}

@Nonnull
public String getKey() {
Copy link
Contributor

Choose a reason for hiding this comment

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

return key;
}

@Nullable
public String getHost() { return host; }
Copy link
Contributor

Choose a reason for hiding this comment

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


@Nullable
public String getPublicKey() { return publicKey; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it is nullable in java unless specified the other way round?


@Override
public String toString() {
return "Integration{" +
"key='" + key + '\'' +
((this.host != null) ? (", host='" + host + '\'') : "") +
((this.publicKey != null) ? (", publicKey='" + publicKey + '\'') : "") +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ Experiment getExperimentForKey(@Nonnull String experimentKey,

List<Audience> getTypedAudiences();

List<Integration> getIntegrations();

Audience getAudience(String audienceId);

Map<String, Experiment> getExperimentKeyMapping();
Expand All @@ -107,6 +109,10 @@ Experiment getExperimentForKey(@Nonnull String experimentKey,

Variation getFlagVariationByKey(String flagKey, String variationKey);

String getHostForODP();

String getPublicKeyForODP();

@Override
String toString();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2016-2019, Optimizely and contributors
* Copyright 2016-2019, 2022, 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 All @@ -16,6 +16,7 @@
*/
package com.optimizely.ab.config.audience;

import com.optimizely.ab.OptimizelyUserContext;
import com.optimizely.ab.config.ProjectConfig;

import javax.annotation.Nonnull;
Expand All @@ -42,7 +43,7 @@ public List<Condition> getConditions() {
}

@Nullable
public Boolean evaluate(ProjectConfig config, Map<String, ?> attributes) {
public Boolean evaluate(ProjectConfig config, OptimizelyUserContext user) {
if (conditions == null) return null;
boolean foundNull = false;
// According to the matrix where:
Expand All @@ -53,7 +54,7 @@ public Boolean evaluate(ProjectConfig config, Map<String, ?> attributes) {
// true and true is true
// null and null is null
for (Condition condition : conditions) {
Boolean conditionEval = condition.evaluate(config, attributes);
Boolean conditionEval = condition.evaluate(config, user);
if (conditionEval == null) {
foundNull = true;
} else if (!conditionEval) { // false with nulls or trues is false.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
/**
*
* Copyright 2022, 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package com.optimizely.ab.config.audience;

public enum AttributeType {
CUSTOM_ATTRIBUTE("custom_attribute"),
THIRD_PARTY_DIMENSION("third_party_dimension");

private final String key;

AttributeType(String key) {
this.key = key;
}

@Override
public String toString() {
return key;
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
/**
*
* Copyright 2018-2021, Optimizely and contributors
* Copyright 2018-2022, 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
* https://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
Expand All @@ -19,6 +19,7 @@
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
import com.fasterxml.jackson.annotation.JsonProperty;
import com.optimizely.ab.OptimizelyUserContext;
import com.optimizely.ab.config.ProjectConfig;
import com.optimizely.ab.internal.InvalidAudienceCondition;
import org.slf4j.Logger;
Expand Down Expand Up @@ -71,7 +72,7 @@ public String getOperandOrId() {

@Nullable
@Override
public Boolean evaluate(ProjectConfig config, Map<String, ?> attributes) {
public Boolean evaluate(ProjectConfig config, OptimizelyUserContext user) {
if (config != null) {
audience = config.getAudienceIdMapping().get(audienceId);
}
Expand All @@ -80,7 +81,7 @@ public Boolean evaluate(ProjectConfig config, Map<String, ?> attributes) {
return null;
}
logger.debug("Starting to evaluate audience \"{}\" with conditions: {}.", audience.getId(), audience.getConditions());
Boolean result = audience.getConditions().evaluate(config, attributes);
Boolean result = audience.getConditions().evaluate(config, user);
logger.debug("Audience \"{}\" evaluated to {}.", audience.getId(), result);
return result;
}
Expand Down
Loading