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

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

Closed
Mosie1 opened this issue May 19, 2022 · 23 comments · Fixed by RicoSuter/NJsonSchema#1522

Comments

@Mosie1
Copy link

Mosie1 commented May 19, 2022

Hello,

Since version 13.16.0 of Nswag, the following attribute is added to all properties of the generated models when generating the C# clients.
[System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull)]

This causes the following errors on value type properties.
The ignore condition 'JsonIgnoreCondition.WhenWritingNull' is not valid on value-type member 'LanguageId' on type 'User'. Consider using 'JsonIgnoreCondition.WhenWritingDefault'.

Is there a way to disable the generation of this attribute?

Kind regards,
Jorsi

@shuaqq2004
Copy link

shuaqq2004 commented May 20, 2022

I had to use v13.15.0... same bug.

bool/int/guid/datetime ..... --> WhenWritingDefault
bool?...Nullable --> WhenWritingNull

@ChrisWCurry
Copy link

Hello,

I am experiencing the same issue (as above).

I am using v10.7.0 of the NJsonSchema and NJsonSchema.CodeGeneration.CSharp NuGet packages when generating classes from a JSON schema using the CSharpGenerator c;lass with:

settings.JsonLibrary = CSharpJsonLibrary.SystemTextJson;

It generates the following property (simple bool):

[System.Text.Json.Serialization.JsonPropertyName("unavailable")] [System.Text.Json.Serialization.JsonIgnore(Condition = System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull)] public bool Unavailable { get; set; }

When I generated the classes via https://apimundo.com/tools?tab=jsonschema-to-csharp it produced the following:

[System.Text.Json.Serialization.JsonPropertyName("unavailable")] public bool Unavailable { get; set; }

The addition of the "System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull" conditions causes System.Text.Json serialization to fail with: "The ignore condition 'JsonIgnoreCondition.WhenWritingNull' is not valid on value-type member 'Unavailable'".

I don't know if this is related to: https://github.com/RicoSuter/NJsonSchema/blob/master/src/NJsonSchema.CodeGeneration.CSharp/Models/PropertyModel.cs

Line 72

/// <summary>Returns the System.Text.Json.Serialization.JsonIgnoreCondition value to be applied to the property.</summary> public string JsonIgnoreCondition => _property switch { { IsRequired: false } => "System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull", { IsRequired: true } when _settings.RequiredPropertiesMustBeDefined => "System.Text.Json.Serialization.JsonIgnoreCondition.Never", _ => null };

Many thanks,

Chris.

@Alundra828
Copy link

It looks like rolling back to 13.15.10 resolves this issue for the time being.

@ChrisWCurry
Copy link

It looks like rolling back to 13.15.10 resolves this issue for the time being.

In my case, moving back to NJsonSchema and NJsonSchema.CodeGeneration.CSharp 10.6.10 resolved my issue.

I did try the 10.7.1 version (which came out on Wednesday), but the issue is still the same in that release.

@binginsin
Copy link

Experiencing the same issue, this seems like quite a serious bug since we cannot use non-nullable value types in our models.
Cannot rollback as we also need #1512
Hoping the pull request gets merged soon.

@timmkrause
Copy link

Same here.

@agilenut
Copy link

agilenut commented Jun 4, 2022

Also experiencing this issue

@bdovaz
Copy link

bdovaz commented Jun 4, 2022

Ping @RicoSuter

@mtbayley
Copy link

Having issues with this also. Will revert to previous version in the meantime.

crobibero added a commit to jellyfin/jellyfin-sdk-csharp that referenced this issue Jun 11, 2022
@gavin2n
Copy link

gavin2n commented Jun 13, 2022

we are also experiencing this issue, however rolling back to 13.15.10 has resolved it for now.

@awp-sirius
Copy link

Same here.

@Keviento
Copy link

Same with v13.16.1. Following solution resolved the issue :

It looks like rolling back to 13.15.10 resolves this issue for the time being.

@agilenut
Copy link

agilenut commented Aug 5, 2022

Any update on this? The latest package version has had a show stopping issue for almost 3 months now.

@frabe1579
Copy link

Same problem here!

@lukewis
Copy link

lukewis commented Sep 2, 2022

@RicoSuter - Thanks for the fix!! I just hit this issue with NSwag.MSBuild. Could we get an updated version of that package published to Nuget?

@bdovaz
Copy link

bdovaz commented Sep 3, 2022

Same problem here!

@ThomasTosik
Copy link

How can i opt-out of this bevaviour or force my values?

