Fixes for OSGi, remove automatic creation of executor for async requests (WIP)

graphql-java-kickstart/bnd.bnd

@@ -1,2 +1,5 @@
 Export-Package: graphql.kickstart.*
-Import-Package: !lombok,*
+Import-Package: !lombok,\
+  graphql.kickstart.*,\
+  graphql.*;version="[20.2,22)",\
+  *

graphql-java-servlet/bnd.bnd

@@ -1,4 +1,13 @@
 Export-Package: graphql.kickstart.servlet.*
-Import-Package: !lombok,*
-Require-Capability: osgi.extender;
-  filter:="(&(osgi.extender=osgi.component)(version>=1.3)(!(version>=2.0)))"
+Import-Package: !lombok,\
+  graphql;version="[20.2,22)",\
+  graphql.execution.instrumentation;version="[20.2,22)",\
+  graphql.execution.preparsed;version="[20.2,22)",\
+  graphql.execution.reactive;version="[20.2,22)",\
+  graphql.schema;version="[20.2,22)",\
+  javax.servlet;version="[3.1,5)",\
+  javax.servlet.http;version="[3.1,5)",\
+  javax.websocket;version="[1.1,2)",\
+  javax.websocket.server;version="[1.1,2)",\
+  *
+Require-Capability: osgi.extender;filter:="(&(osgi.extender=osgi.component)(version>=1.3.0)(!(version>=2.0.0)))"

graphql-java-servlet/build.gradle

