graphql-java-generator / graphql-maven-plugin-project

graphql-maven-plugin is a Maven Plugin for GraphQL, based on graphql-java. It accelerates the development for both the client and the server, by generating the Java code. It allows a quicker development when in contract-first approach, by avoiding to code the boilerplate code.
https://graphql-maven-plugin-project.graphql-java-generator.com
MIT License
115 stars 47 forks source link

Unable to parse variables in GraphQL #205

Closed JamesMustafa closed 6 months ago

JamesMustafa commented 7 months ago

Hello,

I have the following graphQL schema:

type Mutation {
    createRegistration(registration: RegistrationInput!): Void
}

input RegistrationInput {
    serviceId: String!
    versionId: String!
    dependencies: [String]
    tasks: [TaskInput!]
}

input TaskInput {
    name: String!
    retryCount: Int
    taskSpec: JSON!
}

where JSON is scalar.

In my pom.xml file I have a custom scalar inside the configuration of graphql-maven-plugin plugin which looks like this:

<customScalar>
    <graphQLTypeName>JSON</graphQLTypeName>
    <javaType>com.fasterxml.jackson.databind.node.ObjectNode</javaType>
     <graphQLScalarTypeStaticField>graphql.scalars.ExtendedScalars.Json</graphQLScalarTypeStaticField>
</customScalar>

When I try to create the mutation on my client application, the server responds with: [notprivacysafe.graphql.GraphQL] - Query did not parse. The reason is because the properties inside the "taskSpec" which are mapped to ObjectNode are still in JSON format: taskSpec: {"type": "..." } instead of GraphQL query format: taskSpec: { type: "..." }.

On the client side I get the following error:

com.graphql_java_generator.exception.GraphQLRequestExecutionException: 1 error(s) occurred: Invalid syntax with offending token '"type"'

I tried executing the mutation in different ways, including creating my own GraphQLRequest<schema> object for the specified mutation but all of them failed.

etienne-sf commented 7 months ago

Hello,

You've detailed the schema and pom configuration.

Can you show what is the code you're using to actually execute the request ? Especially the GraphQL request itself ?

BTW, what you're trying to do seems complex to me. I guess that on the client side, there is a mismatch while the plugin tries to read the taskSpec content: as the server response is JSON, I guess it can't guess where the custom scaler contents starts.

Can you try by setting this custom scalar as a regular String (or don't use a custom scalar at all) ?

Etienne

JamesMustafa commented 7 months ago

Hi @etienne-sf ,

Indeed the scalar was first set to be a regular String. However, there was a mismatch between the TaskInput class on the server side and the one which is generated on the client side.

The server side TaskInput looks like this:

@Data
@NoArgsConstructor
@AllArgsConstructor
public class TaskInput {

    @NotBlank(message = "cannot be empty or null")
    private String name;

    @Max(value = 10, message = "cannot be more than 10")
    private int retryCount;

    @NotNull(message = "cannot be null")
    private ObjectNode taskSpec;

}

As the scalar was set to be a String, I had private String taskSpec on the client side instead of ObjectNode and then got the following error:

Cannot deserialize value of type `java.lang.String` from Object value (token `JsonToken.START_OBJECT`)
 at [Source: (org.springframework.util.StreamUtils$NonClosingInputStream); line: 13, column: 19] (through reference chain: com.client.graphql.generated.RegistrationInput["tasks"]->java.util.ArrayList[0]->com.client.graphql.generated.TaskInput["taskSpec"])

My client fetches the RegistrationInput object in JSON format from an external service so the most preferred use case for me is the following:

        String validRegistrationJson = """
                {
                    "serviceId": "test-service-id",
                    "versionId": "test-version-id",
                    "tasks": [
                        {
                            "name": "test-task-name",
                            "taskSpec": {
                                "type": "TEST_EXECUTION",
                                ...
                            }
                        }
                    ]
                }""";

        var registration = new ObjectMapper().readValue(validRegistrationJson, RegistrationInput.class);

        var mutationExecutor = graphQLExecutorFactory.getNewMutationExecutor(); //returns the generated MutationExecutor
        mutationExecutor.createRegistration("", registration); //mutation doesn't have a response

I have also tried building the request manually to dive deeper but maybe did some mistakes here:

var request = new GraphQLRequest(HttpGraphQlClient.builder(webClient).build(), "", RequestType.mutation, "createRegistration"
                , InputParameter.newBindParameter("<schema>", "registration","reg", InputParameter.InputParameterType.MANDATORY, "String", true, 0, false)
        );

request.execMutation("reg", validRegistrationJson);

I would appreciate if you can suggest a solution which could work well for the first request example.

etienne-sf commented 7 months ago

Hello,

IMHO, the ´validRegistrationJson´ on your sample is not a valid GraphQL message. In this message, taskSpec is a json object. So taskSpec must be a GraphQL object in your GraphQL schema.

But I guess that if tou do that, it's because you can't statically define which object it is? BTW, you may define the taskSpec field's type as an interface. Then define in your GraphQL schema every possible type for the taskSpec field. You'll just have to be sure that these types implement this interface.

The other way to do this is to use a String in your GraphQL message. To do this, ou can define it as a custom scalar, like you tried.

In either case, it must be done on both sides (server and client).

Étienne

JamesMustafa commented 7 months ago

Hello,

taskSpec should not be instantiated as an object in my case. I want to store it as a JSON object in my database and don't have any additional knowledge about it in my server microservice. I pass it in specific cases to other services, but I don't want to know if it has a type, what type it is, etc. or validate it in any regard except that it's a valid JSON object. That's the reason why I cannot define it as an interface with different type implementations.

I cannot understand what's the benefit of having scalars as it would work with String only? I am a bit concerned of changing the existing structure on the server side because there are stakeholders who already use it and don't want to introduce any backwards incompatible changes. The generated TaskInput on the client side is not used yet. Also isn't there a way to pass the whole JSON payload as a variable and wouldn't it make sense to have such feature? I see it's supported in tools as Insomnia

Screenshot 2023-12-11 at 8 09 39
etienne-sf commented 7 months ago

Hello,

I re-read all the thread. You're right, it should work.

The error is not due to what you said in your first message (because the properties inside the "taskSpec" which are mapped to ObjectNode are still in JSON format): the query and response are serialized in json.

But as you're using the "ExtendedScalars.Json", it means that it should work (this is the part I missed in my previous messages).

I'll do a test on this use case tomorrow.

Étienne

etienne-sf commented 6 months ago

Hello,

I repeated the error you got, and confirm your analysis. And mine :)

