commit 73926747285a04ea28ca1933e24762f60cfc3986
author Sasa Zivkov <sasa.zivkov@sap.com> Tue Oct 08 17:41:17 2013 +0200
committer Saša Živkov <zivkov@gmail.com> Tue Oct 08 15:54:47 2013 +0000
tree 9ef81be1d7cc9500553490c2e1d1b3620067633b
parent d675391765cd5b57e33296fbb548318e104b3478

Create review note also when newObjectId already present in another branch

Using the %base option of the refs/for/master ref one can push a commit
for review even when it is already known to Gerrit (already present in
another branch). Since we used to mark as uninteresting the tips of all
branches, the tree walk used to identify commits for which to create review
notes wouldn't find such a commit.

This change fixes only the easy case when the newObjectId repesents a normal
(non-merge) commit and when the oldObjectId is the parent of the newObjectId.

Change-Id: Ide86cde5a8359b8764a9e1984d29865b604059ec

src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java

diff --git a/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java b/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java
index 4e8642c..eddfadc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/reviewnotes/CreateReviewNotes.java
@@ -38,6 +38,7 @@
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelTypes;
+import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
@@ -122,8 +123,13 @@
 
     RevWalk rw = new RevWalk(git);
     try {
-      rw.markStart(rw.parseCommit(newObjectId));
-      markUninteresting(git, branch, rw, oldObjectId);
+      RevCommit n = rw.parseCommit(newObjectId);
+      rw.markStart(n);
+      if (n.getParentCount() == 1 && n.getParent(0).equals(oldObjectId)) {
+        rw.markUninteresting(rw.parseCommit(oldObjectId));
+      } else {
+        markUninteresting(git, branch, rw, oldObjectId);
+      }
     } catch (Exception e) {
       log.error(e.getMessage(), e);
       return;
@@ -135,7 +141,7 @@
 
     try {
       for (RevCommit c : rw) {
-        ObjectId content = createNoteContent(c);
+        ObjectId content = createNoteContent(loadPatchSet(c, branch));
         if (content != null) {
           monitor.update(1);
           getNotes().set(c, content);
@@ -225,12 +231,8 @@
     return null;
   }
 
-  private ObjectId createNoteContent(RevCommit c)
-      throws OrmException, IOException {
-    return createNoteContent(loadPatchSet(c));
-  }
-
-  private PatchSet loadPatchSet(RevCommit c) throws OrmException {
+  private PatchSet loadPatchSet(RevCommit c, String destBranch)
+      throws OrmException {
     List<PatchSet> patches = reviewDb.patchSets().byRevision(new RevId(c.name()))
         .toList();
     if (patches.isEmpty()) {
@@ -238,9 +240,11 @@
     } else if (patches.size() == 1) {
       return patches.get(0);
     } else {
+      Branch.NameKey dest = new Branch.NameKey(project, destBranch);
       for (PatchSet ps : patches) {
         Change.Id cid = ps.getId().getParentKey();
-        if (reviewDb.changes().get(cid).getProject().equals(project)) {
+        Change change = reviewDb.changes().get(cid);
+        if (change.getDest().equals(dest)) {
           return ps;
         }
       }