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

Support the Specified convention for serializing and deserializing in System.Text.Json #40395

Closed
pierslawson opened this issue Aug 5, 2020 · 11 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json wishlist Issue we would like to prioritize, but we can't commit we will get to it yet
Milestone

Comments

@pierslawson
Copy link

pierslawson commented Aug 5, 2020

Background and Motivation

Both the XmlSerializer and @JamesNK 's JSON.Net support the convention that whether a property on a class (say property XYZ) should be serialized or has been deserialized can be controlled/reported through a second property on the class (which is named by convention as XYZSpecified).

See

https://github.com/JamesNK/Newtonsoft.Json/blob/6b9f467e817854532ea31e6c08abe47c53ac8b5c/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalWriter.cs#L533

https://github.com/JamesNK/Newtonsoft.Json/blob/7217c484e9705b5e76585c8b7fcd489c8e021c23/Src/Newtonsoft.Json.Tests/Serialization/ShouldSerializeTests.cs#L217

This allows any code working with the deserialized data to tell the difference between a property that has a default value because the source document didn't have a value, and a document that did have a value but it happened to equal the default value. It also allows any code working with the serializer to optionally decide whether a particular property should be serialized or not (at run time).

This is extremely useful when trying to make an API backwards compatible. Say you add a property "NumberOfChildren" to a"Person" class. New client applications will send in values for how many children the person has, but old client applications won't. However, if your code cannot tell the difference between a new client application setting the property to zero and an old client just not sending the data at all, you could mistakenly overwrite the NumberOfChildren value stored in your database.

Proposed API

Please have System.Text.Json allow the calling code to optionally (at runtime) decide whether a particular property should be serialized or not on an object by object basis. Also allow the calling code to be able to tell if a property's value is set because it was deserialised or because it is the default value for the property.

Usage Examples

During serialization:

