From: Eric Giffon Date: Tue, 23 May 2023 12:48:53 +0000 (+0200) Subject: SONAR-19340 Raise security hotspots events from CE X-Git-Tag: 10.1.0.73491~193 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=eaf6bcb414be379c352fb66fdaa5e88461c36163;p=sonarqube.git SONAR-19340 Raise security hotspots events from CE --- diff --git a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/DefaultAssigneeIT.java b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/DefaultAssigneeIT.java index 3d04097a7f6..e7b36137021 100644 --- a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/DefaultAssigneeIT.java +++ b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/DefaultAssigneeIT.java @@ -27,6 +27,7 @@ import org.sonar.ce.task.projectanalysis.component.ConfigurationRepository; import org.sonar.ce.task.projectanalysis.component.TestSettingsRepository; import org.sonar.db.DbTester; import org.sonar.db.user.UserDto; +import org.sonar.db.user.UserIdDto; import static org.assertj.core.api.Assertions.assertThat; @@ -43,7 +44,7 @@ public class DefaultAssigneeIT { @Test public void no_default_assignee() { - assertThat(underTest.loadDefaultAssigneeUuid()).isNull(); + assertThat(underTest.loadDefaultAssigneeUserId()).isNull(); } @Test @@ -51,14 +52,17 @@ public class DefaultAssigneeIT { settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "erik"); UserDto userDto = db.users().insertUser("erik"); - assertThat(underTest.loadDefaultAssigneeUuid()).isEqualTo(userDto.getUuid()); + UserIdDto userId = underTest.loadDefaultAssigneeUserId(); + assertThat(userId).isNotNull(); + assertThat(userId.getUuid()).isEqualTo(userDto.getUuid()); + assertThat(userId.getLogin()).isEqualTo(userDto.getLogin()); } @Test public void configured_login_does_not_exist() { settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "erik"); - assertThat(underTest.loadDefaultAssigneeUuid()).isNull(); + assertThat(underTest.loadDefaultAssigneeUserId()).isNull(); } @Test @@ -66,17 +70,23 @@ public class DefaultAssigneeIT { settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "erik"); db.users().insertUser(user -> user.setLogin("erik").setActive(false)); - assertThat(underTest.loadDefaultAssigneeUuid()).isNull(); + assertThat(underTest.loadDefaultAssigneeUserId()).isNull(); } @Test public void default_assignee_is_cached() { settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "erik"); UserDto userDto = db.users().insertUser("erik"); - assertThat(underTest.loadDefaultAssigneeUuid()).isEqualTo(userDto.getUuid()); + UserIdDto userId = underTest.loadDefaultAssigneeUserId(); + assertThat(userId).isNotNull(); + assertThat(userId.getUuid()).isEqualTo(userDto.getUuid()); + assertThat(userId.getLogin()).isEqualTo(userDto.getLogin()); // The setting is updated but the assignee hasn't changed settings.setProperty(CoreProperties.DEFAULT_ISSUE_ASSIGNEE, "other"); - assertThat(underTest.loadDefaultAssigneeUuid()).isEqualTo(userDto.getUuid()); + userId = underTest.loadDefaultAssigneeUserId(); + assertThat(userId).isNotNull(); + assertThat(userId.getUuid()).isEqualTo(userDto.getUuid()); + assertThat(userId.getLogin()).isEqualTo(userDto.getLogin()); } } diff --git a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUserLoaderIT.java b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUserLoaderIT.java index 9e91e0b66a9..86d15f426d2 100644 --- a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUserLoaderIT.java +++ b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUserLoaderIT.java @@ -25,6 +25,7 @@ import org.slf4j.event.Level; import org.sonar.api.testfixtures.log.LogTester; import org.sonar.db.DbTester; import org.sonar.db.user.UserDto; +import org.sonar.db.user.UserIdDto; import org.sonar.server.es.EsTester; import static java.util.Arrays.asList; @@ -49,7 +50,10 @@ public class ScmAccountToUserLoaderIT { ScmAccountToUserLoader underTest = new ScmAccountToUserLoader(db.getDbClient()); assertThat(underTest.load("missing")).isNull(); - assertThat(underTest.load("jesuis@charlie.com")).isEqualTo(user.getUuid()); + UserIdDto result = underTest.load("jesuis@charlie.com"); + assertThat(result).isNotNull(); + assertThat(result.getUuid()).isEqualTo(user.getUuid()); + assertThat(result.getLogin()).isEqualTo(user.getLogin()); } @Test diff --git a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/step/PersistPushEventsStepIT.java b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/step/PersistPushEventsStepIT.java index 8ae97ad8528..5d06b6ea3de 100644 --- a/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/step/PersistPushEventsStepIT.java +++ b/server/sonar-ce-task-projectanalysis/src/it/java/org/sonar/ce/task/projectanalysis/step/PersistPushEventsStepIT.java @@ -74,7 +74,7 @@ public class PersistPushEventsStepIT { @Test public void description() { - assertThat(underTest.getDescription()).isEqualTo("Publishing taint vulnerabilities events"); + assertThat(underTest.getDescription()).isEqualTo("Publishing taint vulnerabilities and security hotspots events"); } @Test diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/DefaultAssignee.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/DefaultAssignee.java index 65bd05b427b..2bf7cbc5cd5 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/DefaultAssignee.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/DefaultAssignee.java @@ -20,12 +20,13 @@ package org.sonar.ce.task.projectanalysis.issue; import javax.annotation.CheckForNull; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.ce.task.projectanalysis.component.ConfigurationRepository; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.user.UserDto; +import org.sonar.db.user.UserIdDto; import static com.google.common.base.Strings.isNullOrEmpty; import static org.sonar.api.CoreProperties.DEFAULT_ISSUE_ASSIGNEE; @@ -36,13 +37,13 @@ import static org.sonar.api.CoreProperties.DEFAULT_ISSUE_ASSIGNEE; */ public class DefaultAssignee { - private static final Logger LOG = Loggers.get(DefaultAssignee.class); + private static final Logger LOG = LoggerFactory.getLogger(DefaultAssignee.class); private final DbClient dbClient; private final ConfigurationRepository configRepository; private boolean loaded = false; - private String userUuid = null; + private UserIdDto userId = null; public DefaultAssignee(DbClient dbClient, ConfigurationRepository configRepository) { this.dbClient = dbClient; @@ -50,26 +51,26 @@ public class DefaultAssignee { } @CheckForNull - public String loadDefaultAssigneeUuid() { + public UserIdDto loadDefaultAssigneeUserId() { if (loaded) { - return userUuid; + return userId; } String login = configRepository.getConfiguration().get(DEFAULT_ISSUE_ASSIGNEE).orElse(null); if (!isNullOrEmpty(login)) { - userUuid = findValidUserUuidFromLogin(login); + userId = findValidUserUuidFromLogin(login); } loaded = true; - return userUuid; + return userId; } - private String findValidUserUuidFromLogin(String login) { + private UserIdDto findValidUserUuidFromLogin(String login) { try (DbSession dbSession = dbClient.openSession(false)) { UserDto user = dbClient.userDao().selectActiveUserByLogin(dbSession, login); if (user == null) { LOG.info("Property {} is set with an unknown login: {}", DEFAULT_ISSUE_ASSIGNEE, login); return null; } - return user.getUuid(); + return new UserIdDto(user.getUuid(), user.getLogin()); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueAssigner.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueAssigner.java index 3b440a0e406..7a59e1a4c24 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueAssigner.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueAssigner.java @@ -23,8 +23,8 @@ import java.util.Comparator; import java.util.Date; import java.util.Optional; import org.apache.commons.lang.StringUtils; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.scm.Changeset; @@ -33,6 +33,7 @@ import org.sonar.ce.task.projectanalysis.scm.ScmInfoRepository; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; import org.sonar.db.issue.IssueDto; +import org.sonar.db.user.UserIdDto; import org.sonar.server.issue.IssueFieldsSetter; import static org.apache.commons.lang.StringUtils.defaultIfEmpty; @@ -45,7 +46,7 @@ import static org.sonar.core.issue.IssueChangeContext.issueChangeContextByScanBu */ public class IssueAssigner extends IssueVisitor { - private static final Logger LOGGER = Loggers.get(IssueAssigner.class); + private static final Logger LOGGER = LoggerFactory.getLogger(IssueAssigner.class); private final ScmInfoRepository scmInfoRepository; private final DefaultAssignee defaultAssignee; @@ -82,9 +83,8 @@ public class IssueAssigner extends IssueVisitor { } if (issue.assignee() == null) { - String assigneeUuid = scmAuthor.map(scmAccountToUser::getNullable).orElse(null); - assigneeUuid = defaultIfEmpty(assigneeUuid, defaultAssignee.loadDefaultAssigneeUuid()); - issueUpdater.setNewAssignee(issue, assigneeUuid, changeContext); + UserIdDto userId = scmAuthor.map(scmAccountToUser::getNullable).orElse(defaultAssignee.loadDefaultAssigneeUserId()); + issueUpdater.setNewAssignee(issue, userId, changeContext); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java index d3c5317cbf9..9d4a6721dc5 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java @@ -216,6 +216,7 @@ public class IssueLifecycle { toIssue.setResolution(fromIssue.resolution()); toIssue.setStatus(fromIssue.status()); toIssue.setAssigneeUuid(fromIssue.assignee()); + toIssue.setAssigneeLogin(fromIssue.assigneeLogin()); toIssue.setAuthorLogin(fromIssue.authorLogin()); toIssue.setTags(fromIssue.tags()); toIssue.setEffort(debtCalculator.calculate(toIssue)); 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 8db892aa480..4b6d801d5ca 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 @@ -62,4 +62,6 @@ public interface Rule { String getDefaultRuleDescription(); String getSeverity(); + + Set getSecurityStandards(); } 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 d3f74921914..aa46996a315 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 @@ -51,6 +51,7 @@ public class RuleImpl implements Rule { private final boolean isAdHoc; private final String defaultRuleDescription; private final String severity; + private final Set securityStandards; public RuleImpl(RuleDto dto) { this.uuid = dto.getUuid(); @@ -66,6 +67,7 @@ public class RuleImpl implements Rule { this.isAdHoc = dto.isAdHoc(); this.defaultRuleDescription = getNonNullDefaultRuleDescription(dto); this.severity = Optional.ofNullable(dto.getSeverityString()).orElse(dto.getAdHocSeverity()); + this.securityStandards = dto.getSecurityStandards(); } private static String getNonNullDefaultRuleDescription(RuleDto dto) { @@ -130,6 +132,11 @@ public class RuleImpl implements Rule { return severity; } + @Override + public Set getSecurityStandards() { + return securityStandards; + } + @Override public boolean equals(@Nullable Object o) { if (this == o) { 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 a41f4c0a774..6f5dd4d2f54 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 @@ -221,5 +221,10 @@ public class RuleRepositoryImpl implements RuleRepository { public String getSeverity() { return addHocRule.getSeverity(); } + + @Override + public Set getSecurityStandards() { + return Collections.emptySet(); + } } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUser.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUser.java index d1b50402c85..b9319831aff 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUser.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUser.java @@ -20,12 +20,13 @@ package org.sonar.ce.task.projectanalysis.issue; import org.sonar.ce.task.projectanalysis.util.cache.MemoryCache; +import org.sonar.db.user.UserIdDto; /** - * Cache of dictionary {SCM account -> SQ user uuid}. Kept in memory + * Cache of dictionary {SCM account -> SQ user uuid and login}. Kept in memory * during the execution of Compute Engine. */ -public class ScmAccountToUser extends MemoryCache { +public class ScmAccountToUser extends MemoryCache { public ScmAccountToUser(ScmAccountToUserLoader loader) { super(loader); } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUserLoader.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUserLoader.java index fac4f079909..979f687d922 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUserLoader.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/ScmAccountToUserLoader.java @@ -23,19 +23,19 @@ import com.google.common.base.Joiner; import java.util.Collection; import java.util.List; import java.util.Map; -import org.sonar.api.utils.log.Logger; -import org.sonar.api.utils.log.Loggers; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.sonar.ce.task.projectanalysis.util.cache.CacheLoader; import org.sonar.db.DbClient; import org.sonar.db.DbSession; -import org.sonar.db.user.UserDto; +import org.sonar.db.user.UserIdDto; /** * Loads the association between a SCM account and a SQ user */ -public class ScmAccountToUserLoader implements CacheLoader { +public class ScmAccountToUserLoader implements CacheLoader { - private static final Logger LOGGER = Loggers.get(ScmAccountToUserLoader.class); + private static final Logger LOGGER = LoggerFactory.getLogger(ScmAccountToUserLoader.class); private final DbClient dbClient; @@ -44,26 +44,27 @@ public class ScmAccountToUserLoader implements CacheLoader { } @Override - public String load(String scmAccount) { + public UserIdDto load(String scmAccount) { try (DbSession dbSession = dbClient.openSession(false)) { - List userUuids = dbClient.userDao().selectUserUuidByScmAccountOrLoginOrEmail(dbSession, scmAccount); - if (userUuids.size() == 1) { - return userUuids.iterator().next(); + List users = dbClient.userDao().selectActiveUsersByScmAccountOrLoginOrEmail(dbSession, scmAccount); + if (users.size() == 1) { + return users.iterator().next(); } - if (!userUuids.isEmpty()) { - List userDtos = dbClient.userDao().selectByUuids(dbSession, userUuids); - Collection logins = userDtos.stream() - .map(UserDto::getLogin) + if (!users.isEmpty()) { + Collection logins = users.stream() + .map(UserIdDto::getLogin) .sorted() .toList(); - LOGGER.warn(String.format("Multiple users share the SCM account '%s': %s", scmAccount, Joiner.on(", ").join(logins))); + if (LOGGER.isWarnEnabled()) { + LOGGER.warn(String.format("Multiple users share the SCM account '%s': %s", scmAccount, Joiner.on(", ").join(logins))); + } } return null; } } @Override - public Map loadAll(Collection scmAccounts) { + public Map loadAll(Collection scmAccounts) { throw new UnsupportedOperationException("Loading by multiple scm accounts is not supported yet"); } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/IssueEvent.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/IssueEvent.java new file mode 100644 index 00000000000..65f4c49eabc --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/IssueEvent.java @@ -0,0 +1,62 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 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.pushevent; + +public abstract class IssueEvent { + + private String key; + private String projectKey; + + protected IssueEvent() { + // nothing to do + } + + protected IssueEvent(String key, String projectKey) { + this.key = key; + this.projectKey = projectKey; + } + + public abstract String getEventName(); + + public String getKey() { + return key; + } + + public void setKey(String key) { + this.key = key; + } + + public String getProjectKey() { + return projectKey; + } + + public void setProjectKey(String projectKey) { + this.projectKey = projectKey; + } + + @Override + public String toString() { + return "IssueEvent{" + + "name='" + getEventName() + '\'' + + ", key='" + key + '\'' + + ", projectKey='" + projectKey + '\'' + + '}'; + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java index 4e3ed70db37..8c3a5c7580e 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactory.java @@ -21,13 +21,16 @@ package org.sonar.ce.task.projectanalysis.pushevent; import com.google.gson.Gson; import com.google.gson.GsonBuilder; -import java.util.Objects; import java.util.Optional; +import java.util.Set; import org.jetbrains.annotations.NotNull; import org.sonar.api.ce.ComputeEngineSide; +import org.sonar.api.rules.RuleType; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; import org.sonar.ce.task.projectanalysis.component.Component; import org.sonar.ce.task.projectanalysis.component.TreeRootHolder; +import org.sonar.ce.task.projectanalysis.issue.Rule; +import org.sonar.ce.task.projectanalysis.issue.RuleRepository; import org.sonar.ce.task.projectanalysis.locations.flow.FlowGenerator; import org.sonar.ce.task.projectanalysis.locations.flow.Location; import org.sonar.ce.task.projectanalysis.locations.flow.TextRange; @@ -36,9 +39,11 @@ import org.sonar.db.protobuf.DbCommons; import org.sonar.db.protobuf.DbIssues; import org.sonar.db.pushevent.PushEventDto; import org.sonar.server.issue.TaintChecker; +import org.sonar.server.security.SecurityStandards; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Objects.requireNonNull; +import static org.sonar.server.security.SecurityStandards.fromSecurityStandards; @ComputeEngineSide public class PushEventFactory { @@ -48,47 +53,78 @@ public class PushEventFactory { private final AnalysisMetadataHolder analysisMetadataHolder; private final TaintChecker taintChecker; private final FlowGenerator flowGenerator; + private final RuleRepository ruleRepository; - public PushEventFactory(TreeRootHolder treeRootHolder, - AnalysisMetadataHolder analysisMetadataHolder, TaintChecker taintChecker, FlowGenerator flowGenerator) { + public PushEventFactory(TreeRootHolder treeRootHolder, AnalysisMetadataHolder analysisMetadataHolder, TaintChecker taintChecker, + FlowGenerator flowGenerator, RuleRepository ruleRepository) { this.treeRootHolder = treeRootHolder; this.analysisMetadataHolder = analysisMetadataHolder; this.taintChecker = taintChecker; this.flowGenerator = flowGenerator; + this.ruleRepository = ruleRepository; } public Optional raiseEventOnIssue(String projectUuid, DefaultIssue currentIssue) { var currentIssueComponentUuid = currentIssue.componentUuid(); - if (!taintChecker.isTaintVulnerability(currentIssue) || currentIssueComponentUuid == null) { + if (currentIssueComponentUuid == null) { return Optional.empty(); } - var component = treeRootHolder.getComponentByUuid(Objects.requireNonNull(currentIssue.componentUuid())); - if (currentIssue.isNew() || currentIssue.isCopied() || isReopened(currentIssue)) { - return Optional.of(raiseTaintVulnerabilityRaisedEvent(projectUuid, component, currentIssue)); + var component = treeRootHolder.getComponentByUuid(currentIssueComponentUuid); + + if (isTaintVulnerability(currentIssue)) { + return raiseTaintVulnerabilityEvent(projectUuid, component, currentIssue); + } + if (isSecurityHotspot(currentIssue)) { + return raiseSecurityHotspotEvent(projectUuid, component, currentIssue); + } + return Optional.empty(); + } + + private boolean isTaintVulnerability(DefaultIssue issue) { + return taintChecker.isTaintVulnerability(issue); + } + + private static boolean isSecurityHotspot(DefaultIssue issue) { + return RuleType.SECURITY_HOTSPOT.equals(issue.type()); + } + + private Optional raiseTaintVulnerabilityEvent(String projectUuid, Component component, DefaultIssue issue) { + if (shouldCreateRaisedEvent(issue)) { + return Optional.of(raiseTaintVulnerabilityRaisedEvent(projectUuid, component, issue)); + } + if (issue.isBeingClosed()) { + return Optional.of(raiseTaintVulnerabilityClosedEvent(projectUuid, issue)); } - if (currentIssue.isBeingClosed()) { - return Optional.of(raiseTaintVulnerabilityClosedEvent(projectUuid, currentIssue)); + return Optional.empty(); + } + + private Optional raiseSecurityHotspotEvent(String projectUuid, Component component, DefaultIssue issue) { + if (shouldCreateRaisedEvent(issue)) { + return Optional.of(raiseSecurityHotspotRaisedEvent(projectUuid, component, issue)); + } + if (issue.isBeingClosed()) { + return Optional.of(raiseSecurityHotspotClosedEvent(projectUuid, component, issue)); } return Optional.empty(); } + private static boolean shouldCreateRaisedEvent(DefaultIssue issue) { + return issue.isNew() || issue.isCopied() || isReopened(issue); + } + private static boolean isReopened(DefaultIssue currentIssue) { var currentChange = currentIssue.currentChange(); if (currentChange == null) { return false; } var status = currentChange.get("status"); - return status != null && status.toString().equals("CLOSED|OPEN"); + return status != null && Set.of("CLOSED|OPEN", "CLOSED|TO_REVIEW").contains(status.toString()); } private PushEventDto raiseTaintVulnerabilityRaisedEvent(String projectUuid, Component component, DefaultIssue issue) { TaintVulnerabilityRaised event = prepareEvent(component, issue); - return new PushEventDto() - .setName("TaintVulnerabilityRaised") - .setProjectUuid(projectUuid) - .setLanguage(issue.language()) - .setPayload(serializeEvent(event)); + return createPushEventDto(projectUuid, issue, event); } private TaintVulnerabilityRaised prepareEvent(Component component, DefaultIssue issue) { @@ -117,16 +153,52 @@ public class PushEventFactory { return mainLocation; } - private static PushEventDto raiseTaintVulnerabilityClosedEvent(String projectUuid, DefaultIssue issue) { - TaintVulnerabilityClosed event = new TaintVulnerabilityClosed(issue.key(), issue.projectKey()); + private static PushEventDto createPushEventDto(String projectUuid, DefaultIssue issue, IssueEvent event) { return new PushEventDto() - .setName("TaintVulnerabilityClosed") + .setName(event.getEventName()) .setProjectUuid(projectUuid) .setLanguage(issue.language()) .setPayload(serializeEvent(event)); } - private static byte[] serializeEvent(Object event) { + private static PushEventDto raiseTaintVulnerabilityClosedEvent(String projectUuid, DefaultIssue issue) { + TaintVulnerabilityClosed event = new TaintVulnerabilityClosed(issue.key(), issue.projectKey()); + return createPushEventDto(projectUuid, issue, event); + } + + private PushEventDto raiseSecurityHotspotRaisedEvent(String projectUuid, Component component, DefaultIssue issue) { + SecurityHotspotRaised event = new SecurityHotspotRaised(); + event.setKey(issue.key()); + event.setProjectKey(issue.projectKey()); + event.setStatus(issue.getStatus()); + event.setCreationDate(issue.creationDate().getTime()); + event.setMainLocation(prepareMainLocation(component, issue)); + event.setRuleKey(issue.getRuleKey().toString()); + event.setVulnerabilityProbability(getVulnerabilityProbability(issue)); + event.setBranch(analysisMetadataHolder.getBranch().getName()); + event.setAssignee(issue.assigneeLogin()); + + return createPushEventDto(projectUuid, issue, event); + } + + private String getVulnerabilityProbability(DefaultIssue issue) { + Rule rule = ruleRepository.getByKey(issue.getRuleKey()); + SecurityStandards.SQCategory sqCategory = fromSecurityStandards(rule.getSecurityStandards()).getSqCategory(); + return sqCategory.getVulnerability().name(); + } + + private static PushEventDto raiseSecurityHotspotClosedEvent(String projectUuid, Component component, DefaultIssue issue) { + SecurityHotspotClosed event = new SecurityHotspotClosed(); + event.setKey(issue.key()); + event.setProjectKey(issue.projectKey()); + event.setStatus(issue.getStatus()); + event.setResolution(issue.resolution()); + event.setFilePath(component.getName()); + + return createPushEventDto(projectUuid, issue, event); + } + + private static byte[] serializeEvent(IssueEvent event) { return GSON.toJson(event).getBytes(UTF_8); } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/SecurityHotspotClosed.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/SecurityHotspotClosed.java new file mode 100644 index 00000000000..3109c1a061c --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/SecurityHotspotClosed.java @@ -0,0 +1,66 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 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.pushevent; + +import com.google.common.annotations.VisibleForTesting; +import javax.annotation.Nullable; + +public class SecurityHotspotClosed extends IssueEvent { + + @VisibleForTesting + static final String EVENT_NAME = "SecurityHotspotClosed"; + + private String status; + private String resolution; + private String filePath; + + public SecurityHotspotClosed() { + // nothing to do + } + + @Override + public String getEventName() { + return EVENT_NAME; + } + + public String getStatus() { + return status; + } + + public void setStatus(String status) { + this.status = status; + } + + public String getResolution() { + return resolution; + } + + public void setResolution(@Nullable String resolution) { + this.resolution = resolution; + } + + public String getFilePath() { + return filePath; + } + + public void setFilePath(String filePath) { + this.filePath = filePath; + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/SecurityHotspotRaised.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/SecurityHotspotRaised.java new file mode 100644 index 00000000000..fa08fa13f76 --- /dev/null +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/SecurityHotspotRaised.java @@ -0,0 +1,103 @@ +/* + * SonarQube + * Copyright (C) 2009-2023 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.pushevent; + +import com.google.common.annotations.VisibleForTesting; +import javax.annotation.Nullable; +import org.sonar.ce.task.projectanalysis.locations.flow.Location; + +public class SecurityHotspotRaised extends IssueEvent { + + @VisibleForTesting + static final String EVENT_NAME = "SecurityHotspotRaised"; + + private String status; + private String vulnerabilityProbability; + private long creationDate; + private Location mainLocation; + private String ruleKey; + private String branch; + private String assignee; + + public SecurityHotspotRaised() { + // nothing to do + } + + @Override + public String getEventName() { + return EVENT_NAME; + } + + public String getStatus() { + return status; + } + + public void setStatus(String status) { + this.status = status; + } + + public String getVulnerabilityProbability() { + return vulnerabilityProbability; + } + + public void setVulnerabilityProbability(String vulnerabilityProbability) { + this.vulnerabilityProbability = vulnerabilityProbability; + } + + public long getCreationDate() { + return creationDate; + } + + public void setCreationDate(long creationDate) { + this.creationDate = creationDate; + } + + public Location getMainLocation() { + return mainLocation; + } + + public void setMainLocation(Location mainLocation) { + this.mainLocation = mainLocation; + } + + public String getRuleKey() { + return ruleKey; + } + + public void setRuleKey(String ruleKey) { + this.ruleKey = ruleKey; + } + + public String getBranch() { + return branch; + } + + public void setBranch(String branch) { + this.branch = branch; + } + + public String getAssignee() { + return assignee; + } + + public void setAssignee(@Nullable String assignee) { + this.assignee = assignee; + } +} diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityClosed.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityClosed.java index 6d4857c1ad3..34b77df014e 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityClosed.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityClosed.java @@ -19,34 +19,20 @@ */ package org.sonar.ce.task.projectanalysis.pushevent; -public class TaintVulnerabilityClosed { +public class TaintVulnerabilityClosed extends IssueEvent { - private String key; - private String projectKey; + private static final String EVENT_NAME = "TaintVulnerabilityClosed"; public TaintVulnerabilityClosed() { // nothing to do } public TaintVulnerabilityClosed(String key, String projectKey) { - this.key = key; - this.projectKey = projectKey; + super(key, projectKey); } - public String getKey() { - return key; + @Override + public String getEventName() { + return EVENT_NAME; } - - public void setKey(String key) { - this.key = key; - } - - public String getProjectKey() { - return projectKey; - } - - public void setProjectKey(String projectKey) { - this.projectKey = projectKey; - } - } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityRaised.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityRaised.java index c988a3e0279..003718bdfc4 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityRaised.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/pushevent/TaintVulnerabilityRaised.java @@ -24,10 +24,10 @@ import java.util.Optional; import org.sonar.ce.task.projectanalysis.locations.flow.Flow; import org.sonar.ce.task.projectanalysis.locations.flow.Location; -public class TaintVulnerabilityRaised { +public class TaintVulnerabilityRaised extends IssueEvent { + + private static final String EVENT_NAME = "TaintVulnerabilityRaised"; - private String key; - private String projectKey; private String branch; private long creationDate; private String ruleKey; @@ -41,20 +41,9 @@ public class TaintVulnerabilityRaised { // nothing to do } - public String getKey() { - return key; - } - - public void setKey(String key) { - this.key = key; - } - - public String getProjectKey() { - return projectKey; - } - - public void setProjectKey(String projectKey) { - this.projectKey = projectKey; + @Override + public String getEventName() { + return EVENT_NAME; } public String getBranch() { diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistPushEventsStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistPushEventsStep.java index 38874cb47e0..6b016fbe001 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistPushEventsStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/PersistPushEventsStep.java @@ -100,7 +100,7 @@ public class PersistPushEventsStep implements ComputationStep { @Override public String getDescription() { - return "Publishing taint vulnerabilities events"; + return "Publishing taint vulnerabilities and security hotspots events"; } } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java index a9334b82c20..f0f251f95c3 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java @@ -112,6 +112,7 @@ public class ProtobufIssueDiskCache implements DiskCache { defaultIssue.setStatus(next.getStatus()); defaultIssue.setResolution(next.hasResolution() ? next.getResolution() : null); defaultIssue.setAssigneeUuid(next.hasAssigneeUuid() ? next.getAssigneeUuid() : null); + defaultIssue.setAssigneeLogin(next.hasAssigneeLogin() ? next.getAssigneeLogin() : null); defaultIssue.setChecksum(next.hasChecksum() ? next.getChecksum() : null); defaultIssue.setAuthorLogin(next.hasAuthorLogin() ? next.getAuthorLogin() : null); next.getCommentsList().forEach(c -> defaultIssue.addComment(toDefaultIssueComment(c))); @@ -164,6 +165,7 @@ public class ProtobufIssueDiskCache implements DiskCache { builder.setStatus(defaultIssue.status()); ofNullable(defaultIssue.resolution()).ifPresent(builder::setResolution); ofNullable(defaultIssue.assignee()).ifPresent(builder::setAssigneeUuid); + ofNullable(defaultIssue.assigneeLogin()).ifPresent(builder::setAssigneeLogin); ofNullable(defaultIssue.checksum()).ifPresent(builder::setChecksum); ofNullable(defaultIssue.authorLogin()).ifPresent(builder::setAuthorLogin); defaultIssue.defaultIssueComments().forEach(c -> builder.addComments(toProtoComment(c))); diff --git a/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto b/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto index 8f6456d11ba..880424a347b 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto +++ b/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto @@ -82,6 +82,7 @@ message Issue { optional string ruleDescriptionContextKey = 44; optional sonarqube.db.issues.MessageFormattings messageFormattings = 45; optional string codeVariants = 46; + optional string assigneeLogin = 47; } message Comment { 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 f6b48bf8b65..532ad64422a 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 @@ -42,6 +42,7 @@ public class DumbRule implements Rule { private String pluginKey; private boolean isExternal; private boolean isAdHoc; + private Set securityStandards = new HashSet<>(); public DumbRule(RuleKey key) { this.key = key; @@ -105,6 +106,11 @@ public class DumbRule implements Rule { return null; } + @Override + public Set getSecurityStandards() { + return securityStandards; + } + @Override public boolean isExternal() { return isExternal; diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueAssignerTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueAssignerTest.java index 286f9ce04b6..620d09d4848 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueAssignerTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueAssignerTest.java @@ -33,6 +33,7 @@ import org.sonar.ce.task.projectanalysis.scm.ScmInfoRepositoryRule; import org.sonar.core.issue.DefaultIssue; import org.sonar.db.protobuf.DbCommons; import org.sonar.db.protobuf.DbIssues; +import org.sonar.db.user.UserIdDto; import org.sonar.server.issue.IssueFieldsSetter; import static java.util.stream.Collectors.joining; @@ -56,9 +57,9 @@ public class IssueAssignerTest { @Rule public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule().setAnalysisDate(123456789L); - private ScmAccountToUser scmAccountToUser = mock(ScmAccountToUser.class); - private DefaultAssignee defaultAssignee = mock(DefaultAssignee.class); - private IssueAssigner underTest = new IssueAssigner(analysisMetadataHolder, scmInfoRepository, scmAccountToUser, defaultAssignee, new IssueFieldsSetter()); + private final ScmAccountToUser scmAccountToUser = mock(ScmAccountToUser.class); + private final DefaultAssignee defaultAssignee = mock(DefaultAssignee.class); + private final IssueAssigner underTest = new IssueAssigner(analysisMetadataHolder, scmInfoRepository, scmAccountToUser, defaultAssignee, new IssueFieldsSetter()); @Before public void before() { @@ -98,38 +99,41 @@ public class IssueAssignerTest { @Test public void assign_but_do_not_set_author_if_too_long() { String scmAuthor = range(0, 256).mapToObj(i -> "s").collect(joining()); - addScmUser(scmAuthor, "John C"); + addScmUser(scmAuthor, buildUserId("u123", "John C")); setSingleChangeset(scmAuthor, 123456789L, "rev-1"); DefaultIssue issue = newIssueOnLines(1); underTest.onIssue(FILE, issue); assertThat(issue.authorLogin()).isNull(); - assertThat(issue.assignee()).isEqualTo("John C"); + assertThat(issue.assignee()).isEqualTo("u123"); + assertThat(issue.assigneeLogin()).isEqualTo("John C"); assertThat(logTester.logs(Level.DEBUG)).contains("SCM account '" + scmAuthor + "' is too long to be stored as issue author"); } @Test public void assign_new_issue_to_author_of_change() { - addScmUser("john", "u123"); + addScmUser("john", buildUserId("u123", "john")); setSingleChangeset("john", 123456789L, "rev-1"); DefaultIssue issue = newIssueOnLines(1); underTest.onIssue(FILE, issue); assertThat(issue.assignee()).isEqualTo("u123"); + assertThat(issue.assigneeLogin()).isEqualTo("john"); } @Test public void assign_new_issue_to_default_assignee_if_author_not_found() { setSingleChangeset("john", 123456789L, "rev-1"); - when(defaultAssignee.loadDefaultAssigneeUuid()).thenReturn("u1234"); + when(defaultAssignee.loadDefaultAssigneeUserId()).thenReturn(new UserIdDto("u1234", "john")); DefaultIssue issue = newIssueOnLines(1); underTest.onIssue(FILE, issue); assertThat(issue.assignee()).isEqualTo("u1234"); + assertThat(issue.assigneeLogin()).isEqualTo("john"); } @Test @@ -145,7 +149,7 @@ public class IssueAssignerTest { @Test public void do_not_assign_issue_if_unassigned_but_already_authored() { - addScmUser("john", "u1234"); + addScmUser("john", buildUserId("u1234", "john")); setSingleChangeset("john", 123456789L, "rev-1"); DefaultIssue issue = newIssueOnLines(1) .setAuthorLogin("john") @@ -159,7 +163,7 @@ public class IssueAssignerTest { @Test public void assign_to_last_committer_of_file_if_issue_is_global_to_file() { - addScmUser("henry", "Henry V"); + addScmUser("henry", buildUserId("u123", "Henry V")); Changeset changeset1 = Changeset.newChangesetBuilder() .setAuthor("john") .setDate(1_000L) @@ -177,22 +181,24 @@ public class IssueAssignerTest { underTest.onIssue(FILE, issue); - assertThat(issue.assignee()).isEqualTo("Henry V"); + assertThat(issue.assignee()).isEqualTo("u123"); + assertThat(issue.assigneeLogin()).isEqualTo("Henry V"); } @Test public void assign_to_default_assignee_if_no_author() { DefaultIssue issue = newIssueOnLines(); - when(defaultAssignee.loadDefaultAssigneeUuid()).thenReturn("u123"); + when(defaultAssignee.loadDefaultAssigneeUserId()).thenReturn(new UserIdDto("u123", "john")); underTest.onIssue(FILE, issue); assertThat(issue.assignee()).isEqualTo("u123"); + assertThat(issue.assigneeLogin()).isEqualTo("john"); } @Test public void assign_to_default_assignee_if_no_scm_on_issue_locations() { - addScmUser("john", "John C"); + addScmUser("john", buildUserId("u123", "John C")); Changeset changeset = Changeset.newChangesetBuilder() .setAuthor("john") .setDate(123456789L) @@ -203,13 +209,14 @@ public class IssueAssignerTest { underTest.onIssue(FILE, issue); - assertThat(issue.assignee()).isEqualTo("John C"); + assertThat(issue.assignee()).isEqualTo("u123"); + assertThat(issue.assigneeLogin()).isEqualTo("John C"); } @Test public void assign_to_author_of_the_most_recent_change_in_all_issue_locations() { - addScmUser("john", "u1"); - addScmUser("jane", "u2"); + addScmUser("john", buildUserId("u1", "John")); + addScmUser("jane", buildUserId("u2", "Jane")); Changeset commit1 = Changeset.newChangesetBuilder() .setAuthor("john") .setDate(1_000L) @@ -227,6 +234,7 @@ public class IssueAssignerTest { assertThat(issue.authorLogin()).isEqualTo("jane"); assertThat(issue.assignee()).isEqualTo("u2"); + assertThat(issue.assigneeLogin()).isEqualTo("Jane"); } private void setSingleChangeset(@Nullable String author, Long date, String revision) { @@ -238,8 +246,8 @@ public class IssueAssignerTest { .build()); } - private void addScmUser(String scmAccount, String userUuid) { - when(scmAccountToUser.getNullable(scmAccount)).thenReturn(userUuid); + private void addScmUser(String scmAccount, UserIdDto userId) { + when(scmAccountToUser.getNullable(scmAccount)).thenReturn(userId); } private static DefaultIssue newIssueOnLines(int... lines) { @@ -259,4 +267,7 @@ public class IssueAssignerTest { .setTextRange(DbCommons.TextRange.newBuilder().setStartLine(line).setEndLine(line).build()).build(); } + private UserIdDto buildUserId(String uuid, String login) { + return new UserIdDto(uuid, login); + } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java index 924ccc81f6d..78a749c6d7c 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java @@ -297,6 +297,7 @@ public class IssueLifecycleTest { .setStatus(STATUS_CLOSED) .setSeverity(BLOCKER) .setAssigneeUuid("base assignee uuid") + .setAssigneeLogin("base assignee login") .setAuthorLogin("base author") .setTags(newArrayList("base tag")) .setOnDisabledRule(true) @@ -326,6 +327,7 @@ public class IssueLifecycleTest { assertThat(raw.resolution()).isEqualTo(RESOLUTION_FIXED); assertThat(raw.status()).isEqualTo(STATUS_CLOSED); assertThat(raw.assignee()).isEqualTo("base assignee uuid"); + assertThat(raw.assigneeLogin()).isEqualTo("base assignee login"); assertThat(raw.authorLogin()).isEqualTo("base author"); assertThat(raw.tags()).containsOnly("base tag"); assertThat(raw.effort()).isEqualTo(DEFAULT_DURATION); @@ -383,6 +385,7 @@ public class IssueLifecycleTest { .setStatus(STATUS_RESOLVED) .setSeverity(BLOCKER) .setAssigneeUuid("base assignee uuid") + .setAssigneeLogin("base assignee login") .setAuthorLogin("base author") .setTags(newArrayList("base tag")) .setOnDisabledRule(true) @@ -410,6 +413,7 @@ public class IssueLifecycleTest { assertThat(raw.resolution()).isEqualTo(RESOLUTION_FALSE_POSITIVE); assertThat(raw.status()).isEqualTo(STATUS_RESOLVED); assertThat(raw.assignee()).isEqualTo("base assignee uuid"); + assertThat(raw.assigneeLogin()).isEqualTo("base assignee login"); assertThat(raw.authorLogin()).isEqualTo("base author"); assertThat(raw.tags()).containsOnly("base tag"); assertThat(raw.codeVariants()).containsOnly("foo", "bar"); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactoryTest.java index 248bf1815aa..028a0c11843 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactoryTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/pushevent/PushEventFactoryTest.java @@ -22,10 +22,11 @@ package org.sonar.ce.task.projectanalysis.pushevent; import com.google.gson.Gson; import java.nio.charset.StandardCharsets; import java.util.Date; -import java.util.List; +import java.util.Set; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.sonar.api.issue.Issue; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.DateUtils; @@ -34,11 +35,13 @@ import org.sonar.ce.task.projectanalysis.analysis.TestBranch; import org.sonar.ce.task.projectanalysis.component.Component.Type; import org.sonar.ce.task.projectanalysis.component.MutableTreeRootHolderRule; import org.sonar.ce.task.projectanalysis.component.ReportComponent; +import org.sonar.ce.task.projectanalysis.issue.RuleRepository; import org.sonar.ce.task.projectanalysis.locations.flow.FlowGenerator; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.FieldDiffs; import org.sonar.db.protobuf.DbCommons; import org.sonar.db.protobuf.DbIssues; +import org.sonar.db.rule.RuleDto; import org.sonar.server.issue.TaintChecker; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; @@ -59,24 +62,25 @@ public class PushEventFactoryTest { @Rule public AnalysisMetadataHolderRule analysisMetadataHolder = new AnalysisMetadataHolderRule() .setBranch(new TestBranch(BRANCH_NAME)); - private final FlowGenerator flowGenerator = new FlowGenerator(treeRootHolder); - private final PushEventFactory underTest = new PushEventFactory(treeRootHolder, analysisMetadataHolder, taintChecker, flowGenerator); + private final RuleRepository ruleRepository = mock(RuleRepository.class); + private final PushEventFactory underTest = new PushEventFactory(treeRootHolder, analysisMetadataHolder, taintChecker, flowGenerator, + ruleRepository); @Before public void setUp() { - when(taintChecker.getTaintRepositories()).thenReturn(List.of("roslyn.sonaranalyzer.security.cs", - "javasecurity", "jssecurity", "tssecurity", "phpsecurity", "pythonsecurity")); - when(taintChecker.isTaintVulnerability(any())).thenReturn(true); + when(ruleRepository.getByKey(RuleKey.of("javasecurity", "S123"))).thenReturn(buildRule()); buildComponentTree(); } @Test - public void raise_event_to_repository_if_taint_vulnerability_is_new() { + public void raiseEventOnIssue_whenNewTaintVulnerability_shouldCreateRaisedEvent() { DefaultIssue defaultIssue = createDefaultIssue() .setNew(true) .setRuleDescriptionContextKey(randomAlphabetic(6)); + when(taintChecker.isTaintVulnerability(any())).thenReturn(true); + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)) .isNotEmpty() .hasValueSatisfying(pushEventDto -> { @@ -91,7 +95,8 @@ public class PushEventFactoryTest { private static void verifyPayload(byte[] payload, DefaultIssue defaultIssue) { assertThat(payload).isNotNull(); - TaintVulnerabilityRaised taintVulnerabilityRaised = gson.fromJson(new String(payload, StandardCharsets.UTF_8), TaintVulnerabilityRaised.class); + TaintVulnerabilityRaised taintVulnerabilityRaised = gson.fromJson(new String(payload, StandardCharsets.UTF_8), + TaintVulnerabilityRaised.class); assertThat(taintVulnerabilityRaised.getProjectKey()).isEqualTo(defaultIssue.projectKey()); assertThat(taintVulnerabilityRaised.getCreationDate()).isEqualTo(defaultIssue.creationDate().getTime()); assertThat(taintVulnerabilityRaised.getKey()).isEqualTo(defaultIssue.key()); @@ -99,18 +104,21 @@ public class PushEventFactoryTest { assertThat(taintVulnerabilityRaised.getRuleKey()).isEqualTo(defaultIssue.ruleKey().toString()); assertThat(taintVulnerabilityRaised.getType()).isEqualTo(defaultIssue.type().name()); assertThat(taintVulnerabilityRaised.getBranch()).isEqualTo(BRANCH_NAME); - String ruleDescriptionContextKey = taintVulnerabilityRaised.getRuleDescriptionContextKey().orElseGet(() -> fail("No rule description context key")); + String ruleDescriptionContextKey = taintVulnerabilityRaised.getRuleDescriptionContextKey().orElseGet(() -> fail("No rule description " + + "context key")); assertThat(ruleDescriptionContextKey).isEqualTo(defaultIssue.getRuleDescriptionContextKey().orElse(null)); } @Test - public void raise_event_to_repository_if_taint_vulnerability_is_reopened() { + public void raiseEventOnIssue_whenReopenedTaintVulnerability_shouldCreateRaisedEvent() { DefaultIssue defaultIssue = createDefaultIssue() .setChanged(true) .setNew(false) .setCopied(false) .setCurrentChange(new FieldDiffs().setDiff("status", "CLOSED", "OPEN")); + when(taintChecker.isTaintVulnerability(any())).thenReturn(true); + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)) .isNotEmpty() .hasValueSatisfying(pushEventDto -> { @@ -120,21 +128,25 @@ public class PushEventFactoryTest { } @Test - public void skip_event_if_taint_vulnerability_status_change() { + public void raiseEventOnIssue_whenTaintVulnerabilityStatusChange_shouldSkipEvent() { DefaultIssue defaultIssue = createDefaultIssue() .setChanged(true) .setNew(false) .setCopied(false) .setCurrentChange(new FieldDiffs().setDiff("status", "OPEN", "FIXED")); + when(taintChecker.isTaintVulnerability(any())).thenReturn(true); + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)).isEmpty(); } @Test - public void raise_event_to_repository_if_taint_vulnerability_is_copied() { + public void raiseEventOnIssue_whenCopiedTaintVulnerability_shouldCreateRaisedEvent() { DefaultIssue defaultIssue = createDefaultIssue() .setCopied(true); + when(taintChecker.isTaintVulnerability(any())).thenReturn(true); + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)) .isNotEmpty() .hasValueSatisfying(pushEventDto -> { @@ -144,13 +156,14 @@ public class PushEventFactoryTest { } @Test - public void raise_event_to_repository_if_taint_vulnerability_is_closed() { + public void raiseEventOnIssue_whenClosedTaintVulnerability_shouldCreateClosedEvent() { DefaultIssue defaultIssue = createDefaultIssue() - .setComponentUuid("") .setNew(false) .setCopied(false) .setBeingClosed(true); + when(taintChecker.isTaintVulnerability(any())).thenReturn(true); + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)) .isNotEmpty() .hasValueSatisfying(pushEventDto -> { @@ -160,7 +173,7 @@ public class PushEventFactoryTest { } @Test - public void skip_issue_if_issue_changed() { + public void raiseEventOnIssue_whenChangedTaintVulnerability_shouldSkipEvent() { DefaultIssue defaultIssue = new DefaultIssue() .setComponentUuid("issue-component-uuid") .setNew(false) @@ -170,11 +183,13 @@ public class PushEventFactoryTest { .setCreationDate(DateUtils.parseDate("2022-01-01")) .setRuleKey(RuleKey.of("javasecurity", "S123")); + when(taintChecker.isTaintVulnerability(any())).thenReturn(true); + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)).isEmpty(); } @Test - public void skip_if_issue_not_from_taint_vulnerability_repository() { + public void raiseEventOnIssue_whenIssueNotFromTaintVulnerabilityRepository_shouldSkipEvent() { DefaultIssue defaultIssue = new DefaultIssue() .setComponentUuid("issue-component-uuid") .setChanged(true) @@ -197,11 +212,11 @@ public class PushEventFactoryTest { } @Test - public void skip_if_issue_is_a_hotspot() { + public void raiseEventOnIssue_whenIssueDoesNotHaveLocations_shouldSkipEvent() { DefaultIssue defaultIssue = new DefaultIssue() .setComponentUuid("issue-component-uuid") .setChanged(true) - .setType(RuleType.SECURITY_HOTSPOT) + .setType(RuleType.VULNERABILITY) .setRuleKey(RuleKey.of("javasecurity", "S123")); when(taintChecker.isTaintVulnerability(any())).thenReturn(false); @@ -210,14 +225,115 @@ public class PushEventFactoryTest { } @Test - public void skip_if_issue_does_not_have_locations() { - DefaultIssue defaultIssue = new DefaultIssue() - .setComponentUuid("issue-component-uuid") + public void raiseEventOnIssue_whenNewHotspot_shouldCreateRaisedEvent() { + DefaultIssue defaultIssue = createDefaultIssue() + .setType(RuleType.SECURITY_HOTSPOT) + .setStatus(Issue.STATUS_TO_REVIEW) + .setNew(true) + .setRuleDescriptionContextKey(randomAlphabetic(6)); + + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)) + .isNotEmpty() + .hasValueSatisfying(pushEventDto -> { + assertThat(pushEventDto.getName()).isEqualTo(SecurityHotspotRaised.EVENT_NAME); + verifyHotspotRaisedEventPayload(pushEventDto.getPayload(), defaultIssue); + assertThat(pushEventDto.getLanguage()).isEqualTo("java"); + assertThat(pushEventDto.getProjectUuid()).isEqualTo("some-project-uuid"); + }); + } + + private static void verifyHotspotRaisedEventPayload(byte[] payload, DefaultIssue defaultIssue) { + assertThat(payload).isNotNull(); + + SecurityHotspotRaised event = gson.fromJson(new String(payload, StandardCharsets.UTF_8), SecurityHotspotRaised.class); + assertThat(event.getProjectKey()).isEqualTo(defaultIssue.projectKey()); + assertThat(event.getCreationDate()).isEqualTo(defaultIssue.creationDate().getTime()); + assertThat(event.getKey()).isEqualTo(defaultIssue.key()); + assertThat(event.getRuleKey()).isEqualTo(defaultIssue.ruleKey().toString()); + assertThat(event.getStatus()).isEqualTo(Issue.STATUS_TO_REVIEW); + assertThat(event.getVulnerabilityProbability()).isEqualTo("LOW"); + assertThat(event.getMainLocation()).isNotNull(); + assertThat(event.getBranch()).isEqualTo(BRANCH_NAME); + assertThat(event.getAssignee()).isEqualTo("some-user-login"); + } + + @Test + public void raiseEventOnIssue_whenReopenedHotspot_shouldCreateRaisedEvent() { + DefaultIssue defaultIssue = createDefaultIssue() + .setType(RuleType.SECURITY_HOTSPOT) .setChanged(true) - .setType(RuleType.VULNERABILITY) - .setRuleKey(RuleKey.of("javasecurity", "S123")); + .setNew(false) + .setCopied(false) + .setCurrentChange(new FieldDiffs().setDiff("status", "CLOSED", "TO_REVIEW")); - when(taintChecker.isTaintVulnerability(any())).thenReturn(false); + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)) + .isNotEmpty() + .hasValueSatisfying(pushEventDto -> { + assertThat(pushEventDto.getName()).isEqualTo(SecurityHotspotRaised.EVENT_NAME); + assertThat(pushEventDto.getPayload()).isNotNull(); + }); + } + + @Test + public void raiseEventOnIssue_whenCopiedHotspot_shouldCreateRaisedEvent() { + DefaultIssue defaultIssue = createDefaultIssue() + .setType(RuleType.SECURITY_HOTSPOT) + .setCopied(true); + + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)) + .isNotEmpty() + .hasValueSatisfying(pushEventDto -> { + assertThat(pushEventDto.getName()).isEqualTo(SecurityHotspotRaised.EVENT_NAME); + assertThat(pushEventDto.getPayload()).isNotNull(); + }); + } + + @Test + public void raiseEventOnIssue_whenClosedHotspot_shouldCreateClosedEvent() { + DefaultIssue defaultIssue = createDefaultIssue() + .setType(RuleType.SECURITY_HOTSPOT) + .setNew(false) + .setCopied(false) + .setBeingClosed(true) + .setStatus(Issue.STATUS_CLOSED) + .setResolution(Issue.RESOLUTION_FIXED); + + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)) + .isNotEmpty() + .hasValueSatisfying(pushEventDto -> { + assertThat(pushEventDto.getName()).isEqualTo(SecurityHotspotClosed.EVENT_NAME); + verifyHotspotClosedEventPayload(pushEventDto.getPayload(), defaultIssue); + assertThat(pushEventDto.getLanguage()).isEqualTo("java"); + assertThat(pushEventDto.getProjectUuid()).isEqualTo("some-project-uuid"); + }); + } + + private static void verifyHotspotClosedEventPayload(byte[] payload, DefaultIssue defaultIssue) { + assertThat(payload).isNotNull(); + + SecurityHotspotClosed event = gson.fromJson(new String(payload, StandardCharsets.UTF_8), SecurityHotspotClosed.class); + assertThat(event.getProjectKey()).isEqualTo(defaultIssue.projectKey()); + assertThat(event.getKey()).isEqualTo(defaultIssue.key()); + assertThat(event.getStatus()).isEqualTo(Issue.STATUS_CLOSED); + assertThat(event.getResolution()).isEqualTo(Issue.RESOLUTION_FIXED); + assertThat(event.getFilePath()).isEqualTo("component-name"); + } + + @Test + public void raiseEventOnIssue_whenChangedHotspot_shouldSkipEvent() { + DefaultIssue defaultIssue = createDefaultIssue() + .setType(RuleType.SECURITY_HOTSPOT) + .setChanged(true) + .setNew(false) + .setCopied(false); + + assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)).isEmpty(); + } + + @Test + public void raiseEventOnIssue_whenComponentUuidNull_shouldSkipEvent() { + DefaultIssue defaultIssue = createDefaultIssue() + .setComponentUuid(null); assertThat(underTest.raiseEventOnIssue("some-project-uuid", defaultIssue)).isEmpty(); } @@ -226,6 +342,7 @@ public class PushEventFactoryTest { treeRootHolder.setRoot(ReportComponent.builder(Type.PROJECT, 1) .setUuid("uuid_1") .addChildren(ReportComponent.builder(Type.FILE, 2) + .setName("component-name") .setUuid("issue-component-uuid") .build()) .addChildren(ReportComponent.builder(Type.FILE, 3) @@ -236,7 +353,11 @@ public class PushEventFactoryTest { private DefaultIssue createDefaultIssue() { return new DefaultIssue() + .setKey("issue-key") + .setProjectKey("project-key") .setComponentUuid("issue-component-uuid") + .setAssigneeUuid("some-user-uuid") + .setAssigneeLogin("some-user-login") .setType(RuleType.VULNERABILITY) .setLanguage("java") .setCreationDate(new Date()) @@ -254,4 +375,10 @@ public class PushEventFactoryTest { .setRuleKey(RuleKey.of("javasecurity", "S123")); } + private org.sonar.ce.task.projectanalysis.issue.Rule buildRule() { + RuleDto ruleDto = new RuleDto(); + ruleDto.setRuleKey(RuleKey.of("javasecurity", "S123")); + ruleDto.setSecurityStandards(Set.of("owasp-a1")); + return new org.sonar.ce.task.projectanalysis.issue.RuleImpl(ruleDto); + } } diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java index 1befd30a3c2..ab8afc81fde 100644 --- a/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java +++ b/server/sonar-db-dao/src/it/java/org/sonar/db/user/UserDaoIT.java @@ -31,6 +31,7 @@ import java.util.Objects; import java.util.Set; import java.util.function.Function; import java.util.stream.IntStream; +import org.assertj.core.groups.Tuple; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -91,7 +92,7 @@ public class UserDaoIT { } @Test - public void selectUserUuidByScmAccountOrLoginOrEmail_findsCorrectResults() { + public void selectActiveUsersByScmAccountOrLoginOrEmail_findsCorrectResults() { String user1 = db.users().insertUser(user -> user.setLogin("user1").setEmail("toto@tata.com")).getUuid(); String user2 = db.users().insertUser(user -> user.setLogin("user2")).getUuid(); String user3 = db.users().insertUser(user -> user.setLogin("user3").setScmAccounts(List.of("scmuser3", "scmuser3bis"))).getUuid(); @@ -99,11 +100,14 @@ public class UserDaoIT { db.users().insertUser(user -> user.setLogin("inactive_user1").setActive(false)); db.users().insertUser(user -> user.setLogin("inactive_user2").setActive(false).setScmAccounts(List.of("inactive_user2"))); - assertThat(underTest.selectUserUuidByScmAccountOrLoginOrEmail(session, "toto@tata.com")).containsExactly(user1); - assertThat(underTest.selectUserUuidByScmAccountOrLoginOrEmail(session, "user2")).containsExactly(user2); - assertThat(underTest.selectUserUuidByScmAccountOrLoginOrEmail(session, "scmuser3")).containsExactly(user3); - assertThat(underTest.selectUserUuidByScmAccountOrLoginOrEmail(session, "inactive_user1")).isEmpty(); - assertThat(underTest.selectUserUuidByScmAccountOrLoginOrEmail(session, "inactive_user2")).isEmpty(); + assertThat(underTest.selectActiveUsersByScmAccountOrLoginOrEmail(session, "toto@tata.com")) + .extracting(UserIdDto::getUuid, UserIdDto::getLogin).containsExactly(new Tuple(user1, "user1")); + assertThat(underTest.selectActiveUsersByScmAccountOrLoginOrEmail(session, "user2")) + .extracting(UserIdDto::getUuid, UserIdDto::getLogin).containsExactly(new Tuple(user2, "user2")); + assertThat(underTest.selectActiveUsersByScmAccountOrLoginOrEmail(session, "scmuser3")) + .extracting(UserIdDto::getUuid, UserIdDto::getLogin).containsExactly(new Tuple(user3, "user3")); + assertThat(underTest.selectActiveUsersByScmAccountOrLoginOrEmail(session, "inactive_user1")).isEmpty(); + assertThat(underTest.selectActiveUsersByScmAccountOrLoginOrEmail(session, "inactive_user2")).isEmpty(); } @Test diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java index 81104ebd175..41307b39273 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java @@ -769,6 +769,7 @@ public final class IssueDto implements Serializable { issue.setChecksum(checksum); issue.setSeverity(severity); issue.setAssigneeUuid(assigneeUuid); + issue.setAssigneeLogin(assigneeLogin); issue.setComponentKey(componentKey); issue.setComponentUuid(componentUuid); issue.setProjectUuid(projectUuid); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java index 84ab1500417..6466c28bcf9 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDao.java @@ -190,8 +190,8 @@ public class UserDao implements Dao { /** * This method is optimized for the first analysis: we tried to keep performance optimal (<10ms) for projects with large number of contributors */ - public List selectUserUuidByScmAccountOrLoginOrEmail(DbSession session, String scmAccountOrLoginOrEmail) { - return mapper(session).selectUserUuidByScmAccountOrLoginOrEmail(scmAccountOrLoginOrEmail); + public List selectActiveUsersByScmAccountOrLoginOrEmail(DbSession session, String scmAccountOrLoginOrEmail) { + return mapper(session).selectActiveUsersByScmAccountOrLoginOrEmail(scmAccountOrLoginOrEmail); } /** diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java index 6ff3eab31c2..c2a525bc360 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserDto.java @@ -76,7 +76,7 @@ public class UserDto implements UserId { return uuid; } - UserDto setUuid(String uuid) { + public UserDto setUuid(String uuid) { this.uuid = uuid; return this; } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserMapper.java b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserMapper.java index 496bba6727c..b2c76e95915 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserMapper.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/user/UserMapper.java @@ -40,7 +40,7 @@ public interface UserMapper { @CheckForNull List selectNullableByScmAccountOrLoginOrEmail(@Param("scmAccount") String scmAccountOrLoginOrEmail); - List selectUserUuidByScmAccountOrLoginOrEmail(@Param("scmAccount") String scmAccountOrLoginOrEmail); + List selectActiveUsersByScmAccountOrLoginOrEmail(@Param("scmAccount") String scmAccountOrLoginOrEmail); /** * Select user by login. Note that disabled users are ignored. diff --git a/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml b/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml index 4b9ff1521de..fe46b569190 100644 --- a/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml +++ b/server/sonar-db-dao/src/main/resources/org/sonar/db/user/UserMapper.xml @@ -380,13 +380,14 @@ user_uuid = #{userUuid,jdbcType=VARCHAR} - + select u.uuid, u.login + from scm_accounts sa left join users u on sa.user_uuid = u.uuid where u.active=${_true} and sa.scm_account = lower(#{scmAccount,jdbcType=VARCHAR}) union - select uuid from - users u + select uuid, login + from users u where active=${_true} and (login=#{scmAccount,jdbcType=VARCHAR} or email=#{scmAccount,jdbcType=VARCHAR} ) diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java index 32b915be9a0..b6d8b7443d9 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java @@ -38,6 +38,7 @@ import org.sonar.core.issue.DefaultIssueComment; import org.sonar.core.issue.IssueChangeContext; import org.sonar.db.protobuf.DbIssues; import org.sonar.db.user.UserDto; +import org.sonar.db.user.UserIdDto; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.isNullOrEmpty; @@ -128,13 +129,14 @@ public class IssueFieldsSetter { /** * Used to set the assignee when it was null */ - public boolean setNewAssignee(DefaultIssue issue, @Nullable String newAssigneeUuid, IssueChangeContext context) { - if (newAssigneeUuid == null) { + public boolean setNewAssignee(DefaultIssue issue, @Nullable UserIdDto userId, IssueChangeContext context) { + if (userId == null) { return false; } checkState(issue.assignee() == null, "It's not possible to update the assignee with this method, please use assign()"); - issue.setFieldChange(context, ASSIGNEE, UNUSED, newAssigneeUuid); - issue.setAssigneeUuid(newAssigneeUuid); + issue.setFieldChange(context, ASSIGNEE, UNUSED, userId.getUuid()); + issue.setAssigneeUuid(userId.getUuid()); + issue.setAssigneeLogin(userId.getLogin()); issue.setUpdateDate(context.date()); issue.setChanged(true); issue.setSendNotifications(true); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java index 29fb552eff3..362c7844d67 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java @@ -37,6 +37,7 @@ import org.sonar.db.protobuf.DbCommons; import org.sonar.db.protobuf.DbIssues; import org.sonar.db.protobuf.DbIssues.MessageFormattingType; import org.sonar.db.user.UserDto; +import org.sonar.db.user.UserIdDto; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -111,9 +112,10 @@ public class IssueFieldsSetterTest { @Test public void set_new_assignee() { - boolean updated = underTest.setNewAssignee(issue, "user_uuid", context); + boolean updated = underTest.setNewAssignee(issue, new UserIdDto("user_uuid", "user_login"), context); assertThat(updated).isTrue(); assertThat(issue.assignee()).isEqualTo("user_uuid"); + assertThat(issue.assigneeLogin()).isEqualTo("user_login"); assertThat(issue.mustSendNotifications()).isTrue(); FieldDiffs.Diff diff = issue.currentChange().get(ASSIGNEE); assertThat(diff.oldValue()).isEqualTo(UNUSED); @@ -132,7 +134,8 @@ public class IssueFieldsSetterTest { public void fail_with_ISE_when_setting_new_assignee_on_already_assigned_issue() { issue.setAssigneeUuid("user_uuid"); - assertThatThrownBy(() -> underTest.setNewAssignee(issue, "another_user_uuid", context)) + UserIdDto userId = new UserIdDto("another_user_uuid", "another_user_login"); + assertThatThrownBy(() -> underTest.setNewAssignee(issue, userId, context)) .isInstanceOf(IllegalStateException.class) .hasMessage("It's not possible to update the assignee with this method, please use assign()"); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java index b66bfe94f31..ffe5f42d7b9 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java @@ -73,6 +73,7 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure. private String status = null; private String resolution = null; private String assigneeUuid = null; + private String assigneeLogin = null; private String checksum = null; private String authorLogin = null; private List comments = null; @@ -348,6 +349,16 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure. return this; } + @CheckForNull + public String assigneeLogin() { + return assigneeLogin; + } + + public DefaultIssue setAssigneeLogin(@Nullable String s) { + this.assigneeLogin = s; + return this; + } + @Override public Date creationDate() { return creationDate; diff --git a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java index e51e8b13e3d..92ae1d4cd7f 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java @@ -19,101 +19,20 @@ */ package org.sonar.core.issue; -import java.text.SimpleDateFormat; import java.util.List; -import java.util.Set; import org.apache.commons.lang.StringUtils; import org.junit.Test; -import org.sonar.api.issue.Issue; -import org.sonar.api.rule.RuleKey; -import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.fail; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; public class DefaultIssueTest { - private static final String TEST_CONTEXT_KEY = "test_context_key"; - private DefaultIssue issue = new DefaultIssue(); - - @Test - public void test_setters_and_getters() throws Exception { - issue.setKey("ABCD") - .setComponentKey("org.sample.Sample") - .setProjectKey("Sample") - .setRuleKey(RuleKey.of("java", "S100")) - .setLanguage("xoo") - .setSeverity("MINOR") - .setManualSeverity(true) - .setMessage("a message") - .setLine(7) - .setGap(1.2d) - .setEffort(Duration.create(28800L)) - .setStatus(Issue.STATUS_CLOSED) - .setResolution(Issue.RESOLUTION_FIXED) - .setAssigneeUuid("julien") - .setAuthorLogin("steph") - .setChecksum("c7b5db46591806455cf082bb348631e8") - .setLocations("loc") - .setLocationsChanged(true) - .setNew(true) - .setIsOnChangedLine(true) - .setIsNewCodeReferenceIssue(true) - .setIsNoLongerNewCodeReferenceIssue(true) - .setBeingClosed(true) - .setOnDisabledRule(true) - .setCopied(true) - .setChanged(true) - .setSendNotifications(true) - .setCreationDate(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-19")) - .setUpdateDate(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-20")) - .setCloseDate(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-21")) - .setSelectedAt(1400000000000L) - .setRuleDescriptionContextKey(TEST_CONTEXT_KEY) - .setType(RuleType.BUG) - .setTags(Set.of("tag1", "tag2")) - .setCodeVariants(Set.of("variant1", "variant2")); - - assertThat((Object) issue.getLocations()).isEqualTo("loc"); - assertThat(issue.locationsChanged()).isTrue(); - assertThat(issue.key()).isEqualTo("ABCD"); - assertThat(issue.componentKey()).isEqualTo("org.sample.Sample"); - assertThat(issue.projectKey()).isEqualTo("Sample"); - assertThat(issue.ruleKey()).isEqualTo(RuleKey.of("java", "S100")); - assertThat(issue.language()).isEqualTo("xoo"); - assertThat(issue.severity()).isEqualTo("MINOR"); - assertThat(issue.manualSeverity()).isTrue(); - assertThat(issue.message()).isEqualTo("a message"); - assertThat(issue.line()).isEqualTo(7); - assertThat(issue.gap()).isEqualTo(1.2d); - assertThat(issue.effort()).isEqualTo(Duration.create(28800L)); - assertThat(issue.status()).isEqualTo(Issue.STATUS_CLOSED); - assertThat(issue.resolution()).isEqualTo(Issue.RESOLUTION_FIXED); - assertThat(issue.assignee()).isEqualTo("julien"); - assertThat(issue.authorLogin()).isEqualTo("steph"); - assertThat(issue.checksum()).isEqualTo("c7b5db46591806455cf082bb348631e8"); - assertThat(issue.isNew()).isTrue(); - assertThat(issue.isOnChangedLine()).isTrue(); - assertThat(issue.isNewCodeReferenceIssue()).isTrue(); - assertThat(issue.isNoLongerNewCodeReferenceIssue()).isTrue(); - assertThat(issue.isToBeMigratedAsNewCodeReferenceIssue()).isFalse(); - assertThat(issue.isCopied()).isTrue(); - assertThat(issue.isBeingClosed()).isTrue(); - assertThat(issue.isOnDisabledRule()).isTrue(); - assertThat(issue.isChanged()).isTrue(); - assertThat(issue.mustSendNotifications()).isTrue(); - assertThat(issue.creationDate()).isEqualTo(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-19")); - assertThat(issue.updateDate()).isEqualTo(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-20")); - assertThat(issue.closeDate()).isEqualTo(new SimpleDateFormat("yyyy-MM-dd").parse("2013-08-21")); - assertThat(issue.selectedAt()).isEqualTo(1400000000000L); - assertThat(issue.getRuleDescriptionContextKey()).contains(TEST_CONTEXT_KEY); - assertThat(issue.type()).isEqualTo(RuleType.BUG); - assertThat(issue.tags()).containsOnly("tag1", "tag2"); - assertThat(issue.codeVariants()).containsOnly("variant1", "variant2"); - } + private final DefaultIssue issue = new DefaultIssue(); @Test public void set_empty_dates() { @@ -187,9 +106,9 @@ public class DefaultIssueTest { List comments = issue.defaultIssueComments(); assertThat(comments).isEmpty(); - + DefaultIssueComment defaultIssueComment = new DefaultIssueComment(); try { - comments.add(new DefaultIssueComment()); + comments.add(defaultIssueComment); fail(); } catch (UnsupportedOperationException e) { // ok @@ -312,4 +231,55 @@ public class DefaultIssueTest { DefaultIssue defaultIssue = new DefaultIssue(); assertThat(defaultIssue.characteristic()).isNull(); } + + @Test + public void setLine_whenLineIsNegative_shouldThrowException() { + int anyNegativeValue = Integer.MIN_VALUE; + assertThatThrownBy(() -> issue.setLine(anyNegativeValue)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage(String.format("Line must be null or greater than zero (got %s)", anyNegativeValue)); + } + + @Test + public void setLine_whenLineIsZero_shouldThrowException() { + assertThatThrownBy(() -> issue.setLine(0)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Line must be null or greater than zero (got 0)"); + } + + @Test + public void setGap_whenGapIsNegative_shouldThrowException() { + Double anyNegativeValue = -1.0; + assertThatThrownBy(() -> issue.setGap(anyNegativeValue)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage(String.format("Gap must be greater than or equal 0 (got %s)", anyNegativeValue)); + } + + @Test + public void setGap_whenGapIsZero_shouldWork() { + issue.setGap(0.0); + assertThat(issue.gap()).isEqualTo(0.0); + } + + @Test + public void effortInMinutes_shouldConvertEffortToMinutes() { + issue.setEffort(Duration.create(60)); + assertThat(issue.effortInMinutes()).isEqualTo(60L); + } + + @Test + public void effortInMinutes_whenNull_shouldReturnNull() { + issue.setEffort(null); + assertThat(issue.effortInMinutes()).isNull(); + } + + @Test + public void tags_whenNull_shouldReturnEmptySet() { + assertThat(issue.tags()).isEmpty(); + } + + @Test + public void codeVariants_whenNull_shouldReturnEmptySet() { + assertThat(issue.codeVariants()).isEmpty(); + } }