Merge "Detect and drop duplicate entries in the Set Access REST endpoint"
diff --git a/java/com/google/gerrit/entities/Permission.java b/java/com/google/gerrit/entities/Permission.java
index e3e3a57..6d2fa32 100644
--- a/java/com/google/gerrit/entities/Permission.java
+++ b/java/com/google/gerrit/entities/Permission.java
@@ -278,7 +278,10 @@
public Permission build() {
setRules(
- rulesBuilders.stream().map(PermissionRule.Builder::build).collect(toImmutableList()));
+ rulesBuilders.stream()
+ .map(PermissionRule.Builder::build)
+ .distinct()
+ .collect(toImmutableList()));
return autoBuild();
}
diff --git a/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
index 7c33ec2..260babd 100644
--- a/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/project/AccessIT.java
@@ -67,6 +67,7 @@
import com.google.gerrit.server.project.ProjectConfig;
import com.google.gerrit.server.schema.GrantRevertPermission;
import com.google.inject.Inject;
+import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -263,6 +264,32 @@
}
@Test
+ public void addDuplicatedAccessSection_doesNotAddDuplicateEntry() throws Exception {
+ ProjectAccessInput accessInput = newProjectAccessInput();
+ AccessSectionInfo accessSectionInfo = createDefaultAccessSectionInfo();
+
+ accessInput.add.put(REFS_HEADS, accessSectionInfo);
+ pApi().access(accessInput);
+ List<String> projectConfigLines =
+ Arrays.asList(projectOperations.project(newProjectName).getConfig().toText().split("\n"));
+ assertThat(projectConfigLines)
+ .containsExactly(
+ "[submit]",
+ "\taction = inherit",
+ "[access \"refs/heads/*\"]",
+ "\tlabel-Code-Review = deny group Registered Users",
+ "\tlabel-Code-Review = -1..+1 group Project Owners",
+ "\tpush = group Registered Users");
+
+ // Apply the same permission once more
+ pApi().access(accessInput);
+
+ List<String> newProjectConfigLines =
+ Arrays.asList(projectOperations.project(newProjectName).getConfig().toText().split("\n"));
+ assertThat(projectConfigLines).isEqualTo(newProjectConfigLines);
+ }
+
+ @Test
public void addAccessSectionForPluginPermission() throws Exception {
try (Registration registration =
extensionRegistry
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
index fed22f2..c1a7627 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/project/ProjectOperationsImplTest.java
@@ -329,7 +329,7 @@
}
@Test
- public void addDuplicatePermissions() throws Exception {
+ public void addDuplicatePermissions_isIgnored() throws Exception {
TestPermission permission =
TestProjectUpdate.allow(Permission.ABANDON).ref("refs/foo").group(REGISTERED_USERS).build();
Project.NameKey key = projectOperations.newProject().create();
@@ -340,9 +340,8 @@
assertThat(config).subsections("access").containsExactly("refs/foo");
assertThat(config)
.subsectionValues("access", "refs/foo")
- .containsExactly(
- "abandon", "group global:Registered-Users",
- "abandon", "group global:Registered-Users");
+ // Duplicated permission was recorded only once
+ .containsExactly("abandon", "group global:Registered-Users");
projectOperations.project(key).forUpdate().add(permission).update();
config = projectOperations.project(key).getConfig();
@@ -350,10 +349,8 @@
assertThat(config).subsections("access").containsExactly("refs/foo");
assertThat(config)
.subsectionValues("access", "refs/foo")
- .containsExactly(
- "abandon", "group global:Registered-Users",
- "abandon", "group global:Registered-Users",
- "abandon", "group global:Registered-Users");
+ // Duplicated permission in request was dropped
+ .containsExactly("abandon", "group global:Registered-Users");
}
@Test
