opendev/gerrit
Code Issues Proposed changes

Convert HashtagsIT to extension API

Browse Source 
Simplify return types of API handlers.

Change-Id: I00d2e54d5315c50651d0bfa3e3e25548fa6f4ca1
This commit is contained in:
Dave Borowitz
2015-03-23 15:48:36 -07:00
parent adfa462788
commit ee2836ce35
3 changed files with 140 additions and 153 deletions
Download Patch File Download Diff File Expand all files Collapse all files

286
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/HashtagsIT.java
View File

@@ -16,218 +16,206 @@ package com.google.gerrit.acceptance.rest.change;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import com.google.common.base.CharMatcher; import com.google.common.collect.Sets;
import com.google.common.base.Splitter; import com.google.common.truth.IterableSubject;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.testutil.ConfigSuite; import com.google.gerrit.testutil.ConfigSuite;
import com.google.gson.reflect.TypeToken;
import org.apache.http.HttpStatus;
import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Config;
import org.junit.Test; import org.junit.Test;
import java.io.IOException; @NoHttpd
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
public class HashtagsIT extends AbstractDaemonTest { public class HashtagsIT extends AbstractDaemonTest {
@ConfigSuite.Default @ConfigSuite.Default
public static Config defaultConfig() { public static Config defaultConfig() {
return NotesMigration.allEnabledConfig(); return NotesMigration.allEnabledConfig();
} }
private void assertResult(RestResponse r, List<String> expected)
throws IOException {
assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_OK);
List<String> result = toHashtagList(r);
assertThat(result).containsExactlyElementsIn(expected);
}
@Test @Test
public void testGetNoHashtags() throws Exception { public void testGetNoHashtags() throws Exception {
// GET hashtags on a change with no hashtags returns an empty list // Get on a change with no hashtags returns an empty list.
String changeId = createChange().getChangeId(); PushOneCommit.Result r = createChange();
assertResult(GET(changeId), ImmutableList.<String>of()); assertThatGet(r).isEmpty();
} }
@Test @Test
public void testAddSingleHashtag() throws Exception { public void testAddSingleHashtag() throws Exception {
String changeId = createChange().getChangeId(); PushOneCommit.Result r = createChange();
// POST adding a single hashtag returns a single hashtag // Adding a single hashtag returns a single hashtag.
List<String> expected = Arrays.asList("tag2"); addHashtags(r, "tag2");
assertResult(POST(changeId, "tag2", null), expected); assertThatGet(r).containsExactly("tag2");
assertResult(GET(changeId), expected);
// POST adding another single hashtag to change that already has one // Adding another single hashtag to change that already has one hashtag
// hashtag returns a sorted list of hashtags with existing and new // returns a sorted list of hashtags with existing and new.
expected = Arrays.asList("tag1", "tag2"); addHashtags(r, "tag1");
assertResult(POST(changeId, "tag1", null), expected); assertThatGet(r).containsExactly("tag1", "tag2").inOrder();
assertResult(GET(changeId), expected);
} }
@Test @Test
public void testAddMultipleHashtags() throws Exception { public void testAddMultipleHashtags() throws Exception {
String changeId = createChange().getChangeId(); PushOneCommit.Result r = createChange();
// POST adding multiple hashtags returns a sorted list of hashtags // Adding multiple hashtags returns a sorted list of hashtags.
List<String> expected = Arrays.asList("tag1", "tag3"); addHashtags(r, "tag3", "tag1");
assertResult(POST(changeId, "tag3, tag1", null), expected); assertThatGet(r).containsExactly("tag1", "tag3").inOrder();
assertResult(GET(changeId), expected);
// POST adding multiple hashtags to change that already has hashtags // Adding multiple hashtags to change that already has hashtags returns a
// returns a sorted list of hashtags with existing and new // sorted list of hashtags with existing and new.
expected = Arrays.asList("tag1", "tag2", "tag3", "tag4"); addHashtags(r, "tag2", "tag4");
assertResult(POST(changeId, "tag2, tag4", null), expected); assertThatGet(r).containsExactly("tag1", "tag2", "tag3", "tag4").inOrder();
assertResult(GET(changeId), expected);
} }
@Test @Test
public void testAddAlreadyExistingHashtag() throws Exception { public void testAddAlreadyExistingHashtag() throws Exception {
// POST adding a hashtag that already exists on the change returns a // Adding a hashtag that already exists on the change returns a sorted list
// sorted list of hashtags without duplicates // of hashtags without duplicates.
String changeId = createChange().getChangeId(); PushOneCommit.Result r = createChange();
List<String> expected = Arrays.asList("tag2"); addHashtags(r, "tag2");
assertResult(POST(changeId, "tag2", null), expected); assertThatGet(r).containsExactly("tag2");
assertResult(GET(changeId), expected); addHashtags(r, "tag2");
assertResult(POST(changeId, "tag2", null), expected); assertThatGet(r).containsExactly("tag2");
assertResult(GET(changeId), expected); addHashtags(r, "tag1", "tag2");
expected = Arrays.asList("tag1", "tag2"); assertThatGet(r).containsExactly("tag1", "tag2").inOrder();
assertResult(POST(changeId, "tag2, tag1", null), expected);
assertResult(GET(changeId), expected);
} }
@Test @Test
public void testHashtagsWithPrefix() throws Exception { public void testHashtagsWithPrefix() throws Exception {
String changeId = createChange().getChangeId(); PushOneCommit.Result r = createChange();
// Leading # is stripped from added tag // Leading # is stripped from added tag.
List<String> expected = Arrays.asList("tag1"); addHashtags(r, "#tag1");
assertResult(POST(changeId, "#tag1", null), expected); assertThatGet(r).containsExactly("tag1");
assertResult(GET(changeId), expected);
// Leading # is stripped from multiple added tags // Leading # is stripped from multiple added tags.
expected = Arrays.asList("tag1", "tag2", "tag3"); addHashtags(r, "#tag2", "#tag3");
assertResult(POST(changeId, "#tag2, #tag3", null), expected); assertThatGet(r).containsExactly("tag1", "tag2", "tag3").inOrder();
assertResult(GET(changeId), expected);
// Leading # is stripped from removed tag // Leading # is stripped from removed tag.
expected = Arrays.asList("tag1", "tag3"); removeHashtags(r, "#tag2");
assertResult(POST(changeId, null, "#tag2"), expected); assertThatGet(r).containsExactly("tag1", "tag3").inOrder();
assertResult(GET(changeId), expected);
// Leading # is stripped from multiple removed tags // Leading # is stripped from multiple removed tags.
expected = Collections.emptyList(); removeHashtags(r, "#tag1", "#tag3");
assertResult(POST(changeId, null, "#tag1, #tag3"), expected); assertThatGet(r).isEmpty();
assertResult(GET(changeId), expected);
// Leading # and space are stripped from added tag // Leading # and space are stripped from added tag.
expected = Arrays.asList("tag1"); addHashtags(r, "# tag1");
assertResult(POST(changeId, "# tag1", null), expected); assertThatGet(r).containsExactly("tag1");
assertResult(GET(changeId), expected);
// Multiple leading # are stripped from added tag // Multiple leading # are stripped from added tag.
expected = Arrays.asList("tag1", "tag2"); addHashtags(r, "##tag2");
assertResult(POST(changeId, "##tag2", null), expected); assertThatGet(r).containsExactly("tag1", "tag2").inOrder();
assertResult(GET(changeId), expected);
// Multiple leading spaces and # are stripped from added tag // Multiple leading spaces and # are stripped from added tag.
expected = Arrays.asList("tag1", "tag2", "tag3"); addHashtags(r, "# # tag3");
assertResult(POST(changeId, " # # tag3", null), expected); assertThatGet(r).containsExactly("tag1", "tag2", "tag3").inOrder();
assertResult(GET(changeId), expected);
} }
@Test @Test
public void testRemoveSingleHashtag() throws Exception { public void testRemoveSingleHashtag() throws Exception {
// POST removing a single tag from a change that only has that tag // Removing a single tag from a change that only has that tag returns an
// returns an empty list // empty list.
String changeId = createChange().getChangeId(); PushOneCommit.Result r = createChange();
List<String> expected = Arrays.asList("tag1"); addHashtags(r, "tag1");
assertResult(POST(changeId, "tag1", null), expected); assertThatGet(r).containsExactly("tag1");
assertResult(POST(changeId, null, "tag1"), ImmutableList.<String>of()); removeHashtags(r, "tag1");
assertResult(GET(changeId), ImmutableList.<String>of()); assertThatGet(r).isEmpty();
// POST removing a single tag from a change that has multiple tags // Removing a single tag from a change that has multiple tags returns a
// returns a sorted list of remaining tags // sorted list of remaining tags.
expected = Arrays.asList("tag1", "tag2", "tag3"); addHashtags(r, "tag1", "tag2", "tag3");
assertResult(POST(changeId, "tag1, tag2, tag3", null), expected); removeHashtags(r, "tag2");
expected = Arrays.asList("tag1", "tag3"); assertThatGet(r).containsExactly("tag1", "tag3").inOrder();
assertResult(POST(changeId, null, "tag2"), expected);
assertResult(GET(changeId), expected);
} }
@Test @Test
public void testRemoveMultipleHashtags() throws Exception { public void testRemoveMultipleHashtags() throws Exception {
// POST removing multiple tags from a change that only has those tags // Removing multiple tags from a change that only has those tags returns an
// returns an empty list // empty list.
String changeId = createChange().getChangeId(); PushOneCommit.Result r = createChange();
List<String> expected = Arrays.asList("tag1", "tag2"); addHashtags(r, "tag1", "tag2");
assertResult(POST(changeId, "tag1, tag2", null), expected); assertThatGet(r).containsExactly("tag1", "tag2").inOrder();
assertResult(POST(changeId, null, "tag1, tag2"), ImmutableList.<String>of()); removeHashtags(r, "tag1", "tag2");
assertResult(GET(changeId), ImmutableList.<String>of()); assertThatGet(r).isEmpty();
// POST removing multiple tags from a change that has multiple changes // Removing multiple tags from a change that has multiple tags returns a
// returns a sorted list of remaining changes // sorted list of remaining tags.
expected = Arrays.asList("tag1", "tag2", "tag3", "tag4"); addHashtags(r, "tag1", "tag2", "tag3", "tag4");
assertResult(POST(changeId, "tag1, tag2, tag3, tag4", null), expected); assertThatGet(r).containsExactly("tag1", "tag2", "tag3", "tag4").inOrder();
expected = Arrays.asList("tag2", "tag4"); removeHashtags(r, "tag2", "tag4");
assertResult(POST(changeId, null, "tag1, tag3"), expected); assertThatGet(r).containsExactly("tag1", "tag3").inOrder();
assertResult(GET(changeId), expected);
} }
@Test @Test
public void testRemoveNotExistingHashtag() throws Exception { public void testRemoveNotExistingHashtag() throws Exception {
// POST removing a single hashtag from change that has no hashtags // Removing a single hashtag from change that has no hashtags returns an
// returns an empty list // empty list.
String changeId = createChange().getChangeId(); PushOneCommit.Result r = createChange();
assertResult(POST(changeId, null, "tag1"), ImmutableList.<String>of()); removeHashtags(r, "tag1");
assertResult(GET(changeId), ImmutableList.<String>of()); assertThatGet(r).isEmpty();
// POST removing a single non-existing tag from a change that only // Removing a single non-existing tag from a change that only has one other
// has one other tag returns a list of only one tag // tag returns a list of only one tag.
List<String> expected = Arrays.asList("tag1"); addHashtags(r, "tag1");
assertResult(POST(changeId, "tag1", null), expected); removeHashtags(r, "tag4");
assertResult(POST(changeId, null, "tag4"), expected); assertThatGet(r).containsExactly("tag1");
assertResult(GET(changeId), expected);
// POST removing a single non-existing tag from a change that has multiple // Removing a single non-existing tag from a change that has multiple tags
// tags returns a sorted list of tags without any deleted // returns a sorted list of tags without any deleted.
expected = Arrays.asList("tag1", "tag2", "tag3"); addHashtags(r, "tag1", "tag2", "tag3");
assertResult(POST(changeId, "tag1, tag2, tag3", null), expected); removeHashtags(r, "tag4");
assertResult(POST(changeId, null, "tag4"), expected); assertThatGet(r).containsExactly("tag1", "tag2", "tag3").inOrder();
assertResult(GET(changeId), expected);
} }
private RestResponse GET(String changeId) throws IOException { @Test
return adminSession.get("/changes/" + changeId + "/hashtags/"); public void testAddAndRemove() throws Exception {
} // Adding and remove hashtags in a single request performs correctly.
PushOneCommit.Result r = createChange();
private RestResponse POST(String changeId, String toAdd, String toRemove) addHashtags(r, "tag1", "tag2");
throws IOException {
HashtagsInput input = new HashtagsInput(); HashtagsInput input = new HashtagsInput();
if (toAdd != null) { input.add = Sets.newHashSet("tag3", "tag4");
input.add = new HashSet<>( input.remove = Sets.newHashSet("tag1");
Lists.newArrayList(Splitter.on(CharMatcher.anyOf(",")).split(toAdd))); gApi.changes().id(r.getChange().getId().get()).setHashtags(input);
} assertThatGet(r).containsExactly("tag2", "tag3", "tag4");
if (toRemove != null) {
input.remove = new HashSet<>( // Adding and removing the same hashtag actually removes it.
Lists.newArrayList(Splitter.on(CharMatcher.anyOf(",")).split(toRemove))); addHashtags(r, "tag1", "tag2");
} input = new HashtagsInput();
return adminSession.post("/changes/" + changeId + "/hashtags/", input); input.add = Sets.newHashSet("tag3", "tag4");
input.remove = Sets.newHashSet("tag3");
gApi.changes().id(r.getChange().getId().get()).setHashtags(input);
assertThatGet(r).containsExactly("tag1", "tag2", "tag4");
} }
private static List<String> toHashtagList(RestResponse r) private IterableSubject<
throws IOException { ? extends IterableSubject<?, String, Iterable<String>>,
List<String> result = String, Iterable<String>>
newGson().fromJson(r.getReader(), assertThatGet(PushOneCommit.Result r) throws Exception {
new TypeToken<List<String>>() {}.getType()); return assertThat((Iterable<String>) gApi.changes()
return result; .id(r.getChange().getId().get())
.getHashtags());
}
private void addHashtags(PushOneCommit.Result r, String... toAdd)
throws Exception {
HashtagsInput input = new HashtagsInput();
input.add = Sets.newHashSet(toAdd);
gApi.changes()
.id(r.getChange().getId().get())
.setHashtags(input);
}
private void removeHashtags(PushOneCommit.Result r, String... toRemove)
throws Exception {
HashtagsInput input = new HashtagsInput();
input.remove = Sets.newHashSet(toRemove);
gApi.changes()
.id(r.getChange().getId().get())
.setHashtags(input);
} }
} }

