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

LINQ-constructed document queries ignore Json annotations based on type constraint #484

Closed
fabianmeyer opened this issue Jun 25, 2019 · 13 comments · Fixed by #4138
Closed
Assignees
Labels
bug Something isn't working customer-reported Issue created by a customer LINQ needs-investigation QUERY

Comments

@fabianmeyer
Copy link

Describe the bug
When using LINQ to create queries in a generic function with a constraint on the document's type parameter, the query generator ignores annotations on the concrete type. Specifically, the generator ignores Json serialization annotations on the properties, leading to incorrect queries.

To Reproduce

using System;
using System.Linq;
using System.Net;
using System.Threading.Tasks;
using Microsoft.Azure.Cosmos;
using Microsoft.Azure.Cosmos.Fluent;
using Newtonsoft.Json;

namespace ConsoleApp
{
    interface ITest
    {
        string Id { get; set; }
        
        string Name { get; set; }
    }

    class Test : ITest
    {
        [JsonProperty("id")] 
        public string Id { get; set; }
        
        [JsonProperty("myweirdnameproperty")] 
        public string Name { get; set; }
    }
    
    class Program
    {
        static async Task Main(string[] args)
        {
            
            var client = new CosmosClientBuilder("AccountEndpoint=https://localhost:8081/;AccountKey=C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw==").Build();
            
            
            var databaseResp = await client.GetDatabase("testdb").ReadAsync();
            var database = databaseResp.StatusCode == HttpStatusCode.OK
                ? databaseResp.Database
                : (await client.CreateDatabaseAsync("testdb")).Database;
            
            var containerResp = await database.GetContainer("testcontainer").ReadContainerAsync();
            var container = containerResp.StatusCode == HttpStatusCode.OK
                ? containerResp.Container
                : (await database.CreateContainerAsync(new ContainerProperties("testcontainer", "/id"))).Container;
            
            
            InterfaceConstraint<Test>(container);
            ClassConstraint<Test>(container);
        }

        private static void InterfaceConstraint<T>(Container container)
        where T :  ITest
        {
            Console.WriteLine($"InterfaceConstraint Query for T = \"{typeof(T)}\": {container.GetItemLinqQueryable<T>().Select(doc => doc.Name).ToSqlQueryText()}");
        }
        
        private static void ClassConstraint<T>(Container container)
            where T :  Test
        {
            Console.WriteLine($"ClassConstraint Query for T = \"{typeof(T)}\": {container.GetItemLinqQueryable<T>().Select(doc => doc.Name).ToSqlQueryText()}");
        }
    }    
}

Expected behavior

Query generator should consider annotation on Test class. Expected output:

InterfaceConstraint Query for T = "ConsoleApp.Test": SELECT VALUE root["myweirdnameproperty"] FROM root
ClassConstraint Query for T = "ConsoleApp.Test": SELECT VALUE root["myweirdnameproperty"] FROM root

Actual behavior

Query generator ignores annotation on Test class. Actual output:

InterfaceConstraint Query for T = "ConsoleApp.Test": SELECT VALUE root["Name"] FROM root
ClassConstraint Query for T = "ConsoleApp.Test": SELECT VALUE root["myweirdnameproperty"] FROM root

Environment summary
SDK Version: Microsoft.Azure.WebJobs.Extensions.CosmosDB 3.0.3, .NET Core 2.2
OS Version: Windows 10,

@j82w
Copy link
Contributor

j82w commented Jun 25, 2019

This is an interesting problem. In v3 we allow a custom serializer. A lot of people have shown interest in using .NET core 3.0 system.text.json serializer because of the performance increase it has over Json.NET. Only option I see is if we added some support for a custom serializer for the property names.

@simplynaveen20 If we just assume the user is always using the default Json.NET implementation could we add support for Json.NET attribute tags?

@j82w j82w added discussion-wanted Need a discussion on an area feature-request New feature or request labels Jun 25, 2019
@fabianmeyer
Copy link
Author

What seems weird to me is that the query builder behaves differently depending on the constraint on T. When the concrete type is used as constraint, the annotations are considered correctly. When the interface is used, the annotations are not considered. Even though T is the same concrete type in both cases and the annotations are present on T in both functions per reflection.

@j82w j82w added bug Something isn't working needs-investigation and removed discussion-wanted Need a discussion on an area feature-request New feature or request labels Jun 25, 2019
@j82w
Copy link
Contributor

j82w commented Jun 25, 2019

The reason this is happening is the constraint on T causes everything to be down cast to ITest as the type. Based on the code right now we don't check for derived types. You can see the code here.

@simplynaveen20 simplynaveen20 self-assigned this Jun 25, 2019
@simplynaveen20
Copy link
Member

Will have a look to this POST GA

@lommez
Copy link

lommez commented Jan 23, 2020

Any updates on this issue?
I need so much to use the JsonIgnore attribute

@holietenk
Copy link

holietenk commented Jun 15, 2020

Any updates?

The following changes in ExpressionToSQL.cs work for me:

PropertyInfo targetProperty = inputExpression.Expression.Type.GetProperty(inputExpression.Member.Name);

string memberName = targetProperty.GetMemberName(context?.serializationOptions);

Of course, this is just for demo purpose only ... :)

@j82w
Copy link
Contributor

j82w commented Jun 15, 2020

@bchong95 can you take a look?

@holietenk
Copy link

any updates? @bchong95 @j82w
do you have plans to fix this?

@thomaslevesque
Copy link
Contributor

thomaslevesque commented Oct 23, 2020

Workaround, until this is fixed: apply the JsonProperty attributes on the interface properties. It's ugly, but it works.
(in addition to the class, otherwise other things will break...)

@CharlieDigital
Copy link

@thomaslevesque Upvote for the simple solution; banging my head against this last night for a few hours!

@AndrewTriesToCode
Copy link

Is this something that will be addressed in V3 or should we plan to work around?

@jcocchi jcocchi assigned leminh98 and unassigned simplynaveen20 Aug 29, 2023
@jcocchi jcocchi added the customer-reported Issue created by a customer label Aug 29, 2023
@jcocchi
Copy link
Contributor

jcocchi commented Aug 29, 2023

@leminh98 can you take a look at this?

@Maya-Painter
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working customer-reported Issue created by a customer LINQ needs-investigation QUERY
Projects
Development

Successfully merging a pull request may close this issue.