Merge branch 'master' of github.com:PolymerLabs/arcs into r2p

Fixed lint errors
alxmrs · Mar 3, 2020 · 99000c2 · 99000c2
2 parents 140111f + 5c6310d
commit 99000c2
Show file tree

Hide file tree

Showing 33 changed files with 964 additions and 246 deletions.
diff --git a/java/arcs/android/crdt/BUILD b/java/arcs/android/crdt/BUILD
@@ -21,6 +21,8 @@ kt_android_library(
         "//java/arcs/core/crdt",
         "//java/arcs/core/data:rawentity",
         "//java/arcs/core/data/util:data-util",
+        "//java/arcs/core/storage:reference",
+        "//java/arcs/core/storage:storage_key",
         "//third_party/java/jsr305_annotations",
     ],
 )

diff --git a/java/arcs/android/crdt/ParcelableReferencable.kt b/java/arcs/android/crdt/ParcelableReferencable.kt
@@ -13,11 +13,12 @@ package arcs.android.crdt
 
 import android.os.Parcel
 import android.os.Parcelable
+import arcs.android.crdt.ParcelableReferencable.Type
 import arcs.core.common.Referencable
 import arcs.core.crdt.CrdtEntity
 import arcs.core.data.RawEntity
 import arcs.core.data.util.ReferencablePrimitive
-import java.lang.IllegalArgumentException
+import arcs.core.storage.Reference
 import javax.annotation.OverridingMethodsMustInvokeSuper
 
 /**
@@ -34,6 +35,7 @@ interface ParcelableReferencable : Parcelable {
         // TODO: Add other ParcelableReferencable subclasses.
         RawEntity(ParcelableRawEntity.CREATOR),
         CrdtEntityReferenceImpl(ParcelableCrdtEntity.ReferenceImpl),
+        StorageReferenceImpl(ParcelableReference.CREATOR),
         Primitive(ParcelableReferencablePrimitive.CREATOR);
 
         override fun writeToParcel(parcel: Parcel, flags: Int) {
@@ -56,6 +58,7 @@ interface ParcelableReferencable : Parcelable {
                 // TODO: Add other ParcelableReferencable subclasses.
                 is ParcelableRawEntity -> Type.RawEntity
                 is ParcelableCrdtEntity.ReferenceImpl -> Type.CrdtEntityReferenceImpl
+                is ParcelableReference -> Type.StorageReferenceImpl
                 is ParcelableReferencablePrimitive -> Type.Primitive
                 else -> throw IllegalArgumentException(
                     "Unsupported Referencable type: ${this.javaClass}"
@@ -71,6 +74,7 @@ interface ParcelableReferencable : Parcelable {
         operator fun invoke(actual: Referencable): ParcelableReferencable = when (actual) {
             // TODO: Add other ParcelableReferencable subclasses.
             is RawEntity -> ParcelableRawEntity(actual)
+            is Reference -> ParcelableReference(actual)
             is CrdtEntity.ReferenceImpl -> ParcelableCrdtEntity.ReferenceImpl(actual)
             is ReferencablePrimitive<*> -> ParcelableReferencablePrimitive(actual)
             else ->

diff --git a/java/arcs/android/crdt/ParcelableReference.kt b/java/arcs/android/crdt/ParcelableReference.kt
@@ -0,0 +1,57 @@
+/*
+ * 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.android.crdt
+
+import android.os.Parcel
+import android.os.Parcelable
+import arcs.android.util.writeProto
+import arcs.core.storage.Reference
+import arcs.core.storage.StorageKeyParser
+
+/** Parcelable version of [Reference]. */
+data class ParcelableReference(override val actual: Reference) : ParcelableReferencable {
+    override fun writeToParcel(parcel: Parcel, flags: Int) {
+        super.writeToParcel(parcel, flags)
+        parcel.writeString(actual.id)
+        parcel.writeString(actual.storageKey.toString())
+        actual.version?.let {
+            parcel.writeProto(it.toProto())
+        } ?: {
+            parcel.writeTypedObject(null, flags)
+        }()
+    }
+
+    override fun describeContents(): Int = 0
+
+    /* Don't use this directly, instead use ParcelableReferencable. */
+    internal companion object CREATOR : Parcelable.Creator<ParcelableReference> {
+        override fun createFromParcel(parcel: Parcel): ParcelableReference {
+            val id = requireNotNull(parcel.readString()) {
+                "Required id not found in parcel for ParcelableReference"
+            }
+            val storageKeyString = requireNotNull(parcel.readString()) {
+                "Required storageKey not found in parcel for ParcelableReference"
+            }
+            val versionMap = parcel.readVersionMap()?.takeIf { it.isNotEmpty() }
+
+            return ParcelableReference(
+                Reference(id, StorageKeyParser.parse(storageKeyString), versionMap)
+            )
+        }
+
+        override fun newArray(size: Int): Array<ParcelableReference?> = arrayOfNulls(size)
+    }
+}
+
+/** Writes the [Reference] to the receiving [Parcel]. */
+fun Parcel.writeReference(reference: Reference, flags: Int) =
+    writeTypedObject(ParcelableReference(reference), flags)
diff --git a/java/arcs/core/crdt/extension/ConversionExtensions.kt b/java/arcs/core/crdt/extension/ConversionExtensions.kt
@@ -19,8 +19,10 @@ import arcs.core.data.util.ReferencablePrimitive
 import arcs.core.util.Base64
 
 /** Converts the [RawEntity] into a [CrdtEntity.Data] model, at the given version. */
