NoteDb: Fix conversion of PatchSet.pushCertificate field

RevisionNote was excluding the trailing newline from the push cert
parsed out of the note, even though it was present in NoteDb. Include
the newline so they match. However, this diff is spurious anyway--
OpenPGP parsers don't care whether there are trailing newlines. So
harden ChangeBundle against trailing newlines not matching as well.

Change-Id: Idc85ab226040ab2e507cf7a83c0b4cd5286ffb58

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ChangeRebuilderIT.java

@@ -358,6 +358,43 @@ public class ChangeRebuilderIT extends AbstractDaemonTest {
     assertDraftsUpToDate(true, id, user);
   }
 
+  @Test
+  public void pushCert() throws Exception {
+    // We don't have the code in our test harness to do signed pushes, so just
+    // use a hard-coded cert. This cert was actually generated by C git 2.2.0
+    // (albeit not for sending to Gerrit).
+    String cert = "certificate version 0.1\n"
+        + "pusher Dave Borowitz <dborowitz@google.com> 1433954361 -0700\n"
+        + "pushee git://localhost/repo.git\n"
+        + "nonce 1433954361-bde756572d665bba81d8\n"
+        + "\n"
+        + "0000000000000000000000000000000000000000"
+        + "b981a177396fb47345b7df3e4d3f854c6bea7"
+        + "s/heads/master\n"
+        + "-----BEGIN PGP SIGNATURE-----\n"
+        + "Version: GnuPG v1\n"
+        + "\n"
+        + "iQEcBAABAgAGBQJVeGg5AAoJEPfTicJkUdPkUggH/RKAeI9/i/LduuiqrL/SSdIa\n"
+        + "9tYaSqJKLbXz63M/AW4Sp+4u+dVCQvnAt/a35CVEnpZz6hN4Kn/tiswOWVJf4CO7\n"
+        + "htNubGs5ZMwvD6sLYqKAnrM3WxV/2TbbjzjZW6Jkidz3jz/WRT4SmjGYiEO7aA+V\n"
+        + "4ZdIS9f7sW5VsHHYlNThCA7vH8Uu48bUovFXyQlPTX0pToSgrWV3JnTxDNxfn3iG\n"
+        + "IL0zTY/qwVCdXgFownLcs6J050xrrBWIKqfcWr3u4D2aCLyR0v+S/KArr7ulZygY\n"
+        + "+SOklImn8TAZiNxhWtA6ens66IiammUkZYFv7SSzoPLFZT4dC84SmGPWgf94NoQ=\n"
+        + "=XFeC\n"
+        + "-----END PGP SIGNATURE-----\n";
+
+    PushOneCommit.Result r = createChange();
+    PatchSet.Id psId = r.getPatchSetId();
+    Change.Id id = psId.getParentKey();
+
+    PatchSet ps = db.patchSets().get(psId);
+    ps.setPushCertificate(cert);
+    db.patchSets().update(Collections.singleton(ps));
+    indexer.index(db, project, id);
+
+    checker.rebuildAndCheckChanges(id);
+  }
+
   private void setInvalidNoteDbState(Change.Id id) throws Exception {
     ReviewDb db = unwrapDb();
     Change c = db.changes().get(id);

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

@@ -22,6 +22,7 @@ import static com.google.gerrit.reviewdb.server.ReviewDbUtil.intKeyOrdering;
 import static com.google.gerrit.server.notedb.ChangeBundle.Source.NOTE_DB;
 import static com.google.gerrit.server.notedb.ChangeBundle.Source.REVIEW_DB;
 
+import com.google.common.base.CharMatcher;
 import com.google.common.base.Joiner;
 import com.google.common.collect.ComparisonChain;
 import com.google.common.collect.ImmutableCollection;
@@ -379,10 +380,20 @@ public class ChangeBundle {
       PatchSet a = as.get(id);
       PatchSet b = bs.get(id);
       String desc = describe(id);
-      diffColumns(diffs, PatchSet.class, desc, bundleA, a, bundleB, b);
+      String pushCertField = "pushCertificate";
+      diffColumnsExcluding(diffs, PatchSet.class, desc, bundleA, a, bundleB, b,
+          pushCertField);
+      diffValues(diffs, desc, trimPushCert(a), trimPushCert(b), pushCertField);
     }
   }
 
