Revision API: Prepare to expose comments and drafts

This is a refactoring change to expose comments and drafts to plugin API.

- remove own DraftInput class from server package and reuse existing one
from the extensions package
- remove Side enum and reuse the existing enum from extensions package
- replace CommentRange with Range in CommentInfo class

This is needed to simplify mapping from server side CommentInfo class
to one from extensions package.  It cannot be used directly because
of AccountInfo class and needs to be mapped in plugin API layer.

Change-Id: If6c4cbce715f89f84459a6cd97073d1de31b0312

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java

@@ -22,8 +22,8 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.acceptance.RestResponse;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
-import com.google.gerrit.extensions.common.Comment;
 import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.server.notedb.NotesMigration;
 import com.google.gerrit.testutil.ConfigSuite;
 import com.google.gson.reflect.TypeToken;
@@ -50,7 +50,7 @@ public class CommentsIT extends AbstractDaemonTest {
     String changeId = r.getChangeId();
     String revId = r.getCommit().getName();
     ReviewInput.CommentInput comment = newCommentInfo(
-        "file1", Comment.Side.REVISION, 1, "comment 1");
+        "file1", Side.REVISION, 1, "comment 1");
     addDraft(changeId, revId, comment);
     Map<String, List<CommentInfo>> result = getDraftComments(changeId, revId);
     assertThat(result).hasSize(1);
@@ -69,7 +69,7 @@ public class CommentsIT extends AbstractDaemonTest {
     String revId = r.getCommit().getName();
     ReviewInput input = new ReviewInput();
     ReviewInput.CommentInput comment = newCommentInfo(
-        file, Comment.Side.REVISION, 1, "comment 1");
+        file, Side.REVISION, 1, "comment 1");
     input.comments = new HashMap<>();
     input.comments.put(comment.path, Lists.newArrayList(comment));
     revision(r).review(input);
@@ -85,7 +85,7 @@ public class CommentsIT extends AbstractDaemonTest {
     String changeId = r.getChangeId();
     String revId = r.getCommit().getName();
     ReviewInput.CommentInput comment = newCommentInfo(
-        "file1", Comment.Side.REVISION, 1, "comment 1");
+        "file1", Side.REVISION, 1, "comment 1");
     addDraft(changeId, revId, comment);
     Map<String, List<CommentInfo>> result = getDraftComments(changeId, revId);
     CommentInfo actual = Iterables.getOnlyElement(result.get(comment.path));
@@ -104,7 +104,7 @@ public class CommentsIT extends AbstractDaemonTest {
     String changeId = r.getChangeId();
     String revId = r.getCommit().getName();
     ReviewInput.CommentInput comment = newCommentInfo(
-        "file1", Comment.Side.REVISION, 1, "comment 1");
+        "file1", Side.REVISION, 1, "comment 1");
     CommentInfo returned = addDraft(changeId, revId, comment);
     CommentInfo actual = getDraftComment(changeId, revId, returned.id);
     assertCommentInfo(comment, actual);
@@ -116,7 +116,7 @@ public class CommentsIT extends AbstractDaemonTest {
     String changeId = r.getChangeId();
     String revId = r.getCommit().getName();
     ReviewInput.CommentInput comment = newCommentInfo(
-        "file1", Comment.Side.REVISION, 1, "comment 1");
+        "file1", Side.REVISION, 1, "comment 1");
     CommentInfo returned = addDraft(changeId, revId, comment);
     deleteDraft(changeId, revId, returned.id);
     Map<String, List<CommentInfo>> drafts = getDraftComments(changeId, revId);
