Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

API revamp and cleansing #140

Merged
merged 27 commits into from
Jul 4, 2017
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
872284b
documenting new API
pgte Jun 28, 2017
929695b
cleansing and new API
pgte Jun 29, 2017
5bf8ad4
get and set specific parts of the config value
pgte Jun 29, 2017
6029487
small fixes
pgte Jun 29, 2017
85380a5
readme: broke down API into sections
pgte Jun 29, 2017
1c877fa
docs: correcting the blocks API
pgte Jun 29, 2017
13499cd
fixed minor spelling mistake on error message
pgte Jun 29, 2017
210e494
documented repo.exists
pgte Jun 29, 2017
8904d7a
blockstore does not need datastore any more
pgte Jun 29, 2017
7c59443
config.set supports object path key
pgte Jun 30, 2017
87a40b1
docs: corrected config api and added example
pgte Jun 30, 2017
41b021e
fail when config doesnt have the key
pgte Jun 30, 2017
424d0d8
docs: API clarifications and slight changes
pgte Jul 1, 2017
77d1e43
docs: more slight readme changes
pgte Jul 1, 2017
bb6dccd
docs: added link to the the ipfs-blocks repo
pgte Jul 1, 2017
90ae830
fix: docs api and example typo
pgte Jul 1, 2017
25bab69
docs: fixed typos
pgte Jul 1, 2017
1cf93ab
using safe-buffer
pgte Jul 3, 2017
bf82528
succumbing to one liner return callback
pgte Jul 3, 2017
174fff0
DRYer and more elegant error ignoration
pgte Jul 3, 2017
8e8055d
validating arguments and supporting get with undefined key
pgte Jul 4, 2017
40791c5
docs: some README fixes
pgte Jul 4, 2017
a156241
docs: added repo.config.exists
pgte Jul 4, 2017
a9293e2
docs: added a bit of explanation to the meaning of each repo type
pgte Jul 4, 2017
85a2810
tests: new test repo out of go-ipfs v0.4.10
pgte Jul 4, 2017
c5695ed
tests: some more interop tests for blocks and datastore
pgte Jul 4, 2017
bda0e7f
tests: fixed missing level file
pgte Jul 4, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 136 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,142 @@ This now has created the following structure, either on disk or as an in memory

## API

See https://ipfs.github.io/js-ipfs-repo
### Setup

#### new Repo(path[, options])

Creates an IPFS Repo.

Arguments:

