diff --git a/lib/services/migrations.js b/lib/services/migrations.js index 6d70748fe..6c2d59005 100644 --- a/lib/services/migrations.js +++ b/lib/services/migrations.js @@ -2,10 +2,10 @@ * Module dependencies. */ import chalk from 'chalk'; -import mongoose from 'mongoose'; import path from 'path'; import { glob } from 'glob'; import logger from './logger.js'; +import migrationRepository from '../../modules/core/repositories/migration.repository.js'; /** * Scan all modules for migration files matching `modules/*/migrations/*.js`. @@ -28,8 +28,7 @@ const discoverMigrationFiles = async () => { * @returns {Promise>} set of executed migration names */ const getExecutedMigrations = async () => { - const Migration = mongoose.model('Migration'); - const records = await Migration.find({}, { name: 1, _id: 0 }).lean(); + const records = await migrationRepository.listExecuted(); return new Set(records.map((r) => r.name)); }; @@ -38,10 +37,7 @@ const getExecutedMigrations = async () => { * @param {string} name - the migration filename used as unique key * @returns {Promise} the created Migration document */ -const recordMigration = async (name) => { - const Migration = mongoose.model('Migration'); - return Migration.create({ name, executedAt: new Date() }); -}; +const recordMigration = (name) => migrationRepository.create(name); /** * Atomically claim a migration by inserting a record before execution. @@ -50,9 +46,8 @@ const recordMigration = async (name) => { * @returns {Promise} true if claimed successfully, false if already claimed by another runner */ const claimMigration = async (name) => { - const Migration = mongoose.model('Migration'); try { - await Migration.create({ name, executedAt: new Date() }); + await migrationRepository.create(name); return true; } catch (err) { // Duplicate key error means another runner already claimed it @@ -66,10 +61,7 @@ const claimMigration = async (name) => { * @param {string} name - the migration filename * @returns {Promise} the deletion result */ -const unclaimMigration = async (name) => { - const Migration = mongoose.model('Migration'); - return Migration.deleteOne({ name }); -}; +const unclaimMigration = (name) => migrationRepository.deleteByName(name); /** * Run a single migration file's `up()` export. @@ -120,6 +112,16 @@ const runMigration = async (filePath, executed) => { return true; }; +/** + * Ensure the Migration collection's indexes are in sync with the schema. + * The `name` field has a unique index that backs the atomic claim logic in + * {@link claimMigration}; without it, concurrent runners can execute the same + * migration twice. Delegates to the repository so the service layer never + * touches Mongoose directly. + * @returns {Promise} Resolves once indexes have been synchronised. + */ +const ensureMigrationIndexes = () => migrationRepository.syncIndexes(); + /** * Run all pending migrations in order. * Scans `modules/*/migrations/*.js`, compares with the `migrations` MongoDB @@ -131,6 +133,11 @@ const run = async () => { // Ensure the Migration model is registered await import(path.resolve('modules/core/models/migration.model.mongoose.js')); + // Ensure the unique index on `name` exists before any claim/record calls. + // Without this, older deployments whose `migrations` collection was created + // before the unique index was added would silently allow duplicate claims. + await ensureMigrationIndexes(); + const files = await discoverMigrationFiles(); if (files.length === 0) { @@ -162,4 +169,4 @@ const run = async () => { return { total: files.length, executed: executedCount }; }; -export default { run, discoverMigrationFiles, getExecutedMigrations, recordMigration, runMigration }; +export default { run, discoverMigrationFiles, getExecutedMigrations, recordMigration, runMigration, ensureMigrationIndexes }; diff --git a/modules/core/repositories/migration.repository.js b/modules/core/repositories/migration.repository.js new file mode 100644 index 000000000..1cc6fd079 --- /dev/null +++ b/modules/core/repositories/migration.repository.js @@ -0,0 +1,42 @@ +/** + * Module dependencies + */ +import mongoose from 'mongoose'; + +/** + * @function syncIndexes + * @description Synchronise the Migration collection's indexes with the schema. + * Creates any missing indexes (notably the unique index on `name` that backs + * the atomic claim logic) and drops any indexes no longer declared on the + * schema. Idempotent and safe to call on every boot. + * @returns {Promise>} Names of indexes dropped during sync. + */ +const syncIndexes = () => mongoose.model('Migration').syncIndexes(); + +/** + * @function listExecuted + * @description Fetch the name of every migration recorded as executed. + * @returns {Promise>} Lean records with only the `name` field. + */ +const listExecuted = () => mongoose.model('Migration').find({}, { name: 1, _id: 0 }).lean(); + +/** + * @function create + * @description Insert a new Migration record. Relies on the unique index on + * `name` to reject duplicates — used both by the public `recordMigration()` + * flow and the atomic claim logic in `claimMigration()`. + * @param {string} name - Migration filename, unique key for the collection. + * @returns {Promise} The created Migration document. + */ +const create = (name) => mongoose.model('Migration').create({ name, executedAt: new Date() }); + +/** + * @function deleteByName + * @description Remove a Migration record by name. Used to unclaim a migration + * when its execution fails so it can be retried on the next boot. + * @param {string} name - Migration filename to delete. + * @returns {Promise} Mongo deletion result. + */ +const deleteByName = (name) => mongoose.model('Migration').deleteOne({ name }); + +export default { syncIndexes, listExecuted, create, deleteByName }; diff --git a/modules/core/tests/migrations.unit.tests.js b/modules/core/tests/migrations.unit.tests.js index 30ffc7fff..2a5c07260 100644 --- a/modules/core/tests/migrations.unit.tests.js +++ b/modules/core/tests/migrations.unit.tests.js @@ -71,5 +71,13 @@ describe('Migrations unit tests:', () => { const json = doc.toJSON(); expect(json.id).toBeDefined(); }); + + it('should declare a unique index on the name field', () => { + const Migration = mongoose.model('Migration'); + // schema.indexes() returns [[fields, options], ...] for compound/explicit indexes. + // For `unique: true` declared on the path, the index is carried on the schema path itself. + const namePath = Migration.schema.path('name'); + expect(namePath.options.unique).toBe(true); + }); }); });