feat: option to include models tagged with @openapi.ignore in the spec file#2221
Conversation
📝 WalkthroughWalkthroughThe includedModels getter in Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant GeneratorBase
participant ModelSet
Caller->>GeneratorBase: get includedModels(options)
alt includeOpenApiIgnored == true
GeneratorBase->>ModelSet: return all models
else includeOpenApiIgnored == false
GeneratorBase->>ModelSet: filter out @@openapi.ignore
end
GeneratorBase-->>Caller: models list
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/plugins/openapi/src/generator-base.ts (3)
18-25: Nit: compute models once and simplify the conditionalMinor readability/refactor: avoid calling
getDataModelsin both branches and unify the return.Apply this diff:
- const includeApiIgnored = this.getOption<boolean>('includeApiIgnored', false); - if (includeApiIgnored) { - return getDataModels(this.model); - } else { - return getDataModels(this.model).filter( - (d) => !hasAttribute(d, '@@openapi.ignore') - ); - } + const includeApiIgnored = this.getOption<boolean>('includeApiIgnored', false); + const models = getDataModels(this.model); + return includeApiIgnored ? models : models.filter((d) => !hasAttribute(d, '@@openapi.ignore'));
18-25: Consider aligning option naming with existing conventionsIf other options use “openapi” consistently, consider renaming to
includeOpenapiIgnoredfor clarity and discoverability. Since this is a new option, this can be done before release to avoid future breaking changes.
18-25: Add tests to cover inclusion/exclusion behaviorRecommend adding tests that assert:
- Default behavior (no option or false) filters out models with '@@openapi.ignore'.
- When
includeApiIgnored: true, ignored models are included.I can help draft integration tests for the OpenAPI plugin that validate the generated schema components.
lenageorgescu
changed the title
lenageorgescu
changed the title
ymc9
left a comment
ymc9
left a comment
There was a problem hiding this comment.
Hi @lenageorgescu-dsl , thanks for working on it. The change looks good to me. Do you want to add documentation for it as well? It's in a separate repo:
https://github.com/zenstackhq/zenstack-docs/blob/main/docs/reference/plugins/openapi.mdx
|
Thanks @ymc9 , I've updated the docs accordingly: |
Thank you! I'm merging this PR and will include it in the upcoming 2.18.1 release. The doc PR will be merged when the release is out. |
2 participants
Add option "includeOpenApiIgnored" to the OpenAPI-Plugin. If set to true, models tagged with @openapi.ignore are also included in the generated spec.
Our use case is to produce separate API specifications for internal and external audiences. By providing a flag in the OpenAPI plugin, we could easily generate two different spec files - one for internal use and one for customers.