SONAR-8061 speed up issue assign by removing duplicate SQL

it/it-tests/src/test/java/it/authorisation/IssuePermissionTest.java

@@ -69,6 +69,8 @@ public void init() {
     SonarScanner publicProject = SonarScanner.create(projectDir("shared/xoo-sample"))
       .setProperty("sonar.projectKey", "publicProject")
       .setProperty("sonar.projectName", "PublicProject");
+
+
     orchestrator.executeBuild(publicProject);
   }
 

server/sonar-server/src/main/java/org/sonar/server/issue/IssueStorage.java

@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import java.util.stream.StreamSupport;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.issue.IssueComment;
@@ -38,10 +39,12 @@
 import org.sonar.db.DbSession;
 import org.sonar.db.issue.IssueChangeDto;
 import org.sonar.db.issue.IssueChangeMapper;
+import org.sonar.db.issue.IssueDto;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
 import static com.google.common.collect.Lists.newArrayList;
 import static java.util.Collections.emptyList;
+import static org.sonar.core.util.stream.MoreCollectors.toSet;
 
 /**
  * Save issues into database. It is executed :
@@ -72,17 +75,17 @@ public void save(DefaultIssue issue) {
     save(newArrayList(issue));
   }
 
-  public void save(DbSession session, DefaultIssue issue) {
-    doSave(session, newArrayList(issue));
+  public IssueDto save(DbSession session, DefaultIssue issue) {
+    return doSave(session, newArrayList(issue)).iterator().next();
   }
 
-  public void save(Iterable<DefaultIssue> issues) {
+  public Collection<IssueDto> save(Iterable<DefaultIssue> issues) {
     try (DbSession session = dbClient.openSession(true)) {
-      doSave(session, issues);
+      return doSave(session, issues);
     }
   }
 
-  private void doSave(DbSession session, Iterable<DefaultIssue> issues) {
+  private Collection<IssueDto> doSave(DbSession session, Iterable<DefaultIssue> issues) {
     // Batch session can not be used for updates. It does not return the number of updated rows,
     // required for detecting conflicts.
     long now = system2.now();
@@ -91,13 +94,15 @@ private void doSave(DbSession session, Iterable<DefaultIssue> issues) {
     List<DefaultIssue> issuesToInsert = firstNonNull(issuesNewOrUpdated.get(true), emptyList());
     List<DefaultIssue> issuesToUpdate = firstNonNull(issuesNewOrUpdated.get(false), emptyList());
 
-    Collection<String> inserted = insert(session, issuesToInsert, now);
-    Collection<String> updated = update(issuesToUpdate, now);
+    Collection<IssueDto> inserted = insert(session, issuesToInsert, now);
+    Collection<IssueDto> updated = update(issuesToUpdate, now);
 
-    Collection<String> issuesInsertedOrUpdated = new ArrayList<>(issuesToInsert.size() + issuesToUpdate.size());
-    issuesInsertedOrUpdated.addAll(inserted);
-    issuesInsertedOrUpdated.addAll(updated);
-    doAfterSave(issuesInsertedOrUpdated);
+    doAfterSave(Stream.concat(inserted.stream(), updated.stream())
+      .map(IssueDto::getKey)
+      .collect(toSet(issuesToInsert.size() + issuesToUpdate.size())));
+
+    return Stream.concat(inserted.stream(), updated.stream())
+      .collect(toSet(issuesToInsert.size() + issuesToUpdate.size()));
   }
 
   protected void doAfterSave(Collection<String> issues) {
@@ -107,13 +112,13 @@ protected void doAfterSave(Collection<String> issues) {
   /**
    * @return the keys of the inserted issues
    */
