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

Fixed SystemTextJson indexed properties handling. #1701

Merged

Conversation

viktoriia-zhel
Copy link
Contributor

The SystemTextJsonReflectionService did not handle properly types with indexed properties. Exception "The JSON property 'item' is defined multiple times on type" was thrown if schema had been generated for type with multiple indexed properties.

In the applied fix indexed properties are detected and not added to the generated schema.
Result schema matches schemas generated with NewtonsoftJsonReflectionService and previous versions of NJsonSchema library.

Fixes RicoSuter/NSwag#4874

@viktoriia-zhel
Copy link
Contributor Author

@lahma Could you, please, help me with a build error that I get?

I am getting " [ERR] Compile: D:\a\NJsonSchema\NJsonSchema\src\NJsonSchema\Generation\JsonSchemaGenerator.cs(1092,25): error CA1859: Change return type of method 'TryGetInheritanceDiscriminatorConverter' from 'object?' to 'NJsonSchema.Generation.JsonSchemaGenerator.SystemTextJsonInheritanceWrapper?' for improved performance (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1859) [D:\a\NJsonSchema\NJsonSchema\src\NJsonSchema\NJsonSchema.csproj::TargetFramework=netstandard2.0]".

I tried to fix it as error message suggested, changed return type of method TryGetInheritanceDiscriminatorConverter to SystemTextJsonInheritanceWrapper. However, that broke unit test When_schema_has_base_schema_then_it_is_referenced_with_Newtonsoft.

Do you know what is a correct type to use here to fix CA1859?
Or can I disable this check for the method TryGetInheritanceDiscriminatorConverter so the build is successful?

@@ -1089,7 +1089,7 @@ private void GenerateInheritanceDiscriminator(Type type, JsonSchema schema, Json
}
}

private object? TryGetInheritanceDiscriminatorConverter(Type type)
private SystemTextJsonInheritanceWrapper? TryGetInheritanceDiscriminatorConverter(Type type)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this really needed?
i think this might break CI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think this should be reverted, it breaks test When_schema_has_base_schema_then_it_is_referenced_with_Newtonsoft.

However, without it build fails with " [ERR] Compile: D:\a\NJsonSchema\NJsonSchema\src\NJsonSchema\Generation\JsonSchemaGenerator.cs(1092,25): error CA1859: Change return type of method 'TryGetInheritanceDiscriminatorConverter' from 'object?' to 'NJsonSchema.Generation.JsonSchemaGenerator.SystemTextJsonInheritanceWrapper?' for improved performance (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1859) [D:\a\NJsonSchema\NJsonSchema\src\NJsonSchema\NJsonSchema.csproj::TargetFramework=netstandard2.0]"."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you suggest a better fix to CA1859?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should be able to use

#pragma warning disable CA1859
private object? TryGetInheritanceDiscriminatorConverter(Type type)
#pragma warning enable CA1859

ViktoriiaZheliezniak added 2 commits June 10, 2024 17:11
@RicoSuter RicoSuter merged commit f353123 into RicoSuter:master Jun 12, 2024
2 checks passed
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.

Nswag v14.0.7 client generation issues with NetTopologySuite
3 participants