diff --git a/src/FSharp.SystemTextJson/All.fs b/src/FSharp.SystemTextJson/All.fs index 66b9d4e..9dc1e66 100644 --- a/src/FSharp.SystemTextJson/All.fs +++ b/src/FSharp.SystemTextJson/All.fs @@ -130,7 +130,7 @@ type JsonFSharpConverterAttribute(fsOptions: JsonFSharpOptions) = member _.UnionFieldNamesFromTypes with set v = fsOptions <- fsOptions.WithUnionFieldNamesFromTypes(v) member _.SkippableOptionFields - with set v = fsOptions <- fsOptions.WithSkippableOptionFields(v) + with set v = fsOptions <- fsOptions.WithSkippableOptionFields(v: SkippableOptionFields) new() = JsonFSharpConverterAttribute(JsonFSharpOptions()) diff --git a/src/FSharp.SystemTextJson/Helpers.fs b/src/FSharp.SystemTextJson/Helpers.fs index 9ca4a94..9ccdd22 100644 --- a/src/FSharp.SystemTextJson/Helpers.fs +++ b/src/FSharp.SystemTextJson/Helpers.fs @@ -38,12 +38,17 @@ let isNullableUnion (ty: Type) = x.Flags.HasFlag(CompilationRepresentationFlags.UseNullAsTrueValue) ) +let isOptionType (ty: Type) = + ty.IsGenericType + && let genTy = ty.GetGenericTypeDefinition() in + genTy = typedefof> || genTy = typedefof> + let isSkippableType (fsOptions: JsonFSharpOptionsRecord) (ty: Type) = if ty.IsGenericType then let genTy = ty.GetGenericTypeDefinition() genTy = typedefof> - || (fsOptions.SkippableOptionFields - && (genTy = typedefof> || genTy = typedefof>)) + || (fsOptions.SkippableOptionFields = SkippableOptionFields.Always + && isOptionType ty) else false @@ -146,7 +151,12 @@ type FieldHelper options.IgnoreNullValues || options.DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull let canBeSkipped = - (ignoreNullValues && (nullValue.IsSome || isClass ty)) || isSkippableWrapperType + if isOptionType ty then + (fsOptions.SkippableOptionFields = SkippableOptionFields.Always) + || (ignoreNullValues + && fsOptions.SkippableOptionFields = SkippableOptionFields.FromJsonSerializerOptions) + else + (ignoreNullValues && (nullValue.IsSome || isClass ty)) || isSkippableWrapperType let deserializeType = if isSkippableWrapperType then ty.GenericTypeArguments[0] else ty @@ -166,7 +176,7 @@ type FieldHelper fun _ -> false let ignoreOnWrite (v: obj) = - isSkip v || (ignoreNullValues && isNull v) + canBeSkipped && (isSkip v || isNull v) let defaultValue = if isSkippableWrapperType || isValueOptionType ty then diff --git a/src/FSharp.SystemTextJson/Options.fs b/src/FSharp.SystemTextJson/Options.fs index 7f34303..5b9e173 100644 --- a/src/FSharp.SystemTextJson/Options.fs +++ b/src/FSharp.SystemTextJson/Options.fs @@ -120,6 +120,14 @@ type JsonFSharpTypes = /// All supported types. | All = 0xfff +type SkippableOptionFields = + /// None and ValueNone fields in records and unions are skippable if the JsonSerializerOptions has DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull. + | FromJsonSerializerOptions = 0 + /// None and ValueNone fields in records and unions are never skippable. + | Never = 1 + /// None and ValueNone fields in records and unions are always skippable. + | Always = 2 + module internal Default = [] @@ -158,7 +166,7 @@ type internal JsonFSharpOptionsRecord = UnionTagCaseInsensitive: bool AllowNullFields: bool IncludeRecordProperties: bool - SkippableOptionFields: bool + SkippableOptionFields: SkippableOptionFields Types: JsonFSharpTypes AllowOverride: bool Overrides: JsonFSharpOptions -> IDictionary } @@ -191,7 +199,7 @@ and JsonFSharpOptions internal (options: JsonFSharpOptionsRecord) = UnionTagCaseInsensitive = unionTagCaseInsensitive AllowNullFields = allowNullFields IncludeRecordProperties = includeRecordProperties - SkippableOptionFields = false + SkippableOptionFields = SkippableOptionFields.FromJsonSerializerOptions Types = types AllowOverride = allowOverride Overrides = emptyOverrides } @@ -262,17 +270,25 @@ and JsonFSharpOptions internal (options: JsonFSharpOptionsRecord) = member _.WithIncludeRecordProperties([] includeRecordProperties) = JsonFSharpOptions({ options with IncludeRecordProperties = includeRecordProperties }) - member _.WithSkippableOptionFields([] skippableOptionFields) = + member _.WithSkippableOptionFields(skippableOptionFields) = JsonFSharpOptions( { options with SkippableOptionFields = skippableOptionFields UnionEncoding = - if skippableOptionFields then + if skippableOptionFields = SkippableOptionFields.Always then options.UnionEncoding ||| JsonUnionEncoding.UnwrapOption else options.UnionEncoding } ) + + member this.WithSkippableOptionFields([] skippableOptionFields) = + if skippableOptionFields then + SkippableOptionFields.Always + else + SkippableOptionFields.FromJsonSerializerOptions + |> this.WithSkippableOptionFields + member _.WithTypes(types) = JsonFSharpOptions({ options with Types = types }) diff --git a/src/FSharp.SystemTextJson/Record.fs b/src/FSharp.SystemTextJson/Record.fs index 2e597ce..d5ca23d 100644 --- a/src/FSharp.SystemTextJson/Record.fs +++ b/src/FSharp.SystemTextJson/Record.fs @@ -199,7 +199,7 @@ type JsonRecordConverter<'T> internal (options: JsonSerializerOptions, fsOptions if requiredFieldCount < minExpectedFieldCount then for i in 0 .. fieldCount - 1 do - if isNull fields[i] && fieldProps[i].MustBePresent then + if fields[i] = defaultFields[i] && fieldProps[i].MustBePresent then failf "Missing field for record type %s: %s" recordType.FullName fieldProps[i].Names[0] ctor fields :?> 'T diff --git a/tests/FSharp.SystemTextJson.Tests/Test.Record.fs b/tests/FSharp.SystemTextJson.Tests/Test.Record.fs index 7429051..ecb3438 100644 --- a/tests/FSharp.SystemTextJson.Tests/Test.Record.fs +++ b/tests/FSharp.SystemTextJson.Tests/Test.Record.fs @@ -223,6 +223,39 @@ module NonStruct = ) Assert.Equal("""{"sa":1,"sb":2,"sc":3,"sd":4}""", actual) + type NonSkO = { x: int option } + + [] + let ``deserialize non-skippable option field even with WhenWritingNull`` () = + let options = + JsonFSharpOptions + .Default() + .WithSkippableOptionFields(SkippableOptionFields.Never) + .ToJsonSerializerOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull) + + let actual = JsonSerializer.Deserialize("""{"x":1}""", options) + Assert.Equal({ x = Some 1 }, actual) + + let actual = JsonSerializer.Deserialize("""{"x":null}""", options) + Assert.Equal({ x = None }, actual) + + let ex = + Assert.Throws(fun () -> JsonSerializer.Deserialize("{}", options) |> ignore) + Assert.Contains("", ex.Message) + + [] + let ``serialize non-skippable option field even with WhenWritingNull`` () = + let options = + JsonFSharpOptions + .Default() + .WithSkippableOptionFields(SkippableOptionFields.Never) + .ToJsonSerializerOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull) + let actual = JsonSerializer.Serialize({ x = Some 1 }, options) + Assert.Equal("""{"x":1}""", actual) + + let actual = JsonSerializer.Serialize({ x = None }, options) + Assert.Equal("""{"x":null}""", actual) + type C = { cx: B } [] @@ -695,6 +728,40 @@ module Struct = ) Assert.Equal("""{"sa":1,"sb":2,"sc":3,"sd":4}""", actual) + [] + type NonSkO = { x: int option } + + [] + let ``deserialize non-skippable option field even with WhenWritingNull`` () = + let options = + JsonFSharpOptions + .Default() + .WithSkippableOptionFields(SkippableOptionFields.Never) + .ToJsonSerializerOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull) + + let actual = JsonSerializer.Deserialize("""{"x":1}""", options) + Assert.Equal({ x = Some 1 }, actual) + + let actual = JsonSerializer.Deserialize("""{"x":null}""", options) + Assert.Equal({ x = None }, actual) + + let ex = + Assert.Throws(fun () -> JsonSerializer.Deserialize("{}", options) |> ignore) + Assert.Contains("", ex.Message) + + [] + let ``serialize non-skippable option field even with WhenWritingNull`` () = + let options = + JsonFSharpOptions + .Default() + .WithSkippableOptionFields(SkippableOptionFields.Never) + .ToJsonSerializerOptions(DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull) + let actual = JsonSerializer.Serialize({ x = Some 1 }, options) + Assert.Equal("""{"x":1}""", actual) + + let actual = JsonSerializer.Serialize({ x = None }, options) + Assert.Equal("""{"x":null}""", actual) + [] type C = { cx: B } diff --git a/tests/FSharp.SystemTextJson.Tests/Test.Regression.fs b/tests/FSharp.SystemTextJson.Tests/Test.Regression.fs index 8ebd0cc..7016d09 100644 --- a/tests/FSharp.SystemTextJson.Tests/Test.Regression.fs +++ b/tests/FSharp.SystemTextJson.Tests/Test.Regression.fs @@ -99,3 +99,15 @@ let ``regression #123`` () = { FirstName = "yarr"; LastName = None; age = 5 }, JsonSerializer.Deserialize("""{"FirstName": "yarr", "age": 5 }""", skipOptions2) ) + +type R = { x: int option } +type RV = { x: int voption } + +[] +let ``regression #154`` () = + let o = + JsonFSharpOptions().WithSkippableOptionFields(false).ToJsonSerializerOptions() + Assert.Throws(fun () -> JsonSerializer.Deserialize("{}", o) |> ignore) + |> ignore + Assert.Throws(fun () -> JsonSerializer.Deserialize("{}", o) |> ignore) + |> ignore