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 03aafd860f10..71ec3574017a 100644 --- 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 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 3e76fcfa5dab..3d41f37a9f92 100644 --- 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 @@ 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 Component getRoot() { @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 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 84a1cab92ded..cb5b312638d0 100644 --- 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 @@ 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 @@ private DefaultIssue toIssue(LineHashSequence lineHashSeq, ScannerReport.Issue r 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 @@ private DefaultIssue toExternalIssue(LineHashSequence lineHashSeq, ScannerReport 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 @@ private DefaultIssue init(DefaultIssue issue) { return issue; } - private DbIssues.Location convertLocation(ScannerReport.IssueLocation source) { + private Optional 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 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 @@ private DbIssues.Location convertLocation(ScannerReport.IssueLocation source) { 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 085c1e8e7af4..2edd2ec1ebbc 100644 --- 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 void getComponentByRef_returns_any_report_component_in_the_tree() { } } + @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 void getComponentByRef_throws_IAE_if_holder_does_not_contain_specified_co 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 9656c73e48d2..edcf51342ec5 100644 --- 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 Component getRoot() { public Component getComponentByRef(int ref) { return delegate.getComponentByRef(ref); } + + @Override + public Optional 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 3da31e3a7c2e..6640bd2c2adf 100644 --- 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.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 @@ 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 void load_issues_from_report() { 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 input = underTest.create(FILE); + + Collection 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"));