From f8806f37774f511671b14ee8807f381c9ef2e5e2 Mon Sep 17 00:00:00 2001 From: Brian Warner Date: Fri, 30 Aug 2024 17:33:58 -0700 Subject: [PATCH] fix(liveslots): collection.clear(pattern) vs getSize The `.clear()` method, on (strong) collections, will delete all entries. But it can also be called with `keyPatt` and/or `valuePatt` argument, and only the matching entries will be deleted. Previously, the `|entryCount` metadata field was unconditionally set to zero for any call to `.clear()`, even those with patterns which caused some entries to be retained. This caused a subsequent `collection.getSize()` to return the wrong value, yielding zero even if the collection still had entries. This commit fixes the logic to decrement the `|entryCount` field by the number of entries actually deleted. As an optimization, it is still set directly to zero if `clear()` is not limited by patterns. fixes #10007 --- .../swingset-liveslots/src/collectionManager.js | 15 ++++++++++----- .../swingset-liveslots/test/collections.test.js | 6 ++---- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/swingset-liveslots/src/collectionManager.js b/packages/swingset-liveslots/src/collectionManager.js index 8ee66541b48..9b8b6d4ff2e 100644 --- a/packages/swingset-liveslots/src/collectionManager.js +++ b/packages/swingset-liveslots/src/collectionManager.js @@ -616,16 +616,21 @@ export function makeCollectionManager( function clearInternal(isDeleting, keyPatt, valuePatt) { let doMoreGC = false; - if (isDeleting || (matchAny(keyPatt) && matchAny(valuePatt))) { + if (isDeleting) { doMoreGC = clearInternalFull(); + // |entryCount will be deleted along with metadata keys + } else if (matchAny(keyPatt) && matchAny(valuePatt)) { + doMoreGC = clearInternalFull(); + if (!hasWeakKeys) { + syscall.vatstoreSet(prefix('|entryCount'), '0'); + } } else { + let numDeleted = 0; for (const k of keys(keyPatt, valuePatt)) { + numDeleted += 1; doMoreGC = deleteInternal(k) || doMoreGC; } - } - if (!hasWeakKeys && !isDeleting) { - // TODO: broken, see #10007 - syscall.vatstoreSet(prefix('|entryCount'), '0'); + updateEntryCount(-numDeleted); } return doMoreGC; } diff --git a/packages/swingset-liveslots/test/collections.test.js b/packages/swingset-liveslots/test/collections.test.js index d63192b025c..f4cc62bfbb9 100644 --- a/packages/swingset-liveslots/test/collections.test.js +++ b/packages/swingset-liveslots/test/collections.test.js @@ -503,8 +503,7 @@ test('map clear', t => { t.is(testStore.getSize(), 0); }); -// see #10007 -test.failing('map clear with pattern', t => { +test('map clear with pattern', t => { const testStore = makeScalarBigMapStore('cmap', { keyShape: M.any() }); testStore.init('a', 'ax'); testStore.init('b', 'bx'); @@ -529,8 +528,7 @@ test('set clear', t => { t.is(testStore.getSize(), 0); }); -// see #10007 -test.failing('set clear with pattern', t => { +test('set clear with pattern', t => { const testStore = makeScalarBigSetStore('cset', { keyShape: M.any() }); testStore.add('a'); testStore.add('b');