From dc0783e439610ad1fc9de64f7bfe70e1ab87a66a Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 12 Aug 2016 11:21:53 +0200 Subject: [PATCH] SONAR-7833 make all public methods of CeLogging non static --- .../main/java/org/sonar/ce/log/CeLogging.java | 4 +- .../step/ExecuteVisitorsStep.java | 9 ++- .../task/step/ComputationStepExecutor.java | 13 ++-- .../taskprocessor/CeWorkerCallableImpl.java | 11 ++- .../java/org/sonar/ce/log/CeLoggingTest.java | 6 +- .../step/ExecuteVisitorsStepTest.java | 75 +++++++++++++++++-- .../step/ComputationStepExecutorTest.java | 26 +++---- 7 files changed, 106 insertions(+), 38 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/ce/log/CeLogging.java b/server/sonar-server/src/main/java/org/sonar/ce/log/CeLogging.java index 2f8e4424d6a..7cdf1c9ff04 100644 --- a/server/sonar-server/src/main/java/org/sonar/ce/log/CeLogging.java +++ b/server/sonar-server/src/main/java/org/sonar/ce/log/CeLogging.java @@ -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 logCeActivity(Logger logger, Supplier logProducer) { + public T logCeActivity(Logger logger, Supplier logProducer) { MDC.put(MDC_CE_ACTIVITY_FLAG, computeCeActivityFlag(logger)); T res = logProducer.get(); MDC.remove(MDC_CE_ACTIVITY_FLAG); diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStep.java index d761a418013..2ad1f0a8935 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStep.java @@ -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 visitors; + private final CeLogging ceLogging; - public ExecuteVisitorsStep(TreeRootHolder treeRootHolder, List visitors) { + public ExecuteVisitorsStep(TreeRootHolder treeRootHolder, List 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 visitors, VisitorsCrawler visitorsCrawler) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/step/ComputationStepExecutor.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/step/ComputationStepExecutor.java index c066be83496..9bbbe34c40b 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/step/ComputationStepExecutor.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/step/ComputationStepExecutor.java @@ -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())); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImpl.java index 436128423ce..617ef9d266f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImpl.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/taskprocessor/CeWorkerCallableImpl.java @@ -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")); } } diff --git a/server/sonar-server/src/test/java/org/sonar/ce/log/CeLoggingTest.java b/server/sonar-server/src/test/java/org/sonar/ce/log/CeLoggingTest.java index 216397f2168..c72d3b31f6f 100644 --- a/server/sonar-server/src/test/java/org/sonar/ce/log/CeLoggingTest.java +++ b/server/sonar-server/src/test/java/org/sonar/ce/log/CeLoggingTest.java @@ -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); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStepTest.java index d357135e49e..d28939958f3 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/step/ExecuteVisitorsStepTest.java @@ -19,10 +19,14 @@ */ 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.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.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 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() { diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java index a1858b51da7..1435f1ec54e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/step/ComputationStepExecutorTest.java @@ -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 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); -- 2.39.5