Custom scalar not respected for array types

[gjvoosten]

I have a custom scalar configured like this:

generateClientCodeConf {
  //…
  customScalars = [ 
    [ 
      graphQLTypeName: "Base64String",
      javaType: "java.lang.Byte[]",
      graphQLScalarTypeStaticField: "org.example.utils.Scalars.GraphQLBase64String"
    ]
  ]
}

However, when mutating an object with a Byte[] field, I get an exception:

graphql.schema.CoercingSerializeException: Expected type 'String' or 'Byte[]' but was 'Byte'
    at app//org.example.utils.Scalars$1.serialize(Scalars.java:31)
    at app//org.example.utils.Scalars$1.serialize(Scalars.java:23)
    at app//com.graphql_java_generator.client.request.InputParameter.getStringContentForGraphqlQuery(InputParameter.java:780)
    at app//com.graphql_java_generator.client.request.InputParameter.getStringContentForAListValue(InputParameter.java:839)
    at app//com.graphql_java_generator.client.request.InputParameter.getStringContentForGraphqlQuery(InputParameter.java:771)
    at app//com.graphql_java_generator.client.request.InputParameter.getStringContentForAnInputTypeValue(InputParameter.java:880)
    at app//com.graphql_java_generator.client.request.InputParameter.getStringContentForGraphqlQuery(InputParameter.java:791)
    at app//com.graphql_java_generator.client.request.InputParameter.getStringContentForGraphqlQuery(InputParameter.java:733)
    at app//com.graphql_java_generator.client.request.InputParameter.appendInputParametersToGraphQLRequests(InputParameter.java:936)
    at app//com.graphql_java_generator.client.request.QueryField.appendToGraphQLRequests(QueryField.java:346)
    at app//com.graphql_java_generator.client.request.QueryField.appendToGraphQLRequests(QueryField.java:376)
    at app//com.graphql_java_generator.client.request.AbstractGraphQLRequest.getPayload(AbstractGraphQLRequest.java:663)
    at app//com.graphql_java_generator.client.request.AbstractGraphQLRequest.buildRequestAsMap(AbstractGraphQLRequest.java:703)
    at app//com.graphql_java_generator.client.request.AbstractGraphQLRequest.buildRequestAsString(AbstractGraphQLRequest.java:686)
    at app//com.graphql_java_generator.client.RequestExecutionImpl.execute(RequestExecutionImpl.java:125)
    at app//org.example.client.util.MutationExecutor.updateObjectXWithBindValues(MutationExecutor.java:9739)
    at app//org.example.client.util.MutationExecutor.updateObjectX(MutationExecutor.java:9683)

It appears that this check: https://github.com/graphql-java-generator/graphql-maven-plugin-project/blob/graphql-maven-plugin-project-1.18.9/graphql-java-client-runtime/src/main/java/com/graphql_java_generator/client/request/InputParameter.java#L768 is too early in the chain, and the custom scalar should take precedence. Naively changing the code like this:

