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

Configurable response types/formats #436

Closed
bajtos opened this issue Jul 13, 2017 · 19 comments
Closed

Configurable response types/formats #436

bajtos opened this issue Jul 13, 2017 · 19 comments

Comments

@bajtos
Copy link
Member

bajtos commented Jul 13, 2017

At the moment, the return value of a controller method/router handler is converted to the response using the following hard-coded algorithm:

  • Object types (including arrays, but also Date, RegExp, Buffer) are converted to JSON and sent with "application/json" content-type.
  • String results are sent as-is with "text/plain" content-type.
  • Anything else is passed to response.write() as-is and no content-type is set.

We need a more flexible setup, we should at least honor "produces" configuration from the API specification.

Acceptance Criteria

  1. A controller method should be able to specify media type for the responses object with @operation. See https://github.com/OAI/OpenAPI-Specification/blob/master/versions/3.0.1.md#response-object

    Note by @bajtos: I believe this is already supported, see e.g. the TodoController in our examples: https://github.com/strongloop/loopback-next/blob/cb308add60bbe938c8e85812676eb817e9e0efc9/examples/todo/src/controllers/todo.controller.ts#L32

  2. A controller method should be able to return JS representation of the response body or a wrapper corresponding to the response objects. The goal: allow controller methods to control response status code and headers. E.g. Location for 201 Created responses to POST requests creating new model instances, Content-Type and Content-Disposition for file downloads.

  3. LoopBack should be able to match the response media types to the Accept request header to determine what content type to be produced for a given request. It will set Content-Type response header.

  4. Have built in support for application/json and text/plain media-types.

EDIT: Move to another task

  1. LoopBack runtime should define an extension point to register http response serializers that can be used to serialize the JS plain/wrapper object into a http response. A serializer should be able to handle certain media types, for example, json, xml, or protocol buffer. The serializer just has to deal with the http response object as a canonical representation.
@raymondfeng
Copy link
Contributor

We probably should start to refactor https://github.com/strongloop/loopback-next/tree/master/packages/repository/src/types into its own package to form the LB.next typing system and leverage it for parameter parsing/conversion/serialization.

@bajtos
Copy link
Member Author

bajtos commented Nov 2, 2017

Labelling as non-MVP, because we have a simple workaround available - users can use their own implementation of send sequence action.

@dustinbarnes
Copy link

@bajtos can you give an example of that? I'm currently fighting the content-type header (it really needs to send application/json when we return a single string), and using the sugar for @get(path, spec), but it seems to ignore whatever is put into the "produces" section of the spec object.

@bajtos
Copy link
Member Author

bajtos commented Dec 5, 2017

@dustinbarnes Take a look at the default implementation of send() here and here.

Here is what you need to do:

  • Create your own provider returning a custom "send" action - you can start by copying our send provider and writeResultToResponse function.
  • Register your custom "send" action - I am not entirely sure how to do that now that REST server have been decoupled from the Application object. @kjdelisle could you please advise and ideally update our docs to show how to do this?

@kjdelisle
Copy link
Contributor

kjdelisle commented Dec 5, 2017

@bajtos @dustinbarnes
Registering your custom send action involves injecting on the RestBindings.SequenceActions.SEND key with a custom Provider for Send.

application.ts

