Skip to content

Commit 4543c76

Browse files
committed
code review fixes
1 parent 3f0f3f4 commit 4543c76

File tree

4 files changed

+22
-18
lines changed

4 files changed

+22
-18
lines changed

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,7 @@ For example, we wish to create a directive that adds suffix to graphql fields.
427427
```
428428

429429
- must be annotated with `@GraphQLDirectiveDefinition` and to supply a wiring class to it (will be explained later)
430-
- the name of the directive will be taken from the class name `(Suffix)` or if annotated with `@GraphQLName` - from its value
430+
- the name of the directive will be taken from the class name (`Suffix`) or if annotated with `@GraphQLName` - from its value
431431
- the description is taken from the `@GraphQLDescription` annotation
432432
- must be annotated with `@Retention` with a `RUNTIME` policy
433433
- must be annotated with `@DirectiveLocations` in order to specify where we can put this directive on (for example - field definition, interface)

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

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
*/
1515
package graphql.annotations.processor;
1616

17-
import graphql.annotations.annotationTypes.directives.definition.GraphQLDirectiveDefinition;
1817
import graphql.annotations.annotationTypes.GraphQLName;
18+
import graphql.annotations.annotationTypes.directives.definition.GraphQLDirectiveDefinition;
1919
import graphql.annotations.processor.directives.CommonPropertiesCreator;
2020
import graphql.annotations.processor.directives.DirectiveArgumentCreator;
2121
import graphql.annotations.processor.directives.DirectiveCreator;
@@ -49,6 +49,7 @@
4949
*/
5050
public class GraphQLAnnotations implements GraphQLAnnotationsProcessor {
5151

52+
public static final String NOT_PROPERLY_ANNOTATION_ERROR = "The supplied class %s is not annotated with a GraphQLDirectiveDefinition annotation";
5253
private GraphQLObjectHandler graphQLObjectHandler;
5354
private GraphQLExtensionsHandler graphQLExtensionsHandler;
5455
private DirectiveCreator directiveCreator;
@@ -131,7 +132,7 @@ public GraphQLObjectType object(Class<?> object) throws GraphQLAnnotationsExcept
131132

132133
@Deprecated
133134
public GraphQLObjectType object(Class<?> object, DirectiveAndWiring... directives) throws GraphQLAnnotationsException {
134-
Arrays.stream(directives).forEach(directive -> this.getContainer().getDirectiveRegistry().put(directive.getDirective().getName(), directive));
135+
Arrays.stream(directives).forEach(directiveAndWiring -> this.getContainer().getDirectiveRegistry().put(directiveAndWiring.getDirective().getName(), directiveAndWiring));
135136
try {
136137
return this.graphQLObjectHandler.getGraphQLType(object, this.getContainer());
137138
} catch (GraphQLAnnotationsException e) {
@@ -143,7 +144,7 @@ public GraphQLObjectType object(Class<?> object, DirectiveAndWiring... directive
143144

144145
public GraphQLDirective directive(Class<?> object) throws GraphQLAnnotationsException {
145146
if (!object.isAnnotationPresent(GraphQLDirectiveDefinition.class)){
146-
throw new GraphQLAnnotationsException(String.format("The supplied class %s is not annotated with a GraphQLDirectiveDefinition annotation", object.getSimpleName()), null);
147+
throw new GraphQLAnnotationsException(String.format(NOT_PROPERLY_ANNOTATION_ERROR, object.getSimpleName()), null);
147148
}
148149

149150
try {
@@ -181,9 +182,9 @@ public Set<GraphQLDirective> directives(Class<?> directivesDeclarationClass) {
181182
Arrays.stream(methods).sequential().filter(method -> method.isAnnotationPresent(GraphQLDirectiveDefinition.class))
182183
.forEach(method -> {
183184
try {
184-
DirectiveAndWiring directive = this.directiveCreator.getDirective(method);
185-
this.getContainer().getDirectiveRegistry().put(directive.getDirective().getName(), directive);
186-
directiveSet.add(directive.getDirective());
185+
DirectiveAndWiring directiveAndWiring = this.directiveCreator.getDirectiveAndWiring(method);
186+
this.getContainer().getDirectiveRegistry().put(directiveAndWiring.getDirective().getName(), directiveAndWiring);
187+
directiveSet.add(directiveAndWiring.getDirective());
187188
} catch (GraphQLAnnotationsException e) {
188189
this.getContainer().getProcessing().clear();
189190
this.getTypeRegistry().clear();

src/main/java/graphql/annotations/processor/directives/DirectiveCreator.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,15 @@
1414
*/
1515
package graphql.annotations.processor.directives;
1616

17+
import graphql.annotations.annotationTypes.directives.definition.DirectiveLocations;
1718
import graphql.annotations.annotationTypes.directives.definition.GraphQLDirectiveDefinition;
1819
import graphql.annotations.directives.AnnotationsDirectiveWiring;
19-
import graphql.annotations.annotationTypes.directives.definition.DirectiveLocations;
2020
import graphql.annotations.processor.DirectiveAndWiring;
2121
import graphql.annotations.processor.exceptions.GraphQLAnnotationsException;
2222
import graphql.introspection.Introspection;
2323
import graphql.schema.GraphQLDirective;
2424

2525
import java.lang.annotation.Retention;
26-
import java.lang.annotation.RetentionPolicy;
2726
import java.lang.reflect.AnnotatedElement;
2827
import java.lang.reflect.Method;
2928
import java.util.Arrays;
@@ -56,7 +55,7 @@ public GraphQLDirective getDirective(Class<?> annotatedClass) {
5655
return builder.build();
5756
}
5857

59-
public DirectiveAndWiring getDirective(Method directiveMethod){
58+
public DirectiveAndWiring getDirectiveAndWiring(Method directiveMethod){
6059
GraphQLDirective.Builder builder = newDirective()
6160
.name(commonPropertiesCreator.getName(directiveMethod))
6261
.description(commonPropertiesCreator.getDescription(directiveMethod));

src/main/java/graphql/annotations/processor/retrievers/fieldBuilders/DirectivesBuilder.java

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,10 @@
3535

3636

3737
public class DirectivesBuilder implements Builder<GraphQLDirective[]> {
38+
public static final String NOT_FOUND_IN_DIRECTIVE_REGISTRY_ERROR = "No directive named %s is found in the directive registry";
39+
public static final String TOO_MUCH_ARGUMENTS_ERROR = "Directive '%s' is supplied with more argument values than it supports";
40+
public static final String DIRECTIVE_ARGUMENT_TYPE_MUST_BE_A_SCALAR = "Directive argument type must be a scalar!";
41+
public static final String COULD_NOT_PARSE_ARGUMENT_VALUE_TO_ARGUMENT_TYPE = "Could not parse argument value to argument type";
3842
private AnnotatedElement object;
3943
private ProcessingElementsContainer container;
4044

@@ -54,7 +58,7 @@ public GraphQLDirective[] build() {
5458
GraphQLDirective graphQLDirective = transformArgs(container.getDirectiveRegistry().get(name).getDirective(), annotation);
5559
graphQLDirectives.add(graphQLDirective);
5660
} else {
57-
throw new GraphQLAnnotationsException(String.format("No directive named %s is found in the directive registry", name), null);
61+
throw new GraphQLAnnotationsException(String.format(NOT_FOUND_IN_DIRECTIVE_REGISTRY_ERROR, name), null);
5862
}
5963
});
6064

@@ -66,7 +70,7 @@ public GraphQLDirective[] build() {
6670
if (container.getDirectiveRegistry().containsKey(x.name())) {
6771
return transformArgs(container.getDirectiveRegistry().get(x.name()).getDirective(), x.argumentsValues());
6872
} else {
69-
throw new GraphQLAnnotationsException(String.format("No directive named %s is found in the directive registry", x.name()), null);
73+
throw new GraphQLAnnotationsException(String.format(NOT_FOUND_IN_DIRECTIVE_REGISTRY_ERROR, x.name()), null);
7074
}
7175
}
7276
).collect(Collectors.toList());
@@ -84,7 +88,7 @@ private GraphQLDirective transformArgs(GraphQLDirective graphQLDirective, Annota
8488
List<GraphQLArgument> arguments = graphQLDirective.getArguments();
8589

8690
if (annotation.annotationType().getDeclaredMethods().length > arguments.size()) {
87-
throw new GraphQLAnnotationsException(String.format("Directive '%s' is supplied with more argument values than it supports", graphQLDirective.getName()), null);
91+
throw new GraphQLAnnotationsException(String.format(TOO_MUCH_ARGUMENTS_ERROR, graphQLDirective.getName()), null);
8892
}
8993

9094
for (int i = 0; i < annotation.annotationType().getDeclaredMethods().length; i++) {
@@ -105,7 +109,7 @@ private GraphQLDirective transformArgs(GraphQLDirective graphQLDirective, String
105109
List<GraphQLArgument> arguments = graphQLDirective.getArguments();
106110

107111
if (argumentValues.length > arguments.size()) {
108-
throw new GraphQLAnnotationsException(String.format("Directive '%s' is supplied with more argument values than it supports", graphQLDirective.getName()), null);
112+
throw new GraphQLAnnotationsException(String.format(TOO_MUCH_ARGUMENTS_ERROR, graphQLDirective.getName()), null);
109113
}
110114

111115
for (int i = 0; i < argumentValues.length; i++) {
@@ -138,10 +142,10 @@ private void transformArgument(Annotation annotation, GraphQLDirective.Builder d
138142
}
139143
builder.value(value);
140144
} catch (Exception e) {
141-
throw new GraphQLAnnotationsException("Could not parse argument value to argument type", e);
145+
throw new GraphQLAnnotationsException(COULD_NOT_PARSE_ARGUMENT_VALUE_TO_ARGUMENT_TYPE, e);
142146
}
143147
} else {
144-
throw new GraphQLAnnotationsException("Directive argument type must be a scalar!", null);
148+
throw new GraphQLAnnotationsException(DIRECTIVE_ARGUMENT_TYPE_MUST_BE_A_SCALAR, null);
145149
}
146150
}));
147151
}
@@ -159,10 +163,10 @@ private void transformArgument(String[] argumentValues, GraphQLDirective.Builder
159163
Object value = ((GraphQLScalarType) graphQLArgument.getType()).getCoercing().parseValue(argumentValue);
160164
builder.value(value);
161165
} catch (Exception e) {
162-
throw new GraphQLAnnotationsException("Could not parse argument value to argument type", e);
166+
throw new GraphQLAnnotationsException(COULD_NOT_PARSE_ARGUMENT_VALUE_TO_ARGUMENT_TYPE, e);
163167
}
164168
} else {
165-
throw new GraphQLAnnotationsException("Directive argument type must be a scalar!", null);
169+
throw new GraphQLAnnotationsException(DIRECTIVE_ARGUMENT_TYPE_MUST_BE_A_SCALAR, null);
166170
}
167171
}));
168172
}

0 commit comments

Comments
 (0)