8000 Fix schema builder to copy extensionDefinitions by dondonz · Pull Request #3470 · 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
19 changes: 10 additions & 9 deletions src/main/java/graphql/schema/GraphQLSchema.java
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public class GraphQLSchema {
private final ImmutableMap<String, ImmutableList<String>> interfaceNameToObjectTypeNames;

/*
* This constructs partial GraphQL schema object which has has the schema (query / mutation / subscription) trees
* This constructs partial GraphQL schema object which has the schema (query / mutation / subscription) trees
Copy link
Member Author

Choose a reason for hiding this comment

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

IntelliJ suggested these grammar edits, unrelated to builder fix

* in it but it does not have the collected types, code registry nor the type references replaced
*
* But it can be traversed to discover all that and filled out later via another constructor.
Expand Down Expand Up @@ -104,7 +104,7 @@ private GraphQLSchema(Builder builder) {
}

/*
* This constructs a full fledged graphql schema object that has not yet had its type references replaced
* This constructs a fully fledged graphql schema object that has not yet had its type references replaced
* but it's otherwise complete
*/
@Internal
Expand Down Expand Up @@ -255,7 +255,7 @@ public <T extends GraphQLType> List<T> getTypes(Collection<String> typeNames) {

/**
* Gets the named type from the schema or null if it's not present.
*
* <p>
* Warning - you are inviting class cast errors if the types are not what you expect.
*
* @param typeName the name of the type to retrieve
Expand Down Expand Up @@ -286,7 +286,7 @@ public boolean containsType(String typeName) {
*
* @return a graphql object type or null if there is one
*
* @throws graphql.GraphQLException if the type is NOT a object type
* @throws graphql.GraphQLException if the type is NOT an object type
*/
public GraphQLObjectType getObjectType(String typeName) {
GraphQLType graphQLType = typeMap.get(typeName);
Expand Down Expand Up @@ -433,14 +433,14 @@ public List<GraphQLDirective> getDirectives() {
}

/**
* @return a map of non repeatable directives by directive name
* @return a map of non-repeatable directives by directive name
*/
public Map<String, GraphQLDirective> getDirectivesByName() {
return directiveDefinitionsHolder.getDirectivesByName();
}

/**
* Returns a named directive that (for legacy reasons) will be only in the set of non repeatable directives
* Returns a named directive that (for legacy reasons) will be only in the set of non-repeatable directives
*
* @param directiveName the name of the directive to retrieve
*
Expand Down Expand Up @@ -535,7 +535,7 @@ public List<GraphQLDirective> getSchemaDirectives(String directiveName) {
* directives for all schema elements, whereas this is just for the schema
* element itself
*
* @return a map of directives
* @return a list of directives
*/
public List<GraphQLAppliedDirective> getSchemaAppliedDirectives() {
return schemaAppliedDirectivesHolder.getAppliedDirectives();
Expand Down Expand Up @@ -641,6 +641,7 @@ public static Builder newSchema(GraphQLSchema existingSchema) {
.query(existingSchema.getQueryType())
.mutation(existingSchema.getMutationType())
.subscription(existingSchema.getSubscriptionType())
.extensionDefinitions(existingSchema.getExtensionDefinitions())
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the key change - add to builder

.introspectionSchemaType(existingSchema.getIntrospectionSchemaType())
.codeRegistry(existingSchema.getCodeRegistry())
.clearAdditionalTypes()
Expand Down Expand Up @@ -873,8 +874,8 @@ public GraphQLSchema build(Set<GraphQLType> additionalTypes) {
/**
* Builds the schema
*
* @param additionalTypes - please don't use this any more
* @param additionalDirectives - please don't use this any more
* @param additionalTypes - please don't use this anymore
* @param additionalDirectives - please don't use this anymore
*
* @return the built schema
*
Expand Down
21 changes: 19 additions & 2 deletions src/test/groovy/graphql/schema/GraphQLSchemaTest.groovy
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import graphql.Directives
import graphql.ExecutionInput
import graphql.GraphQL
import graphql.TestUtil
import graphql.language.Directive
import graphql.language.SchemaExtensionDefinition
import graphql.schema.idl.RuntimeWiring
import graphql.schema.idl.TypeRuntimeWiring
import graphql.util.TraversalControl
Expand Down Expand Up @@ -129,6 +131,23 @@ class GraphQLSchemaTest extends Specification {
))
}

def "schema builder copies extension definitions"() {
setup:
def schemaBuilder = basicSchemaBuilder()
def newDirective = Directive.newDirective().name("pizza").build()
def extension = SchemaExtensionDefinition.newSchemaExtensionDefinition().directive(newDirective).build()
def oldSchema = schemaBuilder.extensionDefinitions([extension]).build()

when:
def newSchema = GraphQLSchema.newSchema(oldSchema).build()

then:
oldSchema.extensionDefinitions.size() == 1
newSchema.extensionDefinitions.size() == 1
((Directive) oldSchema.extensionDefinitions.first().getDirectives().first()).name == "pizza"
((Directive) newSchema.extensionDefinitions.first().getDirectives().first()).name == "pizza"
}

def "clear directives works as expected"() {
setup:
def schemaBuilder = basicSchemaBuilder()
Expand Down Expand Up @@ -233,10 +252,8 @@ class GraphQLSchemaTest extends Specification {
}

union UnionType = Cat | Dog

'''


when:
def schema = TestUtil.schema(sdl)

Expand Down
0