Avoid fields redefinition in extensions

Author: Thomas Draier

src/main/java/graphql/annotations/GraphQLAnnotations.java

@@ -149,14 +149,17 @@ public GraphQLInterfaceType.Builder getIfaceBuilder(Class<?> iface) throws Graph
         if (description != null) {
             builder.description(description.value());
         }
+        List<String> definedFields = new ArrayList<>();
         for (Method method : getOrderedMethods(iface)) {
             boolean valid = !Modifier.isStatic(method.getModifiers()) &&
                     method.getAnnotation(GraphQLField.class) != null;
             if (valid) {
-                builder.field(getField(method));
+                GraphQLFieldDefinition gqlField = getField(method);
+                definedFields.add(gqlField.getName());
+                builder.field(gqlField);
             }
         }
-        builder.fields(getExtensionFields(iface));
+        builder.fields(getExtensionFields(iface, definedFields));
 
         GraphQLTypeResolver typeResolver = iface.getAnnotation(GraphQLTypeResolver.class);
         builder.typeResolver(newInstance(typeResolver.value()));
@@ -292,13 +295,15 @@ public GraphQLObjectType.Builder getObjectBuilder(Class<?> object) throws GraphQ
         if (description != null) {
             builder.description(description.value());
         }
-
+        List<String> fieldsDefined = new ArrayList<>();
         for (Method method : getOrderedMethods(object)) {
             if (method.isBridge() || method.isSynthetic()) {
                 continue;
             }
             if (breadthFirstSearch(method)) {
-                builder.field(getField(method));
+                GraphQLFieldDefinition gqlField = getField(method);
+                fieldsDefined.add(gqlField.getName());
+                builder.field(gqlField);
             }
         }
 
@@ -307,23 +312,25 @@ public GraphQLObjectType.Builder getObjectBuilder(Class<?> object) throws GraphQ
                 continue;
             }
             if (parentalSearch(field)) {
-                builder.field(getField(field));
+                GraphQLFieldDefinition gqlField = getField(field);
+                fieldsDefined.add(gqlField.getName());
+                builder.field(gqlField);
             }
         }
 
         for (Class<?> iface : object.getInterfaces()) {
             if (iface.getAnnotation(GraphQLTypeResolver.class) != null) {
                 builder.withInterface((GraphQLInterfaceType) getInterface(iface));
-                builder.fields(getExtensionFields(iface));
+                builder.fields(getExtensionFields(iface, fieldsDefined));
             }
         }
 
-        builder.fields(getExtensionFields(object));
+        builder.fields(getExtensionFields(object, fieldsDefined));
 
         return builder;
     }
 
-    private List<GraphQLFieldDefinition> getExtensionFields(Class<?> object) {
+    private List<GraphQLFieldDefinition> getExtensionFields(Class<?> object, List<String> fieldsDefined) {
         List<GraphQLFieldDefinition> fields = new ArrayList<>();
         if (extensionsTypeRegistry.containsKey(object)) {
             for (Class<?> aClass : extensionsTypeRegistry.get(object)) {
@@ -332,22 +339,31 @@ private List<GraphQLFieldDefinition> getExtensionFields(Class<?> object) {
                         continue;
                     }
                     if (breadthFirstSearch(method)) {
-                        fields.add(getField(method));
+                        addExtensionField(getField(method), fields, fieldsDefined);
                     }
                 }
                 for (Field field : getAllFields(aClass).values()) {
                     if (Modifier.isStatic(field.getModifiers())) {
                         continue;
                     }
                     if (parentalSearch(field)) {
-                        fields.add(getField(field));
+                        addExtensionField(getField(field), fields, fieldsDefined);
                     }
                 }
             }
         }
         return fields;
     }
 
+    private void addExtensionField(GraphQLFieldDefinition gqlField, List<GraphQLFieldDefinition> fields, List<String> fieldsDefined) {
+        if (!fieldsDefined.contains(gqlField.getName())) {
+            fieldsDefined.add(gqlField.getName());
+            fields.add(gqlField);
+        } else {
+            throw new GraphQLAnnotationsException("Duplicate field found in extension : " + gqlField.getName(), null);
+        }
+    }
+
     public static GraphQLObjectType.Builder objectBuilder(Class<?> object) throws GraphQLAnnotationsException {
         return getInstance().getObjectBuilder(object);
     }
@@ -459,7 +475,7 @@ protected GraphQLFieldDefinition getField(Field field) throws GraphQLAnnotations
 
     private DataFetcher constructDataFetcher(String fieldName, GraphQLDataFetcher annotatedDataFetcher) {
         final String[] args;
-        if ( annotatedDataFetcher.firstArgIsTargetName() ) {
+        if (annotatedDataFetcher.firstArgIsTargetName()) {
             args = Stream.concat(Stream.of(fieldName), stream(annotatedDataFetcher.args())).toArray(String[]::new);
         } else {
             args = annotatedDataFetcher.args();

src/test/java/graphql/annotations/GraphQLExtensionsTest.java

@@ -17,6 +17,7 @@
 import graphql.ExecutionResult;
 import graphql.GraphQL;
 import graphql.schema.*;
+import org.testng.Assert;
 import org.testng.annotations.AfterTest;
 import org.testng.annotations.BeforeTest;
 import org.testng.annotations.Test;
@@ -26,7 +27,7 @@
 
 import static graphql.Scalars.GraphQLString;
 import static graphql.schema.GraphQLSchema.newSchema;
-import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.*;
 
 public class GraphQLExtensionsTest {
 
@@ -70,6 +71,20 @@ public String getField5() {
         }
     }
 
+    @GraphQLTypeExtension(GraphQLExtensionsTest.TestObject.class)
+    public static class TestObjectExtensionInvalid {
+        private TestObject obj;
+
+        public TestObjectExtensionInvalid(TestObject obj) {
+            this.obj = obj;
+        }
+
+        @GraphQLField
+        public String getField() {
+            return "invalid";
+        }
+    }
+
     public static class TestDataFetcher implements DataFetcher {
         @Override
         public Object get(DataFetchingEnvironment environment) {
@@ -113,4 +128,11 @@ public void values() {
         assertEquals(data.get("field5"), "test test5");
     }
 
+    @Test
+    public void testDuplicateField() {
+        GraphQLAnnotations.getInstance().registerTypeExtension(TestObjectExtensionInvalid.class);
+        GraphQLAnnotationsException e = expectThrows(GraphQLAnnotationsException.class, () -> GraphQLAnnotations.object(TestObject.class));
+        assertTrue(e.getMessage().startsWith("Duplicate field"));
+        GraphQLAnnotations.getInstance().unregisterTypeExtension(TestObjectExtensionInvalid.class);
+    }
 }