From 7fcc6a7988295d2558188d5f17dadfb8ac943e0f Mon Sep 17 00:00:00 2001
From: Bret Patterson <bret@bretpatterson.com>
Date: Sat, 3 Sep 2016 09:23:33 -0500
Subject: [PATCH] Fix duplicate field name error when inheriting from base
 class, or overriding field/method from base class

---
 .../graphql/impl/GraphQLObjectMapper.java     | 31 ++++++++++++++++---
 .../graphql/impl/GraphQLObjectMapperTest.java | 23 ++++++++++++++
 2 files changed, 49 insertions(+), 5 deletions(-)

diff --git a/src/main/java/com/bretpatterson/schemagen/graphql/impl/GraphQLObjectMapper.java b/src/main/java/com/bretpatterson/schemagen/graphql/impl/GraphQLObjectMapper.java
index 772e97d..fd75458 100644
--- a/src/main/java/com/bretpatterson/schemagen/graphql/impl/GraphQLObjectMapper.java
+++ b/src/main/java/com/bretpatterson/schemagen/graphql/impl/GraphQLObjectMapper.java
@@ -26,10 +26,13 @@
 import com.bretpatterson.schemagen.graphql.utils.AnnotationUtils;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
+import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import graphql.Scalars;
@@ -64,6 +67,8 @@
 import java.util.Set;
 import java.util.Stack;
 
+import static java.util.Collections.addAll;
+
 /**
  * This is the meat of the schema gen package. Utilizing the configured properties it will traverse the objects provided and generate a type
  * hierarchy for GraphQL.
@@ -347,6 +352,10 @@ private DataFetcher addTypeConverter(Optional<GraphQLTypeConverter> typeConverte
 	public Optional<GraphQLFieldDefinition> getFieldType(Type type, Field field, Optional<Object> targetObject, Optional<String> providedFieldName) {
 		String fieldName = providedFieldName.isPresent() ? providedFieldName.get() : field.getName();
 
+		if (Modifier.isAbstract(field.getModifiers())) {
+			LOGGER.info("Ignoring types {} abstract field {}  ", type, field);
+			return Optional.absent();
+		}
 		if (Modifier.isStatic(field.getModifiers())) {
 			LOGGER.info("Ignoring types {} static field {}  ", type, field);
 			return Optional.absent();
@@ -567,6 +576,7 @@ private GraphQLObjectType buildObject(Type type, Class<?> classType) {
 			Optional<GraphQLDescription> maybeGraphqlDesc = Optional.fromNullable(classItem.getAnnotation(GraphQLDescription.class));
 			Optional<IQueryFactory> queryFactory = Optional.absent();
 			Object objectInstance = null;
+			Set<String> fieldNames = Sets.newHashSet();
 
 			if (maybeGraphqlDesc.isPresent()) {
 				glType.description(maybeGraphqlDesc.get().value());
@@ -593,7 +603,22 @@ private GraphQLObjectType buildObject(Type type, Class<?> classType) {
 			}
 			// end when there are no more super classes and while ignore java.* types
 			while (classItem != null && !classPackage.startsWith("java.")) {
-				fields.addAll(getGraphQLFieldDefinitions(Optional.absent(), type, classItem, Optional.<List<Field>>absent(), Optional.<List<Method>>absent()));
+				Collection<GraphQLFieldDefinition> newFields;
+				if (queryFactory.isPresent()) {
+					newFields = queryFactory.get().newMethodQueriesForObject(this, objectInstance);
+				} else {
+					newFields = getGraphQLFieldDefinitions(Optional.absent(), type, classItem, Optional.<List<Field>>absent(), Optional.<List<Method>>absent());
+				}
+				if (newFields != null) {
+					for (GraphQLFieldDefinition f : newFields) {
+						if (!fieldNames.contains(f.getName())) {
+							fieldNames.add(f.getName());
+							fields.add(f);
+						} else {
+							LOGGER.info("Ignoring duplicate field {} from type {}", f.getName(), classItem.getName());
+						}
+					}
+				}
 
 				// pop currentContext
 				classItem = classItem.getSuperclass();
@@ -602,10 +627,6 @@ private GraphQLObjectType buildObject(Type type, Class<?> classType) {
 				} else {
 					classPackage = "";
 				}
-
-				if (queryFactory.isPresent()) {
-					fields.addAll(queryFactory.get().newMethodQueriesForObject(this, objectInstance));
-				}
 			}
 
 			// exiting context of current type arguments if we processed a generic type
diff --git a/src/test/java/com/bretpatterson/schemagen/graphql/impl/GraphQLObjectMapperTest.java b/src/test/java/com/bretpatterson/schemagen/graphql/impl/GraphQLObjectMapperTest.java
index 3fc1f5a..a5d8ee2 100644
--- a/src/test/java/com/bretpatterson/schemagen/graphql/impl/GraphQLObjectMapperTest.java
+++ b/src/test/java/com/bretpatterson/schemagen/graphql/impl/GraphQLObjectMapperTest.java
@@ -521,4 +521,27 @@ public void testDocumentedAttributes() {
 		assertEquals("The field description", objectType.getFieldDefinition("field").getDescription());
 		assertEquals("The method description", objectType.getFieldDefinition("value").getDescription());
 	}
+
+
+	public abstract class AbstractBaseB {
+		protected int field1 = 1;
+
+		public abstract int getField1();
+	}
+
+	public class SubB extends AbstractBaseB {
+		public int getField1() { return field1; }
+	}
+
+	@Test
+	public void testAbstractMethods() {
+		IGraphQLObjectMapper graphQLObjectMapper = newGraphQLObjectMapper(
+				ImmutableList.<IGraphQLTypeMapper> builder().addAll(GraphQLSchemaBuilder.getDefaultTypeMappers()).build());
+		GraphQLObjectType objectType = (GraphQLObjectType) graphQLObjectMapper.getOutputType(new TypeToken<SubB>() {
+		}.getType());
+
+		assertNotNull(objectType.getFieldDefinition("field1"));
+		assertEquals("field1", objectType.getFieldDefinition("field1").getName());
+		assertEquals(Scalars.GraphQLInt, objectType.getFieldDefinition("field1").getType());
+	}
 }