jet / fscodec Goto Github PK

In a team environment, where the way event contracts are composed is often a matter of debate, it can be useful to have a way to validate that types that will be mapped to JSON adhere to conventions.

It should be pretty possible to, a la AutoFixture.Idioms use reflection to find all DUs that implement TypeShape.UnionEncoder.IUnionContract, and then walk the type with TypeShape counting anomalies such as

• using FSharpList without a converter (use array or [] option)
• using tuples anywhere in a union contract (a converter, e.g. based on a JsonIsomorphism should be permitted, but we definitely don't want random Newtonsoft/STJ representations and/or runtime exceptions)
• Using DateTime (DateTimeOffset is hands down better)
• Using Enums (use nullary unions / TypeSafeEnums in preference)
• using SCDUs without a converter (should probably be using UMX in the data contracts and only using SCDUs where warranted in models)
• using other unions without tagging the type and/or field with a converter (e.g. UnionConverter) (vs ending up e.g. triggering Newtonsofts built-in encoding)

This would enable a team for write a single unit test saying something like:

let [<Fact>] ``All contract types use simple tagged types`` () =
    let checker = FsCodec.NewtonsoftJson.TypeChecker(allowMaps=true) // or any other exceptions to the search
    let anoms =
        TypesImplementing<TypeShape.UnionEncoder.IUnionContract>(typeof<Domain.SampleType>.Assembly)
        |> Seq.collect checker.EnumViolations
        |> List.ofSeq
    test <@ [] = anoms @>

Reasons to have this in FsCodec vs anywhere else

1. it's already using TypeShape
2. we can port it to do the same thing for SystemTextJson

Yes, one could build a Roslyn Analyzer and/or Rider or Ionide checks too!

global.json currently targets a specific .net core 3.1 SDK version

As result when I try to build the project on my machine I get following error:

dotnet build
A compatible installed .NET Core SDK for global.json version [3.1.101] from [/home/klimisa/Personal/dev/jet/FsCodec/global.json] was not found
Install the [3.1.101] .NET Core SDK or update [/home/klimisa/Personal/dev/jet/FsCodec/global.json] with an installed .NET Core SDK:
  3.1.402 [/usr/share/dotnet/sdk]

Therefore, if you have only the latest version then you can't build or run any tests.

This might just be my ignorance, but I am having great difficulty in using the UnionEncoder, with Equinox to create events.

I have this:

type WeirdThing = {value: string; active: bool}
type WeirdThing2 = {value: string; active: bool; location: string }

type Something = 
  | WeirdThing of WeirdThing
  | WeirdThing2 of WeirdThing2

type Event =
  | [<DataMember(Name = "Created")>]          Created of New
  | [<DataMember(Name = "Something")>]       Something of Something
  with interface IUnionContract

I get no error during BUILD - so syntax is good, however, during runtime, when I create an event Something I get a exception:

Union case 'xxxx' contains field that is not an F# record

So now I am wondering whether a DU nested inside a DU is possible using the TypeShape UnionEncoder? What am I missing?

There's no real blocker for a non-RC release, but given the central role the IEvent and IIndexedEvent interfaces occupy in Equinox, Propulsion and apps based thereon, it really is the last chance to do a breaking change for the foreseeable. This is the complete list of ideas - how many actually get actioned is up for debate

• Consider renaming IEvent to something that doesn't clash with / shadow FSharp.Control.IEvent
• make IndexedEventData.new align with EventData.Create
• consider hiding EventData record and/or its equality or comparison facilities
• consider having IUnionEncoder.Encode accept a Context in order that a single Codec instance can contextually enrich events with correlationId and causationId values

Need to apply same as jet/equinox#455 here

System.Text.son currently throws a clear exception if you ask it to serialize a F# DU (which IMO is the correct behavior)

NSJ instead uses its quirky encoding; I've seen many get bitten by it

Would happily take a PR that gives FsCodec.NewtonsoftJson the same failsafe behavior as STJ out of the box

The fast follow would be to port the auto* options from the STJ side, i.e. implement a UnionOrTypeSafeEnumConverterFactory to switch (see https://github.com/jet/FsCodec/blob/master/tests/FsCodec.SystemTextJson.Tests/AutoUnionTests.fs for the behavior)

If the IEventData had an encoding: byte that could be used to push the compression in CosmosStore and DynamoStore outward

e.g. 0 can be Raw, 1 can be Deflated for common UTF8 JSON cases

but probably pluggable to allow it to identify an encoding version or maybe flag json vs protobuf etc

ITimelineEvent and IEventCodec would use it to layer compression/decompression and/or custom decoding

With a default that does the conditional compression when encoding (and/or some other way to preserve the existing DX of it being defaulted on for Equinox.DynamoStore)

Equinox.EventStoreDb can map 'Encoding to octet-stream, octet-steam;deflate, and/or probably also a JSON content-encoding

For CodecJsonElement, NodeType.string vs object is used to infer whether a decompression step is required (currently Equinox.CosmosStore has a flag at category level and supports compression/decompression only for unfolds)

eqx dump can then work off it to default to dumping json

Prompted by jet/equinox#331

While the encoding support started as a very light wrapper of the TypeShape UnionEncoder (before it was called that!). However

• the base interface does not rely on or special case Unions in any way
• is equally focused on encoding and decoding

So... in V2 we should thus consider renaming IUnionEncoder to IEventCodec.
https://github.com/jet/FsCodec/blob/master/src/FsCodec/FsCodec.fs#L31

As this will be a breaking change, it should accompany some meaningful new feature, i.e. #14

Support upconversion from a representation of an event starts with TypeSafeEnumConverter for a given union (and hence has a string value), being able to implicitly upconvert from that if the internal representation adds a non-nullary case (and hence switches to using UnionConverter).

Of course, in general one should have a clear picture of whether something is an enum or a full state with data per case. The point here is to be able to represent data that's specific to a case without that having to stuff it into adjacent optional fields with conditional logic defining when the field should be present etc.

#97 is slightly related, but is going the other direction, and is recovering from a missing converter rather than a natural evolution

Refers to the test case in #64

I'm seeing a null value where I would expect an initialized/zeroed value for the property.

Possibly related to JamesNK/Newtonsoft.Json#2116

I may be misunderstanding this but if you wanted to supply your own custom FsCodec to Equinox you can FsCodec.Codec.Create(encoder, decoder). However, Equinox expects a a 'Format of byte[] ... i.e. IEventCodec<'Event,byte [], 'Context> but it seems that FsCodec.Codec.Create will only allow you to construct a IEventCodec<'Event, string, 'Context>. Notice string vs byte [].

I am sure the work around is to fully implement IEventCodec, but would be nice to have a equinox compatible short hand constructor.

Stemming from https://github.com/jet/equinox/issues/57, the concern of demonstrating and/or providing a starting point for implementing serialization using something other than the first-among-equals (but some are more equal than others) convention-based-encoding system may best be represented by including a template and/or template option.

• Name suggestion, perhaps equinoxFleece ?
• Perhaps a simple gist might suffice (in the interim?)

Backport the autoTypeSafeEnumToJsonString and autoUnionToJsonObject options to avoid the nasty surprised the default impl causes when (not if!) people fall into the default rendering trap for things lke:

a) TypeSafeEnums e.g. type ProductId = ProductA | ProductB (where you just want "ProductA", as the TypeSafeEnumconverter would do if applied explicitly)
b) Unions that should render as a JSON object (record)

 type UnionThatShouldBeAnObject =
     | SimpleProduct of master: ProductId
     | PairedProduct of {| primary: ProductId; backup: ProductId |}

