Azure / azure-cosmos-dotnet-v3

.NET SDK for Azure Cosmos DB for the core SQL API
MIT License
731 stars 489 forks source link

Ship DefaultCosmosJsonSerializer in a separate package #37

Open AndriySvyryd opened 5 years ago

AndriySvyryd commented 5 years ago

Moving out all code that depends on Newtonsoft.Json would make it possible to avoid a transitive reference from a project that doesn't need it.

Microsoft.Azure.Cosmos could become a meta-package to keep the simplicity of the common use case.

j82w commented 5 years ago

@kirankumarkolli do you think it is possible to do this?

The other options is to follow asp.net core, and to use the new built in JSON parser in .NET Core 3.0. Then convert everything to use it, but it would need to be in .net standard before we could take the dependency.

kirankumarkolli commented 5 years ago

Cosmos being a JSON store, SDK/Client needs a JSON parser for intutive POCO typed API's. Currently JSON.net is the most widely used JSON parser in .NET world.

In 3.0+ SDK: JSON parsing API are abstracted by CosmosJsonSerialzier and default implementation is based on JSON.net. Customers are free to provide an override any other implementation and SDK will use it.

This way JSON.net package dependency is only a runtime dependency and when .Net standard support JSON natively can be swapped-out with no impact to API/customer.

JSON.net dependency is only packaging dependency for default implementation. So some package needs to have this dependency so it will be a transitive dependency always. We could explore a dynamic run-time pacakge building API (kind of package builder) but that complicates the every scenarios and might be ideal for SDK's.

bchong95 commented 5 years ago

Any updates on this?

fuocor commented 5 years ago

From a performance perspective the CosmosJsonSerialzier is not the best option because using a Stream forces a copy out of the serializing buffer into whatever buffer Cosmos is using. The abstraction should use an IBufferWriter<byte> for writing and ReadOnlySequence or ReadOnlyMemory for reading and then wrap the default JSON.net around it.