From 4ef20684410e44321041865d473e3d6cf2f9ccd1 Mon Sep 17 00:00:00 2001 From: Michal Duda Date: Fri, 19 Feb 2021 14:29:04 +0100 Subject: [PATCH] SONAR-13848 remove deprecations from api/duplications/show --- .../server/duplication/ws/ShowAction.java | 23 +-- .../server/duplication/ws/ShowActionTest.java | 175 ++++++++---------- 2 files changed, 85 insertions(+), 113 deletions(-) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/duplication/ws/ShowAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/duplication/ws/ShowAction.java index 551ee1c60fb..a6b58143b63 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/duplication/ws/ShowAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/duplication/ws/ShowAction.java @@ -34,8 +34,6 @@ import org.sonar.db.measure.LiveMeasureDto; import org.sonar.server.component.ComponentFinder; import org.sonar.server.user.UserSession; -import static com.google.common.base.Preconditions.checkArgument; -import static org.sonar.server.component.ComponentFinder.ParamNames.UUID_AND_KEY; import static org.sonar.server.ws.KeyExamples.KEY_BRANCH_EXAMPLE_001; import static org.sonar.server.ws.KeyExamples.KEY_PULL_REQUEST_EXAMPLE_001; import static org.sonar.server.ws.WsUtils.writeProtobuf; @@ -43,7 +41,6 @@ import static org.sonar.server.ws.WsUtils.writeProtobuf; public class ShowAction implements DuplicationsWsAction { private static final String PARAM_KEY = "key"; - private static final String PARAM_UUID = "uuid"; private static final String PARAM_BRANCH = "branch"; private static final String PARAM_PULL_REQUEST = "pullRequest"; private final DbClient dbClient; @@ -69,19 +66,17 @@ public class ShowAction implements DuplicationsWsAction { .setResponseExample(getClass().getResource("show-example.json")); action.setChangelog( - new Change("6.5", "The fields 'uuid', 'projectUuid', 'subProjectUuid' are deprecated in the response.")); + new Change("8.8", "Deprecated parameter 'uuid' was removed."), + new Change("8.8", "The fields 'uuid', 'projectUuid', 'subProjectUuid' were removed from the response."), + new Change("6.5", "Parameter 'uuid' is now deprecated."), + new Change("6.5", "The fields 'uuid', 'projectUuid', 'subProjectUuid' are now deprecated in the response.")); action .createParam(PARAM_KEY) .setDescription("File key") + .setRequired(true) .setExampleValue("my_project:/src/foo/Bar.php"); - action - .createParam(PARAM_UUID) - .setDeprecatedSince("6.5") - .setDescription("File ID. If provided, 'key' must not be provided.") - .setExampleValue("584a89f2-8037-4f7b-b82c-8b45d2d63fb2"); - action .createParam(PARAM_BRANCH) .setDescription("Branch key") @@ -110,15 +105,13 @@ public class ShowAction implements DuplicationsWsAction { } private ComponentDto loadComponent(DbSession dbSession, Request request) { - String componentUuid = request.param(PARAM_UUID); + String key = request.mandatoryParam(PARAM_KEY); String branch = request.param(PARAM_BRANCH); String pullRequest = request.param(PARAM_PULL_REQUEST); - checkArgument(componentUuid == null || (branch == null && pullRequest == null), "Parameter '%s' cannot be used at the same time as '%s' or '%s'", PARAM_UUID, - PARAM_BRANCH, PARAM_PULL_REQUEST); if (branch == null && pullRequest == null) { - return componentFinder.getByUuidOrKey(dbSession, componentUuid, request.param(PARAM_KEY), UUID_AND_KEY); + return componentFinder.getByKey(dbSession, key); } - return componentFinder.getByKeyAndOptionalBranchOrPullRequest(dbSession, request.mandatoryParam(PARAM_KEY), branch, pullRequest); + return componentFinder.getByKeyAndOptionalBranchOrPullRequest(dbSession, key, branch, pullRequest); } @CheckForNull diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/duplication/ws/ShowActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/duplication/ws/ShowActionTest.java index 822d9696c6b..6bffda62139 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/duplication/ws/ShowActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/duplication/ws/ShowActionTest.java @@ -23,7 +23,6 @@ import java.util.function.Function; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.ExpectedException; import org.sonar.api.measures.CoreMetrics; import org.sonar.api.server.ws.WebService; import org.sonar.api.web.UserRole; @@ -43,6 +42,7 @@ 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.Assertions.assertThatThrownBy; import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.component.SnapshotTesting.newAnalysis; import static org.sonar.test.JsonAssert.assertJson; @@ -51,17 +51,15 @@ public class ShowActionTest { private static MetricDto dataMetric = MetricToDto.INSTANCE.apply(CoreMetrics.DUPLICATIONS_DATA); - @Rule - public ExpectedException expectedException = ExpectedException.none(); @Rule public UserSessionRule userSessionRule = UserSessionRule.standalone(); @Rule public DbTester db = DbTester.create(); - private DuplicationsParser parser = new DuplicationsParser(db.getDbClient().componentDao()); - private ShowResponseBuilder showResponseBuilder = new ShowResponseBuilder(db.getDbClient()); - - private WsActionTester ws = new WsActionTester(new ShowAction(db.getDbClient(), parser, showResponseBuilder, userSessionRule, TestComponentFinder.from(db))); + private final DuplicationsParser parser = new DuplicationsParser(db.getDbClient().componentDao()); + private final ShowResponseBuilder showResponseBuilder = new ShowResponseBuilder(db.getDbClient()); + private final WsActionTester ws = new WsActionTester(new ShowAction(db.getDbClient(), parser, showResponseBuilder, userSessionRule, + TestComponentFinder.from(db))); @Before public void setUp() { @@ -78,7 +76,7 @@ public class ShowActionTest { assertThat(show.since()).isEqualTo("4.4"); assertThat(show.isInternal()).isFalse(); assertThat(show.responseExampleAsString()).isNotEmpty(); - assertThat(show.params()).hasSize(4); + assertThat(show.params()).extracting(WebService.Param::key).contains("key", "branch", "pullRequest"); } @Test @@ -87,12 +85,6 @@ public class ShowActionTest { verifyCallToFileWithDuplications(file -> request.setParam("key", file.getDbKey())); } - @Test - public void get_duplications_by_file_id() { - TestRequest request = newBaseRequest(); - verifyCallToFileWithDuplications(file -> request.setParam("uuid", file.uuid())); - } - @Test public void return_file_with_missing_duplication_data() { ComponentDto project = db.components().insertPrivateProject(); @@ -130,34 +122,34 @@ public class ShowActionTest { assertJson(result).isSimilarTo( format("{\n" + - " \"duplications\": [\n" + - " {\n" + - " \"blocks\": [\n" + - " {\n" + - " \"from\": 20,\n" + - " \"size\": 5,\n" + - " \"_ref\": \"1\"\n" + - " },\n" + - " {\n" + - " \"from\": 31,\n" + - " \"size\": 5,\n" + - " \"_ref\": \"1\"\n" + - " }\n" + - " ]\n" + - " }\n" + - " ],\n" + - " \"files\": {\n" + - " \"1\": {\n" + - " \"key\": \"%s\",\n" + - " \"name\": \"%s\",\n" + - " \"uuid\": \"%s\",\n" + - " \"project\": \"%s\",\n" + - " \"projectUuid\": \"%s\",\n" + - " \"projectName\": \"%s\"\n" + - " \"branch\": \"%s\"\n" + - " }\n" + - " }\n" + - "}", + " \"duplications\": [\n" + + " {\n" + + " \"blocks\": [\n" + + " {\n" + + " \"from\": 20,\n" + + " \"size\": 5,\n" + + " \"_ref\": \"1\"\n" + + " },\n" + + " {\n" + + " \"from\": 31,\n" + + " \"size\": 5,\n" + + " \"_ref\": \"1\"\n" + + " }\n" + + " ]\n" + + " }\n" + + " ],\n" + + " \"files\": {\n" + + " \"1\": {\n" + + " \"key\": \"%s\",\n" + + " \"name\": \"%s\",\n" + + " \"uuid\": \"%s\",\n" + + " \"project\": \"%s\",\n" + + " \"projectUuid\": \"%s\",\n" + + " \"projectName\": \"%s\"\n" + + " \"branch\": \"%s\"\n" + + " }\n" + + " }\n" + + "}", file.getKey(), file.longName(), file.uuid(), branch.getKey(), branch.uuid(), project.longName(), file.getBranch())); } @@ -182,60 +174,62 @@ public class ShowActionTest { assertJson(result).isSimilarTo( format("{\n" + - " \"duplications\": [\n" + - " {\n" + - " \"blocks\": [\n" + - " {\n" + - " \"from\": 20,\n" + - " \"size\": 5,\n" + - " \"_ref\": \"1\"\n" + - " },\n" + - " {\n" + - " \"from\": 31,\n" + - " \"size\": 5,\n" + - " \"_ref\": \"1\"\n" + - " }\n" + - " ]\n" + - " }\n" + - " ],\n" + - " \"files\": {\n" + - " \"1\": {\n" + - " \"key\": \"%s\",\n" + - " \"name\": \"%s\",\n" + - " \"uuid\": \"%s\",\n" + - " \"project\": \"%s\",\n" + - " \"projectUuid\": \"%s\",\n" + - " \"projectName\": \"%s\"\n" + - " \"pullRequest\": \"%s\"\n" + - " }\n" + - " }\n" + - "}", + " \"duplications\": [\n" + + " {\n" + + " \"blocks\": [\n" + + " {\n" + + " \"from\": 20,\n" + + " \"size\": 5,\n" + + " \"_ref\": \"1\"\n" + + " },\n" + + " {\n" + + " \"from\": 31,\n" + + " \"size\": 5,\n" + + " \"_ref\": \"1\"\n" + + " }\n" + + " ]\n" + + " }\n" + + " ],\n" + + " \"files\": {\n" + + " \"1\": {\n" + + " \"key\": \"%s\",\n" + + " \"name\": \"%s\",\n" + + " \"uuid\": \"%s\",\n" + + " \"project\": \"%s\",\n" + + " \"projectUuid\": \"%s\",\n" + + " \"projectName\": \"%s\"\n" + + " \"pullRequest\": \"%s\"\n" + + " }\n" + + " }\n" + + "}", file.getKey(), file.longName(), file.uuid(), pullRequest.getKey(), pullRequest.uuid(), project.longName(), file.getPullRequest())); } @Test public void fail_if_file_does_not_exist() { - expectedException.expect(NotFoundException.class); + TestRequest request = newBaseRequest().setParam("key", "missing"); - newBaseRequest().setParam("key", "missing").execute(); + assertThatThrownBy(request::execute) + .isInstanceOf(NotFoundException.class); } @Test public void fail_if_user_is_not_allowed_to_access_project() { ComponentDto project = db.components().insertPrivateProject(); ComponentDto file = db.components().insertComponent(newFileDto(project)); + TestRequest request = newBaseRequest().setParam("key", file.getDbKey()); - expectedException.expect(ForbiddenException.class); - - newBaseRequest().setParam("key", file.getDbKey()).execute(); + assertThatThrownBy(request::execute) + .isInstanceOf(ForbiddenException.class); } @Test public void fail_if_no_parameter_provided() { - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Either 'uuid' or 'key' must be provided"); + TestRequest request = newBaseRequest(); - newBaseRequest().execute(); + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("The 'key' parameter is missing"); } @Test @@ -243,27 +237,12 @@ public class ShowActionTest { ComponentDto project = db.components().insertPrivateProject(); userSessionRule.addProjectPermission(UserRole.CODEVIEWER, project); ComponentDto branch = db.components().insertProjectBranch(project); + TestRequest request = ws.newRequest() + .setParam("key", branch.getDbKey()); - expectedException.expect(NotFoundException.class); - expectedException.expectMessage(format("Component key '%s' not found", branch.getDbKey())); - - ws.newRequest() - .setParam("key", branch.getDbKey()) - .execute(); - } - - @Test - public void fail_when_using_branch_uuid() { - ComponentDto project = db.components().insertPrivateProject(); - userSessionRule.addProjectPermission(UserRole.CODEVIEWER, project); - ComponentDto branch = db.components().insertProjectBranch(project); - - expectedException.expect(NotFoundException.class); - expectedException.expectMessage(format("Component id '%s' not found", branch.uuid())); - - ws.newRequest() - .setParam("uuid", branch.uuid()) - .execute(); + assertThatThrownBy(request::execute) + .isInstanceOf(NotFoundException.class) + .hasMessage(format("Component key '%s' not found", branch.getDbKey())); } private TestRequest newBaseRequest() { -- 2.39.5