Update remove book from collection endpoint to require Can Update permission instead of Can Delete

Fix book edit crash when a series or book link already exists

.github/workflows/docker-build-v3.yml

@@ -0,0 +1,102 @@
+# Builds the v3 test image
+#
+# Moves the :v3 tag on Docker Hub and GHCR, the same way master
+# moves :edge. Using the server's v3 branch and react-client master.
+#
+# repository_dispatch (build-v3) is for triggering a build on updates to react-client
+
+name: Build and Push v3 Docker Image
+
+on:
+  workflow_dispatch:
+  push:
+    branches: [v3]
+  repository_dispatch:
+    types: [build-v3]
+
+jobs:
+  build:
+    if: ${{ github.repository == 'advplyr/audiobookshelf' && (github.event_name != 'push' || !contains(github.event.head_commit.message, 'skip ci')) }}
+    runs-on: ubuntu-24.04
+
+    steps:
+      - name: Check out server
+        uses: actions/checkout@v4
+        with:
+          ref: ${{ github.event_name == 'repository_dispatch' && 'v3' || github.sha }}
+
+      - name: Check out React client
+        uses: actions/checkout@v4
+        with:
+          repository: audiobookshelf/audiobookshelf-client-react
+          ref: master
+          path: client-react
+
+      - name: Server sha
+        id: server-sha
+        run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
+
+      - name: Client sha
+        id: client-sha
+        working-directory: client-react
+        run: echo "sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"
+
+      - name: Docker meta
+        id: meta
+        uses: docker/metadata-action@v5
+        with:
+          images: advplyr/audiobookshelf,ghcr.io/${{ github.repository_owner }}/audiobookshelf
+          flavor: |
+            latest=false
+          tags: |
+            type=raw,value=v3
+          labels: |
+            audiobookshelf.server.sha=${{ steps.server-sha.outputs.sha }}
+            audiobookshelf.client.sha=${{ steps.client-sha.outputs.sha }}
+
+      - name: Setup QEMU
+        uses: docker/setup-qemu-action@v3
+
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v3
+
+      - name: Cache Docker layers
+        uses: actions/cache@v4
+        with:
+          path: /tmp/.buildx-cache
+          key: ${{ runner.os }}-buildx-v3-${{ github.sha }}
+          restore-keys: |
+            ${{ runner.os }}-buildx-v3-
+
+      - name: Login to Dockerhub
+        uses: docker/login-action@v3
+        with:
+          username: ${{ secrets.DOCKERHUB_USERNAME }}
+          password: ${{ secrets.DOCKERHUB_PASSWORD }}
+
+      - name: Login to ghcr
+        uses: docker/login-action@v3
+        with:
+          registry: ghcr.io
+          username: ${{ github.repository_owner }}
+          password: ${{ secrets.GHCR_PASSWORD }}
+
+      - name: Build image
+        uses: docker/build-push-action@v6
+        with:
+          file: client-react/Dockerfile
+          context: .
+          build-contexts: |
+            abs-client=client-react
+            abs-server=.
+          tags: ${{ steps.meta.outputs.tags }}
+          labels: ${{ steps.meta.outputs.labels }}
+          platforms: linux/amd64,linux/arm64
+          push: true
+          cache-from: type=local,src=/tmp/.buildx-cache
+          cache-to: type=local,dest=/tmp/.buildx-cache-new,mode=max
+
+      - name: Move cache
+        run: |
+          rm -rf /tmp/.buildx-cache
+          mv /tmp/.buildx-cache-new /tmp/.buildx-cache

client/components/tables/collection/BookTableRow.vue

@@ -42,7 +42,7 @@
         <div v-if="userCanUpdate" class="mx-1" :class="isHovering ? '' : 'ml-6'">
           <ui-icon-btn icon="edit" borderless @click="clickEdit" />
         </div>
-        <div v-if="userCanDelete" class="mx-1">
+        <div v-if="userCanUpdate" class="mx-1">
           <ui-icon-btn icon="close" borderless @click="removeClick" />
         </div>
       </div>
