oneOf support in C# generator

[cpjolly]

oneOf is not currently supported in SwaggerToCSharpClientGenerator. See this issue for full explanation and example

[RicoSuter]

Scenarios to consider:

• oneOf to specify a union type (as described in this issue)
• oneOf to specify inheritance, see Support oneOf inheritance for code generators #13

[adamjones1]

In the inheritance scenario described in #13 this is really just using inheritance to model a union type so I don't think we have two separate cases here. I would suggest then modelling oneOf as a union type in the same way as F# does when consumed from C#: as an abstract base class with a private constructor and nested sub classes each with a public constructor and a property of the case type. This would cover both cases as one in a nice way (to traverse as a union type, pattern matching can be used). Any extra properties on the oneOf type could be made base class members and any 'required' properties (common to all cases) could be base class members implemented to delegate to their case's property. I'd love to see support for this released.

[WarrenFerrell]

@RicoSuter I believe this should be closed as a duplicate of RicoSuter/NSwag#2313 ?

[RicoSuter]

@cpjolly can we close?

[cpjolly]

@RicoSuter - yes, I agree this is a duplicate and I'll close it.

[adamjones1]

Can someone explain to me how 2313 duplicates this? Is the idea that if we want to use oneOf, instead we can use non-required fields corresponding to each case? If so I don't think I agree this should be closed - firstly the fields correspond to an anyOf schema, not a oneOf schema; secondly, what if the OpenAPI spec being consumed is beyond your control and uses oneOf?

[WarrenFerrell]

@RicoSuter I actually agree with @adamjones1 . I This shouldn't be considered a duplicate. I personally had nullable problems because I couldn't get generateOptionalPropertiesAsNullable to work with but for some reason I conflated that problem with the oneOf problem that I had to solve. FWIW I implemented this manually as a generic for each oneOf option and squashed the serialization using the AdditionalProperties dictionary and some cached reflection but this was only because I didn't want to write rewrite out all the properties because I am consuming another party's openapi definition for my client.

PS FWIW This issue is a duplicate of
#13 , #673 ,
#839 ,

and related to
#302
#378

Might be nice to consolidate some of these issues.

[cpjolly]

@RicoSuter , @adamjones1, @WarrenFerrell - I was wrong... this isn't a duplicate so I'm reopening. Hope thats what you guys want... Feel free to merge, close, etc as you see fit...

[RicoSuter]

Yeah, i was also wondering because oneOf is not really supported.. lets keep this open

[WarrenFerrell]

Thoughts on referencing https://github.com/mcintyre321/OneOf and using that to add oneOf support? It would make it extremely easy.

[adamjones1]

I've used this library in another project before and can confirm it works well for the problem it aims to solve. I also think what it represents (an untagged union type) is a better model for oneOf payloads than my suggestion above on 23rd June 19, which models a tagged union type.

I'd be very wary of the cost of forcing a new package dependency on all clients consuming a oneOf schema though. We can just about get away with it with Json.Net but new ones should be added only when absolutely needed IMO. They can't be versioned by NSwag, adding risk when breaking API changes or even malignant changes are made. They add another point of configuration (and thus another point of failure) with NSwag clients, requiring them to add the reference to their project file/Paket files, which itself may involve gaining permission/passing vetting if their NuGet feeds are subject to a corporate whitelist. Given how little used this library is, permission might be hard to get for these clients. Given the library is apparently a one-man effort, it might not be wise to rely on its future anyway.

There's also the issue that OneOf<...> as implemented in the library isn't extendable with new properties, which is good for its typical use case, but in this situation we may want common required properties on a oneOf schema to translate to properties directly on the oneOf type rather than its child types.

In short I think I would be in favour of representing oneOf schemas in C# with types which use the approach to modelling unions in the way that the OneOf library does, but I don't think it itself should be referenced. It would be better to keep NSwag self-contained and slim (we don't need all the features of OneOf) and generate the classes (which are trivial, especially with a code generator) as part of the client instead. I'd also note I don't think it's necessary (or perhaps, even desirable) to fully replicate all the public properties it offers on OneOf<...> - just Match and Switch would be enough for me, and would keep the type run-time safe.

[adamjones1]

I had a look at doing the above in my previous comment this weekend - submitted in PR #1142. It's not fully tested yet but I thought by this point I should try and get some feedback from you.

The approach is that when encountering a oneOf schema with no name it will generate an 'anonymous' OneOf<,> class (with the suitable arity) and a OneOfBase<,> base class. So you'll get something like

public OneOf<string, int> MyPropertyWhichIsEitherAStringOrAnInt { get; }

Thus non-named "one-ofs" are considered equivalent structurally, ie. whether they have the same case and value for that case.

If a oneOf schema does have a name it will generate a class like

public class MyNamedOneOf : OneOfBase<string, int> { ... }

and be referenced like

public MyNamedOneOf MyPropertyWhichIsEitherAStringOrAnInt { get; }

So named "one-ofs" are considered equivalent nominally. It seemed worth having support for both rather than generating names for the anonymous ones, since they make it immediately clear what their structure is just from the type signature and they save code generated.

I only added support for C# because I'm not too familiar with TypeScript, though my understanding is that TS supports untagged union types natively so it should be quite trivial to add them there for someone in the know.

