Merge "Add refs/for/<branch>%submit to auto-merge during push"

This commit is contained in:
Shawn Pearce 2013-04-22 18:08:34 +00:00 committed by Gerrit Code Review
commit c36a0e01d2
9 changed files with 653 additions and 162 deletions

View File

@ -652,6 +652,10 @@ In order to submit, all labels (such as `Verified` and `Code-Review`,
above) must enable submit, and also must not block it. See above for above) must enable submit, and also must not block it. See above for
details on each label. details on each label.
To link:user-upload.html#auto_merge[immediately submit a change on push]
the caller needs to have the Submit permission on `refs/for/<ref>`
(e.g. on `refs/for/refs/heads/master`).
[[category_view_drafts]] [[category_view_drafts]]
View Drafts View Drafts

View File

@ -333,6 +333,26 @@ grant nothing at all. This ensures that accidental pushes don't
make undesired changes to the public repository. make undesired changes to the public repository.
[[auto_merge]]
Auto-Merge during Push
~~~~~~~~~~~~~~~~~~~~~~
Changes can be directly submitted on push. This is primarily useful
for teams that don't want to do code review but want to use Gerrit's
submit strategies to handle contention on busy branches. Using
`%submit` creates a change and submits it immediately, if the caller
has link:access-control.html#category_submit[Submit] permission on
`refs/for/<ref>` (e.g. on `refs/for/refs/heads/master`).
====
git push ssh://john.doe@git.example.com:29418/kernel/common HEAD:refs/for/master%submit
====
On auto-merge of a change neither labels nor submit rules are checked.
If the merge fails the change stays open, but when pushing a new patch
set the merge can be reattempted by using `%submit` again.
repo upload repo upload
----------- -----------

View File

