Collect JsPlugin instances before starting Jetty (#4885)

This reverses a DI dependency, preventing requiring that the server start up before plugins can be collected. Partial #4040
deephaven · Nov 28, 2023 · e09bcfd · e09bcfd
1 parent de0e134
commit e09bcfd
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 49 deletions.
diff --git a/plugin/src/main/java/io/deephaven/plugin/js/JsPluginRegistration.java b/plugin/src/main/java/io/deephaven/plugin/js/JsPluginRegistration.java
@@ -0,0 +1,13 @@
+package io.deephaven.plugin.js;
+
+/**
+ * Observes registration of {@link JsPlugin} instances.
+ */
+public interface JsPluginRegistration {
+    /**
+     * Handles registration of a {@link JsPlugin} instance.
+     * 
+     * @param plugin the registered plugin
+     */
+    void register(JsPlugin plugin);
+}
diff --git a/server/jetty/src/main/java/io/deephaven/server/jetty/JettyBackedGrpcServer.java b/server/jetty/src/main/java/io/deephaven/server/jetty/JettyBackedGrpcServer.java
@@ -3,7 +3,6 @@
  */
 package io.deephaven.server.jetty;
 
-import io.deephaven.plugin.js.JsPlugin;
 import io.deephaven.server.browserstreaming.BrowserStreamInterceptor;
 import io.deephaven.server.runner.GrpcServer;
 import io.deephaven.ssl.config.CiphersIntermediate;
@@ -73,7 +72,6 @@
 import java.util.Map;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
-import java.util.function.Consumer;
 import java.util.function.Supplier;
 
 import static io.grpc.servlet.web.websocket.MultiplexedWebSocketServerStream.GRPC_WEBSOCKETS_MULTIPLEX_PROTOCOL;
@@ -85,20 +83,15 @@ public class JettyBackedGrpcServer implements GrpcServer {
     private static final String JS_PLUGINS_PATH_SPEC = "/" + JsPlugins.JS_PLUGINS + "/*";
 
     private final Server jetty;
-    private final JsPlugins jsPlugins;
     private final boolean websocketsEnabled;
 
     @Inject
     public JettyBackedGrpcServer(
             final JettyConfig config,
-            final GrpcFilter filter) {
+            final GrpcFilter filter,
+            final JsPlugins jsPlugins) {
         jetty = new Server();
         jetty.addConnector(createConnector(jetty, config));
-        try {
-            jsPlugins = JsPlugins.create();
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
-        }
 
         final WebAppContext context =
                 new WebAppContext(null, "/", null, null, null, new ErrorPageErrorHandler(), NO_SESSIONS);
@@ -246,10 +239,6 @@ public <T> T getEndpointInstance(Class<T> endpointClass) {
         jetty.setHandler(handler);
     }
 
-    public Consumer<JsPlugin> jsPluginConsumer() {
-        return jsPlugins;
-    }
-
     @Override
     public void start() throws IOException {
         try {

diff --git a/server/jetty/src/main/java/io/deephaven/server/jetty/JettyServerModule.java b/server/jetty/src/main/java/io/deephaven/server/jetty/JettyServerModule.java
@@ -6,21 +6,21 @@
 import dagger.Binds;
 import dagger.Module;
 import dagger.Provides;
-import io.deephaven.plugin.js.JsPlugin;
+import io.deephaven.plugin.js.JsPluginRegistration;
 import io.deephaven.server.config.ServerConfig;
-import io.deephaven.server.plugin.PluginsModule;
 import io.deephaven.server.runner.GrpcServer;
 import io.grpc.BindableService;
 import io.grpc.ServerInterceptor;
 import io.grpc.servlet.jakarta.ServletAdapter;
 import io.grpc.servlet.jakarta.ServletServerBuilder;
 
 import javax.inject.Named;
+import java.io.IOException;
+import java.io.UncheckedIOException;
 import java.util.Set;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
-import java.util.function.Consumer;
 
 import static io.grpc.internal.GrpcUtil.getThreadFactory;
 
@@ -67,9 +67,15 @@ static ServletAdapter provideGrpcServletAdapter(
         return serverBuilder.buildServletAdapter();
     }
 
+    @Binds
+    JsPluginRegistration bindJsPlugins(JsPlugins plugins);
+
     @Provides
-    @Named(PluginsModule.JS_PLUGIN_CONSUMER_NAME)
-    static Consumer<JsPlugin> providesJsPluginConsumer(JettyBackedGrpcServer server) {
-        return server.jsPluginConsumer();
+    static JsPlugins providesJsPluginRegistration() {
+        try {
+            return JsPlugins.create();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
     }
 }
diff --git a/server/jetty/src/main/java/io/deephaven/server/jetty/JsPlugins.java b/server/jetty/src/main/java/io/deephaven/server/jetty/JsPlugins.java
@@ -6,6 +6,7 @@
 import io.deephaven.plugin.js.JsPlugin;
 import io.deephaven.plugin.js.JsPluginManifestPath;
 import io.deephaven.plugin.js.JsPluginPackagePath;
+import io.deephaven.plugin.js.JsPluginRegistration;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -17,10 +18,14 @@
 
 import static io.deephaven.server.jetty.Json.OBJECT_MAPPER;
 
-class JsPlugins implements Consumer<JsPlugin> {
+/**
+ * Jetty-specific implementation of {@link JsPluginRegistration} to collect plugins and advertise their contents to
+ * connecting client.
+ */
+public class JsPlugins implements JsPluginRegistration {
     static final String JS_PLUGINS = "js-plugins";
 
-    static JsPlugins create() throws IOException {
+    public static JsPlugins create() throws IOException {
         return new JsPlugins(JsPluginsZipFilesystem.create());
     }
 
@@ -35,7 +40,7 @@ public URI filesystem() {
     }
 
     @Override
-    public void accept(JsPlugin jsPlugin) {
+    public void register(JsPlugin jsPlugin) {
         try {
             if (jsPlugin instanceof JsPluginPackagePath) {
                 copy((JsPluginPackagePath) jsPlugin, zipFs);

diff --git a/server/src/main/java/io/deephaven/server/plugin/PluginRegistrationVisitor.java b/server/src/main/java/io/deephaven/server/plugin/PluginRegistrationVisitor.java
@@ -5,23 +5,30 @@
 
 import io.deephaven.plugin.Plugin;
 import io.deephaven.plugin.js.JsPlugin;
+import io.deephaven.plugin.js.JsPluginRegistration;
 import io.deephaven.plugin.type.ObjectType;
 import io.deephaven.plugin.type.ObjectTypeRegistration;
 
+import javax.inject.Inject;
 import java.util.Objects;
 import java.util.function.Consumer;
 
+/**
+ * Plugin {@link io.deephaven.plugin.Registration.Callback} implementation that forwards registered plugins to a
+ * {@link ObjectTypeRegistration} or {@link JsPluginRegistration}.
+ */
 final class PluginRegistrationVisitor
         implements io.deephaven.plugin.Registration.Callback, Plugin.Visitor<PluginRegistrationVisitor> {
 
     private final ObjectTypeRegistration objectTypeRegistration;
-    private final Consumer<JsPlugin> jsPluginConsumer;
+    private final JsPluginRegistration jsPluginRegistration;
 
+    @Inject
     PluginRegistrationVisitor(
             ObjectTypeRegistration objectTypeRegistration,
-            Consumer<JsPlugin> jsPluginConsumer) {
+            JsPluginRegistration jsPluginRegistration) {
         this.objectTypeRegistration = Objects.requireNonNull(objectTypeRegistration);
-        this.jsPluginConsumer = Objects.requireNonNull(jsPluginConsumer);
+        this.jsPluginRegistration = Objects.requireNonNull(jsPluginRegistration);
     }
 
     @Override
@@ -37,7 +44,7 @@ public PluginRegistrationVisitor visit(ObjectType objectType) {
 
     @Override
     public PluginRegistrationVisitor visit(JsPlugin jsPlugin) {
-        jsPluginConsumer.accept(jsPlugin);
+        jsPluginRegistration.register(jsPlugin);
         return this;
     }
 }
diff --git a/server/src/main/java/io/deephaven/server/plugin/PluginsModule.java b/server/src/main/java/io/deephaven/server/plugin/PluginsModule.java
@@ -3,25 +3,21 @@
  */
 package io.deephaven.server.plugin;
 
+import dagger.Binds;
 import dagger.Module;
-import dagger.Provides;
 import io.deephaven.plugin.PluginModule;
 import io.deephaven.plugin.Registration;
 import io.deephaven.plugin.Registration.Callback;
 import io.deephaven.plugin.js.JsPlugin;
-import io.deephaven.plugin.type.ObjectTypeRegistration;
 import io.deephaven.server.plugin.js.JsPluginModule;
 import io.deephaven.server.plugin.type.ObjectTypesModule;
 
-import javax.inject.Named;
-import java.util.function.Consumer;
-
 /**
  * Includes the {@link Module modules} necessary to provide {@link PluginRegistration}.
  *
  * <p>
- * Downstream servers will need to provide an appropriate {@link JsPlugin} {@link Consumer} {@link Named named}
- * {@value JS_PLUGIN_CONSUMER_NAME}, or include {@link io.deephaven.server.plugin.js.JsPluginNoopConsumerModule}.
+ * Downstream servers will need to provide an appropriate {@link JsPlugin} implementation, or include
+ * {@link io.deephaven.server.plugin.js.JsPluginNoopConsumerModule}.
  *
  * <p>
  * Note: runtime plugin registration is not currently supported - ie, no {@link Callback} is provided. See
@@ -33,12 +29,7 @@
  */
 @Module(includes = {ObjectTypesModule.class, JsPluginModule.class, PluginModule.class})
 public interface PluginsModule {
-    String JS_PLUGIN_CONSUMER_NAME = "JsPlugin";
 
-    @Provides
-    static Registration.Callback providesPluginRegistrationCallback(
-            ObjectTypeRegistration objectTypeRegistration,
-            @Named(JS_PLUGIN_CONSUMER_NAME) Consumer<JsPlugin> jsPluginConsumer) {
-        return new PluginRegistrationVisitor(objectTypeRegistration, jsPluginConsumer);
-    }
+    @Binds
+    Registration.Callback bindPluginRegistrationCallback(PluginRegistrationVisitor visitor);
 }
diff --git a/server/src/main/java/io/deephaven/server/plugin/js/JsPluginNoopConsumerModule.java b/server/src/main/java/io/deephaven/server/plugin/js/JsPluginNoopConsumerModule.java
@@ -5,22 +5,16 @@
 
 import dagger.Module;
 import dagger.Provides;
-import io.deephaven.plugin.js.JsPlugin;
-import io.deephaven.server.plugin.PluginsModule;
-
-import javax.inject.Named;
-import java.util.function.Consumer;
+import io.deephaven.plugin.js.JsPluginRegistration;
 
 /**
- * Provides a no-op {@link JsPlugin} {@link Consumer} {@link Named named}
- * {@value PluginsModule#JS_PLUGIN_CONSUMER_NAME}.
+ * Provides a no-op {@link JsPluginRegistration} for servers that don't host JS content.
  */
 @Module
 public interface JsPluginNoopConsumerModule {
 
     @Provides
-    @Named(PluginsModule.JS_PLUGIN_CONSUMER_NAME)
-    static Consumer<JsPlugin> providesNoopJsPluginConsumer() {
+    static JsPluginRegistration providesNoopJsPluginConsumer() {
         return jsPlugin -> {
         };
     }