Add support for STJ-native polymorphic JsonDerivedType and JsonPolymorphic attributes to C# client/schema generator
#1595
Conversation
|
As an aside, it would be really, really nice to also generate Open API inheritance discriminators from the JsonPolymorphicAttribute instead of the NJsonSchema/src/NJsonSchema/Generation/JsonSchemaGenerator.cs Lines 1138 to 1206 in ed250a8 Do you think it's feasible to add support for this? |
|
My use case is that I deserialize a document via Marten. The class used for deserialization is already annotated with the STJ polymorphism. I'd have to add [JsonPolymorphic(TypeDiscriminatorPropertyName = TypeDiscriminatorName)]
[JsonDerivedType(typeof(FieldString), typeDiscriminator: nameof(FieldString))]
[Newtonsoft.Json.JsonConverter(typeof(JsonInheritanceConverter), TypeDiscriminatorName)]
[KnownType(typeof(FieldString))]
public abstract class FieldBase
{
public const string TypeDiscriminatorName = "$t";
}
public class FieldString : FieldBase {}Mixing STJ and Newtonsoft attributes like this is a bit ugly. This would be resolved if the NSwag schema generator could also understand Sorry for the noise, let me know if I should also open a corresponding issue for discussion. |
schnerring
changed the title
JsonDerivedType and JsonPolymorphic attributes to C# client generator
schnerring
force-pushed
the
native-stj-polymorphism
branch
from
c124072
to
cf17636
Compare
Turns out it wasn't too hard (but a bit dirty) to make the C# generator aware of the new STJ polymorphic attributes. Have a look at the unit tests I've added to see how it works. I implemented it in such a way that the schema generator looks for the new attributes first, and if it doesn't find them executes the old logic as is. Todos:
|
schnerring
changed the title
JsonDerivedType and JsonPolymorphic attributes to C# client generatorJsonDerivedType and JsonPolymorphic attributes to C# client/schema generator
|
It would be great if this could be merged. Nswag is literally the last piece of code we have to migrate before we can move to STJ. |
|
This is still open? Please take a look @RicoSuter thanks! |
|
A word of caution before migrating to STJ native polymorphism. STJ currently (.NET 7) only supports the discriminator as the first value found in the json object. There is an open request to address this. FWIW, here is my implementation of the STJ JsonInheritanceConverter/Attribute which addresses the StackOverflowException caused by recursive calls by the JsonSerializer. using System.Reflection;
using System.Text.Json;
using System.Text.Json.Serialization;
using System.Text.Json.Serialization.Metadata;
namespace .....
{
/// <summary>
/// <inheritdoc/>
/// </summary>
/// <typeparam name="TBase"></typeparam>
public class JsonInheritanceConverter<TBase> : JsonConverter<TBase>
{
private readonly string _discriminatorName;
private readonly Type _baseType;
public JsonInheritanceConverter(string discriminatorName)
{
_discriminatorName = discriminatorName;
_baseType = typeof(TBase);
}
/// <summary>
/// <inheritdoc/>
/// </summary>
/// <param name="reader"></param>
/// <param name="typeToConvert"></param>
/// <param name="options"></param>
/// <returns></returns>
public override TBase Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
var document = JsonDocument.ParseValue(ref reader);
var typeToReturn = document.RootElement.TryGetProperty(_discriminatorName, out var discriminator)
? GetObjectSubtype(typeToConvert, discriminator.GetString()!)
: typeToConvert;
// NOTE: To avoid calling this coverter recursively (StackOverflowException), we get the default converter for the base type
// and add to a "copy" of the input JsonSerializerOptions. This will take priority over the annotation and can be de-serialized
// as usual
JsonSerializerOptions readOptions = options;
if (typeToReturn == _baseType)
{
var converter = (JsonConverter<TBase>)JsonTypeInfo.CreateJsonTypeInfo<TBase>(options).Converter;
readOptions = new JsonSerializerOptions(options);
readOptions.Converters.Add(converter);
}
return (TBase)JsonSerializer.Deserialize(document, typeToReturn, readOptions)!;
}
/// <summary>
/// <inheritdoc/>
/// </summary>
/// <param name="writer"></param>
/// <param name="value"></param>
/// <param name="options"></param>
public override void Write(Utf8JsonWriter writer, TBase? value, JsonSerializerOptions options)
{
Type typeToWrite = value!.GetType();
var discriminatorValue = GetSubtypeDiscriminator(typeToWrite);
// NOTE: To avoid calling this coverter recursively (StackOverflowException), we get the default converter for the base type
// and add to a "copy" of the input JsonSerializerOptions. This will take priority over the annotation and can be serialized
// as usual
JsonSerializerOptions writeOptions = options;
if (typeToWrite == _baseType)
{
var converter = (JsonConverter<TBase>)JsonTypeInfo.CreateJsonTypeInfo<TBase>(options).Converter;
writeOptions = new JsonSerializerOptions(options);
writeOptions.Converters.Add(converter);
}
var rootElement = JsonSerializer.SerializeToElement(value, typeToWrite, writeOptions);
writer.WriteStartObject();
// Look to see if discriminatorValue is not already a property and explicitly write if missing
if (!rootElement.EnumerateObject().Any(e => e.Name.Equals(_discriminatorName, StringComparison.InvariantCultureIgnoreCase)))
writer.WriteString(_discriminatorName, discriminatorValue);
rootElement.EnumerateObject().ToList().ForEach(property => property.WriteTo(writer));
writer.WriteEndObject();
}
private Type GetObjectSubtype(Type objectType, string discriminatorValue)
=> CustomAttributeExtensions.GetCustomAttributes<JsonInheritanceAttribute>(IntrospectionExtensions.GetTypeInfo(objectType), true)
.Where(attrib => attrib.Key == discriminatorValue).Select(attrib => attrib.Type).FirstOrDefault() ?? objectType;
private string GetSubtypeDiscriminator(Type objectType)
=> CustomAttributeExtensions.GetCustomAttributes<JsonInheritanceAttribute>(IntrospectionExtensions.GetTypeInfo(objectType), true)
.Where(attrib => attrib.Type == objectType).Select(attrib => attrib.Key).FirstOrDefault() ?? objectType.Name;
}
/// <summary>
/// <inheritdoc/>
/// </summary>
[AttributeUsage(AttributeTargets.Class)]
internal class JsonInheritanceConverterAttribute : JsonConverterAttribute
{
public string DiscriminatorName { get; }
public JsonInheritanceConverterAttribute(Type _, string discriminatorName = "discriminatorValue")
{
DiscriminatorName = discriminatorName;
}
public override JsonConverter? CreateConverter(Type baseType)
{
var converterType = typeof(JsonInheritanceConverter<>).MakeGenericType(baseType);
return (JsonConverter?)Activator.CreateInstance(converterType, DiscriminatorName)
?? throw new InvalidOperationException($"Unable to create {converterType}");
}
}
} |
|
See also: JasperFx/marten#2586 Another hack that works is choosing a discriminator property name like this: [JsonPolymorphic(TypeDiscriminatorPropertyName = "_")] |
|
I think we need to put this behind an eg UseNativeInheritance flag (defalt false for now) so that you can opt-in to this new generator... if you look at the generated code as a black box then there should not really be a difference assuming both have the same features/performance... |
|
I see your point, but wouldn't it be confusing for STJ users having to opt into this explicitly? Would it be reasonable to enable this feature by default if STJ is selected instead of Newtonsoft? |
Since there is a possible complication with the issue dotnet/runtime#72604 it wouldn't it be good to have the flag for now? The flag can be removed in the future when no such complications are known. |
Fair enough. If I understand correctly, some of the work from this PR has been integrated into the master branch (16bc1c0). I'm not sure if there's a NuGet preview, though. I haven't kept up with the latest NSwag changes the past couple of months, so unless Rico or someone else tells me how to continue with the work of this PR, I won't add any changes. |
|
Hi! I cloned the schnerring branch of NJsonSchema and the main branch of NSwag. It seems like NJsonSchema doesn't have a method for generating a client from a complete Open Api spec, but just the different parts. I was hoping to test the code generation of a complete example Open Api spec directly in NJsonSchema. Is that possible? |
|
I'm wondering if the case of |
|
I finally got the chance to take another look at this. The schema generation changes of this PR have already been incorporated in this commit: 16bc1c0 Since a lot of changes have been made since then, I created a new PR instead of rebasing this one. The new PR is my attempt to add the missing STJ polymorphism features to the C# client generator: #1675 |
Beginning with .NET 7, System.Text.Json supports polymorphic type hierarchy serialization and deserialization with attribute annotations.. This PR adds support for the generation of
JsonPolymorphicandJsonDerivedTypeattributes to the C# client generator.This basically deprecates
NJsonSchema/src/NJsonSchema.Tests/Generation/SystemTextJson/JsonInheritanceConverter.cs
Line 14 in ed250a8
It Works On My Machine™, but the addition of unit tests and the changes I've made are pretty naive and the generated STJ code only works for .NET 7 or later. I was looking for something like a ".NET target type" parameter in the Liquid templates, but couldn't find any of the like. @RicoSuter I'd appreciate if you could point me to where else I gotta flesh out the code.
Related issues: