Merge changes Ie59a530f,I0825b3c2

* changes:
  Merge branch 'stable-2.8'
  Add submit rule evaluation tests for custom labels

12 files changed, 187 insertions, 31 deletions

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 6e6ef374be..5c5746b81f 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 static org.junit.Assert.assertEquals;
 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 @@ public abstract class AbstractDaemonTest {
   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 0000000000..83c9f3822b
--- /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 e94d8f3838..a2dd8ec0ab 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.NoHttpd;
 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 @@ public class LabelTypeIT extends AbstractDaemonTest {
     }
   }
 
-  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 @@ public class LabelTypeIT extends AbstractDaemonTest {
       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 6d2dc581a3..cb852a8666 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 @@ public class ChangeJson {
       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 dd5101d2eb..b581c67eb3 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 @@ public class PatchSetInserter {
   private boolean draft;
   private boolean runHooks;
   private boolean sendMail;
+  private Account.Id uploader;
 
   @Inject
   public PatchSetInserter(ChangeHooks hooks,
@@ -205,6 +206,11 @@ public class PatchSetInserter {
     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 @@ public class PatchSetInserter {
       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 b0fe0b5ae8..8de6904e54 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 @@ public class RebaseChange {
         .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 ac2bf9fe97..847eaee969 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 @@ public class ReceiveCommits {
     }
 
     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 7a6ce7af85..a97b3fde2e 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 @@ public class RebaseIfNecessary extends SubmitStrategy {
                 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 621809ab6f..8e8c844900 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 @@ public class CreateBranch implements RestModifyView<ProjectResource, Input> {
         }
       }
 
-      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 a6ad3746f5..858ff9113c 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 @@ public class RefControl {
    *
    * @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 @@ public class RefControl {
     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 12dcd52642..d8f009b638 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 @@ MSG="$1"
 #
 add_ChangeId() {
 	clean_message=`sed -e '
-		/^diff --git a\/.*/{
+		/^diff --git .*/{
 			s///
 			q
 		}
@@ -81,7 +81,7 @@ add_ChangeId() {
 	# 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 0df8c1fc46..ff4b9360c9 100644
--- a/tools/default.defs
+++ b/tools/default.defs
@@ -152,7 +152,8 @@ def gerrit_plugin(
     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(