SideBySide2: Add range to PatchLineComment

Added a column named "range", a quadruple
(start_line, start_character, end_line, end_character) that
represents a comment range.

Modified RPCs to include the range info.

Updated schema to version 80.

Change-Id: Idec295e15e3ab019ea04e4c2f3967eeecc0be55c

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

@@ -15,6 +15,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.gwt.core.client.JavaScriptObject;
 import com.google.gwtjsonrpc.client.impl.ser.JavaSqlTimestamp_JsonSerializer;
@@ -22,10 +23,11 @@ import com.google.gwtjsonrpc.client.impl.ser.JavaSqlTimestamp_JsonSerializer;
 import java.sql.Timestamp;
 
 public class CommentInfo extends JavaScriptObject {
-  public static CommentInfo createLine(String path, Side side, int line,
+  public static CommentInfo createRange(String path, Side side, int line,
-      String in_reply_to, String message) {
+      String in_reply_to, String message, CommentRange range) {
     CommentInfo info = createFile(path, side, in_reply_to, message);
-    info.setLine(line);
+    info.setRange(range);
+    info.setLine(range == null ? line : range.end_line());
     return info;
   }
 
@@ -82,6 +84,10 @@ public class CommentInfo extends JavaScriptObject {
 
   public final native boolean has_line() /*-{ return this.hasOwnProperty('line'); }-*/;
 
+  public final native CommentRange range() /*-{ return this.range; }-*/;
+
+  public final native void setRange(CommentRange range) /*-{ this.range = range; }-*/;
+
   protected CommentInfo() {
   }
 }

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

@@ -14,6 +14,7 @@
 
 package com.google.gerrit.client.changes;
 
+import com.google.gerrit.client.diff.CommentRange;
 import com.google.gerrit.common.changes.Side;
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwtjsonrpc.client.impl.ser.JavaSqlTimestamp_JsonSerializer;
@@ -29,6 +30,7 @@ public class CommentInput extends JavaScriptObject {
     if (original.has_line()) {
       input.setLine(original.line());
     }
+    input.setRange(original.range());
     input.setInReplyTo(original.in_reply_to());
     input.setMessage(original.message());
     return input;
@@ -71,6 +73,10 @@ public class CommentInput extends JavaScriptObject {
 
   public final native boolean has_line() /*-{ return this.hasOwnProperty('line'); }-*/;
 
+  public final native CommentRange range() /*-{ return this.range; }-*/;
+
+  public final native void setRange(CommentRange range) /*-{ this.range = range; }-*/;
+
   protected CommentInput() {
   }
 }

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

@@ -0,0 +1,40 @@
+// Copyright (C) 2013 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.client.diff;
+
+import com.google.gwt.core.client.JavaScriptObject;
+
+public class CommentRange extends JavaScriptObject {
+  public static CommentRange create(int sl, int sc, int el, int ec) {
+    CommentRange r = createObject().cast();
+    r.set(sl, sc, el, ec);
+    return r;
+  }
+
+  public final native int start_line() /*-{ return this.start_line; }-*/;
+  public final native int start_character() /*-{ return this.start_character; }-*/;
+  public final native int end_line() /*-{ return this.end_line; }-*/;
+  public final native int end_character() /*-{ return this.end_character; }-*/;
+
+  private final native void set(int sl, int sc, int el, int ec) /*-{
+    this.start_line = sl;
+    this.start_character = sc;
+    this.end_line = el;
+    this.end_character = ec;
+  }-*/;
+
+  protected CommentRange() {
+  }
+}

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

@@ -466,11 +466,12 @@ public class SideBySide2 extends Screen {
   }
 
   private DraftBox addNewDraft(CodeMirror cm, int line) {
-    return addDraftBox(CommentInfo.createLine(
+    return addDraftBox(CommentInfo.createRange(
         path,
         getSideFromCm(cm),
         line + 1,
         null,
+        null,
         null));
   }
 
@@ -478,8 +479,8 @@ public class SideBySide2 extends Screen {
     if (!replyTo.has_line()) {
       return CommentInfo.createFile(path, replyTo.side(), replyTo.id(), null);
     } else {
-      return CommentInfo.createLine(path, replyTo.side(), replyTo.line(),
+      return CommentInfo.createRange(path, replyTo.side(), replyTo.line(),
-          replyTo.id(), null);
+          replyTo.id(), null, null);
     }
   }
 

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/SaveDraft.java

@@ -84,12 +84,18 @@ class SaveDraft extends Handler<PatchLineComment> {
             throw new IllegalStateException("Parent comment must be on same side");
           }
         }
+        if (comment.getRange() != null
+            && comment.getLine() != comment.getRange().getEndLine()) {
+            throw new IllegalStateException(
+              "Range endLine must be on the same line as the comment");
+        }
 
         final PatchLineComment nc =
             new PatchLineComment(new PatchLineComment.Key(patchKey, ChangeUtil
                 .messageUUID(db)), comment.getLine(), me, comment.getParentUuid());
         nc.setSide(comment.getSide());
         nc.setMessage(comment.getMessage());
+        nc.setRange(comment.getRange());
         db.patchComments().insert(Collections.singleton(nc));
         db.commit();
         return nc;

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

@@ -0,0 +1,90 @@
+// Copyright (C) 2013 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.reviewdb.client;
+
+import com.google.gwtorm.client.Column;
+
+public class CommentRange {
+
+  @Column(id = 1)
+  protected int startLine;
+
+  @Column(id = 2)
+  protected int startCharacter;
+
+  @Column(id = 3)
+  protected int endLine;
+
+  @Column(id = 4)
+  protected int endCharacter;
+
+  protected CommentRange() {
+  }
+
+  public CommentRange(int sl, int sc, int el, int ec) {
+    startLine = sl;
+    startCharacter = sc;
+    endLine = el;
+    endCharacter = ec;
+  }
+
+  public int getStartLine() {
+    return startLine;
+  }
+
+  public int getStartCharacter() {
+    return startCharacter;
+  }
+
+  public int getEndLine() {
+    return endLine;
+  }
+
+  public int getEndCharacter() {
+    return endCharacter;
+  }
+
+  public void setStartLine(int sl) {
+    startLine = sl;
+  }
+
+  public void setStartCharacter(int sc) {
+    startCharacter = sc;
+  }
+
+  public void setEndLine(int el) {
+    endLine = el;
+  }
+
+  public void setEndCharacter(int ec) {
+    endCharacter = ec;
+  }
+
+  @Override
+  public boolean equals(Object obj) {
+    if (obj instanceof CommentRange) {
+      CommentRange other = (CommentRange) obj;
+      return startLine == other.startLine && startCharacter == other.startCharacter &&
+          endLine == other.endLine && endCharacter == other.endCharacter;
+    }
+    return false;
+  }
+
+  @Override
+  public String toString() {
+    return "Range[startLine=" + startLine + ", startCharacter=" + startCharacter
+        + ", endLine=" + endLine + ", endCharacter=" + endCharacter + "]";
+  }
+}

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

@@ -117,6 +117,9 @@ public final class PatchLineComment {
   @Column(id = 8, length = 40, notNull = false)
   protected String parentUuid;
 
+  @Column(id = 9, notNull = false)
+  protected CommentRange range;
+
   protected PatchLineComment() {
   }
 
@@ -189,4 +192,12 @@ public final class PatchLineComment {
   public void setParentUuid(String inReplyTo) {
     parentUuid = inReplyTo;
   }
+
+  public void setRange(CommentRange r) {
+    range = r;
+  }
+
+  public CommentRange getRange() {
+    return range;
+  }
 }

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

@@ -18,6 +18,7 @@ import com.google.common.base.Strings;
 import com.google.gerrit.common.changes.Side;
 import com.google.gerrit.extensions.restapi.Url;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
+import com.google.gerrit.reviewdb.client.CommentRange;
 import com.google.gerrit.server.account.AccountInfo;
 
 import java.sql.Timestamp;
@@ -33,6 +34,7 @@ public class CommentInfo {
   String message;
   Timestamp updated;
   AccountInfo author;
+  CommentRange range;
 
   CommentInfo(PatchLineComment c, AccountInfo.Loader accountLoader) {
     id = Url.encode(c.getKey().get());
@@ -46,6 +48,7 @@ public class CommentInfo {
     inReplyTo = Url.encode(c.getParentUuid());
     message = Strings.emptyToNull(c.getMessage());
     updated = c.getWrittenOn();
+    range = c.getRange();
     if (accountLoader != null) {
       author = accountLoader.get(c.getAuthor());
     }

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

@@ -24,7 +24,6 @@ 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.PatchLineComment.Status;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.change.PutDraft.Input;
@@ -51,18 +50,22 @@ 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()) {
+      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;
+
     PatchLineComment c = new PatchLineComment(
         new PatchLineComment.Key(
             new Patch.Key(rsrc.getPatchSet().getId(), in.path),
             ChangeUtil.messageUUID(db.get())),
-        in.line != null ? in.line : 0,
+        line, rsrc.getAccountId(), Url.decode(in.inReplyTo));
-        rsrc.getAccountId(),
-        Url.decode(in.inReplyTo));
-    c.setStatus(Status.DRAFT);
     c.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
     c.setMessage(in.message.trim());
+    c.setRange(in.range);
     db.get().patchComments().insert(Collections.singleton(c));
     return Response.created(new CommentInfo(c, null));
   }

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

@@ -38,6 +38,7 @@ import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
+import com.google.gerrit.reviewdb.client.CommentRange;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ChangeUtil;
 import com.google.gerrit.server.IdentifiedUser;
@@ -116,6 +117,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
     int line;
     String inReplyTo;
     String message;
+    CommentRange range;
   }
 
   static class Output {
@@ -365,6 +367,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> {
         e.setWrittenOn(timestamp);
         e.setSide(c.side == Side.PARENT ? (short) 0 : (short) 1);
         e.setMessage(c.message);
+        e.setRange(c.range);
         (create ? ins : upd).add(e);
       }
     }

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

@@ -23,6 +23,7 @@ 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.CommentRange;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.change.PutDraft.Input;
 import com.google.gwtorm.server.OrmException;
@@ -41,6 +42,7 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
     Integer line;
     String inReplyTo;
     Timestamp updated; // Accepted but ignored.
+    CommentRange range;
 
     @DefaultInput
     String message;
@@ -67,6 +69,8 @@ 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()) {
+      throw new BadRequestException("range endLine must be on the same line as the comment");
     }
 
     if (in.path != null
@@ -92,13 +96,14 @@ class PutDraft implements RestModifyView<DraftResource, Input> {
     if (in.side != null) {
       e.setSide(in.side == Side.PARENT ? (short) 0 : (short) 1);
     }
-    if (in.line != null) {
-      e.setLine(in.line);
-    }
     if (in.inReplyTo != null) {
       e.setParentUuid(Url.decode(in.inReplyTo));
     }
     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.updated();
     return e;
   }

gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java

@@ -32,7 +32,7 @@ import java.util.List;
 /** A version of the database schema. */
 public abstract class SchemaVersion {
   /** The current schema version. */
-  public static final Class<Schema_79> C = Schema_79.class;
+  public static final Class<Schema_80> C = Schema_80.class;
 
   public static class Module extends AbstractModule {
     @Override

gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_80.java

@@ -0,0 +1,26 @@
+// Copyright (C) 2013 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.schema;
+
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+
+public class Schema_80 extends SchemaVersion {
+
+  @Inject
+  Schema_80(Provider<Schema_79> prior) {
+    super(prior);
+  }
+}

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

@@ -34,6 +34,7 @@ import com.google.gerrit.reviewdb.client.Patch;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchLineComment.Status;
 import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.CommentRange;
 import com.google.gerrit.reviewdb.server.PatchLineCommentAccess;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.account.AccountInfo;
@@ -111,13 +112,13 @@ public class CommentsTest extends TestCase {
     long timeBase = System.currentTimeMillis();
     plc1 = newPatchLineComment(psId1, "Comment1", null,
         "FileOne.txt", Side.REVISION, 1, account1, timeBase,
-        "First Comment");
+        "First Comment", new CommentRange(1, 2, 3, 4));
     plc2 = newPatchLineComment(psId1, "Comment2", "Comment1",
         "FileOne.txt", Side.REVISION, 1, account2, timeBase + 1000,
-        "Reply to First Comment");
+        "Reply to First Comment",  new CommentRange(1, 2, 3, 4));
     plc3 = newPatchLineComment(psId1, "Comment3", "Comment1",
         "FileOne.txt", Side.PARENT, 1, account1, timeBase + 2000,
-        "First Parent Comment");
+        "First Parent Comment",  new CommentRange(1, 2, 3, 4));
 
     expect(plca.publishedByPatchSet(psId1))
         .andAnswer(results(plc1, plc2, plc3)).anyTimes();
@@ -206,16 +207,18 @@ public class CommentsTest extends TestCase {
     assertEquals(plc.getSide() == 0 ? Side.PARENT : Side.REVISION,
         Objects.firstNonNull(ci.side, Side.REVISION));
     assertEquals(plc.getWrittenOn(), ci.updated);
+    assertEquals(plc.getRange(), ci.range);
   }
 
   private static PatchLineComment newPatchLineComment(PatchSet.Id psId,
       String uuid, String inReplyToUuid, String filename, Side side, int line,
-      Account.Id authorId, long millis, String message) {
+      Account.Id authorId, long millis, String message, CommentRange range) {
     Patch.Key p = new Patch.Key(psId, filename);
     PatchLineComment.Key id = new PatchLineComment.Key(p, uuid);
     PatchLineComment plc =
         new PatchLineComment(id, line, authorId, inReplyToUuid);
     plc.setMessage(message);
+    plc.setRange(range);
     plc.setSide(side == Side.PARENT ? (short) 0 : (short) 1);
     plc.setStatus(Status.PUBLISHED);
     plc.setWrittenOn(new Timestamp(millis));