Allow custom scalar, parsed to different types, to bind onto java.lang.Object

[viico]

Hi,

We are migrating an application to Spring Graphql library from kickstart library, we have working queries/mutations. Our application has several queries to get a list of business entity with an input containing a generic filter object (field, operator and value). This filter input has a value property which is an Object type : depending on the operator (equal or in) it could be a String or a List (or anything else). We have created a GraphQLScalarType Object to represent this field.

We have an issue with this filter input and the scalar type for array values. When we pass an array of values we have this exception (the same works with the kickstart library):

org.springframework.beans.InvalidPropertyException: Invalid property 'value[0]' of bean class [issue.spring.graphql.objectlist.BusinessEntityFilterInput]: Property referenced in indexed property path 'value[0]' is neither an array nor a List nor a Map; returned value was [java.lang.Object@74123110]

There is no issue if the scalar type is used directly as input type.

We pushed a minimal application with tests to reproduce the error :

1. clone the repo https://github.com/viico/SpringGraphqlObjectVariableArray
2. launch test of BusinessEntityControllerTests class (it is possible to start the application and send queries)

The application contains 2 queries :

• filterValueIsInstanceOfList which return true if the value of input filter object is instance of List
• inputIsInstanceOfList which return true if the input is instance of List

We set up 4 tests :

• inputIsInstanceOfList_listValue : working test with a an array of 2 values, the input is an instance of List
• filterValueIsInstanceOfList_stringValue : working test with a String value (return false because it is not an array of values)
• filterValueIsInstanceOfList_withException : test failing with the exception above when passing a List of 2 values
• filterValueIsInstanceOfList_withoutVariables : test without exception but with an interrogation, we put the input value directly in the GraphQL query and we have an ArrayValue object, shall we have a List instead ?

After some research we don't see how to solve this issue (the exception) and finish our migration, could you help us ?

Thanks

[djselzlein]

Did you try implementing the parseLiteral method on your custom coercing impl?:
https://github.com/viico/SpringGraphqlObjectVariableArray/blob/main/src/main/java/issue/spring/graphql/objectlist/GraphQlConfig.java

For reference, see graphql.scalars.object.ObjectScalar own parseLiteral and how it handles parsing input when instanceof ArrayValue.
It might be even worth trying to reuse ObjectScalar's own coercing impl on your Scalar config.

[viico]

Hi @djselzlein,

Thanks for you answer, I tried with the ExtendedScalars.Object type but I have the same error (pushed on the main branch). Now, the test filterValueIsInstanceOfList_withoutVariables failed like the other (filterValueIsInstanceOfList_withException) : it is more coherent.

I don't think there is a problem with our custom scalar type, it works perfectly with the kickstart library. I created a branch on the repo, with the same query but using the kickstart library and there is no error (you have to start the project to test).

[rstoyanchev]

Thanks for the sample.

The case seems to be a custom scalar type parsed into a List or String where the target property is java.lang.Object. Effectively this is emulating a union.

Our GraphQL argument binding mechanism takes into account both the GraphQL arguments map and the type information from the target object. Typically, target types are more specific, e.g. complex objects to be created from generic maps, lists, and scalars. So the binding relies on type information from the target to decide what to instantiate, what to convert, etc. In this case, however, the target is java.lang.Object and the binding doesn't expect that.

We could make an exception specifically where the target is java.lang.Object and simply set it to whatever comes from the source map. This would prevent it from being able to nest deeper, e.g. say a List of complex objects, but if target is java.lang.Object not much else can be done. In effect, this is asking for whatever comes from the source map to carry over to the target as is.

Alternatively, you could make this a bit more explicit by creating a Java type with both a List and a String field:

class CustomValue {

    private String stringValue;

    private List<String> stringValue;

    // ...
}

Yet another alternative is to just read the raw arguments map, which in this case is quite simple.

As to why this worked in Kickstart, I can't say for sure, but I suspect its binding mechanism might involve serializing the GraphQL arguments map to JSON and then back to the target Object. That would probably work in your case, but more generally we ran into problems with that approach, see #122.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[viico]

The case seems to be a custom scalar type parsed into a List or String where the target property is java.lang.Object. Effectively this is emulating a union.

I just put this 2 options (String or list of String) in the repo to reproduce the problem but we can have anything in this field (an integer, a list of integer, some complex object...). We would not like explicit all possibilities (like your proposal of CustomValue), I'm not even sure that we can.

I would like to point out that it fails only if the list is in an input object, it works if the list is directly passed as input. The query inputIsInstanceOfList(object: Object!): Boolean works if I put the list in object input. But it doesn't work for the query filterValueIsInstanceOfList(filter: EntityFilterInput!): Boolean if the list is a property of the filter input.

