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

Add StreamFileParam decorator #47

Closed
wants to merge 4 commits into from

Conversation

tchambard
Copy link
Contributor

In order to be able to pipe the request in express handlers implementations (stream) instead of using multer like typescript-rest is doing with FileParam decorator, the new StreamFileParam decorator will generate the correct swagger button for uploading binary file.

@coveralls
Copy link

coveralls commented Jul 6, 2018

Coverage Status

Coverage increased (+0.8%) to 77.5% when pulling 990aa14 on tchambard:master into fcd2ae9 on thiagobustamante:master.

@ngraef
Copy link
Collaborator

ngraef commented Jul 6, 2018

Could you help me understand how this is different from the @FileParam decorator? There doesn't seem to be anything different about the generated spec.

Also, please add some tests.

@tchambard
Copy link
Contributor Author

Hi Nick, sorry for the delay...

I just added some unit tests for @FileParam and @StreamFileParam parameters.

So, the difference between @FileParam and @StreamFileParam is simple.
@FileParam is interpreted by typescript-rest module as a Multer file element, and the content of the multipart request is consumed by the Http server.

With @StreamFileParam, nothing is done, and you still have the chance to pipe your request to another Http server in order to process streaming...

@jackghicks
Copy link

@tchambard Hi, did you get your StreamFileParam feature merged into the base typescript-rest project? I was about to implement a similar thing when I stumbled on this pr.

@sberthier
Copy link

@ngraef I am really interested by this feature. Streams are different from files as they should be consumed (or piped elsewhere) inside rest handler as said by @tchambard .

Any news ?

@tchambard
Copy link
Contributor Author

@ngraef I close this PR to reopen a new one in order to rebase correctly my branch with master.

It could be really good to merge it as some other people are interrested...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants