Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot use Set as property in GraphQL argument input object #394

Closed
AndriuQuesada opened this issue May 25, 2022 · 5 comments
Closed

Cannot use Set as property in GraphQL argument input object #394

AndriuQuesada opened this issue May 25, 2022 · 5 comments
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@AndriuQuesada
Copy link

Hello,
I have a ManyToMany relationship between two entities Team and Job.
The Team class contains the next code

        @ManyToMany
        @Fetch(FetchMode.SUBSELECT)
        @JoinTable(name = "team_job", joinColumns = {
                        @JoinColumn(name = "team_id", referencedColumnName = "id") }, inverseJoinColumns = {
                                        @JoinColumn(name = "job_id", referencedColumnName = "id") })
        private Set<Job> jobs = new HashSet<Job>();

Querying the Teams with their jobs using graphql works perfectly, but when doing a mutation like the next

mutation {
  saveTeam(team: {id: 602, name: "t2", description: "Team 2", jobs: [{id: 1}]}) {
    id
    name
    description
    members {
      id
      username
      email
    }
    jobs {
      id
      jobCode
      moduleName
      assetOwner
    }
  }
}

The next error on the server is raised:

2022-05-25 23:20:19.913 ERROR 15600 --- [     parallel-2] s.g.e.ExceptionResolversExceptionHandler : Unresolved InvalidPropertyException for executionId db62b649-4691-943b-62a3-a579022f86de

