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

feat(functions): expand xor function and add exclusive option for or functionality #2616

Closed

Conversation

cuttingclyde
Copy link

@cuttingclyde cuttingclyde commented May 2, 2024

Fixes #2396 . This has now been merged with stoplightio:develop containing prior "xor" PR #2614.

Checklist

  • Tests added / updated
  • Docs added / updated

Does this PR introduce a breaking change?

  • Yes
  • No

(Just saw JeanArhancet's similar pull request, feat(rulesets): add multiple xor .)

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 an exclusive (default true) option to XOR function to provide OR behavior. exclusive: false 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 xor changes to allow more than two properties and new exclusive option to permit multiple matches.

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 type was defined for each schema, but a schema can get its type in 3 (or 4) ways: type, $ref, or oneOf . (We are not using allOf to define type.) This was ideal for an unresolved xor rule with properties type, $ref, or oneOf . 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 custom xor function:

  fdx-property-type-defined:
    description: A valid type must be set for each property (fdx-schema-type ensures oneOf and $ref entities have type defined)
    message: '{{property}} property has zero or multiple defined types from: "type", "oneOf", "$ref"'
    severity: error
    given:
      - $..properties[*]
    # Do not resolve here so $ref can be used as xor function option
    resolved: false
    then:
      # This uses custom fdx_xor function which enforces XOR for any number of properties ([TODO] spectral xor only allows 2 values)
      function: fdx_xor
      functionOptions:
        properties:
          - type
          - oneOf
          - $ref

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, or description fields. This is an ideal place for an or function, 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 a type: string schema, which could be provided by any of format, example, or pattern fields in the API spec, but again more than one is acceptable.

@cuttingclyde cuttingclyde requested review from a team as code owners May 2, 2024 20:34
@cuttingclyde cuttingclyde requested a review from chohmann May 2, 2024 20:34
@cuttingclyde cuttingclyde changed the title Expand xor function and add or function feat(functions): Expand xor function and add or function May 2, 2024
@cuttingclyde cuttingclyde changed the title feat(functions): Expand xor function and add or function feat(functions): expand xor function and add or function May 3, 2024
@cuttingclyde
Copy link
Author

cuttingclyde commented May 3, 2024

This has now been merged with stoplightio:develop containing prior "xor" PR #2614, "feat(rulesets): add multiple xor", and rest of spectral develop branch.
I've fixed the CircleCI linting issue on PR title, it remains on commit titles.

@cuttingclyde cuttingclyde changed the title feat(functions): expand xor function and add or function feat(functions): expand xor function and add exclusive option for or functionality May 4, 2024
cuttingclyde and others added 3 commits May 20, 2024 11:43
Define exclusive as optional.

Co-authored-by: Brenda Rearden <[email protected]>
Safely handle any type of exclusive argument, not just boolean or null values.

Co-authored-by: Brenda Rearden <[email protected]>
@mnaumanali94 mnaumanali94 requested review from frankkilcommins and removed request for chohmann November 15, 2024 14:44
@frankkilcommins
Copy link
Contributor

Hi @cuttingclyde,

I appreciate that this started as an expand of XOR functionality and evolved as #2614 was merged.

XOR - as in exclusive or has an expected behaviour within programming languages. The remaining part of this proposal adds a exclusive boolean property (defaulting to true) which if set to false introduces non-exclusive behaviour so that the function can work as OR type behaviour.

It would be cleaner, more maintainable, and more usable to introduce a new or core function which offers this behaviour rather than misusing XOR and effectively negating the 'X' part.

Could you rework this and instead add a new core OR function?

@cuttingclyde
Copy link
Author

cuttingclyde commented Dec 17, 2024 via email

@cuttingclyde
Copy link
Author

cuttingclyde commented Dec 17, 2024 via email

@cuttingclyde
Copy link
Author

cuttingclyde commented Dec 21, 2024

@frankkilcommins , Created new PR #2765 from new branch, containing code reverted back to the state where OR function was added and both OR and XOR can take any unlimited number of properties.

I will close this PR with this comment.

This branch can also now be deleted, but I will leave it for now in case @frankkilcommins wants to compare it with the new branch for PR 2765.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support more than 2 logicals in XOR core function
4 participants