SONAR-18877 Compute primary location hash for all issues

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComputeLocationHashesVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComputeLocationHashesVisitor.java
index 6697699a44d1b2fddceea0e44dad439511adaad8..587d18b5adcf337de420066b676a3cab7263d0ef 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComputeLocationHashesVisitor.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComputeLocationHashesVisitor.java
@@ -35,16 +35,17 @@ import org.sonar.db.protobuf.DbIssues;
 import org.sonar.server.issue.TaintChecker;
 
 import static org.apache.commons.lang.StringUtils.defaultIfEmpty;
-import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT;
 
 /**
- * This visitor will update the locations field of issues, by filling the hashes for all locations.
- * It only applies to issues that are taint vulnerabilities or security hotspots, and that are new or were changed.
+ * This visitor will update the locations field of issues, by filling hashes for their locations:
+ * - Primary location hash: for all issues, when needed (ie. is missing or the issue is new/updated)
+ * - Secondary location hash: only for taint vulnerabilities and security hotspots, when needed (the issue is new/updated)
  * For performance reasons, it will read each source code file once and feed the lines to all locations in that file.
  */
 public class ComputeLocationHashesVisitor extends IssueVisitor {
   private static final Pattern MATCH_ALL_WHITESPACES = Pattern.compile("\\s");
-  private final List<DefaultIssue> issues = new LinkedList<>();
+  private final List<DefaultIssue> issuesForAllLocations = new LinkedList<>();
+  private final List<DefaultIssue> issuesForPrimaryLocation = new LinkedList<>();
   private final SourceLinesRepository sourceLinesRepository;
   private final TreeRootHolder treeRootHolder;
   private final TaintChecker taintChecker;
@@ -57,20 +58,41 @@ public class ComputeLocationHashesVisitor extends IssueVisitor {
 
   @Override
   public void beforeComponent(Component component) {
-    issues.clear();
+    issuesForAllLocations.clear();
+    issuesForPrimaryLocation.clear();
   }
 
   @Override
   public void onIssue(Component component, DefaultIssue issue) {
-    if (shouldComputeLocation(issue)) {
-      issues.add(issue);
+    if (issueNeedsLocationHashes(issue)) {
+      if (shouldComputeAllLocationHashes(issue)) {
+        issuesForAllLocations.add(issue);
+      } else if (shouldComputePrimaryLocationHash(issue)) {
+        // Issues in this situation are not necessarily marked as changed, so we do it to ensure persistence
+        issue.setChanged(true);
+        issuesForPrimaryLocation.add(issue);
+      }
     }
   }
 
-  private boolean shouldComputeLocation(DefaultIssue issue) {
-    return (taintChecker.isTaintVulnerability(issue) || SECURITY_HOTSPOT.equals(issue.type()))
-      && !issue.isFromExternalRuleEngine()
-      && (issue.isNew() || issue.locationsChanged());
+  private static boolean issueNeedsLocationHashes(DefaultIssue issue) {
+    DbIssues.Locations locations = issue.getLocations();
+    return locations != null && !issue.isFromExternalRuleEngine();
+  }
+
+  private boolean shouldComputeAllLocationHashes(DefaultIssue issue) {
+    return taintChecker.isTaintVulnerability(issue)
+      && isIssueUpdated(issue);
+  }
+
+  private static boolean shouldComputePrimaryLocationHash(DefaultIssue issue) {
+    DbIssues.Locations locations = issue.getLocations();
+    return (locations.hasTextRange() && !locations.hasChecksum())
+      || isIssueUpdated(issue);
+  }
+
+  private static boolean isIssueUpdated(DefaultIssue issue) {
+    return issue.isNew() || issue.locationsChanged();
   }
 
   @Override
@@ -78,20 +100,10 @@ public class ComputeLocationHashesVisitor extends IssueVisitor {
     Map<Component, List<Location>> locationsByComponent = new HashMap<>();
     List<LocationToSet> locationsToSet = new LinkedList<>();
 
-    for (DefaultIssue issue : issues) {
-      DbIssues.Locations locations = issue.getLocations();
-      if (locations == null) {
-        continue;
-      }
-
-      DbIssues.Locations.Builder primaryLocationBuilder = locations.toBuilder();
-      boolean hasTextRange = addLocations(component, issue, locationsByComponent, primaryLocationBuilder);
-
-      // If any location was added (because it had a text range), we'll need to update the issue at the end with the new object containing the hashes
-      if (hasTextRange) {
-        locationsToSet.add(new LocationToSet(issue, primaryLocationBuilder));
-      }
-    }
+    // Issues that needs both primary and secondary locations hashes
+    extractForAllLocations(component, locationsByComponent, locationsToSet);
+    // Then issues that needs only primary locations
+    extractForPrimaryLocation(component, locationsByComponent, locationsToSet);
 
     // Feed lines to locations, component by component
     locationsByComponent.forEach(this::updateLocationsInComponent);
@@ -102,37 +114,41 @@ public class ComputeLocationHashesVisitor extends IssueVisitor {
     // set new locations to issues
     locationsToSet.forEach(LocationToSet::set);
 
-    issues.clear();
+    issuesForAllLocations.clear();
+    issuesForPrimaryLocation.clear();
   }
 
-  private boolean addLocations(Component component, DefaultIssue issue, Map<Component, List<Location>> locationsByComponent, DbIssues.Locations.Builder primaryLocationBuilder) {
-    boolean hasPrimaryLocation = addPrimaryLocation(component, locationsByComponent, primaryLocationBuilder);
-    boolean hasSecondaryLocations = addSecondaryLocations(issue, locationsByComponent, primaryLocationBuilder);
-    return hasPrimaryLocation || hasSecondaryLocations;
+  private void extractForAllLocations(Component component, Map<Component, List<Location>> locationsByComponent, List<LocationToSet> locationsToSet) {
+    for (DefaultIssue issue : issuesForAllLocations) {
+      DbIssues.Locations.Builder locationsBuilder = ((DbIssues.Locations) issue.getLocations()).toBuilder();
+      addPrimaryLocation(component, locationsByComponent, locationsBuilder);
+      addSecondaryLocations(issue, locationsByComponent, locationsBuilder);
+      locationsToSet.add(new LocationToSet(issue, locationsBuilder));
+    }
   }
 
-  private static boolean addPrimaryLocation(Component component, Map<Component, List<Location>> locationsByComponent, DbIssues.Locations.Builder primaryLocationBuilder) {
-    if (!primaryLocationBuilder.hasTextRange()) {
-      return false;
+  private void extractForPrimaryLocation(Component component, Map<Component, List<Location>> locationsByComponent, List<LocationToSet> locationsToSet) {
+    for (DefaultIssue issue : issuesForPrimaryLocation) {
+      DbIssues.Locations.Builder locationsBuilder = ((DbIssues.Locations) issue.getLocations()).toBuilder();
+      addPrimaryLocation(component, locationsByComponent, locationsBuilder);
+      locationsToSet.add(new LocationToSet(issue, locationsBuilder));
     }
-    PrimaryLocation primaryLocation = new PrimaryLocation(primaryLocationBuilder);
-    locationsByComponent.computeIfAbsent(component, c -> new LinkedList<>()).add(primaryLocation);
-    return true;
   }
 
-  private boolean addSecondaryLocations(DefaultIssue issue, Map<Component, List<Location>> locationsByComponent, DbIssues.Locations.Builder primaryLocationBuilder) {
-    if (SECURITY_HOTSPOT.equals(issue.type())) {
-      return false;
+  private static void addPrimaryLocation(Component component, Map<Component, List<Location>> locationsByComponent, DbIssues.Locations.Builder locationsBuilder) {
+    if (locationsBuilder.hasTextRange()) {
+      PrimaryLocation primaryLocation = new PrimaryLocation(locationsBuilder);
+      locationsByComponent.computeIfAbsent(component, c -> new LinkedList<>()).add(primaryLocation);
     }
+  }
 
-    List<DbIssues.Location.Builder> locationBuilders = primaryLocationBuilder.getFlowBuilderList().stream()
+  private void addSecondaryLocations(DefaultIssue issue, Map<Component, List<Location>> locationsByComponent, DbIssues.Locations.Builder locationsBuilder) {
+    List<DbIssues.Location.Builder> locationBuilders = locationsBuilder.getFlowBuilderList().stream()
       .flatMap(flowBuilder -> flowBuilder.getLocationBuilderList().stream())
       .filter(DbIssues.Location.Builder::hasTextRange)
       .toList();
 
     locationBuilders.forEach(locationBuilder -> addSecondaryLocation(locationBuilder, issue, locationsByComponent));
-
-    return !locationBuilders.isEmpty();
   }
 
   private void addSecondaryLocation(DbIssues.Location.Builder locationBuilder, DefaultIssue issue, Map<Component, List<Location>> locationsByComponent) {

diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComputeLocationHashesVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComputeLocationHashesVisitorTest.java
index e764a9d9b4e023ed04f018f3c809b784b794cf50..671e387fd77780f1f959004d92ac9c0064bc0dec 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComputeLocationHashesVisitorTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComputeLocationHashesVisitorTest.java
@@ -51,11 +51,17 @@ import static org.mockito.Mockito.when;
 
 public class ComputeLocationHashesVisitorTest {
   private static final String EXAMPLE_LINE_OF_CODE_FORMAT = "int example = line + of + code + %d; ";
+  private static final DbCommons.TextRange EXAMPLE_TEXT_RANGE = DbCommons.TextRange.newBuilder()
+    .setStartLine(1).setStartOffset(0)
+    .setEndLine(3).setEndOffset(EXAMPLE_LINE_OF_CODE_FORMAT.length() - 1)
+    .build();
   private static final String LINE_IN_THE_MAIN_FILE = "String string = 'line-in-the-main-file';";
   private static final String ANOTHER_LINE_IN_THE_MAIN_FILE = "String string = 'another-line-in-the-main-file';";
   private static final String LINE_IN_ANOTHER_FILE = "String string = 'line-in-the-another-file';";
+  private static final String CHECKSUM = "CHECKSUM";
 
-  private static final RuleKey RULE_KEY = RuleKey.of("javasecurity", "S001");
+  private static final RuleKey TAINTED_RULE_KEY = RuleKey.of("javasecurity", "key");
+  private static final RuleKey NOT_TAINTED_RULE_KEY = RuleKey.of("java", "key");
   private static final Component FILE_1 = ReportComponent.builder(Component.Type.FILE, 2).build();
   private static final Component FILE_2 = ReportComponent.builder(Component.Type.FILE, 3).build();
   private static final Component ROOT = ReportComponent.builder(Component.Type.PROJECT, 1)
@@ -80,89 +86,112 @@ public class ComputeLocationHashesVisitorTest {
   }
 
   @Test
-  public void do_nothing_if_issue_is_unchanged() {
-    DefaultIssue issue = createIssue()
+  public void beforeCaching_whenIssueUnchangedAndHasChecksum_shouldDoNothing() {
+    DefaultIssue notTaintedIssue = createNotTaintedIssue()
       .setLocationsChanged(false)
       .setNew(false)
-      .setLocations(DbIssues.Locations.newBuilder().setTextRange(createRange(1, 0, 3, EXAMPLE_LINE_OF_CODE_FORMAT.length() - 1)).build());
+      .setLocations(DbIssues.Locations.newBuilder()
+        .setChecksum(CHECKSUM)
+        .setTextRange(EXAMPLE_TEXT_RANGE)
+        .build());
 
-    underTest.beforeComponent(FILE_1);
-    underTest.onIssue(FILE_1, issue);
-    underTest.beforeCaching(FILE_1);
+    executeBeforeCaching(FILE_1, notTaintedIssue);
 
-    DbIssues.Locations locations = issue.getLocations();
-    assertThat(locations.getChecksum()).isEmpty();
+    DbIssues.Locations locations = notTaintedIssue.getLocations();
+    assertThat(locations.getChecksum()).isEqualTo(CHECKSUM);
     verifyNoInteractions(sourceLinesRepository);
   }
 
   @Test
-  public void do_nothing_if_issue_is_not_taint_vulnerability() {
-    DefaultIssue issue = createIssue()
-      .setRuleKey(RuleKey.of("repo", "rule"))
-      .setLocations(DbIssues.Locations.newBuilder().setTextRange(createRange(1, 0, 3, EXAMPLE_LINE_OF_CODE_FORMAT.length() - 1)).build());
+  public void beforeCaching_whenIssueHasNoLocation_shouldDoNothing() {
+    DefaultIssue notTaintedIssue = createNotTaintedIssue();
 
-    underTest.onIssue(FILE_1, issue);
-    underTest.beforeCaching(FILE_1);
+    executeBeforeCaching(FILE_1, notTaintedIssue);
 
-    DbIssues.Locations locations = issue.getLocations();
-    assertThat(locations.getChecksum()).isEmpty();
+    DbIssues.Locations locations = notTaintedIssue.getLocations();
+    assertThat(locations).isNull();
     verifyNoInteractions(sourceLinesRepository);
   }
 
   @Test
-  public void calculates_hash_for_multiple_lines() {
-    DefaultIssue issue = createIssue()
-      .setLocations(DbIssues.Locations.newBuilder().setTextRange(createRange(1, 0, 3, EXAMPLE_LINE_OF_CODE_FORMAT.length() - 1)).build());
+  public void beforeCaching_whenIssueIsExternal_shouldDoNothing() {
+    DefaultIssue notTaintedIssue = createNotTaintedIssue()
+      .setIsFromExternalRuleEngine(true)
+      .setLocations(DbIssues.Locations.newBuilder()
+        .setChecksum(CHECKSUM)
+        .setTextRange(EXAMPLE_TEXT_RANGE)
+        .build());;
 
-    underTest.onIssue(FILE_1, issue);
-    underTest.beforeCaching(FILE_1);
+    executeBeforeCaching(FILE_1, notTaintedIssue);
 
-    assertLocationHashIsMadeOf(issue, "intexample=line+of+code+1;intexample=line+of+code+2;intexample=line+of+code+3;");
+    DbIssues.Locations locations = notTaintedIssue.getLocations();
+    assertThat(locations.getChecksum()).isEqualTo(CHECKSUM);
+    verifyNoInteractions(sourceLinesRepository);
   }
 
   @Test
-  public void calculates_hash_for_multiple_files() {
-    DefaultIssue issue1 = createIssue()
-      .setLocations(DbIssues.Locations.newBuilder().setTextRange(createRange(1, 0, 3, EXAMPLE_LINE_OF_CODE_FORMAT.length() - 1)).build());
-    DefaultIssue issue2 = createIssue()
-      .setLocations(DbIssues.Locations.newBuilder().setTextRange(createRange(1, 0, 1, LINE_IN_ANOTHER_FILE.length())).build());
+  public void beforeCaching_whenIssueHasNoChecksum_shouldComputeChecksum() {
+    DefaultIssue notTaintedIssue = createNotTaintedIssue()
+      .setLocationsChanged(false)
+      .setNew(false)
+      .setLocations(DbIssues.Locations.newBuilder()
+        .setTextRange(EXAMPLE_TEXT_RANGE)
+        .build());
+
+    executeBeforeCaching(FILE_1, notTaintedIssue);
+
+    assertLocationHashIsMadeOf(notTaintedIssue, "intexample=line+of+code+1;intexample=line+of+code+2;intexample=line+of+code+3;");
+    verify(sourceLinesRepository).readLines(FILE_1);
+  }
+
+  @Test
+  public void beforeCaching_whenMultipleLinesTaintedIssue_shouldComputeChecksum() {
+    DefaultIssue taintedIssue = createTaintedIssue()
+      .setLocations(DbIssues.Locations.newBuilder().setTextRange(EXAMPLE_TEXT_RANGE).build());
+
+    executeBeforeCaching(FILE_1, taintedIssue);
 
-    underTest.onIssue(FILE_1, issue1);
-    underTest.beforeCaching(FILE_1);
+    assertLocationHashIsMadeOf(taintedIssue, "intexample=line+of+code+1;intexample=line+of+code+2;intexample=line+of+code+3;");
+  }
 
-    underTest.onIssue(FILE_2, issue2);
-    underTest.beforeCaching(FILE_2);
+  @Test
+  public void beforeCaching_whenMultipleTaintedIssuesAndMultipleComponents_shouldComputeAllChecksums() {
+    DefaultIssue taintedIssue1 = createTaintedIssue()
+      .setLocations(DbIssues.Locations.newBuilder().setTextRange(EXAMPLE_TEXT_RANGE).build());
+    DefaultIssue taintedIssue2 = createTaintedIssue()
+      .setLocations(DbIssues.Locations.newBuilder().setTextRange(createRange(1, 0, 1, LINE_IN_ANOTHER_FILE.length())).build());
 
-    assertLocationHashIsMadeOf(issue1, "intexample=line+of+code+1;intexample=line+of+code+2;intexample=line+of+code+3;");
-    assertLocationHashIsMadeOf(issue2, "Stringstring='line-in-the-another-file';");
+    executeBeforeCaching(FILE_1, taintedIssue1);
+    executeBeforeCaching(FILE_2, taintedIssue2);
+
+    assertLocationHashIsMadeOf(taintedIssue1, "intexample=line+of+code+1;intexample=line+of+code+2;intexample=line+of+code+3;");
+    assertLocationHashIsMadeOf(taintedIssue2, "Stringstring='line-in-the-another-file';");
   }
 
   @Test
-  public void calculates_hash_for_partial_line() {
-    DefaultIssue issue = createIssue()
+  public void beforeCaching_whenPartialLineTaintedIssue_shouldComputeChecksum() {
+    DefaultIssue taintedIssue = createTaintedIssue()
       .setLocations(DbIssues.Locations.newBuilder().setTextRange(createRange(1, 13, 1, EXAMPLE_LINE_OF_CODE_FORMAT.length() - 1)).build());
 
-    underTest.onIssue(FILE_1, issue);
-    underTest.beforeCaching(FILE_1);
+    executeBeforeCaching(FILE_1, taintedIssue);
 
-    assertLocationHashIsMadeOf(issue, "line+of+code+1;");
+    assertLocationHashIsMadeOf(taintedIssue, "line+of+code+1;");
   }
 
   @Test
-  public void calculates_hash_for_partial_multiple_lines() {
-    DefaultIssue issue = createIssue()
+  public void beforeCaching_whenPartialMultipleLinesTaintedIssue_shouldComputeChecksum() {
+    DefaultIssue taintedIssue = createTaintedIssue()
       .setLocations(DbIssues.Locations.newBuilder().setTextRange(createRange(1, 13, 3, 11)).build());
 
-    underTest.onIssue(FILE_1, issue);
-    underTest.beforeCaching(FILE_1);
+    executeBeforeCaching(FILE_1, taintedIssue);
 
-    assertLocationHashIsMadeOf(issue, "line+of+code+1;intexample=line+of+code+2;intexample");
+    assertLocationHashIsMadeOf(taintedIssue, "line+of+code+1;intexample=line+of+code+2;intexample");
   }
 
   @Test
-  public void dont_calculate_hash_if_no_textRange() {
+  public void beforeCaching_whenNoTextRange_shouldNotComputeChecksum() {
     // primary location and one of the secondary locations have no text range
-    DefaultIssue issue = createIssue()
+    DefaultIssue taintedIssue = createTaintedIssue()
       .setLocations(DbIssues.Locations.newBuilder()
         .addFlow(DbIssues.Flow.newBuilder()
           .addLocation(DbIssues.Location.newBuilder()
@@ -177,19 +206,18 @@ public class ComputeLocationHashesVisitorTest {
 
     when(sourceLinesRepository.readLines(FILE_1)).thenReturn(newOneLineIterator(LINE_IN_THE_MAIN_FILE));
 
-    underTest.onIssue(FILE_1, issue);
-    underTest.beforeCaching(FILE_1);
+    executeBeforeCaching(FILE_1, taintedIssue);
 
     verify(sourceLinesRepository).readLines(FILE_1);
     verifyNoMoreInteractions(sourceLinesRepository);
-    DbIssues.Locations locations = issue.getLocations();
+    DbIssues.Locations locations = taintedIssue.getLocations();
     assertThat(locations.getFlow(0).getLocation(0).getChecksum()).isEqualTo(DigestUtils.md5Hex("Stringstring='line-in-the-main-file';"));
     assertThat(locations.getFlow(0).getLocation(1).getChecksum()).isEmpty();
   }
 
   @Test
-  public void calculates_hash_for_multiple_locations() {
-    DefaultIssue issue = createIssue()
+  public void beforeCaching_whenMultipleLocationsInMultipleFiles_shouldComputeAllChecksums() {
+    DefaultIssue taintedIssue = createTaintedIssue()
       .setLocations(DbIssues.Locations.newBuilder()
         .setTextRange(createRange(1, 0, 1, LINE_IN_THE_MAIN_FILE.length()))
         .addFlow(DbIssues.Flow.newBuilder()
@@ -207,18 +235,17 @@ public class ComputeLocationHashesVisitorTest {
     when(sourceLinesRepository.readLines(FILE_1)).thenReturn(newOneLineIterator(LINE_IN_THE_MAIN_FILE));
     when(sourceLinesRepository.readLines(FILE_2)).thenReturn(newOneLineIterator(LINE_IN_ANOTHER_FILE));
 
-    underTest.onIssue(FILE_1, issue);
-    underTest.beforeCaching(FILE_1);
+    executeBeforeCaching(FILE_1, taintedIssue);
 
-    DbIssues.Locations locations = issue.getLocations();
+    DbIssues.Locations locations = taintedIssue.getLocations();
 
     assertThat(locations.getFlow(0).getLocation(0).getChecksum()).isEqualTo(DigestUtils.md5Hex("Stringstring='line-in-the-main-file';"));
     assertThat(locations.getFlow(0).getLocation(1).getChecksum()).isEqualTo(DigestUtils.md5Hex("Stringstring='line-in-the-another-file';"));
   }
 
   @Test
-  public void calculates_hash_for_multiple_locations_in_same_file() {
-    DefaultIssue issue = createIssue()
+  public void beforeCaching_whenMultipleLocationsInSameFile_shouldComputeAllChecksums() {
+    DefaultIssue taintedIssue = createTaintedIssue()
       .setComponentUuid(FILE_1.getUuid())
       .setLocations(DbIssues.Locations.newBuilder()
         .setTextRange(createRange(1, 0, 1, LINE_IN_THE_MAIN_FILE.length()))
@@ -236,32 +263,17 @@ public class ComputeLocationHashesVisitorTest {
 
     when(sourceLinesRepository.readLines(FILE_1)).thenReturn(manyLinesIterator(LINE_IN_THE_MAIN_FILE, ANOTHER_LINE_IN_THE_MAIN_FILE));
 
-    underTest.onIssue(FILE_1, issue);
-    underTest.beforeCaching(FILE_1);
+    executeBeforeCaching(FILE_1, taintedIssue);
 
-    DbIssues.Locations locations = issue.getLocations();
+    DbIssues.Locations locations = taintedIssue.getLocations();
 
     assertThat(locations.getFlow(0).getLocation(0).getChecksum()).isEqualTo(DigestUtils.md5Hex("Stringstring='line-in-the-main-file';"));
     assertThat(locations.getFlow(0).getLocation(1).getChecksum()).isEqualTo(DigestUtils.md5Hex("Stringstring='another-line-in-the-main-file';"));
   }
 
   @Test
-  public void beforeCaching_whenSecurityHotspots_shouldCalculateHashForPrimaryLocation() {
-    DefaultIssue issue = createIssue()
-      .setRuleKey(RuleKey.of("repo", "rule"))
-      .setType(RuleType.SECURITY_HOTSPOT)
-      .setLocations(DbIssues.Locations.newBuilder().setTextRange(createRange(1, 0, 3, EXAMPLE_LINE_OF_CODE_FORMAT.length() - 1)).build());
-    underTest.onIssue(FILE_1, issue);
-
-    underTest.beforeCaching(FILE_1);
-
-    assertLocationHashIsMadeOf(issue, "intexample=line+of+code+1;intexample=line+of+code+2;intexample=line+of+code+3;");
-  }
-
-  @Test
-  public void beforeCaching_whenSecurityHotspots_shouldNotCalculateHashForSecondaryLocations() {
-    DefaultIssue issue = createIssue()
-      .setType(RuleType.SECURITY_HOTSPOT)
+  public void beforeCaching_whenNotTaintedIssue_shouldNotComputeChecksumForSecondaryLocations() {
+    DefaultIssue notTaintedIssue = createNotTaintedIssue()
       .setLocations(DbIssues.Locations.newBuilder()
         .setTextRange(createRange(1, 0, 1, LINE_IN_THE_MAIN_FILE.length()))
         .addFlow(DbIssues.Flow.newBuilder()
@@ -277,16 +289,21 @@ public class ComputeLocationHashesVisitorTest {
         .build());
     when(sourceLinesRepository.readLines(FILE_1)).thenReturn(newOneLineIterator(LINE_IN_THE_MAIN_FILE));
     when(sourceLinesRepository.readLines(FILE_2)).thenReturn(newOneLineIterator(LINE_IN_ANOTHER_FILE));
-    underTest.onIssue(FILE_1, issue);
 
-    underTest.beforeCaching(FILE_1);
+    executeBeforeCaching(FILE_1, notTaintedIssue);
 
-    DbIssues.Locations locations = issue.getLocations();
-    assertLocationHashIsMadeOf(issue, "Stringstring='line-in-the-main-file';");
+    DbIssues.Locations locations = notTaintedIssue.getLocations();
+    assertLocationHashIsMadeOf(notTaintedIssue, "Stringstring='line-in-the-main-file';");
     assertThat(locations.getFlow(0).getLocation(0).getChecksum()).isEmpty();
     assertThat(locations.getFlow(0).getLocation(1).getChecksum()).isEmpty();
   }
 
+  private void executeBeforeCaching(Component component, DefaultIssue issue) {
+    underTest.beforeComponent(component);
+    underTest.onIssue(component, issue);
+    underTest.beforeCaching(component);
+  }
+
   private DbCommons.TextRange createRange(int startLine, int startOffset, int endLine, int endOffset) {
     return DbCommons.TextRange.newBuilder()
       .setStartLine(startLine).setStartOffset(startOffset)
@@ -294,10 +311,18 @@ public class ComputeLocationHashesVisitorTest {
       .build();
   }
 
-  private DefaultIssue createIssue() {
+  private DefaultIssue createTaintedIssue() {
+    return createIssue(TAINTED_RULE_KEY);
+  }
+
+  private DefaultIssue createNotTaintedIssue() {
+    return createIssue(NOT_TAINTED_RULE_KEY);
+  }
+
+  private DefaultIssue createIssue(RuleKey ruleKey) {
     return new DefaultIssue()
       .setLocationsChanged(true)
-      .setRuleKey(RULE_KEY)
+      .setRuleKey(ruleKey)
       .setIsFromExternalRuleEngine(false)
       .setType(RuleType.CODE_SMELL);
   }