-
Notifications
You must be signed in to change notification settings - Fork 9
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
fix: improve push delivery for Android 14 and above #493
Changes from all 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 |
---|---|---|
|
@@ -23,7 +23,6 @@ import io.customer.datapipelines.plugins.ContextPlugin | |
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,7 +31,9 @@ import io.customer.sdk.core.module.CustomerIOModule | |
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.serializer | ||
|
@@ -173,6 +174,12 @@ class CustomerIO private constructor( | |
logger.debug("CustomerIO SDK initialized with DataPipelines module.") | ||
// Migrate unsent events from previous version | ||
migrateTrackingEvents() | ||
|
||
// save settings to storage | ||
analytics.configuration.let { config -> | ||
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? 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 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? 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 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(settings) | ||
} | ||
} | ||
|
||
override var profileAttributes: CustomAttributes | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
package io.customer.messagingpush | ||
|
||
import io.customer.messagingpush.di.httpClient | ||
import io.customer.messagingpush.network.HttpClient | ||
import io.customer.messagingpush.network.HttpRequestParams | ||
import io.customer.sdk.core.di.SDKComponent | ||
import io.customer.sdk.util.EventNames | ||
import org.json.JSONObject | ||
|
||
internal interface PushDeliveryTracker { | ||
fun trackMetric(token: String, event: String, deliveryId: String, onComplete: ((Result<Unit>) -> Unit?)? = null) | ||
} | ||
|
||
internal 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?)? | ||
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? 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.
Curious as to why we moved this to core? It seems like those events are very related to Data Pipelines
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.
Because multiple modules are using it