From 50957cdd8a38c05d49843298abc54fbfaaab57ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Lesaint?= Date: Wed, 27 Mar 2019 17:42:39 +0100 Subject: [PATCH] SONAR-11753 NewIssuesNotification must not do SQL requests for data which is either already retrieved by SendIssueNotificationStep (assignees) or available in the CE task's container --- ...ProjectAnalysisTaskContainerPopulator.java | 6 +- .../ce/task/projectanalysis/issue/Rule.java | 3 + .../task/projectanalysis/issue/RuleImpl.java | 9 + .../issue/RuleRepositoryImpl.java | 6 + .../NewIssuesNotificationFactory.java | 83 +++- .../notification/package-info.java | 23 + .../step/SendIssueNotificationsStep.java | 25 +- .../task/projectanalysis/issue/DumbRule.java | 13 + .../NewIssuesNotificationFactoryTest.java | 419 ++++++++++++++++++ .../step/SendIssueNotificationsStepTest.java | 45 +- .../container/ComputeEngineContainerImpl.java | 2 - .../ComputeEngineContainerImplTest.java | 2 +- .../notification/MyNewIssuesNotification.java | 8 +- .../notification/NewIssuesNotification.java | 129 +++--- .../MyNewIssuesNotificationTest.java | 4 +- .../NewIssuesNotificationTest.java | 79 +++- 16 files changed, 765 insertions(+), 91 deletions(-) create mode 100644 server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/package-info.java create mode 100644 server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/notification/NewIssuesNotificationFactoryTest.java diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java index b467ff75ecc2..7afdefb8eeb3 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/container/ProjectAnalysisTaskContainerPopulator.java @@ -99,6 +99,7 @@ import org.sonar.ce.task.projectanalysis.measure.MeasureRepositoryImpl; import org.sonar.ce.task.projectanalysis.measure.MeasureToMeasureDto; 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.period.PeriodHolderImpl; import org.sonar.ce.task.projectanalysis.qualitygate.EvaluationResultTextConverterImpl; @@ -302,7 +303,10 @@ private static List componentClasses() { SmallChangesetQualityGateSpecialCase.class, // webhooks - WebhookPostTask.class); + WebhookPostTask.class, + + // notifications + NewIssuesNotificationFactory.class); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java index 8f08ed703044..6c48ec3d6b72 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/Rule.java @@ -34,6 +34,9 @@ public interface Rule { String getName(); + @CheckForNull + String getLanguage(); + RuleStatus getStatus(); /** diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java index 3aaa7ca472e0..b858bca8acf8 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleImpl.java @@ -39,6 +39,7 @@ public class RuleImpl implements Rule { private final int id; private final RuleKey key; private final String name; + private final String language; private final RuleStatus status; private final Set tags; private final DebtRemediationFunction remediationFunction; @@ -51,6 +52,7 @@ public RuleImpl(RuleDto dto) { this.id = dto.getId(); this.key = dto.getKey(); this.name = dto.getName(); + this.language = dto.getLanguage(); this.status = dto.getStatus(); this.tags = union(dto.getSystemTags(), dto.getTags()); this.remediationFunction = effectiveRemediationFunction(dto); @@ -75,6 +77,12 @@ public String getName() { return name; } + @Override + @CheckForNull + public String getLanguage() { + return language; + } + @Override public RuleStatus getStatus() { return status; @@ -124,6 +132,7 @@ public String toString() { .add("id", id) .add("key", key) .add("name", name) + .add("language", language) .add("status", status) .add("tags", tags) .add("pluginKey", pluginKey) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImpl.java index 50f3fc4ea27c..a800e7030057 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/RuleRepositoryImpl.java @@ -172,6 +172,12 @@ public String getName() { return addHocRule.getName(); } + @Override + @CheckForNull + public String getLanguage() { + return null; + } + @Override public RuleStatus getStatus() { return RuleStatus.defaultStatus(); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/NewIssuesNotificationFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/NewIssuesNotificationFactory.java index 8c1b8efcccef..60b2629c612c 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/NewIssuesNotificationFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/NewIssuesNotificationFactory.java @@ -19,27 +19,94 @@ */ 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.rule.RuleKey; 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.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 public class NewIssuesNotificationFactory { - private final DbClient dbClient; + private final TreeRootHolder treeRootHolder; + private final RuleRepository ruleRepository; private final Durations durations; + private Map componentsByUuid; - public NewIssuesNotificationFactory(DbClient dbClient, Durations durations) { - this.dbClient = dbClient; + public NewIssuesNotificationFactory(TreeRootHolder treeRootHolder, RuleRepository ruleRepository, Durations durations) { + this.treeRootHolder = treeRootHolder; + this.ruleRepository = ruleRepository; this.durations = durations; } - public MyNewIssuesNotification newMyNewIssuesNotification() { - return new MyNewIssuesNotification(dbClient, durations); + public MyNewIssuesNotification newMyNewIssuesNotification(Map assigneesByUuid) { + verifyAssigneesByUuid(assigneesByUuid); + return new MyNewIssuesNotification(durations, new DetailsSupplierImpl(assigneesByUuid)); + } + + public NewIssuesNotification newNewIssuesNotification(Map assigneesByUuid) { + verifyAssigneesByUuid(assigneesByUuid); + return new NewIssuesNotification(durations, new DetailsSupplierImpl(assigneesByUuid)); + } + + private static void verifyAssigneesByUuid(Map assigneesByUuid) { + requireNonNull(assigneesByUuid, "assigneesByUuid can't be null"); } - public NewIssuesNotification newNewIssuesNotification() { - return new NewIssuesNotification(dbClient, durations); + private class DetailsSupplierImpl implements DetailsSupplier { + private final Map assigneesByUuid; + + private DetailsSupplierImpl(Map assigneesByUuid) { + this.assigneesByUuid = assigneesByUuid; + } + + @Override + public Optional 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 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 lazyLoadComponentsByUuid() { + if (componentsByUuid == null) { + ImmutableMap.Builder 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 getUserNameByUuid(String uuid) { + requireNonNull(uuid, "uuid can't be null"); + return Optional.ofNullable(assigneesByUuid.get(uuid)) + .map(UserDto::getName); + } } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/package-info.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/package-info.java new file mode 100644 index 000000000000..27263b4fbed3 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/notification/package-info.java @@ -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; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java index b6737988677e..b12af90e7674 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStep.java @@ -43,6 +43,7 @@ import org.sonar.ce.task.projectanalysis.component.TypeAwareVisitorAdapter; import org.sonar.ce.task.projectanalysis.issue.IssueCache; 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.core.issue.DefaultIssue; import org.sonar.core.util.CloseableIterator; @@ -53,13 +54,13 @@ import org.sonar.server.issue.notification.IssueChangeNotification; import org.sonar.server.issue.notification.MyNewIssuesNotification; 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.notification.NotificationService; import static java.util.Collections.singleton; import static java.util.stream.Collectors.toList; import static java.util.stream.Collectors.toMap; +import static java.util.stream.Collectors.toSet; import static java.util.stream.StreamSupport.stream; import static org.sonar.ce.task.projectanalysis.component.ComponentVisitor.Order.POST_ORDER; import static org.sonar.db.component.BranchType.PULL_REQUEST; @@ -112,19 +113,19 @@ private void doExecute(NotificationStatistics notificationStatistics, Component long analysisDate = analysisMetadataHolder.getAnalysisDate(); Predicate isOnLeakPredicate = i -> i.isNew() && i.creationDate().getTime() >= truncateToSeconds(analysisDate); NewIssuesStatistics newIssuesStats = new NewIssuesStatistics(isOnLeakPredicate); - Map usersDtoByUuids; + Map assigneesByUuid; try (DbSession dbSession = dbClient.openSession(false)) { Iterable iterable = issueCache::traverse; - List assigneeUuids = stream(iterable.spliterator(), false).map(DefaultIssue::assignee).filter(Objects::nonNull).collect(toList()); - usersDtoByUuids = dbClient.userDao().selectByUuids(dbSession, assigneeUuids).stream().collect(toMap(UserDto::getUuid, dto -> dto)); + Set assigneeUuids = stream(iterable.spliterator(), false).map(DefaultIssue::assignee).filter(Objects::nonNull).collect(toSet()); + assigneesByUuid = dbClient.userDao().selectByUuids(dbSession, assigneeUuids).stream().collect(toMap(UserDto::getUuid, dto -> dto)); } try (CloseableIterator issues = issueCache.traverse()) { - processIssues(newIssuesStats, issues, project, usersDtoByUuids, notificationStatistics); + processIssues(newIssuesStats, issues, project, assigneesByUuid, notificationStatistics); } if (newIssuesStats.hasIssuesOnLeak()) { - sendNewIssuesNotification(newIssuesStats, project, analysisDate, notificationStatistics); - sendMyNewIssuesNotification(newIssuesStats, project, analysisDate, notificationStatistics); + sendNewIssuesNotification(newIssuesStats, project, assigneesByUuid, analysisDate, notificationStatistics); + sendMyNewIssuesNotification(newIssuesStats, project, assigneesByUuid, analysisDate, notificationStatistics); } } @@ -163,10 +164,11 @@ private void sendIssueChangeNotification(DefaultIssue issue, Component project, notificationStatistics.issueChanges++; } - private void sendNewIssuesNotification(NewIssuesStatistics statistics, Component project, long analysisDate, NotificationStatistics notificationStatistics) { + private void sendNewIssuesNotification(NewIssuesStatistics statistics, Component project, Map assigneesByUuid, + long analysisDate, NotificationStatistics notificationStatistics) { NewIssuesStatistics.Stats globalStatistics = statistics.globalStatistics(); NewIssuesNotification notification = newIssuesNotificationFactory - .newNewIssuesNotification() + .newNewIssuesNotification(assigneesByUuid) .setProject(project.getKey(), project.getName(), getBranchName(), getPullRequest()) .setProjectVersion(project.getProjectAttributes().getProjectVersion()) .setAnalysisDate(new Date(analysisDate)) @@ -179,7 +181,8 @@ private void sendNewIssuesNotification(NewIssuesStatistics statistics, Component notificationStatistics.newIssuesDeliveries += service.deliver(notification); } - private void sendMyNewIssuesNotification(NewIssuesStatistics statistics, Component project, long analysisDate, NotificationStatistics notificationStatistics) { + private void sendMyNewIssuesNotification(NewIssuesStatistics statistics, Component project, Map assigneesByUuid, long analysisDate, + NotificationStatistics notificationStatistics) { Map userDtoByUuid = loadUserDtoByUuid(statistics); Set myNewIssuesNotifications = statistics.getAssigneesStatistics().entrySet() .stream() @@ -188,7 +191,7 @@ private void sendMyNewIssuesNotification(NewIssuesStatistics statistics, Compone String assigneeUuid = e.getKey(); NewIssuesStatistics.Stats assigneeStatistics = e.getValue(); MyNewIssuesNotification myNewIssuesNotification = newIssuesNotificationFactory - .newMyNewIssuesNotification() + .newMyNewIssuesNotification(assigneesByUuid) .setAssignee(userDtoByUuid.get(assigneeUuid)); myNewIssuesNotification .setProject(project.getKey(), project.getName(), getBranchName(), getPullRequest()) diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/DumbRule.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/DumbRule.java index 731fcc2ab811..9861faf598f6 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/DumbRule.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/DumbRule.java @@ -21,6 +21,7 @@ import java.util.HashSet; import java.util.Set; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; @@ -33,6 +34,7 @@ public class DumbRule implements Rule { private Integer id; private RuleKey key; private String name; + private String language; private RuleStatus status = RuleStatus.READY; private RuleType type = RuleType.CODE_SMELL; private Set tags = new HashSet<>(); @@ -61,6 +63,12 @@ public String getName() { return requireNonNull(name); } + @Override + @CheckForNull + public String getLanguage() { + return language; + } + @Override public RuleStatus getStatus() { return requireNonNull(status); @@ -106,6 +114,11 @@ public DumbRule setName(String name) { return this; } + public DumbRule setLanguage(@Nullable String language) { + this.language = language; + return this; + } + public DumbRule setStatus(RuleStatus status) { this.status = status; return this; diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/notification/NewIssuesNotificationFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/notification/NewIssuesNotificationFactoryTest.java new file mode 100644 index 000000000000..37a9210253f3 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/notification/NewIssuesNotificationFactoryTest.java @@ -0,0 +1,419 @@ +/* + * 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. + */ +package org.sonar.ce.task.projectanalysis.notification; + +import com.google.common.collect.ImmutableMap; +import java.lang.reflect.Field; +import java.util.Random; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import java.util.stream.Stream; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.utils.Durations; +import org.sonar.ce.task.projectanalysis.component.ReportComponent; +import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; +import org.sonar.ce.task.projectanalysis.issue.DumbRule; +import org.sonar.ce.task.projectanalysis.issue.RuleRepositoryRule; +import org.sonar.db.user.UserDto; +import org.sonar.db.user.UserTesting; +import org.sonar.server.issue.notification.MyNewIssuesNotification; +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.Collections.emptyMap; +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.ce.task.projectanalysis.component.Component.Type.DIRECTORY; +import static org.sonar.ce.task.projectanalysis.component.Component.Type.FILE; +import static org.sonar.ce.task.projectanalysis.component.Component.Type.PROJECT; +import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; + +public class NewIssuesNotificationFactoryTest { + @Rule + public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule(); + @Rule + public RuleRepositoryRule ruleRepository = new RuleRepositoryRule(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + private Durations durations = new Durations(); + private NewIssuesNotificationFactory underTest = new NewIssuesNotificationFactory(treeRootHolder, ruleRepository, durations); + + @Test + public void newMyNewIssuesNotification_throws_NPE_if_assigneesByUuid_is_null() { + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("assigneesByUuid can't be null"); + + underTest.newMyNewIssuesNotification(null); + } + + @Test + public void newNewIssuesNotification_throws_NPE_if_assigneesByUuid_is_null() { + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("assigneesByUuid can't be null"); + + underTest.newNewIssuesNotification(null); + } + + @Test + public void newMyNewIssuesNotification_returns_MyNewIssuesNotification_object_with_the_constructor_Durations() { + MyNewIssuesNotification notification = underTest.newMyNewIssuesNotification(emptyMap()); + + assertThat(readDurationsField(notification)).isSameAs(durations); + } + + @Test + public void newNewIssuesNotification_returns_NewIssuesNotification_object_with_the_constructor_Durations() { + NewIssuesNotification notification = underTest.newNewIssuesNotification(emptyMap()); + + assertThat(readDurationsField(notification)).isSameAs(durations); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getUserNameByUuid_fails_with_NPE_if_uuid_is_null() { + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("uuid can't be null"); + + detailsSupplier.getUserNameByUuid(null); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getUserNameByUuid_always_returns_empty_if_map_argument_is_empty() { + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + assertThat(detailsSupplier.getUserNameByUuid("foo")).isEmpty(); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getUserNameByUuid_returns_name_of_user_from_map_argument() { + Set users = IntStream.range(0, 1 + new Random().nextInt(10)) + .mapToObj(i -> UserTesting.newUserDto().setLogin("user" + i)) + .collect(Collectors.toSet()); + + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification( + users.stream().collect(uniqueIndex(UserDto::getUuid))); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + assertThat(detailsSupplier.getUserNameByUuid("foo")).isEmpty(); + users + .forEach(user -> assertThat(detailsSupplier.getUserNameByUuid(user.getUuid())).contains(user.getName())); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getUserNameByUuid_returns_empty_if_user_has_null_name() { + UserDto user = UserTesting.newUserDto().setLogin("user_noname").setName(null); + + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(ImmutableMap.of(user.getUuid(), user)); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + assertThat(detailsSupplier.getUserNameByUuid(user.getUuid())).isEmpty(); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getUserNameByUuid_fails_with_NPE_if_uuid_is_null() { + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("uuid can't be null"); + + detailsSupplier.getUserNameByUuid(null); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getUserNameByUuid_always_returns_empty_if_map_argument_is_empty() { + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + assertThat(detailsSupplier.getUserNameByUuid("foo")).isEmpty(); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getUserNameByUuid_returns_name_of_user_from_map_argument() { + Set users = IntStream.range(0, 1 + new Random().nextInt(10)) + .mapToObj(i -> UserTesting.newUserDto().setLogin("user" + i)) + .collect(Collectors.toSet()); + + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification( + users.stream().collect(uniqueIndex(UserDto::getUuid))); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + assertThat(detailsSupplier.getUserNameByUuid("foo")).isEmpty(); + users + .forEach(user -> assertThat(detailsSupplier.getUserNameByUuid(user.getUuid())).contains(user.getName())); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getUserNameByUuid_returns_empty_if_user_has_null_name() { + UserDto user = UserTesting.newUserDto().setLogin("user_noname").setName(null); + + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(ImmutableMap.of(user.getUuid(), user)); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + assertThat(detailsSupplier.getUserNameByUuid(user.getUuid())).isEmpty(); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getComponentNameByUuid_fails_with_ISE_if_TreeRootHolder_is_not_initialized() { + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("Holder has not been initialized yet"); + + detailsSupplier.getComponentNameByUuid("foo"); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getComponentNameByUuid_fails_with_NPE_if_uuid_is_null() { + treeRootHolder.setRoot(ReportComponent.builder(PROJECT, 1).setUuid("rootUuid").setName("root").build()); + + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("uuid can't be null"); + + detailsSupplier.getComponentNameByUuid(null); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getComponentNameByUuid_returns_name_of_project_in_TreeRootHolder() { + treeRootHolder.setRoot(ReportComponent.builder(PROJECT, 1).setUuid("rootUuid").setName("root").build()); + + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + assertThat(detailsSupplier.getComponentNameByUuid("rootUuid")).contains("root"); + assertThat(detailsSupplier.getComponentNameByUuid("foo")).isEmpty(); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getComponentNameByUuid_returns_shortName_of_dir_and_file_in_TreeRootHolder() { + treeRootHolder.setRoot(ReportComponent.builder(PROJECT, 1).setUuid("rootUuid").setName("root") + .addChildren(ReportComponent.builder(DIRECTORY, 2).setUuid("dir1Uuid").setName("dir1").setShortName("dir1_short") + .addChildren(ReportComponent.builder(FILE, 21).setUuid("file21Uuid").setName("file21").setShortName("file21_short").build()) + .build()) + .addChildren(ReportComponent.builder(DIRECTORY, 3).setUuid("dir2Uuid").setName("dir2").setShortName("dir2_short") + .addChildren(ReportComponent.builder(FILE, 31).setUuid("file31Uuid").setName("file31").setShortName("file31_short").build()) + .addChildren(ReportComponent.builder(FILE, 32).setUuid("file32Uuid").setName("file32").setShortName("file32_short").build()) + .build()) + .addChildren(ReportComponent.builder(FILE, 11).setUuid("file11Uuid").setName("file11").setShortName("file11_short").build()) + .build()); + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + Stream.of("dir1", "dir2", "file11", "file21", "file31", "file32") + .forEach(name -> { + assertThat(detailsSupplier.getComponentNameByUuid(name + "Uuid")).contains(name + "_short"); + assertThat(detailsSupplier.getComponentNameByUuid(name)).isEmpty(); + }); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getComponentNameByUuid_fails_with_ISE_if_TreeRootHolder_is_not_initialized() { + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + expectedException.expect(IllegalStateException.class); + expectedException.expectMessage("Holder has not been initialized yet"); + + detailsSupplier.getComponentNameByUuid("foo"); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getComponentNameByUuid_fails_with_NPE_if_uuid_is_null() { + treeRootHolder.setRoot(ReportComponent.builder(PROJECT, 1).setUuid("rootUuid").setName("root").build()); + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("uuid can't be null"); + + detailsSupplier.getComponentNameByUuid(null); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getComponentNameByUuid_returns_name_of_project_in_TreeRootHolder() { + treeRootHolder.setRoot(ReportComponent.builder(PROJECT, 1).setUuid("rootUuid").setName("root").build()); + + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + assertThat(detailsSupplier.getComponentNameByUuid("rootUuid")).contains("root"); + assertThat(detailsSupplier.getComponentNameByUuid("foo")).isEmpty(); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getComponentNameByUuid_returns_shortName_of_dir_and_file_in_TreeRootHolder() { + treeRootHolder.setRoot(ReportComponent.builder(PROJECT, 1).setUuid("rootUuid").setName("root") + .addChildren(ReportComponent.builder(DIRECTORY, 2).setUuid("dir1Uuid").setName("dir1").setShortName("dir1_short") + .addChildren(ReportComponent.builder(FILE, 21).setUuid("file21Uuid").setName("file21").setShortName("file21_short").build()) + .build()) + .addChildren(ReportComponent.builder(DIRECTORY, 3).setUuid("dir2Uuid").setName("dir2").setShortName("dir2_short") + .addChildren(ReportComponent.builder(FILE, 31).setUuid("file31Uuid").setName("file31").setShortName("file31_short").build()) + .addChildren(ReportComponent.builder(FILE, 32).setUuid("file32Uuid").setName("file32").setShortName("file32_short").build()) + .build()) + .addChildren(ReportComponent.builder(FILE, 11).setUuid("file11Uuid").setName("file11").setShortName("file11_short").build()) + .build()); + + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + Stream.of("dir1", "dir2", "file11", "file21", "file31", "file32") + .forEach(name -> { + assertThat(detailsSupplier.getComponentNameByUuid(name + "Uuid")).contains(name + "_short"); + assertThat(detailsSupplier.getComponentNameByUuid(name)).isEmpty(); + }); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getRuleDefinitionByRuleKey_fails_with_NPE_if_ruleKey_is_null() { + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("ruleKey can't be null"); + + detailsSupplier.getRuleDefinitionByRuleKey(null); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getRuleDefinitionByRuleKey_always_returns_empty_if_RuleRepository_is_empty() { + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(RuleKey.of("foo", "bar"))).isEmpty(); + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(RuleKey.of("bar", "foo"))).isEmpty(); + } + + @Test + public void newMyNewIssuesNotification_DetailsSupplier_getRuleDefinitionByRuleKey_returns_name_and_language_from_RuleRepository() { + RuleKey rulekey1 = RuleKey.of("foo", "bar"); + RuleKey rulekey2 = RuleKey.of("foo", "donut"); + RuleKey rulekey3 = RuleKey.of("no", "language"); + DumbRule rule1 = ruleRepository.add(rulekey1).setName("rule1").setLanguage("lang1"); + DumbRule rule2 = ruleRepository.add(rulekey2).setName("rule2").setLanguage("lang2"); + DumbRule rule3 = ruleRepository.add(rulekey3).setName("rule3"); + + MyNewIssuesNotification underTest = this.underTest.newMyNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(rulekey1)) + .contains(new RuleDefinition(rule1.getName(), rule1.getLanguage())); + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(rulekey2)) + .contains(new RuleDefinition(rule2.getName(), rule2.getLanguage())); + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(rulekey3)) + .contains(new RuleDefinition(rule3.getName(), null)); + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(RuleKey.of("donut", "foo"))) + .isEmpty(); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getRuleDefinitionByRuleKey_fails_with_NPE_if_ruleKey_is_null() { + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + expectedException.expect(NullPointerException.class); + expectedException.expectMessage("ruleKey can't be null"); + + detailsSupplier.getRuleDefinitionByRuleKey(null); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getRuleDefinitionByRuleKey_always_returns_empty_if_RuleRepository_is_empty() { + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(RuleKey.of("foo", "bar"))).isEmpty(); + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(RuleKey.of("bar", "foo"))).isEmpty(); + } + + @Test + public void newNewIssuesNotification_DetailsSupplier_getRuleDefinitionByRuleKey_returns_name_and_language_from_RuleRepository() { + RuleKey rulekey1 = RuleKey.of("foo", "bar"); + RuleKey rulekey2 = RuleKey.of("foo", "donut"); + RuleKey rulekey3 = RuleKey.of("no", "language"); + DumbRule rule1 = ruleRepository.add(rulekey1).setName("rule1").setLanguage("lang1"); + DumbRule rule2 = ruleRepository.add(rulekey2).setName("rule2").setLanguage("lang2"); + DumbRule rule3 = ruleRepository.add(rulekey3).setName("rule3"); + + NewIssuesNotification underTest = this.underTest.newNewIssuesNotification(emptyMap()); + + DetailsSupplier detailsSupplier = readDetailsSupplier(underTest); + + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(rulekey1)) + .contains(new RuleDefinition(rule1.getName(), rule1.getLanguage())); + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(rulekey2)) + .contains(new RuleDefinition(rule2.getName(), rule2.getLanguage())); + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(rulekey3)) + .contains(new RuleDefinition(rule3.getName(), null)); + assertThat(detailsSupplier.getRuleDefinitionByRuleKey(RuleKey.of("donut", "foo"))) + .isEmpty(); + } + + private static Durations readDurationsField(NewIssuesNotification notification) { + return readField(notification, "durations"); + } + + private static Durations readField(NewIssuesNotification notification, String fieldName) { + try { + Field durationsField = NewIssuesNotification.class.getDeclaredField(fieldName); + durationsField.setAccessible(true); + Object o = durationsField.get(notification); + return (Durations) o; + } catch (IllegalAccessException | NoSuchFieldException e) { + throw new RuntimeException(e); + } + } + + private static DetailsSupplier readDetailsSupplier(NewIssuesNotification notification) { + try { + Field durationsField = NewIssuesNotification.class.getDeclaredField("detailsSupplier"); + durationsField.setAccessible(true); + return (DetailsSupplier) durationsField.get(notification); + } catch (IllegalAccessException | NoSuchFieldException e) { + throw new RuntimeException(e); + } + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java index 6ff4f15ff70c..3de187557bc9 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/SendIssueNotificationsStepTest.java @@ -28,6 +28,7 @@ import java.util.Random; import java.util.stream.IntStream; import java.util.stream.Stream; +import org.assertj.core.groups.Tuple; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -44,6 +45,7 @@ import org.sonar.ce.task.projectanalysis.component.TreeRootHolderRule; import org.sonar.ce.task.projectanalysis.issue.IssueCache; import org.sonar.ce.task.projectanalysis.issue.RuleRepositoryRule; +import org.sonar.ce.task.projectanalysis.notification.NewIssuesNotificationFactory; import org.sonar.ce.task.projectanalysis.util.cache.DiskCache; import org.sonar.ce.task.step.ComputationStep; import org.sonar.ce.task.step.TestComputationStepContext; @@ -56,7 +58,6 @@ import org.sonar.server.issue.notification.IssueChangeNotification; import org.sonar.server.issue.notification.MyNewIssuesNotification; 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.notification.NotificationService; @@ -67,13 +68,16 @@ import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.apache.commons.lang.math.RandomUtils.nextInt; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.groups.Tuple.tuple; import static org.mockito.ArgumentCaptor.forClass; import static org.mockito.ArgumentMatchers.anyCollection; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static org.sonar.ce.task.projectanalysis.component.Component.Type; import static org.sonar.ce.task.projectanalysis.component.ReportComponent.builder; @@ -119,6 +123,9 @@ public class SendIssueNotificationsStepTest extends BaseStepTest { private final Random random = new Random(); private final RuleType[] RULE_TYPES_EXCEPT_HOTSPOTS = Stream.of(RuleType.values()).filter(r -> r != RuleType.SECURITY_HOTSPOT).toArray(RuleType[]::new); private final RuleType randomRuleType = RULE_TYPES_EXCEPT_HOTSPOTS[random.nextInt(RULE_TYPES_EXCEPT_HOTSPOTS.length)]; + @SuppressWarnings("unchecked") + private Class> assigneeCacheType = (Class>) (Object) Map.class; + private ArgumentCaptor> assigneeCacheCaptor = ArgumentCaptor.forClass(assigneeCacheType); private NotificationService notificationService = mock(NotificationService.class); private NewIssuesNotificationFactory newIssuesNotificationFactory = mock(NewIssuesNotificationFactory.class); private NewIssuesNotification newIssuesNotificationMock = createNewIssuesNotificationMock(); @@ -133,8 +140,8 @@ public void setUp() throws Exception { underTest = new SendIssueNotificationsStep(issueCache, ruleRepository, treeRootHolder, notificationService, analysisMetadataHolder, newIssuesNotificationFactory, db.getDbClient()); - when(newIssuesNotificationFactory.newNewIssuesNotification()).thenReturn(newIssuesNotificationMock); - when(newIssuesNotificationFactory.newMyNewIssuesNotification()).thenReturn(myNewIssuesNotificationMock); + when(newIssuesNotificationFactory.newNewIssuesNotification(any(assigneeCacheType))).thenReturn(newIssuesNotificationMock); + when(newIssuesNotificationFactory.newMyNewIssuesNotification(any(assigneeCacheType))).thenReturn(myNewIssuesNotificationMock); } @Test @@ -326,11 +333,14 @@ public void send_new_issues_notification_to_user_only_for_those_assigned_to_her( NewIssuesNotificationFactory newIssuesNotificationFactory = mock(NewIssuesNotificationFactory.class); NewIssuesNotification newIssuesNotificationMock = createNewIssuesNotificationMock(); - when(newIssuesNotificationFactory.newNewIssuesNotification()).thenReturn(newIssuesNotificationMock); + when(newIssuesNotificationFactory.newNewIssuesNotification(assigneeCacheCaptor.capture())) + .thenReturn(newIssuesNotificationMock); MyNewIssuesNotification myNewIssuesNotificationMock1 = createMyNewIssuesNotificationMock(); MyNewIssuesNotification myNewIssuesNotificationMock2 = createMyNewIssuesNotificationMock(); - when(newIssuesNotificationFactory.newMyNewIssuesNotification()).thenReturn(myNewIssuesNotificationMock1).thenReturn(myNewIssuesNotificationMock2); + when(newIssuesNotificationFactory.newMyNewIssuesNotification(any(assigneeCacheType))) + .thenReturn(myNewIssuesNotificationMock1) + .thenReturn(myNewIssuesNotificationMock2); TestComputationStepContext context = new TestComputationStepContext(); new SendIssueNotificationsStep(issueCache, ruleRepository, treeRootHolder, notificationService, analysisMetadataHolder, newIssuesNotificationFactory, db.getDbClient()) @@ -341,6 +351,11 @@ public void send_new_issues_notification_to_user_only_for_those_assigned_to_her( verify(notificationService).deliver(myNewIssuesNotificationMock1); verify(notificationService).deliver(myNewIssuesNotificationMock2); + verify(newIssuesNotificationFactory).newNewIssuesNotification(assigneeCacheCaptor.capture()); + verify(newIssuesNotificationFactory, times(2)).newMyNewIssuesNotification(assigneeCacheCaptor.capture()); + verifyNoMoreInteractions(newIssuesNotificationFactory); + verifyAssigneeCache(assigneeCacheCaptor, perceval, arthur); + Map myNewIssuesNotificationMocksByUsersName = new HashMap<>(); ArgumentCaptor userCaptor1 = forClass(UserDto.class); verify(myNewIssuesNotificationMock1).setAssignee(userCaptor1.capture()); @@ -394,6 +409,12 @@ public void send_new_issues_notification_to_user_only_for_non_backdated_issues() // old API compatibility verify(notificationService).deliver(myNewIssuesNotificationMock); + + verify(newIssuesNotificationFactory).newNewIssuesNotification(assigneeCacheCaptor.capture()); + verify(newIssuesNotificationFactory).newMyNewIssuesNotification(assigneeCacheCaptor.capture()); + verifyNoMoreInteractions(newIssuesNotificationFactory); + verifyAssigneeCache(assigneeCacheCaptor, user); + verify(myNewIssuesNotificationMock).setAssignee(any(UserDto.class)); ArgumentCaptor statsCaptor = forClass(NewIssuesStatistics.Stats.class); verify(myNewIssuesNotificationMock).setStatistics(eq(PROJECT.getName()), statsCaptor.capture()); @@ -408,6 +429,17 @@ public void send_new_issues_notification_to_user_only_for_non_backdated_issues() verifyStatistics(context, 1, 1, 0); } + private static void verifyAssigneeCache(ArgumentCaptor> assigneeCacheCaptor, UserDto... users) { + Map cache = assigneeCacheCaptor.getAllValues().iterator().next(); + assertThat(assigneeCacheCaptor.getAllValues()) + .filteredOn(t -> t != cache) + .isEmpty(); + Tuple[] expected = stream(users).map(user -> tuple(user.getUuid(), user.getUuid(), user.getId(), user.getLogin())).toArray(Tuple[]::new); + assertThat(cache.entrySet()) + .extracting(t -> t.getKey(), t -> t.getValue().getUuid(), t -> t.getValue().getId(), t -> t.getValue().getLogin()) + .containsOnly(expected); + } + @Test public void do_not_send_new_issues_notification_to_user_if_issue_is_backdated() { UserDto user = db.users().insertUser(); @@ -569,7 +601,8 @@ private ComponentDto setUpProjectWithBranch() { return branch; } - private static void verifyStatistics(TestComputationStepContext context, int expectedNewIssuesNotifications, int expectedMyNewIssuesNotifications, int expectedIssueChangesNotifications) { + private static void verifyStatistics(TestComputationStepContext context, int expectedNewIssuesNotifications, int expectedMyNewIssuesNotifications, + int expectedIssueChangesNotifications) { context.getStatistics().assertValue("newIssuesNotifs", expectedNewIssuesNotifications); context.getStatistics().assertValue("myNewIssuesNotifs", expectedMyNewIssuesNotifications); context.getStatistics().assertValue("changesNotifs", expectedIssueChangesNotifications); diff --git a/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java b/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java index fe0b5ae6f304..0aac9ec7ad4c 100644 --- a/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java +++ b/server/sonar-ce/src/main/java/org/sonar/ce/container/ComputeEngineContainerImpl.java @@ -105,7 +105,6 @@ import org.sonar.server.issue.notification.MyNewIssuesEmailTemplate; import org.sonar.server.issue.notification.MyNewIssuesNotificationHandler; import org.sonar.server.issue.notification.NewIssuesEmailTemplate; -import org.sonar.ce.task.projectanalysis.notification.NewIssuesNotificationFactory; import org.sonar.server.issue.notification.NewIssuesNotificationHandler; import org.sonar.server.issue.workflow.FunctionExecutor; import org.sonar.server.issue.workflow.IssueWorkflow; @@ -412,7 +411,6 @@ private static void populateLevel4(ComponentContainer container, Props props) { MyNewIssuesNotificationHandler.newMetadata(), DoNotFixNotificationDispatcher.class, DoNotFixNotificationDispatcher.newMetadata(), - NewIssuesNotificationFactory.class, // used by SendIssueNotificationsStep // Notifications AlertsEmailTemplate.class, diff --git a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java index 023ed44a9ee7..f0033d343922 100644 --- a/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java +++ b/server/sonar-ce/src/test/java/org/sonar/ce/container/ComputeEngineContainerImplTest.java @@ -97,7 +97,7 @@ public void test_real_start() throws IOException { assertThat(picoContainer.getComponentAdapters()) .hasSize( CONTAINER_ITSELF - + 68 // level 4 + + 67 // level 4 + 6 // content of CeConfigurationModule + 4 // content of CeQueueModule + 3 // content of CeHttpModule diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotification.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotification.java index d8651e599dd1..7e1d82ae9bef 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotification.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/MyNewIssuesNotification.java @@ -20,20 +20,18 @@ package org.sonar.server.issue.notification; import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import org.sonar.api.utils.Durations; -import org.sonar.db.DbClient; import org.sonar.db.user.UserDto; -import javax.annotation.Nullable; - import static org.sonar.server.issue.notification.AbstractNewIssuesEmailTemplate.FIELD_ASSIGNEE; public class MyNewIssuesNotification extends NewIssuesNotification { public static final String MY_NEW_ISSUES_NOTIF_TYPE = "my-new-issues"; - public MyNewIssuesNotification(DbClient dbClient, Durations durations) { - super(MY_NEW_ISSUES_NOTIF_TYPE, dbClient, durations); + public MyNewIssuesNotification(Durations durations, DetailsSupplier detailsSupplier) { + super(MY_NEW_ISSUES_NOTIF_TYPE, durations, detailsSupplier); } public MyNewIssuesNotification setAssignee(@Nullable UserDto assignee) { diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java index fcfbe7c3b8d8..13bdb7b353d0 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/notification/NewIssuesNotification.java @@ -26,10 +26,10 @@ import java.util.Map; import java.util.Objects; import java.util.Optional; -import java.util.Set; import java.util.function.ToIntFunction; import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import javax.annotation.concurrent.Immutable; import org.sonar.api.notifications.Notification; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RuleType; @@ -37,16 +37,9 @@ import org.sonar.api.utils.Duration; import org.sonar.api.utils.Durations; import org.sonar.core.util.stream.MoreCollectors; -import org.sonar.db.DbClient; -import org.sonar.db.DbSession; -import org.sonar.db.RowNotFoundException; -import org.sonar.db.component.ComponentDto; -import org.sonar.db.rule.RuleDefinitionDto; -import org.sonar.db.user.UserDto; import org.sonar.server.issue.notification.NewIssuesStatistics.Metric; -import static java.util.stream.Collectors.toMap; -import static java.util.stream.Collectors.toSet; +import static java.util.Objects.requireNonNull; import static org.sonar.server.issue.notification.AbstractNewIssuesEmailTemplate.FIELD_BRANCH; import static org.sonar.server.issue.notification.AbstractNewIssuesEmailTemplate.FIELD_PROJECT_VERSION; import static org.sonar.server.issue.notification.AbstractNewIssuesEmailTemplate.FIELD_PULL_REQUEST; @@ -63,17 +56,76 @@ public class NewIssuesNotification extends Notification { private static final String LABEL = ".label"; private static final String DOT = "."; - private final transient DbClient dbClient; + private final transient DetailsSupplier detailsSupplier; private final transient Durations durations; - public NewIssuesNotification(DbClient dbClient, Durations durations) { - this(TYPE, dbClient, durations); + public NewIssuesNotification(Durations durations, DetailsSupplier detailsSupplier) { + this(TYPE, durations, detailsSupplier); } - protected NewIssuesNotification(String type, DbClient dbClient, Durations durations) { + protected NewIssuesNotification(String type, Durations durations, DetailsSupplier detailsSupplier) { super(type); - this.dbClient = dbClient; this.durations = durations; + this.detailsSupplier = detailsSupplier; + } + + public interface DetailsSupplier { + /** + * @throws NullPointerException if {@code ruleKey} is {@code null} + */ + Optional getRuleDefinitionByRuleKey(RuleKey ruleKey); + + /** + * @throws NullPointerException if {@code uuid} is {@code null} + */ + Optional getComponentNameByUuid(String uuid); + + /** + * @throws NullPointerException if {@code uuid} is {@code null} + */ + Optional getUserNameByUuid(String uuid); + } + + @Immutable + public static final class RuleDefinition { + private final String name; + private final String language; + + public RuleDefinition(String name, @Nullable String language) { + this.name = requireNonNull(name, "name can't be null"); + this.language = language; + } + + public String getName() { + return name; + } + + @CheckForNull + public String getLanguage() { + return language; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + RuleDefinition that = (RuleDefinition) o; + return name.equals(that.name) && Objects.equals(language, that.language); + } + + @Override + public int hashCode() { + return Objects.hash(name, language); + } + + @Override + public String toString() { + return "RuleDefinition{" + name + " (" + language + ')' + '}'; + } } public NewIssuesNotification setAnalysisDate(Date d) { @@ -108,33 +160,23 @@ public NewIssuesNotification setProjectVersion(@Nullable String version) { public NewIssuesNotification setStatistics(String projectName, NewIssuesStatistics.Stats stats) { setDefaultMessage(stats.getDistributedMetricStats(RULE_TYPE).getOnLeak() + " new issues on " + projectName + ".\n"); - try (DbSession dbSession = dbClient.openSession(false)) { - setRuleTypeStatistics(stats); - setAssigneesStatistics(dbSession, stats); - setTagsStatistics(stats); - setComponentsStatistics(dbSession, stats); - setRuleStatistics(dbSession, stats); - } + setRuleTypeStatistics(stats); + setAssigneesStatistics(stats); + setTagsStatistics(stats); + setComponentsStatistics(stats); + setRuleStatistics(stats); return this; } - private void setRuleStatistics(DbSession dbSession, NewIssuesStatistics.Stats stats) { + private void setRuleStatistics(NewIssuesStatistics.Stats stats) { Metric metric = Metric.RULE; List> fiveBiggest = fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getOnLeak); - Set ruleKeys = fiveBiggest - .stream() - .map(Map.Entry::getKey) - .map(RuleKey::parse) - .collect(MoreCollectors.toSet(fiveBiggest.size())); - Map ruleByRuleKey = dbClient.ruleDao().selectDefinitionByKeys(dbSession, ruleKeys) - .stream() - .collect(MoreCollectors.uniqueIndex(s -> s.getKey().toString())); int i = 1; for (Map.Entry ruleStats : fiveBiggest) { String ruleKey = ruleStats.getKey(); - RuleDefinitionDto rule = Optional.ofNullable(ruleByRuleKey.get(ruleKey)) - .orElseThrow(() -> new RowNotFoundException(String.format("Rule with key '%s' does not exist", ruleKey))); + RuleDefinition rule = detailsSupplier.getRuleDefinitionByRuleKey(RuleKey.parse(ruleKey)) + .orElseThrow(() -> new IllegalStateException(String.format("Rule with key '%s' does not exist", ruleKey))); String name = rule.getName() + " (" + rule.getLanguage() + ")"; setFieldValue(metric + DOT + i + LABEL, name); setFieldValue(metric + DOT + i + COUNT, String.valueOf(ruleStats.getValue().getOnLeak())); @@ -142,22 +184,14 @@ private void setRuleStatistics(DbSession dbSession, NewIssuesStatistics.Stats st } } - private void setComponentsStatistics(DbSession dbSession, NewIssuesStatistics.Stats stats) { + private void setComponentsStatistics(NewIssuesStatistics.Stats stats) { Metric metric = Metric.COMPONENT; int i = 1; List> fiveBiggest = fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getOnLeak); - Set componentUuids = fiveBiggest - .stream() - .map(Map.Entry::getKey) - .collect(MoreCollectors.toSet(fiveBiggest.size())); - Map componentDtosByUuid = dbClient.componentDao().selectByUuids(dbSession, componentUuids) - .stream() - .collect(MoreCollectors.uniqueIndex(ComponentDto::uuid)); for (Map.Entry componentStats : fiveBiggest) { String uuid = componentStats.getKey(); - String componentName = Optional.ofNullable(componentDtosByUuid.get(uuid)) - .map(ComponentDto::name) - .orElseThrow(() -> new RowNotFoundException(String.format("Component with uuid '%s' not found", uuid))); + String componentName = detailsSupplier.getComponentNameByUuid(uuid) + .orElseThrow(() -> new IllegalStateException(String.format("Component with uuid '%s' not found", uuid))); setFieldValue(metric + DOT + i + LABEL, componentName); setFieldValue(metric + DOT + i + COUNT, String.valueOf(componentStats.getValue().getOnLeak())); i++; @@ -174,19 +208,14 @@ private void setTagsStatistics(NewIssuesStatistics.Stats stats) { } } - private void setAssigneesStatistics(DbSession dbSession, NewIssuesStatistics.Stats stats) { - + private void setAssigneesStatistics(NewIssuesStatistics.Stats stats) { Metric metric = Metric.ASSIGNEE; List> entries = fiveBiggest(stats.getDistributedMetricStats(metric), MetricStatsInt::getOnLeak); - Set assigneeUuids = entries.stream().map(Map.Entry::getKey).filter(Objects::nonNull).collect(toSet()); - Map userDtoByUuid = dbClient.userDao().selectByUuids(dbSession, assigneeUuids).stream().collect(toMap(UserDto::getUuid, u -> u)); - int i = 1; for (Map.Entry assigneeStats : entries) { String assigneeUuid = assigneeStats.getKey(); - UserDto user = userDtoByUuid.get(assigneeUuid); - String name = user == null ? assigneeUuid : user.getName(); + String name = detailsSupplier.getUserNameByUuid(assigneeUuid).orElse(assigneeUuid); setFieldValue(metric + DOT + i + LABEL, name); setFieldValue(metric + DOT + i + COUNT, String.valueOf(assigneeStats.getValue().getOnLeak())); i++; diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationTest.java index de3eb27e3af2..bcf27a5302d6 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/MyNewIssuesNotificationTest.java @@ -21,9 +21,9 @@ import org.junit.Test; import org.sonar.api.utils.Durations; -import org.sonar.db.DbClient; import org.sonar.db.user.UserDto; import org.sonar.db.user.UserTesting; +import org.sonar.server.issue.notification.NewIssuesNotification.DetailsSupplier; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -31,7 +31,7 @@ public class MyNewIssuesNotificationTest { - private MyNewIssuesNotification underTest = new MyNewIssuesNotification(mock(DbClient.class), mock(Durations.class)); + private MyNewIssuesNotification underTest = new MyNewIssuesNotification(mock(Durations.class), mock(DetailsSupplier.class)); @Test public void set_assignee() { diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/NewIssuesNotificationTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/NewIssuesNotificationTest.java index e3d7b02dbb02..c145f574d73b 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/NewIssuesNotificationTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/notification/NewIssuesNotificationTest.java @@ -20,6 +20,7 @@ package org.sonar.server.issue.notification; import java.util.Date; +import java.util.Optional; import java.util.stream.IntStream; import org.junit.Rule; import org.junit.Test; @@ -31,12 +32,16 @@ import org.sonar.db.issue.IssueDto; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.user.UserDto; +import org.sonar.server.issue.notification.NewIssuesNotification.DetailsSupplier; +import org.sonar.server.issue.notification.NewIssuesNotification.RuleDefinition; import static java.util.Arrays.asList; import static java.util.Collections.singletonList; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.apache.commons.lang.RandomStringUtils.randomAlphanumeric; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import static org.sonar.api.rules.RuleType.BUG; import static org.sonar.api.rules.RuleType.CODE_SMELL; import static org.sonar.db.component.ComponentTesting.newDirectory; @@ -53,7 +58,8 @@ public class NewIssuesNotificationTest { @Rule public DbTester db = DbTester.create(); - private NewIssuesNotification underTest = new NewIssuesNotification(db.getDbClient(), new Durations()); + private DetailsSupplier detailsSupplier = mock(DetailsSupplier.class); + private NewIssuesNotification underTest = new NewIssuesNotification(new Durations(), detailsSupplier); @Test public void set_project_without_branch() { @@ -133,9 +139,8 @@ public void set_date() { @Test public void set_statistics() { - UserDto maynard = db.users().insertUser(u-> u.setLogin("maynard")); - UserDto keenan = db.users().insertUser(u-> u.setLogin("keenan")); - + UserDto maynard = db.users().insertUser(u -> u.setLogin("maynard")); + UserDto keenan = db.users().insertUser(u -> u.setLogin("keenan")); ComponentDto project = db.components().insertPrivateProject(); ComponentDto directory = db.components().insertComponent(newDirectory(project, "path")); ComponentDto file = db.components().insertComponent(newFileDto(directory)); @@ -146,6 +151,9 @@ public void set_statistics() { NewIssuesStatistics.Stats stats = new NewIssuesStatistics.Stats(i -> true); IntStream.rangeClosed(1, 5).forEach(i -> stats.add(issue1.toDefaultIssue())); IntStream.rangeClosed(1, 3).forEach(i -> stats.add(issue2.toDefaultIssue())); + mockDetailsSupplierComponents(project, directory, file); + mockDetailsSupplierRules(rule1, rule2); + mockDetailsSupplierAssignees(maynard, keenan); underTest.setStatistics(project.longName(), stats); @@ -170,6 +178,25 @@ public void set_statistics() { assertThat(underTest.getDefaultMessage()).startsWith("8 new issues on " + project.longName()); } + private void mockDetailsSupplierAssignees(UserDto... users) { + for (UserDto user : users) { + when(detailsSupplier.getUserNameByUuid(user.getUuid())).thenReturn(Optional.of(user.getName())); + } + } + + private void mockDetailsSupplierRules(RuleDefinitionDto... rules) { + for (RuleDefinitionDto rule : rules) { + when(detailsSupplier.getRuleDefinitionByRuleKey(rule.getKey())) + .thenReturn(Optional.of(new RuleDefinition(rule.getName(), rule.getLanguage()))); + } + } + + private void mockDetailsSupplierComponents(ComponentDto... components) { + for (ComponentDto component : components) { + when(detailsSupplier.getComponentNameByUuid(component.uuid())).thenReturn(Optional.of(component.name())); + } + } + @Test public void set_assignee() { ComponentDto project = db.components().insertPrivateProject(); @@ -179,6 +206,9 @@ public void set_assignee() { IssueDto issue = db.issues().insert(rule, project, file, i -> i.setAssigneeUuid(user.getUuid())); NewIssuesStatistics.Stats stats = new NewIssuesStatistics.Stats(i -> true); IntStream.rangeClosed(1, 5).forEach(i -> stats.add(issue.toDefaultIssue())); + mockDetailsSupplierRules(rule); + mockDetailsSupplierAssignees(user); + mockDetailsSupplierComponents(project, file); underTest.setStatistics(project.longName(), stats); @@ -196,7 +226,6 @@ public void add_only_5_assignees_with_biggest_issue_counts() { UserDto user6 = db.users().insertUser(); UserDto user7 = db.users().insertUser(); UserDto user8 = db.users().insertUser(); - ComponentDto project = db.components().insertPrivateProject(); ComponentDto file = db.components().insertComponent(newFileDto(project)); RuleDefinitionDto rule = db.rules().insert(); @@ -209,6 +238,9 @@ public void add_only_5_assignees_with_biggest_issue_counts() { IntStream.rangeClosed(1, 5).forEach(i -> stats.add(db.issues().insert(rule, project, file, issue -> issue.setAssigneeUuid(user6.getUuid())).toDefaultIssue())); IntStream.rangeClosed(1, 4).forEach(i -> stats.add(db.issues().insert(rule, project, file, issue -> issue.setAssigneeUuid(user7.getUuid())).toDefaultIssue())); IntStream.rangeClosed(1, 3).forEach(i -> stats.add(db.issues().insert(rule, project, file, issue -> issue.setAssigneeUuid(user8.getUuid())).toDefaultIssue())); + mockDetailsSupplierAssignees(user1, user2, user3, user4, user5, user6, user7, user8); + mockDetailsSupplierComponents(project, file); + mockDetailsSupplierRules(rule); underTest.setStatistics(project.longName(), stats); @@ -247,6 +279,8 @@ public void add_only_5_components_with_biggest_issue_counts() { IntStream.rangeClosed(1, 4).forEach(i -> stats.add(db.issues().insert(rule, project, file7).toDefaultIssue())); ComponentDto file8 = db.components().insertComponent(newFileDto(project)); IntStream.rangeClosed(1, 3).forEach(i -> stats.add(db.issues().insert(rule, project, file8).toDefaultIssue())); + mockDetailsSupplierComponents(project, file1, file2, file3, file4, file5, file6, file7, file8); + mockDetailsSupplierRules(rule); underTest.setStatistics(project.longName(), stats); @@ -285,6 +319,8 @@ public void add_only_5_rules_with_biggest_issue_counts() { IntStream.rangeClosed(1, 4).forEach(i -> stats.add(db.issues().insert(rule7, project, file).toDefaultIssue())); RuleDefinitionDto rule8 = db.rules().insert(r -> r.setLanguage("Java")); IntStream.rangeClosed(1, 3).forEach(i -> stats.add(db.issues().insert(rule8, project, file).toDefaultIssue())); + mockDetailsSupplierComponents(project, file); + mockDetailsSupplierRules(rule1, rule2, rule3, rule4, rule5, rule6, rule7, rule8); underTest.setStatistics(project.longName(), stats); @@ -310,4 +346,37 @@ public void set_debt() { assertThat(underTest.getFieldValue(EFFORT + ".count")).isEqualTo("55min"); } + @Test + public void RuleDefinition_implements_equals_base_on_name_and_language() { + String name = randomAlphabetic(5); + String language = randomAlphabetic(6); + RuleDefinition underTest = new RuleDefinition(name, language); + + assertThat(underTest) + .isEqualTo(underTest) + .isEqualTo(new RuleDefinition(name, language)); + assertThat(underTest).isNotEqualTo(new RuleDefinition(language, name)); + assertThat(underTest).isNotEqualTo(new RuleDefinition(randomAlphabetic(7), name)); + assertThat(underTest).isNotEqualTo(new RuleDefinition(language, randomAlphabetic(7))); + assertThat(underTest).isNotEqualTo(new RuleDefinition(language, null)); + assertThat(underTest).isNotEqualTo(null); + assertThat(underTest).isNotEqualTo(new Object()); + } + + @Test + public void RuleDefinition_implements_hashcode_base_on_name_and_language() { + String name = randomAlphabetic(5); + String language = randomAlphabetic(6); + RuleDefinition underTest = new RuleDefinition(name, language); + + assertThat(underTest.hashCode()) + .isEqualTo(underTest.hashCode()) + .isEqualTo(new RuleDefinition(name, language).hashCode()); + assertThat(underTest.hashCode()).isNotEqualTo(new RuleDefinition(language, name).hashCode()); + assertThat(underTest.hashCode()).isNotEqualTo(new RuleDefinition(randomAlphabetic(7), name).hashCode()); + assertThat(underTest.hashCode()).isNotEqualTo(new RuleDefinition(language, randomAlphabetic(7)).hashCode()); + assertThat(underTest.hashCode()).isNotEqualTo(new RuleDefinition(language, null).hashCode()); + assertThat(underTest.hashCode()).isNotEqualTo(null); + assertThat(underTest.hashCode()).isNotEqualTo(new Object().hashCode()); + } }