From 4d43f2b9c9e486b8aff7516417d575743a4a368c Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Thu, 11 Aug 2022 10:42:46 -0700 Subject: [PATCH 1/6] Development complete. Manually tested from outside. Need to write unit tests. --- .../java/com/optimizely/ab/odp/ODPConfig.java | 64 ++++++++++++ .../optimizely/ab/odp/ODPSegmentManager.java | 98 +++++++++++++++++++ .../optimizely/ab/odp/ODPSegmentOption.java | 25 +++++ .../com/optimizely/ab/odp/ODPUserKey.java | 34 +++++++ 4 files changed, 221 insertions(+) create mode 100644 core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java create mode 100644 core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java create mode 100644 core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentOption.java create mode 100644 core-api/src/main/java/com/optimizely/ab/odp/ODPUserKey.java diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java new file mode 100644 index 000000000..253f720ed --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java @@ -0,0 +1,64 @@ +/** + * + * Copyright 2022, 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.odp; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +public class ODPConfig { + + private String apiKey; + + private String apiHost; + + private List allSegments; + + public ODPConfig(String apiKey, String apiHost, List allSegments) { + this.apiKey = apiKey; + this.apiHost = apiHost; + this.allSegments = allSegments; + } + + public ODPConfig(String apiKey, String apiHost) { + this(apiKey, apiHost, Collections.emptyList()); + } + + public void setApiKey(String apiKey) { + this.apiKey = apiKey; + } + + public void setApiHost(String apiHost) { + this.apiHost = apiHost; + } + + public String getApiKey() { + return apiKey; + } + + public String getApiHost() { + return apiHost; + } + + public List getAllSegments() { + return allSegments; + } + + public void setAllSegments(List allSegments) { + this.allSegments = allSegments; + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java new file mode 100644 index 000000000..2d5372f9c --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java @@ -0,0 +1,98 @@ +/** + * + * Copyright 2022, 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.odp; + +import com.optimizely.ab.internal.Cache; +import com.optimizely.ab.internal.DefaultLRUCache; +import com.optimizely.ab.odp.parser.ResponseJsonParser; +import com.optimizely.ab.odp.parser.ResponseJsonParserFactory; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.util.Collections; +import java.util.List; + +public class ODPSegmentManager { + + private static final Logger logger = LoggerFactory.getLogger(ODPSegmentManager.class); + + private static final String SEGMENT_URL_PATH = "/v3/graphql"; + + private final ODPApiManager apiManager; + + private final ODPConfig odpConfig; + + private final Cache> segmentsCache; + + public ODPSegmentManager(ODPConfig odpConfig, ODPApiManager apiManager) { + this(odpConfig, apiManager, Cache.DEFAULT_MAX_SIZE, Cache.DEFAULT_TIMEOUT_SECONDS); + } + + public ODPSegmentManager(ODPConfig odpConfig, ODPApiManager apiManager, Cache> cache) { + this.apiManager = apiManager; + this.odpConfig = odpConfig; + this.segmentsCache = cache; + } + + public ODPSegmentManager(ODPConfig odpConfig, ODPApiManager apiManager, Integer cacheSize, Integer cacheTimeoutSeconds) { + this.apiManager = apiManager; + this.odpConfig = odpConfig; + this.segmentsCache = new DefaultLRUCache<>(cacheSize, cacheTimeoutSeconds); + } + + public List getQualifiedSegments(String fsUserId) { + return getQualifiedSegments(ODPUserKey.FS_USER_ID, fsUserId, Collections.emptyList()); + } + public List getQualifiedSegments(String fsUserId, List options) { + return getQualifiedSegments(ODPUserKey.FS_USER_ID, fsUserId, options); + } + + public List getQualifiedSegments(ODPUserKey userKey, String userValue) { + return getQualifiedSegments(userKey, userValue, Collections.emptyList()); + } + + public List getQualifiedSegments(ODPUserKey userKey, String userValue, List options) { + List qualifiedSegments; + String cacheKey = getCacheKey(userKey.getKeyString(), userValue); + + if (options.contains(ODPSegmentOption.RESET_CACHE)) { + segmentsCache.reset(); + } else if (!options.contains(ODPSegmentOption.IGNORE_CACHE)) { + qualifiedSegments = segmentsCache.lookup(cacheKey); + if (qualifiedSegments != null) { + logger.debug("ODP Cache Hit. Returning segments from Cache."); + return qualifiedSegments; + } + } + + logger.debug("ODP Cache Miss. Making a call to ODP Server"); + + ResponseJsonParser parser = ResponseJsonParserFactory.getParser(); + String qualifiedSegmentsResponse = apiManager.fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + SEGMENT_URL_PATH, userKey.getKeyString(), userValue, odpConfig.getAllSegments()); + qualifiedSegments = parser.parseQualifiedSegments(qualifiedSegmentsResponse); + + if (qualifiedSegments != null && !options.contains(ODPSegmentOption.IGNORE_CACHE)) { + segmentsCache.save(cacheKey, qualifiedSegments); + } + + return qualifiedSegments; + } + + private String getCacheKey(String userKey, String userValue) { + return userKey + "-$-" + userValue; + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentOption.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentOption.java new file mode 100644 index 000000000..8e2eb901b --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentOption.java @@ -0,0 +1,25 @@ +/** + * + * Copyright 2022, 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.odp; + +public enum ODPSegmentOption { + + IGNORE_CACHE, + + RESET_CACHE; + +} diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPUserKey.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPUserKey.java new file mode 100644 index 000000000..d7cdbb641 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPUserKey.java @@ -0,0 +1,34 @@ +/** + * + * Copyright 2022, 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.odp; + +public enum ODPUserKey { + + VUID("vuid"), + + FS_USER_ID("fs_user_id"); + + private final String keyString; + + ODPUserKey(String keyString) { + this.keyString = keyString; + } + + public String getKeyString() { + return keyString; + } +} From a7c3ee0b21b8129e90699bad451f38b85b32afc2 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Thu, 11 Aug 2022 17:41:00 -0700 Subject: [PATCH 2/6] Added unit tests --- .../ab/odp/ODPSegmentManagerTest.java | 159 ++++++++++++++++++ 1 file changed, 159 insertions(+) create mode 100644 core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java new file mode 100644 index 000000000..da11f5a48 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java @@ -0,0 +1,159 @@ +/** + * + * Copyright 2022, 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 + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.odp; + +import com.optimizely.ab.internal.Cache; +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.Mockito; + +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; +import static org.junit.Assert.*; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +public class ODPSegmentManagerTest { + + @Mock + Cache> mockCache; + + @Mock + ODPApiManager mockApiManager; + + private static final String API_RESPONSE = "{\"data\":{\"customer\":{\"audiences\":{\"edges\":[{\"node\":{\"name\":\"segment1\",\"state\":\"qualified\"}},{\"node\":{\"name\":\"segment2\",\"state\":\"qualified\"}}]}}}}"; + + @Before + public void setup() { + mockCache = mock(Cache.class); + mockApiManager = mock(ODPApiManager.class); + } + + @Test + public void cacheHit() { + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); + ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId"); + + // Cache lookup called with correct key + verify(mockCache, times(1)).lookup("fs_user_id-$-testId"); + + // Cache hit! No api call was made to the server. + verify(mockApiManager, times(0)).fetchQualifiedSegments(any(), any(), any(), any(), any()); + verify(mockCache, times(0)).save(any(), any()); + verify(mockCache, times(0)).reset(); + + assertEquals(Arrays.asList("segment1-cached", "segment2-cached"), segments); + } + + @Test + public void cacheMiss() { + Mockito.when(mockCache.lookup(any())).thenReturn(null); + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + .thenReturn(API_RESPONSE); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); + ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + List segments = segmentManager.getQualifiedSegments(ODPUserKey.VUID, "testId"); + + // Cache lookup called with correct key + verify(mockCache, times(1)).lookup("vuid-$-testId"); + + // Cache miss! Make api call and save to cache + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "vuid", "testId", Arrays.asList("segment1", "segment2")); + verify(mockCache, times(1)).save("vuid-$-testId", Arrays.asList("segment1", "segment2")); + verify(mockCache, times(0)).reset(); + + assertEquals(Arrays.asList("segment1", "segment2"), segments); + } + + @Test + public void ignoreCache() { + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + .thenReturn(API_RESPONSE); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); + ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", Collections.singletonList(ODPSegmentOption.IGNORE_CACHE)); + + // Cache Ignored! lookup should not be called + verify(mockCache, times(0)).lookup(any()); + + // Cache Ignored! Make API Call but do NOT save because of cacheIgnore + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", Arrays.asList("segment1", "segment2")); + verify(mockCache, times(0)).save(any(), any()); + verify(mockCache, times(0)).reset(); + + assertEquals(Arrays.asList("segment1", "segment2"), segments); + } + + @Test + public void resetCache() { + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + .thenReturn(API_RESPONSE); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); + ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", Collections.singletonList(ODPSegmentOption.RESET_CACHE)); + + // Call reset + verify(mockCache, times(1)).reset(); + + // Cache Reset! lookup should not be called becaues cache would be empty. + verify(mockCache, times(0)).lookup(any()); + + // Cache reset but not Ignored! Make API Call and save to cache + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", Arrays.asList("segment1", "segment2")); + verify(mockCache, times(1)).save("fs_user_id-$-testId", Arrays.asList("segment1", "segment2")); + + assertEquals(Arrays.asList("segment1", "segment2"), segments); + } + + @Test + public void resetAndIgnoreCache() { + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + .thenReturn(API_RESPONSE); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", Arrays.asList("segment1", "segment2")); + ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + List segments = segmentManager + .getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId", Arrays.asList(ODPSegmentOption.RESET_CACHE, ODPSegmentOption.IGNORE_CACHE)); + + // Call reset + verify(mockCache, times(1)).reset(); + + verify(mockCache, times(0)).lookup(any()); + + // Cache is also Ignored! Make API Call but do not save + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + "/v3/graphql", "fs_user_id", "testId", Arrays.asList("segment1", "segment2")); + verify(mockCache, times(0)).save(any(), any()); + + assertEquals(Arrays.asList("segment1", "segment2"), segments); + } +} From f68442e933725993300f3e27ca3ad1e2ed286545 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Tue, 16 Aug 2022 17:15:57 -0700 Subject: [PATCH 3/6] incorporated review feedback --- .../java/com/optimizely/ab/odp/ODPConfig.java | 24 ++++++--- .../optimizely/ab/odp/ODPSegmentManager.java | 12 ++++- .../ab/odp/ODPSegmentManagerTest.java | 52 +++++++++++++++++++ 3 files changed, 80 insertions(+), 8 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java index 253f720ed..0317b5283 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java @@ -16,7 +16,6 @@ */ package com.optimizely.ab.odp; -import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -34,31 +33,42 @@ public ODPConfig(String apiKey, String apiHost, List allSegments) { this.allSegments = allSegments; } + public Boolean isReady() { + return !( + this.apiKey == null || this.apiKey.isEmpty() + || this.apiHost == null || this.apiHost.isEmpty() + ); + } + + public Boolean hasSegments() { + return allSegments != null && !allSegments.isEmpty(); + } + public ODPConfig(String apiKey, String apiHost) { this(apiKey, apiHost, Collections.emptyList()); } - public void setApiKey(String apiKey) { + public synchronized void setApiKey(String apiKey) { this.apiKey = apiKey; } - public void setApiHost(String apiHost) { + public synchronized void setApiHost(String apiHost) { this.apiHost = apiHost; } - public String getApiKey() { + public synchronized String getApiKey() { return apiKey; } - public String getApiHost() { + public synchronized String getApiHost() { return apiHost; } - public List getAllSegments() { + public synchronized List getAllSegments() { return allSegments; } - public void setAllSegments(List allSegments) { + public synchronized void setAllSegments(List allSegments) { this.allSegments = allSegments; } } diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java index 2d5372f9c..bde46015c 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java @@ -66,6 +66,16 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue) { } public List getQualifiedSegments(ODPUserKey userKey, String userValue, List options) { + if (!odpConfig.isReady()) { + logger.warn("ODP Config not ready. apiHost and/or apiKey null. Returning Empty list"); + return Collections.emptyList(); + } + + if (!odpConfig.hasSegments()) { + logger.debug("No Segments are used in the project, Not Fetching segments. Returning empty list"); + return Collections.emptyList(); + } + List qualifiedSegments; String cacheKey = getCacheKey(userKey.getKeyString(), userValue); @@ -79,7 +89,7 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue, L } } - logger.debug("ODP Cache Miss. Making a call to ODP Server"); + logger.debug("ODP Cache Miss. Making a call to ODP Server."); ResponseJsonParser parser = ResponseJsonParserFactory.getParser(); String qualifiedSegmentsResponse = apiManager.fetchQualifiedSegments(odpConfig.getApiKey(), odpConfig.getApiHost() + SEGMENT_URL_PATH, userKey.getKeyString(), userValue, odpConfig.getAllSegments()); diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java index da11f5a48..bfadeed55 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java @@ -16,8 +16,11 @@ */ package com.optimizely.ab.odp; +import ch.qos.logback.classic.Level; import com.optimizely.ab.internal.Cache; +import com.optimizely.ab.internal.LogbackVerifier; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import org.mockito.Mock; import org.mockito.Mockito; @@ -32,6 +35,9 @@ public class ODPSegmentManagerTest { + @Rule + public LogbackVerifier logbackVerifier = new LogbackVerifier(); + @Mock Cache> mockCache; @@ -62,6 +68,8 @@ public void cacheHit() { verify(mockCache, times(0)).save(any(), any()); verify(mockCache, times(0)).reset(); + logbackVerifier.expectMessage(Level.DEBUG, "ODP Cache Hit. Returning segments from Cache."); + assertEquals(Arrays.asList("segment1-cached", "segment2-cached"), segments); } @@ -84,6 +92,8 @@ public void cacheMiss() { verify(mockCache, times(1)).save("vuid-$-testId", Arrays.asList("segment1", "segment2")); verify(mockCache, times(0)).reset(); + logbackVerifier.expectMessage(Level.DEBUG, "ODP Cache Miss. Making a call to ODP Server."); + assertEquals(Arrays.asList("segment1", "segment2"), segments); } @@ -156,4 +166,46 @@ public void resetAndIgnoreCache() { assertEquals(Arrays.asList("segment1", "segment2"), segments); } + + @Test + public void odpConfigNotReady() { + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + + ODPConfig odpConfig = new ODPConfig(null, null, Arrays.asList("segment1", "segment2")); + ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId"); + + // Cache lookup called with correct key + verify(mockCache, times(0)).lookup("fs_user_id-$-testId"); + + // Cache hit! No api call was made to the server. + verify(mockApiManager, times(0)).fetchQualifiedSegments(any(), any(), any(), any(), any()); + verify(mockCache, times(0)).save(any(), any()); + verify(mockCache, times(0)).reset(); + + logbackVerifier.expectMessage(Level.WARN, "ODP Config not ready. apiHost and/or apiKey null. Returning Empty list"); + + assertEquals(Collections.emptyList(), segments); + } + + @Test + public void noSegmentsInProject() { + Mockito.when(mockCache.lookup(any())).thenReturn(Arrays.asList("segment1-cached", "segment2-cached")); + + ODPConfig odpConfig = new ODPConfig("testKey", "testHost", null); + ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); + List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId"); + + // Cache lookup called with correct key + verify(mockCache, times(0)).lookup("fs_user_id-$-testId"); + + // Cache hit! No api call was made to the server. + verify(mockApiManager, times(0)).fetchQualifiedSegments(any(), any(), any(), any(), any()); + verify(mockCache, times(0)).save(any(), any()); + verify(mockCache, times(0)).reset(); + + logbackVerifier.expectMessage(Level.DEBUG, "No Segments are used in the project, Not Fetching segments. Returning empty list"); + + assertEquals(Collections.emptyList(), segments); + } } From 81f35a5c8c2c268a45b661ccc7003ab057bea5a9 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Tue, 16 Aug 2022 17:26:00 -0700 Subject: [PATCH 4/6] Fixed synchronization issues --- .../main/java/com/optimizely/ab/odp/ODPConfig.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java index 0317b5283..ad8667eb4 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPConfig.java @@ -33,21 +33,21 @@ public ODPConfig(String apiKey, String apiHost, List allSegments) { this.allSegments = allSegments; } - public Boolean isReady() { + public ODPConfig(String apiKey, String apiHost) { + this(apiKey, apiHost, Collections.emptyList()); + } + + public synchronized Boolean isReady() { return !( this.apiKey == null || this.apiKey.isEmpty() || this.apiHost == null || this.apiHost.isEmpty() ); } - public Boolean hasSegments() { + public synchronized Boolean hasSegments() { return allSegments != null && !allSegments.isEmpty(); } - public ODPConfig(String apiKey, String apiHost) { - this(apiKey, apiHost, Collections.emptyList()); - } - public synchronized void setApiKey(String apiKey) { this.apiKey = apiKey; } From 89f6e668762b840e16d0eaf381d0631e0b819f79 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Wed, 17 Aug 2022 15:18:07 -0700 Subject: [PATCH 5/6] incorporated more review feedback --- .../java/com/optimizely/ab/odp/ODPSegmentManager.java | 2 +- .../com/optimizely/ab/odp/ODPSegmentManagerTest.java | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java index bde46015c..75b4d9dce 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java @@ -67,7 +67,7 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue) { public List getQualifiedSegments(ODPUserKey userKey, String userValue, List options) { if (!odpConfig.isReady()) { - logger.warn("ODP Config not ready. apiHost and/or apiKey null. Returning Empty list"); + logger.error("ODP is not enabled."); return Collections.emptyList(); } diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java index bfadeed55..c399aacdc 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java @@ -175,15 +175,13 @@ public void odpConfigNotReady() { ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId"); - // Cache lookup called with correct key + // No further methods should be called. verify(mockCache, times(0)).lookup("fs_user_id-$-testId"); - - // Cache hit! No api call was made to the server. verify(mockApiManager, times(0)).fetchQualifiedSegments(any(), any(), any(), any(), any()); verify(mockCache, times(0)).save(any(), any()); verify(mockCache, times(0)).reset(); - logbackVerifier.expectMessage(Level.WARN, "ODP Config not ready. apiHost and/or apiKey null. Returning Empty list"); + logbackVerifier.expectMessage(Level.ERROR, "ODP is not enabled."); assertEquals(Collections.emptyList(), segments); } @@ -196,10 +194,8 @@ public void noSegmentsInProject() { ODPSegmentManager segmentManager = new ODPSegmentManager(odpConfig, mockApiManager, mockCache); List segments = segmentManager.getQualifiedSegments(ODPUserKey.FS_USER_ID, "testId"); - // Cache lookup called with correct key + // No further methods should be called. verify(mockCache, times(0)).lookup("fs_user_id-$-testId"); - - // Cache hit! No api call was made to the server. verify(mockApiManager, times(0)).fetchQualifiedSegments(any(), any(), any(), any(), any()); verify(mockCache, times(0)).save(any(), any()); verify(mockCache, times(0)).reset(); From 96d8558f50fe75bd6d6b0273ffd756b1cde131bd Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Wed, 17 Aug 2022 16:47:49 -0700 Subject: [PATCH 6/6] added correct log message --- .../src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java | 2 +- .../test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java index 75b4d9dce..ffda9c19c 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPSegmentManager.java @@ -67,7 +67,7 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue) { public List getQualifiedSegments(ODPUserKey userKey, String userValue, List options) { if (!odpConfig.isReady()) { - logger.error("ODP is not enabled."); + logger.error("Audience segments fetch failed (ODP is not enabled)"); return Collections.emptyList(); } diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java index c399aacdc..f784d53d0 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPSegmentManagerTest.java @@ -181,7 +181,7 @@ public void odpConfigNotReady() { verify(mockCache, times(0)).save(any(), any()); verify(mockCache, times(0)).reset(); - logbackVerifier.expectMessage(Level.ERROR, "ODP is not enabled."); + logbackVerifier.expectMessage(Level.ERROR, "Audience segments fetch failed (ODP is not enabled)"); assertEquals(Collections.emptyList(), segments); }