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

Update JsonInheritanceConverter.liquid #1502

Merged
merged 3 commits into from May 15, 2022
Merged

Conversation

zyofeng
Copy link
Contributor

@zyofeng zyofeng commented Mar 10, 2022

The JsonInheritanceConverters need to be made public so downstream NSwag Generator can get the DiscriminatorName (i.e. wrapping Client classes in another generated client class)

Error below:
6> ---> Microsoft.CSharp.RuntimeBinder.RuntimeBinderException: 'System.Text.Json.Serialization.JsonConverter' does not contain a definition for 'DiscriminatorName'

The InheritanceConverters needs to be made public so downstream NSwag Generator can get the DiscriminatorName (i.e. wrapping Client classes in another generated client class)

Error below:
6> ---> Microsoft.CSharp.RuntimeBinder.RuntimeBinderException: 'System.Text.Json.Serialization.JsonConverter<BaseClass>' does not contain a definition for 'DiscriminatorName'
@RicoSuter
Copy link
Owner

The JsonInheritanceConverters need to be made public so downstream NSwag Generator can get the DiscriminatorName (i.e. wrapping Client classes in another generated client class)

I dont think that it needs to be public (as in reflection internal is the same as public I think)
Did you try the latest NSwag/NJS version as I think this has already been fixed.

@zyofeng
Copy link
Contributor Author

zyofeng commented Mar 11, 2022

I'm using the latest NSwag.msbuild which is still affected. After manually modifying the accessor to public the problem went away.

@zyofeng
Copy link
Contributor Author

zyofeng commented Mar 15, 2022

The JsonInheritanceConverters need to be made public so downstream NSwag Generator can get the DiscriminatorName (i.e. wrapping Client classes in another generated client class)

I dont think that it needs to be public (as in reflection internal is the same as public I think) Did you try the latest NSwag/NJS version as I think this has already been fixed.

I have created a repo replicating the problem using latest NSwag.AspNetCore as well as NSwag.Build.
https://github.com/zyofeng/NswagGenerationIssue

I think the issue is when client codes are from different projects, NJsonSchema.OpenApiDiscriminator.AddMapping is unable to get the method through reflection due to JsonInheritanceConverter being an internal class from a different project

@zyofeng
Copy link
Contributor Author

zyofeng commented Mar 28, 2022

Alternatively we can use InternalsVisibleTo("NJsonSchema")
But this will make the entire assembly visible to NJsonSchema, which seems a bit excessive.

@zyofeng
Copy link
Contributor Author

zyofeng commented Mar 29, 2022

Also updated DateFormatConverter.liquid due to missing namespace.

@RicoSuter
Copy link
Owner

Can you also fix the tests?

@zyofeng
Copy link
Contributor Author

zyofeng commented Apr 4, 2022

Done.

@RicoSuter RicoSuter merged commit a9848a2 into RicoSuter:master May 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants