[OpenAPI] Switch to OpenAPI v3 for spec and routing

[shimks]

Story

As a LoopBack4 user, I would like to use v3 spec because it's more awesome than v2 spec.

Acceptance Criteria

• Move the current decorators into @loopback/openapi-spec
• Update the router to use v3 spec instead of v2
  • do not add support for multiple-servers during spec generation (outside of MVP)
• Leverage the new JSON schema metadata to construct the schema objects for the v3 spec under the definitions section
• Rewrite any of the current (v2-based) examples where applicable to match the new v3 spec
• Document changes on loopback.io

[bajtos]

Move the routing logic into @loopback/openapi-spec

This does not make sense to me. IMO, the REST component is pretty much tied to OpenAPI. What will remain inside @loopback/rest when we remove router and decorators? Wouldn't it become just a thin/empty wrapper?

Refactor RestServer to accept a component that provides a router

What are the benefits of this? Are they worth the additional complexity? What if we told people that want to use a different spec format to write their own REST-like component, instead of telling them to configure our empty REST server with a custom router?

[kjdelisle]

@bajtos The RestServer would be the harness for handling http/s/2 configuration, as well as spinning up whatever's given by the providers for things like Sequence.

I was mostly hoping to avoid the need to reinvent the decorators for every single custom implementation of REST (i.e. ExpressServer, KoaServer, WhateverServer)

[bajtos]

@kjdelisle I am afraid this still does not make much sense to me.

For example, it's known that Express's router gets slower with the number of routes registered. So even if we wanted to support Express, then IMO we should still build our own router implementation that can be mounted on an express app. See strongloop/strong-remoting#282 and our internal https://github.com/strongloop-internal/scrum-loopback/issues/712 for more details.

I am concerned that in order to support different http servers (express, koa, etc.), we will need to implement (and maintain!) many different flavors of routers, which I don't think we have bandwidth to do.

IMO, if we decide to go down the route of supporting 3rd party http server frameworks, then we should pick only one. I personally prefer Koa, because it has been designed for async/await and has decent performance (see e.g. Fastify benchmarks).

Anyhow, it has been some time since I was working on this part of loopback-next, therefore it's entirely possible that I am missing important constraints that make my arguments moot. I am not against moving some parts of the current REST implementation elsewhere, I just want to avoid situation where we move too much stuff to openapi-related packages.

I think the most important concern to keep in mind is versioning and breaking changes. In my experience, breaking changes will occur more frequently in the routing part, while the typescript typedefs for OpenAPI Spec interfaces will remain mostly constant. I think decorators will be somewhere in the middle. From this point of view, the routing logic should live in its own package.

It makes sense to me to implement an openapispec router (our findRoute sequence action) as a standalone package (e.g. similar to find-my-way router), as such package can be used outside of LoopBack too and thus we may gain wider community around that router than just LoopBack users.

I am not sure what's the best place where to put decorators - at one hand, they are quite coupled to the router implementation, but at the other hand, we want components contributing controllers to be able to depend on those decorators only. So maybe another package is needed? IDK.

[jannyHou]

@kjdelisle @bajtos in my original proposal I suggested to extract the decorators to @loopback/openapi-spec only, it should be similar to loopback-swagger, containing "specgen" and "codegen"(when we support the top-down artifacts generating).
I think I am ok with either: merging decorators into openapi-spec or creating a new module for them, I have a PR #800 for it (extract decorators from @loopback/rest to @loopback/openapi-spec), waiting for Raymond's decorator refactor PR checking in first. If we decide to separate openapi types from the "specgen" and "codegen" I will move them into a new package, it wouldn't take too much effort.

For the routing logic...I don't have strong opinion at this moment, how about I finish the decorator extraction and v3 upgrading first, I believe these two are already a big change :)

[jannyHou]

As a summary, now we have several plans:

• Plan 1: 3 packages
  • openapi-spec: openapi types
  • loopback4-openapi: openapi specgen & codegen
  • openapi-router: openapi routings
• Plan 2: 2 packages
  • openapi-spec: openapi types
  • loopback4-openapi: openapi specgen & codegen, openapi routings
• Plan3: 1 package
  • openapi-spec: openapi types, openapi specgen & codegen, openapi routings

There might be better name for each package, the plans above is about how we organize different functionalities into one or more packages.

Considering the "versioning and breaking changes" I would probably choose the first one. Appreciate more opinions on it :)

[bajtos]

I like "Plan 1" most 👍

