Skip to content

Commit e2ddd4d

Browse files
igdianovigdianov
authored
feat: add optional argument for plural attributes (#138)
* feat: add optional argument parameter for plural attributes * feat: add optional one-to-many tests * fix: polish tests queries * fix: polish optionalArgument method * fix: make collections join fetch default optional true * feat: add join fetch collection auto configuration property * fix: RTE with explicit optional and nested orderBy * fix: add more tests
1 parent d452620 commit e2ddd4d

File tree

15 files changed

+453
-55
lines changed

15 files changed

+453
-55
lines changed

‎graphql-jpa-query-autoconfigure/src/main/java/com/introproventures/graphql/jpa/query/autoconfigure/GraphQLJpaQueryProperties.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ public class GraphQLJpaQueryProperties {
4646
*/
4747
private boolean isDefaultDistinct = true;
4848

49+
/**
50+
* Set default value for optional argument for join fetch collections.
51+
*/
52+
private boolean toManyDefaultOptional = true;
53+
4954
/**
5055
* Enable or disable QraphQL module services.
5156
*/
@@ -143,5 +148,15 @@ public String getPath() {
143148
public void setPath(String path) {
144149
this.path = path;
145150
}
151+
152+
153+
public boolean isToManyDefaultOptional() {
154+
return toManyDefaultOptional;
155+
}
156+
157+
158+
public void setToManyDefaultOptional(boolean toManyDefaultOptional) {
159+
this.toManyDefaultOptional = toManyDefaultOptional;
160+
}
146161

147162
}

‎graphql-jpa-query-autoconfigure/src/test/java/com/introproventures/graphql/jpa/query/autoconfigure/GraphQLSchemaAutoConfigurationTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,10 +99,11 @@ public void contextLoads() {
9999

100100

101101
@Test
102-
public void configurationProperties() {
102+
public void defaultConfigurationProperties() {
103103
// given
104104
assertThat(graphQLJpaQueryProperties.isDefaultDistinct()).isTrue();
105105
assertThat(graphQLJpaQueryProperties.isUseDistinctParameter()).isFalse();
106+
assertThat(graphQLJpaQueryProperties.isToManyDefaultOptional()).isTrue();
106107
}
107108

108109

‎graphql-jpa-query-example-merge/src/main/resources/books.sql

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ insert into author (id, name) values (4, 'Anton Chekhov');
66
insert into book (id, title, author_id, genre) values (5, 'The Cherry Orchard', 4, 'PLAY');
77
insert into book (id, title, author_id, genre) values (6, 'The Seagull', 4, 'PLAY');
88
insert into book (id, title, author_id, genre) values (7, 'Three Sisters', 4, 'PLAY');
9+
insert into author (id, name, genre) values (8, 'Igor Dianov', 'JAVA');

‎graphql-jpa-query-example-model-books/src/main/java/com/introproventures/graphql/jpa/query/schema/model/book/Genre.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,5 +17,5 @@
1717
package com.introproventures.graphql.jpa.query.schema.model.book;
1818

1919
public enum Genre {
20-
NOVEL, PLAY
20+
NOVEL, PLAY, JAVA
2121
}

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

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,15 @@
4646
*/
4747
class GraphQLJpaOneToManyDataFetcher extends GraphQLJpaQueryDataFetcher {
4848

49+
protected static final String OPTIONAL = "optional";
4950
private final PluralAttribute<Object,Object,Object> attribute;
5051

51-
public GraphQLJpaOneToManyDataFetcher(EntityManager entityManager, EntityType<?> entityType, PluralAttribute<Object,Object,Object> attribute) {
52-
super(entityManager, entityType);
52+
public GraphQLJpaOneToManyDataFetcher(EntityManager entityManager,
53+
EntityType<?> entityType,
54+
boolean toManyDefaultOptional,
55+
boolean defaultDistinct,
56+
PluralAttribute<Object,Object,Object> attribute) {
57+
super(entityManager, entityType, defaultDistinct, toManyDefaultOptional);
5358

5459
this.attribute = attribute;
5560
}
@@ -63,12 +68,23 @@ public Object get(DataFetchingEnvironment environment) {
6368

6469
// Resolve collection query if where argument is present or any field in selection has orderBy argument
6570
if(whereArg.isPresent() || hasSelectionAnyOrderBy(field)) {
71+
TypedQuery<?> query = getQuery(environment, field, isDefaultDistinct());
6672

67-
//EntityGraph<?> entityGraph = buildEntityGraph(new Field("select", new SelectionSet(Arrays.asList(field))));
73+
// Let's not pass distinct if enabled to have better performance
74+
if(isDefaultDistinct()) {
75+
query.setHint(HIBERNATE_QUERY_PASS_DISTINCT_THROUGH, false);
76+
}
77+
78+
List<?> resultList = query.getResultList();
79+
80+
// Let's remove any duplicate references for root entities
81+
if(isDefaultDistinct()) {
82+
resultList = resultList.stream()
83+
.distinct()
84+
.collect(Collectors.toList());
85+
}
6886

69-
return getQuery(environment, field, true)
70-
//.setHint("javax.persistence.fetchgraph", entityGraph) // TODO: fix runtime exception
71-
.getResultList();
87+
return resultList;
7288
}
7389

7490
// Let hibernate resolve collection query
@@ -126,6 +142,7 @@ protected TypedQuery<?> getQuery(DataFetchingEnvironment environment, Field fiel
126142
query.select(join.alias(attribute.getName()));
127143

128144
List<Predicate> predicates = getFieldArguments(field, query, cb, join, environment).stream()
145+
.filter(it -> !OPTIONAL.equals(it.getName()))
129146
.map(it -> getPredicate(cb, from, join, environment, it))
130147
.filter(it -> it != null)
131148
.collect(Collectors.toList());

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

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -49,18 +49,21 @@ class GraphQLJpaQueryDataFetcher extends QraphQLJpaBaseDataFetcher {
4949

5050
private boolean defaultDistinct = true;
5151

52-
private static final String HIBERNATE_QUERY_PASS_DISTINCT_THROUGH = "hibernate.query.passDistinctThrough";
53-
private static final String ORG_HIBERNATE_CACHEABLE = "org.hibernate.cacheable";
54-
private static final String ORG_HIBERNATE_FETCH_SIZE = "org.hibernate.fetchSize";
55-
private static final String ORG_HIBERNATE_READ_ONLY = "org.hibernate.readOnly";
56-
private static final String JAVAX_PERSISTENCE_FETCHGRAPH = "javax.persistence.fetchgraph";
57-
58-
public GraphQLJpaQueryDataFetcher(EntityManager entityManager, EntityType<?> entityType) {
59-
super(entityManager, entityType);
52+
protected static final String HIBERNATE_QUERY_PASS_DISTINCT_THROUGH = "hibernate.query.passDistinctThrough";
53+
protected static final String ORG_HIBERNATE_CACHEABLE = "org.hibernate.cacheable";
54+
protected static final String ORG_HIBERNATE_FETCH_SIZE = "org.hibernate.fetchSize";
55+
protected static final String ORG_HIBERNATE_READ_ONLY = "org.hibernate.readOnly";
56+
protected static final String JAVAX_PERSISTENCE_FETCHGRAPH = "javax.persistence.fetchgraph";
57+
58+
private GraphQLJpaQueryDataFetcher(EntityManager entityManager, EntityType<?> entityType, boolean toManyDefaultOptional) {
59+
super(entityManager, entityType, toManyDefaultOptional);
6060
}
6161

62-
public GraphQLJpaQueryDataFetcher(EntityManager entityManager, EntityType<?> entityType, boolean defaultDistinct) {
63-
super(entityManager, entityType);
62+
public GraphQLJpaQueryDataFetcher(EntityManager entityManager,
63+
EntityType<?> entityType,
64+
boolean defaultDistinct,
65+
boolean toManyDefaultOptional) {
66+
super(entityManager, entityType, toManyDefaultOptional);
6467
this.defaultDistinct = defaultDistinct;
6568
}
6669

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

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,8 @@ public class GraphQLJpaSchemaBuilder implements GraphQLSchemaBuilder {
117117

118118
private boolean isUseDistinctParameter = false;
119119
private boolean isDefaultDistinct = true;
120+
// the many end is a collection, and it is always optional by default (empty collection)
121+
private boolean toManyDefaultOptional = true;
120122

121123
public GraphQLJpaSchemaBuilder(EntityManager entityManager) {
122124
this.entityManager = entityManager;
@@ -162,7 +164,7 @@ private GraphQLFieldDefinition getQueryFieldByIdDefinition(EntityType<?> entityT
162164
.name(entityType.getName())
163165
.description(getSchemaDescription( entityType.getJavaType()))
164166
.type(getObjectType(entityType))
165-
.dataFetcher(new GraphQLJpaSimpleDataFetcher(entityManager, entityType))
167+
.dataFetcher(new GraphQLJpaSimpleDataFetcher(entityManager, entityType, toManyDefaultOptional))
166168
.argument(entityType.getAttributes().stream()
167169
.filter(this::isValidInput)
168170
.filter(this::isNotIgnored)
@@ -205,7 +207,10 @@ private GraphQLFieldDefinition getQueryFieldSelectDefinition(EntityType<?> entit
205207
+ "Use the '"+QUERY_SELECT_PARAM_NAME+"' field to request actual fields. "
206208
+ "Use the '"+ORDER_BY_PARAM_NAME+"' on a field to specify sort order for each field. ")
207209
.type(pageType)
208-
.dataFetcher(new GraphQLJpaQueryDataFetcher(entityManager, entityType, isDefaultDistinct))
210+
.dataFetcher(new GraphQLJpaQueryDataFetcher(entityManager,
211+
entityType,
212+
isDefaultDistinct,
213+
toManyDefaultOptional))
209214
.argument(paginationArgument)
210215
.argument(getWhereArgument(entityType));
211216
if (isUseDistinctParameter) {
@@ -637,11 +642,13 @@ && isNotIgnoredOrder(attribute) ) {
637642
if (attribute instanceof SingularAttribute
638643
&& attribute.getPersistentAttributeType() != Attribute.PersistentAttributeType.BASIC) {
639644
ManagedType foreignType = getForeignType(attribute);
645+
SingularAttribute<?,?> singularAttribute = SingularAttribute.class.cast(attribute);
640646

641647
// TODO fix page count query
642648
arguments.add(getWhereArgument(foreignType));
643649

644-
arguments.add(optionalArgument(SingularAttribute.class.cast(attribute)));
650+
// to-one end could be optional
651+
arguments.add(optionalArgument(singularAttribute.isOptional()));
645652

646653
} // Get Sub-Objects fields queries via DataFetcher
647654
else if (attribute instanceof PluralAttribute
@@ -651,7 +658,15 @@ else if (attribute instanceof PluralAttribute
651658
EntityType elementType = (EntityType) ((PluralAttribute) attribute).getElementType();
652659

653660
arguments.add(getWhereArgument(elementType));
654-
dataFetcher = new GraphQLJpaOneToManyDataFetcher(entityManager, baseEntity, (PluralAttribute) attribute);
661+
662+
// make it configurable via builder api
663+
arguments.add(optionalArgument(toManyDefaultOptional));
664+
665+
dataFetcher = new GraphQLJpaOneToManyDataFetcher(entityManager,
666+
baseEntity,
667+
toManyDefaultOptional,
668+
isDefaultDistinct,
669+
(PluralAttribute) attribute);
655670
}
656671

657672
return GraphQLFieldDefinition.newFieldDefinition()
@@ -663,15 +678,15 @@ else if (attribute instanceof PluralAttribute
663678
.build();
664679
}
665680

666-
private GraphQLArgument optionalArgument(SingularAttribute<?,?> attribute) {
681+
private GraphQLArgument optionalArgument(Boolean defaultValue) {
667682
return GraphQLArgument.newArgument()
668683
.name("optional")
669684
.description("Optional association specification")
670685
.type(Scalars.GraphQLBoolean)
671-
.defaultValue(attribute.isOptional())
686+
.defaultValue(defaultValue)
672687
.build();
673-
}
674-
688+
}
689+
675690
protected ManagedType<?> getForeignType(Attribute<?,?> attribute) {
676691
if(SingularAttribute.class.isInstance(attribute))
677692
return (ManagedType<?>) ((SingularAttribute<?,?>) attribute).getType();
@@ -1102,5 +1117,15 @@ public GraphQLSchemaBuilder namingStrategy(NamingStrategy instance) {
11021117

11031118
return this;
11041119
}
1120+
1121+
1122+
public boolean isToManyDefaultOptional() {
1123+
return toManyDefaultOptional;
1124+
}
1125+
1126+
1127+
public void setToManyDefaultOptional(boolean toManyDefaultOptional) {
1128+
this.toManyDefaultOptional = toManyDefaultOptional;
1129+
}
11051130

11061131
}

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616

1717
package com.introproventures.graphql.jpa.query.schema.impl;
1818

19+
import java.util.List;
20+
import java.util.stream.Collectors;
21+
import java.util.stream.Stream;
22+
1923
import javax.persistence.EntityGraph;
2024
import javax.persistence.EntityManager;
2125
import javax.persistence.NoResultException;
@@ -26,14 +30,12 @@
2630
import graphql.language.ObjectValue;
2731
import graphql.schema.DataFetchingEnvironment;
2832

29-
import java.util.List;
30-
import java.util.stream.Collectors;
31-
import java.util.stream.Stream;
32-
3333
class GraphQLJpaSimpleDataFetcher extends QraphQLJpaBaseDataFetcher {
3434

35-
public GraphQLJpaSimpleDataFetcher(EntityManager entityManager, EntityType<?> entityType) {
36-
super(entityManager, entityType);
35+
public GraphQLJpaSimpleDataFetcher(EntityManager entityManager,
36+
EntityType<?> entityType,
37+
boolean toManyDefaultOptional) {
38+
super(entityManager, entityType, toManyDefaultOptional);
3739
}
3840

3941
@Override

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

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -100,16 +100,21 @@ class QraphQLJpaBaseDataFetcher implements DataFetcher<Object> {
100100

101101
protected final EntityManager entityManager;
102102
protected final EntityType<?> entityType;
103+
104+
private boolean toManyDefaultOptional = true;
103105

104106
/**
105107
* Creates JPA entity DataFetcher instance
106108
*
107109
* @param entityManager
108110
* @param entityType
109111
*/
110-
public QraphQLJpaBaseDataFetcher(EntityManager entityManager, EntityType<?> entityType) {
112+
public QraphQLJpaBaseDataFetcher(EntityManager entityManager,
113+
EntityType<?> entityType,
114+
boolean toManyDefaultOptional) {
111115
this.entityManager = entityManager;
112116
this.entityType = entityType;
117+
this.toManyDefaultOptional = toManyDefaultOptional;
113118
}
114119

115120
@Override
@@ -155,6 +160,7 @@ protected final List<Argument> getFieldArguments(Field field, CriteriaQuery<?> q
155160

156161
Path<?> fieldPath = from.get(selectedField.getName());
157162
Join<?,?> join = null;
163+
Optional<Argument> optionalArgument = getArgument(selectedField, OPTIONAL);
158164

159165
// Build predicate arguments for singular attributes only
160166
if(fieldPath.getModel() instanceof SingularAttribute) {
@@ -183,31 +189,28 @@ protected final List<Argument> getFieldArguments(Field field, CriteriaQuery<?> q
183189
if (attribute.getPersistentAttributeType() == Attribute.PersistentAttributeType.MANY_TO_ONE
184190
|| attribute.getPersistentAttributeType() == Attribute.PersistentAttributeType.ONE_TO_ONE
185191
) {
186-
// Let's apply left outer join to retrieve optional associations
187-
Optional<Argument> optionalArgument = getArgument(selectedField, OPTIONAL);
188-
189192
// Let's do fugly conversion
190193
Boolean isOptional = optionalArgument.map(it -> getArgumentValue(environment, it, Boolean.class))
191194
.orElse(attribute.isOptional());
192195

196+
// Let's apply left outer join to retrieve optional associations
193197
join = reuseJoin(from, selectedField.getName(), isOptional);
194198
}
195199
}
196200
} else {
197201
// 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());
202-
203-
// Let's apply left outer join to retrieve optional many-to-many associations
204-
boolean isOptional = (PersistentAttributeType.MANY_TO_MANY == attribute.getPersistentAttributeType());
202+
// Let's do fugly conversion
203+
// the many end is a collection, and it is always optional by default (empty collection)
204+
Boolean isOptional = optionalArgument.map(it -> getArgumentValue(environment, it, Boolean.class))
205+
.orElse(toManyDefaultOptional);
205206

207+
// Let's apply join to retrieve associated collection
206208
join = reuseJoin(from, selectedField.getName(), isOptional);
207209

210+
// TODO add fetch argument parameter
208211
// Let's fetch element collections to avoid filtering their values used where search criteria
209-
from.fetch(selectedField.getName(), isOptional ? JoinType.LEFT : JoinType.INNER);
210-
212+
from.fetch(selectedField.getName(),
213+
isOptional ? JoinType.LEFT : JoinType.INNER);
211214
}
212215

213216
// Let's build join fetch graph to avoid Hibernate error:
@@ -218,9 +221,10 @@ protected final List<Argument> getFieldArguments(Field field, CriteriaQuery<?> q
218221
selectedField);
219222
Map<String, Object> args = environment.getArguments();
220223

221-
DataFetchingEnvironment fieldEnvironment = wherePredicateEnvironment(environment, fieldDefinition, args);
222-
223-
224+
DataFetchingEnvironment fieldEnvironment = wherePredicateEnvironment(environment,
225+
fieldDefinition,
226+
args);
227+
// TODO nested where criteria expressions
224228
getFieldArguments(selectedField, query, cb, join, fieldEnvironment);
225229
}
226230
}

0 commit comments

Comments
 (0)