Merge branch 'stable-3.10'
* stable-3.10:
Add autoAssignField to docs
Restore the ability for global definition of the owners' label
Change-Id: I731dbb25fd3c6c74bdcac7b57bd590bed69b1710
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 9a8edb6..16c720a 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
@@ -90,6 +90,7 @@
private final AutoAssignConfig cfg;
private final PathOwnersEntriesCache cache;
+ private final PluginSettings pluginSettings;
@Inject
public GitRefListener(
@@ -103,7 +104,8 @@
Provider<CurrentUser> currentUserProvider,
ChangeNotes.Factory notesFactory,
AutoAssignConfig cfg,
- PathOwnersEntriesCache cache) {
+ PathOwnersEntriesCache cache,
+ PluginSettings pluginSettings) {
this.api = api;
this.patchListCache = patchListCache;
this.projectCache = projectCache;
@@ -115,6 +117,7 @@
this.notesFactory = notesFactory;
this.cfg = cfg;
this.cache = cache;
+ this.pluginSettings = pluginSettings;
}
@Override
@@ -241,7 +244,8 @@
patchList,
cfg.expandGroups(),
projectNameKey.get(),
- cache);
+ cache,
+ pluginSettings.globalLabel());
Set<Account.Id> allReviewers = Sets.newHashSet();
allReviewers.addAll(owners.get().values());
allReviewers.addAll(owners.getReviewers().values());
diff --git a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/GitRefListenerTest.java b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/GitRefListenerTest.java
index b651572..846f583 100644
--- a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/GitRefListenerTest.java
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/GitRefListenerTest.java
@@ -47,7 +47,8 @@
Provider<CurrentUser> currentUserProvider,
ChangeNotes.Factory notesFactory,
AutoAssignConfig cfg,
- PathOwnersEntriesCache cache) {
+ PathOwnersEntriesCache cache,
+ PluginSettings pluginSettings) {
super(
api,
patchListCache,
@@ -59,7 +60,8 @@
currentUserProvider,
notesFactory,
cfg,
- cache);
+ cache,
+ pluginSettings);
}
@Override
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
index e7e437a..1e5023b 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
@@ -103,7 +103,8 @@
Map<String, FileDiffOutput> fileDiffMap,
boolean expandGroups,
String project,
- PathOwnersEntriesCache cache)
+ PathOwnersEntriesCache cache,
+ Optional<LabelDefinition> globalLabel)
throws InvalidOwnersFileException {
this(
accounts,
@@ -114,7 +115,8 @@
getModifiedPaths(fileDiffMap),
expandGroups,
project,
- cache);
+ cache,
+ globalLabel);
}
public PathOwners(
@@ -126,7 +128,8 @@
DiffSummary diffSummary,
boolean expandGroups,
String project,
- PathOwnersEntriesCache cache)
+ PathOwnersEntriesCache cache,
+ Optional<LabelDefinition> globalLabel)
throws InvalidOwnersFileException {
this(
accounts,
@@ -137,7 +140,8 @@
ImmutableSet.copyOf(diffSummary.getPaths()),
expandGroups,
project,
- cache);
+ cache,
+ globalLabel);
}
public PathOwners(
@@ -149,7 +153,8 @@
Set<String> modifiedPaths,
boolean expandGroups,
String project,
- PathOwnersEntriesCache cache)
+ PathOwnersEntriesCache cache,
+ Optional<LabelDefinition> globalLabel)
throws InvalidOwnersFileException {
this.repositoryManager = repositoryManager;
this.repository = repository;
@@ -170,7 +175,7 @@
matchers = map.getMatchers();
fileOwners = map.getFileOwners();
fileGroupOwners = map.getFileGroupOwners();
- label = map.getLabel();
+ label = globalLabel.or(map::getLabel);
}
/**
* Returns a read only view of the paths to owners mapping.
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 26b0f62..1fd7403 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
@@ -15,6 +15,7 @@
package com.googlesource.gerrit.owners.common;
+import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.annotations.PluginName;
@@ -23,6 +24,8 @@
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import java.util.Optional;
+import java.util.function.Supplier;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
@@ -36,6 +39,8 @@
private final boolean expandGroups;
private final boolean enableSubmitRequirement;
+ private final Supplier<Optional<LabelDefinition>> globalLabel;
+
@Inject
public PluginSettings(PluginConfigFactory configFactory, @PluginName String ownersPluginName) {
this.configFactory = configFactory;
@@ -48,6 +53,10 @@
this.expandGroups = globalPluginConfig.getBoolean("owners", "expandGroups", true);
this.enableSubmitRequirement =
globalPluginConfig.getBoolean("owners", "enableSubmitRequirement", false);
+
+ this.globalLabel =
+ Suppliers.memoize(
+ () -> LabelDefinition.parse(globalPluginConfig.getString("owners", null, "label")));
}
/**
@@ -95,6 +104,19 @@
return configFactory.getFromProjectConfigWithInheritance(projectKey, ownersPluginName);
}
+ /**
+ * Global definition of the review label to use for the owners' plugin.
+ *
+ * <p>When the global label is set in the plugin.owners.label settings the value overrides any
+ * label name defined in the OWNERS files of any repository.
+ *
+ * @return the global label definition or {@link Optional#empty()} if there isn't any global label
+ * definition.
+ */
+ public Optional<LabelDefinition> globalLabel() {
+ return globalLabel.get();
+ }
+
// Logic copied from JGit's TestRepository
private static String normalizeRef(String ref) {
if (Constants.HEAD.equals(ref)) {
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java
index 542ff6d..7ab294f 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/PathOwnersTest.java
@@ -51,7 +51,11 @@
private static final boolean EXPAND_GROUPS = true;
private static final boolean DO_NOT_EXPAND_GROUPS = false;
private static final String EXPECTED_LABEL = "expected-label";
+ private static final Short EXPECTED_LABEL_SCORE = 1;
+ private static final LabelDefinition EXPECTED_LABEL_DEFINITION =
+ new LabelDefinition(EXPECTED_LABEL, EXPECTED_LABEL_SCORE);
private static final String A_LABEL = "a-label";
+ private static final String B_LABEL = "b-label";
private static PathOwnersEntriesCache CACHE_MOCK = new PathOwnersEntriesCacheMock();
public static final String CLASSIC_FILE_TXT = "classic/file.txt";
@@ -80,12 +84,33 @@
Set.of(CLASSIC_FILE_TXT),
EXPAND_GROUPS,
"foo",
- CACHE_MOCK);
+ CACHE_MOCK,
+ Optional.empty());
Set<Account.Id> ownersSet = owners.get().get(CLASSIC_OWNERS);
assertEquals(2, ownersSet.size());
assertTrue(ownersSet.contains(USER_A_ID));
assertTrue(ownersSet.contains(USER_B_ID));
assertTrue(owners.expandGroups());
+ assertThat(owners.getLabel()).isEmpty();
+ }
+
+ @Test
+ public void testGlobalLabel() throws Exception {
+ mockOwners(USER_A_EMAIL_COM, USER_B_EMAIL_COM);
+
+ PathOwners owners =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ emptyList(),
+ branch,
+ Set.of(CLASSIC_FILE_TXT),
+ EXPAND_GROUPS,
+ "foo",
+ CACHE_MOCK,
+ Optional.of(EXPECTED_LABEL_DEFINITION));
+ assertThat(owners.getLabel()).hasValue(EXPECTED_LABEL_DEFINITION);
}
@Test
@@ -102,7 +127,8 @@
Set.of(CLASSIC_FILE_TXT),
DO_NOT_EXPAND_GROUPS,
"foo",
- CACHE_MOCK);
+ CACHE_MOCK,
+ Optional.empty());
Set<String> ownersSet = owners.getFileGroupOwners().get(CLASSIC_FILE_TXT);
assertEquals(2, ownersSet.size());
assertTrue(ownersSet.contains(USER_A));
@@ -124,7 +150,8 @@
Set.of(CLASSIC_FILE_TXT),
EXPAND_GROUPS,
"foo",
- CACHE_MOCK);
+ CACHE_MOCK,
+ Optional.empty());
Set<Account.Id> ownersSet = owners.get().get(CLASSIC_OWNERS);
assertEquals(0, ownersSet.size());
}
@@ -149,7 +176,8 @@
Set.of("classic/file.txt"),
EXPAND_GROUPS,
"foo",
- CACHE_MOCK);
+ CACHE_MOCK,
+ Optional.empty());
Set<Account.Id> ownersSet2 = owners2.get().get(CLASSIC_OWNERS);
// in this case we are inheriting the acct3 from /OWNERS
@@ -164,6 +192,50 @@
}
@Test
+ public void testClassicWithInheritanceAndGlobalLabel() throws Exception {
+ expectConfig("OWNERS", createConfig(true, Optional.of(A_LABEL), owners()));
+ expectConfig(CLASSIC_OWNERS, createConfig(true, Optional.of(B_LABEL), owners()));
+
+ replayAll();
+
+ PathOwners owners2 =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ emptyList(),
+ branch,
+ Set.of("classic/file.txt"),
+ EXPAND_GROUPS,
+ "foo",
+ CACHE_MOCK,
+ Optional.of(EXPECTED_LABEL_DEFINITION));
+ assertThat(owners2.getLabel()).hasValue(EXPECTED_LABEL_DEFINITION);
+ }
+
+ @Test
+ public void testClassicWithoutInheritanceAndGlobalLabel() throws Exception {
+ expectConfig("OWNERS", createConfig(false, Optional.of(A_LABEL), owners()));
+ expectConfig(CLASSIC_OWNERS, createConfig(false, Optional.of(B_LABEL), owners()));
+
+ replayAll();
+
+ PathOwners owners2 =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ emptyList(),
+ branch,
+ Set.of("classic/file.txt"),
+ EXPAND_GROUPS,
+ "foo",
+ CACHE_MOCK,
+ Optional.of(EXPECTED_LABEL_DEFINITION));
+ assertThat(owners2.getLabel()).hasValue(EXPECTED_LABEL_DEFINITION);
+ }
+
+ @Test
public void testRootInheritFromProject() throws Exception {
expectConfig("OWNERS", "master", createConfig(true, owners()));
expectConfig(
@@ -188,7 +260,8 @@
Set.of(fileName),
EXPAND_GROUPS,
"foo",
- CACHE_MOCK);
+ CACHE_MOCK,
+ Optional.empty());
Map<String, Set<Account.Id>> fileOwners = owners.getFileOwners();
assertEquals(1, fileOwners.size());
@@ -233,7 +306,8 @@
Set.of(fileName),
EXPAND_GROUPS,
"foo",
- CACHE_MOCK);
+ CACHE_MOCK,
+ Optional.empty());
Map<String, Set<Account.Id>> fileOwners = owners.getFileOwners();
assertEquals(fileOwners.size(), 1);
@@ -282,7 +356,8 @@
Set.of(sqlFileName, javaFileName),
EXPAND_GROUPS,
"foo",
- CACHE_MOCK);
+ CACHE_MOCK,
+ Optional.empty());
Map<String, Set<Account.Id>> fileOwners = owners.getFileOwners();
assertEquals(fileOwners.size(), 2);
@@ -326,7 +401,8 @@
Set.of("dir/subdir/file.txt"),
EXPAND_GROUPS,
"foo",
- CACHE_MOCK);
+ CACHE_MOCK,
+ Optional.empty());
Set<Account.Id> ownersSet = owners.get().get("dir/subdir/OWNERS");
assertEquals(3, ownersSet.size());
@@ -406,7 +482,8 @@
Set.of("dir/subdir/file.txt"),
EXPAND_GROUPS,
"foo",
- cacheMock);
+ cacheMock,
+ Optional.empty());
assertThat(owners.getFileOwners()).isNotEmpty();
int expectedCacheCalls =
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
index c4c75d4..2aecd88 100644
--- 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
@@ -18,6 +18,7 @@
import static org.mockito.Mockito.when;
import com.google.gerrit.server.config.PluginConfigFactory;
+import java.util.Optional;
import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.Constants;
import org.junit.Test;
@@ -81,4 +82,21 @@
assertThat(pluginSettings.isBranchDisabled(branchName1)).isTrue();
assertThat(pluginSettings.isBranchDisabled(branchName2)).isTrue();
}
+
+ @Test
+ public void globalLabelIsEmptyByDefault() {
+ setupMocks(new Config());
+
+ assertThat(pluginSettings.globalLabel()).isEqualTo(Optional.empty());
+ }
+
+ @Test
+ public void globalLabelSetByConfig() {
+ LabelDefinition globalLabelName = new LabelDefinition("Custom-Label", (short) 2);
+ Config pluginConfig = new Config();
+ pluginConfig.setString("owners", null, "label", "Custom-Label,2");
+ setupMocks(pluginConfig);
+
+ assertThat(pluginSettings.globalLabel()).isEqualTo(Optional.of(globalLabelName));
+ }
}
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java
index a035d93..821db6a 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexTest.java
@@ -31,6 +31,7 @@
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.junit.Before;
@@ -156,7 +157,8 @@
"project/file.sql"), // only .sql matching b,c
EXPAND_GROUPS,
"foo",
- new PathOwnersEntriesCacheMock());
+ new PathOwnersEntriesCacheMock(),
+ Optional.empty());
// assertions on classic owners
Set<Account.Id> ownersSet = owners.get().get("project/OWNERS");
@@ -263,7 +265,8 @@
Set.of("project/file.sql", "another.txt"),
EXPAND_GROUPS,
"foo",
- new PathOwnersEntriesCacheMock());
+ new PathOwnersEntriesCacheMock(),
+ Optional.empty());
Set<String> ownedFiles = owners.getFileOwners().keySet();
assertThat(ownedFiles).containsExactly("project/file.sql");
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 7c532c6..84cc6a2 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
@@ -73,7 +73,8 @@
patchList,
settings.expandGroups(),
projectState.getName(),
- cache);
+ cache,
+ settings.globalLabel());
} catch (InvalidOwnersFileException e) {
// re-throw exception as it is already logged but more importantly it is nicely
// handled by the prolog rules evaluator and results in prolog rule error
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
index 0d8dac4..64638bc 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersSubmitRequirement.java
@@ -216,7 +216,8 @@
getDiff(nameKey, cd.currentPatchSet().commitId()),
pluginSettings.expandGroups(),
nameKey.get(),
- cache);
+ cache,
+ pluginSettings.globalLabel());
return pathOwners;
}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
index a724239..bbe3007 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
@@ -114,7 +114,8 @@
changePaths,
pluginSettings.expandGroups(),
project.get(),
- cache);
+ cache,
+ pluginSettings.globalLabel());
Map<String, Set<GroupOwner>> fileExpandedOwners =
Maps.transformValues(
diff --git a/owners/src/main/resources/Documentation/config.md b/owners/src/main/resources/Documentation/config.md
index debba42..89c6248 100644
--- a/owners/src/main/resources/Documentation/config.md
+++ b/owners/src/main/resources/Documentation/config.md
@@ -28,6 +28,18 @@
expandGroups = false
```
+owners.label
+: Global override for the label and score, separated by a comma, to use by
+ the owners of changes for approving them. When defined, it overrides any
+ other label definition set by the OWNERS at any level in any project.
+
+ Example:
+
+ ```
+ [owners]
+ label = Code-Review, 1
+ ```
+
<a name="owners.enableSubmitRequirement">owners.enableSubmitRequirement</a>
: If set to `true` the approvals are evaluated through the owners plugin
provided submit rule without a need of prolog predicate being added to a
