Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support siteId per tracking request #116

Merged
merged 10 commits into from
Apr 16, 2024
20 changes: 16 additions & 4 deletions example/src/main/java/com/example/MainActivity.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,13 @@
import android.os.Bundle;
import android.os.Handler;
import android.os.Message;
import android.text.Editable;
import android.view.View;
import android.widget.EditText;
import android.widget.TextView;

import com.parsely.parselyandroid.ParselyTracker;
import com.parsely.parselyandroid.SiteIdSource;
import com.parsely.parselyandroid.ParselyTrackerInternal;
import com.parsely.parselyandroid.ParselyVideoMetadata;

Expand All @@ -23,6 +26,7 @@
*/
public class MainActivity extends Activity {

public static final String DEFAULT_SITE_ID = "example.com";
private InternalDebugOnlyData internalDebugOnlyData;

@Override
Expand All @@ -31,7 +35,7 @@ protected void onCreate(Bundle savedInstanceState) {
setContentView(R.layout.activity_main);

// initialize the Parsely tracker with your site id and the current Context
ParselyTracker.init("example.com", 30, this, true);
ParselyTracker.init(DEFAULT_SITE_ID, 30, this, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if ParselyTracker.init() shouldn't also receive a SiteIdSource 🤔
Are there reasons not to?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Because we need some site id to be defined. If ParselyTracker.init() received SiteIdSource, then API consumer could pass SiteIdSource.Default, which would have to be supported and it's not a valid state.

Alternatively, we could make ParselyTracker.init() receive SiteIdSource.Custom, but I think it only shadows implementation and causes confusion as it can raise a question: "custom" to what? In ParselyTracker.trackPageview it's "custom" to the value provided in initialization, so it makes sense there.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Because we need some site id to be defined. If ParselyTracker.init() received SiteIdSource, then API consumer could pass SiteIdSource.Default, which would have to be supported and it's not a valid state.

Alternatively, we could make ParselyTracker.init() receive SiteIdSource.Custom, but I think it only shadows implementation and causes confusion as it can raise a question: "custom" to what? In ParselyTracker.trackPageview it's "custom" to the value provided in initialization, so it makes sense there.

Got it! Makes sense 👍

internalDebugOnlyData = new InternalDebugOnlyData((ParselyTrackerInternal) ParselyTracker.sharedInstance());

final TextView intervalView = (TextView) findViewById(R.id.interval);
Expand Down Expand Up @@ -96,14 +100,14 @@ public void trackPageview(View view) {
// the post has an internet-accessible URL, we will crawl it. urlMetadata is only used
// in the case of app-only content that we can't crawl.
ParselyTracker.sharedInstance().trackPageview(
"http://example.com/article1.html", "http://example.com/", null, null
"http://example.com/article1.html", "http://example.com/", null, null, getSiteId()
);
}

public void startEngagement(View view) {
final Map<String, Object> extraData = new HashMap<>();
extraData.put("product-id", "12345");
ParselyTracker.sharedInstance().startEngagement("http://example.com/article1.html", "http://example.com/", extraData);
ParselyTracker.sharedInstance().startEngagement("http://example.com/article1.html", "http://example.com/", extraData, getSiteId());
updateEngagementStrings();
}

Expand All @@ -124,7 +128,7 @@ public void trackPlay(View view) {
90
);
// NOTE: For videos embedded in an article, "url" should be the URL for that article.
ParselyTracker.sharedInstance().trackPlay("http://example.com/app-videos", "", metadata, null);
ParselyTracker.sharedInstance().trackPlay("http://example.com/app-videos", "", metadata, null, getSiteId());

}

Expand All @@ -135,4 +139,12 @@ public void trackPause(View view) {
public void trackReset(View view) {
ParselyTracker.sharedInstance().resetVideo();
}

private SiteIdSource getSiteId() {
Editable fromEditText = ((EditText) findViewById(R.id.custom_site_id)).getText();
if (fromEditText == null || fromEditText.toString().isEmpty()) {
return SiteIdSource.Default.INSTANCE;
}
return new SiteIdSource.Custom(fromEditText.toString());
}
}
14 changes: 11 additions & 3 deletions example/src/main/res/layout/activity_main.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,27 @@
android:paddingRight="@dimen/activity_horizontal_margin"
android:paddingTop="@dimen/activity_vertical_margin"
tools:context=".MainActivity" >

<ImageView
android:id="@+id/imageView1"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_centerHorizontal="true"
android:src="@drawable/parsely_logo_horizontal" />


<EditText
android:id="@+id/custom_site_id"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_below="@id/imageView1"
android:layout_centerHorizontal="true"
android:hint="Custom site id. Default if empty"/>

<Button android:id="@+id/url_button"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_centerHorizontal="true"
android:layout_below="@id/imageView1"
android:layout_below="@id/custom_site_id"
android:text="@string/button_track_url"
android:onClick="trackPageview" />

Expand Down
33 changes: 27 additions & 6 deletions parsely/api/parsely.api
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ public abstract interface class com/parsely/parselyandroid/ParselyTracker {
public static fun init (Ljava/lang/String;Landroid/content/Context;)V
public abstract fun resetVideo ()V
public static fun sharedInstance ()Lcom/parsely/parselyandroid/ParselyTracker;
public abstract fun startEngagement (Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;)V
public abstract fun startEngagement (Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lcom/parsely/parselyandroid/SiteIdSource;)V
public abstract fun stopEngagement ()V
public abstract fun trackPageview (Ljava/lang/String;Ljava/lang/String;Lcom/parsely/parselyandroid/ParselyMetadata;Ljava/util/Map;)V
public abstract fun trackPageview (Ljava/lang/String;Ljava/lang/String;Lcom/parsely/parselyandroid/ParselyMetadata;Ljava/util/Map;Lcom/parsely/parselyandroid/SiteIdSource;)V
public abstract fun trackPause ()V
public abstract fun trackPlay (Ljava/lang/String;Ljava/lang/String;Lcom/parsely/parselyandroid/ParselyVideoMetadata;Ljava/util/Map;)V
public abstract fun trackPlay (Ljava/lang/String;Ljava/lang/String;Lcom/parsely/parselyandroid/ParselyVideoMetadata;Ljava/util/Map;Lcom/parsely/parselyandroid/SiteIdSource;)V
}

public final class com/parsely/parselyandroid/ParselyTracker$Companion {
Expand All @@ -35,13 +35,34 @@ public final class com/parsely/parselyandroid/ParselyTracker$Companion {
}

public final class com/parsely/parselyandroid/ParselyTracker$DefaultImpls {
public static synthetic fun startEngagement$default (Lcom/parsely/parselyandroid/ParselyTracker;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;ILjava/lang/Object;)V
public static synthetic fun trackPageview$default (Lcom/parsely/parselyandroid/ParselyTracker;Ljava/lang/String;Ljava/lang/String;Lcom/parsely/parselyandroid/ParselyMetadata;Ljava/util/Map;ILjava/lang/Object;)V
public static synthetic fun trackPlay$default (Lcom/parsely/parselyandroid/ParselyTracker;Ljava/lang/String;Ljava/lang/String;Lcom/parsely/parselyandroid/ParselyVideoMetadata;Ljava/util/Map;ILjava/lang/Object;)V
public static synthetic fun startEngagement$default (Lcom/parsely/parselyandroid/ParselyTracker;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;Lcom/parsely/parselyandroid/SiteIdSource;ILjava/lang/Object;)V
public static synthetic fun trackPageview$default (Lcom/parsely/parselyandroid/ParselyTracker;Ljava/lang/String;Ljava/lang/String;Lcom/parsely/parselyandroid/ParselyMetadata;Ljava/util/Map;Lcom/parsely/parselyandroid/SiteIdSource;ILjava/lang/Object;)V
public static synthetic fun trackPlay$default (Lcom/parsely/parselyandroid/ParselyTracker;Ljava/lang/String;Ljava/lang/String;Lcom/parsely/parselyandroid/ParselyVideoMetadata;Ljava/util/Map;Lcom/parsely/parselyandroid/SiteIdSource;ILjava/lang/Object;)V
}

public final class com/parsely/parselyandroid/ParselyVideoMetadata : com/parsely/parselyandroid/ParselyMetadata {
public fun <init> (Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/util/Calendar;I)V
public synthetic fun <init> (Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/util/Calendar;IILkotlin/jvm/internal/DefaultConstructorMarker;)V
}

public abstract class com/parsely/parselyandroid/SiteIdSource {
}

public final class com/parsely/parselyandroid/SiteIdSource$Custom : com/parsely/parselyandroid/SiteIdSource {
public fun <init> (Ljava/lang/String;)V
public final fun component1 ()Ljava/lang/String;
public final fun copy (Ljava/lang/String;)Lcom/parsely/parselyandroid/SiteIdSource$Custom;
public static synthetic fun copy$default (Lcom/parsely/parselyandroid/SiteIdSource$Custom;Ljava/lang/String;ILjava/lang/Object;)Lcom/parsely/parselyandroid/SiteIdSource$Custom;
public fun equals (Ljava/lang/Object;)Z
public final fun getSiteId ()Ljava/lang/String;
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
}

public final class com/parsely/parselyandroid/SiteIdSource$Default : com/parsely/parselyandroid/SiteIdSource {
public static final field INSTANCE Lcom/parsely/parselyandroid/SiteIdSource$Default;
public fun equals (Ljava/lang/Object;)Z
public fun hashCode ()I
public fun toString ()Ljava/lang/String;
}

Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,46 @@ class FunctionalTests {
}
}

/**
* In this scenario, the consumer app starts an engagement session and configures siteId
* with a custom value for the session.
*
* Intervals:
* With current implementation of `HeartbeatIntervalCalculator`, the next intervals are:
* - 10500ms for the first interval
* - 13650ms for the second interval
*
* So after ~27,2s we should observe 2 `heartbeat` events from `startEngagement`.
*
* They both should have the same custom site id.
*/
@Test
fun customSiteIdIsAppliedToConcurrentEventsInEngagementSession() {
ActivityScenario.launch(SampleActivity::class.java).use { scenario ->
// given
val customSiteId = "customSiteId"
val flushInterval = 30.seconds
scenario.onActivity { activity: Activity ->
beforeEach(activity)
server.enqueue(MockResponse().setResponseCode(200))
initializeTracker(activity, flushInterval)

// when
parselyTracker.trackPageview("engagementUrl")
parselyTracker.startEngagement("engagementUrl", siteIdSource = SiteIdSource.Custom(customSiteId))
}

// then
val request = server.takeRequest().toMap()["events"]!!

assertThat(request.filter { it.action=="heartbeat" }).hasSize(2)
.allSatisfy {
assertThat(it.idsite).isEqualTo(customSiteId)
}
}

}

private fun RecordedRequest.toMap(): Map<String, List<Event>> {
val listType: TypeReference<Map<String, List<Event>>> =
object : TypeReference<Map<String, List<Event>>>() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ internal interface ConnectivityStatusProvider {
fun isReachable(): Boolean
}

internal class AndroidConnectivityStatusProvider(private val context: Context): ConnectivityStatusProvider {
internal class AndroidConnectivityStatusProvider(private val context: Context):
ConnectivityStatusProvider {

override fun isReachable(): Boolean {
val cm = context.getSystemService(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
package com.parsely.parselyandroid

import com.parsely.parselyandroid.Logging.log
import java.util.Calendar
import java.util.TimeZone

internal class EventsBuilder(
private val deviceInfoRepository: DeviceInfoRepository,
private val siteId: String,
private val initializationSiteId: String,
private val clock: Clock,
) {
/**
Expand All @@ -16,6 +14,8 @@ internal class EventsBuilder(
* @param action Action to use (e.g. pageview, heartbeat, videostart, vheartbeat)
* @param metadata Metadata to attach to the event.
* @param extraData A Map of additional information to send with the event.
* @param uuid A unique identifier for the event.
* @param siteIdSource The source of the site ID to use for the event.
* @return A Map object representing the event to be sent to Parse.ly.
*/
fun buildEvent(
Expand All @@ -24,15 +24,19 @@ internal class EventsBuilder(
action: String,
metadata: ParselyMetadata?,
extraData: Map<String, Any>?,
uuid: String
uuid: String,
siteIdSource: SiteIdSource,
Comment on lines +27 to +28
Copy link
Collaborator

Choose a reason for hiding this comment

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

Parameter docs for these two are missing.
Perhaps adding more details specially on how siteIdSource works with its default can also be helpful.

): Map<String, Any> {
log("buildEvent called for %s/%s", action, url)

// Main event info
val event: MutableMap<String, Any> = HashMap()
event["url"] = url
event["urlref"] = urlRef
event["idsite"] = siteId
event["idsite"] = when (siteIdSource) {
is SiteIdSource.Default -> initializationSiteId
is SiteIdSource.Custom -> siteIdSource.siteId
}
event["action"] = action

// Make a copy of extraData and add some things.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ internal class FlushQueue(
return@launch
}

val jsonPayload = toParselyEventsPayload(eventsToSend)
if (skipSendingEvents) {
log("Debug mode on. Not sending to Parse.ly. Otherwise, would sent ${eventsToSend.size} events")
log("Debug mode on. Not sending to Parse.ly. Otherwise, would sent ${eventsToSend.size} events: $jsonPayload")
repository.remove(eventsToSend)
return@launch
}
log("Sending request with %d events", eventsToSend.size)
val jsonPayload = toParselyEventsPayload(eventsToSend)
log("POST Data %s", jsonPayload)
log("Requested %s", ParselyTrackerInternal.ROOT_URL)
restClient.send(jsonPayload)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,16 @@ public interface ParselyTracker {
* content). Do not use this for **content also hosted on** URLs Parse.ly
* would normally crawl.
* @param extraData A Map of additional information to send with the event.
* @param siteIdSource The source of the site ID to use for the event. If [SiteIdSource.Default],
* the site ID provided during [init] will be used. Otherwise, the site ID
* provided in the [SiteIdSource.Custom] object will be used.
*/
public fun trackPageview(
url: String,
urlRef: String = "",
urlMetadata: ParselyMetadata? = null,
extraData: Map<String, Any>? = null,
siteIdSource: SiteIdSource = SiteIdSource.Default,
)

/**
Expand All @@ -59,11 +63,13 @@ public interface ParselyTracker {
* @param url The URL of the tracked article (eg: “http://example.com/some-old/article.html“)
* @param urlRef The url of the page that linked to the page being engaged with. Analogous to HTTP referer
* @param extraData A map of additional information to send with the generated `heartbeat` events
* @param siteIdSource The source of the site ID to use for the event.
*/
public fun startEngagement(
url: String,
urlRef: String = "",
extraData: Map<String, Any>? = null
extraData: Map<String, Any>? = null,
siteIdSource: SiteIdSource = SiteIdSource.Default,
)

/**
Expand Down Expand Up @@ -96,12 +102,14 @@ public interface ParselyTracker {
* @param urlRef The url of the page that linked to the page being engaged with. Analogous to HTTP referer
* @param videoMetadata Metadata about the tracked video
* @param extraData A Map of additional information to send with the event
* @param siteIdSource The source of the site ID to use for the event.
*/
public fun trackPlay(
url: String,
urlRef: String = "",
videoMetadata: ParselyVideoMetadata,
extraData: Map<String, Any>? = null,
siteIdSource: SiteIdSource = SiteIdSource.Default,
)

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ internal class ParselyTrackerInternal internal constructor(
urlRef: String,
urlMetadata: ParselyMetadata?,
extraData: Map<String, Any>?,
siteIdSource: SiteIdSource,
) {
if (url.isBlank()) {
Logging.log("url cannot be empty")
Expand All @@ -105,15 +106,17 @@ internal class ParselyTrackerInternal internal constructor(
"pageview",
urlMetadata,
extraData,
pageViewUuid
pageViewUuid,
siteIdSource
)
)
}

override fun startEngagement(
url: String,
urlRef: String,
extraData: Map<String, Any>?
extraData: Map<String, Any>?,
siteIdSource: SiteIdSource,
) {
if (url.isBlank()) {
Logging.log("url cannot be empty")
Expand All @@ -130,7 +133,7 @@ internal class ParselyTrackerInternal internal constructor(

// Start a new EngagementTask
val event =
eventsBuilder.buildEvent(url, urlRef, "heartbeat", null, extraData, pageViewUuid)
eventsBuilder.buildEvent(url, urlRef, "heartbeat", null, extraData, pageViewUuid, siteIdSource)
engagementManager = EngagementManager(
this,
DEFAULT_ENGAGEMENT_INTERVAL_MILLIS.toLong(),
Expand All @@ -154,6 +157,7 @@ internal class ParselyTrackerInternal internal constructor(
urlRef: String,
videoMetadata: ParselyVideoMetadata,
extraData: Map<String, Any>?,
siteIdSource: SiteIdSource,
) {
if (url.isBlank()) {
Logging.log("url cannot be empty")
Expand All @@ -176,12 +180,12 @@ internal class ParselyTrackerInternal internal constructor(

// Enqueue the videostart
val videostartEvent =
eventsBuilder.buildEvent(url, urlRef, "videostart", videoMetadata, extraData, uuid)
eventsBuilder.buildEvent(url, urlRef, "videostart", videoMetadata, extraData, uuid, siteIdSource)
enqueueEvent(videostartEvent)

// Start a new engagement manager for the video.
val hbEvent =
eventsBuilder.buildEvent(url, urlRef, "vheartbeat", videoMetadata, extraData, uuid)
eventsBuilder.buildEvent(url, urlRef, "vheartbeat", videoMetadata, extraData, uuid, siteIdSource)
// TODO: Can we remove some metadata fields from this request?
videoEngagementManager = EngagementManager(
this,
Expand Down
18 changes: 18 additions & 0 deletions parsely/src/main/java/com/parsely/parselyandroid/SiteIdSource.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package com.parsely.parselyandroid

/**
* Configuration for the site ID to be used for an event.
*/
public sealed class SiteIdSource {
/**
* Instruct SDK to use site ID provided during [ParselyTracker.init].
*/
public data object Default : SiteIdSource()

/**
* Instruct SDK to override the site ID for the event.
*
* @param siteId The Parsely public site ID (e.g. "example.com") to use for the event.
*/
public data class Custom(val siteId: String) : SiteIdSource()
}
Loading
Loading