diff --git a/artifacts/2026-02-06/move-to-library-specification.md b/artifacts/2026-02-06/move-to-library-specification.md index 6c98df3c3..6326a61bb 100644 --- a/artifacts/2026-02-06/move-to-library-specification.md +++ b/artifacts/2026-02-06/move-to-library-specification.md @@ -139,9 +139,12 @@ Following the initial implementation, several critical areas were improved: - **Issue**: Metadata `save()` calls for newly created series/authors were not awaited. - **Fix**: Ensured all `.save()` calls are properly awaited. -### 4. Transactional Integrity - FIXED -- **Issue**: The move logic was not wrapped in a DB transaction. -- **Fix**: Wrapped the DB update portion of `handleMoveLibraryItem` in a Sequelize transaction that is committed only if all updates succeed. +### 4. Transactional Integrity & Lock Optimization - FIXED +- **Issue**: The move logic was not wrapped in a DB transaction, and long-running file moves inside transactions would lock the SQLite database for several seconds (causing `SQLITE_BUSY`). +- **Fix**: + - Wrapped the DB update portion of `handleMoveLibraryItem` in a Sequelize transaction. + - **Optimization**: Moved the `fs.move` operation **outside** the transaction. The files are moved first, then the transaction handles the lightning-fast DB updates. If the DB update fails, the files are moved back to their original location. + - **Transaction Propagation**: Updated `Series`, `Author`, and `BookAuthor` model helpers to correctly accept and propagate the transaction object. ### 5. Scanner "ENOTDIR" Error - FIXED - **Issue**: Single-file items (e.g., `.m4b`) were being scanned as directories, leading to `ENOTDIR` errors and causing items to appear with the "has issues" icon. diff --git a/server/controllers/LibraryItemController.js b/server/controllers/LibraryItemController.js index b01ff1624..5c4c6c4df 100644 --- a/server/controllers/LibraryItemController.js +++ b/server/controllers/LibraryItemController.js @@ -46,9 +46,8 @@ const ShareManager = require('../managers/ShareManager') * @param {import('../models/LibraryItem')} libraryItem * @param {import('../models/Library')} targetLibrary * @param {import('../models/LibraryFolder')} targetFolder - * @param {import('sequelize').Transaction} [transaction] */ -async function handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder, transaction = null) { +async function handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder) { const oldPath = libraryItem.path const oldLibraryId = libraryItem.libraryId @@ -73,171 +72,180 @@ async function handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder, t Logger.info(`[LibraryItemController] Moving item "${libraryItem.media.title}" from "${oldPath}" to "${newPath}"`) await fs.move(oldPath, newPath) - // Update library item in database - libraryItem.libraryId = targetLibrary.id - libraryItem.libraryFolderId = targetFolder.id - libraryItem.path = newPath - libraryItem.relPath = newRelPath - libraryItem.isMissing = false - libraryItem.isInvalid = false - libraryItem.changed('updatedAt', true) - await libraryItem.save({ transaction }) - - // Update library files paths - if (libraryItem.libraryFiles?.length) { - libraryItem.libraryFiles = libraryItem.libraryFiles.map((lf) => { - if (lf.metadata?.path) { - lf.metadata.path = lf.metadata.path.replace(oldPath, newPath) - } - if (lf.metadata?.relPath) { - lf.metadata.relPath = lf.metadata.relPath.replace(oldRelPath, newRelPath) - } - return lf - }) - libraryItem.changed('libraryFiles', true) + // Update database within a transaction + const transaction = await Database.sequelize.transaction() + try { + // Update library item in database + libraryItem.libraryId = targetLibrary.id + libraryItem.libraryFolderId = targetFolder.id + libraryItem.path = newPath + libraryItem.relPath = newRelPath + libraryItem.isMissing = false + libraryItem.isInvalid = false + libraryItem.changed('updatedAt', true) await libraryItem.save({ transaction }) - } - // Update media file paths (audioFiles, ebookFile for books; podcastEpisodes for podcasts) - if (libraryItem.isBook) { - // Update audioFiles paths - if (libraryItem.media.audioFiles?.length) { - libraryItem.media.audioFiles = libraryItem.media.audioFiles.map((af) => { - if (af.metadata?.path) { - af.metadata.path = af.metadata.path.replace(oldPath, newPath) + // Update library files paths + if (libraryItem.libraryFiles?.length) { + libraryItem.libraryFiles = libraryItem.libraryFiles.map((lf) => { + if (lf.metadata?.path) { + lf.metadata.path = lf.metadata.path.replace(oldPath, newPath) } - if (af.metadata?.relPath) { - af.metadata.relPath = af.metadata.relPath.replace(oldRelPath, newRelPath) + if (lf.metadata?.relPath) { + lf.metadata.relPath = lf.metadata.relPath.replace(oldRelPath, newRelPath) } - return af + return lf }) - libraryItem.media.changed('audioFiles', true) + libraryItem.changed('libraryFiles', true) + await libraryItem.save({ transaction }) } - // Update ebookFile path - if (libraryItem.media.ebookFile?.metadata?.path) { - libraryItem.media.ebookFile.metadata.path = libraryItem.media.ebookFile.metadata.path.replace(oldPath, newPath) - if (libraryItem.media.ebookFile.metadata?.relPath) { - libraryItem.media.ebookFile.metadata.relPath = libraryItem.media.ebookFile.metadata.relPath.replace(oldRelPath, newRelPath) - } - libraryItem.media.changed('ebookFile', true) - } - // Update coverPath - if (libraryItem.media.coverPath) { - libraryItem.media.coverPath = libraryItem.media.coverPath.replace(oldPath, newPath) - } - await libraryItem.media.save({ transaction }) - } else if (libraryItem.isPodcast) { - // Update coverPath - if (libraryItem.media.coverPath) { - libraryItem.media.coverPath = libraryItem.media.coverPath.replace(oldPath, newPath) - } - await libraryItem.media.save({ transaction }) - // Update podcast episode audio file paths - for (const episode of libraryItem.media.podcastEpisodes || []) { - let episodeUpdated = false - if (episode.audioFile?.metadata?.path) { - episode.audioFile.metadata.path = episode.audioFile.metadata.path.replace(oldPath, newPath) - episodeUpdated = true - } - if (episode.audioFile?.metadata?.relPath) { - episode.audioFile.metadata.relPath = episode.audioFile.metadata.relPath.replace(oldRelPath, newRelPath) - episodeUpdated = true - } - if (episodeUpdated) { - episode.changed('audioFile', true) - await episode.save({ transaction }) - } - } - } - // Handle Series and Authors when moving a book - if (libraryItem.isBook) { - // Handle Series - const bookSeries = await Database.bookSeriesModel.findAll({ - where: { bookId: libraryItem.media.id }, - transaction - }) - for (const bs of bookSeries) { - const sourceSeries = await Database.seriesModel.findByPk(bs.seriesId, { transaction }) - if (sourceSeries) { - const targetSeries = await Database.seriesModel.findOrCreateByNameAndLibrary(sourceSeries.name, targetLibrary.id, transaction) - - // If target series doesn't have a description but source does, copy it - if (!targetSeries.description && sourceSeries.description) { - targetSeries.description = sourceSeries.description - await targetSeries.save({ transaction }) + // Update media file paths (audioFiles, ebookFile for books; podcastEpisodes for podcasts) + if (libraryItem.isBook) { + // Update audioFiles paths + if (libraryItem.media.audioFiles?.length) { + libraryItem.media.audioFiles = libraryItem.media.audioFiles.map((af) => { + if (af.metadata?.path) { + af.metadata.path = af.metadata.path.replace(oldPath, newPath) + } + if (af.metadata?.relPath) { + af.metadata.relPath = af.metadata.relPath.replace(oldRelPath, newRelPath) + } + return af + }) + libraryItem.media.changed('audioFiles', true) + } + // Update ebookFile path + if (libraryItem.media.ebookFile?.metadata?.path) { + libraryItem.media.ebookFile.metadata.path = libraryItem.media.ebookFile.metadata.path.replace(oldPath, newPath) + if (libraryItem.media.ebookFile.metadata?.relPath) { + libraryItem.media.ebookFile.metadata.relPath = libraryItem.media.ebookFile.metadata.relPath.replace(oldRelPath, newRelPath) } - - // Update link - bs.seriesId = targetSeries.id - await bs.save({ transaction }) - - // Check if source series is now empty - const sourceSeriesBooksCount = await Database.bookSeriesModel.count({ where: { seriesId: sourceSeries.id }, transaction }) - if (sourceSeriesBooksCount === 0) { - Logger.info(`[LibraryItemController] Source series "${sourceSeries.name}" in library ${oldLibraryId} is now empty. Deleting.`) - await sourceSeries.destroy({ transaction }) - Database.removeSeriesFromFilterData(oldLibraryId, sourceSeries.id) - - SocketAuthority.emitter('series_removed', { id: sourceSeries.id, libraryId: oldLibraryId }) + libraryItem.media.changed('ebookFile', true) + } + // Update coverPath + if (libraryItem.media.coverPath) { + libraryItem.media.coverPath = libraryItem.media.coverPath.replace(oldPath, newPath) + } + await libraryItem.media.save({ transaction }) + } else if (libraryItem.isPodcast) { + // Update coverPath + if (libraryItem.media.coverPath) { + libraryItem.media.coverPath = libraryItem.media.coverPath.replace(oldPath, newPath) + } + await libraryItem.media.save({ transaction }) + // Update podcast episode audio file paths + for (const episode of libraryItem.media.podcastEpisodes || []) { + let episodeUpdated = false + if (episode.audioFile?.metadata?.path) { + episode.audioFile.metadata.path = episode.audioFile.metadata.path.replace(oldPath, newPath) + episodeUpdated = true + } + if (episode.audioFile?.metadata?.relPath) { + episode.audioFile.metadata.relPath = episode.audioFile.metadata.relPath.replace(oldRelPath, newRelPath) + episodeUpdated = true + } + if (episodeUpdated) { + episode.changed('audioFile', true) + await episode.save({ transaction }) } } } - // Handle Authors - const bookAuthors = await Database.bookAuthorModel.findAll({ - where: { bookId: libraryItem.media.id }, - transaction - }) - for (const ba of bookAuthors) { - const sourceAuthor = await Database.authorModel.findByPk(ba.authorId, { transaction }) - if (sourceAuthor) { - const targetAuthor = await Database.authorModel.findOrCreateByNameAndLibrary(sourceAuthor.name, targetLibrary.id, transaction) + // Handle Series and Authors when moving a book + if (libraryItem.isBook) { + // Handle Series + const bookSeries = await Database.bookSeriesModel.findAll({ + where: { bookId: libraryItem.media.id }, + transaction + }) + for (const bs of bookSeries) { + const sourceSeries = await Database.seriesModel.findByPk(bs.seriesId, { transaction }) + if (sourceSeries) { + const targetSeries = await Database.seriesModel.findOrCreateByNameAndLibrary(sourceSeries.name, targetLibrary.id, transaction) - // Copy description and ASIN if target doesn't have them - let targetAuthorUpdated = false - if (!targetAuthor.description && sourceAuthor.description) { - targetAuthor.description = sourceAuthor.description - targetAuthorUpdated = true - } - if (!targetAuthor.asin && sourceAuthor.asin) { - targetAuthor.asin = sourceAuthor.asin - targetAuthorUpdated = true - } + // If target series doesn't have a description but source does, copy it + if (!targetSeries.description && sourceSeries.description) { + targetSeries.description = sourceSeries.description + await targetSeries.save({ transaction }) + } - // Copy image if target doesn't have one - if (!targetAuthor.imagePath && sourceAuthor.imagePath && (await fs.pathExists(sourceAuthor.imagePath))) { - const ext = Path.extname(sourceAuthor.imagePath) - const newImagePath = Path.posix.join(Path.join(global.MetadataPath, 'authors'), targetAuthor.id + ext) - try { - await fs.copy(sourceAuthor.imagePath, newImagePath) - targetAuthor.imagePath = newImagePath + // Update link + bs.seriesId = targetSeries.id + await bs.save({ transaction }) + + // Check if source series is now empty + const sourceSeriesBooksCount = await Database.bookSeriesModel.count({ where: { seriesId: sourceSeries.id }, transaction }) + if (sourceSeriesBooksCount === 0) { + Logger.info(`[LibraryItemController] Source series "${sourceSeries.name}" in library ${oldLibraryId} is now empty. Deleting.`) + await sourceSeries.destroy({ transaction }) + Database.removeSeriesFromFilterData(oldLibraryId, sourceSeries.id) + + SocketAuthority.emitter('series_removed', { id: sourceSeries.id, libraryId: oldLibraryId }) + } + } + } + + // Handle Authors + const bookAuthors = await Database.bookAuthorModel.findAll({ + where: { bookId: libraryItem.media.id }, + transaction + }) + for (const ba of bookAuthors) { + const sourceAuthor = await Database.authorModel.findByPk(ba.authorId, { transaction }) + if (sourceAuthor) { + const targetAuthor = await Database.authorModel.findOrCreateByNameAndLibrary(sourceAuthor.name, targetLibrary.id, transaction) + + // Copy description and ASIN if target doesn't have them + let targetAuthorUpdated = false + if (!targetAuthor.description && sourceAuthor.description) { + targetAuthor.description = sourceAuthor.description targetAuthorUpdated = true - } catch (err) { - Logger.error(`[LibraryItemController] Failed to copy author image`, err) } - } - - if (targetAuthorUpdated) await targetAuthor.save({ transaction }) - - // Update link - ba.authorId = targetAuthor.id - await ba.save({ transaction }) - - // Check if source author is now empty - const sourceAuthorBooksCount = await Database.bookAuthorModel.getCountForAuthor(sourceAuthor.id, transaction) - if (sourceAuthorBooksCount === 0) { - Logger.info(`[LibraryItemController] Source author "${sourceAuthor.name}" in library ${oldLibraryId} is now empty. Deleting.`) - if (sourceAuthor.imagePath) { - await fs.remove(sourceAuthor.imagePath).catch((err) => Logger.error(`[LibraryItemController] Failed to remove source author image`, err)) + if (!targetAuthor.asin && sourceAuthor.asin) { + targetAuthor.asin = sourceAuthor.asin + targetAuthorUpdated = true } - await sourceAuthor.destroy({ transaction }) - Database.removeAuthorFromFilterData(oldLibraryId, sourceAuthor.id) - SocketAuthority.emitter('author_removed', { id: sourceAuthor.id, libraryId: oldLibraryId }) + // Copy image if target doesn't have one + if (!targetAuthor.imagePath && sourceAuthor.imagePath && (await fs.pathExists(sourceAuthor.imagePath))) { + const ext = Path.extname(sourceAuthor.imagePath) + const newImagePath = Path.posix.join(Path.join(global.MetadataPath, 'authors'), targetAuthor.id + ext) + try { + await fs.copy(sourceAuthor.imagePath, newImagePath) + targetAuthor.imagePath = newImagePath + targetAuthorUpdated = true + } catch (err) { + Logger.error(`[LibraryItemController] Failed to copy author image`, err) + } + } + + if (targetAuthorUpdated) await targetAuthor.save({ transaction }) + + // Update link + ba.authorId = targetAuthor.id + await ba.save({ transaction }) + + // Check if source author is now empty + const sourceAuthorBooksCount = await Database.bookAuthorModel.getCountForAuthor(sourceAuthor.id, transaction) + if (sourceAuthorBooksCount === 0) { + Logger.info(`[LibraryItemController] Source author "${sourceAuthor.name}" in library ${oldLibraryId} is now empty. Deleting.`) + if (sourceAuthor.imagePath) { + await fs.remove(sourceAuthor.imagePath).catch((err) => Logger.error(`[LibraryItemController] Failed to remove source author image`, err)) + } + await sourceAuthor.destroy({ transaction }) + Database.removeAuthorFromFilterData(oldLibraryId, sourceAuthor.id) + + SocketAuthority.emitter('author_removed', { id: sourceAuthor.id, libraryId: oldLibraryId }) + } } } } + + await transaction.commit() + } catch (dbError) { + if (transaction) await transaction.rollback() + throw dbError } // Emit socket events for UI updates @@ -1107,19 +1115,11 @@ class LibraryItemController { continue } - const transaction = await Database.sequelize.transaction() - try { - await handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder, transaction) - await transaction.commit() - successCount++ - } catch (error) { - if (transaction) await transaction.rollback() - failCount++ - errors.push({ id: libraryItem.id, error: error.message || 'Failed to move item' }) - } + await handleMoveLibraryItem(libraryItem, targetLibrary, targetFolder) + successCount++ } catch (error) { failCount++ - errors.push({ id: libraryItem.id, error: error.message || 'Unknown error' }) + errors.push({ id: libraryItem.id, error: error.message || 'Failed to move item' }) } } @@ -1565,14 +1565,7 @@ class LibraryItemController { } try { - const transaction = await Database.sequelize.transaction() - try { - await handleMoveLibraryItem(req.libraryItem, targetLibrary, targetFolder, transaction) - await transaction.commit() - } catch (error) { - await transaction.rollback() - throw error - } + await handleMoveLibraryItem(req.libraryItem, targetLibrary, targetFolder) await Database.resetLibraryIssuesFilterData(sourceLibrary.id) await Database.resetLibraryIssuesFilterData(targetLibrary.id) diff --git a/server/models/Author.js b/server/models/Author.js index 287b66976..2d4145c22 100644 --- a/server/models/Author.js +++ b/server/models/Author.js @@ -50,16 +50,18 @@ class Author extends Model { * * @param {string} authorName * @param {string} libraryId + * @param {import('sequelize').Transaction} [transaction] * @returns {Promise} */ - static async getByNameAndLibrary(authorName, libraryId) { + static async getByNameAndLibrary(authorName, libraryId, transaction = null) { return this.findOne({ where: [ where(fn('lower', col('name')), authorName.toLowerCase()), { libraryId } - ] + ], + transaction }) } @@ -111,16 +113,20 @@ class Author extends Model { * * @param {string} name * @param {string} libraryId + * @param {import('sequelize').Transaction} [transaction] * @returns {Promise} */ - static async findOrCreateByNameAndLibrary(name, libraryId) { - const author = await this.getByNameAndLibrary(name, libraryId) + static async findOrCreateByNameAndLibrary(name, libraryId, transaction = null) { + const author = await this.getByNameAndLibrary(name, libraryId, transaction) if (author) return author - return this.create({ - name, - lastFirst: this.getLastFirst(name), - libraryId - }) + return this.create( + { + name, + lastFirst: this.getLastFirst(name), + libraryId + }, + { transaction } + ) } /** diff --git a/server/models/BookAuthor.js b/server/models/BookAuthor.js index d7d65728e..ede84f698 100644 --- a/server/models/BookAuthor.js +++ b/server/models/BookAuthor.js @@ -27,13 +27,15 @@ class BookAuthor extends Model { * Get number of books for author * * @param {string} authorId + * @param {import('sequelize').Transaction} [transaction] * @returns {Promise} */ - static getCountForAuthor(authorId) { + static getCountForAuthor(authorId, transaction = null) { return this.count({ where: { authorId - } + }, + transaction }) } diff --git a/server/models/Series.js b/server/models/Series.js index 6ca288464..6bcc7a624 100644 --- a/server/models/Series.js +++ b/server/models/Series.js @@ -41,16 +41,18 @@ class Series extends Model { * * @param {string} seriesName * @param {string} libraryId + * @param {import('sequelize').Transaction} [transaction] * @returns {Promise} */ - static async getByNameAndLibrary(seriesName, libraryId) { + static async getByNameAndLibrary(seriesName, libraryId, transaction = null) { return this.findOne({ where: [ where(fn('lower', col('name')), seriesName.toLowerCase()), { libraryId } - ] + ], + transaction }) } @@ -70,16 +72,20 @@ class Series extends Model { * * @param {string} seriesName * @param {string} libraryId + * @param {import('sequelize').Transaction} [transaction] * @returns {Promise} */ - static async findOrCreateByNameAndLibrary(seriesName, libraryId) { - const series = await this.getByNameAndLibrary(seriesName, libraryId) + static async findOrCreateByNameAndLibrary(seriesName, libraryId, transaction = null) { + const series = await this.getByNameAndLibrary(seriesName, libraryId, transaction) if (series) return series - return this.create({ - name: seriesName, - nameIgnorePrefix: getTitleIgnorePrefix(seriesName), - libraryId - }) + return this.create( + { + name: seriesName, + nameIgnorePrefix: getTitleIgnorePrefix(seriesName), + libraryId + }, + { transaction } + ) } /**