From 905a0e4a8b3c2f6b80314c4ac10edbd1c0307ff1 Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Tue, 29 Mar 2022 15:07:36 -0500 Subject: [PATCH] SONAR-15321 Large number of entries in 'issue_changes' --- ...ProjectAnalysisTaskContainerPopulator.java | 2 + .../issue/ComponentIssuesLoader.java | 237 ++++++++++++------ .../issue/IssueChangesToDeleteRepository.java | 37 +++ .../issue/PullRequestSourceBranchMerger.java | 3 +- .../step/CleanIssueChangesStep.java | 57 +++++ .../step/ReportComputationSteps.java | 1 + .../issue/ComponentIssuesLoaderTest.java | 114 ++++++--- .../issue/IntegrateIssuesVisitorTest.java | 2 +- .../IssueChangesToDeleteRepositoryTest.java | 35 +++ .../ProjectTrackerBaseLazyInputTest.java | 4 +- .../issue/SiblingsIssueMergerTest.java | 9 +- .../step/CleanIssueChangesStepTest.java | 66 +++++ .../org/sonar/db/issue/IssueChangeDao.java | 9 +- .../org/sonar/db/issue/IssueChangeMapper.java | 3 + .../org/sonar/db/issue/IssueChangeMapper.xml | 7 + .../sonar/db/issue/IssueChangeDaoTest.java | 20 +- .../hotspot/ws/DeleteCommentAction.java | 2 +- .../server/issue/ws/DeleteCommentAction.java | 2 +- 18 files changed, 483 insertions(+), 127 deletions(-) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepository.java create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStep.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepositoryTest.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStepTest.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 e236cfdd0b3..c8045d67c54 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 @@ -61,6 +61,7 @@ import org.sonar.ce.task.projectanalysis.issue.DefaultAssignee; import org.sonar.ce.task.projectanalysis.issue.EffortAggregator; import org.sonar.ce.task.projectanalysis.issue.IntegrateIssuesVisitor; import org.sonar.ce.task.projectanalysis.issue.IssueAssigner; +import org.sonar.ce.task.projectanalysis.issue.IssueChangesToDeleteRepository; import org.sonar.ce.task.projectanalysis.issue.IssueCounter; import org.sonar.ce.task.projectanalysis.issue.IssueCreationDateCalculator; import org.sonar.ce.task.projectanalysis.issue.IssueLifecycle; @@ -233,6 +234,7 @@ public final class ProjectAnalysisTaskContainerPopulator implements ContainerPop FileSourceDataComputer.class, SourceLineReadersFactory.class, QProfileStatusRepositoryImpl.class, + IssueChangesToDeleteRepository.class, // issues RuleRepositoryImpl.class, 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 a3a58c072a6..2242c66eabf 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 @@ -22,7 +22,7 @@ package org.sonar.ce.task.projectanalysis.issue; import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; +import java.util.Comparator; import java.util.Date; import java.util.List; import java.util.Map; @@ -50,9 +50,12 @@ import static java.util.stream.Collectors.groupingBy; import static java.util.stream.Collectors.toList; import static org.sonar.api.issue.Issue.STATUS_CLOSED; import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; +import static org.sonar.server.issue.IssueFieldsSetter.FROM_BRANCH; +import static org.sonar.server.issue.IssueFieldsSetter.STATUS; public class ComponentIssuesLoader { private static final int DEFAULT_CLOSED_ISSUES_MAX_AGE = 30; + static final int NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP = 15; private static final String PROPERTY_CLOSED_ISSUE_MAX_AGE = "sonar.issuetracking.closedissues.maxage"; private final DbClient dbClient; @@ -60,9 +63,10 @@ public class ComponentIssuesLoader { private final ActiveRulesHolder activeRulesHolder; private final System2 system2; private final int closedIssueMaxAge; + private final IssueChangesToDeleteRepository issueChangesToDeleteRepository; public ComponentIssuesLoader(DbClient dbClient, RuleRepository ruleRepository, ActiveRulesHolder activeRulesHolder, - Configuration configuration, System2 system2) { + Configuration configuration, System2 system2, IssueChangesToDeleteRepository issueChangesToDeleteRepository) { this.dbClient = dbClient; this.activeRulesHolder = activeRulesHolder; this.ruleRepository = ruleRepository; @@ -71,16 +75,7 @@ public class ComponentIssuesLoader { .map(ComponentIssuesLoader::safelyParseClosedIssueMaxAge) .filter(i -> i >= 0) .orElse(DEFAULT_CLOSED_ISSUES_MAX_AGE); - } - - private static Integer safelyParseClosedIssueMaxAge(String str) { - try { - return Integer.parseInt(str); - } catch (NumberFormatException e) { - Loggers.get(ComponentIssuesLoader.class) - .warn("Value of property {} should be an integer >= 0: {}", PROPERTY_CLOSED_ISSUE_MAX_AGE, str); - return null; - } + this.issueChangesToDeleteRepository = issueChangesToDeleteRepository; } public List loadOpenIssues(String componentUuid) { @@ -92,24 +87,25 @@ public class ComponentIssuesLoader { public List loadOpenIssuesWithChanges(String componentUuid) { try (DbSession dbSession = dbClient.openSession(false)) { List result = loadOpenIssues(componentUuid, dbSession); - return loadChanges(dbSession, result); } } + /** + * Loads all comments and changes EXCEPT old changes involving a status change or a move between branches. + */ 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)); - issues.forEach(i -> setChanges(changeDtoByIssueKey, i)); + issues.forEach(i -> setFilteredChanges(changeDtoByIssueKey, i)); return new ArrayList<>(issues); } /** - * Loads the most recent diff changes of the specified issues which contain the latest status and resolution of the - * issue. + * Loads the most recent diff changes of the specified issues which contain the latest status and resolution of the issue. */ public void loadLatestDiffChangesForReopeningOfClosedIssues(Collection issues) { if (issues.isEmpty()) { @@ -121,6 +117,42 @@ public class ComponentIssuesLoader { } } + /** + * Load closed issues for the specified Component, which have at least one line diff in changelog AND are + * not 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. + *

