From f8ff155a7f1d23cf2eac254badd0495f798912e6 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 6 Dec 2017 13:17:22 +0100 Subject: [PATCH] SONAR-10134 Remove QualityGates usage from deselect action --- .../server/qualitygate/ws/DeselectAction.java | 34 +++--- .../qualitygate/ws/QualityGatesWsSupport.java | 11 ++ .../qualitygate/ws/DeselectActionTest.java | 100 ++++++++++-------- .../qualitygate/ws/QualityGatesWsTest.java | 5 +- 4 files changed, 91 insertions(+), 59 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/DeselectAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/DeselectAction.java index 771287b0061..94db5339965 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/DeselectAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/DeselectAction.java @@ -19,7 +19,7 @@ */ package org.sonar.server.qualitygate.ws; -import com.google.common.base.Optional; +import java.util.Optional; import javax.annotation.Nullable; import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; @@ -30,20 +30,20 @@ import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.ComponentDto; import org.sonar.server.component.ComponentFinder; -import org.sonar.server.qualitygate.QualityGates; -import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001; +import static org.sonar.server.qualitygate.QualityGateUpdater.SONAR_QUALITYGATE_PROPERTY; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_PROJECT_ID; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_PROJECT_KEY; +import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001; public class DeselectAction implements QualityGatesWsAction { - private final QualityGates qualityGates; private final DbClient dbClient; private final ComponentFinder componentFinder; + private final QualityGatesWsSupport wsSupport; - public DeselectAction(QualityGates qualityGates, DbClient dbClient, ComponentFinder componentFinder) { - this.qualityGates = qualityGates; + public DeselectAction(DbClient dbClient, ComponentFinder componentFinder, QualityGatesWsSupport wsSupport) { + this.wsSupport = wsSupport; this.dbClient = dbClient; this.componentFinder = componentFinder; } @@ -52,7 +52,11 @@ public class DeselectAction implements QualityGatesWsAction { public void define(WebService.NewController controller) { WebService.NewAction action = controller.createAction("deselect") .setDescription("Remove the association of a project from a quality gate.
" + - "Requires the 'Administer Quality Gates' permission.") + "Requires one of the following permissions:" + + "") .setPost(true) .setSince("4.3") .setHandler(this) @@ -73,26 +77,32 @@ public class DeselectAction implements QualityGatesWsAction { public void handle(Request request, Response response) { try (DbSession dbSession = dbClient.openSession(false)) { ComponentDto project = getProject(dbSession, request.param(PARAM_PROJECT_ID), request.param(PARAM_PROJECT_KEY)); - qualityGates.dissociateProject(dbSession, project); + dissociateProject(dbSession, project); response.noContent(); } } + private void dissociateProject(DbSession dbSession, ComponentDto project) { + wsSupport.checkCanAdminProject(project); + dbClient.propertiesDao().deleteProjectProperty(SONAR_QUALITYGATE_PROPERTY, project.getId(), dbSession); + dbSession.commit(); + } + private ComponentDto getProject(DbSession dbSession, @Nullable String projectId, @Nullable String projectKey) { return selectProjectById(dbSession, projectId) - .or(() -> componentFinder.getByUuidOrKey(dbSession, projectId, projectKey, ComponentFinder.ParamNames.PROJECT_ID_AND_KEY)); + .orElseGet(() -> componentFinder.getByUuidOrKey(dbSession, projectId, projectKey, ComponentFinder.ParamNames.PROJECT_ID_AND_KEY)); } private Optional selectProjectById(DbSession dbSession, @Nullable String projectId) { if (projectId == null) { - return Optional.absent(); + return Optional.empty(); } try { long dbId = Long.parseLong(projectId); - return dbClient.componentDao().selectById(dbSession, dbId); + return Optional.ofNullable(dbClient.componentDao().selectById(dbSession, dbId).orNull()); } catch (NumberFormatException e) { - return Optional.absent(); + return Optional.empty(); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWsSupport.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWsSupport.java index e6df35ca9cc..eee346dcc18 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWsSupport.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWsSupport.java @@ -27,6 +27,7 @@ import org.sonar.api.server.ws.WebService; import org.sonar.api.server.ws.WebService.NewAction; import org.sonar.db.DbClient; import org.sonar.db.DbSession; +import org.sonar.db.component.ComponentDto; import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualitygate.QGateWithOrgDto; import org.sonar.db.qualitygate.QualityGateConditionDto; @@ -36,8 +37,10 @@ import org.sonar.server.user.UserSession; import org.sonarqube.ws.Qualitygates; import static com.google.common.base.Preconditions.checkArgument; +import static org.sonar.api.web.UserRole.ADMIN; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_GATES; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_ORGANIZATION; +import static org.sonar.server.user.AbstractUserSession.insufficientPrivilegesException; import static org.sonar.server.ws.WsUtils.checkFound; import static org.sonar.server.ws.WsUtils.checkFoundWithOptional; @@ -107,6 +110,14 @@ public class QualityGatesWsSupport { userSession.checkPermission(ADMINISTER_QUALITY_GATES, qualityGate.getOrganizationUuid()); } + void checkCanAdminProject(ComponentDto project) { + if (userSession.hasPermission(ADMINISTER_QUALITY_GATES, project.getOrganizationUuid()) + || userSession.hasComponentPermission(ADMIN, project)) { + return; + } + throw insufficientPrivilegesException(); + } + private static void checkNotBuiltIn(QualityGateDto qualityGate) { checkArgument(!qualityGate.isBuiltIn(), "Operation forbidden for built-in Quality Gate '%s'", qualityGate.getName()); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/DeselectActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/DeselectActionTest.java index e9e25f06bc6..d55a7004337 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/DeselectActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/DeselectActionTest.java @@ -25,7 +25,6 @@ import org.junit.rules.ExpectedException; import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.WebService; import org.sonar.api.web.UserRole; -import org.sonar.core.util.UuidFactoryFast; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; @@ -37,7 +36,6 @@ import org.sonar.server.component.TestComponentFinder; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.organization.TestDefaultOrganizationProvider; -import org.sonar.server.qualitygate.QualityGates; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsActionTester; @@ -61,28 +59,22 @@ public class DeselectActionTest { private DbClient dbClient = db.getDbClient(); private DbSession dbSession = db.getSession(); private TestDefaultOrganizationProvider organizationProvider = TestDefaultOrganizationProvider.from(db); - private QualityGates qualityGates = new QualityGates(dbClient, userSession, organizationProvider, UuidFactoryFast.getInstance()); - - private DeselectAction underTest = new DeselectAction(qualityGates, dbClient, TestComponentFinder.from(db)); + private DeselectAction underTest = new DeselectAction(dbClient, TestComponentFinder.from(db), + new QualityGatesWsSupport(db.getDbClient(), userSession, organizationProvider)); private WsActionTester ws = new WsActionTester(underTest); @Test - public void deselect_by_id() { + public void deselect_by_key() { userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); ComponentDto project = db.components().insertPrivateProject(); - - ComponentDto anotherProject = db.components().insertPrivateProject(); - String gateId = valueOf(qualityGate.getId()); - associateProjectToQualityGate(project.getId(), gateId); - associateProjectToQualityGate(anotherProject.getId(), gateId); + associateProjectToQualityGate(project, qualityGate); ws.newRequest() - .setParam("projectId", valueOf(project.getId())) + .setParam("projectKey", project.getDbKey()) .execute(); assertDeselected(project.getId()); - assertSelected(gateId, anotherProject.getId()); } @Test @@ -90,9 +82,7 @@ public class DeselectActionTest { userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); ComponentDto project = db.components().insertPrivateProject(); - - String gateId = valueOf(qualityGate.getId()); - associateProjectToQualityGate(project.getId(), gateId); + associateProjectToQualityGate(project, qualityGate); ws.newRequest() .setParam("projectId", project.uuid()) @@ -102,27 +92,60 @@ public class DeselectActionTest { } @Test - public void deselect_by_key() { + public void deselect_by_id() { userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); ComponentDto project = db.components().insertPrivateProject(); + associateProjectToQualityGate(project, qualityGate); - String gateId = valueOf(qualityGate.getId()); - associateProjectToQualityGate(project.getId(), gateId); + ws.newRequest() + .setParam("projectId", valueOf(project.getId())) + .execute(); + + assertDeselected(project.getId()); + } + + @Test + public void project_admin() { + OrganizationDto organization = db.organizations().insert(); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); + ComponentDto project = db.components().insertPrivateProject(organization); + associateProjectToQualityGate(project, qualityGate); + userSession.logIn().addProjectPermission(UserRole.ADMIN, project); ws.newRequest() .setParam("projectKey", project.getDbKey()) + .setParam("organization", organization.getKey()) .execute(); assertDeselected(project.getId()); } @Test - public void project_admin() { + public void other_project_should_not_be_updated() { + userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); ComponentDto project = db.components().insertPrivateProject(); - associateProjectToQualityGate(project.getId(), valueOf(qualityGate.getId())); - userSession.logIn().addProjectPermission(UserRole.ADMIN, project); + String gateId = valueOf(qualityGate.getId()); + associateProjectToQualityGate(project, qualityGate); + // Another project + ComponentDto anotherProject = db.components().insertPrivateProject(); + associateProjectToQualityGate(anotherProject, qualityGate); + + ws.newRequest() + .setParam("projectKey", project.getKey()) + .execute(); + + assertDeselected(project.getId()); + assertSelected(gateId, anotherProject.getId()); + } + + @Test + public void default_organization_is_used_when_no_organization_parameter() { + userSession.addPermission(ADMINISTER_QUALITY_GATES, db.getDefaultOrganization()); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); + ComponentDto project = db.components().insertPrivateProject(); + associateProjectToQualityGate(project, qualityGate); ws.newRequest() .setParam("projectKey", project.getDbKey()) @@ -155,8 +178,8 @@ public class DeselectActionTest { @Test public void fail_when_anonymous() { - userSession.anonymous(); ComponentDto project = db.components().insertPrivateProject(); + userSession.anonymous(); expectedException.expect(ForbiddenException.class); ws.newRequest() @@ -192,8 +215,7 @@ public class DeselectActionTest { @Test public void fail_when_using_branch_db_key() throws Exception { - OrganizationDto organization = db.getDefaultOrganization(); - ComponentDto project = db.components().insertMainBranch(organization); + ComponentDto project = db.components().insertMainBranch(); userSession.logIn().addProjectPermission(UserRole.ADMIN, project); ComponentDto branch = db.components().insertProjectBranch(project); @@ -207,8 +229,7 @@ public class DeselectActionTest { @Test public void fail_when_using_branch_id() { - OrganizationDto organization = db.getDefaultOrganization(); - ComponentDto project = db.components().insertMainBranch(organization); + ComponentDto project = db.components().insertMainBranch(db.getDefaultOrganization()); userSession.logIn().addProjectPermission(UserRole.ADMIN, project); ComponentDto branch = db.components().insertProjectBranch(project); @@ -230,26 +251,17 @@ public class DeselectActionTest { assertThat(def.changelog()).extracting(Change::getVersion, Change::getDescription).containsExactly( tuple("6.6", "The parameter 'gateId' was removed")); - assertThat(def.params()).extracting(WebService.Param::key) - .containsExactlyInAnyOrder("projectId", "projectKey"); - - WebService.Param projectId = def.param("projectId"); - assertThat(projectId.isRequired()).isFalse(); - assertThat(projectId.deprecatedSince()).isEqualTo("6.1"); - assertThat(projectId.description()).isNotEmpty(); - assertThat(projectId.exampleValue()).isNotEmpty(); - - WebService.Param projectKey = def.param("projectKey"); - assertThat(projectKey.isRequired()).isFalse(); - assertThat(projectKey.since()).isEqualTo("6.1"); - assertThat(projectKey.description()).isNotEmpty(); - assertThat(projectKey.exampleValue()).isNotEmpty(); + assertThat(def.params()) + .extracting(WebService.Param::key, WebService.Param::isRequired) + .containsExactlyInAnyOrder( + tuple("projectKey", false), + tuple("projectId", false)); } - private void associateProjectToQualityGate(long projectId, String gateId) { + private void associateProjectToQualityGate(ComponentDto project, QualityGateDto qualityGate) { dbClient.propertiesDao().saveProperty(dbSession, new PropertyDto() - .setResourceId(projectId) - .setValue(gateId) + .setResourceId(project.getId()) + .setValue(qualityGate.getId().toString()) .setKey(SONAR_QUALITYGATE_PROPERTY)); db.commit(); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java index 937580d8189..6b8127ed5c6 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java @@ -71,8 +71,7 @@ public class QualityGatesWsTest { new SearchAction(projectFinder), new CreateAction(null, null, null, null), new SetAsDefaultAction(qGates), - selectAction, - new DeselectAction(qGates, mock(DbClient.class), mock(ComponentFinder.class)))); + selectAction)); } @Test @@ -81,7 +80,7 @@ public class QualityGatesWsTest { assertThat(controller).isNotNull(); assertThat(controller.path()).isEqualTo("api/qualitygates"); assertThat(controller.description()).isNotEmpty(); - assertThat(controller.actions()).hasSize(7); + assertThat(controller.actions()).hasSize(6); Action setDefault = controller.action("set_as_default"); assertThat(setDefault).isNotNull(); -- 2.39.5