@ -107,11 +107,17 @@ public class GitUtil {
public static String createCommit(Git git, PersonIdent i, String msg) public static String createCommit(Git git, PersonIdent i, String msg)
throws GitAPIException, IOException { throws GitAPIException, IOException {
return createCommit(git, i, msg, true); return createCommit(git, i, msg, true, false);
} }
public static String createCommit(Git git, PersonIdent i, String msg, public static void amendCommit(Git git, PersonIdent i, String msg, String changeId)
boolean insertChangeId) throws GitAPIException, IOException { throws GitAPIException, IOException {
msg = ChangeIdUtil.insertId(msg, ObjectId.fromString(changeId.substring(1)));
createCommit(git, i, msg, false, true);
}
private static String createCommit(Git git, PersonIdent i, String msg,
boolean insertChangeId, boolean amend) throws GitAPIException, IOException {
ObjectId changeId = null; ObjectId changeId = null;
if (insertChangeId) { if (insertChangeId) {
changeId = computeChangeId(git, i, msg); changeId = computeChangeId(git, i, msg);
@ -119,6 +125,7 @@ public class GitUtil {
} }
final CommitCommand commitCmd = git.commit(); final CommitCommand commitCmd = git.commit();
commitCmd.setAmend(amend);
commitCmd.setAuthor(i); commitCmd.setAuthor(i);
commitCmd.setCommitter(i); commitCmd.setCommitter(i);
commitCmd.setMessage(msg); commitCmd.setMessage(msg);

View File

@ -14,27 +14,16 @@
package com.google.gerrit.acceptance.git; package com.google.gerrit.acceptance.git;
import static com.google.gerrit.acceptance.git.GitUtil.add;
import static com.google.gerrit.acceptance.git.GitUtil.cloneProject; import static com.google.gerrit.acceptance.git.GitUtil.cloneProject;
import static com.google.gerrit.acceptance.git.GitUtil.createCommit;
import static com.google.gerrit.acceptance.git.GitUtil.createProject; import static com.google.gerrit.acceptance.git.GitUtil.createProject;
import static com.google.gerrit.acceptance.git.GitUtil.initSsh; import static com.google.gerrit.acceptance.git.GitUtil.initSsh;
import static com.google.gerrit.acceptance.git.GitUtil.pushHead;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import com.google.common.base.Function;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AccountCreator; import com.google.gerrit.acceptance.AccountCreator;
import com.google.gerrit.acceptance.SshSession; import com.google.gerrit.acceptance.SshSession;
import com.google.gerrit.acceptance.TestAccount; import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@ -45,9 +34,6 @@ import com.jcraft.jsch.JSchException;
import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
import org.junit.After; import org.junit.After;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
@ -56,9 +42,7 @@ import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters; import org.junit.runners.Parameterized.Parameters;
import java.io.IOException; import java.io.IOException;
import java.util.Arrays;
import java.util.List; import java.util.List;
import java.util.Set;
@RunWith(Parameterized.class) @RunWith(Parameterized.class)
public class PushForReviewIT extends AbstractDaemonTest { public class PushForReviewIT extends AbstractDaemonTest {
@ -127,30 +111,24 @@ public class PushForReviewIT extends AbstractDaemonTest {
@Test @Test
public void testPushForMaster() throws GitAPIException, OrmException, public void testPushForMaster() throws GitAPIException, OrmException,
IOException { IOException {
PushOneCommit push = new PushOneCommit(); PushOneCommit.Result r = pushTo("refs/for/master");
String ref = "refs/for/master"; r.assertOkStatus();
PushResult r = push.to(ref); r.assertChange(Change.Status.NEW, null);
assertOkStatus(r, ref);
assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, null);
} }
@Test @Test
public void testPushForMasterWithTopic() throws GitAPIException, public void testPushForMasterWithTopic() throws GitAPIException,
OrmException, IOException { OrmException, IOException {
// specify topic in ref // specify topic in ref
PushOneCommit push = new PushOneCommit();
String topic = "my/topic"; String topic = "my/topic";
String ref = "refs/for/master/" + topic; PushOneCommit.Result r = pushTo("refs/for/master/" + topic);
PushResult r = push.to(ref); r.assertOkStatus();
assertOkStatus(r, ref); r.assertChange(Change.Status.NEW, topic);
assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, topic);
// specify topic as option // specify topic as option
push = new PushOneCommit(); r = pushTo("refs/for/master%topic=" + topic);
ref = "refs/for/master%topic=" + topic; r.assertOkStatus();
r = push.to(ref); r.assertChange(Change.Status.NEW, topic);
assertOkStatus(r, ref);
assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, topic);
} }
@Test @Test
@ -158,30 +136,24 @@ public class PushForReviewIT extends AbstractDaemonTest {
IOException, JSchException { IOException, JSchException {
// cc one user // cc one user
TestAccount user = accounts.create("user", "user@example.com", "User"); TestAccount user = accounts.create("user", "user@example.com", "User");
PushOneCommit push = new PushOneCommit();
String topic = "my/topic"; String topic = "my/topic";
String ref = "refs/for/master/" + topic + "%cc=" + user.email; PushOneCommit.Result r = pushTo("refs/for/master/" + topic + "%cc=" + user.email);
PushResult r = push.to(ref); r.assertOkStatus();
assertOkStatus(r, ref); r.assertChange(Change.Status.NEW, topic);
assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, topic);
// cc several users // cc several users
TestAccount user2 = TestAccount user2 =
accounts.create("another-user", "another.user@example.com", "Another User"); accounts.create("another-user", "another.user@example.com", "Another User");
push = new PushOneCommit(); r = pushTo("refs/for/master/" + topic + "%cc=" + admin.email + ",cc="
ref = "refs/for/master/" + topic + "%cc=" + admin.email + ",cc=" + user.email + user.email + ",cc=" + user2.email);
+ ",cc=" + user2.email; r.assertOkStatus();
r = push.to(ref); r.assertChange(Change.Status.NEW, topic);
assertOkStatus(r, ref);
assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, topic);
// cc non-existing user // cc non-existing user
String nonExistingEmail = "non.existing@example.com"; String nonExistingEmail = "non.existing@example.com";
push = new PushOneCommit(); r = pushTo("refs/for/master/" + topic + "%cc=" + admin.email + ",cc="
ref = "refs/for/master/" + topic + "%cc=" + admin.email + ",cc=" + nonExistingEmail + ",cc=" + user.email);
+ nonExistingEmail + ",cc=" + user.email; r.assertErrorStatus("user \"" + nonExistingEmail + "\" not found");
r = push.to(ref);
assertErrorStatus(r, "user \"" + nonExistingEmail + "\" not found", ref);
} }
@Test @Test
@ -189,121 +161,52 @@ public class PushForReviewIT extends AbstractDaemonTest {
OrmException, IOException, JSchException { OrmException, IOException, JSchException {
// add one reviewer // add one reviewer
TestAccount user = accounts.create("user", "user@example.com", "User"); TestAccount user = accounts.create("user", "user@example.com", "User");
PushOneCommit push = new PushOneCommit();
String topic = "my/topic"; String topic = "my/topic";
String ref = "refs/for/master/" + topic + "%r=" + user.email; PushOneCommit.Result r = pushTo("refs/for/master/" + topic + "%r=" + user.email);
PushResult r = push.to(ref); r.assertOkStatus();
assertOkStatus(r, ref); r.assertChange(Change.Status.NEW, topic, user);
assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT,
topic, user);
// add several reviewers // add several reviewers
TestAccount user2 = TestAccount user2 =
accounts.create("another-user", "another.user@example.com", "Another User"); accounts.create("another-user", "another.user@example.com", "Another User");
push = new PushOneCommit(); r = pushTo("refs/for/master/" + topic + "%r=" + admin.email + ",r=" + user.email
ref = "refs/for/master/" + topic + "%r=" + admin.email + ",r=" + user.email + ",r=" + user2.email);
+ ",r=" + user2.email; r.assertOkStatus();
r = push.to(ref);
assertOkStatus(r, ref);
// admin is the owner of the change and should not appear as reviewer // admin is the owner of the change and should not appear as reviewer
assertChange(push.changeId, Change.Status.NEW, PushOneCommit.SUBJECT, r.assertChange(Change.Status.NEW, topic, user, user2);
topic, user, user2);
// add non-existing user as reviewer // add non-existing user as reviewer
String nonExistingEmail = "non.existing@example.com"; String nonExistingEmail = "non.existing@example.com";
push = new PushOneCommit(); r = pushTo("refs/for/master/" + topic + "%r=" + admin.email + ",r="
ref = "refs/for/master/" + topic + "%r=" + admin.email + ",r=" + nonExistingEmail + ",r=" + user.email);
+ nonExistingEmail + ",r=" + user.email; r.assertErrorStatus("user \"" + nonExistingEmail + "\" not found");
r = push.to(ref);
assertErrorStatus(r, "user \"" + nonExistingEmail + "\" not found", ref);
} }
@Test @Test
public void testPushForMasterAsDraft() throws GitAPIException, OrmException, public void testPushForMasterAsDraft() throws GitAPIException, OrmException,
IOException { IOException {
// create draft by pushing to 'refs/drafts/' // create draft by pushing to 'refs/drafts/'
PushOneCommit push = new PushOneCommit(); PushOneCommit.Result r = pushTo("refs/drafts/master");
String ref = "refs/drafts/master"; r.assertOkStatus();
PushResult r = push.to(ref); r.assertChange(Change.Status.DRAFT, null);
assertOkStatus(r, ref);
assertChange(push.changeId, Change.Status.DRAFT, PushOneCommit.SUBJECT, null);
// create draft by using 'draft' option // create draft by using 'draft' option
push = new PushOneCommit(); r = pushTo("refs/for/master%draft");
ref = "refs/for/master%draft"; r.assertOkStatus();
r = push.to(ref); r.assertChange(Change.Status.DRAFT, null);
assertOkStatus(r, ref);
assertChange(push.changeId, Change.Status.DRAFT, PushOneCommit.SUBJECT, null);
} }
@Test @Test
public void testPushForNonExistingBranch() throws GitAPIException, public void testPushForNonExistingBranch() throws GitAPIException,
OrmException, IOException { OrmException, IOException {
PushOneCommit push = new PushOneCommit();
String branchName = "non-existing"; String branchName = "non-existing";
String ref = "refs/for/" + branchName; PushOneCommit.Result r = pushTo("refs/for/" + branchName);
PushResult r = push.to(ref); r.assertErrorStatus("branch " + branchName + " not found");
assertErrorStatus(r, "branch " + branchName + " not found", ref);
} }
private void assertChange(String changeId, Change.Status expectedStatus, private PushOneCommit.Result pushTo(String ref) throws GitAPIException,
String expectedSubject, String expectedTopic, IOException {
TestAccount... expectedReviewers) throws OrmException { PushOneCommit push = new PushOneCommit(db, admin.getIdent());
Change c = return push.to(git, ref);
Iterables.getOnlyElement(db.changes().byKey(new Change.Key(changeId)).toList());
assertEquals(expectedSubject, c.getSubject());
assertEquals(expectedStatus, c.getStatus());
assertEquals(expectedTopic, Strings.emptyToNull(c.getTopic()));
assertReviewers(c, expectedReviewers);
}
private void assertReviewers(Change c, TestAccount... expectedReviewers)
throws OrmException {
Set<Account.Id> expectedReviewerIds =
Sets.newHashSet(Lists.transform(Arrays.asList(expectedReviewers),
new Function<TestAccount, Account.Id>() {
@Override
public Account.Id apply(TestAccount a) {
return a.id;
}
}));
for (PatchSetApproval psa : db.patchSetApprovals().byPatchSet(
c.currentPatchSetId())) {
assertTrue("unexpected reviewer " + psa.getAccountId(),
expectedReviewerIds.remove(psa.getAccountId()));
}
assertTrue("missing reviewers: " + expectedReviewerIds,
expectedReviewerIds.isEmpty());
}
private static void assertOkStatus(PushResult result, String ref) {
assertStatus(Status.OK, null, result, ref);
}
private static void assertErrorStatus(PushResult result,
String expectedMessage, String ref) {
assertStatus(Status.REJECTED_OTHER_REASON, expectedMessage, result, ref);
}
private static void assertStatus(Status expectedStatus,
String expectedMessage, PushResult result, String ref) {
RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref);
assertEquals(refUpdate.getMessage() + "\n" + result.getMessages(),
expectedStatus, refUpdate.getStatus());
assertEquals(expectedMessage, refUpdate.getMessage());
}
private class PushOneCommit {
final static String FILE_NAME = "a.txt";
final static String FILE_CONTENT = "some content";
final static String SUBJECT = "test commit";
String changeId;
public PushResult to(String ref) throws GitAPIException, IOException {
add(git, FILE_NAME, FILE_CONTENT);
changeId = createCommit(git, admin.getIdent(), SUBJECT);
return pushHead(git, ref);
}
} }
} }

