fix: Added support for binding orderBy argument as a variable (#195)

Author: anotender

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

@@ -1027,8 +1027,8 @@ private void setNoOpCoercing(GraphQLType type) {
             GraphQLEnumType.newEnum()
                     .name("OrderBy")
                     .description("Specifies the direction (Ascending / Descending) to sort a field.")
-                    .value("ASC", 0, "Ascending")
-                    .value("DESC", 1, "Descending")
+                    .value("ASC", "ASC", "Ascending")
+                    .value("DESC", "DESC", "Descending")
                     .build();
 
 

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

@@ -107,8 +107,6 @@ class QraphQLJpaBaseDataFetcher implements DataFetcher<Object> {
     private static final String WHERE = "where";
 
     protected static final String OPTIONAL = "optional";
-    
-    protected static final List<String> ARGUMENTS = Arrays.asList(OPTIONAL);
 
     // "__typename" is part of the graphql introspection spec and has to be ignored
     private static final String TYPENAME = "__typename";
@@ -182,16 +180,17 @@ protected final List<Predicate> getFieldPredicates(Field field, CriteriaQuery<?>
                     // Build predicate arguments for singular attributes only
                     if(fieldPath.getModel() instanceof SingularAttribute) {
                         // Process the orderBy clause
-                        Optional<Argument> orderByArgument = selectedField.getArguments().stream()
-                            .filter(this::isOrderByArgument)
-                            .findFirst();
-
-                        if (orderByArgument.isPresent()) {
-                            if ("DESC".equals(((EnumValue) orderByArgument.get().getValue()).getName()))
-                                query.orderBy(cb.desc(fieldPath));
-                            else
-                                query.orderBy(cb.asc(fieldPath));
-                        }
+                        selectedField.getArguments().stream()
+                                .filter(this::isOrderByArgument)
+                                .findFirst()
+                                .map(a -> getOrderByValue(a, environment))
+                                .ifPresent(orderBy -> {
+                                    if ("DESC".equals(orderBy.getName())) {
+                                        query.orderBy(cb.desc(fieldPath));
+                                    } else {
+                                        query.orderBy(cb.asc(fieldPath));
+                                    }
+                                });
 
                         // 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
                         SingularAttribute<?,?> attribute = (SingularAttribute<?,?>) fieldPath.getModel();
@@ -391,12 +390,7 @@ private <R extends Value<?>> R getValue(Argument argument, DataFetchingEnvironme
         Value<?> value = argument.getValue();
 
         if(VariableReference.class.isInstance(value)) {
-            String variableName = VariableReference.class.cast(value)
-                                                         .getName();
-            
-            Object variableValue = environment.getExecutionContext()
-                                              .getVariables()
-                                              .get(variableName);
+            Object variableValue = getVariableReferenceValue((VariableReference) value, environment);
 
             GraphQLArgument graphQLArgument = environment.getExecutionStepInfo()
                                                 .getFieldDefinition()
@@ -408,6 +402,22 @@ private <R extends Value<?>> R getValue(Argument argument, DataFetchingEnvironme
         return (R) value;
     }
 
+    private EnumValue getOrderByValue(Argument argument, DataFetchingEnvironment environment) {
+        Value<?> value = argument.getValue();
+
+        if(VariableReference.class.isInstance(value)) {
+            Object variableValue = getVariableReferenceValue((VariableReference) value, environment);
+            return EnumValue.newEnumValue(variableValue.toString()).build();
+        }
+        return (EnumValue) value;
+    }
+
+    private Object getVariableReferenceValue(VariableReference variableReference, DataFetchingEnvironment env) {
+        return env.getExecutionContext()
+                .getVariables()
+                .get(variableReference.getName());
+    }
+
     protected Predicate getWherePredicate(CriteriaBuilder cb, Root<?> root,  From<?,?> path, DataFetchingEnvironment environment, Argument argument) {
         ObjectValue whereValue = getValue(argument, environment);
 

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

@@ -18,6 +18,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.tuple;
+import static org.assertj.core.api.BDDAssertions.then;
 import static org.assertj.core.util.Lists.list;
 
 import java.util.Arrays;
@@ -28,6 +29,7 @@
 
 import javax.persistence.EntityManager;
 
+import org.assertj.core.util.Maps;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -1568,4 +1570,45 @@ public void shouldNotReturnStaleCacheResultsFromPreviousQueryForEmbeddedCriteria
         assertThat(result2.toString()).isEqualTo(expected2);
     }      
 
+    @Test
+    public void queryWithEnumParameterShouldExecuteWithNoError() {
+        //given
+        String query = "" +
+                "query($orderById: OrderBy) {" +
+                "   Books {" +
+                "       select {" +
+                "           id(orderBy: $orderById)" +
+                "           title" +
+                "       }" +
+                "   }" +
+                "}";
+        Map<String, Object> variables = Maps.newHashMap("orderById",
+                                                        "DESC");
+
+        //when
+        ExecutionResult executionResult = executor.execute(query,
+                                                           variables);
+
+        // then
+        List<GraphQLError> errors = executionResult.getErrors();
+        Map<String, Object> data = executionResult.getData();
+        then(errors).isEmpty();
+        then(data)
+                .isNotNull().isNotEmpty()
+                .extracting("Books")
+                .flatExtracting("select")
+                .extracting("id", "title")
+                .containsExactly(
+                        tuple(7L,
+                              "Three Sisters"),
+                        tuple(6L,
+                              "The Seagull"),
+                        tuple(5L,
+                              "The Cherry Orchard"),
+                        tuple(3L,
+                              "Anna Karenina"),
+                        tuple(2L,
+                              "War and Peace")
+                );
+    }
 }