Skip to content

Commit

Permalink
SONAR-11753 NewIssuesNotification must not do SQL requests
Browse files Browse the repository at this point in the history
for data which is either already retrieved by SendIssueNotificationStep (assignees) or available in the CE task's container
  • Loading branch information
sns-seb authored and sonartech committed Apr 23, 2019
1 parent 5a4caef commit 50957cd
Show file tree
Hide file tree
Showing 16 changed files with 765 additions and 91 deletions.
Expand Up @@ -99,6 +99,7 @@
import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryImpl; import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryImpl;
import org.sonar.ce.task.projectanalysis.measure.MeasureToMeasureDto; import org.sonar.ce.task.projectanalysis.measure.MeasureToMeasureDto;
import org.sonar.ce.task.projectanalysis.metric.MetricModule; import org.sonar.ce.task.projectanalysis.metric.MetricModule;
import org.sonar.ce.task.projectanalysis.notification.NewIssuesNotificationFactory;
import org.sonar.ce.task.projectanalysis.organization.DefaultOrganizationLoader; import org.sonar.ce.task.projectanalysis.organization.DefaultOrganizationLoader;
import org.sonar.ce.task.projectanalysis.period.PeriodHolderImpl; import org.sonar.ce.task.projectanalysis.period.PeriodHolderImpl;
import org.sonar.ce.task.projectanalysis.qualitygate.EvaluationResultTextConverterImpl; import org.sonar.ce.task.projectanalysis.qualitygate.EvaluationResultTextConverterImpl;
Expand Down Expand Up @@ -302,7 +303,10 @@ private static List<Object> componentClasses() {
SmallChangesetQualityGateSpecialCase.class, SmallChangesetQualityGateSpecialCase.class,


// webhooks // webhooks
WebhookPostTask.class); WebhookPostTask.class,

