mirror of
https://github.com/kikootwo/ReadMeABook.git
synced 2026-07-10 15:00:15 +00:00
Use PathMapper.normalizePath for relocation checks
Make path-boundary checks separator-agnostic by exposing and enhancing PathMapper.normalizePath and using it in processMonitorDownload. normalizePath now converts backslashes to forward slashes, collapses redundant separators and `..` segments, and strips trailing separators (except root). The relocation test in processMonitorDownload now compares normalized download/save paths and treats a path as "relocated" only when equal-to or a child of the save path (boundary enforced). Added unit and integration tests to cover Windows backslashes, trailing separators, `..` collapse, sibling-prefix edge cases, and preserved existing wait/retry behavior (#209).
This commit is contained in:
@@ -122,10 +122,16 @@ export async function processMonitorDownload(payload: MonitorDownloadPayload): P
|
||||
throw new Error('Download path not available from download client');
|
||||
}
|
||||
|
||||
// Detect TempPathEnabled race: content_path hasn't been relocated to save_path yet
|
||||
// Detect TempPathEnabled race: content_path hasn't been relocated to save_path yet.
|
||||
// Normalize BOTH operands so the boundary test is separator-agnostic — Windows
|
||||
// clients report backslash paths, which a forward-slash prefix never matches (#209).
|
||||
// "Relocated" means downloadPath is equal to, or a child of, savePath.
|
||||
if (info.savePath && downloadPath) {
|
||||
const normalizedSave = info.savePath.endsWith('/') ? info.savePath : info.savePath + '/';
|
||||
if (!downloadPath.startsWith(normalizedSave)) {
|
||||
const normalizedSave = PathMapper.normalizePath(info.savePath);
|
||||
const normalizedDownload = PathMapper.normalizePath(downloadPath);
|
||||
const isRelocated = normalizedDownload === normalizedSave
|
||||
|| normalizedDownload.startsWith(normalizedSave + '/');
|
||||
if (!isRelocated) {
|
||||
const waitCount = (prevPathWaitCount ?? 0) + 1;
|
||||
const MAX_PATH_WAIT = 30; // Give up after ~5 minutes
|
||||
|
||||
|
||||
@@ -163,15 +163,17 @@ export class PathMapper {
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalizes a file path for consistent comparison
|
||||
* - Converts backslashes to forward slashes
|
||||
* - Removes trailing slashes
|
||||
* - Normalizes redundant separators
|
||||
* Normalizes a file path for consistent, OS-agnostic comparison.
|
||||
* Shared house normalizer — reuse this for any path-boundary/prefix check
|
||||
* rather than hand-building a separator-specific prefix (see #209).
|
||||
* - Converts backslashes to forward slashes (handles Windows clients)
|
||||
* - Normalizes redundant separators and `..` segments
|
||||
* - Removes trailing slashes (except root '/')
|
||||
*
|
||||
* @param filePath - Path to normalize
|
||||
* @returns Normalized path
|
||||
*/
|
||||
private static normalizePath(filePath: string): string {
|
||||
static normalizePath(filePath: string): string {
|
||||
// Convert backslashes to forward slashes
|
||||
let normalized = filePath.replace(/\\/g, '/');
|
||||
|
||||
|
||||
@@ -471,6 +471,218 @@ describe('processMonitorDownload', () => {
|
||||
);
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// TempPathEnabled relocation check (#209): the completion branch must treat a
|
||||
// download as relocated when downloadPath is equal-to/under savePath, comparing
|
||||
// separator-agnostically so Windows backslash paths organize immediately. When
|
||||
// the file is genuinely still in a temp dir outside savePath, the existing
|
||||
// wait/retry protection must be preserved byte-for-byte.
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/** Build a `completed` torrent client mock with the given save/download paths. */
|
||||
const relocationClientMock = (savePath: string, downloadPath: string) => ({
|
||||
clientType: 'qbittorrent',
|
||||
protocol: 'torrent',
|
||||
getDownload: vi.fn().mockResolvedValue({
|
||||
id: 'hash-reloc',
|
||||
name: 'Book',
|
||||
size: 0,
|
||||
bytesDownloaded: 0,
|
||||
progress: 1.0,
|
||||
status: 'completed',
|
||||
downloadSpeed: 0,
|
||||
eta: 0,
|
||||
category: 'readmeabook',
|
||||
savePath,
|
||||
downloadPath,
|
||||
}),
|
||||
});
|
||||
|
||||
/** Stub the deps the completion branch needs to reach addOrganizeJob. */
|
||||
const stubCompletionDeps = () => {
|
||||
downloadClientManagerMock.getClientForProtocol.mockResolvedValue({
|
||||
id: 'client-reloc',
|
||||
type: 'qbittorrent',
|
||||
name: 'qBittorrent',
|
||||
enabled: true,
|
||||
remotePathMappingEnabled: false,
|
||||
});
|
||||
prismaMock.request.update.mockResolvedValue({});
|
||||
prismaMock.downloadHistory.update.mockResolvedValue({});
|
||||
prismaMock.request.findFirst.mockResolvedValue({
|
||||
id: 'req-reloc',
|
||||
audiobook: { id: 'a-reloc' },
|
||||
deletedAt: null,
|
||||
});
|
||||
};
|
||||
|
||||
it('organizes immediately when a Windows backslash path is already relocated (#209)', async () => {
|
||||
downloadClientManagerMock.getClientServiceForProtocol.mockResolvedValue(
|
||||
relocationClientMock('E:\\Torrents\\ReadMeABook', 'E:\\Torrents\\ReadMeABook\\Book.m4b')
|
||||
);
|
||||
stubCompletionDeps();
|
||||
|
||||
const { processMonitorDownload } = await import('@/lib/processors/monitor-download.processor');
|
||||
const result = await processMonitorDownload({
|
||||
requestId: 'req-reloc',
|
||||
downloadHistoryId: 'dh-reloc',
|
||||
downloadClientId: 'hash-reloc',
|
||||
downloadClient: 'qbittorrent',
|
||||
jobId: 'job-reloc',
|
||||
});
|
||||
|
||||
expect(result.completed).toBe(true);
|
||||
expect(jobQueueMock.addOrganizeJob).toHaveBeenCalled();
|
||||
// Must NOT re-schedule a relocation wait.
|
||||
expect(jobQueueMock.addMonitorJob).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('preserves the wait/retry protection when a Windows file is still in a temp dir (#209)', async () => {
|
||||
downloadClientManagerMock.getClientServiceForProtocol.mockResolvedValue(
|
||||
relocationClientMock('E:\\Torrents\\ReadMeABook', 'E:\\Torrents\\incomplete\\Book')
|
||||
);
|
||||
prismaMock.request.update.mockResolvedValue({});
|
||||
prismaMock.downloadHistory.update.mockResolvedValue({});
|
||||
|
||||
const { processMonitorDownload } = await import('@/lib/processors/monitor-download.processor');
|
||||
const result = await processMonitorDownload({
|
||||
requestId: 'req-reloc',
|
||||
downloadHistoryId: 'dh-reloc',
|
||||
downloadClientId: 'hash-reloc',
|
||||
downloadClient: 'qbittorrent',
|
||||
jobId: 'job-reloc',
|
||||
});
|
||||
|
||||
// Exact existing behavior: completed:false + pathWaitCount:1, no organize.
|
||||
expect(result).toMatchObject({ success: true, completed: false, pathWaitCount: 1 });
|
||||
expect(jobQueueMock.addOrganizeJob).not.toHaveBeenCalled();
|
||||
// Re-scheduled with the same delay/arg shape: first wait → delay 2, lastProgress 100,
|
||||
// stallCount 0, pathWaitCount 1.
|
||||
expect(jobQueueMock.addMonitorJob).toHaveBeenCalledWith(
|
||||
'req-reloc', 'dh-reloc', 'hash-reloc', 'qbittorrent', 2, 100, 0, 1
|
||||
);
|
||||
});
|
||||
|
||||
it('treats trailing-separator save paths as relocated (#209)', async () => {
|
||||
// Backslash trailing form.
|
||||
downloadClientManagerMock.getClientServiceForProtocol.mockResolvedValue(
|
||||
relocationClientMock('E:\\Torrents\\ReadMeABook\\', 'E:\\Torrents\\ReadMeABook\\Book.m4b')
|
||||
);
|
||||
stubCompletionDeps();
|
||||
|
||||
const { processMonitorDownload } = await import('@/lib/processors/monitor-download.processor');
|
||||
let result = await processMonitorDownload({
|
||||
requestId: 'req-reloc',
|
||||
downloadHistoryId: 'dh-reloc',
|
||||
downloadClientId: 'hash-reloc',
|
||||
downloadClient: 'qbittorrent',
|
||||
jobId: 'job-reloc',
|
||||
});
|
||||
expect(result.completed).toBe(true);
|
||||
expect(jobQueueMock.addMonitorJob).not.toHaveBeenCalled();
|
||||
|
||||
// Forward-slash trailing form.
|
||||
vi.clearAllMocks();
|
||||
jobQueueMock.addNotificationJob.mockResolvedValue(undefined);
|
||||
downloadClientManagerMock.getClientServiceForProtocol.mockResolvedValue(
|
||||
relocationClientMock('/downloads/', '/downloads/Book.m4b')
|
||||
);
|
||||
stubCompletionDeps();
|
||||
|
||||
result = await processMonitorDownload({
|
||||
requestId: 'req-reloc',
|
||||
downloadHistoryId: 'dh-reloc',
|
||||
downloadClientId: 'hash-reloc',
|
||||
downloadClient: 'qbittorrent',
|
||||
jobId: 'job-reloc',
|
||||
});
|
||||
expect(result.completed).toBe(true);
|
||||
expect(jobQueueMock.addMonitorJob).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('leaves forward-slash (Linux/Docker) relocation behavior unchanged (#209)', async () => {
|
||||
// Already-relocated forward-slash path organizes.
|
||||
downloadClientManagerMock.getClientServiceForProtocol.mockResolvedValue(
|
||||
relocationClientMock('/downloads/readmeabook', '/downloads/readmeabook/Book')
|
||||
);
|
||||
stubCompletionDeps();
|
||||
|
||||
const { processMonitorDownload } = await import('@/lib/processors/monitor-download.processor');
|
||||
let result = await processMonitorDownload({
|
||||
requestId: 'req-reloc',
|
||||
downloadHistoryId: 'dh-reloc',
|
||||
downloadClientId: 'hash-reloc',
|
||||
downloadClient: 'qbittorrent',
|
||||
jobId: 'job-reloc',
|
||||
});
|
||||
expect(result.completed).toBe(true);
|
||||
expect(jobQueueMock.addMonitorJob).not.toHaveBeenCalled();
|
||||
|
||||
// Forward-slash genuine temp path still waits (protection preserved).
|
||||
vi.clearAllMocks();
|
||||
jobQueueMock.addNotificationJob.mockResolvedValue(undefined);
|
||||
downloadClientManagerMock.getClientServiceForProtocol.mockResolvedValue(
|
||||
relocationClientMock('/downloads/readmeabook', '/downloads/incomplete/Book')
|
||||
);
|
||||
prismaMock.request.update.mockResolvedValue({});
|
||||
prismaMock.downloadHistory.update.mockResolvedValue({});
|
||||
|
||||
result = await processMonitorDownload({
|
||||
requestId: 'req-reloc',
|
||||
downloadHistoryId: 'dh-reloc',
|
||||
downloadClientId: 'hash-reloc',
|
||||
downloadClient: 'qbittorrent',
|
||||
jobId: 'job-reloc',
|
||||
});
|
||||
expect(result).toMatchObject({ success: true, completed: false, pathWaitCount: 1 });
|
||||
expect(jobQueueMock.addMonitorJob).toHaveBeenCalledWith(
|
||||
'req-reloc', 'dh-reloc', 'hash-reloc', 'qbittorrent', 2, 100, 0, 1
|
||||
);
|
||||
});
|
||||
|
||||
it('treats an exact-equal single-file path as relocated (#209)', async () => {
|
||||
downloadClientManagerMock.getClientServiceForProtocol.mockResolvedValue(
|
||||
relocationClientMock('E:\\Torrents\\ReadMeABook\\Book.m4b', 'E:\\Torrents\\ReadMeABook\\Book.m4b')
|
||||
);
|
||||
stubCompletionDeps();
|
||||
|
||||
const { processMonitorDownload } = await import('@/lib/processors/monitor-download.processor');
|
||||
const result = await processMonitorDownload({
|
||||
requestId: 'req-reloc',
|
||||
downloadHistoryId: 'dh-reloc',
|
||||
downloadClientId: 'hash-reloc',
|
||||
downloadClient: 'qbittorrent',
|
||||
jobId: 'job-reloc',
|
||||
});
|
||||
|
||||
expect(result.completed).toBe(true);
|
||||
expect(jobQueueMock.addMonitorJob).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not match a sibling-prefix save path as relocated (#209)', async () => {
|
||||
// /downloads2 must NOT be treated as under /downloads — proves the `+ '/'` boundary.
|
||||
downloadClientManagerMock.getClientServiceForProtocol.mockResolvedValue(
|
||||
relocationClientMock('/downloads', '/downloads2/Book')
|
||||
);
|
||||
prismaMock.request.update.mockResolvedValue({});
|
||||
prismaMock.downloadHistory.update.mockResolvedValue({});
|
||||
|
||||
const { processMonitorDownload } = await import('@/lib/processors/monitor-download.processor');
|
||||
const result = await processMonitorDownload({
|
||||
requestId: 'req-reloc',
|
||||
downloadHistoryId: 'dh-reloc',
|
||||
downloadClientId: 'hash-reloc',
|
||||
downloadClient: 'qbittorrent',
|
||||
jobId: 'job-reloc',
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({ success: true, completed: false, pathWaitCount: 1 });
|
||||
expect(jobQueueMock.addOrganizeJob).not.toHaveBeenCalled();
|
||||
expect(jobQueueMock.addMonitorJob).toHaveBeenCalledWith(
|
||||
'req-reloc', 'dh-reloc', 'hash-reloc', 'qbittorrent', 2, 100, 0, 1
|
||||
);
|
||||
});
|
||||
|
||||
it('converts SABnzbd progress from 0.0-1.0 to 0-100 percentage', async () => {
|
||||
const sabClientMock = {
|
||||
clientType: 'sabnzbd',
|
||||
|
||||
@@ -46,6 +46,27 @@ describe('PathMapper', () => {
|
||||
).toThrow('Local path cannot be empty');
|
||||
});
|
||||
|
||||
describe('normalizePath', () => {
|
||||
it('converts backslashes to forward slashes (Windows clients)', () => {
|
||||
expect(PathMapper.normalizePath('E:\\Torrents\\ReadMeABook\\Book.m4b'))
|
||||
.toBe('E:/Torrents/ReadMeABook/Book.m4b');
|
||||
});
|
||||
|
||||
it('normalizes mixed separators consistently', () => {
|
||||
expect(PathMapper.normalizePath('E:\\Torrents/ReadMeABook\\Book'))
|
||||
.toBe('E:/Torrents/ReadMeABook/Book');
|
||||
});
|
||||
|
||||
it('strips trailing separators (both forms)', () => {
|
||||
expect(PathMapper.normalizePath('E:\\Torrents\\ReadMeABook\\')).toBe('E:/Torrents/ReadMeABook');
|
||||
expect(PathMapper.normalizePath('/downloads/readmeabook/')).toBe('/downloads/readmeabook');
|
||||
});
|
||||
|
||||
it('collapses `..` segments and redundant separators', () => {
|
||||
expect(PathMapper.normalizePath('/downloads//readmeabook/../Book')).toBe('/downloads/Book');
|
||||
});
|
||||
});
|
||||
|
||||
describe('reverseTransform', () => {
|
||||
it('returns original path when mapping is disabled', () => {
|
||||
const result = PathMapper.reverseTransform('/downloads/Book', {
|
||||
|
||||
Reference in New Issue
Block a user