Merge branch 'stable-3.4'
* stable-3.4:
Fix REST-API documentation for HTTP GET files-owners
Introduce GenericMatcher for file-based regex fallback logic
Filter out null parent projects when fetching the owners of a file
Fix NPE when returning group owners without expansion
Inherit OWNERS from all parent projects
Add inheritance from parent project
Refactor GetFilesOwnersIT and make FilesOwnersResponse public
Introduce expandGroups option for disabling groups expansion
Add labels information in files-owners API
REST API to expose owners of each file
Allow disabling the owners auto-assigned on a per-branch basis
DRY out the logic to read the owners plugin config
Allow disabling the owners on a per-branch basis
Also fix an error-prone issue in GetFilesOwners where the map
of an Optional was using a side effect implicitly, ignoring
the result.
Change-Id: I6e3de0de278475274d7f97935cf4f7b9d31a079e
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 546e0aa..1082212 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
@@ -20,36 +20,39 @@
import static com.googlesource.gerrit.owners.common.AutoassignConfigModule.PROJECT_CONFIG_AUTOASSIGN_WIP_CHANGES;
import com.google.gerrit.entities.Project;
-import com.google.gerrit.extensions.annotations.PluginName;
import com.google.gerrit.extensions.client.ReviewerState;
-import com.google.gerrit.server.config.PluginConfig;
-import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@Singleton
public class AutoassignConfig {
-
- private final PluginConfigFactory cfgFactory;
- private final String pluginName;
+ private final PluginSettings config;
@Inject
- AutoassignConfig(@PluginName String pluginName, PluginConfigFactory cfgFactory) {
- this.pluginName = pluginName;
- this.cfgFactory = cfgFactory;
+ AutoassignConfig(PluginSettings config) {
+ this.config = config;
}
public boolean autoAssignWip(Project.NameKey projectKey) throws NoSuchProjectException {
- return cfg(projectKey).getEnum(PROJECT_CONFIG_AUTOASSIGN_WIP_CHANGES, TRUE).equals(TRUE);
+ return config
+ .projectSpecificConfig(projectKey)
+ .getEnum(PROJECT_CONFIG_AUTOASSIGN_WIP_CHANGES, TRUE)
+ .equals(TRUE);
}
public ReviewerState autoassignedReviewerState(Project.NameKey projectKey)
throws NoSuchProjectException {
- return cfg(projectKey).getEnum(PROJECT_CONFIG_AUTOASSIGN_FIELD, ReviewerState.REVIEWER);
+ return config
+ .projectSpecificConfig(projectKey)
+ .getEnum(PROJECT_CONFIG_AUTOASSIGN_FIELD, ReviewerState.REVIEWER);
}
- private PluginConfig cfg(Project.NameKey projectKey) throws NoSuchProjectException {
- return cfgFactory.getFromProjectConfigWithInheritance(projectKey, pluginName);
+ public boolean isBranchDisabled(String branch) {
+ return config.isBranchDisabled(branch);
+ }
+
+ public boolean expandGroups() {
+ return config.expandGroups();
}
}
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 0064c36..5be3aa0 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
@@ -46,13 +46,18 @@
import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
import com.google.gerrit.server.project.NoSuchProjectException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
import com.google.inject.Provider;
import java.io.IOException;
+import java.util.Collections;
import java.util.List;
+import java.util.Optional;
import java.util.Set;
+import java.util.stream.Collectors;
import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.lib.ObjectId;
@@ -71,6 +76,7 @@
private final GerritApi api;
private final PatchListCache patchListCache;
+ private final ProjectCache projectCache;
private final GitRepositoryManager repositoryManager;
private final Accounts accounts;
private final ReviewerManager reviewerManager;
@@ -87,6 +93,7 @@
public GitRefListener(
GerritApi api,
PatchListCache patchListCache,
+ ProjectCache projectCache,
GitRepositoryManager repositoryManager,
Accounts accounts,
ReviewerManager reviewerManager,
@@ -96,6 +103,7 @@
AutoassignConfig cfg) {
this.api = api;
this.patchListCache = patchListCache;
+ this.projectCache = projectCache;
this.repositoryManager = repositoryManager;
this.accounts = accounts;
this.reviewerManager = reviewerManager;
@@ -207,9 +215,27 @@
try {
ChangeApi cApi = changes.id(cId.get());
ChangeInfo change = cApi.get();
+ List<NameKey> parentProjectsNameKeys =
+ projectCache
+ .get(NameKey.parse(change.project))
+ .map(
+ p ->
+ p.parents().stream()
+ .map(ProjectState::getNameKey)
+ .collect(Collectors.toList()))
+ .orElse(Collections.emptyList());
+
DiffSummary patchList = getDiffSummary(repository, event, change);
if (patchList != null) {
- PathOwners owners = new PathOwners(accounts, repository, change.branch, patchList);
+ PathOwners owners =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ parentProjectsNameKeys,
+ cfg.isBranchDisabled(change.branch) ? Optional.empty() : Optional.of(change.branch),
+ patchList,
+ cfg.expandGroups());
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 96a8fa0..a99e3b1 100644
--- a/owners-autoassign/src/main/resources/Documentation/config.md
+++ b/owners-autoassign/src/main/resources/Documentation/config.md
@@ -8,6 +8,22 @@
file can be installed like a regular Gerrit plugin, by being dropped to the
`GRRIT_SITE/plugins` directory or installed through the plugin manager.
+## 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
@@ -63,6 +79,11 @@
(partial or full) and exact string comparison. For exact match, path is
relative to the root of the repo.
+> **NOTE:** The `generic` matcher is a special type of regex matching that
+> is applied only when none of the other sections are matching. It is
+> used to define fallback rules. The `generic: .*` is the top-level fallback
+> and can be used with other more specific `generic` matchers.
+
The plugin analyzes the latest patch set by looking at each file directory and
building an OWNERS hierarchy. It stops once it finds an OWNERS file that has
“inherited” set to false (by default it’s true.)
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 fd258bb..52fc441 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,10 @@
import static com.googlesource.gerrit.owners.common.AutoassignConfigModule.PROJECT_CONFIG_AUTOASSIGN_FIELD;
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.TestAccount;
+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.ChangeApi;
import com.google.gerrit.extensions.api.changes.ChangeEditApi;
@@ -34,7 +38,9 @@
import com.google.inject.Module;
import com.googlesource.gerrit.owners.api.OwnersApiModule;
import java.util.Collection;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
import java.util.stream.Collectors;
import org.junit.Ignore;
import org.junit.Test;
@@ -52,6 +58,10 @@
@Inject
private ProjectConfig.Factory projectConfigFactory;
+ private TestAccount user2;
+
+ private TestAccount admin2;
+
AbstractAutoassignIT(String section, ReviewerState assignedUserState) {
this.section = section;
this.assignedUserState = assignedUserState;
@@ -82,6 +92,9 @@
projectConfig.commit(md);
projectCache.evict(project);
}
+
+ user2 = accountCreator.user2();
+ admin2 = accountCreator.admin2();
}
@Test
@@ -116,6 +129,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();
@@ -180,7 +205,7 @@
public void shouldAutoassignUserMatchingPath() throws Exception {
String ownerEmail = user.email();
- addOwnersToRepo("", "suffix", ".java", ownerEmail, NOT_INHERITED);
+ addOwnersToRepo("", NOT_INHERITED, "suffix", ".java", ownerEmail);
Collection<AccountInfo> reviewers =
getAutoassignedAccounts(change(createChange("test change", "foo.java", "foo")).get());
@@ -193,7 +218,7 @@
public void shouldNotAutoassignUserNotMatchingPath() throws Exception {
String ownerEmail = user.email();
- addOwnersToRepo("", "suffix", ".java", ownerEmail, NOT_INHERITED);
+ addOwnersToRepo("", NOT_INHERITED, "suffix", ".java", ownerEmail);
ChangeApi changeApi = change(createChange("test change", "foo.bar", "foo"));
Collection<AccountInfo> reviewers = getAutoassignedAccounts(changeApi.get());
@@ -202,13 +227,109 @@
}
@Test
+ public void shouldAutoassignUserWithGenericTopLevelFallback() throws Exception {
+ String ownerEmail = user.email();
+ String owner2Email = user2.email();
+ String admin2Email = admin2.email();
+
+ addOwnersToRepo(
+ "",
+ NOT_INHERITED,
+ "suffix",
+ ".java",
+ ownerEmail,
+ "generic",
+ ".*\\.c",
+ admin2Email,
+ "generic",
+ ".*",
+ owner2Email);
+
+ ChangeApi changeApi =
+ change(createChangeWithFiles("test change", "foo.bar", "foo", "foo.java", "Java code"));
+ Collection<AccountInfo> reviewers = getAutoassignedAccounts(changeApi.get());
+
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewersEmail(reviewers)).containsExactly(ownerEmail, owner2Email);
+ }
+
+ @Test
+ public void shouldAutoassignUserWithGenericMidLevelFallback() throws Exception {
+ String ownerEmail = user.email();
+ String owner2Email = user2.email();
+ String admin2Email = admin2.email();
+
+ addOwnersToRepo(
+ "",
+ NOT_INHERITED,
+ "suffix",
+ ".java",
+ ownerEmail,
+ "generic",
+ ".*\\.c",
+ admin2Email,
+ "generic",
+ ".*",
+ owner2Email);
+
+ ChangeApi changeApi =
+ change(createChangeWithFiles("test change", "foo.c", "foo", "foo.java", "Java code"));
+ Collection<AccountInfo> reviewers = getAutoassignedAccounts(changeApi.get());
+
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewersEmail(reviewers)).containsExactly(ownerEmail, admin2Email);
+ }
+
+ @Test
+ public void shouldNotAutoassignUserWithNonMatchingGenericFallback() throws Exception {
+ String ownerEmail = user.email();
+ String owner2Email = user2.email();
+
+ addOwnersToRepo(
+ "", NOT_INHERITED, "suffix", ".java", ownerEmail, "generic", "\\.c", owner2Email);
+
+ ChangeApi changeApi =
+ change(createChangeWithFiles("test change", "foo.bar", "foo", "foo.groovy", "Groovy code"));
+ Collection<AccountInfo> reviewers = getAutoassignedAccounts(changeApi.get());
+
+ assertThat(reviewers).isNull();
+ }
+
+ @Test
+ public void shouldAutoassignUserWithMultipleGenericFallback() throws Exception {
+ String admin2Email = admin2.email();
+ String ownerEmail = user.email();
+ String owner2Email = user2.email();
+
+ addOwnersToRepo(
+ "",
+ NOT_INHERITED,
+ "suffix",
+ ".java",
+ admin2Email,
+ "generic",
+ ".*\\.c",
+ ownerEmail,
+ "generic",
+ ".*",
+ owner2Email);
+
+ ChangeApi changeApi =
+ change(createChangeWithFiles("test change", "foo.bar", "foo", "foo.c", "C code"));
+ Collection<AccountInfo> reviewers = getAutoassignedAccounts(changeApi.get());
+
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewersEmail(reviewers)).containsExactly(ownerEmail, owner2Email);
+ }
+
+ @Test
public void shouldAutoassignUserMatchingPathWithInheritance() throws Exception {
String childOwnersEmail = accountCreator.user2().email();
String parentOwnersEmail = user.email();
String childpath = "childpath/";
- addOwnersToRepo("", "suffix", ".java", parentOwnersEmail, NOT_INHERITED);
- addOwnersToRepo(childpath, "suffix", ".java", childOwnersEmail, INHERITED);
+ addOwnersToRepo("", NOT_INHERITED, "suffix", ".java", parentOwnersEmail);
+ addOwnersToRepo(childpath, INHERITED, "suffix", ".java", childOwnersEmail);
ChangeApi changeApi = change(createChange("test change", childpath + "foo.java", "foo"));
Collection<AccountInfo> reviewers = getAutoassignedAccounts(changeApi.get());
@@ -224,7 +345,7 @@
String childpath = "childpath/";
addOwnersToRepo("", parentOwnersEmail, NOT_INHERITED);
- addOwnersToRepo(childpath, "suffix", ".java", childOwnersEmail, NOT_INHERITED);
+ addOwnersToRepo(childpath, NOT_INHERITED, "suffix", ".java", childOwnersEmail);
ChangeApi changeApi = change(createChange("test change", childpath + "foo.java", "foo"));
Collection<AccountInfo> reviewers = getAutoassignedAccounts(changeApi.get());
@@ -233,6 +354,18 @@
assertThat(reviewersEmail(reviewers)).containsExactly(childOwnersEmail);
}
+ protected PushOneCommit.Result createChangeWithFiles(String subject, String... filesWithContent)
+ throws Exception {
+ Map<String, String> files = new HashMap<>();
+ for (int i = 0; i < filesWithContent.length; ) {
+ String fileName = filesWithContent[i++];
+ String fileContent = filesWithContent[i++];
+ files.put(fileName, fileContent);
+ }
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo, subject, files);
+ return push.to("refs/for/master");
+ }
+
private Collection<AccountInfo> getAutoassignedAccounts(ChangeInfo changeInfo)
throws RestApiException {
Collection<AccountInfo> reviewers =
@@ -258,33 +391,35 @@
.assertOkStatus();
}
- private void addOwnersToRepo(
- String parentPath,
- String matchingType,
- String patternMatch,
- String ownerEmail,
- boolean inherited)
+ private void addOwnersToRepo(String parentPath, boolean inherited, String... matchingRules)
throws Exception {
+ StringBuilder ownersStringBuilder =
+ new StringBuilder("inherited: " + inherited + "\n" + "matchers:\n");
+ for (int i = 0; i < matchingRules.length; ) {
+ String matchingType = matchingRules[i++];
+ String patternMatch = matchingRules[i++];
+ String ownerEmail = matchingRules[i++];
+
+ ownersStringBuilder
+ .append("- ")
+ .append(matchingType)
+ .append(": ")
+ .append(patternMatch)
+ .append("\n")
+ .append(" ")
+ .append(section)
+ .append(":\n")
+ .append(" - ")
+ .append(ownerEmail)
+ .append("\n");
+ }
pushFactory
.create(
admin.newIdent(),
testRepo,
"Set OWNERS",
parentPath + "OWNERS",
- "inherited: "
- + inherited
- + "\n"
- + "matchers:\n"
- + "- "
- + matchingType
- + ": "
- + patternMatch
- + "\n"
- + " "
- + section
- + ":\n"
- + " - "
- + ownerEmail)
+ ownersStringBuilder.toString())
.to("refs/heads/master")
.assertOkStatus();
}
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
index e460136..bc093be 100644
--- a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
@@ -22,6 +22,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
import com.google.inject.Provider;
@@ -42,6 +43,7 @@
public GitRefListenerTest(
GerritApi api,
PatchListCache patchListCache,
+ ProjectCache projectCache,
GitRepositoryManager repositoryManager,
Accounts accounts,
ReviewerManager reviewerManager,
@@ -52,6 +54,7 @@
super(
api,
patchListCache,
+ projectCache,
repositoryManager,
accounts,
reviewerManager,
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java
index 2fa2b16..84d8f6c 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ConfigurationParser.java
@@ -78,6 +78,10 @@
return iteratorStream(node.iterator()).map(JsonNode::asText);
}
+ private static <T> Stream<T> flatten(Optional<Stream<T>> optionalStream) {
+ return optionalStream.orElse(Stream.empty());
+ }
+
private Stream<String> toClassicOwnersList(JsonNode jsonNode, String sectionName) {
Stream<String> ownersStream =
Optional.ofNullable(jsonNode.get(sectionName))
@@ -93,6 +97,12 @@
.orElse(Stream.empty())
.flatMap(o -> accounts.find(o).stream())
.collect(Collectors.toSet());
+ Set<String> groupOwners =
+ flatten(
+ getNode(node, "owners")
+ .map(ConfigurationParser::extractAsText)
+ .map(owns -> owns.map(PathOwnersEntry::stripOwnerDomain)))
+ .collect(Collectors.toSet());
Set<Id> reviewers =
getNode(node, "reviewers")
.map(ConfigurationParser::extractAsText)
@@ -101,13 +111,16 @@
.collect(Collectors.toSet());
Optional<Matcher> suffixMatcher =
- getText(node, "suffix").map(el -> new SuffixMatcher(el, owners, reviewers));
+ getText(node, "suffix").map(el -> new SuffixMatcher(el, owners, reviewers, groupOwners));
Optional<Matcher> regexMatcher =
- getText(node, "regex").map(el -> new RegExMatcher(el, owners, reviewers));
+ getText(node, "regex").map(el -> new RegExMatcher(el, owners, reviewers, groupOwners));
Optional<Matcher> partialRegexMatcher =
- getText(node, "partial_regex").map(el -> new PartialRegExMatcher(el, owners, reviewers));
+ getText(node, "partial_regex")
+ .map(el -> new PartialRegExMatcher(el, owners, reviewers, groupOwners));
Optional<Matcher> exactMatcher =
- getText(node, "exact").map(el -> new ExactMatcher(el, owners, reviewers));
+ getText(node, "exact").map(el -> new ExactMatcher(el, owners, reviewers, groupOwners));
+ Optional<Matcher> genericMatcher =
+ getText(node, "generic").map(el -> new GenericMatcher(el, owners, reviewers, groupOwners));
return Optional.ofNullable(
suffixMatcher.orElseGet(
@@ -117,10 +130,13 @@
partialRegexMatcher.orElseGet(
() ->
exactMatcher.orElseGet(
- () -> {
- log.warn("Ignoring invalid element " + node.toString());
- return null;
- })))));
+ () ->
+ genericMatcher.orElseGet(
+ () -> {
+ log.warn(
+ "Ignoring invalid element " + node.toString());
+ return null;
+ }))))));
}
private static Optional<String> getText(JsonNode node, String field) {
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ExactMatcher.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ExactMatcher.java
index d97b01e..7088289 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ExactMatcher.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/ExactMatcher.java
@@ -20,8 +20,9 @@
import java.util.Set;
public class ExactMatcher extends Matcher {
- public ExactMatcher(String path, Set<Account.Id> owners, Set<Account.Id> reviewers) {
- super(path, owners, reviewers);
+ public ExactMatcher(
+ String path, Set<Account.Id> owners, Set<Account.Id> reviewers, Set<String> groupOwners) {
+ super(path, owners, reviewers, groupOwners);
}
@Override
@@ -30,7 +31,7 @@
}
@Override
- protected Matcher clone(Set<Id> owners, Set<Id> reviewers) {
- return new ExactMatcher(path, owners, reviewers);
+ protected Matcher clone(Set<Id> owners, Set<Id> reviewers, Set<String> groupOwners) {
+ return new ExactMatcher(path, owners, reviewers, groupOwners);
}
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/GenericMatcher.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/GenericMatcher.java
new file mode 100644
index 0000000..e37065b
--- /dev/null
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/GenericMatcher.java
@@ -0,0 +1,27 @@
+// 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 com.google.gerrit.entities.Account;
+import java.util.Set;
+
+public class GenericMatcher extends RegExMatcher {
+
+ public GenericMatcher(
+ String path, Set<Account.Id> owners, Set<Account.Id> reviewers, Set<String> groupOwners) {
+ super(path, owners, reviewers, groupOwners);
+ }
+}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/Matcher.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/Matcher.java
index 7c1565f..321b588 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/Matcher.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/Matcher.java
@@ -22,23 +22,38 @@
public abstract class Matcher {
private Set<Account.Id> owners;
private Set<Account.Id> reviewers;
+ private Set<String> groupOwners;
protected String path;
- public Matcher(String key, Set<Account.Id> owners, Set<Account.Id> reviewers) {
+ public Matcher(
+ String key, Set<Account.Id> owners, Set<Account.Id> reviewers, Set<String> groupOwners) {
this.path = key;
this.owners = owners;
this.reviewers = reviewers;
+ this.groupOwners = groupOwners;
}
@Override
public String toString() {
- return "Matcher [path=" + path + ", owners=" + owners + ", reviewers=" + reviewers + "]";
+ return "Matcher [path="
+ + path
+ + ", owners="
+ + owners
+ + ", groupOwners="
+ + groupOwners
+ + ", reviewers="
+ + reviewers
+ + "]";
}
public Set<Account.Id> getOwners() {
return owners;
}
+ public Set<String> getGroupOwners() {
+ return groupOwners;
+ }
+
public void setOwners(Set<Account.Id> owners) {
this.owners = owners;
}
@@ -66,13 +81,16 @@
return this;
}
- return clone(mergeSet(owners, other.owners), mergeSet(reviewers, other.reviewers));
+ return clone(
+ mergeSet(owners, other.owners),
+ mergeSet(reviewers, other.reviewers),
+ mergeSet(groupOwners, other.groupOwners));
}
- protected abstract Matcher clone(Set<Id> owners, Set<Id> reviewers);
+ protected abstract Matcher clone(Set<Id> owners, Set<Id> reviewers, Set<String> groupOwners);
- private Set<Id> mergeSet(Set<Id> set1, Set<Id> set2) {
- ImmutableSet.Builder<Id> setBuilder = ImmutableSet.builder();
+ private <T> Set<T> mergeSet(Set<T> set1, Set<T> set2) {
+ ImmutableSet.Builder<T> setBuilder = ImmutableSet.builder();
return setBuilder.addAll(set1).addAll(set2).build();
}
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/OwnersMap.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/OwnersMap.java
index 111b88e..c4897bb 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/OwnersMap.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/OwnersMap.java
@@ -29,6 +29,7 @@
private Map<String, Matcher> matchers = Maps.newHashMap();
private Map<String, Set<Account.Id>> fileOwners = Maps.newHashMap();
private Map<String, Set<Account.Id>> fileReviewers = Maps.newHashMap();
+ private Map<String, Set<String>> fileGroupOwners = Maps.newHashMap();
@Override
public String toString() {
@@ -79,6 +80,10 @@
return fileReviewers;
}
+ public Map<String, Set<String>> getFileGroupOwners() {
+ return fileGroupOwners;
+ }
+
public void addFileOwners(String file, Set<Id> owners) {
if (owners.isEmpty()) {
return;
@@ -106,4 +111,12 @@
fileReviewers.put(file, Sets.newHashSet(reviewers));
}
}
+
+ public void addFileGroupOwners(String file, Set<String> groupOwners) {
+ if (groupOwners.isEmpty()) {
+ return;
+ }
+
+ fileGroupOwners.computeIfAbsent(file, (f) -> Sets.newHashSet()).addAll(groupOwners);
+ }
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PartialRegExMatcher.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PartialRegExMatcher.java
index 18f04f5..479a287 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PartialRegExMatcher.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PartialRegExMatcher.java
@@ -24,8 +24,9 @@
public class PartialRegExMatcher extends Matcher {
Pattern pattern;
- public PartialRegExMatcher(String path, Set<Account.Id> owners, Set<Account.Id> reviewers) {
- super(path, owners, reviewers);
+ public PartialRegExMatcher(
+ String path, Set<Account.Id> owners, Set<Account.Id> reviewers, Set<String> groupOwners) {
+ super(path, owners, reviewers, groupOwners);
pattern = Pattern.compile(".*" + path + ".*");
}
@@ -35,7 +36,7 @@
}
@Override
- protected Matcher clone(Set<Id> owners, Set<Id> reviewers) {
- return new PartialRegExMatcher(path, owners, reviewers);
+ protected Matcher clone(Set<Id> owners, Set<Id> reviewers, Set<String> groupOwners) {
+ return new PartialRegExMatcher(path, owners, reviewers, groupOwners);
}
}
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 172a824..5583dc9 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
@@ -20,25 +20,30 @@
import static com.google.gerrit.entities.Patch.MERGE_LIST;
import static com.googlesource.gerrit.owners.common.JgitWrapper.getBlobAsBytes;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimaps;
import com.google.common.collect.SetMultimap;
import com.google.common.collect.Sets;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Account.Id;
import com.google.gerrit.entities.Patch;
+import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.DiffSummary;
import com.google.gerrit.server.patch.filediff.FileDiffOutput;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
-import java.util.Iterator;
+import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import java.util.stream.Collectors;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -49,48 +54,100 @@
private static final Logger log = LoggerFactory.getLogger(PathOwners.class);
+ private enum MatcherLevel {
+ Regular,
+ Fallback,
+ CatchAll;
+
+ static MatcherLevel forMatcher(Matcher matcher) {
+ return matcher instanceof GenericMatcher
+ ? (matcher.path.equals(".*") ? CatchAll : Fallback)
+ : Regular;
+ }
+ }
+
private final SetMultimap<String, Account.Id> owners;
private final SetMultimap<String, Account.Id> reviewers;
private final Repository repository;
+ private final List<Project.NameKey> parentProjectsNames;
+
private final ConfigurationParser parser;
private final Set<String> modifiedPaths;
private final Accounts accounts;
+ private final GitRepositoryManager repositoryManager;
+
private Map<String, Matcher> matchers;
private Map<String, Set<Id>> fileOwners;
+ private Map<String, Set<String>> fileGroupOwners;
+
+ private final boolean expandGroups;
+
public PathOwners(
Accounts accounts,
+ GitRepositoryManager repositoryManager,
Repository repository,
- String branch,
- Map<String, FileDiffOutput> fileDiffMap) {
- this(accounts, repository, branch, getModifiedPaths(fileDiffMap));
+ List<Project.NameKey> parentProjectsNames,
+ Optional<String> branchWhenEnabled,
+ Map<String, FileDiffOutput> fileDiffMap,
+ boolean expandGroups) {
+ this(
+ accounts,
+ repositoryManager,
+ repository,
+ parentProjectsNames,
+ branchWhenEnabled,
+ getModifiedPaths(fileDiffMap),
+ expandGroups);
}
public PathOwners(
- Accounts accounts, Repository repository, String branch, DiffSummary diffSummary) {
- this(accounts, repository, branch, ImmutableSet.copyOf(diffSummary.getPaths()));
+ Accounts accounts,
+ GitRepositoryManager repositoryManager,
+ Repository repository,
+ List<Project.NameKey> parentProjectsNames,
+ Optional<String> branchWhenEnabled,
+ DiffSummary diffSummary,
+ boolean expandGroups) {
+ this(
+ accounts,
+ repositoryManager,
+ repository,
+ parentProjectsNames,
+ branchWhenEnabled,
+ ImmutableSet.copyOf(diffSummary.getPaths()),
+ expandGroups);
}
- private PathOwners(
- Accounts accounts, Repository repository, String branch, Set<String> modifiedPaths) {
+ public PathOwners(
+ Accounts accounts,
+ GitRepositoryManager repositoryManager,
+ Repository repository,
+ List<Project.NameKey> parentProjectsNames,
+ Optional<String> branchWhenEnabled,
+ Set<String> modifiedPaths,
+ boolean expandGroups) {
+ this.repositoryManager = repositoryManager;
this.repository = repository;
+ this.parentProjectsNames = parentProjectsNames;
this.modifiedPaths = modifiedPaths;
-
this.parser = new ConfigurationParser(accounts);
this.accounts = accounts;
+ this.expandGroups = expandGroups;
- OwnersMap map = fetchOwners(branch);
+ OwnersMap map = branchWhenEnabled.map(branch -> fetchOwners(branch)).orElse(new OwnersMap());
owners = Multimaps.unmodifiableSetMultimap(map.getPathOwners());
reviewers = Multimaps.unmodifiableSetMultimap(map.getPathReviewers());
matchers = map.getMatchers();
fileOwners = map.getFileOwners();
+ fileGroupOwners = map.getFileGroupOwners();
}
/**
* Returns a read only view of the paths to owners mapping.
@@ -118,6 +175,14 @@
return fileOwners;
}
+ public Map<String, Set<String>> getFileGroupOwners() {
+ return fileGroupOwners;
+ }
+
+ public boolean expandGroups() {
+ return expandGroups;
+ }
+
/**
* Fetched the owners for the associated patch list.
*
@@ -126,42 +191,23 @@
private OwnersMap fetchOwners(String branch) {
OwnersMap ownersMap = new OwnersMap();
try {
- String rootPath = "OWNERS";
-
- PathOwnersEntry projectEntry =
- getOwnersConfig(rootPath, RefNames.REFS_CONFIG)
- .map(
- conf ->
- new PathOwnersEntry(
- rootPath,
- conf,
- accounts,
- Collections.emptySet(),
- Collections.emptySet(),
- Collections.emptySet()))
- .orElse(new PathOwnersEntry());
-
- PathOwnersEntry rootEntry =
- getOwnersConfig(rootPath, branch)
- .map(
- conf ->
- new PathOwnersEntry(
- rootPath,
- conf,
- accounts,
- Collections.emptySet(),
- Collections.emptySet(),
- Collections.emptySet()))
- .orElse(new PathOwnersEntry());
+ // Using a `map` would have needed a try/catch inside the lamba, resulting in more code
+ List<PathOwnersEntry> parentsPathOwnersEntries =
+ getPathOwnersEntries(parentProjectsNames, RefNames.REFS_CONFIG);
+ PathOwnersEntry projectEntry = getPathOwnersEntry(repository, RefNames.REFS_CONFIG);
+ PathOwnersEntry rootEntry = getPathOwnersEntry(repository, branch);
Map<String, PathOwnersEntry> entries = new HashMap<>();
PathOwnersEntry currentEntry = null;
for (String path : modifiedPaths) {
- currentEntry = resolvePathEntry(path, branch, projectEntry, rootEntry, entries);
+ currentEntry =
+ resolvePathEntry(
+ path, branch, projectEntry, parentsPathOwnersEntries, rootEntry, entries);
// add owners and reviewers to file for matcher predicates
ownersMap.addFileOwners(path, currentEntry.getOwners());
ownersMap.addFileReviewers(path, currentEntry.getReviewers());
+ ownersMap.addFileGroupOwners(path, currentEntry.getGroupOwners());
// Only add the path to the OWNERS file to reduce the number of
// entries in the result
@@ -191,26 +237,82 @@
}
}
+ private List<PathOwnersEntry> getPathOwnersEntries(
+ List<Project.NameKey> projectNames, String branch) throws IOException {
+ ImmutableList.Builder<PathOwnersEntry> pathOwnersEntries = ImmutableList.builder();
+ for (Project.NameKey projectName : projectNames) {
+ try (Repository repo = repositoryManager.openRepository(projectName)) {
+ pathOwnersEntries = pathOwnersEntries.add(getPathOwnersEntry(repo, branch));
+ }
+ }
+ return pathOwnersEntries.build();
+ }
+
+ private PathOwnersEntry getPathOwnersEntry(Repository repo, String branch) throws IOException {
+ String rootPath = "OWNERS";
+ return getOwnersConfig(repo, rootPath, branch)
+ .map(
+ conf ->
+ new PathOwnersEntry(
+ rootPath,
+ conf,
+ accounts,
+ Collections.emptySet(),
+ Collections.emptySet(),
+ Collections.emptySet(),
+ Collections.emptySet()))
+ .orElse(new PathOwnersEntry());
+ }
+
private void processMatcherPerPath(
Map<String, Matcher> fullMatchers,
HashMap<String, Matcher> newMatchers,
String path,
OwnersMap ownersMap) {
- Iterator<Matcher> it = fullMatchers.values().iterator();
- while (it.hasNext()) {
- Matcher matcher = it.next();
+
+ Map<MatcherLevel, List<Matcher>> matchersByLevel =
+ fullMatchers.values().stream().collect(Collectors.groupingBy(MatcherLevel::forMatcher));
+ if (findAndAddMatchers(
+ newMatchers, path, ownersMap, matchersByLevel.get(MatcherLevel.Regular))) {
+ return;
+ }
+
+ if (findAndAddMatchers(
+ newMatchers, path, ownersMap, matchersByLevel.get(MatcherLevel.Fallback))) {
+ return;
+ }
+
+ findAndAddMatchers(newMatchers, path, ownersMap, matchersByLevel.get(MatcherLevel.CatchAll));
+ }
+
+ private boolean findAndAddMatchers(
+ HashMap<String, Matcher> newMatchers,
+ String path,
+ OwnersMap ownersMap,
+ @Nullable List<Matcher> matchers) {
+ if (matchers == null) {
+ return false;
+ }
+
+ boolean matchingFound = false;
+
+ for (Matcher matcher : matchers) {
if (matcher.matches(path)) {
newMatchers.put(matcher.getPath(), matcher);
ownersMap.addFileOwners(path, matcher.getOwners());
+ ownersMap.addFileGroupOwners(path, matcher.getGroupOwners());
ownersMap.addFileReviewers(path, matcher.getReviewers());
+ matchingFound = true;
}
}
+ return matchingFound;
}
private PathOwnersEntry resolvePathEntry(
String path,
String branch,
PathOwnersEntry projectEntry,
+ List<PathOwnersEntry> parentsPathOwnersEntries,
PathOwnersEntry rootEntry,
Map<String, PathOwnersEntry> entries)
throws IOException {
@@ -218,18 +320,12 @@
PathOwnersEntry currentEntry = rootEntry;
StringBuilder builder = new StringBuilder();
- if (rootEntry.isInherited()) {
- for (Matcher matcher : projectEntry.getMatchers().values()) {
- if (!currentEntry.hasMatcher(matcher.getPath())) {
- currentEntry.addMatcher(matcher);
- }
- }
- if (currentEntry.getOwners().isEmpty()) {
- currentEntry.setOwners(projectEntry.getOwners());
- }
- if (currentEntry.getOwnersPath() == null) {
- currentEntry.setOwnersPath(projectEntry.getOwnersPath());
- }
+ // Inherit from Project if OWNER in root enables inheritance
+ calculateCurrentEntry(rootEntry, projectEntry, currentEntry);
+
+ // Inherit from Parent Project if OWNER in Project enables inheritance
+ for (PathOwnersEntry parentPathOwnersEntry : parentsPathOwnersEntries) {
+ calculateCurrentEntry(projectEntry, parentPathOwnersEntry, currentEntry);
}
// Iterate through the parent paths, not including the file name
@@ -244,15 +340,22 @@
currentEntry = entries.get(partial);
} else {
String ownersPath = partial + "OWNERS";
- Optional<OwnersConfig> conf = getOwnersConfig(ownersPath, branch);
+ Optional<OwnersConfig> conf = getOwnersConfig(repository, ownersPath, branch);
final Set<Id> owners = currentEntry.getOwners();
final Set<Id> reviewers = currentEntry.getReviewers();
Collection<Matcher> inheritedMatchers = currentEntry.getMatchers().values();
+ Set<String> groupOwners = currentEntry.getGroupOwners();
currentEntry =
conf.map(
c ->
new PathOwnersEntry(
- ownersPath, c, accounts, owners, reviewers, inheritedMatchers))
+ ownersPath,
+ c,
+ accounts,
+ owners,
+ reviewers,
+ inheritedMatchers,
+ groupOwners))
.orElse(currentEntry);
entries.put(partial, currentEntry);
}
@@ -260,6 +363,23 @@
return currentEntry;
}
+ private void calculateCurrentEntry(
+ PathOwnersEntry rootEntry, PathOwnersEntry projectEntry, PathOwnersEntry currentEntry) {
+ if (rootEntry.isInherited()) {
+ for (Matcher matcher : projectEntry.getMatchers().values()) {
+ if (!currentEntry.hasMatcher(matcher.getPath())) {
+ currentEntry.addMatcher(matcher);
+ }
+ }
+ if (currentEntry.getOwners().isEmpty()) {
+ currentEntry.setOwners(projectEntry.getOwners());
+ }
+ if (currentEntry.getOwnersPath() == null) {
+ currentEntry.setOwnersPath(projectEntry.getOwnersPath());
+ }
+ }
+ }
+
/**
* Parses the diff list for any paths that were modified.
*
@@ -290,9 +410,8 @@
* @return config or null if it doesn't exist
* @throws IOException
*/
- private Optional<OwnersConfig> getOwnersConfig(String ownersPath, String branch)
+ private Optional<OwnersConfig> getOwnersConfig(Repository repo, String ownersPath, String branch)
throws IOException {
- return getBlobAsBytes(repository, branch, ownersPath)
- .flatMap(bytes -> parser.getOwnersConfig(bytes));
+ return getBlobAsBytes(repo, branch, ownersPath).flatMap(parser::getOwnersConfig);
}
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
index 6c498d6..168d8bd 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
@@ -16,6 +16,7 @@
package com.googlesource.gerrit.owners.common;
+import com.google.common.base.Splitter;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.gerrit.entities.Account;
@@ -35,6 +36,7 @@
private Set<Account.Id> reviewers = Sets.newHashSet();
private String ownersPath;
private Map<String, Matcher> matchers = Maps.newHashMap();
+ private Set<String> groupOwners = Sets.newHashSet();
public PathOwnersEntry() {
inherited = true;
@@ -46,7 +48,8 @@
Accounts accounts,
Set<Account.Id> inheritedOwners,
Set<Account.Id> inheritedReviewers,
- Collection<Matcher> inheritedMatchers) {
+ Collection<Matcher> inheritedMatchers,
+ Set<String> inheritedGroupOwners) {
this.ownersPath = path;
this.owners =
config.getOwners().stream()
@@ -56,10 +59,15 @@
config.getReviewers().stream()
.flatMap(o -> accounts.find(o).stream())
.collect(Collectors.toSet());
+ this.groupOwners =
+ config.getOwners().stream()
+ .map(PathOwnersEntry::stripOwnerDomain)
+ .collect(Collectors.toSet());
this.matchers = config.getMatchers();
if (config.isInherited()) {
this.owners.addAll(inheritedOwners);
+ this.groupOwners.addAll(inheritedGroupOwners);
this.reviewers.addAll(inheritedReviewers);
for (Matcher matcher : inheritedMatchers) {
addMatcher(matcher);
@@ -92,6 +100,10 @@
return owners;
}
+ public Set<String> getGroupOwners() {
+ return groupOwners;
+ }
+
public void setOwners(Set<Account.Id> owners) {
this.owners = owners;
}
@@ -129,4 +141,8 @@
public boolean hasMatcher(String path) {
return this.matchers.containsKey(path);
}
+
+ public static String stripOwnerDomain(String owner) {
+ return Splitter.on('@').split(owner).iterator().next();
+ }
}
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
new file mode 100644
index 0000000..ef60d58
--- /dev/null
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PluginSettings.java
@@ -0,0 +1,103 @@
+// 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 com.google.common.collect.ImmutableSet;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.annotations.PluginName;
+import com.google.gerrit.server.config.PluginConfig;
+import com.google.gerrit.server.config.PluginConfigFactory;
+import com.google.gerrit.server.project.NoSuchProjectException;
+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
+public class PluginSettings {
+ private final ImmutableSet<String> disabledBranchesPatterns;
+ private final PluginConfigFactory configFactory;
+ private final String ownersPluginName;
+ private final Config globalPluginConfig;
+ private final boolean expandGroups;
+
+ @Inject
+ public PluginSettings(PluginConfigFactory configFactory, @PluginName String ownersPluginName) {
+ this.configFactory = configFactory;
+ this.ownersPluginName = ownersPluginName;
+
+ this.globalPluginConfig = configFactory.getGlobalPluginConfig(ownersPluginName);
+ disabledBranchesPatterns =
+ ImmutableSet.copyOf(globalPluginConfig.getStringList("owners", "disable", "branch"));
+
+ this.expandGroups = globalPluginConfig.getBoolean("owners", "expandGroups", true);
+ }
+
+ /**
+ * Branches that should be ignored for the OWNERS processing.
+ *
+ * @return set of branches regex
+ */
+ public ImmutableSet<String> disabledBranchPatterns() {
+ return disabledBranchesPatterns;
+ }
+
+ /**
+ * 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);
+ }
+
+ /** Returns true if the groups in the OWNERS file should be expanded in a list of account ids. */
+ public boolean expandGroups() {
+ return expandGroups;
+ }
+
+ /**
+ * Project-specific config of the owners plugin.
+ *
+ * @param projectKey project name
+ * @return project-specific plugin config
+ * @throws NoSuchProjectException if the project cannot be found
+ */
+ public PluginConfig projectSpecificConfig(Project.NameKey projectKey)
+ 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/main/java/com/googlesource/gerrit/owners/common/RegExMatcher.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/RegExMatcher.java
index 13f3636..6d1df24 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/RegExMatcher.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/RegExMatcher.java
@@ -23,8 +23,9 @@
public class RegExMatcher extends Matcher {
Pattern pattern;
- public RegExMatcher(String path, Set<Account.Id> owners, Set<Account.Id> reviewers) {
- super(path, owners, reviewers);
+ public RegExMatcher(
+ String path, Set<Account.Id> owners, Set<Account.Id> reviewers, Set<String> groupOwners) {
+ super(path, owners, reviewers, groupOwners);
pattern = Pattern.compile(path);
}
@@ -34,7 +35,7 @@
}
@Override
- protected Matcher clone(Set<Id> owners, Set<Id> reviewers) {
- return new RegExMatcher(path, owners, reviewers);
+ protected Matcher clone(Set<Id> owners, Set<Id> reviewers, Set<String> groupOwners) {
+ return new RegExMatcher(path, owners, reviewers, groupOwners);
}
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/SuffixMatcher.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/SuffixMatcher.java
index 6b56cc4..5dec4c5 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/SuffixMatcher.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/SuffixMatcher.java
@@ -20,8 +20,9 @@
import java.util.Set;
public class SuffixMatcher extends Matcher {
- public SuffixMatcher(String path, Set<Account.Id> owners, Set<Account.Id> reviewers) {
- super(path, owners, reviewers);
+ public SuffixMatcher(
+ String path, Set<Account.Id> owners, Set<Account.Id> reviewers, Set<String> groupOwners) {
+ super(path, owners, reviewers, groupOwners);
}
@Override
@@ -30,7 +31,7 @@
}
@Override
- protected Matcher clone(Set<Id> owners, Set<Id> reviewers) {
- return new SuffixMatcher(path, owners, reviewers);
+ protected Matcher clone(Set<Id> owners, Set<Id> reviewers, Set<String> groupOwners) {
+ return new SuffixMatcher(path, owners, reviewers, groupOwners);
}
}
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/ClassicConfig.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/ClassicConfig.java
index a62ee5b..a9e78b1 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/ClassicConfig.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/ClassicConfig.java
@@ -19,9 +19,13 @@
@Ignore
public class ClassicConfig extends Config {
- public static final String USER_A_EMAIL_COM = "user-a@email.com";
- public static final String USER_B_EMAIL_COM = "user-b@email.com";
- public static final String USER_C_EMAIL_COM = "user-c@email.com";
+ public static final String USER_A = "user-a";
+ public static final String USER_B = "user-b";
+ public static final String USER_C = "user-c";
+ public static final String EMAIL_DOMAIN = "@email.com";
+ public static final String USER_A_EMAIL_COM = USER_A + EMAIL_DOMAIN;
+ public static final String USER_B_EMAIL_COM = USER_B + EMAIL_DOMAIN;
+ public static final String USER_C_EMAIL_COM = USER_C + EMAIL_DOMAIN;
public static final Account.Id USER_A_ID = Account.id(1);
public static final Account.Id USER_B_ID = Account.id(2);
public static final Account.Id USER_C_ID = Account.id(3);
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/Config.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/Config.java
index ea00372..f298164 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/Config.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/Config.java
@@ -20,6 +20,7 @@
import com.google.common.base.Charsets;
import com.google.gerrit.entities.Patch;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListEntry;
import java.io.IOException;
@@ -33,16 +34,22 @@
@Ignore
public abstract class Config {
+ protected GitRepositoryManager repositoryManager;
protected Repository repository;
+ protected Repository parentRepository1;
+ protected Repository parentRepository2;
protected PatchList patchList;
protected ConfigurationParser parser;
protected TestAccounts accounts = new TestAccounts();
- protected String branch = "master";
+ protected Optional<String> branch = Optional.of("master");
public void setup() throws Exception {
PowerMock.mockStatic(JgitWrapper.class);
+ repositoryManager = PowerMock.createMock(GitRepositoryManager.class);
repository = PowerMock.createMock(Repository.class);
+ parentRepository1 = PowerMock.createMock(Repository.class);
+ parentRepository2 = PowerMock.createMock(Repository.class);
parser = new ConfigurationParser(accounts);
}
@@ -54,6 +61,18 @@
.anyTimes();
}
+ void expectConfig(String path, String branch, String config) throws IOException {
+ expect(JgitWrapper.getBlobAsBytes(anyObject(Repository.class), eq(branch), eq(path)))
+ .andReturn(Optional.of(config.getBytes()))
+ .anyTimes();
+ }
+
+ void expectConfig(String path, String branch, Repository repo, String config) throws IOException {
+ expect(JgitWrapper.getBlobAsBytes(eq(repo), eq(branch), eq(path)))
+ .andReturn(Optional.of(config.getBytes()))
+ .anyTimes();
+ }
+
void expectNoConfig(String path) throws IOException {
expect(
JgitWrapper.getBlobAsBytes(
diff --git a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/MatcherConfig.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/MatcherConfig.java
index a2b20d8..51e0b74 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/MatcherConfig.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/MatcherConfig.java
@@ -22,6 +22,7 @@
public static final String MATCH_REGEX = "regex";
public static final String MATCH_SUFFIX = "suffix";
public static final String MATCH_PARTIAL_REGEX = "partial_regex";
+ public static final String MATCH_GENERIC = "generic";
private final String matchType;
private final String matchExpr;
@@ -43,6 +44,10 @@
return new MatcherConfig(MATCH_PARTIAL_REGEX, expr, owners);
}
+ public static MatcherConfig genericMatcher(String expr, String... owners) {
+ return new MatcherConfig(MATCH_GENERIC, expr, owners);
+ }
+
public MatcherConfig(String matchType, String matchExpr, String[] owners) {
super();
this.matchType = matchType;
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 573c82e..e8b4167 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
@@ -14,14 +14,24 @@
package com.googlesource.gerrit.owners.common;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
+import static com.googlesource.gerrit.owners.common.MatcherConfig.suffixMatcher;
+import static java.util.Collections.EMPTY_LIST;
+import static org.easymock.EasyMock.eq;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.junit.Assert.*;
import static org.powermock.api.easymock.PowerMock.replayAll;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import java.io.IOException;
import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
import java.util.Optional;
import java.util.Set;
+import org.eclipse.jgit.lib.Repository;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -34,6 +44,15 @@
@PrepareForTest(JgitWrapper.class)
public class PathOwnersTest extends ClassicConfig {
+ private static final String CLASSIC_OWNERS = "classic/OWNERS";
+ private static final boolean EXPAND_GROUPS = true;
+ private static final boolean DO_NOT_EXPAND_GROUPS = false;
+ public static final String CLASSIC_FILE_TXT = "classic/file.txt";
+ public static final Project.NameKey parentRepository1NameKey =
+ Project.NameKey.parse("parentRepository1");
+ public static final Project.NameKey parentRepository2NameKey =
+ Project.NameKey.parse("parentRepository2");
+
@Override
@Before
public void setup() throws Exception {
@@ -42,29 +61,73 @@
@Test
public void testClassic() throws Exception {
- expectNoConfig("OWNERS");
- expectConfig("classic/OWNERS", createConfig(false, owners(USER_A_EMAIL_COM, USER_B_EMAIL_COM)));
+ mockOwners(USER_A_EMAIL_COM, USER_B_EMAIL_COM);
- creatingPatchList(Arrays.asList("classic/file.txt"));
- replayAll();
-
- PathOwners owners = new PathOwners(accounts, repository, branch, patchList);
- Set<Account.Id> ownersSet = owners.get().get("classic/OWNERS");
+ PathOwners owners =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ Collections.EMPTY_LIST,
+ branch,
+ patchList,
+ EXPAND_GROUPS);
+ 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());
+ }
+
+ @Test
+ public void testFileBasedOwnersUnexpanded() throws Exception {
+ mockOwners(USER_A_EMAIL_COM, USER_B_EMAIL_COM);
+
+ PathOwners owners =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ EMPTY_LIST,
+ branch,
+ patchList,
+ DO_NOT_EXPAND_GROUPS);
+ Set<String> ownersSet = owners.getFileGroupOwners().get(CLASSIC_FILE_TXT);
+ assertEquals(2, ownersSet.size());
+ assertTrue(ownersSet.contains(USER_A));
+ assertTrue(ownersSet.contains(USER_B));
+ assertFalse(owners.expandGroups());
+ }
+
+ @Test
+ public void testDisabledBranch() throws Exception {
+ mockOwners(USER_A_EMAIL_COM);
+
+ PathOwners owners =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ EMPTY_LIST,
+ Optional.empty(),
+ patchList,
+ EXPAND_GROUPS);
+ Set<Account.Id> ownersSet = owners.get().get(CLASSIC_OWNERS);
+ assertEquals(0, ownersSet.size());
}
@Test
public void testClassicWithInheritance() throws Exception {
expectConfig("OWNERS", createConfig(true, owners(USER_C_EMAIL_COM)));
- expectConfig("classic/OWNERS", createConfig(true, owners(USER_A_EMAIL_COM, USER_B_EMAIL_COM)));
+ expectConfig(CLASSIC_OWNERS, createConfig(true, owners(USER_A_EMAIL_COM, USER_B_EMAIL_COM)));
creatingPatchList(Arrays.asList("classic/file.txt"));
replayAll();
- PathOwners owners2 = new PathOwners(accounts, repository, branch, patchList);
- Set<Account.Id> ownersSet2 = owners2.get().get("classic/OWNERS");
+ PathOwners owners2 =
+ new PathOwners(
+ accounts, repositoryManager, repository, EMPTY_LIST, branch, patchList, EXPAND_GROUPS);
+ Set<Account.Id> ownersSet2 = owners2.get().get(CLASSIC_OWNERS);
// in this case we are inheriting the acct3 from /OWNERS
assertEquals(3, ownersSet2.size());
@@ -74,6 +137,118 @@
}
@Test
+ public void testRootInheritFromProject() throws Exception {
+ expectConfig("OWNERS", "master", createConfig(true, owners()));
+ expectConfig(
+ "OWNERS",
+ RefNames.REFS_CONFIG,
+ createConfig(true, owners(), suffixMatcher(".sql", USER_A_EMAIL_COM, USER_B_EMAIL_COM)));
+
+ String fileName = "file.sql";
+ creatingPatchList(Collections.singletonList(fileName));
+ replayAll();
+
+ PathOwners owners =
+ new PathOwners(
+ accounts, repositoryManager, repository, EMPTY_LIST, branch, patchList, EXPAND_GROUPS);
+
+ Map<String, Set<Account.Id>> fileOwners = owners.getFileOwners();
+ assertEquals(1, fileOwners.size());
+
+ Set<Account.Id> ownersSet = fileOwners.get(fileName);
+ assertEquals(2, ownersSet.size());
+ assertTrue(ownersSet.contains(USER_A_ID));
+ assertTrue(ownersSet.contains(USER_B_ID));
+ }
+
+ @Test
+ public void testProjectInheritFromParentProject() throws Exception {
+ expectConfig("OWNERS", "master", createConfig(true, owners()));
+ expectConfig("OWNERS", RefNames.REFS_CONFIG, repository, createConfig(true, owners()));
+ expectConfig(
+ "OWNERS",
+ RefNames.REFS_CONFIG,
+ parentRepository1,
+ createConfig(true, owners(), suffixMatcher(".sql", USER_A_EMAIL_COM, USER_B_EMAIL_COM)));
+
+ String fileName = "file.sql";
+ creatingPatchList(Collections.singletonList(fileName));
+
+ mockParentRepository(parentRepository1NameKey, parentRepository1);
+ replayAll();
+
+ PathOwners owners =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ Arrays.asList(parentRepository1NameKey),
+ branch,
+ patchList,
+ EXPAND_GROUPS);
+
+ Map<String, Set<Account.Id>> fileOwners = owners.getFileOwners();
+ assertEquals(fileOwners.size(), 1);
+
+ Set<Account.Id> ownersSet = fileOwners.get(fileName);
+ assertEquals(2, ownersSet.size());
+ assertTrue(ownersSet.contains(USER_A_ID));
+ assertTrue(ownersSet.contains(USER_B_ID));
+ }
+
+ @Test
+ public void testProjectInheritFromMultipleParentProjects() throws Exception {
+ expectConfig("OWNERS", "master", createConfig(true, owners()));
+ expectConfig("OWNERS", RefNames.REFS_CONFIG, repository, createConfig(true, owners()));
+ expectConfig(
+ "OWNERS",
+ RefNames.REFS_CONFIG,
+ parentRepository1,
+ createConfig(true, owners(), suffixMatcher(".sql", USER_A_EMAIL_COM)));
+ expectConfig(
+ "OWNERS",
+ RefNames.REFS_CONFIG,
+ parentRepository2,
+ createConfig(true, owners(), suffixMatcher(".java", USER_B_EMAIL_COM)));
+
+ String sqlFileName = "file.sql";
+ String javaFileName = "file.java";
+ creatingPatchList(Arrays.asList(sqlFileName, javaFileName));
+
+ mockParentRepository(parentRepository1NameKey, parentRepository1);
+ mockParentRepository(parentRepository2NameKey, parentRepository2);
+ replayAll();
+
+ PathOwners owners =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ Arrays.asList(parentRepository1NameKey, parentRepository2NameKey),
+ branch,
+ patchList,
+ EXPAND_GROUPS);
+
+ Map<String, Set<Account.Id>> fileOwners = owners.getFileOwners();
+ assertEquals(fileOwners.size(), 2);
+
+ Set<Account.Id> ownersSet1 = fileOwners.get(sqlFileName);
+ assertEquals(1, ownersSet1.size());
+ assertTrue(ownersSet1.contains(USER_A_ID));
+
+ Set<Account.Id> ownersSet2 = fileOwners.get(javaFileName);
+ assertEquals(1, ownersSet2.size());
+ assertTrue(ownersSet2.contains(USER_B_ID));
+ }
+
+ private void mockParentRepository(Project.NameKey repositoryName, Repository repository)
+ throws IOException {
+ expect(repositoryManager.openRepository(eq(repositoryName))).andReturn(repository).anyTimes();
+ repository.close();
+ expectLastCall();
+ }
+
+ @Test
public void testClassicWithInheritanceAndDeepNesting() throws Exception {
expectConfig("OWNERS", createConfig(true, owners(USER_C_EMAIL_COM)));
expectConfig("dir/OWNERS", createConfig(true, owners(USER_B_EMAIL_COM)));
@@ -82,7 +257,9 @@
creatingPatchList(Arrays.asList("dir/subdir/file.txt"));
replayAll();
- PathOwners owners = new PathOwners(accounts, repository, branch, patchList);
+ PathOwners owners =
+ new PathOwners(
+ accounts, repositoryManager, repository, EMPTY_LIST, branch, patchList, EXPAND_GROUPS);
Set<Account.Id> ownersSet = owners.get().get("dir/subdir/OWNERS");
assertEquals(3, ownersSet.size());
@@ -100,4 +277,12 @@
assertEquals(1, config.get().getOwners().size());
assertTrue(config.get().getOwners().contains(USER_C_EMAIL_COM));
}
+
+ private void mockOwners(String... owners) throws IOException {
+ expectNoConfig("OWNERS");
+ expectConfig(CLASSIC_OWNERS, createConfig(false, owners(owners)));
+
+ creatingPatchList(Arrays.asList(CLASSIC_FILE_TXT));
+ replayAll();
+ }
}
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-common/src/test/java/com/googlesource/gerrit/owners/common/RegexMatcherTest.java b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexMatcherTest.java
index c61827e..67c4ab6 100644
--- a/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexMatcherTest.java
+++ b/owners-common/src/test/java/com/googlesource/gerrit/owners/common/RegexMatcherTest.java
@@ -23,18 +23,18 @@
public class RegexMatcherTest {
@Test
public void testRegex() {
- RegExMatcher matcher = new RegExMatcher(".*/a.*", null, null);
+ RegExMatcher matcher = new RegExMatcher(".*/a.*", null, null, null);
assertTrue(matcher.matches("xxxxxx/axxxx"));
assertFalse(matcher.matches("axxxx"));
assertFalse(matcher.matches("xxxxx/bxxxx"));
- RegExMatcher matcher2 = new RegExMatcher("a.*.sql", null, null);
+ RegExMatcher matcher2 = new RegExMatcher("a.*.sql", null, null, null);
assertFalse(matcher2.matches("xxxxxx/alfa.sql"));
}
@Test
public void testFloatingRegex() {
- PartialRegExMatcher matcher = new PartialRegExMatcher("a.*.sql", null, null);
+ PartialRegExMatcher matcher = new PartialRegExMatcher("a.*.sql", null, null, null);
assertTrue(matcher.matches("xxxxxxx/alfa.sql"));
assertTrue(matcher.matches("alfa.sqlxxxxx"));
assertFalse(matcher.matches("alfa.bar"));
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 e911f1b..8fea7ed 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
@@ -17,10 +17,12 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static com.googlesource.gerrit.owners.common.MatcherConfig.exactMatcher;
+import static com.googlesource.gerrit.owners.common.MatcherConfig.genericMatcher;
import static com.googlesource.gerrit.owners.common.MatcherConfig.partialRegexMatcher;
import static com.googlesource.gerrit.owners.common.MatcherConfig.regexMatcher;
import static com.googlesource.gerrit.owners.common.MatcherConfig.suffixMatcher;
import static com.googlesource.gerrit.owners.common.StreamUtils.iteratorStream;
+import static java.util.Collections.EMPTY_LIST;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.powermock.api.easymock.PowerMock.replayAll;
@@ -57,6 +59,7 @@
private static final Account.Id ACCOUNT_D_ID = Account.id(4);
private static final Account.Id ACCOUNT_E_ID = Account.id(5);
private static final Account.Id ACCOUNT_F_ID = Account.id(6);
+ private static final boolean EXPAND_GROUPS = true;
@Override
@Before
@@ -150,7 +153,9 @@
replayAll();
// function under test
- PathOwners owners = new PathOwners(accounts, repository, branch, patchList);
+ PathOwners owners =
+ new PathOwners(
+ accounts, repositoryManager, repository, EMPTY_LIST, branch, patchList, EXPAND_GROUPS);
// assertions on classic owners
Set<Account.Id> ownersSet = owners.get().get("project/OWNERS");
@@ -228,7 +233,12 @@
replayAll();
Optional<OwnersConfig> ownersConfigOpt =
- getOwnersConfig(createConfig(false, new String[0], suffixMatcher(".txt", ACCOUNT_B)));
+ getOwnersConfig(
+ createConfig(
+ false,
+ new String[0],
+ suffixMatcher(".txt", ACCOUNT_B),
+ genericMatcher(".*", ACCOUNT_A)));
assertThat(ownersConfigOpt).isPresent();
OwnersConfig ownersConfig = ownersConfigOpt.get();
@@ -246,7 +256,9 @@
creatingPatch("project/file.sql", "another.txt");
replayAll();
- PathOwners owners = new PathOwners(accounts, repository, branch, patchList);
+ PathOwners owners =
+ new PathOwners(
+ accounts, repositoryManager, repository, EMPTY_LIST, branch, patchList, EXPAND_GROUPS);
Set<String> ownedFiles = owners.getFileOwners().keySet();
assertThat(ownedFiles).containsExactly("project/file.sql");
diff --git a/owners/BUILD b/owners/BUILD
index ab6705e..d04d83d 100644
--- a/owners/BUILD
+++ b/owners/BUILD
@@ -1,6 +1,7 @@
-load("//tools/bzl:plugin.bzl", "PLUGIN_DEPS_NEVERLINK", "PLUGIN_TEST_DEPS", "gerrit_plugin")
+load("//tools/bzl:plugin.bzl", "PLUGIN_DEPS", "PLUGIN_DEPS_NEVERLINK", "PLUGIN_TEST_DEPS", "gerrit_plugin")
load("//lib/prolog:prolog.bzl", "prolog_cafe_library")
load("//owners-common:common.bzl", "EXTERNAL_DEPS", "EXTERNAL_TEST_DEPS")
+load("//tools/bzl:junit.bzl", "junit_tests")
PROLOG_PREDICATES = glob(["src/main/java/gerrit_owners/**/*.java"]) + [
"src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java",
@@ -49,5 +50,14 @@
name = "owners__plugin_test_deps",
testonly = 1,
visibility = ["//visibility:public"],
- exports = EXTERNAL_DEPS + EXTERNAL_TEST_DEPS + PLUGIN_TEST_DEPS,
+ exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
+ ":owners",
+ ],
+)
+
+junit_tests(
+ name = "owners_tests",
+ srcs = glob(["src/test/java/**/*.java"]),
+ tags = ["owners"],
+ deps = ["owners__plugin_test_deps"],
)
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 ef0b92b..7b1399a 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnerPredicateProvider.java
@@ -21,13 +21,14 @@
import com.google.gerrit.server.rules.PredicateProvider;
import com.google.inject.Inject;
import com.googlesource.gerrit.owners.common.Accounts;
+import com.googlesource.gerrit.owners.common.PluginSettings;
/** Gerrit OWNERS Prolog Predicate Provider. */
@Listen
public class OwnerPredicateProvider implements PredicateProvider {
@Inject
- public OwnerPredicateProvider(Accounts accounts) {
- OwnersStoredValues.initialize(accounts);
+ public OwnerPredicateProvider(Accounts accounts, PluginSettings config) {
+ OwnersStoredValues.initialize(accounts, config);
}
@Override
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java
index 750c58e..e3a915b 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersModule.java
@@ -25,5 +25,6 @@
DynamicSet.bind(binder(), PredicateProvider.class)
.to(OwnerPredicateProvider.class)
.asEagerSingleton();
+ install(new OwnersRestApiModule());
}
}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersRestApiModule.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersRestApiModule.java
new file mode 100644
index 0000000..ccd3fcb
--- /dev/null
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersRestApiModule.java
@@ -0,0 +1,27 @@
+// 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;
+
+import com.google.gerrit.extensions.restapi.RestApiModule;
+import com.google.gerrit.server.change.RevisionResource;
+import com.googlesource.gerrit.owners.restapi.GetFilesOwners;
+
+public class OwnersRestApiModule extends RestApiModule {
+ @Override
+ protected void configure() {
+ get(RevisionResource.REVISION_KIND, "files-owners").to(GetFilesOwners.class);
+ }
+}
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 749056c..562d7cb 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,21 @@
package com.googlesource.gerrit.owners;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.filediff.FileDiffOutput;
+import com.google.gerrit.server.project.ProjectState;
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.Arrays;
+import java.util.Collections;
+import java.util.List;
import java.util.Map;
+import java.util.Optional;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -33,7 +41,7 @@
public static StoredValue<PathOwners> PATH_OWNERS;
- public static synchronized void initialize(Accounts accounts) {
+ public static synchronized void initialize(Accounts accounts, PluginSettings settings) {
if (PATH_OWNERS != null) {
return;
}
@@ -44,8 +52,23 @@
protected PathOwners createValue(Prolog engine) {
Map<String, FileDiffOutput> patchList = StoredValues.DIFF_LIST.get(engine);
Repository repository = StoredValues.REPOSITORY.get(engine);
+ ProjectState projectState = StoredValues.PROJECT_STATE.get(engine);
+ GitRepositoryManager gitRepositoryManager = StoredValues.REPO_MANAGER.get(engine);
+
+ List<Project.NameKey> maybeParentProjectNameKey =
+ Optional.ofNullable(projectState.getProject().getParent())
+ .map(Arrays::asList)
+ .orElse(Collections.emptyList());
+
String branch = StoredValues.getChange(engine).getDest().branch();
- return new PathOwners(accounts, repository, branch, patchList);
+ return new PathOwners(
+ accounts,
+ gitRepositoryManager,
+ repository,
+ maybeParentProjectNameKey,
+ settings.isBranchDisabled(branch) ? Optional.empty() : Optional.of(branch),
+ patchList,
+ settings.expandGroups());
}
};
}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java b/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java
new file mode 100644
index 0000000..30c9c73
--- /dev/null
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/entities/FilesOwnersResponse.java
@@ -0,0 +1,51 @@
+// 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.entities;
+
+import com.google.common.base.Objects;
+import java.util.Map;
+import java.util.Set;
+
+/* Files to Owners response API representation */
+public class FilesOwnersResponse {
+
+ public final Map<String, Set<GroupOwner>> files;
+ public final Map<Integer, Map<String, Integer>> ownersLabels;
+
+ public FilesOwnersResponse(
+ Map<Integer, Map<String, Integer>> ownersLabels, Map<String, Set<GroupOwner>> files) {
+ this.ownersLabels = ownersLabels;
+ this.files = files;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ FilesOwnersResponse that = (FilesOwnersResponse) o;
+ return Objects.equal(files, that.files) && Objects.equal(ownersLabels, that.ownersLabels);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(files, ownersLabels);
+ }
+
+ @Override
+ public String toString() {
+ return "FilesOwnersResponse{" + "files=" + files + ", ownersLabels=" + ownersLabels + '}';
+ }
+}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/entities/GroupOwner.java b/owners/src/main/java/com/googlesource/gerrit/owners/entities/GroupOwner.java
new file mode 100644
index 0000000..7624ad0
--- /dev/null
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/entities/GroupOwner.java
@@ -0,0 +1,54 @@
+// 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.entities;
+
+import com.google.common.base.Objects;
+
+/** Class representing a file group owner * */
+public class GroupOwner {
+ private final String name;
+
+ public GroupOwner(String name) {
+ this.name = name;
+ }
+
+ /**
+ * Get the {@link GroupOwner} name.
+ *
+ * @return the group owner name.
+ */
+ public String getName() {
+ return name;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ GroupOwner owner = (GroupOwner) o;
+ return Objects.equal(name, owner.name);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(name);
+ }
+
+ @Override
+ public String toString() {
+ return "GroupOwner{name='" + name + "'}";
+ }
+}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/entities/Owner.java b/owners/src/main/java/com/googlesource/gerrit/owners/entities/Owner.java
new file mode 100644
index 0000000..8647344
--- /dev/null
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/entities/Owner.java
@@ -0,0 +1,56 @@
+// 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.entities;
+
+import com.google.common.base.Objects;
+import com.google.gerrit.entities.Account;
+
+/** Class representing a file Owner * */
+public class Owner extends GroupOwner {
+ private final int id;
+
+ public Owner(String name, int id) {
+ super(name);
+ this.id = id;
+ }
+
+ /**
+ * Get the {@link Owner} account id.
+ *
+ * @return an {@code int} representation of the Owner {@link Account.Id}.
+ */
+ public int getId() {
+ return id;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ Owner owner = (Owner) o;
+ return Objects.equal(id, owner.id) && Objects.equal(getName(), owner.getName());
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hashCode(getName(), id);
+ }
+
+ @Override
+ public String toString() {
+ return "Owner{id=" + id + '}';
+ }
+}
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
new file mode 100644
index 0000000..475f1cd
--- /dev/null
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/restapi/GetFilesOwners.java
@@ -0,0 +1,180 @@
+// 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.restapi;
+
+import com.google.common.base.Predicates;
+import com.google.common.collect.Maps;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.PatchSet;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.client.ListChangesOption;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
+import com.google.gerrit.extensions.restapi.Response;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.gerrit.extensions.restapi.RestReadView;
+import com.google.gerrit.server.account.AccountCache;
+import com.google.gerrit.server.change.RevisionResource;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+import com.googlesource.gerrit.owners.common.Accounts;
+import com.googlesource.gerrit.owners.common.PathOwners;
+import com.googlesource.gerrit.owners.common.PluginSettings;
+import com.googlesource.gerrit.owners.entities.FilesOwnersResponse;
+import com.googlesource.gerrit.owners.entities.GroupOwner;
+import com.googlesource.gerrit.owners.entities.Owner;
+import java.util.*;
+import java.util.stream.Collectors;
+import org.eclipse.jgit.lib.Repository;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@Singleton
+public class GetFilesOwners implements RestReadView<RevisionResource> {
+ private static final Logger log = LoggerFactory.getLogger(GetFilesOwners.class);
+
+ private final PatchListCache patchListCache;
+ private final Accounts accounts;
+ private final AccountCache accountCache;
+ private final ProjectCache projectCache;
+ private final GitRepositoryManager repositoryManager;
+ private final PluginSettings pluginSettings;
+ private final GerritApi gerritApi;
+ private final ChangeData.Factory changeDataFactory;
+
+ @Inject
+ GetFilesOwners(
+ PatchListCache patchListCache,
+ Accounts accounts,
+ AccountCache accountCache,
+ ProjectCache projectCache,
+ GitRepositoryManager repositoryManager,
+ PluginSettings pluginSettings,
+ GerritApi gerritApi,
+ ChangeData.Factory changeDataFactory) {
+ this.patchListCache = patchListCache;
+ this.accounts = accounts;
+ this.accountCache = accountCache;
+ this.projectCache = projectCache;
+ this.repositoryManager = repositoryManager;
+ this.pluginSettings = pluginSettings;
+ this.gerritApi = gerritApi;
+ this.changeDataFactory = changeDataFactory;
+ }
+
+ @Override
+ public Response<FilesOwnersResponse> apply(RevisionResource revision)
+ throws AuthException, BadRequestException, ResourceConflictException, Exception {
+ PatchSet ps = revision.getPatchSet();
+ Change change = revision.getChange();
+ int id = revision.getChangeResource().getChange().getChangeId();
+
+ List<Project.NameKey> maybeParentProjectNameKey =
+ projectCache
+ .get(change.getProject())
+ .map(p -> Arrays.asList(p.getProject().getParent()))
+ .filter(Predicates.notNull())
+ .orElse(Collections.emptyList());
+
+ try (Repository repository = repositoryManager.openRepository(change.getProject())) {
+ Set<String> changePaths = new HashSet<>(changeDataFactory.create(change).currentFilePaths());
+
+ String branch = change.getDest().branch();
+ PathOwners owners =
+ new PathOwners(
+ accounts,
+ repositoryManager,
+ repository,
+ maybeParentProjectNameKey,
+ pluginSettings.isBranchDisabled(branch) ? Optional.empty() : Optional.of(branch),
+ changePaths,
+ pluginSettings.expandGroups());
+
+ Map<String, Set<GroupOwner>> fileToOwners =
+ pluginSettings.expandGroups()
+ ? Maps.transformValues(
+ owners.getFileOwners(),
+ ids ->
+ ids.stream()
+ .map(this::getOwnerFromAccountId)
+ .flatMap(Optional::stream)
+ .collect(Collectors.toSet()))
+ : Maps.transformValues(
+ owners.getFileGroupOwners(),
+ groupNames ->
+ groupNames.stream().map(GroupOwner::new).collect(Collectors.toSet()));
+
+ return Response.ok(new FilesOwnersResponse(getLabels(id), fileToOwners));
+ }
+ }
+
+ /**
+ * This method returns ta Map representing the "owners_labels" object of the response. When
+ * serialized the Map, has to to return the following JSON: the following JSON:
+ *
+ * <pre>
+ * {
+ * "1000001" : {
+ * "Code-Review" : 1,
+ * "Verified" : 0
+ * },
+ * "1000003" : {
+ * "Code-Review" : 2,
+ * "Verified" : 1
+ * }
+ * }
+ *
+ * </pre>
+ */
+ private Map<Integer, Map<String, Integer>> getLabels(int id) throws RestApiException {
+ ChangeInfo changeInfo =
+ gerritApi.changes().id(id).get(EnumSet.of(ListChangesOption.DETAILED_LABELS));
+
+ Map<Integer, Map<String, Integer>> ownerToLabels = new HashMap<>();
+
+ changeInfo.labels.forEach(
+ (label, labelInfo) -> {
+ Optional.ofNullable(labelInfo.all)
+ .ifPresent(
+ approvalInfos -> {
+ approvalInfos.forEach(
+ approvalInfo -> {
+ int currentOwnerId = approvalInfo._accountId;
+ Map<String, Integer> currentOwnerLabels =
+ ownerToLabels.getOrDefault(currentOwnerId, new HashMap<>());
+ currentOwnerLabels.put(label, approvalInfo.value);
+ ownerToLabels.put(currentOwnerId, currentOwnerLabels);
+ });
+ });
+ });
+
+ return ownerToLabels;
+ }
+
+ private Optional<Owner> getOwnerFromAccountId(Account.Id accountId) {
+ return accountCache
+ .get(accountId)
+ .map(as -> new Owner(as.account().fullName(), as.account().id().get()));
+ }
+}
diff --git a/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java b/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java
index fd87eb3..cbb23cf 100644
--- a/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java
+++ b/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java
@@ -83,9 +83,12 @@
String path = key.toString();
PathOwners owners = OwnersStoredValues.PATH_OWNERS.get(engine);
Set<String> ownersNames =
- iteratorStream(owners.getFileOwners().get(path).iterator())
- .map(id -> getFullNameFromId(engine, id))
- .collect(Collectors.toSet());
+ owners.expandGroups()
+ ? iteratorStream(owners.getFileOwners().get(path).iterator())
+ .map(id -> getFullNameFromId(engine, id))
+ .collect(Collectors.toSet())
+ : iteratorStream((owners.getFileGroupOwners().get(path)).iterator())
+ .collect(Collectors.toSet());
String ownVerb = ownersNames.size() > 1 ? "-own-" : "-owns-";
String userNames = ownersNames.stream().collect(Collectors.joining("-"));
return SymbolTerm.create(userNames + ownVerb + sanitizeAsSubmitLabel(new File(path).getName()));
diff --git a/owners/src/main/resources/Documentation/config.md b/owners/src/main/resources/Documentation/config.md
index b294ae8..92b7d5e 100644
--- a/owners/src/main/resources/Documentation/config.md
+++ b/owners/src/main/resources/Documentation/config.md
@@ -1,3 +1,30 @@
+## Global configuration
+
+The global plugin configuration is read from the `$GERRIT_SITE/etc/owners.config`
+and is applied across all projects in Gerrit.
+
+owners.disable.branch
+: List of branches regex where the resolution of owners is disabled.
+
+Example:
+
+```
+[owners "disable"]
+ branch = refs/meta/config
+ branch = refs/heads/sandboxes.*
+```
+
+owners.expandGroups
+: Expand owners and groups into account ids. If set to `false` all owners are left untouched, apart from e-mail
+ addresses which have the domain dropped. Defaults to `true`.
+
+Example:
+
+```
+[owners]
+ expandGroups = false
+```
+
## Configuration
Owner approval is determined based on OWNERS files located in the same
@@ -42,6 +69,11 @@
(partial or full) and exact string comparison. For exact match, path is
relative to the root of the repo.
+> **NOTE:** The `generic` matcher is a special type of regex matching that
+> is applied only when none of the other sections are matching. It is
+> used to define fallback rules. The `generic: .*` is the top-level fallback
+> and can be used with other more specific `generic` matchers.
+
The plugin analyzes the latest patch set by looking at each file directory and
building an OWNERS hierarchy. It stops once it finds an OWNERS file that has
“inherited” set to false (by default it’s true.)
@@ -84,6 +116,8 @@
That means that in the absence of any OWNERS file in the target branch, the refs/meta/config
OWNERS is used as global default.
+If the global project OWNERS has the 'inherited: true', it will check for a global project OWNERS
+in all parent projects up to All-Projects.
## Example 1 - OWNERS file without matchers and default Gerrit submit rules
diff --git a/owners/src/main/resources/Documentation/rest-api.md b/owners/src/main/resources/Documentation/rest-api.md
new file mode 100644
index 0000000..4fb4285
--- /dev/null
+++ b/owners/src/main/resources/Documentation/rest-api.md
@@ -0,0 +1,32 @@
+# Rest API
+
+The @PLUGIN@ exposes a Rest API endpoint to list the owners associated to each file and, for each owner,
+its associated labels and votes:
+
+```bash
+GET /changes/{change-id}/revisions/{revision-id}/owners~files-owners
+
+{
+ "files": {
+ "AJavaFile.java":[
+ { "name":"John", "id": 1000002 },
+ { "name":"Bob", "id": 1000001 }
+ ],
+ "Aptyhonfileroot.py":[
+ { "name":"John", "id": 1000002 },
+ { "name":"Bob", "id": 1000001 },
+ { "name":"Jack", "id": 1000003 }
+ ]
+ },
+ "owners_labels" : {
+ "1000002": {
+ "Verified": 1,
+ "Code-Review": 0
+ },
+ "1000001": {
+ "Code-Review": -2
+ }
+ }
+}
+
+```
\ No newline at end of file
diff --git a/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
new file mode 100644
index 0000000..cd23d0d
--- /dev/null
+++ b/owners/src/test/java/com/googlesource/gerrit/owners/restapi/GetFilesOwnersIT.java
@@ -0,0 +1,210 @@
+// 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.restapi;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.acceptance.config.GlobalPluginConfig;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.restapi.Response;
+import com.googlesource.gerrit.owners.entities.FilesOwnersResponse;
+import com.googlesource.gerrit.owners.entities.GroupOwner;
+import com.googlesource.gerrit.owners.entities.Owner;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.commons.compress.utils.Sets;
+import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
+import org.eclipse.jgit.junit.TestRepository;
+import org.junit.Test;
+
+@TestPlugin(name = "owners", httpModule = "com.googlesource.gerrit.owners.OwnersRestApiModule")
+@UseLocalDisk
+public class GetFilesOwnersIT extends LightweightPluginDaemonTest {
+
+ private GetFilesOwners ownersApi;
+ private Owner rootOwner;
+ private Owner projectOwner;
+
+ @Override
+ public void setUpTestPlugin() throws Exception {
+ super.setUpTestPlugin();
+
+ rootOwner = new Owner(admin.fullName(), admin.id().get());
+ projectOwner = new Owner(user.fullName(), user.id().get());
+ ownersApi = plugin.getSysInjector().getInstance(GetFilesOwners.class);
+ }
+
+ @Test
+ public void shouldReturnExactFileOwners() throws Exception {
+ addOwnerFileToRoot(true);
+ String changeId = createChange().getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(rootOwner));
+ }
+
+ @Test
+ public void shouldReturnOwnersLabels() throws Exception {
+ addOwnerFileToRoot(true);
+ String changeId = createChange().getChangeId();
+ approve(changeId);
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().ownersLabels)
+ .containsExactly(admin.id().get(), ImmutableMap.builder().put("Code-Review", 2).build());
+ }
+
+ @Test
+ @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false")
+ public void shouldReturnResponseWithUnexpandedFileOwners() throws Exception {
+ addOwnerFileToRoot(true);
+ String changeId = createChange().getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username())));
+ }
+
+ @Test
+ @GlobalPluginConfig(pluginName = "owners", name = "owners.expandGroups", value = "false")
+ public void shouldReturnResponseWithUnexpandedFileMatchersOwners() throws Exception {
+ addOwnerFileWithMatchersToRoot(true);
+ String changeId = createChange().getChangeId();
+
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(new GroupOwner(admin.username())));
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldReturnInheritedOwnersFromProjectsOwners() throws Exception {
+ assertInheritFromProject(project);
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldReturnInheritedOwnersFromParentProjectsOwners() throws Exception {
+ assertInheritFromProject(allProjects);
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldNotReturnInheritedOwnersFromProjectsOwners() throws Exception {
+ assertNotInheritFromProject(project);
+ }
+
+ @Test
+ @UseLocalDisk
+ public void shouldNotReturnInheritedOwnersFromParentProjectsOwners() throws Exception {
+ addOwnerFileToProjectConfig(project, false);
+ assertNotInheritFromProject(allProjects);
+ }
+
+ private static <T> Response<T> assertResponseOk(Response<T> response) {
+ assertThat(response.statusCode()).isEqualTo(HttpServletResponse.SC_OK);
+ return response;
+ }
+
+ private void assertNotInheritFromProject(Project.NameKey projectNameKey) throws Exception {
+ addOwnerFileToRoot(false);
+ addOwnerFileToProjectConfig(projectNameKey, true);
+
+ String changeId = createChange().getChangeId();
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files).containsExactly("a.txt", Sets.newHashSet(rootOwner));
+ }
+
+ private void assertInheritFromProject(Project.NameKey projectNameKey) throws Exception {
+ addOwnerFileToRoot(true);
+ addOwnerFileToProjectConfig(projectNameKey, true);
+
+ String changeId = createChange().getChangeId();
+ Response<FilesOwnersResponse> resp =
+ assertResponseOk(ownersApi.apply(parseCurrentRevisionResource(changeId)));
+
+ assertThat(resp.value().files)
+ .containsExactly("a.txt", Sets.newHashSet(rootOwner, projectOwner));
+ }
+
+ private void addOwnerFileToProjectConfig(Project.NameKey projectNameKey, boolean inherit)
+ throws Exception {
+ TestRepository<InMemoryRepository> project = cloneProject(projectNameKey);
+ GitUtil.fetch(project, RefNames.REFS_CONFIG + ":" + RefNames.REFS_CONFIG);
+ project.reset(RefNames.REFS_CONFIG);
+ pushFactory
+ .create(
+ admin.newIdent(),
+ project,
+ "Add OWNER file",
+ "OWNERS",
+ String.format(
+ "inherited: %s\nmatchers:\n" + "- suffix: .txt\n owners:\n - %s\n",
+ inherit, user.email()))
+ .to(RefNames.REFS_CONFIG);
+ }
+
+ private void addOwnerFileToRoot(boolean inherit) throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // owners:
+ // - admin
+ merge(
+ createChange(
+ testRepo,
+ "master",
+ "Add OWNER file",
+ "OWNERS",
+ String.format("inherited: %s\nowners:\n- %s\n", inherit, admin.email()),
+ ""));
+ }
+
+ private void addOwnerFileWithMatchersToRoot(boolean inherit) throws Exception {
+ // Add OWNERS file to root:
+ //
+ // inherited: true
+ // matchers:
+ // - suffix: .txt
+ // owners:
+ // - admin@mail.com
+ merge(
+ createChange(
+ testRepo,
+ "master",
+ "Add OWNER file",
+ "OWNERS",
+ String.format(
+ "inherited: %s\nmatchers:\n" + "- suffix: .txt\n owners:\n - %s\n",
+ inherit, admin.email()),
+ ""));
+ }
+}