will render as { "Case": "PairedProduct", "primary": "ProductA", "backup": "ProductA"} or { "Case": "PairedProduct", "master": "ProductA"}, as it would if you applied the UnionConverter explicitly

#96 is a very important related safety feature too

i.e.prevent:

"items": [
        {
          "serviceId": "5c8795be52e34e82883d61babed19513",
          "serviceKind": {
            "Case": "ProductA"
          }
        },
        {
          "serviceId": "8a55ebbf0d404485b50da95bdb53f7b3",
          "serviceKind": {
            "Case": "ProductB"
          }
        }
      ]
    },

and default to

"items": [
                            {
                                "serviceId": "5c8795be52e34e82883d61babed19513",
                                "serviceKind": "ProductB"
                            },
                            {
                                "serviceId": "8a55ebbf0d404485b50da95bdb53f7b3",
                                "serviceKind": "ProductA"
                            }
                        ]

related: JamesNK/Newtonsoft.Json#1662 (comment)

Via dotnet/runtime#55744 (comment)

for autoTypeSafeEnum I would need to split/duplicate the UnionOrTypeSafeEnumConverterFactory as well though - is this what you suggested?

I meant to supply 2 flags to the factory as ctor args which individually control whether to
a) allow automatic Type Safe Enum conversion
b) allow automatic Union conversion

In general, I'd assume that if you like convention based things, you want to start with both behaviors in place.

However, you might want to be able to disable individual behaviors.

Hi!

Would you be interested if I opened a PR to relax requireRecordFields = true?

To address your comments:

a) it's extra yaks to shave

I don't disagree :) However, I am willing to shave this yak.

b) it's questionable whether allowing one to define event contracts that preclude adding extra fields is a useful idea in the first instance

Many of my events simply contain a primitive, e.g.

type BranchId = Guid<branchId>
and [<Measure>] branchId

type DefaultBranchUpdated = { BranchId: BranchId }

type Event =
    | DefaultBranchUpdated of DefaultBranchUpdated

It would simplify my code (and make it more implementation-agnostic) if I could refactor the above to

type Event =
    | DefaultBranchUpdated of BranchId

See VerbatimUtf8EncoderTests.fs and InteropTests.fs - there are edge cases when d fields have null / zero-length / missing values

I'm not sure what a d field is. I'm currently only interested in relaxing the rules to include Guids, but I could easily see this extending to other primitives.

Round-tripping cases like null and/or empty strings etc involves edge cases that stores, FsCodec.NewtonsoftJson.Codec, Interop.fs and InteropTests.fs do not cover, so we disable this