* `path` (string, mandatory): the path for this repo
* `options` (object, optional): may contain the following values
* `lock` (string, defaults to `"fs"` in Node.js, `"memory"` in the browser): what type of lock to use. Lock has to be acquired when opening.
* `storageBackends` (object, optional): may contain the following values, which should each be a class implementing the [datastore interface](https://github.com/ipfs/interface-datastore#readme):
* `root` (defaults to [`datastore-fs`](https://github.com/ipfs/js-datastore-fs#readme) in Node.js and [`datastore-level`](https://github.com/ipfs/js-datastore-level#readme) in the browser)
* `blocks` (defaults to [`datastore-fs`](https://github.com/ipfs/js-datastore-fs#readme) in Node.js and [`datastore-level`](https://github.com/ipfs/js-datastore-level#readme) in the browser)
* `datastore` (defaults to `datastore-level`)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to have a brief description of what the role of each of these datastores are at a high level.

Copy link
Member

@daviddias daviddias Jul 3, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • @pgte, could you add this brief description?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


```js
const repo = new Repo('path/to/repo')
```

#### repo.init (callback)

Creates the necesary folder structure inside the repo.

#### repo.open (callback)

Locks the repo.

#### repo.close (callback)

Unlocks the repo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's break the API description into 3 sets:

  • Set up - init, open, close and exists
  • Repos - Explaining that there are two repos
  • Utility/Convinience - apiAddr, config, version..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

#### repo.exists (callback)

Tells whether this repo exists or not. Callsback with `(err, bool)`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing space


### Repos

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add a line here saying "root repo"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

#### repo.put (key, value:Buffer, callback)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary spaces.

Also, can we put these between ``, helps readability on the readme.


Put a value at the root of the repo.

* `key` can be a buffer, a string or a [Key](https://github.com/ipfs/interface-datastore#keys).

#### repo.get (key, callback)

Get a value at the root of the repo.

* `key` can be a buffer, a string or a [Key](https://github.com/ipfs/interface-datastore#keys).
* `callback` is a callback function `function (err, result:Buffer)`

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's add a line here saying "blocks/blockstore" and link it to the ipfs-block class

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

#### repo.blocks.put (block:Block, callback)

* `block` should be of type [Block](https://github.com/ipfs/js-ipfs-block#readme).

#### repo.blocks.putMany (blocks, callback)

Put many blocks block.

* `block` should be an array of type [Block](https://github.com/ipfs/js-ipfs-block#readme).

#### repo.blocks.get (cid, callback)

Get block.

* `cid` is the content id of [type CID](https://github.com/ipld/js-cid#readme).
* `callback` is a callback function `function (err, result:Buffer)`

#### repo.datastore

This is contains a full implementation of [the `interface-datastore` API](https://github.com/ipfs/interface-datastore#api).


### Convenience
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe call this utils? They are convenience methods but typically these are listed as utils

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


#### repo.config

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Explain why using repo.config is better than repo.set('config')

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

##### repo.config.set(key:string, value, callback)

Set a config value. `value` can be any object that is serializable to JSON.

* `key` is a string specifying the object path. Example:

```js
repo.config.set('a.b.c', 'c value', (err) => {
if (err) throw err
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please use {}, even if they are all in the same line so that it follows our agreed codestyle

repo.config.get((err, config) => {
if (err) throw err
assert.equal(config.a.b.c, 'c value')
})
})
``
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing `


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra \n

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed


##### repo.config.set(value, callback)

Set the whole config value. `value` can be any object that is serializable to JSON.

##### repo.config.get(key:string, callback)

Get a config value. `callback` is a function with the signature: `function (err, value)`, wehre the `
value` is of the same type that was set before.

* `key` is a string specifying the object path. Example:

```js
repo.config.set('a.b.c', (err, value) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be .get

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, fixed

if (err) throw err
console.log('config.a.b.c = ', value)
})
```

##### repo.config.get(callback)

Get the entire config value. `callback` is a function with the signature: `function (err, configValue:Object)`.

#### repo.version

##### repo.version.get (callback)

Gets the repo version.

##### repo.version.set (version:number, callback)

Sets the repo version

#### repo.apiAddr

#### repo.apiAddr.get (callback)

Gets the API address.

#### repo.apiAddr.set (value:string, callback)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should take a String or multiaddr, but should always be a valid multiaddr

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


Sets the API address.


## Notes

Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@
"level-js": "timkuijsten/level.js#idbunwrapper",
"leveldown": "^1.7.2",
"lock-me": "^1.0.2",
"lodash.get": "^4.4.2",
"lodash.has": "^4.5.2",
"lodash.set": "^4.3.2",
"multiaddr": "^2.3.0",
"safe-buffer": "^5.1.1"
},
Expand All @@ -85,4 +88,4 @@
"nginnever <[email protected]>",
"npmcdn-to-unpkg-bot <[email protected]>"
]
}
}
38 changes: 38 additions & 0 deletions src/api-addr.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
'use strict'

const Key = require('interface-datastore').Key

const apiFile = new Key('api')

module.exports = (store) => {
return {
/**
* Get the current configuration from the repo.
*
* @param {function(Error, Object)} callback
* @returns {void}
*/
get (callback) {
store.get(apiFile, (err, value) => callback(err, value && value.toString()))
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you are doing the check for value, this method can be simplified to:

get (callback) {
  store.get(apiFile, (err, value) => callback(err, value && value.toString()))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

/**
* Set the current configuration for this repo.
*
* @param {Object} value - the api address to be written
* @param {function(Error)} callback
* @returns {void}
*/
set (value, callback) {
store.put(apiFile, Buffer.from(value.toString()), callback)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Buffer.from is not available in node@4 you will need to include safe-buffer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

},
/**
* Deletes api file
*
* @param {function(Error, bool)} callback
* @returns {void}
*/
delete (callback) {
store.delete(apiFile, callback)
}
}
}
7 changes: 7 additions & 0 deletions src/backends.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
'use strict'

exports.create = function createBackend (name, path, options) {
const Ctor = options.storageBackends[name]
const backendOptions = Object.assign({}, options.storageBackendOptions[name] || {})
return new Ctor(path, backendOptions)
}
32 changes: 27 additions & 5 deletions src/blockstore.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
'use strict'

const NamespaceStore = require('datastore-core').NamespaceDatastore
const core = require('datastore-core')
const ShardingStore = core.ShardingDatastore
const Key = require('interface-datastore').Key
const base32 = require('base32.js')
const Block = require('ipfs-block')
const setImmediate = require('async/setImmediate')
const reject = require('async/reject')
const CID = require('cids')

const blockPrefix = new Key('blocks')

/**
* Transform a raw buffer to a base32 encoded key.
*
Expand All @@ -31,8 +30,27 @@ const cidToDsKey = (cid) => {
return keyFromBuffer(cid.buffer)
}

module.exports = (repo) => {
const store = new NamespaceStore(repo.store, blockPrefix)
module.exports = (filestore, options, callback) => {
maybeWithSharding(filestore, options, (err, store) => {
if (err) {
callback(err)
return // early
}
Copy link
Contributor

@justinmchase justinmchase Jul 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section can be a single line, which standard will allow:

if (err) return callback(err)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but our linting rules do not, right @diasdavid ;)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❤️ {}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not against to if (err) { return callback(err) } if the goal is to save a couple of lines.


callback(null, createBaseStore(store))
})
}

function maybeWithSharding (filestore, options, callback) {
if (options.sharding) {
const shard = new core.shard.NextToLast(2)
ShardingStore.createOrOpen(filestore, shard, callback)
} else {
setImmediate(() => callback(null, filestore))
}
}

function createBaseStore (store) {
return {
/**
* Get a single block by CID.
Expand Down Expand Up @@ -134,6 +152,10 @@ module.exports = (repo) => {
}

store.delete(cidToDsKey(cid), callback)
},

close (callback) {
store.close(callback)
}
}
}
68 changes: 58 additions & 10 deletions src/config.js
Original file line number Diff line number Diff line change
@@ -1,44 +1,69 @@
'use strict'

const Key = require('interface-datastore').Key
const queue = require('async/queue')
const waterfall = require('async/waterfall')
const _get = require('lodash.get')
const _set = require('lodash.set')
const _has = require('lodash.has')

const configKey = new Key('config')

module.exports = (store) => {
return {
const setQueue = queue(_doSet, 1)

const configStore = {
/**
* Get the current configuration from the repo.
*
* @param {String} key - the config key to get
* @param {function(Error, Object)} callback
* @returns {void}
*/
get (callback) {
store.get(configKey, (err, value) => {
get (key, callback) {
if (typeof key === 'function') {
callback = key
key = undefined
}
store.get(configKey, (err, encodedValue) => {
if (err) {
return callback(err)
}

let config
try {
config = JSON.parse(value.toString())
config = JSON.parse(encodedValue.toString())
} catch (err) {
return callback(err)
}
callback(null, config)
if (key !== undefined && !_has(config, key)) {
callback(new Error('Key does not exist in config'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would be nice to say which key doesn't exist, i.e.Key "foo" does not exist in config

return // early
}
let value = key !== undefined ? _get(config, key) : config
callback(null, value)
})
},
/**
* Set the current configuration for this repo.
*
* @param {Object} config - the config object to be written
* @param {String} key - the config key to be written
* @param {Object} value - the config value to be written
* @param {function(Error)} callback
* @returns {void}
*/
set (config, callback) {
const buf = new Buffer(JSON.stringify(config, null, 2))

store.put(configKey, buf, callback)
set (key, value, callback) {
if (typeof value === 'function') {
callback = value
value = key
key = undefined
}
setQueue.push({
key: key,
value: value
}, callback)
},

/**
* Check if a config file exists.
*
Expand All @@ -49,4 +74,27 @@ module.exports = (store) => {
store.has(configKey, callback)
}
}

return configStore

function _doSet (m, callback) {
const key = m.key
const value = m.value
if (key) {
waterfall(
[
(cb) => configStore.get(cb),
(config, cb) => cb(null, _set(config, key, value)),
_saveAll
],
callback)
} else {
_saveAll(value, callback)
}
}

function _saveAll (config, callback) {
const buf = new Buffer(JSON.stringify(config, null, 2))
store.put(configKey, buf, callback)
}
}
Loading