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

Unwrapping single-field record cases #56

Closed
NickDarvey opened this issue Mar 27, 2020 · 3 comments
Closed

Unwrapping single-field record cases #56

NickDarvey opened this issue Mar 27, 2020 · 3 comments
Labels
enhancement New feature or request released: v0.12

Comments

@NickDarvey
Copy link
Contributor

I'm looking to 'unwrap' a record so that it's properties are at the same level as the case field.

Is there some kinda combination I could set for myOptions which would give me the behaviour I'm testing for here?

type D =
  | Da
  | Db of int
  | Dc of {| x: string; y: bool |}
  | Dd of Dd
and Dd = { x: string; y: bool }

let myOptions = JsonSerializerOptions()
myOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.NamedFields ||| JsonUnionEncoding.UnwrapSingleFieldCases))

[<Fact>]
let ``deserialize unwrapped single-field record cases`` () =
    Assert.Equal(Da, JsonSerializer.Deserialize("""{"Case":"Da"}""", myOptions))
    Assert.Equal(Db 32, JsonSerializer.Deserialize("""{"Case":"Db","Item":32}""", myOptions))
    Assert.Equal(Dc {| x = "test"; y = true |}, JsonSerializer.Deserialize("""{"Case":"Dc","x":"test","y":true}""", myOptions))
    Assert.Equal(Dd { x = "test"; y = true }, JsonSerializer.Deserialize("""{"Case":"Dd","x":"test","y":true}""", myOptions))

[<Fact>]
let ``serialize unwrapped single-field record cases`` () =
    Assert.Equal("""{"Case":"Da"}""", JsonSerializer.Serialize(Da, myOptions))
    Assert.Equal("""{"Case":"Db","Item":32}""", JsonSerializer.Serialize(Db 32, myOptions))
    Assert.Equal("""{"Case":"Dc","x":"test","y":true}""", JsonSerializer.Serialize(Dc {| x = "test"; y = true |}, myOptions))
    Assert.Equal("""{"Case":"Dd","x":"test","y":true}""", JsonSerializer.Serialize(Dd { x = "test"; y = true }, myOptions))
@Tarmil
Copy link
Owner

Tarmil commented Apr 2, 2020

I don't think it's possible with the current options. But this would be a useful feature! It's implemented in WebSharper's JSON library and I've definitely made use of it in the past.

@Tarmil Tarmil added the enhancement New feature or request label Apr 2, 2020
@NickDarvey
Copy link
Contributor Author

NickDarvey commented Apr 14, 2020

I think the implementing the writing might be something like:

/// Extensions (hopefully) coming with .NET 5
/// (https://github.com/dotnet/runtime/issues/31274)
/// from https://github.com/jet/FsCodec/blob/aab8dce0240469351172c43e962aaaa8b57f111f/src/FsCodec.SystemTextJson/JsonSerializerElementExtensions.fs#L21
[<AutoOpen>]
module private JsonSerializerExtensions =
  open System.Runtime.InteropServices

  type private JsonSerializer with
    static member SerializeToElement(value: 'T, [<Optional; DefaultParameterValue(null)>] ?options: JsonSerializerOptions) =
      JsonSerializer.Deserialize<JsonElement>(ReadOnlySpan.op_Implicit(JsonSerializer.SerializeToUtf8Bytes(value, defaultArg options null)))

let writeExpandedSingleField (writer: Utf8JsonWriter) (case: Case) (value: obj) (options: JsonSerializerOptions) =
  writer.WriteStartObject()
  writer.WriteString(fsOptions.UnionTagName, case.Name)
  let element = JsonSerializer.SerializeToElement((case.Dector value).[0], options)
  match element.ValueKind with
  | JsonValueKind.Object ->
    for property in element.EnumerateObject() do property.WriteTo(writer)
  | _ ->
    writer.WritePropertyName(case.Fields.[0].Name)
    element.WriteTo(writer)
  writer.WriteEndObject()

The trickiest part about implementing this enhancement for me is understanding how it should compose with the other options using the JsonUnionEncoding flag enum. I guess I might be having trouble because not all options are expected to compose, like, not every combination is possible (e.g. JsonUnionEncoding.InternalTag doesn't work with JsonUnionEncoding.UnwrapSingleFieldCases, if I understand correctly).

I'm not quite familiar enough with this library to suggest a design here, so if you can recommend how to integrate this with the JsonUnionEncoding options then I'm happy to try to implement it.

(Alternatively, and maybe a different piece of work, perhaps the options could be modelled as a bunch of DU types where only the valid combinations are representable. And/or the JsonUnionConverter is split up so there's not quite so much conditional behaviour.)

@Tarmil
Copy link
Owner

Tarmil commented Apr 16, 2020

I've started an implementation that reuses a chunk of the code for writing records, will create a draft PR soon.

The trickiest part about implementing this enhancement for me is understanding how it should compose with the other options using the JsonUnionEncoding flag enum.

As far as I can tell, it should be possible to use this option any time you have JsonUnionEncoding.NamedFields enabled. But indeed we should be careful about the interaction with the various Unwrap options.

Tarmil added a commit that referenced this issue May 2, 2020
Tarmil added a commit that referenced this issue May 2, 2020
Tarmil added a commit that referenced this issue Jul 21, 2020
Tarmil added a commit that referenced this issue Jul 21, 2020
[#56] Unwrap single-field record cases
@Tarmil Tarmil closed this as completed Jul 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released: v0.12
Projects
None yet
Development

No branches or pull requests

2 participants