Record RestModifyView name in NoteDb reflog
Instead of using a generic string "Update NoteDb refs", record the
class name of the RestModifyView that caused this operation to
execute. This makes inspection of the history of a change much
easier, as its possible to correlate events to specific sections of
the Gerrit server.
The implementation favors going from the RestApiServlet down the
stack, so that plugin handlers are caught first and logged as the
plugin's RestModifyView implementation, even if the plugin is using
an internal API call to a standard view like PutTopic.
Change-Id: Ifd6a9843b6bd5f3e7f1ff7e145fa433f237b147a
diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ReflogIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ReflogIT.java
new file mode 100644
index 0000000..312974f
--- /dev/null
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/notedb/ReflogIT.java
@@ -0,0 +1,57 @@
+// Copyright (C) 2017 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.google.gerrit.acceptance.server.notedb;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.TruthJUnit.assume;
+import static com.google.gerrit.reviewdb.client.RefNames.changeMetaRef;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.UseLocalDisk;
+import com.google.gerrit.reviewdb.client.Change;
+import java.io.File;
+import org.eclipse.jgit.lib.ReflogEntry;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Before;
+import org.junit.Test;
+
+@UseLocalDisk
+public class ReflogIT extends AbstractDaemonTest {
+ @Before
+ public void setUp() throws Exception {
+ assume().that(notesMigration.disableChangeReviewDb()).isTrue();
+ }
+
+ @Test
+ public void guessRestApiInReflog() throws Exception {
+ assume().that(notesMigration.disableChangeReviewDb()).isTrue();
+ PushOneCommit.Result r = createChange();
+ Change.Id id = r.getChange().getId();
+
+ try (Repository repo = repoManager.openRepository(r.getChange().project())) {
+ File log = new File(repo.getDirectory(), "logs/" + changeMetaRef(id));
+ if (!log.exists()) {
+ log.getParentFile().mkdirs();
+ assertThat(log.createNewFile()).isTrue();
+ }
+
+ gApi.changes().id(id.get()).topic("foo");
+ ReflogEntry last = repo.getReflogReader(changeMetaRef(id)).getLastEntry();
+ assertThat(last).named("last RefLogEntry").isNotNull();
+ assertThat(last.getComment()).isEqualTo("change.PutTopic");
+ }
+ }
+}
diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 5b1045a..d1e4e889 100644
--- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+// WARNING: NoteDbUpdateManager cares about the package name RestApiServlet lives in.
package com.google.gerrit.httpd.restapi;
import static com.google.common.base.Preconditions.checkNotNull;
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
index eef16fb..ea1f891 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/NoteDbUpdateManager.java
@@ -24,10 +24,12 @@
import com.google.auto.value.AutoValue;
import com.google.common.collect.HashBasedTable;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Table;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.metrics.Timer1;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
@@ -41,6 +43,7 @@
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.update.ChainedReceiveCommands;
import com.google.gerrit.server.update.RefUpdateUtil;
+import com.google.gerrit.server.update.RetryingRestModifyView;
import com.google.gwtorm.server.OrmConcurrencyException;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -77,6 +80,12 @@
public class NoteDbUpdateManager implements AutoCloseable {
public static final String CHANGES_READ_ONLY = "NoteDb changes are read-only";
+ private static final ImmutableList<String> PACKAGE_PREFIXES =
+ ImmutableList.of("com.google.gerrit.server.", "com.google.gerrit.");
+ private static final ImmutableSet<String> SERVLET_NAMES =
+ ImmutableSet.of(
+ "com.google.gerrit.httpd.restapi.RestApiServlet", RetryingRestModifyView.class.getName());
+
public interface Factory {
NoteDbUpdateManager create(Project.NameKey projectName);
}
@@ -539,7 +548,11 @@
BatchRefUpdate bru = or.repo.getRefDatabase().newBatchUpdate();
bru.setPushCertificate(pushCert);
- bru.setRefLogMessage(firstNonNull(refLogMessage, "Update NoteDb refs"), false);
+ if (refLogMessage != null) {
+ bru.setRefLogMessage(refLogMessage, false);
+ } else {
+ bru.setRefLogMessage(firstNonNull(guessRestApiHandler(), "Update NoteDb refs"), false);
+ }
bru.setRefLogIdent(refLogIdent != null ? refLogIdent : serverIdent.get());
or.cmds.addTo(bru);
bru.setAllowNonFastForwards(true);
@@ -550,6 +563,45 @@
return bru;
}
+ private static String guessRestApiHandler() {
+ StackTraceElement[] trace = Thread.currentThread().getStackTrace();
+ int i = findRestApiServlet(trace);
+ if (i < 0) {
+ return null;
+ }
+ try {
+ for (i--; i >= 0; i--) {
+ String cn = trace[i].getClassName();
+ Class<?> cls = Class.forName(cn);
+ if (RestModifyView.class.isAssignableFrom(cls) && cls != RetryingRestModifyView.class) {
+ return viewName(cn);
+ }
+ }
+ return null;
+ } catch (ClassNotFoundException e) {
+ return null;
+ }
+ }
+
+ private static String viewName(String cn) {
+ String impl = cn.replace('$', '.');
+ for (String p : PACKAGE_PREFIXES) {
+ if (impl.startsWith(p)) {
+ return impl.substring(p.length());
+ }
+ }
+ return impl;
+ }
+
+ private static int findRestApiServlet(StackTraceElement[] trace) {
+ for (int i = 0; i < trace.length; i++) {
+ if (SERVLET_NAMES.contains(trace[i].getClassName())) {
+ return i;
+ }
+ }
+ return -1;
+ }
+
private void addCommands() throws OrmException, IOException {
if (isEmpty()) {
return;
