Support non-object values for JSON scalars

[dpolysiou]

Extend support for JSON scalars, to include not only JSON objects, but also any other JSON value, like strings, numbers and arrays.

[phillip-kruger]

Would it not be better to map the individual values ? So Array to Array, String to String etc ? @jmartisk ??

[dpolysiou]

I uploaded a sample project with a few test cases, and I amended my commit to include separate mappings for the JsonValue subtypes.

There are a couple of points that are unclear to me:

1. When used as an input, this scalar expects the json to be passed as a JSON-encoded string. I don't know if it would be possible to pass raw JSON instead.
2. For the JsonNumber mapping, I used

populateScalar("jakarta.json.JsonNumber", JSON, String.class.getName());

which seems to work, but I don't know if it is correct.

[phillip-kruger]

Should number not map to a java Number ? and Array to Array ?

[dpolysiou]

I tried

        populateScalar("jakarta.json.JsonNumber", JSON, Number.class.getName());
        populateScalar("jakarta.json.JsonArray", JSON, Object[].class.getName());

and got no errors (also, I saw no difference in the query response).

To be honest, I'm having a hard time tracking where this third argument is actually used.

I see it used in io.smallrye.graphql.execution.Classes.isNumberLikeType, but that method will return false for Number. Maybe we should use BigDecimal instead?

I also see it used for io.smallrye.graphql.execution.datafetcher.helper.AbstractHelper.shouldTransform, but that method will return true for any value other than jakarta.json.JsonNumber itself.

[phillip-kruger]

Lets wait for @jmartisk to comment

[jmartisk]

TBH I don't see much value in separately registering JsonString, JsonNumber, etc. How about simply supporting JsonValue and JsonObject? If one wanted to have a string field, they would simply use String, not JsonString IMO.

[jmartisk]

To be honest, I'm having a hard time tracking where this third argument is actually used.

Same here TBH, this was written a long time ago and not by me.

[dpolysiou]

TBH I don't see much value in separately registering JsonString, JsonNumber, etc. How about simply supporting JsonValue and JsonObject?

That's understandable. JsonValue pretty much covers everything, and keeping JsonObject would avoid any breaking changes. Should I remove the rest?

[dpolysiou]

I removed most of the added registrations and kept only the addition of JsonValue.

[phillip-kruger]

@jmartisk Any reason the wildfly feature pack build fails ?

[jmartisk]

Sorry for the delay, looking into it again today.

[jmartisk]

I can get JsonValue working as an output type, but were you able to make it work as an input to an operation? I think that may be quite tricky. And anyway, I don't see much value in attempting it. If one wants to accept a (scalar) value instead of a full json object, just make the operation accept a String or Integer.

[dpolysiou]

JsonValue works as an input type, in the same way that JsonObject does, i.e. it is parsed as a JSON-encoded string.

E.g. if you want to pass an array of strings, you can use

input : "[\"foo\",\"bar\"]"

rather than

input: [ "foo", "bar" ]

That's a little inconvenient, and it's not as shown in the graphql-java-extended-scalars readme.

A simple workaround I tried is mapping the java.lang.Object class to a JSON scalar. That works, although the java method then takes an Object instead of a JsonValue.

[jmartisk]

Ah, I was trying to use a value like "x" and [], that doesn't work, even though these are valid json values (and these do work as output types, when the query for example returns JsonValue.EMPTY_JSON_ARRAY). When you use an object for input, it indeed works.

Generally, it seems that passing a JSON array doesn't work as input (but this can be worked around by making the operation accept List<JsonValue>

I'm not sure what exactly we would have to do to accept these values as JsonValue inputs (it would probably be a big mess), but I guess it's not that important.

[dpolysiou]

Perhaps we could add a special transformation that constructs a jakarta.json.JsonValue out of java.lang.Object, using instanceOf checks and recursion?

[dpolysiou]

Ah, I was trying to use a value like "x" and [], that doesn't work, even though these are valid json values (and these do work as output types, when the query for example returns JsonValue.EMPTY_JSON_ARRAY). When you use an object for input, it indeed works.

Generally, it seems that passing a JSON array doesn't work as input (but this can be worked around by making the operation accept List<JsonValue>

I'm not sure I understand the distinction you draw between the handling of JSON objects and arrays. Unless I'm mistaken, neither really "work" as inputs.

For objects, I would expect to be able to pass:

input: { key : "value" }

which doesn't work. The following works, but it isn't quite right:

input: "{"\key\" : "\value\"}"

[jmartisk]

I'm not sure I understand the distinction you draw between the handling of JSON objects and arrays. Unless I'm mistaken, neither really "work" as inputs.

A json array is a valid JsonValue, so I would expect to be able to pass x: [{a:3}] to a query that is declared with a JsonValue x parameter, and that doesn't work right now. But I don't think this is something we can solve in a straightforward way (unless you have some insight that I don't)

Perhaps we could add a special transformation that constructs a jakarta.json.JsonValue out of java.lang.Object, using instanceOf checks and recursion?

Let's not get into that hole please :D

[t1]

input: { key : "value" }

I suppose that you mean: input: { "key": "value" }... sorry for nitpicking 🤓

[jmartisk]

Let's merge this, it's definitely a good start, even though it has a few shortcomings. Thanks!