From a28241e735fd49d45ba0248ca94f251d0a1d8aeb Mon Sep 17 00:00:00 2001 From: Nicholas Wallace Date: Wed, 13 May 2026 14:42:39 -0700 Subject: [PATCH] Initial migration script and associated test --- .../v2.34.0-add-author-search-name.js | 321 ++++++++++++++++++ .../v2.34.0-add-author-search-name.test.js | 255 ++++++++++++++ 2 files changed, 576 insertions(+) create mode 100644 server/migrations/v2.34.0-add-author-search-name.js create mode 100644 test/server/migrations/v2.34.0-add-author-search-name.test.js diff --git a/server/migrations/v2.34.0-add-author-search-name.js b/server/migrations/v2.34.0-add-author-search-name.js new file mode 100644 index 000000000..ddac8d645 --- /dev/null +++ b/server/migrations/v2.34.0-add-author-search-name.js @@ -0,0 +1,321 @@ +const { Sequelize } = require('sequelize') + +/** + * @typedef MigrationContext + * @property {import('sequelize').QueryInterface} queryInterface - a suquelize QueryInterface object. + * @property {import('../Logger')} logger - a Logger object. + * + * @typedef MigrationOptions + * @property {MigrationContext} context - an object containing the migration context. + */ + +const Author = require('../models/Author') + +const migrationVersion = '2.34.0' +const migrationName = `${migrationVersion}-add-author-search-name` +const loggerPrefix = `[${migrationVersion} migration]` +const AUTHORS_TABLE = 'authors' +const BOOK_AUTHORS_TABLE = 'bookAuthors' +const LIBRARY_ITEMS_TABLE = 'libraryItems' +const AUTHOR_SEARCH_INDEX = 'author_search_name' +const AUTHOR_LAST_FIRST_INDEX = 'author_last_first' +const UNIQUE_SEARCH_INDEX = 'unique_author_search_name_per_library' + +async function indexExists(queryInterface, tableName, indexName) { + const indexes = await queryInterface.showIndex(tableName) + return indexes.some((index) => index.name === indexName) +} + +async function addIndexIfMissing(queryInterface, logger, tableName, indexName, options, transaction = null) { + if (await indexExists(queryInterface, tableName, indexName)) { + logger.info(`${loggerPrefix} index "${indexName}" already exists on "${tableName}"`) + return + } + + logger.info(`${loggerPrefix} adding index "${indexName}" on "${tableName}"`) + await queryInterface.addIndex(tableName, options.fields, { + ...options, + name: indexName, + transaction + }) +} + +async function removeIndexIfPresent(queryInterface, logger, tableName, indexName) { + if (!(await indexExists(queryInterface, tableName, indexName))) { + logger.info(`${loggerPrefix} index "${indexName}" does not exist on "${tableName}"`) + return + } + + logger.info(`${loggerPrefix} removing index "${indexName}" from "${tableName}"`) + await queryInterface.removeIndex(tableName, indexName) +} + +async function backfillAuthorSearchName(queryInterface, logger, transaction, offset = 0) { + while (true) { + const authors = await queryInterface.sequelize.query(`SELECT id, name FROM ${AUTHORS_TABLE} ORDER BY id ASC LIMIT :limit OFFSET :offset`, { + replacements: { limit: 500, offset }, + type: Sequelize.QueryTypes.SELECT, + transaction + }) + + if (!authors.length) return + + logger.info(`${loggerPrefix} backfilling derived author fields for ${authors.length} authors`) + for (const author of authors) { + const derivedFields = Author.buildAuthorDerivedFields(author.name) + await queryInterface.sequelize.query( + `UPDATE ${AUTHORS_TABLE} + SET lastFirst = :lastFirst, + searchName = :searchName + WHERE id = :id`, + { + replacements: { + id: author.id, + lastFirst: derivedFields.lastFirst, + searchName: derivedFields.searchName + }, + transaction + } + ) + } + + if (authors.length < 500) { + return + } + offset += 500 + } +} + +function compareAuthorsForMerge(left, right) { + const leftHasAsin = !!left.asin?.trim() + const rightHasAsin = !!right.asin?.trim() + if (leftHasAsin !== rightHasAsin) return leftHasAsin ? -1 : 1 + + const leftHasDescription = !!left.description?.trim() + const rightHasDescription = !!right.description?.trim() + if (leftHasDescription !== rightHasDescription) return leftHasDescription ? -1 : 1 + + const leftCreatedAt = Number.isFinite(new Date(left.createdAt).getTime()) ? new Date(left.createdAt).getTime() : Number.MAX_SAFE_INTEGER + const rightCreatedAt = Number.isFinite(new Date(right.createdAt).getTime()) ? new Date(right.createdAt).getTime() : Number.MAX_SAFE_INTEGER + if (leftCreatedAt !== rightCreatedAt) return leftCreatedAt - rightCreatedAt + + return String(left.id).localeCompare(String(right.id)) +} + +async function mergeDuplicateAuthors(queryInterface, logger, transaction) { + const authors = await queryInterface.sequelize.query( + `SELECT id, name, asin, description, createdAt, libraryId, searchName + FROM ${AUTHORS_TABLE} + WHERE searchName IS NOT NULL AND searchName != '' + ORDER BY libraryId ASC, searchName ASC, createdAt ASC, id ASC`, + { + type: Sequelize.QueryTypes.SELECT, + transaction + } + ) + + const duplicateGroups = new Map() + for (const author of authors) { + const key = `${author.libraryId}::${author.searchName}` + if (!duplicateGroups.has(key)) duplicateGroups.set(key, []) + duplicateGroups.get(key).push(author) + } + + const groupsToMerge = [...duplicateGroups.values()].filter((authorsInGroup) => authorsInGroup.length > 1) + if (!groupsToMerge.length) { + logger.info(`${loggerPrefix} no duplicate authors found to merge`) + return + } + + logger.info(`${loggerPrefix} merging ${groupsToMerge.length} duplicate author groups`) + + for (const authorsInGroup of groupsToMerge) { + const survivors = [...authorsInGroup].sort(compareAuthorsForMerge) + const survivor = survivors[0] + const duplicateIds = survivors.slice(1).map((author) => author.id) + if (!duplicateIds.length) continue + + logger.info(`${loggerPrefix} merging duplicate authors in library ${survivor.libraryId} for searchName "${survivor.searchName}" into "${survivor.id}"`) + + await queryInterface.sequelize.query( + `UPDATE ${BOOK_AUTHORS_TABLE} + SET authorId = :survivorId + WHERE authorId IN (:duplicateIds)`, + { + replacements: { + survivorId: survivor.id, + duplicateIds + }, + transaction + } + ) + + await queryInterface.sequelize.query( + `DELETE FROM ${AUTHORS_TABLE} + WHERE id IN (:duplicateIds)`, + { + replacements: { + duplicateIds + }, + transaction + } + ) + } +} + +async function cleanupDuplicateBookAuthors(queryInterface, transaction) { + const bookAuthorsTableDescription = await queryInterface.describeTable(BOOK_AUTHORS_TABLE) + if (!bookAuthorsTableDescription?.authorId) return + + await queryInterface.sequelize.query( + `DELETE FROM ${BOOK_AUTHORS_TABLE} + WHERE EXISTS ( + SELECT 1 + FROM ${BOOK_AUTHORS_TABLE} AS duplicateBookAuthors + WHERE duplicateBookAuthors.bookId = ${BOOK_AUTHORS_TABLE}.bookId + AND duplicateBookAuthors.authorId = ${BOOK_AUTHORS_TABLE}.authorId + AND ( + duplicateBookAuthors.createdAt < ${BOOK_AUTHORS_TABLE}.createdAt + OR ( + duplicateBookAuthors.createdAt = ${BOOK_AUTHORS_TABLE}.createdAt + AND duplicateBookAuthors.id < ${BOOK_AUTHORS_TABLE}.id + ) + ) + )`, + { + transaction + } + ) +} + +async function refreshLibraryItemAuthorNames(queryInterface, transaction) { + const libraryItemsTableDescription = await queryInterface.describeTable(LIBRARY_ITEMS_TABLE) + if (!libraryItemsTableDescription?.authorNamesFirstLast || !libraryItemsTableDescription?.authorNamesLastFirst) return + + await queryInterface.sequelize.query( + `UPDATE ${LIBRARY_ITEMS_TABLE} + SET (authorNamesFirstLast, authorNamesLastFirst) = ( + SELECT GROUP_CONCAT(authors.name, ', ' ORDER BY bookAuthors.createdAt ASC), + GROUP_CONCAT(authors.lastFirst, ', ' ORDER BY bookAuthors.createdAt ASC) + FROM authors JOIN bookAuthors ON authors.id = bookAuthors.authorId + WHERE bookAuthors.bookId = ${LIBRARY_ITEMS_TABLE}.mediaId + ) + WHERE mediaType = 'book'`, + { + transaction + } + ) +} + +/** + * This upward migration adds a searchName column to authors and indexes the + * derived fields used for normalized lookup and sorting. + * + * @param {MigrationOptions} options - an object containing the migration context. + * @returns {Promise} - A promise that resolves when the migration is complete. + */ +async function up({ context: { queryInterface, logger } }) { + logger.info(`${loggerPrefix} UPGRADE BEGIN: ${migrationName}`) + + const tableDescription = await queryInterface.describeTable(AUTHORS_TABLE) + + await queryInterface.sequelize.transaction(async (transaction) => { + if (!tableDescription.searchName) { + logger.info(`${loggerPrefix} adding column "searchName" to "${AUTHORS_TABLE}"`) + await queryInterface.addColumn( + AUTHORS_TABLE, + 'searchName', + { + type: Sequelize.DataTypes.STRING + }, + { + transaction + } + ) + } else { + logger.info(`${loggerPrefix} column "searchName" already exists on "${AUTHORS_TABLE}"`) + } + + if (!tableDescription.lastFirst) { + logger.info(`${loggerPrefix} adding column "lastFirst" to "${AUTHORS_TABLE}"`) + await queryInterface.addColumn( + AUTHORS_TABLE, + 'lastFirst', + { + type: Sequelize.DataTypes.STRING + }, + { + transaction + } + ) + } + + await backfillAuthorSearchName(queryInterface, logger, transaction, 0) + await mergeDuplicateAuthors(queryInterface, logger, transaction) + await cleanupDuplicateBookAuthors(queryInterface, transaction) + await refreshLibraryItemAuthorNames(queryInterface, transaction) + + await addIndexIfMissing( + queryInterface, + logger, + AUTHORS_TABLE, + AUTHOR_LAST_FIRST_INDEX, + { + fields: [{ name: 'lastFirst', collate: 'NOCASE' }] + }, + transaction + ) + + await addIndexIfMissing( + queryInterface, + logger, + AUTHORS_TABLE, + AUTHOR_SEARCH_INDEX, + { + fields: [{ name: 'searchName', collate: 'NOCASE' }] + }, + transaction + ) + + await addIndexIfMissing( + queryInterface, + logger, + AUTHORS_TABLE, + UNIQUE_SEARCH_INDEX, + { + fields: ['searchName', 'libraryId'], + unique: true + }, + transaction + ) + }) + + logger.info(`${loggerPrefix} UPGRADE END: ${migrationName}`) +} + +/** + * This downward migration removes the searchName column and indexes added by the + * upward migration. + * + * @param {MigrationOptions} options - an object containing the migration context. + * @returns {Promise} - A promise that resolves when the migration is complete. + */ +async function down({ context: { queryInterface, logger } }) { + logger.info(`${loggerPrefix} DOWNGRADE BEGIN: ${migrationName}`) + + await removeIndexIfPresent(queryInterface, logger, AUTHORS_TABLE, UNIQUE_SEARCH_INDEX) + await removeIndexIfPresent(queryInterface, logger, AUTHORS_TABLE, AUTHOR_SEARCH_INDEX) + await removeIndexIfPresent(queryInterface, logger, AUTHORS_TABLE, AUTHOR_LAST_FIRST_INDEX) + + const tableDescription = await queryInterface.describeTable(AUTHORS_TABLE) + if (tableDescription.searchName) { + logger.info(`${loggerPrefix} removing column "searchName" from "${AUTHORS_TABLE}"`) + await queryInterface.removeColumn(AUTHORS_TABLE, 'searchName') + } else { + logger.info(`${loggerPrefix} column "searchName" does not exist on "${AUTHORS_TABLE}"`) + } + + logger.info(`${loggerPrefix} DOWNGRADE END: ${migrationName}`) +} + +module.exports = { up, down } diff --git a/test/server/migrations/v2.34.0-add-author-search-name.test.js b/test/server/migrations/v2.34.0-add-author-search-name.test.js new file mode 100644 index 000000000..0fa6578ef --- /dev/null +++ b/test/server/migrations/v2.34.0-add-author-search-name.test.js @@ -0,0 +1,255 @@ +const chai = require('chai') +const sinon = require('sinon') +const { expect } = chai + +const { DataTypes, Sequelize } = require('sequelize') +const Logger = require('../../../server/Logger') +const Author = require('../../../server/models/Author') + +const { up, down } = require('../../../server/migrations/v2.34.0-add-author-search-name') + +describe('Migration v2.34.0-add-author-search-name', () => { + let sequelize + let queryInterface + + beforeEach(async () => { + sequelize = new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false }) + queryInterface = sequelize.getQueryInterface() + sinon.stub(Logger, 'info') + + await queryInterface.createTable('authors', { + id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, unique: true }, + name: { type: DataTypes.STRING, allowNull: false }, + lastFirst: { type: DataTypes.STRING, allowNull: true }, + searchName: { type: DataTypes.STRING, allowNull: true }, + asin: { type: DataTypes.STRING, allowNull: true }, + description: { type: DataTypes.TEXT, allowNull: true }, + libraryId: { type: DataTypes.INTEGER, allowNull: false }, + createdAt: { type: DataTypes.DATE, allowNull: true } + }) + + await queryInterface.createTable('bookAuthors', { + id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, unique: true }, + bookId: { type: DataTypes.INTEGER, allowNull: false }, + authorId: { type: DataTypes.INTEGER, allowNull: false }, + createdAt: { type: DataTypes.DATE, allowNull: true } + }) + + await queryInterface.createTable('libraryItems', { + id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, unique: true }, + mediaId: { type: DataTypes.INTEGER, allowNull: false }, + mediaType: { type: DataTypes.STRING, allowNull: false }, + authorNamesFirstLast: { type: DataTypes.STRING, allowNull: true }, + authorNamesLastFirst: { type: DataTypes.STRING, allowNull: true } + }) + + await queryInterface.bulkInsert('authors', [ + { id: 1, name: 'J.R.R. Tolkein', lastFirst: 'Tolkein, J. R. R.', asin: null, description: null, libraryId: 1, createdAt: '2020-01-01T00:00:00.000Z' }, + { id: 2, name: 'JRR Tolkein', lastFirst: 'Tolkein, JRR', asin: 'ASIN-1', description: null, libraryId: 1, createdAt: '2021-01-01T00:00:00.000Z' }, + { id: 3, name: 'John Smith', lastFirst: 'Smith, John', asin: null, description: 'Author bio', libraryId: 1, createdAt: '2020-01-02T00:00:00.000Z' }, + { id: 4, name: 'John Smith', lastFirst: 'Smith, John', asin: null, description: null, libraryId: 1, createdAt: '2019-01-01T00:00:00.000Z' }, + { id: 5, name: 'Anna Lee', lastFirst: 'Lee, Anna', asin: null, description: null, libraryId: 1, createdAt: '2022-01-01T00:00:00.000Z' }, + { id: 6, name: 'Anna-Lee', lastFirst: 'Lee, Anna', asin: null, description: null, libraryId: 1, createdAt: '2018-01-01T00:00:00.000Z' }, + { id: 7, name: 'JRR Tolkein', lastFirst: 'Tolkein, JRR', asin: null, description: null, libraryId: 2, createdAt: '2021-06-01T00:00:00.000Z' }, + { id: 8, name: 'Agatha Christie', lastFirst: 'Christie, Agatha', asin: null, description: null, libraryId: 2, createdAt: '2020-06-01T00:00:00.000Z' } + ]) + + await queryInterface.bulkInsert('bookAuthors', [ + { id: 1, bookId: 101, authorId: 1, createdAt: '2020-02-01T00:00:00.000Z' }, + { id: 2, bookId: 101, authorId: 2, createdAt: '2020-03-01T00:00:00.000Z' }, + { id: 3, bookId: 102, authorId: 3, createdAt: '2020-02-01T00:00:00.000Z' }, + { id: 4, bookId: 102, authorId: 4, createdAt: '2020-03-01T00:00:00.000Z' }, + { id: 5, bookId: 103, authorId: 5, createdAt: '2020-02-01T00:00:00.000Z' }, + { id: 6, bookId: 103, authorId: 6, createdAt: '2020-03-01T00:00:00.000Z' }, + { id: 7, bookId: 104, authorId: 7, createdAt: '2020-02-01T00:00:00.000Z' }, + { id: 8, bookId: 104, authorId: 8, createdAt: '2020-03-01T00:00:00.000Z' } + ]) + + await queryInterface.bulkInsert('libraryItems', [ + { id: 1, mediaId: 101, mediaType: 'book', authorNamesFirstLast: 'stale', authorNamesLastFirst: 'stale' }, + { id: 2, mediaId: 102, mediaType: 'book', authorNamesFirstLast: 'stale', authorNamesLastFirst: 'stale' }, + { id: 3, mediaId: 103, mediaType: 'book', authorNamesFirstLast: 'stale', authorNamesLastFirst: 'stale' }, + { id: 4, mediaId: 104, mediaType: 'book', authorNamesFirstLast: 'stale', authorNamesLastFirst: 'stale' } + ]) + }) + + afterEach(() => { + sinon.restore() + }) + + describe('up', () => { + it('should backfill derived author fields before adding indexes', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const authors = await queryInterface.sequelize.query('SELECT id, name, lastFirst, searchName, asin, description, libraryId FROM authors ORDER BY id ASC') + expect(authors[0]).to.deep.equal([ + { + id: 2, + name: 'JRR Tolkein', + lastFirst: 'Tolkein, JRR', + searchName: 'jrrtolkein', + asin: 'ASIN-1', + description: null, + libraryId: 1 + }, + { + id: 3, + name: 'John Smith', + lastFirst: 'Smith, John', + searchName: 'johnsmith', + asin: null, + description: 'Author bio', + libraryId: 1 + }, + { + id: 6, + name: 'Anna-Lee', + lastFirst: 'Anna-Lee', + searchName: 'annalee', + asin: null, + description: null, + libraryId: 1 + }, + { + id: 7, + name: 'JRR Tolkein', + lastFirst: 'Tolkein, JRR', + searchName: 'jrrtolkein', + asin: null, + description: null, + libraryId: 2 + }, + { + id: 8, + name: 'Agatha Christie', + lastFirst: 'Christie, Agatha', + searchName: 'agathachristie', + asin: null, + description: null, + libraryId: 2 + } + ]) + }) + + it('should merge duplicate authors per library and remap bookAuthors', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const authors = await queryInterface.sequelize.query('SELECT id, name, lastFirst, searchName, asin, description, libraryId FROM authors ORDER BY id ASC') + expect(authors[0]).to.deep.equal([ + { + id: 2, + name: 'JRR Tolkein', + lastFirst: 'Tolkein, JRR', + searchName: 'jrrtolkein', + asin: 'ASIN-1', + description: null, + libraryId: 1 + }, + { + id: 3, + name: 'John Smith', + lastFirst: 'Smith, John', + searchName: 'johnsmith', + asin: null, + description: 'Author bio', + libraryId: 1 + }, + { + id: 6, + name: 'Anna-Lee', + lastFirst: 'Anna-Lee', + searchName: 'annalee', + asin: null, + description: null, + libraryId: 1 + }, + { + id: 7, + name: 'JRR Tolkein', + lastFirst: 'Tolkein, JRR', + searchName: 'jrrtolkein', + asin: null, + description: null, + libraryId: 2 + }, + { + id: 8, + name: 'Agatha Christie', + lastFirst: 'Christie, Agatha', + searchName: 'agathachristie', + asin: null, + description: null, + libraryId: 2 + } + ]) + + const bookAuthors = await queryInterface.sequelize.query('SELECT bookId, authorId FROM bookAuthors ORDER BY bookId ASC, authorId ASC') + expect(bookAuthors[0]).to.deep.equal([ + { bookId: 101, authorId: 2 }, + { bookId: 102, authorId: 3 }, + { bookId: 103, authorId: 6 }, + { bookId: 104, authorId: 7 }, + { bookId: 104, authorId: 8 } + ]) + + const libraryItems = await queryInterface.sequelize.query('SELECT mediaId, authorNamesFirstLast, authorNamesLastFirst FROM libraryItems ORDER BY mediaId ASC') + expect(libraryItems[0]).to.deep.equal([ + { mediaId: 101, authorNamesFirstLast: 'JRR Tolkein', authorNamesLastFirst: 'Tolkein, JRR' }, + { mediaId: 102, authorNamesFirstLast: 'John Smith', authorNamesLastFirst: 'Smith, John' }, + { mediaId: 103, authorNamesFirstLast: 'Anna-Lee', authorNamesLastFirst: 'Anna-Lee' }, + { mediaId: 104, authorNamesFirstLast: 'JRR Tolkein, Agatha Christie', authorNamesLastFirst: 'Tolkein, JRR, Christie, Agatha' } + ]) + }) + + it('should create indexes after the merge completes', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const [[{ count: lastFirstCount }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='author_last_first'`) + expect(lastFirstCount).to.equal(1) + + const [[{ count: searchNameCount }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='author_search_name'`) + expect(searchNameCount).to.equal(1) + + const [[{ count: uniqueCount }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='unique_author_search_name_per_library'`) + expect(uniqueCount).to.equal(1) + + const [[{ count: duplicateCount }]] = await queryInterface.sequelize.query( + `SELECT COUNT(*) as count FROM authors WHERE libraryId = :libraryId AND searchName = :searchName`, + { + replacements: { + libraryId: 1, + searchName: Author.normalizeSearchName('JRR Tolkein') + } + } + ) + expect(Number(duplicateCount)).to.equal(1) + }) + + it('should be idempotent', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await up({ context: { queryInterface, logger: Logger } }) + + const tableDescription = await queryInterface.describeTable('authors') + expect(tableDescription.searchName).to.exist + + const [[{ count: uniqueCount }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='unique_author_search_name_per_library'`) + expect(uniqueCount).to.equal(1) + }) + }) + + describe('down', () => { + it('should remove searchName and its indexes', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + + const tableDescription = await queryInterface.describeTable('authors') + expect(tableDescription.searchName).to.not.exist + + const [[{ count: searchNameCount }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='author_search_name'`) + expect(searchNameCount).to.equal(0) + + const [[{ count: uniqueCount }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='unique_author_search_name_per_library'`) + expect(uniqueCount).to.equal(0) + }) + }) +})