Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add concrete JsPlugin #4925

Merged
merged 14 commits into from
Dec 14, 2023
2 changes: 1 addition & 1 deletion docker/registry/server-base/gradle.properties
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
io.deephaven.project.ProjectType=DOCKER_REGISTRY
deephaven.registry.imageName=ghcr.io/deephaven/server-base:edge
deephaven.registry.imageId=ghcr.io/deephaven/server-base@sha256:bd59f63831db8ff86c3ebab21ffecd1804e58dcfaaf08a67bf0c2b320a2ce179
deephaven.registry.imageId=ghcr.io/deephaven/server-base@sha256:8f9b993a4ce7c78b50b869be840241a0a9d19d3f4f35601f20cd05475abd5753
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
adbc-driver-manager==0.8.0
adbc-driver-postgresql==0.8.0
connectorx==0.3.2; platform.machine == 'x86_64'
deephaven-plugin==0.5.0
deephaven-plugin==0.6.0
java-utilities==0.2.0
jedi==0.18.2
jpy==0.14.0
Expand Down
2 changes: 1 addition & 1 deletion docker/server/src/main/server-netty/requirements.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
adbc-driver-manager==0.8.0
adbc-driver-postgresql==0.8.0
connectorx==0.3.2; platform.machine == 'x86_64'
deephaven-plugin==0.5.0
deephaven-plugin==0.6.0
java-utilities==0.2.0
jedi==0.18.2
jpy==0.14.0
Expand Down
1 change: 1 addition & 0 deletions plugin/src/main/java/io/deephaven/plugin/Plugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
* A plugin is a structured extension point for user-definable behavior.
*
* @see ObjectType
* @see JsPlugin
*/
public interface Plugin extends Registration {

Expand Down
141 changes: 135 additions & 6 deletions plugin/src/main/java/io/deephaven/plugin/js/JsPlugin.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,145 @@
*/
package io.deephaven.plugin.js;

import io.deephaven.annotations.BuildableStyle;
import io.deephaven.plugin.Plugin;
import io.deephaven.plugin.PluginBase;
import org.immutables.value.Value.Check;
import org.immutables.value.Value.Default;
import org.immutables.value.Value.Immutable;

import java.nio.file.Files;
import java.nio.file.Path;

/**
* A js plugin is a {@link Plugin} that allows adding javascript code under the server's URL path "js-plugins/". See
* <a href="https://github.com/deephaven/deephaven-plugins#js-plugins">deephaven-plugins#js-plugins</a> for more details
* about the underlying construction for js plugins.
* A JS plugin is a {@link Plugin} that allows for custom javascript and related content to be served, see
* {@link io.deephaven.plugin.js}.
*
* <p>
* For example, if the following JS plugin was the only JS plugin installed
*
* <pre>
* JsPlugin.builder()
* .name("foo")
* .version("1.0.0")
* .main(Path.of("dist/index.js"))
* .path(Path.of("/path-to/my-plugin"))
* .build()
* </pre>
*
* the manifest served at "js-plugins/manifest.json" would be equivalent to
*
* <pre>
* {
* "plugins": [
* {
* "name": "foo",
* "version": "1.0.0",
* "main": "dist/index.js"
* }
* ]
* }
* </pre>
*
* @see JsPluginPackagePath
* @see JsPluginManifestPath
* and the file "/path-to/my-plugin/dist/index.js" would be served at "js-plugins/foo/dist/index.js". All other files of
* the form "/path-to/my-plugin/{somePath}" will be served at "js-plugins/foo/{somePath}".
Comment on lines +46 to +47
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems kind of redundant - why not just say "All files of from "/path-to/my-plugin/{somePath}" will be served at "js-plugins/foo/{somePath}"? The main by definition is going to be relative to that path, ergo must always be served.

*/
public interface JsPlugin extends Plugin {
@Immutable
@BuildableStyle
public abstract class JsPlugin extends PluginBase {

public static Builder builder() {
return ImmutableJsPlugin.builder();
}

/**
* The JS plugin name. The JS plugin contents will be served via the URL path "js-plugins/{name}/", as well as
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should note that name can have a / in it for subfolders. Likely important to note that we do not want to URL encode that /, we want it to actually be at js-plugins/@scope/name, not js-plugins/@scope%2Fname

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

* included as the "name" field for the manifest entry in "js-plugins/manifest.json".
*
* @return the name
*/
public abstract String name();

/**
* The JS plugin version. Will be included as the "version" field for the manifest entry in
* "js-plugins/manifest.json".
*
* @return the version
*/
public abstract String version();

/**
* The main JS file path, specified relative to {@link #path()}. The main JS file must exist
* ({@code Files.isRegularFile(root().resolve(main()))}) and must be included in {@link #paths()}. Will be included
* as the "main" field for the manifest entry in "js-plugins/manifest.json".
*
* @return the main JS file path
*/
public abstract Path main();

/**
* The directory path of the resources to serve. The resources will be served via the URL path
* "js-plugins/{name}/{relativeToPath}". The path must exist ({@code Files.isDirectory(path())}).
*
* @return the path
*/
public abstract Path path();

/**
* The subset of resources from {@link #path()} to serve. Production installations should preferably be packaged
* with the exact resources necessary (and thus served with {@link Paths#all()}). During development, other subsets
* may be useful if {@link #path()} contains content unrelated to the JS content. By default, is
* {@link Paths#all()}.
*
* @return the paths
*/
@Default
public Paths paths() {
return Paths.all();
}

@Override
public final <T, V extends Plugin.Visitor<T>> T walk(V visitor) {
return visitor.visit(this);
}

@Check
final void checkPath() {
if (!Files.isDirectory(path())) {
throw new IllegalArgumentException(String.format("path ('%s') must exist and be a directory", path()));
}
}

@Check
final void checkMain() {
final Path mainPath = path().resolve(main());
if (!Files.isRegularFile(mainPath)) {
throw new IllegalArgumentException(String.format("main ('%s') must exist and be a regular file", mainPath));
}
}

@Check
final void checkPaths() {
if (!(paths() instanceof PathsInternal)) {
throw new IllegalArgumentException("Must construct one of the approved Paths");
}
final Path relativeMain = path().relativize(path().resolve(main()));
if (!((PathsInternal) paths()).matches(relativeMain)) {
throw new IllegalArgumentException(String.format("main ('%s') is not in paths", relativeMain));
}
}

public interface Builder {
Builder name(String name);

Builder version(String version);

Builder main(Path main);

Builder path(Path path);

Builder paths(Paths paths);

JsPlugin build();
}
}
15 changes: 0 additions & 15 deletions plugin/src/main/java/io/deephaven/plugin/js/JsPluginBase.java

This file was deleted.

This file was deleted.

This file was deleted.

53 changes: 53 additions & 0 deletions plugin/src/main/java/io/deephaven/plugin/js/Paths.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
/**
* Copyright (c) 2016-2023 Deephaven Data Labs and Patent Pending
*/
package io.deephaven.plugin.js;

import java.nio.file.Path;

/**
* The subset of paths to serve, see {@link JsPlugin#paths()}.
*/
public interface Paths {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might consider a different class name here - when dealing with java.nio.file.Path instances, it is common to use java.nio.file.Paths to create them.

Also, javadoc

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to best rename it if necessary - I'm personally less offended by names that overlap in these regards, although I know others disagree. At least java.nio.file.Paths is somewhat discouraged in their javadoc:

