-
Notifications
You must be signed in to change notification settings - Fork 226
feat: add vendor extensions metric with per-extension counts #3021
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
974233d
9a79e40
b957cfe
304374f
5af764e
d8f062b
3f2b9f8
2efddb1
c2439e9
05fbcdf
15e669c
c99afa0
3bb9922
d327074
13e252e
d1430fd
a7053a7
a68162f
561ede2
06d4e23
280804f
ef28db3
bbedead
66b983c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@redocly/openapi-core': patch | ||
| '@redocly/cli': patch | ||
| --- | ||
|
|
||
| Fixed the `stats` command reporting wrong parameter count for AsyncAPI descriptions. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@redocly/openapi-core': minor | ||
| '@redocly/cli': minor | ||
| --- | ||
|
|
||
| Added a Vendor Extensions metric to the `stats` command that reports how many distinct `x-` extensions a description file uses and how often each one occurs. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,9 @@ | ||
| import type { OASStatsAccumulator, AsyncAPIStatsAccumulator } from '../../typings/common.js'; | ||
| import type { | ||
| OASStatsAccumulator, | ||
| AsyncAPIStatsAccumulator, | ||
| SpecVendorExtensionsAccumulator, | ||
| StatsAccumulator, | ||
| } from '../../typings/common.js'; | ||
| import type { | ||
| Oas3Link, | ||
| Oas3Operation, | ||
|
|
@@ -8,9 +13,34 @@ import type { | |
| OasRef, | ||
| } from '../../typings/openapi.js'; | ||
| import type { Oas2Parameter } from '../../typings/swagger.js'; | ||
| import { collectSpecExtension } from '../../utils/spec-extensions.js'; | ||
| import type { UserContext } from '../../walk.js'; | ||
|
|
||
| function finalizeStats( | ||
| statsAccumulator: StatsAccumulator, | ||
| extensions: SpecVendorExtensionsAccumulator | ||
| ) { | ||
| for (const row of Object.values(statsAccumulator)) { | ||
| if (row.items) { | ||
| row.total = row.items.size; | ||
| } | ||
| } | ||
| const extensionNames = Object.keys(extensions).sort(); | ||
| statsAccumulator.xExtensions.total = extensionNames.length; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not use the same approach with Set like in other rows?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Moreover, maybe we can simply calculate the items' size in place instead of assigning the total?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On Set question already answered above. Four consumers (three printers and the portal collector) read .total uniformly for every row, so deriving the size at read time would push an items ? items.size : total check into each of them. This one-time assignment at Root.leave also predates the PR — the loop just replaces the four per-row copies that main already had. |
||
| statsAccumulator.xExtensions.details = Object.fromEntries( | ||
| extensionNames.map((name) => [name, extensions[name]]) | ||
| ); | ||
| } | ||
|
|
||
| export const StatsOAS = (statsAccumulator: OASStatsAccumulator) => { | ||
| const extensions: SpecVendorExtensionsAccumulator = {}; | ||
|
|
||
| return { | ||
| SpecExtension: { | ||
| enter(node: unknown, ctx: UserContext) { | ||
| collectSpecExtension(extensions, ctx.key.toString(), node); | ||
| }, | ||
| }, | ||
| ExternalDocs: { | ||
| leave() { | ||
| statsAccumulator.externalDocs.total++; | ||
|
|
@@ -32,6 +62,11 @@ export const StatsOAS = (statsAccumulator: OASStatsAccumulator) => { | |
| }, | ||
| }, | ||
| WebhooksMap: { | ||
| enter(node: unknown, ctx: UserContext) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We also have other known extensions, like the following: 'x-servers': 'XServerList',
'x-tagGroups': 'TagGroups',
'x-ignoredHeaderParameters': { type: 'array', items: { type: 'string' } },Have you covered them?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, they are covered. Added e2e tests to make it clear |
||
| if (ctx.key === 'x-webhooks') { | ||
| collectSpecExtension(extensions, 'x-webhooks', node); | ||
| } | ||
| }, | ||
| Operation: { | ||
| leave(operation: Oas3Operation) { | ||
| statsAccumulator.webhooks.total++; | ||
|
|
@@ -43,6 +78,13 @@ export const StatsOAS = (statsAccumulator: OASStatsAccumulator) => { | |
| }, | ||
| }, | ||
| }, | ||
| Operation: { | ||
| enter(operation: Oas3Operation, ctx: UserContext) { | ||
| if (ctx.key === 'x-query') { | ||
| collectSpecExtension(extensions, 'x-query', operation); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I see, it has its own type. |
||
| } | ||
| }, | ||
| }, | ||
| Paths: { | ||
| PathItem: { | ||
| leave() { | ||
|
|
@@ -74,17 +116,21 @@ export const StatsOAS = (statsAccumulator: OASStatsAccumulator) => { | |
| }, | ||
| Root: { | ||
| leave() { | ||
| statsAccumulator.parameters.total = statsAccumulator.parameters.items!.size; | ||
| statsAccumulator.refs.total = statsAccumulator.refs.items!.size; | ||
| statsAccumulator.links.total = statsAccumulator.links.items!.size; | ||
| statsAccumulator.tags.total = statsAccumulator.tags.items!.size; | ||
| finalizeStats(statsAccumulator, extensions); | ||
| }, | ||
| }, | ||
| }; | ||
| }; | ||
|
|
||
| export const StatsAsync2 = (statsAccumulator: AsyncAPIStatsAccumulator) => { | ||
| const extensions: SpecVendorExtensionsAccumulator = {}; | ||
|
|
||
| return { | ||
| SpecExtension: { | ||
| enter(node: unknown, ctx: UserContext) { | ||
| collectSpecExtension(extensions, ctx.key.toString(), node); | ||
| }, | ||
| }, | ||
| ExternalDocs: { | ||
| leave() { | ||
| statsAccumulator.externalDocs.total++; | ||
|
|
@@ -116,10 +162,8 @@ export const StatsAsync2 = (statsAccumulator: AsyncAPIStatsAccumulator) => { | |
| }, | ||
| }, | ||
| Parameter: { | ||
| leave(parameter: any) { | ||
| if (parameter.name) { | ||
| statsAccumulator.parameters.items!.add(parameter.name); | ||
| } | ||
| leave(_: unknown, { key }: UserContext) { | ||
| statsAccumulator.parameters.items!.add(key.toString()); | ||
| }, | ||
| }, | ||
| }, | ||
|
|
@@ -133,16 +177,21 @@ export const StatsAsync2 = (statsAccumulator: AsyncAPIStatsAccumulator) => { | |
| }, | ||
| Root: { | ||
| leave() { | ||
| statsAccumulator.parameters.total = statsAccumulator.parameters.items!.size; | ||
| statsAccumulator.refs.total = statsAccumulator.refs.items!.size; | ||
| statsAccumulator.tags.total = statsAccumulator.tags.items!.size; | ||
| finalizeStats(statsAccumulator, extensions); | ||
| }, | ||
| }, | ||
| }; | ||
| }; | ||
|
|
||
| export const StatsAsync3 = (statsAccumulator: AsyncAPIStatsAccumulator) => { | ||
| const extensions: SpecVendorExtensionsAccumulator = {}; | ||
|
|
||
| return { | ||
| SpecExtension: { | ||
| enter(node: unknown, ctx: UserContext) { | ||
| collectSpecExtension(extensions, ctx.key.toString(), node); | ||
| }, | ||
| }, | ||
| ExternalDocs: { | ||
| leave() { | ||
| statsAccumulator.externalDocs.total++; | ||
|
|
@@ -164,10 +213,8 @@ export const StatsAsync3 = (statsAccumulator: AsyncAPIStatsAccumulator) => { | |
| statsAccumulator.channels.total++; | ||
| }, | ||
| Parameter: { | ||
| leave(parameter: any) { | ||
| if (parameter.name) { | ||
| statsAccumulator.parameters.items!.add(parameter.name); | ||
| } | ||
| leave(_: unknown, { key }: UserContext) { | ||
| statsAccumulator.parameters.items!.add(key.toString()); | ||
| }, | ||
| }, | ||
| }, | ||
|
|
@@ -193,9 +240,7 @@ export const StatsAsync3 = (statsAccumulator: AsyncAPIStatsAccumulator) => { | |
| }, | ||
| Root: { | ||
| leave() { | ||
| statsAccumulator.parameters.total = statsAccumulator.parameters.items!.size; | ||
| statsAccumulator.refs.total = statsAccumulator.refs.items!.size; | ||
| statsAccumulator.tags.total = statsAccumulator.tags.items!.size; | ||
| finalizeStats(statsAccumulator, extensions); | ||
| }, | ||
| }, | ||
| }; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just saw we have
itemsinside thetagsaccumulator (one line above). What if we arrange extensions similarly to tags? Is there something else we are going to show in the output except of the extension items and their count?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd need a Map (a Set can't do keyed count++ per occurrence), which turns items into Set | Map<string, {count, props}> and pushes type-narrowing into every consumer — plus a Map isn't JSON-serializable for the JSON printer and the portal collector. Keeping items as "distinct names, size = total" and putting the per-extension payload in details seemed cleaner for me