Expand @deprecated / @Deprecated support for OAS 3.0 #987

jenseng · 2021-05-10T23:48:00Z

Closes #975

OpenAPI 3.0 allows you to mark schemas and parameters as deprecated. See:

This PR enhances tsoa to look for @Deprecated decorators where TS allows them (on parameters and class properties) as well as @deprecated JSDoc annotations (on parameters and class/interface/alias properties) and sets the deprecated field accordingly when generating the 3.0 schema.

All Submissions:

Have you followed the guidelines in our Contributing document?
Have you checked to ensure there aren't other open Pull Requests for the same update/change?
Have you written unit tests?
Have you written unit tests that cover the negative cases (i.e.: if bad data is submitted, does the library respond properly)?
This PR is associated with an existing issue?

If this is a new feature submission:

Has the issue had a maintainer respond to the issue and clarify that the feature is something that aligns with the goals and philosophy of the project?

Potential Problems With The Approach

No major issues, though it didn't seem like there was a clear way to support @deprecated cascading down through non-trivial mapped types, as there is insufficient info in the AST. I was able to support it for straightforward mapped types that use keyof, and this behavior matches intellisense in VS Code, so maybe that's ok? See this comment and these test fixtures for more info.

Test plan

Added relevant tests, all passing, specifically covering:
@deprecated JSDoc annotations on class method parameters (i.e. controller operations)
@deprecated JSDoc annotations on class properties
@deprecated JSDoc annotations on class parameter properties
@deprecated JSDoc annotations on type/interface properties
@deprecated JSDoc annotations cascading through simple mapped types
@Deprecated decorators on class method parameters (i.e. controller operations)
@Deprecated decorators on class properties
@Deprecated decorators on class properties cascading through simple mapped types
@Deprecated decorators on class parameter properties
Negative tests for params/properties/etc that should not be deprecated
Negative tests for deprecated things that should not appear in the spec (e.g. non-public class parameter property)
Negative tests across the board for OAS < 3.0
Also verified end-to-end in my app and it added the missing deprecateds to swagger.json :)

jenseng · 2021-05-10T23:51:43Z

Oops, just realized I missed a test and an implementation detail... although tsoa recognizes deprecated parameters now, it doesn't set that in the OAS 3.0 schema (though it does for deprecated properties). I'll push a fix shortly.

jenseng · 2021-05-11T00:33:32Z

Ok fixed now, added several more tests as well

See lukeautry/tsoa#987

WoH · 2021-05-13T16:26:57Z

packages/cli/src/metadataGeneration/parameterGenerator.ts

@@ -305,6 +318,10 @@ export class ParameterGenerator {
    return undefined;
  }

