]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-16541 Ensure issues pull action response contains correct issue data
authorKlaudio Sinani <klaudio.sinani@sonarsource.com>
Mon, 27 Jun 2022 15:53:39 +0000 (17:53 +0200)
committersonartech <sonartech@sonarsource.com>
Tue, 28 Jun 2022 20:02:52 +0000 (20:02 +0000)
server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDao.java
server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueMapper.java
server/sonar-db-dao/src/main/resources/org/sonar/db/issue/IssueMapper.xml
server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDaoTest.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/PullAction.java
server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/pull/PullActionIssuesRetriever.java
server/sonar-webserver-webapi/src/test/java/org/sonar/server/issue/ws/pull/PullActionIssuesRetrieverTest.java

index 454a2420d16b47d9585b34b62fbd44800131e31e..5d553b776a58aa4df68cf5ff9b08874ff1530c44 100644 (file)
@@ -62,6 +62,10 @@ public class IssueDao implements Dao {
     return mapper(session).selectIssueKeysByComponentUuid(componentUuid);
   }
 
+  public Set<String> selectIssueKeysByComponentUuidAndChangedSinceDate(DbSession session, String componentUuid, long changedSince) {
+    return mapper(session).selectIssueKeysByComponentUuidAndChangedSinceDate(componentUuid, changedSince);
+  }
+
   public List<IssueDto> selectByComponentUuidPaginated(DbSession session, String componentUuid, int page) {
     return mapper(session).selectByComponentUuidPaginated(componentUuid, Pagination.forPage(page).andSize(DEFAULT_PAGE_SIZE));
   }
index bd3db65bb67cf065c8310df9caf638790c90a7fa..3862db81734ab7c3f87ba9c46c99a3c95d26b384 100644 (file)
@@ -39,6 +39,8 @@ public interface IssueMapper {
 
   Set<String> selectIssueKeysByComponentUuid(@Param("componentUuid") String componentUuid);
 
+  Set<String> selectIssueKeysByComponentUuidAndChangedSinceDate(@Param("componentUuid") String componentUuid, @Param("changedSince") long changedSince);
+
   List<IssueDto> selectByComponentUuidPaginated(@Param("componentUuid") String componentUuid,
                                                 @Param("pagination") Pagination pagination);
 
index f79899578ad391240151ed7728f6d4b9e10b8fae..0b0690724ed7a75da545a9a99618bdf4b4d29bad 100644 (file)
       i.component_uuid = #{componentUuid,jdbcType=VARCHAR}
       and i.status = 'CLOSED'
       and i.issue_close_date is not null
-      and i.issue_close_date >= #{closeDateAfter,jdbcType=BIGINT}
+      and i.issue_close_date &gt;= #{closeDateAfter,jdbcType=BIGINT}
     order by
       i.kee, ic.issue_change_creation_date desc
   </select>
       i.project_uuid=#{componentUuid,jdbcType=VARCHAR}
   </select>
 
+  <select id="selectIssueKeysByComponentUuidAndChangedSinceDate" parameterType="map" resultType="string">
+    select
+      i.kee
+    from issues i
+    where
+      i.project_uuid=#{componentUuid,jdbcType=VARCHAR}
+      AND i.issue_update_date &gt;= #{changedSince,jdbcType=BIGINT}
+  </select>
+
   <select id="selectByComponentUuidPaginated" parameterType="map" resultType="Issue">
     select
     <include refid="issueColumns"/>,
             #{language}
         </foreach>
     </if>
-    order by i.issue_creation_date ASC
+    order by i.kee ASC
     limit #{pagination.pageSize,jdbcType=INTEGER} offset #{pagination.offset,jdbcType=INTEGER}
   </select>
 
       <include refid="selectByBranchColumnsOuterQuery"/>
     from (
       select
-        row_number() over(order by i.issue_creation_date ASC) as row_number,
+        row_number() over(order by i.kee ASC) as row_number,
           <include refid="selectByBranchColumns"/>
       from issues i
           inner join project_branches b on i.project_uuid = b.project_uuid
                     #{language}
                 </foreach>
             </if>
-            order by i.issue_creation_date ASC
+            order by i.kee ASC
             ) a
         ) t
       where
