This repository has been archived by the owner on Dec 6, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 16
feat: simplify block to only be data and cid #31
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,3 +35,4 @@ node_modules | |
.node_repl_history | ||
|
||
dist | ||
docs |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,90 +1,66 @@ | ||
'use strict' | ||
|
||
const multihashing = require('multihashing-async') | ||
const setImmediate = require('async/setImmediate') | ||
|
||
module.exports = Block | ||
const CID = require('cids') | ||
|
||
/** | ||
* Represents an immutable block of data that is uniquely referenced with a multihash key. | ||
* Represents an immutable block of data that is uniquely referenced with a cid. | ||
* | ||
* @constructor | ||
* @param {Buffer | string} data - The data to be stored in the block as a buffer or a UTF8 string. | ||
* @param {Buffer} data - The data to be stored in the block as a buffer. | ||
* @param {CID} cid - The cid of the data | ||
* | ||
* @example | ||
* const block = new Block('a012d83b20f9371...') | ||
* const block = new Block(new Buffer('a012d83b20f9371...')) | ||
*/ | ||
function Block (data) { | ||
if (!(this instanceof Block)) { | ||
return new Block(data) | ||
} | ||
class Block { | ||
constructor (data, cid) { | ||
if (!data || !Buffer.isBuffer(data)) { | ||
throw new Error('first argument must be a buffer') | ||
} | ||
|
||
if (!data) { | ||
throw new Error('Block must be constructed with data') | ||
} | ||
if (!cid || !CID.isCID(cid)) { | ||
throw new Error('second argument must be a CID') | ||
} | ||
|
||
if (!(typeof data === 'string' || Buffer.isBuffer(data))) { | ||
throw new Error('data should be Buffer') | ||
this._data = data | ||
this._cid = cid | ||
} | ||
|
||
if (!Buffer.isBuffer(data)) { | ||
data = new Buffer(data) | ||
/** | ||
* The data of this block. | ||
* | ||
* @type {Buffer} | ||
*/ | ||
get data () { | ||
return this._data | ||
} | ||
|
||
this._cache = {} | ||
|
||
data = ensureBuffer(data) | ||
|
||
Object.defineProperty(this, 'data', { | ||
get () { | ||
return data | ||
}, | ||
set () { | ||
throw new Error('Tried to change an immutable block') | ||
} | ||
}) | ||
set data (val) { | ||
throw new Error('Tried to change an immutable block') | ||
} | ||
|
||
/** | ||
* Creates a unique multihash key of this block. | ||
* | ||
* @param {string} [hashFunc='sha2-256'] - The hash function to use. | ||
* @param {function(Error, Multihash)} callback - The callback to execute on completion. | ||
* @returns {void} | ||
* @example | ||
* block.key((multihash) => { | ||
* console.log(multihash) | ||
* }) | ||
* // 'QmeoBGh5g5kHgK3xppJ1...' | ||
**/ | ||
this.key = (hashFunc, callback) => { | ||
if (typeof hashFunc === 'function') { | ||
callback = hashFunc | ||
hashFunc = null | ||
} | ||
|
||
if (!hashFunc) { | ||
hashFunc = 'sha2-256' | ||
} | ||
|
||
if (this._cache[hashFunc]) { | ||
return setImmediate(() => { | ||
callback(null, this._cache[hashFunc]) | ||
}) | ||
} | ||
|
||
multihashing(this.data, hashFunc, (err, multihash) => { | ||
if (err) { | ||
return callback(err) | ||
} | ||
this._cache[hashFunc] = multihash | ||
callback(null, multihash) | ||
}) | ||
* The cid of the data this block represents. | ||
* | ||
* @type {CID} | ||
*/ | ||
get cid () { | ||
return this._cid | ||
} | ||
} | ||
|
||
function ensureBuffer (data) { | ||
if (Buffer.isBuffer(data)) { | ||
return data | ||
set cid (val) { | ||
throw new Error('Tried to change an immutable block') | ||
} | ||
|
||
return new Buffer(data) | ||
/** | ||
* Check if the given value is a Block. | ||
* | ||
* @param {any} other | ||
* @returns {bool} | ||
*/ | ||
static isBlock (other) { | ||
return other && other.constructor.name === 'Block' | ||
} | ||
} | ||
|
||
module.exports = Block |
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
/* eslint-env mocha */ | ||
'use strict' | ||
|
||
const expect = require('chai').expect | ||
const CID = require('cids') | ||
|
||
const Block = require('../src') | ||
|
||
describe('block', () => { | ||
it('create throws', () => { | ||
expect( | ||
() => new Block('string') | ||
).to.throw() | ||
|
||
expect( | ||
() => new Block(new Buffer('hello'), 'cid') | ||
).to.throw() | ||
|
||
expect( | ||
() => new Block('hello', new CID('QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n')) | ||
).to.throw() | ||
}) | ||
|
||
it('create', () => { | ||
const b = new Block(new Buffer('hello'), new CID('QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n')) | ||
|
||
expect(Block.isBlock(b)).to.eql(true) | ||
}) | ||
|
||
it('block stays immutable', () => { | ||
const b = new Block(new Buffer('hello'), new CID('QmdfTbBqBPQ7VNxZEYEj14VmRuZBkqFbiwReogJgS1zR1n')) | ||
|
||
expect( | ||
() => { b.data = 'fail' } | ||
).to.throw( | ||
/immutable/ | ||
) | ||
|
||
expect( | ||
() => { b.cid = 'fail' } | ||
).to.throw( | ||
/immutable/ | ||
) | ||
}) | ||
}) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still making my mind over this, but currently, I strongly feel that public module methods shouldn't be encapsulated in Class objects and made public with
static
.I've seen a lot of cases of people starting to use
static
and then forgetting what it really means and start mutating the state.Plus, it is way more understandable from a Node.js/CJS user if the "module.exports" is the only place that tells me what gets exported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declaring
Block.isBlock = ...
doesn't seem to be any better to me in terms of understanding.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also we started using this a while ago in modules like cid
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where? https://github.com/ipld/js-cid/blob/master/src/index.js#L133-L135
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fine
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one bonus is that jsdoc picks up the right docs for this if we write it as static
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm.....