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 provider uses Newtonsoft.Json configuration for property naming even when a custom CosmosSerializer is provided #4321

Closed
mumby0168 opened this issue Feb 12, 2024 · 6 comments
Assignees

Comments

@mumby0168
Copy link

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

Describe the bug

I have followed the sample from the samples repository to setup a System.Text.Json CosmosSerializer and configured the provided options to use CamelCase as the naming policy.

When I make a query to via the CosmosClient using the Linq provider the outputted query PascalCases the property queried.

For example if I wrote a document such as below:

{
    "firstName": "Haleigh",
    "lastName": "Yundt",
    "storeNumber": "q6",
    "joinedAtUtc": "2023-05-15T15:57:15.5556472+01:00",
    "id": "669252",
    "type": null,
    "_etag": "\"2e00d246-0000-0d00-0000-65c7abab0000\"",
    "_rid": "h8JCAIkmLzoFAAAAAAAAAA==",
    "_self": "dbs/h8JCAA==/colls/h8JCAIkmLzo=/docs/h8JCAIkmLzoFAAAAAAAAAA==/",
    "_attachments": "attachments/",
    "_ts": 1707584427
}

I then used the cosmos client to make a query as below:

IQueryable<TItem> query = container
            .GetItemLinqQueryable<TItem>()
            .Where(e => e.StoreNumber = "q6");

Then the ouputted query would be `select * from c where c.StoreNumber = 'q6`` unless I set the JsonPropertyAttribute on the class.

To Reproduce

  1. Copy the SystemTextJsonCosmosSerializer from the samples.
  2. Configure the serializer against the cosmos client and set the property naming policy to camel case
  3. Define an object to use with at least two properties
  4. Write this to a container
  5. Query this back using the linq queryable option
  6. See no results are returned

Expected behavior

It should either have a way to change this, or some configuration to pass to the client for the linq provider to support different naming.

Actual behavior

Queries the database with the incorrect casing returning no results.

Environment summary
SDK Version: <PackageVersion Include="Microsoft.Azure.Cosmos" Version="3.36.0" />
OS Version (e.g. Windows, Linux, MacOSX)

Additional context

There is a workaround which is to set the [JsonProperty("storeNumber")] on the class used, however, this feels a strange quirk when the library has been setup to use System.Text.Json.

@kirankumarkolli
Copy link
Member

There are two serializers

Type Stage Scenarios Description
CosmosSerializer GA All except LINQ Only for POCO type serialization and se-serialization
Preview: CosmosLinqSerializer Preview All including LINQ Hints to LINIQ about property name conversion

@mumby0168 Can you please test with the CosmosLinqSerializer which is currently in preview?

@Maya-Painter
Copy link
Contributor

Maya-Painter commented Feb 13, 2024

Adding a note here that if you want to apply camel casing using the CosmosLinqSerializer, SerializeMemberName() will need to be updated accordingly, e.g. something like

    class SystemTextJsonLinqSerializer : CosmosLinqSerializer
    {
        private readonly JsonObjectSerializer systemTextJsonSerializer;

        public SystemTextJsonLinqSerializer()
        {
            JsonSerializerOptions options = new()
            {
                PropertyNamingPolicy = JsonNamingPolicy.CamelCase
            };
            this.systemTextJsonSerializer = new JsonObjectSerializer(options);
        }

        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();
            this.systemTextJsonSerializer.Serialize(streamPayload, input, input.GetType(), default);
            streamPayload.Position = 0;
            return streamPayload;
        }

        public override string SerializeMemberName(MemberInfo memberInfo)
        {
            System.Text.Json.Serialization.JsonExtensionDataAttribute jsonExtensionDataAttribute =
                memberInfo.GetCustomAttribute<System.Text.Json.Serialization.JsonExtensionDataAttribute>(true);
            if (jsonExtensionDataAttribute != null)
            {
                return null;
            }

            JsonPropertyNameAttribute jsonPropertyNameAttribute = memberInfo.GetCustomAttribute<JsonPropertyNameAttribute>(true);
            if (!string.IsNullOrEmpty(jsonPropertyNameAttribute?.Name))
            {
                return jsonPropertyNameAttribute.Name;
            }

            return System.Text.Json.JsonNamingPolicy.CamelCase.ConvertName(memberInfo.Name);
        }
    }

@Maya-Painter Maya-Painter self-assigned this Feb 14, 2024
@Maya-Painter
Copy link
Contributor

@mumby0168 Just following up, did using the provided CosmosLinqSerializer address your concerns?

@JarFrank
Copy link

@Maya-Painter
It works for me, but the question is: will you release CosmosLinqSerializer class in the next version? I don't want to use preview in production.

@Maya-Painter
Copy link
Contributor

Maya-Painter commented Feb 20, 2024

@JarFrank thanks for checking. Yes, unless any valid concerns are brought up about the feature by users, we will include it as part of the next public release and update documentation accordingly.

@Maya-Painter
Copy link
Contributor

Closing this issue as addressed. This feature will also be released in the next public version - #4323

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants