Migrate upload validator to use new diff cache Bug: Google b/200147261 Change-Id: I4081375e033f3cb438f667685e8bf9dc12391755
diff --git a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java index 1e805e9..a229ee6 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java +++ b/src/main/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidator.java
@@ -31,7 +31,6 @@ import com.google.gerrit.extensions.annotations.Exports; import com.google.gerrit.extensions.annotations.PluginName; import com.google.gerrit.extensions.api.projects.ProjectConfigEntryType; -import com.google.gerrit.extensions.client.DiffPreferencesInfo; import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.validators.CommentForValidation; import com.google.gerrit.extensions.validators.CommentValidationContext; @@ -45,10 +44,10 @@ import com.google.gerrit.server.git.validators.CommitValidationException; import com.google.gerrit.server.git.validators.CommitValidationListener; import com.google.gerrit.server.git.validators.CommitValidationMessage; -import com.google.gerrit.server.patch.PatchList; -import com.google.gerrit.server.patch.PatchListCache; -import com.google.gerrit.server.patch.PatchListKey; -import com.google.gerrit.server.patch.PatchListNotAvailableException; +import com.google.gerrit.server.patch.DiffNotAvailableException; +import com.google.gerrit.server.patch.DiffOperations; +import com.google.gerrit.server.patch.filediff.FileDiffOutput; +import com.google.gerrit.server.patch.filediff.TaggedEdit; import com.google.gerrit.server.project.NoSuchProjectException; import com.google.inject.AbstractModule; import com.google.inject.Inject; @@ -71,6 +70,7 @@ import java.util.concurrent.ExecutionException; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import org.eclipse.jgit.diff.Edit; import org.eclipse.jgit.diff.RawText; import org.eclipse.jgit.lib.ObjectId; @@ -122,8 +122,8 @@ private final GitRepositoryManager repoManager; private final LoadingCache<String, Pattern> patternCache; private final ContentTypeUtil contentTypeUtil; - private final PatchListCache patchListCache; private final ValidatorConfig validatorConfig; + private final DiffOperations diffOperations; @Inject BlockedKeywordValidator( @@ -132,14 +132,14 @@ @Named(CACHE_NAME) LoadingCache<String, Pattern> patternCache, PluginConfigFactory cfgFactory, GitRepositoryManager repoManager, - PatchListCache patchListCache, + DiffOperations diffOperations, ValidatorConfig validatorConfig) { this.pluginName = pluginName; this.patternCache = patternCache; this.cfgFactory = cfgFactory; this.repoManager = repoManager; this.contentTypeUtil = contentTypeUtil; - this.patchListCache = patchListCache; + this.diffOperations = diffOperations; this.validatorConfig = validatorConfig; } @@ -182,7 +182,7 @@ } catch (NoSuchProjectException | IOException | ExecutionException - | PatchListNotAvailableException e) { + | DiffNotAvailableException e) { throw new CommitValidationException("failed to check on blocked keywords", e); } return Collections.emptyList(); @@ -224,14 +224,12 @@ RevWalk revWalk, ImmutableCollection<Pattern> blockedKeywordPatterns, PluginConfig cfg) - throws IOException, ExecutionException, PatchListNotAvailableException { + throws IOException, ExecutionException, DiffNotAvailableException { List<CommitValidationMessage> messages = new LinkedList<>(); checkCommitMessageForBlockedKeywords(blockedKeywordPatterns, messages, c.getFullMessage()); Map<String, ObjectId> content = CommitUtils.getChangedContent(repo, c, revWalk); - PatchList patchList = - patchListCache.get( - PatchListKey.againstDefaultBase(c, DiffPreferencesInfo.Whitespace.IGNORE_NONE), - project); + Map<String, FileDiffOutput> fileDiffs = + diffOperations.listModifiedFilesAgainstParent(project, c, /* parentNum = */ 0); for (String path : content.keySet()) { ObjectLoader ol = revWalk.getObjectReader().open(content.get(path)); @@ -241,7 +239,13 @@ } } checkLineDiffForBlockedKeywords( - patchList.get(path).getEdits(), blockedKeywordPatterns, messages, path, ol); + fileDiffs.get(path).edits().stream() + .map(TaggedEdit::jgitEdit) + .collect(Collectors.toList()), + blockedKeywordPatterns, + messages, + path, + ol); } return messages; }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java index 245f2dc..c4d0ac4 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/uploadvalidator/BlockedKeywordValidatorTest.java
@@ -18,6 +18,7 @@ import static com.googlesource.gerrit.plugins.uploadvalidator.TestUtils.EMPTY_PLUGIN_CONFIG; import static com.googlesource.gerrit.plugins.uploadvalidator.TestUtils.PATTERN_CACHE; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -27,9 +28,10 @@ import com.google.gerrit.entities.Patch; import com.google.gerrit.entities.Project; import com.google.gerrit.server.git.validators.CommitValidationMessage; -import com.google.gerrit.server.patch.PatchList; -import com.google.gerrit.server.patch.PatchListCache; -import com.google.gerrit.server.patch.PatchListEntry; +import com.google.gerrit.server.patch.DiffOperations; +import com.google.gerrit.server.patch.filediff.Edit; +import com.google.gerrit.server.patch.filediff.FileDiffOutput; +import com.google.gerrit.server.patch.filediff.TaggedEdit; import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; @@ -39,7 +41,6 @@ import java.util.Set; import java.util.regex.Pattern; import org.eclipse.jgit.api.errors.GitAPIException; -import org.eclipse.jgit.diff.Edit; import org.eclipse.jgit.revwalk.RevCommit; import org.eclipse.jgit.revwalk.RevWalk; import org.junit.Test; @@ -79,16 +80,21 @@ @Test public void keywords() throws Exception { - // Mock the PatchListCache to return a diff for each file in our new commit - PatchListCache patchListCacheMock = mock(PatchListCache.class); - PatchList mockPatchList = mock(PatchList.class); - when(patchListCacheMock.get(any(), any(Project.NameKey.class))).thenReturn(mockPatchList); + // Mock DiffOperations to return a diff for each file in our new commit + DiffOperations diffOperationsMock = mock(DiffOperations.class); + Map<String, FileDiffOutput> mockDiffs = mock(Map.class); + when(diffOperationsMock.listModifiedFilesAgainstParent( + any(Project.NameKey.class), any(), anyInt())) + .thenReturn(mockDiffs); for (Map.Entry<String, String> fileContent : FILE_CONTENTS.entrySet()) { - PatchListEntry file = mock(PatchListEntry.class); - when(file.getEdits()) + FileDiffOutput file = mock(FileDiffOutput.class); + when(file.edits()) .thenReturn( - ImmutableList.of(new Edit(0, 0, 0, numberOfLinesInString(fileContent.getValue())))); - when(mockPatchList.get(fileContent.getKey())).thenReturn(file); + ImmutableList.of( + TaggedEdit.create( + Edit.create(0, 0, 0, numberOfLinesInString(fileContent.getValue())), + /* dueToRebase = */ false))); + when(mockDiffs.get(fileContent.getKey())).thenReturn(file); } try (RevWalk rw = new RevWalk(repo)) { @@ -100,7 +106,7 @@ PATTERN_CACHE, null, null, - patchListCacheMock, + diffOperationsMock, null); List<CommitValidationMessage> m = validator.performValidation(