 * @apiNote
 * It is recommended to obtain a {@code Path} via the {@code Path.of} methods
 * instead of via the {@code get} methods defined in this class as this class
 * may be deprecated in a future release.


/**
* Includes all paths.
*
* @return the paths
*/
static Paths all() {
return PathsAll.ALL;
}

/**
* Includes only the paths that are prefixed by {@code prefix}.
*
* @param prefix the prefix
* @return the paths
*/
static Paths ofPrefixes(Path prefix) {
// Note: we have specific overload for single element to explicitly differentiate from Iterable overload since
// Path extends Iterable<Path>.
return PathsPrefixes.builder().addPrefixes(prefix).build();
}

/**
* Includes only the paths that are prefixed by one of {@code prefixes}.
*
* @param prefixes the prefixes
* @return the paths
*/
static Paths ofPrefixes(Path... prefixes) {
return PathsPrefixes.builder().addPrefixes(prefixes).build();
}

/**
* Includes only the paths that are prefixed by one of {@code prefixes}.
*
* @param prefixes the prefixes
* @return the paths
*/
static Paths ofPrefixes(Iterable<? extends Path> prefixes) {
return PathsPrefixes.builder().addAllPrefixes(prefixes).build();
}
}
15 changes: 15 additions & 0 deletions plugin/src/main/java/io/deephaven/plugin/js/PathsAll.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* Copyright (c) 2016-2023 Deephaven Data Labs and Patent Pending
*/
package io.deephaven.plugin.js;

import java.nio.file.Path;

enum PathsAll implements PathsInternal {
ALL;

@Override
public boolean matches(Path path) {
return true;
}
}
10 changes: 10 additions & 0 deletions plugin/src/main/java/io/deephaven/plugin/js/PathsInternal.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/**
* Copyright (c) 2016-2023 Deephaven Data Labs and Patent Pending
*/
package io.deephaven.plugin.js;

import java.nio.file.PathMatcher;

interface PathsInternal extends Paths, PathMatcher {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsolicited advice, and unimportant for this iteration, but might be handy if considering using PathMatcher more generally: FileSystems.getDefault().getPathMatcher(...) supports glob: patterns, but if you do, take care to note that **/* does not match files in the current dir (that is, something which * would match).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah - I considered making PathMatcher the public API, but I don't know what sort of interface limitations we might wish we had imposed if we want an "easy" route-based integration w/ jetty.


}
Loading
Loading