Azure / azure-cosmos-dotnet-v3

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

Add support for property overrides #4584

Open PanosKousidis opened 4 months ago

PanosKousidis commented 4 months ago

I have the following case:

I have a couple of classes that inherit from a base class. This is because the only difference between the classes are the property names in Cosmos. The business logic is done by using the base class object.

The following reproduces the issue

using Microsoft.Azure.Cosmos;
using Microsoft.Azure.Cosmos.Linq;
using Newtonsoft.Json;

public class CosmosSerializerTests
{
    [Fact]
    public void TestInheritance()
    {
        var cosmosClient = new CosmosClient(Settings.CosmosConnectionString);
        var container = cosmosClient.GetDatabase("mydb").GetContainer("myContainer");
        var query = container.GetItemLinqQueryable<ChildClass>()
            .Where(x => x.Name == "test" && x.Name2 == "test2");
        var sql = query.ToQueryDefinition().QueryText;
        Assert.Equal("SELECT VALUE root FROM root WHERE ((root[\"my_custom_child_name\"] = \"test\") " +
                     "AND (root[\"my_custom_child_name2\"] = \"test2\"))", sql);
        //Actual: "SELECT VALUE root FROM root WHERE ((root[\"my_custom_parent_name\"] = \"test\")
        //         AND (root[\"my_custom_child_name2\"] = \"test2\"
    }

    public class ChildClass : ParentClass
    {
        [JsonProperty("my_custom_child_name")]
        public override string? Name { get; set; }

        [JsonProperty("my_custom_child_name2")]
        public string? Name2 { get; set; }
    }

    public abstract class ParentClass
    {
        [JsonProperty("my_custom_parent_name")]
        public abstract string? Name { get; set; }
    }    
}

I believe the JsonProperty attributes should be read from the ChildClass in the scenario above. Instead, it's using the parent's JsonProperty value and translates it to my_custom_parent_name instead of my_custom_child_name. The Name2 property that is not overriding anything works as expected

The above is using Newtonsoft.Json. Switching it to use System.Text.Json.Serialization:

using System.Text.Json.Serialization;

...

public class ChildClass : ParentClass
    {
        [JsonPropertyName("my_custom_child_name")]
        public override string? Name { get; set; }

        [JsonPropertyName("my_custom_child_name2")]
        public string? Name2 { get; set; }
    }

    public abstract class ParentClass
    {
        [JsonPropertyName("my_custom_parent_name")]
        public abstract string? Name { get; set; }
    }   

This results in no translation at all - the test result shows SELECT VALUE root FROM root WHERE ((root["Name"] = "test") AND (root["Name2"] = "test2"))

I tried Microsoft.Azure.Cosmos 3.37.1 and 3.41.0

The alternative I have right now is to execute direct SQL text instead

ealsur commented 4 months ago

Can you share which SDK version you are using?

ealsur commented 4 months ago

Seems related to https://github.com/Azure/azure-cosmos-dotnet-v3/pull/4138 ?

PanosKousidis commented 4 months ago

I just updated my original ticket description with more details

PanosKousidis commented 4 months ago

Can you share which SDK version you are using?

Tried with 3.37.1 and 3.41.0 and the behaviour is the same

PanosKousidis commented 4 months ago

Seems related to #4138 ?

If I'm not mistaken this is about enabling System.Text.Json translation - which did not work in my example, I may be doing something wrong

Maya-Painter commented 4 months ago

@PanosKousidis To enable System.Text.Json translation you need to use a custom serializer. There is an example of how to do this here: https://github.com/Azure/azure-cosmos-dotnet-v3/tree/master/Microsoft.Azure.Cosmos.Samples/Usage/SystemTextJson. Please let me know if you have any questions.

PanosKousidis commented 3 months ago

Thanks @Maya-Painter , but the issue is still there.

I've drilled down and I think the behaviour I see is not Cosmos SDK-related

There is a lambda expression passed in https://github.com/Azure/azure-cosmos-dotnet-v3/blob/6e1d40d33936d2f98b1d2341254d367c5253d5f4/Microsoft.Azure.Cosmos/src/Linq/ExpressionToSQL.cs#L1515-L1521

And even though the expression itself references the ChildClass, the member referenced in "Left" is the Name property of the ParentClass instead of the ChildClass. See screenshot below

image

It is not using the declared property of the ChildClass as seen in the Expression property image

You can probably close this ticket if you also think that there is nothing that can be done on the Cosmos SDK side to alleviate this