add (failing) tests

packages/SwingSet/docs/virtual-objects.md

@@ -254,14 +254,15 @@ To support this, the four `defineKind` functions accept `currentVersion` and `up
 
 and these version annotations will remain in place until the object is upgraded or deleted.
 
-`upgradeState` is a synchronous function which takes `(oldVersion, oldState)` and is responsible for returning `newState`. Every time an old record is accessed (i.e. when a behavior method is invoked, to supply it with `context.state`), if the record's version does not match `currentVersion`, the upgrade function is called. The `newState` is immediately stored back into the DB, with the new version number (so the migration only happens once per record).
+`upgradeState` is a synchronous function which takes `(oldVersion, oldState)` and is responsible for returning the new state. Every time an old record is accessed (i.e. when a behavior method is invoked, to supply it with `context.state`), if the record's version does not match `currentVersion`, the upgrade function is called. The new `state` is immediately stored back into the DB, with the new version number (so the migration only happens once per record).
+
+The actual return value of `upgradeState` is `{ version, state }`. The upgrade process asserts that the returned `version` is equal to `currentVersion`, to catch mistakes where the upgrade function skips a step or has not been updated to match the Kind definition.
 
 `upgradeState` must be prepared to handle data from any historical version. To avoid gaps, authors are encouraged to use a pattern which retains every single-step delta, like this:
 
 ```js
