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
111 changes: 105 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,115 @@
*/
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 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
niloc132 marked this conversation as resolved.
Show resolved Hide resolved
* about the underlying construction for js plugins.
*
* @see JsPluginPackagePath
* @see JsPluginManifestPath
* about the underlying construction for JS plugins.
*/
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 #root()}. 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 root directory of the resources to serve. The root must exist ({@code Files.isDirectory(root())}).
*
* @return the root
*/
public abstract Path root();

/**
* The paths to serve, specified relative to {@link #root()}. The resources will be served via the URL path
* "js-plugins/{name}/{pathRelativeToRoot}". 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 checkRoot() {
if (!Files.isDirectory(root())) {
throw new IllegalArgumentException(String.format("root ('%s') must exist and be a directory", root()));
}
}

@Check
final void checkMain() {
final Path mainPath = root().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 = root().relativize(root().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 root(Path root);

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.

50 changes: 50 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,50 @@
/**
* Copyright (c) 2016-2023 Deephaven Data Labs and Patent Pending
*/
package io.deephaven.plugin.js;

import java.nio.file.Path;

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.INSTANCE;
}

/**
* 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 {
INSTANCE;

@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.


}
53 changes: 53 additions & 0 deletions plugin/src/main/java/io/deephaven/plugin/js/PathsPrefixes.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 io.deephaven.annotations.BuildableStyle;
import org.immutables.value.Value.Check;
import org.immutables.value.Value.Immutable;

import java.nio.file.Path;
import java.util.Set;

@Immutable
@BuildableStyle
abstract class PathsPrefixes implements PathsInternal {

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

public abstract Set<Path> prefixes();

@Override
public final boolean matches(Path path) {
if (prefixes().contains(path)) {
return true;
}
// Note: we could make a more efficient impl w/ a tree-based approach based on the names
for (Path prefix : prefixes()) {
if (path.startsWith(prefix)) {
return true;
}
}
return false;
}

@Check
final void checkPrefixesNonEmpty() {
if (prefixes().isEmpty()) {
throw new IllegalArgumentException("prefixes must be non-empty");
}
}

interface Builder {
Builder addPrefixes(Path element);

Builder addPrefixes(Path... elements);

Builder addAllPrefixes(Iterable<? extends Path> elements);

PathsPrefixes build();
}
}
Loading
Loading