Skip to content

Commit

Permalink
Introduce an Arcs Flag (diffbasedEntityInsertion) to determine whethe…
Browse files Browse the repository at this point in the history
…r to use the diffbased entity insertion approach or not.

PiperOrigin-RevId: 368110612
  • Loading branch information
Alice Johnson authored and arcs-c3po committed Apr 15, 2021
1 parent 1e254aa commit 799d127
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import androidx.lifecycle.LifecycleObserver
import arcs.core.storage.StorageKey
import arcs.core.storage.StorageKeyManager
import arcs.core.storage.database.Database
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseIdentifier
import arcs.core.storage.database.DatabaseManager
import arcs.core.storage.database.DatabasePerformanceStatistics.Snapshot
Expand All @@ -25,6 +26,7 @@ import arcs.core.util.guardedBy
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import java.util.concurrent.atomic.AtomicReference

/**
* [DatabaseManager] implementation which constructs [DatabaseImpl] instances for use on Android
Expand All @@ -34,13 +36,15 @@ import kotlinx.coroutines.sync.withLock
class AndroidSqliteDatabaseManager(
context: Context,
// Maximum size of the database file, if it surpasses this size, the database gets reset.
maxDbSizeBytes: Int? = null
maxDbSizeBytes: Int? = null,
databaseConfig: DatabaseConfig = DatabaseConfig()
) : DatabaseManager, LifecycleObserver {
private val context = context.applicationContext
private val mutex = Mutex()
private val dbCache by guardedBy(mutex, mutableMapOf<DatabaseIdentifier, DatabaseImpl>())
private val maxDbSize = maxDbSizeBytes ?: MAX_DB_SIZE_BYTES
override val registry = AndroidSqliteDatabaseRegistry(context)
override val databaseConfig: AtomicReference<DatabaseConfig> = AtomicReference(databaseConfig)

// TODO(b/174432505): Don't use the GLOBAL_INSTANCE, accept as a constructor param instead.
private val storageKeyManager = StorageKeyManager.GLOBAL_INSTANCE
Expand All @@ -58,7 +62,7 @@ class AndroidSqliteDatabaseManager(
val entry = registry.register(name, persistent)
return mutex.withLock {
dbCache[entry.name to entry.isPersistent]
?: DatabaseImpl(context, storageKeyManager, name, persistent)
?: DatabaseImpl(context, storageKeyManager, name, persistent, databaseConfig::get)
.also {
dbCache[entry.name to entry.isPersistent] = it
}
Expand Down
2 changes: 2 additions & 0 deletions java/arcs/android/storage/database/DatabaseImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import arcs.core.storage.StorageKeyManager
import arcs.core.storage.StorageKeyProtocol
import arcs.core.storage.database.Database
import arcs.core.storage.database.DatabaseClient
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseData
import arcs.core.storage.database.DatabaseOp
import arcs.core.storage.database.DatabasePerformanceStatistics
Expand Down Expand Up @@ -126,6 +127,7 @@ class DatabaseImpl(
private val storageKeyManager: StorageKeyManager,
databaseName: String,
persistent: Boolean = true,
@VisibleForTesting val databaseConfigGetter: () -> DatabaseConfig,
val onDatabaseClose: suspend () -> Unit = {}
) : Database, SQLiteOpenHelper(
context,
Expand Down
18 changes: 18 additions & 0 deletions java/arcs/core/storage/database/DatabaseManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ package arcs.core.storage.database

import arcs.core.common.collectExceptions
import arcs.core.storage.StorageKey
import java.util.concurrent.atomic.AtomicReference
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.supervisorScope

Expand All @@ -26,6 +27,12 @@ interface DatabaseManager {
/** Manifest of [Database]s managed by this [DatabaseManager]. */
val registry: DatabaseRegistry

/**
* Database Configuration. The config can change at any time, subclasses should get fresh reads of
* it every time it's needed.
*/
val databaseConfig: AtomicReference<DatabaseConfig>

/**
* Gets a [Database] for the given [name]. If [persistent] is `false`, the [Database] should
* only exist in-memory (if possible for the current platform).
Expand Down Expand Up @@ -87,6 +94,11 @@ interface DatabaseManager {
* Extracts all IDs of any hard reference that points to the given [backingStorageKey].
*/
suspend fun getAllHardReferenceIds(backingStorageKey: StorageKey): Set<String>

/**
* Updates the database configuration.
*/
fun updateDatabaseConfig(databaseConfig: DatabaseConfig) = this.databaseConfig.set(databaseConfig)
}

/**
Expand Down Expand Up @@ -136,3 +148,9 @@ val DatabaseIdentifier.name: String
/** Whether or not the [Database] should be persisted to disk. */
val DatabaseIdentifier.persistent: Boolean
get() = second

/** Database configurations of the runtime flags relevant to the database and their values. */
data class DatabaseConfig(
/** Determines whether to use the diffbased approach when inserting entities into a collection. */
val diffbasedEntityInsertion: Boolean = false
)
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import arcs.core.data.Schema
import arcs.core.storage.StorageKey
import arcs.core.storage.database.Database
import arcs.core.storage.database.DatabaseClient
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseData
import arcs.core.storage.database.DatabaseIdentifier
import arcs.core.storage.database.DatabaseManager
Expand All @@ -39,6 +40,7 @@ import kotlinx.coroutines.flow.launchIn
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import java.util.concurrent.atomic.AtomicReference