-function upgradeState(oldVersion, oldState) {
-  let version = oldVersion;
-  let state = copy(oldState);
+function upgradeState(version, oldState) {
+  let state = { ...oldState }; // shallowly-mutable copy
   // add comment here describing initial schema
   if (version === 0) {
     state.newThing = INITIAL_VALUE;
@@ -275,12 +276,11 @@ function upgradeState(oldVersion, oldState) {
   // add comment here describing schema for version 2
   // in the future: add a new clause here for each new version
 
-  assert.equal(version, 2); // to match current `currentVersion`
-  return state;
+  return { version, state };
 }
 ```
 
-(TODO: consider returning `{ state, version }`, and have the VOM `assert.equal(version, kind.currentVersion)`, to catch accidents where `currentVersion` is updated but `upgradeState` is not, or failures inside `upgradeState` that skip a step)
+(TODO: consider making `oldState` a shallow-mutable object, instead of a fully hardened object, to make it easier to add/remove top-level properties. However it wouldn't help with deeper mutations. Naming it `state` might make it look like it could be modified in-place, and I think it's better to require it as a return value.)
 
 The current version's `stateShape` constraint is enforced upon the return value from any calls to `upgradeState` during that incarnation, in addition to `initialize` state (for new objects) and the state that results when behavior methods mutate their `state`.
 

packages/swingset-liveslots/test/virtual-objects/test-state-upgrade.js

@@ -0,0 +1,206 @@
+// @ts-nocheck
+/* eslint-disable no-underscore-dangle */
+
+import test from 'ava';
+import '@endo/init/debug.js';
+
+import { Far } from '@endo/marshal';
+import { M } from '@agoric/store';
+import { makeLiveSlots } from '../../src/liveslots.js';
+import { kser } from '../kmarshal.js';
+import { buildSyscall } from '../liveslots-helpers.js';
+import { makeStartVat } from '../util.js';
+import { makeMockGC } from '../mock-gc.js';
+
+const init = value => ({ ...value });
+const behavior = {
+  set: ({ state }, name, value) => (state[name] = value),
+  get: ({ state }, name) => state[name],
+};
+const stateShape0 = { prop1: M.string };
+const stateShape1 = { prop1: M.string, prop2: M.number() };
+const stateShape2 = { prop1renamed: M.string, prop2: M.number() };
+
+async function withNewIncarnation(kvStore, func) {
+  async function buildRootObject(vatPowers, _vatParameters, baggage) {
+    await func(vatPowers.VatData, baggage);
+    return Far('root', {});
+  }
+  const makeNS = () => ({ buildRootObject });
+  const { syscall } = buildSyscall({ kvStore });
+  const gcTools = makeMockGC();
+  const ls = makeLiveSlots(syscall, 'vatA', {}, {}, gcTools, undefined, makeNS);
+  await ls.dispatch(makeStartVat(kser())); // TODO: needed?
+}
+
+test('check helper function', async t => {
+  const kvStore = new Map();
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    const thingHandle = VatData.makeKindHandle('thing');
+    baggage.init('handle', thingHandle);
+    const makeV0 = VatData.defineDurableKind(thingHandle, init, behavior);
+    baggage.init('thingA', makeV0({ prop1: 'valueA' }));
+    baggage.init('thingB', makeV0({ prop1: 'valueB' }));
+
+    t.is(baggage.get('thingA').get('prop1'), 'valueA');
+    t.is(baggage.get('thingA').get('prop2'), undefined);
+    t.is(baggage.get('thingB').get('prop1'), 'valueB');
+    t.is(baggage.get('thingB').get('prop2'), undefined);
+  });
+
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    const thingHandle = baggage.get('handle');
+    const _makeV1 = VatData.defineDurableKind(thingHandle, init, behavior);
+    t.is(baggage.get('thingA').get('prop1'), 'valueA');
+    t.is(baggage.get('thingA').get('prop2'), undefined);
+    t.is(baggage.get('thingB').get('prop1'), 'valueB');
+    t.is(baggage.get('thingB').get('prop2'), undefined);
+  });
+});
+
+test.failing('upgrade state', async t => {
+  const kvStore = new Map();
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    const thingHandle = VatData.makeKindHandle('thing');
+    baggage.init('handle', thingHandle);
+    const makeV0 = VatData.defineDurableKind(thingHandle, init, behavior);
+    baggage.init('thingA', makeV0({ prop1: 'valueA' }));
+    baggage.init('thingB', makeV0({ prop1: 'valueB' }));
+
+    t.is(baggage.get('thingA').get('prop1'), 'valueA');
+    t.is(baggage.get('thingA').get('prop2'), undefined);
+    t.is(baggage.get('thingB').get('prop1'), 'valueB');
+    t.is(baggage.get('thingB').get('prop2'), undefined);
+  });
+
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    function upgrader1(version, oldState) {
+      const state = { ...oldState };
+      if (version === 0) {
+        state.prop2 = 0;
+        version = 1;
+      }
+      return { version, state };
+    }
+    const thingHandle = baggage.get('handle');
+    const _makeV1 = VatData.defineDurableKind(thingHandle, init, behavior, {
+      currentVersion: 1,
+      upgradeState: upgrader1,
+      stateShape: stateShape1,
+    });
+
+    t.is(baggage.get('thingA').get('prop1'), 'valueA');
+    t.is(baggage.get('thingA').get('prop2'), 0);
+    baggage.get('thingA').set('prop2', 5);
+  });
+
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    function upgrader2(version, oldState) {
+      const state = { ...oldState };
+      if (version === 0) {
+        state.prop2 = 0;
+        version = 1;
+      }
+      if (version === 1) {
+        state.prop1renamed = state.prop1;
+        delete state.prop1;
+        state.prop2 *= 10;
+        version = 2;
+      }
+      return { version, state };
+    }
+    const thingHandle = baggage.get('handle');
+    const _makeV2 = VatData.defineDurableKind(thingHandle, init, behavior, {
+      currentVersion: 2,
+      upgradeState: upgrader2,
+      stateShape: stateShape2,
+    });
+
+    t.is(baggage.get('thingA').get('prop1'), undefined);
+    t.is(baggage.get('thingA').get('prop1renamed'), 'valueA');
+    t.is(baggage.get('thingA').get('prop2'), 50);
+    // thingB gets upgraded from 0->2 in a single ugprader2() call
+    t.is(baggage.get('thingB').get('prop1'), undefined);
+    t.is(baggage.get('thingB').get('prop1renamed'), 'valueB');
+    t.is(baggage.get('thingB').get('prop2'), 0);
+  });
+});
+
+test.failing('upgrader throws error', async t => {
+  const kvStore = new Map();
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    const thingHandle = VatData.makeKindHandle('thing');
+    baggage.init('handle', thingHandle);
+    const makeV0 = VatData.defineDurableKind(thingHandle, init, behavior);
+    baggage.init('thingA', makeV0({ prop1: 'valueA' }));
+  });
+
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    function upgrader1(_version, _oldState) {
+      throw Error('error during upgrade');
+    }
+    const thingHandle = baggage.get('handle');
+    const _makeV1 = VatData.defineDurableKind(thingHandle, init, behavior, {
+      currentVersion: 1,
+      upgradeState: upgrader1,
+    });
+
+    t.throws(() => baggage.get('thingA').get('prop1'), {
+      message: /error during upgrade/,
+    });
+  });
+});
+
+test.failing('upgrader makes wrong version', async t => {
+  const kvStore = new Map();
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    const thingHandle = VatData.makeKindHandle('thing');
+    baggage.init('handle', thingHandle);
+    const makeV0 = VatData.defineDurableKind(thingHandle, init, behavior);
+    baggage.init('thingA', makeV0({ prop1: 'valueA' }));
+  });
+
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    function upgrader1(version, oldState) {
+      return { version, state: oldState };
+    }
+    const thingHandle = baggage.get('handle');
+    const _makeV1 = VatData.defineDurableKind(thingHandle, init, behavior, {
+      currentVersion: 1,
+      upgradeState: upgrader1,
+    });
+
+    t.throws(() => baggage.get('thingA').get('prop1'), {
+      message: /XXX error during upgrade/,
+    });
+  });
+});
+
+test.failing('upgrader does not match state shape', async t => {
+  const kvStore = new Map();
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    const thingHandle = VatData.makeKindHandle('thing');
+    baggage.init('handle', thingHandle);
+    const makeV0 = VatData.defineDurableKind(thingHandle, init, behavior, {
+      stateShape: stateShape0,
+    });
+    baggage.init('thingA', makeV0({ prop1: 'valueA' }));
+  });
+
+  await withNewIncarnation(kvStore, (VatData, baggage) => {
+    function upgrader1(version, oldState) {
+      // missing prop2, which stateShape1 requires
+      return { version: 1, state: oldState };
+    }
+    const thingHandle = baggage.get('handle');
+    const _makeV1 = VatData.defineDurableKind(thingHandle, init, behavior, {
+      currentVersion: 1,
+      upgradeState: upgrader1,
+      stateShape: stateShape1,
+    });
+
+    t.throws(() => baggage.get('thingA').get('prop1'), {
+      message: /XXX error during upgrade/,
+    });
+  });
+});