From f74b2f70fbfc622ed0877c2fedcdf0550f370a48 Mon Sep 17 00:00:00 2001 From: Kevin Gatera Date: Tue, 3 Mar 2026 17:42:49 -0500 Subject: [PATCH] make migration replays resilient on postgres --- server/migrations/v2.15.2-index-creation.js | 40 +++++++----- ....4-use-subfolder-for-oidc-redirect-uris.js | 17 +++-- .../migrations/v2.15.2-index-creation.test.js | 53 ++++++++++++++++ ...e-subfolder-for-oidc-redirect-uris.test.js | 63 ++++++++++++++++--- 4 files changed, 145 insertions(+), 28 deletions(-) diff --git a/server/migrations/v2.15.2-index-creation.js b/server/migrations/v2.15.2-index-creation.js index 5f976495c..03113e8df 100644 --- a/server/migrations/v2.15.2-index-creation.js +++ b/server/migrations/v2.15.2-index-creation.js @@ -20,10 +20,8 @@ async function up({ context: { queryInterface, logger } }) { // Create index for bookAuthors logger.info('[2.15.2 migration] Creating index for bookAuthors') const bookAuthorsIndexes = await queryInterface.showIndex('bookAuthors') - if (!bookAuthorsIndexes.some((index) => index.name === 'bookAuthor_authorId')) { - await queryInterface.addIndex('bookAuthors', ['authorId'], { - name: 'bookAuthor_authorId' - }) + if (!hasIndex(bookAuthorsIndexes, 'bookAuthor_authorId')) { + await addIndexIfMissing(queryInterface, 'bookAuthors', ['authorId'], 'bookAuthor_authorId', logger) } else { logger.info('[2.15.2 migration] Index bookAuthor_authorId already exists') } @@ -31,10 +29,8 @@ async function up({ context: { queryInterface, logger } }) { // Create index for bookSeries logger.info('[2.15.2 migration] Creating index for bookSeries') const bookSeriesIndexes = await queryInterface.showIndex('bookSeries') - if (!bookSeriesIndexes.some((index) => index.name === 'bookSeries_seriesId')) { - await queryInterface.addIndex('bookSeries', ['seriesId'], { - name: 'bookSeries_seriesId' - }) + if (!hasIndex(bookSeriesIndexes, 'bookSeries_seriesId')) { + await addIndexIfMissing(queryInterface, 'bookSeries', ['seriesId'], 'bookSeries_seriesId', logger) } else { logger.info('[2.15.2 migration] Index bookSeries_seriesId already exists') } @@ -46,10 +42,8 @@ async function up({ context: { queryInterface, logger } }) { // Create index for podcastEpisode and createdAt logger.info('[2.15.2 migration] Creating index for podcastEpisode and createdAt') const podcastEpisodesIndexes = await queryInterface.showIndex('podcastEpisodes') - if (!podcastEpisodesIndexes.some((index) => index.name === 'podcastEpisode_createdAt_podcastId')) { - await queryInterface.addIndex('podcastEpisodes', ['createdAt', 'podcastId'], { - name: 'podcastEpisode_createdAt_podcastId' - }) + if (!hasIndex(podcastEpisodesIndexes, 'podcastEpisode_createdAt_podcastId')) { + await addIndexIfMissing(queryInterface, 'podcastEpisodes', ['createdAt', 'podcastId'], 'podcastEpisode_createdAt_podcastId', logger) } else { logger.info('[2.15.2 migration] Index podcastEpisode_createdAt_podcastId already exists') } @@ -94,12 +88,30 @@ module.exports = { up, down } async function removeIndexIfExists(queryInterface, tableName, indexName, logger) { const indexes = await queryInterface.showIndex(tableName) - const hasIndex = indexes.some((index) => String(index?.name || index?.indexName || '').toLowerCase() === indexName.toLowerCase()) + const hasIndexWithName = hasIndex(indexes, indexName) - if (!hasIndex) { + if (!hasIndexWithName) { logger.info(`[2.15.2 migration] Index ${indexName} does not exist, skipping removeIndex`) return } await queryInterface.removeIndex(tableName, indexName) } + +function hasIndex(indexes, indexName) { + const expected = String(indexName || '').toLowerCase() + return indexes.some((index) => String(index?.name || index?.indexName || '').toLowerCase() === expected) +} + +async function addIndexIfMissing(queryInterface, tableName, fields, indexName, logger) { + try { + await queryInterface.addIndex(tableName, fields, { name: indexName }) + } catch (error) { + const alreadyExists = + (error?.name === 'SequelizeDatabaseError' && /already exists/i.test(error?.message || '')) || + error?.original?.code === '42P07' + if (!alreadyExists) throw error + + logger.info(`[2.15.2 migration] Index ${indexName} already exists`) + } +} diff --git a/server/migrations/v2.17.4-use-subfolder-for-oidc-redirect-uris.js b/server/migrations/v2.17.4-use-subfolder-for-oidc-redirect-uris.js index 03797e35e..08ab3b983 100644 --- a/server/migrations/v2.17.4-use-subfolder-for-oidc-redirect-uris.js +++ b/server/migrations/v2.17.4-use-subfolder-for-oidc-redirect-uris.js @@ -56,15 +56,23 @@ async function down({ context: { queryInterface, logger } }) { } async function getServerSettings(queryInterface, logger) { - const result = await queryInterface.sequelize.query('SELECT value FROM settings WHERE key = "server-settings";') + const result = await queryInterface.sequelize.query('SELECT value FROM settings WHERE key = :settingsKey;', { + replacements: { settingsKey: 'server-settings' } + }) if (!result[0].length) { logger.error('[2.17.4 migration] Server settings not found') throw new Error('Server settings not found') } + const settingsValue = result[0][0].value + + if (settingsValue && typeof settingsValue === 'object') { + return settingsValue + } + let serverSettings = null try { - serverSettings = JSON.parse(result[0][0].value) + serverSettings = JSON.parse(settingsValue) } catch (error) { logger.error('[2.17.4 migration] Error parsing server settings:', error) throw error @@ -74,9 +82,10 @@ async function getServerSettings(queryInterface, logger) { } async function updateServerSettings(queryInterface, logger, serverSettings) { - await queryInterface.sequelize.query('UPDATE settings SET value = :value WHERE key = "server-settings";', { + await queryInterface.sequelize.query('UPDATE settings SET value = :value WHERE key = :settingsKey;', { replacements: { - value: JSON.stringify(serverSettings) + value: JSON.stringify(serverSettings), + settingsKey: 'server-settings' } }) } diff --git a/test/server/migrations/v2.15.2-index-creation.test.js b/test/server/migrations/v2.15.2-index-creation.test.js index c69142fcd..d9257a483 100644 --- a/test/server/migrations/v2.15.2-index-creation.test.js +++ b/test/server/migrations/v2.15.2-index-creation.test.js @@ -1,5 +1,6 @@ const { expect } = require('chai') const { Sequelize, DataTypes } = require('sequelize') +const sinon = require('sinon') const Logger = require('../../../server/Logger') const { up, down } = require('../../../server/migrations/v2.15.2-index-creation') @@ -29,6 +30,10 @@ describe('migration-v2.15.2-index-creation', () => { }) }) + afterEach(async () => { + if (sequelize) await sequelize.close() + }) + it('up should succeed when legacy podcast index is missing', async () => { await up({ context: { queryInterface, logger: Logger } }) @@ -42,4 +47,52 @@ describe('migration-v2.15.2-index-creation', () => { const indexes = await queryInterface.showIndex('podcastEpisodes') expect(indexes.some((index) => index.name === 'podcast_episodes_created_at')).to.equal(true) }) + + it('up should treat index names case-insensitively', async () => { + const qi = { + showIndex: sinon.stub(), + addIndex: sinon.stub().resolves(), + removeIndex: sinon.stub().resolves() + } + + qi.showIndex.onCall(0).resolves([{ name: 'bookauthor_authorid' }]) + qi.showIndex.onCall(1).resolves([{ name: 'bookseries_seriesid' }]) + qi.showIndex.onCall(2).resolves([{ name: 'podcast_episodes_created_at' }]) + qi.showIndex.onCall(3).resolves([{ name: 'podcastepisode_createdat_podcastid' }]) + + await up({ context: { queryInterface: qi, logger: Logger } }) + + expect(qi.addIndex.called).to.equal(false) + expect(qi.removeIndex.calledOnceWithExactly('podcastEpisodes', 'podcast_episodes_created_at')).to.equal(true) + }) + + it('up should continue when addIndex reports already exists', async () => { + const makePgExistsError = (sql) => { + const err = new Error('relation already exists') + err.name = 'SequelizeDatabaseError' + err.original = { code: '42P07' } + err.sql = sql + return err + } + + const qi = { + showIndex: sinon.stub(), + addIndex: sinon.stub(), + removeIndex: sinon.stub().resolves() + } + + qi.showIndex.onCall(0).resolves([]) + qi.showIndex.onCall(1).resolves([]) + qi.showIndex.onCall(2).resolves([{ name: 'podcast_episodes_created_at' }]) + qi.showIndex.onCall(3).resolves([]) + + qi.addIndex.onCall(0).rejects(makePgExistsError('CREATE INDEX bookAuthor_authorId ON bookAuthors (authorId)')) + qi.addIndex.onCall(1).rejects(makePgExistsError('CREATE INDEX bookSeries_seriesId ON bookSeries (seriesId)')) + qi.addIndex.onCall(2).rejects(makePgExistsError('CREATE INDEX podcastEpisode_createdAt_podcastId ON podcastEpisodes (createdAt, podcastId)')) + + await up({ context: { queryInterface: qi, logger: Logger } }) + + expect(qi.addIndex.callCount).to.equal(3) + expect(qi.removeIndex.calledOnceWithExactly('podcastEpisodes', 'podcast_episodes_created_at')).to.equal(true) + }) }) diff --git a/test/server/migrations/v2.17.4-use-subfolder-for-oidc-redirect-uris.test.js b/test/server/migrations/v2.17.4-use-subfolder-for-oidc-redirect-uris.test.js index 1662d5f98..6794a5696 100644 --- a/test/server/migrations/v2.17.4-use-subfolder-for-oidc-redirect-uris.test.js +++ b/test/server/migrations/v2.17.4-use-subfolder-for-oidc-redirect-uris.test.js @@ -30,11 +30,16 @@ describe('Migration v2.17.4-use-subfolder-for-oidc-redirect-uris', () => { expect(logger.info.calledWith('[2.17.4 migration] UPGRADE BEGIN: 2.17.4-use-subfolder-for-oidc-redirect-uris')).to.be.true expect(logger.info.calledWith('[2.17.4 migration] OIDC is enabled, adding authOpenIDSubfolderForRedirectURLs to server settings')).to.be.true expect(queryInterface.sequelize.query.calledTwice).to.be.true - expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true expect( - queryInterface.sequelize.query.calledWith('UPDATE settings SET value = :value WHERE key = "server-settings";', { + queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = :settingsKey;', { + replacements: { settingsKey: 'server-settings' } + }) + ).to.be.true + expect( + queryInterface.sequelize.query.calledWith('UPDATE settings SET value = :value WHERE key = :settingsKey;', { replacements: { - value: JSON.stringify({ authActiveAuthMethods: ['openid'], authOpenIDSubfolderForRedirectURLs: '' }) + value: JSON.stringify({ authActiveAuthMethods: ['openid'], authOpenIDSubfolderForRedirectURLs: '' }), + settingsKey: 'server-settings' } }) ).to.be.true @@ -49,10 +54,31 @@ describe('Migration v2.17.4-use-subfolder-for-oidc-redirect-uris', () => { expect(logger.info.calledWith('[2.17.4 migration] UPGRADE BEGIN: 2.17.4-use-subfolder-for-oidc-redirect-uris')).to.be.true expect(logger.info.calledWith('[2.17.4 migration] OIDC is not enabled, no action required')).to.be.true expect(queryInterface.sequelize.query.calledOnce).to.be.true - expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect( + queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = :settingsKey;', { + replacements: { settingsKey: 'server-settings' } + }) + ).to.be.true expect(logger.info.calledWith('[2.17.4 migration] UPGRADE END: 2.17.4-use-subfolder-for-oidc-redirect-uris')).to.be.true }) + it('should handle already-parsed object server settings', async () => { + queryInterface.sequelize.query.onFirstCall().resolves([[{ value: { authActiveAuthMethods: ['openid'] } }]]) + queryInterface.sequelize.query.onSecondCall().resolves() + + await up({ context }) + + expect(queryInterface.sequelize.query.calledTwice).to.be.true + expect( + queryInterface.sequelize.query.calledWith('UPDATE settings SET value = :value WHERE key = :settingsKey;', { + replacements: { + value: JSON.stringify({ authActiveAuthMethods: ['openid'], authOpenIDSubfolderForRedirectURLs: '' }), + settingsKey: 'server-settings' + } + }) + ).to.be.true + }) + it('should throw an error if server settings cannot be parsed', async () => { queryInterface.sequelize.query.onFirstCall().resolves([[{ value: 'invalid json' }]]) @@ -60,7 +86,11 @@ describe('Migration v2.17.4-use-subfolder-for-oidc-redirect-uris', () => { await up({ context }) } catch (error) { expect(queryInterface.sequelize.query.calledOnce).to.be.true - expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect( + queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = :settingsKey;', { + replacements: { settingsKey: 'server-settings' } + }) + ).to.be.true expect(logger.error.calledWith('[2.17.4 migration] Error parsing server settings:')).to.be.true expect(error).to.be.instanceOf(Error) } @@ -73,7 +103,11 @@ describe('Migration v2.17.4-use-subfolder-for-oidc-redirect-uris', () => { await up({ context }) } catch (error) { expect(queryInterface.sequelize.query.calledOnce).to.be.true - expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect( + queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = :settingsKey;', { + replacements: { settingsKey: 'server-settings' } + }) + ).to.be.true expect(logger.error.calledWith('[2.17.4 migration] Server settings not found')).to.be.true expect(error).to.be.instanceOf(Error) } @@ -90,11 +124,16 @@ describe('Migration v2.17.4-use-subfolder-for-oidc-redirect-uris', () => { expect(logger.info.calledWith('[2.17.4 migration] DOWNGRADE BEGIN: 2.17.4-use-subfolder-for-oidc-redirect-uris ')).to.be.true expect(logger.info.calledWith('[2.17.4 migration] Removing authOpenIDSubfolderForRedirectURLs from server settings')).to.be.true expect(queryInterface.sequelize.query.calledTwice).to.be.true - expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true expect( - queryInterface.sequelize.query.calledWith('UPDATE settings SET value = :value WHERE key = "server-settings";', { + queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = :settingsKey;', { + replacements: { settingsKey: 'server-settings' } + }) + ).to.be.true + expect( + queryInterface.sequelize.query.calledWith('UPDATE settings SET value = :value WHERE key = :settingsKey;', { replacements: { - value: JSON.stringify({}) + value: JSON.stringify({}), + settingsKey: 'server-settings' } }) ).to.be.true @@ -109,7 +148,11 @@ describe('Migration v2.17.4-use-subfolder-for-oidc-redirect-uris', () => { expect(logger.info.calledWith('[2.17.4 migration] DOWNGRADE BEGIN: 2.17.4-use-subfolder-for-oidc-redirect-uris ')).to.be.true expect(logger.info.calledWith('[2.17.4 migration] authOpenIDSubfolderForRedirectURLs not found in server settings, no action required')).to.be.true expect(queryInterface.sequelize.query.calledOnce).to.be.true - expect(queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = "server-settings";')).to.be.true + expect( + queryInterface.sequelize.query.calledWith('SELECT value FROM settings WHERE key = :settingsKey;', { + replacements: { settingsKey: 'server-settings' } + }) + ).to.be.true expect(logger.info.calledWith('[2.17.4 migration] DOWNGRADE END: 2.17.4-use-subfolder-for-oidc-redirect-uris ')).to.be.true }) })