+  private getParameterDeprecation(node: ts.ParameterDeclaration) {
+    return isExistJSDocTag(node, tag => tag.tagName.text === 'deprecated') || isDecorator(node, identifier => identifier.text === 'Deprecated');


I'm a little torn here, the other (JSDoc) modifiers don't have corresponding Parameter Decorators.

I don't feel too strongly, I can leave off the decorator logic here if you like. Since TS supports decorating parameters I figured why not give people the option. I do think it's good to keep the JSDoc annotation support here since people already use that in the wild and it's supported by IDEs/etc.

I think we keep it, I can't see a case where it's causing confusion especially compared to the issue we'd inevitably get eventually when someone tries to do it using decorators. Intent is clear and it doesn't interfere with anything else.

WoH · 2021-05-13T16:32:01Z

packages/runtime/src/swagger/swagger.ts

@@ -128,6 +128,7 @@ export namespace Swagger {
  }

  export type Parameter = BodyParameter | FormDataParameter | QueryParameter | PathParameter | HeaderParameter;
+  export type Parameter3 = Parameter & { deprecated?: boolean };


I'd prefer to keep this in Parameter and set x-deprecated if we are dealing with v2, wdty?

Sure, I can tweak this

jenseng · 2021-05-13T21:31:58Z

packages/runtime/src/swagger/swagger.ts

@@ -128,6 +128,7 @@ export namespace Swagger {
  }

  export type Parameter = BodyParameter | FormDataParameter | QueryParameter | PathParameter | HeaderParameter;
+  export type Parameter3 = Parameter & { deprecated?: boolean };


Sure, I can tweak this

jenseng · 2021-05-13T21:37:37Z

packages/cli/src/metadataGeneration/parameterGenerator.ts

@@ -305,6 +318,10 @@ export class ParameterGenerator {
    return undefined;
  }

+  private getParameterDeprecation(node: ts.ParameterDeclaration) {
+    return isExistJSDocTag(node, tag => tag.tagName.text === 'deprecated') || isDecorator(node, identifier => identifier.text === 'Deprecated');


I don't feel too strongly, I can leave off the decorator logic here if you like. Since TS supports decorating parameters I figured why not give people the option. I do think it's good to keep the JSDoc annotation support here since people already use that in the wild and it's supported by IDEs/etc.

jenseng · 2021-05-13T21:48:00Z

tests/fixtures/testModel.ts

+
+  /** not deprecated */
+  notDeprecatedProperty?: number;
+  /** type is deprecated, but this property is not */


I'm wondering if i should rethink this, as I'm right in the middle of adding better deprecated support to openapi-generator and this seems at odds with that... In OAS terms, this becomes a ref, and if that corresponding schema happens to be deprecated, then this property should be considered deprecated, as everything from the ref comes in.

By the same token I need a test explicitly around deprecating a type alias (like line 141, but not a primitive); the ref overrides everything including an explicit deprecated. Might need to give that some thought (perhaps emit a warning?)

Here's what I've previously done, I think it was w/ regards to descriptionss along $refs. We still add it to the spec, knowing whoever parses the spec is bound by the spec to treat this as: Any members [other than "$ref] SHALL be ignored. Warning about that seems reasonable, but we don't currently do it.

Seems like was a reasonable choice given that OpenAPI 3.1 allows for this explicitly: https://github.com/OAI/OpenAPI-Specification/blob/main/versions/3.1.0.md#referenceObject

…#975 OpenAPI 3.0 allows you to mark schemas and parameters as deprecated. See: * https://swagger.io/specification/#schemaDeprecated * https://swagger.io/specification/#parameterDeprecated This PR enhances tsoa to look for `@Deprecated` decorators where TS allows them (on parameters and class properties) as well as `@deprecated` JSDoc annotations (on parameters and class/interface/alias properties) and sets the `deprecated` field accordingly when generating the 3.0 schema. Testing notes: * Added relevant tests, all passing * Verified end-to-end in my app with deprecated properties :)

See added test cases around DeprecatedType and DeprecatedClass; when a a class or type is marked as deprecated, ensure that's reflected in the generated spec. For OpenAPI 2, set x-deprecated for any deprecated schemas and parameters.

jenseng · 2021-05-14T18:56:11Z

Rebased against latest master and added a second commit which (I think) addresses the PR feedback...

Rework Parameter/Parameter2/Parameter3
Add x-deprecated to OAS 2
Fix an oversight where deprecated classes/types weren't reflected in the generated spec (see DeprecatedClass and DeprecatedType in the tests)

WoH · 2021-05-15T13:55:58Z

Diffs look good, I'll try to make time to check this out locally tomorrow unless there's anything I should wait for.

jenseng · 2021-05-15T15:02:39Z

I think it’s good to go, but let me know if anything else needs to change. Thanks!

WoH · 2021-05-16T16:06:42Z

LGTM, thanks a lot for this addition!

jenseng force-pushed the support-deprecated-schemas branch from cf96f3c to 2bc8574 Compare May 11, 2021 00:33

jenseng added a commit to jenseng/docs that referenced this pull request May 11, 2021

Expanded @decorator support

3353271

See lukeautry/tsoa#987

jenseng added a commit to jenseng/docs that referenced this pull request May 11, 2021

Document expanded @deprecated support

0e98912

See lukeautry/tsoa#987

jenseng mentioned this pull request May 11, 2021

Document expanded @Deprecated support tsoa-community/docs#32

Merged

WoH reviewed May 13, 2021

View reviewed changes

jenseng commented May 13, 2021

View reviewed changes

jenseng added 2 commits May 14, 2021 12:50

jenseng force-pushed the support-deprecated-schemas branch from 2bc8574 to 05a558e Compare May 14, 2021 18:52

WoH merged commit 704f30f into lukeautry:master May 16, 2021

jenseng deleted the support-deprecated-schemas branch May 17, 2021 15:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expand @deprecated / @Deprecated support for OAS 3.0 #987

Expand @deprecated / @Deprecated support for OAS 3.0 #987

jenseng commented May 10, 2021

jenseng commented May 10, 2021

jenseng commented May 11, 2021

WoH May 13, 2021

jenseng May 13, 2021

WoH May 13, 2021

WoH May 13, 2021

jenseng May 13, 2021

jenseng May 13, 2021

jenseng May 13, 2021

jenseng May 13, 2021

WoH May 13, 2021 •

edited

jenseng commented May 14, 2021

WoH commented May 15, 2021

jenseng commented May 15, 2021

WoH commented May 16, 2021

Expand @deprecated / @Deprecated support for OAS 3.0 #987

Expand @deprecated / @Deprecated support for OAS 3.0 #987

Conversation

jenseng commented May 10, 2021

All Submissions:

If this is a new feature submission:

jenseng commented May 10, 2021

jenseng commented May 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WoH May 13, 2021 • edited

Choose a reason for hiding this comment

jenseng commented May 14, 2021

WoH commented May 15, 2021

jenseng commented May 15, 2021

WoH commented May 16, 2021

WoH May 13, 2021 •

edited