From fed89923245dae44352fed1c2ff5d40bc933cdda Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 30 Mar 2016 12:15:28 +0200 Subject: [PATCH] SONAR-6732 use a SettingsLoad to load task settings allows support of startable components in the CE Task container which would read settings in their own start() method --- .../org/sonar/ce/settings/SettingsLoader.java | 46 ++++++++++++++++++ .../container/ComputeEngineContainerImpl.java | 2 +- .../computation/container/EagerStart.java | 33 +++++++++++++ ...ReportComputeEngineContainerPopulator.java | 2 + .../report/ReportTaskProcessor.java | 16 ++++--- .../properties/ProjectSettingsFactory.java | 2 +- .../sonar/ce/settings/SettingsLoaderTest.java | 47 +++++++++++++++++++ .../ComputeEngineContainerImplTest.java | 45 ++++++++++++++++-- .../step/ComputationStepsTest.java | 5 +- 9 files changed, 184 insertions(+), 14 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/ce/settings/SettingsLoader.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/computation/container/EagerStart.java create mode 100644 server/sonar-server/src/test/java/org/sonar/ce/settings/SettingsLoaderTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/ce/settings/SettingsLoader.java b/server/sonar-server/src/main/java/org/sonar/ce/settings/SettingsLoader.java new file mode 100644 index 00000000000..aa926fabe76 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/ce/settings/SettingsLoader.java @@ -0,0 +1,46 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.settings; + +import org.picocontainer.Startable; +import org.sonar.server.computation.container.EagerStart; + +/** + * Add this class as the first components in the {@link org.sonar.server.computation.container.ComputeEngineContainerImpl} + * to trigger loading of Thread local specific {@link org.sonar.api.config.Settings} in {@link ThreadLocalSettings}. + */ +@EagerStart +public class SettingsLoader implements Startable { + private final ThreadLocalSettings threadLocalSettings; + + public SettingsLoader(ThreadLocalSettings threadLocalSettings) { + this.threadLocalSettings = threadLocalSettings; + } + + @Override + public void start() { + threadLocalSettings.load(); + } + + @Override + public void stop() { + threadLocalSettings.remove(); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/container/ComputeEngineContainerImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/container/ComputeEngineContainerImpl.java index f0e6d0e62bd..ffcb2f8f836 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/container/ComputeEngineContainerImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/container/ComputeEngineContainerImpl.java @@ -75,7 +75,7 @@ public class ComputeEngineContainerImpl extends ComponentContainer implements Co ReflectionLifecycleStrategy lifecycleStrategy = new ReflectionLifecycleStrategy(componentMonitor, "start", "stop", "close") { @Override public boolean isLazy(ComponentAdapter adapter) { - return true; + return adapter.getComponentImplementation().getAnnotation(EagerStart.class) == null; } }; diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/container/EagerStart.java b/server/sonar-server/src/main/java/org/sonar/server/computation/container/EagerStart.java new file mode 100644 index 00000000000..dad0732a672 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/container/EagerStart.java @@ -0,0 +1,33 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.server.computation.container; + +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; + +/** + * Components with this annotation will be eagerly started when loaded into the {@link ComputeEngineContainerImpl}. + */ +@Retention(RetentionPolicy.RUNTIME) +@Target(ElementType.TYPE) +public @interface EagerStart { +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/container/ReportComputeEngineContainerPopulator.java b/server/sonar-server/src/main/java/org/sonar/server/computation/container/ReportComputeEngineContainerPopulator.java index c0c9797d736..d348ee895c3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/container/ReportComputeEngineContainerPopulator.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/container/ReportComputeEngineContainerPopulator.java @@ -23,6 +23,7 @@ import java.util.Arrays; import java.util.List; import javax.annotation.Nullable; import org.sonar.ce.queue.CeTask; +import org.sonar.ce.settings.SettingsLoader; import org.sonar.core.issue.tracking.Tracker; import org.sonar.core.platform.ContainerPopulator; import org.sonar.plugin.ce.ReportAnalysisComponentProvider; @@ -105,6 +106,7 @@ public final class ReportComputeEngineContainerPopulator implements ContainerPop @Override public void populateContainer(ComputeEngineContainer container) { ComputationSteps steps = new ReportComputationSteps(container); + container.add(SettingsLoader.class); container.add(task); container.add(steps); container.addSingletons(componentClasses()); diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/report/ReportTaskProcessor.java b/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/report/ReportTaskProcessor.java index 0a24deab12a..aa1668150a6 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/report/ReportTaskProcessor.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/report/ReportTaskProcessor.java @@ -24,6 +24,7 @@ import java.util.Set; import javax.annotation.CheckForNull; import org.sonar.ce.queue.CeTask; import org.sonar.ce.queue.CeTaskResult; +import org.sonar.ce.settings.SettingsLoader; import org.sonar.ce.settings.ThreadLocalSettings; import org.sonar.ce.taskprocessor.CeTaskProcessor; import org.sonar.core.platform.ComponentContainer; @@ -69,18 +70,19 @@ public class ReportTaskProcessor implements CeTaskProcessor { @Override public CeTaskResult process(CeTask task) { ComputeEngineContainer ceContainer = containerFactory.create(serverContainer, task, componentProviders); - ThreadLocalSettings ceSettings = null; - try { - ceSettings = ceContainer.getComponentByType(ThreadLocalSettings.class); - ceSettings.load(); + try { ceContainer.getComponentByType(ComputationStepExecutor.class).execute(); return ceContainer.getComponentByType(TaskResultHolder.class).getResult(); } finally { - if (ceSettings != null) { - ceSettings.remove(); - } + ensureThreadLocalIsClean(ceContainer); + ceContainer.cleanup(); } } + + /** safety call to clear ThreadLocal even if Pico container fails to call {@link SettingsLoader#stop()}) */ + private static void ensureThreadLocalIsClean(ComputeEngineContainer ceContainer) { + ceContainer.getComponentByType(ThreadLocalSettings.class).remove(); + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/properties/ProjectSettingsFactory.java b/server/sonar-server/src/main/java/org/sonar/server/properties/ProjectSettingsFactory.java index ce358473f0c..5bb613c80c1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/properties/ProjectSettingsFactory.java +++ b/server/sonar-server/src/main/java/org/sonar/server/properties/ProjectSettingsFactory.java @@ -20,8 +20,8 @@ package org.sonar.server.properties; import java.util.List; -import org.sonar.api.config.Settings; import org.sonar.api.ce.ComputeEngineSide; +import org.sonar.api.config.Settings; import org.sonar.api.server.ServerSide; import org.sonar.db.property.PropertiesDao; import org.sonar.db.property.PropertyDto; diff --git a/server/sonar-server/src/test/java/org/sonar/ce/settings/SettingsLoaderTest.java b/server/sonar-server/src/test/java/org/sonar/ce/settings/SettingsLoaderTest.java new file mode 100644 index 00000000000..00c4f1d166d --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/ce/settings/SettingsLoaderTest.java @@ -0,0 +1,47 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.settings; + +import org.junit.Test; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; + +public class SettingsLoaderTest { + private ThreadLocalSettings threadLocalSettings = mock(ThreadLocalSettings.class); + private SettingsLoader underTest = new SettingsLoader(threadLocalSettings); + + @Test + public void start_calls_ThreadLocalSettings_load() { + underTest.start(); + + verify(threadLocalSettings).load(); + verifyNoMoreInteractions(threadLocalSettings); + } + + @Test + public void stop_calls_ThreadLocalSettings_remove() { + underTest.stop(); + + verify(threadLocalSettings).remove(); + verifyNoMoreInteractions(threadLocalSettings); + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/container/ComputeEngineContainerImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/container/ComputeEngineContainerImplTest.java index 13abce63cc8..7f81e4593be 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/container/ComputeEngineContainerImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/container/ComputeEngineContainerImplTest.java @@ -20,6 +20,7 @@ package org.sonar.server.computation.container; import org.junit.Test; +import org.picocontainer.Startable; import org.sonar.core.platform.ComponentContainer; import org.sonar.core.platform.ContainerPopulator; @@ -28,6 +29,8 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; public class ComputeEngineContainerImplTest { + private ComponentContainer parent = new ComponentContainer(); + private ContainerPopulator populator = mock(ContainerPopulator.class); @Test(expected = NullPointerException.class) public void constructor_fails_fast_on_null_container() { @@ -41,8 +44,6 @@ public class ComputeEngineContainerImplTest { @Test public void calls_method_populateContainer_of_passed_in_populator() { - ComponentContainer parent = new ComponentContainer(); - ContainerPopulator populator = mock(ContainerPopulator.class); ComputeEngineContainerImpl ceContainer = new ComputeEngineContainerImpl(parent, populator); verify(populator).populateContainer(ceContainer); @@ -50,12 +51,48 @@ public class ComputeEngineContainerImplTest { @Test public void ce_container_is_not_child_of_specified_container() { - ComponentContainer parent = new ComponentContainer(); - ContainerPopulator populator = mock(ContainerPopulator.class); ComputeEngineContainerImpl ceContainer = new ComputeEngineContainerImpl(parent, populator); assertThat(parent.getChildren()).isEmpty(); verify(populator).populateContainer(ceContainer); } + @Test + public void components_are_started_lazily_unless_they_are_annotated_with_EagerStart() { + final DefaultStartable defaultStartable = new DefaultStartable(); + final EagerStartable eagerStartable = new EagerStartable(); + ComputeEngineContainerImpl ceContainer = new ComputeEngineContainerImpl(parent, new ContainerPopulator() { + @Override + public void populateContainer(ComputeEngineContainer container) { + container.add(defaultStartable); + container.add(eagerStartable); + } + }); + + assertThat(defaultStartable.startCalls).isEqualTo(0); + assertThat(defaultStartable.stopCalls).isEqualTo(0); + assertThat(eagerStartable.startCalls).isEqualTo(1); + assertThat(eagerStartable.stopCalls).isEqualTo(0); + } + + public static class DefaultStartable implements Startable { + protected int startCalls = 0; + protected int stopCalls = 0; + + @Override + public void start() { + startCalls++; + } + + @Override + public void stop() { + stopCalls++; + } + } + + @EagerStart + public static class EagerStartable extends DefaultStartable { + } + + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/ComputationStepsTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/ComputationStepsTest.java index ae71f067071..6bb76038f49 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/ComputationStepsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/ComputationStepsTest.java @@ -25,6 +25,7 @@ import com.google.common.collect.Lists; import java.util.Set; import org.junit.Test; import org.picocontainer.ComponentAdapter; +import org.sonar.ce.settings.ThreadLocalSettings; import org.sonar.core.platform.ComponentContainer; import org.sonar.server.computation.container.ComputeEngineContainerImpl; import org.sonar.server.computation.container.ReportComputeEngineContainerPopulator; @@ -51,7 +52,9 @@ public class ComputationStepsTest { @Test public void all_steps_from_package_step_are_present_in_container() { - ComputeEngineContainerImpl ceContainer = new ComputeEngineContainerImpl(new ComponentContainer(), new ReportComputeEngineContainerPopulator(mock(CeTask.class), null)); + ComponentContainer parent = new ComponentContainer(); + parent.add(mock(ThreadLocalSettings.class)); + ComputeEngineContainerImpl ceContainer = new ComputeEngineContainerImpl(parent, new ReportComputeEngineContainerPopulator(mock(CeTask.class), null)); Set stepsCanonicalNames = StepsExplorer.retrieveStepPackageStepsCanonicalNames(); -- 2.39.5