SystemTextJson: Add option to reject null string values

README.md

@@ -99,6 +99,7 @@ The respective concrete Codec packages include relevant `Converter`/`JsonConvert
 ### `System.Text.Json`-specific low level converters
 
 - `UnionOrTypeSafeEnumConverterFactory`: Global converter that can apply `TypeSafeEnumConverter` to all Discriminated Unions that do not have cases with values, and `UnionConverter` to ones that have values. See [this `System.Text.Json` issue](https://github.com/dotnet/runtime/issues/55744) for background information as to the reasoning behind and tradeoffs involved in applying such a policy.  
+- `RejectNullStringConverter`: Global converter that can reject `null` as a valid string value
 
 ## `FsCodec.NewtonsoftJson.Options`
 
@@ -119,6 +120,7 @@ The respective concrete Codec packages include relevant `Converter`/`JsonConvert
   - `(camelCase = true)`: opts into camel case conversion for `PascalCased` properties and `Dictionary` keys
   - `(autoTypeSafeEnumToJsonString = true)`: triggers usage of `TypeSafeEnumConverter` for any F# Discriminated Unions that only contain nullary cases. See [`AutoUnionTests.fs`](https://github.com/jet/FsCodec/blob/master/tests/FsCodec.SystemTextJson.Tests/AutoUnionTests.fs) for examples  
   - `(autoUnionToJsonObject = true)`: triggers usage of a `UnionConverter` to round-trip F# Discriminated Unions (with at least a single case that has a body) as JSON Object structures. See [`AutoUnionTests.fs`](https://github.com/jet/FsCodec/blob/master/tests/FsCodec.SystemTextJson.Tests/AutoUnionTests.fs) for examples
