diff --git a/golang/cosmos/x/vstorage/vstorage.go b/golang/cosmos/x/vstorage/vstorage.go index b2120948a30..f497a9cf4df 100644 --- a/golang/cosmos/x/vstorage/vstorage.go +++ b/golang/cosmos/x/vstorage/vstorage.go @@ -31,14 +31,35 @@ func NewStorageHandler(keeper Keeper) vstorageHandler { return vstorageHandler{keeper: keeper} } -func unmarshalSinglePathFromArgs(args []json.RawMessage, path *string) error { +func unmarshalString(jsonText json.RawMessage) (string, error) { + var str *string + if err := json.Unmarshal(jsonText, &str); err != nil { + return "", err + } else if str == nil { + return "", fmt.Errorf("cannot unmarshal `null` into string") + } + return *str, nil +} + +func unmarshalSinglePathFromArgs(args []json.RawMessage) (string, error) { if len(args) == 0 { - return fmt.Errorf("missing 'path' argument") + return "", fmt.Errorf("missing 'path' argument") + } else if len(args) != 1 { + return "", fmt.Errorf("extra arguments after 'path'") } - if len(args) != 1 { - return fmt.Errorf("extra arguments after 'path'") + return unmarshalString(args[0]) +} + +func unmarshalPathsFromArgs(args []json.RawMessage) ([]string, error) { + paths := make([]string, len(args)) + for i, arg := range args { + path, err := unmarshalString(arg) + if err != nil { + return nil, err + } + paths[i] = path } - return json.Unmarshal(args[0], path) + return paths, nil } func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret string, err error) { @@ -104,6 +125,17 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s } return "true", nil + case "delete": + paths, err := unmarshalPathsFromArgs(msg.Args) + if err != nil { + return "", err + } + for _, path := range paths { + newEntry := types.NewStorageEntryWithNoData(path) + keeper.SetStorageAndNotify(cctx.Context, newEntry) + } + return "true", nil + case "append": for _, arg := range msg.Args { var entry types.StorageEntry @@ -124,10 +156,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s case "get": // Note that "get" does not (currently) unwrap a StreamCell. - var path string - err = unmarshalSinglePathFromArgs(msg.Args, &path) + path, err := unmarshalSinglePathFromArgs(msg.Args) if err != nil { - return + return "", err } entry := keeper.GetEntry(cctx.Context, path) @@ -141,10 +172,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s return string(bz), nil case "getStoreKey": - var path string - err = unmarshalSinglePathFromArgs(msg.Args, &path) + path, err := unmarshalSinglePathFromArgs(msg.Args) if err != nil { - return + return "", err } value := vstorageStoreKey{ StoreName: keeper.GetStoreName(), @@ -159,10 +189,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s return string(bz), nil case "has": - var path string - err = unmarshalSinglePathFromArgs(msg.Args, &path) + path, err := unmarshalSinglePathFromArgs(msg.Args) if err != nil { - return + return "", err } value := keeper.HasStorage(cctx.Context, path) if !value { @@ -172,10 +201,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s // TODO: "keys" is deprecated case "children", "keys": - var path string - err = unmarshalSinglePathFromArgs(msg.Args, &path) + path, err := unmarshalSinglePathFromArgs(msg.Args) if err != nil { - return + return "", err } children := keeper.GetChildren(cctx.Context, path) if children.Children == nil { @@ -188,10 +216,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s return string(bytes), nil case "entries": - var path string - err = unmarshalSinglePathFromArgs(msg.Args, &path) + path, err := unmarshalSinglePathFromArgs(msg.Args) if err != nil { - return + return "", err } children := keeper.GetChildren(cctx.Context, path) entries := make([][]interface{}, len(children.Children)) @@ -210,10 +237,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s return string(bytes), nil case "values": - var path string - err = unmarshalSinglePathFromArgs(msg.Args, &path) + path, err := unmarshalSinglePathFromArgs(msg.Args) if err != nil { - return + return "", err } children := keeper.GetChildren(cctx.Context, path) vals := make([]string, len(children.Children)) @@ -227,10 +253,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s return string(bytes), nil case "size": - var path string - err = unmarshalSinglePathFromArgs(msg.Args, &path) + path, err := unmarshalSinglePathFromArgs(msg.Args) if err != nil { - return + return "", err } children := keeper.GetChildren(cctx.Context, path) if children.Children == nil { diff --git a/golang/cosmos/x/vstorage/vstorage_test.go b/golang/cosmos/x/vstorage/vstorage_test.go index 02e478aea09..08988ac487c 100644 --- a/golang/cosmos/x/vstorage/vstorage_test.go +++ b/golang/cosmos/x/vstorage/vstorage_test.go @@ -88,7 +88,8 @@ func TestGetAndHas(t *testing.T) { {label: "empty non-terminal", args: []interface{}{"top.empty-non-terminal"}, want: `null`}, {label: "no entry", args: []interface{}{"nosuchpath"}, want: `null`}, {label: "empty args", args: []interface{}{}, errContains: ptr(`missing`)}, - {label: "non-string arg", args: []interface{}{42}, errContains: ptr(`json`)}, + {label: "number arg", args: []interface{}{42}, errContains: ptr(`json`)}, + {label: "null arg", args: []interface{}{nil}, errContains: ptr(`null`)}, {label: "extra args", args: []interface{}{"foo", "bar"}, errContains: ptr(`extra`)}, } for _, desc := range cases { @@ -123,12 +124,13 @@ func TestGetAndHas(t *testing.T) { } } -func doTestSet(t *testing.T, method string, expectNotify bool) { +func doTestSetAndDelete(t *testing.T, setMethod string, expectNotify bool) { kit := makeTestKit() keeper, handler, ctx, cctx := kit.keeper, kit.handler, kit.ctx, kit.cctx type testCase struct { label string + method string args []interface{} errContains *string skipReadBack map[int]bool @@ -144,23 +146,42 @@ func doTestSet(t *testing.T, method string, expectNotify bool) { args: []interface{}{[]string{"baz.a", "qux"}, []string{"baz.b", "qux"}}, }, {label: "other multi-value", - args: []interface{}{[]string{"qux", "A"}, []string{"quux", "B"}}, + args: []interface{}{[]string{"qux", "A"}, []string{"quux", "B"}, []string{"ephemeral", "C"}}, }, {label: "overwrites", args: []interface{}{[]string{"bar"}, []string{"quux", "new"}}, }, - {label: "non-string path", + {label: "deletes", method: "delete", + args: []interface{}{"final.final.corge", "baz.b", "baz.a"}, + }, + {label: "more deletes", method: "delete", + args: []interface{}{"ephemeral", "notfound", "alsonotfound"}, + }, + {label: "restorations", + args: []interface{}{[]string{"baz.b", "quux"}, []string{"baz.a", "quux"}}, + }, + {label: "number path", // TODO: Fully validate input before making changes // args: []interface{}{[]string{"foo", "X"}, []interface{}{42, "new"}}, args: []interface{}{[]interface{}{42, "new"}}, errContains: ptr("path"), }, + {label: "null path", + // TODO: Fully validate input before making changes + // args: []interface{}{[]string{"foo", "Y"}, []interface{}{nil, "new"}}, + args: []interface{}{[]interface{}{nil, "new"}}, + errContains: ptr("path"), + }, {label: "non-string value", // TODO: Fully validate input before making changes // args: []interface{}{[]string{"foo", "X"}, []interface{}{"foo", true}}, args: []interface{}{[]interface{}{"foo", true}}, errContains: ptr("value"), }, + {label: "null path", method: "delete", + args: []interface{}{nil, "foo"}, + errContains: ptr("null"), + }, {label: "self-overwrite", args: []interface{}{[]string{"final.final.corge", "grault"}, []string{"final.final.corge", "garply"}}, skipReadBack: map[int]bool{0: true}, @@ -175,14 +196,31 @@ func doTestSet(t *testing.T, method string, expectNotify bool) { } // Expect events to be alphabetized by key. expectedFlushEvents := sdk.Events{ - stateChangeEvent("baz.a", "qux"), - stateChangeEvent("baz.b", "qux"), + stateChangeEvent("baz.a", "quux"), + stateChangeEvent("baz.b", "quux"), stateChangeEvent("final.final.corge", "garply"), stateChangeEvent("foo", "bar"), stateChangeEvent("quux", "new"), stateChangeEvent("qux", "A"), } for _, desc := range cases { + method := desc.method + if method == "" { + method = setMethod + } else if method == "delete" && setMethod == "setWithoutNotify" { + // "delete" notifies, so replace it with set-empty when testing *without* notifies. + method = setMethod + for i, arg := range desc.args { + if str, ok := arg.(string); ok { + desc.args[i] = []string{str} + } else { + desc.args[i] = []interface{}{arg} + if desc.errContains != nil { + desc.errContains = ptr("path") + } + } + } + } got, err := callReceive(handler, cctx, method, desc.args) if desc.errContains == nil { @@ -195,6 +233,10 @@ func doTestSet(t *testing.T, method string, expectNotify bool) { // Read the data back. for i, arg := range desc.args { entry, ok := arg.([]string) + if method == "delete" { + path, pathOk := arg.(string) + entry, ok = []string{path}, pathOk + } if desc.skipReadBack[i] { continue } else if !ok { @@ -224,12 +266,12 @@ func doTestSet(t *testing.T, method string, expectNotify bool) { // Verify corresponding events. if got := ctx.EventManager().Events(); !reflect.DeepEqual(got, sdk.Events{}) { - t.Errorf("%s got unexpected events before flush %#v", method, got) + t.Errorf("%s got unexpected events before flush %#v", setMethod, got) } keeper.FlushChangeEvents(ctx) if !expectNotify { if got := ctx.EventManager().Events(); !reflect.DeepEqual(got, sdk.Events{}) { - t.Errorf("%s got unexpected events after flush %#v", method, got) + t.Errorf("%s got unexpected events after flush %#v", setMethod, got) } } else if got := ctx.EventManager().Events(); !reflect.DeepEqual(got, expectedFlushEvents) { for _, evt := range got { @@ -237,18 +279,18 @@ func doTestSet(t *testing.T, method string, expectNotify bool) { for i, attr := range evt.Attributes { attrs[i] = fmt.Sprintf("%s=%q", attr.Key, attr.Value) } - t.Logf("%s got event %s<%s>", method, evt.Type, strings.Join(attrs, ", ")) + t.Logf("%s got event %s<%s>", setMethod, evt.Type, strings.Join(attrs, ", ")) } - t.Errorf("%s got after flush events %#v; want %#v", method, got, expectedFlushEvents) + t.Errorf("%s got after flush events %#v; want %#v", setMethod, got, expectedFlushEvents) } } -func TestSet(t *testing.T) { - doTestSet(t, "set", true) +func TestSetAndDelete(t *testing.T) { + doTestSetAndDelete(t, "set", true) } -func TestSetWithoutNotify(t *testing.T) { - doTestSet(t, "setWithoutNotify", false) +func TestSetWithoutNotifyAndDelete(t *testing.T) { + doTestSetAndDelete(t, "setWithoutNotify", false) } // TODO: TestAppend diff --git a/packages/internal/src/lib-chainStorage.js b/packages/internal/src/lib-chainStorage.js index 9f0e53f10db..2f08f69c24c 100644 --- a/packages/internal/src/lib-chainStorage.js +++ b/packages/internal/src/lib-chainStorage.js @@ -36,7 +36,8 @@ const { Fail } = assert; * string-valued data for each node, defaulting to the empty string. * * @typedef {object} StorageNode - * @property {(data: string) => Promise} setValue publishes some data + * @property {(data: string) => Promise} setValue publishes some data to chain storage + * @property {() => Promise} delete removes the chain storage data * @property {() => string} getPath the chain storage path at which the node was constructed * @property {() => Promise} getStoreKey DEPRECATED use getPath * @property {(subPath: string, options?: {sequence?: boolean}) => StorageNode} makeChildNode @@ -44,6 +45,7 @@ const { Fail } = assert; const ChainStorageNodeI = M.interface('StorageNode', { setValue: M.callWhen(M.string()).returns(), + delete: M.callWhen().returns(), getPath: M.call().returns(M.string()), getStoreKey: M.callWhen().returns(M.record()), makeChildNode: M.call(M.string()) @@ -107,18 +109,17 @@ harden(assertPathSegment); * Must match the switch in vstorage.go using `vstorageMessage` type * * @typedef { 'get' | 'getStoreKey' | 'has' | 'children' | 'entries' | 'values' |'size' } StorageGetByPathMessageMethod + * @typedef { 'delete' } StorageMultiPathMessageMethod * @typedef { 'set' | 'setWithoutNotify' | 'append' } StorageUpdateEntriesMessageMethod - * @typedef {StorageGetByPathMessageMethod | StorageUpdateEntriesMessageMethod } StorageMessageMethod + * @typedef { StorageGetByPathMessageMethod | StorageMultiPathMessageMethod | StorageUpdateEntriesMessageMethod } StorageMessageMethod * @typedef { [path: string] } StorageGetByPathMessageArgs + * @typedef { [...paths: string[]] } StorageMultiPathMessageArgs * @typedef { [path: string, value?: string | null] } StorageEntry * @typedef { StorageEntry[] } StorageUpdateEntriesMessageArgs - * @typedef {{ - * method: StorageGetByPathMessageMethod; - * args: StorageGetByPathMessageArgs; - * } | { - * method: StorageUpdateEntriesMessageMethod; - * args: StorageUpdateEntriesMessageArgs; - * }} StorageMessage + * @typedef {{ method: StorageGetByPathMessageMethod, args: StorageGetByPathMessageArgs }} StorageGetByPathMessage + * @typedef {{ method: StorageMultiPathMessageMethod, args: StorageMultiPathMessageArgs }} StorageMultiPathMessage + * @typedef {{ method: StorageUpdateEntriesMessageMethod, args: StorageUpdateEntriesMessageArgs }} StorageUpdateEntriesMessage + * @typedef { StorageGetByPathMessage | StorageMultiPathMessage | StorageUpdateEntriesMessage } StorageMessage */ /** @@ -195,6 +196,11 @@ export const prepareChainStorageNode = zone => { args: [entry], }); }, + /** @type {() => Promise} */ + async delete() { + const { path, messenger } = this.state; + await cb.callE(messenger, { method: 'delete', args: [path] }); + }, // Possible extensions: // * getValue() // * getChildNames() and/or makeChildNodes() diff --git a/packages/internal/src/storage-test-utils.js b/packages/internal/src/storage-test-utils.js index c17e6d0a12f..1756f7b2088 100644 --- a/packages/internal/src/storage-test-utils.js +++ b/packages/internal/src/storage-test-utils.js @@ -182,6 +182,15 @@ export const makeFakeStorageKit = (rootPath, rootOptions) => { } break; } + case 'delete': { + trace('toStorage delete', message); + /** @type {string[]} */ + const keys = message.args; + for (const key of keys) { + data.delete(key); + } + break; + } case 'size': // Intentionally incorrect because it counts non-child descendants, // but nevertheless supports a "has children" test. diff --git a/packages/internal/test/test-storage-test-utils.js b/packages/internal/test/test-storage-test-utils.js index 38021821f9f..ad59d30a2c0 100644 --- a/packages/internal/test/test-storage-test-utils.js +++ b/packages/internal/test/test-storage-test-utils.js @@ -189,6 +189,25 @@ test('makeFakeStorageKit', async t => { [{ method: 'set', args: [[childPath]] }], 'child setValue message', ); + + await childNode.delete(); + t.deepEqual( + messages.slice(-1), + [{ method: 'delete', args: [childPath] }], + 'child delete message', + ); + await deepNode.delete(); + t.deepEqual( + messages.slice(-1), + [{ method: 'delete', args: [deepPath] }], + 'granchild delete message', + ); + await childNode.delete(); + t.deepEqual( + messages.slice(-1), + [{ method: 'delete', args: [childPath] }], + 'child delete message', + ); }); test('makeFakeStorageKit sequence data', async t => { @@ -262,6 +281,19 @@ test('makeFakeStorageKit sequence data', async t => { [{ method: 'append', args: [[deepPath, 'qux']] }], 'manual-sequence grandchild setValue message', ); + + await childNode.delete(); + t.deepEqual( + messages.slice(-1), + [{ method: 'delete', args: [childPath] }], + 'child delete message', + ); + await deepNode.delete(); + t.deepEqual( + messages.slice(-1), + [{ method: 'delete', args: [deepPath] }], + 'granchild delete message', + ); }); const testUnmarshaller = test.macro((t, format) => { diff --git a/packages/notifier/src/storesub.js b/packages/notifier/src/storesub.js index 1068fa3379a..5dcfd3e2781 100644 --- a/packages/notifier/src/storesub.js +++ b/packages/notifier/src/storesub.js @@ -140,10 +140,11 @@ export const makeStoredSubscription = ( /** @type {StoredSubscription} */ const storesub = Far('StoredSubscription', { - // @ts-expect-error getStoreKey type does not have `subscription` getStoreKey: async () => { if (!storageNode) { - return harden({ subscription }); + return /** @type {Awaited['getStoreKey']>>} */ ( + harden({ subscription }) + ); } const storeKey = await E(storageNode).getStoreKey(); return harden({ ...storeKey, subscription }); diff --git a/packages/notifier/src/types-ambient.js b/packages/notifier/src/types-ambient.js index c0e7c40db34..8d6a9c36722 100644 --- a/packages/notifier/src/types-ambient.js +++ b/packages/notifier/src/types-ambient.js @@ -317,11 +317,7 @@ * identifies children by restricted ASCII name and is associated with arbitrary * string-valued data for each node, defaulting to the empty string. * - * @typedef {object} StorageNode - * @property {(data: string) => Promise} setValue publishes some data (append to the node) - * @property {() => string} getPath the chain storage path at which the node was constructed - * @property {() => Promise} getStoreKey DEPRECATED use getPath - * @property {(subPath: string, options?: {sequence?: boolean}) => StorageNode} makeChildNode + * @typedef {import('@agoric/internal/src/lib-chainStorage').StorageNode} StorageNode */ /** diff --git a/packages/notifier/tools/testSupports.js b/packages/notifier/tools/testSupports.js index f1eceb40db7..4adfc808edd 100644 --- a/packages/notifier/tools/testSupports.js +++ b/packages/notifier/tools/testSupports.js @@ -28,6 +28,9 @@ export const makeFakeStorage = (path, publication) => { publication.updateState(value); } }, + delete: async () => { + throw Error('not implemented'); + }, makeChildNode: () => storage, countSetValueCalls: () => setValueCalls, });