// notifications
NewIssuesNotificationFactory.class);
} }


} }
Expand Up @@ -34,6 +34,9 @@ public interface Rule {


String getName(); String getName();


@CheckForNull
String getLanguage();

RuleStatus getStatus(); RuleStatus getStatus();


/** /**
Expand Down
Expand Up @@ -39,6 +39,7 @@ public class RuleImpl implements Rule {
private final int id; private final int id;
private final RuleKey key; private final RuleKey key;
private final String name; private final String name;
private final String language;
private final RuleStatus status; private final RuleStatus status;
private final Set<String> tags; private final Set<String> tags;
private final DebtRemediationFunction remediationFunction; private final DebtRemediationFunction remediationFunction;
Expand All @@ -51,6 +52,7 @@ public RuleImpl(RuleDto dto) {
this.id = dto.getId(); this.id = dto.getId();
this.key = dto.getKey(); this.key = dto.getKey();
this.name = dto.getName(); this.name = dto.getName();
this.language = dto.getLanguage();
this.status = dto.getStatus(); this.status = dto.getStatus();
this.tags = union(dto.getSystemTags(), dto.getTags()); this.tags = union(dto.getSystemTags(), dto.getTags());
this.remediationFunction = effectiveRemediationFunction(dto); this.remediationFunction = effectiveRemediationFunction(dto);
Expand All @@ -75,6 +77,12 @@ public String getName() {
return name; return name;
} }


@Override
@CheckForNull
public String getLanguage() {
return language;
}

@Override @Override
public RuleStatus getStatus() { public RuleStatus getStatus() {
return status; return status;
Expand Down Expand Up @@ -124,6 +132,7 @@ public String toString() {
.add("id", id) .add("id", id)
.add("key", key) .add("key", key)
.add("name", name) .add("name", name)
.add("language", language)
.add("status", status) .add("status", status)
.add("tags", tags) .add("tags", tags)
.add("pluginKey", pluginKey) .add("pluginKey", pluginKey)
Expand Down
Expand Up @@ -172,6 +172,12 @@ public String getName() {
return addHocRule.getName(); return addHocRule.getName();
} }


@Override
@CheckForNull
public String getLanguage() {
return null;
}

@Override @Override
public RuleStatus getStatus() { public RuleStatus getStatus() {
return RuleStatus.defaultStatus(); return RuleStatus.defaultStatus();
Expand Down
Expand Up @@ -19,27 +19,94 @@
*/ */
package org.sonar.ce.task.projectanalysis.notification; package org.sonar.ce.task.projectanalysis.notification;


import com.google.common.collect.ImmutableMap;
import java.util.Map;
import java.util.Optional;
import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.ce.ComputeEngineSide;
import org.sonar.api.rule.RuleKey;
import org.sonar.api.utils.Durations; import org.sonar.api.utils.Durations;
import org.sonar.db.DbClient; import org.sonar.ce.task.projectanalysis.component.Component;
import org.sonar.ce.task.projectanalysis.component.DepthTraversalTypeAwareCrawler;
import org.sonar.ce.task.projectanalysis.component.TreeRootHolder;
import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter;
import org.sonar.ce.task.projectanalysis.issue.RuleRepository;
import org.sonar.db.user.UserDto;
import org.sonar.server.issue.notification.MyNewIssuesNotification; import org.sonar.server.issue.notification.MyNewIssuesNotification;
import org.sonar.server.issue.notification.NewIssuesNotification; import org.sonar.server.issue.notification.NewIssuesNotification;
import org.sonar.server.issue.notification.NewIssuesNotification.DetailsSupplier;
import org.sonar.server.issue.notification.NewIssuesNotification.RuleDefinition;

import static java.util.Objects.requireNonNull;
import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.PRE_ORDER;
import static org.sonar.ce.task.projectanalysis.component.CrawlerDepthLimit.FILE;


@ComputeEngineSide @ComputeEngineSide
public class NewIssuesNotificationFactory { public class NewIssuesNotificationFactory {
private final DbClient dbClient; private final TreeRootHolder treeRootHolder;
private final RuleRepository ruleRepository;
private final Durations durations; private final Durations durations;
private Map<String, Component> componentsByUuid;


public NewIssuesNotificationFactory(DbClient dbClient, Durations durations) { public NewIssuesNotificationFactory(TreeRootHolder treeRootHolder, RuleRepository ruleRepository, Durations durations) {
this.dbClient = dbClient; this.treeRootHolder = treeRootHolder;
this.ruleRepository = ruleRepository;
this.durations = durations; this.durations = durations;
} }


public MyNewIssuesNotification newMyNewIssuesNotification() { public MyNewIssuesNotification newMyNewIssuesNotification(Map<String, UserDto> assigneesByUuid) {
return new MyNewIssuesNotification(dbClient, durations); verifyAssigneesByUuid(assigneesByUuid);
return new MyNewIssuesNotification(durations, new DetailsSupplierImpl(assigneesByUuid));
}

public NewIssuesNotification newNewIssuesNotification(Map<String, UserDto> assigneesByUuid) {
verifyAssigneesByUuid(assigneesByUuid);
return new NewIssuesNotification(durations, new DetailsSupplierImpl(assigneesByUuid));
}

private static void verifyAssigneesByUuid(Map<String, UserDto> assigneesByUuid) {
requireNonNull(assigneesByUuid, "assigneesByUuid can't be null");
} }


public NewIssuesNotification newNewIssuesNotification() { private class DetailsSupplierImpl implements DetailsSupplier {
return new NewIssuesNotification(dbClient, durations); private final Map<String, UserDto> assigneesByUuid;

private DetailsSupplierImpl(Map<String, UserDto> assigneesByUuid) {
this.assigneesByUuid = assigneesByUuid;
}

@Override
public Optional<RuleDefinition> getRuleDefinitionByRuleKey(RuleKey ruleKey) {
requireNonNull(ruleKey, "ruleKey can't be null");
return ruleRepository.findByKey(ruleKey)
.map(t -> new RuleDefinition(t.getName(), t.getLanguage()));
}

@Override
public Optional<String> getComponentNameByUuid(String uuid) {
requireNonNull(uuid, "uuid can't be null");
return Optional.ofNullable(lazyLoadComponentsByUuid().get(uuid))
.map(t -> t.getType() == Component.Type.FILE || t.getType() == Component.Type.DIRECTORY ? t.getShortName() : t.getName());
}

private Map<String, Component> lazyLoadComponentsByUuid() {
if (componentsByUuid == null) {
ImmutableMap.Builder<String, Component> builder = ImmutableMap.builder();
new DepthTraversalTypeAwareCrawler(new TypeAwareVisitorAdapter(FILE, PRE_ORDER) {
@Override
public void visitAny(Component any) {
builder.put(any.getUuid(), any);
}
}).visit(treeRootHolder.getRoot());
componentsByUuid = builder.build();
}
return componentsByUuid;
}

@Override
public Optional<String> getUserNameByUuid(String uuid) {
requireNonNull(uuid, "uuid can't be null");
return Optional.ofNullable(assigneesByUuid.get(uuid))
.map(UserDto::getName);
}
} }
} }
@@ -0,0 +1,23 @@
/*
* SonarQube
* Copyright (C) 2009-2019 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.
*/
@ParametersAreNonnullByDefault
package org.sonar.ce.task.projectanalysis.notification;

import javax.annotation.ParametersAreNonnullByDefault;
Expand Up @@ -43,6 +43,7 @@
import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter; import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter;
import org.sonar.ce.task.projectanalysis.issue.IssueCache; import org.sonar.ce.task.projectanalysis.issue.IssueCache;
import org.sonar.ce.task.projectanalysis.issue.RuleRepository; import org.sonar.ce.task.projectanalysis.issue.RuleRepository;
import org.sonar.ce.task.projectanalysis.notification.NewIssuesNotificationFactory;
import org.sonar.ce.task.step.ComputationStep; import org.sonar.ce.task.step.ComputationStep;
import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.DefaultIssue;
import org.sonar.core.util.CloseableIterator; import org.sonar.core.util.CloseableIterator;
Expand All @@ -53,13 +54,13 @@
import org.sonar.server.issue.notification.IssueChangeNotification; import org.sonar.server.issue.notification.IssueChangeNotification;
import org.sonar.server.issue.notification.MyNewIssuesNotification; import org.sonar.server.issue.notification.MyNewIssuesNotification;
import org.sonar.server.issue.notification.NewIssuesNotification; import org.sonar.server.issue.notification.NewIssuesNotification;
import org.sonar.ce.task.projectanalysis.notification.NewIssuesNotificationFactory;
import org.sonar.server.issue.notification.NewIssuesStatistics; import org.sonar.server.issue.notification.NewIssuesStatistics;
import org.sonar.server.notification.NotificationService; import org.sonar.server.notification.NotificationService;


import static java.util.Collections.singleton; import static java.util.Collections.singleton;
import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toMap; import static java.util.stream.Collectors.toMap;
import static java.util.stream.Collectors.toSet;
import static java.util.stream.StreamSupport.stream; import static java.util.stream.StreamSupport.stream;
import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER; import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER;
import static org.sonar.db.component.BranchType.PULL_REQUEST; import static org.sonar.db.component.BranchType.PULL_REQUEST;
Expand Down Expand Up @@ -112,19 +113,19 @@ private void doExecute(NotificationStatistics notificationStatistics, Component
long analysisDate = analysisMetadataHolder.getAnalysisDate(); long analysisDate = analysisMetadataHolder.getAnalysisDate();
Predicate<DefaultIssue> isOnLeakPredicate = i -> i.isNew() && i.creationDate().getTime() >= truncateToSeconds(analysisDate); Predicate<DefaultIssue> isOnLeakPredicate = i -> i.isNew() && i.creationDate().getTime() >= truncateToSeconds(analysisDate);
NewIssuesStatistics newIssuesStats = new NewIssuesStatistics(isOnLeakPredicate); NewIssuesStatistics newIssuesStats = new NewIssuesStatistics(isOnLeakPredicate);
Map<String, UserDto> usersDtoByUuids; Map<String, UserDto> assigneesByUuid;
try (DbSession dbSession = dbClient.openSession(false)) { try (DbSession dbSession = dbClient.openSession(false)) {
Iterable<DefaultIssue> iterable = issueCache::traverse; Iterable<DefaultIssue> iterable = issueCache::traverse;
List<String> assigneeUuids = stream(iterable.spliterator(), false).map(DefaultIssue::assignee).filter(Objects::nonNull).collect(toList()); Set<String> assigneeUuids = stream(iterable.spliterator(), false).map(DefaultIssue::assignee).filter(Objects::nonNull).collect(toSet());
usersDtoByUuids = dbClient.userDao().selectByUuids(dbSession, assigneeUuids).stream().collect(toMap(UserDto::getUuid, dto -> dto)); assigneesByUuid = dbClient.userDao().selectByUuids(dbSession, assigneeUuids).stream().collect(toMap(UserDto::getUuid, dto -> dto));
} }


try (CloseableIterator<DefaultIssue> issues = issueCache.traverse()) { try (CloseableIterator<DefaultIssue> issues = issueCache.traverse()) {
processIssues(newIssuesStats, issues, project, usersDtoByUuids, notificationStatistics); processIssues(newIssuesStats, issues, project, assigneesByUuid, notificationStatistics);
} }
if (newIssuesStats.hasIssuesOnLeak()) { if (newIssuesStats.hasIssuesOnLeak()) {
sendNewIssuesNotification(newIssuesStats, project, analysisDate, notificationStatistics); sendNewIssuesNotification(newIssuesStats, project, assigneesByUuid, analysisDate, notificationStatistics);
sendMyNewIssuesNotification(newIssuesStats, project, analysisDate, notificationStatistics); sendMyNewIssuesNotification(newIssuesStats, project, assigneesByUuid, analysisDate, notificationStatistics);
} }
} }


