Skip to content

Commit

Permalink
RejectNullConverter WIP
Browse files Browse the repository at this point in the history
  • Loading branch information
bartelink committed Jan 24, 2024
1 parent 739bdda commit eb6e417
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 13 deletions.
3 changes: 2 additions & 1 deletion src/FsCodec.SystemTextJson/FsCodec.SystemTextJson.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<Compile Include="TypeSafeEnumConverter.fs" />
<Compile Include="RejectNullStringConverter.fs" />
<Compile Include="UnionOrTypeSafeEnumConverterFactory.fs" />
<Compile Include="RejectNullConverter.fs" />
<Compile Include="Options.fs" />
<Compile Include="Serdes.fs" />
<Compile Include="Codec.fs" />
Expand All @@ -22,7 +23,7 @@

<PackageReference Include="FSharp.Core" Version="4.5.4" />

<PackageReference Include="System.Text.Json" Version="6.0.1" />
<PackageReference Include="System.Text.Json" Version="8.0.1" />
</ItemGroup>

<ItemGroup>
Expand Down
6 changes: 5 additions & 1 deletion src/FsCodec.SystemTextJson/Options.fs
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,18 @@ type Options private () =
[<Optional; DefaultParameterValue(null)>] ?autoUnionToJsonObject : bool,
// 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) =
[<Optional; DefaultParameterValue(null)>] ?rejectNullStrings: bool,
// Apply <c>RejectNullConverter</c> in order to have serialization throw on <c>null</c> on <c>null</c> or missing <c>list</c> or <c>Set</c> values.
// Wrap the type in <c>option</c> to represent values that can potentially be <c>null</c> or missing
[<Optional; DefaultParameterValue(null)>] ?rejectNull: bool) =
let autoTypeSafeEnumToJsonString = defaultArg autoTypeSafeEnumToJsonString false
let autoUnionToJsonObject = defaultArg autoUnionToJsonObject false
let rejectNullStrings = defaultArg rejectNullStrings false

