Merge changes Ieb9d4fef,I8111b39d,I6883d976,I0145f451,Ie9dc1d14
* changes:
Check for RequestCancelledException in throwable chain
TraceTimer: Check at start and end whether the request is cancelled
Split off cancellation/ package from giant server package
RequestStateContext: Add method to abort cancelled requests
Add a context that allows to register RequestSateProviders
diff --git a/java/com/google/gerrit/httpd/BUILD b/java/com/google/gerrit/httpd/BUILD
index cd3ebb9..ea7c609 100644
--- a/java/com/google/gerrit/httpd/BUILD
+++ b/java/com/google/gerrit/httpd/BUILD
@@ -20,6 +20,7 @@
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/audit",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/git/receive",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index 7e6ab58..92e542a 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -712,50 +712,58 @@
messageOr(e, "Quota limit reached"),
e.caching(),
e);
- } catch (RequestCancelledException e) {
- cause = Optional.of(e);
- switch (e.getCancellationReason()) {
- case CLIENT_CLOSED_REQUEST:
- statusCode = SC_CLIENT_CLOSED_REQUEST;
- break;
- case CLIENT_PROVIDED_DEADLINE_EXCEEDED:
- case SERVER_DEADLINE_EXCEEDED:
- statusCode = SC_REQUEST_TIMEOUT;
- break;
- }
-
- StringBuilder msg = new StringBuilder(e.formatCancellationReason());
- if (e.getCancellationMessage().isPresent()) {
- msg.append("\n\n");
- msg.append(e.getCancellationMessage().get());
- }
-
- responseBytes = replyError(req, res, statusCode, msg.toString(), e);
} catch (Exception e) {
cause = Optional.of(e);
- statusCode = SC_INTERNAL_SERVER_ERROR;
- Optional<ExceptionHook.Status> status = getStatus(e);
- statusCode = status.map(ExceptionHook.Status::statusCode).orElse(SC_INTERNAL_SERVER_ERROR);
-
- if (res.isCommitted()) {
- responseBytes = 0;
- if (statusCode == SC_INTERNAL_SERVER_ERROR) {
- logger.atSevere().withCause(e).log(
- "Error in %s %s, response already committed", req.getMethod(), uriForLogging(req));
- } else {
- logger.atWarning().log(
- "Response for %s %s already committed, wanted to set status %d",
- req.getMethod(), uriForLogging(req), statusCode);
+ Optional<RequestCancelledException> requestCancelledException =
+ RequestCancelledException.getFromCausalChain(e);
+ if (requestCancelledException.isPresent()) {
+ switch (requestCancelledException.get().getCancellationReason()) {
+ case CLIENT_CLOSED_REQUEST:
+ statusCode = SC_CLIENT_CLOSED_REQUEST;
+ break;
+ case CLIENT_PROVIDED_DEADLINE_EXCEEDED:
+ case SERVER_DEADLINE_EXCEEDED:
+ statusCode = SC_REQUEST_TIMEOUT;
+ break;
}
- } else {
- res.reset();
- traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
- if (status.isPresent()) {
- responseBytes = reply(req, res, e, status.get(), getUserMessages(traceContext, e));
+ StringBuilder msg =
+ new StringBuilder(requestCancelledException.get().formatCancellationReason());
+ if (requestCancelledException.get().getCancellationMessage().isPresent()) {
+ msg.append("\n\n");
+ msg.append(requestCancelledException.get().getCancellationMessage().get());
+ }
+
+ responseBytes = replyError(req, res, statusCode, msg.toString(), e);
+ } else {
+ statusCode = SC_INTERNAL_SERVER_ERROR;
+
+ Optional<ExceptionHook.Status> status = getStatus(e);
+ statusCode =
+ status.map(ExceptionHook.Status::statusCode).orElse(SC_INTERNAL_SERVER_ERROR);
+
+ if (res.isCommitted()) {
+ responseBytes = 0;
+ if (statusCode == SC_INTERNAL_SERVER_ERROR) {
+ logger.atSevere().withCause(e).log(
+ "Error in %s %s, response already committed",
+ req.getMethod(), uriForLogging(req));
+ } else {
+ logger.atWarning().log(
+ "Response for %s %s already committed, wanted to set status %d",
+ req.getMethod(), uriForLogging(req), statusCode);
+ }
} else {
- responseBytes = replyInternalServerError(req, res, e, getUserMessages(traceContext, e));
+ res.reset();
+ traceContext.getTraceId().ifPresent(traceId -> res.addHeader(X_GERRIT_TRACE, traceId));
+
+ if (status.isPresent()) {
+ responseBytes = reply(req, res, e, status.get(), getUserMessages(traceContext, e));
+ } else {
+ responseBytes =
+ replyInternalServerError(req, res, e, getUserMessages(traceContext, e));
+ }
}
}
} finally {
diff --git a/java/com/google/gerrit/server/cancellation/BUILD b/java/com/google/gerrit/server/cancellation/BUILD
new file mode 100644
index 0000000..05530a5
--- /dev/null
+++ b/java/com/google/gerrit/server/cancellation/BUILD
@@ -0,0 +1,14 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+java_library(
+ name = "cancellation",
+ srcs = glob(
+ ["*.java"],
+ ),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//java/com/google/gerrit/common:annotations",
+ "//lib:guava",
+ "//lib/commons:lang",
+ ],
+)
diff --git a/java/com/google/gerrit/server/cancellation/RequestCancelledException.java b/java/com/google/gerrit/server/cancellation/RequestCancelledException.java
index 3c668fb..d89701f 100644
--- a/java/com/google/gerrit/server/cancellation/RequestCancelledException.java
+++ b/java/com/google/gerrit/server/cancellation/RequestCancelledException.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.cancellation;
+import com.google.common.base.Throwables;
import com.google.gerrit.common.Nullable;
import java.util.Optional;
import org.apache.commons.lang.WordUtils;
@@ -22,6 +23,17 @@
public class RequestCancelledException extends RuntimeException {
private static final long serialVersionUID = 1L;
+ /**
+ * Checks whether the given exception was caused by {@link RequestCancelledException}. If yes, the
+ * {@link RequestCancelledException} is returned. If not, {@link Optional#empty()} is returned.
+ */
+ public static Optional<RequestCancelledException> getFromCausalChain(Throwable e) {
+ return Throwables.getCausalChain(e).stream()
+ .filter(RequestCancelledException.class::isInstance)
+ .map(RequestCancelledException.class::cast)
+ .findFirst();
+ }
+
private final RequestStateProvider.Reason cancellationReason;
private final Optional<String> cancellationMessage;
diff --git a/java/com/google/gerrit/server/cancellation/RequestStateContext.java b/java/com/google/gerrit/server/cancellation/RequestStateContext.java
new file mode 100644
index 0000000..183d779
--- /dev/null
+++ b/java/com/google/gerrit/server/cancellation/RequestStateContext.java
@@ -0,0 +1,136 @@
+// 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.server.cancellation;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableSet;
+import java.util.HashSet;
+import java.util.Set;
+
+/**
+ * Context that allows to register {@link RequestStateProvider}s.
+ *
+ * <p>The registered {@link RequestStateProvider}s are stored in {@link ThreadLocal} so that they
+ * can be accessed during the request execution (via {@link #getRequestStateProviders()}.
+ *
+ * <p>On {@link #close()} the {@link RequestStateProvider}s that have been registered by this {@code
+ * RequestStateContext} instance are removed from {@link ThreadLocal}.
+ *
+ * <p>Nesting {@code RequestStateContext}s is possible.
+ *
+ * <p>Currently there is no logic to automatically copy the {@link RequestStateContext} to
+ * background threads, but implementing this may be considered in the future. This means that by
+ * default we only support cancellation of the main thread, but not of background threads. That's
+ * fine as all significant work is being done in the main thread.
+ *
+ * <p>{@link com.google.gerrit.server.util.RequestContext} is also a context that is available for
+ * the time of the request, but it is not suitable to manage registrations of {@link
+ * RequestStateProvider}s. Hence {@link RequestStateProvider} registrations are managed by a
+ * separate context, which is this class, {@link RequestStateContext}:
+ *
+ * <ul>
+ * <li>{@link com.google.gerrit.server.util.RequestContext} is an interface that has many
+ * implementations and hence cannot manage a {@link ThreadLocal} state.
+ * <li>{@link com.google.gerrit.server.util.RequestContext} is not an {@link AutoCloseable} and
+ * hence cannot cleanup any {@link ThreadLocal} state on close (turning it into an {@link
+ * AutoCloseable} would require a large refactoring).
+ * <li>Despite the name {@link com.google.gerrit.server.util.RequestContext} is not only used for
+ * requests scopes but also for other scopes that are not a request (e.g. plugin invocations,
+ * email sending, manual scopes).
+ * <li>{@link com.google.gerrit.server.util.RequestContext} is not copied to background and should
+ * not be, but for {@link RequestStateContext} we may consider doing this in the future.
+ * </ul>
+ */
+public class RequestStateContext implements AutoCloseable {
+ /** The {@link RequestStateProvider}s that have been registered for the thread. */
+ private static final ThreadLocal<Set<RequestStateProvider>> threadLocalRequestStateProviders =
+ new ThreadLocal<>();
+
+ /**
+ * Aborts the current request by throwing a {@link RequestCancelledException} if any of the
+ * registered {@link RequestStateProvider}s reports the request as cancelled.
+ *
+ * @throws RequestCancelledException thrown if the current request is cancelled and should be
+ * aborted
+ */
+ public static void abortIfCancelled() throws RequestCancelledException {
+ getRequestStateProviders()
+ .forEach(
+ requestStateProvider ->
+ requestStateProvider.checkIfCancelled(
+ (reason, message) -> {
+ throw new RequestCancelledException(reason, message);
+ }));
+ }
+
+ /** Returns the {@link RequestStateProvider}s that have been registered for the thread. */
+ @VisibleForTesting
+ static ImmutableSet<RequestStateProvider> getRequestStateProviders() {
+ if (threadLocalRequestStateProviders.get() == null) {
+ return ImmutableSet.of();
+ }
+ return ImmutableSet.copyOf(threadLocalRequestStateProviders.get());
+ }
+
+ /** Opens a {@code RequestStateContext}. */
+ public static RequestStateContext open() {
+ return new RequestStateContext();
+ }
+
+ /**
+ * The {@link RequestStateProvider}s that have been registered by this {@code
+ * RequestStateContext}.
+ */
+ private Set<RequestStateProvider> requestStateProviders = new HashSet<>();
+
+ private RequestStateContext() {}
+
+ /**
+ * Registers a {@link RequestStateProvider}.
+ *
+ * @param requestStateProvider the {@link RequestStateProvider} that should be registered
+ * @return the {@code RequestStateContext} instance for chaining calls
+ */
+ public RequestStateContext addRequestStateProvider(RequestStateProvider requestStateProvider) {
+ if (threadLocalRequestStateProviders.get() == null) {
+ threadLocalRequestStateProviders.set(new HashSet<>());
+ }
+ if (threadLocalRequestStateProviders.get().add(requestStateProvider)) {
+ requestStateProviders.add(requestStateProvider);
+ }
+ return this;
+ }
+
+ /**
+ * Closes this {@code RequestStateContext}.
+ *
+ * <p>Ensures that all {@link RequestStateProvider}s that have been registered by this {@code
+ * RequestStateContext} instance are removed from {@link #threadLocalRequestStateProviders}.
+ *
+ * <p>If no {@link RequestStateProvider}s remain in {@link #threadLocalRequestStateProviders},
+ * {@link #threadLocalRequestStateProviders} is unset.
+ */
+ @Override
+ public void close() {
+ if (threadLocalRequestStateProviders.get() != null) {
+ requestStateProviders.forEach(
+ requestStateProvider ->
+ threadLocalRequestStateProviders.get().remove(requestStateProvider));
+ if (threadLocalRequestStateProviders.get().isEmpty()) {
+ threadLocalRequestStateProviders.remove();
+ }
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/cancellation/RequestStateProvider.java b/java/com/google/gerrit/server/cancellation/RequestStateProvider.java
index e1716eb..683ca1d 100644
--- a/java/com/google/gerrit/server/cancellation/RequestStateProvider.java
+++ b/java/com/google/gerrit/server/cancellation/RequestStateProvider.java
@@ -31,6 +31,7 @@
void checkIfCancelled(OnCancelled onCancelled);
/** Callback interface to be invoked if a request is cancelled. */
+ @FunctionalInterface
interface OnCancelled {
/**
* Callback that is invoked if the request is cancelled.
diff --git a/java/com/google/gerrit/server/git/receive/BUILD b/java/com/google/gerrit/server/git/receive/BUILD
index b59d431..5c1cf52 100644
--- a/java/com/google/gerrit/server/git/receive/BUILD
+++ b/java/com/google/gerrit/server/git/receive/BUILD
@@ -18,6 +18,7 @@
"//java/com/google/gerrit/index",
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/logging",
"//java/com/google/gerrit/server/restapi",
"//java/com/google/gerrit/server/util/time",
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index d074f1e..29c2698 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -46,6 +46,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Splitter;
import com.google.common.base.Strings;
+import com.google.common.base.Throwables;
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
import com.google.common.collect.ImmutableList;
@@ -645,10 +646,18 @@
try {
processCommandsUnsafe(commands, progress);
rejectRemaining(commands, INTERNAL_SERVER_ERROR);
- } catch (RequestCancelledException e) {
- StringBuilder msg = new StringBuilder(e.formatCancellationReason());
- if (e.getCancellationMessage().isPresent()) {
- msg.append(String.format(" (%s)", e.getCancellationMessage().get()));
+ } catch (RuntimeException e) {
+ Optional<RequestCancelledException> requestCancelledException =
+ RequestCancelledException.getFromCausalChain(e);
+ if (!requestCancelledException.isPresent()) {
+ Throwables.throwIfUnchecked(e);
+ }
+ StringBuilder msg =
+ new StringBuilder(requestCancelledException.get().formatCancellationReason());
+ if (requestCancelledException.get().getCancellationMessage().isPresent()) {
+ msg.append(
+ String.format(
+ " (%s)", requestCancelledException.get().getCancellationMessage().get()));
}
rejectRemaining(commands, msg.toString());
}
diff --git a/java/com/google/gerrit/server/logging/BUILD b/java/com/google/gerrit/server/logging/BUILD
index c60af0d..ee0168c 100644
--- a/java/com/google/gerrit/server/logging/BUILD
+++ b/java/com/google/gerrit/server/logging/BUILD
@@ -9,6 +9,7 @@
deps = [
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/util/time",
"//lib:gson",
"//lib:guava",
diff --git a/java/com/google/gerrit/server/logging/TraceContext.java b/java/com/google/gerrit/server/logging/TraceContext.java
index 2fc19b5..681dfbc 100644
--- a/java/com/google/gerrit/server/logging/TraceContext.java
+++ b/java/com/google/gerrit/server/logging/TraceContext.java
@@ -24,6 +24,7 @@
import com.google.common.collect.Table;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
+import com.google.gerrit.server.cancellation.RequestStateContext;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
@@ -208,6 +209,7 @@
}
private TraceTimer(Runnable startLogFn, Consumer<Long> doneLogFn) {
+ RequestStateContext.abortIfCancelled();
startLogFn.run();
this.doneLogFn = doneLogFn;
this.stopwatch = Stopwatch.createStarted();
@@ -217,6 +219,7 @@
public void close() {
stopwatch.stop();
doneLogFn.accept(stopwatch.elapsed(TimeUnit.MILLISECONDS));
+ RequestStateContext.abortIfCancelled();
}
}
diff --git a/java/com/google/gerrit/sshd/BUILD b/java/com/google/gerrit/sshd/BUILD
index 0668c1e..f3bd5e1 100644
--- a/java/com/google/gerrit/sshd/BUILD
+++ b/java/com/google/gerrit/sshd/BUILD
@@ -17,6 +17,7 @@
"//java/com/google/gerrit/metrics",
"//java/com/google/gerrit/server",
"//java/com/google/gerrit/server/audit",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/git/receive",
"//java/com/google/gerrit/server/ioutil",
"//java/com/google/gerrit/server/logging",
diff --git a/java/com/google/gerrit/sshd/SshCommand.java b/java/com/google/gerrit/sshd/SshCommand.java
index 93c6c2c..e58040f 100644
--- a/java/com/google/gerrit/sshd/SshCommand.java
+++ b/java/com/google/gerrit/sshd/SshCommand.java
@@ -14,6 +14,7 @@
package com.google.gerrit.sshd;
+import com.google.common.base.Throwables;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.DynamicOptions;
@@ -28,6 +29,7 @@
import com.google.inject.Inject;
import java.io.IOException;
import java.io.PrintWriter;
+import java.util.Optional;
import org.apache.sshd.server.Environment;
import org.apache.sshd.server.channel.ChannelSession;
import org.eclipse.jgit.lib.Config;
@@ -62,10 +64,18 @@
RequestInfo.builder(RequestInfo.RequestType.SSH, user, traceContext).build();
requestListeners.runEach(l -> l.onRequest(requestInfo));
SshCommand.this.run();
- } catch (RequestCancelledException e) {
- StringBuilder msg = new StringBuilder(e.formatCancellationReason());
- if (e.getCancellationMessage().isPresent()) {
- msg.append(String.format(" (%s)", e.getCancellationMessage().get()));
+ } catch (RuntimeException e) {
+ Optional<RequestCancelledException> requestCancelledException =
+ RequestCancelledException.getFromCausalChain(e);
+ if (!requestCancelledException.isPresent()) {
+ Throwables.throwIfUnchecked(e);
+ }
+ StringBuilder msg =
+ new StringBuilder(requestCancelledException.get().formatCancellationReason());
+ if (requestCancelledException.get().getCancellationMessage().isPresent()) {
+ msg.append(
+ String.format(
+ " (%s)", requestCancelledException.get().getCancellationMessage().get()));
}
stderr.println(msg.toString());
} finally {
diff --git a/javatests/com/google/gerrit/acceptance/rest/BUILD b/javatests/com/google/gerrit/acceptance/rest/BUILD
index 84887da..a62d551 100644
--- a/javatests/com/google/gerrit/acceptance/rest/BUILD
+++ b/javatests/com/google/gerrit/acceptance/rest/BUILD
@@ -5,6 +5,7 @@
group = "rest_bindings_collection",
labels = ["rest"],
deps = [
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/logging",
],
)
diff --git a/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java b/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
index 29d54cc..0713fe6 100644
--- a/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/CancellationIT.java
@@ -121,6 +121,27 @@
}
@Test
+ public void handleWrappedRequestCancelledException() throws Exception {
+ ProjectCreationValidationListener projectCreationListener =
+ new ProjectCreationValidationListener() {
+ @Override
+ public void validateNewProject(CreateProjectArgs args) throws ValidationException {
+ // Simulate an exceeded deadline by throwing RequestCancelledException.
+ throw new RuntimeException(
+ new RequestCancelledException(
+ RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED, "deadline = 10m"));
+ }
+ };
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ RestResponse response = adminRestSession.put("/projects/" + name("new"));
+ assertThat(response.getStatusCode()).isEqualTo(SC_REQUEST_TIMEOUT);
+ assertThat(response.getEntityContent())
+ .isEqualTo("Server Deadline Exceeded\n\ndeadline = 10m");
+ }
+ }
+
+ @Test
public void handleClientDisconnectedForPush() throws Exception {
CommitValidationListener commitValidationListener =
new CommitValidationListener() {
@@ -185,6 +206,27 @@
}
@Test
+ public void handleWrappedRequestCancelledExceptionForPush() throws Exception {
+ CommitValidationListener commitValidationListener =
+ new CommitValidationListener() {
+ @Override
+ public List<CommitValidationMessage> onCommitReceived(CommitReceivedEvent receiveEvent)
+ throws CommitValidationException {
+ // Simulate an exceeded deadline by throwing RequestCancelledException.
+ throw new RuntimeException(
+ new RequestCancelledException(
+ RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED, "deadline = 10m"));
+ }
+ };
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(commitValidationListener)) {
+ PushOneCommit push = pushFactory.create(admin.newIdent(), testRepo);
+ PushOneCommit.Result r = push.to("refs/heads/master");
+ r.assertErrorStatus("Server Deadline Exceeded (deadline = 10m)");
+ }
+ }
+
+ @Test
public void handleRequestCancellationWithMessageForPush() throws Exception {
CommitValidationListener commitValidationListener =
new CommitValidationListener() {
diff --git a/javatests/com/google/gerrit/acceptance/ssh/BUILD b/javatests/com/google/gerrit/acceptance/ssh/BUILD
index 5634322..ee1b221 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/BUILD
+++ b/javatests/com/google/gerrit/acceptance/ssh/BUILD
@@ -18,6 +18,7 @@
vm_args = ["-Xmx512m"],
deps = [
":util",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/logging",
"//lib/commons:compress",
],
diff --git a/javatests/com/google/gerrit/acceptance/ssh/SshCancellationIT.java b/javatests/com/google/gerrit/acceptance/ssh/SshCancellationIT.java
index 2cb9637..3e31c16 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/SshCancellationIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/SshCancellationIT.java
@@ -99,4 +99,22 @@
adminSshSession.assertFailure("Server Deadline Exceeded (deadline = 10m)");
}
}
+
+ @Test
+ public void handleWrappedRequestCancelledException() throws Exception {
+ ProjectCreationValidationListener projectCreationListener =
+ new ProjectCreationValidationListener() {
+ @Override
+ public void validateNewProject(CreateProjectArgs args) throws ValidationException {
+ throw new RuntimeException(
+ new RequestCancelledException(
+ RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED, "deadline = 10m"));
+ }
+ };
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(projectCreationListener)) {
+ adminSshSession.exec("gerrit create-project " + name("new"));
+ adminSshSession.assertFailure("Server Deadline Exceeded (deadline = 10m)");
+ }
+ }
}
diff --git a/javatests/com/google/gerrit/server/BUILD b/javatests/com/google/gerrit/server/BUILD
index 7ab7ae9..a2825322 100644
--- a/javatests/com/google/gerrit/server/BUILD
+++ b/javatests/com/google/gerrit/server/BUILD
@@ -55,6 +55,7 @@
"//java/com/google/gerrit/server/account/externalids/testing",
"//java/com/google/gerrit/server/cache/serialize",
"//java/com/google/gerrit/server/cache/testing",
+ "//java/com/google/gerrit/server/cancellation",
"//java/com/google/gerrit/server/fixes/testing",
"//java/com/google/gerrit/server/git/receive:ref_cache",
"//java/com/google/gerrit/server/ioutil",
diff --git a/javatests/com/google/gerrit/server/cancellation/RequestStateContextTest.java b/javatests/com/google/gerrit/server/cancellation/RequestStateContextTest.java
new file mode 100644
index 0000000..e2c43eb
--- /dev/null
+++ b/javatests/com/google/gerrit/server/cancellation/RequestStateContextTest.java
@@ -0,0 +1,178 @@
+// 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.server.cancellation;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth8.assertThat;
+import static com.google.gerrit.testing.GerritJUnit.assertThrows;
+
+import com.google.common.collect.ImmutableSet;
+import org.junit.Test;
+
+public class RequestStateContextTest {
+ @Test
+ public void openContext() {
+ assertNoRequestStateProviders();
+
+ RequestStateProvider requestStateProvider1 = new TestRequestStateProvider();
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider1)) {
+ RequestStateProvider requestStateProvider2 = new TestRequestStateProvider();
+ requestStateContext.addRequestStateProvider(requestStateProvider2);
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+ }
+
+ assertNoRequestStateProviders();
+ }
+
+ @Test
+ public void openNestedContexts() {
+ assertNoRequestStateProviders();
+
+ RequestStateProvider requestStateProvider1 = new TestRequestStateProvider();
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider1)) {
+ RequestStateProvider requestStateProvider2 = new TestRequestStateProvider();
+ requestStateContext.addRequestStateProvider(requestStateProvider2);
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+
+ RequestStateProvider requestStateProvider3 = new TestRequestStateProvider();
+ try (RequestStateContext requestStateContext2 =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider3)) {
+ RequestStateProvider requestStateProvider4 = new TestRequestStateProvider();
+ requestStateContext2.addRequestStateProvider(requestStateProvider4);
+ assertRequestStateProviders(
+ ImmutableSet.of(
+ requestStateProvider1,
+ requestStateProvider2,
+ requestStateProvider3,
+ requestStateProvider4));
+ }
+
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+ }
+
+ assertNoRequestStateProviders();
+ }
+
+ @Test
+ public void openNestedContextsWithSameRequestStateProviders() {
+ assertNoRequestStateProviders();
+
+ RequestStateProvider requestStateProvider1 = new TestRequestStateProvider();
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider1)) {
+ RequestStateProvider requestStateProvider2 = new TestRequestStateProvider();
+ requestStateContext.addRequestStateProvider(requestStateProvider2);
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+
+ try (RequestStateContext requestStateContext2 =
+ RequestStateContext.open().addRequestStateProvider(requestStateProvider1)) {
+ requestStateContext2.addRequestStateProvider(requestStateProvider2);
+
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+ }
+
+ assertRequestStateProviders(ImmutableSet.of(requestStateProvider1, requestStateProvider2));
+ }
+
+ assertNoRequestStateProviders();
+ }
+
+ @Test
+ public void abortIfCancelled_noRequestStateProvider() {
+ assertNoRequestStateProviders();
+
+ // Calling abortIfCancelled() shouldn't throw an exception.
+ RequestStateContext.abortIfCancelled();
+ }
+
+ @Test
+ public void abortIfCancelled_requestNotCancelled() {
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open()
+ .addRequestStateProvider(
+ new RequestStateProvider() {
+ @Override
+ public void checkIfCancelled(OnCancelled onCancelled) {}
+ })) {
+ // Calling abortIfCancelled() shouldn't throw an exception.
+ RequestStateContext.abortIfCancelled();
+ }
+ }
+
+ @Test
+ public void abortIfCancelled_requestCancelled() {
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open()
+ .addRequestStateProvider(
+ new RequestStateProvider() {
+ @Override
+ public void checkIfCancelled(OnCancelled onCancelled) {
+ onCancelled.onCancel(
+ RequestStateProvider.Reason.CLIENT_CLOSED_REQUEST, /* message= */ null);
+ }
+ })) {
+ RequestCancelledException requestCancelledException =
+ assertThrows(
+ RequestCancelledException.class, () -> RequestStateContext.abortIfCancelled());
+ assertThat(requestCancelledException)
+ .hasMessageThat()
+ .isEqualTo("Request cancelled: CLIENT_CLOSED_REQUEST");
+ assertThat(requestCancelledException.getCancellationReason())
+ .isEqualTo(RequestStateProvider.Reason.CLIENT_CLOSED_REQUEST);
+ assertThat(requestCancelledException.getCancellationMessage()).isEmpty();
+ }
+ }
+
+ @Test
+ public void abortIfCancelled_requestCancelled_withMessage() {
+ try (RequestStateContext requestStateContext =
+ RequestStateContext.open()
+ .addRequestStateProvider(
+ new RequestStateProvider() {
+ @Override
+ public void checkIfCancelled(OnCancelled onCancelled) {
+ onCancelled.onCancel(
+ RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED, "deadline = 10m");
+ }
+ })) {
+ RequestCancelledException requestCancelledException =
+ assertThrows(
+ RequestCancelledException.class, () -> RequestStateContext.abortIfCancelled());
+ assertThat(requestCancelledException)
+ .hasMessageThat()
+ .isEqualTo("Request cancelled: SERVER_DEADLINE_EXCEEDED (deadline = 10m)");
+ assertThat(requestCancelledException.getCancellationReason())
+ .isEqualTo(RequestStateProvider.Reason.SERVER_DEADLINE_EXCEEDED);
+ assertThat(requestCancelledException.getCancellationMessage()).hasValue("deadline = 10m");
+ }
+ }
+
+ private void assertNoRequestStateProviders() {
+ assertRequestStateProviders(ImmutableSet.of());
+ }
+
+ private void assertRequestStateProviders(
+ ImmutableSet<RequestStateProvider> expectedRequestStateProviders) {
+ assertThat(RequestStateContext.getRequestStateProviders())
+ .containsExactlyElementsIn(expectedRequestStateProviders);
+ }
+
+ private static class TestRequestStateProvider implements RequestStateProvider {
+ @Override
+ public void checkIfCancelled(OnCancelled onCancelled) {}
+ }
+}
