opendev/gerrit
Code Issues Proposed changes

Make edit's parent the base of the change

Browse Source 
Change-Id: I53186d485638e9c761720630149e575a461f2001
This commit is contained in:
David Ostrovsky 2014-09-11 13:33:45 +02:00
parent 0f8fcab555
commit 0619f65b17
9 changed files with 99 additions and 104 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
Documentation/rest-api-changes.txt
View File

@ -3834,6 +3834,7 @@ The `EditInfo` entity contains information about a change edit.
|Field Name ||Description
|`commit` ||The commit of change edit as
link:#commit-info[CommitInfo] entity.
|`baseRevision`||The revision of the patch set change edit is based on.
|`actions` ||
Actions the caller might be able to perform on this change edit. The
information is a map of view name to link:#action-info[ActionInfo]

37
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/edit/ChangeEditIT.java
View File

@ -392,43 +392,6 @@ public class ChangeEditIT extends AbstractDaemonTest {
}
}
@Test
public void restoreDeletedFileInEdit() throws Exception {
assertEquals(RefUpdate.Result.NEW,
modifier.createEdit(
change,
ps));
Optional<ChangeEdit> edit = editUtil.byChange(change);
assertEquals(RefUpdate.Result.FORCED,
modifier.modifyFile(
edit.get(),
FILE_NAME,
CONTENT_NEW));
edit = editUtil.byChange(change);
assertArrayEquals(CONTENT_NEW,
toBytes(fileUtil.getContent(edit.get().getChange().getProject(),
edit.get().getRevision().get(), FILE_NAME)));
assertEquals(RefUpdate.Result.FORCED,
modifier.deleteFile(
edit.get(),
FILE_NAME));
edit = editUtil.byChange(change);
try {
fileUtil.getContent(edit.get().getChange().getProject(),
edit.get().getRevision().get(), FILE_NAME);
fail("ResourceNotFoundException expected");
} catch (ResourceNotFoundException rnfe) {
}
assertEquals(RefUpdate.Result.FORCED,
modifier.restoreFile(
edit.get(),
FILE_NAME));
edit = editUtil.byChange(change);
assertArrayEquals(CONTENT_OLD,
toBytes(fileUtil.getContent(edit.get().getChange().getProject(),
edit.get().getRevision().get(), FILE_NAME)));
}
@Test
public void restoreDeletedFileInPatchSet() throws Exception {
assertEquals(RefUpdate.Result.NEW,

1
gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/EditInfo.java
View File

@ -18,6 +18,7 @@ import java.util.Map;
public class EditInfo {
public CommitInfo commit;
public String baseRevision;
public Map<String, ActionInfo> actions;
public Map<String, FetchInfo> fetch;
public Map<String, FileInfo> files;

7
gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeInfo.java
View File

@ -212,6 +212,7 @@ public class ChangeInfo extends JavaScriptObject {
public static class EditInfo extends JavaScriptObject {
public final native String name() /*-{ return this.name; }-*/;
public final native String set_name(String n) /*-{ this.name = n; }-*/;
public final native String base_revision() /*-{ return this.base_revision; }-*/;
public final native CommitInfo commit() /*-{ return this.commit; }-*/;
public final native boolean has_actions() /*-{ return this.hasOwnProperty('actions') }-*/;
@ -237,6 +238,7 @@ public class ChangeInfo extends JavaScriptObject {
this._number = 0;
this.name = edit.name;
this.commit = edit.commit;
this.edit_base = edit.base_revision;
}-*/;
public final native int _number() /*-{ return this._number; }-*/;
public final native String name() /*-{ return this.name; }-*/;
@ -245,6 +247,7 @@ public class ChangeInfo extends JavaScriptObject {
public final native boolean is_edit() /*-{ return this._number == 0; }-*/;
public final native CommitInfo commit() /*-{ return this.commit; }-*/;
public final native void set_commit(CommitInfo c) /*-{ this.commit = c; }-*/;
public final native String edit_base() /*-{ return this.edit_base; }-*/;
public final native boolean has_files() /*-{ return this.hasOwnProperty('files') }-*/;
public final native NativeMap<FileInfo> files() /*-{ return this.files; }-*/;
@ -275,9 +278,7 @@ public class ChangeInfo extends JavaScriptObject {
// edit under revisions?
RevisionInfo editInfo = list.get(i);
if (editInfo.is_edit()) {
// parent commit is parent patch set revision
CommitInfo commit = editInfo.commit().parents().get(0);
String parentRevision = commit.commit();
String parentRevision = editInfo.edit_base();
// find parent
for (int j = 0; j < list.length(); j++) {
RevisionInfo parentInfo = list.get(j);

8
gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEdit.java
View File

@ -29,8 +29,9 @@ import org.eclipse.jgit.revwalk.RevCommit;
* A single user's edit for a change.
* <p>
* There is max. one edit per user per change. Edits are stored on refs:
* refs/users/UU/UUUU/edit-CCCC where UU/UUUU is sharded representation
* of user account and CCCC is change number.
* refs/users/UU/UUUU/edit-CCCC/P where UU/UUUU is sharded representation
* of user account, CCCC is change number and P is the patch set number it
* is based on.
*/
public class ChangeEdit {
private final IdentifiedUser user;
@ -70,7 +71,8 @@ public class ChangeEdit {
}
public String getRefName() {
return ChangeEditUtil.editRefName(user.getAccountId(), change.getId());
return ChangeEditUtil.editRefName(user.getAccountId(), change.getId(),
basePatchSet.getId());
}
public RevCommit getEditCommit() {

1
gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditJson.java
View File

@ -57,6 +57,7 @@ public class ChangeEditJson {
throws IOException {
EditInfo out = new EditInfo();
out.commit = fillCommit(edit.getEditCommit());
out.baseRevision = edit.getBasePatchSet().getRevision().get();
out.actions = fillActions(edit);
if (downloadCommands) {
out.fetch = fillFetchMap(edit);

71
gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditModifier.java
View File

@ -15,6 +15,7 @@
package com.google.gerrit.server.edit;
import static com.google.gerrit.server.edit.ChangeEditUtil.editRefName;
import static com.google.gerrit.server.edit.ChangeEditUtil.editRefPrefix;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@ -37,22 +38,27 @@ import org.eclipse.jgit.dircache.DirCacheEditor;
import org.eclipse.jgit.dircache.DirCacheEditor.DeletePath;
import org.eclipse.jgit.dircache.DirCacheEditor.PathEdit;
import org.eclipse.jgit.dircache.DirCacheEntry;
import org.eclipse.jgit.lib.BatchRefUpdate;
import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.FileMode;
import org.eclipse.jgit.lib.NullProgressMonitor;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.merge.ThreeWayMerger;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.treewalk.TreeWalk;
import java.io.IOException;
import java.util.Map;
import java.util.TimeZone;
/**
@ -104,10 +110,11 @@ public class ChangeEditModifier {
IdentifiedUser me = (IdentifiedUser) currentUser.get();
Repository repo = gitManager.openRepository(change.getProject());
String refName = editRefName(me.getAccountId(), change.getId());
String refPrefix = editRefPrefix(me.getAccountId(), change.getId());
try {
if (repo.getRefDatabase().getRef(refName) != null) {
Map<String, Ref> refs = repo.getRefDatabase().getRefs(refPrefix);
if (!refs.isEmpty()) {
throw new ResourceConflictException("edit already exists");
}
@ -116,9 +123,12 @@ public class ChangeEditModifier {
try {
RevCommit base = rw.parseCommit(ObjectId.fromString(
ps.getRevision().get()));
ObjectId commit = createCommit(me, inserter, base, base, base.getTree());
RevCommit changeBase = base.getParent(0);
ObjectId commit = createCommit(me, inserter, base, changeBase, base.getTree());
inserter.flush();
return update(repo, me, refName, rw, ObjectId.zeroId(), commit);
String editRefName = editRefName(me.getAccountId(), change.getId(),
ps.getId());
return update(repo, me, editRefName, rw, ObjectId.zeroId(), commit);
} finally {
rw.release();
inserter.release();
@ -137,7 +147,7 @@ public class ChangeEditModifier {
* @throws InvalidChangeOperationException
* @throws IOException
*/
public RefUpdate.Result rebaseEdit(ChangeEdit edit, PatchSet current)
public void rebaseEdit(ChangeEdit edit, PatchSet current)
throws AuthException, InvalidChangeOperationException, IOException {
if (!currentUser.get().isIdentifiedUser()) {
throw new AuthException("Authentication required");
@ -145,10 +155,12 @@ public class ChangeEditModifier {
Change change = edit.getChange();
IdentifiedUser me = (IdentifiedUser) currentUser.get();
String refName = editRefName(me.getAccountId(), change.getId());
String refName = editRefName(me.getAccountId(), change.getId(),
current.getId());
Repository repo = gitManager.openRepository(change.getProject());
try {
RevWalk rw = new RevWalk(repo);
BatchRefUpdate ru = repo.getRefDatabase().newBatchUpdate();
ObjectInserter inserter = repo.newObjectInserter();
try {
RevCommit editCommit = edit.getEditCommit();
@ -156,24 +168,38 @@ public class ChangeEditModifier {
throw new InvalidChangeOperationException(
"Rebase edit against root commit not implemented");
}
RevCommit mergeTip = rw.parseCommit(ObjectId.fromString(
RevCommit tip = rw.parseCommit(ObjectId.fromString(
current.getRevision().get()));
ThreeWayMerger m = MergeStrategy.RESOLVE.newMerger(repo, true);
m.setObjectInserter(inserter);
m.setBase(editCommit.getParent(0));
if (m.merge(mergeTip, editCommit)) {
m.setBase(ObjectId.fromString(
edit.getBasePatchSet().getRevision().get()));
if (m.merge(tip, editCommit)) {
ObjectId tree = m.getResultTreeId();
CommitBuilder mergeCommit = new CommitBuilder();
mergeCommit.setTreeId(tree);
mergeCommit.setParentId(mergeTip);
mergeCommit.setAuthor(editCommit.getAuthorIdent());
mergeCommit.setCommitter(new PersonIdent(
CommitBuilder commit = new CommitBuilder();
commit.setTreeId(tree);
for (int i = 0; i < tip.getParentCount(); i++) {
commit.addParentId(tip.getParent(i));
}
commit.setAuthor(editCommit.getAuthorIdent());
commit.setCommitter(new PersonIdent(
editCommit.getCommitterIdent(), TimeUtil.nowTs()));
mergeCommit.setMessage(editCommit.getFullMessage());
ObjectId newEdit = inserter.insert(mergeCommit);
commit.setMessage(editCommit.getFullMessage());
ObjectId newEdit = inserter.insert(commit);
inserter.flush();
return update(repo, me, refName, rw, editCommit, newEdit);
ru.addCommand(new ReceiveCommand(ObjectId.zeroId(), newEdit,
refName));
ru.addCommand(new ReceiveCommand(edit.getRef().getObjectId(),
ObjectId.zeroId(), edit.getRefName()));
ru.execute(rw, NullProgressMonitor.INSTANCE);
for (ReceiveCommand cmd : ru.getCommands()) {
if (cmd.getResult() != ReceiveCommand.Result.OK) {
throw new IOException("failed: " + cmd);
}
}
} else {
// TODO(davido): Allow to resolve conflicts inline
throw new InvalidChangeOperationException("merge conflict");
@ -250,15 +276,14 @@ public class ChangeEditModifier {
ObjectReader reader = repo.newObjectReader();
try {
String refName = edit.getRefName();
RevCommit prevEdit = rw.parseCommit(edit.getRef().getObjectId());
PatchSet basePs = edit.getBasePatchSet();
RevCommit base = rw.parseCommit(ObjectId.fromString(
basePs.getRevision().get()));
if (base.getParentCount() == 0) {
RevCommit prevEdit = edit.getEditCommit();
if (prevEdit.getParentCount() == 0) {
throw new InvalidChangeOperationException(
"Modify edit against root commit not implemented");
}
RevCommit base = prevEdit.getParent(0);
base = rw.parseCommit(base);
ObjectId newTree = writeNewTree(op, repo, rw, inserter,
prevEdit, reader, file, content, base);
if (ObjectId.equals(newTree, prevEdit.getTree())) {

71
gerrit-server/src/main/java/com/google/gerrit/server/edit/ChangeEditUtil.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.server.edit;
import static com.google.common.base.Preconditions.checkArgument;
import com.google.common.base.Optional;
import com.google.common.collect.Iterables;
import com.google.gerrit.extensions.restapi.AuthException;
@ -47,7 +49,7 @@ import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import java.io.IOException;
import java.util.List;
import java.util.Map;
/**
* Utility functions to manipulate change edits.
@ -93,15 +95,20 @@ public class ChangeEditUtil {
Repository repo = gitManager.openRepository(change.getProject());
try {
IdentifiedUser me = (IdentifiedUser) user.get();
Ref ref = repo.getRefDatabase().getRef(editRefName(
me.getAccountId(), change.getId()));
if (ref == null) {
String editRefPrefix = editRefPrefix(me.getAccountId(), change.getId());
Map<String, Ref> refs = repo.getRefDatabase().getRefs(editRefPrefix);
if (refs.isEmpty()) {
return Optional.absent();
}
// TODO(davido): Rather than failing when we encounter the corrupt state
// where there is more than one ref, we could silently delete all but the
// current one.
Ref ref = Iterables.getOnlyElement(refs.values());
RevWalk rw = new RevWalk(repo);
try {
RevCommit commit = rw.parseCommit(ref.getObjectId());
PatchSet basePs = getBasePatchSet(change, commit);
PatchSet basePs = getBasePatchSet(change, ref);
return Optional.of(new ChangeEdit(me, change, ref, commit, basePs));
} finally {
rw.release();
@ -171,36 +178,14 @@ public class ChangeEditUtil {
}
}
private PatchSet getBasePatchSet(Change change, RevCommit commit)
private PatchSet getBasePatchSet(Change change, Ref ref)
throws IOException, InvalidChangeOperationException {
if (commit.getParentCount() != 1) {
throw new InvalidChangeOperationException(
"change edit commit has multiple parents");
}
RevCommit parentCommit = commit.getParent(0);
ObjectId rev = parentCommit.getId();
RevId parentRev = new RevId(ObjectId.toString(rev));
try {
List<PatchSet> r = db.get().patchSets().byRevision(parentRev).toList();
if (r.isEmpty()) {
throw new InvalidChangeOperationException(String.format(
"patch set %s change edit is based on doesn't exist",
rev.abbreviate(8).name()));
}
if (r.size() > 1) {
throw new InvalidChangeOperationException(String.format(
"multiple patch sets for change edit parent %s",
rev.abbreviate(8).name()));
}
PatchSet parentPatchSet = Iterables.getOnlyElement(r);
if (!change.getId().equals(
parentPatchSet.getId().getParentKey())) {
throw new InvalidChangeOperationException(String.format(
"different change edit ID %d and its parent patch set %d",
change.getId().get(),
parentPatchSet.getId().getParentKey().get()));
}
return parentPatchSet;
int pos = ref.getName().lastIndexOf("/");
checkArgument(pos > 0, "invalid edit ref: %s", ref.getName());
String psId = ref.getName().substring(pos + 1);
return db.get().patchSets().get(new PatchSet.Id(
change.getId(), Integer.valueOf(psId)));
} catch (OrmException e) {
throw new IOException(e);
}
@ -208,14 +193,28 @@ public class ChangeEditUtil {
/**
* Returns reference for this change edit with sharded user and change number:
* refs/users/UU/UUUU/edit-CCCC.
* refs/users/UU/UUUU/edit-CCCC/P.
*
* @param accountId accout id
* @param changeId change number
* @param psId patch set number
* @return reference for this change edit
*/
static String editRefName(Account.Id accountId, Change.Id changeId) {
return String.format("%s/edit-%d",
static String editRefName(Account.Id accountId, Change.Id changeId,
PatchSet.Id psId) {
return editRefPrefix(accountId, changeId) + psId.get();
}
/**
* Returns reference prefix for this change edit with sharded user and
* change number: refs/users/UU/UUUU/edit-CCCC/.
*
* @param accountId accout id
* @param changeId change number
* @return reference prefix for this change edit
*/
static String editRefPrefix(Account.Id accountId, Change.Id changeId) {
return String.format("%s/edit-%d/",
RefNames.refsUsers(accountId),
changeId.get());
}

6
gerrit-server/src/test/java/com/google/gerrit/server/edit/ChangeEditTest.java
View File

@ -18,6 +18,7 @@ import static org.junit.Assert.assertEquals;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import org.junit.Test;
@ -26,7 +27,8 @@ public class ChangeEditTest {
public void changeEditRef() throws Exception {
Account.Id accountId = new Account.Id(1000042);
Change.Id changeId = new Change.Id(56414);
String refName = ChangeEditUtil.editRefName(accountId, changeId);
assertEquals("refs/users/42/1000042/edit-56414", refName);
PatchSet.Id psId = new PatchSet.Id(changeId, 50);
String refName = ChangeEditUtil.editRefName(accountId, changeId, psId);
assertEquals("refs/users/42/1000042/edit-56414/50", refName);
}
}