smallrye-graphql icon indicating copy to clipboard operation
smallrye-graphql copied to clipboard

Type-safe client custom serialiser for input variable

Open jmini opened this issue 1 year ago • 5 comments

I am using the gitlab graphql api and I am running this mutation request:

mutation workItemUpdate($arg0: WorkItemUpdateInput!) {
  workItemUpdate(input: $arg0) {
    errors
    workItem {
      id
      webUrl
    }
  }
}

The java client I am using is here: https://github.com/unblu/gitlab-workitem-graphql-client/

In particular see the WorkItemUpdateInput class definition which has a member hierarchyWidget of type WorkItemWidgetHierarchyUpdateInput

The documentation is not really clear about the hierarchyWidget: https://docs.gitlab.com/ee/api/graphql/reference/#workitemwidgethierarchyupdateinput

Those are the key point (for the client):

  • for parentId you have 3 values:
    • setting an id (String value) to set a new parent
    • setting null to remove the parent association
    • not setting the attribute to not modify the parent association
  • you can not set parentId and childrenIds in the same request
    • runtime error: "One and only one of children, parent or remove_child is required", so in theory we should do multiple runs, especially in case of "turning a child into a parent or the other way around".

So again this is a case where not setting the value and setting it explicitly to null has a different meaning. https://github.com/json-schema-org/json-schema-spec/issues/584#issuecomment-398875472

And in Java this is always tricky.


I stared an implementation with a NullableProperty<T> helper class (to support the 3 states) annotated with @JsonbTypeSerializer and @JsonbTypeDeserializer

Yasson test (jbang)

///usr/bin/env jbang "$0" "$@" ; exit $?

//DEPS org.eclipse:yasson:3.0.4
//JAVA 11

import java.lang.reflect.Type;
import java.util.Optional;

import jakarta.json.JsonValue;
import jakarta.json.bind.Jsonb;
import jakarta.json.bind.JsonbBuilder;
import jakarta.json.bind.annotation.JsonbTypeDeserializer;
import jakarta.json.bind.annotation.JsonbTypeSerializer;
import jakarta.json.bind.serializer.DeserializationContext;
import jakarta.json.bind.serializer.JsonbDeserializer;
import jakarta.json.bind.serializer.JsonbSerializer;
import jakarta.json.bind.serializer.SerializationContext;
import jakarta.json.stream.JsonGenerator;
import jakarta.json.stream.JsonParser;

public class YassonTest {

    public static void main(String[] args) {
        Jsonb jsonb = JsonbBuilder.create();

        Example example = new Example();

        // Case 1: Set name to "John"
        example.setName(NullableProperty.of("John"));
        System.out.println(jsonb.toJson(example)); // {"name":"John"}

        // Case 2: Set name to NullableProperty.empty()
        example.setName(NullableProperty.empty());
        System.out.println(jsonb.toJson(example)); // {"name":null}

        // Case 3: Set name to null
        example.setName(null);
        System.out.println(jsonb.toJson(example)); // {}

    }

    public static class Example {
        @JsonbTypeSerializer(NullablePropertySerializer.class)
        @JsonbTypeDeserializer(NullablePropertyDeserializer.class)
        private NullableProperty<String> name;

        public NullableProperty<String> getName() {
            return name;
        }

        public void setName(NullableProperty<String> name) {
            this.name = name;
        }
    }

    public static class NullablePropertyDeserializer<T> implements JsonbDeserializer<NullableProperty<T>> {

        @Override
        public NullableProperty<T> deserialize(JsonParser parser, DeserializationContext ctx, Type rtType) {
            JsonValue value = parser.getValue();
            if (value == JsonValue.NULL) {
                return NullableProperty.empty();
            } else {
                T deserializedValue = ctx.deserialize(rtType, parser);
                return NullableProperty.of(deserializedValue);
            }
        }
    }

    public static class NullablePropertySerializer<T> implements JsonbSerializer<NullableProperty<T>> {
        @Override
        public void serialize(NullableProperty<T> obj, JsonGenerator generator, SerializationContext ctx) {
            if (obj.isPresent()) {
                ctx.serialize(obj.get(), generator);
            } else {
                generator.write(JsonValue.NULL);
            }
        }
    }

