SONAR-10781 Filter secondary locations that are not in PR

Filter secondary locations that are not in the PR/short living branch

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolder.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolder.java
index 03aafd860f1020144998230aa14cffc22b6fa0d2..71ec3574017a1d64059afc5c7eb9da46c8123534 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolder.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolder.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.server.computation.task.projectanalysis.component;
 
+import java.util.Optional;
+
 /**
  * The tree of components defined in the scanner report.
  */
@@ -39,4 +41,16 @@ public interface TreeRootHolder {
    */
   Component getComponentByRef(int ref);
 
+
+  /**
+   * Return a component by its batch reference. Returns {@link Optional#empty()} if there's
+   * no {@link Component} with the specified reference
+   *
+   * @throws IllegalStateException if the holder is empty (ie. there is no root yet)
+   * @deprecated This method was introduced as a quick fix for SONAR-10781. Ultimately one should never manipulate component
+   * ref that doesn't exists in the scanner report
+   */
+  @Deprecated
+  Optional<Component> getOptionalComponentByRef(int ref);
+
 }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderImpl.java
index 3e76fcfa5dab6ca0f73437649b1c75b4b701ad2a..3d41f37a9f924d8bbacf2954e9d95d6665a8e7be 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderImpl.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderImpl.java
@@ -21,9 +21,9 @@ package org.sonar.server.computation.task.projectanalysis.component;
 
 import com.google.common.collect.ImmutableMap;
 import java.util.Map;
+import java.util.Optional;
 import javax.annotation.CheckForNull;
 
-import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
 import static java.util.Objects.requireNonNull;
 import static org.sonar.server.computation.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER;
