From 9fdbe31e05f4b3c8721b8f1f5373fb91e31d7043 Mon Sep 17 00:00:00 2001 From: Changcheng Xiao Date: Wed, 24 Oct 2018 09:15:11 +0200 Subject: [PATCH] Use "Project.NameKey" instead of "NameKey" 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 --- .../MultiBaseLocalDiskRepositoryManager.java | 4 +- .../gerrit/server/project/SectionMatcher.java | 3 +- .../gerrit/server/schema/Schema_108.java | 7 +-- .../server/config/RepositoryConfigTest.java | 56 ++++++++++--------- plugins/replication | 2 +- 5 files changed, 37 insertions(+), 35 deletions(-) diff --git a/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java b/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java index 6fafe4e0cc..01e85cf332 100644 --- a/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java +++ b/java/com/google/gerrit/server/git/MultiBaseLocalDiskRepositoryManager.java @@ -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); } diff --git a/java/com/google/gerrit/server/project/SectionMatcher.java b/java/com/google/gerrit/server/project/SectionMatcher.java index 11b1f3709b..3095f2f2a6 100644 --- a/java/com/google/gerrit/server/project/SectionMatcher.java +++ b/java/com/google/gerrit/server/project/SectionMatcher.java @@ -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; } } diff --git a/java/com/google/gerrit/server/schema/Schema_108.java b/java/com/google/gerrit/server/schema/Schema_108.java index 4e62460323..b37fab3b9e 100644 --- a/java/com/google/gerrit/server/schema/Schema_108.java +++ b/java/com/google/gerrit/server/schema/Schema_108.java @@ -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 getOpenChangesByProject(ReviewDb db, UpdateUI ui) throws OrmException { - SortedSet projects = repoManager.list(); - SortedSet nonExistentProjects = Sets.newTreeSet(); + SortedSet projects = repoManager.list(); + SortedSet nonExistentProjects = Sets.newTreeSet(); SetMultimap 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 { diff --git a/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java b/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java index 2edcf7c523..3da35b2fc7 100644 --- a/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java +++ b/javatests/com/google/gerrit/server/config/RepositoryConfigTest.java @@ -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 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 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 diff --git a/plugins/replication b/plugins/replication index 092792edac..a7b900bd52 160000 --- a/plugins/replication +++ b/plugins/replication @@ -1 +1 @@ -Subproject commit 092792edacf9c29732a560a30967b92664cd65f9 +Subproject commit a7b900bd524c333c8ca2825e37fa781ac055ac36