Support configuring code owners backends per branch
We need to be able to migrate code owner config files to a new syntax
branch by branch. Hence it must be possible to configure the code owners
backend per branch.
A branch level code owners backend configuration overrides a project
level code owners backend configuration.
For the branch level code owners backend configuration we support both,
short and full branch names. If both are specified the configuration for
the full branch name takes precedence.
Change-Id: I780d96851356309a7056bf3154d24c77498cddfb
Signed-off-by: Edwin Kempin <ekempin@google.com>
diff --git a/java/com/google/gerrit/plugins/codeowners/CodeOwnersPluginConfiguration.java b/java/com/google/gerrit/plugins/codeowners/CodeOwnersPluginConfiguration.java
index c6cba18..04efd07 100644
--- a/java/com/google/gerrit/plugins/codeowners/CodeOwnersPluginConfiguration.java
+++ b/java/com/google/gerrit/plugins/codeowners/CodeOwnersPluginConfiguration.java
@@ -15,6 +15,7 @@
package com.google.gerrit.plugins.codeowners;
import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.registration.DynamicMap;
@@ -69,26 +70,86 @@
/**
* Returns the configured {@link CodeOwnersBackend}.
*
- * <p>Callers must ensure that the specified project exists. If the specified project doesn't
+ * <p>Callers must ensure that the project of the specified branch exists. If the project doesn't
* exist the call fails with {@link IllegalStateException}.
*
- * @param project project for which the configured code owners backend should be returned
+ * <p>The code owners backend configuration is evaluated in the following order:
+ *
+ * <ul>
+ * <li>backend configuration for branch (with inheritance, first by full branch name, then by
+ * short branch name)
+ * <li>backend configuration for project (with inheritance)
+ * <li>default backend (first globally configured backend, then hard-coded default backend)
+ * </ul>
+ *
+ * <p>The first code owners backend configuration that exists counts and the evaluation is
+ * stopped.
+ *
+ * @param branchNameKey project and branch for which the configured code owners backend should be
+ * returned
* @return the {@link CodeOwnersBackend} that should be used
*/
- public CodeOwnersBackend getBackend(Project.NameKey project) {
- Config pluginConfig = getPluginConfig(project);
+ public CodeOwnersBackend getBackend(BranchNameKey branchNameKey) {
+ Config pluginConfig = getPluginConfig(branchNameKey.project());
+
+ // check if a branch specific backend is configured
+ Optional<CodeOwnersBackend> codeOwnersBackend =
+ getBackendForBranch(pluginConfig, branchNameKey);
+ if (codeOwnersBackend.isPresent()) {
+ return codeOwnersBackend.get();
+ }
+
+ // check if a project specific backend is configured
+ codeOwnersBackend = getBackendForProject(pluginConfig);
+ if (codeOwnersBackend.isPresent()) {
+ return codeOwnersBackend.get();
+ }
+
+ // fall back to the default backend
+ return getDefaultBackend();
+ }
+
+ private Optional<CodeOwnersBackend> getBackendForBranch(
+ Config pluginConfig, BranchNameKey branch) {
+ // check for branch specific backend by full branch name
+ Optional<CodeOwnersBackend> backend = getBackendForBranch(pluginConfig, branch.branch());
+ if (!backend.isPresent()) {
+ // check for branch specific backend by short branch name
+ backend = getBackendForBranch(pluginConfig, branch.shortName());
+ }
+ return backend;
+ }
+
+ private Optional<CodeOwnersBackend> getBackendForBranch(Config pluginConfig, String branch) {
+ String backendName = pluginConfig.getString(SECTION_CODE_OWNERS, branch, KEY_BACKEND);
+ if (backendName == null) {
+ return Optional.empty();
+ }
+ return Optional.of(
+ lookupBackend(backendName)
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "Code owner backend '%s' that is configured in %s.config"
+ + " (parameter %s.%s.%s) not found",
+ backendName, pluginName, SECTION_CODE_OWNERS, branch, KEY_BACKEND))));
+ }
+
+ private Optional<CodeOwnersBackend> getBackendForProject(Config pluginConfig) {
String backendName = pluginConfig.getString(SECTION_CODE_OWNERS, null, KEY_BACKEND);
if (backendName == null) {
- return getDefaultBackend();
+ return Optional.empty();
}
- return lookupBackend(backendName)
- .orElseThrow(
- () ->
- new IllegalStateException(
- String.format(
- "Code owner backend '%s' that is configured in %s.config"
- + " (parameter %s.%s) not found",
- backendName, pluginName, SECTION_CODE_OWNERS, KEY_BACKEND)));
+ return Optional.of(
+ lookupBackend(backendName)
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "Code owner backend '%s' that is configured in %s.config"
+ + " (parameter %s.%s) not found",
+ backendName, pluginName, SECTION_CODE_OWNERS, KEY_BACKEND))));
}
private CodeOwnersBackend getDefaultBackend() {
diff --git a/javatests/com/google/gerrit/plugins/codeowners/CodeOwnersPluginConfigurationTest.java b/javatests/com/google/gerrit/plugins/codeowners/CodeOwnersPluginConfigurationTest.java
index c025076..73307d4 100644
--- a/javatests/com/google/gerrit/plugins/codeowners/CodeOwnersPluginConfigurationTest.java
+++ b/javatests/com/google/gerrit/plugins/codeowners/CodeOwnersPluginConfigurationTest.java
@@ -20,6 +20,7 @@
import com.google.gerrit.acceptance.config.GerritConfig;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.registration.DynamicMap;
@@ -64,7 +65,8 @@
assertThrows(
IllegalStateException.class,
() ->
- codeOwnersPluginConfiguration.getBackend(Project.nameKey("non-existing-project")));
+ codeOwnersPluginConfiguration.getBackend(
+ BranchNameKey.create(Project.nameKey("non-existing-project"), "master")));
assertThat(exception)
.hasMessageThat()
.isEqualTo(
@@ -72,8 +74,15 @@
}
@Test
+ public void getBackendForNonExistingBranch() throws Exception {
+ assertThat(
+ codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "non-existing")))
+ .isInstanceOf(FindOwnersBackend.class);
+ }
+
+ @Test
public void getDefaultBackendWhenNoBackendIsConfigured() throws Exception {
- assertThat(codeOwnersPluginConfiguration.getBackend(project))
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.isInstanceOf(FindOwnersBackend.class);
}
@@ -81,7 +90,7 @@
@GerritConfig(name = "plugin.code-owners.backend", value = TestCodeOwnersBackend.ID)
public void getConfiguredDefaultBackend() throws Exception {
try (AutoCloseable registration = registerTestBackend()) {
- assertThat(codeOwnersPluginConfiguration.getBackend(project))
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.isInstanceOf(TestCodeOwnersBackend.class);
}
}
@@ -91,7 +100,9 @@
public void cannotGetBackendIfNonExistingBackendIsConfigured() throws Exception {
IllegalStateException exception =
assertThrows(
- IllegalStateException.class, () -> codeOwnersPluginConfiguration.getBackend(project));
+ IllegalStateException.class,
+ () ->
+ codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")));
assertThat(exception)
.hasMessageThat()
.isEqualTo(
@@ -103,7 +114,7 @@
public void getBackendConfiguredOnProjectLevel() throws Exception {
configureBackend(project, TestCodeOwnersBackend.ID);
try (AutoCloseable registration = registerTestBackend()) {
- assertThat(codeOwnersPluginConfiguration.getBackend(project))
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.isInstanceOf(TestCodeOwnersBackend.class);
}
}
@@ -113,7 +124,7 @@
public void backendConfiguredOnProjectLevelOverridesDefaultBackend() throws Exception {
configureBackend(project, TestCodeOwnersBackend.ID);
try (AutoCloseable registration = registerTestBackend()) {
- assertThat(codeOwnersPluginConfiguration.getBackend(project))
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.isInstanceOf(TestCodeOwnersBackend.class);
}
}
@@ -122,7 +133,7 @@
public void backendIsInheritedFromParentProject() throws Exception {
configureBackend(allProjects, TestCodeOwnersBackend.ID);
try (AutoCloseable registration = registerTestBackend()) {
- assertThat(codeOwnersPluginConfiguration.getBackend(project))
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.isInstanceOf(TestCodeOwnersBackend.class);
}
}
@@ -132,7 +143,7 @@
public void inheritedBackendOverridesDefaultBackend() throws Exception {
configureBackend(allProjects, TestCodeOwnersBackend.ID);
try (AutoCloseable registration = registerTestBackend()) {
- assertThat(codeOwnersPluginConfiguration.getBackend(project))
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.isInstanceOf(TestCodeOwnersBackend.class);
}
}
@@ -142,7 +153,7 @@
configureBackend(allProjects, TestCodeOwnersBackend.ID);
configureBackend(project, FindOwnersBackend.ID);
try (AutoCloseable registration = registerTestBackend()) {
- assertThat(codeOwnersPluginConfiguration.getBackend(project))
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.isInstanceOf(FindOwnersBackend.class);
}
}
@@ -152,7 +163,9 @@
configureBackend(project, "non-existing-backend");
IllegalStateException exception =
assertThrows(
- IllegalStateException.class, () -> codeOwnersPluginConfiguration.getBackend(project));
+ IllegalStateException.class,
+ () ->
+ codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")));
assertThat(exception)
.hasMessageThat()
.isEqualTo(
@@ -165,16 +178,84 @@
Project.NameKey otherProject = projectOperations.newProject().create();
configureBackend(otherProject, TestCodeOwnersBackend.ID);
try (AutoCloseable registration = registerTestBackend()) {
- assertThat(codeOwnersPluginConfiguration.getBackend(project))
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
+ .isInstanceOf(FindOwnersBackend.class);
+ }
+ }
+
+ @Test
+ public void getBackendConfiguredOnBranchLevel() throws Exception {
+ configureBackend(project, "refs/heads/master", TestCodeOwnersBackend.ID);
+ try (AutoCloseable registration = registerTestBackend()) {
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
+ .isInstanceOf(TestCodeOwnersBackend.class);
+ }
+ }
+
+ @Test
+ public void getBackendConfiguredOnBranchLevelShortName() throws Exception {
+ configureBackend(project, "master", TestCodeOwnersBackend.ID);
+ try (AutoCloseable registration = registerTestBackend()) {
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
+ .isInstanceOf(TestCodeOwnersBackend.class);
+ }
+ }
+
+ @Test
+ public void branchLevelBackendOnFullNameTakesPrecedenceOverBranchLevelBackendOnShortName()
+ throws Exception {
+ configureBackend(project, "master", TestCodeOwnersBackend.ID);
+ configureBackend(project, "refs/heads/master", FindOwnersBackend.ID);
+ try (AutoCloseable registration = registerTestBackend()) {
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
+ .isInstanceOf(FindOwnersBackend.class);
+ }
+ }
+
+ @Test
+ public void branchLevelBackendOverridesProjectLevelBackend() throws Exception {
+ configureBackend(project, TestCodeOwnersBackend.ID);
+ configureBackend(project, "master", FindOwnersBackend.ID);
+ try (AutoCloseable registration = registerTestBackend()) {
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
+ .isInstanceOf(FindOwnersBackend.class);
+ }
+ }
+
+ @Test
+ public void cannotGetBackendIfNonExistingBackendIsConfiguredOnBranchLevel() throws Exception {
+ configureBackend(project, "master", "non-existing-backend");
+ IllegalStateException exception =
+ assertThrows(
+ IllegalStateException.class,
+ () ->
+ codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")));
+ assertThat(exception)
+ .hasMessageThat()
+ .isEqualTo(
+ "Code owner backend 'non-existing-backend' that is configured in code-owners.config"
+ + " (parameter codeOwners.master.backend) not found");
+ }
+
+ @Test
+ public void branchLevelBackendForOtherBranchHasNoEffect() throws Exception {
+ configureBackend(project, "foo", TestCodeOwnersBackend.ID);
+ try (AutoCloseable registration = registerTestBackend()) {
+ assertThat(codeOwnersPluginConfiguration.getBackend(BranchNameKey.create(project, "master")))
.isInstanceOf(FindOwnersBackend.class);
}
}
private void configureBackend(Project.NameKey project, String backendName) throws Exception {
+ configureBackend(project, null, backendName);
+ }
+
+ private void configureBackend(
+ Project.NameKey project, @Nullable String branch, String backendName) throws Exception {
Config codeOwnersConfig = new Config();
codeOwnersConfig.setString(
CodeOwnersPluginConfiguration.SECTION_CODE_OWNERS,
- null,
+ branch,
CodeOwnersPluginConfiguration.KEY_BACKEND,
backendName);
try (TestRepository<Repository> testRepo =
diff --git a/resources/Documentation/config.md b/resources/Documentation/config.md
index 069e37c..c82b1fb 100644
--- a/resources/Documentation/config.md
+++ b/resources/Documentation/config.md
@@ -38,5 +38,20 @@
[plugin.@PLUGIN@.backend](#pluginCodeOwnersBackend) in `gerrit.config`
is used.
+<a id="codeOwners.branch.backend">codeOwners.<branch>.backend</a>
+: The code owners backend that should be used for this branch.
+
+ The branch can be the short or full name. If both configurations exist
+ the one for the full name takes precedence.
+
+ Overrides the per repository setting
+ [codeOwners.backend](#codeOwnersBackend).
+
+ The supported code owner backends are listed at the
+ [Backends](../../../Documentation/backends.html) page.
+
+ If not set, the project level configuration
+ [codeOwners.backend](#codeOwnersBackend) is used.
+
Part of [Gerrit Code Review](../../../Documentation/index.html)
