-
-
Notifications
You must be signed in to change notification settings - Fork 37
feat: Ability to limit combinations #3344
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
Changes from all commits
25b4bed
ac7d133
120fa0d
3972cb2
a0adbb9
21f8b8b
7e08495
d07cefb
6cf2449
30d4e2c
9b05848
1bef48a
5acceb9
b686082
b55f73e
e95c5b5
f40c9a5
568ee1c
81f1b76
ce3303d
3371815
35a75f1
876e45d
5b0ae8d
8d305ad
aa645d9
9527763
60ee257
60cbf12
87aea97
497ad4f
951a11d
3a02b85
3ff885f
f70c318
204ac1a
22f6563
18e622d
c969f22
1101b8b
bac11b9
eb4f21c
01d7d38
61ef57a
7b0fb4e
540716d
415e0b8
ba2f6dd
9251a67
1606f02
2973102
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 |
|---|---|---|
|
|
@@ -47,7 +47,8 @@ export class Diagnostics { | |
| } | ||
| } | ||
| for (const variant of responseVariants) { | ||
| for (const { mimeTypes, schema } of endpoint.getResponses(variant)) { | ||
| const responses = endpoint.getResponses(variant, { maxCombinations: 0 }); // no examples | ||
|
Contributor
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. This
Owner
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. @pullfrog wait a second, there is
Contributor
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. Pushed back on the reviewer's question with evidence that my original concern stands. Verified locally:
Contributor
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. The Confirmed with a direct call in this branch: Two possible fixes: early-return from
Owner
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. I've got your point on odd/even number of properties and how it may affect the result. |
||
| for (const { mimeTypes, schema } of responses) { | ||
| if (!mimeTypes?.includes(contentTypes.json)) continue; | ||
| const reason = findJsonIncompatible(schema, "output"); | ||
| if (reason) { | ||
|
|
@@ -75,6 +76,7 @@ export class Diagnostics { | |
| unrepresentable: "any", | ||
| io: "input", | ||
| }), | ||
| { maxCombinations: 0 }, // not required for this check | ||
| ); | ||
| for (const param of params) { | ||
| if (param in ref.flat.properties) continue; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ import { contentTypes } from "./content-type"; | |
| import { DocumentationError } from "./errors"; | ||
| import { getInputSources, makeCleanId } from "./common-helpers"; | ||
| import { CommonConfig } from "./config-type"; | ||
| import { getSecurityHeaders } from "./security"; | ||
| import { processContainers } from "./logical-container"; | ||
| import { ClientMethod } from "./method"; | ||
| import { | ||
|
|
@@ -86,6 +87,14 @@ interface DocumentationParams { | |
| * @example { users: "About users", files: { description: "About files", url: "https://example.com" } } | ||
| * */ | ||
| tags?: Parameters<typeof depictTags>[0]; | ||
| /** | ||
| * @desc Limits cartesian product when generating examples by combining each property's own examples. | ||
| * @desc Applies to: request/response examples, security scheme alternatives. | ||
| * @example 0 — disables product combinations, keeps concatenations | ||
| * @default Infinity | ||
| * @todo set to 10 or 20 in v28 to avoid too many combinations | ||
| * */ | ||
| maxCombinations?: number; | ||
| } | ||
|
|
||
| export class Documentation extends OpenApiBuilder { | ||
|
|
@@ -158,6 +167,7 @@ export class Documentation extends OpenApiBuilder { | |
| brandHandling, | ||
| tags, | ||
| isHeader, | ||
| maxCombinations, | ||
| hasSummaryFromDescription = true, | ||
| hasHeadMethod = true, | ||
| composition = "inline", | ||
|
|
@@ -173,6 +183,7 @@ export class Documentation extends OpenApiBuilder { | |
| endpoint, | ||
| composition, | ||
| brandHandling, | ||
| maxCombinations, | ||
| makeRef: this.#makeRef.bind(this), | ||
| }; | ||
| const { description, shortDescription, scopes, inputSchema } = endpoint; | ||
|
|
@@ -189,12 +200,12 @@ export class Documentation extends OpenApiBuilder { | |
| ); | ||
|
|
||
| const request = depictRequest({ ...commons, schema: inputSchema }); | ||
| const security = processContainers(endpoint.security); | ||
| const securityHeaders = getSecurityHeaders(endpoint.security); | ||
| const depictedParams = depictRequestParams({ | ||
| ...commons, | ||
| inputSources, | ||
| isHeader, | ||
| security, | ||
| securityHeaders, | ||
| request, | ||
| description: descriptions?.requestParameter?.call(null, { | ||
| method, | ||
|
|
@@ -205,7 +216,9 @@ export class Documentation extends OpenApiBuilder { | |
|
|
||
| const responses: ResponsesObject = {}; | ||
| for (const variant of responseVariants) { | ||
| const apiResponses = endpoint.getResponses(variant); | ||
| const apiResponses = endpoint.getResponses(variant, { | ||
| maxCombinations, | ||
| }); | ||
| for (const { mimeTypes, schema, statusCodes } of apiResponses) { | ||
| for (const statusCode of statusCodes) { | ||
| responses[statusCode] = depictResponse({ | ||
|
|
@@ -243,7 +256,10 @@ export class Documentation extends OpenApiBuilder { | |
| : undefined; | ||
|
|
||
| const securityRefs = depictSecurityRefs( | ||
| depictSecurity(security, inputSources), | ||
| depictSecurity( | ||
| processContainers(endpoint.security, maxCombinations), | ||
|
Contributor
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. Threading |
||
| inputSources, | ||
| ), | ||
| scopes, | ||
| (securitySchema) => { | ||
| const name = this.#ensureUniqSecuritySchemaName(securitySchema); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -57,6 +57,7 @@ export abstract class AbstractEndpoint { | |
| /** @internal */ | ||
| public abstract getResponses( | ||
| variant: ResponseVariant, | ||
| params: { maxCombinations?: number }, | ||
| ): ReadonlyArray<NormalizedResponse>; | ||
| /** @internal */ | ||
| public abstract getOperationId(method: ClientMethod): string | undefined; | ||
|
|
@@ -90,16 +91,16 @@ export class Endpoint< | |
| readonly #def: ConstructorParameters<typeof Endpoint<IN, OUT, CTX>>[0]; | ||
|
|
||
| /** considered expensive operation, only required for generators */ | ||
| #ensureOutputExamples = R.once(() => { | ||
| #ensureOutputExamples(limit?: number) { | ||
| if (globalRegistry.get(this.#def.outputSchema)?.examples?.length) return; // examples on output schema, or pull up: | ||
| if (!isSchema<z.core.$ZodObject>(this.#def.outputSchema, "object")) return; | ||
| const examples = pullResponseExamples(this.#def.outputSchema); | ||
| const examples = pullResponseExamples(this.#def.outputSchema, limit); | ||
| if (!examples.length) return; | ||
| const current = this.#def.outputSchema.meta(); | ||
| globalRegistry | ||
| .remove(this.#def.outputSchema) // reassign to avoid cloning | ||
| .add(this.#def.outputSchema, { ...current, examples }); | ||
| }); | ||
| } | ||
|
|
||
| constructor(def: { | ||
| deprecated?: boolean; | ||
|
|
@@ -172,8 +173,11 @@ export class Endpoint< | |
| } | ||
|
|
||
| /** @internal */ | ||
| public override getResponses(variant: ResponseVariant) { | ||
| if (variant === "positive") this.#ensureOutputExamples(); | ||
| public override getResponses( | ||
| variant: ResponseVariant, | ||
| { maxCombinations }: { maxCombinations?: number }, | ||
| ) { | ||
| if (variant === "positive") this.#ensureOutputExamples(maxCombinations); | ||
|
Contributor
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. With |
||
| return Object.freeze( | ||
| variant === "negative" | ||
| ? this.#def.resultHandler.getNegativeResponse() | ||
|
|
||

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.
The empty-array short-circuit intentionally ignores
limit, which is fine as a local contract forcombinationsbut leaks surprising behavior into every caller that reduces over a list (pullRequestExamples,pullResponseExamples,mergeExamplesviaflattenIO,processContainers). Atlimit=0, a reducer alternates between "ignore the limit and copyrightwholesale" on empty-left iterations and "return[]" on non-empty iterations, producing parity-dependent output — e.g. a 3-property request schema keeps only the last property's examples while a 2-property one returns none. The CHANGELOG description "Set to 0 to disable product but keep concatenations" doesn't match this behavior for multi-property schemas. Either makelimit=0short-circuit to[]here, or restructure the reducers solimit=0skips the product step entirely rather than piggy-backing oncombinations.