Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use System.Text.Json in JSON Schema generator #1014

Closed
4 of 6 tasks
RicoSuter opened this issue Jun 15, 2019 · 25 comments
Closed
4 of 6 tasks

Use System.Text.Json in JSON Schema generator #1014

RicoSuter opened this issue Jun 15, 2019 · 25 comments

Comments

@RicoSuter
Copy link
Owner

RicoSuter commented Jun 15, 2019

Open questions:

  • How can we reflect over the used contracts (replacement for Newtonsoft.Json's contract resolver)?
    • => currently this feature is completely missing in System.Text.Json and we need to use reflection and manually implement the same rules in the schema generator (suboptimal)
  • NJS might reference System.Text.Json and Newtonsoft.Json at the same time to support both libraries depending on a setting
    • E.g. you either set a Newtonsoft.Json contract resolver/serializer settings or System.Text.Json settings/contract metadata
  • System.Text.Json is only .NET Standard 2.0 but NJS supports .NET Standard 1.0 and .NET 4.0
    • Lots of #if and System.Text.Json support will not be active in all target frameworks (will throw NotSupportedExceptions otherwise)

Current implementation (NJS 10.1.15+, NSwag v13.5.0+):

See https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema/Generation/SystemTextJsonUtilities.cs

@RicoSuter
Copy link
Owner Author

/cc @steveharter @joshfree is there already a replacement for the contract resolver available? I once saw some related classes but they were internal (no access to metadata)...

@steveharter
Copy link

For 3.0 there are no public APIs to return the metadata. These were considered but considered out of scope for this release because we are focusing on a minimum viable product for mainstream scenarios.

I'd like to expose the metadata, which would also be required for a new type of "converter" that I would like to add. The new converter would handle some scenarios the existing one can't.

@RicoSuter
Copy link
Owner Author

Ok. As long as metadata is not exposed by System.Text.Json we cannot support this new library in NJsonSchema (JSON Schema generation) and NSwag (Swagger/OpenAPI) - so this might be a huge blocker for people who want to use the new serializer in ASP.NET Core AND OpenAPI.

The solution for now is to use System.Text.Json for serialization and add NSwag with a Newtonsoft.Json contract resolver which resembles the System.Text.Json serializer behavior...

@jemiller0
Copy link

I thought System.Text.Json was .NET Standard 2.1, not 2.0... If so, it wouldn't run on .NET Framework at all.

@RicoSuter
Copy link
Owner Author

It seems to support .net standard 2.0:
https://www.nuget.org/packages/System.Text.Json/4.6.0-preview6.19303.8

@jemiller0
Copy link

I was thinking it used Span and made use of new things in the .NET Core runtime which they weren't going to add to .NET Framework. Maybe I'm confusing that with the latest C# language features that they added which will not be supported on .NET Framework.

@jemiller0
Copy link

I was wrong about that. It says here https://devblogs.microsoft.com/dotnet/try-the-new-system-text-json-apis/ that it works with .NET Framework.

@hez2010
Copy link

hez2010 commented Jul 24, 2019

.NET Standard 1.x has been end of support therefore I think there's no need to support .NET Standard 1.x.

@jemiller0
Copy link

That may be, but, don't expect people to move off .NET Standard 2.0 anytime soon.

@hez2010
Copy link

hez2010 commented Oct 29, 2019

You can multi-target your library to resolve it:
in .csproj file:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFrameworks>netstandard1.4;netstandard2.0;netstandard2.1</TargetFrameworks>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Condition="'$(TargetFramework)' == 'netstandard2.1'" Include="System.Text.Json" Version="4.6.0" />
    <PackageReference Include="Newtonsoft.Json" Version="12.0.2" />
  </ItemGroup>
</Project>

in .cs file

#if NETCOREAPP3_0 || NETSTANDARD2_1
....
#else
...
#endif

@RicoSuter
Copy link
Owner Author

Thx, but referencing the library is the small problem, the blocker is this:
#1014 (comment)

@gdoddsy
Copy link

gdoddsy commented Mar 18, 2020

@RicoSuter - are you able to provide any more info on the solution for now? How do I add NSwag with a Newtonsoft.Json contract resolver?

@RicoSuter
Copy link
Owner Author

If you call AddOpenApiDocument() without using Newtonsoft in ASP.NET Core itself, it should still work but with a the default contract resolver. If the default contract resolver does not reflect what the actual serializer is doing, then you need to change the contract resolver so that it reflects the serializers (in the AddOpenApiDocument(options => options.SerializerSettings = ...)

@hez2010
Copy link

hez2010 commented Mar 20, 2020

I think at least NJsonSchema should recognize JsonIgoreAttribute and other attributes in System.Text.Json while generating OpenApi schema.

@RicoSuter
Copy link
Owner Author

JsonIgoreAttribute from System.Text.Json?

@hez2010
Copy link

hez2010 commented Mar 21, 2020

JsonIgoreAttribute from System.Text.Json?

Sorry, that is a typo. It should be JsonIgnoreAttribute.

https://github.com/dotnet/runtime/blob/b0351370ccd132d95c97b75312fc36adaacc2664/src/libraries/System.Text.Json/ref/System.Text.Json.cs#L767

@jeremyVignelles
Copy link
Contributor

JsonIgnoreAttribute isn't the only one that needs to be supported, here are other examples:

  • JsonExtensionDataAttribute : Same as JSON.Net, but only takes a Dictionary<string,object> or a Dictionary<string, JsonElement>. In the latter case, JsonElement should not be included in the generated schema.
  • JsonPropertyNameAttribute can be used to give the property a new name, the same way as JSON.NET's [JsonProperty]

and I probably missed other attributes...

@RicoSuter
Copy link
Owner Author

RicoSuter commented May 7, 2020

@RicoSuter
Copy link
Owner Author

RicoSuter commented May 7, 2020

We'd need to implement a custom Newtonsoft contract resolver which reads the System.Text.Json options and attributes and correctly maps them... see

https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema/SystemTextJsonUtilities.cs#L33

/cc @Sinhk

@RicoSuter
Copy link
Owner Author

Ref: 5981ecb

@RicoSuter
Copy link
Owner Author

RicoSuter commented May 8, 2020

@steveharter is there already a roadmap for exposing serializer metadata (contract resolver)? Otherwise we need to use reflection and reimplement all your rules manually in the schema generator... (I really want to avoid that).

@panesofglass
Copy link

@RicoSuter Are there any additional steps where you could use some help? I would like to update my F# wrapper with support for F# types to use the System.Text.Json support and would be happy to try to help get this over the finish line.

@RicoSuter
Copy link
Owner Author

@panesofglass thanks for your post.

Currently we map the System.Text.Json “rules” manually to the newtonsoft metadata with a custom contract resolver.

https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema/Generation/SystemTextJsonUtilities.cs#L45

This is a quick solution for now until STJ offers metadata directly. There are probably lots of edge cases and attributes missing... would be great if someone could help complete this (also more unit tests needed). Apart from that you can just set the SerializerOptions property on the generator settings and it will use these rules.

https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema/Generation/JsonSchemaGeneratorSettings.cs#L114

What is missing for you?

@panesofglass
Copy link

I don’t know whether anything is missing. I was uncertain whether this had been released, but it seems it has? I’m happy to look into adding some tests.

@panesofglass
Copy link

I have not yet added tests to this repo, but I've been able to switch to System.Text.Json in the other project. I'll still try to get back to adding tests here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants