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.

Sign up for GitHub

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 package.json js-plugins support #4389

Merged
merged 9 commits into from
Oct 5, 2023
Merged

Add package.json js-plugins support #4389

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
29c18bb
Add multi-homed, package.json JS plugin support
devinrsmith Aug 28, 2023
8d1d20f
Fix
devinrsmith Aug 28, 2023
d5e3631
Consolidate paths
devinrsmith Aug 29, 2023
a6d70d7
Do an move when copying plugins
devinrsmith Aug 29, 2023
b2d7503
JsPlugin
devinrsmith Sep 1, 2023
eaac8d9
Fix test compile
devinrsmith Sep 2, 2023
a4cc53c
Merge remote-tracking branch 'upstream/main' into multi-homed-package…
devinrsmith Sep 11, 2023
cd9b816
Review points
devinrsmith Sep 13, 2023
3a51bc8
More review points
devinrsmith Oct 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions plugin/src/main/java/io/deephaven/plugin/js/JsPlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,4 @@
*/
public interface JsPlugin extends Plugin {

<T> T walk(Visitor<T> visitor);

interface Visitor<T> {
T visit(JsPluginPackagePath packageRoot);

T visit(JsPluginManifestPath manifestRoot);
}
}
5 changes: 0 additions & 5 deletions plugin/src/main/java/io/deephaven/plugin/js/JsPluginManifestPath.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,4 @@ public final Path manifestJson() {
public final JsPluginPackagePath packagePath(String name) {
return JsPluginPackagePath.of(path().resolve(name));
}

@Override
public final <T> T walk(JsPlugin.Visitor<T> visitor) {
return visitor.visit(this);
}
}
5 changes: 0 additions & 5 deletions plugin/src/main/java/io/deephaven/plugin/js/JsPluginPackagePath.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,4 @@ public static JsPluginPackagePath of(Path packageRoot) {
public final Path packageJson() {
return path().resolve(PACKAGE_JSON);
}

@Override
public final <T> T walk(JsPlugin.Visitor<T> visitor) {
return visitor.visit(this);
}
}
35 changes: 10 additions & 25 deletions server/jetty/src/main/java/io/deephaven/server/jetty/JsPlugins.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
package io.deephaven.server.jetty;

import io.deephaven.plugin.js.JsPlugin;
import io.deephaven.plugin.js.JsPlugin.Visitor;
import io.deephaven.plugin.js.JsPluginManifestPath;
import io.deephaven.plugin.js.JsPluginPackagePath;

Expand Down Expand Up @@ -37,33 +36,19 @@ public URI filesystem() {

@Override
public void accept(JsPlugin jsPlugin) {
final IOException ioException = jsPlugin.walk(new CopyToZipFilesystem());
if (ioException != null) {
throw new UncheckedIOException(ioException);
}
}

private class CopyToZipFilesystem implements Visitor<IOException> {

@Override
public IOException visit(JsPluginPackagePath srcPackagePath) {
try {
copy(srcPackagePath, zipFs);
return null;
} catch (IOException e) {
return e;
try {
if (jsPlugin instanceof JsPluginPackagePath) {
copy((JsPluginPackagePath) jsPlugin, zipFs);
return;
}
}

@Override
public IOException visit(JsPluginManifestPath srcManifestPath) {
try {
copyAll(srcManifestPath, zipFs);
return null;
} catch (IOException e) {
return e;
if (jsPlugin instanceof JsPluginManifestPath) {
copyAll((JsPluginManifestPath) jsPlugin, zipFs);
return;
}
} catch (IOException e) {
throw new UncheckedIOException(e);
}
throw new IllegalStateException("Unexpected JsPlugin class: " + jsPlugin.getClass());
}

private static void copy(JsPluginPackagePath srcPackagePath, JsPluginsZipFilesystem dest)
Expand Down
6 changes: 1 addition & 5 deletions server/src/main/java/io/deephaven/server/plugin/js/JsPluginModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
import java.util.Iterator;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;

/**
* Provides the {@link JsPluginManifestPath manifest path} of {@value JS_PLUGIN_RESOURCE_BASE} if the configuration
Expand Down Expand Up @@ -54,10 +53,7 @@ static Set<Registration> providesConfigDirRegistration() {
@Provides
@ElementsIntoSet
static Set<Registration> providesPackageRoots() {
return jsPluginsPackageRoots()
.stream()
.map(Registration.class::cast)
.collect(Collectors.toSet());
return Set.copyOf(jsPluginsPackageRoots());
Copy link
Member

Choose a reason for hiding this comment

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

why not apply to the other unnecessary streams above too?

Copy link
Member Author

Choose a reason for hiding this comment

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

The above aren't streams, but Optionals that turn into Set.of(Element) or Set.of().

Copy link
Member

Choose a reason for hiding this comment

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

providesConfigDirRegistration and providesResourceBaseRegistration, the two methods above this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. @IntoSet might be more appropriate if there is a way to ignore null or optional return values, but I don't suspect that would work...

Copy link
Member

Choose a reason for hiding this comment

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

Ah got it, thanks for clarifying.

}

// deephaven.jsPlugins.resourceBase (manifest root)
Expand Down
1 change: 0 additions & 1 deletion server/src/main/java/io/deephaven/server/runner/DeephavenApiServerComponent.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ public interface DeephavenApiServerComponent {
DeephavenApiServer getServer();

interface Builder<Self extends Builder<Self, Component>, Component extends DeephavenApiServerComponent> {

@BindsInstance
Self withOut(@Nullable @Named("out") PrintStream out);

Expand Down
5 changes: 0 additions & 5 deletions server/src/test/java/io/deephaven/server/runner/DeephavenApiServerTestBase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import dagger.BindsInstance;
import dagger.Component;
import io.deephaven.client.ClientDefaultsModule;
import io.deephaven.configuration.Configuration;
import io.deephaven.engine.context.TestExecutionContext;
import io.deephaven.engine.liveness.LivenessScope;
import io.deephaven.engine.liveness.LivenessScopeStack;
Expand Down Expand Up @@ -65,9 +64,6 @@ interface Builder {
@BindsInstance
Builder withServerConfig(ServerConfig serverConfig);

@BindsInstance
Builder withConfiguration(Configuration config);;

@BindsInstance
Builder withOut(@Named("out") PrintStream out);

Expand Down Expand Up @@ -109,7 +105,6 @@ public void setUp() throws Exception {

serverComponent = DaggerDeephavenApiServerTestBase_TestComponent.builder()
.withServerConfig(config)
.withConfiguration(Configuration.getInstance())
.withAuthorizationProvider(new CommunityAuthorizationProvider())
.withOut(System.out)
.withErr(System.err)
Expand Down
Loading