3
gerrit-server/src/main/java/com/google/gerrit/server/change/GetHashtags.java
View File

@@ -30,9 +30,8 @@ import java.util.Set;
@Singleton @Singleton
public class GetHashtags implements RestReadView<ChangeResource> { public class GetHashtags implements RestReadView<ChangeResource> {
@Override @Override
public Response<? extends Set<String>> apply(ChangeResource req) public Response<Set<String>> apply(ChangeResource req)
throws AuthException, OrmException, IOException, BadRequestException { throws AuthException, OrmException, IOException, BadRequestException {
ChangeControl control = req.getControl(); ChangeControl control = req.getControl();
ChangeNotes notes = control.getNotes().load(); ChangeNotes notes = control.getNotes().load();
Set<String> hashtags = notes.getHashtags(); Set<String> hashtags = notes.getHashtags();

4
gerrit-server/src/main/java/com/google/gerrit/server/change/PostHashtags.java
View File

@@ -38,12 +38,12 @@ public class PostHashtags implements RestModifyView<ChangeResource, HashtagsInpu
} }
@Override @Override
public Response<? extends Set<String>> apply(ChangeResource req, HashtagsInput input) public Response<Set<String>> apply(ChangeResource req, HashtagsInput input)
throws AuthException, OrmException, IOException, BadRequestException, throws AuthException, OrmException, IOException, BadRequestException,
ResourceConflictException { ResourceConflictException {
try { try {
return Response.ok(hashtagsUtil.setHashtags( return Response.<Set<String>> ok(hashtagsUtil.setHashtags(
req.getControl(), input, true, true)); req.getControl(), input, true, true));
} catch (IllegalArgumentException e) { } catch (IllegalArgumentException e) {
throw new BadRequestException(e.getMessage()); throw new BadRequestException(e.getMessage());