+  - `(rejectNullStrings = true)`: triggers usage of [`RejectNullStringConverter`](https://github.com/jet/FsCodec/blob/master/src/FsCodec.SystemTextJson/RejectNullStringConverter.fs) to reject `null` as a value for strings (`string option` is still supported).
 - `Default`: Default settings; same as calling `Create()` produces (same intent as [`JsonSerializerOptions.Default`](https://github.com/dotnet/runtime/pull/61434)) 
 
 ## `Serdes`
@@ -229,18 +231,18 @@ The default settings for FsCodec applies Json.NET's default behavior, which is t
 
 The recommendations here apply particularly to Event Contracts - the data in your store will inevitably outlast your code, so being conservative in the complexity of one's encoding scheme is paramount. Explicit is better than Implicit.
 
-| Type kind | TL;DR | Notes                                                                                                                                                                                                                                                 | Example input | Example output |
-| :--- | :--- |:------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| :--- | :--- |
-| `'t[]` | As per C# | Don't forget to handle `null`                                                                                                                                                                                                                         | `[ 1; 2; 3]` | `[1,2,3]` |
-| `DateTimeOffset` | Roundtrips cleanly | The default `Options.Create` requests `RoundtripKind`                                                                                                                                                                                                | `DateTimeOffset.Now` | `"2019-09-04T20:30:37.272403+01:00"` |
-| `Nullable<'t>` | As per C#; `Nullable()` -> `null`, `Nullable x` -> `x` | OOTB Json.NET and STJ roundtrip cleanly. Works with `Options.CreateDefault()`. Worth considering if your contract does not involve many `option` types                                                                                               | `Nullable 14` | `14` |
-| `'t option` | `Some null`,`None` -> `null`, `Some x` -> `x` _with the converter `Options.Create()` adds_ | OOTB Json.NET does not roundtrip `option` types cleanly; `Options.Create` wires in an `OptionConverter` by default in `FsCodec.NewtonsoftJson`<br/> NOTE `Some null` will produce `null`, but deserialize as `None` - i.e., it's not round-trippable | `Some 14` | `14` | 
-| `string` | As per C#; need to handle `null` | One can use a `string option` to map `null` and `Some null` to `None`                                                                                                                                                                                 | `"Abc"` | `"Abc"` |
-| types with unit of measure | Works well (doesnt encode the unit) | Unit of measure tags are only known to the compiler; Json.NET does not process the tags and treats it as the underlying primitive type                                                                                                                | `54<g>` | `54` | 
-| [`FSharp.UMX`](https://github.com/fsprojects/FSharp.UMX) tagged `string`, `DateTimeOffset` | Works well | [`FSharp.UMX`](https://github.com/fsprojects/FSharp.UMX) enables one to type-tag `string` and `DateTimeOffset` values using the units of measure compiler feature, which Json.NET will render as if they were unadorned                               | `SkuId.parse "54-321"` | `"000-054-321"` |
-| records | Just work | For `System.Text.Json` v `4.x`, usage of `[<CLIMutable>]` or a custom `JsonRecordConverter` was once required                                                                                                                                         | `{\| a = 1; b = Some "x" \|}` | `"{"a":1,"b":"x"}"` |
-| Nullary unions (Enum-like DU's without bodies) | Tag `type` with `TypeSafeEnumConverter` | Works well - guarantees a valid mapping, as opposed to using a `System.Enum` and `StringEnumConverter`, which can map invalid values and/or silently map to `0` etc                                                                                   | `State.NotFound` | `"NotFound"` |
-| Discriminated Unions (where one or more cases has a body) | Tag `type` with `UnionConverter` | This format can be readily consumed in Java, JavaScript and Swift. Nonetheless, exhaust all other avenues before considering encoding a union in JSON. The `"case"` label id can be overridden.                                                       | `Decision.Accepted { result = "54" }` | `{"case": "Accepted","result":"54"}` |
+| Type kind | TL;DR                                                                                                  | Notes                                                                                                                                                                                                                                                 | Example input | Example output |
+| :--- |:-------------------------------------------------------------------------------------------------------|:------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| :--- | :--- |
+| `'t[]` | As per C#                                                                                              | Don't forget to handle `null`                                                                                                                                                                                                                         | `[ 1; 2; 3]` | `[1,2,3]` |
+| `DateTimeOffset` | Roundtrips cleanly                                                                                     | The default `Options.Create` requests `RoundtripKind`                                                                                                                                                                                                | `DateTimeOffset.Now` | `"2019-09-04T20:30:37.272403+01:00"` |
+| `Nullable<'t>` | As per C#; `Nullable()` -> `null`, `Nullable x` -> `x`                                                 | OOTB Json.NET and STJ roundtrip cleanly. Works with `Options.CreateDefault()`. Worth considering if your contract does not involve many `option` types                                                                                               | `Nullable 14` | `14` |
+| `'t option` | `Some null`,`None` -> `null`, `Some x` -> `x` _with the converter `Options.Create()` adds_             | OOTB Json.NET does not roundtrip `option` types cleanly; `Options.Create` wires in an `OptionConverter` by default in `FsCodec.NewtonsoftJson`<br/> NOTE `Some null` will produce `null`, but deserialize as `None` - i.e., it's not round-trippable | `Some 14` | `14` | 
+| `string` | As per C#; need to handle `null`. Can opt into rejecting null values with `(rejectNullStrings = true)` | One can use a `string option` to map `null` and `Some null` to `None`                                                                                                                                                                                 | `"Abc"` | `"Abc"` |
+| types with unit of measure | Works well (doesnt encode the unit)                                                                    | Unit of measure tags are only known to the compiler; Json.NET does not process the tags and treats it as the underlying primitive type                                                                                                                | `54<g>` | `54` | 
+| [`FSharp.UMX`](https://github.com/fsprojects/FSharp.UMX) tagged `string`, `DateTimeOffset` | Works well                                                                                             | [`FSharp.UMX`](https://github.com/fsprojects/FSharp.UMX) enables one to type-tag `string` and `DateTimeOffset` values using the units of measure compiler feature, which Json.NET will render as if they were unadorned                               | `SkuId.parse "54-321"` | `"000-054-321"` |
+| records | Just work                                                                                              | For `System.Text.Json` v `4.x`, usage of `[<CLIMutable>]` or a custom `JsonRecordConverter` was once required                                                                                                                                         | `{\| a = 1; b = Some "x" \|}` | `"{"a":1,"b":"x"}"` |
+| Nullary unions (Enum-like DU's without bodies) | Tag `type` with `TypeSafeEnumConverter`                                                                | Works well - guarantees a valid mapping, as opposed to using a `System.Enum` and `StringEnumConverter`, which can map invalid values and/or silently map to `0` etc                                                                                   | `State.NotFound` | `"NotFound"` |
+| Discriminated Unions (where one or more cases has a body) | Tag `type` with `UnionConverter`                                                                       | This format can be readily consumed in Java, JavaScript and Swift. Nonetheless, exhaust all other avenues before considering encoding a union in JSON. The `"case"` label id can be overridden.                                                       | `Decision.Accepted { result = "54" }` | `{"case": "Accepted","result":"54"}` |
 
 ### _Unsupported_ types and/or constructs
 

src/FsCodec.SystemTextJson/FsCodec.SystemTextJson.fsproj

@@ -8,6 +8,7 @@
     <Compile Include="Pickler.fs" />
     <Compile Include="UnionConverter.fs" />
     <Compile Include="TypeSafeEnumConverter.fs" />
+    <Compile Include="RejectNullStringConverter.fs" />
     <Compile Include="UnionOrTypeSafeEnumConverterFactory.fs" />
     <Compile Include="Options.fs" />
     <Compile Include="Serdes.fs" />

src/FsCodec.SystemTextJson/Options.fs

@@ -57,15 +57,21 @@ type Options private () =
             /// <summary>Apply <c>TypeSafeEnumConverter</c> if possible. Defaults to <c>false</c>.</summary>
             [<Optional; DefaultParameterValue(null)>] ?autoTypeSafeEnumToJsonString : bool,
             /// <summary>Apply <c>UnionConverter</c> for all Discriminated Unions, if <c>TypeSafeEnumConverter</c> not possible. Defaults to <c>false</c>.</summary>
-            [<Optional; DefaultParameterValue(null)>] ?autoUnionToJsonObject : bool) =
+            [<Optional; DefaultParameterValue(null)>] ?autoUnionToJsonObject : bool,
+            /// <summary>Apply <c>RejectNullStringConverter</c> in order to have serialization throw on <c>null</c> strings. Use <c>string option</c> to represent strings that can potentially be <c>null</c>.
+            [<Optional; DefaultParameterValue(null)>] ?rejectNullStrings: bool) =
+
+        let autoTypeSafeEnumToJsonString = defaultArg autoTypeSafeEnumToJsonString false
+        let autoUnionToJsonObject = defaultArg autoUnionToJsonObject false
+        let rejectNullStrings = defaultArg rejectNullStrings false
 
         Options.CreateDefault(
-            converters =
-                (   match autoTypeSafeEnumToJsonString = Some true, autoUnionToJsonObject = Some true with
-                    | tse, u when tse || u ->
-                        let converter : JsonConverter array = [| UnionOrTypeSafeEnumConverterFactory(typeSafeEnum = tse, union = u) |]
-                        if converters = null then converter else Array.append converters converter
-                    | _ -> converters),
+            converters = [|
+                if converters <> null then yield! converters
+                if rejectNullStrings then yield RejectNullStringConverter()
+                if autoTypeSafeEnumToJsonString || autoUnionToJsonObject then
+                    yield UnionOrTypeSafeEnumConverterFactory(typeSafeEnum = autoTypeSafeEnumToJsonString, union = autoUnionToJsonObject)
+            |],
             ?ignoreNulls = ignoreNulls,
             ?indent = indent,
             ?camelCase = camelCase,

src/FsCodec.SystemTextJson/RejectNullStringConverter.fs

@@ -0,0 +1,22 @@
+namespace FsCodec.SystemTextJson
+
+open System.Text.Json.Serialization
+
+module internal Error =
+    [<Literal>]
+    let message = "Expected string, got null. When allowNullStrings is false you must explicitly type optional strings as 'string option'"
+
+type RejectNullStringConverter() =
+    inherit JsonConverter<string>()
+
+    override _.HandleNull = true
+    override _.CanConvert(t) = t = typeof<string>
+
+    override this.Read(reader, _typeToConvert, _options) =
+        let value = reader.GetString()
+        if value = null then nullArg Error.message
+        value
+
+    override this.Write(writer, value, _options) =
+        if value = null then nullArg Error.message
+        writer.WriteStringValue(value)

tests/FsCodec.NewtonsoftJson.Tests/FsCodec.NewtonsoftJson.Tests.fsproj

@@ -16,7 +16,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <PackageReference Include="FsCheck.Xunit" Version="2.14.3" />
+    <PackageReference Include="FsCheck.Xunit" Version="3.0.0-beta2" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1" />
     <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
     <PackageReference Include="Serilog.Sinks.Console" Version="4.0.1" />

tests/FsCodec.NewtonsoftJson.Tests/StreamTests.fs

@@ -15,8 +15,8 @@ let serdes = Serdes Options.Default
 type Rec = { a : int; b : string; c : string }
 
 let [<Fact>] ``Can serialize/deserialize to stream`` () =
-    let value = { a = 10; b = "10"; c = null }
-    use stream = new MemoryStream()    
+    let value = { a = 10; b = "10"; c = "" }
+    use stream = new MemoryStream()
     serdes.SerializeToStream(value, stream)
     stream.Seek(0L, SeekOrigin.Begin) |> ignore
     let value' = serdes.DeserializeFromStream(stream)

tests/FsCodec.NewtonsoftJson.Tests/UnionConverterTests.fs

@@ -11,7 +11,7 @@ open FsCodec.NewtonsoftJson
 open Newtonsoft.Json
 #endif
 
-open FsCheck
+open FsCheck.FSharp
 open Swensen.Unquote.Assertions
 open System
 open System.IO
@@ -307,8 +307,8 @@ let render ignoreNulls = function
     | CaseZ (a, Some b) -> sprintf """{"case":"CaseZ","a":"%s","b":"%s"}""" (string a) (string b)
 
 type FsCheckGenerators =
-    static member CartId = Arb.generate |> Gen.map CartId |> Arb.fromGen
-    static member SkuId = Arb.generate |> Gen.map SkuId |> Arb.fromGen
+    static member CartId = ArbMap.defaults |> ArbMap.generate |> Gen.map CartId |> Arb.fromGen
+    static member SkuId = ArbMap.defaults |> ArbMap.generate |> Gen.map SkuId |> Arb.fromGen
 
 type DomainPropertyAttribute() =
     inherit FsCheck.Xunit.PropertyAttribute(QuietOnSuccess = true, Arbitrary=[| typeof<FsCheckGenerators> |])

tests/FsCodec.SystemTextJson.Tests/AutoUnionTests.fs

@@ -48,11 +48,7 @@ let [<FsCheck.Xunit.Property>] ``auto-encodes Unions and non-unions`` (x : Any)
 
     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 [<FsCheck.Xunit.Property>] ``It round trips`` (x: Any) =
     let encoded = serdes.Serialize x
     let decoded : Any = serdes.Deserialize encoded
     test <@ decoded = x @>

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

@@ -7,7 +7,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="FsCheck.Xunit" Version="2.14.3" />
+    <PackageReference Include="FsCheck.Xunit" Version="3.0.0-beta2" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.1.0" />
     <PackageReference Include="Serilog.Sinks.Console" Version="4.0.1" />
     <PackageReference Include="Unquote" Version="6.1.0" />

tests/FsCodec.SystemTextJson.Tests/SerdesTests.fs

@@ -1,5 +1,6 @@
 module FsCodec.SystemTextJson.Tests.SerdesTests
 
+open System
 open System.Collections.Generic
 open FsCodec.SystemTextJson
 open Swensen.Unquote
@@ -8,6 +9,7 @@ open Xunit
 type Record = { a : int }
 
 type RecordWithOption = { a : int; b : string option }
+type RecordWithString = { c : int; d : string }
 
 /// Characterization tests for OOTB JSON.NET
 /// The aim here is to characterize the gaps that we'll shim; we only want to do that as long as it's actually warranted
@@ -65,6 +67,36 @@ module StjCharacterization =
         let des = serdes.Deserialize ser
         test <@ value = des @>
 
+let [<Fact>] ``RejectNullStringConverter rejects null strings`` () =
+    let serdes = Serdes(Options.Create(rejectNullStrings = true))
+
+    let value: string = null
+    raises<ArgumentNullException> <@ serdes.Serialize value @>
+
+    let value = [| "A"; null |]
+    raises<ArgumentNullException> <@ serdes.Serialize value @>
+
+    let value = { c = 1; d = null }
+    raises<ArgumentNullException> <@ serdes.Serialize value @>
+
+let [<Fact>] ``RejectNullStringConverter serializes strings correctly`` () =
+    let serdes = Serdes(Options.Create(rejectNullStrings = true))
+    let value = { c = 1; d = "some string" }
+    let res = serdes.Serialize value
+    test <@ res = """{"c":1,"d":"some string"}""" @>
+    let des = serdes.Deserialize res
+    test <@ des = value @>
+
+[<Theory; InlineData(true); InlineData(false)>]
+let ``string options are supported regardless of "rejectNullStrings" value`` rejectNullStrings =
+    let serdes = Serdes(Options.Create(rejectNullStrings = rejectNullStrings))
+    let value = [| Some "A"; None |]
+    let res = serdes.Serialize value
+    test <@ res = """["A",null]""" @>
+    let des = serdes.Deserialize res
+    test <@ des = value @>
+
+
 (* Serdes + default Options behavior, i.e. the stuff we do *)
 
 let serdes = Serdes Options.Default