Stop UnionOrTypeSafeEnumConverterFactory handling option/list (#72)

Author: bartelink

CHANGELOG.md

@@ -13,8 +13,10 @@ The `Unreleased` section name is replaced by the expected version of next releas
 ### Removed
 ### Fixed
 
+- `SystemTextJson`: Prevent `UnionConverter` being applied to `option` and `list` types when using `UnionOrTypeSafeEnumConverterFactory`/`SystemTextJson.Options(autoUnion = true)` [#72](https://github.com/jet/FsCodec/pull/72)
+
 <a name="2.3.0"></a>
-## [2.3.0] - 2022-01-14
+## [2.3.0] - 2022-01-14 **Unlisted due to bug fixed in 2.3.1**
 
 ### Changed
 

src/FsCodec.SystemTextJson/UnionOrTypeSafeEnumConverterFactory.fs

@@ -9,8 +9,14 @@ type internal ConverterActivator = delegate of unit -> JsonConverter
 type UnionOrTypeSafeEnumConverterFactory() =
     inherit JsonConverterFactory()
 
+    let isIntrinsic (t : Type) =
+        t.IsGenericType
+        && (t.GetGenericTypeDefinition() = typedefof<option<_>>
+            || t.GetGenericTypeDefinition() = typedefof<list<_>>)
+
     override _.CanConvert(t : Type) =
         Union.isUnion t
+        && not (isIntrinsic t)
 
     override _.CreateConverter(typ, _options) =
         let openConverterType = if Union.hasOnlyNullaryCases typ then typedefof<TypeSafeEnumConverter<_>> else typedefof<UnionConverter<_>>

tests/FsCodec.NewtonsoftJson.Tests/SomeNullHandlingTests.fs

@@ -1,3 +1,20 @@
+#if SYSTEM_TEXT_JSON
+module FsCodec.SytemTextJson.Tests.SomeNullHandlingTests
+
+open FsCodec.SystemTextJson
+open Swensen.Unquote
+open Xunit
+
+let serdes = Options.Create() |> Serdes
+
+let [<Fact>] ``Options.Create does not roundtrip Some null`` () =
+    let value : string option = Some null
+    let ser = serdes.Serialize value
+    "null" =! ser
+    // But it doesn't roundtrip
+    value <>! serdes.Deserialize ser
+
+#else
 module FsCodec.NewtonsoftJson.Tests.SomeNullHandlingTests
 
 open FsCodec.NewtonsoftJson
@@ -19,6 +36,7 @@ let [<Fact>] ``Settings.Create does not roundtrip Some null`` () =
     "null" =! ser
     // But it doesn't roundtrip
     value <>! serdes.Deserialize ser
+#endif
 
 let hasSomeNull value = TypeShape.Generic.exists(fun (x : string option) -> x = Some null) value
 let replaceSomeNullsWithNone value = TypeShape.Generic.map (function Some (null : string) -> None | x -> x) value
@@ -40,7 +58,7 @@ let [<Fact>] ``Workaround is to detect and/or substitute such non-roundtrippable
 type RecordWithStringOptions = { x : int; y : Nested }
 and Nested = { z : string option }
 
-let [<Fact>] ``Can detect and/or substitute null string option when using Settings.Create`` () =
+let [<Fact>] ``Can detect and/or substitute null string option when using Options/Settings.Create`` () =
     let value : RecordWithStringOptions = { x = 9; y = { z = Some null } }
     test <@ hasSomeNull value @>
     let value = replaceSomeNullsWithNone value
@@ -49,8 +67,13 @@ let [<Fact>] ``Can detect and/or substitute null string option when using Settin
     ser =! """{"x":9,"y":{"z":null}}"""
     test <@ value = serdes.Deserialize ser @>
 
+#if SYSTEM_TEXT_JSON
+    // As one might expect, the ignoreNulls setting is also honored
+    let ignoreNullsSerdes = Options.Create(ignoreNulls = true) |> Serdes
+#else
     // As one might expect, the ignoreNulls setting is also honored
-    let ignoreNullsSerdes = Settings.Create(ignoreNulls=true) |> Serdes
+    let ignoreNullsSerdes = Settings.Create(ignoreNulls = true) |> Serdes
+#endif
     let ser = ignoreNullsSerdes.Serialize value
     ser =! """{"x":9,"y":{}}"""
     test <@ value = serdes.Deserialize ser @>

tests/FsCodec.SystemTextJson.Tests/AutoUnionTests.fs

@@ -4,21 +4,42 @@ open FsCodec.SystemTextJson
 open Swensen.Unquote
 
 type ATypeSafeEnum = A | B | C
-type NotAUnion = { body : string }
-type AUnion = D of value : string | E of ATypeSafeEnum | F
+type NotAUnion = { body : string; opt : string option; list: string list }
+type AUnion = D of value : string | E of ATypeSafeEnum | F | G of value : string option
 type Any = Tse of enum : ATypeSafeEnum | Not of NotAUnion | Union of AUnion
 
 let serdes = Options.Create(autoUnion = true) |> Serdes
 
 let [<Xunit.Fact>] ``Basic characteristics`` () =
     test <@ "\"B\"" = serdes.Serialize B @>
-    test <@ "{\"body\":\"A\"}" = serdes.Serialize { body = "A" } @>
+    test <@ "{\"body\":\"A\",\"opt\":null,\"list\":[]}" = serdes.Serialize { body = "A"; opt = None ; list = [] } @>
+    test <@ "{\"body\":\"A\",\"opt\":\"A\",\"list\":[\"A\"]}" = serdes.Serialize { body = "A"; opt = Some "A"; list = ["A"] } @>
+    test <@ "{\"body\":\"A\",\"opt\":\"A\",\"list\":[]}" = serdes.Serialize { body = "A"; opt = Some "A"; list = [] } @>
     test <@ "{\"case\":\"D\",\"value\":\"A\"}" = serdes.Serialize (D "A") @>
+    test <@ "{\"case\":\"G\",\"value\":\"A\"}" = serdes.Serialize (G (Some "A")) @>
     test <@ "{\"case\":\"Tse\",\"enum\":\"B\"}" = serdes.Serialize (Tse B) @>
     test <@ Tse B = serdes.Deserialize "{\"case\":\"Tse\",\"enum\":\"B\"}" @>
-    test <@ Not { body = "A" } = serdes.Deserialize "{\"case\":\"Not\",\"body\":\"A\"}" @>
+    test <@ Not { body = "A"; opt = None; list = [] } = serdes.Deserialize "{\"case\":\"Not\",\"body\":\"A\",\"list\":[]}" @>
+    test <@ Not { body = "A"; opt = None; list = ["A"] } = serdes.Deserialize "{\"case\":\"Not\",\"body\":\"A\",\"list\":[\"A\"]}" @>
 
 let [<FsCheck.Xunit.Property>] ``auto-encodes Unions and non-unions`` (x : Any) =
     let encoded = serdes.Serialize x
     let decoded : Any = serdes.Deserialize encoded
+
+    // Special cases for (non-roundtrippable) Some null => None conversion that STJ (and NSJ OptionConverter) do
+    // See next test for a debatable trick
+    match decoded, x with
+    | Union (G None), Union (G (Some null)) -> ()
+    | Not rr, Not ({ opt = Some null } as rx) -> test <@ rr = { rx with opt = None } @>
+    | _ ->
+
+    test <@ decoded = x @>
+
+(* 🙈 *)
+
+let (|ReplaceSomeNullWithNone|) value = TypeShape.Generic.map (function Some (null : string) -> None | x -> x) value
+
+let [<FsCheck.Xunit.Property>] ``Some null roundtripping hack for tests`` (ReplaceSomeNullWithNone (x : Any)) =
+    let encoded = serdes.Serialize x
+    let decoded : Any = serdes.Deserialize encoded
     test <@ decoded = x @>

tests/FsCodec.SystemTextJson.Tests/FsCodec.SystemTextJson.Tests.fsproj

@@ -36,6 +36,9 @@
     </Compile>
     <Compile Include="InteropTests.fs" />
     <Compile Include="AutoUnionTests.fs" />
+    <Compile Include="..\FsCodec.NewtonsoftJson.Tests\SomeNullHandlingTests.fs">
+      <Link>SomeNullHandlingTests.fs</Link>
+    </Compile>
   </ItemGroup>
 
 </Project>

tests/FsCodec.SystemTextJson.Tests/SerdesTests.fs

@@ -30,6 +30,14 @@ module StjCharacterization =
 
         test <@ value = ootb.Deserialize<RecordWithOption> ser @>
 
+    let [<Fact>] ``OOTB STJ Some null decodes as None as per NSJ`` () =
+        let value = { a = 1; b = Some null }
+        let ser = ootb.Serialize value
+        test <@ ser = """{"a":1,"b":null}""" @>
+
+        // sic: does not roundtrip
+        test <@ { value with b = None } = ootb.Deserialize<RecordWithOption> ser @>
+
     let [<Fact>] ``OOTB STJ lists Just Works`` () =
         let value = [ "A"; "B" ]
         let ser = ootb.Serialize value