diff --git a/src/main/java/io/divolte/server/BrowserSource.java b/src/main/java/io/divolte/server/BrowserSource.java index d7190521..bdfa56b0 100644 --- 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 index 9af4cd48..687372f5 100644 --- 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 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,7 +115,6 @@ 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") @@ -117,9 +122,14 @@ public void handleRequest(final HttpServerExchange exchange) { // 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 index 3b45a1ca..775007e2 100644 --- 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 index fb3852ac..41074783 100644 --- 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 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 index f02bbe7c..b9136a9d 100644 --- 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); @@ -669,16 +671,32 @@ var AUTO_PAGE_VIEW_EVENT = true; SignalQueue.prototype.deliverFirstPendingEvent = function() { var signalQueue = this; var image = new Image(1,1); - image.onload = function() { - // Delete this signal from the array. - var pendingEvents = signalQueue.queue; - pendingEvents.shift(); - // If there are still pending events, schedule the next. - if (0 < pendingEvents.length) { - signalQueue.deliverFirstPendingEvent(); + var firstPendingEvent = signalQueue.queue[0]; + var completionHandler = function() { + signalQueue.onFirstPendingEventCompleted(); + }; + image.onload = completionHandler; + image.onerror = IMAGE_EXPECT_NOCONTENT || !LOGGING ? completionHandler : function(event) { + if (!IMAGE_EXPECT_NOCONTENT) { + error("Error delivering event", firstPendingEvent); } + completionHandler(); }; - image.src = divolteUrl + EVENT_SUFFIX + '?' + this.queue[0]; + // TODO: Implement a timeout for when neither onload or onerror are invoked. + image.src = divolteUrl + EVENT_SUFFIX + '?' + firstPendingEvent; + }; + /** + * @private + * Handler for when the first event in the queue has been completed. + */ + SignalQueue.prototype.onFirstPendingEventCompleted = function() { + // Delete the first event from the queue. + var pendingEvents = this.queue; + pendingEvents.shift(); + // If there are still pending events, schedule the next. + if (0 < pendingEvents.length) { + this.deliverFirstPendingEvent(); + } }; /** diff --git a/src/test/java/io/divolte/server/SeleniumJavaScriptTest.java b/src/test/java/io/divolte/server/SeleniumJavaScriptTest.java index 46c938fd..acc849ca 100644 --- 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 index 1a30cf7f..ddbebb34 100644 --- 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 new file mode 100644 index 00000000..63067bcc --- /dev/null +++ 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 new file mode 100644 index 00000000..5d6191e2 --- /dev/null +++ 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 +}