Merge "ReceiveCommits: Fix set comparison to avoid no-op group updates"

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/GetRelatedIT.java

@@ -27,6 +27,7 @@ import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.common.CommitInfo;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.server.change.ChangesCollection;
 import com.google.gerrit.server.change.GetRelated.ChangeAndCommit;
 import com.google.gerrit.server.change.GetRelated.RelatedInfo;
 import com.google.gerrit.server.edit.ChangeEditModifier;
@@ -71,6 +72,9 @@ public class GetRelatedIT extends AbstractDaemonTest {
   @Inject
   private BatchUpdate.Factory updateFactory;
 
+  @Inject
+  private ChangesCollection changes;
+
   @Test
   public void getRelatedNoResult() throws Exception {
     PushOneCommit push = pushFactory.create(db, admin.getIdent(), testRepo);
@@ -99,6 +103,38 @@ public class GetRelatedIT extends AbstractDaemonTest {
     }
   }
 
+  @Test
+  public void getRelatedLinearSeparatePushes() throws Exception {
+    // 1,1---2,1
+    RevCommit c1_1 = commitBuilder()
+        .add("a.txt", "1")
+        .message("subject: 1")
+        .create();
+    RevCommit c2_1 = commitBuilder()
+        .add("b.txt", "2")
+        .message("subject: 2")
+        .create();
+
+    testRepo.reset(c1_1);
+    pushHead(testRepo, "refs/for/master", false);
+    PatchSet.Id ps1_1 = getPatchSetId(c1_1);
+    String oldETag = changes.parse(ps1_1.getParentKey()).getETag();
+
+    testRepo.reset(c2_1);
+    pushHead(testRepo, "refs/for/master", false);
+    PatchSet.Id ps2_1 = getPatchSetId(c2_1);
+
+    // Push of change 2 should not affect groups (or anything else) of change 1.
+    assertThat(changes.parse(ps1_1.getParentKey()).getETag())
+        .isEqualTo(oldETag);
+
+    for (PatchSet.Id ps : ImmutableList.of(ps2_1, ps1_1)) {
+      assertRelated(ps,
+          changeAndCommit(ps2_1, c2_1, 1),
+          changeAndCommit(ps1_1, c1_1, 1));
+    }
+  }
+
   @Test
   public void getRelatedReorder() throws Exception {
     // 1,1---2,1

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

@@ -2448,7 +2448,7 @@ public class ReceiveCommits {
               if (groups == null) {
                 return false;
               }
-            } else if (Sets.newHashSet(oldGroups).equals(groups)) {
+            } else if (sameGroups(oldGroups, groups)) {
               return false;
             }
             psUtil.setGroups(ctx.getDb(), ctx.getUpdate(psId), ps, groups);
@@ -2459,6 +2459,10 @@ public class ReceiveCommits {
       }
     }
 
+    private boolean sameGroups(List<String> a, List<String> b) {
+      return Sets.newHashSet(a).equals(Sets.newHashSet(b));
+    }
+
     CheckedFuture<Void, RestApiException> updateGroups() {
       final Thread caller = Thread.currentThread();
       ListenableFuture<Void> future = changeUpdateExector.submit(