feat: add AsyncAPI support in the stats command#2553
feat: add AsyncAPI support in the stats command#2553DmitryAnansky merged 10 commits intoRedocly:mainfrom
Conversation
🦋 Changeset detectedLatest commit: de85bd9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
| @@ -1 +1 @@ | |||
| ../../../../resources/pets.yaml No newline at end of file | |||
| ../../../../resources/pets.yaml | |||
There was a problem hiding this comment.
Looks like nothing changed here.
There was a problem hiding this comment.
Could you revert this change? It seems it was accidental.
packages/cli/src/commands/stats.ts
Outdated
|
|
||
| function printStatsStylish(statsAccumulator: StatsAccumulator) { | ||
| // Determine which stats are relevant for a given spec version | ||
| function isStatRelevant(stat: StatsName, specVersion: SpecVersion): boolean { |
There was a problem hiding this comment.
The walker should handle this by itself. To get different output for different specs, please use different statsAccumulator presets. (If I get your intention correctly.)
| }, | ||
| }); | ||
|
|
||
| export const Stats = (statsAccumulator: StatsAccumulator, specVersion?: SpecVersion) => { |
There was a problem hiding this comment.
I suggest creating different Stats rules for different specifications/version: SpecOAS, SpecAsync2, SpecAsync3 (for now). This will help to avoid collisions and later on will ease our way when migrating to a more robust solution (ultimately, the stats should use the same approach as lint and bundle, but it requires a bit of a preparation on our side, so I'd go with this PR first, and then we'll implement what's needed on our side).
There was a problem hiding this comment.
Chances are, we'll need separate statsAccumulators too to be able to scope the stats we're showing to the spec.
| statsAccumulator.externalDocs.total++; | ||
| }, | ||
| // Common visitor for all specs (refs, tags, externalDocs, links) | ||
| const getCommonVisitor = (statsAccumulator: StatsAccumulator) => ({ |
There was a problem hiding this comment.
Please do not use common visitors, just duplicate what's needed. It's better for understanding the actual structure of the data we're trying to get.
| // Handle refs in messages array (AsyncAPI 3 specific) | ||
| // Note: The ref visitor may not catch refs in arrays due to walker limitations, |
There was a problem hiding this comment.
What is the limitation? If there's any that doesn't allow us to walk here, it should be fixed.
tatomyr
left a comment
There was a problem hiding this comment.
Already much better, thank you! One more push needed though--left a couple of comments.
packages/cli/src/commands/stats.ts
Outdated
| parameters: { metric: '👉 Parameters', total: 0, color: 'yellow', items: new Set() }, | ||
| links: { metric: '🔗 Links', total: 0, color: 'cyan', items: new Set() }, | ||
| pathItems: { metric: '🔀 Path Items', total: 0, color: 'green' }, | ||
| channels: { metric: '📡 Channels', total: 0, color: 'green' }, |
There was a problem hiding this comment.
This is actually the point of the split for the accumulators: let's declare only the specific fields there.
| channels: { metric: '📡 Channels', total: 0, color: 'green' }, |
packages/cli/src/commands/stats.ts
Outdated
| links: { metric: '🔗 Links', total: 0, color: 'cyan', items: new Set() }, | ||
| pathItems: { metric: '🔀 Path Items', total: 0, color: 'green' }, |
There was a problem hiding this comment.
There's no links and pathItems in AsyncAPI (and probably other fields too).
packages/cli/src/commands/stats.ts
Outdated
| }); | ||
|
|
||
| // Stats to show for OpenAPI | ||
| const oasStatsToShow: Set<StatsName> = new Set([ |
There was a problem hiding this comment.
Just put only those things into accumulator objects (and then check if they are defined why formatting the output). This will simplify the code as you won't have to pass statsToShow down to the printStats... functions.
packages/cli/src/commands/stats.ts
Outdated
| const isAsyncAPI = specVersion === 'async2' || specVersion === 'async3'; | ||
| const statsAccumulator = isAsyncAPI | ||
| ? createAsyncAPIStatsAccumulator() | ||
| : createOASStatsAccumulator(); | ||
| const statsToShow = isAsyncAPI ? asyncStatsToShow : oasStatsToShow; | ||
|
|
||
| const statsVisitorFn = | ||
| specVersion === 'async2' ? StatsAsync2 : specVersion === 'async3' ? StatsAsync3 : StatsOAS; |
There was a problem hiding this comment.
Could this be simplified like so?
| const isAsyncAPI = specVersion === 'async2' || specVersion === 'async3'; | |
| const statsAccumulator = isAsyncAPI | |
| ? createAsyncAPIStatsAccumulator() | |
| : createOASStatsAccumulator(); | |
| const statsToShow = isAsyncAPI ? asyncStatsToShow : oasStatsToShow; | |
| const statsVisitorFn = | |
| specVersion === 'async2' ? StatsAsync2 : specVersion === 'async3' ? StatsAsync3 : StatsOAS; | |
| const statsVisitor = | |
| specVersion === 'async2' ? StatsAsync2(statsAccumulatorAsync2) : specVersion === 'async3' ? StatsAsync3(statsAccumulatorAsync3) : StatsOAS(statsAccumulatorOAS); |
packages/core/src/typings/common.ts
Outdated
| | 'tags' | ||
| | 'externalDocs' | ||
| | 'pathItems' | ||
| | 'channels' |
There was a problem hiding this comment.
Can we have separate types for different specs?
|
And please rebase your branch against main--there were some changes that might conflict with yours. |
Co-authored-by: Dmytro Anansky <dmytro@redocly.com>
c5fcbbf to
4d9b3f6
Compare
| ref: { | ||
| enter(ref: OasRef) { | ||
| statsAccumulator.refs.items!.add(ref['$ref']); | ||
| statsAccumulator.refs?.items!.add(ref['$ref']); |
There was a problem hiding this comment.
Why do you need the optional chaining here? There are always items in the refs field unless I'm missing something. Just use the appropriate type. You can even infer the keys from the accumulator objects if needed.
There was a problem hiding this comment.
Same for if (statsAccumulator.parameters) { and other precautions.
| statsAccumulator.links.items!.add(link.operationId); | ||
| }, | ||
| }, | ||
| Root: { |
There was a problem hiding this comment.
Is changing the order really necessary? If not, please revert that so it's easier to read the diffs.
| @@ -1 +1 @@ | |||
| ../../../../resources/pets.yaml No newline at end of file | |||
| ../../../../resources/pets.yaml | |||
There was a problem hiding this comment.
Could you revert this change? It seems it was accidental.
| @@ -0,0 +1,606 @@ | |||
| asyncapi: '2.6.0' | |||
There was a problem hiding this comment.
Could you clean up test examples (preferably leaving a minimalistic sample) so it's easier to track the actual statistics?
packages/cli/src/commands/stats.ts
Outdated
| const json: any = {}; | ||
| for (const key of Object.keys(statsAccumulator)) { | ||
| const stat = statsAccumulator[key as keyof StatsAccumulator]; | ||
| if (!stat) continue; |
There was a problem hiding this comment.
What could be the case when there's no stat?
|
The coverage threshold still fails. Please relax it a bit in the config. |
DmitryAnansky
left a comment
There was a problem hiding this comment.
Left a small note regarding the comments in another file.
|
@tibisabau |
What/Why/How?
Added AsyncAPI 2.x and 3.x support to the
statscommand to provide statistics for AsyncAPI documents, similar to existing OpenAPI support. The stats command now counts channels, operations, schemas, references, tags, parameters, and external docs for AsyncAPI specifications.Why: Since AsyncAPI support was added for lint and bundle commands, the stats command should support it as well for consistency and completeness.
How: Extended the Stats visitor in
packages/core/src/rules/other/stats.tsto handle AsyncAPI-specific node types (ChannelMap,NamedChannels,NamedOperations) and map them to appropriate metrics (channels → pathItems, operations → operations).Reference
Closes #2353.
Testing
Screenshots (optional)
N/A
Check yourself
Security