From: Godin Date: Thu, 23 Sep 2010 16:18:26 +0000 (+0000) Subject: SONAR-1780: X-Git-Tag: 2.6~962 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=1006c3556b73e8f07a4446bf9626a866c6f8fa9a;p=sonarqube.git SONAR-1780: * An exception should be thrown in case when two Sonar plugins try to use the same key * Add message to log, when file with plugin was deleted --- diff --git a/sonar-server/src/main/java/org/sonar/server/plugins/PluginDeployer.java b/sonar-server/src/main/java/org/sonar/server/plugins/PluginDeployer.java index 4cfb69828d7..9da1dd0aa11 100644 --- a/sonar-server/src/main/java/org/sonar/server/plugins/PluginDeployer.java +++ b/sonar-server/src/main/java/org/sonar/server/plugins/PluginDeployer.java @@ -32,12 +32,18 @@ import org.sonar.api.utils.ZipUtils; import org.sonar.core.plugin.JpaPlugin; import org.sonar.core.plugin.JpaPluginDao; import org.sonar.server.platform.DefaultServerFileSystem; +import org.sonar.server.platform.ServerStartException; + +import com.google.common.collect.Maps; import java.io.File; import java.io.IOException; import java.net.URL; import java.net.URLClassLoader; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; +import java.util.Map; public final class PluginDeployer implements ServerComponent { @@ -47,8 +53,8 @@ public final class PluginDeployer implements ServerComponent { private DefaultServerFileSystem fileSystem; private JpaPluginDao dao; private PluginClassLoaders classloaders; - private Map pluginByKeys = new HashMap(); - private List deprecatedPlugins = new ArrayList(); + private Map pluginByKeys = Maps.newHashMap(); + private Map deprecatedPlugins = Maps.newHashMap(); public PluginDeployer(Server server, DefaultServerFileSystem fileSystem, JpaPluginDao dao, PluginClassLoaders classloaders) { this.server = server; @@ -105,13 +111,14 @@ public final class PluginDeployer implements ServerComponent { } private void deployDeprecatedPlugins() throws IOException { - for (PluginMetadata deprecatedPlugin : deprecatedPlugins) { - if (deprecatedPlugin.getKey() != null && !pluginByKeys.containsKey(deprecatedPlugin.getKey())) { + for (PluginMetadata deprecatedPlugin : deprecatedPlugins.values()) { + PluginMetadata metadata = pluginByKeys.get(deprecatedPlugin.getKey()); + if (metadata != null) { + FileUtils.deleteQuietly(deprecatedPlugin.getSourceFile()); + Logs.INFO.info("Old plugin " + deprecatedPlugin.getFilename() + " replaced by new " + metadata.getFilename()); + } else { pluginByKeys.put(deprecatedPlugin.getKey(), deprecatedPlugin); deploy(deprecatedPlugin); - - } else { - FileUtils.deleteQuietly(deprecatedPlugin.getSourceFile()); } } } @@ -153,22 +160,22 @@ public final class PluginDeployer implements ServerComponent { private void loadCorePlugins() throws IOException { for (File file : fileSystem.getCorePlugins()) { - registerPluginMetadata(file, true); + registerPluginMetadata(file, true, false); } } private void loadUserPlugins() throws IOException { for (File file : fileSystem.getUserPlugins()) { - registerPluginMetadata(file, false); + registerPluginMetadata(file, false, false); } } private void moveAndLoadDownloadedPlugins() throws IOException { if (fileSystem.getDownloadedPluginsDir().exists()) { - Collection jars = FileUtils.listFiles(fileSystem.getDownloadedPluginsDir(), new String[]{"jar"}, false); + Collection jars = FileUtils.listFiles(fileSystem.getDownloadedPluginsDir(), new String[] { "jar" }, false); for (File jar : jars) { File movedJar = moveDownloadedFile(jar); - registerPluginMetadata(movedJar, false); + registerPluginMetadata(movedJar, false, true); } } } @@ -179,28 +186,39 @@ public final class PluginDeployer implements ServerComponent { return new File(fileSystem.getUserPluginsDir(), jar.getName()); } catch (IOException e) { - LOG.error("Fail to move the downloaded file:" + jar.getAbsolutePath(), e); + LOG.error("Fail to move the downloaded file: " + jar.getAbsolutePath(), e); return null; } } - private void registerPluginMetadata(File file, boolean corePlugin) throws IOException { + private void registerPluginMetadata(File file, boolean corePlugin, boolean canDeleteOld) throws IOException { PluginMetadata metadata = PluginMetadata.createFromJar(file, corePlugin); String pluginKey = metadata.getKey(); if (pluginKey != null) { - PluginMetadata existing = pluginByKeys.get(pluginKey); - if (existing != null) { - FileUtils.deleteQuietly(existing.getSourceFile()); - pluginByKeys.remove(pluginKey); - } - pluginByKeys.put(pluginKey, metadata); - + registerPluginMetadata(pluginByKeys, file, metadata, canDeleteOld); } else if (metadata.isOldManifest()) { loadDeprecatedPlugin(metadata); - deprecatedPlugins.add(metadata); + registerPluginMetadata(deprecatedPlugins, file, metadata, canDeleteOld); } } + private void registerPluginMetadata(Map map, File file, PluginMetadata metadata, boolean canDeleteOld) { + String pluginKey = metadata.getKey(); + PluginMetadata existing = map.get(pluginKey); + if (existing != null) { + if (canDeleteOld) { + FileUtils.deleteQuietly(existing.getSourceFile()); + map.remove(pluginKey); + Logs.INFO.info("Old plugin " + existing.getFilename() + " replaced by new " + metadata.getFilename()); + } else { + throw new ServerStartException("Found two plugins with the same key '" + pluginKey + "': " + + metadata.getFilename() + " and " + + existing.getFilename()); + } + } + map.put(metadata.getKey(), metadata); + } + private void loadDeprecatedPlugin(PluginMetadata plugin) throws IOException { // URLClassLoader locks files on Windows // => copy the file before in a temp directory @@ -209,7 +227,7 @@ public final class PluginDeployer implements ServerComponent { String mainClass = plugin.getMainClass(); try { - URLClassLoader pluginClassLoader = URLClassLoader.newInstance(new URL[]{tempFile.toURI().toURL()}, getClass().getClassLoader()); + URLClassLoader pluginClassLoader = URLClassLoader.newInstance(new URL[] { tempFile.toURI().toURL() }, getClass().getClassLoader()); Plugin pluginInstance = (Plugin) pluginClassLoader.loadClass(mainClass).newInstance(); plugin.setKey(pluginInstance.getKey()); plugin.setDescription(pluginInstance.getDescription()); @@ -218,6 +236,9 @@ public final class PluginDeployer implements ServerComponent { } catch (Exception e) { throw new RuntimeException("The plugin main class can not be created: plugin=" + plugin.getFilename() + ", class=" + mainClass, e); } - } + if (StringUtils.isBlank(plugin.getKey())) { + throw new ServerStartException("Found plugin with empty key: " + plugin.getFilename()); + } + } } diff --git a/sonar-server/src/test/java/org/sonar/server/plugins/PluginDeployerTest.java b/sonar-server/src/test/java/org/sonar/server/plugins/PluginDeployerTest.java index 2b4d6753623..4087029435a 100644 --- a/sonar-server/src/test/java/org/sonar/server/plugins/PluginDeployerTest.java +++ b/sonar-server/src/test/java/org/sonar/server/plugins/PluginDeployerTest.java @@ -31,6 +31,7 @@ import org.sonar.core.plugin.JpaPluginFile; import org.sonar.jpa.test.AbstractDbUnitTestCase; import org.sonar.server.platform.DefaultServerFileSystem; import org.sonar.server.platform.ServerImpl; +import org.sonar.server.platform.ServerStartException; import org.sonar.test.TestUtils; import java.io.File; @@ -39,8 +40,8 @@ import java.text.ParseException; import java.text.SimpleDateFormat; import java.util.List; -import static junit.framework.Assert.assertNotNull; import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertThat; public class PluginDeployerTest extends AbstractDbUnitTestCase { @@ -55,7 +56,7 @@ public class PluginDeployerTest extends AbstractDbUnitTestCase { @Rule public TestName name = new TestName(); - + @Before public void start() throws ParseException { server = new ServerImpl("1", "2.2", new SimpleDateFormat("yyyy-MM-dd").parse("2010-05-18")); @@ -67,7 +68,6 @@ public class PluginDeployerTest extends AbstractDbUnitTestCase { deployer = new PluginDeployer(server, fileSystem, dao, classloaders); } - @Test public void deployPlugin() throws IOException { setupData("shared"); @@ -75,7 +75,7 @@ public class PluginDeployerTest extends AbstractDbUnitTestCase { // check that the plugin is registered in database List plugins = dao.getPlugins(); - assertThat(plugins.size(), is(1)); // no more checkstyle + assertThat(plugins.size(), is(1)); // no more checkstyle JpaPlugin plugin = plugins.get(0); assertThat(plugin.getName(), is("Foo")); assertThat(plugin.getFiles().size(), is(1)); @@ -101,7 +101,7 @@ public class PluginDeployerTest extends AbstractDbUnitTestCase { // check that the plugin is registered in database List plugins = dao.getPlugins(); - assertThat(plugins.size(), is(1)); // no more checkstyle + assertThat(plugins.size(), is(1)); // no more checkstyle JpaPlugin plugin = plugins.get(0); assertThat(plugin.getKey(), is("build-breaker")); assertThat(plugin.getFiles().size(), is(1)); @@ -128,7 +128,7 @@ public class PluginDeployerTest extends AbstractDbUnitTestCase { // check that the plugin is registered in database List plugins = dao.getPlugins(); - assertThat(plugins.size(), is(1)); // no more checkstyle + assertThat(plugins.size(), is(1)); // no more checkstyle JpaPlugin plugin = plugins.get(0); assertThat(plugin.getFiles().size(), is(2)); JpaPluginFile pluginFile = plugin.getFiles().get(1); @@ -153,7 +153,19 @@ public class PluginDeployerTest extends AbstractDbUnitTestCase { // check that the plugin is registered in database List plugins = dao.getPlugins(); - assertThat(plugins.size(), is(0)); + assertThat(plugins.size(), is(0)); + } + + @Test(expected = ServerStartException.class) + public void failIfTwoPluginsWithSameKey() throws IOException { + setupData("shared"); + deployer.start(); + } + + @Test(expected = ServerStartException.class) + public void failIfTwoDeprecatedPluginsWithSameKey() throws IOException { + setupData("shared"); + deployer.start(); } } diff --git a/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoDeprecatedPluginsWithSameKey/extensions/plugins/sonar-build-breaker-plugin-0.1.jar b/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoDeprecatedPluginsWithSameKey/extensions/plugins/sonar-build-breaker-plugin-0.1.jar new file mode 100644 index 00000000000..862614d26c7 Binary files /dev/null and b/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoDeprecatedPluginsWithSameKey/extensions/plugins/sonar-build-breaker-plugin-0.1.jar differ diff --git a/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoDeprecatedPluginsWithSameKey/extensions/plugins/sonar-build-breaker-plugin-0.2.jar b/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoDeprecatedPluginsWithSameKey/extensions/plugins/sonar-build-breaker-plugin-0.2.jar new file mode 100644 index 00000000000..862614d26c7 Binary files /dev/null and b/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoDeprecatedPluginsWithSameKey/extensions/plugins/sonar-build-breaker-plugin-0.2.jar differ diff --git a/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoPluginsWithSameKey/extensions/plugins/foo-plugin1.jar b/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoPluginsWithSameKey/extensions/plugins/foo-plugin1.jar new file mode 100644 index 00000000000..7bcf027151a Binary files /dev/null and b/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoPluginsWithSameKey/extensions/plugins/foo-plugin1.jar differ diff --git a/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoPluginsWithSameKey/extensions/plugins/foo-plugin2.jar b/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoPluginsWithSameKey/extensions/plugins/foo-plugin2.jar new file mode 100644 index 00000000000..7bcf027151a Binary files /dev/null and b/sonar-server/src/test/resources/org/sonar/server/plugins/PluginDeployerTest/failIfTwoPluginsWithSameKey/extensions/plugins/foo-plugin2.jar differ