[FSSDK-12094] Fix Ruby holdout implementation which related to FSC failures

Author: esrakartalOpt

optimizely/decision_service.py

@@ -714,8 +714,8 @@ def get_decision_for_flag(
             reasons.extend(holdout_decision['reasons'])
 
             decision = holdout_decision['decision']
-            # Check if user was bucketed into holdout (has a variation)
-            if decision.variation is None:
+            # Check if user was bucketed into holdout (has a decision)
+            if decision is None:
                 continue
 
             message = (
@@ -730,21 +730,102 @@ def get_decision_for_flag(
                 'reasons': reasons
             }
 
-        # If no holdout decision, fall back to existing experiment/rollout logic
-        # Use get_variations_for_feature_list which handles experiments and rollouts
-        fallback_result = self.get_variations_for_feature_list(
-            project_config, [feature_flag], user_context, decide_options
-        )[0]
-
-        # Merge reasons
-        if fallback_result.get('reasons'):
-            reasons.extend(fallback_result['reasons'])
+        # Check if the feature flag has an experiment and the user is bucketed into that experiment
+        if feature_flag.experimentIds:
+            # Iterate through experiments to find a match
+            for experiment_id in feature_flag.experimentIds:
+                experiment = project_config.get_experiment_from_id(experiment_id)
+                if not experiment:
+                    continue
+
+                # Check for forced decision
+                optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(
+                    feature_flag.key, experiment.key
+                )
+                forced_decision_variation, forced_reasons = self.validated_forced_decision(
+                    project_config, optimizely_decision_context, user_context
+                )
+                reasons.extend(forced_reasons)
+
+                if forced_decision_variation:
+                    decision = Decision(
+                        experiment, forced_decision_variation, enums.DecisionSources.FEATURE_TEST, None
+                    )
+                    return {
+                        'decision': decision,
+                        'error': False,
+                        'reasons': reasons
+                    }
+
+                # Get variation through normal bucketing
+                variation_result = self.get_variation(
+                    project_config, experiment, user_context, user_profile_tracker, reasons, decide_options
+                )
+                cmab_uuid = variation_result['cmab_uuid']
+                variation_reasons = variation_result['reasons']
+                decision_variation = variation_result['variation']
+                error = variation_result['error']
+                reasons.extend(variation_reasons)
+
+                # If there's an error, return immediately
+                if error:
+                    decision = Decision(experiment, None, enums.DecisionSources.FEATURE_TEST, cmab_uuid)
+                    return {
+                        'decision': decision,
+                        'error': True,
+                        'reasons': reasons
+                    }
+
+                # If user is bucketed into a variation, return the decision
+                if decision_variation:
+                    self.logger.debug(
+                        f'User "{user_id}" '
+                        f'bucketed into experiment "{experiment.key}" of feature "{feature_flag.key}".'
+                    )
+                    decision = Decision(
+                        experiment, decision_variation, enums.DecisionSources.FEATURE_TEST, cmab_uuid
+                    )
+                    return {
+                        'decision': decision,
+                        'error': False,
+                        'reasons': reasons
+                    }
+
+        # Check if the feature flag has a rollout and the user is bucketed into that rollout
+        rollout_decision, rollout_reasons = self.get_variation_for_rollout(
+            project_config, feature_flag, user_context
+        )
+        reasons.extend(rollout_reasons)
+
+        if rollout_decision and rollout_decision.variation:
+            # Check if this was a forced decision (last reason contains "forced decision map")
+            is_forced_decision = reasons and 'forced decision map' in reasons[-1] if reasons else False
+
+            if not is_forced_decision:
+                # Only add the "bucketed into rollout" message for normal bucketing
+                message = (
+                    f"The user '{user_id}' is bucketed into a rollout "
+                    f"for feature flag '{feature_flag.key}'."
+                )
+                self.logger.info(message)
+                reasons.append(message)
 
-        return {
-            'decision': fallback_result['decision'],
-            'error': fallback_result.get('error', False),
-            'reasons': reasons
-        }
+            return {
+                'decision': rollout_decision,
+                'error': False,
+                'reasons': reasons
+            }
+        else:
+            message = (
+                f"The user '{user_id}' is not bucketed into a rollout "
+                f"for feature flag '{feature_flag.key}'."
+            )
+            self.logger.info(message)
+            return {
+                'decision': Decision(None, None, enums.DecisionSources.ROLLOUT, None),
+                'error': False,
+                'reasons': reasons
+            }
 
     def get_variation_for_holdout(
         self,
@@ -931,100 +1012,24 @@ def get_variations_for_feature_list(
                 - 'error': Boolean indicating if an error occurred during the decision process.
                 - 'reasons': List of log messages representing decision making for each feature.
         """
-        decide_reasons: list[str] = []
-
+        ignore_ups = False
         if options:
             ignore_ups = OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE in options
-        else:
-            ignore_ups = False
 
         user_profile_tracker: Optional[UserProfileTracker] = None
-        if self.user_profile_service is not None and not ignore_ups:
-            user_profile_tracker = UserProfileTracker(user_context.user_id, self.user_profile_service, self.logger)
-            user_profile_tracker.load_user_profile(decide_reasons, None)
+        if not ignore_ups and self.user_profile_service:
+            user_id = user_context.user_id
+            user_profile_tracker = UserProfileTracker(user_id, self.user_profile_service, self.logger)
+            user_profile_tracker.load_user_profile([], None)
 
         decisions = []
+        for feature_flag in features:
+            decision = self.get_decision_for_flag(
+                feature_flag, user_context, project_config, options, user_profile_tracker
+            )
+            decisions.append(decision)
 
-        for feature in features:
-            feature_reasons = decide_reasons.copy()
-            experiment_decision_found = False  # Track if an experiment decision was made for the feature
-
-            # Check if the feature flag is under an experiment
-            if feature.experimentIds:
-                for experiment_id in feature.experimentIds:
-                    experiment = project_config.get_experiment_from_id(experiment_id)
-                    decision_variation: Optional[Union[entities.Variation, VariationDict]] = None
-
-                    if experiment:
-                        optimizely_decision_context = OptimizelyUserContext.OptimizelyDecisionContext(
-                            feature.key, experiment.key)
-                        forced_decision_variation, reasons_received = self.validated_forced_decision(
-                            project_config, optimizely_decision_context, user_context)
-                        feature_reasons.extend(reasons_received)
-
-                        if forced_decision_variation:
-                            decision_variation = forced_decision_variation
-                            cmab_uuid = None
-                            error = False
-                        else:
-                            variation_result = self.get_variation(
-                                project_config, experiment, user_context, user_profile_tracker, feature_reasons, options
-                            )
-                            cmab_uuid = variation_result['cmab_uuid']
-                            variation_reasons = variation_result['reasons']
-                            decision_variation = variation_result['variation']
-                            error = variation_result['error']
-                            feature_reasons.extend(variation_reasons)
-
-                        if error:
-                            decision = Decision(experiment, None, enums.DecisionSources.FEATURE_TEST, cmab_uuid)
-                            decision_result: DecisionResult = {
-                                'decision': decision,
-                                'error': True,
-                                'reasons': feature_reasons
-                            }
-                            decisions.append(decision_result)
-                            experiment_decision_found = True
-                            break
-
-                        if decision_variation:
-                            self.logger.debug(
-                                f'User "{user_context.user_id}" '
-                                f'bucketed into experiment "{experiment.key}" of feature "{feature.key}".'
-                            )
-                            decision = Decision(experiment, decision_variation,
-                                                enums.DecisionSources.FEATURE_TEST, cmab_uuid)
-                            decision_result = {
-                                'decision': decision,
-                                'error': False,
-                                'reasons': feature_reasons
-                            }
-                            decisions.append(decision_result)
-                            experiment_decision_found = True  # Mark that a decision was found
-                            break  # Stop after the first successful experiment decision
-
-            # Only process rollout if no experiment decision was found and no error
-            if not experiment_decision_found:
-                rollout_decision, rollout_reasons = self.get_variation_for_rollout(project_config,
-                                                                                   feature,
-                                                                                   user_context)
-                if rollout_reasons:
-                    feature_reasons.extend(rollout_reasons)
-                if rollout_decision:
-                    self.logger.debug(f'User "{user_context.user_id}" '
-                                      f'bucketed into rollout for feature "{feature.key}".')
-                else:
-                    self.logger.debug(f'User "{user_context.user_id}" '
-                                      f'not bucketed into any rollout for feature "{feature.key}".')
-
-                decision_result = {
-                    'decision': rollout_decision,
-                    'error': False,
-                    'reasons': feature_reasons
-                }
-                decisions.append(decision_result)
-
-        if self.user_profile_service is not None and user_profile_tracker is not None and ignore_ups is False:
+        if user_profile_tracker:
             user_profile_tracker.save_user_profile()
 
         return decisions

optimizely/optimizely.py

@@ -433,7 +433,7 @@ def _get_feature_variable_for_type(
         decision_result = self.decision_service.get_variation_for_feature(project_config, feature_flag, user_context)
         decision = decision_result['decision']
 
-        if decision.variation:
+        if decision and decision.variation:
 
             feature_enabled = self._get_feature_enabled(decision.variation)
             if feature_enabled:
@@ -792,23 +792,24 @@ def is_feature_enabled(self, feature_key: str, user_id: str, attributes: Optiona
         user_context = OptimizelyUserContext(self, self.logger, user_id, attributes, False)
 
         decision = self.decision_service.get_variation_for_feature(project_config, feature, user_context)['decision']
-        cmab_uuid = decision.cmab_uuid
+        cmab_uuid = decision.cmab_uuid if decision else None
 
-        is_source_experiment = decision.source == enums.DecisionSources.FEATURE_TEST
-        is_source_rollout = decision.source == enums.DecisionSources.ROLLOUT
+        is_source_experiment = decision.source == enums.DecisionSources.FEATURE_TEST if decision else False
+        is_source_rollout = decision.source == enums.DecisionSources.ROLLOUT if decision else False
 
-        if decision.variation:
+        if decision and decision.variation:
             if self._get_feature_enabled(decision.variation) is True:
                 feature_enabled = True
 
-        if (is_source_rollout or not decision.variation) and project_config.get_send_flag_decisions_value():
+        if (decision and (is_source_rollout or not decision.variation) and
+                project_config.get_send_flag_decisions_value()):
             self._send_impression_event(
                 project_config, decision.experiment, decision.variation, feature.key, decision.experiment.key if
                 decision.experiment else '', str(decision.source), feature_enabled, user_id, attributes, cmab_uuid
             )
 
         # Send event if Decision came from an experiment.
-        if is_source_experiment and decision.variation and decision.experiment:
+        if decision and is_source_experiment and decision.variation and decision.experiment:
             source_info = {
                 'experiment_key': decision.experiment.key,
                 'variation_key': self._get_variation_key(decision.variation),
@@ -1244,25 +1245,27 @@ def _create_optimizely_decision(
     ) -> OptimizelyDecision:
         user_id = user_context.user_id
         feature_enabled = False
-        if flag_decision.variation is not None:
+        if flag_decision and flag_decision.variation is not None:
             if self._get_feature_enabled(flag_decision.variation):
                 feature_enabled = True
 
         self.logger.info(f'Feature {flag_key} is enabled for user {user_id} {feature_enabled}"')
 
         # Create Optimizely Decision Result.
         attributes = user_context.get_user_attributes()
-        rule_key = flag_decision.experiment.key if flag_decision.experiment else None
+        rule_key = flag_decision.experiment.key if (flag_decision and flag_decision.experiment) else None
         all_variables = {}
-        decision_source = flag_decision.source
+        decision_source = flag_decision.source if flag_decision else None
         decision_event_dispatched = False
 
         feature_flag = project_config.feature_key_map.get(flag_key)
 
         # Send impression event if Decision came from a feature
         # test and decide options doesn't include disableDecisionEvent
         if OptimizelyDecideOption.DISABLE_DECISION_EVENT not in decide_options:
-            if decision_source == DecisionSources.FEATURE_TEST or project_config.send_flag_decisions:
+            if (decision_source == DecisionSources.FEATURE_TEST or
+                    decision_source == DecisionSources.HOLDOUT or
+                    project_config.send_flag_decisions):
                 self._send_impression_event(project_config,
                                             flag_decision.experiment,
                                             flag_decision.variation,