Skip to content

Commit

Permalink
Merge pull request #8100 from Agoric/gibson-8098-store-set-no-ordinal
Browse files Browse the repository at this point in the history 
fix(swingset-liveslots): Throw a "not found" error for store.set(missingRemotable, val)
  • Loading branch information
@mergify
mergify[bot] authored Sep 20, 2023
2 parents 3f47cf0 + 5a06ded commit 4a1da4c
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 35 deletions.
68 changes: 33 additions & 35 deletions packages/swingset-liveslots/src/collectionManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,14 @@ function throwNotDurable(value, slotIndex, serializedValue) {
Fail`value is not durable: ${value} at slot ${q(slotIndex)} of ${serializedValue.body}`;
}

function failNotFound(key, label) {
Fail`key ${key} not found in collection ${q(label)}`;
}

function failNotIterable(value) {
Fail`provided data source is not iterable: ${value}`;
}

function prefixc(collectionID, dbEntryKey) {
return `vc.${collectionID}.${dbEntryKey}`;
}
Expand Down Expand Up @@ -339,24 +347,26 @@ export function makeCollectionManager(
const { keyShape } = getSchema();
if (!matches(key, keyShape)) {
return false;
}
if (passStyleOf(key) === 'remotable') {
} else if (passStyleOf(key) === 'remotable') {
return getOrdinal(key) !== undefined;
} else {
return syscall.vatstoreGet(keyToDBKey(key)) !== undefined;
}
}

function mustGet(key, label) {
if (passStyleOf(key) === 'remotable' && getOrdinal(key) === undefined) {
failNotFound(key, label);
}
const dbKey = keyToDBKey(key);
const result = syscall.vatstoreGet(dbKey) || failNotFound(key, label);
return { dbKey, result };
}

function get(key) {
const { keyShape, label } = getSchema();
mustMatch(key, keyShape, makeInvalidKeyTypeMsg(label));
if (passStyleOf(key) === 'remotable' && getOrdinal(key) === undefined) {
throw Fail`key ${key} not found in collection ${q(label)}`;
}
const result = syscall.vatstoreGet(keyToDBKey(key));
if (!result) {
throw Fail`key ${key} not found in collection ${q(label)}`;
}
const { result } = mustGet(key, label);
return unserializeValue(JSON.parse(result));
}

Expand Down Expand Up @@ -427,9 +437,7 @@ export function makeCollectionManager(
}
}
}
const dbKey = keyToDBKey(key);
const rawBefore = syscall.vatstoreGet(dbKey);
rawBefore || Fail`key ${key} not found in collection ${q(label)}`;
const { dbKey, result: rawBefore } = mustGet(key, label);
const before = JSON.parse(rawBefore);
vrm.updateReferenceCounts(before.slots, after.slots);
syscall.vatstoreSet(dbKey, JSON.stringify(after));
Expand All @@ -438,12 +446,7 @@ export function makeCollectionManager(
function deleteInternal(key) {
const { keyShape, label } = getSchema();
mustMatch(key, keyShape, makeInvalidKeyTypeMsg(label));
if (passStyleOf(key) === 'remotable' && getOrdinal(key) === undefined) {
throw Fail`key ${key} not found in collection ${q(label)}`;
}
const dbKey = keyToDBKey(key);
const rawValue = syscall.vatstoreGet(dbKey);
rawValue || Fail`key ${key} not found in collection ${q(label)}`;
const { dbKey, result: rawValue } = mustGet(key, label);
const value = JSON.parse(rawValue);
const doMoreGC1 = value.slots.map(vrm.removeReachableVref).some(b => b);
syscall.vatstoreDelete(dbKey);
Expand Down Expand Up @@ -479,18 +482,15 @@ export function makeCollectionManager(
const end = prefix(coverEnd); // exclusive

const generationAtStart = currentGenerationNumber;
function checkGen() {
if (generationAtStart !== currentGenerationNumber) {
Fail`keys in store cannot be added to during iteration`;
}
}
const checkGen = () =>
currentGenerationNumber === generationAtStart ||
Fail`keys in store cannot be added to during iteration`;

const needToMatchKey = !matchAny(keyPatt);
const needToMatchValue = !matchAny(valuePatt);

// we always get the dbKey, but we might not need to unserialize it
// we might not need to unserialize the dbKey or get the dbValue
const needKeys = yieldKeys || needToMatchKey;
// we don't always need the dbValue
const needValues = yieldValues || needToMatchValue;

/**
Expand Down Expand Up @@ -631,11 +631,10 @@ export function makeCollectionManager(

const addAllToSet = elems => {
if (typeof elems[Symbol.iterator] !== 'function') {
if (Object.isFrozen(elems) && isCopySet(elems)) {
elems = getCopySetKeys(elems);
} else {
Fail`provided data source is not iterable: ${elems}`;
}
elems =
Object.isFrozen(elems) && isCopySet(elems)
? getCopySetKeys(elems)
: failNotIterable(elems);
}
for (const elem of elems) {
addToSet(elem);
Expand All @@ -644,11 +643,10 @@ export function makeCollectionManager(

const addAllToMap = mapEntries => {
if (typeof mapEntries[Symbol.iterator] !== 'function') {
if (Object.isFrozen(mapEntries) && isCopyMap(mapEntries)) {
mapEntries = getCopyMapEntries(mapEntries);
} else {
Fail`provided data source is not iterable: ${mapEntries}`;
}
mapEntries =
Object.isFrozen(mapEntries) && isCopyMap(mapEntries)
? getCopyMapEntries(mapEntries)
: failNotIterable(mapEntries);
}
for (const [key, value] of mapEntries) {
if (has(key)) {
Expand Down
14 changes: 14 additions & 0 deletions packages/swingset-liveslots/test/test-collections.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ function exerciseMapOperations(t, collectionName, testStore) {
() => testStore.set(86, 'not work'),
m(`key 86 not found in collection "${collectionName}"`),
);
t.throws(
() => testStore.set(somethingMissing, 'not work'),
m(
`key "[Alleged: something missing]" not found in collection "${collectionName}"`,
),
);
t.throws(
() => testStore.init(47, 'already there'),
m(`key 47 already registered in collection "${collectionName}"`),
Expand All @@ -118,11 +124,17 @@ function exerciseMapOperations(t, collectionName, testStore) {
t.is(testStore.get(somethingElse), something);

testStore.delete(47);
testStore.delete(something);
t.falsy(testStore.has(47));
t.falsy(testStore.has(something));
t.throws(
() => testStore.get(47),
m(`key 47 not found in collection "${collectionName}"`),
);
t.throws(
() => testStore.get(something),
m(`key "[Alleged: something]" not found in collection "${collectionName}"`),
);
t.throws(
() => testStore.delete(22),
m(`key 22 not found in collection "${collectionName}"`),
Expand All @@ -147,7 +159,9 @@ function exerciseSetOperations(t, collectionName, testStore) {
t.notThrows(() => testStore.add(47));

testStore.delete(47);
testStore.delete(something);
t.falsy(testStore.has(47));
t.falsy(testStore.has(something));
t.throws(
() => testStore.delete(22),
m(`key 22 not found in collection "${collectionName}"`),
Expand Down

0 comments on commit 4a1da4c

Please sign in to comment.