ChangeApiImpl: Don't catch RestApiException when editing hashtag

If a RestApiException (or, importantly, any subclass of RestApiException)
is raised when performing the hashtag edit, it is caught and rethrown as
RestApiException with a generic "Cannot post hashtags" message.

Any specific exception type, and its message, is then lost.

Don't catch RestApiException. Instead let it propagate up to the caller.

Also add tests to confirm the behavior when adding hashtags with and
without the "edit hashtags" permission.

Change-Id: Ib708d871462972b3f0644351d062de08ef1c9bee
This commit is contained in:
David Pursehouse
2017-03-29 21:05:39 +09:00
parent 01a139b2e9
commit 483baef151
2 changed files with 23 additions and 1 deletions

View File

@@ -17,6 +17,7 @@ package com.google.gerrit.acceptance.rest.change;
import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.TruthJUnit.assume; import static com.google.common.truth.TruthJUnit.assume;
import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
import static java.util.concurrent.TimeUnit.SECONDS; import static java.util.concurrent.TimeUnit.SECONDS;
import com.google.common.collect.Iterables; import com.google.common.collect.Iterables;
@@ -25,8 +26,10 @@ import com.google.common.truth.IterableSubject;
import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.extensions.api.changes.HashtagsInput; import com.google.gerrit.extensions.api.changes.HashtagsInput;
import com.google.gerrit.extensions.common.ChangeMessageInfo; import com.google.gerrit.extensions.common.ChangeMessageInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.testutil.TestTimeUtil; import com.google.gerrit.testutil.TestTimeUtil;
import org.junit.AfterClass; import org.junit.AfterClass;
import org.junit.Before; import org.junit.Before;
@@ -244,6 +247,25 @@ public class HashtagsIT extends AbstractDaemonTest {
assertMessage(r, "Hashtag added: MyHashtag"); assertMessage(r, "Hashtag added: MyHashtag");
} }
@Test
public void addHashtagWithoutPermissionNotAllowed() throws Exception {
PushOneCommit.Result r = createChange();
setApiUser(user);
exception.expect(AuthException.class);
exception.expectMessage("Editing hashtags not permitted");
addHashtags(r, "MyHashtag");
}
@Test
public void addHashtagWithPermissionAllowed() throws Exception {
PushOneCommit.Result r = createChange();
grant(Permission.EDIT_HASHTAGS, project, "refs/heads/master", false, REGISTERED_USERS);
setApiUser(user);
addHashtags(r, "MyHashtag");
assertThatGet(r).containsExactly("MyHashtag");
assertMessage(r, "Hashtag added: MyHashtag");
}
private IterableSubject assertThatGet(PushOneCommit.Result r) throws Exception { private IterableSubject assertThatGet(PushOneCommit.Result r) throws Exception {
return assertThat(gApi.changes().id(r.getChange().getId().get()).getHashtags()); return assertThat(gApi.changes().id(r.getChange().getId().get()).getHashtags());
} }

View File

@@ -454,7 +454,7 @@ class ChangeApiImpl implements ChangeApi {
public void setHashtags(HashtagsInput input) throws RestApiException { public void setHashtags(HashtagsInput input) throws RestApiException {
try { try {
postHashtags.apply(change, input); postHashtags.apply(change, input);
} catch (RestApiException | UpdateException e) { } catch (UpdateException e) {
throw new RestApiException("Cannot post hashtags", e); throw new RestApiException("Cannot post hashtags", e);
} }
} }