diff --git a/server/managers/AbMergeManager.js b/server/managers/AbMergeManager.js index 9e06f99c..61c1d500 100644 --- a/server/managers/AbMergeManager.js +++ b/server/managers/AbMergeManager.js @@ -7,7 +7,7 @@ const { writeConcatFile } = require('../utils/ffmpegHelpers') const ffmpegHelpers = require('../utils/ffmpegHelpers') const Ffmpeg = require('../libs/fluentFfmpeg') const SocketAuthority = require('../SocketAuthority') -const fileUtils = require('../utils/fileUtils') +const { isWritable, copyToExisting } = require('../utils/fileUtils') const TrackProgressMonitor = require('../objects/TrackProgressMonitor') class AbMergeManager { @@ -64,7 +64,7 @@ class AbMergeManager { async runAudiobookMerge(libraryItem, task, encodingOptions) { // Make sure the target directory is writable - if (!(await fileUtils.isWritable(libraryItem.path))) { + if (!(await isWritable(libraryItem.path))) { Logger.error(`[AbMergeManager] Target directory is not writable: ${libraryItem.path}`) task.setFailed('Target directory is not writable') this.removeTask(task, true) @@ -139,18 +139,35 @@ class AbMergeManager { } // Move library item tracks to cache - for (const trackPath of task.data.originalTrackPaths) { + for (const [index, trackPath] of task.data.originalTrackPaths.entries()) { const trackFilename = Path.basename(trackPath) const moveToPath = Path.join(task.data.itemCachePath, trackFilename) Logger.debug(`[AbMergeManager] Backing up original track "${trackPath}" to ${moveToPath}`) - await fs.move(trackPath, moveToPath, { overwrite: true }).catch((err) => { - Logger.error(`[AbMergeManager] Failed to move track "${trackPath}" to "${moveToPath}"`, err) - }) + if (index === 0) { + // copy the first track to the cache directory + await fs.copy(trackPath, moveToPath).catch((err) => { + Logger.error(`[AbMergeManager] Failed to copy track "${trackPath}" to "${moveToPath}"`, err) + }) + } else { + // move the rest of the tracks to the cache directory + await fs.move(trackPath, moveToPath, { overwrite: true }).catch((err) => { + Logger.error(`[AbMergeManager] Failed to move track "${trackPath}" to "${moveToPath}"`, err) + }) + } } - // Move m4b to target + // Move m4b to target, preserving the original track's permissions Logger.debug(`[AbMergeManager] Moving m4b from ${task.data.tempFilepath} to ${task.data.targetFilepath}`) - await fs.move(task.data.tempFilepath, task.data.targetFilepath) + try { + await copyToExisting(task.data.tempFilepath, task.data.originalTrackPaths[0]) + await fs.rename(task.data.originalTrackPaths[0], task.data.targetFilepath) + await fs.remove(task.data.tempFilepath) + } catch (err) { + Logger.error(`[AbMergeManager] Failed to move m4b from ${task.data.tempFilepath} to ${task.data.targetFilepath}`, err) + task.setFailed('Failed to move m4b file') + this.removeTask(task, true) + return + } // Remove ffmetadata file await fs.remove(task.data.ffmetadataPath) diff --git a/server/utils/ffmpegHelpers.js b/server/utils/ffmpegHelpers.js index 8e8d270d..f3c40bf6 100644 --- a/server/utils/ffmpegHelpers.js +++ b/server/utils/ffmpegHelpers.js @@ -4,7 +4,7 @@ const ffmpgegUtils = require('../libs/fluentFfmpeg/utils') const fs = require('../libs/fsExtra') const Path = require('path') const Logger = require('../Logger') -const { filePathToPOSIX } = require('./fileUtils') +const { filePathToPOSIX, copyToExisting } = require('./fileUtils') const LibraryItem = require('../objects/LibraryItem') function escapeSingleQuotes(path) { @@ -250,11 +250,12 @@ module.exports.writeFFMetadataFile = writeFFMetadataFile * @param {string} metadataFilePath - Path to the ffmetadata file. * @param {number} track - The track number to embed in the audio file. * @param {string} mimeType - The MIME type of the audio file. - * @param {Ffmpeg} ffmpeg - The Ffmpeg instance to use (optional). Used for dependency injection in tests. * @param {function(number): void|null} progressCB - A callback function to report progress. + * @param {Ffmpeg} ffmpeg - The Ffmpeg instance to use (optional). Used for dependency injection in tests. + * @param {function(string, string): Promise} copyFunc - The function to use for copying files (optional). Used for dependency injection in tests. * @returns {Promise} A promise that resolves if the operation is successful, rejects otherwise. */ -async function addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, progressCB = null, ffmpeg = Ffmpeg()) { +async function addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, progressCB = null, ffmpeg = Ffmpeg(), copyFunc = copyToExisting) { const isMp4 = mimeType === 'audio/mp4' const isMp3 = mimeType === 'audio/mpeg' @@ -316,7 +317,8 @@ async function addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataF Logger.debug('[ffmpegHelpers] ffmpeg stderr:', stderr) Logger.debug('[ffmpegHelpers] Moving temp file to audio file path:', `"${tempFilePath}"`, '->', `"${audioFilePath}"`) try { - await fs.move(tempFilePath, audioFilePath, { overwrite: true }) + await copyFunc(tempFilePath, audioFilePath) + await fs.remove(tempFilePath) resolve() } catch (error) { Logger.error(`[ffmpegHelpers] Failed to move temp file to audio file path: "${tempFilePath}" -> "${audioFilePath}"`, error) diff --git a/server/utils/fileUtils.js b/server/utils/fileUtils.js index 5488b4d4..c68d2786 100644 --- a/server/utils/fileUtils.js +++ b/server/utils/fileUtils.js @@ -464,3 +464,46 @@ module.exports.getDirectoriesInPath = async (dirPath, level) => { return [] } } + +/** + * Copies a file from the source path to an existing destination path, preserving the destination's permissions. + * + * @param {string} srcPath - The path of the source file. + * @param {string} destPath - The path of the existing destination file. + * @returns {Promise} A promise that resolves when the file has been successfully copied. + * @throws {Error} If there is an error reading the source file or writing the destination file. + */ +async function copyToExisting(srcPath, destPath) { + return new Promise((resolve, reject) => { + // Create a readable stream from the source file + const readStream = fs.createReadStream(srcPath) + + // Create a writable stream to the destination file + const writeStream = fs.createWriteStream(destPath, { flags: 'w' }) + + // Pipe the read stream to the write stream + readStream.pipe(writeStream) + + // Handle the end of the stream + writeStream.on('finish', () => { + Logger.debug(`[copyToExisting] Successfully copied file from ${srcPath} to ${destPath}`) + resolve() + }) + + // Handle errors + readStream.on('error', (error) => { + Logger.error(`[copyToExisting] Error reading from source file ${srcPath}: ${error.message}`) + readStream.close() + writeStream.close() + reject(error) + }) + + writeStream.on('error', (error) => { + Logger.error(`[copyToExisting] Error writing to destination file ${destPath}: ${error.message}`) + readStream.close() + writeStream.close() + reject(error) + }) + }) +} +module.exports.copyToExisting = copyToExisting diff --git a/test/server/utils/ffmpegHelpers.test.js b/test/server/utils/ffmpegHelpers.test.js index ad069998..95a2c585 100644 --- a/test/server/utils/ffmpegHelpers.test.js +++ b/test/server/utils/ffmpegHelpers.test.js @@ -1,9 +1,11 @@ const { expect } = require('chai') const sinon = require('sinon') -const { generateFFMetadata, addCoverAndMetadataToFile } = require('../../../server/utils/ffmpegHelpers') +const fileUtils = require('../../../server/utils/fileUtils') const fs = require('../../../server/libs/fsExtra') const EventEmitter = require('events') +const { generateFFMetadata, addCoverAndMetadataToFile } = require('../../../server/utils/ffmpegHelpers') + global.isWin = process.platform === 'win32' describe('generateFFMetadata', () => { @@ -81,9 +83,10 @@ describe('addCoverAndMetadataToFile', () => { ffmpegStub.run = sinon.stub().callsFake(() => { ffmpegStub.emit('end') }) - const fsMove = sinon.stub(fs, 'move').resolves() + const copyStub = sinon.stub().resolves() + const fsRemoveStub = sinon.stub(fs, 'remove').resolves() - return { audioFilePath, coverFilePath, metadataFilePath, track, mimeType, ffmpegStub, fsMove } + return { audioFilePath, coverFilePath, metadataFilePath, track, mimeType, ffmpegStub, copyStub, fsRemoveStub } } let audioFilePath = null @@ -92,7 +95,8 @@ describe('addCoverAndMetadataToFile', () => { let track = null let mimeType = null let ffmpegStub = null - let fsMove = null + let copyStub = null + let fsRemoveStub = null beforeEach(() => { const input = createTestSetup() audioFilePath = input.audioFilePath @@ -101,12 +105,13 @@ describe('addCoverAndMetadataToFile', () => { track = input.track mimeType = input.mimeType ffmpegStub = input.ffmpegStub - fsMove = input.fsMove + copyStub = input.copyStub + fsRemoveStub = input.fsRemoveStub }) it('should add cover image and metadata to audio file', async () => { // Act - await addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, null, ffmpegStub) + await addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, null, ffmpegStub, copyStub) // Assert expect(ffmpegStub.input.calledThrice).to.be.true @@ -125,10 +130,11 @@ describe('addCoverAndMetadataToFile', () => { expect(ffmpegStub.run.calledOnce).to.be.true - expect(fsMove.calledOnce).to.be.true - expect(fsMove.firstCall.args[0]).to.equal('/path/to/audio/file.tmp.mp3') - expect(fsMove.firstCall.args[1]).to.equal('/path/to/audio/file.mp3') - expect(fsMove.firstCall.args[2]).to.deep.equal({ overwrite: true }) + expect(copyStub.calledOnce).to.be.true + expect(copyStub.firstCall.args[0]).to.equal('/path/to/audio/file.tmp.mp3') + expect(copyStub.firstCall.args[1]).to.equal('/path/to/audio/file.mp3') + expect(fsRemoveStub.calledOnce).to.be.true + expect(fsRemoveStub.firstCall.args[0]).to.equal('/path/to/audio/file.tmp.mp3') // Restore the stub sinon.restore() @@ -139,7 +145,7 @@ describe('addCoverAndMetadataToFile', () => { coverFilePath = null // Act - await addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, null, ffmpegStub) + await addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, null, ffmpegStub, copyStub) // Assert expect(ffmpegStub.input.calledTwice).to.be.true @@ -157,10 +163,11 @@ describe('addCoverAndMetadataToFile', () => { expect(ffmpegStub.run.calledOnce).to.be.true - expect(fsMove.calledOnce).to.be.true - expect(fsMove.firstCall.args[0]).to.equal('/path/to/audio/file.tmp.mp3') - expect(fsMove.firstCall.args[1]).to.equal('/path/to/audio/file.mp3') - expect(fsMove.firstCall.args[2]).to.deep.equal({ overwrite: true }) + expect(copyStub.callCount).to.equal(1) + expect(copyStub.firstCall.args[0]).to.equal('/path/to/audio/file.tmp.mp3') + expect(copyStub.firstCall.args[1]).to.equal('/path/to/audio/file.mp3') + expect(fsRemoveStub.calledOnce).to.be.true + expect(fsRemoveStub.firstCall.args[0]).to.equal('/path/to/audio/file.tmp.mp3') // Restore the stub sinon.restore() @@ -174,7 +181,7 @@ describe('addCoverAndMetadataToFile', () => { // Act try { - await addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, null, ffmpegStub) + await addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, null, ffmpegStub, copyStub) expect.fail('Expected an error to be thrown') } catch (error) { // Assert @@ -198,7 +205,8 @@ describe('addCoverAndMetadataToFile', () => { expect(ffmpegStub.run.calledOnce).to.be.true - expect(fsMove.called).to.be.false + expect(copyStub.called).to.be.false + expect(fsRemoveStub.called).to.be.false // Restore the stub sinon.restore() @@ -210,7 +218,7 @@ describe('addCoverAndMetadataToFile', () => { audioFilePath = '/path/to/audio/file.m4b' // Act - await addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, null, ffmpegStub) + await addCoverAndMetadataToFile(audioFilePath, coverFilePath, metadataFilePath, track, mimeType, null, ffmpegStub, copyStub) // Assert expect(ffmpegStub.input.calledThrice).to.be.true @@ -229,10 +237,11 @@ describe('addCoverAndMetadataToFile', () => { expect(ffmpegStub.run.calledOnce).to.be.true - expect(fsMove.calledOnce).to.be.true - expect(fsMove.firstCall.args[0]).to.equal('/path/to/audio/file.tmp.m4b') - expect(fsMove.firstCall.args[1]).to.equal('/path/to/audio/file.m4b') - expect(fsMove.firstCall.args[2]).to.deep.equal({ overwrite: true }) + expect(copyStub.calledOnce).to.be.true + expect(copyStub.firstCall.args[0]).to.equal('/path/to/audio/file.tmp.m4b') + expect(copyStub.firstCall.args[1]).to.equal('/path/to/audio/file.m4b') + expect(fsRemoveStub.calledOnce).to.be.true + expect(fsRemoveStub.firstCall.args[0]).to.equal('/path/to/audio/file.tmp.m4b') // Restore the stub sinon.restore()