Options.CreateDefault(
converters = [|
if rejectNullStrings then RejectNullStringConverter()
if defaultArg rejectNull false then RejectNullConverterFactory()
if autoTypeSafeEnumToJsonString || autoUnionToJsonObject then
UnionOrTypeSafeEnumConverterFactory(typeSafeEnum = autoTypeSafeEnumToJsonString, union = autoUnionToJsonObject)
if converters <> null then yield! converters
Expand Down
42 changes: 42 additions & 0 deletions src/FsCodec.SystemTextJson/RejectNullConverter.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
namespace FsCodec.SystemTextJson

open System
open System.Linq.Expressions
open System.Text.Json
open System.Text.Json.Serialization

type RejectNullConverter<'T>() =
inherit System.Text.Json.Serialization.JsonConverter<'T>()

static let defaultConverter = JsonSerializerOptions.Default.GetConverter(typeof<'T>) :?> JsonConverter<'T>
let msg () = sprintf "Expected value, got null. When rejectNull is true you must explicitly wrap optional %s values in an 'option'" typeof<'T>.Name

override _.HandleNull = true

override _.Read(reader, typeToConvert, options) =
if reader.TokenType = JsonTokenType.Null then msg () |> nullArg else
defaultConverter.Read(&reader, typeToConvert, options)
// Pretty sure the above is the correct approach (and this unsurprisingly loops, blowing the stack)
// JsonSerializer.Deserialize(&reader, typeToConvert, options) :?> 'T

override _.Write(writer, value, options) =
if value |> box |> isNull then msg () |> nullArg
defaultConverter.Write(writer, value, options)
// JsonSerializer.Serialize<'T>(writer, value, options)

type RejectNullConverterFactory(predicate) =
inherit JsonConverterFactory()
new() =
RejectNullConverterFactory(fun (t: Type) ->
t.IsGenericType
&& let gtd = t.GetGenericTypeDefinition() in gtd = typedefof<Set<_>> || gtd = typedefof<list<_>>)
override _.CanConvert(t: Type) = predicate t

override _.CreateConverter(t, _options) =
let openConverterType = typedefof<RejectNullConverter<_>>
let constructor = openConverterType.MakeGenericType(t).GetConstructors() |> Array.head
let newExpression = Expression.New(constructor)
let lambda = Expression.Lambda(typeof<ConverterActivator>, newExpression)

let activator = lambda.Compile() :?> ConverterActivator
activator.Invoke()
11 changes: 4 additions & 7 deletions src/FsCodec.SystemTextJson/RejectNullStringConverter.fs
Original file line number Diff line number Diff line change
@@ -1,22 +1,19 @@
namespace FsCodec.SystemTextJson

open System.Text.Json.Serialization
type RejectNullStringConverter() =
inherit System.Text.Json.Serialization.JsonConverter<string>()

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
if value = null then nullArg message
value

override this.Write(writer, value, _options) =
if value = null then nullArg Error.message
if value = null then nullArg message
writer.WriteStringValue(value)
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,10 @@ type UnionOrTypeSafeEnumConverterFactory(typeSafeEnum, union) =
inherit JsonConverterFactory()

static let hasConverterAttribute = memoize (fun (t: Type) -> t.IsDefined(typeof<JsonConverterAttribute>, true))
let isIntrinsic (t: Type) =
t.IsGenericType
&& (let gtd = t.GetGenericTypeDefinition() in gtd = typedefof<option<_>> || gtd = typedefof<list<_>>)

override _.CanConvert(t: Type) =
not (isIntrinsic t)
t.IsGenericType
&& not (let gtd = t.GetGenericTypeDefinition() in gtd = typedefof<option<_>> || gtd = typedefof<list<_>>)
&& FsCodec.Union.isUnion t
&& not (hasConverterAttribute t)
&& ((typeSafeEnum && union)
Expand Down
27 changes: 27 additions & 0 deletions tests/FsCodec.SystemTextJson.Tests/SerdesTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ module FsCodec.SystemTextJson.Tests.SerdesTests

open System
open System.Collections.Generic
open System.Text.Json.Serialization.Metadata
open FsCodec.SystemTextJson
open Swensen.Unquote
open Xunit
Expand Down Expand Up @@ -79,6 +80,32 @@ let [<Fact>] ``RejectNullStringConverter rejects null strings`` () =
let value = { c = 1; d = null }
raises<ArgumentNullException> <@ serdes.Serialize value @>

type WithList = { x: int; y: list<int> }

let [<Fact>] ``RejectNullConverter rejects null lists and Sets`` () =
#if false // requires WithList to be CLIMutable, which would be a big imposition
let tir =
DefaultJsonTypeInfoResolver()
.WithAddedModifier(fun x ->
// if x.Kind <> JsonTypeInfoKind.Object then
for p in x.Properties do
let pt = p.PropertyType
if pt.IsGenericType && (let gtd = pt.GetGenericTypeDefinition() in gtd = typedefof<list<_>> || gtd = typedefof<Set<_>>) then
p.IsRequired <- true)
let serdes = Options.Create(TypeInfoResolver = tir) |> Serdes
#else
let serdes = Options.Create(rejectNull = true) |> Serdes
#endif

// Fails with NRE when RejectNullConverter delegates to Default list<int> Converter
// seems akin to https://github.com/dotnet/runtime/issues/86483
let res = serdes.Deserialize<WithList> """{"x":0,"y":[1]}"""
test <@ [1] = res.y @>

raises<exn> <@ serdes.Deserialize<WithList> """{"x":0}""" @>
// PROBLEM: there doesn't seem to be a way to intercept explicitly passed nulls
// raises<JsonException> <@ serdes.Deserialize<WithList> """{"x":0,"y":null}""" @>

let [<Fact>] ``RejectNullStringConverter serializes strings correctly`` () =
let serdes = Serdes(Options.Create(rejectNullStrings = true))
let value = { c = 1; d = "some string" }
Expand Down

0 comments on commit eb6e417

Please sign in to comment.