Skip to content

Commit

Permalink
Remove the need to launch a new coroutine when resurrecting Arcs
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 345294507
  • Loading branch information
jblebrun authored and arcs-c3po committed Dec 2, 2020
1 parent 890cbd3 commit 84fc7ee
Show file tree
Hide file tree
Showing 11 changed files with 135 additions and 101 deletions.
14 changes: 6 additions & 8 deletions java/arcs/core/host/AbstractArcHost.kt
Original file line number Diff line number Diff line change
Expand Up @@ -582,15 +582,13 @@ abstract class AbstractArcHost(

/** Helper used by implementors of [ResurrectableHost]. */
@Suppress("UNUSED_PARAMETER")
fun onResurrected(arcId: String, affectedKeys: List<StorageKey>) {
auxillaryScope.launch {
if (isRunning(arcId)) {
return@launch
}
val context = lookupOrCreateArcHostContext(arcId)
val partition = contextToPartition(arcId, context)
startArc(partition)
suspend fun onResurrected(arcId: String, affectedKeys: List<StorageKey>) {
if (isRunning(arcId)) {
return
}
val context = lookupOrCreateArcHostContext(arcId)
val partition = contextToPartition(arcId, context)
startArc(partition)
}

private fun contextToPartition(arcId: String, context: ArcHostContext) =
Expand Down
7 changes: 4 additions & 3 deletions java/arcs/sdk/android/labs/host/AndroidResurrectableHost.kt
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
*/
package arcs.sdk.android.labs.host

/* ktlint-disable import-ordering */
import android.content.Context
import androidx.lifecycle.Lifecycle
import arcs.core.host.ArcHost
Expand Down Expand Up @@ -43,11 +44,11 @@ abstract class AndroidResurrectableHost(
schedulerProvider = schedulerProvider,
storageEndpointManager = storageEndpointManager,
particles = *particles
), ResurrectableHost {
),
ResurrectableHost {

override val resurrectionHelper: ResurrectionHelper = ResurrectionHelper(
context,
::onResurrected
context
)

override fun maybeRequestResurrection(context: ArcHostContext) {
Expand Down
37 changes: 31 additions & 6 deletions java/arcs/sdk/android/labs/host/ArcHostHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import android.os.Bundle
import android.os.Parcelable
import android.os.ResultReceiver
import androidx.annotation.VisibleForTesting
import arcs.android.common.resurrection.ResurrectionRequest
import arcs.android.host.parcelables.ActualParcelable
import arcs.android.host.parcelables.ParcelableParticleIdentifier
import arcs.android.host.parcelables.ParcelablePlanPartition
Expand All @@ -32,6 +33,8 @@ import arcs.core.host.ArcHostException
import arcs.core.host.ArcState
import arcs.core.host.ArcStateChangeRegistration
import arcs.core.host.ParticleIdentifier
import arcs.core.storage.StorageKeyParser
import arcs.core.util.TaggedLog
import kotlin.reflect.KClass
import kotlinx.coroutines.CoroutineName
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -71,7 +74,7 @@ class ArcHostHelper(
) {
private val job = SupervisorJob() + Dispatchers.Unconfined + CoroutineName("ArcHostHelper")
private val arcHostByHostId = mutableMapOf<String, ArcHost>()

private val log = TaggedLog { "ArcHostHelper" }
init {
arcHosts.forEach { arcHostByHostId[it.hostId] = it }
}
Expand All @@ -89,16 +92,38 @@ class ArcHostHelper(
onStartCommandSuspendable(intent)
}

private suspend fun resurrectArc(intent: Intent, arcHost: ResurrectableHost) {
val targetId = intent.getStringExtra(ResurrectionRequest.EXTRA_REGISTRATION_TARGET_ID)
if (targetId == null) {
log.warning { "Received resurrection intent with null target ID" }
return
}

val notifiers = intent.getStringArrayListExtra(ResurrectionRequest.EXTRA_RESURRECT_NOTIFIER)
if (notifiers == null) {
log.warning { "Received resurrection intent with null notifiers" }
return
}

arcHost.onResurrected(
targetId,
notifiers.map(StorageKeyParser.Companion::parse) ?: emptyList()
)
}

@VisibleForTesting
suspend fun onStartCommandSuspendable(intent: Intent?) {
arcHostByHostId.values.forEach {
if (it is ResurrectableHost) {
it.resurrectionHelper.onStartCommand(intent)
}
val action = intent?.action ?: return

if (action.startsWith(ResurrectionRequest.ACTION_RESURRECT)) {
arcHostByHostId.values
.filterIsInstance<ResurrectableHost>()
.forEach {
resurrectArc(intent, it)
}
}

// Ignore other actions
val action = intent?.action ?: return
if (!action.startsWith(ArcHostHelper.ACTION_HOST_INTENT)) return

// Ignore Intent when it doesn't target our Service
Expand Down
2 changes: 2 additions & 0 deletions java/arcs/sdk/android/labs/host/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,14 @@ arcs_kt_android_library(
srcs = glob(["*.kt"]),
manifest = "//java/arcs/android/common:AndroidManifest.xml",
deps = [
"//java/arcs/android/common/resurrection",
"//java/arcs/android/host/parcelables",
"//java/arcs/core/common",
"//java/arcs/core/data",
"//java/arcs/core/host",
"//java/arcs/core/storage",
"//java/arcs/core/storage:storage_key",
"//java/arcs/core/util",
"//java/arcs/jvm/util",
"//java/arcs/sdk/android/storage",
"//third_party/java/androidx/annotation",
Expand Down
2 changes: 1 addition & 1 deletion java/arcs/sdk/android/labs/host/ResurrectableHost.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,5 +23,5 @@ interface ResurrectableHost : ArcHost {
val resurrectionHelper: ResurrectionHelper

/** Invoked by [ArcHostHelper.onStartCommand] when [ResurrectorService] needs to wake an arc. */
fun onResurrected(arcId: String, affectedKeys: List<StorageKey>)
suspend fun onResurrected(arcId: String, affectedKeys: List<StorageKey>)
}
20 changes: 1 addition & 19 deletions java/arcs/sdk/android/storage/ResurrectionHelper.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import android.content.Context
import android.content.Intent
import arcs.android.common.resurrection.ResurrectionRequest
import arcs.core.storage.StorageKey
import arcs.core.storage.StorageKeyParser
import arcs.sdk.android.storage.service.StorageService

/**
Expand Down Expand Up @@ -55,25 +54,8 @@ import arcs.sdk.android.storage.service.StorageService
* ```
*/
class ResurrectionHelper(
private val context: Context,
private val onResurrected: (targetId: String, List<StorageKey>) -> Unit
private val context: Context
) {
/**
* Determines whether or not the given [intent] represents a resurrection, and if it does:
* calls [onResurrected].
*/
fun onStartCommand(intent: Intent?) {
if (intent?.action?.startsWith(ResurrectionRequest.ACTION_RESURRECT) != true) return

val targetId =
intent.getStringExtra(ResurrectionRequest.EXTRA_REGISTRATION_TARGET_ID) ?: return
val notifiers = intent.getStringArrayListExtra(
ResurrectionRequest.EXTRA_RESURRECT_NOTIFIER
) ?: return

onResurrected(targetId, notifiers.map(StorageKeyParser.Companion::parse))
}

/**
* Issue a request to be resurrected by the [StorageService] whenever the data identified by
* the provided [keys] changes.
Expand Down
81 changes: 79 additions & 2 deletions javatests/arcs/android/host/ArcHostHelperTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import android.os.Handler
import android.os.ResultReceiver
import androidx.test.ext.junit.runners.AndroidJUnit4
import androidx.test.platform.app.InstrumentationRegistry
import arcs.android.common.resurrection.ResurrectionRequest
import arcs.core.common.ArcId
import arcs.core.data.EntityType
import arcs.core.data.FieldType.Companion.Text
Expand All @@ -32,16 +33,21 @@ import arcs.core.host.ArcState
import arcs.core.host.ArcStateChangeCallback
import arcs.core.host.ArcStateChangeRegistration
import arcs.core.host.ParticleIdentifier
import arcs.core.storage.DefaultStorageKeyManager
import arcs.core.storage.StorageKey
import arcs.core.storage.keys.VolatileStorageKey
import arcs.core.storage.testutil.DummyStorageKey
import arcs.core.util.guardedBy
import arcs.sdk.android.labs.host.ArcHostHelper
import arcs.sdk.android.labs.host.ResurrectableHost
import arcs.sdk.android.labs.host.createGetRegisteredParticlesIntent
import arcs.sdk.android.labs.host.createLookupArcStatusIntent
import arcs.sdk.android.labs.host.createPauseArcHostIntent
import arcs.sdk.android.labs.host.createStartArcHostIntent
import arcs.sdk.android.labs.host.createStopArcHostIntent
import arcs.sdk.android.labs.host.createUnpauseArcHostIntent
import arcs.sdk.android.labs.host.toComponentName
import arcs.sdk.android.storage.ResurrectionHelper
import com.google.common.truth.Truth.assertThat
import kotlin.coroutines.resume
import kotlin.coroutines.suspendCoroutine
Expand Down Expand Up @@ -85,16 +91,21 @@ class ArcHostHelperTest {

val planPartition = Plan.Partition("id", "FooHost", listOf(particleSpec))

open class TestArcHost : ArcHost {
open class TestArcHost(context: Context) : ArcHost, ResurrectableHost {
private val hostMutex = Mutex()

var startArcCalls: MutableList<Plan.Partition> by guardedBy(hostMutex, mutableListOf())
var stopArcCalls: MutableList<Plan.Partition> by guardedBy(hostMutex, mutableListOf())
var registeredParticles: MutableList<ParticleIdentifier> by guardedBy(
hostMutex, mutableListOf()
)
var lastResurrectedArcId: String? = null
var lastResurrectedKeys: List<StorageKey> = emptyList()

var paused = false

override val resurrectionHelper = ResurrectionHelper(context)

suspend fun startCalls() = hostMutex.withLock { startArcCalls }
suspend fun stopCalls() = hostMutex.withLock { stopArcCalls }
suspend fun particles() = hostMutex.withLock { registeredParticles }
Expand All @@ -112,6 +123,11 @@ class ArcHostHelperTest {
}
}

override suspend fun onResurrected(arcId: String, affectedKeys: List<StorageKey>) {
lastResurrectedArcId = arcId
lastResurrectedKeys = affectedKeys
}

override suspend fun pause() {
paused = true
}
Expand Down Expand Up @@ -153,8 +169,9 @@ class ArcHostHelperTest {
TestArcHost.status = ArcState.Stopped
context = InstrumentationRegistry.getInstrumentation().targetContext
service = Robolectric.setupService(TestAndroidArcHostService::class.java)
arcHost = TestArcHost()
arcHost = TestArcHost(context)
helper = ArcHostHelper(service, arcHost)
DefaultStorageKeyManager.addParser(DummyStorageKey)
}

@Test
Expand Down Expand Up @@ -271,6 +288,66 @@ class ArcHostHelperTest {
assertThat(arcHost.paused).isFalse()
}

@Test
fun onStartCommand_withNullIntent_doesNotResurrect() = runBlockingTest {
helper.onStartCommandSuspendable(null)
assertThat(arcHost.lastResurrectedArcId).isNull()
assertThat(arcHost.lastResurrectedKeys).isEmpty()
}

@Test
fun onStartCommand_withNullIntentAction_doesNotResurrect() = runBlockingTest {
val intent = Intent()
helper.onStartCommandSuspendable(intent)
assertThat(arcHost.lastResurrectedArcId).isNull()
assertThat(arcHost.lastResurrectedKeys).isEmpty()
}

@Test
fun onStartCommand_withNoResurrectionTarget_doesNotResurrect() = runBlockingTest {
val dummyStorageKey1 = DummyStorageKey("foo")
val dummyStorageKey2 = DummyStorageKey("bar")
val notifiers: ArrayList<String> = ArrayList(
listOf(
dummyStorageKey1.toString(),
dummyStorageKey2.toString()
)
)
val intent = Intent(ResurrectionRequest.ACTION_RESURRECT).apply {
putStringArrayListExtra(ResurrectionRequest.EXTRA_RESURRECT_NOTIFIER, notifiers)
}
helper.onStartCommandSuspendable(intent)
assertThat(arcHost.lastResurrectedArcId).isNull()
assertThat(arcHost.lastResurrectedKeys).isEmpty()
}

@Test
fun onStartCommand_withNoNotifiers_doesNotResurrect() = runBlockingTest {
val resurrectingArcId = "arcId"
val intent = Intent(ResurrectionRequest.ACTION_RESURRECT).apply {
putExtra(ResurrectionRequest.EXTRA_REGISTRATION_TARGET_ID, resurrectingArcId)
}
helper.onStartCommandSuspendable(intent)
assertThat(arcHost.lastResurrectedArcId).isNull()
assertThat(arcHost.lastResurrectedKeys).isEmpty()
}

@Test
fun onStartCommand_withResurrectionTargetAndNotifiers_callsOnResurrect() = runBlockingTest {
val resurrectingArcId = "arcId"
val dummyStorageKey1 = DummyStorageKey("foo")
val dummyStorageKey2 = DummyStorageKey("bar")
val notifiers = listOf(dummyStorageKey1, dummyStorageKey2)
val notifiersExtra = ArrayList(notifiers.map { it.toString() })
val intent = Intent(ResurrectionRequest.ACTION_RESURRECT).apply {
putStringArrayListExtra(ResurrectionRequest.EXTRA_RESURRECT_NOTIFIER, notifiersExtra)
putExtra(ResurrectionRequest.EXTRA_REGISTRATION_TARGET_ID, resurrectingArcId)
}
helper.onStartCommandSuspendable(intent)
assertThat(arcHost.lastResurrectedArcId).isEqualTo(resurrectingArcId)
assertThat(arcHost.lastResurrectedKeys).containsExactlyElementsIn(notifiers)
}

private fun <T> runWithResult(
intent: Intent,
transformer: (Bundle?) -> T
Expand Down
3 changes: 3 additions & 0 deletions javatests/arcs/android/host/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ arcs_kt_android_test_suite(
deps = [
":schemas",
":services",
"//java/arcs/android/common/resurrection",
"//java/arcs/android/labs/host",
"//java/arcs/android/labs/host/prod",
"//java/arcs/android/storage/database",
Expand All @@ -27,10 +28,12 @@ arcs_kt_android_test_suite(
"//java/arcs/core/data:schema_fields",
"//java/arcs/core/entity",
"//java/arcs/core/host",
"//java/arcs/core/storage:storage_key",
"//java/arcs/core/storage/api",
"//java/arcs/core/storage/driver:ramdisk",
"//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/core/util",
Expand Down
2 changes: 1 addition & 1 deletion javatests/arcs/android/host/TestExternalArcHostService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ abstract class TestExternalArcHostService : Service() {
),
ResurrectableHost {
override val resurrectionHelper: ResurrectionHelper =
ResurrectionHelper(context, ::onResurrected)
ResurrectionHelper(context)

override val arcHostContextCapability = testingCapability
}
Expand Down
46 changes: 1 addition & 45 deletions javatests/arcs/sdk/android/storage/ResurrectionHelperTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -47,51 +47,7 @@ class ResurrectionHelperTest {

callbackCalls.clear()

helper = ResurrectionHelper(context) { _, keys -> callbackCalls.add(keys) }
}

@Test
fun onStartCommand_doesNotCallOnResurrected_when_intentIsNull() {
helper.onStartCommand(null)
assertThat(callbackCalls).isEmpty()
}

@Test
fun onStartCommand_doesNotCallOnResurrected_when_intentActionIsNull() {
helper.onStartCommand(Intent())
assertThat(callbackCalls).isEmpty()
}

@Test
fun onStartCommand_doesNotCallOnResurrected_when_intentActionDoesNotMatch() {
helper.onStartCommand(Intent().apply { action = "Incorrect" })
assertThat(callbackCalls).isEmpty()
}

@Test
fun onStartCommand_doesNotCallOnResurrected_when_notifierExtra_isMissing() {
helper.onStartCommand(Intent().apply { action = ResurrectionRequest.ACTION_RESURRECT })
assertThat(callbackCalls).isEmpty()
}

@Test
fun onStartCommand_callsOnResurrected_whenStarsAlign() {
val storageKeys = listOf(
RamDiskStorageKey("foo"),
RamDiskStorageKey("bar")
)

val intent = Intent().apply {
action = ResurrectionRequest.ACTION_RESURRECT
putExtra(
ResurrectionRequest.EXTRA_RESURRECT_NOTIFIER,
ArrayList(storageKeys.map(StorageKey::toString))
)
putExtra(ResurrectionRequest.EXTRA_REGISTRATION_TARGET_ID, "test")
}

helper.onStartCommand(intent)
assertThat(callbackCalls).containsExactly(storageKeys)
helper = ResurrectionHelper(context)
}

@Test
Expand Down
Loading

0 comments on commit 84fc7ee

Please sign in to comment.