Skip to content

Commit 4140312

Browse files
dialmasterdialmaster
authored
Merge pull request #346 from DialmasterOrg/343-retry-manage-failed-downloads
fix(downloads): special chars in channel names
2 parents ca3beb7 + a4c2276 commit 4140312

File tree

2 files changed

+224
-7
lines changed

2 files changed

+224
-7
lines changed

‎server/modules/__tests__/videoDownloadPostProcessFiles.test.js‎

Lines changed: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -747,4 +747,215 @@ describe('videoDownloadPostProcessFiles', () => {
747747
expect(moveCalls.length).toBe(0);
748748
});
749749
});
750+
751+
describe('special character handling in channel names', () => {
752+
it('uses filesystem path for channel with # character (temp downloads + subfolder)', async () => {
753+
// Scenario: Channel name in metadata is "Fred again . #" but yt-dlp sanitizes to "Fred again . ."
754+
const sanitizedChannelName = 'Fred again . .';
755+
const rawChannelName = 'Fred again . #';
756+
const tempVideoPath = `/tmp/youtarr-downloads/${sanitizedChannelName}/Fred again . . - Video Title [abc123]/Video Title [abc123].mp4`;
757+
const tempVideoDir = `/tmp/youtarr-downloads/${sanitizedChannelName}/Fred again . . - Video Title [abc123]`;
758+
process.argv = ['node', 'script', tempVideoPath];
759+
760+
Channel.findOne.mockResolvedValue({
761+
sub_folder: 'Music',
762+
uploader: rawChannelName
763+
});
764+
765+
tempPathManager.isEnabled.mockReturnValue(true);
766+
tempPathManager.isTempPath.mockReturnValue(true);
767+
768+
fs.readFileSync.mockReturnValue(JSON.stringify({
769+
id: 'abc123',
770+
title: 'Video Title',
771+
uploader: rawChannelName, // Raw name with #
772+
channel_id: 'channel123'
773+
}));
774+
775+
const tempJsonPath = `${tempVideoDir}/Video Title [abc123].info.json`;
776+
fs.existsSync.mockImplementation((path) => {
777+
return path === tempJsonPath || path.includes('/library/__Music/Fred again');
778+
});
779+
fs.pathExists.mockResolvedValue(false);
780+
781+
await loadModule();
782+
await settleAsync();
783+
784+
// Verify fs.move uses the SANITIZED channel name from filesystem, not raw metadata
785+
expect(fs.move).toHaveBeenCalledWith(
786+
tempVideoDir,
787+
expect.stringContaining(`/library/__Music/${sanitizedChannelName}/Fred again`)
788+
);
789+
790+
// Verify ensureDir was called with sanitized name
791+
expect(fs.ensureDir).toHaveBeenCalledWith(
792+
expect.stringContaining(`/library/__Music/${sanitizedChannelName}`)
793+
);
794+
795+
// Verify _actual_filepath uses sanitized name
796+
expect(fs.writeFileSync).toHaveBeenCalledWith(
797+
tempJsonPath,
798+
expect.stringContaining(`"_actual_filepath": "/library/__Music/${sanitizedChannelName}/Fred again`)
799+
);
800+
});
801+
802+
it('uses filesystem path for channel with trailing dots (no temp downloads + subfolder)', async () => {
803+
// Scenario: Channel name is "Fred again . ." with trailing dots
804+
const channelName = 'Fred again . .';
805+
const videoPathInChannel = `/library/${channelName}/Video Title [abc123]/Video Title [abc123].mp4`;
806+
process.argv = ['node', 'script', videoPathInChannel];
807+
808+
Channel.findOne.mockResolvedValue({
809+
sub_folder: 'Music',
810+
uploader: channelName
811+
});
812+
813+
fs.readFileSync.mockReturnValue(JSON.stringify({
814+
id: 'abc123',
815+
title: 'Video Title',
816+
uploader: channelName,
817+
channel_id: 'channel123'
818+
}));
819+
820+
fs.existsSync.mockImplementation((path) => {
821+
return path === `/library/${channelName}/Video Title [abc123]/Video Title [abc123].info.json` ||
822+
path === videoPathInChannel;
823+
});
824+
fs.pathExists.mockResolvedValue(false);
825+
fs.readdir.mockResolvedValue(['Video Title [abc123]']);
826+
827+
await loadModule();
828+
await settleAsync();
829+
830+
// Verify fs.ensureDir was called with the channel name from filesystem
831+
expect(fs.ensureDir).toHaveBeenCalledWith(
832+
expect.stringContaining('/library/__Music')
833+
);
834+
835+
// Verify fs.move uses the actual channel name from filesystem
836+
expect(fs.move).toHaveBeenCalledWith(
837+
`/library/${channelName}`,
838+
`/library/__Music/${channelName}`
839+
);
840+
841+
// Verify success log
842+
expect(logger.info).toHaveBeenCalledWith(
843+
expect.objectContaining({
844+
expectedPath: `/library/__Music/${channelName}`
845+
}),
846+
'[Post-Process] Successfully moved to subfolder'
847+
);
848+
});
849+
850+
it('uses filesystem path for channel with colon character', async () => {
851+
// Scenario: Channel name with : which gets sanitized by yt-dlp
852+
const sanitizedChannelName = 'Test Channel';
853+
const rawChannelName = 'Test: Channel';
854+
const tempVideoPath = `/tmp/youtarr-downloads/${sanitizedChannelName}/Video Title [abc123]/Video Title [abc123].mp4`;
855+
const tempVideoDir = `/tmp/youtarr-downloads/${sanitizedChannelName}/Video Title [abc123]`;
856+
process.argv = ['node', 'script', tempVideoPath];
857+
858+
Channel.findOne.mockResolvedValue({
859+
sub_folder: 'Education',
860+
uploader: rawChannelName
861+
});
862+
863+
tempPathManager.isEnabled.mockReturnValue(true);
864+
tempPathManager.isTempPath.mockReturnValue(true);
865+
866+
fs.readFileSync.mockReturnValue(JSON.stringify({
867+
id: 'abc123',
868+
title: 'Video Title',
869+
uploader: rawChannelName, // Raw name with :
870+
channel_id: 'channel123'
871+
}));
872+
873+
const tempJsonPath = `${tempVideoDir}/Video Title [abc123].info.json`;
874+
fs.existsSync.mockImplementation((path) => {
875+
return path === tempJsonPath || path.includes('/library/__Education/Test Channel');
876+
});
877+
fs.pathExists.mockResolvedValue(false);
878+
879+
await loadModule();
880+
await settleAsync();
881+
882+
// Verify fs.move uses the SANITIZED channel name from filesystem
883+
expect(fs.move).toHaveBeenCalledWith(
884+
tempVideoDir,
885+
expect.stringContaining(`/library/__Education/${sanitizedChannelName}/Video Title`)
886+
);
887+
888+
// Verify no errors logged (ensureDir should succeed)
889+
expect(logger.error).not.toHaveBeenCalledWith(
890+
expect.objectContaining({ error: expect.anything() }),
891+
'[Post-Process] ERROR during move operation'
892+
);
893+
});
894+
895+
it('handles channel name with multiple special characters', async () => {
896+
// Scenario: Channel name with multiple special chars: #<>:|?*
897+
const sanitizedChannelName = 'Channel Name';
898+
const rawChannelName = 'Channel #<>:|?* Name';
899+
const videoPath = `/library/${sanitizedChannelName}/Video Title [abc123]/Video Title [abc123].mp4`;
900+
process.argv = ['node', 'script', videoPath];
901+
902+
fs.readFileSync.mockReturnValue(JSON.stringify({
903+
id: 'abc123',
904+
title: 'Video Title',
905+
uploader: rawChannelName, // Raw name with special chars
906+
channel_id: 'channel123'
907+
}));
908+
909+
fs.existsSync.mockImplementation((path) => {
910+
return path === `/library/${sanitizedChannelName}/Video Title [abc123]/Video Title [abc123].info.json` ||
911+
path === videoPath;
912+
});
913+
914+
await loadModule();
915+
await settleAsync();
916+
917+
// Verify basic post-processing succeeds without errors
918+
expect(fs.moveSync).toHaveBeenCalledWith(
919+
expect.stringContaining('/library/Channel Name/Video Title [abc123]/Video Title [abc123].info.json'),
920+
'/mock/jobs/info/abc123.info.json',
921+
{ overwrite: true }
922+
);
923+
924+
// Verify no critical errors
925+
expect(process.exit).not.toHaveBeenCalledWith(1);
926+
});
927+
928+
it('writes _actual_filepath with __ prefix when channel has subfolder (non-temp)', async () => {
929+
// This test verifies the fix for line 240 (missing __ prefix)
930+
const channelName = 'Test Channel';
931+
const videoPath = `/library/${channelName}/Video Title [abc123]/Video Title [abc123].mp4`;
932+
process.argv = ['node', 'script', videoPath];
933+
934+
Channel.findOne.mockResolvedValue({
935+
sub_folder: 'Music',
936+
uploader: channelName
937+
});
938+
939+
fs.readFileSync.mockReturnValue(JSON.stringify({
940+
id: 'abc123',
941+
title: 'Video Title',
942+
uploader: channelName,
943+
channel_id: 'channel123'
944+
}));
945+
946+
fs.existsSync.mockImplementation((path) => {
947+
return path === `/library/${channelName}/Video Title [abc123]/Video Title [abc123].info.json` ||
948+
path === videoPath;
949+
});
950+
951+
await loadModule();
952+
await settleAsync();
953+
954+
// Verify that _actual_filepath includes the __ prefix for subfolder
955+
expect(fs.writeFileSync).toHaveBeenCalledWith(
956+
expect.stringContaining('/library/Test Channel/Video Title [abc123]/Video Title [abc123].info.json'),
957+
expect.stringContaining('"_actual_filepath": "/library/__Music/Test Channel/Video Title [abc123]/Video Title [abc123].mp4"')
958+
);
959+
});
960+
});
750961
});