org.springframework.beans.InvalidPropertyException: Invalid property 'jobs[0]' of bean class [gbi.involve.erafortb.domain.Team]: Illegal attempt 
to get property 'jobs' threw exception; nested exception is org.springframework.beans.InvalidPropertyException: Invalid property 'jobs[0]' of bean class [gbi.involve.erafortb.domain.Team]: Cannot get element with index 0 from Set of size 0, accessed using property path 'jobs[0]'
        at org.springframework.beans.AbstractNestablePropertyAccessor.getPropertyValue(AbstractNestablePropertyAccessor.java:687) ~[classes/:na] 
        at org.springframework.beans.AbstractNestablePropertyAccessor.getNestedPropertyAccessor(AbstractNestablePropertyAccessor.java:826) ~[classes/:na]
        at org.springframework.beans.AbstractNestablePropertyAccessor.getPropertyAccessorForPropertyPath(AbstractNestablePropertyAccessor.java:802) ~[classes/:na]
        at org.springframework.beans.AbstractNestablePropertyAccessor.setPropertyValue(AbstractNestablePropertyAccessor.java:269) ~[classes/:na] 
        at org.springframework.beans.AbstractPropertyAccessor.setPropertyValues(AbstractPropertyAccessor.java:104) ~[spring-beans-5.3.20.jar:5.3.20]
        at org.springframework.validation.DataBinder.applyPropertyValues(DataBinder.java:889) ~[spring-context-5.3.20.jar:5.3.20]
        at org.springframework.validation.DataBinder.doBind(DataBinder.java:780) ~[spring-context-5.3.20.jar:5.3.20]
        at org.springframework.validation.DataBinder.bind(DataBinder.java:765) ~[spring-context-5.3.20.jar:5.3.20]
        at org.springframework.graphql.data.GraphQlArgumentBinder.createValue(GraphQlArgumentBinder.java:228) ~[spring-graphql-1.0.0.jar:1.0.0]  
        at org.springframework.graphql.data.GraphQlArgumentBinder.bind(GraphQlArgumentBinder.java:144) ~[spring-graphql-1.0.0.jar:1.0.0]
        at org.springframework.graphql.data.method.annotation.support.ArgumentMethodArgumentResolver.resolveArgument(ArgumentMethodArgumentResolver.java:58) ~[spring-graphql-1.0.0.jar:1.0.0]
        at org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:83) ~[spring-graphql-1.0.0.jar:1.0.0]
        at org.springframework.graphql.data.method.annotation.support.DataFetcherHandlerMethod.getMethodArgumentValues(DataFetcherHandlerMethod.java:170) ~[spring-graphql-1.0.0.jar:1.0.0]
        at org.springframework.graphql.data.method.annotation.support.DataFetcherHandlerMethod.invoke(DataFetcherHandlerMethod.java:115) ~[spring-graphql-1.0.0.jar:1.0.0]
        at org.springframework.graphql.data.method.annotation.support.AnnotatedControllerConfigurer$SchemaMappingDataFetcher.get(AnnotatedControllerConfigurer.java:497) ~[spring-graphql-1.0.0.jar:1.0.0]
        at org.springframework.graphql.execution.ContextDataFetcherDecorator.lambda$get$0(ContextDataFetcherDecorator.java:64) ~[spring-graphql-1.0.0.jar:1.0.0]
        at org.springframework.graphql.execution.ReactorContextManager.invokeCallable(ReactorContextManager.java:104) ~[spring-graphql-1.0.0.jar:1.0.0]
        at org.springframework.graphql.execution.ContextDataFetcherDecorator.get(ContextDataFetcherDecorator.java:63) ~[spring-graphql-1.0.0.jar:1.0.0]
        at org.springframework.boot.actuate.metrics.graphql.GraphQlMetricsInstrumentation.lambda$instrumentDataFetcher$1(GraphQlMetricsInstrumentation.java:98) ~[spring-boot-actuator-2.7.0-SNAPSHOT.jar:2.7.0-SNAPSHOT]
        at graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation.lambda$instrumentDataFetcher$0(DataLoaderDispatcherInstrumentation.java:87) ~[graphql-java-18.1.jar:na]
        at graphql.execution.ExecutionStrategy.fetchField(ExecutionStrategy.java:279) ~[graphql-java-18.1.jar:na]
        at graphql.execution.ExecutionStrategy.resolveFieldWithInfo(ExecutionStrategy.java:210) ~[graphql-java-18.1.jar:na]
        at graphql.execution.ExecutionStrategy.resolveField(ExecutionStrategy.java:182) ~[graphql-java-18.1.jar:na]
        at graphql.execution.AsyncSerialExecutionStrategy.lambda$execute$1(AsyncSerialExecutionStrategy.java:43) ~[graphql-java-18.1.jar:na]     
        at graphql.execution.Async.eachSequentiallyImpl(Async.java:80) ~[graphql-java-18.1.jar:na]
        at graphql.execution.Async.eachSequentially(Async.java:69) ~[graphql-java-18.1.jar:na]
        at graphql.execution.AsyncSerialExecutionStrategy.execute(AsyncSerialExecutionStrategy.java:38) ~[graphql-java-18.1.jar:na]
        at graphql.execution.Execution.executeOperation(Execution.java:160) ~[graphql-java-18.1.jar:na]
        at graphql.execution.Execution.execute(Execution.java:106) ~[graphql-java-18.1.jar:na]
        at graphql.GraphQL.execute(GraphQL.java:641) ~[graphql-java-18.1.jar:na]
        at graphql.GraphQL.lambda$parseValidateAndExecute$11(GraphQL.java:561) ~[graphql-java-18.1.jar:na]
        at java.base/java.util.concurrent.CompletableFuture.uniComposeStage(CompletableFuture.java:1187) ~[na:na]
        at java.base/java.util.concurrent.CompletableFuture.thenCompose(CompletableFuture.java:2309) ~[na:na]
        at graphql.GraphQL.parseValidateAndExecute(GraphQL.java:556) ~[graphql-java-18.1.jar:na]
        at graphql.GraphQL.executeAsync(GraphQL.java:524) ~[graphql-java-18.1.jar:na]
        at org.springframework.graphql.execution.DefaultExecutionGraphQlService.lambda$execute$2(DefaultExecutionGraphQlService.java:81) ~[spring-graphql-1.0.0.jar:1.0.0]
        at reactor.core.publisher.MonoDeferContextual.subscribe(MonoDeferContextual.java:47) ~[reactor-core-3.4.18.jar:3.4.18]
        at reactor.core.publisher.InternalMonoOperator.subscribe(InternalMonoOperator.java:64) ~[reactor-core-3.4.18.jar:3.4.18]
        at reactor.core.publisher.MonoFlatMap$FlatMapMain.onNext(MonoFlatMap.java:157) ~[reactor-core-3.4.18.jar:3.4.18]
        at reactor.core.publisher.MonoDelay$MonoDelayRunnable.propagateDelay(MonoDelay.java:271) ~[reactor-core-3.4.18.jar:3.4.18]
        at reactor.core.publisher.MonoDelay$MonoDelayRunnable.run(MonoDelay.java:286) ~[reactor-core-3.4.18.jar:3.4.18]
        at reactor.core.scheduler.SchedulerTask.call(SchedulerTask.java:68) ~[reactor-core-3.4.18.jar:3.4.18]
        at reactor.core.scheduler.SchedulerTask.call(SchedulerTask.java:28) ~[reactor-core-3.4.18.jar:3.4.18]
        at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264) ~[na:na]
        at java.base/java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) ~[na:na]     
        at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) ~[na:na]
        at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) ~[na:na]
        at java.base/java.lang.Thread.run(Thread.java:833) ~[na:na]
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label May 25, 2022
@bclozel bclozel self-assigned this Jun 9, 2022
@bclozel
Copy link
Member

bclozel commented Jun 23, 2022

We've discussed this issue and we'd like to give an update here.
We think that the binding process is using the setter to set the "jobs" property using the JSON map provided by GraphQL Java - in this case, the data binder is using the java bean property syntax "jobs[0]" and this only works with indexed collections. We need to have a deeper look.

In the meantime, we think that having a constructor with arguments would work with the current implementation. This might be problematic if you're binding to a JPA entity - but we also think that binding directly to an entity is a bit strange in the first place (you might want to use specific objects for your API). For the time being we're not sure about the solution - we might improve the binding process or just document this as a limitation.

Can you try creating a constructor with arguments and see if that solves the issue?

@JBodkin-Amphora
Copy link

JBodkin-Amphora commented Aug 5, 2022

I've been seeing a similar issue whereby we're trying to bind to an entity using the constructor (kotlin). This is failing as GraphQL creates a List and the Entity is expecting a Set. After debugging the code, I found that the argument binder is creating a new list rather than a set.

This is the line in question: https://github.com/spring-projects/spring-graphql/blob/main/spring-graphql/src/main/java/org/springframework/graphql/data/GraphQlArgumentBinder.java#L212

Instead of passing in the rawCollection when binding, I'm thinking we should pass in the constructor type instead, using this method:

public static <E> Collection<E> createCollection(Class<?> collectionType, @Nullable Class<?> elementType, int capacity)

Stack Trace

org.springframework.beans.BeanInstantiationException: Failed to instantiate [xxx.Entity]: Illegal arguments for constructor; nested exception is java.lang.IllegalArgumentException: argument type mismatch
	at org.springframework.beans.BeanUtils.instantiateClass(BeanUtils.java:221)
	at org.springframework.graphql.data.GraphQlArgumentBinder.createValue(GraphQlArgumentBinder.java:304)
	at org.springframework.graphql.data.GraphQlArgumentBinder.createValueOrNull(GraphQlArgumentBinder.java:235)
	at org.springframework.graphql.data.GraphQlArgumentBinder.createCollection(GraphQlArgumentBinder.java:220)
	at org.springframework.graphql.data.GraphQlArgumentBinder.createValue(GraphQlArgumentBinder.java:288)
	at org.springframework.graphql.data.GraphQlArgumentBinder.createValueOrNull(GraphQlArgumentBinder.java:235)
	at org.springframework.graphql.data.GraphQlArgumentBinder.createValue(GraphQlArgumentBinder.java:291)
	at org.springframework.graphql.data.GraphQlArgumentBinder.bind(GraphQlArgumentBinder.java:163)
	at org.springframework.graphql.data.method.annotation.support.ArgumentMethodArgumentResolver.resolveArgument(ArgumentMethodArgumentResolver.java:58)

@bclozel bclozel added the for: team-attention An issue we need to discuss as a team to make progress label Sep 8, 2022
@bclozel
Copy link
Member

bclozel commented Sep 8, 2022

Thanks @JBodkin-Amphora , I've created #485 to solve this. Sorry for the late reply. This should now work for constructor binding.

In @AndriuQuesada 's case, constructor binding is not being used and we're instead using the java bean property notation to bind to an obejct. The Spring Framework reference documentation for binding nested properties only refers to naturally ordered collections but does not mention Sets.

When binding to a set, the property accessor tries to access the element and, unlike other Collections, does not try to "auto-grow" the collection. Maybe this is something that could be taken care of in Spring Framework. We'll need to discuss that further in the team. Note that the Spring Boot Binder seems to support this notation for Set.

@bclozel bclozel added status: pending-design-work and removed for: team-attention An issue we need to discuss as a team to make progress labels Sep 22, 2022
@bclozel
Copy link
Member

bclozel commented Oct 21, 2022

This might be fixed by #516

@rstoyanchev rstoyanchev assigned rstoyanchev and unassigned bclozel Oct 21, 2022
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged status: pending-design-work labels Oct 21, 2022
@rstoyanchev rstoyanchev added this to the 1.1.0 milestone Oct 21, 2022
@rstoyanchev
Copy link
Contributor

I've added a test that confirms this is fixed by #516, which I will commit shortly. This will therefore be available in the upcoming 1.1 release.

rstoyanchev added a commit that referenced this issue Oct 21, 2022
@rstoyanchev rstoyanchev changed the title ManyToMany HashSet Property mutation: Cannot get element with index 0 from Set of size 0, accessed using property path 'jobs[0]' Cannot use Set as property for GraphQL argument input object Nov 9, 2022
@rstoyanchev rstoyanchev changed the title Cannot use Set as property for GraphQL argument input object Cannot use Set as property in GraphQL argument input object Nov 9, 2022
koenpunt pushed a commit to koenpunt/spring-graphql that referenced this issue Feb 2, 2023
koenpunt pushed a commit to koenpunt/spring-graphql that referenced this issue Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

5 participants