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

Make Serdes stateful #70

Merged
merged 7 commits into from
Jan 5, 2022
Merged

Make Serdes stateful #70

merged 7 commits into from
Jan 5, 2022

Conversation

bartelink
Copy link
Collaborator

Enables cleaner integration of #69

@bartelink bartelink marked this pull request as ready for review January 5, 2022 14:41
@bartelink bartelink merged commit bed22c3 into master Jan 5, 2022
@bartelink bartelink deleted the stateful-serdes branch January 5, 2022 15:38
@deviousasti
Copy link
Member

I completely missed this PR.
Why not keep static methods in Serdes which just uses the default settings?
This is a pretty major deprecation.

@bartelink
Copy link
Collaborator Author

bartelink commented Jan 30, 2022

Sorry, I didn't spend a long time flagging it (I mentioned it on the DDD-CSQRS-ES #equinox slack); it was to my mind a pretty straightforward logical implication of the autoUnion mode facility (esp the TypeSafeEnumOrUnionConverterFactory) being introduced as part of the topping and tailing of UnionConverter in v 2.3.0

I'm sure you're aware, but for avoidance of doubt, there are Obsolete ones; this is not binary breaking (though in V3 I intend to remove them outright).

With regard to having a global default, looking at the completely default JsonSerializerOptions vs the ones that Options.Create have, there are some arguably debatable choices:

  • no PascalCase to camelCase mapping (yes that's default for STJ, but its not for newtonsoft)
  • no UnsafeRelaxedJsonEscaping

While I'm happy with the policies in it, the introduction of policies that are hard to have a singular view on is what forced the issue, in particular the use of the TypeSafeEnumOrUnionConverterFactory. (and that was even before #71 entered the picture, although I did have such tradeoffs very much in mind when doing this)

IME its bad news to have a global strategy that random parts of your app can invoke without telling anyone, even if it feels great to have an obvious global policy that nobody needs to debate or second guess. (This is ironically based on hard won experience in the cart system that ironically Settings.Default!)

The positives of this design are:

  • you have a single type (consider it an interface) that can fulfil symmetric ser/des roundtrip
  • you can define a given part of an app by requiring a JsonSerializerOptions input

In particular this means one can define a set of JSO for a given scope (batch of controllers, part of a domain layer, kafka topic contract) and tie any ser/des activity to that.

Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants