opendev/gerrit
Code Issues Proposed changes

Use "Project.NameKey" instead of "NameKey"

Browse Source 
A nested class can be static imported when the class does not
require additional context. Apparently, "NameKey" doesn't meet this
requirement because there are several nested classes named like
this, e.g. Project.NameKey, AccountGroup.NameKey, Branch.NameKey.
Therefore using "NameKey" without its outer class makes the code
hard to read.

Change-Id: Ic13b6e3da38222314c31258925b0b39b4cced4ef
This commit is contained in:
Changcheng Xiao 2018-10-24 09:15:11 +02:00
parent d80ab93c98
commit 9fdbe31e05
5 changed files with 37 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java
View File

@ -17,7 +17,7 @@ package com.google.gerrit.server.git;
import static com.google.common.base.Preconditions.checkState;
import com.google.gerrit.lifecycle.LifecycleModule;
import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.config.RepositoryConfig;
import com.google.gerrit.server.config.SitePaths;
@ -54,7 +54,7 @@ public class MultiBaseLocalDiskRepositoryManager extends LocalDiskRepositoryMana
}
@Override
public Path getBasePath(NameKey name) {
public Path getBasePath(Project.NameKey name) {
Path alternateBasePath = config.getBasePath(name);
return alternateBasePath != null ? alternateBasePath : super.getBasePath(name);
}

3
java/com/google/gerrit/server/project/SectionMatcher.java
View File

@ -16,7 +16,6 @@ package com.google.gerrit.server.project;
import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.server.CurrentUser;
/**
@ -57,7 +56,7 @@ public class SectionMatcher extends RefPatternMatcher {
return matcher;
}
public NameKey getProject() {
public Project.NameKey getProject() {
return project;
}
}

7
java/com/google/gerrit/server/schema/Schema_108.java
View File

@ -24,7 +24,6 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Change.Status;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.GitRepositoryManager;
@ -145,8 +144,8 @@ public class Schema_108 extends SchemaVersion {
private SetMultimap<Project.NameKey, Change.Id> getOpenChangesByProject(ReviewDb db, UpdateUI ui)
throws OrmException {
SortedSet<NameKey> projects = repoManager.list();
SortedSet<NameKey> nonExistentProjects = Sets.newTreeSet();
SortedSet<Project.NameKey> projects = repoManager.list();
SortedSet<Project.NameKey> nonExistentProjects = Sets.newTreeSet();
SetMultimap<Project.NameKey, Change.Id> openByProject =
MultimapBuilder.hashKeys().hashSetValues().build();
for (Change c : db.changes().all()) {
@ -155,7 +154,7 @@ public class Schema_108 extends SchemaVersion {
continue;
}
NameKey projectKey = c.getProject();
Project.NameKey projectKey = c.getProject();
if (!projects.contains(projectKey)) {
nonExistentProjects.add(projectKey);
} else {

56
javatests/com/google/gerrit/server/config/RepositoryConfigTest.java
View File

@ -19,7 +19,7 @@ import static com.google.common.truth.Truth8.assertThat;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.extensions.client.SubmitType;
import com.google.gerrit.reviewdb.client.Project.NameKey;
import com.google.gerrit.reviewdb.client.Project;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
@ -41,35 +41,35 @@ public class RepositoryConfigTest {
@Test
public void defaultSubmitTypeWhenNotConfigured() {
// Check expected value explicitly rather than depending on constant.
assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.INHERIT);
}
@Test
public void defaultSubmitTypeForStarFilter() {
configureDefaultSubmitType("*", SubmitType.CHERRY_PICK);
assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.CHERRY_PICK);
configureDefaultSubmitType("*", SubmitType.FAST_FORWARD_ONLY);
assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.FAST_FORWARD_ONLY);
configureDefaultSubmitType("*", SubmitType.REBASE_IF_NECESSARY);
assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.REBASE_IF_NECESSARY);
configureDefaultSubmitType("*", SubmitType.REBASE_ALWAYS);
assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.REBASE_ALWAYS);
}
@Test
public void defaultSubmitTypeForSpecificFilter() {
configureDefaultSubmitType("someProject", SubmitType.CHERRY_PICK);
assertThat(repoCfg.getDefaultSubmitType(new NameKey("someOtherProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someOtherProject")))
.isEqualTo(RepositoryConfig.DEFAULT_SUBMIT_TYPE);
assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.CHERRY_PICK);
}
@ -79,13 +79,13 @@ public class RepositoryConfigTest {
configureDefaultSubmitType("somePath/*", SubmitType.CHERRY_PICK);
configureDefaultSubmitType("*", SubmitType.MERGE_ALWAYS);
assertThat(repoCfg.getDefaultSubmitType(new NameKey("someProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("someProject")))
.isEqualTo(SubmitType.MERGE_ALWAYS);
assertThat(repoCfg.getDefaultSubmitType(new NameKey("somePath/someProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("somePath/someProject")))
.isEqualTo(SubmitType.CHERRY_PICK);
assertThat(repoCfg.getDefaultSubmitType(new NameKey("somePath/somePath/someProject")))
assertThat(repoCfg.getDefaultSubmitType(new Project.NameKey("somePath/somePath/someProject")))
.isEqualTo(SubmitType.REBASE_IF_NECESSARY);
}
@ -99,14 +99,14 @@ public class RepositoryConfigTest {
@Test
public void ownerGroupsWhenNotConfigured() {
assertThat(repoCfg.getOwnerGroups(new NameKey("someProject"))).isEmpty();
assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someProject"))).isEmpty();
}
@Test
public void ownerGroupsForStarFilter() {
ImmutableList<String> ownerGroups = ImmutableList.of("group1", "group2");
configureOwnerGroups("*", ownerGroups);
assertThat(repoCfg.getOwnerGroups(new NameKey("someProject")))
assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someProject")))
.containsExactlyElementsIn(ownerGroups);
}
@ -114,8 +114,8 @@ public class RepositoryConfigTest {
public void ownerGroupsForSpecificFilter() {
ImmutableList<String> ownerGroups = ImmutableList.of("group1", "group2");
configureOwnerGroups("someProject", ownerGroups);
assertThat(repoCfg.getOwnerGroups(new NameKey("someOtherProject"))).isEmpty();
assertThat(repoCfg.getOwnerGroups(new NameKey("someProject")))
assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someOtherProject"))).isEmpty();
assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someProject")))
.containsExactlyElementsIn(ownerGroups);
}
@ -129,13 +129,13 @@ public class RepositoryConfigTest {
configureOwnerGroups("somePath/*", ownerGroups2);
configureOwnerGroups("somePath/somePath/*", ownerGroups3);
assertThat(repoCfg.getOwnerGroups(new NameKey("someProject")))
assertThat(repoCfg.getOwnerGroups(new Project.NameKey("someProject")))
.containsExactlyElementsIn(ownerGroups1);
assertThat(repoCfg.getOwnerGroups(new NameKey("somePath/someProject")))
assertThat(repoCfg.getOwnerGroups(new Project.NameKey("somePath/someProject")))
.containsExactlyElementsIn(ownerGroups2);
assertThat(repoCfg.getOwnerGroups(new NameKey("somePath/somePath/someProject")))
assertThat(repoCfg.getOwnerGroups(new Project.NameKey("somePath/somePath/someProject")))
.containsExactlyElementsIn(ownerGroups3);
}
@ -149,22 +149,24 @@ public class RepositoryConfigTest {
@Test
public void basePathWhenNotConfigured() {
assertThat(repoCfg.getBasePath(new NameKey("someProject"))).isNull();
assertThat(repoCfg.getBasePath(new Project.NameKey("someProject"))).isNull();
}
@Test
public void basePathForStarFilter() {
String basePath = "/someAbsolutePath/someDirectory";
configureBasePath("*", basePath);
assertThat(repoCfg.getBasePath(new NameKey("someProject")).toString()).isEqualTo(basePath);
assertThat(repoCfg.getBasePath(new Project.NameKey("someProject")).toString())
.isEqualTo(basePath);
}
@Test
public void basePathForSpecificFilter() {
String basePath = "/someAbsolutePath/someDirectory";
configureBasePath("someProject", basePath);
assertThat(repoCfg.getBasePath(new NameKey("someOtherProject"))).isNull();
assertThat(repoCfg.getBasePath(new NameKey("someProject")).toString()).isEqualTo(basePath);
assertThat(repoCfg.getBasePath(new Project.NameKey("someOtherProject"))).isNull();
assertThat(repoCfg.getBasePath(new Project.NameKey("someProject")).toString())
.isEqualTo(basePath);
}
@Test
@ -179,12 +181,14 @@ public class RepositoryConfigTest {
configureBasePath("project/*", basePath3);
configureBasePath("*", basePath4);
assertThat(repoCfg.getBasePath(new NameKey("project1")).toString()).isEqualTo(basePath1);
assertThat(repoCfg.getBasePath(new NameKey("project/project/someProject")).toString())
assertThat(repoCfg.getBasePath(new Project.NameKey("project1")).toString())
.isEqualTo(basePath1);
assertThat(repoCfg.getBasePath(new Project.NameKey("project/project/someProject")).toString())
.isEqualTo(basePath2);
assertThat(repoCfg.getBasePath(new NameKey("project/someProject")).toString())
assertThat(repoCfg.getBasePath(new Project.NameKey("project/someProject")).toString())
.isEqualTo(basePath3);
assertThat(repoCfg.getBasePath(new NameKey("someProject")).toString()).isEqualTo(basePath4);
assertThat(repoCfg.getBasePath(new Project.NameKey("someProject")).toString())
.isEqualTo(basePath4);
}
@Test

2
plugins/replication

@ -1 +1 @@
Subproject commit 092792edacf9c29732a560a30967b92664cd65f9
Subproject commit a7b900bd524c333c8ca2825e37fa781ac055ac36