Skip to content

Commit 1f34b2a

Browse files
rashidspaliabbasrizvi
rashidsp
authored and
aliabbasrizvi
committed
feat(api): Feature variable APIs return default variable value when featureEnabled property is false. (#171)
1 parent 92490ee commit 1f34b2a

File tree

4 files changed

+229
-14
lines changed

4 files changed

+229
-14
lines changed

Diff for: ‎optimizely/optimizely.py

+15-2
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,25 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ
208208
)
209209
return None
210210

211+
variable_value = variable.defaultValue
212+
211213
decision = self.decision_service.get_variation_for_feature(feature_flag, user_id, attributes)
212214
if decision.variation:
213-
variable_value = self.config.get_variable_value_for_variation(variable, decision.variation)
214215

216+
feature_enabled = decision.variation.featureEnabled
217+
if feature_enabled:
218+
variable_value = self.config.get_variable_value_for_variation(variable, decision.variation)
219+
self.logger.info(
220+
'Got variable value "%s" for variable "%s" of feature flag "%s".' % (
221+
variable_value, variable_key, feature_key
222+
)
223+
)
224+
else:
225+
self.logger.info(
226+
'Feature "%s" for variation "%s" is not enabled. '
227+
'Returning the default variable value "%s".' % (feature_key, decision.variation.key, variable_value)
228+
)
215229
else:
216-
variable_value = variable.defaultValue
217230
self.logger.info(
218231
'User "%s" is not in any variation or rollout rule. '
219232
'Returning default value for variable "%s" of feature flag "%s".' % (user_id, variable_key, feature_key)

Diff for: ‎tests/base.py

+45-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright 2016-2018, Optimizely
1+
# Copyright 2016-2019, Optimizely
22
# Licensed under the Apache License, Version 2.0 (the "License");
33
# you may not use this file except in compliance with the License.
44
# You may obtain a copy of the License at
@@ -307,7 +307,29 @@ def setUp(self, config_dict='config_dict'):
307307
'variations': [{
308308
'key': '211129',
309309
'id': '211129',
310-
'featureEnabled': True
310+
'featureEnabled': True,
311+
'variables': [{
312+
'id': '132', 'value': 'true'
313+
}, {
314+
'id': '135', 'value': '395'
315+
}, {
316+
'id': '134', 'value': '39.99'
317+
}, {
318+
'id': '133', 'value': 'Hello audience'
319+
}]
320+
}, {
321+
'key': '211229',
322+
'id': '211229',
323+
'featureEnabled': False,
324+
'variables': [{
325+
'id': '132', 'value': 'true'
326+
}, {
327+
'id': '135', 'value': '395'
328+
}, {
329+
'id': '134', 'value': '39.99'
330+
}, {
331+
'id': '133', 'value': 'Hello audience'
332+
}]
311333
}]
312334
}, {
313335
'id': '211137',
@@ -379,7 +401,27 @@ def setUp(self, config_dict='config_dict'):
379401
'key': 'test_feature_in_rollout',
380402
'experimentIds': [],
381403
'rolloutId': '211111',
382-
'variables': [],
404+
'variables': [{
405+
'id': '132',
406+
'key': 'is_running',
407+
'defaultValue': 'false',
408+
'type': 'boolean'
409+
}, {
410+
'id': '133',
411+
'key': 'message',
412+
'defaultValue': 'Hello',
413+
'type': 'string'
414+
}, {
415+
'id': '134',
416+
'key': 'price',
417+
'defaultValue': '99.99',
418+
'type': 'double'
419+
}, {
420+
'id': '135',
421+
'key': 'count',
422+
'defaultValue': '999',
423+
'type': 'integer'
424+
}]
383425
}, {
384426
'id': '91113',
385427
'key': 'test_feature_in_group',

Diff for: ‎tests/test_config.py

+37-3
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright 2016-2018, Optimizely
1+
# Copyright 2016-2019, Optimizely
22
# Licensed under the Apache License, Version 2.0 (the "License");
33
# you may not use this file except in compliance with the License.
44
# You may obtain a copy of the License at
@@ -885,7 +885,19 @@ def test_get_feature_from_key__valid_feature_key(self):
885885
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
886886
project_config = opt_obj.config
887887

888-
expected_feature = entities.FeatureFlag('91112', 'test_feature_in_rollout', [], '211111', {})
888+
expected_feature = entities.FeatureFlag(
889+
'91112',
890+
'test_feature_in_rollout',
891+
[],
892+
'211111',
893+
{
894+
'is_running': entities.Variable('132', 'is_running', 'boolean', 'false'),
895+
'message': entities.Variable('133', 'message', 'string', 'Hello'),
896+
'price': entities.Variable('134', 'price', 'double', '99.99'),
897+
'count': entities.Variable('135', 'count', 'integer', '999')
898+
}
899+
)
900+
889901
self.assertEqual(expected_feature, project_config.get_feature_from_key('test_feature_in_rollout'))
890902

891903
def test_get_feature_from_key__invalid_feature_key(self):
@@ -916,7 +928,29 @@ def test_get_rollout_from_id__valid_rollout_id(self):
916928
'variations': [{
917929
'key': '211129',
918930
'id': '211129',
919-
'featureEnabled': True
931+
'featureEnabled': True,
932+
'variables': [{
933+
'id': '132', 'value': 'true'
934+
}, {
935+
'id': '135', 'value': '395'
936+
}, {
937+
'id': '134', 'value': '39.99'
938+
}, {
939+
'id': '133', 'value': 'Hello audience'
940+
}]
941+
}, {
942+
'key': '211229',
943+
'id': '211229',
944+
'featureEnabled': False,
945+
'variables': [{
946+
'id': '132', 'value': 'true'
947+
}, {
948+
'id': '135', 'value': '395'
949+
}, {
950+
'id': '134', 'value': '39.99'
951+
}, {
952+
'id': '133', 'value': 'Hello audience'
953+
}]
920954
}]
921955
}, {
922956
'id': '211137',

Diff for: ‎tests/test_optimizely.py

+132-6
Original file line numberDiff line numberDiff line change
@@ -2242,6 +2242,122 @@ def test_get_feature_variable__returns_none_if_invalid_variable_key(self):
22422242
mock.call('Variable with key "invalid_variable" not found in the datafile.')
22432243
])
22442244

