Make migration management more robust

server/Database.js

@@ -170,14 +170,13 @@ class Database {
       throw new Error('Database connection failed')
     }
 
-    if (!this.isNew) {
-      try {
-        const migrationManager = new MigrationManager(this.sequelize, global.ConfigPath)
-        await migrationManager.runMigrations(packageJson.version)
-      } catch (error) {
-        Logger.error(`[Database] Failed to run migrations`, error)
-        throw new Error('Database migration failed')
-      }
+    try {
+      const migrationManager = new MigrationManager(this.sequelize, global.ConfigPath)
+      await migrationManager.init(packageJson.version)
+      if (!this.isNew) await migrationManager.runMigrations()
+    } catch (error) {
+      Logger.error(`[Database] Failed to run migrations`, error)
+      throw new Error('Database migration failed')
     }
 
     await this.buildModels(force)

server/managers/MigrationManager.js

@@ -1,36 +1,66 @@
 const { Umzug, SequelizeStorage } = require('umzug')
-const { Sequelize } = require('sequelize')
+const { Sequelize, DataTypes } = require('sequelize')
 const semver = require('semver')
 const path = require('path')
+const Module = require('module')
 const fs = require('../libs/fsExtra')
 const Logger = require('../Logger')
 
 class MigrationManager {
+  static MIGRATIONS_META_TABLE = 'migrationsMeta'
+
   constructor(sequelize, configPath = global.configPath) {
-    if (!sequelize || !(sequelize instanceof Sequelize)) {
-      throw new Error('Sequelize instance is required for MigrationManager.')
-    }
+    if (!sequelize || !(sequelize instanceof Sequelize)) throw new Error('Sequelize instance is required for MigrationManager.')
     this.sequelize = sequelize
-    if (!configPath) {
-      throw new Error('Config path is required for MigrationManager.')
-    }
+    if (!configPath) throw new Error('Config path is required for MigrationManager.')
     this.configPath = configPath
+    this.migrationsSourceDir = path.join(__dirname, '..', 'migrations')
+    this.initialized = false
     this.migrationsDir = null
     this.maxVersion = null
     this.databaseVersion = null
     this.serverVersion = null
     this.umzug = null
   }
 
-  async runMigrations(serverVersion) {
-    await this.init(serverVersion)
+  async init(serverVersion) {
+    if (!(await fs.pathExists(this.configPath))) throw new Error(`Config path does not exist: ${this.configPath}`)
+
+    this.migrationsDir = path.join(this.configPath, 'migrations')
+
+    this.serverVersion = this.extractVersionFromTag(serverVersion)
+    if (!this.serverVersion) throw new Error(`Invalid server version: ${serverVersion}. Expected a version tag like v1.2.3.`)
+
+    await this.fetchVersionsFromDatabase()
+    if (!this.maxVersion || !this.databaseVersion) throw new Error('Failed to fetch versions from the database.')
+
+    if (semver.gt(this.serverVersion, this.maxVersion)) {
+      try {
+        await this.copyMigrationsToConfigDir()
+      } catch (error) {
+        throw new Error('Failed to copy migrations to the config directory.', { cause: error })
+      }
+
+      try {
+        await this.updateMaxVersion()
+      } catch (error) {
+        throw new Error('Failed to update max version in the database.', { cause: error })
+      }
+    }
+
+    this.initialized = true
+  }
+
+  async runMigrations() {
+    if (!this.initialized) throw new Error('MigrationManager is not initialized. Call init() first.')
 
     const versionCompare = semver.compare(this.serverVersion, this.databaseVersion)
     if (versionCompare == 0) {
       Logger.info('[MigrationManager] Database is already up to date.')
       return
     }
 
+    this.initUmzug()
     const migrations = await this.umzug.migrations()
     const executedMigrations = (await this.umzug.executed()).map((m) => m.name)
 
@@ -51,7 +81,7 @@ class MigrationManager {
         Logger.info('Created a backup of the original database.')
 
         // Run migrations
-        await this.umzug[migrationDirection]({ migrations: migrationsToRun })
+        await this.umzug[migrationDirection]({ migrations: migrationsToRun, rerun: 'ALLOW' })
 
         // Clean up the backup
         await fs.remove(backupDbPath)
@@ -60,7 +90,7 @@ class MigrationManager {
       } catch (error) {
         Logger.error('[MigrationManager] Migration failed:', error)
 
-        this.sequelize.close()
+        await this.sequelize.close()
 
         // Step 3: If migration fails, save the failed original and restore the backup
         const failedDbPath = path.join(this.configPath, 'absdatabase.failed.sqlite')
@@ -75,45 +105,40 @@ class MigrationManager {
     } else {
       Logger.info('[MigrationManager] No migrations to run.')
     }
-  }
-
-  async init(serverVersion, umzugStorage = new SequelizeStorage({ sequelize: this.sequelize })) {
-    if (!(await fs.pathExists(this.configPath))) throw new Error(`Config path does not exist: ${this.configPath}`)
-
-    this.migrationsDir = path.join(this.configPath, 'migrations')
-
-    this.serverVersion = this.extractVersionFromTag(serverVersion)
-    if (!this.serverVersion) throw new Error(`Invalid server version: ${serverVersion}. Expected a version tag like v1.2.3.`)
 
-    await this.fetchVersionsFromDatabase()
-    if (!this.maxVersion || !this.databaseVersion) throw new Error('Failed to fetch versions from the database.')
-
-    if (semver.gt(this.serverVersion, this.maxVersion)) {
-      try {
-        await this.copyMigrationsToConfigDir()
-      } catch (error) {
-        throw new Error('Failed to copy migrations to the config directory.', { cause: error })
-      }
-
-      try {
-        await this.updateMaxVersion(serverVersion)
-      } catch (error) {
-        throw new Error('Failed to update max version in the database.', { cause: error })
-      }
-    }
+    await this.updateDatabaseVersion()
+  }
 
-    // Step 4: Initialize the Umzug instance
+  initUmzug(umzugStorage = new SequelizeStorage({ sequelize: this.sequelize })) {
     if (!this.umzug) {
       // This check is for dependency injection in tests
       const cwd = this.migrationsDir
 
       const parent = new Umzug({
         migrations: {
-          glob: ['*.js', { cwd }]
+          glob: ['*.js', { cwd }],
+          resolve: (params) => {
+            // make script think it's in migrationsSourceDir
+            const migrationPath = params.path
+            const migrationName = params.name
+            const contents = fs.readFileSync(migrationPath, 'utf8')
+            const fakePath = path.join(this.migrationsSourceDir, path.basename(migrationPath))
+            const module = new Module(fakePath)
+            module.filename = fakePath
+            module.paths = Module._nodeModulePaths(this.migrationsSourceDir)
+            module._compile(contents, fakePath)
+            const script = module.exports
+            return {
+              name: migrationName,
+              path: migrationPath,
+              up: script.up,
+              down: script.down
+            }
+          }
         },
-        context: this.sequelize.getQueryInterface(),
+        context: { queryInterface: this.sequelize.getQueryInterface(), logger: Logger },
         storage: umzugStorage,
-        logger: Logger.info
+        logger: Logger
       })
 
       // Sort migrations by version
@@ -130,18 +155,38 @@ class MigrationManager {
   }
 
   async fetchVersionsFromDatabase() {
-    const [result] = await this.sequelize.query("SELECT json_extract(value, '$.version') AS version, json_extract(value, '$.maxVersion') AS maxVersion FROM settings WHERE key = :key", {
-      replacements: { key: 'server-settings' },
+    await this.checkOrCreateMigrationsMetaTable()
+
+    const [{ version }] = await this.sequelize.query("SELECT value as version FROM :migrationsMeta WHERE key = 'version'", {
+      replacements: { migrationsMeta: MigrationManager.MIGRATIONS_META_TABLE },
       type: Sequelize.QueryTypes.SELECT
     })
+    this.databaseVersion = version
 
-    if (result) {
-      try {
-        this.maxVersion = this.extractVersionFromTag(result.maxVersion) || '0.0.0'
-        this.databaseVersion = this.extractVersionFromTag(result.version)
-      } catch (error) {
-        Logger.error('[MigrationManager] Failed to parse server settings from the database.', error)
-      }
+    const [{ maxVersion }] = await this.sequelize.query("SELECT value as maxVersion FROM :migrationsMeta WHERE key = 'maxVersion'", {
+      replacements: { migrationsMeta: MigrationManager.MIGRATIONS_META_TABLE },
+      type: Sequelize.QueryTypes.SELECT
+    })
+    this.maxVersion = maxVersion
+  }
+
+  async checkOrCreateMigrationsMetaTable() {
+    const queryInterface = this.sequelize.getQueryInterface()
+    if (!(await queryInterface.tableExists(MigrationManager.MIGRATIONS_META_TABLE))) {
+      await queryInterface.createTable(MigrationManager.MIGRATIONS_META_TABLE, {
+        key: {
+          type: DataTypes.STRING,
+          allowNull: false
+        },
+        value: {
+          type: DataTypes.STRING,
+          allowNull: false
+        }
+      })
+      await this.sequelize.query("INSERT INTO :migrationsMeta (key, value) VALUES ('version', :version), ('maxVersion', '0.0.0')", {
+        replacements: { version: this.serverVersion, migrationsMeta: MigrationManager.MIGRATIONS_META_TABLE },
+        type: Sequelize.QueryTypes.INSERT
+      })
     }
   }
 
@@ -152,18 +197,16 @@ class MigrationManager {
   }
 
   async copyMigrationsToConfigDir() {
-    const migrationsSourceDir = path.join(__dirname, '..', 'migrations')
-
     await fs.ensureDir(this.migrationsDir) // Ensure the target directory exists
 
-    if (!(await fs.pathExists(migrationsSourceDir))) return
+    if (!(await fs.pathExists(this.migrationsSourceDir))) return
 
-    const files = await fs.readdir(migrationsSourceDir)
+    const files = await fs.readdir(this.migrationsSourceDir)
     await Promise.all(
       files
         .filter((file) => path.extname(file) === '.js')
         .map(async (file) => {
-          const sourceFile = path.join(migrationsSourceDir, file)
+          const sourceFile = path.join(this.migrationsSourceDir, file)
           const targetFile = path.join(this.migrationsDir, file)
           await fs.copy(sourceFile, targetFile) // Asynchronously copy the files
         })
@@ -189,13 +232,29 @@ class MigrationManager {
     }
   }
 
-  async updateMaxVersion(serverVersion) {
-    await this.sequelize.query("UPDATE settings SET value = JSON_SET(value, '$.maxVersion', ?) WHERE key = 'server-settings'", {
-      replacements: [serverVersion],
-      type: Sequelize.QueryTypes.UPDATE
-    })
+  async updateMaxVersion() {
+    try {
+      await this.sequelize.query("UPDATE :migrationsMeta SET value = :maxVersion WHERE key = 'maxVersion'", {
+        replacements: { maxVersion: this.serverVersion, migrationsMeta: MigrationManager.MIGRATIONS_META_TABLE },
+        type: Sequelize.QueryTypes.UPDATE
+      })
+    } catch (error) {
+      throw new Error('Failed to update maxVersion in the migrationsMeta table.', { cause: error })
+    }
     this.maxVersion = this.serverVersion
   }
+
+  async updateDatabaseVersion() {
+    try {
+      await this.sequelize.query("UPDATE :migrationsMeta SET value = :version WHERE key = 'version'", {
+        replacements: { version: this.serverVersion, migrationsMeta: MigrationManager.MIGRATIONS_META_TABLE },
+        type: Sequelize.QueryTypes.UPDATE
+      })
+    } catch (error) {
+      throw new Error('Failed to update version in the migrationsMeta table.', { cause: error })
+    }
+    this.databaseVersion = this.serverVersion
+  }
 }
 
 module.exports = MigrationManager

server/migrations/readme.md

@@ -15,16 +15,18 @@ When writing a migration, keep the following guidelines in mind:
   - `server_version` should be the version of the server that the migration was created for (this should usually be the next server release).
   - `migration_name` should be a short description of the changes that the migration makes.
 
-- The script should export two async functions: `up` and `down`. The `up` function should contain the script that applies the changes to the database, and the `down` function should contain the script that undoes the changes. The `up` and `down` functions should accept a single object parameter with a `context` property that contains a reference to a Sequelize [`QueryInterface`](https://sequelize.org/docs/v6/other-topics/query-interface/) object. A typical migration script might look like this:
+- The script should export two async functions: `up` and `down`. The `up` function should contain the script that applies the changes to the database, and the `down` function should contain the script that undoes the changes. The `up` and `down` functions should accept a single object parameter with a `context` property that contains a reference to a Sequelize [`QueryInterface`](https://sequelize.org/docs/v6/other-topics/query-interface/) object, and a [Logger](https://github.com/advplyr/audiobookshelf/blob/423a2129d10c6d8aaac9e8c75941fa6283889602/server/Logger.js#L4) object for logging. A typical migration script might look like this:
 
   ```javascript
-  async function up({context: queryInterface}) {
+  async function up({ context: { queryInterface, logger } }) {
     // Upwards migration script
+    logger.info('migrating ...');
     ...
   }
 
-  async function down({context: queryInterface}) {
+  async function down({ context: { queryInterface, logger } }) {
     // Downward migration script
+    logger.info('reverting ...');
     ...
   }
 
@@ -33,14 +35,15 @@ When writing a migration, keep the following guidelines in mind:
 
 - Always implement both the `up` and `down` functions.
 - The `up` and `down` functions should be idempotent (i.e., they should be safe to run multiple times).
-- It's your responsibility to make sure that the down migration undoes the changes made by the up migration.
+- Prefer using only `queryInterface` and `logger` parameters, the `sequelize` module, and node.js built-in modules in your migration scripts. You can require other modules, but be aware that they might not be available or change from they ones you tested with.
+- It's your responsibility to make sure that the down migration reverts the changes made by the up migration.
 - Log detailed information on every step of the migration. Use `Logger.info()` and `Logger.error()`.
 - Test tour migrations thoroughly before committing them.
   - write unit tests for your migrations (see `test/server/migrations` for an example)
   - you can force a server version change by modifying the `version` field in `package.json` on your dev environment (but don't forget to revert it back before committing)
 
 ## How migrations are run
 
-Migrations are run automatically when the server starts, when the server detects that the server version has changed. Migrations are always run server version order (from oldest to newest up migrations if the server version increased, and from newest to oldest down migrations if the server version decreased). Only the relevant migrations are run, based on the new and old server versions.
+Migrations are run automatically when the server starts, when the server detects that the server version has changed. Migrations are always run in server version order (from oldest to newest up migrations if the server version increased, and from newest to oldest down migrations if the server version decreased). Only the relevant migrations are run, based on the new and old server versions.
 
 This means that you can switch between server releases without having to worry about running migrations manually. The server will automatically apply the necessary migrations when it starts.