From 5d4373c2777e1d9ff70304397c3756a5f06e0dc8 Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Sun, 5 May 2019 22:59:47 -0700 Subject: [PATCH] fix: build join fetch from selection graph --- .../schema/impl/JpaPredicateBuilder.java | 25 ++++- .../impl/QraphQLJpaBaseDataFetcher.java | 41 +++++--- .../GraphQLExecutorListCriteriaTests.java | 6 +- .../query/schema/GraphQLExecutorTests.java | 22 +++-- .../schema/StarwarsQueryExecutorTests.java | 97 +++++++++++-------- 5 files changed, 124 insertions(+), 67 deletions(-) diff --git a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/JpaPredicateBuilder.java b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/JpaPredicateBuilder.java index a680092ea..000edea47 100644 --- a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/JpaPredicateBuilder.java +++ b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/JpaPredicateBuilder.java @@ -29,7 +29,9 @@ import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.Expression; +import javax.persistence.criteria.Fetch; import javax.persistence.criteria.From; +import javax.persistence.criteria.Join; import javax.persistence.criteria.Path; import javax.persistence.criteria.Predicate; @@ -413,8 +415,27 @@ else if (type.equals(UUID.class)) { return getUuidPredicate((Path) field, predicateFilter); } else if(Collection.class.isAssignableFrom(type)) { - if(field.getModel() == null) - return from.join(filter.getField()).in(value); + // collection join for plural attributes + // TODO need better detection + if(field.getModel() == null) { + Join join = null; + + for(Fetch fetch: from.getFetches()) { + if(fetch.getAttribute().getName().equals(filter.getField())) + join = (Join) fetch; + } + + if(join == null) { + join = (Join) from.fetch(filter.getField()); + } + + Predicate in = join.in(value); + + if(filter.getCriterias().contains(PredicateFilter.Criteria.NIN)) + return cb.not(in); + + return in; + } } else if(type.isEnum()) { return getEnumPredicate((Path>) field, predicateFilter); } // TODO need better detection mechanism diff --git a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/QraphQLJpaBaseDataFetcher.java b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/QraphQLJpaBaseDataFetcher.java index 11f634987..ffadbed0c 100644 --- a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/QraphQLJpaBaseDataFetcher.java +++ b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/QraphQLJpaBaseDataFetcher.java @@ -40,6 +40,7 @@ import javax.persistence.TypedQuery; import javax.persistence.criteria.CriteriaBuilder; import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.Fetch; import javax.persistence.criteria.From; import javax.persistence.criteria.Join; import javax.persistence.criteria.JoinType; @@ -153,6 +154,7 @@ protected final List getFieldArguments(Field field, CriteriaQuery q if(!TYPENAME.equals(selectedField.getName()) && !IntrospectionUtils.isTransient(from.getJavaType(), selectedField.getName())) { Path fieldPath = from.get(selectedField.getName()); + Join join = null; // Build predicate arguments for singular attributes only if(fieldPath.getModel() instanceof SingularAttribute) { @@ -187,24 +189,39 @@ protected final List getFieldArguments(Field field, CriteriaQuery q // Let's do fugly conversion Boolean isOptional = optionalArgument.map(it -> getArgumentValue(environment, it, Boolean.class)) .orElse(attribute.isOptional()); - - reuseJoin(from, selectedField.getName(), isOptional); + + join = reuseJoin(from, selectedField.getName(), isOptional); } } } else { - // We must add plural attributes with explicit join to avoid Hibernate error: - // "query specified join fetching, but the owner of the fetched association was not present in the select list" - PluralAttribute attribute = getAttribute(selectedField.getName()); + // We must add plural attributes with explicit join + GraphQLObjectType objectType = getObjectType(environment); + EntityType entityType = getEntityType(objectType); + + PluralAttribute attribute = (PluralAttribute) entityType.getAttribute(selectedField.getName()); // Let's apply left outer join to retrieve optional many-to-many associations boolean isOptional = (PersistentAttributeType.MANY_TO_MANY == attribute.getPersistentAttributeType()); - reuseJoin(from, selectedField.getName(), isOptional); + join = reuseJoin(from, selectedField.getName(), isOptional); // Let's fetch element collections to avoid filtering their values used where search criteria - if(PersistentAttributeType.ELEMENT_COLLECTION == attribute.getPersistentAttributeType()) { - from.fetch(selectedField.getName()); - } + from.fetch(selectedField.getName(), isOptional ? JoinType.LEFT : JoinType.INNER); + + } + + // Let's build join fetch graph to avoid Hibernate error: + // "query specified join fetching, but the owner of the fetched association was not present in the select list" + if(join != null && selectedField.getSelectionSet() != null) { + GraphQLFieldDefinition fieldDefinition = getFieldDef(environment.getGraphQLSchema(), + this.getObjectType(environment), + selectedField); + Map args = environment.getArguments(); + + DataFetchingEnvironment fieldEnvironment = wherePredicateEnvironment(environment, fieldDefinition, args); + + + getFieldArguments(selectedField, query, cb, join, fieldEnvironment); } } } @@ -741,12 +758,12 @@ private Path getCompoundJoinedPath(From rootPath, String fieldName, bool // trying to find already existing joins to reuse private Join reuseJoin(From path, String fieldName, boolean outer) { - for (Join join : path.getJoins()) { + for (Fetch join : path.getFetches()) { if (join.getAttribute().getName().equals(fieldName)) { - return join; + return (Join) join; } } - return outer ? path.join(fieldName, JoinType.LEFT) : path.join(fieldName); + return outer ? (Join) path.fetch(fieldName, JoinType.LEFT) : (Join) path.fetch(fieldName); } @SuppressWarnings( { "unchecked", "rawtypes" } ) diff --git a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorListCriteriaTests.java b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorListCriteriaTests.java index 208c42208..a31bc88cc 100644 --- a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorListCriteriaTests.java +++ b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorListCriteriaTests.java @@ -90,8 +90,10 @@ public void queryWithWhereInsideOneToManyRelationsWithExplictAND() { "}"; String expected = "{Authors={select=[" - + "{id=1, name=Leo Tolstoy, books=[{id=2, title=War and Peace, genre=NOVEL}]}" - + "]}}"; + + "{id=1, name=Leo Tolstoy, books=[" + + "{id=2, title=War and Peace, genre=NOVEL}, " + + "{id=3, title=Anna Karenina, genre=NOVEL}]" + + "}]}}"; //when: Object result = executor.execute(query).getData(); diff --git a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java index 5db4eb7e0..55a6a8719 100644 --- a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java +++ b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java @@ -638,8 +638,13 @@ public void queryWithWhereInsideOneToManyRelationsImplicitAND() { " }" + "}"; - String expected = "{Authors={select=[" - + "{id=1, name=Leo Tolstoy, books=[{id=2, title=War and Peace, genre=NOVEL}]}" + String expected = "{Authors={select=[{" + + "id=1, " + + "name=Leo Tolstoy, " + + "books=[" + + "{id=2, title=War and Peace, genre=NOVEL}, " + + "{id=3, title=Anna Karenina, genre=NOVEL}" + + "]}" + "]}}"; //when: @@ -674,7 +679,9 @@ public void queryWithWhereInsideOneToManyRelationsWithExplictAND() { "}"; String expected = "{Authors={select=[" - + "{id=1, name=Leo Tolstoy, books=[{id=2, title=War and Peace, genre=NOVEL}]}" + + "{id=1, name=Leo Tolstoy, books=[" + + "{id=2, title=War and Peace, genre=NOVEL}, " + + "{id=3, title=Anna Karenina, genre=NOVEL}]}" + "]}}"; //when: @@ -830,15 +837,14 @@ public void queryWithWhereInsideManyToOneNestedRelationsWithOnToManyCollectionFi String expected = "{Books={select=[{" + "id=2, " - + "title=War and Peace, " - + "genre=NOVEL, " + + "title=War and Peace, genre=NOVEL, " + "author={" + "id=1, " + "name=Leo Tolstoy, " + "books=[" - + "{id=3, title=Anna Karenina, genre=NOVEL}" - + "]" - + "}" + + "{id=3, title=Anna Karenina, genre=NOVEL}, " + + "{id=2, title=War and Peace, genre=NOVEL}" + + "]}" + "}]}}"; //when: diff --git a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/StarwarsQueryExecutorTests.java b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/StarwarsQueryExecutorTests.java index 9c0518f53..02b519edf 100644 --- a/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/StarwarsQueryExecutorTests.java +++ b/graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/StarwarsQueryExecutorTests.java @@ -402,10 +402,10 @@ public void queryByCollectionOfEnumsAtRootLevel() { String expected = "{Humans={select=[" - + "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " - + "{name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " - + "{name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}" - + "]}}"; + + "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, " + + "{name=Han Solo, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, " + + "{name=Leia Organa, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}" + + "]}}"; //when: Object result = executor.execute(query).getData(); @@ -741,30 +741,39 @@ public void queryWithWhereInsideOneToManyRelations() { "}"; String expected = "{Humans={select=[" - + "{id=1000, name=Luke Skywalker, favoriteDroid={name=C-3PO}, friends=[" - + "{name=C-3PO, appearsIn=[A_NEW_HOPE]}, " - + "{name=Han Solo, appearsIn=[A_NEW_HOPE]}, " - + "{name=Leia Organa, appearsIn=[A_NEW_HOPE]}, " - + "{name=R2-D2, appearsIn=[A_NEW_HOPE]}" - + "]}, " - + "{id=1001, name=Darth Vader, favoriteDroid={name=R2-D2}, friends=[" - + "{name=Wilhuff Tarkin, appearsIn=[A_NEW_HOPE]}" - + "]}, " - + "{id=1002, name=Han Solo, favoriteDroid=null, friends=[" - + "{name=Leia Organa, appearsIn=[A_NEW_HOPE]}, " - + "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE]}, " - + "{name=R2-D2, appearsIn=[A_NEW_HOPE]}" - + "]}, " - + "{id=1003, name=Leia Organa, favoriteDroid=null, friends=[" - + "{name=C-3PO, appearsIn=[A_NEW_HOPE]}, " - + "{name=Han Solo, appearsIn=[A_NEW_HOPE]}, " - + "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE]}, " - + "{name=R2-D2, appearsIn=[A_NEW_HOPE]}" - + "]}, " - + "{id=1004, name=Wilhuff Tarkin, favoriteDroid=null, friends=[" - + "{name=Darth Vader, appearsIn=[A_NEW_HOPE]}" - + "]}" - + "]}}"; + + "{id=1000, name=Luke Skywalker, favoriteDroid={name=C-3PO}, " + + "friends=[" + + "{name=C-3PO, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}" + + "]" + + "}, " + + "{id=1001, name=Darth Vader, favoriteDroid={name=R2-D2}, " + + "friends=[" + + "{name=Wilhuff Tarkin, appearsIn=[A_NEW_HOPE]}" + + "]" + + "}, " + + "{id=1002, name=Han Solo, favoriteDroid=null, " + + "friends=[" + + "{name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}" + + "]" + + "}, " + + "{id=1003, name=Leia Organa, favoriteDroid=null, " + + "friends=[" + + "{name=C-3PO, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}" + + "]" + + "}, " + + "{id=1004, name=Wilhuff Tarkin, favoriteDroid=null, " + + "friends=[" + + "{name=Darth Vader, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}" + + "]" + + "}]}}"; //when: Object result = executor.execute(query).getData(); @@ -1030,9 +1039,9 @@ public void queryFilterNestedManyToManyRelationCriteria() { + "}" + "}, " + "friends=[" + + "{name=Leia Organa}, " + "{name=C-3PO}, " + "{name=Han Solo}, " - + "{name=Leia Organa}, " + "{name=R2-D2}" + "]}" + "]}}"; @@ -1067,14 +1076,16 @@ public void queryWithWhereInsideOneToManyRelationsShouldApplyFilterCriterias() { " }" + "}"; - String expected = "{Humans={select=[" - + "{id=1000, name=Luke Skywalker, favoriteDroid={name=C-3PO}, friends=[" - + "{name=C-3PO, appearsIn=[A_NEW_HOPE]}, " - + "{name=Han Solo, appearsIn=[A_NEW_HOPE]}, " - + "{name=Leia Organa, appearsIn=[A_NEW_HOPE]}, " - + "{name=R2-D2, appearsIn=[A_NEW_HOPE]}" - + "]}" - + "]}}"; + String expected = "{Humans={select=[{" + + "id=1000, name=Luke Skywalker, " + + "favoriteDroid={name=C-3PO}, " + + "friends=[" + + "{name=C-3PO, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " + + "{name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}" + + "]" + + "}]}}"; //when: Object result = executor.execute(query).getData(); @@ -1158,11 +1169,11 @@ public void queryWithNestedWhereSearchCriteriaShouldFetchElementCollectionsAttri "}"; String expected = "{Characters={select=[" - + "{id=1000, name=Luke Skywalker, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " - + "{id=1002, name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " - + "{id=1003, name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " - + "{id=2000, name=C-3PO, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, " - + "{id=2001, name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}" + + "{id=1000, name=Luke Skywalker, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, " + + "{id=1002, name=Han Solo, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, " + + "{id=1003, name=Leia Organa, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, " + + "{id=2000, name=C-3PO, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, " + + "{id=2001, name=R2-D2, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}" + "]}}"; //when: @@ -1198,7 +1209,7 @@ public void queryWithNestedWhereCompoundSearchCriteriaShouldFetchElementCollecti + "name=Luke Skywalker, " + "homePlanet=Tatooine, " + "favoriteDroid={name=C-3PO}, " - + "appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]" + + "appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]" + "}]}}"; //when: