feat(functions): expand xor function and add or function#2765
feat(functions): expand xor function and add or function#2765cuttingclyde wants to merge 197 commits into
Conversation
frankkilcommins
left a comment
There was a problem hiding this comment.
@cuttingclyde apologies in the delayed review here.
Looks great. Just a few textual adjustments and I'm suggesting that for XOR and OR usage we should remain at a minimum of two enumerated properties. For singular usage, we'd recommend defined as the function.
|
@frankkilcommins - Updated as requested the best I could. In the year since making this contribution I've switched laptops and am not able to get the tests to run locally by following the CONTRIBUTION.md instructions as I did last year. |
frankkilcommins
left a comment
There was a problem hiding this comment.
Thanks for the adjustments.
I spotted a few minor issues in the tests which if adjusted hopefully will result in them passing.
To pass the linting check rules, you will also have to modify your commit messages to follow conventional commits (e.g. feat(functions): add OR function)
| ).toEqual([]); | ||
| }); | ||
|
|
||
| it('given one property, should show no error message', async () => { |
There was a problem hiding this comment.
I believe this test will now return an error as Or expects minItems: 2
There was a problem hiding this comment.
I'm not sure how a validation error message would look in Core Functions testing, I've updated with validation error text and commented out for now.
| [ | ||
| new RulesetValidationError( | ||
| 'invalid-function-options', | ||
| '"or" requires one or more enumerated "properties", i.e. ["default", "example"], ["title", "summary", "description"], etc.', |
There was a problem hiding this comment.
This does not match what will now be returned by the JSON Schema:
'"or" requires at least two enumerated "properties", i.e. ["default", "example"], ["title", "summary", "description"], etc.'
| [ | ||
| new RulesetValidationError( | ||
| 'invalid-function-options', | ||
| '"or" requires one or more enumerated "properties", i.e. ["default", "example"], ["title", "summary", "description"], etc.', |
There was a problem hiding this comment.
This does not match what's now returned by the JSON Schema: '"or" requires at least two enumerated "properties", i.e. ["default", "example"], ["title", "summary", "description"], etc.'
| ).toEqual([]); | ||
| }); | ||
|
|
||
| it('given 1 property, should return no error message', async () => { |
There was a problem hiding this comment.
This test will now return an error based on minItems: 2
There was a problem hiding this comment.
Same comment as above, added validation error text and commented out.
| expect(await runXor([], opts)).toEqual([]); | ||
| }); | ||
|
|
||
| it.each([{ properties: ['foo'] }])('given valid %p options, should not throw', async opts => { |
There was a problem hiding this comment.
This will likely throw an error now due to minItems: 2 constraint.
There was a problem hiding this comment.
Correct, removed this test both here and in Or tests.
|
@cuttingclyde Would you be able to take a look at the tests - if not, we can potentially look at fixing the tests on our own. |
|
@mnaumanali94, thanks. I've updated to correct the comments, but am unfortunately still unable to run them locally. |
frankkilcommins
left a comment
There was a problem hiding this comment.
Thanks @cuttingclyde. Apologies, I missed two tests in my previous review. I've added comments for those now.
Also to pass the lint checks, you need to update all commits to follow conventional commits format. Is that something you can also take care of?
Thanks in advance.
| // ]); | ||
| // }); | ||
|
|
||
| it('given no properties, should show no error message', async () => { |
There was a problem hiding this comment.
This test also fails due to minItems constraint
| // ]); | ||
| // }); | ||
|
|
||
| it('given no properties, should return no error message', async () => { |
There was a problem hiding this comment.
this test also fails due to minItems constraint.
There was a problem hiding this comment.
Thanks, I should have caught those too. 😦
Looks like git would require a force push to change older, pushed messages of prior commits, I';m not familiar with and don't think I'm comfortable doing that. Oh, and it looks like I accidentally closed this PR when first adding this comment, I'll reopen. |
No problem. I can help you here if needed and update the commit messages on your fork.
😄 no problem at all. |
d191043 to
437a79a
Compare
Bumps [tar](https://github.com/isaacs/node-tar) from 6.1.15 to 6.2.1. - [Release notes](https://github.com/isaacs/node-tar/releases) - [Changelog](https://github.com/isaacs/node-tar/blob/main/CHANGELOG.md) - [Commits](isaacs/node-tar@v6.1.15...v6.2.1) --- updated-dependencies: - dependency-name: tar dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…tio#2594) Co-authored-by: Nauman <mnaumanali94@gmail.com>
Bumps [es5-ext](https://github.com/medikoo/es5-ext) from 0.10.53 to 0.10.64. - [Release notes](https://github.com/medikoo/es5-ext/releases) - [Changelog](https://github.com/medikoo/es5-ext/blob/main/CHANGELOG.md) - [Commits](medikoo/es5-ext@v0.10.53...v0.10.64) --- updated-dependencies: - dependency-name: es5-ext dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Nauman <mnaumanali94@gmail.com>
stoplightio#2581) * feat(rulesets): add oas3_1-servers-in-webhook and oas3_1-callbacks-in-webhook rules * feat(rulesets): fix oas3_1-servers-in-webhook and oas3_1-callbacks-in-webhook rules * feat(rulesets): fix lint in openapi-rules.md * feat(rulesets): check servers array at webhook operation level * feat(rulesets): show warning if callbacks defined in a callback --------- Co-authored-by: Nauman <mnaumanali94@gmail.com>
* docs(asyncapi): update asyncapi ruleset link. * docs(asyncapi): update asyncapi ruleset link --------- Co-authored-by: Nauman <mnaumanali94@gmail.com>
* feat(rulesets): improve {oas2,oas3}-valid-schema rule
* build(rulesets): try sth
* build(rulesets): abandon props mangling
* feat(rulesets): improve oas3.1 schema
* docs(repo): mention oas3-schema
* chore(rulesets): remove ajv-merge-patch dep
---------
Co-authored-by: Nauman <mnaumanali94@gmail.com>
… properties (stoplightio#2573) Required readOnly and writeOnly properties should not be considered required for respectively request and response bodies.
…n) (stoplightio#2615) Co-authored-by: Brenda Rearden <brendarearden@gmail.com>
* **cli:** update dependencies and trigger docker release ([c87eacf](stoplightio@c87eacf))
* **cli:** Trigger cli release ([stoplightio#2695](stoplightio#2695)) ([c48a929](stoplightio@c48a929)) * **cli:** trigger docker release ([920f7b5](stoplightio@920f7b5)) * **cli:** update dependencies and trigger docker release ([c87eacf](stoplightio@c87eacf)) * **core:** fix for TypeError "this.formats.has is not a function" ([stoplightio#2664](stoplightio#2664)) ([75d642d](stoplightio@75d642d)) * **core:** require new formats version ([stoplightio#2725](stoplightio#2725)) ([8ec328f](stoplightio@8ec328f)) * **deps:** fix package.json ([0161072](stoplightio@0161072)) * **formats:** update spectral core version ([6b196da](stoplightio@6b196da)) * **formatters:** update spectral core to latest version ([8a382f4](stoplightio@8a382f4)) * **functions:** update spectral core to latest ([ede60f3](stoplightio@ede60f3)) * **repo:** remove discord link and fix typo in github bug template ([stoplightio#2642](stoplightio#2642)) ([048924d](stoplightio@048924d)) * **repo:** update yarn lock ([362cdb4](stoplightio@362cdb4)) * **rulesets:** always allow string examples in asyncapi schema ([stoplightio#2625](stoplightio#2625)) ([4e2f797](stoplightio@4e2f797)) * **rulesets:** example validation for required readOnly and writeOnly properties ([stoplightio#2573](stoplightio#2573)) ([ae1fea5](stoplightio@ae1fea5)) * **rulesets:** fixed array-items type property selector ([stoplightio#2638](stoplightio#2638)) ([0845fb5](stoplightio@0845fb5)) * **rulesets:** remove step summary rule ([stoplightio#2692](stoplightio#2692)) ([d5a566f](stoplightio@d5a566f)) * **rulesets:** update spectral core to latest ([d74c2b0](stoplightio@d74c2b0)) * **rulesets:** use uri-reference for oauth security schemes ([stoplightio#2652](stoplightio#2652)) ([c411e63](stoplightio@c411e63)) * **cli:** require new stoplight dependencies ([stoplightio#2726](stoplightio#2726)) ([8a736b5](stoplightio@8a736b5)) * **cli:** trigger release ([87a90b3](stoplightio@87a90b3)) * **core:** trigger release ([b73d5e8](stoplightio@b73d5e8)) * **formats:** add arazzo format ([stoplightio#2663](stoplightio#2663)) ([dc1a8ef](stoplightio@dc1a8ef)) * **formatters:** add code climate (GitLab) formatter ([stoplightio#2648](stoplightio#2648)) ([41eca61](stoplightio@41eca61)) * **formatters:** add markdown formatter ([stoplightio#2662](stoplightio#2662)) ([b5edf5e](stoplightio@b5edf5e)) * **rulesets:** add AsyncAPI v3 support ([stoplightio#2697](stoplightio#2697)) ([3d69be7](stoplightio@3d69be7)) * **rulesets:** add multiple xor ([stoplightio#2614](stoplightio#2614)) ([af9c742](stoplightio@af9c742)) * **rulesets:** add new rule that requires sibling items field for type array ([stoplightio#2632](stoplightio#2632)) ([24198bc](stoplightio@24198bc)) * **rulesets:** add oas3_1-servers-in-webhook and oas3_1-callbacks-in… ([stoplightio#2581](stoplightio#2581)) ([7a8cc0e](stoplightio@7a8cc0e)) * **rulesets:** improve {oas2,oas3}-valid-schema rule ([stoplightio#2574](stoplightio#2574)) ([8df2c36](stoplightio@8df2c36)) * **rulesets:** initial rulesets for the Arazzo Specification ([stoplightio#2672](stoplightio#2672)) ([8443232](stoplightio@8443232))
* **cli:** choose proxy agent based on requester protocol ([stoplightio#2521](stoplightio#2521)) ([056f2e1](stoplightio@056f2e1)) * **cli:** clarify usage of --format ([stoplightio#2575](stoplightio#2575)) ([96eee89](stoplightio@96eee89)) * **cli:** Trigger cli release ([stoplightio#2695](stoplightio#2695)) ([c48a929](stoplightio@c48a929)) * **cli:** trigger docker release ([920f7b5](stoplightio@920f7b5)) * **cli:** update dependencies and trigger docker release ([c87eacf](stoplightio@c87eacf)) * **core:** fix for TypeError "this.formats.has is not a function" ([stoplightio#2664](stoplightio#2664)) ([75d642d](stoplightio@75d642d)) * **core:** pointer in overrides are applied too broadly ([stoplightio#2511](stoplightio#2511)) ([69403c1](stoplightio@69403c1)) * **core:** require new formats version ([stoplightio#2725](stoplightio#2725)) ([8ec328f](stoplightio@8ec328f)) * **deps:** fix package.json ([0161072](stoplightio@0161072)) * **formats:** update spectral core version ([6b196da](stoplightio@6b196da)) * **formatters:** update spectral core to latest version ([8a382f4](stoplightio@8a382f4)) * **functions:** update spectral core to latest ([ede60f3](stoplightio@ede60f3)) * **parsers:** update @stoplight/yaml from ~4.2.3 to ~4.3.0 ([91fdded](stoplightio@91fdded)) * **repo:** remove discord link and fix typo in github bug template ([stoplightio#2642](stoplightio#2642)) ([048924d](stoplightio@048924d)) * **repo:** update yarn lock ([362cdb4](stoplightio@362cdb4)) * **ruleset-migrator:** update @stoplight/json from ~3.20.1 to ~3.21.0 ([3f7eebc](stoplightio@3f7eebc)) * **rulesets:** always allow string examples in asyncapi schema ([stoplightio#2625](stoplightio#2625)) ([4e2f797](stoplightio@4e2f797)) * **rulesets:** example validation for required readOnly and writeOnly properties ([stoplightio#2573](stoplightio#2573)) ([ae1fea5](stoplightio@ae1fea5)) * **rulesets:** fixed array-items type property selector ([stoplightio#2638](stoplightio#2638)) ([0845fb5](stoplightio@0845fb5)) * **rulesets:** oasExample should clean id fields from non-schema objects ([stoplightio#2561](stoplightio#2561)) ([7f7583e](stoplightio@7f7583e)) * **rulesets:** remove step summary rule ([stoplightio#2692](stoplightio#2692)) ([d5a566f](stoplightio@d5a566f)) * **rulesets:** tweak server variables function ([stoplightio#2533](stoplightio#2533)) ([244cbda](stoplightio@244cbda)) * **rulesets:** update spectral core to latest ([d74c2b0](stoplightio@d74c2b0)) * **rulesets:** use uri-reference for oauth security schemes ([stoplightio#2652](stoplightio#2652)) ([c411e63](stoplightio@c411e63)) * **cli:** add sarif formatter ([stoplightio#2532](stoplightio#2532)) ([959a86a](stoplightio@959a86a)) * **cli:** require new stoplight dependencies ([stoplightio#2726](stoplightio#2726)) ([8a736b5](stoplightio@8a736b5)) * **cli:** require newer version of all Spectral dependencies ([10ddd97](stoplightio@10ddd97)) * **cli:** trigger release ([87a90b3](stoplightio@87a90b3)) * **cli:** use hpagent ([stoplightio#2513](stoplightio#2513)) ([9b2d347](stoplightio@9b2d347)) * **core:** trigger release ([b73d5e8](stoplightio@b73d5e8)) * **formats:** add arazzo format ([stoplightio#2663](stoplightio#2663)) ([dc1a8ef](stoplightio@dc1a8ef)) * **formats:** jsonSchemaLoose format should search for enum keyword ([stoplightio#2551](stoplightio#2551)) ([0835545](stoplightio@0835545)) * **formatters:** add code climate (GitLab) formatter ([stoplightio#2648](stoplightio#2648)) ([41eca61](stoplightio@41eca61)) * **formatters:** add GitHub Actions formatter ([stoplightio#2508](stoplightio#2508)) ([6904927](stoplightio@6904927)) * **formatters:** add markdown formatter ([stoplightio#2662](stoplightio#2662)) ([b5edf5e](stoplightio@b5edf5e)) * **formatters:** add sarif formatter ([stoplightio#2532](stoplightio#2532)) ([908c308](stoplightio@908c308)) * **rulesets:** add AsyncAPI v3 support ([stoplightio#2697](stoplightio#2697)) ([3d69be7](stoplightio@3d69be7)) * **rulesets:** add multiple xor ([stoplightio#2614](stoplightio#2614)) ([af9c742](stoplightio@af9c742)) * **rulesets:** add new rule that requires sibling items field for type array ([stoplightio#2632](stoplightio#2632)) ([24198bc](stoplightio@24198bc)) * **rulesets:** add oas3_1-servers-in-webhook and oas3_1-callbacks-in… ([stoplightio#2581](stoplightio#2581)) ([7a8cc0e](stoplightio@7a8cc0e)) * **rulesets:** add oas3-server-variables rule ([stoplightio#2526](stoplightio#2526)) ([4c4de85](stoplightio@4c4de85)) * **rulesets:** add scope validation to oas{2,3}-operation-security-defined rules ([stoplightio#2538](stoplightio#2538)) ([68aacd6](stoplightio@68aacd6)) * **rulesets:** improve {oas2,oas3}-valid-schema rule ([stoplightio#2574](stoplightio#2574)) ([8df2c36](stoplightio@8df2c36)) * **rulesets:** initial rulesets for the Arazzo Specification ([stoplightio#2672](stoplightio#2672)) ([8443232](stoplightio@8443232))
* **deps:** bump spectral-core dependents ([stoplightio#2742](stoplightio#2742)) ([30c0349](stoplightio@30c0349)) * **deps:** fix CVE related to jsonpath-plus ([169db7a](stoplightio@169db7a))
Bumps [cross-spawn](https://github.com/moxystudio/node-cross-spawn) from 6.0.5 to 6.0.6. - [Changelog](https://github.com/moxystudio/node-cross-spawn/blob/v6.0.6/CHANGELOG.md) - [Commits](moxystudio/node-cross-spawn@v6.0.5...v6.0.6) --- updated-dependencies: - dependency-name: cross-spawn dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* fix(deps): update dependencies * chore(repo): fix links * chore(repo): fix async api link
* **deps:** update dependencies ([stoplightio#2794](stoplightio#2794)) ([9e6b885](stoplightio@9e6b885))
* **core:** trigger release ([415dc76](stoplightio@415dc76)) * **deps:** bump spectral-core dependents ([stoplightio#2742](stoplightio#2742)) ([30c0349](stoplightio@30c0349)) * **deps:** trigger release ([7b6b74e](stoplightio@7b6b74e)) * **deps:** update dependencies ([stoplightio#2794](stoplightio#2794)) ([9e6b885](stoplightio@9e6b885))
* **core:** trigger release ([415dc76](stoplightio@415dc76)) * **deps:** trigger release ([7b6b74e](stoplightio@7b6b74e)) * **deps:** update dependencies ([stoplightio#2794](stoplightio#2794)) ([9e6b885](stoplightio@9e6b885))
* **core:** trigger release ([415dc76](stoplightio@415dc76)) * **deps:** bump spectral-core dependents ([stoplightio#2742](stoplightio#2742)) ([30c0349](stoplightio@30c0349)) * **deps:** fix CVE related to jsonpath-plus ([169db7a](stoplightio@169db7a)) * **deps:** trigger release ([7b6b74e](stoplightio@7b6b74e)) * **deps:** update dependencies ([stoplightio#2794](stoplightio#2794)) ([9e6b885](stoplightio@9e6b885))
* **core:** trigger release ([415dc76](stoplightio@415dc76)) * **deps:** bump spectral-core dependents ([stoplightio#2742](stoplightio#2742)) ([30c0349](stoplightio@30c0349)) * **deps:** fix CVE related to jsonpath-plus ([169db7a](stoplightio@169db7a)) * **deps:** trigger release ([7b6b74e](stoplightio@7b6b74e)) * **deps:** update dependencies ([stoplightio#2794](stoplightio#2794)) ([9e6b885](stoplightio@9e6b885))
* **core:** trigger release ([415dc76](stoplightio@415dc76)) * **deps:** trigger release ([7b6b74e](stoplightio@7b6b74e)) * **deps:** update dependencies ([stoplightio#2794](stoplightio#2794)) ([9e6b885](stoplightio@9e6b885))
* **deps:** update spectral-core in cli ([35687cd](stoplightio@35687cd))
Bumps [elliptic](https://github.com/indutny/elliptic) from 6.6.0 to 6.6.1. - [Commits](indutny/elliptic@v6.6.0...v6.6.1) --- updated-dependencies: - dependency-name: elliptic dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Nauman <mnaumanali94@gmail.com>
Co-authored-by: Nauman <mnaumanali94@gmail.com>
f37fcbd to
d6e1cbb
Compare
|
@cuttingclyde unfortunately I fat fingered one of the commit messages in the rebase and as usual a rebase on top of a rebase is not fun (especially as it's over ~200 commits). I'm not comfortable in resolving and merging this PR. I'll work on a new PR to supersede this today or tomorrow and attribute the changes to you. |
|
superseded by #2798 |
Replaces PR #2616 with addition of independent or function.
Fixes #2396 . This has now been merged with stoplightio:develop containing prior "xor" PR #2614.
Checklist
Does this PR introduce a breaking change?
(Just saw JeanArhancet's similar pull request, #2614 .)
FDX (see below) has extended XOR here to support from 1 to any number of properties. This would match the XOR logical function, that as long as exactly one of the (1 or more) properties is defined, then the rule passes. With just one property defined in the functionOptions, xor becomes redundant with defined function.
Updated error response messages to account for possibility of schema matching any number of properties or schema not matching any of any number of functionOption properties.
Updated functionOptions error messages to show example arrays containing from one to three ("and etc") properties.
Since then can take an array which implements AND-ing of rule requirements, this PR also defines and adds a new OR function. OR works just like xor for requiring at least one matching property, but then permits more than one matching property as valid and satisfying the rule.
Updated functions documentation for
xorchanges to allow more than two properties and neworfunction.Additional context
The Financial Data Exchange (FDX, https://financialdataexchange.org) industry consortium for Open Banking delivers 13 OpenAPI 3.1 files with nearly 17,000 lines, 47 paths, 70 operations and 374 defined schemas, We enforce our FDX API Style Guide using 48 custom spectral rules and 23 overrides over 870 ruleset lines. We wanted to enforce that
typewas defined for each schema, but a schema can get its type in 3 (or 4) ways:type,$ref, oroneOf. (We are not usingallOfto definetype.) This was ideal for an unresolved xor rule with propertiestype,$ref, oroneOf. We initially implemented this as a custom function which met our needs, so are now contributing this to the spectral community. Our rule using our customxorfunction:Secondly, there is an OR-rule opportunity to ensure that defined schemas contain sufficient documentation, as a style guide requirement. In particular, each schema ought to have its use / purpose described for users, which can be done in any of
title,summary, ordescriptionfields. This is an ideal place for anorfunction, since any one of those can provide the required schema documentation text, but two or three of them is also just as valid (or better!). A similar example (also included in this PR's tests), would be requiring a helpful example for atype: stringschema, which could be provided by any offormat,example, orpatternfields in the API spec, but again more than one is acceptable.