From 82eb5d299a37754eef8623e7e759cbbf32d029f1 Mon Sep 17 00:00:00 2001
From: Robert Muir <rmuir@apache.org>
Date: Tue, 20 Oct 2015 09:58:20 -0400
Subject: [PATCH] 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 #14205
Closes #14207
---
 .../elasticsearch/plugins/PluginManager.java  | 10 ++---
 .../plugins/PluginManagerIT.java              | 41 +++++++++++++++++++
 2 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java
index 700c0f7be22..f319e64f5e7 100644
--- a/core/src/main/java/org/elasticsearch/plugins/PluginManager.java
+++ b/core/src/main/java/org/elasticsearch/plugins/PluginManager.java
@@ -220,11 +220,6 @@ public class PluginManager {
         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);
@@ -232,6 +227,11 @@ public class PluginManager {
             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());
+        }
+
         // read optional security policy (extra permissions)
         // if it exists, confirm or warn the user
         Path policy = root.resolve(PluginInfo.ES_PLUGIN_POLICY);
diff --git a/core/src/test/java/org/elasticsearch/plugins/PluginManagerIT.java b/core/src/test/java/org/elasticsearch/plugins/PluginManagerIT.java
index e98794cfc99..4706612e155 100644
--- a/core/src/test/java/org/elasticsearch/plugins/PluginManagerIT.java
+++ b/core/src/test/java/org/elasticsearch/plugins/PluginManagerIT.java
@@ -57,6 +57,7 @@ import java.nio.file.FileVisitResult;
 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;
@@ -64,6 +65,7 @@ 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;
 
@@ -407,6 +409,45 @@ public class PluginManagerIT extends ESIntegTestCase {
         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);
+    }
+
     public void testInstallSitePluginVerbose() throws IOException {
         String pluginName = "fake-plugin";
         Path pluginDir = createTempDir().resolve(pluginName);