@@ -177,12 +177,12 @@ public class CommentsIT extends AbstractDaemonTest {
     assertThat(actual.message).isEqualTo(expected.message);
     assertThat(actual.inReplyTo).isEqualTo(expected.inReplyTo);
     if (actual.side == null) {
-      assertThat(Comment.Side.REVISION).isEqualTo(expected.side);
+      assertThat(Side.REVISION).isEqualTo(expected.side);
     }
   }
 
   private ReviewInput.CommentInput newCommentInfo(String path,
-      Comment.Side side, int line, String message) {
+      Side side, int line, String message) {
     ReviewInput.CommentInput input = new ReviewInput.CommentInput();
     input.path = path;
     input.side = side;

gerrit-extension-api/BUCK

@@ -6,8 +6,10 @@ gwt_module(
   srcs = glob([
     SRC + 'api/projects/ProjectState.java',
     SRC + 'common/ChangeStatus.java',
+    SRC + 'common/Comment.java',
     SRC + 'common/InheritableBoolean.java',
     SRC + 'common/ListChangesOption.java',
+    SRC + 'common/Side.java',
     SRC + 'common/SubmitType.java',
     SRC + 'common/Theme.java',
     SRC + 'webui/GerritTopMenu.java',

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DraftInput.java

@@ -0,0 +1,20 @@
+// Copyright (C) 2014 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.extensions.api.changes;
+
+import com.google.gerrit.extensions.common.Comment;
+
+public class DraftInput extends Comment {
+}

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/Comment.java

@@ -20,16 +20,12 @@ public abstract class Comment {
   public String id;
   public String path;
   public Side side;
-  public int line;
+  public Integer line;
   public Range range;
   public String inReplyTo;
   public Timestamp updated;
   public String message;
 
-  public static enum Side {
-    PARENT, REVISION
-  }
-
   public static class Range {
     public int startLine;
     public int startCharacter;

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/Side.java

@@ -1,4 +1,4 @@
-// Copyright (C) 2012 The Android Open Source Project
+// Copyright (C) 2014 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.
@@ -12,9 +12,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.gerrit.common.changes;
+package com.google.gerrit.extensions.common;
 
-/** The side on which a comment was added. */
 public enum Side {
   PARENT, REVISION
 }

gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LineComment.java

@@ -19,7 +19,7 @@ import com.google.gerrit.client.changes.CommentInfo;
 import com.google.gerrit.client.diff.DisplaySide;
 import com.google.gerrit.client.ui.CommentLinkProcessor;
 import com.google.gerrit.client.ui.InlineHyperlink;
-import com.google.gerrit.common.changes.Side;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gwt.core.client.GWT;
 import com.google.gwt.dom.client.Element;

gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java

@@ -16,7 +16,7 @@ package com.google.gerrit.client.changes;
 
 import com.google.gerrit.client.account.AccountInfo;
 import com.google.gerrit.client.diff.CommentRange;
-import com.google.gerrit.common.changes.Side;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwtjsonrpc.client.impl.ser.JavaSqlTimestamp_JsonSerializer;
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java

@@ -20,7 +20,7 @@ import com.google.gerrit.client.patches.SkippedLine;
 import com.google.gerrit.client.rpc.CallbackGroup;
 import com.google.gerrit.client.rpc.Natives;
 import com.google.gerrit.client.ui.CommentLinkProcessor;
-import com.google.gerrit.common.changes.Side;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gwt.core.client.JsArray;
 

gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java

@@ -20,7 +20,7 @@ import com.google.gerrit.client.changes.CommentInfo;
 import com.google.gerrit.client.rpc.GerritCallback;
 import com.google.gerrit.client.ui.CommentLinkProcessor;
 import com.google.gerrit.client.ui.CommentPanel;
-import com.google.gerrit.common.changes.Side;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;

gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchLineComment.java

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.reviewdb.client;
 
+import com.google.gerrit.extensions.common.Comment.Range;
 import com.google.gwtorm.client.Column;
 import com.google.gwtorm.client.StringKey;
 
@@ -273,4 +274,11 @@ public final class PatchLineComment {
     builder.append('}');
     return builder.toString();
   }
+
+  public void fromRange(Range r) {
+    range = r == null
+        ? null
+        : new CommentRange(range.startLine, range.startCharacter,
+            range.endLine, range.endCharacter);
+  }
 }

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

@@ -15,8 +15,9 @@
 package com.google.gerrit.server.change;
 
 import com.google.common.base.Strings;
-import com.google.gerrit.common.changes.Side;
 import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.common.Comment.Range;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.reviewdb.client.CommentRange;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
@@ -33,7 +34,7 @@ public class CommentInfo {
   String message;
   Timestamp updated;
   AccountInfo author;
-  CommentRange range;
+  Range range;
 
   CommentInfo(PatchLineComment c, AccountLoader accountLoader) {
     id = Url.encode(c.getKey().get());
@@ -47,9 +48,21 @@ public class CommentInfo {
     inReplyTo = Url.encode(c.getParentUuid());
     message = Strings.emptyToNull(c.getMessage());
     updated = c.getWrittenOn();
-    range = c.getRange();
+    range = toRange(c.getRange());
     if (accountLoader != null) {
       author = accountLoader.get(c.getAuthor());
     }
   }
+
+  public static Range toRange(CommentRange commentRange) {
+    Range range = null;
+    if (commentRange != null) {
+      range = new Range();
+      range.startLine = commentRange.getStartLine();
+      range.startCharacter = commentRange.getStartCharacter();
+      range.endLine = commentRange.getEndLine();
+      range.endCharacter = commentRange.getEndCharacter();
+    }
+    return range;
+  }
 }

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

@@ -18,7 +18,8 @@ import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
 
 import com.google.common.base.Strings;
 import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.common.changes.Side;
+import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
@@ -28,7 +29,6 @@ import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.PatchLineCommentsUtil;
-import com.google.gerrit.server.change.PutDraft.Input;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.patch.PatchListCache;
 import com.google.gwtorm.server.OrmException;
@@ -41,7 +41,7 @@ import java.sql.Timestamp;
 import java.util.Collections;
 
 @Singleton
-class CreateDraft implements RestModifyView<RevisionResource, Input> {
+class CreateDraft implements RestModifyView<RevisionResource, DraftInput> {
   private final Provider<ReviewDb> db;
   private final ChangeUpdate.Factory updateFactory;
   private final PatchLineCommentsUtil plcUtil;
@@ -59,7 +59,7 @@ class CreateDraft implements RestModifyView<RevisionResource, Input> {
   }
 
   @Override
-  public Response<CommentInfo> apply(RevisionResource rsrc, Input in)
+  public Response<CommentInfo> apply(RevisionResource rsrc, DraftInput in)
       throws BadRequestException, OrmException, IOException {
     if (Strings.isNullOrEmpty(in.path)) {
       throw new BadRequestException("path must be non-empty");
@@ -67,13 +67,13 @@ class CreateDraft implements RestModifyView<RevisionResource, Input> {
       throw new BadRequestException("message must be non-empty");
     } else if (in.line != null && in.line <= 0) {
       throw new BadRequestException("line must be > 0");
-    } else if (in.line != null && in.range != null && in.line != in.range.getEndLine()) {
+    } else if (in.line != null && in.range != null && in.line != in.range.endLine) {
       throw new BadRequestException("range endLine must be on the same line as the comment");
     }
 
     int line = in.line != null
         ? in.line
-        : in.range != null ? in.range.getEndLine() : 0;
+        : in.range != null ? in.range.endLine : 0;
 
     Timestamp now = TimeUtil.nowTs();
     ChangeUpdate update = updateFactory.create(rsrc.getControl(), now);
@@ -85,7 +85,7 @@ class CreateDraft implements RestModifyView<RevisionResource, Input> {
         line, rsrc.getAccountId(), Url.decode(in.inReplyTo), now);
     c.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
     c.setMessage(in.message.trim());
-    c.setRange(in.range);
+    c.fromRange(in.range);
     setCommentRevId(c, patchListCache, rsrc.getChange(), rsrc.getPatchSet());
     plcUtil.insertComments(db.get(), update, Collections.singleton(c));
     update.commit();

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

@@ -18,7 +18,7 @@ import static com.google.common.base.MoreObjects.firstNonNull;
 
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
-import com.google.gerrit.common.changes.Side;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.extensions.restapi.RestReadView;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.server.ReviewDb;

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

@@ -34,7 +34,7 @@ import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.CommentInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.DraftHandling;
 import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
-import com.google.gerrit.extensions.common.Comment.Side;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;

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

@@ -17,18 +17,16 @@ package com.google.gerrit.server.change;
 import static com.google.gerrit.server.PatchLineCommentsUtil.setCommentRevId;
 
 import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.common.changes.Side;
+import com.google.gerrit.extensions.api.changes.DraftInput;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.DefaultInput;
 import com.google.gerrit.extensions.restapi.Response;
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.Url;
-import com.google.gerrit.reviewdb.client.CommentRange;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.PatchLineCommentsUtil;
-import com.google.gerrit.server.change.PutDraft.Input;
 import com.google.gerrit.server.notedb.ChangeUpdate;
 import com.google.gerrit.server.patch.PatchListCache;
 import com.google.gwtorm.server.OrmException;
@@ -37,23 +35,10 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 
 import java.io.IOException;
-import java.sql.Timestamp;
 import java.util.Collections;
 
 @Singleton
-class PutDraft implements RestModifyView<DraftResource, Input> {
-  static class Input {
-    String id;
-    String path;
-    Side side;
-    Integer line;
-    String inReplyTo;
-    Timestamp updated; // Accepted but ignored.
-    CommentRange range;
-
-    @DefaultInput
-    String message;
-  }
+class PutDraft implements RestModifyView<DraftResource, DraftInput> {
 
   private final Provider<ReviewDb> db;
   private final DeleteDraft delete;
@@ -75,7 +60,7 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
   }
 
   @Override
-  public Response<CommentInfo> apply(DraftResource rsrc, Input in) throws
+  public Response<CommentInfo> apply(DraftResource rsrc, DraftInput in) throws
       BadRequestException, OrmException, IOException {
     PatchLineComment c = rsrc.getComment();
     ChangeUpdate update = updateFactory.create(rsrc.getControl());
@@ -85,7 +70,7 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
       throw new BadRequestException("id must match URL");
     } else if (in.line != null && in.line < 0) {
       throw new BadRequestException("line must be >= 0");
-    } else if (in.line != null && in.range != null && in.line != in.range.getEndLine()) {
+    } else if (in.line != null && in.range != null && in.line != in.range.endLine) {
       throw new BadRequestException("range endLine must be on the same line as the comment");
     }
 
@@ -117,7 +102,7 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
     return Response.ok(new CommentInfo(c, null));
   }
 
-  private PatchLineComment update(PatchLineComment e, Input in) {
+  private PatchLineComment update(PatchLineComment e, DraftInput in) {
     if (in.side != null) {
       e.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
     }
@@ -126,8 +111,8 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
     }
     e.setMessage(in.message.trim());
     if (in.range != null || in.line != null) {
-      e.setRange(in.range);
-      e.setLine(in.range != null ? in.range.getEndLine() : in.line);
+      e.fromRange(in.range);
+      e.setLine(in.range != null ? in.range.endLine : in.line);
     }
     e.setWrittenOn(TimeUtil.nowTs());
     return e;

gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java

@@ -27,8 +27,8 @@ import com.google.common.base.MoreObjects;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Lists;
 import com.google.gerrit.common.TimeUtil;
-import com.google.gerrit.common.changes.Side;
 import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.common.Side;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
@@ -443,7 +443,11 @@ public class CommentsTest  {
         MoreObjects.firstNonNull(ci.side, Side.REVISION));
     assertEquals(TimeUtil.roundToSecond(plc.getWrittenOn()),
         TimeUtil.roundToSecond(ci.updated));
-    assertEquals(plc.getRange(), ci.range);
+    assertEquals(plc.getWrittenOn(), ci.updated);
+    assertEquals(plc.getRange().getStartLine(), ci.range.startLine);
+    assertEquals(plc.getRange().getStartCharacter(), ci.range.startCharacter);
+    assertEquals(plc.getRange().getEndLine(), ci.range.endLine);
+    assertEquals(plc.getRange().getEndCharacter(), ci.range.endCharacter);
   }
 
   private static PatchLineComment newPatchLineComment(PatchSet.Id psId,