Skip to content
This repository has been archived by the owner on Feb 12, 2024. It is now read-only.

Commit

Permalink
refactor: remove proxy api object and detect initialisaion state
Browse files Browse the repository at this point in the history
Ask the repo if it has been initialised, if so, allow the user to
skip the `.init()` step and move on to `.start()`

Removes the proxy api object in favour of vanilla functions because
it was causing errors to be thrown if you even referenced properties
that were from a different api state.

E.g. with an unitialised repo:

```javascript
const ipfs = await IPFS.create({
  init: false,
  start: false
})

// no invocation, just referencing the property causes an error to be thrown
console.info(ipfs.start)
```

I'd looked at changing the proxy behaviour to return a function that
throws if invoked, but at the time the proxy is called you don't know
what the calling code is going to do with the return value so it's hard
to know if it's accessing a function or a property - the return value is
just put on the stack and interacted with so it seemed simpler to just
pull it out and define the API up front.

A nice future improvement might be to have `.init`, `.start` and `.stop`
export functions that update the API - that way after `.stop` has been
invoked, it could restore the API from the post-`.init` state, but this
can come later.

Also upgrades `ipfsd-ctl` to pass refs only during factory creation.

Depends on:

- [ ] ipfs/js-ipfsd-ctl#457
- [ ] ipfs/js-ipfs-repo#219
- [ ] ipfs-inactive/npm-go-ipfs-dep#40

fix: do not allow parallel init, start or stop

If the user is calling `.init`, `.start` or `.stop` from the code
in multiple places simultaneously, they probably have a bug in
their code and we should let them know instead of masking it.
  • Loading branch information
achingbrain committed Feb 13, 2020
1 parent 60fe851 commit 7f3a2d7
Show file tree
Hide file tree
Showing 12 changed files with 496 additions and 72 deletions.
3 changes: 1 addition & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@
"ipfs-http-response": "^0.5.0",
"ipfs-mfs": "^1.0.0",
"ipfs-multipart": "^0.3.0",
"ipfs-repo": "^0.30.0",
"ipfs-repo": "^1.0.0",
"ipfs-unixfs": "^0.3.0",
"ipfs-unixfs-exporter": "^0.41.0",
"ipfs-unixfs-importer": "^0.44.0",
Expand Down Expand Up @@ -155,7 +155,6 @@
"multicodec": "^1.0.0",
"multihashes": "~0.4.14",
"multihashing-async": "^0.8.0",
"p-defer": "^3.0.0",
"p-queue": "^6.1.0",
"parse-duration": "^0.1.2",
"peer-id": "^0.13.5",
Expand Down
127 changes: 116 additions & 11 deletions src/core/api-manager.js
Original file line number Diff line number Diff line change
@@ -1,23 +1,128 @@
'use strict'

const noop = () => {}
const defaultApi = (onUndef = noop) => ({
add: onUndef,
bitswap: {
stat: onUndef,
unwant: onUndef,
wantlist: onUndef
},
block: {
get: onUndef,
put: onUndef,
rm: onUndef,
stat: onUndef
},
bootstrap: {
add: onUndef,
list: onUndef,
rm: onUndef
},
cat: onUndef,
config: onUndef,
dag: {
get: onUndef,
put: onUndef,
resolve: onUndef,
tree: onUndef
},
dns: onUndef,
files: {
chmod: onUndef,
cp: onUndef,
flush: onUndef,
ls: onUndef,
mkdir: onUndef,
mv: onUndef,
read: onUndef,
rm: onUndef,
stat: onUndef,
touch: onUndef,
write: onUndef
},
get: onUndef,
id: onUndef,
init: onUndef,
isOnline: onUndef,
key: {
export: onUndef,
gen: onUndef,
import: onUndef,
info: onUndef,
list: onUndef,
rename: onUndef,
rm: onUndef
},
ls: onUndef,
name: {
publish: onUndef,
pubsub: {
cancel: onUndef,
state: onUndef,
subs: onUndef
}
},
object: {
data: onUndef,
get: onUndef,
links: onUndef,
new: onUndef,
patch: {
addLink: onUndef,
appendData: onUndef,
rmLink: onUndef,
setData: onUndef
},
put: onUndef,
stat: onUndef
},
pin: onUndef,
ping: onUndef,
pubsub: {
subscribe: onUndef,
unsubscribe: onUndef,
publish: onUndef,
ls: onUndef,
peers: onUndef
},
refs: onUndef,
repo: {
gc: onUndef,
stat: onUndef,
version: onUndef
},
resolve: onUndef,
start: onUndef,
stats: {
bitswap: onUndef,
bw: onUndef,
repo: onUndef
},
stop: onUndef,
swarm: {
addrs: onUndef,
connect: onUndef,
disconnect: onUndef,
localAddrs: onUndef,
peers: onUndef
},
version: onUndef
})

