Make owners-api.jar lib a mandatory component
Refactor the dependency between owners-autoassign
and owners-api by making the installation of the
owners-api.jar into $GERRIT_SITE/lib explicit and
mandatory.
This simplifies the code and makes the dependency
between the two components cleaner.
From a user's perspective, this is a breaking change
because existing setups would need to add the owners-api.jar
into the $GERRIT_SITE/lib directory, otherwise the
owners-autoassign plugin would fail to load.
Change-Id: I71bd5386a40513fea1f26751b6ccbc95f0d17907
diff --git a/owners-autoassign/BUILD b/owners-autoassign/BUILD
index 024be42..4aa4cec 100644
--- a/owners-autoassign/BUILD
+++ b/owners-autoassign/BUILD
@@ -15,8 +15,8 @@
],
resources = glob(["src/main/**/*"]),
deps = [
+ ":owners-api-neverlink",
"//owners-common",
- "//plugins/owners-api",
],
)
@@ -28,10 +28,16 @@
visibility = ["//visibility:public"],
deps = PLUGIN_DEPS_NEVERLINK + [
"//owners-common",
- "//plugins/owners-api",
+ ":owners-api-neverlink",
],
)
+java_library(
+ name = "owners-api-neverlink",
+ neverlink = 1,
+ exports = ["//plugins/owners-api"],
+)
+
junit_tests(
name = "owners_autoassign_tests",
testonly = 1,
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/AutoassignModule.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/AutoassignModule.java
index d6bdaef..783e1df 100644
--- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/AutoassignModule.java
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/AutoassignModule.java
@@ -16,13 +16,34 @@
package com.googlesource.gerrit.owners.common;
+import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.inject.AbstractModule;
+import com.google.inject.Inject;
+import com.google.inject.Scopes;
+import com.googlesource.gerrit.owners.api.OwnersAttentionSet;
public class AutoassignModule extends AbstractModule {
+
+ private final Class<? extends OwnersAttentionSet> ownersAttentionSetImpl;
+
+ @Inject
+ public AutoassignModule() {
+ this(DefaultAddAllOwnersToAttentionSet.class);
+ }
+
+ @VisibleForTesting
+ public AutoassignModule(Class<? extends OwnersAttentionSet> ownersAttentionSetImpl) {
+ this.ownersAttentionSetImpl = ownersAttentionSetImpl;
+ }
+
@Override
protected void configure() {
DynamicSet.bind(binder(), GitReferenceUpdatedListener.class).to(GitRefListener.class);
+ DynamicItem.bind(binder(), OwnersAttentionSet.class)
+ .to(ownersAttentionSetImpl)
+ .in(Scopes.SINGLETON);
}
}
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/DefaultAddAllOwnersToAttentionSet.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/DefaultAddAllOwnersToAttentionSet.java
new file mode 100644
index 0000000..d53a239
--- /dev/null
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/DefaultAddAllOwnersToAttentionSet.java
@@ -0,0 +1,29 @@
+// 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.entities.Account.Id;
+import com.google.gerrit.extensions.common.ChangeInfo;
+import com.googlesource.gerrit.owners.api.OwnersAttentionSet;
+import java.util.Collection;
+
+class DefaultAddAllOwnersToAttentionSet implements OwnersAttentionSet {
+
+ @Override
+ public Collection<Id> addToAttentionSet(ChangeInfo changeInfo, Collection<Id> owners) {
+ return owners;
+ }
+}
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 040e3f5..c3d8e2d 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
@@ -38,8 +38,6 @@
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;
@@ -53,15 +51,6 @@
private final IdentifiedUser.GenericFactory userFactory;
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
@@ -70,12 +59,14 @@
GerritApi gApi,
IdentifiedUser.GenericFactory userFactory,
ChangeData.Factory changeDataFactory,
- PermissionBackend permissionBackend) {
+ PermissionBackend permissionBackend,
+ DynamicItem<OwnersAttentionSet> ownersForAttentionSet) {
this.requestContext = requestContext;
this.gApi = gApi;
this.userFactory = userFactory;
this.changeDataFactory = changeDataFactory;
this.permissionBackend = permissionBackend;
+ this.ownersForAttentionSet = ownersForAttentionSet;
}
public void addReviewers(ChangeApi cApi, Collection<Account.Id> reviewers)
@@ -87,12 +78,6 @@
// 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)) {
@@ -109,7 +94,7 @@
in.ignoreAutomaticAttentionSetRules = true;
in.addToAttentionSet =
- reviewersAccounts.stream()
+ ownersForAttentionSet.get().addToAttentionSet(changeInfo, reviewers).stream()
.map(
(reviewer) ->
new AttentionSetInput(
diff --git a/owners-autoassign/src/main/resources/Documentation/config.md b/owners-autoassign/src/main/resources/Documentation/config.md
index 6db4b1d..63dca0e 100644
--- a/owners-autoassign/src/main/resources/Documentation/config.md
+++ b/owners-autoassign/src/main/resources/Documentation/config.md
@@ -1,3 +1,13 @@
+## Setup
+
+The owners-autoassign plugin depends on the shared library `owners-api.jar`
+which needs to be installed into the `$GERRIT_SITE/lib` and requires a
+restart of the Gerrit service.
+
+Once the `owners-api.jar` is loaded at Gerrit startup, the `owners-autoassign.jar`
+file can be installed like a regular Gerrit plugin, by being dropped to the
+`GRRIT_SITE/plugins` directory or installed through the plugin manager.
+
## Configuration
Owner approval is determined based on OWNERS files located in the same
diff --git a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/AbstractAutoassignIT.java b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/AbstractAutoassignIT.java
index e8fa11b..eecbe77 100644
--- a/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/AbstractAutoassignIT.java
+++ b/owners-autoassign/src/test/java/com/googlesource/gerrit/owners/common/AbstractAutoassignIT.java
@@ -23,6 +23,8 @@
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.inject.AbstractModule;
+import com.google.inject.Module;
+import com.googlesource.gerrit.owners.api.OwnersApiModule;
import java.util.Collection;
import java.util.List;
import java.util.stream.Collectors;
@@ -46,6 +48,11 @@
}
}
+ @Override
+ public Module createModule() {
+ return new OwnersApiModule();
+ }
+
@Test
public void shouldAutoassignOneOwner() throws Exception {
String ownerEmail = user.email();
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
index b26b2f9..bcdca29 100644
--- 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
@@ -23,10 +23,8 @@
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;
@@ -34,7 +32,8 @@
@TestPlugin(
name = "owners-autoassign",
- sysModule = "com.googlesource.gerrit.owners.common.OwnersAutoassignWithAttentionSetIT$TestModule")
+ sysModule =
+ "com.googlesource.gerrit.owners.common.OwnersAutoassignWithAttentionSetIT$TestModule")
public class OwnersAutoassignWithAttentionSetIT extends LightweightPluginDaemonTest {
@Override
@@ -45,11 +44,7 @@
public static class TestModule extends AbstractModule {
@Override
protected void configure() {
- install(new AutoassignModule());
-
- DynamicItem.bind(binder(), OwnersAttentionSet.class)
- .to(SelectFirstOwnerForAttentionSet.class)
- .in(Scopes.SINGLETON);
+ install(new AutoassignModule(SelectFirstOwnerForAttentionSet.class));
}
}
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
index 1e0f8ec..1b75bf1 100644
--- a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
@@ -28,6 +28,8 @@
import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
+import com.google.inject.Module;
+import com.googlesource.gerrit.owners.api.OwnersApiModule;
import java.util.stream.StreamSupport;
import org.eclipse.jgit.transport.ReceiveCommand.Type;
import org.junit.Test;
@@ -43,6 +45,11 @@
String anOldObjectId = "anOldRef";
String aNewObjectId = "aNewRef";
+ @Override
+ public Module createModule() {
+ return new OwnersApiModule();
+ }
+
public static class TestModule extends AbstractModule {
@Override
protected void configure() {