@@ -79,8 +79,7 @@ export default {
   },
   computed: {
     translateDistance() {
-      if (!this.userCanUpdate && !this.userCanDelete) return 'translate-x-0'
-      else if (!this.userCanUpdate || !this.userCanDelete) return '-translate-x-12'
+      if (!this.userCanUpdate) return 'translate-x-0'
       return '-translate-x-24'
     },
     media() {
@@ -144,9 +143,6 @@ export default {
     },
     userCanUpdate() {
       return this.$store.getters['user/getUserCanUpdate']
-    },
-    userCanDelete() {
-      return this.$store.getters['user/getUserCanDelete']
     }
   },
   methods: {

server/Server.js

@@ -4,7 +4,6 @@ const express = require('express')
 const http = require('http')
 const util = require('util')
 const fs = require('./libs/fsExtra')
-const fileUpload = require('./libs/expressFileupload')
 const cookieParser = require('cookie-parser')
 const axios = require('axios')
 
@@ -13,6 +12,7 @@ const { version } = require('../package.json')
 // Utils
 const is = require('./libs/requestIp/isJs')
 const fileUtils = require('./utils/fileUtils')
+const { sweepUploadTempFiles } = require('./utils/fileUploadMiddleware')
 const { toNumber } = require('./utils/index')
 const { getRequestOrigin } = require('./utils/requestUtils')
 const Logger = require('./Logger')
@@ -199,6 +199,7 @@ class Server {
 
     await this.cleanUserData() // Remove invalid user item progress
     await CacheManager.ensureCachePaths()
+    await sweepUploadTempFiles()
 
     await ShareManager.init()
     await this.backupManager.init()
@@ -336,16 +337,6 @@ class Server {
 
     this.server = http.createServer(app)
 
-    // Skip file upload parsing for internal-api routes (Next.js proxies read multipart bodies).
-    router.use(
-      /^(?!\/internal-api).*/,
-      fileUpload({
-        defCharset: 'utf8',
-        defParamCharset: 'utf8',
-        useTempFiles: true,
-        tempFileDir: Path.join(global.MetadataPath, 'tmp')
-      })
-    )
     router.use(express.urlencoded({ extended: true, limit: '5mb' }))
 
     // Skip JSON parsing for internal-api routes

server/auth/LocalAuthStrategy.js

@@ -49,6 +49,12 @@ class LocalAuthStrategy {
    * @param {Function} done - Passport callback
    */
   async verifyCredentials(req, username, password, done) {
+    if (typeof username !== 'string' || typeof password !== 'string') {
+      this.logFailedLoginAttempt(req, typeof username === 'string' ? username : '[non-string]', 'Invalid username or password')
+      done(null, null)
+      return
+    }
+
     // Load the user given it's username
     const user = await Database.userModel.getUserByUsername(username.toLowerCase())
 

server/auth/OidcAuthStrategy.js

@@ -393,7 +393,15 @@ class OidcAuthStrategy {
    * @returns {string|null}
    */
   getEndSessionUrl(req, idToken, authMethod) {
-    const client = this.getClient()
+    // Logout trusts the auth_method cookie. getClient() throws when OpenID is not
+    // configured, and that throw is outside the request promise, so it exits the process.
+    let client
+    try {
+      client = this.getClient()
+    } catch (error) {
+      Logger.error(`[OidcAuth] Cannot build end session URL - ${error.message}`)
+      return null
+    }
 
     if (client.issuer.end_session_endpoint && client.issuer.end_session_endpoint.length > 0) {
       let postLogoutRedirectUri = null

server/controllers/CollectionController.js

@@ -96,12 +96,8 @@ class CollectionController {
       return res.status(500).send('Failed to create collection')
     }
 
-    // Load books expanded
-    newCollection.books = await newCollection.getBooksExpandedWithLibraryItem()
-
-    // Note: The old collection model stores expanded libraryItems in the books property
-    const jsonExpanded = newCollection.toOldJSONExpanded()
-    SocketAuthority.emitter('collection_added', jsonExpanded)
+    const jsonExpanded = await newCollection.getOldJsonExpanded(req.user)
+    if (jsonExpanded) SocketAuthority.clientEmitter(req.user.id, 'collection_added', jsonExpanded)
     res.json(jsonExpanded)
   }
 
@@ -196,9 +192,9 @@ class CollectionController {
       }
     }
 
-    const jsonExpanded = await req.collection.getOldJsonExpanded()
-    if (wasUpdated) {
-      SocketAuthority.emitter('collection_updated', jsonExpanded)
+    const jsonExpanded = await req.collection.getOldJsonExpanded(req.user)
+    if (wasUpdated && jsonExpanded) {
+      SocketAuthority.clientEmitter(req.user.id, 'collection_updated', jsonExpanded)
     }
     res.json(jsonExpanded)
   }
@@ -212,14 +208,17 @@ class CollectionController {
    * @param {Response} res
    */
   async delete(req, res) {
-    const jsonExpanded = await req.collection.getOldJsonExpanded()
+    const jsonExpanded = await req.collection.getOldJsonExpanded(req.user)
 
     // Close rss feed - remove from db and emit socket event
     await RssFeedManager.closeFeedForEntityId(req.collection.id)
 
+    if (jsonExpanded) {
+      SocketAuthority.clientEmitter(req.user.id, 'collection_removed', jsonExpanded)
+    }
+
     await req.collection.destroy()
 
-    SocketAuthority.emitter('collection_removed', jsonExpanded)
     res.sendStatus(200)
   }
 
@@ -254,8 +253,8 @@ class CollectionController {
       bookId: libraryItem.mediaId,
       order: collectionBooks.length + 1
     })
-    const jsonExpanded = await req.collection.getOldJsonExpanded()
-    SocketAuthority.emitter('collection_updated', jsonExpanded)
+    const jsonExpanded = await req.collection.getOldJsonExpanded(req.user)
+    if (jsonExpanded) SocketAuthority.clientEmitter(req.user.id, 'collection_updated', jsonExpanded)
     res.json(jsonExpanded)
   }
 
@@ -299,10 +298,10 @@ class CollectionController {
         order++
       }
 
-      jsonExpanded = await req.collection.getOldJsonExpanded()
-      SocketAuthority.emitter('collection_updated', jsonExpanded)
+      jsonExpanded = await req.collection.getOldJsonExpanded(req.user)
+      if (jsonExpanded) SocketAuthority.clientEmitter(req.user.id, 'collection_updated', jsonExpanded)
     } else {
-      jsonExpanded = await req.collection.getOldJsonExpanded()
+      jsonExpanded = await req.collection.getOldJsonExpanded(req.user)
     }
     res.json(jsonExpanded)
   }
@@ -361,10 +360,10 @@ class CollectionController {
     if (hasUpdated) {
       await Database.collectionBookModel.bulkCreate(collectionBooksToAdd)
 
-      jsonExpanded = await req.collection.getOldJsonExpanded()
-      SocketAuthority.emitter('collection_updated', jsonExpanded)
+      jsonExpanded = await req.collection.getOldJsonExpanded(req.user)
+      if (jsonExpanded) SocketAuthority.clientEmitter(req.user.id, 'collection_updated', jsonExpanded)
     } else {
-      jsonExpanded = await req.collection.getOldJsonExpanded()
+      jsonExpanded = await req.collection.getOldJsonExpanded(req.user)
     }
     res.json(jsonExpanded)
   }
@@ -417,9 +416,9 @@ class CollectionController {
       order++
     }
 
-    let jsonExpanded = await req.collection.getOldJsonExpanded()
-    if (hasUpdated) {
-      SocketAuthority.emitter('collection_updated', jsonExpanded)
+    let jsonExpanded = await req.collection.getOldJsonExpanded(req.user)
+    if (hasUpdated && jsonExpanded) {
+      SocketAuthority.clientEmitter(req.user.id, 'collection_updated', jsonExpanded)
     }
     res.json(jsonExpanded)
   }
@@ -447,7 +446,7 @@ class CollectionController {
     if (req.method == 'DELETE' && !req.params.bookId && !req.user.canDelete) {
       Logger.warn(`[CollectionController] User "${req.user.username}" attempted to delete without permission`)
       return res.sendStatus(403)
-    } else if ((req.method == 'PATCH' || req.method == 'POST') && !req.user.canUpdate) {
+    } else if ((req.method == 'PATCH' || req.method == 'POST' || (req.method == 'DELETE' && req.params.bookId)) && !req.user.canUpdate) {
       Logger.warn(`[CollectionController] User "${req.user.username}" attempted to update without permission`)
       return res.sendStatus(403)
     }

server/controllers/MeController.js

@@ -495,7 +495,7 @@ class MeController {
     for (const mediaProgress of mediaProgressesInProgress) {
       const oldMediaProgress = mediaProgress.getOldMediaProgress()
       const libraryItem = libraryItems.find((li) => li.id === oldMediaProgress.libraryItemId)
-      if (libraryItem) {
+      if (libraryItem && req.user.checkCanAccessLibraryItem(libraryItem)) {
         if (oldMediaProgress.episodeId && libraryItem.isPodcast) {
           const episode = libraryItem.media.podcastEpisodes.find((ep) => ep.id === oldMediaProgress.episodeId)
           if (episode) {

server/controllers/SessionController.js

@@ -361,6 +361,11 @@ class SessionController {
       return res.sendStatus(404)
     }
 
+    if (playbackSession.userId !== req.user.id && !req.user.isAdminOrUp) {
+      Logger.error(`[SessionController] Non-admin user "${req.user.username}" attempted to access session belonging to another user "${req.params.id}"`)
+      return res.sendStatus(403)
+    }
+
     if (req.method == 'DELETE' && !req.user.canDelete) {
       Logger.warn(`[SessionController] User "${req.user.username}" attempted to delete without permission`)
       return res.sendStatus(403)

server/libs/expressFileupload/processMultipart.js

@@ -26,6 +26,18 @@ const waitFlushProperty = Symbol('wait flush property symbol');
  */
 module.exports = (options, req, res, next) => {
   req.files = null;
+  const pendingCleanups = [];
+  const runPendingCleanups = () => {
+    while (pendingCleanups.length) {
+      const cleanupFn = pendingCleanups.pop();
+      try {
+        cleanupFn();
+      } catch (err) {
+        debugLog(options, `Temp file cleanup failed: ${err}`);
+      }
+    }
+  };
+  res.once('close', runPendingCleanups);
 
   // Build busboy options and init busboy instance.
   const busboyOptions = buildOptions(options, { headers: req.headers });
@@ -62,6 +74,7 @@ module.exports = (options, req, res, next) => {
     } = options.useTempFiles
         ? tempFileHandler(options, field, filename) // Upload into temporary file.
         : memHandler(options, field, filename);     // Upload into RAM.
+    pendingCleanups.push(cleanup);
 
     const writePromise = options.useTempFiles
       ? getWritePromise().catch(err => {
@@ -161,6 +174,7 @@ module.exports = (options, req, res, next) => {
 
   busboy.on('error', (err) => {
     debugLog(options, `Busboy error`);
+    runPendingCleanups();
     next(err);
   });
 

server/libs/expressFileupload/tempFileHandler.js

@@ -28,6 +28,18 @@ module.exports = (options, fieldname, filename) => {
       reject(err);
     });
   });
+  // cleanup() can run from an error path and again on response close.
+  let cleanedUp = false;
+
+  // Windows cannot unlink while the write stream still holds the file.
+  const closeWriteStream = () => new Promise((resolve) => {
+    if (writeStream.destroyed || writeStream.closed) {
+      return resolve();
+    }
+    writeStream.once('close', resolve);
+    writeStream.once('error', resolve);
+    writeStream.end();
+  });
 
   return {
     dataHandler: (data) => {
@@ -51,13 +63,21 @@ module.exports = (options, fieldname, filename) => {
       return Buffer.concat([]);
     },
     cleanup: () => {
+      if (cleanedUp) return Promise.resolve();
+      cleanedUp = true;
       completed = true;
       debugLog(options, `Cleaning up temporary file ${tempFilePath}...`);
-      writeStream.end();
-      deleteFile(tempFilePath, err => (err 
-        ? debugLog(options, `Cleaning up temporary file ${tempFilePath} failed: ${err}`)
-        : debugLog(options, `Cleaning up temporary file ${tempFilePath} done.`)
-      ));
+      return closeWriteStream().then(() => new Promise((resolve) => {
+        // ENOENT: handler already mv()'d the file; that is success.
+        deleteFile(tempFilePath, err => {
+          if (err && err.code !== 'ENOENT') {
+            debugLog(options, `Cleaning up temporary file ${tempFilePath} failed: ${err}`);
+          } else {
+            debugLog(options, `Cleaning up temporary file ${tempFilePath} done.`);
+          }
+          resolve();
+        });
+      }));
     },
     getWritePromise: () => writePromise
   };

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,7 +467,15 @@ class Book extends Model {
     const authorsAdded = []
     for (const authorName of newAuthorNames) {
       const { author, created } = await authorModel.findOrCreateByNameAndLibrary(authorName, libraryId)
-      await bookAuthorModel.create({ bookId: this.id, authorId: author.id })
+      // 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
+      try {
+        await bookAuthorModel.create({ bookId: this.id, authorId: author.id })
+      } catch (error) {
+        if (error.name !== 'SequelizeUniqueConstraintError') throw error
+        this.authors.push(author)
+        continue
+      }
       if (created) {
         SocketAuthority.emitter('author_added', author.toOldJSON())
       } else {
@@ -522,11 +531,23 @@ class Book extends Model {
         }
       } else {
         const series = await seriesModel.findOrCreateByNameAndLibrary(seriesObj.name, libraryId)
-        series.bookSeries = await bookSeriesModel.create({ bookId: this.id, seriesId: series.id, sequence: seriesObjSequence })
+        try {
+          series.bookSeries = await bookSeriesModel.create({ bookId: this.id, seriesId: series.id, sequence: seriesObjSequence })
+          seriesAdded.push(series)
+          hasUpdates = true
+          Logger.debug(`[Book] "${this.title}" Added series "${series.name}"`)
+        } catch (error) {
+          if (error.name !== 'SequelizeUniqueConstraintError') throw error
+          series.bookSeries = await bookSeriesModel.findOne({ where: { bookId: this.id, seriesId: series.id } })
+          if (!series.bookSeries) throw error
+          if (series.bookSeries.sequence !== seriesObjSequence) {
+            series.bookSeries.sequence = seriesObjSequence
+            await series.bookSeries.save()
+            hasUpdates = true
+            Logger.debug(`[Book] "${this.title}" Updated series "${series.name}" sequence ${seriesObjSequence}`)
+          }
+        }
         this.series.push(series)
-        seriesAdded.push(series)
-        hasUpdates = true
-        Logger.debug(`[Book] "${this.title}" Added series "${series.name}"`)
       }
     }
 

server/models/Series.js

@@ -75,11 +75,21 @@ class Series extends Model {
   static async findOrCreateByNameAndLibrary(seriesName, libraryId) {
     const series = await this.getByNameAndLibrary(seriesName, libraryId)
     if (series) return series
-    return this.create({
-      name: seriesName,
-      nameIgnorePrefix: getTitleIgnorePrefix(seriesName),
-      libraryId
-    })
+    try {
+      return await this.create({
+        name: seriesName,
+        nameIgnorePrefix: getTitleIgnorePrefix(seriesName),
+        libraryId
+      })
+    } catch (error) {
+      if (error.name !== 'SequelizeUniqueConstraintError') throw error
+      // The unique index is on the exact name. lower() can miss that row.
+      const existingSeries = await this.findOne({
+        where: { name: seriesName, libraryId }
+      })
+      if (!existingSeries) throw error
+      return existingSeries
+    }
   }
 
   /**

server/models/User.js

@@ -743,10 +743,10 @@ class User extends Model {
           },
           {
             model: this.sequelize.models.podcast,
-            attributes: ['id', 'title'],
+            attributes: ['id', 'title', 'explicit', 'tags'],
             include: {
               model: this.sequelize.models.libraryItem,
-              attributes: ['id']
+              attributes: ['id', 'libraryId', 'mediaType']
             }
           }
         ]
@@ -758,15 +758,38 @@ class User extends Model {
           statusCode: 404
         }
       }
+      const episodeLibraryItem = podcastEpisode.podcast?.libraryItem
+      if (!episodeLibraryItem) {
+        Logger.error(`[User] createUpdateMediaProgress: library item for episode ${progressPayload.episodeId} not found`)
+        return {
+          error: 'Library item not found',
+          statusCode: 404
+        }
+      }
+      if (progressPayload.libraryItemId && episodeLibraryItem.id !== progressPayload.libraryItemId) {
+        Logger.error(`[User] createUpdateMediaProgress: episode ${progressPayload.episodeId} does not belong to library item ${progressPayload.libraryItemId}`)
+        return {
+          error: 'Episode does not belong to library item',
+          statusCode: 400
+        }
+      }
+      episodeLibraryItem.media = podcastEpisode.podcast
+      if (!this.checkCanAccessLibraryItem(episodeLibraryItem)) {
+        Logger.error(`[User] createUpdateMediaProgress: user "${this.username}" attempted to update progress for inaccessible library item "${episodeLibraryItem.id}"`)
+        return {
+          error: 'User does not have access to this library item',
+          statusCode: 403
+        }
+      }
       mediaItemId = podcastEpisode.id
       mediaProgress = podcastEpisode.mediaProgresses?.[0]
       podcastId = podcastEpisode.podcastId
     } else {
       const libraryItem = await this.sequelize.models.libraryItem.findByPk(progressPayload.libraryItemId, {
-        attributes: ['id', 'mediaId', 'mediaType'],
+        attributes: ['id', 'libraryId', 'mediaId', 'mediaType'],
         include: {
           model: this.sequelize.models.book,
-          attributes: ['id', 'title'],
+          attributes: ['id', 'title', 'explicit', 'tags'],
           required: false,
           include: {
             model: this.sequelize.models.mediaProgress,
@@ -787,6 +810,12 @@ class User extends Model {
           error: 'Library item is not a book',
           statusCode: 400
         }
+      } else if (!this.checkCanAccessLibraryItem(libraryItem)) {
+        Logger.error(`[User] createUpdateMediaProgress: user "${this.username}" attempted to update progress for inaccessible library item "${libraryItem.id}"`)
+        return {
+          error: 'User does not have access to this library item',
+          statusCode: 403
+        }
       }
 
       mediaItemId = libraryItem.media.id

server/providers/iTunes.js

@@ -27,6 +27,55 @@ const htmlSanitizer = require('../utils/htmlSanitizer')
  * @property {boolean} explicit
  */
 
+/**
+ * Split a list of authors on commas. A piece with no space is a credential and is glued onto the previous name.
+ *
+ * @param {string} text
+ * @returns {string[]}
+ */
+function parseAuthorPieces(text) {
+  const authors = []
+  for (const piece of (text || '').split(',')) {
+    const name = piece.trim()
+    if (!name) continue
+    if (!name.includes(' ') && authors.length) {
+      authors[authors.length - 1] = `${authors[authors.length - 1]} ${name}`
+    } else {
+      authors.push(name)
+    }
+  }
+  return authors
+}
+
+function stripCommasFromName(name) {
+  return (name || '').replace(/,/g, '').replace(/\s+/g, ' ').trim()
+}
+
+/**
+ * Apple lists authors as "Name1, Name2 & Name3", but a comma can also sit inside one name ("Rick Hanson, PhD").
+ * A comma-separated piece with no space is treated as a credential and glued onto the previous name.
+ * The text after the last " & " is a single author, so commas there are removed.
+ * @see https://github.com/advplyr/audiobookshelf/issues/1022
+ * @see https://github.com/advplyr/audiobookshelf/issues/2661
+ *
+ * @param {string} artistName
+ * @returns {string}
+ */
+function cleanAuthor(artistName) {
+  const raw = (artistName || '').trim()
+  if (!raw) return ''
+
+  const lastAmpIndex = raw.lastIndexOf(' & ')
+  if (lastAmpIndex === -1) {
+    return stripCommasFromName(raw)
+  }
+
+  const authors = parseAuthorPieces(raw.slice(0, lastAmpIndex))
+  const lastAuthor = stripCommasFromName(raw.slice(lastAmpIndex + 3))
+  if (lastAuthor) authors.push(lastAuthor)
+  return authors.join(', ')
+}
+
 class iTunes {
   #responseTimeout = 10000
 
@@ -103,15 +152,11 @@ class iTunes {
   }
 
   cleanAudiobook(data) {
-    // artistName can be "Name1, Name2 & Name3" so we refactor this to "Name1, Name2, Name3"
-    //  see: https://github.com/advplyr/audiobookshelf/issues/1022
-    const author = (data.artistName || '').split(' & ').join(', ')
-
     return {
       id: data.collectionId,
       artistId: data.artistId,
       title: data.collectionName,
-      author,
+      author: cleanAuthor(data.artistName),
       description: data.description || null,
       publishedYear: data.releaseDate ? data.releaseDate.split('-')[0] : null,
       genres: data.primaryGenreName ? [data.primaryGenreName] : null,

server/routers/ApiRouter.js

@@ -11,6 +11,7 @@ const date = require('../libs/dateAndTime')
 
 const CacheManager = require('../managers/CacheManager')
 const RssFeedManager = require('../managers/RssFeedManager')
+const { fileUploadParser, requireUploadPermission } = require('../utils/fileUploadMiddleware')
 
 const LibraryController = require('../controllers/LibraryController')
 const UserController = require('../controllers/UserController')
@@ -110,7 +111,7 @@ class ApiRouter {
     this.router.get('/items/:id/download', LibraryItemController.middleware.bind(this), LibraryItemController.download.bind(this))
     this.router.patch('/items/:id/media', LibraryItemController.middleware.bind(this), LibraryItemController.updateMedia.bind(this))
     this.router.get('/items/:id/cover', LibraryItemController.getCover.bind(this))
-    this.router.post('/items/:id/cover', LibraryItemController.middleware.bind(this), LibraryItemController.uploadCover.bind(this))
+    this.router.post('/items/:id/cover', LibraryItemController.middleware.bind(this), requireUploadPermission, fileUploadParser, LibraryItemController.uploadCover.bind(this))
     this.router.patch('/items/:id/cover', LibraryItemController.middleware.bind(this), LibraryItemController.updateCover.bind(this))
     this.router.delete('/items/:id/cover', LibraryItemController.middleware.bind(this), LibraryItemController.removeCover.bind(this))
     this.router.post('/items/:id/match', LibraryItemController.middleware.bind(this), LibraryItemController.match.bind(this))
@@ -202,7 +203,7 @@ class ApiRouter {
     this.router.delete('/backups/:id', BackupController.middleware.bind(this), BackupController.delete.bind(this))
     this.router.get('/backups/:id/download', BackupController.middleware.bind(this), BackupController.download.bind(this))
     this.router.get('/backups/:id/apply', BackupController.middleware.bind(this), BackupController.apply.bind(this))
-    this.router.post('/backups/upload', BackupController.middleware.bind(this), BackupController.upload.bind(this))
+    this.router.post('/backups/upload', BackupController.middleware.bind(this), fileUploadParser, BackupController.upload.bind(this))
     this.router.patch('/backups/path', BackupController.middleware.bind(this), BackupController.updatePath.bind(this))
 
     //
@@ -343,7 +344,7 @@ class ApiRouter {
     //
     // Misc Routes
     //
-    this.router.post('/upload', MiscController.handleUpload.bind(this))
+    this.router.post('/upload', requireUploadPermission, fileUploadParser, MiscController.handleUpload.bind(this))
     this.router.get('/tasks', MiscController.getTasks.bind(this))
     this.router.patch('/settings', MiscController.updateServerSettings.bind(this))
     this.router.patch('/sorting-prefixes', MiscController.updateSortingPrefixes.bind(this))

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,6 +750,15 @@ class BookScanner {
       bookMetadata.description = htmlSanitizer.sanitize(bookMetadata.description)
     }
 
+    // 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
   }
 

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/fileUploadMiddleware.js

@@ -0,0 +1,76 @@
+const Path = require('path')
+const fs = require('../libs/fsExtra')
+const fileUpload = require('../libs/expressFileupload')
+const Logger = require('../Logger')
+
+/**
+ * Multipart parser for the three routes that read req.files.
+ * Reads MetadataPath per request so route registration does not depend on it.
+ *
+ * @param {import('express').Request} req
+ * @param {import('express').Response} res
+ * @param {import('express').NextFunction} next
+ */
+function fileUploadParser(req, res, next) {
+  fileUpload({
+    defCharset: 'utf8',
+    defParamCharset: 'utf8',
+    useTempFiles: true,
+    tempFileDir: Path.join(global.MetadataPath, 'tmp')
+  })(req, res, next)
+}
+module.exports.fileUploadParser = fileUploadParser
+
+/**
+ * Reject users without upload permission before the multipart parser writes temp files.
+ *
+ * @param {import('express').Request} req
+ * @param {import('express').Response} res
+ * @param {import('express').NextFunction} next
+ */
+function requireUploadPermission(req, res, next) {
+  if (!req.user?.canUpload) {
+    Logger.warn(`User "${req.user?.username || 'unknown'}" attempted to upload without permission`)
+    return res.sendStatus(403)
+  }
+  next()
+}
+module.exports.requireUploadPermission = requireUploadPermission
+
+/**
+ * Remove leftover upload temp files (tmp-*) from metadata/tmp.
+ * Comic extraction uses the same directory with other names; those stay.
+ *
+ * @returns {Promise<void>}
+ */
+async function sweepUploadTempFiles() {
+  const tmpDir = Path.join(global.MetadataPath, 'tmp')
+  let names
+  try {
+    names = await fs.readdir(tmpDir)
+  } catch (error) {
+    if (error.code === 'ENOENT') return
+    Logger.warn(`[FileUpload] Failed to read upload temp directory "${tmpDir}": ${error.message}`)
+    return
+  }
+
+  let removed = 0
+  for (const name of names) {
+    if (!name.startsWith('tmp-')) continue
+    const fullPath = Path.join(tmpDir, name)
+    try {
+      const stat = await fs.lstat(fullPath)
+      if (!stat.isFile()) continue
+      await fs.unlink(fullPath)
+      removed++
+    } catch (error) {
+      if (error.code === 'ENOENT') continue
+      Logger.warn(`[FileUpload] Failed to remove leftover upload temp file "${fullPath}": ${error.message}`)
+    }
+  }
+
+  if (removed) {
+    Logger.info(`[FileUpload] Removed ${removed} leftover upload temp file(s) from "${tmpDir}"`)
+  }
+}
+module.exports.sweepUploadTempFiles = sweepUploadTempFiles

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/models/Book.test.js

@@ -0,0 +1,71 @@
+const { expect } = require('chai')
+const { Sequelize } = require('sequelize')
+const sinon = require('sinon')
+
+const Database = require('../../../server/Database')
+const Logger = require('../../../server/Logger')
+
+describe('Book author and series updates', () => {
+  /** @type {import('../../../server/models/Library')} */
+  let library
+  /** @type {import('../../../server/models/Book')} */
+  let book
+
+  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' })
+    book = await Database.bookModel.create({ title: 'Test Book', audioFiles: [], tags: [], narrators: [], genres: [], chapters: [] })
+    book.authors = []
+    book.series = []
+  })
+
+  afterEach(async () => {
+    sinon.restore()
+    await Database.sequelize.sync({ force: true })
+  })
+
+  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' })
+
+    const result = await book.updateSeriesFromRequest([{ name: 'Crescent City', sequence: '3' }], library.id)
+
+    expect(result.hasUpdates).to.be.false
+    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.seriesModel, 'getByNameAndLibrary').resolves(null)
+
+    const result = await book.updateSeriesFromRequest([{ name: 'Crescent City', sequence: '3' }], library.id)
+
+    expect(result.seriesAdded).to.have.lengthOf(1)
+    expect(result.seriesAdded[0].id).to.equal(series.id)
+    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 })
+
+    const result = await book.updateAuthorsFromRequest(['Sarah Maas'], library.id)
+
+    expect(result.authorsAdded).to.have.lengthOf(0)
+    const links = await Database.bookAuthorModel.findAll({ where: { bookId: book.id, authorId: author.id } })
+    expect(links).to.have.lengthOf(1)
+  })
+})

test/server/providers/iTunes.test.js

@@ -0,0 +1,53 @@
+const iTunes = require('../../../server/providers/iTunes')
+const { expect } = require('chai')
+
+describe('iTunes', () => {
+  let itunes
+
+  beforeEach(() => {
+    itunes = new iTunes()
+  })
+
+  describe('cleanAudiobook', () => {
+    function authorFrom(artistName) {
+      return itunes.cleanAudiobook({ artistName }).author
+    }
+
+    it('should return an empty string if artistName is missing', () => {
+      expect(authorFrom()).to.equal('')
+      expect(authorFrom('')).to.equal('')
+    })
+
+    it('should keep a single author without commas', () => {
+      expect(authorFrom('Joy Fielding')).to.equal('Joy Fielding')
+    })
+
+    it('should strip commas from a single author with a credential', () => {
+      expect(authorFrom('Rick Hanson, PhD')).to.equal('Rick Hanson PhD')
+    })
+
+    it('should split two authors joined by an ampersand', () => {
+      expect(authorFrom('Name1 & Name2')).to.equal('Name1, Name2')
+    })
+
+    it('should split three authors using commas and an ampersand', () => {
+      expect(authorFrom('Keltie Knight, Becca Tobin & Jac Vanek')).to.equal('Keltie Knight, Becca Tobin, Jac Vanek')
+    })
+
+    it('should glue a one-word credential onto the previous author before the ampersand', () => {
+      expect(authorFrom('Rick Hanson, PhD & Jane Doe')).to.equal('Rick Hanson PhD, Jane Doe')
+    })
+
+    it('should glue several one-word credentials in a row', () => {
+      expect(authorFrom('Rick Hanson, PhD, MD & Jane Doe')).to.equal('Rick Hanson PhD MD, Jane Doe')
+    })
+
+    it('should strip commas from the last author', () => {
+      expect(authorFrom('Rick Hanson & Jane Doe, PhD')).to.equal('Rick Hanson, Jane Doe PhD')
+    })
+
+    it('should glue credentials before the ampersand and strip commas from the last author', () => {
+      expect(authorFrom('Rick Hanson, PhD & Jane Doe, PhD')).to.equal('Rick Hanson PhD, Jane Doe PhD')
+    })
+  })
+})

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)
+  })
+})

test/server/utils/fileUploadMiddleware.test.js

@@ -0,0 +1,221 @@
+const { expect } = require('chai')
+const express = require('express')
+const fs = require('fs')
+const http = require('http')
+const os = require('os')
+const path = require('path')
+const sinon = require('sinon')
+
+const Logger = require('../../../server/Logger')
+const { fileUploadParser, requireUploadPermission, sweepUploadTempFiles } = require('../../../server/utils/fileUploadMiddleware')
+
+function listTmpFiles(dir) {
+  if (!fs.existsSync(dir)) return []
+  return fs.readdirSync(dir).filter((name) => name.startsWith('tmp-'))
+}
+
+function sendMultipart(port, urlPath, extraHeaders = {}) {
+  return new Promise((resolve, reject) => {
+    const boundary = '----AbsUploadTestBoundary'
+    const payload = [
+      `--${boundary}`,
+      'Content-Disposition: form-data; name="f"; filename="probe.txt"',
+      'Content-Type: text/plain',
+      '',
+      'probe',
+      `--${boundary}--`,
+      ''
+    ].join('\r\n')
+
+    const req = http.request(
+      {
+        hostname: '127.0.0.1',
+        port,
+        path: urlPath,
+        method: 'POST',
+        headers: {
+          'Content-Type': `multipart/form-data; boundary=${boundary}`,
+          'Content-Length': Buffer.byteLength(payload),
+          ...extraHeaders
+        }
+      },
+      (res) => {
+        res.resume()
+        res.on('end', () => resolve(res.statusCode))
+      }
+    )
+    req.on('error', reject)
+    req.write(payload)
+    req.end()
+  })
+}
+
+function listen(app) {
+  const server = http.createServer(app)
+  return new Promise((resolve) => {
+    server.listen(0, '127.0.0.1', () => {
+      resolve({ server, port: server.address().port })
+    })
+  })
+}
+
+function closeServer(server) {
+  return new Promise((resolve, reject) => {
+    server.close((err) => (err ? reject(err) : resolve()))
+  })
+}
+
+async function waitFor(predicate, timeoutMs = 2000) {
+  const started = Date.now()
+  while (Date.now() - started < timeoutMs) {
+    if (predicate()) return
+    await new Promise((resolve) => setTimeout(resolve, 25))
+  }
+  throw new Error('Timed out waiting for condition')
+}
+
+describe('fileUploadMiddleware', () => {
+  let metadataDir
+  let tmpDir
+  let previousMetadataPath
+
+  beforeEach(() => {
+    previousMetadataPath = global.MetadataPath
+    metadataDir = fs.mkdtempSync(path.join(os.tmpdir(), 'abs-upload-meta-'))
+    tmpDir = path.join(metadataDir, 'tmp')
+    global.MetadataPath = metadataDir
+    sinon.stub(Logger, 'info')
+    sinon.stub(Logger, 'warn')
+  })
+
+  afterEach(() => {
+    sinon.restore()
+    global.MetadataPath = previousMetadataPath
+    fs.rmSync(metadataDir, { recursive: true, force: true })
+  })
+
+  describe('requireUploadPermission', () => {
+    it('returns 403 when the user cannot upload', () => {
+      const req = { user: { canUpload: false, username: 'guest' } }
+      const res = { sendStatus: sinon.stub() }
+      const next = sinon.stub()
+
+      requireUploadPermission(req, res, next)
+
+      expect(res.sendStatus.calledOnceWithExactly(403)).to.equal(true)
+      expect(next.called).to.equal(false)
+    })
+
+    it('calls next when the user can upload', () => {
+      const req = { user: { canUpload: true, username: 'uploader' } }
+      const res = { sendStatus: sinon.stub() }
+      const next = sinon.stub()
+
+      requireUploadPermission(req, res, next)
+
+      expect(next.calledOnce).to.equal(true)
+      expect(res.sendStatus.called).to.equal(false)
+    })
+  })
+
+  describe('sweepUploadTempFiles', () => {
+    it('removes leftover tmp-* files and leaves other names', async () => {
+      fs.mkdirSync(tmpDir, { recursive: true })
+      fs.writeFileSync(path.join(tmpDir, 'tmp-1-123'), 'stale')
+      fs.writeFileSync(path.join(tmpDir, 'tmp-2-456'), 'stale')
+      fs.writeFileSync(path.join(tmpDir, 'comic-extract-keep'), 'keep')
+      fs.mkdirSync(path.join(tmpDir, 'tmp-not-a-file'))
+
+      await sweepUploadTempFiles()
+
+      expect(fs.existsSync(path.join(tmpDir, 'tmp-1-123'))).to.equal(false)
+      expect(fs.existsSync(path.join(tmpDir, 'tmp-2-456'))).to.equal(false)
+      expect(fs.existsSync(path.join(tmpDir, 'comic-extract-keep'))).to.equal(true)
+      expect(fs.existsSync(path.join(tmpDir, 'tmp-not-a-file'))).to.equal(true)
+    })
+
+    it('returns when the temp directory does not exist', async () => {
+      await sweepUploadTempFiles()
+      expect(fs.existsSync(tmpDir)).to.equal(false)
+    })
+  })
+
+  describe('upload parser mounting', () => {
+    it('does not write tmp-* files for a route without the parser', async () => {
+      const app = express()
+      app.use((req, res) => res.sendStatus(404))
+      const { server, port } = await listen(app)
+
+      try {
+        const status = await sendMultipart(port, '/not-a-route')
+        expect(status).to.equal(404)
+        expect(listTmpFiles(tmpDir)).to.deep.equal([])
+      } finally {
+        await closeServer(server)
+      }
+    })
+
+    it('does not write tmp-* files when permission is rejected before parsing', async () => {
+      const app = express()
+      app.use((req, res, next) => {
+        req.user = { canUpload: false, username: 'guest' }
+        next()
+      })
+      app.post('/upload', requireUploadPermission, fileUploadParser, (req, res) => {
+        res.sendStatus(200)
+      })
+      const { server, port } = await listen(app)
+
+      try {
+        const status = await sendMultipart(port, '/upload')
+        expect(status).to.equal(403)
+        expect(listTmpFiles(tmpDir)).to.deep.equal([])
+      } finally {
+        await closeServer(server)
+      }
+    })
+
+    it('removes the temp file when the handler rejects without moving it', async () => {
+      const app = express()
+      app.use((req, res, next) => {
+        req.user = { canUpload: true, username: 'uploader' }
+        next()
+      })
+      app.post('/upload', requireUploadPermission, fileUploadParser, (req, res) => {
+        res.status(400).send('Invalid request no file or url')
+      })
+      const { server, port } = await listen(app)
+
+      try {
+        const status = await sendMultipart(port, '/upload')
+        expect(status).to.equal(400)
+        await waitFor(() => listTmpFiles(tmpDir).length === 0)
+      } finally {
+        await closeServer(server)
+      }
+    })
+
+    it('keeps a moved file and leaves no tmp-* leftover', async () => {
+      const dest = path.join(metadataDir, 'moved.txt')
+      const app = express()
+      app.use((req, res, next) => {
+        req.user = { canUpload: true, username: 'uploader' }
+        next()
+      })
+      app.post('/upload', requireUploadPermission, fileUploadParser, async (req, res) => {
+        await req.files.f.mv(dest)
+        res.sendStatus(200)
+      })
+      const { server, port } = await listen(app)
+
+      try {
+        const status = await sendMultipart(port, '/upload')
+        expect(status).to.equal(200)
+        expect(fs.readFileSync(dest, 'utf8')).to.equal('probe')
+        await waitFor(() => listTmpFiles(tmpDir).length === 0)
+      } finally {
+        await closeServer(server)
+      }
+    })
+  })
+})

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')