From 7f683fa6bc237f0a8bd7d0d7d51e1145b38f8b87 Mon Sep 17 00:00:00 2001 From: StarAppeal Date: Fri, 26 Sep 2025 04:01:19 +0200 Subject: [PATCH] implement file duplication check in upload process and enhance test coverage --- src/rest/restStorage.ts | 6 ++ src/services/s3Service.ts | 36 +++++++- tests/helpers/testSetup.ts | 9 +- tests/rest/restStorage.test.ts | 23 +++++ tests/services/s3Service.test.ts | 153 +++++++++++++++++++++++++++---- 5 files changed, 204 insertions(+), 23 deletions(-) diff --git a/src/rest/restStorage.ts b/src/rest/restStorage.ts index a01819e..a2e9821 100644 --- a/src/rest/restStorage.ts +++ b/src/rest/restStorage.ts @@ -24,6 +24,12 @@ export class RestStorage { } const userId = req.payload.uuid; + + const isDuplicate = await this.s3Service.isFileDuplicate(req.file, userId); + if (isDuplicate) { + return badRequest(res, "File was already uploaded."); + } + const objectKey = await this.s3Service.uploadFile(req.file, userId); return created(res, { message: "File uploaded successfully", objectKey }); diff --git a/src/services/s3Service.ts b/src/services/s3Service.ts index fe649b1..b5b1bce 100644 --- a/src/services/s3Service.ts +++ b/src/services/s3Service.ts @@ -65,21 +65,23 @@ export class S3Service { } async uploadFile(file: Express.Multer.File, userId: string): Promise { - const fileExtension = file.originalname.split(".").pop(); - const objectKey = `user-${userId}/${randomUUID()}.${fileExtension}`; + const objectKey = `user-${userId}/${randomUUID()}_${file.originalname}`; const command = new PutObjectCommand({ Bucket: this.bucketName, Key: objectKey, Body: file.buffer, ContentType: file.mimetype, + Metadata: { + originalname: encodeURIComponent(file.originalname), + }, }); await this.client.send(command); return objectKey; } - async listFilesForUser(userId: string): Promise<{ key: string; lastModified: Date }[]> { + async listFilesForUser(userId: string): Promise<{ key: string; lastModified: Date; originalName?: string }[]> { const command = new ListObjectsV2Command({ Bucket: this.bucketName, Prefix: `user-${userId}/`, @@ -91,10 +93,38 @@ export class S3Service { response.Contents?.map((item) => ({ key: item.Key!, lastModified: item.LastModified!, + originalName: this.extractOriginalNameFromKey(item.Key!), })) || [] ); } + async isFileDuplicate(file: Express.Multer.File, userId: string): Promise { + const existingFiles = await this.listFilesForUser(userId); + const fileName = file.originalname.toLowerCase(); + + // Prüfen, ob eine Datei mit demselben Namen bereits existiert + for (const existingFile of existingFiles) { + const existingFileName = this.extractOriginalNameFromKey(existingFile.key); + if (existingFileName && existingFileName.toLowerCase() === fileName) { + return true; + } + } + + return false; + } + + private extractOriginalNameFromKey(key: string): string | undefined { + // Extrahiere den Dateinamen aus dem Objektschlüssel + // Format: user-{userId}/{uuid}_{originalname} + const parts = key.split("/"); + if (parts.length >= 2) { + const filename = parts[parts.length - 1]; + const filenameMatch = filename.match(/[^_]+_(.+)$/); + return filenameMatch ? filenameMatch[1] : undefined; + } + return undefined; + } + async deleteFile(objectKey: string): Promise { const command = new DeleteObjectCommand({ Bucket: this.bucketName, diff --git a/tests/helpers/testSetup.ts b/tests/helpers/testSetup.ts index b93d0a0..7895092 100644 --- a/tests/helpers/testSetup.ts +++ b/tests/helpers/testSetup.ts @@ -6,7 +6,7 @@ import { PasswordUtils } from "../../src/utils/passwordUtils"; export const defaultMockPayload = { uuid: "test-user-uuid", username: "testuser", - id: "test-user-id" + id: "test-user-id", }; /** @@ -121,9 +121,9 @@ export const createMockSpotifyApiService = () => ({ getCurrentlyPlaying: vi.fn(), }); -export const createMockSpotifyPollingService = () => ({ +export const createMockSpotifyPollingService = () => ({ startPollingForUser: vi.fn(), - stopPollingForUser: vi.fn() + stopPollingForUser: vi.fn(), }); export const createMockS3Service = () => ({ @@ -132,4 +132,5 @@ export const createMockS3Service = () => ({ listFilesForUser: vi.fn(), deleteFile: vi.fn(), getSignedDownloadUrl: vi.fn(), -}); \ No newline at end of file + isFileDuplicate: vi.fn(), +}); diff --git a/tests/rest/restStorage.test.ts b/tests/rest/restStorage.test.ts index 9468166..3a1ee2a 100644 --- a/tests/rest/restStorage.test.ts +++ b/tests/rest/restStorage.test.ts @@ -35,6 +35,8 @@ describe("RestStorage", () => { describe("POST /upload", () => { it("should upload a file and return a 201 Created response", async () => { + mockS3Service.isFileDuplicate.mockResolvedValue(false); + const objectKey = `user-${requestingUserUUID}/generated-uuid.jpg`; mockS3Service.uploadFile.mockResolvedValue(objectKey); @@ -48,6 +50,14 @@ describe("RestStorage", () => { objectKey: objectKey, }); + expect(mockS3Service.isFileDuplicate).toHaveBeenCalledOnce(); + expect(mockS3Service.isFileDuplicate).toHaveBeenCalledWith( + expect.objectContaining({ + originalname: "test.jpg", + }), + requestingUserUUID + ); + expect(mockS3Service.uploadFile).toHaveBeenCalledOnce(); expect(mockS3Service.uploadFile).toHaveBeenCalledWith( expect.objectContaining({ @@ -63,6 +73,19 @@ describe("RestStorage", () => { expect(response.body.data.message).toBe("No file provided."); expect(mockS3Service.uploadFile).not.toHaveBeenCalled(); }); + + it("should return a 400 Bad Request if the file is a duplicate", async () => { + mockS3Service.isFileDuplicate.mockResolvedValue(true); + + const response = await request(app) + .post("/storage/upload") + .attach("image", Buffer.from("duplicate image data"), "duplicate.jpg") + .expect(400); + + expect(response.body.data.message).toBe("File was already uploaded."); + expect(mockS3Service.isFileDuplicate).toHaveBeenCalledOnce(); + expect(mockS3Service.uploadFile).not.toHaveBeenCalled(); + }); }); describe("GET /files", () => { diff --git a/tests/services/s3Service.test.ts b/tests/services/s3Service.test.ts index 1ddfc30..3dcf845 100644 --- a/tests/services/s3Service.test.ts +++ b/tests/services/s3Service.test.ts @@ -15,7 +15,6 @@ import { S3Client, CreateBucketCommand, PutObjectCommand, - GetObjectCommand, ListObjectsV2Command, DeleteObjectCommand, } from "@aws-sdk/client-s3"; @@ -44,6 +43,13 @@ describe("S3Service", () => { () => ({ send: mockSend, + config: { + region: "mock-region", + credentials: { + accessKeyId: "mock-access-key", + secretAccessKey: "mock-secret-key", + }, + }, }) as never ); @@ -90,7 +96,7 @@ describe("S3Service", () => { const objectKey = await s3Service.uploadFile(mockFile as never, userId); - expect(objectKey).toMatch(/^user-user-123\/[a-f0-9-]+\.jpg$/); + expect(objectKey).toMatch(/^user-user-123\/[a-f0-9-]+_test-image\.jpg$/); expect(mockSend).toHaveBeenCalledOnce(); expect(mockSend).toHaveBeenCalledWith(expect.any(PutObjectCommand)); @@ -109,8 +115,14 @@ describe("S3Service", () => { it("should return a correctly formatted list of files for a user", async () => { const mockS3Response = { Contents: [ - { Key: `user-${userId}/file1.txt`, LastModified: new Date("2023-01-01") }, - { Key: `user-${userId}/image.jpg`, LastModified: new Date("2023-01-02") }, + { + Key: `user-${userId}/uuid1_file1.txt`, + LastModified: new Date("2023-01-01"), + }, + { + Key: `user-${userId}/uuid2_image.jpg`, + LastModified: new Date("2023-01-02"), + }, ], }; mockSend.mockResolvedValue(mockS3Response); @@ -125,10 +137,16 @@ describe("S3Service", () => { expect(sentCommand.Prefix).toBe(`user-${userId}/`); expect(files).toHaveLength(2); - expect(files).toEqual([ - { key: `user-${userId}/file1.txt`, lastModified: new Date("2023-01-01") }, - { key: `user-${userId}/image.jpg`, lastModified: new Date("2023-01-02") }, - ]); + expect(files).toContainEqual({ + key: `user-${userId}/uuid1_file1.txt`, + lastModified: new Date("2023-01-01"), + originalName: "file1.txt", + }); + expect(files).toContainEqual({ + key: `user-${userId}/uuid2_image.jpg`, + lastModified: new Date("2023-01-02"), + originalName: "image.jpg", + }); }); it("should return an empty array if the user has no files", async () => { @@ -178,7 +196,106 @@ describe("S3Service", () => { }); }); - // ignore test for now. + describe("isFileDuplicate", () => { + it("should correctly identify duplicate files", async () => { + const userId = "user-123"; + const mockFile = { + originalname: "duplicate-image.jpg", + buffer: Buffer.from("test-data"), + mimetype: "image/jpeg", + }; + + const mockFiles = [ + { + key: `user-${userId}/file1_original-file.txt`, + lastModified: new Date("2023-01-01"), + originalName: "original-file.txt", + }, + { + key: `user-${userId}/file2_duplicate-image.jpg`, + lastModified: new Date("2023-01-02"), + originalName: "duplicate-image.jpg", + }, + ]; + + mockSend.mockResolvedValueOnce({ + Contents: mockFiles.map((file) => ({ + Key: file.key, + LastModified: file.lastModified, + })), + }); + + const isDuplicate = await s3Service.isFileDuplicate(mockFile as never, userId); + + expect(isDuplicate).toBe(true); + expect(mockSend).toHaveBeenCalledWith(expect.any(ListObjectsV2Command)); + }); + + it("should correctly identify non-duplicate files", async () => { + const userId = "user-123"; + const mockFile = { + originalname: "new-image.jpg", + buffer: Buffer.from("test-data"), + mimetype: "image/jpeg", + }; + + const mockFiles = [ + { + key: `user-${userId}/file1_existing-file.txt`, + lastModified: new Date("2023-01-01"), + originalName: "existing-file.txt", + }, + { + key: `user-${userId}/file2_another-image.jpg`, + lastModified: new Date("2023-01-02"), + originalName: "another-image.jpg", + }, + ]; + + mockSend.mockResolvedValueOnce({ + Contents: mockFiles.map((file) => ({ + Key: file.key, + LastModified: file.lastModified, + })), + }); + + const isDuplicate = await s3Service.isFileDuplicate(mockFile as never, userId); + + expect(isDuplicate).toBe(false); + expect(mockSend).toHaveBeenCalledWith(expect.any(ListObjectsV2Command)); + }); + + it("should handle empty file lists correctly", async () => { + const userId = "user-123"; + const mockFile = { + originalname: "test-image.jpg", + buffer: Buffer.from("test-data"), + mimetype: "image/jpeg", + }; + + mockSend.mockResolvedValueOnce({ + Contents: [], + }); + + const isDuplicate = await s3Service.isFileDuplicate(mockFile as never, userId); + + expect(isDuplicate).toBe(false); + expect(mockSend).toHaveBeenCalledWith(expect.any(ListObjectsV2Command)); + }); + }); + + describe("extractOriginalNameFromKey", () => { + it("should correctly extract the original filename from an object key", () => { + const originalName = (s3Service as any).extractOriginalNameFromKey("user-123/abc123_original-file.jpg"); + expect(originalName).toBe("original-file.jpg"); + }); + + it("should return undefined for invalid object keys", () => { + const originalName = (s3Service as any).extractOriginalNameFromKey("invalid-key"); + expect(originalName).toBeUndefined(); + }); + }); + describe("getSignedDownloadUrl", () => { it("should generate a signed URL for a given object key", async () => { const objectKey = "user-123/image.png"; @@ -190,14 +307,18 @@ describe("S3Service", () => { expect(signedUrl).toBe(fakeSignedUrl); + // Prüfung, dass getSignedUrl korrekt aufgerufen wurde expect(mockGetSignedUrl).toHaveBeenCalledOnce(); - expect(mockGetSignedUrl).toHaveBeenCalledWith(expect.any(Object), expect.any(GetObjectCommand), { - expiresIn: 300, - }); - - const passedCommand = (mockGetSignedUrl.mock.calls[0][1] as GetObjectCommand).input; - expect(passedCommand.Bucket).toBe("test-bucket"); - expect(passedCommand.Key).toBe(objectKey); + expect(mockGetSignedUrl).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ + input: { + Bucket: "test-bucket", + Key: objectKey, + }, + }), + { expiresIn: 300 } + ); }); }); });