search

smallrye / smallrye-graphql

Implementation for MicroProfile GraphQL
Apache License 2.0
153 stars 88 forks source link

No json object support in type-safe client. #1482

Open Jacrispys opened 1 year ago

Jacrispys commented 1 year ago

No, it's not implemented, yet, and we'd have to decide about what json classes we'd support.

If you don't need the data field of your image class, just remove it! In fact, you should remove all fields that you don't need, not only because of the bloat in the payload, it could also be expensive for the server to fetch the data and/or count against your usage/complexity threshold.

Originally posted by @t1 in https://github.com/smallrye/smallrye-graphql/discussions/1454#discussioncomment-3211724

Jacrispys commented 1 year ago

Will link PR to solve…

t1 commented 1 year ago

MicroProfile supports JSON-B, so that has to be supported. Other APIs like Jackson are optional.

Jacrispys commented 1 year ago

Alright, I'll get a basic IMPL up tmrw if I have time!

Jacrispys commented 1 year ago

@t1 i plan on doing my homework tonight and reading src for where I need to PR, could you point me to the package I will mostly be working in for this?

Jacrispys commented 1 year ago

bump @t1 im not sure where to look to find where to add a new type to the type safe client.

t1 commented 1 year ago

The code you'll need to change is mainly in client/implementation/src/main/java/io/smallrye/graphql/client/impl/typesafe/json.

Please remember to also add tests, probably to a new JsonBehavior class in client/tck/src/main/java/tck/graphql/typesafe.

Jacrispys commented 1 year ago

Alright, sounds good thanks!

Jacrispys commented 1 year ago

Weird thing... When using the Jakarta JsonObject to parse, it gives this NPE image image

When navigating to the code in my fork, it points to the type being null: image

Which doesn't make sense, because to my knowledge the type field should just be getting object.class.getType() or something similar, which cannot be null.

t1 commented 1 year ago

I'm not sure, but it could be that it recurses into the fields of the JsonObject class and some of those fields is generic, i.e. a ParameterizedType or so. I guess that's too deep, you'll have to stop it from recursing into JsonObject when building the query.

t1 commented 2 weeks ago

@Jacrispys: Can we close this issue?

Jacrispys commented 2 weeks ago

@t1 Yes, I mostly abandoned the project that required json objects a while ago due to many reasons. If you would still like to support JSON objects in the future feel free to use this issue as reference as well as the adjacent discussion (https://github.com/smallrye/smallrye-graphql/discussions/1454#discussioncomment-3228075), but I do not have the time nor the justification to implement this feature myself.