View File

@ -0,0 +1,179 @@
// Copyright (C) 2013 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.acceptance.git;
import static com.google.gerrit.acceptance.git.GitUtil.add;
import static com.google.gerrit.acceptance.git.GitUtil.amendCommit;
import static com.google.gerrit.acceptance.git.GitUtil.createCommit;
import static com.google.gerrit.acceptance.git.GitUtil.pushHead;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import com.google.common.base.Function;
import com.google.common.base.Strings;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.transport.PushResult;
import org.eclipse.jgit.transport.RemoteRefUpdate;
import org.eclipse.jgit.transport.RemoteRefUpdate.Status;
import java.io.IOException;
import java.util.Arrays;
import java.util.Set;
public class PushOneCommit {
public final static String SUBJECT = "test commit";
private final static String FILE_NAME = "a.txt";
private final static String FILE_CONTENT = "some content";
private final ReviewDb db;
private final PersonIdent i;
private final String subject;
private final String fileName;
private final String content;
private String changeId;
public PushOneCommit(ReviewDb db, PersonIdent i) {
this(db, i, SUBJECT, FILE_NAME, FILE_CONTENT);
}
public PushOneCommit(ReviewDb db, PersonIdent i, String subject,
String fileName, String content) {
this(db, i, subject, fileName, content, null);
}
public PushOneCommit(ReviewDb db, PersonIdent i, String subject,
String fileName, String content, String changeId) {
this.db = db;
this.i = i;
this.subject = subject;
this.fileName = fileName;
this.content = content;
this.changeId = changeId;
}
public Result to(Git git, String ref)
throws GitAPIException, IOException {
add(git, fileName, content);
if (changeId != null) {
amendCommit(git, i, subject, changeId);
} else {
changeId = createCommit(git, i, subject);
}
return new Result(db, ref, pushHead(git, ref), changeId, subject);
}
public static class Result {
private final ReviewDb db;
private final String ref;
private final PushResult result;
private final String changeId;
private final String subject;
private Result(ReviewDb db, String ref, PushResult result, String changeId,
String subject) {
this.db = db;
this.ref = ref;
this.result = result;
this.changeId = changeId;
this.subject = subject;
}
public PatchSet.Id getPatchSetId() throws OrmException {
return Iterables.getOnlyElement(
db.changes().byKey(new Change.Key(changeId))).currentPatchSetId();
}
public String getChangeId() {
return changeId;
}
public void assertChange(Change.Status expectedStatus,
String expectedTopic, TestAccount... expectedReviewers)
throws OrmException {
Change c =
Iterables.getOnlyElement(db.changes().byKey(new Change.Key(changeId)).toList());
assertEquals(subject, c.getSubject());
assertEquals(expectedStatus, c.getStatus());
assertEquals(expectedTopic, Strings.emptyToNull(c.getTopic()));
assertReviewers(c, expectedReviewers);
}
private void assertReviewers(Change c, TestAccount... expectedReviewers)
throws OrmException {
Set<Account.Id> expectedReviewerIds =
Sets.newHashSet(Lists.transform(Arrays.asList(expectedReviewers),
new Function<TestAccount, Account.Id>() {
@Override
public Account.Id apply(TestAccount a) {
return a.id;
}
}));
for (PatchSetApproval psa : db.patchSetApprovals().byPatchSet(
c.currentPatchSetId())) {
assertTrue("unexpected reviewer " + psa.getAccountId(),
expectedReviewerIds.remove(psa.getAccountId()));
}
assertTrue("missing reviewers: " + expectedReviewerIds,
expectedReviewerIds.isEmpty());
}
public void assertOkStatus() {
assertStatus(Status.OK, null);
}
public void assertErrorStatus(String expectedMessage) {
assertStatus(Status.REJECTED_OTHER_REASON, expectedMessage);
}
private void assertStatus(Status expectedStatus, String expectedMessage) {
RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref);
assertEquals(message(refUpdate),
expectedStatus, refUpdate.getStatus());
assertEquals(expectedMessage, refUpdate.getMessage());
}
public void assertMessage(String expectedMessage) {
RemoteRefUpdate refUpdate = result.getRemoteUpdate(ref);
assertTrue(message(refUpdate), message(refUpdate).toLowerCase().contains(
expectedMessage.toLowerCase()));
}
private String message(RemoteRefUpdate refUpdate) {
StringBuilder b = new StringBuilder();
if (refUpdate.getMessage() != null) {
b.append(refUpdate.getMessage());
b.append("\n");
}
b.append(result.getMessages());
return b.toString();
}
}
}

