Skip to content

Commit

Permalink
Run GarbageCollection via aidl, change 2/3: add a new worker which ca…
Browse files Browse the repository at this point in the history
…lls the aidl method. The worker is not yet scheduled. The worker is in the sdk package to avoid a circular dependency with StorageService.kt. It needs a custom factory when constructed with additional parameters.

PiperOrigin-RevId: 351662257
  • Loading branch information
galganif authored and arcs-c3po committed Jan 13, 2021
1 parent e9b5d4e commit 74a3779
Show file tree
Hide file tree
Showing 8 changed files with 269 additions and 21 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright 2020 Google LLC.
*
* This code may only be used under the BSD style license found at
* http://polymer.github.io/LICENSE.txt
*
* Code distributed by Google as part of this project is also subject to an additional IP rights
* grant found at
* http://polymer.github.io/PATENTS.txt
*/
package arcs.sdk.android.storage.service

import android.content.Context
import androidx.work.Worker
import androidx.work.WorkerParameters
import arcs.core.util.TaggedLog
import kotlin.reflect.KClass
import kotlinx.coroutines.runBlocking

/**
* Implementation of a [Worker] which performs periodic scan of storage and deletes unused data.
*/
class DatabaseGarbageCollectionPeriodicTaskV2(
appContext: Context,
workerParams: WorkerParameters,
private val binderHelper: BindHelper = DefaultBindHelper(appContext),
private val storageServiceClass: KClass<out StorageService> = StorageService::class
) : Worker(appContext, workerParams) {

// WorkManager requires a constructor with exactly those two parameters.
constructor(appContext: Context, workerParams: WorkerParameters) :
this(appContext, workerParams, DefaultBindHelper(appContext))

private val log = TaggedLog { WORKER_TAG }

init {
log.debug { "Created." }
}

// Note on `runBlocking` usage:
// `doWork` is run synchronously by the work manager.
// This is one of the rare cases that runBlocking was intended for: implementing the
// functionality of a synchronous API but requiring the use of suspending methods.
//
// Returning from `doWork` is a signal of completion, so we must block here.
//
// To avoid blocking thread issues, ensure that the work manager will not schedule this
// work on any important threads that you're using elsewhere.
override fun doWork(): Result = runBlocking {
log.debug { "Running." }
val success = StorageServiceManagerEndpoint(
binderHelper,
this,
storageServiceClass.java
).runGarbageCollection()
log.debug { "Success=$success." }
// Indicate whether the task finished successfully with the Result
Result.success()
}

companion object {
/**
* Unique name of the worker, used to enqueue the periodic task in [WorkManager].
* This needs to be different from the v1 task.
*/
const val WORKER_TAG = "DatabaseGarbageCollectionPeriodicTaskV2"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,20 @@ class StorageServiceManagerEndpoint(
}
}

/**
* Binds to the IStorageServiceManager and starts a garbage collection run.
*/
suspend fun runGarbageCollection(): Boolean {
return runIResultCallbackOnStorageServiceManager { manager, callback ->
manager.runGarbageCollection(callback)
}
}

/**
* Binds to the IStorageServiceManager, and runs the given block on it. It can be used to run one
* of the methods that take a [IResultCallback].
*/
suspend fun runIResultCallbackOnStorageServiceManager(
private suspend fun runIResultCallbackOnStorageServiceManager(
block: (IStorageServiceManager, IResultCallback) -> Unit
): Boolean {
return withBoundService {
Expand Down
1 change: 1 addition & 0 deletions java/arcs/sdk/android/storage/service/testutil/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ arcs_kt_android_library(
srcs = glob(["*.kt"]),
deps = [
"//java/arcs/sdk/android/storage/service",
"//third_party/java/androidx/work",
"//third_party/java/robolectric",
"//third_party/kotlin/kotlinx_atomicfu",
],
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package arcs.sdk.android.storage.service.testutil

import android.content.Context
import androidx.work.Worker
import androidx.work.WorkerFactory
import androidx.work.WorkerParameters
import arcs.sdk.android.storage.service.DatabaseGarbageCollectionPeriodicTaskV2

/**
* A [WorkerFactory] that injects a [TestBindHelper] in garbage collection tasks.
*/
class TestWorkerFactory : WorkerFactory() {
override fun createWorker(
appContext: Context,
workerClassName: String,
workerParameters: WorkerParameters
): Worker? {
if (workerClassName == DatabaseGarbageCollectionPeriodicTaskV2::class.java.name) {
return DatabaseGarbageCollectionPeriodicTaskV2(
appContext,
workerParameters,
TestBindHelper(appContext)
)
}
return null
}
}
13 changes: 9 additions & 4 deletions javatests/arcs/android/integration/IntegrationEnvironment.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,10 @@ import arcs.jvm.host.ExplicitHostRegistry
import arcs.jvm.util.JvmTime
import arcs.sdk.Particle
import arcs.sdk.android.storage.AndroidStorageServiceEndpointManager
import arcs.sdk.android.storage.service.DatabaseGarbageCollectionPeriodicTaskV2
import arcs.sdk.android.storage.service.StorageServiceManagerEndpoint
import arcs.sdk.android.storage.service.testutil.TestBindHelper
import arcs.sdk.android.storage.service.testutil.TestWorkerFactory
import kotlin.coroutines.CoroutineContext
import kotlin.time.Duration
import kotlin.time.hours
Expand Down Expand Up @@ -207,7 +209,8 @@ class IntegrationEnvironment(

WorkManagerTestInitHelper.initializeTestWorkManager(
context,
Configuration.Builder().setExecutor(SynchronousExecutor()).build()
Configuration.Builder().setExecutor(SynchronousExecutor())
.setWorkerFactory(TestWorkerFactory()).build()
)

workManagerTestDriver = requireNotNull(WorkManagerTestInitHelper.getTestDriver(context)) {
Expand Down Expand Up @@ -287,13 +290,15 @@ class IntegrationEnvironment(

suspend fun getEntitiesCount() = dbManager.getEntitiesCount(persistent = true)

fun triggerCleanupWork() {
fun triggerCleanupWork(useV2: Boolean = false) {
// Advance 49 hours, as only entities older than 48 hours are garbage collected.
advanceClock(49.hours)
triggerWork(PeriodicCleanupTask.WORKER_TAG)
// Need to run twice, once to mark orphans, once to delete them
triggerWork(DatabaseGarbageCollectionPeriodicTask.WORKER_TAG)
triggerWork(DatabaseGarbageCollectionPeriodicTask.WORKER_TAG)
val tag = if (useV2) DatabaseGarbageCollectionPeriodicTaskV2.WORKER_TAG
else DatabaseGarbageCollectionPeriodicTask.WORKER_TAG
triggerWork(tag)
triggerWork(tag)
}

private fun triggerWork(tag: String) {
Expand Down
8 changes: 8 additions & 0 deletions javatests/arcs/sdk/android/storage/service/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,22 @@ arcs_kt_android_test_suite(
"//java/arcs/android/storage/service",
"//java/arcs/android/storage/service:aidl",
"//java/arcs/android/storage/ttl",
"//java/arcs/android/util/testutil",
"//java/arcs/core/crdt",
"//java/arcs/core/data",
"//java/arcs/core/data:schema_fields",
"//java/arcs/core/entity",
"//java/arcs/core/entity/testutil",
"//java/arcs/core/host",
"//java/arcs/core/storage",
"//java/arcs/core/storage/api",
"//java/arcs/core/storage/keys",
"//java/arcs/core/storage/referencemode",
"//java/arcs/core/storage/testutil",
"//java/arcs/core/testutil",
"//java/arcs/core/testutil/handles",
"//java/arcs/jvm/storage/database/testutil",
"//java/arcs/jvm/util/testutil",
"//java/arcs/sdk/android/storage",
"//java/arcs/sdk/android/storage/service",
"//java/arcs/sdk/android/storage/service/testutil",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
package arcs.sdk.android.storage.service

import android.app.Application
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.work.ListenableWorker.Result
import androidx.work.testing.TestWorkerBuilder
import androidx.work.testing.WorkManagerTestInitHelper
import arcs.android.storage.database.AndroidSqliteDatabaseManager
import arcs.android.util.testutil.AndroidLogRule
import arcs.core.data.CollectionType
import arcs.core.data.EntityType
import arcs.core.data.HandleMode
import arcs.core.data.SchemaRegistry
import arcs.core.entity.ForeignReferenceCheckerImpl
import arcs.core.entity.HandleSpec
import arcs.core.entity.ReadWriteCollectionHandle
import arcs.core.entity.awaitReady
import arcs.core.entity.testutil.DummyEntity
import arcs.core.entity.testutil.InlineDummyEntity
import arcs.core.host.HandleManagerImpl
import arcs.core.host.SimpleSchedulerProvider
import arcs.core.storage.api.DriverAndKeyConfigurator
import arcs.core.storage.keys.DatabaseStorageKey
import arcs.core.storage.referencemode.ReferenceModeStorageKey
import arcs.core.storage.testutil.testDatabaseStorageEndpointManager
import arcs.core.testutil.handles.dispatchCreateReference
import arcs.core.testutil.handles.dispatchFetchAll
import arcs.core.testutil.handles.dispatchRemove
import arcs.core.testutil.handles.dispatchStore
import arcs.jvm.util.testutil.FakeTime
import arcs.sdk.android.storage.service.testutil.TestWorkerFactory
import com.google.common.truth.Truth.assertThat
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.runBlocking
import org.junit.Before
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith

@Suppress("EXPERIMENTAL_API_USAGE", "UNCHECKED_CAST")
@RunWith(AndroidJUnit4::class)
class DatabaseGarbageCollectionPeriodicTaskV2Test {

@get:Rule
val log = AndroidLogRule()

private val schedulerProvider = SimpleSchedulerProvider(Dispatchers.Default)
private val backingKey = DatabaseStorageKey.Persistent(
"entities-backing",
DummyEntity.SCHEMA_HASH
)
private val collectionKey = ReferenceModeStorageKey(
backingKey = backingKey,
storageKey = DatabaseStorageKey.Persistent("collection", DummyEntity.SCHEMA_HASH)
)
private lateinit var databaseManager: AndroidSqliteDatabaseManager
private val fakeTime = FakeTime()
private lateinit var worker: DatabaseGarbageCollectionPeriodicTaskV2
private val storageEndpointManager = testDatabaseStorageEndpointManager()

@Before
fun setUp() {
val app: Application = ApplicationProvider.getApplicationContext()
databaseManager = AndroidSqliteDatabaseManager(app)
DriverAndKeyConfigurator.configure(databaseManager)
SchemaRegistry.register(DummyEntity.SCHEMA)
SchemaRegistry.register(InlineDummyEntity.SCHEMA)
WorkManagerTestInitHelper.initializeTestWorkManager(app)
worker = TestWorkerBuilder.from(
app,
DatabaseGarbageCollectionPeriodicTaskV2::class.java
).setWorkerFactory(TestWorkerFactory()).build() as DatabaseGarbageCollectionPeriodicTaskV2
}

@Test
fun garbageCollectionWorkerTest() = runBlocking {
// Set time in the past as only entity older than 2 days are garbage collected.
fakeTime.millis = 1L

val handle = createCollectionHandle()
val entity = DummyEntity().apply {
num = 1.0
texts = setOf("1", "one")
inlineEntity = InlineDummyEntity().apply {
text = "inline"
}
}
handle.dispatchStore(entity)

// Create a reference to entity1, so that we can check the value (but don't persist the
// reference or the entity won't be garbage collected)
val ref1 = handle.dispatchCreateReference(entity)

// Trigger gc worker twice (entity are removed only after being orphan for two runs).
assertThat(worker.doWork()).isEqualTo(Result.success())
assertThat(worker.doWork()).isEqualTo(Result.success())
// Check that the entity is still there as it is still in the collection.
assertThat(ref1.dereference()).isEqualTo(entity)

// Now remove from the collection.
handle.dispatchRemove(entity)
assertThat(handle.dispatchFetchAll()).isEmpty()

// Trigger gc worker twice again.
assertThat(worker.doWork()).isEqualTo(Result.success())
assertThat(worker.doWork()).isEqualTo(Result.success())

// Make sure the subsequent dereference will actually hit the DB.
storageEndpointManager.reset()

// After the second run, the tombstone is gone.
assertThat(ref1.dereference()).isEqualTo(null)
}

@Suppress("UNCHECKED_CAST")
private suspend fun createCollectionHandle() =
HandleManagerImpl(
time = fakeTime,
scheduler = schedulerProvider("test"),
storageEndpointManager = storageEndpointManager,
foreignReferenceChecker = ForeignReferenceCheckerImpl(emptyMap())
).createHandle(
HandleSpec(
"name",
HandleMode.ReadWrite,
CollectionType(EntityType(DummyEntity.SCHEMA)),
DummyEntity
),
collectionKey
).awaitReady() as ReadWriteCollectionHandle<DummyEntity>
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import android.app.Application
import androidx.test.core.app.ApplicationProvider
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.work.testing.WorkManagerTestInitHelper
import arcs.android.crdt.toProto
import arcs.core.crdt.CrdtException
import arcs.core.data.Schema
import arcs.core.data.SchemaFields
Expand Down Expand Up @@ -139,35 +138,34 @@ class StorageServiceManagerEndpointTest {
}

@Test
fun runIResultCallbackOnStorageServiceManager_success() = runBlocking {
fun runGarbageCollection_success() = runBlocking {
var dbManagerGcCalled = false
val databaseManager = FakeDatabaseManager { dbManagerGcCalled = true }
DriverAndKeyConfigurator.configure(databaseManager)
val testBindHelper = TestBindHelper(app)
val endpoint = StorageServiceManagerEndpoint(testBindHelper, this@runBlocking)
var called = false

endpoint.runIResultCallbackOnStorageServiceManager { _, callback ->
called = true
callback.onResult(null)
}
endpoint.runGarbageCollection()

assertThat(called).isTrue()
assertThat(dbManagerGcCalled).isTrue()
assertThat(testBindHelper.activeBindings()).isEqualTo(0)
}

@Test
fun runIResultCallbackOnStorageServiceManager_fail() = runBlocking {
fun runGarbageCollection_fail() = runBlocking {
val databaseManager = FakeDatabaseManager {
throw Exception("message")
}
DriverAndKeyConfigurator.configure(databaseManager)
val testBindHelper = TestBindHelper(app)
val endpoint = StorageServiceManagerEndpoint(testBindHelper, this@runBlocking)
var called = false

val e = assertFailsWith<CrdtException> {
endpoint.runIResultCallbackOnStorageServiceManager { _, callback ->
called = true
callback.onResult(CrdtException("message").toProto().toByteArray())
}
endpoint.runGarbageCollection()
}

assertThat(e.message).isEqualTo("message")
assertThat(called).isTrue()
assertThat(e.message).isEqualTo("GarbageCollection failed")
assertThat(e.cause?.cause?.message).isEqualTo("java.lang.Exception: message")
assertThat(testBindHelper.activeBindings()).isEqualTo(0)
}
}

0 comments on commit 74a3779

Please sign in to comment.