Merge "Convert draft comment endpoints to BatchUpdate"

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

@@ -22,12 +22,11 @@ import com.google.gerrit.server.change.DeleteDraftComment;
 import com.google.gerrit.server.change.DraftCommentResource;
 import com.google.gerrit.server.change.GetDraftComment;
 import com.google.gerrit.server.change.PutDraftComment;
+import com.google.gerrit.server.git.UpdateException;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.assistedinject.Assisted;
 
-import java.io.IOException;
-
 class DraftApiImpl implements DraftApi {
   interface Factory {
     DraftApiImpl create(DraftCommentResource d);
@@ -62,7 +61,7 @@ class DraftApiImpl implements DraftApi {
   public CommentInfo update(DraftInput in) throws RestApiException {
     try {
       return putDraft.apply(draft, in).value();
-    } catch (IOException | OrmException e) {
+    } catch (UpdateException | OrmException e) {
       throw new RestApiException("Cannot update draft", e);
     }
   }
@@ -71,7 +70,7 @@ class DraftApiImpl implements DraftApi {
   public void delete() throws RestApiException {
     try {
       deleteDraft.apply(draft, null);
-    } catch (IOException | OrmException e) {
+    } catch (UpdateException e) {
       throw new RestApiException("Cannot delete draft", e);
     }
   }

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

@@ -347,7 +347,7 @@ class RevisionApiImpl implements RevisionApi {
       return changes.id(revision.getChange().getId().get())
           .revision(revision.getPatchSet().getId().get())
           .draft(id);
-    } catch (IOException | OrmException e) {
+    } catch (UpdateException | OrmException e) {
       throw new RestApiException("Cannot create draft", e);
     }
   }

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

@@ -22,36 +22,39 @@ import com.google.gerrit.extensions.api.changes.DraftInput;
 import com.google.gerrit.extensions.client.Side;
 import com.google.gerrit.extensions.common.CommentInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 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.restapi.Url;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.PatchLineCommentsUtil;
-import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.git.BatchUpdate;
+import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
+import com.google.gerrit.server.git.UpdateException;
 import com.google.gerrit.server.patch.PatchListCache;
 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;
 
 @Singleton
 public class CreateDraftComment implements RestModifyView<RevisionResource, DraftInput> {
   private final Provider<ReviewDb> db;
-  private final ChangeUpdate.Factory updateFactory;
+  private final BatchUpdate.Factory updateFactory;
   private final Provider<CommentJson> commentJson;
   private final PatchLineCommentsUtil plcUtil;
   private final PatchListCache patchListCache;
 
   @Inject
   CreateDraftComment(Provider<ReviewDb> db,
-      ChangeUpdate.Factory updateFactory,
+      BatchUpdate.Factory updateFactory,
       Provider<CommentJson> commentJson,
       PatchLineCommentsUtil plcUtil,
       PatchListCache patchListCache) {
@@ -64,7 +67,7 @@ public class CreateDraftComment implements RestModifyView<RevisionResource, Draf
 
   @Override
   public Response<CommentInfo> apply(RevisionResource rsrc, DraftInput in)
-      throws BadRequestException, OrmException, IOException {
+      throws RestApiException, UpdateException, OrmException {
     if (Strings.isNullOrEmpty(in.path)) {
       throw new BadRequestException("path must be non-empty");
     } else if (in.message == null || in.message.trim().isEmpty()) {
@@ -75,24 +78,50 @@ public class CreateDraftComment implements RestModifyView<RevisionResource, Draf
       throw new BadRequestException("range endLine must be on the same line as the comment");
     }
 
-    int line = in.line != null
+    try (BatchUpdate bu = updateFactory.create(
-        ? in.line
+        db.get(), rsrc.getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
-        : in.range != null ? in.range.endLine : 0;
+      Op op = new Op(rsrc.getPatchSet().getId(), in);
+      bu.addOp(rsrc.getChange().getId(), op);
+      bu.execute();
+      return Response.created(
+          commentJson.get().setFillAccounts(false).format(op.comment));
+    }
+  }
 
-    Timestamp now = TimeUtil.nowTs();
+  private class Op extends BatchUpdate.Op {
-    ChangeUpdate update = updateFactory.create(rsrc.getControl(), now);
+    private final PatchSet.Id psId;
+    private final DraftInput in;
 
-    PatchLineComment c = new PatchLineComment(
+    private PatchLineComment comment;
-        new PatchLineComment.Key(
+
-            new Patch.Key(rsrc.getPatchSet().getId(), in.path),
+    private Op(PatchSet.Id psId, DraftInput in) {
-            ChangeUtil.messageUUID(db.get())),
+      this.psId = psId;
-        line, rsrc.getAccountId(), Url.decode(in.inReplyTo), now);
+      this.in = in;
-    c.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
+    }
-    c.setMessage(in.message.trim());
+
-    c.setRange(in.range);
+    @Override
-    setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
+    public void updateChange(ChangeContext ctx)
-    plcUtil.insertComments(db.get(), update, Collections.singleton(c));
+        throws ResourceNotFoundException, OrmException {
-    update.commit();
+      PatchSet ps = ctx.getDb().patchSets().get(psId);
-    return Response.created(commentJson.get().setFillAccounts(false).format(c));
+      if (ps == null) {
+        throw new ResourceNotFoundException("patch set not found: " + psId);
+      }
+      int line = in.line != null
+          ? in.line
+          : in.range != null ? in.range.endLine : 0;
+      comment = new PatchLineComment(
+          new PatchLineComment.Key(
+              new Patch.Key(ps.getId(), in.path),
+              ChangeUtil.messageUUID(ctx.getDb())),
+          line, ctx.getUser().getAccountId(), Url.decode(in.inReplyTo),
+          ctx.getWhen());
+      comment.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
+      comment.setMessage(in.message.trim());
+      comment.setRange(in.range);
+      setCommentRevId(
+          comment, patchListCache, ctx.getChange(), ps);
+      plcUtil.insertComments(
+          ctx.getDb(), ctx.getChangeUpdate(), Collections.singleton(comment));
+    }
   }
 }

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

@@ -16,37 +16,44 @@ package com.google.gerrit.server.change;
 
 import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
 
+import com.google.common.base.Optional;
+import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 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.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.PatchLineCommentsUtil;
 import com.google.gerrit.server.change.DeleteDraftComment.Input;
-import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.git.BatchUpdate;
+import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
+import com.google.gerrit.server.git.UpdateException;
 import com.google.gerrit.server.patch.PatchListCache;
 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.util.Collections;
 
 @Singleton
-public class DeleteDraftComment implements RestModifyView<DraftCommentResource, Input> {
+public class DeleteDraftComment
+    implements RestModifyView<DraftCommentResource, Input> {
   static class Input {
   }
 
   private final Provider<ReviewDb> db;
   private final PatchLineCommentsUtil plcUtil;
-  private final ChangeUpdate.Factory updateFactory;
+  private final BatchUpdate.Factory updateFactory;
   private final PatchListCache patchListCache;
 
   @Inject
   DeleteDraftComment(Provider<ReviewDb> db,
       PatchLineCommentsUtil plcUtil,
-      ChangeUpdate.Factory updateFactory,
+      BatchUpdate.Factory updateFactory,
       PatchListCache patchListCache) {
     this.db = db;
     this.plcUtil = plcUtil;
@@ -56,13 +63,41 @@ public class DeleteDraftComment implements RestModifyView<DraftCommentResource,
 
   @Override
   public Response<CommentInfo> apply(DraftCommentResource rsrc, Input input)
-      throws OrmException, IOException {
+      throws RestApiException, UpdateException {
-    ChangeUpdate update = updateFactory.create(rsrc.getControl());
+    try (BatchUpdate bu = updateFactory.create(
-
+        db.get(), rsrc.getChange().getProject(), rsrc.getControl().getUser(),
-    PatchLineComment c = rsrc.getComment();
+        TimeUtil.nowTs())) {
-    setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
+      Op op = new Op(rsrc.getComment().getKey());
-    plcUtil.deleteComments(db.get(), update, Collections.singleton(c));
+      bu.addOp(rsrc.getChange().getId(), op);
-    update.commit();
+      bu.execute();
+    }
     return Response.none();
   }
+
+  private class Op extends BatchUpdate.Op {
+    private final PatchLineComment.Key key;
+
+    private Op(PatchLineComment.Key key) {
+      this.key = key;
+    }
+
+    @Override
+    public void updateChange(ChangeContext ctx)
+        throws ResourceNotFoundException, OrmException {
+      Optional<PatchLineComment> maybeComment =
+          plcUtil.get(ctx.getDb(), ctx.getChangeNotes(), key);
+      if (!maybeComment.isPresent()) {
+        return; // Nothing to do.
+      }
+      PatchSet.Id psId = key.getParentKey().getParentKey();
+      PatchSet ps = ctx.getDb().patchSets().get(psId);
+      if (ps == null) {
+        throw new ResourceNotFoundException("patch set not found: " + psId);
+      }
+      PatchLineComment c = maybeComment.get();
+      setCommentRevId(c, patchListCache, ctx.getChange(), ps);
+      plcUtil.deleteComments(
+          ctx.getDb(), ctx.getChangeUpdate(), Collections.singleton(c));
+    }
+  }
 }

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

@@ -16,26 +16,31 @@ package com.google.gerrit.server.change;
 
 import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
 
+import com.google.common.base.Optional;
 import com.google.gerrit.common.TimeUtil;
 import com.google.gerrit.extensions.api.changes.DraftInput;
 import com.google.gerrit.extensions.client.Side;
 import com.google.gerrit.extensions.common.CommentInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
+import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 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.restapi.Url;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.PatchLineCommentsUtil;
-import com.google.gerrit.server.notedb.ChangeUpdate;
+import com.google.gerrit.server.git.BatchUpdate;
+import com.google.gerrit.server.git.BatchUpdate.ChangeContext;
+import com.google.gerrit.server.git.UpdateException;
 import com.google.gerrit.server.patch.PatchListCache;
 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.util.Collections;
 
 @Singleton
@@ -44,7 +49,7 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
   private final Provider<ReviewDb> db;
   private final DeleteDraftComment delete;
   private final PatchLineCommentsUtil plcUtil;
-  private final ChangeUpdate.Factory updateFactory;
+  private final BatchUpdate.Factory updateFactory;
   private final Provider<CommentJson> commentJson;
   private final PatchListCache patchListCache;
 
@@ -52,7 +57,7 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
   PutDraftComment(Provider<ReviewDb> db,
       DeleteDraftComment delete,
       PatchLineCommentsUtil plcUtil,
-      ChangeUpdate.Factory updateFactory,
+      BatchUpdate.Factory updateFactory,
       Provider<CommentJson> commentJson,
       PatchListCache patchListCache) {
     this.db = db;
@@ -65,9 +70,7 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
 
   @Override
   public Response<CommentInfo> apply(DraftCommentResource rsrc, DraftInput in) throws
-      BadRequestException, OrmException, IOException {
+      RestApiException, UpdateException, OrmException {
-    PatchLineComment c = rsrc.getComment();
-    ChangeUpdate update = updateFactory.create(rsrc.getControl());
     if (in == null || in.message == null || in.message.trim().isEmpty()) {
       return delete.apply(rsrc, null);
     } else if (in.id != null && !rsrc.getId().equals(in.id)) {
@@ -78,34 +81,74 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
       throw new BadRequestException("range endLine must be on the same line as the comment");
     }
 
-    if (in.path != null
+    try (BatchUpdate bu = updateFactory.create(
-        && !in.path.equals(c.getKey().getParentKey().getFileName())) {
+        db.get(), rsrc.getChange().getProject(), rsrc.getControl().getUser(),
-      // Updating the path alters the primary key, which isn't possible.
+        TimeUtil.nowTs())) {
-      // Delete then recreate the comment instead of an update.
+      Op op = new Op(rsrc.getComment().getKey(), in);
-
+      bu.addOp(rsrc.getChange().getId(), op);
-      plcUtil.deleteComments(db.get(), update, Collections.singleton(c));
+      bu.execute();
-      c = new PatchLineComment(
+      return Response.ok(
-          new PatchLineComment.Key(
+          commentJson.get().setFillAccounts(false).format(op.comment));
-              new Patch.Key(rsrc.getPatchSet().getId(), in.path),
-              c.getKey().get()),
-          c.getLine(),
-          rsrc.getAuthorId(),
-          c.getParentUuid(), TimeUtil.nowTs());
-      setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
-      plcUtil.insertComments(db.get(), update,
-          Collections.singleton(update(c, in)));
-    } else {
-      if (c.getRevId() == null) {
-        setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
-      }
-      plcUtil.updateComments(db.get(), update,
-          Collections.singleton(update(c, in)));
     }
-    update.commit();
-    return Response.ok(commentJson.get().setFillAccounts(false).format(c));
   }
 
-  private PatchLineComment update(PatchLineComment e, DraftInput in) {
+  private class Op extends BatchUpdate.Op {
+    private final PatchLineComment.Key key;
+    private final DraftInput in;
+
+    private PatchLineComment comment;
+
+    private Op(PatchLineComment.Key key, DraftInput in) {
+      this.key = key;
+      this.in = in;
+    }
+
+    @Override
+    public void updateChange(ChangeContext ctx)
+        throws ResourceNotFoundException, OrmException {
+      Optional<PatchLineComment> maybeComment =
+          plcUtil.get(ctx.getDb(), ctx.getChangeNotes(), key);
+      if (!maybeComment.isPresent()) {
+        // Disappeared out from under us. Can't easily fall back to insert,
+        // because the input might be missing required fields. Just give up.
+        throw new ResourceNotFoundException("comment not found: " + key);
+      }
+      comment = maybeComment.get();
+
+      PatchSet.Id psId = comment.getKey().getParentKey().getParentKey();
+      PatchSet ps = ctx.getDb().patchSets().get(psId);
+      if (ps == null) {
+        throw new ResourceNotFoundException("patch set not found: " + psId);
+      }
+      if (in.path != null
+          && !in.path.equals(comment.getKey().getParentKey().getFileName())) {
+        // Updating the path alters the primary key, which isn't possible.
+        // Delete then recreate the comment instead of an update.
+
+        plcUtil.deleteComments(
+            ctx.getDb(), ctx.getChangeUpdate(), Collections.singleton(comment));
+        comment = new PatchLineComment(
+            new PatchLineComment.Key(
+                new Patch.Key(psId, in.path),
+                comment.getKey().get()),
+            comment.getLine(),
+            ctx.getUser().getAccountId(),
+            comment.getParentUuid(), ctx.getWhen());
+        setCommentRevId(comment, patchListCache, ctx.getChange(), ps);
+        plcUtil.insertComments(ctx.getDb(), ctx.getChangeUpdate(),
+            Collections.singleton(update(comment, in)));
+      } else {
+        if (comment.getRevId() == null) {
+          setCommentRevId(
+              comment, patchListCache, ctx.getChange(), ps);
+        }
+        plcUtil.updateComments(ctx.getDb(), ctx.getChangeUpdate(),
+            Collections.singleton(update(comment, in)));
+      }
+    }
+  }
+
+  private static PatchLineComment update(PatchLineComment e, DraftInput in) {
     if (in.side != null) {
       e.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
     }

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

@@ -21,6 +21,7 @@ import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.edit.ChangeEdit;
 import com.google.gerrit.server.notedb.ChangeNotes;
@@ -63,6 +64,10 @@ public class RevisionResource implements RestResource, HasETag {
     return getControl().getChange();
   }
 
+  public Project.NameKey getProject() {
+    return getChange().getProject();
+  }
+
   public ChangeNotes getNotes() {
     return getChangeResource().getNotes();
   }