Skip to content

Commit d8e1402

Browse files
rstoyanchevrstoyanchev
committed
Merge branch '1.0.x'
2 parents eb9226d + a116505 commit d8e1402

File tree

4 files changed

+200
-15
lines changed

4 files changed

+200
-15
lines changed

‎spring-graphql/src/main/java/org/springframework/graphql/data/method/HandlerMethod.java

Lines changed: 76 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717

1818
import java.lang.annotation.Annotation;
1919
import java.lang.reflect.Method;
20+
import java.util.ArrayList;
21+
import java.util.Arrays;
22+
import java.util.List;
2023
import java.util.stream.Collectors;
2124
import java.util.stream.IntStream;
2225

@@ -26,6 +29,7 @@
2629
import org.springframework.beans.factory.BeanFactory;
2730
import org.springframework.core.BridgeMethodResolver;
2831
import org.springframework.core.MethodParameter;
32+
import org.springframework.core.ResolvableType;
2933
import org.springframework.core.annotation.AnnotatedElementUtils;
3034
import org.springframework.core.annotation.SynthesizingMethodParameter;
3135
import org.springframework.lang.Nullable;
@@ -67,6 +71,9 @@ public class HandlerMethod {
6771

6872
private final MethodParameter[] parameters;
6973

74+
@Nullable
75+
private volatile List<Annotation[][]> interfaceParameterAnnotations;
76+
7077

7178
/**
7279
* Constructor with a handler instance and a method.
@@ -241,6 +248,40 @@ public String getShortLogMessage() {
241248
return getBeanType().getSimpleName() + "#" + this.method.getName() + "[" + args + " args]";
242249
}
243250

251+
private List<Annotation[][]> getInterfaceParameterAnnotations() {
252+
List<Annotation[][]> parameterAnnotations = this.interfaceParameterAnnotations;
253+
if (parameterAnnotations == null) {
254+
parameterAnnotations = new ArrayList<>();
255+
for (Class<?> ifc : ClassUtils.getAllInterfacesForClassAsSet(this.method.getDeclaringClass())) {
256+
for (Method candidate : ifc.getMethods()) {
257+
if (isOverrideFor(candidate)) {
258+
parameterAnnotations.add(candidate.getParameterAnnotations());
259+
}
260+
}
261+
}
262+
this.interfaceParameterAnnotations = parameterAnnotations;
263+
}
264+
return parameterAnnotations;
265+
}
266+
267+
private boolean isOverrideFor(Method candidate) {
268+
if (!candidate.getName().equals(this.method.getName()) ||
269+
candidate.getParameterCount() != this.method.getParameterCount()) {
270+
return false;
271+
}
272+
Class<?>[] paramTypes = this.method.getParameterTypes();
273+
if (Arrays.equals(candidate.getParameterTypes(), paramTypes)) {
274+
return true;
275+
}
276+
for (int i = 0; i < paramTypes.length; i++) {
277+
if (paramTypes[i] !=
278+
ResolvableType.forMethodParameter(candidate, i, this.method.getDeclaringClass()).resolve()) {
279+
return false;
280+
}
281+
}
282+
return true;
283+
}
284+
244285

245286
@Override
246287
public boolean equals(@Nullable Object other) {
@@ -323,6 +364,9 @@ protected String formatInvokeError(String text, Object[] args) {
323364
*/
324365
protected class HandlerMethodParameter extends SynthesizingMethodParameter {
325366

367+
@Nullable
368+
private volatile Annotation[] combinedAnnotations;
369+
326370
public HandlerMethodParameter(int index) {
327371
super(HandlerMethod.this.bridgedMethod, index);
328372
}
@@ -347,8 +391,38 @@ public <T extends Annotation> boolean hasMethodAnnotation(Class<T> annotationTyp
347391
}
348392

349393
@Override
350-
public HandlerMethodParameter clone() {
351-
return new HandlerMethodParameter(this);
394+
public Annotation[] getParameterAnnotations() {
395+
Annotation[] anns = this.combinedAnnotations;
396+
if (anns == null) {
397+
anns = super.getParameterAnnotations();
398+
int index = getParameterIndex();
399+
if (index >= 0) {
400+
for (Annotation[][] ifcAnns : getInterfaceParameterAnnotations()) {
401+
if (index < ifcAnns.length) {
402+
Annotation[] paramAnns = ifcAnns[index];
403+
if (paramAnns.length > 0) {
404+
List<Annotation> merged = new ArrayList<>(anns.length + paramAnns.length);
405+
merged.addAll(Arrays.asList(anns));
406+
for (Annotation paramAnn : paramAnns) {
407+
boolean existingType = false;
408+
for (Annotation ann : anns) {
409+
if (ann.annotationType() == paramAnn.annotationType()) {
410+
existingType = true;
411+
break;
412+
}
413+
}
414+
if (!existingType) {
415+
merged.add(adaptAnnotation(paramAnn));
416+
}
417+
}
418+
anns = merged.toArray(new Annotation[0]);
419+
}
420+
}
421+
}
422+
}
423+
this.combinedAnnotations = anns;
424+
}
425+
return anns;
352426
}
353427
}
354428

‎spring-graphql/src/main/java/org/springframework/graphql/execution/ContextDataFetcherDecorator.java

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -103,22 +103,31 @@ static GraphQLTypeVisitor createVisitor(List<SubscriptionExceptionResolver> reso
103103

104104
return new GraphQLTypeVisitorStub() {
105105
@Override
106-
public TraversalControl visitGraphQLFieldDefinition(GraphQLFieldDefinition fieldDefinition,
107-
TraverserContext<GraphQLSchemaElement> context) {
106+
public TraversalControl visitGraphQLFieldDefinition(
107+
GraphQLFieldDefinition fieldDefinition, TraverserContext<GraphQLSchemaElement> context) {
108108

109109
GraphQLCodeRegistry.Builder codeRegistry = context.getVarFromParents(GraphQLCodeRegistry.Builder.class);
110110
GraphQLFieldsContainer parent = (GraphQLFieldsContainer) context.getParentNode();
111111
DataFetcher<?> dataFetcher = codeRegistry.getDataFetcher(parent, fieldDefinition);
112112

113-
if (dataFetcher.getClass().getPackage().getName().startsWith("graphql.")) {
114-
return TraversalControl.CONTINUE;
113+
if (applyDecorator(dataFetcher)) {
114+
boolean handlesSubscription = parent.getName().equals("Subscription");
115+
dataFetcher = new ContextDataFetcherDecorator(dataFetcher, handlesSubscription, compositeResolver);
116+
codeRegistry.dataFetcher(parent, fieldDefinition, dataFetcher);
115117
}
116118

117-
boolean handlesSubscription = parent.getName().equals("Subscription");
118-
dataFetcher = new ContextDataFetcherDecorator(dataFetcher, handlesSubscription, compositeResolver);
119-
codeRegistry.dataFetcher(parent, fieldDefinition, dataFetcher);
120119
return TraversalControl.CONTINUE;
121120
}
121+
122+
private boolean applyDecorator(DataFetcher<?> dataFetcher) {
123+
Class<?> type = dataFetcher.getClass();
124+
String packageName = type.getPackage().getName();
125+
if (packageName.startsWith("graphql.")) {
126+
return (type.getSimpleName().startsWith("DataFetcherFactories") ||
127+
packageName.startsWith("graphql.validation"));
128+
}
129+
return true;
130+
}
122131
};
123132
}
124133

‎spring-graphql/src/test/java/org/springframework/graphql/data/method/annotation/support/DataFetcherHandlerMethodTests.java

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
package org.springframework.graphql.data.method.annotation.support;
1717

1818

19+
import java.lang.reflect.Method;
20+
import java.util.Collections;
1921
import java.util.concurrent.Callable;
2022
import java.util.concurrent.CompletableFuture;
2123

@@ -27,10 +29,14 @@
2729
import org.mockito.Mockito;
2830

2931
import org.springframework.core.task.SimpleAsyncTaskExecutor;
32+
import org.springframework.graphql.data.GraphQlArgumentBinder;
3033
import org.springframework.graphql.data.method.HandlerMethod;
3134
import org.springframework.graphql.data.method.HandlerMethodArgumentResolver;
3235
import org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite;
36+
import org.springframework.graphql.data.method.annotation.Argument;
37+
import org.springframework.graphql.data.method.annotation.QueryMapping;
3338
import org.springframework.lang.Nullable;
39+
import org.springframework.util.ClassUtils;
3440

3541
import static org.assertj.core.api.Assertions.assertThat;
3642

@@ -41,6 +47,22 @@
4147
*/
4248
public class DataFetcherHandlerMethodTests {
4349

50+
@Test
51+
void annotatedMethodsOnInterface() {
52+
53+
HandlerMethodArgumentResolverComposite resolvers = new HandlerMethodArgumentResolverComposite();
54+
resolvers.addResolver(new ArgumentMethodArgumentResolver(new GraphQlArgumentBinder()));
55+
56+
DataFetcherHandlerMethod handlerMethod = new DataFetcherHandlerMethod(
57+
handlerMethodFor(new TestController(), "hello"), resolvers, null, null, false);
58+
59+
Object result = handlerMethod.invoke(
60+
DataFetchingEnvironmentImpl.newDataFetchingEnvironment()
61+
.arguments(Collections.singletonMap("name", "Neil"))
62+
.build());
63+
64+
assertThat(result).isEqualTo("Hello, Neil");
65+
}
4466

4567
@Test
4668
void callableReturnValue() throws Exception {
@@ -49,8 +71,8 @@ void callableReturnValue() throws Exception {
4971
resolvers.addResolver(Mockito.mock(HandlerMethodArgumentResolver.class));
5072

5173
DataFetcherHandlerMethod handlerMethod = new DataFetcherHandlerMethod(
52-
new HandlerMethod(new TestController(), TestController.class.getMethod("handleAndReturnCallable")),
53-
resolvers, null, new SimpleAsyncTaskExecutor(), false);
74+
handlerMethodFor(new TestController(), "handleAndReturnCallable"), resolvers, null,
75+
new SimpleAsyncTaskExecutor(), false);
5476

5577
DataFetchingEnvironment environment = DataFetchingEnvironmentImpl
5678
.newDataFetchingEnvironment()
@@ -64,8 +86,26 @@ void callableReturnValue() throws Exception {
6486
assertThat(future.get()).isEqualTo("A");
6587
}
6688

89+
private static HandlerMethod handlerMethodFor(Object controller, String methodName) {
90+
Method method = ClassUtils.getMethod(controller.getClass(), methodName, (Class<?>[]) null);
91+
return new HandlerMethod(controller, method);
92+
}
93+
94+
95+
interface TestInterface {
96+
97+
@QueryMapping
98+
String hello(@Argument String name);
6799

68-
private static class TestController {
100+
}
101+
102+
@SuppressWarnings("unused")
103+
private static class TestController implements TestInterface {
104+
105+
@Override
106+
public String hello(String name) {
107+
return "Hello, " + name;
108+
}
69109

70110
@Nullable
71111
public Callable<String> handleAndReturnCallable() {

‎spring-graphql/src/test/java/org/springframework/graphql/execution/ContextDataFetcherDecoratorTests.java

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,19 @@
1919
import java.time.Duration;
2020
import java.util.Collections;
2121
import java.util.List;
22+
import java.util.concurrent.CompletableFuture;
23+
import java.util.function.BiConsumer;
2224

2325
import graphql.ExecutionInput;
2426
import graphql.ExecutionResult;
2527
import graphql.GraphQL;
2628
import graphql.GraphQLError;
2729
import graphql.GraphqlErrorBuilder;
30+
import graphql.schema.DataFetcher;
31+
import graphql.schema.DataFetcherFactories;
32+
import graphql.schema.GraphQLFieldDefinition;
33+
import graphql.schema.idl.SchemaDirectiveWiring;
34+
import graphql.schema.idl.SchemaDirectiveWiringEnvironment;
2835
import io.micrometer.context.ContextRegistry;
2936
import io.micrometer.context.ContextSnapshot;
3037
import org.junit.jupiter.api.Test;
@@ -42,10 +49,18 @@
4249
* Tests for {@link ContextDataFetcherDecorator}.
4350
* @author Rossen Stoyanchev
4451
*/
52+
@SuppressWarnings("ReactiveStreamsUnusedPublisher")
4553
public class ContextDataFetcherDecoratorTests {
4654

47-
private static final String SCHEMA_CONTENT =
48-
"type Query { greeting: String, greetings: [String] } type Subscription { greetings: String }";
55+
private static final String SCHEMA_CONTENT = "" +
56+
"directive @UpperCase on FIELD_DEFINITION " +
57+
"type Query { " +
58+
" greeting: String @UpperCase, " +
59+
" greetings: [String] " +
60+
"} " +
61+
"type Subscription { " +
62+
" greetings: String " +
63+
"}";
4964

5065

5166
@Test
@@ -112,7 +127,7 @@ void fluxDataFetcherSubscription() throws Exception {
112127
}
113128

114129
@Test
115-
void fluxDataFetcherSubscriptionThrowException() throws Exception {
130+
void fluxDataFetcherSubscriptionThrowingException() throws Exception {
116131

117132
SubscriptionExceptionResolver resolver =
118133
SubscriptionExceptionResolver.forSingleError(exception ->
@@ -176,4 +191,51 @@ void dataFetcherWithThreadLocalContext() {
176191
}
177192
}
178193

194+
@Test // gh-440
195+
void dataFetcherDecoratedWithDataFetcherFactories() {
196+
197+
SchemaDirectiveWiring directiveWiring = new SchemaDirectiveWiring() {
198+
199+
@SuppressWarnings("unchecked")
200+
@Override
201+
public GraphQLFieldDefinition onField(SchemaDirectiveWiringEnvironment<GraphQLFieldDefinition> env) {
202+
if (env.getDirective("UpperCase") != null) {
203+
return env.setFieldDataFetcher(DataFetcherFactories.wrapDataFetcher(
204+
env.getFieldDataFetcher(),
205+
((dataFetchingEnv, value) -> {
206+
if (value instanceof String) {
207+
return ((String) value).toUpperCase();
208+
}
209+
else if (value instanceof Mono) {
210+
return ((Mono<String>) value).map(String::toUpperCase);
211+
}
212+
else {
213+
throw new IllegalArgumentException();
214+
}
215+
})));
216+
}
217+
else {
218+
return env.getElement();
219+
}
220+
}
221+
};
222+
223+
BiConsumer<SchemaDirectiveWiring, DataFetcher<?>> tester = (schemaDirectiveWiring, dataFetcher) -> {
224+
225+
GraphQL graphQl = GraphQlSetup.schemaContent(SCHEMA_CONTENT)
226+
.queryFetcher("greeting", dataFetcher)
227+
.runtimeWiring(builder -> builder.directiveWiring(directiveWiring))
228+
.toGraphQl();
229+
230+
ExecutionInput input = ExecutionInput.newExecutionInput().query("{ greeting }").build();
231+
Mono<ExecutionResult> resultMono = Mono.fromFuture(graphQl.executeAsync(input));
232+
233+
String greeting = ResponseHelper.forResult(resultMono).toEntity("greeting", String.class);
234+
assertThat(greeting).isEqualTo("HELLO");
235+
};
236+
237+
tester.accept(directiveWiring, env -> CompletableFuture.completedFuture("hello"));
238+
tester.accept(directiveWiring, env -> Mono.just("hello"));
239+
}
240+
179241
}

0 commit comments

Comments
 (0)