diff options
3 files changed, 207 insertions, 11 deletions
diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java index 083cf8edc54..610556f0483 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/ShowAction.java @@ -21,7 +21,10 @@ package org.sonar.server.hotspot.ws; import com.google.common.collect.ImmutableSet; import java.util.Objects; +import java.util.Optional; import java.util.Set; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.RuleType; import org.sonar.api.server.ws.Request; @@ -34,12 +37,15 @@ import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueDto; import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.db.user.UserDto; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.issue.IssueChangelog; import org.sonar.server.issue.IssueChangelog.ChangelogLoadingContext; import org.sonar.server.issue.TextRangeResponseFormatter; +import org.sonar.server.issue.ws.UserResponseFormatter; import org.sonar.server.security.SecurityStandards; import org.sonar.server.user.UserSession; +import org.sonarqube.ws.Common; import org.sonarqube.ws.Hotspots; import org.sonarqube.ws.Hotspots.ShowWsResponse; @@ -58,14 +64,17 @@ public class ShowAction implements HotspotsWsAction { private final UserSession userSession; private final HotspotWsResponseFormatter responseFormatter; private final TextRangeResponseFormatter textRangeFormatter; + private final UserResponseFormatter userFormatter; private final IssueChangelog issueChangelog; public ShowAction(DbClient dbClient, UserSession userSession, HotspotWsResponseFormatter responseFormatter, - TextRangeResponseFormatter textRangeFormatter, IssueChangelog issueChangelog) { + TextRangeResponseFormatter textRangeFormatter, UserResponseFormatter userFormatter, + IssueChangelog issueChangelog) { this.dbClient = dbClient; this.userSession = userSession; this.responseFormatter = responseFormatter; this.textRangeFormatter = textRangeFormatter; + this.userFormatter = userFormatter; this.issueChangelog = issueChangelog; } @@ -95,10 +104,12 @@ public class ShowAction implements HotspotsWsAction { .orElseThrow(() -> new NotFoundException(format("Hotspot '%s' does not exist", hotspotKey))); Components components = loadComponents(dbSession, hotspot); + Users users = loadUsers(dbSession, hotspot); RuleDefinitionDto rule = loadRule(dbSession, hotspot); ShowWsResponse.Builder responseBuilder = ShowWsResponse.newBuilder(); formatHotspot(responseBuilder, hotspot); + formatUsers(responseBuilder, users, hotspot); formatComponents(components, responseBuilder); formatRule(responseBuilder, rule); formatTextRange(hotspot, responseBuilder); @@ -108,21 +119,41 @@ public class ShowAction implements HotspotsWsAction { } } + private Users loadUsers(DbSession dbSession, IssueDto hotspot) { + UserDto assignee = ofNullable(hotspot.getAssigneeUuid()) + .map(uuid -> dbClient.userDao().selectByUuid(dbSession, uuid)) + .orElse(null); + UserDto author = ofNullable(hotspot.getAuthorLogin()) + .map(login -> { + if (assignee != null && assignee.getLogin().equals(login)) { + return assignee; + } + return dbClient.userDao().selectByLogin(dbSession, login); + }) + .orElse(null); + return new Users(assignee, author); + } + private void formatHotspot(ShowWsResponse.Builder builder, IssueDto hotspot) { builder.setKey(hotspot.getKey()); ofNullable(hotspot.getStatus()).ifPresent(builder::setStatus); ofNullable(hotspot.getResolution()).ifPresent(builder::setResolution); ofNullable(hotspot.getLine()).ifPresent(builder::setLine); builder.setMessage(nullToEmpty(hotspot.getMessage())); - ofNullable(hotspot.getAssigneeUuid()).ifPresent(builder::setAssignee); - // FIXME Filter author only if user is member of the organization (as done in issues/search WS) - // if (data.getUserOrganizationUuids().contains(component.getOrganizationUuid())) { - builder.setAuthor(nullToEmpty(hotspot.getAuthorLogin())); - // } builder.setCreationDate(formatDateTime(hotspot.getIssueCreationDate())); builder.setUpdateDate(formatDateTime(hotspot.getIssueUpdateDate())); } + private void formatUsers(ShowWsResponse.Builder responseBuilder, Users users, IssueDto hotspot) { + Common.User.Builder userBuilder = Common.User.newBuilder(); + users.getAssignee().map(t -> userFormatter.formatUser(userBuilder, t)).ifPresent(responseBuilder::setAssignee); + + Common.User author = users.getAuthor() + .map(t -> userFormatter.formatUser(userBuilder, t)) + .orElseGet(() -> userBuilder.clear().setLogin(nullToEmpty(hotspot.getAuthorLogin())).build()); + responseBuilder.setAuthor(author); + } + private void formatComponents(Components components, ShowWsResponse.Builder responseBuilder) { responseBuilder .setProject(responseFormatter.formatComponent(Hotspots.Component.newBuilder(), components.getProject())) @@ -199,4 +230,23 @@ public class ShowAction implements HotspotsWsAction { } } + private static final class Users { + @CheckForNull + private final UserDto assignee; + @CheckForNull + private final UserDto author; + + private Users(@Nullable UserDto assignee, @Nullable UserDto author) { + this.assignee = assignee; + this.author = author; + } + + public Optional<UserDto> getAssignee() { + return ofNullable(assignee); + } + + public Optional<UserDto> getAuthor() { + return ofNullable(author); + } + } } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java index d9b1d85b0f8..062b6e19c32 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java @@ -52,12 +52,16 @@ import org.sonar.db.protobuf.DbCommons; import org.sonar.db.protobuf.DbIssues; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleTesting; +import org.sonar.db.user.UserDto; import org.sonar.server.es.EsTester; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.issue.AvatarResolver; +import org.sonar.server.issue.AvatarResolverImpl; import org.sonar.server.issue.IssueChangelog; import org.sonar.server.issue.IssueChangelog.ChangelogLoadingContext; import org.sonar.server.issue.TextRangeResponseFormatter; +import org.sonar.server.issue.ws.UserResponseFormatter; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.security.SecurityStandards; import org.sonar.server.security.SecurityStandards.SQCategory; @@ -65,6 +69,7 @@ import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.WsActionTester; import org.sonarqube.ws.Common; +import org.sonarqube.ws.Common.User; import org.sonarqube.ws.Hotspots; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; @@ -93,11 +98,13 @@ public class ShowActionTest { private DbClient dbClient = dbTester.getDbClient(); private TestDefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(dbTester); - private TextRangeResponseFormatter textRangeFormatter = new TextRangeResponseFormatter(); + private AvatarResolver avatarResolver = new AvatarResolverImpl(); private HotspotWsResponseFormatter responseFormatter = new HotspotWsResponseFormatter(defaultOrganizationProvider); private IssueChangelog issueChangelog = Mockito.mock(IssueChangelog.class); + private UserResponseFormatter userFormatter = new UserResponseFormatter(new AvatarResolverImpl()); + private TextRangeResponseFormatter textRangeFormatter = new TextRangeResponseFormatter(); - private ShowAction underTest = new ShowAction(dbClient, userSessionRule, responseFormatter, textRangeFormatter, issueChangelog); + private ShowAction underTest = new ShowAction(dbClient, userSessionRule, responseFormatter, textRangeFormatter, userFormatter, issueChangelog); private WsActionTester actionTester = new WsActionTester(underTest); @Test @@ -281,6 +288,145 @@ public class ShowActionTest { assertThat(textRange.getEndOffset()).isEqualTo(endOffset); } + @Test + public void returns_no_assignee_when_user_does_not_exist() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setAssigneeUuid(randomAlphabetic(10))); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.hasAssignee()).isFalse(); + } + + @Test + public void returns_assignee_details_when_user_exists() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + UserDto assignee = dbTester.users().insertUser(); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setAssigneeUuid(assignee.getUuid())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + User wsAssignee = response.getAssignee(); + assertThat(wsAssignee.getLogin()).isEqualTo(assignee.getLogin()); + assertThat(wsAssignee.getName()).isEqualTo(assignee.getName()); + assertThat(wsAssignee.getActive()).isEqualTo(assignee.isActive()); + assertThat(wsAssignee.getAvatar()).isEqualTo(avatarResolver.create(assignee)); + } + + @Test + public void returns_no_avatar_if_assignee_has_no_email() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + UserDto assignee = dbTester.users().insertUser(t -> t.setEmail(null)); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setAssigneeUuid(assignee.getUuid())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.getAssignee().hasAvatar()).isFalse(); + } + + @Test + public void returns_inactive_when_author_is_inactive() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + UserDto assignee = dbTester.users().insertUser(t -> t.setActive(false)); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setAssigneeUuid(assignee.getUuid())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.getAssignee().getActive()).isFalse(); + } + + @Test + public void returns_author_login_when_user_does_not_exist() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + String authorLogin = randomAlphabetic(10); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setAuthorLogin(authorLogin)); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + User wsAuthor = response.getAuthor(); + assertThat(wsAuthor.getLogin()).isEqualTo(authorLogin); + assertThat(wsAuthor.hasName()).isFalse(); + assertThat(wsAuthor.hasActive()).isFalse(); + assertThat(wsAuthor.hasAvatar()).isFalse(); + } + + @Test + public void returns_author_details_when_user_exists() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + UserDto author = dbTester.users().insertUser(); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setAuthorLogin(author.getLogin())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + User wsAuthor = response.getAuthor(); + assertThat(wsAuthor.getLogin()).isEqualTo(author.getLogin()); + assertThat(wsAuthor.getName()).isEqualTo(author.getName()); + assertThat(wsAuthor.getActive()).isEqualTo(author.isActive()); + assertThat(wsAuthor.getAvatar()).isEqualTo(avatarResolver.create(author)); + } + + @Test + public void returns_no_avatar_if_author_has_no_email() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + UserDto author = dbTester.users().insertUser(t -> t.setEmail(null)); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setAuthorLogin(author.getLogin())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.getAuthor().hasAvatar()).isFalse(); + } + + @Test + public void returns_inactive_if_author_is_inactive() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + UserDto author = dbTester.users().insertUser(t -> t.setActive(false)); + IssueDto hotspot = dbTester.issues().insertIssue(newHotspot(project, file, rule) + .setAuthorLogin(author.getLogin())); + + Hotspots.ShowWsResponse response = newRequest(hotspot) + .executeProtobuf(Hotspots.ShowWsResponse.class); + + assertThat(response.getAuthor().getActive()).isFalse(); + } + @DataProvider public static Object[][] randomTextRangeValues() { int startLine = RANDOM.nextInt(200); @@ -329,6 +475,7 @@ public class ShowActionTest { Hotspots.ShowWsResponse response = newRequest(hotspot) .executeProtobuf(Hotspots.ShowWsResponse.class); + Hotspots.Rule wsRule = response.getRule(); assertThat(wsRule.getSecurityCategory()).isEqualTo(expected.getKey()); assertThat(wsRule.getVulnerabilityProbability()).isEqualTo(expected.getVulnerability().name()); diff --git a/sonar-ws/src/main/protobuf/ws-hotspots.proto b/sonar-ws/src/main/protobuf/ws-hotspots.proto index a4ce5baed97..d67bc02ef6d 100644 --- a/sonar-ws/src/main/protobuf/ws-hotspots.proto +++ b/sonar-ws/src/main/protobuf/ws-hotspots.proto @@ -60,9 +60,8 @@ message ShowWsResponse { optional string resolution = 6; optional int32 line = 7; optional string message = 8; - optional string assignee = 9; - // SCM login of the committer who introduced the issue - optional string author = 10; + optional sonarqube.ws.commons.User assignee = 9; + optional sonarqube.ws.commons.User author = 10; optional string creationDate = 11; optional string updateDate = 12; optional sonarqube.ws.commons.TextRange textRange = 13; |