Azure / azure-cosmos-dotnet-v3

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

LINQ queries doesn't use custom CosmosSerializer #2685

Closed zdrlik closed 8 months ago

zdrlik commented 3 years ago

We are continuously addressing and improving the SDK, if possible, make sure the problem persist in the latest SDK version.

Describe the bug I need to use custom serializer (implementing serialization with System.Text.Json) for CosmosClient I decorate properties of my classes with JsonPropertyName attribute to translate class property names to attribute names of documents in my database. But SQL queries created from LINQ queries contains original property names, not names from JsonPropertyName.

To Reproduce

  1. Create documents in database with column names e.g. id, project_id
  2. Create C# project with class:
public record MyItem
{
    [JsonPropertyName("id")]
    public string Id { get; set; }

    [JsonPropertyName("project_id")]
    public string ProjectId { get; set; }`
}
  1. Write custom serializer using System.Text.Json:
public class CosmosSystemTextJsonSerializer : CosmosSerializer
{
    private readonly JsonObjectSerializer _systemTextJsonSerializer;

    public CosmosSystemTextJsonSerializer(JsonSerializerOptions jsonSerializerOptions)
    {
        _systemTextJsonSerializer = new JsonObjectSerializer(jsonSerializerOptions);
    }

    public override T FromStream<T>(Stream stream)
    {
        if (stream == null)
            throw new ArgumentNullException(nameof(stream));

        using (stream)
        {
            if (stream.CanSeek && stream.Length == 0)
            {
                return default;
            }

            if (typeof(Stream).IsAssignableFrom(typeof(T)))
            {
                return (T)(object)stream;
            }

            return (T)this._systemTextJsonSerializer.Deserialize(stream, typeof(T), default);
        }
    }

    public override Stream ToStream<T>(T input)
    {
        MemoryStream streamPayload = new MemoryStream();
        _systemTextJsonSerializer.Serialize(streamPayload, input, typeof(T), default);
        streamPayload.Position = 0;
        return streamPayload;
    }
}
  1. Create CosmosClient with custom serializer:
var cosmosSystemTextJsonSerializer = new CosmosSystemTextJsonSerializer(new JsonSerializerOptions());
var cosmosClientOptions = new CosmosClientOptions()
{
    Serializer = cosmosSystemTextJsonSerializer
};
var cosmosClient = new CosmosClient(connectionString, cosmosClientOptions);
  1. Query with LINQ
container = cosmosClient.GetDatabase(databaseName).GetContainer(containerName));
var projectId = _projectId.ToString();
var query = container.GetItemLinqQueryable<MyItem>().Where(i => i.ProjectId == projectId);
var feedIterator = query.ToFeedIterator();
  1. LINQ query is translated to SQL query

SELECT VALUE root FROM root WHERE (root["ProjectId"] = "XXX")

Expected behavior LINQ query is translated to SQL query using atribute names from JsonPropertyName attributes:

SELECT VALUE root FROM root WHERE (root["project_id"] = "XXX")

Actual behavior LINQ query is translated to SQL query with original property names, not names from JsonPropertyName attributes:

SELECT VALUE root FROM root WHERE (root["ProjectId"] = "XXX")

Environment summary SDK Version: 3.20.1 OS Version: Windows 10

Additional context

evgenxpp commented 3 years ago

TypeSystem doesn't take into account JsonPropertyNameAttribute. As a straightforward workaround you can mark your properties by DataMemberAttribute or Newtonsoft.Json.JsonPropertyAttribure

https://github.com/Azure/azure-cosmos-dotnet-v3/blob/090af9c26a04d2d208e5443554c6c01d81482dd6/Microsoft.Azure.Cosmos/src/Linq/TypeSystem.cs#L25

thomasvdb commented 2 years ago

We have the same problem and used the following workaround. According to the documentation of the GetItemLinqQueryable you can add specific serializer options here as well.

So we did this:

var query = container.GetItemLinqQueryable<MyItem>(linqSerializerOptions: new CosmosLinqSerializerOptions { PropertyNamingPolicy = CosmosPropertyNamingPolicy.CamelCase }).Where(i => i.ProjectId == projectId));

This fixes the issue for now.

wbuck commented 1 year ago

This is also problematic when you're serializing enum's as string's. You can of course decorate every single enum with the [JsonConverter(typeof(StringEnumConverter))] attribute, but that's not the greatest solution to this issue.

sven5 commented 1 year ago

Hi guys,

this issue is open for almost 2 years now. Do you have any actual plans when to fix it? @sourabh1007

sourabh1007 commented 1 year ago

@leminh98

leminh98 commented 1 year ago

The issue is planned to be addressed in the next few months.

ilmax commented 12 months ago

Hey @leminh98, any progress here?

Arcalise08 commented 10 months ago

They say they want to address it. Yet a PR has been open for six months which adds the support

https://github.com/Azure/azure-cosmos-dotnet-v3/issues/3862

ilmax commented 10 months ago

I am amazed by the fact that they ship a half working solution and then we're left hanging, waiting for such basic functionality for ages... It's clear that no one inside MS is using this SKD, otherwise it would be fixed immediately.

sven5 commented 10 months ago

The really annoying fact here is, that other packages already using System.Text and now we have incompatibilities. I'm currently using Azure Search SDK and this uses System.Text.Json - now, the deserialization of entities is not compatible. 😠

Come on guys, this case is open for over 2 years now.

jbt00000 commented 9 months ago

If you don't feel like doing the work of fixing the 110 line file of DefaultCosmosLinqSerializer.cs (an act that would have saved the community tens of thousands of hours of attempting to use System.Text.Json, failing on this one library, then reverting their entire code bases), at least make ICosmosLinqSerializer public and give us a delegate or factory to use a user supplied class.

sven5 commented 9 months ago

@jbt00000 I couldn't agree more.

jbt00000 commented 8 months ago

@Maya-Painter - YEAH!!!

Can't wait for the next release of the SDK!

Maya-Painter commented 8 months ago

Hi all, we've merged what we hope will be a solution to this issue by allowing the custom serializer to be used for LINQ translations. This also unblocks having System.Text.Json attributes applied in LINQ queries.

We plan to release this as part of the next preview package, but I also wanted to take this time to confirm these changes meet your needs before finalizing them as part of our public API. If you try the changes out, please follow up in the thread if this update addresses your concerns. If it doesn't, please let us know why.

To use your own custom serializer for LINQ translations you must implement CosmosLinqSerializer and set it as a custom serializer on the CosmosClient. There is a sample of this for serialization with System.Text.Json in CosmosLinqSerializer.cs.

jbt00000 commented 8 months ago

What is the pre-release nuget package version we can target?

machie27 commented 8 months ago

@Maya-Painter, that is good news!

When is this preview package available on NuGet?

If we register a custom serializer in CosmosClientOptions will this also be implemented by the CosmosLinqSerializer?

For example if we do it like this:

Custom serializator class:

    public sealed class CosmosSystemTextJsonSerializer :  CosmosLinqSerializer
    {
        private readonly JsonObjectSerializer _systemTextJsonSerializer;

        public CosmosSystemTextJsonSerializer(JsonSerializerOptions jsonSerializerOptions)
        {
            _systemTextJsonSerializer = new JsonObjectSerializer(jsonSerializerOptions);
        }

        public override T FromStream<T>(Stream stream)
        {
            if (stream == null)
            {
                throw new ArgumentNullException(nameof(stream));
            }

            using (stream)
            {
                if (stream.CanSeek && stream.Length == 0)
                {
                    return default;
                }

                if (typeof(Stream).IsAssignableFrom(typeof(T)))
                {
                    return (T)(object)stream;
                }

                return (T)_systemTextJsonSerializer.Deserialize(stream, typeof(T), default);
            }
        }

        public override Stream ToStream<T>(T input)
        {
            var streamPayload = new MemoryStream();
            _systemTextJsonSerializer.Serialize(streamPayload, input, typeof(T), default);
            streamPayload.Position = 0;
            return streamPayload;
        }
    }

and then register it like so for example:

CosmosClientOptions cosmosClientOptions = new()
{
    ...
    Serializer = new CosmosSystemTextJsonSerializer(new JsonSerializerOptions()
    {
        PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
        DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
        Converters =
        {
            new JsonStringEnumConverter()
        }
    })
};
Maya-Painter commented 8 months ago

@jbt00000 The preview package is not released yet. We do not have a release scheduled yet but are hoping to get it out by the end of the month.

@machie27 CosmosLinqSerializer is an abstract class that inherits from CosmosSerializer so the first line of your implementation should instead look like this

public sealed class CosmosSystemTextJsonSerializer : CosmosLinqSerializer