Add tests for ChangeNotesParser parse failures

Change-Id: I895b75bb502c2057aea0eb24c6b43c721efda7b9

gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeUpdate.java

@@ -15,9 +15,7 @@
 package com.google.gerrit.server.notedb;
 
 import static com.google.common.base.Preconditions.checkArgument;
-import static com.google.gerrit.server.notedb.ChangeNoteUtil.GERRIT_PLACEHOLDER_HOST;
 
-import com.google.gerrit.common.data.AccountInfo;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
@@ -171,10 +169,8 @@ public abstract class AbstractChangeUpdate extends VersionedMetaData {
   }
 
   protected PersonIdent newIdent(Account author, Date when) {
-    return new PersonIdent(
+    return ChangeNoteUtil.newIdent(author, when, serverIdent,
-        new AccountInfo(author).getName(anonymousCowardName),
+        anonymousCowardName);
-        author.getId().get() + "@" + GERRIT_PLACEHOLDER_HOST,
-        when, serverIdent.getTimeZone());
   }
 
   /**

gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNoteUtil.java

@@ -14,11 +14,16 @@
 
 package com.google.gerrit.server.notedb;
 
+import com.google.gerrit.common.data.AccountInfo;
+import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.RefNames;
 
+import org.eclipse.jgit.lib.PersonIdent;
 import org.eclipse.jgit.revwalk.FooterKey;
 
+import java.util.Date;
+
 public class ChangeNoteUtil {
   static final String GERRIT_PLACEHOLDER_HOST = "gerrit";
 
@@ -43,6 +48,14 @@ public class ChangeNoteUtil {
     return r.toString();
   }
 
+  static PersonIdent newIdent(Account author, Date when,
+      PersonIdent serverIdent, String anonymousCowardName) {
+    return new PersonIdent(
+        new AccountInfo(author).getName(anonymousCowardName),
+        author.getId().get() + "@" + GERRIT_PLACEHOLDER_HOST,
+        when, serverIdent.getTimeZone());
+  }
+
   private ChangeNoteUtil() {
   }
 }

gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java

@@ -80,12 +80,12 @@ public class AbstractChangeNotesTest {
   protected IdentifiedUser.GenericFactory userFactory;
   protected IdentifiedUser otherUser;
   protected InMemoryRepository repo;
+  protected InMemoryRepositoryManager repoManager;
   protected PersonIdent serverIdent;
   protected Project.NameKey project;
 
   private AllUsersNameProvider allUsers;
   private Injector injector;
-  private InMemoryRepositoryManager repoManager;
   private String systemTimeZone;
   private volatile long clockStepMs;
 

gerrit-server/src/test/java/com/google/gerrit/server/notedb/ChangeNotesParserTest.java

@@ -0,0 +1,218 @@
+// 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.server.notedb;
+
+import static org.junit.Assert.fail;
+
+import com.google.gerrit.server.util.TimeUtil;
+
+import org.eclipse.jgit.errors.ConfigInvalidException;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.eclipse.jgit.lib.CommitBuilder;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectInserter;
+import org.eclipse.jgit.lib.PersonIdent;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+
+public class ChangeNotesParserTest extends AbstractChangeNotesTest {
+  private TestRepository<InMemoryRepository> testRepo;
+  private RevWalk walk;
+
+  @Before
+  public void setUpTestRepo() throws Exception {
+    testRepo = new TestRepository<>(repo);
+    walk = new RevWalk(repo);
+  }
+
+  @After
+  public void tearDownTestRepo() throws Exception {
+    walk.release();
+  }
+
+  @Test
+  public void parseAuthor() throws Exception {
+    assertParseSucceeds("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n");
+    assertParseFails(writeCommit("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n",
+        new PersonIdent("Change Owner", "owner@example.com",
+          serverIdent.getWhen(), serverIdent.getTimeZone())));
+    assertParseFails(writeCommit("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n",
+        new PersonIdent("Change Owner", "x@gerrit",
+          serverIdent.getWhen(), serverIdent.getTimeZone())));
+  }
+
+  @Test
+  public void parseStatus() throws Exception {
+    assertParseSucceeds("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Status: NEW\n");
+    assertParseSucceeds("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Status: new\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Status: OOPS\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Status: NEW\n"
+        + "Status: NEW\n");
+  }
+
+  @Test
+  public void parsePatchSetId() throws Exception {
+    assertParseSucceeds("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n");
+    assertParseFails("Update change\n"
+        + "\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Patch-Set: 1\n");
+    assertParseSucceeds("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: x\n");
+  }
+
+  @Test
+  public void parseApproval() throws Exception {
+    assertParseSucceeds("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Label: Label1=+1\n"
+        + "Label: Label2=1\n"
+        + "Label: Label3=0\n"
+        + "Label: Label4=-1\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Label: Label1=X\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Label: Label1 = 1\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Label: X+Y\n");
+  }
+
+  @Test
+  public void parseSubmitRecords() throws Exception {
+    assertParseSucceeds("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Submitted-with: NOT_READY\n"
+        + "Submitted-with: OK: Verified: Change Owner <1@gerrit>\n"
+        + "Submitted-with: NEED: Code-Review\n"
+        + "Submitted-with: NOT_READY\n"
+        + "Submitted-with: OK: Verified: Change Owner <1@gerrit>\n"
+        + "Submitted-with: NEED: Alternative-Code-Review\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Submitted-with: OOPS\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Submitted-with: NEED: X+Y\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Submitted-with: OK: X+Y: Change Owner <1@gerrit>\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Submitted-with: OK: Code-Review: 1@gerrit\n");
+  }
+
+  @Test
+  public void parseReviewer() throws Exception {
+    assertParseSucceeds("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Reviewer: Change Owner <1@gerrit>\n"
+        + "CC: Other Account <2@gerrit>\n");
+    assertParseFails("Update change\n"
+        + "\n"
+        + "Patch-Set: 1\n"
+        + "Reviewer: 1@gerrit\n");
+  }
+
+  private RevCommit writeCommit(String body) throws Exception {
+    return writeCommit(body, ChangeNoteUtil.newIdent(
+        changeOwner.getAccount(), TimeUtil.nowTs(), serverIdent,
+        "Anonymous Coward"));
+  }
+
+  private RevCommit writeCommit(String body, PersonIdent author)
+      throws Exception {
+    ObjectInserter ins = testRepo.getRepository().newObjectInserter();
+    try {
+      CommitBuilder cb = new CommitBuilder();
+      cb.setAuthor(author);
+      cb.setCommitter(new PersonIdent(serverIdent, author.getWhen()));
+      cb.setTreeId(testRepo.tree());
+      cb.setMessage(body);
+      ObjectId id = ins.insert(cb);
+      ins.flush();
+      RevCommit commit = walk.parseCommit(id);
+      walk.parseBody(commit);
+      return commit;
+    } finally {
+      ins.release();
+    }
+  }
+
+  private void assertParseSucceeds(String body) throws Exception {
+    try (ChangeNotesParser parser = newParser(writeCommit(body))) {
+      parser.parseAll();
+    }
+  }
+
+  private void assertParseFails(String body) throws Exception {
+    assertParseFails(writeCommit(body));
+  }
+
+  private void assertParseFails(RevCommit commit) throws Exception {
+    try (ChangeNotesParser parser = newParser(commit)) {
+      parser.parseAll();
+      fail("Expected parse to fail:\n" + commit.getFullMessage());
+    } catch (ConfigInvalidException e) {
+      // Expected.
+    }
+  }
+
+  private ChangeNotesParser newParser(ObjectId tip) throws Exception {
+    return new ChangeNotesParser(newChange(), tip, walk, repoManager);
+  }
+}