Allow custom initial attention-set from owners
Avoid adding too many reviewers automatically to the attention-set
when a change is created. The OWNERS list, because of hierarchy, may
pull a lot of names as reviewers, but that doesn't mean that all of them
should have the initial task to provide the first feedback.
Expose an API (OwnersAttentionSet that allows an external script
or plugin to specify the logic for selecting the accounts for the
attention set based on:
- Owners
- Change
The new API is contained in owners-api.jar that needs to be installed
and configured as a libModule as follows:
[gerrit]
installModule = com.googlesource.gerrit.owners.api.OwnersApiModule
Implementing the API for adding owners to attention-set is quite easy
and can be done also through scripting.
In Groovy it would be as easy as:
class MySelector implements OwnersAttentionSet {
Collection<Account.Id> addToAttentionSet
(ChangeInfo changeInfo,
Collection<Account.Id> owners) {
owners.take(2)
}
}
For existing owners-autoassign plugins, the injection of the
DynamicItem<OwnersAttentionSet> is optional and therefore is fully
transparent and all owners are added to the attention-set as per
Gerrit default mechanism.
Bug: Issue 14452
Change-Id: Ie2a6fbc289c66d99986bbaacb6ea4c9864515513
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/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/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..f6553f1
--- /dev/null
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignIT.java
@@ -0,0 +1,64 @@
+// 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.acceptance.TestPlugin;
+import com.google.gerrit.acceptance.UseLocalDisk;
+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.vmware.gerrit.owners.common.OwnersAutoassignIT$TestModule")
+public class OwnersAutoassignIT extends LightweightPluginDaemonTest {
+
+ public static class TestModule extends AbstractModule {
+ @Override
+ protected void configure() {
+ install(new AutoassignModule());
+ }
+ }
+
+ @UseLocalDisk
+ @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);
+ }
+}
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..d4bfb19
--- /dev/null
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/OwnersAutoassignWithAttentionSetIT.java
@@ -0,0 +1,92 @@
+// 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.acceptance.UseLocalDisk;
+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.vmware.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());
+ }
+ }
+
+ @UseLocalDisk
+ @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/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 = [
