Skip to content

Commit b66b91e

Browse files
nordfjordnordfjord
authored
SystemTextJson: Add option to reject null string values (#87)
1 parent fd36954 commit b66b91e

10 files changed

Lines changed: 90 additions & 31 deletions

File tree

‎README.md‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ The respective concrete Codec packages include relevant `Converter`/`JsonConvert
9999
### `System.Text.Json`-specific low level converters
100100

101101
- `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.
102+
- `RejectNullStringConverter`: Global converter that can reject `null` as a valid string value
102103

103104
## `FsCodec.NewtonsoftJson.Options`
104105

@@ -119,6 +120,7 @@ The respective concrete Codec packages include relevant `Converter`/`JsonConvert
119120
- `(camelCase = true)`: opts into camel case conversion for `PascalCased` properties and `Dictionary` keys
120121
- `(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
121122
- `(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
123+
- `(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).
122124
- `Default`: Default settings; same as calling `Create()` produces (same intent as [`JsonSerializerOptions.Default`](https://github.com/dotnet/runtime/pull/61434))
123125

124126
## `Serdes`
@@ -229,18 +231,18 @@ The default settings for FsCodec applies Json.NET's default behavior, which is t
229231

230232
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.
231233

232-
| Type kind | TL;DR | Notes | Example input | Example output |
233-
| :--- | :--- |:------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| :--- | :--- |
234-
| `'t[]` | As per C# | Don't forget to handle `null` | `[ 1; 2; 3]` | `[1,2,3]` |
235-
| `DateTimeOffset` | Roundtrips cleanly | The default `Options.Create` requests `RoundtripKind` | `DateTimeOffset.Now` | `"2019-09-04T20:30:37.272403+01:00"` |
236-
| `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` |
237-
| `'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` |
238-
| `string` | As per C#; need to handle `null` | One can use a `string option` to map `null` and `Some null` to `None` | `"Abc"` | `"Abc"` |
239-
| 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` |
240-
| [`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"` |
241-
| 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"}"` |
242-
| 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"` |
243-
| 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"}` |
234+
| Type kind | TL;DR | Notes | Example input | Example output |
235+
| :--- |:-------------------------------------------------------------------------------------------------------|:------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| :--- | :--- |
236+
| `'t[]` | As per C# | Don't forget to handle `null` | `[ 1; 2; 3]` | `[1,2,3]` |
237+
| `DateTimeOffset` | Roundtrips cleanly | The default `Options.Create` requests `RoundtripKind` | `DateTimeOffset.Now` | `"2019-09-04T20:30:37.272403+01:00"` |
238+
| `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` |
239+
| `'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` |
240+
| `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"` |
241+
| 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` |
242+
| [`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"` |
243+
| 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"}"` |
244+
| 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"` |
245+
| 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"}` |
244246

245247
### _Unsupported_ types and/or constructs
246248

‎src/FsCodec.SystemTextJson/FsCodec.SystemTextJson.fsproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
<Compile Include="Pickler.fs" />
99
<Compile Include="UnionConverter.fs" />
1010
<Compile Include="TypeSafeEnumConverter.fs" />
11+
<Compile Include="RejectNullStringConverter.fs" />
1112
<Compile Include="UnionOrTypeSafeEnumConverterFactory.fs" />
1213
<Compile Include="Options.fs" />
1314
<Compile Include="Serdes.fs" />

‎src/FsCodec.SystemTextJson/Options.fs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,21 @@ type Options private () =
5757
/// <summary>Apply <c>TypeSafeEnumConverter</c> if possible. Defaults to <c>false</c>.</summary>
5858
[<Optional; DefaultParameterValue(null)>] ?autoTypeSafeEnumToJsonString : bool,
5959
/// <summary>Apply <c>UnionConverter</c> for all Discriminated Unions, if <c>TypeSafeEnumConverter</c> not possible. Defaults to <c>false</c>.</summary>
60-
[<Optional; DefaultParameterValue(null)>] ?autoUnionToJsonObject : bool) =
60+
[<Optional; DefaultParameterValue(null)>] ?autoUnionToJsonObject : bool,
61+
/// <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>.
62+
[<Optional; DefaultParameterValue(null)>] ?rejectNullStrings: bool) =
63+
64+
let autoTypeSafeEnumToJsonString = defaultArg autoTypeSafeEnumToJsonString false
65+
let autoUnionToJsonObject = defaultArg autoUnionToJsonObject false
66+
let rejectNullStrings = defaultArg rejectNullStrings false
6167

6268
Options.CreateDefault(
63-
converters =
64-
( match autoTypeSafeEnumToJsonString = Some true, autoUnionToJsonObject = Some true with
65-
| tse, u when tse || u ->
66-
let converter : JsonConverter array = [| UnionOrTypeSafeEnumConverterFactory(typeSafeEnum = tse, union = u) |]
67-
if converters = null then converter else Array.append converters converter
68-
| _ -> converters),
69+
converters = [|
70+
if converters <> null then yield! converters
71+
if rejectNullStrings then yield RejectNullStringConverter()
72+
if autoTypeSafeEnumToJsonString || autoUnionToJsonObject then
73+
yield UnionOrTypeSafeEnumConverterFactory(typeSafeEnum = autoTypeSafeEnumToJsonString, union = autoUnionToJsonObject)
74+
|],
6975
?ignoreNulls = ignoreNulls,
7076
?indent = indent,
7177
?camelCase = camelCase,

‎src/FsCodec.SystemTextJson/RejectNullStringConverter.fs‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
namespace FsCodec.SystemTextJson
2+
3+
open System.Text.Json.Serialization
4+
5+
module internal Error =
6+
[<Literal>]
7+
let message = "Expected string, got null. When allowNullStrings is false you must explicitly type optional strings as 'string option'"
8+
9+
type RejectNullStringConverter() =
10+
inherit JsonConverter<string>()
11+
12+
override _.HandleNull = true
13+
override _.CanConvert(t) = t = typeof<string>
14+
15+
override this.Read(reader, _typeToConvert, _options) =
16+
let value = reader.GetString()
17+
if value = null then nullArg Error.message
18+
value
19+
20+
override this.Write(writer, value, _options) =
21+
if value = null then nullArg Error.message
22+
writer.WriteStringValue(value)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
</ItemGroup>
1717

1818
<ItemGroup>
19-
<PackageReference Include="FsCheck.Xunit" Version="2.14.3" />
19+
<PackageReference Include="FsCheck.Xunit" Version="3.0.0-beta2" />
2020
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1" />
2121
<PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
2222
<PackageReference Include="Serilog.Sinks.Console" Version="4.0.1" />

‎tests/FsCodec.NewtonsoftJson.Tests/StreamTests.fs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ let serdes = Serdes Options.Default
1515
type Rec = { a : int; b : string; c : string }
1616

1717
let [<Fact>] ``Can serialize/deserialize to stream`` () =
18-
let value = { a = 10; b = "10"; c = null }
19-
use stream = new MemoryStream()
18+
let value = { a = 10; b = "10"; c = "" }
19+
use stream = new MemoryStream()
2020
serdes.SerializeToStream(value, stream)
2121
stream.Seek(0L, SeekOrigin.Begin) |> ignore
2222
let value' = serdes.DeserializeFromStream(stream)

‎tests/FsCodec.NewtonsoftJson.Tests/UnionConverterTests.fs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ open FsCodec.NewtonsoftJson
1111
open Newtonsoft.Json
1212
#endif
1313

14-
open FsCheck
14+
open FsCheck.FSharp
1515
open Swensen.Unquote.Assertions
1616
open System
1717
open System.IO
@@ -307,8 +307,8 @@ let render ignoreNulls = function
307307
| CaseZ (a, Some b) -> sprintf """{"case":"CaseZ","a":"%s","b":"%s"}""" (string a) (string b)
308308

309309
type FsCheckGenerators =
310-
static member CartId = Arb.generate |> Gen.map CartId |> Arb.fromGen
311-
static member SkuId = Arb.generate |> Gen.map SkuId |> Arb.fromGen
310+
static member CartId = ArbMap.defaults |> ArbMap.generate |> Gen.map CartId |> Arb.fromGen
311+
static member SkuId = ArbMap.defaults |> ArbMap.generate |> Gen.map SkuId |> Arb.fromGen
312312

313313
type DomainPropertyAttribute() =
314314
inherit FsCheck.Xunit.PropertyAttribute(QuietOnSuccess = true, Arbitrary=[| typeof<FsCheckGenerators> |])

‎tests/FsCodec.SystemTextJson.Tests/AutoUnionTests.fs‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,7 @@ let [<FsCheck.Xunit.Property>] ``auto-encodes Unions and non-unions`` (x : Any)
4848

4949
test <@ decoded = x @>
5050

51-
(* 🙈 *)
52-
53-
let (|ReplaceSomeNullWithNone|) value = TypeShape.Generic.map (function Some (null : string) -> None | x -> x) value
54-
55-
let [<FsCheck.Xunit.Property>] ``Some null roundtripping hack for tests`` (ReplaceSomeNullWithNone (x : Any)) =
51+
let [<FsCheck.Xunit.Property>] ``It round trips`` (x: Any) =
5652
let encoded = serdes.Serialize x
5753
let decoded : Any = serdes.Deserialize encoded
5854
test <@ decoded = x @>

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
</PropertyGroup>
88

99
<ItemGroup>
10-
<PackageReference Include="FsCheck.Xunit" Version="2.14.3" />
10+
<PackageReference Include="FsCheck.Xunit" Version="3.0.0-beta2" />
1111
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.1.0" />
1212
<PackageReference Include="Serilog.Sinks.Console" Version="4.0.1" />
1313
<PackageReference Include="Unquote" Version="6.1.0" />

‎tests/FsCodec.SystemTextJson.Tests/SerdesTests.fs‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
module FsCodec.SystemTextJson.Tests.SerdesTests
22

3+
open System
34
open System.Collections.Generic
45
open FsCodec.SystemTextJson
56
open Swensen.Unquote
@@ -8,6 +9,7 @@ open Xunit
89
type Record = { a : int }
910

1011
type RecordWithOption = { a : int; b : string option }
12+
type RecordWithString = { c : int; d : string }
1113

1214
/// Characterization tests for OOTB JSON.NET
1315
/// 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 =
6567
let des = serdes.Deserialize ser
6668
test <@ value = des @>
6769

70+
let [<Fact>] ``RejectNullStringConverter rejects null strings`` () =
71+
let serdes = Serdes(Options.Create(rejectNullStrings = true))
72+
73+
let value: string = null
74+
raises<ArgumentNullException> <@ serdes.Serialize value @>
75+
76+
let value = [| "A"; null |]
77+
raises<ArgumentNullException> <@ serdes.Serialize value @>
78+
79+
let value = { c = 1; d = null }
80+
raises<ArgumentNullException> <@ serdes.Serialize value @>
81+
82+
let [<Fact>] ``RejectNullStringConverter serializes strings correctly`` () =
83+
let serdes = Serdes(Options.Create(rejectNullStrings = true))
84+
let value = { c = 1; d = "some string" }
85+
let res = serdes.Serialize value
86+
test <@ res = """{"c":1,"d":"some string"}""" @>
87+
let des = serdes.Deserialize res
88+
test <@ des = value @>
89+
90+
[<Theory; InlineData(true); InlineData(false)>]
91+
let ``string options are supported regardless of "rejectNullStrings" value`` rejectNullStrings =
92+
let serdes = Serdes(Options.Create(rejectNullStrings = rejectNullStrings))
93+
let value = [| Some "A"; None |]
94+
let res = serdes.Serialize value
95+
test <@ res = """["A",null]""" @>
96+
let des = serdes.Deserialize res
97+
test <@ des = value @>
98+
99+
68100
(* Serdes + default Options behavior, i.e. the stuff we do *)
69101

70102
let serdes = Serdes Options.Default

0 commit comments

Comments
 (0)