check "plugin already installed" before jar hell check.

In the case of a plugin using the deprecated `isolated=false` functionality this will cause confusion otherwise. Closes elastic#14205 Closes elastic#14207
javanna · Oct 20, 2015 · d9ac96e · d9ac96e
1 parent e1a7cc2
commit d9ac96e
Show file tree

Hide file tree

Showing 2 changed files with 46 additions and 5 deletions.
diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java
@@ -216,18 +216,18 @@ private void extract(PluginHandle pluginHandle, Terminal terminal, Path pluginFi
         PluginInfo info = PluginInfo.readFromProperties(root);
         terminal.println(VERBOSE, "%s", info);
 
-        // check for jar hell before any copying
-        if (info.isJvm()) {
-            jarHellCheck(root, info.isIsolated());
-        }
-
         // update name in handle based on 'name' property found in descriptor file
         pluginHandle = new PluginHandle(info.getName(), pluginHandle.version, pluginHandle.user);
         final Path extractLocation = pluginHandle.extractedDir(environment);
         if (Files.exists(extractLocation)) {
             throw new IOException("plugin directory " + extractLocation.toAbsolutePath() + " already exists. To update the plugin, uninstall it first using 'remove " + pluginHandle.name + "' command");
         }
 
+        // check for jar hell before any copying
+        if (info.isJvm()) {
+            jarHellCheck(root, info.isIsolated());
+        }
+
         // install plugin
         FileSystemUtils.copyDirectoryRecursively(root, extractLocation);
         terminal.println("Installed %s into %s", pluginHandle.name, extractLocation.toAbsolutePath());

diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginManagerIT.java b/core/src/test/java/org/elasticsearch/plugins/PluginManagerIT.java
@@ -62,13 +62,15 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.SimpleFileVisitor;
+import java.nio.file.StandardOpenOption;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.nio.file.attribute.PosixFileAttributeView;
 import java.nio.file.attribute.PosixFileAttributes;
 import java.nio.file.attribute.PosixFilePermission;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
+import java.util.jar.JarOutputStream;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipOutputStream;
 
@@ -390,6 +392,45 @@ public void testInstallPlugin() throws IOException {
         assertThatPluginIsListed(pluginName);
     }
 
+    /**
+     * @deprecated support for this is not going to stick around, seriously.
+     */
+    @Deprecated
+    public void testAlreadyInstalledNotIsolated() throws Exception {
+        String pluginName = "fake-plugin";
+        Path pluginDir = createTempDir().resolve(pluginName);
+        Files.createDirectories(pluginDir);
+        // create a jar file in the plugin
+        Path pluginJar = pluginDir.resolve("fake-plugin.jar");
+        try (ZipOutputStream out = new JarOutputStream(Files.newOutputStream(pluginJar, StandardOpenOption.CREATE))) {
+            out.putNextEntry(new ZipEntry("foo.class"));
+            out.closeEntry();
+        }
+        String pluginUrl = createPlugin(pluginDir,
+            "description", "fake desc",
+            "name", pluginName,
+            "version", "1.0",
+            "elasticsearch.version", Version.CURRENT.toString(),
+            "java.version", System.getProperty("java.specification.version"),
+            "isolated", "false",
+            "jvm", "true",
+            "classname", "FakePlugin");
+
+        // install
+        ExitStatus status = new PluginManagerCliParser(terminal).execute(args("install " + pluginUrl));
+        assertEquals("unexpected exit status: output: " + terminal.getTerminalOutput(), ExitStatus.OK, status);
+
+        // install again
+        status = new PluginManagerCliParser(terminal).execute(args("install " + pluginUrl));
+        List<String> output = terminal.getTerminalOutput();
+        assertEquals("unexpected exit status: output: " + output, ExitStatus.IO_ERROR, status);
+        boolean foundExpectedMessage = false;
+        for (String line : output) {
+            foundExpectedMessage |= line.contains("already exists");
+        }
+        assertTrue(foundExpectedMessage);
+    }
+
     @Test
     public void testInstallSitePlugin() throws IOException {
         String pluginName = "fake-plugin";