feat: add rule to validate query and querystring parameters usage#2551
feat: add rule to validate query and querystring parameters usage#2551
Conversation
🦋 Changeset detectedLatest commit: 5d7330b 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 |
|
📦 A new experimental 🧪 version v0.0.0-snapshot.1770712109 of Redocly CLI has been published for testing. Install with NPM: npm install @redocly/cli@0.0.0-snapshot.1770712109
# or
npm install @redocly/openapi-core@0.0.0-snapshot.1770712109
# or
npm install @redocly/respect-core@0.0.0-snapshot.1770712109 |
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
packages/core/src/rules/oas3/spec-no-mixed-query-and-querystring-parameters.ts
Outdated
Show resolved
Hide resolved
.changeset/young-ghosts-drop.md
Outdated
| "@redocly/cli": minor | ||
| --- | ||
|
|
||
| Added `spec-no-mixed-query-and-querystring-parameters` rule to validate `query` and `querystring` parameter usage in OpenAPI 3.2. |
There was a problem hiding this comment.
Maybe it's also worth checking that it's the only one querystring declared on the Operation/PathItem to follow the other part of the spec restriction (https://spec.openapis.org/oas/v3.2.0.html#parameter-locations)? I'd suggest renaming the rule to something more general then, i.e. to spec-querystring-parameters.
packages/core/src/rules/oas3/__tests__/spec-no-mixed-query-and-querystring-parameters.test.ts
Outdated
Show resolved
Hide resolved
packages/core/src/rules/oas3/__tests__/spec-querystring-parameters.test.ts
Show resolved
Hide resolved
packages/core/src/rules/oas3/__tests__/spec-querystring-parameters.test.ts
Show resolved
Hide resolved
| }, | ||
|
|
||
| Parameter(parameter: Oas3Parameter, ctx: UserContext) { | ||
| const location = ctx.parentLocations.PathItem.child(['parameters', ctx.key]); |
There was a problem hiding this comment.
This looks a bit too complicated. Cannot we simply use the ctx.location or something similar instead?
| return parameters.filter((p) => !('$ref' in p) && p.in === 'querystring').length; | ||
| } | ||
|
|
||
| const QUERYSTRING_ONCE_MESSAGE = |
There was a problem hiding this comment.
Doesn't make sense to declare a const. Simply use it in place.
| } | ||
|
|
||
| const QUERYSTRING_ONCE_MESSAGE = | ||
| 'Parameters with `in: querystring` should be defined only once per path/operation parameter set (OpenAPI 3.2).'; |
There was a problem hiding this comment.
| 'Parameters with `in: querystring` should be defined only once per path/operation parameter set (OpenAPI 3.2).'; | |
| 'Parameters with `in: querystring` should be defined only once per path/operation parameter set.'; |
I don't think it's needed.
|
@tatomyr: should this rule be documented in Built-in rules? |
| import { BaseResolver } from '../../../resolve.js'; | ||
| import { createConfig } from '../../../config/index.js'; | ||
|
|
||
| describe('OAS3 spec-querystring-parameters (OAS 3.2)', () => { |
There was a problem hiding this comment.
Although querystring was introduced in 3.2, this rule also included into other that 3.2 specs, do we need to mention (OAS 3.2) in describe?
There was a problem hiding this comment.
No, it won't appear on other spec flavours.
There was a problem hiding this comment.
Oh, you mean in the describe's message? Let it remain there for the sake of clarity.
There was a problem hiding this comment.
I think now it makes sense to keep it, I did the changes that @tatomyr suggested below
|
Please disregard the failing smokes. They are related to a separate issue. |
|
|
||
| # spec-querystring-parameters | ||
|
|
||
| Enforce valid use of parameters with `in: querystring` (OpenAPI 3.2). |
There was a problem hiding this comment.
| Enforce valid use of parameters with `in: querystring` (OpenAPI 3.2). | |
| Enforces valid use of querystring parameters. |
Please follow the style of other documents.
|
|
||
| | Option | Type | Description | | ||
| | -------- | ------ | ------------------------------------------------------------------------------------------------- | | ||
| | severity | string | Possible values: `off`, `warn`, `error`. Default `off` (3.0/3.1), `error` in `recommended` (3.2). | |
There was a problem hiding this comment.
| | severity | string | Possible values: `off`, `warn`, `error`. Default `off` (3.0/3.1), `error` in `recommended` (3.2). | | |
| | severity | string | Possible values: `off`, `warn`, `error`. Default `error` (in `recommended` configuration). | |
| /search: | ||
| get: | ||
| parameters: | ||
| - name: qs1 |
There was a problem hiding this comment.
Coudl you come up with something meaningful? This is the documentation that our users will face, and as a user I'd love to see a well-thought example.
| - [path-not-include-query](./path-not-include-query.md) | ||
| - [operation-parameters-unique](./operation-parameters-unique.md) | ||
| - [configurable rules](../configurable-rules.md) |
There was a problem hiding this comment.
| - [path-not-include-query](./path-not-include-query.md) | |
| - [operation-parameters-unique](./operation-parameters-unique.md) | |
| - [configurable rules](../configurable-rules.md) | |
| - [path-not-include-query](./path-not-include-query.md) # -- not sure about this one | |
| - [operation-parameters-unique](./operation-parameters-unique.md) |
| - [security-defined](./oas/security-defined.md): Security rules must be defined, either globally or per-operation | ||
| - [struct](./common/struct.md): Conform to the declared OpenAPI specification version | ||
| - [spec-components-invalid-map-name](./oas/spec-components-invalid-map-name.md): Use only alphanumeric and basic punctuation as key names in the components section | ||
| - [spec-querystring-parameters](./oas/spec-querystring-parameters.md): Enforce valid use of `in: querystring` (OpenAPI 3.2): at most one per path/operation, and not mixed with `in: query` |
There was a problem hiding this comment.
Please also check if we need to add the rule reference to a sidebar.
|
Left comments mostly related to the docs. |
4 participants
What/Why/How?
A new rule has been added because if any parameters are in
in: query, you cannot usein: querystring, and vice versa.Reference
https://github.com/Redocly/redocly/issues/15509
Testing
Screenshots (optional)
Check yourself
Security