Skip to content

Commit

Permalink
Pipe devToolsProxy into stores (PolymerLabs#6037)
Browse files Browse the repository at this point in the history
* Pipe devToolsProxy into stores

* Fix tests.

* Merge conflict resolution

Co-authored-by: Sarah Heimlich <[email protected]>
  • Loading branch information
SHeimlich and Sarah Heimlich authored Sep 2, 2020
1 parent 94c0346 commit 2a44210
Show file tree
Hide file tree
Showing 18 changed files with 92 additions and 54 deletions.
4 changes: 2 additions & 2 deletions java/arcs/android/devtools/DevToolsService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ open class DevToolsService : Service() {
val service = initialize()
val proxy = service.devToolsProxy

forwardProxyMessageToken = proxy.registerBindingContextProxyMessageCallback(
forwardProxyMessageToken = proxy.registerRefModeStoreProxyMessageCallback(
object : IStorageServiceCallback.Stub() {
override fun onProxyMessage(proxyMessage: ByteArray) {
scope.launch {
Expand Down Expand Up @@ -123,7 +123,7 @@ open class DevToolsService : Service() {
override fun onDestroy() {
super.onDestroy()
devToolsServer.close()
devToolsProxy?.deRegisterBindingContextProxyMessageCallback(forwardProxyMessageToken)
devToolsProxy?.deRegisterRefModeStoreProxyMessageCallback(forwardProxyMessageToken)
scope.cancel()
}

Expand Down
7 changes: 3 additions & 4 deletions java/arcs/android/storage/service/BindingContext.kt
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,11 @@ import kotlinx.coroutines.withTimeout
*/
@ExperimentalCoroutinesApi
class DeferredStore<Data : CrdtData, Op : CrdtOperation, T>(
options: StoreOptions
options: StoreOptions,
private val devToolsProxy: DevToolsProxyImpl?
) {
private val store = SuspendableLazy<ActiveStore<Data, Op, T>> {
DefaultActivationFactory(options)
DefaultActivationFactory(options, devToolsProxy)
}
suspend operator fun invoke() = store()
}
Expand Down Expand Up @@ -147,8 +148,6 @@ class BindingContext(
resultCallback.takeIf { it.asBinder().isBinderAlive }?.onResult(null)

val actualMessage = proxyMessage.decodeProxyMessage()
// TODO: (sarahheimlich) remove once we dive into stores (b/162955831)
devToolsProxy?.onBindingContextProxyMessage(proxyMessage)

(store() as ActiveStore<CrdtData, CrdtOperation, Any?>).let { store ->
if (store.onProxyMessage(actualMessage)) {
Expand Down
28 changes: 16 additions & 12 deletions java/arcs/android/storage/service/DevToolsProxyImpl.kt
Original file line number Diff line number Diff line change
@@ -1,33 +1,37 @@
package arcs.android.storage.service

import arcs.android.storage.toProto
import arcs.core.storage.DevToolsProxy
import arcs.core.storage.ProxyMessage

/**
* Implementation of [IDevToolsProxy] to allow communication between the [StorageService] and
* [DevToolsService]
*/
class DevToolsProxyImpl : IDevToolsProxy.Stub() {
private val onBindingContextProxyMessageCallbacks = mutableMapOf<Int, IStorageServiceCallback>()
private var bindingContextCallbackCounter = 0
class DevToolsProxyImpl : IDevToolsProxy.Stub(), DevToolsProxy {
private val onRefModeStoreProxyMessageCallbacks = mutableMapOf<Int, IStorageServiceCallback>()
private var refModeStoreCallbackCounter = 0

/**
* TODO: (sarahheimlich) remove once we dive into stores (b/162955831)
*
* Execute the callbacks to be called with the [BindingContext] receives a [ProxyMessage]
*/
fun onBindingContextProxyMessage(proxyMessage: ByteArray) {
onBindingContextProxyMessageCallbacks.forEach { (_, callback) ->
callback.onProxyMessage(proxyMessage)
override fun onRefModeStoreProxyMessage(proxyMessage: ProxyMessage<*, *, *>) {
onRefModeStoreProxyMessageCallbacks.forEach { (_, callback) ->
callback.onProxyMessage(proxyMessage.toProto().toByteArray())
}
}

override fun registerBindingContextProxyMessageCallback(
override fun registerRefModeStoreProxyMessageCallback(
callback: IStorageServiceCallback
): Int {
bindingContextCallbackCounter++
onBindingContextProxyMessageCallbacks.put(bindingContextCallbackCounter, callback)
return bindingContextCallbackCounter
refModeStoreCallbackCounter++
onRefModeStoreProxyMessageCallbacks.put(refModeStoreCallbackCounter, callback)
return refModeStoreCallbackCounter
}

override fun deRegisterBindingContextProxyMessageCallback(callbackToken: Int) {
onBindingContextProxyMessageCallbacks.remove(callbackToken)
override fun deRegisterRefModeStoreProxyMessageCallback(callbackToken: Int) {
onRefModeStoreProxyMessageCallbacks.remove(callbackToken)
}
}
12 changes: 4 additions & 8 deletions java/arcs/android/storage/service/IDevToolsProxy.aidl
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,12 @@ import arcs.android.storage.service.IStorageServiceCallback;
interface IDevToolsProxy {

/**
* TODO: (sarahheimlich) remove once we dive into stores (b/162955831)
*
* Register a callback to be called with the [BindingContext] receives a [ProxyMessage]
* Register a callback to be called when the [ReferenceModeStore] receives a [ProxyMessage]
*/
int registerBindingContextProxyMessageCallback(in IStorageServiceCallback callback);
int registerRefModeStoreProxyMessageCallback(in IStorageServiceCallback callback);

/**
* TODO: (sarahheimlich) remove once we dive into stores (b/162955831)
*
* Remove a callback that is called with the [BindingContext] receives a [ProxyMessage]
* Remove a callback that is called when the [ReferenceModeStore] receives a [ProxyMessage]
*/
oneway void deRegisterBindingContextProxyMessageCallback(in int callbackToken);
oneway void deRegisterRefModeStoreProxyMessageCallback(in int callbackToken);
}
12 changes: 12 additions & 0 deletions java/arcs/core/storage/DevToolsProxy.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package arcs.core.storage

/**
* Exposed API to communicate between an [ActiveStore] and [DevToolsService].
*/
interface DevToolsProxy {

/**
* Function to call when a [ReferenceModeStore] receives a [ProxyMessage].
*/
fun onRefModeStoreProxyMessage(proxyMessage: ProxyMessage<*, *, *>)
}
10 changes: 7 additions & 3 deletions java/arcs/core/storage/ReferenceModeStore.kt
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,8 @@ class ReferenceModeStore private constructor(
val containerStore: DirectStore<CrdtData, CrdtOperation, Any?>,
/* internal */
val backingKey: StorageKey,
backingType: Type
backingType: Type,
private val devToolsProxy: DevToolsProxy?
) : ActiveStore<RefModeStoreData, RefModeStoreOp, RefModeStoreOutput>(options) {
// TODO(#5551): Consider including a hash of the storage key in log prefix.
private val log = TaggedLog { "ReferenceModeStore" }
Expand Down Expand Up @@ -202,6 +203,7 @@ class ReferenceModeStore private constructor(
): Boolean {
log.verbose { "onProxyMessage: $message" }
val refModeMessage = message.sanitizeForRefModeStore(type)
devToolsProxy?.onRefModeStoreProxyMessage(message)
return receiveQueue.enqueueAndWait {
handleProxyMessage(refModeMessage)
}
Expand Down Expand Up @@ -689,7 +691,8 @@ class ReferenceModeStore private constructor(

@Suppress("UNCHECKED_CAST")
suspend fun create(
options: StoreOptions
options: StoreOptions,
devToolsProxy: DevToolsProxy?
): ReferenceModeStore {
val refableOptions =
requireNotNull(
Expand Down Expand Up @@ -727,7 +730,8 @@ class ReferenceModeStore private constructor(
refableOptions,
containerStore,
storageKey.backingKey,
type.containedType
type.containedType,
devToolsProxy
).also { refModeStore ->
// Since `on` is a suspending method, we need to setup the container store callback
// here in this create method, which is inside of a coroutine.
Expand Down
8 changes: 5 additions & 3 deletions java/arcs/core/storage/Store.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,11 @@ import kotlinx.coroutines.ExperimentalCoroutinesApi
object DefaultActivationFactory : ActivationFactory {
@ExperimentalCoroutinesApi
override suspend fun <Data : CrdtData, Op : CrdtOperation, T> invoke(
options: StoreOptions
options: StoreOptions,
devToolsProxy: DevToolsProxy?
): ActiveStore<Data, Op, T> = when (options.storageKey) {
is ReferenceModeStorageKey ->
ReferenceModeStore.create(options) as ActiveStore<Data, Op, T>
ReferenceModeStore.create(options, devToolsProxy) as ActiveStore<Data, Op, T>
else -> DirectStore.create(options)
}
}
Expand All @@ -38,6 +39,7 @@ object DefaultActivationFactory : ActivationFactory {
*/
interface ActivationFactory {
suspend operator fun <Data : CrdtData, Op : CrdtOperation, T> invoke(
options: StoreOptions
options: StoreOptions,
devToolsProxy: DevToolsProxy?
): ActiveStore<Data, Op, T>
}
2 changes: 1 addition & 1 deletion java/arcs/core/storage/StoreManager.kt
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class StoreManager(
storeOptions: StoreOptions
) = storesMutex.withLock {
stores.getOrPut(storeOptions.storageKey) {
activationFactory<Data, Op, T>(storeOptions)
activationFactory<Data, Op, T>(storeOptions, null)
} as ActiveStore<Data, Op, T>
}

Expand Down
4 changes: 3 additions & 1 deletion java/arcs/sdk/android/storage/ServiceStore.kt
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import arcs.core.data.EntityType
import arcs.core.data.SingletonType
import arcs.core.storage.ActivationFactory
import arcs.core.storage.ActiveStore
import arcs.core.storage.DevToolsProxy
import arcs.core.storage.ProxyCallback
import arcs.core.storage.ProxyMessage
import arcs.core.storage.StoreOptions
Expand Down Expand Up @@ -61,7 +62,8 @@ class ServiceStoreFactory(
private val connectionFactory: ConnectionFactory? = null
) : ActivationFactory {
override suspend operator fun <Data : CrdtData, Op : CrdtOperation, ConsumerData> invoke(
options: StoreOptions
options: StoreOptions,
devToolsProxy: DevToolsProxy?
): ServiceStore<Data, Op, ConsumerData> {
val storeContext = coroutineContext + CoroutineName("ServiceStore(${options.storageKey})")
val parcelableType = when (options.type) {
Expand Down
2 changes: 1 addition & 1 deletion java/arcs/sdk/android/storage/service/StorageService.kt
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ open class StorageService : ResurrectorService() {
return BindingContext(
stores.computeIfAbsent(options.storageKey) {
@Suppress("UNCHECKED_CAST")
DeferredStore<CrdtData, CrdtOperation, Any>(options)
DeferredStore<CrdtData, CrdtOperation, Any>(options, devToolsProxy)
},
coroutineContext,
stats,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,8 @@ class ReferenceModeStoreDatabaseImplIntegrationTest {
StoreOptions(
testKey,
CollectionType(EntityType(schema))
)
),
null
)
}

Expand All @@ -710,7 +711,8 @@ class ReferenceModeStoreDatabaseImplIntegrationTest {
StoreOptions(
testKey,
SingletonType(EntityType(schema))
)
),
null
)
}

Expand Down
3 changes: 2 additions & 1 deletion javatests/arcs/android/storage/service/BindingContextTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ class BindingContextTest {
StoreOptions(
storageKey,
CountType()
)
),
null
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -663,7 +663,8 @@ class ReferenceModeStoreDatabaseIntegrationTest {
StoreOptions(
testKey,
CollectionType(EntityType(schema))
)
),
null
)
}

Expand All @@ -672,7 +673,8 @@ class ReferenceModeStoreDatabaseIntegrationTest {
StoreOptions(
testKey,
SingletonType(EntityType(schema))
)
),
null
)
}

Expand Down
15 changes: 10 additions & 5 deletions javatests/arcs/core/storage/ReferenceModeStoreStabilityTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ class ReferenceModeStoreStabilityTest {
StoreOptions(
storageKey,
SingletonType(EntityType(schema))
)
),
null
)

val modelValue = CompletableDeferred<RefModeStoreData.Singleton>()
Expand Down Expand Up @@ -128,7 +129,8 @@ class ReferenceModeStoreStabilityTest {
StoreOptions(
storageKey,
CollectionType(EntityType(schema))
)
),
null
)

val modelValue = CompletableDeferred<RefModeStoreData.Set>()
Expand Down Expand Up @@ -192,7 +194,8 @@ class ReferenceModeStoreStabilityTest {
StoreOptions(
storageKey,
CollectionType(EntityType(schema))
)
),
null
)

val modelValue = CompletableDeferred<RefModeStoreData.Set>()
Expand Down Expand Up @@ -254,7 +257,8 @@ class ReferenceModeStoreStabilityTest {
StoreOptions(
storageKey,
SingletonType(EntityType(schema))
)
),
null
)

val modelValue = CompletableDeferred<RefModeStoreData.Singleton>()
Expand Down Expand Up @@ -316,7 +320,8 @@ class ReferenceModeStoreStabilityTest {
StoreOptions(
storageKey,
CollectionType(EntityType(schema))
)
),
null
)

val modelValue = CompletableDeferred<RefModeStoreData.Set>()
Expand Down
12 changes: 8 additions & 4 deletions javatests/arcs/core/storage/ReferenceModeStoreTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,8 @@ class ReferenceModeStoreTest {
StoreOptions(
testKey,
SingletonType(EntityType(schema))
)
),
null
)
}
}
Expand All @@ -103,7 +104,8 @@ class ReferenceModeStoreTest {
StoreOptions(
testKey,
CollectionType(EntityType(schema))
)
),
null
)

assertThat(store).isInstanceOf(ReferenceModeStore::class.java)
Expand Down Expand Up @@ -779,7 +781,8 @@ class ReferenceModeStoreTest {
StoreOptions(
testKey,
CollectionType(EntityType(schema))
)
),
null
)
}

Expand All @@ -788,7 +791,8 @@ class ReferenceModeStoreTest {
StoreOptions(
testKey,
SingletonType(EntityType(schema))
)
),
null
)
}

Expand Down
4 changes: 2 additions & 2 deletions javatests/arcs/core/storage/ReferenceTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ class ReferenceTest {
storageKey = refModeKey,
type = CollectionType(EntityType(Person.SCHEMA))
)
val store: CollectionStore<RawEntity> = DefaultActivationFactory(options)
val store: CollectionStore<RawEntity> = DefaultActivationFactory(options, null)

val addPeople = listOf(
CrdtSet.Operation.Add(
Expand Down Expand Up @@ -90,7 +90,7 @@ class ReferenceTest {

@Suppress("UNCHECKED_CAST")
val directCollection: CollectionStore<Reference> =
DefaultActivationFactory(collectionOptions)
DefaultActivationFactory(collectionOptions, null)

val job = Job()
val me = directCollection.on(ProxyCallback {
Expand Down
Loading

0 comments on commit 2a44210

Please sign in to comment.