From 83a4344a01de4f65c5024fdf2d177a04457ccd2f Mon Sep 17 00:00:00 2001 From: Paul Nothaft Date: Thu, 15 Jan 2026 15:43:13 +0100 Subject: [PATCH] fix: prevent database migration restart failures - Move migrations table insert inside PostgreSQL transaction for atomicity - Add PostgreSQL error codes 42701 (duplicate column), 42710 (duplicate object), and 23505 (unique violation) to error handling - Make migrations 006 and 008 idempotent with column existence checks Fixes #107 --- .../006_add_photo_counter_to_categories.js | 39 +++++++++------- ...add_language_support_to_email_templates.js | 46 +++++++++++-------- backend/migrations/run-migrations-safe.js | 23 +++++++--- backend/migrations/run-migrations.js | 17 +++++-- 4 files changed, 81 insertions(+), 44 deletions(-) diff --git a/backend/migrations/legacy/006_add_photo_counter_to_categories.js b/backend/migrations/legacy/006_add_photo_counter_to_categories.js index 6f378aad..4f4a46a6 100644 --- a/backend/migrations/legacy/006_add_photo_counter_to_categories.js +++ b/backend/migrations/legacy/006_add_photo_counter_to_categories.js @@ -1,22 +1,27 @@ exports.up = async function(knex) { - // Add photo_counter column to photo_categories table - await knex.schema.alterTable('photo_categories', function(table) { - table.integer('photo_counter').defaultTo(0).notNullable(); - }); + // Check if photo_counter column already exists to make migration idempotent + const hasPhotoCounter = await knex.schema.hasColumn('photo_categories', 'photo_counter'); - // Initialize counters based on existing photos - const categories = await knex('photo_categories').select('id'); - - for (const category of categories) { - const photoCount = await knex('photos') - .where('category_id', category.id) - .count('id as count') - .first(); - - if (photoCount && photoCount.count > 0) { - await knex('photo_categories') - .where('id', category.id) - .update({ photo_counter: photoCount.count }); + if (!hasPhotoCounter) { + // Add photo_counter column to photo_categories table + await knex.schema.alterTable('photo_categories', function(table) { + table.integer('photo_counter').defaultTo(0).notNullable(); + }); + + // Initialize counters based on existing photos + const categories = await knex('photo_categories').select('id'); + + for (const category of categories) { + const photoCount = await knex('photos') + .where('category_id', category.id) + .count('id as count') + .first(); + + if (photoCount && photoCount.count > 0) { + await knex('photo_categories') + .where('id', category.id) + .update({ photo_counter: photoCount.count }); + } } } }; diff --git a/backend/migrations/legacy/008_add_language_support_to_email_templates.js b/backend/migrations/legacy/008_add_language_support_to_email_templates.js index 40718509..d2604504 100644 --- a/backend/migrations/legacy/008_add_language_support_to_email_templates.js +++ b/backend/migrations/legacy/008_add_language_support_to_email_templates.js @@ -1,23 +1,33 @@ exports.up = async function(knex) { - // Add language-specific columns to email_templates - await knex.schema.alterTable('email_templates', function(table) { - // Add English versions (rename existing columns for consistency) - table.renameColumn('subject', 'subject_en'); - table.renameColumn('body_html', 'body_html_en'); - table.renameColumn('body_text', 'body_text_en'); - - // Add German versions - table.string('subject_de'); - table.text('body_html_de'); - table.text('body_text_de'); - }); + // Check which columns already exist to make migration idempotent + const hasSubjectEn = await knex.schema.hasColumn('email_templates', 'subject_en'); + const hasSubjectDe = await knex.schema.hasColumn('email_templates', 'subject_de'); + const hasSubjectOriginal = await knex.schema.hasColumn('email_templates', 'subject'); - // Copy existing values to German columns as defaults - await knex('email_templates').update({ - subject_de: knex.raw('subject_en'), - body_html_de: knex.raw('body_html_en'), - body_text_de: knex.raw('body_text_en') - }); + // Only rename columns if they haven't been renamed yet + if (hasSubjectOriginal && !hasSubjectEn) { + await knex.schema.alterTable('email_templates', function(table) { + table.renameColumn('subject', 'subject_en'); + table.renameColumn('body_html', 'body_html_en'); + table.renameColumn('body_text', 'body_text_en'); + }); + } + + // Only add German columns if they don't exist + if (!hasSubjectDe) { + await knex.schema.alterTable('email_templates', function(table) { + table.string('subject_de'); + table.text('body_html_de'); + table.text('body_text_de'); + }); + + // Copy existing values to German columns as defaults + await knex('email_templates').update({ + subject_de: knex.raw('subject_en'), + body_html_de: knex.raw('body_html_en'), + body_text_de: knex.raw('body_text_en') + }); + } }; exports.down = async function(knex) { diff --git a/backend/migrations/run-migrations-safe.js b/backend/migrations/run-migrations-safe.js index 99c5abee..f66271d0 100644 --- a/backend/migrations/run-migrations-safe.js +++ b/backend/migrations/run-migrations-safe.js @@ -67,26 +67,37 @@ async function runMigrationSafely(filepath) { const migrationPath = path.join(__dirname, filepath); const migration = require(migrationPath); const filename = path.basename(filepath); - + if (migration.up) { console.log(`Running migration: ${filepath}`); - + // Run migration in a transaction if possible + // IMPORTANT: Include the migrations table insert INSIDE the transaction + // to ensure atomicity between schema changes and tracking if (db.client.config.client === 'pg') { await db.transaction(async (trx) => { await migration.up(trx); + // Insert migration record inside transaction for atomicity + await trx('migrations').insert({ filename }); }); } else { await migration.up(db); + await db('migrations').insert({ filename }); } - - await db('migrations').insert({ filename }); + console.log(`Migration ${filepath} completed successfully`); } } catch (error) { // Check if error is because schema already exists - if (error.code === '42P07' || // PostgreSQL: relation already exists - error.code === 'SQLITE_ERROR' && error.message.includes('already exists')) { + // PostgreSQL error codes: + // - 42P07: duplicate_table (relation already exists) + // - 42701: duplicate_column (column already exists) + // - 42710: duplicate_object (constraint, index, etc. already exists) + // - 23505: unique_violation (migration record already exists) + const schemaExistsErrors = ['42P07', '42701', '42710', '23505']; + const isSQLiteAlreadyExists = error.code === 'SQLITE_ERROR' && error.message.includes('already exists'); + + if (schemaExistsErrors.includes(error.code) || isSQLiteAlreadyExists) { console.log(`Migration ${filepath} - schema already exists, marking as applied`); await markMigrationAsApplied(path.basename(filepath)); } else { diff --git a/backend/migrations/run-migrations.js b/backend/migrations/run-migrations.js index 3ab7f7b1..86c9114f 100644 --- a/backend/migrations/run-migrations.js +++ b/backend/migrations/run-migrations.js @@ -26,11 +26,22 @@ async function runMigration(filepath) { const migrationPath = path.join(__dirname, filepath); const migration = require(migrationPath); const filename = path.basename(filepath); - + if (migration.up) { console.log(`Running migration: ${filepath}`); - await migration.up(db); - await db('migrations').insert({ filename }); + + // Run migration in a transaction if PostgreSQL to ensure atomicity + // between schema changes and migration tracking + if (db.client.config.client === 'pg') { + await db.transaction(async (trx) => { + await migration.up(trx); + await trx('migrations').insert({ filename }); + }); + } else { + await migration.up(db); + await db('migrations').insert({ filename }); + } + console.log(`Migration ${filepath} completed`); } }