From 8e20735feb68451fea9afcd4e155cc98b6446627 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Tue, 15 Feb 2022 15:20:04 +0000 Subject: [PATCH 1/8] Remove deviceCreatedTimestamp and eventId from AbstractEvent builder --- .../tracker/events/AbstractEvent.java | 45 ------------------- .../snowplow/tracker/events/Event.java | 7 --- 2 files changed, 52 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java index 92bb8988..306feb0d 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java @@ -71,31 +71,6 @@ public T customContext(List context) { return self(); } - /** - * A custom event timestamp. - * - * @param timestamp the event timestamp as - * unix epoch - * @return itself - * Use {@link #trueTimestamp} or {@link #deviceCreatedTimestamp} - */ - @Deprecated - public T timestamp(long timestamp) { - return deviceCreatedTimestamp(timestamp); - } - - /** - * Adjust the device-created timestamp. This is usually not what you want, check {@link #trueTimestamp}. - * - * @param timestamp the event timestamp as - * unix epoch - * @return itself - */ - public T deviceCreatedTimestamp(long timestamp) { - this.deviceCreatedTimestamp = timestamp; - return self(); - } - /** * The true timestamp of that event (as determined by the user). * @@ -108,17 +83,6 @@ public T trueTimestamp(Long timestamp) { return self(); } - /** - * A custom eventId for the event. - * - * @param eventId the eventId - * @return itself - */ - public T eventId(String eventId) { - this.eventId = eventId; - return self(); - } - /** * A custom subject for the event. * @@ -164,15 +128,6 @@ public List getContext() { return new ArrayList<>(this.context); } - /** - * @return the event's timestamp - * @deprecated Use {@link #getTrueTimestamp()} or {@link #getDeviceCreatedTimestamp()} - */ - @Override - public long getTimestamp() { - return this.deviceCreatedTimestamp; - } - /** * @return the event's device created timestamp. */ diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Event.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Event.java index 935279a1..3ae348e1 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Event.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Event.java @@ -28,13 +28,6 @@ public interface Event { */ List getContext(); - /** - * @return the event's timestamp - * Use {@link #getTrueTimestamp()} or {@link #getDeviceCreatedTimestamp()} - */ - @Deprecated - long getTimestamp(); - /** * @return the event's true timestamp */ From 50c274f7cfde42f977c2f684f38d3d13ee720b3c Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Tue, 15 Feb 2022 15:50:20 +0000 Subject: [PATCH 2/8] Set eid and dtm at TrackerPayload creation --- .../com/snowplowanalytics/snowplow/tracker/Tracker.java | 9 +++++++++ .../snowplow/tracker/events/AbstractEvent.java | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java index 8079ab9b..37e07633 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java @@ -256,6 +256,15 @@ private List eventTypeSpecificPreProcessing(Event event) { return eventList; } +// private void addDefaultPayloadParameters(Event event, TrackerPayload payload) { +// payload.add(Parameter.EID, Utils.getEventId()); +// if (event.getTrueTimestamp() != null) { +// payload.add(Parameter.TRUE_TIMESTAMP, Long.toString(event.getTrueTimestamp())); +// } +// payload.add(Parameter.DEVICE_CREATED_TIMESTAMP, Long.toString(System.currentTimeMillis())); +// +// } + private void addTrackerParameters(TrackerPayload payload) { payload.add(Parameter.PLATFORM, this.parameters.getPlatform().toString()); payload.add(Parameter.APP_ID, this.parameters.getAppId()); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java index 306feb0d..0a901033 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java @@ -173,11 +173,11 @@ public Subject getSubject() { * @return the TrackerPayload with appended values. */ protected TrackerPayload putDefaultParams(TrackerPayload payload) { - payload.add(Parameter.EID, getEventId()); + payload.add(Parameter.EID, Utils.getEventId()); if (getTrueTimestamp()!=null) { payload.add(Parameter.TRUE_TIMESTAMP, Long.toString(getTrueTimestamp())); } - payload.add(Parameter.DEVICE_CREATED_TIMESTAMP, Long.toString(getDeviceCreatedTimestamp())); + payload.add(Parameter.DEVICE_CREATED_TIMESTAMP, Long.toString(System.currentTimeMillis())); return payload; } } From 1ff0165abccf62ba23da7878cab33eb895df9b7b Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Thu, 3 Mar 2022 13:37:13 +0000 Subject: [PATCH 3/8] Remove method calls from Tracker (tests failing) --- .../snowplow/tracker/Tracker.java | 43 ++++++++----------- .../tracker/emitter/BatchEmitter.java | 3 +- .../tracker/events/AbstractEvent.java | 26 ----------- .../snowplow/tracker/events/Event.java | 10 ----- 4 files changed, 18 insertions(+), 64 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java index 37e07633..a7cfc8ac 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java @@ -195,6 +195,10 @@ public TrackerParameters getParameters() { /** * Handles tracking the different types of events that * the Tracker can encounter. + * A TrackerPayload object will be created from the Event. This is passed to the configured Emitter. + *

+ * Implementation note: As a side effect of adding a payload to the Emitter, + * it triggers an Emitter thread to emit a batch of events. * * @param event the event to track */ @@ -202,13 +206,13 @@ public void track(Event event) { // a list because Ecommerce events become multiple Payloads List processedEvents = eventTypeSpecificPreProcessing(event); for (Event processedEvent : processedEvents) { - // Event ID (eid) and device_created_timestamp (dtm) are generated when the Event is initialised + // Event ID (eid) and device_created_timestamp (dtm) are generated when the TrackerPayload is created TrackerPayload payload = (TrackerPayload) processedEvent.getPayload(); addTrackerParameters(payload); addContext(processedEvent, payload); addSubject(processedEvent, payload); - this.emitter.add(payload); + emitter.add(payload); } } @@ -223,7 +227,7 @@ private List eventTypeSpecificPreProcessing(Event event) { if (eventClass.equals(Unstructured.class)) { // Need to set the Base64 rule for Unstructured events final Unstructured unstructured = (Unstructured) event; - unstructured.setBase64Encode(this.parameters.getBase64Encoded()); + unstructured.setBase64Encode(parameters.getBase64Encoded()); eventList.add(unstructured); } else if (eventClass.equals(EcommerceTransaction.class)) { @@ -231,23 +235,19 @@ private List eventTypeSpecificPreProcessing(Event event) { eventList.add(ecommerceTransaction); // Track each item individually - for (final EcommerceTransactionItem item : ecommerceTransaction.getItems()) { - item.setDeviceCreatedTimestamp(ecommerceTransaction.getDeviceCreatedTimestamp()); - eventList.add(item); - } + eventList.addAll(ecommerceTransaction.getItems()); + } else if (eventClass.equals(Timing.class) || eventClass.equals(ScreenView.class)) { // Timing and ScreenView events are wrapper classes for Unstructured events // Need to create Unstructured events from them to send. final Unstructured unstructured = Unstructured.builder() .eventData((SelfDescribingJson) event.getPayload()) .customContext(event.getContext()) - .deviceCreatedTimestamp(event.getDeviceCreatedTimestamp()) .trueTimestamp(event.getTrueTimestamp()) - .eventId(event.getEventId()) .subject(event.getSubject()) .build(); - unstructured.setBase64Encode(this.parameters.getBase64Encoded()); + unstructured.setBase64Encode(parameters.getBase64Encoded()); eventList.add(unstructured); } else { @@ -256,20 +256,11 @@ private List eventTypeSpecificPreProcessing(Event event) { return eventList; } -// private void addDefaultPayloadParameters(Event event, TrackerPayload payload) { -// payload.add(Parameter.EID, Utils.getEventId()); -// if (event.getTrueTimestamp() != null) { -// payload.add(Parameter.TRUE_TIMESTAMP, Long.toString(event.getTrueTimestamp())); -// } -// payload.add(Parameter.DEVICE_CREATED_TIMESTAMP, Long.toString(System.currentTimeMillis())); -// -// } - private void addTrackerParameters(TrackerPayload payload) { - payload.add(Parameter.PLATFORM, this.parameters.getPlatform().toString()); - payload.add(Parameter.APP_ID, this.parameters.getAppId()); - payload.add(Parameter.NAMESPACE, this.parameters.getNamespace()); - payload.add(Parameter.TRACKER_VERSION, this.parameters.getTrackerVersion()); + payload.add(Parameter.PLATFORM, parameters.getPlatform().toString()); + payload.add(Parameter.APP_ID, parameters.getAppId()); + payload.add(Parameter.NAMESPACE, parameters.getNamespace()); + payload.add(Parameter.TRACKER_VERSION, parameters.getTrackerVersion()); } private void addContext(Event event, TrackerPayload payload) { @@ -278,7 +269,7 @@ private void addContext(Event event, TrackerPayload payload) { // Build the final context and add it to the payload if (entities != null && entities.size() > 0) { SelfDescribingJson envelope = getFinalContext(entities); - payload.addMap(envelope.getMap(), this.parameters.getBase64Encoded(), Parameter.CONTEXT_ENCODED, Parameter.CONTEXT); + payload.addMap(envelope.getMap(), parameters.getBase64Encoded(), Parameter.CONTEXT_ENCODED, Parameter.CONTEXT); } } @@ -302,8 +293,8 @@ private void addSubject(Event event, TrackerPayload payload) { // Add subject if available if (eventSubject != null) { payload.addMap(new HashMap<>(eventSubject.getSubject())); - } else if (this.subject != null) { - payload.addMap(new HashMap<>(this.subject.getSubject())); + } else if (subject != null) { + payload.addMap(new HashMap<>(subject.getSubject())); } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index c76951eb..5d68b095 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -108,8 +108,7 @@ protected BatchEmitter(final Builder builder) { /** * Adds a TrackerPayload to the concurrent queue buffer *

- * Implementation note: Be aware that calling `close()` on a BatchEmitter instance - * has a side-effect and will shutdown that ExecutorService. + * Implementation note: As a side effect it triggers an Emitter thread to emit a batch of events. * * @param payload a payload */ diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java index 0a901033..9b765fbc 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java @@ -40,22 +40,16 @@ public abstract class AbstractEvent implements Event { protected final List context; - protected long deviceCreatedTimestamp; - /** * The true timestamp may be null if none is set. */ protected Long trueTimestamp; - - protected final String eventId; protected final Subject subject; public static abstract class Builder> { private List context = new LinkedList<>(); - private long deviceCreatedTimestamp = System.currentTimeMillis(); protected Long trueTimestamp = null; - private String eventId = Utils.getEventId(); private Subject subject = null; protected abstract T self(); @@ -110,13 +104,9 @@ protected AbstractEvent(Builder builder) { // Precondition checks Preconditions.checkNotNull(builder.context); - Preconditions.checkNotNull(builder.eventId); - Preconditions.checkArgument(!builder.eventId.isEmpty(), "eventId cannot be empty"); this.context = builder.context; - this.deviceCreatedTimestamp = builder.deviceCreatedTimestamp; this.trueTimestamp = builder.trueTimestamp; - this.eventId = builder.eventId; this.subject = builder.subject; } @@ -128,14 +118,6 @@ public List getContext() { return new ArrayList<>(this.context); } - /** - * @return the event's device created timestamp. - */ - @Override - public long getDeviceCreatedTimestamp() { - return deviceCreatedTimestamp; - } - /** * @return the event's true timestamp. */ @@ -144,14 +126,6 @@ public Long getTrueTimestamp() { return trueTimestamp; } - /** - * @return the event id - */ - @Override - public String getEventId() { - return this.eventId; - } - /** * @return the event subject */ diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Event.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Event.java index 3ae348e1..b9909913 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Event.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Event.java @@ -33,16 +33,6 @@ public interface Event { */ Long getTrueTimestamp(); - /** - * @return the event's device created timestamp - */ - long getDeviceCreatedTimestamp(); - - /** - * @return the event id - */ - String getEventId(); - /** * @return the event subject */ From fd9c5d0a94d630647bbf37560e3f6b960ad741e8 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Fri, 4 Mar 2022 12:40:03 +0000 Subject: [PATCH 4/8] Add tests to TrackerPayload --- .../snowplow/tracker/Subject.java | 2 + .../snowplow/tracker/Tracker.java | 2 +- .../tracker/events/AbstractEvent.java | 2 +- .../events/EcommerceTransactionItem.java | 16 -- .../tracker/payload/TrackerPayload.java | 16 ++ .../snowplow/tracker/TrackerTest.java | 175 +++++++++--------- .../tracker/emitter/BatchEmitterTest.java | 9 +- .../tracker/payload/TrackerPayloadTest.java | 16 ++ 8 files changed, 125 insertions(+), 113 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java index c61958e8..1b0ff8f3 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Subject.java @@ -21,6 +21,8 @@ /** * An object for managing extra event decoration. + * All the properties are optional. However, the timezone is set by default, + * to that of the server. */ public class Subject { diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java index a7cfc8ac..75e815cc 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java @@ -187,7 +187,7 @@ public DevicePlatform getPlatform() { * @return the wrapper containing the Tracker parameters */ public TrackerParameters getParameters() { - return this.parameters; + return parameters; } // --- Event Tracking Functions diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java index 9b765fbc..a7c45ce9 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java @@ -148,7 +148,7 @@ public Subject getSubject() { */ protected TrackerPayload putDefaultParams(TrackerPayload payload) { payload.add(Parameter.EID, Utils.getEventId()); - if (getTrueTimestamp()!=null) { + if (getTrueTimestamp() != null) { payload.add(Parameter.TRUE_TIMESTAMP, Long.toString(getTrueTimestamp())); } payload.add(Parameter.DEVICE_CREATED_TIMESTAMP, Long.toString(System.currentTimeMillis())); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransactionItem.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransactionItem.java index 0a928f4c..0062b8e9 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransactionItem.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransactionItem.java @@ -140,15 +140,6 @@ protected EcommerceTransactionItem(Builder builder) { this.currency = builder.currency; } - /** - * @param timestamp the new timestamp - * Use {@link #setTrueTimestamp(long)} or {@link #setTrueTimestamp(long)} - */ - @Deprecated - public void setTimestamp(long timestamp) { - setDeviceCreatedTimestamp(timestamp); - } - /** * @param timestamp the new timestamp */ @@ -156,13 +147,6 @@ public void setTrueTimestamp(long timestamp) { this.trueTimestamp = timestamp; } - /** - * @param timestamp the new timestamp - */ - public void setDeviceCreatedTimestamp(Long timestamp) { - this.deviceCreatedTimestamp = timestamp; - } - /** * Returns a TrackerPayload which can be stored into * the local database. diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java index 93bb9303..bb146d0c 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java @@ -29,6 +29,22 @@ public class TrackerPayload implements Payload { private static final Logger LOGGER = LoggerFactory.getLogger(TrackerPayload.class); protected final Map payload = new LinkedHashMap<>(); + private final String eventId; + private final Long deviceCreatedTimestamp; + + + public TrackerPayload() { + eventId = Utils.getEventId(); + deviceCreatedTimestamp = System.currentTimeMillis(); + } + + public String getEventId() { + return eventId; + } + + public Long getDeviceCreatedTimestamp() { + return deviceCreatedTimestamp; + } /** * Add a key-value pair to the payload: - Checks that the key is not null or diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java index b2840266..b8cbedfc 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java @@ -15,8 +15,6 @@ import java.util.*; import static java.util.Collections.singletonList; -import com.snowplowanalytics.snowplow.tracker.emitter.BatchPayload; -import org.junit.Assert; import org.junit.Before; import org.junit.Test; import static org.junit.Assert.*; @@ -31,7 +29,6 @@ public class TrackerTest { public static final String EXPECTED_CONTEXTS = "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/contexts/jsonschema/1-0-1\",\"data\":[{\"schema\":\"schema\",\"data\":{\"foo\":\"bar\"}}]}"; - public static final String EXPECTED_EVENT_ID = "15e9b149-6029-4f6e-8447-5b9797c9e6be"; public static class MockEmitter implements Emitter { public ArrayList eventList = new ArrayList<>(); @@ -75,6 +72,27 @@ public void setUp() { // --- Event Tests + @Test + public void testEventHasEventIdAndDeviceTimestamp() throws InterruptedException { + tracker.track(Unstructured.builder() + .eventData(new SelfDescribingJson( + "iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0", + ImmutableMap.of("foo", "bar") + )) + .build()); + + Thread.sleep(500); + + Map result = mockEmitter.eventList.get(0).getMap(); + + // this throws an exception if it's not a valid UUID string + UUID.fromString(result.get("eid")); + + long currentTime = System.currentTimeMillis(); + long timeDifference = Long.parseLong(result.get("dtm")) - currentTime; + assertTrue(timeDifference < 1000); + } + @Test public void testEcommerceEvent() throws InterruptedException { // Given @@ -87,9 +105,7 @@ public void testEcommerceEvent() throws InterruptedException { .category("category") .currency("currency") .customContext(contexts) - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .build(); // When @@ -105,9 +121,7 @@ public void testEcommerceEvent() throws InterruptedException { .currency("currency") .items(item) .customContext(contexts) - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .build()); // Then @@ -117,15 +131,13 @@ public void testEcommerceEvent() throws InterruptedException { assertEquals(2, results.size()); Map result1 = results.get(0).getMap(); - assertEquals(ImmutableMap.builder() + Map expected1 = ImmutableMap.builder() .put("e", "tr") .put("tr_cu", "currency") .put("co", EXPECTED_CONTEXTS) - .put("eid", EXPECTED_EVENT_ID) .put("tna", "AF003") .put("aid", "cloudfront") .put("tr_sh", "3.0") - .put("dtm", "123456") .put("ttm", "456789") .put("tz", "Etc/UTC") .put("tr_co", "country") @@ -137,19 +149,19 @@ public void testEcommerceEvent() throws InterruptedException { .put("tr_tt", "1.0") .put("tr_ci", "city") .put("tr_st", "state") - .build(), result1); + .build(); + + assertTrue(result1.entrySet().containsAll(expected1.entrySet())); Map result2 = results.get(1).getMap(); - assertEquals(ImmutableMap.builder() + Map expected2 = ImmutableMap.builder() .put("ti_nm", "name") .put("ti_id", "order_id") .put("e", "ti") .put("co", EXPECTED_CONTEXTS) - .put("eid", EXPECTED_EVENT_ID) .put("tna", "AF003") .put("aid", "cloudfront") .put("ti_cu", "currency") - .put("dtm", "123456") .put("ttm", "456789") .put("tz", "Etc/UTC") .put("ti_pr", "1.0") @@ -158,7 +170,9 @@ public void testEcommerceEvent() throws InterruptedException { .put("tv", Version.TRACKER) .put("ti_ca", "category") .put("ti_sk", "sku") - .build(), result2); + .build(); + + assertTrue(result2.entrySet().containsAll(expected2.entrySet())); } @Test @@ -166,32 +180,30 @@ public void testUnstructuredEventWithContext() throws InterruptedException { // When tracker.track(Unstructured.builder() .eventData(new SelfDescribingJson( - "payload", + "iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0", ImmutableMap.of("foo", "bar") )) .customContext(contexts) - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .build()); // Then Thread.sleep(500); Map result = mockEmitter.eventList.get(0).getMap(); - assertEquals(ImmutableMap.builder() + Map expected = ImmutableMap.builder() .put("p", "srv") .put("tv", Version.TRACKER) .put("e", "ue") .put("co", EXPECTED_CONTEXTS) - .put("eid", EXPECTED_EVENT_ID) .put("tna", "AF003") .put("tz", "Etc/UTC") - .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"payload\",\"data\":{\"foo\":\"bar\"}}}") - .put("dtm", "123456") + .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0\",\"data\":{\"foo\":\"bar\"}}}") .put("ttm", "456789") .put("aid", "cloudfront") - .build(), result); + .build(); + + assertTrue(result.entrySet().containsAll(expected.entrySet())); } @Test @@ -199,30 +211,28 @@ public void testUnstructuredEventWithoutContext() throws InterruptedException { // When tracker.track(Unstructured.builder() .eventData(new SelfDescribingJson( - "payload", + "iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0", ImmutableMap.of("foo", "baær") )) - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .build()); // Then Thread.sleep(500); Map result = mockEmitter.eventList.get(0).getMap(); - assertEquals(ImmutableMap.builder() + Map expected = ImmutableMap.builder() .put("p", "srv") .put("tv", Version.TRACKER) - .put("eid", EXPECTED_EVENT_ID) .put("e", "ue") .put("tna", "AF003") .put("tz", "Etc/UTC") - .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"payload\",\"data\":{\"foo\":\"baær\"}}}") - .put("dtm", "123456") + .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0\",\"data\":{\"foo\":\"baær\"}}}") .put("ttm", "456789") .put("aid", "cloudfront") - .build(), result); + .build(); + + assertTrue(result.entrySet().containsAll(expected.entrySet())); } @Test @@ -230,33 +240,31 @@ public void testUnstructuredEventWithoutTrueTimestamp() throws InterruptedExcept // When tracker.track(Unstructured.builder() .eventData(new SelfDescribingJson( - "payload", + "iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0", ImmutableMap.of("foo", "baær") )) - .deviceCreatedTimestamp(123456) - .eventId(EXPECTED_EVENT_ID) .build()); // Then Thread.sleep(500); Map result = mockEmitter.eventList.get(0).getMap(); - assertEquals(ImmutableMap.builder() + Map expected = ImmutableMap.builder() .put("p", "srv") .put("tv", Version.TRACKER) - .put("eid", EXPECTED_EVENT_ID) .put("e", "ue") .put("tna", "AF003") .put("tz", "Etc/UTC") - .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"payload\",\"data\":{\"foo\":\"baær\"}}}") - .put("dtm", "123456") + .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0\",\"data\":{\"foo\":\"baær\"}}}") .put("aid", "cloudfront") - .build(), result); + .build(); + + assertTrue(result.entrySet().containsAll(expected.entrySet())); } @Test public void testTrackPageView() throws InterruptedException { - tracker = new Tracker.TrackerBuilder(this.mockEmitter, "AF003", "cloudfront") + tracker = new Tracker.TrackerBuilder(mockEmitter, "AF003", "cloudfront") .subject(new Subject.SubjectBuilder().build()) .base64(false) .build(); @@ -268,17 +276,14 @@ public void testTrackPageView() throws InterruptedException { .pageTitle("title") .referrer("referer") .customContext(contexts) - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .build()); // Then Thread.sleep(500); Map result = mockEmitter.eventList.get(0).getMap(); - assertEquals(ImmutableMap.builder() - .put("dtm", "123456") + Map expected = ImmutableMap.builder() .put("ttm", "456789") .put("tz", "Etc/UTC") .put("e", "pv") @@ -286,12 +291,13 @@ public void testTrackPageView() throws InterruptedException { .put("tv", Version.TRACKER) .put("p", "srv") .put("co", EXPECTED_CONTEXTS) - .put("eid", EXPECTED_EVENT_ID) .put("tna", "AF003") .put("aid", "cloudfront") .put("refr", "referer") .put("url", "url") - .build(), result); + .build(); + + assertTrue(result.entrySet().containsAll(expected.entrySet())); } @Test @@ -301,9 +307,7 @@ public void testTrackTwoEvents() throws InterruptedException { .pageUrl("url") .pageTitle("title") .referrer("referer") - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId("9783090a-dace-4c85-a75c-933b4596a6c5") .build()); Thread.sleep(500); @@ -312,9 +316,7 @@ public void testTrackTwoEvents() throws InterruptedException { .pageUrl("url") .pageTitle("title") .referrer("referer") - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId("39139d43-ea13-4163-8559-adea258bf9c4") .build()); // Then @@ -324,36 +326,36 @@ public void testTrackTwoEvents() throws InterruptedException { assertEquals(2, results.size()); Map result1 = results.get(0).getMap(); - assertEquals(ImmutableMap.builder() - .put("dtm", "123456") + Map expected1 = ImmutableMap.builder() .put("ttm", "456789") .put("tz", "Etc/UTC") .put("e", "pv") .put("page", "title") .put("tv", Version.TRACKER) .put("p", "srv") - .put("eid", "9783090a-dace-4c85-a75c-933b4596a6c5") .put("tna", "AF003") .put("aid", "cloudfront") .put("refr", "referer") .put("url", "url") - .build(), result1); + .build(); + + assertTrue(result1.entrySet().containsAll(expected1.entrySet())); Map result2 = results.get(1).getMap(); - assertEquals(ImmutableMap.builder() - .put("dtm", "123456") + Map expected2 = ImmutableMap.builder() .put("ttm", "456789") .put("tz", "Etc/UTC") .put("e", "pv") .put("page", "title") .put("tv", Version.TRACKER) .put("p", "srv") - .put("eid", "39139d43-ea13-4163-8559-adea258bf9c4") .put("tna", "AF003") .put("aid", "cloudfront") .put("refr", "referer") .put("url", "url") - .build(), result2); + .build(); + + assertTrue(result2.entrySet().containsAll(expected2.entrySet())); } @Test @@ -363,28 +365,26 @@ public void testTrackScreenView() throws InterruptedException { .name("name") .id("id") .customContext(contexts) - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .build()); // Then Thread.sleep(500); Map result = mockEmitter.eventList.get(0).getMap(); - assertEquals(ImmutableMap.builder() - .put("dtm", "123456") + Map expected = ImmutableMap.builder() .put("ttm", "456789") .put("tz", "Etc/UTC") .put("e", "ue") .put("tv", Version.TRACKER) .put("p", "srv") .put("co", EXPECTED_CONTEXTS) - .put("eid", EXPECTED_EVENT_ID) .put("tna", "AF003") .put("aid", "cloudfront") .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/screen_view/jsonschema/1-0-0\",\"data\":{\"id\":\"id\",\"name\":\"name\"}}}") - .build(), result); + .build(); + + assertTrue(result.entrySet().containsAll(expected.entrySet())); } @Test @@ -393,27 +393,25 @@ public void testTrackScreenViewWithTimestamp() throws InterruptedException { tracker.track(ScreenView.builder() .name("name") .id("id") - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .build()); // Then Thread.sleep(500); Map result = mockEmitter.eventList.get(0).getMap(); - assertEquals(ImmutableMap.builder() - .put("dtm", "123456") + Map expected = ImmutableMap.builder() .put("ttm", "456789") .put("tz", "Etc/UTC") .put("e", "ue") .put("tv", Version.TRACKER) .put("p", "srv") - .put("eid", EXPECTED_EVENT_ID) .put("tna", "AF003") .put("aid", "cloudfront") .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/screen_view/jsonschema/1-0-0\",\"data\":{\"id\":\"id\",\"name\":\"name\"}}}") - .build(), result); + .build(); + + assertTrue(result.entrySet().containsAll(expected.entrySet())); } @Test @@ -423,28 +421,26 @@ public void testTrackScreenViewWithDefaultContextAndTimestamp() throws Interrupt .name("name") .id("id") .customContext(contexts) - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .build()); // Then Thread.sleep(500); Map result = mockEmitter.eventList.get(0).getMap(); - assertEquals(ImmutableMap.builder() + Map expected = ImmutableMap.builder() .put("p", "srv") .put("tv", Version.TRACKER) .put("e", "ue") .put("co", EXPECTED_CONTEXTS) - .put("eid", EXPECTED_EVENT_ID) .put("tna", "AF003") .put("tz", "Etc/UTC") .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/screen_view/jsonschema/1-0-0\",\"data\":{\"id\":\"id\",\"name\":\"name\"}}}") - .put("dtm", "123456") .put("ttm", "456789") .put("aid", "cloudfront") - .build(), result); + .build(); + + assertTrue(result.entrySet().containsAll(expected.entrySet())); } @Test @@ -456,28 +452,26 @@ public void testTrackTiming() throws InterruptedException { .variable("variable") .timing(10) .customContext(contexts) - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .build()); // Then Thread.sleep(500); Map result = mockEmitter.eventList.get(0).getMap(); - assertEquals(ImmutableMap.builder() + Map expected = ImmutableMap.builder() .put("p", "srv") .put("tv", Version.TRACKER) .put("e", "ue") .put("co", EXPECTED_CONTEXTS) - .put("eid", EXPECTED_EVENT_ID) .put("tna", "AF003") .put("tz", "Etc/UTC") .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/timing/jsonschema/1-0-0\",\"data\":{\"category\":\"category\",\"label\":\"label\",\"timing\":10,\"variable\":\"variable\"}}}") - .put("dtm", "123456") .put("ttm", "456789") .put("aid", "cloudfront") - .build(), result); + .build(); + + assertTrue(result.entrySet().containsAll(expected.entrySet())); } @Test @@ -494,9 +488,7 @@ public void testTrackTimingWithSubject() throws InterruptedException { .variable("variable") .timing(10) .customContext(contexts) - .deviceCreatedTimestamp(123456) .trueTimestamp(456789L) - .eventId(EXPECTED_EVENT_ID) .subject(s1) .build()); @@ -504,20 +496,21 @@ public void testTrackTimingWithSubject() throws InterruptedException { Thread.sleep(500); Map result = mockEmitter.eventList.get(0).getMap(); - assertEquals(ImmutableMap.builder() + Map expected = ImmutableMap.builder() .put("p", "srv") .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/timing/jsonschema/1-0-0\",\"data\":{\"category\":\"category\",\"label\":\"label\",\"timing\":10,\"variable\":\"variable\"}}}") .put("tv", Version.TRACKER) .put("e", "ue") .put("ip", "127.0.0.1") .put("co", EXPECTED_CONTEXTS) - .put("eid", EXPECTED_EVENT_ID) .put("tna", "AF003") .put("tz", "Etc/UTC") - .put("dtm", "123456") .put("ttm", "456789") .put("aid", "cloudfront") - .build(), result); + .build(); + + assertTrue(result.entrySet().containsAll(expected.entrySet())); + } // --- Tracker Setter & Getter Tests @@ -538,17 +531,23 @@ public void testSetDefaultPlatform() { @Test public void testSetSubject() { + // Subject objects always have timezone set TimeZone.setDefault(TimeZone.getTimeZone("Etc/UTC")); + Subject s1 = new Subject.SubjectBuilder().build(); + s1.setLanguage("EN"); Tracker tracker = new Tracker.TrackerBuilder(mockEmitter, "AF003", "cloudfront") .subject(s1) .build(); + Subject s2 = new Subject.SubjectBuilder().build(); s2.setColorDepth(24); tracker.setSubject(s2); + Map subjectPairs = new HashMap<>(); subjectPairs.put("tz", "Etc/UTC"); subjectPairs.put("cd", "24"); + assertEquals(subjectPairs, tracker.getSubject().getSubject()); } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index 5c310e41..6fea25a8 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -25,9 +25,6 @@ import org.junit.Before; import org.junit.Test; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.*; - import com.snowplowanalytics.snowplow.tracker.payload.SelfDescribingJson; import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; import com.snowplowanalytics.snowplow.tracker.events.PageView; @@ -136,8 +133,6 @@ public void addToBuffer_withMore10Payloads_shouldEmptyBuffer() throws Interrupte Thread.sleep(500); Assert.assertTrue(mockHttpClientAdapter.isPostCalled); - @SuppressWarnings("unchecked") - List> capturedPayload = (List>) mockHttpClientAdapter.capturedPayload.getMap().get("data"); Assert.assertEquals(0, emitter.getBuffer().size()); Assert.assertEquals(1, mockHttpClientAdapter.postCounter); @@ -358,10 +353,10 @@ private void assertPayload(List payloads, List Date: Fri, 4 Mar 2022 17:20:51 +0000 Subject: [PATCH 5/8] Remove Hamcrest dependency --- build.gradle | 1 - 1 file changed, 1 deletion(-) diff --git a/build.gradle b/build.gradle index 639091a3..8fea99c3 100644 --- a/build.gradle +++ b/build.gradle @@ -83,7 +83,6 @@ dependencies { testCompileOnly 'junit:junit:4.13' testRuntimeOnly 'org.junit.vintage:junit-vintage-engine' - testImplementation 'org.hamcrest:hamcrest:2.2' testImplementation 'com.squareup.okhttp3:mockwebserver:4.9.2' } From eec1ce023e53da200d90dcb4e65766655ef0b44c Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Fri, 4 Mar 2022 18:03:34 +0000 Subject: [PATCH 6/8] Return eventId from Tracker.track() --- .../snowplow/tracker/Tracker.java | 21 ++++++- .../tracker/emitter/AbstractEmitter.java | 2 +- .../tracker/emitter/BatchEmitter.java | 4 +- .../snowplow/tracker/emitter/Emitter.java | 2 +- .../tracker/emitter/SimpleEmitter.java | 6 +- .../snowplow/tracker/TrackerTest.java | 61 +++++++++++++++---- .../tracker/emitter/BatchEmitterTest.java | 34 ++++------- 7 files changed, 90 insertions(+), 40 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java index 75e815cc..0c71640e 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java @@ -195,14 +195,22 @@ public TrackerParameters getParameters() { /** * Handles tracking the different types of events that * the Tracker can encounter. - * A TrackerPayload object will be created from the Event. This is passed to the configured Emitter. + * A TrackerPayload object - or more than one, in the case of eCommerceTransaction events - + * will be created from the Event. This is passed to the configured Emitter. + * If the event was successfully added to the Emitter buffer for sending, + * a list containing the payload's eventId string (a UUID) is returned. + * ECommerceTransactions will return all the relevant eventIds in the list. + * If the Emitter event buffer is full, the payload will be lost. In this case, this method + * returns a list containing null. *

* Implementation note: As a side effect of adding a payload to the Emitter, * it triggers an Emitter thread to emit a batch of events. * * @param event the event to track + * @return a list of eventIDs (UUIDs) */ - public void track(Event event) { + public List track(Event event) { + List results = new ArrayList<>(); // a list because Ecommerce events become multiple Payloads List processedEvents = eventTypeSpecificPreProcessing(event); for (Event processedEvent : processedEvents) { @@ -212,8 +220,15 @@ public void track(Event event) { addTrackerParameters(payload); addContext(processedEvent, payload); addSubject(processedEvent, payload); - emitter.add(payload); + + boolean addedToBuffer = emitter.add(payload); + if (addedToBuffer) { + results.add(payload.getEventId()); + } else { + results.add(null); + } } + return results; } private List eventTypeSpecificPreProcessing(Event event) { diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java index e56e4c14..00779196 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/AbstractEmitter.java @@ -133,7 +133,7 @@ protected AbstractEmitter(final Builder builder) { * @param payload an payload */ @Override - public abstract void add(TrackerPayload payload); + public abstract boolean add(TrackerPayload payload); /** * Customize the emitter batch size to any valid integer greater than zero. diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java index 5d68b095..c13ea4fe 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitter.java @@ -113,7 +113,7 @@ protected BatchEmitter(final Builder builder) { * @param payload a payload */ @Override - public void add(final TrackerPayload payload) { + public boolean add(final TrackerPayload payload) { boolean result = eventStore.addEvent(payload); if (!isClosing) { @@ -125,6 +125,8 @@ public void add(final TrackerPayload payload) { if (!result) { LOGGER.error("Unable to add payload to emitter, emitter buffer is full"); } + + return result; } /** diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java index aac70315..c9cb9b50 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/Emitter.java @@ -27,7 +27,7 @@ public interface Emitter { * * @param payload a payload to be emitted */ - void add(TrackerPayload payload); + boolean add(TrackerPayload payload); /** * Customize the emitter batch size to any valid integer diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java index 715da53b..a6cd3bf8 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/emitter/SimpleEmitter.java @@ -56,8 +56,12 @@ protected SimpleEmitter(final Builder builder) { * @param payload a payload */ @Override - public void add(TrackerPayload payload) { + public boolean add(TrackerPayload payload) { executor.execute(getGetRequestRunnable(payload)); + + // This result doesn't mean anything + // The return type is for BatchEmitter's benefit + return true; } /** diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java index b8cbedfc..322b4f62 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java @@ -34,25 +34,18 @@ public static class MockEmitter implements Emitter { public ArrayList eventList = new ArrayList<>(); @Override - public void add(TrackerPayload payload) { + public boolean add(TrackerPayload payload) { eventList.add(payload); + return true; } - @Override public void setBatchSize(int batchSize) {} - @Override public void flushBuffer() {} - @Override - public int getBatchSize() { - return 0; - } - + public int getBatchSize() { return 0; } @Override - public List getBuffer() { - return null; - } + public List getBuffer() { return null; } } MockEmitter mockEmitter; @@ -93,6 +86,52 @@ public void testEventHasEventIdAndDeviceTimestamp() throws InterruptedException assertTrue(timeDifference < 1000); } + @Test + public void testTrackReturnsEventID() throws InterruptedException { + // a list to allow for eCommerceTransaction + List result = tracker.track(Unstructured.builder() + .eventData(new SelfDescribingJson( + "iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0", + ImmutableMap.of("foo", "bar") + )) + .build()); + + Thread.sleep(500); + + // this throws an exception if it's not a valid UUID string + UUID.fromString(result.get(0)); + } + + @Test + public void testTrackReturnsNullIfEventWasDropped() throws InterruptedException { + class FailingMockEmitter implements Emitter { + @Override + public boolean add(TrackerPayload payload) { return false; } + @Override + public void setBatchSize(int batchSize) {} + @Override + public void flushBuffer() {} + @Override + public int getBatchSize() { return 0; } + @Override + public List getBuffer() { return null; } + } + FailingMockEmitter failingMockEmitter = new FailingMockEmitter(); + tracker = new Tracker.TrackerBuilder(failingMockEmitter, "AF003", "cloudfront").build(); + + List result = tracker.track(Unstructured.builder() + .eventData(new SelfDescribingJson( + "iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0", + ImmutableMap.of("foo", "bar") + )) + .build()); + + Thread.sleep(500); + + // this throws an exception if it's not a valid UUID string + assertNull(result.get(0)); + } + @Test public void testEcommerceEvent() throws InterruptedException { // Given diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java index 6fea25a8..2953c7f1 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/emitter/BatchEmitterTest.java @@ -57,14 +57,10 @@ public int get(TrackerPayload payload) { } @Override - public String getUrl() { - return null; - } + public String getUrl() { return null; } @Override - public Object getHttpClient() { - return null; - } + public Object getHttpClient() { return null; } } // this class fails to "send" the first 4 requests @@ -84,19 +80,13 @@ public int post(SelfDescribingJson payload) { } @Override - public int get(TrackerPayload payload) { - return 0; - } + public int get(TrackerPayload payload) { return 0; } @Override - public String getUrl() { - return null; - } + public String getUrl() { return null; } @Override - public Object getHttpClient() { - return null; - } + public Object getHttpClient() { return null; } } @Before @@ -111,16 +101,15 @@ public void setUp() { @Test public void addToBuffer_withLess10Payloads_shouldNotEmptyBuffer() throws InterruptedException { - List payloads = createPayloads(2); - for (TrackerPayload payload : payloads) { - emitter.add(payload); - } + TrackerPayload payload = createPayload(); + boolean result = emitter.add(payload); Thread.sleep(500); + Assert.assertTrue(result); Assert.assertFalse(mockHttpClientAdapter.isPostCalled); - Assert.assertEquals(2, emitter.getBuffer().size()); - Assert.assertEquals(payloads, emitter.getBuffer()); + Assert.assertEquals(1, emitter.getBuffer().size()); + Assert.assertEquals(payload, emitter.getBuffer().get(0)); } @Test @@ -148,9 +137,10 @@ public void addToBuffer_doesNotAddEventIfBufferFull() { emitter.add(createPayload()); TrackerPayload differentPayload = createPayload(); - emitter.add(differentPayload); + boolean result = emitter.add(differentPayload); Assert.assertFalse(emitter.getBuffer().contains(differentPayload)); + Assert.assertFalse(result); } @Test From 3d0b317f8928612b93415e155620623c55740357 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Mon, 7 Mar 2022 12:14:59 +0000 Subject: [PATCH 7/8] Add eid and dtm to TrackerPayload map directly --- .../snowplow/tracker/Tracker.java | 5 ++-- .../tracker/events/AbstractEvent.java | 4 +-- .../tracker/events/EcommerceTransaction.java | 2 +- .../events/EcommerceTransactionItem.java | 2 +- .../snowplow/tracker/events/PageView.java | 2 +- .../snowplow/tracker/events/ScreenView.java | 8 +++--- .../snowplow/tracker/events/Structured.java | 2 +- .../snowplow/tracker/events/Unstructured.java | 2 +- .../tracker/payload/TrackerPayload.java | 4 +++ .../snowplow/tracker/TrackerTest.java | 26 +++---------------- .../tracker/http/HttpClientAdapterTest.java | 7 ++++- .../payload/SelfDescribingJsonTest.java | 6 ++++- .../tracker/payload/TrackerPayloadTest.java | 6 +++++ 13 files changed, 38 insertions(+), 38 deletions(-) diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java index 0c71640e..abf41b58 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java @@ -199,7 +199,7 @@ public TrackerParameters getParameters() { * will be created from the Event. This is passed to the configured Emitter. * If the event was successfully added to the Emitter buffer for sending, * a list containing the payload's eventId string (a UUID) is returned. - * ECommerceTransactions will return all the relevant eventIds in the list. + * EcommerceTransactions will return all the relevant eventIds in the list. * If the Emitter event buffer is full, the payload will be lost. In this case, this method * returns a list containing null. *

@@ -214,7 +214,8 @@ public List track(Event event) { // a list because Ecommerce events become multiple Payloads List processedEvents = eventTypeSpecificPreProcessing(event); for (Event processedEvent : processedEvents) { - // Event ID (eid) and device_created_timestamp (dtm) are generated when the TrackerPayload is created + // Event ID (eid) and device_created_timestamp (dtm) are generated when + // the TrackerPayload is created TrackerPayload payload = (TrackerPayload) processedEvent.getPayload(); addTrackerParameters(payload); diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java index a7c45ce9..369bb3a1 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/AbstractEvent.java @@ -146,12 +146,10 @@ public Subject getSubject() { * @param payload the payload to add to. * @return the TrackerPayload with appended values. */ - protected TrackerPayload putDefaultParams(TrackerPayload payload) { - payload.add(Parameter.EID, Utils.getEventId()); + protected TrackerPayload putTrueTimestamp(TrackerPayload payload) { if (getTrueTimestamp() != null) { payload.add(Parameter.TRUE_TIMESTAMP, Long.toString(getTrueTimestamp())); } - payload.add(Parameter.DEVICE_CREATED_TIMESTAMP, Long.toString(System.currentTimeMillis())); return payload; } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransaction.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransaction.java index d7f31752..9151e014 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransaction.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransaction.java @@ -209,7 +209,7 @@ public TrackerPayload getPayload() { payload.add(Parameter.TR_STATE, this.state); payload.add(Parameter.TR_COUNTRY, this.country); payload.add(Parameter.TR_CURRENCY, this.currency); - return putDefaultParams(payload); + return putTrueTimestamp(payload); } /** diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransactionItem.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransactionItem.java index 0062b8e9..37a89cbc 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransactionItem.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/EcommerceTransactionItem.java @@ -163,6 +163,6 @@ public TrackerPayload getPayload() { payload.add(Parameter.TI_ITEM_PRICE, Double.toString(this.price)); payload.add(Parameter.TI_ITEM_QUANTITY, Integer.toString(this.quantity)); payload.add(Parameter.TI_ITEM_CURRENCY, this.currency); - return putDefaultParams(payload); + return putTrueTimestamp(payload); } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/PageView.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/PageView.java index 27bed72f..87da01f4 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/PageView.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/PageView.java @@ -102,6 +102,6 @@ public TrackerPayload getPayload() { payload.add(Parameter.PAGE_URL, this.pageUrl); payload.add(Parameter.PAGE_TITLE, this.pageTitle); payload.add(Parameter.PAGE_REFR, this.referrer); - return putDefaultParams(payload); + return putTrueTimestamp(payload); } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/ScreenView.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/ScreenView.java index d84adc57..9b1bf1a0 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/ScreenView.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/ScreenView.java @@ -19,6 +19,8 @@ import com.snowplowanalytics.snowplow.tracker.payload.SelfDescribingJson; import com.snowplowanalytics.snowplow.tracker.payload.TrackerPayload; +import java.util.LinkedHashMap; + public class ScreenView extends AbstractEvent { private final String name; @@ -79,9 +81,9 @@ protected ScreenView(Builder builder) { * @return the payload as a SelfDescribingJson. */ public SelfDescribingJson getPayload() { - TrackerPayload payload = new TrackerPayload(); - payload.add(Parameter.SV_ID, this.id); - payload.add(Parameter.SV_NAME, this.name); + LinkedHashMap payload = new LinkedHashMap<>(); + payload.put(Parameter.SV_ID, this.id); + payload.put(Parameter.SV_NAME, this.name); return new SelfDescribingJson(Constants.SCHEMA_SCREEN_VIEW, payload); } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Structured.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Structured.java index e80da843..06e7f282 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Structured.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Structured.java @@ -131,6 +131,6 @@ public TrackerPayload getPayload() { payload.add(Parameter.SE_PROPERTY, this.property); payload.add(Parameter.SE_VALUE, this.value != null ? Double.toString(this.value) : null); - return putDefaultParams(payload); + return putTrueTimestamp(payload); } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Unstructured.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Unstructured.java index a73b4575..37174422 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Unstructured.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/events/Unstructured.java @@ -89,6 +89,6 @@ public TrackerPayload getPayload() { payload.add(Parameter.EVENT, Constants.EVENT_UNSTRUCTURED); payload.addMap(envelope.getMap(), this.base64Encode, Parameter.UNSTRUCTURED_ENCODED, Parameter.UNSTRUCTURED); - return putDefaultParams(payload); + return putTrueTimestamp(payload); } } diff --git a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java index bb146d0c..21af7f98 100644 --- a/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java +++ b/src/main/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayload.java @@ -16,6 +16,7 @@ import java.util.LinkedHashMap; import java.util.Map; +import com.snowplowanalytics.snowplow.tracker.constants.Parameter; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -36,6 +37,9 @@ public class TrackerPayload implements Payload { public TrackerPayload() { eventId = Utils.getEventId(); deviceCreatedTimestamp = System.currentTimeMillis(); + + add(Parameter.EID, eventId); + add(Parameter.DEVICE_CREATED_TIMESTAMP, Long.toString(deviceCreatedTimestamp)); } public String getEventId() { diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java index 322b4f62..f570ad90 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java @@ -66,28 +66,7 @@ public void setUp() { // --- Event Tests @Test - public void testEventHasEventIdAndDeviceTimestamp() throws InterruptedException { - tracker.track(Unstructured.builder() - .eventData(new SelfDescribingJson( - "iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0", - ImmutableMap.of("foo", "bar") - )) - .build()); - - Thread.sleep(500); - - Map result = mockEmitter.eventList.get(0).getMap(); - - // this throws an exception if it's not a valid UUID string - UUID.fromString(result.get("eid")); - - long currentTime = System.currentTimeMillis(); - long timeDifference = Long.parseLong(result.get("dtm")) - currentTime; - assertTrue(timeDifference < 1000); - } - - @Test - public void testTrackReturnsEventID() throws InterruptedException { + public void testTrackReturnsEventIdIfSuccessful() throws InterruptedException { // a list to allow for eCommerceTransaction List result = tracker.track(Unstructured.builder() .eventData(new SelfDescribingJson( @@ -128,7 +107,6 @@ public void flushBuffer() {} Thread.sleep(500); - // this throws an exception if it's not a valid UUID string assertNull(result.get(0)); } @@ -423,6 +401,8 @@ public void testTrackScreenView() throws InterruptedException { .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/screen_view/jsonschema/1-0-0\",\"data\":{\"id\":\"id\",\"name\":\"name\"}}}") .build(); + System.out.println(expected); + System.out.println(result); assertTrue(result.entrySet().containsAll(expected.entrySet())); } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/http/HttpClientAdapterTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/http/HttpClientAdapterTest.java index 4152c374..aff33299 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/http/HttpClientAdapterTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/http/HttpClientAdapterTest.java @@ -92,10 +92,15 @@ public void get_withSuccessfulStatusCode_isOk() throws Exception { data.add("space", "b a r"); adapter.get(data); + String eventId = data.getEventId(); + String dtm = Long.toString(data.getDeviceCreatedTimestamp()); + // Then assertEquals(1, mockWebServer.getRequestCount()); RecordedRequest recordedRequest = mockWebServer.takeRequest(); - assertEquals("/i?foo=bar&space=b%20a%20r", recordedRequest.getPath()); + + String expectedString = "/i?eid=" + eventId + "&dtm=" + dtm + "&foo=bar&space=b%20a%20r"; + assertEquals(expectedString, recordedRequest.getPath()); assertEquals("GET", recordedRequest.getMethod()); } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJsonTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJsonTest.java index 1c831fac..77565065 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJsonTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/SelfDescribingJsonTest.java @@ -47,8 +47,12 @@ public void testMakeSdjWithObject() { public void testMakeSdjWithTrackerPayload() { TrackerPayload data = new TrackerPayload(); data.add("value", "key"); + String eventId = data.getEventId(); + String dtm = Long.toString(data.getDeviceCreatedTimestamp()); + SelfDescribingJson sdj = new SelfDescribingJson("schema_string", data); - String expected = "{\"schema\":\"schema_string\",\"data\":{\"value\":\"key\"}}"; + + String expected = "{\"schema\":\"schema_string\",\"data\":{\"eid\":\"" + eventId + "\",\"dtm\":\"" + dtm + "\",\"value\":\"key\"}}"; String sdjString = sdj.toString(); assertNotNull(sdj); assertEquals(expected, sdjString); diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayloadTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayloadTest.java index 9ac4667e..d5faeb0d 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayloadTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayloadTest.java @@ -29,6 +29,9 @@ public void testGetEventId() { TrackerPayload payload = new TrackerPayload(); // this throws an exception if it's not a valid UUID string UUID.fromString(payload.getEventId()); + + assertTrue(payload.getMap().containsKey("eid")); + assertEquals(payload.getEventId(), payload.getMap().get("eid")); } @Test @@ -37,6 +40,9 @@ public void testGetDeviceCreatedTimestamp() { TrackerPayload payload = new TrackerPayload(); long timeDifference = payload.getDeviceCreatedTimestamp() - currentTime; assertTrue(timeDifference < 1000); + + assertTrue(payload.getMap().containsKey("dtm")); + assertEquals(Long.toString(payload.getDeviceCreatedTimestamp()), payload.getMap().get("dtm")); } @Test From cfb5d5e52d469f1b08a2e89c4c2c1a9148ae5457 Mon Sep 17 00:00:00 2001 From: Miranda Wilson Date: Tue, 8 Mar 2022 11:15:21 +0000 Subject: [PATCH 8/8] Improve clarity of Tracker tests --- .../snowplow/tracker/TrackerTest.java | 39 ++++++++++--------- .../tracker/payload/TrackerPayloadTest.java | 10 ++++- 2 files changed, 28 insertions(+), 21 deletions(-) diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java index f570ad90..8b922cc0 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/TrackerTest.java @@ -77,8 +77,14 @@ public void testTrackReturnsEventIdIfSuccessful() throws InterruptedException { Thread.sleep(500); - // this throws an exception if it's not a valid UUID string - UUID.fromString(result.get(0)); + boolean isValidEventId = true; + try { + UUID.fromString(result.get(0)); + } catch (Exception e) { + isValidEventId = false; + } + + assertTrue(isValidEventId); } @Test @@ -258,7 +264,7 @@ public void testUnstructuredEventWithoutTrueTimestamp() throws InterruptedExcept tracker.track(Unstructured.builder() .eventData(new SelfDescribingJson( "iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0", - ImmutableMap.of("foo", "baær") + ImmutableMap.of("foo", "bar") )) .build()); @@ -324,15 +330,14 @@ public void testTrackTwoEvents() throws InterruptedException { .pageUrl("url") .pageTitle("title") .referrer("referer") - .trueTimestamp(456789L) + .trueTimestamp(123456L) .build()); - Thread.sleep(500); - - tracker.track(PageView.builder() - .pageUrl("url") - .pageTitle("title") - .referrer("referer") + tracker.track(Unstructured.builder() + .eventData(new SelfDescribingJson( + "iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0", + ImmutableMap.of("foo", "bar") + )) .trueTimestamp(456789L) .build()); @@ -344,7 +349,7 @@ public void testTrackTwoEvents() throws InterruptedException { Map result1 = results.get(0).getMap(); Map expected1 = ImmutableMap.builder() - .put("ttm", "456789") + .put("ttm", "123456") .put("tz", "Etc/UTC") .put("e", "pv") .put("page", "title") @@ -361,15 +366,13 @@ public void testTrackTwoEvents() throws InterruptedException { Map result2 = results.get(1).getMap(); Map expected2 = ImmutableMap.builder() .put("ttm", "456789") - .put("tz", "Etc/UTC") - .put("e", "pv") - .put("page", "title") - .put("tv", Version.TRACKER) .put("p", "srv") + .put("tv", Version.TRACKER) + .put("e", "ue") .put("tna", "AF003") + .put("tz", "Etc/UTC") + .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/example/jsonschema/1-0-0\",\"data\":{\"foo\":\"bar\"}}}") .put("aid", "cloudfront") - .put("refr", "referer") - .put("url", "url") .build(); assertTrue(result2.entrySet().containsAll(expected2.entrySet())); @@ -401,8 +404,6 @@ public void testTrackScreenView() throws InterruptedException { .put("ue_pr", "{\"schema\":\"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0\",\"data\":{\"schema\":\"iglu:com.snowplowanalytics.snowplow/screen_view/jsonschema/1-0-0\",\"data\":{\"id\":\"id\",\"name\":\"name\"}}}") .build(); - System.out.println(expected); - System.out.println(result); assertTrue(result.entrySet().containsAll(expected.entrySet())); } diff --git a/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayloadTest.java b/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayloadTest.java index d5faeb0d..0d566440 100644 --- a/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayloadTest.java +++ b/src/test/java/com/snowplowanalytics/snowplow/tracker/payload/TrackerPayloadTest.java @@ -27,9 +27,15 @@ public class TrackerPayloadTest { @Test public void testGetEventId() { TrackerPayload payload = new TrackerPayload(); - // this throws an exception if it's not a valid UUID string - UUID.fromString(payload.getEventId()); + boolean isValidEventId = true; + try { + UUID.fromString(payload.getEventId()); + } catch (Exception e) { + isValidEventId = false; + } + + assertTrue(isValidEventId); assertTrue(payload.getMap().containsKey("eid")); assertEquals(payload.getEventId(), payload.getMap().get("eid")); }