Merge branch 'stable-2.13'

* stable-2.13:
  Make error message for rejecting Egit placeholder Change-Id consistent
  Add tests for pushing without Change-Id and with invalid Change-Ids
  RestApiMetrics: Fix description of error count metric
  commit-msg hook: Add Change-Id after Depends-On: footer

Change-Id: I0a779290d4e750c6d30e665d12e75c5498f2c2c2

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/AbstractPushForReview.java

@@ -59,6 +59,7 @@ import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.testutil.FakeEmailSender.Message;
 import com.google.gerrit.testutil.TestTimeUtil;
 
+import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.junit.TestRepository;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Repository;
@@ -839,6 +840,72 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
     assertPushRejected(pushHead(testRepo, r, false), r, "no new changes");
   }
 
+  @Test
+  public void pushWithoutChangeId() throws Exception {
+    RevCommit c = createCommit(testRepo, "Message without Change-Id");
+    assertThat(GitUtil.getChangeId(testRepo, c).isPresent()).isFalse();
+    pushForReviewRejected(testRepo,
+        "missing Change-Id in commit message footer");
+
+    ProjectConfig config = projectCache.checkedGet(project).getConfig();
+    config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
+    saveProjectConfig(project, config);
+    pushForReviewOk(testRepo);
+  }
+
+  @Test
+  public void pushWithMultipleChangeIds() throws Exception {
+    createCommit(testRepo,
+        "Message with multiple Change-Id\n"
+            + "\n"
+            + "Change-Id: I10f98c2ef76e52e23aa23be5afeb71e40b350e86\n"
+            + "Change-Id: Ie9a132e107def33bdd513b7854b50de911edba0a\n");
+    pushForReviewRejected(testRepo,
+        "multiple Change-Id lines in commit message footer");
+
+    ProjectConfig config = projectCache.checkedGet(project).getConfig();
+    config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
+    saveProjectConfig(project, config);
+    pushForReviewRejected(testRepo,
+        "multiple Change-Id lines in commit message footer");
+  }
+
+  @Test
+  public void pushWithInvalidChangeId() throws Exception {
+    createCommit(testRepo, "Message with invalid Change-Id\n"
+        + "\n"
+        + "Change-Id: X\n");
+    pushForReviewRejected(testRepo,
+        "invalid Change-Id line format in commit message footer");
+
+    ProjectConfig config = projectCache.checkedGet(project).getConfig();
+    config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
+    saveProjectConfig(project, config);
+    pushForReviewRejected(testRepo,
+        "invalid Change-Id line format in commit message footer");
+  }
+
+  @Test
+  public void pushWithInvalidChangeIdFromEgit() throws Exception {
+    createCommit(testRepo, "Message with invalid Change-Id\n"
+        + "\n"
+        + "Change-Id: I0000000000000000000000000000000000000000\n");
+    pushForReviewRejected(testRepo,
+        "invalid Change-Id line format in commit message footer");
+
+    ProjectConfig config = projectCache.checkedGet(project).getConfig();
+    config.getProject().setRequireChangeID(InheritableBoolean.FALSE);
+    saveProjectConfig(project, config);
+    pushForReviewRejected(testRepo,
+        "invalid Change-Id line format in commit message footer");
+  }
+
+  private static RevCommit createCommit(TestRepository<?> testRepo,
+      String message) throws Exception {
+    return testRepo.branch("HEAD").commit().message(message)
+        .add("a.txt", "content").create();
+  }
+
   @Test
   public void cantAutoCloseChangeAlreadyMergedToBranch() throws Exception {
     PushOneCommit.Result r1 = createChange();
@@ -1239,4 +1306,27 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest {
     assertThat(cds).named("change " + id).hasSize(1);
     return cds.get(0);
   }
+
+  private static void pushForReviewOk(TestRepository<?> testRepo)
+      throws GitAPIException {
+    pushForReview(testRepo, RemoteRefUpdate.Status.OK, null);
+  }
+
+  private static void pushForReviewRejected(TestRepository<?> testRepo,
+      String expectedMessage) throws GitAPIException {
+    pushForReview(testRepo, RemoteRefUpdate.Status.REJECTED_OTHER_REASON,
+        expectedMessage);
+  }
+
+  private static void pushForReview(TestRepository<?> testRepo,
+      RemoteRefUpdate.Status expectedStatus, String expectedMessage)
+          throws GitAPIException {
+    String ref = "refs/for/master";
+    PushResult r = pushHead(testRepo, ref);
+    RemoteRefUpdate refUpdate = r.getRemoteUpdate(ref);
+    assertThat(refUpdate.getStatus()).isEqualTo(expectedStatus);
+    if (expectedMessage != null) {
+      assertThat(refUpdate.getMessage()).contains(expectedMessage);
+    }
+  }
 }

gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiMetrics.java

@@ -50,7 +50,7 @@ public class RestApiMetrics {
 
     errorCount = metrics.newCounter(
         "http/server/rest_api/error_count",
-        new Description("REST API calls by view")
+        new Description("REST API errors by view")
           .setRate(),
         view,
         Field.ofInteger("error_code", "HTTP status code"));

gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java

@@ -1762,13 +1762,6 @@ public class ReceiveCommits {
         }
 
         String idStr = idList.get(idList.size() - 1).trim();
-        if (idStr.matches("^I00*$")) {
-          // Reject this invalid line from EGit.
-          reject(magicBranch.cmd, "invalid Change-Id");
-          newChanges = Collections.emptyList();
-          return;
-        }
-
         pending.add(new ChangeLookup(c, new Change.Key(idStr)));
         int n = pending.size() + newChanges.size();
         if (maxBatchChanges != 0 && n > maxBatchChanges) {

gerrit-server/src/main/java/com/google/gerrit/server/git/validators/CommitValidators.java

@@ -268,7 +268,9 @@ public class CommitValidators {
         throw new CommitValidationException(errMsg, messages);
       } else {
         String v = idList.get(idList.size() - 1).trim();
-        if (!CHANGE_ID.matcher(v).matches()) {
+        // Reject Change-Ids with wrong format and invalid placeholder ID from
+        // Egit (I0000000000000000000000000000000000000000).
+        if (!CHANGE_ID.matcher(v).matches() || v.matches("^I00*$")) {
           String errMsg = String.format(INVALID_CHANGE_ID_MSG, sha1);
           messages.add(
             getMissingChangeIdErrorMsg(errMsg, receiveEvent.commit));

gerrit-server/src/main/resources/com/google/gerrit/server/tools/root/hooks/commit-msg

@@ -19,7 +19,7 @@
 
 unset GREP_OPTIONS
 
-CHANGE_ID_AFTER="Bug|Issue|Test|Feature|Fixes|Fixed"
+CHANGE_ID_AFTER="Bug|Depends-On|Issue|Test|Feature|Fixes|Fixed"
 MSG="$1"
 
 # Check for, and add if missing, a unique Change-Id