From 92fb3dc68dff7aa937f005bbda644a2dc27f62d6 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 27 Jul 2018 18:02:43 +0200 Subject: [PATCH] SONAR-11077 add statistics to CE PersistIssuesStep --- .../step/PersistIssuesStep.java | 24 ++++++++++- .../step/PersistIssuesStepTest.java | 40 +++++++++++++++---- .../sonar/ce/task/step/ComputationStep.java | 3 +- .../ce/task/step/ComputationStepExecutor.java | 3 +- .../task/step/TestComputationStepContext.java | 7 ++-- 5 files changed, 62 insertions(+), 15 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStep.java index 2c49f92d78e..fd830f733f9 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStep.java @@ -54,6 +54,7 @@ public class PersistIssuesStep implements ComputationStep { @Override public void execute(ComputationStep.Context context) { + IssueStatistics statistics = new IssueStatistics(); try (DbSession dbSession = dbClient.openSession(true); CloseableIterator issues = issueCache.traverse()) { @@ -61,26 +62,32 @@ public class PersistIssuesStep implements ComputationStep { IssueChangeMapper changeMapper = dbSession.getMapper(IssueChangeMapper.class); while (issues.hasNext()) { DefaultIssue issue = issues.next(); - boolean saved = persistIssueIfRequired(mapper, issue); + boolean saved = persistIssueIfRequired(mapper, issue, statistics); if (saved) { issueStorage.insertChanges(changeMapper, issue); } } dbSession.flushStatements(); dbSession.commit(); + } finally { + statistics.dumpTo(context); } } - private boolean persistIssueIfRequired(IssueMapper mapper, DefaultIssue issue) { + private boolean persistIssueIfRequired(IssueMapper mapper, DefaultIssue issue, IssueStatistics issueStatistics) { if (issue.isNew() || issue.isCopied()) { persistNewIssue(mapper, issue); + issueStatistics.inserts++; return true; } if (issue.isChanged()) { persistChangedIssue(mapper, issue); + issueStatistics.updates++; return true; } + + issueStatistics.untouched++; return false; } @@ -104,4 +111,17 @@ public class PersistIssuesStep implements ComputationStep { public String getDescription() { return "Persist issues"; } + + private static class IssueStatistics { + private int inserts = 0; + private int updates = 0; + private int untouched = 0; + + private void dumpTo(ComputationStep.Context context) { + context.getStatistics() + .add("inserts", String.valueOf(inserts)) + .add("updates", String.valueOf(updates)) + .add("untouched", String.valueOf(untouched)); + } + } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java index d2f758013f6..a5a7552b0ac 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/PersistIssuesStepTest.java @@ -57,6 +57,7 @@ import org.sonar.server.rule.ExternalRuleCreator; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.data.MapEntry.entry; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; @@ -67,7 +68,7 @@ import static org.sonar.db.component.ComponentTesting.newFileDto; public class PersistIssuesStepTest extends BaseStepTest { - public static final long NOW = 1_400_000_000_000L; + private static final long NOW = 1_400_000_000_000L; @Rule public TemporaryFolder temp = new TemporaryFolder(); @@ -98,7 +99,8 @@ public class PersistIssuesStepTest extends BaseStepTest { when(system2.now()).thenReturn(NOW); reportReader.setMetadata(ScannerReport.Metadata.getDefaultInstance()); - underTest = new PersistIssuesStep(dbClient, system2, new UpdateConflictResolver(), new RuleRepositoryImpl(externalRuleCreator, dbClient, analysisMetadataHolder), issueCache, new IssueStorage()); + underTest = new PersistIssuesStep(dbClient, system2, new UpdateConflictResolver(), new RuleRepositoryImpl(externalRuleCreator, dbClient, analysisMetadataHolder), issueCache, + new IssueStorage()); } @After @@ -143,7 +145,8 @@ public class PersistIssuesStepTest extends BaseStepTest { .setCreationDate(new Date(NOW)))) .close(); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); IssueDto result = dbClient.issueDao().selectOrFailByKey(session, "ISSUE"); assertThat(result.getKey()).isEqualTo("ISSUE"); @@ -156,6 +159,8 @@ public class PersistIssuesStepTest extends BaseStepTest { List changes = dbClient.issueChangeDao().selectByIssueKeys(session, Arrays.asList("ISSUE")); assertThat(changes).extracting(IssueChangeDto::getChangeType).containsExactly(IssueChangeDto.TYPE_COMMENT, IssueChangeDto.TYPE_FIELD_CHANGE); + assertThat(context.getStatistics().getAll()).containsOnly( + entry("inserts", "1"), entry("updates", "0"), entry("untouched", "0")); } @Test @@ -193,7 +198,9 @@ public class PersistIssuesStepTest extends BaseStepTest { .setDiff("technicalDebt", null, 1L) .setCreationDate(new Date(NOW)))) .close(); - underTest.execute(new TestComputationStepContext()); + + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); IssueDto result = dbClient.issueDao().selectOrFailByKey(session, "ISSUE"); assertThat(result.getKey()).isEqualTo("ISSUE"); @@ -206,6 +213,8 @@ public class PersistIssuesStepTest extends BaseStepTest { List changes = dbClient.issueChangeDao().selectByIssueKeys(session, Arrays.asList("ISSUE")); assertThat(changes).extracting(IssueChangeDto::getChangeType).containsExactly(IssueChangeDto.TYPE_COMMENT, IssueChangeDto.TYPE_FIELD_CHANGE); + assertThat(context.getStatistics().getAll()).containsOnly( + entry("inserts", "1"), entry("updates", "0"), entry("untouched", "0")); } @Test @@ -230,7 +239,8 @@ public class PersistIssuesStepTest extends BaseStepTest { .setNew(true) .setType(RuleType.BUG)).close(); - underTest.execute(new TestComputationStepContext()); + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); IssueDto result = dbClient.issueDao().selectOrFailByKey(session, "ISSUE"); assertThat(result.getKey()).isEqualTo("ISSUE"); @@ -240,6 +250,8 @@ public class PersistIssuesStepTest extends BaseStepTest { assertThat(result.getSeverity()).isEqualTo(BLOCKER); assertThat(result.getStatus()).isEqualTo(STATUS_OPEN); assertThat(result.getType()).isEqualTo(RuleType.BUG.getDbConstant()); + assertThat(context.getStatistics().getAll()).containsOnly( + entry("inserts", "1"), entry("updates", "0"), entry("untouched", "0")); } @Test @@ -262,11 +274,15 @@ public class PersistIssuesStepTest extends BaseStepTest { .setNew(false) .setChanged(true)) .close(); - underTest.execute(new TestComputationStepContext()); + + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); IssueDto issueReloaded = db.getDbClient().issueDao().selectByKey(db.getSession(), issue.getKey()).get(); assertThat(issueReloaded.getStatus()).isEqualTo(STATUS_CLOSED); assertThat(issueReloaded.getResolution()).isEqualTo(RESOLUTION_FIXED); + assertThat(context.getStatistics().getAll()).containsOnly( + entry("inserts", "0"), entry("updates", "1"), entry("untouched", "0")); } @Test @@ -296,13 +312,17 @@ public class PersistIssuesStepTest extends BaseStepTest { .setCreatedAt(new Date(NOW)) .setNew(true))) .close(); - underTest.execute(new TestComputationStepContext()); + + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); IssueChangeDto issueChangeDto = db.getDbClient().issueChangeDao().selectByIssueKeys(db.getSession(), singletonList(issue.getKey())).get(0); assertThat(issueChangeDto) .extracting(IssueChangeDto::getChangeType, IssueChangeDto::getUserUuid, IssueChangeDto::getChangeData, IssueChangeDto::getIssueKey, IssueChangeDto::getIssueChangeCreationDate) .containsOnly(IssueChangeDto.TYPE_COMMENT, "john_uuid", "Some text", issue.getKey(), NOW); + assertThat(context.getStatistics().getAll()).containsOnly( + entry("inserts", "0"), entry("updates", "1"), entry("untouched", "0")); } @Test @@ -330,13 +350,17 @@ public class PersistIssuesStepTest extends BaseStepTest { .setDiff("technicalDebt", null, 1L) .setCreationDate(new Date(NOW)))) .close(); - underTest.execute(new TestComputationStepContext()); + + TestComputationStepContext context = new TestComputationStepContext(); + underTest.execute(context); IssueChangeDto issueChangeDto = db.getDbClient().issueChangeDao().selectByIssueKeys(db.getSession(), singletonList(issue.getKey())).get(0); assertThat(issueChangeDto) .extracting(IssueChangeDto::getChangeType, IssueChangeDto::getUserUuid, IssueChangeDto::getChangeData, IssueChangeDto::getIssueKey, IssueChangeDto::getIssueChangeCreationDate) .containsOnly(IssueChangeDto.TYPE_FIELD_CHANGE, "john_uuid", "technicalDebt=1", issue.getKey(), NOW); + assertThat(context.getStatistics().getAll()).containsOnly( + entry("inserts", "0"), entry("updates", "1"), entry("untouched", "0")); } } diff --git a/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStep.java b/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStep.java index 1d9f67e2e9d..28eb3cfc05e 100644 --- a/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStep.java +++ b/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStep.java @@ -45,11 +45,12 @@ public interface ComputationStep { */ interface Statistics { /** + * @return this * @throws NullPointerException if key or value is null * @throws IllegalArgumentException if key has already been set * @throws IllegalArgumentException if key is "time", to avoid conflict with the profiler field with same name */ - void add(String key, Object value); + Statistics add(String key, Object value); } interface Context { diff --git a/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStepExecutor.java b/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStepExecutor.java index d706b8914fa..381a824cc32 100644 --- a/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStepExecutor.java +++ b/server/sonar-ce-task/src/main/java/org/sonar/ce/task/step/ComputationStepExecutor.java @@ -94,12 +94,13 @@ public final class ComputationStepExecutor { } @Override - public void add(String key, Object value) { + public ComputationStep.Statistics add(String key, Object value) { requireNonNull(key, "Statistic has null key"); requireNonNull(value, () -> String.format("Statistic with key [%s] has null value", key)); checkArgument(!key.equalsIgnoreCase("time"), "Statistic with key [time] is not accepted"); checkArgument(!profiler.hasContext(key), "Statistic with key [%s] is already present", key); profiler.addContext(key, value); + return this; } } diff --git a/server/sonar-ce-task/src/test/java/org/sonar/ce/task/step/TestComputationStepContext.java b/server/sonar-ce-task/src/test/java/org/sonar/ce/task/step/TestComputationStepContext.java index dc560ed8aaa..33edcdc16fd 100644 --- a/server/sonar-ce-task/src/test/java/org/sonar/ce/task/step/TestComputationStepContext.java +++ b/server/sonar-ce-task/src/test/java/org/sonar/ce/task/step/TestComputationStepContext.java @@ -32,10 +32,10 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class TestComputationStepContext implements ComputationStep.Context { - private final ComputationStep.Statistics statistics = new TestStatistics(); + private final TestStatistics statistics = new TestStatistics(); @Override - public ComputationStep.Statistics getStatistics() { + public TestStatistics getStatistics() { return statistics; } @@ -43,12 +43,13 @@ public class TestComputationStepContext implements ComputationStep.Context { private final Map map = new HashMap<>(); @Override - public void add(String key, Object value) { + public ComputationStep.Statistics add(String key, Object value) { requireNonNull(key, "Statistic has null key"); requireNonNull(value, () -> String.format("Statistic with key [%s] has null value", key)); checkArgument(!key.equalsIgnoreCase("time"), "Statistic with key [time] is not accepted"); checkArgument(!map.containsKey(key), "Statistic with key [%s] is already present", key); map.put(key, value); + return this; } public Map getAll() { -- 2.39.5