diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java index 9667c52f95..0c628ff092 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java +++ b/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> 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> 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> 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; diff --git a/gerrit-extension-api/BUCK b/gerrit-extension-api/BUCK index aaa641c516..5cce9aa400 100644 --- a/gerrit-extension-api/BUCK +++ b/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', diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DraftInput.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/DraftInput.java new file mode 100644 index 0000000000..3176870cc5 --- /dev/null +++ b/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 { +} diff --git a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/Comment.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/Comment.java index b7ad9a7dd1..a2c9fea549 100644 --- a/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/Comment.java +++ b/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; diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/changes/Side.java b/gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/Side.java similarity index 80% rename from gerrit-common/src/main/java/com/google/gerrit/common/changes/Side.java rename to gerrit-extension-api/src/main/java/com/google/gerrit/extensions/common/Side.java index 4a9ddf8cc6..1942f1edb7 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/changes/Side.java +++ b/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 } \ No newline at end of file diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LineComment.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LineComment.java index 6540638b16..f580035567 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/change/LineComment.java +++ b/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; 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 e1ec47a5db..903acdb8fb 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 @@ -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; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java index c829976842..f8dc50ec15 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/diff/CommentManager.java +++ b/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; diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java index e9832c35f4..8d9a368368 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/patches/CommentEditorPanel.java +++ b/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; 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 d8fb1159c3..52113da742 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 @@ -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); + } } 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 23da8385a9..f8867a98a3 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 @@ -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; + } } 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 1cd39bebbd..03a262c136 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 @@ -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 { +class CreateDraft implements RestModifyView { private final Provider db; private final ChangeUpdate.Factory updateFactory; private final PatchLineCommentsUtil plcUtil; @@ -59,7 +59,7 @@ class CreateDraft implements RestModifyView { } @Override - public Response apply(RevisionResource rsrc, Input in) + public Response 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 { 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 { 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(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java index be047db645..89535b2fdf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ListDrafts.java +++ b/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; 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 a01f049f88..74619ac512 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 @@ -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; 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 949c0c6b61..3a9f50cb66 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 @@ -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 { - 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 { private final Provider db; private final DeleteDraft delete; @@ -75,7 +60,7 @@ class PutDraft implements RestModifyView { } @Override - public Response apply(DraftResource rsrc, Input in) throws + public Response 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 { 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 { 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 { } 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; 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 206e4c67d0..d550dc20b4 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 @@ -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,