Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/SPLITTER_WORKER_DESIGN.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,8 @@ npm install pdf-to-png-converter

**基本用法**:

> ⚠️ `outputFolder` 会被库按 `path.join(process.cwd(), outputFolder)` 解析,无法指向 Windows 上其他盘符或 UNC 路径下的目录(见 #75)。实际实现(`PDFSplitter.ts`)不传 `outputFolder`,而是取返回的 `content` 自行写入 `{uploadsDir}/{taskId}/split/`。本文档中传 `outputFolder` 的示例仅用于说明库的 API 或早期设计。

```typescript
import { pdfToPng } from "pdf-to-png-converter";

Expand Down
5 changes: 4 additions & 1 deletion docs/TESTING_GUIDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,17 @@ npm run test:coverage
| `src/core/infrastructure/adapters/llm/__tests__/LLMClient.test.ts` | 15 | Base class backward compatibility |
| `src/core/infrastructure/adapters/llm/__tests__/LLMClientFactory.test.ts` | 8 | Client factory creation |

#### Splitter Adapters (4 files)
#### Splitter Adapters (7 files)

| File | Description |
|------|-------------|
| `src/core/infrastructure/adapters/split/__tests__/PDFSplitter.test.ts` | PDF splitting functionality |
| `src/core/infrastructure/adapters/split/__tests__/PDFSplitter.win32.test.ts` | PDF output location with Windows path semantics (cross-drive, UNC, same drive) on any host OS |
| `src/core/infrastructure/adapters/split/__tests__/PDFSplitter.integration.test.ts` | PDF splitting with the real converter and file system |
| `src/core/infrastructure/adapters/split/__tests__/ImageSplitter.test.ts` | Image splitting functionality |
| `src/core/infrastructure/adapters/split/__tests__/SplitterFactory.test.ts` | Splitter factory creation |
| `src/core/infrastructure/adapters/split/__tests__/ImagePathUtil.test.ts` | Image path utilities |
| `src/core/infrastructure/adapters/split/__tests__/FileWaitUtil.test.ts` | Waiting for uploaded source files to become available |

#### Infrastructure Services (2 files)

Expand Down
141 changes: 83 additions & 58 deletions src/core/infrastructure/adapters/split/PDFSplitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import { ImagePathUtil } from './ImagePathUtil.js';
import { FileWaitUtil } from './FileWaitUtil.js';
import { WORKER_CONFIG } from '../../config/worker.config.js';

/** A rendered page, held in memory until it is written to the split directory. */
type RenderedPage = Omit<PageInfo, 'imagePath'> & { content: Buffer };

/**
* PDF splitter implementation using pdf-to-png-converter.
*
Expand All @@ -17,6 +20,8 @@ import { WORKER_CONFIG } from '../../config/worker.config.js';
* - Retry logic for transient errors (3 attempts with exponential backoff)
* - Graceful handling of non-retryable errors (password-protected PDFs)
* - Sequential page numbering (page-1.png, page-2.png, ...)
* - Output location independent of process.cwd() (pages are written by this class,
* see renderPagesWithRetry)
*/
export class PDFSplitter implements ISplitter {
private readonly uploadsDir: string;
Expand Down Expand Up @@ -48,29 +53,29 @@ export class PDFSplitter implements ISplitter {
// Retry with short delays to handle antivirus scanning or filesystem sync delays
await FileWaitUtil.waitForFile(sourcePath, this.uploadsDir, taskId, filename, 'PDFSplitter');

let renderedPages: RenderedPage[];
try {
// Step 1: Get total page count with retry
const totalPages = await this.getPDFPageCountWithRetry(sourcePath);

// Step 2: Parse page range (defaults to all pages if not specified)
const pageNumbers = PageRangeParser.parse(task.page_range, totalPages);

// Step 3: Convert specified pages with retry
// Step 3: Render specified pages with retry
// Pass the page_range string to determine if user explicitly specified pages
const pages = await this.convertPagesWithRetry(
sourcePath,
taskId,
pageNumbers,
task.page_range
);

return {
pages,
totalPages: pages.length,
};
renderedPages = await this.renderPagesWithRetry(sourcePath, pageNumbers, task.page_range);
} catch (error) {
throw this.wrapError(error, taskId, filename);
throw await this.wrapError(error, sourcePath, filename);
}

// Step 4: Write the images. Kept apart from the steps above so that an output
// failure is never reported as a problem with the source PDF.
const pages = await this.writePagesWithRetry(taskId, renderedPages);

return {
pages,
totalPages: pages.length,
};
}

/**
Expand Down Expand Up @@ -99,36 +104,28 @@ export class PDFSplitter implements ISplitter {
}

/**
* Convert specified pages with retry logic.
* Render specified pages to in-memory PNGs with retry logic.
*/
private async convertPagesWithRetry(
private async renderPagesWithRetry(
pdfPath: string,
taskId: string,
pageNumbers: number[],
pageRange?: string | null
): Promise<PageInfo[]> {
const taskDir = ImagePathUtil.getTaskDir(taskId);

// Ensure task directory exists
await fs.mkdir(taskDir, { recursive: true });

): Promise<RenderedPage[]> {
return this.withRetry(async () => {
// Convert all specified pages
// IMPORTANT: pdf-to-png-converter expects a RELATIVE path
// It will internally do: path.join(process.cwd(), outputFolder)
// So we must convert absolute path to relative path
const relativeOutputFolder = path.relative(process.cwd(), taskDir);

// IMPORTANT: do not pass outputFolder to pdf-to-png-converter. It resolves it with
// path.join(process.cwd(), outputFolder), which cannot address a directory on another
// Windows drive or UNC share (#75). We take the PNG buffers and write them ourselves.
const options: any = {
outputFolder: relativeOutputFolder,
returnPageContent: true,
viewportScale: WORKER_CONFIG.splitter.viewportScale,
strictPagesToProcess: false,
verbosityLevel: 0,
};

// Only add pagesToProcess if user explicitly specified a page range
// If pageRange is empty/null, let pdf-to-png-converter process all pages
if (pageRange && pageRange.trim() !== '') {
const hasPageRange = Boolean(pageRange && pageRange.trim() !== '');
if (hasPageRange) {
options.pagesToProcess = pageNumbers; // Array of page numbers
}
// Otherwise, don't specify pagesToProcess to convert all pages
Expand All @@ -139,29 +136,45 @@ export class PDFSplitter implements ISplitter {
throw new Error('PDF conversion produced no output');
}

// Rename files to page-{N}.png format and build PageInfo array
const pages: PageInfo[] = [];

for (let i = 0; i < result.length; i++) {
const pageNum = i + 1; // Sequential numbering
// If page range was specified, use the mapped page numbers; otherwise use sequential numbering
const sourcePageNum = pageRange && pageRange.trim() !== '' ? pageNumbers[i] : pageNum;
const targetPath = ImagePathUtil.getPath(taskId, pageNum);

// Rename from temporary name to standard format
if (result[i].path !== targetPath) {
await fs.rename(result[i].path, targetPath);
return result.map((output, i) => {
const page = i + 1; // Sequential numbering
if (!output.content) {
throw new Error(`PDF conversion produced no image for page ${page}`);
}
return {
page,
// If page range was specified, use the mapped page numbers; otherwise use sequential numbering
pageSource: hasPageRange ? pageNumbers[i] : page,
content: output.content,
};
});
}, 'convert PDF pages');
}

pages.push({
page: pageNum,
pageSource: sourcePageNum,
imagePath: targetPath,
});
}
/**
* Write rendered pages to the task split directory as page-{N}.png with retry logic.
*/
private async writePagesWithRetry(
taskId: string,
renderedPages: RenderedPage[]
): Promise<PageInfo[]> {
const taskDir = ImagePathUtil.getTaskDir(taskId);

return pages;
}, 'convert PDF pages');
try {
return await this.withRetry(async () => {
await fs.mkdir(taskDir, { recursive: true });

const pages: PageInfo[] = [];
for (const { page, pageSource, content } of renderedPages) {
const imagePath = ImagePathUtil.getPath(taskId, page);
await fs.writeFile(imagePath, content);
pages.push({ page, pageSource, imagePath });
}
return pages;
}, 'write page images');
} catch (error) {
throw new Error(`Failed to write split images to ${taskDir}: ${(error as Error).message}`);
}
}

/**
Expand Down Expand Up @@ -215,10 +228,19 @@ export class PDFSplitter implements ISplitter {
}

/**
* Wrap errors with friendly, actionable messages.
* Wrap source read/render errors with friendly, actionable messages.
*/
private wrapError(error: unknown, _taskId: string, filename: string): Error {
private async wrapError(error: unknown, sourcePath: string, filename: string): Promise<Error> {
const err = error as Error;

// Ask the file system instead of matching ENOENT in the message: an ENOENT can
// also come from other files, and the message embeds paths with arbitrary words.
if (await this.isFileMissing(sourcePath)) {
return new Error(
`PDF file not found: ${filename}. The file may have been moved or deleted.`
);
}

const message = err.message.toLowerCase();

if (message.includes('password') || message.includes('encrypted')) {
Expand All @@ -233,16 +255,19 @@ export class PDFSplitter implements ISplitter {
);
}

if (message.includes('enoent') || message.includes('file not found')) {
return new Error(
`PDF file not found: ${filename}. The file may have been moved or deleted.`
);
}

// Generic error with context
return new Error(`Failed to split PDF ${filename}: ${err.message}`);
}

private async isFileMissing(filePath: string): Promise<boolean> {
try {
await fs.access(filePath);
return false;
} catch (error) {
return (error as NodeJS.ErrnoException).code === 'ENOENT';
}
}

/**
* Clean up temporary files for a task.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { promises as fs } from 'fs';
import os from 'os';
import path from 'path';
import { PDFDocument } from 'pdf-lib';
import { PDFSplitter } from '../PDFSplitter.js';
import { ImagePathUtil } from '../ImagePathUtil.js';

/**
* Runs against the real pdf-to-png-converter, pdf-lib and file system, because
* the unit tests mock the library whose path handling caused issue #75.
*/
describe('PDFSplitter with the real converter', () => {
const TASK_ID = 'task-1';
const PNG_SIGNATURE = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
const PAGE_SIZE_PT = 100;

let workDir: string;
let uploadsDir: string;
let cwd: string;
let splitDir: string;
let splitter: PDFSplitter;

beforeEach(async () => {
workDir = await fs.mkdtemp(path.join(os.tmpdir(), 'pdf-splitter-'));
uploadsDir = path.join(workDir, 'userData', 'files');
cwd = path.join(workDir, 'install-dir');
splitDir = path.join(uploadsDir, TASK_ID, 'split');
await fs.mkdir(path.join(uploadsDir, TASK_ID), { recursive: true });
await fs.mkdir(cwd);

const pdf = await PDFDocument.create();
pdf.addPage([PAGE_SIZE_PT, PAGE_SIZE_PT]);
pdf.addPage([PAGE_SIZE_PT, PAGE_SIZE_PT]);
await fs.writeFile(path.join(uploadsDir, TASK_ID, 'doc.pdf'), await pdf.save());

vi.spyOn(process, 'cwd').mockReturnValue(cwd);
ImagePathUtil.init(uploadsDir);
splitter = new PDFSplitter(uploadsDir);
});

afterEach(async () => {
ImagePathUtil.reset();
vi.restoreAllMocks();
await fs.rm(workDir, { recursive: true, force: true });
});

it('should render every page to the split dir and leave nothing in cwd', async () => {
const result = await splitter.split({ id: TASK_ID, filename: 'doc.pdf', page_range: '' });

expect(result.pages).toEqual([
{ page: 1, pageSource: 1, imagePath: path.join(splitDir, 'page-1.png') },
{ page: 2, pageSource: 2, imagePath: path.join(splitDir, 'page-2.png') },
]);
expect((await fs.readdir(splitDir)).sort()).toEqual(['page-1.png', 'page-2.png']);
expect(await fs.readdir(cwd)).toEqual([]);

const png = await fs.readFile(path.join(splitDir, 'page-1.png'));
expect(png.subarray(0, PNG_SIGNATURE.length).equals(PNG_SIGNATURE)).toBe(true);
// IHDR width: the configured viewport scale (2.0) must still be applied.
expect(png.readUInt32BE(16)).toBe(PAGE_SIZE_PT * 2);
});

it('should number pages sequentially and keep the source page for a page range', async () => {
const result = await splitter.split({ id: TASK_ID, filename: 'doc.pdf', page_range: '2' });

expect(result.pages).toEqual([
{ page: 1, pageSource: 2, imagePath: path.join(splitDir, 'page-1.png') },
]);
expect(await fs.readdir(splitDir)).toEqual(['page-1.png']);
});

it('should not blame the source PDF when the split dir cannot be created', async () => {
vi.spyOn(splitter as any, 'sleep').mockResolvedValue(undefined);
vi.spyOn(console, 'warn').mockImplementation(() => {});
// A regular file in place of the split dir makes the real mkdir fail.
await fs.writeFile(splitDir, '');

const failure = splitter.split({ id: TASK_ID, filename: 'doc.pdf', page_range: '' });

await expect(failure).rejects.toThrow(`Failed to write split images to ${splitDir}`);
await expect(failure).rejects.not.toThrow(/moved or deleted/);
});
});
Loading
Loading