Skip to content

Commit 5ec0c0e

Browse files
yarinvakyarinvak
authored
Merge pull request #172 from osher-sade/method-data-fetcher-fix
Fixed a bug where MethodDataFetcher caused NullPointerException
2 parents 0d3016d + 48d7bc3 commit 5ec0c0e

File tree

3 files changed

+123
-14
lines changed

3 files changed

+123
-14
lines changed

‎src/main/java/graphql/annotations/dataFetchers/MethodDataFetcher.java‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,20 @@
1414
*/
1515
package graphql.annotations.dataFetchers;
1616

17-
import graphql.annotations.processor.ProcessingElementsContainer;
17+
import graphql.annotations.annotationTypes.GraphQLBatched;
1818
import graphql.annotations.annotationTypes.GraphQLInvokeDetached;
1919
import graphql.annotations.annotationTypes.GraphQLName;
20+
import graphql.annotations.processor.ProcessingElementsContainer;
2021
import graphql.annotations.processor.typeFunctions.TypeFunction;
2122
import graphql.schema.*;
22-
import graphql.schema.GraphQLType;
2323

2424
import java.lang.reflect.*;
2525
import java.util.ArrayList;
2626
import java.util.List;
2727
import java.util.Map;
2828

29-
import static graphql.annotations.processor.util.ReflectionKit.constructNewInstance;
3029
import static graphql.annotations.processor.util.NamingKit.toGraphqlName;
30+
import static graphql.annotations.processor.util.ReflectionKit.constructNewInstance;
3131
import static graphql.annotations.processor.util.ReflectionKit.newInstance;
3232

3333
public class MethodDataFetcher<T> implements DataFetcher<T> {
@@ -46,10 +46,7 @@ public MethodDataFetcher(Method method, TypeFunction typeFunction, ProcessingEle
4646
public T get(DataFetchingEnvironment environment) {
4747
try {
4848
T obj;
49-
50-
if (Modifier.isStatic(method.getModifiers())) {
51-
obj = null;
52-
} else if (method.getAnnotation(GraphQLInvokeDetached.class) != null) {
49+
if (method.isAnnotationPresent(GraphQLBatched.class) || method.isAnnotationPresent(GraphQLInvokeDetached.class)) {
5350
obj = newInstance((Class<T>) method.getDeclaringClass());
5451
} else if (!method.getDeclaringClass().isInstance(environment.getSource())) {
5552
obj = newInstance((Class<T>) method.getDeclaringClass(), environment.getSource());
@@ -59,8 +56,14 @@ public T get(DataFetchingEnvironment environment) {
5956
return null;
6057
}
6158
}
62-
return (T)method.invoke(obj, invocationArgs(environment, container));
63-
} catch (IllegalAccessException | InvocationTargetException e) {
59+
60+
if (obj == null && environment.getSource() != null) {
61+
Object value = getFieldValue(environment.getSource(), method.getName());
62+
return (T) value;
63+
}
64+
65+
return (T) method.invoke(obj, invocationArgs(environment, container));
66+
} catch (IllegalAccessException | InvocationTargetException | NoSuchFieldException e) {
6467
throw new RuntimeException(e);
6568
}
6669
}
@@ -131,4 +134,10 @@ private Object buildArg(Type p, GraphQLType graphQLType, Object arg) {
131134
return arg;
132135
}
133136
}
137+
138+
private Object getFieldValue(Object source, String fieldName) throws IllegalAccessException, NoSuchFieldException {
139+
Field field = source.getClass().getDeclaredField(fieldName);
140+
field.setAccessible(true);
141+
return field.get(source);
142+
}
134143
}

‎src/main/java/graphql/annotations/processor/util/ReflectionKit.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,4 @@ public static <T> T newInstance(Class<T> clazz, Object parameter) {
6969
}
7070
return null;
7171
}
72-
73-
7472
}

‎src/test/java/graphql/annotations/MethodDataFetcherTest.java‎

Lines changed: 105 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,27 @@
1414
*/
1515
package graphql.annotations;
1616

17+
import graphql.ExceptionWhileDataFetching;
18+
import graphql.ExecutionResult;
19+
import graphql.GraphQL;
20+
import graphql.annotations.annotationTypes.GraphQLDataFetcher;
21+
import graphql.annotations.annotationTypes.GraphQLField;
22+
import graphql.annotations.annotationTypes.GraphQLInvokeDetached;
23+
import graphql.annotations.annotationTypes.GraphQLType;
1724
import graphql.annotations.dataFetchers.MethodDataFetcher;
1825
import graphql.annotations.processor.GraphQLAnnotations;
19-
import graphql.schema.DataFetchingEnvironmentImpl;
26+
import graphql.schema.*;
2027
import org.testng.annotations.BeforeMethod;
2128
import org.testng.annotations.Test;
2229

2330
import java.util.ArrayList;
2431
import java.util.HashMap;
32+
import java.util.Map;
2533

