SONAR-10313 Do not retrieve rules in request twice

When building IssueQuery and loading SearchResponseData

6 files changed, 25 insertions, 69 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java
index fd3508f252d..123de44f103 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQuery.java
@@ -28,6 +28,7 @@ import java.util.Set;
 import javax.annotation.CheckForNull;
 import javax.annotation.Nullable;
 import org.apache.commons.lang.builder.ReflectionToStringBuilder;
+import org.sonar.db.rule.RuleDefinitionDto;
 
 import static com.google.common.base.Preconditions.checkArgument;
 import static org.sonar.server.es.SearchOptions.MAX_LIMIT;
@@ -63,7 +64,7 @@ public class IssueQuery {
   private final Collection<String> directories;
   private final Collection<String> files;
   private final Collection<String> views;
-  private final Collection<Integer> rules;
+  private final Collection<RuleDefinitionDto> rules;
   private final Collection<String> assignees;
   private final Collection<String> authors;
   private final Collection<String> languages;
@@ -162,7 +163,7 @@ public class IssueQuery {
     return views;
   }
 
-  public Collection<Integer> rules() {
+  public Collection<RuleDefinitionDto> rules() {
     return rules;
   }
 
@@ -273,7 +274,7 @@ public class IssueQuery {
     private Collection<String> directories;
     private Collection<String> files;
     private Collection<String> views;
-    private Collection<Integer> rules;
+    private Collection<RuleDefinitionDto> rules;
     private Collection<String> assignees;
     private Collection<String> authors;
     private Collection<String> languages;
@@ -353,7 +354,7 @@ public class IssueQuery {
       return this;
     }
 
-    public Builder rules(@Nullable Collection<Integer> rules) {
+    public Builder rules(@Nullable Collection<RuleDefinitionDto> rules) {
       this.rules = rules;
       return this;
     }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryFactory.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryFactory.java
index 5eea597845c..18dd1eef55a 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryFactory.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueQueryFactory.java
@@ -28,6 +28,7 @@ import java.time.Period;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.Date;
 import java.util.HashSet;
 import java.util.List;
@@ -44,7 +45,6 @@ import org.sonar.api.resources.Qualifiers;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.server.ServerSide;
 import org.sonar.api.web.UserRole;
-import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.component.ComponentDto;
@@ -364,14 +364,11 @@ public class IssueQueryFactory {
   }
 
   @CheckForNull
-  private Collection<Integer> ruleKeysToRuleId(DbSession dbSession, @Nullable Collection<String> rules) {
+  private Collection<RuleDefinitionDto> ruleKeysToRuleId(DbSession dbSession, @Nullable Collection<String> rules) {
     if (rules != null) {
-      return dbClient.ruleDao().selectDefinitionByKeys(dbSession, transform(rules, RuleKey::parse))
-        .stream()
-        .map(RuleDefinitionDto::getId)
-        .collect(MoreCollectors.toList());
+      return dbClient.ruleDao().selectDefinitionByKeys(dbSession, transform(rules, RuleKey::parse));
     }
-    return null;
+    return Collections.emptyList();
   }
 
   private static String toProjectUuid(ComponentDto componentDto) {
diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java
index d70760a5cbb..9237803edc1 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchAction.java
@@ -21,6 +21,7 @@ package org.sonar.server.issue.ws;
 
 import com.google.common.base.Joiner;
 import com.google.common.collect.Collections2;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import java.util.Arrays;
 import java.util.Collection;
@@ -363,7 +364,9 @@ public class SearchAction implements IssuesWsAction {
         (!query.projectUuids().isEmpty()) || query.organizationUuid() != null, "Facet(s) '%s' require to also filter by project or organization",
         COMA_JOINER.join(facetsRequiringProjectOrOrganizationParameter));
     }
-    SearchResponseData data = searchResponseLoader.load(collector, facets);
+    SearchResponseData preloadedData = new SearchResponseData(emptyList());
+    preloadedData.setRules(ImmutableList.copyOf(query.rules()));
+    SearchResponseData data = searchResponseLoader.load(preloadedData, collector, facets);
 
     // format response
 
diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java
index 7647b198edb..7e03b90f08c 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SearchResponseLoader.java
@@ -29,7 +29,6 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import javax.annotation.Nullable;
-import org.sonar.api.rule.RuleKey;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.DbClient;
@@ -80,30 +79,9 @@ public class SearchResponseLoader {
 
   /**
    * The issue keys are given by the multi-criteria search in Elasticsearch index.
-   */
-  public SearchResponseData load(Collector collector, @Nullable Facets facets) {
-    try (DbSession dbSession = dbClient.openSession(false)) {
-      SearchResponseData result = new SearchResponseData(dbClient.issueDao().selectByOrderedKeys(dbSession, collector.getIssueKeys()));
-      collector.collect(result.getIssues());
-
-      loadRules(collector, dbSession, result);
-      // order is important - loading of comments complete the list of users: loadComments() is
-      // before loadUsers()
-      loadComments(collector, dbSession, result);
-      loadUsers(collector, dbSession, result);
-      loadComponents(collector, dbSession, result);
-      loadOrganizations(dbSession, result);
-      loadActionsAndTransitions(collector, result);
-      completeTotalEffortFromFacet(facets, result);
-      return result;
-    }
-  }
-
-  /**
-   * The issue keys are given by the multi-criteria search in Elasticsearch index.
    * <p>
-   * Same as {@link #load(Collector, Facets)} but will only retrieve from DB data which is not already provided by the
-   * specified preloaded {@link SearchResponseData}.<br/>
+   * 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/>
    * The returned {@link SearchResponseData} is <strong>not</strong> the one specified as argument.
    * </p>
    */
@@ -186,23 +164,17 @@ public class SearchResponseLoader {
   private void loadRules(SearchResponseData preloadedResponseData, Collector collector, DbSession dbSession, SearchResponseData result) {
     if (collector.contains(RULES)) {
       List<RuleDefinitionDto> preloadedRules = firstNonNull(preloadedResponseData.getRules(), emptyList());
-      Set<RuleKey> preloaedRuleKeys = preloadedRules.stream().map(RuleDefinitionDto::getKey).collect(MoreCollectors.toSet());
-      Set<RuleKey> ruleKeysToLoad = copyOf(difference(collector.get(RULES), preloaedRuleKeys));
+      Set<Integer> preloaedRuleKeys = preloadedRules.stream().map(RuleDefinitionDto::getId).collect(MoreCollectors.toSet());
+      Set<Integer> ruleKeysToLoad = copyOf(difference(collector.get(RULES), preloaedRuleKeys));
       if (ruleKeysToLoad.isEmpty()) {
         result.setRules(preloadedResponseData.getRules());
       } else {
-        List<RuleDefinitionDto> loadedRules = dbClient.ruleDao().selectDefinitionByKeys(dbSession, ruleKeysToLoad);
+        List<RuleDefinitionDto> loadedRules = dbClient.ruleDao().selectDefinitionByIds(dbSession, ruleKeysToLoad);
         result.setRules(concat(preloadedRules.stream(), loadedRules.stream()).collect(toList(preloadedRules.size() + loadedRules.size())));
       }
     }
   }
 
-  private void loadUsers(Collector collector, DbSession dbSession, SearchResponseData result) {
-    if (collector.contains(USERS)) {
-      result.setUsers(dbClient.userDao().selectByLogins(dbSession, collector.getList(USERS)));
-    }
-  }
-
   private void loadComments(Collector collector, DbSession dbSession, SearchResponseData result) {
     if (collector.contains(COMMENTS)) {
       List<IssueChangeDto> comments = dbClient.issueChangeDao().selectByTypeAndIssueKeys(dbSession, collector.getIssueKeys(), IssueChangeDto.TYPE_COMMENT);
@@ -220,20 +192,6 @@ public class SearchResponseLoader {
     return userSession.isLoggedIn() && userSession.getLogin().equals(dto.getUserLogin());
   }
 
-  private void loadRules(Collector collector, DbSession dbSession, SearchResponseData result) {
-    if (collector.contains(RULES)) {
-      result.setRules(dbClient.ruleDao().selectDefinitionByIds(dbSession, collector.getList(RULES)));
-    }
-  }
-
-  private void loadComponents(Collector collector, DbSession dbSession, SearchResponseData result) {
-    // always load components and projects, because some issue fields still relate to component ids/keys.
-    // They should be dropped but are kept for backward-compatibility (see SearchResponseFormat)
-    result.addComponents(dbClient.componentDao().selectByUuids(dbSession, collector.getComponentUuids()));
-    result.addComponents(dbClient.componentDao().selectSubProjectsByComponentUuids(dbSession, collector.getComponentUuids()));
-    addProjectUuids(collector, dbSession, result);
-  }
-
   private void loadOrganizations(DbSession dbSession, SearchResponseData result) {
     Collection<ComponentDto> components = result.getComponents();
     dbClient.organizationDao().selectByUuids(
@@ -364,10 +322,6 @@ public class SearchResponseLoader {
       return (Set<T>) fieldValues.get(key);
     }
 
-    <T> List<T> getList(SearchAdditionalField key) {
-      return newArrayList(get(key));
-    }
-
     boolean contains(SearchAdditionalField field) {
       return fields.contains(field);
     }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryTest.java
index 8c89d26a8a9..c1cb1df4585 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueQueryTest.java
@@ -25,6 +25,7 @@ import java.util.Date;
 import org.junit.Test;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.rule.Severity;
+import org.sonar.db.rule.RuleDefinitionDto;
 import org.sonar.server.issue.IssueQuery.PeriodStart;
 
 import static com.google.common.collect.Lists.newArrayList;
@@ -36,8 +37,8 @@ public class IssueQueryTest {
 
   @Test
   public void build_query() {
-    int ruleId = nextInt(1000);
-    PeriodStart filterDate = new IssueQuery.PeriodStart(new Date(10_000_000_000L),false);
+    RuleDefinitionDto rule = new RuleDefinitionDto().setId(nextInt(1000));
+    PeriodStart filterDate = new IssueQuery.PeriodStart(new Date(10_000_000_000L), false);
     IssueQuery query = IssueQuery.builder()
       .issueKeys(newArrayList("ABCDE"))
       .severities(newArrayList(Severity.BLOCKER))
@@ -46,7 +47,7 @@ public class IssueQueryTest {
       .projectUuids(newArrayList("PROJECT"))
       .componentUuids(newArrayList("org/struts/Action.java"))
       .moduleUuids(newArrayList("org.struts:core"))
-      .rules(newArrayList(ruleId))
+      .rules(newArrayList(rule))
       .assignees(newArrayList("gargantua"))
       .languages(newArrayList("xoo"))
       .tags(newArrayList("tag1", "tag2"))
@@ -77,7 +78,7 @@ public class IssueQueryTest {
     assertThat(query.branchUuid()).isEqualTo("my_branch");
     assertThat(query.createdAfterByProjectUuids()).containsOnly(entry("PROJECT", filterDate));
     assertThat(query.assigned()).isTrue();
-    assertThat(query.rules()).containsOnly(ruleId);
+    assertThat(query.rules()).containsOnly(rule);
     assertThat(query.createdAfter()).isNotNull();
     assertThat(query.createdBefore()).isNotNull();
     assertThat(query.createdAt()).isNotNull();
diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexTest.java
index 70767cbf405..8552cd1dee9 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/issue/index/IssueIndexTest.java
@@ -526,8 +526,8 @@ public class IssueIndexTest {
 
     indexIssues(newDoc("I1", file).setRuleId(ruleDefinitionDto.getId()));
 
-    assertThatSearchReturnsOnly(IssueQuery.builder().rules(singletonList(ruleDefinitionDto.getId())), "I1");
-    assertThatSearchReturnsEmpty(IssueQuery.builder().rules(singletonList(-1)));
+    assertThatSearchReturnsOnly(IssueQuery.builder().rules(singletonList(ruleDefinitionDto)), "I1");
+    assertThatSearchReturnsEmpty(IssueQuery.builder().rules(singletonList(new RuleDefinitionDto().setId(-1))));
   }
 
   @Test