Add LibraryItemController test for delete/batchDelete/updateMedia endpoint functions to correctly remove authors & series with no books

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

server/objects/LibraryItem.js

@@ -262,7 +262,7 @@ class LibraryItem {
    * @returns {Promise<LibraryFile>} null if not saved
    */
   async saveMetadata() {
-    if (this.isSavingMetadata) return null
+    if (this.isSavingMetadata || !global.MetadataPath) return null
 
     this.isSavingMetadata = true
 

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)

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