SONAR-6465 Apply feedback from PR

diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java
index 28a06a356a951dd726d292227bef302f2236bfd4..ae039a0830ac62131a803c0610e92b783d21e156 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/user/ws/SearchAction.java
@@ -21,11 +21,10 @@
 package org.sonar.server.user.ws;
 
 import com.google.common.base.Function;
+import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Collections2;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Maps;
 import com.google.common.collect.Multimap;
-import com.google.common.collect.Multimaps;
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
@@ -38,6 +37,7 @@ import org.sonar.api.server.ws.WebService.Param;
 import org.sonar.api.utils.text.JsonWriter;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.DbSession;
+import org.sonar.core.persistence.MyBatis;
 import org.sonar.server.db.DbClient;
 import org.sonar.server.es.SearchOptions;
 import org.sonar.server.es.SearchResult;
@@ -88,8 +88,8 @@ public class SearchAction implements UsersWsAction {
     List<String> fields = request.paramAsStrings(Param.FIELDS);
     SearchResult<UserDoc> result = userIndex.search(request.param(Param.TEXT_QUERY), options);
 
-    Multimap<String, String> groupsByLogin = Multimaps.forMap(Maps.<String, String>newHashMap());
-    DbSession session = dbClient.openSession(false);
+    Multimap<String, String> groupsByLogin = ArrayListMultimap.create();
+    DbSession dbSession = dbClient.openSession(false);
     try {
       Collection<String> logins = Collections2.transform(result.getDocs(), new Function<UserDoc, String>() {
         @Override
@@ -97,9 +97,9 @@ public class SearchAction implements UsersWsAction {
           return input.login();
         }
       });
-      groupsByLogin = dbClient.groupMembershipDao().selectGroupsByLogins(session, logins);
+      groupsByLogin = dbClient.groupMembershipDao().selectGroupsByLogins(dbSession, logins);
     } finally {
-      session.close();
+      MyBatis.closeQuietly(dbSession);
     }
 
     JsonWriter json = response.newJsonWriter().beginObject();
@@ -117,12 +117,7 @@ public class SearchAction implements UsersWsAction {
       writeIfNeeded(json, user.name(), FIELD_NAME, fields);
       writeIfNeeded(json, user.email(), FIELD_EMAIL, fields);
       writeGroupsIfNeeded(json, groupsByLogin.get(user.login()), fields);
-      if (fieldIsWanted(FIELD_SCM_ACCOUNTS, fields)) {
-        json.name(FIELD_SCM_ACCOUNTS)
-          .beginArray()
-          .values(user.scmAccounts())
-          .endArray();
-      }
+      writeScmAccountsIfNeeded(json, fields, user);
       json.endObject();
     }
     json.endArray();
@@ -144,6 +139,15 @@ public class SearchAction implements UsersWsAction {
     }
   }
 
+  private void writeScmAccountsIfNeeded(JsonWriter json, List<String> fields, UserDoc user) {
+    if (fieldIsWanted(FIELD_SCM_ACCOUNTS, fields)) {
+      json.name(FIELD_SCM_ACCOUNTS)
+        .beginArray()
+        .values(user.scmAccounts())
+        .endArray();
+    }
+  }
+
   private boolean fieldIsWanted(String field, @Nullable List<String> fields) {
     return fields == null || fields.isEmpty() || fields.contains(field);
   }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java
index 1e9c7fa2b5de31d63843590b94aba3a4282e2d9b..f574c9e5b83c285db59c85ed30009ccb273f87dd 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/UserUpdaterTest.java
@@ -28,11 +28,7 @@ import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.junit.runner.RunWith;
 import org.mockito.ArgumentCaptor;
-import org.mockito.Captor;
-import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
 import org.sonar.api.CoreProperties;
 import org.sonar.api.config.Settings;
 import org.sonar.api.platform.NewUserHandler;
@@ -59,11 +55,11 @@ import static com.google.common.collect.Lists.newArrayList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.data.MapEntry.entry;
 import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 @Category(DbTests.class)
