From 879307ed36e8975ca553739cfc1031ddc89fba36 Mon Sep 17 00:00:00 2001 From: tjroach Date: Fri, 15 Sep 2023 12:00:17 -0400 Subject: [PATCH 1/4] Fix Analytics/Push OptIn issue --- .../analytics/pinpoint/PinpointManager.kt | 9 +- .../AWSPinpointAnalyticsPluginBehaviorTest.kt | 3 +- .../pinpoint/core/TargetingClient.kt | 178 ++---------------- .../core/endpointProfile/EndpointProfile.kt | 9 +- .../core/ConstructTargetingClasses.kt | 11 +- .../pinpoint/core/TargetingClientTest.kt | 96 ++++++++-- .../AWSPinpointPushNotificationsPlugin.kt | 15 +- 7 files changed, 131 insertions(+), 190 deletions(-) diff --git a/aws-analytics-pinpoint/src/main/java/com/amplifyframework/analytics/pinpoint/PinpointManager.kt b/aws-analytics-pinpoint/src/main/java/com/amplifyframework/analytics/pinpoint/PinpointManager.kt index 56beb1db79..f0f3112e1d 100644 --- a/aws-analytics-pinpoint/src/main/java/com/amplifyframework/analytics/pinpoint/PinpointManager.kt +++ b/aws-analytics-pinpoint/src/main/java/com/amplifyframework/analytics/pinpoint/PinpointManager.kt @@ -17,6 +17,7 @@ package com.amplifyframework.analytics.pinpoint import android.content.Context import aws.sdk.kotlin.services.pinpoint.PinpointClient import aws.smithy.kotlin.runtime.auth.awscredentials.CredentialsProvider +import com.amplifyframework.core.store.EncryptedKeyValueRepository import com.amplifyframework.pinpoint.core.AnalyticsClient import com.amplifyframework.pinpoint.core.TargetingClient import com.amplifyframework.pinpoint.core.data.AndroidAppDetails @@ -61,12 +62,18 @@ internal class PinpointManager constructor( Context.MODE_PRIVATE ) + val encryptedStore = EncryptedKeyValueRepository( + context, + "${awsPinpointConfiguration.appId}$PINPOINT_SHARED_PREFS_SUFFIX" + ) + val androidAppDetails = AndroidAppDetails(context, awsPinpointConfiguration.appId) val androidDeviceDetails = AndroidDeviceDetails(context) targetingClient = TargetingClient( context, pinpointClient, - sharedPrefs, + encryptedStore, + sharedPrefs.getUniqueId(), androidAppDetails, androidDeviceDetails, ) diff --git a/aws-analytics-pinpoint/src/test/java/com/amplifyframework/analytics/pinpoint/AWSPinpointAnalyticsPluginBehaviorTest.kt b/aws-analytics-pinpoint/src/test/java/com/amplifyframework/analytics/pinpoint/AWSPinpointAnalyticsPluginBehaviorTest.kt index b5f352cba1..d2b17cb2cc 100644 --- a/aws-analytics-pinpoint/src/test/java/com/amplifyframework/analytics/pinpoint/AWSPinpointAnalyticsPluginBehaviorTest.kt +++ b/aws-analytics-pinpoint/src/test/java/com/amplifyframework/analytics/pinpoint/AWSPinpointAnalyticsPluginBehaviorTest.kt @@ -150,7 +150,8 @@ class AWSPinpointAnalyticsPluginBehaviorTest { sharedPrefs.getUniqueId(), androidAppDetails, androidDeviceDetails, - ApplicationProvider.getApplicationContext() + ApplicationProvider.getApplicationContext(), + mockk() ) } val actualEndpoint = slot() diff --git a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt index 5e1143998d..f48d48af71 100644 --- a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt +++ b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt @@ -15,8 +15,8 @@ package com.amplifyframework.pinpoint.core import android.content.Context -import android.content.SharedPreferences import aws.sdk.kotlin.services.pinpoint.PinpointClient +import aws.sdk.kotlin.services.pinpoint.model.ChannelType import aws.sdk.kotlin.services.pinpoint.model.EndpointDemographic import aws.sdk.kotlin.services.pinpoint.model.EndpointLocation import aws.sdk.kotlin.services.pinpoint.model.EndpointRequest @@ -31,41 +31,32 @@ import com.amplifyframework.analytics.UserProfile import com.amplifyframework.annotations.InternalAmplifyApi import com.amplifyframework.core.Amplify import com.amplifyframework.core.category.CategoryType +import com.amplifyframework.core.store.KeyValueRepository import com.amplifyframework.pinpoint.core.data.AndroidAppDetails import com.amplifyframework.pinpoint.core.data.AndroidDeviceDetails import com.amplifyframework.pinpoint.core.endpointProfile.EndpointProfile import com.amplifyframework.pinpoint.core.endpointProfile.EndpointProfileLocation import com.amplifyframework.pinpoint.core.endpointProfile.EndpointProfileUser import com.amplifyframework.pinpoint.core.models.AWSPinpointUserProfileBehavior -import com.amplifyframework.pinpoint.core.util.getUniqueId import com.amplifyframework.pinpoint.core.util.millisToIsoDate -import com.amplifyframework.pinpoint.core.util.putString -import java.util.concurrent.ConcurrentHashMap import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch -import org.json.JSONException -import org.json.JSONObject @InternalAmplifyApi class TargetingClient( context: Context, private val pinpointClient: PinpointClient, - private val prefs: SharedPreferences, + store: KeyValueRepository, + uniqueId: String, appDetails: AndroidAppDetails, deviceDetails: AndroidDeviceDetails, - coroutineDispatcher: CoroutineDispatcher = Dispatchers.Default + coroutineDispatcher: CoroutineDispatcher = Dispatchers.Default, ) { - private val endpointProfile = EndpointProfile(prefs.getUniqueId(), appDetails, deviceDetails, context) - private val globalAttributes: MutableMap> - private val globalMetrics: MutableMap + private val endpointProfile = EndpointProfile(uniqueId, appDetails, deviceDetails, context, store) private val coroutineScope = CoroutineScope(coroutineDispatcher) - init { - globalAttributes = loadAttributes() - globalMetrics = loadMetrics() - } /** * Allows you to tie a user to their actions and record traits about them. It includes @@ -135,17 +126,6 @@ class TargetingClient( * @return The current device endpoint profile */ fun currentEndpoint(): EndpointProfile { - // Add global attributes. - if (globalAttributes.isNotEmpty()) { - for ((key, value) in globalAttributes) { - endpointProfile.addAttribute(key, value) - } - } - if (globalMetrics.isNotEmpty()) { - for ((key, value) in globalMetrics) { - endpointProfile.addMetric(key, value) - } - } return endpointProfile } @@ -164,13 +144,6 @@ class TargetingClient( * @param endpointProfile An instance of an EndpointProfile to be updated */ fun updateEndpointProfile(endpointProfile: EndpointProfile) { - // Add global attributes. - for ((key, value) in globalAttributes) { - endpointProfile.addAttribute(key, value) - } - for ((key, value) in globalMetrics) { - endpointProfile.addMetric(key, value) - } executeUpdate(endpointProfile) } @@ -211,12 +184,12 @@ class TargetingClient( this.location = location this.demographic = demographic effectiveDate = endpointProfile.effectiveDate.millisToIsoDate() - if (endpointProfile.address != "" && endpointProfile.channelType != null) { + if (endpointProfile.address == "" && endpointProfile.channelType == ChannelType.Gcm) { + optOut = "ALL" // opt out from all notifications if we have a push channel type but no token + } else if (endpointProfile.address != "" && endpointProfile.channelType != null) { optOut = "NONE" // no opt out, send notifications address = endpointProfile.address channelType = endpointProfile.channelType - } else { - optOut = "ALL" // opt out from all notifications } attributes = endpointProfile.allAttributes @@ -242,138 +215,11 @@ class TargetingClient( } } - private fun saveAttributes() { - val jsonObject = JSONObject(globalAttributes as MutableMap<*, *>) - val jsonString = jsonObject.toString() - prefs.putString(CUSTOM_ATTRIBUTES_KEY, jsonString) - } - - private fun loadAttributes(): MutableMap> { - val outputMap: MutableMap> = ConcurrentHashMap() - val jsonString: String? = prefs.getString(CUSTOM_ATTRIBUTES_KEY, null) - if (jsonString.isNullOrBlank()) { - return outputMap - } - try { - val jsonObject = JSONObject(jsonString) - val keysItr = jsonObject.keys() - while (keysItr.hasNext()) { - val key = keysItr.next() - val jsonArray = jsonObject.getJSONArray(key) - val listValues: MutableList = ArrayList() - for (i in 0 until jsonArray.length()) { - listValues.add(jsonArray.getString(i)) - } - outputMap[key] = listValues - } - } catch (e: JSONException) { - e.printStackTrace() - } - return outputMap - } - - private fun saveMetrics() { - val jsonObject = JSONObject(globalMetrics as MutableMap<*, *>) - val jsonString = jsonObject.toString() - prefs.putString(CUSTOM_METRICS_KEY, jsonString) - } - - private fun loadMetrics(): MutableMap { - val outputMap: MutableMap = ConcurrentHashMap() - val jsonString: String? = prefs.getString(CUSTOM_METRICS_KEY, null) - if (jsonString.isNullOrBlank()) { - return outputMap - } - try { - val jsonObject = JSONObject(jsonString) - val keysItr = jsonObject.keys() - while (keysItr.hasNext()) { - val key = keysItr.next() - val value = jsonObject.getDouble(key) - outputMap[key] = value - } - } catch (e: JSONException) { - e.printStackTrace() - } - return outputMap - } - - /** - * Adds the specified attribute to the current endpoint profile generated by this client. - * Note: The maximum allowed attributes/metrics is 250. Attempts to add more may be ignored - * - * @param attributeName the name of the attribute to add - * @param attributeValues the value of the attribute - */ - fun addAttribute(attributeName: String?, attributeValues: List?) { - if (attributeName == null) { - LOG.debug("Null attribute name provided to addGlobalAttribute.") - return - } - if (attributeValues == null) { - LOG.debug("Null attribute values provided to addGlobalAttribute.") - return - } - globalAttributes[attributeName] = attributeValues - saveAttributes() - } - - /** - * Removes the specified attribute. All subsequently created events will no - * longer have this global attribute. from the current endpoint profile generated by this client. - * - * @param attributeName the name of the attribute to remove - */ - fun removeAttribute(attributeName: String?) { - if (attributeName == null) { - LOG.warn("Null attribute name provided to removeGlobalAttribute.") - return - } - endpointProfile.addAttribute(attributeName, null) - globalAttributes.remove(attributeName) - saveAttributes() - } - - /** - * Adds the specified metric to the current endpoint profile generated by this client. Note: The - * maximum allowed attributes and metrics on an endpoint update is 250. Attempts - * to add more may be ignored - * - * @param metricName the name of the metric to add - * @param metricValue the value of the metric - */ - fun addMetric(metricName: String?, metricValue: Double?) { - if (metricName == null) { - LOG.warn("Null metric name provided to addGlobalMetric.") - return - } - if (metricValue == null) { - LOG.warn("Null metric value provided to addGlobalMetric.") - return - } - globalMetrics[metricName] = metricValue - saveMetrics() - } - - /** - * Removes the specified metric from the current endpoint profile generated by this client. - * - * @param metricName the name of the metric to remove - */ - fun removeMetric(metricName: String?) { - if (metricName == null) { - LOG.warn("Null metric name provided to removeGlobalMetric.") - return - } - endpointProfile.addMetric(metricName, null) - globalMetrics.remove(metricName) - saveMetrics() - } - companion object { + @InternalAmplifyApi + const val AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY = "FCMDeviceToken" + private val LOG = Amplify.Logging.logger(CategoryType.ANALYTICS, "amplify:aws-analytics-pinpoint") - private const val CUSTOM_ATTRIBUTES_KEY = "ENDPOINT_PROFILE_CUSTOM_ATTRIBUTES" - private const val CUSTOM_METRICS_KEY = "ENDPOINT_PROFILE_CUSTOM_METRICS" private const val USER_NAME_KEY = "name" private const val USER_PLAN_KEY = "plan" diff --git a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/endpointProfile/EndpointProfile.kt b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/endpointProfile/EndpointProfile.kt index f2782dbf81..9799d5daa5 100644 --- a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/endpointProfile/EndpointProfile.kt +++ b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/endpointProfile/EndpointProfile.kt @@ -20,6 +20,8 @@ import aws.sdk.kotlin.services.pinpoint.model.ChannelType import com.amplifyframework.annotations.InternalAmplifyApi import com.amplifyframework.core.Amplify import com.amplifyframework.core.category.CategoryType +import com.amplifyframework.core.store.KeyValueRepository +import com.amplifyframework.pinpoint.core.TargetingClient import com.amplifyframework.pinpoint.core.data.AndroidAppDetails import com.amplifyframework.pinpoint.core.data.AndroidDeviceDetails import com.amplifyframework.pinpoint.core.util.millisToIsoDate @@ -39,14 +41,17 @@ class EndpointProfile( uniqueId: String, appDetails: AndroidAppDetails, deviceDetails: AndroidDeviceDetails, - applicationContext: Context + applicationContext: Context, + private val store: KeyValueRepository ) { private val attributes: MutableMap> = ConcurrentHashMap() private val metrics: MutableMap = ConcurrentHashMap() private val currentNumOfAttributesAndMetrics = AtomicInteger(0) var channelType: ChannelType? = null - var address: String = "" + val address: String get() { + return store.get(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY) ?: "" + } private val country: String = try { applicationContext.resources.configuration.locales[0].isO3Country diff --git a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/ConstructTargetingClasses.kt b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/ConstructTargetingClasses.kt index a54670933d..133a97c41d 100644 --- a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/ConstructTargetingClasses.kt +++ b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/ConstructTargetingClasses.kt @@ -20,6 +20,7 @@ import android.content.Context import android.content.SharedPreferences import androidx.test.core.app.ApplicationProvider import aws.sdk.kotlin.services.pinpoint.PinpointClient +import com.amplifyframework.core.store.KeyValueRepository import com.amplifyframework.pinpoint.core.data.AndroidAppDetails import com.amplifyframework.pinpoint.core.data.AndroidDeviceDetails import com.amplifyframework.pinpoint.core.endpointProfile.EndpointProfile @@ -48,6 +49,7 @@ internal val country = "en_US" internal val effectiveDate = 0L internal val preferences = mockk() +internal val store = mockk() internal val appDetails = AndroidAppDetails(appID, appTitle, packageName, versionCode, versionName) internal val deviceDetails = AndroidDeviceDetails(carrier = carrier, locale = locale) internal val applicationContext = mockk() @@ -55,6 +57,7 @@ internal val applicationContext = mockk() internal fun setup() { mockkStatic("com.amplifyframework.pinpoint.core.util.SharedPreferencesUtilKt") every { preferences.getUniqueId() }.returns(uniqueID) + every { store.get(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY) } returns "" every { applicationContext.resources.configuration.locales[0].isO3Country } .returns(country) } @@ -65,7 +68,8 @@ internal fun constructEndpointProfile(): EndpointProfile { preferences.getUniqueId(), appDetails, deviceDetails, - applicationContext + applicationContext, + store ) endpointProfile.effectiveDate = effectiveDate return endpointProfile @@ -83,8 +87,9 @@ internal fun constructTargetingClient(): TargetingClient { return TargetingClient( applicationContext, pinpointClient, - prefs, + store, + prefs.getUniqueId(), appDetails, - deviceDetails + deviceDetails, ) } diff --git a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt index 18cde9f160..7c3e7a6079 100644 --- a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt +++ b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt @@ -18,11 +18,13 @@ package com.amplifyframework.pinpoint.core import android.os.Build import aws.sdk.kotlin.services.pinpoint.PinpointClient +import aws.sdk.kotlin.services.pinpoint.model.ChannelType import aws.sdk.kotlin.services.pinpoint.model.EndpointRequest import aws.sdk.kotlin.services.pinpoint.model.UpdateEndpointRequest import aws.sdk.kotlin.services.pinpoint.model.UpdateEndpointResponse import io.mockk.coEvery import io.mockk.coVerify +import io.mockk.every import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.runTest import org.junit.Assert.assertEquals @@ -47,21 +49,92 @@ class TargetingClientTest { } @Test - fun testCurrentEndpoint() { - targetingClient.addAttribute("attribute", listOf("a", "b", "c")) - targetingClient.addMetric("metric", 2.0) - val endpoint = targetingClient.currentEndpoint() - assertEquals(endpoint.getAttribute("attribute"), listOf("a", "b", "c")) - assertEquals(endpoint.getMetric("metric"), 2.0) + fun testUpdateEndpointProfile() = runTest { + setup() + targetingClient = constructTargetingClient() + + val expectedToken = "token123" + every { store.get(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY) } returns expectedToken + + val updateEndpointResponse = UpdateEndpointResponse.invoke {} + coEvery { pinpointClient.updateEndpoint(ofType(UpdateEndpointRequest::class)) }.returns(updateEndpointResponse) + targetingClient.updateEndpointProfile() + + coVerify { + pinpointClient.updateEndpoint( + coWithArg { + assertNotNull(it.endpointRequest) + val request: EndpointRequest = it.endpointRequest!! + assertEquals("app id", it.applicationId) + assertEquals(expectedToken, request.address) + } + ) + } } @Test - fun testUpdateEndpointProfile() = runTest { + fun testUpdateEndpointProfileOptsIn() = runTest { setup() targetingClient = constructTargetingClient() + targetingClient.currentEndpoint().channelType = ChannelType.Gcm + + val expectedToken = "token123" + every { store.get(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY) } returns expectedToken + + + val updateEndpointResponse = UpdateEndpointResponse.invoke {} + coEvery { pinpointClient.updateEndpoint(ofType(UpdateEndpointRequest::class)) }.returns(updateEndpointResponse) + targetingClient.updateEndpointProfile() + + coVerify { + pinpointClient.updateEndpoint( + coWithArg { + assertNotNull(it.endpointRequest) + val request: EndpointRequest = it.endpointRequest!! + assertEquals("app id", it.applicationId) + assertEquals(expectedToken, request.address) + assertEquals("NONE", request.optOut) + } + ) + } + } + + @Test + fun testUpdateEndpointProfileOptsOut() = runTest { + setup() + targetingClient = constructTargetingClient() + targetingClient.currentEndpoint().channelType = ChannelType.Gcm + + val expectedToken = "" + every { store.get(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY) } returns expectedToken + + + val updateEndpointResponse = UpdateEndpointResponse.invoke {} + coEvery { pinpointClient.updateEndpoint(ofType(UpdateEndpointRequest::class)) }.returns(updateEndpointResponse) + targetingClient.updateEndpointProfile() + + coVerify { + pinpointClient.updateEndpoint( + coWithArg { + assertNotNull(it.endpointRequest) + val request: EndpointRequest = it.endpointRequest!! + assertEquals("app id", it.applicationId) + assertEquals(expectedToken, request.address) + assertEquals("ALL", request.optOut) + } + ) + } + } + + @Test + fun testUpdateEndpointProfileOptOutNotTouched() = runTest { + setup() + targetingClient = constructTargetingClient() + targetingClient.currentEndpoint().channelType = null + + val expectedToken = "" + every { store.get(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY) } returns expectedToken - targetingClient.addAttribute("attribute", listOf("a1", "a2")) - targetingClient.addMetric("metric", 1.0) val updateEndpointResponse = UpdateEndpointResponse.invoke {} coEvery { pinpointClient.updateEndpoint(ofType(UpdateEndpointRequest::class)) }.returns(updateEndpointResponse) @@ -69,12 +142,11 @@ class TargetingClientTest { coVerify { pinpointClient.updateEndpoint( - coWithArg { + coWithArg { assertNotNull(it.endpointRequest) val request: EndpointRequest = it.endpointRequest!! assertEquals("app id", it.applicationId) - assertEquals(listOf("a1", "a2"), request.attributes?.get("attribute") ?: listOf("wrong")) - assertEquals(1.0, request.metrics?.get("metric") ?: -1.0, 0.01) + assertEquals(expectedToken, request.address) } ) } diff --git a/aws-push-notifications-pinpoint/src/main/java/com/amplifyframework/pushnotifications/pinpoint/AWSPinpointPushNotificationsPlugin.kt b/aws-push-notifications-pinpoint/src/main/java/com/amplifyframework/pushnotifications/pinpoint/AWSPinpointPushNotificationsPlugin.kt index a907986499..8dd20206a6 100644 --- a/aws-push-notifications-pinpoint/src/main/java/com/amplifyframework/pushnotifications/pinpoint/AWSPinpointPushNotificationsPlugin.kt +++ b/aws-push-notifications-pinpoint/src/main/java/com/amplifyframework/pushnotifications/pinpoint/AWSPinpointPushNotificationsPlugin.kt @@ -57,7 +57,6 @@ class AWSPinpointPushNotificationsPlugin : PushNotificationsPlugin) { try { - store.put(AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY, token) + store.put(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY, token) // targetingClient needs to send the address, optOut etc. to Pinpoint so we can receive campaigns/journeys val endpointProfile = targetingClient.currentEndpoint().apply { channelType = ChannelType.Gcm - address = token } targetingClient.updateEndpointProfile(endpointProfile) From 4d63a5ee628ac813e0e50f85108d7dd22a02f067 Mon Sep 17 00:00:00 2001 From: tjroach Date: Fri, 15 Sep 2023 12:49:41 -0400 Subject: [PATCH 2/4] Continue fixes to prevent unintended user opt outs --- .../analytics/pinpoint/PinpointManager.kt | 2 +- .../pinpoint/core/EventRecorder.kt | 5 +- .../pinpoint/core/TargetingClient.kt | 168 +++++++++++++++++- .../core/ConstructTargetingClasses.kt | 2 +- .../pinpoint/core/TargetingClientTest.kt | 27 --- .../AWSPinpointPushNotificationsPlugin.kt | 2 +- 6 files changed, 168 insertions(+), 38 deletions(-) diff --git a/aws-analytics-pinpoint/src/main/java/com/amplifyframework/analytics/pinpoint/PinpointManager.kt b/aws-analytics-pinpoint/src/main/java/com/amplifyframework/analytics/pinpoint/PinpointManager.kt index f0f3112e1d..5b01db9e90 100644 --- a/aws-analytics-pinpoint/src/main/java/com/amplifyframework/analytics/pinpoint/PinpointManager.kt +++ b/aws-analytics-pinpoint/src/main/java/com/amplifyframework/analytics/pinpoint/PinpointManager.kt @@ -73,7 +73,7 @@ internal class PinpointManager constructor( context, pinpointClient, encryptedStore, - sharedPrefs.getUniqueId(), + sharedPrefs, androidAppDetails, androidDeviceDetails, ) diff --git a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/EventRecorder.kt b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/EventRecorder.kt index 99d110d984..519d223e36 100644 --- a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/EventRecorder.kt +++ b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/EventRecorder.kt @@ -18,6 +18,7 @@ import android.content.Context import android.database.Cursor import android.net.Uri import aws.sdk.kotlin.services.pinpoint.PinpointClient +import aws.sdk.kotlin.services.pinpoint.model.ChannelType import aws.sdk.kotlin.services.pinpoint.model.EndpointDemographic import aws.sdk.kotlin.services.pinpoint.model.EndpointItemResponse import aws.sdk.kotlin.services.pinpoint.model.EndpointLocation @@ -286,12 +287,10 @@ class EventRecorder( demographic = endpointDemographic effectiveDate = endpointProfile.effectiveDate.millisToIsoDate() - if (endpointProfile.address != "" && endpointProfile.channelType != null) { + if (endpointProfile.address != "" && endpointProfile.channelType == ChannelType.Gcm) { optOut = "NONE" // no opt out, send notifications address = endpointProfile.address channelType = endpointProfile.channelType - } else { - optOut = "ALL" // opt out from all notifications } attributes = endpointProfile.allAttributes diff --git a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt index f48d48af71..363ba20591 100644 --- a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt +++ b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt @@ -15,6 +15,7 @@ package com.amplifyframework.pinpoint.core import android.content.Context +import android.content.SharedPreferences import aws.sdk.kotlin.services.pinpoint.PinpointClient import aws.sdk.kotlin.services.pinpoint.model.ChannelType import aws.sdk.kotlin.services.pinpoint.model.EndpointDemographic @@ -38,25 +39,36 @@ import com.amplifyframework.pinpoint.core.endpointProfile.EndpointProfile import com.amplifyframework.pinpoint.core.endpointProfile.EndpointProfileLocation import com.amplifyframework.pinpoint.core.endpointProfile.EndpointProfileUser import com.amplifyframework.pinpoint.core.models.AWSPinpointUserProfileBehavior +import com.amplifyframework.pinpoint.core.util.getUniqueId import com.amplifyframework.pinpoint.core.util.millisToIsoDate +import com.amplifyframework.pinpoint.core.util.putString import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import org.json.JSONException +import org.json.JSONObject +import java.util.concurrent.ConcurrentHashMap @InternalAmplifyApi class TargetingClient( context: Context, private val pinpointClient: PinpointClient, store: KeyValueRepository, - uniqueId: String, + private val prefs: SharedPreferences, appDetails: AndroidAppDetails, deviceDetails: AndroidDeviceDetails, coroutineDispatcher: CoroutineDispatcher = Dispatchers.Default, ) { - private val endpointProfile = EndpointProfile(uniqueId, appDetails, deviceDetails, context, store) + private val endpointProfile = EndpointProfile(prefs.getUniqueId(), appDetails, deviceDetails, context, store) + private val globalAttributes: MutableMap> + private val globalMetrics: MutableMap private val coroutineScope = CoroutineScope(coroutineDispatcher) + init { + globalAttributes = loadAttributes() + globalMetrics = loadMetrics() + } /** * Allows you to tie a user to their actions and record traits about them. It includes @@ -126,6 +138,17 @@ class TargetingClient( * @return The current device endpoint profile */ fun currentEndpoint(): EndpointProfile { + // Add global attributes. + if (globalAttributes.isNotEmpty()) { + for ((key, value) in globalAttributes) { + endpointProfile.addAttribute(key, value) + } + } + if (globalMetrics.isNotEmpty()) { + for ((key, value) in globalMetrics) { + endpointProfile.addMetric(key, value) + } + } return endpointProfile } @@ -134,6 +157,13 @@ class TargetingClient( * TargetingClient attributes and Metrics are added to the endpoint profile. */ fun updateEndpointProfile() { + // Add global attributes. + for ((key, value) in globalAttributes) { + endpointProfile.addAttribute(key, value) + } + for ((key, value) in globalMetrics) { + endpointProfile.addMetric(key, value) + } executeUpdate(currentEndpoint()) } @@ -184,9 +214,7 @@ class TargetingClient( this.location = location this.demographic = demographic effectiveDate = endpointProfile.effectiveDate.millisToIsoDate() - if (endpointProfile.address == "" && endpointProfile.channelType == ChannelType.Gcm) { - optOut = "ALL" // opt out from all notifications if we have a push channel type but no token - } else if (endpointProfile.address != "" && endpointProfile.channelType != null) { + if (endpointProfile.address != "" && endpointProfile.channelType == ChannelType.Gcm) { optOut = "NONE" // no opt out, send notifications address = endpointProfile.address channelType = endpointProfile.channelType @@ -215,11 +243,141 @@ class TargetingClient( } } + private fun saveAttributes() { + val jsonObject = JSONObject(globalAttributes as MutableMap<*, *>) + val jsonString = jsonObject.toString() + prefs.putString(CUSTOM_ATTRIBUTES_KEY, jsonString) + } + + private fun loadAttributes(): MutableMap> { + val outputMap: MutableMap> = ConcurrentHashMap() + val jsonString: String? = prefs.getString(CUSTOM_ATTRIBUTES_KEY, null) + if (jsonString.isNullOrBlank()) { + return outputMap + } + try { + val jsonObject = JSONObject(jsonString) + val keysItr = jsonObject.keys() + while (keysItr.hasNext()) { + val key = keysItr.next() + val jsonArray = jsonObject.getJSONArray(key) + val listValues: MutableList = ArrayList() + for (i in 0 until jsonArray.length()) { + listValues.add(jsonArray.getString(i)) + } + outputMap[key] = listValues + } + } catch (e: JSONException) { + e.printStackTrace() + } + return outputMap + } + + private fun saveMetrics() { + val jsonObject = JSONObject(globalMetrics as MutableMap<*, *>) + val jsonString = jsonObject.toString() + prefs.putString(CUSTOM_METRICS_KEY, jsonString) + } + + private fun loadMetrics(): MutableMap { + val outputMap: MutableMap = ConcurrentHashMap() + val jsonString: String? = prefs.getString(CUSTOM_METRICS_KEY, null) + if (jsonString.isNullOrBlank()) { + return outputMap + } + try { + val jsonObject = JSONObject(jsonString) + val keysItr = jsonObject.keys() + while (keysItr.hasNext()) { + val key = keysItr.next() + val value = jsonObject.getDouble(key) + outputMap[key] = value + } + } catch (e: JSONException) { + e.printStackTrace() + } + return outputMap + } + + /** + * Adds the specified attribute to the current endpoint profile generated by this client. + * Note: The maximum allowed attributes/metrics is 250. Attempts to add more may be ignored + * + * @param attributeName the name of the attribute to add + * @param attributeValues the value of the attribute + */ + fun addAttribute(attributeName: String?, attributeValues: List?) { + if (attributeName == null) { + LOG.debug("Null attribute name provided to addGlobalAttribute.") + return + } + if (attributeValues == null) { + LOG.debug("Null attribute values provided to addGlobalAttribute.") + return + } + globalAttributes[attributeName] = attributeValues + saveAttributes() + } + + /** + * Removes the specified attribute. All subsequently created events will no + * longer have this global attribute. from the current endpoint profile generated by this client. + * + * @param attributeName the name of the attribute to remove + */ + fun removeAttribute(attributeName: String?) { + if (attributeName == null) { + LOG.warn("Null attribute name provided to removeGlobalAttribute.") + return + } + endpointProfile.addAttribute(attributeName, null) + globalAttributes.remove(attributeName) + saveAttributes() + } + + /** + * Adds the specified metric to the current endpoint profile generated by this client. Note: The + * maximum allowed attributes and metrics on an endpoint update is 250. Attempts + * to add more may be ignored + * + * @param metricName the name of the metric to add + * @param metricValue the value of the metric + */ + fun addMetric(metricName: String?, metricValue: Double?) { + if (metricName == null) { + LOG.warn("Null metric name provided to addGlobalMetric.") + return + } + if (metricValue == null) { + LOG.warn("Null metric value provided to addGlobalMetric.") + return + } + globalMetrics[metricName] = metricValue + saveMetrics() + } + + /** + * Removes the specified metric from the current endpoint profile generated by this client. + * + * @param metricName the name of the metric to remove + */ + fun removeMetric(metricName: String?) { + if (metricName == null) { + LOG.warn("Null metric name provided to removeGlobalMetric.") + return + } + endpointProfile.addMetric(metricName, null) + globalMetrics.remove(metricName) + saveMetrics() + } + companion object { @InternalAmplifyApi const val AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY = "FCMDeviceToken" private val LOG = Amplify.Logging.logger(CategoryType.ANALYTICS, "amplify:aws-analytics-pinpoint") + private const val CUSTOM_ATTRIBUTES_KEY = "ENDPOINT_PROFILE_CUSTOM_ATTRIBUTES" + private const val CUSTOM_METRICS_KEY = "ENDPOINT_PROFILE_CUSTOM_METRICS" private const val USER_NAME_KEY = "name" private const val USER_PLAN_KEY = "plan" diff --git a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/ConstructTargetingClasses.kt b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/ConstructTargetingClasses.kt index 133a97c41d..4f9d3ae0fb 100644 --- a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/ConstructTargetingClasses.kt +++ b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/ConstructTargetingClasses.kt @@ -88,7 +88,7 @@ internal fun constructTargetingClient(): TargetingClient { applicationContext, pinpointClient, store, - prefs.getUniqueId(), + prefs, appDetails, deviceDetails, ) diff --git a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt index 7c3e7a6079..0e7854cb58 100644 --- a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt +++ b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt @@ -99,33 +99,6 @@ class TargetingClientTest { } } - @Test - fun testUpdateEndpointProfileOptsOut() = runTest { - setup() - targetingClient = constructTargetingClient() - targetingClient.currentEndpoint().channelType = ChannelType.Gcm - - val expectedToken = "" - every { store.get(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY) } returns expectedToken - - - val updateEndpointResponse = UpdateEndpointResponse.invoke {} - coEvery { pinpointClient.updateEndpoint(ofType(UpdateEndpointRequest::class)) }.returns(updateEndpointResponse) - targetingClient.updateEndpointProfile() - - coVerify { - pinpointClient.updateEndpoint( - coWithArg { - assertNotNull(it.endpointRequest) - val request: EndpointRequest = it.endpointRequest!! - assertEquals("app id", it.applicationId) - assertEquals(expectedToken, request.address) - assertEquals("ALL", request.optOut) - } - ) - } - } - @Test fun testUpdateEndpointProfileOptOutNotTouched() = runTest { setup() diff --git a/aws-push-notifications-pinpoint/src/main/java/com/amplifyframework/pushnotifications/pinpoint/AWSPinpointPushNotificationsPlugin.kt b/aws-push-notifications-pinpoint/src/main/java/com/amplifyframework/pushnotifications/pinpoint/AWSPinpointPushNotificationsPlugin.kt index 8dd20206a6..cdd7a60d5c 100644 --- a/aws-push-notifications-pinpoint/src/main/java/com/amplifyframework/pushnotifications/pinpoint/AWSPinpointPushNotificationsPlugin.kt +++ b/aws-push-notifications-pinpoint/src/main/java/com/amplifyframework/pushnotifications/pinpoint/AWSPinpointPushNotificationsPlugin.kt @@ -139,7 +139,7 @@ class AWSPinpointPushNotificationsPlugin : PushNotificationsPlugin Date: Fri, 15 Sep 2023 12:51:34 -0400 Subject: [PATCH 3/4] Continue fixes to prevent unintended user opt outs --- .../pinpoint/core/TargetingClient.kt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt index 363ba20591..ae86d33932 100644 --- a/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt +++ b/aws-pinpoint-core/src/main/java/com/amplifyframework/pinpoint/core/TargetingClient.kt @@ -42,13 +42,13 @@ import com.amplifyframework.pinpoint.core.models.AWSPinpointUserProfileBehavior import com.amplifyframework.pinpoint.core.util.getUniqueId import com.amplifyframework.pinpoint.core.util.millisToIsoDate import com.amplifyframework.pinpoint.core.util.putString +import java.util.concurrent.ConcurrentHashMap import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import org.json.JSONException import org.json.JSONObject -import java.util.concurrent.ConcurrentHashMap @InternalAmplifyApi class TargetingClient( @@ -157,13 +157,6 @@ class TargetingClient( * TargetingClient attributes and Metrics are added to the endpoint profile. */ fun updateEndpointProfile() { - // Add global attributes. - for ((key, value) in globalAttributes) { - endpointProfile.addAttribute(key, value) - } - for ((key, value) in globalMetrics) { - endpointProfile.addMetric(key, value) - } executeUpdate(currentEndpoint()) } @@ -174,6 +167,13 @@ class TargetingClient( * @param endpointProfile An instance of an EndpointProfile to be updated */ fun updateEndpointProfile(endpointProfile: EndpointProfile) { + // Add global attributes. + for ((key, value) in globalAttributes) { + endpointProfile.addAttribute(key, value) + } + for ((key, value) in globalMetrics) { + endpointProfile.addMetric(key, value) + } executeUpdate(endpointProfile) } From 55139df04df875bc57e366ebf620eacc7a8714d2 Mon Sep 17 00:00:00 2001 From: tjroach Date: Fri, 15 Sep 2023 13:12:45 -0400 Subject: [PATCH 4/4] lint --- .../com/amplifyframework/pinpoint/core/TargetingClientTest.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt index 0e7854cb58..0c48e88a88 100644 --- a/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt +++ b/aws-pinpoint-core/src/test/java/com/amplifyframework/pinpoint/core/TargetingClientTest.kt @@ -81,7 +81,6 @@ class TargetingClientTest { val expectedToken = "token123" every { store.get(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY) } returns expectedToken - val updateEndpointResponse = UpdateEndpointResponse.invoke {} coEvery { pinpointClient.updateEndpoint(ofType(UpdateEndpointRequest::class)) }.returns(updateEndpointResponse) targetingClient.updateEndpointProfile() @@ -108,7 +107,6 @@ class TargetingClientTest { val expectedToken = "" every { store.get(TargetingClient.AWS_PINPOINT_PUSHNOTIFICATIONS_DEVICE_TOKEN_KEY) } returns expectedToken - val updateEndpointResponse = UpdateEndpointResponse.invoke {} coEvery { pinpointClient.updateEndpoint(ofType(UpdateEndpointRequest::class)) }.returns(updateEndpointResponse) targetingClient.updateEndpointProfile()