2245+
def test_get_feature_variable__returns_default_value_if_feature_not_enabled(self):
2246+
""" Test that get_feature_variable_* returns default value if feature is not enabled for the user. """
2247+
2248+
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
2249+
mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment')
2250+
mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111128')
2251+
2252+
# Boolean
2253+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2254+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2255+
decision_service.DECISION_SOURCE_EXPERIMENT)), \
2256+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2257+
2258+
self.assertTrue(opt_obj.get_feature_variable_boolean('test_feature_in_experiment', 'is_working', 'test_user'))
2259+
2260+
mock_client_logger.info.assert_called_once_with(
2261+
'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
2262+
'Returning the default variable value "true".'
2263+
)
2264+
2265+
# Double
2266+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2267+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2268+
decision_service.DECISION_SOURCE_EXPERIMENT)), \
2269+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2270+
self.assertEqual(10.99,
2271+
opt_obj.get_feature_variable_double('test_feature_in_experiment', 'cost', 'test_user'))
2272+
2273+
mock_client_logger.info.assert_called_once_with(
2274+
'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
2275+
'Returning the default variable value "10.99".'
2276+
)
2277+
2278+
# Integer
2279+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2280+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2281+
decision_service.DECISION_SOURCE_EXPERIMENT)), \
2282+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2283+
self.assertEqual(999,
2284+
opt_obj.get_feature_variable_integer('test_feature_in_experiment', 'count', 'test_user'))
2285+
2286+
mock_client_logger.info.assert_called_once_with(
2287+
'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
2288+
'Returning the default variable value "999".'
2289+
)
2290+
2291+
# String
2292+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2293+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2294+
decision_service.DECISION_SOURCE_EXPERIMENT)), \
2295+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2296+
self.assertEqual('devel',
2297+
opt_obj.get_feature_variable_string('test_feature_in_experiment', 'environment', 'test_user'))
2298+
2299+
mock_client_logger.info.assert_called_once_with(
2300+
'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
2301+
'Returning the default variable value "devel".'
2302+
)
2303+
2304+
def test_get_feature_variable__returns_default_value_if_feature_not_enabled_in_rollout(self):
2305+
""" Test that get_feature_variable_* returns default value if feature is not enabled for the user. """
2306+
2307+
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
2308+
mock_experiment = opt_obj.config.get_experiment_from_key('211127')
2309+
mock_variation = opt_obj.config.get_variation_from_id('211127', '211229')
2310+
2311+
# Boolean
2312+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2313+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2314+
decision_service.DECISION_SOURCE_ROLLOUT)), \
2315+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2316+
self.assertFalse(opt_obj.get_feature_variable_boolean('test_feature_in_rollout', 'is_running', 'test_user'))
2317+
2318+
mock_client_logger.info.assert_called_once_with(
2319+
'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
2320+
'Returning the default variable value "false".'
2321+
)
2322+
2323+
# Double
2324+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2325+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2326+
decision_service.DECISION_SOURCE_ROLLOUT)), \
2327+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2328+
self.assertEqual(99.99,
2329+
opt_obj.get_feature_variable_double('test_feature_in_rollout', 'price', 'test_user'))
2330+
2331+
mock_client_logger.info.assert_called_once_with(
2332+
'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
2333+
'Returning the default variable value "99.99".'
2334+
)
2335+
2336+
# Integer
2337+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2338+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2339+
decision_service.DECISION_SOURCE_ROLLOUT)), \
2340+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2341+
self.assertEqual(999,
2342+
opt_obj.get_feature_variable_integer('test_feature_in_rollout', 'count', 'test_user'))
2343+
2344+
mock_client_logger.info.assert_called_once_with(
2345+
'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
2346+
'Returning the default variable value "999".'
2347+
)
2348+
2349+
# String
2350+
with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
2351+
return_value=decision_service.Decision(mock_experiment, mock_variation,
2352+
decision_service.DECISION_SOURCE_ROLLOUT)), \
2353+
mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2354+
self.assertEqual('Hello',
2355+
opt_obj.get_feature_variable_string('test_feature_in_rollout', 'message', 'test_user'))
2356+
mock_client_logger.info.assert_called_once_with(
2357+
'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
2358+
'Returning the default variable value "Hello".'
2359+
)
2360+
22452361
def test_get_feature_variable__returns_none_if_type_mismatch(self):
22462362
""" Test that get_feature_variable_* returns None if type mismatch. """
22472363

