Google Git
Sign in
gerrit/gerrit/9f911903c9b6e1280357bfd5741ee70dcf3cb00f^!/.
commit
9f911903c9b6e1280357bfd5741ee70dcf3cb00f
[log]
author
Patrick Hiesel <hiesel@google.com>
Mon Jan 16 09:44:31 2023 +0000
committer
Gerrit Code Review <noreply-gerritcodereview@google.com>
Mon Jan 16 09:44:31 2023 +0000
tree
8593ac17e962076163949b89d5106edd017e9518
parent
6e8e5de448cb1e696780dd6268ce96218158fbe4 [diff]
Revert "Fix ChangeInfoDiffer.getAddedForMap method"

This reverts commit 6e8e5de448cb1e696780dd6268ce96218158fbe4.

Reason for revert: Need to revert dependency I4fc40fa8e

Change-Id: I2c8398d66966b05e9764c4976909d30877466917

diff --git a/java/com/google/gerrit/extensions/common/ChangeInfoDiffer.java b/java/com/google/gerrit/extensions/common/ChangeInfoDiffer.java
index 51c35dc..24182cc 100644
--- a/java/com/google/gerrit/extensions/common/ChangeInfoDiffer.java
+++ b/java/com/google/gerrit/extensions/common/ChangeInfoDiffer.java

@@ -150,17 +150,16 @@
   /** Returns {@code null} if nothing has been added to {@code oldCollection} */
   @Nullable
   private static ImmutableList<?> getAddedForCollection(
-      @Nullable Collection<?> oldCollection, Collection<?> newCollection) {
-    ImmutableList<?> notInOldCollection = getAdditionsForCollection(oldCollection, newCollection);
+      Collection<?> oldCollection, Collection<?> newCollection) {
+    ImmutableList<?> notInOldCollection = getAdditions(oldCollection, newCollection);
     return notInOldCollection.isEmpty() ? null : notInOldCollection;
   }
 
   @Nullable