diff --git a/graphql-java-client-runtime/src/main/java/com/graphql_java_generator/client/request/InputParameter.java b/graphql-java-client-runtime/src/main/java/com/graphql_java_generator/client/request/InputParameter.java
index f363bdb3c..2cc3346cd 100644
--- a/graphql-java-client-runtime/src/main/java/com/graphql_java_generator/client/request/InputParameter.java
+++ b/graphql-java-client-runtime/src/main/java/com/graphql_java_generator/client/request/InputParameter.java
@@ -765,6 +765,12 @@ public class InputParameter {
                } else if (writingGraphQLVariables && val.getClass().isEnum()) {
                        // When writing an enum value in the variables section, values should be between double quotes
                        return "\"" + val.toString() + "\"";
+               } else if (graphQLScalarTypeParam != null) {
+                       Object ret = graphQLScalarTypeParam.getCoercing().serialize(val);
+                       if (ret instanceof String) {
+                               return getStringValue((String) ret);
+                       } else
+                               return ret.toString();
                } else if (val.getClass().isArray()) {
                        // Doing just "Arrays.asList(val)" results in a StackOverflowError!
                        // We have to indicate that val is an array!
@@ -776,12 +782,6 @@ public class InputParameter {
                } else if (val instanceof RawGraphQLString) {
                        // The value is a part of the GraphQL request. Let's write it as is.
                        return ((RawGraphQLString) val).toString();
-               } else if (graphQLScalarTypeParam != null) {
-                       Object ret = graphQLScalarTypeParam.getCoercing().serialize(val);
-                       if (ret instanceof String) {
-                               return getStringValue((String) ret);
-                       } else
-                               return ret.toString();
                } else if (val instanceof String) {
                        // The value is a String. Let's limit it by double quotes
                        return getStringValue((String) val);

fixes my problem (but please note that graphQLScalarTypeParam will then always be null in the remainder of the if-then-else, so it probably needs a better fix).

[etienne-sf]

Hello,

Thanks for the detailed analysis, and fix proposal.

If I undesrtand correctly, you're using the Base64String custom scalar as an input parameter of a query (or mutation), and the exception is thrown when you execute this query. Is this correct ?

Etienne

[gjvoosten]

Correct; the error above happened when trying to send a mutation.

[etienne-sf]

Hello,

This one has lots of internal impacts. Now solved. It will be available in the next release.

Would it be possible for you to test the correction before the release, by building the master branch ?

Etienne

[gjvoosten]

Would it be possible for you to test the correction before the release, by building the master branch ?

I get test failures in graphql-java-client-runtime when trying to build latest master:

[ERROR] Failures: 
[ERROR]   GraphQLReactiveWebSocketHandlerTest.testEncode:60 expected: <{"post":{"topicId":"22","input":{"authorId":"12","date":"2021-03-13","publiclyAvailable":true,"title":"a title","content":"some content"}},"aCustomScalar":[["2021-04-01","2021-04-02"],["2021-04-03","2021-04-04"]],"aDate":"2021-10-11","anEnum":"ADMIN","anIntParam":666}> but was: <{"post":{"topicId":"22","input":{"authorId":"12","date":"2021-03-13","publiclyAvailable":true,"title":"a title","content":"some content"}},"aCustomScalar":[[1617228000000,1617314400000],[1617400800000,1617487200000]],"aDate":1633903200000,"anEnum":"ADMIN","anIntParam":666}>
[INFO] 
[ERROR] Tests run: 195, Failures: 1, Errors: 0, Skipped: 7

[etienne-sf]

Grrr It works for me. It's an issue with the date custom scalar serialization/deserialization. The dates appear to not been properly serialized.

What OS and java version are you using ?

BTW: you've 195 unit tests executed and one failure, which means that the base64 unit test is Ok. It would be better to have the whole build to be ok to also execute the base 64 IT tests, that run latter. But it's a first passed test.

[gjvoosten]

I'm building on Fedora 37, and get the same test failure on a clean build using either Java 17, 11, or 1.8 (to get that last one to build I needed to comment out the <release>8</release> on the main pom's maven-compiler-plugin, as JDK 1.8 doesn't recognise the --release 8 option).

I can do a mvn clean install -DskipTests of course, but I need a build of the graphql-gradle-plugin installed to my local Maven repo and using these artifacts; what would I need to do for that?

[etienne-sf]

Ok, I'll test with a Fedora 37. I'll need to setup an environment for that, so it will probably take some time.

About the 8, yes I know. It's written in the prerequisite described in the wiki. It's the better way I found to control the target release: if you know a better way, I'll be happy to use it... :)

And yes, the mvn clean install -DskipTests is not a good option: I'll test with the Fedora release and check that, before doing a new release.

[gjvoosten]

I'm also using the packaged Maven, so:

$ mvn -v
Apache Maven 3.8.5 (Red Hat 3.8.5-3)

Thanks for taking the time to address this!

[etienne-sf]

Hello,

I didn't install a Fedora box yet. But there may be an issue in the Date and DateTime custom scalar implementation provided by the plugin: it didn't force a Locale (as it seems useless for me here, but this remain IT... ;) ).

Is it possible for you to update your local git, and retry a build ?

[gjvoosten]

As it happens I saw your commit and was already doing that. :wink: Unfortunately, the error is still there:

[INFO] Running com.graphql_java_generator.client.GraphQLReactiveWebSocketHandlerTest
[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.01 s <<< FAILURE! - in com.graphql_java_generator.client.GraphQLReactiveWebSocketHandlerTest
[ERROR] com.graphql_java_generator.client.GraphQLReactiveWebSocketHandlerTest.testEncode  Time elapsed: 0.007 s  <<< FAILURE!
org.opentest4j.AssertionFailedError: expected: <{"post":{"topicId":"22","input":{"authorId":"12","date":"2021-03-13","publiclyAvailable":true,"title":"a title","content":"some content"}},"aCustomScalar":[["2021-04-01","2021-04-02"],["2021-04-03","2021-04-04"]],"aDate":"2021-10-11","anEnum":"ADMIN","anIntParam":666}> but was: <{"post":{"topicId":"22","input":{"authorId":"12","date":"2021-03-13","publiclyAvailable":true,"title":"a title","content":"some content"}},"aCustomScalar":[[1617228000000,1617314400000],[1617400800000,1617487200000]],"aDate":1633903200000,"anEnum":"ADMIN","anIntParam":666}>
    at com.graphql_java_generator.client.GraphQLReactiveWebSocketHandlerTest.testEncode(GraphQLReactiveWebSocketHandlerTest.java:60)

[etienne-sf]

Ok, thank you for your very quick test.

I'll try it with a fedora box.

[etienne-sf]

I could repeat the building error.

So I still released the 1.18.10 version.

[gjvoosten]

I can confirm that this issue is indeed solved with release 1.18.10; thanks @etienne-sf !