export class YourApp extends RepositoryMixin(Application) {
  constructor() {
    super();
    // Assume your controller setup and other items are in here as well.
    this.bind(RestBindings.SequenceActions.SEND).toProvider(CustomSendProvider);
  }

custom-send-provider.ts

import { Send } from "@loopback/rest";
import { Provider, BoundValue } from "@loopback/context";
import { writeResultToResponse } from "@loopback/rest";

export class CustomSendProvider implements Provider<BoundValue>{
  value(): Send | Promise<Send> {
    return writeResultToResponse; // Replace this with your own implementation.
  }
}

If you're implementing multiple servers in your application, binding this key at the Application-level context will make that binding the default for all of the servers connected to it. This means that if you wanted different custom providers for different RestServer instances, you'd want to perform those bindings elsewhere (like in an async override of the start method).

In this example, PrivateSendProvider might allow stack traces to be returned via the response, whereas PublicSendProvider will not.

in application.ts

async start() {
   const publicServer = await this.getServer('public');
   const privateServer = await this.getServer('private');
   publicServer.bind(RestBindings.SequenceActions.SEND).toProvider(PublicSendProvider);
   privateServer.bind(RestBindings.SequenceActions.SEND).toProvider(PrivateSendProvider);
   await super.start(); // Don't forget to call start!
}

@bajtos
Copy link
Member Author

bajtos commented Jan 15, 2018

I created a follow-up issue to capture the instructions from Kevin in our docs - see #863.

@dhmlau
Copy link
Member

dhmlau commented Mar 22, 2018

Does it fall under the "Validation and type conversion" epic #755 ?

@bajtos
Copy link
Member Author

bajtos commented Mar 23, 2018

I don't think so. As I understand #755, it deals with input parameters. This story deals with outputs.

I think the best epic to assign this story to is an epic for implementing a local in-process API Explorer and/or a capability to server static files.

@shimks
Copy link
Contributor

shimks commented Apr 30, 2018

@bajtos Could we get an acceptance criteria for this issue? Thanks

@shimks
Copy link
Contributor

shimks commented May 9, 2018

@bajtos ^^^

@acrodrig
Copy link

Hello. I am interested in having a JSONP response. I seem to believe this was available in LB3 out of the box. @bajtos when you refer to:

A controller method should be able to return JS representation of the response body or a wrapper corresponding to the response objects.

I assume you mean JSONP? If not, what would be the best way to achieve JSONP.

There seems to be no way of differentiating the response based on the "Accept" header being sent.

@bajtos bajtos added the p1 label Apr 9, 2019
@bajtos
Copy link
Member Author

bajtos commented Apr 15, 2019

@acrodrig

A controller method should be able to return JS representation of the response body or a wrapper corresponding to the response objects.

I assume you mean JSONP?

Not really. We want to allow the controller method to handle more aspects of the HTTP response that will be produced, e.g. status code, headers, etc.

See #436 (comment)

export class CustomerController {

  @get('/:id', {responses: {...}})
  async findById(id: string): RestResponse {
    return {
      statusCode: 200,
      headers: {...};
      content: {
        'application/json': customer,
      },
      links: {...};
    }
  }
}

If not, what would be the best way to achieve JSONP.

Our current recommendation is to write a custom implementation of sequence action send, see https://loopback.io/doc/en/lb4/Sequence.html#customizing-sequence-actions.

I seem to believe this was available in LB3 out of the box.

Yes, indeed - see https://github.com/strongloop/strong-remoting/blob/33fbd72fb46035f707c3c62dce6b36ab075fb61e/lib/http-context.js#L480-L483

I created a new issue to keep track of this feature - see #2752

@mdbetancourt
Copy link
Contributor

mdbetancourt commented Feb 27, 2020

What about this, I think it can work

export class CustomerController {

  @get('/:id', {responses: {...}})
  async findById(id: string, @inject(Http.RESPONSE) response): Response {
    return Object.assign(response, {
      statusCode: 200,
      headers: {...};
      content: {
        'application/json': customer,
      }
      links: {...};
    })
  }
}

because writeResultToResponse bypass response if controller return the response

@stale
Copy link

stale bot commented Jul 14, 2021

This issue has been marked stale because it has not seen activity within six months. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository. This issue will be closed within 30 days of being stale.

@stale stale bot added the stale label Jul 14, 2021
@stale
Copy link

stale bot commented Aug 13, 2021

This issue has been closed due to continued inactivity. Thank you for your understanding. If you believe this to be in error, please contact one of the code owners, listed in the CODEOWNERS file at the top-level of this repository.

@stale stale bot closed this as completed Aug 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests