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

ManyToMany HashSet Property mutation: Cannot get element with index 0 from Set of size 0, accessed using property path 'jobs[0]'

Open AndriuQuesada opened this issue 3 years ago • 3 comments

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]

AndriuQuesada avatar May 25 '22 21:05 AndriuQuesada

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?

bclozel avatar Jun 23 '22 08:06 bclozel

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)

JBodkin-Amphora avatar Aug 05 '22 21:08 JBodkin-Amphora

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 avatar Sep 08 '22 13:09 bclozel

This might be fixed by #516

bclozel avatar Oct 21 '22 10:10 bclozel

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 avatar Oct 21 '22 16:10 rstoyanchev