From c51cc790961bddc08070346de50e2aaaee7484a4 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 23 Nov 2017 14:49:07 +0100 Subject: [PATCH] SONAR-10087 Move rename logic from QualityGates to ws --- .../server/qualitygate/QualityGates.java | 12 -- .../server/qualitygate/ws/QGateWsSupport.java | 4 + .../server/qualitygate/ws/RenameAction.java | 42 ++++- .../server/qualitygate/QualityGatesTest.java | 43 ----- .../qualitygate/ws/QualityGatesWsTest.java | 23 +-- .../qualitygate/ws/RenameActionTest.java | 172 ++++++++++++++++++ 6 files changed, 213 insertions(+), 83 deletions(-) create mode 100644 server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/RenameActionTest.java 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 7340d2364d4..941acb3ac30 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 @@ -69,18 +69,6 @@ public class QualityGates { this.organizationProvider = organizationProvider; } - public QualityGateDto rename(long idToRename, String name) { - checkIsQualityGateAdministrator(); - try (DbSession dbSession = dbClient.openSession(false)) { - QualityGateDto toRename = getNonNullQgate(idToRename); - validateQualityGate(dbSession, idToRename, name); - toRename.setName(name); - dao.update(toRename, dbSession); - dbSession.commit(); - return toRename; - } - } - public QualityGateDto copy(long sourceId, String destinationName) { checkIsQualityGateAdministrator(); getNonNullQgate(sourceId); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QGateWsSupport.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QGateWsSupport.java index 0a67641c680..854e9e0939d 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QGateWsSupport.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QGateWsSupport.java @@ -82,4 +82,8 @@ public class QGateWsSupport { return Long.valueOf(defaultQgate.getValue()); } + void checkCanEdit() { + userSession.checkPermission(ADMINISTER_QUALITY_GATES, defaultOrganizationProvider.get().getUuid()); + } + } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/RenameAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/RenameAction.java index 4421c21f000..deb2a084a86 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/RenameAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/RenameAction.java @@ -23,19 +23,28 @@ import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.text.JsonWriter; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; import org.sonar.db.qualitygate.QualityGateDto; -import org.sonar.server.qualitygate.QualityGates; +import org.sonar.server.qualitygate.QualityGateFinder; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Strings.isNullOrEmpty; import static org.sonar.server.qualitygate.ws.CreateAction.NAME_MAXIMUM_LENGTH; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_ID; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_NAME; +import static org.sonar.server.util.Validation.CANT_BE_EMPTY_MESSAGE; public class RenameAction implements QualityGatesWsAction { - private final QualityGates qualityGates; + private final DbClient dbClient; + private final QualityGateFinder qualityGateFinder; + private final QGateWsSupport wsSupport; - public RenameAction(QualityGates qualityGates) { - this.qualityGates = qualityGates; + public RenameAction(DbClient dbClient, QualityGateFinder qualityGateFinder, QGateWsSupport wsSupport) { + this.dbClient = dbClient; + this.qualityGateFinder = qualityGateFinder; + this.wsSupport = wsSupport; } @Override @@ -43,7 +52,7 @@ public class RenameAction implements QualityGatesWsAction { WebService.NewAction action = controller.createAction("rename") .setPost(true) .setDescription("Rename a Quality Gate.
" + - "Requires the 'Administer Quality Gates' permission.") + "Requires the 'Administer Quality Gates' permission.") .setSince("4.3") .setHandler(this); @@ -61,10 +70,29 @@ public class RenameAction implements QualityGatesWsAction { @Override public void handle(Request request, Response response) { - long idToRename = QualityGatesWs.parseId(request, PARAM_ID); - QualityGateDto renamedQualityGate = qualityGates.rename(idToRename, request.mandatoryParam(PARAM_NAME)); + wsSupport.checkCanEdit(); + long id = QualityGatesWs.parseId(request, PARAM_ID); + QualityGateDto renamedQualityGate = rename(id, request.mandatoryParam(PARAM_NAME)); JsonWriter writer = response.newJsonWriter(); QualityGatesWs.writeQualityGate(renamedQualityGate, writer).close(); } + private QualityGateDto rename(long id, String name) { + try (DbSession dbSession = dbClient.openSession(false)) { + QualityGateDto qualityGate = qualityGateFinder.getById(dbSession, id); + checkArgument(!isNullOrEmpty(name), CANT_BE_EMPTY_MESSAGE, "Name"); + checkNotAlreadyExists(dbSession, qualityGate, name); + qualityGate.setName(name); + dbClient.qualityGateDao().update(qualityGate, dbSession); + dbSession.commit(); + return qualityGate; + } + } + + private void checkNotAlreadyExists(DbSession dbSession, QualityGateDto qualityGate, String name) { + QualityGateDto existingQgate = dbClient.qualityGateDao().selectByName(dbSession, name); + boolean isModifyingCurrentQgate = existingQgate == null || existingQgate.getId().equals(qualityGate.getId()); + checkArgument(isModifyingCurrentQgate, "Name '%s' has already been taken", name); + } + } 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 f8f6b4729f9..dc1939f4751 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 @@ -39,8 +39,6 @@ import org.sonar.db.qualitygate.QualityGateConditionDao; import org.sonar.db.qualitygate.QualityGateConditionDto; import org.sonar.db.qualitygate.QualityGateDao; import org.sonar.db.qualitygate.QualityGateDto; -import org.sonar.server.exceptions.BadRequestException; -import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.tester.UserSessionRule; @@ -91,47 +89,6 @@ public class QualityGatesTest { userSession.logIn().addPermission(OrganizationPermission.ADMINISTER_QUALITY_GATES, organizationProvider.get().getUuid()); } - @Test - public void should_rename_qgate() { - long id = QUALITY_GATE_ID; - String name = "SG-1"; - QualityGateDto existing = new QualityGateDto().setId(id).setName("Golden"); - when(dao.selectById(dbSession, id)).thenReturn(existing); - QualityGateDto sg1 = underTest.rename(id, name); - assertThat(sg1.getName()).isEqualTo(name); - verify(dao).selectById(dbSession, id); - verify(dao).selectByName(dbSession, name); - verify(dao).update(sg1, dbSession); - } - - @Test - public void should_allow_rename_with_same_name() { - long id = QUALITY_GATE_ID; - String name = "SG-1"; - QualityGateDto existing = new QualityGateDto().setId(id).setName(name); - when(dao.selectById(dbSession, id)).thenReturn(existing); - QualityGateDto sg1 = underTest.rename(id, name); - assertThat(sg1.getName()).isEqualTo(name); - verify(dao).selectById(dbSession, id); - verify(dao).selectByName(dbSession, name); - verify(dao).update(sg1, dbSession); - } - - @Test(expected = NotFoundException.class) - public void should_fail_rename_on_inexistent_qgate() { - underTest.rename(QUALITY_GATE_ID, "Unknown"); - } - - @Test(expected = BadRequestException.class) - public void should_fail_rename_on_duplicate_name() { - long id = QUALITY_GATE_ID; - String name = "SG-1"; - QualityGateDto existing = new QualityGateDto().setId(id).setName("Golden"); - when(dao.selectById(dbSession, id)).thenReturn(existing); - when(dao.selectByName(dbSession, name)).thenReturn(new QualityGateDto().setId(666L).setName(name)); - underTest.rename(id, name); - } - @Test public void should_select_default_qgate() { long defaultId = QUALITY_GATE_ID; 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 13d2cada045..20284ab9edf 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 @@ -70,7 +70,7 @@ public class QualityGatesWsTest { new SearchAction(projectFinder), new CreateAction(null, null, null, null), new CopyAction(qGates), - new DestroyAction(qGates), new RenameAction(qGates), + new DestroyAction(qGates), new SetAsDefaultAction(qGates), new UnsetDefaultAction(qGates), new CreateConditionAction(null, null, null, null), new UpdateConditionAction(null, null, null, null), @@ -86,7 +86,7 @@ public class QualityGatesWsTest { assertThat(controller).isNotNull(); assertThat(controller.path()).isEqualTo("api/qualitygates"); assertThat(controller.description()).isNotEmpty(); - assertThat(controller.actions()).hasSize(13); + assertThat(controller.actions()).hasSize(12); Action create = controller.action("create"); assertThat(create).isNotNull(); @@ -113,15 +113,6 @@ public class QualityGatesWsTest { assertThat(destroy.param("id")).isNotNull(); assertThat(destroy.isInternal()).isFalse(); - Action rename = controller.action("rename"); - assertThat(rename).isNotNull(); - assertThat(rename.handler()).isNotNull(); - assertThat(rename.since()).isEqualTo("4.3"); - assertThat(rename.isPost()).isTrue(); - assertThat(rename.param("id")).isNotNull(); - assertThat(rename.param("name")).isNotNull(); - assertThat(rename.isInternal()).isFalse(); - Action setDefault = controller.action("set_as_default"); assertThat(setDefault).isNotNull(); assertThat(setDefault.handler()).isNotNull(); @@ -175,16 +166,6 @@ public class QualityGatesWsTest { .assertJson("{\"id\":42,\"name\":\"Copied QG\"}"); } - @Test - public void rename_nominal() throws Exception { - Long id = 42L; - String name = "New QG"; - when(qGates.rename(id, name)).thenReturn(new QualityGateDto().setId(id).setName(name)); - tester.newPostRequest("api/qualitygates", "rename").setParam("id", id.toString()).setParam("name", name).execute() - .assertJson("{\"id\":42,\"name\":\"New QG\"}"); - ; - } - @Test public void set_as_default_nominal() throws Exception { Long id = 42L; diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/RenameActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/RenameActionTest.java new file mode 100644 index 00000000000..59d733bb055 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/RenameActionTest.java @@ -0,0 +1,172 @@ +/* + * SonarQube + * Copyright (C) 2009-2017 SonarSource SA + * mailto:info AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.qualitygate.ws; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.server.ws.WebService; +import org.sonar.api.utils.System2; +import org.sonar.db.DbTester; +import org.sonar.db.qualitygate.QualityGateDto; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.organization.DefaultOrganizationProvider; +import org.sonar.server.organization.TestDefaultOrganizationProvider; +import org.sonar.server.qualitygate.QualityGateFinder; +import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.ws.WsActionTester; + +import static java.lang.String.format; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.AssertionsForClassTypes.tuple; +import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_GATES; +import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_PROFILES; +import static org.sonar.test.JsonAssert.assertJson; + +public class RenameActionTest { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + @Rule + public DbTester db = DbTester.create(System2.INSTANCE); + + private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); + + private WsActionTester ws = new WsActionTester( + new RenameAction(db.getDbClient(), new QualityGateFinder(db.getDbClient()), new QGateWsSupport(db.getDbClient(), userSession, defaultOrganizationProvider))); + + @Test + public void verify_definition() { + WebService.Action action = ws.getDef(); + assertThat(action.key()).isEqualTo("rename"); + assertThat(action.since()).isEqualTo("4.3"); + assertThat(action.changelog()).isEmpty(); + assertThat(action.params()) + .extracting(WebService.Param::key, WebService.Param::isRequired) + .containsExactlyInAnyOrder(tuple("id", true), tuple("name", true)); + } + + @Test + public void rename() { + logAsQualityGateAdminister(); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(qg -> qg.setName("old name")); + + ws.newRequest() + .setParam("id", qualityGate.getId().toString()) + .setParam("name", "new name") + .execute(); + + assertThat(db.getDbClient().qualityGateDao().selectById(db.getSession(), qualityGate.getId()).getName()).isEqualTo("new name"); + } + + @Test + public void response_contains_quality_gate() { + logAsQualityGateAdminister(); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(qg -> qg.setName("old name")); + + String result = ws.newRequest() + .setParam("id", qualityGate.getId().toString()) + .setParam("name", "new name") + .execute() + .getInput(); + + assertJson(result).isSimilarTo( + format("{\n" + + " \"id\": %s,\n" + + " \"name\": \"new name\"\n" + + "}", + qualityGate.getId())); + } + + @Test + public void rename_with_same_name() { + logAsQualityGateAdminister(); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(qg -> qg.setName("name")); + + ws.newRequest() + .setParam("id", qualityGate.getId().toString()) + .setParam("name", "name") + .execute(); + + assertThat(db.getDbClient().qualityGateDao().selectById(db.getSession(), qualityGate.getId()).getName()).isEqualTo("name"); + } + + @Test + public void fail_on_empty_name() { + logAsQualityGateAdminister(); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Name can't be empty"); + + ws.newRequest() + .setParam("id", qualityGate.getId().toString()) + .setParam("name", "") + .execute(); + } + + @Test + public void fail_when_using_existing_name() { + logAsQualityGateAdminister(); + QualityGateDto qualityGate1 = db.qualityGates().insertQualityGate(); + QualityGateDto qualityGate2 = db.qualityGates().insertQualityGate(); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage(format("Name '%s' has already been taken", qualityGate2.getName())); + + ws.newRequest() + .setParam("id", qualityGate1.getId().toString()) + .setParam("name", qualityGate2.getName()) + .execute(); + } + + @Test + public void fail_on_unknown_quality_gate() { + logAsQualityGateAdminister(); + + expectedException.expect(NotFoundException.class); + + ws.newRequest() + .setParam("id", "123") + .setParam("name", "new name") + .execute(); + } + + @Test + public void fail_when_not_quality_gates_administer() { + userSession.logIn("john").addPermission(ADMINISTER_QUALITY_PROFILES, defaultOrganizationProvider.get().getUuid()); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(qg -> qg.setName("old name")); + + expectedException.expect(ForbiddenException.class); + + ws.newRequest() + .setParam("id", qualityGate.getId().toString()) + .setParam("name", "new name") + .execute(); + } + + private void logAsQualityGateAdminister() { + userSession.logIn("john").addPermission(ADMINISTER_QUALITY_GATES, defaultOrganizationProvider.get().getUuid()); + } +} -- 2.39.5