From 678fc8173780cbb94ae8e12cc04c88be6d6cd28e Mon Sep 17 00:00:00 2001 From: Piotr Swigon Date: Mon, 2 Mar 2020 15:37:41 -0800 Subject: [PATCH 1/5] Support synthetic join handles in the parser and recipe data structures (#4800) --- src/runtime/manifest-ast-nodes.ts | 10 ++++-- src/runtime/manifest-parser.pegjs | 11 +++++++ src/runtime/manifest.ts | 24 ++++++++++++++- src/runtime/recipe/handle.ts | 17 +++++++++-- src/runtime/recipe/recipe.ts | 5 ++- src/runtime/tests/manifest-parser-test.ts | 7 +++++ src/runtime/tests/manifest-test.ts | 37 +++++++++++++++++++++++ src/runtime/tests/recipe-test.ts | 33 ++++++++++++++++++-- 8 files changed, 136 insertions(+), 8 deletions(-) diff --git a/src/runtime/manifest-ast-nodes.ts b/src/runtime/manifest-ast-nodes.ts index 98efce5945d..b684efeb131 100644 --- a/src/runtime/manifest-ast-nodes.ts +++ b/src/runtime/manifest-ast-nodes.ts @@ -378,7 +378,7 @@ export interface RecipeRequire extends BaseNode { items: RecipeItem[]; } -export type RecipeItem = RecipeParticle | RecipeHandle | RequireHandleSection | RecipeRequire | RecipeSlot | RecipeSearch | RecipeConnection | Description; +export type RecipeItem = RecipeParticle | RecipeHandle | RecipeSyntheticHandle | RequireHandleSection | RecipeRequire | RecipeSlot | RecipeSearch | RecipeConnection | Description; export const RELAXATION_KEYWORD = 'someof'; @@ -413,6 +413,12 @@ export interface RecipeHandle extends BaseNode { annotation: ParameterizedAnnotation|null; } +export interface RecipeSyntheticHandle extends BaseNode { + kind: 'synthetic-handle'; + name: string|null; + associations: string[]; +} + export interface RecipeParticleSlotConnection extends BaseNode { kind: 'slot-connection'; param: string; @@ -760,7 +766,7 @@ export function preSlandlesDirectionToDirection(direction: Direction, isOptional } export type SlotDirection = 'provides' | 'consumes'; -export type Fate = 'use' | 'create' | 'map' | 'copy' | '?' | '`slot'; +export type Fate = 'use' | 'create' | 'map' | 'copy' | 'join' | '?' | '`slot'; export type ParticleHandleConnectionType = TypeVariable|CollectionType| BigCollectionType|ReferenceType|SlotType|SchemaInline|TypeName; diff --git a/src/runtime/manifest-parser.pegjs b/src/runtime/manifest-parser.pegjs index 7ac1f8c1255..dc05292608d 100644 --- a/src/runtime/manifest-parser.pegjs +++ b/src/runtime/manifest-parser.pegjs @@ -836,6 +836,7 @@ RecipeNode RecipeItem = RecipeParticle / RecipeHandle + / RecipeSyntheticHandle / RequireHandleSection / RecipeRequire / RecipeSlot @@ -1079,6 +1080,16 @@ RecipeHandle }); } +RecipeSyntheticHandle + = name:NameWithColon? 'join' whiteSpace '(' whiteSpace? first:lowerIdent rest:(whiteSpace? ',' whiteSpace? lowerIdent)* ')' eolWhiteSpace + { + return toAstNode({ + kind: 'synthetic-handle', + name, + associations: [first].concat(rest.map(t => t[3])), + }); + } + RecipeRequire = 'require' eolWhiteSpace items:(Indent (SameIndent (RecipeParticle / RequireHandleSection / RecipeSlot))*)? { diff --git a/src/runtime/manifest.ts b/src/runtime/manifest.ts index f89c111d4fc..9473dcb43a4 100644 --- a/src/runtime/manifest.ts +++ b/src/runtime/manifest.ts @@ -772,7 +772,8 @@ ${e.message} const items = { require: recipeItems.filter(item => item.kind === 'require') as AstNode.RecipeRequire[], handles: recipeItems.filter(item => item.kind === 'handle') as AstNode.RecipeHandle[], - byHandle: new Map(), + syntheticHandles: recipeItems.filter(item => item.kind === 'synthetic-handle') as AstNode.RecipeSyntheticHandle[], + byHandle: new Map(), // requireHandles are handles constructed by the 'handle' keyword. This is intended to replace handles. requireHandles: recipeItems.filter(item => item.kind === 'requireHandle') as AstNode.RequireHandleSection[], particles: recipeItems.filter(item => item.kind === 'recipe-particle') as AstNode.RecipeParticle[], @@ -825,6 +826,27 @@ ${e.message} items.byHandle.set(handle, item); } + for (const item of items.syntheticHandles) { + const handle = recipe.newHandle(); + handle.fate = 'join'; + + if (item.name) { + assert(!items.byName.has(item.name), `duplicate handle name: ${item.name}`); + handle.localName = item.name; + items.byName.set(item.name, {item, handle}); + } + + for (const association of item.associations) { + const associatedItem = items.byName.get(association); + assert(associatedItem, `unrecognized name: ${association}`); + const associatedHandle = associatedItem && associatedItem.handle; + assert(associatedHandle, `only handles allowed to be joined: "${association}" is not a handle`); + handle.associateHandle(associatedHandle); + } + + items.byHandle.set(handle, item); + } + const prepareEndpoint = (connection, info) => { switch (info.targetType) { case 'particle': { diff --git a/src/runtime/recipe/handle.ts b/src/runtime/recipe/handle.ts index 69a3f5961e9..e041c27f116 100644 --- a/src/runtime/recipe/handle.ts +++ b/src/runtime/recipe/handle.ts @@ -36,6 +36,7 @@ export class Handle implements Comparable { private _originalFate: Fate | null = null; private _originalId: string | null = null; private _connections: HandleConnection[] = []; + private _associatedHandles: Handle[] = []; private _mappedType: Type | undefined = undefined; private _storageKey: StorageKey | undefined = undefined; capabilities: Capabilities; @@ -102,6 +103,7 @@ export class Handle implements Comparable { // attached HandleConnection objects. handle._connections = []; handle._pattern = this._pattern; + handle._associatedHandles = this._associatedHandles.map(h => cloneMap.get(h) as Handle); } return handle; } @@ -123,7 +125,7 @@ export class Handle implements Comparable { _mergedFate(fates: Fate[]) { assert(fates.length > 0, `Cannot merge empty fates list`); // Merging handles only used in coalesce-recipe strategy, which is only done for use/create/? fates. - assert(!fates.includes('map') && !fates.includes('copy'), `Merging map/copy not supported yet`); + assert(!fates.some(f => f === 'map' || f === 'copy' || f === 'join'), `Merging map/copy/join not supported yet`); // If all fates were `use` keep their fate, otherwise set to `create`. return fates.every(fate => fate === 'use') ? 'use' : 'create'; @@ -206,6 +208,7 @@ export class Handle implements Comparable { get localName() { return this._localName; } set localName(name: string) { this._localName = name; } get connections() { return this._connections; } // HandleConnection* + get associatedHandles() { return this._associatedHandles; } get storageKey() { return this._storageKey; } set storageKey(key: StorageKey) { this._storageKey = key; } get pattern() { return this._pattern; } @@ -216,6 +219,7 @@ export class Handle implements Comparable { set immediateValue(value: ParticleSpec) { this._immediateValue = value; } get ttl() { return this._ttl; } set ttl(ttl: Ttl) { this._ttl = ttl; } + get isSynthetic() { return this.fate === 'join'; } // Join handles are the first type of synthetic handles, other may come. static effectiveType(handleType: Type, connections: {type?: Type, direction?: Direction, relaxed?: boolean}[]) { const variableMap = new Map(); @@ -332,13 +336,17 @@ export class Handle implements Comparable { // E.g. hostedParticle = ShowProduct return undefined; } + const getName = (h:Handle) => ((nameMap && nameMap.get(h)) || h.localName); // TODO: type? maybe output in a comment const result: string[] = []; - const name = (nameMap && nameMap.get(this)) || this.localName; + const name = getName(this); if (name) { result.push(`${name}:`); } result.push(this.fate); + if (this.associatedHandles.length) { + result.push(`(${this.associatedHandles.map(h => getName(h)).join(', ')})`); + } if (this.capabilities && !this.capabilities.isEmpty()) { result.push(this.capabilities.toString()); } @@ -376,4 +384,9 @@ export class Handle implements Comparable { findConnectionByDirection(dir: Direction): HandleConnection|undefined { return this._connections.find(conn => conn.direction === dir); } + + associateHandle(handle: Handle) { + assert(this.fate === 'join'); + this._associatedHandles.push(handle); + } } diff --git a/src/runtime/recipe/recipe.ts b/src/runtime/recipe/recipe.ts index 4387bf52d18..8b323b9f552 100644 --- a/src/runtime/recipe/recipe.ts +++ b/src/runtime/recipe/recipe.ts @@ -540,7 +540,10 @@ export class Recipe implements Cloneable { recipe._name = this.name; recipe._verbs = recipe._verbs.concat(...this._verbs); - this._handles.forEach(cloneTheThing); + + // Clone regular handles first, then synthetic ones, as synthetic can depend on regular. + this._handles.filter(h => !h.isSynthetic).forEach(cloneTheThing); + this._handles.filter(h => h.isSynthetic).forEach(cloneTheThing); this._particles.forEach(cloneTheThing); this._slots.forEach(cloneTheThing); this._connectionConstraints.forEach(cloneTheThing); diff --git a/src/runtime/tests/manifest-parser-test.ts b/src/runtime/tests/manifest-parser-test.ts index 4b79696fa73..d4158f7dd8f 100644 --- a/src/runtime/tests/manifest-parser-test.ts +++ b/src/runtime/tests/manifest-parser-test.ts @@ -57,6 +57,13 @@ describe('manifest parser', () => { h1: create 'my-id' #anotherTag @ttl(1h) h2: create @ttl ( 30m )`); }); + it('parses recipes with a synthetic join handles', () => { + parse(` + recipe + people: map #folks + places: map #locations + pairs: join (people, places)`); + }); it('parses recipe handles with capabilities', () => { parse(` recipe Thing diff --git a/src/runtime/tests/manifest-test.ts b/src/runtime/tests/manifest-test.ts index 96bdd652ce4..5db39d52490 100644 --- a/src/runtime/tests/manifest-test.ts +++ b/src/runtime/tests/manifest-test.ts @@ -311,6 +311,43 @@ ${particleStr1} assert.notStrictEqual(manifestA.stores[0].id.toString(), manifestB.stores[0].id.toString()); }); + it('can parse a recipe with a synthetic join handle', async () => { + const manifest = await parseManifest(` + recipe + people: map #folks + other: map #products + pairs: join (people, places) + places: map #locations`); + const verify = (manifest: Manifest) => { + const [recipe] = manifest.recipes; + assert.lengthOf(recipe.handles, 4); + const people = recipe.handles.find(h => h.tags.includes('folks')); + assert.equal(people.fate, 'map'); + const places = recipe.handles.find(h => h.tags.includes('locations')); + assert.equal(places.fate, 'map'); + + const pairs = recipe.handles.find(h => h.fate === 'join'); + assert.equal(pairs.fate, 'join'); + assert.lengthOf(pairs.associatedHandles, 2); + + assert.include(pairs.associatedHandles, people); + assert.include(pairs.associatedHandles, places); + }; + verify(manifest); + verify(await parseManifest(manifest.toString())); + }); + it('fails to parse a recipe with an invalid synthetic join handle', async () => { + try { + await parseManifest(` + recipe + people: map #folks + things: map #products + pairs: join (people, locations)`); + assert.fail(); + } catch (e) { + assert.include(e.message, 'unrecognized name: locations'); + } + }); it('supports recipes with constraints', async () => { const manifest = await parseManifest(` schema S diff --git a/src/runtime/tests/recipe-test.ts b/src/runtime/tests/recipe-test.ts index a40c15f7933..866321122e4 100644 --- a/src/runtime/tests/recipe-test.ts +++ b/src/runtime/tests/recipe-test.ts @@ -12,11 +12,10 @@ import {assert} from '../../platform/chai-web.js'; import {Loader} from '../../platform/loader.js'; import {Manifest} from '../manifest.js'; import {Modality} from '../modality.js'; -import {Type} from '../type.js'; import {Capabilities} from '../capabilities.js'; -import {Flags} from '../flags.js'; import {Entity} from '../entity.js'; import {TtlUnits, Ttl} from '../recipe/ttl.js'; +import {Recipe} from '../recipe/recipe.js'; import {TestVolatileMemoryProvider} from '../testing/test-volatile-memory-provider.js'; import {RamDiskStorageDriverProvider} from '../storageNG/drivers/ramdisk.js'; @@ -838,4 +837,34 @@ describe('recipe', () => { assert.isFalse(recipes[2].isLongRunning); assert.isTrue(recipes[3].isLongRunning); }); + it('can normalize and clone a recipe with a synthetic join handle', async () => { + const [recipe] = (await Manifest.parse(` + recipe + people: map #folks + other: map #products + pairs: join (people, places) + places: map #locations`)).recipes; + + const verify = (recipe: Recipe) => { + assert.lengthOf(recipe.handles, 4); + const people = recipe.handles.find(h => h.tags.includes('folks')); + assert.equal(people.fate, 'map'); + const places = recipe.handles.find(h => h.tags.includes('locations')); + assert.equal(places.fate, 'map'); + + const pairs = recipe.handles.find(h => h.fate === 'join'); + assert.equal(pairs.fate, 'join'); + assert.lengthOf(pairs.associatedHandles, 2); + + assert.include(pairs.associatedHandles, people); + assert.include(pairs.associatedHandles, places); + }; + + verify(recipe); + + recipe.normalize(); + verify(recipe); + + verify(recipe.clone()); + }); }); From ba7a1074ffb487da00625dcfce536763da0b9395 Mon Sep 17 00:00:00 2001 From: Gogul Balakrishnan Date: Mon, 2 Mar 2020 17:51:35 -0800 Subject: [PATCH 2/5] Add a decoder for PrimitiveTypeProto and an option to disable android constraints in BUILD. (#4793) --- java/arcs/core/data/proto/BUILD | 36 +++++++++++++++++++ .../arcs/core/data/proto/TypeProtoDecoders.kt | 26 ++++++++++++++ javatests/arcs/core/data/proto/BUILD | 3 ++ .../core/data/proto/TypeProtoDecodersTest.kt | 26 ++++++++++++++ .../java/arcs/build_defs/internal/kotlin.bzl | 15 ++++++-- 5 files changed, 103 insertions(+), 3 deletions(-) create mode 100644 java/arcs/core/data/proto/TypeProtoDecoders.kt create mode 100644 javatests/arcs/core/data/proto/TypeProtoDecodersTest.kt diff --git a/java/arcs/core/data/proto/BUILD b/java/arcs/core/data/proto/BUILD index f08160dde2e..6e7db7aba5b 100644 --- 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 new file mode 100644 index 00000000000..dbef7c92b02 --- /dev/null +++ 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/javatests/arcs/core/data/proto/BUILD b/javatests/arcs/core/data/proto/BUILD index 40282727e58..4ef0167009a 100644 --- a/javatests/arcs/core/data/proto/BUILD +++ b/javatests/arcs/core/data/proto/BUILD @@ -12,7 +12,10 @@ arcs_kt_jvm_test_suite( data = ["//java/arcs/core/data/testdata:examples"], package = "arcs.core.data.proto", deps = [ + "//java/arcs/core/data", + "//java/arcs/core/data/proto:proto_for_test", "//java/arcs/core/data/proto:recipe_java_proto", + "//java/arcs/core/testutil", "//java/arcs/repoutils", "//third_party/java/arcs/deps:protobuf_java", "//third_party/java/junit:junit-android", diff --git a/javatests/arcs/core/data/proto/TypeProtoDecodersTest.kt b/javatests/arcs/core/data/proto/TypeProtoDecodersTest.kt new file mode 100644 index 00000000000..85a3d63acf5 --- /dev/null +++ b/javatests/arcs/core/data/proto/TypeProtoDecodersTest.kt @@ -0,0 +1,26 @@ +package arcs.core.data.proto + +import arcs.core.data.PrimitiveType +import arcs.core.testutil.assertThrows +import arcs.repoutils.runfilesDir +import com.google.common.truth.Truth.assertThat +import com.google.protobuf.Message.Builder +import com.google.protobuf.Message +import com.google.protobuf.TextFormat +import java.io.File +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@RunWith(JUnit4::class) +class TypeProtoDecodersTest { + @Test + fun decodesPrimitiveTypes() { + assertThat(PrimitiveTypeProto.TEXT.decode()).isEqualTo(PrimitiveType.Text) + assertThat(PrimitiveTypeProto.BOOLEAN.decode()).isEqualTo(PrimitiveType.Boolean) + assertThat(PrimitiveTypeProto.NUMBER.decode()).isEqualTo(PrimitiveType.Number) + assertThrows(IllegalArgumentException::class) { + PrimitiveTypeProto.UNRECOGNIZED.decode() + } + } +} diff --git a/third_party/java/arcs/build_defs/internal/kotlin.bzl b/third_party/java/arcs/build_defs/internal/kotlin.bzl index 6830ce8a8f2..92eac101f59 100644 --- a/third_party/java/arcs/build_defs/internal/kotlin.bzl +++ b/third_party/java/arcs/build_defs/internal/kotlin.bzl @@ -70,7 +70,8 @@ def arcs_kt_jvm_library(**kwargs): Args: **kwargs: Set of args to forward to kt_jvm_library """ - constraints = kwargs.pop("constraints", ["android"]) + add_android_constraints = kwargs.pop("add_android_constraints", True) + constraints = kwargs.pop("constraints", ["android"] if add_android_constraints else []) disable_lint_checks = kwargs.pop("disable_lint_checks", []) exports = kwargs.pop("exports", []) kotlincopts = kwargs.pop("kotlincopts", []) @@ -134,7 +135,9 @@ def arcs_kt_library( deps = [], platforms = DEFAULT_LIBRARY_PLATFORMS, exports = None, - visibility = None): + visibility = None, + testonly = 0, + add_android_constraints = True): """Declares Kotlin library targets for multiple platforms. Args: @@ -145,6 +148,8 @@ def arcs_kt_library( are: "jvm", "js", "wasm". Defaults to "jvm" and "js". exports: List; Optional list of deps to export from this build rule. visibility: List; List of visibilities + add_android_constraints: Adds `constraints = ["android"]` to `kt_jvm_library` rule. + testonly: Marks this target to be used only for tests. """ _check_platforms(platforms) @@ -156,6 +161,7 @@ def arcs_kt_library( deps = [_to_jvm_dep(dep) for dep in deps], exports = exports, visibility = visibility, + add_android_constraints = add_android_constraints, ) if "js" in platforms: @@ -183,7 +189,8 @@ def arcs_kt_particles( srcs = [], deps = [], platforms = DEFAULT_PARTICLE_PLATFORMS, - visibility = None): + visibility = None, + add_android_constraints = True): """Performs final compilation of wasm and bundling if necessary. Args: @@ -196,6 +203,7 @@ def arcs_kt_particles( platforms: List of platforms for which to compile. Valid options are: "jvm", "js", "wasm". Defaults to "jvm" and "js". visibility: list of visibilities + add_android_constraints: Adds `constraints = ["android"]` to `kt_jvm_library` rule. """ _check_platforms(platforms) @@ -210,6 +218,7 @@ def arcs_kt_particles( srcs = srcs, deps = deps, visibility = visibility, + add_android_constraints = add_android_constraints, ) if "js" in platforms: From a6957979be506e3b7afdbf077aae4b06942b1bea Mon Sep 17 00:00:00 2001 From: Jason Feinstein Date: Mon, 2 Mar 2020 18:14:07 -0800 Subject: [PATCH 3/5] Create RawEntityDereferencerTest, storage Reference-> CrdtEntity.Reference (#4812) * Create RawEntityDereferencerTest, make storage Reference implement CrdtEntity.Reference. Also: Create ParcelableReference. * Add dep. Also, apparently read/writeBoolean is Q-only. * Just write null if there is no version map. * Just write null if there is no version map. --- java/arcs/android/crdt/BUILD | 2 + .../android/crdt/ParcelableReferencable.kt | 6 +- java/arcs/android/crdt/ParcelableReference.kt | 57 ++++ .../crdt/extension/ConversionExtensions.kt | 6 +- java/arcs/core/data/Reference.kt | 2 +- java/arcs/core/storage/DirectStore.kt | 2 +- java/arcs/core/storage/ReferenceModeStore.kt | 5 +- java/arcs/core/storage/driver/Database.kt | 11 +- .../storage/handle/RawEntityDereferencer.kt | 21 +- .../storage/ParcelableReferenceTest.kt | 104 +++++++ .../core/storage/driver/DatabaseDriverTest.kt | 7 +- .../handle/RawEntityDereferencerTest.kt | 255 ++++++++++++++++++ 12 files changed, 459 insertions(+), 19 deletions(-) create mode 100644 java/arcs/android/crdt/ParcelableReference.kt create mode 100644 javatests/arcs/android/storage/ParcelableReferenceTest.kt create mode 100644 javatests/arcs/core/storage/handle/RawEntityDereferencerTest.kt diff --git a/java/arcs/android/crdt/BUILD b/java/arcs/android/crdt/BUILD index fb20a3e7488..bce9b916a3d 100644 --- 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 index 54304773ac7..22727f42aa1 100644 --- 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 new file mode 100644 index 00000000000..158b3526964 --- /dev/null +++ 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 { + 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 = 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 index 32eec3fa26a..d172436158a 100644 --- 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 index ed73952b0ca..0950460127f 100644 --- 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 { +interface Reference : arcs.core.crdt.CrdtEntity.Reference { /** * Fetches the actual [Entity] value being referenced from storage. * diff --git a/java/arcs/core/storage/DirectStore.kt b/java/arcs/core/storage/DirectStore.kt index 1af5afd7441..09561478736 100644 --- a/java/arcs/core/storage/DirectStore.kt +++ b/java/arcs/core/storage/DirectStore.kt @@ -92,7 +92,7 @@ class DirectStore /* 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 index 3942b27e6ed..e8568d5cd94 100644 --- 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 index edf3252de5d..efdedc5acd7 100644 --- a/java/arcs/core/storage/driver/Database.kt +++ b/java/arcs/core/storage/driver/Database.kt @@ -271,7 +271,11 @@ class DatabaseDriver( )?.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( 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 index d6182dd7f52..f2ef761b5ac 100644 --- a/java/arcs/core/storage/handle/RawEntityDereferencer.kt +++ b/java/arcs/core/storage/handle/RawEntityDereferencer.kt @@ -73,17 +73,18 @@ class RawEntityDereferencer( 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 } + 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/javatests/arcs/android/storage/ParcelableReferenceTest.kt b/javatests/arcs/android/storage/ParcelableReferenceTest.kt new file mode 100644 index 00000000000..dbbd1cda60d --- /dev/null +++ b/javatests/arcs/android/storage/ParcelableReferenceTest.kt @@ -0,0 +1,104 @@ +/* + * 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.storage + +import android.os.Parcel +import androidx.test.ext.junit.runners.AndroidJUnit4 +import arcs.android.crdt.ParcelableRawEntity +import arcs.android.crdt.readReferencable +import arcs.android.crdt.writeReference +import arcs.core.crdt.VersionMap +import arcs.core.data.RawEntity +import arcs.core.storage.Reference +import arcs.core.storage.driver.RamDiskStorageKey +import com.google.common.truth.Truth.assertThat +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith + +@RunWith(AndroidJUnit4::class) +class ParcelableReferenceTest { + @Before + fun setUp() { + RamDiskStorageKey.registerParser() + } + + @Test + fun parcelableRoundtrip_works_withNullVersionMap() { + val expected = Reference("myId", RamDiskStorageKey("backingKey"), null) + + // Create a parcel and populate it with a ParcelableOperations object. + val marshalled = with(Parcel.obtain()) { + writeReference(expected, 0) + marshall() + } + + // Now unmarshall the parcel, so we can verify the contents. + val unmarshalled = with(Parcel.obtain()) { + unmarshall(marshalled, 0, marshalled.size) + setDataPosition(0) + readReferencable() + } + assertThat(unmarshalled).isEqualTo(expected) + } + + @Test + fun parcelableRoundtrip_works_withNonNullVersionMap() { + val expected = Reference( + "myId", + RamDiskStorageKey("backingKey"), + VersionMap("foo" to 1) + ) + + // Create a parcel and populate it with a ParcelableOperations object. + val marshalled = with(Parcel.obtain()) { + writeReference(expected, 0) + marshall() + } + + // Now unmarshall the parcel, so we can verify the contents. + val unmarshalled = with(Parcel.obtain()) { + unmarshall(marshalled, 0, marshalled.size) + setDataPosition(0) + readReferencable() + } + assertThat(unmarshalled).isEqualTo(expected) + } + + @Test + fun parcelableRoundtripWorks_whenReference_isPartOfRawEntity() { + val expectedReference = Reference( + "myId", + RamDiskStorageKey("backingKey"), + VersionMap("foo" to 1) + ) + val expected = RawEntity( + "myId", + singletons = mapOf("foo" to expectedReference), + collections = emptyMap() + ) + + // Create a parcel and populate it with a ParcelableOperations object. + val marshalled = with(Parcel.obtain()) { + writeTypedObject(ParcelableRawEntity(expected), 0) + marshall() + } + + // Now unmarshall the parcel, so we can verify the contents. + val unmarshalled = with(Parcel.obtain()) { + unmarshall(marshalled, 0, marshalled.size) + setDataPosition(0) + readReferencable() + } + assertThat(unmarshalled).isEqualTo(expected) + } +} diff --git a/javatests/arcs/core/storage/driver/DatabaseDriverTest.kt b/javatests/arcs/core/storage/driver/DatabaseDriverTest.kt index 32fa6ebdc0d..e9da1846db1 100644 --- a/javatests/arcs/core/storage/driver/DatabaseDriverTest.kt +++ b/javatests/arcs/core/storage/driver/DatabaseDriverTest.kt @@ -95,7 +95,12 @@ class DatabaseDriverTest { calledWithVersion = version } - assertThat(calledWithData).isEqualTo(entity.toCrdtEntityData(VersionMap())) + assertThat(calledWithData).isEqualTo( + entity.toCrdtEntityData(VersionMap()) { + if (it is Reference) it + else buildReference(it) + } + ) assertThat(calledWithVersion).isEqualTo(1) } diff --git a/javatests/arcs/core/storage/handle/RawEntityDereferencerTest.kt b/javatests/arcs/core/storage/handle/RawEntityDereferencerTest.kt new file mode 100644 index 00000000000..fe877161072 --- /dev/null +++ b/javatests/arcs/core/storage/handle/RawEntityDereferencerTest.kt @@ -0,0 +1,255 @@ +/* + * 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.storage.handle + +import arcs.core.common.Referencable +import arcs.core.crdt.CrdtEntity +import arcs.core.crdt.CrdtEntity.Reference.Companion.buildReference +import arcs.core.crdt.VersionMap +import arcs.core.data.FieldType +import arcs.core.data.RawEntity +import arcs.core.data.Schema +import arcs.core.data.SchemaFields +import arcs.core.data.util.toReferencable +import arcs.core.storage.Driver +import arcs.core.storage.DriverFactory +import arcs.core.storage.Reference +import arcs.core.storage.driver.RamDisk +import arcs.core.storage.driver.RamDiskDriverProvider +import arcs.core.storage.driver.RamDiskStorageKey +import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.runBlockingTest +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 + +@Suppress("EXPERIMENTAL_API_USAGE", "MapGetWithNotNullAssertionOperator") +@RunWith(JUnit4::class) +class RawEntityDereferencerTest { + // Self-referential schema. + private val schema = Schema( + emptyList(), + SchemaFields( + singletons = mapOf( + "name" to FieldType.Text, + "sibling" to FieldType.EntityRef("abc") + ), + collections = emptyMap() + ), + "abc" + ) + private val backingKey = RamDiskStorageKey("people") + private lateinit var aliceDriver: Driver + private lateinit var bobDriver: Driver + // TODO: Test with an activation factory in android-specific tests. + private val dereferencer = RawEntityDereferencer(schema, entityActivationFactory = null) + private val referenceBuilder = { refable: Referencable -> + if (refable is Reference) refable + else buildReference(refable) + } + + private val alice = RawEntity( + "aliceId", + singletons = mapOf( + "name" to "Alice Entity".toReferencable(), + "sibling" to Reference("bobId", backingKey, VersionMap()) + .also { it.dereferencer = this.dereferencer } + ), + collections = emptyMap() + ) + private val bob = RawEntity( + "bobId", + singletons = mapOf( + "name" to "Bob Entity".toReferencable(), + "sibling" to Reference("aliceId", backingKey, VersionMap()) + .also { it.dereferencer = this.dereferencer } + ), + collections = emptyMap() + ) + + @Before + fun setUp() = runBlocking { + RamDiskDriverProvider() + RamDisk.clear() + + aliceDriver = DriverFactory.getDriver(backingKey.childKeyWithComponent("aliceId"))!! + bobDriver = DriverFactory.getDriver(backingKey.childKeyWithComponent("bobId"))!! + + aliceDriver.send(CrdtEntity.Data(VersionMap("alice" to 1), alice, referenceBuilder), 1) + bobDriver.send(CrdtEntity.Data(VersionMap("bob" to 1), bob, referenceBuilder), 1) + Unit + } + + @Test + fun dereference_canDereference_friend() = runBlockingTest { + val dereferencedBob = (alice.singletons["sibling"] as Reference) + .dereference(this.coroutineContext) + assertThat(dereferencedBob!!.id).isEqualTo(bob.id) + assertThat(dereferencedBob.singletons["name"]!!.unwrap()) + .isEqualTo(bob.singletons["name"]!!.unwrap()) + assertThat(dereferencedBob.singletons["sibling"]!!.unwrap()) + .isEqualTo(bob.singletons["sibling"]!!.unwrap()) + } + + @Test + fun dereference_canDereference_sibling_of_sibling_of_sibling() = runBlockingTest { + val dereferencedBob = + (alice.singletons["sibling"] as Reference).dereference(this.coroutineContext)!! + val dereferencedAliceFromBob = + (dereferencedBob.singletons["sibling"] as Reference) + .also { it.dereferencer = dereferencer } + .dereference(this.coroutineContext)!! + val dereferencedBobFromAliceFromBob = + (dereferencedAliceFromBob.singletons["sibling"] as Reference) + .also { it.dereferencer = dereferencer } + .dereference(this.coroutineContext)!! + + assertThat(dereferencedAliceFromBob.id).isEqualTo(alice.id) + assertThat(dereferencedAliceFromBob.singletons["name"]!!.unwrap()) + .isEqualTo(alice.singletons["name"]!!.unwrap()) + assertThat(dereferencedAliceFromBob.singletons["sibling"]!!.unwrap()) + .isEqualTo(alice.singletons["sibling"]!!.unwrap()) + + assertThat(dereferencedBobFromAliceFromBob.id).isEqualTo(bob.id) + assertThat(dereferencedBobFromAliceFromBob.singletons["name"]!!.unwrap()) + .isEqualTo(bob.singletons["name"]!!.unwrap()) + assertThat(dereferencedBobFromAliceFromBob.singletons["sibling"]!!.unwrap()) + .isEqualTo(bob.singletons["sibling"]!!.unwrap()) + } + + @Test + fun rawEntity_matches_schema_isTrue_whenEntityIsEmpty_andSchemaIsEmpty() { + val entity = RawEntity(singletons = emptyMap(), collections = emptyMap()) + val schema = Schema( + emptyList(), + SchemaFields( + singletons = emptyMap(), + collections = emptyMap() + ), + "abc" + ) + + assertThat(entity matches schema).isTrue() + } + + @Test + fun rawEntity_matches_schema_isFalse_whenEntityIsEmpty_butSchemaIsNot() { + val entity = RawEntity(singletons = emptyMap(), collections = emptyMap()) + val schemaOne = Schema( + emptyList(), + SchemaFields( + singletons = mapOf("name" to FieldType.Text), + collections = emptyMap() + ), + "abc" + ) + val schemaTwo = Schema( + emptyList(), + SchemaFields( + singletons = emptyMap(), + collections = mapOf("friends" to FieldType.EntityRef("def")) + ), + "abc" + ) + + assertThat(entity matches schemaOne).isFalse() + assertThat(entity matches schemaTwo).isFalse() + } + + @Test + fun rawEntity_matches_schema_isTrue_ifSingletonIsFound_inSchema() { + val entity = RawEntity( + singletons = mapOf("name" to "Sundar".toReferencable()), + collections = emptyMap() + ) + val schema = Schema( + emptyList(), + SchemaFields( + singletons = mapOf( + "name" to FieldType.Text, + "age" to FieldType.Number + ), + collections = emptyMap() + ), + "abc" + ) + + assertThat(entity matches schema).isTrue() + } + + @Test + fun rawEntity_matches_schema_isFalse_ifNoSingletonsFound_inSchema() { + val entity = RawEntity( + singletons = mapOf("foo" to "bar".toReferencable()), + collections = emptyMap() + ) + val schema = Schema( + emptyList(), + SchemaFields( + singletons = mapOf( + "name" to FieldType.Text, + "age" to FieldType.Number + ), + collections = emptyMap() + ), + "abc" + ) + + assertThat(entity matches schema).isFalse() + } + + @Test + fun rawEntity_matches_schema_isTrue_ifCollectionIsFound_inSchema() { + val entity = RawEntity( + singletons = emptyMap(), + collections = mapOf( + "friends" to setOf( + Reference("Susan", RamDiskStorageKey("susan"), null) + ) + ) + ) + val schema = Schema( + emptyList(), + SchemaFields( + singletons = emptyMap(), + collections = mapOf("friends" to FieldType.EntityRef("def")) + ), + "abc" + ) + + assertThat(entity matches schema).isTrue() + } + + @Test + fun rawEntity_matches_schema_isTrue_ifNoCollectionsAreFound_inSchema() { + val entity = RawEntity( + singletons = emptyMap(), + collections = mapOf( + "not_friends" to setOf( + Reference("Susan", RamDiskStorageKey("susan"), null) + ) + ) + ) + val schema = Schema( + emptyList(), + SchemaFields( + singletons = emptyMap(), + collections = mapOf("friends" to FieldType.EntityRef("def")) + ), + "abc" + ) + + assertThat(entity matches schema).isFalse() + } +} From b5578ea297e4d3033fdfdefd5f3a325e2cbe503f Mon Sep 17 00:00:00 2001 From: Jason Feinstein Date: Tue, 3 Mar 2020 09:54:17 -0800 Subject: [PATCH 4/5] Add tests for dereferencing references to the HandleManagerTest(s) (#4816) * Add dereferencing tests to the android handle manager test. * Add dereferencing tests to core HandleManager. --- .../storage/handle/RawEntityDereferencer.kt | 11 +- java/arcs/sdk/android/storage/BUILD | 1 + java/arcs/sdk/android/storage/ServiceStore.kt | 10 +- .../handle/AndroidHandleManagerTest.kt | 230 +++++++++++------- javatests/arcs/android/storage/handle/BUILD | 1 + .../core/storage/handle/HandleManagerTest.kt | 123 +++++++--- .../java/arcs/build_defs/internal/kotlin.bzl | 5 +- 7 files changed, 240 insertions(+), 141 deletions(-) diff --git a/java/arcs/core/storage/handle/RawEntityDereferencer.kt b/java/arcs/core/storage/handle/RawEntityDereferencer.kt index f2ef761b5ac..d3bc3e4feab 100644 --- 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,10 +70,13 @@ 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 }?.copy(id = reference.id) + 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) + } } } diff --git a/java/arcs/sdk/android/storage/BUILD b/java/arcs/sdk/android/storage/BUILD index 972633c4a68..3efbbe28632 100644 --- 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 index a823f777266..567b28e5682 100644 --- a/java/arcs/sdk/android/storage/ServiceStore.kt +++ b/java/arcs/sdk/android/storage/ServiceStore.kt @@ -103,9 +103,7 @@ class ServiceStore( @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( "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/storage/handle/AndroidHandleManagerTest.kt b/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt index 2aa9512e07c..72c65cad82f 100644 --- a/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt +++ b/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt @@ -15,6 +15,7 @@ import arcs.core.data.Schema import arcs.core.data.SchemaFields import arcs.core.data.SchemaName import arcs.core.data.util.toReferencable +import arcs.core.storage.Reference import arcs.core.storage.StorageKey import arcs.core.storage.driver.RamDisk import arcs.core.storage.driver.RamDiskStorageKey @@ -36,30 +37,33 @@ import org.junit.runner.RunWith import org.mockito.Mockito.times import org.mockito.Mockito.verify - @Suppress("EXPERIMENTAL_API_USAGE") @RunWith(AndroidJUnit4::class) class AndroidHandleManagerTest { private lateinit var app: Application + private val backingKey = RamDiskStorageKey("entities") + val entity1 = RawEntity( "entity1", - singletons=mapOf( + singletons = mapOf( "name" to "Jason".toReferencable(), "age" to 21.toReferencable(), - "is_cool" to false.toReferencable() + "is_cool" to false.toReferencable(), + "best_friend" to Reference("entity2", backingKey, null) ), - collections=emptyMap() + collections = emptyMap() ) val entity2 = RawEntity( "entity2", - singletons=mapOf( + singletons = mapOf( "name" to "Jason".toReferencable(), "age" to 22.toReferencable(), - "is_cool" to true.toReferencable() + "is_cool" to true.toReferencable(), + "best_friend" to Reference("entity1", backingKey, null) ), - collections=emptyMap() + collections = emptyMap() ) private val schema = Schema( @@ -68,7 +72,8 @@ class AndroidHandleManagerTest { singletons = mapOf( "name" to FieldType.Text, "age" to FieldType.Number, - "is_cool" to FieldType.Boolean + "is_cool" to FieldType.Boolean, + "best_friend" to FieldType.EntityRef("1234acf") ), collections = emptyMap() ), @@ -76,12 +81,12 @@ class AndroidHandleManagerTest { ) private val singletonKey = ReferenceModeStorageKey( - backingKey = RamDiskStorageKey("single-back"), + backingKey = backingKey, storageKey = RamDiskStorageKey("single-ent") ) private val setKey = ReferenceModeStorageKey( - backingKey = RamDiskStorageKey("set-back"), + backingKey = backingKey, storageKey = RamDiskStorageKey("set-ent") ) @@ -121,107 +126,144 @@ class AndroidHandleManagerTest { } @Test - fun testCreateSingletonHandle() = runBlockingTest { - handleManagerTest { hm -> - val singletonHandle = hm.singletonHandle(singletonKey, schema) - singletonHandle.store(entity1) - - // Now read back from a different handle - val readbackHandle = hm.singletonHandle(singletonKey, schema) - val readBack = readbackHandle.fetch() - assertThat(readBack).isEqualTo(entity1) - } + fun testCreateSingletonHandle() = handleManagerTest { hm -> + val singletonHandle = hm.singletonHandle(singletonKey, schema) + singletonHandle.store(entity1) + + // Now read back from a different handle + val readbackHandle = hm.singletonHandle(singletonKey, schema) + val readBack = readbackHandle.fetch() + assertThat(readBack).isEqualTo(entity1) + } + + @Test + fun testDereferencingFromSingletonEntity() = handleManagerTest { hm -> + val singleton1Handle = hm.singletonHandle(singletonKey, schema) + val singleton1Handle2 = hm.singletonHandle(singletonKey, schema) + singleton1Handle.store(entity1) + + // Create a second handle for the second entity, so we can store it. + val singleton2Handle = hm.singletonHandle( + ReferenceModeStorageKey(backingKey, RamDiskStorageKey("entity2")), + schema + ) + val singleton2Handle2 = hm.singletonHandle( + ReferenceModeStorageKey(backingKey, RamDiskStorageKey("entity2")), + schema + ) + singleton2Handle.store(entity2) + + // Now read back entity1, and dereference its best_friend. + val dereferencedEntity2 = + (singleton1Handle2.fetch()!!.singletons["best_friend"] as Reference) + .also { + // Check that it's alive + assertThat(it.isAlive(coroutineContext)).isTrue() + } + .dereference(coroutineContext) + assertThat(dereferencedEntity2).isEqualTo(entity2) + + // Do the same for entity2's best_friend + val dereferencedEntity1 = + (singleton2Handle2.fetch()!!.singletons["best_friend"] as Reference) + .dereference(coroutineContext) + assertThat(dereferencedEntity1).isEqualTo(entity1) } @Test - fun testCreateSetHandle() = runBlockingTest { - handleManagerTest { hm -> - val setHandle = hm.setHandle(setKey, schema) - setHandle.store(entity1) - setHandle.store(entity2) - - // Now read back from a different handle - val secondHandle = hm.setHandle(setKey, schema) - val readBack = secondHandle.fetchAll() - assertThat(readBack).containsExactly(entity1, entity2) + fun testCreateSetHandle() = handleManagerTest { hm -> + val setHandle = hm.setHandle(setKey, schema) + setHandle.store(entity1) + setHandle.store(entity2) + + // Now read back from a different handle + val secondHandle = hm.setHandle(setKey, schema) + val readBack = secondHandle.fetchAll() + assertThat(readBack).containsExactly(entity1, entity2) + } + + @Test + fun testDereferencingFromSetHandleEntity() = handleManagerTest { hm -> + val setHandle = hm.setHandle(setKey, schema) + setHandle.store(entity1) + setHandle.store(entity2) + + val secondHandle = hm.setHandle(setKey, schema) + secondHandle.fetchAll().also { assertThat(it).hasSize(2) }.forEach { entity -> + val expectedBestFriend = if (entity.id == "entity1") entity2 else entity1 + val actualBestFriend = (entity.singletons["best_friend"] as Reference) + .dereference() + assertThat(actualBestFriend).isEqualTo(expectedBestFriend) } } private fun testMapForKey(key: StorageKey) = VersionMap(key.toKeyString() to 1) @Test - fun testSetHandleOnUpdate() = runBlockingTest { - handleManagerTest { hm -> - val testCallback1 = mock>() - val testCallback2 = mock>() - val firstHandle = hm.setHandle(setKey, schema, testCallback1) - val secondHandle = hm.setHandle(setKey, schema, testCallback2) - - val expectedAdd = CrdtSet.Operation.Add( - setKey.toKeyString(), - testMapForKey(setKey), - entity1 - ) - secondHandle.store(entity1) - verify(testCallback1, times(1)).onUpdate(firstHandle, expectedAdd) - verify(testCallback2, times(1)).onUpdate(secondHandle, expectedAdd) - - firstHandle.remove(entity1) - val expectedRemove = CrdtSet.Operation.Remove( - setKey.toKeyString(), - testMapForKey(setKey), - entity1 - ) - verify(testCallback1, times(1)).onUpdate(firstHandle, expectedRemove) - verify(testCallback2, times(1)).onUpdate(secondHandle, expectedRemove) - } + fun testSetHandleOnUpdate() = handleManagerTest { hm -> + val testCallback1 = mock>() + val testCallback2 = mock>() + val firstHandle = hm.setHandle(setKey, schema, testCallback1) + val secondHandle = hm.setHandle(setKey, schema, testCallback2) + + val expectedAdd = CrdtSet.Operation.Add( + setKey.toKeyString(), + testMapForKey(setKey), + entity1 + ) + secondHandle.store(entity1) + verify(testCallback1, times(1)).onUpdate(firstHandle, expectedAdd) + verify(testCallback2, times(1)).onUpdate(secondHandle, expectedAdd) + + firstHandle.remove(entity1) + val expectedRemove = CrdtSet.Operation.Remove( + setKey.toKeyString(), + testMapForKey(setKey), + entity1 + ) + verify(testCallback1, times(1)).onUpdate(firstHandle, expectedRemove) + verify(testCallback2, times(1)).onUpdate(secondHandle, expectedRemove) } @Test - fun testSingletonHandleOnUpdate() = runBlockingTest { - handleManagerTest { hm -> - val testCallback1 = mock>() - val testCallback2 = mock>() - val firstHandle = hm.singletonHandle(singletonKey, schema, testCallback1) - val secondHandle = hm.singletonHandle(singletonKey, schema, testCallback2) - secondHandle.store(entity1) - val expectedAdd = CrdtSingleton.Operation.Update( - singletonKey.toKeyString(), - testMapForKey(singletonKey), - entity1 - ) - verify(testCallback1, times(1)).onUpdate(firstHandle, expectedAdd) - verify(testCallback2, times(1)).onUpdate(secondHandle, expectedAdd) - firstHandle.clear() - - val expectedRemove = CrdtSingleton.Operation.Clear( - singletonKey.toKeyString(), - testMapForKey(singletonKey) - ) - verify(testCallback1, times(1)).onUpdate(firstHandle, expectedRemove) - verify(testCallback2, times(1)).onUpdate(secondHandle, expectedRemove) - } + fun testSingletonHandleOnUpdate() = handleManagerTest { hm -> + val testCallback1 = mock>() + val testCallback2 = mock>() + val firstHandle = hm.singletonHandle(singletonKey, schema, testCallback1) + val secondHandle = hm.singletonHandle(singletonKey, schema, testCallback2) + secondHandle.store(entity1) + val expectedAdd = CrdtSingleton.Operation.Update( + singletonKey.toKeyString(), + testMapForKey(singletonKey), + entity1 + ) + verify(testCallback1, times(1)).onUpdate(firstHandle, expectedAdd) + verify(testCallback2, times(1)).onUpdate(secondHandle, expectedAdd) + firstHandle.clear() + + val expectedRemove = CrdtSingleton.Operation.Clear( + singletonKey.toKeyString(), + testMapForKey(singletonKey) + ) + verify(testCallback1, times(1)).onUpdate(firstHandle, expectedRemove) + verify(testCallback2, times(1)).onUpdate(secondHandle, expectedRemove) } @Test - fun testSetSyncOnRegister() = runBlockingTest { - handleManagerTest { hm -> - val testCallback = mock>() - val firstHandle = hm.setHandle(setKey, schema, testCallback) - verify(testCallback, times(1)).onSync(firstHandle) - firstHandle.fetchAll() - verify(testCallback, times(1)).onSync(firstHandle) - } + fun testSetSyncOnRegister() = handleManagerTest { hm -> + val testCallback = mock>() + val firstHandle = hm.setHandle(setKey, schema, testCallback) + verify(testCallback, times(1)).onSync(firstHandle) + firstHandle.fetchAll() + verify(testCallback, times(1)).onSync(firstHandle) } @Test - fun testSingletonSyncOnRegister() = runBlockingTest { - handleManagerTest { hm -> - val testCallback = mock>() - val firstHandle = hm.singletonHandle(setKey, schema, testCallback) - verify(testCallback, times(1)).onSync(firstHandle) - firstHandle.fetch() - verify(testCallback, times(1)).onSync(firstHandle) - } + fun testSingletonSyncOnRegister() = handleManagerTest { hm -> + val testCallback = mock>() + val firstHandle = hm.singletonHandle(setKey, schema, testCallback) + verify(testCallback, times(1)).onSync(firstHandle) + firstHandle.fetch() + verify(testCallback, times(1)).onSync(firstHandle) } } diff --git a/javatests/arcs/android/storage/handle/BUILD b/javatests/arcs/android/storage/handle/BUILD index a051ff1f5f6..b77cb3265a7 100644 --- a/javatests/arcs/android/storage/handle/BUILD +++ b/javatests/arcs/android/storage/handle/BUILD @@ -20,6 +20,7 @@ kt_android_library( arcs_kt_android_test_suite( name = "handle", + size = "medium", srcs = glob(["*Test.kt"]), manifest = "AndroidManifest.xml", package = "arcs.android.storage.handle", diff --git a/javatests/arcs/core/storage/handle/HandleManagerTest.kt b/javatests/arcs/core/storage/handle/HandleManagerTest.kt index 146f47e8171..4302dd081ff 100644 --- a/javatests/arcs/core/storage/handle/HandleManagerTest.kt +++ b/javatests/arcs/core/storage/handle/HandleManagerTest.kt @@ -7,14 +7,16 @@ import arcs.core.data.SchemaFields import arcs.core.data.SchemaName import arcs.core.data.util.toReferencable import arcs.core.storage.DriverFactory +import arcs.core.storage.Reference +import arcs.core.storage.driver.RamDisk import arcs.core.storage.driver.RamDiskDriverProvider import arcs.core.storage.driver.RamDiskStorageKey import arcs.core.storage.referencemode.ReferenceModeStorageKey import arcs.core.util.Log import arcs.jvm.util.testutil.TimeImpl import com.google.common.truth.Truth.assertThat +import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.test.runBlockingTest import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -23,24 +25,28 @@ import org.junit.runners.JUnit4 @Suppress("EXPERIMENTAL_API_USAGE") @RunWith(JUnit4::class) class HandleManagerTest { + private val backingKey = RamDiskStorageKey("entities") + val entity1 = RawEntity( "entity1", - singletons=mapOf( + singletons = mapOf( "name" to "Jason".toReferencable(), "age" to 21.toReferencable(), - "is_cool" to false.toReferencable() + "is_cool" to false.toReferencable(), + "best_friend" to Reference("entity2", backingKey, null) ), - collections=emptyMap() + collections = emptyMap() ) val entity2 = RawEntity( "entity2", - singletons=mapOf( + singletons = mapOf( "name" to "Jason".toReferencable(), "age" to 22.toReferencable(), - "is_cool" to true.toReferencable() + "is_cool" to true.toReferencable(), + "best_friend" to Reference("entity1", backingKey, null) ), - collections=emptyMap() + collections = emptyMap() ) private val schema = Schema( @@ -49,7 +55,8 @@ class HandleManagerTest { singletons = mapOf( "name" to FieldType.Text, "age" to FieldType.Number, - "is_cool" to FieldType.Boolean + "is_cool" to FieldType.Boolean, + "best_friend" to FieldType.EntityRef("1234acf") ), collections = emptyMap() ), @@ -57,52 +64,100 @@ class HandleManagerTest { ) private val singletonKey = ReferenceModeStorageKey( - backingKey = RamDiskStorageKey("single-back"), + backingKey = backingKey, storageKey = RamDiskStorageKey("single-ent") ) private val setKey = ReferenceModeStorageKey( - backingKey = RamDiskStorageKey("set-back"), + backingKey = backingKey, storageKey = RamDiskStorageKey("set-ent") ) @Before fun setup() { Log.level = Log.Level.Debug + RamDisk.clear() DriverFactory.register(RamDiskDriverProvider()) } @Test - fun testCreateSingletonHandle() = runBlockingTest { - handleManagerTest { hm -> - val singletonHandle = hm.singletonHandle(singletonKey, schema) - singletonHandle.store(entity1) - - // Now read back from a different handle - val readbackHandle = hm.singletonHandle(singletonKey, schema) - val readBack = readbackHandle.fetch() - assertThat(readBack).isEqualTo(entity1) - } + fun testCreateSingletonHandle() = handleManagerTest { hm -> + val singletonHandle = hm.singletonHandle(singletonKey, schema) + singletonHandle.store(entity1) + + // Now read back from a different handle + val readbackHandle = hm.singletonHandle(singletonKey, schema) + val readBack = readbackHandle.fetch() + assertThat(readBack).isEqualTo(entity1) } @Test - fun testCreateSetHandle() = runBlockingTest { - handleManagerTest { hm -> - val setHandle = hm.setHandle(setKey, schema) - setHandle.store(entity1) - setHandle.store(entity2) - - // Now read back from a different handle - val readbackHandle = hm.setHandle(setKey, schema) - val readBack = readbackHandle.fetchAll() - assertThat(readBack).containsExactly(entity1, entity2) + fun testDereferencingFromSingletonEntity() = handleManagerTest { hm -> + val singleton1Handle = hm.singletonHandle(singletonKey, schema) + val singleton1Handle2 = hm.singletonHandle(singletonKey, schema) + singleton1Handle.store(entity1) + + // Create a second handle for the second entity, so we can store it. + val singleton2Handle = hm.singletonHandle( + ReferenceModeStorageKey(backingKey, RamDiskStorageKey("entity2")), + schema + ) + val singleton2Handle2 = hm.singletonHandle( + ReferenceModeStorageKey(backingKey, RamDiskStorageKey("entity2")), + schema + ) + singleton2Handle.store(entity2) + + // Now read back entity1, and dereference its best_friend. + val dereferencedEntity2 = + (singleton1Handle2.fetch()!!.singletons["best_friend"] as Reference) + .also { + // Check that it's alive + assertThat(it.isAlive(coroutineContext)).isTrue() + } + .dereference(coroutineContext) + assertThat(dereferencedEntity2).isEqualTo(entity2) + + // Do the same for entity2's best_friend + val dereferencedEntity1 = + (singleton2Handle2.fetch()!!.singletons["best_friend"] as Reference) + .dereference(coroutineContext) + assertThat(dereferencedEntity1).isEqualTo(entity1) + } + + @Test + fun testCreateSetHandle() = handleManagerTest { hm -> + val setHandle = hm.setHandle(setKey, schema) + setHandle.store(entity1) + setHandle.store(entity2) + + // Now read back from a different handle + val readbackHandle = hm.setHandle(setKey, schema) + val readBack = readbackHandle.fetchAll() + assertThat(readBack).containsExactly(entity1, entity2) + } + + @Test + fun testDereferencingFromSetHandleEntity() = handleManagerTest { hm -> + val setHandle = hm.setHandle(setKey, schema) + setHandle.store(entity1) + setHandle.store(entity2) + + val secondHandle = hm.setHandle(setKey, schema) + secondHandle.fetchAll().also { assertThat(it).hasSize(2) }.forEach { entity -> + val expectedBestFriend = if (entity.id == "entity1") entity2 else entity1 + val actualBestFriend = (entity.singletons["best_friend"] as Reference) + .dereference(coroutineContext) + assertThat(actualBestFriend).isEqualTo(expectedBestFriend) } } - private fun handleManagerTest(block: suspend (HandleManager) -> Unit) { + // TODO: Make runBlockingTest work. + private fun handleManagerTest( + block: suspend CoroutineScope.(HandleManager) -> Unit + ) = runBlocking { val hm = HandleManager(TimeImpl()) - runBlocking { - block(hm) - } + this@runBlocking.block(hm) + Unit } } diff --git a/third_party/java/arcs/build_defs/internal/kotlin.bzl b/third_party/java/arcs/build_defs/internal/kotlin.bzl index 92eac101f59..2c6715ccc61 100644 --- a/third_party/java/arcs/build_defs/internal/kotlin.bzl +++ b/third_party/java/arcs/build_defs/internal/kotlin.bzl @@ -275,7 +275,7 @@ def arcs_kt_particles( visibility = visibility, ) -def arcs_kt_android_test_suite(name, manifest, package, srcs = None, tags = [], deps = [], data = []): +def arcs_kt_android_test_suite(name, manifest, package, srcs = None, tags = [], deps = [], data = [], size = "small"): """Defines Kotlin Android test targets for a directory. Defines a Kotlin Android library (kt_android_library) for all of the sources @@ -291,6 +291,7 @@ def arcs_kt_android_test_suite(name, manifest, package, srcs = None, tags = [], tags: optional list of tags for the test targets deps: list of dependencies for the kt_android_library data: list of files available to the test at runtime + size: the size of the test, defaults to "small". Options are: "small", "medium", "large", etc. """ if not srcs: srcs = native.glob(["*.kt"]) @@ -311,7 +312,7 @@ def arcs_kt_android_test_suite(name, manifest, package, srcs = None, tags = [], class_name = src[:-3] android_local_test( name = class_name, - size = "small", + size = size, manifest = manifest, test_class = "%s.%s" % (package, class_name), tags = tags, From 5c6310da9d1e372544e816c1dd595ef25e5099bf Mon Sep 17 00:00:00 2001 From: jblebrun Date: Tue, 3 Mar 2020 11:02:34 -0800 Subject: [PATCH 5/5] Simplify service test pattern (#4817) * Make a simple test lifecycle registry, instead of creating empty testactivity * Remove use of `runBlockingTest`: according to https://github.com/Kotlin/kotlinx.coroutines/issues/1222, if the test results in coroutines being finished on other threads, it's possible to receive "This job has not yet completed" exceptions, even though the jobs were properly terminated. Since we don't need the delay-skipping properties of `runBlockingTest`, I think it's OK to use `runBlocking`. --- javatests/arcs/android/host/BUILD | 12 +- .../handle/AndroidHandleManagerTest.kt | 108 ++++++------------ .../storage/handle/AndroidManifest.xml | 6 - javatests/arcs/android/storage/handle/BUILD | 14 --- .../android/storage/handle/TestActivity.kt | 5 - 5 files changed, 48 insertions(+), 97 deletions(-) delete mode 100644 javatests/arcs/android/storage/handle/TestActivity.kt diff --git a/javatests/arcs/android/host/BUILD b/javatests/arcs/android/host/BUILD index 055bccd4533..ba4ab0d17f9 100644 --- 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", diff --git a/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt b/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt index 72c65cad82f..bfcc4eb2ee7 100644 --- a/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt +++ b/javatests/arcs/android/storage/handle/AndroidHandleManagerTest.kt @@ -2,6 +2,8 @@ package arcs.android.storage.handle import android.app.Application import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry import androidx.test.core.app.ActivityScenario import androidx.test.core.app.ApplicationProvider import androidx.test.ext.junit.runners.AndroidJUnit4 @@ -27,10 +29,7 @@ import arcs.sdk.android.storage.service.DefaultConnectionFactory import arcs.sdk.android.storage.service.testutil.TestBindingDelegate import com.google.common.truth.Truth.assertThat import com.nhaarman.mockitokotlin2.mock -import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.test.TestCoroutineScope -import kotlinx.coroutines.test.runBlockingTest import org.junit.Before import org.junit.Test import org.junit.runner.RunWith @@ -39,11 +38,16 @@ import org.mockito.Mockito.verify @Suppress("EXPERIMENTAL_API_USAGE") @RunWith(AndroidJUnit4::class) -class AndroidHandleManagerTest { +class AndroidHandleManagerTest : LifecycleOwner { + private lateinit var lifecycle: LifecycleRegistry + override fun getLifecycle() = lifecycle + private lateinit var app: Application private val backingKey = RamDiskStorageKey("entities") + private lateinit var handleManager: HandleManager + val entity1 = RawEntity( "entity1", singletons = mapOf( @@ -93,61 +97,35 @@ class AndroidHandleManagerTest { @Before fun setUp() { RamDisk.clear() + lifecycle = LifecycleRegistry(this).apply { + setCurrentState(Lifecycle.State.CREATED) + setCurrentState(Lifecycle.State.STARTED) + setCurrentState(Lifecycle.State.RESUMED) + } app = ApplicationProvider.getApplicationContext() - app.setTheme(R.style.Theme_AppCompat); // Initialize WorkManager for instrumentation tests. WorkManagerTestInitHelper.initializeTestWorkManager(app) - } - - fun handleManagerTest( - block: suspend TestCoroutineScope.(HandleManager) -> Unit - ) = runBlockingTest { - val scenario = ActivityScenario.launch(TestActivity::class.java) - - scenario.moveToState(Lifecycle.State.STARTED) - - val activityJob = launch { - scenario.onActivity { activity -> - val hf = AndroidHandleManager( - lifecycle = activity.lifecycle, - context = activity, - connectionFactory = DefaultConnectionFactory(activity, TestBindingDelegate(app)), - coroutineContext = coroutineContext - ) - runBlocking { - this@runBlockingTest.block(hf) - } - scenario.close() - } - } - - activityJob.join() - } - @Test - fun testCreateSingletonHandle() = handleManagerTest { hm -> - val singletonHandle = hm.singletonHandle(singletonKey, schema) - singletonHandle.store(entity1) - - // Now read back from a different handle - val readbackHandle = hm.singletonHandle(singletonKey, schema) - val readBack = readbackHandle.fetch() - assertThat(readBack).isEqualTo(entity1) + handleManager = AndroidHandleManager( + lifecycle = lifecycle, + context = app, + connectionFactory = DefaultConnectionFactory(app, TestBindingDelegate(app)) + ) } @Test - fun testDereferencingFromSingletonEntity() = handleManagerTest { hm -> - val singleton1Handle = hm.singletonHandle(singletonKey, schema) - val singleton1Handle2 = hm.singletonHandle(singletonKey, schema) + fun singleton_dereferenceEntity() = runBlocking { + val singleton1Handle = handleManager.singletonHandle(singletonKey, schema) + val singleton1Handle2 = handleManager.singletonHandle(singletonKey, schema) singleton1Handle.store(entity1) // Create a second handle for the second entity, so we can store it. - val singleton2Handle = hm.singletonHandle( + val singleton2Handle = handleManager.singletonHandle( ReferenceModeStorageKey(backingKey, RamDiskStorageKey("entity2")), schema ) - val singleton2Handle2 = hm.singletonHandle( + val singleton2Handle2 = handleManager.singletonHandle( ReferenceModeStorageKey(backingKey, RamDiskStorageKey("entity2")), schema ) @@ -171,28 +149,16 @@ class AndroidHandleManagerTest { } @Test - fun testCreateSetHandle() = handleManagerTest { hm -> - val setHandle = hm.setHandle(setKey, schema) - setHandle.store(entity1) - setHandle.store(entity2) - - // Now read back from a different handle - val secondHandle = hm.setHandle(setKey, schema) - val readBack = secondHandle.fetchAll() - assertThat(readBack).containsExactly(entity1, entity2) - } - - @Test - fun testDereferencingFromSetHandleEntity() = handleManagerTest { hm -> - val setHandle = hm.setHandle(setKey, schema) + fun set_dereferenceEntity () = runBlocking { + val setHandle = handleManager.setHandle(setKey, schema) setHandle.store(entity1) setHandle.store(entity2) - val secondHandle = hm.setHandle(setKey, schema) + val secondHandle = handleManager.setHandle(setKey, schema) secondHandle.fetchAll().also { assertThat(it).hasSize(2) }.forEach { entity -> val expectedBestFriend = if (entity.id == "entity1") entity2 else entity1 val actualBestFriend = (entity.singletons["best_friend"] as Reference) - .dereference() + .dereference(coroutineContext) assertThat(actualBestFriend).isEqualTo(expectedBestFriend) } } @@ -200,11 +166,11 @@ class AndroidHandleManagerTest { private fun testMapForKey(key: StorageKey) = VersionMap(key.toKeyString() to 1) @Test - fun testSetHandleOnUpdate() = handleManagerTest { hm -> + fun set_onHandleUpdate() = runBlocking { val testCallback1 = mock>() val testCallback2 = mock>() - val firstHandle = hm.setHandle(setKey, schema, testCallback1) - val secondHandle = hm.setHandle(setKey, schema, testCallback2) + val firstHandle = handleManager.setHandle(setKey, schema, testCallback1) + val secondHandle = handleManager.setHandle(setKey, schema, testCallback2) val expectedAdd = CrdtSet.Operation.Add( setKey.toKeyString(), @@ -226,11 +192,11 @@ class AndroidHandleManagerTest { } @Test - fun testSingletonHandleOnUpdate() = handleManagerTest { hm -> + fun singleton_OnHandleUpdate() = runBlocking { val testCallback1 = mock>() val testCallback2 = mock>() - val firstHandle = hm.singletonHandle(singletonKey, schema, testCallback1) - val secondHandle = hm.singletonHandle(singletonKey, schema, testCallback2) + val firstHandle = handleManager.singletonHandle(singletonKey, schema, testCallback1) + val secondHandle = handleManager.singletonHandle(singletonKey, schema, testCallback2) secondHandle.store(entity1) val expectedAdd = CrdtSingleton.Operation.Update( singletonKey.toKeyString(), @@ -250,18 +216,18 @@ class AndroidHandleManagerTest { } @Test - fun testSetSyncOnRegister() = handleManagerTest { hm -> + fun set_syncOnRegister() = runBlocking { val testCallback = mock>() - val firstHandle = hm.setHandle(setKey, schema, testCallback) + val firstHandle = handleManager.setHandle(setKey, schema, testCallback) verify(testCallback, times(1)).onSync(firstHandle) firstHandle.fetchAll() verify(testCallback, times(1)).onSync(firstHandle) } @Test - fun testSingletonSyncOnRegister() = handleManagerTest { hm -> + fun singleton_syncOnRegister() = runBlocking { val testCallback = mock>() - val firstHandle = hm.singletonHandle(setKey, schema, testCallback) + val firstHandle = handleManager.singletonHandle(setKey, schema, testCallback) verify(testCallback, times(1)).onSync(firstHandle) firstHandle.fetch() verify(testCallback, times(1)).onSync(firstHandle) diff --git a/javatests/arcs/android/storage/handle/AndroidManifest.xml b/javatests/arcs/android/storage/handle/AndroidManifest.xml index 39989a6b2ed..e3951be6d0e 100644 --- a/javatests/arcs/android/storage/handle/AndroidManifest.xml +++ b/javatests/arcs/android/storage/handle/AndroidManifest.xml @@ -16,12 +16,6 @@ - - - - - - diff --git a/javatests/arcs/android/storage/handle/BUILD b/javatests/arcs/android/storage/handle/BUILD index b77cb3265a7..121e0ce84ab 100644 --- a/javatests/arcs/android/storage/handle/BUILD +++ b/javatests/arcs/android/storage/handle/BUILD @@ -2,22 +2,11 @@ load( "//third_party/java/arcs/build_defs:build_defs.bzl", "arcs_kt_android_test_suite", ) -load("//tools/build_defs/kotlin:rules.bzl", "kt_android_library") 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 = "handle", size = "medium", @@ -25,7 +14,6 @@ arcs_kt_android_test_suite( manifest = "AndroidManifest.xml", package = "arcs.android.storage.handle", deps = [ - ":test_app", "//java/arcs/android/crdt", "//java/arcs/android/storage", "//java/arcs/android/storage/handle", @@ -45,14 +33,12 @@ arcs_kt_android_test_suite( "//java/arcs/sdk/android/storage/service/testutil", "//third_party/android/androidx_test/core", "//third_party/android/androidx_test/ext/junit", - "//third_party/java/androidx/appcompat", "//third_party/java/androidx/work:testing", "//third_party/java/junit:junit-android", "//third_party/java/mockito:mockito-android", "//third_party/java/robolectric", "//third_party/java/truth:truth-android", "//third_party/kotlin/kotlinx_coroutines", - "//third_party/kotlin/kotlinx_coroutines:kotlinx_coroutines_test", "//third_party/kotlin/mockito_kotlin:mockito_kotlin-android", ], ) diff --git a/javatests/arcs/android/storage/handle/TestActivity.kt b/javatests/arcs/android/storage/handle/TestActivity.kt deleted file mode 100644 index c6a27da5cd2..00000000000 --- a/javatests/arcs/android/storage/handle/TestActivity.kt +++ /dev/null @@ -1,5 +0,0 @@ -package arcs.android.storage.handle - -import androidx.appcompat.app.AppCompatActivity - -class TestActivity : AppCompatActivity()