Skip to content

Conversation

@mschnee
Copy link
Contributor

@mschnee mschnee commented Jan 13, 2020

Partially implements: #4300
See: #4406

This has been split from #4406 as a more manageable, smaller PR.

Added

This PR adds @oas.tags to Controller classes and methods, and adds an array of tag strings in accordance with the OpenAPI Operation Object specification.

As the spec defines tags as an optional property, this feature will only add tags: string[] when tags are set, and never an empty array of tags.

Using the @oas.tags decorator will set the tags on a path, and will be appended to any @tags set on a class.

This decorator is added to the oas namespace export in order to prevent naming conflicts with other popular decorators. The existing decorators in @loopback/openapi-v3 have also been added to the oas namespace (but not removed from the main export).

Not supported

This PR has no bearing on the top-level tags entity in the OpeAPI Tag Object specification. Operation Tags in the spec are themselves convenience for easily grouping and sorting paths and operations, and don't require an entry in the top level.

Examples

@oas.tags('Foo', 'Bar')
class MyController {
  @oas.tags('Baz')
  @oas.get('/greet')
  public async greet() {
    // tags will be [Foo, Bar, Baz]
  }
}

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
  • (n/a) Affected artifact templates in packages/cli were updated
  • Affected example projects in examples/* were updated

@mschnee mschnee requested a review from raymondfeng January 14, 2020 21:41
@raymondfeng
Copy link
Contributor

I wonder if we should expose such decorators under a namespace (for example, @oas.tags) to avoid confusion against @bind({tags: ...}.

@mschnee
Copy link
Contributor Author

mschnee commented Jan 15, 2020

That's a question I don't myself have an opinion on as a developer. I do know that sometimes, vscode intellisense will auto-import in a way I wasn't expecting: for example, typing @get might cause it to import { get } from 'http';,

But at the same time, I could import as instead:

import * as oas from '@loopback/openapi-v3';
@oas.api({})
@oas.deprecated()
class MyController {
    @oas.get('/greet')
    public async greet() {}
}

My only opinion as a developer is to be able to use some convenience decorators so that I don't have to write and maintain such verbose json blocks.

@mschnee mschnee requested a review from raymondfeng January 15, 2020 23:44
@raymondfeng
Copy link
Contributor

@bajtos Please review.

Copy link
Contributor

@emonddr emonddr left a comment

Choose a reason for hiding this comment

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

  1. I agree with @raymondfeng about : @oas.tag
  2. Wondering why todo controller from examples is being changed.

@mschnee mschnee force-pushed the feat/tags-decorator branch 2 times, most recently from 0671683 to 7623123 Compare January 27, 2020 20:53
@mschnee
Copy link
Contributor Author

mschnee commented Jan 27, 2020

☝️ I am attempting to resolve build issues after the most recent rebase.

@mschnee mschnee force-pushed the feat/tags-decorator branch 2 times, most recently from 68e2fc1 to 43ba86e Compare January 28, 2020 00:51
@mschnee mschnee force-pushed the feat/tags-decorator branch from 43ba86e to 55f199f Compare January 28, 2020 17:28
## Convenience Decorators

While you can supply a fully valid OpenAPI specification for the class-level
`@api` decorator, and full operation OpenAPI specification for `@op` and the
Copy link
Contributor

Choose a reason for hiding this comment

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

@op -> @operation?

allow you to supply specific OpenAPI information without requiring you to use
verbose JSON.

## OAS
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe ## Shortcuts for OpenAPI Spec (OAS) Objects?

@dougal83
Copy link
Contributor

Thinking about OAS enhancers, would it be a good/practical idea to add an optional description for each tag that would populate a sibling extension property? This information can be used (and cleaned up) by enhancer that would populate spec.tags. Draft tag enhancer.

  "tags": [
    "PingController"
  ],
  "x-tags": [
    {"description": "Api Ping response" }
  ],

@raymondfeng
Copy link
Contributor

@mschnee Please address my last two comments so that we can land this PR soon.

@raymondfeng
Copy link
Contributor

@dougal83 Please note there is a top level tags as an array of TagObject. The operation level tags is string[] containing references to top-level tags by name.

@mschnee
Copy link
Contributor Author

mschnee commented Jan 30, 2020

Busy week, sorry for the delay! I am making the requested updated now.

@dougal83 I love the idea in general- the OAS spec has a top level tags block that allows for more extensive description. You sen peruse the spec documents here. It's not required to reference a tag in the operation spec, though. They can be the same, but the strings in a path tags are completely arbitrary.

For the first iteration of these convenience decorators, I'm trying to limit the scope to the direct developer experience of writing controllers.

You could probably do this now by creating your own top-level tags object, and referencing it:

// in tags file
export const myTags {
   Foo: {
      name: 'foo',
      description: 'foo tag'
   },
  Bar: {
    name: 'bar',
    description: 'bar tag'
  }
}

export const oasTags = Object.values(myTags); // use this in your api spec

// in controller
import { myTags } from '../../wherever'

@oas.tags(myTags.Foo.name, myTags.Bar.name)
class MyController {}

@mschnee mschnee force-pushed the feat/tags-decorator branch from 55f199f to d9e856a Compare January 30, 2020 17:55
@mschnee mschnee requested a review from raymondfeng January 30, 2020 17:55
@mschnee
Copy link
Contributor Author

mschnee commented Jan 30, 2020

Documentation updated.

Thanks for your patience and quick response @raymondfeng ! I'm going to take all of your feedback here and apply it to my PRs for the @deprecated and @response decorators.

@raymondfeng raymondfeng merged commit a8722dc into loopbackio:master Jan 30, 2020
@raymondfeng
Copy link
Contributor

@mschnee PR landed. Great contribution! 🥇

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.

5 participants