@@ -16,9 +16,9 @@ dependencies {
     api(project(':graphql-java-kickstart'))
 
     // Servlet
-    compileOnly "jakarta.servlet:jakarta.servlet-api:6.0.0"
-    compileOnly "jakarta.websocket:jakarta.websocket-api:2.1.1"
-    compileOnly "jakarta.websocket:jakarta.websocket-client-api:2.1.1"
+    api "jakarta.servlet:jakarta.servlet-api:6.0.0"
+    api "jakarta.websocket:jakarta.websocket-api:2.1.1"
+    api "jakarta.websocket:jakarta.websocket-client-api:2.1.1"
     implementation "org.slf4j:slf4j-api:$LIB_SLF4J_VER"
 
     // OSGi

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLConfiguration.java

@@ -18,9 +18,6 @@
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.Executor;
-import java.util.concurrent.LinkedBlockingQueue;
-import java.util.concurrent.ThreadPoolExecutor;
-import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
 import lombok.Getter;
 
@@ -67,12 +64,12 @@ public static GraphQLConfiguration.Builder with(GraphQLSchema schema) {
   }
 
   public static GraphQLConfiguration.Builder with(GraphQLSchemaServletProvider schemaProvider) {
-    return new Builder(GraphQLInvocationInputFactory.newBuilder(schemaProvider));
+    return new Builder().with(GraphQLInvocationInputFactory.newBuilder(schemaProvider));
   }
 
   public static GraphQLConfiguration.Builder with(
       GraphQLInvocationInputFactory invocationInputFactory) {
-    return new Builder(invocationInputFactory);
+    return new Builder().with(invocationInputFactory);
   }
 
   public GraphQLInvocationInputFactory getInvocationInputFactory() {
@@ -144,19 +141,9 @@ public static class Builder {
     private Supplier<BatchInputPreProcessor> batchInputPreProcessorSupplier =
         NoOpBatchInputPreProcessor::new;
     private GraphQLResponseCacheManager responseCacheManager;
-    private int asyncCorePoolSize = 10;
-    private int asyncMaxPoolSize = 200;
     private Executor asyncExecutor;
     private AsyncTaskDecorator asyncTaskDecorator;
 
-    private Builder(GraphQLInvocationInputFactory.Builder invocationInputFactoryBuilder) {
-      this.invocationInputFactoryBuilder = invocationInputFactoryBuilder;
-    }
-
-    private Builder(GraphQLInvocationInputFactory invocationInputFactory) {
-      this.invocationInputFactory = invocationInputFactory;
-    }
-
     public Builder with(GraphQLInvoker graphQLInvoker) {
       this.graphQLInvoker = graphQLInvoker;
       return this;
@@ -183,17 +170,39 @@ public Builder with(List<GraphQLServletListener> listeners) {
       return this;
     }
 
+    public Builder with(GraphQLInvocationInputFactory.Builder invocationInputFactoryBuilder) {
+      if (this.invocationInputFactoryBuilder != null) {
+        throw new IllegalArgumentException("Cannot set invocationInputFactoryBuilder if invocationInputFactory is used");
+      }
+      this.invocationInputFactoryBuilder = invocationInputFactoryBuilder;
+      return this;
+    }
+
+    public Builder with(GraphQLInvocationInputFactory invocationInputFactory) {
+      if (this.invocationInputFactoryBuilder != null) {
+        throw new IllegalArgumentException("Cannot set invocationInputFactory if invocationInputFactoryBuilder is used");
+      }
+      this.invocationInputFactory = invocationInputFactory;
+      return this;
+    }
+
     public Builder with(GraphQLServletContextBuilder contextBuilder) {
+      if (this.invocationInputFactoryBuilder == null) {
+        throw new IllegalArgumentException("Cannot use a contextBuilder without setting invocationInputFactoryBuilder first");
+      }
       this.invocationInputFactoryBuilder.withGraphQLContextBuilder(contextBuilder);
       return this;
     }
 
     public Builder with(GraphQLServletRootObjectBuilder rootObjectBuilder) {
+      if (this.invocationInputFactoryBuilder == null) {
+        throw new IllegalArgumentException("Cannot use a rootObjectBuilder without setting invocationInputFactoryBuilder first");
+      }
       this.invocationInputFactoryBuilder.withGraphQLRootObjectBuilder(rootObjectBuilder);
       return this;
     }
 
-    public Builder with(long subscriptionTimeout) {
+    public Builder subscriptionTimeout(long subscriptionTimeout) {
       this.subscriptionTimeout = subscriptionTimeout;
       return this;
     }
@@ -208,16 +217,6 @@ public Builder with(Executor asyncExecutor) {
       return this;
     }
 
-    public Builder asyncCorePoolSize(int asyncCorePoolSize) {
-      this.asyncCorePoolSize = asyncCorePoolSize;
-      return this;
-    }
-
-    public Builder asyncMaxPoolSize(int asyncMaxPoolSize) {
-      this.asyncMaxPoolSize = asyncMaxPoolSize;
-      return this;
-    }
-
     public Builder with(ContextSetting contextSetting) {
       if (contextSetting != null) {
         this.contextSetting = contextSetting;
@@ -249,20 +248,12 @@ public Builder with(AsyncTaskDecorator asyncTaskDecorator) {
       return this;
     }
 
-    private Executor getAsyncExecutor() {
+    private Executor getAsyncTaskExecutor() {
       if (asyncExecutor != null) {
-        return asyncExecutor;
+        return new AsyncTaskExecutor(asyncExecutor, asyncTaskDecorator);
       }
-      return new ThreadPoolExecutor(
-          asyncCorePoolSize,
-          asyncMaxPoolSize,
-          60,
-          TimeUnit.SECONDS,
-          new LinkedBlockingQueue<>(Integer.MAX_VALUE));
-    }
 
-    private Executor getAsyncTaskExecutor() {
-      return new AsyncTaskExecutor(getAsyncExecutor(), asyncTaskDecorator);
+      return null;
     }
 
     public GraphQLConfiguration build() {

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLHttpServlet.java

@@ -1,6 +1,7 @@
 package graphql.kickstart.servlet;
 
 import graphql.schema.GraphQLSchema;
+import java.util.concurrent.ThreadPoolExecutor;
 
 /** @author Michiel Oliemans */
 public abstract class GraphQLHttpServlet extends AbstractGraphQLHttpServlet {

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/GraphQLWebsocketServlet.java

@@ -265,7 +265,7 @@ public boolean isShutDown() {
     return isShutDown.get();
   }
 
-  private SubscriptionProtocolFactory getSubscriptionProtocolFactory(List<String> accept) {
+  public SubscriptionProtocolFactory getSubscriptionProtocolFactory(List<String> accept) {
     for (String protocol : accept) {
       for (SubscriptionProtocolFactory subscriptionProtocolFactory :
           subscriptionProtocolFactories) {

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/HttpRequestInvokerImpl.java

@@ -42,7 +42,7 @@ public void execute(
       HttpServletRequest request,
       HttpServletResponse response,
       ListenerHandler listenerHandler) {
-    if (request.isAsyncSupported()) {
+    if (request.isAsyncSupported() && configuration.getAsyncExecutor() != null) {
       invokeAndHandleAsync(invocationInput, request, response, listenerHandler);
     } else {
       handle(invocationInput, request, response, listenerHandler);

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/OsgiGraphQLHttpServlet.java

@@ -16,6 +16,7 @@
 import graphql.kickstart.servlet.core.GraphQLServletRootObjectBuilder;
 import graphql.kickstart.servlet.osgi.GraphQLCodeRegistryProvider;
 import graphql.kickstart.servlet.osgi.GraphQLDirectiveProvider;
+import graphql.kickstart.servlet.osgi.GraphQLConfigurationProvider;
 import graphql.kickstart.servlet.osgi.GraphQLMutationProvider;
 import graphql.kickstart.servlet.osgi.GraphQLProvider;
 import graphql.kickstart.servlet.osgi.GraphQLQueryProvider;
@@ -41,6 +42,7 @@ public class OsgiGraphQLHttpServlet extends AbstractGraphQLHttpServlet {
 
   public OsgiGraphQLHttpServlet() {
     schemaBuilder.updateSchema();
+    schemaBuilder.updateConfiguration();
   }
 
   @Activate
@@ -53,15 +55,23 @@ public void deactivate() {
     schemaBuilder.deactivate();
   }
 
+  public OsgiSchemaBuilder getSchemaBuilder() {
+    return schemaBuilder;
+  }
+
   @Override
   protected GraphQLConfiguration getConfiguration() {
-    return schemaBuilder.buildConfiguration();
+    return schemaBuilder.getConfiguration();
   }
 
   protected void updateSchema() {
     schemaBuilder.updateSchema();
   }
 
+  protected void updateConfiguration() {
+    schemaBuilder.updateConfiguration();
+  }
+
   @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
   public void bindProvider(GraphQLProvider provider) {
     if (provider instanceof GraphQLQueryProvider) {
@@ -82,7 +92,11 @@ public void bindProvider(GraphQLProvider provider) {
     if (provider instanceof GraphQLCodeRegistryProvider) {
       schemaBuilder.setCodeRegistryProvider((GraphQLCodeRegistryProvider) provider);
     }
+    if (provider instanceof GraphQLConfigurationProvider) {
+      schemaBuilder.setConfigurationBuilderProvider((GraphQLConfigurationProvider) provider);
+    }
     updateSchema();
+    updateConfiguration();
   }
 
   public void unbindProvider(GraphQLProvider provider) {
@@ -104,6 +118,9 @@ public void unbindProvider(GraphQLProvider provider) {
     if (provider instanceof GraphQLCodeRegistryProvider) {
       schemaBuilder.setCodeRegistryProvider(() -> GraphQLCodeRegistry.newCodeRegistry().build());
     }
+    if (provider instanceof GraphQLConfigurationProvider) {
+      schemaBuilder.setConfigurationBuilderProvider(GraphQLConfiguration.Builder::new);
+    }
     updateSchema();
   }
 
@@ -165,28 +182,34 @@ public void unbindDirectivesProvider(GraphQLDirectiveProvider directiveProvider)
   @Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
   public void bindServletListener(GraphQLServletListener listener) {
     schemaBuilder.add(listener);
+    updateConfiguration();
   }
 
   public void unbindServletListener(GraphQLServletListener listener) {
     schemaBuilder.remove(listener);
+    updateConfiguration();
   }
 
   @Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC)
   public void setContextBuilder(GraphQLServletContextBuilder contextBuilder) {
     schemaBuilder.setContextBuilder(contextBuilder);
+    updateConfiguration();
   }
 
   public void unsetContextBuilder(GraphQLServletContextBuilder contextBuilder) {
     schemaBuilder.setContextBuilder(new DefaultGraphQLServletContextBuilder());
+    updateConfiguration();
   }
 
   @Reference(cardinality = ReferenceCardinality.OPTIONAL, policy = ReferencePolicy.DYNAMIC)
   public void setRootObjectBuilder(GraphQLServletRootObjectBuilder rootObjectBuilder) {
     schemaBuilder.setRootObjectBuilder(rootObjectBuilder);
+    updateConfiguration();
   }
 
   public void unsetRootObjectBuilder(GraphQLRootObjectBuilder rootObjectBuilder) {
     schemaBuilder.setRootObjectBuilder(new DefaultGraphQLRootObjectBuilder());
+    updateConfiguration();
   }
 
   @Reference(
@@ -195,10 +218,12 @@ public void unsetRootObjectBuilder(GraphQLRootObjectBuilder rootObjectBuilder) {
       policyOption = ReferencePolicyOption.GREEDY)
   public void setExecutionStrategyProvider(ExecutionStrategyProvider provider) {
     schemaBuilder.setExecutionStrategyProvider(provider);
+    updateConfiguration();
   }
 
   public void unsetExecutionStrategyProvider(ExecutionStrategyProvider provider) {
     schemaBuilder.setExecutionStrategyProvider(new DefaultExecutionStrategyProvider());
+    updateConfiguration();
   }
 
   @Reference(
@@ -207,10 +232,12 @@ public void unsetExecutionStrategyProvider(ExecutionStrategyProvider provider) {
       policyOption = ReferencePolicyOption.GREEDY)
   public void setInstrumentationProvider(InstrumentationProvider provider) {
     schemaBuilder.setInstrumentationProvider(provider);
+    updateConfiguration();
   }
 
   public void unsetInstrumentationProvider(InstrumentationProvider provider) {
     schemaBuilder.setInstrumentationProvider(new NoOpInstrumentationProvider());
+    updateConfiguration();
   }
 
   @Reference(
@@ -219,10 +246,12 @@ public void unsetInstrumentationProvider(InstrumentationProvider provider) {
       policyOption = ReferencePolicyOption.GREEDY)
   public void setErrorHandler(GraphQLErrorHandler errorHandler) {
     schemaBuilder.setErrorHandler(errorHandler);
+    updateConfiguration();
   }
 
   public void unsetErrorHandler(GraphQLErrorHandler errorHandler) {
     schemaBuilder.setErrorHandler(new DefaultGraphQLErrorHandler());
+    updateConfiguration();
   }
 
   @Reference(
@@ -231,10 +260,12 @@ public void unsetErrorHandler(GraphQLErrorHandler errorHandler) {
       policyOption = ReferencePolicyOption.GREEDY)
   public void setPreparsedDocumentProvider(PreparsedDocumentProvider preparsedDocumentProvider) {
     schemaBuilder.setPreparsedDocumentProvider(preparsedDocumentProvider);
+    updateConfiguration();
   }
 
   public void unsetPreparsedDocumentProvider(PreparsedDocumentProvider preparsedDocumentProvider) {
     schemaBuilder.setPreparsedDocumentProvider(NoOpPreparsedDocumentProvider.INSTANCE);
+    updateConfiguration();
   }
 
   @Reference(
@@ -251,6 +282,20 @@ public void unbindCodeRegistryProvider(GraphQLCodeRegistryProvider graphQLCodeRe
     updateSchema();
   }
 
+  @Reference(
+      cardinality = ReferenceCardinality.OPTIONAL,
+      policy = ReferencePolicy.DYNAMIC,
+      policyOption = ReferencePolicyOption.GREEDY)
+  public void bindConfigurationProvider(GraphQLConfigurationProvider graphQLConfigurationProvider) {
+    schemaBuilder.setConfigurationBuilderProvider(graphQLConfigurationProvider);
+    updateSchema();
+  }
+
+  public void unbindConfigurationProvider(GraphQLConfigurationProvider graphQLConfigurationProvider) {
+    schemaBuilder.setConfigurationBuilderProvider(GraphQLConfiguration.Builder::new);
+    updateSchema();
+  }
+
   @interface Config {
 
     int schema_update_delay() default 0;

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/OsgiSchemaBuilder.java

@@ -24,8 +24,8 @@
 import graphql.kickstart.servlet.core.GraphQLServletRootObjectBuilder;
 import graphql.kickstart.servlet.input.GraphQLInvocationInputFactory;
 import graphql.kickstart.servlet.osgi.GraphQLCodeRegistryProvider;
-import graphql.kickstart.servlet.osgi.GraphQLFieldProvider;
 import graphql.kickstart.servlet.osgi.GraphQLDirectiveProvider;
+import graphql.kickstart.servlet.osgi.GraphQLConfigurationProvider;
 import graphql.kickstart.servlet.osgi.GraphQLMutationProvider;
 import graphql.kickstart.servlet.osgi.GraphQLQueryProvider;
 import graphql.kickstart.servlet.osgi.GraphQLSubscriptionProvider;
@@ -43,10 +43,12 @@
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
+import lombok.Getter;
 import lombok.Setter;
 
 @Setter
-class OsgiSchemaBuilder {
+public class OsgiSchemaBuilder {
 
   private final List<GraphQLQueryProvider> queryProviders = new ArrayList<>();
   private final List<GraphQLMutationProvider> mutationProviders = new ArrayList<>();
@@ -65,23 +67,25 @@ class OsgiSchemaBuilder {
       NoOpPreparsedDocumentProvider.INSTANCE;
   private GraphQLCodeRegistryProvider codeRegistryProvider =
       () -> GraphQLCodeRegistry.newCodeRegistry().build();
+  private GraphQLConfigurationProvider configurationBuilderProvider = GraphQLConfiguration.Builder::new;
 
   private GraphQLSchemaServletProvider schemaProvider;
 
-  private ScheduledExecutorService executor;
+  private ScheduledExecutorService schemaExecutor;
   private ScheduledFuture<?> updateFuture;
   private int schemaUpdateDelay;
+  @Getter private GraphQLConfiguration configuration;
 
   void activate(int schemaUpdateDelay) {
     this.schemaUpdateDelay = schemaUpdateDelay;
     if (schemaUpdateDelay != 0) {
-      executor = Executors.newSingleThreadScheduledExecutor();
+      schemaExecutor = Executors.newSingleThreadScheduledExecutor();
     }
   }
 
   void deactivate() {
-    if (executor != null) {
-      executor.shutdown();
+    if (schemaExecutor != null) {
+      schemaExecutor.shutdown();
     }
   }
 
@@ -94,7 +98,7 @@ void updateSchema() {
       }
 
       updateFuture =
-          executor.schedule(this::doUpdateSchema, schemaUpdateDelay, TimeUnit.MILLISECONDS);
+          schemaExecutor.schedule(this::doUpdateSchema, schemaUpdateDelay, TimeUnit.MILLISECONDS);
     }
   }
 
@@ -140,25 +144,21 @@ private Set<GraphQLType> buildTypes() {
   }
 
   private GraphQLObjectType buildMutationType() {
-    return buildObjectType("Mutation", new ArrayList<>(mutationProviders));
+    return buildObjectType("Mutation", mutationProviders.stream().flatMap(s -> s.getMutations().stream()));
   }
 
   private GraphQLObjectType buildSubscriptionType() {
-    return buildObjectType("Subscription", new ArrayList<>(subscriptionProviders));
+    return buildObjectType("Subscription", subscriptionProviders.stream().flatMap(s -> s.getSubscriptions().stream()));
   }
 
-  private GraphQLObjectType buildObjectType(String name, List<GraphQLFieldProvider> providers) {
-    if (!providers.isEmpty()) {
-      final GraphQLObjectType.Builder typeBuilder =
-          newObject().name(name).description("Root " + name.toLowerCase() + " type");
+  private GraphQLObjectType buildObjectType(String name, Stream<GraphQLFieldDefinition> fields) {
+    final GraphQLObjectType.Builder typeBuilder =
+        newObject().name(name).description("Root " + name.toLowerCase() + " type");
 
-      for (GraphQLFieldProvider provider : providers) {
-        provider.getFields().forEach(typeBuilder::field);
-      }
+    fields.forEach(typeBuilder::field);
 
-      if (!typeBuilder.build().getFieldDefinitions().isEmpty()) {
-        return typeBuilder.build();
-      }
+    if (!typeBuilder.build().getFieldDefinitions().isEmpty()) {
+      return typeBuilder.build();
     }
     return null;
   }
@@ -214,8 +214,9 @@ GraphQLSchemaServletProvider getSchemaProvider() {
     return schemaProvider;
   }
 
-  GraphQLConfiguration buildConfiguration() {
-    return GraphQLConfiguration.with(buildInvocationInputFactory())
+  void updateConfiguration() {
+    configuration = configurationBuilderProvider.getConfigurationBuilder()
+        .with(buildInvocationInputFactory())
         .with(buildQueryInvoker())
         .with(buildObjectMapper())
         .with(listeners)

graphql-java-servlet/src/main/java/graphql/kickstart/servlet/osgi/GraphQLConfigurationProvider.java

@@ -0,0 +1,8 @@
+package graphql.kickstart.servlet.osgi;
+
+import graphql.kickstart.servlet.GraphQLConfiguration;
+
+public interface GraphQLConfigurationProvider extends GraphQLProvider {
+
+  GraphQLConfiguration.Builder getConfigurationBuilder();
+}

graphql-java-servlet/src/test/groovy/graphql/kickstart/servlet/OsgiGraphQLHttpServletSpec.groovy

@@ -336,6 +336,7 @@ class OsgiGraphQLHttpServletSpec extends Specification {
 
     when:
     servlet.unsetContextBuilder(contextBuilder)
+    servlet.updateSchema()
     then:
     servlet.configuration.invocationInputFactory.create(request).executionInput.context instanceof DefaultGraphQLContext
   }
@@ -356,6 +357,7 @@ class OsgiGraphQLHttpServletSpec extends Specification {
 
     when:
     servlet.unsetRootObjectBuilder(rootObjectBuilder)
+    servlet.updateSchema()
     then:
     servlet.configuration.invocationInputFactory.create(request).executionInput.root != rootObject
   }
@@ -376,6 +378,7 @@ class OsgiGraphQLHttpServletSpec extends Specification {
 
     when:
     servlet.unsetExecutionStrategyProvider(executionStrategy)
+    servlet.updateSchema()
     def invocationInput2 = servlet.configuration.invocationInputFactory.create(request)
     servlet.configuration.graphQLInvoker.query(invocationInput2)