SONAR-11077 add statistics to CE PersistIssuesStep

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<DefaultIssue> 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<IssueChangeDto> 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<IssueChangeDto> 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<String, Object> 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<String, Object> getAll() {