From c966034f1ae3cad524083b49979248818d50ca39 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Tue, 20 Dec 2022 19:55:58 -0800 Subject: [PATCH 1/3] return null when fetch fails --- .../optimizely/ab/OptimizelyUserContext.java | 26 ++++++++++++------- .../ab/OptimizelyUserContextTest.java | 2 +- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java index 5ba15b5b4..7c12dbb1a 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java @@ -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 getQualifiedSegments() { } public void setQualifiedSegments(List 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 segmentOptions) { List segments = optimizely.fetchQualifiedSegments(userId, segmentOptions); - if (segments != null) { - setQualifiedSegments(segments); - } + setQualifiedSegments(segments); return segments != null; } @@ -332,9 +342,7 @@ public Boolean fetchQualifiedSegments(@Nonnull List segmentOpt */ public void fetchQualifiedSegments(ODPSegmentCallback callback, List segmentOptions) { optimizely.fetchQualifiedSegments(userId, segments -> { - if (segments != null) { - setQualifiedSegments(segments); - } + setQualifiedSegments(segments); callback.onCompleted(segments != null); }, segmentOptions); } diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 5dd47a8cf..6cf535f58 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -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)."); } From 7f874566eeb9f528577857a15d563ff2b7586c1d Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 29 Dec 2022 16:16:11 +0500 Subject: [PATCH 2/3] Added a function to create a copy of userContext, this will avoid sending odpEvent upon creating just a copy of UserContext --- .../main/java/com/optimizely/ab/Optimizely.java | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 0fbacaa3b..78c408b5c 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -436,7 +436,7 @@ private Boolean isFeatureEnabled(@Nonnull ProjectConfig projectConfig, Map 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 getFeatureVariableValueForType(@Nonnull String featureKey, String variableValue = variable.getDefaultValue(); Map 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 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 attributes) throws UnknownExperimentException { Map 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 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 options) { From b1845f916397f91252a847d17d941bae782a1b03 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 5 Jan 2023 19:06:14 +0500 Subject: [PATCH 3/3] Updated headers --- core-api/src/main/java/com/optimizely/ab/Optimizely.java | 2 +- .../src/main/java/com/optimizely/ab/OptimizelyUserContext.java | 2 +- .../test/java/com/optimizely/ab/OptimizelyUserContextTest.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 78c408b5c..12b9f33d1 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/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. * diff --git a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java b/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java index 7c12dbb1a..7cb8753f4 100644 --- a/core-api/src/main/java/com/optimizely/ab/OptimizelyUserContext.java +++ b/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. diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 6cf535f58..8f8bae834 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/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.