From: Simon Brandhof Date: Thu, 9 Aug 2018 09:33:02 +0000 (+0200) Subject: SONAR-9904 extract traversal of issue changesets in Changeset X-Git-Tag: 7.5~593 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=1dd2b87f1c156bbfbb630f2aa014f90b86559ee3;p=sonarqube.git SONAR-9904 extract traversal of issue changesets in Changeset --- diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCreationDateCalculator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCreationDateCalculator.java index 64816ce6b2a..83cc898e07e 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCreationDateCalculator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueCreationDateCalculator.java @@ -21,22 +21,11 @@ package org.sonar.ce.task.projectanalysis.issue; import java.util.Comparator; import java.util.Date; -import java.util.HashSet; -import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.function.Supplier; -import java.util.stream.IntStream; import javax.annotation.Nullable; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.DateUtils; -import org.sonar.ce.task.projectanalysis.component.Component; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.IssueChangeContext; -import org.sonar.db.protobuf.DbCommons.TextRange; -import org.sonar.db.protobuf.DbIssues; -import org.sonar.db.protobuf.DbIssues.Flow; -import org.sonar.db.protobuf.DbIssues.Location; import org.sonar.ce.task.projectanalysis.analysis.Analysis; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.analysis.ScannerPlugin; @@ -46,6 +35,8 @@ import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolder; import org.sonar.ce.task.projectanalysis.scm.Changeset; import org.sonar.ce.task.projectanalysis.scm.ScmInfo; import org.sonar.ce.task.projectanalysis.scm.ScmInfoRepository; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.IssueChangeContext; import org.sonar.server.issue.IssueFieldsSetter; import static org.sonar.core.issue.IssueChangeContext.createScan; @@ -84,13 +75,13 @@ public class IssueCreationDateCalculator extends IssueVisitor { .orElseThrow(illegalStateException("The rule with key '%s' raised an issue, but no rule with that key was found", issue.getRuleKey())); if (rule.isExternal()) { - getScmChangeDate(component, issue).ifPresent(changeDate -> updateDate(issue, changeDate)); + getDateOfLatestChange(component, issue).ifPresent(changeDate -> updateDate(issue, changeDate)); } else { // Rule can't be inactive (see contract of IssueVisitor) ActiveRule activeRule = activeRulesHolder.get(issue.getRuleKey()).get(); if (firstAnalysis || activeRuleIsNew(activeRule, lastAnalysisOptional.get()) || ruleImplementationChanged(activeRule.getRuleKey(), activeRule.getPluginKey(), lastAnalysisOptional.get())) { - getScmChangeDate(component, issue).ifPresent(changeDate -> updateDate(issue, changeDate)); + getDateOfLatestChange(component, issue).ifPresent(changeDate -> updateDate(issue, changeDate)); } } } @@ -124,9 +115,9 @@ public class IssueCreationDateCalculator extends IssueVisitor { return lastAnalysisDate < ruleCreationDate; } - private Optional getScmChangeDate(Component component, DefaultIssue issue) { + private Optional getDateOfLatestChange(Component component, DefaultIssue issue) { return getScmInfo(component) - .flatMap(scmInfo -> getChangeset(component, scmInfo, issue)) + .flatMap(scmInfo -> getLatestChangeset(component, scmInfo, issue)) .map(IssueCreationDateCalculator::getChangeDate); } @@ -138,36 +129,17 @@ public class IssueCreationDateCalculator extends IssueVisitor { return scmInfoRepository.getScmInfo(component); } - private static Optional getChangeset(Component component, ScmInfo scmInfo, DefaultIssue issue) { - Set involvedLines = new HashSet<>(); - DbIssues.Locations locations = issue.getLocations(); - if (locations != null) { - if (locations.hasTextRange()) { - addLines(involvedLines, locations.getTextRange()); - } - for (Flow f : locations.getFlowList()) { - for (Location l : f.getLocationList()) { - if (Objects.equals(l.getComponentId(), component.getUuid())) { - // Ignore locations in other files, since it is currently not very common, and this is hard to load SCM by component UUID. - addLines(involvedLines, l.getTextRange()); - } - } - } - if (!involvedLines.isEmpty()) { - return involvedLines.stream() - .filter(scmInfo::hasChangesetForLine) - .map(scmInfo::getChangesetForLine) - .max(Comparator.comparingLong(Changeset::getDate)); - } + private static Optional getLatestChangeset(Component component, ScmInfo scmInfo, DefaultIssue issue) { + Optional mostRecentChangeset = IssueLocations.allLinesFor(issue, component.getUuid()) + .filter(scmInfo::hasChangesetForLine) + .mapToObj(scmInfo::getChangesetForLine) + .max(Comparator.comparingLong(Changeset::getDate)); + if (mostRecentChangeset.isPresent()) { + return mostRecentChangeset; } - return Optional.of(scmInfo.getLatestChangeset()); } - private static void addLines(Set involvedLines, TextRange range) { - IntStream.rangeClosed(range.getStartLine(), range.getEndLine()).forEach(involvedLines::add); - } - private static Date getChangeDate(Changeset changesetForLine) { return DateUtils.longToDate(changesetForLine.getDate()); } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLocations.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLocations.java new file mode 100644 index 00000000000..ed356f02427 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLocations.java @@ -0,0 +1,57 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.task.projectanalysis.issue; + +import java.util.Objects; +import java.util.stream.IntStream; +import java.util.stream.Stream; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.db.protobuf.DbCommons; +import org.sonar.db.protobuf.DbIssues; + +class IssueLocations { + + private IssueLocations() { + // do not instantiate + } + + /** + * Extract the lines of all the locations in the specified component. All the flows and secondary locations + * are taken into account. The lines present in multiple flows and locations are kept + * duplicated. Ordering of results is not guaranteed. + *

+ * TODO should be a method of DefaultIssue, as soon as it's no + * longer in sonar-core and can access sonar-db-dao. + */ + public static IntStream allLinesFor(DefaultIssue issue, String componentId) { + DbIssues.Locations locations = issue.getLocations(); + if (locations == null) { + return IntStream.empty(); + } + + Stream textRanges = Stream.concat( + locations.hasTextRange() ? Stream.of(locations.getTextRange()) : Stream.empty(), + locations.getFlowList().stream() + .flatMap(f -> f.getLocationList().stream()) + .filter(l -> Objects.equals(l.getComponentId(), componentId)) + .map(DbIssues.Location::getTextRange)); + return textRanges.flatMapToInt(range -> IntStream.rangeClosed(range.getStartLine(), range.getEndLine())); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLocationsTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLocationsTest.java new file mode 100644 index 00000000000..4d23cf46457 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLocationsTest.java @@ -0,0 +1,89 @@ +/* + * SonarQube + * Copyright (C) 2009-2018 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.ce.task.projectanalysis.issue; + +import org.junit.Test; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.db.protobuf.DbCommons; +import org.sonar.db.protobuf.DbIssues; + +import static org.assertj.core.api.Assertions.assertThat; + +public class IssueLocationsTest { + + @Test + public void allLinesFor_filters_lines_for_specified_component() { + DbIssues.Locations.Builder locations = DbIssues.Locations.newBuilder(); + locations.addFlowBuilder() + .addLocation(newLocation("file1", 5, 5)) + .addLocation(newLocation("file1", 10, 12)) + .addLocation(newLocation("file1", 15, 15)) + .addLocation(newLocation("file2", 10, 11)) + .build(); + DefaultIssue issue = new DefaultIssue().setLocations(locations.build()); + + assertThat(IssueLocations.allLinesFor(issue, "file1")).containsExactlyInAnyOrder(5, 10, 11, 12, 15); + assertThat(IssueLocations.allLinesFor(issue, "file2")).containsExactlyInAnyOrder(10, 11); + assertThat(IssueLocations.allLinesFor(issue, "file3")).isEmpty(); + } + + @Test + public void allLinesFor_traverses_all_flows() { + DbIssues.Locations.Builder locations = DbIssues.Locations.newBuilder(); + locations.addFlowBuilder() + .addLocation(newLocation("file1", 5, 5)) + .addLocation(newLocation("file2", 10, 11)) + .build(); + locations.addFlowBuilder() + .addLocation(newLocation("file1", 7, 9)) + .addLocation(newLocation("file2", 12, 12)) + .build(); + DefaultIssue issue = new DefaultIssue().setLocations(locations.build()); + + assertThat(IssueLocations.allLinesFor(issue, "file1")).containsExactlyInAnyOrder(5, 7, 8, 9); + assertThat(IssueLocations.allLinesFor(issue, "file2")).containsExactlyInAnyOrder(10, 11, 12); + } + + @Test + public void allLinesFor_keeps_duplicated_lines() { + DbIssues.Locations.Builder locations = DbIssues.Locations.newBuilder(); + locations.addFlowBuilder() + .addLocation(newLocation("file1", 5, 5)) + .addLocation(newLocation("file1", 4, 6)) + .build(); + DefaultIssue issue = new DefaultIssue().setLocations(locations.build()); + + assertThat(IssueLocations.allLinesFor(issue, "file1")).containsExactlyInAnyOrder(4, 5, 5, 6); + } + + @Test + public void allLinesFor_returns_empty_if_no_locations_are_set() { + DefaultIssue issue = new DefaultIssue().setLocations(null); + + assertThat(IssueLocations.allLinesFor(issue, "file1")).isEmpty(); + } + + private static DbIssues.Location newLocation(String componentId, int startLine, int endLine) { + return DbIssues.Location.newBuilder() + .setComponentId(componentId) + .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(startLine).setEndLine(endLine).build()) + .build(); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/scm/ChangesetTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/scm/ChangesetTest.java index 9137f911e25..70bbca1435c 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/scm/ChangesetTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/scm/ChangesetTest.java @@ -28,7 +28,7 @@ import static org.assertj.core.api.Assertions.assertThat; public class ChangesetTest { @Rule - public ExpectedException thrown = ExpectedException.none(); + public ExpectedException expectedException = ExpectedException.none(); @Test public void create_changeset() { @@ -55,17 +55,17 @@ public class ChangesetTest { } @Test - public void fail_with_NPE_when_setting_null_date() throws Exception { - thrown.expect(NullPointerException.class); - thrown.expectMessage("Date cannot be null"); + public void fail_with_NPE_when_setting_null_date() { + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("Date cannot be null"); Changeset.newChangesetBuilder().setDate(null); } @Test public void fail_with_NPE_when_building_without_date() { - thrown.expect(NullPointerException.class); - thrown.expectMessage("Date cannot be null"); + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("Date cannot be null"); Changeset.newChangesetBuilder() .setAuthor("john") @@ -85,7 +85,7 @@ public class ChangesetTest { } @Test - public void equals_and_hashcode_are_based_on_all_fields() throws Exception { + public void equals_and_hashcode_are_based_on_all_fields() { Changeset.Builder changesetBuilder = Changeset.newChangesetBuilder() .setAuthor("john") .setDate(123456789L)