]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-15321 Large number of entries in 'issue_changes'
authorDuarte Meneses <duarte.meneses@sonarsource.com>
Tue, 29 Mar 2022 20:07:36 +0000 (15:07 -0500)
committersonartech <sonartech@sonarsource.com>
Thu, 7 Apr 2022 20:02:52 +0000 (20:02 +0000)
18 files changed:
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoader.java
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepository.java [new file with mode: 0644]
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/PullRequestSourceBranchMerger.java
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStep.java [new file with mode: 0644]
server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/ReportComputationSteps.java
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ComponentIssuesLoaderTest.java
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IntegrateIssuesVisitorTest.java
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueChangesToDeleteRepositoryTest.java [new file with mode: 0644]
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/ProjectTrackerBaseLazyInputTest.java
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/SiblingsIssueMergerTest.java
server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/CleanIssueChangesStepTest.java [new file with mode: 0644]
server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeDao.java
server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueChangeMapper.java
server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueChangeMapper.xml
server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/DeleteCommentAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/DeleteCommentAction.java

index e236cfdd0b34ba1cb36f9f23c7f6c6f5386de829..c8045d67c544bbc47f079388e8b9c442b22f6b91 100644 (file)
@@ -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,
index a3a58c072a672304acbcee913ec6fb300b806117..2242c66eabf1ad0e9aa630ab2d104ba9b529d38b 100644 (file)
@@ -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<DefaultIssue> loadOpenIssues(String componentUuid) {
@@ -92,24 +87,25 @@ public class ComponentIssuesLoader {
   public List<DefaultIssue> loadOpenIssuesWithChanges(String componentUuid) {
     try (DbSession dbSession = dbClient.openSession(false)) {
       List<DefaultIssue> 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<DefaultIssue> loadChanges(DbSession dbSession, Collection<DefaultIssue> issues) {
     Map<String, List<IssueChangeDto>> 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<DefaultIssue> 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.
+   * <p>
+   * 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.
+   * <p>
+   * 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<DefaultIssue> 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:
    * <ul>
@@ -128,36 +160,20 @@ public class ComponentIssuesLoader {
    *   <li>data from DB is streamed</li>
    *   <li>only the latest change(s) with status and resolution are added to the {@link DefaultIssue} objects</li>
    * </ul>
+   *
+   * 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<DefaultIssue> issues) {
     Map<String, DefaultIssue> 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<IssueChangeDto>() {
-        private DefaultIssue currentIssue = null;
-        private boolean previousStatusFound = false;
-        private boolean previousResolutionFound = false;
-
-        @Override
-        public void handleResult(ResultContext<? extends IssueChangeDto> 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<String, List<IssueChangeDto>> changeDtoByIssueKey, DefaultIssue i) {
-    changeDtoByIssueKey.computeIfAbsent(i.key(), k -> emptyList())
-      .forEach(c -> addChangeOrComment(i, c));
-  }
+  private void setFilteredChanges(Map<String, List<IssueChangeDto>> changeDtoByIssueKey, DefaultIssue i) {
+    List<IssueChangeDto> 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.
-   * <p>
-   * 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.
-   * <p>
-   * 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<DefaultIssue> 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<DefaultIssue> 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<String, DefaultIssue> issuesByKey;
+    private DefaultIssue currentIssue = null;
+    private boolean previousStatusFound = false;
+    private boolean previousResolutionFound = false;
+
+    private CollectLastStatusAndResolution(Map<String, DefaultIssue> 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 (file)
index 0000000..f3ad70f
--- /dev/null
@@ -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<String> uuids = new HashSet<>();
+
+  public void add(String uuid) {
+    uuids.add(uuid);
+  }
+
+  public Set<String> getUuids() {
+    return unmodifiableSet(uuids);
+  }
+}
index c8215601656bcd856445ab3d2fde58fe26753be4..ab621aad156d2b8ef48572005cb06fba77dc38e0 100644 (file)
@@ -32,8 +32,7 @@ public class PullRequestSourceBranchMerger {
   private final IssueLifecycle issueLifecycle;
   private final TrackerSourceBranchInputFactory sourceBranchInputFactory;
 
-  public PullRequestSourceBranchMerger(Tracker<DefaultIssue, DefaultIssue> tracker, IssueLifecycle issueLifecycle,
-    TrackerSourceBranchInputFactory sourceBranchInputFactory) {
+  public PullRequestSourceBranchMerger(Tracker<DefaultIssue, DefaultIssue> 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 (file)
index 0000000..8cbdafa
--- /dev/null
@@ -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<String> 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";
+  }
+}
index b7f17b328b8e9dba0cc24c13d7966f963b89a77c..de5442922b05e8d11207748bc0c162b52dc05d6a 100644 (file)
@@ -96,6 +96,7 @@ public class ReportComputationSteps extends AbstractComputationSteps {
     PersistDuplicationDataStep.class,
     PersistAdHocRulesStep.class,
     PersistIssuesStep.class,
+    CleanIssueChangesStep.class,
     PersistProjectLinksStep.class,
     PersistEventsStep.class,
     PersistFileSourcesStep.class,
index 193433a31e3011dc7805452d0d78c2fc07d7f21a..97c05b0411488edae0f361aee6d7a9d19739c468 100644 (file)
@@ -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<IssueChangeDto> 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() {
index a2f8d33ec5b87f8d0cdf9a7263d5b0fa683f34e4..986f6d1f9af183537f959b4d6ea3d78cbe12fc8c 100644 (file)
@@ -131,7 +131,7 @@ public class IntegrateIssuesVisitorTest {
   private ArgumentCaptor<DefaultIssue> 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 (file)
index 0000000..f98ad57
--- /dev/null
@@ -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");
+  }
+}
index 4ea39c070747157fd6c88b5bc7804f745fe55041..9b93522b55281eac40bce05232c8f17b62935884 100644 (file)
@@ -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, "/"));
index 6c7b7202a655d1b47451ffb261bf02b5c47c25cc..776b24352f08b8a5f521ffa09858365836f69c6b 100644 (file)
@@ -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<DefaultIssue, SiblingIssue> 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 (file)
index 0000000..3a00fb9
--- /dev/null
@@ -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());
+  }
+}
index 145cf21cf1435c17aa967ff102e050622069bc46..911ec3a5263b8bf3f22ddb51625b309635c538e0 100644 (file)
@@ -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<String> 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;
index 40952ca9b54ad7c1d652f9d0e88e20c4a8f02bf9..f0768738ddb4cdc1e12470e86a3b4d9c6e8d95fb 100644 (file)
@@ -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<String> uuids);
+
   int update(IssueChangeDto change);
 
   @CheckForNull
index 7b934fbdb19bdb1340a3bc0cb73282c249346103..9936b5bac62fc5dbabf780f74b7b17e4f6f3da17 100644 (file)
     delete from issue_changes where kee=#{id}
   </delete>
 
+  <delete id="deleteByUuids" parameterType="map">
+    delete from issue_changes where uuid in
+    <foreach collection="changeUuids" open="(" close=")" item="changeUuid" separator=",">
+      #{changeUuid, jdbcType=VARCHAR}
+    </foreach>
+  </delete>
+
   <update id="update" parameterType="map">
     update issue_changes set change_data=#{changeData}, updated_at=#{updatedAt,jdbcType=BIGINT} where kee=#{kee,jdbcType=VARCHAR}
   </update>
index 0d71f7b569b5de9371904d99cb748f04b03bd6f1..40879d7a51218269e7ab482514008b6ad5791abf 100644 (file)
@@ -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,
index 4318df407b0ac559b18551034134f58a8d5f8c42..2d747669ab29c176f2fa1d2cb85d5678aca88556 100644 (file)
@@ -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);
   }
 }
index f37031799536a9823f020a6e254a5a5ea9b3658d..7089228fa13fa60b6e56e2fa5ba5876b43835df8 100644 (file)
@@ -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();
   }