Skip to content

Commit d113831

Browse files
rstoyanchevrstoyanchev
committed
Improve "Connection" schema type checks
ConnectionFieldTypeVisitor now checks the complete structure of Connection, Edge, and PageInfo schema types before decorating a DataFetcher. See gh-709
1 parent 095d720 commit d113831

File tree

2 files changed

+142
-19
lines changed

2 files changed

+142
-19
lines changed

‎spring-graphql/src/main/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitor.java‎

Lines changed: 49 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@
3434
import graphql.schema.GraphQLCodeRegistry;
3535
import graphql.schema.GraphQLFieldDefinition;
3636
import graphql.schema.GraphQLFieldsContainer;
37+
import graphql.schema.GraphQLList;
3738
import graphql.schema.GraphQLNonNull;
3839
import graphql.schema.GraphQLObjectType;
40+
import graphql.schema.GraphQLOutputType;
3941
import graphql.schema.GraphQLSchemaElement;
4042
import graphql.schema.GraphQLType;
4143
import graphql.schema.GraphQLTypeVisitorStub;
@@ -101,14 +103,54 @@ public TraversalControl visitGraphQLFieldDefinition(
101103
return TraversalControl.CONTINUE;
102104
}
103105

104-
private static boolean isConnectionField(GraphQLFieldDefinition fieldDefinition) {
105-
GraphQLType returnType = fieldDefinition.getType();
106-
if (returnType instanceof GraphQLNonNull nonNullType) {
107-
returnType = nonNullType.getWrappedType();
106+
private static boolean isConnectionField(GraphQLFieldDefinition field) {
107+
GraphQLObjectType type = getAsObjectType(field);
108+
if (type == null || !type.getName().endsWith("Connection")) {
109+
return false;
108110
}
109-
return (returnType instanceof GraphQLObjectType objectType &&
110-
objectType.getName().endsWith("Connection") &&
111-
objectType.getField("pageInfo") != null);
111+
112+
GraphQLObjectType edgeType = getEdgeType(type.getField("edges"));
113+
if (edgeType == null || !edgeType.getName().endsWith("Edge")) {
114+
return false;
115+
}
116+
if (edgeType.getField("node") == null || edgeType.getField("cursor") == null) {
117+
return false;
118+
}
119+
120+
GraphQLObjectType pageInfoType = getAsObjectType(type.getField("pageInfo"));
121+
if (pageInfoType == null || !pageInfoType.getName().equals("PageInfo")) {
122+
return false;
123+
}
124+
if (pageInfoType.getField("hasPreviousPage") == null || pageInfoType.getField("hasNextPage") == null ||
125+
pageInfoType.getField("startCursor") == null || pageInfoType.getField("endCursor") == null) {
126+
return false;
127+
}
128+
129+
return true;
130+
}
131+
132+
@Nullable
133+
private static GraphQLObjectType getAsObjectType(@Nullable GraphQLFieldDefinition field) {
134+
return (getType(field) instanceof GraphQLObjectType type ? type : null);
135+
}
136+
137+
@Nullable
138+
private static GraphQLObjectType getEdgeType(@Nullable GraphQLFieldDefinition field) {
139+
if (getType(field) instanceof GraphQLList listType) {
140+
if (listType.getWrappedType() instanceof GraphQLObjectType type) {
141+
return type;
142+
}
143+
}
144+
return null;
145+
}
146+
147+
@Nullable
148+
private static GraphQLType getType(@Nullable GraphQLFieldDefinition field) {
149+
if (field == null) {
150+
return null;
151+
}
152+
GraphQLOutputType type = field.getType();
153+
return (type instanceof GraphQLNonNull nonNullType ? nonNullType.getWrappedType() : type);
112154
}
113155

114156

‎spring-graphql/src/test/java/org/springframework/graphql/data/pagination/ConnectionFieldTypeVisitorTests.java‎

Lines changed: 93 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,14 +19,28 @@
1919
import java.util.Collection;
2020
import java.util.List;
2121

22+
import graphql.schema.DataFetcher;
23+
import graphql.schema.FieldCoordinates;
24+
import graphql.schema.GraphQLFieldDefinition;
25+
import graphql.schema.GraphQLSchema;
2226
import graphql.schema.PropertyDataFetcher;
27+
import graphql.schema.SchemaTransformer;
28+
import graphql.schema.idl.RuntimeWiring;
29+
import graphql.schema.idl.SchemaGenerator;
30+
import graphql.schema.idl.SchemaParser;
31+
import graphql.schema.idl.TypeDefinitionRegistry;
32+
import org.junit.jupiter.api.Nested;
2333
import org.junit.jupiter.api.Test;
2434
import reactor.core.publisher.Mono;
2535

36+
import org.springframework.core.io.Resource;
2637
import org.springframework.graphql.BookSource;
2738
import org.springframework.graphql.ExecutionGraphQlResponse;
2839
import org.springframework.graphql.GraphQlSetup;
2940
import org.springframework.graphql.ResponseHelper;
41+
import org.springframework.graphql.execution.ConnectionTypeDefinitionConfigurer;
42+
43+
import static org.assertj.core.api.Assertions.assertThat;
3044

3145
/**
3246
* Unit tests for {@link ConnectionFieldTypeVisitor}.
@@ -69,18 +83,6 @@ void paginationDataFetcher() {
6983
);
7084
}
7185

72-
@Test // gh-707
73-
void trivialDataFetcherIsSkipped() {
74-
75-
Mono<ExecutionGraphQlResponse> response = GraphQlSetup.schemaResource(BookSource.paginationSchema)
76-
.dataFetcher("Query", "books", new PropertyDataFetcher<>("books"))
77-
.connectionSupport(new ListConnectionAdapter())
78-
.toGraphQlService()
79-
.execute(BookSource.booksConnectionQuery(null));
80-
81-
ResponseHelper.forResponse(response).assertData("{\"books\":null}");
82-
}
83-
8486
@Test // gh-707
8587
void nullValueTreatedAsEmptyConnection() {
8688

@@ -103,6 +105,85 @@ void nullValueTreatedAsEmptyConnection() {
103105
}
104106

105107

108+
@Nested
109+
class DecorationTests {
110+
111+
@Test // gh-707
112+
void trivialDataFetcherIsNotDecorated() throws Exception {
113+
114+
FieldCoordinates coordinates = FieldCoordinates.coordinates("Query", "books");
115+
DataFetcher<?> dataFetcher = new PropertyDataFetcher<>("books");
116+
117+
DataFetcher<?> actual =
118+
applyConnectionFieldTypeVisitor(BookSource.paginationSchema, coordinates, dataFetcher);
119+
120+
assertThat(actual).isSameAs(dataFetcher);
121+
}
122+
123+
@Test // gh-709
124+
void connectionTypeWithoutEdgesIsNotDecorated() throws Exception {
125+
126+
String schemaContent = """
127+
type Query {
128+
puzzles: PuzzleConnection
129+
}
130+
type PuzzleConnection {
131+
pageInfo: PuzzlePageInfo!
132+
puzzles: [PuzzleEdge]
133+
}
134+
type PuzzlePageInfo {
135+
total: Int!
136+
}
137+
type PuzzleEdge {
138+
puzzle: Puzzle
139+
cursor: String!
140+
}
141+
type Puzzle {
142+
title: String!
143+
}
144+
""";
145+
146+
FieldCoordinates coordinates = FieldCoordinates.coordinates("Query", "puzzles");
147+
DataFetcher<?> dataFetcher = env -> null;
148+
149+
DataFetcher<?> actual = applyConnectionFieldTypeVisitor(schemaContent, coordinates, dataFetcher);
150+
151+
assertThat(actual).isSameAs(dataFetcher);
152+
}
153+
154+
private static DataFetcher<?> applyConnectionFieldTypeVisitor(
155+
Object schemaSource, FieldCoordinates coordinates, DataFetcher<?> fetcher) throws Exception {
156+
157+
TypeDefinitionRegistry registry;
158+
if (schemaSource instanceof Resource resource) {
159+
registry = new SchemaParser().parse(resource.getInputStream());
160+
}
161+
else if (schemaSource instanceof String schemaContent) {
162+
registry = new SchemaParser().parse(schemaContent);
163+
}
164+
else {
165+
throw new IllegalArgumentException();
166+
}
167+
168+
new ConnectionTypeDefinitionConfigurer().configure(registry);
169+
170+
GraphQLSchema schema = new SchemaGenerator().makeExecutableSchema(
171+
registry, RuntimeWiring.newRuntimeWiring()
172+
.type(coordinates.getTypeName(), b -> b.dataFetcher(coordinates.getFieldName(), fetcher))
173+
.build());
174+
175+
ConnectionFieldTypeVisitor visitor =
176+
ConnectionFieldTypeVisitor.create(List.of(new ListConnectionAdapter()));
177+
178+
schema = new SchemaTransformer().transform(schema, visitor);
179+
GraphQLFieldDefinition field = schema.getFieldDefinition(coordinates);
180+
return schema.getCodeRegistry().getDataFetcher(coordinates, field);
181+
}
182+
183+
}
184+
185+
186+
106187
private static class ListConnectionAdapter implements ConnectionAdapter {
107188

108189
private int initialOffset = 0;

0 commit comments

Comments
 (0)