summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBruce Zu <bruce.zu@sonymobile.com>2013-06-08 14:10:50 +0800
committerBruce Zu <bruce.zu@sonymobile.com>2013-06-09 08:20:53 +0000
commit2cee70dae844261241e7476706245a7eaa0c5e6b (patch)
tree1bdfeb99f567186bdae45c486885ee6bc7b5159b
parentffe4ea0748a1afa83432edf8c4dcea53a623c5ef (diff)
Fix: failed to validate Change-Id of some new patch-sets pushed by 'refs/changes'
Those new patch-sets pushed by 'refs/changes/xxxxx/new' or 'refs/changes/xxxxx' can not get Change-Id validation, because the CommitValidators.NEW_PATCHSET can not match them. Before 15d5ac0e7, the CommitValidators.NEW_PATCHSET was same with ReceiveCommits.NEW_PATCHSET and there was not this hole. 15d5ac0e7 updated CommitValidators.NEW_PATCHSET to ensure validating the Change-Id of [a] and [b]. This commit replaces CommitValidators.NEW_PATCHSET with ReceiveCommits.NEW_PATCHSET and only keep the later one to plug this hole, and updates the ref name of ReceiveCommand for [a] and [b] to keep [a] and [b]'s Chang-Id get validation as before. [a] is of new change: 'refs/changes/nn/mmmnn/x' -> 'refs/publish/<branch name> [b] is of new patch-set of an existing change: 'refs/changes/nn/mmmnn/x' -> 'refs/changes/nn/mmmnn/new' Now the ref pattern of ReceiveCommand created via UI falls into line with those pushed via ssh command. [a]: new change created by 'Revert' button. [b]: new patch-set created by 'Edit Commit Message' icon. Change-Id: Idbc7764db9edcc067445e7246c03eb1c3e686955
-rw-r--r--gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java14
-rw-r--r--gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java2
-rw-r--r--gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java11
3 files changed, 16 insertions, 11 deletions
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 c9fb4fe2b6..9569a97737 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
@@ -42,6 +42,7 @@ import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.RefControl;
import com.google.gerrit.server.util.IdGenerator;
+import com.google.gerrit.server.util.MagicBranch;
import com.google.gwtorm.server.AtomicUpdate;
import com.google.gwtorm.server.OrmConcurrencyException;
import com.google.gwtorm.server.OrmException;
@@ -256,11 +257,14 @@ public class ChangeUtil {
ps.setUploader(change.getOwner());
ps.setRevision(new RevId(revertCommit.name()));
+ String ref = refControl.getRefName();
+ final String cmdRef =
+ MagicBranch.NEW_PUBLISH_CHANGE
+ + ref.substring(ref.lastIndexOf("/") + 1);
CommitReceivedEvent commitReceivedEvent =
new CommitReceivedEvent(new ReceiveCommand(ObjectId.zeroId(),
- revertCommit.getId(), ps.getRefName()), refControl
- .getProjectControl().getProject(), refControl.getRefName(),
- revertCommit, user);
+ revertCommit.getId(), cmdRef), refControl.getProjectControl()
+ .getProject(), refControl.getRefName(), revertCommit, user);
try {
commitValidators.validateForGerritCommits(commitReceivedEvent);
@@ -377,9 +381,11 @@ public class ChangeUtil {
final PatchSetInfo info =
patchSetInfoFactory.get(newCommit, newPatchSet.getId());
+ final String refName = newPatchSet.getRefName();
CommitReceivedEvent commitReceivedEvent =
new CommitReceivedEvent(new ReceiveCommand(ObjectId.zeroId(),
- newCommit.getId(), newPatchSet.getRefName()), refControl
+ newCommit.getId(), refName.substring(0,
+ refName.lastIndexOf("/") + 1) + "new"), refControl
.getProjectControl().getProject(), refControl.getRefName(),
newCommit, user);
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 7f3f2fd369..bcc30144fc 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
@@ -144,7 +144,7 @@ public class ReceiveCommits {
private static final Logger log =
LoggerFactory.getLogger(ReceiveCommits.class);
- private static final Pattern NEW_PATCHSET =
+ public static final Pattern NEW_PATCHSET =
Pattern.compile("^refs/changes/(?:[0-9][0-9]/)?([1-9][0-9]*)(?:/new)?$");
private static final FooterKey CHANGE_ID = new FooterKey("Change-Id");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
index 8abe501cde..a57f9239fa 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java
@@ -22,6 +22,7 @@ import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.events.CommitReceivedEvent;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.git.ReceiveCommits;
import com.google.gerrit.server.git.ValidationError;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.RefControl;
@@ -47,7 +48,6 @@ import java.net.URL;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
-import java.util.regex.Pattern;
import javax.annotation.Nullable;
@@ -57,9 +57,6 @@ public class CommitValidators {
private static final FooterKey CHANGE_ID = new FooterKey("Change-Id");
- private static final Pattern NEW_PATCHSET = Pattern
- .compile("^refs/changes/(?:[0-9][0-9])?(/[1-9][0-9]*){1,2}(?:/new)?$");
-
public interface Factory {
CommitValidators create(RefControl refControl, SshInfo sshInfo,
Repository repo);
@@ -99,7 +96,8 @@ public class CommitValidators {
validators.add(new CommitterUploaderValidator(refControl, canonicalWebUrl));
validators.add(new SignedOffByValidator(refControl, canonicalWebUrl));
if (MagicBranch.isMagicBranch(receiveEvent.command.getRefName())
- || NEW_PATCHSET.matcher(receiveEvent.command.getRefName()).matches()) {
+ || ReceiveCommits.NEW_PATCHSET.matcher(
+ receiveEvent.command.getRefName()).matches()) {
validators.add(new ChangeIdValidator(refControl, canonicalWebUrl, sshInfo));
}
validators.add(new ConfigValidator(refControl, repo));
@@ -132,7 +130,8 @@ public class CommitValidators {
validators.add(new AuthorUploaderValidator(refControl, canonicalWebUrl));
validators.add(new SignedOffByValidator(refControl, canonicalWebUrl));
if (MagicBranch.isMagicBranch(receiveEvent.command.getRefName())
- || NEW_PATCHSET.matcher(receiveEvent.command.getRefName()).matches()) {
+ || ReceiveCommits.NEW_PATCHSET.matcher(
+ receiveEvent.command.getRefName()).matches()) {
validators.add(new ChangeIdValidator(refControl, canonicalWebUrl, sshInfo));
}
validators.add(new ConfigValidator(refControl, repo));