diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java index 5a8ba0c605..5b3795e1d4 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -15,10 +15,14 @@ package com.google.gerrit.acceptance.git; import static com.google.gerrit.acceptance.GitUtil.cloneProject; +import static org.junit.Assert.assertEquals; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.TestAccount; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.LabelInfo; +import com.google.gerrit.extensions.restapi.RestApiException; import com.google.gerrit.reviewdb.client.Change; import com.google.gwtorm.server.OrmException; @@ -143,6 +147,43 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { r.assertChange(Change.Status.DRAFT, null); } + @Test + public void testPushForMasterWithApprovals() throws GitAPIException, + IOException, RestApiException { + PushOneCommit.Result r = pushTo("refs/for/master/%l=Code-Review"); + r.assertOkStatus(); + ChangeInfo ci = get(r.getChangeId()); + LabelInfo cr = ci.labels.get("Code-Review"); + assertEquals(1, cr.all.size()); + assertEquals("Administrator", cr.all.get(0).name); + assertEquals(1, cr.all.get(0).value.intValue()); + + PushOneCommit push = + pushFactory.create(db, admin.getIdent(), PushOneCommit.SUBJECT, + "b.txt", "anotherContent", r.getChangeId()); + r = push.to(git, "refs/for/master/%l=Code-Review+2"); + + ci = get(r.getChangeId()); + cr = ci.labels.get("Code-Review"); + assertEquals(1, cr.all.size()); + assertEquals("Administrator", cr.all.get(0).name); + assertEquals(2, cr.all.get(0).value.intValue()); + } + + @Test + public void testPushForMasterWithApprovals_MissingLabel() throws GitAPIException, + IOException { + PushOneCommit.Result r = pushTo("refs/for/master/%l=Verify"); + r.assertErrorStatus("label \"Verify\" is not a configured label"); + } + + @Test + public void testPushForMasterWithApprovals_ValueOutOfRange() throws GitAPIException, + IOException, RestApiException { + PushOneCommit.Result r = pushTo("refs/for/master/%l=Code-Review-3"); + r.assertErrorStatus("label \"Code-Review\": -3 is not a valid value"); + } + @Test public void testPushForNonExistingBranch() throws GitAPIException, OrmException, IOException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java index 64b51690e7..f98393be2b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ApprovalsUtil.java @@ -32,6 +32,8 @@ import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelTypes; +import com.google.gerrit.common.data.Permission; +import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; @@ -49,9 +51,11 @@ import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import java.sql.Timestamp; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Set; /** @@ -215,6 +219,52 @@ public class ApprovalsUtil { return Collections.unmodifiableList(cells); } + public void addApprovals(ReviewDb db, ChangeUpdate update, LabelTypes labelTypes, + PatchSet ps, PatchSetInfo info, Change change, ChangeControl changeCtl, + Map approvals) throws OrmException { + if (!approvals.isEmpty()) { + checkApprovals(approvals, labelTypes, change, changeCtl); + List cells = new ArrayList<>(approvals.size()); + Timestamp ts = TimeUtil.nowTs(); + for (Map.Entry vote : approvals.entrySet()) { + LabelType lt = labelTypes.byLabel(vote.getKey()); + cells.add(new PatchSetApproval(new PatchSetApproval.Key( + ps.getId(), + info.getCommitter().getAccount(), + lt.getLabelId()), + vote.getValue(), + ts)); + update.putApproval(vote.getKey(), vote.getValue()); + } + db.patchSetApprovals().insert(cells); + } + } + + public static void checkLabel(LabelTypes labelTypes, String name, Short value) { + LabelType label = labelTypes.byLabel(name); + if (label == null) { + throw new IllegalArgumentException(String.format( + "label \"%s\" is not a configured label", name)); + } + if (label.getValue(value) == null) { + throw new IllegalArgumentException(String.format( + "label \"%s\": %d is not a valid value", name, value)); + } + } + + private static void checkApprovals(Map approvals, LabelTypes labelTypes, + Change change, ChangeControl changeCtl) { + for (Map.Entry vote : approvals.entrySet()) { + String name = vote.getKey(); + Short value = vote.getValue(); + PermissionRange range = changeCtl.getRange(Permission.forLabel(name)); + if (range == null || !range.contains(value)) { + throw new IllegalArgumentException(String.format( + "applying label \"%s\": %d is restricted", name, value)); + } + } + } + public ListMultimap byChange(ReviewDb db, ChangeNotes notes) throws OrmException { if (!migration.readPatchSetApprovals()) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java index ff94f35ef7..33647f3e8d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java @@ -32,6 +32,7 @@ import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.mail.CreateChangeSender; import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.patch.PatchSetInfoFactory; +import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.RefControl; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; @@ -45,6 +46,7 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.Collections; +import java.util.Map; import java.util.Set; public class ChangeInserter { @@ -72,6 +74,7 @@ public class ChangeInserter { private ChangeMessage changeMessage; private Set reviewers; private Set extraCC; + private Map approvals; private boolean runHooks; private boolean sendMail; @@ -100,6 +103,7 @@ public class ChangeInserter { this.commit = commit; this.reviewers = Collections.emptySet(); this.extraCC = Collections.emptySet(); + this.approvals = Collections.emptyMap(); this.runHooks = true; this.sendMail = true; @@ -152,14 +156,20 @@ public class ChangeInserter { return patchSet; } + public ChangeInserter setApprovals(Map approvals) { + this.approvals = approvals; + return this; + } + public PatchSetInfo getPatchSetInfo() { return patchSetInfo; } public Change insert() throws OrmException, IOException { ReviewDb db = dbProvider.get(); + ChangeControl ctl = refControl.getProjectControl().controlFor(change); ChangeUpdate update = updateFactory.create( - refControl.getProjectControl().controlFor(change), + ctl, change.getCreatedOn()); db.changes().beginTransaction(change.getId()); try { @@ -169,6 +179,8 @@ public class ChangeInserter { LabelTypes labelTypes = refControl.getProjectControl().getLabelTypes(); approvalsUtil.addReviewers(db, update, labelTypes, change, patchSet, patchSetInfo, reviewers, Collections. emptySet()); + approvalsUtil.addApprovals(db, update, labelTypes, patchSet, patchSetInfo, + change, ctl, approvals); if (messageIsForChange()) { insertMessage(db); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 82a5248d61..ac0a95ed78 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -50,6 +50,7 @@ import com.google.gerrit.common.ChangeHookRunner.HookResult; import com.google.gerrit.common.ChangeHooks; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.data.Capable; +import com.google.gerrit.common.data.LabelType; import com.google.gerrit.common.data.LabelTypes; import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.PermissionRule; @@ -102,6 +103,7 @@ import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.RefControl; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.ssh.SshInfo; +import com.google.gerrit.server.util.LabelVote; import com.google.gerrit.server.util.MagicBranch; import com.google.gerrit.server.util.RequestScopePropagator; import com.google.gerrit.server.util.TimeUtil; @@ -1070,7 +1072,10 @@ public class ReceiveCommits { RefControl ctl; Set reviewer = Sets.newLinkedHashSet(); Set cc = Sets.newLinkedHashSet(); + Map labels = new HashMap<>(); List baseCommit; + LabelTypes labelTypes; + CmdLineParser clp; @Option(name = "--base", metaVar = "BASE", usage = "merge base of changes") List base; @@ -1099,9 +1104,23 @@ public class ReceiveCommits { draft = !publish; } - MagicBranchInput(ReceiveCommand cmd) { + @Option(name = "-l", metaVar = "LABEL+VALUE", + usage = "label(s) to assign (defaults to +1 if no value provided") + void addLabel(final String token) throws CmdLineException { + LabelVote v = LabelVote.parse(token); + try { + LabelType.checkName(v.getLabel()); + ApprovalsUtil.checkLabel(labelTypes, v.getLabel(), v.getValue()); + } catch (IllegalArgumentException e) { + throw clp.reject(e.getMessage()); + } + labels.put(v.getLabel(), v.getValue()); + } + + MagicBranchInput(ReceiveCommand cmd, LabelTypes labelTypes) { this.cmd = cmd; this.draft = cmd.getRefName().startsWith(MagicBranch.NEW_DRAFT_CHANGE); + this.labelTypes = labelTypes; } boolean isDraft() { @@ -1116,6 +1135,10 @@ public class ReceiveCommits { return new MailRecipients(reviewer, cc); } + Map getLabels() { + return labels; + } + String parse(CmdLineParser clp, Repository repo, Set refs) throws CmdLineException { String ref = MagicBranch.getDestBranchName(cmd.getRefName()); @@ -1158,6 +1181,10 @@ public class ReceiveCommits { } return ref.substring(0, split); } + + void setCmdLineParser(CmdLineParser clp) { + this.clp = clp; + } } private void parseMagicBranch(final ReceiveCommand cmd) { @@ -1167,12 +1194,13 @@ public class ReceiveCommits { return; } - magicBranch = new MagicBranchInput(cmd); + magicBranch = new MagicBranchInput(cmd, labelTypes); magicBranch.reviewer.addAll(reviewersFromCommandLine); magicBranch.cc.addAll(ccFromCommandLine); String ref; CmdLineParser clp = optionParserFactory.create(magicBranch); + magicBranch.setCmdLineParser(clp); try { ref = magicBranch.parse(clp, repo, rp.getAdvertisedRefs().keySet()); } catch (CmdLineException e) { @@ -1593,8 +1621,10 @@ public class ReceiveCommits { final Account.Id me = currentUser.getAccountId(); final List footerLines = commit.getFooterLines(); final MailRecipients recipients = new MailRecipients(); + Map approvals = new HashMap<>(); if (magicBranch != null) { recipients.add(magicBranch.getMailRecipients()); + approvals = magicBranch.getLabels(); } recipients.add(getRecipientsFromFooters(accountResolver, ps, footerLines)); recipients.remove(me); @@ -1606,6 +1636,7 @@ public class ReceiveCommits { ins .setReviewers(recipients.getReviewers()) + .setApprovals(approvals) .setMessage(msg) .setSendMail(false) .insert(); @@ -1923,8 +1954,10 @@ public class ReceiveCommits { final Account.Id me = currentUser.getAccountId(); final List footerLines = newCommit.getFooterLines(); final MailRecipients recipients = new MailRecipients(); + Map approvals = new HashMap<>(); if (magicBranch != null) { recipients.add(magicBranch.getMailRecipients()); + approvals = magicBranch.getLabels(); } recipients.add(getRecipientsFromFooters(accountResolver, newPatchSet, footerLines)); recipients.remove(me); @@ -1952,6 +1985,8 @@ public class ReceiveCommits { approvalCopier.copy(db, changeCtl, newPatchSet); approvalsUtil.addReviewers(db, update, labelTypes, change, newPatchSet, info, recipients.getReviewers(), oldRecipients.getAll()); + approvalsUtil.addApprovals(db, update, labelTypes, newPatchSet, info, + change, changeCtl, approvals); recipients.add(oldRecipients); msg = diff --git a/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/CmdLineParser.java b/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/CmdLineParser.java index c60c0d28e0..e330834152 100644 --- a/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/CmdLineParser.java +++ b/gerrit-util-cli/src/main/java/com/google/gerrit/util/cli/CmdLineParser.java @@ -54,7 +54,6 @@ import org.kohsuke.args4j.spi.OptionHandler; import org.kohsuke.args4j.spi.Setter; import org.kohsuke.args4j.spi.FieldSetter; - import java.io.StringWriter; import java.io.Writer; import java.lang.annotation.Annotation; @@ -443,4 +442,8 @@ public class CmdLineParser { return false; } } + + public CmdLineException reject(String message) { + return new CmdLineException(parser, message); + } }