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

Add possibility to ignore JsonElement during serializing. #512

Closed
bjornen77 opened this issue Dec 4, 2019 · 5 comments
Closed

Add possibility to ignore JsonElement during serializing. #512

bjornen77 opened this issue Dec 4, 2019 · 5 comments

Comments

@bjornen77
Copy link
Contributor

bjornen77 commented Dec 4, 2019

I am trying to use the new System.Text.Json to Serialize and Deserialize messages based on the JSONRPC2.0 specification(https://www.jsonrpc.org/specification). In the specification there are JSON properties that may be omitted, for example the params property.

The params property value could be any JSON value, which I think maps good to the JsonElement type.

However, when serializing, and the params property is omitted, an InvalidOperationException occurs. Is this by design? Or would it be a good idea to treat the ValuKind.Undefined the same as a null value with IgnoreNullValues=true? Perhaps a new setting IgnoreUndefinedValues=true?

I created a simple example program to reproduce this issue:

class Program
{
    public static void Main(string[] args)
    {
        var request = new Request();
        var json = JsonSerializer.Serialize(request, new JsonSerializerOptions() {IgnoreNullValues = true});
    }

}
class Request
{
    public string Method { get; set; } = "Test";
    public JsonElement Params { get; set; }
}

Using JsonElement? or object instead works, but I think that using JsonElement will be best if possible. Because, If using object instead, I must unbox it to a JsonElement after deserializing.

@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Dec 4, 2019
@bjornen77 bjornen77 changed the title Serializing JsonElement with ValueKind.Unknown throws InvalidOperationException. Serializing JsonElement with ValueKind.Undefined throws InvalidOperationException. Dec 4, 2019
@bjornen77 bjornen77 changed the title Serializing JsonElement with ValueKind.Undefined throws InvalidOperationException. Add possibility to ignore JsonElement during serializing. Dec 4, 2019
@bjornen77
Copy link
Contributor Author

To summarize: The suggestion is to treat a JsonElement property with no value(JsonValueKind.Undefined) the same as a property with a null value, when JsonSerializerOption IgnoreNullValues=true (or adding a new JsonSerializerOption, for example IgnoreUndefinedValues=true).

@layomia
Copy link
Contributor

layomia commented Dec 12, 2019

A JsonElement is a value type and can't be null even when it has no value, so I don't think the serializer should treat it as such. cc @ahsonkhan

Since you want to ignore Params on serialization when default, an option is to change the type to JsonElement? and use options.IgnoreNullValues.

As you mention in #779 (comment), conditional ignore semantics, global and/or per-property, "ignore when default", would also help here.

@ahsonkhan
Copy link
Member

I agree. Is there a reason why you can't use JsonElement? here?

This specific issue isn't actionable and we have existing issues to enable more flexible ignore semantics, so I consider this issue as a duplicate of the others that @layomia linked to.

@ahsonkhan ahsonkhan removed the untriaged New issue has not been triaged by the area owner label Dec 12, 2019
@ahsonkhan ahsonkhan added this to the 5.0 milestone Dec 12, 2019
@bjornen77
Copy link
Contributor Author

@ahsonkhan There is no reason why I can´t use JsonElement? at the moment and this is what I ended up doing. If there will be other possibilities in the future I could change this(if it will be better in some way).

I would like to thank you all for the great work you do with dotnet, I also think the new S.T.J is a good addition to the platform and it works great!

@ahsonkhan
Copy link
Member

There is no reason why I can´t use JsonElement? at the moment and this is what I ended up doing. If there will be other possibilities in the future I could change this(if it will be better in some way).

Great. That sounds good.

I would like to thank you all for the great work you do with dotnet, I also think the new S.T.J is a good addition to the platform and it works great!

Appreciate it. We have a few up-for-grabs issues if you are interested in contributing :)
https://github.com/dotnet/corefx/issues?q=is%3Aopen+is%3Aissue+label%3Aup-for-grabs+label%3Aarea-System.Text.Json

@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants