From 4b6828f0e98b5284ff0eacda9bcd8796bc6b8d10 Mon Sep 17 00:00:00 2001 From: simonbrandhof Date: Thu, 30 Sep 2010 09:37:00 +0000 Subject: [PATCH] the property sonar.core.codeCoveragePlugin is not used when it's set from UI console --- .../sonar/batch/BatchPluginRepository.java | 34 +++++------ .../java/org/sonar/batch/ProjectBatch.java | 4 +- .../batch/BatchPluginRepositoryTest.java | 58 ++++++++++--------- .../core/plugin/AbstractPluginRepository.java | 5 +- .../plugin/AbstractPluginRepositoryTest.java | 3 +- .../plugins/ServerPluginRepository.java | 3 +- .../plugins/ServerPluginRepositoryTest.java | 12 ++-- 7 files changed, 64 insertions(+), 55 deletions(-) diff --git a/sonar-batch/src/main/java/org/sonar/batch/BatchPluginRepository.java b/sonar-batch/src/main/java/org/sonar/batch/BatchPluginRepository.java index 6ee190fabec..891d8e1fbf7 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/BatchPluginRepository.java +++ b/sonar-batch/src/main/java/org/sonar/batch/BatchPluginRepository.java @@ -24,11 +24,13 @@ import org.apache.commons.configuration.Configuration; import org.apache.commons.lang.ArrayUtils; import org.apache.commons.lang.StringUtils; import org.picocontainer.MutablePicoContainer; +import org.picocontainer.PicoContainer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.BatchExtension; import org.sonar.api.Plugin; import org.sonar.api.batch.AbstractCoverageExtension; +import org.sonar.api.resources.Project; import org.sonar.api.utils.SonarException; import org.sonar.core.plugin.AbstractPluginRepository; import org.sonar.core.plugin.JpaPlugin; @@ -48,19 +50,17 @@ public class BatchPluginRepository extends AbstractPluginRepository { private Map classloaders; private String baseUrl; private JpaPluginDao dao; - private Configuration configuration; - public BatchPluginRepository(JpaPluginDao dao, ServerMetadata server, Configuration configuration) { + public BatchPluginRepository(JpaPluginDao dao, ServerMetadata server) { this.dao = dao; this.baseUrl = server.getUrl() + "/deploy/plugins/"; - this.configuration = configuration; } /** - * only for unit tests + * for unit tests only */ - BatchPluginRepository(Configuration configuration) { - this.configuration = configuration; + BatchPluginRepository() { + } public void start() { @@ -109,15 +109,6 @@ public class BatchPluginRepository extends AbstractPluginRepository { invokeExtensionProviders(pico); } - boolean shouldRegisterCoverageExtension(String pluginKey) { - String[] selectedPluginKeys = configuration.getStringArray(AbstractCoverageExtension.PARAM_PLUGIN); - if (ArrayUtils.isEmpty(selectedPluginKeys)) { - selectedPluginKeys = new String[]{AbstractCoverageExtension.DEFAULT_PLUGIN}; - } - String oldCoveragePluginKey = getOldCoveragePluginKey(pluginKey); - return ArrayUtils.contains(selectedPluginKeys, pluginKey) || ArrayUtils.contains(selectedPluginKeys, oldCoveragePluginKey); - } - private String getOldCoveragePluginKey(String pluginKey) { if (StringUtils.equals("sonar-jacoco-plugin", pluginKey)) { return "jacoco"; @@ -129,14 +120,23 @@ public class BatchPluginRepository extends AbstractPluginRepository { } @Override - protected boolean shouldRegisterExtension(String pluginKey, Object extension) { + protected boolean shouldRegisterExtension(PicoContainer container, String pluginKey, Object extension) { boolean ok = isType(extension, BatchExtension.class); if (ok && isType(extension, AbstractCoverageExtension.class)) { - ok = shouldRegisterCoverageExtension(pluginKey); + ok = shouldRegisterCoverageExtension(pluginKey, container.getComponent(Configuration.class)); if (!ok) { LOG.debug("The following extension is ignored: " + extension + ". See the parameter " + AbstractCoverageExtension.PARAM_PLUGIN); } } return ok; } + + boolean shouldRegisterCoverageExtension(String pluginKey, Configuration conf) { + String[] selectedPluginKeys = conf.getStringArray(AbstractCoverageExtension.PARAM_PLUGIN); + if (ArrayUtils.isEmpty(selectedPluginKeys)) { + selectedPluginKeys = new String[]{AbstractCoverageExtension.DEFAULT_PLUGIN}; + } + String oldCoveragePluginKey = getOldCoveragePluginKey(pluginKey); + return ArrayUtils.contains(selectedPluginKeys, pluginKey) || ArrayUtils.contains(selectedPluginKeys, oldCoveragePluginKey); + } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/ProjectBatch.java b/sonar-batch/src/main/java/org/sonar/batch/ProjectBatch.java index eb0f9b18db8..d47ed8d6939 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/ProjectBatch.java +++ b/sonar-batch/src/main/java/org/sonar/batch/ProjectBatch.java @@ -68,7 +68,6 @@ public class ProjectBatch { public void startChildContainer(DefaultSonarIndex index, Project project) { batchContainer = globalContainer.makeChildContainer(); - batchContainer.getComponent(BatchPluginRepository.class).registerPlugins(batchContainer); batchContainer.as(Characteristics.CACHE).addComponent(project); batchContainer.as(Characteristics.CACHE).addComponent(project.getPom()); @@ -76,6 +75,9 @@ public class ProjectBatch { batchContainer.as(Characteristics.CACHE).addComponent(index.getBucket(project).getSnapshot()); batchContainer.as(Characteristics.CACHE).addComponent(project.getConfiguration()); + //need to be registered after the Configuration + batchContainer.getComponent(BatchPluginRepository.class).registerPlugins(batchContainer); + batchContainer.as(Characteristics.CACHE).addComponent(DaoFacade.class); batchContainer.as(Characteristics.CACHE).addComponent(RulesDao.class); batchContainer.as(Characteristics.CACHE).addComponent(org.sonar.api.database.daos.RulesDao.class); diff --git a/sonar-batch/src/test/java/org/sonar/batch/BatchPluginRepositoryTest.java b/sonar-batch/src/test/java/org/sonar/batch/BatchPluginRepositoryTest.java index 337d40f81e1..604c8296206 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/BatchPluginRepositoryTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/BatchPluginRepositoryTest.java @@ -22,9 +22,11 @@ package org.sonar.batch; import org.apache.commons.configuration.Configuration; import org.apache.commons.configuration.PropertiesConfiguration; import org.junit.Test; +import org.picocontainer.MutablePicoContainer; import org.sonar.api.BatchExtension; import org.sonar.api.ServerExtension; import org.sonar.api.batch.AbstractCoverageExtension; +import org.sonar.api.utils.IocContainer; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertThat; @@ -33,62 +35,64 @@ public class BatchPluginRepositoryTest { @Test public void shouldRegisterBatchExtension() { - BatchPluginRepository repository = new BatchPluginRepository(new PropertiesConfiguration()); + MutablePicoContainer pico = IocContainer.buildPicoContainer(); + pico.addComponent(new PropertiesConfiguration()); + BatchPluginRepository repository = new BatchPluginRepository(); // check classes - assertThat(repository.shouldRegisterExtension("foo", FakeBatchExtension.class), is(true)); - assertThat(repository.shouldRegisterExtension("foo", FakeServerExtension.class), is(false)); - assertThat(repository.shouldRegisterExtension("foo", String.class), is(false)); + assertThat(repository.shouldRegisterExtension(pico, "foo", FakeBatchExtension.class), is(true)); + assertThat(repository.shouldRegisterExtension(pico, "foo", FakeServerExtension.class), is(false)); + assertThat(repository.shouldRegisterExtension(pico, "foo", String.class), is(false)); // check objects - assertThat(repository.shouldRegisterExtension("foo", new FakeBatchExtension()), is(true)); - assertThat(repository.shouldRegisterExtension("foo", new FakeServerExtension()), is(false)); - assertThat(repository.shouldRegisterExtension("foo", "bar"), is(false)); + assertThat(repository.shouldRegisterExtension(pico, "foo", new FakeBatchExtension()), is(true)); + assertThat(repository.shouldRegisterExtension(pico, "foo", new FakeServerExtension()), is(false)); + assertThat(repository.shouldRegisterExtension(pico, "foo", "bar"), is(false)); } @Test public void shouldRegisterOnlyCoberturaExtensionByDefault() { - Configuration conf = new PropertiesConfiguration(); - BatchPluginRepository repository = new BatchPluginRepository(conf); - assertThat(repository.shouldRegisterCoverageExtension("cobertura"), is(true)); - assertThat(repository.shouldRegisterCoverageExtension("clover"), is(false)); + BatchPluginRepository repository = new BatchPluginRepository(); + PropertiesConfiguration conf = new PropertiesConfiguration(); + assertThat(repository.shouldRegisterCoverageExtension("cobertura", conf), is(true)); + assertThat(repository.shouldRegisterCoverageExtension("clover", conf), is(false)); } @Test public void shouldRegisterCustomCoverageExtension() { Configuration conf = new PropertiesConfiguration(); conf.setProperty(AbstractCoverageExtension.PARAM_PLUGIN, "clover,phpunit"); - BatchPluginRepository repository = new BatchPluginRepository(conf); - assertThat(repository.shouldRegisterCoverageExtension("cobertura"), is(false)); - assertThat(repository.shouldRegisterCoverageExtension("clover"), is(true)); - assertThat(repository.shouldRegisterCoverageExtension("phpunit"), is(true)); - assertThat(repository.shouldRegisterCoverageExtension("other"), is(false)); + BatchPluginRepository repository = new BatchPluginRepository(); + assertThat(repository.shouldRegisterCoverageExtension("cobertura", conf), is(false)); + assertThat(repository.shouldRegisterCoverageExtension("clover", conf), is(true)); + assertThat(repository.shouldRegisterCoverageExtension("phpunit", conf), is(true)); + assertThat(repository.shouldRegisterCoverageExtension("other", conf), is(false)); } @Test public void shouldActivateOldVersionOfEmma() { Configuration conf = new PropertiesConfiguration(); conf.setProperty(AbstractCoverageExtension.PARAM_PLUGIN, "emma"); - BatchPluginRepository repository = new BatchPluginRepository(conf); + BatchPluginRepository repository = new BatchPluginRepository(); - assertThat(repository.shouldRegisterCoverageExtension("sonar-emma-plugin"), is(true)); - assertThat(repository.shouldRegisterCoverageExtension("emma"), is(true)); + assertThat(repository.shouldRegisterCoverageExtension("sonar-emma-plugin", conf), is(true)); + assertThat(repository.shouldRegisterCoverageExtension("emma", conf), is(true)); - assertThat(repository.shouldRegisterCoverageExtension("sonar-jacoco-plugin"), is(false)); - assertThat(repository.shouldRegisterCoverageExtension("jacoco"), is(false)); - assertThat(repository.shouldRegisterCoverageExtension("clover"), is(false)); - assertThat(repository.shouldRegisterCoverageExtension("cobertura"), is(false)); + assertThat(repository.shouldRegisterCoverageExtension("sonar-jacoco-plugin", conf), is(false)); + assertThat(repository.shouldRegisterCoverageExtension("jacoco", conf), is(false)); + assertThat(repository.shouldRegisterCoverageExtension("clover", conf), is(false)); + assertThat(repository.shouldRegisterCoverageExtension("cobertura", conf), is(false)); } @Test public void shouldActivateOldVersionOfJacoco() { Configuration conf = new PropertiesConfiguration(); conf.setProperty(AbstractCoverageExtension.PARAM_PLUGIN, "cobertura,jacoco"); - BatchPluginRepository repository = new BatchPluginRepository(conf); + BatchPluginRepository repository = new BatchPluginRepository(); - assertThat(repository.shouldRegisterCoverageExtension("sonar-jacoco-plugin"), is(true)); - assertThat(repository.shouldRegisterCoverageExtension("jacoco"), is(true)); - assertThat(repository.shouldRegisterCoverageExtension("emma"), is(false)); + assertThat(repository.shouldRegisterCoverageExtension("sonar-jacoco-plugin", conf), is(true)); + assertThat(repository.shouldRegisterCoverageExtension("jacoco", conf), is(true)); + assertThat(repository.shouldRegisterCoverageExtension("emma", conf), is(false)); } public static class FakeBatchExtension implements BatchExtension { diff --git a/sonar-core/src/main/java/org/sonar/core/plugin/AbstractPluginRepository.java b/sonar-core/src/main/java/org/sonar/core/plugin/AbstractPluginRepository.java index 9a15c9e06ba..a1e51e95d9a 100644 --- a/sonar-core/src/main/java/org/sonar/core/plugin/AbstractPluginRepository.java +++ b/sonar-core/src/main/java/org/sonar/core/plugin/AbstractPluginRepository.java @@ -24,6 +24,7 @@ import com.google.common.collect.HashBiMap; import com.google.common.collect.Maps; import org.picocontainer.Characteristics; import org.picocontainer.MutablePicoContainer; +import org.picocontainer.PicoContainer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.sonar.api.*; @@ -70,7 +71,7 @@ public abstract class AbstractPluginRepository implements PluginRepository { } private void registerExtension(MutablePicoContainer container, Plugin plugin, String pluginKey, Object extension) { - if (shouldRegisterExtension(pluginKey, extension)) { + if (shouldRegisterExtension(container, pluginKey, extension)) { LOG.debug("Register the extension: {}", extension); container.as(Characteristics.CACHE).addComponent(getExtensionKey(extension), extension); pluginByExtension.put(extension, plugin); @@ -78,7 +79,7 @@ public abstract class AbstractPluginRepository implements PluginRepository { } } - protected abstract boolean shouldRegisterExtension(String pluginKey, Object extension); + protected abstract boolean shouldRegisterExtension(PicoContainer container, String pluginKey, Object extension); public Collection getPlugins() { return pluginByKey.values(); diff --git a/sonar-core/src/test/java/org/sonar/core/plugin/AbstractPluginRepositoryTest.java b/sonar-core/src/test/java/org/sonar/core/plugin/AbstractPluginRepositoryTest.java index de2343a07f5..6e1aa0d93de 100644 --- a/sonar-core/src/test/java/org/sonar/core/plugin/AbstractPluginRepositoryTest.java +++ b/sonar-core/src/test/java/org/sonar/core/plugin/AbstractPluginRepositoryTest.java @@ -21,6 +21,7 @@ package org.sonar.core.plugin; import org.junit.Test; import org.picocontainer.MutablePicoContainer; +import org.picocontainer.PicoContainer; import org.sonar.api.BatchExtension; import org.sonar.api.ExtensionProvider; import org.sonar.api.Plugin; @@ -72,7 +73,7 @@ public class AbstractPluginRepositoryTest { MutablePicoContainer pico = IocContainer.buildPicoContainer(); AbstractPluginRepository repository = new AbstractPluginRepository() { @Override - protected boolean shouldRegisterExtension(String pluginKey, Object extension) { + protected boolean shouldRegisterExtension(PicoContainer container, String pluginKey, Object extension) { return isType(extension, ServerExtension.class); } }; diff --git a/sonar-server/src/main/java/org/sonar/server/plugins/ServerPluginRepository.java b/sonar-server/src/main/java/org/sonar/server/plugins/ServerPluginRepository.java index 318a5ac5769..bf14cd42349 100644 --- a/sonar-server/src/main/java/org/sonar/server/plugins/ServerPluginRepository.java +++ b/sonar-server/src/main/java/org/sonar/server/plugins/ServerPluginRepository.java @@ -21,6 +21,7 @@ package org.sonar.server.plugins; import org.picocontainer.Characteristics; import org.picocontainer.MutablePicoContainer; +import org.picocontainer.PicoContainer; import org.sonar.api.Plugin; import org.sonar.api.ServerExtension; import org.sonar.api.utils.SonarException; @@ -64,7 +65,7 @@ public class ServerPluginRepository extends AbstractPluginRepository { } @Override - protected boolean shouldRegisterExtension(String pluginKey, Object extension) { + protected boolean shouldRegisterExtension(PicoContainer container, String pluginKey, Object extension) { return isType(extension, ServerExtension.class); } } diff --git a/sonar-server/src/test/java/org/sonar/server/plugins/ServerPluginRepositoryTest.java b/sonar-server/src/test/java/org/sonar/server/plugins/ServerPluginRepositoryTest.java index d6d3115e806..99190fe2beb 100644 --- a/sonar-server/src/test/java/org/sonar/server/plugins/ServerPluginRepositoryTest.java +++ b/sonar-server/src/test/java/org/sonar/server/plugins/ServerPluginRepositoryTest.java @@ -32,14 +32,14 @@ public class ServerPluginRepositoryTest { ServerPluginRepository repository = new ServerPluginRepository(); // check classes - assertThat(repository.shouldRegisterExtension("foo", FakeBatchExtension.class), is(false)); - assertThat(repository.shouldRegisterExtension("foo", FakeServerExtension.class), is(true)); - assertThat(repository.shouldRegisterExtension("foo", String.class), is(false)); + assertThat(repository.shouldRegisterExtension(null, "foo", FakeBatchExtension.class), is(false)); + assertThat(repository.shouldRegisterExtension(null, "foo", FakeServerExtension.class), is(true)); + assertThat(repository.shouldRegisterExtension(null, "foo", String.class), is(false)); // check objects - assertThat(repository.shouldRegisterExtension("foo", new FakeBatchExtension()), is(false)); - assertThat(repository.shouldRegisterExtension("foo", new FakeServerExtension()), is(true)); - assertThat(repository.shouldRegisterExtension("foo", "foo"), is(false)); + assertThat(repository.shouldRegisterExtension(null, "foo", new FakeBatchExtension()), is(false)); + assertThat(repository.shouldRegisterExtension(null, "foo", new FakeServerExtension()), is(true)); + assertThat(repository.shouldRegisterExtension(null, "foo", "foo"), is(false)); } public static class FakeBatchExtension implements BatchExtension { -- 2.39.5