Fix multiple bugs

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.

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)

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<Author>}
    */
-  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<Author>}
    */
-  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 }
+    )
   }
 
   /**

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<number>}
    */
-  static getCountForAuthor(authorId) {
+  static getCountForAuthor(authorId, transaction = null) {
     return this.count({
       where: {
         authorId
-      }
+      },
+      transaction
     })
   }
 

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<Series>}
    */
-  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<Series>}
    */
-  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 }
+    )
   }
 
   /**