fix(odp): check odp identifiers not empty before sending (#509)

Author: jaeopt

core-api/src/main/java/com/optimizely/ab/Optimizely.java

@@ -1468,6 +1468,15 @@ public ODPManager getODPManager() {
         return odpManager;
     }
 
+
+    /**
+     * Send an event to the ODP server.
+     *
+     * @param type the event type (default = "fullstack").
+     * @param action the event action name.
+     * @param identifiers a dictionary for identifiers. The caller must provide at least one key-value pair unless non-empty common identifiers have been set already with {@link ODPManager.Builder#withUserCommonIdentifiers(Map) }.
+     * @param data a dictionary for associated data. The default event data will be added to this data before sending to the ODP server.
+     */
     public void sendODPEvent(@Nullable String type, @Nonnull String action, @Nullable Map<String, String> identifiers, @Nullable Map<String, Object> data) {
         if (odpManager != null) {
             ODPEvent event = new ODPEvent(type, action, identifiers, data);

core-api/src/main/java/com/optimizely/ab/odp/ODPEvent.java

@@ -24,10 +24,10 @@
 public class ODPEvent {
     public static final String EVENT_TYPE_FULLSTACK = "fullstack";
 
-    private String type;
-    private String action;
-    private Map<String, String> identifiers;
-    private Map<String, Object> data;
+    @Nonnull private String type;
+    @Nonnull private String action;
+    @Nonnull private Map<String, String> identifiers;
+    @Nonnull private Map<String, Object> data;
 
     public ODPEvent(@Nullable String type, @Nonnull String action, @Nullable Map<String, String> identifiers, @Nullable Map<String, Object> data) {
         this.type = type == null ? EVENT_TYPE_FULLSTACK : type;
@@ -84,4 +84,10 @@ public Boolean isDataValid() {
         }
         return true;
     }
+
+    @Transient
+    public Boolean isIdentifiersValid() {
+        return !identifiers.isEmpty();
+    }
+
 }

core-api/src/main/java/com/optimizely/ab/odp/ODPEventManager.java

@@ -119,12 +119,19 @@ public void identifyUser(@Nullable String vuid, @Nullable String userId) {
     }
 
     public void sendEvent(ODPEvent event) {
+        event.setData(augmentCommonData(event.getData()));
+        event.setIdentifiers(augmentCommonIdentifiers(event.getIdentifiers()));
+
+        if (!event.isIdentifiersValid()) {
+            logger.error("ODP event send failed (event identifiers must have at least one key-value pair)");
+            return;
+        }
+
         if (!event.isDataValid()) {
-            logger.error("ODP event send failed (ODP data is not valid)");
+            logger.error("ODP event send failed (event data is not valid)");
             return;
         }
-        event.setData(augmentCommonData(event.getData()));
-        event.setIdentifiers(augmentCommonIdentifiers(event.getIdentifiers()));
+
         processEvent(event);
     }
 

core-api/src/test/java/com/optimizely/ab/odp/ODPEventManagerTest.java

@@ -61,7 +61,7 @@ public void logAndDiscardEventWhenEventManagerIsNotRunning() {
         ODPConfig odpConfig = new ODPConfig("key", "host", null);
         ODPEventManager eventManager = new ODPEventManager(mockApiManager);
         eventManager.updateSettings(odpConfig);
-        ODPEvent event = new ODPEvent("test-type", "test-action", Collections.emptyMap(), Collections.emptyMap());
+        ODPEvent event = new ODPEvent("test-type", "test-action", Collections.singletonMap("any-key", "any-value"), Collections.emptyMap());
         eventManager.sendEvent(event);
         logbackVerifier.expectMessage(Level.WARN, "Failed to Process ODP Event. ODPEventManager is not running");
     }
@@ -72,7 +72,7 @@ public void logAndDiscardEventWhenODPConfigNotReady() {
         ODPEventManager eventManager = new ODPEventManager(mockApiManager);
         eventManager.updateSettings(odpConfig);
         eventManager.start();
-        ODPEvent event = new ODPEvent("test-type", "test-action", Collections.emptyMap(), Collections.emptyMap());
+        ODPEvent event = new ODPEvent("test-type", "test-action", Collections.singletonMap("any-key", "any-value"), Collections.emptyMap());
         eventManager.sendEvent(event);
         logbackVerifier.expectMessage(Level.DEBUG, "Unable to Process ODP Event. ODPConfig is not ready.");
     }
@@ -92,6 +92,21 @@ public void dispatchEventsInCorrectNumberOfBatches() throws InterruptedException
         Mockito.verify(mockApiManager, times(3)).sendEvents(eq("key"), eq("http://www.odp-host.com/v3/events"), any());
     }
 
+    @Test
+    public void logAndDiscardEventWhenIdentifiersEmpty()  throws InterruptedException {
+        int flushInterval = 0;
+        ODPConfig odpConfig = new ODPConfig("key", "http://www.odp-host.com", null);
+        ODPEventManager eventManager = new ODPEventManager(mockApiManager, null, flushInterval);
+        eventManager.updateSettings(odpConfig);
+        eventManager.start();
+
+        ODPEvent event = new ODPEvent("test-type", "test-action", Collections.emptyMap(), Collections.emptyMap());
+        eventManager.sendEvent(event);
+        Thread.sleep(500);
+        Mockito.verify(mockApiManager, never()).sendEvents(eq("key"), eq("http://www.odp-host.com/v3/events"), any());
+        logbackVerifier.expectMessage(Level.ERROR, "ODP event send failed (event identifiers must have at least one key-value pair)");
+    }
+
     @Test
     public void dispatchEventsWithCorrectPayload() throws InterruptedException {
         Mockito.reset(mockApiManager);