From 871859523b79452634fa62a939ec719ca9da04f7 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Wed, 28 Sep 2016 16:28:35 +0200 Subject: [PATCH] Add missing DbSession parameters to PermissionDao --- .../qualityprofile/QProfileProjectLookup.java | 13 +++--- .../sonar/server/user/ServerUserSession.java | 34 +++++++++----- .../sonar/db/permission/PermissionDao.java | 45 +++++-------------- .../sonar/db/permission/PermissionMapper.java | 2 +- .../db/permission/PermissionDaoTest.java | 24 +++++----- 5 files changed, 51 insertions(+), 67 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileProjectLookup.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileProjectLookup.java index 7528a2496de..e66f126f112 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileProjectLookup.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileProjectLookup.java @@ -25,13 +25,12 @@ import java.util.Collection; import java.util.List; import java.util.Map; import javax.annotation.CheckForNull; -import org.sonar.api.component.Component; import org.sonar.api.ce.ComputeEngineSide; +import org.sonar.api.component.Component; import org.sonar.api.server.ServerSide; import org.sonar.api.web.UserRole; import org.sonar.db.DbClient; import org.sonar.db.DbSession; -import org.sonar.db.MyBatis; import org.sonar.db.qualityprofile.QualityProfileDto; import org.sonar.server.user.UserSession; @@ -48,17 +47,17 @@ public class QProfileProjectLookup { } public List projects(int profileId) { - DbSession session = db.openSession(false); + DbSession dbSession = db.openSession(false); try { - QualityProfileDto qualityProfile = db.qualityProfileDao().selectById(session, profileId); + QualityProfileDto qualityProfile = db.qualityProfileDao().selectById(dbSession, profileId); QProfileValidations.checkProfileIsNotNull(qualityProfile); Map componentsByKeys = Maps.newHashMap(); - for (Component component : db.qualityProfileDao().selectProjects(qualityProfile.getName(), qualityProfile.getLanguage(), session)) { + for (Component component : db.qualityProfileDao().selectProjects(qualityProfile.getName(), qualityProfile.getLanguage(), dbSession)) { componentsByKeys.put(component.key(), component); } List result = Lists.newArrayList(); - Collection authorizedProjectKeys = db.permissionDao().selectAuthorizedRootProjectsKeys(userSession.getUserId(), UserRole.USER); + Collection authorizedProjectKeys = db.permissionDao().selectAuthorizedRootProjectsKeys(dbSession, userSession.getUserId(), UserRole.USER); for (Map.Entry entry : componentsByKeys.entrySet()) { if (authorizedProjectKeys.contains(entry.getKey())) { result.add(entry.getValue()); @@ -67,7 +66,7 @@ public class QProfileProjectLookup { return result; } finally { - MyBatis.closeQuietly(session); + db.closeSession(dbSession); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java b/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java index dc660367fe2..6034429bb2d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java +++ b/server/sonar-server/src/main/java/org/sonar/server/user/ServerUserSession.java @@ -19,9 +19,6 @@ */ package org.sonar.server.user; -import static com.google.common.collect.Maps.newHashMap; -import static java.util.Objects.requireNonNull; - import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; @@ -38,6 +35,9 @@ import org.sonar.db.permission.PermissionDao; import org.sonar.db.user.GroupDto; import org.sonar.db.user.UserDto; +import static com.google.common.collect.Maps.newHashMap; +import static java.util.Objects.requireNonNull; + /** * Part of the current HTTP session */ @@ -54,7 +54,7 @@ public class ServerUserSession extends AbstractUserSession { this.permissionDao = dbClient.permissionDao(); this.resourceDao = dbClient.resourceDao(); this.globalPermissions = null; - if(userDto != null){ + if (userDto != null) { this.setLogin(userDto.getLogin()); this.setName(userDto.getName()); this.setUserId(userDto.getId().intValue()); @@ -62,12 +62,12 @@ public class ServerUserSession extends AbstractUserSession { } } - public static ServerUserSession createForUser(DbClient dbClient, UserDto userDto){ + public static ServerUserSession createForUser(DbClient dbClient, UserDto userDto) { requireNonNull(userDto, "UserDto must not be null"); return new ServerUserSession(dbClient, userDto); } - public static ServerUserSession createForAnonymous(DbClient dbClient){ + public static ServerUserSession createForAnonymous(DbClient dbClient) { return new ServerUserSession(dbClient, null); } @@ -94,11 +94,16 @@ public class ServerUserSession extends AbstractUserSession { private boolean hasProjectPermission(String permission, String projectKey) { if (!projectPermissionsCheckedByKey.contains(permission)) { - Collection projectKeys = permissionDao.selectAuthorizedRootProjectsKeys(userId, permission); - for (String key : projectKeys) { - projectKeyByPermission.put(permission, key); + DbSession dbSession = dbClient.openSession(false); + try { + Collection projectKeys = permissionDao.selectAuthorizedRootProjectsKeys(dbSession, userId, permission); + for (String key : projectKeys) { + projectKeyByPermission.put(permission, key); + } + projectPermissionsCheckedByKey.add(permission); + } finally { + dbClient.closeSession(dbSession); } - projectPermissionsCheckedByKey.add(permission); } return projectKeyByPermission.get(permission).contains(projectKey); } @@ -106,8 +111,13 @@ public class ServerUserSession extends AbstractUserSession { // To keep private private boolean hasProjectPermissionByUuid(String permission, String projectUuid) { if (!projectPermissionsCheckedByUuid.contains(permission)) { - Collection projectUuids = permissionDao.selectAuthorizedRootProjectsUuids(userId, permission); - addProjectPermission(permission, projectUuids); + DbSession dbSession = dbClient.openSession(false); + try { + Collection projectUuids = permissionDao.selectAuthorizedRootProjectsUuids(dbSession, userId, permission); + addProjectPermission(permission, projectUuids); + } finally { + dbClient.closeSession(dbSession); + } } return projectUuidByPermission.get(permission).contains(projectUuid); } diff --git a/sonar-db/src/main/java/org/sonar/db/permission/PermissionDao.java b/sonar-db/src/main/java/org/sonar/db/permission/PermissionDao.java index 1ac0caad1aa..70774f47bae 100644 --- a/sonar-db/src/main/java/org/sonar/db/permission/PermissionDao.java +++ b/sonar-db/src/main/java/org/sonar/db/permission/PermissionDao.java @@ -20,7 +20,7 @@ package org.sonar.db.permission; import java.util.Collection; -import java.util.Collections; +import java.util.HashMap; import java.util.List; import java.util.Map; import javax.annotation.Nullable; @@ -28,7 +28,6 @@ import org.sonar.db.Dao; import org.sonar.db.DbSession; import org.sonar.db.MyBatis; -import static com.google.common.collect.Maps.newHashMap; import static org.sonar.db.DatabaseUtils.executeLargeInputs; /** @@ -46,65 +45,41 @@ public class PermissionDao implements Dao { this.mybatis = mybatis; } - public Collection keepAuthorizedProjectIds(final DbSession session, final Collection componentIds, @Nullable final Integer userId, final String role) { - if (componentIds.isEmpty()) { - return Collections.emptySet(); - } + public Collection keepAuthorizedProjectIds(DbSession session, Collection componentIds, @Nullable Integer userId, String role) { return executeLargeInputs( componentIds, partition -> { if (userId == null) { return session.getMapper(PermissionMapper.class).keepAuthorizedProjectIdsForAnonymous(role, componentIds); - } else { - return session.getMapper(PermissionMapper.class).keepAuthorizedProjectIdsForUser(userId, role, componentIds); } + return session.getMapper(PermissionMapper.class).keepAuthorizedProjectIdsForUser(userId, role, componentIds); }); } - public Collection selectAuthorizedRootProjectsKeys(@Nullable Integer userId, String role) { - DbSession session = mybatis.openSession(false); - try { - return selectAuthorizedRootProjectsKeys(userId, role, session); - - } finally { - MyBatis.closeQuietly(session); - } - } - - public Collection selectAuthorizedRootProjectsUuids(@Nullable Integer userId, String role) { - DbSession session = mybatis.openSession(false); - try { - return selectAuthorizedRootProjectsUuids(userId, role, session); - - } finally { - MyBatis.closeQuietly(session); - } - } - - private static Collection selectAuthorizedRootProjectsKeys(@Nullable Integer userId, String role, DbSession session) { + public Collection selectAuthorizedRootProjectsKeys(DbSession dbSession, @Nullable Integer userId, String role) { String sql; - Map params = newHashMap(); + Map params = new HashMap<>(2); sql = "selectAuthorizedRootProjectsKeys"; params.put(USER_ID_PARAM, userId); params.put("role", role); - return session.selectList(sql, params); + return dbSession.selectList(sql, params); } - private static Collection selectAuthorizedRootProjectsUuids(@Nullable Integer userId, String role, DbSession session) { + public Collection selectAuthorizedRootProjectsUuids(DbSession dbSession, @Nullable Integer userId, String role) { String sql; - Map params = newHashMap(); + Map params = new HashMap<>(2); sql = "selectAuthorizedRootProjectsUuids"; params.put(USER_ID_PARAM, userId); params.put("role", role); - return session.selectList(sql, params); + return dbSession.selectList(sql, params); } public List selectGlobalPermissions(@Nullable String userLogin) { DbSession session = mybatis.openSession(false); try { - Map params = newHashMap(); + Map params = new HashMap<>(1); params.put("userLogin", userLogin); return session.selectList("selectGlobalPermissions", params); } finally { diff --git a/sonar-db/src/main/java/org/sonar/db/permission/PermissionMapper.java b/sonar-db/src/main/java/org/sonar/db/permission/PermissionMapper.java index 796986ffdb4..8010812c5cc 100644 --- a/sonar-db/src/main/java/org/sonar/db/permission/PermissionMapper.java +++ b/sonar-db/src/main/java/org/sonar/db/permission/PermissionMapper.java @@ -33,6 +33,6 @@ public interface PermissionMapper { List keepAuthorizedProjectIdsForAnonymous(@Param("role") String role, @Param("componentIds") Collection componentIds); - List keepAuthorizedProjectIdsForUser(@Param("userId") Integer userId, @Param("role") String role, @Param("componentIds") Collection componentIds); + List keepAuthorizedProjectIdsForUser(@Param("userId") long userId, @Param("role") String role, @Param("componentIds") Collection componentIds); } diff --git a/sonar-db/src/test/java/org/sonar/db/permission/PermissionDaoTest.java b/sonar-db/src/test/java/org/sonar/db/permission/PermissionDaoTest.java index 739f2e83fb4..43137eb4e29 100644 --- a/sonar-db/src/test/java/org/sonar/db/permission/PermissionDaoTest.java +++ b/sonar-db/src/test/java/org/sonar/db/permission/PermissionDaoTest.java @@ -141,12 +141,12 @@ public class PermissionDaoTest { public void should_return_root_project_keys_for_user() { dbTester.prepareDbUnit(getClass(), "should_return_root_project_keys_for_user.xml"); - Collection rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(USER, "user"); + Collection rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(dbTester.getSession(), USER, "user"); assertThat(rootProjectIds).containsOnly(PROJECT); // user does not have the role "admin" - rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(USER, "admin"); + rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(dbTester.getSession(), USER, "admin"); assertThat(rootProjectIds).isEmpty(); } @@ -155,12 +155,12 @@ public class PermissionDaoTest { // but user is not in an authorized group dbTester.prepareDbUnit(getClass(), "should_return_root_project_keys_for_group.xml"); - Collection rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(USER, "user"); + Collection rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(dbTester.getSession(), USER, "user"); assertThat(rootProjectIds).containsOnly(PROJECT); // user does not have the role "admin" - rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(USER, "admin"); + rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(dbTester.getSession(), USER, "admin"); assertThat(rootProjectIds).isEmpty(); } @@ -168,12 +168,12 @@ public class PermissionDaoTest { public void should_return_root_project_keys_for_anonymous() { dbTester.prepareDbUnit(getClass(), "should_return_root_project_keys_for_anonymous.xml"); - Collection rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(null, "user"); + Collection rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(dbTester.getSession(), null, "user"); assertThat(rootProjectIds).containsOnly(PROJECT); // group does not have the role "admin" - rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(null, "admin"); + rootProjectIds = authorization.selectAuthorizedRootProjectsKeys(dbTester.getSession(), null, "admin"); assertThat(rootProjectIds).isEmpty(); } @@ -181,12 +181,12 @@ public class PermissionDaoTest { public void should_return_root_project_uuids_for_user() { dbTester.prepareDbUnit(getClass(), "should_return_root_project_keys_for_user.xml"); - Collection rootProjectUuids = authorization.selectAuthorizedRootProjectsUuids(USER, "user"); + Collection rootProjectUuids = authorization.selectAuthorizedRootProjectsUuids(dbTester.getSession(), USER, "user"); assertThat(rootProjectUuids).containsOnly("ABCD"); // user does not have the role "admin" - rootProjectUuids = authorization.selectAuthorizedRootProjectsKeys(USER, "admin"); + rootProjectUuids = authorization.selectAuthorizedRootProjectsKeys(dbTester.getSession(), USER, "admin"); assertThat(rootProjectUuids).isEmpty(); } @@ -195,12 +195,12 @@ public class PermissionDaoTest { // but user is not in an authorized group dbTester.prepareDbUnit(getClass(), "should_return_root_project_keys_for_group.xml"); - Collection rootProjectUuids = authorization.selectAuthorizedRootProjectsUuids(USER, "user"); + Collection rootProjectUuids = authorization.selectAuthorizedRootProjectsUuids(dbTester.getSession(), USER, "user"); assertThat(rootProjectUuids).containsOnly("ABCD"); // user does not have the role "admin" - rootProjectUuids = authorization.selectAuthorizedRootProjectsKeys(USER, "admin"); + rootProjectUuids = authorization.selectAuthorizedRootProjectsKeys(dbTester.getSession(), USER, "admin"); assertThat(rootProjectUuids).isEmpty(); } @@ -208,12 +208,12 @@ public class PermissionDaoTest { public void should_return_root_project_uuids_for_anonymous() { dbTester.prepareDbUnit(getClass(), "should_return_root_project_keys_for_anonymous.xml"); - Collection rootProjectUuids = authorization.selectAuthorizedRootProjectsUuids(null, "user"); + Collection rootProjectUuids = authorization.selectAuthorizedRootProjectsUuids(dbTester.getSession(), null, "user"); assertThat(rootProjectUuids).containsOnly("ABCD"); // group does not have the role "admin" - rootProjectUuids = authorization.selectAuthorizedRootProjectsKeys(null, "admin"); + rootProjectUuids = authorization.selectAuthorizedRootProjectsKeys(dbTester.getSession(), null, "admin"); assertThat(rootProjectUuids).isEmpty(); } -- 2.39.5