opendev/gerrit
Code Issues Proposed changes

Merge "Refactor array usage to list for RepositoryConfig and GroupSetProvider"

Browse Source
This commit is contained in:
David Pursehouse
2016-04-08 14:15:15 +00:00
committed by Gerrit Code Review
parent 580172d6d6 823a4c25d4
commit 7538a345e1
5 changed files with 37 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
gerrit-server/src/main/java/com/google/gerrit/server/config/GitReceivePackGroupsProvider.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.config; package com.google.gerrit.server.config;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.util.ServerRequestContext; import com.google.gerrit.server.util.ServerRequestContext;
@@ -30,8 +31,8 @@ public class GitReceivePackGroupsProvider extends GroupSetProvider {
@GerritServerConfig Config config, @GerritServerConfig Config config,
ThreadLocalRequestContext threadContext, ThreadLocalRequestContext threadContext,
ServerRequestContext serverCtx) { ServerRequestContext serverCtx) {
super(gb, threadContext, serverCtx, config.getStringList("receive", null, super(gb, threadContext, serverCtx, ImmutableList.copyOf(
"allowGroup")); config.getStringList("receive", null, "allowGroup")));
// If no group was set, default to "registered users" // If no group was set, default to "registered users"
// //

5
gerrit-server/src/main/java/com/google/gerrit/server/config/GitUploadPackGroupsProvider.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.config; package com.google.gerrit.server.config;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet;
import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.group.SystemGroupBackend;
@@ -29,8 +30,8 @@ public class GitUploadPackGroupsProvider extends GroupSetProvider {
@GerritServerConfig Config config, @GerritServerConfig Config config,
ThreadLocalRequestContext threadContext, ThreadLocalRequestContext threadContext,
ServerRequestContext serverCtx) { ServerRequestContext serverCtx) {
super(gb, threadContext, serverCtx, config.getStringList("upload", null, super(gb, threadContext, serverCtx, ImmutableList.copyOf(
"allowGroup")); config.getStringList("upload", null, "allowGroup")));
// If no group was set, default to "registered users" and "anonymous" // If no group was set, default to "registered users" and "anonymous"
// //

3
gerrit-server/src/main/java/com/google/gerrit/server/config/GroupSetProvider.java
View File

@@ -28,6 +28,7 @@ import com.google.inject.Provider;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import java.util.List;
import java.util.Set; import java.util.Set;
public abstract class GroupSetProvider implements public abstract class GroupSetProvider implements
@@ -40,7 +41,7 @@ public abstract class GroupSetProvider implements
@Inject @Inject
protected GroupSetProvider(GroupBackend groupBackend, protected GroupSetProvider(GroupBackend groupBackend,
ThreadLocalRequestContext threadContext, ThreadLocalRequestContext threadContext,
ServerRequestContext serverCtx, String[] groupNames) { ServerRequestContext serverCtx, List<String> groupNames) {
RequestContext ctx = threadContext.setContext(serverCtx); RequestContext ctx = threadContext.setContext(serverCtx);
try { try {
ImmutableSet.Builder<AccountGroup.UUID> builder = ImmutableSet.builder(); ImmutableSet.Builder<AccountGroup.UUID> builder = ImmutableSet.builder();

7
gerrit-server/src/main/java/com/google/gerrit/server/config/RepositoryConfig.java
View File

@@ -14,6 +14,7 @@
package com.google.gerrit.server.config; package com.google.gerrit.server.config;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -46,9 +47,9 @@ public class RepositoryConfig {
DEFAULT_SUBMIT_TYPE_NAME, SubmitType.MERGE_IF_NECESSARY); DEFAULT_SUBMIT_TYPE_NAME, SubmitType.MERGE_IF_NECESSARY);
} }
public String[] getOwnerGroups(Project.NameKey project) { public List<String> getOwnerGroups(Project.NameKey project) {
return cfg.getStringList(SECTION_NAME, findSubSection(project.get()), return ImmutableList.copyOf(cfg.getStringList(SECTION_NAME,
OWNER_GROUP_NAME); findSubSection(project.get()), OWNER_GROUP_NAME));
} }
public Path getBasePath(Project.NameKey project) { public Path getBasePath(Project.NameKey project) {

51
gerrit-server/src/test/java/com/google/gerrit/server/config/RepositoryConfigTest.java
View File

@@ -16,7 +16,7 @@ package com.google.gerrit.server.config;
import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth.assertThat;
import com.google.common.collect.Lists; import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.reviewdb.client.Project.NameKey; import com.google.gerrit.reviewdb.client.Project.NameKey;
@@ -26,7 +26,6 @@ import org.junit.Test;
import java.nio.file.Path; import java.nio.file.Path;
import java.nio.file.Paths; import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List; import java.util.List;
public class RepositoryConfigTest { public class RepositoryConfigTest {
@@ -98,48 +97,46 @@ public class RepositoryConfigTest {
@Test @Test
public void testOwnerGroupsWhenNotConfigured() { public void testOwnerGroupsWhenNotConfigured() {
assertThat(repoCfg.getOwnerGroups(new NameKey("someProject"))).isEqualTo( assertThat(repoCfg.getOwnerGroups(new NameKey("someProject"))).isEmpty();
new String[] {});
} }
@Test @Test
public void testOwnerGroupsForStarFilter() { public void testOwnerGroupsForStarFilter() {
String[] ownerGroups = new String[] {"group1", "group2"}; ImmutableList<String> ownerGroups = ImmutableList.of("group1", "group2");
configureOwnerGroups("*", Lists.newArrayList(ownerGroups)); configureOwnerGroups("*", ownerGroups);
assertThat(repoCfg.getOwnerGroups(new NameKey("someProject"))).isEqualTo( assertThat(repoCfg.getOwnerGroups(new NameKey("someProject")))
ownerGroups); .containsExactlyElementsIn(ownerGroups);
} }
@Test @Test
public void testOwnerGroupsForSpecificFilter() { public void testOwnerGroupsForSpecificFilter() {
String[] ownerGroups = new String[] {"group1", "group2"}; ImmutableList<String> ownerGroups = ImmutableList.of("group1", "group2");
configureOwnerGroups("someProject", Lists.newArrayList(ownerGroups)); configureOwnerGroups("someProject", ownerGroups);
assertThat(repoCfg.getOwnerGroups(new NameKey("someOtherProject"))) assertThat(repoCfg.getOwnerGroups(new NameKey("someOtherProject")))
.isEqualTo(new String[] {}); .isEmpty();
assertThat(repoCfg.getOwnerGroups(new NameKey("someProject"))).isEqualTo( assertThat(repoCfg.getOwnerGroups(new NameKey("someProject")))
ownerGroups); .containsExactlyElementsIn(ownerGroups);
} }
@Test @Test
public void testOwnerGroupsForStartWithFilter() { public void testOwnerGroupsForStartWithFilter() {
String[] ownerGroups1 = new String[] {"group1"}; ImmutableList<String> ownerGroups1 = ImmutableList.of("group1");
String[] ownerGroups2 = new String[] {"group2"}; ImmutableList<String> ownerGroups2 = ImmutableList.of("group2");
String[] ownerGroups3 = new String[] {"group3"}; ImmutableList<String> ownerGroups3 = ImmutableList.of("group3");
configureOwnerGroups("*", Lists.newArrayList(ownerGroups1)); configureOwnerGroups("*", ownerGroups1);
configureOwnerGroups("somePath/*", Lists.newArrayList(ownerGroups2)); configureOwnerGroups("somePath/*", ownerGroups2);
configureOwnerGroups("somePath/somePath/*", configureOwnerGroups("somePath/somePath/*", ownerGroups3);
Lists.newArrayList(ownerGroups3));
assertThat(repoCfg.getOwnerGroups(new NameKey("someProject"))).isEqualTo( assertThat(repoCfg.getOwnerGroups(new NameKey("someProject")))
ownerGroups1); .containsExactlyElementsIn(ownerGroups1);
assertThat(repoCfg.getOwnerGroups(new NameKey("somePath/someProject"))) assertThat(repoCfg.getOwnerGroups(new NameKey("somePath/someProject")))
.isEqualTo(ownerGroups2); .containsExactlyElementsIn(ownerGroups2);
assertThat( assertThat(
repoCfg.getOwnerGroups(new NameKey("somePath/somePath/someProject"))) repoCfg.getOwnerGroups(new NameKey("somePath/somePath/someProject")))
.isEqualTo(ownerGroups3); .containsExactlyElementsIn(ownerGroups3);
} }
private void configureOwnerGroups(String projectFilter, private void configureOwnerGroups(String projectFilter,
@@ -196,8 +193,10 @@ public class RepositoryConfigTest {
@Test @Test
public void testAllBasePath() { public void testAllBasePath() {
List<Path> allBasePaths = Arrays.asList(Paths.get("/someBasePath1"), ImmutableList<Path> allBasePaths = ImmutableList.of(
Paths.get("/someBasePath2"), Paths.get("/someBasePath2")); Paths.get("/someBasePath1"),
Paths.get("/someBasePath2"),
Paths.get("/someBasePath2"));
configureBasePath("*", allBasePaths.get(0).toString()); configureBasePath("*", allBasePaths.get(0).toString());
configureBasePath("project/*", allBasePaths.get(1).toString()); configureBasePath("project/*", allBasePaths.get(1).toString());