From d90b0d685b76311b1d4e68cab5de5fb766629565 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 11:32:46 +0100 Subject: [PATCH 01/18] refactor: create `deviceInfo` for every `buildEvent` Such flow makes code easier to be extracted --- .../parsely/parselyandroid/EventsBuilder.java | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java b/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java index 2f1c1ae8..2f7756b1 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java @@ -30,13 +30,12 @@ class EventsBuilder { private final SharedPreferences settings; private final String siteId; - private Map deviceInfo; + private String adKey = null; public EventsBuilder(@NonNull final Context context, @NonNull final String siteId) { this.context = context; this.siteId = siteId; settings = context.getSharedPreferences("parsely-prefs", 0); - deviceInfo = collectDeviceInfo(null); new GetAdKey(context).execute(); } @@ -74,6 +73,8 @@ Map buildEvent( if (extraData != null) { data.putAll(extraData); } + + final Map deviceInfo = collectDeviceInfo(); data.put("manufacturer", deviceInfo.get("manufacturer")); data.put("os", deviceInfo.get("os")); data.put("os_version", deviceInfo.get("os_version")); @@ -101,13 +102,11 @@ Map buildEvent( *

* Collects info about the device and user to use in Parsely events. */ - private Map collectDeviceInfo(@Nullable final String adKey) { + private Map collectDeviceInfo() { Map dInfo = new HashMap<>(); // TODO: screen dimensions (maybe?) - PLog("adkey is: %s, uuid is %s", adKey, getSiteUuid()); - final String uuid = (adKey != null) ? adKey : getSiteUuid(); - dInfo.put("parsely_site_uuid", uuid); + dInfo.put("parsely_site_uuid", getParselySiteUuid()); dInfo.put("manufacturer", android.os.Build.MANUFACTURER); dInfo.put("os", "android"); dInfo.put("os_version", String.format("%d", android.os.Build.VERSION.SDK_INT)); @@ -119,6 +118,12 @@ private Map collectDeviceInfo(@Nullable final String adKey) { return dInfo; } + private String getParselySiteUuid() { + PLog("adkey is: %s, uuid is %s", adKey, getSiteUuid()); + final String uuid = (adKey != null) ? adKey : getSiteUuid(); + return uuid; + } + /** * Get the UUID for this user. */ @@ -179,7 +184,7 @@ protected String doInBackground(Void... params) { @Override protected void onPostExecute(String advertId) { - deviceInfo = collectDeviceInfo(advertId); + adKey = advertId; } } } From 9a420f0382acf8b4c29e279287cab66e97e881a5 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 11:40:26 +0100 Subject: [PATCH 02/18] refactor: extract creating "device info" to a separate class --- .../parselyandroid/DeviceInfoRepository.java | 124 ++++++++++++++++++ .../parsely/parselyandroid/EventsBuilder.java | 113 +--------------- .../parselyandroid/ParselyTracker.java | 2 +- .../parselyandroid/EventsBuilderTest.kt | 2 +- 4 files changed, 131 insertions(+), 110 deletions(-) create mode 100644 parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.java diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.java b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.java new file mode 100644 index 00000000..e378189d --- /dev/null +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.java @@ -0,0 +1,124 @@ +package com.parsely.parselyandroid; + +import static com.parsely.parselyandroid.ParselyTracker.PLog; + +import android.content.Context; +import android.content.SharedPreferences; +import android.os.AsyncTask; +import android.provider.Settings; + +import androidx.annotation.NonNull; + +import com.google.android.gms.ads.identifier.AdvertisingIdClient; +import com.google.android.gms.common.GooglePlayServicesNotAvailableException; +import com.google.android.gms.common.GooglePlayServicesRepairableException; + +import java.io.IOException; +import java.util.HashMap; +import java.util.Map; + +class DeviceInfoRepository { + + private static final String UUID_KEY = "parsely-uuid"; + private String adKey = null; + @NonNull + private final Context context; + private final SharedPreferences settings; + + DeviceInfoRepository(@NonNull Context context) { + this.context = context; + settings = context.getSharedPreferences("parsely-prefs", 0); + new GetAdKey(context).execute(); + } + + /** + * Collect device-specific info. + *

+ * Collects info about the device and user to use in Parsely events. + */ + Map collectDeviceInfo() { + Map dInfo = new HashMap<>(); + + // TODO: screen dimensions (maybe?) + dInfo.put("parsely_site_uuid", getParselySiteUuid()); + dInfo.put("manufacturer", android.os.Build.MANUFACTURER); + dInfo.put("os", "android"); + dInfo.put("os_version", String.format("%d", android.os.Build.VERSION.SDK_INT)); + + // FIXME: Not passed in event or used anywhere else. + CharSequence txt = context.getPackageManager().getApplicationLabel(context.getApplicationInfo()); + dInfo.put("appname", txt.toString()); + + return dInfo; + } + + private String getParselySiteUuid() { + PLog("adkey is: %s, uuid is %s", adKey, getSiteUuid()); + final String uuid = (adKey != null) ? adKey : getSiteUuid(); + return uuid; + } + + /** + * Get the UUID for this user. + */ + //TODO: docs about where we get this UUID from and how. + private String getSiteUuid() { + String uuid = ""; + try { + uuid = settings.getString(UUID_KEY, ""); + if (uuid.equals("")) { + uuid = generateSiteUuid(); + } + } catch (Exception ex) { + PLog("Exception caught during site uuid generation: %s", ex.toString()); + } + return uuid; + } + + /** + * Read the Parsely UUID from application context or make a new one. + * + * @return The UUID to use for this user. + */ + private String generateSiteUuid() { + String uuid = Settings.Secure.getString(context.getApplicationContext().getContentResolver(), + Settings.Secure.ANDROID_ID); + PLog(String.format("Generated UUID: %s", uuid)); + return uuid; + } + /** + * Async task to get adKey for this device. + */ + private class GetAdKey extends AsyncTask { + private final Context mContext; + + public GetAdKey(Context context) { + mContext = context; + } + + @Override + protected String doInBackground(Void... params) { + AdvertisingIdClient.Info idInfo = null; + String advertId = null; + try { + idInfo = AdvertisingIdClient.getAdvertisingIdInfo(mContext); + } catch (GooglePlayServicesRepairableException | IOException | + GooglePlayServicesNotAvailableException | IllegalArgumentException e) { + PLog("No Google play services or error! falling back to device uuid"); + // fall back to device uuid on google play errors + advertId = getSiteUuid(); + } + try { + advertId = idInfo.getId(); + } catch (NullPointerException e) { + advertId = getSiteUuid(); + } + return advertId; + } + + @Override + protected void onPostExecute(String advertId) { + adKey = advertId; + } + } +} diff --git a/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java b/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java index 2f7756b1..af804e7d 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java @@ -3,40 +3,27 @@ import static com.parsely.parselyandroid.ParselyTracker.PLog; import android.content.Context; -import android.content.SharedPreferences; -import android.os.AsyncTask; -import android.provider.Settings; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import com.google.android.gms.ads.identifier.AdvertisingIdClient; -import com.google.android.gms.common.GooglePlayServicesNotAvailableException; -import com.google.android.gms.common.GooglePlayServicesRepairableException; - -import java.io.IOException; import java.util.Calendar; import java.util.HashMap; import java.util.Map; import java.util.TimeZone; class EventsBuilder { - private static final String UUID_KEY = "parsely-uuid"; private static final String VIDEO_START_ID_KEY = "vsid"; private static final String PAGE_VIEW_ID_KEY = "pvid"; - @NonNull - private final Context context; - private final SharedPreferences settings; private final String siteId; - private String adKey = null; + @NonNull + private final DeviceInfoRepository deviceInfoRepository; - public EventsBuilder(@NonNull final Context context, @NonNull final String siteId) { - this.context = context; + public EventsBuilder(@NonNull final DeviceInfoRepository deviceInfoRepository, @NonNull final String siteId) { this.siteId = siteId; - settings = context.getSharedPreferences("parsely-prefs", 0); - new GetAdKey(context).execute(); + this.deviceInfoRepository = deviceInfoRepository; } /** @@ -74,7 +61,7 @@ Map buildEvent( data.putAll(extraData); } - final Map deviceInfo = collectDeviceInfo(); + final Map deviceInfo = deviceInfoRepository.collectDeviceInfo(); data.put("manufacturer", deviceInfo.get("manufacturer")); data.put("os", deviceInfo.get("os")); data.put("os_version", deviceInfo.get("os_version")); @@ -97,94 +84,4 @@ Map buildEvent( return event; } - /** - * Collect device-specific info. - *

- * Collects info about the device and user to use in Parsely events. - */ - private Map collectDeviceInfo() { - Map dInfo = new HashMap<>(); - - // TODO: screen dimensions (maybe?) - dInfo.put("parsely_site_uuid", getParselySiteUuid()); - dInfo.put("manufacturer", android.os.Build.MANUFACTURER); - dInfo.put("os", "android"); - dInfo.put("os_version", String.format("%d", android.os.Build.VERSION.SDK_INT)); - - // FIXME: Not passed in event or used anywhere else. - CharSequence txt = context.getPackageManager().getApplicationLabel(context.getApplicationInfo()); - dInfo.put("appname", txt.toString()); - - return dInfo; - } - - private String getParselySiteUuid() { - PLog("adkey is: %s, uuid is %s", adKey, getSiteUuid()); - final String uuid = (adKey != null) ? adKey : getSiteUuid(); - return uuid; - } - - /** - * Get the UUID for this user. - */ - //TODO: docs about where we get this UUID from and how. - private String getSiteUuid() { - String uuid = ""; - try { - uuid = settings.getString(UUID_KEY, ""); - if (uuid.equals("")) { - uuid = generateSiteUuid(); - } - } catch (Exception ex) { - PLog("Exception caught during site uuid generation: %s", ex.toString()); - } - return uuid; - } - - /** - * Read the Parsely UUID from application context or make a new one. - * - * @return The UUID to use for this user. - */ - private String generateSiteUuid() { - String uuid = Settings.Secure.getString(context.getApplicationContext().getContentResolver(), - Settings.Secure.ANDROID_ID); - PLog(String.format("Generated UUID: %s", uuid)); - return uuid; - } - /** - * Async task to get adKey for this device. - */ - private class GetAdKey extends AsyncTask { - private final Context mContext; - - public GetAdKey(Context context) { - mContext = context; - } - - @Override - protected String doInBackground(Void... params) { - AdvertisingIdClient.Info idInfo = null; - String advertId = null; - try { - idInfo = AdvertisingIdClient.getAdvertisingIdInfo(mContext); - } catch (GooglePlayServicesRepairableException | IOException | - GooglePlayServicesNotAvailableException | IllegalArgumentException e) { - PLog("No Google play services or error! falling back to device uuid"); - // fall back to device uuid on google play errors - advertId = getSiteUuid(); - } - try { - advertId = idInfo.getId(); - } catch (NullPointerException e) { - advertId = getSiteUuid(); - } - return advertId; - } - - @Override - protected void onPostExecute(String advertId) { - adKey = advertId; - } - } } diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index 8c44f8a5..3e08ffba 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -69,7 +69,7 @@ public class ParselyTracker { */ protected ParselyTracker(String siteId, int flushInterval, Context c) { context = c.getApplicationContext(); - eventsBuilder = new EventsBuilder(context, siteId); + eventsBuilder = new EventsBuilder(new DeviceInfoRepository(context), siteId); localStorageRepository = new LocalStorageRepository(context); flushManager = new FlushManager(this, flushInterval * 1000L, ParselyCoroutineScopeKt.getSdkScope()); diff --git a/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt index 5630a8d5..1d7507e9 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt @@ -18,7 +18,7 @@ class EventsBuilderTest { fun setUp() { val applicationContext = ApplicationProvider.getApplicationContext() sut = EventsBuilder( - applicationContext, + DeviceInfoRepository(applicationContext), TEST_SITE_ID, ) } From 17c5e8241e5a95ca03c142e6c7251e99b456ddd7 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 11:42:39 +0100 Subject: [PATCH 03/18] Rename .java to .kt --- .../{DeviceInfoRepository.java => DeviceInfoRepository.kt} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename parsely/src/main/java/com/parsely/parselyandroid/{DeviceInfoRepository.java => DeviceInfoRepository.kt} (100%) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.java b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt similarity index 100% rename from parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.java rename to parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt From 65e25dd09537ef9fa77f0a459e074ae73c78a030 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 11:42:39 +0100 Subject: [PATCH 04/18] refactor: move `DeviceInfoRepository` to Kotlin --- .../parselyandroid/DeviceInfoRepository.kt | 176 +++++++++--------- 1 file changed, 87 insertions(+), 89 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt index e378189d..7aa43374 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt @@ -1,124 +1,122 @@ -package com.parsely.parselyandroid; - -import static com.parsely.parselyandroid.ParselyTracker.PLog; - -import android.content.Context; -import android.content.SharedPreferences; -import android.os.AsyncTask; -import android.provider.Settings; - -import androidx.annotation.NonNull; - -import com.google.android.gms.ads.identifier.AdvertisingIdClient; -import com.google.android.gms.common.GooglePlayServicesNotAvailableException; -import com.google.android.gms.common.GooglePlayServicesRepairableException; - -import java.io.IOException; -import java.util.HashMap; -import java.util.Map; - -class DeviceInfoRepository { - - private static final String UUID_KEY = "parsely-uuid"; - private String adKey = null; - @NonNull - private final Context context; - private final SharedPreferences settings; - - DeviceInfoRepository(@NonNull Context context) { - this.context = context; - settings = context.getSharedPreferences("parsely-prefs", 0); - new GetAdKey(context).execute(); +package com.parsely.parselyandroid + +import android.content.Context +import android.content.SharedPreferences +import android.os.AsyncTask +import android.os.Build +import android.provider.Settings +import com.google.android.gms.ads.identifier.AdvertisingIdClient +import com.google.android.gms.common.GooglePlayServicesNotAvailableException +import com.google.android.gms.common.GooglePlayServicesRepairableException +import java.io.IOException + +internal class DeviceInfoRepository(private val context: Context) { + private var adKey: String? = null + private val settings: SharedPreferences = context.getSharedPreferences("parsely-prefs", 0) + + init { + GetAdKey(context).execute() } /** * Collect device-specific info. - *

+ * + * * Collects info about the device and user to use in Parsely events. */ - Map collectDeviceInfo() { - Map dInfo = new HashMap<>(); + fun collectDeviceInfo(): Map { + val dInfo: MutableMap = HashMap() // TODO: screen dimensions (maybe?) - dInfo.put("parsely_site_uuid", getParselySiteUuid()); - dInfo.put("manufacturer", android.os.Build.MANUFACTURER); - dInfo.put("os", "android"); - dInfo.put("os_version", String.format("%d", android.os.Build.VERSION.SDK_INT)); + dInfo["parsely_site_uuid"] = parselySiteUuid + dInfo["manufacturer"] = Build.MANUFACTURER + dInfo["os"] = "android" + dInfo["os_version"] = String.format("%d", Build.VERSION.SDK_INT) // FIXME: Not passed in event or used anywhere else. - CharSequence txt = context.getPackageManager().getApplicationLabel(context.getApplicationInfo()); - dInfo.put("appname", txt.toString()); - - return dInfo; + val txt = context.packageManager.getApplicationLabel(context.applicationInfo) + dInfo["appname"] = txt.toString() + return dInfo } - private String getParselySiteUuid() { - PLog("adkey is: %s, uuid is %s", adKey, getSiteUuid()); - final String uuid = (adKey != null) ? adKey : getSiteUuid(); - return uuid; - } + private val parselySiteUuid: String + get() { + ParselyTracker.PLog("adkey is: %s, uuid is %s", adKey, siteUuid) + return if (adKey != null) adKey!! else siteUuid!! + } - /** - * Get the UUID for this user. - */ - //TODO: docs about where we get this UUID from and how. - private String getSiteUuid() { - String uuid = ""; - try { - uuid = settings.getString(UUID_KEY, ""); - if (uuid.equals("")) { - uuid = generateSiteUuid(); + private val siteUuid: String? + /** + * Get the UUID for this user. + */ + get() { + var uuid: String? = "" + try { + uuid = settings.getString(UUID_KEY, "") + if (uuid == "") { + uuid = generateSiteUuid() + } + } catch (ex: Exception) { + ParselyTracker.PLog( + "Exception caught during site uuid generation: %s", + ex.toString() + ) } - } catch (Exception ex) { - PLog("Exception caught during site uuid generation: %s", ex.toString()); + return uuid } - return uuid; - } /** * Read the Parsely UUID from application context or make a new one. * * @return The UUID to use for this user. */ - private String generateSiteUuid() { - String uuid = Settings.Secure.getString(context.getApplicationContext().getContentResolver(), - Settings.Secure.ANDROID_ID); - PLog(String.format("Generated UUID: %s", uuid)); - return uuid; + private fun generateSiteUuid(): String { + val uuid = Settings.Secure.getString( + context.applicationContext.contentResolver, + Settings.Secure.ANDROID_ID + ) + ParselyTracker.PLog(String.format("Generated UUID: %s", uuid)) + return uuid } + /** * Async task to get adKey for this device. */ - private class GetAdKey extends AsyncTask { - private final Context mContext; - - public GetAdKey(Context context) { - mContext = context; - } - - @Override - protected String doInBackground(Void... params) { - AdvertisingIdClient.Info idInfo = null; - String advertId = null; + private inner class GetAdKey(private val mContext: Context) : + AsyncTask() { + protected override fun doInBackground(vararg params: Void?): String? { + var idInfo: AdvertisingIdClient.Info? = null + var advertId: String? = null try { - idInfo = AdvertisingIdClient.getAdvertisingIdInfo(mContext); - } catch (GooglePlayServicesRepairableException | IOException | - GooglePlayServicesNotAvailableException | IllegalArgumentException e) { - PLog("No Google play services or error! falling back to device uuid"); + idInfo = AdvertisingIdClient.getAdvertisingIdInfo(mContext) + } catch (e: GooglePlayServicesRepairableException) { + ParselyTracker.PLog("No Google play services or error! falling back to device uuid") // fall back to device uuid on google play errors - advertId = getSiteUuid(); + advertId = siteUuid + } catch (e: IOException) { + ParselyTracker.PLog("No Google play services or error! falling back to device uuid") + advertId = siteUuid + } catch (e: GooglePlayServicesNotAvailableException) { + ParselyTracker.PLog("No Google play services or error! falling back to device uuid") + advertId = siteUuid + } catch (e: IllegalArgumentException) { + ParselyTracker.PLog("No Google play services or error! falling back to device uuid") + advertId = siteUuid } try { - advertId = idInfo.getId(); - } catch (NullPointerException e) { - advertId = getSiteUuid(); + advertId = idInfo!!.id + } catch (e: NullPointerException) { + advertId = siteUuid } - return advertId; + return advertId } - @Override - protected void onPostExecute(String advertId) { - adKey = advertId; + override fun onPostExecute(advertId: String?) { + adKey = advertId } } + + companion object { + private const val UUID_KEY = "parsely-uuid" + } } From ce1d56ca11fcb5013a816dde8b5f9de7e91bf0d9 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 12:01:56 +0100 Subject: [PATCH 05/18] refactor: make `EventsBuilderTest` not test `DeviceInfoRepository` Introducing an interface, allowed to create `FakeDeviceInfoRepository`. --- .../parselyandroid/DeviceInfoRepository.kt | 9 ++++++-- .../parsely/parselyandroid/EventsBuilder.java | 6 ++--- .../parselyandroid/ParselyTracker.java | 2 +- .../parselyandroid/EventsBuilderTest.kt | 23 ++++++++++--------- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt index 7aa43374..c5510c71 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt @@ -9,8 +9,13 @@ import com.google.android.gms.ads.identifier.AdvertisingIdClient import com.google.android.gms.common.GooglePlayServicesNotAvailableException import com.google.android.gms.common.GooglePlayServicesRepairableException import java.io.IOException +import kotlinx.coroutines.launch -internal class DeviceInfoRepository(private val context: Context) { +internal interface DeviceInfoRepository{ + fun collectDeviceInfo(): Map +} + +internal open class AndroidDeviceInfoRepository(private val context: Context): DeviceInfoRepository { private var adKey: String? = null private val settings: SharedPreferences = context.getSharedPreferences("parsely-prefs", 0) @@ -24,7 +29,7 @@ internal class DeviceInfoRepository(private val context: Context) { * * Collects info about the device and user to use in Parsely events. */ - fun collectDeviceInfo(): Map { + override fun collectDeviceInfo(): Map { val dInfo: MutableMap = HashMap() // TODO: screen dimensions (maybe?) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java b/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java index af804e7d..b70c1aec 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/EventsBuilder.java @@ -62,11 +62,9 @@ Map buildEvent( } final Map deviceInfo = deviceInfoRepository.collectDeviceInfo(); - data.put("manufacturer", deviceInfo.get("manufacturer")); - data.put("os", deviceInfo.get("os")); - data.put("os_version", deviceInfo.get("os_version")); data.put("ts", now.getTimeInMillis()); - data.put("parsely_site_uuid", deviceInfo.get("parsely_site_uuid")); + data.putAll(deviceInfo); + event.put("data", data); if (metadata != null) { diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index 3e08ffba..cb4f399a 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -69,7 +69,7 @@ public class ParselyTracker { */ protected ParselyTracker(String siteId, int flushInterval, Context c) { context = c.getApplicationContext(); - eventsBuilder = new EventsBuilder(new DeviceInfoRepository(context), siteId); + eventsBuilder = new EventsBuilder(new AndroidDeviceInfoRepository(context), siteId); localStorageRepository = new LocalStorageRepository(context); flushManager = new FlushManager(this, flushInterval * 1000L, ParselyCoroutineScopeKt.getSdkScope()); diff --git a/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt index 1d7507e9..f02a8ad3 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt @@ -1,7 +1,6 @@ package com.parsely.parselyandroid import android.content.Context -import android.provider.Settings import androidx.test.core.app.ApplicationProvider import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.MapAssert @@ -11,14 +10,13 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) -class EventsBuilderTest { +internal class EventsBuilderTest { private lateinit var sut: EventsBuilder @Before fun setUp() { - val applicationContext = ApplicationProvider.getApplicationContext() sut = EventsBuilder( - DeviceInfoRepository(applicationContext), + FakeDeviceInfoRepository(), TEST_SITE_ID, ) } @@ -116,7 +114,7 @@ class EventsBuilderTest { // then @Suppress("UNCHECKED_CAST") - assertThat(event["data"] as Map).hasSize(5) + assertThat(event["data"] as Map).hasSize(2) } @Test @@ -139,7 +137,7 @@ class EventsBuilderTest { // then @Suppress("UNCHECKED_CAST") - assertThat(event["data"] as Map).hasSize(7) + assertThat(event["data"] as Map).hasSize(4) .containsAllEntriesOf(extraData) } @@ -192,19 +190,22 @@ class EventsBuilderTest { @Suppress("UNCHECKED_CAST") it as Map assertThat(it) - .hasSize(5) - .containsEntry("os", "android") + .hasSize(2) + .containsAllEntriesOf(FAKE_DEVICE_INFO) .hasEntrySatisfying("ts") { timestamp -> assertThat(timestamp as Long).isBetween(1111111111111, 9999999999999) } - .containsEntry("manufacturer", "robolectric") - .containsEntry("os_version", "33") - .containsEntry("parsely_site_uuid", null) } companion object { const val TEST_SITE_ID = "Example" const val TEST_URL = "http://example.com/some-old/article.html" const val TEST_UUID = "123e4567-e89b-12d3-a456-426614174000" + + val FAKE_DEVICE_INFO = mapOf("device" to "info") + } + + class FakeDeviceInfoRepository: DeviceInfoRepository { + override fun collectDeviceInfo(): Map = FAKE_DEVICE_INFO } } From 96acab087ac42285bb953671974147e1d3d42b69 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 12:06:33 +0100 Subject: [PATCH 06/18] refactor: move `GetAdKey` to Coroutines --- .../parselyandroid/DeviceInfoRepository.kt | 22 ++++++++----------- .../parselyandroid/ParselyTracker.java | 2 +- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt index c5510c71..66d63b21 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt @@ -2,25 +2,28 @@ package com.parsely.parselyandroid import android.content.Context import android.content.SharedPreferences -import android.os.AsyncTask import android.os.Build import android.provider.Settings import com.google.android.gms.ads.identifier.AdvertisingIdClient import com.google.android.gms.common.GooglePlayServicesNotAvailableException import com.google.android.gms.common.GooglePlayServicesRepairableException import java.io.IOException +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch internal interface DeviceInfoRepository{ fun collectDeviceInfo(): Map } -internal open class AndroidDeviceInfoRepository(private val context: Context): DeviceInfoRepository { +internal open class AndroidDeviceInfoRepository( + private val context: Context, + private val coroutineScope: CoroutineScope +): DeviceInfoRepository { private var adKey: String? = null private val settings: SharedPreferences = context.getSharedPreferences("parsely-prefs", 0) init { - GetAdKey(context).execute() + retrieveAdKey() } /** @@ -84,16 +87,12 @@ internal open class AndroidDeviceInfoRepository(private val context: Context): D return uuid } - /** - * Async task to get adKey for this device. - */ - private inner class GetAdKey(private val mContext: Context) : - AsyncTask() { - protected override fun doInBackground(vararg params: Void?): String? { + private fun retrieveAdKey() { + coroutineScope.launch { var idInfo: AdvertisingIdClient.Info? = null var advertId: String? = null try { - idInfo = AdvertisingIdClient.getAdvertisingIdInfo(mContext) + idInfo = AdvertisingIdClient.getAdvertisingIdInfo(context) } catch (e: GooglePlayServicesRepairableException) { ParselyTracker.PLog("No Google play services or error! falling back to device uuid") // fall back to device uuid on google play errors @@ -113,10 +112,7 @@ internal open class AndroidDeviceInfoRepository(private val context: Context): D } catch (e: NullPointerException) { advertId = siteUuid } - return advertId - } - override fun onPostExecute(advertId: String?) { adKey = advertId } } diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index cb4f399a..6d59fb60 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -69,7 +69,7 @@ public class ParselyTracker { */ protected ParselyTracker(String siteId, int flushInterval, Context c) { context = c.getApplicationContext(); - eventsBuilder = new EventsBuilder(new AndroidDeviceInfoRepository(context), siteId); + eventsBuilder = new EventsBuilder(new AndroidDeviceInfoRepository(context, ParselyCoroutineScopeKt.getSdkScope()), siteId); localStorageRepository = new LocalStorageRepository(context); flushManager = new FlushManager(this, flushInterval * 1000L, ParselyCoroutineScopeKt.getSdkScope()); From 2c0cc68417747f9964f58f5ca2b49119cb316538 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 12:08:44 +0100 Subject: [PATCH 07/18] refactor: simplify AdKey retrieval There's no need to differentiate between different type of exceptions. --- .../parselyandroid/DeviceInfoRepository.kt | 28 ++++--------------- 1 file changed, 5 insertions(+), 23 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt index 66d63b21..cb7cd50e 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt @@ -89,31 +89,13 @@ internal open class AndroidDeviceInfoRepository( private fun retrieveAdKey() { coroutineScope.launch { - var idInfo: AdvertisingIdClient.Info? = null - var advertId: String? = null - try { - idInfo = AdvertisingIdClient.getAdvertisingIdInfo(context) - } catch (e: GooglePlayServicesRepairableException) { - ParselyTracker.PLog("No Google play services or error! falling back to device uuid") - // fall back to device uuid on google play errors - advertId = siteUuid - } catch (e: IOException) { - ParselyTracker.PLog("No Google play services or error! falling back to device uuid") - advertId = siteUuid - } catch (e: GooglePlayServicesNotAvailableException) { - ParselyTracker.PLog("No Google play services or error! falling back to device uuid") - advertId = siteUuid - } catch (e: IllegalArgumentException) { + adKey = try { + val idInfo = AdvertisingIdClient.getAdvertisingIdInfo(context) + idInfo.id + } catch (e: Exception) { ParselyTracker.PLog("No Google play services or error! falling back to device uuid") - advertId = siteUuid + siteUuid } - try { - advertId = idInfo!!.id - } catch (e: NullPointerException) { - advertId = siteUuid - } - - adKey = advertId } } From 25d06f11fbb148f7c7159eb205bcad5fe9ce76fe Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 12:11:15 +0100 Subject: [PATCH 08/18] fix: remove `appname` from device info It was not used. Also, it's not expected that the mobile sdk will send this field. --- .../java/com/parsely/parselyandroid/DeviceInfoRepository.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt index cb7cd50e..493237c9 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt @@ -41,9 +41,6 @@ internal open class AndroidDeviceInfoRepository( dInfo["os"] = "android" dInfo["os_version"] = String.format("%d", Build.VERSION.SDK_INT) - // FIXME: Not passed in event or used anywhere else. - val txt = context.packageManager.getApplicationLabel(context.applicationInfo) - dInfo["appname"] = txt.toString() return dInfo } From c5962bced8009ceeea793bc365ff98f8b6a1aeda Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 12:11:44 +0100 Subject: [PATCH 09/18] style: remove unused imports --- .../java/com/parsely/parselyandroid/DeviceInfoRepository.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt index 493237c9..9fd082e7 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt @@ -5,9 +5,6 @@ import android.content.SharedPreferences import android.os.Build import android.provider.Settings import com.google.android.gms.ads.identifier.AdvertisingIdClient -import com.google.android.gms.common.GooglePlayServicesNotAvailableException -import com.google.android.gms.common.GooglePlayServicesRepairableException -import java.io.IOException import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch From 4c9149413b37de32fcc1f12d537ced5ba78f0ab2 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 13:41:53 +0100 Subject: [PATCH 10/18] tests: make `EventsBuilderTest` not Robolectric test As `DeviceInfoRepository` is now faked, the test doesn't have to use Robolectric anymore. --- .../test/java/com/parsely/parselyandroid/EventsBuilderTest.kt | 3 --- 1 file changed, 3 deletions(-) diff --git a/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt index f02a8ad3..e76210c2 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/EventsBuilderTest.kt @@ -1,7 +1,5 @@ package com.parsely.parselyandroid -import android.content.Context -import androidx.test.core.app.ApplicationProvider import org.assertj.core.api.Assertions.assertThat import org.assertj.core.api.MapAssert import org.junit.Before @@ -9,7 +7,6 @@ import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner -@RunWith(RobolectricTestRunner::class) internal class EventsBuilderTest { private lateinit var sut: EventsBuilder From 9cd6ccec866c4d9074588db5f50283c4ba79a580 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 15:27:59 +0100 Subject: [PATCH 11/18] refactor: extract getting ad key to `AdvertisementIdProvider` It encapsulates all logic related to retrieving an ad key --- .../parselyandroid/AdvertisementIdProvider.kt | 31 ++++++++++++++++++ .../parselyandroid/DeviceInfoRepository.kt | 32 ++++++------------- .../parselyandroid/ParselyTracker.java | 2 +- 3 files changed, 42 insertions(+), 23 deletions(-) create mode 100644 parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt diff --git a/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt new file mode 100644 index 00000000..8cb92da3 --- /dev/null +++ b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt @@ -0,0 +1,31 @@ +package com.parsely.parselyandroid + +import android.content.Context +import com.google.android.gms.ads.identifier.AdvertisingIdClient +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.launch + +internal class AdvertisementIdProvider( + private val context: Context, + coroutineScope: CoroutineScope +) : IdProvider { + + private var adKey: String? = null + + init { + coroutineScope.launch { + try { + val idInfo = AdvertisingIdClient.getAdvertisingIdInfo(context) + idInfo.id + } catch (e: Exception) { + ParselyTracker.PLog("No Google play services or error!") + } + } + } + + override fun provide(): String? = adKey +} + +internal fun interface IdProvider { + fun provide(): String? +} diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt index 9fd082e7..7f56eb54 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt @@ -4,9 +4,6 @@ import android.content.Context import android.content.SharedPreferences import android.os.Build import android.provider.Settings -import com.google.android.gms.ads.identifier.AdvertisingIdClient -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.launch internal interface DeviceInfoRepository{ fun collectDeviceInfo(): Map @@ -14,15 +11,10 @@ internal interface DeviceInfoRepository{ internal open class AndroidDeviceInfoRepository( private val context: Context, - private val coroutineScope: CoroutineScope + private val advertisementIdProvider: IdProvider ): DeviceInfoRepository { - private var adKey: String? = null private val settings: SharedPreferences = context.getSharedPreferences("parsely-prefs", 0) - init { - retrieveAdKey() - } - /** * Collect device-specific info. * @@ -43,8 +35,16 @@ internal open class AndroidDeviceInfoRepository( private val parselySiteUuid: String get() { + val adKey = advertisementIdProvider.provide() + ParselyTracker.PLog("adkey is: %s, uuid is %s", adKey, siteUuid) - return if (adKey != null) adKey!! else siteUuid!! + + return if (adKey != null) { + adKey + } else { + ParselyTracker.PLog("falling back to device uuid") + siteUuid .orEmpty() + } } private val siteUuid: String? @@ -81,18 +81,6 @@ internal open class AndroidDeviceInfoRepository( return uuid } - private fun retrieveAdKey() { - coroutineScope.launch { - adKey = try { - val idInfo = AdvertisingIdClient.getAdvertisingIdInfo(context) - idInfo.id - } catch (e: Exception) { - ParselyTracker.PLog("No Google play services or error! falling back to device uuid") - siteUuid - } - } - } - companion object { private const val UUID_KEY = "parsely-uuid" } diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index 6d59fb60..c58a93db 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -69,7 +69,7 @@ public class ParselyTracker { */ protected ParselyTracker(String siteId, int flushInterval, Context c) { context = c.getApplicationContext(); - eventsBuilder = new EventsBuilder(new AndroidDeviceInfoRepository(context, ParselyCoroutineScopeKt.getSdkScope()), siteId); + eventsBuilder = new EventsBuilder(new AndroidDeviceInfoRepository(context, new AdvertisementIdProvider(context, ParselyCoroutineScopeKt.getSdkScope())), siteId); localStorageRepository = new LocalStorageRepository(context); flushManager = new FlushManager(this, flushInterval * 1000L, ParselyCoroutineScopeKt.getSdkScope()); From 6ff145acba14bf1da74863c1ae26e441e6437c0c Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 16:09:39 +0100 Subject: [PATCH 12/18] refactor: extract getting uuid to `UuidProvider` It encapsulates all logic related to retrieving uuid --- .../parselyandroid/AdvertisementIdProvider.kt | 49 +++++++++++++++++++ .../parselyandroid/DeviceInfoRepository.kt | 47 ++---------------- .../parselyandroid/ParselyTracker.java | 6 ++- 3 files changed, 57 insertions(+), 45 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt index 8cb92da3..e0b75c07 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt @@ -1,6 +1,8 @@ package com.parsely.parselyandroid import android.content.Context +import android.content.SharedPreferences +import android.provider.Settings import com.google.android.gms.ads.identifier.AdvertisingIdClient import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch @@ -26,6 +28,53 @@ internal class AdvertisementIdProvider( override fun provide(): String? = adKey } +internal class UuidProvider(private val context: Context) : IdProvider { + private val settings: SharedPreferences = context.getSharedPreferences("parsely-prefs", 0) + + private val siteUuid: String? + /** + * Get the UUID for this user. + */ + get() { + var uuid: String? = "" + try { + uuid = settings.getString(UUID_KEY, "") + if (uuid == "") { + uuid = generateSiteUuid() + } + } catch (ex: Exception) { + ParselyTracker.PLog( + "Exception caught during site uuid generation: %s", + ex.toString() + ) + } + return uuid + } + + /** + * Read the Parsely UUID from application context or make a new one. + * + * @return The UUID to use for this user. + */ + private fun generateSiteUuid(): String { + val uuid = Settings.Secure.getString( + context.applicationContext.contentResolver, + Settings.Secure.ANDROID_ID + ) + ParselyTracker.PLog(String.format("Generated UUID: %s", uuid)) + return uuid + } + + override fun provide(): String? { + return siteUuid + } + + companion object { + private const val UUID_KEY = "parsely-uuid" + } + +} + internal fun interface IdProvider { fun provide(): String? } diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt index 7f56eb54..844db97f 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt @@ -1,19 +1,15 @@ package com.parsely.parselyandroid -import android.content.Context -import android.content.SharedPreferences import android.os.Build -import android.provider.Settings internal interface DeviceInfoRepository{ fun collectDeviceInfo(): Map } internal open class AndroidDeviceInfoRepository( - private val context: Context, - private val advertisementIdProvider: IdProvider + private val advertisementIdProvider: IdProvider, + private val uuidProvider: IdProvider, ): DeviceInfoRepository { - private val settings: SharedPreferences = context.getSharedPreferences("parsely-prefs", 0) /** * Collect device-specific info. @@ -36,6 +32,7 @@ internal open class AndroidDeviceInfoRepository( private val parselySiteUuid: String get() { val adKey = advertisementIdProvider.provide() + val siteUuid = uuidProvider.provide() ParselyTracker.PLog("adkey is: %s, uuid is %s", adKey, siteUuid) @@ -46,42 +43,4 @@ internal open class AndroidDeviceInfoRepository( siteUuid .orEmpty() } } - - private val siteUuid: String? - /** - * Get the UUID for this user. - */ - get() { - var uuid: String? = "" - try { - uuid = settings.getString(UUID_KEY, "") - if (uuid == "") { - uuid = generateSiteUuid() - } - } catch (ex: Exception) { - ParselyTracker.PLog( - "Exception caught during site uuid generation: %s", - ex.toString() - ) - } - return uuid - } - - /** - * Read the Parsely UUID from application context or make a new one. - * - * @return The UUID to use for this user. - */ - private fun generateSiteUuid(): String { - val uuid = Settings.Secure.getString( - context.applicationContext.contentResolver, - Settings.Secure.ANDROID_ID - ) - ParselyTracker.PLog(String.format("Generated UUID: %s", uuid)) - return uuid - } - - companion object { - private const val UUID_KEY = "parsely-uuid" - } } diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index c58a93db..6104d7f8 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -69,7 +69,11 @@ public class ParselyTracker { */ protected ParselyTracker(String siteId, int flushInterval, Context c) { context = c.getApplicationContext(); - eventsBuilder = new EventsBuilder(new AndroidDeviceInfoRepository(context, new AdvertisementIdProvider(context, ParselyCoroutineScopeKt.getSdkScope())), siteId); + eventsBuilder = new EventsBuilder( + new AndroidDeviceInfoRepository( + new AdvertisementIdProvider(context, ParselyCoroutineScopeKt.getSdkScope()), + new UuidProvider(context) + ), siteId); localStorageRepository = new LocalStorageRepository(context); flushManager = new FlushManager(this, flushInterval * 1000L, ParselyCoroutineScopeKt.getSdkScope()); From 59a83d8c5b216095ea2450a3a4c3a996bddb36a2 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 16:21:38 +0100 Subject: [PATCH 13/18] tests: add unit tests for `UuidProvider` --- .../parselyandroid/UuidProviderTest.kt | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 parsely/src/test/java/com/parsely/parselyandroid/UuidProviderTest.kt diff --git a/parsely/src/test/java/com/parsely/parselyandroid/UuidProviderTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/UuidProviderTest.kt new file mode 100644 index 00000000..6804b00e --- /dev/null +++ b/parsely/src/test/java/com/parsely/parselyandroid/UuidProviderTest.kt @@ -0,0 +1,56 @@ +package com.parsely.parselyandroid + +import android.app.Application +import android.provider.Settings +import androidx.test.core.app.ApplicationProvider +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +internal class UuidProviderTest { + + lateinit var sut: UuidProvider + + @Before + fun setUp() { + sut = UuidProvider(ApplicationProvider.getApplicationContext()) + } + + @Test + fun `given no site uuid is stored, when requesting uuid, then return ANDROID_ID value`() { + // given + val fakeAndroidId = "test id" + Settings.Secure.putString( + ApplicationProvider.getApplicationContext().contentResolver, + Settings.Secure.ANDROID_ID, + fakeAndroidId + ) + + // when + val result= sut.provide() + + // then + assertThat(result).isEqualTo(fakeAndroidId) + } + + @Test + fun `given site uuid already requested, when requesting uuid, then return same uuid`() { + // given + val fakeAndroidId = "test id" + Settings.Secure.putString( + ApplicationProvider.getApplicationContext().contentResolver, + Settings.Secure.ANDROID_ID, + fakeAndroidId + ) + val storedValue = sut.provide() + + // when + val result = sut.provide() + + // then + assertThat(result).isEqualTo(storedValue) + } +} From 3bcbb381781fc9d1d6e7c836da1de8e5f28da9ad Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 16:34:27 +0100 Subject: [PATCH 14/18] refactor: do not query `ANDROID_ID` from `SharedPreferences`. Actually, it was never working as we never were saving any value to those `SharedPreferences`. This change seems fine because we rely in vast majority of cases on AdvertisingId - only in case of lack of Google Play Services we fallback to `ANDROID_ID` which is the same until factory reset. Closes: #59 --- .../parselyandroid/AdvertisementIdProvider.kt | 53 ++++--------------- .../parselyandroid/DeviceInfoRepository.kt | 8 +-- .../parselyandroid/ParselyTracker.java | 2 +- ...oviderTest.kt => AndroidIdProviderTest.kt} | 6 +-- 4 files changed, 18 insertions(+), 51 deletions(-) rename parsely/src/test/java/com/parsely/parselyandroid/{UuidProviderTest.kt => AndroidIdProviderTest.kt} (90%) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt index e0b75c07..f698b55b 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt @@ -1,7 +1,6 @@ package com.parsely.parselyandroid import android.content.Context -import android.content.SharedPreferences import android.provider.Settings import com.google.android.gms.ads.identifier.AdvertisingIdClient import kotlinx.coroutines.CoroutineScope @@ -28,51 +27,19 @@ internal class AdvertisementIdProvider( override fun provide(): String? = adKey } -internal class UuidProvider(private val context: Context) : IdProvider { - private val settings: SharedPreferences = context.getSharedPreferences("parsely-prefs", 0) - - private val siteUuid: String? - /** - * Get the UUID for this user. - */ - get() { - var uuid: String? = "" - try { - uuid = settings.getString(UUID_KEY, "") - if (uuid == "") { - uuid = generateSiteUuid() - } - } catch (ex: Exception) { - ParselyTracker.PLog( - "Exception caught during site uuid generation: %s", - ex.toString() - ) - } - return uuid +internal class AndroidIdProvider(private val context: Context) : IdProvider { + override fun provide(): String? { + val uuid = try { + Settings.Secure.getString( + context.applicationContext.contentResolver, + Settings.Secure.ANDROID_ID + ) + } catch (ex: Exception) { + null } - - /** - * Read the Parsely UUID from application context or make a new one. - * - * @return The UUID to use for this user. - */ - private fun generateSiteUuid(): String { - val uuid = Settings.Secure.getString( - context.applicationContext.contentResolver, - Settings.Secure.ANDROID_ID - ) - ParselyTracker.PLog(String.format("Generated UUID: %s", uuid)) + ParselyTracker.PLog(String.format("Android ID: %s", uuid)) return uuid } - - override fun provide(): String? { - return siteUuid - } - - companion object { - private const val UUID_KEY = "parsely-uuid" - } - } internal fun interface IdProvider { diff --git a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt index 844db97f..fc899215 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/DeviceInfoRepository.kt @@ -8,7 +8,7 @@ internal interface DeviceInfoRepository{ internal open class AndroidDeviceInfoRepository( private val advertisementIdProvider: IdProvider, - private val uuidProvider: IdProvider, + private val androidIdProvider: IdProvider, ): DeviceInfoRepository { /** @@ -32,15 +32,15 @@ internal open class AndroidDeviceInfoRepository( private val parselySiteUuid: String get() { val adKey = advertisementIdProvider.provide() - val siteUuid = uuidProvider.provide() + val androidId = androidIdProvider.provide() - ParselyTracker.PLog("adkey is: %s, uuid is %s", adKey, siteUuid) + ParselyTracker.PLog("adkey is: %s, uuid is %s", adKey, androidId) return if (adKey != null) { adKey } else { ParselyTracker.PLog("falling back to device uuid") - siteUuid .orEmpty() + androidId .orEmpty() } } } diff --git a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java index 6104d7f8..60b4bf03 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java +++ b/parsely/src/main/java/com/parsely/parselyandroid/ParselyTracker.java @@ -72,7 +72,7 @@ protected ParselyTracker(String siteId, int flushInterval, Context c) { eventsBuilder = new EventsBuilder( new AndroidDeviceInfoRepository( new AdvertisementIdProvider(context, ParselyCoroutineScopeKt.getSdkScope()), - new UuidProvider(context) + new AndroidIdProvider(context) ), siteId); localStorageRepository = new LocalStorageRepository(context); flushManager = new FlushManager(this, flushInterval * 1000L, diff --git a/parsely/src/test/java/com/parsely/parselyandroid/UuidProviderTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/AndroidIdProviderTest.kt similarity index 90% rename from parsely/src/test/java/com/parsely/parselyandroid/UuidProviderTest.kt rename to parsely/src/test/java/com/parsely/parselyandroid/AndroidIdProviderTest.kt index 6804b00e..eaf24427 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/UuidProviderTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/AndroidIdProviderTest.kt @@ -10,13 +10,13 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) -internal class UuidProviderTest { +internal class AndroidIdProviderTest { - lateinit var sut: UuidProvider + lateinit var sut: AndroidIdProvider @Before fun setUp() { - sut = UuidProvider(ApplicationProvider.getApplicationContext()) + sut = AndroidIdProvider(ApplicationProvider.getApplicationContext()) } @Test From c6f68bf0a00fdbe38525718490c19127e8878a08 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 14 Nov 2023 16:43:14 +0100 Subject: [PATCH 15/18] tests: add unit tests for `AndroidDeviceInfoRepository` --- .../AndroidDeviceInfoRepositoryTest.kt | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 parsely/src/test/java/com/parsely/parselyandroid/AndroidDeviceInfoRepositoryTest.kt diff --git a/parsely/src/test/java/com/parsely/parselyandroid/AndroidDeviceInfoRepositoryTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/AndroidDeviceInfoRepositoryTest.kt new file mode 100644 index 00000000..0f41f7b6 --- /dev/null +++ b/parsely/src/test/java/com/parsely/parselyandroid/AndroidDeviceInfoRepositoryTest.kt @@ -0,0 +1,78 @@ +package com.parsely.parselyandroid + +import org.assertj.core.api.Assertions.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import org.robolectric.shadows.ShadowBuild + +private const val SDK_VERSION = 33 +private const val MANUFACTURER = "test manufacturer" + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [SDK_VERSION]) +internal class AndroidDeviceInfoRepositoryTest { + + private lateinit var sut: AndroidDeviceInfoRepository + + @Before + fun setUp() { + ShadowBuild.setManufacturer(MANUFACTURER) + } + + @Test + fun `given the advertisement id exists, when collecting device info, then parsely site uuid is advertisement id`() { + // given + val advertisementId = "ad id" + sut = AndroidDeviceInfoRepository( + advertisementIdProvider = { advertisementId }, + androidIdProvider = { "android id" }) + + // when + val result = sut.collectDeviceInfo() + + // then + assertThat(result).isEqualTo(expectedConstantDeviceInfo + ("parsely_site_uuid" to advertisementId)) + } + + @Test + fun `given the advertisement is null and android id is not, when collecting device info, then parsely id is android id`() { + // given + val androidId = "android id" + sut = AndroidDeviceInfoRepository( + advertisementIdProvider = { null }, + androidIdProvider = { androidId } + ) + + // when + val result = sut.collectDeviceInfo() + + // then + assertThat(result).isEqualTo(expectedConstantDeviceInfo + ("parsely_site_uuid" to androidId)) + } + + @Test + fun `given both advertisement id and android id are null, when collecting device info, then parsely id is empty`() { + // given + sut = AndroidDeviceInfoRepository( + advertisementIdProvider = { null }, + androidIdProvider = { null } + ) + + // when + val result = sut.collectDeviceInfo() + + // then + assertThat(result).isEqualTo(expectedConstantDeviceInfo + ("parsely_site_uuid" to "")) + } + + private companion object { + val expectedConstantDeviceInfo = mapOf( + "manufacturer" to MANUFACTURER, + "os" to "android", + "os_version" to "$SDK_VERSION" + ) + } +} From 7533f97e0e7575b354ce0ef0a4ba403c6148f7ea Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 5 Dec 2023 17:40:53 +0100 Subject: [PATCH 16/18] fix: assign advertisement id in AdvertisementIdProvider --- .../java/com/parsely/parselyandroid/AdvertisementIdProvider.kt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt index f698b55b..ba76ca1c 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt @@ -16,8 +16,7 @@ internal class AdvertisementIdProvider( init { coroutineScope.launch { try { - val idInfo = AdvertisingIdClient.getAdvertisingIdInfo(context) - idInfo.id + adKey = AdvertisingIdClient.getAdvertisingIdInfo(context).id } catch (e: Exception) { ParselyTracker.PLog("No Google play services or error!") } From 473dc7e6f65f798f22ae70a6ec54906ec8d2eec1 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Tue, 5 Dec 2023 17:47:10 +0100 Subject: [PATCH 17/18] tests: make SUT test-scoped in AndroidDeviceInfoRepositoryTest --- .../parselyandroid/AndroidDeviceInfoRepositoryTest.kt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/parsely/src/test/java/com/parsely/parselyandroid/AndroidDeviceInfoRepositoryTest.kt b/parsely/src/test/java/com/parsely/parselyandroid/AndroidDeviceInfoRepositoryTest.kt index 0f41f7b6..08ef47b8 100644 --- a/parsely/src/test/java/com/parsely/parselyandroid/AndroidDeviceInfoRepositoryTest.kt +++ b/parsely/src/test/java/com/parsely/parselyandroid/AndroidDeviceInfoRepositoryTest.kt @@ -15,8 +15,6 @@ private const val MANUFACTURER = "test manufacturer" @Config(sdk = [SDK_VERSION]) internal class AndroidDeviceInfoRepositoryTest { - private lateinit var sut: AndroidDeviceInfoRepository - @Before fun setUp() { ShadowBuild.setManufacturer(MANUFACTURER) @@ -26,7 +24,7 @@ internal class AndroidDeviceInfoRepositoryTest { fun `given the advertisement id exists, when collecting device info, then parsely site uuid is advertisement id`() { // given val advertisementId = "ad id" - sut = AndroidDeviceInfoRepository( + val sut = AndroidDeviceInfoRepository( advertisementIdProvider = { advertisementId }, androidIdProvider = { "android id" }) @@ -41,7 +39,7 @@ internal class AndroidDeviceInfoRepositoryTest { fun `given the advertisement is null and android id is not, when collecting device info, then parsely id is android id`() { // given val androidId = "android id" - sut = AndroidDeviceInfoRepository( + val sut = AndroidDeviceInfoRepository( advertisementIdProvider = { null }, androidIdProvider = { androidId } ) @@ -56,7 +54,7 @@ internal class AndroidDeviceInfoRepositoryTest { @Test fun `given both advertisement id and android id are null, when collecting device info, then parsely id is empty`() { // given - sut = AndroidDeviceInfoRepository( + val sut = AndroidDeviceInfoRepository( advertisementIdProvider = { null }, androidIdProvider = { null } ) From 0c3102d82753ebc1c8ec672898b5526be4936b20 Mon Sep 17 00:00:00 2001 From: Wojtek Zieba Date: Thu, 7 Dec 2023 11:24:01 +0100 Subject: [PATCH 18/18] chore: add a comment for AdvertisementIdProvider#provide --- .../com/parsely/parselyandroid/AdvertisementIdProvider.kt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt index ba76ca1c..e9f11ce6 100644 --- a/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt +++ b/parsely/src/main/java/com/parsely/parselyandroid/AdvertisementIdProvider.kt @@ -23,6 +23,10 @@ internal class AdvertisementIdProvider( } } + /** + * @return advertisement id if the coroutine in the constructor finished executing AdvertisingIdClient#getAdvertisingIdInfo + * null otherwise + */ override fun provide(): String? = adKey }