-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[API Proposal]: Support Custom Data on JsonSerializerOptions
#71718
Comments
Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis Issue DetailsBackground and motivationWhile working on a client library for Elasticsearch, making heavy use of System.Text.Json, a common issue I have run into is the need to access some additional state inside custom converters. The Elasticsearch client has a settings class Currently, this forces an instance of each converter which requires access to the settings to be created in advance and added to the I have come up with a rather nasty workaround where I register a converter purely to hold state, which can then be retrieved from the options to gain access to the settings. internal sealed class ExtraSerializationData : JsonConverter<ExtraSerializationData>
{
public ExtraSerializationData(IElasticsearchClientSettings settings) => Settings = settings;
public IElasticsearchClientSettings Settings { get; }
public override ExtraSerializationData? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) => throw new NotImplementedException();
public override void Write(Utf8JsonWriter writer, ExtraSerializationData value, JsonSerializerOptions options) => throw new NotImplementedException();
} This can then be accessed in the public override void Write(Utf8JsonWriter writer, SortOptions value, JsonSerializerOptions options)
{
writer.WriteStartObject();
if (value.AdditionalPropertyName is IUrlParameter urlParameter)
{
var extraData = options.GetConverter(typeof(ExtraSerializationData)) as ExtraSerializationData;
var propertyName = urlParameter.GetString(extraData.Settings);
writer.WritePropertyName(propertyName);
}
else
{
throw new JsonException();
}
JsonSerializer.Serialize(writer, value.Variant, value.Variant.GetType(), options);
writer.WriteEndObject();
} API ProposalI would like to propose adding support to attach extra custom data (a property bag) to public sealed class JsonSerializerOptions
{
public IReadOnlyDictionary<string, object> CustomData { get; set; } = new Dictionary<string, object>(0); // some cached default empty value
} This adds a property to act as a property bag for user-provided data. I'm proposing this be exposed as API UsageDefine options with custom data: var options = new JsonSerializerOptions
{
CustomData = new Dictionary<string, object> {{ "settings", new Settings() }}
} Access within a custom converter: public override void Write(Utf8JsonWriter writer, SortOptions value, JsonSerializerOptions options)
{
writer.WriteStartObject();
if (value.AdditionalPropertyName is IUrlParameter urlParameter && options.CustomData.TryGetValue("settings", out var settings))
{
var propertyName = urlParameter.GetString(settings);
writer.WritePropertyName(propertyName);
}
else
{
throw new JsonException();
}
JsonSerializer.Serialize(writer, value.Variant, value.Variant.GetType(), options);
writer.WriteEndObject();
} Alternative DesignsAlternative DesignIf the RisksThis could potentially be misused to hold mutable types which could cause thread-safety issues. Documentation could be added to guide the intended usage.
|
It's a reasonable requirement, but I don't think Related to #63795, we've been thinking about exposing async converter primitives which would also involve exposing the internal |
Thanks for the feedback @eiriktsarpalis. To be clear, in our use case, our There's already potential for misuse if developers create new instances of the options per serialization operation. I'm not sure this promotes that much more for the common scenario of attaching additional context needed for many/all serialization in converters. I'll have to dig into the proposal for async converters to understand if custom state there would meet our need. Would we replace the existing converters with async ones? Would those apply to all scenarios, even synchronous serializer methods? |
It would most likely involve exposing a couple of virtual methods in
Yes, the converter methods that pass serialization state are used in both synchronous and asynchronous serialization. It just happens that async converters are implemented by passing their state machines via the serialization state objects. |
Do you have an example of what this would look like? |
Feature needs prototyping before we can propose something concrete, but roughly speaking it might look as follows: var writer = new Utf8JsonWriter();
var state = new WriteState { UserState = new MyState() };
string json = JsonSerializer.Serialize(writer, new Foo(), ref state);
public class MyConverter : JsonResumableConverter<Foo>
{
public bool TryWrite(Utf8JsonWriter writer, Foo value, JsonSerializerOptions options, ref WriteState state)
{
var myState = (MyState)state.UserState;
/* serialization logic using state */
}
} where |
What happens when you don't control the callsite responsible for calling Serialize/Deserailize (like when using a framework)? Attaching this to options seems like the logical place since it:
|
That might be an option, although given the nature of |
@stevejgordon in the meantime, have you considered using a |
I've often wanted the inheritance model, using |
@eiriktsarpalis I hadn't considered that, only because I was completely unaware of its existence! :-) Looks like it could work for our requirement based on a small sample I put together this morning. Is this essentially how you'd expect a solution to look in such a case? using System.Runtime.CompilerServices;
using System.Text.Json;
using System.Text.Json.Serialization;
using Test;
var clientOne = new Client(new ClientSettings { Value = 1 });
var clientTwo = new Client(new ClientSettings { Value = 2 });
var data = new MyTypeToSerialize { Name = "Test" };
var resultOne = clientOne.Serializer.Serialize(data);
var resultTwo = clientTwo.Serializer.Serialize(data);
Console.WriteLine(resultOne);
Console.WriteLine(resultTwo);
Console.ReadKey();
namespace Test
{
public class Client
{
internal static ConditionalWeakTable<JsonSerializerOptions, ClientSettings> SettingsTable { get; } = new();
public Client(ClientSettings settings)
{
Settings = settings;
Serializer = new MySerializer();
SettingsTable.Add(Serializer.Options, Settings);
}
public ClientSettings Settings { get; }
public MySerializer Serializer { get; }
}
public class ClientSettings
{
public int Value { get; init; }
}
public class MySerializer
{
public MySerializer() => Options = new JsonSerializerOptions();
public JsonSerializerOptions Options { get; }
public string Serialize<T>(T value) => JsonSerializer.Serialize(value, Options);
}
[JsonConverter(typeof(MyTypeToSerializeConverter))]
public class MyTypeToSerialize
{
public string? Name { get; set; }
}
internal sealed class MyTypeToSerializeConverter : JsonConverter<MyTypeToSerialize>
{
public override MyTypeToSerialize? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options) =>
throw new NotImplementedException();
public override void Write(Utf8JsonWriter writer, MyTypeToSerialize value, JsonSerializerOptions options)
{
writer.WriteStartObject();
writer.WritePropertyName("name");
writer.WriteStringValue(value.Name ?? string.Empty);
if (Client.SettingsTable.TryGetValue(options, out var settings))
{
writer.WritePropertyName("settingsValue");
writer.WriteNumberValue(settings.Value);
}
writer.WriteEndObject();
}
}
} Output:
|
That's it more or less. I would probably also hide some of the implementation details behind extension methods: public static class Client
{
private static ConditionalWeakTable<JsonSerializerOptions, ClientSettings> SettingsTable { get; } = new();
public static ClientSettings? GetClientSettings(this JsonSerializerOptions options)
=> SettingsTable.TryGetValue(options, out var settings) ? settings : null;
public static void SetClientSettings(this JsonSerializerOptions options, ClientSettings value) => SettingsTable.Add(options, value);
} |
Cool, thanks for the suggestion @eiriktsarpalis. Our client can't be static though. I'll likely switch to something based on |
Related to #59892 (comment). While the callback interfaces do not handle state, we might want to consider extending the callbacks in the contract model so that any user-defined state can be handled appropriately. |
Closing this in favor of #63795 |
@eiriktsarpalis Finally completed the blog post which describes the path to the final solution for my use case. |
Related: #34773 |
I don't think #63795 addresses the requirements described in this issue, as explained in this comment.
Maybe not for a particular serialization operation, but what about "all serialization operations using this JsonSerializationOptions instance"? That's a legitimate use case, and currently the only ways to do it are cumbersome, to say the least (see @stevejgordon's blog post, or mine). The |
@thomaslevesque thanks, I hadn't read your article before. It seems like the primary use case preventing you from injecting state directly to converters is I would guess that unsealing public class JsonSerializerOptions
{
public object? UserData { get; set; } // As with other options properties, gets locked once used
} This should work fine assuming the app author owns all custom converters, but we might want to consider a design where multiple converter designs interoperate: public class JsonSerializerOptions
{
public IDictionary<string, object?> UserData { get; } // As with other options properties, gets locked once used
} |
@stevejgordon you might want to consider using a internal sealed class WildcardQueryConverter : JsonConverterFactory
{
public override bool CanConvert(Type type) => type == typeof(WildcardQuery);
public override JsonConverter? CreateConverter(Type type, JsonSerializerOptions options)
{
options.TryGetClientSettings(out var settings);
// same implementation as the original converter accepting `settings` parametrically
// the new instance is scoped to the current `JsonSerializerOptions` so it is safe to hardcode.
return new WildcardQueryConverterImplementation(settings);
}
} |
@eiriktsarpalis That's a good shout, thanks. I will review how easy that is to achieve with our code-gen. |
Thanks for your reply @eiriktsarpalis!
In this case, yes. I can't just add the converter to the
No, because I don't control the instantiation of the
It's not perfect, but it would work. Your next suggestion with a property bag is probably better. |
Interesting thread as it represents the long waiting option before i can fully remove the My use case : i want to modify some information depending context. Example : hide emails for non essentials partners. Newtonsoft
sample code (copy/paste to roslynpad) #r "nuget:Newtonsoft.Json/13.0.3"
using System.Runtime.Serialization;
using Newtonsoft.Json;
var user = new User() {
usr_mail = "login@domain.tld",
usr_nickname = "nickname"
};
var context = "GDPR"; // pass serialization context
var settings = new JsonSerializerSettings {
Context = new StreamingContext(StreamingContextStates.Other, context)
};
JsonConvert.SerializeObject(user, settings).Dump();
// {"usr_mail":null,"usr_nickname":"nickname"}
public class User {
public string usr_mail { get; set; }
public string usr_nickname { get; set; }
[OnSerializing]
internal void OnSerializedMethod(StreamingContext context) {
// retrieve serialization context and do stuff
if (context.Context is string ctx && ctx == "GDPR") {
usr_mail = null;
}
}
} System.Text.JsonThe current best effort is in NET7 : i can hide same field but not depending the context as i does not exist. sample code (copy/paste to roslynpad) #r "nuget: System.Text.Json, 7.0.3"
using System.Runtime.Serialization;
using System.Text.Json.Serialization;
using System.Text.Json;
var user = new User() {
usr_mail = "login@domain.tld",
usr_nickname = "nickname"
};
JsonSerializer.Serialize(user).Dump();
// {"usr_mail":null,"usr_nickname":"nickname"}
public class User : IJsonOnSerializing {
public string usr_mail { get; set; }
public string usr_nickname { get; set; }
void IJsonOnSerializing.OnSerializing() {
usr_mail = null;
}
} If some of you have a workaround, i will appreciate 😀 |
Background and motivation
While working on a client library for Elasticsearch, making heavy use of System.Text.Json, a common issue I have run into is the need to access some additional state inside custom converters. The Elasticsearch client has a settings class
ElasticsearchClientSettings
which includes state used to infer values for certain types at runtime.Currently, this forces an instance of each converter which requires access to the settings to be created in advance and added to the
JsonSerializerOptions
(example). These converters can then accept anElasticsearchClientSettings
instance via their constructor. Some of these instances may never be needed if the types they (de)serialize are not used by consumers of our library. Additionally, we have some converters which we code generate which makes creating such instances and adding them to theJsonSerializerOptions
more complicated.I have come up with a rather nasty workaround where I register a converter purely to hold state, which can then be retrieved from the options to gain access to the settings.
This can then be accessed in the
Write
method of a custom converter:API Proposal
I would like to propose adding support to attach extra custom data (a property bag) to
JsonSerializerOptions
, to be accessible inside theRead
andWrite
methods of custom converters derived fromJsonConveter<T>
by accessing theJsonSerializerOptions
.This adds a property to act as a property bag for user-provided data. I'm proposing this be exposed as
IReadOnlyDictionary
to avoid new items being added/modified after the options become immutable. It could beIDictionary
to support scenarios where some converters need to add data for subsequent use, although that sounds risky. I imagine this should be initialised with a cached empty dictionary in cases where the user does not explicitly set this.API Usage
Define options with custom data:
Access within a custom converter:
Alternative Designs
If the
JsonSerializerOptions
were unsealed, that could also support my scenario. We could define a subclass ofJsonSerializerOptions
with extra properties. Inside our converters that require that data, they could try casting to the derived type and access any necessary extra properties.Risks
This could potentially be misused to hold mutable types which could cause thread-safety issues. Documentation could be added to guide the intended usage.
The text was updated successfully, but these errors were encountered: