Skip to content

Commit d452620

Browse files
igdianovigdianov
authored
fix: build fetch joins from selection graph (#137)
1 parent 764c439 commit d452620

File tree

5 files changed

+124
-67
lines changed

5 files changed

+124
-67
lines changed

‎graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/JpaPredicateBuilder.java

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929

3030
import javax.persistence.criteria.CriteriaBuilder;
3131
import javax.persistence.criteria.Expression;
32+
import javax.persistence.criteria.Fetch;
3233
import javax.persistence.criteria.From;
34+
import javax.persistence.criteria.Join;
3335
import javax.persistence.criteria.Path;
3436
import javax.persistence.criteria.Predicate;
3537

@@ -413,8 +415,27 @@ else if (type.equals(UUID.class)) {
413415
return getUuidPredicate((Path<UUID>) field, predicateFilter);
414416
}
415417
else if(Collection.class.isAssignableFrom(type)) {
416-
if(field.getModel() == null)
417-
return from.join(filter.getField()).in(value);
418+
// collection join for plural attributes
419+
// TODO need better detection
420+
if(field.getModel() == null) {
421+
Join<?,?> join = null;
422+
423+
for(Fetch<?,?> fetch: from.getFetches()) {
424+
if(fetch.getAttribute().getName().equals(filter.getField()))
425+
join = (Join<?,?>) fetch;
426+
}
427+
428+
if(join == null) {
429+
join = (Join<?,?>) from.fetch(filter.getField());
430+
}
431+
432+
Predicate in = join.in(value);
433+
434+
if(filter.getCriterias().contains(PredicateFilter.Criteria.NIN))
435+
return cb.not(in);
436+
437+
return in;
438+
}
418439
} else if(type.isEnum()) {
419440
return getEnumPredicate((Path<Enum<?>>) field, predicateFilter);
420441
} // TODO need better detection mechanism

‎graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/QraphQLJpaBaseDataFetcher.java

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import javax.persistence.TypedQuery;
4141
import javax.persistence.criteria.CriteriaBuilder;
4242
import javax.persistence.criteria.CriteriaQuery;
43+
import javax.persistence.criteria.Fetch;
4344
import javax.persistence.criteria.From;
4445
import javax.persistence.criteria.Join;
4546
import javax.persistence.criteria.JoinType;
@@ -153,6 +154,7 @@ protected final List<Argument> getFieldArguments(Field field, CriteriaQuery<?> q
153154
if(!TYPENAME.equals(selectedField.getName()) && !IntrospectionUtils.isTransient(from.getJavaType(), selectedField.getName())) {
154155

155156
Path<?> fieldPath = from.get(selectedField.getName());
157+
Join<?,?> join = null;
156158

157159
// Build predicate arguments for singular attributes only
158160
if(fieldPath.getModel() instanceof SingularAttribute) {
@@ -187,24 +189,39 @@ protected final List<Argument> getFieldArguments(Field field, CriteriaQuery<?> q
187189
// Let's do fugly conversion
188190
Boolean isOptional = optionalArgument.map(it -> getArgumentValue(environment, it, Boolean.class))
189191
.orElse(attribute.isOptional());
190-
191-
reuseJoin(from, selectedField.getName(), isOptional);
192+
193+
join = reuseJoin(from, selectedField.getName(), isOptional);
192194
}
193195
}
194196
} else {
195-
// We must add plural attributes with explicit join to avoid Hibernate error:
196-
// "query specified join fetching, but the owner of the fetched association was not present in the select list"
197-
PluralAttribute<?, ?, ?> attribute = getAttribute(selectedField.getName());
197+
// We must add plural attributes with explicit join
198+
GraphQLObjectType objectType = getObjectType(environment);
199+
EntityType<?> entityType = getEntityType(objectType);
200+
201+
PluralAttribute<?, ?, ?> attribute = (PluralAttribute<?, ?, ?>) entityType.getAttribute(selectedField.getName());
198202

199203
// Let's apply left outer join to retrieve optional many-to-many associations
200204
boolean isOptional = (PersistentAttributeType.MANY_TO_MANY == attribute.getPersistentAttributeType());
201205

202-
reuseJoin(from, selectedField.getName(), isOptional);
206+
join = reuseJoin(from, selectedField.getName(), isOptional);
203207

204208
// Let's fetch element collections to avoid filtering their values used where search criteria
205-
if(PersistentAttributeType.ELEMENT_COLLECTION == attribute.getPersistentAttributeType()) {
206-
from.fetch(selectedField.getName());
207-
}
209+
from.fetch(selectedField.getName(), isOptional ? JoinType.LEFT : JoinType.INNER);
210+
211+
}
212+
213+
// Let's build join fetch graph to avoid Hibernate error:
214+
// "query specified join fetching, but the owner of the fetched association was not present in the select list"
215+
if(join != null && selectedField.getSelectionSet() != null) {
216+
GraphQLFieldDefinition fieldDefinition = getFieldDef(environment.getGraphQLSchema(),
217+
this.getObjectType(environment),
218+
selectedField);
219+
Map<String, Object> args = environment.getArguments();
220+
221+
DataFetchingEnvironment fieldEnvironment = wherePredicateEnvironment(environment, fieldDefinition, args);
222+
223+
224+
getFieldArguments(selectedField, query, cb, join, fieldEnvironment);
208225
}
209226
}
210227
}
@@ -741,12 +758,12 @@ private Path<?> getCompoundJoinedPath(From<?,?> rootPath, String fieldName, bool
741758
// trying to find already existing joins to reuse
742759
private Join<?,?> reuseJoin(From<?, ?> path, String fieldName, boolean outer) {
743760

744-
for (Join<?,?> join : path.getJoins()) {
761+
for (Fetch<?,?> join : path.getFetches()) {
745762
if (join.getAttribute().getName().equals(fieldName)) {
746-
return join;
763+
return (Join<?,?>) join;
747764
}
748765
}
749-
return outer ? path.join(fieldName, JoinType.LEFT) : path.join(fieldName);
766+
return outer ? (Join<?,?>) path.fetch(fieldName, JoinType.LEFT) : (Join<?,?>) path.fetch(fieldName);
750767
}
751768

752769
@SuppressWarnings( { "unchecked", "rawtypes" } )

‎graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorListCriteriaTests.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,10 @@ public void queryWithWhereInsideOneToManyRelationsWithExplictAND() {
9090
"}";
9191

9292
String expected = "{Authors={select=["
93-
+ "{id=1, name=Leo Tolstoy, books=[{id=2, title=War and Peace, genre=NOVEL}]}"
94-
+ "]}}";
93+
+ "{id=1, name=Leo Tolstoy, books=["
94+
+ "{id=2, title=War and Peace, genre=NOVEL}, "
95+
+ "{id=3, title=Anna Karenina, genre=NOVEL}]"
96+
+ "}]}}";
9597

9698
//when:
9799
Object result = executor.execute(query).getData();

‎graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -638,8 +638,13 @@ public void queryWithWhereInsideOneToManyRelationsImplicitAND() {
638638
" }" +
639639
"}";
640640

641-
String expected = "{Authors={select=["
642-
+ "{id=1, name=Leo Tolstoy, books=[{id=2, title=War and Peace, genre=NOVEL}]}"
641+
String expected = "{Authors={select=[{"
642+
+ "id=1, "
643+
+ "name=Leo Tolstoy, "
644+
+ "books=["
645+
+ "{id=2, title=War and Peace, genre=NOVEL}, "
646+
+ "{id=3, title=Anna Karenina, genre=NOVEL}"
647+
+ "]}"
643648
+ "]}}";
644649

645650
//when:
@@ -674,7 +679,9 @@ public void queryWithWhereInsideOneToManyRelationsWithExplictAND() {
674679
"}";
675680

676681
String expected = "{Authors={select=["
677-
+ "{id=1, name=Leo Tolstoy, books=[{id=2, title=War and Peace, genre=NOVEL}]}"
682+
+ "{id=1, name=Leo Tolstoy, books=["
683+
+ "{id=2, title=War and Peace, genre=NOVEL}, "
684+
+ "{id=3, title=Anna Karenina, genre=NOVEL}]}"
678685
+ "]}}";
679686

680687
//when:
@@ -830,15 +837,14 @@ public void queryWithWhereInsideManyToOneNestedRelationsWithOnToManyCollectionFi
830837

831838
String expected = "{Books={select=[{"
832839
+ "id=2, "
833-
+ "title=War and Peace, "
834-
+ "genre=NOVEL, "
840+
+ "title=War and Peace, genre=NOVEL, "
835841
+ "author={"
836842
+ "id=1, "
837843
+ "name=Leo Tolstoy, "
838844
+ "books=["
839-
+ "{id=3, title=Anna Karenina, genre=NOVEL}"
840-
+ "]"
841-
+ "}"
845+
+ "{id=3, title=Anna Karenina, genre=NOVEL}, "
846+
+ "{id=2, title=War and Peace, genre=NOVEL}"
847+
+ "]}"
842848
+ "}]}}";
843849

844850
//when:

‎graphql-jpa-query-schema/src/test/java/com/introproventures/graphql/jpa/query/schema/StarwarsQueryExecutorTests.java

Lines changed: 54 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -402,10 +402,10 @@ public void queryByCollectionOfEnumsAtRootLevel() {
402402

403403

404404
String expected = "{Humans={select=["
405-
+ "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
406-
+ "{name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
407-
+ "{name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}"
408-
+ "]}}";
405+
+ "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, "
406+
+ "{name=Han Solo, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, "
407+
+ "{name=Leia Organa, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}"
408+
+ "]}}";
409409

410410
//when:
411411
Object result = executor.execute(query).getData();
@@ -741,30 +741,39 @@ public void queryWithWhereInsideOneToManyRelations() {
741741
"}";
742742

743743
String expected = "{Humans={select=["
744-
+ "{id=1000, name=Luke Skywalker, favoriteDroid={name=C-3PO}, friends=["
745-
+ "{name=C-3PO, appearsIn=[A_NEW_HOPE]}, "
746-
+ "{name=Han Solo, appearsIn=[A_NEW_HOPE]}, "
747-
+ "{name=Leia Organa, appearsIn=[A_NEW_HOPE]}, "
748-
+ "{name=R2-D2, appearsIn=[A_NEW_HOPE]}"
749-
+ "]}, "
750-
+ "{id=1001, name=Darth Vader, favoriteDroid={name=R2-D2}, friends=["
751-
+ "{name=Wilhuff Tarkin, appearsIn=[A_NEW_HOPE]}"
752-
+ "]}, "
753-
+ "{id=1002, name=Han Solo, favoriteDroid=null, friends=["
754-
+ "{name=Leia Organa, appearsIn=[A_NEW_HOPE]}, "
755-
+ "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE]}, "
756-
+ "{name=R2-D2, appearsIn=[A_NEW_HOPE]}"
757-
+ "]}, "
758-
+ "{id=1003, name=Leia Organa, favoriteDroid=null, friends=["
759-
+ "{name=C-3PO, appearsIn=[A_NEW_HOPE]}, "
760-
+ "{name=Han Solo, appearsIn=[A_NEW_HOPE]}, "
761-
+ "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE]}, "
762-
+ "{name=R2-D2, appearsIn=[A_NEW_HOPE]}"
763-
+ "]}, "
764-
+ "{id=1004, name=Wilhuff Tarkin, favoriteDroid=null, friends=["
765-
+ "{name=Darth Vader, appearsIn=[A_NEW_HOPE]}"
766-
+ "]}"
767-
+ "]}}";
744+
+ "{id=1000, name=Luke Skywalker, favoriteDroid={name=C-3PO}, "
745+
+ "friends=["
746+
+ "{name=C-3PO, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
747+
+ "{name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
748+
+ "{name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
749+
+ "{name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}"
750+
+ "]"
751+
+ "}, "
752+
+ "{id=1001, name=Darth Vader, favoriteDroid={name=R2-D2}, "
753+
+ "friends=["
754+
+ "{name=Wilhuff Tarkin, appearsIn=[A_NEW_HOPE]}"
755+
+ "]"
756+
+ "}, "
757+
+ "{id=1002, name=Han Solo, favoriteDroid=null, "
758+
+ "friends=["
759+
+ "{name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
760+
+ "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
761+
+ "{name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}"
762+
+ "]"
763+
+ "}, "
764+
+ "{id=1003, name=Leia Organa, favoriteDroid=null, "
765+
+ "friends=["
766+
+ "{name=C-3PO, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
767+
+ "{name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
768+
+ "{name=Luke Skywalker, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
769+
+ "{name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}"
770+
+ "]"
771+
+ "}, "
772+
+ "{id=1004, name=Wilhuff Tarkin, favoriteDroid=null, "
773+
+ "friends=["
774+
+ "{name=Darth Vader, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}"
775+
+ "]"
776+
+ "}]}}";
768777

769778
//when:
770779
Object result = executor.execute(query).getData();
@@ -1030,9 +1039,9 @@ public void queryFilterNestedManyToManyRelationCriteria() {
10301039
+ "}"
10311040
+ "}, "
10321041
+ "friends=["
1042+
+ "{name=Leia Organa}, "
10331043
+ "{name=C-3PO}, "
10341044
+ "{name=Han Solo}, "
1035-
+ "{name=Leia Organa}, "
10361045
+ "{name=R2-D2}"
10371046
+ "]}"
10381047
+ "]}}";
@@ -1067,14 +1076,16 @@ public void queryWithWhereInsideOneToManyRelationsShouldApplyFilterCriterias() {
10671076
" }" +
10681077
"}";
10691078

1070-
String expected = "{Humans={select=["
1071-
+ "{id=1000, name=Luke Skywalker, favoriteDroid={name=C-3PO}, friends=["
1072-
+ "{name=C-3PO, appearsIn=[A_NEW_HOPE]}, "
1073-
+ "{name=Han Solo, appearsIn=[A_NEW_HOPE]}, "
1074-
+ "{name=Leia Organa, appearsIn=[A_NEW_HOPE]}, "
1075-
+ "{name=R2-D2, appearsIn=[A_NEW_HOPE]}"
1076-
+ "]}"
1077-
+ "]}}";
1079+
String expected = "{Humans={select=[{"
1080+
+ "id=1000, name=Luke Skywalker, "
1081+
+ "favoriteDroid={name=C-3PO}, "
1082+
+ "friends=["
1083+
+ "{name=C-3PO, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
1084+
+ "{name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
1085+
+ "{name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
1086+
+ "{name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}"
1087+
+ "]"
1088+
+ "}]}}";
10781089

10791090
//when:
10801091
Object result = executor.execute(query).getData();
@@ -1158,11 +1169,11 @@ public void queryWithNestedWhereSearchCriteriaShouldFetchElementCollectionsAttri
11581169
"}";
11591170

11601171
String expected = "{Characters={select=["
1161-
+ "{id=1000, name=Luke Skywalker, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
1162-
+ "{id=1002, name=Han Solo, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
1163-
+ "{id=1003, name=Leia Organa, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
1164-
+ "{id=2000, name=C-3PO, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}, "
1165-
+ "{id=2001, name=R2-D2, appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]}"
1172+
+ "{id=1000, name=Luke Skywalker, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, "
1173+
+ "{id=1002, name=Han Solo, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, "
1174+
+ "{id=1003, name=Leia Organa, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, "
1175+
+ "{id=2000, name=C-3PO, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}, "
1176+
+ "{id=2001, name=R2-D2, appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]}"
11661177
+ "]}}";
11671178

11681179
//when:
@@ -1198,7 +1209,7 @@ public void queryWithNestedWhereCompoundSearchCriteriaShouldFetchElementCollecti
11981209
+ "name=Luke Skywalker, "
11991210
+ "homePlanet=Tatooine, "
12001211
+ "favoriteDroid={name=C-3PO}, "
1201-
+ "appearsIn=[A_NEW_HOPE, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI, THE_FORCE_AWAKENS]"
1212+
+ "appearsIn=[A_NEW_HOPE, THE_FORCE_AWAKENS, EMPIRE_STRIKES_BACK, RETURN_OF_THE_JEDI]"
12021213
+ "}]}}";
12031214

12041215
//when:

0 commit comments

Comments
 (0)