add discriminator property support

[eli-bl]

This more fully fixes #219, and supersedes #717.

Any valid discriminator definition as described in OpenAPI 3.1.0 should work correctly with these changes, including:

• Discriminators with a mapping that specifies all valid discriminator values.
• Discriminators with no mapping, in which the discriminator value for each type is the simple name of the type (equivalent to {"Model1": {"$ref": "#/components/schemas/Model1"}}).
• Discriminators with a mapping that specifies values for some classes, but not all of them; in this case the default for the unspecified classes is the same as if there was no mapping.
• Unions of unions where each of the sub-unions has its own discriminator mapping.

Validation during parsing is not as thorough as it could be, since currently the property lists inside ModelProperty are not available at the time the UnionProperty is parsed (I do have some further changes in mind to improve this, but I wanted to keep the scope of this PR manageable). But there is some basic validation, like "you can't have a non-object schema, or an inline schema, if there's a discriminator"... so this is a breaking change in the sense that generation will now fail on some invalid specs that previously would have passed.

[eli-bl]

This is necessary in order for the discriminator in the spec to really be valid: a discriminator property must be a required property in all of the variant schemas. My current implementation wouldn't actually catch a mistake like this, but I figured it was best to have the test spec be valid.

[eli-bl]

At least, that's my reading of what OpenAPI says. I'm basing it on the statement "The expectation now is that a property with name petType MUST be present in the response payload, and the value will correspond to the name of a schema defined in the OAS document."

However, this is a case (one of many) of the OpenAPI specification using a mix of normative and non-normative language. The MUST seems unambiguous, but it's in a paragraph that's describing this specific example with types of pets, where the schemas do say the property is required. So do they mean a discriminator property must always be required, or just that that's the behavior this example is illustrating?

Logically I feel like it makes sense for it to be required, and if it isn't, then I'm not sure what the client behavior should be: (a) refuse to parse the object if the property is missing, or (b) just fall back to "try parsing it as each of these types until one of them works"? What I've implemented so far in the generated code is (a).

[eli-bl]

Broke this out into a separate helper below, since it doesn't rely on any closure variables.

[eli-bl]

Here's a case where I'm not 100% sure what the behavior should be:

components:
  schemas:
    ModelA:
      type: object
      properties:
        modelType:
          type: string
    ModelB:
      type: object
      properties:
        modelType:
          type: string
    NullType:
      type: null
    NullableUnionType:
      oneOf:
        - $ref: "#/components/schemas/ModelA"
        - $ref: "#/components/schemas/ModelB"
        - $ref: "#/components/schemas/NullType"
      discriminator:
        property: modelType

Should this be considered valid? In my current implementation, it is not valid, because that's my literal reading of this statement from the spec: "The expectation now is that a property with name [the discriminator property name] MUST be present in the response payload." If the value is null, then it does not have a modelType property.

However, like many things in OpenAPI, the spec language doesn't really say "if you do this, it's a malformed spec"; it's just a thing that would not have a well-defined behavior if you were writing a validator or a client generator. When someone raised a question about this scenario in the OpenAPI spec repo, the official answer was basically that. They just advised people that instead of writing their specs that way, to avoid ambiguity they should do it like this:

    NullableUnionType:
      oneOf:
        - $ref: "#/components/schemas/NullType"
        - oneOf:
            - $ref: "#/components/schemas/ModelA"
            - $ref: "#/components/schemas/ModelB"
          discriminator:
            property: modelType

My implementation does support that nested approach. What I'm wondering is whether it would make sense to loosen it so it also supports the questionably-valid shorter version, i.e. have a specific loophole for letting one of the variants be null, simply because I'm guessing it's not uncommon for people to write specs that way.

And if so, should it also allow an inline - type: null instead of a named NullType? The OpenAPI language says that "when using the discriminator, inline specs will not be considered"... but in this example, it arguably would not be "using" the discriminator in a case where the deserialization logic detected a None value and returned early.

[eli-bl]

Putting this back to draft because I have a couple ideas to simplify/clarify things.

[eli-bl]

See this comment: https://github.com/openapi-generators/openapi-python-client/pull/1154/files#diff-91d61e88486ae43dc0579d44f7964667c53749e7091907d6a064d80b57a4b26dR145

[eli-bl]

OK, I pushed some more changes to simplify things a bit.

There was some extra validation that I could add in a follow-up PR, which I don't think is essential to the feature. I was also thinking about adding a new category of end-to-end tests that verify the behavior of the generated code (importing a generated model class and calling from_dict on it, etc.), but I could do that as a follow-up too and it's not specific to this feature.

[eli-bl]

As described in comments in union.py, there can be multiple discriminators if there was a nested union. The logic here tries each discriminator property in order; if that property exists, it checks the value, and if it finds a matching class, it uses that class. If no matches were found for any discriminator then it raises an error.