microsoft / typespec

https://typespec.io/
MIT License
4.12k stars 198 forks source link

Mulit-layer inheritance with single discriminator #4291

Open ArcturusZhang opened 2 weeks ago

ArcturusZhang commented 2 weeks ago

There is a scenario in swagger world that we could have the types as following:

@discriminator("kind")
model Base {
    kind: string;
}

model A extends Base {
    kind: "A";
}

model B extends A {
    kind: "B";
}

but the typespec now does not support this yet because of the type constraint in kind in A.

Since it is a goal for our generator, we still need to have the ability to support swagger input, in a way that we will write a tool or something to convert the swagger into tspCodeModel.json and then let our generator to consume it, therefore our input types should still be able to represent this scenario that typespec currently does not support.

As for the generated code of this, one proposal is like the following:

public abstract class Base {
    protected Base(string kind) {} // or private protected

    internal string Kind {get;}
}

class A : Base {
    public A() : base("A") {}

    private protected A(string kind) : base(kind) {}

    internal A(string kind, int x) : base(kind) {} // this has to have the `kind` otherwise B does not have a way to pass in its value of kind when deserialization
}

class B : A {
    public B() : base("B") {}

    internal B(int x, int y) : base("B", x) {}
}

This solution is:

  1. add a protected ctor in A, and it is only for B to call
  2. add a kind parameter in A's internal full ctor to accept a kind (or an extra internal ctor just like we have the protected)

The other alternative solution is that we could do the autorest.csharp way: making the discriminator property internally settable

public abstract class Base {
    protected Base() {} // or private protected

    internal string Kind {get; set;}
}

class A : Base {
    public A() : base()
    {
        Kind = "A";
    }

    internal A(string kind, int x) : base(kind) {} // this has to have the `kind` otherwise B does not have a way to pass in its value of kind when deserialization
}

class B : A {
    public B() : base()
    {
        Kind = "B";
    }

    internal B(int x, int y) : base("B", x) {}
}
ArcturusZhang commented 2 weeks ago

Since typespec does not support this yet, we cannot have a cadl-ranch case for this, but for us, we could always construct the input types for this scenario and get the generated code, and maybe verify the serialization and deserialization to validate they are working properly which should be close enough for what a cadl-ranch test case could validate.