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.
| const includeApiIgnored = this.getOption<boolean>('includeApiIgnored', false); | ||
| if (includeApiIgnored) { | ||
| return getDataModels(this.model); | ||
| } else { | ||
| return getDataModels(this.model).filter( | ||
| (d) => !hasAttribute(d, '@@openapi.ignore') | ||
| ); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Confirm attribute token: '@@openapi.ignore' vs '@openapi.ignored'
PR description mentions '@openapi.ignored' but the code filters by '@@openapi.ignore'. Please verify the canonical attribute name used across the codebase and docs to avoid confusion and mismatches.
Run this script to scan the repo for both tokens and the new option name:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Searching for attribute usages:"
rg -n --hidden -S "@@openapi.ignore|@openapi.ignored|@openapi.ignore|@@openapi.ignored"
echo
echo "Searching for 'includeApiIgnored' usages and docs:"
rg -n --hidden -S "includeApiIgnored"
echo
echo "Common hasAttribute checks around openapi ignore:"
rg -n --hidden -S "hasAttribute\\(.*openapi\\.ignore"Length of output: 1344
Use consistent attribute token “@@openapi.ignore”
The only attribute token found in code, tests, and the ZModel spec is @@openapi.ignore. There are no occurrences of @openapi.ignored or similar. Please update any documentation and the PR description to reference @@openapi.ignore so it matches the implementation and test suite.
• Update the PR description (and any docs/README) to use @@openapi.ignore.
• Ensure examples in documentation/tests consistently show @@openapi.ignore() if using it as a decorator.
• Confirm that consumers are aware that the flag is includeApiIgnored but the attribute to opt-out remains @@openapi.ignore.
🤖 Prompt for AI Agents
In packages/plugins/openapi/src/generator-base.ts around lines 18 to 25, the
review points out inconsistent attribute token naming across docs/PR
description/tests versus the implementation which uses @@openapi.ignore; update
the PR description, any documentation (README, examples, doc comments) and test
descriptions to consistently reference @@openapi.ignore (use @@openapi.ignore()
when shown as a decorator), ensure no leftover references to @openapi.ignored or
similar remain, and add a short clarifying note in docs that the runtime option
is includeApiIgnored but the attribute to opt out is @@openapi.ignore.
lenageorgescu-dsl
changed the title
lenageorgescu-dsl
changed the title
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: |
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.