Merge changes from topic 'inline-3'

* changes:
  InlineEdit: Add REST endpoint to retrieve commit message
  InlineEdit: Add REST endpoint to modify commit message
  InlineEdit: Add support for changing commit message
This commit is contained in:
David Pursehouse 2014-12-01 06:31:56 +00:00 committed by Gerrit Code Review
commit 87c5cd9fe2
7 changed files with 289 additions and 4 deletions

View File

@ -1293,6 +1293,34 @@ response "`204 No Content`" is returned.
HTTP/1.1 204 No Content 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-edit-file]]
=== Delete file in Change Edit === Delete file in Change Edit
-- --
@ -1341,6 +1369,30 @@ specified file was deleted in the change edit "`204 No Content`" is returned.
RnJvbSA3ZGFkY2MxNTNmZGVhMTdhYTg0ZmYzMmE2ZTI0NWRiYjY... 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-edit]]
=== Publish Change Edit === Publish Change Edit
-- --
@ -3912,6 +3964,17 @@ path within change edit.
|`restore_path`|optional|Path to file to restore. |`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]] [[check-result]]
=== CheckResult === CheckResult
The `CheckResult` entity contains the results of a consistency check on The `CheckResult` entity contains the results of a consistency check on

View File

@ -30,12 +30,15 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.RestResponse; import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.acceptance.RestSession; 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.EditInfo;
import com.google.gerrit.extensions.common.ListChangesOption;
import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.BinaryResult;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb; 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.Post;
import com.google.gerrit.server.change.ChangeEdits.Put; import com.google.gerrit.server.change.ChangeEdits.Put;
import com.google.gerrit.server.change.FileContentUtil; import com.google.gerrit.server.change.FileContentUtil;
@ -234,6 +237,57 @@ public class ChangeEditIT extends AbstractDaemonTest {
assertThat(edit.isPresent()).isFalse(); assertThat(edit.isPresent()).isFalse();
} }
@Test
public void updateMessage() throws Exception {
assertThat(modifier.createEdit(change, getCurrentPatchSet(changeId)))
.isEqualTo(RefUpdate.Result.NEW);
Optional<ChangeEdit> 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<ChangeEdit> 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 @Test
public void retrieveEdit() throws Exception { public void retrieveEdit() throws Exception {
RestResponse r = adminSession.get(urlEdit()); RestResponse r = adminSession.get(urlEdit());
@ -539,6 +593,12 @@ public class ChangeEditIT extends AbstractDaemonTest {
+ "/edit/"; + "/edit/";
} }
private String urlEditMessage() {
return "/changes/"
+ change.getChangeId()
+ "/edit:message";
}
private String urlEditFile() { private String urlEditFile() {
return urlEdit() return urlEdit()
+ "/" + "/"

View File

@ -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) public void deleteDraftChange(Change change)
throws NoSuchChangeException, OrmException, IOException { throws NoSuchChangeException, OrmException, IOException {
Change.Id changeId = change.getId(); Change.Id changeId = change.getId();

View File

@ -24,6 +24,7 @@ import com.google.gerrit.extensions.restapi.AcceptsDelete;
import com.google.gerrit.extensions.restapi.AcceptsPost; import com.google.gerrit.extensions.restapi.AcceptsPost;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException; 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.ChildCollection;
import com.google.gerrit.extensions.restapi.DefaultInput; import com.google.gerrit.extensions.restapi.DefaultInput;
import com.google.gerrit.extensions.restapi.IdString; 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.Change;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb; 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.ChangeEdit;
import com.google.gerrit.server.edit.ChangeEditJson; import com.google.gerrit.server.edit.ChangeEditJson;
import com.google.gerrit.server.edit.ChangeEditModifier; import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.edit.ChangeEditUtil; import com.google.gerrit.server.edit.ChangeEditUtil;
import com.google.gerrit.server.patch.PatchListNotAvailableException; import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
@ -434,4 +437,72 @@ public class ChangeEdits implements
} }
} }
} }
@Singleton
public static class EditMessage implements
RestModifyView<ChangeResource, EditMessage.Input> {
public static class Input {
@DefaultInput
public String message;
}
private final Provider<ReviewDb> db;
private final ChangeEditModifier editModifier;
private final ChangeEditUtil editUtil;
@Inject
EditMessage(Provider<ReviewDb> 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<ChangeEdit> 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<ChangeResource> {
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<ChangeEdit> 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();
}
}
} }

View File

@ -108,6 +108,8 @@ public class Module extends RestApiModule {
delete(CHANGE_KIND, "edit").to(DeleteChangeEdit.class); delete(CHANGE_KIND, "edit").to(DeleteChangeEdit.class);
child(CHANGE_KIND, "edit:publish").to(PublishChangeEdit.class); child(CHANGE_KIND, "edit:publish").to(PublishChangeEdit.class);
child(CHANGE_KIND, "edit:rebase").to(RebaseChangeEdit.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); put(CHANGE_EDIT_KIND, "/").to(ChangeEdits.Put.class);
delete(CHANGE_EDIT_KIND).to(ChangeEdits.DeleteContent.class); delete(CHANGE_EDIT_KIND).to(ChangeEdits.DeleteContent.class);
get(CHANGE_EDIT_KIND, "/").to(ChangeEdits.Get.class); get(CHANGE_EDIT_KIND, "/").to(ChangeEdits.Get.class);

View File

@ -15,10 +15,12 @@
package com.google.gerrit.server.edit; package com.google.gerrit.server.edit;
import static com.google.common.base.Preconditions.checkNotNull; 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.editRefName;
import static com.google.gerrit.server.edit.ChangeEditUtil.editRefPrefix; import static com.google.gerrit.server.edit.ChangeEditUtil.editRefPrefix;
import static org.eclipse.jgit.lib.Constants.OBJ_BLOB; import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
import com.google.common.base.Strings;
import com.google.common.io.ByteStreams; import com.google.common.io.ByteStreams;
import com.google.gerrit.common.Nullable; import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.TimeUtil; 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. * Modify file in existing change edit from its base commit.
* *
@ -322,12 +373,19 @@ public class ChangeEditModifier {
private ObjectId createCommit(IdentifiedUser me, ObjectInserter inserter, private ObjectId createCommit(IdentifiedUser me, ObjectInserter inserter,
RevCommit prevEdit, RevCommit base, ObjectId tree) throws IOException { 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(); CommitBuilder builder = new CommitBuilder();
builder.setTreeId(tree); builder.setTreeId(tree);
builder.setParentIds(base); builder.setParentIds(base);
builder.setAuthor(prevEdit.getAuthorIdent()); builder.setAuthor(prevEdit.getAuthorIdent());
builder.setCommitter(getCommitterIdent(me)); builder.setCommitter(getCommitterIdent(me));
builder.setMessage(prevEdit.getFullMessage()); builder.setMessage(msg);
return inserter.insert(builder); return inserter.insert(builder);
} }

View File

@ -224,8 +224,9 @@ public class ChangeEditUtil {
throws IOException, ResourceConflictException { throws IOException, ResourceConflictException {
RevCommit parent = rw.parseCommit(ObjectId.fromString( RevCommit parent = rw.parseCommit(ObjectId.fromString(
basePatchSet.getRevision().get())); basePatchSet.getRevision().get()));
if (parent.getTree().equals(edit.getTree())) { if (parent.getTree().equals(edit.getTree())
throw new ResourceConflictException("identical tree"); && edit.getFullMessage().equals(parent.getFullMessage())) {
throw new ResourceConflictException("identical tree and message");
} }
return writeSquashedCommit(rw, inserter, parent, edit); return writeSquashedCommit(rw, inserter, parent, edit);
} }
@ -278,7 +279,7 @@ public class ChangeEditUtil {
mergeCommit.addParentId(parent.getParent(i)); mergeCommit.addParentId(parent.getParent(i));
} }
mergeCommit.setAuthor(parent.getAuthorIdent()); mergeCommit.setAuthor(parent.getAuthorIdent());
mergeCommit.setMessage(parent.getFullMessage()); mergeCommit.setMessage(edit.getFullMessage());
mergeCommit.setCommitter(edit.getCommitterIdent()); mergeCommit.setCommitter(edit.getCommitterIdent());
mergeCommit.setTreeId(edit.getTree()); mergeCommit.setTreeId(edit.getTree());