Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

SystemTextJsonSerializer with ReferenceHandler.Preserve in options #569

Open
dallas1287 opened this issue May 28, 2023 · 1 comment
Open

Comments

@dallas1287
Copy link

Description

trying to use SystemTextJsonSerializer(new JsonSerializerOptions() { ReferenceHandler = ReferenceHandler.Preserve })
to ignore cycles in many to many relationships doesn't work because it distorts the json and the server doesn't parse the variables correctly. The first problem is that it loses case insensitivity so it expects all my variables to match the case of my c# variables. The second problem is that when ReferenceHandler.Preserve is set it add id variables to the json and that gets sent over to the graphql server and it doesn't know how to parse it. So I guess my question is does this setting in JsonSerializerOptions just not work?

Steps to reproduce

Create a small schema that uses standard c# uppercase variables then write a query or a mutation that only uses lower case variables. If you run the queries with the default SystemTextJsonSerializer constructor it works fine and the case insensitivity is applied. If you add the ReferenceHandler.Preserve option it no longer parses your query appropriately and the back end will report an error of not being able to find any of your non-nullable variables because they dont match case.

Expected result

It works as before and ignores any detected loops

Actual result

applying the ReferenceHandler = ReferenceHandler.Preserve leads to loss of case insensitivity and json that can't be parsed by the server

Environment

@Shane32 Shane32 transferred this issue from graphql-dotnet/graphql-dotnet May 28, 2023
@Shane32
Copy link
Member

Shane32 commented May 28, 2023

As SystemTextJsonSerializer is part of the GraphQL.NET client library, I'm transferring this issue to the applicable repo.

This behavior is the function of System.Text.Json's design and not part of GraphQL.NET. Here is the applicable MS docs:

As for the client library, it is unlikely that the option would be compatible with typical GraphQL servers because, as you already noticed, it generates additional metadata JSON properties expecting that the recipient understands these properties when deserializing.

However, let's say you were talking to a GraphQL.NET server implementation, and configured its System.Text.Json serializer in the same fashion. And let's say you had an input type configured to support recursion. (See the spec here which allows this.) And then you sent a request containing a cycle in the object tree. As GraphQL.NET server is not programmed for this scenario, it is likely you would crash the server with a stack overflow exception while attempting to parse the request tree. (I did not test this, but it is very likely.)

There is only one situation where it could reliably be used with a properly-configured GraphQL.NET server implementation, and that is if the node containing the cycle were a scalar node of type AnyGraphType (or similar) allowing a deserialized JSON object to be passed directly through to the field resolver. Your field resolver would then be responsible for handling a data structure containing cycles.

loss of case insensitivity

You would have to refer to the System.Text.Json documentation to see if this is expected behavior and make changes appropriately. @rose-a please correct me if I'm wrong: ReferenceHandler.Preserve is not a feature "supported" by the GraphQL.NET team.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants