recipe2plan 2/n: Validate Handles, Tests (#4834)

alxmrs · web-flow · commit 337b7b26500e · 2020-03-04T15:55:06.000-08:00
* creating structure of ts r2p script * Created fast CLI rapper for r2p script * WIP figuring out ways to resolve a recipe * fix sp * Passing lint, still WIP * Build macros for recipe2plan work * Lightweight iteration on recipe2plan, stubbed out tests * Revised method to find corresponding create handles * Added type info * Added method to get all handles to manifest + get all handles by Id * fix: no flatMap * - currently, fails since it cannot find associated stores for handles. * uncleaned, but working recipe resolution * Cleaned up for r2p, pt 1 * stubbed out second test (short, short) * Squashed commit of the following: commit 62dcc57 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 14:38:14 2020 -0800 Fixed test, added TODO * updates to test, can't get two to fail * Squashed commit of the following: commit bede8d9 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 14:52:02 2020 -0800 Fixed build rule, simplified runtime commit 62dcc57 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 14:38:14 2020 -0800 Fixed test, added TODO * can't get test to fail... hmm... * Squashed commit of the following: commit f6df54a Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 15:44:32 2020 -0800 tools/sigh lint commit bede8d9 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 14:52:02 2020 -0800 Fixed build rule, simplified runtime commit 62dcc57 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 14:38:14 2020 -0800 Fixed test, added TODO * adjusting recipe copying to include triggers * Revert "adjusting recipe copying to include triggers" This reverts commit 9bc3734 * four tests complete * Added test, fixed minor async issue * Squashed commit of the following: commit 4b05a28 Merge: f4c1d26 99000c2 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 11:18:14 2020 -0800 Merge branch 'r2p' of github.com:alxrsngrtn/arcs into r2p commit f4c1d26 Merge: 140111f 5c6310d Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 11:14:35 2020 -0800 Merge branch 'master' of github.com:PolymerLabs/arcs into r2p Fixed lint errors commit 99000c2 Merge: 140111f 5c6310d Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 11:14:35 2020 -0800 Merge branch 'master' of github.com:PolymerLabs/arcs into r2p Fixed lint errors commit 140111f Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 11:08:12 2020 -0800 Removed flatMap, added TODOs and link to GH issue commit 5c6310d Author: jblebrun <jibbl@google.com> Date: Tue Mar 3 11:02:34 2020 -0800 Simplify service test pattern (#4817) * Make a simple test lifecycle registry, instead of creating empty testactivity * Remove use of `runBlockingTest`: according to Kotlin/kotlinx.coroutines#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`. commit 472bc84 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 10:27:56 2020 -0800 Improved build rules commit ca1ebf8 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 10:17:13 2020 -0800 Impl suggestsions for r2p commit b5578ea Author: Jason Feinstein <jwf@google.com> Date: Tue Mar 3 09:54:17 2020 -0800 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. commit a695797 Author: Jason Feinstein <jwf@google.com> Date: Mon Mar 2 18:14:07 2020 -0800 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. commit ba7a107 Author: Gogul Balakrishnan <bgogul@google.com> Date: Mon Mar 2 17:51:35 2020 -0800 Add a decoder for PrimitiveTypeProto and an option to disable android constraints in BUILD. (#4793) commit f6df54a Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 15:44:32 2020 -0800 tools/sigh lint commit bede8d9 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 14:52:02 2020 -0800 Fixed build rule, simplified runtime commit 62dcc57 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 14:38:14 2020 -0800 Fixed test, added TODO * fix ineq * existing tests passing * Updating invalid type test * Squashed commit of the following: commit b3d92d0 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Wed Mar 4 12:17:17 2020 -0800 updated test name commit ba7884b Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Wed Mar 4 12:16:09 2020 -0800 nested unit tests commit ef36fdf Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Wed Mar 4 12:15:00 2020 -0800 renamed to tryResolve commit 5703f02 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Wed Mar 4 12:07:18 2020 -0800 rm generator commit 658cb1a Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Wed Mar 4 12:01:54 2020 -0800 implemented more review suggestions commit 0056ee3 Merge: cae89c3 0f0f82a Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Wed Mar 4 11:54:23 2020 -0800 Merge branch 'master' of github.com:PolymerLabs/arcs into r2p commit 0f0f82a Author: Maria Mandlis <mmandlis@chromium.org> Date: Wed Mar 4 09:33:29 2020 -0800 add queryable capability (#4794) commit d032c75 Author: Maria Mandlis <mmandlis@chromium.org> Date: Wed Mar 4 09:23:07 2020 -0800 add creatimeTimestamp to entities (kt) (#4823) commit 30267fc Author: jblebrun <jibbl@google.com> Date: Wed Mar 4 09:09:39 2020 -0800 Remove some unused deps (#4831) commit c519181 Author: Cameron Silvestrini <csilvestrini@users.noreply.github.com> Date: Wed Mar 4 17:14:07 2020 +1100 Increase size of DatabaseImplTest to medium (#4827) * Increase size of DatabaseImplTest to medium Was a bit flaky. * Check flakiness * Revert presubmit tweak commit fa621c6 Author: jblebrun <jibbl@google.com> Date: Tue Mar 3 22:05:07 2020 -0800 Fix a race condition when setting up `ServiceStore` message channel (#4826) This seems to be the cause of the flakiness in `AndroidAllocatorTest` and `AndroidHandleManagerTest` (#4781) commit 78bf4f0 Author: Gogul Balakrishnan <bgogul@google.com> Date: Tue Mar 3 18:00:49 2020 -0800 Utility to convert type proto to field type (if possible). (#4814) commit cae89c3 Merge: 0360752 e587539 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 15:50:34 2020 -0800 Merge branch 'master' of github.com:PolymerLabs/arcs into r2p commit e587539 Author: jblebrun <jibbl@google.com> Date: Tue Mar 3 15:07:16 2020 -0800 Apply simplified test pattern tests in `arcs.android.host` (#4821) As in 5c6310d commit 1af1aac Author: Joshua Pratt <jp10010101010000@gmail.com> Date: Wed Mar 4 09:48:15 2020 +1100 Multinomials (#4804) * multinomial * added tests * making constant readable again * reindent * added multivariate rearrangement tests * updated comment * pr comments: Co-authored-by: Ragav Sachdeva <ragavsachdeva007@gmail.com> commit 0360752 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 11:42:31 2020 -0800 fix, bad comparison op commit 4b05a28 Merge: f4c1d26 99000c2 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 11:18:14 2020 -0800 Merge branch 'r2p' of github.com:alxrsngrtn/arcs into r2p commit f4c1d26 Merge: 140111f 5c6310d Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 11:14:35 2020 -0800 Merge branch 'master' of github.com:PolymerLabs/arcs into r2p Fixed lint errors commit 99000c2 Merge: 140111f 5c6310d Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 11:14:35 2020 -0800 Merge branch 'master' of github.com:PolymerLabs/arcs into r2p Fixed lint errors commit 140111f Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 11:08:12 2020 -0800 Removed flatMap, added TODOs and link to GH issue commit 5c6310d Author: jblebrun <jibbl@google.com> Date: Tue Mar 3 11:02:34 2020 -0800 Simplify service test pattern (#4817) * Make a simple test lifecycle registry, instead of creating empty testactivity * Remove use of `runBlockingTest`: according to Kotlin/kotlinx.coroutines#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`. commit 472bc84 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 10:27:56 2020 -0800 Improved build rules commit ca1ebf8 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Tue Mar 3 10:17:13 2020 -0800 Impl suggestsions for r2p commit b5578ea Author: Jason Feinstein <jwf@google.com> Date: Tue Mar 3 09:54:17 2020 -0800 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. commit a695797 Author: Jason Feinstein <jwf@google.com> Date: Mon Mar 2 18:14:07 2020 -0800 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. commit ba7a107 Author: Gogul Balakrishnan <bgogul@google.com> Date: Mon Mar 2 17:51:35 2020 -0800 Add a decoder for PrimitiveTypeProto and an option to disable android constraints in BUILD. (#4793) commit f6df54a Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 15:44:32 2020 -0800 tools/sigh lint commit bede8d9 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 14:52:02 2020 -0800 Fixed build rule, simplified runtime commit 62dcc57 Author: Alex Rosengarten <alxrsngrtn@google.com> Date: Mon Mar 2 14:38:14 2020 -0800 Fixed test, added TODO * fixed nested tests * rewording, reorganizing * assert more ergonomic
diff --git a/src/tools/storage-key-recipe-resolver.ts b/src/tools/storage-key-recipe-resolver.ts
@@ -32,13 +32,14 @@ export class StorageKeyRecipeResolver {
   /**
    * Produces resolved recipes with storage keys.
    *
-   * TODO(alxr): Apply to long-running recipes appropriately.
+   * TODO(#4818) Add passes to assign storage keys.
    * @throws Error if recipe fails to resolve on first or second pass.
    * @yields Resolved recipes with storage keys
    */
   async resolve(): Promise<Recipe[]> {
     const recipes = [];
     for (const recipe of this.runtime.context.allRecipes) {
+      this.validateHandles(recipe);
       const arc = this.runtime.newArc(this.getArcId(recipe), ramDiskStorageKeyPrefixForTest());
       const opts = {errors: new Map<Recipe | RecipeComponent, string>()};
       const resolved = await this.tryResolve(recipe, arc, opts);
@@ -63,7 +64,8 @@ export class StorageKeyRecipeResolver {
    */
   async tryResolve(recipe: Recipe, arc: Arc, opts?: IsValidOptions): Promise<Recipe | null> {
     const normalized = recipe.clone();
-    normalized.normalize();
+    const successful = normalized.normalize(opts);
+    if (!successful) return null;
     if (normalized.isResolved()) return normalized;
 
     return await (new RecipeResolver(arc).resolve(recipe, opts));
@@ -97,14 +99,14 @@ export class StorageKeyRecipeResolver {
   }
 
   /**
-   * TODO(#4818) method to match `map` and `copy` fated handles with storage keys from `create` handles.
+   * Checks that handles are existent, disambiguous, and initiated by a long-running arc.
    *
-   * @throws when a mapped handle is associated with too many stores (ambiguous mapping).
-   * @throws when a mapped handle isn't associated with any store (no matches found).
-   * @throws when handle is mapped to a handle from an ephemeral recipe.
+   * @throws when a map or copy handle is associated with too many stores (ambiguous mapping).
+   * @throws when a map or copy handle isn't associated with any store (no matches found).
+   * @throws when a map or copy handle is associated with a handle from an ephemeral recipe.
    * @param recipe long-running or ephemeral recipe
    */
-  matchKeysToHandles(recipe: Recipe) {
+  validateHandles(recipe: Recipe) {
     recipe.handles
       .filter(h => h.fate === 'map' || h.fate === 'copy')
       .forEach(handle => {
@@ -121,8 +123,6 @@ export class StorageKeyRecipeResolver {
         if (!match.recipe.isLongRunning) {
           throw Error(`Handle ${handle.localName} mapped to ephemeral handle ${match.localName}.`);
         }
-
-        handle.storageKey = match.storageKey;
       });
   }
 }
diff --git a/src/tools/tests/storage-key-recipe-resolver-test.ts b/src/tools/tests/storage-key-recipe-resolver-test.ts
@@ -11,10 +11,11 @@
 import {Manifest} from '../../runtime/manifest.js';
 import {assert} from '../../platform/chai-node.js';
 import {StorageKeyRecipeResolver} from '../storage-key-recipe-resolver.js';
+import {assertThrowsAsync} from '../../testing/test-util.js';
 
 describe('recipe2plan', () => {
   describe('storage-key-recipe-resolver', () => {
-    it('Resolves mapping a handle from a long running arc into another long running arc', async () => {
+    it('resolves mapping a handle from a long running arc into another long running arc', async () => {
       const manifest = await Manifest.parse(`\
     particle Reader
       data: reads Thing {name: Text}
@@ -28,33 +29,134 @@ describe('recipe2plan', () => {
     
     @trigger
       launch startup
-      arcId myArcId
+      arcId writeArcId
     recipe WritingRecipe
       thing: create persistent 'my-handle-id' 
       Writer
         data: writes thing
 
     @trigger
       launch startup
-      arcId otherArcId
+      arcId readArcId
     recipe ReadingRecipe
       data: map 'my-handle-id'
       Reader
         data: reads data`);
 
       const resolver = new StorageKeyRecipeResolver(manifest);
-      for  (const it of (await resolver.resolve())) {
+      for (const it of (await resolver.resolve())) {
         assert.isTrue(it.isResolved());
       }
     });
+    it('fails to resolve mapping a handle from a short running arc into another short running arc', async () => {
+      const manifest = await Manifest.parse(`\
+    particle Reader
+      data: reads Thing {name: Text}
+
+    particle Writer
+       data: writes Thing {name: Text}
+    
+    recipe WritingRecipe
+      thing: create persistent 'my-handle-id' 
+      Writer
+        data: writes thing
+
+    recipe ReadingRecipe
+      data: map 'my-handle-id'
+      Reader
+        data: reads data`);
+
+      const resolver = new StorageKeyRecipeResolver(manifest);
+      await assertThrowsAsync(async () => await resolver.resolve(), Error, 'Handle data mapped to ephemeral handle thing.');
+    });
+    it('fails to resolve mapping a handle from a short running arc into a long running arc', async () => {
+      const manifest = await Manifest.parse(`\
+    particle Reader
+      data: reads Thing {name: Text}
+
+    particle Writer
+       data: writes Thing {name: Text}
+    
+    recipe WritingRecipe
+      thing: create persistent 'my-handle-id' 
+      Writer
+        data: writes thing
+
+    @trigger
+      launch startup
+      arcId readArcId
+    recipe ReadingRecipe
+      data: map 'my-handle-id'
+      Reader
+        data: reads data`);
+
+      const resolver = new StorageKeyRecipeResolver(manifest);
+      await assertThrowsAsync(async () => await resolver.resolve(), Error, 'Handle data mapped to ephemeral handle thing.');
+    });
+    it('resolves mapping a handle from a long running arc into a short running arc', async () => {
+      const manifest = await Manifest.parse(`\
+    particle Reader
+      data: reads Thing {name: Text}
+
+    particle Writer
+       data: writes Thing {name: Text}
+    
+    @trigger
+      launch startup
+      arcId writeArcId
+    recipe WritingRecipe
+      thing: create persistent 'my-handle-id' 
+      Writer
+        data: writes thing
+
+    recipe ReadingRecipe
+      data: map 'my-handle-id'
+      Reader
+        data: reads data`);
+
+      const resolver = new StorageKeyRecipeResolver(manifest);
+      for (const it of await resolver.resolve()) {
+        assert.isTrue(it.isResolved());
+      }
+    });
+    it('Invalid Type: If Reader reads {name: Text, age: Number} it is not valid', async () => {
+      const manifest = await Manifest.parse(`\
+    particle Reader
+      data: reads Thing {name: Text, age: Number}
+
+    particle Writer
+       data: writes Thing {name: Text}
+    
+    @trigger
+      launch startup
+      arcId writeArcId
+    recipe WritingRecipe
+      thing: create persistent 'my-handle-id' 
+      Writer
+        data: writes thing
+
+    @trigger
+      launch startup
+      arcId readArcId
+    recipe ReadingRecipe
+      data: map 'my-handle-id'
+      Reader
+        data: reads data`);
+
+      const resolver = new StorageKeyRecipeResolver(manifest);
+      // TODO: specify the correct error to be thrown
+      await assertThrowsAsync(resolver.resolve);
+    });
     // TODO(alxr): Flush out outlined unit tests
-    it.skip('Short + Short: If WritingRecipe is short lived, it is not valid', () => {});
-    it.skip('Short + Long: If WritingRecipe is short lived and Reading is long lived, it is not valid', () => {});
-    it.skip('Invalid Type: If Reader reads {name: Text, age: Number} it is not valid', () => {});
-    it.skip('No arc id: If arcId of WritingRecipe is not there, it is not valid', () => {});
-    it.skip('No handleId: If id of handle in WritingRecipe is not provided, it is not valid', () => {});
-    it.skip('Ambiguous handle: If there are 2 WritingRecipes creating the same handle, it is not valid', () => {});
-    it.skip('Ambiguous handle + tag disambiguation: If there are 2 WritingRecipes creating the same handle but with different tags and mapping uses one of the tags, it is valid', () => {});
-    it.skip('No Handle: If there is no writing handle, it is not valid', () => {});
+    it.skip('No arc id: If arcId of WritingRecipe is not there, it is not valid', () => {
+    });
+    it.skip('No handleId: If id of handle in WritingRecipe is not provided, it is not valid', () => {
+    });
+    it.skip('Ambiguous handle: If there are 2 WritingRecipes creating the same handle, it is not valid', () => {
+    });
+    it.skip('Ambiguous handle + tag disambiguation: If there are 2 WritingRecipes creating the same handle but with different tags and mapping uses one of the tags, it is valid', () => {
+    });
+    it.skip('No Handle: If there is no writing handle, it is not valid', () => {
+    });
   });
 });
