]> source.dussan.org Git - sonarqube.git/commitdiff
Revert "SONAR-5402 Project administrators should be able to change quality profiles...
authorSimon Brandhof <simon.brandhof@sonarsource.com>
Fri, 1 Aug 2014 15:26:42 +0000 (17:26 +0200)
committerSimon Brandhof <simon.brandhof@sonarsource.com>
Fri, 1 Aug 2014 15:31:29 +0000 (17:31 +0200)
This reverts commit ed3e542f92ec51b689aa0c7fe044910b3e815b44.

server/sonar-server/src/main/java/org/sonar/server/component/persistence/ComponentDao.java
server/sonar-server/src/main/java/org/sonar/server/duplication/ws/DuplicationsJsonWriter.java
server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java
server/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileProjectOperations.java
server/sonar-server/src/test/java/org/sonar/server/component/persistence/ComponentDaoTest.java
server/sonar-server/src/test/java/org/sonar/server/duplication/ws/DuplicationsJsonWriterTest.java
server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java
sonar-core/src/main/java/org/sonar/core/component/db/ComponentMapper.java

index 4bf68386b8c0f3f949ebd9bc971bf22b1d8c995e..657287999b7acee92d55eacaf08000f5560be740 100644 (file)
@@ -43,15 +43,6 @@ public class ComponentDao extends BaseDao<ComponentMapper, ComponentDto, String>
   }
 
   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<ComponentMapper, ComponentDto, String>
   }
 
   @Override
-  @CheckForNull
   protected ComponentDto doGetNullableByKey(DbSession session, String key) {
     return mapper(session).selectByKey(key);
   }
index 2c6addbbfb461254add9b6d595e007f17cb98a8e..ccc2afb70320c8de02124bc640e8d9a2dc4b08c8 100644 (file)
@@ -125,7 +125,7 @@ public class DuplicationsJsonWriter implements ServerComponent {
   private ComponentDto getProject(@Nullable Long projectId, Map<Long, ComponentDto> 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);
       }
index 41e406bfee5c10ad83dea454f0fb52fe512b0724..e627be18a6c9d450a2c9b8fedfa9582860d58695 100644 (file)
@@ -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");
-    }
-  }
 }
index 801438c873a9e0ddbe5c1055c923976621049de3..429b192a336f456aee68539a5664d65ada565b37 100644 (file)
@@ -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);
   }
 
 }
index 35a4fa5c9e050453125712c9e59036977e900133..190127ecca3cd19c0bbf0527001d045c785e8214 100644 (file)
@@ -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
index 226f7d4ecec0bcf96ec587d651928b36c8e45f84..6a26a07187193c362afbc0caf832e8eb91bb8779 100644 (file)
@@ -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<DuplicationsParser.Block> 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<DuplicationsParser.Block> 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<DuplicationsParser.Block> blocks = newArrayList();
 
index 1de5e771424b474bd15c566362c3c1d03843b4bf..bc0f6d85dc0566602e2e3c3d8fdf4e19a75ae4a9 100644 (file)
@@ -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<PropertyDto> 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<QualityGateConditionDto> 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<QualityGateConditionDto> 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<PropertyDto> 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<PropertyDto> 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);
index 7146f92cf012f5aa95fd630dba3881cfbe7fef5e..8e925d67ca0cd3eaa2cdd36e4328a935cd4ca38a 100644 (file)
@@ -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);
 
   /**