mirror of
https://github.com/advplyr/audiobookshelf.git
synced 2026-07-07 09:51:37 +00:00
fix(placeholders): avoid false promotion on upload
Only promote and rescan placeholder items when uploaded files include supported audio or ebook media types, so artwork-only uploads do not convert placeholders prematurely. Also clean up newly created placeholder directories when placeholder creation rolls back in author and series flows, while preserving pre-existing directories. Add controller tests covering media/non-media upload behavior and rollback directory cleanup conditions.
This commit is contained in:
parent
fc325a4095
commit
5ae65d31dd
6 changed files with 263 additions and 2 deletions
|
|
@ -353,8 +353,11 @@ class AuthorController {
|
|||
return res.status(400).send('Library item already exists at that path')
|
||||
}
|
||||
|
||||
let createdPlaceholderDirectory = false
|
||||
try {
|
||||
const placeholderDirectoryExisted = await fs.pathExists(outputDirectory)
|
||||
await fs.ensureDir(outputDirectory)
|
||||
createdPlaceholderDirectory = !placeholderDirectoryExisted
|
||||
} catch (error) {
|
||||
Logger.error(`[AuthorController] Failed to create placeholder directory "${outputDirectory}"`, error)
|
||||
return res.status(500).send('Failed to create placeholder directory')
|
||||
|
|
@ -430,6 +433,11 @@ class AuthorController {
|
|||
} catch (error) {
|
||||
Logger.error('[AuthorController] Failed to create author placeholder', error)
|
||||
await transaction.rollback()
|
||||
if (createdPlaceholderDirectory) {
|
||||
await fs.remove(outputDirectory).catch((cleanupError) => {
|
||||
Logger.error(`[AuthorController] Failed to remove placeholder directory after rollback "${outputDirectory}"`, cleanupError)
|
||||
})
|
||||
}
|
||||
return res.status(500).send('Failed to create placeholder')
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ const patternValidation = require('../libs/nodeCron/pattern-validation')
|
|||
const { isObject, getTitleIgnorePrefix } = require('../utils/index')
|
||||
const fileUtils = require('../utils/fileUtils')
|
||||
const { sanitizeFilename } = fileUtils
|
||||
const globals = require('../utils/globals')
|
||||
const LibraryItemScanner = require('../scanner/LibraryItemScanner')
|
||||
const LibraryScanner = require('../scanner/LibraryScanner')
|
||||
|
||||
|
|
@ -146,6 +147,14 @@ class MiscController {
|
|||
}
|
||||
|
||||
if (placeholderItem) {
|
||||
const hasUploadMedia = files.some((file) => {
|
||||
const ext = Path.extname(file?.name || '')
|
||||
.slice(1)
|
||||
.toLowerCase()
|
||||
if (!ext) return false
|
||||
return globals.SupportedAudioTypes.includes(ext) || globals.SupportedEbookTypes.includes(ext)
|
||||
})
|
||||
|
||||
// Promote placeholder and attach files when upload targets a placeholder folder.
|
||||
const updateDetails = {
|
||||
libraryFolderId: folder.id,
|
||||
|
|
@ -153,9 +162,11 @@ class MiscController {
|
|||
path: outputDirectory,
|
||||
isFile: placeholderItem.isFile
|
||||
}
|
||||
if (hasUploadMedia) {
|
||||
await LibraryScanner.promotePlaceholder(placeholderItem)
|
||||
await LibraryItemScanner.scanLibraryItem(placeholderItem.id, updateDetails)
|
||||
}
|
||||
}
|
||||
|
||||
res.sendStatus(200)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -195,8 +195,11 @@ class SeriesController {
|
|||
return res.status(400).send('Library item already exists at that path')
|
||||
}
|
||||
|
||||
let createdPlaceholderDirectory = false
|
||||
try {
|
||||
const placeholderDirectoryExisted = await fs.pathExists(outputDirectory)
|
||||
await fs.ensureDir(outputDirectory)
|
||||
createdPlaceholderDirectory = !placeholderDirectoryExisted
|
||||
} catch (error) {
|
||||
Logger.error(`[SeriesController] Failed to create placeholder directory "${outputDirectory}"`, error)
|
||||
return res.status(500).send('Failed to create placeholder directory')
|
||||
|
|
@ -280,6 +283,11 @@ class SeriesController {
|
|||
} catch (error) {
|
||||
Logger.error(`[SeriesController] Failed to create series placeholder`, error)
|
||||
await transaction.rollback()
|
||||
if (createdPlaceholderDirectory) {
|
||||
await fs.remove(outputDirectory).catch((cleanupError) => {
|
||||
Logger.error(`[SeriesController] Failed to remove placeholder directory after rollback "${outputDirectory}"`, cleanupError)
|
||||
})
|
||||
}
|
||||
return res.status(500).send('Failed to create placeholder')
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -32,6 +32,7 @@ describe('AuthorController placeholders', () => {
|
|||
})
|
||||
|
||||
sinon.stub(Logger, 'warn')
|
||||
sinon.stub(fs, 'pathExists').resolves(false)
|
||||
sinon.stub(fs, 'ensureDir').resolves()
|
||||
|
||||
library = await Database.libraryModel.create({ name: 'Test Library', mediaType: 'book' })
|
||||
|
|
@ -167,4 +168,61 @@ describe('AuthorController placeholders', () => {
|
|||
expect(payload.folderId).to.equal(secondaryLibraryFolder.id)
|
||||
expect(payload.path.startsWith(secondaryLibraryFolder.path)).to.be.true
|
||||
})
|
||||
|
||||
it('removes placeholder directory when creation fails after directory is created', async () => {
|
||||
sinon.stub(fs, 'remove').resolves()
|
||||
fs.pathExists.resolves(false)
|
||||
sinon.stub(Database.libraryItemModel, 'create').rejects(new Error('library item create failed'))
|
||||
|
||||
const fakeReq = {
|
||||
params: {
|
||||
id: author.id
|
||||
},
|
||||
author,
|
||||
user,
|
||||
body: {}
|
||||
}
|
||||
|
||||
const fakeRes = {
|
||||
status: sinon.stub().returnsThis(),
|
||||
json: sinon.spy(),
|
||||
sendStatus: sinon.spy(),
|
||||
send: sinon.spy()
|
||||
}
|
||||
|
||||
await AuthorController.createPlaceholder.bind(apiRouter)(fakeReq, fakeRes)
|
||||
|
||||
expect(fakeRes.status.calledWith(500)).to.be.true
|
||||
expect(fakeRes.send.calledWith('Failed to create placeholder')).to.be.true
|
||||
expect(fs.remove.calledOnce).to.be.true
|
||||
expect(fs.remove.firstCall.args[0]).to.equal('/test/Jane Doe/Placeholder')
|
||||
})
|
||||
|
||||
it('does not remove placeholder directory on rollback when it already existed', async () => {
|
||||
sinon.stub(fs, 'remove').resolves()
|
||||
fs.pathExists.resolves(true)
|
||||
sinon.stub(Database.libraryItemModel, 'create').rejects(new Error('library item create failed'))
|
||||
|
||||
const fakeReq = {
|
||||
params: {
|
||||
id: author.id
|
||||
},
|
||||
author,
|
||||
user,
|
||||
body: {}
|
||||
}
|
||||
|
||||
const fakeRes = {
|
||||
status: sinon.stub().returnsThis(),
|
||||
json: sinon.spy(),
|
||||
sendStatus: sinon.spy(),
|
||||
send: sinon.spy()
|
||||
}
|
||||
|
||||
await AuthorController.createPlaceholder.bind(apiRouter)(fakeReq, fakeRes)
|
||||
|
||||
expect(fakeRes.status.calledWith(500)).to.be.true
|
||||
expect(fakeRes.send.calledWith('Failed to create placeholder')).to.be.true
|
||||
expect(fs.remove.called).to.be.false
|
||||
})
|
||||
})
|
||||
|
|
|
|||
118
test/server/controllers/MiscController.placeholders.test.js
Normal file
118
test/server/controllers/MiscController.placeholders.test.js
Normal file
|
|
@ -0,0 +1,118 @@
|
|||
const { expect } = require('chai')
|
||||
const { Sequelize } = require('sequelize')
|
||||
const sinon = require('sinon')
|
||||
|
||||
const MiscController = require('../../../server/controllers/MiscController')
|
||||
const Database = require('../../../server/Database')
|
||||
const fs = require('../../../server/libs/fsExtra')
|
||||
const LibraryScanner = require('../../../server/scanner/LibraryScanner')
|
||||
const LibraryItemScanner = require('../../../server/scanner/LibraryItemScanner')
|
||||
|
||||
describe('MiscController placeholder uploads', () => {
|
||||
let library
|
||||
let folder
|
||||
let placeholderItem
|
||||
|
||||
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()
|
||||
|
||||
library = await Database.libraryModel.create({ name: 'Test Library', mediaType: 'book' })
|
||||
folder = await Database.libraryFolderModel.create({ path: '/library', libraryId: library.id })
|
||||
const book = await Database.bookModel.create({ title: 'Placeholder', audioFiles: [], tags: [], narrators: [], genres: [], chapters: [] })
|
||||
placeholderItem = await Database.libraryItemModel.create({
|
||||
libraryFiles: [],
|
||||
mediaId: book.id,
|
||||
mediaType: 'book',
|
||||
libraryId: library.id,
|
||||
libraryFolderId: folder.id,
|
||||
relPath: 'Author/Title',
|
||||
path: '/library/Author/Title',
|
||||
isFile: false,
|
||||
isPlaceholder: true
|
||||
})
|
||||
|
||||
sinon.stub(fs, 'ensureDir').resolves()
|
||||
sinon.stub(LibraryScanner, 'promotePlaceholder').resolves()
|
||||
sinon.stub(LibraryItemScanner, 'scanLibraryItem').resolves()
|
||||
})
|
||||
|
||||
afterEach(async () => {
|
||||
sinon.restore()
|
||||
await Database.sequelize.sync({ force: true })
|
||||
})
|
||||
|
||||
it('does not promote placeholder-targeted uploads when files are non-media only', async () => {
|
||||
const fakeReq = {
|
||||
user: {
|
||||
canUpload: true,
|
||||
username: 'admin',
|
||||
checkCanAccessLibrary: () => true
|
||||
},
|
||||
body: {
|
||||
title: 'Placeholder',
|
||||
library: library.id,
|
||||
folder: folder.id,
|
||||
placeholder: `id:${placeholderItem.id}`
|
||||
},
|
||||
files: {
|
||||
cover: {
|
||||
name: 'cover.jpg',
|
||||
mv: sinon.stub().resolves()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const fakeRes = {
|
||||
status: sinon.stub().returnsThis(),
|
||||
sendStatus: sinon.spy(),
|
||||
send: sinon.spy()
|
||||
}
|
||||
|
||||
await MiscController.handleUpload(fakeReq, fakeRes)
|
||||
|
||||
expect(LibraryScanner.promotePlaceholder.called).to.be.false
|
||||
expect(LibraryItemScanner.scanLibraryItem.called).to.be.false
|
||||
expect(fakeRes.sendStatus.calledWith(200)).to.be.true
|
||||
})
|
||||
|
||||
it('promotes placeholder-targeted uploads when files include media', async () => {
|
||||
const fakeReq = {
|
||||
user: {
|
||||
canUpload: true,
|
||||
username: 'admin',
|
||||
checkCanAccessLibrary: () => true
|
||||
},
|
||||
body: {
|
||||
title: 'Placeholder',
|
||||
library: library.id,
|
||||
folder: folder.id,
|
||||
placeholder: `id:${placeholderItem.id}`
|
||||
},
|
||||
files: {
|
||||
audio: {
|
||||
name: 'track-01.mp3',
|
||||
mv: sinon.stub().resolves()
|
||||
},
|
||||
cover: {
|
||||
name: 'cover.jpg',
|
||||
mv: sinon.stub().resolves()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const fakeRes = {
|
||||
status: sinon.stub().returnsThis(),
|
||||
sendStatus: sinon.spy(),
|
||||
send: sinon.spy()
|
||||
}
|
||||
|
||||
await MiscController.handleUpload(fakeReq, fakeRes)
|
||||
|
||||
expect(LibraryScanner.promotePlaceholder.calledOnce).to.be.true
|
||||
expect(LibraryItemScanner.scanLibraryItem.calledOnce).to.be.true
|
||||
expect(fakeRes.sendStatus.calledWith(200)).to.be.true
|
||||
})
|
||||
})
|
||||
|
|
@ -32,6 +32,7 @@ describe('SeriesController placeholders', () => {
|
|||
})
|
||||
|
||||
sinon.stub(Logger, 'warn')
|
||||
sinon.stub(fs, 'pathExists').resolves(false)
|
||||
sinon.stub(fs, 'ensureDir').resolves()
|
||||
|
||||
library = await Database.libraryModel.create({ name: 'Test Library', mediaType: 'book' })
|
||||
|
|
@ -309,4 +310,61 @@ describe('SeriesController placeholders', () => {
|
|||
expect(payload.path.startsWith(libraryFolder.path)).to.be.true
|
||||
expect(payload.path).to.include('/Test Series/Placeholder')
|
||||
})
|
||||
|
||||
it('removes placeholder directory when creation fails after directory is created', async () => {
|
||||
sinon.stub(fs, 'remove').resolves()
|
||||
fs.pathExists.resolves(false)
|
||||
sinon.stub(Database.libraryItemModel, 'create').rejects(new Error('library item create failed'))
|
||||
|
||||
const fakeReq = {
|
||||
params: {
|
||||
id: library.id,
|
||||
seriesId: series.id
|
||||
},
|
||||
user,
|
||||
body: {}
|
||||
}
|
||||
|
||||
const fakeRes = {
|
||||
status: sinon.stub().returnsThis(),
|
||||
json: sinon.spy(),
|
||||
sendStatus: sinon.spy(),
|
||||
send: sinon.spy()
|
||||
}
|
||||
|
||||
await SeriesController.createPlaceholder.bind(apiRouter)(fakeReq, fakeRes)
|
||||
|
||||
expect(fakeRes.status.calledWith(500)).to.be.true
|
||||
expect(fakeRes.send.calledWith('Failed to create placeholder')).to.be.true
|
||||
expect(fs.remove.calledOnce).to.be.true
|
||||
expect(fs.remove.firstCall.args[0]).to.equal('/test/Test Series/Placeholder')
|
||||
})
|
||||
|
||||
it('does not remove placeholder directory on rollback when it already existed', async () => {
|
||||
sinon.stub(fs, 'remove').resolves()
|
||||
fs.pathExists.resolves(true)
|
||||
sinon.stub(Database.libraryItemModel, 'create').rejects(new Error('library item create failed'))
|
||||
|
||||
const fakeReq = {
|
||||
params: {
|
||||
id: library.id,
|
||||
seriesId: series.id
|
||||
},
|
||||
user,
|
||||
body: {}
|
||||
}
|
||||
|
||||
const fakeRes = {
|
||||
status: sinon.stub().returnsThis(),
|
||||
json: sinon.spy(),
|
||||
sendStatus: sinon.spy(),
|
||||
send: sinon.spy()
|
||||
}
|
||||
|
||||
await SeriesController.createPlaceholder.bind(apiRouter)(fakeReq, fakeRes)
|
||||
|
||||
expect(fakeRes.status.calledWith(500)).to.be.true
|
||||
expect(fakeRes.send.calledWith('Failed to create placeholder')).to.be.true
|
||||
expect(fs.remove.called).to.be.false
|
||||
})
|
||||
})
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue