Fix stability of issue tracking

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/DebtAggregator.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/DebtAggregator.java
index c8c410a65035d35cc179880edaa85ceb525e08bf..68ac76489abc85a6b19f078a48a21e0acb4f9eec 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/DebtAggregator.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/DebtAggregator.java
@@ -26,7 +26,6 @@ import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.tracking.Tracking;
 import org.sonar.server.computation.component.Component;
 import org.sonar.server.computation.debt.Characteristic;
 import org.sonar.server.computation.debt.DebtModelHolder;
@@ -56,7 +55,7 @@ public class DebtAggregator extends IssueVisitor {
   }
 
   @Override
-  public void beforeComponent(Component component, Tracking tracking) {
+  public void beforeComponent(Component component) {
     currentDebt = new Debt();
     debtsByComponentRef.put(component.getRef(), currentDebt);
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueAssigner.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueAssigner.java
index 7f49d9831541c21778a008678af721463f0b8d29..937a8d217d7ed3ee05de1fb1730122ee7d647588 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueAssigner.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueAssigner.java
@@ -28,7 +28,6 @@ import javax.annotation.Nullable;
 import org.apache.commons.lang.StringUtils;
 import org.sonar.batch.protocol.output.BatchReport;
 import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.tracking.Tracking;
 import org.sonar.server.computation.batch.BatchReportReader;
 import org.sonar.server.computation.component.Component;
 import org.sonar.server.source.index.SourceLineDoc;
@@ -62,21 +61,12 @@ public class IssueAssigner extends IssueVisitor {
     this.defaultAssigne = defaultAssigne;
   }
 
-  @Override
-  public void beforeComponent(Component component, Tracking tracking) {
-    // optimization - do not load data if there are no new issues
-    if (!tracking.getUnmatchedRaws().isEmpty()) {
-      scmChangesets = loadScmChangesetsFromReport(component);
-      if (scmChangesets == null) {
-        scmChangesets = loadScmChangesetsFromIndex(component);
-      }
-      computeLastCommitDateAndAuthor();
-    }
-  }
-
   @Override
   public void onIssue(Component component, DefaultIssue issue) {
     if (issue.isNew()) {
+      // optimization - do not load SCM data of this component if there are no new issues
+      loadScmChangesetsIfNeeded(component);
+
       String scmAuthor = guessScmAuthor(issue.line());
       issue.setAuthorLogin(scmAuthor);
       if (scmAuthor != null) {
@@ -90,6 +80,16 @@ public class IssueAssigner extends IssueVisitor {
     }
   }
 
+  private void loadScmChangesetsIfNeeded(Component component) {
+    if (scmChangesets == null) {
+      scmChangesets = loadScmChangesetsFromReport(component);
+      if (scmChangesets == null) {
+        scmChangesets = loadScmChangesetsFromIndex(component);
+      }
+      computeLastCommitDateAndAuthor();
+    }
+  }
+
   @Override
   public void afterComponent(Component component) {
     lastCommitDate = 0L;

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueCounter.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueCounter.java
index 8c27aefac3589336aad86e957592ca6e92907f82..957ed281c2759468003ec749b32e0e4781a36ad4 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueCounter.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueCounter.java
@@ -27,7 +27,6 @@ import java.util.Map;
 import javax.annotation.Nullable;
 import org.sonar.api.issue.Issue;
 import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.tracking.Tracking;
 import org.sonar.server.computation.component.Component;
 import org.sonar.server.computation.measure.Measure;
 import org.sonar.server.computation.measure.MeasureRepository;
@@ -106,7 +105,7 @@ public class IssueCounter extends IssueVisitor {
   }
 
   @Override
-  public void beforeComponent(Component component, Tracking tracking) {
+  public void beforeComponent(Component component) {
     // TODO optimization no need to instantiate counter if no open issues
     currentCounters = new Counters();
     countersByComponentRef.put(component.getRef(), currentCounters);

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitor.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitor.java
index 6b3768dc26bc9a8e1686e14ce1fbd96d50027d94..d5e53dc4fb97359a815a6c5d2aafbe723de4b47f 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitor.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitor.java
@@ -20,7 +20,6 @@
 package org.sonar.server.computation.issue;
 
 import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.tracking.Tracking;
 import org.sonar.server.computation.component.Component;
 
 public abstract class IssueVisitor {
@@ -29,7 +28,7 @@ public abstract class IssueVisitor {
    * This method is called for each component before processing its issues.
    * The component does not necessarily have issues.
    */
-  public void beforeComponent(Component component, Tracking tracking) {
+  public void beforeComponent(Component component) {
 
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitors.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitors.java
index 51ddee18a058edc131f612a383c86266ee36deaf..1df195a01a395278c1fa6a9fa21a37464875aaa5 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitors.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitors.java
@@ -20,7 +20,6 @@
 package org.sonar.server.computation.issue;
 
 import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.tracking.Tracking;
 import org.sonar.server.computation.component.Component;
 
 public class IssueVisitors {
@@ -31,9 +30,9 @@ public class IssueVisitors {
     this.visitors = visitors;
   }
 
-  public void beforeComponent(Component component, Tracking tracking) {
+  public void beforeComponent(Component component) {
     for (IssueVisitor visitor : visitors) {
-      visitor.beforeComponent(component, tracking);
+      visitor.beforeComponent(component);
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/NewDebtAggregator.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/NewDebtAggregator.java
index d26b324ff14d1a816830159e3897c1e7aab6168b..c1d48f631b7b18b4807e9ec4b576f42e626944ec 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/NewDebtAggregator.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/NewDebtAggregator.java
@@ -28,7 +28,6 @@ import java.util.Map;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.db.IssueChangeDto;
-import org.sonar.core.issue.tracking.Tracking;
 import org.sonar.server.computation.component.Component;
 import org.sonar.server.computation.measure.Measure;
 import org.sonar.server.computation.measure.MeasureRepository;
@@ -61,7 +60,7 @@ public class NewDebtAggregator extends IssueVisitor {
   }
 
   @Override
-  public void beforeComponent(Component component, Tracking tracking) {
+  public void beforeComponent(Component component) {
     currentSum = new DebtSum();
     sumsByComponentRef.put(component.getRef(), currentSum);
     List<IssueChangeDto> changes = dbClient.issueChangeDao().selectChangelogOfNonClosedIssuesByComponent(component.getUuid());

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/TrackerBaseInputFactory.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/TrackerBaseInputFactory.java
index 8f5feb2573cff5fd5735f75a7803a494f3ae9435..cf7da885f81e38607fd1e69768d9ca25cd7e8939 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/TrackerBaseInputFactory.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/TrackerBaseInputFactory.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.server.computation.issue;
 
+import com.google.common.base.Objects;
+import java.util.Collections;
 import java.util.List;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.tracking.Input;
@@ -58,7 +60,7 @@ public class TrackerBaseInputFactory {
       DbSession session = dbClient.openSession(false);
       try {
         List<String> hashes = dbClient.fileSourceDao().selectLineHashes(session, component.getUuid());
-        return new LineHashSequence(hashes);
+        return new LineHashSequence(Objects.firstNonNull(hashes, Collections.<String>emptyList()));
       } finally {
         MyBatis.closeQuietly(session);
       }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/IntegrateIssuesStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/IntegrateIssuesStep.java
index c1c234d32e2fdba1c22871e79ffcc526759531a0..2a8eb12123b5c47d995347d6f562cf774debf783 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/IntegrateIssuesStep.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/IntegrateIssuesStep.java
@@ -75,22 +75,23 @@ public class IntegrateIssuesStep implements ComputationStep {
   }
 
   private void processIssues(Component component) {
-    Tracking<DefaultIssue, DefaultIssue> tracking = tracker.track(component);
     DiskCache<DefaultIssue>.DiskAppender cacheAppender = issueCache.newAppender();
     try {
-      issueVisitors.beforeComponent(component, tracking);
+      Tracking<DefaultIssue, DefaultIssue> tracking = tracker.track(component);
+      issueVisitors.beforeComponent(component);
       fillNewOpenIssues(component, tracking, cacheAppender);
       fillExistingOpenIssues(component, tracking, cacheAppender);
       closeUnmatchedBaseIssues(component, tracking, cacheAppender);
       issueVisitors.afterComponent(component);
+    } catch (Exception e) {
+      Loggers.get(getClass()).error(String.format("Fail to process issues of %s", component.getKey()), e);
     } finally {
       cacheAppender.close();
     }
   }
 
   private void fillNewOpenIssues(Component component, Tracking<DefaultIssue, DefaultIssue> tracking, DiskCache<DefaultIssue>.DiskAppender cacheAppender) {
-    Set<DefaultIssue> issues = tracking.getUnmatchedRaws();
-    for (DefaultIssue issue : issues) {
+    for (DefaultIssue issue : tracking.getUnmatchedRaws()) {
       issueLifecycle.initNewOpenIssue(issue);
       process(component, issue, cacheAppender);
     }
@@ -113,7 +114,6 @@ public class IntegrateIssuesStep implements ComputationStep {
 
   private void closeUnmatchedBaseIssues(Component component, Tracking<DefaultIssue, DefaultIssue> tracking, DiskCache<DefaultIssue>.DiskAppender cacheAppender) {
     for (DefaultIssue issue : tracking.getUnmatchedBases()) {
-      Loggers.get(getClass()).info("--- close base " + issue);
       // TODO should replace flag "beingClosed" by express call to transition "automaticClose"
       issue.setBeingClosed(true);
       // TODO manual issues -> was updater.setResolution(newIssue, Issue.RESOLUTION_REMOVED, changeContext);. Is it a problem ?

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/DebtAggregatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/DebtAggregatorTest.java
index 2542ac0c1751b67a50da502568d1f7b25e2f9438..44af4c3436e6356563603fd81e206ddbeb86a546 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/DebtAggregatorTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/DebtAggregatorTest.java
@@ -93,7 +93,7 @@ public class DebtAggregatorTest {
     DefaultIssue unresolvedWithoutDebt = new DefaultIssue().setRuleKey(rule.getKey());
     DefaultIssue resolved = new DefaultIssue().setDebt(Duration.create(50)).setResolution(RESOLUTION_FIXED).setRuleKey(rule.getKey());
 
-    underTest.beforeComponent(file, mock(Tracking.class));
+    underTest.beforeComponent(file);
     underTest.onIssue(file, unresolved1);
     underTest.onIssue(file, unresolved2);
     underTest.onIssue(file, unresolvedWithoutDebt);
@@ -118,10 +118,10 @@ public class DebtAggregatorTest {
     DefaultIssue fileIssue = new DefaultIssue().setDebt(Duration.create(10)).setRuleKey(rule.getKey());
     DefaultIssue projectIssue = new DefaultIssue().setDebt(Duration.create(30)).setRuleKey(rule.getKey());
 
-    underTest.beforeComponent(file, mock(Tracking.class));
+    underTest.beforeComponent(file);
     underTest.onIssue(file, fileIssue);
     underTest.afterComponent(file);
-    underTest.beforeComponent(project, mock(Tracking.class));
+    underTest.beforeComponent(project);
     underTest.onIssue(project, projectIssue);
     underTest.afterComponent(project);
 

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueAssignerTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueAssignerTest.java
index d5f8ab5a2e86b629b500fa2bb6aef37e626f5966..edc5a305279c8b4bd51dd4b3ad06b4155f5f7c79 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueAssignerTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueAssignerTest.java
@@ -26,7 +26,6 @@ import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.api.config.Settings;
 import org.sonar.batch.protocol.output.BatchReport;
-import org.sonar.core.issue.tracking.Tracking;
 import org.sonar.server.computation.batch.BatchReportReaderRule;
 import org.sonar.server.computation.component.Component;
 import org.sonar.server.computation.component.DumbComponent;
@@ -67,43 +66,43 @@ public class IssueAssignerTest {
       .addChangesetIndexByLine(1)
       .build());
 
-    underTest.beforeComponent(file, mock(Tracking.class));
-//    underTest.onIssue(file, issue);
-//    sut.init("ANY_UUID", 123_456_789, reportReader);
-//
-//    assertThat(sut.lineAuthor(1)).isEqualTo("charb");
-//    assertThat(sut.lineAuthor(2)).isEqualTo("charb");
-//    assertThat(sut.lineAuthor(3)).isEqualTo("wolinski");
-//    // compute last author
-//    assertThat(sut.lineAuthor(4)).isEqualTo("wolinski");
-//    assertThat(sut.lineAuthor(null)).isEqualTo("wolinski");
+    underTest.beforeComponent(file);
+    // underTest.onIssue(file, issue);
+    // sut.init("ANY_UUID", 123_456_789, reportReader);
+    //
+    // assertThat(sut.lineAuthor(1)).isEqualTo("charb");
+    // assertThat(sut.lineAuthor(2)).isEqualTo("charb");
+    // assertThat(sut.lineAuthor(3)).isEqualTo("wolinski");
+    // // compute last author
+    // assertThat(sut.lineAuthor(4)).isEqualTo("wolinski");
+    // assertThat(sut.lineAuthor(null)).isEqualTo("wolinski");
   }
 
-//  @Test
-//  public void line_author_from_index() throws Exception {
-//    esTester.putDocuments(SourceLineIndexDefinition.INDEX, SourceLineIndexDefinition.TYPE,
-//      newSourceLine("cabu", "123-456-789", 123_456_789, 1),
-//      newSourceLine("cabu", "123-456-789", 123_456_789, 2),
-//      newSourceLine("cabu", "123-123-789", 123_456_789, 3),
-//      newSourceLine("wolinski", "987-654-321", 987_654_321, 4),
-//      newSourceLine("cabu", "123-456-789", 123_456_789, 5)
-//      );
-//
-//    sut.init("DEFAULT_UUID", 123, reportReader);
-//
-//    assertThat(sut.lineAuthor(1)).isEqualTo("cabu");
-//    assertThat(sut.lineAuthor(2)).isEqualTo("cabu");
-//    assertThat(sut.lineAuthor(3)).isEqualTo("cabu");
-//    assertThat(sut.lineAuthor(4)).isEqualTo("wolinski");
-//    assertThat(sut.lineAuthor(5)).isEqualTo("cabu");
-//    assertThat(sut.lineAuthor(6)).isEqualTo("wolinski");
-//  }
-//
-//  @Test(expected = IllegalStateException.class)
-//  public void fail_when_component_ref_is_not_filled() {
-//    sut.init("ANY_UUID", null, reportReader);
-//    sut.lineAuthor(0);
-//  }
+  // @Test
+  // public void line_author_from_index() throws Exception {
+  // esTester.putDocuments(SourceLineIndexDefinition.INDEX, SourceLineIndexDefinition.TYPE,
+  // newSourceLine("cabu", "123-456-789", 123_456_789, 1),
+  // newSourceLine("cabu", "123-456-789", 123_456_789, 2),
+  // newSourceLine("cabu", "123-123-789", 123_456_789, 3),
+  // newSourceLine("wolinski", "987-654-321", 987_654_321, 4),
+  // newSourceLine("cabu", "123-456-789", 123_456_789, 5)
+  // );
+  //
+  // sut.init("DEFAULT_UUID", 123, reportReader);
+  //
+  // assertThat(sut.lineAuthor(1)).isEqualTo("cabu");
+  // assertThat(sut.lineAuthor(2)).isEqualTo("cabu");
+  // assertThat(sut.lineAuthor(3)).isEqualTo("cabu");
+  // assertThat(sut.lineAuthor(4)).isEqualTo("wolinski");
+  // assertThat(sut.lineAuthor(5)).isEqualTo("cabu");
+  // assertThat(sut.lineAuthor(6)).isEqualTo("wolinski");
+  // }
+  //
+  // @Test(expected = IllegalStateException.class)
+  // public void fail_when_component_ref_is_not_filled() {
+  // sut.init("ANY_UUID", null, reportReader);
+  // sut.lineAuthor(0);
+  // }
 
   private BatchReport.Changesets.Changeset.Builder newChangeset(String author, String revision, long date) {
     return BatchReport.Changesets.Changeset.newBuilder()

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueCounterTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueCounterTest.java
index 67988ec1f5ad6f51591514d518d86b3a17415bbf..d4f2994eda1d834c4e159148d24237f0f870c38d 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueCounterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueCounterTest.java
@@ -26,7 +26,6 @@ import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.tracking.Tracking;
 import org.sonar.server.computation.batch.BatchReportReaderRule;
 import org.sonar.server.computation.batch.TreeRootHolderRule;
 import org.sonar.server.computation.component.Component;
@@ -104,7 +103,6 @@ public class IssueCounterTest {
   @Rule
   public PeriodsHolderRule periodsHolder = new PeriodsHolderRule();
 
-  Tracking tracking = mock(Tracking.class);
   MetricRepository metricRepository = mock(MetricRepository.class);
   MeasureRepository measureRepository;
   IssueCounter sut;
@@ -122,21 +120,21 @@ public class IssueCounterTest {
     periodsHolder.setPeriods();
 
     // bottom-up traversal -> from files to project
-    sut.beforeComponent(FILE1, tracking);
+    sut.beforeComponent(FILE1);
     sut.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER));
     sut.onIssue(FILE1, createIssue(RESOLUTION_FIXED, STATUS_CLOSED, MAJOR));
     sut.onIssue(FILE1, createIssue(RESOLUTION_FALSE_POSITIVE, STATUS_RESOLVED, MAJOR));
     sut.afterComponent(FILE1);
 
-    sut.beforeComponent(FILE2, tracking);
+    sut.beforeComponent(FILE2);
     sut.onIssue(FILE2, createIssue(null, STATUS_CONFIRMED, BLOCKER));
     sut.onIssue(FILE2, createIssue(null, STATUS_CONFIRMED, MAJOR));
     sut.afterComponent(FILE2);
 
-    sut.beforeComponent(FILE3, tracking);
+    sut.beforeComponent(FILE3);
     sut.afterComponent(FILE3);
 
-    sut.beforeComponent(PROJECT, tracking);
+    sut.beforeComponent(PROJECT);
     sut.afterComponent(PROJECT);
 
     // count by status
@@ -163,18 +161,18 @@ public class IssueCounterTest {
     periodsHolder.setPeriods();
 
     // bottom-up traversal -> from files to project
-    sut.beforeComponent(FILE1, tracking);
+    sut.beforeComponent(FILE1);
     sut.onIssue(FILE1, createIssue(null, STATUS_OPEN, BLOCKER));
     // this resolved issue is ignored
     sut.onIssue(FILE1, createIssue(RESOLUTION_FIXED, STATUS_CLOSED, MAJOR));
     sut.afterComponent(FILE1);
 
-    sut.beforeComponent(FILE2, tracking);
+    sut.beforeComponent(FILE2);
     sut.onIssue(FILE2, createIssue(null, STATUS_CONFIRMED, BLOCKER));
     sut.onIssue(FILE2, createIssue(null, STATUS_CONFIRMED, MAJOR));
     sut.afterComponent(FILE2);
 
-    sut.beforeComponent(PROJECT, tracking);
+    sut.beforeComponent(PROJECT);
     sut.afterComponent(PROJECT);
 
     assertThat(measureRepository.getRawMeasure(FILE1, BLOCKER_ISSUES_METRIC).get().getIntValue()).isEqualTo(1);
@@ -195,7 +193,7 @@ public class IssueCounterTest {
     Period period = newPeriod(3, 1500000000000L);
     periodsHolder.setPeriods(period);
 
-    sut.beforeComponent(FILE1, tracking);
+    sut.beforeComponent(FILE1);
     // created before -> existing issues
     sut.onIssue(FILE1, createIssueAt(null, STATUS_OPEN, BLOCKER, period.getSnapshotDate() - 1000000L));
     // created during the first analysis starting the period -> existing issues
@@ -205,10 +203,10 @@ public class IssueCounterTest {
     sut.onIssue(FILE1, createIssueAt(RESOLUTION_FIXED, STATUS_CLOSED, MAJOR, period.getSnapshotDate() + 200000L));
     sut.afterComponent(FILE1);
 
-    sut.beforeComponent(FILE2, tracking);
+    sut.beforeComponent(FILE2);
     sut.afterComponent(FILE2);
 
-    sut.beforeComponent(PROJECT, tracking);
+    sut.beforeComponent(PROJECT);
     sut.afterComponent(PROJECT);
 
     assertVariation(FILE1, NEW_ISSUES_METRIC, period.getIndex(), 1);

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/NewDebtAggregatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/NewDebtAggregatorTest.java
index cf864b8a8e05891eaf5e9686c3cbbe68eee5ec06..f8f217394b9d99138de09b5232094fb649d48106 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/NewDebtAggregatorTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/NewDebtAggregatorTest.java
@@ -25,7 +25,6 @@ import org.mockito.Mockito;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.api.utils.Duration;
 import org.sonar.core.issue.DefaultIssue;
-import org.sonar.core.issue.tracking.Tracking;
 import org.sonar.server.computation.component.Component;
 import org.sonar.server.computation.component.DumbComponent;
 import org.sonar.server.computation.measure.Measure;
@@ -78,7 +77,7 @@ public class NewDebtAggregatorTest {
     when(calculator.calculate(same(unresolved2), anyList(), same(PERIOD))).thenReturn(3L);
     verifyNoMoreInteractions(calculator);
 
-    underTest.beforeComponent(file, mock(Tracking.class));
+    underTest.beforeComponent(file);
     underTest.onIssue(file, unresolved1);
     underTest.onIssue(file, unresolved2);
     underTest.onIssue(file, unresolvedWithoutDebt);
@@ -105,7 +104,7 @@ public class NewDebtAggregatorTest {
     DefaultIssue unresolved = new DefaultIssue().setDebt(Duration.create(10));
     verifyZeroInteractions(calculator);
 
-    underTest.beforeComponent(file, mock(Tracking.class));
+    underTest.beforeComponent(file);
     underTest.onIssue(file, unresolved);
     underTest.afterComponent(file);
 

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java
index 55138a349950c9bd69e03dcb5b1bae5cafefced9..70ac173b3c4a4bda47f5797ff8f21e257e31fadd 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java
@@ -123,15 +123,14 @@ class BlockRecognizer<RAW extends Trackable, BASE extends Trackable> {
     for (RAW raw : raws) {
       for (BASE base : bases) {
         if (result.containsUnmatchedBase(base) && base.getRuleKey().equals(raw.getRuleKey())) {
-          result.associateRawToBase(raw, base);
-          result.markRawAsAssociated(raw);
+          result.match(raw, base);
           break;
         }
       }
     }
   }
 
-  private static <T extends Trackable> Multimap<Integer, T> groupByLine(Collection<T> trackables, BlockHashSequence hashSequence) {
+  private static <T extends Trackable> Multimap<Integer, T> groupByLine(Iterable<T> trackables, BlockHashSequence hashSequence) {
     Multimap<Integer, T> result = LinkedHashMultimap.create();
     for (T trackable : trackables) {
       Integer line = trackable.getLine();

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java
index e7cd196bbca521c2b9c2a7819a6cf31388163651..9709ca8886e8386f302f49070fdd200ca73ccd68 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java
@@ -23,9 +23,7 @@ import com.google.common.base.Predicate;
 import com.google.common.base.Strings;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Multimap;
-import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashSet;
 import java.util.Objects;
 import javax.annotation.Nonnull;
 import org.apache.commons.lang.StringUtils;
@@ -75,7 +73,6 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> {
       baseSearch.put(factory.create(base), base);
     }
 
-    Collection<RAW> trackedRaws = new ArrayList<>();
     for (RAW raw : tracking.getUnmatchedRaws()) {
       SearchKey rawKey = factory.create(raw);
       Collection<BASE> bases = baseSearch.get(rawKey);
@@ -83,21 +80,19 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> {
         // TODO taking the first one. Could be improved if there are more than 2 issues on the same line.
         // Message could be checked to take the best one.
         BASE match = bases.iterator().next();
-        tracking.associateRawToBase(raw, match);
+        tracking.match(raw, match);
         baseSearch.remove(rawKey, match);
-        trackedRaws.add(raw);
       }
     }
-    tracking.markRawsAsAssociated(trackedRaws);
   }
 
   private void relocateManualIssues(Input<RAW> rawInput, Input<BASE> baseInput, Tracking<RAW, BASE> tracking) {
     // FIXME copy of Set if required to avoid concurrent modifications (see tracking.associateManualIssueToLine())
-    Iterable<BASE> manualIssues = from(new HashSet<>(tracking.getUnmatchedBases())).filter(IsManual.INSTANCE);
+    Iterable<BASE> manualIssues = from(tracking.getUnmatchedBases()).filter(IsManual.INSTANCE);
     for (BASE base : manualIssues) {
       if (base.getLine() == null) {
         // no need to relocate. Location is unchanged.
-        tracking.associateManualIssueToLine(base, null);
+        tracking.keepManualIssueOpen(base, null);
       } else {
         String baseHash = base.getLineHash();
         if (Strings.isNullOrEmpty(baseHash)) {
@@ -106,11 +101,11 @@ public class Tracker<RAW extends Trackable, BASE extends Trackable> {
         if (!Strings.isNullOrEmpty(baseHash)) {
           int[] rawLines = rawInput.getLineHashSequence().getLinesForHash(baseHash);
           if (rawLines.length == 1) {
-            tracking.associateManualIssueToLine(base, rawLines[0]);
+            tracking.keepManualIssueOpen(base, rawLines[0]);
           } else if (rawLines.length == 0 && rawInput.getLineHashSequence().hasLine(base.getLine())) {
             // still valid (???). We didn't manage to correctly detect code move, so the
             // issue is kept at the same location, even if code changes
-            tracking.associateManualIssueToLine(base, base.getLine());
+            tracking.keepManualIssueOpen(base, base.getLine());
           }
           // TODO if hash found multiple times, , pick the closest line
         }

diff --git a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java
index 579c6f51623cedcad37e912f6039b99e3d22b5ba..33b126a630054551829670b50d4687dda218ea3e 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java
@@ -20,47 +20,61 @@
 package org.sonar.core.issue.tracking;
 
 import com.google.common.base.Objects;
+import com.google.common.base.Predicate;
 import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Multimap;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.IdentityHashMap;
 import java.util.Map;
-import java.util.Set;
 import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 public class Tracking<RAW extends Trackable, BASE extends Trackable> {
 
   /**
-   * Tracked issues -> a raw issue is associated to a base issue
+   * Matched issues -> a raw issue is associated to a base issue
    */
   private final IdentityHashMap<RAW, BASE> rawToBase = new IdentityHashMap<>();
+  private final IdentityHashMap<BASE, RAW> baseToRaw = new IdentityHashMap<>();
 
-  /**
-   * The raw issues that are not associated to a base issue.
-   */
-  private final Set<RAW> unmatchedRaws = Collections.newSetFromMap(new IdentityHashMap<RAW, Boolean>());
+  private final Collection<RAW> raws;
+  private final Collection<BASE> bases;
 
-  /**
-   * IdentityHashSet of the base issues that are not associated to a raw issue.
-   */
-  private final Set<BASE> unmatchedBases = Collections.newSetFromMap(new IdentityHashMap<BASE, Boolean>());
+  private final Predicate<RAW> unmatchedRawPredicate = new Predicate<RAW>() {
+    @Override
+    public boolean apply(@Nonnull RAW raw) {
+      return !rawToBase.containsKey(raw);
+    }
+  };
+
+  private final Predicate<BASE> unmatchedBasePredicate = new Predicate<BASE>() {
+    @Override
+    public boolean apply(@Nonnull BASE raw) {
+      return !baseToRaw.containsKey(raw);
+    }
+  };
 
   /**
    * The manual issues that are still valid (related source code still exists). They
    * are grouped by line. Lines start with 1. The key 0 references the manual
    * issues that do not relate to a line.
    */
-  private final Multimap<Integer, BASE> openManualIssues = ArrayListMultimap.create();
+  private final Multimap<Integer, BASE> openManualIssuesByLine = ArrayListMultimap.create();
 
   public Tracking(Input<RAW> rawInput, Input<BASE> baseInput) {
-    this.unmatchedRaws.addAll(rawInput.getIssues());
-    this.unmatchedBases.addAll(baseInput.getIssues());
+    this.raws = rawInput.getIssues();
+    this.bases = baseInput.getIssues();
   }
 
-  public Set<RAW> getUnmatchedRaws() {
-    return unmatchedRaws;
+  /**
+   * Returns an Iterable to be traversed when matching issues. That means
+   * that the traversal does not fail if method {@link #match(Trackable, Trackable)}
+   * is called.
+   */
+  public Iterable<RAW> getUnmatchedRaws() {
+    return Iterables.filter(raws, unmatchedRawPredicate);
   }
 
   public Map<RAW, BASE> getMatchedRaws() {
@@ -73,58 +87,42 @@ public class Tracking<RAW extends Trackable, BASE extends Trackable> {
   }
 
   /**
-   * The base issues that are not matched by a raw issue and that need to be closed. Manual
+   * The base issues that are not matched by a raw issue and that need to be closed.
    */
-  public Set<BASE> getUnmatchedBases() {
-    return unmatchedBases;
+  public Iterable<BASE> getUnmatchedBases() {
+    return Iterables.filter(bases, unmatchedBasePredicate);
   }
 
   boolean containsUnmatchedBase(BASE base) {
-    return unmatchedBases.contains(base);
+    return !baseToRaw.containsKey(base);
   }
 
-  void associateRawToBase(RAW raw, BASE base) {
+  void match(RAW raw, BASE base) {
     rawToBase.put(raw, base);
-    if (!unmatchedBases.remove(base)) {
-      throw new IllegalStateException(String.format("Fail to associate base issue %s to %s among %s", base, raw, this));
-    }
-  }
-
-  void markRawAsAssociated(RAW raw) {
-    if (!unmatchedRaws.remove(raw)) {
-      throw new IllegalStateException(String.format("Fail to mark issue as associated: %s among %s", raw, this));
-    }
-  }
-
-  void markRawsAsAssociated(Collection<RAW> c) {
-    // important : do not use unmatchedRaws.removeAll(Collection) as it's buggy. See:
-    // http://stackoverflow.com/questions/19682542/why-identityhashmap-keyset-removeallkeys-does-not-use-identity-is-it-a-bug/19682543#19682543
-    // http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6588783
-    for (RAW raw : c) {
-      markRawAsAssociated(raw);
-    }
+    baseToRaw.put(base, raw);
   }
 
   boolean isComplete() {
-    return unmatchedRaws.isEmpty() || unmatchedBases.isEmpty();
+    return rawToBase.size() == raws.size();
   }
 
   public Multimap<Integer, BASE> getOpenManualIssuesByLine() {
-    return openManualIssues;
+    return openManualIssuesByLine;
   }
 
-  void associateManualIssueToLine(BASE manualIssue, @Nullable Integer line) {
-    openManualIssues.put(line, manualIssue);
-    unmatchedBases.remove(manualIssue);
+  void keepManualIssueOpen(BASE manualIssue, @Nullable Integer line) {
+    openManualIssuesByLine.put(line, manualIssue);
+    baseToRaw.put(manualIssue, null);
   }
 
   @Override
   public String toString() {
     return Objects.toStringHelper(this)
       .add("rawToBase", rawToBase)
-      .add("unmatchedRaws", unmatchedRaws)
-      .add("unmatchedBases", unmatchedBases)
-      .add("openManualIssues", openManualIssues)
+      .add("baseToRaw", baseToRaw)
+      .add("raws", raws)
+      .add("bases", bases)
+      .add("openManualIssuesByLine", openManualIssuesByLine)
       .toString();
   }
 }