From a6987bc379a59d3d64e5860d216fec5a90c6fa44 Mon Sep 17 00:00:00 2001 From: bymyself Date: Tue, 20 Jan 2026 14:55:36 -0800 Subject: [PATCH] Address CodeRabbit review comments - Import ImageRef from maskEditorDataStore instead of duplicating - Replace 'any' with proper UploadApiResponse type - Add validation for dataURL strings - Fix test mocking: use vi.spyOn for global.fetch - Fix uploadMediaBatch test to use distinct response mocks - Add test for invalid dataURL rejection - Fix uploadMultipleFiles to return array of successful paths - Optimize file size test to avoid timeout --- src/extensions/core/load3d/Load3dUtils.ts | 8 +++- .../assets/services/uploadService.test.ts | 43 ++++++++++++++----- src/platform/assets/services/uploadService.ts | 33 ++++++++++---- 3 files changed, 62 insertions(+), 22 deletions(-) diff --git a/src/extensions/core/load3d/Load3dUtils.ts b/src/extensions/core/load3d/Load3dUtils.ts index 6ed3c71e9..ff9224225 100644 --- a/src/extensions/core/load3d/Load3dUtils.ts +++ b/src/extensions/core/load3d/Load3dUtils.ts @@ -108,12 +108,16 @@ class Load3dUtils { return `/view?${params}` } - static async uploadMultipleFiles(files: FileList, subfolder: string = '3d') { + static async uploadMultipleFiles( + files: FileList, + subfolder: string = '3d' + ): Promise { const uploadPromises = Array.from(files).map((file) => this.uploadFile(file, subfolder) ) - await Promise.all(uploadPromises) + const results = await Promise.all(uploadPromises) + return results.filter((path): path is string => path !== undefined) } static getThumbnailFilename(modelFilename: string): string { diff --git a/src/platform/assets/services/uploadService.test.ts b/src/platform/assets/services/uploadService.test.ts index abcb079ee..83f708398 100644 --- a/src/platform/assets/services/uploadService.test.ts +++ b/src/platform/assets/services/uploadService.test.ts @@ -56,9 +56,9 @@ describe('uploadService', () => { it('uploads dataURL successfully', async () => { const dataURL = 'data:image/png;base64,iVBORw0KGgo=' - global.fetch = vi.fn().mockResolvedValue({ + const fetchSpy = vi.spyOn(global, 'fetch').mockResolvedValue({ blob: () => Promise.resolve(new Blob(['content'])) - }) + } as Response) const mockResponse = { status: 200, @@ -70,9 +70,21 @@ describe('uploadService', () => { vi.mocked(api.fetchApi).mockResolvedValue(mockResponse as any) - const result = await uploadMedia({ source: dataURL }) + try { + const result = await uploadMedia({ source: dataURL }) + expect(result.success).toBe(true) + } finally { + fetchSpy.mockRestore() + } + }) - expect(result.success).toBe(true) + it('rejects invalid dataURL', async () => { + const invalidURL = 'not-a-data-url' + + const result = await uploadMedia({ source: invalidURL }) + + expect(result.success).toBe(false) + expect(result.error).toContain('Invalid data URL') }) it('includes subfolder in FormData', async () => { @@ -96,7 +108,12 @@ describe('uploadService', () => { }) it('validates file size', async () => { - const largeFile = new File(['x'.repeat(200 * 1024 * 1024)], 'large.png') + // Create a file that reports as 200MB without actually allocating that much memory + const largeFile = new File(['content'], 'large.png') + Object.defineProperty(largeFile, 'size', { + value: 200 * 1024 * 1024, + writable: false + }) const result = await uploadMedia( { source: largeFile }, @@ -166,15 +183,19 @@ describe('uploadService', () => { new File(['2'], 'file2.png') ] - const mockResponse = { + const mockResponse1 = { status: 200, - json: vi - .fn() - .mockResolvedValueOnce({ name: 'file1.png' }) - .mockResolvedValueOnce({ name: 'file2.png' }) + json: vi.fn().mockResolvedValue({ name: 'file1.png', subfolder: '' }) } - vi.mocked(api.fetchApi).mockResolvedValue(mockResponse as any) + const mockResponse2 = { + status: 200, + json: vi.fn().mockResolvedValue({ name: 'file2.png', subfolder: '' }) + } + + vi.mocked(api.fetchApi) + .mockResolvedValueOnce(mockResponse1 as any) + .mockResolvedValueOnce(mockResponse2 as any) const results = await uploadMediaBatch( mockFiles.map((source) => ({ source })) diff --git a/src/platform/assets/services/uploadService.ts b/src/platform/assets/services/uploadService.ts index b98687a65..72f638969 100644 --- a/src/platform/assets/services/uploadService.ts +++ b/src/platform/assets/services/uploadService.ts @@ -1,5 +1,6 @@ import type { ResultItemType } from '@/schemas/apiSchema' import { api } from '@/scripts/api' +import type { ImageRef } from '@/stores/maskEditorDataStore' interface UploadInput { source: File | Blob | string @@ -14,10 +15,10 @@ interface UploadConfig { maxSizeMB?: number } -interface ImageRef { - filename: string - subfolder: string - type: string +interface UploadApiResponse { + name: string + subfolder?: string + type?: string } interface UploadResult { @@ -26,7 +27,11 @@ interface UploadResult { name: string subfolder: string error?: string - response: any + response: UploadApiResponse | null +} + +function isDataURL(str: string): boolean { + return typeof str === 'string' && str.startsWith('data:') } async function convertToFile( @@ -45,9 +50,19 @@ async function convertToFile( } // dataURL string - const blob = await fetch(source).then((r) => r.blob()) - const name = filename || `upload-${Date.now()}.png` - return new File([blob], name, { type: mimeType }) + if (!isDataURL(source)) { + throw new Error(`Invalid data URL: ${source.substring(0, 50)}...`) + } + + try { + const blob = await fetch(source).then((r) => r.blob()) + const name = filename || `upload-${Date.now()}.png` + return new File([blob], name, { type: mimeType }) + } catch (error) { + throw new Error( + `Failed to convert data URL to file: ${error instanceof Error ? error.message : String(error)}` + ) + } } function validateFileSize(file: File, maxSizeMB?: number): string | null { @@ -110,7 +125,7 @@ export async function uploadMedia( } } - const data = await resp.json() + const data: UploadApiResponse = await resp.json() const path = data.subfolder ? `${data.subfolder}/${data.name}` : data.name return {