NJsonSchema: Recent JsonIgnoreCondition change breaks NSwag C# clients

Caused by #1522

Example from string serialized enums, here is the source enum:

public enum Scope
{
    Value1 = 1,
    Value2 = 2,
    Value3 = 3,
}

Relevant part of OpenApi json:

{
  // ...
  "Scope": {
    "enum": [
      "Value1",
      "Value2",
      "Value3"
    ],
    "type": "string",
    "additionalProperties": false,
    "x-enumNames": [
      "Value1",
      "Value2",
      "Value3"
    ]
  }
  // ...
}

Generated enum in C# client:

public enum Scope
{
    [System.Runtime.Serialization.EnumMember(Value = @"Value1")]
    Value1 = 0,
    [System.Runtime.Serialization.EnumMember(Value = @"Value2")]
    Value2 = 1,
    [System.Runtime.Serialization.EnumMember(Value = @"Value3")]
    Value3 = 2,
}

And finally the property w/ the enum in a generated class:

[System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingDefault)]   
[System.Text.Json.Serialization.JsonConverter(typeof(System.Text.Json.Serialization.JsonStringEnumConverter))]
public Scope Scope { get; set; }

When Value1 is used, the JSON property is never serialized, which causes the Scope enum to have an undefined value of 0 at the C# controller side.

The attribute can also have unintended consoquences when generating clients against runtimes that have different semantics from C#. For example, it’s impossible to get the serializer to write a zero int or DateTime.MinValue property. IMO the explicit ignore condition should never be used for value types, and I think it’s debatable if it’s required at all; when serializing you can opt to use JsonSerializerOptions.DefaultIgnoreCondition.

About this issue

Original URL
State: closed
Created 2 years ago
Reactions: 8
Comments: 28 (4 by maintainers)

Commits related to this issue

Adds CSharp System.Text.Json setting OptionalPropertiesWriteDefaultValue fixes #1564 — committed to psiservices-dbrink/NJsonSchema by psiservices-dbrink 7 months ago

Most upvoted comments

@ErikPilsits-RJW Thanks, that’s great.

I originally dismissed that idea because I assumed it was more effort than it was worth, but you encouraged me to take a proper look at custom templates and it seems to work perfectly.

For the benefit of others, here’s what I did (I’m using NSwag from the command line via the dotnet tool, so this may or not apply to other people’s setup):

Created a new folder, NSwagTemplates.
Updated my NSwag config file to point to this new folder: "templateDirectory": "NSwagTemplates",
Created the file Class.liquid in this folder with the contents copied from https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema.CodeGeneration.CSharp/Templates/Class.liquid
Replaced lines 56-58 with the code provided by @ErikPilsits-RJW in the comment above (https://github.com/RicoSuter/NJsonSchema/issues/1564#issuecomment-1440227702)
Ran Nswag as normal - dotnet tool run nswag run NSwagConfig.nswag

JsonIgnoreCondition.WhenWritingDefault is no longer present in the output.

What still confuses me is why this change was made in the first place 😕

mooski on May 23, 2023

Hi there, I’m the original author of the JsonIgnoreCondition code for System.Text.Json and I thought I should share a few thoughts.

Generally if a property is not required by the JSON schema, it must be generated as a nullable type, as null is (conventionally) the only way to express omission in C#. If WhenWritingNull is causing problems then the problem is with the property type. If you have a property which is both nullable and not required in the schema, then the best you can do is to hope that there is no semantic difference between sending null and omission.

WhenWritingDefault makes no sense at all, as obviously it needs to be possible to send the 0 value.

jcracknell on Jun 12, 2023

Thank you very much @mooski for you workaround, you saved my day!

Pls consider fixing this, this is a very sneaky and annoying bug.

cheesi on Sep 8, 2023

@ThomasTosik @ajdust @mooski

I really encourage you to try my workaround. Getting custom templates set up is not hard at all, and gives you flexibility to change behavior to your requirements. There’s a bit of a lift when a new version of nswag is released, if the release updates the internal templates, as you’ll have to merge those changes into your own template. But in practice this has not been hard for me and does not happen with every release.

https://github.com/RicoSuter/NJsonSchema/issues/1564#issuecomment-1440227702

ErikPilsits-RJW on May 23, 2023

I think the most flexible behavior is to leave the attribute out altogether. WhenWritingDefault should imo never be used for the reasons outlined above, and even WhenWritingNull can be problematic since the JSON property will completely be missing, which might cause problems if the property is required (be it null or not). If performance is a concern, the JsonSerializerOptions.DefaultIgnoreCondition can be configured separately (as you would in a NSwag-generated client).

ovska on Jan 4, 2023

I use Swashbuckle to generate my specs, so I guess I don’t see the difference here between using [Required] or [SwaggerSchema(Required = new[] { "enumValue" })] attributes. They result in the same spec being generated as your example.

For my usage, my point was I don’t want these properties marked as required in the spec, so none of these three are options.

ErikPilsits-RJW on Jan 31, 2023

This is complicated… I’ll try to make it make sense…

Our projects use Swashbuckle to generate openapi specs, which we use nswag to generate c# and typescript clients.

This is an issue in certain validation scenarios in combination with nullable semantics. For example I often set value types in request models as nullable, ie a nullable enum property. This allows me to distinguish between a missing request value and a default value for a value type during request validation (using FluentValidation for example).

But openapi with schema $ref’s doesn’t handle this well unless the enum itself is decorated with a swagger attribute to set Nullable = true. This is not ideal since it’s likely unintended to set that enum as nullable in every context.

In any case, when that is not done, then nswag generates the enum as non-nullable. Unless the nswag option to “generate optional schema properties as nullable” is enabled, then you get a non-nullable enum with the WhenWritingDefault attribute. This makes it impossible to send a request with the default value (it is not serialized and therefore validation fails for a missing property).

I don’t see any clean way around this at the moment. I do not want to set the “generate optional…” flag, because that litters my c# clients with nullable properties where they are not intended (for example in response models, where defining especially value types as “required” is not desired).

So I agree, I think we need an option to disable writing WhenWritingDefault.

ErikPilsits-RJW on Jan 27, 2023

@RicoSuter It would be great if you can focus on this issue - I think the community needs after one year a fix and not only a workaround. Please make a featureflag or rollback this feature or fix this WhenWritingDefault behaviour or whatever. I had a look at the code but for me there are too many unknowns so I can’t do it.

@jcracknell As you wrote you introduced this JsonIgnore feature in issue #1513. You are the knowledge owner of this stuff - please bring it to a good end - now this feature is in a broken state. Sorry but thats the truth.

altmann on Nov 6, 2023