Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/10017 parse file pipe builder #10025

Merged
Merged
6 changes: 6 additions & 0 deletions packages/common/pipes/file/parse-file-options.interface.ts
Expand Up @@ -5,4 +5,10 @@ export interface ParseFileOptions {
validators?: FileValidator[];
errorHttpStatusCode?: ErrorHttpStatusCode;
exceptionFactory?: (error: string) => any;

/**
* Defines if file parameter is required.
* @default true
*/
fileIsRequired?: boolean;
}
16 changes: 14 additions & 2 deletions packages/common/pipes/file/parse-file.pipe.ts
@@ -1,9 +1,10 @@
import { isUndefined } from '../../utils/shared.utils';
import { Injectable, Optional } from '../../decorators/core';
import { HttpStatus } from '../../enums';
import { HttpErrorByCode } from '../../utils/http-error-by-code.util';
import { PipeTransform } from '../../interfaces/features/pipe-transform.interface';
import { ParseFileOptions } from './parse-file-options.interface';
import { HttpErrorByCode } from '../../utils/http-error-by-code.util';
import { FileValidator } from './file-validator.interface';
import { ParseFileOptions } from './parse-file-options.interface';

/**
* Defines the built-in ParseFile Pipe. This pipe can be used to validate incoming files
Expand All @@ -19,22 +20,33 @@ import { FileValidator } from './file-validator.interface';
export class ParseFilePipe implements PipeTransform<any> {
protected exceptionFactory: (error: string) => any;
private readonly validators: FileValidator[];
private readonly fileIsRequired: boolean;

constructor(@Optional() options: ParseFileOptions = {}) {
const {
exceptionFactory,
errorHttpStatusCode = HttpStatus.BAD_REQUEST,
validators = [],
fileIsRequired,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed to required semantic, but I maintained the full word fileIsRequired to be explicit here. In the consumer side, when creating a ParseFilePipe, I think it's worth it to specify what is required:

const parseFilePipe = new ParseFilePipe({
  validators: [],
  fileIsRequired: false,
});

} = options;

this.exceptionFactory =
exceptionFactory ||
(error => new HttpErrorByCode[errorHttpStatusCode](error));

this.validators = validators;
this.fileIsRequired = fileIsRequired ?? true;
}

async transform(value: any): Promise<any> {
if (isUndefined(value)) {
if (this.fileIsRequired) {
throw this.exceptionFactory('File is required');
}

return value;
}

if (this.validators.length) {
await this.validate(value);
}
Expand Down
60 changes: 60 additions & 0 deletions packages/common/test/pipes/file/parse-file.pipe.spec.ts
Expand Up @@ -116,5 +116,65 @@ describe('ParseFilePipe', () => {
});
});
});

describe('when fileIsRequired is false', () => {
beforeEach(() => {
parseFilePipe = new ParseFilePipe({
validators: [],
fileIsRequired: false,
});
});

it('should pass validation if no file is provided', async () => {
const requestFile = undefined;

await expect(parseFilePipe.transform(requestFile)).to.eventually.eql(
requestFile,
);
});
});

describe('when fileIsRequired is true', () => {
beforeEach(() => {
parseFilePipe = new ParseFilePipe({
validators: [],
fileIsRequired: true,
});
});

it('should throw an error if no file is provided', async () => {
const requestFile = undefined;

await expect(parseFilePipe.transform(requestFile)).to.be.rejectedWith(
BadRequestException,
);
});

it('should pass validation if a file is provided', async () => {
const requestFile = {
path: 'some-path',
};

await expect(parseFilePipe.transform(requestFile)).to.eventually.eql(
requestFile,
);
});
});

describe('when fileIsRequired is not explicitly provided', () => {
beforeEach(() => {
parseFilePipe = new ParseFilePipe({
validators: [new AlwaysInvalidValidator({})],
});
});

it('should throw an error if no file is provided', async () => {
const requestFile = undefined;

await expect(parseFilePipe.transform(requestFile)).to.be.rejectedWith(
BadRequestException,
);
});
});
});
});
12 changes: 12 additions & 0 deletions sample/29-file-upload/e2e/app/app.e2e-spec.ts
Expand Up @@ -51,6 +51,18 @@ describe('E2E FileTest', () => {
.expect(400);
});

it('should throw when file is required but no file is uploaded', async () => {
return request(app.getHttpServer())
.post('/file/fail-validation')
.expect(400);
});

it('should allow for optional file uploads with validation enabled (fixes #10017)', () => {
return request(app.getHttpServer())
.post('/file/pass-validation')
.expect(201);
});

afterAll(async () => {
await app.close();
});
Expand Down
8 changes: 5 additions & 3 deletions sample/29-file-upload/src/app.controller.ts
Expand Up @@ -42,13 +42,15 @@ export class AppController {
.addFileTypeValidator({
fileType: 'json',
})
.build(),
.build({
fileIsRequired: false,
}),
)
file: Express.Multer.File,
file?: Express.Multer.File,
) {
return {
body,
file: file.buffer.toString(),
file: file?.buffer.toString(),
};
}

Expand Down