From 4e0a4ef0198ffffd56a5a27b00b3a32bc72587ee Mon Sep 17 00:00:00 2001 From: jackiettran <41605212+jackiettran@users.noreply.github.com> Date: Fri, 19 Dec 2025 18:58:30 -0500 Subject: [PATCH] updated upload unit tests for s3 image handling --- backend/tests/unit/routes/upload.test.js | 140 ++++++- .../__tests__/services/uploadService.test.ts | 348 +++++++++++++++--- 2 files changed, 435 insertions(+), 53 deletions(-) diff --git a/backend/tests/unit/routes/upload.test.js b/backend/tests/unit/routes/upload.test.js index a1b1417..a455aa6 100644 --- a/backend/tests/unit/routes/upload.test.js +++ b/backend/tests/unit/routes/upload.test.js @@ -446,15 +446,137 @@ describe('Upload Routes', () => { }); }); - // Note: The GET /upload/signed-url/*key route uses Express 5 wildcard syntax - // which is not fully compatible with the test environment when mocking. - // The S3OwnershipService functionality is tested separately in s3OwnershipService.test.js - // The route integration is verified in integration tests. - describe('GET /upload/signed-url/*key (wildcard route)', () => { - it('should be defined as a route', () => { - // The route exists and is properly configured - // Full integration testing of wildcard routes is done in integration tests - expect(true).toBe(true); + describe('GET /upload/signed-url/:key(*)', () => { + const mockSignedUrl = 'https://bucket.s3.amazonaws.com/messages/uuid.jpg?signature=abc'; + + beforeEach(() => { + mockGetPresignedDownloadUrl.mockResolvedValue(mockSignedUrl); + mockCanAccessFile.mockResolvedValue({ authorized: true }); + }); + + it('should return signed URL for authorized private content (messages)', async () => { + const response = await request(app) + .get('/upload/signed-url/messages/550e8400-e29b-41d4-a716-446655440000.jpg') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(200); + expect(response.body.url).toBe(mockSignedUrl); + expect(response.body.expiresIn).toBe(3600); + + expect(mockCanAccessFile).toHaveBeenCalledWith( + 'messages/550e8400-e29b-41d4-a716-446655440000.jpg', + 'user-123' + ); + expect(mockGetPresignedDownloadUrl).toHaveBeenCalledWith( + 'messages/550e8400-e29b-41d4-a716-446655440000.jpg' + ); + }); + + it('should return signed URL for authorized condition-check content', async () => { + const response = await request(app) + .get('/upload/signed-url/condition-checks/550e8400-e29b-41d4-a716-446655440000.jpg') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(200); + expect(response.body.url).toBe(mockSignedUrl); + + expect(mockCanAccessFile).toHaveBeenCalledWith( + 'condition-checks/550e8400-e29b-41d4-a716-446655440000.jpg', + 'user-123' + ); + }); + + it('should require authentication', async () => { + const response = await request(app) + .get('/upload/signed-url/messages/uuid.jpg'); + + expect(response.status).toBe(401); + expect(mockGetPresignedDownloadUrl).not.toHaveBeenCalled(); + }); + + it('should return 503 when S3 is disabled', async () => { + mockIsEnabled.mockReturnValue(false); + + const response = await request(app) + .get('/upload/signed-url/messages/uuid.jpg') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(503); + }); + + it('should return 400 for public folder paths (items)', async () => { + const response = await request(app) + .get('/upload/signed-url/items/uuid.jpg') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(400); + expect(response.body.error).toBe('Signed URLs only for private content'); + expect(mockGetPresignedDownloadUrl).not.toHaveBeenCalled(); + }); + + it('should return 400 for public folder paths (profiles)', async () => { + const response = await request(app) + .get('/upload/signed-url/profiles/uuid.jpg') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(400); + expect(response.body.error).toBe('Signed URLs only for private content'); + }); + + it('should return 400 for public folder paths (forum)', async () => { + const response = await request(app) + .get('/upload/signed-url/forum/uuid.jpg') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(400); + expect(response.body.error).toBe('Signed URLs only for private content'); + }); + + it('should return 403 when user is not authorized to access file', async () => { + mockCanAccessFile.mockResolvedValue({ + authorized: false, + reason: 'Not a participant in this message' + }); + + const response = await request(app) + .get('/upload/signed-url/messages/uuid.jpg') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(403); + expect(response.body.error).toBe('Access denied'); + expect(mockGetPresignedDownloadUrl).not.toHaveBeenCalled(); + }); + + it('should handle URL-encoded keys', async () => { + const response = await request(app) + .get('/upload/signed-url/messages%2Fuuid.jpg') + .set('Authorization', 'Bearer valid-token'); + + // The key should be decoded + expect(mockCanAccessFile).toHaveBeenCalledWith( + expect.stringContaining('messages'), + 'user-123' + ); + }); + + it('should handle S3 service errors gracefully', async () => { + mockGetPresignedDownloadUrl.mockRejectedValue(new Error('S3 error')); + + const response = await request(app) + .get('/upload/signed-url/messages/uuid.jpg') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(500); + }); + + it('should handle ownership service errors gracefully', async () => { + mockCanAccessFile.mockRejectedValue(new Error('Database error')); + + const response = await request(app) + .get('/upload/signed-url/messages/uuid.jpg') + .set('Authorization', 'Bearer valid-token'); + + expect(response.status).toBe(500); }); }); }); diff --git a/frontend/src/__tests__/services/uploadService.test.ts b/frontend/src/__tests__/services/uploadService.test.ts index e42f908..c9908f0 100644 --- a/frontend/src/__tests__/services/uploadService.test.ts +++ b/frontend/src/__tests__/services/uploadService.test.ts @@ -23,12 +23,88 @@ jest.mock('../../services/api'); const mockedApi = api as jest.Mocked; +// Mock XMLHttpRequest for uploadToS3 tests +class MockXMLHttpRequest { + static instances: MockXMLHttpRequest[] = []; + + status = 200; + readyState = 4; + responseText = ''; + upload = { + onprogress: null as ((e: { lengthComputable: boolean; loaded: number; total: number }) => void) | null, + }; + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + + private headers: Record = {}; + private method = ''; + private url = ''; + + constructor() { + MockXMLHttpRequest.instances.push(this); + } + + open(method: string, url: string) { + this.method = method; + this.url = url; + } + + setRequestHeader(key: string, value: string) { + this.headers[key] = value; + } + + send(_data: unknown) { + // Use Promise.resolve().then for async behavior in tests + // This allows promises to resolve without real delays + Promise.resolve().then(() => { + if (this.upload.onprogress) { + this.upload.onprogress({ lengthComputable: true, loaded: 50, total: 100 }); + this.upload.onprogress({ lengthComputable: true, loaded: 100, total: 100 }); + } + if (this.onload) { + this.onload(); + } + }); + } + + getHeaders() { + return this.headers; + } + + getMethod() { + return this.method; + } + + getUrl() { + return this.url; + } + + static reset() { + MockXMLHttpRequest.instances = []; + } + + static getLastInstance() { + return MockXMLHttpRequest.instances[MockXMLHttpRequest.instances.length - 1]; + } +} + +// Store original XMLHttpRequest +const originalXMLHttpRequest = global.XMLHttpRequest; + describe('Upload Service', () => { beforeEach(() => { jest.clearAllMocks(); + MockXMLHttpRequest.reset(); // Reset environment variables process.env.REACT_APP_S3_BUCKET = 'test-bucket'; process.env.REACT_APP_AWS_REGION = 'us-east-1'; + // Mock XMLHttpRequest globally + (global as unknown as { XMLHttpRequest: typeof MockXMLHttpRequest }).XMLHttpRequest = MockXMLHttpRequest; + }); + + afterEach(() => { + // Restore original XMLHttpRequest + (global as unknown as { XMLHttpRequest: typeof XMLHttpRequest }).XMLHttpRequest = originalXMLHttpRequest; }); describe('getPublicImageUrl', () => { @@ -173,18 +249,42 @@ describe('Upload Service', () => { }); describe('uploadToS3', () => { - // Note: XMLHttpRequest mocking is complex and can cause timeouts. - // The uploadToS3 function is a thin wrapper around XHR. - // Testing focuses on verifying the function signature and basic behavior. + const mockFile = new File(['test content'], 'photo.jpg', { type: 'image/jpeg' }); + const mockUploadUrl = 'https://presigned-url.s3.amazonaws.com/items/uuid.jpg?signature=abc'; - it('should export uploadToS3 function', () => { - expect(typeof uploadToS3).toBe('function'); + it('should upload file successfully', async () => { + await uploadToS3(mockFile, mockUploadUrl); + + const instance = MockXMLHttpRequest.getLastInstance(); + expect(instance.getMethod()).toBe('PUT'); + expect(instance.getUrl()).toBe(mockUploadUrl); + expect(instance.getHeaders()['Content-Type']).toBe('image/jpeg'); }); - it('should accept file, url, and options parameters', () => { - // Verify function signature + it('should call onProgress callback during upload', async () => { + const onProgress = jest.fn(); + + await uploadToS3(mockFile, mockUploadUrl, { onProgress }); + + // Progress should be called at least once + expect(onProgress).toHaveBeenCalled(); + // Should receive percentage values + expect(onProgress).toHaveBeenCalledWith(expect.any(Number)); + }); + + it('should export uploadToS3 function with correct signature', () => { + expect(typeof uploadToS3).toBe('function'); + // Function accepts file, url, and optional options expect(uploadToS3.length).toBeGreaterThanOrEqual(2); }); + + it('should set correct content-type header', async () => { + const pngFile = new File(['test'], 'image.png', { type: 'image/png' }); + await uploadToS3(pngFile, mockUploadUrl); + + const instance = MockXMLHttpRequest.getLastInstance(); + expect(instance.getHeaders()['Content-Type']).toBe('image/png'); + }); }); describe('confirmUploads', () => { @@ -214,70 +314,230 @@ describe('Upload Service', () => { }); describe('uploadFile', () => { - it('should call getPresignedUrl and confirmUploads in sequence', async () => { - // Test the flow without mocking XMLHttpRequest (which is complex) - // Instead test that the functions are called with correct parameters - const file = new File(['test'], 'photo.jpg', { type: 'image/jpeg' }); - const presignResponse: PresignedUrlResponse = { - uploadUrl: 'https://presigned.s3.amazonaws.com', - key: 'items/uuid.jpg', - publicUrl: 'https://bucket.s3.amazonaws.com/items/uuid.jpg', - expiresAt: new Date().toISOString(), - }; + const mockFile = new File(['test content'], 'photo.jpg', { type: 'image/jpeg' }); + const presignResponse: PresignedUrlResponse = { + uploadUrl: 'https://presigned.s3.amazonaws.com/items/uuid.jpg', + key: 'items/uuid.jpg', + publicUrl: 'https://bucket.s3.amazonaws.com/items/uuid.jpg', + expiresAt: new Date().toISOString(), + }; + it('should complete full upload flow successfully', async () => { + // Mock presign response mockedApi.post.mockResolvedValueOnce({ data: presignResponse }); + // Mock confirm response + mockedApi.post.mockResolvedValueOnce({ + data: { confirmed: [presignResponse.key], total: 1 }, + }); - // Just test getPresignedUrl is called correctly - await getPresignedUrl('item', file); + const result = await uploadFile('item', mockFile); + expect(result).toEqual({ + key: presignResponse.key, + publicUrl: presignResponse.publicUrl, + }); + + // Verify presign was called expect(mockedApi.post).toHaveBeenCalledWith('/upload/presign', { uploadType: 'item', contentType: 'image/jpeg', fileName: 'photo.jpg', - fileSize: file.size, + fileSize: mockFile.size, }); + + // Verify confirm was called + expect(mockedApi.post).toHaveBeenCalledWith('/upload/confirm', { + keys: [presignResponse.key], + }); + }); + + it('should throw error when upload verification fails', async () => { + mockedApi.post.mockResolvedValueOnce({ data: presignResponse }); + // Mock confirm returning empty confirmed array + mockedApi.post.mockResolvedValueOnce({ + data: { confirmed: [], total: 1 }, + }); + + await expect(uploadFile('item', mockFile)).rejects.toThrow('Upload verification failed'); + }); + + it('should pass onProgress to uploadToS3', async () => { + const onProgress = jest.fn(); + + mockedApi.post.mockResolvedValueOnce({ data: presignResponse }); + mockedApi.post.mockResolvedValueOnce({ + data: { confirmed: [presignResponse.key], total: 1 }, + }); + + await uploadFile('item', mockFile, { onProgress }); + + // onProgress should have been called during XHR upload + expect(onProgress).toHaveBeenCalled(); + }); + + it('should work with different upload types', async () => { + const messagePresignResponse = { + ...presignResponse, + key: 'messages/uuid.jpg', + publicUrl: null, // Messages are private + }; + + mockedApi.post.mockResolvedValueOnce({ data: messagePresignResponse }); + mockedApi.post.mockResolvedValueOnce({ + data: { confirmed: [messagePresignResponse.key], total: 1 }, + }); + + const result = await uploadFile('message', mockFile); + + expect(result.key).toBe('messages/uuid.jpg'); + expect(mockedApi.post).toHaveBeenCalledWith('/upload/presign', expect.objectContaining({ + uploadType: 'message', + })); }); }); describe('uploadFiles', () => { + const mockFiles = [ + new File(['test1'], 'photo1.jpg', { type: 'image/jpeg' }), + new File(['test2'], 'photo2.png', { type: 'image/png' }), + ]; + + const presignResponses: PresignedUrlResponse[] = [ + { + uploadUrl: 'https://presigned1.s3.amazonaws.com/items/uuid1.jpg', + key: 'items/uuid1.jpg', + publicUrl: 'https://bucket.s3.amazonaws.com/items/uuid1.jpg', + expiresAt: new Date().toISOString(), + }, + { + uploadUrl: 'https://presigned2.s3.amazonaws.com/items/uuid2.png', + key: 'items/uuid2.png', + publicUrl: 'https://bucket.s3.amazonaws.com/items/uuid2.png', + expiresAt: new Date().toISOString(), + }, + ]; + it('should return empty array for empty files array', async () => { const result = await uploadFiles('item', []); expect(result).toEqual([]); expect(mockedApi.post).not.toHaveBeenCalled(); }); - it('should call getPresignedUrls with correct parameters', async () => { - const files = [ - new File(['test1'], 'photo1.jpg', { type: 'image/jpeg' }), - new File(['test2'], 'photo2.png', { type: 'image/png' }), - ]; - - const presignResponses: PresignedUrlResponse[] = [ - { - uploadUrl: 'https://presigned1.s3.amazonaws.com', - key: 'items/uuid1.jpg', - publicUrl: 'https://bucket.s3.amazonaws.com/items/uuid1.jpg', - expiresAt: new Date().toISOString(), - }, - { - uploadUrl: 'https://presigned2.s3.amazonaws.com', - key: 'items/uuid2.png', - publicUrl: 'https://bucket.s3.amazonaws.com/items/uuid2.png', - expiresAt: new Date().toISOString(), - }, - ]; - + it('should complete full batch upload flow successfully', async () => { mockedApi.post.mockResolvedValueOnce({ data: { uploads: presignResponses } }); + mockedApi.post.mockResolvedValueOnce({ + data: { + confirmed: presignResponses.map((p) => p.key), + total: 2, + }, + }); - await getPresignedUrls('item', files); + const result = await uploadFiles('item', mockFiles); + expect(result).toHaveLength(2); + expect(result[0]).toEqual({ + key: 'items/uuid1.jpg', + publicUrl: 'https://bucket.s3.amazonaws.com/items/uuid1.jpg', + }); + expect(result[1]).toEqual({ + key: 'items/uuid2.png', + publicUrl: 'https://bucket.s3.amazonaws.com/items/uuid2.png', + }); + + // Verify batch presign was called expect(mockedApi.post).toHaveBeenCalledWith('/upload/presign-batch', { uploadType: 'item', files: [ - { contentType: 'image/jpeg', fileName: 'photo1.jpg', fileSize: files[0].size }, - { contentType: 'image/png', fileName: 'photo2.png', fileSize: files[1].size }, + { contentType: 'image/jpeg', fileName: 'photo1.jpg', fileSize: mockFiles[0].size }, + { contentType: 'image/png', fileName: 'photo2.png', fileSize: mockFiles[1].size }, ], }); + + // Verify confirm was called with all keys + expect(mockedApi.post).toHaveBeenCalledWith('/upload/confirm', { + keys: ['items/uuid1.jpg', 'items/uuid2.png'], + }); + }); + + it('should filter out unconfirmed uploads', async () => { + mockedApi.post.mockResolvedValueOnce({ data: { uploads: presignResponses } }); + // Only first file confirmed + mockedApi.post.mockResolvedValueOnce({ + data: { + confirmed: ['items/uuid1.jpg'], + total: 2, + }, + }); + + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); + + const result = await uploadFiles('item', mockFiles); + + // Only confirmed uploads should be returned + expect(result).toHaveLength(1); + expect(result[0].key).toBe('items/uuid1.jpg'); + + // Should log warning about failed verification + expect(consoleSpy).toHaveBeenCalledWith('1 uploads failed verification'); + + consoleSpy.mockRestore(); + }); + + it('should handle all uploads failing verification', async () => { + mockedApi.post.mockResolvedValueOnce({ data: { uploads: presignResponses } }); + mockedApi.post.mockResolvedValueOnce({ + data: { + confirmed: [], + total: 2, + }, + }); + + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); + + const result = await uploadFiles('item', mockFiles); + + expect(result).toHaveLength(0); + expect(consoleSpy).toHaveBeenCalledWith('2 uploads failed verification'); + + consoleSpy.mockRestore(); + }); + + it('should upload all files in parallel', async () => { + mockedApi.post.mockResolvedValueOnce({ data: { uploads: presignResponses } }); + mockedApi.post.mockResolvedValueOnce({ + data: { + confirmed: presignResponses.map((p) => p.key), + total: 2, + }, + }); + + await uploadFiles('item', mockFiles); + + // Should have created 2 XHR instances for parallel uploads + expect(MockXMLHttpRequest.instances.length).toBe(2); + }); + + it('should work with different upload types', async () => { + const forumResponses = presignResponses.map((r) => ({ + ...r, + key: r.key.replace('items/', 'forum/'), + publicUrl: r.publicUrl.replace('items/', 'forum/'), + })); + + mockedApi.post.mockResolvedValueOnce({ data: { uploads: forumResponses } }); + mockedApi.post.mockResolvedValueOnce({ + data: { + confirmed: forumResponses.map((p) => p.key), + total: 2, + }, + }); + + const result = await uploadFiles('forum', mockFiles); + + expect(result[0].key).toBe('forum/uuid1.jpg'); + expect(mockedApi.post).toHaveBeenCalledWith('/upload/presign-batch', expect.objectContaining({ + uploadType: 'forum', + })); }); });