Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GraphQLCodeRegistry.Builder is null in custom SchemaDirectiveWirings #268

Closed
stieglma opened this issue Apr 27, 2019 · 5 comments
Closed
Milestone

Comments

@stieglma
Copy link

Hi,

I recently tried to upgrade to the newest version of this library, (graphql-spring-boot-starter is now 5.8.1 and graphql-java-tools is now 5.6.0 both of them seem to be graphql-v12 so I think they should be compatible) and didn't find a solution for the new style of creating directives.

Consider this bean in my spring-boot configuration class:

    @Bean
    public SchemaDirective isAuthorizedDirective() {
        return new SchemaDirective("isAuthorized", new IsAuthorizedDirectiveResolver());
    }

and the respective class:

public class IsAuthorizedDirectiveResolver implements SchemaDirectiveWiring {

    @Override
    public GraphQLFieldDefinition onField(SchemaDirectiveWiringEnvironment<GraphQLFieldDefinition> env) {
        GraphQLFieldDefinition field = env.getElement();
        GraphQLFieldsContainer parent = env.getFieldsContainer();

        DataFetcher<?> originalDataFetcher = env.getCodeRegistry().getDataFetcher(parent, field);
        DataFetcher<?> authDataFetcher = dataFetchingEnvironment -> {
            Authentication auth = SecurityContextHolder.getContext().getAuthentication();
            if (auth instanceof JWTAuthentication) {
                return originalDataFetcher.get(dataFetchingEnvironment);
            }

            throw new UnauthorizedError();
        };

        // adding the authorized fetcher to the code registry builder
        env.getCodeRegistry().dataFetcher(parent, field, authDataFetcher);

        return field;
    }
}

On application startup I now run into nullpointer exceptions in the line DataFetcher<?> originalDataFetcher = env.getCodeRegistry().getDataFetcher(parent, field); because getCodeRegistry returns null.

I debugged that now quite a while and the problem seems to be that in DirectiveBehavior.toParameters the conversion to SchemaGeneratorDirectiveHelper.Parameters only stores the runtime wiring, which is ignored while creating the SchemaDirectiveWiringEnvironmentImpl.

A solution could be to unpack the GraphQLCodeRegistry.Builder from the runtime-wiring, either at the first conversion from directive behavior parameters to schema generator parameters, or afterwards.

Maybe I do also only have to change how the directive is added?

@almyy
Copy link
Contributor

almyy commented Apr 30, 2019

I'm seeing the same issue, but in my case it's on an argument directive.

@b4eEX
Copy link
Contributor

b4eEX commented Apr 30, 2019

The culprit appears to be https://github.com/graphql-java-kickstart/graphql-java-tools/blob/master/src/main/kotlin/graphql/schema/idl/DirectiveBehavior.kt#L58, it explicitly passes null as GraphQLCodeRegistry.Builder.

@b4eEX
Copy link
Contributor

b4eEX commented Apr 30, 2019

So, this patch

Index: src/main/kotlin/graphql/schema/idl/DirectiveBehavior.kt
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
--- src/main/kotlin/graphql/schema/idl/DirectiveBehavior.kt	(date 1555748287000)
+++ src/main/kotlin/graphql/schema/idl/DirectiveBehavior.kt	(date 1556633224000)
@@ -55,7 +55,8 @@
     }
 
     data class Params(val runtimeWiring: RuntimeWiring) {
-        internal fun toParameters() = SchemaGeneratorDirectiveHelper.Parameters(null, runtimeWiring, null, null)
+        internal fun toParameters() = SchemaGeneratorDirectiveHelper.Parameters(null, runtimeWiring, null,
+                GraphQLCodeRegistry.newCodeRegistry(runtimeWiring.codeRegistry))
     }
 
 }

works around the NPE, but I'm not really sure this is the right solution. I would guess that Builder is passed so that the directive could actually edit the code registry, and this solution does not support that.

@alexgenco
Copy link

FWIW this is still an issue with 5.6.0 + graphql-java 13.0.

@oliemansm
Copy link
Member

Code registry builder is now more actively being used by the lib, so should be easy next step to fix this issue now. Will take a look

@oliemansm oliemansm added this to the 5.6.1 milestone Jun 18, 2019
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

No branches or pull requests

5 participants