diff --git a/aws-storage-s3/build.gradle.kts b/aws-storage-s3/build.gradle.kts index 84aa231961..1d7ac240ff 100644 --- a/aws-storage-s3/build.gradle.kts +++ b/aws-storage-s3/build.gradle.kts @@ -42,6 +42,7 @@ dependencies { testImplementation(libs.test.mockk) testImplementation(libs.test.androidx.workmanager) testImplementation(libs.test.kotlin.coroutines) + testImplementation(libs.test.kotest.assertions) testImplementation(project(":aws-storage-s3")) androidTestImplementation(project(":testutils")) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt index d882146b43..6413cf7c45 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt @@ -15,11 +15,11 @@ package com.amplifyframework.storage.s3.transfer -import android.annotation.SuppressLint import android.content.ContentValues import android.content.Context import android.database.Cursor import android.net.Uri +import androidx.annotation.VisibleForTesting import aws.sdk.kotlin.services.s3.model.CompletedPart import aws.sdk.kotlin.services.s3.model.ObjectCannedAcl import com.amplifyframework.core.Amplify @@ -33,33 +33,33 @@ import java.io.File /** * SQlite database to store transfer records */ -@SuppressLint("VisibleForTests") internal class TransferDB private constructor(context: Context) { - private var transferDBHelper: TransferDBHelper = synchronized(this) { - TransferDBHelper(context) - } + private val transferDBHelper = TransferDBHelper(context) - private val logger = - Amplify.Logging.logger( - CategoryType.STORAGE, - AWSS3StoragePlugin.AWS_S3_STORAGE_LOG_NAMESPACE.format(this::class.java.simpleName) - ) + private val logger = Amplify.Logging.logger( + CategoryType.STORAGE, + AWSS3StoragePlugin.AWS_S3_STORAGE_LOG_NAMESPACE.format(this::class.java.simpleName) + ) companion object { private const val QUERY_PLACE_HOLDER_STRING = ",?" - private val instance: TransferDB? = null - @JvmStatic + @Volatile + @VisibleForTesting + var instance: TransferDB? = null + fun getInstance(context: Context): TransferDB { - return instance ?: TransferDB(context) + // Use double check locking for thread safety. It is important that there is never more than one instance + // of this class since it holds an SQLiteOpenHelper. + return instance ?: synchronized(this) { + instance ?: TransferDB(context).also { instance = it } + } } } fun closeDB() { - synchronized(this) { - transferDBHelper.close() - } + transferDBHelper.close() } /** diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDBHelper.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDBHelper.kt index f334bdeaab..211ea745e5 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDBHelper.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDBHelper.kt @@ -24,14 +24,16 @@ import android.database.sqlite.SQLiteOpenHelper import android.database.sqlite.SQLiteQueryBuilder import android.net.Uri import android.text.TextUtils -import androidx.annotation.VisibleForTesting import com.amplifyframework.core.Amplify import com.amplifyframework.core.category.CategoryType import com.amplifyframework.storage.s3.AWSS3StoragePlugin -@VisibleForTesting -internal class TransferDBHelper(private val context: Context) : - SQLiteOpenHelper(context, DATABASE_NAME, null, DATABASE_VERSION) { +internal class TransferDBHelper(private val context: Context) : SQLiteOpenHelper( + context, + DATABASE_NAME, + null, + DATABASE_VERSION +) { internal val contentUri: Uri private val uriMatcher: UriMatcher diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt index a06f91ba7d..8a4bd9cb30 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt @@ -43,7 +43,7 @@ import kotlin.math.min * download files. It inserts upload and download records into the database and * enqueue a WorkRequest for WorkManager to service the transfer */ -internal class TransferManager @JvmOverloads constructor( +internal class TransferManager( context: Context, s3: S3Client, private val pluginKey: String, @@ -51,7 +51,8 @@ internal class TransferManager @JvmOverloads constructor( ) { private val transferDB: TransferDB = TransferDB.getInstance(context) - val transferStatusUpdater: TransferStatusUpdater = TransferStatusUpdater.getInstance(context) + val transferStatusUpdater: TransferStatusUpdater = TransferStatusUpdater(transferDB) + private val logger = Amplify.Logging.logger( CategoryType.STORAGE, diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferStatusUpdater.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferStatusUpdater.kt index 704c2792c3..ec91195c25 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferStatusUpdater.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferStatusUpdater.kt @@ -15,7 +15,6 @@ package com.amplifyframework.storage.s3.transfer -import android.content.Context import android.os.Handler import android.os.Looper import com.amplifyframework.core.Amplify @@ -28,7 +27,7 @@ import java.util.concurrent.ConcurrentHashMap /** * Updates transfer status to observers and to local DB **/ -internal class TransferStatusUpdater private constructor( +internal class TransferStatusUpdater( private val transferDB: TransferDB ) { private val logger = @@ -64,13 +63,7 @@ internal class TransferStatusUpdater private constructor( } companion object { - internal const val TEMP_FILE_PREFIX = "aws-s3-d861b25a-1edf-11eb-adc1-0242ac120002" - - @JvmStatic - fun getInstance(context: Context): TransferStatusUpdater { - return TransferStatusUpdater(TransferDB.getInstance(context)) - } } @Synchronized diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/TransferDBTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/TransferDBTest.kt new file mode 100644 index 0000000000..af99857b0b --- /dev/null +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/TransferDBTest.kt @@ -0,0 +1,28 @@ +package com.amplifyframework.storage.s3.transfer + +import io.kotest.matchers.types.shouldBeSameInstanceAs +import org.junit.After +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment + +@RunWith(RobolectricTestRunner::class) +class TransferDBTest { + + @After + fun teardown() { + // Clear instance + TransferDB.instance = null + } + + @Test + fun `getInstance returns the same object`() { + val context = RuntimeEnvironment.getApplication() + + val db1 = TransferDB.getInstance(context) + val db2 = TransferDB.getInstance(context) + + db1 shouldBeSameInstanceAs db2 + } +}