Merge pull request #178 from gnaaruag/dev-merge

Check empty file uploads, tests for the same and tests for local uploads on file interceptor

packages/common/src/controllers/file-upload.controller.ts

@@ -51,6 +51,13 @@ export class FileUploadController {
     file?: { url: string } | undefined;
   }> {
     try {
+      // file.size is an added attribute that comes from MultipartFile Interface defined in file-upload.interface
+      if (file.buffer.length === 0) {
+        return {
+          statusCode: 400,
+          message: 'empty file uploads are not allowed'
+        }
+      }
       const fileUploadRequestDto = new FileUploadRequestDTO();
       fileUploadRequestDto.file = file;
       fileUploadRequestDto.destination = destination;

packages/common/src/interceptors/file-upload.interceptor.ts

@@ -81,6 +81,8 @@ export function FastifyFileInterceptor(
       next: CallHandler,
     ): Promise<Observable<any>> {
       const ctx = context.switchToHttp();
+      const request = ctx.getRequest();
+      request.raw = request.raw || request;
 
       await new Promise<void>((resolve, reject) =>
         this.multer.single(fieldName)(

packages/common/test/app.e2e-spec.ts

@@ -21,4 +21,97 @@ describe('AppController (e2e)', () => {
       .expect(200)
       .expect('Hello World!');
   });
+
+  it('/files/upload-file (POST); for file with content', async () => {
+
+    const mockDestination = 'uploads';
+    const mockFilename = 'content.txt';
+
+    const response = await request(app.getHttpServer())
+      .post(
+        `/files/upload-file?destination=${mockDestination}&filename=${mockFilename}`,
+      )
+      .attach('file', Buffer.from('content'), mockFilename);
+
+    expect(response.body).toEqual({
+      message: 'File uploaded successfully',
+      file: { url: `${mockDestination}/${mockFilename}` },
+    });
+  });
+
+  it('/files/upload-file (POST); for empty file check', async () => {
+    const mockDestination = 'uploads';
+    const mockFilename = 'empty.txt';
+
+    const response = await request(app.getHttpServer())
+      .post(
+        `/files/upload-file?destination=${mockDestination}&filename=${mockFilename}`,
+      )
+      .attach('file', Buffer.from(''), mockFilename);
+
+
+    expect(response.body).toEqual({
+      statusCode: 400,
+      message: 'empty file uploads are not allowed',
+    });
+  });
+});
+
+describe('Tests for correct setup and functioning of file upload service', () => {
+  let app: INestApplication;
+  const OLD_ENV = process.env;
+
+  beforeEach(async () => {
+    jest.resetModules();
+    process.env = { ...OLD_ENV };
+    const moduleFixture: TestingModule = await Test.createTestingModule({
+      imports: [AppModule],
+    }).compile();
+
+    app = moduleFixture.createNestApplication();
+    await app.init();
+  });
+
+  afterAll(() => {
+    process.env = OLD_ENV;
+  });
+
+  it('Throws error if process.env.STORAGE_ENDPOINT is not set up', async () => {
+    process.env.STORAGE_ENDPOINT = 'this/path/does/not/exist';
+    try {
+      await app.init();
+    } catch (error) {
+      expect(error.name).toBe('InternalServerErrorException');
+      expect(error.message).toBe(
+        'Storage location does not exist. Make sure this location is present and accessible by the application.',
+      );
+    }
+  });
+
+  it('should allow empty destination parameter and store in root of STORAGE_ENDPOINT', async () => {
+    const response = await request(app.getHttpServer())
+      .post(`/files/upload-file`)
+      .query({ filename: 'test.txt' })
+      .query({ destination: '' })
+      .attach('file', Buffer.from('content'), 'test.txt');
+    console.log(response.status);
+    expect(response.status).toBe(201);
+  });
+
+  it('throws error if destination does not exist', async () => {
+    const mockDestination = 'notUploads';
+    const mockFilename = 'content.txt';
+
+    try {
+      await request(app.getHttpServer())
+        .post(
+          `/files/upload-file?destination=${mockDestination}&filename=${mockFilename}`,
+        )
+        .attach('file', Buffer.from('content'), mockFilename);
+    } catch (error) {
+      expect(error.message).toBe(
+        'Given destination path does not exist. Please create one.',
+      );
+    }
+  });
 });

packages/common/test/file-upload/file-upload.service.spec.ts

@@ -1,5 +1,5 @@
 import { FileUploadService } from '../../src/services/file-upload.service';
-import { InternalServerErrorException, Logger } from '@nestjs/common';
+import { BadRequestException, InternalServerErrorException, Logger } from '@nestjs/common';
 import { Client } from 'minio';
 import * as fs from 'fs';
 import * as path from 'path';
@@ -22,6 +22,7 @@ describe('FileUploadService', () => {
   const mockLogger = {
     log: jest.fn(),
     error: jest.fn(),
+    verbose: jest.fn(),
   };
   const mockConfigService = {
     get: jest.fn((key: string) => {
@@ -41,9 +42,9 @@ describe('FileUploadService', () => {
     (Client as jest.Mock).mockImplementation(() => mockMinioClient);
     jest.spyOn(Logger.prototype, 'log').mockImplementation(mockLogger.log);
     jest.spyOn(Logger.prototype, 'error').mockImplementation(mockLogger.error);
+    jest.spyOn(Logger.prototype, 'verbose').mockImplementation(mockLogger.verbose);
 
     service = new FileUploadService(mockConfigService as unknown as ConfigService);
-    // Remove the assignment to 'useSSL'
 
     (path.join as jest.Mock).mockImplementation((...paths) => paths.join('/'));
   });
@@ -65,9 +66,8 @@ describe('FileUploadService', () => {
     const mockFilename = 'test.txt';
 
     beforeEach(() => {
-      (fs.existsSync as jest.Mock).mockReturnValue(false);
-      (fs.mkdirSync as jest.Mock).mockImplementation(() => {});
-      (fs.writeFileSync as jest.Mock).mockImplementation(() => {});
+      (fs.existsSync as jest.Mock).mockReturnValue(true);
+      (fs.writeFileSync as jest.Mock).mockImplementation(() => { });
     });
 
     it('should save a file locally', async () => {
@@ -77,21 +77,18 @@ describe('FileUploadService', () => {
         file: mockFile,
       };
       const result = await service.saveLocalFile(saveToLocaleRequestDto);
-      expect(result).toEqual(mockDestination);
+      expect(result).toEqual(`${mockDestination}/${mockFilename}`);
       expect(fs.existsSync).toHaveBeenCalledWith(
         expect.stringContaining(mockDestination),
       );
-      expect(fs.mkdirSync).toHaveBeenCalledWith(
-        expect.stringContaining(mockDestination),
-        { recursive: true },
-      );
       expect(fs.writeFileSync).toHaveBeenCalledWith(
         expect.stringContaining(`${mockDestination}/${mockFilename}`),
         mockFile.buffer,
       );
     });
 
-    it('should handle directory creation errors', async () => {
+    it('should handle directory errors', async () => {
+      (fs.existsSync as jest.Mock).mockReturnValue(false); // Simulate destination path exists
       (fs.mkdirSync as jest.Mock).mockImplementation(() => {
         throw new Error('Directory creation error');
       });
@@ -101,10 +98,7 @@ describe('FileUploadService', () => {
         file: mockFile,
       };
       await expect(service.saveLocalFile(saveToLocaleRequestDto)).rejects.toThrow(
-        InternalServerErrorException,
-      );
-      expect(mockLogger.error).toHaveBeenCalledWith(
-        'Error creating directory: Directory creation error',
+        new BadRequestException('Given destination path does not exist. Please create one.'),
       );
     });
   });
@@ -159,4 +153,4 @@ describe('FileUploadService', () => {
       );
     });
   });
-});
+});

packages/common/test/interceptor/file-upload.interceptor.spec.ts

@@ -14,6 +14,8 @@ describe('FastifyFileInterceptor', () => {
     expect(interceptor).toBeDefined();
   });
 
+  // file upload tests 
+
   it('should handle file upload', async () => {
     const file = { originalname: 'test.jpg', mimetype: 'image/jpeg' };
     const context = createMockContext(file);
@@ -25,26 +27,69 @@ describe('FastifyFileInterceptor', () => {
     expect(nextHandler.handle).toHaveBeenCalled();
   });
 
-
+  it('should accept files with simple names', async () => {
+    const file = { originalname: 'test.txt', mimetype: 'text/plain' };
+    const context = createMockContext(file);
+    const nextHandler = createMockNextHandler();
 
-  it('should handle errors', async () => {
-    const errorMessage = 'File upload failed';
-    const file = { originalname: 'test.jpg', mimetype: 'image/jpeg' };
+    await interceptor.intercept(context, nextHandler);
+
+    expect(context.switchToHttp().getRequest().file).toEqual(file);
+    expect(nextHandler.handle).toHaveBeenCalled();
+  })
+
+  it('should accept files with multiple periods in them', async () => {
+    const file = { originalname: 'text.tar.gz', mimetype: 'application/gzip' };
     const context = createMockContext(file);
     const nextHandler = createMockNextHandler();
-
-    // Mock the multer middleware to throw an error
-    jest.spyOn(interceptor['multer'], 'array').mockImplementation(() => {
-      return (req, res, callback) => {
-        callback(new Error(errorMessage));
-      };
+
+    await interceptor.intercept(context, nextHandler);
+
+    expect(context.switchToHttp().getRequest().file).toEqual(file);
+    expect(nextHandler.handle).toHaveBeenCalled();
+  })  
+
+  it('should accept files without extensions', async () => {
+    const file = { originalname: 'text', mimetype: 'text/plain' };
+    const context = createMockContext(file);
+    const nextHandler = createMockNextHandler();
+
+    await interceptor.intercept(context, nextHandler);
+
+    expect(context.switchToHttp().getRequest().file).toEqual(file);
+    expect(nextHandler.handle).toHaveBeenCalled();
+  })  
+
+  it('should accept files with spaces in their name', async () => {
+    const file = { originalname: 'data file.txt', mimetype: 'text/plain' };
+    const context = createMockContext(file);
+    const nextHandler = createMockNextHandler();
+
+    await interceptor.intercept(context, nextHandler);
+
+    expect(context.switchToHttp().getRequest().file).toEqual(file);
+    expect(nextHandler.handle).toHaveBeenCalled();
+  })  
+
+  it('should throw Error uploading file on illegal filename', async () => {
+    const file = { originalname: '../foo.bar.cls', mimetype: 'text/plain' };
+    const context = createMockContext(file);
+    const nextHandler = createMockNextHandler();
+
+    jest.spyOn(interceptor, 'intercept').mockImplementation(() => {
+        throw new Error('Illegal filename');
     });
-
-    await expect(interceptor.intercept(context, nextHandler)).rejects.toThrow(errorMessage);
-  });
-
 
+    try {
+        await interceptor.intercept(context, nextHandler);
+    } catch (error) {
+        expect(error).toEqual(new Error('Illegal filename'));
+    }
+
+    expect(nextHandler.handle).not.toHaveBeenCalled();
 
+  })  
+
   it('should handle getting an uploaded file when file is not present or null', async () => {
     const contextWithUndefinedFile = createMockContext(undefined);
     const contextWithNullFile = createMockContext(null);
@@ -59,8 +104,23 @@ describe('FastifyFileInterceptor', () => {
     expect(nextHandler.handle).toHaveBeenCalled();
   });
 
-
+  it('should handle errors', async () => {
+    const errorMessage = 'File upload failed';
+    const file = { originalname: 'test.jpg', mimetype: 'image/jpeg' };
+    const context = createMockContext(file);
+    const nextHandler = createMockNextHandler();
+
+    // Mock the multer middleware to throw an error
+    jest.spyOn(interceptor['multer'], 'array').mockImplementation(() => {
+      return (req, res, callback) => {
+        callback(new Error(errorMessage));
+      };
+    });
+
+    await expect(interceptor.intercept(context, nextHandler)).rejects.toThrow(errorMessage);
+  });
 });
+
 function createMockContext(file: any): ExecutionContext {
   const mockHttpContext = {
     getRequest: jest.fn().mockReturnValue({ raw: { headers: { 'content-type': 'multipart/form-data' } }, file }),