8000 Return empty singleton DirectivesHolder if directives are empty by gnawf · Pull Request #3518 · graphql-java/graphql-java · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/main/java/graphql/DirectivesUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import graphql.util.FpKit;

import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -91,7 +92,6 @@ public static GraphQLDirective getFirstDirective(String name, Map<String, List<G
* directives collection takes precedence.
*
* @param directiveContainer the schema element holding applied directives
*
* @return a combined list unique by name
*/
public static List<GraphQLAppliedDirective> toAppliedDirectives(GraphQLDirectiveContainer directiveContainer) {
Expand All @@ -104,7 +104,6 @@ public static List<GraphQLAppliedDirective> toAppliedDirectives(GraphQLDirective
*
* @param appliedDirectives the applied directives to use
* @param directives the legacy directives to use
*
* @return a combined list unique by name
*/
public static List<GraphQLAppliedDirective> toAppliedDirectives(Collection<GraphQLAppliedDirective> appliedDirectives, Collection<GraphQLDirective> directives) {
Expand All @@ -127,6 +126,7 @@ public static List<GraphQLAppliedDirective> toAppliedDirectives(Collection<Graph
* A holder class that breaks a list of directives into maps to be more easily accessible in using classes
*/
public static class DirectivesHolder {
private static final DirectivesHolder EMPTY_HOLDER = new DirectivesHolder(Collections.emptyList(), Collections.emptyList());

private final ImmutableMap<String, List<GraphQLDirective>> allDirectivesByName;
private final ImmutableMap<String, GraphQLDirective> nonRepeatableDirectivesByName;
Expand All @@ -145,7 +145,14 @@ public DirectivesHolder(Collection<GraphQLDirective> allDirectives, Collection<G

this.allAppliedDirectives = ImmutableList.copyOf(allAppliedDirectives);
this.allAppliedDirectivesByName = ImmutableMap.copyOf(FpKit.groupingBy(allAppliedDirectives, GraphQLAppliedDirective::getName));
}

public static DirectivesHolder create(List<GraphQLDirective> directives, List<GraphQLAppliedDirective> appliedDirectives) {
if (directives.isEmpty() && appliedDirectives.isEmpty()) {
return EMPTY_HOLDER;
}

return new DirectivesHolder(directives, appliedDirectives);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we make the constructor private?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean I think this is a private thing, but it's not marked with @Internal so I don't want to mess with the API.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Deprecate it then? For future removal?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The DirectivesUtil class itself is marked with @Internal, maybe that means everything under it also is. Not sure...

8000 Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Internal would apply to all methods inside the class

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup all code in the class is internal. We can break it at will! Use at your own risk!

}

public ImmutableMap<String, List<GraphQLDirective>> getAllDirectivesByName() {
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/GraphQLArgument.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ private GraphQLArgument(String name,
this.value = value;
this.definition = definition;
this.deprecationReason = deprecationReason;
this.directivesHolder = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directivesHolder = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
}


Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/GraphQLEnumType.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private GraphQLEnumType(String name,
this.description = description;
this.definition = definition;
this.extensionDefinitions = ImmutableList.copyOf(extensionDefinitions);
this.directivesHolder = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directivesHolder = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
this.valueDefinitionMap = buildMap(values);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ private GraphQLEnumValueDefinition(String name,
this.description = description;
this.value = value;
this.deprecationReason = deprecationReason;
this.directivesHolder = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directivesHolder = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
this.definition = definition;
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/GraphQLFieldDefinition.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private GraphQLFieldDefinition(String name,
this.originalType = type;
this.dataFetcherFactory = dataFetcherFactory;
this.arguments = ImmutableList.copyOf(arguments);
this.directivesHolder = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directivesHolder = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
this.deprecationReason = deprecationReason;
this.definition = definition;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/GraphQLInputObjectField.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ private GraphQLInputObjectField(
this.originalType = type;
this.defaultValue = defaultValue;
this.description = description;
this.directivesHolder = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directivesHolder = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
this.definition = definition;
this.deprecationReason = deprecationReason;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/GraphQLInputObjectType.java
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ private GraphQLInputObjectType(String name,
this.description = description;
this.definition = definition;
this.extensionDefinitions = ImmutableList.copyOf(extensionDefinitions);
this.directives = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directives = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
this.fieldMap = buildDefinitionMap(fields);
this.isOneOf = hasOneOf(directives, appliedDirectives);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/GraphQLInterfaceType.java
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ private GraphQLInterfaceType(String name,
this.interfaceComparator = interfaceComparator;
this.originalInterfaces = ImmutableList.copyOf(sortTypes(interfaceComparator, interfaces));
this.extensionDefinitions = ImmutableList.copyOf(extensionDefinitions);
this.directivesHolder = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directivesHolder = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
this.fieldDefinitionsByName = buildDefinitionMap(fieldDefinitions);
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/GraphQLObjectType.java
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ private GraphQLObjectType(String name,
this.originalInterfaces = ImmutableList.copyOf(sortTypes(interfaceComparator, interfaces));
this.definition = definition;
this.extensionDefinitions = ImmutableList.copyOf(extensionDefinitions);
this.directivesHolder = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directivesHolder = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
this.fieldDefinitionsByName = buildDefinitionMap(fieldDefinitions);
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/GraphQLScalarType.java
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ private GraphQLScalarType(String name,
this.description = description;
this.coercing = coercing;
this.definition = definition;
this.directivesHolder = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directivesHolder = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
this.extensionDefinitions = ImmutableList.copyOf(extensionDefinitions);
this.specifiedByUrl = specifiedByUrl;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/graphql/schema/GraphQLUnionType.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ private GraphQLUnionType(String name,
this.typeResolver = typeResolver;
this.definition = definition;
this.extensionDefinitions = ImmutableList.copyOf(extensionDefinitions);
this.directives = new DirectivesUtil.DirectivesHolder(directives, appliedDirectives);
this.directives = DirectivesUtil.DirectivesHolder.create(directives, appliedDirectives);
}

void replaceTypes(List<GraphQLNamedOutputType> types) {
Expand Down
0