From 5f604362dfda619e9f82165c2d81a0fc4c1b6b03 Mon Sep 17 00:00:00 2001 From: hugo Date: Sun, 15 Feb 2026 13:28:51 +0100 Subject: [PATCH] fix: thumbnail generation on image change --- src/main/engine/MediaEngine.ts | 72 ++++++ src/main/ipc/handlers.ts | 38 +++ src/main/preload.ts | 2 + src/renderer/components/Editor/Editor.tsx | 22 +- src/renderer/types/electron.d.ts | 2 + tests/engine/MediaEngine.test.ts | 273 ++++++++++++++++++++++ tests/engine/MetaEngine.test.ts | 22 ++ 7 files changed, 430 insertions(+), 1 deletion(-) diff --git a/src/main/engine/MediaEngine.ts b/src/main/engine/MediaEngine.ts index da9bf36..9718b8a 100644 --- a/src/main/engine/MediaEngine.ts +++ b/src/main/engine/MediaEngine.ts @@ -611,6 +611,78 @@ export class MediaEngine extends EventEmitter { return updated; } + /** + * Replace the actual file content for an existing media item. + * This will: + * - Check if the new file has a different checksum + * - Replace the file if checksum differs + * - Update size, dimensions (for images), and checksum in database + * - Regenerate thumbnails for images + * + * @returns The updated MediaData if file was replaced, null if media not found or checksum unchanged + */ + async replaceMediaFile(id: string, newSourcePath: string): Promise { + const db = getDatabase().getLocal(); + const dbMedia = await db.select().from(media).where(eq(media.id, id)).get(); + + if (!dbMedia) { + return null; + } + + // Read the new source file + const newBuffer = await fs.readFile(newSourcePath); + const newChecksum = this.calculateChecksum(newBuffer); + + // If checksum is the same, no need to replace + if (dbMedia.checksum === newChecksum) { + return null; + } + + // Copy new file to existing location + await fs.copyFile(newSourcePath, dbMedia.filePath); + + // Get new dimensions for images + let width = dbMedia.width; + let height = dbMedia.height; + if (dbMedia.mimeType.startsWith('image/') && !dbMedia.mimeType.includes('svg')) { + try { + const sharp = (await import('sharp')).default; + const imageMetadata = await sharp(dbMedia.filePath).metadata(); + width = imageMetadata.width ?? width; + height = imageMetadata.height ?? height; + } catch (error) { + console.error('Failed to get image dimensions:', error); + } + } + + const now = new Date(); + + // Update database + await db.update(media) + .set({ + size: newBuffer.length, + width, + height, + checksum: newChecksum, + updatedAt: now, + }) + .where(eq(media.id, id)); + + // Regenerate thumbnails for images + if (dbMedia.mimeType.startsWith('image/') && !dbMedia.mimeType.includes('svg')) { + // Await thumbnail generation to ensure it completes before returning + await this.generateThumbnails(id, dbMedia.filePath); + } + + // Get the updated media data + const updated = await this.getMedia(id); + if (updated) { + this.emit('mediaFileReplaced', updated); + } + + return updated; + } + async deleteMedia(id: string): Promise { const db = getDatabase().getLocal(); const existing = await db.select().from(media).where(eq(media.id, id)).get(); diff --git a/src/main/ipc/handlers.ts b/src/main/ipc/handlers.ts index bc93f26..96de629 100644 --- a/src/main/ipc/handlers.ts +++ b/src/main/ipc/handlers.ts @@ -326,6 +326,43 @@ export function registerIpcHandlers(): void { return engine.updateMedia(id, data); }); + safeHandle('media:replaceFile', async (_, id: string, newSourcePath: string) => { + const engine = getMediaEngine(); + return engine.replaceMediaFile(id, newSourcePath); + }); + + safeHandle('media:replaceFileDialog', async (_, id: string) => { + // Get the current media to determine file type filter + const engine = getMediaEngine(); + const currentMedia = await engine.getMedia(id); + if (!currentMedia) { + return null; + } + + // Build filter based on current media type + let filters: { name: string; extensions: string[] }[] = []; + if (currentMedia.mimeType.startsWith('image/')) { + filters = [ + { name: 'Images', extensions: ['jpg', 'jpeg', 'png', 'gif', 'webp', 'svg', 'bmp'] }, + { name: 'All Files', extensions: ['*'] }, + ]; + } else { + filters = [{ name: 'All Files', extensions: ['*'] }]; + } + + const result = await dialog.showOpenDialog({ + title: 'Replace Media File', + filters, + properties: ['openFile'], + }); + + if (result.canceled || result.filePaths.length === 0) { + return null; + } + + return engine.replaceMediaFile(id, result.filePaths[0]); + }); + safeHandle('media:delete', async (_, id: string) => { const engine = getMediaEngine(); return engine.deleteMedia(id); @@ -979,6 +1016,7 @@ export function registerIpcHandlers(): void { mediaEngine.on('mediaImported', forwardEvent('media:imported')); mediaEngine.on('mediaUpdated', forwardEvent('media:updated')); mediaEngine.on('mediaDeleted', forwardEvent('media:deleted')); + mediaEngine.on('mediaFileReplaced', forwardEvent('media:fileReplaced')); mediaEngine.on('rebuildStarted', forwardEvent('media:rebuildStarted')); mediaEngine.on('databaseRebuilt', forwardEvent('media:databaseRebuilt')); diff --git a/src/main/preload.ts b/src/main/preload.ts index c9b2761..2bc64a9 100644 --- a/src/main/preload.ts +++ b/src/main/preload.ts @@ -48,6 +48,8 @@ contextBridge.exposeInMainWorld('electronAPI', { import: (sourcePath: string, metadata?: unknown) => ipcRenderer.invoke('media:import', sourcePath, metadata), importDialog: () => ipcRenderer.invoke('media:importDialog'), update: (id: string, data: unknown) => ipcRenderer.invoke('media:update', id, data), + replaceFile: (id: string, newSourcePath: string) => ipcRenderer.invoke('media:replaceFile', id, newSourcePath), + replaceFileDialog: (id: string) => ipcRenderer.invoke('media:replaceFileDialog', id), delete: (id: string) => ipcRenderer.invoke('media:delete', id), get: (id: string) => ipcRenderer.invoke('media:get', id), getUrl: (id: string) => ipcRenderer.invoke('media:getUrl', id), diff --git a/src/renderer/components/Editor/Editor.tsx b/src/renderer/components/Editor/Editor.tsx index af179a1..0f7b91c 100644 --- a/src/renderer/components/Editor/Editor.tsx +++ b/src/renderer/components/Editor/Editor.tsx @@ -1644,6 +1644,25 @@ const MediaEditor: React.FC<{ mediaId: string }> = ({ mediaId }) => { } }; + const handleReplaceFile = async () => { + try { + const updated = await window.electronAPI?.media.replaceFileDialog(item.id); + if (updated) { + updateMedia(item.id, updated as Partial); + showToast.success('File replaced (thumbnails regenerated)'); + } + // null means user cancelled or file unchanged - no action needed + } catch (error) { + console.error('Failed to replace media file:', error); + const err = error as Error; + showErrorModal({ + title: 'Replace Failed', + message: err.message || 'Failed to replace media file', + stack: err.stack, + }); + } + }; + const handleDelete = async () => { try { // Fetch posts that link to this media @@ -1735,6 +1754,7 @@ const MediaEditor: React.FC<{ mediaId: string }> = ({ mediaId }) => { )} )} + @@ -1745,7 +1765,7 @@ const MediaEditor: React.FC<{ mediaId: string }> = ({ mediaId }) => { {item.mimeType.startsWith('image/') ? (
{item.alt { // Fallback to placeholder if image fails to load diff --git a/src/renderer/types/electron.d.ts b/src/renderer/types/electron.d.ts index 0d0b398..77b93de 100644 --- a/src/renderer/types/electron.d.ts +++ b/src/renderer/types/electron.d.ts @@ -302,6 +302,8 @@ export interface ElectronAPI { import: (sourcePath: string, metadata?: Partial) => Promise; importDialog: () => Promise; update: (id: string, data: Partial) => Promise; + replaceFile: (id: string, newSourcePath: string) => Promise; + replaceFileDialog: (id: string) => Promise; delete: (id: string) => Promise; get: (id: string) => Promise; getUrl: (id: string) => Promise; diff --git a/tests/engine/MediaEngine.test.ts b/tests/engine/MediaEngine.test.ts index b7f5458..f7957c7 100644 --- a/tests/engine/MediaEngine.test.ts +++ b/tests/engine/MediaEngine.test.ts @@ -237,11 +237,20 @@ describe('MediaEngine', () => { }); describe('Media Import', () => { + let consoleErrorSpy: ReturnType; + beforeEach(() => { + // Spy on console.error to suppress expected error output (sharp can't read mock files) + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + // Setup a source file for import const imageBuffer = Buffer.from('fake-image-data'); mockFiles.set('/source/image.jpg', imageBuffer); }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + }); it('should import media from source path', async () => { const media = await mediaEngine.importMedia('/source/image.jpg'); @@ -1434,6 +1443,9 @@ tags: ["nature", "sunset"]`; it('should skip non-image media', async () => { const fs = await import('fs/promises'); + // Spy on console.error to suppress expected error output + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.mocked(mockLocalDb.select).mockImplementation(() => { const chain = createSelectChain(); chain.where = vi.fn().mockReturnValue({ @@ -1455,6 +1467,14 @@ tags: ["nature", "sunset"]`; call => String(call[0]).includes('thumbnail') ); expect(thumbnailWrites).toHaveLength(0); + + // Verify error was logged (graceful degradation behavior) + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to generate thumbnails:', + expect.any(Error) + ); + + consoleErrorSpy.mockRestore(); }); }); @@ -1497,4 +1517,257 @@ tags: ["nature", "sunset"]`; expect(paths.large).toBeNull(); }); }); + + describe('replaceMediaFile', () => { + let consoleErrorSpy: ReturnType; + + beforeEach(() => { + // Spy on console.error to suppress expected error output (sharp can't read mock files) + consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(() => { + consoleErrorSpy.mockRestore(); + }); + + it('should return null for non-existent media', async () => { + // Mock database to return nothing + vi.mocked(mockLocalDb.select).mockImplementation(() => { + const chain = createSelectChain(); + chain.where = vi.fn().mockReturnValue({ + ...chain, + get: vi.fn().mockResolvedValue(undefined), + }); + return chain; + }); + + const result = await mediaEngine.replaceMediaFile('non-existent-id', '/source/new-image.jpg'); + expect(result).toBeNull(); + }); + + it('should replace file and update database when checksum differs', async () => { + const fs = await import('fs/promises'); + + // Spy on console.error to suppress expected error output (sharp can't read mock file) + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + // Setup new source file with different content + const newImageBuffer = Buffer.from('new-image-data-different'); + mockFiles.set('/source/new-image.jpg', newImageBuffer); + + // Setup existing media in database with different checksum + const existingMedia = { + id: 'media-id-123', + projectId: 'default', + filename: 'media-id-123.jpg', + originalName: 'original.jpg', + mimeType: 'image/jpeg', + size: 100, + width: 800, + height: 600, + filePath: '/mock/media/2025/01/media-id-123.jpg', + sidecarPath: '/mock/media/2025/01/media-id-123.jpg.meta', + createdAt: new Date('2025-01-15'), + updatedAt: new Date('2025-01-15'), + checksum: 'old-checksum', + tags: '[]', + }; + + vi.mocked(mockLocalDb.select).mockImplementation(() => { + const chain = createSelectChain(); + chain.where = vi.fn().mockReturnValue({ + ...chain, + get: vi.fn().mockResolvedValue(existingMedia), + }); + return chain; + }); + + // Clear any previous mock calls + vi.mocked(fs.copyFile).mockClear(); + + const result = await mediaEngine.replaceMediaFile('media-id-123', '/source/new-image.jpg'); + + expect(result).not.toBeNull(); + expect(result!.id).toBe('media-id-123'); + // File should be copied to the existing location + expect(fs.copyFile).toHaveBeenCalledWith('/source/new-image.jpg', existingMedia.filePath); + + // Verify error was logged (graceful degradation for image dimensions) + expect(consoleErrorSpy).toHaveBeenCalledWith( + 'Failed to get image dimensions:', + expect.any(Error) + ); + + consoleErrorSpy.mockRestore(); + }); + + it('should not replace file when checksum is the same', async () => { + const fs = await import('fs/promises'); + const crypto = await import('crypto'); + + // Create content that we know the checksum of + const imageBuffer = Buffer.from('same-content'); + const checksum = crypto.createHash('md5').update(imageBuffer).digest('hex'); + mockFiles.set('/source/same-image.jpg', imageBuffer); + + // Setup existing media with same checksum + const existingMedia = { + id: 'media-id-456', + projectId: 'default', + filename: 'media-id-456.jpg', + originalName: 'original.jpg', + mimeType: 'image/jpeg', + size: imageBuffer.length, + width: 800, + height: 600, + filePath: '/mock/media/2025/01/media-id-456.jpg', + sidecarPath: '/mock/media/2025/01/media-id-456.jpg.meta', + createdAt: new Date('2025-01-15'), + updatedAt: new Date('2025-01-15'), + checksum: checksum, // Same checksum as the source file + tags: '[]', + }; + + vi.mocked(mockLocalDb.select).mockImplementation(() => { + const chain = createSelectChain(); + chain.where = vi.fn().mockReturnValue({ + ...chain, + get: vi.fn().mockResolvedValue(existingMedia), + }); + return chain; + }); + + vi.mocked(fs.copyFile).mockClear(); + + const result = await mediaEngine.replaceMediaFile('media-id-456', '/source/same-image.jpg'); + + // Should return null because file hasn't changed + expect(result).toBeNull(); + // File should NOT be copied + expect(fs.copyFile).not.toHaveBeenCalled(); + }); + + it('should emit mediaFileReplaced event when file is replaced', async () => { + const fs = await import('fs/promises'); + + const newImageBuffer = Buffer.from('event-test-data'); + mockFiles.set('/source/event-test.jpg', newImageBuffer); + + const existingMedia = { + id: 'media-event-id', + projectId: 'default', + filename: 'media-event-id.jpg', + originalName: 'original.jpg', + mimeType: 'image/jpeg', + size: 100, + width: 800, + height: 600, + filePath: '/mock/media/2025/01/media-event-id.jpg', + sidecarPath: '/mock/media/2025/01/media-event-id.jpg.meta', + createdAt: new Date('2025-01-15'), + updatedAt: new Date('2025-01-15'), + checksum: 'different-checksum', + tags: '[]', + }; + + vi.mocked(mockLocalDb.select).mockImplementation(() => { + const chain = createSelectChain(); + chain.where = vi.fn().mockReturnValue({ + ...chain, + get: vi.fn().mockResolvedValue(existingMedia), + }); + return chain; + }); + + const eventHandler = vi.fn(); + mediaEngine.on('mediaFileReplaced', eventHandler); + + await mediaEngine.replaceMediaFile('media-event-id', '/source/event-test.jpg'); + + expect(eventHandler).toHaveBeenCalledWith( + expect.objectContaining({ id: 'media-event-id' }) + ); + }); + + it('should call generateThumbnails for image files when checksum differs', async () => { + const newImageBuffer = Buffer.from('thumbnail-test-data'); + mockFiles.set('/source/thumb-test.jpg', newImageBuffer); + + const existingMedia = { + id: 'media-thumb-id', + projectId: 'default', + filename: 'media-thumb-id.jpg', + originalName: 'original.jpg', + mimeType: 'image/jpeg', + size: 100, + width: 800, + height: 600, + filePath: '/mock/media/2025/01/media-thumb-id.jpg', + sidecarPath: '/mock/media/2025/01/media-thumb-id.jpg.meta', + createdAt: new Date('2025-01-15'), + updatedAt: new Date('2025-01-15'), + checksum: 'old-checksum-different', + tags: '[]', + }; + + vi.mocked(mockLocalDb.select).mockImplementation(() => { + const chain = createSelectChain(); + chain.where = vi.fn().mockReturnValue({ + ...chain, + get: vi.fn().mockResolvedValue(existingMedia), + }); + return chain; + }); + + // Spy on generateThumbnails + const generateThumbnailsSpy = vi.spyOn(mediaEngine, 'generateThumbnails').mockResolvedValue({ + small: '/mock/thumbnails/media-thumb-id-small.webp', + medium: '/mock/thumbnails/media-thumb-id-medium.webp', + large: '/mock/thumbnails/media-thumb-id-large.webp', + }); + + await mediaEngine.replaceMediaFile('media-thumb-id', '/source/thumb-test.jpg'); + + expect(generateThumbnailsSpy).toHaveBeenCalledWith('media-thumb-id', existingMedia.filePath); + + generateThumbnailsSpy.mockRestore(); + }); + + it('should not generate thumbnails for non-image files', async () => { + const pdfBuffer = Buffer.from('pdf-content'); + mockFiles.set('/source/document.pdf', pdfBuffer); + + const existingMedia = { + id: 'media-pdf-id', + projectId: 'default', + filename: 'media-pdf-id.pdf', + originalName: 'document.pdf', + mimeType: 'application/pdf', + size: 100, + filePath: '/mock/media/2025/01/media-pdf-id.pdf', + sidecarPath: '/mock/media/2025/01/media-pdf-id.pdf.meta', + createdAt: new Date('2025-01-15'), + updatedAt: new Date('2025-01-15'), + checksum: 'old-pdf-checksum', + tags: '[]', + }; + + vi.mocked(mockLocalDb.select).mockImplementation(() => { + const chain = createSelectChain(); + chain.where = vi.fn().mockReturnValue({ + ...chain, + get: vi.fn().mockResolvedValue(existingMedia), + }); + return chain; + }); + + const generateThumbnailsSpy = vi.spyOn(mediaEngine, 'generateThumbnails'); + + await mediaEngine.replaceMediaFile('media-pdf-id', '/source/document.pdf'); + + expect(generateThumbnailsSpy).not.toHaveBeenCalled(); + + generateThumbnailsSpy.mockRestore(); + }); + }); }); diff --git a/tests/engine/MetaEngine.test.ts b/tests/engine/MetaEngine.test.ts index 8abe9bd..77116b6 100644 --- a/tests/engine/MetaEngine.test.ts +++ b/tests/engine/MetaEngine.test.ts @@ -480,11 +480,22 @@ describe('MetaEngine', () => { }); it('should throw non-ENOENT errors when loading project metadata', async () => { + // Spy on console.error to suppress expected error output + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + // Mock readFile to throw a non-ENOENT error const originalReadFile = vi.mocked(fs.readFile); originalReadFile.mockRejectedValueOnce(Object.assign(new Error('Permission denied'), { code: 'EACCES' })); await expect(metaEngine.loadProjectMetadata()).rejects.toThrow('Permission denied'); + + // Verify error was logged before rethrowing + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[MetaEngine] Failed to load project metadata:', + expect.any(Error) + ); + + consoleErrorSpy.mockRestore(); }); it('should handle ENOENT error when loading categories (no file)', async () => { @@ -496,11 +507,22 @@ describe('MetaEngine', () => { }); it('should throw non-ENOENT errors when loading categories', async () => { + // Spy on console.error to suppress expected error output + const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + // Mock readFile to throw a non-ENOENT error const originalReadFile = vi.mocked(fs.readFile); originalReadFile.mockRejectedValueOnce(Object.assign(new Error('Disk full'), { code: 'ENOSPC' })); await expect(metaEngine.loadCategories()).rejects.toThrow('Disk full'); + + // Verify error was logged before rethrowing + expect(consoleErrorSpy).toHaveBeenCalledWith( + '[MetaEngine] Failed to load categories:', + expect.any(Error) + ); + + consoleErrorSpy.mockRestore(); }); it('should emit projectMetadataChanged event when metadata is modified', async () => {