SpecifiedTestClass` c = new SpecifiedTestClass();
c.Name = "James";
c.Age = 27;
c.NameSpecified = false;
var json = JsonSerializer.Serialize(c);
StringAssert.AreEqual(@"{
  ""Age"": 27
}", json);

During deserialization:

string mikeFullDisclosureString = "{\"Name\": \"Mike Person\", \"NumberOfChildren\": \"0\"}";
var mike = JsonSerializer.Deserialize<FamilyDetails>(mikeFullDisclosureString);
Assert.AreEqual(true, mike.NumberOfChildrenSpecified);

Alternative Designs

Maybe tere is already a way to achieve this that I have missed.

Risks

Performance overhead of looking for "Specified" properties. Also, most "Specified" properties need to be ignored either by using the JsonIgnore or a convention to ignore them.

@pierslawson pierslawson added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Aug 5, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-Serialization untriaged New issue has not been triaged by the area owner labels Aug 5, 2020
@pierslawson pierslawson changed the title Support the Specified convention for serialising and deserialising in System.Text.Json Support the Specified convention for serializing and deserializing in System.Text.Json Aug 6, 2020
@layomia layomia added this to the Future milestone Aug 17, 2020
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Aug 17, 2020
@rubenprins
Copy link

We have a use-case where we receive import data in JSON form, where the JSON schema supports many fields, but clients usually only send some fields. It's crucial we know which fields are present and which aren't, and neither defaults nor nulls work for this. With Newtonsoft.Json this was a breeze. But migrating to System.Text.Json is a real headache!

Right now, we can get away by setting the Specified properties in the property setters (lots of code bloat), but it's no longer possible to reserialize/roundtrip back to JSON.


This is related to #593 and #50294.

Note that System.Xml and Newtonsoft.Json also support an additional variant via a bool ShouldSerializeXyz() method next to a property Xyz. This method (which is only used for serialization) is treated the same as an XyzSpecified property.

The Specified convention is both for serialization, to emit/omit properties, and deserialization, to flag which properties were present in the payload.

The XyzSpecified convention can largely be emulated by making a property setter also set its buddy Specified property (as long as you don't use AutoMapper et al.), but it's impossible to emulate the Specified convention for serialization via a JsonConverter, as you cannot keep the remainder of the JsonSerializer's functionality without completely reimplementing it (property naming, ordering, property-level converters, null/default handling, cycle detection, etc.)

It looks fairly simple to add this support to ObjectDefaultConverter and JsonPropertyInfo (e.g., making ShouldSerialize a method bool ShouldSerialize(object instance) and handling the Specified setter via the Set action). But everything there is internal and not open to such extensions right now. So not only would you need to reimplement the entire Converter, but also the entire metadata API.

@eiriktsarpalis
Copy link
Member

What benefits would such a convention provide over the using existing JsonIgnoreAttribute? Making migration easier presumably?

@rubenprins
Copy link

@eiriktsarpalis JsonIgnoreAttribute is global, static, and compile-time only; XyzSpecified is a property, and thus it can instruct the serializer to omit properties on a per-instance basis, and is not tied to defaults or null values (a null valued property is different from an omitted optional property in JSON schema).

Also, XyzSpecified is set during deserialization too, so you can inspect the deserialization result to see whether (optional) properties were present in the JSON payload, without needing to fall back to a JsonDocument.

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jan 13, 2022

I believe this could be easily implemented as a third-party extension once #63686 has been completed. We might want to make sure this particular use case is supported, perhaps also document a code sample.

cc @krwq

@eiriktsarpalis eiriktsarpalis added the wishlist Issue we would like to prioritize, but we can't commit we will get to it yet label Jan 13, 2022
@eiriktsarpalis eiriktsarpalis modified the milestones: 7.0.0, Future Jan 13, 2022
@KillerBoogie
Copy link

Another generic option would be to have a special value undefined besides the special value null. This of course requires a major change for .Net CLI and is probably not a practical option.

@KillerBoogie
Copy link

Since .Net 5 there is a solution to the problem that this issue tries to solve using Optional<T> and a Custom Converter. See https://stackoverflow.com/questions/63418549/custom-json-serializer-for-optional-property-with-system-text-json and https://stackoverflow.com/questions/71024060/distinguish-between-null-and-not-present-using-json-merge-patch-with-netcore-web/71074603#71074603

It took me a few weeks to find this solution and effort to adopt it when enable and nullable is set.

Two steps from the .Net team would make it easy to handle this common requirement:

  1. Add Optional<T> from Microsoft.CodeAnalysis to System and extend it to Optional<T?>
  2. Add a standard converter for Optional<T?>

optional: Add a JsonIgnoreCondition 'WhenOptionalIsUndefined' or 'WhenOptionalHasNoValue'

@KillerBoogie
Copy link

My proposal hasn't received any feedback. Is this feature already planned for version 7?

@eiriktsarpalis
Copy link
Member

Per my comment in #40395 (comment), we believe that the feature can be implemented using customized contract resolvers (under proposal #63686). We have listed this particular issue as part of the acceptance criteria.

@KillerBoogie
Copy link

KillerBoogie commented Apr 18, 2022

Thanks for your answer. Since Optionals<T?> are required for any serious project with access permissions or PATCH Merge commands Optionals<T?> should be supported out of the box, without having to write any customized contract resolver.

@eiriktsarpalis
Copy link
Member

The release of the contract customization feature in .NET 7 Preview 6 should make it possible to add support for the Specified convention via a custom contract resolver. Here is a functional test demonstrating how it could be implemented:

[Fact]
public static void SpecifiedContractResolverScenario()
{
var options = new JsonSerializerOptions { TypeInfoResolver = new SpecifiedContractResolver() };
var value = new SpecifiedContractResolver.TestClass { String = "str", Int = 42 };
string json = JsonSerializer.Serialize(value, options);
Assert.Equal("""{}""", json);
value.IntSpecified = true;
json = JsonSerializer.Serialize(value, options);
Assert.Equal("""{"Int":42}""", json);
value.StringSpecified = true;
json = JsonSerializer.Serialize(value, options);
Assert.Equal("""{"String":"str","Int":42}""", json);
}
internal class SpecifiedContractResolver : DefaultJsonTypeInfoResolver
{
public class TestClass
{
public string String { get; set; }
[JsonIgnore]
public bool StringSpecified { get; set; }
public int Int { get; set; }
[JsonIgnore]
public bool IntSpecified { get; set; }
}
public override JsonTypeInfo GetTypeInfo(Type type, JsonSerializerOptions options)
{
JsonTypeInfo jsonTypeInfo = base.GetTypeInfo(type, options);
foreach (JsonPropertyInfo property in jsonTypeInfo.Properties)
{
PropertyInfo? specifiedProperty = type.GetProperty(property.Name + "Specified", BindingFlags.Instance | BindingFlags.Public);
if (specifiedProperty != null && specifiedProperty.CanRead && specifiedProperty.PropertyType == typeof(bool))
{
property.ShouldSerialize = (obj, _) => (bool)specifiedProperty.GetValue(obj);
}
}
return jsonTypeInfo;
}
}

@eiriktsarpalis
Copy link
Member

Per my earlier comment, adding support for the "Specified" convention should be achievable via contract customization starting in .NET 7. We're not planning on adding out-of-the-box support for this, but it should still be possible for third-party extension libraries to ship contract resolvers offering this functionality.

@eiriktsarpalis eiriktsarpalis closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Oct 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json wishlist Issue we would like to prioritize, but we can't commit we will get to it yet
Projects
None yet
Development

No branches or pull requests

7 participants