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

feat: adds @oas.response decorator #4550

Merged
merged 1 commit into from
Feb 10, 2020

Conversation

mschnee
Copy link
Contributor

@mschnee mschnee commented Jan 31, 2020

Partially implements: #4300
See: #4406

Adds

This PR adds the @oas.response(code: number, ...modelOrSpec: ModelOrSpec[]) decorator.

Note

The decorator only affects the OpenAPI specification object that is created, and anything that uses it to verify and validate responses. This decorator has no impact on the actual response code. It also doesn't add any typings or lint configuration that could force response compliance at compile/lint time (if that's even possible).

Example

@model()
class SuccessModel extends Model {
  constructor(err: Partial<SuccessModel>) {
    super(err);
  }
  @property({default: 'Hi there!'})
  message: string;
}

class FooNotFound extends Model {
  @property()
  message: string;
}

class BarNotFound extends Model {
  @property()
  message: string;
}

class BazNotFound extends Model {
  @property()
  message: string;
}

class MyController {
  @oas.get('/greet/{foo}/{bar}',
  @oas.response(200, SuccessModel)
  @oas.response(404, FooNotFound, BarNotFound)
  @oas.response(404, BazNotFound)
  greet() {
    return new SuccessModel({message: 'Hello, world!'});
  }
}

Checklist

  • npm test passes on your machine
  • New tests added or existing tests modified to cover all changes
  • Code conforms with the style guide
  • API Documentation in code was updated
  • Documentation in /docs/site was updated
  • Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

package.json Outdated Show resolved Hide resolved
@mschnee
Copy link
Contributor Author

mschnee commented Feb 4, 2020

Note: I'd love to figure out some way to use these decorators as response code hints in the future, but I'm not entirely sure how to go about doing that in a way that fits the current patterns.

e.g.

@oas.response(404, SomeErrorModel)

If the invoked method returns or throws instanceof SomeErrorModel (or ideally, a more direct test), and that model isn't associated with any other status codes for that route, and if the status has not already been set explicitly, then assume the developer intends to return 404.

@raymondfeng
Copy link
Contributor

@mschnee Can you rebase your PR against latest master branch with git fetch --all && git rebase upstream/master?

@mschnee mschnee force-pushed the feat/response-decorator branch 2 times, most recently from a3c1284 to 9651fe2 Compare February 6, 2020 21:19
@mschnee
Copy link
Contributor Author

mschnee commented Feb 7, 2020

Rebased from upstream and squashed!

Copy link
Member

@bajtos bajtos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love the user experience of the proposed API 👍

I'll leave the detailed review and approval to @raymondfeng.

docs/site/decorators/Decorators_openapi.md Outdated Show resolved Hide resolved
docs/site/decorators/Decorators_openapi.md Outdated Show resolved Hide resolved
@mschnee mschnee force-pushed the feat/response-decorator branch 2 times, most recently from c72441a to f34a97e Compare February 7, 2020 23:12
package.json Outdated Show resolved Hide resolved
@@ -20,6 +20,7 @@
"@loopback/repository": "^1.19.1",
"@loopback/testlab": "^1.10.3",
"@types/debug": "^4.1.5",
"@types/http-status": "^1.1.2",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see http-status as a dev dependency.

}

const successSchema: ResponseObject = {
description: httpStatus['200'],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should require http-status to be a dev dependency. The build is passing because the module is transitively added as follows:

rest
└─┬ [email protected]
  └── [email protected] 

responseCode: number,
...responseModelOrSpec: ResponseModelOrSpec[]
) {
const messageKey = String(responseCode) as keyof httpStatus.HttpStatus;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes http-status a regular dependency (instead of dev). Please add it to package.json.

// This file is licensed under the MIT License.
// License text available at https://opensource.org/licenses/MIT
import {MethodMultiDecoratorFactory} from '@loopback/core';
import * as httpStatus from 'http-status';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be import httpStatus from 'http-status' now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll make these changes today

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

Successfully merging this pull request may close these issues.

4 participants