From 3065f6824c72d504c8e317c7d2d6a2c682081c4f Mon Sep 17 00:00:00 2001 From: Sébastien Lesaint Date: Tue, 14 Aug 2018 18:07:10 +0200 Subject: SONAR-8368 reopen closed issues (restore status) but those from Hotspots rules and manual vulnerabilities --- .../ProjectAnalysisTaskContainerPopulator.java | 2 + .../projectanalysis/issue/BaseInputFactory.java | 66 +++++ .../CloseIssuesOnRemovedComponentsVisitor.java | 2 +- .../issue/ClosedIssuesInputFactory.java | 56 ++++ .../issue/ComponentIssuesLoader.java | 108 ++++++-- .../task/projectanalysis/issue/IssueLifecycle.java | 19 +- .../issue/MergeBranchTrackerExecution.java | 3 +- .../issue/ShortBranchIssuesLoader.java | 26 +- .../issue/ShortBranchTrackerExecution.java | 4 +- .../issue/TrackerBaseInputFactory.java | 42 +-- .../projectanalysis/issue/TrackerExecution.java | 40 ++- .../issue/TrackerMergeBranchInputFactory.java | 4 +- .../CloseIssuesOnRemovedComponentsVisitorTest.java | 2 +- .../issue/ClosedIssuesInputFactoryTest.java | 100 +++++++ .../issue/ComponentIssuesLoaderTest.java | 85 ++++++ .../issue/IntegrateIssuesVisitorTest.java | 13 +- .../projectanalysis/issue/IssueLifecycleTest.java | 10 +- .../issue/MergeBranchTrackerExecutionTest.java | 6 +- .../issue/ShortBranchIssueMergerTest.java | 4 +- .../issue/TrackerBaseInputFactoryTest.java | 6 +- .../issue/TrackerExecutionTest.java | 122 +++++++++ .../src/main/java/org/sonar/db/issue/IssueDto.java | 7 + .../main/java/org/sonar/db/issue/IssueMapper.java | 4 +- .../resources/org/sonar/db/issue/IssueMapper.xml | 24 ++ .../java/org/sonar/db/issue/IssueMapperTest.java | 291 ++++++++++++++++++++- .../sonar/server/issue/workflow/IssueWorkflow.java | 39 ++- .../server/issue/workflow/PreviousStatusWas.java | 50 ++++ .../server/issue/workflow/IssueWorkflowTest.java | 155 +++++++++++ .../java/org/sonar/core/issue/DefaultIssue.java | 9 +- .../main/java/org/sonar/core/issue/FieldDiffs.java | 19 ++ .../sonar/core/issue/tracking/AbstractTracker.java | 36 ++- .../issue/tracking/FilteringBaseInputWrapper.java | 54 ++++ .../core/issue/tracking/NonClosedTracking.java | 46 ++++ .../org/sonar/core/issue/tracking/Tracker.java | 52 +++- .../org/sonar/core/issue/tracking/Tracking.java | 16 +- .../org/sonar/core/issue/tracking/TrackerTest.java | 32 +-- .../scanner/issue/tracking/LocalIssueTracking.java | 2 +- 37 files changed, 1413 insertions(+), 143 deletions(-) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/BaseInputFactory.java create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ClosedIssuesInputFactory.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ClosedIssuesInputFactoryTest.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoaderTest.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerExecutionTest.java create mode 100644 server/sonar-server-common/src/main/java/org/sonar/server/issue/workflow/PreviousStatusWas.java create mode 100644 sonar-core/src/main/java/org/sonar/core/issue/tracking/FilteringBaseInputWrapper.java create mode 100644 sonar-core/src/main/java/org/sonar/core/issue/tracking/NonClosedTracking.java diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java index 22346b5606e..55b2be91c70 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java @@ -49,6 +49,7 @@ import org.sonar.ce.task.projectanalysis.filemove.SourceSimilarityImpl; import org.sonar.ce.task.projectanalysis.filesystem.ComputationTempFolderProvider; import org.sonar.ce.task.projectanalysis.issue.BaseIssuesLoader; import org.sonar.ce.task.projectanalysis.issue.CloseIssuesOnRemovedComponentsVisitor; +import org.sonar.ce.task.projectanalysis.issue.ClosedIssuesInputFactory; import org.sonar.ce.task.projectanalysis.issue.ComponentIssuesLoader; import org.sonar.ce.task.projectanalysis.issue.ComponentIssuesRepositoryImpl; import org.sonar.ce.task.projectanalysis.issue.ComponentsWithUnprocessedIssues; @@ -254,6 +255,7 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop TrackerBaseInputFactory.class, TrackerRawInputFactory.class, TrackerMergeBranchInputFactory.class, + ClosedIssuesInputFactory.class, Tracker.class, TrackerExecution.class, ShortBranchTrackerExecution.class, diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/BaseInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/BaseInputFactory.java new file mode 100644 index 00000000000..7b79d01dc53 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/BaseInputFactory.java @@ -0,0 +1,66 @@ +/* + * 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.Collections; +import java.util.List; +import javax.annotation.Nullable; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.core.issue.tracking.LazyInput; +import org.sonar.core.issue.tracking.LineHashSequence; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; + +public abstract class BaseInputFactory { + private static final LineHashSequence EMPTY_LINE_HASH_SEQUENCE = new LineHashSequence(Collections.emptyList()); + + abstract Input create(Component component); + + abstract static class BaseLazyInput extends LazyInput { + private final DbClient dbClient; + final Component component; + final String effectiveUuid; + + BaseLazyInput(DbClient dbClient, Component component, @Nullable MovedFilesRepository.OriginalFile originalFile) { + this.dbClient = dbClient; + this.component = component; + this.effectiveUuid = originalFile == null ? component.getUuid() : originalFile.getUuid(); + } + + @Override + protected LineHashSequence loadLineHashSequence() { + if (component.getType() != Component.Type.FILE) { + return EMPTY_LINE_HASH_SEQUENCE; + } + + try (DbSession session = dbClient.openSession(false)) { + List hashes = dbClient.fileSourceDao().selectLineHashes(session, effectiveUuid); + if (hashes == null || hashes.isEmpty()) { + return EMPTY_LINE_HASH_SEQUENCE; + } + return new LineHashSequence(hashes); + } + } + + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitor.java index 88649984193..506c2aedece 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitor.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitor.java @@ -57,7 +57,7 @@ public class CloseIssuesOnRemovedComponentsVisitor extends TypeAwareVisitorAdapt DiskCache.DiskAppender cacheAppender = issueCache.newAppender(); try { for (String deletedComponentUuid : deletedComponentUuids) { - List issues = issuesLoader.loadForComponentUuid(deletedComponentUuid); + List issues = issuesLoader.loadOpenIssues(deletedComponentUuid); for (DefaultIssue issue : issues) { issue.setBeingClosed(true); // TODO should be renamed diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ClosedIssuesInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ClosedIssuesInputFactory.java new file mode 100644 index 00000000000..d6ad6ea61b9 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ClosedIssuesInputFactory.java @@ -0,0 +1,56 @@ +/* + * 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.List; +import javax.annotation.Nullable; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.db.DbClient; + +public class ClosedIssuesInputFactory extends BaseInputFactory { + private final ComponentIssuesLoader issuesLoader; + private final DbClient dbClient; + private final MovedFilesRepository movedFilesRepository; + + public ClosedIssuesInputFactory(ComponentIssuesLoader issuesLoader, DbClient dbClient, MovedFilesRepository movedFilesRepository) { + this.issuesLoader = issuesLoader; + this.dbClient = dbClient; + this.movedFilesRepository = movedFilesRepository; + } + + public Input create(Component component) { + return new ClosedIssuesLazyInput(dbClient, component, movedFilesRepository.getOriginalFile(component).orNull()); + } + + private class ClosedIssuesLazyInput extends BaseLazyInput { + + ClosedIssuesLazyInput(DbClient dbClient, Component component, @Nullable MovedFilesRepository.OriginalFile originalFile) { + super(dbClient, component, originalFile); + } + + @Override + protected List loadIssues() { + return issuesLoader.loadClosedIssues(effectiveUuid); + } + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoader.java index 13bb7eabb58..f44149c50e9 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoader.java @@ -19,17 +19,24 @@ */ package org.sonar.ce.task.projectanalysis.issue; +import com.google.common.collect.ImmutableList; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import java.util.Map; +import java.util.Optional; +import org.apache.ibatis.session.ResultContext; +import org.apache.ibatis.session.ResultHandler; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; +import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolder; import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.FieldDiffs; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.issue.IssueChangeDto; +import org.sonar.db.issue.IssueDto; import org.sonar.db.issue.IssueMapper; -import org.sonar.ce.task.projectanalysis.qualityprofile.ActiveRulesHolder; import static java.util.Collections.emptyList; import static java.util.stream.Collectors.groupingBy; @@ -41,34 +48,48 @@ public class ComponentIssuesLoader { private final ActiveRulesHolder activeRulesHolder; public ComponentIssuesLoader(DbClient dbClient, RuleRepository ruleRepository, ActiveRulesHolder activeRulesHolder) { - this.activeRulesHolder = activeRulesHolder; this.dbClient = dbClient; + this.activeRulesHolder = activeRulesHolder; this.ruleRepository = ruleRepository; } - public List loadForComponentUuid(String componentUuid) { + public List loadOpenIssues(String componentUuid) { try (DbSession dbSession = dbClient.openSession(false)) { - return loadForComponentUuid(componentUuid, dbSession); + return loadOpenIssues(componentUuid, dbSession); } } - public List loadForComponentUuidWithChanges(String componentUuid) { + public List loadOpenIssuesWithChanges(String componentUuid) { try (DbSession dbSession = dbClient.openSession(false)) { - List result = loadForComponentUuid(componentUuid, dbSession); + List result = loadOpenIssues(componentUuid, dbSession); - Map> changeDtoByIssueKey = dbClient.issueChangeDao() - .selectByIssueKeys(dbSession, result.stream().map(DefaultIssue::key).collect(toList())) - .stream() - .collect(groupingBy(IssueChangeDto::getIssueKey)); + return loadChanges(dbSession, result); + } + } - return result - .stream() - .peek(i -> setChanges(changeDtoByIssueKey, i)) - .collect(toList()); + public void loadChanges(Collection issues) { + if (issues.isEmpty()) { + return; } + + try (DbSession dbSession = dbClient.openSession(false)) { + loadChanges(dbSession, issues); + } + } + + public List loadChanges(DbSession dbSession, Collection issues) { + Map> changeDtoByIssueKey = dbClient.issueChangeDao() + .selectByIssueKeys(dbSession, issues.stream().map(DefaultIssue::key).collect(toList())) + .stream() + .collect(groupingBy(IssueChangeDto::getIssueKey)); + + return issues + .stream() + .peek(i -> setChanges(changeDtoByIssueKey, i)) + .collect(toList()); } - private List loadForComponentUuid(String componentUuid, DbSession dbSession) { + private List loadOpenIssues(String componentUuid, DbSession dbSession) { List result = new ArrayList<>(); dbSession.getMapper(IssueMapper.class).scrollNonClosedByComponentUuid(componentUuid, resultContext -> { DefaultIssue issue = (resultContext.getResultObject()).toDefaultIssue(); @@ -84,10 +105,10 @@ public class ComponentIssuesLoader { issue.setSelectedAt(System.currentTimeMillis()); result.add(issue); }); - return result; + return ImmutableList.copyOf(result); } - public static void setChanges(Map> changeDtoByIssueKey, DefaultIssue i) { + private static void setChanges(Map> changeDtoByIssueKey, DefaultIssue i) { changeDtoByIssueKey.computeIfAbsent(i.key(), k -> emptyList()).forEach(c -> { switch (c.getChangeType()) { case IssueChangeDto.TYPE_FIELD_CHANGE: @@ -105,4 +126,57 @@ public class ComponentIssuesLoader { private boolean isActive(RuleKey ruleKey) { return activeRulesHolder.get(ruleKey).isPresent(); } + + /** + * Load closed issues for the specified Component, which have at least one line diff in changelog AND are + * neither hotspots nor manual vulnerabilities. + *

+ * Closed issues do not have a line number in DB (it is unset when the issue is closed), this method + * returns {@link DefaultIssue} objects which line number is populated from the most recent diff logging + * the removal of the line. Closed issues which do not have such diff are not loaded. + */ + public List loadClosedIssues(String componentUuid) { + try (DbSession dbSession = dbClient.openSession(false)) { + return loadClosedIssues(componentUuid, dbSession); + } + } + + private static List loadClosedIssues(String componentUuid, DbSession dbSession) { + ClosedIssuesResultHandler handler = new ClosedIssuesResultHandler(); + dbSession.getMapper(IssueMapper.class).scrollClosedByComponentUuid(componentUuid, handler); + return ImmutableList.copyOf(handler.issues); + } + + private static class ClosedIssuesResultHandler implements ResultHandler { + private final List issues = new ArrayList<>(); + private String previousIssueKey = null; + + @Override + public void handleResult(ResultContext resultContext) { + IssueDto resultObject = resultContext.getResultObject(); + + // issue are ordered by most recent change first, only the first row for a given issue is of interest + if (previousIssueKey != null && previousIssueKey.equals(resultObject.getKey())) { + return; + } + + FieldDiffs fieldDiffs = FieldDiffs.parse(resultObject.getLineChangeData() + .orElseThrow(() -> new IllegalStateException("Line Change data should be populated"))); + Optional line = Optional.ofNullable(fieldDiffs.get("line")) + .map(diff -> (String) diff.oldValue()) + .filter(str -> !str.isEmpty()) + .map(Integer::parseInt); + if (!line.isPresent()) { + return; + } + + previousIssueKey = resultObject.getKey(); + DefaultIssue issue = resultObject.toDefaultIssue(); + issue.setLine(line.get()); + // FIXME + issue.setSelectedAt(System.currentTimeMillis()); + + issues.add(issue); + } + } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java index 3d7a98e91fb..26d63bb95dc 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java @@ -88,34 +88,34 @@ public class IssueLifecycle { public void copyExistingOpenIssueFromLongLivingBranch(DefaultIssue raw, DefaultIssue base, String fromLongBranchName) { raw.setKey(Uuids.create()); raw.setNew(false); - copyIssueAttributes(raw, base); + copyAttributesOfIssueFromOtherBranch(raw, base); raw.setFieldChange(changeContext, IssueFieldsSetter.FROM_LONG_BRANCH, fromLongBranchName, analysisMetadataHolder.getBranch().getName()); } public void mergeConfirmedOrResolvedFromShortLivingBranch(DefaultIssue raw, DefaultIssue base, String fromShortBranchName) { - copyIssueAttributes(raw, base); + copyAttributesOfIssueFromOtherBranch(raw, base); raw.setFieldChange(changeContext, IssueFieldsSetter.FROM_SHORT_BRANCH, fromShortBranchName, analysisMetadataHolder.getBranch().getName()); } - private void copyIssueAttributes(DefaultIssue to, DefaultIssue from) { + private void copyAttributesOfIssueFromOtherBranch(DefaultIssue to, DefaultIssue from) { to.setCopied(true); copyFields(to, from); if (from.manualSeverity()) { to.setManualSeverity(true); to.setSeverity(from.severity()); } - copyChanges(to, from); + copyChangesOfIssueFromOtherBranch(to, from); } - private static void copyChanges(DefaultIssue raw, DefaultIssue base) { - base.defaultIssueComments().forEach(c -> raw.addComment(copy(raw.key(), c))); - base.changes().forEach(c -> copy(raw.key(), c).ifPresent(raw::addChange)); + private static void copyChangesOfIssueFromOtherBranch(DefaultIssue raw, DefaultIssue base) { + base.defaultIssueComments().forEach(c -> raw.addComment(copyComment(raw.key(), c))); + base.changes().forEach(c -> copyFieldDiffOfIssueFromOtherBranch(raw.key(), c).ifPresent(raw::addChange)); } /** * Copy a comment from another issue */ - private static DefaultIssueComment copy(String issueKey, DefaultIssueComment c) { + private static DefaultIssueComment copyComment(String issueKey, DefaultIssueComment c) { DefaultIssueComment comment = new DefaultIssueComment(); comment.setIssueKey(issueKey); comment.setKey(Uuids.create()); @@ -129,7 +129,7 @@ public class IssueLifecycle { /** * Copy a diff from another issue */ - private static Optional copy(String issueKey, FieldDiffs c) { + private static Optional copyFieldDiffOfIssueFromOtherBranch(String issueKey, FieldDiffs c) { FieldDiffs result = new FieldDiffs(); result.setIssueKey(issueKey); result.setUserUuid(c.userUuid()); @@ -149,6 +149,7 @@ public class IssueLifecycle { raw.setNew(false); setType(raw); copyFields(raw, base); + base.changes().forEach(raw::addChange); if (raw.isFromHotspot() != base.isFromHotspot()) { // This is to force DB update of the issue raw.setChanged(true); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/MergeBranchTrackerExecution.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/MergeBranchTrackerExecution.java index 745e79c8ea9..dbd8b08d872 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/MergeBranchTrackerExecution.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/MergeBranchTrackerExecution.java @@ -23,7 +23,6 @@ import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.tracking.Tracker; import org.sonar.core.issue.tracking.Tracking; -import org.sonar.ce.task.projectanalysis.component.Component; public class MergeBranchTrackerExecution { private final TrackerRawInputFactory rawInputFactory; @@ -38,6 +37,6 @@ public class MergeBranchTrackerExecution { } public Tracking track(Component component) { - return tracker.track(rawInputFactory.create(component), mergeInputFactory.create(component)); + return tracker.trackNonClosed(rawInputFactory.create(component), mergeInputFactory.create(component)); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchIssuesLoader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchIssuesLoader.java index a75f1608c74..b943350ce1c 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchIssuesLoader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchIssuesLoader.java @@ -31,24 +31,24 @@ import org.sonar.core.issue.DefaultIssue; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; -import org.sonar.db.issue.IssueChangeDto; import org.sonar.db.issue.IssueDto; import org.sonar.db.issue.ShortBranchIssueDto; -import org.sonar.ce.task.projectanalysis.component.Component; -import org.sonar.ce.task.projectanalysis.component.ShortBranchComponentsWithIssues; -import static java.util.stream.Collectors.groupingBy; -import static java.util.stream.Collectors.toMap; import static org.sonar.api.utils.DateUtils.longToDate; +import static org.sonar.core.util.stream.MoreCollectors.toList; +import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; public class ShortBranchIssuesLoader { private final ShortBranchComponentsWithIssues shortBranchComponentsWithIssues; private final DbClient dbClient; + private final ComponentIssuesLoader componentIssuesLoader; - public ShortBranchIssuesLoader(ShortBranchComponentsWithIssues shortBranchComponentsWithIssues, DbClient dbClient) { + public ShortBranchIssuesLoader(ShortBranchComponentsWithIssues shortBranchComponentsWithIssues, DbClient dbClient, + ComponentIssuesLoader componentIssuesLoader) { this.shortBranchComponentsWithIssues = shortBranchComponentsWithIssues; this.dbClient = dbClient; + this.componentIssuesLoader = componentIssuesLoader; } public Collection loadCandidateIssuesForMergingInTargetBranch(Component component) { @@ -57,6 +57,7 @@ public class ShortBranchIssuesLoader { if (uuids.isEmpty()) { return Collections.emptyList(); } + try (DbSession session = dbClient.openSession(false)) { return dbClient.issueDao().selectOpenByComponentUuids(session, uuids) .stream() @@ -74,17 +75,16 @@ public class ShortBranchIssuesLoader { if (lightIssues.isEmpty()) { return Collections.emptyMap(); } + Map issuesByKey = lightIssues.stream().collect(Collectors.toMap(ShortBranchIssue::getKey, i -> i)); try (DbSession session = dbClient.openSession(false)) { - - Map> changeDtoByIssueKey = dbClient.issueChangeDao() - .selectByIssueKeys(session, issuesByKey.keySet()).stream().collect(groupingBy(IssueChangeDto::getIssueKey)); - - return dbClient.issueDao().selectByKeys(session, issuesByKey.keySet()) + List issues = dbClient.issueDao().selectByKeys(session, issuesByKey.keySet()) .stream() .map(IssueDto::toDefaultIssue) - .peek(i -> ComponentIssuesLoader.setChanges(changeDtoByIssueKey, i)) - .collect(toMap(i -> issuesByKey.get(i.key()), i -> i)); + .collect(toList(issuesByKey.size())); + componentIssuesLoader.loadChanges(session, issues); + return issues.stream() + .collect(uniqueIndex(i -> issuesByKey.get(i.key()), i -> i, issues.size())); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecution.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecution.java index 6dca1decd57..6893b0d24a7 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecution.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchTrackerExecution.java @@ -46,12 +46,12 @@ public class ShortBranchTrackerExecution { Input baseInput = baseInputFactory.create(component); Input mergeInput = mergeInputFactory.create(component); - Tracking mergeTracking = tracker.track(rawInput, mergeInput); + Tracking mergeTracking = tracker.trackNonClosed(rawInput, mergeInput); List unmatchedRaws = mergeTracking.getUnmatchedRaws().collect(MoreCollectors.toList()); Input unmatchedRawInput = new DefaultTrackingInput(unmatchedRaws, rawInput.getLineHashSequence(), rawInput.getBlockHashSequence()); // do second tracking with base branch using raws issues that are still unmatched - return tracker.track(unmatchedRawInput, baseInput); + return tracker.trackNonClosed(unmatchedRawInput, baseInput); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerBaseInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerBaseInputFactory.java index 0ba69ce041f..e55972ee647 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerBaseInputFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerBaseInputFactory.java @@ -19,26 +19,19 @@ */ package org.sonar.ce.task.projectanalysis.issue; -import java.util.Collections; import java.util.List; -import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository; +import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository.OriginalFile; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.tracking.Input; -import org.sonar.core.issue.tracking.LazyInput; -import org.sonar.core.issue.tracking.LineHashSequence; import org.sonar.db.DbClient; -import org.sonar.db.DbSession; -import org.sonar.ce.task.projectanalysis.component.Component; -import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository; -import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository.OriginalFile; /** * Factory of {@link Input} of base data for issue tracking. Data are lazy-loaded. */ -public class TrackerBaseInputFactory { - private static final LineHashSequence EMPTY_LINE_HASH_SEQUENCE = new LineHashSequence(Collections.emptyList()); +public class TrackerBaseInputFactory extends BaseInputFactory { private final ComponentIssuesLoader issuesLoader; private final DbClient dbClient; @@ -51,37 +44,18 @@ public class TrackerBaseInputFactory { } public Input create(Component component) { - return new BaseLazyInput(component, movedFilesRepository.getOriginalFile(component).orNull()); + return new TrackerBaseLazyInput(dbClient, component, movedFilesRepository.getOriginalFile(component).orNull()); } - private class BaseLazyInput extends LazyInput { - private final Component component; - @CheckForNull - private final String effectiveUuid; - - private BaseLazyInput(Component component, @Nullable OriginalFile originalFile) { - this.component = component; - this.effectiveUuid = originalFile == null ? component.getUuid() : originalFile.getUuid(); - } - - @Override - protected LineHashSequence loadLineHashSequence() { - if (component.getType() != Component.Type.FILE) { - return EMPTY_LINE_HASH_SEQUENCE; - } + private class TrackerBaseLazyInput extends BaseLazyInput { - try (DbSession session = dbClient.openSession(false)) { - List hashes = dbClient.fileSourceDao().selectLineHashes(session, effectiveUuid); - if (hashes == null || hashes.isEmpty()) { - return EMPTY_LINE_HASH_SEQUENCE; - } - return new LineHashSequence(hashes); - } + private TrackerBaseLazyInput(DbClient dbClient, Component component, @Nullable OriginalFile originalFile) { + super(dbClient, component, originalFile); } @Override protected List loadIssues() { - return issuesLoader.loadForComponentUuid(effectiveUuid); + return issuesLoader.loadOpenIssues(effectiveUuid); } } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerExecution.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerExecution.java index deb980b43d6..29ab66bc3aa 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerExecution.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerExecution.java @@ -19,26 +19,54 @@ */ package org.sonar.ce.task.projectanalysis.issue; +import java.util.Set; +import org.sonar.api.issue.Issue; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.core.issue.tracking.NonClosedTracking; import org.sonar.core.issue.tracking.Tracker; import org.sonar.core.issue.tracking.Tracking; -import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.core.util.stream.MoreCollectors; public class TrackerExecution { - protected final TrackerBaseInputFactory baseInputFactory; - protected final TrackerRawInputFactory rawInputFactory; - protected final Tracker tracker; + private final TrackerBaseInputFactory baseInputFactory; + private final TrackerRawInputFactory rawInputFactory; + private final ClosedIssuesInputFactory closedIssuesInputFactory; + private final Tracker tracker; + private final ComponentIssuesLoader componentIssuesLoader; public TrackerExecution(TrackerBaseInputFactory baseInputFactory, TrackerRawInputFactory rawInputFactory, - Tracker tracker) { + ClosedIssuesInputFactory closedIssuesInputFactory, Tracker tracker, + ComponentIssuesLoader componentIssuesLoader) { this.baseInputFactory = baseInputFactory; this.rawInputFactory = rawInputFactory; + this.closedIssuesInputFactory = closedIssuesInputFactory; this.tracker = tracker; + this.componentIssuesLoader = componentIssuesLoader; } public Tracking track(Component component) { - return tracker.track(rawInputFactory.create(component), baseInputFactory.create(component)); + Input rawInput = rawInputFactory.create(component); + Input openBaseIssuesInput = baseInputFactory.create(component); + NonClosedTracking openIssueTracking = tracker.trackNonClosed(rawInput, openBaseIssuesInput); + if (openIssueTracking.isComplete()) { + return openIssueTracking; + } + + Input closedIssuesBaseInput = closedIssuesInputFactory.create(component); + Tracking closedIssuesTracking = tracker.trackClosed(openIssueTracking, closedIssuesBaseInput); + + // changes of closed issues need to be loaded in order to: + // - compute right transition from workflow + // - recover fields values from before they were closed + Set matchesClosedIssues = closedIssuesTracking.getMatchedRaws().values().stream() + .filter(t -> Issue.STATUS_CLOSED.equals(t.getStatus())) + .collect(MoreCollectors.toSet()); + componentIssuesLoader.loadChanges(matchesClosedIssues); + + return closedIssuesTracking; } + } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerMergeBranchInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerMergeBranchInputFactory.java index 8334e26a69f..e5c2e42f5b8 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerMergeBranchInputFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerMergeBranchInputFactory.java @@ -30,8 +30,6 @@ import org.sonar.core.issue.tracking.LazyInput; import org.sonar.core.issue.tracking.LineHashSequence; import org.sonar.db.DbClient; import org.sonar.db.DbSession; -import org.sonar.ce.task.projectanalysis.component.Component; -import org.sonar.ce.task.projectanalysis.component.MergeBranchComponentUuids; public class TrackerMergeBranchInputFactory { private static final LineHashSequence EMPTY_LINE_HASH_SEQUENCE = new LineHashSequence(Collections.emptyList()); @@ -81,7 +79,7 @@ public class TrackerMergeBranchInputFactory { if (mergeBranchComponentUuid == null) { return Collections.emptyList(); } - return mergeIssuesLoader.loadForComponentUuidWithChanges(mergeBranchComponentUuid); + return mergeIssuesLoader.loadOpenIssuesWithChanges(mergeBranchComponentUuid); } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitorTest.java index 54a1bfd47cc..ea836e621f6 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/CloseIssuesOnRemovedComponentsVisitorTest.java @@ -67,7 +67,7 @@ public class CloseIssuesOnRemovedComponentsVisitorTest { when(componentsWithUnprocessedIssues.getUuids()).thenReturn(newHashSet(fileUuid)); DefaultIssue issue = new DefaultIssue().setKey(issueUuid); - when(issuesLoader.loadForComponentUuid(fileUuid)).thenReturn(Collections.singletonList(issue)); + when(issuesLoader.loadOpenIssues(fileUuid)).thenReturn(Collections.singletonList(issue)); underTest.visit(ReportComponent.builder(PROJECT, 1).build()); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ClosedIssuesInputFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ClosedIssuesInputFactoryTest.java new file mode 100644 index 00000000000..0c8dc4901df --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ClosedIssuesInputFactoryTest.java @@ -0,0 +1,100 @@ +/* + * 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 com.google.common.base.Optional; +import com.google.common.collect.ImmutableList; +import java.util.List; +import org.junit.Test; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.component.ReportComponent; +import org.sonar.ce.task.projectanalysis.filemove.MovedFilesRepository; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.db.DbClient; + +import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +public class ClosedIssuesInputFactoryTest { + private ComponentIssuesLoader issuesLoader = mock(ComponentIssuesLoader.class); + private DbClient dbClient = mock(DbClient.class); + private MovedFilesRepository movedFilesRepository = mock(MovedFilesRepository.class); + private ClosedIssuesInputFactory underTest = new ClosedIssuesInputFactory(issuesLoader, dbClient, movedFilesRepository); + + @Test + public void underTest_returns_inputFactory_loading_closed_issues_only_when_getIssues_is_called() { + String componentUuid = randomAlphanumeric(12); + ReportComponent component = ReportComponent.builder(Component.Type.FILE, 1).setUuid(componentUuid).build(); + when(movedFilesRepository.getOriginalFile(component)).thenReturn(Optional.absent()); + + Input input = underTest.create(component); + + verifyZeroInteractions(dbClient, issuesLoader); + + List issues = ImmutableList.of(new DefaultIssue(), new DefaultIssue()); + when(issuesLoader.loadClosedIssues(componentUuid)).thenReturn(issues); + + assertThat(input.getIssues()).isSameAs(issues); + } + + @Test + public void underTest_returns_inputFactory_loading_closed_issues_from_moved_component_when_present() { + String componentUuid = randomAlphanumeric(12); + String originalComponentUuid = randomAlphanumeric(12); + ReportComponent component = ReportComponent.builder(Component.Type.FILE, 1).setUuid(componentUuid).build(); + when(movedFilesRepository.getOriginalFile(component)) + .thenReturn(Optional.of(new MovedFilesRepository.OriginalFile(1, originalComponentUuid, randomAlphanumeric(2)))); + + Input input = underTest.create(component); + + verifyZeroInteractions(dbClient, issuesLoader); + + List issues = ImmutableList.of(); + when(issuesLoader.loadClosedIssues(originalComponentUuid)).thenReturn(issues); + + assertThat(input.getIssues()).isSameAs(issues); + } + + @Test + public void underTest_returns_inputFactory_which_caches_loaded_issues() { + String componentUuid = randomAlphanumeric(12); + ReportComponent component = ReportComponent.builder(Component.Type.FILE, 1).setUuid(componentUuid).build(); + when(movedFilesRepository.getOriginalFile(component)).thenReturn(Optional.absent()); + + Input input = underTest.create(component); + + verifyZeroInteractions(dbClient, issuesLoader); + + List issues = ImmutableList.of(new DefaultIssue()); + when(issuesLoader.loadClosedIssues(componentUuid)).thenReturn(issues); + + assertThat(input.getIssues()).isSameAs(issues); + + reset(issuesLoader); + + assertThat(input.getIssues()).isSameAs(issues); + verifyZeroInteractions(issuesLoader); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoaderTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoaderTest.java new file mode 100644 index 00000000000..9bcc114976e --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoaderTest.java @@ -0,0 +1,85 @@ +/* + * 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.Date; +import java.util.List; +import org.junit.Rule; +import org.junit.Test; +import org.sonar.api.issue.Issue; +import org.sonar.api.rules.RuleType; +import org.sonar.api.utils.System2; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.FieldDiffs; +import org.sonar.db.DbClient; +import org.sonar.db.DbTester; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.component.ComponentTesting; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.organization.OrganizationDto; +import org.sonar.db.rule.RuleDefinitionDto; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.api.utils.DateUtils.addDays; + +public class ComponentIssuesLoaderTest { + @Rule + public DbTester dbTester = DbTester.create(System2.INSTANCE); + + private DbClient dbClient = dbTester.getDbClient(); + private ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, + null /* not used in loadClosedIssues */, null /* not used in loadClosedIssues */); + + @Test + public void loadClosedIssues_returns_single_DefaultIssue_by_issue_based_on_first_row() { + OrganizationDto organization = dbTester.organizations().insert(); + ComponentDto project = dbTester.components().insertPublicProject(organization); + ComponentDto file = dbTester.components().insertComponent(ComponentTesting.newFileDto(project)); + RuleDefinitionDto rule = dbTester.rules().insert(t -> t.setType(RuleType.CODE_SMELL)); + IssueDto issue = dbTester.issues().insert(rule, project, file, t -> t.setStatus(Issue.STATUS_CLOSED).setIsFromHotspot(false)); + Date creationDate = new Date(); + dbTester.issues().insertFieldDiffs(issue, new FieldDiffs().setCreationDate(addDays(creationDate, -5)).setDiff("line", 10, "")); + dbTester.issues().insertFieldDiffs(issue, new FieldDiffs().setCreationDate(creationDate).setDiff("line", 20, "")); + dbTester.issues().insertFieldDiffs(issue, new FieldDiffs().setCreationDate(addDays(creationDate, -10)).setDiff("line", 30, "")); + + List defaultIssues = underTest.loadClosedIssues(file.uuid()); + + assertThat(defaultIssues).hasSize(1); + assertThat(defaultIssues.iterator().next().getLine()).isEqualTo(20); + } + + @Test + public void loadClosedIssues_returns_single_DefaultIssue_ignoring_lines_without_old_values() { + OrganizationDto organization = dbTester.organizations().insert(); + ComponentDto project = dbTester.components().insertPublicProject(organization); + ComponentDto file = dbTester.components().insertComponent(ComponentTesting.newFileDto(project)); + RuleDefinitionDto rule = dbTester.rules().insert(t -> t.setType(RuleType.CODE_SMELL)); + IssueDto issue = dbTester.issues().insert(rule, project, file, t -> t.setStatus(Issue.STATUS_CLOSED).setIsFromHotspot(false)); + Date creationDate = new Date(); + dbTester.issues().insertFieldDiffs(issue, new FieldDiffs().setCreationDate(addDays(creationDate, -5)).setDiff("line", null, "")); + dbTester.issues().insertFieldDiffs(issue, new FieldDiffs().setCreationDate(creationDate).setDiff("line", 20, null)); // if new value is null, neither old nor new is stored in DB + dbTester.issues().insertFieldDiffs(issue, new FieldDiffs().setCreationDate(addDays(creationDate, -10)).setDiff("line", 30, "")); + + List defaultIssues = underTest.loadClosedIssues(file.uuid()); + + assertThat(defaultIssues).hasSize(1); + assertThat(defaultIssues.iterator().next().getLine()).isEqualTo(30); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java index c209bb445a3..bc4fe0f02b0 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java @@ -49,6 +49,7 @@ import org.sonar.ce.task.projectanalysis.source.SourceLinesHashRepository; import org.sonar.ce.task.projectanalysis.source.SourceLinesRepositoryRule; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.tracking.Tracker; +import org.sonar.db.DbClient; import org.sonar.db.DbTester; import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; @@ -105,8 +106,6 @@ public class IntegrateIssuesVisitorTest { public RuleRepositoryRule ruleRepositoryRule = new RuleRepositoryRule(); @Rule public SourceLinesRepositoryRule fileSourceRepository = new SourceLinesRepositoryRule(); - @Rule - public RuleRepositoryRule ruleRepository = new RuleRepositoryRule(); private AnalysisMetadataHolder analysisMetadataHolder = mock(AnalysisMetadataHolder.class); private IssueFilter issueFilter = mock(IssueFilter.class); @@ -137,11 +136,13 @@ public class IntegrateIssuesVisitorTest { defaultIssueCaptor = ArgumentCaptor.forClass(DefaultIssue.class); when(movedFilesRepository.getOriginalFile(any(Component.class))).thenReturn(Optional.absent()); - TrackerRawInputFactory rawInputFactory = new TrackerRawInputFactory(treeRootHolder, reportReader, sourceLinesHash, new CommonRuleEngineImpl(), issueFilter, ruleRepository, + DbClient dbClient = dbTester.getDbClient(); + TrackerRawInputFactory rawInputFactory = new TrackerRawInputFactory(treeRootHolder, reportReader, sourceLinesHash, new CommonRuleEngineImpl(), issueFilter, ruleRepositoryRule, activeRulesHolder); - TrackerBaseInputFactory baseInputFactory = new TrackerBaseInputFactory(issuesLoader, dbTester.getDbClient(), movedFilesRepository); - TrackerMergeBranchInputFactory mergeInputFactory = new TrackerMergeBranchInputFactory(issuesLoader, mergeBranchComponentsUuids, dbTester.getDbClient()); - tracker = new TrackerExecution(baseInputFactory, rawInputFactory, new Tracker<>()); + TrackerBaseInputFactory baseInputFactory = new TrackerBaseInputFactory(issuesLoader, dbClient, movedFilesRepository); + TrackerMergeBranchInputFactory mergeInputFactory = new TrackerMergeBranchInputFactory(issuesLoader, mergeBranchComponentsUuids, dbClient); + ClosedIssuesInputFactory closedIssuesInputFactory = new ClosedIssuesInputFactory(issuesLoader, dbClient, movedFilesRepository); + tracker = new TrackerExecution(baseInputFactory, rawInputFactory, closedIssuesInputFactory, new Tracker<>(), new ComponentIssuesLoader(dbClient, ruleRepositoryRule, activeRulesHolder)); shortBranchTracker = new ShortBranchTrackerExecution(baseInputFactory, rawInputFactory, mergeInputFactory, new Tracker<>()); mergeBranchTracker = new MergeBranchTrackerExecution(rawInputFactory, mergeInputFactory, new Tracker<>()); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java index d629e26f98b..2c9a3a2bc30 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java @@ -39,6 +39,7 @@ import org.sonar.server.issue.workflow.IssueWorkflow; import static com.google.common.collect.Lists.newArrayList; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.entry; import static org.assertj.core.groups.Tuple.tuple; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -274,7 +275,9 @@ public class IssueLifecycleTest { .setGap(15d) .setEffort(Duration.create(15L)) .setManualSeverity(false) - .setLocations(issueLocations); + .setLocations(issueLocations) + .addChange(new FieldDiffs().setDiff("foo", "bar", "donut")) + .addChange(new FieldDiffs().setDiff("file", "A", "B")); when(debtCalculator.calculate(raw)).thenReturn(DEFAULT_DURATION); @@ -293,6 +296,11 @@ public class IssueLifecycleTest { assertThat(raw.isOnDisabledRule()).isTrue(); assertThat(raw.selectedAt()).isEqualTo(1000L); assertThat(raw.isChanged()).isFalse(); + assertThat(raw.changes()).hasSize(2); + assertThat(raw.changes().get(0).diffs()) + .containsOnly(entry("foo", new FieldDiffs.Diff("bar", "donut"))); + assertThat(raw.changes().get(1).diffs()) + .containsOnly(entry("file", new FieldDiffs.Diff("A", "B"))); verify(updater).setPastSeverity(raw, BLOCKER, issueChangeContext); verify(updater).setPastLine(raw, 10); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/MergeBranchTrackerExecutionTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/MergeBranchTrackerExecutionTest.java index d3b3e538ae0..ccad3e2498f 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/MergeBranchTrackerExecutionTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/MergeBranchTrackerExecutionTest.java @@ -26,8 +26,8 @@ import org.mockito.MockitoAnnotations; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.tracking.Input; +import org.sonar.core.issue.tracking.NonClosedTracking; import org.sonar.core.issue.tracking.Tracker; -import org.sonar.core.issue.tracking.Tracking; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -55,10 +55,10 @@ public class MergeBranchTrackerExecutionTest { public void testTracking() { Input rawInput = mock(Input.class); Input mergeInput = mock(Input.class); - Tracking result = mock(Tracking.class); + NonClosedTracking result = mock(NonClosedTracking.class); when(rawInputFactory.create(component)).thenReturn(rawInput); when(mergeInputFactory.create(component)).thenReturn(mergeInput); - when(tracker.track(rawInput, mergeInput)).thenReturn(result); + when(tracker.trackNonClosed(rawInput, mergeInput)).thenReturn(result); assertThat(underTest.track(component)).isEqualTo(result); } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchIssueMergerTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchIssueMergerTest.java index 093c985529a..2fabe28f1b3 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchIssueMergerTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ShortBranchIssueMergerTest.java @@ -37,6 +37,7 @@ import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.FieldDiffs; import org.sonar.core.issue.tracking.SimpleTracker; +import org.sonar.db.DbClient; import org.sonar.db.DbTester; import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; @@ -92,7 +93,8 @@ public class ShortBranchIssueMergerTest { @Before public void setUp() { MockitoAnnotations.initMocks(this); - copier = new ShortBranchIssueMerger(new ShortBranchIssuesLoader(new ShortBranchComponentsWithIssues(treeRootHolder, db.getDbClient()), db.getDbClient()), tracker, + DbClient dbClient = db.getDbClient(); + copier = new ShortBranchIssueMerger(new ShortBranchIssuesLoader(new ShortBranchComponentsWithIssues(treeRootHolder, dbClient), dbClient, new ComponentIssuesLoader(dbClient, null, null)), tracker, issueLifecycle); projectDto = db.components().insertMainBranch(p -> p.setDbKey(PROJECT_KEY).setUuid(PROJECT_UUID)); branch1Dto = db.components().insertProjectBranch(projectDto, b -> b.setKey("myBranch1") diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerBaseInputFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerBaseInputFactoryTest.java index d91235cc066..0b747be5f25 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerBaseInputFactoryTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerBaseInputFactoryTest.java @@ -80,7 +80,7 @@ public class TrackerBaseInputFactoryTest { public void create_returns_Input_which_retrieves_issues_of_specified_file_component_when_it_has_no_original_file() { underTest.create(FILE).getIssues(); - verify(issuesLoader).loadForComponentUuid(FILE_UUID); + verify(issuesLoader).loadOpenIssues(FILE_UUID); } @Test @@ -92,7 +92,7 @@ public class TrackerBaseInputFactoryTest { underTest.create(FILE).getIssues(); - verify(issuesLoader).loadForComponentUuid(originalUuid); - verify(issuesLoader, times(0)).loadForComponentUuid(FILE_UUID); + verify(issuesLoader).loadOpenIssues(originalUuid); + verify(issuesLoader, times(0)).loadOpenIssues(FILE_UUID); } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerExecutionTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerExecutionTest.java new file mode 100644 index 00000000000..d7551a1b5b0 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerExecutionTest.java @@ -0,0 +1,122 @@ +/* + * 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.ArrayList; +import java.util.Collections; +import java.util.Random; +import java.util.Set; +import java.util.stream.IntStream; +import org.junit.Test; +import org.sonar.api.issue.Issue; +import org.sonar.ce.task.projectanalysis.component.Component; +import org.sonar.ce.task.projectanalysis.component.ReportComponent; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.tracking.Input; +import org.sonar.core.issue.tracking.NonClosedTracking; +import org.sonar.core.issue.tracking.Tracker; +import org.sonar.core.issue.tracking.Tracking; + +import static java.util.stream.Collectors.toSet; +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import static org.mockito.Mockito.when; +import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; + +public class TrackerExecutionTest { + private final TrackerRawInputFactory rawInputFactory = mock(TrackerRawInputFactory.class); + private final TrackerBaseInputFactory baseInputFactory = mock(TrackerBaseInputFactory.class); + private final ClosedIssuesInputFactory closedIssuesInputFactory = mock(ClosedIssuesInputFactory.class); + private final Tracker tracker = mock(Tracker.class); + private final ComponentIssuesLoader componentIssuesLoader = mock(ComponentIssuesLoader.class); + + private TrackerExecution underTest = new TrackerExecution(baseInputFactory, rawInputFactory, closedIssuesInputFactory, tracker, componentIssuesLoader); + + private Input rawInput = mock(Input.class); + private Input openIssuesInput = mock(Input.class); + private Input closedIssuesInput = mock(Input.class); + private NonClosedTracking nonClosedTracking = mock(NonClosedTracking.class); + private Tracking closedTracking = mock(Tracking.class); + + @Test + public void track_tracks_only_nonClosed_issues_if_tracking_returns_complete_from_Tracker() { + ReportComponent component = ReportComponent.builder(Component.Type.FILE, 1).build(); + when(rawInputFactory.create(component)).thenReturn(rawInput); + when(baseInputFactory.create(component)).thenReturn(openIssuesInput); + when(closedIssuesInputFactory.create(any())).thenThrow(new IllegalStateException("closedIssuesInputFactory should not be called")); + when(nonClosedTracking.isComplete()).thenReturn(true); + when(tracker.trackNonClosed(rawInput, openIssuesInput)).thenReturn(nonClosedTracking); + when(tracker.trackClosed(any(), any())).thenThrow(new IllegalStateException("trackClosed should not be called")); + + Tracking tracking = underTest.track(component); + + assertThat(tracking).isSameAs(nonClosedTracking); + verify(tracker).trackNonClosed(rawInput, openIssuesInput); + verifyNoMoreInteractions(tracker); + } + + @Test + public void track_tracks_nonClosed_issues_and_then_closedOnes_if_tracking_returns_incomplete() { + ReportComponent component = ReportComponent.builder(Component.Type.FILE, 1).build(); + when(rawInputFactory.create(component)).thenReturn(rawInput); + when(baseInputFactory.create(component)).thenReturn(openIssuesInput); + when(closedIssuesInputFactory.create(component)).thenReturn(closedIssuesInput); + when(nonClosedTracking.isComplete()).thenReturn(false); + when(tracker.trackNonClosed(rawInput, openIssuesInput)).thenReturn(nonClosedTracking); + when(tracker.trackClosed(nonClosedTracking, closedIssuesInput)).thenReturn(closedTracking); + + Tracking tracking = underTest.track(component); + + assertThat(tracking).isSameAs(closedTracking); + verify(tracker).trackNonClosed(rawInput, openIssuesInput); + verify(tracker).trackClosed(nonClosedTracking, closedIssuesInput); + verifyNoMoreInteractions(tracker); + } + + @Test + public void track_loadChanges_on_matched_closed_issues() { + ReportComponent component = ReportComponent.builder(Component.Type.FILE, 1).build(); + when(rawInputFactory.create(component)).thenReturn(rawInput); + when(baseInputFactory.create(component)).thenReturn(openIssuesInput); + when(closedIssuesInputFactory.create(component)).thenReturn(closedIssuesInput); + when(nonClosedTracking.isComplete()).thenReturn(false); + when(tracker.trackNonClosed(rawInput, openIssuesInput)).thenReturn(nonClosedTracking); + when(tracker.trackClosed(nonClosedTracking, closedIssuesInput)).thenReturn(closedTracking); + Set mappedClosedIssues = IntStream.range(1, 2 + new Random().nextInt(2)) + .mapToObj(i -> new DefaultIssue().setKey("closed" + i).setStatus(Issue.STATUS_CLOSED)) + .collect(toSet()); + + ArrayList mappedBaseIssues = new ArrayList<>(mappedClosedIssues); + Issue.STATUSES.stream().filter(t -> !Issue.STATUS_CLOSED.equals(t)).forEach(s -> mappedBaseIssues.add(new DefaultIssue().setKey(s).setStatus(s))); + Collections.shuffle(mappedBaseIssues); + when(closedTracking.getMatchedRaws()).thenReturn(mappedBaseIssues.stream().collect(uniqueIndex(i -> new DefaultIssue().setKey("raw_for_" + i.key()), i -> i))); + + Tracking tracking = underTest.track(component); + + assertThat(tracking).isSameAs(closedTracking); + verify(tracker).trackNonClosed(rawInput, openIssuesInput); + verify(tracker).trackClosed(nonClosedTracking, closedIssuesInput); + verify(componentIssuesLoader).loadChanges(mappedClosedIssues); + verifyNoMoreInteractions(tracker); + } +} diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java index df8b2f9fb07..252431d5a26 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java @@ -28,6 +28,7 @@ import com.google.protobuf.InvalidProtocolBufferException; import java.io.Serializable; import java.util.Collection; import java.util.Date; +import java.util.Optional; import java.util.Set; import javax.annotation.CheckForNull; import javax.annotation.Nullable; @@ -97,6 +98,8 @@ public final class IssueDto implements Serializable { private String filePath; private String tags; private boolean isFromHotspot; + // populate only when retrieving closed issue for issue tracking + private String lineChangeData; /** * On batch side, component keys and uuid are useless @@ -698,6 +701,10 @@ public final class IssueDto implements Serializable { return this; } + public Optional getLineChangeData() { + return Optional.ofNullable(lineChangeData); + } + @Override public String toString() { return ToStringBuilder.reflectionToString(this, ToStringStyle.SHORT_PREFIX_STYLE); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueMapper.java index 0f80126c88c..9098c986d15 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueMapper.java @@ -43,7 +43,9 @@ public interface IssueMapper { int updateIfBeforeSelectedDate(IssueDto issue); void scrollNonClosedByComponentUuid(@Param("componentUuid") String componentUuid, ResultHandler handler); - + + void scrollClosedByComponentUuid(@Param("componentUuid") String componentUuid, ResultHandler handler); + List selectNonClosedByComponentUuidExcludingExternals(@Param("componentUuid") String componentUuid); List selectNonClosedByModuleOrProject(@Param("projectUuid") String projectUuid, @Param("likeModuleUuidPath") String likeModuleUuidPath); diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml index 3c30811cc82..ad2ff5da259 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml @@ -216,6 +216,30 @@ i.issue_type <> 4 and (i.from_hotspot is NULL or i.from_hotspot = ${_false}) + +