From 1ab0e1a974fa34875294567bbbb70b993c289466 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Fri, 14 Oct 2022 01:02:10 -0700 Subject: [PATCH 1/8] Implemented ODP Manager and added unit tests --- .../java/com/optimizely/ab/odp/ODPConfig.java | 4 + .../com/optimizely/ab/odp/ODPManager.java | 45 ++++++++ .../optimizely/ab/odp/ODPSegmentManager.java | 10 +- .../com/optimizely/ab/odp/ODPManagerTest.java | 107 ++++++++++++++++++ 4 files changed, 165 insertions(+), 1 deletion(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java create mode 100644 core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.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 index ad8667eb4..03634febd 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 @@ -71,4 +71,8 @@ public synchronized List getAllSegments() { public synchronized void setAllSegments(List allSegments) { this.allSegments = allSegments; } + + public Boolean equals(ODPConfig toCompare) { + return getApiHost().equals(toCompare.getApiHost()) && getApiKey().equals(toCompare.getApiKey()) && getAllSegments().equals(toCompare.allSegments); + } } 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 new file mode 100644 index 000000000..94c7d47d7 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPManager.java @@ -0,0 +1,45 @@ +package com.optimizely.ab.odp; + +import javax.annotation.Nonnull; +import java.util.List; + +public class ODPManager { + private volatile ODPConfig odpConfig; + private final ODPSegmentManager segmentManager; + private final ODPEventManager eventManager; + + public ODPManager(@Nonnull ODPConfig odpConfig, @Nonnull ODPApiManager apiManager) { + this(odpConfig, new ODPSegmentManager(odpConfig, apiManager), new ODPEventManager(odpConfig, apiManager)); + } + + public ODPManager(@Nonnull ODPConfig odpConfig, @Nonnull ODPSegmentManager segmentManager, @Nonnull ODPEventManager eventManager) { + this.odpConfig = odpConfig; + this.segmentManager = segmentManager; + this.eventManager = eventManager; + this.eventManager.start(); + } + + public ODPSegmentManager getSegmentManager() { + return segmentManager; + } + + public ODPEventManager getEventManager() { + return eventManager; + } + + public Boolean updateSettings(String apiHost, String apiKey, List allSegments) { + ODPConfig newConfig = new ODPConfig(apiKey, apiHost, allSegments); + if (!odpConfig.equals(newConfig)) { + odpConfig = newConfig; + eventManager.updateSettings(odpConfig); + segmentManager.resetCache(); + segmentManager.updateSettings(odpConfig); + return true; + } + return false; + } + + public void close() { + eventManager.stop(); + } +} 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 ffda9c19c..352c4ec8f 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 @@ -34,7 +34,7 @@ public class ODPSegmentManager { private final ODPApiManager apiManager; - private final ODPConfig odpConfig; + private volatile ODPConfig odpConfig; private final Cache> segmentsCache; @@ -105,4 +105,12 @@ public List getQualifiedSegments(ODPUserKey userKey, String userValue, L private String getCacheKey(String userKey, String userValue) { return userKey + "-$-" + userValue; } + + public void updateSettings(ODPConfig odpConfig) { + this.odpConfig = odpConfig; + } + + public void resetCache() { + segmentsCache.reset(); + } } diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java new file mode 100644 index 000000000..4a323d185 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java @@ -0,0 +1,107 @@ +package com.optimizely.ab.odp; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mock; +import org.mockito.Mockito; + +import java.util.Arrays; + +import static org.mockito.Matchers.*; +import static org.mockito.Mockito.*; +import static org.junit.Assert.*; + +public class ODPManagerTest { + private static final String API_RESPONSE = "{\"data\":{\"customer\":{\"audiences\":{\"edges\":[{\"node\":{\"name\":\"segment1\",\"state\":\"qualified\"}},{\"node\":{\"name\":\"segment2\",\"state\":\"qualified\"}}]}}}}"; + + @Mock + ODPApiManager mockApiManager; + + @Mock + ODPEventManager mockEventManager; + + @Mock + ODPSegmentManager mockSegmentManager; + + @Before + public void setup() { + mockApiManager = mock(ODPApiManager.class); + mockEventManager = mock(ODPEventManager.class); + mockSegmentManager = mock(ODPSegmentManager.class); + } + + @Test + public void shouldStartEventManagerWhenODPManagerIsInitialized() { + ODPConfig config = new ODPConfig("test-key", "test-host"); + ODPManager odpManager = new ODPManager(config, mockSegmentManager, mockEventManager); + verify(mockEventManager, times(1)).start(); + } + + @Test + public void shouldStopEventManagerWhenCloseIsCalled() { + ODPConfig config = new ODPConfig("test-key", "test-host"); + ODPManager odpManager = new ODPManager(config, mockSegmentManager, mockEventManager); + + // Stop is not called in the default flow. + verify(mockEventManager, times(0)).stop(); + + odpManager.close(); + // stop should be called when odpManager is closed. + verify(mockEventManager, times(1)).stop(); + } + + @Test + public void shouldUseNewSettingsInEventManagerWhenODPConfigIsUpdated() throws InterruptedException { + Mockito.when(mockApiManager.sendEvents(any(), any(), any())).thenReturn(200); + ODPConfig config = new ODPConfig("test-key", "test-host", Arrays.asList("segment1", "segment2")); + ODPManager odpManager = new ODPManager(config, mockApiManager); + + odpManager.getEventManager().identifyUser("vuid", "fsuid"); + Thread.sleep(2000); + verify(mockApiManager, times(1)) + .sendEvents(eq("test-key"), eq("test-host/v3/events"), any()); + + odpManager.updateSettings("test-host-updated", "test-key-updated", Arrays.asList("segment1")); + odpManager.getEventManager().identifyUser("vuid", "fsuid"); + Thread.sleep(1200); + verify(mockApiManager, times(1)) + .sendEvents(eq("test-key-updated"), eq("test-host-updated/v3/events"), any()); + } + + @Test + public void shouldUseNewSettingsInSegmentManagerWhenODPConfigIsUpdated() { + Mockito.when(mockApiManager.fetchQualifiedSegments(anyString(), anyString(), anyString(), anyString(), anyList())) + .thenReturn(API_RESPONSE); + ODPConfig config = new ODPConfig("test-key", "test-host", Arrays.asList("segment1", "segment2")); + ODPManager odpManager = new ODPManager(config, mockApiManager); + + odpManager.getSegmentManager().getQualifiedSegments("test-id"); + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(eq("test-key"), eq("test-host/v3/graphql"), any(), any(), any()); + + odpManager.updateSettings("test-host-updated", "test-key-updated", Arrays.asList("segment1")); + odpManager.getSegmentManager().getQualifiedSegments("test-id"); + verify(mockApiManager, times(1)) + .fetchQualifiedSegments(eq("test-key-updated"), eq("test-host-updated/v3/graphql"), any(), any(), any()); + } + + @Test + public void shouldGetEventManager() { + ODPConfig config = new ODPConfig("test-key", "test-host"); + ODPManager odpManager = new ODPManager(config, mockSegmentManager, mockEventManager); + assertNotNull(odpManager.getEventManager()); + + odpManager = new ODPManager(config, mockApiManager); + assertNotNull(odpManager.getEventManager()); + } + + @Test + public void shouldGetSegmentManager() { + ODPConfig config = new ODPConfig("test-key", "test-host"); + ODPManager odpManager = new ODPManager(config, mockSegmentManager, mockEventManager); + assertNotNull(odpManager.getSegmentManager()); + + odpManager = new ODPManager(config, mockApiManager); + assertNotNull(odpManager.getSegmentManager()); + } +} \ No newline at end of file From 007be8991f0aeb107ddf20d45c2dbc3b08590459 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Fri, 14 Oct 2022 01:13:44 -0700 Subject: [PATCH 2/8] increased wait time for event to flush --- .../src/test/java/com/optimizely/ab/odp/ODPManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java index 4a323d185..3d2ab0158 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java @@ -63,7 +63,7 @@ public void shouldUseNewSettingsInEventManagerWhenODPConfigIsUpdated() throws In odpManager.updateSettings("test-host-updated", "test-key-updated", Arrays.asList("segment1")); odpManager.getEventManager().identifyUser("vuid", "fsuid"); - Thread.sleep(1200); + Thread.sleep(2000); verify(mockApiManager, times(1)) .sendEvents(eq("test-key-updated"), eq("test-host-updated/v3/events"), any()); } From f493cbfc55ef49d4aa1598fd3a2d21536be9d63b Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Fri, 14 Oct 2022 01:18:41 -0700 Subject: [PATCH 3/8] Added license headers. --- .../java/com/optimizely/ab/odp/ODPManager.java | 16 ++++++++++++++++ .../com/optimizely/ab/odp/ODPManagerTest.java | 18 +++++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) 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 94c7d47d7..cb7e04f99 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 @@ -1,3 +1,19 @@ +/** + * + * 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 javax.annotation.Nonnull; diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java index 3d2ab0158..466f5c851 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java @@ -1,3 +1,19 @@ +/** + * + * 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 org.junit.Before; @@ -63,7 +79,7 @@ public void shouldUseNewSettingsInEventManagerWhenODPConfigIsUpdated() throws In odpManager.updateSettings("test-host-updated", "test-key-updated", Arrays.asList("segment1")); odpManager.getEventManager().identifyUser("vuid", "fsuid"); - Thread.sleep(2000); + Thread.sleep(1200); verify(mockApiManager, times(1)) .sendEvents(eq("test-key-updated"), eq("test-host-updated/v3/events"), any()); } From cf08d59366445fdad95ff03139b6df80f4c18579 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Fri, 14 Oct 2022 01:28:51 -0700 Subject: [PATCH 4/8] fixed SpotBugsTest --- .../src/test/java/com/optimizely/ab/odp/ODPManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java index 466f5c851..3e1f656f5 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java @@ -49,7 +49,7 @@ public void setup() { @Test public void shouldStartEventManagerWhenODPManagerIsInitialized() { ODPConfig config = new ODPConfig("test-key", "test-host"); - ODPManager odpManager = new ODPManager(config, mockSegmentManager, mockEventManager); + new ODPManager(config, mockSegmentManager, mockEventManager); verify(mockEventManager, times(1)).start(); } From ba0365f4f85281d7187afec45ad8f7f37a67dbc7 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Fri, 14 Oct 2022 01:35:08 -0700 Subject: [PATCH 5/8] Added a line break at the end of Test file. --- .../src/test/java/com/optimizely/ab/odp/ODPManagerTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java index 3e1f656f5..924c88836 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPManagerTest.java @@ -120,4 +120,4 @@ public void shouldGetSegmentManager() { odpManager = new ODPManager(config, mockApiManager); assertNotNull(odpManager.getSegmentManager()); } -} \ No newline at end of file +} From ad5c86adeb6e4d602e36eada29629e29212c888c Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Mon, 17 Oct 2022 13:13:26 -0700 Subject: [PATCH 6/8] Added force flush on when settings update. --- .../java/com/optimizely/ab/odp/ODPConfig.java | 4 +++ .../optimizely/ab/odp/ODPEventManager.java | 33 ++++++++++++++++--- .../ab/odp/ODPEventManagerTest.java | 11 ++++++- 3 files changed, 42 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 03634febd..b83a80bda 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 @@ -75,4 +75,8 @@ public synchronized void setAllSegments(List allSegments) { public Boolean equals(ODPConfig toCompare) { return getApiHost().equals(toCompare.getApiHost()) && getApiKey().equals(toCompare.getApiKey()) && getAllSegments().equals(toCompare.allSegments); } + + public ODPConfig getClone() { + return new ODPConfig(apiKey, apiHost, allSegments); + } } diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java index 7cc601f29..3bb4ec3e9 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java @@ -50,7 +50,7 @@ public class ODPEventManager { // The eventQueue needs to be thread safe. We are not doing anything extra for thread safety here // because `LinkedBlockingQueue` itself is thread safe. - private final BlockingQueue eventQueue = new LinkedBlockingQueue<>(); + private final BlockingQueue eventQueue = new LinkedBlockingQueue<>(); public ODPEventManager(@Nonnull ODPConfig odpConfig, @Nonnull ODPApiManager apiManager) { this(odpConfig, apiManager, null, null, null); @@ -71,7 +71,10 @@ public void start() { } public void updateSettings(ODPConfig odpConfig) { - this.odpConfig = odpConfig; + if (!this.odpConfig.equals(odpConfig)) { + eventQueue.offer(new FlushEvent(this.odpConfig)); + this.odpConfig = odpConfig; + } } public void identifyUser(@Nullable String vuid, String userId) { @@ -137,7 +140,7 @@ private class EventDispatcherThread extends Thread { public void run() { while (true) { try { - ODPEvent nextEvent; + Object nextEvent; // If batch has events, set the timeout to remaining time for flush interval, // otherwise wait for the new event indefinitely @@ -158,12 +161,17 @@ public void run() { continue; } + if (nextEvent instanceof FlushEvent) { + flush(((FlushEvent) nextEvent).getOdpConfig()); + continue; + } + if (currentBatch.size() == 0) { // Batch starting, create a new flush time nextFlushTime = new Date().getTime() + flushInterval; } - currentBatch.add(nextEvent); + currentBatch.add((ODPEvent) nextEvent); if (currentBatch.size() >= batchSize) { flush(); @@ -176,7 +184,7 @@ public void run() { logger.debug("Exiting ODP Event Dispatcher Thread."); } - private void flush() { + private void flush(ODPConfig odpConfig) { if (odpConfig.isReady()) { String payload = ODPJsonSerializerFactory.getSerializer().serializeEvents(currentBatch); String endpoint = odpConfig.getApiHost() + EVENT_URL_PATH; @@ -192,8 +200,23 @@ private void flush() { currentBatch.clear(); } + private void flush() { + flush(odpConfig); + } + public void signalStop() { shouldStop = true; } } + + private static class FlushEvent { + private final ODPConfig odpConfig; + public FlushEvent(ODPConfig odpConfig) { + this.odpConfig = odpConfig.getClone(); + } + + public ODPConfig getOdpConfig() { + return odpConfig; + } + } } diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java index 7be51e415..d4f7eba87 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java @@ -216,7 +216,16 @@ public void applyUpdatedODPConfigWhenAvailable() throws InterruptedException { Thread.sleep(500); Mockito.verify(mockApiManager, times(2)).sendEvents(eq("key"), eq("http://www.odp-host.com/v3/events"), any()); eventManager.updateSettings(new ODPConfig("new-key", "http://www.new-odp-host.com")); - Thread.sleep(1500); + + // Should immediately Flush current batch with old ODP config when settings are changed + Thread.sleep(100); + Mockito.verify(mockApiManager, times(3)).sendEvents(eq("key"), eq("http://www.odp-host.com/v3/events"), any()); + + // New events should use new config + for (int i = 0; i < 10; i++) { + eventManager.sendEvent(getEvent(i)); + } + Thread.sleep(100); Mockito.verify(mockApiManager, times(1)).sendEvents(eq("new-key"), eq("http://www.new-odp-host.com/v3/events"), any()); } From 3bf6a03c91debfaf92130be227c0842cb347e418 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Tue, 18 Oct 2022 10:57:41 -0700 Subject: [PATCH 7/8] Added data types verifcation for events --- .../java/com/optimizely/ab/odp/ODPConfig.java | 2 +- .../java/com/optimizely/ab/odp/ODPEvent.java | 17 +++++++++++++++++ .../com/optimizely/ab/odp/ODPEventManager.java | 7 +++++-- .../optimizely/ab/odp/ODPEventManagerTest.java | 16 ++++++++++++++++ 4 files changed, 39 insertions(+), 3 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 b83a80bda..25402b172 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 @@ -76,7 +76,7 @@ public Boolean equals(ODPConfig toCompare) { return getApiHost().equals(toCompare.getApiHost()) && getApiKey().equals(toCompare.getApiKey()) && getAllSegments().equals(toCompare.allSegments); } - public ODPConfig getClone() { + public synchronized ODPConfig getClone() { return new ODPConfig(apiKey, apiHost, allSegments); } } diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java index 903bcf663..d52318d70 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java @@ -17,6 +17,7 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.beans.Transient; import java.util.Collections; import java.util.Map; @@ -64,4 +65,20 @@ public Map getData() { public void setData(Map data) { this.data = data; } + + @Transient + public Boolean isDataValid() { + for (Object entry: this.data.values()) { + if ( + !( entry instanceof String + || entry instanceof Integer + || entry instanceof Long + || entry instanceof Boolean + || entry instanceof Float + || entry instanceof Double)) { + return false; + } + } + return true; + } } diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java index 3bb4ec3e9..b3884417e 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java @@ -71,8 +71,7 @@ public void start() { } public void updateSettings(ODPConfig odpConfig) { - if (!this.odpConfig.equals(odpConfig)) { - eventQueue.offer(new FlushEvent(this.odpConfig)); + if (!this.odpConfig.equals(odpConfig) && eventQueue.offer(new FlushEvent(this.odpConfig))) { this.odpConfig = odpConfig; } } @@ -88,6 +87,10 @@ public void identifyUser(@Nullable String vuid, String userId) { } public void sendEvent(ODPEvent event) { + if (!event.isDataValid()) { + logger.error("Error Sending Event: ODP data is not valid"); + return; + } event.setData(augmentCommonData(event.getData())); processEvent(event); } diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java index d4f7eba87..8b6c80bfe 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java @@ -229,6 +229,22 @@ public void applyUpdatedODPConfigWhenAvailable() throws InterruptedException { Mockito.verify(mockApiManager, times(1)).sendEvents(eq("new-key"), eq("http://www.new-odp-host.com/v3/events"), any()); } + @Test + public void validateEventData() { + ODPEvent event = new ODPEvent("type", "action", null, null); + Map data = new HashMap<>(); + + data.put("String", "string Value"); + data.put("Integer", 100); + data.put("Float", 33.89); + data.put("Boolean", true); + event.setData(data); + assertTrue(event.isDataValid()); + + data.put("RandomObject", new Object()); + assertFalse(event.isDataValid()); + } + private ODPEvent getEvent(int id) { Map identifiers = new HashMap<>(); identifiers.put("identifier1", "value1-" + id); From 620ca8dc9bcd9cc754f833ea983e89b5232504e9 Mon Sep 17 00:00:00 2001 From: zashraf1985 Date: Wed, 19 Oct 2022 18:41:49 -0700 Subject: [PATCH 8/8] 1. Allowed null value in event data 2. Fixed ODP event error message --- core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java | 3 ++- .../src/main/java/com/optimizely/ab/odp/ODPEventManager.java | 2 +- .../test/java/com/optimizely/ab/odp/ODPEventManagerTest.java | 1 + 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java index d52318d70..de7001ca8 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java @@ -75,7 +75,8 @@ public Boolean isDataValid() { || entry instanceof Long || entry instanceof Boolean || entry instanceof Float - || entry instanceof Double)) { + || entry instanceof Double + || entry == null)) { return false; } } diff --git a/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java b/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java index b3884417e..ab4ce301e 100644 --- a/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java +++ b/core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java @@ -88,7 +88,7 @@ public void identifyUser(@Nullable String vuid, String userId) { public void sendEvent(ODPEvent event) { if (!event.isDataValid()) { - logger.error("Error Sending Event: ODP data is not valid"); + logger.error("ODP event send failed (ODP data is not valid)"); return; } event.setData(augmentCommonData(event.getData())); diff --git a/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java b/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java index 8b6c80bfe..fd4287e0f 100644 --- a/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java +++ b/core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java @@ -238,6 +238,7 @@ public void validateEventData() { data.put("Integer", 100); data.put("Float", 33.89); data.put("Boolean", true); + data.put("null", null); event.setData(data); assertTrue(event.isDataValid());