@@ -2284,15 +2400,25 @@ def test_get_feature_variable_returns__variable_value__typed_audience_match(self
22842400
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences))
22852401

22862402
# Should be included in the feature test via greater-than match audience with id '3468206647'
2287-
self.assertEqual(
2288-
'xyz',
2289-
opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'lasers': 71})
2403+
with mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2404+
self.assertEqual(
2405+
'xyz',
2406+
opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'lasers': 71})
2407+
)
2408+
2409+
mock_client_logger.info.assert_called_once_with(
2410+
'Got variable value "xyz" for variable "x" of feature flag "feat_with_var".'
22902411
)
22912412

22922413
# Should be included in the feature test via exact match boolean audience with id '3468206643'
2293-
self.assertEqual(
2294-
'xyz',
2295-
opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'should_do_it': True})
2414+
with mock.patch.object(opt_obj, 'logger') as mock_client_logger:
2415+
self.assertEqual(
2416+
'xyz',
2417+
opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'should_do_it': True})
2418+
)
2419+
2420+
mock_client_logger.info.assert_called_once_with(
2421+
'Got variable value "xyz" for variable "x" of feature flag "feat_with_var".'
22962422
)
22972423

22982424
def test_get_feature_variable_returns__default_value__typed_audience_match(self):

0 commit comments

Comments
 (0)