From 251930ec8326956a505bfe2ef6863c5a423fbe14 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 14 Dec 2016 19:28:38 +0100 Subject: [PATCH] SONAR-7293 Replace Ruby WS api/issues/delete_comment by Java WS --- .../test/java/it/issue/IssueActionTest.java | 11 ++ .../issue/InternalRubyIssueService.java | 8 - .../server/issue/IssueCommentService.java | 59 ------- .../server/issue/ws/DeleteCommentAction.java | 123 +++++++++++++ .../sonar/server/issue/ws/IssueWsModule.java | 3 +- .../org/sonar/server/issue/ws/IssuesWs.java | 14 -- .../issue/InternalRubyIssueServiceTest.java | 3 +- .../server/issue/IssueCommentServiceTest.java | 106 ----------- .../issue/ws/DeleteCommentActionTest.java | 167 ++++++++++++++++++ .../app/controllers/api/issues_controller.rb | 17 -- .../org/sonar/db/issue/IssueChangeDao.java | 16 +- .../sonar/db/issue/IssueChangeDaoTest.java | 5 +- .../ws/client/issue/IssuesService.java | 11 +- .../ws/client/issue/IssuesWsParameters.java | 1 + .../ws/client/issue/IssuesServiceTest.java | 11 ++ 15 files changed, 332 insertions(+), 223 deletions(-) delete mode 100644 server/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java create mode 100644 server/sonar-server/src/main/java/org/sonar/server/issue/ws/DeleteCommentAction.java delete mode 100644 server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceTest.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/issue/ws/DeleteCommentActionTest.java diff --git a/it/it-tests/src/test/java/it/issue/IssueActionTest.java b/it/it-tests/src/test/java/it/issue/IssueActionTest.java index 86b70342e85..0932a4ceb7f 100644 --- a/it/it-tests/src/test/java/it/issue/IssueActionTest.java +++ b/it/it-tests/src/test/java/it/issue/IssueActionTest.java @@ -116,6 +116,17 @@ public class IssueActionTest extends AbstractIssueTest { assertThat(reloaded.getComments().getComments(0).getHtmlText()).isEqualTo("new comment"); } + @Test + public void delete_comment() throws Exception { + Issues.Comment comment = issuesService.addComment(new AddCommentRequest(randomIssue.getKey(), "this is my *comment*")).getIssue().getComments().getComments(0); + Issue issue = issuesService.deleteComment(comment.getKey()).getIssue(); + assertThat(issue.getComments().getCommentsList()).isEmpty(); + + // reload issue + Issue reloaded = issueRule.getByKey(randomIssue.getKey()); + assertThat(reloaded.getComments().getCommentsList()).isEmpty(); + } + /** * SONAR-4352 */ diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java index 3ff07370d8f..9d29797020a 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/InternalRubyIssueService.java @@ -21,7 +21,6 @@ package org.sonar.server.issue; import com.google.common.annotations.VisibleForTesting; import java.util.Map; -import org.sonar.api.issue.IssueComment; import org.sonar.api.server.ServerSide; import org.sonar.server.es.SearchOptions; import org.sonar.server.user.UserSession; @@ -38,23 +37,16 @@ import org.sonarqube.ws.client.issue.IssuesWsParameters; @ServerSide public class InternalRubyIssueService { - private final IssueCommentService commentService; private final IssueBulkChangeService issueBulkChangeService; private final UserSession userSession; public InternalRubyIssueService( - IssueCommentService commentService, IssueBulkChangeService issueBulkChangeService, UserSession userSession) { - this.commentService = commentService; this.issueBulkChangeService = issueBulkChangeService; this.userSession = userSession; } - public IssueComment deleteComment(String commentKey) { - return commentService.deleteComment(commentKey); - } - /** * Execute a bulk change */ diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java deleted file mode 100644 index 7916b81878d..00000000000 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueCommentService.java +++ /dev/null @@ -1,59 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2016 SonarSource SA - * mailto:contact 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.issue; - -import com.google.common.base.Strings; -import java.util.Objects; -import org.sonar.api.issue.IssueComment; -import org.sonar.core.issue.DefaultIssueComment; -import org.sonar.db.DbClient; -import org.sonar.server.exceptions.ForbiddenException; -import org.sonar.server.exceptions.NotFoundException; -import org.sonar.server.user.UserSession; - -public class IssueCommentService { - - private final DbClient dbClient; - private final IssueService issueService; - private final UserSession userSession; - - public IssueCommentService(DbClient dbClient, IssueService issueService, UserSession userSession) { - this.dbClient = dbClient; - this.issueService = issueService; - this.userSession = userSession; - } - - public IssueComment deleteComment(String commentKey) { - DefaultIssueComment comment = dbClient.issueChangeDao().selectDefaultCommentByKey(commentKey); - if (comment == null) { - throw new NotFoundException("Comment not found: " + commentKey); - } - if (Strings.isNullOrEmpty(comment.userLogin()) || !Objects.equals(comment.userLogin(), userSession.getLogin())) { - throw new ForbiddenException("You can only delete your own comments"); - } - - // check authorization - issueService.getByKey(comment.issueKey()); - - dbClient.issueChangeDao().delete(commentKey); - return comment; - } - -} diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DeleteCommentAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DeleteCommentAction.java new file mode 100644 index 00000000000..fd8ba6c6d35 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/DeleteCommentAction.java @@ -0,0 +1,123 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact 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.issue.ws; + +import java.util.Objects; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.stream.Stream; +import org.sonar.api.server.ws.Request; +import org.sonar.api.server.ws.Response; +import org.sonar.api.server.ws.WebService; +import org.sonar.core.util.stream.Collectors; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.db.issue.IssueChangeDto; +import org.sonar.db.issue.IssueDto; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.issue.IssueFinder; +import org.sonar.server.user.UserSession; + +import static com.google.common.base.Preconditions.checkArgument; +import static java.lang.String.format; +import static org.sonar.core.util.Uuids.UUID_EXAMPLE_01; +import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_DELETE_COMMENT; +import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_COMMENT; + +public class DeleteCommentAction implements IssuesWsAction { + + private final UserSession userSession; + private final DbClient dbClient; + private final IssueFinder issueFinder; + private final OperationResponseWriter responseWriter; + + public DeleteCommentAction(UserSession userSession, DbClient dbClient, IssueFinder issueFinder, OperationResponseWriter responseWriter) { + this.userSession = userSession; + this.dbClient = dbClient; + this.issueFinder = issueFinder; + this.responseWriter = responseWriter; + } + + @Override + public void define(WebService.NewController context) { + WebService.NewAction action = context.createAction(ACTION_DELETE_COMMENT) + .setDescription("Delete a comment.
" + + "Requires authentication and the following permission: 'Browse' on the project of the specified issue.
" + + "Since 6.3, the response contains the issue with all details, not the removed comment.
" + + "Since 6.3, 'key' parameter has been renamed to %s", PARAM_COMMENT) + .setSince("3.6") + .setHandler(this) + .setPost(true); + + action.createParam(PARAM_COMMENT) + .setDescription("Comment key") + .setDeprecatedKey("key") + .setSince("6.3") + .setRequired(true) + .setExampleValue(UUID_EXAMPLE_01); + } + + @Override + public void handle(Request request, Response response) { + userSession.checkLoggedIn(); + try (DbSession dbSession = dbClient.openSession(false)) { + IssueDto issueDto = Stream.of(request) + .map(loadCommentData(dbSession)) + .peek(deleteComment(dbSession)) + .collect(Collectors.toOneElement()) + .getIssueDto(); + responseWriter.write(issueDto.getKey(), request, response); + } + } + + private Function loadCommentData(DbSession dbSession) { + return request -> new CommentData(dbSession, request.mandatoryParam(PARAM_COMMENT)); + } + + private Consumer deleteComment(DbSession dbSession) { + return commentData -> { + dbClient.issueChangeDao().delete(dbSession, commentData.getIssueChangeDto().getKey()); + dbSession.commit(); + }; + } + + private class CommentData { + private final IssueChangeDto issueChangeDto; + private final IssueDto issueDto; + + CommentData(DbSession dbSession, String commentKey) { + this.issueChangeDto = dbClient.issueChangeDao().selectCommentByKey(dbSession, commentKey) + .orElseThrow(() -> new NotFoundException(format("Comment with key '%s' does not exist", commentKey))); + // Load issue now to quickly fail if user hasn't permission to see it + this.issueDto = issueFinder.getByKey(dbSession, issueChangeDto.getIssueKey()); + checkArgument(Objects.equals(issueChangeDto.getUserLogin(), userSession.getLogin()), "You can only delete your own comments"); + } + + IssueChangeDto getIssueChangeDto() { + return issueChangeDto; + } + + IssueDto getIssueDto() { + return issueDto; + } + } + +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java index 41a45c45eac..dce951ef939 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssueWsModule.java @@ -23,7 +23,6 @@ import org.sonar.core.platform.Module; import org.sonar.server.issue.ActionFinder; import org.sonar.server.issue.InternalRubyIssueService; import org.sonar.server.issue.IssueBulkChangeService; -import org.sonar.server.issue.IssueCommentService; import org.sonar.server.issue.IssueFieldsSetter; import org.sonar.server.issue.IssueFinder; import org.sonar.server.issue.IssueQueryService; @@ -47,7 +46,6 @@ public class IssueWsModule extends Module { IssueFieldsSetter.class, FunctionExecutor.class, IssueWorkflow.class, - IssueCommentService.class, InternalRubyIssueService.class, IssueBulkChangeService.class, IssueService.class, @@ -59,6 +57,7 @@ public class IssueWsModule extends Module { WsResponseCommonFormat.class, AddCommentAction.class, EditCommentAction.class, + DeleteCommentAction.class, AssignAction.class, DoTransitionAction.class, SearchAction.class, diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java index e2930adfb43..354e1424144 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/IssuesWs.java @@ -52,23 +52,9 @@ public class IssuesWs implements WebService { } private static void defineRailsActions(NewController controller) { - defineDeleteCommentAction(controller); defineBulkChangeAction(controller); } - private static void defineDeleteCommentAction(NewController controller) { - WebService.NewAction action = controller.createAction(DELETE_COMMENT_ACTION) - .setDescription("Delete a comment. Requires authentication and Browse permission on project") - .setSince("3.6") - .setHandler(RailsHandler.INSTANCE) - .setPost(true); - - action.createParam("key") - .setDescription("Key of the comment") - .setRequired(true) - .setExampleValue("392160d3-a4f2-4c52-a565-e4542cfa2096"); - } - private static void defineBulkChangeAction(NewController controller) { WebService.NewAction action = controller.createAction(BULK_CHANGE_ACTION) .setDescription("Bulk change on issues. Requires authentication and User role on project(s)") diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java index 543aa461e48..6b5ed00f5a6 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/InternalRubyIssueServiceTest.java @@ -39,10 +39,9 @@ public class InternalRubyIssueServiceTest { @Rule public UserSessionRule userSessionRule = UserSessionRule.standalone(); - IssueCommentService commentService = mock(IssueCommentService.class); IssueBulkChangeService issueBulkChangeService = mock(IssueBulkChangeService.class); - InternalRubyIssueService underTest = new InternalRubyIssueService(commentService, issueBulkChangeService, userSessionRule); + InternalRubyIssueService underTest = new InternalRubyIssueService(issueBulkChangeService, userSessionRule); @Test public void execute_bulk_change() { diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceTest.java deleted file mode 100644 index 189f3b7723c..00000000000 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueCommentServiceTest.java +++ /dev/null @@ -1,106 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2016 SonarSource SA - * mailto:contact 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.issue; - -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.ExpectedException; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; -import org.sonar.core.issue.DefaultIssueComment; -import org.sonar.core.permission.GlobalPermissions; -import org.sonar.db.DbClient; -import org.sonar.db.DbSession; -import org.sonar.db.issue.IssueChangeDao; -import org.sonar.server.exceptions.ForbiddenException; -import org.sonar.server.exceptions.NotFoundException; -import org.sonar.server.tester.UserSessionRule; - -import static org.mockito.Mockito.anyString; -import static org.mockito.Mockito.never; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -@RunWith(MockitoJUnitRunner.class) -public class IssueCommentServiceTest { - - @Mock - private DbClient dbClient; - - @Mock - private DbSession session; - - @Mock - private IssueService issueService; - - @Mock - private IssueChangeDao changeDao; - - @Mock - private IssueCommentService issueCommentService; - - @Rule - public ExpectedException throwable = ExpectedException.none(); - @Rule - public UserSessionRule userSessionRule = UserSessionRule.standalone().login("admin").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); - - @Before - public void setUp() { - when(dbClient.openSession(false)).thenReturn(session); - when(dbClient.issueChangeDao()).thenReturn(changeDao); - - issueCommentService = new IssueCommentService(dbClient, issueService, userSessionRule); - } - - @Test - public void should_delete_comment() { - when(changeDao.selectDefaultCommentByKey("ABCD")).thenReturn(new DefaultIssueComment().setUserLogin("admin").setIssueKey("EFGH")); - - issueCommentService.deleteComment("ABCD"); - - verify(changeDao).delete("ABCD"); - verify(issueService).getByKey("EFGH"); - } - - @Test - public void should_not_delete_not_found_comment() { - throwable.expect(NotFoundException.class); - - when(changeDao.selectDefaultCommentByKey("ABCD")).thenReturn(null); - - issueCommentService.deleteComment("ABCD"); - - verify(changeDao, never()).delete(anyString()); - } - - @Test - public void should_prevent_delete_others_comment() { - throwable.expect(ForbiddenException.class); - - when(changeDao.selectDefaultCommentByKey("ABCD")).thenReturn(new DefaultIssueComment().setUserLogin("julien")); - - issueCommentService.deleteComment("ABCD"); - - verify(changeDao, never()).delete(anyString()); - } - -} diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DeleteCommentActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DeleteCommentActionTest.java new file mode 100644 index 00000000000..592386656d0 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/DeleteCommentActionTest.java @@ -0,0 +1,167 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact 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.issue.ws; + +import javax.annotation.Nullable; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.sonar.api.server.ws.Request; +import org.sonar.api.server.ws.Response; +import org.sonar.api.server.ws.WebService; +import org.sonar.db.DbClient; +import org.sonar.db.DbTester; +import org.sonar.db.issue.IssueChangeDto; +import org.sonar.db.issue.IssueDbTester; +import org.sonar.db.issue.IssueDto; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.issue.IssueFinder; +import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.ws.TestRequest; +import org.sonar.server.ws.TestResponse; +import org.sonar.server.ws.WsActionTester; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.sonar.api.web.UserRole.CODEVIEWER; +import static org.sonar.api.web.UserRole.USER; +import static org.sonar.core.util.Protobuf.setNullable; + +public class DeleteCommentActionTest { + + @Rule + public ExpectedException expectedException = ExpectedException.none(); + + @Rule + public DbTester dbTester = DbTester.create(); + + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); + + private DbClient dbClient = dbTester.getDbClient(); + + private IssueDbTester issueDbTester = new IssueDbTester(dbTester); + + private OperationResponseWriter responseWriter = mock(OperationResponseWriter.class); + + private WsActionTester tester = new WsActionTester( + new DeleteCommentAction(userSession, dbClient, new IssueFinder(dbClient, userSession), responseWriter)); + + @Test + public void delete_comment() throws Exception { + IssueDto issueDto = issueDbTester.insertIssue(); + IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, "john", "please fix it"); + userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid()); + + call(commentDto.getKey()); + + verify(responseWriter).write(eq(issueDto.getKey()), any(Request.class), any(Response.class)); + assertThat(dbClient.issueChangeDao().selectCommentByKey(dbTester.getSession(), commentDto.getKey())).isNotPresent(); + } + + @Test + public void delete_comment_using_deprecated_key_parameter() throws Exception { + IssueDto issueDto = issueDbTester.insertIssue(); + IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, "john", "please fix it"); + userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid()); + + tester.newRequest().setParam("key", commentDto.getKey()).setParam("text", "please have a look").execute(); + + verify(responseWriter).write(eq(issueDto.getKey()), any(Request.class), any(Response.class)); + assertThat(dbClient.issueChangeDao().selectCommentByKey(dbTester.getSession(), commentDto.getKey())).isNotPresent(); + } + + @Test + public void fail_when_comment_does_not_belong_to_current_user() throws Exception { + IssueDto issueDto = issueDbTester.insertIssue(); + IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, "john", "please fix it"); + userSession.login("another").addProjectUuidPermissions(USER, issueDto.getProjectUuid()); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("You can only delete your own comments"); + call(commentDto.getKey()); + } + + @Test + public void fail_when_comment_has_not_user() throws Exception { + IssueDto issueDto = issueDbTester.insertIssue(); + IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, null, "please fix it"); + userSession.login("john").addProjectUuidPermissions(USER, issueDto.getProjectUuid()); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("You can only delete your own comments"); + call(commentDto.getKey()); + } + + @Test + public void fail_when_missing_comment_key() throws Exception { + userSession.login("john"); + + expectedException.expect(IllegalArgumentException.class); + call(null); + } + + @Test + public void fail_when_comment_does_not_exist() throws Exception { + userSession.login("john"); + + expectedException.expect(NotFoundException.class); + call("ABCD"); + } + + @Test + public void fail_when_not_authenticated() throws Exception { + expectedException.expect(UnauthorizedException.class); + call("ABCD"); + } + + @Test + public void fail_when_not_enough_permission() throws Exception { + IssueDto issueDto = issueDbTester.insertIssue(); + IssueChangeDto commentDto = issueDbTester.insertComment(issueDto, "john", "please fix it"); + userSession.login("john").addProjectUuidPermissions(CODEVIEWER, issueDto.getProjectUuid()); + + expectedException.expect(ForbiddenException.class); + call(commentDto.getKey()); + } + + @Test + public void test_definition() { + WebService.Action action = tester.getDef(); + assertThat(action.key()).isEqualTo("delete_comment"); + assertThat(action.isPost()).isTrue(); + assertThat(action.isInternal()).isFalse(); + assertThat(action.params()).hasSize(1); + assertThat(action.responseExample()).isNull(); + } + + private TestResponse call(@Nullable String commentKey) { + TestRequest request = tester.newRequest(); + setNullable(commentKey, comment -> request.setParam("comment", comment)); + return request.execute(); + } + +} diff --git a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb index 2416697a6b8..0ede211a63a 100644 --- a/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb +++ b/server/sonar-web/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb @@ -21,23 +21,6 @@ # since 3.6 class Api::IssuesController < Api::ApiController - # - # POST /api/issues/delete_comment?key= - # - # -- Mandatory parameters - # 'key' is the comment key - # - # -- Example - # curl -X POST -v -u admin:admin 'http://localhost:9000/api/issues/delete_comment?key=392160d3-a4f2-4c52-a565-e4542cfa2096' - # - def delete_comment - verify_post_request - require_parameters :key - - comment = Internal.issues.deleteComment(params[:key]) - render :json => jsonp({:comment => Issue.comment_to_hash(comment)}) - end - # # Execute a bulk change on a list of issues # diff --git a/sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDao.java b/sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDao.java index ff7fcaa53fa..468e4bd7cfd 100644 --- a/sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDao.java +++ b/sonar-db/src/main/java/org/sonar/db/issue/IssueChangeDao.java @@ -93,17 +93,11 @@ public class IssueChangeDao implements Dao { mapper(session).insert(change); } - public boolean delete(String key) { - DbSession session = mybatis.openSession(false); - try { - IssueChangeMapper mapper = mapper(session); - int count = mapper.delete(key); - session.commit(); - return count == 1; - - } finally { - MyBatis.closeQuietly(session); - } + public boolean delete(DbSession session, String key) { + IssueChangeMapper mapper = mapper(session); + int count = mapper.delete(key); + session.commit(); + return count == 1; } public boolean update(DbSession dbSession, IssueChangeDto change) { diff --git a/sonar-db/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java b/sonar-db/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java index 7f9349356d6..fbc29109f82 100644 --- a/sonar-db/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java +++ b/sonar-db/src/test/java/org/sonar/db/issue/IssueChangeDaoTest.java @@ -137,7 +137,8 @@ public class IssueChangeDaoTest { public void delete() { db.prepareDbUnit(getClass(), "delete.xml"); - assertThat(underTest.delete("COMMENT-2")).isTrue(); + assertThat(underTest.delete(db.getSession(), "COMMENT-2")).isTrue(); + db.commit(); db.assertDbUnit(getClass(), "delete-result.xml", "issue_changes"); } @@ -146,7 +147,7 @@ public class IssueChangeDaoTest { public void delete_unknown_key() { db.prepareDbUnit(getClass(), "delete.xml"); - assertThat(underTest.delete("UNKNOWN")).isFalse(); + assertThat(underTest.delete(db.getSession(), "UNKNOWN")).isFalse(); } @Test diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesService.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesService.java index 7bd9b0f8d6f..c589cb5e9bf 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesService.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesService.java @@ -34,6 +34,7 @@ import static org.sonar.api.server.ws.WebService.Param.SORT; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_ADD_COMMENT; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_ASSIGN; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_CHANGELOG; +import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_DELETE_COMMENT; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_DO_TRANSITION; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_EDIT_COMMENT; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_SEARCH; @@ -114,10 +115,16 @@ public class IssuesService extends BaseService { Issues.Operation.parser()); } + public Issues.Operation deleteComment(String commentKey) { + return call(new PostRequest(path(ACTION_DELETE_COMMENT)) + .setParam(PARAM_COMMENT, commentKey), + Issues.Operation.parser()); + } + public Issues.Operation editComment(EditCommentRequest request) { return call(new PostRequest(path(ACTION_EDIT_COMMENT)) - .setParam(PARAM_COMMENT, request.getComment()) - .setParam(PARAM_TEXT, request.getText()), + .setParam(PARAM_COMMENT, request.getComment()) + .setParam(PARAM_TEXT, request.getText()), Issues.Operation.parser()); } diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesWsParameters.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesWsParameters.java index bc20871b002..0ef2de55f06 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesWsParameters.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesWsParameters.java @@ -33,6 +33,7 @@ public class IssuesWsParameters { public static final String ACTION_CHANGELOG = "changelog"; public static final String ACTION_ADD_COMMENT = "add_comment"; public static final String ACTION_EDIT_COMMENT = "edit_comment"; + public static final String ACTION_DELETE_COMMENT = "delete_comment"; public static final String ACTION_ASSIGN = "assign"; public static final String ACTION_AUTHORS = "authors"; public static final String ACTION_DO_TRANSITION = "do_transition"; diff --git a/sonar-ws/src/test/java/org/sonarqube/ws/client/issue/IssuesServiceTest.java b/sonar-ws/src/test/java/org/sonarqube/ws/client/issue/IssuesServiceTest.java index 34416229f5c..eea04ace36f 100644 --- a/sonar-ws/src/test/java/org/sonarqube/ws/client/issue/IssuesServiceTest.java +++ b/sonar-ws/src/test/java/org/sonarqube/ws/client/issue/IssuesServiceTest.java @@ -85,6 +85,17 @@ public class IssuesServiceTest { .andNoOtherParam(); } + @Test + public void delete_comment() { + underTest.deleteComment("ABCD"); + PostRequest request = serviceTester.getPostRequest(); + + assertThat(serviceTester.getPostParser()).isSameAs(Issues.Operation.parser()); + serviceTester.assertThat(request) + .hasParam("comment", "ABCD") + .andNoOtherParam(); + } + @Test public void edit_comment() { underTest.editComment(new EditCommentRequest("ABCD", "Please help me to fix this issue")); -- 2.39.5