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

Spike: Create Route in JSLB4 #2474

Open
hacksparrow opened this Issue Feb 26, 2019 · 3 comments

Comments

Projects
None yet
4 participants
@hacksparrow
Copy link
Member

commented Feb 26, 2019

Follow up task of #1978.

Acceptance criteria

  • Should be created as a class

  • Should be useable with JSLB4 Application class

  • Should have access to:

    • The LB4 request object and metadata contributed by LB4 components eg: @loopback/authentication.
    • A Model Repository
    • The LB4 response object

    UPDATE 2019-03-12 @raymondfeng @hacksparrow and @bajtos discussed this requirement and agreed it's good enough to pass requestContext as the first argument to the handler function. We need to preserve backwards compatibility for handlers that are not expecting the context in their parameters.

@bajtos

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

(1)

Should be created as a class

This part does not make sense to me. Routes are implemented as functions, see e.g. here:

https://github.com/strongloop/loopback4-example-javascript/blob/c9c654934644da705b81eb3450f6450ee37df5b7/server/application.js#L15

    this.route('get', '/hi', {responses: {}}, () => 'Hi!');

(2)
In addition to the list of objects the route should have access to, the route must be also able to receive parameters parsed by LB4 from the request. The parameters are described in the provided OpenAPI spec (this.route argument number 3), either via parameters or requestBody.

(3)
Having access to a Model is not very useful, it needs to access a Repository instance that can be used query model's data and make any changes.

(4)
To describe route's inputs and outputs, we need a helper function to build JSON schema from LB4 model class. I believe that in TypeScript, this conversion is done automatically by @param and @response decorators. In JavaScript, we need to be more explicit.

// a mock-up I am typing from my head, may be wrong in spec details
this.route('post', '/products', {
  requestBody: {
    content: {
      'application/json': {
        schema: buildOpenApiSchemaForModel(Product)
      },
    },
  },
  responses: {
    '200': {
      description: 'Product created',
      content: 
        'application/json': {
          schema: buildOpenApiSchemaForModel(Product)
        },
    },
  },
}, (data) => productRepository.create(data));

I think we will need to address this part for controllers too (#2478) depending on which approach we choose.

(5)
As you can see, building the OperationObject requires a lot of boilerplate code. It would be great to create a follow-up spike to look for ways how to simplify this part. For example:

// a mock-up I am typing from my head, may be wrong in spec details
this.route('post', '/products', {
  requestBody: buildRequestBodySpecForModel(Product),
  responses: {
    '200': buildResponseSpecForModel(Product, 'ProductyCreated')
  },
}, (data) => productRepository.create(data));
@bajtos

This comment has been minimized.

Copy link
Member

commented Feb 26, 2019

See also #1978 (comment) for few ideas on how to make routes easy to implement in JavaScript.

@emonddr

This comment has been minimized.

Copy link
Contributor

commented Apr 4, 2019

@hacksparrow , we would like you to update the Acceptance Criteria based on the thread discussion. Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.