Skip to content

Add support for "file" return args#284

Merged
bajtos merged 2 commits intomasterfrom
feature/stream-buffer-returns
Feb 26, 2016
Merged

Add support for "file" return args#284
bajtos merged 2 commits intomasterfrom
feature/stream-buffer-returns

Conversation

@bajtos
Copy link
Member

@bajtos bajtos commented Feb 24, 2016

  1. Fix handling of return args with http.target:header. Arguments mapped to a response http header should not be included in the response body.

  2. Allow users to specify { type: 'file', root: true } for the single argument that will be sent directly as the response body.

The following values are supported for file the file argument:

  • String
  • Buffer
  • ReadableStream (anything that exposes .pipe() method)

Example usage:

MyModel.download = function(cb) {
  // this can be for example http.request()
  getTheStreamBody(function(err, stream) {
    if (err) return cb(err);
    // stream can be any of: string, buffer, ReadableStream (e.g. http.IncomingMessage)
    cb(null, stream, 'application/octet-stream');
  });
}

MyModel.remoteMethod('download', {
  isStatic: true,
  returns: [
    { arg: 'body', type: 'file', root: true },
    { arg: 'Content-Type', type: 'string', http: { target: 'header' } },
  ],
});

Fix strongloop/loopback#2063
Connect to strongloop/loopback#213
Connect to #35

/to @ritch please review
/cc @raymondfeng @STRML

@bajtos bajtos assigned ritch and unassigned ritch Feb 24, 2016
@bajtos bajtos added the #review label Feb 24, 2016
@bajtos bajtos mentioned this pull request Feb 24, 2016
Closed
@bajtos
Fix handling of args with http.target:header
588740b 
"returns" arguments mapped to an http header should not be included
in the response body.
@bajtos bajtos force-pushed the feature/stream-buffer-returns branch from 3953bfd to 7f1cef1 Compare February 25, 2016 08:10
@bajtos
Copy link
Member Author

bajtos commented Feb 25, 2016

I have simplified and reworked this feature. Instead of adding a new flag, I added a new type called "file":

{
  returns: { arg: 'body', type: 'file', root: true }
}

This way the feature is focused on developer's intention (I want to return a raw file response) instead of implementation details (I want to skip json/xml encoding and send the value in raw form). This should make it easier for consumers of remoting metadata (e.g. loopback-swagger) to correctly handle this case.

@ritch
Copy link
Member

ritch commented Feb 25, 2016

We should add a note to these docs: http://apidocs.strongloop.com/strong-remoting/#sharedmethod about the new file type.

Otherwise LGTM 👍

@ritch ritch assigned bajtos and unassigned ritch Feb 25, 2016
@bajtos
Copy link
Member Author

bajtos commented Feb 26, 2016

We should add a note to these docs: http://apidocs.strongloop.com/strong-remoting/#sharedmethod about the new file type.

Added, see 0d4ea21

I'd like to document this feature in more details as part of #35, I'll probably create a new "doc" story for that.

@bajtos
Add support for "file" return args
a85068e 
Allow users to specify { type: 'file', root: true } for the single
argument that will be sent directly as the response body.

The following values are supported for file the file argument:
 - String
 - Buffer
 - ReadableStream (anything that exposes .pipe() method)
@bajtos bajtos force-pushed the feature/stream-buffer-returns branch from 0d4ea21 to a85068e Compare February 26, 2016 09:25
@bajtos bajtos changed the title Stream and Buffer returns Stream and Buffer return arg Feb 26, 2016
@bajtos bajtos changed the title Stream and Buffer return arg Add new return arg type "file" Feb 26, 2016
@bajtos bajtos changed the title Add new return arg type "file" Add support for "file" return args Feb 26, 2016
bajtos added a commit that referenced this pull request Feb 26, 2016
@bajtos
Merge pull request #284 from strongloop/feature/stream-buffer-returns
a6a75fc 
Add support for "file" return args
@bajtos bajtos merged commit a6a75fc into master Feb 26, 2016
@bajtos bajtos deleted the feature/stream-buffer-returns branch February 26, 2016 09:54
@bajtos bajtos removed the #review label Feb 26, 2016
@bajtos bajtos mentioned this pull request Feb 26, 2016
Merged
bajtos added a commit that referenced this pull request Feb 26, 2016
@bajtos
Merge pull request #285 from strongloop/feature/stream-buffer-returns…
7e9aea5 
…-2.x

Add support for "file" return args [2.x]

A back-port of #284
@raymondfeng
Copy link
Member

raymondfeng commented Feb 29, 2016

It's a great feature. What about accepts? If one of the args is file, we should only handle the content-type multipart/form-data. See http://swagger.io/specification/

@bajtos bajtos mentioned this pull request Apr 7, 2016
Closed
3 tasks
@bajtos
Copy link
Member Author

bajtos commented Apr 7, 2016

It's a great feature. What about accepts? If one of the args is file, we should only handle the content-type multipart/form-data. See http://swagger.io/specification/

@raymondfeng sounds good, please open a new issue to keep track of this feature request.

This was referenced Apr 7, 2016
Closed
Closed
@bajtos bajtos mentioned this pull request Aug 17, 2016
Merged
@bajtos bajtos mentioned this pull request Sep 13, 2017
Closed
This was referenced Jan 10, 2019
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@bajtos bajtos

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@bajtos @ritch @raymondfeng @fabien