From a053e5ba1a776eb9348a3d7a396cc91eb1103982 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Tue, 27 Aug 2019 16:16:17 -0700 Subject: [PATCH 1/2] Add default EventHandler. --- .../com/optimizely/ab/event/BatchEventProcessor.java | 7 ++++++- .../optimizely/ab/event/BatchEventProcessorTest.java | 9 ++++++--- core-httpclient-impl/README.md | 10 ++++++++-- 3 files changed, 20 insertions(+), 6 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java b/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java index c99d3bb52..630b26be4 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java +++ b/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java @@ -56,7 +56,7 @@ public class BatchEventProcessor implements EventProcessor, AutoCloseable { private static final Object FLUSH_SIGNAL = new Object(); private final BlockingQueue eventQueue; - private final EventHandler eventHandler; + final EventHandler eventHandler; final int batchSize; final long flushInterval; @@ -313,6 +313,11 @@ public BatchEventProcessor build(boolean shouldStart) { timeoutMillis = DEFAULT_TIMEOUT_INTERVAL; } + if (eventHandler == null) { + logger.warn("EventHandler was not configured, defaulting to a no-op handler."); + eventHandler = new NoopEventHandler(); + } + if (executor == null) { final ThreadFactory threadFactory = Executors.defaultThreadFactory(); executor = Executors.newSingleThreadExecutor(runnable -> { diff --git a/core-api/src/test/java/com/optimizely/ab/event/BatchEventProcessorTest.java b/core-api/src/test/java/com/optimizely/ab/event/BatchEventProcessorTest.java index b9d45fc88..ee3abb6fc 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/BatchEventProcessorTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/BatchEventProcessorTest.java @@ -252,7 +252,6 @@ public void testInvalidBatchSizeUsesDefault() { .withEventQueue(eventQueue) .withBatchSize(-1) .withFlushInterval(MAX_DURATION_MS) - .withEventHandler(new NoopEventHandler()) .withNotificationCenter(notificationCenter) .withTimeout(TIMEOUT_MS, TimeUnit.MILLISECONDS) .build(); @@ -266,7 +265,6 @@ public void testInvalidFlushIntervalUsesDefault() { .withEventQueue(eventQueue) .withBatchSize(MAX_BATCH_SIZE) .withFlushInterval(-1L) - .withEventHandler(new NoopEventHandler()) .withNotificationCenter(notificationCenter) .withTimeout(TIMEOUT_MS, TimeUnit.MILLISECONDS) .build(); @@ -280,7 +278,6 @@ public void testInvalidTimeoutUsesDefault() { .withEventQueue(eventQueue) .withBatchSize(MAX_BATCH_SIZE) .withFlushInterval(MAX_DURATION_MS) - .withEventHandler(new NoopEventHandler()) .withNotificationCenter(notificationCenter) .withTimeout(-1L, TimeUnit.MILLISECONDS) .build(); @@ -288,6 +285,12 @@ public void testInvalidTimeoutUsesDefault() { assertEquals(eventProcessor.timeoutMillis, BatchEventProcessor.DEFAULT_TIMEOUT_INTERVAL); } + @Test + public void testDefaultEventHandler() throws InterruptedException { + eventProcessor = BatchEventProcessor.builder().build(); + assertNotNull(eventProcessor.eventHandler); + } + private void setEventProcessor(EventHandler eventHandler) { eventProcessor = BatchEventProcessor.builder() .withEventQueue(eventQueue) diff --git a/core-httpclient-impl/README.md b/core-httpclient-impl/README.md index 4d7c32c14..0546a8f68 100644 --- a/core-httpclient-impl/README.md +++ b/core-httpclient-impl/README.md @@ -24,6 +24,8 @@ compile 'com.optimizely.ab:core-httpclient-impl:{VERSION}' ## Basic usage ```java +package com.optimizely; + import com.optimizely.ab.Optimizely; import com.optimizely.ab.OptimizelyFactory; @@ -38,9 +40,13 @@ public class App { ## Advanced usage ```java +package com.optimizely; + import com.optimizely.ab.Optimizely; +import com.optimizely.ab.config.ProjectConfigManager; import com.optimizely.ab.config.HttpProjectConfigManager; import com.optimizely.ab.event.AsyncEventHandler; +import com.optimizely.ab.event.EventHandler; import java.util.concurrent.TimeUnit; public class App { @@ -54,12 +60,12 @@ public class App { ProjectConfigManager projectConfigManager = HttpProjectConfigManager.builder() .withSdkKey(sdkKey) - .withPollingInterval(1, TimeUnit.MINUTES) + .withPollingInterval(1L, TimeUnit.MINUTES) .build(); Optimizely optimizely = Optimizely.builder() - .withConfig(projectConfigManager) .withEventHandler(eventHandler) + .withConfigManager(projectConfigManager) .build(); } } From 1a74cdc13fae8159ba0acdc70418504f02a40a8c Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Tue, 27 Aug 2019 17:52:03 -0700 Subject: [PATCH 2/2] Throw exception instead. --- .../com/optimizely/ab/event/BatchEventProcessor.java | 5 ++--- .../optimizely/ab/event/BatchEventProcessorTest.java | 12 ++++++++---- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java b/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java index 630b26be4..201f6e8ee 100644 --- a/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java +++ b/core-api/src/main/java/com/optimizely/ab/event/BatchEventProcessor.java @@ -56,7 +56,7 @@ public class BatchEventProcessor implements EventProcessor, AutoCloseable { private static final Object FLUSH_SIGNAL = new Object(); private final BlockingQueue eventQueue; - final EventHandler eventHandler; + private final EventHandler eventHandler; final int batchSize; final long flushInterval; @@ -314,8 +314,7 @@ public BatchEventProcessor build(boolean shouldStart) { } if (eventHandler == null) { - logger.warn("EventHandler was not configured, defaulting to a no-op handler."); - eventHandler = new NoopEventHandler(); + throw new IllegalArgumentException("EventHandler was not configured"); } if (executor == null) { diff --git a/core-api/src/test/java/com/optimizely/ab/event/BatchEventProcessorTest.java b/core-api/src/test/java/com/optimizely/ab/event/BatchEventProcessorTest.java index ee3abb6fc..8f8e94a8a 100644 --- a/core-api/src/test/java/com/optimizely/ab/event/BatchEventProcessorTest.java +++ b/core-api/src/test/java/com/optimizely/ab/event/BatchEventProcessorTest.java @@ -67,7 +67,9 @@ public void setUp() throws Exception { @After public void tearDown() throws Exception { - eventProcessor.close(); + if (eventProcessor != null) { + eventProcessor.close(); + } } @Test @@ -252,6 +254,7 @@ public void testInvalidBatchSizeUsesDefault() { .withEventQueue(eventQueue) .withBatchSize(-1) .withFlushInterval(MAX_DURATION_MS) + .withEventHandler(new NoopEventHandler()) .withNotificationCenter(notificationCenter) .withTimeout(TIMEOUT_MS, TimeUnit.MILLISECONDS) .build(); @@ -265,6 +268,7 @@ public void testInvalidFlushIntervalUsesDefault() { .withEventQueue(eventQueue) .withBatchSize(MAX_BATCH_SIZE) .withFlushInterval(-1L) + .withEventHandler(new NoopEventHandler()) .withNotificationCenter(notificationCenter) .withTimeout(TIMEOUT_MS, TimeUnit.MILLISECONDS) .build(); @@ -278,6 +282,7 @@ public void testInvalidTimeoutUsesDefault() { .withEventQueue(eventQueue) .withBatchSize(MAX_BATCH_SIZE) .withFlushInterval(MAX_DURATION_MS) + .withEventHandler(new NoopEventHandler()) .withNotificationCenter(notificationCenter) .withTimeout(-1L, TimeUnit.MILLISECONDS) .build(); @@ -285,10 +290,9 @@ public void testInvalidTimeoutUsesDefault() { assertEquals(eventProcessor.timeoutMillis, BatchEventProcessor.DEFAULT_TIMEOUT_INTERVAL); } - @Test - public void testDefaultEventHandler() throws InterruptedException { + @Test(expected = IllegalArgumentException.class) + public void testDefaultEventHandler() { eventProcessor = BatchEventProcessor.builder().build(); - assertNotNull(eventProcessor.eventHandler); } private void setEventProcessor(EventHandler eventHandler) {