• openapi-spec can be possibly replaced with a 3rd-party package like openapi3-ts
• @loopback/router or @loopback/openapi-router (I'd call it "router", not "routes") should provide implementation of findRoute sequence actions (not necessarily with the same interface)
• @loopback4/openapi can contain utility functions like specgen.

I am not sure where to put decorators like @get, @param etc., whether we want them in "router" or "openapi" spec, or maybe in a fourth package.

As for codegen, I'd prefer to put that in a standalone package, because it's almost completely unrelated to specgen code.

Anyways, I think these are details that we will figure out once we start splitting @loopback/rest into smaller packages and build a better understanding of coupling and dependencies.

[jannyHou]

@bajtos

I am not sure where to put decorators like @get, @param etc., whether we want them in "router" or "openapi" spec, or maybe in a fourth package.

I would like to extract those path decorators into a specgen package, later on we can add model decorators(model --> openapi3 schema, cc @kjdelisle @shimks ) in that package too

openapi-spec can be possibly replaced with a 3rd-party package like openapi3-ts

openapi-spec contains some sugar types like

 */
export interface ParametersDefinitionsObject
extends MapObject<OAS3.ParameterObject> {
/**
 * A single parameter definition, mapping a "name" to the parameter it
 * defines.
 */
[name: string]: OAS3.ParameterObject;
}

So I think there is still a value to keep the openapi-spec package.

[jannyHou]

Status of this issue:

• Move the routing logic into @loopback/openapi-router

  • I didn't cover this part in my spike and also missed the discussion why we want to move it, if anyone familiar with the rest router wants to create a new package for it, appreciate your help :)

• Move the path decorators into @loopback/openapi-v2
  Done

• I have another commit for upgrading from v2-->v3: 846e791

  • PR for it: [DO NOT REVIEW, WIP]Upgrade/v2 v3 #845
  • work in progress: swagger2's single server --> openapi3's component.servers
  • to be done: v3 validator

  Copied from my chat with @kjdelisle :

  Given a look of the validator impt in oai-ts-core and the one in swagger-parser, I don't find any standard validation rules they follow...the rules seem like arbitrary, e.g. validate uniqueness, if we only care about the spec type, and we have some important validation rules for our own use, we can build our own impt instead of using those modules(needs more time though...) or if the official online openapi validator is opensource we can make a typescript version of it.

---

Updated the content...a package name with @ displays on the edit page, but disappears in comment....wrap them with ``

[jannyHou]

I would like to see more opinions the content-type:
In openapi3, requestBody is separated from parameters, example see:

"/pets/{petId}":
  post:
    requestBody:
      description: user to add to the system
      required: true
      content:
        application/json: 
          schema:
            type: array
            items:
              $ref: '#/components/schemas/Pet'
          examples:
            - name: Fluffy
              petType: Cat
            - http://example.com/pet.json
    parameters:
      - name: petId
        in: path
        description: ID of pet to update
        required: true
        type: string

And you can specify multiple contents in one requestBody object.

For different content types, do they usually share the same schema or not?
This determines for one controller function, do we apply @requestBody on multiple parameters.

@strongloop/loopback-next Any thought? Thanks.

Never mind, not a concern anymore. People can provide schema to override our generated one if they need. While this is a behaviour worth documented.

[raymondfeng]

See #940 (comment)

[bajtos]

@jannyHou

you can specify multiple contents in one requestBody object.

In our MVP release, we should be focusing on API servers using JSON payloads in both request and responses. Is there a way how to defer the design work needed to support non-JSON request bodies and/or multiple accepted content types after MVP?

One of your code example shown above looks perfect to me:

async function create(
  @requestBody(requestBodySpec: RequestBodySpec) order: Order
):void {}

Under the hood, requestBody can fill in any missing OpenAPI Spec metadata with values relevant to JSON requests.

In the future, we can modify requestBody to support multiple content type. I can imagine different solutions, see below - but again, we should leave this out of scope of our MVP work.

// assuming the schema is always the same
async function create(
  @requestBody(requestBodySpec, {
    required: true,
    acceptedTypes: ['application/json', 'application/xml']
  })
  data: Order
);

// assuming different schemas
async function create(
  @requestBody({
    'application/json': { /* spec */ },
    'application/xml': { /* spec */ },
  )
  data: JsonOrder | XmlOrder
);

[jannyHou]

Is there a way how to defer the design work needed to support non-JSON request bodies and/or multiple accepted content types after MVP?

@bajtos I make application/json as our default media-type and I believe the check in parse.ts will catch the content-type we don't support

Raymond suggests to try blog-driven development in the retro meeting, and I am writing a blog draft that contains the UX of new decorator @requestBody, will cover the topic "content-type" there.

[jannyHou]

The new request format in OpenAPI 3.0.0 raises a big http handler(parser) problem: parameters in body(formData) are now separated from those ones in query | path | header | cookie, which results in an openapi operation spec CANNOT serve as a complete routing reference any more, because it loses the argument position info.

For example:

class MyController {
    function hi(foo: string, bar: string, baz: number) {
      // say hi
    }
}

// The corresponding swagger spec
swaggerOperationSpec: {
  parameters: [
    {foo: {in: 'formData', ...fooSpec}},
    {bar: {in: 'query', ...barSpec}},
    {baz: {in: 'formData', ...bazSpec}},
  ]
}

// The corresponding openapi spec
openapiOperationSpec: {
  requestBody: {
    type: 'object',
    properties: {
      foo: {type: 'string', ...fooSpec},
      baz: {type: 'number', ...bazSpec}
    }
  },
  parameters: [
    {bar: {in: 'query', ...barSpec}}
  ]
}

TL:DR The openapi 3 operation spec is not consistent with a function's arguments any more.
An article contains a valid upgrade scenario for it: search "Request Format" in https://blog.readme.io/an-example-filled-guide-to-swagger-3-2/#request-format

I had a discussion with @virkt25 (thank you for the idea), we take an approach to preserve the argument position's info by binding parameter and requestBody metadata to another common key in the controller decorators: @param and requestBody.

requestBody(requestBody: RequestBodySpec) {
  ParameterDecoratorFactory.createDecorator<RequestBodyObject>(
      // used for retrieving requestBody spec, 
      // separate from parameter spec
      OAS3_REQUEST_BODY_KEY, 
      requestBodySpec as RequestBodyObject,
    )(target, member, index);

  ParameterDecoratorFactory.createDecorator<RequestBodyArgument>(
      // used for retrieving argument spec
      // shared with parameter metadata
      OPERATION_ARGUMENTS_KEY, 
      {in: 'body', spec: requestBodySpec as RequestBodyObject},
    )(target, member, index);
}

param(requestBody: RequestBodySpec) {
  ParameterDecoratorFactory.createDecorator<PamameterObject>(
      // used for retrieving parameter spec, 
      // separate from requestBody spec
      OAS3_PARAMETERS_KEY,
      parameterSpec,
    )(target, member, index);

  ParameterDecoratorFactory.createDecorator<PamameterObject>(
      // used for retrieving argument spec
      // shared with requestBody metadata
      OPERATION_ARGUMENTS_KEY,
      parameterSpec
    )(target, member, index);
}

The parser provider function consumes the argument spec(order guaranteed) to build arguments before invoking the method.

This solves the problem for bottom-up approach, but not for the top-down ones:

server.api()
server.route()
@api()
class MyController {

}

They still need to provide two api/route spec:

• one for openapi operation spec, all the arguments in body/formdata are merged into requestBody, and you cannot infer the function's argument order from it.
• one for routing, which preserves the argument spec in order.

option 1

A solution would be build a helper function that generates parameters and requestBody spec from the argument spec. Use argument spec for http_handler's parser function. Then the next question is: how do we describe the argument spec? Whatever spec shape we take, it to some extend falls back to a [email protected] like spec.

option 2

Add a new property in the route constructor for building arguments, it can be an array of names(string) that simply maps to the element name (parameter, or requestBody, or a property of requestBody) in an openapi operation spec.

// something like:
['body.foo', 'bar', 'body.baz']

To retrieve the metadata bound to the common key OPERATION_ARGUMENTS_KEY, a similar property is already added.

The only additional work for a user is providing this property when define routes in the top-down styles above.

Personally I like this one.

option 3

Another solution could be only allow one requestBody argument in a function, and "hardcode" the position for it, for example:

• If it's optional, define it as the last argument
• If it's required, define it as the first argument
  This is not friendly for users migrate from [email protected], they have to parse the whole body from http request instead of parsing only the formData they want. Especially when the whole body contains heavy data. But I feel this is what the new openapi operation spec encourage and imply people to do.

@strongloop/loopback-next Any thought? This could be a high level design.

[virkt25]

Great summary @jannyHou! I've been thinking about this some more and I think we can do without the double decorators ... which would avoid the impact on memory performance. Here's what I'm thinking ...

@requestBody() and @param decorators for OAS 3 both store the metadata into the same key to maintain order (OPERATION_ARGUMENTS_KEY) from your example. We don't need to store the data in separate keys for OAS since we have no such need since the purpose of that metadata is to save information and allow us to consume it later and there's 2 ways this information is consumed AFAIK.

1. parser => Needs the args in order so they can be spread into the controller method accordingly.
2. /oas.json => To serve the actual OAS Spec for the app so others know what to expect ... this is currently generated dynamically and cached (I think) ... but we can just write a new function here to go over the metadata from OPERATION_ARGUMENTS_KEY and transform it into real OAS because we won't store the metadata as OAS compliant (since requestBody / param will store to same key) ... but with this approach a user won't know the difference since the app has to be compliant with the behaviour and be able to serve the spec file ... the intermediate representation in metadata is completely up to us.

[bajtos]

@jannyHou @virkt25 great job! Your proposed approach looks reasonable to me.

Regarding the position of body argument in controller method signature in top-down approach: I personally prefer your third option, where the position of the body argument is hardcoded. I'd rather avoid changing the position depending on whether the argument is required or optional, because a small change in spec (required: true -> false) can break the implementation. I expect that the body argument will be more often required than optional, in which case I think we should always put it at the first position.

Alternatively, we can introduce an extension field allowing users to specify the position of the body argument. For example:

openapiOperationSpec: {
  requestBody: {
    type: 'object',
    properties: {/*...*/},
    'x-parameter-index': -1 // 0 means first, 1 means second, -1 means last
  },
  parameters: [
    {bar: {in: 'query', ...barSpec}}
  ]
}

When this extension field is not provided, we can default to our hard-coded convention (first or last, depending how we decide.)

Add a new property in the route constructor for building arguments, it can be an array of names(string) that simply maps to the element name (parameter, or requestBody, or a property of requestBody) in an openapi operation spec.

// something like:
['body.foo', 'bar', 'body.baz']

I actually like this proposal too, although I think this configuration should stay inside OpenAPISpec for consistency with the x-controller and x-method-name approach we are already using. So in addition to these existing extension fields, I can imagine adding another extension field x-method-arguments that can contain the array shown above.

This is not friendly for users migrate from [email protected], they have to parse the whole body from http request instead of parsing only the formData they want. Especially when the whole body contains heavy data. But I feel this is what the new openapi operation spec encourage and infer people to do.

I think we should follow what OpenAPI Spec encourages, at least for the MVP scope. Let's discuss whether we want to allow formData-like access to individual properties from body payload in a new (non-MVP) GitHub issue.

[jannyHou]

Thank you all for replying so quickly and such valuable feedbacks!

@virkt25

We don't need to store the data in separate keys for OAS since we have no such need since the purpose of that metadata is to save

For option 1, exactly 👍 and now I kind of understand why Kyu said use a shared key in the first place.

While if we take option 2, I think 3 keys are still needed.

My concern for option 1, 2 is, if we only allow one whole requestBody in the args, then it's pretty much the same as option 3 with Miroslav's suggestion:

although I think this configuration should stay inside OpenAPISpec for consistency with the x-controller and x-method-name approach we are already using. So in addition to these existing extension fields, I can imagine adding another extension field x-method-arguments that can contain the array shown above.

If we allow multiple formData, where to provide the whole requestBody spec and the potential conflict resolve among requestBody spec, requestBody.type and its properties would be annoying.

The critical limitation for option 3 is dropping the formData(or say partial requestBody) support, and there is no easy way to get it back without a big design change. But it requires the least code change for v3 upgrade, and has the best compatibility of our rest component, considering how we implement the routing and parser.

Now I tend to agree with @bajtos 's suggestion: taking option 3 but using an extension property to specify the position:

openapiOperationSpec: {
  requestBody: {
    type: 'object',
   properties: {/*...*/},
   'x-parameter-index': -1 // 0 means first, 1 means second, -1 means last
 },
 parameters: [
   {bar: {in: 'query', ...barSpec}}
 ]
}

Actually we also discussed the extension property yesterday, a concern is we may not want to inject too many loopback specific properties into the openapi file, but x-parameter-index doesn't quite give me that feeling, treating it like a general extension that describe an arg's position makes sense.

BTW, when making choices, there are several things worth taking into account:

• validation: type coerce and schema validation
• extensibility: formData
• data serialization
• DI refactor

[raymondfeng]

+1 to use x-parameter-index for MVP to customize the position of the requestBody.

For longer term, I would like to propose that we turn @param decorators into a variant of @inject so that these parameters will be resolved behind the scene from the rest.request by method injection. The position of each parameter does not matter any more. There is no need to pre-build the args[] in the rest invoker. And it's also ideal to allow on-demand parsing/deserialization of http request into corresponding parameters.

[jannyHou]

Prototype PR: #916

The PRs for this story are split into 4, please see comment #916 (comment) for details.

The 1st and 2nd PRs are merged. I am working on the 3rd one.

[kjdelisle]

This has slipped again. Moving to March 2018

[arthurdm]

Hi everyone. What URL will Loopback use to expose the OpenAPI v3 document? It would be great if it was consistent with the Java MicroProfile OpenAPI model, which exposes the YAML OAS3 document at /openapi.

[shimks]

@arash01 Currently, the OpenAPI spec is hosted at the endpoint /openapi.json and /openpai.yaml, each in their respective formats.

[virkt25]

Closing as completed.