34+
import static graphql.schema.GraphQLSchema.newSchema;
35+
import static org.testng.Assert.*;
36+
37+
@SuppressWarnings("unchecked")
2638
public class MethodDataFetcherTest {
2739

2840
@BeforeMethod
@@ -41,10 +53,100 @@ public String method() throws TestException {
4153
@Test(expectedExceptions = RuntimeException.class)
4254
public void exceptionRethrowing() {
4355
try {
44-
MethodDataFetcher methodDataFetcher = new MethodDataFetcher(getClass().getMethod("method"),null,null);
45-
methodDataFetcher.get(new DataFetchingEnvironmentImpl(this, new HashMap<String,Object>(), null, null, null, new ArrayList<>(), null, null, null, null, null, null, null));
56+
MethodDataFetcher methodDataFetcher = new MethodDataFetcher(getClass().getMethod("method"), null, null);
57+
methodDataFetcher.get(new DataFetchingEnvironmentImpl(this, new HashMap<>(), null, null, null, new ArrayList<>(), null, null, null, null, null, null, null));
4658
} catch (NoSuchMethodException e) {
4759
e.printStackTrace();
4860
}
4961
}
62+
63+
64+
@GraphQLType
65+
public static class ApiType {
66+
@GraphQLField
67+
public int a() {
68+
return 1;
69+
}
70+
71+
@GraphQLField
72+
@GraphQLInvokeDetached
73+
public int b() {
74+
return 2;
75+
}
76+
77+
@GraphQLField
78+
public int c() {
79+
return 4;
80+
}
81+
}
82+
83+
public static class InternalType {
84+
public int a = 123;
85+
public int b;
86+
}
87+
88+
@GraphQLType
89+
public static class Query {
90+
@GraphQLField
91+
@GraphQLDataFetcher(MyFetcher.class)
92+
public ApiType field;
93+
94+
@GraphQLField
95+
@GraphQLDataFetcher(MyApiFetcher.class)
96+
public ApiType apiField;
97+
}
98+
99+
public static class MyFetcher implements DataFetcher<InternalType> {
100+
public InternalType get(DataFetchingEnvironment environment) {
101+
return new InternalType();
102+
}
103+
}
104+
105+
public static class MyApiFetcher implements DataFetcher<ApiType> {
106+
public ApiType get(DataFetchingEnvironment environment) {
107+
return new ApiType();
108+
}
109+
}
110+
111+
112+
@Test
113+
public void queryingOneFieldNotAnnotatedWithGraphQLInvokeDetached_valueIsDeterminedByEntity() {
114+
GraphQLObjectType object = GraphQLAnnotations.object(Query.class);
115+
GraphQLSchema schema = newSchema().query(object).build();
116+
117+
ExecutionResult result = GraphQL.newGraphQL(schema).build().execute("query { field { a } }");
118+
assertTrue(result.getErrors().isEmpty());
119+
assertEquals(((Map<String, Map<String, Integer>>) result.getData()).get("field").get("a").toString(), "123");
120+
}
121+
122+
@Test
123+
public void queryingOneFieldAnnotatedWithGraphQLInvokeDetached_valueIsDeterminedByApiEntity() {
124+
GraphQLObjectType object = GraphQLAnnotations.object(Query.class);
125+
GraphQLSchema schema = newSchema().query(object).build();
126+
127+
ExecutionResult result = GraphQL.newGraphQL(schema).build().execute("query { field { b } }");
128+
assertTrue(result.getErrors().isEmpty());
129+
assertEquals(((Map<String, Map<String, Integer>>) result.getData()).get("field").get("b").toString(), "2");
130+
}
131+
132+
@Test
133+
public void queryingFieldsFromApiEntityFetcher_valueIsDeterminedByApiEntity() {
134+
GraphQLObjectType object = GraphQLAnnotations.object(Query.class);
135+
GraphQLSchema schema = newSchema().query(object).build();
136+
137+
ExecutionResult result = GraphQL.newGraphQL(schema).build().execute("query { apiField { a b } }");
138+
assertTrue(result.getErrors().isEmpty());
139+
assertEquals(((Map<String, Map<String, Integer>>) result.getData()).get("apiField").get("a").toString(), "1");
140+
assertEquals(((Map<String, Map<String, Integer>>) result.getData()).get("apiField").get("b").toString(), "2");
141+
}
142+
143+
@Test
144+
public void queryingFieldsFromNoApiEntityFetcher_noMatchingFieldInEntity_throwException(){
145+
GraphQLObjectType object = GraphQLAnnotations.object(Query.class);
146+
GraphQLSchema schema = newSchema().query(object).build();
147+
148+
ExecutionResult result = GraphQL.newGraphQL(schema).build().execute("query { field { c } }");
149+
assertFalse(result.getErrors().isEmpty());
150+
assertTrue(((ExceptionWhileDataFetching)result.getErrors().get(0)).getException().getCause() instanceof NoSuchFieldException);
151+
}
50152
}

0 commit comments

Comments
 (0)