Merge "Return X-Gerrit-UpdatedRef in the response headers of WRITE requests"
diff --git a/Documentation/rest-api.txt b/Documentation/rest-api.txt
index eabcaa9..8709843 100644
--- a/Documentation/rest-api.txt
+++ b/Documentation/rest-api.txt
@@ -244,6 +244,38 @@
Given the trace ID an administrator can find the corresponding logs and
investigate issues more easily.
+[[updated-refs]]
+=== X-Gerrit-UpdatedRef
+For each write REST request, we return X-Gerrit-UpdatedRef headers as the refs
+that were updated in the current request (involved in a ref transaction in the
+current request).
+
+The format of those headers is `PROJECT_NAME~REF_NAME~OLD_SHA-1~NEW_SHA-1`. The
+project and ref names are URL-encoded, and must use %7E for '~'.
+
+A new SHA-1 of `0000000000000000000000000000000000000000` is treated as a
+deleted ref.
+If the new SHA-1 is not `0000000000000000000000000000000000000000`, the ref was
+either updated or created.
+If the old SHA-1 is `0000000000000000000000000000000000000000`, the ref was
+created.
+
+.Example Request
+----
+ DELETE /changes/myProject~master~I8473b95934b5732ac55d26311a706c9c2bde9940
+----
+
+.Example Response
+----
+HTTP/1.1 204 NO CONTENT
+ Content-Disposition: attachment
+ Content-Type: application/json; charset=UTF-8
+ X-Gerrit-UpdatedRef: myProject~refs%2Fchanges%2F01%2F1%2F1~deadbeefdeadbeefdeadbeefdeadbeefdeadbeef~0000000000000000000000000000000000000000
+ X-Gerrit-UpdatedRef: myProject~refs%2Fchanges%2F01%2F1%2Fmeta~deadbeefdeadbeefdeadbeefdeadbeefdeadbeef~0000000000000000000000000000000000000000
+
+ )]}'
+----
+
GERRIT
------
Part of link:index.html[Gerrit Code Review]
diff --git a/java/com/google/gerrit/httpd/CacheBasedWebSession.java b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
index 7212e3e..0a54448 100644
--- a/java/com/google/gerrit/httpd/CacheBasedWebSession.java
+++ b/java/com/google/gerrit/httpd/CacheBasedWebSession.java
@@ -42,7 +42,7 @@
import org.eclipse.jgit.http.server.GitSmartHttpTools;
@RequestScoped
-public abstract class CacheBasedWebSession implements WebSession {
+public abstract class CacheBasedWebSession extends WebSession {
@VisibleForTesting public static final String ACCOUNT_COOKIE = "GerritAccount";
protected static final long MAX_AGE_MINUTES = HOURS.toMinutes(12);
diff --git a/java/com/google/gerrit/httpd/GitReferenceUpdatedTracker.java b/java/com/google/gerrit/httpd/GitReferenceUpdatedTracker.java
new file mode 100644
index 0000000..c37d30b
--- /dev/null
+++ b/java/com/google/gerrit/httpd/GitReferenceUpdatedTracker.java
@@ -0,0 +1,43 @@
+// Copyright (C) 2021 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.httpd;
+
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.extensions.registration.DynamicItem;
+import com.google.inject.Inject;
+import com.google.inject.Singleton;
+
+/**
+ * Stores the updated refs whenever they are updated, so that we can export this information in the
+ * response headers.
+ */
+@Singleton
+public class GitReferenceUpdatedTracker implements GitReferenceUpdatedListener {
+
+ private final DynamicItem<WebSession> webSession;
+
+ @Inject
+ GitReferenceUpdatedTracker(DynamicItem<WebSession> webSession) {
+ this.webSession = webSession;
+ }
+
+ @Override
+ public void onGitReferenceUpdated(GitReferenceUpdatedListener.Event event) {
+ WebSession currentSession = webSession.get();
+ if (currentSession != null) {
+ currentSession.addRefUpdatedEvents(event);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/httpd/HttpdModule.java b/java/com/google/gerrit/httpd/HttpdModule.java
new file mode 100644
index 0000000..1f1ec2f
--- /dev/null
+++ b/java/com/google/gerrit/httpd/HttpdModule.java
@@ -0,0 +1,14 @@
+package com.google.gerrit.httpd;
+
+import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+
+public class HttpdModule extends FactoryModule {
+ @Override
+ protected void configure() {
+ bind(GitReferenceUpdatedListener.class)
+ .annotatedWith(Exports.named(GitReferenceUpdatedTracker.class.getSimpleName()))
+ .to(GitReferenceUpdatedTracker.class);
+ }
+}
diff --git a/java/com/google/gerrit/httpd/WebSession.java b/java/com/google/gerrit/httpd/WebSession.java
index daf30ff..df8402e 100644
--- a/java/com/google/gerrit/httpd/WebSession.java
+++ b/java/com/google/gerrit/httpd/WebSession.java
@@ -16,30 +16,61 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AuthResult;
+import com.google.inject.servlet.RequestScoped;
+import java.util.List;
+import java.util.concurrent.CopyOnWriteArrayList;
-public interface WebSession {
- boolean isSignedIn();
+/**
+ * A thread safe class that contains details about a specific user web session.
+ *
+ * <p>WARNING: All implementors must have {@link RequestScoped} annotation to maintain thread
+ * safety.
+ */
+public abstract class WebSession {
+ public abstract boolean isSignedIn();
@Nullable
- String getXGerritAuth();
+ public abstract String getXGerritAuth();
- boolean isValidXGerritAuth(String keyIn);
+ public abstract boolean isValidXGerritAuth(String keyIn);
- CurrentUser getUser();
+ public abstract CurrentUser getUser();
- void login(AuthResult res, boolean rememberMe);
+ public abstract void login(AuthResult res, boolean rememberMe);
/** Set the user account for this current request only. */
- void setUserAccountId(Account.Id id);
+ public abstract void setUserAccountId(Account.Id id);
- boolean isAccessPathOk(AccessPath path);
+ public abstract boolean isAccessPathOk(AccessPath path);
- void setAccessPathOk(AccessPath path, boolean ok);
+ public abstract void setAccessPathOk(AccessPath path, boolean ok);
- void logout();
+ public abstract void logout();
- String getSessionId();
+ public abstract String getSessionId();
+
+ /**
+ * Store and return the ref updates in this session. This class is {@link RequestScoped}, hence
+ * this is thread safe.
+ *
+ * <p>The same session could perform separate requests one after another, so resetting the ref
+ * updates is necessary between requests.
+ */
+ private List<GitReferenceUpdatedListener.Event> refUpdatedEvents = new CopyOnWriteArrayList<>();
+
+ public List<GitReferenceUpdatedListener.Event> getRefUpdatedEvents() {
+ return refUpdatedEvents;
+ }
+
+ public void addRefUpdatedEvents(GitReferenceUpdatedListener.Event event) {
+ refUpdatedEvents.add(event);
+ }
+
+ public void resetRefUpdatedEvents() {
+ refUpdatedEvents.clear();
+ }
}
diff --git a/java/com/google/gerrit/httpd/init/WebAppInitializer.java b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
index 3a77a8a..079f306 100644
--- a/java/com/google/gerrit/httpd/init/WebAppInitializer.java
+++ b/java/com/google/gerrit/httpd/init/WebAppInitializer.java
@@ -28,6 +28,7 @@
import com.google.gerrit.httpd.GitOverHttpModule;
import com.google.gerrit.httpd.H2CacheBasedWebSession;
import com.google.gerrit.httpd.HttpCanonicalWebUrlProvider;
+import com.google.gerrit.httpd.HttpdModule;
import com.google.gerrit.httpd.RequestCleanupFilter;
import com.google.gerrit.httpd.RequestContextFilter;
import com.google.gerrit.httpd.RequestMetricsFilter;
@@ -393,6 +394,7 @@
modules.add(RequestMetricsFilter.module());
modules.add(sysInjector.getInstance(GerritAuthModule.class));
modules.add(sysInjector.getInstance(GitOverHttpModule.class));
+ modules.add(sysInjector.getInstance(HttpdModule.class));
modules.add(RequestCleanupFilter.module());
modules.add(SetThreadNameFilter.module());
modules.add(AllRequestFilter.module());
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 269d1c4..b50707b 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -66,6 +66,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.registration.DynamicSet;
@@ -97,6 +98,7 @@
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
+import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.httpd.WebSession;
import com.google.gerrit.httpd.restapi.ParameterParser.QueryParams;
import com.google.gerrit.json.OutputFormat;
@@ -204,6 +206,7 @@
private static final String FORM_TYPE = "application/x-www-form-urlencoded";
@VisibleForTesting public static final String X_GERRIT_TRACE = "X-Gerrit-Trace";
+ @VisibleForTesting public static final String X_GERRIT_UPDATED_REF = "X-Gerrit-UpdatedRef";
private static final String X_REQUESTED_WITH = "X-Requested-With";
private static final String X_GERRIT_AUTH = "X-Gerrit-Auth";
@@ -593,6 +596,8 @@
throw new ResourceNotFoundException();
}
+ setXGerritUpdatedRefResponseHeaders(req, res);
+
if (response instanceof Response.Redirect) {
CacheHeaders.setNotCacheable(res);
String location = ((Response.Redirect) response).location();
@@ -753,6 +758,33 @@
}
}
+ /**
+ * Fill in the refs that were updated during this request in the response header. The updated refs
+ * will be in the form of "project~ref~updated_SHA-1".
+ */
+ private void setXGerritUpdatedRefResponseHeaders(
+ HttpServletRequest request, HttpServletResponse response) {
+ for (GitReferenceUpdatedListener.Event refUpdate :
+ globals.webSession.get().getRefUpdatedEvents()) {
+ String refUpdateFormat =
+ String.format(
+ "%s~%s~%s~%s",
+ // encode the project and ref names since they may contain `~`
+ Url.encode(refUpdate.getProjectName()),
+ Url.encode(refUpdate.getRefName()),
+ refUpdate.getOldObjectId(),
+ refUpdate.getNewObjectId());
+
+ if (isRead(request)) {
+ logger.atWarning().log(
+ "request %s performed a ref update %s although the request is a READ request",
+ request.getRequestURL().toString(), refUpdateFormat);
+ }
+ response.addHeader(X_GERRIT_UPDATED_REF, refUpdateFormat);
+ }
+ globals.webSession.get().resetRefUpdatedEvents();
+ }
+
private String getEtagWithRetry(
HttpServletRequest req,
TraceContext traceContext,
diff --git a/java/com/google/gerrit/pgm/Daemon.java b/java/com/google/gerrit/pgm/Daemon.java
index 07bab24..29c5788 100644
--- a/java/com/google/gerrit/pgm/Daemon.java
+++ b/java/com/google/gerrit/pgm/Daemon.java
@@ -32,6 +32,7 @@
import com.google.gerrit.httpd.GitOverHttpModule;
import com.google.gerrit.httpd.H2CacheBasedWebSession;
import com.google.gerrit.httpd.HttpCanonicalWebUrlProvider;
+import com.google.gerrit.httpd.HttpdModule;
import com.google.gerrit.httpd.RequestCleanupFilter;
import com.google.gerrit.httpd.RequestContextFilter;
import com.google.gerrit.httpd.RequestMetricsFilter;
@@ -580,6 +581,7 @@
modules.add(H2CacheBasedWebSession.module());
modules.add(sysInjector.getInstance(GerritAuthModule.class));
modules.add(sysInjector.getInstance(GitOverHttpModule.class));
+ modules.add(sysInjector.getInstance(HttpdModule.class));
if (sshd) {
modules.add(new ProjectQoSFilter.Module());
}
diff --git a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
index bc45460..4d90492 100644
--- a/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/RestApiServletIT.java
@@ -14,15 +14,25 @@
package com.google.gerrit.acceptance.rest;
+import static com.google.common.net.HttpHeaders.ORIGIN;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.httpd.restapi.RestApiServlet.X_GERRIT_UPDATED_REF;
import static org.apache.http.HttpStatus.SC_OK;
import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.PushOneCommit.Result;
import com.google.gerrit.acceptance.RestResponse;
+import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.httpd.restapi.RestApiServlet;
import java.io.IOException;
+import java.util.List;
import java.util.regex.Pattern;
import org.apache.http.message.BasicHeader;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Repository;
import org.junit.Test;
public class RestApiServletIT extends AbstractDaemonTest {
@@ -61,6 +71,200 @@
.containsMatch(ANY_SPACE);
}
+ @Test
+ public void xGerritUpdatedRefNotSetForReadRequests() throws Exception {
+ RestResponse response = adminRestSession.getWithHeader(ANY_REST_API, ACCEPT_STAR_HEADER);
+ assertThat(response.getStatusCode()).isEqualTo(SC_OK);
+ assertThat(response.getHeader(X_GERRIT_UPDATED_REF)).isNull();
+ }
+
+ @Test
+ public void xGerritUpdatedRefSetForDifferentWriteRequests() throws Exception {
+ Result change = createChange();
+ String origin = adminRestSession.url();
+ String project = change.getChange().project().get();
+ String metaRef = RefNames.changeMetaRef(change.getChange().getId());
+
+ ObjectId originalMetaRefSha1 = getMetaRefSha1(change);
+
+ RestResponse response =
+ adminRestSession.putWithHeader(
+ "/changes/" + change.getChangeId() + "/topic", new BasicHeader(ORIGIN, origin), "A");
+ response.assertOK();
+ assertThat(gApi.changes().id(change.getChangeId()).topic()).isEqualTo("A");
+ ObjectId firstMetaRefSha1 = getMetaRefSha1(change);
+
+ // Meta ref updated because of topic update.
+ assertThat(response.getHeader(X_GERRIT_UPDATED_REF))
+ .isEqualTo(
+ String.format(
+ "%s~%s~%s~%s",
+ Url.encode(project),
+ Url.encode(metaRef),
+ originalMetaRefSha1.getName(),
+ firstMetaRefSha1.getName()));
+
+ response =
+ adminRestSession.putWithHeader(
+ "/changes/" + change.getChangeId() + "/topic", new BasicHeader(ORIGIN, origin), "B");
+ response.assertOK();
+ assertThat(gApi.changes().id(change.getChangeId()).topic()).isEqualTo("B");
+
+ ObjectId secondMetaRefSha1 = getMetaRefSha1(change);
+
+ // Meta ref updated again because of another topic update.
+ assertThat(response.getHeader(X_GERRIT_UPDATED_REF))
+ .isEqualTo(
+ String.format(
+ "%s~%s~%s~%s",
+ Url.encode(project),
+ Url.encode(metaRef),
+ firstMetaRefSha1.getName(),
+ secondMetaRefSha1.getName()));
+
+ // Ensure the meta ref SHA-1 changed for the project~metaRef which means we return different
+ // X-Gerrit-UpdatedRef headers.
+ assertThat(secondMetaRefSha1).isNotEqualTo(firstMetaRefSha1);
+ }
+
+ @Test
+ public void xGerritUpdatedRefDeleted() throws Exception {
+ Result change = createChange();
+ String project = change.getChange().project().get();
+ String metaRef = RefNames.changeMetaRef(change.getChange().getId());
+ String patchSetRef = RefNames.patchSetRef(change.getPatchSetId());
+
+ ObjectId originalMetaRefSha1 = getMetaRefSha1(change);
+ ObjectId originalchangeRefSha1 = change.getCommit().getId();
+
+ RestResponse response = adminRestSession.delete("/changes/" + change.getChangeId());
+ response.assertNoContent();
+
+ List<String> headers = response.getHeaders(X_GERRIT_UPDATED_REF);
+
+ // The change was deleted, so the refs were deleted which means they are ObjectId.zeroId().
+ assertThat(headers)
+ .containsExactly(
+ String.format(
+ "%s~%s~%s~%s",
+ Url.encode(project),
+ Url.encode(metaRef),
+ originalMetaRefSha1.getName(),
+ ObjectId.zeroId().getName()),
+ String.format(
+ "%s~%s~%s~%s",
+ Url.encode(project),
+ Url.encode(patchSetRef),
+ originalchangeRefSha1.getName(),
+ ObjectId.zeroId().getName()));
+ }
+
+ @Test
+ public void xGerritUpdatedRefWithProjectNameContainingTilde() throws Exception {
+ Project.NameKey project = createProjectOverAPI("~~pr~oje~ct~~~~", null, true, null);
+ Result change = createChange(cloneProject(project, admin));
+ String metaRef = RefNames.changeMetaRef(change.getChange().getId());
+ String patchSetRef = RefNames.patchSetRef(change.getPatchSetId());
+
+ ObjectId originalMetaRefSha1 = getMetaRefSha1(change);
+ ObjectId originalchangeRefSha1 = change.getCommit().getId();
+
+ RestResponse response = adminRestSession.delete("/changes/" + change.getChangeId());
+ response.assertNoContent();
+
+ List<String> headers = response.getHeaders(X_GERRIT_UPDATED_REF);
+
+ // The change was deleted, so the refs were deleted which means they are ObjectId.zeroId().
+ assertThat(headers)
+ .containsExactly(
+ String.format(
+ "%s~%s~%s~%s",
+ Url.encode(project.get()),
+ Url.encode(metaRef),
+ originalMetaRefSha1.getName(),
+ ObjectId.zeroId().getName()),
+ String.format(
+ "%s~%s~%s~%s",
+ Url.encode(project.get()),
+ Url.encode(patchSetRef),
+ originalchangeRefSha1.getName(),
+ ObjectId.zeroId().getName()));
+
+ // Ensures ~ gets encoded to %7E.
+ assertThat(Url.encode(project.get())).endsWith("%7E%7Epr%7Eoje%7Ect%7E%7E%7E%7E");
+ }
+
+ @Test
+ public void xGerritUpdatedRefSetMultipleHeadersForSubmit() throws Exception {
+ Result change1 = createChange();
+ Result change2 = createChange();
+ String metaRef1 = RefNames.changeMetaRef(change1.getChange().getId());
+ String metaRef2 = RefNames.changeMetaRef(change2.getChange().getId());
+
+ gApi.changes().id(change1.getChangeId()).current().review(ReviewInput.approve());
+ gApi.changes().id(change2.getChangeId()).current().review(ReviewInput.approve());
+
+ Project.NameKey project = change1.getChange().project();
+
+ try (Repository repository = repoManager.openRepository(project)) {
+ ObjectId originalFirstMetaRefSha1 = getMetaRefSha1(change1);
+ ObjectId originalSecondMetaRefSha1 = getMetaRefSha1(change2);
+ ObjectId originalDestinationBranchSha1 =
+ repository.resolve(change1.getChange().change().getDest().branch());
+
+ RestResponse response =
+ adminRestSession.post("/changes/" + change2.getChangeId() + "/submit");
+ response.assertOK();
+
+ ObjectId firstMetaRefSha1 = getMetaRefSha1(change1);
+ ObjectId secondMetaRefSha1 = getMetaRefSha1(change2);
+
+ List<String> headers = response.getHeaders(X_GERRIT_UPDATED_REF);
+
+ String branch = change1.getChange().change().getDest().branch();
+ String branchSha1 =
+ repository
+ .getRefDatabase()
+ .exactRef(change1.getChange().change().getDest().branch())
+ .getObjectId()
+ .name();
+
+ // During submit, all relevant meta refs of the latest patchset are updated + the destination
+ // branch/es.
+ // TODO(paiking): This doesn't work well for torn submissions: If the changes were in
+ // different projects in the same topic, and we tried to submit those changes together, it's
+ // possible that the first submission only submitted one of the changes, and then the retry
+ // submitted the other change. If that happens, when the user retries, they will not get the
+ // meta ref updates for the change that got submitted on the previous submission attempt.
+ // Ideally, submit should be idempotent and always return all meta refs on all submission
+ // attempts.
+ assertThat(headers)
+ .containsExactly(
+ String.format(
+ "%s~%s~%s~%s",
+ Url.encode(project.get()),
+ Url.encode(metaRef1),
+ originalFirstMetaRefSha1.getName(),
+ firstMetaRefSha1.getName()),
+ String.format(
+ "%s~%s~%s~%s",
+ Url.encode(project.get()),
+ Url.encode(metaRef2),
+ originalSecondMetaRefSha1.getName(),
+ secondMetaRefSha1.getName()),
+ String.format(
+ "%s~%s~%s~%s",
+ Url.encode(project.get()),
+ Url.encode(branch),
+ originalDestinationBranchSha1.getName(),
+ branchSha1));
+ }
+ }
+
+ private ObjectId getMetaRefSha1(Result change) {
+ return change.getChange().notes().getRevision();
+ }
+
private RestResponse prettyJsonRestResponse(String ppArgument, int ppValue) throws Exception {
RestResponse response =
adminRestSession.getWithHeader(
