]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-9973 TaskContainer now support errors during start of components
authorSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Thu, 2 Nov 2017 10:44:33 +0000 (11:44 +0100)
committerSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Fri, 3 Nov 2017 13:29:16 +0000 (14:29 +0100)
TaskContainer must now be started by a specific method call #bootup() (rather than it being started in constructor)
TaskContainer is now AutoCloseable (method cleanup() is therefor replaced by method close())
This change will break at compile time and runtime plugins relying on TaskContainer and TaskContainerImpl

server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainer.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/container/TaskContainerImpl.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/taskprocessor/ReportTaskProcessor.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/container/TaskContainerImplTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulatorTest.java

index e830163292d5280a31343d19a71f9b3a9265e871..53d1f8d6ca10b18a3b26a88c4badfc55b44f36b7 100644 (file)
@@ -25,16 +25,22 @@ import org.sonar.core.platform.ComponentContainer;
 import org.sonar.core.platform.ContainerPopulator;
 
 /**
- * The Compute Engine container. Created for a specific parent {@link ComponentContainer} and a specific {@link CeTask}.
+ * The Compute Engine task container. Created for a specific parent {@link ComponentContainer} and a specific {@link CeTask}.
  */
-public interface TaskContainer extends ContainerPopulator.Container {
+public interface TaskContainer extends ContainerPopulator.Container, AutoCloseable {
 
   ComponentContainer getParent();
 
+  /**
+   * Starts task container, starting any startable component in it.
+   */
+  void bootup();
+
   /**
    * Cleans up resources after process has been called and has returned.
    */
-  void cleanup();
+  @Override
+  void close();
 
   /**
    * Access to the underlying pico container.
index 27888a6f9be44dcc4d4a0d4129793f2a02b0837f..853187cd562f066a7239675d76423c5e238558de 100644 (file)
@@ -42,7 +42,6 @@ public class TaskContainerImpl extends ComponentContainer implements TaskContain
     super(createContainer(requireNonNull(parent)), parent.getComponentByType(PropertyDefinitions.class));
 
     populateContainer(requireNonNull(populator));
-    startComponents();
   }
 
   private void populateContainer(ContainerPopulator<TaskContainer> populator) {
@@ -74,16 +73,21 @@ public class TaskContainerImpl extends ComponentContainer implements TaskContain
   }
 
   @Override
-  public void cleanup() {
-    try {
-      stopComponents();
-    } catch (Throwable t) {
-      Loggers.get(TaskContainerImpl.class).error("Cleanup of container failed", t);
-    }
+  public void bootup() {
+    startComponents();
   }
 
   @Override
   public String toString() {
     return "TaskContainerImpl";
   }
+
+  @Override
+  public void close() {
+    try {
+      stopComponents();
+    } catch (Throwable t) {
+      Loggers.get(TaskContainerImpl.class).error("Cleanup of container failed", t);
+    }
+  }
 }
index b31c53afdb9753ddcd1d0fc6e15ef653a967048c..200b9e4c28f3f888ed8332fd943c0430dba9ea0b 100644 (file)
@@ -24,7 +24,6 @@ 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.taskprocessor.CeTaskProcessor;
 import org.sonar.core.platform.ComponentContainer;
 import org.sonar.db.ce.CeTaskTypes;
@@ -33,7 +32,6 @@ import org.sonar.server.computation.task.container.TaskContainer;
 import org.sonar.server.computation.task.projectanalysis.container.ContainerFactory;
 import org.sonar.server.computation.task.step.ComputationStepExecutor;
 import org.sonar.server.computation.taskprocessor.TaskResultHolder;
-import org.sonar.server.setting.ThreadLocalSettings;
 
 public class ReportTaskProcessor implements CeTaskProcessor {
 
@@ -69,20 +67,11 @@ public class ReportTaskProcessor implements CeTaskProcessor {
 
   @Override
   public CeTaskResult process(CeTask task) {
-    TaskContainer ceContainer = containerFactory.create(serverContainer, task, componentProviders);
+    try (TaskContainer ceContainer = containerFactory.create(serverContainer, task, componentProviders)) {
+      ceContainer.bootup();
 
-    try {
       ceContainer.getComponentByType(ComputationStepExecutor.class).execute();
       return ceContainer.getComponentByType(TaskResultHolder.class).getResult();
-    } finally {
-      ensureThreadLocalIsClean(ceContainer);
-
-      ceContainer.cleanup();
     }
   }
-
-  /** safety call to clear ThreadLocal even if Pico container fails to call {@link SettingsLoader#stop()}) */
-  private static void ensureThreadLocalIsClean(TaskContainer ceContainer) {
-    ceContainer.getComponentByType(ThreadLocalSettings.class).unload();
-  }
 }
index aa0d746b6492c394d7f3cb5601b9c7ff64a7b811..bd0c28462d460cd93f821fb1dfa92488a74266fc 100644 (file)
@@ -58,16 +58,14 @@ public class TaskContainerImplTest {
   }
 
   @Test
-  public void components_are_started_lazily_unless_they_are_annotated_with_EagerStart() {
+  public void bootup_starts_components_lazily_unless_they_are_annotated_with_EagerStart() {
     final DefaultStartable defaultStartable = new DefaultStartable();
     final EagerStartable eagerStartable = new EagerStartable();
-    TaskContainerImpl ceContainer = new TaskContainerImpl(parent, new ContainerPopulator<TaskContainer>() {
-      @Override
-      public void populateContainer(TaskContainer container) {
-        container.add(defaultStartable);
-        container.add(eagerStartable);
-      }
+    TaskContainerImpl ceContainer = new TaskContainerImpl(parent, container -> {
+      container.add(defaultStartable);
+      container.add(eagerStartable);
     });
+    ceContainer.bootup();
 
     assertThat(defaultStartable.startCalls).isEqualTo(0);
     assertThat(defaultStartable.stopCalls).isEqualTo(0);
@@ -75,6 +73,24 @@ public class TaskContainerImplTest {
     assertThat(eagerStartable.stopCalls).isEqualTo(0);
   }
 
+  @Test
+  public void close_stops_started_components() {
+    final DefaultStartable defaultStartable = new DefaultStartable();
+    final EagerStartable eagerStartable = new EagerStartable();
+    TaskContainerImpl ceContainer = new TaskContainerImpl(parent, container -> {
+      container.add(defaultStartable);
+      container.add(eagerStartable);
+    });
+    ceContainer.bootup();
+
+    ceContainer.close();
+
+    assertThat(defaultStartable.startCalls).isEqualTo(0);
+    assertThat(defaultStartable.stopCalls).isEqualTo(0);
+    assertThat(eagerStartable.startCalls).isEqualTo(1);
+    assertThat(eagerStartable.stopCalls).isEqualTo(1);
+  }
+
   public static class DefaultStartable implements Startable {
     protected int startCalls = 0;
     protected int stopCalls = 0;
index 4fbe85c0476cbf8b8c9ae936e8d4bb1f146ec15d..5231a360e3e422feab06353824afce73eab0fbe4 100644 (file)
@@ -128,13 +128,18 @@ public class ProjectAnalysisTaskContainerPopulatorTest {
 
     private List<Object> added = new ArrayList<>();
 
+    @Override
+    public void bootup() {
+      // no effect
+    }
+
     @Override
     public ComponentContainer getParent() {
       throw new UnsupportedOperationException("getParent is not implemented");
     }
 
     @Override
-    public void cleanup() {
+    public void close() {
       throw new UnsupportedOperationException("cleanup is not implemented");
     }