Azure / azure-cosmos-dotnet-v3

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

[VNext] System.Text.Json as default serializer #2533

Open kirankumarkolli opened 3 years ago

kirankumarkolli commented 3 years ago

CosmosClient supports customizing the serializers #CosmosLinqSerializer.

NOTE: CosmosSerializer will be deprecated in future, prefer CosmosLinqSerializer

CosmosClient library includes two implementations for JSON.net and System.Text.Json (https://learn.microsoft.com/en-us/dotnet/api/system.text.json) with JSON.net being the default serializer.

Applications can change the default to System.Text.Json through CosmosClientOptions

            CosmosClientOptions options = new CosmosClientOptions()
            {
                UseSystemTextJsonSerializerWithOptions = new System.Text.Json.JsonSerializerOptions()
                {
                    PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase,
                }
            };

            CosmosClient client = new(
                ...,
                options
            );

This work item is to change the default to System.Text.json for next major version.

j82w commented 3 years ago

LINQ is still in a design phase so there are not any guarantees, but it should use the System.Text.Json

benc-uk commented 3 years ago

Just lost several hours to "The input content is invalid because the required properties – ‘id; ‘ – are missing” errors Only to discover that [JsonPropertyName("id")] was being ignored

It's not like the System.Text.Json changes are very new :(

StefanSchoof commented 3 years ago

Did you change your using to System.Text.Json https://docs.microsoft.com/en-us/dotnet/api/system.text.json.serialization.jsonpropertynameattribute?view=net-5.0 or do you used the newtonsoft namespace?

j82w commented 3 years ago

@benc-uk the .NET v3 SDK was released before System.Text.Json was GA. Switching the entire SDK over from newtonsoft to System.Text.Json is a breaking change which would require a v4 SDK. Please look at the following sample on how to make the v3 SDK use the new System.Text.Json: https://github.com/Azure/azure-cosmos-dotnet-v3/tree/master/Microsoft.Azure.Cosmos.Samples/Usage/SystemTextJson

martinnormark commented 3 years ago

@j82w Is there a way to configure the serializer used by the IQueryable implementation you get when calling GetItemLinqQueryable on a container?

I have some dynamic data stored as ExpandoObject in C#. Using System.Text.Json serialize as expected but when expressed in queries it seems to serialize using Newtonsoft.Json.

ExpandoObject serialized with System.Text.Json:

{
    "Name": "John Doe"
}

Using Newtonsoft.Json it produces:

{
    "Name": { "ValueKind": 3 }
}

It seems to be serializing directly using JsonConvert and not base it on configuration of any sort (given that I have found the right spot): https://github.com/Azure/azure-cosmos-dotnet-v3/blob/56aa5338d44ffd19ce61b264b24dd16d46df6477/Microsoft.Azure.Cosmos/src/Linq/CosmosLinqQuery.cs#L139

This problem is outlined in way more detail here: https://josef.codes/custom-dictionary-string-object-jsonconverter-for-system-text-json/

curia-damiano commented 2 years ago

+1

Robar666 commented 2 years ago

+1

Romfos commented 2 years ago

+1

YipingRuan commented 2 years ago

Just lost several hours to "The input content is invalid because the required properties – ‘id; ‘ – are missing” errors Only to discover that [JsonPropertyName("id")] was being ignored

It's not like the System.Text.Json changes are very new :(

Maybe https://github.com/Azure/azure-cosmos-dotnet-v3/pull/3282

AraHaan commented 2 years ago

Oh btw any plans to update the Newtonsoft dependency to the latest version to avoid possibly getting a version of that in the v3 that is prone to security issues? I think v10 is prone but v13 is not (that we know of so far).

After that the EFCore cosmos provider in dotnet/efcore would need to be patched to use a version of Microsoft.Azure.Cosmos that upgrades the newtonsoft to v13 as well.

And please look into it before patch Tuesday where they will ship the 6.0.7 servicing release of efcore and the .NET shared frameworks that come with the .NET SDK.

hkusulja commented 2 years ago

Hello, any progress on this? We want to avoid Newtonsoft JSON and move to System.Text.Json ...

danielmarbach commented 2 years ago

@hkusulja while the dependency is not yet removed, have you considered adding a customer serializer like shown in https://github.com/Azure/azure-cosmos-dotnet-v3/blob/master/Microsoft.Azure.Cosmos.Samples/Usage/SystemTextJson/CosmosSystemTextJsonSerializer.cs?

YipingRuan commented 2 years ago

@hkusulja while the dependency is not yet removed, have you considered adding a customer serializer like shown in https://github.com/Azure/azure-cosmos-dotnet-v3/blob/master/Microsoft.Azure.Cosmos.Samples/Usage/SystemTextJson/CosmosSystemTextJsonSerializer.cs?

Does this solve the Linq property name casing issue?

onionhammer commented 2 years ago

It also doesn't solve the issue of Enums as strings in linq.

hkusulja commented 1 year ago

Please update status and information about statement:

Official support for System.Text.Json is also planned for v4 of the client.

0xakihiko commented 1 year ago

Getting this through would be extremely appreciated. Using cosmosdb means introducing another third party dependency of newtonsoft for serialization, especially with the security risks it can potentially impose.

Robar666 commented 8 months ago

Any updates on this issue?

bartelink commented 8 months ago

Preview has the bits for LINQ to work with it - next release will include it too (see https://github.com/Azure/azure-cosmos-dotnet-v3/pull/4323)

I have a proposal for how this can be resolved in https://github.com/Azure/azure-cosmos-dotnet-v3/issues/4325

@kirankumarkolli you could close this in favor of it and we'd not need a vnext :D

bartelink commented 1 month ago

@kirankumarkolli @kundadebdatta Can you add a status summary covering the current state of play (what is the shortest one liner OOTB right now) and close this please?

kirankumarkolli commented 1 month ago

@bartelink updated the description, this issue is to track the default change for vNext so will still be open.

AndriySvyryd commented 1 month ago

And the main package won't have a dependency on JSON.net in vNext, right? https://github.com/Azure/azure-cosmos-dotnet-v3/issues/37

kirankumarkolli commented 1 month ago

Yes that's the goal and this issues is to track it.