My opinion is that the JSON extended scalar doesn't work properly. It should encapsulate the json string into a valid GraphQL string, so that the GraphQL data can be properly transported.

There is still a question I have in your use case: on which side do you use the GraphQL maven plugin? For both the client and the server?

Étienne

etienne-sf commented 6 months ago

Hello,

The issue is already logged in the graphql-java-extended-scalars project: https://github.com/graphql-java/graphql-java-extended-scalars/issues/109

My opinion is that this is due to a bug in their implementation. As stated above, my opinion is that the proper implementation is to encapsulate the json into a GraphQL String for transport.

I'll propose this in the graphql-java-extended-scalars's issue

Etienne

etienne-sf commented 6 months ago

Hum hum

I've continued looking for this issue. It seems that the graphql-java implementation is the same as the one in the one referenced in the Apollo server. And the plugin seems to properly use it.

So either this analysis is right and the issue is in the spring graphql package, or I missed something (which is possible, as the generated message seems erroneous to me).

Again : are you using the graphql plugin on both sides (client and server) ? If yes, I can easily adapt the json custom scalar implementation to something that will work.

JamesMustafa commented 6 months ago

Hi, the graphql plugin is used on client side only

JamesMustafa commented 6 months ago

Hi @etienne-sf are there any updates on this topic? Can you please suggest some workarounds until the issue is fixed as it's currently a blocker for us?

etienne-sf commented 6 months ago

I created some test projects, and this issue seems to be within the plugin, with different issues, depending on the way the custom scalar is used:

All this makes this issue a tricky one. In #207, razilevin solved it by removing all the code in the AbstractCustomJacksonDeserializer, which of course is just for test. The existing code is needed, and the complexity is to embed a proper management of the JSON custom scalar in it.

In other words: I'm working on it, but it's a tricky one, as it impacts a complex part of the plugin.

Etienne

etienne-sf commented 6 months ago

Hello,

To make all the possible use cases, I still need work.

But if you're only sending the JSON to the server (not receiving it in a server's response), then the use of GraphQL variables should work.

For instance, for this schema:

  scalar JSON

  type Query {
    text(input: JSON): String
  }

This query should work:

query testJson($json: JSON) {text(input: $json) {} }

By calling it this way:

        String response = this.query.exec(
                "query testJson($json: JSON) {text(input: $json) {} }", 
                "json", json).getText();

Hope this helps Etienne

JamesMustafa commented 6 months ago

Hi @etienne-sf, will try it out and reach you back! Thanks for the suggestions and happy holidays! 😊

etienne-sf commented 6 months ago

Hello,

The 2.4 version has been released. It handles the JSON and Object custom scalars.

Etienne