Merge pull request #5691 from mikiher/dedupe-authors

Dedupe repeated book authors

server/models/Book.js

@@ -439,6 +439,7 @@ class Book extends Model {
    */
   async updateAuthorsFromRequest(authors, libraryId) {
     if (!Array.isArray(authors)) return null
+    authors = parseNameString.dedupeNames(authors)
 
     if (!this.authors) {
       throw new Error(`[Book] Cannot update authors because authors are not loaded for book ${this.id}`)
@@ -466,6 +467,8 @@ class Book extends Model {
     const authorsAdded = []
     for (const authorName of newAuthorNames) {
       const { author, created } = await authorModel.findOrCreateByNameAndLibrary(authorName, libraryId)
+      // findOrCreate matches case-insensitively, so a second spelling can resolve to an author just linked
+      if (this.authors.some((au) => au.id === author.id)) continue
       await bookAuthorModel.create({ bookId: this.id, authorId: author.id })
       if (created) {
         SocketAuthority.emitter('author_added', author.toOldJSON())

server/scanner/BookScanner.js

@@ -703,6 +703,9 @@ 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.
+    bookMetadata.authors = parseNameString.dedupeNames(bookMetadata.authors)
+
     return bookMetadata
   }
 

server/scanner/LibraryScan.js

@@ -29,6 +29,9 @@ class LibraryScan {
     /** @type {string[]} */
     this.seriesRemovedFromBooks = []
 
+    /** @type {string} item label while a library item is being scanned, so a thrown error can name it */
+    this.failureItem = null
+
     this.logs = []
   }
 

server/scanner/LibraryScanner.js

@@ -116,7 +116,8 @@ class LibraryScanner {
     } catch (err) {
       libraryScan.setComplete()
 
-      Logger.error(`[LibraryScanner] Library scan ${libraryScan.id} failed after ${libraryScan.elapsedTimestamp} | ${libraryScan.resultStats}.`, err)
+      const failedItem = libraryScan.failureItem ? ` while scanning ${libraryScan.failureItem}` : ''
+      Logger.error(`[LibraryScanner] Library scan ${libraryScan.id} failed after ${libraryScan.elapsedTimestamp} | ${libraryScan.resultStats}${failedItem}.`, err)
 
       task.data.scanResults = libraryScan.scanResults
       const taskFailedString = {
@@ -169,6 +170,7 @@ class LibraryScanner {
     const libraryItemIdsMissing = []
     let libraryItemsUpdated = []
     for (const existingLibraryItem of existingLibraryItems) {
+      libraryScan.failureItem = scanItemFailureLabel(existingLibraryItem.title, existingLibraryItem.path)
       // First try to find matching library item with exact file path
       let libraryItemData = libraryItemDataFound.find((lid) => lid.path === existingLibraryItem.path)
       if (!libraryItemData) {
@@ -221,6 +223,8 @@ class LibraryScanner {
         }
       }
 
+      libraryScan.failureItem = null
+
       // Emit item updates in chunks of 10 to client
       if (libraryItemsUpdated.length === 10) {
         SocketAuthority.libraryItemsEmitter('items_updated', libraryItemsUpdated)
@@ -259,12 +263,14 @@ class LibraryScanner {
     if (libraryItemDataFound.length) {
       let newLibraryItems = []
       for (const libraryItemData of libraryItemDataFound) {
+        libraryScan.failureItem = scanItemFailureLabel(libraryItemData.mediaMetadata?.title, libraryItemData.path)
         const newLibraryItem = await LibraryItemScanner.scanNewLibraryItem(libraryItemData, libraryScan.library.settings, libraryScan)
         if (newLibraryItem) {
           newLibraryItems.push(newLibraryItem)
 
           libraryScan.resultsAdded++
         }
+        libraryScan.failureItem = null
 
         // Emit new items in chunks of 10 to client
         if (newLibraryItems.length === 10) {
@@ -597,7 +603,12 @@ class LibraryScanner {
         }
         // Scan library item for updates
         Logger.debug(`[LibraryScanner] Folder update for relative path "${itemDir}" is in library item "${existingLibraryItem.media.title}" with id "${existingLibraryItem.id}" - scan for updates`)
-        itemGroupingResults[itemDir] = await LibraryItemScanner.scanLibraryItem(existingLibraryItem.id, updatedLibraryItemDetails)
+        try {
+          itemGroupingResults[itemDir] = await LibraryItemScanner.scanLibraryItem(existingLibraryItem.id, updatedLibraryItemDetails)
+        } catch (err) {
+          Logger.error(`[LibraryScanner] Failed while scanning ${scanItemFailureLabel(existingLibraryItem.media?.title, existingLibraryItem.path)}`)
+          throw err
+        }
         continue
       } else if (library.settings.audiobooksOnly && !hasAudioFiles(fileUpdateGroup, itemDir)) {
         Logger.debug(`[LibraryScanner] Folder update for relative path "${itemDir}" has no audio files`)
@@ -626,7 +637,13 @@ class LibraryScanner {
 
       Logger.debug(`[LibraryScanner] Folder update group must be a new item "${itemDir}" in library "${library.name}"`)
       const isSingleMediaItem = isSingleMediaFile(fileUpdateGroup, itemDir)
-      const newLibraryItem = await LibraryItemScanner.scanPotentialNewLibraryItem(fullPath, library, folder, isSingleMediaItem)
+      let newLibraryItem
+      try {
+        newLibraryItem = await LibraryItemScanner.scanPotentialNewLibraryItem(fullPath, library, folder, isSingleMediaItem)
+      } catch (err) {
+        Logger.error(`[LibraryScanner] Failed while scanning ${scanItemFailureLabel(null, fullPath)}`)
+        throw err
+      }
       if (newLibraryItem) {
         SocketAuthority.libraryItemEmitter('item_added', newLibraryItem)
       }
@@ -638,6 +655,16 @@ class LibraryScanner {
 }
 module.exports = new LibraryScanner()
 
+/**
+ * @param {string} title
+ * @param {string} path
+ * @returns {string}
+ */
+function scanItemFailureLabel(title, path) {
+  if (title && path) return `"${title}" at "${path}"`
+  return `"${path || title || 'unknown item'}"`
+}
+
 function ItemToFileInoMatch(libraryItem1, libraryItem2) {
   return libraryItem1.isFile && libraryItem2.libraryFiles.some((lf) => lf.ino === libraryItem1.ino)
 }

server/utils/generators/abmetadataGenerator.js

@@ -1,5 +1,6 @@
 const Logger = require('../../Logger')
 const parseSeriesString = require('../parsers/parseSeriesString')
+const { dedupeNames } = require('../parsers/parseNameString')
 
 const mediaTypeKeys = {
   book: {
@@ -140,8 +141,7 @@ function validateMetadataValue(key, value, expectedType) {
       return undefined
     }
 
-    const cleanedArray = value.filter((t) => typeof t === 'string')
-    return [...new Set(cleanedArray.map((t) => t.trim()).filter((t) => t))]
+    return dedupeNames(value)
   }
 
   Logger.warn(`[abmetadataGenerator] Unknown expected type "${expectedType}" for key "${key}"`)

server/utils/parsers/parseNameString.js

@@ -35,6 +35,28 @@ module.exports.nameToLastFirst = (firstLast) => {
   return `${nameObj.last_name}, ${nameObj.first_name}`
 }
 
+/**
+ * Trim names, drop blanks, and drop exact repeats. The first spelling is kept.
+ * Callers that build bookAuthors rows should use this instead of deduping per metadata source.
+ *
+ * @param {string[]} names
+ * @returns {string[]}
+ */
+function dedupeNames(names) {
+  if (!Array.isArray(names)) return []
+  const seen = new Set()
+  const deduped = []
+  for (const name of names) {
+    if (typeof name !== 'string') continue
+    const trimmed = name.trim()
+    if (!trimmed || seen.has(trimmed)) continue
+    seen.add(trimmed)
+    deduped.push(trimmed)
+  }
+  return deduped
+}
+module.exports.dedupeNames = dedupeNames
+
 /**
  * Parses a name string into an array of names
  *
@@ -62,7 +84,7 @@ module.exports.parse = (nameString) => {
   // If names are in Chinese，Japanese and Korean languages, return as is.
   if (/[\u4e00-\u9fff\u3040-\u30ff\u31f0-\u31ff]/.test(splitNames[0])) {
     return {
-      names: splitNames
+      names: dedupeNames(splitNames)
     }
   }
 
@@ -102,7 +124,7 @@ module.exports.parse = (nameString) => {
   names = names.filter((n) => n.first_name || n.last_name)
 
   // Set name strings and remove duplicates
-  const namesArray = [...new Set(names.map((a) => (a.first_name ? `${a.first_name} ${a.last_name}` : a.last_name)))]
+  const namesArray = dedupeNames(names.map((a) => (a.first_name ? `${a.first_name} ${a.last_name}` : a.last_name)))
 
   return {
     names: namesArray // Array of first last

test/server/utils/parsers/parseNameString.test.js

@@ -1,6 +1,6 @@
 const chai = require('chai')
 const expect = chai.expect
-const { parse, nameToLastFirst } = require('../../../../server/utils/parsers/parseNameString')
+const { parse, nameToLastFirst, dedupeNames } = require('../../../../server/utils/parsers/parseNameString')
 
 describe('parseNameString', () => {
   describe('parse', () => {
@@ -80,6 +80,20 @@ describe('parseNameString', () => {
     })
   })
 
+  describe('dedupeNames', () => {
+    it('trims, drops blanks, and keeps the first spelling of an exact repeat', () => {
+      expect(dedupeNames(['  Stephen King ', 'Stephen King', '', '   ', 'stephen king'])).to.deep.equal(['Stephen King', 'stephen king'])
+    })
+
+    it('preserves order and ignores non-strings', () => {
+      expect(dedupeNames(['Amy Yao', null, 'Amy Yao', 3, 'Raymond Chandler'])).to.deep.equal(['Amy Yao', 'Raymond Chandler'])
+    })
+
+    it('returns an empty array for non-arrays', () => {
+      expect(dedupeNames(null)).to.deep.equal([])
+    })
+  })
+
   describe('nameToLastFirst', () => {
     it('converts First Last to Last, First format', () => {
       const result = nameToLastFirst('John Smith')