Recent JsonIgnoreCondition change breaks NSwag C# clients #1564

ovska · 2022-10-14T10:09:39Z

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.

The text was updated successfully, but these errors were encountered:

ThomasTosik · 2022-12-12T14:11:06Z

I agree with @ovska. The changes are causing some harm and the worst part is that i can not opt-out of it easily. I have also stumbled upon it here: RicoSuter/NSwag#4012 (comment)

RicoSuter · 2022-12-13T07:17:34Z

Can you popose a new PR? Is it really necessary to add a new option? Isnt there just a right behavior?

ovska · 2023-01-04T07:05:26Z

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).

alefcarlos · 2023-01-24T21:17:37Z

Same problem here

ErikPilsits-RJW · 2023-01-27T21:36:51Z

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.

alefcarlos · 2023-01-30T23:37:02Z

To solve my issue I added the enum to the required props(POCO is generated with Never option).

My case is like the @ErikPilsits-RJW said, I dont use [Required] attribute, all my validation are made using FluentValidations. So I cant use the default model validation behaviour. I created an Schema Filter to select all the properties marked with an specific attribute.

    public class SwaggerRequiredPropertyFilter : ISchemaFilter
    {
        public void Apply(OpenApiSchema schema, SchemaFilterContext context)
        {
            var props = context.Type.GetProperties().Where(p => p.GetCustomAttribute<SwaggerRequiredPropertyAttribute>() is not null).ToList();
            if (props.Count is 0)
                return;
            var explicitRequired = new HashSet<string>();
            foreach (var item in schema.Properties)

            {
                if (props.Any(x => string.Compare(item.Key, x.Name, StringComparison.InvariantCultureIgnoreCase) is 0))
                {
                    explicitRequired.Add(item.Key);
                }
            }
            schema.Required.UnionWith(explicitRequired);
        }
    }

epilsits · 2023-01-30T23:50:53Z

In my case, the enum in the request is only conditionally required, it depends on other properties which are validated via FluentValidation. So the [Required] attribute is not a solution for me.

alefcarlos · 2023-01-31T13:38:14Z

@epilsits I dont use [Required] property as well, but the only way to solve that, for now, was to include the enum in the required list.

public class AddWeatherForecastRequest
{
    public DateTime? Date { get; set; }

    public string? Summary { get; set; }

    [SwaggerRequiredProperty]
    public MyEnum EnumValue { get; set; }

    public int Age { get; set; }
}

[AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
public class SwaggerRequiredPropertyAttribute : Attribute
{
}

And the schema filter:

    public class SwaggerRequiredPropertyFilter : ISchemaFilter
    {
        public void Apply(OpenApiSchema schema, SchemaFilterContext context)
        {
            var props = context.Type.GetProperties().Where(p => p.GetCustomAttribute<SwaggerRequiredPropertyAttribute>() is not null).ToList();
            if (props.Count is 0)
                return;
            var explicitRequired = new HashSet<string>();
            foreach (var item in schema.Properties)

            {
                if (props.Any(x => string.Compare(item.Key, x.Name, StringComparison.InvariantCultureIgnoreCase) is 0))
                {
                    explicitRequired.Add(item.Key);
                }
            }
            schema.Required.UnionWith(explicitRequired);
        }
    }

The generated OpenApi spec:

 "schemas": {
      "AddWeatherForecastRequest": {
        "required": [
          "enumValue"
        ],
        "type": "object",
        "properties": {
          "date": {
            "type": "string",
            "format": "date-time",
            "nullable": true
          },
          "summary": {
            "type": "string",
            "nullable": true
          },
          "enumValue": {
            "allOf": [
              {
                "$ref": "#/components/schemas/MyEnum"
              }
            ]
          },
          "age": {
            "type": "integer",
            "format": "int32"
          }
        },
        "additionalProperties": false
      },
      "MyEnum": {
        "enum": [
          "Initial",
          "Secondary"
        ],
        "type": "string"
      }
    },

Then the DTO generated by NSwag:

[System.CodeDom.Compiler.GeneratedCode("NJsonSchema", "13.18.0.0 (NJsonSchema v10.8.0.0 (Newtonsoft.Json v13.0.0.0))")]
    public partial class AddWeatherForecastRequest
    {

        [System.Text.Json.Serialization.JsonPropertyName("date")]

        [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingDefault)]   
        public System.DateTimeOffset? Date { get; set; } = default!;

        [System.Text.Json.Serialization.JsonPropertyName("summary")]

        [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingDefault)]   
        public string? Summary { get; set; } = default!;

        [System.Text.Json.Serialization.JsonPropertyName("enumValue")]

        [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.Never)]   
        [System.ComponentModel.DataAnnotations.Required(AllowEmptyStrings = true)]
        [System.Text.Json.Serialization.JsonConverter(typeof(System.Text.Json.Serialization.JsonStringEnumConverter))]
        public MyEnum EnumValue { get; set; } = default!;

        [System.Text.Json.Serialization.JsonPropertyName("age")]

        [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingDefault)]   
        public int Age { get; set; } = default!;

        public string ToJson()
        {

            var options = new System.Text.Json.JsonSerializerOptions();

            return System.Text.Json.JsonSerializer.Serialize(this, options);

        }
        public static AddWeatherForecastRequest FromJson(string data)
        {

            var options = new System.Text.Json.JsonSerializerOptions();

            return System.Text.Json.JsonSerializer.Deserialize<AddWeatherForecastRequest>(data, options);

        }

    }

