Fix smallrye#700, fix smallrye#701 Check modifiers while considering …

…a field and/or method as a graphql field.

common/schema-builder/src/main/java/io/smallrye/graphql/schema/creator/FieldCreator.java

@@ -43,7 +43,7 @@ public FieldCreator(ReferenceCreator referenceCreator) {
     public Optional<Field> createFieldForInterface(MethodInfo methodInfo, Reference parentObjectReference) {
         Annotations annotationsForMethod = Annotations.getAnnotationsForInterfaceField(methodInfo);
 
-        if (!IgnoreHelper.shouldIgnore(annotationsForMethod)) {
+        if (isGraphQlField(Direction.OUT, null, methodInfo)) {
             Type returnType = methodInfo.returnType();
 
             // Name
@@ -98,7 +98,7 @@ public Optional<Field> createFieldForPojo(Direction direction, FieldInfo fieldIn
             Reference parentObjectReference) {
         Annotations annotationsForPojo = Annotations.getAnnotationsForPojo(direction, fieldInfo, methodInfo);
 
-        if (!IgnoreHelper.shouldIgnore(annotationsForPojo, fieldInfo)) {
+        if (isGraphQlField(direction, fieldInfo, methodInfo)) {
             Type methodType = getMethodType(methodInfo, direction);
 
             // Name
@@ -151,53 +151,115 @@ public Optional<Field> createFieldForPojo(Direction direction, FieldInfo fieldIn
      * @return a Field model object
      */
     public Optional<Field> createFieldForPojo(Direction direction, FieldInfo fieldInfo, Reference parentObjectReference) {
-        if (Modifier.isPublic(fieldInfo.flags())) {
+        if (isGraphQlField(direction, fieldInfo, null)) {
             Annotations annotationsForPojo = Annotations.getAnnotationsForPojo(direction, fieldInfo);
 
-            if (!IgnoreHelper.shouldIgnore(annotationsForPojo, fieldInfo)) {
-
-                // Name
-                String name = getFieldName(direction, annotationsForPojo, fieldInfo.name());
+            // Name
+            String name = getFieldName(direction, annotationsForPojo, fieldInfo.name());
 
-                // Field Type
-                Type fieldType = fieldInfo.type();
+            // Field Type
+            Type fieldType = fieldInfo.type();
 
-                // Description
-                Optional<String> maybeDescription = DescriptionHelper.getDescriptionForField(annotationsForPojo, fieldType);
+            // Description
+            Optional<String> maybeDescription = DescriptionHelper.getDescriptionForField(annotationsForPojo, fieldType);
 
-                Reference reference = referenceCreator.createReferenceForPojoField(direction, fieldType, fieldType,
-                        annotationsForPojo, parentObjectReference);
+            Reference reference = referenceCreator.createReferenceForPojoField(direction, fieldType, fieldType,
+                    annotationsForPojo, parentObjectReference);
 
-                Field field = new Field(fieldInfo.name(),
-                        MethodHelper.getPropertyName(direction, fieldInfo.name()),
-                        name,
-                        maybeDescription.orElse(null),
-                        reference);
+            Field field = new Field(fieldInfo.name(),
+                    MethodHelper.getPropertyName(direction, fieldInfo.name()),
+                    name,
+                    maybeDescription.orElse(null),
+                    reference);
 
-                // NotNull
-                if (NonNullHelper.markAsNonNull(fieldType, annotationsForPojo)) {
-                    field.setNotNull(true);
-                }
+            // NotNull
+            if (NonNullHelper.markAsNonNull(fieldType, annotationsForPojo)) {
+                field.setNotNull(true);
+            }
 
-                // Wrapper
-                field.setWrapper(WrapperCreator.createWrapper(fieldType).orElse(null));
+            // Wrapper
+            field.setWrapper(WrapperCreator.createWrapper(fieldType).orElse(null));
 
-                // TransformInfo
-                field.setTransformation(FormatHelper.getFormat(fieldType, annotationsForPojo).orElse(null));
+            // TransformInfo
+            field.setTransformation(FormatHelper.getFormat(fieldType, annotationsForPojo).orElse(null));
 
-                // MappingInfo
-                field.setMapping(MappingHelper.getMapping(field, annotationsForPojo).orElse(null));
+            // MappingInfo
+            field.setMapping(MappingHelper.getMapping(field, annotationsForPojo).orElse(null));
 
-                // Default Value
-                field.setDefaultValue(DefaultValueHelper.getDefaultValue(annotationsForPojo).orElse(null));
+            // Default Value
+            field.setDefaultValue(DefaultValueHelper.getDefaultValue(annotationsForPojo).orElse(null));
 
-                return Optional.of(field);
-            }
-            return Optional.empty();
+            return Optional.of(field);
         }
         return Optional.empty();
     }
 
+    /**
+     * Checks if method and/or field are useable as a GraphQL-Field.
+     *
+     * @param direction the direction, IN if the field should be used on an input type, OUT otherwise
+     * @param fieldInfo the field. If null, methodInfo must be provided
+     * @param methodInfo the method. If null, fieldInfo must be provided
+     * @return if it is an GraphQL field
+     */
+    protected static boolean isGraphQlField(Direction direction, FieldInfo fieldInfo, MethodInfo methodInfo) {
+        boolean methodAccessible = isPossibleField(methodInfo);
+        boolean fieldAccessible = isPossibleField(direction, fieldInfo);
+
+        if (!methodAccessible && !fieldAccessible) {
+            return false;
+        }
+
+        Annotations annotationsForPojo = Annotations.getAnnotationsForPojo(direction, fieldInfo, methodInfo);
+        if (IgnoreHelper.shouldIgnore(annotationsForPojo, fieldInfo)) {
+            return false;
+        }
+
+        return true;
+    }
+
+    /**
+     * Checks if the method is a possible GraphQl field (by method access).
+     * This means that the method:
+     * <ul>
+     * </ul>
+     * <li>exists</li>
+     * <li>is public</li>
+     * <li>and is not static</li>
+     * </ul>
+     * 
+     * @param methodInfo the method
+     * @return if the method is a possible GraphQl field
+     */
+    private static boolean isPossibleField(MethodInfo methodInfo) {
+        return methodInfo != null
+                && Modifier.isPublic(methodInfo.flags())
+                && !Modifier.isStatic(methodInfo.flags());
+    }
+
+    /**
+     * Checks if the field is a possible GraphQl field (by field access).
+     *
+     * This means that the field:
+     * <ul>
+     * </ul>
+     * <li>exists</li>
+     * <li>is public</li>
+     * <li>is not static</li>
+     * <li>and is not final, if it's an input field</li>
+     * </ul>
+     * 
+     * @param direction the direction
+     * @param fieldInfo the method
+     * @return if the field is a possible GraphQl field
+     */
+    private static boolean isPossibleField(Direction direction, FieldInfo fieldInfo) {
+        return fieldInfo != null
+                && !(direction == Direction.IN && Modifier.isFinal(fieldInfo.flags()))
+                && Modifier.isPublic(fieldInfo.flags())
+                && !Modifier.isStatic(fieldInfo.flags());
+    }
+
     private static void validateFieldType(Direction direction, MethodInfo methodInfo) {
         Type returnType = methodInfo.returnType();
         if (direction.equals(Direction.OUT) && returnType.kind().equals(Type.Kind.VOID)) {

common/schema-builder/src/test/java/io/smallrye/graphql/schema/creator/FieldCreatorTest.java

@@ -0,0 +1,138 @@
+package io.smallrye.graphql.schema.creator;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.io.IOException;
+import java.util.Optional;
+
+import org.jboss.jandex.*;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
+
+import io.smallrye.graphql.schema.IndexCreator;
+import io.smallrye.graphql.schema.helper.Direction;
+import io.smallrye.graphql.schema.helper.TypeAutoNameStrategy;
+import io.smallrye.graphql.schema.model.Field;
+import io.smallrye.graphql.schema.model.Reference;
+
+class FieldCreatorTest {
+
+    private final ReferenceCreator referenceCreator = new ReferenceCreator(TypeAutoNameStrategy.Default);
+    private final FieldCreator fieldCreator = new FieldCreator(referenceCreator);
+
+    /**
+     * Tests the visibility of fields with certain field- and method-modifier combinations.
+     *
+     * @param direction the direction of the graphql-field to test
+     * @param fieldName the field name, may be null or empty if just a method is provided
+     * @param methodName the method name, may be null or empty if just a method is provided
+     * @param expected the expected visibility
+     */
+    @ParameterizedTest
+    @CsvSource(value = {
+            " IN, PUBLIC_STATIC_FIELD, , false",
+            " IN, PUBLIC_STATIC_FIELD, setField, true",
+            " IN, privateField, ,false",
+            " IN, publicFinalField, , false",
+            " IN, publicFinalField, setField, true",
+            " IN, publicField, , true",
+            " IN, publicTransientField, , false",
+            " IN, publicTransientField, setField, false",
+            " IN, ,setStaticField, false",
+
+            "OUT, PUBLIC_STATIC_FIELD, , false",
+            "OUT, PUBLIC_STATIC_FIELD, getField, true",
+            "OUT, privateField, , false",
+            "OUT, publicFinalField, , true",
+            "OUT, publicField, , true",
+            "OUT, publicTransientField, , false",
+            "OUT, publicTransientField, getField, false",
+            "OUT, ,getStaticField, false",
+    })
+    void testVisibility(Direction direction, String fieldName, String methodName, boolean expected) throws IOException {
+        MethodInfo method = getMethod(methodName);
+        FieldInfo fieldInfo = getField(fieldName);
+
+        final boolean isVisible = FieldCreator.isGraphQlField(direction, fieldInfo, method);
+        assertEquals(expected, isVisible);
+    }
+
+    @ParameterizedTest
+    @CsvSource(value = {
+            " IN, PUBLIC_STATIC_FIELD, false",
+            " IN, privateField, false",
+            " IN, publicFinalField, false",
+            " IN, publicField, true",
+            "OUT, PUBLIC_STATIC_FIELD, false",
+            "OUT, privateField, false",
+            "OUT, publicFinalField, true",
+            "OUT, publicField, true",
+    })
+    void testFieldsWithoutMethod(Direction direction, String fieldName, boolean expected) throws IOException {
+        FieldInfo field = getField(fieldName);
+
+        final Optional<Field> fieldForPojo = fieldCreator.createFieldForPojo(direction, field, new Reference());
+
+        assertEquals(expected, fieldForPojo.isPresent());
+    }
+
+    @ParameterizedTest
+    @CsvSource(value = {
+            "OUT, getStaticField, false",
+            " IN, setStaticField, false",
+    })
+    void testMethodsWithoutField(Direction direction, String methodName, boolean expected) throws IOException {
+        MethodInfo method = getMethod(methodName);
+
+        final Optional<Field> fieldForPojo = fieldCreator.createFieldForPojo(direction, null, method, new Reference());
+
+        assertEquals(expected, fieldForPojo.isPresent());
+    }
+
+    @ParameterizedTest
+    @CsvSource(value = {
+            "OUT, privateField, getField, true",
+            " IN, privateField, setField, true",
+    })
+    void testMethodsWithField(Direction direction, String fieldName, String methodName, boolean expected) throws IOException {
+        MethodInfo method = getMethod(methodName);
+        FieldInfo fieldInfo = getField(fieldName);
+
+        final boolean isVisible = FieldCreator.isGraphQlField(direction, fieldInfo, method);
+        assertEquals(expected, isVisible);
+        //final Optional<Field> fieldForPojo = fieldCreator.createFieldForPojo(direction, null, method, new Reference());
+        //assertEquals(expected, fieldForPojo.isPresent());
+    }
+
+    @ParameterizedTest
+    @CsvSource(value = {
+            "OUT, getStaticField, false",
+    })
+    void testInterfaceMethods(Direction direction, String methodName, boolean expected) throws IOException {
+        MethodInfo method = getMethod(methodName);
+
+        final Optional<Field> fieldForPojo = fieldCreator.createFieldForInterface(method, new Reference());
+        assertEquals(expected, fieldForPojo.isPresent());
+    }
+
+    private static FieldInfo getField(String name) throws IOException {
+        if (name == null || name.isEmpty()) {
+            return null;
+        }
+        Index complete = IndexCreator.index(SimplePojo.class);
+
+        ClassInfo classByName = complete.getClassByName(DotName.createSimple(SimplePojo.class.getName()));
+        return classByName.field(name);
+    }
+
+    private static MethodInfo getMethod(String name) throws IOException {
+        if (name == null || name.isEmpty()) {
+            return null;
+        }
+        Index complete = IndexCreator.index(SimplePojo.class);
+
+        ClassInfo classByName = complete.getClassByName(DotName.createSimple(SimplePojo.class.getName()));
+        return classByName.firstMethod(name);
+    }
+
+}

common/schema-builder/src/test/java/io/smallrye/graphql/schema/creator/SimplePojo.java

@@ -0,0 +1,42 @@
+package io.smallrye.graphql.schema.creator;
+
+/**
+ * Used to Test FieldCreator.
+ *
+ * Just contains some fields and methods with different modifiers.
+ */
+public class SimplePojo {
+
+    public transient String publicTransientField = "";
+
+    public static String PUBLIC_STATIC_FIELD = "";
+
+    public final String publicFinalField = "";
+
+    public String publicField = "";
+
+    private String privateField = "";
+
+    public static String getStaticField() {
+        return "";
+    }
+
+    public static void setStaticField(String s) {
+    }
+
+    public void setPrivateField(final String privateField) {
+        this.privateField = privateField;
+    }
+
+    public String getPrivateField() {
+        return privateField;
+    }
+
+    public String getField() {
+        return publicTransientField;
+    }
+
+    public void setField(final String publicTransientField) {
+        this.publicTransientField = publicTransientField;
+    }
+}