Only emit append in non-sparse mode when contiguous length updates (#108)

* Only emit `append` in non-sparse mode when contiguous length updates

* Skip `append` in non-sparse mode

* Add `oncontigupdate` hook

* Test non-sparse appends

* Assert final `contiguousLength`

* Emit append on all sessions

Author: kasperisager

index.js

@@ -297,7 +297,8 @@ module.exports = class Hypercore extends EventEmitter {
       crypto: this.crypto,
       legacy: opts.legacy,
       auth: opts.auth,
-      onupdate: this._oncoreupdate.bind(this)
+      onupdate: this._oncoreupdate.bind(this),
+      oncontigupdate: this._oncorecontigupdate.bind(this)
     })
 
     if (opts.userData) {
@@ -484,9 +485,10 @@ module.exports = class Hypercore extends EventEmitter {
           if (s._snapshot && bitfield.start < s._snapshot.compatLength) s._snapshot.compatLength = bitfield.start
         }
 
-        if (appended) {
-          s.emit('append')
-        }
+        // For sparse sessions, immediately emit appends. Non-sparse sessions
+        // are handled separately and only emit appends when their contiguous
+        // length is updated.
+        if (appended && s.sparse) s.emit('append')
       }
 
       this.replicator.onupgrade()
@@ -505,6 +507,16 @@ module.exports = class Hypercore extends EventEmitter {
     }
   }
 
+  _oncorecontigupdate () {
+    // For non-sparse sessions, emit appends only when the contiguous length is
+    // updated.
+    for (let i = 0; i < this.sessions.length; i++) {
+      const s = this.sessions[i]
+
+      if (!s.sparse) s.emit('append')
+    }
+  }
+
   _onpeerupdate (added, peer) {
     const name = added ? 'peer-add' : 'peer-remove'
 

lib/core.js

@@ -9,8 +9,9 @@ const { BAD_ARGUMENT, STORAGE_EMPTY, STORAGE_CONFLICT, INVALID_SIGNATURE } = req
 const m = require('./messages')
 
 module.exports = class Core {
-  constructor (header, crypto, oplog, tree, blocks, bitfield, auth, legacy, onupdate) {
+  constructor (header, crypto, oplog, tree, blocks, bitfield, auth, legacy, onupdate, oncontigupdate) {
     this.onupdate = onupdate
+    this.oncontigupdate = oncontigupdate
     this.header = header
     this.crypto = crypto
     this.oplog = oplog
@@ -164,7 +165,7 @@ module.exports = class Core {
       }
     }
 
-    return new this(header, crypto, oplog, tree, blocks, bitfield, auth, !!opts.legacy, opts.onupdate || noop)
+    return new this(header, crypto, oplog, tree, blocks, bitfield, auth, !!opts.legacy, opts.onupdate || noop, opts.oncontigupdate || noop)
   }
 
   _shouldFlush () {
@@ -202,6 +203,7 @@ module.exports = class Core {
       while (this.bitfield.get(i)) i++
 
       this.header.contiguousLength = i
+      this.oncontigupdate()
     }
   }
 
@@ -285,6 +287,7 @@ module.exports = class Core {
       batch.commit()
 
       this.header.contiguousLength = batch.length
+      this.oncontigupdate()
       this.header.tree.length = batch.length
       this.header.tree.rootHash = hash
       this.header.tree.signature = batch.signature
@@ -472,6 +475,7 @@ module.exports = class Core {
     const appended = batch.length > batch.ancestors
 
     this.header.contiguousLength = Math.min(batch.ancestors, this.header.contiguousLength)
+    this.oncontigupdate()
     this.header.tree.fork = batch.fork
     this.header.tree.length = batch.length
     this.header.tree.rootHash = batch.hash()

test/replicate.js

@@ -600,7 +600,7 @@ test('one inflight request to a peer per block', async function (t) {
 })
 
 test('non-sparse replication', async function (t) {
-  t.plan(6)
+  t.plan(2)
 
   const a = await create()
   const b = await create(a.key, { sparse: false })
@@ -609,9 +609,32 @@ test('non-sparse replication', async function (t) {
 
   replicate(a, b, t)
 
+  const download = t.test('download')
+
+  let contiguousLength = 0
+
   b
-    .once('download', () => t.is(b.core.tree.length, 5))
+    // The tree length should be updated to the full length when the first block
+    // is downloaded.
+    .once('download', () => download.is(b.core.tree.length, 5))
+
+    // When blocks are downloaded, the reported length should always match the
+    // contiguous length.
     .on('download', (i) => {
-      t.is(b.length, b.contiguousLength, `block ${i}`)
+      download.is(b.length, b.contiguousLength, `block ${i}`)
+    })
+
+    // Appends should only be emitted when the contiguous length is updated and
+    // never when it's zero.
+    .on('append', () => {
+      if (contiguousLength >= b.contiguousLength) {
+        download.fail('append emitted before contiguous length updated')
+      }
+
+      contiguousLength = b.contiguousLength
     })
+
+  await download
+
+  t.is(contiguousLength, b.length)
 })