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

[API Proposal]: Add Lock() and IsLocked methods to JsonSerializerOptions #72268

Closed
eiriktsarpalis opened this issue Jul 15, 2022 · 5 comments
Closed
Labels
api-ready-for-review API is ready for review, it is NOT ready for implementation area-System.Text.Json Cost:S Work that requires one engineer up to 1 week

Comments

@eiriktsarpalis
Copy link
Member

Background and motivation

The JsonSerializerOptions class supports mutation of its settings, until first used in a serialization operation. After that, the class gets locked and any attempt to update settings will result in InvalidOperationException:

var options = new JsonSerializerOptions();
options.WriteIndented = true; // success, options is mutable
JsonSerializer.Serialize(someValue, options);
options.WriteIndented = true; // throws, options is locked

The issue with this design is that users have no clear indication of whether an options instance is locked or not. Often, users need to serialize a throwaway value just to make sure no further modification can take place. We want to make this state more transparent and controllable by users.

API Proposal

namespace System.Text.Json

public partial class JsonSerializerOptions
{
    public bool IsLocked { get; }
    public void Lock(); // idempotent lock operation
}

API Usage

var options = new JsonSerializerOptions { TypeInfoResolver = new MyFancyResolver(), WriteIndented = true };

Console.WriteLine(options.IsLocked); // False
var converter = options.GetConverter(typeof(MyPoco)); // Returns an uncached converter instance

options.Lock(); // lock the instance and initialize the metadata cache

Console.WriteLine(options.IsLocked); // True
converter = options.GetConverter(typeof(MyPoco)); // Returns a cached converter instance

Alternative Designs

We might want to consider exposing equivalent functionality to JsonTypeInfo and JsonPropertyInfo, whose design uses a similar locking design.

Risks

No response

@eiriktsarpalis eiriktsarpalis added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jul 15, 2022
@eiriktsarpalis eiriktsarpalis added this to the 8.0.0 milestone Jul 15, 2022
@ghost
Copy link

ghost commented Jul 15, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

The JsonSerializerOptions class supports mutation of its settings, until first used in a serialization operation. After that, the class gets locked and any attempt to update settings will result in InvalidOperationException:

var options = new JsonSerializerOptions();
options.WriteIndented = true; // success, options is mutable
JsonSerializer.Serialize(someValue, options);
options.WriteIndented = true; // throws, options is locked

The issue with this design is that users have no clear indication of whether an options instance is locked or not. Often, users need to serialize a throwaway value just to make sure no further modification can take place. We want to make this state more transparent and controllable by users.

API Proposal

namespace System.Text.Json

public partial class JsonSerializerOptions
{
    public bool IsLocked { get; }
    public void Lock(); // idempotent lock operation
}

API Usage

var options = new JsonSerializerOptions { TypeInfoResolver = new MyFancyResolver(), WriteIndented = true };

Console.WriteLine(options.IsLocked); // False
var converter = options.GetConverter(typeof(MyPoco)); // Returns an uncached converter instance

options.Lock(); // lock the instance and initialize the metadata cache

Console.WriteLine(options.IsLocked); // True
converter = options.GetConverter(typeof(MyPoco)); // Returns a cached converter instance

Alternative Designs

We might want to consider exposing equivalent functionality to JsonTypeInfo and JsonPropertyInfo, whose design uses a similar locking design.

Risks

No response

Author: eiriktsarpalis
Assignees: -
Labels:

api-suggestion, area-System.Text.Json

Milestone: 8.0.0

@eiriktsarpalis eiriktsarpalis added Cost:S Work that requires one engineer up to 1 week api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jul 15, 2022
@svick
Copy link
Contributor

svick commented Jul 15, 2022

This has been previously discussed at #54482. I think one of the two issues should be closed.

Also, I stand by the comment I made there saying that Freeze would be a better name for this. (Especially since Lock in .Net usually implies mutual exclusion, and this is not that.)

@eiriktsarpalis
Copy link
Member Author

eiriktsarpalis commented Jul 15, 2022

Also, I stand by the comment I made there saying that Freeze would be a better name for this. (Especially since Lock in .Net usually implies mutual exclusion, and this is not that.)

That's a good point, although I've never heard of "Freeze" being used in this context. Any other alternatives? One that I can think of is IsImmutable/MakeImmutable().

This has been previously discussed at #54482.

That's my bad, I should have gone through the backlog before opening this.

@Clockwork-Muse
Copy link
Contributor

Any other alternatives?

Build?

@eiriktsarpalis
Copy link
Member Author

Closing as duplicate of #54482.

@eiriktsarpalis eiriktsarpalis removed this from the 8.0.0 milestone Jul 15, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Aug 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-ready-for-review API is ready for review, it is NOT ready for implementation area-System.Text.Json Cost:S Work that requires one engineer up to 1 week
Projects
None yet
Development

No branches or pull requests

3 participants