Port PutTopic to use the BatchUpdate interface

Change-Id: I776c8e605610b48e0054921b77053255f037f871

gerrit-server/src/main/java/com/google/gerrit/server/api/changes/ChangeApiImpl.java

@@ -220,7 +220,7 @@ class ChangeApiImpl implements ChangeApi {
     in.topic = topic;
     try {
       putTopic.apply(change, in);
-    } catch (OrmException | IOException e) {
+    } catch (OrmException | IOException | UpdateException e) {
       throw new RestApiException("Cannot set topic", e);
     }
   }

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

@@ -19,7 +19,9 @@ import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.DefaultInput;
+import com.google.gerrit.extensions.restapi.ResourceConflictException;
 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;
@@ -29,25 +31,29 @@ import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.change.PutTopic.Input;
-import com.google.gerrit.server.index.ChangeIndexer;
+import com.google.gerrit.server.git.BatchUpdate;
+import com.google.gerrit.server.git.BatchUpdate.ChangeOp;
+import com.google.gerrit.server.git.UpdateException;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.project.ChangeControl;
-import com.google.gwtorm.server.AtomicUpdate;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import java.io.IOException;
+import java.sql.Timestamp;
+import java.util.Collections;
+import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicReference;
 
 @Singleton
 public class PutTopic implements RestModifyView<ChangeResource, Input>,
     UiAction<ChangeResource> {
   private final Provider<ReviewDb> dbProvider;
-  private final ChangeIndexer indexer;
   private final ChangeHooks hooks;
-  private final ChangeUpdate.Factory updateFactory;
   private final ChangeMessagesUtil cmUtil;
+  private final BatchUpdate.Factory batchUpdateFactory;
 
   public static class Input {
     @DefaultInput
@@ -55,80 +61,87 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>,
   }
 
   @Inject
-  PutTopic(Provider<ReviewDb> dbProvider, ChangeIndexer indexer,
+  PutTopic(Provider<ReviewDb> dbProvider,
-      ChangeHooks hooks, ChangeUpdate.Factory updateFactory,
+      ChangeHooks hooks,
-      ChangeMessagesUtil cmUtil) {
+      ChangeMessagesUtil cmUtil,
+      BatchUpdate.Factory batchUpdateFactory) {
     this.dbProvider = dbProvider;
-    this.indexer = indexer;
     this.hooks = hooks;
-    this.updateFactory = updateFactory;
     this.cmUtil = cmUtil;
+    this.batchUpdateFactory = batchUpdateFactory;
   }
 
   @Override
   public Response<String> apply(ChangeResource req, Input input)
-      throws AuthException, OrmException, IOException {
+      throws AuthException, UpdateException, RestApiException, OrmException,
+      IOException {
     if (input == null) {
       input = new Input();
     }
+    final String inputTopic = input.topic;
 
     ChangeControl control = req.getControl();
-    Change change = req.getChange();
     if (!control.canEditTopicName()) {
       throw new AuthException("changing topic not permitted");
     }
 
-    ReviewDb db = dbProvider.get();
+    final Change.Id id = req.getChange().getId();
-    final String newTopicName = Strings.nullToEmpty(input.topic);
+    final IdentifiedUser caller = (IdentifiedUser) control.getCurrentUser();
-    String oldTopicName = Strings.nullToEmpty(change.getTopic());
+    final AtomicReference<Change> change = new AtomicReference<>();
-    if (!oldTopicName.equals(newTopicName)) {
+    final AtomicReference<String> oldTopicName = new AtomicReference<>();
-      String summary;
+    final AtomicReference<String> newTopicName = new AtomicReference<>();
-      if (oldTopicName.isEmpty()) {
-        summary = "Topic set to " + newTopicName;
-      } else if (newTopicName.isEmpty()) {
-        summary = "Topic " + oldTopicName + " removed";
-      } else {
-        summary = String.format(
-            "Topic changed from %s to %s",
-            oldTopicName, newTopicName);
-      }
 
-      IdentifiedUser currentUser = ((IdentifiedUser) control.getCurrentUser());
+    final Timestamp now = TimeUtil.nowTs();
-      ChangeMessage cmsg = new ChangeMessage(
+    try (BatchUpdate u = batchUpdateFactory.create(dbProvider.get(),
-          new ChangeMessage.Key(change.getId(), ChangeUtil.messageUUID(db)),
+        req.getChange().getProject(), now)) {
-          currentUser.getAccountId(), TimeUtil.nowTs(),
+      u.addChangeOp(new ChangeOp(req.getControl()) {
-          change.currentPatchSetId());
-      cmsg.setMessage(summary);
-      ChangeUpdate update;
-
-      db.changes().beginTransaction(change.getId());
-      try {
-        change = db.changes().atomicUpdate(change.getId(),
-          new AtomicUpdate<Change>() {
         @Override
-            public Change update(Change change) {
+        public void call(ReviewDb db, ChangeUpdate update) throws OrmException,
-              change.setTopic(Strings.emptyToNull(newTopicName));
+            ResourceConflictException {
-              ChangeUtil.updated(change);
+          Change c = db.changes().get(id);
-              return change;
+          String n = Strings.nullToEmpty(inputTopic);
+          String o = Strings.nullToEmpty(c.getTopic());
+          if (o.equals(n)) {
+            return;
           }
-          });
+          String summary;
+          if (o.isEmpty()) {
+            summary = "Topic set to " + n;
+          } else if (n.isEmpty()) {
+            summary = "Topic " + o + " removed";
+          } else {
+            summary = String.format("Topic changed from %s to %s", o, n);
+          }
+          c.setTopic(Strings.emptyToNull(n));
+          ChangeUtil.updated(c);
+          db.changes().update(Collections.singleton(c));
 
-        //TODO(yyonas): atomic update was not propagated
+          ChangeMessage cmsg = new ChangeMessage(
-        update = updateFactory.create(control);
+              new ChangeMessage.Key(id, ChangeUtil.messageUUID(db)),
+              caller.getAccountId(), now, c.currentPatchSetId());
+          cmsg.setMessage(summary);
           cmUtil.addChangeMessage(db, update, cmsg);
 
-        db.commit();
+          change.set(c);
-      } finally {
+          oldTopicName.set(o);
-        db.rollback();
+          newTopicName.set(n);
         }
-      update.commit();
+      });
-      indexer.index(db, change);
+      u.addPostOp(new Callable<Void>() {
-      hooks.doTopicChangedHook(change, currentUser.getAccount(),
+        @Override
-          oldTopicName, db);
+        public Void call() throws OrmException {
+          Change c = change.get();
+          if (c != null) {
+            hooks.doTopicChangedHook(change.get(), caller.getAccount(),
+                oldTopicName.get(), dbProvider.get());
           }
-    return Strings.isNullOrEmpty(newTopicName)
+          return null;
-        ? Response.<String>none()
+        }
-        : Response.ok(newTopicName);
+      });
+      u.execute();
+    }
+    String n = newTopicName.get();
+    return Strings.isNullOrEmpty(n) ? Response.<String> none() : Response.ok(n);
   }
 
   @Override