aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSimon Brandhof <simon.brandhof@sonarsource.com>2015-07-03 11:17:15 +0200
committerSimon Brandhof <simon.brandhof@sonarsource.com>2015-07-03 11:17:15 +0200
commit7f218017d505877feb4415b0568dde3fb8492172 (patch)
treee749abe8eb50a3d4e3463b0affa03bba255c8dec
parent27bb7a8e4109529b275a6e3be5b72d01014f0879 (diff)
downloadsonarqube-7f218017d505877feb4415b0568dde3fb8492172.tar.gz
sonarqube-7f218017d505877feb4415b0568dde3fb8492172.zip
Fix stability of issue tracking
-rw-r--r--server/sonar-server/src/main/java/org/sonar/server/computation/issue/DebtAggregator.java3
-rw-r--r--server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueAssigner.java26
-rw-r--r--server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueCounter.java3
-rw-r--r--server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitor.java3
-rw-r--r--server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueVisitors.java5
-rw-r--r--server/sonar-server/src/main/java/org/sonar/server/computation/issue/NewDebtAggregator.java3
-rw-r--r--server/sonar-server/src/main/java/org/sonar/server/computation/issue/TrackerBaseInputFactory.java4
-rw-r--r--server/sonar-server/src/main/java/org/sonar/server/computation/step/IntegrateIssuesStep.java10
-rw-r--r--server/sonar-server/src/test/java/org/sonar/server/computation/issue/DebtAggregatorTest.java6
-rw-r--r--server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueAssignerTest.java71
-rw-r--r--server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueCounterTest.java22
-rw-r--r--server/sonar-server/src/test/java/org/sonar/server/computation/issue/NewDebtAggregatorTest.java5
-rw-r--r--sonar-core/src/main/java/org/sonar/core/issue/tracking/BlockRecognizer.java5
-rw-r--r--sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracker.java15
-rw-r--r--sonar-core/src/main/java/org/sonar/core/issue/tracking/Tracking.java92
15 files changed, 129 insertions, 144 deletions
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 c8c410a6503..68ac76489ab 100644
--- 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 7f49d983154..937a8d217d7 100644
--- 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;
@@ -63,20 +62,11 @@ public class IssueAssigner extends IssueVisitor {
}
@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 8c27aefac35..957ed281c27 100644
--- 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 6b3768dc26b..d5e53dc4fb9 100644
--- 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 51ddee18a05..1df195a01a3 100644
--- 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 d26b324ff14..c1d48f631b7 100644
--- 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 8f5feb2573c..cf7da885f81 100644
--- 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 c1c234d32e2..2a8eb12123b 100644
--- 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 2542ac0c175..44af4c3436e 100644
--- 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 d5f8ab5a2e8..edc5a305279 100644
--- 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 67988ec1f5a..d4f2994eda1 100644
--- 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 cf864b8a8e0..f8f217394b9 100644
--- 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 55138a34995..70ac173b3c4 100644
--- 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 e7cd196bbca..9709ca8886e 100644
--- 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 579c6f51623..33b126a6300 100644
--- 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();
}
}