From f6abb639e31b8f2854a750ecc28a6ce81744f0a2 Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Thu, 14 Apr 2016 15:41:33 -0700
Subject: [PATCH 01/10] Order matters!

---
 src/block-service.js | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/block-service.js b/src/block-service.js
index d7bffb0..c68183a 100644
--- a/src/block-service.js
+++ b/src/block-service.js
@@ -26,15 +26,15 @@ function BlockService (ipfsRepo, exchange) {
   }
 
   this.getBlock = (multihash, extension, callback) => {
-    if (!multihash) {
-      return callback(new Error('Invalid multihash'))
-    }
-
     if (typeof extension === 'function') {
       callback = extension
       extension = undefined
     }
 
+    if (!multihash) {
+      return callback(new Error('Invalid multihash'))
+    }
+
     ipfsRepo.datastore.createReadStream(multihash, extension)
       .pipe(bl((err, data) => {
         if (err) { return callback(err) }
@@ -43,15 +43,15 @@ function BlockService (ipfsRepo, exchange) {
   }
 
   this.getBlocks = (multihashes, extension, callback) => {
-    if (!Array.isArray(multihashes)) {
-      return callback(new Error('Invalid batch of multihashes'))
-    }
-
     if (typeof extension === 'function') {
       callback = extension
       extension = undefined
     }
 
+    if (!Array.isArray(multihashes)) {
+      return callback(new Error('Invalid batch of multihashes'))
+    }
+
     const blocks = []
 
     async.each(multihashes, (multihash, next) => {
@@ -65,28 +65,28 @@ function BlockService (ipfsRepo, exchange) {
   }
 
   this.deleteBlock = (multihash, extension, callback) => {
-    if (!multihash) {
-      return callback(new Error('Invalid multihash'))
-    }
-
     if (typeof extension === 'function') {
       callback = extension
       extension = undefined
     }
 
+    if (!multihash) {
+      return callback(new Error('Invalid multihash'))
+    }
+
     ipfsRepo.datastore.remove(multihash, extension, callback)
   }
 
   this.deleteBlocks = (multihashes, extension, callback) => {
-    if (!Array.isArray(multihashes)) {
-      return callback('Invalid batch of multihashes')
-    }
-
     if (typeof extension === 'function') {
       callback = extension
       extension = undefined
     }
 
+    if (!Array.isArray(multihashes)) {
+      return callback('Invalid batch of multihashes')
+    }
+
     async.each(multihashes, (multihash, next) => {
       this.deleteBlock(multihash, extension, next)
     }, (err) => {

From 5f669b7177a2559a3862bf4c0410cb84d27eee52 Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Thu, 14 Apr 2016 16:04:57 -0700
Subject: [PATCH 02/10] Fixes getBlocks: was never working.

---
 src/block-service.js | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/block-service.js b/src/block-service.js
index c68183a..3e1ad59 100644
--- a/src/block-service.js
+++ b/src/block-service.js
@@ -58,6 +58,7 @@ function BlockService (ipfsRepo, exchange) {
       this.getBlock(multihash, extension, (err, block) => {
         if (err) { return next(err) }
         blocks.push(block)
+        next()
       })
     }, (err) => {
       callback(err, blocks)

From 803e4e19a11d18f7fc031c289e56d5f1277acfcb Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Thu, 14 Apr 2016 16:07:46 -0700
Subject: [PATCH 03/10] Fixes missing error type.

---
 src/block-service.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/block-service.js b/src/block-service.js
index 3e1ad59..93faa05 100644
--- a/src/block-service.js
+++ b/src/block-service.js
@@ -85,7 +85,7 @@ function BlockService (ipfsRepo, exchange) {
     }
 
     if (!Array.isArray(multihashes)) {
-      return callback('Invalid batch of multihashes')
+      return callback(new Error('Invalid batch of multihashes'))
     }
 
     async.each(multihashes, (multihash, next) => {

From afe3405b34cc1775b32ab07bfd973f1406f5e519 Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Thu, 14 Apr 2016 16:15:57 -0700
Subject: [PATCH 04/10] Returns !block + !error when block not present.

---
 src/block-service.js | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/src/block-service.js b/src/block-service.js
index 93faa05..8edfee8 100644
--- a/src/block-service.js
+++ b/src/block-service.js
@@ -35,11 +35,19 @@ function BlockService (ipfsRepo, exchange) {
       return callback(new Error('Invalid multihash'))
     }
 
-    ipfsRepo.datastore.createReadStream(multihash, extension)
-      .pipe(bl((err, data) => {
-        if (err) { return callback(err) }
-        callback(null, new Block(data, extension))
-      }))
+    ipfsRepo.datastore.exists(multihash, (err, exists) => {
+      if (err) { return callback(err) }
+
+      if (exists) {
+        ipfsRepo.datastore.createReadStream(multihash, extension)
+          .pipe(bl((err, data) => {
+            if (err) { return callback(err) }
+            callback(null, new Block(data, extension))
+          }))
+      } else {
+        callback(null, null)
+      }
+    })
   }
 
   this.getBlocks = (multihashes, extension, callback) => {
@@ -57,7 +65,9 @@ function BlockService (ipfsRepo, exchange) {
     async.each(multihashes, (multihash, next) => {
       this.getBlock(multihash, extension, (err, block) => {
         if (err) { return next(err) }
-        blocks.push(block)
+        if (block) {
+          blocks.push(block)
+        }
         next()
       })
     }, (err) => {

From 94d0b12796a9de046fdd1f4b60186779ce86172d Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Thu, 14 Apr 2016 16:19:09 -0700
Subject: [PATCH 05/10] Adds tests for blocks and service.

---
 test/block-service-test.js | 84 +++++++++++++++++++++++++++++++++++---
 test/block.spec.js         |  7 ++++
 2 files changed, 85 insertions(+), 6 deletions(-)

diff --git a/test/block-service-test.js b/test/block-service-test.js
index ad1cc22..4cb245e 100644
--- a/test/block-service-test.js
+++ b/test/block-service-test.js
@@ -15,7 +15,7 @@ module.exports = (repo) => {
       done()
     })
 
-    it('store a block', (done) => {
+    it('store and get a block', (done) => {
       const b = new Block('A random data block')
       bs.addBlock(b, (err) => {
         expect(err).to.not.exist
@@ -28,7 +28,7 @@ module.exports = (repo) => {
       })
     })
 
-    it('store a block, with custom extension', (done) => {
+    it('store and get a block, with custom extension', (done) => {
       const b = new Block('A random data block', 'ext')
       bs.addBlock(b, (err) => {
         expect(err).to.not.exist
@@ -44,7 +44,8 @@ module.exports = (repo) => {
     it('get a non existent block', (done) => {
       const b = new Block('Not stored')
       bs.getBlock(b.key, (err, block) => {
-        expect(err).to.exist
+        expect(err).to.not.exist
+        expect(block).to.not.exist
         done()
       })
     })
@@ -60,6 +61,61 @@ module.exports = (repo) => {
       })
     })
 
+    it('addBlocks: bad invocation', (done) => {
+      const b1 = new Block('1')
+
+      bs.addBlocks(b1, (err) => {
+        expect(err).to.be.an('error')
+        done()
+      })
+    })
+
+    it('getBlock: bad invocation', (done) => {
+      bs.getBlock(null, (err) => {
+        expect(err).to.be.an('error')
+        done()
+      })
+    })
+
+    it('getBlocks: bad invocation', (done) => {
+      bs.getBlocks(null, 'protobuf', (err) => {
+        expect(err).to.be.an('error')
+        done()
+      })
+    })
+
+    it('get many blocks', (done) => {
+      const b1 = new Block('1')
+      const b2 = new Block('2')
+      const b3 = new Block('3')
+
+      bs.addBlocks([b1, b2, b3], (err) => {
+        expect(err).to.not.exist
+
+        bs.getBlocks([b1.key, b2.key, b3.key], (err, blocks) => {
+          expect(err).to.not.exist
+          expect(blocks).to.have.lengthOf(3)
+          done()
+        })
+      })
+    })
+
+    it('get many blocks: partial success', (done) => {
+      const b1 = new Block('a1')
+      const b2 = new Block('a2')
+      const b3 = new Block('a3')
+
+      bs.addBlocks([b1, b3], (err) => {
+        expect(err).to.not.exist
+
+        bs.getBlocks([b1.key, b2.key, b3.key], (err, blocks) => {
+          expect(err).to.not.exist
+          expect(blocks).to.have.lengthOf(2)
+          done()
+        })
+      })
+    })
+
     it('delete a block', (done) => {
       const b = new Block('Will not live that much')
       bs.addBlock(b, (err) => {
@@ -67,13 +123,21 @@ module.exports = (repo) => {
         bs.deleteBlock(b.key, (err) => {
           expect(err).to.not.exist
           bs.getBlock(b.key, (err, block) => {
-            expect(err).to.exist
+            expect(err).to.not.exist
+            expect(block).to.not.exist
             done()
           })
         })
       })
     })
 
+    it('deleteBlock: bad invocation', (done) => {
+      bs.deleteBlock(null, (err) => {
+        expect(err).to.be.an('error')
+        done()
+      })
+    })
+
     it('delete a block, with custom extension', (done) => {
       const b = new Block('Will not live that much', 'ext')
       bs.addBlock(b, (err) => {
@@ -81,7 +145,8 @@ module.exports = (repo) => {
         bs.deleteBlock(b.key, 'ext', (err) => {
           expect(err).to.not.exist
           bs.getBlock(b.key, 'ext', (err, block) => {
-            expect(err).to.exist
+            expect(err).to.not.exist
+            expect(block).to.not.exist
             done()
           })
         })
@@ -101,10 +166,17 @@ module.exports = (repo) => {
       const b2 = new Block('2')
       const b3 = new Block('3')
 
-      bs.deleteBlocks([b1, b2, b3], (err) => {
+      bs.deleteBlocks([b1, b2, b3], 'data', (err) => {
         expect(err).to.not.exist
         done()
       })
     })
+
+    it('deleteBlocks: bad invocation', (done) => {
+      bs.deleteBlocks(null, (err) => {
+        expect(err).to.be.an('error')
+        done()
+      })
+    })
   })
 }
diff --git a/test/block.spec.js b/test/block.spec.js
index 84bde71..856cc8d 100644
--- a/test/block.spec.js
+++ b/test/block.spec.js
@@ -12,6 +12,13 @@ describe('block', () => {
     expect(b.extension).to.be.eql('data')
   })
 
+  it('create /wo new', () => {
+    const b = Block('random-data')
+    expect(b.key).to.exist
+    expect(b.data).to.exist
+    expect(b.extension).to.be.eql('data')
+  })
+
   it('fail to create an empty block', () => {
     expect(() => new Block()).to.throw()
   })

From 560497f477faf15497f289df56565f1ddad1b734 Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Thu, 14 Apr 2016 16:22:36 -0700
Subject: [PATCH 06/10] Updates README.

---
 README.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/README.md b/README.md
index e63af74..b87e76b 100644
--- a/README.md
+++ b/README.md
@@ -149,11 +149,13 @@ Asynchronously adds an array of block instances to the underlying repo.
 #### bs.getBlock(multihash, callback(err, block))
 
 Asynchronously returns the block whose content multihash matches `multihash`.
+Returns an error (`err.code === 'ENOENT'`) if the block does not exist.
 
 #### bs.getBlocks(multihashes, callback(err, blocks))
 
 Asynchronously returns the blocks whose content multihashes match the array
 `multihashes`.
+Returns an error (`err.code === 'ENOENT'`) if *any* the blocks do not exist.
 
 *Does not guarantee atomicity.*
 

From 27e4b2e83745ef78e06eb6c9ab93bd0f4a210937 Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Fri, 15 Apr 2016 16:12:49 -0700
Subject: [PATCH 07/10] Errors getBlock(s) on non-existence.

---
 src/block-service.js       | 18 +++++-------------
 test/block-service-test.js | 13 +++++--------
 2 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/src/block-service.js b/src/block-service.js
index 8edfee8..e4b6365 100644
--- a/src/block-service.js
+++ b/src/block-service.js
@@ -35,19 +35,11 @@ function BlockService (ipfsRepo, exchange) {
       return callback(new Error('Invalid multihash'))
     }
 
-    ipfsRepo.datastore.exists(multihash, (err, exists) => {
-      if (err) { return callback(err) }
-
-      if (exists) {
-        ipfsRepo.datastore.createReadStream(multihash, extension)
-          .pipe(bl((err, data) => {
-            if (err) { return callback(err) }
-            callback(null, new Block(data, extension))
-          }))
-      } else {
-        callback(null, null)
-      }
-    })
+    ipfsRepo.datastore.createReadStream(multihash, extension)
+      .pipe(bl((err, data) => {
+        if (err) { return callback(err) }
+        callback(null, new Block(data, extension))
+      }))
   }
 
   this.getBlocks = (multihashes, extension, callback) => {
diff --git a/test/block-service-test.js b/test/block-service-test.js
index 4cb245e..a89e89b 100644
--- a/test/block-service-test.js
+++ b/test/block-service-test.js
@@ -44,8 +44,7 @@ module.exports = (repo) => {
     it('get a non existent block', (done) => {
       const b = new Block('Not stored')
       bs.getBlock(b.key, (err, block) => {
-        expect(err).to.not.exist
-        expect(block).to.not.exist
+        expect(err).to.exist
         done()
       })
     })
@@ -109,8 +108,8 @@ module.exports = (repo) => {
         expect(err).to.not.exist
 
         bs.getBlocks([b1.key, b2.key, b3.key], (err, blocks) => {
-          expect(err).to.not.exist
-          expect(blocks).to.have.lengthOf(2)
+          expect(err).to.exist
+          expect(blocks).to.have.lengthOf(0)
           done()
         })
       })
@@ -123,8 +122,7 @@ module.exports = (repo) => {
         bs.deleteBlock(b.key, (err) => {
           expect(err).to.not.exist
           bs.getBlock(b.key, (err, block) => {
-            expect(err).to.not.exist
-            expect(block).to.not.exist
+            expect(err).to.exist
             done()
           })
         })
@@ -145,8 +143,7 @@ module.exports = (repo) => {
         bs.deleteBlock(b.key, 'ext', (err) => {
           expect(err).to.not.exist
           bs.getBlock(b.key, 'ext', (err, block) => {
-            expect(err).to.not.exist
-            expect(block).to.not.exist
+            expect(err).to.exist
             done()
           })
         })

From 6ae98a17548af495d2088a7fc9c4d4a3ec6346c4 Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Fri, 15 Apr 2016 16:31:00 -0700
Subject: [PATCH 08/10] Handles async properly in test.

---
 test/block-service-test.js | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/block-service-test.js b/test/block-service-test.js
index a89e89b..e0c5ba9 100644
--- a/test/block-service-test.js
+++ b/test/block-service-test.js
@@ -109,7 +109,7 @@ module.exports = (repo) => {
 
         bs.getBlocks([b1.key, b2.key, b3.key], (err, blocks) => {
           expect(err).to.exist
-          expect(blocks).to.have.lengthOf(0)
+          expect(blocks).to.have.length.below(3)
           done()
         })
       })

From 6779e25133e139321a55d99ebc9099ce7f471e0d Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Thu, 14 Apr 2016 16:22:36 -0700
Subject: [PATCH 09/10] Updates README.

---
 README.md | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/README.md b/README.md
index b87e76b..4d8bc5a 100644
--- a/README.md
+++ b/README.md
@@ -151,6 +151,8 @@ Asynchronously adds an array of block instances to the underlying repo.
 Asynchronously returns the block whose content multihash matches `multihash`.
 Returns an error (`err.code === 'ENOENT'`) if the block does not exist.
 
+If the block could not be found, expect `err.code` to be `'ENOENT'`.
+
 #### bs.getBlocks(multihashes, callback(err, blocks))
 
 Asynchronously returns the blocks whose content multihashes match the array

From 58f11918a9ceec5b5cfcc5c564539be93449b38f Mon Sep 17 00:00:00 2001
From: Stephen Whitmore <stephen.whitmore@gmail.com>
Date: Mon, 18 Apr 2016 14:54:31 -0700
Subject: [PATCH 10/10] Updates getBlocks API.

---
 README.md                  | 13 ++++++++++++-
 src/block-service.js       | 10 +++++-----
 test/block-service-test.js | 24 +++++++++++++++++++++---
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/README.md b/README.md
index 4d8bc5a..3317b99 100644
--- a/README.md
+++ b/README.md
@@ -157,7 +157,18 @@ If the block could not be found, expect `err.code` to be `'ENOENT'`.
 
 Asynchronously returns the blocks whose content multihashes match the array
 `multihashes`.
-Returns an error (`err.code === 'ENOENT'`) if *any* the blocks do not exist.
+
+`blocks` is an object that maps each `multihash` to an object of the form
+
+```js
+{
+  err: Error
+  block: Block
+}
+```
+
+Expect `blocks[multihash].err.code === 'ENOENT'`  and `blocks[multihash].block
+=== null` if a block did not exist.
 
 *Does not guarantee atomicity.*
 
diff --git a/src/block-service.js b/src/block-service.js
index e4b6365..1f89101 100644
--- a/src/block-service.js
+++ b/src/block-service.js
@@ -52,18 +52,18 @@ function BlockService (ipfsRepo, exchange) {
       return callback(new Error('Invalid batch of multihashes'))
     }
 
-    const blocks = []
+    var results = {}
 
     async.each(multihashes, (multihash, next) => {
       this.getBlock(multihash, extension, (err, block) => {
-        if (err) { return next(err) }
-        if (block) {
-          blocks.push(block)
+        results[multihash] = {
+          err: err,
+          block: block
         }
         next()
       })
     }, (err) => {
-      callback(err, blocks)
+      callback(err, results)
     })
   }
 
diff --git a/test/block-service-test.js b/test/block-service-test.js
index e0c5ba9..880816a 100644
--- a/test/block-service-test.js
+++ b/test/block-service-test.js
@@ -93,7 +93,16 @@ module.exports = (repo) => {
 
         bs.getBlocks([b1.key, b2.key, b3.key], (err, blocks) => {
           expect(err).to.not.exist
-          expect(blocks).to.have.lengthOf(3)
+          expect(Object.keys(blocks)).to.have.lengthOf(3)
+          expect(blocks[b1.key]).to.exist
+          expect(blocks[b1.key].err).to.not.exist
+          expect(blocks[b1.key].block.data).to.deep.equal(b1.data)
+          expect(blocks[b2.key]).to.exist
+          expect(blocks[b2.key].err).to.not.exist
+          expect(blocks[b2.key].block.data).to.deep.equal(b2.data)
+          expect(blocks[b3.key]).to.exist
+          expect(blocks[b3.key].err).to.not.exist
+          expect(blocks[b3.key].block.data).to.deep.equal(b3.data)
           done()
         })
       })
@@ -108,8 +117,17 @@ module.exports = (repo) => {
         expect(err).to.not.exist
 
         bs.getBlocks([b1.key, b2.key, b3.key], (err, blocks) => {
-          expect(err).to.exist
-          expect(blocks).to.have.length.below(3)
+          expect(err).to.not.exist
+          expect(Object.keys(blocks)).to.have.lengthOf(3)
+          expect(blocks[b1.key]).to.exist
+          expect(blocks[b1.key].err).to.not.exist
+          expect(blocks[b1.key].block.data).to.deep.equal(b1.data)
+          expect(blocks[b2.key]).to.exist
+          expect(blocks[b2.key].err).to.exist
+          expect(blocks[b2.key].block).to.not.exist
+          expect(blocks[b3.key]).to.exist
+          expect(blocks[b3.key].err).to.not.exist
+          expect(blocks[b3.key].block.data).to.deep.equal(b3.data)
           done()
         })
       })