View File

@ -0,0 +1,300 @@
// Copyright (C) 2013 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.acceptance.git;
import static com.google.gerrit.acceptance.git.GitUtil.cloneProject;
import static com.google.gerrit.acceptance.git.GitUtil.createProject;
import static com.google.gerrit.acceptance.git.GitUtil.initSsh;
import static org.junit.Assert.assertEquals;
import com.google.common.collect.Iterables;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AccountCreator;
import com.google.gerrit.acceptance.SshSession;
import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.Permission;
import com.google.gerrit.common.data.PermissionRule;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.git.CommitMergeStatus;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.RefSpec;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import java.io.IOException;
import java.util.List;
public class SubmitOnPushIT extends AbstractDaemonTest {
@Inject
private AccountCreator accounts;
@Inject
private SchemaFactory<ReviewDb> reviewDbProvider;
@Inject
private GitRepositoryManager repoManager;
@Inject
private MetaDataUpdate.Server metaDataUpdateFactory;
@Inject
private ProjectCache projectCache;
@Inject
private GroupCache groupCache;
@Inject
private @GerritPersonIdent PersonIdent serverIdent;
private TestAccount admin;
private Project.NameKey project;
private Git git;
private ReviewDb db;
@Before
public void setUp() throws Exception {
admin =
accounts.create("admin", "admin@example.com", "Administrator",
"Administrators");
project = new Project.NameKey("p");
initSsh(admin);
SshSession sshSession = new SshSession(admin);
createProject(sshSession, project.get());
git = cloneProject(sshSession.getUrl() + "/" + project.get());
sshSession.close();
db = reviewDbProvider.open();
}
@After
public void cleanup() {
db.close();
}
@Test
public void submitOnPush() throws GitAPIException, OrmException,
IOException, ConfigInvalidException {
grantSubmit(project, "refs/for/refs/heads/master");
PushOneCommit.Result r = pushTo("refs/for/master%submit");
r.assertOkStatus();
r.assertChange(Change.Status.MERGED, null, admin);
assertSubmitApproval(r.getPatchSetId());
assertCommit(project, "refs/heads/master");
}
@Test
public void submitOnPushToRefsMetaConfig() throws GitAPIException,
OrmException, IOException, ConfigInvalidException {
grantSubmit(project, "refs/for/refs/meta/config");
git.fetch().setRefSpecs(new RefSpec("refs/meta/config:refs/meta/config")).call();
ObjectId objectId = git.getRepository().getRef("refs/meta/config").getObjectId();
git.checkout().setName(objectId.getName()).call();
PushOneCommit.Result r = pushTo("refs/for/refs/meta/config%submit");
r.assertOkStatus();
r.assertChange(Change.Status.MERGED, null, admin);
assertSubmitApproval(r.getPatchSetId());
assertCommit(project, "refs/meta/config");
}
@Test
public void submitOnPushMergeConflict() throws GitAPIException, OrmException,
IOException, ConfigInvalidException {
String master = "refs/heads/master";
ObjectId objectId = git.getRepository().getRef(master).getObjectId();
push(master, "one change", "a.txt", "some content");
git.checkout().setName(objectId.getName()).call();
grantSubmit(project, "refs/for/refs/heads/master");
PushOneCommit.Result r =
push("refs/for/master%submit", "other change", "a.txt", "other content");
r.assertOkStatus();
r.assertChange(Change.Status.NEW, null, admin);
r.assertMessage(CommitMergeStatus.PATH_CONFLICT.getMessage());
}
@Test
public void submitOnPushSuccessfulMerge() throws GitAPIException, OrmException,
IOException, ConfigInvalidException {
String master = "refs/heads/master";
ObjectId objectId = git.getRepository().getRef(master).getObjectId();
push(master, "one change", "a.txt", "some content");
git.checkout().setName(objectId.getName()).call();
grantSubmit(project, "refs/for/refs/heads/master");
PushOneCommit.Result r =
push("refs/for/master%submit", "other change", "b.txt", "other content");
r.assertOkStatus();
r.assertChange(Change.Status.MERGED, null, admin);
assertMergeCommit(master, "other change");
}
@Test
public void submitOnPushNewPatchSet() throws GitAPIException,
OrmException, IOException, ConfigInvalidException {
PushOneCommit.Result r =
push("refs/for/master", PushOneCommit.SUBJECT, "a.txt", "some content");
grantSubmit(project, "refs/for/refs/heads/master");
r = push("refs/for/master%submit", PushOneCommit.SUBJECT, "a.txt",
"other content", r.getChangeId());
r.assertOkStatus();
r.assertChange(Change.Status.MERGED, null, admin);
Change c = Iterables.getOnlyElement(db.changes().byKey(
new Change.Key(r.getChangeId())).toList());
assertEquals(2, db.patchSets().byChange(c.getId()).toList().size());
assertSubmitApproval(r.getPatchSetId());
assertCommit(project, "refs/heads/master");
}
@Test
public void submitOnPushNotAllowed_Error() throws GitAPIException,
OrmException, IOException {
PushOneCommit.Result r = pushTo("refs/for/master%submit");
r.assertErrorStatus("submit not allowed");
}
@Test
public void submitOnPushNewPatchSetNotAllowed_Error() throws GitAPIException,
OrmException, IOException, ConfigInvalidException {
PushOneCommit.Result r =
push("refs/for/master", PushOneCommit.SUBJECT, "a.txt", "some content");
r = push("refs/for/master%submit", PushOneCommit.SUBJECT, "a.txt",
"other content", r.getChangeId());
r.assertErrorStatus("submit not allowed");
}
@Test
public void submitOnPushingDraft_Error() throws GitAPIException,
OrmException, IOException {
PushOneCommit.Result r = pushTo("refs/for/master%draft,submit");
r.assertErrorStatus("cannot submit draft");
}
@Test
public void submitOnPushToNonExistingBranch_Error() throws GitAPIException,
OrmException, IOException {
String branchName = "non-existing";
PushOneCommit.Result r = pushTo("refs/for/" + branchName + "%submit");
r.assertErrorStatus("branch " + branchName + " not found");
}
private void grantSubmit(Project.NameKey project, String ref)
throws RepositoryNotFoundException, IOException, ConfigInvalidException {
MetaDataUpdate md = metaDataUpdateFactory.create(project);
md.setMessage("Grant submit on " + ref);
ProjectConfig config = ProjectConfig.read(md);
AccessSection s = config.getAccessSection(ref, true);
Permission p = s.getPermission(Permission.SUBMIT, true);
AccountGroup adminGroup = groupCache.get(new AccountGroup.NameKey("Administrators"));
p.add(new PermissionRule(config.resolve(adminGroup)));
config.commit(md);
projectCache.evict(config.getProject());
}
private void assertSubmitApproval(PatchSet.Id patchSetId) throws OrmException {
List<PatchSetApproval> approvals = db.patchSetApprovals().byPatchSet(patchSetId).toList();
assertEquals(1, approvals.size());
PatchSetApproval a = approvals.get(0);
assertEquals(PatchSetApproval.LabelId.SUBMIT.get(), a.getLabel());
assertEquals(1, a.getValue());
assertEquals(admin.id, a.getAccountId());
}
private void assertCommit(Project.NameKey project, String branch) throws IOException {
Repository r = repoManager.openRepository(project);
try {
RevWalk rw = new RevWalk(r);
try {
RevCommit c = rw.parseCommit(r.getRef(branch).getObjectId());
assertEquals(PushOneCommit.SUBJECT, c.getShortMessage());
assertEquals(admin.email, c.getAuthorIdent().getEmailAddress());
assertEquals(admin.email, c.getCommitterIdent().getEmailAddress());
} finally {
rw.release();
}
} finally {
r.close();
}
}
private void assertMergeCommit(String branch, String subject) throws IOException {
Repository r = repoManager.openRepository(project);
try {
RevWalk rw = new RevWalk(r);
try {
RevCommit c = rw.parseCommit(r.getRef(branch).getObjectId());
assertEquals(2, c.getParentCount());
assertEquals("Merge \"" + subject + "\"", c.getShortMessage());
assertEquals(admin.email, c.getAuthorIdent().getEmailAddress());
assertEquals(serverIdent.getEmailAddress(), c.getCommitterIdent().getEmailAddress());
} finally {
rw.release();
}
} finally {
r.close();
}
}
private PushOneCommit.Result pushTo(String ref) throws GitAPIException,
IOException {
PushOneCommit push = new PushOneCommit(db, admin.getIdent());
return push.to(git, ref);
}
private PushOneCommit.Result push(String ref, String subject,
String fileName, String content) throws GitAPIException, IOException {
PushOneCommit push =
new PushOneCommit(db, admin.getIdent(), subject, fileName, content);
return push.to(git, ref);
}
private PushOneCommit.Result push(String ref, String subject,
String fileName, String content, String changeId) throws GitAPIException,
IOException {
PushOneCommit push = new PushOneCommit(db, admin.getIdent(), subject,
fileName, content, changeId);
return push.to(git, ref);
}
}

