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

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

Open
kirankumarkolli opened this issue Jun 11, 2021 · 22 comments
Open

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

kirankumarkolli opened this issue Jun 11, 2021 · 22 comments
Labels
feature-request New feature or request VNext Next Major version of SDK

Comments

@kirankumarkolli
Copy link
Member

kirankumarkolli commented Jun 11, 2021

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.

@kirankumarkolli kirankumarkolli added the VNext Next Major version of SDK label Jun 11, 2021
@j82w j82w added the feature-request New feature or request label Jun 14, 2021
@j82w
Copy link
Contributor

j82w commented Jul 19, 2021

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

@benc-uk
Copy link

benc-uk commented Aug 6, 2021

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
Copy link

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
Copy link
Contributor

j82w commented Aug 6, 2021

@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
Copy link

martinnormark commented Oct 6, 2021

@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):

return JsonConvert.SerializeObject(querySpec);

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

@curia-damiano
Copy link

+1

2 similar comments
@Robar666
Copy link

+1

@Romfos
Copy link

Romfos commented Dec 14, 2021

+1

@YipingRuan
Copy link

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
#3282

@AraHaan
Copy link

AraHaan commented Jul 5, 2022

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
Copy link

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

@danielmarbach
Copy link

@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
Copy link

@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
Copy link
Contributor

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

@hkusulja
Copy link

Please update status and information about statement:

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

@0xakihiko
Copy link

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
Copy link

Any updates on this issue?

@bartelink
Copy link
Contributor

bartelink commented Feb 22, 2024

Preview has the bits for LINQ to work with it - next release will include it too (see #4323)

I have a proposal for how this can be resolved in #4325

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

@bartelink
Copy link
Contributor

bartelink commented Sep 7, 2024

@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
Copy link
Member Author

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

@AndriySvyryd
Copy link

And the main package won't have a dependency on JSON.net in vNext, right?
#37

@kirankumarkolli
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request New feature or request VNext Next Major version of SDK
Projects
None yet
Development

No branches or pull requests