From a13143245b5bc1aafd8fac0c27b796be452da39f Mon Sep 17 00:00:00 2001 From: mikiher Date: Sat, 8 Feb 2025 12:29:23 +0200 Subject: [PATCH] Improve page load queries on title, titleIgnorePrefix, and addedAt sort order --- .../v2.19.1-copy-title-to-library-items.js | 156 ++++++++++++++++++ server/models/Book.js | 9 + server/models/LibraryItem.js | 14 +- server/scanner/BookScanner.js | 2 + .../utils/queries/libraryItemsBookFilters.js | 43 ++++- ...2.19.1-copy-title-to-library-items.test.js | 148 +++++++++++++++++ 6 files changed, 363 insertions(+), 9 deletions(-) create mode 100644 server/migrations/v2.19.1-copy-title-to-library-items.js create mode 100644 test/server/migrations/v2.19.1-copy-title-to-library-items.test.js diff --git a/server/migrations/v2.19.1-copy-title-to-library-items.js b/server/migrations/v2.19.1-copy-title-to-library-items.js new file mode 100644 index 00000000..8f982333 --- /dev/null +++ b/server/migrations/v2.19.1-copy-title-to-library-items.js @@ -0,0 +1,156 @@ +const util = require('util') + +/** + * @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 migrationVersion = '2.19.1' +const migrationName = `${migrationVersion}-copy-title-to-library-items` +const loggerPrefix = `[${migrationVersion} migration]` + +/** + * This upward migration adds a title column to the libraryItems table, copies the title from the book to the libraryItem, + * and creates a new index on the title column. In addition it sets a trigger on the books table to update the title column + * in the libraryItems table when a book is updated. + * + * @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 } }) { + // Upwards migration script + logger.info(`${loggerPrefix} UPGRADE BEGIN: ${migrationName}`) + + await addColumn(queryInterface, logger, 'libraryItems', 'title', { type: queryInterface.sequelize.Sequelize.STRING, allowNull: true }) + await copyColumn(queryInterface, logger, 'books', 'title', 'id', 'libraryItems', 'title', 'mediaId') + await addTrigger(queryInterface, logger, 'books', 'title', 'id', 'libraryItems', 'title', 'mediaId') + await addIndex(queryInterface, logger, 'libraryItems', ['libraryId', 'mediaType', { name: 'title', collate: 'NOCASE' }]) + + await addColumn(queryInterface, logger, 'libraryItems', 'titleIgnorePrefix', { type: queryInterface.sequelize.Sequelize.STRING, allowNull: true }) + await copyColumn(queryInterface, logger, 'books', 'titleIgnorePrefix', 'id', 'libraryItems', 'titleIgnorePrefix', 'mediaId') + await addTrigger(queryInterface, logger, 'books', 'titleIgnorePrefix', 'id', 'libraryItems', 'titleIgnorePrefix', 'mediaId') + await addIndex(queryInterface, logger, 'libraryItems', ['libraryId', 'mediaType', { name: 'titleIgnorePrefix', collate: 'NOCASE' }]) + + await addIndex(queryInterface, logger, 'libraryItems', ['libraryId', 'mediaType', 'createdAt']) + + logger.info(`${loggerPrefix} UPGRADE END: ${migrationName}`) +} + +/** + * This downward migration script removes the title column from the libraryItems table, removes the trigger on the books table, + * and removes the index on the title column. + * + * @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 } }) { + // Downward migration script + logger.info(`${loggerPrefix} DOWNGRADE BEGIN: ${migrationName}`) + + await removeIndex(queryInterface, logger, 'libraryItems', ['libraryId', 'mediaType', 'title']) + await removeTrigger(queryInterface, logger, 'libraryItems', 'title') + await removeColumn(queryInterface, logger, 'libraryItems', 'title') + + await removeIndex(queryInterface, logger, 'libraryItems', ['libraryId', 'mediaType', 'titleIgnorePrefix']) + await removeTrigger(queryInterface, logger, 'libraryItems', 'titleIgnorePrefix') + await removeColumn(queryInterface, logger, 'libraryItems', 'titleIgnorePrefix') + + await removeIndex(queryInterface, logger, 'libraryItems', ['libraryId', 'mediaType', 'createdAt']) + + logger.info(`${loggerPrefix} DOWNGRADE END: ${migrationName}`) +} + +/** + * Utility function to add an index to a table. If the index already z`exists, it logs a message and continues. + * + * @param {import('sequelize').QueryInterface} queryInterface + * @param {import ('../Logger')} logger + * @param {string} tableName + * @param {string[]} columns + */ +async function addIndex(queryInterface, logger, tableName, columns) { + const columnString = columns.map((column) => util.inspect(column)).join(', ') + const indexName = convertToSnakeCase(`${tableName}_${columns.map((column) => (typeof column === 'string' ? column : column.name)).join('_')}`) + try { + logger.info(`${loggerPrefix} adding index on [${columnString}] to table ${tableName}. index name: ${indexName}"`) + await queryInterface.addIndex(tableName, columns) + logger.info(`${loggerPrefix} added index on [${columnString}] to table ${tableName}. index name: ${indexName}"`) + } catch (error) { + if (error.name === 'SequelizeDatabaseError' && error.message.includes('already exists')) { + logger.info(`${loggerPrefix} index [${columnString}] for table "${tableName}" already exists`) + } else { + throw error + } + } +} + +/** + * Utility function to remove an index from a table. + * Sequelize implemets it using DROP INDEX IF EXISTS, so it won't throw an error if the index doesn't exist. + * + * @param {import('sequelize').QueryInterface} queryInterface + * @param {import ('../Logger')} logger + * @param {string} tableName + * @param {string[]} columns + */ +async function removeIndex(queryInterface, logger, tableName, columns) { + logger.info(`${loggerPrefix} removing index [${columns.join(', ')}] from table "${tableName}"`) + await queryInterface.removeIndex(tableName, columns) + logger.info(`${loggerPrefix} removed index [${columns.join(', ')}] from table "${tableName}"`) +} + +async function addColumn(queryInterface, logger, table, column, options) { + logger.info(`${loggerPrefix} adding column "${column}" to table "${table}"`) + await queryInterface.addColumn(table, column, options) + logger.info(`${loggerPrefix} added column "${column}" to table "${table}"`) +} + +async function removeColumn(queryInterface, logger, table, column) { + logger.info(`${loggerPrefix} removing column "${column}" from table "${table}"`) + await queryInterface.removeColumn(table, column) + logger.info(`${loggerPrefix} removed column "${column}" from table "${table}"`) +} + +async function copyColumn(queryInterface, logger, sourceTable, sourceColumn, sourceIdColumn, targetTable, targetColumn, targetIdColumn) { + logger.info(`${loggerPrefix} copying column "${sourceColumn}" from table "${sourceTable}" to table "${targetTable}"`) + await queryInterface.sequelize.query(` + UPDATE ${targetTable} + SET ${targetColumn} = ${sourceTable}.${sourceColumn} + FROM ${sourceTable} + WHERE ${targetTable}.${targetIdColumn} = ${sourceTable}.${sourceIdColumn} + `) + logger.info(`${loggerPrefix} copied column "${sourceColumn}" from table "${sourceTable}" to table "${targetTable}"`) +} + +async function addTrigger(queryInterface, logger, sourceTable, sourceColumn, sourceIdColumn, targetTable, targetColumn, targetIdColumn) { + logger.info(`${loggerPrefix} adding trigger to update ${targetTable}.${targetColumn} when ${sourceTable}.${sourceColumn} is updated`) + const triggerName = convertToSnakeCase(`update_${targetTable}_${targetColumn}`) + await queryInterface.sequelize.query(` + CREATE TRIGGER ${triggerName} + AFTER UPDATE OF ${sourceColumn} ON ${sourceTable} + FOR EACH ROW + BEGIN + UPDATE ${targetTable} + SET ${targetColumn} = NEW.${sourceColumn} + WHERE ${targetTable}.${targetIdColumn} = NEW.${sourceIdColumn}; + END; + `) + logger.info(`${loggerPrefix} added trigger to update ${targetTable}.${targetColumn} when ${sourceTable}.${sourceColumn} is updated`) +} + +async function removeTrigger(queryInterface, logger, targetTable, targetColumn) { + logger.info(`${loggerPrefix} removing trigger to update ${targetTable}.${targetColumn}`) + const triggerName = convertToSnakeCase(`update_${targetTable}_${targetColumn}`) + await queryInterface.sequelize.query(`DROP TRIGGER IF EXISTS ${triggerName}`) + logger.info(`${loggerPrefix} removed trigger to update ${targetTable}.${targetColumn}`) +} + +function convertToSnakeCase(str) { + return str.replace(/([A-Z])/g, '_$1').toLowerCase() +} + +module.exports = { up, down } diff --git a/server/models/Book.js b/server/models/Book.js index 3684608d..fd24c269 100644 --- a/server/models/Book.js +++ b/server/models/Book.js @@ -3,6 +3,7 @@ const Logger = require('../Logger') const { getTitlePrefixAtEnd, getTitleIgnorePrefix } = require('../utils') const parseNameString = require('../utils/parsers/parseNameString') const htmlSanitizer = require('../utils/htmlSanitizer') +const libraryItemsBookFilters = require('../utils/queries/libraryItemsBookFilters') /** * @typedef EBookFileObject @@ -192,6 +193,14 @@ class Book extends Model { ] } ) + + Book.addHook('afterDestroy', async (instance) => { + libraryItemsBookFilters.clearCountCache('afterDestroy ') + }) + + Book.addHook('afterCreate', async (instance) => { + libraryItemsBookFilters.clearCountCache('afterCreate') + }) } /** diff --git a/server/models/LibraryItem.js b/server/models/LibraryItem.js index 3ed4e31e..bd11e585 100644 --- a/server/models/LibraryItem.js +++ b/server/models/LibraryItem.js @@ -73,6 +73,10 @@ class LibraryItem extends Model { /** @type {Book.BookExpanded|Podcast.PodcastExpanded} - only set when expanded */ this.media + /** @type {string} */ + this.title // Only used for sorting + /** @type {string} */ + this.titleIgnorePrefix // Only used for sorting } /** @@ -677,7 +681,9 @@ class LibraryItem extends Model { lastScan: DataTypes.DATE, lastScanVersion: DataTypes.STRING, libraryFiles: DataTypes.JSON, - extraData: DataTypes.JSON + extraData: DataTypes.JSON, + title: DataTypes.STRING, + titleIgnorePrefix: DataTypes.STRING }, { sequelize, @@ -695,6 +701,12 @@ class LibraryItem extends Model { { fields: ['libraryId', 'mediaType', 'size'] }, + { + fields: ['libraryId', 'mediaType', { name: 'title', collate: 'NOCASE' }] + }, + { + fields: ['libraryId', 'mediaType', { name: 'titleIgnorePrefix', collate: 'NOCASE' }] + }, { fields: ['libraryId', 'mediaId', 'mediaType'] }, diff --git a/server/scanner/BookScanner.js b/server/scanner/BookScanner.js index 74798dd6..210f20f9 100644 --- a/server/scanner/BookScanner.js +++ b/server/scanner/BookScanner.js @@ -521,6 +521,8 @@ class BookScanner { libraryItemObj.isMissing = false libraryItemObj.isInvalid = false libraryItemObj.extraData = {} + libraryItemObj.title = bookMetadata.title + libraryItemObj.titleIgnorePrefix = getTitleIgnorePrefix(bookMetadata.title) // Set isSupplementary flag on ebook library files for (const libraryFile of libraryItemObj.libraryFiles) { diff --git a/server/utils/queries/libraryItemsBookFilters.js b/server/utils/queries/libraryItemsBookFilters.js index 9e74276a..3adb929e 100644 --- a/server/utils/queries/libraryItemsBookFilters.js +++ b/server/utils/queries/libraryItemsBookFilters.js @@ -4,6 +4,9 @@ const Logger = require('../../Logger') const authorFilters = require('./authorFilters') const ShareManager = require('../../managers/ShareManager') +const { profile } = require('../profiler') + +const countCache = new Map() module.exports = { /** @@ -270,9 +273,9 @@ module.exports = { } if (global.ServerSettings.sortingIgnorePrefix) { - return [[Sequelize.literal('titleIgnorePrefix COLLATE NOCASE'), dir]] + return [[Sequelize.literal('`libraryItem`.`titleIgnorePrefix` COLLATE NOCASE'), dir]] } else { - return [[Sequelize.literal('`book`.`title` COLLATE NOCASE'), dir]] + return [[Sequelize.literal('`libraryItem`.`title` COLLATE NOCASE'), dir]] } } else if (sortBy === 'sequence') { const nullDir = sortDesc ? 'DESC NULLS FIRST' : 'ASC NULLS LAST' @@ -336,6 +339,28 @@ module.exports = { return { booksToExclude, bookSeriesToInclude } }, + clearCountCache(hook) { + Logger.debug(`[LibraryItemsBookFilters] book.${hook}: Clearing count cache`) + countCache.clear() + }, + + async findAndCountAll(findOptions, limit, offset) { + const findOptionsKey = JSON.stringify(findOptions) + Logger.debug(`[LibraryItemsBookFilters] findOptionsKey: ${findOptionsKey}`) + + findOptions.limit = limit || null + findOptions.offset = offset + + if (countCache.has(findOptionsKey)) { + const rows = await Database.bookModel.findAll(findOptions) + return { rows, count: countCache.get(findOptionsKey) } + } else { + const result = await Database.bookModel.findAndCountAll(findOptions) + countCache.set(findOptionsKey, result.count) + return result + } + }, + /** * Get library items for book media type using filter and sort * @param {string} libraryId @@ -411,7 +436,8 @@ module.exports = { if (includeRSSFeed) { libraryItemIncludes.push({ model: Database.feedModel, - required: filterGroup === 'feed-open' + required: filterGroup === 'feed-open', + separate: true }) } if (filterGroup === 'feed-open' && !includeRSSFeed) { @@ -560,7 +586,7 @@ module.exports = { } } - const { rows: books, count } = await Database.bookModel.findAndCountAll({ + const findOptions = { where: bookWhere, distinct: true, attributes: bookAttributes, @@ -577,10 +603,11 @@ module.exports = { ...bookIncludes ], order: sortOrder, - subQuery: false, - limit: limit || null, - offset - }) + subQuery: false + } + + const findAndCountAll = process.env.QUERY_PROFILING ? profile(this.findAndCountAll) : this.findAndCountAll + const { rows: books, count } = await findAndCountAll(findOptions, limit, offset) const libraryItems = books.map((bookExpanded) => { const libraryItem = bookExpanded.libraryItem diff --git a/test/server/migrations/v2.19.1-copy-title-to-library-items.test.js b/test/server/migrations/v2.19.1-copy-title-to-library-items.test.js new file mode 100644 index 00000000..5b767856 --- /dev/null +++ b/test/server/migrations/v2.19.1-copy-title-to-library-items.test.js @@ -0,0 +1,148 @@ +const chai = require('chai') +const sinon = require('sinon') +const { expect } = chai + +const { DataTypes, Sequelize } = require('sequelize') +const Logger = require('../../../server/Logger') + +const { up, down } = require('../../../server/migrations/v2.19.1-copy-title-to-library-items') + +describe('Migration v2.19.1-copy-title-to-library-items', () => { + let sequelize + let queryInterface + let loggerInfoStub + + beforeEach(async () => { + sequelize = new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false }) + queryInterface = sequelize.getQueryInterface() + loggerInfoStub = sinon.stub(Logger, 'info') + + await queryInterface.createTable('books', { + id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, unique: true }, + title: { type: DataTypes.STRING, allowNull: true }, + titleIgnorePrefix: { type: DataTypes.STRING, allowNull: true } + }) + + await queryInterface.createTable('libraryItems', { + id: { type: DataTypes.INTEGER, allowNull: false, primaryKey: true, unique: true }, + libraryId: { type: DataTypes.INTEGER, allowNull: false }, + mediaType: { type: DataTypes.STRING, allowNull: false }, + mediaId: { type: DataTypes.INTEGER, allowNull: false }, + createdAt: { type: DataTypes.DATE, allowNull: false } + }) + + await queryInterface.bulkInsert('books', [ + { id: 1, title: 'The Book 1', titleIgnorePrefix: 'Book 1, The' }, + { id: 2, title: 'Book 2', titleIgnorePrefix: 'Book 2' } + ]) + + await queryInterface.bulkInsert('libraryItems', [ + { id: 1, libraryId: 1, mediaType: 'book', mediaId: 1, createdAt: '2025-01-01 00:00:00.000 +00:00' }, + { id: 2, libraryId: 2, mediaType: 'book', mediaId: 2, createdAt: '2025-01-02 00:00:00.000 +00:00' } + ]) + }) + + afterEach(() => { + sinon.restore() + }) + + describe('up', () => { + it('should copy title and titleIgnorePrefix to libraryItems', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const [libraryItems] = await queryInterface.sequelize.query('SELECT * FROM libraryItems') + expect(libraryItems).to.deep.equal([ + { id: 1, libraryId: 1, mediaType: 'book', mediaId: 1, title: 'The Book 1', titleIgnorePrefix: 'Book 1, The', createdAt: '2025-01-01 00:00:00.000 +00:00' }, + { id: 2, libraryId: 2, mediaType: 'book', mediaId: 2, title: 'Book 2', titleIgnorePrefix: 'Book 2', createdAt: '2025-01-02 00:00:00.000 +00:00' } + ]) + }) + + it('should add index on title to libraryItems', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='library_items_library_id_media_type_title_ignore_prefix'`) + expect(count).to.equal(1) + }) + + it('should add trigger to books.title to update libraryItems.title', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='trigger' AND name='update_library_items_title'`) + expect(count).to.equal(1) + }) + + it('should add index on titleIgnorePrefix to libraryItems', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='library_items_library_id_media_type_title_ignore_prefix'`) + expect(count).to.equal(1) + }) + + it('should add trigger to books.titleIgnorePrefix to update libraryItems.titleIgnorePrefix', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='trigger' AND name='update_library_items_title_ignore_prefix'`) + expect(count).to.equal(1) + }) + + it('should add index on createdAt to libraryItems', async () => { + await up({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='library_items_library_id_media_type_created_at'`) + expect(count).to.equal(1) + }) + }) + + describe('down', () => { + it('should remove title and titleIgnorePrefix from libraryItems', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + + const [libraryItems] = await queryInterface.sequelize.query('SELECT * FROM libraryItems') + expect(libraryItems).to.deep.equal([ + { id: 1, libraryId: 1, mediaType: 'book', mediaId: 1, createdAt: '2025-01-01 00:00:00.000 +00:00' }, + { id: 2, libraryId: 2, mediaType: 'book', mediaId: 2, createdAt: '2025-01-02 00:00:00.000 +00:00' } + ]) + }) + + it('should remove title trigger from books', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='trigger' AND name='update_library_items_title'`) + expect(count).to.equal(0) + }) + + it('should remove titleIgnorePrefix trigger from books', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='trigger' AND name='update_library_items_title_ignore_prefix'`) + expect(count).to.equal(0) + }) + + it('should remove index on titleIgnorePrefix from libraryItems', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='library_items_library_id_media_type_title_ignore_prefix'`) + expect(count).to.equal(0) + }) + + it('should remove index on title from libraryItems', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='library_items_library_id_media_type_title'`) + expect(count).to.equal(0) + }) + + it('should remove index on createdAt from libraryItems', async () => { + await up({ context: { queryInterface, logger: Logger } }) + await down({ context: { queryInterface, logger: Logger } }) + + const [[{ count }]] = await queryInterface.sequelize.query(`SELECT COUNT(*) as count FROM sqlite_master WHERE type='index' AND name='library_items_library_id_media_type_created_at'`) + expect(count).to.equal(0) + }) + }) +})