Skip to content

Commit ac3fabb

Browse files
committed
Improve check for duplicate unmapped field
Closes gh-961
1 parent 9bdd869 commit ac3fabb

File tree

3 files changed

+59
-22
lines changed

3 files changed

+59
-22
lines changed

spring-graphql/src/main/java/org/springframework/graphql/execution/SchemaMappingInspector.java

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,10 @@ private void checkSchemaFields() {
145145
private void checkFieldsContainer(
146146
GraphQLFieldsContainer fieldContainer, @Nullable ResolvableType resolvableType) {
147147

148+
if (!this.inspectedTypes.add(fieldContainer.getName())) {
149+
return;
150+
}
151+
148152
String typeName = fieldContainer.getName();
149153
Map<String, DataFetcher> dataFetcherMap = this.dataFetchers.getOrDefault(typeName, Collections.emptyMap());
150154

@@ -196,10 +200,6 @@ private void checkField(
196200

197201
TypePair typePair = TypePair.resolveTypePair(parent, field, resolvableType, this.schema);
198202

199-
if (addAndCheckIfAlreadyInspected(typePair.outputType())) {
200-
return;
201-
}
202-
203203
MultiValueMap<GraphQLType, ResolvableType> typePairs = new LinkedMultiValueMap<>();
204204
if (typePair.outputType() instanceof GraphQLUnionType unionType) {
205205
typePairs.putAll(this.interfaceUnionLookup.resolveUnion(unionType));
@@ -250,10 +250,6 @@ private PropertyDescriptor getProperty(ResolvableType resolvableType, String fie
250250
}
251251
}
252252

253-
private boolean addAndCheckIfAlreadyInspected(GraphQLType type) {
254-
return (type instanceof GraphQLNamedOutputType outputType && !this.inspectedTypes.add(outputType.getName()));
255-
}
256-
257253
private static boolean isNotScalarOrEnumType(GraphQLType type) {
258254
return !(type instanceof GraphQLScalarType || type instanceof GraphQLEnumType);
259255
}

spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorInterfaceTests.java

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.graphql.execution;
1818

1919
import java.util.List;
20+
import java.util.Map;
2021

2122
import org.junit.jupiter.api.Nested;
2223
import org.junit.jupiter.api.Test;
@@ -53,7 +54,7 @@ interface Vehicle {
5354

5455

5556
@Nested
56-
class InterfaceFieldsNotOnJavaInterface {
57+
class UnmappedFields {
5758

5859
@Test
5960
void reportUnmappedFields() {
@@ -83,10 +84,10 @@ List<Vehicle> vehicles() {
8384

8485

8586
@Nested
86-
class GraphQlAndJavaTypeNameMismatch {
87+
class ClassNameAndClassResolver {
8788

8889
@Test
89-
void useClassNameFunction() {
90+
void classNameFunction() {
9091

9192
SchemaReport report = inspectSchema(schema,
9293
initializer -> initializer.classNameFunction(type -> type.getName() + "Impl"),
@@ -100,13 +101,12 @@ void useClassNameFunction() {
100101
}
101102

102103
@Test
103-
void useClassNameTypeResolver() {
104+
void classNameTypeResolver() {
104105

105-
ClassNameTypeResolver typeResolver = new ClassNameTypeResolver();
106-
typeResolver.addMapping(CarImpl.class, "Car");
106+
Map<Class<?>, String> mappings = Map.of(CarImpl.class, "Car");
107107

108108
SchemaReport report = inspectSchema(schema,
109-
initializer -> initializer.classResolver(ClassResolver.create(typeResolver.getMappings())),
109+
initializer -> initializer.classResolver(ClassResolver.create(mappings)),
110110
VehicleController.class);
111111

112112
assertThatReport(report)
@@ -131,6 +131,47 @@ List<Vehicle> vehicles() {
131131
}
132132

133133

134+
@Nested // gh-961
135+
class UnmappedFieldsReportedOnlyOnce {
136+
137+
@Test
138+
void reportUnmappedFields() {
139+
140+
String schema = SchemaMappingInspectorInterfaceTests.schema + """
141+
extend type Query {
142+
cars: [Car]
143+
}
144+
""";
145+
146+
SchemaReport report = inspectSchema(schema, VehicleController.class);
147+
assertThatReport(report)
148+
.hasSkippedTypeCount(0)
149+
.hasUnmappedFieldCount(2)
150+
.containsUnmappedFields("Car", "price", "engineType");
151+
}
152+
153+
interface Vehicle {
154+
String name();
155+
}
156+
record Car(String name) implements Vehicle { }
157+
record Bike(String name, int price) implements Vehicle { }
158+
159+
@Controller
160+
static class VehicleController {
161+
162+
@QueryMapping
163+
List<Vehicle> vehicles() {
164+
throw new UnsupportedOperationException();
165+
}
166+
167+
@QueryMapping
168+
List<Car> cars() {
169+
throw new UnsupportedOperationException();
170+
}
171+
}
172+
}
173+
174+
134175
@Nested
135176
class SkippedTypes {
136177

spring-graphql/src/test/java/org/springframework/graphql/execution/SchemaMappingInspectorUnionTests.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package org.springframework.graphql.execution;
1818

1919
import java.util.List;
20+
import java.util.Map;
2021

2122
import org.junit.jupiter.api.Nested;
2223
import org.junit.jupiter.api.Test;
@@ -48,7 +49,7 @@ public class SchemaMappingInspectorUnionTests extends SchemaMappingInspectorTest
4849

4950

5051
@Nested
51-
class InterfaceFieldsNotOnJavaInterface {
52+
class UnmappedFields {
5253

5354
@Test
5455
void reportUnmappedFieldsByCheckingReturnTypePackage() {
@@ -97,10 +98,10 @@ List<Object> search() {
9798

9899

99100
@Nested
100-
class GraphQlAndJavaTypeNameMismatch {
101+
class ClassNameAndClassResolver {
101102

102103
@Test
103-
void useClassNameFunction() {
104+
void classNameFunction() {
104105

105106
SchemaReport report = inspectSchema(schema,
106107
initializer -> initializer.classNameFunction(type -> type.getName() + "Impl"),
@@ -114,13 +115,12 @@ void useClassNameFunction() {
114115
}
115116

116117
@Test
117-
void useClassNameTypeResolver() {
118+
void classNameTypeResolver() {
118119

119-
ClassNameTypeResolver typeResolver = new ClassNameTypeResolver();
120-
typeResolver.addMapping(PhotoImpl.class, "Photo");
120+
Map<Class<?>, String> mappings = Map.of(PhotoImpl.class, "Photo");
121121

122122
SchemaReport report = inspectSchema(schema,
123-
initializer -> initializer.classResolver(ClassResolver.create(typeResolver.getMappings())),
123+
initializer -> initializer.classResolver(ClassResolver.create(mappings)),
124124
SearchController.class);
125125

126126
assertThatReport(report)

0 commit comments

Comments
 (0)