From 6e69d8a410ea8b532cfa9f4b985bf011a177cb92 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Mon, 3 Dec 2018 15:23:42 -0800
Subject: [PATCH] AccessSection/Permission: Change return types to
 ImmutableList

These were recently improved in I9a58b2e8 and Ic5514036 to return
immutable copies. Removing them causes ErrorProne failures in the
@Ignored tests, which intentionally try to modify these ImmutableLists.
The ErrorProne failure is testing the same thing as the tests
themselves, and much less verbosely; just remove the tests.

Change-Id: I1492113696166512056a2ce08ce2bff2d254c523
---
 .../gerrit/common/data/AccessSection.java      |  2 +-
 .../google/gerrit/common/data/Permission.java  |  2 +-
 .../gerrit/common/data/AccessSectionTest.java  | 17 -----------------
 .../gerrit/common/data/PermissionTest.java     | 18 ------------------
 4 files changed, 2 insertions(+), 37 deletions(-)

diff --git a/java/com/google/gerrit/common/data/AccessSection.java b/java/com/google/gerrit/common/data/AccessSection.java
index c8d8d413ad..b3da199ec1 100644
--- a/java/com/google/gerrit/common/data/AccessSection.java
+++ b/java/com/google/gerrit/common/data/AccessSection.java
@@ -37,7 +37,7 @@ public class AccessSection extends RefConfigSection implements Comparable<Access
     super(refPattern);
   }
 
-  public List<Permission> getPermissions() {
+  public ImmutableList<Permission> getPermissions() {
     return permissions == null ? ImmutableList.of() : ImmutableList.copyOf(permissions);
   }
 
diff --git a/java/com/google/gerrit/common/data/Permission.java b/java/com/google/gerrit/common/data/Permission.java
index a30d412970..2e9c2d6669 100644
--- a/java/com/google/gerrit/common/data/Permission.java
+++ b/java/com/google/gerrit/common/data/Permission.java
@@ -158,7 +158,7 @@ public class Permission implements Comparable<Permission> {
     exclusiveGroup = newExclusiveGroup;
   }
 
-  public List<PermissionRule> getRules() {
+  public ImmutableList<PermissionRule> getRules() {
     return rules == null ? ImmutableList.of() : ImmutableList.copyOf(rules);
   }
 
diff --git a/javatests/com/google/gerrit/common/data/AccessSectionTest.java b/javatests/com/google/gerrit/common/data/AccessSectionTest.java
index 68f192cf11..faf9d6c752 100644
--- a/javatests/com/google/gerrit/common/data/AccessSectionTest.java
+++ b/javatests/com/google/gerrit/common/data/AccessSectionTest.java
@@ -22,7 +22,6 @@ import java.util.ArrayList;
 import java.util.List;
 import java.util.Locale;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class AccessSectionTest extends GerritBaseTests {
@@ -152,22 +151,6 @@ public class AccessSectionTest extends GerritBaseTests {
     assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
   }
 
-  @Ignore
-  @Test
-  public void cannotAddPermissionByModifyingListThatWasRetrievedFromAccessSection() {
-    Permission submitPermission = new Permission(Permission.SUBMIT);
-    accessSection.getPermissions().add(submitPermission);
-    assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
-
-    List<Permission> permissions = new ArrayList<>();
-    permissions.add(new Permission(Permission.ABANDON));
-    permissions.add(new Permission(Permission.REBASE));
-    accessSection.setPermissions(permissions);
-    assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
-    accessSection.getPermissions().add(submitPermission);
-    assertThat(accessSection.getPermission(Permission.SUBMIT)).isNull();
-  }
-
   @Test
   public void removePermission() {
     Permission abandonPermission = new Permission(Permission.ABANDON);
diff --git a/javatests/com/google/gerrit/common/data/PermissionTest.java b/javatests/com/google/gerrit/common/data/PermissionTest.java
index b5d2587d0f..23380e7f79 100644
--- a/javatests/com/google/gerrit/common/data/PermissionTest.java
+++ b/javatests/com/google/gerrit/common/data/PermissionTest.java
@@ -22,7 +22,6 @@ import com.google.gerrit.testing.GerritBaseTests;
 import java.util.ArrayList;
 import java.util.List;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class PermissionTest extends GerritBaseTests {
@@ -187,23 +186,6 @@ public class PermissionTest extends GerritBaseTests {
     assertThat(permission.getRule(groupReference3)).isNull();
   }
 
-  @Ignore
-  @Test
-  public void cannotAddPermissionByModifyingListThatWasRetrievedFromAccessSection() {
-    GroupReference groupReference1 = new GroupReference(new AccountGroup.UUID("uuid-1"), "group1");
-    PermissionRule permissionRule1 = new PermissionRule(groupReference1);
-    permission.getRules().add(permissionRule1);
-    assertThat(permission.getRule(groupReference1)).isNull();
-
-    List<PermissionRule> rules = new ArrayList<>();
-    rules.add(new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-2"), "group2")));
-    rules.add(new PermissionRule(new GroupReference(new AccountGroup.UUID("uuid-3"), "group3")));
-    permission.setRules(rules);
-    assertThat(permission.getRule(groupReference1)).isNull();
-    permission.getRules().add(permissionRule1);
-    assertThat(permission.getRule(groupReference1)).isNull();
-  }
-
   @Test
   public void getNonExistingRule() {
     GroupReference groupReference = new GroupReference(new AccountGroup.UUID("uuid-1"), "group1");