I'm totally okay with excluding string.

I got this far with the implementation before realizing "I should probably ask first." >_>

        let shape =
            match shapeof<'Contract> with
            | Shape.FSharpUnion (:? ShapeFSharpUnion<'Contract> as s) -> s
            | _ ->
                sprintf "Type '%O' is not an F# union" typeof<'Contract>
                |> invalidArg "Union"
        let isAllowed (scase : ShapeFSharpUnionCase<_>) =
            match scase.Fields with
            | [| field |] ->
                match field.Member with
                | Shape.FSharpRecord _
                | Shape.Guid _ -> true
                | _ -> false
            | _ -> false

Provide an upconversion that lets you write the updated form per TypeSafeEnumConverter, but also accept mangled versions that predate impl of #96

i.e. read:

"items": [
        {
          "serviceId": "5c8795be52e34e82883d61babed19513",
          "serviceKind": {
            "Case": "ProductA"
          }
        },
        {
          "serviceId": "8a55ebbf0d404485b50da95bdb53f7b3",
          "serviceKind": {
            "Case": "ProductB"
          }
        }
      ]
    },

but write:

"items": [
                            {
                                "serviceId": "5c8795be52e34e82883d61babed19513",
                                "serviceKind": "ProductB"
                            },
                            {
                                "serviceId": "8a55ebbf0d404485b50da95bdb53f7b3",
                                "serviceKind": "ProductA"
                            }
                        ]

related: JamesNK/Newtonsoft.Json#1662 (comment)

As part of #38, we need to avoid uglification for usage in events and messages by using the technique in dotnet/runtime#28567 (comment)
cc @ylibrach @NickDarvey
(do they have any other such "opinion" stuff?)

Main constraint on the impl is that we don't want to foist a netstandard2.1 dependency on anyone - the codec may hence need to be a separate project/nuget - benchmarks with the memory store should make sense as a way of validating any potential raw perf improvement.

For Equinox.EventStore, the wiring should be straightforward

Wrt Equinox.Cosmos, this could become more messy due to the way in which the data is emitted (and how the DocDb client dll wants to manage its parsing)

Given:

[<Newtonsoft.Json.JsonConverter(typeof<TypeSafeEnumConverter>)>]
type Log = A | B
[<Newtonsoft.Json.JsonConverter(typeof<UnionConverter>, "rule", "Unknown")>] Input =
type DU of = 
    | CaseA of req: Log * res:Log 

the following parses, yielding a Log value of null and NullReferenceException when you touch the res value

"{ "rule": "CaseA", "req": "A" }

this should instead provide a clear parser error

Hi! The code currently requires exactly one hyphen in a stream name. Is it reasonable to loosen that restriction? This would permit representing UUIDs in the canonical RFC-4122 format.

The readme implies this convention is from EventStore. Their category projections allow for some customizations, but notably do not require exactly one hyphen. In fact, when specifying the delimiting character, one must also specify whether to consider the first or last occurrence of the delimiter. Docs.

Some stream name examples just to demonstrate what it would look like:
account-40DC8220-D240-4530-847C-9F41E286C0A2
account-FDEED87B-A89A-40AC-90B4-E89FEE29454E_1A26E0BA-1826-4504-8183-C5EAD0693DB3

Of course, conventions being conventions, all this may be ineligible for change.

Would be nice to have some examples as section in readme.md or a folder. It is hard to make first impression of what this library is from description as it stands now.

Also:

What is " versionable serialization strategies"?

"The converters are employed in diverse systems across Jet, both for [de]coding Events within Event-sourced streams, and for HTTP requests/responses. As such, format changes need to be interoperable"
It does not matter, is it events or configuration or reports. I'd reduce it to "converters are used in production code in Jet, so any changes must produce backward compatible json". That explains it all without artificial limits is scope to events processing.

"Less [converters] is more - has a converter really proved itself broadly applicable ?"
I do not understand referred link. What refactoring rule of thumb is doing here?

"this is not the final complete set of converters; Json.net is purposefully extensible and limited only by your imagination, for better or worse"
Without example, it is hard to understand what it is all about.

"Concrete Converter implementations"
Would be nice to provide direct link.

"Naturally, the library naturally"
:)

"going that extra mile here is unwarranted for now given the implementation is in F#"
Do not understand

List of breaking changes to consider for next major version:

• Shift all naming to favor STJ-based naming and/or genericity
  • Make Newtonsoft converters - e.g. TypeSafeEnumConverter generic over the DU case in question (probably less important now given autoTypeSafeEnumAsJsonString mode in #71) - skipped for now unless someone values it enough to make it a PR
  • Rename Settings to Options.
• Remove net461 support, target netstandard/net6.0 only (Equinox 4, Propulsion 3 is doing the same)

In order to support roundtripping the EventId used to identify and/or deduplicate events in EventStore, SqlStreamStore and more:

• IEventData should expose it (as a Guid)
• TimelineEvent ctor should all-but require one (default to Guid.Empty if nothing relevant available on a given store)
• EventData ctor should default it to Guid.NewGuid()