Before these changes there was no attribute what so ever (so the default behaviour of STJ i assume). Then it was briefly System.Text.Json.Serialization.JsonIgnoreCondition.WhenWritingNull. And now its different again. This is kinda breaking...

This breaks alot of my connectors, assumptions and tests. Often a called backend service is not that easily to change.

From my current understanding of the code base its now either "WhenWritingDefault" or "Never" depending of the property if "required" or not. And nothing else?

The broken stuff is also subtle. Took me some time to find out the an INT 0 was not send because the whole property was disregarded because it was not required.

@gabrielmaldi
Copy link

I agree with @ThomasTosik, there should be a way to opt-out of the JsonIgnore attribute altogether. For the time being, I removed it this way:

public class EmbeddedTemplateFactory : DefaultTemplateFactory
{
    public EmbeddedTemplateFactory(CSharpClientGeneratorSettings settings)
        : base(settings.CSharpGeneratorSettings, new[] { settings.GetType().Assembly, settings.CSharpGeneratorSettings.GetType().Assembly })
    {
    }

    protected override string GetEmbeddedLiquidTemplate(string language, string template)
    {
        var result = base.GetEmbeddedLiquidTemplate(language, template);

        // Do not include JsonIgnore attributes.
        // https://github.com/RicoSuter/NJsonSchema/blob/e045ae82b55d4e8ae9f6690a27d907c699e3c9ee/src/NJsonSchema.CodeGeneration.CSharp/Templates/Class.liquid#L56
        // https://github.com/RicoSuter/NJsonSchema/blob/e045ae82b55d4e8ae9f6690a27d907c699e3c9ee/src/NJsonSchema.CodeGeneration.CSharp/Models/PropertyModel.cs#L72
        result = result.Replace("property.HasJsonIgnoreCondition", "false");

        return result;
    }
}
var json = await client.GetStringAsync(url);
var document = await OpenApiDocument.FromJsonAsync(json);
var settings = new CSharpClientGeneratorSettings
{
    // ...
};
settings.CSharpGeneratorSettings.TemplateFactory = new EmbeddedTemplateFactory(settings);
var generator = new RefitGenerator(document, settings);
var proxy = generator.GenerateFile();

@rebecca-work
Copy link

Just another +1 for this issue. We are consuming an API outside of our control, that has valid enum values of 0, that aren't marked as required, but we still want to send them, but aren't being serialized and sent.

@binginsin
Copy link

It would be nice to control this behaviour in a better way as there is no default that will suit everyone (except for never ignoring, but that sends unnecessary data over the network).

For people who need desperately need this changed, look into custom templates. At my work I have created a nuget package that contains multiple templates which completely change the generated code (to fit our use which was very non standard). The templates could definitely be used to modify this behaviour and should be quicker to do than change to a different generator.
Once I have time i will look at updating / writing a guide on using templates with MsBuild/OpenApiReference tags.

@gabrielmaldi
Copy link

@rebecca-work @binginsin, you can use the workaround I described in #4012 (comment)

@mooski
Copy link

mooski commented May 23, 2023

I couldn't figure out how to use that workaround - we just use NSwag on the command line (dotnet tool), so I'm not sure how custom templates fit in to that toolchain.

(Also I commented on this earlier but then realised there's another open issue tracking this, so moved my comment there instead: RicoSuter/NJsonSchema#1564)

@espenrl
Copy link
Contributor

espenrl commented Aug 9, 2023

I used this code to circumvent the problem at runtime.

internal partial class UserClient // NSwag generated client
{
    partial void UpdateJsonSerializerSettings(System.Text.Json.JsonSerializerOptions settings)
    {
        settings.TypeInfoResolver = new DefaultJsonTypeInfoResolver
        {
            Modifiers = { JsonHelper.AlwaysSerializeValueTypeProperties }
        };
    }
}

file sealed class JsonHelper
{
    private static readonly Func<object, object?, bool> AlwaysSerialize = (_, __) => true;

    /// <summary>
    /// NSwag generates JsonIgnoreCondition.WhenWritingDefault for value type properties (enum, int, bool, etc.) which is wrong.
    /// </summary>
    public static void AlwaysSerializeValueTypeProperties(JsonTypeInfo ti)
    {
        // https://learn.microsoft.com/en-us/dotnet/standard/serialization/system-text-json/custom-contracts

        foreach (var prop in ti.Properties)
        {
            if (prop.PropertyType.IsValueType && prop.AttributeProvider is not null)
            {
                prop.ShouldSerialize = AlwaysSerialize;
            }
        }
    }
}

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 a pull request may close this issue.