+  private static String trimPushCert(PatchSet ps) {
+    if (ps.getPushCertificate() == null) {
+      return null;
+    }
+    return CharMatcher.is('\n').trimTrailingFrom(ps.getPushCertificate());
+  }
+
   private static void diffPatchSetApprovals(List<String> diffs,
       ChangeBundle bundleA, ChangeBundle bundleB) {
     Map<PatchSetApproval.Key, PatchSetApproval> as = bundleA.patchSetApprovals;

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

@@ -37,7 +37,7 @@ class RevisionNote {
       "certificate version ".getBytes(UTF_8);
   // See org.eclipse.jgit.transport.PushCertificateParser.END_SIGNATURE
   private static final byte[] END_SIGNATURE =
-      "-----END PGP SIGNATURE-----".getBytes(UTF_8);
+      "-----END PGP SIGNATURE-----\n".getBytes(UTF_8);
 
   private static void trimLeadingEmptyLines(byte[] bytes, MutableInteger p) {
     while (p.value < bytes.length && bytes[p.value] == '\n') {

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

@@ -482,6 +482,34 @@ public class ChangeBundleTest {
     assertDiffs(b3, b1, msg);
   }
 
+  @Test
+  public void diffPatchSetsIgnoresTrailingNewlinesInPushCertificate()
+      throws Exception {
+    subWindowResolution();
+    Change c = TestChanges.newChange(project, accountId);
+    PatchSet ps1 = new PatchSet(c.currentPatchSetId());
+    ps1.setRevision(new RevId("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"));
+    ps1.setUploader(accountId);
+    ps1.setCreatedOn(roundToSecond(TimeUtil.nowTs()));
+    ps1.setPushCertificate("some cert");
+    PatchSet ps2 = clone(ps1);
+    ps2.setPushCertificate(ps2.getPushCertificate() + "\n\n");
+
+    ChangeBundle b1 = new ChangeBundle(c, messages(), patchSets(ps1),
+        approvals(), comments(), NOTE_DB);
+    ChangeBundle b2 = new ChangeBundle(c, messages(), patchSets(ps2),
+        approvals(), comments(), REVIEW_DB);
+    assertNoDiffs(b1, b2);
+    assertNoDiffs(b2, b1);
+
+    b1 = new ChangeBundle(c, messages(), patchSets(ps1), approvals(),
+        comments(), REVIEW_DB);
+    b2 = new ChangeBundle(c, messages(), patchSets(ps2), approvals(),
+        comments(), NOTE_DB);
+    assertNoDiffs(b1, b2);
+    assertNoDiffs(b2, b1);
+  }
+
   @Test
   public void diffPatchSetApprovalKeySets() throws Exception {
     Change c = TestChanges.newChange(project, accountId);

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

@@ -23,7 +23,6 @@ import static com.google.gerrit.testutil.TestChanges.incrementPatchSet;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.eclipse.jgit.lib.Constants.OBJ_BLOB;
 
-import com.google.common.base.CharMatcher;
 import com.google.common.base.Function;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
@@ -900,7 +899,6 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
       + "\n"
       + "Nor is this a real signature.\n"
       + "-----END PGP SIGNATURE-----\n";
-    String trimmedCert = CharMatcher.is('\n').trimTrailingFrom(pushCert);
 
     // ps2 with push cert
     Change c = newChange();
@@ -917,8 +915,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
     assertThat(readNote(notes, commit)).isEqualTo(pushCert);
     Map<PatchSet.Id, PatchSet> patchSets = notes.getPatchSets();
     assertThat(patchSets.get(psId1).getPushCertificate()).isNull();
-    assertThat(patchSets.get(psId2).getPushCertificate())
-        .isEqualTo(trimmedCert);
+    assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert);
     assertThat(notes.getComments()).isEmpty();
 
     // comment on ps2
@@ -946,8 +943,7 @@ public class ChangeNotesTest extends AbstractChangeNotesTest {
         + "\n");
     patchSets = notes.getPatchSets();
     assertThat(patchSets.get(psId1).getPushCertificate()).isNull();
-    assertThat(patchSets.get(psId2).getPushCertificate())
-        .isEqualTo(trimmedCert);
+    assertThat(patchSets.get(psId2).getPushCertificate()).isEqualTo(pushCert);
     assertThat(notes.getComments()).isNotEmpty();
   }