diff --git a/android/src/main/java/com/amplitude/android/migration/DatabaseStorage.kt b/android/src/main/java/com/amplitude/android/migration/DatabaseStorage.kt index f3fb917a..a0dd66b8 100644 --- a/android/src/main/java/com/amplitude/android/migration/DatabaseStorage.kt +++ b/android/src/main/java/com/amplitude/android/migration/DatabaseStorage.kt @@ -5,6 +5,7 @@ import android.database.Cursor import android.database.sqlite.SQLiteDatabase import android.database.sqlite.SQLiteException import android.database.sqlite.SQLiteOpenHelper +import com.amplitude.common.Logger import com.amplitude.common.android.LogcatLogger import com.amplitude.core.Amplitude import com.amplitude.core.Configuration @@ -39,18 +40,25 @@ object DatabaseConstants { * The SDK doesn't need to write/read from local sqlite database. * This storage class is used for migrating events only. */ -class DatabaseStorage(context: Context, databaseName: String) : SQLiteOpenHelper( +class DatabaseStorage( + context: Context, + databaseName: String, + private val logger: Logger +) : SQLiteOpenHelper( context, databaseName, null, DatabaseConstants.DATABASE_VERSION ) { private var file: File = context.getDatabasePath(databaseName) + private var isValidDatabaseFile = true var currentDbVersion: Int = DatabaseConstants.DATABASE_VERSION private set override fun onCreate(db: SQLiteDatabase) { - throw NotImplementedError() + // File exists but it is not a legacy database for some reason. + this.isValidDatabaseFile = false + logger.error("Attempt to re-create existing legacy database file ${file.absolutePath}") } override fun onUpgrade(db: SQLiteDatabase?, oldVersion: Int, newVersion: Int) { @@ -121,6 +129,10 @@ class DatabaseStorage(context: Context, databaseName: String) : SQLiteOpenHelper var cursor: Cursor? = null try { val db = readableDatabase + if (!isValidDatabaseFile) { + return arrayListOf() + } + cursor = queryDb( db, table, @@ -220,6 +232,10 @@ class DatabaseStorage(context: Context, databaseName: String) : SQLiteOpenHelper var cursor: Cursor? = null try { val db = readableDatabase + if (!isValidDatabaseFile) { + return null + } + cursor = queryDb( db, table, @@ -304,7 +320,7 @@ object DatabaseStorageProvider { val databaseName = getDatabaseName(configuration.instanceName) var storage = instances[databaseName] if (storage == null) { - storage = DatabaseStorage(configuration.context, databaseName) + storage = DatabaseStorage(configuration.context, databaseName, configuration.loggerProvider.getLogger(amplitude)) instances[databaseName] = storage } diff --git a/android/src/test/java/com/amplitude/android/migration/DatabaseStorageTest.kt b/android/src/test/java/com/amplitude/android/migration/DatabaseStorageTest.kt index eff584ce..5067eeed 100644 --- a/android/src/test/java/com/amplitude/android/migration/DatabaseStorageTest.kt +++ b/android/src/test/java/com/amplitude/android/migration/DatabaseStorageTest.kt @@ -3,6 +3,7 @@ package com.amplitude.android.migration import android.content.Context import android.database.Cursor import android.database.sqlite.SQLiteDatabase +import com.amplitude.common.jvm.ConsoleLogger import io.mockk.every import io.mockk.mockk import io.mockk.mockkConstructor @@ -29,17 +30,10 @@ class DatabaseStorageTest { @BeforeEach fun setUp() { context = mockk(relaxed = true) - databaseStorage = DatabaseStorage(context!!, "") + databaseStorage = DatabaseStorage(context!!, "", ConsoleLogger()) db = mockk() } - @Test - fun databaseStorage_onCreate_throwsNotImplementedError() { - Assertions.assertThrows(NotImplementedError::class.java) { - databaseStorage?.onCreate(db!!) - } - } - @Test fun databaseStorage_onUpgrade_shouldSaveCurrentDbVersion() { Assertions.assertEquals(DatabaseConstants.DATABASE_VERSION, databaseStorage?.currentDbVersion) @@ -64,7 +58,7 @@ class DatabaseStorageTest { every { anyConstructed().put(any(), any()) } returns JSONObject() every { anyConstructed().get("event_id") } returnsMany listOf(1, 2) - val mockedDatabaseStorage = spyk(DatabaseStorage(context!!, ""), recordPrivateCalls = true) + val mockedDatabaseStorage = spyk(DatabaseStorage(context!!, "", ConsoleLogger()), recordPrivateCalls = true) every { mockedDatabaseStorage["queryDb"]( any(), @@ -86,7 +80,7 @@ class DatabaseStorageTest { @Test fun databaseStorage_removeEvent_returnsEventsContent() { - val mockedDatabaseStorage = spyk(DatabaseStorage(context!!, ""), recordPrivateCalls = true) + val mockedDatabaseStorage = spyk(DatabaseStorage(context!!, "", ConsoleLogger()), recordPrivateCalls = true) every { mockedDatabaseStorage.close() } answers { nothing } every { mockedDatabaseStorage.writableDatabase } returns db every { db!!.delete(any(), any(), arrayOf("2")) } returns 2 diff --git a/android/src/test/java/com/amplitude/android/migration/RemnantDataMigrationTest.kt b/android/src/test/java/com/amplitude/android/migration/RemnantDataMigrationTest.kt index ef7ca6b0..4d2b7d77 100644 --- a/android/src/test/java/com/amplitude/android/migration/RemnantDataMigrationTest.kt +++ b/android/src/test/java/com/amplitude/android/migration/RemnantDataMigrationTest.kt @@ -5,6 +5,7 @@ import androidx.test.core.app.ApplicationProvider import com.amplitude.android.Amplitude import com.amplitude.android.Configuration import com.amplitude.core.Storage +import com.amplitude.core.utilities.ConsoleLoggerProvider import kotlinx.coroutines.runBlocking import org.json.JSONArray import org.junit.Test @@ -37,6 +38,11 @@ class RemnantDataMigrationTest { checkLegacyDataMigration("legacy_v4.sqlite", 4, false) } + @Test + fun `not database file should not fail`() { + checkLegacyDataMigration("not_db_file", 0) + } + private fun checkLegacyDataMigration(legacyDbName: String, dbVersion: Int, migrateLegacyData: Boolean = true) { val context = ApplicationProvider.getApplicationContext() @@ -46,6 +52,7 @@ class RemnantDataMigrationTest { if (inputStream != null) { copyStream(inputStream, dbPath) } + val isValidDbFile = inputStream != null && legacyDbName != "not_db_file" val amplitude = Amplitude( Configuration( @@ -53,6 +60,7 @@ class RemnantDataMigrationTest { context, instanceName = instanceName, migrateLegacyData = migrateLegacyData, + loggerProvider = ConsoleLoggerProvider() ) ) @@ -63,7 +71,7 @@ class RemnantDataMigrationTest { amplitude.isBuilt.await() val identity = amplitude.idContainer.identityManager.getIdentity() - if (inputStream != null && migrateLegacyData) { + if (isValidDbFile && migrateLegacyData) { Assertions.assertEquals(deviceId, identity.deviceId) Assertions.assertEquals(userId, identity.userId) } else { @@ -74,7 +82,7 @@ class RemnantDataMigrationTest { amplitude.storage.rollover() amplitude.identifyInterceptStorage.rollover() - if (inputStream != null && migrateLegacyData) { + if (isValidDbFile && migrateLegacyData) { Assertions.assertEquals(1684219150343, amplitude.storage.read(Storage.Constants.PREVIOUS_SESSION_ID)?.toLongOrNull()) Assertions.assertEquals(1684219150344, amplitude.storage.read(Storage.Constants.LAST_EVENT_TIME)?.toLongOrNull()) Assertions.assertEquals(2, amplitude.storage.read(Storage.Constants.LAST_EVENT_ID)?.toLongOrNull()) @@ -85,7 +93,7 @@ class RemnantDataMigrationTest { } val eventsData = amplitude.storage.readEventsContent() - if (inputStream != null && migrateLegacyData) { + if (isValidDbFile && migrateLegacyData) { val jsonEvents = JSONArray() for (eventsPath in eventsData) { val eventsString = amplitude.storage.getEventsString(eventsPath) @@ -128,7 +136,7 @@ class RemnantDataMigrationTest { } val interceptedIdentifiesData = amplitude.identifyInterceptStorage.readEventsContent() - if (inputStream != null && dbVersion >= 4 && migrateLegacyData) { + if (isValidDbFile && dbVersion >= 4 && migrateLegacyData) { val jsonInterceptedIdentifies = JSONArray() for (eventsPath in interceptedIdentifiesData) { val eventsString = amplitude.storage.getEventsString(eventsPath) @@ -170,7 +178,7 @@ class RemnantDataMigrationTest { } // User/device id should not be cleaned. - if (inputStream != null) { + if (isValidDbFile) { Assertions.assertEquals(deviceId, databaseStorage.getValue(RemnantDataMigration.DEVICE_ID_KEY)) Assertions.assertEquals(userId, databaseStorage.getValue(RemnantDataMigration.USER_ID_KEY)) } else { diff --git a/android/src/test/resources/not_db_file b/android/src/test/resources/not_db_file new file mode 100644 index 00000000..286b9c9f --- /dev/null +++ b/android/src/test/resources/not_db_file @@ -0,0 +1 @@ +It is not a sqlite database. \ No newline at end of file