From 00cc5932731c3dd656feeb38a6d17aa53a9d96a9 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 1 Aug 2014 17:26:42 +0200 Subject: [PATCH] Revert "SONAR-5402 Project administrators should be able to change quality profiles and gates" This reverts commit ed3e542f92ec51b689aa0c7fe044910b3e815b44. --- .../component/persistence/ComponentDao.java | 10 ---- .../ws/DuplicationsJsonWriter.java | 2 +- .../server/qualitygate/QualityGates.java | 24 ++++----- .../QProfileProjectOperations.java | 19 +++---- .../persistence/ComponentDaoTest.java | 16 +----- .../ws/DuplicationsJsonWriterTest.java | 12 ++--- .../server/qualitygate/QualityGatesTest.java | 51 +++++-------------- .../core/component/db/ComponentMapper.java | 6 --- 8 files changed, 41 insertions(+), 99 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/component/persistence/ComponentDao.java b/server/sonar-server/src/main/java/org/sonar/server/component/persistence/ComponentDao.java index 4bf68386b8c..657287999b7 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/component/persistence/ComponentDao.java +++ b/server/sonar-server/src/main/java/org/sonar/server/component/persistence/ComponentDao.java @@ -43,15 +43,6 @@ public class ComponentDao extends BaseDao } public ComponentDto getById(Long id, DbSession session) { - ComponentDto componentDto = getNullableById(id, session); - if (componentDto == null) { - throw new NotFoundException(String.format("Project with id '%s' not found", id)); - } - return componentDto; - } - - @CheckForNull - public ComponentDto getNullableById(Long id, DbSession session) { return mapper(session).selectById(id); } @@ -77,7 +68,6 @@ public class ComponentDao extends BaseDao } @Override - @CheckForNull protected ComponentDto doGetNullableByKey(DbSession session, String key) { return mapper(session).selectByKey(key); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsJsonWriter.java b/server/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsJsonWriter.java index 2c6addbbfb4..ccc2afb7032 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsJsonWriter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsJsonWriter.java @@ -125,7 +125,7 @@ public class DuplicationsJsonWriter implements ServerComponent { private ComponentDto getProject(@Nullable Long projectId, Map projectsById, DbSession session) { ComponentDto project = projectsById.get(projectId); if (project == null && projectId != null) { - project = componentDao.getNullableById(projectId, session); + project = componentDao.getById(projectId, session); if (project != null) { projectsById.put(project.getId(), project); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java index 41e406bfee5..e627be18a6c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java @@ -29,8 +29,6 @@ import org.sonar.api.measures.CoreMetrics; import org.sonar.api.measures.Metric; import org.sonar.api.measures.Metric.ValueType; import org.sonar.api.measures.MetricFinder; -import org.sonar.api.web.UserRole; -import org.sonar.core.component.ComponentDto; import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.MyBatis; @@ -209,11 +207,12 @@ public class QualityGates { } public void associateProject(Long qGateId, Long projectId) { + checkPermission(UserSession.get()); + DbSession session = myBatis.openSession(false); try { getNonNullQgate(qGateId); - ComponentDto project = componentDao.getById(projectId, session); - checkPermission(UserSession.get(), project.key()); + checkNonNullProject(projectId, session); propertiesDao.setProperty(new PropertyDto().setKey(SONAR_QUALITYGATE_PROPERTY).setResourceId(projectId).setValue(qGateId.toString())); } finally { MyBatis.closeQuietly(session); @@ -221,11 +220,12 @@ public class QualityGates { } public void dissociateProject(Long qGateId, Long projectId) { + checkPermission(UserSession.get()); + DbSession session = myBatis.openSession(false); try { getNonNullQgate(qGateId); - ComponentDto project = componentDao.getById(projectId, session); - checkPermission(UserSession.get(), project.key()); + checkNonNullProject(projectId, session); propertiesDao.deleteProjectProperty(SONAR_QUALITYGATE_PROPERTY, projectId); } finally { MyBatis.closeQuietly(session); @@ -338,6 +338,12 @@ public class QualityGates { return condition; } + private void checkNonNullProject(long projectId, DbSession session) { + if (!componentDao.existsById(projectId, session)) { + throw new NotFoundException("There is no project with id=" + projectId); + } + } + private void validateQualityGate(@Nullable Long updatingQgateId, @Nullable String name) { Errors errors = new Errors(); if (Strings.isNullOrEmpty(name)) { @@ -360,10 +366,4 @@ public class QualityGates { private void checkPermission(UserSession userSession) { userSession.checkGlobalPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN); } - - private void checkPermission(UserSession userSession, String projectKey) { - if (!userSession.hasGlobalPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN) && !userSession.hasProjectPermission(UserRole.ADMIN, projectKey)) { - throw new ForbiddenException("Insufficient privileges"); - } - } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileProjectOperations.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileProjectOperations.java index 801438c873a..429b192a336 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileProjectOperations.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileProjectOperations.java @@ -22,7 +22,6 @@ package org.sonar.server.qualityprofile; import org.sonar.api.ServerComponent; import org.sonar.api.component.Component; -import org.sonar.api.web.UserRole; import org.sonar.core.component.ComponentDto; import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.persistence.DbSession; @@ -30,12 +29,9 @@ import org.sonar.core.persistence.MyBatis; import org.sonar.core.properties.PropertyDto; import org.sonar.core.qualityprofile.db.QualityProfileDto; import org.sonar.server.db.DbClient; -import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.user.UserSession; -import javax.annotation.Nullable; - public class QProfileProjectOperations implements ServerComponent { private final DbClient db; @@ -45,6 +41,7 @@ public class QProfileProjectOperations implements ServerComponent { } public void addProject(int profileId, long projectId, UserSession userSession) { + checkPermission(userSession); DbSession session = db.openSession(false); try { addProject(profileId, projectId, userSession, session); @@ -55,8 +52,8 @@ public class QProfileProjectOperations implements ServerComponent { } void addProject(int profileId, long projectId, UserSession userSession, DbSession session) { + checkPermission(userSession); ComponentDto project = (ComponentDto) findProjectNotNull(projectId, session); - checkPermission(userSession, project.key()); QualityProfileDto qualityProfile = findNotNull(profileId, session); db.propertiesDao().setProperty(new PropertyDto().setKey( @@ -65,10 +62,10 @@ public class QProfileProjectOperations implements ServerComponent { } public void removeProject(int profileId, long projectId, UserSession userSession) { + checkPermission(userSession); DbSession session = db.openSession(false); try { ComponentDto project = (ComponentDto) findProjectNotNull(projectId, session); - checkPermission(userSession, project.key()); QualityProfileDto qualityProfile = findNotNull(profileId, session); db.propertiesDao().deleteProjectProperty(QProfileProjectLookup.PROFILE_PROPERTY_PREFIX + qualityProfile.getLanguage(), project.getId(), session); @@ -79,10 +76,10 @@ public class QProfileProjectOperations implements ServerComponent { } public void removeProject(String language, long projectId, UserSession userSession) { + checkPermission(userSession); DbSession session = db.openSession(false); try { ComponentDto project = (ComponentDto) findProjectNotNull(projectId, session); - checkPermission(userSession, project.key()); db.propertiesDao().deleteProjectProperty(QProfileProjectLookup.PROFILE_PROPERTY_PREFIX + language, project.getId(), session); session.commit(); @@ -92,7 +89,7 @@ public class QProfileProjectOperations implements ServerComponent { } public void removeAllProjects(int profileId, UserSession userSession) { - checkPermission(userSession, null); + checkPermission(userSession); DbSession session = db.openSession(false); try { QualityProfileDto qualityProfile = findNotNull(profileId, session); @@ -117,10 +114,8 @@ public class QProfileProjectOperations implements ServerComponent { return component; } - private void checkPermission(UserSession userSession, @Nullable String projectKey) { - if (!userSession.hasGlobalPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN) && projectKey != null && !userSession.hasProjectPermission(UserRole.ADMIN, projectKey)) { - throw new ForbiddenException("Insufficient privileges"); - } + private void checkPermission(UserSession userSession) { + userSession.checkGlobalPermission(GlobalPermissions.QUALITY_PROFILE_ADMIN); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/component/persistence/ComponentDaoTest.java b/server/sonar-server/src/test/java/org/sonar/server/component/persistence/ComponentDaoTest.java index 35a4fa5c9e0..190127ecca3 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/component/persistence/ComponentDaoTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/component/persistence/ComponentDaoTest.java @@ -97,21 +97,7 @@ public class ComponentDaoTest extends AbstractDaoTestCase { setupData("shared"); assertThat(dao.getById(4L, session)).isNotNull(); - } - - @Test(expected = NotFoundException.class) - public void fail_to_get_by_id_when_project_not_found() { - setupData("shared"); - - dao.getById(111L, session); - } - - @Test - public void get_nullable_by_id() { - setupData("shared"); - - assertThat(dao.getNullableById(4L, session)).isNotNull(); - assertThat(dao.getNullableById(111L, session)).isNull(); + assertThat(dao.getById(111L, session)).isNull(); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsJsonWriterTest.java b/server/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsJsonWriterTest.java index 226f7d4ecec..6a26a071871 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsJsonWriterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsJsonWriterTest.java @@ -66,8 +66,8 @@ public class DuplicationsJsonWriterTest { when(componentDao.getNullableByKey(session, key1)).thenReturn(file1); when(componentDao.getNullableByKey(session, key2)).thenReturn(file2); - when(componentDao.getNullableById(1L, session)).thenReturn(new ComponentDto().setId(1L).setKey("org.codehaus.sonar:sonar").setLongName("SonarQube")); - when(componentDao.getNullableById(5L, session)).thenReturn(new ComponentDto().setId(5L).setKey("org.codehaus.sonar:sonar-ws-client").setLongName("SonarQube :: Web Service Client")); + when(componentDao.getById(1L, session)).thenReturn(new ComponentDto().setId(1L).setKey("org.codehaus.sonar:sonar").setLongName("SonarQube")); + when(componentDao.getById(5L, session)).thenReturn(new ComponentDto().setId(5L).setKey("org.codehaus.sonar:sonar-ws-client").setLongName("SonarQube :: Web Service Client")); List blocks = newArrayList(); blocks.add(new DuplicationsParser.Block(newArrayList( @@ -112,8 +112,8 @@ public class DuplicationsJsonWriterTest { verify(componentDao, times(2)).getNullableByKey(eq(session), anyString()); // Verify call to dao is cached when searching for project / sub project - verify(componentDao, times(1)).getNullableById(eq(1L), eq(session)); - verify(componentDao, times(1)).getNullableById(eq(5L), eq(session)); + verify(componentDao, times(1)).getById(eq(1L), eq(session)); + verify(componentDao, times(1)).getById(eq(5L), eq(session)); } @Test @@ -125,7 +125,7 @@ public class DuplicationsJsonWriterTest { when(componentDao.getNullableByKey(session, key1)).thenReturn(file1); when(componentDao.getNullableByKey(session, key2)).thenReturn(file2); - when(componentDao.getNullableById(1L, session)).thenReturn(new ComponentDto().setId(1L).setKey("org.codehaus.sonar:sonar").setLongName("SonarQube")); + when(componentDao.getById(1L, session)).thenReturn(new ComponentDto().setId(1L).setKey("org.codehaus.sonar:sonar").setLongName("SonarQube")); List blocks = newArrayList(); blocks.add(new DuplicationsParser.Block(newArrayList( @@ -171,7 +171,7 @@ public class DuplicationsJsonWriterTest { ComponentDto file1 = new ComponentDto().setId(10L).setQualifier("FIL").setKey(key1).setLongName("PropertyDeleteQuery").setProjectId(1L); when(componentDao.getNullableByKey(session, key1)).thenReturn(file1); - when(componentDao.getNullableById(1L, session)).thenReturn(new ComponentDto().setId(1L).setKey("org.codehaus.sonar:sonar").setLongName("SonarQube")); + when(componentDao.getById(1L, session)).thenReturn(new ComponentDto().setId(1L).setKey("org.codehaus.sonar:sonar").setLongName("SonarQube")); List blocks = newArrayList(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java index 1de5e771424..bc0f6d85dc0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java @@ -34,8 +34,6 @@ import org.sonar.api.measures.CoreMetrics; import org.sonar.api.measures.Metric; import org.sonar.api.measures.Metric.ValueType; import org.sonar.api.measures.MetricFinder; -import org.sonar.api.web.UserRole; -import org.sonar.core.component.ComponentDto; import org.sonar.core.permission.GlobalPermissions; import org.sonar.core.persistence.DbSession; import org.sonar.core.persistence.MyBatis; @@ -62,7 +60,10 @@ import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyLong; import static org.mockito.Matchers.anyString; import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; @RunWith(MockitoJUnitRunner.class) public class QualityGatesTest { @@ -90,20 +91,15 @@ public class QualityGatesTest { QualityGates qGates; - static final String PROJECT_KEY = "SonarQube"; - - UserSession authorizedProfileAdminUserSession = MockUserSession.create().setLogin("gaudol").setName("Olivier").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); - UserSession authorizedProjectAdminUserSession = MockUserSession.create().setLogin("gaudol").setName("Olivier").addProjectPermissions(UserRole.ADMIN, PROJECT_KEY); + UserSession authorizedUserSession = MockUserSession.create().setLogin("gaudol").setName("Olivier").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN); UserSession unauthenticatedUserSession = MockUserSession.create(); UserSession unauthorizedUserSession = MockUserSession.create().setLogin("polop").setName("Polop"); @Before public void initialize() { - when(componentDao.getById(anyLong(), eq(session))).thenReturn(new ComponentDto().setId(1L).setKey(PROJECT_KEY)); - when(myBatis.openSession(false)).thenReturn(session); qGates = new QualityGates(dao, conditionDao, metricFinder, propertiesDao, componentDao, myBatis); - UserSessionTestUtils.setUserSession(authorizedProfileAdminUserSession); + UserSessionTestUtils.setUserSession(authorizedUserSession); } @Test @@ -218,7 +214,7 @@ public class QualityGatesTest { public void should_select_default_qgate() throws Exception { long defaultId = 42L; String defaultName = "Default Name"; - when(dao.selectById(defaultId)).thenReturn(new QualityGateDto().setId(defaultId).setName(defaultName)); + when(dao.selectById(defaultId)).thenReturn(new QualityGateDto().setId(defaultId).setName(defaultName )); qGates.setDefault(defaultId); verify(dao).selectById(defaultId); ArgumentCaptor propertyCaptor = ArgumentCaptor.forClass(PropertyDto.class); @@ -464,7 +460,7 @@ public class QualityGatesTest { QualityGateConditionDto cond1 = new QualityGateConditionDto().setMetricId(metric1Id); QualityGateConditionDto cond2 = new QualityGateConditionDto().setMetricId(metric2Id); Collection conditions = ImmutableList.of(cond1, cond2); - when(conditionDao.selectForQualityGate(qGateId)).thenReturn(conditions); + when(conditionDao.selectForQualityGate(qGateId)).thenReturn(conditions ); Metric metric1 = mock(Metric.class); when(metric1.getKey()).thenReturn(metric1Key); when(metricFinder.findById((int) metric1Id)).thenReturn(metric1); @@ -486,7 +482,7 @@ public class QualityGatesTest { QualityGateConditionDto cond1 = new QualityGateConditionDto().setMetricId(metric1Id); QualityGateConditionDto cond2 = new QualityGateConditionDto().setMetricId(metric2Id); Collection conditions = ImmutableList.of(cond1, cond2); - when(conditionDao.selectForQualityGate(qGateId)).thenReturn(conditions); + when(conditionDao.selectForQualityGate(qGateId)).thenReturn(conditions ); Metric metric1 = mock(Metric.class); when(metric1.getKey()).thenReturn(metric1Key); when(metricFinder.findById((int) metric1Id)).thenReturn(metric1); @@ -513,6 +509,7 @@ public class QualityGatesTest { Long qGateId = 42L; Long projectId = 24L; when(dao.selectById(qGateId)).thenReturn(new QualityGateDto().setId(qGateId)); + when(componentDao.existsById(projectId, session)).thenReturn(true); qGates.associateProject(qGateId, projectId); verify(dao).selectById(qGateId); ArgumentCaptor propertyCaptor = ArgumentCaptor.forClass(PropertyDto.class); @@ -523,21 +520,12 @@ public class QualityGatesTest { assertThat(property.getValue()).isEqualTo("42"); } - @Test - public void associate_project_with_project_admin_permission() { - UserSessionTestUtils.setUserSession(authorizedProjectAdminUserSession); - + @Test(expected = NotFoundException.class) + public void should_fail_associate_project_on_not_existing_project() { Long qGateId = 42L; Long projectId = 24L; when(dao.selectById(qGateId)).thenReturn(new QualityGateDto().setId(qGateId)); - qGates.associateProject(qGateId, projectId); - verify(dao).selectById(qGateId); - ArgumentCaptor propertyCaptor = ArgumentCaptor.forClass(PropertyDto.class); - verify(propertiesDao).setProperty(propertyCaptor.capture()); - PropertyDto property = propertyCaptor.getValue(); - assertThat(property.getKey()).isEqualTo("sonar.qualitygate"); - assertThat(property.getResourceId()).isEqualTo(projectId); - assertThat(property.getValue()).isEqualTo("42"); + qGates.associateProject(qGateId , projectId); } @Test @@ -545,18 +533,7 @@ public class QualityGatesTest { Long qGateId = 42L; Long projectId = 24L; when(dao.selectById(qGateId)).thenReturn(new QualityGateDto().setId(qGateId)); - qGates.dissociateProject(qGateId, projectId); - verify(dao).selectById(qGateId); - verify(propertiesDao).deleteProjectProperty("sonar.qualitygate", projectId); - } - - @Test - public void dissociate_project_with_project_admin_permission() { - UserSessionTestUtils.setUserSession(authorizedProjectAdminUserSession); - - Long qGateId = 42L; - Long projectId = 24L; - when(dao.selectById(qGateId)).thenReturn(new QualityGateDto().setId(qGateId)); + when(componentDao.existsById(projectId, session)).thenReturn(true); qGates.dissociateProject(qGateId, projectId); verify(dao).selectById(qGateId); verify(propertiesDao).deleteProjectProperty("sonar.qualitygate", projectId); diff --git a/sonar-core/src/main/java/org/sonar/core/component/db/ComponentMapper.java b/sonar-core/src/main/java/org/sonar/core/component/db/ComponentMapper.java index 7146f92cf01..8e925d67ca0 100644 --- a/sonar-core/src/main/java/org/sonar/core/component/db/ComponentMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/component/db/ComponentMapper.java @@ -22,8 +22,6 @@ package org.sonar.core.component.db; import org.apache.ibatis.annotations.Param; import org.sonar.core.component.ComponentDto; -import javax.annotation.CheckForNull; - import java.util.List; /** @@ -31,16 +29,12 @@ import java.util.List; */ public interface ComponentMapper { - @CheckForNull ComponentDto selectByKey(String key); - @CheckForNull ComponentDto selectById(long id); - @CheckForNull ComponentDto selectRootProjectByKey(String key); - @CheckForNull ComponentDto selectParentModuleByKey(String key); /** -- 2.39.5