From 5a8df5bddc6d770220742ed0edd3604060b13524 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 20 Apr 2017 14:50:47 +0200 Subject: [PATCH] SONAR-9120 api/issues/set_tags response contains issue information --- .../org/sonar/server/issue/IssueService.java | 37 +-- .../sonar/server/issue/ws/SetTagsAction.java | 51 ++-- .../server/issue/IssueServiceMediumTest.java | 36 --- .../server/issue/ws/SetTagsActionTest.java | 221 +++++++++++++++--- .../server/issue/ws/SetTypeActionTest.java | 1 - 5 files changed, 227 insertions(+), 119 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java index dc29fbec856..b38f7f948a5 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java @@ -19,39 +19,21 @@ */ package org.sonar.server.issue; -import java.util.Collection; -import java.util.Date; import java.util.List; import java.util.Map; import javax.annotation.Nullable; import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.server.ServerSide; -import org.sonar.core.issue.DefaultIssue; -import org.sonar.core.issue.IssueChangeContext; -import org.sonar.db.DbClient; -import org.sonar.db.DbSession; import org.sonar.server.issue.index.IssueIndex; -import org.sonar.server.user.UserSession; @ServerSide @ComputeEngineSide public class IssueService { - private final DbClient dbClient; private final IssueIndex issueIndex; - private final IssueFinder issueFinder; - private final IssueFieldsSetter issueFieldsSetter; - private final IssueUpdater issueUpdater; - private final UserSession userSession; - - public IssueService(DbClient dbClient, IssueIndex issueIndex, IssueFinder issueFinder, IssueFieldsSetter issueFieldsSetter, IssueUpdater issueUpdater, UserSession userSession) { - this.dbClient = dbClient; + public IssueService(IssueIndex issueIndex) { this.issueIndex = issueIndex; - this.issueFinder = issueFinder; - this.issueFieldsSetter = issueFieldsSetter; - this.issueUpdater = issueUpdater; - this.userSession = userSession; } public List listAuthors(@Nullable String textQuery, int pageSize) { @@ -61,23 +43,6 @@ public class IssueService { return issueIndex.listAuthors(query, textQuery, pageSize); } - public Collection setTags(String issueKey, Collection tags) { - userSession.checkLoggedIn(); - - DbSession session = dbClient.openSession(false); - try { - DefaultIssue issue = issueFinder.getByKey(session, issueKey).toDefaultIssue(); - IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.getLogin()); - if (issueFieldsSetter.setTags(issue, tags, context)) { - issueUpdater.saveIssue(session, issue, context, null); - } - return issue.tags(); - - } finally { - session.close(); - } - } - public Map listTagsForComponent(IssueQuery query, int pageSize) { return issueIndex.countTags(query, pageSize); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SetTagsAction.java b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SetTagsAction.java index 79250c1c12b..ed9c43e948f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SetTagsAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/issue/ws/SetTagsAction.java @@ -21,16 +21,23 @@ package org.sonar.server.issue.ws; import com.google.common.base.MoreObjects; import com.google.common.io.Resources; -import java.util.Collection; import java.util.Collections; +import java.util.Date; import java.util.List; +import org.sonar.api.server.ws.Change; 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.server.ws.WebService.NewAction; -import org.sonar.api.utils.text.JsonWriter; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.util.Uuids; -import org.sonar.server.issue.IssueService; +import org.sonar.db.DbClient; +import org.sonar.db.DbSession; +import org.sonar.server.issue.IssueFieldsSetter; +import org.sonar.server.issue.IssueFinder; +import org.sonar.server.issue.IssueUpdater; +import org.sonar.server.user.UserSession; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_SET_TAGS; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ISSUE; @@ -41,10 +48,21 @@ import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_TAGS; */ public class SetTagsAction implements IssuesWsAction { - private final IssueService service; + private final UserSession userSession; + private final DbClient dbClient; + private final IssueFinder issueFinder; + private final IssueFieldsSetter issueFieldsSetter; + private final IssueUpdater issueUpdater; + private final OperationResponseWriter responseWriter; - public SetTagsAction(IssueService service) { - this.service = service; + public SetTagsAction(UserSession userSession, DbClient dbClient, IssueFinder issueFinder, IssueFieldsSetter issueFieldsSetter, IssueUpdater issueUpdater, + OperationResponseWriter responseWriter) { + this.userSession = userSession; + this.dbClient = dbClient; + this.issueFinder = issueFinder; + this.issueFieldsSetter = issueFieldsSetter; + this.issueUpdater = issueUpdater; + this.responseWriter = responseWriter; } @Override @@ -53,8 +71,8 @@ public class SetTagsAction implements IssuesWsAction { .setPost(true) .setSince("5.1") .setDescription("Set tags on an issue.
" + - "Requires authentication and Browse permission on project
" + - "Since 6.3, the parameter 'key' has been replaced by '%s'", PARAM_ISSUE) + "Requires authentication and Browse permission on project") + .setChangelog(new Change("6.4", "response contains issue information instead of list of tags")) .setResponseExample(Resources.getResource(this.getClass(), "set_tags-example.json")) .setHandler(this); action.createParam(PARAM_ISSUE) @@ -72,12 +90,19 @@ public class SetTagsAction implements IssuesWsAction { public void handle(Request request, Response response) throws Exception { String key = request.mandatoryParam(PARAM_ISSUE); List tags = MoreObjects.firstNonNull(request.paramAsStrings(PARAM_TAGS), Collections.emptyList()); - Collection resultTags = service.setTags(key, tags); - JsonWriter json = response.newJsonWriter().beginObject().name("tags").beginArray(); - for (String tag : resultTags) { - json.value(tag); + setTags(key, tags); + responseWriter.write(key, request, response); + } + + private void setTags(String issueKey, List tags) { + userSession.checkLoggedIn(); + try (DbSession session = dbClient.openSession(false)) { + DefaultIssue issue = issueFinder.getByKey(session, issueKey).toDefaultIssue(); + IssueChangeContext context = IssueChangeContext.createUser(new Date(), userSession.getLogin()); + if (issueFieldsSetter.setTags(issue, tags, context)) { + issueUpdater.saveIssue(session, issue, context, null); + } } - json.endArray().endObject().close(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java index d07dbe8a7e6..b2f04caedd9 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java @@ -20,7 +20,6 @@ package org.sonar.server.issue; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.Sets; import org.junit.After; import org.junit.Before; import org.junit.ClassRule; @@ -88,41 +87,6 @@ public class IssueServiceMediumTest { session.close(); } - @Test - public void set_tags() { - RuleDto rule = newRule(); - ComponentDto project = newProject(); - ComponentDto file = newFile(project); - userSessionRule.logIn("john").addProjectUuidPermissions(UserRole.USER, project.uuid()); - - IssueDto issue = saveIssue(IssueTesting.newDto(rule, file, project)); - - assertThat(getIssueByKey(issue.getKey()).tags()).isEmpty(); - - // Tags are lowercased - service.setTags(issue.getKey(), ImmutableSet.of("bug", "Convention")); - assertThat(getIssueByKey(issue.getKey()).tags()).containsOnly("bug", "convention"); - - // nulls and empty tags are ignored - service.setTags(issue.getKey(), Sets.newHashSet("security", null, "", "convention")); - assertThat(getIssueByKey(issue.getKey()).tags()).containsOnly("security", "convention"); - - // tag validation - try { - service.setTags(issue.getKey(), ImmutableSet.of("pol op")); - } catch (Exception exception) { - assertThat(exception).isInstanceOf(IllegalArgumentException.class); - } - assertThat(getIssueByKey(issue.getKey()).tags()).containsOnly("security", "convention"); - - // unchanged tags - service.setTags(issue.getKey(), ImmutableSet.of("convention", "security")); - assertThat(getIssueByKey(issue.getKey()).tags()).containsOnly("security", "convention"); - - service.setTags(issue.getKey(), ImmutableSet.of()); - assertThat(getIssueByKey(issue.getKey()).tags()).isEmpty(); - } - @Test public void list_component_tags() { RuleDto rule = newRule(); diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTagsActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTagsActionTest.java index d5e3d9d6df5..2a9dbf7d550 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTagsActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTagsActionTest.java @@ -19,44 +19,195 @@ */ package org.sonar.server.issue.ws; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; -import org.junit.Before; +import com.google.common.base.Joiner; +import java.util.Arrays; +import java.util.List; +import javax.annotation.Nullable; +import org.junit.Rule; import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.runners.MockitoJUnitRunner; +import org.junit.rules.ExpectedException; +import org.sonar.api.config.MapSettings; +import org.sonar.api.server.ws.Request; +import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService.Action; import org.sonar.api.server.ws.WebService.Param; -import org.sonar.server.issue.IssueService; -import org.sonar.server.ws.WsTester; +import org.sonar.api.utils.System2; +import org.sonar.core.issue.FieldDiffs; +import org.sonar.db.DbClient; +import org.sonar.db.DbTester; +import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.IssueDto; +import org.sonar.db.issue.IssueTesting; +import org.sonar.db.rule.RuleDefinitionDto; +import org.sonar.server.es.EsTester; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.UnauthorizedException; +import org.sonar.server.issue.IssueFieldsSetter; +import org.sonar.server.issue.IssueFinder; +import org.sonar.server.issue.IssueUpdater; +import org.sonar.server.issue.ServerIssueStorage; +import org.sonar.server.issue.index.IssueIndexDefinition; +import org.sonar.server.issue.index.IssueIndexer; +import org.sonar.server.issue.index.IssueIteratorFactory; +import org.sonar.server.notification.NotificationManager; +import org.sonar.server.organization.DefaultOrganizationProvider; +import org.sonar.server.organization.TestDefaultOrganizationProvider; +import org.sonar.server.rule.DefaultRuleFinder; +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 java.util.Collections.singletonList; 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.mockito.Mockito.when; +import static org.sonar.api.web.UserRole.ISSUE_ADMIN; +import static org.sonar.api.web.UserRole.USER; +import static org.sonar.core.util.Protobuf.setNullable; +import static org.sonar.core.util.stream.MoreCollectors.join; +import static org.sonar.db.component.ComponentTesting.newFileDto; -@RunWith(MockitoJUnitRunner.class) public class SetTagsActionTest { - @Mock - IssueService service; - SetTagsAction sut; - WsTester tester; + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Rule + public DbTester db = DbTester.create(); + @Rule + public EsTester esTester = new EsTester(new IssueIndexDefinition(new MapSettings())); + @Rule + public UserSessionRule userSession = UserSessionRule.standalone(); - @Before - public void setUp() { - sut = new SetTagsAction(service); - tester = new WsTester(new IssuesWs(sut)); + private System2 system2 = mock(System2.class); + private DbClient dbClient = db.getDbClient(); + private DefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); + private OperationResponseWriter responseWriter = mock(OperationResponseWriter.class); + private IssueIndexer issueIndexer = new IssueIndexer(esTester.client(), new IssueIteratorFactory(dbClient)); + + private WsActionTester ws = new WsActionTester(new SetTagsAction(userSession, dbClient, new IssueFinder(dbClient, userSession), new IssueFieldsSetter(), + new IssueUpdater(dbClient, + new ServerIssueStorage(system2, new DefaultRuleFinder(dbClient, defaultOrganizationProvider), dbClient, issueIndexer), mock(NotificationManager.class)), + responseWriter)); + + @Test + public void set_tags() { + IssueDto issueDto = db.issues().insertIssue(newIssue().setTags(singletonList("old-tag"))); + setUserWithBrowsePermission(issueDto.getProjectUuid()); + + call(issueDto.getKey(), "bug", "todo"); + + verify(responseWriter).write(eq(issueDto.getKey()), any(Request.class), any(Response.class)); + IssueDto issueReloaded = dbClient.issueDao().selectByKey(db.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getTags()).containsOnly("bug", "todo"); + } + + @Test + public void remove_existing_tags_when_value_is_not_set() { + IssueDto issueDto = db.issues().insertIssue(newIssue().setTags(singletonList("old-tag"))); + setUserWithBrowsePermission(issueDto.getProjectUuid()); + + call(issueDto.getKey()); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(db.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getTags()).isEmpty(); } @Test - public void should_define() { - Action action = tester.controller("api/issues").action("set_tags"); + public void remove_existing_tags_when_value_is_empty_string() { + IssueDto issueDto = db.issues().insertIssue(newIssue().setTags(singletonList("old-tag"))); + setUserWithBrowsePermission(issueDto.getProjectUuid()); + + call(issueDto.getKey(), ""); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(db.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getTags()).isEmpty(); + } + + @Test + public void set_tags_using_deprecated_key_param() { + IssueDto issueDto = db.issues().insertIssue(newIssue().setTags(singletonList("old-tag"))); + setUserWithBrowsePermission(issueDto.getProjectUuid()); + + ws.newRequest().setParam("key", issueDto.getKey()).setParam("tags", "bug").execute(); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(db.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getTags()).containsOnly("bug"); + } + + @Test + public void tags_are_stored_as_lowercase() { + IssueDto issueDto = db.issues().insertIssue(newIssue().setTags(singletonList("old-tag"))); + setUserWithBrowsePermission(issueDto.getProjectUuid()); + + call(issueDto.getKey(), "bug", "Convention"); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(db.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getTags()).containsOnly("bug", "convention"); + } + + @Test + public void empty_tags_are_ignored() { + IssueDto issueDto = db.issues().insertIssue(newIssue().setTags(singletonList("old-tag"))); + setUserWithBrowsePermission(issueDto.getProjectUuid()); + + call(issueDto.getKey(), "security", "", "convention"); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(db.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getTags()).containsOnly("security", "convention"); + } + + @Test + public void insert_entry_in_changelog_when_setting_tags() throws Exception { + IssueDto issueDto = db.issues().insertIssue(newIssue().setTags(singletonList("old-tag"))); + setUserWithBrowsePermission(issueDto.getProjectUuid()); + + call(issueDto.getKey(), "new-tag"); + + List fieldDiffs = dbClient.issueChangeDao().selectChangelogByIssue(db.getSession(), issueDto.getKey()); + assertThat(fieldDiffs).hasSize(1); + assertThat(fieldDiffs.get(0).diffs()).hasSize(1); + assertThat(fieldDiffs.get(0).diffs().get("tags").oldValue()).isEqualTo("old-tag"); + assertThat(fieldDiffs.get(0).diffs().get("tags").newValue()).isEqualTo("new-tag"); + } + + @Test + public void fail_when_bad_tag_format() { + IssueDto issueDto = db.issues().insertIssue(newIssue().setTags(singletonList("old-tag"))); + setUserWithBrowsePermission(issueDto.getProjectUuid()); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Tag 'pol op' is invalid. Rule tags accept only the characters: a-z, 0-9, '+', '-', '#', '.'"); + + call(issueDto.getKey(), "pol op"); + } + + @Test + public void fail_when_not_authenticated() throws Exception { + expectedException.expect(UnauthorizedException.class); + + call("ABCD", "bug"); + } + + @Test + public void fail_when_missing_browse_permission() throws Exception { + IssueDto issueDto = db.issues().insertIssue(); + userSession.logIn("john").addProjectUuidPermissions(ISSUE_ADMIN, issueDto.getProjectUuid()); + + expectedException.expect(ForbiddenException.class); + + call(issueDto.getKey(), "bug"); + } + + @Test + public void test_definition() { + Action action = ws.getDef(); assertThat(action.description()).isNotEmpty(); assertThat(action.responseExampleAsString()).isNotEmpty(); assertThat(action.isPost()).isTrue(); assertThat(action.isInternal()).isFalse(); - assertThat(action.handler()).isEqualTo(sut); assertThat(action.params()).hasSize(2); Param query = action.param("issue"); @@ -70,20 +221,24 @@ public class SetTagsActionTest { assertThat(pageSize.exampleValue()).isNotEmpty(); } - @Test - public void should_set_tags() throws Exception { - when(service.setTags("polop", ImmutableList.of("palap"))).thenReturn(ImmutableSet.of("palap")); - tester.newPostRequest("api/issues", "set_tags").setParam("issue", "polop").setParam("tags", "palap").execute() - .assertJson("{\"tags\":[\"palap\"]}"); - verify(service).setTags("polop", ImmutableList.of("palap")); + private TestResponse call(@Nullable String issueKey, String... tags) { + TestRequest request = ws.newRequest(); + setNullable(issueKey, issue -> request.setParam("issue", issue)); + if (tags.length > 0) { + request.setParam("tags", Arrays.stream(tags).collect(join(Joiner.on(",")))); + } + return request.execute(); } - @Test - public void should_set_tags_with_deprecated_key() throws Exception { - when(service.setTags("polop", ImmutableList.of("palap"))).thenReturn(ImmutableSet.of("palap")); - tester.newPostRequest("api/issues", "set_tags").setParam("key", "polop").setParam("tags", "palap").execute() - .assertJson("{\"tags\":[\"palap\"]}"); - verify(service).setTags("polop", ImmutableList.of("palap")); + private IssueDto newIssue() { + RuleDefinitionDto rule = db.rules().insert(); + ComponentDto project = db.components().insertProject(); + ComponentDto file = db.components().insertComponent(newFileDto(project)); + return IssueTesting.newIssue(rule, file, project); + } + + private void setUserWithBrowsePermission(String projectUuid) { + userSession.logIn("john").addProjectUuidPermissions(USER, projectUuid); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java index f301451f73c..ca5cd1b4436 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java @@ -80,7 +80,6 @@ public class SetTypeActionTest { @Rule public UserSessionRule userSession = UserSessionRule.standalone(); - private System2 system2 = mock(System2.class); private DbClient dbClient = dbTester.getDbClient(); -- 2.39.5