Skip to content

fix: build join fetch from selection graph #137

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
May 6, 2019
Merged

fix: build join fetch from selection graph #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 23 additions & 2 deletions ...src/main/java/com/introproventures/graphql/jpa/query/schema/impl/JpaPredicateBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -413,8 +415,27 @@ else if (type.equals(UUID.class)) {
return getUuidPredicate((Path<UUID>) 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<Enum<?>>) field, predicateFilter);
} // TODO need better detection mechanism
Expand Down
41 changes: 29 additions & 12 deletions ...in/java/com/introproventures/graphql/jpa/query/schema/impl/QraphQLJpaBaseDataFetcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -153,6 +154,7 @@ protected final List<Argument> 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) {
Expand Down Expand Up @@ -187,24 +189,39 @@ protected final List<Argument> 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<String, Object> args = environment.getArguments();

DataFetchingEnvironment fieldEnvironment = wherePredicateEnvironment(environment, fieldDefinition, args);


getFieldArguments(selectedField, query, cb, join, fieldEnvironment);
}
}
}
Expand Down Expand Up @@ -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" } )
Expand Down
6 changes: 4 additions & 2 deletions .../java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorListCriteriaTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
22 changes: 14 additions & 8 deletions ...ema/src/test/java/com/introproventures/graphql/jpa/query/schema/GraphQLExecutorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down
97 changes: 54 additions & 43 deletions ...c/test/java/com/introproventures/graphql/jpa/query/schema/StarwarsQueryExecutorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -1030,9 +1039,9 @@ public void queryFilterNestedManyToManyRelationCriteria() {
+ "}"
+ "}, "
+ "friends=["
+ "{name=Leia Organa}, "
+ "{name=C-3PO}, "
+ "{name=Han Solo}, "
+ "{name=Leia Organa}, "
+ "{name=R2-D2}"
+ "]}"
+ "]}}";
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down