fix: repo auto-migration regression (#3718)

achingbrain · web-flow · commit b5470d40ea45 · 2021-06-16T13:20:37.000+01:00
The recent js-ipfs-repo release changed how options are merged which resulted in the wrong `autoMigrate` value being used. Pass it in explicitly and add a test to prevent future regressions. Fixes #3712
diff --git a/packages/ipfs-core/package.json b/packages/ipfs-core/package.json
@@ -132,6 +132,7 @@
     "ipld-git": "^0.6.1",
     "iso-url": "^1.0.0",
     "nanoid": "^3.1.12",
+    "p-defer": "^3.0.0",
     "rimraf": "^3.0.2",
     "sinon": "^10.0.1"
   }
diff --git a/packages/ipfs-core/src/components/storage.js b/packages/ipfs-core/src/components/storage.js
@@ -9,7 +9,7 @@ const uint8ArrayToString = require('uint8arrays/to-string')
 const PeerId = require('peer-id')
 const { mergeOptions } = require('../utils')
 const configService = require('./config')
-const { NotEnabledError } = require('../errors')
+const { NotEnabledError, NotInitializedError } = require('../errors')
 const createLibP2P = require('./libp2p')
 
 /**
@@ -45,10 +45,14 @@ class Storage {
    * @param {IPFSOptions} options
    */
   static async start (print, options) {
-    const { repoAutoMigrate, repo: inputRepo } = options
+    const { repoAutoMigrate, repo: inputRepo, onMigrationProgress } = options
 
     const repo = (typeof inputRepo === 'string' || inputRepo == null)
-      ? createRepo(print, { path: inputRepo, autoMigrate: Boolean(repoAutoMigrate) })
+      ? createRepo(print, {
+        path: inputRepo,
+        autoMigrate: repoAutoMigrate,
+        onMigrationProgress: onMigrationProgress
+      })
       : inputRepo
 
     const { peerId, keychain, isNew } = await loadRepo(print, repo, options)
@@ -198,14 +202,16 @@ const configureRepo = async (repo, options) => {
   const profiles = (options.init && options.init.profiles) || []
   const pass = options.pass
   const original = await repo.config.getAll()
-  // @ts-ignore TODO: move config types to repo
   const changed = mergeConfigs(applyProfiles(original, profiles), config)
 
   if (original !== changed) {
     await repo.config.replace(changed)
   }
 
-  // @ts-ignore - Identity may not be present
+  if (!changed.Identity || !changed.Identity.PrivKey) {
+    throw new NotInitializedError('No private key was found in the config, please intialize the repo')
+  }
+
   const peerId = await PeerId.createFromPrivKey(changed.Identity.PrivKey)
   const libp2p = await createLibP2P({
     options: undefined,
diff --git a/packages/ipfs-core/src/runtime/repo-browser.js b/packages/ipfs-core/src/runtime/repo-browser.js
@@ -2,13 +2,21 @@
 
 const IPFSRepo = require('ipfs-repo')
 
+/**
+ * @typedef {import('ipfs-repo-migrations').ProgressCallback} MigrationProgressCallback
+ */
+
 /**
  * @param {import('../types').Print} print
  * @param {object} options
  * @param {string} [options.path]
- * @param {boolean} options.autoMigrate
+ * @param {boolean} [options.autoMigrate]
+ * @param {MigrationProgressCallback} [options.onMigrationProgress]
  */
 module.exports = (print, options) => {
   const repoPath = options.path || 'ipfs'
-  return new IPFSRepo(repoPath, { autoMigrate: options.autoMigrate })
+  return new IPFSRepo(repoPath, {
+    autoMigrate: options.autoMigrate,
+    onMigrationProgress: options.onMigrationProgress || print
+  })
 }
diff --git a/packages/ipfs-core/src/runtime/repo-nodejs.js b/packages/ipfs-core/src/runtime/repo-nodejs.js
@@ -4,25 +4,28 @@ const os = require('os')
 const IPFSRepo = require('ipfs-repo')
 const path = require('path')
 
+/**
+ * @typedef {import('ipfs-repo-migrations').ProgressCallback} MigrationProgressCallback
+ */
+
 /**
  * @param {import('../types').Print} print
  * @param {object} options
  * @param {string} [options.path]
- * @param {boolean} options.autoMigrate
+ * @param {boolean} [options.autoMigrate]
+ * @param {MigrationProgressCallback} [options.onMigrationProgress]
  */
-module.exports = (print, options = { autoMigrate: true }) => {
+module.exports = (print, options = {}) => {
   const repoPath = options.path || path.join(os.homedir(), '.jsipfs')
   /**
    * @type {number}
    */
   let lastMigration
 
   /**
-   * @param {number} version
-   * @param {string} percentComplete
-   * @param {string} message
+   * @type {MigrationProgressCallback}
    */
-  const onMigrationProgress = (version, percentComplete, message) => {
+  const onMigrationProgress = options.onMigrationProgress || function (version, percentComplete, message) {
     if (version !== lastMigration) {
       lastMigration = version
 
@@ -33,7 +36,7 @@ module.exports = (print, options = { autoMigrate: true }) => {
   }
 
   return new IPFSRepo(repoPath, {
-    autoMigrate: options.autoMigrate,
+    autoMigrate: options.autoMigrate != null ? options.autoMigrate : true,
     onMigrationProgress: onMigrationProgress
   })
 }
diff --git a/packages/ipfs-core/test/create-node.spec.js b/packages/ipfs-core/test/create-node.spec.js
@@ -8,19 +8,23 @@ const { isNode } = require('ipfs-utils/src/env')
 const tmpDir = require('ipfs-utils/src/temp-dir')
 const PeerId = require('peer-id')
 const { supportedKeys } = require('libp2p-crypto/src/keys')
-const IPFS = require('../')
+const IPFS = require('../src')
+const defer = require('p-defer')
+const uint8ArrayToString = require('uint8arrays/to-string')
 
 // This gets replaced by `create-repo-browser.js` in the browser
 const createTempRepo = require('./utils/create-repo-nodejs.js')
 
 describe('create node', function () {
   let tempRepo
 
-  beforeEach(() => {
-    tempRepo = createTempRepo()
+  beforeEach(async () => {
+    tempRepo = await createTempRepo()
   })
 
-  afterEach(() => tempRepo.teardown())
+  afterEach(() => {
+    tempRepo.teardown()
+  })
 
   it('should create a node with a custom repo path', async function () {
     this.timeout(80 * 1000)
@@ -240,8 +244,8 @@ describe('create node', function () {
       })
     }
 
-    const repoA = createTempRepo()
-    const repoB = createTempRepo()
+    const repoA = await createTempRepo()
+    const repoB = await createTempRepo()
     const [nodeA, nodeB] = await Promise.all([createNode(repoA), createNode(repoB)])
     const [idA, idB] = await Promise.all([nodeA.id(), nodeB.id()])
 
@@ -285,4 +289,38 @@ describe('create node', function () {
 
     await expect(node.start()).to.eventually.be.rejected().with.property('code', 'ERR_WEBSOCKET_STAR_SWARM_ADDR_NOT_SUPPORTED')
   })
+
+  it('should auto-migrate repos by default', async function () {
+    this.timeout(80 * 1000)
+
+    const deferred = defer()
+    const id = await PeerId.create({
+      bits: 512
+    })
+
+    // create an old-looking repo
+    const repo = await createTempRepo({
+      version: 1,
+      spec: 1,
+      config: {
+        Identity: {
+          PeerId: id.toString(),
+          PrivKey: uint8ArrayToString(id.marshalPrivKey(), 'base64pad')
+        }
+      }
+    })
+
+    const node = await IPFS.create({
+      repo: repo.path,
+      onMigrationProgress: () => {
+        // migrations are happening
+        deferred.resolve()
+      }
+    })
+
+    await deferred.promise
+
+    await node.stop()
+    await repo.teardown()
+  })
 })
diff --git a/packages/ipfs-core/test/utils/create-node.js b/packages/ipfs-core/test/utils/create-node.js
@@ -5,7 +5,7 @@ const IPFS = require('../../')
 const createTempRepo = require('./create-repo-nodejs')
 
 module.exports = async (config = {}) => {
-  const repo = createTempRepo()
+  const repo = await createTempRepo()
   const ipfs = await IPFS.create(mergeOptions({
     silent: true,
     repo,
diff --git a/packages/ipfs-core/test/utils/create-repo-browser.js b/packages/ipfs-core/test/utils/create-repo-browser.js
@@ -9,10 +9,33 @@ const idb = self.indexedDB ||
   self.webkitIndexedDB ||
   self.msIndexedDB
 
-module.exports = function createTempRepo (repoPath) {
-  repoPath = repoPath || '/ipfs-' + nanoid()
+/**
+ * @param {object} options
+ * @param {string} [options.path]
+ * @param {number} [options.version]
+ * @param {number} [options.spec]
+ * @param {import('ipfs-core-types/src/config').Config} [options.config]
+ */
+module.exports = async function createTempRepo (options = {}) {
+  options.path = options.path || `ipfs-${nanoid()}`
 
-  const repo = new IPFSRepo(repoPath)
+  await createDB(options.path, (objectStore) => {
+    const encoder = new TextEncoder()
+
+    if (options.version) {
+      objectStore.put(encoder.encode(`${options.version}`), '/version')
+    }
+
+    if (options.spec) {
+      objectStore.put(encoder.encode(`${options.spec}`), '/datastore_spec')
+    }
+
+    if (options.config) {
+      objectStore.put(encoder.encode(JSON.stringify(options.config)), '/config')
+    }
+  })
+
+  const repo = new IPFSRepo(options.path)
 
   repo.teardown = async () => {
     try {
@@ -23,9 +46,50 @@ module.exports = function createTempRepo (repoPath) {
       }
     }
 
-    idb.deleteDatabase(repoPath)
-    idb.deleteDatabase(repoPath + '/blocks')
+    idb.deleteDatabase(options.path)
+    idb.deleteDatabase(options.path + '/blocks')
   }
 
   return repo
 }
+
+/**
+ * Allows pre-filling the root IndexedDB object store with data
+ *
+ * @param {string} path
+ * @param {(objectStore: IDBObjectStore) => void} fn
+ */
+function createDB (path, fn) {
+  return new Promise((resolve, reject) => {
+    const request = idb.open(path, 1)
+
+    request.onupgradeneeded = () => {
+      const db = request.result
+
+      db.onerror = () => {
+        reject(new Error('Could not create database'))
+      }
+
+      db.createObjectStore(path)
+    }
+
+    request.onsuccess = () => {
+      const db = request.result
+
+      const transaction = db.transaction(path, 'readwrite')
+      transaction.onerror = () => {
+        reject(new Error('Could not add data to database'))
+      }
+      transaction.oncomplete = () => {
+        db.close()
+        resolve()
+      }
+
+      const objectStore = transaction.objectStore(path)
+
+      fn(objectStore)
+
+      transaction.commit()
+    }
+  })
+}
diff --git a/packages/ipfs-core/test/utils/create-repo-nodejs.js b/packages/ipfs-core/test/utils/create-repo-nodejs.js
@@ -5,11 +5,33 @@ const clean = require('./clean')
 const os = require('os')
 const path = require('path')
 const { nanoid } = require('nanoid')
+const fs = require('fs').promises
 
-module.exports = function createTempRepo (repoPath) {
-  repoPath = repoPath || path.join(os.tmpdir(), '/ipfs-test-' + nanoid())
+/**
+ * @param {object} options
+ * @param {string} [options.path]
+ * @param {number} [options.version]
+ * @param {number} [options.spec]
+ * @param {import('ipfs-core-types/src/config').Config} [options.config]
+ */
+module.exports = async function createTempRepo (options = {}) {
+  options.path = options.path || path.join(os.tmpdir(), '/ipfs-test-' + nanoid())
 
-  const repo = new IPFSRepo(repoPath)
+  await fs.mkdir(options.path)
+
+  if (options.version) {
+    await fs.writeFile(path.join(options.path, 'version'), `${options.version}`)
+  }
+
+  if (options.spec) {
+    await fs.writeFile(path.join(options.path, 'spec'), `${options.spec}`)
+  }
+
+  if (options.config) {
+    await fs.writeFile(path.join(options.path, 'config'), JSON.stringify(options.config))
+  }
+
+  const repo = new IPFSRepo(options.path)
 
   repo.teardown = async () => {
     try {
@@ -20,7 +42,7 @@ module.exports = function createTempRepo (repoPath) {
       }
     }
 
-    await clean(repoPath)
+    await clean(options.path)
   }
 
   return repo

Original file line number	Diff line number	Diff line change
`@@ -132,6 +132,7 @@`
`132`	`132`	`"ipld-git": "^0.6.1",`
`133`	`133`	`"iso-url": "^1.0.0",`
`134`	`134`	`"nanoid": "^3.1.12",`
	`135`	`+ "p-defer": "^3.0.0",`
`135`	`136`	`"rimraf": "^3.0.2",`
`136`	`137`	`"sinon": "^10.0.1"`
`137`	`138`	`}`