-  private Collection<String> insert(DbSession session, Iterable<DefaultIssue> issuesToInsert, long now) {
-    List<String> inserted = newArrayList();
+  private Collection<IssueDto> insert(DbSession session, Iterable<DefaultIssue> issuesToInsert, long now) {
+    List<IssueDto> inserted = newArrayList();
     int count = 0;
     IssueChangeMapper issueChangeMapper = session.getMapper(IssueChangeMapper.class);
     for (DefaultIssue issue : issuesToInsert) {
-      String key = doInsert(session, now, issue);
-      inserted.add(key);
+      IssueDto issueDto = doInsert(session, now, issue);
+      inserted.add(issueDto);
       insertChanges(issueChangeMapper, issue);
       if (count > BatchSession.MAX_BATCH_SIZE) {
         session.commit();
@@ -124,19 +129,19 @@ private Collection<String> insert(DbSession session, Iterable<DefaultIssue> issu
     return inserted;
   }
 
-  protected abstract String doInsert(DbSession batchSession, long now, DefaultIssue issue);
+  protected abstract IssueDto doInsert(DbSession batchSession, long now, DefaultIssue issue);
 
   /**
    * @return the keys of the updated issues
    */
-  private Collection<String> update(List<DefaultIssue> issuesToUpdate, long now) {
-    Collection<String> updated = new ArrayList<>();
+  private Collection<IssueDto> update(List<DefaultIssue> issuesToUpdate, long now) {
+    Collection<IssueDto> updated = new ArrayList<>();
     if (!issuesToUpdate.isEmpty()) {
       try (DbSession dbSession = dbClient.openSession(false)) {
         IssueChangeMapper issueChangeMapper = dbSession.getMapper(IssueChangeMapper.class);
         for (DefaultIssue issue : issuesToUpdate) {
-          String key = doUpdate(dbSession, now, issue);
-          updated.add(key);
+          IssueDto issueDto = doUpdate(dbSession, now, issue);
+          updated.add(issueDto);
           insertChanges(issueChangeMapper, issue);
         }
         dbSession.commit();
@@ -145,7 +150,7 @@ private Collection<String> update(List<DefaultIssue> issuesToUpdate, long now) {
     return updated;
   }
 
-  protected abstract String doUpdate(DbSession batchSession, long now, DefaultIssue issue);
+  protected abstract IssueDto doUpdate(DbSession batchSession, long now, DefaultIssue issue);
 
   private void insertChanges(IssueChangeMapper mapper, DefaultIssue issue) {
     for (IssueComment comment : issue.comments()) {

server/sonar-server/src/main/java/org/sonar/server/issue/IssueUpdater.java

@@ -28,10 +28,15 @@
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.component.ComponentDto;
+import org.sonar.db.issue.IssueDto;
 import org.sonar.db.rule.RuleDefinitionDto;
 import org.sonar.server.issue.notification.IssueChangeNotification;
+import org.sonar.server.issue.ws.SearchResponseData;
 import org.sonar.server.notification.NotificationManager;
 
+import static java.util.Collections.singleton;
+import static java.util.Collections.singletonList;
+
 public class IssueUpdater {
 
   private final DbClient dbClient;
@@ -44,21 +49,46 @@ public IssueUpdater(DbClient dbClient, IssueStorage issueStorage, NotificationMa
     this.notificationService = notificationService;
   }
 
-  public void saveIssue(DbSession session, DefaultIssue issue, IssueChangeContext context, @Nullable String comment) {
-    issueStorage.save(session, issue);
+  /**
+   * Same as {@link #saveIssue(DbSession, DefaultIssue, IssueChangeContext, String)} but populates the specified
+   * {@link SearchResponseData} with the DTOs (rule and components) retrieved from DB to save the issue.
+   */
+  public SearchResponseData saveIssueAndPreloadSearchResponseData(DbSession session, DefaultIssue issue, IssueChangeContext context, @Nullable String comment) {
     Optional<RuleDefinitionDto> rule = getRuleByKey(session, issue.getRuleKey());
     ComponentDto project = dbClient.componentDao().selectOrFailByUuid(session, issue.projectUuid());
+    ComponentDto component = dbClient.componentDao().selectOrFailByUuid(session, issue.componentUuid());
+    IssueDto issueDto = saveIssue(session, issue, context, comment, rule, project, component);
+
+    SearchResponseData preloadedSearchResponseData = new SearchResponseData(issueDto);
+    rule.ifPresent(r -> preloadedSearchResponseData.setRules(singletonList(r)));
+    preloadedSearchResponseData.addComponents(singleton(project));
+    preloadedSearchResponseData.addComponents(singleton(component));
+    return preloadedSearchResponseData;
+  }
+
+  public IssueDto saveIssue(DbSession session, DefaultIssue issue, IssueChangeContext context, @Nullable String comment) {
+    Optional<RuleDefinitionDto> rule = getRuleByKey(session, issue.getRuleKey());
+    ComponentDto project = dbClient.componentDao().selectOrFailByUuid(session, issue.projectUuid());
+    ComponentDto component = dbClient.componentDao().selectOrFailByUuid(session, issue.componentUuid());
+    return saveIssue(session, issue, context, comment, rule, project, component);
+  }
+
+  private IssueDto saveIssue(DbSession session, DefaultIssue issue, IssueChangeContext context, @Nullable String comment,
+    Optional<RuleDefinitionDto> rule, ComponentDto project, ComponentDto component) {
+    IssueDto issueDto = issueStorage.save(session, issue);
     notificationService.scheduleForSending(new IssueChangeNotification()
       .setIssue(issue)
       .setChangeAuthorLogin(context.login())
-      .setRuleName(rule.isPresent() ? rule.get().getName() : null)
+      .setRuleName(rule.map(RuleDefinitionDto::getName).orElse(null))
       .setProject(project.getKey(), project.name())
-      .setComponent(dbClient.componentDao().selectOrFailByUuid(session, issue.componentUuid()))
+      .setComponent(component)
       .setComment(comment));
+    return issueDto;
   }
 
   private Optional<RuleDefinitionDto> getRuleByKey(DbSession session, RuleKey ruleKey) {
     Optional<RuleDefinitionDto> rule = Optional.ofNullable(dbClient.ruleDao().selectDefinitionByKey(session, ruleKey).orElse(null));
     return (rule.isPresent() && rule.get().getStatus() != RuleStatus.REMOVED) ? rule : Optional.empty();
   }
+
 }

server/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueStorage.java

@@ -44,21 +44,21 @@ public ServerIssueStorage(System2 system2, RuleFinder ruleFinder, DbClient dbCli
   }
 
   @Override
-  protected String doInsert(DbSession session, long now, DefaultIssue issue) {
+  protected IssueDto doInsert(DbSession session, long now, DefaultIssue issue) {
     ComponentDto component = component(session, issue);
     ComponentDto project = project(session, issue);
     int ruleId = rule(issue).getId();
     IssueDto dto = IssueDto.toDtoForServerInsert(issue, component, project, ruleId, now);
 
     getDbClient().issueDao().insert(session, dto);
-    return dto.getKey();
+    return dto;
   }
 
   @Override
-  protected String doUpdate(DbSession session, long now, DefaultIssue issue) {
+  protected IssueDto doUpdate(DbSession session, long now, DefaultIssue issue) {
     IssueDto dto = IssueDto.toDtoForUpdate(issue, now);
     getDbClient().issueDao().update(session, dto);
-    return dto.getKey();
+    return dto;
   }
 
   @Override

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

@@ -54,7 +54,6 @@
 import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ISSUE;
 
 public class AssignAction implements IssuesWsAction {
-
   private static final String DEPRECATED_PARAM_ME = "me";
   private static final String ASSIGN_TO_ME_VALUE = "_me";
 
@@ -107,11 +106,11 @@ public void handle(Request request, Response response) throws Exception {
     userSession.checkLoggedIn();
     String assignee = getAssignee(request);
     String key = request.mandatoryParam(PARAM_ISSUE);
-    assign(key, assignee);
-    responseWriter.write(key, request, response);
+    SearchResponseData preloadedResponseData = assign(key, assignee);
+    responseWriter.write(key, preloadedResponseData, request, response);
   }
 
-  private void assign(String issueKey, @Nullable String assignee) {
+  private SearchResponseData assign(String issueKey, @Nullable String assignee) {
     try (DbSession dbSession = dbClient.openSession(false)) {
       IssueDto issueDto = issueFinder.getByKey(dbSession, issueKey);
       DefaultIssue issue = issueDto.toDefaultIssue();
@@ -121,8 +120,9 @@ private void assign(String issueKey, @Nullable String assignee) {
       }
       IssueChangeContext context = IssueChangeContext.createUser(new Date(system2.now()), userSession.getLogin());
       if (issueFieldsSetter.assign(issue, user, context)) {
-        issueUpdater.saveIssue(dbSession, issue, context, null);
+        return issueUpdater.saveIssueAndPreloadSearchResponseData(dbSession, issue, context, null);
       }
+      return new SearchResponseData(issueDto);
     }
   }
 

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

@@ -24,7 +24,7 @@
 import org.sonar.server.ws.WsUtils;
 import org.sonarqube.ws.Issues;
 
-import static java.util.Collections.singletonList;
+import static java.util.Collections.singleton;
 import static org.sonar.server.issue.ws.SearchAdditionalField.ALL_ADDITIONAL_FIELDS;
 
 public class OperationResponseWriter {
@@ -38,12 +38,20 @@ public OperationResponseWriter(SearchResponseLoader loader, SearchResponseFormat
   }
 
   public void write(String issueKey, Request request, Response response) {
-    SearchResponseLoader.Collector collector = new SearchResponseLoader.Collector(
-      ALL_ADDITIONAL_FIELDS, singletonList(issueKey));
+    SearchResponseLoader.Collector collector = new SearchResponseLoader.Collector(ALL_ADDITIONAL_FIELDS, singleton(issueKey));
     SearchResponseData data = loader.load(collector, null);
 
     Issues.Operation responseBody = format.formatOperation(data);
 
     WsUtils.writeProtobuf(responseBody, request, response);
   }
+
+  public void write(String issueKey, SearchResponseData preloadedResponseData, Request request, Response response) {
+    SearchResponseLoader.Collector collector = new SearchResponseLoader.Collector(ALL_ADDITIONAL_FIELDS, singleton(issueKey));
+    SearchResponseData data = loader.load(preloadedResponseData, collector, null);
+
+    Issues.Operation responseBody = format.formatOperation(data);
+
+    WsUtils.writeProtobuf(responseBody, request, response);
+  }
 }

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

@@ -20,6 +20,7 @@
 package org.sonar.server.issue.ws;
 
 import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ListMultimap;
 import java.util.Collection;
 import java.util.HashMap;
@@ -56,6 +57,11 @@ public class SearchResponseData {
   private final ListMultimap<String, Transition> transitionsByIssueKey = ArrayListMultimap.create();
   private final Set<String> updatableComments = new HashSet<>();
 
+  public SearchResponseData(IssueDto issue) {
+    checkNotNull(issue);
+    this.issues = ImmutableList.of(issue);
+  }
+
   public SearchResponseData(List<IssueDto> issues) {
     checkNotNull(issues);
     this.issues = issues;