View File

@ -106,6 +106,10 @@ public class Submit implements RestModifyView<RevisionResource, Input> {
checkSubmitRule(rsrc); checkSubmitRule(rsrc);
change = submit(rsrc, caller); change = submit(rsrc, caller);
if (change == null) {
throw new ResourceConflictException("change is "
+ status(dbProvider.get().changes().get(rsrc.getChange().getId())));
}
if (input.waitForMerge) { if (input.waitForMerge) {
mergeQueue.merge(change.getDest()); mergeQueue.merge(change.getDest());
@ -123,21 +127,7 @@ public class Submit implements RestModifyView<RevisionResource, Input> {
case MERGED: case MERGED:
return new Output(Status.MERGED, change); return new Output(Status.MERGED, change);
case NEW: case NEW:
// If the merge was attempted and it failed the system usually ChangeMessage msg = getConflictMessage(rsrc);
// writes a comment as a ChangeMessage and sets status to NEW.
// Find the relevant message and report that as the conflict.
final Timestamp before = rsrc.getChange().getLastUpdatedOn();
ChangeMessage msg = Iterables.getFirst(Iterables.filter(
Lists.reverse(dbProvider.get().changeMessages()
.byChange(change.getId())
.toList()),
new Predicate<ChangeMessage>() {
@Override
public boolean apply(ChangeMessage input) {
return input.getAuthor() == null
&& input.getWrittenOn().getTime() >= before.getTime();
}
}), null);
if (msg != null) { if (msg != null) {
throw new ResourceConflictException(msg.getMessage()); throw new ResourceConflictException(msg.getMessage());
} }
@ -146,8 +136,30 @@ public class Submit implements RestModifyView<RevisionResource, Input> {
} }
} }
private Change submit(RevisionResource rsrc, IdentifiedUser caller) /**
throws OrmException, ResourceConflictException { * If the merge was attempted and it failed the system usually writes a
* comment as a ChangeMessage and sets status to NEW. Find the relevant
* message and return it.
*/
public ChangeMessage getConflictMessage(RevisionResource rsrc)
throws OrmException {
final Timestamp before = rsrc.getChange().getLastUpdatedOn();
ChangeMessage msg = Iterables.getFirst(Iterables.filter(
Lists.reverse(dbProvider.get().changeMessages()
.byChange(rsrc.getChange().getId())
.toList()),
new Predicate<ChangeMessage>() {
@Override
public boolean apply(ChangeMessage input) {
return input.getAuthor() == null
&& input.getWrittenOn().getTime() >= before.getTime();
}
}), null);
return msg;
}
public Change submit(RevisionResource rsrc, IdentifiedUser caller)
throws OrmException {
final Timestamp timestamp = new Timestamp(System.currentTimeMillis()); final Timestamp timestamp = new Timestamp(System.currentTimeMillis());
Change change = rsrc.getChange(); Change change = rsrc.getChange();
ReviewDb db = dbProvider.get(); ReviewDb db = dbProvider.get();
@ -169,8 +181,7 @@ public class Submit implements RestModifyView<RevisionResource, Input> {
} }
}); });
if (change == null) { if (change == null) {
throw new ResourceConflictException("change is " return null;
+ status(db.changes().get(rsrc.getChange().getId())));
} }
db.commit(); db.commit();
} finally { } finally {

View File

@ -14,7 +14,7 @@
package com.google.gerrit.server.git; package com.google.gerrit.server.git;
enum CommitMergeStatus { public enum CommitMergeStatus {
/** */ /** */
CLEAN_MERGE("Change has been successfully merged into the git repository."), CLEAN_MERGE("Change has been successfully merged into the git repository."),

View File

@ -59,6 +59,9 @@ import com.google.gerrit.server.GerritPersonIdent;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.change.ChangeInserter; import com.google.gerrit.server.change.ChangeInserter;
import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.config.AllProjectsName; import com.google.gerrit.server.config.AllProjectsName;
import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.config.TrackingFooters;
@ -87,6 +90,7 @@ import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.ResultSet; import com.google.gwtorm.server.ResultSet;
import com.google.gwtorm.server.SchemaFactory; import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@ -277,6 +281,8 @@ public class ReceiveCommits {
private Map<String, Ref> allRefs; private Map<String, Ref> allRefs;
private final SubmoduleOp.Factory subOpFactory; private final SubmoduleOp.Factory subOpFactory;
private final Provider<Submit> submitProvider;
private final MergeQueue mergeQueue;
private final List<CommitValidationMessage> messages = new ArrayList<CommitValidationMessage>(); private final List<CommitValidationMessage> messages = new ArrayList<CommitValidationMessage>();
private ListMultimap<Error, String> errors = LinkedListMultimap.create(); private ListMultimap<Error, String> errors = LinkedListMultimap.create();
@ -316,7 +322,9 @@ public class ReceiveCommits {
ReceiveConfig config, ReceiveConfig config,
@Assisted final ProjectControl projectControl, @Assisted final ProjectControl projectControl,
@Assisted final Repository repo, @Assisted final Repository repo,
final SubmoduleOp.Factory subOpFactory) throws IOException { final SubmoduleOp.Factory subOpFactory,
final Provider<Submit> submitProvider,
final MergeQueue mergeQueue) throws IOException {
this.currentUser = (IdentifiedUser) projectControl.getCurrentUser(); this.currentUser = (IdentifiedUser) projectControl.getCurrentUser();
this.db = db; this.db = db;
this.schemaFactory = schemaFactory; this.schemaFactory = schemaFactory;
@ -351,6 +359,8 @@ public class ReceiveCommits {
this.rejectCommits = loadRejectCommitsMap(); this.rejectCommits = loadRejectCommitsMap();
this.subOpFactory = subOpFactory; this.subOpFactory = subOpFactory;
this.submitProvider = submitProvider;
this.mergeQueue = mergeQueue;
this.messageSender = new ReceivePackMessageSender(); this.messageSender = new ReceivePackMessageSender();
@ -1000,6 +1010,9 @@ public class ReceiveCommits {
@Option(name = "--draft", usage = "mark new/update changes as draft") @Option(name = "--draft", usage = "mark new/update changes as draft")
boolean draft; boolean draft;
@Option(name = "--submit", usage = "immediately submit the change")
boolean submit;
@Option(name = "-r", metaVar = "EMAIL", usage = "add reviewer to changes") @Option(name = "-r", metaVar = "EMAIL", usage = "add reviewer to changes")
void reviewer(Account.Id id) { void reviewer(Account.Id id) {
reviewer.add(id); reviewer.add(id);
@ -1024,6 +1037,10 @@ public class ReceiveCommits {
return draft; return draft;
} }
boolean isSubmit() {
return submit;
}
MailRecipients getMailRecipients() { MailRecipients getMailRecipients() {
return new MailRecipients(reviewer, cc); return new MailRecipients(reviewer, cc);
} }
@ -1120,6 +1137,16 @@ public class ReceiveCommits {
return; return;
} }
if (magicBranch.isDraft() && magicBranch.isSubmit()) {
reject(cmd, "cannot submit draft");
return;
}
if (magicBranch.isSubmit() && !projectControl.controlForRef(
MagicBranch.NEW_CHANGE + ref).canSubmit()) {
reject(cmd, "submit not allowed");
}
// Validate that the new commits are connected with the target // Validate that the new commits are connected with the target
// branch. If they aren't, we want to abort. We do this check by // branch. If they aren't, we want to abort. We do this check by
// looking to see if we can compute a merge base between the new // looking to see if we can compute a merge base between the new
@ -1493,6 +1520,41 @@ public class ReceiveCommits {
return "send-email newchange"; return "send-email newchange";
} }
})); }));
if (magicBranch != null && magicBranch.isSubmit()) {
submit(projectControl.controlFor(change), ps);
}
}
}
private void submit(ChangeControl changeCtl, PatchSet ps) throws OrmException {
Submit submit = submitProvider.get();
RevisionResource rsrc = new RevisionResource(new ChangeResource(changeCtl), ps);
Change c = submit.submit(rsrc, currentUser);
if (c == null) {
addError("Submitting change " + changeCtl.getChange().getChangeId()
+ " failed.");
} else {
addMessage("");
mergeQueue.merge(c.getDest());
c = db.changes().get(c.getId());
switch (c.getStatus()) {
case SUBMITTED:
addMessage("Change " + c.getChangeId() + " submitted.");
break;
case MERGED:
addMessage("Change " + c.getChangeId() + " merged.");
break;
case NEW:
ChangeMessage msg = submit.getConflictMessage(rsrc);
if (msg != null) {
addMessage("Change " + c.getChangeId() + ": " + msg.getMessage());
break;
}
default:
addMessage("change " + c.getChangeId() + " is "
+ c.getStatus().name().toLowerCase());
}
} }
} }
@ -1865,6 +1927,11 @@ public class ReceiveCommits {
return "send-email newpatchset"; return "send-email newpatchset";
} }
})); }));
if (magicBranch != null && magicBranch.isSubmit()) {
submit(changeCtl, newPatchSet);
}
return newPatchSet.getId(); return newPatchSet.getId();
} }
} }