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

SystemTextJson: Add option to reject null string values #87

Merged
merged 11 commits into from
Nov 30, 2022
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/FsCodec.SystemTextJson/FsCodec.SystemTextJson.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
11 changes: 8 additions & 3 deletions src/FsCodec.SystemTextJson/Options.fs
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,19 @@ 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>When set to <c>false</c> the codec will throw on <c>null</c> strings. Use <c>string option</c> to allow nulls.
nordfjord marked this conversation as resolved.
Show resolved Hide resolved
[<Optional; DefaultParameterValue(null)>] ?allowNullStrings : bool) =

let defaultConverters: JsonConverter array =
if allowNullStrings = Some false then [| RejectNullStringConverter() |] else Array.empty
bartelink marked this conversation as resolved.
Show resolved Hide resolved
let converters = if converters = null then defaultConverters else Array.append converters defaultConverters

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
Array.append converters [| UnionOrTypeSafeEnumConverterFactory(typeSafeEnum = tse, union = u) |]
bartelink marked this conversation as resolved.
Show resolved Hide resolved
| _ -> converters),
?ignoreNulls = ignoreNulls,
?indent = indent,
Expand Down
18 changes: 18 additions & 0 deletions src/FsCodec.SystemTextJson/RejectNullStringConverter.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
namespace FsCodec.SystemTextJson

open System.Text.Json.Serialization

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 "Expected string, got null. When allowNullStrings is false you must explicitly type optional strings as 'string option'"
bartelink marked this conversation as resolved.
Show resolved Hide resolved
value

override this.Write(writer, value, _options) =
if value = null then nullArg "Expected string, got null. When allowNullStrings is false you must explicitly type optional strings as 'string option'"
writer.WriteStringValue(value)
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
4 changes: 2 additions & 2 deletions tests/FsCodec.NewtonsoftJson.Tests/StreamTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 3 additions & 3 deletions tests/FsCodec.NewtonsoftJson.Tests/UnionConverterTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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> |])
Expand Down
6 changes: 1 addition & 5 deletions tests/FsCodec.SystemTextJson.Tests/AutoUnionTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -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 @>
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down
25 changes: 25 additions & 0 deletions tests/FsCodec.SystemTextJson.Tests/SerdesTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,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
Expand Down Expand Up @@ -121,3 +122,27 @@ let [<Fact>] ``Switches off the HTML over-escaping mechanism`` () =
test <@ ser = """{"a":1,"b":"\"+"}""" @>
let des = serdes.Deserialize ser
test <@ value = des @>

let [<Fact>] ``Rejects null strings`` () =
let serdes = Serdes(Options.Create(allowNullStrings = false))
let value: string = null
raises <@ serdes.Serialize value @>
bartelink marked this conversation as resolved.
Show resolved Hide resolved

let value = [| "A"; null |]
raises <@ serdes.Serialize value @>

let value = [| Some "A"; None |]
bartelink marked this conversation as resolved.
Show resolved Hide resolved
let res = serdes.Serialize value
test <@ res = """["A",null]""" @>
let des = serdes.Deserialize res
test <@ des = value @>

let value = { c = 1; d = null }
raises <@ serdes.Serialize value @>

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 @>