Skip to content

Commit

Permalink
Merge 8fb6b73 into 46651fa
Browse files Browse the repository at this point in the history
  • Loading branch information
ozayr-zaviar committed Apr 29, 2021
2 parents 46651fa + 8fb6b73 commit 55b5c19
Show file tree
Hide file tree
Showing 5 changed files with 357 additions and 123 deletions.
60 changes: 7 additions & 53 deletions optimizely/decision_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,39 +413,6 @@ def get_variation_for_rollout(self, project_config, rollout, user_id, attributes

return Decision(None, None, enums.DecisionSources.ROLLOUT), decide_reasons

def get_experiment_in_group(self, project_config, group, bucketing_id):
""" Determine which experiment in the group the user is bucketed into.
Args:
project_config: Instance of ProjectConfig.
group: The group to bucket the user into.
bucketing_id: ID to be used for bucketing the user.
Returns:
Experiment if the user is bucketed into an experiment in the specified group. None otherwise
and array of log messages representing decision making.
"""
decide_reasons = []
experiment_id = self.bucketer.find_bucket(
project_config, bucketing_id, group.id, group.trafficAllocation)
if experiment_id:
experiment = project_config.get_experiment_from_id(experiment_id)
if experiment:
message = 'User with bucketing ID "%s" is in experiment %s of group %s.' % \
(bucketing_id, experiment.key, group.id)
self.logger.info(
message
)
decide_reasons.append(message)
return experiment, decide_reasons
message = 'User with bucketing ID "%s" is not in any experiments of group %s.' % (bucketing_id, group.id)
self.logger.info(
message
)
decide_reasons.append(message)

return None, decide_reasons

def get_variation_for_feature(self, project_config, feature, user_id, attributes=None, ignore_user_profile=False):
""" Returns the experiment/variation the user is bucketed in for the given feature.
Expand All @@ -462,31 +429,18 @@ def get_variation_for_feature(self, project_config, feature, user_id, attributes
decide_reasons = []
bucketing_id, reasons = self._get_bucketing_id(user_id, attributes)
decide_reasons += reasons
# First check if the feature is in a mutex group
if feature.groupId:
group = project_config.get_group(feature.groupId)
if group:
experiment, reasons = self.get_experiment_in_group(project_config, group, bucketing_id)
decide_reasons += reasons
if experiment and experiment.id in feature.experimentIds:

# Check if the feature flag is under an experiment and the the user is bucketed into one of these experiments
if feature.experimentIds:
# Evaluate each experiment ID and return the first bucketed experiment variation
for experiment in feature.experimentIds:
experiment = project_config.get_experiment_from_id(experiment)
if experiment:
variation, variation_reasons = self.get_variation(
project_config, experiment, user_id, attributes, ignore_user_profile)
decide_reasons += variation_reasons
if variation:
return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST), decide_reasons
else:
self.logger.error(enums.Errors.INVALID_GROUP_ID.format('_get_variation_for_feature'))

# Next check if the feature is being experimented on
elif feature.experimentIds:
# If an experiment is not in a group, then the feature can only be associated with one experiment
experiment = project_config.get_experiment_from_id(feature.experimentIds[0])
if experiment:
variation, variation_reasons = self.get_variation(
project_config, experiment, user_id, attributes, ignore_user_profile)
decide_reasons += variation_reasons
if variation:
return Decision(experiment, variation, enums.DecisionSources.FEATURE_TEST), decide_reasons

# Next check if user is part of a rollout
if feature.rolloutId:
Expand Down
149 changes: 149 additions & 0 deletions tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,69 @@ def setUp(self, config_dict='config_dict'):
},
],
},
{
'key': 'test_experiment3',
'status': 'Running',
'layerId': '6',
'audienceIds': [],
'id': '111134',
'forcedVariations': {},
'trafficAllocation': [
{'entityId': '222239', 'endOfRange': 2500},
{'entityId': '', 'endOfRange': 5000},
{'entityId': '', 'endOfRange': 7500},
{'entityId': '', 'endOfRange': 10000}
],
'variations': [
{
'id': '222239',
'key': 'control',
'variables': [],
}
],
},
{
'key': 'test_experiment4',
'status': 'Running',
'layerId': '7',
'audienceIds': [],
'id': '111135',
'forcedVariations': {},
'trafficAllocation': [
{'entityId': '', 'endOfRange': 2500},
{'entityId': '222240', 'endOfRange': 5000},
{'entityId': '', 'endOfRange': 7500},
{'entityId': '', 'endOfRange': 10000}
],
'variations': [
{
'id': '222240',
'key': 'control',
'variables': [],
}
],
},
{
'key': 'test_experiment5',
'status': 'Running',
'layerId': '8',
'audienceIds': [],
'id': '111136',
'forcedVariations': {},
'trafficAllocation': [
{'entityId': '', 'endOfRange': 2500},
{'entityId': '', 'endOfRange': 5000},
{'entityId': '222241', 'endOfRange': 7500},
{'entityId': '', 'endOfRange': 10000}
],
'variations': [
{
'id': '222241',
'key': 'control',
'variables': [],
}
],
},
],
'groups': [
{
Expand Down Expand Up @@ -239,6 +302,72 @@ def setUp(self, config_dict='config_dict'):
{'entityId': '32222', "endOfRange": 3000},
{'entityId': '32223', 'endOfRange': 7500},
],
},
{
'id': '19229',
'policy': 'random',
'experiments': [
{
'id': '42222',
'key': 'group_2_exp_1',
'status': 'Running',
"audienceConditions": [
"or",
"11160"
],
'audienceIds': ['11160'],
'layerId': '211183',
'variations': [
{'key': 'var_1', 'id': '38901'},
],
'forcedVariations': {},
'trafficAllocation': [
{'entityId': '38901', 'endOfRange': 10000}
],
},
{
'id': '42223',
'key': 'group_2_exp_2',
'status': 'Running',
"audienceConditions": [
"or",
"11160"
],
'audienceIds': ['11160'],
'layerId': '211184',
'variations': [
{'key': 'var_1', 'id': '38905'}
],
'forcedVariations': {},
'trafficAllocation': [
{'entityId': '38905', 'endOfRange': 10000}
],
},
{
'id': '42224',
'key': 'group_2_exp_3',
'status': 'Running',
"audienceConditions": [
"or",
"11160"
],
'audienceIds': ['11160'],
'layerId': '211185',
'variations': [
{'key': 'var_1', 'id': '38906'}
],
'forcedVariations': {},
'trafficAllocation': [
{'entityId': '38906', 'endOfRange': 10000}
],
}
],
'trafficAllocation': [
{'entityId': '42222', "endOfRange": 2500},
{'entityId': '42223', 'endOfRange': 5000},
{'entityId': '42224', "endOfRange": 7500},
{'entityId': '', 'endOfRange': 10000},
],
}
],
'attributes': [{'key': 'test_attribute', 'id': '111094'}],
Expand All @@ -255,6 +384,12 @@ def setUp(self, config_dict='config_dict'):
'{"name": "test_attribute", "type": "custom_attribute", "value": "test_value_2"}]]]',
'id': '11159',
},
{
'name': 'Test attribute users 3',
'conditions': "[\"and\", [\"or\", [\"or\", {\"match\": \"exact\", \"name\": \
\"experiment_attr\", \"type\": \"custom_attribute\", \"value\": \"group_experiment\"}]]]",
'id': '11160',
}
],
'rollouts': [
{'id': '201111', 'experiments': []},
Expand Down Expand Up @@ -364,6 +499,20 @@ def setUp(self, config_dict='config_dict'):
'rolloutId': '211111',
'variables': [],
},
{
'id': '91115',
'key': 'test_feature_in_exclusion_group',
'experimentIds': ['42222', '42223', '42224'],
'rolloutId': '211111',
'variables': [],
},
{
'id': '91116',
'key': 'test_feature_in_multiple_groups',
'experimentIds': ['111134', '111135', '111136'],
'rolloutId': '211111',
'variables': [],
},
],
}

Expand Down
81 changes: 12 additions & 69 deletions tests/test_decision_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -1277,7 +1277,6 @@ def test_get_variation_for_feature__returns_variation_if_user_not_in_experiment_
side_effect=[[False, []], [True, []]],
) as mock_audience_check, self.mock_decision_logger as mock_decision_service_logging, mock.patch(
"optimizely.bucketer.Bucketer.bucket", return_value=[expected_variation, []]):

decision, _ = self.decision_service.get_variation_for_feature(
self.project_config, feature, "test_user"
)
Expand Down Expand Up @@ -1320,9 +1319,6 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_group(self)
"group_exp_1", "28901"
)
with mock.patch(
"optimizely.decision_service.DecisionService.get_experiment_in_group",
return_value=(self.project_config.get_experiment_from_key("group_exp_1"), []),
) as mock_get_experiment_in_group, mock.patch(
"optimizely.decision_service.DecisionService.get_variation",
return_value=(expected_variation, []),
) as mock_decision:
Expand All @@ -1338,9 +1334,6 @@ def test_get_variation_for_feature__returns_variation_for_feature_in_group(self)
variation_received,
)

mock_get_experiment_in_group.assert_called_once_with(
self.project_config, self.project_config.get_group("19228"), 'test_user')

mock_decision.assert_called_once_with(
self.project_config,
self.project_config.get_experiment_from_key("group_exp_1"),
Expand All @@ -1356,11 +1349,9 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_group(self):
feature = self.project_config.get_feature_from_key("test_feature_in_group")

with mock.patch(
"optimizely.decision_service.DecisionService.get_experiment_in_group",
return_value=[None, []],
) as mock_get_experiment_in_group, mock.patch(
"optimizely.decision_service.DecisionService.get_variation"
) as mock_decision:
"optimizely.decision_service.DecisionService.get_variation",
return_value=[None, []],
):
variation_received, _ = self.decision_service.get_variation_for_feature(
self.project_config, feature, "test_user"
)
Expand All @@ -1369,11 +1360,6 @@ def test_get_variation_for_feature__returns_none_for_user_not_in_group(self):
variation_received,
)

mock_get_experiment_in_group.assert_called_once_with(
self.project_config, self.project_config.get_group("19228"), "test_user")

self.assertFalse(mock_decision.called)

def test_get_variation_for_feature__returns_none_for_user_not_in_experiment(self):
""" Test that get_variation_for_feature returns None for user not in the associated experiment. """

Expand Down Expand Up @@ -1405,16 +1391,12 @@ def test_get_variation_for_feature__returns_none_for_invalid_group_id(self):
feature = self.project_config.get_feature_from_key("test_feature_in_group")
feature.groupId = "aabbccdd"

with self.mock_decision_logger as mock_decision_service_logging:
variation_received, _ = self.decision_service.get_variation_for_feature(
self.project_config, feature, "test_user"
)
self.assertEqual(
decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT),
variation_received,
)
mock_decision_service_logging.error.assert_called_once_with(
enums.Errors.INVALID_GROUP_ID.format("_get_variation_for_feature")
variation_received, _ = self.decision_service.get_variation_for_feature(
self.project_config, feature, "test_user"
)
self.assertEqual(
decision_service.Decision(None, None, enums.DecisionSources.ROLLOUT),
variation_received,
)

def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_not_associated_with_feature(
Expand All @@ -1424,10 +1406,9 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no
not targeting a feature, then None is returned. """

feature = self.project_config.get_feature_from_key("test_feature_in_group")

with mock.patch(
"optimizely.decision_service.DecisionService.get_experiment_in_group",
return_value=[self.project_config.get_experiment_from_key("group_exp_2"), []],
"optimizely.decision_service.DecisionService.get_variation",
return_value=[None, []],
) as mock_decision:
variation_received, _ = self.decision_service.get_variation_for_feature(
self.project_config, feature, "test_user"
Expand All @@ -1438,43 +1419,5 @@ def test_get_variation_for_feature__returns_none_for_user_in_group_experiment_no
)

mock_decision.assert_called_once_with(
self.project_config, self.project_config.get_group("19228"), "test_user"
)

def test_get_experiment_in_group(self):
""" Test that get_experiment_in_group returns the bucketed experiment for the user. """

group = self.project_config.get_group("19228")
experiment = self.project_config.get_experiment_from_id("32222")
with mock.patch(
"optimizely.bucketer.Bucketer.find_bucket", return_value="32222"
), self.mock_decision_logger as mock_decision_service_logging:
variation_received, _ = self.decision_service.get_experiment_in_group(
self.project_config, group, "test_user"
)
self.assertEqual(
experiment,
variation_received,
)

mock_decision_service_logging.info.assert_called_once_with(
'User with bucketing ID "test_user" is in experiment group_exp_1 of group 19228.'
)

def test_get_experiment_in_group__returns_none_if_user_not_in_group(self):
""" Test that get_experiment_in_group returns None if the user is not bucketed into the group. """

group = self.project_config.get_group("19228")
with mock.patch(
"optimizely.bucketer.Bucketer.find_bucket", return_value=None
), self.mock_decision_logger as mock_decision_service_logging:
variation_received, _ = self.decision_service.get_experiment_in_group(
self.project_config, group, "test_user"
)
self.assertIsNone(
variation_received
)

mock_decision_service_logging.info.assert_called_once_with(
'User with bucketing ID "test_user" is not in any experiments of group 19228.'
self.project_config, self.project_config.get_experiment_from_id("32222"), "test_user", None, False
)
Loading

0 comments on commit 55b5c19

Please sign in to comment.