Merge branch 'master' into oakbani/decision-is-feat-enabled

optimizely · Apr 2, 2019 · c6c2b48 · c6c2b48
2 parents 638de4a + 1f34b2a
commit c6c2b48
Show file tree

Hide file tree

Showing 4 changed files with 229 additions and 14 deletions.
diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py
@@ -208,12 +208,25 @@ def _get_feature_variable_for_type(self, feature_key, variable_key, variable_typ
       )
       return None
 
+    variable_value = variable.defaultValue
+
     decision = self.decision_service.get_variation_for_feature(feature_flag, user_id, attributes)
     if decision.variation:
-      variable_value = self.config.get_variable_value_for_variation(variable, decision.variation)
 
+      feature_enabled = decision.variation.featureEnabled
+      if feature_enabled:
+        variable_value = self.config.get_variable_value_for_variation(variable, decision.variation)
+        self.logger.info(
+          'Got variable value "%s" for variable "%s" of feature flag "%s".' % (
+            variable_value, variable_key, feature_key
+          )
+        )
+      else:
+        self.logger.info(
+          'Feature "%s" for variation "%s" is not enabled. '
+          'Returning the default variable value "%s".' % (feature_key, decision.variation.key, variable_value)
+        )
     else:
-      variable_value = variable.defaultValue
       self.logger.info(
         'User "%s" is not in any variation or rollout rule. '
         'Returning default value for variable "%s" of feature flag "%s".' % (user_id, variable_key, feature_key)

diff --git a/tests/base.py b/tests/base.py
@@ -1,4 +1,4 @@
-# Copyright 2016-2018, Optimizely
+# Copyright 2016-2019, Optimizely
 # 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
@@ -307,7 +307,29 @@ def setUp(self, config_dict='config_dict'):
           'variations': [{
             'key': '211129',
             'id': '211129',
-            'featureEnabled': True
+            'featureEnabled': True,
+            'variables': [{
+               'id': '132', 'value': 'true'
+             }, {
+               'id': '135', 'value': '395'
+             }, {
+               'id': '134', 'value': '39.99'
+             }, {
+               'id': '133', 'value': 'Hello audience'
+             }]
+          }, {
+            'key': '211229',
+            'id': '211229',
+            'featureEnabled': False,
+            'variables': [{
+               'id': '132', 'value': 'true'
+              }, {
+               'id': '135', 'value': '395'
+             }, {
+               'id': '134', 'value': '39.99'
+             }, {
+               'id': '133', 'value': 'Hello audience'
+             }]
           }]
         }, {
           'id': '211137',
@@ -379,7 +401,27 @@ def setUp(self, config_dict='config_dict'):
         'key': 'test_feature_in_rollout',
         'experimentIds': [],
         'rolloutId': '211111',
-        'variables': [],
+        'variables': [{
+            'id': '132',
+            'key': 'is_running',
+            'defaultValue': 'false',
+            'type': 'boolean'
+          }, {
+            'id': '133',
+            'key': 'message',
+            'defaultValue': 'Hello',
+            'type': 'string'
+          }, {
+            'id': '134',
+            'key': 'price',
+            'defaultValue': '99.99',
+            'type': 'double'
+          }, {
+            'id': '135',
+            'key': 'count',
+            'defaultValue': '999',
+            'type': 'integer'
+          }]
       }, {
         'id': '91113',
         'key': 'test_feature_in_group',

diff --git a/tests/test_config.py b/tests/test_config.py
@@ -1,4 +1,4 @@
-# Copyright 2016-2018, Optimizely
+# Copyright 2016-2019, Optimizely
 # 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
@@ -885,7 +885,19 @@ def test_get_feature_from_key__valid_feature_key(self):
     opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
     project_config = opt_obj.config
 
-    expected_feature = entities.FeatureFlag('91112', 'test_feature_in_rollout', [], '211111', {})
+    expected_feature = entities.FeatureFlag(
+      '91112',
+      'test_feature_in_rollout',
+      [],
+      '211111',
+      {
+        'is_running': entities.Variable('132', 'is_running', 'boolean', 'false'),
+        'message': entities.Variable('133', 'message', 'string', 'Hello'),
+        'price': entities.Variable('134', 'price', 'double', '99.99'),
+        'count': entities.Variable('135', 'count', 'integer', '999')
+      }
+    )
+
     self.assertEqual(expected_feature, project_config.get_feature_from_key('test_feature_in_rollout'))
 
   def test_get_feature_from_key__invalid_feature_key(self):
@@ -916,7 +928,29 @@ def test_get_rollout_from_id__valid_rollout_id(self):
       'variations': [{
         'key': '211129',
         'id': '211129',
-        'featureEnabled': True
+        'featureEnabled': True,
+        'variables': [{
+           'id': '132', 'value': 'true'
+         }, {
+           'id': '135', 'value': '395'
+         }, {
+           'id': '134', 'value': '39.99'
+         }, {
+           'id': '133', 'value': 'Hello audience'
+         }]
+      }, {
+        'key': '211229',
+        'id': '211229',
+        'featureEnabled': False,
+        'variables': [{
+           'id': '132', 'value': 'true'
+         }, {
+           'id': '135', 'value': '395'
+         }, {
+           'id': '134', 'value': '39.99'
+         }, {
+           'id': '133', 'value': 'Hello audience'
+         }]
       }]
     }, {
       'id': '211137',

diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py
@@ -2397,6 +2397,122 @@ def test_get_feature_variable__returns_none_if_invalid_variable_key(self):
       mock.call('Variable with key "invalid_variable" not found in the datafile.')
     ])
 
