Objects with multiple levels of inheritance specified by different discriminators do not deserialize correctly

[daniel-wise-ansys]

What are you generating using Kiota, clients or plugins?

API Client/SDK

In what context or format are you using Kiota?

Nuget tool

Client library/SDK language

Csharp

Describe the bug

Deserializing an object that has multiple levels of inheritance, when each level uses a different property as the discriminator, does not produce an object of the correct type. Instead it produces an object of the first derived type from the base type.

E.g. with the pseudo-structure:

BaseType
   discriminator: intermediateDiscriminator
   mappings: 
      intermediateA: IntermediateTypeA

IntermediateTypeA
   discriminator: derivedDiscriminator
   mappings: 
      derivedA: DerivedTypeA

DerivedTypeA

a serialized object with properties intermediateDiscriminator: intermediateA and derivedDiscriminator: derivedA should deserialize as a DerivedTypeA, but instead produces an IntermediateTypeA.

See the attached spec and minimal reproduction steps for a more concrete example.

This is somewhat related to https://github.com/microsoft/kiota/issues/2432 but the problems mentioned in that case seem to be because the same discriminator property is used for different types, whereas here we are using different properties for the nested types.

Looking at the generated code, it looks like CreateFromDiscriminatorValue on the base class should call CreateFromDiscriminatorValue on its derived classes instead of calling the constructors directly, so that they chain all the way to the most derived class.

Expected behavior

The object should deserialize as the derived type as specified by the discriminators.

How to reproduce

Have an API with a POST endpoint for /test/testPost, that returns the following JSON: {"type":"anotherString", "derivedType":"typeA"}

Using kiota, generate C# code from the attached spec.

Call the TestPost endpoint from the generated code. This will return a TestClassWithAnotherString instead of the expected TestClassAfterStringA.

Open API description file

MultiLevelInheritance_MRE.json

Kiota Version

1.15.0+b535a94064cd8c14a022aaba42964467d5db525a

Latest Kiota version known to work for scenario above?(Not required)

No response

Known Workarounds

No response

Configuration

No response

Debug output

Click to expand log

``` ```

Other information

No response

[baywet]

Hi @daniel-wise-ansys Thanks for using kiota and for reaching out. I believe this is a duplicate of #2432 as you rightly identified. I'm curious to why you chose to open a new issue in this case?

[daniel-wise-ansys]

My impression is that the issues are related, but not the same. If every level of inheritance uses different discriminator properties then each base type would know about all its directly derived types, which would each know about their own derived types and so on, so there would be no need to search the entire tree and cause the performance issues mentioned in #2432. So my hope was that this could be fixed separately/more easily. Although I see in your latest comments you mention the possibility of circular dependencies, so perhaps it is not as simple as I hoped.

[baywet]

Thanks for pointing this out, I didn't catch the fact that your model used different properties for the different levels. This is the first time I come across such a design. Can you expand on the reasons for such a design please? (as opposed to using the same property for every level)

Yes, depending on how the schemas are distributed (same namespace or not), how the mappings are defined, how the type resolution works for any given language, doing everything recursively is more likely to end up with circular reference. (the current model actually can result in such issues today in Go, but it's less likely with the 1 level limitation)

[daniel-wise-ansys]

I believe the design is that way because we have situations where we need to resolve multiple levels of inheritance e.g. we might have an endpoint that can return an object of a base type and the client needs to be able to determine which deeply derived type it is. And that's difficult to do without multiple properties, at least without running into the kinds of performance problems described in #2432. As for why the data model is like that, I think it pre-dates the API.

[baywet]

Thanks for the additional information here. Since recursive discrimination is not supported on a single property, I don't think we could support recursive discrimination over multiple properties today. As for supporting discrimination on multiple properties for a single inheritance level, I'm not sure it provides value/is intended by OAS.

Regardless, even assuming recursive discrimination was supported on a single property, I'm not sure how we'd evolve the design to account for multiple properties. Do you have any suggestion here?

[daniel-wise-ansys]

Admittedly I have only looked at the generated code for C#, so this might be more difficult in the general case, but it very nearly works already. For our case, we have tried changing the CreateFromDiscriminatorValue methods from being like:

public static BaseClass CreateFromDiscriminatorValue(IParseNode parseNode)
{
    _ = parseNode ?? throw new ArgumentNullException(nameof(parseNode));
    var mappingValue = parseNode.GetChildNode("type")?.GetStringValue();
    return mappingValue switch
    {
        "foo" => new Foo(),
        "bar" => new Bar(),
        _ => new BaseClass(),
    };
}

to

public static BaseClass CreateFromDiscriminatorValue(IParseNode parseNode)
{
    _ = parseNode ?? throw new ArgumentNullException(nameof(parseNode));
    var mappingValue = parseNode.GetChildNode("type")?.GetStringValue();
    return mappingValue switch
    {
        "foo" => Foo.CreateFromDiscriminatorValue(parseNode),
        "bar" => Bar.CreateFromDiscriminatorValue(parseNode),
        _ => new BaseClass(),
    };
}

and that was the only change required to get it to work.

That does rely on the fact that we have no circular references, but keeping track of what types have already been seen and breaking out of the recursion if you return to one doesn't feel too hard.

[baywet]

Thanks for exploring this further. So we're essentially saying that if we enable recursive discrimination, this scenario (multiple discriminator properties) would be enabled as well? Let us then jump on the other issue and figure out a design that's o(1) not o(n) if possible and non-breaking to see if we can make progress on this topic.