Provide an alternative way of initializing the SimpleGraphQLServlet

[kaqqao]

I often get asked what to do with the servlet instance once it's created, as the servlet spec provides no programmatic way to register it. Some servlet containers do, when started programmatically (e.g. Jetty), but there's nothing portable. Spring also allows this, but if you already have Spring, creating a custom servlet is already a strange requirement, in the face of Spring MVC controllers etc.

So what people end up doing is what I did in the HowToGraphQL tutorial: subclass SimpleGraphQLServlet, handle the entire setup in the constructor, and register that subclass with the container (using @WebServlet or web.xml) so the container can initialize it as usual. The problem with this is that all the setup steps need to be static and weirdly inlined as the call to super needs to be the very first thing in the constructor. This is very unintuitive and cumbersome. Worse yet, until recently, there was no accessible non-deprecated constructor to call from the subclass, making the only obvious strategy obsolete.

So what I propose is to allow SimpleGraphQLServlet to initialize its state in the usual Servlet#init(ServletConfig) method instead of the constructor. This method exists in the spec exactly because the constructor is often a very inconvenient place to setup servlets.

E.g.

class SimpleGraphQLServlet {
    
    @Override
    public void init(ServletConfig config) {
        Builder builder = getConfiguration();
        if (builder != null) {
            this.schemaProvider = builder.schemaProvider;
            ... // the same as the current constructor
        }
    }
   
    //this is for the subclasses to optionally override
    protected Builder getConfiguration() {
        return null;
    }
}

This would allow a very vanilla use of the servlet, compatible with any container with no surprises or workarounds needed:

@WebServlet(urlPatterns = "/graphql") //or via web.xml
public class GraphQLEndpoint extends SimpleGraphQLServlet {

    @Override
    protected Builder getConfiguration() {
       return SimpleGraphQLServlet.builder(schema)
           .withInstrumentation(..) //etc
    }
}

This is just a quick example of what I mean. Instead of a Builder, getConfiguration could return some Configuration object you could get e.g. by calling Builder#toConfiguration instead of Builder#build or whatever.

I could, of course, implement this if you'd want me to.

[apottere]

This seems reasonable, it might be tough to get everything out of the constructor for the simple servlet though. Some of those things need to be passed to the parent servlet.

[kaqqao]

Yup, I'd expect the parent class would need similar refactoring. But it doesn't seem to bad, and it doesn't have to break the existing API. Would do away with a lot of confusion, IMHO.

[alamothe]

When is expected to have the referenced patch released? Right now, there is no other way to use SimpleGraphQLServlet but to call a deprecated constructor

[dgruntz]

I see another work around this deprecated constructor: One could define a proper HTTP-Servlet and forward all the invocations to an instance of SimpleGraphQLServlet. The servlet would look as follows:

@WebServlet("/graphql")
public class GraphQLEndpoint extends HttpServlet {
	
	private SimpleGraphQLServlet graphQLServlet;
	
	@Override
	protected void service(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
		graphQLServlet.service(req, resp);
	}

	@Override
	public void init() throws ServletException {
		graphQLServlet = SimpleGraphQLServlet.builder(buildSchema()).withInstrumentation(new TracingInstrumentation()).build();
	}
	...
}

However, if I try this I get the following exception:

java.lang.NoClassDefFoundError: graphql/execution/instrumentation/NoOpInstrumentation
    at graphql.servlet.SimpleGraphQLServlet$Builder.<init>(SimpleGraphQLServlet.java:134) ~[graphql-java-servlet-4.7.0.jar:na]

But beyond that this seems to be a reasonable solution/workaround.

[kaqqao]

@dgruntz Your exception is a graphql-java version mismatch. You're probably trying to use v8.0 which isn't yet compatible.
Overriding dependency versions is scary business.

Approach wise, it's a decent solution.

[dgruntz]

@kaqqao Thanks for your hint! I switched from v8.0 to v7.0 and now it works. I now use the following dependencies:

compile "com.graphql-java:graphql-java:7.0"
compile "com.graphql-java:graphql-java-tools:4.3.0"
compile "com.graphql-java:graphql-java-servlet:4.7.0"

This means that my proposal above is a workaround for the deprecated constructor.

[kaqqao]

@dgruntz You don't have to declare a dependency to graphql-java at all, the servelt will drag in the correct version.

And yes, I agree, your approach is an interesting workaround.

[alamothe]

The newest version reverted to a private constructor again - why?

[oliemansm]

@kaqqao The refactoring in 6.x.x opens up more possibilities for this but I see that you'd still have to write quite some boilerplate yourself to get it working with sensible defaults. Should be simplified a bit more.

[oliemansm]

@kaqqao Finally got around to work on this. See #127.

[kaqqao]

Excellent! Thanks @oliemansm! 😁

[soutinho5555]

How can you after implementing a Graphql webservlet, implement a set of integration tests of the services of your graphql schema?