+  def test_get_feature_variable__returns_default_value_if_feature_not_enabled(self):
+    """ Test that get_feature_variable_* returns default value if feature is not enabled for the user. """
+
+    opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
+    mock_experiment = opt_obj.config.get_experiment_from_key('test_experiment')
+    mock_variation = opt_obj.config.get_variation_from_id('test_experiment', '111128')
+
+    # Boolean
+    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
+                    return_value=decision_service.Decision(mock_experiment, mock_variation,
+                                                           decision_service.DECISION_SOURCE_EXPERIMENT)), \
+         mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+
+      self.assertTrue(opt_obj.get_feature_variable_boolean('test_feature_in_experiment', 'is_working', 'test_user'))
+
+    mock_client_logger.info.assert_called_once_with(
+      'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
+      'Returning the default variable value "true".'
+    )
+
+    # Double
+    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
+                    return_value=decision_service.Decision(mock_experiment, mock_variation,
+                                                           decision_service.DECISION_SOURCE_EXPERIMENT)), \
+         mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+      self.assertEqual(10.99,
+                       opt_obj.get_feature_variable_double('test_feature_in_experiment', 'cost', 'test_user'))
+
+    mock_client_logger.info.assert_called_once_with(
+      'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
+      'Returning the default variable value "10.99".'
+    )
+
+    # Integer
+    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
+                    return_value=decision_service.Decision(mock_experiment, mock_variation,
+                                                           decision_service.DECISION_SOURCE_EXPERIMENT)), \
+         mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+      self.assertEqual(999,
+                       opt_obj.get_feature_variable_integer('test_feature_in_experiment', 'count', 'test_user'))
+
+    mock_client_logger.info.assert_called_once_with(
+      'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
+      'Returning the default variable value "999".'
+    )
+
+    # String
+    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
+                    return_value=decision_service.Decision(mock_experiment, mock_variation,
+                                                           decision_service.DECISION_SOURCE_EXPERIMENT)), \
+         mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+      self.assertEqual('devel',
+                       opt_obj.get_feature_variable_string('test_feature_in_experiment', 'environment', 'test_user'))
+
+    mock_client_logger.info.assert_called_once_with(
+      'Feature "test_feature_in_experiment" for variation "control" is not enabled. '
+      'Returning the default variable value "devel".'
+    )
+
+  def test_get_feature_variable__returns_default_value_if_feature_not_enabled_in_rollout(self):
+    """ Test that get_feature_variable_* returns default value if feature is not enabled for the user. """
+
+    opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features))
+    mock_experiment = opt_obj.config.get_experiment_from_key('211127')
+    mock_variation = opt_obj.config.get_variation_from_id('211127', '211229')
+
+    # Boolean
+    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
+                    return_value=decision_service.Decision(mock_experiment, mock_variation,
+                                                           decision_service.DECISION_SOURCE_ROLLOUT)), \
+         mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+      self.assertFalse(opt_obj.get_feature_variable_boolean('test_feature_in_rollout', 'is_running', 'test_user'))
+
+    mock_client_logger.info.assert_called_once_with(
+      'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
+      'Returning the default variable value "false".'
+    )
+
+    # Double
+    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
+                    return_value=decision_service.Decision(mock_experiment, mock_variation,
+                                                           decision_service.DECISION_SOURCE_ROLLOUT)), \
+         mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+      self.assertEqual(99.99,
+                       opt_obj.get_feature_variable_double('test_feature_in_rollout', 'price', 'test_user'))
+
+    mock_client_logger.info.assert_called_once_with(
+      'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
+      'Returning the default variable value "99.99".'
+    )
+
+    # Integer
+    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
+                    return_value=decision_service.Decision(mock_experiment, mock_variation,
+                                                           decision_service.DECISION_SOURCE_ROLLOUT)), \
+         mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+      self.assertEqual(999,
+                       opt_obj.get_feature_variable_integer('test_feature_in_rollout', 'count', 'test_user'))
+
+    mock_client_logger.info.assert_called_once_with(
+      'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
+      'Returning the default variable value "999".'
+    )
+
+    # String
+    with mock.patch('optimizely.decision_service.DecisionService.get_variation_for_feature',
+                    return_value=decision_service.Decision(mock_experiment, mock_variation,
+                                                           decision_service.DECISION_SOURCE_ROLLOUT)), \
+         mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+      self.assertEqual('Hello',
+                       opt_obj.get_feature_variable_string('test_feature_in_rollout', 'message', 'test_user'))
+    mock_client_logger.info.assert_called_once_with(
+      'Feature "test_feature_in_rollout" for variation "211229" is not enabled. '
+      'Returning the default variable value "Hello".'
+    )
+
   def test_get_feature_variable__returns_none_if_type_mismatch(self):
     """ Test that get_feature_variable_* returns None if type mismatch. """
 
@@ -2439,15 +2555,25 @@ def test_get_feature_variable_returns__variable_value__typed_audience_match(self
     opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_typed_audiences))
 
     # Should be included in the feature test via greater-than match audience with id '3468206647'
-    self.assertEqual(
-      'xyz',
-      opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'lasers': 71})
+    with mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+      self.assertEqual(
+        'xyz',
+        opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'lasers': 71})
+      )
+
+    mock_client_logger.info.assert_called_once_with(
+      'Got variable value "xyz" for variable "x" of feature flag "feat_with_var".'
     )
 
     # Should be included in the feature test via exact match boolean audience with id '3468206643'
-    self.assertEqual(
-      'xyz',
-      opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'should_do_it': True})
+    with mock.patch.object(opt_obj, 'logger') as mock_client_logger:
+      self.assertEqual(
+        'xyz',
+        opt_obj.get_feature_variable_string('feat_with_var', 'x', 'user1', {'should_do_it': True})
+      )
+
+    mock_client_logger.info.assert_called_once_with(
+      'Got variable value "xyz" for variable "x" of feature flag "feat_with_var".'
     )
 
   def test_get_feature_variable_returns__default_value__typed_audience_match(self):