From 6296f4415d8c0600ff856e60f67717523e5ba4c3 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 27 Nov 2023 15:11:06 -0600 Subject: [PATCH 1/3] Collect JsPlugin instances before starting Jetty This reverses a DI dependency, preventing requiring that the server start up before plugins can be collected. Partial #4040 --- .../plugin/js/JsPluginRegistration.java | 12 ++++++++++++ .../server/jetty/JettyBackedGrpcServer.java | 15 ++------------- .../server/jetty/JettyServerModule.java | 18 ++++++++++++------ .../io/deephaven/server/jetty/JsPlugins.java | 9 +++++++-- .../plugin/PluginRegistrationVisitor.java | 15 +++++++++++---- .../server/plugin/PluginsModule.java | 19 +++++-------------- .../plugin/js/JsPluginNoopConsumerModule.java | 12 +++--------- 7 files changed, 52 insertions(+), 48 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..201e93eada1 --- /dev/null +++ b/plugin/src/main/java/io/deephaven/plugin/js/JsPluginRegistration.java @@ -0,0 +1,12 @@ +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..b6d81758e8e 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,7 +18,11 @@ 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. + */ +class JsPlugins implements JsPluginRegistration { static final String JS_PLUGINS = "js-plugins"; static JsPlugins create() throws IOException { @@ -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..4c6ebdb69bd 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 -> { }; } From 4a69485f1e04d3bd9c11c340130a5459a3bd5105 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 27 Nov 2023 15:46:43 -0600 Subject: [PATCH 2/3] Spotless --- .../java/io/deephaven/plugin/js/JsPluginRegistration.java | 1 + .../io/deephaven/server/plugin/PluginRegistrationVisitor.java | 4 ++-- 2 files changed, 3 insertions(+), 2 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 index 201e93eada1..2271aad2d57 100644 --- a/plugin/src/main/java/io/deephaven/plugin/js/JsPluginRegistration.java +++ b/plugin/src/main/java/io/deephaven/plugin/js/JsPluginRegistration.java @@ -6,6 +6,7 @@ public interface JsPluginRegistration { /** * Handles registration of a {@link JsPlugin} instance. + * * @param plugin the registered plugin */ void register(JsPlugin plugin); 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 4c6ebdb69bd..79d766f66f8 100644 --- a/server/src/main/java/io/deephaven/server/plugin/PluginRegistrationVisitor.java +++ b/server/src/main/java/io/deephaven/server/plugin/PluginRegistrationVisitor.java @@ -14,8 +14,8 @@ import java.util.function.Consumer; /** - * Plugin {@link io.deephaven.plugin.Registration.Callback} implementation that forwards registered plugins - * to a {@link ObjectTypeRegistration} or {@link JsPluginRegistration}. + * 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 { From 2f5d9f3243e9f5bd04edb312d10e8fdf167fe648 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Mon, 27 Nov 2023 17:01:06 -0600 Subject: [PATCH 3/3] Make JsPlugin public --- .../src/main/java/io/deephaven/server/jetty/JsPlugins.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 b6d81758e8e..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 @@ -22,10 +22,10 @@ * Jetty-specific implementation of {@link JsPluginRegistration} to collect plugins and advertise their contents to * connecting client. */ -class JsPlugins implements JsPluginRegistration { +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()); }