Merge branch 'master' into v3

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

@@ -469,7 +469,13 @@ class Book extends Model {
       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 })
+      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 {
@@ -525,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

@@ -728,10 +728,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']
             }
           }
         ]
@@ -743,15 +743,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,
@@ -772,6 +795,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/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/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/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)
+  })
+})