‎server/modules/videoDownloadPostProcessFiles.js‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ const videoDirectory = path.dirname(videoPath);
2424
// Poster image uses same filename as video but with .jpg extension
2525
const imagePath = path.join(videoDirectory, parsedPath.name + '.jpg');
2626

27+
// Extract the actual channel folder name that yt-dlp created (already sanitized)
28+
// This is more reliable than using jsonData.uploader which may contain special characters
29+
// that yt-dlp sanitizes differently (e.g., #, :, <, >, etc.)
30+
const actualChannelFolderName = path.basename(path.dirname(videoDirectory));
31+
2732
const sleep = (ms) => new Promise((resolve) => setTimeout(resolve, ms));
2833

2934
async function moveWithRetries(src, dest, { retries = 5, delayMs = 200 } = {}) {
@@ -197,9 +202,9 @@ async function copyChannelPosterIfNeeded(channelId, channelFolderPath) {
197202
if (channel && channel.sub_folder) {
198203
channelSubFolder = channel.sub_folder;
199204
const baseDir = configModule.directoryPath;
200-
const channelName = jsonData.uploader || jsonData.channel || channel.uploader;
201-
// Add __ prefix for namespace safety
202-
targetChannelFolder = path.join(baseDir, `__${channel.sub_folder}`, channelName);
205+
// Use the actual channel folder name from the filesystem (yt-dlp already sanitized it)
206+
// instead of jsonData.uploader which may contain special characters
207+
targetChannelFolder = path.join(baseDir, `__${channel.sub_folder}`, actualChannelFolderName);
203208
console.log(`[Post-Process] Channel has subfolder setting: ${channel.sub_folder}`);
204209
}
205210
} catch (err) {
@@ -231,7 +236,8 @@ async function copyChannelPosterIfNeeded(channelId, channelFolderPath) {
231236
const videoDirectoryName = path.basename(videoDirectory);
232237
const videoFileName = path.basename(videoPath);
233238
const baseDir = configModule.directoryPath;
234-
finalVideoPathForJson = path.join(baseDir, channelSubFolder, channelName, videoDirectoryName, videoFileName);
239+
// Add __ prefix for namespace safety (consistent with targetChannelFolder)
240+
finalVideoPathForJson = path.join(baseDir, `__${channelSubFolder}`, channelName, videoDirectoryName, videoFileName);
235241
} else {
236242
// No subfolder - use current path
237243
finalVideoPathForJson = videoPath;
@@ -488,9 +494,9 @@ async function copyChannelPosterIfNeeded(channelId, channelFolderPath) {
488494

489495
if (channel && channel.sub_folder) {
490496
const baseDir = configModule.directoryPath;
491-
const channelName = jsonData.uploader || jsonData.channel || channel.uploader;
492-
// Add __ prefix for namespace safety
493-
const expectedPath = path.join(baseDir, `__${channel.sub_folder}`, channelName);
497+
// Use the actual channel folder name from the filesystem (yt-dlp already sanitized it)
498+
// instead of jsonData.uploader which may contain special characters
499+
const expectedPath = path.join(baseDir, `__${channel.sub_folder}`, actualChannelFolderName);
494500

495501
// Check if we're not already in the correct subfolder
496502
if (currentChannelFolder !== expectedPath) {

0 commit comments

Comments
 (0)