-fun RawEntity.toCrdtEntityData(versionMap: VersionMap): CrdtEntity.Data =
-    CrdtEntity.Data(versionMap.copy(), this) { CrdtEntity.ReferenceImpl(it.id) }
+fun RawEntity.toCrdtEntityData(
+    versionMap: VersionMap,
+    referenceBuilder: (Referencable) -> CrdtEntity.Reference = { CrdtEntity.ReferenceImpl(it.id) }
+): CrdtEntity.Data = CrdtEntity.Data(versionMap.copy(), this, referenceBuilder)
 
 private fun Any?.toReferencable(): Referencable {
     requireNotNull(this) { "Cannot create a referencable from a null value." }

diff --git a/java/arcs/core/data/Reference.kt b/java/arcs/core/data/Reference.kt
@@ -27,7 +27,7 @@ import kotlinx.coroutines.Dispatchers
  *
  * Developers can check the liveness of a [Reference] using either [isAlive] or [isDead].
  */
-interface Reference<T : Referencable> {
+interface Reference<T : Referencable> : arcs.core.crdt.CrdtEntity.Reference {
     /**
      * Fetches the actual [Entity] value being referenced from storage.
      *

diff --git a/java/arcs/core/data/proto/BUILD b/java/arcs/core/data/proto/BUILD
@@ -1,5 +1,10 @@
+load(
+    "//third_party/java/arcs/build_defs:build_defs.bzl",
+    "arcs_kt_library",
+)
 load(
     "//third_party/java/arcs/build_defs:native.oss.bzl",
+    "android_proto_library",
     "java_proto_library",
     "proto_library",
 )
@@ -8,11 +13,42 @@ licenses(["notice"])
 
 package(default_visibility = ["//visibility:public"])
 
+arcs_kt_library(
+    name = "proto",
+    srcs = glob(
+        ["*.kt"],
+    ),
+    deps = [
+        ":recipe_java_proto_lite",
+        "//java/arcs/core/data",
+    ],
+)
+
+# This target pulls in the java protobuf library, as opposed to the *java_lite*
+# protobuf library. The regular protobuf library is needed for tests.
+arcs_kt_library(
+    name = "proto_for_test",
+    testonly = 1,
+    srcs = glob(
+        ["*.kt"],
+    ),
+    add_android_constraints = False,
+    deps = [
+        ":recipe_java_proto",
+        "//java/arcs/core/data",
+    ],
+)
+
 proto_library(
     name = "recipe_proto",
     srcs = ["recipe.proto"],
 )
 
+android_proto_library(
+    name = "recipe_java_proto_lite",
+    deps = [":recipe_proto"],
+)
+
 java_proto_library(
     name = "recipe_java_proto",
     deps = [":recipe_proto"],

diff --git a/java/arcs/core/data/proto/TypeProtoDecoders.kt b/java/arcs/core/data/proto/TypeProtoDecoders.kt
@@ -0,0 +1,26 @@
+/*
+ * 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.core.data.proto
+
+import arcs.core.data.PrimitiveType
+
+/**
+ * Converts a [PrimitiveTypeProto] protobuf instance into a native kotlin [PrimitiveType] instance.
+ */
+fun PrimitiveTypeProto.decode(): PrimitiveType =
+    when (this) {
+        PrimitiveTypeProto.TEXT -> PrimitiveType.Text
+        PrimitiveTypeProto.NUMBER -> PrimitiveType.Number
+        PrimitiveTypeProto.BOOLEAN -> PrimitiveType.Boolean
+        PrimitiveTypeProto.UNRECOGNIZED ->
+            throw IllegalArgumentException("Unknown PrimitiveTypeProto value.")
+    }
diff --git a/java/arcs/core/storage/DirectStore.kt b/java/arcs/core/storage/DirectStore.kt
@@ -92,7 +92,7 @@ class DirectStore<Data : CrdtData, Op : CrdtOperation, T> /* internal */ constru
         return when (message) {
             is ProxyMessage.SyncRequest -> {
                 callbacks.value[message.id]?.invoke(
-                    ProxyMessage.ModelUpdate(localModel.data, message.id)
+                    ProxyMessage.ModelUpdate(getLocalData(), message.id)
                 )
                 true
             }

diff --git a/java/arcs/core/storage/ReferenceModeStore.kt b/java/arcs/core/storage/ReferenceModeStore.kt
@@ -566,7 +566,10 @@ class ReferenceModeStore private constructor(
             entity,
             VersionMap(crdtKey to maxVersion),
             fieldVersionProvider
-        ) { CrdtEntity.ReferenceImpl(it.id) }
+        ) {
+            if (it is Reference) it
+            else CrdtEntity.Reference.buildReference(it)
+        }
     }
 
     companion object {

diff --git a/java/arcs/core/storage/driver/Database.kt b/java/arcs/core/storage/driver/Database.kt
@@ -271,7 +271,11 @@ class DatabaseDriver<Data : Any>(
                 )?.also {
                     dataAndVersion = when (it) {
                         is DatabaseData.Entity ->
-                            it.rawEntity.toCrdtEntityData(it.versionMap)
+                            it.rawEntity.toCrdtEntityData(it.versionMap) { refable ->
+                                // Use the storage reference if it is one.
+                                if (refable is Reference) refable
+                                else CrdtEntity.Reference.buildReference(refable)
+                            }
                         is DatabaseData.Singleton ->
                             it.reference.toCrdtSingletonData(it.versionMap)
                         is DatabaseData.Collection ->
@@ -361,7 +365,10 @@ class DatabaseDriver<Data : Any>(
         val actualData = when (data) {
             is DatabaseData.Singleton -> data.reference.toCrdtSingletonData(data.versionMap)
             is DatabaseData.Collection -> data.values.toCrdtSetData(data.versionMap)
-            is DatabaseData.Entity -> data.rawEntity.toCrdtEntityData(data.versionMap)
+            is DatabaseData.Entity -> data.rawEntity.toCrdtEntityData(data.versionMap) {
+                if (it is Reference) it
+                else CrdtEntity.Reference.buildReference(it)
+            }
         } as Data
 
         // Stash it locally.

diff --git a/java/arcs/core/storage/handle/RawEntityDereferencer.kt b/java/arcs/core/storage/handle/RawEntityDereferencer.kt
@@ -43,7 +43,7 @@ class RawEntityDereferencer(
     override suspend fun dereference(
         reference: Reference,
         coroutineContext: CoroutineContext
-    ): RawEntity? = withContext(coroutineContext) {
+    ): RawEntity? {
         log.debug { "De-referencing $reference" }
 
         val storageKey = reference.storageKey.childKeyWithComponent(reference.id)
@@ -70,20 +70,24 @@ class RawEntityDereferencer(
                 true
             }
         )
-        launch { store.onProxyMessage(ProxyMessage.SyncRequest(token)) }
 
-        // Only return the item if we've actually managed to pull it out of the database.
-        deferred.await().takeIf { it matches schema }
+        return withContext(coroutineContext) {
+            launch { store.onProxyMessage(ProxyMessage.SyncRequest(token)) }
+
+            // Only return the item if we've actually managed to pull it out of the database.
+            deferred.await().takeIf { it matches schema }?.copy(id = reference.id)
+        }
     }
+}
 
-    private infix fun RawEntity.matches(schema: Schema): Boolean {
-        // Only allow empty to match if the Schema is also empty.
-        // TODO: Is this a correct assumption?
-        if (singletons.isEmpty() && collections.isEmpty())
-            return schema.fields.singletons.isEmpty() && schema.fields.collections.isEmpty()
+/* internal */
+infix fun RawEntity.matches(schema: Schema): Boolean {
+    // Only allow empty to match if the Schema is also empty.
+    // TODO: Is this a correct assumption?
+    if (singletons.isEmpty() && collections.isEmpty())
+        return schema.fields.singletons.isEmpty() && schema.fields.collections.isEmpty()
 
-        // Return true if any of the RawEntity's fields are part of the Schema.
-        return (singletons.isEmpty() || singletons.keys.any { it in schema.fields.singletons }) &&
-            (collections.isEmpty() || collections.keys.any { it in schema.fields.collections })
-    }
+    // Return true if any of the RawEntity's fields are part of the Schema.
+    return (singletons.isEmpty() || singletons.keys.any { it in schema.fields.singletons }) &&
+        (collections.isEmpty() || collections.keys.any { it in schema.fields.collections })
 }
diff --git a/java/arcs/sdk/android/storage/BUILD b/java/arcs/sdk/android/storage/BUILD
@@ -21,5 +21,6 @@ kt_android_library(
         "//third_party/java/androidx/annotation",
         "//third_party/java/androidx/lifecycle",
         "//third_party/kotlin/kotlinx_coroutines",
+        "//third_party/kotlin/kotlinx_coroutines:kotlinx_coroutines_android",
     ],
 )
diff --git a/java/arcs/sdk/android/storage/ServiceStore.kt b/java/arcs/sdk/android/storage/ServiceStore.kt
@@ -103,9 +103,7 @@ class ServiceStore<Data : CrdtData, Op : CrdtOperation, ConsumerData>(
     @Suppress("UNCHECKED_CAST")
     override suspend fun getLocalData(): Data {
         val service = checkNotNull(storageService)
-        val channel = ParcelableProxyMessageChannel(
-            coroutineContext
-        )
+        val channel = ParcelableProxyMessageChannel(coroutineContext)
         service.getLocalData(channel)
         val flow = channel.asFlow()
         val modelUpdate =
@@ -140,12 +138,10 @@ class ServiceStore<Data : CrdtData, Op : CrdtOperation, ConsumerData>(
             "Connection to StorageService is already alive."
         }
         val connection = connectionFactory(options, crdtType)
+        // Need to initiate the connection on the main thread.
         val service = connection.connectAsync().await()
 
-        val messageChannel =
-            ParcelableProxyMessageChannel(
-                coroutineContext
-            )
+        val messageChannel = ParcelableProxyMessageChannel(coroutineContext)
         serviceCallbackToken = withContext(coroutineContext) {
             service.registerCallback(messageChannel)
         }

diff --git a/javatests/arcs/android/host/BUILD b/javatests/arcs/android/host/BUILD
@@ -9,6 +9,16 @@ licenses(["notice"])
 
 package(default_visibility = ["//visibility:public"])
 
+kt_android_library(
+    name = "test_app",
+    testonly = 1,
+    srcs = ["TestActivity.kt"],
+    manifest = ":AndroidManifest.xml",
+    deps = [
+        "//third_party/java/androidx/appcompat",
+    ],
+)
+
 arcs_kt_android_test_suite(
     name = "host",
     srcs = glob(["*Test.kt"]),
@@ -17,6 +27,7 @@ arcs_kt_android_test_suite(
     deps = [
         ":schemas",
         ":services",
+        ":test_app",
         "//java/arcs/android/crdt",
         "//java/arcs/android/host",
         "//java/arcs/android/sdk/host",
@@ -42,7 +53,6 @@ arcs_kt_android_test_suite(
         "//java/arcs/sdk/android/storage",
         "//java/arcs/sdk/android/storage/service",
         "//java/arcs/sdk/android/storage/service/testutil",
-        "//javatests/arcs/android/storage/handle:test_app",
         "//javatests/arcs/core/allocator:allocator-test-util",
         "//third_party/android/androidx_test/core",
         "//third_party/android/androidx_test/ext/junit",