Add reproducer for #20

[stephenfin]

Add a reproducer for #20. I was hoping to actually fix this issue in this PR, but I suspect fixing this will require more extensive changes than originally planned so I only add tests for now.

In addition to the reproducer for #20, I also add some other tests related to the nullable type, fix some typos in the README, and add a simple tox.ini file.

PS: As a long-term fix for #20, I'm thinking that we could remove the include_nullable_validator function and instead do an initial one time scan of the schema, setting nullable = False where it makes sense (i.e. wherever a sibling attribute wouldn't override it). To be able to do this though, we'd also need to do up front resolution of JSON references. I'm looking into this approach.

Commits:

• Add tox file
• README: Correct some typos
• tests: Add tests for nullable object type
• tests: Add test for nullable refs

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@0ab91b1). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #55   +/-   ##
=========================================
  Coverage          ?   78.78%           
=========================================
  Files             ?        6           
  Lines             ?      264           
  Branches          ?       41           
=========================================
  Hits              ?      208           
  Misses            ?       46           
  Partials          ?       10           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[p1c2u]

This subschema is invalid (not nullable). That means the whole allof should be invalid isn't it?

[stephenfin]

As above.

[p1c2u]

This subschema is invalid (not nullable). That means the whole allof should be invalid isn't it?

[stephenfin]

Is it? I'd have expected the reference this to flatten to:

{
    "nullable": true,
    "allOf": [
        {
            "required": ["id", "name"],
            "properties": {
                "id": {"type": "integer", "format": "int64"},
                "name": {"type": "string"},
                "tag": {"type": "string"}
            }
        }
    ]
}

which would in-turn flatten to:

{
    "nullable": true,
    "required": ["id", "name"],
    "properties": {
        "id": {"type": "integer", "format": "int64"},
        "name": {"type": "string"},
        "tag": {"type": "string"}
    }
}

Is my understanding incorrect? It might well be! 😄

[stephenfin]

Ahh, wait, I see your top-level comment now.

allOf can not be used to “extend” a schema to add more details to it in the sense of object-oriented inheritance. Instances must independently be valid against “all of” the schemas in the allOf. See the section on Extending Closed Schemas for more information.

json-schema.org/understanding-json-schema/reference/combining.html#allof

So this being the case, I wonder if the schemas should be outright rejected as invalid?

[p1c2u]

It looks like the case of the following:

allOf can not be used to “extend” a schema to add more details to it in the sense of object-oriented inheritance. Instances must independently be valid against “all of” the schemas in the allOf. See the section on Extending Closed Schemas for more information.

https://json-schema.org/understanding-json-schema/reference/combining.html#allof

[stephenfin]

Sorry for hugely delayed response here. I forgot about this 🙈

[stephenfin]

I think I have mentioned it somewhere in here, but this thread is worth a read OAI/OpenAPI-Specification#1368. Specifically, this comment suggests what I'm doing here as a workaround, and it seems from #20 that many people were expecting this workaround to work here as it did previously. I guess it would be good to either (a) start supporting that workaround again or (b) formally reject this workaround and insist users upgrade to OpenAPI 3.1 which works around this issue.

fwiw, I just upgraded the project I was seeing this issue on to OpenAPI 3.1 and it was a relatively smooth process.