diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java index 5f6a9c57c2..34c0638c93 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInfo.java +++ b/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, - String in_reply_to, String message) { + public static CommentInfo createRange(String path, Side side, int line, + 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() { } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInput.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInput.java index 592e087cc2..c96a67f15a 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/CommentInput.java +++ b/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() { } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentRange.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentRange.java new file mode 100644 index 0000000000..f6fb1ba1a5 --- /dev/null +++ b/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() { + } +} \ No newline at end of file diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java index c953df8453..85b917ce68 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/SideBySide2.java +++ b/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(), - replyTo.id(), null); + return CommentInfo.createRange(path, replyTo.side(), replyTo.line(), + replyTo.id(), null, null); } } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/SaveDraft.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/SaveDraft.java index 18ab5ff5fc..66b5ec1787 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/SaveDraft.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/SaveDraft.java @@ -84,12 +84,18 @@ class SaveDraft extends Handler { 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; diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/CommentRange.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/CommentRange.java new file mode 100644 index 0000000000..b0f3879ae0 --- /dev/null +++ b/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 + "]"; + } +} \ No newline at end of file diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchLineComment.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchLineComment.java index af35e52fa2..0119c3e811 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/PatchLineComment.java +++ b/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; + } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java index c0942b2aef..fe372f3dfd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CommentInfo.java +++ b/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()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java index 9ef1cfe073..1b7bbe7dbb 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraft.java +++ b/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 { 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, - rsrc.getAccountId(), - Url.decode(in.inReplyTo)); - c.setStatus(Status.DRAFT); + line, rsrc.getAccountId(), Url.decode(in.inReplyTo)); 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)); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index b9ca124e17..91c6abe358 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/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 { int line; String inReplyTo; String message; + CommentRange range; } static class Output { @@ -365,6 +367,7 @@ public class PostReview implements RestModifyView { 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); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java index c6fc4fcee3..005d493a3d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraft.java +++ b/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 { Integer line; String inReplyTo; Timestamp updated; // Accepted but ignored. + CommentRange range; @DefaultInput String message; @@ -67,6 +69,8 @@ class PutDraft implements RestModifyView { 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 { 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; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java index ac59f65e32..9a89064a60 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/SchemaVersion.java +++ b/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 C = Schema_79.class; + public static final Class C = Schema_80.class; public static class Module extends AbstractModule { @Override diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_80.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_80.java new file mode 100644 index 0000000000..2cf8d2a1bf --- /dev/null +++ b/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 prior) { + super(prior); + } +} diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java index 6a9a93fdcd..f1d986d776 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/change/CommentsTest.java +++ b/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));