tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From plur...@apache.org
Subject [2/3] incubator-tinkerpop git commit: added dependency grabber test
Date Sat, 17 Oct 2015 23:21:55 GMT
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/tp30
Commit: 729396db8811a98c8b10f540ff45e6cf5f1dcd0c
Parents: 955a98f
Author: Jason Plurad <pluradj@apache.org>
Authored: Sat Oct 17 18:15:34 2015 -0400
Committer: Jason C. Plurad <pluradj@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();
             }


Mime
View raw message