diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 3a027b1d27..2a4c1412ac 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -1293,6 +1293,34 @@ response "`204 No Content`" is returned. HTTP/1.1 204 No Content ---- +[[put-change-edit-message]] +=== Change commit message in Change Edit +-- +'PUT /changes/link:#change-id[\{change-id\}]/edit:message +-- + +Modify commit message. The request body needs to include a +link:#change-edit-message-input[ChangeEditMessageInput] +entity. + +.Request +---- + PUT /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit:message HTTP/1.0 + Content-Type: application/json;charset=UTF-8 + + { + "message": "New commit message\n\nChange-Id: I10394472cbd17dd12454f229e4f6de00b143a444" + } +---- + +If a change edit doesn't exist for this change yet, it is created. As +response "`204 No Content`" is returned. + +.Response +---- + HTTP/1.1 204 No Content +---- + [[delete-edit-file]] === Delete file in Change Edit -- @@ -1341,6 +1369,30 @@ specified file was deleted in the change edit "`204 No Content`" is returned. RnJvbSA3ZGFkY2MxNTNmZGVhMTdhYTg0ZmYzMmE2ZTI0NWRiYjY... ---- +[[get-edit-message]] +=== Retrieve commit message from Change Edit or current patch set of the change +-- +'GET /changes/link:#change-id[\{change-id\}]/edit:message +-- + +Retrieves commit message from change edit or from current patch set when +change edit doesn't exist. + +.Request +---- + GET /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940/edit:message HTTP/1.0 +---- + +The commit message is returned as base64 encoded string. + +.Response +---- + HTTP/1.1 200 OK + + VGhpcyBpcyBhIGNvbW1pdCBtZXNzYWdlCgpDaGFuZ2UtSWQ6IElhYzhmZGM1MGRlZjFiYWUzYjAz +M2JhNjcxZTk0OTBmNzUxNDU5ZGUzCg== +---- + [[publish-edit]] === Publish Change Edit -- @@ -3912,6 +3964,17 @@ path within change edit. |`restore_path`|optional|Path to file to restore. |=========================== +[[change-edit-message-input]] +=== ChangeEditMessageInput +The `ChangeEditMessageInput` entity contains information for changing +the commit message within a change edit. + +[options="header",cols="1,^1,5"] +|=========================== +|Field Name ||Description +|`message` ||New commit message. +|=========================== + [[check-result]] === CheckResult The `CheckResult` entity contains the results of a consistency check on diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java index d7b10f885a..82165d3b0a 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java @@ -30,12 +30,15 @@ import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.RestSession; +import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.EditInfo; +import com.google.gerrit.extensions.common.ListChangesOption; import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.change.ChangeEdits.EditMessage; import com.google.gerrit.server.change.ChangeEdits.Post; import com.google.gerrit.server.change.ChangeEdits.Put; import com.google.gerrit.server.change.FileContentUtil; @@ -234,6 +237,57 @@ public class ChangeEditIT extends AbstractDaemonTest { assertThat(edit.isPresent()).isFalse(); } + @Test + public void updateMessage() throws Exception { + assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId))) + .isEqualTo(RefUpdate.Result.NEW); + Optional edit = editUtil.byChange(change); + + try { + modifier.modifyMessage( + edit.get(), + edit.get().getEditCommit().getFullMessage()); + fail("InvalidChangeOperationException expected"); + } catch (InvalidChangeOperationException ex) { + assertThat(ex.getMessage()).isEqualTo( + "New commit message cannot be same as existing commit message"); + } + + String msg = String.format("New commit message\n\nChange-Id: %s", + change.getKey()); + assertThat(modifier.modifyMessage(edit.get(), msg)).isEqualTo( + RefUpdate.Result.FORCED); + edit = editUtil.byChange(change); + assertThat(edit.get().getEditCommit().getFullMessage()).isEqualTo(msg); + + editUtil.publish(edit.get()); + assertThat(editUtil.byChange(change).isPresent()).isFalse(); + + ChangeInfo info = get(changeId, ListChangesOption.CURRENT_COMMIT, + ListChangesOption.CURRENT_REVISION); + assertThat(info.revisions.get(info.currentRevision).commit.message) + .isEqualTo(msg); + } + + @Test + public void updateMessageRest() throws Exception { + EditMessage.Input in = new EditMessage.Input(); + in.message = String.format("New commit message\n\nChange-Id: %s", + change.getKey()); + assertThat(adminSession.put(urlEditMessage(), in).getStatusCode()) + .isEqualTo(SC_NO_CONTENT); + Optional edit = editUtil.byChange(change); + assertThat(edit.get().getEditCommit().getFullMessage()) + .isEqualTo(in.message); + in.message = String.format("New commit message2\n\nChange-Id: %s", + change.getKey()); + assertThat(adminSession.put(urlEditMessage(), in).getStatusCode()) + .isEqualTo(SC_NO_CONTENT); + edit = editUtil.byChange(change); + assertThat(edit.get().getEditCommit().getFullMessage()) + .isEqualTo(in.message); + } + @Test public void retrieveEdit() throws Exception { RestResponse r = adminSession.get(urlEdit()); @@ -539,6 +593,12 @@ public class ChangeEditIT extends AbstractDaemonTest { + "/edit/"; } + private String urlEditMessage() { + return "/changes/" + + change.getChangeId() + + "/edit:message"; + } + private String urlEditFile() { return urlEdit() + "/" diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index 071d6c91e4..432a2c3e35 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -450,6 +450,36 @@ public class ChangeUtil { } } + public String getMessage(Change change) + throws NoSuchChangeException, OrmException, + MissingObjectException, IncorrectObjectTypeException, IOException { + Change.Id changeId = change.getId(); + PatchSet ps = db.get().patchSets().get(change.currentPatchSetId()); + if (ps == null) { + throw new NoSuchChangeException(changeId); + } + + Repository git; + try { + git = gitManager.openRepository(change.getProject()); + } catch (RepositoryNotFoundException e) { + throw new NoSuchChangeException(changeId, e); + } + try { + RevWalk revWalk = new RevWalk(git); + try { + RevCommit commit = + revWalk.parseCommit(ObjectId.fromString(ps.getRevision() + .get())); + return commit.getFullMessage(); + } finally { + revWalk.release(); + } + } finally { + git.close(); + } + } + public void deleteDraftChange(Change change) throws NoSuchChangeException, OrmException, IOException { Change.Id changeId = change.getId(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java index d3c07e05a2..94b73a151c 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeEdits.java @@ -24,6 +24,7 @@ import com.google.gerrit.extensions.restapi.AcceptsDelete; import com.google.gerrit.extensions.restapi.AcceptsPost; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; +import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.ChildCollection; import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.IdString; @@ -38,12 +39,14 @@ import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.edit.ChangeEdit; import com.google.gerrit.server.edit.ChangeEditJson; import com.google.gerrit.server.edit.ChangeEditModifier; import com.google.gerrit.server.edit.ChangeEditUtil; import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.project.InvalidChangeOperationException; +import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -434,4 +437,72 @@ public class ChangeEdits implements } } } + + @Singleton + public static class EditMessage implements + RestModifyView { + public static class Input { + @DefaultInput + public String message; + } + + private final Provider db; + private final ChangeEditModifier editModifier; + private final ChangeEditUtil editUtil; + + @Inject + EditMessage(Provider db, + ChangeEditModifier editModifier, + ChangeEditUtil editUtil) { + this.db = db; + this.editModifier = editModifier; + this.editUtil = editUtil; + } + + @Override + public Object apply(ChangeResource rsrc, Input input) throws AuthException, + IOException, InvalidChangeOperationException, BadRequestException, + ResourceConflictException, OrmException { + Optional edit = editUtil.byChange(rsrc.getChange()); + if (!edit.isPresent()) { + editModifier.createEdit(rsrc.getChange(), + db.get().patchSets().get(rsrc.getChange().currentPatchSetId())); + edit = editUtil.byChange(rsrc.getChange()); + } + + if (input == null || Strings.isNullOrEmpty(input.message)) { + throw new BadRequestException("commit message must be provided"); + } + + editModifier.modifyMessage(edit.get(), input.message); + return Response.none(); + } + } + + @Singleton + public static class GetMessage implements RestReadView { + private final ChangeUtil changeUtil; + private final ChangeEditUtil editUtil; + + @Inject + GetMessage(ChangeUtil changeUtil, + ChangeEditUtil editUtil) { + this.changeUtil = changeUtil; + this.editUtil = editUtil; + } + + @Override + public BinaryResult apply(ChangeResource rsrc) throws AuthException, IOException, + OrmException, NoSuchChangeException { + Optional edit = editUtil.byChange(rsrc.getChange()); + // TODO(davido): Clean this up by returning 404 when edit doesn't exist. + // Client should call GET /changes/{id}/revisions/current/commit in this + // case; or, to be consistent with GET content logic, the client could + // call directly the right endpoint. + String m = edit.isPresent() + ? edit.get().getEditCommit().getFullMessage() + : changeUtil.getMessage(rsrc.getChange()); + return BinaryResult.create(m).base64(); + } + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index ce38778523..d2d3db342e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -108,6 +108,8 @@ public class Module extends RestApiModule { delete(CHANGE_KIND, "edit").to(DeleteChangeEdit.class); child(CHANGE_KIND, "edit:publish").to(PublishChangeEdit.class); child(CHANGE_KIND, "edit:rebase").to(RebaseChangeEdit.class); + put(CHANGE_KIND, "edit:message").to(ChangeEdits.EditMessage.class); + get(CHANGE_KIND, "edit:message").to(ChangeEdits.GetMessage.class); put(CHANGE_EDIT_KIND, "/").to(ChangeEdits.Put.class); delete(CHANGE_EDIT_KIND).to(ChangeEdits.DeleteContent.class); get(CHANGE_EDIT_KIND, "/").to(ChangeEdits.Get.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java index 94797fc69e..a3829f9f7a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java @@ -15,10 +15,12 @@ package com.google.gerrit.server.edit; import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; import static com.google.gerrit.server.edit.ChangeEditUtil.editRefName; import static com.google.gerrit.server.edit.ChangeEditUtil.editRefPrefix; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; +import com.google.common.base.Strings; import com.google.common.io.ByteStreams; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; @@ -216,6 +218,55 @@ public class ChangeEditModifier { } } + /** + * Modify commit message in existing change edit. + * + * @param edit change edit + * @param msg new commit message + * @return result + * @throws AuthException + * @throws InvalidChangeOperationException + * @throws IOException + */ + public RefUpdate.Result modifyMessage(ChangeEdit edit, String msg) + throws AuthException, InvalidChangeOperationException, IOException { + checkState(!Strings.isNullOrEmpty(msg), "message cannot be null"); + if (!currentUser.get().isIdentifiedUser()) { + throw new AuthException("Authentication required"); + } + RevCommit prevEdit = edit.getEditCommit(); + if (prevEdit.getParentCount() == 0) { + throw new InvalidChangeOperationException( + "Modify edit against root commit not supported"); + } + + if (prevEdit.getFullMessage().equals(msg)) { + throw new InvalidChangeOperationException( + "New commit message cannot be same as existing commit message"); + } + + IdentifiedUser me = (IdentifiedUser) currentUser.get(); + Repository repo = gitManager.openRepository(edit.getChange().getProject()); + try { + RevWalk rw = new RevWalk(repo); + ObjectInserter inserter = repo.newObjectInserter(); + try { + String refName = edit.getRefName(); + ObjectId commit = createCommit(me, inserter, prevEdit, + rw.parseCommit(prevEdit.getParent(0)), + prevEdit.getTree(), + msg); + inserter.flush(); + return update(repo, me, refName, rw, prevEdit, commit); + } finally { + rw.release(); + inserter.release(); + } + } finally { + repo.close(); + } + } + /** * Modify file in existing change edit from its base commit. * @@ -322,12 +373,19 @@ public class ChangeEditModifier { private ObjectId createCommit(IdentifiedUser me, ObjectInserter inserter, RevCommit prevEdit, RevCommit base, ObjectId tree) throws IOException { + return createCommit(me, inserter, prevEdit, base, tree, + prevEdit.getFullMessage()); + } + + private ObjectId createCommit(IdentifiedUser me, ObjectInserter inserter, + RevCommit prevEdit, RevCommit base, ObjectId tree, String msg) + throws IOException { CommitBuilder builder = new CommitBuilder(); builder.setTreeId(tree); builder.setParentIds(base); builder.setAuthor(prevEdit.getAuthorIdent()); builder.setCommitter(getCommitterIdent(me)); - builder.setMessage(prevEdit.getFullMessage()); + builder.setMessage(msg); return inserter.insert(builder); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java index 2c83121f4d..a8c543c125 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java @@ -224,8 +224,9 @@ public class ChangeEditUtil { throws IOException, ResourceConflictException { RevCommit parent = rw.parseCommit(ObjectId.fromString( basePatchSet.getRevision().get())); - if (parent.getTree().equals(edit.getTree())) { - throw new ResourceConflictException("identical tree"); + if (parent.getTree().equals(edit.getTree()) + && edit.getFullMessage().equals(parent.getFullMessage())) { + throw new ResourceConflictException("identical tree and message"); } return writeSquashedCommit(rw, inserter, parent, edit); } @@ -278,7 +279,7 @@ public class ChangeEditUtil { mergeCommit.addParentId(parent.getParent(i)); } mergeCommit.setAuthor(parent.getAuthorIdent()); - mergeCommit.setMessage(parent.getFullMessage()); + mergeCommit.setMessage(edit.getFullMessage()); mergeCommit.setCommitter(edit.getCommitterIdent()); mergeCommit.setTreeId(edit.getTree());