index 33690a61a5592a25ae6eb245892860c608eec9aa..941a2b31b1d79b4e75b15faec586db7b4226f5eb 100644 (file)
@@ -24,6 +24,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.IntStream;
 import java.util.stream.Stream;
 import org.junit.Rule;
 import org.junit.Test;
@@ -138,6 +139,21 @@ public class IssueDaoTest {
     assertThat(issues).containsOnly("I1", "I2");
   }
 
+  @Test
+  public void selectIssueKeysByComponentUuidAndChangedSince() {
+    long t1 = 1_340_000_000_000L;
+    long t2 = 1_400_000_000_000L;
+    // contains I1 and I2
+    prepareTables();
+    // Insert I3, I4, where t1 < t2
+    IntStream.range(3, 5).forEach(i -> underTest.insert(db.getSession(), newIssueDto("I" + i).setUpdatedAt(t1)));
+
+    Set<String> issues = underTest.selectIssueKeysByComponentUuidAndChangedSinceDate(db.getSession(), PROJECT_UUID, t2);
+
+    // results are not ordered, so do not use "containsExactly"
+    assertThat(issues).containsOnly("I1", "I2");
+  }
+
   @Test
   public void selectByComponentUuidPaginated() {
     // contains I1 and I2
index 660f74d6d4c7b99265d9f6f5133fbfd287d872e6..e22016d35575bee622ada7b6c029d6b19059a8b3 100644 (file)
@@ -21,8 +21,10 @@ package org.sonar.server.issue.ws;
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Consumer;
 import javax.annotation.Nullable;
 import org.sonar.api.server.ws.Request;
@@ -37,6 +39,7 @@ import org.sonar.server.issue.ws.pull.PullActionIssuesRetriever;
 import org.sonar.server.issue.ws.pull.PullActionResponseWriter;
 import org.sonar.server.user.UserSession;
 
+import static java.util.Optional.*;
 import static org.sonar.api.web.UserRole.USER;
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_PULL;
 
@@ -122,7 +125,7 @@ public class PullAction implements IssuesWsAction {
       pullActionResponseWriter.appendTimestampToResponse(outputStream);
       var pullActionQueryParams = new IssueQueryParams(projectDto.get().getUuid(), branchName,
         languages, ruleRepositories, resolvedOnly, changedSince);
-      retrieveAndSendIssues(dbSession, pullActionQueryParams, outputStream);
+      retrieveAndSendIssues(dbSession, projectDto.get().getUuid(), pullActionQueryParams, outputStream);
     }
   }
 
@@ -133,13 +136,14 @@ public class PullAction implements IssuesWsAction {
     userSession.checkProjectPermission(USER, projectDto.get());
   }
 
-  private void retrieveAndSendIssues(DbSession dbSession, IssueQueryParams queryParams, OutputStream outputStream)
+  private void retrieveAndSendIssues(DbSession dbSession, String componentUuid, IssueQueryParams queryParams, OutputStream outputStream)
     throws IOException {
 
     var issuesRetriever = new PullActionIssuesRetriever(dbClient, queryParams);
 
+    Set<String> issueKeysSnapshot = new HashSet<>(getIssueKeysSnapshot(componentUuid, queryParams.getChangedSince()));
     Consumer<List<IssueDto>> listConsumer = issueDtos -> pullActionResponseWriter.appendIssuesToResponse(issueDtos, outputStream);
-    issuesRetriever.processIssuesByBatch(dbSession, listConsumer);
+    issuesRetriever.processIssuesByBatch(dbSession, issueKeysSnapshot, listConsumer);
 
     if (queryParams.getChangedSince() != null) {
       // in the "incremental mode" we need to send SonarLint also recently closed issues keys
@@ -147,4 +151,16 @@ public class PullAction implements IssuesWsAction {
       pullActionResponseWriter.appendClosedIssuesUuidsToResponse(closedIssues, outputStream);
     }
   }
+
+  private Set<String> getIssueKeysSnapshot(String componentUuid, @Nullable Long changedSince) {
+    try (DbSession dbSession = dbClient.openSession(false)) {
+      Optional<Long> changedSinceDate = ofNullable(changedSince);
+
+      if (changedSinceDate.isPresent()) {
+        return dbClient.issueDao().selectIssueKeysByComponentUuidAndChangedSinceDate(dbSession, componentUuid, changedSinceDate.get());
+      }
+
+      return dbClient.issueDao().selectIssueKeysByComponentUuid(dbSession, componentUuid);
+    }
+  }
 }