    public static class NullableProperty<T> {
        private final T value;

        private NullableProperty(T value) {
            this.value = value;
        }

        public static <T> NullableProperty<T> of(T value) {
            if (value == null) {
                throw new IllegalArgumentException("Use NullableProperty.empty() for null values");
            }
            return new NullableProperty<>(value);
        }

        public static <T> NullableProperty<T> empty() {
            return new NullableProperty<>(null);
        }

        public Optional<T> toOptional() {
            return Optional.ofNullable(value);
        }

        public T get() {
            return value;
        }

        public boolean isPresent() {
            return value != null;
        }
    }
}

It works great at Json-b level, but I have the feeling that the typesafe client is doing something else when it comes to the serialization of the Input object.

jmini avatar Jan 21 '25 07:01 jmini

Right, we don't use json-b for serializing input objects, the typesafe client implements that manually. Sorry I'm probably not following here, isn't the @JsonbNillable annotation enough?

jmartisk avatar Jan 22 '25 12:01 jmartisk

With @JsonbNillable on parentId

https://github.com/unblu/gitlab-workitem-graphql-client/blob/b93a9f97e53e2647a0531c33b2dc67be940fd80c/src/main/java/graphql/gitlab/model/WorkItemWidgetHierarchyUpdateInput.java#L26-L27

This is always sending null when the value is not set.


But I need to be in control and be able to (at JSON level):

  • set the value to null --> to remove a parent relation
  • not set the parentId attribute --> to let the parent relation unchanged and this is mandatory when childrenIds is set.
  • set the value to a String --> to add a parent relation.

If I really display the arg0 for the different cases:

Remove a parent relation:

{
  "arg0": {
    "id": "gid://gitlab/WorkItem/123",
    "hierarchyWidget": {
      "parentId": null
    }
  }
}

Add a child relation:

{
  "input": {
    "id": "gid://gitlab/WorkItem/123",
    "hierarchyWidget": {
      "childrenIds": [
        "gid://gitlab/WorkItem/789"
      ]
    }
  }
}

--> note that parentId is not set.

Set a parent relation:

{
  "arg0": {
    "id": "gid://gitlab/WorkItem/123",
    "hierarchyWidget": {
      "parentId": "gid://gitlab/WorkItem/456"
    }
  }
}

Problematic call:

{
  "arg0":{
    "id": "gid://gitlab/WorkItem/123",
    "hierarchyWidget": {
      "parentId": null,
      "childrenIds": [
        "gid://gitlab/WorkItem/789"
      ]
    }
  }
}

--> error: One and only one of children, parent or remove_child is required

Which is what I currently get the java client with nillable. I have no way to express that parentId should not be set.

jmini avatar Jan 22 '25 16:01 jmini

My solution https://github.com/unblu/gitlab-workitem-graphql-client/commit/6c578ba27d0ab3f98a139668c34b7b18c3eb6d87 with @JsonbTypeSerializer and @JsonbTypeDeserializer that is not working in the typesafe client as discussed in https://github.com/smallrye/smallrye-graphql/issues/2257#issuecomment-2607181244

jmini avatar Jan 22 '25 16:01 jmini

With https://github.com/unblu/gitlab-workitem-graphql-client/commit/96673d87850c148ef174fbf4c5d163db745ae3ea I am now trying a different approach:

Have WorkItemWidgetHierarchyUpdateInput as interface with 2 classes implementing it:

  • WorkItemWidgetHierarchyUpdateInputWithParent
  • WorkItemWidgetHierarchyUpdateInputWithChildren

At the end since the server do not support setting both childrenIds and parentId at the same time, it doesn't feel wrong from a java modeling point-of-view.

I now need to verify if it works with the smallrye-graphql typesafe client, when running the different queries.

jmini avatar Jan 22 '25 17:01 jmini

I have updated my WorkItemScript to use the approach discussed https://github.com/smallrye/smallrye-graphql/issues/2257#issuecomment-2607808479

Having the WorkItemWidgetHierarchyUpdateInput as an interface seems to work for the graphql-typesafe-client engine.

jmini avatar Jan 23 '25 08:01 jmini