Allow adding reviewers in the OWNERS file
Enable the use of the OWNERS file for adding
extra accounts for being invited automatically to review
without being owners.
Example:
inherited: true
owners:
- some.email@example.com
- User Name
- group/Group of Users
reviewers:
- John Doe
- group/Group of Reviewers
- john@smith.com
NOTE: There is an overlapping of functionality
with the reviewers plugin. The purpose of this extra
support is to avoid duplication of configurations and
painful alignments between the OWNERS and reviewers.config
files.
Bug: Issue 14626
Change-Id: I512e5e73a97fd93b807b25e8ec0597109da6c5cf
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 81559f7..a38a237 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
@@ -147,8 +147,10 @@
PathOwners owners = new PathOwners(accounts, repository, change.branch, patchList);
Set<Account.Id> allReviewers = Sets.newHashSet();
allReviewers.addAll(owners.get().values());
+ allReviewers.addAll(owners.getReviewers().values());
for (Matcher matcher : owners.getMatchers().values()) {
allReviewers.addAll(matcher.getOwners());
+ allReviewers.addAll(matcher.getReviewers());
}
logger.debug("Autoassigned reviewers are: {}", allReviewers.toString());
reviewerManager.addReviewers(cApi, allReviewers);
diff --git a/owners-autoassign/src/main/resources/Documentation/config.md b/owners-autoassign/src/main/resources/Documentation/config.md
index b294ae8..6db4b1d 100644
--- a/owners-autoassign/src/main/resources/Documentation/config.md
+++ b/owners-autoassign/src/main/resources/Documentation/config.md
@@ -84,6 +84,27 @@
That means that in the absence of any OWNERS file in the target branch, the refs/meta/config
OWNERS is used as global default.
+## Additional non-owners added as reviewers
+
+The OWNERS file can also contain a section called `reviewers` that allows
+to add extra people as reviewers to a change without having to make them
+owners and therefore without having any impact on the underlying validation
+rules.
+
+See for instance the example below, where `john@example.com` is added as an additional
+reviewer in addition to the owners.
+
+```yaml
+inherited: true
+owners:
+- some.email@example.com
+- User Name
+reviewers:
+- john@example.com
+```
+
+The `reviewers` optional section can be added in any place where `owners` is specified
+and can be also associated with matchers exactly in the same way that `owners` do.
## Example 1 - OWNERS file without matchers and default Gerrit submit rules
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
new file mode 100644
index 0000000..70d6728
--- /dev/null
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/AbstractAutoassignIT.java
@@ -0,0 +1,204 @@
+// Copyright (C) 2021 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 com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.restapi.RestApiException;
+import com.google.inject.AbstractModule;
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+import org.junit.Ignore;
+import org.junit.Test;
+
+@Ignore
+public abstract class AbstractAutoassignIT extends LightweightPluginDaemonTest {
+ private final String section;
+ private final boolean INHERITED = true;
+ private final boolean NOT_INHERITED = false;
+
+ AbstractAutoassignIT(String section) {
+ this.section = section;
+ }
+
+ public static class TestModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ install(new AutoassignModule());
+ }
+ }
+
+ @Test
+ public void shouldAutoassignUserInPath() throws Exception {
+ String ownerEmail = user.email();
+
+ addOwnersToRepo("", ownerEmail, NOT_INHERITED);
+
+ Collection<AccountInfo> reviewers = getChangeReviewers(change(createChange()));
+
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewers).hasSize(1);
+ assertThat(reviewersEmail(reviewers).get(0)).isEqualTo(ownerEmail);
+ }
+
+ @Test
+ public void shouldAutoassignUserInPathWithInheritance() throws Exception {
+ String childOwnersEmail = accountCreator.user2().email();
+ String parentOwnersEmail = user.email();
+ String childpath = "childpath/";
+
+ addOwnersToRepo("", parentOwnersEmail, NOT_INHERITED);
+ addOwnersToRepo(childpath, childOwnersEmail, INHERITED);
+
+ Collection<AccountInfo> reviewers =
+ getChangeReviewers(change(createChange("test change", childpath + "foo.txt", "foo")));
+
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewersEmail(reviewers)).containsExactly(parentOwnersEmail, childOwnersEmail);
+ }
+
+ @Test
+ public void shouldAutoassignUserInPathWithoutInheritance() throws Exception {
+ String childOwnersEmail = accountCreator.user2().email();
+ String parentOwnersEmail = user.email();
+ String childpath = "childpath/";
+
+ addOwnersToRepo("", parentOwnersEmail, NOT_INHERITED);
+ addOwnersToRepo(childpath, childOwnersEmail, NOT_INHERITED);
+
+ Collection<AccountInfo> reviewers =
+ getChangeReviewers(change(createChange("test change", childpath + "foo.txt", "foo")));
+
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewersEmail(reviewers)).containsExactly(childOwnersEmail);
+ }
+
+ @Test
+ public void shouldAutoassignUserMatchingPath() throws Exception {
+ String ownerEmail = user.email();
+
+ addOwnersToRepo("", "suffix", ".java", ownerEmail, NOT_INHERITED);
+
+ Collection<AccountInfo> reviewers =
+ getChangeReviewers(change(createChange("test change", "foo.java", "foo")));
+
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewersEmail(reviewers)).containsExactly(ownerEmail);
+ }
+
+ @Test
+ public void shouldNotAutoassignUserNotMatchingPath() throws Exception {
+ String ownerEmail = user.email();
+
+ addOwnersToRepo("", "suffix", ".java", ownerEmail, NOT_INHERITED);
+
+ ChangeApi changeApi = change(createChange("test change", "foo.bar", "foo"));
+ Collection<AccountInfo> reviewers = getChangeReviewers(changeApi);
+
+ assertThat(reviewers).isNull();
+ }
+
+ @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);
+
+ ChangeApi changeApi = change(createChange("test change", childpath + "foo.java", "foo"));
+ Collection<AccountInfo> reviewers = getChangeReviewers(changeApi);
+
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewersEmail(reviewers)).containsExactly(parentOwnersEmail, childOwnersEmail);
+ }
+
+ @Test
+ public void shouldAutoassignUserMatchingPathWithoutInheritance() throws Exception {
+ String childOwnersEmail = accountCreator.user2().email();
+ String parentOwnersEmail = user.email();
+ String childpath = "childpath/";
+
+ addOwnersToRepo("", parentOwnersEmail, NOT_INHERITED);
+ addOwnersToRepo(childpath, "suffix", ".java", childOwnersEmail, NOT_INHERITED);
+
+ ChangeApi changeApi = change(createChange("test change", childpath + "foo.java", "foo"));
+ Collection<AccountInfo> reviewers = getChangeReviewers(changeApi);
+
+ assertThat(reviewers).isNotNull();
+ assertThat(reviewersEmail(reviewers)).containsExactly(childOwnersEmail);
+ }
+
+ private Collection<AccountInfo> getChangeReviewers(ChangeApi changeApi) throws RestApiException {
+ Collection<AccountInfo> reviewers = changeApi.get().reviewers.get(ReviewerState.REVIEWER);
+ return reviewers;
+ }
+
+ private List<String> reviewersEmail(Collection<AccountInfo> reviewers) {
+ List<String> reviewersEmail = reviewers.stream().map(a -> a.email).collect(Collectors.toList());
+ return reviewersEmail;
+ }
+
+ private void addOwnersToRepo(String parentPath, String ownerEmail, boolean inherited)
+ throws Exception {
+ pushFactory
+ .create(
+ admin.newIdent(),
+ testRepo,
+ "Set OWNERS",
+ parentPath + "OWNERS",
+ "inherited: " + inherited + "\n" + section + ":\n" + "- " + ownerEmail)
+ .to("refs/heads/master")
+ .assertOkStatus();
+ }
+
+ private void addOwnersToRepo(
+ String parentPath,
+ String matchingType,
+ String patternMatch,
+ String ownerEmail,
+ boolean inherited)
+ throws Exception {
+ pushFactory
+ .create(
+ admin.newIdent(),
+ testRepo,
+ "Set OWNERS",
+ parentPath + "OWNERS",
+ "inherited: "
+ + inherited
+ + "\n"
+ + "matchers:\n"
+ + "- "
+ + matchingType
+ + ": "
+ + patternMatch
+ + "\n"
+ + " "
+ + section
+ + ":\n"
+ + " - "
+ + ownerEmail)
+ .to("refs/heads/master")
+ .assertOkStatus();
+ }
+}
diff --git a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignIT.java b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignIT.java
index bd7dd8e..dbe6d41 100644
--- a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignIT.java
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignIT.java
@@ -15,48 +15,14 @@
package com.googlesource.gerrit.owners.common;
-import static com.google.common.truth.Truth.assertThat;
-
-import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.acceptance.TestPlugin;
-import com.google.gerrit.extensions.api.changes.ChangeApi;
-import com.google.gerrit.extensions.client.ReviewerState;
-import com.google.gerrit.extensions.common.AccountInfo;
-import com.google.inject.AbstractModule;
-import java.util.Collection;
-import org.junit.Test;
@TestPlugin(
name = "owners-api",
- sysModule = "com.googlesource.gerrit.owners.common.OwnersAutoassignIT$TestModule")
-public class OwnersAutoassignIT extends LightweightPluginDaemonTest {
+ sysModule = "com.googlesource.gerrit.owners.common.AbstractAutoassignIT$TestModule")
+public class OwnersAutoassignIT extends AbstractAutoassignIT {
- public static class TestModule extends AbstractModule {
- @Override
- protected void configure() {
- install(new AutoassignModule());
- }
- }
-
- @Test
- public void shouldAutoassignOneOwner() throws Exception {
- String ownerEmail = user.email();
-
- pushFactory
- .create(
- admin.newIdent(),
- testRepo,
- "Set OWNERS",
- "OWNERS",
- "inherited: false\n" + "owners:\n" + "- " + ownerEmail)
- .to("refs/heads/master")
- .assertOkStatus();
-
- ChangeApi changeApi = change(createChange());
- Collection<AccountInfo> reviewers = changeApi.get().reviewers.get(ReviewerState.REVIEWER);
-
- assertThat(reviewers).hasSize(1);
- AccountInfo reviewer = reviewers.iterator().next();
- assertThat(reviewer.email).isEqualTo(ownerEmail);
+ public OwnersAutoassignIT() {
+ super("owners");
}
}
diff --git a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/ReviewersAutoassignIT.java b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/ReviewersAutoassignIT.java
new file mode 100644
index 0000000..c88edfb
--- /dev/null
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/ReviewersAutoassignIT.java
@@ -0,0 +1,28 @@
+// Copyright (C) 2021 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.acceptance.TestPlugin;
+
+@TestPlugin(
+ name = "owners-api",
+ sysModule = "com.googlesource.gerrit.owners.common.AbstractAutoassignIT$TestModule")
+public class ReviewersAutoassignIT extends AbstractAutoassignIT {
+
+ public ReviewersAutoassignIT() {
+ super("reviewers");
+ }
+}
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 5b1049d..2fa2b16 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
@@ -30,7 +30,6 @@
import org.slf4j.LoggerFactory;
public class ConfigurationParser {
-
private static final Logger log = LoggerFactory.getLogger(OwnersConfig.class);
private Accounts accounts;
@@ -55,21 +54,14 @@
}
private void addClassicMatcher(JsonNode jsonNode, OwnersConfig ret) {
- Optional<Stream<String>> owners =
- Optional.ofNullable(jsonNode.get("owners")).map(ConfigurationParser::extractOwners);
- ret.setOwners(flattenSet(owners));
- }
-
- private static <T> Set<T> flattenSet(Optional<Stream<T>> optionalStream) {
- return flatten(optionalStream).collect(Collectors.toSet());
- }
-
- private static <T> Stream<T> flatten(Optional<Stream<T>> optionalStream) {
- return optionalStream.orElse(Stream.empty());
+ ret.setOwners(toClassicOwnersList(jsonNode, "owners").collect(Collectors.toSet()));
+ ret.setReviewers(toClassicOwnersList(jsonNode, "reviewers").collect(Collectors.toSet()));
}
private void addMatchers(JsonNode jsonNode, OwnersConfig ret) {
- getNode(jsonNode, "matchers").map(this::getMatchers).ifPresent(m -> m.forEach(ret::addMatcher));
+ getNode(jsonNode, "matchers")
+ .map(m -> getMatchers(m))
+ .ifPresent(m -> m.forEach(ret::addMatcher));
}
private Stream<Matcher> getMatchers(JsonNode node) {
@@ -79,29 +71,43 @@
.map(m -> m.get());
}
- private static Stream<String> extractOwners(JsonNode node) {
+ private static Stream<String> extractAsText(JsonNode node) {
if (node.isTextual()) {
return Stream.of(node.asText());
}
return iteratorStream(node.iterator()).map(JsonNode::asText);
}
+ private Stream<String> toClassicOwnersList(JsonNode jsonNode, String sectionName) {
+ Stream<String> ownersStream =
+ Optional.ofNullable(jsonNode.get(sectionName))
+ .map(ConfigurationParser::extractAsText)
+ .orElse(Stream.empty());
+ return ownersStream;
+ }
+
private Optional<Matcher> toMatcher(JsonNode node) {
Set<Id> owners =
- flatten(getNode(node, "owners").map(ConfigurationParser::extractOwners))
+ getNode(node, "owners")
+ .map(ConfigurationParser::extractAsText)
+ .orElse(Stream.empty())
.flatMap(o -> accounts.find(o).stream())
.collect(Collectors.toSet());
- if (owners.isEmpty()) {
- log.warn("Matchers must contain a list of owners");
- return Optional.empty();
- }
+ Set<Id> reviewers =
+ getNode(node, "reviewers")
+ .map(ConfigurationParser::extractAsText)
+ .orElse(Stream.empty())
+ .flatMap(o -> accounts.find(o).stream())
+ .collect(Collectors.toSet());
Optional<Matcher> suffixMatcher =
- getText(node, "suffix").map(el -> new SuffixMatcher(el, owners));
- Optional<Matcher> regexMatcher = getText(node, "regex").map(el -> new RegExMatcher(el, owners));
+ getText(node, "suffix").map(el -> new SuffixMatcher(el, owners, reviewers));
+ Optional<Matcher> regexMatcher =
+ getText(node, "regex").map(el -> new RegExMatcher(el, owners, reviewers));
Optional<Matcher> partialRegexMatcher =
- getText(node, "partial_regex").map(el -> new PartialRegExMatcher(el, owners));
- Optional<Matcher> exactMatcher = getText(node, "exact").map(el -> new ExactMatcher(el, owners));
+ getText(node, "partial_regex").map(el -> new PartialRegExMatcher(el, owners, reviewers));
+ Optional<Matcher> exactMatcher =
+ getText(node, "exact").map(el -> new ExactMatcher(el, owners, reviewers));
return Optional.ofNullable(
suffixMatcher.orElseGet(
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 07dadbe..2d1520a 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
@@ -19,8 +19,8 @@
import java.util.Set;
public class ExactMatcher extends Matcher {
- public ExactMatcher(String path, Set<Account.Id> owners) {
- super(path, owners);
+ public ExactMatcher(String path, Set<Account.Id> owners, Set<Account.Id> reviewers) {
+ super(path, owners, reviewers);
}
@Override
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 412fa79..026084f 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
@@ -19,16 +19,18 @@
public abstract class Matcher {
private Set<Account.Id> owners;
+ private Set<Account.Id> reviewers;
protected String path;
- public Matcher(String key, Set<Account.Id> owners) {
+ public Matcher(String key, Set<Account.Id> owners, Set<Account.Id> reviewers) {
this.path = key;
this.owners = owners;
+ this.reviewers = reviewers;
}
@Override
public String toString() {
- return "Matcher [path=" + path + ", owners=" + owners + "]";
+ return "Matcher [path=" + path + ", owners=" + owners + ", reviewers=" + reviewers + "]";
}
public Set<Account.Id> getOwners() {
@@ -39,6 +41,14 @@
this.owners = owners;
}
+ public Set<Account.Id> getReviewers() {
+ return reviewers;
+ }
+
+ public void setReviewers(Set<Account.Id> reviewers) {
+ this.reviewers = reviewers;
+ }
+
public void setPath(String path) {
this.path = path;
}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/OwnersConfig.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/OwnersConfig.java
index 0c29516..d6e40eb 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/OwnersConfig.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/OwnersConfig.java
@@ -30,8 +30,8 @@
/** Flag for marking that this OWNERS file inherits from the parent OWNERS. */
private boolean inherited = true;
- /** Set of OWNER email addresses. */
private Set<String> owners = Sets.newHashSet();
+ private Set<String> reviewers = Sets.newHashSet();
/** Map name of matcher and Matcher (value + Set Owners) */
private Map<String, Matcher> matchers = Maps.newHashMap();
@@ -63,6 +63,14 @@
this.owners = owners;
}
+ public Set<String> getReviewers() {
+ return reviewers;
+ }
+
+ public void setReviewers(Set<String> reviewers) {
+ this.reviewers = reviewers;
+ }
+
public Map<String, Matcher> getMatchers() {
return matchers;
}
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 06d9d12..111b88e 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
@@ -25,8 +25,10 @@
public class OwnersMap {
private SetMultimap<String, Account.Id> pathOwners = HashMultimap.create();
+ private SetMultimap<String, Account.Id> pathReviewers = HashMultimap.create();
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();
@Override
public String toString() {
@@ -45,6 +47,14 @@
this.pathOwners = pathOwners;
}
+ public SetMultimap<String, Account.Id> getPathReviewers() {
+ return pathReviewers;
+ }
+
+ public void setPathReviewers(SetMultimap<String, Account.Id> pathReviewers) {
+ this.pathReviewers = pathReviewers;
+ }
+
public Map<String, Matcher> getMatchers() {
return matchers;
}
@@ -57,10 +67,18 @@
pathOwners.putAll(ownersPath, owners);
}
+ public void addPathReviewers(String ownersPath, Set<Id> reviewers) {
+ pathOwners.putAll(ownersPath, reviewers);
+ }
+
public Map<String, Set<Id>> getFileOwners() {
return fileOwners;
}
+ public Map<String, Set<Id>> getFileReviewers() {
+ return fileReviewers;
+ }
+
public void addFileOwners(String file, Set<Id> owners) {
if (owners.isEmpty()) {
return;
@@ -74,4 +92,18 @@
fileOwners.put(file, Sets.newHashSet(owners));
}
}
+
+ public void addFileReviewers(String file, Set<Id> reviewers) {
+ if (reviewers.isEmpty()) {
+ return;
+ }
+
+ Set<Id> set = fileReviewers.get(file);
+ if (set != null) {
+ // add new owners removing duplicates
+ set.addAll(reviewers);
+ } else {
+ fileReviewers.put(file, Sets.newHashSet(reviewers));
+ }
+ }
}
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 c046c13..a8d49c4 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
@@ -23,8 +23,8 @@
public class PartialRegExMatcher extends Matcher {
Pattern pattern;
- public PartialRegExMatcher(String path, Set<Account.Id> owners) {
- super(path, owners);
+ public PartialRegExMatcher(String path, Set<Account.Id> owners, Set<Account.Id> reviewers) {
+ super(path, owners, reviewers);
pattern = Pattern.compile(".*" + path + ".*");
}
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 77289a3..299130c 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
@@ -49,6 +49,8 @@
private final SetMultimap<String, Account.Id> owners;
+ private final SetMultimap<String, Account.Id> reviewers;
+
private final Repository repository;
private final PatchList patchList;
@@ -69,6 +71,7 @@
OwnersMap map = fetchOwners(branch);
owners = Multimaps.unmodifiableSetMultimap(map.getPathOwners());
+ reviewers = Multimaps.unmodifiableSetMultimap(map.getPathReviewers());
matchers = map.getMatchers();
fileOwners = map.getFileOwners();
}
@@ -82,6 +85,15 @@
return owners;
}
+ /**
+ * Returns a read only view of the paths to reviewers mapping.
+ *
+ * @return multimap of paths to reviewers
+ */
+ public SetMultimap<String, Account.Id> getReviewers() {
+ return reviewers;
+ }
+
public Map<String, Matcher> getMatchers() {
return matchers;
}
@@ -102,12 +114,18 @@
PathOwnersEntry projectEntry =
getOwnersConfig(rootPath, RefNames.REFS_CONFIG)
- .map(conf -> new PathOwnersEntry(rootPath, conf, accounts, Collections.emptySet()))
+ .map(
+ conf ->
+ new PathOwnersEntry(
+ rootPath, conf, accounts, Collections.emptySet(), Collections.emptySet()))
.orElse(new PathOwnersEntry());
PathOwnersEntry rootEntry =
getOwnersConfig(rootPath, branch)
- .map(conf -> new PathOwnersEntry(rootPath, conf, accounts, Collections.emptySet()))
+ .map(
+ conf ->
+ new PathOwnersEntry(
+ rootPath, conf, accounts, Collections.emptySet(), Collections.emptySet()))
.orElse(new PathOwnersEntry());
Set<String> modifiedPaths = getModifiedPaths();
@@ -116,13 +134,15 @@
for (String path : modifiedPaths) {
currentEntry = resolvePathEntry(path, branch, projectEntry, rootEntry, entries);
- // add owners to file for matcher predicates
+ // add owners and reviewers to file for matcher predicates
ownersMap.addFileOwners(path, currentEntry.getOwners());
+ ownersMap.addFileReviewers(path, currentEntry.getReviewers());
// Only add the path to the OWNERS file to reduce the number of
// entries in the result
if (currentEntry.getOwnersPath() != null) {
ownersMap.addPathOwners(currentEntry.getOwnersPath(), currentEntry.getOwners());
+ ownersMap.addPathReviewers(currentEntry.getOwnersPath(), currentEntry.getReviewers());
}
ownersMap.addMatchers(currentEntry.getMatchers());
}
@@ -157,6 +177,7 @@
if (matcher.matches(path)) {
newMatchers.put(matcher.getPath(), matcher);
ownersMap.addFileOwners(path, matcher.getOwners());
+ ownersMap.addFileReviewers(path, matcher.getReviewers());
}
}
}
@@ -200,8 +221,9 @@
String ownersPath = partial + "OWNERS";
Optional<OwnersConfig> conf = getOwnersConfig(ownersPath, branch);
final Set<Id> owners = currentEntry.getOwners();
+ final Set<Id> reviewers = currentEntry.getReviewers();
currentEntry =
- conf.map(c -> new PathOwnersEntry(ownersPath, c, accounts, owners))
+ conf.map(c -> new PathOwnersEntry(ownersPath, c, accounts, owners, reviewers))
.orElse(currentEntry);
if (conf.map(OwnersConfig::isInherited).orElse(false)) {
for (Matcher m : currentEntry.getMatchers().values()) {
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 d2c73ec..01cdb7f 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
@@ -31,20 +31,31 @@
*/
class PathOwnersEntry {
private final boolean inherited;
+ private Set<Account.Id> owners = Sets.newHashSet();
+ private Set<Account.Id> reviewers = Sets.newHashSet();
public PathOwnersEntry() {
inherited = true;
}
public PathOwnersEntry(
- String path, OwnersConfig config, Accounts accounts, Set<Account.Id> inheritedOwners) {
+ String path,
+ OwnersConfig config,
+ Accounts accounts,
+ Set<Account.Id> inheritedOwners,
+ Set<Account.Id> inheritedReviewers) {
this.ownersPath = path;
this.owners =
config.getOwners().stream()
.flatMap(o -> accounts.find(o).stream())
.collect(Collectors.toSet());
+ this.reviewers =
+ config.getReviewers().stream()
+ .flatMap(o -> accounts.find(o).stream())
+ .collect(Collectors.toSet());
if (config.isInherited()) {
this.owners.addAll(inheritedOwners);
+ this.reviewers.addAll(inheritedReviewers);
}
this.matchers = config.getMatchers();
this.inherited = config.isInherited();
@@ -62,7 +73,6 @@
}
private String ownersPath;
- private Set<Account.Id> owners = Sets.newHashSet();
private Map<String, Matcher> matchers = Maps.newHashMap();
@@ -82,6 +92,14 @@
this.owners = owners;
}
+ public Set<Account.Id> getReviewers() {
+ return reviewers;
+ }
+
+ public void setReviewers(Set<Account.Id> reviewers) {
+ this.reviewers = reviewers;
+ }
+
public String getOwnersPath() {
return ownersPath;
}
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 45fd615..1d96646 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
@@ -22,8 +22,8 @@
public class RegExMatcher extends Matcher {
Pattern pattern;
- public RegExMatcher(String path, Set<Account.Id> owners) {
- super(path, owners);
+ public RegExMatcher(String path, Set<Account.Id> owners, Set<Account.Id> reviewers) {
+ super(path, owners, reviewers);
pattern = Pattern.compile(path);
}
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 39e8b32..c9a8a15 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
@@ -19,8 +19,8 @@
import java.util.Set;
public class SuffixMatcher extends Matcher {
- public SuffixMatcher(String path, Set<Account.Id> owners) {
- super(path, owners);
+ public SuffixMatcher(String path, Set<Account.Id> owners, Set<Account.Id> reviewers) {
+ super(path, owners, reviewers);
}
@Override
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 1fd557b..c61827e 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);
+ RegExMatcher matcher = new RegExMatcher(".*/a.*", null, null);
assertTrue(matcher.matches("xxxxxx/axxxx"));
assertFalse(matcher.matches("axxxx"));
assertFalse(matcher.matches("xxxxx/bxxxx"));
- RegExMatcher matcher2 = new RegExMatcher("a.*.sql", null);
+ RegExMatcher matcher2 = new RegExMatcher("a.*.sql", null, null);
assertFalse(matcher2.matches("xxxxxx/alfa.sql"));
}
@Test
public void testFloatingRegex() {
- PartialRegExMatcher matcher = new PartialRegExMatcher("a.*.sql", null);
+ PartialRegExMatcher matcher = new PartialRegExMatcher("a.*.sql", null, null);
assertTrue(matcher.matches("xxxxxxx/alfa.sql"));
assertTrue(matcher.matches("alfa.sqlxxxxx"));
assertFalse(matcher.matches("alfa.bar"));