-  private static ImmutableList<Object> getAdditionsForCollection(
-      @Nullable Collection<?> oldCollection, Collection<?> newCollection) {
-    if (oldCollection == null) {
-      return ImmutableList.copyOf(newCollection);
-    }
+  private static ImmutableList<Object> getAdditions(
+      Collection<?> oldCollection, Collection<?> newCollection) {
+    if (oldCollection == null)
+      return newCollection != null ? ImmutableList.copyOf(newCollection) : null;
 
     Map<Object, List<Object>> duplicatesMap = newCollection.stream().collect(groupingBy(v -> v));
     oldCollection.forEach(
@@ -174,18 +173,7 @@
 
   /** Returns {@code null} if nothing has been added to {@code oldMap} */
   @Nullable
-  private static ImmutableMap<Object, Object> getAddedForMap(
-      @Nullable Map<?, ?> oldMap, Map<?, ?> newMap) {
-    ImmutableMap<Object, Object> notInOldMap = getAdditionsForMap(oldMap, newMap);
-    return notInOldMap.isEmpty() ? null : notInOldMap;
-  }
-
-  @Nullable
-  private static ImmutableMap<Object, Object> getAdditionsForMap(
-      @Nullable Map<?, ?> oldMap, Map<?, ?> newMap) {
-    if (oldMap == null) {
-      return ImmutableMap.copyOf(newMap);
-    }
+  private static ImmutableMap<Object, Object> getAddedForMap(Map<?, ?> oldMap, Map<?, ?> newMap) {
     ImmutableMap.Builder<Object, Object> additionsBuilder = ImmutableMap.builder();
     for (Map.Entry<?, ?> entry : newMap.entrySet()) {
       Object added = getAdded(oldMap.get(entry.getKey()), entry.getValue());
@@ -193,7 +181,8 @@
         additionsBuilder.put(entry.getKey(), added);
       }
     }
-    return additionsBuilder.build();
+    ImmutableMap<Object, Object> additions = additionsBuilder.build();
+    return additions.isEmpty() ? null : additions;
   }
 
   private static Object get(Field field, Object obj) {

diff --git a/javatests/com/google/gerrit/extensions/common/ChangeInfoDifferTest.java b/javatests/com/google/gerrit/extensions/common/ChangeInfoDifferTest.java
index f45d33b..7ed236a 100644
--- a/javatests/com/google/gerrit/extensions/common/ChangeInfoDifferTest.java
+++ b/javatests/com/google/gerrit/extensions/common/ChangeInfoDifferTest.java

@@ -48,7 +48,6 @@
     assertThat(diff.added().messages).isNull();
     assertThat(diff.added().reviewers).isNull();
     assertThat(diff.added().hashtags).isNull();
-    assertThat(diff.added().removableLabels).isNull();
     assertThat(diff.removed()._number).isNull();
     assertThat(diff.removed().branch).isNull();
     assertThat(diff.removed().project).isNull();
@@ -57,7 +56,6 @@
     assertThat(diff.removed().messages).isNull();
     assertThat(diff.removed().reviewers).isNull();
     assertThat(diff.removed().hashtags).isNull();
-    assertThat(diff.removed().removableLabels).isNull();
   }
 
   @Test
@@ -317,295 +315,6 @@
   }
 
   @Test
-  public void getDiff_removableLabelsEmpty_returnsNullRemovableLabels() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    oldChangeInfo.removableLabels = ImmutableMap.of();
-    newChangeInfo.removableLabels = ImmutableMap.of();
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels).isNull();
-    assertThat(diff.removed().removableLabels).isNull();
-  }
-
-  @Test
-  public void getDiff_removableLabelsNullAndEmpty_returnsEmptyRemovableLabels() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    newChangeInfo.removableLabels = ImmutableMap.of();
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels).isEmpty();
-    assertThat(diff.removed().removableLabels).isNull();
-  }
-
-  @Test
-  public void getDiff_removableLabelsEmptyAndNull_returnsEmptyRemovableLabels() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    oldChangeInfo.removableLabels = ImmutableMap.of();
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels).isNull();
-    assertThat(diff.removed().removableLabels).isEmpty();
-  }
-
-  @Test
-  public void getDiff_removableLabelsLabelAdded() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "Cow";
-    AccountInfo acc2 = new AccountInfo();
-    acc2.name = "Pig";
-    AccountInfo acc3 = new AccountInfo();
-    acc3.name = "Cat";
-    AccountInfo acc4 = new AccountInfo();
-    acc4.name = "Dog";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of(
-            "Code-Review",
-            ImmutableMap.of("+1", ImmutableList.of(acc1), "-1", ImmutableList.of(acc2, acc3)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of(
-            "Code-Review",
-            ImmutableMap.of("+1", ImmutableList.of(acc1), "-1", ImmutableList.of(acc2, acc3)),
-            "Verified",
-            ImmutableMap.of("-1", ImmutableList.of(acc4)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Verified", ImmutableMap.of("-1", ImmutableList.of(acc4))));
-    assertThat(diff.removed().removableLabels).isNull();
-  }
-
-  @Test
-  public void getDiff_removableLabelsLabelRemoved() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "Cow";
-    AccountInfo acc2 = new AccountInfo();
-    acc2.name = "Pig";
-    AccountInfo acc3 = new AccountInfo();
-    acc3.name = "Cat";
-    AccountInfo acc4 = new AccountInfo();
-    acc4.name = "Dog";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of(
-            "Code-Review",
-            ImmutableMap.of("+1", ImmutableList.of(acc1), "-1", ImmutableList.of(acc2, acc3)),
-            "Verified",
-            ImmutableMap.of("-1", ImmutableList.of(acc4)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of(
-            "Code-Review",
-            ImmutableMap.of("+1", ImmutableList.of(acc1), "-1", ImmutableList.of(acc2, acc3)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels).isNull();
-    assertThat(diff.removed().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Verified", ImmutableMap.of("-1", ImmutableList.of(acc4))));
-  }
-
-  @Test
-  public void getDiff_removableLabelsVoteAdded() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "acc1";
-    AccountInfo acc2 = new AccountInfo();
-    acc2.name = "acc2";
-    AccountInfo acc3 = new AccountInfo();
-    acc3.name = "acc3";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of(
-            "Code-Review",
-            ImmutableMap.of("+1", ImmutableList.of(acc1), "-1", ImmutableList.of(acc2, acc3)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("-1", ImmutableList.of(acc2, acc3))));
-    assertThat(diff.removed().removableLabels).isNull();
-  }
-
-  @Test
-  public void getDiff_removableLabelsVoteRemoved() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "acc1";
-    AccountInfo acc2 = new AccountInfo();
-    acc2.name = "acc2";
-    AccountInfo acc3 = new AccountInfo();
-    acc3.name = "acc3";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of(
-            "Code-Review",
-            ImmutableMap.of("+1", ImmutableList.of(acc1), "-1", ImmutableList.of(acc2, acc3)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels).isNull();
-    assertThat(diff.removed().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("-1", ImmutableList.of(acc2, acc3))));
-  }
-
-  @Test
-  public void getDiff_removableLabelsAccountAdded() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "acc1";
-    AccountInfo acc2 = new AccountInfo();
-    acc2.name = "acc2";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1, acc2)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc2))));
-    assertThat(diff.removed().removableLabels).isNull();
-  }
-
-  @Test
-  public void getDiff_removableLabelsAccountRemoved() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "acc1";
-    AccountInfo acc2 = new AccountInfo();
-    acc2.name = "acc2";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1, acc2)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc2))));
-    assertThat(diff.removed().removableLabels).isNull();
-  }
-
-  @Test
-  public void getDiff_removableLabelsAccountChanged() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "acc1";
-    AccountInfo acc2 = new AccountInfo();
-    acc2.name = "acc2";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc2)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc2))));
-    assertThat(diff.removed().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1))));
-  }
-
-  @Test
-  public void getDiff_removableLabelsScoreChanged() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "acc1";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("-1", ImmutableList.of(acc1)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("-1", ImmutableList.of(acc1))));
-    assertThat(diff.removed().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1))));
-  }
-
-  @Test
-  public void getDiff_removableLabelsLabelChanged() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "acc1";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of("Verified", ImmutableMap.of("+1", ImmutableList.of(acc1)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Verified", ImmutableMap.of("+1", ImmutableList.of(acc1))));
-    assertThat(diff.removed().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1))));
-  }
-
-  @Test
-  public void getDiff_removableLabelsLabelScoreAndAccountChanged() {
-    ChangeInfo oldChangeInfo = new ChangeInfo();
-    ChangeInfo newChangeInfo = new ChangeInfo();
-    AccountInfo acc1 = new AccountInfo();
-    acc1.name = "acc1";
-    AccountInfo acc2 = new AccountInfo();
-    acc2.name = "acc2";
-
-    oldChangeInfo.removableLabels =
-        ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1)));
-    newChangeInfo.removableLabels =
-        ImmutableMap.of("Verified", ImmutableMap.of("-1", ImmutableList.of(acc2)));
-
-    ChangeInfoDifference diff = ChangeInfoDiffer.getDifference(oldChangeInfo, newChangeInfo);
-
-    assertThat(diff.added().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Verified", ImmutableMap.of("-1", ImmutableList.of(acc2))));
-    assertThat(diff.removed().removableLabels)
-        .containsExactlyEntriesIn(
-            ImmutableMap.of("Code-Review", ImmutableMap.of("+1", ImmutableList.of(acc1))));
-  }
-
-  @Test
   public void getDiff_assertCanConstructAllChangeInfoReferences() throws Exception {
     buildObjectWithFullFields(ChangeInfo.class);
   }