Also, note that it currently is quite restrictive with the scope of what it considers a union type: there must be no properties or additional properties, etc. I thought this would be appropriate for a first pass, but it should be clear how to extend it later if wanted - a named OneOf should be generated with the extra stuff added on.

[keels4444]

Hi Adam,

With your fix, would the following schema definition be supported?

"nena-lgt:LogEventBodyType": {
"properties": {
"OneOf": [
{
"$ref": "#/components/schemas/nena-lgt%3AAdditionalAgencyBodyType"
},
{
"$ref": "#/components/schemas/nena-lgt%3AAdditionalDataQueryBodyType"
}
]
},
"type": "object"
}

Thanks,
Ryan

[adamjones1]

@keels4444 I haven't tested that, but I believe it should do. The criteria which it tests for a schema to be considered a union type is:

bool IsUnionType => 
	Properties.Count == 0 &&
	!AllowAdditionalProperties &&
	(Items == null || !Items.Any()) &&
	AdditionalItemsSchema == null &&
	!IsEnumeration &&
	Reference == null &&
	Not == null &&
	AllOf.Count == 0 &&
	AnyOf.Count == 0 &&
	OneOf.Count > 1;

[keels4444]

@adamjones1 - Thanks for the reply. Do you know when you expect the next release to be ready? Is it possible to get a pre-release and I can help test the condition above?

[adamjones1]

@keels4444 That would be a question for @RicoSuter, I'm not a maintainer - I would guess it would be best to wait until the PR is reviewed before doing deeper testing though as it could change a fair bit in code review.

[MariaCobretti]

this issue with oneOf not being supported is lingering around for 2 years now. @adamjones1 thank you for getting at it and I hope @RicoSuter gets a chance to review it soon. I also would appreciate a pre release package for c#

[AndreTheHunter]

@RicoSuter any update on this issue?

[johnwc]

@RicoSuter I may have a possible solution for at least one scenario of oneOf.

After generating the code, I manually changed the class as described below. And it works very well.

With api spec being

  schemas:
    CheckAddressResponse:
      type: object
      description: ""
      properties:
        CreditSpent:
          type: integer
          format: int32
        CreditBalance:
          type: integer
          format: int32
        TransmissionResults:
          type: string
        Records:
          oneOf:
            - type: array
              items:
                $ref: "#/components/schemas/CheckAddressRecord"
            - $ref: "#/components/schemas/InvalidRecord"

From...

[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.1.0.0 (NJsonSchema v11.0.2.0 (Newtonsoft.Json v13.0.0.0))")]
internal partial class CheckAddressResponse
{

    [System.Text.Json.Serialization.JsonPropertyName("CreditSpent")]
    public int CreditSpent { get; set; }

    [System.Text.Json.Serialization.JsonPropertyName("CreditBalance")]
    public int CreditBalance { get; set; }

    [System.Text.Json.Serialization.JsonPropertyName("TransmissionResults")]
    public string TransmissionResults { get; set; }

    [System.Text.Json.Serialization.JsonPropertyName("Records")]
    public System.Collections.Generic.ICollection<CheckAddressRecord> Records { get; set; }

    private System.Collections.Generic.IDictionary<string, object> _additionalProperties;

    [System.Text.Json.Serialization.JsonExtensionData]
    public System.Collections.Generic.IDictionary<string, object> AdditionalProperties
    {
        get { return _additionalProperties ?? (_additionalProperties = new System.Collections.Generic.Dictionary<string, object>()); }
        set { _additionalProperties = value; }
    }

}

To...
I also changed the _settings static field from private to internal to be able to use the same settings in the JsonSerializer.Deserialize method call.

[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "14.1.0.0 (NJsonSchema v11.0.2.0 (Newtonsoft.Json v13.0.0.0))")]
internal partial class CheckAddressResponse
{

    [System.Text.Json.Serialization.JsonPropertyName("CreditSpent")]
    public int CreditSpent { get; set; }

    [System.Text.Json.Serialization.JsonPropertyName("CreditBalance")]
    public int CreditBalance { get; set; }

    [System.Text.Json.Serialization.JsonPropertyName("TransmissionResults")]
    public string TransmissionResults { get; set; }

    [System.Text.Json.Serialization.JsonIgnore()]
    public System.Collections.Generic.ICollection<CheckAddressRecord> RecordsOption1 => JsonSerializer.Deserialize<CheckAddressRecord[]>(RecordsElement, ApiClientSDK._settings.Value);

    [System.Text.Json.Serialization.JsonIgnore()]
    public InvalidRecord RecordsOption2 => JsonSerializer.Deserialize<InvalidRecord>(RecordsElement, ApiClientSDK._settings.Value);

    [System.Text.Json.Serialization.JsonPropertyName("Records")]
    public System.Text.Json.JsonElement RecordsElement { get; set; }

    private System.Collections.Generic.IDictionary<string, object> _additionalProperties;

    [System.Text.Json.Serialization.JsonExtensionData]
    public System.Collections.Generic.IDictionary<string, object> AdditionalProperties
    {
        get { return _additionalProperties ?? (_additionalProperties = new System.Collections.Generic.Dictionary<string, object>()); }
        set { _additionalProperties = value; }
    }

}

Knowing that it can be either, I can check the RecordsElement if it is Array kind to know which one to call.