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

[Bug/ enhancement] oas-schema-check , valid schema is set as invalid , when leveraging required + allOf #468

Open
LasneF opened this issue Mar 4, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@LasneF
Copy link

LasneF commented Mar 4, 2024

given this object , vaccum raise an error about pom not defined

.\component-descriptor.yml:2617:7 | error | required field pom is not defined in properties | oas-schema-check

accoding to JSonSchema the scenario is valid , even if to me looks bad design

still Vacuum need to clearly differenciate was is invalid from structural point of view vs what is opinonated rules

=> not sure if it is a bug in the json schema structure validator, if it is a design , and opinionated rules

validated against https://github.com/hyperjump-io/json-schema & https://json-everything.net/json-schema/

{
    "type": "object",
    "required": [
        "tata" , "pom"
    ],
    "properties": {
        "tata": {
            "type": "number"
        }
    },
    "allOf": [
        {
            "type": "object",
            "properties": {
                "pom": {
                    "type": "string"
                }
            }
        }
    ]
}
@daveshanley
Copy link
Owner

This is an upgrade to the rule, it's not a bug - this rule is manual, and it's not checking that deep I don't think when performing a required check.

@daveshanley daveshanley added the enhancement New feature or request label Mar 4, 2024
@LasneF
Copy link
Author

LasneF commented Mar 5, 2024

notice that to me the rules is accurate and to me (opinionned but not factual) it s a good practice , and to me this looks bad design. but that can be not the case for others

as it is a valid pattern ; it should be a dedidated rules.

=> Need to put in place a sharp issue naming convention : each case should have its own unique identifier
should be easy to put in place straight

=> given the fact that we have unique identifier for each case , adding a tag mechanism could be usefull like here schema , good practice etc , that you can display to categorize the issue . can be then leverage in a ui for filtering / sorting purpose

nb: do not change the rules just tag it as good practice , and not

i would propose naming : required-properties-missing , tag schema, good practice , and proper documentation show casing what is good , what is definitly wrong , was is passable like the use case propose

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

No branches or pull requests

2 participants