/** [DatabaseManager] which generates fake [Database] objects. */
open class FakeDatabaseManager(val onGarbageCollection: () -> Unit = {}) : DatabaseManager {
Expand All @@ -49,6 +51,7 @@ open class FakeDatabaseManager(val onGarbageCollection: () -> Unit = {}) : Datab
private val _manifest = FakeDatabaseRegistry()
private val clients = arrayListOf<DatabaseClient>()
override val registry: FakeDatabaseRegistry = _manifest
override val databaseConfig: AtomicReference<DatabaseConfig> = AtomicReference(DatabaseConfig())

fun addClients(vararg clients: DatabaseClient) = this.clients.addAll(clients)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import arcs.core.data.SchemaFields
import arcs.core.data.util.toReferencable
import arcs.core.storage.RawReference
import arcs.core.storage.StorageKeyManager
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseData
import arcs.core.storage.database.DatabaseManager
import arcs.core.storage.testutil.DummyStorageKey
Expand Down Expand Up @@ -267,6 +268,27 @@ class AndroidSqliteDatabaseManagerTest {
assertThat(manager.removeEntitiesHardReferencing(refKey, "id2")).isEqualTo(0)
}

@Test
fun updateDatabaseConfig_propagatesToDatabases() = runBlockingTest {
val database = manager.getDatabase("foo", true) as DatabaseImpl

val managerDatabaseConfigBefore = manager.databaseConfig.get()
val databaseDatabaseConfigBefore = database.databaseConfigGetter()

manager.updateDatabaseConfig(DatabaseConfig(diffbasedEntityInsertion = true))

val managerDatabaseConfigAfter = manager.databaseConfig.get()
val databaseDatabaseConfigAfter = database.databaseConfigGetter()

val expectedBefore = DatabaseConfig()
val expectedAfter = DatabaseConfig(diffbasedEntityInsertion = true)

assertThat(managerDatabaseConfigBefore).isEqualTo(expectedBefore)
assertThat(databaseDatabaseConfigBefore).isEqualTo(expectedBefore)
assertThat(managerDatabaseConfigAfter).isEqualTo(expectedAfter)
assertThat(databaseDatabaseConfigAfter).isEqualTo(expectedAfter)
}

private fun entityWithHardRef(refId: String) = DatabaseData.Entity(
RawEntity(
collections = mapOf(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import arcs.android.common.map
import arcs.android.common.transaction
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.testutil.DummyStorageKeyManager
import com.google.common.truth.Truth.assertThat
import org.junit.Test
Expand Down Expand Up @@ -47,7 +48,8 @@ class DatabaseDowngradeTest {
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"arcs",
true
true,
{ DatabaseConfig() }
)

// Open up the databaseImpl, so it performs a downgrade.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import arcs.android.storage.database.testutil.SmallIntegerIdGenerator
import arcs.android.util.testutil.AndroidLogRule
import arcs.core.data.SchemaRegistry
import arcs.core.storage.StorageKeyManager
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.testutil.DummyStorageKey
import arcs.core.storage.testutil.DummyStorageKeyManager
import arcs.core.testutil.runFuzzTest
Expand Down Expand Up @@ -48,7 +49,8 @@ class DatabaseImplFuzzTest {
database = DatabaseImpl(
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"test.sqlite3"
"test.sqlite3",
databaseConfigGetter = { DatabaseConfig() }
)
StorageKeyManager.GLOBAL_INSTANCE.addParser(DummyStorageKey)
}
Expand Down
8 changes: 6 additions & 2 deletions javatests/arcs/android/storage/database/DatabaseImplTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import arcs.core.storage.RawReference
import arcs.core.storage.StorageKey
import arcs.core.storage.StorageKeyManager
import arcs.core.storage.database.DatabaseClient
import arcs.core.storage.database.DatabaseConfig
import arcs.core.storage.database.DatabaseData
import arcs.core.storage.database.DatabaseOp
import arcs.core.storage.database.ReferenceWithVersion
Expand Down Expand Up @@ -91,7 +92,8 @@ class DatabaseImplTest(private val parameters: ParameterizedBuildFlags) {
database = DatabaseImpl(
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"test.sqlite3"
"test.sqlite3",
databaseConfigGetter = { DatabaseConfig() }
)
db = database.writableDatabase
StorageKeyManager.GLOBAL_INSTANCE.addParser(DummyStorageKey)
Expand Down Expand Up @@ -3974,7 +3976,8 @@ class DatabaseImplTest(private val parameters: ParameterizedBuildFlags) {
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"test.sqlite3",
persistent = false
persistent = false,
{ DatabaseConfig() }
)

assertThat(inMemoryDatabase.getSize()).isGreaterThan(0)
Expand Down Expand Up @@ -4134,6 +4137,7 @@ class DatabaseImplTest(private val parameters: ParameterizedBuildFlags) {
ApplicationProvider.getApplicationContext(),
DummyStorageKeyManager(),
"test.sqlite3",
databaseConfigGetter = { DatabaseConfig() },
onDatabaseClose = {
onCloseCalled = true
}
Expand Down

0 comments on commit 799d127

Please sign in to comment.