Skip to content

post processing on schema directive wiring afterr schema is build - #2082

Merged
bbakerman merged 11 commits into
masterfrom
schema-directive-wiring-is-performed-after-schema-is-built
Nov 13, 2020
Merged

bbakerman merged 11 commits into
masterfrom
schema-directive-wiring-is-performed-after-schema-is-built

Conversation

@bbakerman

@bbakerman bbakerman commented Oct 25, 2020

Copy link
Copy Markdown
Member

This is related to the bug #2070

Previously the SchemaDirectiveWiring was called as each element was built. HOwever with type references this meant the callers did not get a full schema

This moves the directive wiring to a post processor and hence it can invoke them after we have a fill schema yet still change them

@bbakerman
bbakerman requested a review from andimarek October 29, 2020 11:09
@bbakerman bbakerman added this to the 16.0 milestone Oct 29, 2020
* @throws graphql.AssertException if there is not field in context at the time of the directive wiring callback
*/
GraphQLFieldDefinition setFieldDataFetcher(DataFetcher newDataFetcher);
GraphQLFieldDefinition setFieldDataFetcher(DataFetcher<?> newDataFetcher);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

prevent IDEA warnings on generics

if (isIntrospectionType(node)) {
return true;
}
NamedNode<?> definition = suitableFunc.apply(node);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

the schema directive wiring code makes an assumption we have a SDL AST definition behind it - so it's SDL only

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we need to restrict it? Is it bad that we restrict it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We don't want any one tweaking introspection types I think


// they may have changed the arguments to the fieldDefinition so reflect that
fieldDefinition = fieldDefinition.transform(builder -> builder.clearArguments().arguments(newArgs));
if (isNotTheSameObjects(startingArgs, newArgs)) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I changed this so that IF the node did not change, we just return the old node - slight optimisation in tree building

@bbakerman
bbakerman merged commit fb86ee6 into master Nov 13, 2020
@andimarek
andimarek deleted the schema-directive-wiring-is-performed-after-schema-is-built branch May 4, 2021 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants