Merge pull request #5714 from mikiher/fix-library-scan-race

Fix library scan crash when a series or book link already exists

server/scanner/BookScanner.js

@@ -224,12 +224,16 @@ class BookScanner {
           if (!media.authors.some((au) => au.name === authorName)) {
             const existingAuthorId = await Database.getAuthorIdByName(libraryItemData.libraryId, authorName)
             if (existingAuthorId) {
-              await Database.bookAuthorModel.create({
-                bookId: media.id,
-                authorId: existingAuthorId
-              })
-              libraryScan.authorsNumBooksChangedIds.add(existingAuthorId)
-              libraryScan.addLog(LogLevel.DEBUG, `Updating book "${bookMetadata.title}" added author "${authorName}"`)
+              try {
+                await Database.bookAuthorModel.create({
+                  bookId: media.id,
+                  authorId: existingAuthorId
+                })
+                libraryScan.authorsNumBooksChangedIds.add(existingAuthorId)
+                libraryScan.addLog(LogLevel.DEBUG, `Updating book "${bookMetadata.title}" added author "${authorName}"`)
+              } catch (error) {
+                if (error.name !== 'SequelizeUniqueConstraintError') throw error
+              }
               authorsUpdated = true
             } else {
               const newAuthor = await Database.authorModel.create({
@@ -262,22 +266,51 @@ class BookScanner {
           if (!existingBookSeries) {
             const existingSeriesId = await Database.getSeriesIdByName(libraryItemData.libraryId, seriesObj.name)
             if (existingSeriesId) {
-              await Database.bookSeriesModel.create({
-                bookId: media.id,
-                seriesId: existingSeriesId,
-                sequence: seriesObj.sequence
-              })
-              libraryScan.addLog(LogLevel.DEBUG, `Updating book "${bookMetadata.title}" added series "${seriesObj.name}"${seriesObj.sequence ? ` with sequence "${seriesObj.sequence}"` : ''}`)
+              try {
+                await Database.bookSeriesModel.create({
+                  bookId: media.id,
+                  seriesId: existingSeriesId,
+                  sequence: seriesObj.sequence
+                })
+                libraryScan.addLog(LogLevel.DEBUG, `Updating book "${bookMetadata.title}" added series "${seriesObj.name}"${seriesObj.sequence ? ` with sequence "${seriesObj.sequence}"` : ''}`)
+              } catch (error) {
+                if (error.name !== 'SequelizeUniqueConstraintError') throw error
+              }
               seriesUpdated = true
             } else {
-              const newSeries = await Database.seriesModel.create({
-                name: seriesObj.name,
-                nameIgnorePrefix: getTitleIgnorePrefix(seriesObj.name),
-                libraryId: libraryItemData.libraryId
-              })
-              await media.addSeries(newSeries, { through: { sequence: seriesObj.sequence } })
-              Database.addSeriesToFilterData(libraryItemData.libraryId, newSeries.name, newSeries.id)
-              libraryScan.addLog(LogLevel.DEBUG, `Updating book "${bookMetadata.title}" added new series "${seriesObj.name}"${seriesObj.sequence ? ` with sequence "${seriesObj.sequence}"` : ''}`)
+              let newSeries
+              let seriesCreated = false
+              try {
+                newSeries = await Database.seriesModel.create({
+                  name: seriesObj.name,
+                  nameIgnorePrefix: getTitleIgnorePrefix(seriesObj.name),
+                  libraryId: libraryItemData.libraryId
+                })
+                seriesCreated = true
+              } catch (error) {
+                if (error.name !== 'SequelizeUniqueConstraintError') throw error
+                newSeries = await Database.seriesModel.findOne({
+                  where: { name: seriesObj.name, libraryId: libraryItemData.libraryId }
+                })
+                if (!newSeries) throw error
+              }
+              if (seriesCreated) {
+                Database.addSeriesToFilterData(libraryItemData.libraryId, newSeries.name, newSeries.id)
+              }
+              try {
+                await Database.bookSeriesModel.create({
+                  bookId: media.id,
+                  seriesId: newSeries.id,
+                  sequence: seriesObj.sequence
+                })
+                if (seriesCreated) {
+                  libraryScan.addLog(LogLevel.DEBUG, `Updating book "${bookMetadata.title}" added new series "${seriesObj.name}"${seriesObj.sequence ? ` with sequence "${seriesObj.sequence}"` : ''}`)
+                } else {
+                  libraryScan.addLog(LogLevel.DEBUG, `Updating book "${bookMetadata.title}" added series "${seriesObj.name}"${seriesObj.sequence ? ` with sequence "${seriesObj.sequence}"` : ''}`)
+                }
+              } catch (error) {
+                if (error.name !== 'SequelizeUniqueConstraintError') throw error
+              }
               seriesUpdated = true
             }
           } else if (seriesObj.sequence && existingBookSeries.bookSeries.sequence !== seriesObj.sequence) {
@@ -507,24 +540,38 @@ class BookScanner {
       }
     }
     if (bookMetadata.series.length) {
+      const linkedSeriesIds = new Set()
       for (const seriesObj of bookMetadata.series) {
         if (!seriesObj.name) continue
-        const matchingSeriesId = await Database.getSeriesIdByName(libraryItemData.libraryId, seriesObj.name)
-        if (matchingSeriesId) {
-          bookObject.bookSeries.push({
-            seriesId: matchingSeriesId,
-            sequence: seriesObj.sequence
-          })
-        } else {
-          bookObject.bookSeries.push({
-            sequence: seriesObj.sequence,
-            series: {
+        let seriesId = await Database.getSeriesIdByName(libraryItemData.libraryId, seriesObj.name)
+        if (!seriesId) {
+          let series
+          let seriesCreated = false
+          try {
+            series = await Database.seriesModel.create({
               name: seriesObj.name,
               nameIgnorePrefix: getTitleIgnorePrefix(seriesObj.name),
               libraryId: libraryItemData.libraryId
-            }
-          })
+            })
+            seriesCreated = true
+          } catch (error) {
+            if (error.name !== 'SequelizeUniqueConstraintError') throw error
+            series = await Database.seriesModel.findOne({
+              where: { name: seriesObj.name, libraryId: libraryItemData.libraryId }
+            })
+            if (!series) throw error
+          }
+          seriesId = series.id
+          if (seriesCreated) {
+            Database.addSeriesToFilterData(libraryItemData.libraryId, series.name, series.id)
+          }
         }
+        if (linkedSeriesIds.has(seriesId)) continue
+        linkedSeriesIds.add(seriesId)
+        bookObject.bookSeries.push({
+          seriesId,
+          sequence: seriesObj.sequence
+        })
       }
     }
 
@@ -703,8 +750,14 @@ class BookScanner {
       bookMetadata.description = htmlSanitizer.sanitize(bookMetadata.description)
     }
 
-    // One dedupe after every metadata source. A repeated name inserts the same bookAuthor twice and aborts the scan.
+    // One dedupe after every metadata source. A repeated name inserts the same bookAuthor or bookSeries twice and aborts the scan.
     bookMetadata.authors = parseNameString.dedupeNames(bookMetadata.authors)
+    const seenSeriesNames = new Set()
+    bookMetadata.series = bookMetadata.series.filter((series) => {
+      if (!series?.name || seenSeriesNames.has(series.name)) return false
+      seenSeriesNames.add(series.name)
+      return true
+    })
 
     return bookMetadata
   }

test/server/scanner/BookScanner.test.js

@@ -0,0 +1,123 @@
+const { expect } = require('chai')
+const { Sequelize } = require('sequelize')
+const sinon = require('sinon')
+
+const Database = require('../../../server/Database')
+const Logger = require('../../../server/Logger')
+const BookScanner = require('../../../server/scanner/BookScanner')
+const LibraryScan = require('../../../server/scanner/LibraryScan')
+
+describe('BookScanner.rescanExistingBookLibraryItem', () => {
+  /** @type {import('../../../server/models/Library')} */
+  let library
+  /** @type {import('../../../server/models/LibraryItem')} */
+  let libraryItem
+  /** @type {import('../../../server/models/Book')} */
+  let book
+  /** @type {LibraryScan} */
+  let libraryScan
+
+  const librarySettings = { audiobooksOnly: true }
+
+  beforeEach(async () => {
+    global.ServerSettings = {}
+    Database.sequelize = new Sequelize({ dialect: 'sqlite', storage: ':memory:', logging: false })
+    Database.sequelize.uppercaseFirst = (str) => (str ? `${str[0].toUpperCase()}${str.substr(1)}` : '')
+    await Database.buildModels()
+
+    sinon.stub(Logger, 'info')
+    sinon.stub(Logger, 'debug')
+    sinon.stub(Logger, 'warn')
+    sinon.stub(Logger, 'error')
+
+    library = await Database.libraryModel.create({ name: 'Test Library', mediaType: 'book' })
+    const libraryFolder = await Database.libraryFolderModel.create({ path: '/test', libraryId: library.id })
+    book = await Database.bookModel.create({ title: 'Test Book', audioFiles: [], tags: [], narrators: [], genres: [], chapters: [], coverPath: '/cover.jpg' })
+    libraryItem = await Database.libraryItemModel.create({
+      libraryFiles: [],
+      mediaId: book.id,
+      mediaType: 'book',
+      libraryId: library.id,
+      libraryFolderId: libraryFolder.id
+    })
+    libraryItem.isMissing = true
+    libraryItem.isFile = true
+    // Stale associations: the rows exist in the database, but this scan's loaded book does not see them
+    sinon.stub(libraryItem, 'getMedia').callsFake(async () => {
+      book.authors = []
+      book.series = []
+      book.audioFiles = []
+      book.coverPath = '/cover.jpg'
+      return book
+    })
+
+    libraryScan = new LibraryScan()
+    libraryScan.library = library
+  })
+
+  afterEach(async () => {
+    sinon.restore()
+    await Database.sequelize.sync({ force: true })
+  })
+
+  function scanData() {
+    return {
+      hasAudioFileChanges: false,
+      audioLibraryFiles: [],
+      imageLibraryFiles: [],
+      imageLibraryFilesRemoved: [],
+      imageLibraryFilesModified: [],
+      ebookLibraryFiles: [],
+      ebookLibraryFilesModified: [],
+      libraryId: library.id
+    }
+  }
+
+  it('does not throw when the bookSeries link already exists', async () => {
+    const series = await Database.seriesModel.create({ name: 'Crescent City', libraryId: library.id })
+    await Database.bookSeriesModel.create({ bookId: book.id, seriesId: series.id, sequence: '3' })
+    sinon.stub(BookScanner, 'getBookMetadataFromScanData').resolves({
+      title: 'Test Book',
+      authors: [],
+      series: [{ name: 'Crescent City', sequence: '3' }]
+    })
+
+    const result = await BookScanner.rescanExistingBookLibraryItem(libraryItem, scanData(), librarySettings, libraryScan)
+
+    expect(result.wasUpdated).to.be.true
+    const links = await Database.bookSeriesModel.findAll({ where: { bookId: book.id, seriesId: series.id } })
+    expect(links).to.have.lengthOf(1)
+  })
+
+  it('reuses an existing series when create hits the name unique constraint', async () => {
+    const series = await Database.seriesModel.create({ name: 'Crescent City', libraryId: library.id })
+    sinon.stub(Database, 'getSeriesIdByName').resolves(null)
+    sinon.stub(BookScanner, 'getBookMetadataFromScanData').resolves({
+      title: 'Test Book',
+      authors: [],
+      series: [{ name: 'Crescent City', sequence: '3' }]
+    })
+
+    await BookScanner.rescanExistingBookLibraryItem(libraryItem, scanData(), librarySettings, libraryScan)
+
+    const seriesRows = await Database.seriesModel.findAll({ where: { libraryId: library.id, name: 'Crescent City' } })
+    expect(seriesRows).to.have.lengthOf(1)
+    const links = await Database.bookSeriesModel.findAll({ where: { bookId: book.id, seriesId: series.id } })
+    expect(links).to.have.lengthOf(1)
+  })
+
+  it('does not throw when the bookAuthor link already exists', async () => {
+    const author = await Database.authorModel.create({ name: 'Sarah Maas', libraryId: library.id })
+    await Database.bookAuthorModel.create({ bookId: book.id, authorId: author.id })
+    sinon.stub(BookScanner, 'getBookMetadataFromScanData').resolves({
+      title: 'Test Book',
+      authors: ['Sarah Maas'],
+      series: []
+    })
+
+    await BookScanner.rescanExistingBookLibraryItem(libraryItem, scanData(), librarySettings, libraryScan)
+
+    const links = await Database.bookAuthorModel.findAll({ where: { bookId: book.id, authorId: author.id } })
+    expect(links).to.have.lengthOf(1)
+  })
+})