From 6661b73fd4fc4d928d90ca945b03903d7c137dd3 Mon Sep 17 00:00:00 2001
From: Andre Staltz <andre@staltz.com>
Date: Wed, 28 Feb 2024 15:32:36 +0200
Subject: [PATCH] add() can replace a dataless msg with a dataful

---
 lib/index.js     | 56 ++++++++++++++++++++++++------------
 test/add.test.js | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 112 insertions(+), 18 deletions(-)

diff --git a/lib/index.js b/lib/index.js
index fbeb6cb..bc8f3b2 100644
--- a/lib/index.js
+++ b/lib/index.js
@@ -512,6 +512,14 @@ function initDB(peer, config) {
     return tangleID
   }
 
+  /**
+   * @param {string} msgID
+   * @param {CB<void>} cb
+   */
+  function bypassPredelete(msgID, cb) {
+    cb()
+  }
+
   /**
    * @param {Msg} msg
    * @param {MsgID | null} tangleID
@@ -529,30 +537,42 @@ function initDB(peer, config) {
     // TODO: optimize this. Perhaps have a Map() of msgID -> record
     // Or even better, a bloom filter. If you just want to answer no/perhaps.
     let rec
-    if ((rec = getRecord(msgID))) return cb(null, rec)
-    else rec = { msg, id: msgID }
+    let maybePredelete = bypassPredelete
+    if ((rec = getRecord(msgID))) {
+      // If existing record is dataless but new is dataful, then delete
+      if (rec.msg.data === null && msg.data !== null) {
+        maybePredelete = del
+        rec = { msg, id: msgID }
+      } else {
+        return cb(null, rec)
+      }
+    } else rec = { msg, id: msgID }
 
-    tangleID ??= inferTangleID(rec)
+    const actualTangleID = tangleID ?? inferTangleID(rec)
 
     let err
-    if ((err = verifyRec(rec, tangleID))) {
+    if ((err = verifyRec(rec, actualTangleID))) {
       return cb(new Error('add() failed to verify msg', { cause: err }))
     }
 
-    // The majority of cases don't have ghosts to be removed, but this operation
-    // is silent and cheap if there are no ghosts.
-    removeGhost(tangleID, msgID, (err) => {
-      // prettier-ignore
-      if (err) return cb(new Error('add() failed to remove ghost', { cause: err }))
-      logAppend(msgID, msg, (err, rec) => {
-        if (err) return cb(new Error('add() failed in the log', { cause: err }))
-        const doneable = msgsBeingAdded.get(msgID)
-        msgsBeingAdded.delete(msgID)
-        queueMicrotask(() => {
-          doneable?.done([null, rec])
-          onRecordAdded.set(rec)
+    maybePredelete(msgID, (err) => {
+      if (err) return cb(new Error('add() failed to predelete', { cause: err }))
+      // The majority of cases don't have ghosts to be removed, but this
+      // operation is silent and cheap if there are no ghosts.
+      removeGhost(actualTangleID, msgID, (err) => {
+        // prettier-ignore
+        if (err) return cb(new Error('add() failed to remove ghost', { cause: err }))
+        logAppend(msgID, msg, (err, rec) => {
+          if (err)
+            return cb(new Error('add() failed in the log', { cause: err }))
+          const doneable = msgsBeingAdded.get(msgID)
+          msgsBeingAdded.delete(msgID)
+          queueMicrotask(() => {
+            doneable?.done([null, rec])
+            onRecordAdded.set(rec)
+          })
+          cb(null, rec)
         })
-        cb(null, rec)
       })
     })
   }
@@ -1230,7 +1250,7 @@ function initDB(peer, config) {
       // prettier-ignore
       if (err) return cb?.(err)
       rescanLogPostCompaction(cb)
-    });
+    })
   }
 
   return {
diff --git a/test/add.test.js b/test/add.test.js
index f230604..91e6b5c 100644
--- a/test/add.test.js
+++ b/test/add.test.js
@@ -60,5 +60,79 @@ test('add()', async (t) => {
     assert.deepEqual(stats, { totalBytes: 2072, deletedBytes: 0 })
   })
 
+  await t.test('dataful msg replacing a dataless msg', async (t) => {
+    const rootMsg = MsgV4.createMoot(id, 'something', keypair)
+    const rootID = MsgV4.getMsgID(rootMsg)
+    await p(peer.db.add)(rootMsg, rootID)
+
+    const tangle = new MsgV4.Tangle(rootID)
+    tangle.add(rootID, rootMsg)
+
+    const msg1Dataful = MsgV4.create({
+      keypair,
+      account: id,
+      accountTips: [id],
+      domain: 'something',
+      data: { text: 'first' },
+      tangles: {
+        [rootID]: tangle,
+      },
+    })
+    const msg1Dataless = { ...msg1Dataful, data: null }
+    const msg1ID = MsgV4.getMsgID(msg1Dataful)
+
+    tangle.add(msg1ID, msg1Dataful)
+
+    const msg2 = MsgV4.create({
+      keypair,
+      account: id,
+      accountTips: [id],
+      domain: 'something',
+      data: { text: 'second' },
+      tangles: {
+        [rootID]: tangle,
+      },
+    })
+    const msg2ID = MsgV4.getMsgID(msg2)
+
+    await p(peer.db.add)(msg1Dataless, rootID)
+    await p(peer.db.add)(msg2, rootID)
+
+    // We expect there to be 3 msgs: root, dataless msg1, dataful msg2
+    {
+      const ids = []
+      const texts = []
+      for (const rec of peer.db.records()) {
+        if (rec.msg.metadata.domain === 'something') {
+          ids.push(rec.id)
+          texts.push(rec.msg.data?.text)
+        }
+      }
+      assert.deepEqual(ids, [rootID, msg1ID, msg2ID])
+      assert.deepEqual(texts, [undefined, undefined, 'second'])
+      const stats = await p(peer.db.log.stats)()
+      assert.deepEqual(stats, { totalBytes: 3718, deletedBytes: 0 })
+    }
+
+    await p(peer.db.add)(msg1Dataful, rootID)
+
+    // We expect there to be 3 msgs: root, (deleted) dataless msg1, dataful msg2
+    // and dataful msg1 appended at the end
+    {
+      const ids = []
+      const texts = []
+      for (const rec of peer.db.records()) {
+        if (rec.msg.metadata.domain === 'something') {
+          ids.push(rec.id)
+          texts.push(rec.msg.data?.text)
+        }
+      }
+      assert.deepEqual(ids, [rootID, msg2ID, msg1ID])
+      assert.deepEqual(texts, [undefined, 'second', 'first'])
+      const stats = await p(peer.db.log.stats)()
+      assert.deepEqual(stats, { totalBytes: 4340, deletedBytes: 610 })
+    }
+  })
+
   await p(peer.close)(true)
 })