Merge branch 'stable-3.4'
* stable-3.4:
Fix gerrit module name
Change-Id: Iddbad9b4652f74b41ece376de3826b25a89d622f
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl
index bca48e4..5ad1930 100644
--- a/external_plugin_deps.bzl
+++ b/external_plugin_deps.bzl
@@ -2,13 +2,12 @@
JACKSON_VER = "2.9.7"
-def external_plugin_deps(omit_jackson_core = True):
- if not omit_jackson_core:
- maven_jar(
- name = "jackson-core",
- artifact = "com.fasterxml.jackson.core:jackson-core:" + JACKSON_VER,
- sha1 = "4b7f0e0dc527fab032e9800ed231080fdc3ac015",
- )
+def external_plugin_deps():
+ maven_jar(
+ name = "jackson-core",
+ artifact = "com.fasterxml.jackson.core:jackson-core:" + JACKSON_VER,
+ sha1 = "4b7f0e0dc527fab032e9800ed231080fdc3ac015",
+ )
maven_jar(
name = "jackson-databind",
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 4ed43b2..931c05a 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,14 +16,35 @@
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);
install(new AutoassignConfigModule());
}
}
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/GitRefListener.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
index 756332c..0064c36 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
@@ -40,7 +40,8 @@
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeNotesCommit;
import com.google.gerrit.server.notedb.ChangeNotesCommit.ChangeNotesRevWalk;
-import com.google.gerrit.server.patch.PatchList;
+import com.google.gerrit.server.patch.DiffSummary;
+import com.google.gerrit.server.patch.DiffSummaryKey;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListKey;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -206,7 +207,7 @@
try {
ChangeApi cApi = changes.id(cId.get());
ChangeInfo change = cApi.get();
- PatchList patchList = getPatchList(repository, event, change);
+ DiffSummary patchList = getDiffSummary(repository, event, change);
if (patchList != null) {
PathOwners owners = new PathOwners(accounts, repository, change.branch, patchList);
Set<Account.Id> allReviewers = Sets.newHashSet();
@@ -226,7 +227,7 @@
}
}
- private PatchList getPatchList(Repository repository, Event event, ChangeInfo change) {
+ private DiffSummary getDiffSummary(Repository repository, Event event, ChangeInfo change) {
ObjectId newId = null;
PatchListKey plKey;
try {
@@ -240,7 +241,8 @@
}
plKey = PatchListKey.againstCommit(null, newId, IGNORE_NONE);
}
- return patchListCache.get(plKey, Project.nameKey(change.project));
+ return patchListCache.getDiffSummary(
+ DiffSummaryKey.fromPatchListKey(plKey), Project.nameKey(change.project));
} catch (PatchListNotAvailableException | IOException e) {
logger.warn("Could not load patch list for change {}", change.id, e);
}
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 ea1975c..34d6e49 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,10 +21,10 @@
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.Project.NameKey;
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.api.changes.ReviewerInput;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.registration.DynamicItem;
@@ -58,15 +58,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;
private final AutoassignConfig cfg;
@@ -76,14 +67,16 @@
OneOffRequestContext requestContext,
GerritApi gApi,
IdentifiedUser.GenericFactory userFactory,
- ChangeData.Factory changeDataFactory,
PermissionBackend permissionBackend,
+ ChangeData.Factory changeDataFactory,
+ DynamicItem<OwnersAttentionSet> ownersForAttentionSet,
AutoassignConfig cfg) {
this.requestContext = requestContext;
this.gApi = gApi;
this.userFactory = userFactory;
this.changeDataFactory = changeDataFactory;
this.permissionBackend = permissionBackend;
+ this.ownersForAttentionSet = ownersForAttentionSet;
this.cfg = cfg;
}
@@ -107,7 +100,7 @@
Collection<Account.Id> validOwnersForAttentionSet = new ArrayList<>(accountsIds.size());
for (Account.Id account : accountsIds) {
if (!currentReviewers.contains(account.get()) && isVisibleTo(changeInfo, account)) {
- AddReviewerInput addReviewerInput = new AddReviewerInput();
+ ReviewerInput addReviewerInput = new ReviewerInput();
addReviewerInput.reviewer = account.toString();
addReviewerInput.state = reviewerState;
in.reviewers.add(addReviewerInput);
@@ -117,7 +110,8 @@
}
} else {
log.warn(
- "Not adding account {} as reviewer to change {} because the associated ref is not visible",
+ "Not adding account {} as reviewer to change {} because the associated ref is not"
+ + " visible",
account,
changeInfo._number);
}
@@ -137,7 +131,7 @@
in.ignoreAutomaticAttentionSetRules = true;
in.addToAttentionSet =
- reviewersAccounts.stream()
+ ownersForAttentionSet.get().addToAttentionSet(changeInfo, reviewersAccounts).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 afb534c..96a8fa0 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.
+
## Project configuration
The project configuration `autoAssignWip` controls the automatic
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 4aa6ff4..fd258bb 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
@@ -20,9 +20,9 @@
import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
import com.google.gerrit.common.RawInputUtil;
-import com.google.gerrit.extensions.api.changes.AddReviewerInput;
import com.google.gerrit.extensions.api.changes.ChangeApi;
import com.google.gerrit.extensions.api.changes.ChangeEditApi;
+import com.google.gerrit.extensions.api.changes.ReviewerInput;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.ChangeInfo;
@@ -31,6 +31,8 @@
import com.google.gerrit.server.project.ProjectConfig;
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.Collection;
import java.util.List;
import java.util.stream.Collectors;
@@ -63,6 +65,11 @@
}
@Override
+ public Module createModule() {
+ return new OwnersApiModule();
+ }
+
+ @Override
public void setUpTestPlugin() throws Exception {
super.setUpTestPlugin();
@@ -120,7 +127,7 @@
assertThat(reviewers).hasSize(1);
// Switch user from CC to Reviewer or the other way around
- AddReviewerInput switchReviewerInput = new AddReviewerInput();
+ ReviewerInput switchReviewerInput = new ReviewerInput();
switchReviewerInput.reviewer = ownerEmail;
switchReviewerInput.state =
assignedUserState == ReviewerState.REVIEWER ? ReviewerState.CC : ReviewerState.REVIEWER;
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 36795a6..56cf33a 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
@@ -30,12 +30,10 @@
import com.google.gerrit.extensions.api.groups.GroupInput;
import com.google.gerrit.extensions.client.ReviewerState;
import com.google.gerrit.extensions.common.ChangeInfo;
-import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.inject.AbstractModule;
import com.google.inject.Inject;
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;
@@ -58,11 +56,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));
}
}
@@ -88,7 +82,7 @@
@Test
public void shouldAddToAttentionSetOneUserIfAnotherUserHasNoPermission() throws Exception {
- TestAccount userWithAccessToProject = accountCreator.user();
+ TestAccount userWithAccessToProject = accountCreator.user1();
TestAccount userWithNoAccessToProject = accountCreator.user2();
AccountGroup.UUID groupWithNoAccessToProject =
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 2cbceda..884c986 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
@@ -35,6 +35,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 com.googlesource.gerrit.owners.common.AutoassignConfigModule;
import java.util.HashMap;
import java.util.Map;
@@ -54,6 +56,11 @@
String anOldObjectId = "anOldRef";
String aNewObjectId = "aNewRef";
+ @Override
+ public Module createModule() {
+ return new OwnersApiModule();
+ }
+
public static class TestModule extends AbstractModule {
@Override
protected void configure() {
diff --git a/owners-common/common.bzl b/owners-common/common.bzl
index c571145..d2ce9f5 100644
--- a/owners-common/common.bzl
+++ b/owners-common/common.bzl
@@ -1,5 +1,5 @@
EXTERNAL_DEPS = [
- "@jackson-core//jar:neverlink",
+ "@jackson-core//jar",
"@jackson-databind//jar",
"@jackson-annotations//jar",
"@jackson-dataformat-yaml//jar",
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 6bf288b..172a824 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwners.java
@@ -20,6 +20,7 @@
import static com.google.gerrit.entities.Patch.MERGE_LIST;
import static com.googlesource.gerrit.owners.common.JgitWrapper.getBlobAsBytes;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.common.collect.Multimaps;
import com.google.common.collect.SetMultimap;
@@ -28,8 +29,8 @@
import com.google.gerrit.entities.Account.Id;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.RefNames;
-import com.google.gerrit.server.patch.PatchList;
-import com.google.gerrit.server.patch.PatchListEntry;
+import com.google.gerrit.server.patch.DiffSummary;
+import com.google.gerrit.server.patch.filediff.FileDiffOutput;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
@@ -54,19 +55,34 @@
private final Repository repository;
- private final PatchList patchList;
-
private final ConfigurationParser parser;
+ private final Set<String> modifiedPaths;
+
private final Accounts accounts;
private Map<String, Matcher> matchers;
private Map<String, Set<Id>> fileOwners;
- public PathOwners(Accounts accounts, Repository repository, String branch, PatchList patchList) {
+ public PathOwners(
+ Accounts accounts,
+ Repository repository,
+ String branch,
+ Map<String, FileDiffOutput> fileDiffMap) {
+ this(accounts, repository, branch, getModifiedPaths(fileDiffMap));
+ }
+
+ public PathOwners(
+ Accounts accounts, Repository repository, String branch, DiffSummary diffSummary) {
+ this(accounts, repository, branch, ImmutableSet.copyOf(diffSummary.getPaths()));
+ }
+
+ private PathOwners(
+ Accounts accounts, Repository repository, String branch, Set<String> modifiedPaths) {
this.repository = repository;
- this.patchList = patchList;
+ this.modifiedPaths = modifiedPaths;
+
this.parser = new ConfigurationParser(accounts);
this.accounts = accounts;
@@ -76,7 +92,6 @@
matchers = map.getMatchers();
fileOwners = map.getFileOwners();
}
-
/**
* Returns a read only view of the paths to owners mapping.
*
@@ -139,7 +154,6 @@
Collections.emptySet()))
.orElse(new PathOwnersEntry());
- Set<String> modifiedPaths = getModifiedPaths();
Map<String, PathOwnersEntry> entries = new HashMap<>();
PathOwnersEntry currentEntry = null;
for (String path : modifiedPaths) {
@@ -247,22 +261,22 @@
}
/**
- * Parses the patch list for any paths that were modified.
+ * Parses the diff list for any paths that were modified.
*
* @return set of modified paths.
*/
- private Set<String> getModifiedPaths() {
+ private static Set<String> getModifiedPaths(Map<String, FileDiffOutput> patchList) {
Set<String> paths = Sets.newHashSet();
- for (PatchListEntry patch : patchList.getPatches()) {
+ for (Map.Entry<String, FileDiffOutput> patch : patchList.entrySet()) {
// Ignore commit message and Merge List
- String newName = patch.getNewName();
+ String newName = patch.getKey();
if (!COMMIT_MSG.equals(newName) && !MERGE_LIST.equals(newName)) {
paths.add(newName);
// If a file was moved then we need approvals for old and new
// path
- if (patch.getChangeType() == Patch.ChangeType.RENAMED) {
- paths.add(patch.getOldName());
+ if (patch.getValue().changeType() == Patch.ChangeType.RENAMED) {
+ paths.add(patch.getValue().oldPath().get());
}
}
}
diff --git a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
index f721539..749056c 100644
--- a/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
+++ b/owners/src/main/java/com/googlesource/gerrit/owners/OwnersStoredValues.java
@@ -16,12 +16,13 @@
package com.googlesource.gerrit.owners;
-import com.google.gerrit.server.patch.PatchList;
+import com.google.gerrit.server.patch.filediff.FileDiffOutput;
import com.google.gerrit.server.rules.StoredValue;
import com.google.gerrit.server.rules.StoredValues;
import com.googlecode.prolog_cafe.lang.Prolog;
import com.googlesource.gerrit.owners.common.Accounts;
import com.googlesource.gerrit.owners.common.PathOwners;
+import java.util.Map;
import org.eclipse.jgit.lib.Repository;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -41,7 +42,7 @@
new StoredValue<PathOwners>() {
@Override
protected PathOwners createValue(Prolog engine) {
- PatchList patchList = StoredValues.PATCH_LIST.get(engine);
+ Map<String, FileDiffOutput> patchList = StoredValues.DIFF_LIST.get(engine);
Repository repository = StoredValues.REPOSITORY.get(engine);
String branch = StoredValues.getChange(engine).getDest().branch();
return new PathOwners(accounts, repository, branch, patchList);