+ * To not depend on purge configuration of closed issues, only issues which close date is less than 30 days ago at + * 00H00 are returned. + */ + public List loadClosedIssues(String componentUuid) { + if (closedIssueMaxAge == 0) { + return emptyList(); + } + + Date date = new Date(system2.now()); + long closeDateAfter = date.toInstant() + .minus(closedIssueMaxAge, ChronoUnit.DAYS) + .truncatedTo(ChronoUnit.DAYS) + .toEpochMilli(); + try (DbSession dbSession = dbClient.openSession(false)) { + return loadClosedIssues(dbSession, componentUuid, closeDateAfter); + } + } + + private static Integer safelyParseClosedIssueMaxAge(String str) { + try { + return Integer.parseInt(str); + } catch (NumberFormatException e) { + Loggers.get(ComponentIssuesLoader.class) + .warn("Value of property {} should be an integer >= 0: {}", PROPERTY_CLOSED_ISSUE_MAX_AGE, str); + return null; + } + } + /** * To be efficient both in term of memory and speed: *

    @@ -128,36 +160,20 @@ public class ComponentIssuesLoader { *
  • data from DB is streamed
    • *
  • only the latest change(s) with status and resolution are added to the {@link DefaultIssue} objects
    • *
+ * + * While loading the changes for the issues, this class will also collect old status changes that should be deleted. */ private void loadLatestDiffChangesForReopeningOfClosedIssues(DbSession dbSession, Collection issues) { Map issuesByKey = issues.stream().collect(uniqueIndex(DefaultIssue::key)); + CollectIssueChangesToDeleteResultHandler collectChangesToDelete = new CollectIssueChangesToDeleteResultHandler(issueChangesToDeleteRepository); + CollectLastStatusAndResolution collectLastStatusAndResolution = new CollectLastStatusAndResolution(issuesByKey); - dbClient.issueChangeDao() - .scrollDiffChangesOfIssues(dbSession, issuesByKey.keySet(), new ResultHandler() { - private DefaultIssue currentIssue = null; - private boolean previousStatusFound = false; - private boolean previousResolutionFound = false; - - @Override - public void handleResult(ResultContext resultContext) { - IssueChangeDto issueChangeDto = resultContext.getResultObject(); - if (currentIssue == null || !currentIssue.key().equals(issueChangeDto.getIssueKey())) { - currentIssue = issuesByKey.get(issueChangeDto.getIssueKey()); - previousStatusFound = false; - previousResolutionFound = false; - } + dbClient.issueChangeDao().scrollDiffChangesOfIssues(dbSession, issuesByKey.keySet(), resultContext -> { + IssueChangeDto issueChangeDto = resultContext.getResultObject(); + FieldDiffs fieldDiffs = issueChangeDto.toFieldDiffs(); - if (currentIssue != null) { - FieldDiffs fieldDiffs = issueChangeDto.toFieldDiffs(); - boolean hasPreviousStatus = fieldDiffs.get("status") != null; - boolean hasPreviousResolution = fieldDiffs.get("resolution") != null; - if ((!previousStatusFound && hasPreviousStatus) || (!previousResolutionFound && hasPreviousResolution)) { - currentIssue.addChange(fieldDiffs); - } - previousStatusFound |= hasPreviousStatus; - previousResolutionFound |= hasPreviousResolution; - } - } + collectChangesToDelete.handle(issueChangeDto, fieldDiffs); + collectLastStatusAndResolution.handle(issueChangeDto, fieldDiffs); }); } @@ -176,24 +192,44 @@ public class ComponentIssuesLoader { issue.setSelectedAt(System.currentTimeMillis()); result.add(issue); }); - return Collections.unmodifiableList(result); + return unmodifiableList(result); } - private static void setChanges(Map> changeDtoByIssueKey, DefaultIssue i) { - changeDtoByIssueKey.computeIfAbsent(i.key(), k -> emptyList()) - .forEach(c -> addChangeOrComment(i, c)); - } + private void setFilteredChanges(Map> changeDtoByIssueKey, DefaultIssue i) { + List sortedIssueChanges = changeDtoByIssueKey.computeIfAbsent(i.key(), k -> emptyList()).stream() + .sorted(Comparator.comparing(IssueChangeDto::getIssueChangeCreationDate).reversed()) + .collect(toList()); - private static void addChangeOrComment(DefaultIssue i, IssueChangeDto c) { - switch (c.getChangeType()) { - case IssueChangeDto.TYPE_FIELD_CHANGE: - i.addChange(c.toFieldDiffs()); - break; - case IssueChangeDto.TYPE_COMMENT: - i.addComment(c.toComment()); - break; - default: - throw new IllegalStateException("Unknown change type: " + c.getChangeType()); + int statusCount = 0; + int branchCount = 0; + + for (IssueChangeDto c : sortedIssueChanges) { + switch (c.getChangeType()) { + case IssueChangeDto.TYPE_FIELD_CHANGE: + FieldDiffs fieldDiffs = c.toFieldDiffs(); + // To limit the amount of changes that copied issues carry over, we only keep the 15 most recent changes that involve a status change or a move between branches. + if (fieldDiffs.get(STATUS) != null) { + statusCount++; + if (statusCount > NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP) { + issueChangesToDeleteRepository.add(c.getUuid()); + break; + } + } + if (fieldDiffs.get(FROM_BRANCH) != null) { + branchCount++; + if (branchCount > NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP) { + issueChangesToDeleteRepository.add(c.getUuid()); + break; + } + } + i.addChange(c.toFieldDiffs()); + break; + case IssueChangeDto.TYPE_COMMENT: + i.addComment(c.toComment()); + break; + default: + throw new IllegalStateException("Unknown change type: " + c.getChangeType()); + } } } @@ -201,32 +237,6 @@ public class ComponentIssuesLoader { return activeRulesHolder.get(ruleKey).isPresent(); } - /** - * Load closed issues for the specified Component, which have at least one line diff in changelog AND are - * not 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. - *

- * To not depend on purge configuration of closed issues, only issues which close date is less than 30 days ago at - * 00H00 are returned. - */ - public List loadClosedIssues(String componentUuid) { - if (closedIssueMaxAge == 0) { - return emptyList(); - } - - Date date = new Date(system2.now()); - long closeDateAfter = date.toInstant() - .minus(closedIssueMaxAge, ChronoUnit.DAYS) - .truncatedTo(ChronoUnit.DAYS) - .toEpochMilli(); - try (DbSession dbSession = dbClient.openSession(false)) { - return loadClosedIssues(dbSession, componentUuid, closeDateAfter); - } - } - private static List loadClosedIssues(DbSession dbSession, String componentUuid, long closeDateAfter) { ClosedIssuesResultHandler handler = new ClosedIssuesResultHandler(); dbSession.getMapper(IssueMapper.class).scrollClosedByComponentUuid(componentUuid, closeDateAfter, handler); @@ -266,4 +276,67 @@ public class ComponentIssuesLoader { issues.add(issue); } } + + private static class CollectLastStatusAndResolution { + private final Map issuesByKey; + private DefaultIssue currentIssue = null; + private boolean previousStatusFound = false; + private boolean previousResolutionFound = false; + + private CollectLastStatusAndResolution(Map issuesByKey) { + this.issuesByKey = issuesByKey; + } + + /** + * Assumes that changes are sorted by issue key and date desc + */ + public void handle(IssueChangeDto issueChangeDto, FieldDiffs fieldDiffs) { + if (currentIssue == null || !currentIssue.key().equals(issueChangeDto.getIssueKey())) { + currentIssue = issuesByKey.get(issueChangeDto.getIssueKey()); + previousStatusFound = false; + previousResolutionFound = false; + } + + if (currentIssue != null) { + boolean hasPreviousStatus = fieldDiffs.get("status") != null; + boolean hasPreviousResolution = fieldDiffs.get("resolution") != null; + + if ((!previousStatusFound && hasPreviousStatus) || (!previousResolutionFound && hasPreviousResolution)) { + currentIssue.addChange(fieldDiffs); + } + previousStatusFound |= hasPreviousStatus; + previousResolutionFound |= hasPreviousResolution; + } + } + } + + /** + * Collects issue changes related to status changes that should be cleaned up. + * If we have more than 15 status changes recorded for an issue, only the 15 most recent ones should be kept. + */ + private static class CollectIssueChangesToDeleteResultHandler { + private final IssueChangesToDeleteRepository issueChangesToDeleteRepository; + private String currentIssueKey; + private int statusChangeCount; + + public CollectIssueChangesToDeleteResultHandler(IssueChangesToDeleteRepository issueChangesToDeleteRepository) { + this.issueChangesToDeleteRepository = issueChangesToDeleteRepository; + } + + /** + * Assumes that changes are sorted by issue key and date desc + */ + public void handle(IssueChangeDto dto, FieldDiffs fieldDiffs) { + if (currentIssueKey == null || !currentIssueKey.equals(dto.getIssueKey())) { + currentIssueKey = dto.getIssueKey(); + statusChangeCount = 0; + } + if (fieldDiffs.get(STATUS) != null) { + statusChangeCount++; + if (statusChangeCount > NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP) { + issueChangesToDeleteRepository.add(dto.getUuid()); + } + } + } + } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepository.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepository.java new file mode 100644 index 00000000000..f3ad70f64a4 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepository.java @@ -0,0 +1,37 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 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.HashSet; +import java.util.Set; + +import static java.util.Collections.unmodifiableSet; + +public class IssueChangesToDeleteRepository { + private final Set uuids = new HashSet<>(); + + public void add(String uuid) { + uuids.add(uuid); + } + + public Set getUuids() { + return unmodifiableSet(uuids); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestSourceBranchMerger.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestSourceBranchMerger.java index c8215601656..ab621aad156 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestSourceBranchMerger.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestSourceBranchMerger.java @@ -32,8 +32,7 @@ public class PullRequestSourceBranchMerger { private final IssueLifecycle issueLifecycle; private final TrackerSourceBranchInputFactory sourceBranchInputFactory; - public PullRequestSourceBranchMerger(Tracker tracker, IssueLifecycle issueLifecycle, - TrackerSourceBranchInputFactory sourceBranchInputFactory) { + public PullRequestSourceBranchMerger(Tracker tracker, IssueLifecycle issueLifecycle, TrackerSourceBranchInputFactory sourceBranchInputFactory) { this.tracker = tracker; this.issueLifecycle = issueLifecycle; this.sourceBranchInputFactory = sourceBranchInputFactory; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStep.java new file mode 100644 index 00000000000..8cbdafa2435 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStep.java @@ -0,0 +1,57 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 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.step; + +import java.util.Set; +import org.sonar.ce.task.projectanalysis.issue.IssueChangesToDeleteRepository; +import org.sonar.ce.task.step.ComputationStep; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; + +public class CleanIssueChangesStep implements ComputationStep { + private final IssueChangesToDeleteRepository issueChangesToDeleteRepository; + private final DbClient dbClient; + + public CleanIssueChangesStep(IssueChangesToDeleteRepository issueChangesToDeleteRepository, DbClient dbClient) { + this.issueChangesToDeleteRepository = issueChangesToDeleteRepository; + this.dbClient = dbClient; + } + + @Override + public void execute(Context context) { + Set uuids = issueChangesToDeleteRepository.getUuids(); + context.getStatistics().add("changes", uuids.size()); + + if (uuids.isEmpty()) { + return; + } + + try (DbSession dbSession = dbClient.openSession(false)) { + + dbClient.issueChangeDao().deleteByUuids(dbSession, issueChangesToDeleteRepository.getUuids()); + dbSession.commit(); + } + } + + @Override + public String getDescription() { + return "Delete issue changes"; + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ReportComputationSteps.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ReportComputationSteps.java index b7f17b328b8..de5442922b0 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ReportComputationSteps.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ReportComputationSteps.java @@ -96,6 +96,7 @@ public class ReportComputationSteps extends AbstractComputationSteps { PersistDuplicationDataStep.class, PersistAdHocRulesStep.class, PersistIssuesStep.class, + CleanIssueChangesStep.class, PersistProjectLinksStep.class, PersistEventsStep.class, PersistFileSourcesStep.class, 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 index 193433a31e3..97c05b04114 100644 --- 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 @@ -19,7 +19,6 @@ */ package org.sonar.ce.task.projectanalysis.issue; -import com.google.common.collect.ImmutableList; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; @@ -29,7 +28,9 @@ import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Random; +import java.util.function.Consumer; import java.util.stream.IntStream; +import java.util.stream.LongStream; import javax.annotation.Nullable; import org.junit.Rule; import org.junit.Test; @@ -39,15 +40,18 @@ import org.sonar.api.config.internal.MapSettings; import org.sonar.api.issue.Issue; import org.sonar.api.utils.System2; import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.DefaultIssueComment; 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.IssueChangeDto; import org.sonar.db.issue.IssueDto; import org.sonar.db.rule.RuleDefinitionDto; import static java.util.Collections.emptyList; +import static java.util.Collections.singleton; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -57,6 +61,7 @@ import static org.sonar.api.issue.Issue.STATUS_CLOSED; import static org.sonar.api.rules.RuleType.CODE_SMELL; import static org.sonar.api.utils.DateUtils.addDays; import static org.sonar.api.utils.DateUtils.parseDateTime; +import static org.sonar.ce.task.projectanalysis.issue.ComponentIssuesLoader.NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP; @RunWith(DataProviderRunner.class) public class ComponentIssuesLoaderTest { @@ -66,8 +71,9 @@ public class ComponentIssuesLoaderTest { @Rule public DbTester db = DbTester.create(System2.INSTANCE); - private DbClient dbClient = db.getDbClient(); - private System2 system2 = mock(System2.class); + private final DbClient dbClient = db.getDbClient(); + private final System2 system2 = mock(System2.class); + private final IssueChangesToDeleteRepository issueChangesToDeleteRepository = new IssueChangesToDeleteRepository(); @Test public void loadClosedIssues_returns_single_DefaultIssue_by_issue_based_on_first_row() { @@ -210,19 +216,32 @@ public class ComponentIssuesLoaderTest { DbClient dbClient = mock(DbClient.class); Configuration configuration = newConfiguration("0"); String componentUuid = randomAlphabetic(15); - ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, - null /* not used in loadClosedIssues */, null /* not used in loadClosedIssues */, configuration, system2); + ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, null, null, configuration, system2, issueChangesToDeleteRepository); assertThat(underTest.loadClosedIssues(componentUuid)).isEmpty(); verifyNoInteractions(dbClient, system2); } + @Test + public void loadLatestDiffChangesForReopeningOfClosedIssues_collects_issue_changes_to_delete() { + IssueDto issue = db.issues().insert(); + for (long i = 0; i < NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP + 5; i++) { + db.issues().insertChange(issue, diffIssueChangeModifier(i, "status")); + } + // should not be deleted + db.issues().insertChange(issue, diffIssueChangeModifier(-1, "other")); + + ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, null, null, newConfiguration("0"), null, issueChangesToDeleteRepository); + + underTest.loadLatestDiffChangesForReopeningOfClosedIssues(singleton(new DefaultIssue().setKey(issue.getKey()))); + assertThat(issueChangesToDeleteRepository.getUuids()).containsOnly("0", "1", "2", "3", "4"); + } + @Test public void loadLatestDiffChangesForReopeningOfClosedIssues_does_not_query_DB_if_issue_list_is_empty() { DbClient dbClient = mock(DbClient.class); - ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, - null /* not used in method */, null /* not used in method */, newConfiguration("0"), null /* not used by method */); + ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, null, null, newConfiguration("0"), null, issueChangesToDeleteRepository); underTest.loadLatestDiffChangesForReopeningOfClosedIssues(emptyList()); @@ -232,18 +251,14 @@ public class ComponentIssuesLoaderTest { @Test @UseDataProvider("statusOrResolutionFieldName") public void loadLatestDiffChangesForReopeningOfClosedIssues_add_diff_change_with_most_recent_status_or_resolution(String statusOrResolutionFieldName) { - ComponentDto project = db.components().insertPublicProject(); - ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project)); - RuleDefinitionDto rule = db.rules().insert(); - IssueDto issue = db.issues().insert(rule, project, file); + IssueDto issue = db.issues().insert(); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith(statusOrResolutionFieldName, "val1")).setIssueChangeCreationDate(5)); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith(statusOrResolutionFieldName, "val2")).setIssueChangeCreationDate(20)); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith(statusOrResolutionFieldName, "val3")).setIssueChangeCreationDate(13)); - ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, - null /* not used in method */, null /* not used in method */, newConfiguration("0"), null /* not used by method */); + ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, null, null, newConfiguration("0"), null, issueChangesToDeleteRepository); DefaultIssue defaultIssue = new DefaultIssue().setKey(issue.getKey()); - underTest.loadLatestDiffChangesForReopeningOfClosedIssues(ImmutableList.of(defaultIssue)); + underTest.loadLatestDiffChangesForReopeningOfClosedIssues(singleton(defaultIssue)); assertThat(defaultIssue.changes()) .hasSize(1); @@ -255,19 +270,15 @@ public class ComponentIssuesLoaderTest { @Test public void loadLatestDiffChangesForReopeningOfClosedIssues_add_single_diff_change_when_most_recent_status_and_resolution_is_the_same_diff() { - ComponentDto project = db.components().insertPublicProject(); - ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project)); - RuleDefinitionDto rule = db.rules().insert(); - IssueDto issue = db.issues().insert(rule, project, file); + IssueDto issue = db.issues().insert(); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus1")).setIssueChangeCreationDate(5)); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus2")).setIssueChangeCreationDate(19)); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus3", "resolution", "valRes3")).setIssueChangeCreationDate(20)); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("resolution", "valRes4")).setIssueChangeCreationDate(13)); - ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, - null /* not used in method */, null /* not used in method */, newConfiguration("0"), null /* not used by method */); + ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, null, null, newConfiguration("0"), null, issueChangesToDeleteRepository); DefaultIssue defaultIssue = new DefaultIssue().setKey(issue.getKey()); - underTest.loadLatestDiffChangesForReopeningOfClosedIssues(ImmutableList.of(defaultIssue)); + underTest.loadLatestDiffChangesForReopeningOfClosedIssues(singleton(defaultIssue)); assertThat(defaultIssue.changes()) .hasSize(1); @@ -283,19 +294,16 @@ public class ComponentIssuesLoaderTest { @Test public void loadLatestDiffChangesForReopeningOfClosedIssues_adds_2_diff_changes_if_most_recent_status_and_resolution_are_not_the_same_diff() { - ComponentDto project = db.components().insertPublicProject(); - ComponentDto file = db.components().insertComponent(ComponentTesting.newFileDto(project)); - RuleDefinitionDto rule = db.rules().insert(); - IssueDto issue = db.issues().insert(rule, project, file); + IssueDto issue = db.issues().insert(); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus1")).setIssueChangeCreationDate(5)); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus2", "resolution", "valRes2")).setIssueChangeCreationDate(19)); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("status", "valStatus3")).setIssueChangeCreationDate(20)); db.issues().insertChange(issue, t -> t.setChangeData(randomDiffWith("resolution", "valRes4")).setIssueChangeCreationDate(13)); - ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, - null /* not used in method */, null /* not used in method */, newConfiguration("0"), null /* not used by method */); + ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, null /* not used in method */, null /* not used in method */, + newConfiguration("0"), null /* not used by method */, issueChangesToDeleteRepository); DefaultIssue defaultIssue = new DefaultIssue().setKey(issue.getKey()); - underTest.loadLatestDiffChangesForReopeningOfClosedIssues(ImmutableList.of(defaultIssue)); + underTest.loadLatestDiffChangesForReopeningOfClosedIssues(singleton(defaultIssue)); assertThat(defaultIssue.changes()) .hasSize(2); @@ -309,6 +317,52 @@ public class ComponentIssuesLoaderTest { .hasSize(1); } + @Test + public void loadChanges_should_filter_out_old_status_changes() { + IssueDto issue = db.issues().insert(); + for (int i = 0; i < NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP + 1; i++) { + db.issues().insertChange(issue, diffIssueChangeModifier(i, "status")); + } + // these are kept + db.issues().insertChange(issue, diffIssueChangeModifier(NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP + 1, "other")); + db.issues().insertChange(issue, t -> t + .setChangeType(IssueChangeDto.TYPE_COMMENT) + .setKey("comment1")); + + ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, null, null, newConfiguration("0"), null, issueChangesToDeleteRepository); + DefaultIssue defaultIssue = new DefaultIssue().setKey(issue.getKey()); + underTest.loadChanges(db.getSession(), singleton(defaultIssue)); + + assertThat(defaultIssue.changes()) + .extracting(d -> d.creationDate().getTime()) + .containsOnly(LongStream.rangeClosed(1, NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP + 1).boxed().toArray(Long[]::new)); + assertThat(defaultIssue.defaultIssueComments()).extracting(DefaultIssueComment::key).containsOnly("comment1"); + assertThat(issueChangesToDeleteRepository.getUuids()).containsOnly("0"); + } + + @Test + public void loadChanges_should_filter_out_old_from_branch_changes() { + IssueDto issue = db.issues().insert(); + for (int i = 0; i < NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP + 1; i++) { + db.issues().insertChange(issue, diffIssueChangeModifier(i, "from_branch")); + } + + ComponentIssuesLoader underTest = new ComponentIssuesLoader(dbClient, null, null, newConfiguration("0"), null, issueChangesToDeleteRepository); + DefaultIssue defaultIssue = new DefaultIssue().setKey(issue.getKey()); + underTest.loadChanges(db.getSession(), singleton(defaultIssue)); + assertThat(defaultIssue.changes()) + .extracting(d -> d.creationDate().getTime()) + .containsOnly(LongStream.rangeClosed(1, NUMBER_STATUS_AND_BRANCH_CHANGES_TO_KEEP).boxed().toArray(Long[]::new)); + assertThat(issueChangesToDeleteRepository.getUuids()).containsOnly("0"); + } + + private Consumer diffIssueChangeModifier(long created, String field) { + return issueChangeDto -> issueChangeDto + .setChangeData(new FieldDiffs().setDiff(field, "A", "B").toEncodedString()) + .setIssueChangeCreationDate(created) + .setUuid(String.valueOf(created)); + } + private static boolean hasValue(@Nullable FieldDiffs.Diff t, String value) { if (t == null) { return false; @@ -371,8 +425,8 @@ public class ComponentIssuesLoaderTest { } private ComponentIssuesLoader newComponentIssuesLoader(Configuration configuration) { - return new ComponentIssuesLoader(dbClient, - null /* not used in loadClosedIssues */, null /* not used in loadClosedIssues */, configuration, system2); + return new ComponentIssuesLoader(dbClient, null /* not used in loadClosedIssues */, null /* not used in loadClosedIssues */, + configuration, system2, issueChangesToDeleteRepository); } private static Configuration newEmptySettings() { 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 a2f8d33ec5b..986f6d1f9af 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 @@ -131,7 +131,7 @@ public class IntegrateIssuesVisitorTest { private ArgumentCaptor defaultIssueCaptor; private final ComponentIssuesLoader issuesLoader = new ComponentIssuesLoader(dbTester.getDbClient(), ruleRepositoryRule, activeRulesHolderRule, new MapSettings().asConfig(), - System2.INSTANCE); + System2.INSTANCE, mock(IssueChangesToDeleteRepository.class)); private IssueTrackingDelegator trackingDelegator; private TrackerExecution tracker; private PullRequestTrackerExecution prBranchTracker; diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepositoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepositoryTest.java new file mode 100644 index 00000000000..f98ad57b909 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepositoryTest.java @@ -0,0 +1,35 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 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 static org.assertj.core.api.Assertions.assertThat; + +public class IssueChangesToDeleteRepositoryTest { + private final IssueChangesToDeleteRepository repository = new IssueChangesToDeleteRepository(); + + @Test + public void get_returns_all_issues_added() { + repository.add("a"); + repository.add("b"); + assertThat(repository.getUuids()).containsOnly("a", "b"); + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ProjectTrackerBaseLazyInputTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ProjectTrackerBaseLazyInputTest.java index 4ea39c07074..9b93522b552 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ProjectTrackerBaseLazyInputTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ProjectTrackerBaseLazyInputTest.java @@ -67,7 +67,7 @@ public class ProjectTrackerBaseLazyInputTest { private RuleDefinitionDto rule; private ComponentDto rootProjectDto; private ComponentIssuesLoader issuesLoader = new ComponentIssuesLoader(dbTester.getDbClient(), ruleRepositoryRule, activeRulesHolderRule, new MapSettings().asConfig(), - System2.INSTANCE); + System2.INSTANCE, mock(IssueChangesToDeleteRepository.class)); private ReportModulesPath reportModulesPath; @Before @@ -146,7 +146,7 @@ public class ProjectTrackerBaseLazyInputTest { } @Test - public void empty_path_if_module_missing_in_report_and_db_and_for_slash_folder () { + public void empty_path_if_module_missing_in_report_and_db_and_for_slash_folder() { ComponentDto module = dbTester.components().insertComponent(newModuleDto(rootProjectDto).setPath(null)); when(reportModulesPath.get()).thenReturn(Collections.emptyMap()); ComponentDto folder = dbTester.components().insertComponent(newDirectory(module, "/")); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/SiblingsIssueMergerTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/SiblingsIssueMergerTest.java index 6c7b7202a65..776b24352f0 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/SiblingsIssueMergerTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/SiblingsIssueMergerTest.java @@ -82,9 +82,9 @@ public class SiblingsIssueMergerTest { private static final org.sonar.ce.task.projectanalysis.component.Component FILE_1 = builder( org.sonar.ce.task.projectanalysis.component.Component.Type.FILE, FILE_1_REF) - .setKey(FILE_1_KEY) - .setUuid(FILE_1_UUID) - .build(); + .setKey(FILE_1_KEY) + .setUuid(FILE_1_UUID) + .build(); private final SimpleTracker tracker = new SimpleTracker<>(); private SiblingsIssueMerger copier; @@ -100,7 +100,8 @@ public class SiblingsIssueMergerTest { @Before public void setUp() { DbClient dbClient = db.getDbClient(); - ComponentIssuesLoader componentIssuesLoader = new ComponentIssuesLoader(dbClient, null, null, new MapSettings().asConfig(), System2.INSTANCE); + ComponentIssuesLoader componentIssuesLoader = new ComponentIssuesLoader(dbClient, null, null, new MapSettings().asConfig(), System2.INSTANCE, + mock(IssueChangesToDeleteRepository.class)); copier = new SiblingsIssueMerger(new SiblingsIssuesLoader(new SiblingComponentsWithOpenIssues(treeRootHolder, metadataHolder, dbClient), dbClient, componentIssuesLoader), tracker, issueLifecycle); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStepTest.java new file mode 100644 index 00000000000..3a00fb972c1 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStepTest.java @@ -0,0 +1,66 @@ +/* + * SonarQube + * Copyright (C) 2009-2022 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.step; + +import org.junit.Rule; +import org.junit.Test; +import org.sonar.ce.task.projectanalysis.issue.IssueChangesToDeleteRepository; +import org.sonar.ce.task.step.TestComputationStepContext; +import org.sonar.db.DbTester; +import org.sonar.db.issue.IssueChangeDto; +import org.sonar.db.issue.IssueDto; + +import static java.util.Collections.singleton; +import static org.assertj.core.api.Assertions.assertThat; + +public class CleanIssueChangesStepTest { + @Rule + public DbTester db = DbTester.create(); + private final IssueChangesToDeleteRepository repository = new IssueChangesToDeleteRepository(); + private final CleanIssueChangesStep cleanIssueChangesStep = new CleanIssueChangesStep(repository, db.getDbClient()); + private final TestComputationStepContext context = new TestComputationStepContext(); + + @Test + public void steps_deletes_all_changes_in_repository() { + IssueDto issue1 = db.issues().insert(); + IssueChangeDto change1 = db.issues().insertChange(issue1); + IssueChangeDto change2 = db.issues().insertChange(issue1); + + repository.add(change1.getUuid()); + + cleanIssueChangesStep.execute(context); + assertThat(db.getDbClient().issueChangeDao().selectByIssueKeys(db.getSession(), singleton(issue1.getKey()))) + .extracting(IssueChangeDto::getUuid) + .containsOnly(change2.getUuid()); + } + + @Test + public void steps_does_nothing_if_no_uuid() { + IssueDto issue1 = db.issues().insert(); + IssueChangeDto change1 = db.issues().insertChange(issue1); + IssueChangeDto change2 = db.issues().insertChange(issue1); + + cleanIssueChangesStep.execute(context); + + assertThat(db.getDbClient().issueChangeDao().selectByIssueKeys(db.getSession(), singleton(issue1.getKey()))) + .extracting(IssueChangeDto::getUuid) + .containsOnly(change1.getUuid(), change2.getUuid()); + } +} diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeDao.java index 145cf21cf14..911ec3a5263 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeDao.java @@ -22,6 +22,7 @@ package org.sonar.db.issue; import java.util.Collection; import java.util.List; import java.util.Optional; +import java.util.Set; import org.apache.ibatis.session.ResultHandler; import org.sonar.core.issue.FieldDiffs; import org.sonar.core.util.stream.MoreCollectors; @@ -31,6 +32,7 @@ import org.sonar.db.DbSession; import static java.util.Collections.singletonList; import static org.sonar.db.DatabaseUtils.executeLargeInputs; import static org.sonar.db.DatabaseUtils.executeLargeInputsWithoutOutput; +import static org.sonar.db.DatabaseUtils.executeLargeUpdates; public class IssueChangeDao implements Dao { @@ -65,13 +67,18 @@ public class IssueChangeDao implements Dao { mapper(session).insert(change); } - public boolean delete(DbSession session, String key) { + public boolean deleteByKey(DbSession session, String key) { IssueChangeMapper mapper = mapper(session); int count = mapper.delete(key); session.commit(); return count == 1; } + public void deleteByUuids(DbSession session, Set uuids) { + IssueChangeMapper mapper = mapper(session); + executeLargeUpdates(uuids, mapper::deleteByUuids); + } + public boolean update(DbSession dbSession, IssueChangeDto change) { int count = mapper(dbSession).update(change); return count == 1; diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeMapper.java index 40952ca9b54..f0768738ddb 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeMapper.java @@ -19,6 +19,7 @@ */ package org.sonar.db.issue; +import java.util.Collection; import java.util.List; import javax.annotation.CheckForNull; import org.apache.ibatis.annotations.Param; @@ -30,6 +31,8 @@ public interface IssueChangeMapper { int delete(String key); + void deleteByUuids(@Param("changeUuids") Collection uuids); + int update(IssueChangeDto change); @CheckForNull diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueChangeMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueChangeMapper.xml index 7b934fbdb19..9936b5bac62 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueChangeMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueChangeMapper.xml @@ -29,6 +29,13 @@ delete from issue_changes where kee=#{id} + + delete from issue_changes where uuid in + + #{changeUuid, jdbcType=VARCHAR} + + + update issue_changes set change_data=#{changeData}, updated_at=#{updatedAt,jdbcType=BIGINT} where kee=#{kee,jdbcType=VARCHAR} diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java index 0d71f7b569b..40879d7a512 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java @@ -22,6 +22,7 @@ package org.sonar.db.issue; import java.util.ArrayList; import java.util.List; import java.util.Optional; +import java.util.Set; import java.util.stream.Stream; import org.apache.ibatis.session.ResultContext; import org.apache.ibatis.session.ResultHandler; @@ -35,6 +36,7 @@ import org.sonar.db.DbTester; import static com.google.common.collect.ImmutableList.of; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; +import static java.util.Collections.singleton; import static java.util.Collections.singletonList; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; @@ -108,17 +110,29 @@ public class IssueChangeDaoTest { IssueChangeDto issueChange1 = db.issues().insertChange(issue); IssueChangeDto issueChange2 = db.issues().insertChange(issue); - assertThat(underTest.delete(db.getSession(), issueChange1.getKey())).isTrue(); + assertThat(underTest.deleteByKey(db.getSession(), issueChange1.getKey())).isTrue(); assertThat(db.countRowsOfTable(db.getSession(), "issue_changes")).isOne(); } + @Test + public void deleteByUuids() { + IssueDto issue = db.issues().insertIssue(); + IssueChangeDto issueChange1 = db.issues().insertChange(issue); + IssueChangeDto issueChange2 = db.issues().insertChange(issue); + IssueChangeDto issueChange3 = db.issues().insertChange(issue); + + underTest.deleteByUuids(db.getSession(), Set.of(issueChange1.getUuid(), issueChange2.getUuid())); + assertThat(underTest.selectByIssueKeys(db.getSession(), singleton(issue.getKey()))).extracting(IssueChangeDto::getIssueKey).containsOnly(issue.getKey()); + assertThat(db.countRowsOfTable(db.getSession(), "issue_changes")).isOne(); + } + @Test public void delete_unknown_key() { IssueDto issue = db.issues().insertIssue(); db.issues().insertChange(issue); - assertThat(underTest.delete(db.getSession(), "UNKNOWN")).isFalse(); + assertThat(underTest.deleteByKey(db.getSession(), "UNKNOWN")).isFalse(); } @Test @@ -181,7 +195,7 @@ public class IssueChangeDaoTest { .setIssueKey("other_issue_uuid") .setChangeData("new comment") .setUpdatedAt(DateUtils.parseDate("2013-06-30").getTime()))) - .isFalse(); + .isFalse(); assertThat(underTest.selectByIssueKeys(db.getSession(), singletonList(issue.getKey()))) .extracting(IssueChangeDto::getKey, IssueChangeDto::getIssueKey, IssueChangeDto::getChangeData, IssueChangeDto::getChangeType, diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/DeleteCommentAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/DeleteCommentAction.java index 4318df407b0..2d747669ab2 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/DeleteCommentAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/DeleteCommentAction.java @@ -88,6 +88,6 @@ public class DeleteCommentAction implements HotspotsWsAction { } private void deleteComment(DbSession dbSession, String commentKey) { - dbClient.issueChangeDao().delete(dbSession, commentKey); + dbClient.issueChangeDao().deleteByKey(dbSession, commentKey); } } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/DeleteCommentAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/DeleteCommentAction.java index f3703179953..7089228fa13 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/DeleteCommentAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/DeleteCommentAction.java @@ -91,7 +91,7 @@ public class DeleteCommentAction implements IssuesWsAction { } private void deleteComment(DbSession dbSession, CommentData commentData) { - dbClient.issueChangeDao().delete(dbSession, commentData.getIssueChangeDto().getKey()); + dbClient.issueChangeDao().deleteByKey(dbSession, commentData.getIssueChangeDto().getKey()); dbSession.commit(); } -- 2.39.5