Skip to content
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

matrix parameter type not supported with openapi3 #1863

Closed
lintaba opened this issue Oct 3, 2021 · 6 comments · Fixed by #1864
Closed

matrix parameter type not supported with openapi3 #1863

lintaba opened this issue Oct 3, 2021 · 6 comments · Fixed by #1864
Labels
enhancement New feature or request OpenAPI Issues related to the OpenAPI ruleset released

Comments

@lintaba
Copy link
Contributor

lintaba commented Oct 3, 2021

Describe the bug
openapi 3 (or swagger2) supports matrix parameter style, which looks like this:
/foo/bar;key=value/baz
in this case, the query path should look like this: /foo/bar{;key}/baz.
however its currently not supported, and gives the following error message:
Parameter "key" must be used in path "/foo/bar{;key}/baz".
It most probably comes from https://github.com/stoplightio/spectral/blob/develop/packages/rulesets/src/oas/functions/oasPathParam.ts#L5 , where the regex does not matches ;. Note to mention, that it may also contains asterix (*) and question mark (?) by the specification.

Also it seems it doesnt work even without the ; , when there are more than one parameters are given.

To Reproduce

  1. Given this OpenAPI/AsyncAPI document '...'
{
  "openapi": "3.0.0",
  "info": { "title": "matrix failure", "version": "1.0"},
  "servers": [{"url": "http://localhost:3000"}],
  "paths": {
    "/foo{;key}": {
      "parameters": [
        {
          "name": "key",
          "in": "path",
          "schema": {
            "type": "string"
          },
          "required": true,
          "style": "matrix"
        }
      ],
      "get": { "responses": { "200": { "description": "OK"} } } },
    "/foo{key}{another}": {
      "parameters": [
        {
          "name": "key",
          "in": "path",
          "schema": {
            "type": "string"
          },
          "required": true,
          "style": "matrix"
        },
        {
          "name": "another",
          "in": "path",
          "schema": {
            "type": "string"
          },
          "required": true,
          "style": "matrix"
        }
      ],
      "get": { "responses": { "200": { "description": "OK"} } } }
	}
}
  1. Run this CLI command:
$ cat .spectral.json 
{\n\t"extends": ["spectral:oas", "spectral:asyncapi"]\n}
$ spectral lint example.json
[...]
 8:28    error  path-params                 Parameter "key" must be used in path "/foo{;key}".             paths./foo{;key}.parameters[0]
  30:9    error  path-params            Parameter "another" must be used in path "/foo{key}{another}".                            paths./foo{key}{another}.parameters[1]
 40:13    error  path-params            Operation must define parameter "{keyanother}" as expected by path "/foo{key}{another}".  paths./foo{key}{another}.get

Expected behavior
Expected to support matrix parameters :)

Environment (remove any that are not applicable):

  • Library version: 6.0.0
  • OS: osx

Additional context

related specs: https://swagger.io/docs/specification/serialization/#query

@P0lip P0lip added OpenAPI Issues related to the OpenAPI ruleset t/bug Something isn't working labels Oct 4, 2021
@P0lip
Copy link
Contributor

P0lip commented Oct 4, 2021

Hey!
Your findings are indeed correct. We don't support matrix parameter type just yet.
The support for that would need to be added somewhere in https://github.com/stoplightio/spectral/blob/develop/packages/rulesets/src/oas/functions/oasPathParam.ts as you indicated in the issue.
Would you be open to tackling this issue?
Happy to provide you any support you may need to get started.

@lintaba
Copy link
Contributor Author

lintaba commented Oct 4, 2021

Hey!

I've tried to set up a working copy to check whether my changes are correct, but i hadnt succeeded. (cloned the repo, but havent found a working entry point :/ ) If you can help me set it up, i can check if my fix is correct.

However I think the following change would do the trick:
https://github.com/stoplightio/spectral/blob/develop/packages/rulesets/src/oas/functions/oasPathParam.ts#L5

const pathRegex = /(\{;?\??[a-zA-Z0-9_-]+\*?\})/g;

and probably this also needed:
https://github.com/stoplightio/spectral/blob/develop/packages/rulesets/src/oas/functions/oasPathParam.ts#L121

      const p = match[0].replace(/[{}?*;]/g, '');

Also I'm not sure about the reasoning behind that trailing + in the pathRegex. It now matches for {foo}{bar} as one match, instead of two. Is it the intended behaviour? My proposed change would change it, however that may break existing behavior. It definitely not compatible with matrix parameter type (or needs a bit more hack to slice it later)

@lintaba
Copy link
Contributor Author

lintaba commented Oct 4, 2021

I made a PR with these changes, but cannot validate/test it on my own :/

@raleigh04 raleigh04 added enhancement New feature or request and removed t/bug Something isn't working labels Oct 4, 2021
@P0lip
Copy link
Contributor

P0lip commented Oct 4, 2021

Hey!
You can verify the changes over here https://github.com/stoplightio/spectral/blob/develop/packages/rulesets/src/oas/__tests__/path-params.test.ts

Just add any examples, and the expected results and then run yarn test.jest packages/rulesets/src/oas/__tests__/path-params.test.ts

@lintaba
Copy link
Contributor Author

lintaba commented Oct 4, 2021

Thanks @P0lip, with your help I was able to write two tests for the given examples. Both cases are covered, tests are green for these changes (and also fails without my fix.)

#1864

P0lip added a commit that referenced this issue Oct 18, 2021
* support matrix parameter style for openapi3

#1863

* fix(ruleset): added tests for matrix parameters

Co-authored-by: Jakub Rożek <jakub@stoplight.io>
stoplight-bot pushed a commit that referenced this issue Oct 19, 2021
# [@stoplight/spectral-rulesets-v1.2.6](https://github.com/stoplightio/spectral/compare/@stoplight/spectral-rulesets-v1.2.5...@stoplight/spectral-rulesets-v1.2.6) (2021-10-19)

### Bug Fixes

* **rulesets:** support matrix parameter style for openapi3 ([#1864](#1864)) ([44b59d5](44b59d5)), closes [#1863](#1863)
@stoplight-bot
Copy link
Collaborator

🎉 This issue has been resolved in version @stoplight/spectral-rulesets-v1.2.6 🎉

The release is available on npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request OpenAPI Issues related to the OpenAPI ruleset released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants