From 0cb2161d334037519a6e19721fdb278a53968009 Mon Sep 17 00:00:00 2001 From: Reece Browne Date: Wed, 3 Sep 2025 15:09:47 +0100 Subject: [PATCH 1/3] Restore filewithid changes --- .../fileEditor/FileEditorThumbnail.tsx | 4 +- .../components/pageEditor/FileThumbnail.tsx | 4 +- frontend/src/contexts/FileContext.tsx | 42 ++++---- frontend/src/contexts/file/fileHooks.ts | 6 +- frontend/src/contexts/file/fileSelectors.ts | 42 ++++---- .../hooks/tools/shared/useToolOperation.ts | 48 ++++------ frontend/src/hooks/usePDFProcessor.ts | 3 +- frontend/src/hooks/useThumbnailGeneration.ts | 5 +- .../services/enhancedPDFProcessingService.ts | 13 +-- frontend/src/services/pdfProcessingService.ts | 3 +- .../tests/convert/ConvertIntegration.test.tsx | 12 ++- frontend/src/tests/utils/testFileHelpers.ts | 28 ++++++ frontend/src/types/fileContext.ts | 95 +++++++++++++++---- 13 files changed, 193 insertions(+), 112 deletions(-) create mode 100644 frontend/src/tests/utils/testFileHelpers.ts diff --git a/frontend/src/components/fileEditor/FileEditorThumbnail.tsx b/frontend/src/components/fileEditor/FileEditorThumbnail.tsx index e82483898..bfeb404c5 100644 --- a/frontend/src/components/fileEditor/FileEditorThumbnail.tsx +++ b/frontend/src/components/fileEditor/FileEditorThumbnail.tsx @@ -61,8 +61,8 @@ const FileEditorThumbnail = ({ // Resolve the actual File object for pin/unpin operations const actualFile = useMemo(() => { - return activeFiles.find((f: File) => f.name === file.name && f.size === file.size); - }, [activeFiles, file.name, file.size]); + return activeFiles.find(f => f.fileId === file.id); + }, [activeFiles, file.id]); const isPinned = actualFile ? isFilePinned(actualFile) : false; const downloadSelectedFile = useCallback(() => { diff --git a/frontend/src/components/pageEditor/FileThumbnail.tsx b/frontend/src/components/pageEditor/FileThumbnail.tsx index 1eda1f6c8..ad81ce463 100644 --- a/frontend/src/components/pageEditor/FileThumbnail.tsx +++ b/frontend/src/components/pageEditor/FileThumbnail.tsx @@ -61,8 +61,8 @@ const FileThumbnail = ({ // Resolve the actual File object for pin/unpin operations const actualFile = useMemo(() => { - return activeFiles.find((f: File) => f.name === file.name && f.size === file.size); - }, [activeFiles, file.name, file.size]); + return activeFiles.find(f => f.fileId === file.id); + }, [activeFiles, file.id]); const isPinned = actualFile ? isFilePinned(actualFile) : false; const downloadSelectedFile = useCallback(() => { diff --git a/frontend/src/contexts/FileContext.tsx b/frontend/src/contexts/FileContext.tsx index 39faa0643..97ac3c645 100644 --- a/frontend/src/contexts/FileContext.tsx +++ b/frontend/src/contexts/FileContext.tsx @@ -19,7 +19,10 @@ import { FileContextStateValue, FileContextActionsValue, FileContextActions, - FileRecord + FileId, + FileRecord, + FileWithId, + createFileWithId } from '../types/fileContext'; // Import modular components @@ -29,7 +32,6 @@ import { AddedFile, addFiles, consumeFiles, undoConsumeFiles, createFileActions import { FileLifecycleManager } from './file/lifecycle'; import { FileStateContext, FileActionsContext } from './file/contexts'; import { IndexedDBProvider, useIndexedDB } from './IndexedDBContext'; -import { FileId } from '../types/file'; const DEBUG = process.env.NODE_ENV === 'development'; @@ -79,7 +81,7 @@ function FileContextInner({ } // File operations using unified addFiles helper with persistence - const addRawFiles = useCallback(async (files: File[], options?: { insertAfterPageId?: string; selectFiles?: boolean }): Promise => { + const addRawFiles = useCallback(async (files: File[], options?: { insertAfterPageId?: string; selectFiles?: boolean }): Promise => { const addedFilesWithIds = await addFiles('raw', { files, ...options }, stateRef, filesRef, dispatch, lifecycleManager); // Auto-select the newly added files if requested @@ -98,15 +100,15 @@ function FileContextInner({ })); } - return addedFilesWithIds.map(({ file }) => file); + return addedFilesWithIds.map(({ file, id }) => createFileWithId(file, id)); }, [indexedDB, enablePersistence]); - const addProcessedFiles = useCallback(async (filesWithThumbnails: Array<{ file: File; thumbnail?: string; pageCount?: number }>): Promise => { + const addProcessedFiles = useCallback(async (filesWithThumbnails: Array<{ file: File; thumbnail?: string; pageCount?: number }>): Promise => { const result = await addFiles('processed', { filesWithThumbnails }, stateRef, filesRef, dispatch, lifecycleManager); - return result.map(({ file }) => file); + return result.map(({ file, id }) => createFileWithId(file, id)); }, []); - const addStoredFiles = useCallback(async (filesWithMetadata: Array<{ file: File; originalId: FileId; metadata: any }>, options?: { selectFiles?: boolean }): Promise => { + const addStoredFiles = useCallback(async (filesWithMetadata: Array<{ file: File; originalId: FileId; metadata: any }>, options?: { selectFiles?: boolean }): Promise => { const result = await addFiles('stored', { filesWithMetadata }, stateRef, filesRef, dispatch, lifecycleManager); // Auto-select the newly added files if requested @@ -114,7 +116,7 @@ function FileContextInner({ selectFiles(result); } - return result.map(({ file }) => file); + return result.map(({ file, id }) => createFileWithId(file, id)); }, []); // Action creators @@ -140,24 +142,14 @@ function FileContextInner({ }); }, []); - // File-to-ID wrapper functions for pinning - const pinFileWrapper = useCallback((file: File) => { - const fileId = findFileId(file); - if (fileId) { - baseActions.pinFile(fileId); - } else { - console.warn('File not found for pinning:', file.name); - } - }, [baseActions, findFileId]); + // File pinning functions - use FileWithId directly + const pinFileWrapper = useCallback((file: FileWithId) => { + baseActions.pinFile(file.fileId); + }, [baseActions]); - const unpinFileWrapper = useCallback((file: File) => { - const fileId = findFileId(file); - if (fileId) { - baseActions.unpinFile(fileId); - } else { - console.warn('File not found for unpinning:', file.name); - } - }, [baseActions, findFileId]); + const unpinFileWrapper = useCallback((file: FileWithId) => { + baseActions.unpinFile(file.fileId); + }, [baseActions]); // Complete actions object const actions = useMemo(() => ({ diff --git a/frontend/src/contexts/file/fileHooks.ts b/frontend/src/contexts/file/fileHooks.ts index e1b8e5cc4..609d38ab5 100644 --- a/frontend/src/contexts/file/fileHooks.ts +++ b/frontend/src/contexts/file/fileHooks.ts @@ -9,7 +9,7 @@ import { FileContextStateValue, FileContextActionsValue } from './contexts'; -import { FileRecord } from '../../types/fileContext'; +import { FileRecord, FileWithId } from '../../types/fileContext'; import { FileId } from '../../types/file'; /** @@ -123,7 +123,7 @@ export function useFileRecord(fileId: FileId): { file?: File; record?: FileRecor /** * Hook for all files (use sparingly - causes re-renders on file list changes) */ -export function useAllFiles(): { files: File[]; records: FileRecord[]; fileIds: FileId[] } { +export function useAllFiles(): { files: FileWithId[]; records: FileRecord[]; fileIds: FileId[] } { const { state, selectors } = useFileState(); return useMemo(() => ({ @@ -136,7 +136,7 @@ export function useAllFiles(): { files: File[]; records: FileRecord[]; fileIds: /** * Hook for selected files (optimized for selection-based UI) */ -export function useSelectedFiles(): { files: File[]; records: FileRecord[]; fileIds: FileId[] } { +export function useSelectedFiles(): { files: FileWithId[]; records: FileRecord[]; fileIds: FileId[] } { const { state, selectors } = useFileState(); return useMemo(() => ({ diff --git a/frontend/src/contexts/file/fileSelectors.ts b/frontend/src/contexts/file/fileSelectors.ts index 2111693cf..e5876813b 100644 --- a/frontend/src/contexts/file/fileSelectors.ts +++ b/frontend/src/contexts/file/fileSelectors.ts @@ -6,7 +6,9 @@ import { FileId } from '../../types/file'; import { FileRecord, FileContextState, - FileContextSelectors + FileContextSelectors, + FileWithId, + createFileWithId } from '../../types/fileContext'; /** @@ -17,11 +19,19 @@ export function createFileSelectors( filesRef: React.MutableRefObject> ): FileContextSelectors { return { - getFile: (id: FileId) => filesRef.current.get(id), + getFile: (id: FileId) => { + const file = filesRef.current.get(id); + return file ? createFileWithId(file, id) : undefined; + }, getFiles: (ids?: FileId[]) => { const currentIds = ids || stateRef.current.files.ids; - return currentIds.map(id => filesRef.current.get(id)).filter(Boolean) as File[]; + return currentIds + .map(id => { + const file = filesRef.current.get(id); + return file ? createFileWithId(file, id) : undefined; + }) + .filter(Boolean) as FileWithId[]; }, getFileRecord: (id: FileId) => stateRef.current.files.byId[id], @@ -35,8 +45,11 @@ export function createFileSelectors( getSelectedFiles: () => { return stateRef.current.ui.selectedFileIds - .map(id => filesRef.current.get(id)) - .filter(Boolean) as File[]; + .map(id => { + const file = filesRef.current.get(id); + return file ? createFileWithId(file, id) : undefined; + }) + .filter(Boolean) as FileWithId[]; }, getSelectedFileRecords: () => { @@ -52,8 +65,11 @@ export function createFileSelectors( getPinnedFiles: () => { return Array.from(stateRef.current.pinnedFiles) - .map(id => filesRef.current.get(id)) - .filter(Boolean) as File[]; + .map(id => { + const file = filesRef.current.get(id); + return file ? createFileWithId(file, id) : undefined; + }) + .filter(Boolean) as FileWithId[]; }, getPinnedFileRecords: () => { @@ -62,16 +78,8 @@ export function createFileSelectors( .filter(Boolean); }, - isFilePinned: (file: File) => { - // Find FileId by matching File object properties - const fileId = (Object.keys(stateRef.current.files.byId) as FileId[]).find(id => { - const storedFile = filesRef.current.get(id); - return storedFile && - storedFile.name === file.name && - storedFile.size === file.size && - storedFile.lastModified === file.lastModified; - }); - return fileId ? stateRef.current.pinnedFiles.has(fileId) : false; + isFilePinned: (file: FileWithId) => { + return stateRef.current.pinnedFiles.has(file.fileId); }, // Stable signature for effects - prevents unnecessary re-renders diff --git a/frontend/src/hooks/tools/shared/useToolOperation.ts b/frontend/src/hooks/tools/shared/useToolOperation.ts index d8d35176d..03275b0e7 100644 --- a/frontend/src/hooks/tools/shared/useToolOperation.ts +++ b/frontend/src/hooks/tools/shared/useToolOperation.ts @@ -6,10 +6,8 @@ import { useToolState, type ProcessingProgress } from './useToolState'; import { useToolApiCalls, type ApiCallsConfig } from './useToolApiCalls'; import { useToolResources } from './useToolResources'; import { extractErrorMessage } from '../../../utils/toolErrorHandler'; -import { createOperation } from '../../../utils/toolOperationTracker'; +import { FileWithId, extractFiles, FileId, FileRecord } from '../../../types/fileContext'; import { ResponseHandler } from '../../../utils/toolResponseProcessor'; -import { FileId } from '../../../types/file'; -import { FileRecord } from '../../../types/fileContext'; // Re-export for backwards compatibility export type { ProcessingProgress, ResponseHandler }; @@ -104,7 +102,7 @@ export interface ToolOperationHook { progress: ProcessingProgress | null; // Actions - executeOperation: (params: TParams, selectedFiles: File[]) => Promise; + executeOperation: (params: TParams, selectedFiles: FileWithId[]) => Promise; resetResults: () => void; clearError: () => void; cancelOperation: () => void; @@ -130,7 +128,7 @@ export const useToolOperation = ( config: ToolOperationConfig ): ToolOperationHook => { const { t } = useTranslation(); - const { recordOperation, markOperationApplied, markOperationFailed, addFiles, consumeFiles, undoConsumeFiles, findFileId, actions: fileActions, selectors } = useFileContext(); + const { addFiles, consumeFiles, undoConsumeFiles, actions: fileActions, selectors } = useFileContext(); // Composed hooks const { state, actions } = useToolState(); @@ -146,7 +144,7 @@ export const useToolOperation = ( const executeOperation = useCallback(async ( params: TParams, - selectedFiles: File[] + selectedFiles: FileWithId[] ): Promise => { // Validation if (selectedFiles.length === 0) { @@ -160,9 +158,6 @@ export const useToolOperation = ( return; } - // Setup operation tracking - const { operation, operationId, fileId } = createOperation(config.operationType, params, selectedFiles); - recordOperation(fileId, operation); // Reset state actions.setLoading(true); @@ -173,6 +168,9 @@ export const useToolOperation = ( try { let processedFiles: File[]; + // Convert FileWithId to regular File objects for API processing + const validRegularFiles = extractFiles(validFiles); + switch (config.toolType) { case ToolType.singleFile: // Individual file processing - separate API call per file @@ -184,7 +182,7 @@ export const useToolOperation = ( }; processedFiles = await processFiles( params, - validFiles, + validRegularFiles, apiCallsConfig, actions.setProgress, actions.setStatus @@ -194,7 +192,7 @@ export const useToolOperation = ( case ToolType.multiFile: // Multi-file processing - single API call with all files actions.setStatus('Processing files...'); - const formData = config.buildFormData(params, validFiles); + const formData = config.buildFormData(params, validRegularFiles); const endpoint = typeof config.endpoint === 'function' ? config.endpoint(params) : config.endpoint; const response = await axios.post(endpoint, formData, { responseType: 'blob' }); @@ -202,11 +200,11 @@ export const useToolOperation = ( // Multi-file responses are typically ZIP files that need extraction, but some may return single PDFs if (config.responseHandler) { // Use custom responseHandler for multi-file (handles ZIP extraction) - processedFiles = await config.responseHandler(response.data, validFiles); + processedFiles = await config.responseHandler(response.data, validRegularFiles); } else if (response.data.type === 'application/pdf' || (response.headers && response.headers['content-type'] === 'application/pdf')) { // Single PDF response (e.g. split with merge option) - use original filename - const originalFileName = validFiles[0]?.name || 'document.pdf'; + const originalFileName = validRegularFiles[0]?.name || 'document.pdf'; const singleFile = new File([response.data], originalFileName, { type: 'application/pdf' }); processedFiles = [singleFile]; } else { @@ -222,7 +220,7 @@ export const useToolOperation = ( case ToolType.custom: actions.setStatus('Processing files...'); - processedFiles = await config.customProcessor(params, validFiles); + processedFiles = await config.customProcessor(params, validRegularFiles); break; } @@ -246,17 +244,13 @@ export const useToolOperation = ( // Build parallel arrays of IDs and records for undo tracking for (const file of validFiles) { - const fileId = findFileId(file); - if (fileId) { - const record = selectors.getFileRecord(fileId); - if (record) { - inputFileIds.push(fileId); - inputFileRecords.push(record); - } else { - console.warn(`No file record found for file: ${file.name}`); - } + const fileId = file.fileId; + const record = selectors.getFileRecord(fileId); + if (record) { + inputFileIds.push(fileId); + inputFileRecords.push(record); } else { - console.warn(`No file ID found for file: ${file.name}`); + console.warn(`No file record found for file: ${file.name}`); } } @@ -264,24 +258,22 @@ export const useToolOperation = ( // Store operation data for undo (only store what we need to avoid memory bloat) lastOperationRef.current = { - inputFiles: validFiles, // Keep original File objects for undo + inputFiles: extractFiles(validFiles), // Convert to File objects for undo inputFileRecords: inputFileRecords.map(record => ({ ...record })), // Deep copy to avoid reference issues outputFileIds }; - markOperationApplied(fileId, operationId); } } catch (error: any) { const errorMessage = config.getErrorMessage?.(error) || extractErrorMessage(error); actions.setError(errorMessage); actions.setStatus(''); - markOperationFailed(fileId, operationId, errorMessage); } finally { actions.setLoading(false); actions.setProgress(null); } - }, [t, config, actions, recordOperation, markOperationApplied, markOperationFailed, addFiles, consumeFiles, findFileId, processFiles, generateThumbnails, createDownloadInfo, cleanupBlobUrls, extractZipFiles, extractAllZipFiles]); + }, [t, config, actions, addFiles, consumeFiles, processFiles, generateThumbnails, createDownloadInfo, cleanupBlobUrls, extractZipFiles, extractAllZipFiles]); const cancelOperation = useCallback(() => { cancelApiCalls(); diff --git a/frontend/src/hooks/usePDFProcessor.ts b/frontend/src/hooks/usePDFProcessor.ts index ab3b5e007..b88bfbe2b 100644 --- a/frontend/src/hooks/usePDFProcessor.ts +++ b/frontend/src/hooks/usePDFProcessor.ts @@ -1,6 +1,7 @@ import { useState, useCallback } from 'react'; import { PDFDocument, PDFPage } from '../types/pageEditor'; import { pdfWorkerManager } from '../services/pdfWorkerManager'; +import { createQuickKey } from '../types/fileContext'; export function usePDFProcessor() { const [loading, setLoading] = useState(false); @@ -75,7 +76,7 @@ export function usePDFProcessor() { // Create pages without thumbnails initially - load them lazily for (let i = 1; i <= totalPages; i++) { pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, originalPageNumber: i, thumbnail: null, // Will be loaded lazily diff --git a/frontend/src/hooks/useThumbnailGeneration.ts b/frontend/src/hooks/useThumbnailGeneration.ts index 8eba26214..310634045 100644 --- a/frontend/src/hooks/useThumbnailGeneration.ts +++ b/frontend/src/hooks/useThumbnailGeneration.ts @@ -1,5 +1,6 @@ import { useCallback, useRef } from 'react'; import { thumbnailGenerationService } from '../services/thumbnailGenerationService'; +import { createQuickKey } from '../types/fileContext'; import { FileId } from '../types/file'; // Request queue to handle concurrent thumbnail requests @@ -71,8 +72,8 @@ async function processRequestQueue() { console.log(`📸 Batch generating ${requests.length} thumbnails for pages: ${pageNumbers.slice(0, 5).join(', ')}${pageNumbers.length > 5 ? '...' : ''}`); - // Use file name as fileId for PDF document caching - const fileId = file.name + '_' + file.size + '_' + file.lastModified as FileId; + // Use quickKey for PDF document caching (same metadata, consistent format) + const fileId = createQuickKey(file) as FileId; const results = await thumbnailGenerationService.generateThumbnails( fileId, diff --git a/frontend/src/services/enhancedPDFProcessingService.ts b/frontend/src/services/enhancedPDFProcessingService.ts index f9f067c30..2d4bf4644 100644 --- a/frontend/src/services/enhancedPDFProcessingService.ts +++ b/frontend/src/services/enhancedPDFProcessingService.ts @@ -5,6 +5,7 @@ import { FileHasher } from '../utils/fileHash'; import { FileAnalyzer } from './fileAnalyzer'; import { ProcessingErrorHandler } from './processingErrorHandler'; import { pdfWorkerManager } from './pdfWorkerManager'; +import { createQuickKey } from '../types/fileContext'; export class EnhancedPDFProcessingService { private static instance: EnhancedPDFProcessingService; @@ -201,7 +202,7 @@ export class EnhancedPDFProcessingService { const thumbnail = await this.renderPageThumbnail(page, config.thumbnailQuality); pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail, rotation: 0, @@ -251,7 +252,7 @@ export class EnhancedPDFProcessingService { const thumbnail = await this.renderPageThumbnail(page, config.thumbnailQuality); pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail, rotation: 0, @@ -266,7 +267,7 @@ export class EnhancedPDFProcessingService { // Create placeholder pages for remaining pages for (let i = priorityCount + 1; i <= totalPages; i++) { pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail: null, // Will be loaded lazily rotation: 0, @@ -313,7 +314,7 @@ export class EnhancedPDFProcessingService { const thumbnail = await this.renderPageThumbnail(page, config.thumbnailQuality); pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail, rotation: 0, @@ -334,7 +335,7 @@ export class EnhancedPDFProcessingService { // Create placeholders for remaining pages for (let i = firstChunkEnd + 1; i <= totalPages; i++) { pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail: null, rotation: 0, @@ -368,7 +369,7 @@ export class EnhancedPDFProcessingService { const pages: PDFPage[] = []; for (let i = 1; i <= totalPages; i++) { pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail: null, rotation: 0, diff --git a/frontend/src/services/pdfProcessingService.ts b/frontend/src/services/pdfProcessingService.ts index 065f53210..7ede9334d 100644 --- a/frontend/src/services/pdfProcessingService.ts +++ b/frontend/src/services/pdfProcessingService.ts @@ -1,6 +1,7 @@ import { ProcessedFile, ProcessingState, PDFPage } from '../types/processing'; import { ProcessingCache } from './processingCache'; import { pdfWorkerManager } from './pdfWorkerManager'; +import { createQuickKey } from '../types/fileContext'; export class PDFProcessingService { private static instance: PDFProcessingService; @@ -113,7 +114,7 @@ export class PDFProcessingService { const thumbnail = canvas.toDataURL(); pages.push({ - id: `${file.name}-page-${i}`, + id: `${createQuickKey(file)}-page-${i}`, pageNumber: i, thumbnail, rotation: 0, diff --git a/frontend/src/tests/convert/ConvertIntegration.test.tsx b/frontend/src/tests/convert/ConvertIntegration.test.tsx index 41a768838..37a25c2a5 100644 --- a/frontend/src/tests/convert/ConvertIntegration.test.tsx +++ b/frontend/src/tests/convert/ConvertIntegration.test.tsx @@ -18,6 +18,8 @@ import { FileContextProvider } from '../../contexts/FileContext'; import { I18nextProvider } from 'react-i18next'; import i18n from '../../i18n/config'; import axios from 'axios'; +import { createTestFileWithId } from '../utils/testFileHelpers'; +import { FileWithId } from '../../types/fileContext'; // Mock axios vi.mock('axios'); @@ -55,9 +57,9 @@ const createTestFile = (name: string, content: string, type: string): File => { return new File([content], name, { type }); }; -const createPDFFile = (): File => { +const createPDFFile = (): FileWithId => { const pdfContent = '%PDF-1.4\n1 0 obj\n<<\n/Type /Catalog\n/Pages 2 0 R\n>>\nendobj\ntrailer\n<<\n/Size 2\n/Root 1 0 R\n>>\nstartxref\n0\n%%EOF'; - return createTestFile('test.pdf', pdfContent, 'application/pdf'); + return createTestFileWithId('test.pdf', pdfContent, 'application/pdf'); }; // Test wrapper component @@ -162,7 +164,7 @@ describe('Convert Tool Integration Tests', () => { wrapper: TestWrapper }); - const testFile = createTestFile('invalid.txt', 'not a pdf', 'text/plain'); + const testFile = createTestFileWithId('invalid.txt', 'not a pdf', 'text/plain'); const parameters: ConvertParameters = { fromExtension: 'pdf', toExtension: 'png', @@ -426,7 +428,7 @@ describe('Convert Tool Integration Tests', () => { }); const files = [ createPDFFile(), - createTestFile('test2.pdf', '%PDF-1.4...', 'application/pdf') + createTestFileWithId('test2.pdf', '%PDF-1.4...', 'application/pdf') ] const parameters: ConvertParameters = { fromExtension: 'pdf', @@ -527,7 +529,7 @@ describe('Convert Tool Integration Tests', () => { wrapper: TestWrapper }); - const corruptedFile = createTestFile('corrupted.pdf', 'not-a-pdf', 'application/pdf'); + const corruptedFile = createTestFileWithId('corrupted.pdf', 'not-a-pdf', 'application/pdf'); const parameters: ConvertParameters = { fromExtension: 'pdf', toExtension: 'png', diff --git a/frontend/src/tests/utils/testFileHelpers.ts b/frontend/src/tests/utils/testFileHelpers.ts new file mode 100644 index 000000000..be8cd9774 --- /dev/null +++ b/frontend/src/tests/utils/testFileHelpers.ts @@ -0,0 +1,28 @@ +/** + * Test utilities for creating FileWithId objects in tests + */ + +import { FileWithId, createFileWithId } from '../../types/fileContext'; + +/** + * Create a FileWithId object for testing purposes + */ +export function createTestFileWithId( + name: string, + content: string = 'test content', + type: string = 'application/pdf' +): FileWithId { + const file = new File([content], name, { type }); + return createFileWithId(file); +} + +/** + * Create multiple FileWithId objects for testing + */ +export function createTestFilesWithId( + files: Array<{ name: string; content?: string; type?: string }> +): FileWithId[] { + return files.map(({ name, content = 'test content', type = 'application/pdf' }) => + createTestFileWithId(name, content, type) + ); +} \ No newline at end of file diff --git a/frontend/src/types/fileContext.ts b/frontend/src/types/fileContext.ts index 9210f9ce9..5520e2cde 100644 --- a/frontend/src/types/fileContext.ts +++ b/frontend/src/types/fileContext.ts @@ -5,6 +5,9 @@ import { PageOperation } from './pageEditor'; import { FileId, FileMetadata } from './file'; +// Re-export FileId for convenience +export type { FileId }; + export type ModeType = | 'viewer' | 'pageEditor' @@ -82,6 +85,67 @@ export function createQuickKey(file: File): string { return `${file.name}|${file.size}|${file.lastModified}`; } +// File with embedded UUID - replaces loose File + FileId parameter passing +export interface FileWithId extends File { + readonly fileId: FileId; + readonly quickKey: string; // Fast deduplication key: name|size|lastModified +} + +// Type guard to check if a File object has an embedded fileId +export function isFileWithId(file: File): file is FileWithId { + return 'fileId' in file && typeof (file as any).fileId === 'string' && + 'quickKey' in file && typeof (file as any).quickKey === 'string'; +} + +// Create a FileWithId from a regular File object +export function createFileWithId(file: File, id?: FileId): FileWithId { + const fileId = id || createFileId(); + const quickKey = createQuickKey(file); + + // File properties are not enumerable, so we need to copy them explicitly + // This avoids prototype chain issues while preserving all File functionality + const fileWithId = { + // Explicitly copy File properties (they're not enumerable) + name: file.name, + size: file.size, + type: file.type, + lastModified: file.lastModified, + webkitRelativePath: file.webkitRelativePath, + + // Add our custom properties + fileId: fileId, + quickKey: quickKey, + + // Preserve File prototype methods by binding them to the original file + arrayBuffer: file.arrayBuffer.bind(file), + slice: file.slice.bind(file), + stream: file.stream.bind(file), + text: file.text.bind(file) + } as FileWithId; + + return fileWithId; +} + +// Extract FileIds from FileWithId array +export function extractFileIds(files: FileWithId[]): FileId[] { + return files.map(file => file.fileId); +} + +// Extract regular File objects from FileWithId array +export function extractFiles(files: FileWithId[]): File[] { + return files as File[]; +} + +// Check if an object is a File or FileWithId (replaces instanceof File checks) +export function isFileObject(obj: any): obj is File | FileWithId { + return obj && + typeof obj.name === 'string' && + typeof obj.size === 'number' && + typeof obj.type === 'string' && + typeof obj.lastModified === 'number' && + typeof obj.arrayBuffer === 'function'; +} + export function toFileRecord(file: File, id?: FileId): FileRecord { @@ -215,18 +279,18 @@ export type FileContextAction = export interface FileContextActions { // File management - lightweight actions only - addFiles: (files: File[], options?: { insertAfterPageId?: string; selectFiles?: boolean }) => Promise; - addProcessedFiles: (filesWithThumbnails: Array<{ file: File; thumbnail?: string; pageCount?: number }>) => Promise; - addStoredFiles: (filesWithMetadata: Array<{ file: File; originalId: FileId; metadata: FileMetadata }>, options?: { selectFiles?: boolean }) => Promise; + addFiles: (files: File[], options?: { insertAfterPageId?: string; selectFiles?: boolean }) => Promise; + addProcessedFiles: (filesWithThumbnails: Array<{ file: File; thumbnail?: string; pageCount?: number }>) => Promise; + addStoredFiles: (filesWithMetadata: Array<{ file: File; originalId: FileId; metadata: FileMetadata }>, options?: { selectFiles?: boolean }) => Promise; removeFiles: (fileIds: FileId[], deleteFromStorage?: boolean) => Promise; updateFileRecord: (id: FileId, updates: Partial) => void; reorderFiles: (orderedFileIds: FileId[]) => void; clearAllFiles: () => Promise; clearAllData: () => Promise; - // File pinning - pinFile: (file: File) => void; - unpinFile: (file: File) => void; + // File pinning - accepts FileWithId for safer type checking + pinFile: (file: FileWithId) => void; + unpinFile: (file: FileWithId) => void; // File consumption (replace unpinned files with outputs) consumeFiles: (inputFileIds: FileId[], outputFiles: File[]) => Promise; @@ -253,26 +317,17 @@ export interface FileContextActions { // File selectors (separate from actions to avoid re-renders) export interface FileContextSelectors { - // File access - no state dependency, uses ref - getFile: (id: FileId) => File | undefined; - getFiles: (ids?: FileId[]) => File[]; - - // Record access - uses normalized state + getFile: (id: FileId) => FileWithId | undefined; + getFiles: (ids?: FileId[]) => FileWithId[]; getFileRecord: (id: FileId) => FileRecord | undefined; getFileRecords: (ids?: FileId[]) => FileRecord[]; - - // Derived selectors getAllFileIds: () => FileId[]; - getSelectedFiles: () => File[]; + getSelectedFiles: () => FileWithId[]; getSelectedFileRecords: () => FileRecord[]; - - // Pinned files selectors getPinnedFileIds: () => FileId[]; - getPinnedFiles: () => File[]; + getPinnedFiles: () => FileWithId[]; getPinnedFileRecords: () => FileRecord[]; - isFilePinned: (file: File) => boolean; - - // Stable signature for effect dependencies + isFilePinned: (file: FileWithId) => boolean; getFilesSignature: () => string; } From 4886ef7ddb8a12f6d82d668421d8ff242c366da6 Mon Sep 17 00:00:00 2001 From: Reece Browne Date: Wed, 3 Sep 2025 16:09:43 +0100 Subject: [PATCH 2/3] Fix filewithid use --- frontend/.eslintrc.js | 55 +++++++++++++ frontend/src/App.tsx | 3 + .../src/components/fileEditor/FileEditor.tsx | 8 +- frontend/src/components/shared/FileCard.tsx | 2 +- frontend/src/components/shared/FileGrid.tsx | 8 +- .../tools/convert/ConvertSettings.tsx | 23 ++---- .../tools/convert/ConvertToPdfaSettings.tsx | 3 +- .../tools/shared/FileStatusIndicator.tsx | 3 +- .../components/tools/shared/FilesToolStep.tsx | 3 +- .../tools/shared/createToolFlow.tsx | 3 +- frontend/src/components/viewer/Viewer.tsx | 3 +- .../src/hooks/tools/shared/useBaseTool.ts | 3 +- frontend/src/hooks/useFileManager.ts | 2 +- frontend/src/hooks/useFileWithUrl.ts | 5 +- frontend/src/hooks/useIndexedDBThumbnail.ts | 5 +- .../src/hooks/usePdfSignatureDetection.ts | 3 +- .../ConvertSmartDetectionIntegration.test.tsx | 64 +++++++-------- frontend/src/types/fileIdSafety.d.ts | 50 ++++++++++++ frontend/src/utils/fileIdSafety.ts | 79 +++++++++++++++++++ 19 files changed, 257 insertions(+), 68 deletions(-) create mode 100644 frontend/.eslintrc.js create mode 100644 frontend/src/types/fileIdSafety.d.ts create mode 100644 frontend/src/utils/fileIdSafety.ts diff --git a/frontend/.eslintrc.js b/frontend/.eslintrc.js new file mode 100644 index 000000000..cd779cd72 --- /dev/null +++ b/frontend/.eslintrc.js @@ -0,0 +1,55 @@ +module.exports = { + extends: [ + 'react-app', + 'react-app/jest' + ], + rules: { + // Custom rules to prevent dangerous file.name as ID patterns + 'no-file-name-as-id': 'error', + 'prefer-file-with-id': 'warn' + }, + overrides: [ + { + files: ['**/*.ts', '**/*.tsx'], + rules: { + // Prevent file.name being used where FileId is expected + 'no-restricted-syntax': [ + 'error', + { + selector: 'MemberExpression[object.name="file"][property.name="name"]', + message: 'Avoid using file.name directly. Use FileWithId.fileId or safeGetFileId() instead to prevent ID collisions.' + }, + { + selector: 'CallExpression[callee.name="createOperation"] > ArrayExpression > CallExpression[callee.property.name="map"] > ArrowFunctionExpression > MemberExpression[object.name="f"][property.name="name"]', + message: 'Dangerous pattern: Using file.name as ID in createOperation. Use FileWithId.fileId instead.' + }, + { + selector: 'ArrayExpression[elements.length>0] CallExpression[callee.property.name="map"] > ArrowFunctionExpression > MemberExpression[property.name="name"]', + message: 'Potential file.name as ID usage detected. Ensure proper FileId usage instead of file.name.' + } + ] + } + } + ], + settings: { + // Custom settings for our file ID validation + 'file-id-validation': { + // Functions that should only accept FileId, not strings + 'file-id-only-functions': [ + 'recordOperation', + 'markOperationApplied', + 'markOperationFailed', + 'removeFiles', + 'updateFileRecord', + 'pinFile', + 'unpinFile' + ], + // Functions that should accept FileWithId instead of File + 'file-with-id-functions': [ + 'createOperation', + 'executeOperation', + 'isFilePinned' + ] + } + } +}; \ No newline at end of file diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 8a30d3869..97503ba21 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -14,6 +14,9 @@ import "./styles/cookieconsent.css"; import "./index.css"; import { RightRailProvider } from "./contexts/RightRailContext"; +// Import file ID safety validators (development only) +import "./utils/fileIdSafety"; + // Loading component for i18next suspense const LoadingFallback = () => (
void; - onMergeFiles?: (files: File[]) => void; + onOpenPageEditor?: (file: FileWithId) => void; + onMergeFiles?: (files: FileWithId[]) => void; toolMode?: boolean; showUpload?: boolean; showBulkActions?: boolean; @@ -421,7 +421,7 @@ const FileEditor = ({ if (startIndex === -1) return; const recordsToMerge = activeFileRecords.slice(startIndex); - const filesToMerge = recordsToMerge.map(r => selectors.getFile(r.id)).filter(Boolean) as File[]; + const filesToMerge = recordsToMerge.map(r => selectors.getFile(r.id)).filter(Boolean) as FileWithId[]; if (onMergeFiles) { onMergeFiles(filesToMerge); } diff --git a/frontend/src/components/shared/FileCard.tsx b/frontend/src/components/shared/FileCard.tsx index 73ae01dba..b433be2ca 100644 --- a/frontend/src/components/shared/FileCard.tsx +++ b/frontend/src/components/shared/FileCard.tsx @@ -25,7 +25,7 @@ interface FileCardProps { const FileCard = ({ file, record, onRemove, onDoubleClick, onView, onEdit, isSelected, onSelect, isSupported = true }: FileCardProps) => { const { t } = useTranslation(); // Use record thumbnail if available, otherwise fall back to IndexedDB lookup - const fileMetadata = record ? { id: record.id, name: file.name, type: file.type, size: file.size, lastModified: file.lastModified } : null; + const fileMetadata = record ? { id: record.id, name: record.name, type: record.type, size: record.size, lastModified: record.lastModified } : null; const { thumbnail: indexedDBThumb, isGenerating } = useIndexedDBThumbnail(fileMetadata); const thumb = record?.thumbnailUrl || indexedDBThumb; const [isHovered, setIsHovered] = useState(false); diff --git a/frontend/src/components/shared/FileGrid.tsx b/frontend/src/components/shared/FileGrid.tsx index 1a43196d6..2da1e2b09 100644 --- a/frontend/src/components/shared/FileGrid.tsx +++ b/frontend/src/components/shared/FileGrid.tsx @@ -124,8 +124,12 @@ const FileGrid = ({ style={{ overflowY: "auto", width: "100%" }} > {displayFiles.map((item, idx) => { - const fileId = item.record?.id || item.file.name as FileId /* FIX ME: This doesn't seem right */; - const originalIdx = files.findIndex(f => (f.record?.id || f.file.name) === fileId); + if (!item.record?.id) { + console.error('FileGrid: File missing FileRecord with proper ID:', item.file.name); + return null; + } + const fileId = item.record.id; + const originalIdx = files.findIndex(f => f.record?.id === fileId); const supported = isFileSupported ? isFileSupported(item.file.name) : true; return ( void; getAvailableToExtensions: (fromExtension: string) => Array<{value: string, label: string, group: string}>; - selectedFiles: File[]; + selectedFiles: FileWithId[]; disabled?: boolean; } @@ -129,7 +129,7 @@ const ConvertSettings = ({ }; const filterFilesByExtension = (extension: string) => { - const files = activeFiles.map(fileId => selectors.getFile(fileId)).filter(Boolean) as File[]; + const files = activeFiles.map(fileId => selectors.getFile(fileId)).filter(Boolean) as FileWithId[]; return files.filter(file => { const fileExtension = detectFileExtension(file.name); @@ -143,21 +143,8 @@ const ConvertSettings = ({ }); }; - const updateFileSelection = (files: File[]) => { - // Map File objects to their actual IDs in FileContext - const fileIds = files.map(file => { - // Find the file ID by matching file properties - const fileRecord = state.files.ids - .map(id => selectors.getFileRecord(id)) - .find(record => - record && - record.name === file.name && - record.size === file.size && - record.lastModified === file.lastModified - ); - return fileRecord?.id; - }).filter((id): id is FileId => id !== undefined); // Type guard to ensure only strings - + const updateFileSelection = (files: FileWithId[]) => { + const fileIds = files.map(file => file.fileId); setSelectedFiles(fileIds); }; diff --git a/frontend/src/components/tools/convert/ConvertToPdfaSettings.tsx b/frontend/src/components/tools/convert/ConvertToPdfaSettings.tsx index e1a662bd2..ccad7ce67 100644 --- a/frontend/src/components/tools/convert/ConvertToPdfaSettings.tsx +++ b/frontend/src/components/tools/convert/ConvertToPdfaSettings.tsx @@ -3,11 +3,12 @@ import { Stack, Text, Select, Alert } from '@mantine/core'; import { useTranslation } from 'react-i18next'; import { ConvertParameters } from '../../../hooks/tools/convert/useConvertParameters'; import { usePdfSignatureDetection } from '../../../hooks/usePdfSignatureDetection'; +import { FileWithId } from '../../../types/fileContext'; interface ConvertToPdfaSettingsProps { parameters: ConvertParameters; onParameterChange: (key: keyof ConvertParameters, value: any) => void; - selectedFiles: File[]; + selectedFiles: FileWithId[]; disabled?: boolean; } diff --git a/frontend/src/components/tools/shared/FileStatusIndicator.tsx b/frontend/src/components/tools/shared/FileStatusIndicator.tsx index 9b375fc2f..214159b05 100644 --- a/frontend/src/components/tools/shared/FileStatusIndicator.tsx +++ b/frontend/src/components/tools/shared/FileStatusIndicator.tsx @@ -6,9 +6,10 @@ import UploadIcon from '@mui/icons-material/Upload'; import { useFilesModalContext } from "../../../contexts/FilesModalContext"; import { useAllFiles } from "../../../contexts/FileContext"; import { useFileManager } from "../../../hooks/useFileManager"; +import { FileWithId } from "../../../types/fileContext"; export interface FileStatusIndicatorProps { - selectedFiles?: File[]; + selectedFiles?: FileWithId[]; placeholder?: string; } diff --git a/frontend/src/components/tools/shared/FilesToolStep.tsx b/frontend/src/components/tools/shared/FilesToolStep.tsx index b062e9c02..11f7d4e02 100644 --- a/frontend/src/components/tools/shared/FilesToolStep.tsx +++ b/frontend/src/components/tools/shared/FilesToolStep.tsx @@ -1,9 +1,10 @@ import React from 'react'; import { useTranslation } from 'react-i18next'; import FileStatusIndicator from './FileStatusIndicator'; +import { FileWithId } from '../../../types/fileContext'; export interface FilesToolStepProps { - selectedFiles: File[]; + selectedFiles: FileWithId[]; isCollapsed?: boolean; onCollapsedClick?: () => void; placeholder?: string; diff --git a/frontend/src/components/tools/shared/createToolFlow.tsx b/frontend/src/components/tools/shared/createToolFlow.tsx index b6a7594c6..1bee8b9fc 100644 --- a/frontend/src/components/tools/shared/createToolFlow.tsx +++ b/frontend/src/components/tools/shared/createToolFlow.tsx @@ -4,9 +4,10 @@ import { createToolSteps, ToolStepProvider } from './ToolStep'; import OperationButton from './OperationButton'; import { ToolOperationHook } from '../../../hooks/tools/shared/useToolOperation'; import { ToolWorkflowTitle, ToolWorkflowTitleProps } from './ToolWorkflowTitle'; +import { FileWithId } from '../../../types/fileContext'; export interface FilesStepConfig { - selectedFiles: File[]; + selectedFiles: FileWithId[]; isCollapsed?: boolean; placeholder?: string; onCollapsedClick?: () => void; diff --git a/frontend/src/components/viewer/Viewer.tsx b/frontend/src/components/viewer/Viewer.tsx index 0932e995b..53d9e98e2 100644 --- a/frontend/src/components/viewer/Viewer.tsx +++ b/frontend/src/components/viewer/Viewer.tsx @@ -15,6 +15,7 @@ import { fileStorage } from "../../services/fileStorage"; import SkeletonLoader from '../shared/SkeletonLoader'; import { useFileState, useFileActions, useCurrentFile } from "../../contexts/FileContext"; import { useFileWithUrl } from "../../hooks/useFileWithUrl"; +import { isFileObject } from "../../types/fileContext"; import { FileId } from "../../types/file"; @@ -201,7 +202,7 @@ const Viewer = ({ const effectiveFile = React.useMemo(() => { if (previewFile) { // Validate the preview file - if (!(previewFile instanceof File)) { + if (!isFileObject(previewFile)) { return null; } diff --git a/frontend/src/hooks/tools/shared/useBaseTool.ts b/frontend/src/hooks/tools/shared/useBaseTool.ts index 64e9af59e..4995803e5 100644 --- a/frontend/src/hooks/tools/shared/useBaseTool.ts +++ b/frontend/src/hooks/tools/shared/useBaseTool.ts @@ -4,10 +4,11 @@ import { useEndpointEnabled } from '../../useEndpointConfig'; import { BaseToolProps } from '../../../types/tool'; import { ToolOperationHook } from './useToolOperation'; import { BaseParametersHook } from './useBaseParameters'; +import { FileWithId } from '../../../types/fileContext'; interface BaseToolReturn { // File management - selectedFiles: File[]; + selectedFiles: FileWithId[]; // Tool-specific hooks params: BaseParametersHook; diff --git a/frontend/src/hooks/useFileManager.ts b/frontend/src/hooks/useFileManager.ts index f47430fd2..8df1e2754 100644 --- a/frontend/src/hooks/useFileManager.ts +++ b/frontend/src/hooks/useFileManager.ts @@ -2,7 +2,7 @@ import { useState, useCallback } from 'react'; import { useIndexedDB } from '../contexts/IndexedDBContext'; import { FileMetadata } from '../types/file'; import { generateThumbnailForFile } from '../utils/thumbnailUtils'; -import { FileId } from '../types/file'; +import { FileId } from '../types/fileContext'; export const useFileManager = () => { const [loading, setLoading] = useState(false); diff --git a/frontend/src/hooks/useFileWithUrl.ts b/frontend/src/hooks/useFileWithUrl.ts index fd2f2d604..84c06baa2 100644 --- a/frontend/src/hooks/useFileWithUrl.ts +++ b/frontend/src/hooks/useFileWithUrl.ts @@ -1,4 +1,5 @@ import { useMemo } from 'react'; +import { isFileObject } from '../types/fileContext'; /** * Hook to convert a File object to { file: File; url: string } format @@ -8,8 +9,8 @@ export function useFileWithUrl(file: File | Blob | null): { file: File | Blob; u return useMemo(() => { if (!file) return null; - // Validate that file is a proper File or Blob object - if (!(file instanceof File) && !(file instanceof Blob)) { + // Validate that file is a proper File, FileWithId, or Blob object + if (!isFileObject(file) && !(file instanceof Blob)) { console.warn('useFileWithUrl: Expected File or Blob, got:', file); return null; } diff --git a/frontend/src/hooks/useIndexedDBThumbnail.ts b/frontend/src/hooks/useIndexedDBThumbnail.ts index 4f0d77c0e..a6251db3c 100644 --- a/frontend/src/hooks/useIndexedDBThumbnail.ts +++ b/frontend/src/hooks/useIndexedDBThumbnail.ts @@ -2,6 +2,7 @@ import { useState, useEffect } from "react"; import { FileMetadata } from "../types/file"; import { useIndexedDB } from "../contexts/IndexedDBContext"; import { generateThumbnailForFile } from "../utils/thumbnailUtils"; +import { FileId } from "../types/fileContext"; /** * Calculate optimal scale for thumbnail generation @@ -53,7 +54,7 @@ export function useIndexedDBThumbnail(file: FileMetadata | undefined | null): { // Try to load file from IndexedDB using new context if (file.id && indexedDB) { - const loadedFile = await indexedDB.loadFile(file.id); + const loadedFile = await indexedDB.loadFile(file.id as FileId); if (!loadedFile) { throw new Error('File not found in IndexedDB'); } @@ -70,7 +71,7 @@ export function useIndexedDBThumbnail(file: FileMetadata | undefined | null): { // Save thumbnail to IndexedDB for persistence if (file.id && indexedDB && thumbnail) { try { - await indexedDB.updateThumbnail(file.id, thumbnail); + await indexedDB.updateThumbnail(file.id as FileId, thumbnail); } catch (error) { console.warn('Failed to save thumbnail to IndexedDB:', error); } diff --git a/frontend/src/hooks/usePdfSignatureDetection.ts b/frontend/src/hooks/usePdfSignatureDetection.ts index 17f90f2d9..84075f972 100644 --- a/frontend/src/hooks/usePdfSignatureDetection.ts +++ b/frontend/src/hooks/usePdfSignatureDetection.ts @@ -1,13 +1,14 @@ import { useState, useEffect } from 'react'; import * as pdfjsLib from 'pdfjs-dist'; import { pdfWorkerManager } from '../services/pdfWorkerManager'; +import { FileWithId } from '../types/fileContext'; export interface PdfSignatureDetectionResult { hasDigitalSignatures: boolean; isChecking: boolean; } -export const usePdfSignatureDetection = (files: File[]): PdfSignatureDetectionResult => { +export const usePdfSignatureDetection = (files: FileWithId[]): PdfSignatureDetectionResult => { const [hasDigitalSignatures, setHasDigitalSignatures] = useState(false); const [isChecking, setIsChecking] = useState(false); diff --git a/frontend/src/tests/convert/ConvertSmartDetectionIntegration.test.tsx b/frontend/src/tests/convert/ConvertSmartDetectionIntegration.test.tsx index 4e9fb7908..3ba6cc589 100644 --- a/frontend/src/tests/convert/ConvertSmartDetectionIntegration.test.tsx +++ b/frontend/src/tests/convert/ConvertSmartDetectionIntegration.test.tsx @@ -14,6 +14,8 @@ import i18n from '../../i18n/config'; import axios from 'axios'; import { detectFileExtension } from '../../utils/fileUtils'; import { FIT_OPTIONS } from '../../constants/convertConstants'; +import { createTestFileWithId, createTestFilesWithId } from '../utils/testFileHelpers'; +import { FileWithId } from '../../types/fileContext'; // Mock axios vi.mock('axios'); @@ -81,7 +83,7 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { }); // Create mock DOCX file - const docxFile = new File(['docx content'], 'document.docx', { type: 'application/vnd.openxmlformats-officedocument.wordprocessingml.document' }); + const docxFile = createTestFileWithId('document.docx', 'docx content', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document'); // Test auto-detection act(() => { @@ -117,7 +119,7 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { }); // Create mock unknown file - const unknownFile = new File(['unknown content'], 'document.xyz', { type: 'application/octet-stream' }); + const unknownFile = createTestFileWithId('document.xyz', 'unknown content', 'application/octet-stream'); // Test auto-detection act(() => { @@ -156,11 +158,11 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { }); // Create mock image files - const imageFiles = [ - new File(['jpg content'], 'photo1.jpg', { type: 'image/jpeg' }), - new File(['png content'], 'photo2.png', { type: 'image/png' }), - new File(['gif content'], 'photo3.gif', { type: 'image/gif' }) - ]; + const imageFiles = createTestFilesWithId([ + { name: 'photo1.jpg', content: 'jpg content', type: 'image/jpeg' }, + { name: 'photo2.png', content: 'png content', type: 'image/png' }, + { name: 'photo3.gif', content: 'gif content', type: 'image/gif' } + ]); // Test smart detection for all images act(() => { @@ -202,11 +204,11 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { }); // Create mixed file types - const mixedFiles = [ - new File(['pdf content'], 'document.pdf', { type: 'application/pdf' }), - new File(['docx content'], 'spreadsheet.xlsx', { type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' }), - new File(['pptx content'], 'presentation.pptx', { type: 'application/vnd.openxmlformats-officedocument.presentationml.presentation' }) - ]; + const mixedFiles = createTestFilesWithId([ + { name: 'document.pdf', content: 'pdf content', type: 'application/pdf' }, + { name: 'spreadsheet.xlsx', content: 'docx content', type: 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet' }, + { name: 'presentation.pptx', content: 'pptx content', type: 'application/vnd.openxmlformats-officedocument.presentationml.presentation' } + ]); // Test smart detection for mixed types act(() => { @@ -243,10 +245,10 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { }); // Create mock web files - const webFiles = [ - new File(['content'], 'page1.html', { type: 'text/html' }), - new File(['zip content'], 'site.zip', { type: 'application/zip' }) - ]; + const webFiles = createTestFilesWithId([ + { name: 'page1.html', content: 'content', type: 'text/html' }, + { name: 'site.zip', content: 'zip content', type: 'application/zip' } + ]); // Test smart detection for web files act(() => { @@ -288,7 +290,7 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { wrapper: TestWrapper }); - const htmlFile = new File(['content'], 'page.html', { type: 'text/html' }); + const htmlFile = createTestFileWithId('page.html', 'content', 'text/html'); // Set up HTML conversion parameters act(() => { @@ -318,7 +320,7 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { wrapper: TestWrapper }); - const emlFile = new File(['email content'], 'email.eml', { type: 'message/rfc822' }); + const emlFile = createTestFileWithId('email.eml', 'email content', 'message/rfc822'); // Set up email conversion parameters act(() => { @@ -355,7 +357,7 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { wrapper: TestWrapper }); - const pdfFile = new File(['pdf content'], 'document.pdf', { type: 'application/pdf' }); + const pdfFile = createTestFileWithId('document.pdf', 'pdf content', 'application/pdf'); // Set up PDF/A conversion parameters act(() => { @@ -392,10 +394,10 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { wrapper: TestWrapper }); - const imageFiles = [ - new File(['jpg1'], 'photo1.jpg', { type: 'image/jpeg' }), - new File(['jpg2'], 'photo2.jpg', { type: 'image/jpeg' }) - ]; + const imageFiles = createTestFilesWithId([ + { name: 'photo1.jpg', content: 'jpg1', type: 'image/jpeg' }, + { name: 'photo2.jpg', content: 'jpg2', type: 'image/jpeg' } + ]); // Set up image conversion parameters act(() => { @@ -432,10 +434,10 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { wrapper: TestWrapper }); - const imageFiles = [ - new File(['jpg1'], 'photo1.jpg', { type: 'image/jpeg' }), - new File(['jpg2'], 'photo2.jpg', { type: 'image/jpeg' }) - ]; + const imageFiles = createTestFilesWithId([ + { name: 'photo1.jpg', content: 'jpg1', type: 'image/jpeg' }, + { name: 'photo2.jpg', content: 'jpg2', type: 'image/jpeg' } + ]); // Set up for separate processing act(() => { @@ -477,10 +479,10 @@ describe('Convert Tool - Smart Detection Integration Tests', () => { }) .mockRejectedValueOnce(new Error('File 2 failed')); - const mixedFiles = [ - new File(['file1'], 'doc1.txt', { type: 'text/plain' }), - new File(['file2'], 'doc2.xyz', { type: 'application/octet-stream' }) - ]; + const mixedFiles = createTestFilesWithId([ + { name: 'doc1.txt', content: 'file1', type: 'text/plain' }, + { name: 'doc2.xyz', content: 'file2', type: 'application/octet-stream' } + ]); // Set up for separate processing (mixed smart detection) act(() => { diff --git a/frontend/src/types/fileIdSafety.d.ts b/frontend/src/types/fileIdSafety.d.ts new file mode 100644 index 000000000..3235d3b3a --- /dev/null +++ b/frontend/src/types/fileIdSafety.d.ts @@ -0,0 +1,50 @@ +/** + * Type safety declarations to prevent file.name/UUID confusion + */ + +import { FileId, FileWithId, OperationType, FileOperation } from './fileContext'; + +declare global { + namespace FileIdSafety { + // Mark functions that should never accept file.name as parameters + type SafeFileIdFunction any> = T extends (...args: infer P) => infer R + ? P extends readonly [string, ...any[]] + ? never // Reject string parameters in first position for FileId functions + : T + : T; + + // Mark functions that should only accept FileWithId, not regular File + type FileWithIdOnlyFunction any> = T extends (...args: infer P) => infer R + ? P extends readonly [File, ...any[]] + ? never // Reject File parameters in first position for FileWithId functions + : T + : T; + + // Utility type to enforce FileWithId usage + type RequireFileWithId = T extends File ? FileWithId : T; + } + + // Extend Window interface to add runtime validation helpers + interface Window { + __FILE_ID_DEBUG?: boolean; + __validateFileId?: (id: string, context: string) => void; + } +} + +// Augment FileContext types to prevent bypassing FileWithId +declare module '../contexts/FileContext' { + export interface StrictFileContextActions { + pinFile: (file: FileWithId) => void; // Must be FileWithId + unpinFile: (file: FileWithId) => void; // Must be FileWithId + addFiles: (files: File[], options?: { insertAfterPageId?: string }) => Promise; // Returns FileWithId + consumeFiles: (inputFileIds: FileId[], outputFiles: File[]) => Promise; // Returns FileWithId + } + + export interface StrictFileContextSelectors { + getFile: (id: FileId) => FileWithId | undefined; // Returns FileWithId + getFiles: (ids?: FileId[]) => FileWithId[]; // Returns FileWithId[] + isFilePinned: (file: FileWithId) => boolean; // Must be FileWithId + } +} + +export {}; \ No newline at end of file diff --git a/frontend/src/utils/fileIdSafety.ts b/frontend/src/utils/fileIdSafety.ts new file mode 100644 index 000000000..6837786ed --- /dev/null +++ b/frontend/src/utils/fileIdSafety.ts @@ -0,0 +1,79 @@ +/** + * Runtime validation utilities for FileId safety + */ + +import { FileId } from '../types/fileContext'; + +// Validate that a string is a proper FileId (has UUID format) +export function isValidFileId(id: string): id is FileId { + // Check UUID v4 format: 8-4-4-4-12 hex digits + const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + return uuidRegex.test(id); +} + +// Detect potentially dangerous file.name usage as ID +export function isDangerousFileNameAsId(fileName: string, context: string = ''): boolean { + // Check if it's definitely a UUID (safe) + if (isValidFileId(fileName)) { + return false; + } + + // Check if it's a quickKey (safe) - format: name|size|lastModified + if (/^.+\|\d+\|\d+$/.test(fileName)) { + return false; // quickKeys are legitimate, not dangerous + } + + // Common patterns that suggest file.name is being used as ID + const dangerousPatterns = [ + /^[^-]+-page-\d+$/, // pattern: filename-page-123 + /\.(pdf|jpg|png|doc|docx)$/i, // ends with file extension + /\s/, // contains whitespace (filenames often have spaces) + /[()[\]{}]/, // contains brackets/parentheses common in filenames + /['"]/, // contains quotes + /[^a-zA-Z0-9\-._]/ // contains special characters not in UUIDs + ]; + + // Check dangerous patterns + const isDangerous = dangerousPatterns.some(pattern => pattern.test(fileName)); + + if (isDangerous && context) { + console.warn(`⚠️ Potentially dangerous file.name usage detected in ${context}: "${fileName}"`); + } + + return isDangerous; +} + +// Runtime validation for FileId usage in development +export function validateFileId(id: string, context: string): void { + if (process.env.NODE_ENV === 'development') { + // Check if it looks like a dangerous file.name usage + if (isDangerousFileNameAsId(id, context)) { + console.error(`💀 DANGEROUS: file.name used as FileId in ${context}! This will cause ID collisions.`); + console.trace('Stack trace:'); + } + } +} + +// Runtime validation for File vs FileWithId usage +export function validateFileWithId(file: File, context: string): void { + // Check if file has embedded fileId + if (!('fileId' in file)) { + console.warn(`⚠️ Regular File object used where FileWithId expected in ${context}: "${file.name}"`); + console.warn('Consider using FileWithId for better type safety'); + } +} + +// Assertion for FileId validation (throws in development) +export function assertValidFileId(id: string, context: string): void { + if (process.env.NODE_ENV === 'development') { + if (isDangerousFileNameAsId(id, context)) { + throw new Error(`ASSERTION FAILED: Dangerous file.name as FileId detected in ${context}: "${id}"`); + } + } +} + +// Global debug helpers (can be enabled in dev tools) +if (typeof window !== 'undefined') { + window.__FILE_ID_DEBUG = process.env.NODE_ENV === 'development'; + window.__validateFileId = validateFileId; +} \ No newline at end of file From 399a340170f674b436a474a14e47346c457beef8 Mon Sep 17 00:00:00 2001 From: Reece Browne <74901996+reecebrowne@users.noreply.github.com> Date: Wed, 3 Sep 2025 16:49:13 +0100 Subject: [PATCH 3/3] Update frontend/.eslintrc.js Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- frontend/.eslintrc.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/.eslintrc.js b/frontend/.eslintrc.js index cd779cd72..6146714b4 100644 --- a/frontend/.eslintrc.js +++ b/frontend/.eslintrc.js @@ -5,8 +5,7 @@ module.exports = { ], rules: { // Custom rules to prevent dangerous file.name as ID patterns - 'no-file-name-as-id': 'error', - 'prefer-file-with-id': 'warn' + 'no-file-name-as-id': 'error' }, overrides: [ {