SONAR-12718 use User for author and assignee in api/hotspots/show

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 083cf8edc54fd2684b5a82886bf722bcc0681465..610556f0483598dd1e1a91d688aecde502b41b36 100644 (file)
--- 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 d9b1d85b0f8da55a67de1ea8855c158384f810f9..062b6e19c32c36cad5d9c3f8b264d7146d0625ff 100644 (file)
--- 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 a4ce5baed97ec33a149ed47cb3462cc18c2a65e8..d67bc02ef6d58e20cc5967412bf3bedf2a1c4f79 100644 (file)
--- 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;