From c496db7c95752407164aa55d4dc6f35e207db9f1 Mon Sep 17 00:00:00 2001 From: advplyr Date: Sun, 1 Dec 2024 09:51:26 -0600 Subject: [PATCH] Fix:Remove authors with no books when a books is removed #3668 - Handles bulk delete, single delete, deleting library folder, and removing items with issues - Also handles bulk editing and removing authors --- server/controllers/LibraryController.js | 66 +++++++++- server/controllers/LibraryItemController.js | 127 +++++++++++++++----- server/routers/ApiRouter.js | 67 +++-------- 3 files changed, 177 insertions(+), 83 deletions(-) diff --git a/server/controllers/LibraryController.js b/server/controllers/LibraryController.js index 84d6193d..fc15488d 100644 --- a/server/controllers/LibraryController.js +++ b/server/controllers/LibraryController.js @@ -400,19 +400,48 @@ class LibraryController { model: Database.podcastEpisodeModel, attributes: ['id'] } + }, + { + model: Database.bookModel, + attributes: ['id'], + include: [ + { + model: Database.bookAuthorModel, + attributes: ['authorId'] + }, + { + model: Database.bookSeriesModel, + attributes: ['seriesId'] + } + ] } ] }) Logger.info(`[LibraryController] Removed folder "${folder.path}" from library "${req.library.name}" with ${libraryItemsInFolder.length} library items`) + const seriesIds = [] + const authorIds = [] for (const libraryItem of libraryItemsInFolder) { let mediaItemIds = [] if (req.library.isPodcast) { mediaItemIds = libraryItem.media.podcastEpisodes.map((pe) => pe.id) } else { mediaItemIds.push(libraryItem.mediaId) + if (libraryItem.media.bookAuthors.length) { + authorIds.push(...libraryItem.media.bookAuthors.map((ba) => ba.authorId)) + } + if (libraryItem.media.bookSeries.length) { + seriesIds.push(...libraryItem.media.bookSeries.map((bs) => bs.seriesId)) + } } Logger.info(`[LibraryController] Removing library item "${libraryItem.id}" from folder "${folder.path}"`) - await this.handleDeleteLibraryItem(libraryItem.mediaType, libraryItem.id, mediaItemIds) + await this.handleDeleteLibraryItem(libraryItem.id, mediaItemIds) + } + + if (authorIds.length) { + await this.checkRemoveAuthorsWithNoBooks(authorIds) + } + if (seriesIds.length) { + await this.checkRemoveEmptySeries(seriesIds) } // Remove folder @@ -501,7 +530,7 @@ class LibraryController { mediaItemIds.push(libraryItem.mediaId) } Logger.info(`[LibraryController] Removing library item "${libraryItem.id}" from library "${req.library.name}"`) - await this.handleDeleteLibraryItem(libraryItem.mediaType, libraryItem.id, mediaItemIds) + await this.handleDeleteLibraryItem(libraryItem.id, mediaItemIds) } // Set PlaybackSessions libraryId to null @@ -580,6 +609,8 @@ class LibraryController { * DELETE: /api/libraries/:id/issues * Remove all library items missing or invalid * + * @this {import('../routers/ApiRouter')} + * * @param {LibraryControllerRequest} req * @param {Response} res */ @@ -605,6 +636,20 @@ class LibraryController { model: Database.podcastEpisodeModel, attributes: ['id'] } + }, + { + model: Database.bookModel, + attributes: ['id'], + include: [ + { + model: Database.bookAuthorModel, + attributes: ['authorId'] + }, + { + model: Database.bookSeriesModel, + attributes: ['seriesId'] + } + ] } ] }) @@ -615,15 +660,30 @@ class LibraryController { } Logger.info(`[LibraryController] Removing ${libraryItemsWithIssues.length} items with issues`) + const authorIds = [] + const seriesIds = [] for (const libraryItem of libraryItemsWithIssues) { let mediaItemIds = [] if (req.library.isPodcast) { mediaItemIds = libraryItem.media.podcastEpisodes.map((pe) => pe.id) } else { mediaItemIds.push(libraryItem.mediaId) + if (libraryItem.media.bookAuthors.length) { + authorIds.push(...libraryItem.media.bookAuthors.map((ba) => ba.authorId)) + } + if (libraryItem.media.bookSeries.length) { + seriesIds.push(...libraryItem.media.bookSeries.map((bs) => bs.seriesId)) + } } Logger.info(`[LibraryController] Removing library item "${libraryItem.id}" with issue`) - await this.handleDeleteLibraryItem(libraryItem.mediaType, libraryItem.id, mediaItemIds) + await this.handleDeleteLibraryItem(libraryItem.id, mediaItemIds) + } + + if (authorIds.length) { + await this.checkRemoveAuthorsWithNoBooks(authorIds) + } + if (seriesIds.length) { + await this.checkRemoveEmptySeries(seriesIds) } // Set numIssues to 0 for library filter data diff --git a/server/controllers/LibraryItemController.js b/server/controllers/LibraryItemController.js index 64069ac5..92bc3833 100644 --- a/server/controllers/LibraryItemController.js +++ b/server/controllers/LibraryItemController.js @@ -96,6 +96,8 @@ class LibraryItemController { * Optional query params: * ?hard=1 * + * @this {import('../routers/ApiRouter')} + * * @param {RequestWithUser} req * @param {Response} res */ @@ -103,14 +105,36 @@ class LibraryItemController { const hardDelete = req.query.hard == 1 // Delete from file system const libraryItemPath = req.libraryItem.path - const mediaItemIds = req.libraryItem.mediaType === 'podcast' ? req.libraryItem.media.episodes.map((ep) => ep.id) : [req.libraryItem.media.id] - await this.handleDeleteLibraryItem(req.libraryItem.mediaType, req.libraryItem.id, mediaItemIds) + const mediaItemIds = [] + const authorIds = [] + const seriesIds = [] + if (req.libraryItem.isPodcast) { + mediaItemIds.push(...req.libraryItem.media.episodes.map((ep) => ep.id)) + } else { + mediaItemIds.push(req.libraryItem.media.id) + if (req.libraryItem.media.metadata.authors?.length) { + authorIds.push(...req.libraryItem.media.metadata.authors.map((au) => au.id)) + } + if (req.libraryItem.media.metadata.series?.length) { + seriesIds.push(...req.libraryItem.media.metadata.series.map((se) => se.id)) + } + } + + await this.handleDeleteLibraryItem(req.libraryItem.id, mediaItemIds) if (hardDelete) { Logger.info(`[LibraryItemController] Deleting library item from file system at "${libraryItemPath}"`) await fs.remove(libraryItemPath).catch((error) => { Logger.error(`[LibraryItemController] Failed to delete library item from file system at "${libraryItemPath}"`, error) }) } + + if (authorIds.length) { + await this.checkRemoveAuthorsWithNoBooks(authorIds) + } + if (seriesIds.length) { + await this.checkRemoveEmptySeries(seriesIds) + } + await Database.resetLibraryIssuesFilterData(req.libraryItem.libraryId) res.sendStatus(200) } @@ -212,15 +236,6 @@ class LibraryItemController { if (hasUpdates) { libraryItem.updatedAt = Date.now() - if (seriesRemoved.length) { - // Check remove empty series - Logger.debug(`[LibraryItemController] Series was removed from book. Check if series is now empty.`) - await this.checkRemoveEmptySeries( - libraryItem.media.id, - seriesRemoved.map((se) => se.id) - ) - } - if (isPodcastAutoDownloadUpdated) { this.cronManager.checkUpdatePodcastCron(libraryItem) } @@ -232,10 +247,12 @@ class LibraryItemController { if (authorsRemoved.length) { // Check remove empty authors Logger.debug(`[LibraryItemController] Authors were removed from book. Check if authors are now empty.`) - await this.checkRemoveAuthorsWithNoBooks( - libraryItem.libraryId, - authorsRemoved.map((au) => au.id) - ) + await this.checkRemoveAuthorsWithNoBooks(authorsRemoved.map((au) => au.id)) + } + if (seriesRemoved.length) { + // Check remove empty series + Logger.debug(`[LibraryItemController] Series were removed from book. Check if series are now empty.`) + await this.checkRemoveEmptySeries(seriesRemoved.map((se) => se.id)) } } res.json({ @@ -450,6 +467,8 @@ class LibraryItemController { * Optional query params: * ?hard=1 * + * @this {import('../routers/ApiRouter')} + * * @param {RequestWithUser} req * @param {Response} res */ @@ -477,14 +496,33 @@ class LibraryItemController { for (const libraryItem of itemsToDelete) { const libraryItemPath = libraryItem.path Logger.info(`[LibraryItemController] (${hardDelete ? 'Hard' : 'Soft'}) deleting Library Item "${libraryItem.media.metadata.title}" with id "${libraryItem.id}"`) - const mediaItemIds = libraryItem.mediaType === 'podcast' ? libraryItem.media.episodes.map((ep) => ep.id) : [libraryItem.media.id] - await this.handleDeleteLibraryItem(libraryItem.mediaType, libraryItem.id, mediaItemIds) + const mediaItemIds = [] + const seriesIds = [] + const authorIds = [] + if (libraryItem.isPodcast) { + mediaItemIds.push(...libraryItem.media.episodes.map((ep) => ep.id)) + } else { + mediaItemIds.push(libraryItem.media.id) + if (libraryItem.media.metadata.series?.length) { + seriesIds.push(...libraryItem.media.metadata.series.map((se) => se.id)) + } + if (libraryItem.media.metadata.authors?.length) { + authorIds.push(...libraryItem.media.metadata.authors.map((au) => au.id)) + } + } + await this.handleDeleteLibraryItem(libraryItem.id, mediaItemIds) if (hardDelete) { Logger.info(`[LibraryItemController] Deleting library item from file system at "${libraryItemPath}"`) await fs.remove(libraryItemPath).catch((error) => { Logger.error(`[LibraryItemController] Failed to delete library item from file system at "${libraryItemPath}"`, error) }) } + if (seriesIds.length) { + await this.checkRemoveEmptySeries(seriesIds) + } + if (authorIds.length) { + await this.checkRemoveAuthorsWithNoBooks(authorIds) + } } await Database.resetLibraryIssuesFilterData(libraryId) @@ -494,6 +532,8 @@ class LibraryItemController { /** * POST: /api/items/batch/update * + * @this {import('../routers/ApiRouter')} + * * @param {RequestWithUser} req * @param {Response} res */ @@ -503,39 +543,62 @@ class LibraryItemController { return res.sendStatus(500) } + // Ensure that each update payload has a unique library item id + const libraryItemIds = [...new Set(updatePayloads.map((up) => up.id))] + if (!libraryItemIds.length || libraryItemIds.length !== updatePayloads.length) { + Logger.error(`[LibraryItemController] Batch update failed. Each update payload must have a unique library item id`) + return res.sendStatus(400) + } + + // Get all library items to update + const libraryItems = await Database.libraryItemModel.getAllOldLibraryItems({ + id: libraryItemIds + }) + if (updatePayloads.length !== libraryItems.length) { + Logger.error(`[LibraryItemController] Batch update failed. Not all library items found`) + return res.sendStatus(404) + } + let itemsUpdated = 0 + const seriesIdsRemoved = [] + const authorIdsRemoved = [] + for (const updatePayload of updatePayloads) { const mediaPayload = updatePayload.mediaPayload - const libraryItem = await Database.libraryItemModel.getOldById(updatePayload.id) - if (!libraryItem) return null + const libraryItem = libraryItems.find((li) => li.id === updatePayload.id) await this.createAuthorsAndSeriesForItemUpdate(mediaPayload, libraryItem.libraryId) - let seriesRemoved = [] - if (libraryItem.isBook && mediaPayload.metadata?.series) { - const seriesIdsInUpdate = (mediaPayload.metadata?.series || []).map((se) => se.id) - seriesRemoved = libraryItem.media.metadata.series.filter((se) => !seriesIdsInUpdate.includes(se.id)) + if (libraryItem.isBook) { + if (Array.isArray(mediaPayload.metadata?.series)) { + const seriesIdsInUpdate = mediaPayload.metadata.series.map((se) => se.id) + const seriesRemoved = libraryItem.media.metadata.series.filter((se) => !seriesIdsInUpdate.includes(se.id)) + seriesIdsRemoved.push(...seriesRemoved.map((se) => se.id)) + } + if (Array.isArray(mediaPayload.metadata?.authors)) { + const authorIdsInUpdate = mediaPayload.metadata.authors.map((au) => au.id) + const authorsRemoved = libraryItem.media.metadata.authors.filter((au) => !authorIdsInUpdate.includes(au.id)) + authorIdsRemoved.push(...authorsRemoved.map((au) => au.id)) + } } if (libraryItem.media.update(mediaPayload)) { Logger.debug(`[LibraryItemController] Updated library item media ${libraryItem.media.metadata.title}`) - if (seriesRemoved.length) { - // Check remove empty series - Logger.debug(`[LibraryItemController] Series was removed from book. Check if series is now empty.`) - await this.checkRemoveEmptySeries( - libraryItem.media.id, - seriesRemoved.map((se) => se.id) - ) - } - await Database.updateLibraryItem(libraryItem) SocketAuthority.emitter('item_updated', libraryItem.toJSONExpanded()) itemsUpdated++ } } + if (seriesIdsRemoved.length) { + await this.checkRemoveEmptySeries(seriesIdsRemoved) + } + if (authorIdsRemoved.length) { + await this.checkRemoveAuthorsWithNoBooks(authorIdsRemoved) + } + res.json({ success: true, updates: itemsUpdated diff --git a/server/routers/ApiRouter.js b/server/routers/ApiRouter.js index 7f21c3ac..0657b389 100644 --- a/server/routers/ApiRouter.js +++ b/server/routers/ApiRouter.js @@ -348,11 +348,10 @@ class ApiRouter { // /** * Remove library item and associated entities - * @param {string} mediaType * @param {string} libraryItemId * @param {string[]} mediaItemIds array of bookId or podcastEpisodeId */ - async handleDeleteLibraryItem(mediaType, libraryItemId, mediaItemIds) { + async handleDeleteLibraryItem(libraryItemId, mediaItemIds) { const numProgressRemoved = await Database.mediaProgressModel.destroy({ where: { mediaItemId: mediaItemIds @@ -362,29 +361,6 @@ class ApiRouter { Logger.info(`[ApiRouter] Removed ${numProgressRemoved} media progress entries for library item "${libraryItemId}"`) } - // TODO: Remove open sessions for library item - - // Remove series if empty - if (mediaType === 'book') { - // TODO: update filter data - const bookSeries = await Database.bookSeriesModel.findAll({ - where: { - bookId: mediaItemIds[0] - }, - include: { - model: Database.seriesModel, - include: { - model: Database.bookModel - } - } - }) - for (const bs of bookSeries) { - if (bs.series.books.length === 1) { - await this.removeEmptySeries(bs.series) - } - } - } - // remove item from playlists const playlistsWithItem = await Database.playlistModel.getPlaylistsForMediaItemIds(mediaItemIds) for (const playlist of playlistsWithItem) { @@ -423,6 +399,7 @@ class ApiRouter { // purge cover cache await CacheManager.purgeCoverCache(libraryItemId) + // Remove metadata file if in /metadata/items dir const itemMetadataPath = Path.join(global.MetadataPath, 'items', libraryItemId) if (await fs.pathExists(itemMetadataPath)) { Logger.info(`[ApiRouter] Removing item metadata at "${itemMetadataPath}"`) @@ -437,32 +414,27 @@ class ApiRouter { } /** - * Used when a series is removed from a book - * Series is removed if it only has 1 book + * After deleting book(s), remove empty series * - * @param {string} bookId * @param {string[]} seriesIds */ - async checkRemoveEmptySeries(bookId, seriesIds) { + async checkRemoveEmptySeries(seriesIds) { if (!seriesIds?.length) return - const bookSeries = await Database.bookSeriesModel.findAll({ + const series = await Database.seriesModel.findAll({ where: { - bookId, - seriesId: seriesIds + id: seriesIds }, - include: [ - { - model: Database.seriesModel, - include: { - model: Database.bookModel - } - } - ] + attributes: ['id', 'name', 'libraryId'], + include: { + model: Database.bookModel, + attributes: ['id'] + } }) - for (const bs of bookSeries) { - if (bs.series.books.length === 1) { - await this.removeEmptySeries(bs.series) + + for (const s of series) { + if (!s.books.length) { + await this.removeEmptySeries(s) } } } @@ -471,11 +443,10 @@ class ApiRouter { * Remove authors with no books and unset asin, description and imagePath * Note: Other implementation is in BookScanner.checkAuthorsRemovedFromBooks (can be merged) * - * @param {string} libraryId * @param {string[]} authorIds * @returns {Promise} */ - async checkRemoveAuthorsWithNoBooks(libraryId, authorIds) { + async checkRemoveAuthorsWithNoBooks(authorIds) { if (!authorIds?.length) return const bookAuthorsToRemove = ( @@ -495,10 +466,10 @@ class ApiRouter { }, sequelize.where(sequelize.literal('(SELECT count(*) FROM bookAuthors ba WHERE ba.authorId = author.id)'), 0) ], - attributes: ['id', 'name'], + attributes: ['id', 'name', 'libraryId'], raw: true }) - ).map((au) => ({ id: au.id, name: au.name })) + ).map((au) => ({ id: au.id, name: au.name, libraryId: au.libraryId })) if (bookAuthorsToRemove.length) { await Database.authorModel.destroy({ @@ -506,7 +477,7 @@ class ApiRouter { id: bookAuthorsToRemove.map((au) => au.id) } }) - bookAuthorsToRemove.forEach(({ id, name }) => { + bookAuthorsToRemove.forEach(({ id, name, libraryId }) => { Database.removeAuthorFromFilterData(libraryId, id) // TODO: Clients were expecting full author in payload but its unnecessary SocketAuthority.emitter('author_removed', { id, libraryId })