From bd4427a8bf4c8957c38ca1f93b0b16eba05d5952 Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Sun, 7 Apr 2019 16:11:02 -0700 Subject: [PATCH 1/8] fix: Apply left outer join to retrieve optional associations --- .../impl/QraphQLJpaBaseDataFetcher.java | 10 +- .../schema/StarwarsQueryExecutorTests.java | 125 +++++++++++++++++- 2 files changed, 128 insertions(+), 7 deletions(-) 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 d90257965..6a830c23e 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 @@ -177,14 +177,15 @@ protected final List getFieldArguments(Field field, CriteriaQuery q if (attribute.getPersistentAttributeType() == Attribute.PersistentAttributeType.MANY_TO_ONE || attribute.getPersistentAttributeType() == Attribute.PersistentAttributeType.ONE_TO_ONE ) { - reuseJoin(from, selectedField.getName(), false); + // Apply left outer join to retrieve optional associations + reuseJoin(from, selectedField.getName(), true); } } } else { // We must add plural attributes with explicit fetch to avoid Hibernate error: // "query specified join fetching, but the owner of the fetched association was not present in the select list" - // TODO Let's try detect optional relation and apply join type - reuseJoin(from, selectedField.getName(), false); + // Apply left outer join to retrieve optional associations + reuseJoin(from, selectedField.getName(), true); } } } @@ -259,7 +260,8 @@ protected Predicate getPredicate(CriteriaBuilder cb, Root from, From pat // If the argument is a list, let's assume we need to join and do an 'in' clause if (argumentEntityAttribute instanceof PluralAttribute) { - return reuseJoin(from, argument.getName(), false) + // Apply left outer join to retrieve optional associations + return reuseJoin(from, argument.getName(), true) .in(convertValue(environment, argument, argument.getValue())); } 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 bb899a275..1f2796a98 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 @@ -740,8 +740,29 @@ 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=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]}" + + "]}" + "]}}"; //when: @@ -749,7 +770,7 @@ public void queryWithWhereInsideOneToManyRelations() { //then: assertThat(result.toString()).isEqualTo(expected); - } + } @Test @@ -935,4 +956,102 @@ public void queryFilterNestedManyToManyRelationCriteria() { } + @Test + public void queryWithWhereInsideOneToManyRelationsShouldApplyFilterCriterias() { + //given: + String query = "query { " + + " Humans(where: {" + + "friends: {appearsIn: {IN: A_NEW_HOPE}} " + + "favoriteDroid: {name: {EQ: \"C-3PO\"}} " + + "}) {" + + " select {" + + " id" + + " name" + + " favoriteDroid {" + + " name" + + " }" + + " friends {" + + " name" + + " appearsIn" + + " }" + + " }" + + " }" + + "}"; + + 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]}" + + "]}" + + "]}}"; + + //when: + Object result = executor.execute(query).getData(); + + //then: + assertThat(result.toString()).isEqualTo(expected); + } + + @Test + public void queryWithOneToManyRelationsShouldUseLeftOuterJoin() { + //given: + String query = "query { " + + " Humans {" + + " select {" + + " id" + + " name" + + " homePlanet" + + " favoriteDroid {" + + " name" + + " }" + + " }" + + " }" + + "}"; + + String expected = "{Humans={select=[" + + "{id=1000, name=Luke Skywalker, homePlanet=Tatooine, favoriteDroid={name=C-3PO}}, " + + "{id=1001, name=Darth Vader, homePlanet=Tatooine, favoriteDroid={name=R2-D2}}, " + + "{id=1002, name=Han Solo, homePlanet=null, favoriteDroid=null}, " + + "{id=1003, name=Leia Organa, homePlanet=Alderaan, favoriteDroid=null}, " + + "{id=1004, name=Wilhuff Tarkin, homePlanet=null, favoriteDroid=null}" + + "]}}"; + + //when: + Object result = executor.execute(query).getData(); + + //then: + assertThat(result.toString()).isEqualTo(expected); + } + + + @Test + public void queryWithWhereOneToManyRelationsShouldUseLeftOuterJoinAndApplyCriteria() { + //given: + String query = "query { " + + " Humans(where: {favoriteDroid: {name: {EQ: \"C-3PO\"}}}) {" + + " select {" + + " id" + + " name" + + " homePlanet" + + " favoriteDroid {" + + " name" + + " }" + + " }" + + " }" + + "}"; + + String expected = "{Humans={select=[" + + "{id=1000, name=Luke Skywalker, homePlanet=Tatooine, favoriteDroid={name=C-3PO}}" + + "]}}"; + + //when: + Object result = executor.execute(query).getData(); + + //then: + assertThat(result.toString()).isEqualTo(expected); + } + + } From a9c6208600f25da1cae7971ac93ca2c2c71dc5f9 Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Fri, 26 Apr 2019 20:26:19 -0700 Subject: [PATCH 2/8] chore: polish GraphQLJPASchemaBuilder --- .../schema/impl/GraphQLJpaSchemaBuilder.java | 61 +++++++++++-------- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java index 70352052f..31d7a6c3e 100644 --- a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java +++ b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java @@ -36,7 +36,13 @@ import javax.persistence.Convert; import javax.persistence.EntityManager; import javax.persistence.Transient; -import javax.persistence.metamodel.*; +import javax.persistence.metamodel.Attribute; +import javax.persistence.metamodel.EmbeddableType; +import javax.persistence.metamodel.EntityType; +import javax.persistence.metamodel.ManagedType; +import javax.persistence.metamodel.PluralAttribute; +import javax.persistence.metamodel.SingularAttribute; +import javax.persistence.metamodel.Type; import com.introproventures.graphql.jpa.query.annotation.GraphQLDescription; import com.introproventures.graphql.jpa.query.annotation.GraphQLIgnore; @@ -221,20 +227,12 @@ private GraphQLArgument distinctArgument(EntityType entityType) { } private GraphQLArgument getWhereArgument(ManagedType managedType) { - String typeName=""; - if (managedType instanceof EmbeddableType){ - typeName = managedType.getJavaType().getSimpleName()+"EmbeddableType"; - } else if (managedType instanceof EntityType) { - typeName = ((EntityType)managedType).getName(); - } - - String type = namingStrategy.pluralize(typeName)+"CriteriaExpression"; - - GraphQLArgument whereArgument = whereArgumentsMap.get(managedType.getJavaType()); - - if(whereArgument != null) - return whereArgument; - + return whereArgumentsMap.computeIfAbsent(managedType.getJavaType(), (javaType) -> computeWhereArgument(managedType)); + } + + private GraphQLArgument computeWhereArgument(ManagedType managedType) { + String type=resolveWhereArgumentTypeName(managedType); + GraphQLInputObjectType whereInputObject = GraphQLInputObjectType.newInputObject() .name(type) .description("Where logical AND specification of the provided list of criteria expressions") @@ -266,29 +264,38 @@ private GraphQLArgument getWhereArgument(ManagedType managedType) { ) .build(); - whereArgument = GraphQLArgument.newArgument() - .name(QUERY_WHERE_PARAM_NAME) - .description("Where logical specification") - .type(whereInputObject) - .build(); - - whereArgumentsMap.put(managedType.getJavaType(), whereArgument); - - return whereArgument; + return GraphQLArgument.newArgument() + .name(QUERY_WHERE_PARAM_NAME) + .description("Where logical specification") + .type(whereInputObject) + .build(); } - private GraphQLInputObjectType getWhereInputType(ManagedType managedType) { - return inputObjectCache.computeIfAbsent(managedType, this::computeWhereInputType); + private String resolveWhereArgumentTypeName(ManagedType managedType) { + String typeName=resolveTypeName(managedType); + + return namingStrategy.pluralize(typeName)+"CriteriaExpression"; } - private String resolveWhereInputTypeName(ManagedType managedType) { + private String resolveTypeName(ManagedType managedType) { String typeName=""; + if (managedType instanceof EmbeddableType){ typeName = managedType.getJavaType().getSimpleName()+"EmbeddableType"; } else if (managedType instanceof EntityType) { typeName = ((EntityType)managedType).getName(); } + + return typeName; + } + + private GraphQLInputObjectType getWhereInputType(ManagedType managedType) { + return inputObjectCache.computeIfAbsent(managedType, this::computeWhereInputType); + } + + private String resolveWhereInputTypeName(ManagedType managedType) { + String typeName=resolveTypeName(managedType); return namingStrategy.pluralize(typeName)+"RelationCriteriaExpression"; From 52cb95ca450ddc6018fef530d31cfb56adf16ca6 Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Fri, 26 Apr 2019 22:41:25 -0700 Subject: [PATCH 3/8] chore: clean DriodFunction orphan --- .../query/example/starwars/DroidFunction.java | 27 ------------------- 1 file changed, 27 deletions(-) delete mode 100644 graphql-jpa-query-example-merge/src/main/java/com/introproventures/graphql/jpa/query/example/starwars/DroidFunction.java diff --git a/graphql-jpa-query-example-merge/src/main/java/com/introproventures/graphql/jpa/query/example/starwars/DroidFunction.java b/graphql-jpa-query-example-merge/src/main/java/com/introproventures/graphql/jpa/query/example/starwars/DroidFunction.java deleted file mode 100644 index 88512879d..000000000 --- a/graphql-jpa-query-example-merge/src/main/java/com/introproventures/graphql/jpa/query/example/starwars/DroidFunction.java +++ /dev/null @@ -1,27 +0,0 @@ -package com.introproventures.graphql.jpa.query.example.starwars; - -import javax.persistence.Entity; -import javax.persistence.Id; -import javax.persistence.Table; - -import com.introproventures.graphql.jpa.query.annotation.GraphQLDescription; -import lombok.Data; -import lombok.EqualsAndHashCode; - - -@Entity(name = "droidFunction") -@Table(name = "droid_function") -@GraphQLDescription("Represents the functions a droid can have") -@Data -@EqualsAndHashCode() -public class DroidFunction { - - @Id - @GraphQLDescription("Primary Key for the DroidFunction Class") - String id; - - String function; - - - -} From c7754eb10b1d77012a3aa48bc421704ae13d88a1 Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Fri, 26 Apr 2019 22:42:27 -0700 Subject: [PATCH 4/8] fix: change primaryFunction association type to @OneToOne --- .../graphql/jpa/query/schema/model/starwars/Droid.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/graphql-jpa-query-example-model-starwars/src/main/java/com/introproventures/graphql/jpa/query/schema/model/starwars/Droid.java b/graphql-jpa-query-example-model-starwars/src/main/java/com/introproventures/graphql/jpa/query/schema/model/starwars/Droid.java index 12404e0c7..d4cc2f27f 100644 --- a/graphql-jpa-query-example-model-starwars/src/main/java/com/introproventures/graphql/jpa/query/schema/model/starwars/Droid.java +++ b/graphql-jpa-query-example-model-starwars/src/main/java/com/introproventures/graphql/jpa/query/schema/model/starwars/Droid.java @@ -19,10 +19,9 @@ import javax.persistence.Entity; import javax.persistence.FetchType; import javax.persistence.JoinColumn; -import javax.persistence.ManyToOne; +import javax.persistence.OneToOne; import com.introproventures.graphql.jpa.query.annotation.GraphQLDescription; - import lombok.Data; import lombok.EqualsAndHashCode; @@ -32,7 +31,7 @@ @EqualsAndHashCode(callSuper=true) public class Droid extends Character { - @ManyToOne(fetch = FetchType.LAZY) + @OneToOne(fetch = FetchType.LAZY) @JoinColumn(name = "primary_function") DroidFunction primaryFunction; From a6761b461c84c7638e72249480de9a3f0852ee74 Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Fri, 26 Apr 2019 22:43:30 -0700 Subject: [PATCH 5/8] feat: first crack at optional argument join handling --- .../schema/impl/GraphQLJpaSchemaBuilder.java | 13 ++++- .../impl/QraphQLJpaBaseDataFetcher.java | 52 ++++++++++++++++--- 2 files changed, 56 insertions(+), 9 deletions(-) diff --git a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java index 31d7a6c3e..a0b082f82 100644 --- a/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java +++ b/graphql-jpa-query-schema/src/main/java/com/introproventures/graphql/jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java @@ -640,6 +640,8 @@ && isNotIgnoredOrder(attribute) ) { // TODO fix page count query arguments.add(getWhereArgument(foreignType)); + + arguments.add(optionalArgument(SingularAttribute.class.cast(attribute))); } // Get Sub-Objects fields queries via DataFetcher else if (attribute instanceof PluralAttribute @@ -651,7 +653,7 @@ else if (attribute instanceof PluralAttribute arguments.add(getWhereArgument(elementType)); dataFetcher = new GraphQLJpaOneToManyDataFetcher(entityManager, baseEntity, (PluralAttribute) attribute); } - + return GraphQLFieldDefinition.newFieldDefinition() .name(attribute.getName()) .description(getSchemaDescription(attribute.getJavaMember())) @@ -660,6 +662,15 @@ else if (attribute instanceof PluralAttribute .argument(arguments) .build(); } + + private GraphQLArgument optionalArgument(SingularAttribute attribute) { + return GraphQLArgument.newArgument() + .name("optional") + .description("Optional association specification") + .type(Scalars.GraphQLBoolean) + .defaultValue(attribute.isOptional()) + .build(); + } protected ManagedType getForeignType(Attribute attribute) { if(SingularAttribute.class.isInstance(attribute)) 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 6a830c23e..f81afc2f0 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 @@ -167,7 +167,7 @@ protected final List getFieldArguments(Field field, CriteriaQuery q // Process where arguments clauses. arguments.addAll(selectedField.getArguments() .stream() - .filter(it -> !isOrderByArgument(it)) + .filter(it -> !isOrderByArgument(it) && !isOptional(it)) .map(it -> new Argument(selectedField.getName() + "." + it.getName(), it.getValue())) .collect(Collectors.toList())); @@ -178,14 +178,34 @@ protected final List getFieldArguments(Field field, CriteriaQuery q || attribute.getPersistentAttributeType() == Attribute.PersistentAttributeType.ONE_TO_ONE ) { // Apply left outer join to retrieve optional associations - reuseJoin(from, selectedField.getName(), true); + Optional optional = selectedField.getArguments() + .stream() + .filter(it -> isOptional(it)) + .findFirst(); + + boolean isOptional = optional.map(it -> it.getValue()) + .map(BooleanValue.class::cast) + .map(BooleanValue::isValue) + .orElseGet(() -> attribute.isOptional()); + + reuseJoin(from, selectedField.getName(), isOptional); } } } else { // We must add plural attributes with explicit fetch to avoid Hibernate error: // "query specified join fetching, but the owner of the fetched association was not present in the select list" - // Apply left outer join to retrieve optional associations - reuseJoin(from, selectedField.getName(), true); + + // Apply left outer join to retrieve optional many-to-many associations + PluralAttribute attribute = entityType.getPluralAttributes() + .stream() + .filter(it -> it.getName() + .equals(selectedField.getName())) + .findFirst() + .get(); + + boolean isOptional = attribute.getPersistentAttributeType().equals(PersistentAttributeType.MANY_TO_MANY) ? true : false; + + reuseJoin(from, selectedField.getName(), isOptional); } } } @@ -252,6 +272,10 @@ protected boolean isOrderByArgument(Argument argument) { return GraphQLJpaSchemaBuilder.ORDER_BY_PARAM_NAME.equals(argument.getName()); } + protected boolean isOptional(Argument argument) { + return "optional".equals(argument.getName()); + } + @SuppressWarnings( "unchecked" ) protected Predicate getPredicate(CriteriaBuilder cb, Root from, From path, DataFetchingEnvironment environment, Argument argument) { @@ -396,7 +420,13 @@ private Predicate getFieldPredicate(String fieldName, CriteriaBuilder cb, From !Logical.names().contains(it.getName())) + .map(it -> getAttribute(environment, argument)) + .map(this::isOptional) + .orElse(false); + + return getArgumentPredicate(cb, reuseJoin(path, fieldName, isOptional), wherePredicateEnvironment(environment, fieldDefinition, arguments), new Argument(logical.name(), expressionValue)); } @@ -521,9 +551,7 @@ private Join reuseJoin(From path, String fieldName, boolean outer) { for (Join join : path.getJoins()) { if (join.getAttribute().getName().equals(fieldName)) { - if ((join.getJoinType() == JoinType.LEFT) == outer) { - return join; - } + return join; } } return outer ? path.join(fieldName, JoinType.LEFT) : path.join(fieldName); @@ -631,6 +659,14 @@ private Attribute getAttribute(DataFetchingEnvironment environment, Argumen return entityType.getAttribute(argument.getName()); } + private boolean isOptional(Attribute attribute) { + if(SingularAttribute.class.isInstance(attribute)) { + return SingularAttribute.class.cast(attribute).isOptional(); + } + + return false; + } + /** * Resolve JPA model entity type from GraphQL objectType * From 62be381e1ed11c15e9cbb2f52792b372704762bc Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Fri, 26 Apr 2019 22:44:08 -0700 Subject: [PATCH 6/8] fix: set author association type to optional=false --- .../graphql/jpa/query/schema/model/book/Book.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/graphql-jpa-query-example-model-books/src/main/java/com/introproventures/graphql/jpa/query/schema/model/book/Book.java b/graphql-jpa-query-example-model-books/src/main/java/com/introproventures/graphql/jpa/query/schema/model/book/Book.java index 56aa31ec4..efb530e1d 100644 --- a/graphql-jpa-query-example-model-books/src/main/java/com/introproventures/graphql/jpa/query/schema/model/book/Book.java +++ b/graphql-jpa-query-example-model-books/src/main/java/com/introproventures/graphql/jpa/query/schema/model/book/Book.java @@ -43,7 +43,7 @@ public class Book { @GraphQLIgnoreFilter String description; - @ManyToOne(fetch=FetchType.LAZY) + @ManyToOne(fetch=FetchType.LAZY, optional = false) Author author; @Enumerated(EnumType.STRING) From 280a1b75941dee5e7ec91a86ceee5dea4ac2b2e8 Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Sun, 28 Apr 2019 14:54:29 -0700 Subject: [PATCH 7/8] fix: add optional unit tests --- .../impl/QraphQLJpaBaseDataFetcher.java | 105 +++++++++++------- .../schema/StarwarsQueryExecutorTests.java | 89 +++++++++++++++ 2 files changed, 155 insertions(+), 39 deletions(-) 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 f81afc2f0..96d350e0c 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 @@ -90,6 +90,8 @@ */ class QraphQLJpaBaseDataFetcher implements DataFetcher { + private static final String OPTIONAL = "optional"; + // "__typename" is part of the graphql introspection spec and has to be ignored private static final String TYPENAME = "__typename"; @@ -167,45 +169,36 @@ protected final List getFieldArguments(Field field, CriteriaQuery q // Process where arguments clauses. arguments.addAll(selectedField.getArguments() .stream() - .filter(it -> !isOrderByArgument(it) && !isOptional(it)) + .filter(it -> !isOrderByArgument(it) && !isOptionalArgument(it)) .map(it -> new Argument(selectedField.getName() + "." + it.getName(), it.getValue())) .collect(Collectors.toList())); - // Check if it's an object and the foreign side is One. Then we can eagerly fetch causing an inner join instead of 2 queries + // Check if it's an object and the foreign side is One. Then we can eagerly join causing an inner join instead of 2 queries if (fieldPath.getModel() instanceof SingularAttribute) { SingularAttribute attribute = (SingularAttribute) fieldPath.getModel(); if (attribute.getPersistentAttributeType() == Attribute.PersistentAttributeType.MANY_TO_ONE || attribute.getPersistentAttributeType() == Attribute.PersistentAttributeType.ONE_TO_ONE ) { - // Apply left outer join to retrieve optional associations - Optional optional = selectedField.getArguments() - .stream() - .filter(it -> isOptional(it)) - .findFirst(); - - boolean isOptional = optional.map(it -> it.getValue()) - .map(BooleanValue.class::cast) - .map(BooleanValue::isValue) - .orElseGet(() -> attribute.isOptional()); + // Let's apply left outer join to retrieve optional associations + Optional optionalArgument = getArgument(selectedField, OPTIONAL); + + // Let's do fugly conversion + Boolean isOptional = optionalArgument.map(it -> getArgumentValue(environment, it, Boolean.class)) + .orElse(attribute.isOptional()); reuseJoin(from, selectedField.getName(), isOptional); } } - } else { - // We must add plural attributes with explicit fetch to avoid Hibernate error: + } 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" - - // Apply left outer join to retrieve optional many-to-many associations - PluralAttribute attribute = entityType.getPluralAttributes() - .stream() - .filter(it -> it.getName() - .equals(selectedField.getName())) - .findFirst() - .get(); + PluralAttribute attribute = getAttribute(selectedField.getName()); - boolean isOptional = attribute.getPersistentAttributeType().equals(PersistentAttributeType.MANY_TO_MANY) ? true : false; + // 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); + } } } } @@ -272,10 +265,22 @@ protected boolean isOrderByArgument(Argument argument) { return GraphQLJpaSchemaBuilder.ORDER_BY_PARAM_NAME.equals(argument.getName()); } - protected boolean isOptional(Argument argument) { - return "optional".equals(argument.getName()); + protected boolean isOptionalArgument(Argument argument) { + return OPTIONAL.equals(argument.getName()); } + protected Optional getArgument(Field selectedField, String argumentName) { + return selectedField.getArguments() + .stream() + .filter(it -> it.getName() + .equals(argumentName)) + .findFirst(); + } + + protected > R getAttribute(String attributeName) { + return (R) entityType.getAttribute(attributeName); + } + @SuppressWarnings( "unchecked" ) protected Predicate getPredicate(CriteriaBuilder cb, Root from, From path, DataFetchingEnvironment environment, Argument argument) { @@ -285,7 +290,7 @@ protected Predicate getPredicate(CriteriaBuilder cb, Root from, From pat // If the argument is a list, let's assume we need to join and do an 'in' clause if (argumentEntityAttribute instanceof PluralAttribute) { // Apply left outer join to retrieve optional associations - return reuseJoin(from, argument.getName(), true) + return reuseJoin(from, argument.getName(), false) .in(convertValue(environment, argument, argument.getValue())); } @@ -298,7 +303,7 @@ protected Predicate getPredicate(CriteriaBuilder cb, Root from, From pat } else { String fieldName = argument.getName().split("\\.")[0]; - From join = getCompoundJoin(path, argument.getName(), false); + From join = getCompoundJoin(path, argument.getName(), true); Argument where = new Argument("where", argument.getValue()); Map variables = Optional.ofNullable(environment.getContext()) .filter(it -> it instanceof Map) @@ -414,17 +419,15 @@ private Predicate getFieldPredicate(String fieldName, CriteriaBuilder cb, From arguments = new LinkedHashMap<>(); + boolean isOptional = false; - if(Logical.names().contains(argument.getName())) + if(Logical.names().contains(argument.getName())) { arguments.put(logical.name(), environment.getArgument(argument.getName())); - else + } else { arguments.put(logical.name(), environment.getArgument(fieldName)); - - boolean isOptional = Optional.of(argument) - .filter(it -> !Logical.names().contains(it.getName())) - .map(it -> getAttribute(environment, argument)) - .map(this::isOptional) - .orElse(false); + + isOptional = isOptionalAttribute(getAttribute(environment, argument)); + } return getArgumentPredicate(cb, reuseJoin(path, fieldName, isOptional), wherePredicateEnvironment(environment, fieldDefinition, arguments), @@ -659,7 +662,7 @@ private Attribute getAttribute(DataFetchingEnvironment environment, Argumen return entityType.getAttribute(argument.getName()); } - private boolean isOptional(Attribute attribute) { + private boolean isOptionalAttribute(Attribute attribute) { if(SingularAttribute.class.isInstance(attribute)) { return SingularAttribute.class.cast(attribute).isOptional(); } @@ -815,14 +818,38 @@ protected final Stream flatten(Field field) { @SuppressWarnings( "unchecked" ) - protected final R getObjectFieldValue(ObjectValue objectValue, String fieldName) { + protected final > R getObjectFieldValue(ObjectValue objectValue, String fieldName) { return (R) getObjectField(objectValue, fieldName).map(it-> it.getValue()) .orElse(new NullValue()); } @SuppressWarnings( "unchecked" ) - protected final R getArgumentValue(Argument argument) { - return (R) argument.getValue(); + protected final T getArgumentValue(DataFetchingEnvironment environment, Argument argument, Class type) { + Value value = argument.getValue(); + + if(VariableReference.class.isInstance(value)) { + return (T) + environment.getExecutionContext() + .getVariables() + .get(VariableReference.class.cast(value).getName()); + } + else if (BooleanValue.class.isInstance(value)) { + return (T) new Boolean(BooleanValue.class.cast(value).isValue()); + } + else if (IntValue.class.isInstance(value)) { + return (T) IntValue.class.cast(value).getValue(); + } + else if (StringValue.class.isInstance(value)) { + return (T) StringValue.class.cast(value).getValue(); + } + else if (FloatValue.class.isInstance(value)) { + return (T) FloatValue.class.cast(value).getValue(); + } + else if (NullValue.class.isInstance(value)) { + return (T) null; + } + + throw new IllegalArgumentException("Not supported"); } protected final Optional getObjectField(ObjectValue objectValue, String fieldName) { 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 1f2796a98..cbe6952b5 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 @@ -18,6 +18,7 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -772,6 +773,94 @@ public void queryWithWhereInsideOneToManyRelations() { assertThat(result.toString()).isEqualTo(expected); } + @Test + public void queryHumansWithFavoriteDroidDefaultOptionalTrue() { + //given: + String query = "query { " + + "Humans {" + + " select {" + + " id" + + " name" + + " homePlanet" + + " favoriteDroid {" + + " name" + + " }" + + " }" + + " }" + + "}"; + + String expected = "{Humans={select=[" + + "{id=1000, name=Luke Skywalker, homePlanet=Tatooine, favoriteDroid={name=C-3PO}}, " + + "{id=1001, name=Darth Vader, homePlanet=Tatooine, favoriteDroid={name=R2-D2}}, " + + "{id=1002, name=Han Solo, homePlanet=null, favoriteDroid=null}, " + + "{id=1003, name=Leia Organa, homePlanet=Alderaan, favoriteDroid=null}, " + + "{id=1004, name=Wilhuff Tarkin, homePlanet=null, favoriteDroid=null}" + + "]}}"; + + //when: + Object result = executor.execute(query).getData(); + + //then: + assertThat(result.toString()).isEqualTo(expected); + } + + @Test + public void queryHumansWittFavorideDroidExplicitOptionalFalse() { + //given: + String query = "query { " + + "Humans {" + + " select {" + + " id" + + " name" + + " homePlanet" + + " favoriteDroid(optional: false) {" + + " name" + + " }" + + " }" + + " }" + + "}"; + + String expected = "{Humans={select=[" + + "{id=1000, name=Luke Skywalker, homePlanet=Tatooine, favoriteDroid={name=C-3PO}}, " + + "{id=1001, name=Darth Vader, homePlanet=Tatooine, favoriteDroid={name=R2-D2}}" + + "]}}"; + + //when: + Object result = executor.execute(query).getData(); + + //then: + assertThat(result.toString()).isEqualTo(expected); + } + + @Test + public void queryHumansWittFavorideDroidExplicitOptionalFalseParameterBinding() { + //given: + String query = "query($optional: Boolean) { " + + "Humans {" + + " select {" + + " id" + + " name" + + " homePlanet" + + " favoriteDroid(optional: $optional) {" + + " name" + + " }" + + " }" + + " }" + + "}"; + + Map variables = Collections.singletonMap("optional", false); + + String expected = "{Humans={select=[" + + "{id=1000, name=Luke Skywalker, homePlanet=Tatooine, favoriteDroid={name=C-3PO}}, " + + "{id=1001, name=Darth Vader, homePlanet=Tatooine, favoriteDroid={name=R2-D2}}" + + "]}}"; + + //when: + Object result = executor.execute(query, variables).getData(); + + //then: + assertThat(result.toString()).isEqualTo(expected); + } @Test public void queryFilterManyToOneEmbdeddedCriteria() { From ea84a002e0ef1b8f08221262547eec149078fbec Mon Sep 17 00:00:00 2001 From: Igor Dianov Date: Sun, 28 Apr 2019 15:01:16 -0700 Subject: [PATCH 8/8] fix: compile error --- .../graphql/jpa/query/schema/impl/QraphQLJpaBaseDataFetcher.java | 1 - 1 file changed, 1 deletion(-) 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 96d350e0c..6c43a4a33 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 @@ -198,7 +198,6 @@ protected final List getFieldArguments(Field field, CriteriaQuery q boolean isOptional = (PersistentAttributeType.MANY_TO_MANY == attribute.getPersistentAttributeType()); reuseJoin(from, selectedField.getName(), isOptional); - } } } }