-
Notifications
You must be signed in to change notification settings - Fork 9
fix: improve push delivery for Android 14 and above #493
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
Changes from 2 commits
8623a75
44dafd2
bc258b1
21364bc
7b4ecc7
c54d6bf
fdbe919
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| package io.customer.sdk.data.model | ||
|
|
||
| import kotlinx.serialization.Serializable | ||
|
|
||
| @Serializable | ||
| data class Settings(val writeKey: String, val apiHost: String) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,10 @@ | ||
| package io.customer.datapipelines.util | ||
| package io.customer.sdk.util | ||
|
|
||
| /** | ||
| * Event names to identify specific events in data pipelines so they can be | ||
| * reflected on Journeys. | ||
| */ | ||
| internal object EventNames { | ||
| object EventNames { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious as to why we moved this to core? It seems like those events are very related to Data Pipelines
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because multiple modules are using it |
||
| const val DEVICE_UPDATE = "Device Created or Updated" | ||
| const val DEVICE_DELETE = "Device Deleted" | ||
| const val METRIC_DELIVERY = "Report Delivery Event" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -174,6 +174,7 @@ public final class io/customer/sdk/CustomerIO : io/customer/sdk/DataPipelineInst | |
| public fun getProfileAttributes ()Ljava/util/Map; | ||
| public fun getRegisteredDeviceToken ()Ljava/lang/String; | ||
| public fun getUserId ()Ljava/lang/String; | ||
| public final fun getWriteKey ()Ljava/lang/String; | ||
|
||
| public fun identify (Ljava/lang/String;Ljava/lang/Object;Lkotlinx/serialization/SerializationStrategy;)V | ||
| public fun initialize ()V | ||
| public static final fun instance ()Lio/customer/sdk/CustomerIO; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,6 @@ | |
| import io.customer.datapipelines.plugins.CustomerIODestination | ||
| import io.customer.datapipelines.plugins.DataPipelinePublishedEvents | ||
| import io.customer.datapipelines.plugins.ScreenFilterPlugin | ||
| import io.customer.datapipelines.util.EventNames | ||
| import io.customer.sdk.communication.Event | ||
| import io.customer.sdk.communication.subscribe | ||
| import io.customer.sdk.core.di.AndroidSDKComponent | ||
|
|
@@ -32,9 +31,12 @@ | |
| import io.customer.sdk.core.util.CioLogLevel | ||
| import io.customer.sdk.core.util.Logger | ||
| import io.customer.sdk.data.model.CustomAttributes | ||
| import io.customer.sdk.data.model.Settings | ||
| import io.customer.sdk.events.TrackMetric | ||
| import io.customer.sdk.util.EventNames | ||
| import io.customer.tracking.migration.MigrationProcessor | ||
| import kotlinx.serialization.SerializationStrategy | ||
| import kotlinx.serialization.json.Json | ||
| import kotlinx.serialization.serializer | ||
|
|
||
| /** | ||
|
|
@@ -173,6 +175,14 @@ | |
| logger.debug("CustomerIO SDK initialized with DataPipelines module.") | ||
| // Migrate unsent events from previous version | ||
| migrateTrackingEvents() | ||
|
|
||
| // save settings to storage | ||
| analytics.configuration.let { config -> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this configuration object is available through analytics, what's the main reason for storing it locally? Is it for cases when the SDK hasn't been initialized yet?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. analytics is a dependency/part of
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add a test for this?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let me try adding test for these
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would be nice to add these tests for the |
||
| val settings = Settings(writeKey = config.writeKey, apiHost = config.apiHost) | ||
| globalPreferenceStore.saveSettings( | ||
| Json.encodeToString(Settings.serializer(), settings) | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| override var profileAttributes: CustomAttributes | ||
|
|
@@ -282,6 +292,9 @@ | |
| override val userId: String? | ||
| get() = analytics.userId() | ||
|
|
||
| val writeKey: String | ||
| get() = analytics.configuration.writeKey | ||
|
|
||
| override var deviceAttributes: CustomAttributes | ||
| get() = emptyMap() | ||
| set(value) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,19 @@ public final class io/customer/messagingpush/ModuleMessagingPushFCM : io/custome | |
| public final class io/customer/messagingpush/ModuleMessagingPushFCM$Companion { | ||
| } | ||
|
|
||
| public abstract interface class io/customer/messagingpush/PushDeliveryTracker { | ||
| public abstract fun trackMetric (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V | ||
| } | ||
|
|
||
| public final class io/customer/messagingpush/PushDeliveryTracker$DefaultImpls { | ||
| public static synthetic fun trackMetric$default (Lio/customer/messagingpush/PushDeliveryTracker;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)V | ||
| } | ||
|
|
||
| public final class io/customer/messagingpush/PushDeliveryTrackerImpl : io/customer/messagingpush/PushDeliveryTracker { | ||
| public fun <init> ()V | ||
| public fun trackMetric (Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Lkotlin/jvm/functions/Function1;)V | ||
| } | ||
|
|
||
|
||
| public final class io/customer/messagingpush/activity/NotificationClickReceiverActivity : android/app/Activity, io/customer/sdk/tracking/TrackableScreen { | ||
| public static final field Companion Lio/customer/messagingpush/activity/NotificationClickReceiverActivity$Companion; | ||
| public static final field NOTIFICATION_PAYLOAD_EXTRA Ljava/lang/String; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| package io.customer.messagingpush | ||
|
|
||
| import HttpClient | ||
| import HttpRequestParams | ||
| import io.customer.messagingpush.di.httpClient | ||
| import io.customer.sdk.core.di.SDKComponent | ||
| import io.customer.sdk.util.EventNames | ||
| import org.json.JSONObject | ||
|
|
||
| interface PushDeliveryTracker { | ||
| fun trackMetric(token: String, event: String, deliveryId: String, onComplete: ((Result<Unit>) -> Unit?)? = null) | ||
| } | ||
|
|
||
| class PushDeliveryTrackerImpl : PushDeliveryTracker { | ||
|
||
|
|
||
| private val httpClient: HttpClient | ||
| get() = SDKComponent.httpClient | ||
|
|
||
| /** | ||
| * Tracks a metric by performing a single POST request with JSON. | ||
| * Returns a `Result<Unit>`. | ||
| */ | ||
| override fun trackMetric( | ||
| token: String, | ||
| event: String, | ||
| deliveryId: String, | ||
| onComplete: ((Result<Unit>) -> Unit?)? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need a callback here? I'm not sure if the caller can do much in case of failure?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added it, in case we wanted to do some logging based on it or perform any operation based on its result. |
||
| ) { | ||
| val propertiesJson = JSONObject().apply { | ||
| put("recipient", token) | ||
| put("metric", event.lowercase()) | ||
| put("deliveryId", deliveryId) | ||
| } | ||
| val topLevelJson = JSONObject().apply { | ||
| put("anonymousId", deliveryId) | ||
| put("properties", propertiesJson) | ||
| put("event", EventNames.METRIC_DELIVERY) | ||
| } | ||
|
|
||
| val params = HttpRequestParams( | ||
| path = "/track", | ||
| headers = mapOf( | ||
| "Content-Type" to "application/json; charset=utf-8" | ||
| ), | ||
| body = topLevelJson.toString() | ||
| ) | ||
|
|
||
| // Perform request | ||
| httpClient.request(params) { result -> | ||
| val mappedResult = result.map { /* we only need success/failure */ } | ||
| if (onComplete != null) { | ||
| onComplete(mappedResult) | ||
| } | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it make sense for this method to accept
Settingsso that the encoding/decoding is encapsulated within this store?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think it does make sense, i wanted to avoid strong coupling of serialization with storage, but since string format could it make it generic and prone to break, it probably wiser to make this method stricter.