Implement support for 204 No Content event responses.

asnare · asnare · commit 666f362333c6 · 2017-03-20T18:28:30.000+01:00
This is configurable, and intended for the web source instead of the 1x1 GIF.
diff --git a/src/main/java/io/divolte/server/BrowserSource.java b/src/main/java/io/divolte/server/BrowserSource.java
@@ -49,6 +49,7 @@ public BrowserSource(final ValidatedConfiguration vc,
         this(sourceName,
              vc.configuration().getSourceConfiguration(sourceName, BrowserSourceConfiguration.class).prefix,
              vc.configuration().getSourceConfiguration(sourceName, BrowserSourceConfiguration.class).eventSuffix,
+             vc.configuration().getSourceConfiguration(sourceName, BrowserSourceConfiguration.class).useNoContent,
              loadTrackingJavaScript(vc, sourceName),
              processingPool,
              vc.configuration().sourceIndex(sourceName));
@@ -57,6 +58,7 @@ public BrowserSource(final ValidatedConfiguration vc,
     private BrowserSource(final String sourceName,
                           final String pathPrefix,
                           final String eventSuffix,
+                          final boolean useNoContent,
                           final TrackingJavaScriptResource trackingJavascript,
                           final IncomingRequestProcessingPool processingPool,
                           final int sourceIndex) {
@@ -65,7 +67,7 @@ private BrowserSource(final String sourceName,
         this.eventSuffix = eventSuffix;
         javascriptName = trackingJavascript.getScriptName();
         javascriptHandler = new AllowedMethodsHandler(new JavaScriptHandler(trackingJavascript), Methods.GET);
-        final ClientSideCookieEventHandler clientSideCookieEventHandler = new ClientSideCookieEventHandler(processingPool, sourceIndex);
+        final ClientSideCookieEventHandler clientSideCookieEventHandler = new ClientSideCookieEventHandler(processingPool, useNoContent, sourceIndex);
         eventHandler = new AllowedMethodsHandler(clientSideCookieEventHandler, Methods.GET);
     }
 
diff --git a/src/main/java/io/divolte/server/ClientSideCookieEventHandler.java b/src/main/java/io/divolte/server/ClientSideCookieEventHandler.java
@@ -56,7 +56,7 @@ public final class ClientSideCookieEventHandler implements HttpHandler {
     private final static ETag SENTINEL_ETAG = new ETag(false, "6b3edc43-20ec-4078-bc47-e965dd76b88a");
     private final static String SENTINEL_ETAG_VALUE = SENTINEL_ETAG.toString();
 
-    private final ByteBuffer transparentImage;
+    private final Optional<ByteBuffer> transparentImage;
     private final IncomingRequestProcessingPool processingPool;
     private final int sourceIndex;
 
@@ -82,17 +82,23 @@ public final class ClientSideCookieEventHandler implements HttpHandler {
 
     private static final ObjectReader EVENT_PARAMETERS_READER = new ObjectMapper(new MincodeFactory()).reader();
 
-    public ClientSideCookieEventHandler(final IncomingRequestProcessingPool processingPool, final int sourceIndex) {
+    public ClientSideCookieEventHandler(final IncomingRequestProcessingPool processingPool,
+                                        final boolean sendEmptyResponse,
+                                        final int sourceIndex) {
         this.sourceIndex = sourceIndex;
         this.processingPool = Objects.requireNonNull(processingPool);
 
-        try {
-            this.transparentImage = ByteBuffer.wrap(
-                Resources.toByteArray(Resources.getResource("transparent1x1.gif"))
-            ).asReadOnlyBuffer();
-        } catch (final IOException e) {
-            // Should throw something more specific than this.
-            throw new RuntimeException("Could not load transparent image resource.", e);
+        if (sendEmptyResponse) {
+            this.transparentImage = Optional.empty();
+        } else {
+            try {
+                this.transparentImage = Optional.of(ByteBuffer.wrap(
+                    Resources.toByteArray(Resources.getResource("transparent1x1.gif"))
+                ).asReadOnlyBuffer());
+            } catch (final IOException e) {
+                // Should throw something more specific than this.
+                throw new RuntimeException("Could not load transparent image resource.", e);
+            }
         }
     }
 
@@ -109,17 +115,21 @@ public void handleRequest(final HttpServerExchange exchange) {
          * As a last resort, we try to detect duplicates via the ETag header.
          */
         exchange.getResponseHeaders()
-                .put(Headers.CONTENT_TYPE, "image/gif")
                 .put(Headers.ETAG, SENTINEL_ETAG_VALUE)
                 .put(Headers.CACHE_CONTROL, "private, no-cache, proxy-revalidate")
                 .put(Headers.PRAGMA, "no-cache")
                 .put(Headers.EXPIRES, "Fri, 14 Apr 1995 11:30:00 GMT");
 
         // If an ETag is present, this is a duplicate event.
         if (ETagUtils.handleIfNoneMatch(exchange, SENTINEL_ETAG, true)) {
-            // Default status code what we want: 200 OK.
             // Sending the response before logging the event!
-            exchange.getResponseSender().send(transparentImage.slice());
+            // We send either 200 OK or 204 No Content, depending on whether we have the image to send.
+            if (transparentImage.isPresent()) {
+                exchange.getResponseSender().send(transparentImage.get().slice());
+            } else {
+                exchange.setStatusCode(StatusCodes.NO_CONTENT)
+                        .endExchange();
+            }
 
             try {
                 logEvent(exchange);
@@ -131,8 +141,8 @@ public void handleRequest(final HttpServerExchange exchange) {
             if (logger.isDebugEnabled()) {
                 logger.debug("Ignoring duplicate event from {}: {}", sourceAddress, getFullUrl(exchange));
             }
-            exchange.setStatusCode(StatusCodes.NOT_MODIFIED);
-            exchange.endExchange();
+            exchange.setStatusCode(StatusCodes.NOT_MODIFIED)
+                    .endExchange();
         }
     }
 
@@ -166,13 +176,12 @@ public DivolteEvent parseRequest() throws IncompleteRequestException {
             final boolean isFirstInSession = queryParamFromExchange(exchange, FIRST_IN_SESSION_QUERY_PARAM).map(TRUE_STRING::equals).orElseThrow(IncompleteRequestException::new);
             final Instant clientTimeStamp = Instant.ofEpochMilli(queryParamFromExchange(exchange, CLIENT_TIMESTAMP_QUERY_PARAM).map(ClientSideCookieEventHandler::tryParseBase36Long).orElseThrow(IncompleteRequestException::new));
 
-            final DivolteEvent event = DivolteEvent.createBrowserEvent(exchange, corrupt, partyId, sessionId, eventId,
-                                                                       requestTime, clientTimeStamp,
-                                                                       isNewPartyId, isFirstInSession,
-                                                                       queryParamFromExchange(exchange, EVENT_TYPE_QUERY_PARAM),
-                                                                       eventParameterSupplier(exchange),
-                                                                       browserEventData(exchange, pageViewId));
-            return event;
+            return DivolteEvent.createBrowserEvent(exchange, corrupt, partyId, sessionId, eventId,
+                                                   requestTime, clientTimeStamp, isNewPartyId,
+                                                   isFirstInSession,
+                                                   queryParamFromExchange(exchange, EVENT_TYPE_QUERY_PARAM),
+                                                   eventParameterSupplier(exchange),
+                                                   browserEventData(exchange, pageViewId));
         }
     }
 
diff --git a/src/main/java/io/divolte/server/config/BrowserSourceConfiguration.java b/src/main/java/io/divolte/server/config/BrowserSourceConfiguration.java
@@ -22,6 +22,7 @@ public class BrowserSourceConfiguration extends SourceConfiguration {
     private static final String DEFAULT_PARTY_TIMEOUT = "730 days";
     private static final String DEFAULT_SESSION_COOKIE = "_dvs";
     private static final String DEFAULT_SESSION_TIMEOUT = "30 minutes";
+    private static final String DEFAULT_USE_NO_CONTENT = "false";
     private static final String DEFAULT_PREFIX = "/";
     private static final String DEFAULT_EVENT_SUFFIX = "csc-event";
 
@@ -33,6 +34,7 @@ public class BrowserSourceConfiguration extends SourceConfiguration {
             DurationDeserializer.parseDuration(DEFAULT_PARTY_TIMEOUT),
             DEFAULT_SESSION_COOKIE,
             DurationDeserializer.parseDuration(DEFAULT_SESSION_TIMEOUT),
+            DEFAULT_USE_NO_CONTENT,
             JavascriptConfiguration.DEFAULT_JAVASCRIPT_CONFIGURATION);
 
     public final String prefix;
@@ -42,6 +44,7 @@ public class BrowserSourceConfiguration extends SourceConfiguration {
     public final Duration partyTimeout;
     public final String sessionCookie;
     public final Duration sessionTimeout;
+    public final boolean useNoContent;
 
     @Valid
     public final JavascriptConfiguration javascript;
@@ -55,6 +58,7 @@ public class BrowserSourceConfiguration extends SourceConfiguration {
                                @JsonProperty(defaultValue=DEFAULT_PARTY_TIMEOUT) final Duration partyTimeout,
                                @JsonProperty(defaultValue=DEFAULT_SESSION_COOKIE) final String sessionCookie,
                                @JsonProperty(defaultValue=DEFAULT_SESSION_TIMEOUT) final Duration sessionTimeout,
+                               @JsonProperty(defaultValue=DEFAULT_USE_NO_CONTENT) final String useNoContent,
                                final JavascriptConfiguration javascript) {
         // TODO: register a custom deserializer with Jackson that uses the defaultValue property from the annotation to fix this
         this.prefix = Optional.ofNullable(prefix).map(BrowserSourceConfiguration::ensureTrailingSlash).orElse(DEFAULT_PREFIX);
@@ -64,6 +68,7 @@ public class BrowserSourceConfiguration extends SourceConfiguration {
         this.partyTimeout = Optional.ofNullable(partyTimeout).orElseGet(() -> DurationDeserializer.parseDuration(DEFAULT_PARTY_TIMEOUT));
         this.sessionCookie = Optional.ofNullable(sessionCookie).orElse(DEFAULT_SESSION_COOKIE);
         this.sessionTimeout = Optional.ofNullable(sessionTimeout).orElseGet(() -> DurationDeserializer.parseDuration(DEFAULT_SESSION_TIMEOUT));
+        this.useNoContent = Boolean.valueOf(Optional.ofNullable(useNoContent).orElse(DEFAULT_USE_NO_CONTENT));
         this.javascript = Optional.ofNullable(javascript).orElse(JavascriptConfiguration.DEFAULT_JAVASCRIPT_CONFIGURATION);
     }
 
@@ -81,6 +86,7 @@ protected MoreObjects.ToStringHelper toStringHelper() {
                 .add("partyTimeout", partyTimeout)
                 .add("sessionCookie", sessionCookie)
                 .add("sessionTimeout", sessionTimeout)
+                .add("useNoContent", useNoContent)
                 .add("javascript", javascript);
     }
 
diff --git a/src/main/java/io/divolte/server/js/TrackingJavaScriptResource.java b/src/main/java/io/divolte/server/js/TrackingJavaScriptResource.java
@@ -53,6 +53,7 @@ private static ImmutableMap<String, Object> createScriptConstants(final BrowserS
         builder.put(SCRIPT_CONSTANT_NAME, browserSourceConfiguration.javascript.name);
         builder.put("EVENT_SUFFIX", browserSourceConfiguration.eventSuffix);
         builder.put("AUTO_PAGE_VIEW_EVENT", browserSourceConfiguration.javascript.autoPageViewEvent);
+        builder.put("IMAGE_EXPECT_NOCONTENT", browserSourceConfiguration.useNoContent);
         return builder.build();
     }
 
diff --git a/src/main/resources/divolte.js b/src/main/resources/divolte.js
@@ -38,6 +38,8 @@ var SCRIPT_NAME = 'divolte.js';
 var EVENT_SUFFIX = 'csc-event';
 /** @define {boolean} */
 var AUTO_PAGE_VIEW_EVENT = true;
+/** @define {boolean} */
+var IMAGE_EXPECT_NOCONTENT = false;
 
 (function (global, factory) {
   factory(global);
@@ -674,8 +676,10 @@ var AUTO_PAGE_VIEW_EVENT = true;
       signalQueue.onFirstPendingEventCompleted();
     };
     image.onload = completionHandler;
-    image.onerror = !LOGGING ? completionHandler : function(event) {
-      warn("Error delivering event", firstPendingEvent);
+    image.onerror = IMAGE_EXPECT_NOCONTENT || !LOGGING ? completionHandler : function(event) {
+      if (!IMAGE_EXPECT_NOCONTENT) {
+        error("Error delivering event", firstPendingEvent);
+      }
       completionHandler();
     };
     // TODO: Implement a timeout for when neither onload or onerror are invoked.
diff --git a/src/test/java/io/divolte/server/SeleniumJavaScriptTest.java b/src/test/java/io/divolte/server/SeleniumJavaScriptTest.java
@@ -283,4 +283,23 @@ public void shouldUseConfiguredEventSuffix() throws Exception {
 
         assertEquals("pageView", eventData.eventType.get());
     }
+
+    @Test
+    public void shouldSupportNoContentResponsesForEventDelivery() throws Exception {
+        doSetUp("selenium-test-use-no-content-response.conf");
+        Preconditions.checkState(null != driver && null != server);
+        driver.get(urlOf(BASIC));
+
+        final EventPayload firstPayload = server.waitForEvent();
+        final DivolteEvent firstEventData = firstPayload.event;
+        assertEquals("pageView", firstEventData.eventType.get());
+
+        driver.findElement(By.id("custom")).click();
+        final EventPayload secondPayload = server.waitForEvent();
+        final DivolteEvent secondEventData = secondPayload.event;
+
+        assertTrue(secondEventData.eventType.isPresent());
+        assertEquals("custom", secondEventData.eventType.get());
+        // No need to check the parameters; we're only concerned that the event arrives.
+    }
 }
diff --git a/src/test/java/io/divolte/server/ServerSinkSourceConfigurationTest.java b/src/test/java/io/divolte/server/ServerSinkSourceConfigurationTest.java
@@ -178,6 +178,13 @@ public void shouldSupportLongSourcePaths() throws IOException, InterruptedExcept
         testServer.get().waitForEvent();
     }
 
+    @Test
+    public void shouldSupportNoContentResponsesFromBrowserSources() throws IOException {
+        // Test that the browser source returns 204 No Content when configured so.
+        startServer("browser-source-no-content-response.conf");
+        request("", 204);
+    }
+
     @Test
     public void shouldSupportMultipleBrowserSources() throws IOException, InterruptedException {
         // Test that multiple browser sources are supported.
diff --git a/src/test/resources/browser-source-no-content-response.conf b/src/test/resources/browser-source-no-content-response.conf
@@ -0,0 +1,28 @@
+//
+// Copyright 2016 GoDataDriven B.V.
+//
+// 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.
+//
+
+// Specify a single browser source that isn't used by any mappings.
+divolte {
+  sources.no-content-source {
+    type = browser
+    use_no_content = true
+  }
+  mappings.test = {
+    sources = [no-content-source]
+    // Need at least one sink.
+    sinks = [hdfs]
+  }
+}
diff --git a/src/test/resources/selenium-test-use-no-content-response.conf b/src/test/resources/selenium-test-use-no-content-response.conf
@@ -0,0 +1,21 @@
+//
+// Copyright 2016 GoDataDriven B.V.
+//
+// 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.
+//
+
+// Instead of using the default 'csc-event' endpoint, use a different value.
+divolte.sources.browser {
+  type = browser
+  use_no_content = true
+}

Original file line number	Diff line number	Diff line change
`@@ -53,6 +53,7 @@ private static ImmutableMap<String, Object> createScriptConstants(final BrowserS`
`53`	`53`	`builder.put(SCRIPT_CONSTANT_NAME, browserSourceConfiguration.javascript.name);`
`54`	`54`	`builder.put("EVENT_SUFFIX", browserSourceConfiguration.eventSuffix);`
`55`	`55`	`builder.put("AUTO_PAGE_VIEW_EVENT", browserSourceConfiguration.javascript.autoPageViewEvent);`
	`56`	`+ builder.put("IMAGE_EXPECT_NOCONTENT", browserSourceConfiguration.useNoContent);`
`56`	`57`	`return builder.build();`
`57`	`58`	`}`
`58`	`59`