fix: ODP - getQualifiedSegments should return null when fetch fails (#496)

Author: zashraf1985

core-api/src/main/java/com/optimizely/ab/Optimizely.java

@@ -1,5 +1,5 @@
 /****************************************************************************
- * Copyright 2016-2022, Optimizely, Inc. and contributors                   *
+ * Copyright 2016-2023, Optimizely, Inc. and contributors                   *
  *                                                                          *
  * Licensed under the Apache License, Version 2.0 (the "License");          *
  * you may not use this file except in compliance with the License.         *
@@ -436,7 +436,7 @@ private Boolean isFeatureEnabled(@Nonnull ProjectConfig projectConfig,
 
         Map<String, ?> copiedAttributes = copyAttributes(attributes);
         FeatureDecision.DecisionSource decisionSource = FeatureDecision.DecisionSource.ROLLOUT;
-        FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContext(userId, copiedAttributes), projectConfig).getResult();
+        FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContextCopy(userId, copiedAttributes), projectConfig).getResult();
         Boolean featureEnabled = false;
         SourceInfo sourceInfo = new RolloutSourceInfo();
         if (featureDecision.decisionSource != null) {
@@ -745,7 +745,7 @@ <T> T getFeatureVariableValueForType(@Nonnull String featureKey,
 
         String variableValue = variable.getDefaultValue();
         Map<String, ?> copiedAttributes = copyAttributes(attributes);
-        FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContext(userId, copiedAttributes), projectConfig).getResult();
+        FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContextCopy(userId, copiedAttributes), projectConfig).getResult();
         Boolean featureEnabled = false;
         if (featureDecision.variation != null) {
             if (featureDecision.variation.getFeatureEnabled()) {
@@ -880,7 +880,7 @@ public OptimizelyJSON getAllFeatureVariables(@Nonnull String featureKey,
         }
 
         Map<String, ?> copiedAttributes = copyAttributes(attributes);
-        FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContext(userId, copiedAttributes), projectConfig, Collections.emptyList()).getResult();
+        FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, createUserContextCopy(userId, copiedAttributes), projectConfig, Collections.emptyList()).getResult();
         Boolean featureEnabled = false;
         Variation variation = featureDecision.variation;
 
@@ -982,7 +982,7 @@ private Variation getVariation(@Nonnull ProjectConfig projectConfig,
                                    @Nonnull String userId,
                                    @Nonnull Map<String, ?> attributes) throws UnknownExperimentException {
         Map<String, ?> copiedAttributes = copyAttributes(attributes);
-        Variation variation = decisionService.getVariation(experiment, createUserContext(userId, copiedAttributes), projectConfig).getResult();
+        Variation variation = decisionService.getVariation(experiment, createUserContextCopy(userId, copiedAttributes), projectConfig).getResult();
         String notificationType = NotificationCenter.DecisionNotificationType.AB_TEST.toString();
 
         if (projectConfig.getExperimentFeatureKeyMapping().get(experiment.getId()) != null) {
@@ -1172,6 +1172,14 @@ public OptimizelyUserContext createUserContext(@Nonnull String userId) {
         return new OptimizelyUserContext(this, userId);
     }
 
+    private OptimizelyUserContext createUserContextCopy(@Nonnull String userId, @Nonnull Map<String, ?> attributes) {
+        if (userId == null) {
+            logger.warn("The userId parameter must be nonnull.");
+            return null;
+        }
+        return new OptimizelyUserContext(this, userId, attributes, Collections.EMPTY_MAP, null, false);
+    }
+
     OptimizelyDecision decide(@Nonnull OptimizelyUserContext user,
                               @Nonnull String key,
                               @Nonnull List<OptimizelyDecideOption> options) {

core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2020-2022, Optimizely and contributors
+ *    Copyright 2020-2023, 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.
@@ -76,7 +76,9 @@ public OptimizelyUserContext(@Nonnull Optimizely optimizely,
             this.forcedDecisionsMap = new ConcurrentHashMap<>(forcedDecisionsMap);
         }
 
-        this.qualifiedSegments = Collections.synchronizedList( qualifiedSegments == null ? new LinkedList<>(): qualifiedSegments);
+        if (qualifiedSegments != null) {
+            this.qualifiedSegments = Collections.synchronizedList(new LinkedList<>(qualifiedSegments));
+        }
 
         if (shouldIdentifyUser == null || shouldIdentifyUser) {
             optimizely.identifyUser(userId);
@@ -109,6 +111,10 @@ public OptimizelyUserContext copy() {
      * @return boolean Is user qualified for a segment.
      */
     public boolean isQualifiedFor(@Nonnull String segment) {
+        if (qualifiedSegments == null) {
+            return false;
+        }
+
         return qualifiedSegments.contains(segment);
     }
 
@@ -293,8 +299,14 @@ public List<String> getQualifiedSegments() {
     }
 
     public void setQualifiedSegments(List<String> qualifiedSegments) {
-        this.qualifiedSegments.clear();
-        this.qualifiedSegments.addAll(qualifiedSegments);
+        if (qualifiedSegments == null) {
+            this.qualifiedSegments = null;
+        } else if (this.qualifiedSegments == null) {
+            this.qualifiedSegments = Collections.synchronizedList(new LinkedList<>(qualifiedSegments));
+        } else {
+            this.qualifiedSegments.clear();
+            this.qualifiedSegments.addAll(qualifiedSegments);
+        }
     }
 
     /**
@@ -315,9 +327,7 @@ public Boolean fetchQualifiedSegments() {
      */
     public Boolean fetchQualifiedSegments(@Nonnull List<ODPSegmentOption> segmentOptions) {
         List<String> segments = optimizely.fetchQualifiedSegments(userId, segmentOptions);
-        if (segments != null) {
-            setQualifiedSegments(segments);
-        }
+        setQualifiedSegments(segments);
         return segments != null;
     }
 
@@ -332,9 +342,7 @@ public Boolean fetchQualifiedSegments(@Nonnull List<ODPSegmentOption> segmentOpt
      */
     public void fetchQualifiedSegments(ODPSegmentCallback callback, List<ODPSegmentOption> segmentOptions) {
         optimizely.fetchQualifiedSegments(userId, segments -> {
-            if (segments != null) {
-                setQualifiedSegments(segments);
-            }
+            setQualifiedSegments(segments);
             callback.onCompleted(segments != null);
         }, segmentOptions);
     }

core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2021-2022, Optimizely and contributors
+ *    Copyright 2021-2023, 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.
@@ -1709,7 +1709,7 @@ public void fetchQualifiedSegmentsAsyncError() throws InterruptedException {
         });
 
         countDownLatch.await();
-        assertEquals(Collections.emptyList(), userContext.getQualifiedSegments());
+        assertEquals(null, userContext.getQualifiedSegments());
         logbackVerifier.expectMessage(Level.ERROR, "Audience segments fetch failed (ODP is not enabled).");
     }