This is not the best solution, but the only one I could think.

ErikPilsits-RJW · 2023-01-31T14:35:22Z

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.

alefcarlos · 2023-01-31T14:58:13Z

@ErikPilsits-RJW I tried the [SwaggerSchema(Required = new[] { "enumValue" })] but if the developer changes the property name and forgets to update the Required value from the attribute it would result in bad behaviour.

That was the reason I created a custom filter, it only needs to decorate the property.

ErikPilsits-RJW · 2023-02-22T15:11:06Z

A workaround for now is to modify the NJsonSchema liquid template Class.liquid. Update lines 56-58 as needed. This change will only output the condition if it's "Never". The source for the condition string is here https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema.CodeGeneration.CSharp/Models/PropertyModel.cs

{%-   if property.HasJsonIgnoreCondition and property.JsonIgnoreCondition contains "JsonIgnoreCondition.Never" %}
    [System.Text.Json.Serialization.JsonIgnore(Condition = {{ property.JsonIgnoreCondition }})]   
{%-   endif -%}

ThomasTosik · 2023-05-03T07:47:10Z

Any news on this? Will there be ever a proper fix or opt-out? Currently i am stuck to a specific NSwag version before these changes.

ajdust · 2023-05-04T17:04:41Z

Doing package upgrade and just ran into this breaking change. Like others, went looking for a configuration flag to disable this JsonIgnore attribute being added to everything. Found this issue instead. Downgraded back to NSwag version 13.15.10. Really appreciate RicoSuter's and other contributors work on this project, but would suggest making the JsonIgnore attribute configurable, as this change breaks a ton of projects :)

mooski · 2023-05-23T16:06:55Z

Same here, this has been a problem for over a year so we're having to start looking for an alternative to using NSwag since it looks like this isn't going to be fixed in the near future.