Expand Down Expand Up @@ -163,10 +164,11 @@ private void sendIssueChangeNotification(DefaultIssue issue, Component project,
notificationStatistics.issueChanges++; notificationStatistics.issueChanges++;
} }


private void sendNewIssuesNotification(NewIssuesStatistics statistics, Component project, long analysisDate, NotificationStatistics notificationStatistics) { private void sendNewIssuesNotification(NewIssuesStatistics statistics, Component project, Map<String, UserDto> assigneesByUuid,
long analysisDate, NotificationStatistics notificationStatistics) {
NewIssuesStatistics.Stats globalStatistics = statistics.globalStatistics(); NewIssuesStatistics.Stats globalStatistics = statistics.globalStatistics();
NewIssuesNotification notification = newIssuesNotificationFactory NewIssuesNotification notification = newIssuesNotificationFactory
.newNewIssuesNotification() .newNewIssuesNotification(assigneesByUuid)
.setProject(project.getKey(), project.getName(), getBranchName(), getPullRequest()) .setProject(project.getKey(), project.getName(), getBranchName(), getPullRequest())
.setProjectVersion(project.getProjectAttributes().getProjectVersion()) .setProjectVersion(project.getProjectAttributes().getProjectVersion())
.setAnalysisDate(new Date(analysisDate)) .setAnalysisDate(new Date(analysisDate))
Expand All @@ -179,7 +181,8 @@ private void sendNewIssuesNotification(NewIssuesStatistics statistics, Component
notificationStatistics.newIssuesDeliveries += service.deliver(notification); notificationStatistics.newIssuesDeliveries += service.deliver(notification);
} }


private void sendMyNewIssuesNotification(NewIssuesStatistics statistics, Component project, long analysisDate, NotificationStatistics notificationStatistics) { private void sendMyNewIssuesNotification(NewIssuesStatistics statistics, Component project, Map<String, UserDto> assigneesByUuid, long analysisDate,
NotificationStatistics notificationStatistics) {
Map<String, UserDto> userDtoByUuid = loadUserDtoByUuid(statistics); Map<String, UserDto> userDtoByUuid = loadUserDtoByUuid(statistics);
Set<MyNewIssuesNotification> myNewIssuesNotifications = statistics.getAssigneesStatistics().entrySet() Set<MyNewIssuesNotification> myNewIssuesNotifications = statistics.getAssigneesStatistics().entrySet()
.stream() .stream()
Expand All @@ -188,7 +191,7 @@ private void sendMyNewIssuesNotification(NewIssuesStatistics statistics, Compone
String assigneeUuid = e.getKey(); String assigneeUuid = e.getKey();
NewIssuesStatistics.Stats assigneeStatistics = e.getValue(); NewIssuesStatistics.Stats assigneeStatistics = e.getValue();
MyNewIssuesNotification myNewIssuesNotification = newIssuesNotificationFactory MyNewIssuesNotification myNewIssuesNotification = newIssuesNotificationFactory
.newMyNewIssuesNotification() .newMyNewIssuesNotification(assigneesByUuid)
.setAssignee(userDtoByUuid.get(assigneeUuid)); .setAssignee(userDtoByUuid.get(assigneeUuid));
myNewIssuesNotification myNewIssuesNotification
.setProject(project.getKey(), project.getName(), getBranchName(), getPullRequest()) .setProject(project.getKey(), project.getName(), getBranchName(), getPullRequest())
Expand Down
Expand Up @@ -21,6 +21,7 @@


import java.util.HashSet; import java.util.HashSet;
import java.util.Set; import java.util.Set;
import javax.annotation.CheckForNull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleKey;
import org.sonar.api.rule.RuleStatus; import org.sonar.api.rule.RuleStatus;
Expand All @@ -33,6 +34,7 @@ public class DumbRule implements Rule {
private Integer id; private Integer id;
private RuleKey key; private RuleKey key;
private String name; private String name;
private String language;
private RuleStatus status = RuleStatus.READY; private RuleStatus status = RuleStatus.READY;
private RuleType type = RuleType.CODE_SMELL; private RuleType type = RuleType.CODE_SMELL;
private Set<String> tags = new HashSet<>(); private Set<String> tags = new HashSet<>();
Expand Down Expand Up @@ -61,6 +63,12 @@ public String getName() {
return requireNonNull(name); return requireNonNull(name);
} }


@Override
@CheckForNull
public String getLanguage() {
return language;
}

@Override @Override
public RuleStatus getStatus() { public RuleStatus getStatus() {
return requireNonNull(status); return requireNonNull(status);
Expand Down Expand Up @@ -106,6 +114,11 @@ public DumbRule setName(String name) {
return this; return this;
} }


public DumbRule setLanguage(@Nullable String language) {
this.language = language;
return this;
}

public DumbRule setStatus(RuleStatus status) { public DumbRule setStatus(RuleStatus status) {
this.status = status; this.status = status;
return this; return this;
Expand Down

0 comments on commit 50957cd

Please sign in to comment.