diff --git a/common/schema-builder/src/main/java/io/smallrye/graphql/schema/creator/FieldCreator.java b/common/schema-builder/src/main/java/io/smallrye/graphql/schema/creator/FieldCreator.java index 6f4ff5543..bae9fd856 100644 --- a/common/schema-builder/src/main/java/io/smallrye/graphql/schema/creator/FieldCreator.java +++ b/common/schema-builder/src/main/java/io/smallrye/graphql/schema/creator/FieldCreator.java @@ -43,7 +43,7 @@ public FieldCreator(ReferenceCreator referenceCreator) { public Optional 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 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 createFieldForPojo(Direction direction, FieldInfo fieldIn * @return a Field model object */ public Optional 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 maybeDescription = DescriptionHelper.getDescriptionForField(annotationsForPojo, fieldType); + // Description + Optional 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: + *
    + *
+ *
  • exists
    • + *
  • is public
    • + *
  • and is not static
    • + * + * + * @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: + *
      + *
    + *
  • exists
    • + *
  • is public
    • + *
  • is not static
    • + *
  • and is not final, if it's an input field
    • + * + * + * @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)) { diff --git a/common/schema-builder/src/test/java/io/smallrye/graphql/schema/creator/FieldCreatorTest.java b/common/schema-builder/src/test/java/io/smallrye/graphql/schema/creator/FieldCreatorTest.java new file mode 100644 index 000000000..7011c3ce4 --- /dev/null +++ b/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 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 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 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 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); + } + +} diff --git a/common/schema-builder/src/test/java/io/smallrye/graphql/schema/creator/SimplePojo.java b/common/schema-builder/src/test/java/io/smallrye/graphql/schema/creator/SimplePojo.java new file mode 100644 index 000000000..5a58a4690 --- /dev/null +++ b/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; + } +} diff --git a/server/tck/src/test/java/io/smallrye/graphql/SmallRyeGraphQLArchiveProcessor.java b/server/tck/src/test/java/io/smallrye/graphql/SmallRyeGraphQLArchiveProcessor.java index ed34f1be7..5a228bef8 100644 --- a/server/tck/src/test/java/io/smallrye/graphql/SmallRyeGraphQLArchiveProcessor.java +++ b/server/tck/src/test/java/io/smallrye/graphql/SmallRyeGraphQLArchiveProcessor.java @@ -16,6 +16,7 @@ import io.smallrye.graphql.test.apps.context.api.ContextApi; import io.smallrye.graphql.test.apps.defaultvalue.api.DefaultValueParrotAPI; import io.smallrye.graphql.test.apps.error.api.ErrorApi; +import io.smallrye.graphql.test.apps.fieldexistence.api.FieldExistenceApi; import io.smallrye.graphql.test.apps.generics.api.ControllerWithGenerics; import io.smallrye.graphql.test.apps.grouping.api.BookGraphQLApi; import io.smallrye.graphql.test.apps.jsonp.api.JsonPApi; @@ -28,7 +29,7 @@ /** * Creates the deployable unit with all the needed dependencies. - * + * * @author Phillip Kruger (phillip.kruger@redhat.com) */ public class SmallRyeGraphQLArchiveProcessor implements ApplicationArchiveProcessor { @@ -90,6 +91,8 @@ public void process(Archive applicationArchive, TestClass testClass) { war.addPackage(JsonPApi.class.getPackage()); war.addPackage(BatchApi.class.getPackage()); war.addPackage(MappingResource.class.getPackage()); + war.addPackage(FieldExistenceApi.class.getPackage()); + } } } diff --git a/server/tck/src/test/java/io/smallrye/graphql/test/apps/fieldexistence/api/FieldExistenceApi.java b/server/tck/src/test/java/io/smallrye/graphql/test/apps/fieldexistence/api/FieldExistenceApi.java new file mode 100644 index 000000000..4c894fe96 --- /dev/null +++ b/server/tck/src/test/java/io/smallrye/graphql/test/apps/fieldexistence/api/FieldExistenceApi.java @@ -0,0 +1,14 @@ +package io.smallrye.graphql.test.apps.fieldexistence.api; + +import org.eclipse.microprofile.graphql.GraphQLApi; +import org.eclipse.microprofile.graphql.Query; + +@GraphQLApi +public class FieldExistenceApi { + + @Query + public FieldExistencePojo fieldExistencePojo(FieldExistencePojo fieldExistencePojo) { + return new FieldExistencePojo(); + } + +} diff --git a/server/tck/src/test/java/io/smallrye/graphql/test/apps/fieldexistence/api/FieldExistencePojo.java b/server/tck/src/test/java/io/smallrye/graphql/test/apps/fieldexistence/api/FieldExistencePojo.java new file mode 100644 index 000000000..2ed6aa859 --- /dev/null +++ b/server/tck/src/test/java/io/smallrye/graphql/test/apps/fieldexistence/api/FieldExistencePojo.java @@ -0,0 +1,11 @@ +package io.smallrye.graphql.test.apps.fieldexistence.api; + +public class FieldExistencePojo { + + public String publicField = "publicField"; + + public static final String PUBLIC_STATIC_FINAL_FIELD = "PUBLIC_STATIC_FINAL_FIELD"; + + public final String publicFinalField = "finalField"; + +} diff --git a/server/tck/src/test/resources/tests/fieldExistenceTests.csv b/server/tck/src/test/resources/tests/fieldExistenceTests.csv new file mode 100644 index 000000000..f3639eef8 --- /dev/null +++ b/server/tck/src/test/resources/tests/fieldExistenceTests.csv @@ -0,0 +1,8 @@ +# Test existence of fields with certain modifiers +1| type FieldExistencePojo | publicField: String | public field should exist on output types +2| type FieldExistencePojo | publicFinalField: String | public final field should exist on output types +3| type FieldExistencePojo | !PUBLIC_STATIC_FINAL_FIELD: String | static field should not exist on output types + +4| input FieldExistencePojoInput | publicField: String | public field should exist on input types +5| input FieldExistencePojoInput | !publicFinalField: String | public final field should not exist on input types +6| input FieldExistencePojoInput | !PUBLIC_STATIC_FINAL_FIELD: String | static field should not exist on input types