From 8b5df69c2c99096c545c461bfb1dc0486c907629 Mon Sep 17 00:00:00 2001 From: Adam Uhlir Date: Fri, 28 Jun 2019 12:21:04 +0200 Subject: [PATCH 01/11] feat: integration of js-ipfs-repo-migrations Integration of js-ipfs-repo-migrations brings automatic repo migrations to ipfs-repo (both in-browser and fs). It is possible to control the automatic migration using either config's setting 'repoDisableAutoMigration' or IPFSRepo's option 'disableAutoMigration'. License: MIT Signed-off-by: Adam Uhlir --- README.md | 6 ++ package.json | 4 +- src/default-options-browser.js | 1 + src/default-options.js | 1 + src/errors/index.js | 15 ++++ src/index.js | 55 +++++++++++-- src/version.js | 9 +- test/browser.js | 21 +++++ test/migrations-test.js | 146 +++++++++++++++++++++++++++++++++ test/node.js | 36 +++++++- test/options-test.js | 1 + test/repo-test.js | 28 ++----- 12 files changed, 290 insertions(+), 33 deletions(-) create mode 100644 test/migrations-test.js diff --git a/README.md b/README.md index c48600eb..6fc4eee1 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) @@ -318,6 +319,11 @@ 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`). + ## Contribute There are some ways you can make this module better: diff --git a/package.json b/package.json index ce6ee028..3d3fab93 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,8 @@ "multihashes": "~0.4.14", "multihashing-async": "~0.7.0", "ncp": "^2.0.0", - "rimraf": "^2.6.3" + "rimraf": "^2.6.3", + "sinon": "^7.3.1" }, "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": "AuHau/js-ipfs-repo-migrations#dev", "just-safe-get": "^1.3.0", "just-safe-set": "^2.1.0", "lodash.has": "^4.5.2", diff --git a/src/default-options-browser.js b/src/default-options-browser.js index 9a848d4b..227d327e 100644 --- a/src/default-options-browser.js +++ b/src/default-options-browser.js @@ -2,6 +2,7 @@ // Default configuration for a repo in the browser module.exports = { + disableAutoMigration: false, lock: 'memory', storageBackends: { root: require('datastore-level'), diff --git a/src/default-options.js b/src/default-options.js index e9322d67..e5f6a5f7 100644 --- a/src/default-options.js +++ b/src/default-options.js @@ -2,6 +2,7 @@ // Default configuration for a repo in node.js module.exports = { + disableAutoMigration: false, lock: 'fs', storageBackends: { root: require('datastore-fs'), diff --git a/src/errors/index.js b/src/errors/index.js index 5a1d7327..fbe05711 100644 --- a/src/errors/index.js +++ b/src/errors/index.js @@ -30,6 +30,21 @@ class NotFoundError extends Error { 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 = 'ERR_INVALID_REPO_VERSION' + this.message = message + } +} + +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..22cc9b50 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') @@ -26,8 +28,6 @@ const lockers = { fs: require('./lock') } -const repoVersion = require('./constants').repoVersion - /** * IpfsRepo implements all required functionality to read and write to an ipfs repo. * @@ -64,7 +64,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 +92,17 @@ 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) + + if (!await this.version.check(constants.repoVersion)) { + log('Something is fishy') + if (!this.options.disableAutoMigration) { + log('Let see what') + 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') { @@ -264,6 +275,40 @@ class IpfsRepo { } } + async _migrate (toVersion) { + let disableMigrationsConfig + try { + disableMigrationsConfig = await this.config.get('repoDisableAutoMigration') + } catch (e) { + if (e.code === ERRORS.NotFoundError.code) { + disableMigrationsConfig = false + } else { + throw e + } + } + + if (disableMigrationsConfig) { + throw new ERRORS.InvalidRepoVersionError('Incompatible repo versions. Automatic migrations disabled. Please migrate the repo manually.') + } + + const currentRepoVersion = await this.version.get() + log(currentRepoVersion) + if (currentRepoVersion >= toVersion) { + if (currentRepoVersion > toVersion) { + log('Your repo\'s version is higher then this version of js-ipfs-repo require! You should revert it.') + } + + log('Nothing to migrate') + return + } + + if (toVersion > migrator.getLatestMigrationVersion()) { + throw new Error('The ipfs-repo-migrations package does not have migration for version: ' + toVersion) + } + + return migrator.migrate(this.path, { toVersion: toVersion, ignoreLock: true, repoOptions: this.options }) + } + async _storageMaxStat () { try { const max = await this.config.get('Datastore.StorageMax') @@ -299,7 +344,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/browser.js b/test/browser.js index b5f61670..f3d7ce44 100644 --- a/test/browser.js +++ b/test/browser.js @@ -4,8 +4,29 @@ const IPFSRepo = require('../src') +async function createTempRepo ({ dontOpen, opts }) { + const date = Date.now().toString() + const repoPath = 'test-repo-for-' + date + + const repo = new IPFSRepo(repoPath, opts) + await repo.init({}) + + if (!dontOpen) { + await repo.open() + } + + return { + path: repoPath, + instance: repo, + teardown: async () => { + } + } +} + 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..3a3e6e05 --- /dev/null +++ b/test/migrations-test.js @@ -0,0 +1,146 @@ +/* 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 teardown + let repo + let migrateStub + let repoVersionStub + let getLatestMigrationVersionStub + + before(() => { + repoVersionStub = sinon.stub(constants, 'repoVersion') + migrateStub = sinon.stub(migrator, 'migrate') + getLatestMigrationVersionStub = sinon.stub(migrator, 'getLatestMigrationVersion') + }) + + after(() => { + repoVersionStub.restore() + migrateStub.restore() + getLatestMigrationVersionStub.restore() + }) + + beforeEach(async () => { + ({ instance: repo, teardown } = await createTempRepo({})) + sinon.reset() + }) + + afterEach(async () => { + await teardown() + }) + + 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 option disableAutoMigration is true', async () => { + migrateStub.resolves() + repoVersionStub.resolves(8) + getLatestMigrationVersionStub.returns(9) + + await repo.version.set(7) + await repo.close() + + const newOpts = Object.assign({}, repo.options) + newOpts.disableAutoMigration = true + const newRepo = new IPFSRepo(repo.path, newOpts) + + expect(migrateStub.called).to.be.false() + try { + await newRepo.open() + throw Error('Should throw error') + } catch (e) { + expect(e.code).to.equal(errors.InvalidRepoVersionError.code) + } + + expect(migrateStub.called).to.be.false() + }) + + it('should not migrate when config option repoDisableAutoMigration is true', async () => { + migrateStub.resolves() + repoVersionStub.resolves(8) + getLatestMigrationVersionStub.returns(9) + + await repo.config.set('repoDisableAutoMigration', true) + await repo.version.set(7) + await repo.close() + + expect(migrateStub.called).to.be.false() + try { + await repo.open() + throw Error('Should throw error') + } catch (e) { + expect(migrateStub.called).to.be.false() + expect(e.code).to.equal(errors.InvalidRepoVersionError.code) + } + }) + + 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 not migrate when current repo versions is higher then expected', async () => { + migrateStub.resolves() + repoVersionStub.value(8) + + await repo.version.set(9) + await repo.close() + + expect(migrateStub.called).to.be.false() + + await repo.open() + + expect(migrateStub.called).to.be.false() + }) + + it('should throw error if ipfs-repo-migrations does not contain expected migration', async () => { + migrateStub.resolves() + repoVersionStub.value(8) + getLatestMigrationVersionStub.returns(7) + + await repo.version.set(7) + await repo.close() + + try { + await repo.open() + throw Error('Should throw') + } catch (e) { + expect(e.message).to.include('package does not have migration') + } + }) + }) +} diff --git a/test/node.js b/test/node.js index 9fd46ca7..fbed965d 100644 --- a/test/node.js +++ b/test/node.js @@ -16,8 +16,39 @@ const fsstat = promisify(fs.stat) const IPFSRepo = require('../src') +async function createTempRepo ({ init, dontOpen, opts }) { + const testRepoPath = path.join(__dirname, 'test-repo') + const date = Date.now().toString() + const repoPath = testRepoPath + '-for-' + date + + const repo = new IPFSRepo(repoPath, opts) + + if (init) { + await repo.init({}) + } else { + await asyncNcp(testRepoPath, repoPath) + } + + if (!dontOpen) { + await repo.open() + } + + return { + path: repoPath, + instance: repo, + teardown: async () => { + try { + await repo.close() + } catch (e) { + } + await asyncRimraf(repoPath) + } + } +} + describe('IPFS Repo Tests onNode.js', () => { require('./options-test') + require('./migrations-test')(createTempRepo) const customLock = { lockName: 'test.lock', @@ -87,7 +118,10 @@ describe('IPFS Repo Tests onNode.js', () => { }) after(async () => { - await repo.close() + try { + await repo.close() + } catch (e) { + } await asyncRimraf(repoPath) }) diff --git a/test/options-test.js b/test/options-test.js index 8b2be95e..e1e8cddd 100644 --- a/test/options-test.js +++ b/test/options-test.js @@ -66,6 +66,7 @@ function noop () {} function expectedRepoOptions () { const options = { + disableAutoMigration: false, lock: process.browser ? 'memory' : 'fs', storageBackends: { // packages are exchanged to browser-compatible 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() }) }) From ed669f6698dd311f3d3bc7cfc61b7e4b823518b3 Mon Sep 17 00:00:00 2001 From: Adam Uhlir Date: Mon, 1 Jul 2019 14:19:06 +0200 Subject: [PATCH 02/11] Throw error when repo's version is higher then expected License: MIT Signed-off-by: Adam Uhlir --- src/index.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/index.js b/src/index.js index 22cc9b50..0a686810 100644 --- a/src/index.js +++ b/src/index.js @@ -293,11 +293,12 @@ class IpfsRepo { const currentRepoVersion = await this.version.get() log(currentRepoVersion) - if (currentRepoVersion >= toVersion) { - if (currentRepoVersion > toVersion) { - log('Your repo\'s version is higher then this version of js-ipfs-repo require! You should revert it.') - } + if (currentRepoVersion > toVersion) { + throw new ERRORS.InvalidRepoVersionError('Your repo\'s version is higher then this version of js-ipfs-repo require! You have to revert it.') + } + + if (currentRepoVersion === toVersion) { log('Nothing to migrate') return } From 7d998c52a0c24c4f6ccedb14a7ba6e527389633d Mon Sep 17 00:00:00 2001 From: Adam Uhlir Date: Tue, 23 Jul 2019 22:59:59 +0200 Subject: [PATCH 03/11] Tweaks License: MIT Signed-off-by: Adam Uhlir --- README.md | 1 + src/default-options-browser.js | 2 +- src/default-options.js | 2 +- src/index.js | 30 +++++++++++++----------------- test/migrations-test.js | 28 +++++++++------------------- test/options-test.js | 2 +- 6 files changed, 26 insertions(+), 39 deletions(-) diff --git a/README.md b/README.md index 6fc4eee1..ed5c4694 100644 --- a/README.md +++ b/README.md @@ -137,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()`) diff --git a/src/default-options-browser.js b/src/default-options-browser.js index 227d327e..3e36cfef 100644 --- a/src/default-options-browser.js +++ b/src/default-options-browser.js @@ -2,7 +2,7 @@ // Default configuration for a repo in the browser module.exports = { - disableAutoMigration: false, + autoMigrate: true, lock: 'memory', storageBackends: { root: require('datastore-level'), diff --git a/src/default-options.js b/src/default-options.js index e5f6a5f7..913a264e 100644 --- a/src/default-options.js +++ b/src/default-options.js @@ -2,7 +2,7 @@ // Default configuration for a repo in node.js module.exports = { - disableAutoMigration: false, + autoMigrate: true, lock: 'fs', storageBackends: { root: require('datastore-fs'), diff --git a/src/index.js b/src/index.js index 0a686810..f26eb6b8 100644 --- a/src/index.js +++ b/src/index.js @@ -22,6 +22,7 @@ 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'), @@ -93,10 +94,9 @@ class IpfsRepo { log('creating keystore') this.keys = backends.create('keys', path.join(this.path, 'keys'), this.options) - if (!await this.version.check(constants.repoVersion)) { - log('Something is fishy') - if (!this.options.disableAutoMigration) { - log('Let see what') + 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.') @@ -275,38 +275,34 @@ class IpfsRepo { } } - async _migrate (toVersion) { - let disableMigrationsConfig + async _isAutoMigrationEnabled () { + let autoMigrateConfig try { - disableMigrationsConfig = await this.config.get('repoDisableAutoMigration') + autoMigrateConfig = await this.config.get(AUTO_MIGRATE_CONFIG_KEY) } catch (e) { if (e.code === ERRORS.NotFoundError.code) { - disableMigrationsConfig = false + autoMigrateConfig = true } else { throw e } } + log(`optin: ${this.options.autoMigrate}; config: ${autoMigrateConfig}`) - if (disableMigrationsConfig) { - throw new ERRORS.InvalidRepoVersionError('Incompatible repo versions. Automatic migrations disabled. Please migrate the repo manually.') - } + return autoMigrateConfig && this.options.autoMigrate + } + async _migrate (toVersion) { const currentRepoVersion = await this.version.get() - log(currentRepoVersion) if (currentRepoVersion > toVersion) { throw new ERRORS.InvalidRepoVersionError('Your repo\'s version is higher then this version of js-ipfs-repo require! You have to revert it.') } if (currentRepoVersion === toVersion) { - log('Nothing to migrate') return } - if (toVersion > migrator.getLatestMigrationVersion()) { - throw new Error('The ipfs-repo-migrations package does not have migration for version: ' + toVersion) - } - + log('migrating to version ' + toVersion) return migrator.migrate(this.path, { toVersion: toVersion, ignoreLock: true, repoOptions: this.options }) } diff --git a/test/migrations-test.js b/test/migrations-test.js index 3a3e6e05..84437858 100644 --- a/test/migrations-test.js +++ b/test/migrations-test.js @@ -57,7 +57,7 @@ module.exports = (createTempRepo) => { expect(migrateStub.called).to.be.true() }) - it('should not migrate when option disableAutoMigration is true', async () => { + it('should not migrate when option autoMigrate is false', async () => { migrateStub.resolves() repoVersionStub.resolves(8) getLatestMigrationVersionStub.returns(9) @@ -66,7 +66,7 @@ module.exports = (createTempRepo) => { await repo.close() const newOpts = Object.assign({}, repo.options) - newOpts.disableAutoMigration = true + newOpts.autoMigrate = false const newRepo = new IPFSRepo(repo.path, newOpts) expect(migrateStub.called).to.be.false() @@ -80,12 +80,12 @@ module.exports = (createTempRepo) => { expect(migrateStub.called).to.be.false() }) - it('should not migrate when config option repoDisableAutoMigration is true', async () => { + it('should not migrate when config option repoAutoMigrate is false', async () => { migrateStub.resolves() repoVersionStub.resolves(8) getLatestMigrationVersionStub.returns(9) - await repo.config.set('repoDisableAutoMigration', true) + await repo.config.set('repoAutoMigrate', false) await repo.version.set(7) await repo.close() @@ -122,25 +122,15 @@ module.exports = (createTempRepo) => { expect(migrateStub.called).to.be.false() - await repo.open() - - expect(migrateStub.called).to.be.false() - }) - - it('should throw error if ipfs-repo-migrations does not contain expected migration', async () => { - migrateStub.resolves() - repoVersionStub.value(8) - getLatestMigrationVersionStub.returns(7) - - await repo.version.set(7) - await repo.close() - try { await repo.open() - throw Error('Should throw') + throw Error('Should throw error') } catch (e) { - expect(e.message).to.include('package does not have migration') + expect(migrateStub.called).to.be.false() + expect(e.code).to.equal(errors.InvalidRepoVersionError.code) } + + expect(migrateStub.called).to.be.false() }) }) } diff --git a/test/options-test.js b/test/options-test.js index e1e8cddd..17b688c7 100644 --- a/test/options-test.js +++ b/test/options-test.js @@ -66,7 +66,7 @@ function noop () {} function expectedRepoOptions () { const options = { - disableAutoMigration: false, + autoMigrate: true, lock: process.browser ? 'memory' : 'fs', storageBackends: { // packages are exchanged to browser-compatible From 1ba2851925b35f70ae00c104ff173f28c34df32f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= Date: Thu, 10 Oct 2019 17:50:37 +0200 Subject: [PATCH 04/11] fix: readme changes --- README.md | 4 ++++ package.json | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ed5c4694..9afb534c 100644 --- a/README.md +++ b/README.md @@ -325,6 +325,10 @@ Returned promise resolves to a `boolean` indicating the existence of the lock. 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 3d3fab93..0a34dbfd 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "err-code": "^1.1.2", "interface-datastore": "~0.7.0", "ipfs-block": "~0.8.1", - "ipfs-repo-migrations": "AuHau/js-ipfs-repo-migrations#dev", + "ipfs-repo-migrations": "ipfs/js-ipfs-repo-migrations#dev", "just-safe-get": "^1.3.0", "just-safe-set": "^2.1.0", "lodash.has": "^4.5.2", From 930e6db9a3858d0b867c5e7d93fcda44d6deca2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= Date: Thu, 17 Oct 2019 18:27:36 +0200 Subject: [PATCH 05/11] fix: tests setup --- package.json | 2 +- test/migrations-test.js | 7 +------ test/node.js | 10 ++-------- 3 files changed, 4 insertions(+), 15 deletions(-) diff --git a/package.json b/package.json index 0a34dbfd..0f30b3bc 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "err-code": "^1.1.2", "interface-datastore": "~0.7.0", "ipfs-block": "~0.8.1", - "ipfs-repo-migrations": "ipfs/js-ipfs-repo-migrations#dev", + "ipfs-repo-migrations": "ipfs/js-ipfs-repo-migrations#master", "just-safe-get": "^1.3.0", "just-safe-set": "^2.1.0", "lodash.has": "^4.5.2", diff --git a/test/migrations-test.js b/test/migrations-test.js index 84437858..d30640a5 100644 --- a/test/migrations-test.js +++ b/test/migrations-test.js @@ -15,7 +15,6 @@ const IPFSRepo = require('../src') module.exports = (createTempRepo) => { describe('Migrations tests', () => { - let teardown let repo let migrateStub let repoVersionStub @@ -34,14 +33,10 @@ module.exports = (createTempRepo) => { }) beforeEach(async () => { - ({ instance: repo, teardown } = await createTempRepo({})) + ({ instance: repo } = await createTempRepo({})) sinon.reset() }) - afterEach(async () => { - await teardown() - }) - it('should migrate by default', async () => { migrateStub.resolves() repoVersionStub.value(8) diff --git a/test/node.js b/test/node.js index fbed965d..7a3d212a 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')) @@ -19,7 +20,7 @@ const IPFSRepo = require('../src') async function createTempRepo ({ init, dontOpen, opts }) { 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, opts) @@ -36,13 +37,6 @@ async function createTempRepo ({ init, dontOpen, opts }) { return { path: repoPath, instance: repo, - teardown: async () => { - try { - await repo.close() - } catch (e) { - } - await asyncRimraf(repoPath) - } } } From e5f742c1711bb8445c9b72a1102e08db69dffd4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= Date: Thu, 17 Oct 2019 18:34:50 +0200 Subject: [PATCH 06/11] style: lint --- test/node.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/node.js b/test/node.js index 7a3d212a..cab5666d 100644 --- a/test/node.js +++ b/test/node.js @@ -36,7 +36,7 @@ async function createTempRepo ({ init, dontOpen, opts }) { return { path: repoPath, - instance: repo, + instance: repo } } From 5657037b86ef30fc8385fbb7dfa66221aef8e626 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= Date: Thu, 17 Oct 2019 19:17:43 +0200 Subject: [PATCH 07/11] fix: tweaks --- src/index.js | 7 +------ test/browser.js | 4 +--- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/src/index.js b/src/index.js index f26eb6b8..270ba9c2 100644 --- a/src/index.js +++ b/src/index.js @@ -281,12 +281,11 @@ class IpfsRepo { autoMigrateConfig = await this.config.get(AUTO_MIGRATE_CONFIG_KEY) } catch (e) { if (e.code === ERRORS.NotFoundError.code) { - autoMigrateConfig = true + autoMigrateConfig = true // Config's default value is True } else { throw e } } - log(`optin: ${this.options.autoMigrate}; config: ${autoMigrateConfig}`) return autoMigrateConfig && this.options.autoMigrate } @@ -298,10 +297,6 @@ class IpfsRepo { throw new ERRORS.InvalidRepoVersionError('Your repo\'s version is higher then this version of js-ipfs-repo require! You have to revert it.') } - if (currentRepoVersion === toVersion) { - return - } - log('migrating to version ' + toVersion) return migrator.migrate(this.path, { toVersion: toVersion, ignoreLock: true, repoOptions: this.options }) } diff --git a/test/browser.js b/test/browser.js index f3d7ce44..3cf1fbf2 100644 --- a/test/browser.js +++ b/test/browser.js @@ -17,9 +17,7 @@ async function createTempRepo ({ dontOpen, opts }) { return { path: repoPath, - instance: repo, - teardown: async () => { - } + instance: repo } } From c73f9d2b2e54a2e3033e92282aa2a09a0254ab87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= Date: Thu, 24 Oct 2019 12:04:14 +0200 Subject: [PATCH 08/11] feat: automatic reversion of repo --- package.json | 2 +- src/index.js | 9 +++++---- test/migrations-test.js | 31 ++++++++++++++++--------------- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index 0f30b3bc..5bf4622c 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "err-code": "^1.1.2", "interface-datastore": "~0.7.0", "ipfs-block": "~0.8.1", - "ipfs-repo-migrations": "ipfs/js-ipfs-repo-migrations#master", + "ipfs-repo-migrations": "github:ipfs/js-ipfs-repo-migrations#master", "just-safe-get": "^1.3.0", "just-safe-set": "^2.1.0", "lodash.has": "^4.5.2", diff --git a/src/index.js b/src/index.js index 270ba9c2..cf2b6b32 100644 --- a/src/index.js +++ b/src/index.js @@ -294,11 +294,12 @@ class IpfsRepo { const currentRepoVersion = await this.version.get() if (currentRepoVersion > toVersion) { - throw new ERRORS.InvalidRepoVersionError('Your repo\'s version is higher then this version of js-ipfs-repo require! You have to revert it.') + 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 }) } - - log('migrating to version ' + toVersion) - return migrator.migrate(this.path, { toVersion: toVersion, ignoreLock: true, repoOptions: this.options }) } async _storageMaxStat () { diff --git a/test/migrations-test.js b/test/migrations-test.js index d30640a5..198a04f3 100644 --- a/test/migrations-test.js +++ b/test/migrations-test.js @@ -17,18 +17,21 @@ 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() }) @@ -67,9 +70,9 @@ module.exports = (createTempRepo) => { expect(migrateStub.called).to.be.false() try { await newRepo.open() - throw Error('Should throw error') - } catch (e) { - expect(e.code).to.equal(errors.InvalidRepoVersionError.code) + expect.fail('should have thrown error') + } catch (err) { + expect(err.code).to.equal(errors.InvalidRepoVersionError.code) } expect(migrateStub.called).to.be.false() @@ -87,10 +90,10 @@ module.exports = (createTempRepo) => { expect(migrateStub.called).to.be.false() try { await repo.open() - throw Error('Should throw error') - } catch (e) { + expect.fail('should have thrown error') + } catch (err) { expect(migrateStub.called).to.be.false() - expect(e.code).to.equal(errors.InvalidRepoVersionError.code) + expect(err.code).to.equal(errors.InvalidRepoVersionError.code) } }) @@ -108,23 +111,21 @@ module.exports = (createTempRepo) => { expect(migrateStub.called).to.be.false() }) - it('should not migrate when current repo versions is higher then expected', async () => { - migrateStub.resolves() + 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() - try { - await repo.open() - throw Error('Should throw error') - } catch (e) { - expect(migrateStub.called).to.be.false() - expect(e.code).to.equal(errors.InvalidRepoVersionError.code) - } + await repo.open() + expect(revertStub.called).to.be.true() expect(migrateStub.called).to.be.false() }) }) From be948935d80e37eb5f4582a17019f82e2639d57b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= Date: Wed, 6 Nov 2019 10:50:49 +0100 Subject: [PATCH 09/11] fix: pr feedback --- src/default-options-browser.js | 1 - src/default-options.js | 1 - src/errors/index.js | 9 ++-- src/index.js | 11 +++-- test/blockstore-test.js | 3 ++ test/browser.js | 14 ++---- test/migrations-test.js | 84 +++++++++++++++++----------------- test/node.js | 31 +++---------- test/options-test.js | 1 - 9 files changed, 66 insertions(+), 89 deletions(-) diff --git a/src/default-options-browser.js b/src/default-options-browser.js index 3e36cfef..9a848d4b 100644 --- a/src/default-options-browser.js +++ b/src/default-options-browser.js @@ -2,7 +2,6 @@ // Default configuration for a repo in the browser module.exports = { - autoMigrate: true, lock: 'memory', storageBackends: { root: require('datastore-level'), diff --git a/src/default-options.js b/src/default-options.js index 913a264e..e9322d67 100644 --- a/src/default-options.js +++ b/src/default-options.js @@ -2,7 +2,6 @@ // Default configuration for a repo in node.js module.exports = { - autoMigrate: true, lock: 'fs', storageBackends: { root: require('datastore-fs'), diff --git a/src/errors/index.js b/src/errors/index.js index fbe05711..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,8 +21,7 @@ class NotFoundError extends Error { constructor (message) { super(message) this.name = 'NotFoundError' - this.code = 'ERR_NOT_FOUND' - this.message = message + this.code = NotFoundError.code } } @@ -37,8 +35,7 @@ class InvalidRepoVersionError extends Error { constructor (message) { super(message) this.name = 'InvalidRepoVersionError' - this.code = 'ERR_INVALID_REPO_VERSION' - this.message = message + this.code = InvalidRepoVersionError.code } } diff --git a/src/index.js b/src/index.js index cf2b6b32..83e0c9dd 100644 --- a/src/index.js +++ b/src/index.js @@ -251,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(), @@ -276,6 +275,10 @@ 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) @@ -287,7 +290,7 @@ class IpfsRepo { } } - return autoMigrateConfig && this.options.autoMigrate + return autoMigrateConfig } async _migrate (toVersion) { @@ -327,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) 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 3cf1fbf2..32a5e23b 100644 --- a/test/browser.js +++ b/test/browser.js @@ -4,21 +4,15 @@ const IPFSRepo = require('../src') -async function createTempRepo ({ dontOpen, opts }) { +async function createTempRepo (options = {}) { const date = Date.now().toString() const repoPath = 'test-repo-for-' + date - const repo = new IPFSRepo(repoPath, opts) + const repo = new IPFSRepo(repoPath, options) await repo.init({}) + await repo.open() - if (!dontOpen) { - await repo.open() - } - - return { - path: repoPath, - instance: repo - } + return repo } describe('IPFS Repo Tests on the Browser', () => { diff --git a/test/migrations-test.js b/test/migrations-test.js index 198a04f3..3a00e780 100644 --- a/test/migrations-test.js +++ b/test/migrations-test.js @@ -36,65 +36,65 @@ module.exports = (createTempRepo) => { }) beforeEach(async () => { - ({ instance: repo } = await createTempRepo({})) + repo = await createTempRepo() sinon.reset() }) - it('should migrate by default', async () => { - migrateStub.resolves() - repoVersionStub.value(8) - getLatestMigrationVersionStub.returns(9) - - await repo.version.set(7) - await repo.close() + // 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() + expect(migrateStub.called).to.be.false() - await repo.open() + 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.be.true() + expect(migrateStub.called).to.eq(result) + }) }) - it('should not migrate when option autoMigrate is false', async () => { + it('should migrate by default', async () => { migrateStub.resolves() - repoVersionStub.resolves(8) + repoVersionStub.value(8) getLatestMigrationVersionStub.returns(9) await repo.version.set(7) await repo.close() - const newOpts = Object.assign({}, repo.options) - newOpts.autoMigrate = false - const newRepo = new IPFSRepo(repo.path, newOpts) - - expect(migrateStub.called).to.be.false() - try { - await newRepo.open() - expect.fail('should have thrown error') - } catch (err) { - expect(err.code).to.equal(errors.InvalidRepoVersionError.code) - } - expect(migrateStub.called).to.be.false() - }) - - it('should not migrate when config option repoAutoMigrate is false', async () => { - migrateStub.resolves() - repoVersionStub.resolves(8) - getLatestMigrationVersionStub.returns(9) - await repo.config.set('repoAutoMigrate', false) - await repo.version.set(7) - await repo.close() + await repo.open() - expect(migrateStub.called).to.be.false() - try { - await repo.open() - expect.fail('should have thrown error') - } catch (err) { - expect(migrateStub.called).to.be.false() - expect(err.code).to.equal(errors.InvalidRepoVersionError.code) - } + expect(migrateStub.called).to.be.true() }) it('should not migrate when versions matches', async () => { diff --git a/test/node.js b/test/node.js index cab5666d..970b2724 100644 --- a/test/node.js +++ b/test/node.js @@ -17,27 +17,13 @@ const fsstat = promisify(fs.stat) const IPFSRepo = require('../src') -async function createTempRepo ({ init, dontOpen, opts }) { - const testRepoPath = path.join(__dirname, 'test-repo') +async function createTempRepo (options = {}) { const date = Date.now().toString() const repoPath = path.join(os.tmpdir(), 'test-repo-for-' + date) - - const repo = new IPFSRepo(repoPath, opts) - - if (init) { - await repo.init({}) - } else { - await asyncNcp(testRepoPath, repoPath) - } - - if (!dontOpen) { - await repo.open() - } - - return { - path: repoPath, - instance: repo - } + 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', () => { @@ -98,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) @@ -112,10 +98,7 @@ describe('IPFS Repo Tests onNode.js', () => { }) after(async () => { - try { - await repo.close() - } catch (e) { - } + await repo.close() await asyncRimraf(repoPath) }) diff --git a/test/options-test.js b/test/options-test.js index 17b688c7..8b2be95e 100644 --- a/test/options-test.js +++ b/test/options-test.js @@ -66,7 +66,6 @@ function noop () {} function expectedRepoOptions () { const options = { - autoMigrate: true, lock: process.browser ? 'memory' : 'fs', storageBackends: { // packages are exchanged to browser-compatible From e737408e767beebad03d9b7e22f2d901498aa6a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= Date: Wed, 6 Nov 2019 11:31:50 +0100 Subject: [PATCH 10/11] chore: update dev-dependencies --- package.json | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 5bf4622c..cd65606b 100644 --- a/package.json +++ b/package.json @@ -43,16 +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", - "sinon": "^7.3.1" + "rimraf": "^3.0.0", + "sinon": "^7.5.0" }, "dependencies": { "base32.js": "~0.1.0", From 3eb921654d24c74f3ede0ce95820a321affef586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adam=20Uhl=C3=AD=C5=99?= Date: Wed, 6 Nov 2019 11:45:35 +0100 Subject: [PATCH 11/11] chore: add released js-ipfs-repo-migrations --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index cd65606b..c1396ece 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ "err-code": "^1.1.2", "interface-datastore": "~0.7.0", "ipfs-block": "~0.8.1", - "ipfs-repo-migrations": "github:ipfs/js-ipfs-repo-migrations#master", + "ipfs-repo-migrations": "~0.1.0", "just-safe-get": "^1.3.0", "just-safe-set": "^2.1.0", "lodash.has": "^4.5.2",