Add a convenience class for making REST API handlers retryable

We want to get all callers of BatchUpdate to use the new RetryHelper,
but explicitly passing a lambda to the RetryHelper instance in every
case is going to be a lot of boilerplate. Add some sugar for the most
common case, a REST API handler. Using the RetryingRestModifyView,
converting a handler to be retry-friendly is as simple as adding the
base class and changing a method name (and some minor cleanup like
removing a BatchUpdate.Factory field and twiddling exceptions).

Adding an intervening abstract class requires a tweak to the way
RestApiServlet extracts the input type using reflection. Fortunately,
there is a TypeLiteral method that does exactly what we want.

Change-Id: If2b78f0d556a49972ea9976057dd5f9dc21ec750

gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.httpd.restapi;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS;
 import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
 import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS;
@@ -115,6 +116,7 @@ import com.google.gson.stream.MalformedJsonException;
 import com.google.gwtexpui.server.CacheHeaders;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
+import com.google.inject.TypeLiteral;
 import com.google.inject.util.Providers;
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
@@ -638,40 +640,21 @@ public class RestApiServlet extends HttpServlet {
   }
 
   private static Type inputType(RestModifyView<RestResource, Object> m) {
-    Type inputType = extractInputType(m.getClass());
+    // MyModifyView implements RestModifyView<SomeResource, MyInput>
-    if (inputType == null) {
+    TypeLiteral<?> typeLiteral = TypeLiteral.get(m.getClass());
-      throw new IllegalStateException(
-          String.format(
-              "View %s does not correctly implement %s",
-              m.getClass(), RestModifyView.class.getSimpleName()));
-    }
-    return inputType;
-  }
 
-  @SuppressWarnings("rawtypes")
+    // RestModifyView<SomeResource, MyInput>
-  private static Type extractInputType(Class clazz) {
+    // This is smart enough to resolve even when there are intervening subclasses, even if they have
-    for (Type t : clazz.getGenericInterfaces()) {
+    // reordered type arguments.
-      if (t instanceof ParameterizedType
+    TypeLiteral<?> supertypeLiteral = typeLiteral.getSupertype(RestModifyView.class);
-          && ((ParameterizedType) t).getRawType() == RestModifyView.class) {
-        return ((ParameterizedType) t).getActualTypeArguments()[1];
-      }
-    }
 
-    if (clazz.getSuperclass() != null) {
+    Type supertype = supertypeLiteral.getType();
-      Type i = extractInputType(clazz.getSuperclass());
+    checkState(
-      if (i != null) {
+        supertype instanceof ParameterizedType,
-        return i;
+        "supertype of %s is not parameterized: %s",
-      }
+        typeLiteral,
-    }
+        supertypeLiteral);
-
+    return ((ParameterizedType) supertype).getActualTypeArguments()[1];
-    for (Class t : clazz.getInterfaces()) {
-      Type i = extractInputType(t);
-      if (i != null) {
-        return i;
-      }
-    }
-
-    return null;
   }
 
   private Object parseRequest(HttpServletRequest req, Type type)

gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java

@@ -19,7 +19,6 @@ import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.restapi.DefaultInput;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.webui.UiAction;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
@@ -34,6 +33,8 @@ import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.BatchUpdateOp;
 import com.google.gerrit.server.update.ChangeContext;
 import com.google.gerrit.server.update.Context;
+import com.google.gerrit.server.update.RetryHelper;
+import com.google.gerrit.server.update.RetryingRestModifyView;
 import com.google.gerrit.server.update.UpdateException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
@@ -41,10 +42,10 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 @Singleton
-public class PutTopic implements RestModifyView<ChangeResource, Input>, UiAction<ChangeResource> {
+public class PutTopic extends RetryingRestModifyView<ChangeResource, Input, Response<String>>
+    implements UiAction<ChangeResource> {
   private final Provider<ReviewDb> dbProvider;
   private final ChangeMessagesUtil cmUtil;
-  private final BatchUpdate.Factory batchUpdateFactory;
   private final TopicEdited topicEdited;
 
   public static class Input {
@@ -55,29 +56,27 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>, UiAction
   PutTopic(
       Provider<ReviewDb> dbProvider,
       ChangeMessagesUtil cmUtil,
-      BatchUpdate.Factory batchUpdateFactory,
+      RetryHelper retryHelper,
       TopicEdited topicEdited) {
+    super(retryHelper);
     this.dbProvider = dbProvider;
     this.cmUtil = cmUtil;
-    this.batchUpdateFactory = batchUpdateFactory;
     this.topicEdited = topicEdited;
   }
 
   @Override
-  public Response<String> apply(ChangeResource req, Input input)
+  protected Response<String> applyImpl(
+      BatchUpdate.Factory updateFactory, ChangeResource req, Input input)
       throws UpdateException, RestApiException, PermissionBackendException {
     req.permissions().check(ChangePermission.EDIT_TOPIC_NAME);
-
     Op op = new Op(input != null ? input : new Input());
     try (BatchUpdate u =
-        batchUpdateFactory.create(
+        updateFactory.create(
             dbProvider.get(), req.getChange().getProject(), req.getUser(), TimeUtil.nowTs())) {
       u.addOp(req.getId(), op);
       u.execute();
     }
-    return Strings.isNullOrEmpty(op.newTopicName)
+    return Strings.isNullOrEmpty(op.newTopicName) ? Response.none() : Response.ok(op.newTopicName);
-        ? Response.<String>none()
-        : Response.ok(op.newTopicName);
   }
 
   private class Op implements BatchUpdateOp {

gerrit-server/src/main/java/com/google/gerrit/server/update/RetryingRestModifyView.java

@@ -0,0 +1,35 @@
+// 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.server.update;
+
+import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.RestResource;
+
+public abstract class RetryingRestModifyView<R extends RestResource, I, O>
+    implements RestModifyView<R, I> {
+  private final RetryHelper retryHelper;
+
+  protected RetryingRestModifyView(RetryHelper retryHelper) {
+    this.retryHelper = retryHelper;
+  }
+
+  @Override
+  public final O apply(R resource, I input) throws Exception {
+    return retryHelper.execute((updateFactory) -> applyImpl(updateFactory, resource, input));
+  }
+
+  protected abstract O applyImpl(BatchUpdate.Factory updateFactory, R resource, I input)
+      throws Exception;
+}