We can see in the stacktrace that the error is not at graphql-java lib level.

The main branch of the repo to reproduce contains an example with the ObjectScalar of the lib https://github.com/graphql-java/graphql-java-extended-scalars : we have the same error. This official lib of graphql-java explicit this custom scalar type in comment : An object scalar allows you to have a multi level data value without defining it in the graphql schema., that's exactly what we want. I understood that the spring library use this graphql-java library, so I don't understand why this "official" custom scalar type is not compatible with the Spring library ?

[rstoyanchev]

Did you read the rest of my comment? I provided an explanation and proposed a solution. What do you think about it?

[viico]

Yes we read your comment entirely (at least 10 times). I'm sorry, english is not our native language, I'm afraid we missed something.

I guess I understand your explanation and I thought I answered to your proposal of CustomValue.

We could make an exception specifically where the target is java.lang.Object and simply set it to whatever comes from the source map. This would prevent it from being able to nest deeper, e.g. say a List of complex objects, but if target is java.lang.Object not much else can be done. In effect, this is asking for whatever comes from the source map to carry over to the target as is.

I'm not sure if this is a proposal of solution, if so I think it can be great to implement this, it corresponds exactly to : An object scalar allows you to have a multi level data value without defining it in the graphql schema..

Yet another alternative is to just read the raw arguments map, which in this case is quite simple.

I guess this is an alternative of implementation for you ? I don't see where I can read the raw arguments map myself.

[rstoyanchev]

Yes we read your comment entirely (at least 10 times). I'm sorry, english is not our native language, I'm afraid we missed something.

Okay, no worries. Just wanted to make sure we understand each other before I proceed with any changes.

I'm not sure if this is a proposal of solution, if so I think it can be great to implement this, it corresponds exactly to : An object scalar allows you to have a multi level data value without defining it in the graphql schema.

Yes, that's my proposal, and I'll go ahead with it.

Yet another alternative is to just read the raw arguments map, which in this case is quite simple.
I guess this is an alternative of implementation for you ? I don't see where I can read the raw arguments map myself.

You can always declare DataFetchingEnvironment as a controller method argument, and all that's available is in there, but there is also a more convenient way, by declaring an argument of type @Argument Map<String, Object>. Now that I mention this, I see that we can make it a little more visible in the reference docs.

[rstoyanchev]

I spoke too soon. In the present implementation, which relies on DataBinder to bind the raw arguments map from GraphQL Java onto the target Object, there isn't a good way to bind a Map/List value to an Object property with a setter.

That said, I did make a change so you can do this via constructor binding. Adjusting your example to the below should work:

public class BusinessEntityFilterInput {

    private final String operator;

    private final Object value;

    public BusinessEntityFilterInput(String operator, Object value) {
        this.operator = operator;
        this.value = value;
    }

    public String getOperator() {
        return operator;
    }

    public Object getValue() {
        return value;
    }

}

As I mentioned in my last comment, you can also bypass data binding completely, and work with the raw arguments map:

    @QueryMapping
    public boolean filterValueIsInstanceOfList(@Argument Map<String, Object> filter) {
        return filter.get("value") instanceof List;
    }

[viico]

Hi @rstoyanchev,

Thanks for your work. I would like test this 1.0.2 release but I can't find them in the central repo (https://mvnrepository.com/artifact/org.springframework.graphql/spring-graphql).

Even if I found it in the search maven site (https://search.maven.org/artifact/org.springframework.graphql/spring-graphql/1.0.2/jar).

I waited but it was 2 days ago, do you know why this release is not available ?

[rstoyanchev]

I'm not sure why it's not showing up in the UI, but it is there in the repo and has been for 2 days. It should work fine in your Maven or Gradle build.

[viico]

My bad it's just an UI problem.

I tried with the release 1.0.2 and I still have the error.

I debug and I figure out that exception is throw before your modification (else if added at line 287 of GraphQLArgumentBinder class). Problems occurs when initBindValues method is called (line 259). I did go deeper and exception is throw in the method processKeyedProperty of the class AbstractNestablePropertyAccessor.

[rstoyanchev]

Could you clarify if you switched to using constructor args or not? As I explained in #447 (comment), there is no simple way to fix the issue when using setters for your Object property, but it should work if you switch to constructor args.

[viico]

My bad again, when I first read your comment i said to myself "don't forget to do this", but I did... All is ok when I made what you said.

Thanks you

[rstoyanchev]

Note that in 1.1.x, this issue should be fixed also for the case with setters/getters, see #516.