Nested Discriminators

[mmmasent]

Yardarm does not correctly parse discriminators for a nest set of abstracts:

AbstractA
ImplementationB: AbstractA

AbstractC: AbstractA
ImplementationD: AbstractC

Yardarm will currently parse that as:

IA
ImpelemntationB: IA

C: IA
ImplementationD: C

Since C is not cast to an interface, it will not get the discriminator attribute to map subsets of C. Instead, only the discriminator for AbstractA is created and parsing any further than the top level abstract level is difficult.

[brantburnett]

Can you attach an example OpenAPI schema to this issue that produces this behavior? Thanks.

[mmmasent]

@brantburnett

        "AFakeAbstract": {
            "type": "object",
            "required": [
                "fakeType"
            ],
            "allOf": [
                {
                    "$ref": "#/components/schemas/BillingSchedule"
                }
            ],
            "properties": {
                "fakeType": {
                    "$ref": "#/components/schemas/FakeEnumType"
                },
                "fakePropertyA": {
                    "type": "integer",
                    "format": "int32"
                }
            },
            "oneOf": [
                {
                    "$ref": "#/components/schemas/FakeAImplementation"
                },
                {
                    "$ref": "#/components/schemas/FakeBImplementation"
                }
            ],
            "discriminator": {
                "propertyName": "type",
                "mapping": {
                    "FakeA": "#/components/schemas/FakeAImplementation",
                    "FakeB": "#/components/schemas/FakeBImplementation"
                }
            }
        }

In the above example, the Fake Abstract inherits, through the allOf, the parent BillingSchedule, but is also a base class, through oneOf

Just to hopefully better clarify the issue:

Billing Schedule is an abstract class that only has the one of tag and therefore yardarm generates it as an interface with discriminator:

AFakeAbstract is an abstract class that inherits from an abstract parent. Yardarm will generate this as a normal class without any discriminator to parse the concrete classes:

AFakeAbstract should instead be an interface, inheriting the parent interface, with it's own subset of discriminator values, at least from how Yardarm currently generates these types of classes.

[brantburnett]

@mmmasent

I apologize, but I'm still having a hard time following this scenario. Can you perhaps provide a more complete real-world scenario with the full YAML or JSON? From what I can tell, you're describing a case where a single JSON object would actually have two different discriminators on it, which seems unlikely to me. If each one had 3 variants, that would make 9 different concrete implementations.

Even if that is a real-world scenario, I'm not sure that it's one that maps to C# well at all and may not be supportable, except maybe via JsonExtensionData. But I could just not be understanding it. I believe part of my confusion may be that your explanation talks a lot about things in the schema being "abstract", when it's really the C# implementations that are abstract to try our best to map to the high flexibility of the schema. I think we need to focus on what the schema is describing in JSON first, then decide how best to implement that in C# (if it's even possible).

[mmmasent]

This is an issue with having a oneOf and allOf tag on the same object. There are real world cases for multiple inheritance; for hopefully a little more clarity, imagine an api spec that creates animal objects based on certain requirements and returns the base class for a GET List endpoint:

In the above example, Yardarm fails to parse the second level of base classes. It will create the super class Animal with discriminator s for Mammal, Bird, Fish, but will not parse those classes with discriminators so the generated client will not be able to discriminate the 3rd level of classes.

In this example, Animal will only have the oneOf tag, whereas the second level (Mammal, Bird, Fish) will have both oneOf and allOf tags since they both inherit and are base classes themselves. The final level will only have allOf tags since they do not have derived classes.

An Example of the schema for the second level was provided above. Yardarm will currently only check for the existence of one of the tags, but not both. It seems like a viable solution might simply use the current one of and all of generators and combine them into a single generator.

In my opinion, this seems like a viable real world scenario that we would want to support as there can be more or less complex multi inheritance trees as well.

It should be noted that this issue is closely related to #18, but this deals solely of parsing discriminated class objects whereas the other is solely about property generation issues on derived classes. Whatever solution is used for this should not break the fix for the other issue and vice versa.

[mmmasent]

To further provide a more complete schema set for an example scenario, I'm providing a slightly modified json schema from one of the commits in a related PR:

        "BillingScheduleType": {
            "enum": [
                "FakeAbstract",
            ],
            "type": "string"
        },
        "BillingSchedule": {
            "required": [
                "type"
            ],
            "type": "object",
            "properties": {
                "type": {
                    "$ref": "#/components/schemas/BillingScheduleType"
                }
            },
            "oneOf": [
                {
                    "$ref": "#/components/schemas/AFakeAbstract"
                }
            ],
            "discriminator": {
                "propertyName": "type",
                "mapping": {
                    "FakeAbstract": "#/components/schemas/AFakeAbstract"
                }
            }
        },
        "FakeEnumType": {
            "enum": [
                "FakeA",
                "FakeB"
            ],
            "type": "string"
        },
        "AFakeAbstract": {
            "type": "object",
            "required": [
                "fakeType"
            ],
            "allOf": [
                {
                    "$ref": "#/components/schemas/BillingSchedule"
                }
            ],
            "properties": {
                "fakeType": {
                    "$ref": "#/components/schemas/FakeEnumType"
                },
                "fakePropertyA": {
                    "type": "integer",
                    "format": "int32"
                }
            },
            "oneOf": [
                {
                    "$ref": "#/components/schemas/FakeAImplementation"
                },
                {
                    "$ref": "#/components/schemas/FakeBImplementation"
                }
            ],
            "discriminator": {
                "propertyName": "type",
                "mapping": {
                    "FakeA": "#/components/schemas/FakeAImplementation",
                    "FakeB": "#/components/schemas/FakeBImplementation"
                }
            }
        },
        "FakeAImplementation": {
            "allOf": [
                {
                    "$ref": "#/components/schemas/AFakeAbstract"
                },
                {
                    "required": [
                        "happyDays",
                        "type"
                    ],
                    "type": "object",
                    "properties": {
                        "happyDays": {
                            "type": "integer",
                            "format": "int32"
                        }
                    }
                }
            ]
        },
        "FakeBImplementation": {
            "allOf": [
                {
                    "$ref": "#/components/schemas/AFakeAbstract"
                },
                {
                    "required": [
                        "sadDays",
                        "type"
                    ],
                    "type": "object",
                    "properties": {
                        "sadDays": {
                            "type": "string"
                        }
                    }
                }
            ]
        }

Note: Slightly modified and not tested against yardarm. But for a full example schema file, you can check here: https://github.com/CenterEdge/Yardarm/blob/120b8fbb524332f79ce7b19b9b942229d05365f2/src/Yardarm.CommandLine/inheritanceOnlyTestSpec.json

[brantburnett]

@mmmasent I'm not sure your example is a real-world example, though. It certainly might be, in which case we should consider it. In my mind, if I were making a schema for this scenario the discriminator would only be on one type, not two. I'd write the various models and their abstract parents without any discriminators, just inheritance. Then I'd make a single type with a discriminator and oneOf array that referenced all the different flavors allowed.

Do you have a real-world API example?