@@ -52,11 +52,15 @@ public class TreeRootHolderImpl implements MutableTreeRootHolder {
 
   @Override
   public Component getComponentByRef(int ref) {
+    return getOptionalComponentByRef(ref)
+      .orElseThrow(() -> new IllegalArgumentException(String.format("Component with ref '%s' can't be found", ref)));
+  }
+
+  @Override
+  public Optional<Component> getOptionalComponentByRef(int ref) {
     checkInitialized();
     ensureComponentByRefIsPopulated();
-    Component component = componentsByRef.get(ref);
-    checkArgument(component != null, "Component with ref '%s' can't be found", ref);
-    return component;
+    return Optional.ofNullable(componentsByRef.get(ref));
   }
 
   private void ensureComponentByRefIsPopulated() {

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactory.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactory.java
index 84a1cab92deda5ebcc894127629d7db1dbd923bc..cb5b312638d05904ed3945b5ba8163ae1b73bdae 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactory.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactory.java
@@ -22,6 +22,7 @@ package org.sonar.server.computation.task.projectanalysis.issue;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Optional;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.RuleType;
@@ -169,7 +170,7 @@ public class TrackerRawInputFactory {
         if (flow.getLocationCount() > 0) {
           DbIssues.Flow.Builder dbFlowBuilder = DbIssues.Flow.newBuilder();
           for (ScannerReport.IssueLocation location : flow.getLocationList()) {
-            dbFlowBuilder.addLocation(convertLocation(location));
+            convertLocation(location).ifPresent(dbFlowBuilder::addLocation);
           }
           dbLocationsBuilder.addFlow(dbFlowBuilder);
         }
@@ -206,7 +207,7 @@ public class TrackerRawInputFactory {
         if (flow.getLocationCount() > 0) {
           DbIssues.Flow.Builder dbFlowBuilder = DbIssues.Flow.newBuilder();
           for (ScannerReport.IssueLocation location : flow.getLocationList()) {
-            dbFlowBuilder.addLocation(convertLocation(location));
+            convertLocation(location).ifPresent(dbFlowBuilder::addLocation);
           }
           dbLocationsBuilder.addFlow(dbFlowBuilder);
         }
@@ -251,10 +252,15 @@ public class TrackerRawInputFactory {
       return issue;
     }
 
-    private DbIssues.Location convertLocation(ScannerReport.IssueLocation source) {
+    private Optional<DbIssues.Location> convertLocation(ScannerReport.IssueLocation source) {
       DbIssues.Location.Builder target = DbIssues.Location.newBuilder();
       if (source.getComponentRef() != 0 && source.getComponentRef() != component.getReportAttributes().getRef()) {
-        target.setComponentId(treeRootHolder.getComponentByRef(source.getComponentRef()).getUuid());
+        // SONAR-10781 Component might not exist because on short living branch and PR, only changed components are included in the report
+        Optional<Component> optionalComponent = treeRootHolder.getOptionalComponentByRef(source.getComponentRef());
+        if (!optionalComponent.isPresent()) {
+          return Optional.empty();
+        }
+        target.setComponentId(optionalComponent.get().getUuid());
       }
       if (isNotEmpty(source.getMsg())) {
         target.setMsg(source.getMsg());
@@ -264,7 +270,7 @@ public class TrackerRawInputFactory {
         DbCommons.TextRange.Builder targetRange = convertTextRange(sourceRange);
         target.setTextRange(targetRange);
       }
-      return target.build();
+      return Optional.of(target.build());
     }
 
     private DbCommons.TextRange.Builder convertTextRange(ScannerReport.TextRange sourceRange) {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderImplTest.java
index 085c1e8e7af4ba9e150f4be3d70be8bbdc4d04b9..2edd2ec1ebbc54897c8ed515687872d54798785a 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderImplTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderImplTest.java
@@ -99,6 +99,15 @@ public class TreeRootHolderImplTest {
     }
   }
 
+  @Test
+  public void getOptionalComponentByRef_returns_any_report_component_in_the_tree() {
+    underTest.setRoot(SOME_REPORT_COMPONENT_TREE);
+
+    for (int i = 1; i <= 4; i++) {
+      assertThat(underTest.getOptionalComponentByRef(i).get().getReportAttributes().getRef()).isEqualTo(i);
+    }
+  }
+
   @Test
   public void getComponentByRef_throws_IAE_if_holder_does_not_contain_specified_component() {
     underTest.setRoot(SOME_REPORT_COMPONENT_TREE);
@@ -109,6 +118,13 @@ public class TreeRootHolderImplTest {
     underTest.getComponentByRef(6);
   }
 
+  @Test
+  public void getOptionalComponentByRef_returns_empty_if_holder_does_not_contain_specified_component() {
+    underTest.setRoot(SOME_REPORT_COMPONENT_TREE);
+
+    assertThat(underTest.getOptionalComponentByRef(6)).isEmpty();
+  }
+
   @Test
   public void getComponentByRef_throws_IAE_if_holder_contains_View_tree() {
     underTest.setRoot(SOME_VIEWS_COMPONENT_TREE);

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderRule.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderRule.java
index 9656c73e48d2b4c00e033b71fdccf388d15dbf1b..edcf51342ec53284e1ce321a0554b8a75455f945 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderRule.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/component/TreeRootHolderRule.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.server.computation.task.projectanalysis.component;
 
+import java.util.Optional;
 import org.junit.rules.ExternalResource;
 
 public class TreeRootHolderRule extends ExternalResource implements TreeRootHolder {
@@ -44,4 +45,9 @@ public class TreeRootHolderRule extends ExternalResource implements TreeRootHold
   public Component getComponentByRef(int ref) {
     return delegate.getComponentByRef(ref);
   }
+
+  @Override
+  public Optional<Component> getOptionalComponentByRef(int ref) {
+    return delegate.getOptionalComponentByRef(ref);
+  }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java
index 3da31e3a7c2e1103ff7576783da76aa23abf44b0..6640bd2c2adf422d78591a22acfd24eba2ccf98f 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/issue/TrackerRawInputFactoryTest.java
@@ -30,6 +30,7 @@ import org.sonar.api.rule.Severity;
 import org.sonar.api.utils.Duration;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.tracking.Input;
+import org.sonar.db.protobuf.DbIssues;
 import org.sonar.scanner.protocol.Constants;
 import org.sonar.scanner.protocol.output.ScannerReport;
 import org.sonar.scanner.protocol.output.ScannerReport.TextRange;
@@ -55,10 +56,15 @@ import static org.mockito.Mockito.when;
 
 public class TrackerRawInputFactoryTest {
 
+  private static final String FILE_UUID = "fake_uuid";
+  private static final String ANOTHER_FILE_UUID = "another_fake_uuid";
   private static int FILE_REF = 2;
+  private static int NOT_IN_REPORT_FILE_REF = 3;
+  private static int ANOTHER_FILE_REF = 4;
 
-  private static ReportComponent PROJECT = ReportComponent.builder(Component.Type.PROJECT, 1).build();
-  private static ReportComponent FILE = ReportComponent.builder(Component.Type.FILE, FILE_REF).build();
+  private static ReportComponent FILE = ReportComponent.builder(Component.Type.FILE, FILE_REF).setUuid(FILE_UUID).build();
+  private static ReportComponent ANOTHER_FILE = ReportComponent.builder(Component.Type.FILE, ANOTHER_FILE_REF).setUuid(ANOTHER_FILE_UUID).build();
+  private static ReportComponent PROJECT = ReportComponent.builder(Component.Type.PROJECT, 1).addChildren(FILE, ANOTHER_FILE).build();
 
   @Rule
   public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule().setRoot(PROJECT);
@@ -133,6 +139,52 @@ public class TrackerRawInputFactoryTest {
     assertThat(issue.debt()).isNull();
   }
 
+  // SONAR-10781
+  @Test
+  public void load_issues_from_report_missing_secondary_location_component() {
+    RuleKey ruleKey = RuleKey.of("java", "S001");
+    markRuleAsActive(ruleKey);
+    when(issueFilter.accept(any(), eq(FILE))).thenReturn(true);
+
+    when(sourceLinesHash.getLineHashesMatchingDBVersion(FILE)).thenReturn(Collections.singletonList("line"));
+    ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder()
+      .setTextRange(TextRange.newBuilder().setStartLine(2).build())
+      .setMsg("the message")
+      .setRuleRepository(ruleKey.repository())
+      .setRuleKey(ruleKey.rule())
+      .setSeverity(Constants.Severity.BLOCKER)
+      .setGap(3.14)
+      .addFlow(ScannerReport.Flow.newBuilder()
+        .addLocation(ScannerReport.IssueLocation.newBuilder()
+          .setComponentRef(FILE_REF)
+          .setMsg("Secondary location in same file")
+          .setTextRange(TextRange.newBuilder().setStartLine(2).build()))
+        .addLocation(ScannerReport.IssueLocation.newBuilder()
+          .setComponentRef(NOT_IN_REPORT_FILE_REF)
+          .setMsg("Secondary location in a missing file")
+          .setTextRange(TextRange.newBuilder().setStartLine(3).build()))
+        .addLocation(ScannerReport.IssueLocation.newBuilder()
+          .setComponentRef(ANOTHER_FILE_REF)
+          .setMsg("Secondary location in another file")
+          .setTextRange(TextRange.newBuilder().setStartLine(3).build()))
+        .build())
+      .build();
+    reportReader.putIssues(FILE.getReportAttributes().getRef(), singletonList(reportIssue));
+    Input<DefaultIssue> input = underTest.create(FILE);
+
+    Collection<DefaultIssue> issues = input.getIssues();
+    assertThat(issues).hasSize(1);
+    DefaultIssue issue = Iterators.getOnlyElement(issues.iterator());
+
+    DbIssues.Locations locations = issue.getLocations();
+    // fields set by analysis report
+    assertThat(locations.getFlowList()).hasSize(1);
+    assertThat(locations.getFlow(0).getLocationList()).hasSize(2);
+    // Not component id if location is in the same file
+    assertThat(locations.getFlow(0).getLocation(0).getComponentId()).isEmpty();
+    assertThat(locations.getFlow(0).getLocation(1).getComponentId()).isEqualTo(ANOTHER_FILE_UUID);
+  }
+
   @Test
   public void load_external_issues_from_report() {
     when(sourceLinesHash.getLineHashesMatchingDBVersion(FILE)).thenReturn(Collections.singletonList("line"));