]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-7833 make all public methods of CeLogging non static
authorSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Fri, 12 Aug 2016 09:21:53 +0000 (11:21 +0200)
committerSébastien Lesaint <sebastien.lesaint@sonarsource.com>
Tue, 16 Aug 2016 06:20:15 +0000 (08:20 +0200)
server/sonar-server/src/main/java/org/sonar/ce/log/CeLogging.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStep.java
server/sonar-server/src/main/java/org/sonar/server/computation/task/step/ComputationStepExecutor.java
server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImpl.java
server/sonar-server/src/test/java/org/sonar/ce/log/CeLoggingTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStepTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java

index 2f8e4424d6ac700f18208c38865641e184fc98de..7cdf1c9ff045ee8ecf842778f9173cd8011f73cf 100644 (file)
@@ -55,13 +55,13 @@ public class CeLogging {
     // TODO clear task UUID from MDF (cf. SONAR-7960)
   }
 
-  public static void logCeActivity(Logger logger, Runnable logProducer) {
+  public void logCeActivity(Logger logger, Runnable logProducer) {
     MDC.put(MDC_CE_ACTIVITY_FLAG, computeCeActivityFlag(logger));
     logProducer.run();
     MDC.remove(MDC_CE_ACTIVITY_FLAG);
   }
 
-  public static <T> T logCeActivity(Logger logger, Supplier<T> logProducer) {
+  public <T> T logCeActivity(Logger logger, Supplier<T> logProducer) {
     MDC.put(MDC_CE_ACTIVITY_FLAG, computeCeActivityFlag(logger));
     T res = logProducer.get();
     MDC.remove(MDC_CE_ACTIVITY_FLAG);
index d761a41801376dcc30a6796b60146f9dfe331463..2ad1f0a8935e4bcb6d27fc65b4ad5a2ff6118074 100644 (file)
@@ -23,23 +23,24 @@ import java.util.List;
 import java.util.Map;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
+import org.sonar.ce.log.CeLogging;
 import org.sonar.server.computation.task.projectanalysis.component.ComponentVisitor;
 import org.sonar.server.computation.task.projectanalysis.component.TreeRootHolder;
 import org.sonar.server.computation.task.projectanalysis.component.VisitorsCrawler;
 import org.sonar.server.computation.task.step.ComputationStep;
 
-import static org.sonar.ce.log.CeLogging.logCeActivity;
-
 public class ExecuteVisitorsStep implements ComputationStep {
 
   private static final Logger LOGGER = Loggers.get(ExecuteVisitorsStep.class);
 
   private final TreeRootHolder treeRootHolder;
   private final List<ComponentVisitor> visitors;
+  private final CeLogging ceLogging;
 
-  public ExecuteVisitorsStep(TreeRootHolder treeRootHolder, List<ComponentVisitor> visitors) {
+  public ExecuteVisitorsStep(TreeRootHolder treeRootHolder, List<ComponentVisitor> visitors, CeLogging ceLogging) {
     this.treeRootHolder = treeRootHolder;
     this.visitors = visitors;
+    this.ceLogging = ceLogging;
   }
 
   @Override
@@ -51,7 +52,7 @@ public class ExecuteVisitorsStep implements ComputationStep {
   public void execute() {
     VisitorsCrawler visitorsCrawler = new VisitorsCrawler(visitors);
     visitorsCrawler.visit(treeRootHolder.getRoot());
-    logCeActivity(LOGGER, () -> logVisitorExecutionDurations(visitors, visitorsCrawler));
+    ceLogging.logCeActivity(LOGGER, () -> logVisitorExecutionDurations(visitors, visitorsCrawler));
   }
 
   private static void logVisitorExecutionDurations(List<ComponentVisitor> visitors, VisitorsCrawler visitorsCrawler) {
index c066be8349617723332419b9129d64c7cb75edfd..9bbbe34c40be88c5da8187c06f317bab0a4fdad1 100644 (file)
@@ -23,14 +23,14 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
+import org.sonar.ce.log.CeLogging;
 import org.sonar.core.util.logs.Profiler;
 
-import static org.sonar.ce.log.CeLogging.logCeActivity;
-
 public final class ComputationStepExecutor {
   private static final Logger LOGGER = Loggers.get(ComputationStepExecutor.class);
 
   private final ComputationSteps steps;
+  private final CeLogging ceLogging;
   @CheckForNull
   private final Listener listener;
 
@@ -38,12 +38,13 @@ public final class ComputationStepExecutor {
    * Used when no {@link ComputationStepExecutor.Listener} is available in pico
    * container.
    */
-  public ComputationStepExecutor(ComputationSteps steps) {
-    this(steps, null);
+  public ComputationStepExecutor(ComputationSteps steps, CeLogging ceLogging) {
+    this(steps, ceLogging, null);
   }
 
-  public ComputationStepExecutor(ComputationSteps steps, @Nullable Listener listener) {
+  public ComputationStepExecutor(ComputationSteps steps, CeLogging ceLogging, @Nullable Listener listener) {
     this.steps = steps;
+    this.ceLogging = ceLogging;
     this.listener = listener;
   }
 
@@ -64,7 +65,7 @@ public final class ComputationStepExecutor {
     for (ComputationStep step : steps.instances()) {
       stepProfiler.start();
       step.execute();
-      logCeActivity(LOGGER, () -> stepProfiler.stopInfo(step.getDescription()));
+      ceLogging.logCeActivity(LOGGER, () -> stepProfiler.stopInfo(step.getDescription()));
     }
   }
 
index 436128423cec5464f9fcc5c0353ad021169bc49b..617ef9d266ff17fabcb68fa4957bd947ec5b8606 100644 (file)
@@ -31,7 +31,6 @@ import org.sonar.db.ce.CeActivityDto;
 import org.sonar.server.computation.queue.InternalCeQueue;
 
 import static java.lang.String.format;
-import static org.sonar.ce.log.CeLogging.logCeActivity;
 
 public class CeWorkerCallableImpl implements CeWorkerCallable {
 
@@ -92,18 +91,18 @@ public class CeWorkerCallableImpl implements CeWorkerCallable {
     }
   }
 
-  private static Profiler startActivityProfiler(CeTask task) {
+  private Profiler startActivityProfiler(CeTask task) {
     Profiler profiler = Profiler.create(LOG);
     addContext(profiler, task);
-    return logCeActivity(LOG, () -> profiler.startInfo("Execute task"));
+    return ceLogging.logCeActivity(LOG, () -> profiler.startInfo("Execute task"));
   }
 
-  private static void stopActivityProfiler(Profiler profiler, CeTask task, CeActivityDto.Status status) {
+  private void stopActivityProfiler(Profiler profiler, CeTask task, CeActivityDto.Status status) {
     addContext(profiler, task);
     if (status == CeActivityDto.Status.FAILED) {
-      logCeActivity(LOG, () -> profiler.stopError("Executed task"));
+      ceLogging.logCeActivity(LOG, () -> profiler.stopError("Executed task"));
     } else {
-      logCeActivity(LOG, () -> profiler.stopInfo("Executed task"));
+      ceLogging.logCeActivity(LOG, () -> profiler.stopInfo("Executed task"));
     }
   }
 
index 216397f21680fdb0e9c354e2181e68739f476750..c72d3b31f6f80649301a0cd0e6bbcca2d1b64da3 100644 (file)
@@ -63,6 +63,8 @@ public class CeLoggingTest {
   private LogbackHelper helper = new LogbackHelper();
   private File logDir;
 
+  private CeLogging underTest = new CeLogging();
+
   @Before
   public void setUp() throws Exception {
     this.logDir = temp.newFolder();
@@ -136,7 +138,7 @@ public class CeLoggingTest {
 
   private void callLogCeActivityOfRunnableAndVerify(String expectedMdcValue, Level logLevel) {
     AtomicBoolean called = new AtomicBoolean(false);
-    CeLogging.logCeActivity(
+    underTest.logCeActivity(
       createLogger(logLevel),
       () -> {
         assertThat(MDC.get(MDC_CE_ACTIVITY_FLAG)).isEqualTo(expectedMdcValue);
@@ -180,7 +182,7 @@ public class CeLoggingTest {
 
   private void callLogCeActivityOfSupplierAndVerify(Level logLevel, String expectedFlag) {
     AtomicBoolean called = new AtomicBoolean(false);
-    CeLogging.logCeActivity(
+    underTest.logCeActivity(
       createLogger(logLevel),
       () -> {
         assertThat(MDC.get(MDC_CE_ACTIVITY_FLAG)).isEqualTo(expectedFlag);
index d357135e49ed260f0ca26d99b52b2d0927d307f6..d28939958f3f68f58c223f7ca76cbf30cd284bcd 100644 (file)
  */
 package org.sonar.server.computation.task.projectanalysis.step;
 
-import java.util.Arrays;
+import java.util.List;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.sonar.api.utils.log.LogTester;
+import org.sonar.api.utils.log.Logger;
+import org.sonar.api.utils.log.LoggerLevel;
+import org.sonar.ce.log.CeLogging;
 import org.sonar.server.computation.task.projectanalysis.component.Component;
 import org.sonar.server.computation.task.projectanalysis.component.ComponentVisitor;
 import org.sonar.server.computation.task.projectanalysis.component.CrawlerDepthLimit;
@@ -34,7 +38,12 @@ import org.sonar.server.computation.task.projectanalysis.metric.Metric;
 import org.sonar.server.computation.task.projectanalysis.metric.MetricImpl;
 import org.sonar.server.computation.task.projectanalysis.metric.MetricRepositoryRule;
 
+import static java.util.Arrays.asList;
+import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Matchers.any;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
 import static org.sonar.api.measures.CoreMetrics.NCLOC;
 import static org.sonar.api.measures.CoreMetrics.NCLOC_KEY;
 import static org.sonar.server.computation.task.projectanalysis.component.Component.Type.DIRECTORY;
@@ -56,14 +65,16 @@ public class ExecuteVisitorsStepTest {
 
   @Rule
   public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule();
-
   @Rule
   public MetricRepositoryRule metricRepository = new MetricRepositoryRule()
     .add(1, NCLOC)
     .add(new MetricImpl(2, TEST_METRIC_KEY, "name", Metric.MetricType.INT));
-
   @Rule
   public MeasureRepositoryRule measureRepository = MeasureRepositoryRule.create(treeRootHolder, metricRepository);
+  @Rule
+  public LogTester logTester = new LogTester();
+
+  private CeLogging ceLogging = spy(new CeLogging());
 
   @Before
   public void setUp() throws Exception {
@@ -83,7 +94,7 @@ public class ExecuteVisitorsStepTest {
 
   @Test
   public void execute_with_type_aware_visitor() throws Exception {
-    ExecuteVisitorsStep underStep = new ExecuteVisitorsStep(treeRootHolder, Arrays.<ComponentVisitor>asList(new TestTypeAwareVisitor()));
+    ExecuteVisitorsStep underStep = new ExecuteVisitorsStep(treeRootHolder, singletonList(new TestTypeAwareVisitor()), ceLogging);
 
     measureRepository.addRawMeasure(FILE_1_REF, NCLOC_KEY, newMeasureBuilder().create(1));
     measureRepository.addRawMeasure(FILE_2_REF, NCLOC_KEY, newMeasureBuilder().create(2));
@@ -102,7 +113,7 @@ public class ExecuteVisitorsStepTest {
 
   @Test
   public void execute_with_path_aware_visitor() throws Exception {
-    ExecuteVisitorsStep underStep = new ExecuteVisitorsStep(treeRootHolder, Arrays.<ComponentVisitor>asList(new TestPathAwareVisitor()));
+    ExecuteVisitorsStep underStep = new ExecuteVisitorsStep(treeRootHolder, singletonList(new TestPathAwareVisitor()), ceLogging);
 
     measureRepository.addRawMeasure(FILE_1_REF, NCLOC_KEY, newMeasureBuilder().create(1));
     measureRepository.addRawMeasure(FILE_2_REF, NCLOC_KEY, newMeasureBuilder().create(1));
@@ -116,6 +127,60 @@ public class ExecuteVisitorsStepTest {
     assertThat(measureRepository.getAddedRawMeasure(ROOT_REF, TEST_METRIC_KEY).get().getIntValue()).isEqualTo(2);
   }
 
+  @Test
+  public void execute_logs_at_info_level_all_execution_duration_of_all_visitors() {
+    ExecuteVisitorsStep underStep = new ExecuteVisitorsStep(
+      treeRootHolder,
+      asList(new VisitorA(), new VisitorB(), new VisitorC()),
+      ceLogging);
+
+    underStep.execute();
+
+    verify(ceLogging).logCeActivity(any(Logger.class), any(Runnable.class));
+    List<String> logs = logTester.logs(LoggerLevel.INFO);
+    assertThat(logs).hasSize(4);
+    assertThat(logs.get(0)).isEqualTo("  Execution time for each component visitor:");
+    assertThat(logs.get(1)).startsWith("  - VisitorA | time=");
+    assertThat(logs.get(2)).startsWith("  - VisitorB | time=");
+    assertThat(logs.get(3)).startsWith("  - VisitorC | time=");
+  }
+
+  private static class VisitorA implements ComponentVisitor {
+    @Override
+    public Order getOrder() {
+      return Order.PRE_ORDER;
+    }
+
+    @Override
+    public CrawlerDepthLimit getMaxDepth() {
+      return CrawlerDepthLimit.PROJECT;
+    }
+  }
+
+  private static class VisitorB implements ComponentVisitor {
+    @Override
+    public Order getOrder() {
+      return Order.PRE_ORDER;
+    }
+
+    @Override
+    public CrawlerDepthLimit getMaxDepth() {
+      return CrawlerDepthLimit.PROJECT;
+    }
+  }
+
+  private static class VisitorC implements ComponentVisitor {
+    @Override
+    public Order getOrder() {
+      return Order.PRE_ORDER;
+    }
+
+    @Override
+    public CrawlerDepthLimit getMaxDepth() {
+      return CrawlerDepthLimit.PROJECT;
+    }
+  }
+
   private class TestTypeAwareVisitor extends TypeAwareVisitorAdapter {
 
     public TestTypeAwareVisitor() {
index a1858b51da785e0362806db5854b8f98a65f74e5..1435f1ec54e2d0ed0004d06ae3918aa711e9e11e 100644 (file)
@@ -27,9 +27,7 @@ import org.junit.rules.ExpectedException;
 import org.mockito.InOrder;
 import org.sonar.api.utils.log.LogTester;
 import org.sonar.api.utils.log.LoggerLevel;
-import org.sonar.server.computation.task.step.ComputationStep;
-import org.sonar.server.computation.task.step.ComputationStepExecutor;
-import org.sonar.server.computation.task.step.ComputationSteps;
+import org.sonar.ce.log.CeLogging;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.fail;
@@ -51,9 +49,11 @@ public class ComputationStepExecutorTest {
   private final ComputationStep computationStep2 = mockComputationStep("step2");
   private final ComputationStep computationStep3 = mockComputationStep("step3");
 
+  private CeLogging ceLogging = new CeLogging();
+
   @Test
   public void execute_call_execute_on_each_ComputationStep_in_order_returned_by_instances_method() {
-    new ComputationStepExecutor(mockComputationSteps(computationStep1, computationStep2, computationStep3))
+    new ComputationStepExecutor(mockComputationSteps(computationStep1, computationStep2, computationStep3), ceLogging)
       .execute();
 
     InOrder inOrder = inOrder(computationStep1, computationStep2, computationStep3);
@@ -75,7 +75,7 @@ public class ComputationStepExecutorTest {
       .when(computationStep)
       .execute();
 
-    ComputationStepExecutor computationStepExecutor = new ComputationStepExecutor(mockComputationSteps(computationStep));
+    ComputationStepExecutor computationStepExecutor = new ComputationStepExecutor(mockComputationSteps(computationStep), ceLogging);
 
     expectedException.expect(RuntimeException.class);
     expectedException.expectMessage(message);
@@ -85,8 +85,8 @@ public class ComputationStepExecutorTest {
 
   @Test
   public void execute_logs_end_timing_for_each_ComputationStep_called() {
-    new ComputationStepExecutor(mockComputationSteps(computationStep1, computationStep2))
-        .execute();
+    new ComputationStepExecutor(mockComputationSteps(computationStep1, computationStep2), ceLogging)
+      .execute();
 
     List<String> infoLogs = logTester.logs(LoggerLevel.INFO);
     assertThat(infoLogs).hasSize(2);
@@ -96,8 +96,8 @@ public class ComputationStepExecutorTest {
 
   @Test
   public void execute_calls_listener_finished_method_with_all_step_runs() {
-    new ComputationStepExecutor(mockComputationSteps(computationStep1, computationStep2), listener)
-        .execute();
+    new ComputationStepExecutor(mockComputationSteps(computationStep1, computationStep2), ceLogging, listener)
+      .execute();
 
     verify(listener).finished(true);
     verifyNoMoreInteractions(listener);
@@ -107,12 +107,12 @@ public class ComputationStepExecutorTest {
   public void execute_calls_listener_finished_method_even_if_a_step_throws_an_exception() {
     RuntimeException toBeThrown = new RuntimeException("simulating failing execute Step method");
     doThrow(toBeThrown)
-        .when(computationStep1)
-        .execute();
+      .when(computationStep1)
+      .execute();
 
     try {
-      new ComputationStepExecutor(mockComputationSteps(computationStep1, computationStep2), listener)
-          .execute();
+      new ComputationStepExecutor(mockComputationSteps(computationStep1, computationStep2), ceLogging, listener)
+        .execute();
       fail("exception toBeThrown should have been raised");
     } catch (RuntimeException e) {
       assertThat(e).isSameAs(toBeThrown);