SONAR-8899 Replace User by UserDto in Function

server/sonar-server/src/main/java/org/sonar/server/issue/AssignAction.java

@@ -19,62 +19,58 @@
  */
 package org.sonar.server.issue;
 
-import com.google.common.base.Strings;
 import java.util.Collection;
 import java.util.Map;
 import org.sonar.api.issue.condition.IsUnResolved;
 import org.sonar.api.server.ServerSide;
-import org.sonar.api.user.User;
-import org.sonar.api.user.UserFinder;
 import org.sonar.core.issue.DefaultIssue;
+import org.sonar.db.DbClient;
+import org.sonar.db.DbSession;
+import org.sonar.db.user.UserDto;
 import org.sonar.server.user.UserSession;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Strings.isNullOrEmpty;
+import static org.sonar.server.ws.WsUtils.checkFound;
+
 @ServerSide
 public class AssignAction extends Action {
 
   public static final String ASSIGN_KEY = "assign";
   public static final String ASSIGNEE_PARAMETER = "assignee";
   public static final String VERIFIED_ASSIGNEE = "verifiedAssignee";
 
-  private final UserFinder userFinder;
+  private final DbClient dbClient;
   private final IssueFieldsSetter issueUpdater;
 
-  public AssignAction(UserFinder userFinder, IssueFieldsSetter issueUpdater) {
+  public AssignAction(DbClient dbClient, IssueFieldsSetter issueUpdater) {
     super(ASSIGN_KEY);
-    this.userFinder = userFinder;
+    this.dbClient = dbClient;
     this.issueUpdater = issueUpdater;
     super.setConditions(new IsUnResolved());
   }
 
   @Override
   public boolean verify(Map<String, Object> properties, Collection<DefaultIssue> issues, UserSession userSession) {
-    String assignee = assigneeValue(properties);
-    if (!Strings.isNullOrEmpty(assignee)) {
-      User user = selectUser(assignee);
-      if (user == null) {
-        throw new IllegalArgumentException("Unknown user: " + assignee);
-      }
-      properties.put(VERIFIED_ASSIGNEE, user);
-    } else {
-      properties.put(VERIFIED_ASSIGNEE, null);
-    }
+    String assignee = getAssigneeValue(properties);
+    properties.put(VERIFIED_ASSIGNEE, isNullOrEmpty(assignee) ? null : getUser(assignee));
     return true;
   }
 
   @Override
   public boolean execute(Map<String, Object> properties, Context context) {
-    if (!properties.containsKey(VERIFIED_ASSIGNEE)) {
-      throw new IllegalArgumentException("Assignee is missing from the execution parameters");
-    }
-    User assignee = (User) properties.get(VERIFIED_ASSIGNEE);
+    checkArgument(properties.containsKey(VERIFIED_ASSIGNEE), "Assignee is missing from the execution parameters");
+    UserDto assignee = (UserDto) properties.get(VERIFIED_ASSIGNEE);
     return issueUpdater.assign(context.issue(), assignee, context.issueChangeContext());
   }
 
-  private String assigneeValue(Map<String, Object> properties) {
+  private static String getAssigneeValue(Map<String, Object> properties) {
     return (String) properties.get(ASSIGNEE_PARAMETER);
   }
 
-  private User selectUser(String assigneeKey) {
-    return userFinder.findByLogin(assigneeKey);
+  private UserDto getUser(String assigneeKey) {
+    try (DbSession dbSession = dbClient.openSession(false)) {
+      return checkFound(dbClient.userDao().selectByLogin(dbSession, assigneeKey), "Unknown user: %s", assigneeKey);
+    }
   }
 }

server/sonar-server/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java

@@ -33,12 +33,12 @@
 import org.sonar.api.rules.RuleType;
 import org.sonar.api.server.ServerSide;
 import org.sonar.api.server.rule.RuleTagFormat;
-import org.sonar.api.user.User;
 import org.sonar.api.utils.Duration;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.DefaultIssueComment;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.core.util.stream.Collectors;
+import org.sonar.db.user.UserDto;
 
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Strings.isNullOrEmpty;
@@ -109,13 +109,13 @@ public boolean setManualSeverity(DefaultIssue issue, String severity, IssueChang
     return false;
   }
 
-  public boolean assign(DefaultIssue issue, @Nullable User user, IssueChangeContext context) {
+  public boolean assign(DefaultIssue issue, @Nullable UserDto user, IssueChangeContext context) {
     String sanitizedAssignee = null;
     if (user != null) {
-      sanitizedAssignee = StringUtils.defaultIfBlank(user.login(), null);
+      sanitizedAssignee = StringUtils.defaultIfBlank(user.getLogin(), null);
     }
     if (!Objects.equals(sanitizedAssignee, issue.assignee())) {
-      String newAssigneeName = user != null ? user.name() : null;
+      String newAssigneeName = user != null ? user.getName() : null;
       issue.setFieldChange(context, ASSIGNEE, UNUSED, newAssigneeName);
       issue.setAssignee(sanitizedAssignee);
       issue.setUpdateDate(context.date());

server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java

@@ -27,12 +27,11 @@
 import javax.annotation.Nullable;
 import org.sonar.api.ce.ComputeEngineSide;
 import org.sonar.api.server.ServerSide;
-import org.sonar.api.user.User;
-import org.sonar.api.user.UserFinder;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
+import org.sonar.db.user.UserDto;
 import org.sonar.server.issue.index.IssueIndex;
 import org.sonar.server.user.UserSession;
 
@@ -48,17 +47,14 @@ public class IssueService {
   private final IssueFinder issueFinder;
   private final IssueFieldsSetter issueFieldsSetter;
   private final IssueUpdater issueUpdater;
-  private final UserFinder userFinder;
   private final UserSession userSession;
 
-  public IssueService(DbClient dbClient, IssueIndex issueIndex, IssueFinder issueFinder, IssueFieldsSetter issueFieldsSetter, IssueUpdater issueUpdater,
-    UserFinder userFinder, UserSession userSession) {
+  public IssueService(DbClient dbClient, IssueIndex issueIndex, IssueFinder issueFinder, IssueFieldsSetter issueFieldsSetter, IssueUpdater issueUpdater, UserSession userSession) {
     this.dbClient = dbClient;
     this.issueIndex = issueIndex;
     this.issueFinder = issueFinder;
     this.issueFieldsSetter = issueFieldsSetter;
     this.issueUpdater = issueUpdater;
-    this.userFinder = userFinder;
     this.userSession = userSession;
   }
 
@@ -68,10 +64,10 @@ public void assign(String issueKey, @Nullable String assignee) {
     DbSession session = dbClient.openSession(false);
     try {
       DefaultIssue issue = issueFinder.getByKey(session, issueKey).toDefaultIssue();
-      User user = null;
+      UserDto user = null;
       if (!Strings.isNullOrEmpty(assignee)) {
-        user = userFinder.findByLogin(assignee);
-        checkRequest(user != null, "Unknown user: %s", assignee);
+        user = dbClient.userDao().selectByLogin(session, assignee);
+        checkRequest(user != null && user.isActive(), "Unknown user: %s", assignee);
       }
       IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.getLogin());
       if (issueFieldsSetter.assign(issue, user, context)) {

server/sonar-server/src/main/java/org/sonar/server/issue/workflow/Function.java

@@ -21,13 +21,13 @@
 
 import javax.annotation.Nullable;
 import org.sonar.api.issue.Issue;
-import org.sonar.api.user.User;
+import org.sonar.db.user.UserDto;
 
 interface Function {
   interface Context {
     Issue issue();
 
-    Context setAssignee(@Nullable User user);
+    Context setAssignee(@Nullable UserDto user);
 
     Context setResolution(@Nullable String s);
 

server/sonar-server/src/main/java/org/sonar/server/issue/workflow/FunctionExecutor.java

@@ -23,9 +23,9 @@
 import org.sonar.api.ce.ComputeEngineSide;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.server.ServerSide;
-import org.sonar.api.user.User;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
+import org.sonar.db.user.UserDto;
 import org.sonar.server.issue.IssueFieldsSetter;
 
 @ServerSide
@@ -64,7 +64,7 @@ public Issue issue() {
     }
 
     @Override
-    public Function.Context setAssignee(@Nullable User user) {
+    public Function.Context setAssignee(@Nullable UserDto user) {
       updater.assign(issue, user, changeContext);
       return this;
     }

server/sonar-server/src/test/java/org/sonar/server/issue/AssignActionTest.java

@@ -19,108 +19,92 @@
  */
 package org.sonar.server.issue;
 
-import java.util.List;
+import com.google.common.collect.ImmutableMap;
+import java.util.Date;
+import java.util.HashMap;
 import java.util.Map;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
 import org.sonar.api.issue.Issue;
-import org.sonar.api.user.User;
-import org.sonar.api.user.UserFinder;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
-import org.sonar.core.user.DefaultUser;
-import org.sonar.server.user.ThreadLocalUserSession;
-
-import static com.google.common.collect.Lists.newArrayList;
-import static com.google.common.collect.Maps.newHashMap;
+import org.sonar.db.DbTester;
+import org.sonar.db.user.UserDto;
+import org.sonar.server.exceptions.NotFoundException;
+import org.sonar.server.issue.ws.BulkChangeAction;
+import org.sonar.server.tester.UserSessionRule;
+
+import static java.util.Collections.emptyMap;
+import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Mockito.any;
-import static org.mockito.Mockito.eq;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
+import static org.sonar.db.user.UserTesting.newUserDto;
 
 public class AssignActionTest {
 
-  private AssignAction action;
-
-  private final UserFinder userFinder = mock(UserFinder.class);
-  private IssueFieldsSetter issueUpdater = mock(IssueFieldsSetter.class);
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
 
   @Rule
-  public ExpectedException throwable = ExpectedException.none();
+  public UserSessionRule userSession = UserSessionRule.standalone();
 
-  @Before
-  public void before() {
-    action = new AssignAction(userFinder, issueUpdater);
-  }
+  @Rule
+  public DbTester dbTester = DbTester.create();
 
-  @Test
-  public void should_execute() {
-    User assignee = new DefaultUser();
+  private IssueFieldsSetter issueUpdater = new IssueFieldsSetter();
 
-    Map<String, Object> properties = newHashMap();
-    properties.put(AssignAction.VERIFIED_ASSIGNEE, assignee);
-    DefaultIssue issue = mock(DefaultIssue.class);
+  private IssueChangeContext issueChangeContext = IssueChangeContext.createUser(new Date(), "emmerik");
 
-    Action.Context context = mock(Action.Context.class);
-    when(context.issue()).thenReturn(issue);
+  private DefaultIssue issue = new DefaultIssue().setKey("ABC");
+  private Action.Context context = new BulkChangeAction.ActionContext(issue, issueChangeContext);
 
-    action.execute(properties, context);
-    verify(issueUpdater).assign(eq(issue), eq(assignee), any(IssueChangeContext.class));
-  }
+  private AssignAction action = new AssignAction(dbTester.getDbClient(), issueUpdater);
 
   @Test
-  public void should_fail_if_assignee_is_not_verified() {
-    throwable.expect(IllegalArgumentException.class);
-    throwable.expectMessage("Assignee is missing from the execution parameters");
+  public void assign_issue() {
+    UserDto assignee = newUserDto();
 
-    Map<String, Object> properties = newHashMap();
+    action.execute(ImmutableMap.of("verifiedAssignee", assignee), context);
 
-    Action.Context context = mock(Action.Context.class);
+    assertThat(issue.assignee()).isEqualTo(assignee.getLogin());
+  }
+
+  @Test
+  public void fail_if_assignee_is_not_verified() {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Assignee is missing from the execution parameters");
 
-    action.execute(properties, context);
+    action.execute(emptyMap(), context);
   }
 
   @Test
-  public void should_verify_assignee_exists() {
+  public void verify_that_assignee_exists() {
     String assignee = "arthur";
-    Map<String, Object> properties = newHashMap();
-    properties.put("assignee", assignee);
+    Map<String, Object> properties = new HashMap<>(ImmutableMap.of("assignee", assignee));
+    UserDto user = dbTester.users().insertUser(assignee);
 
-    User user = new DefaultUser().setLogin(assignee);
+    boolean verify = action.verify(properties, singletonList(issue), userSession);
 
-    List<DefaultIssue> issues = newArrayList(new DefaultIssue().setKey("ABC"));
-    when(userFinder.findByLogin(assignee)).thenReturn(user);
-    assertThat(action.verify(properties, issues, mock(ThreadLocalUserSession.class))).isTrue();
-    assertThat(properties.get(AssignAction.VERIFIED_ASSIGNEE)).isEqualTo(user);
+    assertThat(verify).isTrue();
+    UserDto verifiedUser = (UserDto) properties.get(AssignAction.VERIFIED_ASSIGNEE);
+    assertThat(verifiedUser.getLogin()).isEqualTo(user.getLogin());
   }
 
   @Test
-  public void should_fail_if_assignee_does_not_exists() {
-    throwable.expect(IllegalArgumentException.class);
-    throwable.expectMessage("Unknown user: arthur");
+  public void fail_if_assignee_does_not_exists() {
+    expectedException.expect(NotFoundException.class);
+    expectedException.expectMessage("Unknown user: arthur");
 
-    String assignee = "arthur";
-    Map<String, Object> properties = newHashMap();
-    properties.put("assignee", assignee);
-
-    List<DefaultIssue> issues = newArrayList(new DefaultIssue().setKey("ABC"));
-    when(userFinder.findByLogin(assignee)).thenReturn(null);
-    action.verify(properties, issues, mock(ThreadLocalUserSession.class));
+    action.verify(ImmutableMap.of("assignee", "arthur"), singletonList(issue), userSession);
   }
 
   @Test
-  public void should_unassign_if_assignee_is_empty() {
-    String assignee = "";
-    Map<String, Object> properties = newHashMap();
-    properties.put("assignee", assignee);
-
-    List<DefaultIssue> issues = newArrayList(new DefaultIssue().setKey("ABC"));
-    action.verify(properties, issues, mock(ThreadLocalUserSession.class));
-    assertThat(action.verify(properties, issues, mock(ThreadLocalUserSession.class))).isTrue();
+  public void unassign_issue_if_assignee_is_empty() {
+    Map<String, Object> properties = new HashMap<>(ImmutableMap.of("assignee", ""));
+
+    boolean verify = action.verify(properties, singletonList(issue), userSession);
+
+    assertThat(verify).isTrue();
     assertThat(properties.containsKey(AssignAction.VERIFIED_ASSIGNEE)).isTrue();
     assertThat(properties.get(AssignAction.VERIFIED_ASSIGNEE)).isNull();
   }