nest icon indicating copy to clipboard operation
nest copied to clipboard

feat(platform): add fastify-multipart support for fastify platform

Open maronfranc opened this issue 3 years ago • 17 comments

add FileInterceptor, FilesInterceptor, AnyFilesInterceptor and FileFieldsInterceptor for platform-fastify

closes #6894

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x] The commit message follows our guidelines: https://github.com/nestjs/nest/blob/master/CONTRIBUTING.md
  • [x] Tests for the changes have been added (for bug fixes / features)
  • [ ] Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Other... Please describe:

What is the current behavior?

Currently file upload interceptor is an important feature that is missing in @nestjs/platform-fastify.

Issue Number: N/A

What is the new behavior?

FileInterceptor(s)

Behave as platform-express.

MultipartWrapper

A fastify-multipart wrapper that is responsible to handle file streams and return theses streams result to interceptor.

  • It expands fastify-multipart options object adding fileFilter:
    • this filter acts in a similar ways as multer fileFilter
  • It is responsable consume fileStream:
    • add size and originalname to file
    • if options.dest is truthy:
      • recursively write folder and then file
      • add path and destination to file object
    • else skip fileStream consumption.
  • It handles fastify-multipart req.files() object differently in each method:
    • files() and any() return array of files
    • fileFields() return object with array of files

filterAsyncGenerator

An utils function that filters AsyncGenerator validating options.fileFilter callback conditions before the write iteration begins.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

I hope this pull request help us find the best solution for platform-fastify file upload interceptor.

maronfranc avatar Apr 20 '21 04:04 maronfranc

Pull Request Test Coverage Report for Build a3454d6e-4d99-46cf-8ab9-60f563025985

  • 239 of 243 (98.35%) changed or added relevant lines in 13 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.2%) to 94.782%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/platform-fastify/multipart/interceptors/any-files.interceptor.ts 13 14 92.86%
packages/platform-fastify/multipart/interceptors/file.interceptor.ts 12 13 92.31%
packages/platform-fastify/multipart/multipart/multipart-wrapper.ts 146 147 99.32%
packages/platform-fastify/multipart/multipart/multipart.utils.ts 11 12 91.67%
<!-- Total: 239 243
Totals Coverage Status
Change from base Build 7b78aa8f-2ae9-4f8e-9a68-151f2b96d8c7: 0.2%
Covered Lines: 5358
Relevant Lines: 5653

💛 - Coveralls

coveralls avatar Apr 20 '21 04:04 coveralls

This would be awesome. Does this support the @UploadedFile() param decorator? Can't find it in the pr.

TheNoim avatar Apr 26 '21 11:04 TheNoim

This would be awesome. Does this support the @UploadedFile() param decorator? Can't find it in the pr.

Yes, it does support @UploadedFile() and @UploadedFiles(). Here you can find a sample code for this feature.

maronfranc avatar Apr 27 '21 04:04 maronfranc

Hi guys, is there anything new? What should we do to merge this feature?

andreferraro avatar Aug 11 '21 17:08 andreferraro

Waiting for this so long. We want to move to fastify, this is a blocker. We would be very happy if this is merged.

m-risto avatar Sep 16 '21 17:09 m-risto

@micalevisk Hi, I see you approved PR. Does this mean that PR will get merged with next release?

kscerbiakas avatar Sep 27 '21 08:09 kscerbiakas

@kscerbiakas

Does this mean that PR will get merged with next release?

Unfortunately, no. I'm not a member of Nestjs's core team. It just means that I liked the proposed solution and tested it myself. It doesn't mean there is no room for improvement, of course.

I do this from time to time with old PRs that I think are good to merge. Just to ping the core team (in some sort) :)

micalevisk avatar Sep 27 '21 14:09 micalevisk

Any status update on this PR?

kscerbiakas avatar Nov 28 '21 12:11 kscerbiakas

You may interested: nest-fastify-multer, it works fine in my project.

yeongjet avatar Dec 17 '21 02:12 yeongjet

any progress on this?

vytautas-pranskunas- avatar Apr 10 '22 09:04 vytautas-pranskunas-

@kamilmysliwiec, any chances to include this PR in Nest v9.0.0 release?

DamianGlowala avatar Jun 19 '22 13:06 DamianGlowala

@kamilmysliwiec Any updates on this issue?

EvilCheetah avatar Sep 20 '22 20:09 EvilCheetah

Love how there are 2 pull requests fixing this issue, however it is still not possible to implement this on the nestjs standards.

PringlePot avatar Oct 13 '22 18:10 PringlePot

Any new updates for this MR? It seems that a lot of people have been asking for this for a long time and it makes sense for better feature parity between the express and fastify platforms. Is there a problem with the proposed solutions that I'm not aware of?

jackgdll avatar Feb 17 '23 20:02 jackgdll

Any updates? @kamilmysliwiec

SzymonGonet avatar Aug 30 '23 19:08 SzymonGonet

What's blocking the PR to be merged @kamilmysliwiec @jmcdo29?

xegulon avatar Nov 06 '23 11:11 xegulon