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

Added JsonSerializerSettings / JsonSerializer support #28

Merged
merged 4 commits into from
May 18, 2019

Conversation

stevewgh
Copy link

Any methods that use JObject.FromObject or JObject.ToObject can now use a JsonSerializer which is constructed from a JsonSerializationSettings instance, allowing custom serialization rules to be applied transparently without the event store being aware.

The JsonSerializationSettings instance is injected from the AzureDocumentDbStorageEngineBuilder. This allows custom serialization converters (and other Json.Net extensions) to be provided to the AzureDocumentDbStorageEngine and allows control over how the events are serialized.

I tried to avoid this change by providing the JsonSerializationSettings instance to the DocumentClient, but the use of JObject.FromObject and JObject.ToObject used their own JsonSerializationSettings which ignored the converters.

An alternative to this PR is to change the type of the DocumentDbStorageEvent.Body property to object instead of JObject which would force the DocumentClient to perform the body serialization, however, this may break existing stores due to the contract change.

Some use cases for this level of event serialization

stevewgh added 2 commits June 19, 2018 21:03
…e a common JsonSerializationSettings that originates from the AzureDocumentDbStorageEngineBuilder. This allows custom serialization converters (and other Json.Net extensions) to be provided to the AzureDocumentDbStorageEngine.
…onSerializerSettings, it has a dependency on JsonSerializer - changes made to support that.
@asosMikeGore
Copy link

Hi Steve,

Thanks for this, the PR looks good! I've appended some changes to your PR to update the docs, and also change the existing when_initializing_all_expected_resources_are_created test. It looks like Cosmos is now adding additional exclude paths when creating collections to ignore the etag field, which then breaks the count assertions. Therefore, I've just tweaked the assertion to take this into account.

@asosMikeGore asosMikeGore merged commit 1f9f306 into ASOS:master May 18, 2019
@stevewgh stevewgh deleted the jsonsettings branch October 23, 2020 09:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants