From 6550b7269291228c426d436ac52e8df6d2520620 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Thu, 8 Apr 2021 01:53:38 +0300 Subject: [PATCH 1/6] Implemented ability to deserialize discriminated unions regardless of union tag position --- src/FSharp.SystemTextJson/Helpers.fs | 5 ++- src/FSharp.SystemTextJson/Union.fs | 46 ++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/FSharp.SystemTextJson/Helpers.fs b/src/FSharp.SystemTextJson/Helpers.fs index 9a99e19..2a0094e 100644 --- a/src/FSharp.SystemTextJson/Helpers.fs +++ b/src/FSharp.SystemTextJson/Helpers.fs @@ -19,8 +19,11 @@ let readExpecting expectedTokenType expectedLabel (reader: byref if not (reader.Read()) || reader.TokenType <> expectedTokenType then fail expectedLabel &reader ty +let inline readIsExpectingPropertyNamed (expectedPropertyName: string) (reader: byref) ty = + (reader.Read()) && reader.TokenType = JsonTokenType.PropertyName && (reader.ValueTextEquals expectedPropertyName) + let readExpectingPropertyNamed (expectedPropertyName: string) (reader: byref) ty = - if not (reader.Read()) || reader.TokenType <> JsonTokenType.PropertyName || not (reader.ValueTextEquals expectedPropertyName) then + if not <| readIsExpectingPropertyNamed expectedPropertyName &reader ty then fail ("\"" + expectedPropertyName + "\"") &reader ty let isNullableUnion (ty: Type) = diff --git a/src/FSharp.SystemTextJson/Union.fs b/src/FSharp.SystemTextJson/Union.fs index f721ac5..563e5e7 100644 --- a/src/FSharp.SystemTextJson/Union.fs +++ b/src/FSharp.SystemTextJson/Union.fs @@ -5,6 +5,7 @@ open System.Collections.Generic open System.Text.Json open FSharp.Reflection open System.Text.Json.Serialization.Helpers +open System.Buffers type private Field = { @@ -153,7 +154,7 @@ type JsonUnionConverter<'T> else ValueNone - let getCaseByTag (reader: byref) = + let getCaseByTag tag = let found = match casesByName with | ValueNone -> @@ -161,18 +162,18 @@ type JsonUnionConverter<'T> let mutable i = 0 while found.IsNone && i < cases.Length do let case = cases.[i] - if reader.ValueTextEquals(case.Name) then + if case.Name.Equals(tag, StringComparison.InvariantCulture) then found <- ValueSome case else i <- i + 1 found | ValueSome d -> - match d.TryGetValue(reader.GetString()) with + match d.TryGetValue(tag) with | true, c -> ValueSome c | false, _ -> ValueNone match found with | ValueNone -> - raise (JsonException("Unknown case for union type " + ty.FullName + ": " + reader.GetString())) + raise (JsonException("Unknown case for union type " + ty.FullName + ": " + tag)) | ValueSome case -> case @@ -286,11 +287,32 @@ type JsonUnionConverter<'T> else readFieldsAsArray &reader case options + let getCaseFromValue (reader: byref) = + let document = JsonDocument.ParseValue(&reader) + getCaseByTag (document.RootElement.ToString()) + + let getCaseFromDocument (reader: Utf8JsonReader) = + let mutable reader = reader + let document = JsonDocument.ParseValue(&reader) + match document.RootElement.TryGetProperty fsOptions.UnionTagName with + | true, element -> getCaseByTag (element.GetString()) + | false, _ -> + sprintf "Failed to find union case field for %s: expected %s" ty.FullName fsOptions.UnionFieldsName + |> JsonException + |> raise + + let getCase (reader: byref) = + let mutable snapshot = reader + if readIsExpectingPropertyNamed fsOptions.UnionTagName &snapshot ty then + readExpectingPropertyNamed fsOptions.UnionTagName &reader ty + readExpecting JsonTokenType.String "case name" &reader ty + getCaseFromValue &reader + else + getCaseFromDocument reader + let readAdjacentTag (reader: byref) (options: JsonSerializerOptions) = expectAlreadyRead JsonTokenType.StartObject "object" &reader ty - readExpectingPropertyNamed fsOptions.UnionTagName &reader ty - readExpecting JsonTokenType.String "case name" &reader ty - let case = getCaseByTag &reader + let case = getCase &reader let res = if case.Fields.Length > 0 then readExpectingPropertyNamed fsOptions.UnionFieldsName &reader ty @@ -303,7 +325,7 @@ type JsonUnionConverter<'T> let readExternalTag (reader: byref) (options: JsonSerializerOptions) = expectAlreadyRead JsonTokenType.StartObject "object" &reader ty readExpecting JsonTokenType.PropertyName "case name" &reader ty - let case = getCaseByTag &reader + let case = getCaseByTag (reader.GetString()) let res = readFields &reader case options readExpecting JsonTokenType.EndObject "end of object" &reader ty res @@ -311,14 +333,12 @@ type JsonUnionConverter<'T> let readInternalTag (reader: byref) (options: JsonSerializerOptions) = if namedFields then expectAlreadyRead JsonTokenType.StartObject "object" &reader ty - readExpectingPropertyNamed fsOptions.UnionTagName &reader ty - readExpecting JsonTokenType.String "case name" &reader ty - let case = getCaseByTag &reader + let case = getCase &reader readFieldsAsRestOfObject &reader case false options else expectAlreadyRead JsonTokenType.StartArray "array" &reader ty readExpecting JsonTokenType.String "case name" &reader ty - let case = getCaseByTag &reader + let case = getCaseFromValue &reader readFieldsAsRestOfArray &reader case options let readUntagged (reader: byref) (options: JsonSerializerOptions) = @@ -408,7 +428,7 @@ type JsonUnionConverter<'T> | JsonTokenType.Null when Helpers.isNullableUnion ty -> (null : obj) :?> 'T | JsonTokenType.String when unwrapFieldlessTags -> - let case = getCaseByTag &reader + let case = getCaseFromValue &reader case.Ctor [||] :?> 'T | _ -> match baseFormat with From 790e69fe9271d65fe3966589f13c9994878dd8b2 Mon Sep 17 00:00:00 2001 From: Loic Denuziere Date: Sun, 18 Apr 2021 20:27:48 +0200 Subject: [PATCH 2/6] Add tests for deserializing discriminated unions out of order --- .../FSharp.SystemTextJson.Tests/Test.Union.fs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/FSharp.SystemTextJson.Tests/Test.Union.fs b/tests/FSharp.SystemTextJson.Tests/Test.Union.fs index 6ea2ed0..192378c 100644 --- a/tests/FSharp.SystemTextJson.Tests/Test.Union.fs +++ b/tests/FSharp.SystemTextJson.Tests/Test.Union.fs @@ -17,6 +17,7 @@ module NonStruct = Assert.Equal(Aa, JsonSerializer.Deserialize """{"Case":"Aa"}""") Assert.Equal(Ab 32, JsonSerializer.Deserialize """{"Case":"Ab","Fields":[32]}""") Assert.Equal(Ac("test", true), JsonSerializer.Deserialize """{"Case":"Ac","Fields":["test",true]}""") + Assert.Equal(Ac("test", true), JsonSerializer.Deserialize """{"Fields":["test",true],"Case":"Ac"}""") [] let ``serialize via explicit converter`` () = @@ -37,6 +38,7 @@ module NonStruct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"Ba"}""", options)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"Bb","Fields":[32]}""", options)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"Bc","Fields":["test",true]}""", options)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":["test",true],"Case":"Bc"}""", options)) [] let ``serialize via options`` () = @@ -67,6 +69,7 @@ module NonStruct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"ba"}""", tagPolicyOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"bb","Fields":[32]}""", tagPolicyOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"bc","Fields":["test",true]}""", tagPolicyOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":["test",true],"Case":"bc"}""", tagPolicyOptions)) [] let ``serialize AdjacentTag with tag policy`` () = @@ -82,6 +85,7 @@ module NonStruct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"bA"}""", tagCaseInsensitiveOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"bB", "Fields":[32]}""", tagCaseInsensitiveOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"bC", "Fields":["test",true]}""", tagCaseInsensitiveOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":["test",true],"Case":"bC"}""", tagCaseInsensitiveOptions)) [] let ``serialize AdjacentTag with case insensitive tag`` () = @@ -98,6 +102,7 @@ module NonStruct = let ``deserialize UseNull`` () = Assert.Equal(Ca, JsonSerializer.Deserialize("""null""", options)) Assert.Equal(Cb 32, JsonSerializer.Deserialize("""{"Case":"Cb","Fields":[32]}""", options)) + Assert.Equal(Cb 32, JsonSerializer.Deserialize("""{"Fields":[32],"Case":"Cb"}""", options)) [] let ``serialize UseNull`` () = @@ -187,6 +192,7 @@ module NonStruct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"Ba"}""", adjacentTagNamedFieldsOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"Bb","Fields":{"Item":32}}""", adjacentTagNamedFieldsOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"Bc","Fields":{"x":"test","Item2":true}}""", adjacentTagNamedFieldsOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":{"x":"test","Item2":true},"Case":"Bc"}""", adjacentTagNamedFieldsOptions)) [] let ``serialize AdjacentTag NamedFields`` () = @@ -202,6 +208,7 @@ module NonStruct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"ba"}""", adjacentTagNamedFieldsTagPolicyOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"bb","Fields":{"Item":32}}""", adjacentTagNamedFieldsTagPolicyOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"bc","Fields":{"x":"test","Item2":true}}""", adjacentTagNamedFieldsTagPolicyOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":{"x":"test","Item2":true},"Case":"bc"}""", adjacentTagNamedFieldsTagPolicyOptions)) [] let ``serialize AdjacentTag NamedFields with tag policy`` () = @@ -247,6 +254,7 @@ module NonStruct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"Ba"}""", internalTagNamedFieldsOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"Bb","Item":32}""", internalTagNamedFieldsOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"Bc","x":"test","Item2":true}""", internalTagNamedFieldsOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"x":"test","Item2":true,"Case":"Bc"}""", internalTagNamedFieldsOptions)) [] let ``serialize InternalTag NamedFields`` () = @@ -262,6 +270,7 @@ module NonStruct = Assert.Equal(S(1,Skip,Skip,Skip), JsonSerializer.Deserialize("""{"Case":"S","a":1}""", internalTagNamedFieldsOptions)) Assert.Equal(S(1,Include 2,Include None,Include ValueNone), JsonSerializer.Deserialize("""{"Case":"S","a":1,"b":2,"c":null,"d":null}""", internalTagNamedFieldsOptions)) Assert.Equal(S(1,Include 2,Include(Some 3),Include(ValueSome 4)), JsonSerializer.Deserialize("""{"Case":"S","a":1,"b":2,"c":3,"d":4}""", internalTagNamedFieldsOptions)) + Assert.Equal(S(1,Include 2,Include(Some 3),Include(ValueSome 4)), JsonSerializer.Deserialize("""{"a":1,"b":2,"Case":"S","c":3,"d":4}""", internalTagNamedFieldsOptions)) [] let ``serialize InternalTag NamedFields with Skippable fields`` () = @@ -277,6 +286,7 @@ module NonStruct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"ba"}""", internalTagNamedFieldsTagPolicyOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"bb","Item":32}""", internalTagNamedFieldsTagPolicyOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"bc","x":"test","Item2":true}""", internalTagNamedFieldsTagPolicyOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"x":"test","Case":"bc","Item2":true}""", internalTagNamedFieldsTagPolicyOptions)) [] let ``serialize InternalTag NamedFields with tag policy`` () = @@ -292,6 +302,7 @@ module NonStruct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"type":"Ba"}""", internalTagNamedFieldsConfiguredTagOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"type":"Bb","Item":32}""", internalTagNamedFieldsConfiguredTagOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"type":"Bc","x":"test","Item2":true}""", internalTagNamedFieldsConfiguredTagOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"x":"test","type":"Bc","Item2":true}""", internalTagNamedFieldsConfiguredTagOptions)) [] let ``serialize InternalTag NamedFields alternative Tag`` () = @@ -307,6 +318,7 @@ module NonStruct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"type":"Ba"}""", adjacentTagNamedFieldsConfiguredFieldsOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"type":"Bb","args":[32]}""", adjacentTagNamedFieldsConfiguredFieldsOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"type":"Bc","args":["test",true]}""", adjacentTagNamedFieldsConfiguredFieldsOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"args":["test",true],"type":"Bc"}""", adjacentTagNamedFieldsConfiguredFieldsOptions)) [] let ``serialize AdjacentTag NamedFields alternative Fields`` () = @@ -650,6 +662,7 @@ module Struct = Assert.Equal(Aa, JsonSerializer.Deserialize """{"Case":"Aa"}""") Assert.Equal(Ab 32, JsonSerializer.Deserialize """{"Case":"Ab","Fields":[32]}""") Assert.Equal(Ac("test", true), JsonSerializer.Deserialize """{"Case":"Ac","Fields":["test",true]}""") + Assert.Equal(Ac("test", true), JsonSerializer.Deserialize """{"Fields":["test",true],"Case":"Ac"}""") [] let ``serialize via explicit converter`` () = @@ -671,6 +684,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"Ba"}""", options)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"Bb","Fields":[32]}""", options)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"Bc","Fields":["test",true]}""", options)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":["test",true],"Case":"Bc"}""", options)) [] let ``serialize via options`` () = @@ -714,6 +728,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"ba"}""", tagPolicyOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"bb","Fields":[32]}""", tagPolicyOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"bc","Fields":["test",true]}""", tagPolicyOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":["test",true],"Case":"bc"}""", tagPolicyOptions)) [] let ``serialize AdjacentTag with tag policy`` () = @@ -729,6 +744,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"bA"}""", tagCaseInsensitiveOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"bB", "Fields":[32]}""", tagCaseInsensitiveOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"bC", "Fields":["test",true]}""", tagCaseInsensitiveOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":["test",true], "Case":"bC"}""", tagCaseInsensitiveOptions)) [] let ``serialize AdjacentTag with case insensitive tag`` () = @@ -819,6 +835,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"Ba"}""", adjacentTagNamedFieldsOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"Bb","Fields":{"Item":32}}""", adjacentTagNamedFieldsOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"Bc","Fields":{"x":"test","Item2":true}}""", adjacentTagNamedFieldsOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":{"x":"test","Item2":true},"Case":"Bc"}""", adjacentTagNamedFieldsOptions)) [] let ``serialize AdjacentTag NamedFields`` () = @@ -834,6 +851,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"ba"}""", adjacentTagNamedFieldsTagPolicyOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"bb","Fields":{"Item":32}}""", adjacentTagNamedFieldsTagPolicyOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"bc","Fields":{"x":"test","Item2":true}}""", adjacentTagNamedFieldsTagPolicyOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":{"x":"test","Item2":true},"Case":"bc"}""", adjacentTagNamedFieldsTagPolicyOptions)) [] let ``serialize AdjacentTag NamedFields with tag policy`` () = @@ -879,6 +897,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"Ba"}""", internalTagNamedFieldsOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"Bb","Item":32}""", internalTagNamedFieldsOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"Bc","x":"test","Item2":true}""", internalTagNamedFieldsOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"x":"test","Case":"Bc","Item2":true}""", internalTagNamedFieldsOptions)) [] let ``serialize InternalTag NamedFields`` () = @@ -895,6 +914,7 @@ module Struct = Assert.Equal(S(1,Skip,Skip,Skip), JsonSerializer.Deserialize("""{"Case":"S","a":1}""", internalTagNamedFieldsOptions)) Assert.Equal(S(1,Include 2,Include None,Include ValueNone), JsonSerializer.Deserialize("""{"Case":"S","a":1,"b":2,"c":null,"d":null}""", internalTagNamedFieldsOptions)) Assert.Equal(S(1,Include 2,Include(Some 3),Include(ValueSome 4)), JsonSerializer.Deserialize("""{"Case":"S","a":1,"b":2,"c":3,"d":4}""", internalTagNamedFieldsOptions)) + Assert.Equal(S(1,Include 2,Include(Some 3),Include(ValueSome 4)), JsonSerializer.Deserialize("""{"a":1,"b":2,"Case":"S","c":3,"d":4}""", internalTagNamedFieldsOptions)) [] let ``serialize InternalTag NamedFields with Skippable fields`` () = @@ -910,6 +930,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"ba"}""", internalTagNamedFieldsTagPolicyOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"bb","Item":32}""", internalTagNamedFieldsTagPolicyOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"bc","x":"test","Item2":true}""", internalTagNamedFieldsTagPolicyOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"x":"test","Item2":true,"Case":"bc"}""", internalTagNamedFieldsTagPolicyOptions)) [] let ``serialize InternalTag NamedFields with tag policy`` () = @@ -925,6 +946,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"type":"Ba"}""", internalTagNamedFieldsConfiguredTagOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"type":"Bb","Item":32}""", internalTagNamedFieldsConfiguredTagOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"type":"Bc","x":"test","Item2":true}""", internalTagNamedFieldsConfiguredTagOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"x":"test","type":"Bc","Item2":true}""", internalTagNamedFieldsConfiguredTagOptions)) [] let ``serialize InternalTag NamedFields alternative Tag`` () = @@ -940,6 +962,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"type":"Ba"}""", adjacentTagConfiguredFieldsOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"type":"Bb","args":[32]}""", adjacentTagConfiguredFieldsOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"type":"Bc","args":["test",true]}""", adjacentTagConfiguredFieldsOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"args":["test",true],"type":"Bc"}""", adjacentTagConfiguredFieldsOptions)) [] let ``serialize AdjacentTag NamedFields alternative Fields`` () = @@ -955,6 +978,7 @@ module Struct = Assert.Equal(Ba, JsonSerializer.Deserialize("""{"Case":"Ba"}""", unwrapSingleFieldCasesOptions)) Assert.Equal(Bb 32, JsonSerializer.Deserialize("""{"Case":"Bb","Fields":32}""", unwrapSingleFieldCasesOptions)) Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Case":"Bc","Fields":["test",true]}""", unwrapSingleFieldCasesOptions)) + Assert.Equal(Bc("test", true), JsonSerializer.Deserialize("""{"Fields":["test",true],"Case":"Bc"}""", unwrapSingleFieldCasesOptions)) [] let ``serialize unwrapped single-field cases`` () = From 99d268cab19ef6651cb366724d32ca3e393a10e2 Mon Sep 17 00:00:00 2001 From: Loic Denuziere Date: Sun, 18 Apr 2021 20:42:16 +0200 Subject: [PATCH 3/6] Fix reading unions out of order with AdjacentTag --- src/FSharp.SystemTextJson/Union.fs | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/FSharp.SystemTextJson/Union.fs b/src/FSharp.SystemTextJson/Union.fs index 563e5e7..afd8bca 100644 --- a/src/FSharp.SystemTextJson/Union.fs +++ b/src/FSharp.SystemTextJson/Union.fs @@ -306,19 +306,22 @@ type JsonUnionConverter<'T> if readIsExpectingPropertyNamed fsOptions.UnionTagName &snapshot ty then readExpectingPropertyNamed fsOptions.UnionTagName &reader ty readExpecting JsonTokenType.String "case name" &reader ty - getCaseFromValue &reader + struct (getCaseFromValue &reader, false) else - getCaseFromDocument reader + struct (getCaseFromDocument reader, true) let readAdjacentTag (reader: byref) (options: JsonSerializerOptions) = expectAlreadyRead JsonTokenType.StartObject "object" &reader ty - let case = getCase &reader + let struct (case, usedDocument) = getCase &reader let res = if case.Fields.Length > 0 then readExpectingPropertyNamed fsOptions.UnionFieldsName &reader ty readFields &reader case options else case.Ctor [||] :?> 'T + if usedDocument then + reader.Read() |> ignore + reader.Skip() readExpecting JsonTokenType.EndObject "end of object" &reader ty res @@ -333,7 +336,7 @@ type JsonUnionConverter<'T> let readInternalTag (reader: byref) (options: JsonSerializerOptions) = if namedFields then expectAlreadyRead JsonTokenType.StartObject "object" &reader ty - let case = getCase &reader + let struct (case, usedDocument) = getCase &reader readFieldsAsRestOfObject &reader case false options else expectAlreadyRead JsonTokenType.StartArray "array" &reader ty From 4be913e767f3a0bf9f759343ba28e41dbdb9df4d Mon Sep 17 00:00:00 2001 From: Loic Denuziere Date: Wed, 26 May 2021 15:25:41 +0200 Subject: [PATCH 4/6] Remove some string allocation when matching union cases --- build.fsx | 2 +- src/FSharp.SystemTextJson/Union.fs | 41 ++++++++++++++++++++++-------- 2 files changed, 31 insertions(+), 12 deletions(-) diff --git a/build.fsx b/build.fsx index 07ad4ae..b540905 100644 --- a/build.fsx +++ b/build.fsx @@ -93,7 +93,7 @@ Target.create "TestTrim" <| fun _ -> /// project(s) as part of the run. Target.create "Benchmark" (fun _ -> DotNet.exec (fun o -> { o with - WorkingDirectory = Paths.benchmarks } ) "run" "-c release --filter \"*\"" + WorkingDirectory = Paths.benchmarks } ) "run" "-c release --runtimes netcoreapp50 --filter \"*\"" |> checkOk "Benchmarks" ) diff --git a/src/FSharp.SystemTextJson/Union.fs b/src/FSharp.SystemTextJson/Union.fs index afd8bca..e1f8f97 100644 --- a/src/FSharp.SystemTextJson/Union.fs +++ b/src/FSharp.SystemTextJson/Union.fs @@ -154,7 +154,7 @@ type JsonUnionConverter<'T> else ValueNone - let getCaseByTag tag = + let getCaseByTagReader (reader: byref) = let found = match casesByName with | ValueNone -> @@ -162,7 +162,30 @@ type JsonUnionConverter<'T> let mutable i = 0 while found.IsNone && i < cases.Length do let case = cases.[i] - if case.Name.Equals(tag, StringComparison.InvariantCulture) then + if reader.ValueTextEquals(case.Name) then + found <- ValueSome case + else + i <- i + 1 + found + | ValueSome d -> + match d.TryGetValue(reader.GetString()) with + | true, c -> ValueSome c + | false, _ -> ValueNone + match found with + | ValueNone -> + raise (JsonException("Unknown case for union type " + ty.FullName + ": " + reader.GetString())) + | ValueSome case -> + case + + let getCaseByTagString tag = + let found = + match casesByName with + | ValueNone -> + let mutable found = ValueNone + let mutable i = 0 + while found.IsNone && i < cases.Length do + let case = cases.[i] + if case.Name.Equals(tag, StringComparison.OrdinalIgnoreCase) then found <- ValueSome case else i <- i + 1 @@ -287,15 +310,11 @@ type JsonUnionConverter<'T> else readFieldsAsArray &reader case options - let getCaseFromValue (reader: byref) = - let document = JsonDocument.ParseValue(&reader) - getCaseByTag (document.RootElement.ToString()) - let getCaseFromDocument (reader: Utf8JsonReader) = let mutable reader = reader let document = JsonDocument.ParseValue(&reader) match document.RootElement.TryGetProperty fsOptions.UnionTagName with - | true, element -> getCaseByTag (element.GetString()) + | true, element -> getCaseByTagString (element.GetString()) | false, _ -> sprintf "Failed to find union case field for %s: expected %s" ty.FullName fsOptions.UnionFieldsName |> JsonException @@ -306,7 +325,7 @@ type JsonUnionConverter<'T> if readIsExpectingPropertyNamed fsOptions.UnionTagName &snapshot ty then readExpectingPropertyNamed fsOptions.UnionTagName &reader ty readExpecting JsonTokenType.String "case name" &reader ty - struct (getCaseFromValue &reader, false) + struct (getCaseByTagReader &reader, false) else struct (getCaseFromDocument reader, true) @@ -328,7 +347,7 @@ type JsonUnionConverter<'T> let readExternalTag (reader: byref) (options: JsonSerializerOptions) = expectAlreadyRead JsonTokenType.StartObject "object" &reader ty readExpecting JsonTokenType.PropertyName "case name" &reader ty - let case = getCaseByTag (reader.GetString()) + let case = getCaseByTagReader &reader let res = readFields &reader case options readExpecting JsonTokenType.EndObject "end of object" &reader ty res @@ -341,7 +360,7 @@ type JsonUnionConverter<'T> else expectAlreadyRead JsonTokenType.StartArray "array" &reader ty readExpecting JsonTokenType.String "case name" &reader ty - let case = getCaseFromValue &reader + let case = getCaseByTagReader &reader readFieldsAsRestOfArray &reader case options let readUntagged (reader: byref) (options: JsonSerializerOptions) = @@ -431,7 +450,7 @@ type JsonUnionConverter<'T> | JsonTokenType.Null when Helpers.isNullableUnion ty -> (null : obj) :?> 'T | JsonTokenType.String when unwrapFieldlessTags -> - let case = getCaseFromValue &reader + let case = getCaseByTagReader &reader case.Ctor [||] :?> 'T | _ -> match baseFormat with From d209b904a47828a732d118832b43888b4f7e3e0d Mon Sep 17 00:00:00 2001 From: Loic Denuziere Date: Wed, 26 May 2021 16:07:18 +0200 Subject: [PATCH 5/6] Add default AllowUnorderedTag flag that enables reading union tag from document --- src/FSharp.SystemTextJson/Options.fs | 12 +++++++---- src/FSharp.SystemTextJson/Union.fs | 6 +++++- .../FSharp.SystemTextJson.Tests/Test.Union.fs | 20 +++++++++---------- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/FSharp.SystemTextJson/Options.fs b/src/FSharp.SystemTextJson/Options.fs index d9d472f..d523477 100644 --- a/src/FSharp.SystemTextJson/Options.fs +++ b/src/FSharp.SystemTextJson/Options.fs @@ -77,13 +77,17 @@ type JsonUnionEncoding = /// the fields of this record are encoded directly as fields of the object representing the union. | UnwrapRecordCases = 0x00_00_21_00 + /// In AdjacentTag and InternalTag mode, allow deserializing unions + /// where the tag is not the first field in the JSON object. + | AllowUnorderedTag = 0x00_00_40_00 + //// Specific formats - | Default = 0x00_00_0C_01 - | NewtonsoftLike = 0x00_00_00_01 - | ThothLike = 0x00_00_02_04 - | FSharpLuLike = 0x00_00_16_02 + | Default = 0x00_00_4C_01 // AdjacentTag ||| UnwrapOption ||| UnwrapSingleCaseUnions ||| AllowUnorderedTag + | NewtonsoftLike = 0x00_00_40_01 // AdjacentTag ||| AllowUnorderedTag + | ThothLike = 0x00_00_42_04 // InternalTag ||| BareFieldlessTags ||| AllowUnorderedTag + | FSharpLuLike = 0x00_00_56_02 // ExternalTag ||| BareFieldlessTags ||| UnwrapOption ||| UnwrapSingleFieldCases ||| AllowUnorderedTag type JsonUnionTagName = string type JsonUnionFieldsName = string diff --git a/src/FSharp.SystemTextJson/Union.fs b/src/FSharp.SystemTextJson/Union.fs index e1f8f97..9b31dd9 100644 --- a/src/FSharp.SystemTextJson/Union.fs +++ b/src/FSharp.SystemTextJson/Union.fs @@ -326,8 +326,12 @@ type JsonUnionConverter<'T> readExpectingPropertyNamed fsOptions.UnionTagName &reader ty readExpecting JsonTokenType.String "case name" &reader ty struct (getCaseByTagReader &reader, false) - else + elif fsOptions.UnionEncoding.HasFlag JsonUnionEncoding.AllowUnorderedTag then struct (getCaseFromDocument reader, true) + else + sprintf "Failed to find union case field for %s: expected %s" ty.FullName fsOptions.UnionFieldsName + |> JsonException + |> raise let readAdjacentTag (reader: byref) (options: JsonSerializerOptions) = expectAlreadyRead JsonTokenType.StartObject "object" &reader ty diff --git a/tests/FSharp.SystemTextJson.Tests/Test.Union.fs b/tests/FSharp.SystemTextJson.Tests/Test.Union.fs index 192378c..3bb3ede 100644 --- a/tests/FSharp.SystemTextJson.Tests/Test.Union.fs +++ b/tests/FSharp.SystemTextJson.Tests/Test.Union.fs @@ -185,7 +185,7 @@ module NonStruct = Assert.Equal("""{"x":"test","Item2":true}""", JsonSerializer.Serialize(Bc("test", true), untaggedOptions)) let adjacentTagNamedFieldsOptions = JsonSerializerOptions() - adjacentTagNamedFieldsOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.NamedFields)) + adjacentTagNamedFieldsOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.NamedFields ||| JsonUnionEncoding.AllowUnorderedTag)) [] let ``deserialize AdjacentTag NamedFields`` () = @@ -201,7 +201,7 @@ module NonStruct = Assert.Equal("""{"Case":"Bc","Fields":{"x":"test","Item2":true}}""", JsonSerializer.Serialize(Bc("test", true), adjacentTagNamedFieldsOptions)) let adjacentTagNamedFieldsTagPolicyOptions = JsonSerializerOptions() - adjacentTagNamedFieldsTagPolicyOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.NamedFields, unionTagNamingPolicy = JsonNamingPolicy.CamelCase)) + adjacentTagNamedFieldsTagPolicyOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.NamedFields ||| JsonUnionEncoding.AllowUnorderedTag, unionTagNamingPolicy = JsonNamingPolicy.CamelCase)) [] let ``deserialize AdjacentTag NamedFields with tag policy`` () = @@ -279,7 +279,7 @@ module NonStruct = Assert.Equal("""{"Case":"S","a":1,"b":2,"c":3,"d":4}""", JsonSerializer.Serialize(S(1,Include 2,Include(Some 3),Include(ValueSome 4)), internalTagNamedFieldsOptions)) let internalTagNamedFieldsTagPolicyOptions = JsonSerializerOptions() - internalTagNamedFieldsTagPolicyOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.NamedFields, unionTagNamingPolicy = JsonNamingPolicy.CamelCase)) + internalTagNamedFieldsTagPolicyOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.NamedFields ||| JsonUnionEncoding.AllowUnorderedTag, unionTagNamingPolicy = JsonNamingPolicy.CamelCase)) [] let ``deserialize InternalTag NamedFields with tag policy`` () = @@ -295,7 +295,7 @@ module NonStruct = Assert.Equal("""{"Case":"bc","x":"test","Item2":true}""", JsonSerializer.Serialize(Bc("test", true), internalTagNamedFieldsTagPolicyOptions)) let internalTagNamedFieldsConfiguredTagOptions = JsonSerializerOptions() - internalTagNamedFieldsConfiguredTagOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.NamedFields, "type")) + internalTagNamedFieldsConfiguredTagOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.NamedFields ||| JsonUnionEncoding.AllowUnorderedTag, "type")) [] let ``deserialize InternalTag NamedFields alternative Tag`` () = @@ -311,7 +311,7 @@ module NonStruct = Assert.Equal("""{"type":"Bc","x":"test","Item2":true}""", JsonSerializer.Serialize(Bc("test", true), internalTagNamedFieldsConfiguredTagOptions)) let adjacentTagNamedFieldsConfiguredFieldsOptions = JsonSerializerOptions() - adjacentTagNamedFieldsConfiguredFieldsOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag, "type", "args")) + adjacentTagNamedFieldsConfiguredFieldsOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.AllowUnorderedTag, "type", "args")) [] let ``deserialize AdjacentTag NamedFields alternative Fields`` () = @@ -828,7 +828,7 @@ module Struct = Assert.Equal("""{"x":"test","Item2":true}""", JsonSerializer.Serialize(Bc("test", true), untaggedOptions)) let adjacentTagNamedFieldsOptions = JsonSerializerOptions() - adjacentTagNamedFieldsOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.NamedFields)) + adjacentTagNamedFieldsOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.NamedFields ||| JsonUnionEncoding.AllowUnorderedTag)) [] let ``deserialize AdjacentTag NamedFields`` () = @@ -844,7 +844,7 @@ module Struct = Assert.Equal("""{"Case":"Bc","Fields":{"x":"test","Item2":true}}""", JsonSerializer.Serialize(Bc("test", true), adjacentTagNamedFieldsOptions)) let adjacentTagNamedFieldsTagPolicyOptions = JsonSerializerOptions() - adjacentTagNamedFieldsTagPolicyOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.NamedFields, unionTagNamingPolicy = JsonNamingPolicy.CamelCase)) + adjacentTagNamedFieldsTagPolicyOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.NamedFields ||| JsonUnionEncoding.AllowUnorderedTag, unionTagNamingPolicy = JsonNamingPolicy.CamelCase)) [] let ``deserialize AdjacentTag NamedFields with tag policy`` () = @@ -923,7 +923,7 @@ module Struct = Assert.Equal("""{"Case":"S","a":1,"b":2,"c":3,"d":4}""", JsonSerializer.Serialize(S(1,Include 2,Include(Some 3),Include(ValueSome 4)), internalTagNamedFieldsOptions)) let internalTagNamedFieldsTagPolicyOptions = JsonSerializerOptions() - internalTagNamedFieldsTagPolicyOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.NamedFields, unionTagNamingPolicy = JsonNamingPolicy.CamelCase)) + internalTagNamedFieldsTagPolicyOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.NamedFields ||| JsonUnionEncoding.AllowUnorderedTag, unionTagNamingPolicy = JsonNamingPolicy.CamelCase)) [] let ``deserialize InternalTag NamedFields with tag policy`` () = @@ -939,7 +939,7 @@ module Struct = Assert.Equal("""{"Case":"bc","x":"test","Item2":true}""", JsonSerializer.Serialize(Bc("test", true), internalTagNamedFieldsTagPolicyOptions)) let internalTagNamedFieldsConfiguredTagOptions = JsonSerializerOptions() - internalTagNamedFieldsConfiguredTagOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.NamedFields, "type")) + internalTagNamedFieldsConfiguredTagOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.NamedFields ||| JsonUnionEncoding.AllowUnorderedTag, "type")) [] let ``deserialize InternalTag NamedFields alternative Tag`` () = @@ -955,7 +955,7 @@ module Struct = Assert.Equal("""{"type":"Bc","x":"test","Item2":true}""", JsonSerializer.Serialize(Bc("test", true), internalTagNamedFieldsConfiguredTagOptions)) let adjacentTagConfiguredFieldsOptions = JsonSerializerOptions() - adjacentTagConfiguredFieldsOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag, "type", "args")) + adjacentTagConfiguredFieldsOptions.Converters.Add(JsonFSharpConverter(JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.AllowUnorderedTag, "type", "args")) [] let ``deserialize AdjacentTag NamedFields alternative Fields`` () = From bf04dce1447a005bb4b3fbdde5d3e8e3275c9ee1 Mon Sep 17 00:00:00 2001 From: Loic Denuziere Date: Wed, 26 May 2021 17:04:02 +0200 Subject: [PATCH 6/6] Document AllowUnorderedTag --- docs/Customizing.md | 25 +++++++++++++++++++++++++ src/FSharp.SystemTextJson/Union.fs | 4 ++-- 2 files changed, 27 insertions(+), 2 deletions(-) diff --git a/docs/Customizing.md b/docs/Customizing.md index 678b534..cd16f6b 100644 --- a/docs/Customizing.md +++ b/docs/Customizing.md @@ -406,6 +406,27 @@ type Location = // Instead of {"Item":{"lat":48.858,"long":2.295}} ``` +#### `AllowUnorderedTag` + +`JsonUnionEncoding.AllowUnorderedTag` is enabled by default. +It takes effect during deserialization in AdjacentTag and InternalTag modes. +When it is disabled, the name of the case must be the first field of the JSON object. +When it is enabled, the name of the case may come later in the object, at the cost of a slight performance penalty if it does. + +For example, without `AllowUnorderedTag`, the following will fail to parse: + +```fsharp +JsonSerializer.Deserialize("""{"Fields":[3.14],"Case":"WithOneArg"}""", options) +// --> Error: Failed to find union case field for Example: expected Case +``` + +Whereas with `AllowUnorderedTag`, it will succeed: + +```fsharp +JsonSerializer.Deserialize("""{"Fields":[3.14],"Case":"WithOneArg"}""", options) +// --> WithOneArg 3.14 +``` + ### Combined flags `JsonUnionEncoding` also contains a few items that combine several of the above flags. @@ -417,6 +438,7 @@ type Location = JsonUnionEncoding.AdjacentTag ||| JsonUnionEncoding.UnwrapOption ||| JsonUnionEncoding.UnwrapSingleCaseUnions + ||| JsonUnionEncoding.AllowUnorderedTag ``` It is particularly useful if you want to use the default encoding with some additional options, for example: @@ -430,6 +452,7 @@ type Location = ```fsharp JsonUnionEncoding.AdjacentTag + ||| JsonUnionEncoding.AllowUnorderedTag ``` * `JsonUnionEncoding.ThothLike` causes similar behavior to the library [Thoth.Json](https://thoth-org.github.io/Thoth.Json/). @@ -438,6 +461,7 @@ type Location = ```fsharp JsonUnionEncoding.InternalTag ||| JsonUnionEncoding.UnwrapFieldlessTags + ||| JsonUnionEncoding.AllowUnorderedTag ``` * `JsonUnionEncoding.FSharpLuLike` causes similar behavior to the library [FSharpLu.Json](https://github.com/microsoft/fsharplu/wiki/FSharpLu.Json) in Compact mode. @@ -448,6 +472,7 @@ type Location = ||| JsonUnionEncoding.UnwrapFieldlessTags ||| JsonUnionEncoding.UnwrapOption ||| JsonUnionEncoding.UnwrapSingleFieldCases + ||| JsonUnionEncoding.AllowUnorderedTag ``` ## `unionTagName` diff --git a/src/FSharp.SystemTextJson/Union.fs b/src/FSharp.SystemTextJson/Union.fs index 9b31dd9..9bd77f0 100644 --- a/src/FSharp.SystemTextJson/Union.fs +++ b/src/FSharp.SystemTextJson/Union.fs @@ -316,7 +316,7 @@ type JsonUnionConverter<'T> match document.RootElement.TryGetProperty fsOptions.UnionTagName with | true, element -> getCaseByTagString (element.GetString()) | false, _ -> - sprintf "Failed to find union case field for %s: expected %s" ty.FullName fsOptions.UnionFieldsName + sprintf "Failed to find union case field for %s: expected %s" ty.FullName fsOptions.UnionTagName |> JsonException |> raise @@ -329,7 +329,7 @@ type JsonUnionConverter<'T> elif fsOptions.UnionEncoding.HasFlag JsonUnionEncoding.AllowUnorderedTag then struct (getCaseFromDocument reader, true) else - sprintf "Failed to find union case field for %s: expected %s" ty.FullName fsOptions.UnionFieldsName + sprintf "Failed to find union case field for %s: expected %s" ty.FullName fsOptions.UnionTagName |> JsonException |> raise