-@RunWith(MockitoJUnitRunner.class)
 public class UserUpdaterTest {
 
   @ClassRule
@@ -72,17 +68,13 @@ public class UserUpdaterTest {
   @ClassRule
   public static EsTester es = new EsTester().addDefinitions(new UserIndexDefinition(new Settings()));
 
-  @Mock
-  System2 system2;
+  System2 system2 = mock(System2.class);
 
-  @Mock
-  NewUserNotifier newUserNotifier;
+  NewUserNotifier newUserNotifier = mock(NewUserNotifier.class);
 
-  @Mock
-  SecurityRealmFactory realmFactory;
+  SecurityRealmFactory realmFactory = mock(SecurityRealmFactory.class);
 
-  @Captor
-  ArgumentCaptor<NewUserHandler.Context> newUserHandler;
+  ArgumentCaptor<NewUserHandler.Context> newUserHandler = ArgumentCaptor.forClass(NewUserHandler.Context.class);
 
   Settings settings;
   UserDao userDao;

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java
index 38f507b27baade86331f6bc70e8254f08635e2ef..83b484554cb39f7b97020312a507fde527318552 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/ChangePasswordActionTest.java
@@ -25,9 +25,6 @@ import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
 import org.sonar.api.config.Settings;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.System2;
@@ -58,7 +55,6 @@ import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-@RunWith(MockitoJUnitRunner.class)
 public class ChangePasswordActionTest {
 
   static final Settings settings = new Settings().setProperty("sonar.defaultGroup", "sonar-users");
@@ -68,6 +64,7 @@ public class ChangePasswordActionTest {
 
   @ClassRule
   public static final EsTester esTester = new EsTester().addDefinitions(new UserIndexDefinition(settings));
+
   @Rule
   public UserSessionRule userSessionRule = UserSessionRule.standalone().login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN);
 
@@ -83,8 +80,7 @@ public class ChangePasswordActionTest {
 
   DbSession session;
 
-  @Mock
-  SecurityRealmFactory realmFactory;
+  SecurityRealmFactory realmFactory = mock(SecurityRealmFactory.class);
 
   @Before
   public void setUp() {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java
index 9c99614d6d804c3632da9a13cfd1c2c682226785..943620ae905292c8a862ee7d5c5bf343fb77249b 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/CreateActionTest.java
@@ -26,9 +26,7 @@ import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
+import org.junit.experimental.categories.Category;
 import org.sonar.api.config.Settings;
 import org.sonar.api.i18n.I18n;
 import org.sonar.api.server.ws.WebService;
@@ -53,12 +51,13 @@ import org.sonar.server.user.index.UserIndex;
 import org.sonar.server.user.index.UserIndexDefinition;
 import org.sonar.server.user.index.UserIndexer;
 import org.sonar.server.ws.WsTester;
+import org.sonar.test.DbTests;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
-@RunWith(MockitoJUnitRunner.class)
+@Category(DbTests.class)
 public class CreateActionTest {
 
   static final Settings settings = new Settings().setProperty("sonar.defaultGroup", "sonar-users");
@@ -68,6 +67,7 @@ public class CreateActionTest {
 
   @ClassRule
   public static final EsTester esTester = new EsTester().addDefinitions(new UserIndexDefinition(settings));
+
   @Rule
   public UserSessionRule userSessionRule = UserSessionRule.standalone();
 
@@ -83,8 +83,7 @@ public class CreateActionTest {
 
   DbSession session;
 
-  @Mock
-  I18n i18n;
+  I18n i18n = mock(I18n.class);
 
   @Before
   public void setUp() {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
index 871672759bb859cc5d6c44f67806177b4b677214..d29262ac408cc271ca4632df99f25de3ec173842 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/DeactivateActionTest.java
@@ -25,9 +25,7 @@ import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
+import org.junit.experimental.categories.Category;
 import org.sonar.api.config.Settings;
 import org.sonar.api.i18n.I18n;
 import org.sonar.api.server.ws.WebService;
@@ -51,11 +49,12 @@ import org.sonar.server.user.index.UserIndex;
 import org.sonar.server.user.index.UserIndexDefinition;
 import org.sonar.server.user.index.UserIndexer;
 import org.sonar.server.ws.WsTester;
+import org.sonar.test.DbTests;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 
-@RunWith(MockitoJUnitRunner.class)
+@Category(DbTests.class)
 public class DeactivateActionTest {
 
   static final Settings settings = new Settings();
@@ -79,8 +78,7 @@ public class DeactivateActionTest {
 
   DbSession session;
 
-  @Mock
-  I18n i18n;
+  I18n i18n = mock(I18n.class);
 
   @Before
   public void setUp() {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java
index 2a1370d1eab603a1e611620c0a10d7aec17b61a8..d3e8b7d421b1e4c639c5e2363da7dbe2b86ceda9 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/user/ws/SearchActionTest.java
@@ -28,6 +28,7 @@ import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.experimental.categories.Category;
 import org.sonar.api.config.Settings;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.System2;
@@ -49,9 +50,11 @@ import org.sonar.server.user.index.UserDoc;
 import org.sonar.server.user.index.UserIndex;
 import org.sonar.server.user.index.UserIndexDefinition;
 import org.sonar.server.ws.WsTester;
+import org.sonar.test.DbTests;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+@Category(DbTests.class)
 public class SearchActionTest {
 
   @ClassRule

diff --git a/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipDao.java b/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipDao.java
index 84447d42d1f2e838cad9f7198d72f3dd6f0a3c0e..9a03d9335c30f9ccf4ca7be562c2d1fa7cb8fef0 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/user/GroupMembershipDao.java
@@ -36,7 +36,6 @@ import org.sonar.core.persistence.DaoComponent;
 import org.sonar.core.persistence.DaoUtils;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.persistence.MyBatis;
-import org.sonar.core.util.NonNullInputFunction;
 
 public class GroupMembershipDao implements DaoComponent {
 
@@ -84,9 +83,9 @@ public class GroupMembershipDao implements DaoComponent {
 
   public Multimap<String, String> selectGroupsByLogins(final DbSession session, Collection<String> logins) {
     final Multimap<String, String> result = ArrayListMultimap.create();
-    DaoUtils.executeLargeInputs(logins, new NonNullInputFunction<List<String>, List<LoginGroup>>() {
+    DaoUtils.executeLargeInputs(logins, new Function<List<String>, List<LoginGroup>>() {
       @Override
-      protected List<LoginGroup> doApply(List<String> input) {
+      public List<LoginGroup> apply(@Nonnull List<String> input) {
         List<LoginGroup> groupMemberships = mapper(session).selectGroupsByLogins(input);
         for (LoginGroup membership : groupMemberships) {
           result.put(membership.login(), membership.groupName());