(Found this issue via RicoSuter/NSwag#4012)

ErikPilsits-RJW · 2023-05-23T16:18:28Z

@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.

#1564 (comment)

mooski · 2023-05-23T17:00:17Z

@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 (Recent JsonIgnoreCondition change breaks NSwag C# clients #1564 (comment))
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 😕

ThomasTosik · 2023-06-01T07:45:18Z

We already had some liquid templates in place but i didnt know that i could also replace the NJsonSchema through the liquid templates as well. Thanks!

jcracknell · 2023-06-12T12:03:35Z

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.

laurabrovkina · 2023-07-28T02:08:23Z

Our team had the same issue with the NSwag autogenerated client and the model in it. We had similar issues as has been described on this page: Non-nullable Enum with value 0 and boolean with value false properties were set to null instead of what was expected from them. JsonIgnoreCondition.WhenWritingDefault was responsible for that and we had a choice to downgrade the package before this breaking change had been introduced (which is NSwag.MSBuild 13.15.0).
The solution described by @mooski was working quite well too. Also, we came up with the idea that if we mark all optional properties as Nullable.
For this, you can add an option to dotnet command or NSwagStudio configuration, generateOptionalPropertiesAsNullable should be set to true and it would do the trick even if the JsonIgnoreCondition.WhenWritingDefault is presented for such enum or boolean.
I hope this information could help engineers in a similar situation. Thanks

cheesi · 2023-09-08T14:11:01Z

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

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

altmann · 2023-11-06T16:48:00Z

@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.

fixes RicoSuter#1564

RicoSuter · 2023-12-08T09:10:05Z

So if we revert the PRs
https://github.com/RicoSuter/NJsonSchema/pull/1513/files
https://github.com/RicoSuter/NJsonSchema/pull/1522/files
then it should be fixed?
i.e. we just remove the ignore attribute at all...

altmann · 2023-12-08T10:32:27Z

I think yes. I also found only these two PRs about this feature.

ErikPilsits-RJW · 2023-12-08T14:41:04Z

Was it intended to completely remove the attribute? Even the JsonIgnoreCondition.Never when the property is required?

RicoSuter · 2023-12-08T15:04:00Z

Is it actually needed to have this "Never"? Before the PR which introduced this attribute at all it seemed to be fine.

espenrl · 2023-12-08T20:53:45Z

Hi, our team is depedent upon JsonIgnoreCondition as this controls if a property is output to json or not.

What follows are simplified examples of json transferred over the wire to the backend.

Case 1: set year to 2023 in backend

{
  "Year": 2023
}

Case 2: set year to null in backend

{
  "Year": null
}

Case 3: do not touch year at all

As Year is not specified the backend will respect this and refrain from updating it.

{
}

Our team are using JsonIgnoreCondition to choose between case 2 and case 3 in our MAUI frontend.

david-brink-talogy · 2023-12-12T19:43:31Z

@espenrl, You should be able to configure a default ignore condition by implementing UpdateJsonSerializerSettings in your client.

    static partial void UpdateJsonSerializerSettings(JsonSerializerOptions settings) => settings.DefaultIgnoreCondition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull;

espenrl · 2023-12-13T09:34:30Z

@espenrl, You should be able to configure a default ignore condition by implementing UpdateJsonSerializerSettings in your client.
    static partial void UpdateJsonSerializerSettings(JsonSerializerOptions settings) => settings.DefaultIgnoreCondition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull;

That's great. Thanks for the tip!

mooski mentioned this issue May 23, 2023

The ignore condition 'JsonIgnoreCondition.WhenWritingNull' is not valid on value-type member RicoSuter/NSwag#4012

Closed

david-brink-talogy pushed a commit to david-brink-talogy/NJsonSchema that referenced this issue Dec 7, 2023

Adds CSharp System.Text.Json setting OptionalPropertiesWriteDefaultValue

7c2bb7b

fixes RicoSuter#1564

david-brink-talogy mentioned this issue Dec 7, 2023

Adds CSharp System.Text.Json setting OptionalPropertiesWriteDefaultValue #1656

Closed

RicoSuter closed this as completed in e1a2a79 Dec 8, 2023

isteyak mentioned this issue Apr 12, 2024

jsonignore attributes is not showing in latest nswag 14.0.7 version generated code RicoSuter/NSwag#4852

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Recent JsonIgnoreCondition change breaks NSwag C# clients #1564

Recent JsonIgnoreCondition change breaks NSwag C# clients #1564

ovska commented Oct 14, 2022 •

edited

ThomasTosik commented Dec 12, 2022

RicoSuter commented Dec 13, 2022

ovska commented Jan 4, 2023

alefcarlos commented Jan 24, 2023

ErikPilsits-RJW commented Jan 27, 2023

alefcarlos commented Jan 30, 2023 •

edited

epilsits commented Jan 30, 2023

alefcarlos commented Jan 31, 2023 •

edited

ErikPilsits-RJW commented Jan 31, 2023

alefcarlos commented Jan 31, 2023

ErikPilsits-RJW commented Feb 22, 2023 •

edited

ThomasTosik commented May 3, 2023 •

edited

ajdust commented May 4, 2023

mooski commented May 23, 2023

ErikPilsits-RJW commented May 23, 2023

mooski commented May 23, 2023 •

edited

ThomasTosik commented Jun 1, 2023 •

edited

jcracknell commented Jun 12, 2023

laurabrovkina commented Jul 28, 2023 •

edited

cheesi commented Sep 8, 2023

altmann commented Nov 6, 2023

RicoSuter commented Dec 8, 2023 •

edited

altmann commented Dec 8, 2023

ErikPilsits-RJW commented Dec 8, 2023

RicoSuter commented Dec 8, 2023

espenrl commented Dec 8, 2023

david-brink-talogy commented Dec 12, 2023

espenrl commented Dec 13, 2023

Recent JsonIgnoreCondition change breaks NSwag C# clients #1564

Recent JsonIgnoreCondition change breaks NSwag C# clients #1564

Comments

ovska commented Oct 14, 2022 • edited

ThomasTosik commented Dec 12, 2022

RicoSuter commented Dec 13, 2022

ovska commented Jan 4, 2023

alefcarlos commented Jan 24, 2023

ErikPilsits-RJW commented Jan 27, 2023

alefcarlos commented Jan 30, 2023 • edited

epilsits commented Jan 30, 2023

alefcarlos commented Jan 31, 2023 • edited

ErikPilsits-RJW commented Jan 31, 2023

alefcarlos commented Jan 31, 2023

ErikPilsits-RJW commented Feb 22, 2023 • edited

ThomasTosik commented May 3, 2023 • edited

ajdust commented May 4, 2023

mooski commented May 23, 2023

ErikPilsits-RJW commented May 23, 2023

mooski commented May 23, 2023 • edited

ThomasTosik commented Jun 1, 2023 • edited

jcracknell commented Jun 12, 2023

laurabrovkina commented Jul 28, 2023 • edited

cheesi commented Sep 8, 2023

altmann commented Nov 6, 2023

RicoSuter commented Dec 8, 2023 • edited

altmann commented Dec 8, 2023

ErikPilsits-RJW commented Dec 8, 2023

RicoSuter commented Dec 8, 2023

espenrl commented Dec 8, 2023

Case 1: set year to 2023 in backend

Case 2: set year to null in backend

Case 3: do not touch year at all

david-brink-talogy commented Dec 12, 2023

espenrl commented Dec 13, 2023

ovska commented Oct 14, 2022 •

edited

alefcarlos commented Jan 30, 2023 •

edited

alefcarlos commented Jan 31, 2023 •

edited

ErikPilsits-RJW commented Feb 22, 2023 •

edited

ThomasTosik commented May 3, 2023 •

edited

mooski commented May 23, 2023 •

edited

ThomasTosik commented Jun 1, 2023 •

edited

laurabrovkina commented Jul 28, 2023 •

edited

RicoSuter commented Dec 8, 2023 •

edited