From c966034f1ae3cad524083b49979248818d50ca39 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Tue, 20 Dec 2022 19:55:58 -0800 Subject: [PATCH 1/5] 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/5] 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 4a065d86d8ad205456b3abe34cf60d9e6d848f7d Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Mon, 2 Jan 2023 23:25:44 +0500 Subject: [PATCH 3/5] Added odp config disable functionality --- .../java/com/optimizely/ab/Optimizely.java | 12 +++++---- .../com/optimizely/ab/odp/ODPManager.java | 26 ++++++++++++++++--- .../com/optimizely/ab/HttpClientUtils.java | 12 ++++----- 3 files changed, 34 insertions(+), 16 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..61bf3e0d4 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -122,7 +122,7 @@ private Optimizely(@Nonnull EventHandler eventHandler, this.defaultDecideOptions = defaultDecideOptions; this.odpManager = odpManager; - if (odpManager != null) { + if (odpManager != null && odpManager.getEventManager() != null) { odpManager.getEventManager().start(); if (getProjectConfig() != null) { updateODPSettings(); @@ -1439,7 +1439,9 @@ public int addNotificationHandler(Class clazz, NotificationHandler han public List fetchQualifiedSegments(String userId, @Nonnull List segmentOptions) { if (odpManager != null) { synchronized (odpManager) { - return odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions); + return odpManager.getSegmentManager() != null ? + odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions) + : null; } } logger.error("Audience segments fetch failed (ODP is not enabled)."); @@ -1447,7 +1449,7 @@ public List fetchQualifiedSegments(String userId, @Nonnull List segmentOptions) { - if (odpManager == null) { + if (odpManager == null || odpManager.getSegmentManager() == null) { logger.error("Audience segments fetch failed (ODP is not enabled)."); callback.onCompleted(null); } else { @@ -1461,7 +1463,7 @@ public ODPManager getODPManager() { } public void sendODPEvent(@Nullable String type, @Nonnull String action, @Nullable Map identifiers, @Nullable Map data) { - if (odpManager != null) { + if (odpManager != null && odpManager.getEventManager() != null) { ODPEvent event = new ODPEvent(type, action, identifiers, data); odpManager.getEventManager().sendEvent(event); } else { @@ -1471,7 +1473,7 @@ public void sendODPEvent(@Nullable String type, @Nonnull String action, @Nullabl public void identifyUser(@Nonnull String userId) { ODPManager odpManager = getODPManager(); - if (odpManager != null) { + if (odpManager != null && odpManager.getEventManager() != null) { odpManager.getEventManager().identifyUser(userId); } } diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java index 3e198a209..c82971b9c 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java @@ -36,9 +36,18 @@ private ODPManager(@Nonnull ODPApiManager apiManager) { } private ODPManager(@Nonnull ODPSegmentManager segmentManager, @Nonnull ODPEventManager eventManager) { - this.segmentManager = segmentManager; - this.eventManager = eventManager; - this.eventManager.start(); + this(segmentManager, eventManager, false); + } + + private ODPManager(@Nonnull ODPSegmentManager segmentManager, @Nonnull ODPEventManager eventManager, boolean disable) { + if (!disable) { + this.segmentManager = segmentManager; + this.eventManager = eventManager; + this.eventManager.start(); + } else { + this.segmentManager = null; + this.eventManager = null; + } } public ODPSegmentManager getSegmentManager() { @@ -77,6 +86,7 @@ public static class Builder { private Integer cacheSize; private Integer cacheTimeoutSeconds; private Cache> cacheImpl; + private boolean disable; /** * Provide a custom {@link ODPManager} instance which makes http calls to fetch segments and send events. @@ -156,7 +166,15 @@ public Builder withSegmentCache(Cache> cacheImpl) { return this; } + public Builder withOdpDisabled(Boolean disable) { + this.disable = disable; + return this; + } + public ODPManager build() { + if (disable) { + return null; + } if ((segmentManager == null || eventManager == null) && apiManager == null) { logger.warn("ApiManager instance is needed when using default EventManager or SegmentManager"); return null; @@ -179,7 +197,7 @@ public ODPManager build() { } } - if (eventManager == null) { + if (eventManager == null && !disable) { eventManager = new ODPEventManager(apiManager); } diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java index c35bb4b3f..e2b9ed9c0 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java @@ -39,13 +39,11 @@ private HttpClientUtils() { .build(); public static RequestConfig getDefaultRequestConfigWithTimeout(int timeoutMillis) { - if (requestConfigWithTimeout == null) { - requestConfigWithTimeout = RequestConfig.custom() - .setConnectTimeout(timeoutMillis) - .setConnectionRequestTimeout(CONNECTION_REQUEST_TIMEOUT_MS) - .setSocketTimeout(SOCKET_TIMEOUT_MS) - .build(); - } + requestConfigWithTimeout = RequestConfig.custom() + .setConnectTimeout(timeoutMillis) + .setConnectionRequestTimeout(CONNECTION_REQUEST_TIMEOUT_MS) + .setSocketTimeout(timeoutMillis) + .build(); return requestConfigWithTimeout; } From 92cb910334e4e8c97df37028fa788d1e54f8f557 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 5 Jan 2023 18:32:21 +0500 Subject: [PATCH 4/5] Reverting adding disable odp Option in sdk --- .../java/com/optimizely/ab/Optimizely.java | 12 ++++----- .../com/optimizely/ab/odp/ODPManager.java | 26 +++---------------- 2 files changed, 9 insertions(+), 29 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 61bf3e0d4..78c408b5c 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -122,7 +122,7 @@ private Optimizely(@Nonnull EventHandler eventHandler, this.defaultDecideOptions = defaultDecideOptions; this.odpManager = odpManager; - if (odpManager != null && odpManager.getEventManager() != null) { + if (odpManager != null) { odpManager.getEventManager().start(); if (getProjectConfig() != null) { updateODPSettings(); @@ -1439,9 +1439,7 @@ public int addNotificationHandler(Class clazz, NotificationHandler han public List fetchQualifiedSegments(String userId, @Nonnull List segmentOptions) { if (odpManager != null) { synchronized (odpManager) { - return odpManager.getSegmentManager() != null ? - odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions) - : null; + return odpManager.getSegmentManager().getQualifiedSegments(userId, segmentOptions); } } logger.error("Audience segments fetch failed (ODP is not enabled)."); @@ -1449,7 +1447,7 @@ public List fetchQualifiedSegments(String userId, @Nonnull List segmentOptions) { - if (odpManager == null || odpManager.getSegmentManager() == null) { + if (odpManager == null) { logger.error("Audience segments fetch failed (ODP is not enabled)."); callback.onCompleted(null); } else { @@ -1463,7 +1461,7 @@ public ODPManager getODPManager() { } public void sendODPEvent(@Nullable String type, @Nonnull String action, @Nullable Map identifiers, @Nullable Map data) { - if (odpManager != null && odpManager.getEventManager() != null) { + if (odpManager != null) { ODPEvent event = new ODPEvent(type, action, identifiers, data); odpManager.getEventManager().sendEvent(event); } else { @@ -1473,7 +1471,7 @@ public void sendODPEvent(@Nullable String type, @Nonnull String action, @Nullabl public void identifyUser(@Nonnull String userId) { ODPManager odpManager = getODPManager(); - if (odpManager != null && odpManager.getEventManager() != null) { + if (odpManager != null) { odpManager.getEventManager().identifyUser(userId); } } diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java index c82971b9c..3e198a209 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java @@ -36,18 +36,9 @@ private ODPManager(@Nonnull ODPApiManager apiManager) { } private ODPManager(@Nonnull ODPSegmentManager segmentManager, @Nonnull ODPEventManager eventManager) { - this(segmentManager, eventManager, false); - } - - private ODPManager(@Nonnull ODPSegmentManager segmentManager, @Nonnull ODPEventManager eventManager, boolean disable) { - if (!disable) { - this.segmentManager = segmentManager; - this.eventManager = eventManager; - this.eventManager.start(); - } else { - this.segmentManager = null; - this.eventManager = null; - } + this.segmentManager = segmentManager; + this.eventManager = eventManager; + this.eventManager.start(); } public ODPSegmentManager getSegmentManager() { @@ -86,7 +77,6 @@ public static class Builder { private Integer cacheSize; private Integer cacheTimeoutSeconds; private Cache> cacheImpl; - private boolean disable; /** * Provide a custom {@link ODPManager} instance which makes http calls to fetch segments and send events. @@ -166,15 +156,7 @@ public Builder withSegmentCache(Cache> cacheImpl) { return this; } - public Builder withOdpDisabled(Boolean disable) { - this.disable = disable; - return this; - } - public ODPManager build() { - if (disable) { - return null; - } if ((segmentManager == null || eventManager == null) && apiManager == null) { logger.warn("ApiManager instance is needed when using default EventManager or SegmentManager"); return null; @@ -197,7 +179,7 @@ public ODPManager build() { } } - if (eventManager == null && !disable) { + if (eventManager == null) { eventManager = new ODPEventManager(apiManager); } From 4424257813b921dadf19dfbdaca5632f380f9f32 Mon Sep 17 00:00:00 2001 From: mnoman09 Date: Thu, 5 Jan 2023 19:03:20 +0500 Subject: [PATCH 5/5] header update --- .../src/main/java/com/optimizely/ab/HttpClientUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java b/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java index e2b9ed9c0..dc786c4f6 100644 --- a/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java +++ b/core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2017, 2019, Optimizely and contributors + * Copyright 2016-2017, 2019, 2022-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.