Swashbuckle.AspNetCore icon indicating copy to clipboard operation
Swashbuckle.AspNetCore copied to clipboard

NullReferenceException for Newtonsoft with either of the Polymorph options toggled on when overriding Property handling.

Open esbenbach opened this issue 1 year ago • 2 comments

Version details: PackageVersion 6.4.0 Newtonsoft.Json support (Swashbuckle.AspNetCore.Newtonsoft) dotnet 6

Problem and some background: I get a null reference exception for a property that is "redirected" during de-serialization using Newtonsoft.Json. The code is working but the schema generation fails.

I have some old serialized data that i need to support, so I can't just "fix" the offending class/property - and fixing the serialized data is unfortunately also a last resort thing if we can't find another way that will work (the serialized data will age and be irrelevant within a year or two, but it would be nice to move forward before that!).

Anyway, on to the issue:

I have an abstract base class and derived class something like

public abstract class AccountTransaction
{
    [JsonIgnore] // Related to internal property workaround
    public DateTimeOffset? AccountingTime { get; set; }

    [JsonProperty("AccountingTime")]
    private DateTime? InternalAccountingTime
    {
        get
        {
            return this.AccountingTime.HasValue ? this.AccountingTime.Value.UtcDateTime : (DateTime?)null;
        }

        set
        {
            this.AccountingTime = value == DateTime.MinValue ? null : value;
        }
    }
}

public class Allowance : AccountTransaction
{
  public string Example {get; set;}
}

There are other properties on both, they are exluded for brevity.

When I toggle on either of the polymorphic options ( UseAllOfForInheritance() or UseOneOfForPolymorphism()) i get a NullReferenceException.

It seems like the issue is in the NewtonsoftDataContractResolver when trying to get the MemberInfo from the jsonProperty: jsonProperty.TryGetMemberInfo(out MemberInfo memberInfo); it does not find anything and return null.

This "null" value is then used in dataProperties causing a nullrefence exception further into the schema generation process - seemingly inside SwaggerGen.Schemagenerator.CreateObjectSchame it contains the lines:

                    applicableDataProperties = applicableDataProperties
                        .Where(dataProperty => dataProperty.**MemberInfo**.DeclaringType == dataContract.UnderlyingType);

Where memberInfo is null.

esbenbach avatar Sep 23 '22 10:09 esbenbach

@domaindrivendev - can you think of any way to get arond this issue. Or point me in a direction if I need to do a PR to fix it.

esbenbach avatar Nov 18 '22 07:11 esbenbach

It turns out that if I change the TryGetMemberInfo implementation to include "non-public" properties (using binding flags), everything works as expected. Not really sure how much of a breaking change this may be, so for now im NOT going to create a PR.

Just inform anyone looking that the issue can be solved by duplicating the ContractResolver and the JsonPropertyExtensions classes and then replacing the TryGetMemberInfo with the following implementations

    public static bool TryGetMemberInfo(this JsonProperty jsonProperty, out MemberInfo? memberInfo)
    {
        ArgumentNullException.ThrowIfNull(jsonProperty.UnderlyingName, nameof(jsonProperty.UnderlyingName));
        memberInfo = jsonProperty.DeclaringType?.GetMember(jsonProperty.UnderlyingName, BindingFlags.Instance | BindingFlags.Static | BindingFlags.Public | BindingFlags.NonPublic)?.FirstOrDefault();
        return (memberInfo != null);
    }

esbenbach avatar Nov 21 '22 09:11 esbenbach

This issue is stale because it has been open for 60 days with no activity. It will be automatically closed in 14 days if no further updates are made.

github-actions[bot] avatar Apr 16 '24 01:04 github-actions[bot]

Just from looking at the issue, it seems reasonable to consider non-public members if they are explicitly annotated with something that suggests they should be included in the schema.

martincostello avatar Apr 16 '24 06:04 martincostello

FYI I have been running with a custom resolver since i created the issue. Since its only applied for this specific type I still can't tell if it will break anything else.

In my case, the property in question is annotated with JsonProperty. But if its part of the Json string and there is a corresponding object property, why would the resolver need additional clarification? (other than to not break stuff backwards of course :) )

IF there is a requirement for explicit annotation, what would said annotation involve? Just that there is JsonPropertyAttribute? (or DataMemberAttribute presumably...)

esbenbach avatar Apr 16 '24 06:04 esbenbach

And probably that it's not ignored somehow.

However if you think your use case is niche enough that a custom converter is an acceptable solution then we can just close this and leave things as-is.

martincostello avatar Apr 16 '24 06:04 martincostello

For me its not a big problem either way, but it seems like a bug since serializer and deserializer actually picks it up.

The issue is rather specific since it requires:

  • A private property that is being serialized under a different name.
  • A public property with the same name property name as the serialized version.
  • Either polymorphic option enabled.
  • use of newtonsoft json (not sure how STJ performs here).

Feel free to close it if you think its to edge case, just thought that changing the bindingflags might be a simple fix :)

esbenbach avatar Apr 16 '24 06:04 esbenbach

I think I'll close as you're happy it's an edge case and our backlog is currently...quite large 😅

We can always revisit in the future if there's a compelling new scanerio.

martincostello avatar Apr 16 '24 06:04 martincostello