From 652b67b2d7661d3d522009830fca08236973181f Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Wed, 25 Sep 2024 14:08:15 -0700 Subject: [PATCH 01/12] feat: Migrate storage to v3 --- .../java/com/amplitude/android/Amplitude.kt | 18 +- .../com/amplitude/android/Configuration.kt | 27 ++- .../migration/AndroidStorageMigration.kt | 75 ++++++++ .../migration/ApiKeyStorageMigration.kt | 27 --- .../migration/IdentityStorageMigration.kt | 26 +++ .../android/migration/MigrationManager.kt | 55 ++++++ .../android/migration/RemnantDataMigration.kt | 8 +- .../android/migration/StorageKeyMigration.kt | 106 ----------- .../storage/AndroidStorageContextV1.kt | 102 +++++++++++ .../storage/AndroidStorageContextV2.kt | 102 +++++++++++ .../storage/AndroidStorageContextV3.kt | 56 ++++++ .../android/storage/AndroidStorageV2.kt | 166 ++++++++++++++++++ .../storage/LegacySdkStorageContext.kt | 15 ++ .../com/amplitude/android/storage/README.md | 7 + .../android/storage/StorageVersion.kt | 5 + .../android/utilities/AndroidStorage.kt | 86 ++++----- .../amplitude/android/AmplitudeSessionTest.kt | 5 +- ...Test.kt => AndroidStorageMigrationTest.kt} | 10 +- .../main/java/com/amplitude/core/Amplitude.kt | 3 + .../amplitude/core/utilities/FileStorage.kt | 3 +- .../core/utilities/EventsFileManagerTest.kt | 32 ++-- .../com/amplitude/id/FileIdentityStorage.kt | 10 +- .../com/amplitude/id/IMIdentityStorage.kt | 5 + .../com/amplitude/id/IdentityConfiguration.kt | 3 +- .../java/com/amplitude/id/IdentityStorage.kt | 2 + .../amplitude/id/utilities/PropertiesFile.kt | 13 +- 26 files changed, 726 insertions(+), 241 deletions(-) create mode 100644 android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt delete mode 100644 android/src/main/java/com/amplitude/android/migration/ApiKeyStorageMigration.kt create mode 100644 android/src/main/java/com/amplitude/android/migration/IdentityStorageMigration.kt create mode 100644 android/src/main/java/com/amplitude/android/migration/MigrationManager.kt delete mode 100644 android/src/main/java/com/amplitude/android/migration/StorageKeyMigration.kt create mode 100644 android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt create mode 100644 android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt create mode 100644 android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV3.kt create mode 100644 android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt create mode 100644 android/src/main/java/com/amplitude/android/storage/LegacySdkStorageContext.kt create mode 100644 android/src/main/java/com/amplitude/android/storage/README.md create mode 100644 android/src/main/java/com/amplitude/android/storage/StorageVersion.kt rename android/src/test/java/com/amplitude/android/migration/{StorageKeyMigrationTest.kt => AndroidStorageMigrationTest.kt} (95%) diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index bb5d0c61..4435f862 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -1,18 +1,18 @@ package com.amplitude.android import android.content.Context -import com.amplitude.android.migration.ApiKeyStorageMigration -import com.amplitude.android.migration.RemnantDataMigration +import com.amplitude.android.migration.MigrationManager import com.amplitude.android.plugins.AnalyticsConnectorIdentityPlugin import com.amplitude.android.plugins.AnalyticsConnectorPlugin import com.amplitude.android.plugins.AndroidContextPlugin import com.amplitude.android.plugins.AndroidLifecyclePlugin import com.amplitude.android.plugins.AndroidNetworkConnectivityCheckerPlugin +import com.amplitude.android.storage.AndroidStorageContextV3 +import com.amplitude.android.storage.LegacySdkStorageContext import com.amplitude.core.Amplitude import com.amplitude.core.events.BaseEvent import com.amplitude.core.platform.plugins.AmplitudeDestination import com.amplitude.core.platform.plugins.GetAmpliExtrasPlugin -import com.amplitude.core.utilities.FileStorage import com.amplitude.id.IdentityConfiguration import kotlinx.coroutines.launch @@ -36,22 +36,24 @@ open class Amplitude( override fun createIdentityConfiguration(): IdentityConfiguration { val configuration = configuration as Configuration - val storageDirectory = configuration.context.getDir("${FileStorage.STORAGE_PREFIX}-${configuration.instanceName}", Context.MODE_PRIVATE) return IdentityConfiguration( instanceName = configuration.instanceName, apiKey = configuration.apiKey, identityStorageProvider = configuration.identityStorageProvider, - storageDirectory = storageDirectory, - logger = configuration.loggerProvider.getLogger(this) + storageDirectory = AndroidStorageContextV3.getIdentityStorageDirectory(configuration), + logger = configuration.loggerProvider.getLogger(this), + fileName = AndroidStorageContextV3.getIdentityStorageFileName() ) } override suspend fun buildInternal(identityConfiguration: IdentityConfiguration) { - ApiKeyStorageMigration(this).execute() + val migrationManager = MigrationManager(this) + migrationManager.migrateOldStorage() if ((this.configuration as Configuration).migrateLegacyData) { - RemnantDataMigration(this).execute() + val legacySdkStorageContext = LegacySdkStorageContext(this) + legacySdkStorageContext.migrateToLatestVersion() } this.createIdentityContainer(identityConfiguration) diff --git a/android/src/main/java/com/amplitude/android/Configuration.kt b/android/src/main/java/com/amplitude/android/Configuration.kt index 0e71a02a..2c620870 100644 --- a/android/src/main/java/com/amplitude/android/Configuration.kt +++ b/android/src/main/java/com/amplitude/android/Configuration.kt @@ -1,8 +1,8 @@ package com.amplitude.android import android.content.Context +import com.amplitude.android.storage.AndroidStorageContextV3 import com.amplitude.android.utilities.AndroidLoggerProvider -import com.amplitude.android.utilities.AndroidStorageProvider import com.amplitude.core.Configuration import com.amplitude.core.EventCallBack import com.amplitude.core.LoggerProvider @@ -10,8 +10,8 @@ import com.amplitude.core.ServerZone import com.amplitude.core.StorageProvider import com.amplitude.core.events.IngestionMetadata import com.amplitude.core.events.Plan -import com.amplitude.id.FileIdentityStorageProvider import com.amplitude.id.IdentityStorageProvider +import java.io.File open class Configuration( apiKey: String, @@ -20,7 +20,7 @@ open class Configuration( override var flushIntervalMillis: Int = FLUSH_INTERVAL_MILLIS, override var instanceName: String = DEFAULT_INSTANCE, override var optOut: Boolean = false, - override var storageProvider: StorageProvider = AndroidStorageProvider(), + override var storageProvider: StorageProvider = AndroidStorageContextV3.eventsStorageProvider, override var loggerProvider: LoggerProvider = AndroidLoggerProvider(), override var minIdLength: Int? = null, override var partnerId: String? = null, @@ -41,8 +41,8 @@ open class Configuration( var minTimeBetweenSessionsMillis: Long = MIN_TIME_BETWEEN_SESSIONS_MILLIS, autocapture: Set = setOf(AutocaptureOption.SESSIONS), override var identifyBatchIntervalMillis: Long = IDENTIFY_BATCH_INTERVAL_MILLIS, - override var identifyInterceptStorageProvider: StorageProvider = AndroidStorageProvider(), - override var identityStorageProvider: IdentityStorageProvider = FileIdentityStorageProvider(), + override var identifyInterceptStorageProvider: StorageProvider = AndroidStorageContextV3.identifyInterceptStorageProvider, + override var identityStorageProvider: IdentityStorageProvider = AndroidStorageContextV3.identityStorageProvider, var migrateLegacyData: Boolean = true, override var offline: Boolean? = false, override var deviceId: String? = null, @@ -84,7 +84,7 @@ open class Configuration( flushIntervalMillis: Int = FLUSH_INTERVAL_MILLIS, instanceName: String = DEFAULT_INSTANCE, optOut: Boolean = false, - storageProvider: StorageProvider = AndroidStorageProvider(), + storageProvider: StorageProvider = AndroidStorageContextV3.eventsStorageProvider, loggerProvider: LoggerProvider = AndroidLoggerProvider(), minIdLength: Int? = null, partnerId: String? = null, @@ -106,8 +106,8 @@ open class Configuration( trackingSessionEvents: Boolean = true, @Suppress("DEPRECATION") defaultTracking: DefaultTrackingOptions = DefaultTrackingOptions(), identifyBatchIntervalMillis: Long = IDENTIFY_BATCH_INTERVAL_MILLIS, - identifyInterceptStorageProvider: StorageProvider = AndroidStorageProvider(), - identityStorageProvider: IdentityStorageProvider = FileIdentityStorageProvider(), + identifyInterceptStorageProvider: StorageProvider = AndroidStorageContextV3.identifyInterceptStorageProvider, + identityStorageProvider: IdentityStorageProvider = AndroidStorageContextV3.identityStorageProvider, migrateLegacyData: Boolean = true, offline: Boolean? = false, deviceId: String? = null, @@ -154,6 +154,8 @@ open class Configuration( this.defaultTracking = defaultTracking } + private var storageDirectory: File? = null + // A backing property to store the autocapture options. Any changes to `trackingSessionEvents` // or the `defaultTracking` options will be reflected in this property. private var _autocapture: MutableSet = autocapture.toMutableSet() @@ -181,4 +183,13 @@ open class Configuration( private fun DefaultTrackingOptions.updateAutocaptureOnPropertyChange() { _autocapture = autocaptureOptions } + + internal fun getStorageDirectory(): File { + if (storageDirectory == null) { + val dir = context.getDir("amplitude", Context.MODE_PRIVATE) + storageDirectory = File(dir, "${context.packageName}/$instanceName/analytics/") + storageDirectory?.mkdirs() + } + return storageDirectory!! + } } diff --git a/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt b/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt new file mode 100644 index 00000000..aa54e98f --- /dev/null +++ b/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt @@ -0,0 +1,75 @@ +package com.amplitude.android.migration + +import com.amplitude.android.storage.AndroidStorageV2 +import com.amplitude.common.Logger +import com.amplitude.core.Storage +import com.amplitude.core.utilities.toBaseEvent +import org.json.JSONObject + +class AndroidStorageMigration( + private val source: AndroidStorageV2, + private val destination: AndroidStorageV2, + private val logger: Logger +) { + suspend fun execute() { + moveSourceEventFilesToDestination() + moveSimpleValues() + } + + private suspend fun moveSourceEventFilesToDestination() { + try { + source.rollover() + val sourceEventFiles = source.readEventsContent() as List + if (sourceEventFiles.isEmpty()) { + return + } + + for (sourceEventFilePath in sourceEventFiles) { + val events = source.readEventsContent() as List + var count = 0 + for (event in events) { + try { + count++ + destination.writeEvent(JSONObject(event).toBaseEvent()) + } catch (e: Exception) { + logger.error("can't move event ($event) from file $sourceEventFilePath: ${e.message}") + } + } + logger.debug("Migrated $count/${events.size} events from $sourceEventFilePath") + source.removeFile(sourceEventFilePath) + } + } catch (e: Exception) { + logger.error("can't move event files: ${e.message}") + } + } + + private suspend fun moveSimpleValues() { + moveSimpleValue(Storage.Constants.PREVIOUS_SESSION_ID) + moveSimpleValue(Storage.Constants.LAST_EVENT_TIME) + moveSimpleValue(Storage.Constants.LAST_EVENT_ID) + + moveSimpleValue(Storage.Constants.OPT_OUT) + moveSimpleValue(Storage.Constants.Events) + moveSimpleValue(Storage.Constants.APP_VERSION) + moveSimpleValue(Storage.Constants.APP_BUILD) + } + + private suspend fun moveSimpleValue(key: Storage.Constants) { + try { + val sourceValue = source.read(key) ?: return + val destinationValue = destination.read(key) + if (destinationValue == null) { + try { + logger.debug("Migrating $key with value $sourceValue") + destination.write(key, sourceValue) + } catch (e: Exception) { + logger.error("can't write destination $key: ${e.message}") + return + } + } + source.remove(key) + } catch (e: Exception) { + logger.error("can't move $key: ${e.message}") + } + } +} diff --git a/android/src/main/java/com/amplitude/android/migration/ApiKeyStorageMigration.kt b/android/src/main/java/com/amplitude/android/migration/ApiKeyStorageMigration.kt deleted file mode 100644 index 9507a66e..00000000 --- a/android/src/main/java/com/amplitude/android/migration/ApiKeyStorageMigration.kt +++ /dev/null @@ -1,27 +0,0 @@ -package com.amplitude.android.migration - -import com.amplitude.android.Amplitude -import com.amplitude.android.Configuration -import com.amplitude.android.utilities.AndroidStorage - -class ApiKeyStorageMigration( - private val amplitude: Amplitude, -) { - suspend fun execute() { - val configuration = amplitude.configuration as Configuration - val logger = amplitude.logger - - val storage = amplitude.storage as? AndroidStorage - if (storage != null) { - val apiKeyStorage = AndroidStorage(configuration.context, configuration.apiKey, logger, storage.prefix, amplitude.diagnostics) - StorageKeyMigration(apiKeyStorage, storage, logger).execute() - } - - val identifyInterceptStorage = amplitude.identifyInterceptStorage as? AndroidStorage - if (identifyInterceptStorage != null) { - val apiKeyStorage = - AndroidStorage(configuration.context, configuration.apiKey, logger, identifyInterceptStorage.prefix, amplitude.diagnostics) - StorageKeyMigration(apiKeyStorage, identifyInterceptStorage, logger).execute() - } - } -} diff --git a/android/src/main/java/com/amplitude/android/migration/IdentityStorageMigration.kt b/android/src/main/java/com/amplitude/android/migration/IdentityStorageMigration.kt new file mode 100644 index 00000000..b88c0745 --- /dev/null +++ b/android/src/main/java/com/amplitude/android/migration/IdentityStorageMigration.kt @@ -0,0 +1,26 @@ +package com.amplitude.android.migration + +import com.amplitude.common.Logger +import com.amplitude.id.IdentityStorage + +class IdentityStorageMigration( + private val source: IdentityStorage, + private val destination: IdentityStorage, + private val logger: Logger +) { + fun execute() { + try { + val identity = source.load() + logger.debug("Loaded old identity: $identity") + if (identity.userId != null) { + destination.saveUserId(identity.userId) + } + if (identity.deviceId != null) { + destination.saveDeviceId(identity.deviceId) + } + source.delete() + } catch (e: Exception) { + logger.error("Unable to migrate file identity storage: ${e.message}") + } + } +} \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/migration/MigrationManager.kt b/android/src/main/java/com/amplitude/android/migration/MigrationManager.kt new file mode 100644 index 00000000..a53973e9 --- /dev/null +++ b/android/src/main/java/com/amplitude/android/migration/MigrationManager.kt @@ -0,0 +1,55 @@ +package com.amplitude.android.migration + +import android.content.Context +import android.content.SharedPreferences +import com.amplitude.android.Amplitude +import com.amplitude.android.Configuration +import com.amplitude.android.storage.AndroidStorageContextV1 +import com.amplitude.android.storage.AndroidStorageContextV2 +import com.amplitude.android.storage.LegacySdkStorageContext +import com.amplitude.android.storage.StorageVersion +import com.amplitude.common.Logger + +internal class MigrationManager(private val amplitude: Amplitude) { + private val sharedPreferences: SharedPreferences + private val config: Configuration = amplitude.configuration as Configuration + private val logger: Logger = amplitude.logger + private val currentStorageVersion: Int + + init { + sharedPreferences = config.context.getSharedPreferences( + "amplitude-android-${config.instanceName}", + Context.MODE_PRIVATE + ) + currentStorageVersion = sharedPreferences.getInt("storage_version", 0) + } + + suspend fun migrateOldStorage() { + if (currentStorageVersion < StorageVersion.V3.rawValue) { + logger.debug("Migrating storage to version ${StorageVersion.V3.rawValue}") + safePerformMigration() + } else { + amplitude.logger.debug("Storage already at version ${StorageVersion.V3.rawValue}") + } + } + + internal suspend fun safePerformMigration() { + try { + val config = amplitude.configuration as Configuration + if (config.migrateLegacyData) { + val legacySdkStorageContext = LegacySdkStorageContext(amplitude) + legacySdkStorageContext.migrateToLatestVersion() + } + + val storageContextV1 = AndroidStorageContextV1(amplitude, config) + storageContextV1.migrateToLatestVersion() + + val storageContextV2 = AndroidStorageContextV2(amplitude, config) + storageContextV2.migrateToLatestVersion() + + sharedPreferences.edit().putInt("storage_version", StorageVersion.V3.rawValue).apply() + } catch (ex: Throwable) { + logger.error("Failed to migrate storage: ${ex.message}") + } + } +} \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/migration/RemnantDataMigration.kt b/android/src/main/java/com/amplitude/android/migration/RemnantDataMigration.kt index 3fc9e288..d2193d07 100644 --- a/android/src/main/java/com/amplitude/android/migration/RemnantDataMigration.kt +++ b/android/src/main/java/com/amplitude/android/migration/RemnantDataMigration.kt @@ -15,9 +15,7 @@ import org.json.JSONObject * 4. deletes data from sqlite table */ -class RemnantDataMigration( - val amplitude: Amplitude, -) { +class RemnantDataMigration(val amplitude: Amplitude, private val databaseStorage: DatabaseStorage) { companion object { const val DEVICE_ID_KEY = "device_id" const val USER_ID_KEY = "user_id" @@ -26,11 +24,7 @@ class RemnantDataMigration( const val PREVIOUS_SESSION_ID_KEY = "previous_session_id" } - lateinit var databaseStorage: DatabaseStorage - suspend fun execute() { - databaseStorage = DatabaseStorageProvider.getStorage(amplitude) - val firstRunSinceUpgrade = amplitude.storage.read(Storage.Constants.LAST_EVENT_TIME)?.toLongOrNull() == null moveDeviceAndUserId() diff --git a/android/src/main/java/com/amplitude/android/migration/StorageKeyMigration.kt b/android/src/main/java/com/amplitude/android/migration/StorageKeyMigration.kt deleted file mode 100644 index 52f7217a..00000000 --- a/android/src/main/java/com/amplitude/android/migration/StorageKeyMigration.kt +++ /dev/null @@ -1,106 +0,0 @@ -package com.amplitude.android.migration - -import com.amplitude.android.utilities.AndroidStorage -import com.amplitude.common.Logger -import com.amplitude.core.Storage -import java.io.File -import java.util.UUID - -class StorageKeyMigration( - private val source: AndroidStorage, - private val destination: AndroidStorage, - private val logger: Logger -) { - suspend fun execute() { - if (source.storageKey == destination.storageKey) { - return - } - moveSourceEventFilesToDestination() - moveSimpleValues() - } - - private suspend fun moveSourceEventFilesToDestination() { - try { - source.rollover() - val sourceEventFiles = source.readEventsContent() as List - if (sourceEventFiles.isEmpty()) { - return - } - - for (sourceEventFilePath in sourceEventFiles) { - val sourceEventFile = File(sourceEventFilePath) - var destinationEventFile = - File(sourceEventFilePath.replace(source.storageKey, destination.storageKey)) - if (destinationEventFile.exists()) { - var fileExtension = destinationEventFile.extension - if (fileExtension != "") { - fileExtension = ".$fileExtension" - } - destinationEventFile = File( - destinationEventFile.parent, - "${destinationEventFile.nameWithoutExtension}-${UUID.randomUUID()}$fileExtension" - ) - } - try { - sourceEventFile.renameTo(destinationEventFile) - } catch (e: Exception) { - logger.error("can't rename $sourceEventFile to $destinationEventFile: ${e.message}") - } - } - } catch (e: Exception) { - logger.error("can't move event files: ${e.message}") - } - } - - private suspend fun moveSimpleValues() { - moveSimpleValue(Storage.Constants.PREVIOUS_SESSION_ID) - moveSimpleValue(Storage.Constants.LAST_EVENT_TIME) - moveSimpleValue(Storage.Constants.LAST_EVENT_ID) - - moveSimpleValue(Storage.Constants.OPT_OUT) - moveSimpleValue(Storage.Constants.Events) - moveSimpleValue(Storage.Constants.APP_VERSION) - moveSimpleValue(Storage.Constants.APP_BUILD) - - moveFileIndex() - } - - private suspend fun moveSimpleValue(key: Storage.Constants) { - try { - val sourceValue = source.read(key) ?: return - - val destinationValue = destination.read(key) - if (destinationValue == null) { - try { - destination.write(key, sourceValue) - } catch (e: Exception) { - logger.error("can't write destination $key: ${e.message}") - return - } - } - - source.remove(key) - } catch (e: Exception) { - logger.error("can't move $key: ${e.message}") - } - } - - private fun moveFileIndex() { - try { - val sourceFileIndexKey = "amplitude.events.file.index.${source.storageKey}" - val destinationFileIndexKey = "amplitude.events.file.index.${destination.storageKey}" - if (source.sharedPreferences.contains(sourceFileIndexKey)) { - val fileIndex = source.sharedPreferences.getLong(sourceFileIndexKey, -1) - try { - destination.sharedPreferences.edit().putLong(destinationFileIndexKey, fileIndex).commit() - } catch (e: Exception) { - logger.error("can't write file index: ${e.message}") - return - } - source.sharedPreferences.edit().remove(sourceFileIndexKey).commit() - } - } catch (e: Exception) { - logger.error("can't move file index: ${e.message}") - } - } -} diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt new file mode 100644 index 00000000..682e7c06 --- /dev/null +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt @@ -0,0 +1,102 @@ +package com.amplitude.android.storage + +import android.content.Context +import com.amplitude.android.Amplitude +import com.amplitude.android.Configuration +import com.amplitude.android.migration.AndroidStorageMigration +import com.amplitude.android.migration.IdentityStorageMigration +import com.amplitude.android.utilities.AndroidStorage +import com.amplitude.core.utilities.FileStorage +import com.amplitude.id.FileIdentityStorage +import com.amplitude.id.IdentityConfiguration + +/** + * Data is stored in storage in the following format + * /app_amplitude-kotlin-{api_key} + * /amplitude-identify-{api_key}.properties (this stores the user id, device id and api key) + * /app_amplitude-disk-queue (this stores the events) + * /{instance_name}-0 + * /{instance_name}-1.tmp + * /app_amplitude-identify-intercept-disk-queue + * /{instance_name}-0 + * /{instance_name}-1.tmp + * /shared_prefs + * /amplitude-android-{api_key}.xml + */ +internal class AndroidStorageContextV1( + private val amplitude: Amplitude, + configuration: Configuration +) { + /** + * Stores all event data in storage + */ + private val eventsStorage: AndroidStorage + + /** + * Stores all identity data in storage (user id, device id etc) + */ + private val identityStorage: FileIdentityStorage + + /** + * Stores identifies intercepted by the SDK to reduce data sent over to the server + */ + private val identifyInterceptStorage: AndroidStorage + + init { + eventsStorage = AndroidStorage( + configuration.context, + configuration.apiKey, + configuration.loggerProvider.getLogger(amplitude), + null, + amplitude.diagnostics + ) + + identityStorage = FileIdentityStorage( + generateIdentityConfiguration(amplitude, configuration) + ) + + identifyInterceptStorage = AndroidStorage( + configuration.context, + configuration.instanceName, + configuration.loggerProvider.getLogger(amplitude), + "amplitude-identify-intercept", + amplitude.diagnostics + ) + } + + private fun generateIdentityConfiguration( + amplitude: Amplitude, + configuration: Configuration + ): IdentityConfiguration { + val storageDirectory = configuration.context.getDir( + "${FileStorage.STORAGE_PREFIX}-${configuration.apiKey}", + Context.MODE_PRIVATE + ) + + return IdentityConfiguration( + instanceName = configuration.instanceName, + apiKey = configuration.apiKey, + identityStorageProvider = configuration.identityStorageProvider, + storageDirectory = storageDirectory, + logger = configuration.loggerProvider.getLogger(amplitude), + fileName = "amplitude-identify-${configuration.apiKey}" + ) + } + + suspend fun migrateToLatestVersion() { + val identityMigration = + IdentityStorageMigration(identityStorage, amplitude.identityStorage, amplitude.logger) + identityMigration.execute() + + (amplitude.storage as? AndroidStorageV2)?.let { + val migrator = AndroidStorageMigration(eventsStorage.storageV2, it, amplitude.logger) + migrator.execute() + } + + (amplitude.identifyInterceptStorage as? AndroidStorageV2)?.let { + val migrator = + AndroidStorageMigration(identifyInterceptStorage.storageV2, it, amplitude.logger) + migrator.execute() + } + } +} \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt new file mode 100644 index 00000000..58b9e91d --- /dev/null +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt @@ -0,0 +1,102 @@ +package com.amplitude.android.storage + +import android.content.Context +import com.amplitude.android.Amplitude +import com.amplitude.android.Configuration +import com.amplitude.android.migration.AndroidStorageMigration +import com.amplitude.android.migration.IdentityStorageMigration +import com.amplitude.android.utilities.AndroidStorage +import com.amplitude.core.utilities.FileStorage +import com.amplitude.id.FileIdentityStorage +import com.amplitude.id.IdentityConfiguration + +/** + * Data is stored in storage in the following format + * /app_amplitude-kotlin-{instance_name} + * /amplitude-identify-{instance_name}.properties (this stores the user id, device id and api key) + * /app_amplitude-disk-queue (this stores the events) + * /{instance_name}-0 + * /{instance_name}-1.tmp + * /app_amplitude-identify-intercept-disk-queue + * /{instance_name}-0 + * /{instance_name}-1.tmp + * /shared_prefs + * /amplitude-android-{instance_name}.xml + */ +internal class AndroidStorageContextV2( + private val amplitude: Amplitude, + configuration: Configuration +) { + /** + * Stores all event data in storage + */ + private val eventsStorage: AndroidStorage + + /** + * Stores all identity data in storage (user id, device id etc) + */ + private val identityStorage: FileIdentityStorage + + /** + * Stores identifies intercepted by the SDK to reduce data sent over to the server + */ + private val identifyInterceptStorage: AndroidStorage + + init { + eventsStorage = AndroidStorage( + configuration.context, + configuration.instanceName, + configuration.loggerProvider.getLogger(amplitude), + null, + amplitude.diagnostics + ) + + identityStorage = FileIdentityStorage( + generateIdentityConfiguration(amplitude, configuration) + ) + + identifyInterceptStorage = AndroidStorage( + configuration.context, + configuration.instanceName, + configuration.loggerProvider.getLogger(amplitude), + "amplitude-identify-intercept", + amplitude.diagnostics + ) + } + + private fun generateIdentityConfiguration( + amplitude: Amplitude, + configuration: Configuration + ): IdentityConfiguration { + val storageDirectory = configuration.context.getDir( + "${FileStorage.STORAGE_PREFIX}-${configuration.instanceName}", + Context.MODE_PRIVATE + ) + + return IdentityConfiguration( + instanceName = configuration.instanceName, + apiKey = configuration.apiKey, + identityStorageProvider = configuration.identityStorageProvider, + storageDirectory = storageDirectory, + logger = configuration.loggerProvider.getLogger(amplitude), + fileName = "amplitude-identify-${configuration.instanceName}" + ) + } + + suspend fun migrateToLatestVersion() { + val identityMigration = + IdentityStorageMigration(identityStorage, amplitude.identityStorage, amplitude.logger) + identityMigration.execute() + + (amplitude.storage as? AndroidStorageV2)?.let { + val migrator = AndroidStorageMigration(eventsStorage.storageV2, it, amplitude.logger) + migrator.execute() + } + + (amplitude.identifyInterceptStorage as? AndroidStorageV2)?.let { + val migrator = + AndroidStorageMigration(identifyInterceptStorage.storageV2, it, amplitude.logger) + migrator.execute() + } + } +} \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV3.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV3.kt new file mode 100644 index 00000000..5fc1f143 --- /dev/null +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV3.kt @@ -0,0 +1,56 @@ +package com.amplitude.android.storage + +import com.amplitude.android.Configuration +import com.amplitude.core.StorageProvider +import com.amplitude.id.FileIdentityStorageProvider +import com.amplitude.id.IdentityStorageProvider +import java.io.File + +/** + * Data is stored in storage in the following format + * /amplitude + * /package_name + * /instance_name + * /analytics + * /events (stores the events) + * /{instance_name}-0 + * /{instance_name}-1.tmp + * /identify-intercept (stores the intercepted identifies) + * /{instance_name}-0 + * /{instance_name}-1.tmp + * /identity.properties (this stores the user id, device id and api key) + * /shared_prefs + * /amplitude-android-{instance_name}.xml + */ +internal object AndroidStorageContextV3 { + /** + * Stores all event data in storage + */ + val eventsStorageProvider: StorageProvider = AndroidEventsStorageProviderV2() + + /** + * Stores all identity data in storage (user id, device id etc) + */ + val identityStorageProvider: IdentityStorageProvider = FileIdentityStorageProvider() + + /** + * Stores identifies intercepted by the SDK to reduce data sent over to the server + */ + val identifyInterceptStorageProvider: StorageProvider = AndroidIdentifyInterceptStorageProviderV2() + + fun getEventsStorageDirectory(configuration: Configuration): File { + return File(configuration.getStorageDirectory(), "events") + } + + fun getIdentifyInterceptStorageDirectory(configuration: Configuration): File { + return File(configuration.getStorageDirectory(), "identify-intercept") + } + + fun getIdentityStorageDirectory(configuration: Configuration): File { + return configuration.getStorageDirectory() + } + + fun getIdentityStorageFileName(): String { + return "identity" + } +} \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt new file mode 100644 index 00000000..a09b15e9 --- /dev/null +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt @@ -0,0 +1,166 @@ +package com.amplitude.android.storage + +import android.content.Context +import android.content.SharedPreferences +import com.amplitude.android.utilities.AndroidKVS +import com.amplitude.common.Logger +import com.amplitude.core.Amplitude +import com.amplitude.core.Configuration +import com.amplitude.core.EventCallBack +import com.amplitude.core.Storage +import com.amplitude.core.StorageProvider +import com.amplitude.core.events.BaseEvent +import com.amplitude.core.platform.EventPipeline +import com.amplitude.core.utilities.Diagnostics +import com.amplitude.core.utilities.EventsFileManager +import com.amplitude.core.utilities.EventsFileStorage +import com.amplitude.core.utilities.FileResponseHandler +import com.amplitude.core.utilities.JSONUtil +import com.amplitude.core.utilities.ResponseHandler +import kotlinx.coroutines.CoroutineDispatcher +import kotlinx.coroutines.CoroutineScope +import org.json.JSONArray +import java.io.File + +class AndroidStorageV2( + /** + * A generic key to differentiate multiple storage instances. + */ + val storageKey: String, + private val logger: Logger, + /** + * A place where the storage stores some metadata to manage this storage + */ + val sharedPreferences: SharedPreferences, + /** + * A directory where the storage stores the actual data. This should not be shared with other + * storage instances + */ + storageDirectory: File, + diagnostics: Diagnostics, +) : Storage, EventsFileStorage { + private val eventsFile = + EventsFileManager( + storageDirectory, + storageKey, + AndroidKVS(sharedPreferences), + logger, + diagnostics + ) + private val eventCallbacksMap = mutableMapOf() + + override suspend fun writeEvent(event: BaseEvent) { + eventsFile.storeEvent(JSONUtil.eventToString(event)) + event.callback?.let { callback -> + event.insertId?.let { + eventCallbacksMap.put(it, callback) + } + } + } + + override suspend fun write( + key: Storage.Constants, + value: String, + ) { + sharedPreferences.edit().putString(key.rawVal, value).apply() + } + + override suspend fun remove(key: Storage.Constants) { + sharedPreferences.edit().remove(key.rawVal).apply() + } + + override suspend fun rollover() { + eventsFile.rollover() + } + + override fun read(key: Storage.Constants): String? { + return sharedPreferences.getString(key.rawVal, null) + } + + override fun readEventsContent(): List { + return eventsFile.read() + } + + override fun releaseFile(filePath: String) { + eventsFile.release(filePath) + } + + override suspend fun getEventsString(content: Any): String { + return eventsFile.getEventString(content as String) + } + + override fun getResponseHandler( + eventPipeline: EventPipeline, + configuration: Configuration, + scope: CoroutineScope, + dispatcher: CoroutineDispatcher, + ): ResponseHandler { + return FileResponseHandler( + this, + eventPipeline, + configuration, + scope, + dispatcher, + logger, + ) + } + + override fun removeFile(filePath: String): Boolean { + return eventsFile.remove(filePath) + } + + override fun getEventCallback(insertId: String): EventCallBack? { + return eventCallbacksMap[insertId] + } + + override fun removeEventCallback(insertId: String) { + eventCallbacksMap.remove(insertId) + } + + override fun splitEventFile( + filePath: String, + events: JSONArray, + ) { + eventsFile.splitFile(filePath, events) + } +} + +class AndroidEventsStorageProviderV2 : StorageProvider { + override fun getStorage( + amplitude: Amplitude, + prefix: String?, + ): Storage { + val configuration = amplitude.configuration as com.amplitude.android.Configuration + val sharedPreferencesName = "amplitude-events-${configuration.context.packageName}-${configuration.instanceName}" + val sharedPreferences = configuration.context.getSharedPreferences(sharedPreferencesName, Context.MODE_PRIVATE) + return AndroidStorageV2( + configuration.instanceName, + configuration.loggerProvider.getLogger(amplitude), + sharedPreferences, + AndroidStorageContextV3.getEventsStorageDirectory(configuration), + amplitude.diagnostics, + ) + } +} + +class AndroidIdentifyInterceptStorageProviderV2 : StorageProvider { + override fun getStorage( + amplitude: Amplitude, + prefix: String?, + ): Storage { + val configuration = amplitude.configuration as com.amplitude.android.Configuration + val sharedPreferences = configuration.context.getSharedPreferences( + "amplitude-id-intercept-${configuration.context.packageName}-${configuration.instanceName}", + Context.MODE_PRIVATE + ) + return AndroidStorageV2( + configuration.instanceName, + configuration.loggerProvider.getLogger(amplitude), + sharedPreferences, + AndroidStorageContextV3.getIdentifyInterceptStorageDirectory(configuration), + amplitude.diagnostics, + ) + } +} + + diff --git a/android/src/main/java/com/amplitude/android/storage/LegacySdkStorageContext.kt b/android/src/main/java/com/amplitude/android/storage/LegacySdkStorageContext.kt new file mode 100644 index 00000000..843d0fa8 --- /dev/null +++ b/android/src/main/java/com/amplitude/android/storage/LegacySdkStorageContext.kt @@ -0,0 +1,15 @@ +package com.amplitude.android.storage + +import com.amplitude.android.Amplitude +import com.amplitude.android.migration.DatabaseStorage +import com.amplitude.android.migration.DatabaseStorageProvider +import com.amplitude.android.migration.RemnantDataMigration + +internal class LegacySdkStorageContext(val amplitude: Amplitude) { + private val databaseStorage: DatabaseStorage = DatabaseStorageProvider.getStorage(amplitude) + + suspend fun migrateToLatestVersion() { + val remnantDataMigration = RemnantDataMigration(amplitude, databaseStorage) + remnantDataMigration.execute() + } +} \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/storage/README.md b/android/src/main/java/com/amplitude/android/storage/README.md new file mode 100644 index 00000000..c53cfbaa --- /dev/null +++ b/android/src/main/java/com/amplitude/android/storage/README.md @@ -0,0 +1,7 @@ +As the Android SDK evolves, we'll end up with different storage configuration. Its important that +we have a single place to keep track of all the storage configurations. + +For every new storage version, please make sure you have a corresponding storage context class which +houses all the storage related fields. Also, please make sure the storage context also outlines +how the data is structured in disk so that when we move to a new version of storage, we can make +sure we don't leave any data behind. \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/storage/StorageVersion.kt b/android/src/main/java/com/amplitude/android/storage/StorageVersion.kt new file mode 100644 index 00000000..260d28d6 --- /dev/null +++ b/android/src/main/java/com/amplitude/android/storage/StorageVersion.kt @@ -0,0 +1,5 @@ +package com.amplitude.android.storage + +enum class StorageVersion(val rawValue:Int) { + V3(3), +} \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidStorage.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidStorage.kt index 10fa9ef3..803306b7 100644 --- a/android/src/main/java/com/amplitude/android/utilities/AndroidStorage.kt +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidStorage.kt @@ -2,81 +2,80 @@ package com.amplitude.android.utilities import android.content.Context import android.content.SharedPreferences +import com.amplitude.android.storage.AndroidStorageV2 import com.amplitude.common.Logger -import com.amplitude.core.Amplitude import com.amplitude.core.Configuration import com.amplitude.core.EventCallBack import com.amplitude.core.Storage -import com.amplitude.core.StorageProvider import com.amplitude.core.events.BaseEvent import com.amplitude.core.platform.EventPipeline import com.amplitude.core.utilities.Diagnostics -import com.amplitude.core.utilities.EventsFileManager import com.amplitude.core.utilities.EventsFileStorage -import com.amplitude.core.utilities.FileResponseHandler -import com.amplitude.core.utilities.JSONUtil import com.amplitude.core.utilities.ResponseHandler import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope import org.json.JSONArray -import java.io.File class AndroidStorage( context: Context, val storageKey: String, - private val logger: Logger, + logger: Logger, internal val prefix: String?, - private val diagnostics: Diagnostics, + diagnostics: Diagnostics, ) : Storage, EventsFileStorage { companion object { const val STORAGE_PREFIX = "amplitude-android" } - internal val sharedPreferences: SharedPreferences = - context.getSharedPreferences("${getPrefix()}-$storageKey", Context.MODE_PRIVATE) - private val storageDirectory: File = context.getDir(getDir(), Context.MODE_PRIVATE) - private val eventsFile = - EventsFileManager(storageDirectory, storageKey, AndroidKVS(sharedPreferences), logger, diagnostics) - private val eventCallbacksMap = mutableMapOf() + val sharedPreferences: SharedPreferences + internal val storageV2: AndroidStorageV2 + + init { + val sharedPreferencesFile = "${getPrefix()}-$storageKey" + sharedPreferences = context.getSharedPreferences(sharedPreferencesFile, Context.MODE_PRIVATE) + val storageDirectory = context.getDir(getDir(), Context.MODE_PRIVATE) + storageV2 = AndroidStorageV2( + storageKey, + logger, + sharedPreferences, + storageDirectory, + diagnostics, + ) + } override suspend fun writeEvent(event: BaseEvent) { - eventsFile.storeEvent(JSONUtil.eventToString(event)) - event.callback?.let { callback -> - event.insertId?.let { - eventCallbacksMap.put(it, callback) - } - } + storageV2.writeEvent(event) } override suspend fun write( key: Storage.Constants, value: String, ) { - sharedPreferences.edit().putString(key.rawVal, value).apply() + storageV2.write(key, value) } override suspend fun remove(key: Storage.Constants) { - sharedPreferences.edit().remove(key.rawVal).apply() + storageV2.remove(key) } override suspend fun rollover() { - eventsFile.rollover() + storageV2.rollover() } override fun read(key: Storage.Constants): String? { - return sharedPreferences.getString(key.rawVal, null) + return storageV2.read(key) } override fun readEventsContent(): List { - return eventsFile.read() + return storageV2.readEventsContent() } override fun releaseFile(filePath: String) { - eventsFile.release(filePath) + storageV2.releaseFile(filePath) } override suspend fun getEventsString(content: Any): String { - return eventsFile.getEventString(content as String) + return storageV2.getEventsString(content) } override fun getResponseHandler( @@ -85,33 +84,26 @@ class AndroidStorage( scope: CoroutineScope, dispatcher: CoroutineDispatcher, ): ResponseHandler { - return FileResponseHandler( - this, - eventPipeline, - configuration, - scope, - dispatcher, - logger, - ) + return storageV2.getResponseHandler(eventPipeline, configuration, scope, dispatcher) } override fun removeFile(filePath: String): Boolean { - return eventsFile.remove(filePath) + return storageV2.removeFile(filePath) } override fun getEventCallback(insertId: String): EventCallBack? { - return eventCallbacksMap[insertId] + return storageV2.getEventCallback(insertId) } override fun removeEventCallback(insertId: String) { - eventCallbacksMap.remove(insertId) + return storageV2.removeEventCallback(insertId) } override fun splitEventFile( filePath: String, events: JSONArray, ) { - eventsFile.splitFile(filePath, events) + return storageV2.splitEventFile(filePath, events) } private fun getPrefix(): String { @@ -126,18 +118,4 @@ class AndroidStorage( } } -class AndroidStorageProvider : StorageProvider { - override fun getStorage( - amplitude: Amplitude, - prefix: String?, - ): Storage { - val configuration = amplitude.configuration as com.amplitude.android.Configuration - return AndroidStorage( - configuration.context, - configuration.instanceName, - configuration.loggerProvider.getLogger(amplitude), - prefix, - amplitude.diagnostics, - ) - } -} + diff --git a/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt b/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt index 967b4d54..e058e6dc 100644 --- a/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt +++ b/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt @@ -28,6 +28,7 @@ import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import java.io.File @ExperimentalCoroutinesApi class AmplitudeSessionTest { @@ -51,7 +52,9 @@ class AmplitudeSessionTest { val configuration = IdentityConfiguration( instanceName, - identityStorageProvider = IMIdentityStorageProvider() + identityStorageProvider = IMIdentityStorageProvider(), + storageDirectory = File("/tmp/amplitude-kotlin-identity-test"), + fileName = "identity", ) IdentityContainer.getInstance(configuration) } diff --git a/android/src/test/java/com/amplitude/android/migration/StorageKeyMigrationTest.kt b/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt similarity index 95% rename from android/src/test/java/com/amplitude/android/migration/StorageKeyMigrationTest.kt rename to android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt index 2ca0a5ff..87edea2a 100644 --- a/android/src/test/java/com/amplitude/android/migration/StorageKeyMigrationTest.kt +++ b/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt @@ -16,7 +16,7 @@ import java.io.File import java.util.UUID @RunWith(RobolectricTestRunner::class) -class StorageKeyMigrationTest { +class AndroidStorageMigrationTest { private val testDiagnostics = Diagnostics() @Test @@ -46,7 +46,7 @@ class StorageKeyMigrationTest { Assertions.assertNull(destinationLastEventId) Assertions.assertEquals(-1, destinationFileIndex) - val migration = StorageKeyMigration(source, destination, logger) + val migration = AndroidStorageMigration(source.storageV2, destination.storageV2, logger) runBlocking { migration.execute() } @@ -98,7 +98,7 @@ class StorageKeyMigrationTest { var destinationEventFiles = destination.readEventsContent() as List Assertions.assertEquals(0, destinationEventFiles.size) - val migration = StorageKeyMigration(source, destination, logger) + val migration = AndroidStorageMigration(source.storageV2, destination.storageV2, logger) runBlocking { migration.execute() } @@ -131,7 +131,7 @@ class StorageKeyMigrationTest { Assertions.assertNull(destinationLastEventTime) Assertions.assertNull(destinationLastEventId) - val migration = StorageKeyMigration(source, destination, logger) + val migration = AndroidStorageMigration(source.storageV2, destination.storageV2, logger) runBlocking { migration.execute() } @@ -178,7 +178,7 @@ class StorageKeyMigrationTest { val destinationFileSizes = destinationEventFiles.map { File(it).length() } - val migration = StorageKeyMigration(source, destination, logger) + val migration = AndroidStorageMigration(source.storageV2, destination.storageV2, logger) runBlocking { migration.execute() } diff --git a/core/src/main/java/com/amplitude/core/Amplitude.kt b/core/src/main/java/com/amplitude/core/Amplitude.kt index 9b4edf86..23d00370 100644 --- a/core/src/main/java/com/amplitude/core/Amplitude.kt +++ b/core/src/main/java/com/amplitude/core/Amplitude.kt @@ -32,6 +32,7 @@ import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.asCoroutineDispatcher import kotlinx.coroutines.async import kotlinx.coroutines.launch +import java.io.File import java.util.UUID import java.util.concurrent.Executors @@ -85,6 +86,8 @@ open class Amplitude internal constructor( apiKey = configuration.apiKey, identityStorageProvider = configuration.identityStorageProvider, logger = logger, + storageDirectory = File("/tmp/amplitude-identity/${configuration.instanceName}"), + fileName = "amplitude-identify-${configuration.instanceName}", ) } diff --git a/core/src/main/java/com/amplitude/core/utilities/FileStorage.kt b/core/src/main/java/com/amplitude/core/utilities/FileStorage.kt index 6878d62f..ce5752de 100644 --- a/core/src/main/java/com/amplitude/core/utilities/FileStorage.kt +++ b/core/src/main/java/com/amplitude/core/utilities/FileStorage.kt @@ -27,7 +27,8 @@ class FileStorage( private val storageDirectory = File("/tmp/${getPrefix()}/$storageKey") private val storageDirectoryEvents = File(storageDirectory, "events") - private val propertiesFile = PropertiesFile(storageDirectory, storageKey, getPrefix(), null) + private val propertiesFile = + PropertiesFile(storageDirectory, "${getPrefix()}-$storageKey", null) private val eventsFile = EventsFileManager(storageDirectoryEvents, storageKey, propertiesFile, logger, diagnostics) private val eventCallbacksMap = mutableMapOf() diff --git a/core/src/test/kotlin/com/amplitude/core/utilities/EventsFileManagerTest.kt b/core/src/test/kotlin/com/amplitude/core/utilities/EventsFileManagerTest.kt index 7f7ab1f6..3c11aea6 100644 --- a/core/src/test/kotlin/com/amplitude/core/utilities/EventsFileManagerTest.kt +++ b/core/src/test/kotlin/com/amplitude/core/utilities/EventsFileManagerTest.kt @@ -22,7 +22,7 @@ class EventsFileManagerTest { fun `test store event and read`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -67,7 +67,7 @@ class EventsFileManagerTest { fun `rollover should finish current non-empty temp file`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -98,7 +98,7 @@ class EventsFileManagerTest { fun `rollover should ignore current empty temp file`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -112,7 +112,7 @@ class EventsFileManagerTest { fun `remove should delete a file`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -130,7 +130,7 @@ class EventsFileManagerTest { fun `test split`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -168,7 +168,7 @@ class EventsFileManagerTest { file0.writeText("{\"eventType\":\"test1\"}\u0000{\"eventType\":\"test2\"}\u0000") val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -188,7 +188,7 @@ class EventsFileManagerTest { file0.writeText("{\"eventType\":\"test1\"}\u0000{\"eventType\":\"test2\"}\u0000{\"eventType\":\"test3\"\u0000") val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val diagnostics = Diagnostics() val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, diagnostics) @@ -208,7 +208,7 @@ class EventsFileManagerTest { fun `verify delimiter in event names`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -230,7 +230,7 @@ class EventsFileManagerTest { createEarlierVersionEventFiles() val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) val filePaths = eventsFileManager.read() @@ -277,7 +277,7 @@ class EventsFileManagerTest { file.writeText("{\"eventType\":\"test15\"},{\"eventType\":\"test16\"}]") val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -303,7 +303,7 @@ class EventsFileManagerTest { file.writeText("{\"eventType\":\"test15\"},{\"eventType\":\"test16\\nsuffix\"}]") val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -321,7 +321,7 @@ class EventsFileManagerTest { fun `concurrent writes to the same event file manager instance`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) runBlocking { @@ -363,7 +363,7 @@ class EventsFileManagerTest { fun `concurrent write from multiple threads`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) for (i in 0..100) { @@ -394,8 +394,8 @@ class EventsFileManagerTest { fun `concurrent write to two instances with same configuration`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile1 = PropertiesFile(tempDir, storageKey, "test-prefix", logger) - val propertiesFile2 = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile1 = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) + val propertiesFile2 = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) val eventsFileManager1 = EventsFileManager(tempDir, storageKey, propertiesFile1, logger, testDiagnostics) val eventsFileManager2 = @@ -446,7 +446,7 @@ class EventsFileManagerTest { fun `concurrent write from multiple threads on multiple instances`() { val logger = ConsoleLogger() val storageKey = "storageKey" - val propertiesFile = PropertiesFile(tempDir, storageKey, "test-prefix", logger) + val propertiesFile = PropertiesFile(tempDir, "test-prefix-$storageKey", logger) for (i in 0..100) { val eventsFileManager = EventsFileManager(tempDir, storageKey, propertiesFile, logger, testDiagnostics) diff --git a/id/src/main/java/com/amplitude/id/FileIdentityStorage.kt b/id/src/main/java/com/amplitude/id/FileIdentityStorage.kt index 00f64d2f..cf8caa50 100644 --- a/id/src/main/java/com/amplitude/id/FileIdentityStorage.kt +++ b/id/src/main/java/com/amplitude/id/FileIdentityStorage.kt @@ -8,7 +8,6 @@ class FileIdentityStorage(val configuration: IdentityConfiguration) : IdentitySt private val propertiesFile: PropertiesFile companion object { - const val STORAGE_PREFIX = "amplitude-identity" const val USER_ID_KEY = "user_id" const val DEVICE_ID_KEY = "device_id" const val API_KEY = "api_key" @@ -16,10 +15,9 @@ class FileIdentityStorage(val configuration: IdentityConfiguration) : IdentitySt } init { - val instanceName = configuration.instanceName - val storageDirectory = configuration.storageDirectory ?: File("/tmp/$STORAGE_PREFIX/$instanceName") + val storageDirectory = configuration.storageDirectory createDirectory(storageDirectory) - propertiesFile = PropertiesFile(storageDirectory, instanceName, STORAGE_PREFIX, configuration.logger) + propertiesFile = PropertiesFile(storageDirectory, configuration.fileName, configuration.logger) propertiesFile.load() safetyCheck() } @@ -64,6 +62,10 @@ class FileIdentityStorage(val configuration: IdentityConfiguration) : IdentitySt val savedApiKey = propertiesFile.getString(apiKey, null) ?: return true return savedApiKey == configValue } + + override fun delete() { + propertiesFile.deletePropertiesFile() + } } class FileIdentityStorageProvider : IdentityStorageProvider { diff --git a/id/src/main/java/com/amplitude/id/IMIdentityStorage.kt b/id/src/main/java/com/amplitude/id/IMIdentityStorage.kt index 25e86938..bec3f8b6 100644 --- a/id/src/main/java/com/amplitude/id/IMIdentityStorage.kt +++ b/id/src/main/java/com/amplitude/id/IMIdentityStorage.kt @@ -18,6 +18,11 @@ class IMIdentityStorage : IdentityStorage { override fun saveDeviceId(deviceId: String?) { this.deviceId = deviceId } + + override fun delete() { + userId = null + deviceId = null + } } /** diff --git a/id/src/main/java/com/amplitude/id/IdentityConfiguration.kt b/id/src/main/java/com/amplitude/id/IdentityConfiguration.kt index 121674da..65c4b773 100644 --- a/id/src/main/java/com/amplitude/id/IdentityConfiguration.kt +++ b/id/src/main/java/com/amplitude/id/IdentityConfiguration.kt @@ -11,6 +11,7 @@ data class IdentityConfiguration( val apiKey: String? = null, val experimentApiKey: String? = null, val identityStorageProvider: IdentityStorageProvider, - val storageDirectory: File? = null, + val storageDirectory: File, + val fileName: String, val logger: Logger? = null ) diff --git a/id/src/main/java/com/amplitude/id/IdentityStorage.kt b/id/src/main/java/com/amplitude/id/IdentityStorage.kt index f4af8542..0ec7182f 100644 --- a/id/src/main/java/com/amplitude/id/IdentityStorage.kt +++ b/id/src/main/java/com/amplitude/id/IdentityStorage.kt @@ -7,6 +7,8 @@ interface IdentityStorage { fun saveUserId(userId: String?) fun saveDeviceId(deviceId: String?) + + fun delete() } interface IdentityStorageProvider { diff --git a/id/src/main/java/com/amplitude/id/utilities/PropertiesFile.kt b/id/src/main/java/com/amplitude/id/utilities/PropertiesFile.kt index 88c5d2b7..b6750b36 100644 --- a/id/src/main/java/com/amplitude/id/utilities/PropertiesFile.kt +++ b/id/src/main/java/com/amplitude/id/utilities/PropertiesFile.kt @@ -6,11 +6,14 @@ import java.io.FileInputStream import java.io.FileOutputStream import java.util.Properties -class PropertiesFile(directory: File, key: String, prefix: String, logger: Logger?) : KeyValueStore { +class PropertiesFile( + directory: File, + fileNameWithoutExtension: String, + private val logger: Logger? +) : KeyValueStore { internal var underlyingProperties: Properties = Properties() - private val propertiesFileName = "$prefix-$key.properties" + private val propertiesFileName = "$fileNameWithoutExtension.properties" private val propertiesFile = File(directory, propertiesFileName) - private val logger = logger /** * Check if underlying file exists, and load properties if true @@ -74,6 +77,10 @@ class PropertiesFile(directory: File, key: String, prefix: String, logger: Logge save() return true } + + fun deletePropertiesFile() { + propertiesFile.delete() + } } /** From b6616c7fb4b1369d63ad5c3af72f49957d5354fb Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Wed, 25 Sep 2024 15:44:53 -0700 Subject: [PATCH 02/12] feat: Fixing tests --- .../migration/AndroidStorageMigration.kt | 16 +++-- .../storage/AndroidStorageContextV1.kt | 58 +++++++++++----- .../storage/AndroidStorageContextV2.kt | 66 +++++++++++++------ .../android/storage/AndroidStorageV2.kt | 10 ++- .../amplitude/android/utilities/AndroidKVS.kt | 4 ++ .../migration/AndroidStorageMigrationTest.kt | 49 ++++++++------ .../core/utilities/EventsFileManager.kt | 7 +- .../amplitude/id/utilities/PropertiesFile.kt | 6 ++ 8 files changed, 148 insertions(+), 68 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt b/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt index aa54e98f..4e5ebf7a 100644 --- a/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt +++ b/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt @@ -3,8 +3,8 @@ package com.amplitude.android.migration import com.amplitude.android.storage.AndroidStorageV2 import com.amplitude.common.Logger import com.amplitude.core.Storage -import com.amplitude.core.utilities.toBaseEvent -import org.json.JSONObject +import com.amplitude.core.utilities.toEvents +import org.json.JSONArray class AndroidStorageMigration( private val source: AndroidStorageV2, @@ -21,23 +21,27 @@ class AndroidStorageMigration( source.rollover() val sourceEventFiles = source.readEventsContent() as List if (sourceEventFiles.isEmpty()) { + source.cleanupMetadata() return } for (sourceEventFilePath in sourceEventFiles) { - val events = source.readEventsContent() as List + val events = source.getEventsString(sourceEventFilePath) var count = 0 - for (event in events) { + val baseEvents = JSONArray(events).toEvents() + for (event in baseEvents) { try { count++ - destination.writeEvent(JSONObject(event).toBaseEvent()) + destination.writeEvent(event) } catch (e: Exception) { logger.error("can't move event ($event) from file $sourceEventFilePath: ${e.message}") } } - logger.debug("Migrated $count/${events.size} events from $sourceEventFilePath") + logger.debug("Migrated $count/${baseEvents.size} events from $sourceEventFilePath") source.removeFile(sourceEventFilePath) } + source.cleanupMetadata() + destination.rollover() } catch (e: Exception) { logger.error("can't move event files: ${e.message}") } diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt index 682e7c06..0c12f652 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt @@ -1,14 +1,15 @@ package com.amplitude.android.storage import android.content.Context +import android.content.SharedPreferences import com.amplitude.android.Amplitude import com.amplitude.android.Configuration import com.amplitude.android.migration.AndroidStorageMigration import com.amplitude.android.migration.IdentityStorageMigration -import com.amplitude.android.utilities.AndroidStorage import com.amplitude.core.utilities.FileStorage import com.amplitude.id.FileIdentityStorage import com.amplitude.id.IdentityConfiguration +import java.io.File /** * Data is stored in storage in the following format @@ -30,7 +31,7 @@ internal class AndroidStorageContextV1( /** * Stores all event data in storage */ - private val eventsStorage: AndroidStorage + private val eventsStorage: AndroidStorageV2 /** * Stores all identity data in storage (user id, device id etc) @@ -40,26 +41,45 @@ internal class AndroidStorageContextV1( /** * Stores identifies intercepted by the SDK to reduce data sent over to the server */ - private val identifyInterceptStorage: AndroidStorage + private val identifyInterceptStorage: AndroidStorageV2 + + private val storageDirectories = mutableListOf() init { - eventsStorage = AndroidStorage( - configuration.context, - configuration.apiKey, - configuration.loggerProvider.getLogger(amplitude), - null, - amplitude.diagnostics + eventsStorage = createAndroidStorage( + configuration, + "amplitude-disk-queue", + "amplitude-android-${configuration.apiKey}" + ) + + identifyInterceptStorage = createAndroidStorage( + configuration, + "amplitude-identify-intercept-disk-queue", + "amplitude-identify-intercept-${configuration.apiKey}" ) + val identityConfig = generateIdentityConfiguration(amplitude, configuration) + storageDirectories.add(identityConfig.storageDirectory) identityStorage = FileIdentityStorage( - generateIdentityConfiguration(amplitude, configuration) + identityConfig ) + } + + private fun createAndroidStorage( + configuration: Configuration, + storageDirName: String, + sharedPreferencesName: String + ): AndroidStorageV2 { + val storageDirectory = configuration.context.getDir(storageDirName, Context.MODE_PRIVATE) + storageDirectories.add(storageDirectory) - identifyInterceptStorage = AndroidStorage( - configuration.context, - configuration.instanceName, + val sharedPreferences = + configuration.context.getSharedPreferences(sharedPreferencesName, Context.MODE_PRIVATE) + return AndroidStorageV2( + configuration.apiKey, configuration.loggerProvider.getLogger(amplitude), - "amplitude-identify-intercept", + sharedPreferences, + storageDirectory, amplitude.diagnostics ) } @@ -89,14 +109,20 @@ internal class AndroidStorageContextV1( identityMigration.execute() (amplitude.storage as? AndroidStorageV2)?.let { - val migrator = AndroidStorageMigration(eventsStorage.storageV2, it, amplitude.logger) + val migrator = AndroidStorageMigration(eventsStorage, it, amplitude.logger) migrator.execute() } (amplitude.identifyInterceptStorage as? AndroidStorageV2)?.let { val migrator = - AndroidStorageMigration(identifyInterceptStorage.storageV2, it, amplitude.logger) + AndroidStorageMigration(identifyInterceptStorage, it, amplitude.logger) migrator.execute() } + + for (dir in storageDirectories) { + if (dir.list()?.isEmpty() == true) { + dir.delete() + } + } } } \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt index 58b9e91d..8485f7ce 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt @@ -1,6 +1,7 @@ package com.amplitude.android.storage import android.content.Context +import android.content.SharedPreferences import com.amplitude.android.Amplitude import com.amplitude.android.Configuration import com.amplitude.android.migration.AndroidStorageMigration @@ -9,6 +10,7 @@ import com.amplitude.android.utilities.AndroidStorage import com.amplitude.core.utilities.FileStorage import com.amplitude.id.FileIdentityStorage import com.amplitude.id.IdentityConfiguration +import java.io.File /** * Data is stored in storage in the following format @@ -30,7 +32,7 @@ internal class AndroidStorageContextV2( /** * Stores all event data in storage */ - private val eventsStorage: AndroidStorage + private val eventsStorage: AndroidStorageV2 /** * Stores all identity data in storage (user id, device id etc) @@ -40,28 +42,24 @@ internal class AndroidStorageContextV2( /** * Stores identifies intercepted by the SDK to reduce data sent over to the server */ - private val identifyInterceptStorage: AndroidStorage + private val identifyInterceptStorage: AndroidStorageV2 + + private val storageDirectories = mutableListOf() init { - eventsStorage = AndroidStorage( - configuration.context, - configuration.instanceName, - configuration.loggerProvider.getLogger(amplitude), - null, - amplitude.diagnostics - ) + eventsStorage = createAndroidStorage( + configuration, + "amplitude-disk-queue", + "amplitude-android-${configuration.instanceName}") - identityStorage = FileIdentityStorage( - generateIdentityConfiguration(amplitude, configuration) - ) + identifyInterceptStorage = createAndroidStorage( + configuration, + "amplitude-identify-intercept-disk-queue", + "amplitude-identify-intercept-${configuration.instanceName}") - identifyInterceptStorage = AndroidStorage( - configuration.context, - configuration.instanceName, - configuration.loggerProvider.getLogger(amplitude), - "amplitude-identify-intercept", - amplitude.diagnostics - ) + val identityConfiguration = generateIdentityConfiguration(amplitude, configuration) + storageDirectories.add(identityConfiguration.storageDirectory) + identityStorage = FileIdentityStorage(identityConfiguration) } private fun generateIdentityConfiguration( @@ -83,20 +81,46 @@ internal class AndroidStorageContextV2( ) } + + private fun createAndroidStorage( + configuration: Configuration, + storageDirName: String, + sharedPreferencesName: String + ): AndroidStorageV2 { + val storageDirectory = configuration.context.getDir(storageDirName, Context.MODE_PRIVATE) + storageDirectories.add(storageDirectory) + + val sharedPreferences = + configuration.context.getSharedPreferences(sharedPreferencesName, Context.MODE_PRIVATE) + return AndroidStorageV2( + configuration.instanceName, + configuration.loggerProvider.getLogger(amplitude), + sharedPreferences, + storageDirectory, + amplitude.diagnostics + ) + } + suspend fun migrateToLatestVersion() { val identityMigration = IdentityStorageMigration(identityStorage, amplitude.identityStorage, amplitude.logger) identityMigration.execute() (amplitude.storage as? AndroidStorageV2)?.let { - val migrator = AndroidStorageMigration(eventsStorage.storageV2, it, amplitude.logger) + val migrator = AndroidStorageMigration(eventsStorage, it, amplitude.logger) migrator.execute() } (amplitude.identifyInterceptStorage as? AndroidStorageV2)?.let { val migrator = - AndroidStorageMigration(identifyInterceptStorage.storageV2, it, amplitude.logger) + AndroidStorageMigration(identifyInterceptStorage, it, amplitude.logger) migrator.execute() } + + for (dir in storageDirectories) { + if (dir.list()?.isEmpty() == true) { + dir.delete() + } + } } } \ No newline at end of file diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt index a09b15e9..e06aaf3e 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt @@ -26,7 +26,7 @@ class AndroidStorageV2( /** * A generic key to differentiate multiple storage instances. */ - val storageKey: String, + storageKey: String, private val logger: Logger, /** * A place where the storage stores some metadata to manage this storage @@ -123,6 +123,10 @@ class AndroidStorageV2( ) { eventsFile.splitFile(filePath, events) } + + fun cleanupMetadata() { + eventsFile.cleanupMetadata() + } } class AndroidEventsStorageProviderV2 : StorageProvider { @@ -131,7 +135,7 @@ class AndroidEventsStorageProviderV2 : StorageProvider { prefix: String?, ): Storage { val configuration = amplitude.configuration as com.amplitude.android.Configuration - val sharedPreferencesName = "amplitude-events-${configuration.context.packageName}-${configuration.instanceName}" + val sharedPreferencesName = "amplitude-events-${configuration.instanceName}" val sharedPreferences = configuration.context.getSharedPreferences(sharedPreferencesName, Context.MODE_PRIVATE) return AndroidStorageV2( configuration.instanceName, @@ -150,7 +154,7 @@ class AndroidIdentifyInterceptStorageProviderV2 : StorageProvider { ): Storage { val configuration = amplitude.configuration as com.amplitude.android.Configuration val sharedPreferences = configuration.context.getSharedPreferences( - "amplitude-id-intercept-${configuration.context.packageName}-${configuration.instanceName}", + "amplitude-identify-intercept-${configuration.instanceName}", Context.MODE_PRIVATE ) return AndroidStorageV2( diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidKVS.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidKVS.kt index 901e6de9..85b40414 100644 --- a/android/src/main/java/com/amplitude/android/utilities/AndroidKVS.kt +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidKVS.kt @@ -11,4 +11,8 @@ class AndroidKVS(private val sharedPreferences: SharedPreferences) : KeyValueSto override fun putLong(key: String, value: Long): Boolean { return sharedPreferences.edit().putLong(key, value).commit() } + + override fun deleteKey(key: String) { + sharedPreferences.edit().remove(key).commit() + } } diff --git a/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt b/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt index 87edea2a..8b0dca69 100644 --- a/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt +++ b/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt @@ -7,7 +7,10 @@ import com.amplitude.common.jvm.ConsoleLogger import com.amplitude.core.Storage import com.amplitude.core.events.BaseEvent import com.amplitude.core.utilities.Diagnostics +import com.amplitude.core.utilities.toEvents import kotlinx.coroutines.runBlocking +import org.json.JSONArray +import org.junit.Assert import org.junit.Test import org.junit.jupiter.api.Assertions import org.junit.runner.RunWith @@ -64,12 +67,10 @@ class AndroidStorageMigrationTest { destinationPreviousSessionId = destination.read(Storage.Constants.PREVIOUS_SESSION_ID) destinationLastEventTime = destination.read(Storage.Constants.LAST_EVENT_TIME) destinationLastEventId = destination.read(Storage.Constants.LAST_EVENT_ID) - destinationFileIndex = destination.sharedPreferences.getLong(destinationFileIndexKey, -1) Assertions.assertEquals("123", destinationPreviousSessionId) Assertions.assertEquals("456", destinationLastEventTime) Assertions.assertEquals("789", destinationLastEventId) - Assertions.assertEquals(1234567, destinationFileIndex) } @Test @@ -93,8 +94,6 @@ class AndroidStorageMigrationTest { var sourceEventFiles = source.readEventsContent() as List Assertions.assertEquals(3, sourceEventFiles.size) - val sourceFileSizes = sourceEventFiles.map { File(it).length() } - var destinationEventFiles = destination.readEventsContent() as List Assertions.assertEquals(0, destinationEventFiles.size) @@ -106,12 +105,13 @@ class AndroidStorageMigrationTest { sourceEventFiles = source.readEventsContent() as List Assertions.assertEquals(0, sourceEventFiles.size) - destinationEventFiles = destination.readEventsContent() as List - Assertions.assertEquals(3, destinationEventFiles.size) - - for ((index, destinationEventFile) in destinationEventFiles.withIndex()) { - val fileSize = File(destinationEventFile).length() - Assertions.assertEquals(sourceFileSizes[index], fileSize) + runBlocking { + val events = getEventsFromStorage(destination) + Assertions.assertEquals(4, events.size) + Assert.assertEquals("event-1", events[0].eventType) + Assert.assertEquals("event-22", events[1].eventType) + Assert.assertEquals("event-333", events[2].eventType) + Assert.assertEquals("event-4444", events[3].eventType) } } @@ -166,8 +166,6 @@ class AndroidStorageMigrationTest { val sourceEventFiles = source.readEventsContent() as List Assertions.assertEquals(2, sourceEventFiles.size) - val sourceFileSizes = sourceEventFiles.map { File(it).length() } - runBlocking { destination.writeEvent(createEvent(333)) destination.rollover() @@ -176,20 +174,29 @@ class AndroidStorageMigrationTest { var destinationEventFiles = destination.readEventsContent() as List Assertions.assertEquals(1, destinationEventFiles.size) - val destinationFileSizes = destinationEventFiles.map { File(it).length() } - val migration = AndroidStorageMigration(source.storageV2, destination.storageV2, logger) runBlocking { migration.execute() } - destinationEventFiles = destination.readEventsContent() as List - Assertions.assertEquals("-0", destinationEventFiles[0].substring(destinationEventFiles[0].length - 2)) - Assertions.assertTrue(destinationEventFiles[1].contains("-0-")) - Assertions.assertEquals("-1", destinationEventFiles[2].substring(destinationEventFiles[0].length - 2)) - Assertions.assertEquals(destinationFileSizes[0], File(destinationEventFiles[0]).length()) - Assertions.assertEquals(sourceFileSizes[0], File(destinationEventFiles[1]).length()) - Assertions.assertEquals(sourceFileSizes[1], File(destinationEventFiles[2]).length()) + runBlocking { + val events = getEventsFromStorage(destination) + Assertions.assertEquals(3, events.size) + Assert.assertEquals("event-333", events[0].eventType) + Assert.assertEquals("event-1", events[1].eventType) + Assert.assertEquals("event-22", events[2].eventType) + } + + } + + private suspend fun getEventsFromStorage(storage: Storage): List { + val files = storage.readEventsContent() as List + val events = mutableListOf() + for (file in files) { + val content = JSONArray(storage.getEventsString(file)) + events.addAll(content.toEvents()) + } + return events } private fun createEvent(eventIndex: Int): BaseEvent { diff --git a/core/src/main/java/com/amplitude/core/utilities/EventsFileManager.kt b/core/src/main/java/com/amplitude/core/utilities/EventsFileManager.kt index 47e01167..26720789 100644 --- a/core/src/main/java/com/amplitude/core/utilities/EventsFileManager.kt +++ b/core/src/main/java/com/amplitude/core/utilities/EventsFileManager.kt @@ -28,7 +28,7 @@ class EventsFileManager( ) { private val fileIndexKey = "amplitude.events.file.index.$storageKey" private val storageVersionKey = "amplitude.events.file.version.$storageKey" - val filePathSet: MutableSet = Collections.newSetFromMap(ConcurrentHashMap()) + val filePathSet: MutableSet = Collections.newSetFromMap(ConcurrentHashMap()) val curFile: MutableMap = ConcurrentHashMap() companion object { @@ -201,6 +201,11 @@ class EventsFileManager( filePathSet.remove(filePath) } + fun cleanupMetadata() { + kvs.deleteKey(fileIndexKey) + kvs.deleteKey(storageVersionKey) + } + private fun finish(file: File?) { rename(file ?: return) incrementFileIndex() diff --git a/id/src/main/java/com/amplitude/id/utilities/PropertiesFile.kt b/id/src/main/java/com/amplitude/id/utilities/PropertiesFile.kt index b6750b36..c4c299bd 100644 --- a/id/src/main/java/com/amplitude/id/utilities/PropertiesFile.kt +++ b/id/src/main/java/com/amplitude/id/utilities/PropertiesFile.kt @@ -55,6 +55,11 @@ class PropertiesFile( return true } + override fun deleteKey(key: String) { + underlyingProperties.remove(key) + save() + } + fun putString(key: String, value: String): Boolean { underlyingProperties.setProperty(key, value) save() @@ -89,4 +94,5 @@ class PropertiesFile( interface KeyValueStore { fun getLong(key: String, defaultVal: Long): Long fun putLong(key: String, value: Long): Boolean + fun deleteKey(key: String) } From b2a7e541c1d289603ab2d4991578061455f75361 Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Wed, 25 Sep 2024 15:48:27 -0700 Subject: [PATCH 03/12] feat: Fixing tests --- .../com/amplitude/android/storage/AndroidStorageContextV2.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt index 8485f7ce..33baeec3 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt @@ -77,7 +77,7 @@ internal class AndroidStorageContextV2( identityStorageProvider = configuration.identityStorageProvider, storageDirectory = storageDirectory, logger = configuration.loggerProvider.getLogger(amplitude), - fileName = "amplitude-identify-${configuration.instanceName}" + fileName = "amplitude-identity-${configuration.instanceName}" ) } From 614ac34ba361766ac39b13ff7928d6b8e585fdf4 Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Wed, 25 Sep 2024 16:10:10 -0700 Subject: [PATCH 04/12] chore: minor fix --- android/src/main/java/com/amplitude/android/Amplitude.kt | 4 ---- 1 file changed, 4 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index 4435f862..f3612429 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -51,10 +51,6 @@ open class Amplitude( val migrationManager = MigrationManager(this) migrationManager.migrateOldStorage() - if ((this.configuration as Configuration).migrateLegacyData) { - val legacySdkStorageContext = LegacySdkStorageContext(this) - legacySdkStorageContext.migrateToLatestVersion() - } this.createIdentityContainer(identityConfiguration) if (this.configuration.offline != AndroidNetworkConnectivityCheckerPlugin.Disabled) { From df4ada86fe96b631022847681f69ea86e0a38660 Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Wed, 25 Sep 2024 16:16:34 -0700 Subject: [PATCH 05/12] chore: minor fix --- .../com/amplitude/android/storage/AndroidStorageContextV1.kt | 4 ++-- .../com/amplitude/android/storage/AndroidStorageContextV2.kt | 2 +- core/src/main/java/com/amplitude/core/Amplitude.kt | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt index 0c12f652..e190e7b2 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt @@ -14,7 +14,7 @@ import java.io.File /** * Data is stored in storage in the following format * /app_amplitude-kotlin-{api_key} - * /amplitude-identify-{api_key}.properties (this stores the user id, device id and api key) + * /amplitude-identity-{api_key}.properties (this stores the user id, device id and api key) * /app_amplitude-disk-queue (this stores the events) * /{instance_name}-0 * /{instance_name}-1.tmp @@ -99,7 +99,7 @@ internal class AndroidStorageContextV1( identityStorageProvider = configuration.identityStorageProvider, storageDirectory = storageDirectory, logger = configuration.loggerProvider.getLogger(amplitude), - fileName = "amplitude-identify-${configuration.apiKey}" + fileName = "amplitude-identity-${configuration.apiKey}" ) } diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt index 33baeec3..7333a099 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt @@ -15,7 +15,7 @@ import java.io.File /** * Data is stored in storage in the following format * /app_amplitude-kotlin-{instance_name} - * /amplitude-identify-{instance_name}.properties (this stores the user id, device id and api key) + * /amplitude-identity-{instance_name}.properties (this stores the user id, device id and api key) * /app_amplitude-disk-queue (this stores the events) * /{instance_name}-0 * /{instance_name}-1.tmp diff --git a/core/src/main/java/com/amplitude/core/Amplitude.kt b/core/src/main/java/com/amplitude/core/Amplitude.kt index 23d00370..e90bed03 100644 --- a/core/src/main/java/com/amplitude/core/Amplitude.kt +++ b/core/src/main/java/com/amplitude/core/Amplitude.kt @@ -87,7 +87,7 @@ open class Amplitude internal constructor( identityStorageProvider = configuration.identityStorageProvider, logger = logger, storageDirectory = File("/tmp/amplitude-identity/${configuration.instanceName}"), - fileName = "amplitude-identify-${configuration.instanceName}", + fileName = "amplitude-identity-${configuration.instanceName}", ) } From 19f98f3d509d06624e3463cbe67f4bd19171216f Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Wed, 25 Sep 2024 16:19:00 -0700 Subject: [PATCH 06/12] chore: fix lint issues --- .../src/main/java/com/amplitude/android/Amplitude.kt | 1 - .../android/migration/IdentityStorageMigration.kt | 2 +- .../amplitude/android/migration/MigrationManager.kt | 2 +- .../android/storage/AndroidStorageContextV1.kt | 3 +-- .../android/storage/AndroidStorageContextV2.kt | 11 +++++------ .../main/java/com/amplitude/id/FileIdentityStorage.kt | 1 - 6 files changed, 8 insertions(+), 12 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Amplitude.kt b/android/src/main/java/com/amplitude/android/Amplitude.kt index f3612429..f46b5b7d 100644 --- a/android/src/main/java/com/amplitude/android/Amplitude.kt +++ b/android/src/main/java/com/amplitude/android/Amplitude.kt @@ -8,7 +8,6 @@ import com.amplitude.android.plugins.AndroidContextPlugin import com.amplitude.android.plugins.AndroidLifecyclePlugin import com.amplitude.android.plugins.AndroidNetworkConnectivityCheckerPlugin import com.amplitude.android.storage.AndroidStorageContextV3 -import com.amplitude.android.storage.LegacySdkStorageContext import com.amplitude.core.Amplitude import com.amplitude.core.events.BaseEvent import com.amplitude.core.platform.plugins.AmplitudeDestination diff --git a/android/src/main/java/com/amplitude/android/migration/IdentityStorageMigration.kt b/android/src/main/java/com/amplitude/android/migration/IdentityStorageMigration.kt index b88c0745..11fcdc72 100644 --- a/android/src/main/java/com/amplitude/android/migration/IdentityStorageMigration.kt +++ b/android/src/main/java/com/amplitude/android/migration/IdentityStorageMigration.kt @@ -23,4 +23,4 @@ class IdentityStorageMigration( logger.error("Unable to migrate file identity storage: ${e.message}") } } -} \ No newline at end of file +} diff --git a/android/src/main/java/com/amplitude/android/migration/MigrationManager.kt b/android/src/main/java/com/amplitude/android/migration/MigrationManager.kt index a53973e9..b5d23b99 100644 --- a/android/src/main/java/com/amplitude/android/migration/MigrationManager.kt +++ b/android/src/main/java/com/amplitude/android/migration/MigrationManager.kt @@ -52,4 +52,4 @@ internal class MigrationManager(private val amplitude: Amplitude) { logger.error("Failed to migrate storage: ${ex.message}") } } -} \ No newline at end of file +} diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt index e190e7b2..f77700b6 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt @@ -1,7 +1,6 @@ package com.amplitude.android.storage import android.content.Context -import android.content.SharedPreferences import com.amplitude.android.Amplitude import com.amplitude.android.Configuration import com.amplitude.android.migration.AndroidStorageMigration @@ -125,4 +124,4 @@ internal class AndroidStorageContextV1( } } } -} \ No newline at end of file +} diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt index 7333a099..7795b451 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt @@ -1,12 +1,10 @@ package com.amplitude.android.storage import android.content.Context -import android.content.SharedPreferences import com.amplitude.android.Amplitude import com.amplitude.android.Configuration import com.amplitude.android.migration.AndroidStorageMigration import com.amplitude.android.migration.IdentityStorageMigration -import com.amplitude.android.utilities.AndroidStorage import com.amplitude.core.utilities.FileStorage import com.amplitude.id.FileIdentityStorage import com.amplitude.id.IdentityConfiguration @@ -50,12 +48,14 @@ internal class AndroidStorageContextV2( eventsStorage = createAndroidStorage( configuration, "amplitude-disk-queue", - "amplitude-android-${configuration.instanceName}") + "amplitude-android-${configuration.instanceName}" + ) identifyInterceptStorage = createAndroidStorage( configuration, "amplitude-identify-intercept-disk-queue", - "amplitude-identify-intercept-${configuration.instanceName}") + "amplitude-identify-intercept-${configuration.instanceName}" + ) val identityConfiguration = generateIdentityConfiguration(amplitude, configuration) storageDirectories.add(identityConfiguration.storageDirectory) @@ -81,7 +81,6 @@ internal class AndroidStorageContextV2( ) } - private fun createAndroidStorage( configuration: Configuration, storageDirName: String, @@ -123,4 +122,4 @@ internal class AndroidStorageContextV2( } } } -} \ No newline at end of file +} diff --git a/id/src/main/java/com/amplitude/id/FileIdentityStorage.kt b/id/src/main/java/com/amplitude/id/FileIdentityStorage.kt index cf8caa50..d535af08 100644 --- a/id/src/main/java/com/amplitude/id/FileIdentityStorage.kt +++ b/id/src/main/java/com/amplitude/id/FileIdentityStorage.kt @@ -2,7 +2,6 @@ package com.amplitude.id import com.amplitude.id.utilities.PropertiesFile import com.amplitude.id.utilities.createDirectory -import java.io.File class FileIdentityStorage(val configuration: IdentityConfiguration) : IdentityStorage { private val propertiesFile: PropertiesFile From e36222c9778d496e46bc1343fe26a9c80850abcb Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Wed, 25 Sep 2024 16:24:31 -0700 Subject: [PATCH 07/12] chore: fix lint issues --- .../com/amplitude/android/storage/AndroidStorageContextV3.kt | 2 +- .../java/com/amplitude/android/storage/AndroidStorageV2.kt | 2 -- .../com/amplitude/android/storage/LegacySdkStorageContext.kt | 2 +- .../main/java/com/amplitude/android/storage/StorageVersion.kt | 4 ++-- .../java/com/amplitude/android/utilities/AndroidStorage.kt | 2 -- 5 files changed, 4 insertions(+), 8 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV3.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV3.kt index 5fc1f143..353827bf 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV3.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV3.kt @@ -53,4 +53,4 @@ internal object AndroidStorageContextV3 { fun getIdentityStorageFileName(): String { return "identity" } -} \ No newline at end of file +} diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt index e06aaf3e..b85c6717 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageV2.kt @@ -166,5 +166,3 @@ class AndroidIdentifyInterceptStorageProviderV2 : StorageProvider { ) } } - - diff --git a/android/src/main/java/com/amplitude/android/storage/LegacySdkStorageContext.kt b/android/src/main/java/com/amplitude/android/storage/LegacySdkStorageContext.kt index 843d0fa8..32e91227 100644 --- a/android/src/main/java/com/amplitude/android/storage/LegacySdkStorageContext.kt +++ b/android/src/main/java/com/amplitude/android/storage/LegacySdkStorageContext.kt @@ -12,4 +12,4 @@ internal class LegacySdkStorageContext(val amplitude: Amplitude) { val remnantDataMigration = RemnantDataMigration(amplitude, databaseStorage) remnantDataMigration.execute() } -} \ No newline at end of file +} diff --git a/android/src/main/java/com/amplitude/android/storage/StorageVersion.kt b/android/src/main/java/com/amplitude/android/storage/StorageVersion.kt index 260d28d6..d195935a 100644 --- a/android/src/main/java/com/amplitude/android/storage/StorageVersion.kt +++ b/android/src/main/java/com/amplitude/android/storage/StorageVersion.kt @@ -1,5 +1,5 @@ package com.amplitude.android.storage -enum class StorageVersion(val rawValue:Int) { +enum class StorageVersion(val rawValue: Int) { V3(3), -} \ No newline at end of file +} diff --git a/android/src/main/java/com/amplitude/android/utilities/AndroidStorage.kt b/android/src/main/java/com/amplitude/android/utilities/AndroidStorage.kt index 803306b7..b23814b9 100644 --- a/android/src/main/java/com/amplitude/android/utilities/AndroidStorage.kt +++ b/android/src/main/java/com/amplitude/android/utilities/AndroidStorage.kt @@ -117,5 +117,3 @@ class AndroidStorage( return "amplitude-disk-queue" } } - - From c4d112a092bdad9e58ba72bf15fbeb9ccb75c048 Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Wed, 25 Sep 2024 17:10:47 -0700 Subject: [PATCH 08/12] chore: fix lint issues --- .../com/amplitude/android/storage/AndroidStorageContextV1.kt | 4 ++-- .../android/migration/AndroidStorageMigrationTest.kt | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt index f77700b6..52533573 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt @@ -88,7 +88,7 @@ internal class AndroidStorageContextV1( configuration: Configuration ): IdentityConfiguration { val storageDirectory = configuration.context.getDir( - "${FileStorage.STORAGE_PREFIX}-${configuration.apiKey}", + "${FileStorage.STORAGE_PREFIX}-${configuration.instanceName}", Context.MODE_PRIVATE ) @@ -98,7 +98,7 @@ internal class AndroidStorageContextV1( identityStorageProvider = configuration.identityStorageProvider, storageDirectory = storageDirectory, logger = configuration.loggerProvider.getLogger(amplitude), - fileName = "amplitude-identity-${configuration.apiKey}" + fileName = "amplitude-identity-${configuration.instanceName}" ) } diff --git a/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt b/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt index 8b0dca69..bb41016f 100644 --- a/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt +++ b/android/src/test/java/com/amplitude/android/migration/AndroidStorageMigrationTest.kt @@ -15,7 +15,6 @@ import org.junit.Test import org.junit.jupiter.api.Assertions import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner -import java.io.File import java.util.UUID @RunWith(RobolectricTestRunner::class) @@ -186,7 +185,6 @@ class AndroidStorageMigrationTest { Assert.assertEquals("event-1", events[1].eventType) Assert.assertEquals("event-22", events[2].eventType) } - } private suspend fun getEventsFromStorage(storage: Storage): List { From e1b5f3376ce0a8ccd2471be5382e818e79d0c41a Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Wed, 25 Sep 2024 21:08:00 -0700 Subject: [PATCH 09/12] chore: fix tests --- .../test/java/com/amplitude/android/AmplitudeSessionTest.kt | 3 ++- android/src/test/java/com/amplitude/android/AmplitudeTest.kt | 2 ++ .../amplitude/android/plugins/AndroidLifecyclePluginTest.kt | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt b/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt index e058e6dc..79fadbf4 100644 --- a/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt +++ b/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt @@ -70,7 +70,8 @@ class AmplitudeSessionTest { private fun createConfiguration(storageProvider: StorageProvider? = null, shouldTrackSessions: Boolean = true): Configuration { val context = mockk(relaxed = true) var connectivityManager = mockk(relaxed = true) - every { context!!.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager + every { context.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager + every { context.getDir(any(), any()) } returns File("/tmp/amplitude-kotlin-test") return Configuration( apiKey = "api-key", diff --git a/android/src/test/java/com/amplitude/android/AmplitudeTest.kt b/android/src/test/java/com/amplitude/android/AmplitudeTest.kt index dd72a8c0..cca6431e 100644 --- a/android/src/test/java/com/amplitude/android/AmplitudeTest.kt +++ b/android/src/test/java/com/amplitude/android/AmplitudeTest.kt @@ -31,6 +31,7 @@ import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Assertions import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test +import java.io.File import kotlin.concurrent.thread open class StubPlugin : EventPlugin { @@ -49,6 +50,7 @@ class AmplitudeTest { context = mockk(relaxed = true) connectivityManager = mockk(relaxed = true) every { context!!.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager + every { context!!.getDir(any(), any()) } returns File("/tmp/amplitude-kotlin-test") mockkStatic(AndroidLifecyclePlugin::class) diff --git a/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt b/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt index e80faa44..d4bc04ba 100644 --- a/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt +++ b/android/src/test/java/com/amplitude/android/plugins/AndroidLifecyclePluginTest.kt @@ -37,6 +37,7 @@ import org.junit.jupiter.api.Assertions import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config +import java.io.File @ExperimentalCoroutinesApi @RunWith(RobolectricTestRunner::class) @@ -88,6 +89,7 @@ class AndroidLifecyclePluginTest { connectivityManager = mockk(relaxed = true) every { mockedContext!!.getSystemService(Context.CONNECTIVITY_SERVICE) } returns connectivityManager + every { mockedContext!!.getDir(any(), any()) } returns File("/tmp/amplitude-kotlin-test") configuration = Configuration( apiKey = "api-key", From c3e62281e4acea136af49c36411dab1a9210700b Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Thu, 26 Sep 2024 11:28:38 -0700 Subject: [PATCH 10/12] chore: adding more tests --- .../migration/AndroidStorageMigration.kt | 4 +- .../android/migration/RemnantDataMigration.kt | 2 + .../storage/AndroidStorageContextV1.kt | 10 +- .../storage/AndroidStorageContextV2.kt | 27 +- .../android/migration/MigrationManagerTest.kt | 331 ++++++++++++++++++ .../com/amplitude/id/IdentityContainer.kt | 5 + 6 files changed, 361 insertions(+), 18 deletions(-) create mode 100644 android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt diff --git a/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt b/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt index 4e5ebf7a..04af2df6 100644 --- a/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt +++ b/android/src/main/java/com/amplitude/android/migration/AndroidStorageMigration.kt @@ -12,11 +12,11 @@ class AndroidStorageMigration( private val logger: Logger ) { suspend fun execute() { - moveSourceEventFilesToDestination() + moveEventsToDestination() moveSimpleValues() } - private suspend fun moveSourceEventFilesToDestination() { + private suspend fun moveEventsToDestination() { try { source.rollover() val sourceEventFiles = source.readEventsContent() as List diff --git a/android/src/main/java/com/amplitude/android/migration/RemnantDataMigration.kt b/android/src/main/java/com/amplitude/android/migration/RemnantDataMigration.kt index d2193d07..0fc039b6 100644 --- a/android/src/main/java/com/amplitude/android/migration/RemnantDataMigration.kt +++ b/android/src/main/java/com/amplitude/android/migration/RemnantDataMigration.kt @@ -35,6 +35,8 @@ class RemnantDataMigration(val amplitude: Amplitude, private val databaseStorage moveIdentifies() } moveEvents() + amplitude.storage.rollover() + amplitude.identifyInterceptStorage.rollover() } private fun moveDeviceAndUserId() { diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt index 52533573..bafc2dc6 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV1.kt @@ -30,17 +30,17 @@ internal class AndroidStorageContextV1( /** * Stores all event data in storage */ - private val eventsStorage: AndroidStorageV2 + val eventsStorage: AndroidStorageV2 /** * Stores all identity data in storage (user id, device id etc) */ - private val identityStorage: FileIdentityStorage + val identityStorage: FileIdentityStorage /** * Stores identifies intercepted by the SDK to reduce data sent over to the server */ - private val identifyInterceptStorage: AndroidStorageV2 + val identifyInterceptStorage: AndroidStorageV2 private val storageDirectories = mutableListOf() @@ -84,7 +84,7 @@ internal class AndroidStorageContextV1( } private fun generateIdentityConfiguration( - amplitude: Amplitude, + amplitude: Amplitude?, configuration: Configuration ): IdentityConfiguration { val storageDirectory = configuration.context.getDir( @@ -97,7 +97,7 @@ internal class AndroidStorageContextV1( apiKey = configuration.apiKey, identityStorageProvider = configuration.identityStorageProvider, storageDirectory = storageDirectory, - logger = configuration.loggerProvider.getLogger(amplitude), + logger = if (amplitude != null) configuration.loggerProvider.getLogger(amplitude) else null, fileName = "amplitude-identity-${configuration.instanceName}" ) } diff --git a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt index 7795b451..4096fceb 100644 --- a/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt +++ b/android/src/main/java/com/amplitude/android/storage/AndroidStorageContextV2.kt @@ -30,17 +30,17 @@ internal class AndroidStorageContextV2( /** * Stores all event data in storage */ - private val eventsStorage: AndroidStorageV2 + val eventsStorage: AndroidStorageV2 /** * Stores all identity data in storage (user id, device id etc) */ - private val identityStorage: FileIdentityStorage + val identityStorage: FileIdentityStorage /** * Stores identifies intercepted by the SDK to reduce data sent over to the server */ - private val identifyInterceptStorage: AndroidStorageV2 + val identifyInterceptStorage: AndroidStorageV2 private val storageDirectories = mutableListOf() @@ -105,15 +105,20 @@ internal class AndroidStorageContextV2( IdentityStorageMigration(identityStorage, amplitude.identityStorage, amplitude.logger) identityMigration.execute() - (amplitude.storage as? AndroidStorageV2)?.let { - val migrator = AndroidStorageMigration(eventsStorage, it, amplitude.logger) - migrator.execute() - } + if (amplitude.configuration.instanceName == com.amplitude.core.Configuration.DEFAULT_INSTANCE) { + // We only migrate events from the previous storage if instance name is the default instance + // When instance names are substrings of each other, we run into data access issues + // + (amplitude.storage as? AndroidStorageV2)?.let { + val migrator = AndroidStorageMigration(eventsStorage, it, amplitude.logger) + migrator.execute() + } - (amplitude.identifyInterceptStorage as? AndroidStorageV2)?.let { - val migrator = - AndroidStorageMigration(identifyInterceptStorage, it, amplitude.logger) - migrator.execute() + (amplitude.identifyInterceptStorage as? AndroidStorageV2)?.let { + val migrator = + AndroidStorageMigration(identifyInterceptStorage, it, amplitude.logger) + migrator.execute() + } } for (dir in storageDirectories) { diff --git a/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt b/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt new file mode 100644 index 00000000..ab580d98 --- /dev/null +++ b/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt @@ -0,0 +1,331 @@ +package com.amplitude.android.migration + +import android.content.Context +import androidx.test.core.app.ApplicationProvider +import com.amplitude.android.Amplitude +import com.amplitude.android.Configuration +import com.amplitude.android.storage.AndroidStorageContextV1 +import com.amplitude.android.storage.AndroidStorageContextV2 +import com.amplitude.core.Storage +import com.amplitude.core.events.BaseEvent +import com.amplitude.core.utilities.ConsoleLoggerProvider +import com.amplitude.core.utilities.toEvents +import com.amplitude.id.IdentityContainer +import kotlinx.coroutines.runBlocking +import org.json.JSONArray +import org.junit.Assert +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.io.File +import java.io.FileOutputStream +import java.io.InputStream +import java.util.UUID + +@RunWith(RobolectricTestRunner::class) +class MigrationManagerTest { + lateinit var context: Context + + private val legacyUserId: String = "android-kotlin-sample-user-legacy" + private val legacyDeviceId = "22833898-c487-4536-b213-40f207abdce0R" + + @Before + fun init() { + context = ApplicationProvider.getApplicationContext() + } + + @Test + fun `test migration from legacy SDK`() { + val databaseName = "legacy_v4.sqlite" + val instanceName = "test-instance" + val apiKey = "test-api-key" + val inputStream = javaClass.classLoader?.getResourceAsStream(databaseName)!! + val dbPath = context.getDatabasePath("com.amplitude.api_$instanceName") + copyStream(inputStream, dbPath) + + val amplitude = waitAndGetBuiltAmplitudeInstance(instanceName, apiKey) + runBlocking { + Assert.assertEquals(legacyDeviceId, amplitude.getDeviceId()) + Assert.assertEquals(legacyUserId, amplitude.getUserId()) + + val events = getEventsFromStorage(amplitude.storage) + Assert.assertEquals(4, events.size) + } + } + + @Test + fun `test migration from legacy SDK with different instance names should not migrate data`() { + val databaseName = "legacy_v4.sqlite" + val instanceName = "test-instance" + val apiKey = "test-api-key" + val inputStream = javaClass.classLoader?.getResourceAsStream(databaseName)!! + val dbPath = context.getDatabasePath("com.amplitude.api_$instanceName") + copyStream(inputStream, dbPath) + + val differentAmplitudeInstance = + waitAndGetBuiltAmplitudeInstance("different-instance name", apiKey) + runBlocking { + Assert.assertNotNull(legacyDeviceId, differentAmplitudeInstance.getDeviceId()) + Assert.assertNull(differentAmplitudeInstance.getUserId()) + + val events = getEventsFromStorage(differentAmplitudeInstance.storage) + Assert.assertEquals(0, events.size) + } + + val correctAmplitudeInstance = waitAndGetBuiltAmplitudeInstance(instanceName, apiKey) + runBlocking { + Assert.assertEquals(legacyDeviceId, correctAmplitudeInstance.getDeviceId()) + Assert.assertEquals(legacyUserId, correctAmplitudeInstance.getUserId()) + + val events = getEventsFromStorage(correctAmplitudeInstance.storage) + Assert.assertEquals(4, events.size) + } + } + + @Test + fun `test migration from api key based storage`() { + var amplitude = waitAndGetBuiltAmplitudeInstance("test-instance", "test-api-key") + cleanupMigrationVersionMarker(amplitude.configuration as Configuration) + + // Clear this because the next amplitude instance will just reuse the old instance of id container + IdentityContainer.clearInstanceCache() + + val storageContextV1 = + AndroidStorageContextV1(amplitude, amplitude.configuration as Configuration) + val events = listOf(getBaseEvent("test_event"), getBaseEvent("test_event2")) + val identifies = listOf( + getBaseEvent("\$identify", mutableMapOf("key1" to "value1", "key2" to "value2")), + getBaseEvent("\$identify", mutableMapOf("key1" to "value1", "key2" to "value2")), + ) + + // Populate legacy data + runBlocking { + storageContextV1.identityStorage.saveUserId(legacyUserId) + storageContextV1.identityStorage.saveDeviceId(legacyDeviceId) + events.forEach { + storageContextV1.eventsStorage.writeEvent(it) + } + identifies.forEach { + storageContextV1.identifyInterceptStorage.writeEvent(it) + } + } + + amplitude = waitAndGetBuiltAmplitudeInstance("test-instance", "test-api-key") + runBlocking { + val eventsFromStorage = getEventsFromStorage(amplitude.storage) + Assert.assertEquals(2, eventsFromStorage.size) + assertEvents(events, eventsFromStorage) + + val identifiesFromStorage = getEventsFromStorage(amplitude.identifyInterceptStorage) + Assert.assertEquals(2, identifiesFromStorage.size) + assertEvents(identifies, identifiesFromStorage) + + Assert.assertEquals(legacyUserId, amplitude.getUserId()) + Assert.assertEquals(legacyDeviceId, amplitude.getDeviceId()) + } + } + + @Test + fun `test migration from api key based storage with different instance name`() { + var amplitude = waitAndGetBuiltAmplitudeInstance("test-instance", "test-api-key") + cleanupMigrationVersionMarker(amplitude.configuration as Configuration) + + // Clear this because the next amplitude instance will just reuse the old instance of id container + IdentityContainer.clearInstanceCache() + + val storageContextV1 = + AndroidStorageContextV1(amplitude, amplitude.configuration as Configuration) + val events = listOf(getBaseEvent("test_event"), getBaseEvent("test_event2")) + val identifies = listOf( + getBaseEvent("\$identify", mutableMapOf("key1" to "value1", "key2" to "value2")), + getBaseEvent("\$identify", mutableMapOf("key1" to "value1", "key2" to "value2")), + ) + + // Populate legacy data + runBlocking { + storageContextV1.identityStorage.saveUserId(legacyUserId) + storageContextV1.identityStorage.saveDeviceId(legacyDeviceId) + events.forEach { + storageContextV1.eventsStorage.writeEvent(it) + } + identifies.forEach { + storageContextV1.identifyInterceptStorage.writeEvent(it) + } + } + + amplitude = waitAndGetBuiltAmplitudeInstance("test-instance-2", "test-api-key-2") + runBlocking { + val eventsFromStorage = getEventsFromStorage(amplitude.storage) + Assert.assertEquals(0, eventsFromStorage.size) + + val identifiesFromStorage = getEventsFromStorage(amplitude.identifyInterceptStorage) + Assert.assertEquals(0, identifiesFromStorage.size) + + Assert.assertNotEquals(legacyUserId, amplitude.getUserId()) + Assert.assertNotEquals(legacyDeviceId, amplitude.getDeviceId()) + } + } + + @Test + fun `test migration from instance name based storage`() { + var amplitude = waitAndGetBuiltAmplitudeInstance(null, "test-api-key") + cleanupMigrationVersionMarker(amplitude.configuration as Configuration) + + // Clear this because the next amplitude instance will just reuse the old instance of id container + IdentityContainer.clearInstanceCache() + + val storageContextV2 = + AndroidStorageContextV2(amplitude, amplitude.configuration as Configuration) + val events = listOf(getBaseEvent("test_event"), getBaseEvent("test_event2")) + val identifies = listOf( + getBaseEvent("\$identify", mutableMapOf("key1" to "value1", "key2" to "value2")), + getBaseEvent("\$identify", mutableMapOf("key1" to "value1", "key2" to "value2")), + ) + + // Populate legacy data + runBlocking { + storageContextV2.identityStorage.saveUserId(legacyUserId) + storageContextV2.identityStorage.saveDeviceId(legacyDeviceId) + events.forEach { + storageContextV2.eventsStorage.writeEvent(it) + } + identifies.forEach { + storageContextV2.identifyInterceptStorage.writeEvent(it) + } + } + + amplitude = waitAndGetBuiltAmplitudeInstance(null, "test-api-key") + runBlocking { + val eventsFromStorage = getEventsFromStorage(amplitude.storage) + Assert.assertEquals(2, eventsFromStorage.size) + assertEvents(events, eventsFromStorage) + + val identifiesFromStorage = getEventsFromStorage(amplitude.identifyInterceptStorage) + Assert.assertEquals(2, identifiesFromStorage.size) + assertEvents(identifies, identifiesFromStorage) + + Assert.assertEquals(legacyUserId, amplitude.getUserId()) + Assert.assertEquals(legacyDeviceId, amplitude.getDeviceId()) + } + } + + @Test + fun `test migration from instance name based storage for non default instances`() { + var amplitude = waitAndGetBuiltAmplitudeInstance("test-instance", "test-api-key") + cleanupMigrationVersionMarker(amplitude.configuration as Configuration) + + // Clear this because the next amplitude instance will just reuse the old instance of id container + IdentityContainer.clearInstanceCache() + + val storageContextV2 = + AndroidStorageContextV2(amplitude, amplitude.configuration as Configuration) + val events = listOf(getBaseEvent("test_event"), getBaseEvent("test_event2")) + val identifies = listOf( + getBaseEvent("\$identify", mutableMapOf("key1" to "value1", "key2" to "value2")), + getBaseEvent("\$identify", mutableMapOf("key1" to "value1", "key2" to "value2")), + ) + + // Populate legacy data + runBlocking { + storageContextV2.identityStorage.saveUserId(legacyUserId) + storageContextV2.identityStorage.saveDeviceId(legacyDeviceId) + events.forEach { + storageContextV2.eventsStorage.writeEvent(it) + } + identifies.forEach { + storageContextV2.identifyInterceptStorage.writeEvent(it) + } + } + + amplitude = waitAndGetBuiltAmplitudeInstance("test-instance", "test-api-key") + runBlocking { + // since this is a non default instance name, we shouldn't have migrated events and + // ident data + val eventsFromStorage = getEventsFromStorage(amplitude.storage) + Assert.assertEquals(0, eventsFromStorage.size) + + val identifiesFromStorage = getEventsFromStorage(amplitude.identifyInterceptStorage) + Assert.assertEquals(0, identifiesFromStorage.size) + + Assert.assertEquals(legacyUserId, amplitude.getUserId()) + Assert.assertEquals(legacyDeviceId, amplitude.getDeviceId()) + } + } + + private fun assertEvents(original: List, new: List) { + Assert.assertEquals(original.size, new.size) + for (i in original.indices) { + Assert.assertEquals(original[i].eventType, new[i].eventType) + Assert.assertEquals(original[i].insertId, new[i].insertId) + Assert.assertEquals(original[i].userProperties, new[i].userProperties) + } + } + + private fun getBaseEvent( + eventType: String, + userProperties: MutableMap = mutableMapOf() + ): BaseEvent { + return BaseEvent().apply { + this.eventType = eventType + this.insertId = UUID.randomUUID().toString() + this.userProperties = userProperties + } + } + + private fun waitAndGetBuiltAmplitudeInstance( + instanceName: String?, + apiKey: String, + ): Amplitude { + val amplitude = Amplitude( + generateConfiguration(instanceName, apiKey) + ) + runBlocking { + amplitude.isBuilt.await() + } + return amplitude + } + + private fun generateConfiguration( + instanceName: String?, + apiKey: String, + ): Configuration { + return Configuration( + apiKey, + context, + instanceName = instanceName ?: com.amplitude.core.Configuration.DEFAULT_INSTANCE, + loggerProvider = ConsoleLoggerProvider() + ) + } + + private fun copyStream(src: InputStream, dst: File) { + src.use { srcStream -> + FileOutputStream(dst).use { dstStream -> + // Transfer bytes from in to out + val buf = ByteArray(1024) + var len: Int + while (srcStream.read(buf).also { len = it } > 0) { + dstStream.write(buf, 0, len) + } + } + } + } + + private fun cleanupMigrationVersionMarker(configuration: Configuration) { + val sharedPreferences = configuration.context.getSharedPreferences( + "amplitude-android-${configuration.instanceName}", + Context.MODE_PRIVATE + ) + sharedPreferences.edit().remove("storage_version").apply() + } + + private suspend fun getEventsFromStorage(storage: Storage): List { + val files = storage.readEventsContent() as List + val events = mutableListOf() + for (file in files) { + val content = JSONArray(storage.getEventsString(file)) + events.addAll(content.toEvents()) + } + return events + } +} \ No newline at end of file diff --git a/id/src/main/java/com/amplitude/id/IdentityContainer.kt b/id/src/main/java/com/amplitude/id/IdentityContainer.kt index dd6dc315..e961a93f 100644 --- a/id/src/main/java/com/amplitude/id/IdentityContainer.kt +++ b/id/src/main/java/com/amplitude/id/IdentityContainer.kt @@ -22,6 +22,11 @@ class IdentityContainer private constructor(val configuration: IdentityConfigura } } } + + @JvmStatic + fun clearInstanceCache() { + instances.clear() + } } init { From 23408af4996e6020d744d5f8a6bddf3f2213fac9 Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Thu, 26 Sep 2024 11:36:23 -0700 Subject: [PATCH 11/12] chore: fix lint --- .../com/amplitude/android/migration/MigrationManagerTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt b/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt index ab580d98..952b438e 100644 --- a/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt +++ b/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt @@ -328,4 +328,4 @@ class MigrationManagerTest { } return events } -} \ No newline at end of file +} From e4b18e17f241834559e12889cd042aa503cf6bb4 Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Thu, 26 Sep 2024 11:44:37 -0700 Subject: [PATCH 12/12] chore: fix tests --- .../com/amplitude/android/migration/MigrationManagerTest.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt b/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt index 952b438e..dcd2ed2f 100644 --- a/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt +++ b/android/src/test/java/com/amplitude/android/migration/MigrationManagerTest.kt @@ -33,12 +33,13 @@ class MigrationManagerTest { @Before fun init() { context = ApplicationProvider.getApplicationContext() + IdentityContainer.clearInstanceCache() } @Test fun `test migration from legacy SDK`() { val databaseName = "legacy_v4.sqlite" - val instanceName = "test-instance" + val instanceName = "test-instance-legacy" val apiKey = "test-api-key" val inputStream = javaClass.classLoader?.getResourceAsStream(databaseName)!! val dbPath = context.getDatabasePath("com.amplitude.api_$instanceName")