From e09bcfdaca4264ecd06a73b0e4acab5f850e88ff Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Tue, 28 Nov 2023 12:07:11 -0600 Subject: [PATCH] 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 --- .../plugin/js/JsPluginRegistration.java | 13 +++++++++++++ .../server/jetty/JettyBackedGrpcServer.java | 15 ++------------- .../server/jetty/JettyServerModule.java | 18 ++++++++++++------ .../io/deephaven/server/jetty/JsPlugins.java | 11 ++++++++--- .../plugin/PluginRegistrationVisitor.java | 15 +++++++++++---- .../server/plugin/PluginsModule.java | 19 +++++-------------- .../plugin/js/JsPluginNoopConsumerModule.java | 12 +++--------- 7 files changed, 54 insertions(+), 49 deletions(-) create mode 100644 plugin/src/main/java/io/deephaven/plugin/js/JsPluginRegistration.java diff --git a/plugin/src/main/java/io/deephaven/plugin/js/JsPluginRegistration.java b/plugin/src/main/java/io/deephaven/plugin/js/JsPluginRegistration.java new file mode 100644 index 00000000000..2271aad2d57 --- /dev/null +++ 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 index f41aafb01f0..7b921233312 100644 --- 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 getEndpointInstance(Class endpointClass) { jetty.setHandler(handler); } - public Consumer 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 index 5e78b77af8b..a50753c0348 100644 --- 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,9 +6,8 @@ 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; @@ -16,11 +15,12 @@ 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 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 index 20861c94e6a..ffebf7b1700 100644 --- 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 { +/** + * 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 index 7b7cb921fd2..79d766f66f8 100644 --- 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 { private final ObjectTypeRegistration objectTypeRegistration; - private final Consumer jsPluginConsumer; + private final JsPluginRegistration jsPluginRegistration; + @Inject PluginRegistrationVisitor( ObjectTypeRegistration objectTypeRegistration, - Consumer 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 index 24c2452b163..8c3b6f27497 100644 --- 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}. * *

- * 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}. * *

* 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 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 index 68097d3adf8..a1b90168be3 100644 --- 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 providesNoopJsPluginConsumer() { + static JsPluginRegistration providesNoopJsPluginConsumer() { return jsPlugin -> { }; }