From 2b5484243b649ed2dac3c996eb1d9b8e907b4c4a Mon Sep 17 00:00:00 2001 From: advplyr Date: Sun, 1 Dec 2024 12:44:21 -0600 Subject: [PATCH] Add LibraryItemController test for delete/batchDelete/updateMedia endpoint functions to correctly remove authors & series with no books --- server/managers/CacheManager.js | 1 + server/objects/LibraryItem.js | 2 +- server/routers/ApiRouter.js | 10 +- .../controllers/LibraryItemController.test.js | 202 ++++++++++++++++++ 4 files changed, 210 insertions(+), 5 deletions(-) create mode 100644 test/server/controllers/LibraryItemController.test.js diff --git a/server/managers/CacheManager.js b/server/managers/CacheManager.js index f0375691..b44b65de 100644 --- a/server/managers/CacheManager.js +++ b/server/managers/CacheManager.js @@ -86,6 +86,7 @@ class CacheManager { } async purgeEntityCache(entityId, cachePath) { + if (!entityId || !cachePath) return [] return Promise.all( (await fs.readdir(cachePath)).reduce((promises, file) => { if (file.startsWith(entityId)) { diff --git a/server/objects/LibraryItem.js b/server/objects/LibraryItem.js index 0259ee4c..84a37897 100644 --- a/server/objects/LibraryItem.js +++ b/server/objects/LibraryItem.js @@ -262,7 +262,7 @@ class LibraryItem { * @returns {Promise} null if not saved */ async saveMetadata() { - if (this.isSavingMetadata) return null + if (this.isSavingMetadata || !global.MetadataPath) return null this.isSavingMetadata = true diff --git a/server/routers/ApiRouter.js b/server/routers/ApiRouter.js index 0657b389..a92796e8 100644 --- a/server/routers/ApiRouter.js +++ b/server/routers/ApiRouter.js @@ -400,10 +400,12 @@ class ApiRouter { 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}"`) - await fs.remove(itemMetadataPath) + if (global.MetadataPath) { + const itemMetadataPath = Path.join(global.MetadataPath, 'items', libraryItemId) + if (await fs.pathExists(itemMetadataPath)) { + Logger.info(`[ApiRouter] Removing item metadata at "${itemMetadataPath}"`) + await fs.remove(itemMetadataPath) + } } await Database.libraryItemModel.removeById(libraryItemId) diff --git a/test/server/controllers/LibraryItemController.test.js b/test/server/controllers/LibraryItemController.test.js new file mode 100644 index 00000000..3e7c58b2 --- /dev/null +++ b/test/server/controllers/LibraryItemController.test.js @@ -0,0 +1,202 @@ +const { expect } = require('chai') +const { Sequelize } = require('sequelize') +const sinon = require('sinon') + +const Database = require('../../../server/Database') +const ApiRouter = require('../../../server/routers/ApiRouter') +const LibraryItemController = require('../../../server/controllers/LibraryItemController') +const ApiCacheManager = require('../../../server/managers/ApiCacheManager') +const RssFeedManager = require('../../../server/managers/RssFeedManager') +const Logger = require('../../../server/Logger') + +describe('LibraryItemController', () => { + /** @type {ApiRouter} */ + let apiRouter + + 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() + + apiRouter = new ApiRouter({ + apiCacheManager: new ApiCacheManager(), + rssFeedManager: new RssFeedManager() + }) + + sinon.stub(Logger, 'info') + }) + + afterEach(async () => { + sinon.restore() + + // Clear all tables + await Database.sequelize.sync({ force: true }) + }) + + describe('checkRemoveAuthorsAndSeries', () => { + let libraryItem1Id + let libraryItem2Id + let author1Id + let author2Id + let author3Id + let series1Id + let series2Id + + beforeEach(async () => { + const newLibrary = await Database.libraryModel.create({ name: 'Test Library', mediaType: 'book' }) + const newLibraryFolder = await Database.libraryFolderModel.create({ path: '/test', libraryId: newLibrary.id }) + + const newBook = await Database.bookModel.create({ title: 'Test Book', audioFiles: [], tags: [], narrators: [], genres: [], chapters: [] }) + const newLibraryItem = await Database.libraryItemModel.create({ libraryFiles: [], mediaId: newBook.id, mediaType: 'book', libraryId: newLibrary.id, libraryFolderId: newLibraryFolder.id }) + libraryItem1Id = newLibraryItem.id + + const newBook2 = await Database.bookModel.create({ title: 'Test Book 2', audioFiles: [], tags: [], narrators: [], genres: [], chapters: [] }) + const newLibraryItem2 = await Database.libraryItemModel.create({ libraryFiles: [], mediaId: newBook2.id, mediaType: 'book', libraryId: newLibrary.id, libraryFolderId: newLibraryFolder.id }) + libraryItem2Id = newLibraryItem2.id + + const newAuthor = await Database.authorModel.create({ name: 'Test Author', libraryId: newLibrary.id }) + author1Id = newAuthor.id + const newAuthor2 = await Database.authorModel.create({ name: 'Test Author 2', libraryId: newLibrary.id }) + author2Id = newAuthor2.id + const newAuthor3 = await Database.authorModel.create({ name: 'Test Author 3', imagePath: '/fake/path/author.png', libraryId: newLibrary.id }) + author3Id = newAuthor3.id + + // Book 1 has Author 1, Author 2 and Author 3 + await Database.bookAuthorModel.create({ bookId: newBook.id, authorId: newAuthor.id }) + await Database.bookAuthorModel.create({ bookId: newBook.id, authorId: newAuthor2.id }) + await Database.bookAuthorModel.create({ bookId: newBook.id, authorId: newAuthor3.id }) + + // Book 2 has Author 2 + await Database.bookAuthorModel.create({ bookId: newBook2.id, authorId: newAuthor2.id }) + + const newSeries = await Database.seriesModel.create({ name: 'Test Series', libraryId: newLibrary.id }) + series1Id = newSeries.id + const newSeries2 = await Database.seriesModel.create({ name: 'Test Series 2', libraryId: newLibrary.id }) + series2Id = newSeries2.id + + // Book 1 is in Series 1 and Series 2 + await Database.bookSeriesModel.create({ bookId: newBook.id, seriesId: newSeries.id }) + await Database.bookSeriesModel.create({ bookId: newBook.id, seriesId: newSeries2.id }) + + // Book 2 is in Series 2 + await Database.bookSeriesModel.create({ bookId: newBook2.id, seriesId: newSeries2.id }) + }) + + it('should remove authors and series with no books on library item delete', async () => { + const oldLibraryItem = await Database.libraryItemModel.getOldById(libraryItem1Id) + + const fakeReq = { + query: {}, + libraryItem: oldLibraryItem + } + const fakeRes = { + sendStatus: sinon.spy() + } + await LibraryItemController.delete.bind(apiRouter)(fakeReq, fakeRes) + + expect(fakeRes.sendStatus.calledWith(200)).to.be.true + + // Author 1 should be removed because it has no books + const author1Exists = await Database.authorModel.checkExistsById(author1Id) + expect(author1Exists).to.be.false + + // Author 2 should not be removed because it still has Book 2 + const author2Exists = await Database.authorModel.checkExistsById(author2Id) + expect(author2Exists).to.be.true + + // Author 3 should not be removed because it has an image + const author3Exists = await Database.authorModel.checkExistsById(author3Id) + expect(author3Exists).to.be.true + + // Series 1 should be removed because it has no books + const series1Exists = await Database.seriesModel.checkExistsById(series1Id) + expect(series1Exists).to.be.false + + // Series 2 should not be removed because it still has Book 2 + const series2Exists = await Database.seriesModel.checkExistsById(series2Id) + expect(series2Exists).to.be.true + }) + + it('should remove authors and series with no books on library item batch delete', async () => { + // Batch delete library item 1 + const fakeReq = { + query: {}, + user: { + canDelete: true + }, + body: { + libraryItemIds: [libraryItem1Id] + } + } + const fakeRes = { + sendStatus: sinon.spy() + } + await LibraryItemController.batchDelete.bind(apiRouter)(fakeReq, fakeRes) + + expect(fakeRes.sendStatus.calledWith(200)).to.be.true + + // Author 1 should be removed because it has no books + const author1Exists = await Database.authorModel.checkExistsById(author1Id) + expect(author1Exists).to.be.false + + // Author 2 should not be removed because it still has Book 2 + const author2Exists = await Database.authorModel.checkExistsById(author2Id) + expect(author2Exists).to.be.true + + // Author 3 should not be removed because it has an image + const author3Exists = await Database.authorModel.checkExistsById(author3Id) + expect(author3Exists).to.be.true + + // Series 1 should be removed because it has no books + const series1Exists = await Database.seriesModel.checkExistsById(series1Id) + expect(series1Exists).to.be.false + + // Series 2 should not be removed because it still has Book 2 + const series2Exists = await Database.seriesModel.checkExistsById(series2Id) + expect(series2Exists).to.be.true + }) + + it('should remove authors and series with no books on library item update media', async () => { + const oldLibraryItem = await Database.libraryItemModel.getOldById(libraryItem1Id) + + // Update library item 1 remove all authors and series + const fakeReq = { + query: {}, + body: { + metadata: { + authors: [], + series: [] + } + }, + libraryItem: oldLibraryItem + } + const fakeRes = { + json: sinon.spy() + } + await LibraryItemController.updateMedia.bind(apiRouter)(fakeReq, fakeRes) + + expect(fakeRes.json.calledOnce).to.be.true + + // Author 1 should be removed because it has no books + const author1Exists = await Database.authorModel.checkExistsById(author1Id) + expect(author1Exists).to.be.false + + // Author 2 should not be removed because it still has Book 2 + const author2Exists = await Database.authorModel.checkExistsById(author2Id) + expect(author2Exists).to.be.true + + // Author 3 should not be removed because it has an image + const author3Exists = await Database.authorModel.checkExistsById(author3Id) + expect(author3Exists).to.be.true + + // Series 1 should be removed because it has no books + const series1Exists = await Database.seriesModel.checkExistsById(series1Id) + expect(series1Exists).to.be.false + + // Series 2 should not be removed because it still has Book 2 + const series2Exists = await Database.seriesModel.checkExistsById(series2Id) + expect(series2Exists).to.be.true + }) + }) +})