module.exports = class ApiManager {
constructor () {
this._api = {}
this._onUndef = () => undefined
this.api = new Proxy(this._api, {
get: (_, prop) => {
if (prop === 'then') return undefined // Not a promise!
return this._api[prop] === undefined ? this._onUndef(prop) : this._api[prop]
}
})
this.api = {
...defaultApi()
}
}

update (nextApi, onUndef) {
const prevApi = { ...this._api }
const prevUndef = this._onUndef
Object.keys(this._api).forEach(k => { delete this._api[k] })
Object.assign(this._api, nextApi)
if (onUndef) this._onUndef = onUndef
Object.keys(this.api).forEach(k => { delete this.api[k] })
Object.assign(this.api, defaultApi(onUndef), nextApi)
this._onUndef = onUndef || noop
return { cancel: () => this.update(prevApi, prevUndef), api: this.api }
}
}
40 changes: 11 additions & 29 deletions src/core/components/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,21 @@ const PeerId = require('peer-id')
const PeerInfo = require('peer-info')
const mergeOptions = require('merge-options')
const getDefaultConfig = require('../runtime/config-nodejs.js')
const createRepo = require('../runtime/repo-nodejs')
const Keychain = require('libp2p-keychain')
const NoKeychain = require('./no-keychain')
const mortice = require('mortice')
const { DAGNode } = require('ipld-dag-pb')
const UnixFs = require('ipfs-unixfs')
const multicodec = require('multicodec')
const {
AlreadyInitializingError,
AlreadyInitializedError,
NotStartedError,
NotEnabledError
AlreadyInitializingError,
NotStartedError
} = require('../errors')
const BlockService = require('ipfs-block-service')
const Ipld = require('ipld')
const getDefaultIpldOptions = require('../runtime/ipld-nodejs')
const createPreloader = require('../preload')
const { ERR_REPO_NOT_INITIALIZED } = require('ipfs-repo').errors
const IPNS = require('../ipns')
const OfflineDatastore = require('../ipns/routing/offline-datastore')
const initAssets = require('../runtime/init-assets-nodejs')
Expand All @@ -32,9 +29,14 @@ const Components = require('./')
module.exports = ({
apiManager,
print,
options: constructorOptions
options: constructorOptions,
repo
}) => async function init (options) {
const { cancel } = apiManager.update({ init: () => { throw new AlreadyInitializingError() } })
const { cancel } = apiManager.update({
init: () => {
throw new AlreadyInitializingError()
}
})

try {
options = options || {}
Expand All @@ -49,30 +51,9 @@ module.exports = ({
options.config = mergeOptions(options.config, constructorOptions.config)
}

options.repo = options.repo || constructorOptions.repo
options.repoAutoMigrate = options.repoAutoMigrate || constructorOptions.repoAutoMigrate

const repo = typeof options.repo === 'string' || options.repo == null
? createRepo({ path: options.repo, autoMigrate: options.repoAutoMigrate })
: options.repo

let isInitialized = true

if (repo.closed) {
try {
await repo.open()
} catch (err) {
if (err.code === ERR_REPO_NOT_INITIALIZED) {
isInitialized = false
} else {
throw err
}
}
}

if (!isInitialized && options.allowNew === false) {
throw new NotEnabledError('new repo initialization is not enabled')
}
const isInitialized = await repo.isInitialized()

const { peerId, keychain } = isInitialized
? await initExistingRepo(repo, options)
Expand Down Expand Up @@ -212,6 +193,7 @@ async function initNewRepo (repo, { privateKey, emptyRepo, bits, profiles, confi
}

async function initExistingRepo (repo, { config: newConfig, profiles, pass }) {
await repo.open()
let config = await repo.config.get()

if (newConfig || profiles) {
Expand Down
21 changes: 13 additions & 8 deletions src/core/components/start.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
const Bitswap = require('ipfs-bitswap')
const multiaddr = require('multiaddr')
const get = require('dlv')
const defer = require('p-defer')
const IPNS = require('../ipns')
const routingConfig = require('../ipns/routing/config')
const { AlreadyInitializedError, NotEnabledError } = require('../errors')
const {
AlreadyInitializedError,
AlreadyStartingError,
AlreadyStartedError,
NotEnabledError
} = require('../errors')
const Components = require('./')
const createMfsPreload = require('../mfs-preload')

Expand All @@ -24,8 +28,11 @@ module.exports = ({
print,
repo
}) => async function start () {
const startPromise = defer()
const { cancel } = apiManager.update({ start: () => startPromise.promise })
const { cancel } = apiManager.update({
start: () => {
throw new AlreadyStartingError()
}
})

try {
// The repo may be closed if previously stopped
Expand Down Expand Up @@ -97,14 +104,12 @@ module.exports = ({
repo
})

apiManager.update(api, () => undefined)
apiManager.update(api)
} catch (err) {
cancel()
startPromise.reject(err)
throw err
}

startPromise.resolve(apiManager.api)
return apiManager.api
}

Expand Down Expand Up @@ -234,7 +239,7 @@ function createApi ({
version: Components.repo.version({ repo })
},
resolve,
start: () => apiManager.api,
start: async () => { throw new AlreadyStartedError() }, // eslint-disable-line require-await
stats: {
bitswap: Components.bitswap.stat({ bitswap }),
bw: libp2p.metrics
Expand Down
18 changes: 11 additions & 7 deletions src/core/components/stop.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
'use strict'

const defer = require('p-defer')
const { NotStartedError, AlreadyInitializedError } = require('../errors')
const {
NotStartedError,
AlreadyInitializedError,
AlreadyStoppingError
} = require('../errors')
const Components = require('./')

module.exports = ({
Expand All @@ -22,8 +25,11 @@ module.exports = ({
print,
repo
}) => async function stop () {
const stopPromise = defer()
const { cancel } = apiManager.update({ stop: () => stopPromise.promise })
const { cancel } = apiManager.update({
stop: () => {
throw new AlreadyStoppingError()
}
})

try {
blockService.unsetExchange()
Expand Down Expand Up @@ -58,11 +64,9 @@ module.exports = ({
apiManager.update(api, () => { throw new NotStartedError() })
} catch (err) {
cancel()
stopPromise.reject(err)
throw err
}

stopPromise.resolve(apiManager.api)
return apiManager.api
}

Expand Down Expand Up @@ -182,7 +186,7 @@ function createApi ({
bw: notStarted,
repo: Components.repo.stat({ repo })
},
stop: () => apiManager.api,
stop: notStarted,
swarm: {
addrs: notStarted,
connect: notStarted,
Expand Down
33 changes: 33 additions & 0 deletions src/core/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,39 @@ class NotStartedError extends Error {
NotStartedError.code = 'ERR_NOT_STARTED'
exports.NotStartedError = NotStartedError

class AlreadyStartingError extends Error {
constructor (message = 'already starting') {
super(message)
this.name = 'AlreadyStartingError'
this.code = AlreadyStartingError.code
}
}

AlreadyStartingError.code = 'ERR_ALREADY_STARTING'
exports.AlreadyStartingError = AlreadyStartingError

class AlreadyStartedError extends Error {
constructor (message = 'already started') {
super(message)
this.name = 'AlreadyStartedError'
this.code = AlreadyStartedError.code
}
}

AlreadyStartedError.code = 'ERR_ALREADY_STARTED'
exports.AlreadyStartedError = AlreadyStartedError

class AlreadyStoppingError extends Error {
constructor (message = 'already started') {
super(message)
this.name = 'AlreadyStoppingError'
this.code = AlreadyStartedError.code
}
}

AlreadyStoppingError.code = 'ERR_ALREADY_STOPPING'
exports.AlreadyStoppingError = AlreadyStoppingError

class NotEnabledError extends Error {
constructor (message = 'not enabled') {
super(message)
Expand Down
Loading

0 comments on commit 7f3a2d7

Please sign in to comment.