dont erase a trail msg redundantly

staltz · Feb 20, 2024 · 9075f98 · 9075f98
1 parent 94f5160
commit 9075f98
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 25 deletions.
diff --git a/lib/index.js b/lib/index.js
@@ -55,12 +55,12 @@ function initGC(peer, config) {
     const done = multicb({ pluck: 1 })
 
     /**
-     * @param {string} explanation
+     * @param {string} errExplanation
      */
-    function makeRecCB(explanation) {
+    function makeRecCB(errExplanation) {
       const cb = done()
       return (/**@type {Error=}*/ err) => {
-        if (err) debug('%s: %s', explanation, flattenCauseChain(err))
+        if (err) debug('%s: %s', errExplanation, flattenCauseChain(err))
         cb()
       }
     }
@@ -70,29 +70,41 @@ function initGC(peer, config) {
       if (!rec.msg) continue
       const { id: msgID, msg } = rec
       const [purpose, details] = peer.goals.getMsgPurpose(msgID, msg)
-      if (purpose === 'goal') continue // don't cleanup
-      if (purpose === 'none') {
-        const recCB = makeRecCB('Failed to delete msg when cleaning up')
-        debug('Deleting msg %s with purpose=none', msgID)
-        peer.db.del(msgID, recCB)
-        waiting = true
-      } else if (purpose === 'ghost') {
-        const { tangleID, span } = details
-        const recCB = makeRecCB('Failed to delete ghost msg when cleaning up')
-        // TODO: Could one msg be a ghostable in MANY tangles? Or just one?
-        debug('Deleting and ghosting msg %s with purpose=ghost', msgID)
-        peer.db.ghosts.add({ tangleID, msgID, span }, (err) => {
-          if (err) return recCB(err)
+      switch (purpose) {
+        case 'goal': {
+          continue // don't cleanup
+        }
+        case 'none': {
+          const recCB = makeRecCB('Failed to delete msg when cleaning up')
+          debug('Deleting msg %s with purpose=none', msgID)
           peer.db.del(msgID, recCB)
-        })
-        waiting = true
-      } else if (purpose === 'trail') {
-        const recCB = makeRecCB('Failed to erase trail msg when cleaning up')
-        debug('Erasing msg %s with purpose=trail', msgID)
-        peer.db.erase(msgID, recCB)
-        waiting = true
-      } else {
-        cb(new Error('Unreachable'))
+          waiting = true
+          continue
+        }
+        case 'ghost': {
+          const { tangleID, span } = details
+          const recCB = makeRecCB('Failed to delete ghost msg when cleaning up')
+          // TODO: Could one msg be a ghostable in MANY tangles? Or just one?
+          debug('Deleting and ghosting msg %s with purpose=ghost', msgID)
+          peer.db.ghosts.add({ tangleID, msgID, span }, (err) => {
+            if (err) return recCB(err)
+            peer.db.del(msgID, recCB)
+          })
+          waiting = true
+          continue
+        }
+        case 'trail': {
+          if (!msg.data) continue // it's already erased
+          const recCB = makeRecCB('Failed to erase trail msg when cleaning up')
+          debug('Erasing msg %s with purpose=trail', msgID)
+          peer.db.erase(msgID, recCB)
+          waiting = true
+          continue
+        }
+        default: {
+          cb(new Error('Unreachable'))
+          return
+        }
       }
     }
 

diff --git a/test/feed-holes.test.js b/test/feed-holes.test.js
@@ -52,13 +52,25 @@ test('Feed holes', async (t) => {
   alice.goals.set(postFeedID, 'newest-4')
   assert('alice set a goal for newest-4 of post feed')
 
+  // Mock db.erase so we can inspect how many times it was called
+  const prevErase = alice.db.erase
+  const calledErase = []
+  alice.db.erase = (msgID, cb) => {
+    calledErase.push(msgID)
+    prevErase(msgID, cb)
+  }
+
   await p(alice.gc.forceImmediately)()
+  assert.deepEqual(calledErase, [posts[2]], 'erased A2')
 
   assert.deepEqual(
     getTexts([...alice.db.msgs()]),
     [/*                                     */ 'A7', 'A8', 'A9'],
     'alice has only the end of the feed'
   )
 
+  await p(alice.gc.forceImmediately)()
+  assert.deepEqual(calledErase, [posts[2]], 'no double erasing')
+
   await p(alice.close)(true)
 })