Extend CommitMessageFetcher to handle non-commit objects
CommitMessageFetcher expected to fetch for commit objects and failed
for non-commit objects.
With the advent of NoteDB, RefUpdatedEvents sometimes contain
non-commit objects. For example `refs/sequences/changes` typically
references a blob. When this ref gets updated, Gerrit sends a
corresponding RefUpdatedEvent with blobs as oldRev and newRev, which
used to trip up CommitMessageFetcher.
To avoid issues, we extend CommitMessageFetcher to work on ids of
arbitrary objects.
Add Mockito dependency for executing the associated unit-tests.
Bug: Issue 10686
Change-Id: I892b9299f73e71660b1a7d77d7f282a4f563001e
(cherry picked from commit 59135736c5bba8b432a81bcc1722576a66563f11)
diff --git a/BUILD b/BUILD
index 03f8e0a..401da87 100644
--- a/BUILD
+++ b/BUILD
@@ -42,5 +42,6 @@
exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
":its-base__plugin",
":its-base_tests-utils",
+ "@mockito//jar",
],
)
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl
new file mode 100644
index 0000000..5eafc37
--- /dev/null
+++ b/external_plugin_deps.bzl
@@ -0,0 +1,33 @@
+load("//tools/bzl:maven_jar.bzl", "maven_jar")
+
+def external_plugin_deps():
+ maven_jar(
+ name = "mockito",
+ artifact = "org.mockito:mockito-core:2.27.0",
+ sha1 = "835fc3283b481f4758b8ef464cd560c649c08b00",
+ deps = [
+ "@byte-buddy//jar",
+ "@byte-buddy-agent//jar",
+ "@objenesis//jar",
+ ],
+ )
+
+ BYTE_BUDDY_VER = "1.9.10"
+
+ maven_jar(
+ name = "byte-buddy",
+ artifact = "net.bytebuddy:byte-buddy:" + BYTE_BUDDY_VER,
+ sha1 = "211a2b4d3df1eeef2a6cacf78d74a1f725e7a840",
+ )
+
+ maven_jar(
+ name = "byte-buddy-agent",
+ artifact = "net.bytebuddy:byte-buddy-agent:" + BYTE_BUDDY_VER,
+ sha1 = "9674aba5ee793e54b864952b001166848da0f26b",
+ )
+
+ maven_jar(
+ name = "objenesis",
+ artifact = "org.objenesis:objenesis:2.6",
+ sha1 = "639033469776fd37c08358c6b92a4761feb2af4b",
+ )
diff --git a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcher.java b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcher.java
index 905a2a3..7066f59 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcher.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcher.java
@@ -7,6 +7,7 @@
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevObject;
import org.eclipse.jgit.revwalk.RevWalk;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -21,22 +22,33 @@
this.repoManager = repoManager;
}
- public String fetch(String projectName, String commitId) throws IOException {
+ public String fetch(String projectName, String objectId) throws IOException {
try (Repository repo = repoManager.openRepository(new NameKey(projectName))) {
try (RevWalk revWalk = new RevWalk(repo)) {
- RevCommit commit = revWalk.parseCommit(ObjectId.fromString(commitId));
- return commit.getFullMessage();
+ RevObject obj = revWalk.peel(revWalk.parseAny(ObjectId.fromString(objectId)));
+ if (obj instanceof RevCommit) {
+ return ((RevCommit) obj).getFullMessage();
+ }
+ // objectId was found, but it's not a commit.
+ // Since the objectId was found, it's nothing to worry about and we do not need to alert the
+ // user. We silently return the empty string as blobs, trees, ... do not have a proper
+ // commit message.
+ //
+ // Parsing a non-commit objectId (and reaching this point) will happen for example on NoteDB
+ // sites when Gerrit updates `refs/sequences/changes` (which does not point at a commit, but
+ // a blob) on All-Projects and the corresponding RefUpdatedEvent gets processed.
+ return "";
}
}
}
- public String fetchGuarded(String projectName, String commitId) {
+ public String fetchGuarded(String projectName, String objectId) {
String ret = "";
try {
- ret = fetch(projectName, commitId);
+ ret = fetch(projectName, objectId);
} catch (IOException e) {
log.error(
- "Could not fetch commit message for commit " + commitId + " of project " + projectName,
+ "Could not fetch commit message for commit " + objectId + " of project " + projectName,
e);
}
return ret;
diff --git a/src/test/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcherTest.java b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcherTest.java
new file mode 100644
index 0000000..e833951
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/its/base/util/CommitMessageFetcherTest.java
@@ -0,0 +1,160 @@
+// 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.its.base.util;
+
+import static com.google.common.truth.Truth.assertThat;
+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.reviewdb.client.Project;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.googlesource.gerrit.plugins.its.base.testutil.LoggingMockingTestCase;
+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.Test;
+
+public class CommitMessageFetcherTest extends LoggingMockingTestCase {
+ 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() throws IOException {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetch("ProjectFoo", objectIdBlob);
+
+ assertThat(commitMessage).isEmpty();
+ }
+
+ @Test
+ public void testFetchTree() throws IOException {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetch("ProjectFoo", objectIdTree);
+
+ assertThat(commitMessage).isEmpty();
+ }
+
+ @Test
+ public void testFetchCommit() throws IOException {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetch("ProjectFoo", objectIdCommit);
+
+ assertThat(commitMessage).isEqualTo("CommitMsg\n");
+ }
+
+ @Test
+ public void testFetchGuardedBlob() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetchGuarded("ProjectFoo", objectIdBlob);
+
+ assertThat(commitMessage).isEmpty();
+ }
+
+ @Test
+ public void testFetchGuardedTree() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetchGuarded("ProjectFoo", objectIdTree);
+
+ assertThat(commitMessage).isEmpty();
+ }
+
+ @Test
+ public void testFetchGuardedCommit() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetchGuarded("ProjectFoo", objectIdCommit);
+
+ assertThat(commitMessage).isEqualTo("CommitMsg\n");
+ }
+
+ @Test
+ public void testFetchGuardedMissing() {
+ CommitMessageFetcher fetcher = createCommitMessageFetcher();
+ String commitMessage = fetcher.fetchGuarded("ProjectFoo", objectIdMissing);
+
+ assertThat(commitMessage).isEmpty();
+
+ assertLogMessageContains(objectIdMissing);
+ }
+
+ @Override
+ public void setUp() throws Exception {
+ super.setUp();
+
+ 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(new Project.NameKey("ProjectFoo")))).thenReturn(repo);
+ }
+
+ private CommitMessageFetcher createCommitMessageFetcher() {
+ return new CommitMessageFetcher(repoManager);
+ }
+}