index ace5d3aef1f654bd61837b77464796bb5b8581f4..8ed3738514bd6e85f11d1d13051ae33f5a2e0db7 100644 (file)
@@ -20,7 +20,9 @@
 package org.sonar.server.issue.ws.pull;
 
 import java.util.List;
+import java.util.Set;
 import java.util.function.Consumer;
+import java.util.stream.Collectors;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.issue.IssueDto;
@@ -38,13 +40,13 @@ public class PullActionIssuesRetriever {
     this.issueQueryParams = queryParams;
   }
 
-  public void processIssuesByBatch(DbSession dbSession, Consumer<List<IssueDto>> listConsumer) {
+  public void processIssuesByBatch(DbSession dbSession, Set<String> issueKeysSnapshot, Consumer<List<IssueDto>> listConsumer) {
     int nextPage = 1;
     boolean hasMoreIssues = true;
 
     while (hasMoreIssues) {
       List<IssueDto> issueDtos = nextOpenIssues(dbSession, nextPage);
-      listConsumer.accept(issueDtos);
+      listConsumer.accept(filterDuplicateIssues(issueDtos, issueKeysSnapshot));
       nextPage++;
       if (issueDtos.isEmpty() || issueDtos.size() < DEFAULT_PAGE_SIZE) {
         hasMoreIssues = false;
@@ -52,6 +54,23 @@ public class PullActionIssuesRetriever {
     }
   }
 
+  private static List<IssueDto> filterDuplicateIssues(List<IssueDto> issues, Set<String> issueKeysSnapshot) {
+    return issues
+      .stream()
+      .filter(issue -> isUniqueIssue(issue.getKee(), issueKeysSnapshot))
+      .collect(Collectors.toList());
+  }
+
+  private static boolean isUniqueIssue(String issueKey, Set<String> issueKeysSnapshot) {
+    boolean isUniqueIssue = issueKeysSnapshot.contains(issueKey);
+
+    if (isUniqueIssue) {
+      issueKeysSnapshot.remove(issueKey);
+    }
+
+    return isUniqueIssue;
+  }
+
   public List<String> retrieveClosedIssues(DbSession dbSession) {
     return dbClient.issueDao().selectRecentlyClosedIssues(dbSession, issueQueryParams);
   }
index 6820fe7e276e5103c23f2cc7b2c3816173255596..460f4e572447e1fe4231c32946694f743cbe4dbd 100644 (file)
@@ -21,6 +21,7 @@ package org.sonar.server.issue.ws.pull;
 
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
@@ -63,7 +64,7 @@ public class PullActionIssuesRetrieverTest {
     List<IssueDto> returnedDtos = new ArrayList<>();
     Consumer<List<IssueDto>> listConsumer = returnedDtos::addAll;
 
-    pullActionIssuesRetriever.processIssuesByBatch(dbClient.openSession(true), listConsumer);
+    pullActionIssuesRetriever.processIssuesByBatch(dbClient.openSession(true), Set.of(), listConsumer);
 
     assertThat(returnedDtos).isEmpty();
   }
@@ -71,15 +72,52 @@ public class PullActionIssuesRetrieverTest {
   @Test
   public void processIssuesByBatch_givenThousandOneIssuesReturnedByDatabase_thousandOneIssuesConsumed() {
     var pullActionIssuesRetriever = new PullActionIssuesRetriever(dbClient, queryParams);
-    List<IssueDto> thousandIssues = IntStream.rangeClosed(1, 1000).mapToObj(i -> new IssueDto()).collect(Collectors.toList());
+    List<IssueDto> thousandIssues = IntStream.rangeClosed(1, 1000).mapToObj(i -> new IssueDto().setKee(Integer.toString(i))).collect(Collectors.toList());
+    IssueDto singleIssue = new IssueDto().setKee("kee");
     when(issueDao.selectByBranch(any(), any(), anyInt()))
       .thenReturn(thousandIssues)
-      .thenReturn(List.of(new IssueDto()));
+      .thenReturn(List.of(singleIssue));
     List<IssueDto> returnedDtos = new ArrayList<>();
     Consumer<List<IssueDto>> listConsumer = returnedDtos::addAll;
 
-    pullActionIssuesRetriever.processIssuesByBatch(dbClient.openSession(true), listConsumer);
+    Set<String> thousandIssueUuidsSnapshot = thousandIssues.stream().map(IssueDto::getKee).collect(Collectors.toSet());
+    thousandIssueUuidsSnapshot.add(singleIssue.getKee());
+    pullActionIssuesRetriever.processIssuesByBatch(dbClient.openSession(true), thousandIssueUuidsSnapshot, listConsumer);
 
     assertThat(returnedDtos).hasSize(1001);
   }
+
+  @Test
+  public void processIssuesByBatch_filter_out_duplicate_issue_entries() {
+    var pullActionIssuesRetriever = new PullActionIssuesRetriever(dbClient, queryParams);
+    IssueDto issue1 = new IssueDto().setKee("kee1");
+    IssueDto issue2 = new IssueDto().setKee("kee2");
+    List<IssueDto> issues = List.of(issue1, issue1, issue1, issue2);
+    when(issueDao.selectByBranch(any(), any(), anyInt()))
+      .thenReturn(issues);
+    List<IssueDto> returnedDtos = new ArrayList<>();
+    Consumer<List<IssueDto>> listConsumer = returnedDtos::addAll;
+
+    Set<String> thousandIssueKeysSnapshot = issues.stream().map(IssueDto::getKee).collect(Collectors.toSet());
+    pullActionIssuesRetriever.processIssuesByBatch(dbClient.openSession(true), thousandIssueKeysSnapshot, listConsumer);
+
+    assertThat(returnedDtos)
+      .hasSize(2)
+      .containsExactlyInAnyOrder(issue1, issue2);
+  }
+
+  @Test
+  public void processIssuesByBatch_correctly_processes_all_issues_regardless_of_creation_timestamp() {
+    var pullActionIssuesRetriever = new PullActionIssuesRetriever(dbClient, queryParams);
+    List<IssueDto> issuesWithSameCreationTimestamp = IntStream.rangeClosed(1, 100).mapToObj(i -> new IssueDto().setKee(Integer.toString(i)).setCreatedAt(100L)).collect(Collectors.toList());
+    when(issueDao.selectByBranch(any(), any(), anyInt()))
+      .thenReturn(issuesWithSameCreationTimestamp);
+    List<IssueDto> returnedDtos = new ArrayList<>();
+    Consumer<List<IssueDto>> listConsumer = returnedDtos::addAll;
+
+    Set<String> issueKeysSnapshot = issuesWithSameCreationTimestamp.stream().map(IssueDto::getKee).collect(Collectors.toSet());
+    pullActionIssuesRetriever.processIssuesByBatch(dbClient.openSession(true), issueKeysSnapshot, listConsumer);
+
+    assertThat(returnedDtos).hasSize(100);
+  }
 }