From 70e10a4f93eb67f73816acfaed477cce54b3178b Mon Sep 17 00:00:00 2001 From: James Brunton Date: Mon, 18 Aug 2025 15:45:05 +0100 Subject: [PATCH] Redesign ToolOperationConfig typing --- .../useAddPasswordOperation.test.ts | 8 +- .../addPassword/useAddPasswordOperation.ts | 6 +- .../useChangePermissionsOperation.test.ts | 8 +- .../useChangePermissionsOperation.ts | 6 +- .../tools/compress/useCompressOperation.ts | 4 +- .../tools/convert/useConvertOperation.ts | 9 +- .../src/hooks/tools/ocr/useOCROperation.ts | 4 +- .../useRemovePasswordOperation.test.ts | 6 +- .../useRemovePasswordOperation.ts | 4 +- .../tools/sanitize/useSanitizeOperation.ts | 4 +- .../hooks/tools/shared/useToolOperation.ts | 127 ++++++++++-------- .../hooks/tools/split/useSplitOperation.ts | 4 +- 12 files changed, 104 insertions(+), 86 deletions(-) diff --git a/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.test.ts b/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.test.ts index 747cd67b7..91e36a854 100644 --- a/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.test.ts +++ b/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.test.ts @@ -20,13 +20,13 @@ vi.mock('../../../utils/toolErrorHandler', () => ({ })); // Import the mocked function -import { ToolOperationConfig, ToolOperationHook, useToolOperation } from '../shared/useToolOperation'; +import { SingleFileToolOperationConfig, ToolOperationHook, useToolOperation } from '../shared/useToolOperation'; describe('useAddPasswordOperation', () => { const mockUseToolOperation = vi.mocked(useToolOperation); - const getToolConfig = (): ToolOperationConfig => mockUseToolOperation.mock.calls[0][0]; + const getToolConfig = () => mockUseToolOperation.mock.calls[0][0] as SingleFileToolOperationConfig; const mockToolOperationReturn: ToolOperationHook = { files: [], @@ -91,7 +91,7 @@ describe('useAddPasswordOperation', () => { }; const testFile = new File(['test content'], 'test.pdf', { type: 'application/pdf' }); - const formData = buildFormData(testParameters, testFile as any /* FIX ME */); + const formData = buildFormData(testParameters, testFile); // Verify the form data contains the file expect(formData.get('fileInput')).toBe(testFile); @@ -112,7 +112,7 @@ describe('useAddPasswordOperation', () => { }); test.each([ - { property: 'multiFileEndpoint' as const, expectedValue: false }, + { property: 'toolType' as const, expectedValue: 'singleFile' }, { property: 'endpoint' as const, expectedValue: '/api/v1/security/add-password' }, { property: 'filePrefix' as const, expectedValue: 'translated-addPassword.filenamePrefix_' }, { property: 'operationType' as const, expectedValue: 'addPassword' } diff --git a/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.ts b/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.ts index d94b9650e..c4f6ef828 100644 --- a/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.ts +++ b/frontend/src/hooks/tools/addPassword/useAddPasswordOperation.ts @@ -20,11 +20,11 @@ export const useAddPasswordOperation = () => { }; return useToolOperation({ + toolType: 'singleFile', + buildFormData, operationType: 'addPassword', endpoint: '/api/v1/security/add-password', - buildFormData, filePrefix: t('addPassword.filenamePrefix', 'encrypted') + '_', - multiFileEndpoint: false, - getErrorMessage: createStandardErrorHandler(t('addPassword.error.failed', 'An error occurred while encrypting the PDF.')) + getErrorMessage: createStandardErrorHandler(t('addPassword.error.failed', 'An error occurred while encrypting the PDF.')), }); }; diff --git a/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.test.ts b/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.test.ts index 845403841..ff5e69d6b 100644 --- a/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.test.ts +++ b/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.test.ts @@ -20,12 +20,12 @@ vi.mock('../../../utils/toolErrorHandler', () => ({ })); // Import the mocked function -import { ToolOperationConfig, ToolOperationHook, useToolOperation } from '../shared/useToolOperation'; +import { SingleFileToolOperationConfig, ToolOperationHook, useToolOperation } from '../shared/useToolOperation'; describe('useChangePermissionsOperation', () => { const mockUseToolOperation = vi.mocked(useToolOperation); - const getToolConfig = (): ToolOperationConfig => mockUseToolOperation.mock.calls[0][0]; + const getToolConfig = () => mockUseToolOperation.mock.calls[0][0] as SingleFileToolOperationConfig; const mockToolOperationReturn: ToolOperationHook = { files: [], @@ -86,7 +86,7 @@ describe('useChangePermissionsOperation', () => { const buildFormData = callArgs.buildFormData; const testFile = new File(['test content'], 'test.pdf', { type: 'application/pdf' }); - const formData = buildFormData(testParameters, testFile as any /* FIX ME */); + const formData = buildFormData(testParameters, testFile); // Verify the form data contains the file expect(formData.get('fileInput')).toBe(testFile); @@ -106,7 +106,7 @@ describe('useChangePermissionsOperation', () => { }); test.each([ - { property: 'multiFileEndpoint' as const, expectedValue: false }, + { property: 'toolType' as const, expectedValue: 'singleFile' }, { property: 'endpoint' as const, expectedValue: '/api/v1/security/add-password' }, { property: 'filePrefix' as const, expectedValue: 'permissions_' }, { property: 'operationType' as const, expectedValue: 'changePermissions' } diff --git a/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.ts b/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.ts index 664a423ff..c4f55adc7 100644 --- a/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.ts +++ b/frontend/src/hooks/tools/changePermissions/useChangePermissionsOperation.ts @@ -25,13 +25,13 @@ export const useChangePermissionsOperation = () => { }; return useToolOperation({ + toolType: 'singleFile', + buildFormData, operationType: 'changePermissions', endpoint: '/api/v1/security/add-password', // Change Permissions is a fake endpoint for the Add Password tool - buildFormData, filePrefix: 'permissions_', - multiFileEndpoint: false, getErrorMessage: createStandardErrorHandler( t('changePermissions.error.failed', 'An error occurred while changing PDF permissions.') - ) + ), }); }; diff --git a/frontend/src/hooks/tools/compress/useCompressOperation.ts b/frontend/src/hooks/tools/compress/useCompressOperation.ts index ebcac8b66..b69fa9d00 100644 --- a/frontend/src/hooks/tools/compress/useCompressOperation.ts +++ b/frontend/src/hooks/tools/compress/useCompressOperation.ts @@ -33,11 +33,11 @@ export const useCompressOperation = () => { const { t } = useTranslation(); return useToolOperation({ + toolType: 'singleFile', + buildFormData, operationType: 'compress', endpoint: '/api/v1/misc/compress-pdf', - buildFormData, filePrefix: 'compressed_', - multiFileEndpoint: false, // Individual API calls per file getErrorMessage: createStandardErrorHandler(t('compress.error.failed', 'An error occurred while compressing the PDF.')) }); }; diff --git a/frontend/src/hooks/tools/convert/useConvertOperation.ts b/frontend/src/hooks/tools/convert/useConvertOperation.ts index f14302aaa..7f8715722 100644 --- a/frontend/src/hooks/tools/convert/useConvertOperation.ts +++ b/frontend/src/hooks/tools/convert/useConvertOperation.ts @@ -129,11 +129,10 @@ export const useConvertOperation = () => { }, [t]); return useToolOperation({ - operationType: 'convert', - endpoint: '', // Not used with customProcessor but required - buildFormData, // Not used with customProcessor but required - filePrefix: 'converted_', + toolType: 'custom', customProcessor: customConvertProcessor, // Convert handles its own routing + operationType: 'convert', + filePrefix: 'converted_', getErrorMessage: (error) => { if (error.response?.data && typeof error.response.data === 'string') { return error.response.data; @@ -142,6 +141,6 @@ export const useConvertOperation = () => { return error.message; } return t("convert.errorConversion", "An error occurred while converting the file."); - } + }, }); }; diff --git a/frontend/src/hooks/tools/ocr/useOCROperation.ts b/frontend/src/hooks/tools/ocr/useOCROperation.ts index 419a46f60..b9b08f45f 100644 --- a/frontend/src/hooks/tools/ocr/useOCROperation.ts +++ b/frontend/src/hooks/tools/ocr/useOCROperation.ts @@ -96,11 +96,11 @@ export const useOCROperation = () => { }, [t, extractZipFiles]); const ocrConfig: ToolOperationConfig = { + toolType: 'singleFile', + buildFormData, operationType: 'ocr', endpoint: '/api/v1/misc/ocr-pdf', - buildFormData, filePrefix: 'ocr_', - multiFileEndpoint: false, // Process files individually responseHandler, // use shared flow getErrorMessage: (error) => error.message?.includes('OCR tools') && error.message?.includes('not installed') diff --git a/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.test.ts b/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.test.ts index 66cd240fc..4b83d74a8 100644 --- a/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.test.ts +++ b/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.test.ts @@ -20,12 +20,12 @@ vi.mock('../../../utils/toolErrorHandler', () => ({ })); // Import the mocked function -import { ToolOperationConfig, ToolOperationHook, useToolOperation } from '../shared/useToolOperation'; +import { SingleFileToolOperationConfig, ToolOperationHook, useToolOperation } from '../shared/useToolOperation'; describe('useRemovePasswordOperation', () => { const mockUseToolOperation = vi.mocked(useToolOperation); - const getToolConfig = (): ToolOperationConfig => mockUseToolOperation.mock.calls[0][0]; + const getToolConfig = () => mockUseToolOperation.mock.calls[0][0] as SingleFileToolOperationConfig; const mockToolOperationReturn: ToolOperationHook = { files: [], @@ -91,7 +91,7 @@ describe('useRemovePasswordOperation', () => { }); test.each([ - { property: 'multiFileEndpoint' as const, expectedValue: false }, + { property: 'toolType' as const, expectedValue: 'singleFile' }, { property: 'endpoint' as const, expectedValue: '/api/v1/security/remove-password' }, { property: 'filePrefix' as const, expectedValue: 'translated-removePassword.filenamePrefix_' }, { property: 'operationType' as const, expectedValue: 'removePassword' } diff --git a/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.ts b/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.ts index 37f8fa0db..5331118d5 100644 --- a/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.ts +++ b/frontend/src/hooks/tools/removePassword/useRemovePasswordOperation.ts @@ -14,11 +14,11 @@ export const useRemovePasswordOperation = () => { }; return useToolOperation({ + toolType: 'singleFile', + buildFormData, operationType: 'removePassword', endpoint: '/api/v1/security/remove-password', - buildFormData, filePrefix: t('removePassword.filenamePrefix', 'decrypted') + '_', - multiFileEndpoint: false, getErrorMessage: createStandardErrorHandler(t('removePassword.error.failed', 'An error occurred while removing the password from the PDF.')) }); }; diff --git a/frontend/src/hooks/tools/sanitize/useSanitizeOperation.ts b/frontend/src/hooks/tools/sanitize/useSanitizeOperation.ts index c83086121..f307f153d 100644 --- a/frontend/src/hooks/tools/sanitize/useSanitizeOperation.ts +++ b/frontend/src/hooks/tools/sanitize/useSanitizeOperation.ts @@ -22,11 +22,11 @@ export const useSanitizeOperation = () => { const { t } = useTranslation(); return useToolOperation({ + toolType: 'singleFile', + buildFormData, operationType: 'sanitize', endpoint: '/api/v1/security/sanitize-pdf', - buildFormData, filePrefix: t('sanitize.filenamePrefix', 'sanitized') + '_', - multiFileEndpoint: false, // Individual API calls per file getErrorMessage: createStandardErrorHandler(t('sanitize.error.failed', 'An error occurred while sanitising the PDF.')) }); }; diff --git a/frontend/src/hooks/tools/shared/useToolOperation.ts b/frontend/src/hooks/tools/shared/useToolOperation.ts index 623b93d84..534a594f8 100644 --- a/frontend/src/hooks/tools/shared/useToolOperation.ts +++ b/frontend/src/hooks/tools/shared/useToolOperation.ts @@ -12,6 +12,8 @@ import { ResponseHandler } from '../../../utils/toolResponseProcessor'; // Re-export for backwards compatibility export type { ProcessingProgress, ResponseHandler }; +export type ToolConfigType = 'singleFile' | 'multiFile' | 'custom'; + /** * Configuration for tool operations defining processing behavior and API integration. * @@ -20,49 +22,63 @@ export type { ProcessingProgress, ResponseHandler }; * 2. Multi-file tools: multiFileEndpoint: true, single API call with all files * 3. Complex tools: customProcessor handles all processing logic */ -export interface ToolOperationConfig { +interface BaseToolOperationConfig { /** Operation identifier for tracking and logging */ operationType: string; - /** - * 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 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); /* FIX ME */ - /** Prefix added to processed filenames (e.g., 'compressed_', 'split_') */ filePrefix: string; - /** - * 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; - /** - * 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; - /** Extract user-friendly error messages from API errors */ getErrorMessage?: (error: any) => string; } +export interface SingleFileToolOperationConfig extends BaseToolOperationConfig { + /** This tool processes one file at a time. */ + toolType: 'singleFile'; + + /** Builds FormData for API request. */ + buildFormData: ((params: TParams, file: File) => FormData); + + /** API endpoint for the operation. Can be static string or function for dynamic routing. */ + endpoint: string | ((params: TParams) => string); + + customProcessor?: undefined; +} + +export interface MultiFileToolOperationConfig extends BaseToolOperationConfig { + /** This tool processes multiple files at once. */ + toolType: 'multiFile'; + + /** Builds FormData for API request. */ + buildFormData: ((params: TParams, files: File[]) => FormData); + + /** API endpoint for the operation. Can be static string or function for dynamic routing. */ + endpoint: string | ((params: TParams) => string); + + customProcessor?: undefined; +} + +export interface CustomToolOperationConfig extends BaseToolOperationConfig { + /** This tool has custom behaviour. */ + toolType: 'custom'; + + buildFormData?: undefined; + endpoint?: undefined; + + /** + * Custom processing logic that completely bypasses standard file processing. + * This 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; +} + +export type ToolOperationConfig = SingleFileToolOperationConfig | MultiFileToolOperationConfig | CustomToolOperationConfig; + /** * Complete tool operation interface with execution capability */ @@ -100,7 +116,7 @@ export { createStandardErrorHandler } from '../../../utils/toolErrorHandler'; * @param config - Tool operation configuration * @returns Hook interface with state and execution methods */ -export const useToolOperation = ( +export const useToolOperation = ( config: ToolOperationConfig ): ToolOperationHook => { const { t } = useTranslation(); @@ -140,15 +156,28 @@ export const useToolOperation = ( try { let processedFiles: File[]; - if (config.customProcessor) { - actions.setStatus('Processing files...'); - processedFiles = await config.customProcessor(params, validFiles); - } else { - // Use explicit multiFileEndpoint flag to determine processing approach - if (config.multiFileEndpoint) { + switch (config.toolType) { + case 'singleFile': + // Individual file processing - separate API call per file + const apiCallsConfig: ApiCallsConfig = { + endpoint: config.endpoint, + buildFormData: config.buildFormData, + filePrefix: config.filePrefix, + responseHandler: config.responseHandler + }; + processedFiles = await processFiles( + params, + validFiles, + apiCallsConfig, + actions.setProgress, + actions.setStatus + ); + break; + + case 'multiFile': // 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); + const formData = config.buildFormData(params, validFiles); const endpoint = typeof config.endpoint === 'function' ? config.endpoint(params) : config.endpoint; const response = await axios.post(endpoint, formData, { responseType: 'blob' }); @@ -166,22 +195,12 @@ export const useToolOperation = ( processedFiles = await extractAllZipFiles(response.data); } } - } else { - // Individual file processing - separate API call per file - const apiCallsConfig: ApiCallsConfig = { - endpoint: config.endpoint, - buildFormData: config.buildFormData as (params: TParams, file: File) => FormData, - filePrefix: config.filePrefix, - responseHandler: config.responseHandler - }; - processedFiles = await processFiles( - params, - validFiles, - apiCallsConfig, - actions.setProgress, - actions.setStatus - ); - } + break; + + case 'custom': + actions.setStatus('Processing files...'); + processedFiles = await config.customProcessor(params, validFiles); + break; } if (processedFiles.length > 0) { diff --git a/frontend/src/hooks/tools/split/useSplitOperation.ts b/frontend/src/hooks/tools/split/useSplitOperation.ts index 7702bffad..90551f2e4 100644 --- a/frontend/src/hooks/tools/split/useSplitOperation.ts +++ b/frontend/src/hooks/tools/split/useSplitOperation.ts @@ -59,11 +59,11 @@ export const useSplitOperation = () => { const { t } = useTranslation(); return useToolOperation({ + toolType: 'multiFile', + buildFormData, operationType: 'split', endpoint: (params) => getEndpoint(params), - buildFormData: buildFormData, // Multi-file signature: (params, selectedFiles) => FormData filePrefix: 'split_', - multiFileEndpoint: true, // Single API call with all files getErrorMessage: createStandardErrorHandler(t('split.error.failed', 'An error occurred while splitting the PDF.')) }); };