Allow disabling the owners auto-assigned on a per-branch basis
Define a new global configuration file <plugin-name>.config which allows
to selectively disable the resolution and auto-assignment of owners on a
per-branch basis.
Also apply the branch normalization into a generic ref using the same
logic across owners and owners-autoassign.
Change-Id: If6047069032012b6411251b06e3aff14f2a52a29
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/AutoassignConfig.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/AutoassignConfig.java
index 48ec686..a50ac69 100644
--- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/AutoassignConfig.java
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/AutoassignConfig.java
@@ -47,4 +47,8 @@
.projectSpecificConfig(projectKey)
.getEnum(PROJECT_CONFIG_AUTOASSIGN_FIELD, ReviewerState.REVIEWER);
}
+
+ public boolean isBranchDisabled(String branch) {
+ return config.isBranchDisabled(branch);
+ }
}
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
index 70cc5a6..3f30dc2 100644
--- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
@@ -209,7 +209,12 @@
ChangeInfo change = cApi.get();
PatchList patchList = getPatchList(repository, event, change);
if (patchList != null) {
- PathOwners owners = new PathOwners(accounts, repository, Optional.of(change.branch), patchList);
+ PathOwners owners =
+ new PathOwners(
+ accounts,
+ repository,
+ cfg.isBranchDisabled(change.branch) ? Optional.empty() : Optional.of(change.branch),
+ patchList);
Set<Account.Id> allReviewers = Sets.newHashSet();
allReviewers.addAll(owners.get().values());
allReviewers.addAll(owners.getReviewers().values());
diff --git a/owners-autoassign/src/main/resources/Documentation/config.md b/owners-autoassign/src/main/resources/Documentation/config.md
index afb534c..fa8a30f 100644
--- a/owners-autoassign/src/main/resources/Documentation/config.md
+++ b/owners-autoassign/src/main/resources/Documentation/config.md
@@ -1,3 +1,19 @@
+## Global configuration
+
+The global plugin configuration is read from the `$GERRIT_SITE/etc/owners-autoassign.config`
+and is applied across all projects in Gerrit.
+
+owners.disable.branch
+: List of branches regex where the resolution and auto-assignment of owners is disabled.
+
+Example:
+
+```
+[owners "disable"]
+ branch = refs/meta/config
+ branch = refs/heads/sandboxes.*
+```
+
## Project configuration
The project configuration `autoAssignWip` controls the automatic
diff --git a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/AbstractAutoassignIT.java b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/AbstractAutoassignIT.java
index 4aa6ff4..3ca6f35 100644
--- a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/AbstractAutoassignIT.java
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/AbstractAutoassignIT.java
@@ -19,6 +19,8 @@
import static com.googlesource.gerrit.owners.common.AutoassignConfigModule.PROJECT_CONFIG_AUTOASSIGN_FIELD;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.config.GlobalPluginConfig;
import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.ChangeApi;
@@ -109,6 +111,18 @@
}
@Test
+ @UseLocalDisk // Required when using @GlobalPluginConfig
+ @GlobalPluginConfig(
+ pluginName = "owners-api",
+ name = "owners.disable.branch",
+ value = "refs/heads/master")
+ public void shouldNotAutoassignUserInPathWhenBranchIsDisabled() throws Exception {
+ addOwnersToRepo("", user.email(), NOT_INHERITED);
+
+ assertThat(getAutoassignedAccounts(change(createChange()).get())).isNull();
+ }
+
+ @Test
public void shouldNotReAutoassignUserInPath() throws Exception {
String ownerEmail = user.email();
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PluginSettings.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PluginSettings.java
index 02f98ad..2c8fc11 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PluginSettings.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PluginSettings.java
@@ -24,6 +24,7 @@
import com.google.inject.Inject;
import com.google.inject.Singleton;
import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Constants;
/** Global owners plugin's settings defined globally or on a per-project basis. */
@Singleton
@@ -53,6 +54,20 @@
}
/**
+ * Check if the branch or ref is enabled for processing.
+ *
+ * <p>NOTE: If the branch does not start with 'refs/heads' it will then normalized into a
+ * ref-name.
+ *
+ * @param branch or ref name
+ * @return true if the branch or ref is disabled for processing.
+ */
+ public boolean isBranchDisabled(String branch) {
+ String normalizedRef = normalizeRef(branch);
+ return disabledBranchesPatterns.stream().anyMatch(normalizedRef::matches);
+ }
+
+ /**
* Project-specific config of the owners plugin.
*
* @param projectKey project name
@@ -63,4 +78,18 @@
throws NoSuchProjectException {
return configFactory.getFromProjectConfigWithInheritance(projectKey, ownersPluginName);
}
+
+ // Logic copied from JGit's TestRepository
+ private static String normalizeRef(String ref) {
+ if (Constants.HEAD.equals(ref)) {
+ // nothing
+ } else if ("FETCH_HEAD".equals(ref)) {
+ // nothing
+ } else if ("MERGE_HEAD".equals(ref)) {
+ // nothing
+ } else if (ref.startsWith(Constants.R_REFS)) {
+ // nothing
+ } else ref = Constants.R_HEADS + ref;
+ return ref;
+ }
}
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PluginSettingsTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PluginSettingsTest.java
new file mode 100644
index 0000000..c4c75d4
--- /dev/null
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PluginSettingsTest.java
@@ -0,0 +1,84 @@
+// Copyright (C) 2022 The Android Open Source Project
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.googlesource.gerrit.owners.common;
+
+import static com.google.common.truth.Truth.assertThat;
+import static org.mockito.Mockito.when;
+
+import com.google.gerrit.server.config.PluginConfigFactory;
+import org.eclipse.jgit.lib.Config;
+import org.eclipse.jgit.lib.Constants;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+@RunWith(MockitoJUnitRunner.class)
+public class PluginSettingsTest {
+ private static final String PLUGIN_NAME = "plugin-name";
+ @Mock PluginConfigFactory mockPluginConfigFactory;
+
+ private PluginSettings pluginSettings;
+
+ public void setupMocks(Config pluginConfig) {
+ when(mockPluginConfigFactory.getGlobalPluginConfig(PLUGIN_NAME)).thenReturn(pluginConfig);
+ pluginSettings = new PluginSettings(mockPluginConfigFactory, PLUGIN_NAME);
+ }
+
+ @Test
+ public void allBranchesAreEnabledByDefault() {
+ setupMocks(new Config());
+
+ assertThat(pluginSettings.disabledBranchPatterns()).isEmpty();
+ assertThat(pluginSettings.isBranchDisabled("some-branch")).isFalse();
+ }
+
+ @Test
+ public void branchRefShouldBeDisabled() {
+ String branchName = "refs/heads/some-branch";
+ Config pluginConfig = new Config();
+ pluginConfig.setString("owners", "disable", "branch", branchName);
+ setupMocks(pluginConfig);
+
+ assertThat(pluginSettings.disabledBranchPatterns()).contains(branchName);
+ assertThat(pluginSettings.isBranchDisabled(branchName)).isTrue();
+ }
+
+ @Test
+ public void branchNameShouldBeDisabled() {
+ String branchName = "some-branch";
+ String branchRefName = Constants.R_HEADS + branchName;
+ Config pluginConfig = new Config();
+ pluginConfig.setString("owners", "disable", "branch", branchRefName);
+ setupMocks(pluginConfig);
+
+ assertThat(pluginSettings.disabledBranchPatterns()).contains(branchRefName);
+ assertThat(pluginSettings.isBranchDisabled(branchName)).isTrue();
+ }
+
+ @Test
+ public void branchNameShouldBeDisabledByRegex() {
+ String branchName1 = "some-branch-1";
+ String branchName2 = "some-branch-2";
+ String branchRefRegex = Constants.R_HEADS + "some-branch-\\d";
+ Config pluginConfig = new Config();
+ pluginConfig.setString("owners", "disable", "branch", branchRefRegex);
+ setupMocks(pluginConfig);
+
+ assertThat(pluginSettings.disabledBranchPatterns()).contains(branchRefRegex);
+ assertThat(pluginSettings.isBranchDisabled(branchName1)).isTrue();
+ assertThat(pluginSettings.isBranchDisabled(branchName2)).isTrue();
+ }
+}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java
index b96ab5b..7b1399a 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java
@@ -28,7 +28,7 @@
public class OwnerPredicateProvider implements PredicateProvider {
@Inject
public OwnerPredicateProvider(Accounts accounts, PluginSettings config) {
- OwnersStoredValues.initialize(accounts, config.disabledBranchPatterns());
+ OwnersStoredValues.initialize(accounts, config);
}
@Override
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
index e749cf2..2d2267e 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
@@ -16,13 +16,13 @@
package com.googlesource.gerrit.owners;
-import com.google.common.collect.ImmutableSet;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.rules.StoredValue;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlesource.gerrit.owners.common.Accounts;
import com.googlesource.gerrit.owners.common.PathOwners;
+import com.googlesource.gerrit.owners.common.PluginSettings;
import java.util.Optional;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
@@ -34,8 +34,7 @@
public static StoredValue<PathOwners> PATH_OWNERS;
- public static synchronized void initialize(
- Accounts accounts, ImmutableSet<String> disablePatterns) {
+ public static synchronized void initialize(Accounts accounts, PluginSettings settings) {
if (PATH_OWNERS != null) {
return;
}
@@ -47,12 +46,11 @@
PatchList patchList = StoredValues.PATCH_LIST.get(engine);
Repository repository = StoredValues.REPOSITORY.get(engine);
String branch = StoredValues.getChange(engine).getDest().branch();
- for (String pattern : disablePatterns) {
- if (branch.matches(pattern)) {
- return new PathOwners(accounts, repository, Optional.empty(), patchList);
- }
- }
- return new PathOwners(accounts, repository, Optional.of(branch), patchList);
+ return new PathOwners(
+ accounts,
+ repository,
+ settings.isBranchDisabled(branch) ? Optional.empty() : Optional.of(branch),
+ patchList);
}
};
}
