You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by dk...@apache.org on 2015/10/22 15:08:22 UTC
[02/17] incubator-tinkerpop git commit: added dependency grabber test
added dependency grabber test
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/729396db
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/729396db
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/729396db
Branch: refs/heads/TINKERPOP3-861
Commit: 729396db8811a98c8b10f540ff45e6cf5f1dcd0c
Parents: 955a98f
Author: Jason Plurad <pl...@apache.org>
Authored: Sat Oct 17 18:15:34 2015 -0400
Committer: Jason C. Plurad <pl...@us.ibm.com>
Committed: Sat Oct 17 18:18:55 2015 -0400
----------------------------------------------------------------------
CHANGELOG.asciidoc | 2 +-
.../console/commands/InstallCommand.groovy | 6 --
.../groovy/util/DependencyGrabber.groovy | 62 ++++++++------
.../groovy/util/DependencyGrabberTest.java | 90 ++++++++++++++++++++
.../server/util/GremlinServerInstall.java | 8 +-
5 files changed, 131 insertions(+), 37 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/729396db/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index c30b76e..b60c926 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -25,7 +25,7 @@ image::https://raw.githubusercontent.com/apache/incubator-tinkerpop/master/docs/
TinkerPop 3.0.2 (NOT OFFICIALLY RELEASED YET)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-* Cleaned up `ext` directory when plugin installation fails for `gremlin-server` and `gremlin-console`.
+* Cleaned up `ext/` directory when plugin installation fails for `gremlin-server` and `gremlin-console`.
* Fixed issues in `gremlin-server` when configured for HTTP basic authentication.
* Made `BulkLoaderVertexProgram` work for any persistent TP3-supporting graph (input and output).
* `TreeSideEffectStep` now implements `PathProcessor` which fixed a `ComputerVerificationStrategy` issue.
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/729396db/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/commands/InstallCommand.groovy
----------------------------------------------------------------------
diff --git a/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/commands/InstallCommand.groovy b/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/commands/InstallCommand.groovy
index 2bc7a60..a2efe2a 100644
--- a/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/commands/InstallCommand.groovy
+++ b/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/commands/InstallCommand.groovy
@@ -53,12 +53,6 @@ class InstallCommand extends CommandSupport {
final def pluginsThatNeedRestart = grabDeps(dep)
return "Loaded: " + arguments + (pluginsThatNeedRestart.size() == 0 ? "" : " - restart the console to use $pluginsThatNeedRestart")
} catch (Exception ex) {
- if (!(ex instanceof IllegalStateException)) {
- // IllegalStateException is thrown if a module with the same name is already installed.
- final def uninstall = new UninstallCommand(shell, mediator)
- final List<String> module = Collections.singletonList(artifact.getArtifact())
- uninstall.execute(module)
- }
return ex.message
}
}
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/729396db/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabber.groovy
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabber.groovy b/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabber.groovy
index e17c4a6..6b5242d 100644
--- a/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabber.groovy
+++ b/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabber.groovy
@@ -52,7 +52,7 @@ class DependencyGrabber {
final String extClassPath = getPathFromDependency(dep)
final File f = new File(extClassPath)
if (!f.exists()) {
- return "There is no module with the name ${dep.module} to remove - $extClassPath"
+ return "There is no module with the name ${dep.module} to remove - ${extClassPath}"
}
else {
f.deleteDir()
@@ -68,9 +68,16 @@ class DependencyGrabber {
final File f = new File(extClassPath)
if (f.exists()) throw new IllegalStateException("a module with the name ${dep.module} is already installed")
- if (!f.mkdirs()) throw new IOException("could not create directory at ${f}")
- if (!new File(extLibPath).mkdirs()) throw new IOException("could not create directory at ${extLibPath}")
- if (!new File(extPluginPath).mkdirs()) throw new IOException("could not create directory at ${extPluginPath}")
+
+ try {
+ if (!f.mkdirs()) throw new IOException("could not create directory at ${f}")
+ if (!new File(extLibPath).mkdirs()) throw new IOException("could not create directory at ${extLibPath}")
+ if (!new File(extPluginPath).mkdirs()) throw new IOException("could not create directory at ${extPluginPath}")
+ } catch (IOException ioe) {
+ // installation failed. make sure to cleanup directories.
+ deleteDependenciesFromPath(artifact)
+ throw ioe
+ }
new File(extClassPath + fileSep + "plugin-info.txt").withWriter { out -> out << [artifact.group, artifact.artifact, artifact.version].join(":") }
@@ -95,28 +102,35 @@ class DependencyGrabber {
"structure can lead to unexpected behavior.")
}
- final def dependencyLocations = [] as Set<URI>
- dependencyLocations.addAll(Grape.resolve([classLoader: this.classLoaderToUse], null, dep))
-
- // for the "plugin" path ignore slf4j related jars. they are already in the path and will create duplicate
- // bindings which generate annoying log messages that make you think stuff is wrong. also, don't bring
- // over files that are already on the path. these dependencies will be part of the classpath
- //
- // additional dependencies are outside those pulled by grape and are defined in the manifest of the plugin jar.
- // if a plugin uses that setting, it should force "restart" when the plugin is activated. right now,
- // it is up to the plugin developer to enforce that setting.
- dependencyLocations.collect(convertUriToPath(fs))
+ try {
+ final def dependencyLocations = [] as Set<URI>
+ dependencyLocations.addAll(Grape.resolve([classLoader: this.classLoaderToUse], null, dep))
+
+ // for the "plugin" path ignore slf4j related jars. they are already in the path and will create duplicate
+ // bindings which generate annoying log messages that make you think stuff is wrong. also, don't bring
+ // over files that are already on the path. these dependencies will be part of the classpath
+ //
+ // additional dependencies are outside those pulled by grape and are defined in the manifest of the plugin jar.
+ // if a plugin uses that setting, it should force "restart" when the plugin is activated. right now,
+ // it is up to the plugin developer to enforce that setting.
+ dependencyLocations.collect(convertUriToPath(fs))
+ .findAll { !(it.fileName.toFile().name ==~ /(slf4j|logback\-classic)-.*\.jar/) }
+ .findAll {!filesAlreadyInPath.collect { it.getFileName().toString() }.contains(it.fileName.toFile().name)}
+ .each(copyTo(targetPluginPath))
+ getAdditionalDependencies(targetPluginPath, artifact).collect(convertUriToPath(fs))
.findAll { !(it.fileName.toFile().name ==~ /(slf4j|logback\-classic)-.*\.jar/) }
- .findAll {!filesAlreadyInPath.collect { it.getFileName().toString() }.contains(it.fileName.toFile().name)}
+ .findAll { !filesAlreadyInPath.collect { it.getFileName().toString() }.contains(it.fileName.toFile().name)}
.each(copyTo(targetPluginPath))
- getAdditionalDependencies(targetPluginPath, artifact).collect(convertUriToPath(fs))
- .findAll { !(it.fileName.toFile().name ==~ /(slf4j|logback\-classic)-.*\.jar/) }
- .findAll { !filesAlreadyInPath.collect { it.getFileName().toString() }.contains(it.fileName.toFile().name)}
- .each(copyTo(targetPluginPath))
-
- // get dependencies for the lib path. the lib path should not filter out any jars - used for reference
- dependencyLocations.collect(convertUriToPath(fs)).each(copyTo(targetLibPath))
- getAdditionalDependencies(targetLibPath, artifact).collect(convertUriToPath(fs)).each(copyTo(targetLibPath))
+
+ // get dependencies for the lib path. the lib path should not filter out any jars - used for reference
+ dependencyLocations.collect(convertUriToPath(fs)).each(copyTo(targetLibPath))
+ getAdditionalDependencies(targetLibPath, artifact).collect(convertUriToPath(fs)).each(copyTo(targetLibPath))
+ }
+ catch (Exception e) {
+ // installation failed. make sure to cleanup directories.
+ deleteDependenciesFromPath(artifact)
+ throw e
+ }
// the ordering of jars seems to matter in some cases (e.g. neo4j). the plugin system allows the plugin
// to place a Gremlin-Plugin-Paths entry in the jar manifest file to define where specific jar files should
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/729396db/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabberTest.java
----------------------------------------------------------------------
diff --git a/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabberTest.java b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabberTest.java
new file mode 100644
index 0000000..b6ac3e2
--- /dev/null
+++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabberTest.java
@@ -0,0 +1,90 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.groovy.util;
+
+import groovy.lang.GroovyClassLoader;
+import java.io.File;
+import org.apache.commons.io.FileUtils;
+import org.apache.tinkerpop.gremlin.groovy.plugin.Artifact;
+import org.apache.tinkerpop.gremlin.util.Gremlin;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+/**
+ * @author Jason Plurad (http://github.com/pluradj)
+ */
+public class DependencyGrabberTest {
+ private static final GroovyClassLoader dummyClassLoader = new GroovyClassLoader();
+ private static final File extTestDir = new File(System.getProperty("user.dir"), "/target/test-dep-ext");
+ DependencyGrabber dg = null;
+
+ @Before
+ public void setUp() {
+ FileUtils.deleteQuietly(extTestDir);
+ dg = new DependencyGrabber(dummyClassLoader, extTestDir.getAbsolutePath());
+ }
+
+ @After
+ public void tearDown() {
+ FileUtils.deleteQuietly(extTestDir);
+ }
+
+ @Test
+ public void shouldInstallAndUninstallDependencies() {
+ final String pkg = "org.apache.tinkerpop";
+ final String name = "gremlin-groovy";
+ final String ver = Gremlin.version();
+ final Artifact a = new Artifact(pkg, name, ver);
+
+ // install the plugin
+ final File pluginDir = new File(extTestDir, name);
+ dg.copyDependenciesToPath(a);
+ assertTrue(pluginDir.exists());
+
+ // delete the plugin
+ dg.deleteDependenciesFromPath(a);
+ assertFalse(pluginDir.exists());
+ }
+
+ @Test(expected=IllegalStateException.class)
+ public void shouldThrowIllegalStateException() {
+ final String pkg = "org.apache.tinkerpop";
+ final String name = "gremlin-groovy";
+ final String ver = Gremlin.version();
+ final Artifact a = new Artifact(pkg, name, ver);
+
+ // install the plugin for the first time
+ dg.copyDependenciesToPath(a);
+ final File pluginDir = new File(extTestDir, name);
+ assertTrue(pluginDir.exists());
+
+ // attempt to install plugin a second time
+ try {
+ dg.copyDependenciesToPath(a);
+ } catch (IllegalStateException ise) {
+ // validate that the plugin dir wasn't deleted by accident
+ assertTrue(pluginDir.exists());
+ // throw the IllegalStateException
+ throw ise;
+ }
+ }
+}
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/729396db/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinServerInstall.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinServerInstall.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinServerInstall.java
index ffc8377..03317d3 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinServerInstall.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinServerInstall.java
@@ -32,15 +32,11 @@ public class GremlinServerInstall {
if (arguments.length != 3) {
System.out.println("Usage: <group> <artifact> <version>");
} else {
- final Artifact artifact = new Artifact(arguments[0], arguments[1], arguments[2]);
- final DependencyGrabber grabber = new DependencyGrabber(dummyClassLoader, getExtensionPath());
try {
+ final Artifact artifact = new Artifact(arguments[0], arguments[1], arguments[2]);
+ final DependencyGrabber grabber = new DependencyGrabber(dummyClassLoader, getExtensionPath());
grabber.copyDependenciesToPath(artifact);
} catch (Exception iae) {
- if (!(iae instanceof IllegalStateException)) {
- // IllegalStateException is thrown if a module with the same name is already installed.
- grabber.deleteDependenciesFromPath(artifact);
- }
System.out.println(String.format("Could not install the dependency: %s", iae.getMessage()));
iae.printStackTrace();
}