Merge changes I100e4422,I6fcd8090,Ifef2ed6a,I70237fcb,Id7e778c2, ...
* changes:
Use RetryingRestModifyView for RebaseChangeEdit.Rebase
Do not throw NotImplementedException from PublishChangeEdit and RebaseChangeEdit
Implement views() method in PublishChangeEdit and RebaseChangeEdit
Do not reuse ChangeEditResource for child resources of 3 different collections
Let REST API binding tests fail if '405 Method Not Allowed' is returned
Extend REST binding tests to cover endpoints that are implemented by AcceptsDelete
Extend REST binding tests to cover endpoints that are implemented by AcceptsPost
Test that the change edit REST endpoints are correctly bound
Test that the change message REST endpoints are correctly bound
diff --git a/java/com/google/gerrit/server/change/ChangeEditResource.java b/java/com/google/gerrit/server/change/ChangeEditResource.java
index 392709e..5419ee9 100644
--- a/java/com/google/gerrit/server/change/ChangeEditResource.java
+++ b/java/com/google/gerrit/server/change/ChangeEditResource.java
@@ -32,6 +32,10 @@
public class ChangeEditResource implements RestResource {
public static final TypeLiteral<RestView<ChangeEditResource>> CHANGE_EDIT_KIND =
new TypeLiteral<RestView<ChangeEditResource>>() {};
+ public static final TypeLiteral<RestView<ChangeEditResource.Publish>> CHANGE_EDIT_PUBLISH_KIND =
+ new TypeLiteral<RestView<ChangeEditResource.Publish>>() {};
+ public static final TypeLiteral<RestView<ChangeEditResource.Rebase>> CHANGE_EDIT_REBASE_KIND =
+ new TypeLiteral<RestView<ChangeEditResource.Rebase>>() {};
private final ChangeResource change;
private final ChangeEdit edit;
@@ -60,4 +64,16 @@
public String getPath() {
return path;
}
+
+ public static class Publish extends ChangeEditResource {
+ public Publish(ChangeResource change, ChangeEdit edit, String path) {
+ super(change, edit, path);
+ }
+ }
+
+ public static class Rebase extends ChangeEditResource {
+ public Rebase(ChangeResource change, ChangeEdit edit, String path) {
+ super(change, edit, path);
+ }
+ }
}
diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java
index d27b136..ef0ba2d 100644
--- a/java/com/google/gerrit/server/restapi/change/Module.java
+++ b/java/com/google/gerrit/server/restapi/change/Module.java
@@ -15,6 +15,8 @@
package com.google.gerrit.server.restapi.change;
import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_KIND;
+import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_PUBLISH_KIND;
+import static com.google.gerrit.server.change.ChangeEditResource.CHANGE_EDIT_REBASE_KIND;
import static com.google.gerrit.server.change.ChangeMessageResource.CHANGE_MESSAGE_KIND;
import static com.google.gerrit.server.change.ChangeResource.CHANGE_KIND;
import static com.google.gerrit.server.change.CommentResource.COMMENT_KIND;
@@ -65,6 +67,8 @@
DynamicMap.mapOf(binder(), REVIEWER_KIND);
DynamicMap.mapOf(binder(), REVISION_KIND);
DynamicMap.mapOf(binder(), CHANGE_EDIT_KIND);
+ DynamicMap.mapOf(binder(), CHANGE_EDIT_PUBLISH_KIND);
+ DynamicMap.mapOf(binder(), CHANGE_EDIT_REBASE_KIND);
DynamicMap.mapOf(binder(), VOTE_KIND);
DynamicMap.mapOf(binder(), CHANGE_MESSAGE_KIND);
diff --git a/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java b/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java
index b356f18..36a0561 100644
--- a/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java
+++ b/java/com/google/gerrit/server/restapi/change/PublishChangeEdit.java
@@ -19,8 +19,8 @@
import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.RestView;
@@ -44,28 +44,32 @@
@Singleton
public class PublishChangeEdit
- implements ChildCollection<ChangeResource, ChangeEditResource>, AcceptsPost<ChangeResource> {
+ implements ChildCollection<ChangeResource, ChangeEditResource.Publish>,
+ AcceptsPost<ChangeResource> {
+ private final DynamicMap<RestView<ChangeEditResource.Publish>> views;
private final Publish publish;
@Inject
- PublishChangeEdit(Publish publish) {
+ PublishChangeEdit(DynamicMap<RestView<ChangeEditResource.Publish>> views, Publish publish) {
+ this.views = views;
this.publish = publish;
}
@Override
- public DynamicMap<RestView<ChangeEditResource>> views() {
- throw new NotImplementedException();
+ public DynamicMap<RestView<ChangeEditResource.Publish>> views() {
+ return views;
}
@Override
- public RestView<ChangeResource> list() {
- throw new NotImplementedException();
+ public RestView<ChangeResource> list() throws ResourceNotFoundException {
+ throw new ResourceNotFoundException();
}
@Override
- public ChangeEditResource parse(ChangeResource parent, IdString id) {
- throw new NotImplementedException();
+ public ChangeEditResource.Publish parse(ChangeResource parent, IdString id)
+ throws ResourceNotFoundException {
+ throw new ResourceNotFoundException();
}
@Override
diff --git a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
index 7e1bb4d..47af894 100644
--- a/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
+++ b/java/com/google/gerrit/server/restapi/change/RebaseChangeEdit.java
@@ -20,11 +20,10 @@
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ChildCollection;
import com.google.gerrit.extensions.restapi.IdString;
-import com.google.gerrit.extensions.restapi.NotImplementedException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.Response;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.change.ChangeEditResource;
@@ -33,6 +32,9 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.project.InvalidChangeOperationException;
+import com.google.gerrit.server.update.BatchUpdate;
+import com.google.gerrit.server.update.RetryHelper;
+import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -41,28 +43,32 @@
@Singleton
public class RebaseChangeEdit
- implements ChildCollection<ChangeResource, ChangeEditResource>, AcceptsPost<ChangeResource> {
+ implements ChildCollection<ChangeResource, ChangeEditResource.Rebase>,
+ AcceptsPost<ChangeResource> {
+ private final DynamicMap<RestView<ChangeEditResource.Rebase>> views;
private final Rebase rebase;
@Inject
- RebaseChangeEdit(Rebase rebase) {
+ RebaseChangeEdit(DynamicMap<RestView<ChangeEditResource.Rebase>> views, Rebase rebase) {
+ this.views = views;
this.rebase = rebase;
}
@Override
- public DynamicMap<RestView<ChangeEditResource>> views() {
- throw new NotImplementedException();
+ public DynamicMap<RestView<ChangeEditResource.Rebase>> views() {
+ return views;
}
@Override
- public RestView<ChangeResource> list() {
- throw new NotImplementedException();
+ public RestView<ChangeResource> list() throws ResourceNotFoundException {
+ throw new ResourceNotFoundException();
}
@Override
- public ChangeEditResource parse(ChangeResource parent, IdString id) {
- throw new NotImplementedException();
+ public ChangeEditResource.Rebase parse(ChangeResource parent, IdString id)
+ throws ResourceNotFoundException {
+ throw new ResourceNotFoundException();
}
@Override
@@ -71,19 +77,23 @@
}
@Singleton
- public static class Rebase implements RestModifyView<ChangeResource, Input> {
-
+ public static class Rebase extends RetryingRestModifyView<ChangeResource, Input, Response<?>> {
private final GitRepositoryManager repositoryManager;
private final ChangeEditModifier editModifier;
@Inject
- Rebase(GitRepositoryManager repositoryManager, ChangeEditModifier editModifier) {
+ Rebase(
+ RetryHelper retryHelper,
+ GitRepositoryManager repositoryManager,
+ ChangeEditModifier editModifier) {
+ super(retryHelper);
this.repositoryManager = repositoryManager;
this.editModifier = editModifier;
}
@Override
- public Response<?> apply(ChangeResource rsrc, Input in)
+ protected Response<?> applyImpl(
+ BatchUpdate.Factory updateFactory, ChangeResource rsrc, Input in)
throws AuthException, ResourceConflictException, IOException, OrmException,
PermissionBackendException {
Project.NameKey project = rsrc.getProject();
diff --git a/javatests/com/google/gerrit/acceptance/rest/AbstractRestApiBindingsTest.java b/javatests/com/google/gerrit/acceptance/rest/AbstractRestApiBindingsTest.java
index 26bc345..2bb3dca 100644
--- a/javatests/com/google/gerrit/acceptance/rest/AbstractRestApiBindingsTest.java
+++ b/javatests/com/google/gerrit/acceptance/rest/AbstractRestApiBindingsTest.java
@@ -18,6 +18,7 @@
import static com.google.common.truth.Truth.assertWithMessage;
import static org.apache.http.HttpStatus.SC_FORBIDDEN;
import static org.apache.http.HttpStatus.SC_INTERNAL_SERVER_ERROR;
+import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
import static org.apache.http.HttpStatus.SC_NOT_FOUND;
import com.google.auto.value.AutoValue;
@@ -80,8 +81,13 @@
String msg = String.format("%s %s returned %d: %s", method, uri, status, body);
if (restCall.expectedResponseCode().isPresent()) {
assertWithMessage(msg).that(status).isEqualTo(restCall.expectedResponseCode().get());
+ if (restCall.expectedMessage().isPresent()) {
+ assertWithMessage(msg).that(body).contains(restCall.expectedMessage().get());
+ }
} else {
- assertWithMessage(msg).that(status).isNotIn(ImmutableList.of(SC_FORBIDDEN, SC_NOT_FOUND));
+ assertWithMessage(msg)
+ .that(status)
+ .isNotIn(ImmutableList.of(SC_FORBIDDEN, SC_NOT_FOUND, SC_METHOD_NOT_ALLOWED));
assertWithMessage(msg).that(status).isLessThan(SC_INTERNAL_SERVER_ERROR);
}
}
@@ -123,6 +129,8 @@
abstract Optional<Integer> expectedResponseCode();
+ abstract Optional<String> expectedMessage();
+
String uri(String... args) {
String uriFormat = uriFormat();
int expectedArgNum = StringUtils.countMatches(uriFormat, "%s");
@@ -144,6 +152,8 @@
abstract Builder expectedResponseCode(int expectedResponseCode);
+ abstract Builder expectedMessage(String expectedMessage);
+
abstract RestCall build();
}
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java
index 8de85a9..e5c16de 100644
--- a/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/AccountsRestApiBindingsIT.java
@@ -14,6 +14,9 @@
package com.google.gerrit.acceptance.rest;
+import static com.google.gerrit.acceptance.rest.AbstractRestApiBindingsTest.Method.PUT;
+import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
+
import com.google.common.collect.ImmutableList;
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.extensions.common.ChangeInput;
@@ -40,7 +43,11 @@
RestCall.put("/accounts/%s/name"),
RestCall.delete("/accounts/%s/name"),
RestCall.get("/accounts/%s/username"),
- RestCall.put("/accounts/%s/username"),
+ RestCall.builder(PUT, "/accounts/%s/username")
+ // Changing the username is not allowed.
+ .expectedResponseCode(SC_METHOD_NOT_ALLOWED)
+ .expectedMessage("Username cannot be changed.")
+ .build(),
RestCall.get("/accounts/%s/active"),
RestCall.put("/accounts/%s/active"),
RestCall.delete("/accounts/%s/active"),
diff --git a/javatests/com/google/gerrit/acceptance/rest/ChangesRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ChangesRestApiBindingsIT.java
index 53c544d..a7f1329 100644
--- a/javatests/com/google/gerrit/acceptance/rest/ChangesRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/ChangesRestApiBindingsIT.java
@@ -59,7 +59,6 @@
RestCall.delete("/changes/%s/topic"),
RestCall.get("/changes/%s/in"),
RestCall.get("/changes/%s/hashtags"),
- RestCall.post("/changes/%s/hashtags"),
RestCall.get("/changes/%s/comments"),
RestCall.get("/changes/%s/robotcomments"),
RestCall.get("/changes/%s/drafts"),
@@ -98,6 +97,7 @@
.expectedResponseCode(SC_NOT_FOUND)
.build(),
RestCall.get("/changes/%s/edit"),
+ RestCall.post("/changes/%s/edit"),
RestCall.post("/changes/%s/edit:rebase"),
RestCall.get("/changes/%s/edit:message"),
RestCall.put("/changes/%s/edit:message"),
@@ -115,7 +115,8 @@
* identifier.
*/
private static final ImmutableList<RestCall> CHANGE_ENDPOINTS_NOTEDB =
- ImmutableList.of(RestCall.post("/changes/%s/rebuild.notedb"));
+ ImmutableList.of(
+ RestCall.post("/changes/%s/hashtags"), RestCall.post("/changes/%s/rebuild.notedb"));
/**
* Reviewer REST endpoints to be tested, each URL contains placeholders for the change identifier
@@ -239,7 +240,29 @@
RestCall.get("/changes/%s/revisions/%s/files/%s/diff"),
RestCall.get("/changes/%s/revisions/%s/files/%s/blame"));
- // TODO(ekempin): Add tests for change message and change edit REST endpoints
+ /**
+ * Change message REST endpoints to be tested, each URL contains placeholders for the change
+ * identifier and the change message identifier.
+ */
+ private static final ImmutableList<RestCall> CHANGE_MESSAGE_ENDPOINTS =
+ ImmutableList.of(RestCall.get("/changes/%s/messages/%s"));
+
+ /**
+ * Change edit REST endpoints to be tested, each URL contains placeholders for the change
+ * identifier and the change edit identifier.
+ */
+ private static final ImmutableList<RestCall> CHANGE_EDIT_ENDPOINTS =
+ ImmutableList.of(
+ // Create change edit by deleting an existing file.
+ RestCall.delete("/changes/%s/edit/%s"),
+
+ // Calls on existing change edit.
+ RestCall.get("/changes/%s/edit/%s"),
+ RestCall.put("/changes/%s/edit/%s"),
+ RestCall.get("/changes/%s/edit/%s/meta"),
+
+ // Delete content of a file in an existing change edit.
+ RestCall.delete("/changes/%s/edit/%s"));
private static final String FILENAME = "test.txt";
@@ -426,6 +449,24 @@
execute(REVISION_FILE_ENDPOINTS, changeId, "current", FILENAME);
}
+ @Test
+ public void changeMessageEndpoints() throws Exception {
+ String changeId = createChange().getChangeId();
+
+ // A change message is created on change creation.
+ String changeMessageId = Iterables.getOnlyElement(gApi.changes().id(changeId).messages()).id;
+
+ execute(CHANGE_MESSAGE_ENDPOINTS, changeId, changeMessageId);
+ }
+
+ @Test
+ public void changeEditEndpoints() throws Exception {
+ String changeId = createChange("Subject", FILENAME, "content").getChangeId();
+
+ // The change edit is created by the first REST call.
+ execute(CHANGE_EDIT_ENDPOINTS, changeId, FILENAME);
+ }
+
private static Comment.Range createRange(
int startLine, int startCharacter, int endLine, int endCharacter) {
Comment.Range range = new Comment.Range();
diff --git a/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java
index b930b20..508d407 100644
--- a/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/ConfigRestApiBindingsIT.java
@@ -56,6 +56,7 @@
RestCall.get("/config/server/summary"),
RestCall.get("/config/server/capabilities"),
RestCall.get("/config/server/caches"),
+ RestCall.post("/config/server/caches"),
RestCall.get("/config/server/tasks"));
/**
diff --git a/javatests/com/google/gerrit/acceptance/rest/PluginsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/PluginsRestApiBindingsIT.java
index aba677f..07ea3d0 100644
--- a/javatests/com/google/gerrit/acceptance/rest/PluginsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/PluginsRestApiBindingsIT.java
@@ -42,9 +42,9 @@
RestCall.get("/plugins/%s/gerrit~status"),
// POST (and PUT) requests don't require the 'gerrit~' prefix in front of the view name.
- RestCall.post("/plugins/%s/enable"),
- RestCall.post("/plugins/%s/disable"),
- RestCall.post("/plugins/%s/reload"),
+ RestCall.post("/plugins/%s/gerrit~enable"),
+ RestCall.post("/plugins/%s/gerrit~disable"),
+ RestCall.post("/plugins/%s/gerrit~reload"),
// Plugin deletion must be tested last
RestCall.delete("/plugins/%s"));
diff --git a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
index d59f037..a86a739 100644
--- a/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/ProjectsRestApiBindingsIT.java
@@ -19,6 +19,7 @@
import static com.google.gerrit.acceptance.rest.AbstractRestApiBindingsTest.Method.GET;
import static com.google.gerrit.reviewdb.client.RefNames.REFS_DASHBOARDS;
import static com.google.gerrit.server.restapi.project.DashboardsCollection.DEFAULT_DASHBOARD_NAME;
+import static org.apache.http.HttpStatus.SC_METHOD_NOT_ALLOWED;
import static org.apache.http.HttpStatus.SC_NOT_FOUND;
import com.google.common.collect.ImmutableList;
@@ -93,7 +94,11 @@
RestCall.get("/projects/%s/branches/%s"),
RestCall.put("/projects/%s/branches/%s"),
RestCall.get("/projects/%s/branches/%s/mergeable"),
- RestCall.get("/projects/%s/branches/%s/reflog"),
+ RestCall.builder(GET, "/projects/%s/branches/%s/reflog")
+ // The tests use DfsRepository which does not support getting the reflog.
+ .expectedResponseCode(SC_METHOD_NOT_ALLOWED)
+ .expectedMessage("reflog not supported on")
+ .build(),
RestCall.builder(GET, "/projects/%s/branches/%s/files")
// GET /projects/<project>/branches/<branch>/files is not implemented
.expectedResponseCode(SC_NOT_FOUND)
diff --git a/javatests/com/google/gerrit/acceptance/rest/RootCollectionsRestApiBindingsIT.java b/javatests/com/google/gerrit/acceptance/rest/RootCollectionsRestApiBindingsIT.java
index 131ed1a..a2c4ea6 100644
--- a/javatests/com/google/gerrit/acceptance/rest/RootCollectionsRestApiBindingsIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/RootCollectionsRestApiBindingsIT.java
@@ -18,6 +18,7 @@
import static org.apache.http.HttpStatus.SC_NOT_FOUND;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.acceptance.GerritConfig;
import org.junit.Test;
/**
@@ -39,6 +40,7 @@
.expectedResponseCode(SC_NOT_FOUND)
.build(),
RestCall.get("/changes/"),
+ RestCall.post("/changes/"),
RestCall.get("/groups/"),
RestCall.put("/groups/new-group"),
RestCall.get("/plugins/"),
@@ -47,6 +49,7 @@
RestCall.put("/projects/new-project"));
@Test
+ @GerritConfig(name = "plugins.allowRemoteAdmin", value = "true")
public void rootEndpoints() throws Exception {
execute(ROOT_ENDPOINTS);
}
