Make submit endpoints consistently return ChangeInfo
So far, the CHANGE_KIND submit endpoint returns ChangeInfo, while the
REVISION_KIND submit endpoint returns nothing. Change the revision
endpoint to be consistent and return ChangeInfo.
While at it, we can get rid of some strange "Output" object and clean
the code there a bit.
This change breaks the implementors of RevisionApi, which means it
should be mentioned in the release notes.
Change-Id: I47b370242c1cd5f4888abfed825a3728f07cbc96
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index c0a6c5e..f5fe6f0 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -4412,6 +4412,33 @@
POST /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/revisions/674ac754f91e64a0efb8087e59a176484bd534d1/submit HTTP/1.0
----
+As response a link:#change-info[ChangeInfo] entity is returned that
+describes the submitted/merged change.
+
+.Response
+----
+ HTTP/1.1 200 OK
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+
+ )]}'
+ {
+ "id": "myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940",
+ "project": "myProject",
+ "branch": "master",
+ "change_id": "I8473b95934b5732ac55d26311a706c9c2bde9940",
+ "subject": "Implementing Feature X",
+ "status": "MERGED",
+ "created": "2013-02-01 09:59:32.126000000",
+ "updated": "2013-02-21 11:16:36.775000000",
+ "submitted": "2013-02-21 11:16:36.615000000",
+ "_number": 3965,
+ "owner": {
+ "name": "John Doe"
+ }
+ }
+----
+
If the revision cannot be submitted, e.g. because the submit rule
doesn't allow submitting the revision or the revision is not the
current revision, the response is "`409 Conflict`" and the error
diff --git a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
index 73e6a4e..229b9d4 100644
--- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
+++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java
@@ -44,12 +44,12 @@
ReviewResult review(ReviewInput in) throws RestApiException;
- default void submit() throws RestApiException {
+ default ChangeInfo submit() throws RestApiException {
SubmitInput in = new SubmitInput();
- submit(in);
+ return submit(in);
}
- void submit(SubmitInput in) throws RestApiException;
+ ChangeInfo submit(SubmitInput in) throws RestApiException;
default BinaryResult submitPreview() throws RestApiException {
return submitPreview("zip");
@@ -200,7 +200,7 @@
}
@Override
- public void submit(SubmitInput in) throws RestApiException {
+ public ChangeInfo submit(SubmitInput in) throws RestApiException {
throw new NotImplementedException();
}
diff --git a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
index 573f2f5..ab96c6b 100644
--- a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
+++ b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java
@@ -261,9 +261,9 @@
}
@Override
- public void submit(SubmitInput in) throws RestApiException {
+ public ChangeInfo submit(SubmitInput in) throws RestApiException {
try {
- submit.apply(revision, in);
+ return submit.apply(revision, in).value();
} catch (Exception e) {
throw asRestApiException("Cannot submit change", e);
}
diff --git a/java/com/google/gerrit/server/restapi/change/Submit.java b/java/com/google/gerrit/server/restapi/change/Submit.java
index 801340b..d709f6b 100644
--- a/java/com/google/gerrit/server/restapi/change/Submit.java
+++ b/java/com/google/gerrit/server/restapi/change/Submit.java
@@ -102,14 +102,6 @@
private static final String CLICK_FAILURE_TOOLTIP = "Clicking the button would fail";
private static final String CHANGE_UNMERGEABLE = "Problems with integrating this change";
- public static class Output {
- transient Change change;
-
- private Output(Change c) {
- change = c;
- }
- }
-
private final GitRepositoryManager repoManager;
private final PermissionBackend permissionBackend;
private final ChangeData.Factory changeDataFactory;
@@ -126,6 +118,7 @@
private final Provider<InternalChangeQuery> queryProvider;
private final PatchSetUtil psUtil;
private final ProjectCache projectCache;
+ private final ChangeJson.Factory json;
@Inject
Submit(
@@ -138,7 +131,8 @@
@GerritServerConfig Config cfg,
Provider<InternalChangeQuery> queryProvider,
PatchSetUtil psUtil,
- ProjectCache projectCache) {
+ ProjectCache projectCache,
+ ChangeJson.Factory json) {
this.repoManager = repoManager;
this.permissionBackend = permissionBackend;
this.changeDataFactory = changeDataFactory;
@@ -173,10 +167,11 @@
this.queryProvider = queryProvider;
this.psUtil = psUtil;
this.projectCache = projectCache;
+ this.json = json;
}
@Override
- public Response<Output> apply(RevisionResource rsrc, SubmitInput input)
+ public Response<ChangeInfo> apply(RevisionResource rsrc, SubmitInput input)
throws RestApiException, RepositoryNotFoundException, IOException, PermissionBackendException,
UpdateException, ConfigInvalidException {
input.onBehalfOf = Strings.emptyToNull(input.onBehalfOf);
@@ -192,12 +187,11 @@
.orElseThrow(illegalState(rsrc.getProject()))
.checkStatePermitsWrite();
- return mergeChange(rsrc, submitter, input);
+ return Response.ok(json.noOptions().format(mergeChange(rsrc, submitter, input)));
}
@UsedAt(UsedAt.Project.GOOGLE)
- public Response<Output> mergeChange(
- RevisionResource rsrc, IdentifiedUser submitter, SubmitInput input)
+ public Change mergeChange(RevisionResource rsrc, IdentifiedUser submitter, SubmitInput input)
throws RestApiException, IOException, UpdateException, ConfigInvalidException,
PermissionBackendException {
Change change = rsrc.getChange();
@@ -218,7 +212,7 @@
updatedChange = op.merge(change, submitter, true, input, false);
if (updatedChange.isMerged()) {
- return Response.ok(new Output(updatedChange));
+ return updatedChange;
}
throw new IllegalStateException(
@@ -474,13 +468,11 @@
public static class CurrentRevision implements RestModifyView<ChangeResource, SubmitInput> {
private final Submit submit;
- private final ChangeJson.Factory json;
private final PatchSetUtil psUtil;
@Inject
- CurrentRevision(Submit submit, ChangeJson.Factory json, PatchSetUtil psUtil) {
+ CurrentRevision(Submit submit, PatchSetUtil psUtil) {
this.submit = submit;
- this.json = json;
this.psUtil = psUtil;
}
@@ -491,8 +483,7 @@
throw new ResourceConflictException("current revision is missing");
}
- Response<Output> response = submit.apply(new RevisionResource(rsrc, ps), input);
- return Response.ok(json.noOptions().format(response.value().change));
+ return submit.apply(new RevisionResource(rsrc, ps), input);
}
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
index abfd7896..f0cdc1d 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RevisionIT.java
@@ -152,7 +152,8 @@
PushOneCommit.Result r = createChange();
String changeId = project.get() + "~master~" + r.getChangeId();
gApi.changes().id(changeId).current().review(ReviewInput.approve());
- gApi.changes().id(changeId).current().submit();
+ ChangeInfo changeInfo = gApi.changes().id(changeId).current().submit();
+ assertThat(changeInfo.status).isEqualTo(ChangeStatus.MERGED);
assertThat(gApi.changes().id(changeId).get().status).isEqualTo(ChangeStatus.MERGED);
}