diff --git a/server/controllers/MeController.js b/server/controllers/MeController.js index ca600d373..03841b5ca 100644 --- a/server/controllers/MeController.js +++ b/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) { diff --git a/server/models/Book.js b/server/models/Book.js index df2014c4e..0446b3d90 100644 --- a/server/models/Book.js +++ b/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}"`) } } diff --git a/server/models/Series.js b/server/models/Series.js index 6ca288464..6ca10a9ba 100644 --- a/server/models/Series.js +++ b/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 + } } /** diff --git a/server/models/User.js b/server/models/User.js index ac7fde061..7a0a1f9f9 100644 --- a/server/models/User.js +++ b/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 diff --git a/server/scanner/BookScanner.js b/server/scanner/BookScanner.js index f8762a1cf..30ac1ff1a 100644 --- a/server/scanner/BookScanner.js +++ b/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 } diff --git a/test/server/models/Book.test.js b/test/server/models/Book.test.js new file mode 100644 index 000000000..991291cf2 --- /dev/null +++ b/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) + }) +}) diff --git a/test/server/scanner/BookScanner.test.js b/test/server/scanner/BookScanner.test.js new file mode 100644 index 000000000..13de9cc4f --- /dev/null +++ b/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) + }) +})