Split out commit message extraction to make things better testable RevCommit has final fields, and RevWalk etc is a pain to mock out. So we split that part out to make writing tests easier. CommitMessageFetcher and its test come from the its-base plugin. Change-Id: I5a24ae94a5e6418bba5e04464719e27ac3b19368
diff --git a/BUILD b/BUILD index 4fd31b4..5368f19 100644 --- a/BUILD +++ b/BUILD
@@ -1,4 +1,11 @@ -load("//tools/bzl:plugin.bzl", "gerrit_plugin") +load("@rules_java//java:defs.bzl", "java_library") +load("//tools/bzl:junit.bzl", "junit_tests") +load( + "//tools/bzl:plugin.bzl", + "PLUGIN_DEPS", + "PLUGIN_TEST_DEPS", + "gerrit_plugin", +) gerrit_plugin( name = "zuul", @@ -9,3 +16,23 @@ "Gerrit-Module: com.googlesource.gerrit.plugins.zuul.Module", ], ) + +junit_tests( + name = "zuul_tests", + testonly = 1, + srcs = glob(["src/test/java/**/*.java"]), + tags = ["zuul"], + deps = [ + ":zuul__plugin_test_deps", + ], +) + +java_library( + name = "zuul__plugin_test_deps", + testonly = 1, + visibility = ["//visibility:public"], + exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [ + ":zuul__plugin", + ], +) +
diff --git a/src/main/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcher.java b/src/main/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcher.java new file mode 100644 index 0000000..961cad8 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcher.java
@@ -0,0 +1,43 @@ +// Copyright (C) 2020 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.plugins.zuul; + +import com.google.gerrit.entities.Project; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.inject.Inject; +import java.io.IOException; +import org.eclipse.jgit.errors.RepositoryNotFoundException; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.revwalk.RevWalk; + +public class CommitMessageFetcher { + private final GitRepositoryManager repoManager; + + @Inject + CommitMessageFetcher(GitRepositoryManager repoManager) { + this.repoManager = repoManager; + } + + public String fetch(Project.NameKey p, String rev) + throws RepositoryNotFoundException, IOException { + try (Repository repo = repoManager.openRepository(p); + RevWalk rw = new RevWalk(repo)) { + RevCommit commit = rw.parseCommit(ObjectId.fromString(rev)); + return commit.getFullMessage(); + } + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java b/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java index 55a7123..6e8b1a7 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java +++ b/src/main/java/com/googlesource/gerrit/plugins/zuul/GetCrd.java
@@ -24,7 +24,6 @@ import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.TopLevelResource; import com.google.gerrit.server.change.RevisionResource; -import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.restapi.change.ChangesCollection; import com.google.gerrit.server.restapi.change.QueryChanges; @@ -36,22 +35,18 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import org.eclipse.jgit.errors.RepositoryNotFoundException; -import org.eclipse.jgit.lib.ObjectId; -import org.eclipse.jgit.lib.Repository; -import org.eclipse.jgit.revwalk.RevCommit; -import org.eclipse.jgit.revwalk.RevWalk; @Singleton public class GetCrd implements RestReadView<RevisionResource> { private static final FluentLogger logger = FluentLogger.forEnclosingClass(); private final ChangesCollection changes; - private final GitRepositoryManager repoManager; + private final CommitMessageFetcher commitMessageFetcher; @Inject - GetCrd(ChangesCollection changes, GitRepositoryManager repoManager) { + GetCrd(ChangesCollection changes, CommitMessageFetcher commitMessageFetcher) { this.changes = changes; - this.repoManager = repoManager; + this.commitMessageFetcher = commitMessageFetcher; } @Override @@ -67,18 +62,14 @@ // get depends on info Project.NameKey p = rsrc.getChange().getProject(); - try (Repository repo = repoManager.openRepository(p); - RevWalk rw = new RevWalk(repo)) { - String rev = rsrc.getPatchSet().commitId().getName(); - RevCommit commit = rw.parseCommit(ObjectId.fromString(rev)); - String commitMsg = commit.getFullMessage(); - Pattern pattern = Pattern.compile("[Dd]epends-[Oo]n:? (I[0-9a-f]{8,40})", Pattern.DOTALL); - Matcher matcher = pattern.matcher(commitMsg); - while (matcher.find()) { - String otherId = matcher.group(1); - logger.atFinest().log("Change %s depends on change %s", thisId, otherId); - out.dependsOn.add(otherId); - } + String rev = rsrc.getPatchSet().commitId().getName(); + String commitMsg = commitMessageFetcher.fetch(p, rev); + Pattern pattern = Pattern.compile("[Dd]epends-[Oo]n:? (I[0-9a-f]{8,40})", Pattern.DOTALL); + Matcher matcher = pattern.matcher(commitMsg); + while (matcher.find()) { + String otherId = matcher.group(1); + logger.atFinest().log("Change %s depends on change %s", thisId, otherId); + out.dependsOn.add(otherId); } // get needed by info
diff --git a/src/test/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcherTest.java b/src/test/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcherTest.java new file mode 100644 index 0000000..edda09e --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/zuul/CommitMessageFetcherTest.java
@@ -0,0 +1,123 @@ +// Copyright (C) 2020 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.plugins.zuul; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.gerrit.testing.GerritJUnit.assertThrows; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.google.gerrit.entities.Project; +import com.google.gerrit.server.git.GitRepositoryManager; +import java.io.IOException; +import java.math.BigInteger; +import java.util.HashSet; +import java.util.Set; +import org.eclipse.jgit.errors.MissingObjectException; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.ObjectLoader; +import org.eclipse.jgit.lib.ObjectReader; +import org.eclipse.jgit.lib.Repository; +import org.junit.Before; +import org.junit.Test; + +public class CommitMessageFetcherTest { + private GitRepositoryManager repoManager; + private Repository repo; + private String objectIdBlob = "24c5735c3e8ce8fd18d312e9e58149a62236c01a"; + private String objectIdTree = "3faaefce19558dfc8d9c976f09ae4897f45cb242"; + private String objectIdCommit = "95aed53c03b6d3df0912bdd9bb1d0c6eaf619f58"; + private String objectIdMissing = "0123456789012345678901234567890123456789"; + + private byte rawBlob[] = "def\n".getBytes(); + private byte rawTree[] = sha1append("100644 abc\000", objectIdBlob); + private byte rawCommit[] = + ("tree\0003faaefce19558dfc8d9c976f09ae4897f45cb242\n" + + "author Author <author@example.org> 1592579853 +0200\n" + + "committer Committer <committer@example.org> 1592579853 +0200\n" + + "\n" + + "CommitMsg\n") + .getBytes(); + + private static byte[] sha1append(String left, String sha1sum) { + int leftLen = left.length(); + byte[] right = (new BigInteger(sha1sum, 16)).toByteArray(); + byte[] ret = new byte[leftLen + right.length]; + System.arraycopy(left.getBytes(), 0, ret, 0, leftLen); + System.arraycopy(right, 0, ret, leftLen, right.length); + return ret; + } + + @Test + public void testFetchBlob() { + CommitMessageFetcher fetcher = createCommitMessageFetcher(); + assertThrows( + IOException.class, () -> fetcher.fetch(Project.nameKey("ProjectFoo"), objectIdBlob)); + } + + @Test + public void testFetchTree() { + CommitMessageFetcher fetcher = createCommitMessageFetcher(); + assertThrows( + IOException.class, () -> fetcher.fetch(Project.nameKey("ProjectFoo"), objectIdTree)); + } + + @Test + public void testFetchCommit() throws IOException { + CommitMessageFetcher fetcher = createCommitMessageFetcher(); + String commitMessage = fetcher.fetch(Project.nameKey("ProjectFoo"), objectIdCommit); + + assertThat(commitMessage).isEqualTo("CommitMsg\n"); + } + + @Before + public void setUp() throws Exception { + ObjectLoader objectLoaderBlob = mock(ObjectLoader.class); + when(objectLoaderBlob.getCachedBytes(anyInt())).thenReturn(rawBlob); + when(objectLoaderBlob.getType()).thenReturn(Constants.OBJ_BLOB); + + ObjectLoader objectLoaderTree = mock(ObjectLoader.class); + when(objectLoaderTree.getCachedBytes(anyInt())).thenReturn(rawTree); + when(objectLoaderTree.getType()).thenReturn(Constants.OBJ_TREE); + + ObjectLoader objectLoaderCommit = mock(ObjectLoader.class); + when(objectLoaderCommit.getCachedBytes(anyInt())).thenReturn(rawCommit); + when(objectLoaderCommit.getType()).thenReturn(Constants.OBJ_COMMIT); + + Set<ObjectId> shallowCommits = new HashSet<>(); + shallowCommits.add(ObjectId.fromString(objectIdCommit)); + + ObjectReader objectReader = mock(ObjectReader.class); + when(objectReader.getShallowCommits()).thenReturn(shallowCommits); + when(objectReader.open(ObjectId.fromString(objectIdBlob))).thenReturn(objectLoaderBlob); + when(objectReader.open(ObjectId.fromString(objectIdTree))).thenReturn(objectLoaderTree); + when(objectReader.open(ObjectId.fromString(objectIdCommit))).thenReturn(objectLoaderCommit); + when(objectReader.open(ObjectId.fromString(objectIdMissing))) + .thenThrow( + new MissingObjectException(ObjectId.fromString(objectIdMissing), Constants.OBJ_COMMIT)); + + repo = mock(Repository.class); + when(repo.newObjectReader()).thenReturn(objectReader); + + repoManager = mock(GitRepositoryManager.class); + when(repoManager.openRepository(eq(Project.nameKey("ProjectFoo")))).thenReturn(repo); + } + + private CommitMessageFetcher createCommitMessageFetcher() { + return new CommitMessageFetcher(repoManager); + } +}