Merge branch 'stable-3.2' into stable-3.3
* stable-3.2:
Trigger owners-autoassign processing for change readyForReview
Change-Id: I33b00227bae5e923bedb6cc7c03d64286ff8ebd1
diff --git a/owners-api/BUILD b/owners-api/BUILD
new file mode 100644
index 0000000..ccf66e6
--- /dev/null
+++ b/owners-api/BUILD
@@ -0,0 +1,36 @@
+load("//tools/bzl:junit.bzl", "junit_tests")
+load("//tools/bzl:plugin.bzl", "PLUGIN_DEPS", "PLUGIN_DEPS_NEVERLINK", "PLUGIN_TEST_DEPS", "gerrit_plugin")
+
+gerrit_plugin(
+ name = "owners-api",
+ srcs = glob([
+ "src/main/java/**/*.java",
+ ]),
+ dir_name = "owners",
+ manifest_entries = [
+ "Implementation-Title: Gerrit OWNERS api plugin",
+ "Implementation-URL: https://gerrit.googlesource.com/plugins/owners",
+ "Gerrit-PluginName: owners-api",
+ "Gerrit-Module: com.googlesource.gerrit.owners.common.AutoassignModule",
+ ],
+ resources = glob(["src/main/**/*"]),
+ deps = [],
+)
+
+java_library(
+ name = "owners-api_deps",
+ srcs = glob([
+ "src/main/java/**/*.java",
+ ]),
+ visibility = ["//visibility:public"],
+ deps = PLUGIN_DEPS_NEVERLINK,
+)
+
+junit_tests(
+ name = "owners_api_tests",
+ testonly = 1,
+ srcs = glob(["src/test/java/**/*.java"]),
+ deps = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
+ ":owners-api_deps",
+ ],
+)
diff --git a/owners-api/src/main/java/com/googlesource/gerrit/owners/api/OwnersApiModule.java b/owners-api/src/main/java/com/googlesource/gerrit/owners/api/OwnersApiModule.java
new file mode 100644
index 0000000..54ee51d
--- /dev/null
+++ b/owners-api/src/main/java/com/googlesource/gerrit/owners/api/OwnersApiModule.java
@@ -0,0 +1,27 @@
+// 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.api;
+
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.inject.AbstractModule;
+
+public class OwnersApiModule extends AbstractModule {
+
+ @Override
+ protected void configure() {
+ DynamicItem.itemOf(binder(), OwnersAttentionSet.class);
+ }
+}
diff --git a/owners-api/src/main/java/com/googlesource/gerrit/owners/api/OwnersAttentionSet.java b/owners-api/src/main/java/com/googlesource/gerrit/owners/api/OwnersAttentionSet.java
new file mode 100644
index 0000000..31cd2a4
--- /dev/null
+++ b/owners-api/src/main/java/com/googlesource/gerrit/owners/api/OwnersAttentionSet.java
@@ -0,0 +1,33 @@
+// 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.api;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import java.util.Collection;
+
+/** API to expose a mechanism to selectively add owners to the attention-set. */
+public interface OwnersAttentionSet {
+
+ /**
+ * Select the owners that should be added to the attention-set.
+ *
+ * @param changeInfo change under review
+ * @param owners set of owners associated with a change.
+ * @return subset of owners that need to be added to the attention-set.
+ */
+ Collection<Account.Id> addToAttentionSet(ChangeInfo changeInfo, Collection<Account.Id> owners);
+}
diff --git a/owners-api/src/test/java/com/googlesource/gerrit/owners/api/OwnersAttentionSetIT.java b/owners-api/src/test/java/com/googlesource/gerrit/owners/api/OwnersAttentionSetIT.java
new file mode 100644
index 0000000..8a3ec18
--- /dev/null
+++ b/owners-api/src/test/java/com/googlesource/gerrit/owners/api/OwnersAttentionSetIT.java
@@ -0,0 +1,67 @@
+// 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.api;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.entities.Account.Id;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.google.inject.Scopes;
+import java.util.Collection;
+import org.junit.Test;
+
+@TestPlugin(
+ name = "owners-api",
+ sysModule = "com.googlesource.gerrit.owners.api.OwnersAttentionSetIT$TestModule")
+public class OwnersAttentionSetIT extends LightweightPluginDaemonTest {
+
+ @Inject private DynamicItem<OwnersAttentionSet> ownerAttentionSetItem;
+
+ @Override
+ public Module createModule() {
+ return new OwnersApiModule();
+ }
+
+ public static class TestModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ DynamicItem.bind(binder(), OwnersAttentionSet.class)
+ .to(SelectFirstOwnerForAttentionSet.class)
+ .in(Scopes.SINGLETON);
+ }
+ }
+
+ public static class SelectFirstOwnerForAttentionSet implements OwnersAttentionSet {
+ @Override
+ public Collection<Id> addToAttentionSet(ChangeInfo changeInfo, Collection<Id> owners) {
+ return null;
+ }
+ }
+
+ @Test
+ public void shouldAllowOwnersAttentionSetOverride() {
+ OwnersAttentionSet attentionSetSelector = ownerAttentionSetItem.get();
+
+ assertThat(attentionSetSelector).isNotNull();
+ assertThat(attentionSetSelector.getClass()).isEqualTo(SelectFirstOwnerForAttentionSet.class);
+ }
+}
diff --git a/owners-autoassign/BUILD b/owners-autoassign/BUILD
index 463afcb..024be42 100644
--- a/owners-autoassign/BUILD
+++ b/owners-autoassign/BUILD
@@ -12,11 +12,11 @@
"Implementation-URL: https://gerrit.googlesource.com/plugins/owners",
"Gerrit-PluginName: owners-autoassign",
"Gerrit-Module: com.googlesource.gerrit.owners.common.AutoassignModule",
- "Gerrit-ApiVersion: 2.16",
],
resources = glob(["src/main/**/*"]),
deps = [
"//owners-common",
+ "//plugins/owners-api",
],
)
@@ -28,6 +28,7 @@
visibility = ["//visibility:public"],
deps = PLUGIN_DEPS_NEVERLINK + [
"//owners-common",
+ "//plugins/owners-api",
],
)
@@ -37,6 +38,7 @@
srcs = glob(["src/test/java/**/*.java"]),
deps = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
"//owners-common",
+ "//plugins/owners-api",
":owners-autoassign_deps",
],
)
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 6f53d04..7792583 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
@@ -170,8 +170,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/java/com/googlesource/gerrit/owners/common/ReviewerManager.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java
index c3708b5..eef9321 100644
--- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java
@@ -21,9 +21,11 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.AttentionSetInput;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.permissions.ChangePermission;
@@ -33,8 +35,12 @@
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
+import com.googlesource.gerrit.owners.api.OwnersAttentionSet;
import java.util.ArrayList;
import java.util.Collection;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.stream.Collectors;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -48,6 +54,16 @@
private final ChangeData.Factory changeDataFactory;
private final PermissionBackend permissionBackend;
+ /**
+ * TODO: The optional injection here is needed for keeping backward compatibility with existing
+ * setups that do not have the owners-api.jar configured as Gerrit libModule.
+ *
+ * <p>Once merged to master, the optional injection can go and this can be moved as extra argument
+ * in the constructor.
+ */
+ @Inject(optional = true)
+ private DynamicItem<OwnersAttentionSet> ownersForAttentionSet;
+
@Inject
public ReviewerManager(
OneOffRequestContext requestContext,
@@ -71,6 +87,12 @@
// TODO(davido): Switch back to using changes API again,
// when it supports batch mode for adding reviewers
ReviewInput in = new ReviewInput();
+ Collection<Account.Id> reviewersAccounts =
+ Optional.ofNullable(ownersForAttentionSet)
+ .map(DynamicItem::get)
+ .filter(Objects::nonNull)
+ .map(owners -> owners.addToAttentionSet(changeInfo, reviewers))
+ .orElse(reviewers);
in.reviewers = new ArrayList<>(reviewers.size());
for (Account.Id account : reviewers) {
if (isVisibleTo(changeInfo, account)) {
@@ -84,6 +106,16 @@
changeInfo._number);
}
}
+
+ in.ignoreAutomaticAttentionSetRules = true;
+ in.addToAttentionSet =
+ reviewersAccounts.stream()
+ .map(
+ (reviewer) ->
+ new AttentionSetInput(
+ reviewer.toString(), "Selected as member of the OWNERS file"))
+ .collect(Collectors.toList());
+
gApi.changes().id(changeInfo.id).current().review(in);
}
} catch (RestApiException e) {
diff --git a/owners-autoassign/src/main/resources/Documentation/attention-set.md b/owners-autoassign/src/main/resources/Documentation/attention-set.md
new file mode 100644
index 0000000..7ad9a55
--- /dev/null
+++ b/owners-autoassign/src/main/resources/Documentation/attention-set.md
@@ -0,0 +1,88 @@
+## Attention-Set
+
+The owners-autoassign plugin allows to customize the selection of owners
+that need to be added to the attention-set.
+By default, Gerrit adds all reviewers to the attention-set, which could
+not be ideal when the list of owners automatically assigned could be
+quite long, due to the hierarchy of the OWNERS files in the parent
+directories.
+
+The `owners-api.jar` libModule included in the owners' plugin project contains
+a generic interface that can be used to customize Gerrit's default
+attention-set behaviour.
+
+## owner-api setup
+
+Copy the `owners-api.jar` libModule into the $GERRIT_SITE/lib directory
+and add the following entry to `gerrit.config`:
+
+```
+[gerrit]
+ installModule = com.googlesource.gerrit.owners.api.OwnersApiModule
+```
+
+## Customization of the attention-set selection
+
+The OwnersAttentionSet API, contained in the owners-api.jar libModule,
+provides the following interface:
+
+```
+public interface OwnersAttentionSet {
+
+ Collection<Account.Id> addToAttentionSet(ChangeInfo changeInfo, Collection<Account.Id> owners);
+}
+```
+
+Any other plugin, or script, can implement the interface and provide
+an alternative implementation of the Gerrit's default mechanism.
+
+Example: select two random owners and add to the attention set by adding the
+following script as $GERRIT_SITE/plugins/owners-attentionset-1.0.groovy.
+
+```
+import com.google.inject.*
+import com.google.gerrit.common.*
+import com.google.gerrit.entities.*
+import com.google.gerrit.extensions.common.*
+import com.google.gerrit.extensions.registration.*
+import com.googlesource.gerrit.owners.api.*
+import java.util.*
+
+@Singleton
+class MyAttentionSet implements OwnersAttentionSet {
+ def desiredAttentionSet = 3
+
+ Collection<Account.Id> addToAttentionSet(ChangeInfo changeInfo, Collection<Account.Id> owners) {
+ def currentAttentionSet = changeInfo.attentionSet.size()
+
+ // There is already the desired number of attention-set
+ if (currentAttentionSet >= desiredAttentionSet) {
+ return Collections.emptyList()
+ }
+
+ // All owners are within the attention-set limits
+ if (owners.size() <= desiredAttentionSet) {
+ return owners
+ }
+
+ // Select randomly some owners for the attention-set
+ def shuffledOwners = owners.asType(List)
+ Collections.shuffle shuffledOwners
+ return shuffledOwners.subList(0,desiredAttentionSet)
+ }
+}
+
+class MyAttentionSetModule extends AbstractModule {
+
+ protected void configure() {
+ DynamicItem.bind(binder(), OwnersAttentionSet.class)
+ .to(MyAttentionSet.class)
+ .in(Scopes.SINGLETON)
+ }
+}
+
+modules = [ MyAttentionSetModule.class ]
+```
+
+**NOTE**: Install the [groovy-provider plugin](https://gerrit.googlesource.com/plugins/scripting/groovy-provider/)
+for enabling Gerrit to load Groovy scripts as plugins.
\ No newline at end of file
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
new file mode 100644
index 0000000..dbe6d41
--- /dev/null
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignIT.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 OwnersAutoassignIT extends AbstractAutoassignIT {
+
+ public OwnersAutoassignIT() {
+ super("owners");
+ }
+}
diff --git a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignWithAttentionSetIT.java b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignWithAttentionSetIT.java
new file mode 100644
index 0000000..b26b2f9
--- /dev/null
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignWithAttentionSetIT.java
@@ -0,0 +1,90 @@
+// 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 static java.util.stream.Collectors.toList;
+
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.entities.Account.Id;
+import com.google.gerrit.extensions.client.ReviewerState;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.inject.AbstractModule;
+import com.google.inject.Module;
+import com.google.inject.Scopes;
+import com.googlesource.gerrit.owners.api.OwnersApiModule;
+import com.googlesource.gerrit.owners.api.OwnersAttentionSet;
+import java.util.Collection;
+import org.junit.Test;
+
+@TestPlugin(
+ name = "owners-autoassign",
+ sysModule = "com.googlesource.gerrit.owners.common.OwnersAutoassignWithAttentionSetIT$TestModule")
+public class OwnersAutoassignWithAttentionSetIT extends LightweightPluginDaemonTest {
+
+ @Override
+ public Module createModule() {
+ return new OwnersApiModule();
+ }
+
+ public static class TestModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ install(new AutoassignModule());
+
+ DynamicItem.bind(binder(), OwnersAttentionSet.class)
+ .to(SelectFirstOwnerForAttentionSet.class)
+ .in(Scopes.SINGLETON);
+ }
+ }
+
+ public static class SelectFirstOwnerForAttentionSet implements OwnersAttentionSet {
+
+ @Override
+ public Collection<Id> addToAttentionSet(ChangeInfo changeInfo, Collection<Id> owners) {
+ return owners.stream().limit(1).collect(toList());
+ }
+ }
+
+ @Test
+ public void shouldAutoassignTwoOwnersWithOneAttentionSet() throws Exception {
+ String ownerEmail1 = user.email();
+ String ownerEmail2 = accountCreator.user2().email();
+
+ pushFactory
+ .create(
+ admin.newIdent(),
+ testRepo,
+ "Set OWNERS",
+ "OWNERS",
+ "inherited: false\n"
+ + "owners:\n"
+ + "- "
+ + ownerEmail1
+ + "\n"
+ + "- "
+ + ownerEmail2
+ + "\n")
+ .to("refs/heads/master")
+ .assertOkStatus();
+
+ ChangeInfo change = change(createChange()).get();
+ assertThat(change.reviewers.get(ReviewerState.REVIEWER)).hasSize(2);
+ assertThat(change.attentionSet).hasSize(1);
+ }
+}
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..d97b01e 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
@@ -16,15 +16,21 @@
package com.googlesource.gerrit.owners.common;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Account.Id;
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
public boolean matches(String pathToMatch) {
return pathToMatch.equals(path);
}
+
+ @Override
+ protected Matcher clone(Set<Id> owners, Set<Id> reviewers) {
+ return new ExactMatcher(path, owners, reviewers);
+ }
}
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..7c1565f 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
@@ -14,21 +14,25 @@
package com.googlesource.gerrit.owners.common;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Account.Id;
import java.util.Set;
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 +43,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;
}
@@ -48,4 +60,19 @@
}
public abstract boolean matches(String pathToMatch);
+
+ public Matcher merge(Matcher other) {
+ if (other == null) {
+ return this;
+ }
+
+ return clone(mergeSet(owners, other.owners), mergeSet(reviewers, other.reviewers));
+ }
+
+ protected abstract Matcher clone(Set<Id> owners, Set<Id> reviewers);
+
+ private Set<Id> mergeSet(Set<Id> set1, Set<Id> set2) {
+ ImmutableSet.Builder<Id> setBuilder = ImmutableSet.builder();
+ return setBuilder.addAll(set1).addAll(set2).build();
+ }
}
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..18f04f5 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
@@ -16,6 +16,7 @@
package com.googlesource.gerrit.owners.common;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Account.Id;
import java.util.Set;
import java.util.regex.Pattern;
@@ -23,8 +24,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 + ".*");
}
@@ -32,4 +33,9 @@
public boolean matches(String pathToMatch) {
return pattern.matcher(pathToMatch).matches();
}
+
+ @Override
+ protected Matcher clone(Set<Id> owners, Set<Id> reviewers) {
+ return new PartialRegExMatcher(path, owners, reviewers);
+ }
}
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..6bf288b 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
@@ -31,6 +31,7 @@
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListEntry;
import java.io.IOException;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Iterator;
@@ -49,6 +50,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 +72,7 @@
OwnersMap map = fetchOwners(branch);
owners = Multimaps.unmodifiableSetMultimap(map.getPathOwners());
+ reviewers = Multimaps.unmodifiableSetMultimap(map.getPathReviewers());
matchers = map.getMatchers();
fileOwners = map.getFileOwners();
}
@@ -82,6 +86,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 +115,28 @@
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(),
+ 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(),
+ Collections.emptySet()))
.orElse(new PathOwnersEntry());
Set<String> modifiedPaths = getModifiedPaths();
@@ -116,13 +145,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 +188,7 @@
if (matcher.matches(path)) {
newMatchers.put(matcher.getPath(), matcher);
ownersMap.addFileOwners(path, matcher.getOwners());
+ ownersMap.addFileReviewers(path, matcher.getReviewers());
}
}
}
@@ -200,14 +232,14 @@
String ownersPath = partial + "OWNERS";
Optional<OwnersConfig> conf = getOwnersConfig(ownersPath, branch);
final Set<Id> owners = currentEntry.getOwners();
+ final Set<Id> reviewers = currentEntry.getReviewers();
+ Collection<Matcher> inheritedMatchers = currentEntry.getMatchers().values();
currentEntry =
- conf.map(c -> new PathOwnersEntry(ownersPath, c, accounts, owners))
+ conf.map(
+ c ->
+ new PathOwnersEntry(
+ ownersPath, c, accounts, owners, reviewers, inheritedMatchers))
.orElse(currentEntry);
- if (conf.map(OwnersConfig::isInherited).orElse(false)) {
- for (Matcher m : currentEntry.getMatchers().values()) {
- currentEntry.addMatcher(m);
- }
- }
entries.put(partial, currentEntry);
}
}
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..6c498d6 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,22 +31,40 @@
*/
class PathOwnersEntry {
private final boolean inherited;
+ private Set<Account.Id> owners = Sets.newHashSet();
+ private Set<Account.Id> reviewers = Sets.newHashSet();
+ private String ownersPath;
+ private Map<String, Matcher> matchers = Maps.newHashMap();
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,
+ Collection<Matcher> inheritedMatchers) {
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());
+ this.matchers = config.getMatchers();
+
if (config.isInherited()) {
this.owners.addAll(inheritedOwners);
+ this.reviewers.addAll(inheritedReviewers);
+ for (Matcher matcher : inheritedMatchers) {
+ addMatcher(matcher);
+ }
}
- this.matchers = config.getMatchers();
this.inherited = config.isInherited();
}
@@ -61,13 +79,9 @@
+ "]";
}
- private String ownersPath;
- private Set<Account.Id> owners = Sets.newHashSet();
-
- private Map<String, Matcher> matchers = Maps.newHashMap();
-
public void addMatcher(Matcher matcher) {
- this.matchers.put(matcher.getPath(), matcher);
+ Matcher currMatchers = this.matchers.get(matcher.getPath());
+ this.matchers.put(matcher.getPath(), matcher.merge(currMatchers));
}
public Map<String, Matcher> getMatchers() {
@@ -82,6 +96,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..13f3636 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
@@ -16,14 +16,15 @@
package com.googlesource.gerrit.owners.common;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Account.Id;
import java.util.Set;
import java.util.regex.Pattern;
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);
}
@@ -31,4 +32,9 @@
public boolean matches(String pathToMatch) {
return pattern.matcher(pathToMatch).matches();
}
+
+ @Override
+ protected Matcher clone(Set<Id> owners, Set<Id> reviewers) {
+ return new RegExMatcher(path, owners, reviewers);
+ }
}
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..6b56cc4 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
@@ -16,15 +16,21 @@
package com.googlesource.gerrit.owners.common;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Account.Id;
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
public boolean matches(String pathToMatch) {
return pathToMatch.endsWith(path);
}
+
+ @Override
+ protected Matcher clone(Set<Id> owners, Set<Id> reviewers) {
+ return new SuffixMatcher(path, owners, reviewers);
+ }
}
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"));
diff --git a/owners/BUILD b/owners/BUILD
index 3acc05b..ab6705e 100644
--- a/owners/BUILD
+++ b/owners/BUILD
@@ -36,7 +36,6 @@
"Implementation-URL: https://gerrit.googlesource.com/plugins/owners",
"Gerrit-PluginName: owners",
"Gerrit-Module: com.googlesource.gerrit.owners.OwnersModule",
- "Gerrit-ApiVersion: 2.16",
],
resources = glob(["src/main/resources/**/*"]),
deps = [
