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

Update System.Text.Json sample to indicate not fully working status #4168

Closed
Arcalise08 opened this issue Nov 3, 2023 · 5 comments · Fixed by #4170
Closed

Update System.Text.Json sample to indicate not fully working status #4168

Arcalise08 opened this issue Nov 3, 2023 · 5 comments · Fixed by #4170
Assignees
Labels
customer-reported Issue created by a customer QUERY

Comments

@Arcalise08
Copy link

Arcalise08 commented Nov 3, 2023

System.Text.Json is broken in the cosmos sdk v3 for .net. Its been broken for years now at this point with only workarounds (. Making your own cosmos serializer, But then linq isnt respecting serializer attributes for system.text.json). There are many many open tickets showing the issues and every one of them acknowledges the issues and says it'll be fixed in xyz timeframe, It's now many months past all the given dates and there is no update.

Is the official response for this to use Newtonsoft, Counter to everything the ASP.NET team is doing to get rid of Newtonsoft. Or is there some reason for the work being stalled for this long?

Besides getting info, I think its fair to say the official cosmos sample should be updated to highlight the fact that it doesn't fully work. Many people, myself included, are starting the work to transfer their projects to System.Text.Json, and at first glance it does appear that cosmos sdk can do that without issue, Only to find out after days or weeks of work that the sdk linq provider isnt working correctly

@ealsur
Copy link
Member

ealsur commented Nov 6, 2023

Hi, thanks for your feedback.

Can you clarify your statement about broken? Is that referring to Query with LINQ provider or are you referring to other scenarios?

@Arcalise08
Copy link
Author

Arcalise08 commented Nov 6, 2023

Sure can.

Heres the serializer suggested in the samples for System.Text.Json support. That mostly works. But the linq provider for cosmos sdk does not appear to respect the custom serializer attributes. So if you go in and mark that a property like...

  [JsonPropertyName("id)]
  public string Id { get; set; }

It serializes the class how you expect. But attempt a linq query like

MyContainer.Where(x => x.Id == "someId");

it will be translated to

SELECT * FROM c WHERE c.Id == "someId"

Notice the uppercase I in Id instead of lowercase as it should be.

This report has already been made numerous times.

#2685
#3250

This issue includes all JSON attributes, not just JsonPropertyName but also any custom converters like an enum to string converter mentioned here
#3697

In issue #3697 we see that a timeline of may is mentioned for a fix. Its now well past that and there is no word of a workaround. There doesn't appear to be mention of the issues in the sample. Are we just hoping people don't notice or something? Why have an official sample showing that something is supported when a large portion of it does not work? All this does is inspire deep frustration and disappointment.

@ealsur
Copy link
Member

ealsur commented Nov 6, 2023

Thanks for the clarification.

So the scenario is Query with LINQ based on:

the linq provider for cosmos sdk does not appear to respect the custom serializer attributes

As you correctly mentioned, there are already reports and tracking issues for this topic and those are the best places to follow up with. Feel free to ping the assigned people on the linked Issues.

Could you clarify what is your intent in the creation of this Issue? Is it to report something new on the same topic?

@Arcalise08
Copy link
Author

I have bumped already on all issues mentioning it that I could find. I have a few intentions with this issue. But it mostly revolves around the communication around the issue. I would love to have the following questions answered definitively.

  • With this issue in mind, And seeing no clear workarounds. Is it fair to say that consumers of the sdk should NOT use system.text.json currently.
  • If the above question is yes, Is the work to get System.Text.Json working underway? Is there any timeline to release?
  • Since this issue has been known for a long time. Why havent the samples been updated to include the issue if its not promptly being fixed.

I think the only real actionable portion of my issue is updating the sample. So I will change my title to reflect what can be done now. But I don't believe I am the only person looking for the answers to the questions above. And they need to be answered if this work isnt coming down relatively soon.

@Arcalise08 Arcalise08 changed the title System.Text.Json Update System.Text.Json sample to indicate not fully working status Nov 6, 2023
@adityasa
Copy link
Contributor

adityasa commented Nov 6, 2023

@Arcalise08 Thanks for reporting this. System.Text.Json attributes are not honored by LINQ and we recently updated baseline tests to reflect existing behavior regarding LINQ serialization:
#4114

We will also update the sample to reflect this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer-reported Issue created by a customer QUERY
Projects
None yet
4 participants