SONAR-5375 Optimize issue tracking in case we have many issues of the same rule and on the same file

2 files changed, 61 insertions, 38 deletions

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java
index 876112646d9..d5dbcccc83f 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java
@@ -20,8 +20,6 @@
 
 package org.sonar.plugins.core.issue;
 
-import org.sonar.plugins.core.issue.tracking.SourceChecksum;
-
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Objects;
 import com.google.common.collect.LinkedHashMultimap;
@@ -37,6 +35,7 @@ import org.sonar.plugins.core.issue.tracking.HashedSequenceComparator;
 import org.sonar.plugins.core.issue.tracking.IssueTrackingBlocksRecognizer;
 import org.sonar.plugins.core.issue.tracking.RollingHashSequence;
 import org.sonar.plugins.core.issue.tracking.RollingHashSequenceComparator;
+import org.sonar.plugins.core.issue.tracking.SourceChecksum;
 import org.sonar.plugins.core.issue.tracking.StringText;
 import org.sonar.plugins.core.issue.tracking.StringTextComparator;
 
@@ -67,7 +66,6 @@ public class IssueTracking implements BatchExtension {
     }
   }
 
-
   @VisibleForTesting
   void mapIssues(Collection<DefaultIssue> newIssues, @Nullable Collection<IssueDto> lastIssues, SourceHashHolder sourceHashHolder, IssueTrackingResult result) {
     boolean hasLastScan = false;
@@ -93,7 +91,7 @@ public class IssueTracking implements BatchExtension {
 
     // Match the key of the issue. (For manual issues)
     for (DefaultIssue newIssue : newIssues) {
-      mapIssue(newIssue, findLastIssueWithSameKey(newIssue, result.unmatchedForRule(newIssue.ruleKey())), result);
+      mapIssue(newIssue, result.unmatchedByKeyForRule(newIssue.ruleKey()).get(newIssue.key()), result);
     }
 
     // Try first to match issues on same rule with same line and with same checksum (but not necessarily with same message)
@@ -101,7 +99,7 @@ public class IssueTracking implements BatchExtension {
       if (isNotAlreadyMapped(newIssue, result)) {
         mapIssue(
           newIssue,
-          findLastIssueWithSameLineAndChecksum(newIssue, result.unmatchedForRule(newIssue.ruleKey())),
+          findLastIssueWithSameLineAndChecksum(newIssue, result),
           result);
       }
     }
@@ -176,7 +174,7 @@ public class IssueTracking implements BatchExtension {
       if (isNotAlreadyMapped(newIssue, result)) {
         mapIssue(
           newIssue,
-          findLastIssueWithSameChecksumAndMessage(newIssue, result.unmatchedForRule(newIssue.ruleKey())),
+          findLastIssueWithSameChecksumAndMessage(newIssue, result.unmatchedByKeyForRule(newIssue.ruleKey()).values()),
           result);
       }
     }
@@ -186,7 +184,7 @@ public class IssueTracking implements BatchExtension {
       if (isNotAlreadyMapped(newIssue, result)) {
         mapIssue(
           newIssue,
-          findLastIssueWithSameLineAndMessage(newIssue, result.unmatchedForRule(newIssue.ruleKey())),
+          findLastIssueWithSameLineAndMessage(newIssue, result.unmatchedByKeyForRule(newIssue.ruleKey()).values()),
           result);
       }
     }
@@ -197,7 +195,7 @@ public class IssueTracking implements BatchExtension {
       if (isNotAlreadyMapped(newIssue, result)) {
         mapIssue(
           newIssue,
-          findLastIssueWithSameChecksum(newIssue, result.unmatchedForRule(newIssue.ruleKey())),
+          findLastIssueWithSameChecksum(newIssue, result.unmatchedByKeyForRule(newIssue.ruleKey()).values()),
           result);
       }
     }
@@ -263,20 +261,10 @@ public class IssueTracking implements BatchExtension {
     return null;
   }
 
-  private IssueDto findLastIssueWithSameLineAndChecksum(DefaultIssue newIssue, Collection<IssueDto> lastIssues) {
-    for (IssueDto pastIssue : lastIssues) {
-      if (isSameLine(newIssue, pastIssue) && isSameChecksum(newIssue, pastIssue)) {
-        return pastIssue;
-      }
-    }
-    return null;
-  }
-
-  private IssueDto findLastIssueWithSameKey(DefaultIssue newIssue, Collection<IssueDto> lastIssues) {
-    for (IssueDto pastIssue : lastIssues) {
-      if (isSameKey(newIssue, pastIssue)) {
-        return pastIssue;
-      }
+  private IssueDto findLastIssueWithSameLineAndChecksum(DefaultIssue newIssue, IssueTrackingResult result) {
+    Collection<IssueDto> sameRuleAndSameLineAndSameChecksum = result.unmatchedForRuleAndForLineAndForChecksum(newIssue.ruleKey(), newIssue.line(), newIssue.checksum());
+    if (!sameRuleAndSameLineAndSameChecksum.isEmpty()) {
+      return sameRuleAndSameLineAndSameChecksum.iterator().next();
     }
     return null;
   }
@@ -301,10 +289,6 @@ public class IssueTracking implements BatchExtension {
     return Objects.equal(newIssue.message(), pastIssue.getMessage());
   }
 
-  private boolean isSameKey(DefaultIssue newIssue, IssueDto pastIssue) {
-    return Objects.equal(newIssue.key(), pastIssue.getKee());
-  }
-
   private void mapIssue(DefaultIssue issue, @Nullable IssueDto ref, IssueTrackingResult result) {
     if (ref != null) {
       result.setMatch(issue, ref);
diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingResult.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingResult.java
index a3f84600a89..147d89ef01c 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingResult.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingResult.java
@@ -19,29 +19,51 @@
  */
 package org.sonar.plugins.core.issue;
 
-import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
-import com.google.common.collect.Sets;
+import org.apache.commons.lang.StringUtils;
 import org.sonar.api.issue.internal.DefaultIssue;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.core.issue.db.IssueDto;
 
+import javax.annotation.Nullable;
+
 import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
 import java.util.Map;
-import java.util.Set;
 
 class IssueTrackingResult {
-  private final Set<IssueDto> unmatched = Sets.newHashSet();
-  private final Multimap<RuleKey, IssueDto> unmatchedByRule = LinkedHashMultimap.create();
+  private final Map<String, IssueDto> unmatchedByKey = new HashMap<String, IssueDto>();
+  private final Map<RuleKey, Map<String, IssueDto>> unmatchedByRuleAndKey = new HashMap<RuleKey, Map<String, IssueDto>>();
+  private final Map<RuleKey, Map<Integer, Multimap<String, IssueDto>>> unmatchedByRuleAndLineAndChecksum =
+    new HashMap<RuleKey, Map<Integer, Multimap<String, IssueDto>>>();
   private final Map<DefaultIssue, IssueDto> matched = Maps.newIdentityHashMap();
 
   Collection<IssueDto> unmatched() {
-    return unmatched;
+    return unmatchedByKey.values();
+  }
+
+  Map<String, IssueDto> unmatchedByKeyForRule(RuleKey ruleKey) {
+    return unmatchedByRuleAndKey.containsKey(ruleKey) ? unmatchedByRuleAndKey.get(ruleKey) : Collections.<String, IssueDto>emptyMap();
   }
 
-  Collection<IssueDto> unmatchedForRule(RuleKey ruleKey) {
-    return unmatchedByRule.get(ruleKey);
+  Collection<IssueDto> unmatchedForRuleAndForLineAndForChecksum(RuleKey ruleKey, @Nullable Integer line, @Nullable String checksum) {
+    if (!unmatchedByRuleAndLineAndChecksum.containsKey(ruleKey)) {
+      return Collections.emptyList();
+    }
+    Map<Integer, Multimap<String, IssueDto>> unmatchedForRule = unmatchedByRuleAndLineAndChecksum.get(ruleKey);
+    Integer lineNotNull = line != null ? line : 0;
+    if (!unmatchedForRule.containsKey(lineNotNull)) {
+      return Collections.emptyList();
+    }
+    Multimap<String, IssueDto> unmatchedForRuleAndLine = unmatchedForRule.get(lineNotNull);
+    String checksumNotNull = StringUtils.defaultString(checksum, "");
+    if (!unmatchedForRuleAndLine.containsKey(checksumNotNull)) {
+      return Collections.emptyList();
+    }
+    return unmatchedForRuleAndLine.get(checksumNotNull);
   }
 
   Collection<DefaultIssue> matched() {
@@ -57,13 +79,30 @@ class IssueTrackingResult {
   }
 
   void addUnmatched(IssueDto i) {
-    unmatched.add(i);
-    unmatchedByRule.put(RuleKey.of(i.getRuleRepo(), i.getRule()), i);
+    unmatchedByKey.put(i.getKee(), i);
+    RuleKey ruleKey = RuleKey.of(i.getRuleRepo(), i.getRule());
+    if (!unmatchedByRuleAndKey.containsKey(ruleKey)) {
+      unmatchedByRuleAndKey.put(ruleKey, new HashMap<String, IssueDto>());
+      unmatchedByRuleAndLineAndChecksum.put(ruleKey, new HashMap<Integer, Multimap<String, IssueDto>>());
+    }
+    unmatchedByRuleAndKey.get(ruleKey).put(i.getKee(), i);
+    Map<Integer, Multimap<String, IssueDto>> unmatchedForRule = unmatchedByRuleAndLineAndChecksum.get(ruleKey);
+    Integer lineNotNull = i.getLine() != null ? i.getLine() : 0;
+    if (!unmatchedForRule.containsKey(lineNotNull)) {
+      unmatchedForRule.put(lineNotNull, HashMultimap.<String, IssueDto>create());
+    }
+    Multimap<String, IssueDto> unmatchedForRuleAndLine = unmatchedForRule.get(lineNotNull);
+    String checksumNotNull = StringUtils.defaultString(i.getChecksum(), "");
+    unmatchedForRuleAndLine.put(checksumNotNull, i);
   }
 
   void setMatch(DefaultIssue issue, IssueDto matching) {
     matched.put(issue, matching);
-    unmatchedByRule.remove(RuleKey.of(matching.getRuleRepo(), matching.getRule()), matching);
-    unmatched.remove(matching);
+    RuleKey ruleKey = RuleKey.of(matching.getRuleRepo(), matching.getRule());
+    unmatchedByRuleAndKey.get(ruleKey).remove(matching.getKee());
+    unmatchedByKey.remove(matching.getKee());
+    Integer lineNotNull = matching.getLine() != null ? matching.getLine() : 0;
+    String checksumNotNull = StringUtils.defaultString(matching.getChecksum(), "");
+    unmatchedByRuleAndLineAndChecksum.get(ruleKey).get(lineNotNull).get(checksumNotNull).remove(matching);
   }
 }