Adapt to Gerrit v3.{5,6} with the new diff-cache
Gerrit has removed the PatchList cache from v3.6 onwards
and replaced with the new DiffCache.
Avoid using PatchList and use instead the new DiffSummary
coming out of the box from the new DiffCache.
Also add jackson-core explicitly as it isn't provided by
Gerrit anymore and include it as part of the owners plugins.
Change-Id: I99494051eb1dbbb1ca58bbc8e85bdf8a94a3733a
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/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 489381c..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
@@ -67,10 +67,9 @@
OneOffRequestContext requestContext,
GerritApi gApi,
IdentifiedUser.GenericFactory userFactory,
+ PermissionBackend permissionBackend,
ChangeData.Factory changeDataFactory,
- PermissionBackend permissionBackend,
DynamicItem<OwnersAttentionSet> ownersForAttentionSet,
- PermissionBackend permissionBackend,
AutoassignConfig cfg) {
this.requestContext = requestContext;
this.gApi = gApi;
@@ -111,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);
}
@@ -131,7 +131,7 @@
in.ignoreAutomaticAttentionSetRules = true;
in.addToAttentionSet =
- ownersForAttentionSet.get().addToAttentionSet(changeInfo, reviewers).stream()
+ ownersForAttentionSet.get().addToAttentionSet(changeInfo, reviewersAccounts).stream()
.map(
(reviewer) ->
new AttentionSetInput(
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 164d121..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;
@@ -127,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 a5c683b..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
@@ -82,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-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);
