ZonedDateTime serialization error (Version: 2.4.1)

[ClaudiuGuja]

Issue: ZonedDateTime string not escaped.

When sending a variable through graphql of type ZonedDateTime, in GraphQLRequestSerializer method getEntry, the ZonedDateTime goes through the last else branch, and toString() is called on it. This leads to a request that does not fit the JSON format and a BadRequest exception is thrown.

Instead, when sending a string that contains the result of toString() called on the ZonedDateTime, the string is escaped at line 100 of GraphQLRequestSerializer, and the call is made correctly.

Possible solutions:

• add another branch to the getEntry method
• ability to inject a custom ObjectMapper into GraphQLRequestSerializer. Sadly this means no static methods and brings with it another host of problems

[ClaudiuGuja]

I have also tried to use the custom mapping options

customTypesMapping = [
            ZonedDateTime: "java.time.ZonedDateTime"
    ]
    customAnnotationsMapping = [
            ZonedDateTime: "@com.fasterxml.jackson.databind.annotation.JsonSerialize(using = graphqlclient.config.CustomSerializer.class)"
    ]

the CustomSerializer having the method

 @Override
    public void serialize(ZonedDateTime value, JsonGenerator gen, SerializerProvider provider) throws IOException {
        gen.writeString("anything");
    }

but still returning the same error, as if the serialization is not applied

[kobylynskyi]

@ClaudiuGuja as of today it is not supported. Can you please try adding a custom type mapping for ZonedDateTime?
ZonedDateTime: String

[ClaudiuGuja]

Solution I went with:

• Make a JsonSerializationService that can be injected with an ObjectMapper
• Have the method of toHttpJsonBody with it's current functionality, and in getEntry instead of

else {
            return input.toString();
        }

I've added

{
            try {
                return objectMapper.writeValueAsString(input);
            } catch (JsonProcessingException e) {
                throw new UnableToBuildJsonQueryException(e);
            }
        }

I would like to ask though if the customAnnotationMappings are only for the results, and not for the requests, or if there are plans in the future to add the posibility of formatting on the query requests as well

[kobylynskyi]

@ClaudiuGuja annotations are being added to all types, so ObjectMapper inside GraphQLRequestSerializer should pick it up.
Can you please show your schema excerpt which you have problems with?

[ClaudiuGuja]

@kobylynskyi

build.gradle config:

customTypesMapping = [
            ZonedDateTime: "java.time.ZonedDateTime"
    ]
    customAnnotationsMapping = [
            ZonedDateTime: "@SomeAnnotation"
    ]

schema.graphqls

type Query {
    queryINeed(
        input: ZonedDateTime!
    ): [ResponseContainingDate]
    otherQueryUsedAsExamle(
        input: InputContainingDate!
    ): String
}

type ResponseContainingDate{
    a: ZonedDateTime
}

input InputContainingDate{
    a: ZonedDateTime
}

# ZonedDateTime Scalar
scalar ZonedDateTime

Conclusions from the example above:
indeed the annotation is added to the generated classes InputContainingDate, and ResponseContainingDate, but it's not added to QueryINeedRequest.

But even so, both OtherQueryUsedAsExampleRequest and QueryINeedRequest are have toHttpJsonBody called on them. When I tried following the serialization, I saw that getEntry(inputEntry.getValue, jsonQuery) is called of them, and inside, toString is called on both. so that they are both serialized wrongly (not in JSON format), not through the ObjectMapper.

[kobylynskyi]

Thanks for sharing the sample schema.
You are right, just adding an annotation to the field in the request class is not enough. We would need to use object mapper to serialize the whole object. But the thing is that if we use OBJECT_MAPPER.writeValueAsString(input); then the query is being serialized not in GraphQL-style:

So we need to find a way how to serialize the whole GraphQLRequest properly.

[kobylynskyi]

As of today, I would suggest you to add a custom mapping from ZonedDateTime to String:
customTypesMapping = [ "ZonedDateTime": "String" ]
But this will impact all other places where you might want to keep as java.time.ZonedDateTime.

As another solution, I can add a possibility to specify custom type mapping for operation input:
customTypesMapping = [ "queryINeed.input": "String" ]
If you are ok with this, I will release it today/tomorrow as part of v2.4.2.

In both scenarios you can pass a serialized value of ZonedDateTime in the format that you wish (while you are constructing a GraphQLRequest object):

QueryINeedQueryRequest request = new QueryINeedQueryRequest.Builder()
        .setInput(ZonedDateTime.now().format(DateTimeFormatter.ISO_DATE_TIME))
        .build();

[ClaudiuGuja]

Indeed, just using the ObjectMapper there doesn't solve the whole problem. I'd like to think a bit more on how this might be solved before applying a fix. At the moment I can do by with just using the ObjectMapper, but if the input is a more complex object, then the JSON format is not good enough anymore

[ClaudiuGuja]

@kobylynskyi , how about the following solution:

• add a marker interface to the input types
• have getEntry() check if the input is of the marker interface type. If so, then use toString() on it. if not, then ObjectMapper.

This way, the input field names can still be formatted to fit with graphql, and the values should be ok if they pass through the objectMapper (tested for example on Int, to make sure that it doesn't add the quotation marks to it)

[ClaudiuGuja]

That would still leave the issue of ZonedDateTimes that are directly in the request, since the request keeps it's parameters in a Map, and no annotations can be added there. For that, perhaps a getter method for the ObjectMapper from GraphQLRequestSerializer, so that people can customize it at the start of their application running.

Thoughts?

[kobylynskyi]

@ClaudiuGuja yes, with this approach we will still have the issue with custom types serialization.
Maybe we need to introduce another config taking a list of types for which we delegate the serialization to ObjectMapper instead of a toString(). But then we would need to somehow make ObjectMapper injectable (or at least configurable)

[jxnu-liguobin]

Hi,are you talking about escaping parameters, like this id parameter

{
    "query": "query { human(id: \"1001\"){ name id appearsIn friends { name id appearsIn friends { name id appearsIn secretBackstory __typename } secretBackstory __typename } homePlanet secretBackstory email __typename } }"
}

Can we support the following format?

{
    "operationName": "createUserVariable",
    "variables": {
        "userVariable": {
            "name": "testgraphql",
            "key": "test_132",
            "valueType": "int",
            "description": "132"
        }
    },
    "query": "mutation createUserVariable($userVariable: VariableInput!) {\n  createUserVariable(userVariable: $userVariable) {\n    name\n    __typename\n  }\n}\n"
}

put the parameters in variables independently

[kobylynskyi]

@jxnu-liguobin, there are no issues with escaping parameters.
The issue is that GraphQLRequestSerializer serializes everything using toString() (which is fine in most cases because it is overridden by all classes generated by this plugin). But if custom non-generated class is supplied (like ZonedDateTime), then ZonedDateTime.toString() returns it's own default format (e.g.: 2020-07-30T22:17:17.884-05:00[America/Chicago]) which is not what we want.

---

The only workaround as of today is to add a custom mapping from ZonedDateTime to String:

customTypesMapping = [ "ZonedDateTime": "String" ]

So that users can pass a serialized value of ZonedDateTime in the desired format while creating a GraphQLRequest:

QueryINeedQueryRequest request = new QueryINeedQueryRequest.Builder()
        .setInput(ZonedDateTime.now().format(DateTimeFormatter.ISO_DATE_TIME))
        .build();

[jxnu-liguobin]

Oh, it does seem like a common scene.