From a3046192bafc71c3e937601fbcdf139d36a21be8 Mon Sep 17 00:00:00 2001 From: Daniel Ocampo Date: Tue, 6 Jun 2017 10:36:02 +1200 Subject: [PATCH 1/3] Add support for arguments for DataFetcher when GraphQLField is declared as method. --- gradle/wrapper/gradle-wrapper.properties | 4 +- .../annotations/GraphQLAnnotations.java | 39 +++++---- .../annotations/GraphQLDataFetcherTest.java | 80 +++++++++++++++++-- 3 files changed, 96 insertions(+), 27 deletions(-) diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index f1df8551..4fad7b22 100644 --- a/gradle/wrapper/gradle-wrapper.properties +++ b/gradle/wrapper/gradle-wrapper.properties @@ -1,6 +1,6 @@ -#Thu Oct 27 10:18:00 ICT 2016 +#Tue Jun 06 09:11:54 NZST 2017 distributionBase=GRADLE_USER_HOME distributionPath=wrapper/dists zipStoreBase=GRADLE_USER_HOME zipStorePath=wrapper/dists -distributionUrl=https\://services.gradle.org/distributions/gradle-3.1-bin.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-3.1-all.zip diff --git a/src/main/java/graphql/annotations/GraphQLAnnotations.java b/src/main/java/graphql/annotations/GraphQLAnnotations.java index c61390ad..c08858f2 100644 --- a/src/main/java/graphql/annotations/GraphQLAnnotations.java +++ b/src/main/java/graphql/annotations/GraphQLAnnotations.java @@ -382,21 +382,7 @@ protected GraphQLFieldDefinition getField(Field field) throws GraphQLAnnotations GraphQLDataFetcher dataFetcher = field.getAnnotation(GraphQLDataFetcher.class); DataFetcher actualDataFetcher = null; if (nonNull(dataFetcher)) { - final String[] args; - if ( dataFetcher.firstArgIsTargetName() ) { - args = Stream.concat(Stream.of(field.getName()), stream(dataFetcher.args())).toArray(String[]::new); - } else { - args = dataFetcher.args(); - } - if (args.length == 0) { - actualDataFetcher = newInstance(dataFetcher.value()); - } else { - try { - final Constructor ctr = dataFetcher.value().getDeclaredConstructor( - stream(args).map(v -> String.class).toArray(Class[]::new)); - actualDataFetcher = constructNewInstance(ctr, (Object[]) args); - } catch (final NoSuchMethodException e) {} - } + actualDataFetcher = constructDataFetcher(field.getName(), dataFetcher); } if (actualDataFetcher == null) { @@ -440,6 +426,26 @@ protected GraphQLFieldDefinition getField(Field field) throws GraphQLAnnotations return new GraphQLFieldDefinitionWrapper(builder.build()); } + private DataFetcher constructDataFetcher(String fieldName, GraphQLDataFetcher annotatedDataFetcher) { + final String[] args; + if ( annotatedDataFetcher.firstArgIsTargetName() ) { + args = Stream.concat(Stream.of(fieldName), stream(annotatedDataFetcher.args())).toArray(String[]::new); + } else { + args = annotatedDataFetcher.args(); + } + if (args.length == 0) { + return newInstance(annotatedDataFetcher.value()); + } else { + try { + final Constructor ctr = annotatedDataFetcher.value().getDeclaredConstructor( + stream(args).map(v -> String.class).toArray(Class[]::new)); + return constructNewInstance(ctr, (Object[]) args); + } catch (final NoSuchMethodException e) { + throw new GraphQLAnnotationsException("Unable to instantiate DataFetcher via constructor for: " + fieldName, e); + } + } + } + protected GraphQLFieldDefinition field(Field field) throws IllegalAccessException, InstantiationException { return getInstance().getField(field); } @@ -480,7 +486,6 @@ private boolean isConnection(AccessibleObject obj, Class klass, GraphQLOutput protected GraphQLFieldDefinition getField(Method method) throws GraphQLAnnotationsException { GraphQLFieldDefinition.Builder builder = newFieldDefinition(); - String name = method.getName().replaceFirst("^(is|get|set)(.+)", "$2"); name = Character.toLowerCase(name.charAt(0)) + name.substring(1); GraphQLName nameAnn = method.getAnnotation(GraphQLName.class); @@ -564,7 +569,7 @@ protected GraphQLFieldDefinition getField(Method method) throws GraphQLAnnotatio } else if (dataFetcher == null) { actualDataFetcher = new MethodDataFetcher(method, typeFunction); } else { - actualDataFetcher = newInstance(dataFetcher.value()); + actualDataFetcher = constructDataFetcher(method.getName(), dataFetcher); } if (method.isAnnotationPresent(GraphQLRelayMutation.class) && relay != null) { diff --git a/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java b/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java index 618e1553..f02ec52f 100644 --- a/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java +++ b/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java @@ -14,17 +14,13 @@ */ package graphql.annotations; -import java.util.HashMap; - import graphql.ExecutionResult; import graphql.GraphQL; -import graphql.schema.DataFetcher; -import graphql.schema.DataFetchingEnvironment; -import graphql.schema.GraphQLObjectType; -import graphql.schema.GraphQLSchema; -import graphql.schema.PropertyDataFetcher; +import graphql.schema.*; import org.testng.annotations.Test; +import java.util.HashMap; + import static graphql.schema.GraphQLSchema.newSchema; import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertNotNull; @@ -33,7 +29,7 @@ public class GraphQLDataFetcherTest { @Test - public void shouldUsePreferredConstructor() { + public void shouldUsePreferredConstructorForField() { // Given final GraphQLObjectType object = GraphQLAnnotations.object(GraphQLDataFetcherTest.TestGraphQLQuery.class); final GraphQLSchema schema = newSchema().query(object).build(); @@ -49,6 +45,38 @@ public void shouldUsePreferredConstructor() { assertTrue(((HashMap)data.get("sample")).get("isBad")); } + @Test + public void shouldUseProvidedSoloArgumentForDataFetcherDeclaredInMethod() { + // Given + final GraphQLObjectType object = GraphQLAnnotations.object(TestMethodWithDataFetcherGraphQLQuery.class); + final GraphQLSchema schema = newSchema().query(object).build(); + final GraphQL graphql = new GraphQL(schema); + + // When + final ExecutionResult result = graphql.execute("{great}"); + + // Then + final HashMap data = (HashMap) result.getData(); + assertNotNull(data); + assertFalse((Boolean)data.get("great")); + } + + @Test + public void shouldUseTargetAndArgumentsForDataFetcherDeclaredInMethod() { + // Given + final GraphQLObjectType object = GraphQLAnnotations.object(TestMethodWithDataFetcherGraphQLQuery.class); + final GraphQLSchema schema = newSchema().query(object).build(); + final GraphQL graphql = new GraphQL(schema); + + // When + final ExecutionResult result = graphql.execute("{sample {bad}}"); + + // Then + final HashMap data = (HashMap) result.getData(); + assertNotNull(data); + assertTrue(((HashMap)data.get("sample")).get("bad")); + } + @GraphQLName("Query") public static class TestGraphQLQuery { @GraphQLField @@ -58,6 +86,17 @@ public TestSample sample() { // Note that GraphQL uses TestSample to build the g } } + @GraphQLName("Query") + public static class TestMethodWithDataFetcherGraphQLQuery { + @GraphQLField + @GraphQLDataFetcher(value = SampleOneArgDataFetcher.class, args = "true") + public Boolean great() { return false; } + + @GraphQLField + @GraphQLDataFetcher(SampleDataFetcher.class) + public TestSampleMethod sample() { return null; } + } + public static class TestSample { @GraphQLField @GraphQLDataFetcher(value = PropertyDataFetcher.class, args = "isGreat") @@ -69,6 +108,14 @@ public static class TestSample { } + public static class TestSampleMethod { + + @GraphQLField + @GraphQLDataFetcher(value = SampleMultiArgDataFetcher.class, firstArgIsTargetName = true, args = {"true"}) + public Boolean isBad() { return false; } // Defaults to FieldDataFetcher + + } + public static class SampleDataFetcher implements DataFetcher { @Override public Object get(final DataFetchingEnvironment environment) { @@ -76,6 +123,23 @@ public Object get(final DataFetchingEnvironment environment) { } } + public static class SampleOneArgDataFetcher implements DataFetcher { + private boolean flip = false; + + public SampleOneArgDataFetcher(String flip) { + this.flip = Boolean.valueOf(flip); + } + + @Override + public Object get(DataFetchingEnvironment environment) { + if ( flip ) { + return !flip; + } else { + return flip; + } + } + } + public static class SampleMultiArgDataFetcher extends PropertyDataFetcher { private boolean flip = false; From 46d900e3cd19e87103e051188c4c65215e0b2edc Mon Sep 17 00:00:00 2001 From: Daniel Ocampo Date: Tue, 6 Jun 2017 11:15:06 +1200 Subject: [PATCH 2/3] Use direct imports. --- .../java/graphql/annotations/GraphQLDataFetcherTest.java | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java b/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java index f02ec52f..5ab21b7d 100644 --- a/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java +++ b/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java @@ -16,7 +16,11 @@ import graphql.ExecutionResult; import graphql.GraphQL; -import graphql.schema.*; +import graphql.schema.DataFetcher; +import graphql.schema.DataFetchingEnvironment; +import graphql.schema.GraphQLObjectType; +import graphql.schema.GraphQLSchema; +import graphql.schema.PropertyDataFetcher; import org.testng.annotations.Test; import java.util.HashMap; From fe1088ffa8f0a5b92abea59f09894ebd992e60bd Mon Sep 17 00:00:00 2001 From: Daniel Ocampo Date: Fri, 9 Jun 2017 10:36:25 +1200 Subject: [PATCH 3/3] Formatting --- .../java/graphql/annotations/GraphQLDataFetcherTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java b/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java index adda19ad..15273a7e 100644 --- a/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java +++ b/src/test/java/graphql/annotations/GraphQLDataFetcherTest.java @@ -118,9 +118,9 @@ public static class TestSampleMethod { @GraphQLDataFetcher(value = SampleMultiArgDataFetcher.class, firstArgIsTargetName = true, args = {"true"}) public Boolean isBad() { return false; } // Defaults to FieldDataFetcher - } + } - public static class SampleDataFetcher implements DataFetcher { + public static class SampleDataFetcher implements DataFetcher { @Override public Object get(final DataFetchingEnvironment environment) { return new Sample(); // Notice that it return a Sample, not a TestSample @@ -155,7 +155,7 @@ public SampleMultiArgDataFetcher(String target, String flip) { @Override public Object get(DataFetchingEnvironment environment) { final Object result = super.get(environment); - if ( flip ) { + if (flip) { return !(Boolean)result; } else { return result;