You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by pl...@apache.org on 2015/10/18 19:25:52 UTC
incubator-tinkerpop git commit: Cleanup after failed plugin install
Repository: incubator-tinkerpop
Updated Branches:
refs/heads/master 2250c56c0 -> 41453ea01
Cleanup after failed plugin install
Project: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/commit/41453ea0
Tree: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/tree/41453ea0
Diff: http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/diff/41453ea0
Branch: refs/heads/master
Commit: 41453ea01b2f1bde389e2ef718af8b58402bd3e1
Parents: 2250c56
Author: Jason Plurad <pl...@apache.org>
Authored: Sun Oct 18 17:24:13 2015 +0000
Committer: Jason Plurad <pl...@apache.org>
Committed: Sun Oct 18 17:24:13 2015 +0000
----------------------------------------------------------------------
CHANGELOG.asciidoc | 2 +
.../groovy/util/DependencyGrabber.groovy | 73 +++++++++-----
.../groovy/util/DependencyGrabberTest.java | 101 +++++++++++++++++++
3 files changed, 153 insertions(+), 23 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/41453ea0/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 1162b8b..13f3516 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -75,6 +75,8 @@ 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`.
+* 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.
* Added a shell script that verifies source and binary distributions.
http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/41453ea0/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 f8065d7..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
@@ -47,6 +47,19 @@ class DependencyGrabber {
this.extensionDirectory = extensionDirectory
}
+ def String deleteDependenciesFromPath(final Artifact artifact) {
+ final def dep = makeDepsMap(artifact)
+ 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}"
+ }
+ else {
+ f.deleteDir()
+ return "Uninstalled ${dep.module}"
+ }
+ }
+
def Set<String> copyDependenciesToPath(final Artifact artifact) {
final def dep = makeDepsMap(artifact)
final String extClassPath = getPathFromDependency(dep)
@@ -55,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(":") }
@@ -82,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/41453ea0/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..95e4c9c
--- /dev/null
+++ b/gremlin-groovy/src/test/java/org/apache/tinkerpop/gremlin/groovy/util/DependencyGrabberTest.java
@@ -0,0 +1,101 @@
+/*
+ * 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.junit.AfterClass;
+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");
+ private static final DependencyGrabber dg = new DependencyGrabber(dummyClassLoader, extTestDir.getAbsolutePath());
+
+ @AfterClass
+ public static void tearDown() {
+ FileUtils.deleteQuietly(extTestDir);
+ }
+
+ @Test
+ public void should0_InstallAndUninstallDependencies() {
+ final String pkg = "org.apache.tinkerpop";
+ final String name = "tinkergraph-gremlin";
+ final String ver = "3.0.1-incubating";
+ 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 should1_ThrowIllegalStateException() {
+ final String pkg = "org.apache.tinkerpop";
+ final String name = "gremlin-groovy";
+ final String ver = "3.0.1-incubating";
+ final Artifact a = new Artifact(pkg, name, ver);
+
+ // install the plugin for the first time
+ final File pluginDir = new File(extTestDir, name);
+ dg.copyDependenciesToPath(a);
+ 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;
+ }
+ }
+
+ @Test(expected=RuntimeException.class)
+ public void should2_ThrowRuntimeException() {
+ final String pkg = "org.apache.tinkerpop";
+ final String name = "gremlin-bogus";
+ final String ver = "3.0.1-incubating";
+ final Artifact a = new Artifact(pkg, name, ver);
+
+ // attempt to install bogus plugin
+ try {
+ dg.copyDependenciesToPath(a);
+ } catch (RuntimeException re) {
+ // validate that the plugin dir was deleted
+ final File pluginDir = new File(extTestDir, name);
+ assertFalse(pluginDir.exists());
+ // throw the RuntimeException
+ throw re;
+ }
+ }
+}