diff --git a/java/com/google/gerrit/server/group/Rebuild.java b/java/com/google/gerrit/server/group/Rebuild.java index fcf873c25f..d3d59f8a87 100644 --- a/java/com/google/gerrit/server/group/Rebuild.java +++ b/java/com/google/gerrit/server/group/Rebuild.java @@ -103,7 +103,7 @@ public class Rebuild implements RestModifyView { GroupBundle noteDbBundle = bundleFactory.fromNoteDb(repo, uuid); - List diffs = GroupBundle.compare(reviewDbBundle, noteDbBundle); + List diffs = GroupBundle.compareWithAudits(reviewDbBundle, noteDbBundle); if (diffs.isEmpty()) { return BinaryResult.create("No differences between ReviewDb and NoteDb"); } diff --git a/java/com/google/gerrit/server/group/db/GroupBundle.java b/java/com/google/gerrit/server/group/db/GroupBundle.java index 03bb1fde0b..5d339c0506 100644 --- a/java/com/google/gerrit/server/group/db/GroupBundle.java +++ b/java/com/google/gerrit/server/group/db/GroupBundle.java @@ -259,8 +259,18 @@ public abstract class GroupBundle { return new AutoValue_GroupBundle.Builder().members().memberAudit().byId().byIdAudit(); } - public static ImmutableList compare( + public static ImmutableList compareWithAudits( GroupBundle reviewDbBundle, GroupBundle noteDbBundle) { + return compare(reviewDbBundle, noteDbBundle, true); + } + + public static ImmutableList compareWithoutAudits( + GroupBundle reviewDbBundle, GroupBundle noteDbBundle) { + return compare(reviewDbBundle, noteDbBundle, false); + } + + private static ImmutableList compare( + GroupBundle reviewDbBundle, GroupBundle noteDbBundle, boolean compareAudits) { // Normalize the ReviewDb bundle to what we expect in NoteDb. This means that values in error // messages will not reflect the actual data in ReviewDb, but it will make it easier for humans // to see the difference. @@ -293,7 +303,9 @@ public abstract class GroupBundle { + ("ReviewDb: " + reviewDbBundle.members() + "\n") + ("NoteDb : " + noteDbBundle.members())); } - if (!areMemberAuditsConsideredEqual(reviewDbBundle.memberAudit(), noteDbBundle.memberAudit())) { + if (compareAudits + && !areMemberAuditsConsideredEqual( + reviewDbBundle.memberAudit(), noteDbBundle.memberAudit())) { result.add( "AccountGroupMemberAudits differ\n" + ("ReviewDb: " + reviewDbBundle.memberAudit() + "\n") @@ -305,7 +317,8 @@ public abstract class GroupBundle { + ("ReviewDb: " + reviewDbBundle.byId() + "\n") + ("NoteDb : " + noteDbBundle.byId())); } - if (!areByIdAuditsConsideredEqual(reviewDbBundle.byIdAudit(), noteDbBundle.byIdAudit())) { + if (compareAudits + && !areByIdAuditsConsideredEqual(reviewDbBundle.byIdAudit(), noteDbBundle.byIdAudit())) { result.add( "AccountGroupByIdAudits differ\n" + ("ReviewDb: " + reviewDbBundle.byIdAudit() + "\n") diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java index d331471ee5..6b8eb14380 100644 --- a/javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java +++ b/javatests/com/google/gerrit/acceptance/api/group/GroupRebuilderIT.java @@ -214,7 +214,7 @@ public class GroupRebuilderIT extends AbstractDaemonTest { } private void assertMigratedCleanly(GroupBundle noteDbBundle, GroupBundle expectedReviewDbBundle) { - assertThat(GroupBundle.compare(expectedReviewDbBundle, noteDbBundle)).isEmpty(); + assertThat(GroupBundle.compareWithAudits(expectedReviewDbBundle, noteDbBundle)).isEmpty(); } private ImmutableList log(GroupInfo g) throws Exception { diff --git a/javatests/com/google/gerrit/server/group/db/GroupBundleTest.java b/javatests/com/google/gerrit/server/group/db/GroupBundleTest.java index 6c67877640..b449090f5f 100644 --- a/javatests/com/google/gerrit/server/group/db/GroupBundleTest.java +++ b/javatests/com/google/gerrit/server/group/db/GroupBundleTest.java @@ -66,7 +66,7 @@ public class GroupBundleTest extends GerritBaseTests { AccountGroup g2 = new AccountGroup(reviewDbBundle.group()); g2.setDescription("Hello!"); GroupBundle noteDbBundle = GroupBundle.builder().source(Source.NOTE_DB).group(g2).build(); - assertThat(GroupBundle.compare(reviewDbBundle, noteDbBundle)) + assertThat(GroupBundle.compareWithAudits(reviewDbBundle, noteDbBundle)) .containsExactly( "AccountGroups differ\n" + ("ReviewDb: AccountGroup{name=group, groupId=1, description=null," @@ -93,11 +93,32 @@ public class GroupBundleTest extends GerritBaseTests { + "NoteDb : []"); } + @Test + public void compareIgnoreAudits() throws Exception { + GroupBundle reviewDbBundle = newBundle().source(Source.REVIEW_DB).build(); + AccountGroup group = new AccountGroup(reviewDbBundle.group()); + + AccountGroupMember member = + new AccountGroupMember(new AccountGroupMember.Key(new Account.Id(1), group.getId())); + AccountGroupMemberAudit memberAudit = + new AccountGroupMemberAudit(member, new Account.Id(2), ts); + AccountGroupById byId = + new AccountGroupById( + new AccountGroupById.Key(group.getId(), new AccountGroup.UUID("subgroup-2"))); + AccountGroupByIdAud byIdAudit = new AccountGroupByIdAud(byId, new Account.Id(3), ts); + + GroupBundle noteDbBundle = + newBundle().source(Source.NOTE_DB).memberAudit(memberAudit).byIdAudit(byIdAudit).build(); + + assertThat(GroupBundle.compareWithAudits(reviewDbBundle, noteDbBundle)).isNotEmpty(); + assertThat(GroupBundle.compareWithoutAudits(reviewDbBundle, noteDbBundle)).isEmpty(); + } + @Test public void compareEqual() throws Exception { GroupBundle reviewDbBundle = newBundle().source(Source.REVIEW_DB).build(); GroupBundle noteDbBundle = newBundle().source(Source.NOTE_DB).build(); - assertThat(GroupBundle.compare(reviewDbBundle, noteDbBundle)).isEmpty(); + assertThat(GroupBundle.compareWithAudits(reviewDbBundle, noteDbBundle)).isEmpty(); } private GroupBundle.Builder newBundle() { diff --git a/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java b/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java index bf97083ec0..51cf9879c1 100644 --- a/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java +++ b/javatests/com/google/gerrit/server/group/db/GroupRebuilderTest.java @@ -592,7 +592,7 @@ public class GroupRebuilderTest extends AbstractGroupTest { } private void assertMigratedCleanly(GroupBundle noteDbBundle, GroupBundle expectedReviewDbBundle) { - assertThat(GroupBundle.compare(expectedReviewDbBundle, noteDbBundle)).isEmpty(); + assertThat(GroupBundle.compareWithAudits(expectedReviewDbBundle, noteDbBundle)).isEmpty(); } private AccountGroup newGroup(String name) {