SONAR-10313 Keep the order of issues on Search

server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDao.java

@@ -19,15 +19,10 @@
  */
 package org.sonar.db.issue;
 
-import com.google.common.base.Function;
-import com.google.common.base.Predicates;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
 import org.apache.ibatis.session.ResultHandler;
 import org.sonar.db.Dao;
 import org.sonar.db.DbSession;
@@ -35,18 +30,17 @@ import org.sonar.db.RowNotFoundException;
 import org.sonar.db.WildcardPosition;
 import org.sonar.db.component.ComponentDto;
 
-import static com.google.common.collect.FluentIterable.from;
 import static org.sonar.db.DaoDatabaseUtils.buildLikeValue;
 import static org.sonar.db.DatabaseUtils.executeLargeInputs;
 
 public class IssueDao implements Dao {
 
-  public java.util.Optional<IssueDto> selectByKey(DbSession session, String key) {
-    return java.util.Optional.ofNullable(mapper(session).selectByKey(key));
+  public Optional<IssueDto> selectByKey(DbSession session, String key) {
+    return Optional.ofNullable(mapper(session).selectByKey(key));
   }
 
   public IssueDto selectOrFailByKey(DbSession session, String key) {
-    java.util.Optional<IssueDto> issue = selectByKey(session, key);
+    Optional<IssueDto> issue = selectByKey(session, key);
     if (!issue.isPresent()) {
       throw new RowNotFoundException(String.format("Issue with key '%s' does not exist", key));
     }
@@ -57,39 +51,12 @@ public class IssueDao implements Dao {
    * Gets a list issues by their keys. The result does NOT contain {@code null} values for issues not found, so
    * the size of result may be less than the number of keys. A single issue is returned
    * if input keys contain multiple occurrences of a key.
-   * <p>Results may be in a different order as input keys (see {@link #selectByOrderedKeys(DbSession, List)}).</p>
+   * <p>Results may be in a different order as input keys.</p>
    */
   public List<IssueDto> selectByKeys(final DbSession session, Collection<String> keys) {
     return executeLargeInputs(keys, mapper(session)::selectByKeys);
   }
 
-  /**
-   * Gets a list issues by their keys. The result does NOT contain {@code null} values for issues not found, so
-   * the size of result may be less than the number of keys. A single issue is returned
-   * if input keys contain multiple occurrences of a key.
-   * <p>Contrary to {@link #selectByKeys(DbSession, Collection)}, results are in the same order as input keys.</p>
-   */
-  public List<IssueDto> selectByOrderedKeys(DbSession session, List<String> keys) {
-    List<IssueDto> unordered = selectByKeys(session, keys);
-    return from(keys).transform(new KeyToIssue(unordered)).filter(Predicates.notNull()).toList();
-  }
-
-  private static class KeyToIssue implements Function<String, IssueDto> {
-    private final Map<String, IssueDto> map = new HashMap<>();
-
-    private KeyToIssue(Collection<IssueDto> unordered) {
-      for (IssueDto dto : unordered) {
-        map.put(dto.getKey(), dto);
-      }
-    }
-
-    @Nullable
-    @Override
-    public IssueDto apply(@Nonnull String issueKey) {
-      return map.get(issueKey);
-    }
-  }
-
   public Set<String> selectComponentUuidsOfOpenIssuesForProjectUuid(DbSession session, String projectUuid) {
     return mapper(session).selectComponentUuidsOfOpenIssuesForProjectUuid(projectUuid);
   }

server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDaoTest.java

@@ -122,18 +122,6 @@ public class IssueDaoTest {
     assertThat(issues).extracting("key").containsOnly("I1", "I2");
   }
 
-  @Test
-  public void selectByOrderedKeys() {
-    // contains I1 and I2
-    prepareTables();
-
-    Iterable<IssueDto> issues = underTest.selectByOrderedKeys(db.getSession(), asList("I1", "I2", "I3"));
-    assertThat(issues).extracting("key").containsExactly("I1", "I2");
-
-    issues = underTest.selectByOrderedKeys(db.getSession(), asList("I2", "I3", "I1"));
-    assertThat(issues).extracting("key").containsExactly("I2", "I1");
-  }
-
   @Test
   public void scrollNonClosedByComponentUuid() {
     RuleDefinitionDto rule = db.rules().insert();

server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java

@@ -19,15 +19,19 @@
  */
 package org.sonar.server.issue.ws;
 
+import com.google.common.base.Function;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.MultimapBuilder;
 import com.google.common.collect.SetMultimap;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.util.stream.MoreCollectors;
@@ -80,8 +84,7 @@ public class SearchResponseLoader {
   /**
    * The issue keys are given by the multi-criteria search in Elasticsearch index.
    * <p>
-   * Same as {@link #load(SearchResponseData, Collector, Facets)} but will only retrieve from DB data which is not
-   * already provided by the specified preloaded {@link SearchResponseData}.<br/>
+   * It will only retrieve from DB data which is not already provided by the specified preloaded {@link SearchResponseData}.<br/>
    * The returned {@link SearchResponseData} is <strong>not</strong> the one specified as argument.
    * </p>
    */
@@ -106,14 +109,23 @@ public class SearchResponseLoader {
   private List<IssueDto> loadIssues(SearchResponseData preloadedResponseData, Collector collector, DbSession dbSession) {
     List<IssueDto> preloadedIssues = preloadedResponseData.getIssues();
     Set<String> preloadedIssueKeys = preloadedIssues.stream().map(IssueDto::getKey).collect(MoreCollectors.toSet(preloadedIssues.size()));
-    Set<String> issueKeysToLoad = copyOf(difference(ImmutableSet.copyOf(collector.getIssueKeys()), preloadedIssueKeys));
+
+    ImmutableSet<String> issueKeys = ImmutableSet.copyOf(collector.getIssueKeys());
+    Set<String> issueKeysToLoad = copyOf(difference(issueKeys, preloadedIssueKeys));
+
     if (issueKeysToLoad.isEmpty()) {
-      return preloadedIssues;
+      return issueKeys.stream()
+        .map(new KeyToIssueFunction(preloadedIssues)::apply).filter(Objects::nonNull)
+        .collect(Collectors.toList());
     }
 
     List<IssueDto> loadedIssues = dbClient.issueDao().selectByKeys(dbSession, issueKeysToLoad);
-    return concat(preloadedIssues.stream(), loadedIssues.stream())
+    List<IssueDto> unorderedIssues = concat(preloadedIssues.stream(), loadedIssues.stream())
       .collect(toList(preloadedIssues.size() + loadedIssues.size()));
+
+    return issueKeys.stream()
+      .map(new KeyToIssueFunction(unorderedIssues)::apply).filter(Objects::nonNull)
+      .collect(Collectors.toList());
   }
 
   private void loadUsers(SearchResponseData preloadedResponseData, Collector collector, DbSession dbSession, SearchResponseData result) {
@@ -338,4 +350,20 @@ public class SearchResponseLoader {
       return projectUuids;
     }
   }
+
+  private static class KeyToIssueFunction implements Function<String, IssueDto> {
+    private final Map<String, IssueDto> map = new HashMap<>();
+
+    private KeyToIssueFunction(Collection<IssueDto> unordered) {
+      for (IssueDto dto : unordered) {
+        map.put(dto.getKey(), dto);
+      }
+    }
+
+    @Nullable
+    @Override
+    public IssueDto apply(String issueKey) {
+      return map.get(issueKey);
+    }
+  }
 }

server/sonar-server/src/test/java/org/sonar/server/issue/ws/SearchActionTest.java

@@ -19,6 +19,8 @@
  */
 package org.sonar.server.issue.ws;
 
+import com.google.gson.JsonElement;
+import com.google.gson.JsonParser;
 import java.time.Clock;
 import org.junit.Before;
 import org.junit.Rule;
@@ -627,11 +629,16 @@ public class SearchActionTest {
     session.commit();
     indexIssues();
 
-    ws.newRequest()
+    TestResponse response = ws.newRequest()
       .setParam("sort", IssueQuery.SORT_BY_UPDATE_DATE)
       .setParam("asc", "false")
-      .execute()
-      .assertJson(this.getClass(), "sort_by_updated_at.json");
+      .execute();
+
+    JsonElement parse = new JsonParser().parse(response.getInput());
+
+    assertThat(parse.getAsJsonObject().get("issues").getAsJsonArray())
+      .extracting(o -> o.getAsJsonObject().get("key").getAsString())
+      .containsExactly("82fd47d4-b650-4037-80bc-7b112bd4eac3", "82fd47d4-b650-4037-80bc-7b112bd4eac1", "82fd47d4-b650-4037-80bc-7b112bd4eac2");
   }
 
   @Test

server/sonar-server/src/test/resources/org/sonar/server/issue/ws/SearchActionTest/sort_by_updated_at.json

@@ -1,28 +0,0 @@
-{
-  "issues": [
-    {
-      "organization": "my-org-2",
-      "key": "82fd47d4-b650-4037-80bc-7b112bd4eac2",
-      "component": "FILE_KEY",
-      "project": "PROJECT_KEY",
-      "rule": "xoo:x1",
-      "updateDate": "2014-11-01T00:00:00+0100"
-    },
-    {
-      "organization": "my-org-2",
-      "key": "82fd47d4-b650-4037-80bc-7b112bd4eac1",
-      "component": "FILE_KEY",
-      "project": "PROJECT_KEY",
-      "rule": "xoo:x1",
-      "updateDate": "2014-11-02T00:00:00+0100"
-    },
-    {
-      "organization": "my-org-2",
-      "key": "82fd47d4-b650-4037-80bc-7b112bd4eac3",
-      "component": "FILE_KEY",
-      "project": "PROJECT_KEY",
-      "rule": "xoo:x1",
-      "updateDate": "2014-11-03T00:00:00+0100"
-    }
-  ]
-}

tests/src/test/java/org/sonarqube/tests/issue/IssueCreationDatePluginChangedTest.java

@@ -23,12 +23,10 @@ import com.google.common.collect.ImmutableList;
 import com.sonar.orchestrator.Orchestrator;
 import com.sonar.orchestrator.build.SonarScanner;
 import java.io.File;
-import java.io.IOException;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.util.Date;
 import java.util.List;
-import javax.mail.MessagingException;
 import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -150,7 +148,9 @@ public class IssueCreationDatePluginChangedTest {
 
     // New analysis that should raise 2 new issues that will be backdated
     ORCHESTRATOR.executeBuild(scanner);
-    issues = getIssues(issueQuery().components("creation-date-sample:src/main/xoo/sample/Sample.xoo"));
+    issues = getIssues(issueQuery()
+      .components("creation-date-sample:src/main/xoo/sample/Sample.xoo")
+      .sort("FILE_LINE"));
     assertThat(issues)
       .extracting(Issue::line, Issue::creationDate)
       .containsExactly(tuple(1, dateTimeParse("2005-01-01T00:00:00+0000")),