diff --git a/README.md b/README.md index c48600eb..9afb534c 100644 --- a/README.md +++ b/README.md @@ -28,6 +28,7 @@ This is the implementation of the [IPFS repo spec](https://github.com/ipfs/specs - [Use in a browser Using a script tag](#use-in-a-browser-using-a-script-tag) - [Usage](#usage) - [API](#api) +- [Notes](#notes) - [Contribute](#contribute) - [License](#license) @@ -136,6 +137,7 @@ Arguments: * `path` (string, mandatory): the path for this repo * `options` (object, optional): may contain the following values + * `autoMigrate` (bool, defaults to `true`): controls automatic migrations of repository. * `lock` ([Lock](#lock) or string *Deprecated*): what type of lock to use. Lock has to be acquired when opening. string can be `"fs"` or `"memory"`. * `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). Defines the back-end type used for gets and puts of values at the root (`repo.set()`, `repo.get()`) @@ -318,6 +320,15 @@ Returned promise resolves to a `boolean` indicating the existence of the lock. - [Explanation of how repo is structured](https://github.com/ipfs/js-ipfs-repo/pull/111#issuecomment-279948247) +### Migrations + +When there is a new repo migration and the version of repo is increased, don't +forget to propagate the changes into the test repo (`test/test-repo`). + +**For tools that run mainly in the browser environment, be aware that disabling automatic +migrations leaves the user with no way to run the migrations because there is no CLI in the browser. In such +a case, you should provide a way to trigger migrations manually.** + ## Contribute There are some ways you can make this module better: diff --git a/package.json b/package.json index ce6ee028..c1396ece 100644 --- a/package.json +++ b/package.json @@ -43,15 +43,16 @@ "npm": ">=3.0.0" }, "devDependencies": { - "aegir": "^19.0.3", + "aegir": "^20.4.1", "chai": "^4.2.0", "dirty-chai": "^2.0.1", "lodash": "^4.17.11", "memdown": "^4.0.0", - "multihashes": "~0.4.14", - "multihashing-async": "~0.7.0", + "multihashes": "~0.4.15", + "multihashing-async": "~0.8.0", "ncp": "^2.0.0", - "rimraf": "^2.6.3" + "rimraf": "^3.0.0", + "sinon": "^7.5.0" }, "dependencies": { "base32.js": "~0.1.0", @@ -64,6 +65,7 @@ "err-code": "^1.1.2", "interface-datastore": "~0.7.0", "ipfs-block": "~0.8.1", + "ipfs-repo-migrations": "~0.1.0", "just-safe-get": "^1.3.0", "just-safe-set": "^2.1.0", "lodash.has": "^4.5.2", diff --git a/src/errors/index.js b/src/errors/index.js index 5a1d7327..67628414 100644 --- a/src/errors/index.js +++ b/src/errors/index.js @@ -7,8 +7,7 @@ class LockExistsError extends Error { constructor (message) { super(message) this.name = 'LockExistsError' - this.code = 'ERR_LOCK_EXISTS' - this.message = message + this.code = LockExistsError.code } } @@ -22,14 +21,27 @@ class NotFoundError extends Error { constructor (message) { super(message) this.name = 'NotFoundError' - this.code = 'ERR_NOT_FOUND' - this.message = message + this.code = NotFoundError.code } } NotFoundError.code = 'ERR_NOT_FOUND' exports.NotFoundError = NotFoundError +/** + * Error raised when version of the stored repo is not compatible with version of this package. + */ +class InvalidRepoVersionError extends Error { + constructor (message) { + super(message) + this.name = 'InvalidRepoVersionError' + this.code = InvalidRepoVersionError.code + } +} + +InvalidRepoVersionError.code = 'ERR_INVALID_REPO_VERSION' +exports.InvalidRepoVersionError = InvalidRepoVersionError + exports.ERR_REPO_NOT_INITIALIZED = 'ERR_REPO_NOT_INITIALIZED' exports.ERR_REPO_ALREADY_OPEN = 'ERR_REPO_ALREADY_OPEN' exports.ERR_REPO_ALREADY_CLOSED = 'ERR_REPO_ALREADY_CLOSED' diff --git a/src/index.js b/src/index.js index 5b203be2..83e0c9dd 100644 --- a/src/index.js +++ b/src/index.js @@ -6,7 +6,9 @@ const path = require('path') const debug = require('debug') const Big = require('bignumber.js') const errcode = require('err-code') +const migrator = require('ipfs-repo-migrations') +const constants = require('./constants') const backends = require('./backends') const version = require('./version') const config = require('./config') @@ -20,14 +22,13 @@ const ERRORS = require('./errors') const log = debug('repo') const noLimit = Number.MAX_SAFE_INTEGER +const AUTO_MIGRATE_CONFIG_KEY = 'repoAutoMigrate' const lockers = { memory: require('./lock-memory'), fs: require('./lock') } -const repoVersion = require('./constants').repoVersion - /** * IpfsRepo implements all required functionality to read and write to an ipfs repo. * @@ -64,7 +65,7 @@ class IpfsRepo { await this._openRoot() await this.config.set(buildConfig(config)) await this.spec.set(buildDatastoreSpec(config)) - await this.version.set(repoVersion) + await this.version.set(constants.repoVersion) } /** @@ -92,6 +93,16 @@ class IpfsRepo { this.blocks = await blockstore(blocksBaseStore, this.options.storageBackendOptions.blocks) log('creating keystore') this.keys = backends.create('keys', path.join(this.path, 'keys'), this.options) + + const isCompatible = await this.version.check(constants.repoVersion) + if (!isCompatible) { + if (await this._isAutoMigrationEnabled()) { + await this._migrate(constants.repoVersion) + } else { + throw new ERRORS.InvalidRepoVersionError('Incompatible repo versions. Automatic migrations disabled. Please migrate the repo manually.') + } + } + this.closed = false log('all opened') } catch (err) { @@ -176,7 +187,7 @@ class IpfsRepo { [config] = await Promise.all([ this.config.exists(), this.spec.exists(), - this.version.check(repoVersion) + this.version.exists() ]) } catch (err) { if (err.code === 'ERR_NOT_FOUND') { @@ -240,8 +251,7 @@ class IpfsRepo { */ async stat (options) { options = Object.assign({}, { human: false }, options) - let storageMax, blocks, version, datastore, keys - [storageMax, blocks, version, datastore, keys] = await Promise.all([ + const [storageMax, blocks, version, datastore, keys] = await Promise.all([ this._storageMaxStat(), this._blockStat(), this.version.get(), @@ -264,6 +274,37 @@ class IpfsRepo { } } + async _isAutoMigrationEnabled () { + if (this.options.autoMigrate !== undefined) { + return this.options.autoMigrate + } + + let autoMigrateConfig + try { + autoMigrateConfig = await this.config.get(AUTO_MIGRATE_CONFIG_KEY) + } catch (e) { + if (e.code === ERRORS.NotFoundError.code) { + autoMigrateConfig = true // Config's default value is True + } else { + throw e + } + } + + return autoMigrateConfig + } + + async _migrate (toVersion) { + const currentRepoVersion = await this.version.get() + + if (currentRepoVersion > toVersion) { + log('reverting to version ' + toVersion) + return migrator.revert(this.path, toVersion, { ignoreLock: true, repoOptions: this.options }) + } else { + log('migrating to version ' + toVersion) + return migrator.migrate(this.path, toVersion, { ignoreLock: true, repoOptions: this.options }) + } + } + async _storageMaxStat () { try { const max = await this.config.get('Datastore.StorageMax') @@ -289,7 +330,7 @@ class IpfsRepo { } async function getSize (queryFn) { - let sum = new Big(0) + const sum = new Big(0) for await (const block of queryFn.query({})) { sum.plus(block.value.byteLength) .plus(block.key._buf.byteLength) @@ -299,7 +340,7 @@ async function getSize (queryFn) { module.exports = IpfsRepo module.exports.utils = { blockstore: require('./blockstore-utils') } -module.exports.repoVersion = repoVersion +module.exports.repoVersion = constants.repoVersion module.exports.errors = ERRORS function buildOptions (_options) { diff --git a/src/version.js b/src/version.js index cd0db0ce..c7df0960 100644 --- a/src/version.js +++ b/src/version.js @@ -3,7 +3,6 @@ const Key = require('interface-datastore').Key const debug = require('debug') const log = debug('repo:version') -const errcode = require('err-code') const versionKey = new Key('version') @@ -36,9 +35,9 @@ module.exports = (store) => { return store.put(versionKey, Buffer.from(String(version))) }, /** - * Check the current version, and return an error on missmatch + * Check the current version, and returns true if versions matches * @param {number} expected - * @returns {void} + * @returns {boolean} */ async check (expected) { const version = await this.get() @@ -47,9 +46,7 @@ module.exports = (store) => { // TODO: Clean up the compatibility logic. Repo feature detection would be ideal, or a better version schema const compatibleVersion = (version === 6 && expected === 7) || (expected === 6 && version === 7) - if (version !== expected && !compatibleVersion) { - throw errcode(new Error(`ipfs repo needs migration: expected version v${expected}, found version v${version}`), 'ERR_INVALID_REPO_VERSION') - } + return version === expected || compatibleVersion } } } diff --git a/test/blockstore-test.js b/test/blockstore-test.js index 07ca44e6..128b4ce8 100644 --- a/test/blockstore-test.js +++ b/test/blockstore-test.js @@ -85,9 +85,11 @@ module.exports = (repo) => { close () { } + has () { return true } + batch () { return { put () { @@ -217,6 +219,7 @@ module.exports = (repo) => { close () { } + get (c) { if (c.toString() === key.toString()) { throw err diff --git a/test/browser.js b/test/browser.js index b5f61670..32a5e23b 100644 --- a/test/browser.js +++ b/test/browser.js @@ -4,8 +4,21 @@ const IPFSRepo = require('../src') +async function createTempRepo (options = {}) { + const date = Date.now().toString() + const repoPath = 'test-repo-for-' + date + + const repo = new IPFSRepo(repoPath, options) + await repo.init({}) + await repo.open() + + return repo +} + describe('IPFS Repo Tests on the Browser', () => { require('./options-test') + require('./migrations-test')(createTempRepo) + const repo = new IPFSRepo('myrepo') before(async () => { diff --git a/test/migrations-test.js b/test/migrations-test.js new file mode 100644 index 00000000..3a00e780 --- /dev/null +++ b/test/migrations-test.js @@ -0,0 +1,132 @@ +/* eslint max-nested-callbacks: ["error", 8] */ +/* eslint-env mocha */ +'use strict' + +const chai = require('chai') +const dirtyChai = require('dirty-chai') +const expect = chai.expect +chai.use(dirtyChai) +const sinon = require('sinon') + +const migrator = require('ipfs-repo-migrations') +const constants = require('../src/constants') +const errors = require('../src/errors') +const IPFSRepo = require('../src') + +module.exports = (createTempRepo) => { + describe('Migrations tests', () => { + let repo + let migrateStub + let revertStub + let repoVersionStub + let getLatestMigrationVersionStub + + before(() => { + repoVersionStub = sinon.stub(constants, 'repoVersion') + migrateStub = sinon.stub(migrator, 'migrate') + revertStub = sinon.stub(migrator, 'revert') + getLatestMigrationVersionStub = sinon.stub(migrator, 'getLatestMigrationVersion') + }) + + after(() => { + repoVersionStub.restore() + migrateStub.restore() + revertStub.restore() + getLatestMigrationVersionStub.restore() + }) + + beforeEach(async () => { + repo = await createTempRepo() + sinon.reset() + }) + + // Testing migration logic + const migrationLogic = [ + { config: true, option: true, result: true }, + { config: true, option: false, result: false }, + { config: true, option: undefined, result: true }, + { config: false, option: true, result: true }, + { config: false, option: false, result: false }, + { config: false, option: undefined, result: false }, + { config: undefined, option: true, result: true }, + { config: undefined, option: false, result: false }, + { config: undefined, option: undefined, result: true } + ] + + migrationLogic.forEach(({ config, option, result }) => { + it(`should ${result ? '' : 'not '}migrate when config=${config} and option=${option}`, async () => { + migrateStub.resolves() + repoVersionStub.value(8) + getLatestMigrationVersionStub.returns(9) + + if (config !== undefined) { + await repo.config.set('repoAutoMigrate', config) + } + await repo.version.set(7) + await repo.close() + + const newOpts = Object.assign({}, repo.options) + newOpts.autoMigrate = option + const newRepo = new IPFSRepo(repo.path, newOpts) + + expect(migrateStub.called).to.be.false() + + try { + await newRepo.open() + if (!result) expect.fail('should have thrown error') + } catch (err) { + expect(err.code).to.equal(errors.InvalidRepoVersionError.code) + } + + expect(migrateStub.called).to.eq(result) + }) + }) + + it('should migrate by default', async () => { + migrateStub.resolves() + repoVersionStub.value(8) + getLatestMigrationVersionStub.returns(9) + + await repo.version.set(7) + await repo.close() + + expect(migrateStub.called).to.be.false() + + await repo.open() + + expect(migrateStub.called).to.be.true() + }) + + it('should not migrate when versions matches', async () => { + migrateStub.resolves() + repoVersionStub.value(8) + + await repo.version.set(8) + await repo.close() + + expect(migrateStub.called).to.be.false() + + await repo.open() + + expect(migrateStub.called).to.be.false() + }) + + it('should revert when current repo versions is higher then expected', async () => { + revertStub.resolves() + repoVersionStub.value(8) + + expect(revertStub.called).to.be.false() + + await repo.version.set(9) + await repo.close() + + expect(migrateStub.called).to.be.false() + expect(revertStub.called).to.be.false() + + await repo.open() + + expect(revertStub.called).to.be.true() + expect(migrateStub.called).to.be.false() + }) + }) +} diff --git a/test/node.js b/test/node.js index 9fd46ca7..970b2724 100644 --- a/test/node.js +++ b/test/node.js @@ -6,6 +6,7 @@ const rimraf = require('rimraf') const fs = require('fs') const path = require('path') const promisify = require('util').promisify +const os = require('os') const chai = require('chai') chai.use(require('dirty-chai')) @@ -16,8 +17,18 @@ const fsstat = promisify(fs.stat) const IPFSRepo = require('../src') +async function createTempRepo (options = {}) { + const date = Date.now().toString() + const repoPath = path.join(os.tmpdir(), 'test-repo-for-' + date) + await asyncNcp(path.join(__dirname, 'test-repo'), repoPath) + const repo = new IPFSRepo(repoPath, options) + await repo.open() + return repo +} + describe('IPFS Repo Tests onNode.js', () => { require('./options-test') + require('./migrations-test')(createTempRepo) const customLock = { lockName: 'test.lock', @@ -73,7 +84,7 @@ describe('IPFS Repo Tests onNode.js', () => { repos.forEach((r) => describe(r.name, () => { const testRepoPath = path.join(__dirname, 'test-repo') const date = Date.now().toString() - const repoPath = testRepoPath + '-for-' + date + const repoPath = path.join(os.tmpdir(), 'test-repo-for-' + date) const repo = new IPFSRepo(repoPath, r.opts) diff --git a/test/repo-test.js b/test/repo-test.js index ef23c583..d4f62e21 100644 --- a/test/repo-test.js +++ b/test/repo-test.js @@ -72,41 +72,29 @@ module.exports = (repo) => { expect(await repo.version.get()).to.equal(v1) }) - it('succeeds when requested version is the same as the actual version', async () => { + it('returns true when requested version is the same as the actual version', async () => { await repo.version.set(5) - await repo.version.check(5) + expect(await repo.version.check(5)).to.be.true() }) - it('throws when requesting a past version', async () => { + it('returns false when requesting a past version', async () => { await repo.version.set(5) - - try { - await repo.version.check(4) - throw new Error('Should have thrown error') - } catch (err) { - expect(err.code).to.equal('ERR_INVALID_REPO_VERSION') - } + expect(await repo.version.check(4)).to.be.false() }) - it('throws when requesting a future version', async () => { + it('returns false when requesting a future version', async () => { await repo.version.set(1) - - try { - await repo.version.check(2) - throw new Error('Should have thrown error') - } catch (err) { - expect(err.code).to.equal('ERR_INVALID_REPO_VERSION') - } + expect(await repo.version.check(2)).to.be.false() }) it('treats v6 and v7 as the same', async () => { await repo.version.set(7) - await repo.version.check(6) + expect(await repo.version.check(6)).to.be.true() }) it('treats v7 and v6 as the same', async () => { await repo.version.set(6) - await repo.version.check(7) + expect(await repo.version.check(7)).to.be.true() }) })