Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down
54 changes: 35 additions & 19 deletions src/main/java/com/snowplowanalytics/snowplow/tracker/Tracker.java
Original file line number Diff line number Diff line change
Expand Up @@ -187,29 +187,49 @@ public DevicePlatform getPlatform() {
* @return the wrapper containing the Tracker parameters
*/
public TrackerParameters getParameters() {
return this.parameters;
return parameters;
}

// --- Event Tracking Functions

/**
* Handles tracking the different types of events that
* the Tracker can encounter.
* 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.
Comment on lines +198 to +204
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So pitiful that those legacy eCommerce transaction events force us to do this :(
I'm wondering... what if we return just the eventID of the transaction event without the eventID of all the ecommerce items events?
Essentially those item events are like entities tracked as events due to legacy.
BTW, just an idea, let's keep this as you implemented now.

* <p>
* <b>Implementation note: </b><em>As a side effect of adding a payload to the Emitter,
* it triggers an Emitter thread to emit a batch of events.</em>
*
* @param event the event to track
* @return a list of eventIDs (UUIDs)
*/
public void track(Event event) {
public List<String> track(Event event) {
List<String> results = new ArrayList<>();
// a list because Ecommerce events become multiple Payloads
List<Event> 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);

boolean addedToBuffer = emitter.add(payload);
if (addedToBuffer) {
results.add(payload.getEventId());
} else {
results.add(null);
}
}
return results;
}

private List<Event> eventTypeSpecificPreProcessing(Event event) {
Expand All @@ -223,31 +243,27 @@ private List<Event> 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)) {
final EcommerceTransaction ecommerceTransaction = (EcommerceTransaction) 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 {
Expand All @@ -257,10 +273,10 @@ private List<Event> eventTypeSpecificPreProcessing(Event event) {
}

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) {
Expand All @@ -269,7 +285,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);
}
}

Expand All @@ -293,8 +309,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()));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,12 @@ protected BatchEmitter(final Builder<?> builder) {
/**
* Adds a TrackerPayload to the concurrent queue buffer
* <p>
* <b>Implementation note: </b><em>Be aware that calling `close()` on a BatchEmitter instance
* has a side-effect and will shutdown that ExecutorService.</em>
* <b>Implementation note: </b><em>As a side effect it triggers an Emitter thread to emit a batch of events.</em>
*
* @param payload a payload
*/
@Override
public void add(final TrackerPayload payload) {
public boolean add(final TrackerPayload payload) {
boolean result = eventStore.addEvent(payload);

if (!isClosing) {
Expand All @@ -126,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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,16 @@ public abstract class AbstractEvent implements Event {

protected final List<SelfDescribingJson> 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<T extends Builder<T>> {

private List<SelfDescribingJson> 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();
Expand All @@ -71,31 +65,6 @@ public T customContext(List<SelfDescribingJson> 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).
*
Expand All @@ -108,17 +77,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.
*
Expand Down Expand Up @@ -146,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;
}

Expand All @@ -164,23 +118,6 @@ public List<SelfDescribingJson> 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.
*/
@Override
public long getDeviceCreatedTimestamp() {
return deviceCreatedTimestamp;
}

/**
* @return the event's true timestamp.
*/
Expand All @@ -189,14 +126,6 @@ public Long getTrueTimestamp() {
return trueTimestamp;
}

/**
* @return the event id
*/
@Override
public String getEventId() {
return this.eventId;
}

/**
* @return the event subject
*/
Expand All @@ -217,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, getEventId());
if (getTrueTimestamp()!=null) {
protected TrackerPayload putTrueTimestamp(TrackerPayload payload) {
if (getTrueTimestamp() != null) {
payload.add(Parameter.TRUE_TIMESTAMP, Long.toString(getTrueTimestamp()));
}
payload.add(Parameter.DEVICE_CREATED_TIMESTAMP, Long.toString(getDeviceCreatedTimestamp()));
return payload;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is quite ugly because the AbstractEvent tries to cover both types of events: SelfDescribing and atomic (primitive) events.

Otherwise we could use super to pass the values of the abstract class and everything would make much more sense. For example, ScreenView is quite ugly because of this.

A correct distinction between SelfDescribing and Primitive would help to write the code in this way.

public TrackerPayload getPayload() {
    TrackerPayload payload = super();
    payload.add(Parameter.EVENT [...]

and the abstract class would have...

public TrackerPayload getPayload() {
    TrackerPayload payload = new TrackerPayload();
    if (getTrueTimestamp() != null) {
        payload.add(Parameter.TRUE_TIMESTAMP, Long.toString(getTrueTimestamp()));
    }
    return payload;
}

Anyway, for now we can't do anything like this. It's ok until we don't change the way the events work in the tracker.

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,29 +140,13 @@ 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
*/
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.
Expand All @@ -179,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);
}
}
Loading