From 70bebb9a56293b6e746c69654fd0dca94ecccd3a Mon Sep 17 00:00:00 2001 From: Reece Browne Date: Tue, 5 Aug 2025 14:29:57 +0100 Subject: [PATCH] Additional clean up --- CLAUDE.md | 60 ++++++++++------ .../tools/compress/useCompressOperation.ts | 5 +- .../tools/convert/useConvertOperation.ts | 72 +++++++++++++------ .../src/hooks/tools/ocr/useOCROperation.ts | 4 +- .../src/hooks/tools/shared/useToolApiCalls.ts | 2 - .../hooks/tools/shared/useToolOperation.ts | 67 +++++++++++------ .../hooks/tools/split/useSplitOperation.ts | 3 +- .../services/enhancedPDFProcessingService.ts | 5 +- frontend/src/types/processing.ts | 1 - 9 files changed, 144 insertions(+), 75 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 894c981b8..be4e92201 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,31 +77,51 @@ Without cleanup: browser crashes with memory leaks. - **toolResponseProcessor**: API response handling (single/zip/custom) - **toolOperationTracker**: FileContext integration utilities -**Tool Implementation Pattern**: -1. Create hook in `frontend/src/hooks/tools/[toolname]/use[ToolName]Operation.ts` -2. Define parameters interface and validation -3. Implement `buildFormData` function for API requests -4. Configure `useToolOperation` with endpoints and settings -5. UI components consume the hook's state and actions +**Three Tool Patterns**: -**Example Pattern** (see `useCompressOperation.ts`): +**Pattern 1: Single-File Tools** (Individual processing) +- Backend processes one file per API call +- Set `multiFileEndpoint: false` +- Examples: Compress, Rotate ```typescript -export const useCompressOperation = () => { - const { t } = useTranslation(); - - return useToolOperation({ - operationType: 'compress', - endpoint: '/api/v1/misc/compress-pdf', - buildFormData, - filePrefix: 'compressed_', - validateParams: (params) => { /* validation logic */ }, - getErrorMessage: createStandardErrorHandler(t('compress.error.failed')) - }); -}; +return useToolOperation({ + operationType: 'compress', + endpoint: '/api/v1/misc/compress-pdf', + buildFormData: (params, file: File) => { /* single file */ }, + multiFileEndpoint: false, + filePrefix: 'compressed_' +}); +``` + +**Pattern 2: Multi-File Tools** (Batch processing) +- Backend accepts `MultipartFile[]` arrays in single API call +- Set `multiFileEndpoint: true` +- Examples: Split, Merge, Overlay +```typescript +return useToolOperation({ + operationType: 'split', + endpoint: '/api/v1/general/split-pages', + buildFormData: (params, files: File[]) => { /* all files */ }, + multiFileEndpoint: true, + filePrefix: 'split_' +}); +``` + +**Pattern 3: Complex Tools** (Custom processing) +- Tools with complex routing logic or non-standard processing +- Provide `customProcessor` for full control +- Examples: Convert, OCR +```typescript +return useToolOperation({ + operationType: 'convert', + customProcessor: async (params, files) => { /* custom logic */ }, + filePrefix: 'converted_' +}); ``` **Benefits**: -- **Consistent**: All tools follow same pattern and interface +- **No Timeouts**: Operations run until completion (supports 100GB+ files) +- **Consistent**: All tools follow same pattern and interface - **Maintainable**: Single responsibility hooks, easy to test and modify - **i18n Ready**: Built-in internationalization support - **Type Safe**: Full TypeScript support with generic interfaces diff --git a/frontend/src/hooks/tools/compress/useCompressOperation.ts b/frontend/src/hooks/tools/compress/useCompressOperation.ts index fc28f54d2..05ea40423 100644 --- a/frontend/src/hooks/tools/compress/useCompressOperation.ts +++ b/frontend/src/hooks/tools/compress/useCompressOperation.ts @@ -31,14 +31,13 @@ const buildFormData = (parameters: CompressParameters, file: File): FormData => export const useCompressOperation = () => { const { t } = useTranslation(); - + return useToolOperation({ operationType: 'compress', endpoint: '/api/v1/misc/compress-pdf', buildFormData, filePrefix: 'compressed_', - singleFileMode: false, // Process files individually - timeout: 60000, // 1 minute timeout per file +1 multiFileEndpoint: false, // Individual API calls per file validateParams: (params) => { if (params.compressionMethod === 'filesize' && !params.fileSizeValue) { return { valid: false, errors: [t('compress.validation.fileSizeRequired', 'File size value is required when using filesize method')] }; diff --git a/frontend/src/hooks/tools/convert/useConvertOperation.ts b/frontend/src/hooks/tools/convert/useConvertOperation.ts index e69f072b5..3aeb77222 100644 --- a/frontend/src/hooks/tools/convert/useConvertOperation.ts +++ b/frontend/src/hooks/tools/convert/useConvertOperation.ts @@ -5,10 +5,8 @@ import { ConvertParameters } from './useConvertParameters'; import { detectFileExtension } from '../../../utils/fileUtils'; import { createFileFromApiResponse } from '../../../utils/fileResponseUtils'; import { useToolOperation, ToolOperationConfig } from '../shared/useToolOperation'; - import { getEndpointUrl, isImageFormat, isWebFormat } from '../../../utils/convertUtils'; - const shouldProcessFilesSeparately = ( selectedFiles: File[], parameters: ConvertParameters @@ -31,18 +29,6 @@ const shouldProcessFilesSeparately = ( ); }; -const createFileFromResponse = ( - responseData: any, - headers: any, - originalFileName: string, - targetExtension: string -): File => { - const originalName = originalFileName.split('.')[0]; - const fallbackFilename = `${originalName}_converted.${targetExtension}`; - - return createFileFromApiResponse(responseData, headers, fallbackFilename); -}; - const buildFormData = (parameters: ConvertParameters, selectedFiles: File[]): FormData => { const formData = new FormData(); @@ -83,20 +69,66 @@ const buildFormData = (parameters: ConvertParameters, selectedFiles: File[]): Fo return formData; }; +const createFileFromResponse = ( + responseData: any, + headers: any, + originalFileName: string, + targetExtension: string +): File => { + const originalName = originalFileName.split('.')[0]; + const fallbackFilename = `${originalName}_converted.${targetExtension}`; + + return createFileFromApiResponse(responseData, headers, fallbackFilename); +}; + export const useConvertOperation = () => { const { t } = useTranslation(); + const customConvertProcessor = useCallback(async ( + parameters: ConvertParameters, + selectedFiles: File[] + ): Promise => { + const processedFiles: File[] = []; + const endpoint = getEndpointUrl(parameters.fromExtension, parameters.toExtension); + + if (!endpoint) { + throw new Error('Unsupported conversion format'); + } + + // Convert-specific routing logic: decide batch vs individual processing + if (shouldProcessFilesSeparately(selectedFiles, parameters)) { + // Individual processing for complex cases (PDF→image, smart detection, etc.) + for (const file of selectedFiles) { + const formData = buildFormData(parameters, [file]); + const response = await axios.post(endpoint, formData, { responseType: 'blob' }); + + const convertedFile = createFileFromResponse(response.data, response.headers, file.name, parameters.toExtension); + + processedFiles.push(convertedFile); + } + } else { + // Batch processing for simple cases (image→PDF combine) + const formData = buildFormData(parameters, selectedFiles); + const response = await axios.post(endpoint, formData, { responseType: 'blob' }); + + const baseFilename = selectedFiles.length === 1 + ? selectedFiles[0].name + : 'converted_files'; + + const convertedFile = createFileFromResponse(response.data, response.headers, baseFilename, parameters.toExtension); + processedFiles.push(convertedFile); + } + + return processedFiles; + }, [t]); return useToolOperation({ operationType: 'convert', - endpoint: (params) => getEndpointUrl(params.fromExtension, params.toExtension) || '', - buildFormData: buildFormData, // Clean multi-file signature: (params, selectedFiles) => FormData + endpoint: '', // Not used with customProcessor but required + buildFormData, // Not used with customProcessor but required filePrefix: 'converted_', - responseHandler: { - type: 'single' - }, + customProcessor: customConvertProcessor, // Convert handles its own routing validateParams: (params) => { - // Add any validation if needed return { valid: true }; }, getErrorMessage: (error) => { diff --git a/frontend/src/hooks/tools/ocr/useOCROperation.ts b/frontend/src/hooks/tools/ocr/useOCROperation.ts index 326756f69..ecfe32fd7 100644 --- a/frontend/src/hooks/tools/ocr/useOCROperation.ts +++ b/frontend/src/hooks/tools/ocr/useOCROperation.ts @@ -87,8 +87,7 @@ export const useOCROperation = () => { try { const formData = buildFormData(file, parameters); const response = await axios.post('/api/v1/misc/ocr-pdf', formData, { - responseType: "blob", - timeout: 300000 // 5 minute timeout for OCR + responseType: "blob" }); // Check for HTTP errors @@ -174,7 +173,6 @@ export const useOCROperation = () => { buildFormData, // Not used with customProcessor but required filePrefix: 'ocr_', customProcessor: customOCRProcessor, - timeout: 300000, // 5 minute timeout for OCR validateParams: (params) => { if (params.languages.length === 0) { return { valid: false, errors: [t('ocr.validation.languageRequired', 'Please select at least one language for OCR processing.')] }; diff --git a/frontend/src/hooks/tools/shared/useToolApiCalls.ts b/frontend/src/hooks/tools/shared/useToolApiCalls.ts index fa21cda07..639751063 100644 --- a/frontend/src/hooks/tools/shared/useToolApiCalls.ts +++ b/frontend/src/hooks/tools/shared/useToolApiCalls.ts @@ -8,7 +8,6 @@ export interface ApiCallsConfig { buildFormData: (file: File, params: TParams) => FormData; filePrefix: string; responseHandler?: ResponseHandler; - timeout?: number; } export const useToolApiCalls = () => { @@ -39,7 +38,6 @@ export const useToolApiCalls = () => { const endpoint = typeof config.endpoint === 'function' ? config.endpoint(params) : config.endpoint; const response = await axios.post(endpoint, formData, { responseType: 'blob', - timeout: config.timeout || 120000, cancelToken: cancelTokenRef.current.token }); diff --git a/frontend/src/hooks/tools/shared/useToolOperation.ts b/frontend/src/hooks/tools/shared/useToolOperation.ts index 7ea25d95c..55252f05a 100644 --- a/frontend/src/hooks/tools/shared/useToolOperation.ts +++ b/frontend/src/hooks/tools/shared/useToolOperation.ts @@ -18,38 +18,57 @@ export interface ValidationResult { export type { ProcessingProgress, ResponseHandler }; /** - * Configuration for tool operations defining processing behavior and API integration + * Configuration for tool operations defining processing behavior and API integration. + * + * Supports three patterns: + * 1. Single-file tools: multiFileEndpoint: false, processes files individually + * 2. Multi-file tools: multiFileEndpoint: true, single API call with all files + * 3. Complex tools: customProcessor handles all processing logic */ export interface ToolOperationConfig { /** Operation identifier for tracking and logging */ operationType: string; - /** API endpoint for the operation (can be string or function for dynamic endpoints) */ + /** + * API endpoint for the operation. Can be static string or function for dynamic routing. + * Not used when customProcessor is provided. + */ endpoint: string | ((params: TParams) => string); - /** Builds FormData for API request - signature indicates single-file vs multi-file capability */ + /** + * Builds FormData for API request. Signature determines processing approach: + * - (params, file: File) => FormData: Single-file processing + * - (params, files: File[]) => FormData: Multi-file processing + * Not used when customProcessor is provided. + */ buildFormData: ((params: TParams, file: File) => FormData) | ((params: TParams, files: File[]) => FormData); - /** Prefix for processed filenames (e.g., 'compressed_', 'repaired_') */ + /** Prefix added to processed filenames (e.g., 'compressed_', 'split_') */ filePrefix: string; - /** How to handle API responses */ + /** + * Whether this tool uses backends that accept MultipartFile[] arrays. + * - true: Single API call with all files (backend uses MultipartFile[]) + * - false/undefined: Individual API calls per file (backend uses single MultipartFile) + * Ignored when customProcessor is provided. + */ + multiFileEndpoint?: boolean; + + /** How to handle API responses (e.g., ZIP extraction, single file response) */ responseHandler?: ResponseHandler; - /** Process files individually or as a batch */ - singleFileMode?: boolean; - - /** Custom processing logic that bypasses default file processing */ + /** + * Custom processing logic that completely bypasses standard file processing. + * When provided, tool handles all API calls, response processing, and file creation. + * Use for tools with complex routing logic or non-standard processing requirements. + */ customProcessor?: (params: TParams, files: File[]) => Promise; - /** Validate parameters before execution */ + /** Validate parameters before execution. Return validation errors if invalid. */ validateParams?: (params: TParams) => ValidationResult; - /** Extract user-friendly error messages */ + /** Extract user-friendly error messages from API errors */ getErrorMessage?: (error: any) => string; - - /** Request timeout in milliseconds */ - timeout?: number; } /** @@ -78,8 +97,16 @@ export interface ToolOperationHook { export { createStandardErrorHandler } from '../../../utils/toolErrorHandler'; /** - * Shared hook for tool operations with consistent error handling, progress tracking, + * Shared hook for tool operations providing consistent error handling, progress tracking, * and FileContext integration. Eliminates boilerplate while maintaining flexibility. + * + * Supports three tool patterns: + * 1. Single-file tools: Set multiFileEndpoint: false, processes files individually + * 2. Multi-file tools: Set multiFileEndpoint: true, single API call with all files + * 3. Complex tools: Provide customProcessor for full control over processing logic + * + * @param config - Tool operation configuration + * @returns Hook interface with state and execution methods */ export const useToolOperation = ( config: ToolOperationConfig @@ -133,11 +160,8 @@ export const useToolOperation = ( actions.setStatus('Processing files...'); processedFiles = await config.customProcessor(params, validFiles); } else { - // Detect if buildFormData signature is multi-file or single-file - // Both have 2 params now, so check if second param expects an array - const isMultiFileFormData = /files|selectedFiles/.test(config.buildFormData.toString()); - - if (isMultiFileFormData) { + // Use explicit multiFileEndpoint flag to determine processing approach + if (config.multiFileEndpoint) { // Multi-file processing - single API call with all files actions.setStatus('Processing files...'); const formData = (config.buildFormData as (params: TParams, files: File[]) => FormData)(params, validFiles); @@ -164,8 +188,7 @@ export const useToolOperation = ( endpoint: config.endpoint, buildFormData: (file: File, params: TParams) => (config.buildFormData as (params: TParams, file: File) => FormData)(params, file), filePrefix: config.filePrefix, - responseHandler: config.responseHandler, - timeout: config.timeout + responseHandler: config.responseHandler }; processedFiles = await processFiles( params, diff --git a/frontend/src/hooks/tools/split/useSplitOperation.ts b/frontend/src/hooks/tools/split/useSplitOperation.ts index 4a4d01b5e..9994c23c7 100644 --- a/frontend/src/hooks/tools/split/useSplitOperation.ts +++ b/frontend/src/hooks/tools/split/useSplitOperation.ts @@ -63,8 +63,9 @@ export const useSplitOperation = () => { return useToolOperation({ operationType: 'split', endpoint: (params) => getEndpoint(params), - buildFormData: buildFormData, // Clean multi-file signature: (params, selectedFiles) => FormData + buildFormData: buildFormData, // Multi-file signature: (params, selectedFiles) => FormData filePrefix: 'split_', + multiFileEndpoint: true, // Single API call with all files responseHandler: { type: 'zip', useZipExtractor: true diff --git a/frontend/src/services/enhancedPDFProcessingService.ts b/frontend/src/services/enhancedPDFProcessingService.ts index ea825e353..984d106e4 100644 --- a/frontend/src/services/enhancedPDFProcessingService.ts +++ b/frontend/src/services/enhancedPDFProcessingService.ts @@ -28,8 +28,7 @@ export class EnhancedPDFProcessingService { thumbnailQuality: 'medium', priorityPageCount: 10, useWebWorker: false, - maxRetries: 3, - timeoutMs: 300000 // 5 minutes + maxRetries: 3 }; private constructor() {} @@ -87,7 +86,7 @@ export class EnhancedPDFProcessingService { estimatedTime: number ): Promise { // Create cancellation token - const cancellationToken = ProcessingErrorHandler.createTimeoutController(config.timeoutMs); + const cancellationToken = new AbortController(); // Set initial state const state: ProcessingState = { diff --git a/frontend/src/types/processing.ts b/frontend/src/types/processing.ts index 65b996d7f..ee2f5be17 100644 --- a/frontend/src/types/processing.ts +++ b/frontend/src/types/processing.ts @@ -69,7 +69,6 @@ export interface ProcessingConfig { priorityPageCount: number; // Number of priority pages to process first useWebWorker: boolean; maxRetries: number; - timeoutMs: number; } export interface FileAnalysis {