Skip to content

Commit

Permalink
Update to STJ V6
Browse files Browse the repository at this point in the history
  • Loading branch information
bartelink committed Sep 9, 2021
1 parent b45fefc commit 927271c
Show file tree
Hide file tree
Showing 8 changed files with 18 additions and 62 deletions.
6 changes: 2 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ While this may not seem like a sufficiently large set of converters for a large
### Core converters

The respective concrete Codec packages include relevant `Converter`/`JsonConverter` in order to facilitate interoperable and versionable renderings:
- `JsonOptionConverter` / [`OptionConverter`](https://github.com/jet/FsCodec/blob/master/src/FsCodec.NewtonsoftJson/OptionConverter.fs#L7) represents F#'s `Option<'t>` as a value or `null`; included in the standard `Settings.Create`/`Options.Create` profile. `System.Text.Json` reimplementation :pray: [@ylibrach](https://github.com/ylibrach)
- [`TypeSafeEnumConverter`](https://github.com/jet/FsCodec/blob/master/src/FsCodec.NewtonsoftJson/TypeSafeEnumConverter.fs#L33) represents discriminated union (whose cases are all nullary), as a `string` in a trustworthy manner (`Newtonsoft.Json.Converters.StringEnumConverter` permits values outside the declared values) :pray: [@amjjd](https://github.com/amjjd)
- [`UnionConverter`](https://github.com/jet/FsCodec/blob/master/src/FsCodec.NewtonsoftJson/UnionConverter.fs#L71) represents F# discriminated unions as a single JSON `object` with both the tag value and the body content as named fields directly within :pray: [@amjdd](https://github.com/amjjd); `System.Text.Json` reimplementation :pray: [@NickDarvey](https://github.com/NickDarvey)

Expand Down Expand Up @@ -106,7 +105,6 @@ The respective concrete Codec packages include relevant `Converter`/`JsonConvert
[`FsCodec.SystemTextJson.Options`](https://github.com/jet/FsCodec/blob/stj/src/FsCodec.SystemTextJson/Options.fs#L8) provides a clean syntax for building a `System.Text.Json.Serialization.JsonSerializerOptions` as per `FsCodec.NewtonsoftJson.Settings`, above. Methods:
- `CreateDefault`: equivalent to generating a `new JsonSerializerSettings()` without any overrides of any kind
- `Create`: as `CreateDefault` with the following difference:
- adds a `JsonOptionConverter`; included in default `Settings` (see _Converters_, below)
- Inhibits the HTML-safe escaping that `System.Text.Json` provides as a default by overriding `Encoder` with `System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping`

## `Serdes`
Expand All @@ -126,7 +124,7 @@ If you follow the policies covered in the rest of the documentation here, your D
2. Types that require a global converter to be registered. _While it may seem that the second set is open-ended and potentially vast, experience teaches that you want to keep it minimal._. This boils down to:
- records arrays and all other good choices for types Just Work already
- `Nullable<MyType>`: Handled out of the box by both NSJ and STJ - requires no converters, provides excellent interop with other CLR languages. Would recommend.
- `MyType option`: Covered by the global `OptionConverter`/`JsonOptionConverter` (see below for a clean way to add them to the default MVC view rendering configuration). Note that while this works well with ASP.NET Core, it may be problematic if you share contracts (yes, not saying you should) or rely on things like Swashbuckle which will need to be aware of the types when they reflect over them.
- `MyType option`: Covered by the global `OptionConverter` for Newtonsoft, handled intrinsically by `System.Text.Json` versions `>= 6` (see below for a clean way to add them to the default MVC view rendering configuration). Note that while this works well with ASP.NET Core, it may be problematic if you share contracts (yes, not saying you should) or rely on things like Swashbuckle which will need to be aware of the types when they reflect over them.

**The bottom line is that using exotic types in DTOs is something to think very hard about before descending into. The next sections are thus only relevant if you decide to add that extra complexity to your system...**

Expand All @@ -153,7 +151,7 @@ The equivalent for the native `System.Text.Json` looks like this:
|> Seq.iter options.JsonSerializerOptions.Converters.Add
) |> ignore

_As of `System.Text.Json` v5, the only converter used under the hood is `FsCodec.SystemTextJson.JsonOptionConverter`. [In v6, the `OptionConverter` goes](https://github.com/dotnet/runtime/pull/55108)._
_As of `System.Text.Json` v 6, thanks [to the great work of the .NET team](https://github.com/dotnet/runtime/pull/55108), the internal `JsonOptionConverter` goes, making the above a no-op._

# Examples: `FsCodec.(Newtonsoft|SystemText)Json`

Expand Down
3 changes: 1 addition & 2 deletions src/FsCodec.SystemTextJson/FsCodec.SystemTextJson.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

<ItemGroup>
<Compile Include="JsonSerializerElementExtensions.fs" />
<Compile Include="JsonOptionConverter.fs" />
<Compile Include="Pickler.fs" />
<Compile Include="UnionConverter.fs" />
<Compile Include="TypeSafeEnumConverter.fs" />
Expand All @@ -26,7 +25,7 @@

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

<PackageReference Include="System.Text.Json" Version="5.0.1" />
<PackageReference Include="System.Text.Json" Version="6.0.0-preview.7.21377.19" />
<PackageReference Include="TypeShape" Version="8.0.0" />
</ItemGroup>

Expand Down
2 changes: 1 addition & 1 deletion src/FsCodec.SystemTextJson/Interop.fs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type InteropExtensions =
else JsonSerializer.Deserialize(System.ReadOnlySpan.op_Implicit x)
static member private MapTo(x: JsonElement) : byte[] =
if x.ValueKind = JsonValueKind.Undefined then null
else JsonSerializer.SerializeToUtf8Bytes(x, InteropExtensions.NoOverEscapingOptions)
else JsonSerializer.SerializeToUtf8Bytes(x, options = InteropExtensions.NoOverEscapingOptions)
// Avoid introduction of HTML escaping for things like quotes etc (as standard Options.Create() profile does)
static member private NoOverEscapingOptions =
System.Text.Json.JsonSerializerOptions(Encoder = System.Text.Encodings.Web.JavaScriptEncoder.UnsafeRelaxedJsonEscaping)
Expand Down
36 changes: 0 additions & 36 deletions src/FsCodec.SystemTextJson/JsonOptionConverter.fs

This file was deleted.

5 changes: 2 additions & 3 deletions src/FsCodec.SystemTextJson/Options.fs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ open System.Text.Json.Serialization

type Options private () =

static let defaultConverters : JsonConverter[] = [| JsonOptionConverter() |]
static let defaultConverters : JsonConverter[] = [| |]

/// Creates a default set of serializer options used by Json serialization. When used with no args, same as `JsonSerializerOptions()`
static member CreateDefault
Expand All @@ -34,12 +34,11 @@ type Options private () =
options

/// Opinionated helper that creates serializer settings that represent good defaults for F# <br/>
/// - Always prepends `[JsonOptionConverter()]` to any converters supplied <br/>
/// - no camel case conversion - assumption is you'll use records with camelCased names <br/>
/// - renders values with `UnsafeRelaxedJsonEscaping` - i.e. minimal escaping as per `NewtonsoftJson`<br/>
/// Everything else is as per CreateDefault:- i.e. emit nulls instead of omitting fields, no indenting, no camelCase conversion
static member Create
( /// List of converters to apply. Implicit [JsonOptionConverter()] will be prepended and/or be used as a default
( /// List of converters to apply. Implicit converters may be prepended and/or be used as a default
[<Optional; ParamArray>] converters : JsonConverter[],
/// Use multi-line, indented formatting when serializing JSON; defaults to false.
[<Optional; DefaultParameterValue(null)>] ?indent : bool,
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 @@ -77,15 +77,15 @@ type TestDU =
// Centred on ignoreNulls for backcompat; round-tripping test covers the case where they get rendered too

#if SYSTEM_TEXT_JSON
let serializeWith<'t> profile value = JsonSerializer.Serialize(value, profile)
let serializeWith<'t> profile value = JsonSerializer.Serialize(value, options = profile)
let defaultOptions = Options.Create(camelCase = false, ignoreNulls = true)
let serializeDefault<'t> value = serializeWith<'t> defaultOptions value

let deserializeWith<'t> profile (serialized : string) = JsonSerializer.Deserialize<'t>(serialized, profile)
let deserializeWith<'t> profile (serialized : string) = JsonSerializer.Deserialize<'t>(serialized, options = profile)
let inline deserializeDefault<'t> serialized = deserializeWith<'t> defaultOptions serialized

let assertIgnoreNullsIs value (profile : JsonSerializerOptions) =
profile.IgnoreNullValues =! value
profile.DefaultIgnoreCondition =! if value then JsonIgnoreCondition.Always else JsonIgnoreCondition.Never
#else
let serializeWith<'t> (profile : JsonSerializerSettings) (value : 't) = JsonConvert.SerializeObject(value, profile)
let settings = Settings.Create(camelCase = false, ignoreNulls = true)
Expand Down
19 changes: 7 additions & 12 deletions tests/FsCodec.SystemTextJson.Tests/SerdesTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type RecordWithOption = { a : int; b : string option }
module StjCharacterization =
let ootbOptions = Options.CreateDefault()

let [<Fact>] ``OOTB STJ records`` () =
let [<Fact>] ``OOTB STJ records Just Works`` () =
// Ver 5.x includes standard support for calling a single ctor (4.x required a custom implementation)
let value = { a = 1 }
let ser = Serdes.Serialize(value, ootbOptions)
Expand All @@ -23,35 +23,30 @@ module StjCharacterization =
let res = Serdes.Deserialize<Record>(ser, ootbOptions)
test <@ res = value @>

let [<Fact>] ``OOTB STJ options`` () =
let [<Fact>] ``OOTB STJ options Just Works`` () =
let value = { a = 1; b = Some "str" }
let ser = Serdes.Serialize(value, ootbOptions)
test <@ ser = """{"a":1,"b":{"Value":"str"}}""" @>
test <@ ser = """{"a":1,"b":"str"}""" @>

let correctSer = """{"a":1,"b":"str"}"""
raisesWith <@ Serdes.Deserialize<RecordWithOption>(correctSer, ootbOptions) @>
<| fun e -> <@ e.Message.Contains "The JSON value could not be converted to Microsoft.FSharp.Core.FSharpOption`1[System.String]" @>
test <@ value = Serdes.Deserialize<RecordWithOption>(ser, ootbOptions) @>

let [<Fact>] ``OOTB STJ lists`` () =
let [<Fact>] ``OOTB STJ lists Just Works`` () =
let value = [ "A"; "B" ]
let ser = Serdes.Serialize(value, ootbOptions)
test <@ ser = """["A","B"]""" @>

let correctSer = """["A,"B"]"""
raisesWith <@ Serdes.Deserialize<string list>(correctSer, ootbOptions) @>
<| fun e -> <@ e.Message.Contains "The collection type 'Microsoft.FSharp.Collections.FSharpList`1[System.String]' is abstract, an interface, or is read only, and could not be instantiated and populated" @>
test <@ value = Serdes.Deserialize<string list>(ser, ootbOptions) @>

// System.Text.Json's JsonSerializerOptions by default escapes HTML-sensitive characters when generating JSON strings
// while this arguably makes sense as a default
// - it's not particularly relevant for event encodings
// - and is not in alignment with the FsCodec.NewtonsoftJson default options
// see https://github.com/dotnet/runtime/issues/28567#issuecomment-53581752 for lowdown
let asRequiredForExamples : System.Text.Json.Serialization.JsonConverter [] = [| JsonOptionConverter() |]
type OverescapedOptions() as this =
inherit TheoryData<System.Text.Json.JsonSerializerOptions>()

do // OOTB System.Text.Json over-escapes HTML-sensitive characters - `CreateDefault` honors this
this.Add(Options.CreateDefault(converters = asRequiredForExamples)) // the value we use here requires two custom Converters
this.Add(Options.CreateDefault()) // the value we use here requires two custom Converters
// Options.Create provides a simple way to override it
this.Add(Options.Create(unsafeRelaxedJsonEscaping = false))
let [<Theory; ClassData(typedefof<OverescapedOptions>)>] ``provides various ways to use HTML-escaped encoding``(opts : System.Text.Json.JsonSerializerOptions) =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@ let [<Fact>] happy () =
test <@ Joy = Serdes.Deserialize("\"Joy\"", optionsWithOutcomeConverter) @>
test <@ Some Joy = Serdes.Deserialize("\"Joy\"", optionsWithOutcomeConverter) @>
raises<KeyNotFoundException> <@ Serdes.Deserialize<Outcome>("\"Confusion\"", optionsWithOutcomeConverter) @>
raises<JsonException> <@ Serdes.Deserialize<Outcome> "1" @>
// Was a JsonException prior to V6
raises<NotSupportedException> <@ Serdes.Deserialize<Outcome> "1" @>

let [<Fact>] sad () =
raises<ArgumentException> <@ TypeSafeEnum.tryParse<string> "Wat" @>
Expand Down

0 comments on commit 927271c

Please sign in to comment.