Merge changes Ie59a530f,I0825b3c2
* changes:
Merge branch 'stable-2.8'
Add submit rule evaluation tests for custom labels
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
index 6e6ef37..5c5746b 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
@@ -22,6 +22,7 @@
import com.google.common.base.Joiner;
import com.google.common.primitives.Chars;
import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.ListChangesOption;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -196,4 +197,10 @@
protected static Gson newGson() {
return OutputFormat.JSON_COMPACT.newGson();
}
+
+ protected RevisionApi revision(PushOneCommit.Result r) throws Exception {
+ return gApi.changes()
+ .id(r.getChangeId())
+ .current();
+ }
}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
new file mode 100644
index 0000000..83c9f38
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/CustomLabelIT.java
@@ -0,0 +1,150 @@
+// Copyright (C) 2014 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.server.project;
+
+import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
+import static com.google.gerrit.server.project.Util.category;
+import static com.google.gerrit.server.project.Util.grant;
+import static com.google.gerrit.server.project.Util.value;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.common.data.LabelType;
+import com.google.gerrit.common.data.Permission;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.common.LabelInfo;
+import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.git.MetaDataUpdate;
+import com.google.gerrit.server.git.ProjectConfig;
+import com.google.gerrit.server.group.SystemGroupBackend;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.inject.Inject;
+
+import org.junit.Before;
+import org.junit.Test;
+
+@NoHttpd
+public class CustomLabelIT extends AbstractDaemonTest {
+
+ @Inject
+ private ProjectCache projectCache;
+
+ @Inject
+ private AllProjectsName allProjects;
+
+ @Inject
+ private MetaDataUpdate.Server metaDataUpdateFactory;
+
+ private final LabelType Q = category("CustomLabel",
+ value(1, "Positive"),
+ value(0, "No score"),
+ value(-1, "Negative"));
+
+ @Before
+ public void setUp() throws Exception {
+ ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
+ AccountGroup.UUID anonymousUsers =
+ SystemGroupBackend.getGroup(ANONYMOUS_USERS).getUUID();
+ grant(cfg, Permission.forLabel(Q.getName()), -1, 1, anonymousUsers,
+ "refs/heads/*");
+ saveProjectConfig(cfg);
+ }
+
+ @Test
+ public void customLabelNoOp_NegativeVoteNotBlock() throws Exception {
+ Q.setFunctionName("NoOp");
+ saveLabelConfig();
+ PushOneCommit.Result r = createChange();
+ revision(r).review(new ReviewInput().label(Q.getName(), -1));
+ ChangeInfo c = get(r.getChangeId());
+ LabelInfo q = c.labels.get(Q.getName());
+ assertEquals(1, q.all.size());
+ assertNull(q.rejected);
+ assertNotNull(q.disliked);
+ }
+
+ @Test
+ public void customLabelNoBlock_NegativeVoteNotBlock() throws Exception {
+ Q.setFunctionName("NoBlock");
+ saveLabelConfig();
+ PushOneCommit.Result r = createChange();
+ revision(r).review(new ReviewInput().label(Q.getName(), -1));
+ ChangeInfo c = get(r.getChangeId());
+ LabelInfo q = c.labels.get(Q.getName());
+ assertEquals(1, q.all.size());
+ assertNull(q.rejected);
+ assertNotNull(q.disliked);
+ }
+
+ @Test
+ public void customLabelMaxNoBlock_NegativeVoteNotBlock() throws Exception {
+ Q.setFunctionName("MaxNoBlock");
+ saveLabelConfig();
+ PushOneCommit.Result r = createChange();
+ revision(r).review(new ReviewInput().label(Q.getName(), -1));
+ ChangeInfo c = get(r.getChangeId());
+ LabelInfo q = c.labels.get(Q.getName());
+ assertEquals(1, q.all.size());
+ assertNull(q.rejected);
+ assertNotNull(q.disliked);
+ }
+
+ @Test
+ public void customLabelAnyWithBlock_NegativeVoteBlock() throws Exception {
+ Q.setFunctionName("AnyWithBlock");
+ saveLabelConfig();
+ PushOneCommit.Result r = createChange();
+ revision(r).review(new ReviewInput().label(Q.getName(), -1));
+ ChangeInfo c = get(r.getChangeId());
+ LabelInfo q = c.labels.get(Q.getName());
+ assertEquals(1, q.all.size());
+ assertNull(q.disliked);
+ assertNotNull(q.rejected);
+ }
+
+ @Test
+ public void customLabelMaxWithBlock_NegativeVoteBlock() throws Exception {
+ saveLabelConfig();
+ PushOneCommit.Result r = createChange();
+ revision(r).review(new ReviewInput().label(Q.getName(), -1));
+ ChangeInfo c = get(r.getChangeId());
+ LabelInfo q = c.labels.get(Q.getName());
+ assertEquals(1, q.all.size());
+ assertNull(q.disliked);
+ assertNotNull(q.rejected);
+ }
+
+ private void saveLabelConfig() throws Exception {
+ ProjectConfig cfg = projectCache.checkedGet(allProjects).getConfig();
+ cfg.getLabelSections().put(Q.getName(), Q);
+ saveProjectConfig(cfg);
+ }
+
+ private void saveProjectConfig(ProjectConfig cfg) throws Exception {
+ MetaDataUpdate md = metaDataUpdateFactory.create(allProjects);
+ try {
+ cfg.commit(md);
+ } finally {
+ md.close();
+ }
+ projectCache.evict(allProjects);
+ }
+}
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java
index e94d8f3..a2dd8ec 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/LabelTypeIT.java
@@ -15,7 +15,6 @@
package com.google.gerrit.acceptance.server.project;
import static com.google.common.base.Preconditions.checkNotNull;
-import static com.google.gerrit.extensions.common.ListChangesOption.DETAILED_LABELS;
import static org.junit.Assert.assertEquals;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -23,7 +22,6 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.common.data.LabelType;
import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.api.changes.RevisionApi;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.LabelInfo;
import com.google.gerrit.server.config.AllProjectsName;
@@ -235,12 +233,6 @@
}
}
- private RevisionApi revision(PushOneCommit.Result r) throws Exception {
- return gApi.changes()
- .id(r.getChangeId())
- .current();
- }
-
private void merge(PushOneCommit.Result r) throws Exception {
revision(r).review(ReviewInput.approve());
revision(r).submit();
@@ -261,7 +253,7 @@
throws Exception {
// Don't use asserts from PushOneCommit so we can test the round-trip
// through JSON instead of querying the DB directly.
- ChangeInfo c = get(r.getChangeId(), DETAILED_LABELS);
+ ChangeInfo c = get(r.getChangeId());
LabelInfo cr = c.labels.get("Code-Review");
assertEquals(1, cr.all.size());
assertEquals("Administrator", cr.all.get(0).name);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
index 6d2dc58..cb852a8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeJson.java
@@ -482,18 +482,12 @@
return;
}
- if (score != 0) {
- if (score == type.getMin().getValue()) {
- label.rejected = accountLoader.get(accountId);
- } else if (score == type.getMax().getValue()) {
- label.approved = accountLoader.get(accountId);
- } else if (score < 0) {
- label.disliked = accountLoader.get(accountId);
- label.value = score;
- } else if (score > 0 && label.disliked == null) {
- label.recommended = accountLoader.get(accountId);
- label.value = score;
- }
+ if (score < 0) {
+ label.disliked = accountLoader.get(accountId);
+ label.value = score;
+ } else if (score > 0 && label.disliked == null) {
+ label.recommended = accountLoader.get(accountId);
+ label.value = score;
}
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
index dd5101d..b581c67 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -104,6 +104,7 @@
private boolean draft;
private boolean runHooks;
private boolean sendMail;
+ private Account.Id uploader;
@Inject
public PatchSetInserter(ChangeHooks hooks,
@@ -205,6 +206,11 @@
return this;
}
+ public PatchSetInserter setUploader(Account.Id uploader) {
+ this.uploader = uploader;
+ return this;
+ }
+
public Change insert() throws InvalidChangeOperationException, OrmException,
IOException {
init();
@@ -321,6 +327,9 @@
patchSet.setRevision(new RevId(commit.name()));
}
patchSet.setDraft(draft);
+ if (uploader != null) {
+ patchSet.setUploader(uploader);
+ }
}
private void validate() throws InvalidChangeOperationException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java
index b0fe0b5..8de6904 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/RebaseChange.java
@@ -299,6 +299,7 @@
.setCopyLabels(true)
.setValidatePolicy(validate)
.setDraft(originalPatchSet.isDraft())
+ .setUploader(uploader.getAccountId())
.setSendMail(sendMail)
.setRunHooks(runHooks);
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 ac2bf9f..847eaee 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
@@ -954,7 +954,7 @@
}
RefControl ctl = projectControl.controlForRef(cmd.getRefName());
- if (ctl.canCreate(rp.getRevWalk(), obj)) {
+ if (ctl.canCreate(rp.getRevWalk(), obj, allRefs.values().contains(obj))) {
validateNewCommits(ctl, cmd);
batch.addCommand(cmd);
} else {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
index 7a6ce7a..a97b3fd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
@@ -99,8 +99,9 @@
args.db, n.notes(), n.getPatchsetId())) {
approvals.add(new PatchSetApproval(newPatchSet.getId(), a));
}
- args.db.patchSetApprovals().insert(approvals);
-
+ // rebaseChange.rebase() may already have copied some approvals,
+ // use upsert, not insert, to avoid constraint violation on database
+ args.db.patchSetApprovals().upsert(approvals);
newMergeTip =
(CodeReviewCommit) args.rw.parseCommit(ObjectId
.fromString(newPatchSet.getRevision().get()));
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
index 621809a..8e8c844 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
@@ -127,7 +127,7 @@
}
}
- if (!refControl.canCreate(rw, object)) {
+ if (!refControl.canCreate(rw, object, true)) {
throw new AuthException("Cannot create \"" + ref + "\"");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index a6ad374..858ff91 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -233,9 +233,10 @@
*
* @param rw revision pool {@code object} was parsed in.
* @param object the object the user will start the reference with.
+ * @param existsOnServer the object exists on server or not.
* @return {@code true} if the user specified can create a new Git ref
*/
- public boolean canCreate(RevWalk rw, RevObject object) {
+ public boolean canCreate(RevWalk rw, RevObject object, boolean existsOnServer) {
if (!canWrite()) {
return false;
}
@@ -253,8 +254,8 @@
if (object instanceof RevCommit) {
return getCurrentUser().getCapabilities().canAdministrateServer()
|| (owner && !isBlocked(Permission.CREATE))
- || (canPerform(Permission.CREATE) && projectControl.canReadCommit(rw,
- (RevCommit) object));
+ || (canPerform(Permission.CREATE) && (!existsOnServer && canUpdate() || projectControl
+ .canReadCommit(rw, (RevCommit) object)));
} else if (object instanceof RevTag) {
final RevTag tag = (RevTag) object;
try {
diff --git a/gerrit-server/src/main/resources/com/google/gerrit/server/tools/root/hooks/commit-msg b/gerrit-server/src/main/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
index 12dcd52..d8f009b 100644
--- a/gerrit-server/src/main/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
+++ b/gerrit-server/src/main/resources/com/google/gerrit/server/tools/root/hooks/commit-msg
@@ -26,7 +26,7 @@
#
add_ChangeId() {
clean_message=`sed -e '
- /^diff --git a\/.*/{
+ /^diff --git .*/{
s///
q
}
@@ -81,7 +81,7 @@
# Skip the line starting with the diff command and everything after it,
# up to the end of the file, assuming it is only patch data.
# If more than one line before the diff was empty, strip all but one.
- /^diff --git a/ {
+ /^diff --git / {
blankLines = 0
while (getline) { }
next
diff --git a/tools/default.defs b/tools/default.defs
index 0df8c1f..ff4b936 100644
--- a/tools/default.defs
+++ b/tools/default.defs
@@ -152,7 +152,8 @@
mf_src = []
mf_cmd += 'echo "Manifest-Version: 1.0" >$OUT;'
mf_cmd += 'echo "Gerrit-ApiType: %s" >>$OUT;' % type
- mf_cmd += 'echo "Implementation-Version: $v" >>$OUT'
+ mf_cmd += 'echo "Implementation-Version: $v" >>$OUT;'
+ mf_cmd += 'echo "Implementation-Vendor: Gerrit Code Review" >>$OUT'
for line in manifest_entries:
mf_cmd += ';echo "%s" >> $OUT' % line
genrule(
