]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-9120 api/issues/set_tags response contains issue information
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Thu, 20 Apr 2017 12:50:47 +0000 (14:50 +0200)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Fri, 21 Apr 2017 14:01:31 +0000 (16:01 +0200)
server/sonar-server/src/main/java/org/sonar/server/issue/IssueService.java
server/sonar-server/src/main/java/org/sonar/server/issue/ws/SetTagsAction.java
server/sonar-server/src/test/java/org/sonar/server/issue/IssueServiceMediumTest.java
server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTagsActionTest.java
server/sonar-server/src/test/java/org/sonar/server/issue/ws/SetTypeActionTest.java

index dc29fbec856bbc4399e4438207a57197d09bc150..b38f7f948a507bac1d8386f9492acdfb27c10441 100644 (file)
  */
 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<String> listAuthors(@Nullable String textQuery, int pageSize) {
@@ -61,23 +43,6 @@ public class IssueService {
     return issueIndex.listAuthors(query, textQuery, pageSize);
   }
 
-  public Collection<String> setTags(String issueKey, Collection<String> 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<String, Long> listTagsForComponent(IssueQuery query, int pageSize) {
     return issueIndex.countTags(query, pageSize);
   }
index 79250c1c12bbf52fc5c1100186983f0e55d5785a..ed9c43e948f9acee521a638081f7326f262f39f0 100644 (file)
@@ -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. <br/>" +
-    "Requires authentication and Browse permission on project<br/>" +
-    "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<String> tags = MoreObjects.firstNonNull(request.paramAsStrings(PARAM_TAGS), Collections.<String>emptyList());
-    Collection<String> 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<String> 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();
   }
 
 }
index d07dbe8a7e6c738065202fcaf8a34443b97fccd3..b2f04caedd9e7648e927a0201114bcc17f744f92 100644 (file)
@@ -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.<String>of());
-    assertThat(getIssueByKey(issue.getKey()).tags()).isEmpty();
-  }
-
   @Test
   public void list_component_tags() {
     RuleDto rule = newRule();
index d5e3d9d6df58ae653fc502d77f1f42ae54e90350..2a9dbf7d5503a87567e310db9014484e248dd6cd 100644 (file)
  */
 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> 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);
   }
 
 }
index f301451f73cee611c5053eb9880259831e04d3a7..ca5cd1b443675744fd69f26fdc4676bb3abc4f73 100644 (file)
@@ -80,7 +80,6 @@ public class SetTypeActionTest {
   @Rule
   public UserSessionRule userSession = UserSessionRule.standalone();
 
-
   private System2 system2 = mock(System2.class);
 
   private DbClient dbClient = dbTester.getDbClient();