diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java index c095551b3b..755e0c0914 100644 --- a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java +++ b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java @@ -25,4 +25,24 @@ public interface PerPatchsetOperations { * @return the corresponding {@code TestPatchset} */ TestPatchset get(); + + /** + * Starts the fluent chain to create a new comment. The returned builder can be used to specify + * the attributes of the new comment. To create the comment for real, {@link + * TestCommentCreation.Builder#create()} must be called. + * + *

Example: + * + *

+   * String createdCommentUuid = changeOperations
+   *     .change(changeId)
+   *     .currentPatchset()
+   *     .onLine(2)
+   *     .ofFile("file1")
+   *     .create();
+   * 
+ * + * @return builder to create a new comment + */ + TestCommentCreation.Builder newComment(); } diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java index 8c9a495faf..47b23e254b 100644 --- a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java +++ b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java @@ -14,10 +14,36 @@ package com.google.gerrit.acceptance.testsuite.change; +import static com.google.gerrit.server.CommentsUtil.setCommentCommitId; + +import com.google.gerrit.acceptance.testsuite.change.TestCommentCreation.CommentSide; +import com.google.gerrit.entities.HumanComment; +import com.google.gerrit.entities.Patch; import com.google.gerrit.entities.PatchSet; +import com.google.gerrit.entities.Project; +import com.google.gerrit.extensions.client.Comment; +import com.google.gerrit.extensions.client.Comment.Range; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.extensions.restapi.UnprocessableEntityException; +import com.google.gerrit.server.CommentsUtil; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.IdentifiedUser.GenericFactory; +import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.patch.PatchListCache; +import com.google.gerrit.server.patch.PatchListNotAvailableException; +import com.google.gerrit.server.update.BatchUpdate; +import com.google.gerrit.server.update.BatchUpdateOp; +import com.google.gerrit.server.update.ChangeContext; +import com.google.gerrit.server.update.UpdateException; +import com.google.gerrit.server.util.time.TimeUtil; import com.google.inject.Inject; import com.google.inject.assistedinject.Assisted; +import java.io.IOException; +import java.sql.Timestamp; +import org.eclipse.jgit.lib.ObjectInserter; +import org.eclipse.jgit.lib.Repository; +import org.eclipse.jgit.revwalk.RevWalk; /** * The implementation of {@link PerPatchsetOperations}. @@ -26,6 +52,12 @@ import com.google.inject.assistedinject.Assisted; * separation between interface and implementation to enhance clarity. */ public class PerPatchsetOperationsImpl implements PerPatchsetOperations { + private final GitRepositoryManager repositoryManager; + private final IdentifiedUser.GenericFactory userFactory; + private final BatchUpdate.Factory batchUpdateFactory; + private final CommentsUtil commentsUtil; + private final PatchListCache patchListCache; + private final ChangeNotes changeNotes; private final PatchSet.Id patchsetId; @@ -35,7 +67,18 @@ public class PerPatchsetOperationsImpl implements PerPatchsetOperations { @Inject private PerPatchsetOperationsImpl( - @Assisted ChangeNotes changeNotes, @Assisted PatchSet.Id patchsetId) { + GitRepositoryManager repositoryManager, + GenericFactory userFactory, + BatchUpdate.Factory batchUpdateFactory, + CommentsUtil commentsUtil, + PatchListCache patchListCache, + @Assisted ChangeNotes changeNotes, + @Assisted PatchSet.Id patchsetId) { + this.repositoryManager = repositoryManager; + this.userFactory = userFactory; + this.batchUpdateFactory = batchUpdateFactory; + this.commentsUtil = commentsUtil; + this.patchListCache = patchListCache; this.changeNotes = changeNotes; this.patchsetId = patchsetId; } @@ -45,4 +88,83 @@ public class PerPatchsetOperationsImpl implements PerPatchsetOperations { PatchSet patchset = changeNotes.getPatchSets().get(patchsetId); return TestPatchset.builder().patchsetId(patchsetId).commitId(patchset.commitId()).build(); } + + @Override + public TestCommentCreation.Builder newComment() { + return TestCommentCreation.builder(this::createComment); + } + + private String createComment(TestCommentCreation commentCreation) + throws IOException, RestApiException, UpdateException { + Project.NameKey project = changeNotes.getProjectName(); + + try (Repository repository = repositoryManager.openRepository(project); + ObjectInserter objectInserter = repository.newObjectInserter(); + RevWalk revWalk = new RevWalk(objectInserter.newReader())) { + Timestamp now = TimeUtil.nowTs(); + + // Use identity of change owner until the API allows to specify the commenter. + IdentifiedUser changeOwner = userFactory.create(changeNotes.getChange().getOwner()); + CommentAdditionOp commentAdditionOp = new CommentAdditionOp(commentCreation); + try (BatchUpdate batchUpdate = batchUpdateFactory.create(project, changeOwner, now)) { + batchUpdate.setRepository(repository, revWalk, objectInserter); + batchUpdate.addOp(changeNotes.getChangeId(), commentAdditionOp); + batchUpdate.execute(); + } + return commentAdditionOp.createdCommentUuid; + } + } + + private class CommentAdditionOp implements BatchUpdateOp { + private String createdCommentUuid; + private final TestCommentCreation commentCreation; + + public CommentAdditionOp(TestCommentCreation commentCreation) { + this.commentCreation = commentCreation; + } + + @Override + public boolean updateChange(ChangeContext context) throws Exception { + HumanComment comment = toNewComment(context, commentCreation); + context.getUpdate(patchsetId).putComment(HumanComment.Status.PUBLISHED, comment); + createdCommentUuid = comment.key.uuid; + return true; + } + + private HumanComment toNewComment(ChangeContext context, TestCommentCreation commentCreation) + throws UnprocessableEntityException, PatchListNotAvailableException { + String message = commentCreation.message().orElse("The text of a test comment."); + + String filePath = commentCreation.file().orElse(Patch.PATCHSET_LEVEL); + short side = commentCreation.side().orElse(CommentSide.PATCHSET_COMMIT).getNumericSide(); + Boolean unresolved = commentCreation.unresolved().orElse(null); + String parentUuid = commentCreation.parentUuid().orElse(null); + HumanComment newComment = + commentsUtil.newHumanComment( + context, filePath, patchsetId, side, message, unresolved, parentUuid); + + commentCreation.line().ifPresent(line -> newComment.setLineNbrAndRange(line, null)); + // Specification of range trumps explicit line specification. + commentCreation + .range() + .map(this::toCommentRange) + .ifPresent(range -> newComment.setLineNbrAndRange(null, range)); + + setCommentCommitId( + newComment, + patchListCache, + context.getChange(), + changeNotes.getPatchSets().get(patchsetId)); + return newComment; + } + + private Comment.Range toCommentRange(TestRange range) { + Comment.Range commentRange = new Range(); + commentRange.startLine = range.start().line(); + commentRange.startCharacter = range.start().charOffset(); + commentRange.endLine = range.end().line(); + commentRange.endCharacter = range.end().charOffset(); + return commentRange; + } + } } diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java new file mode 100644 index 0000000000..b61e4b6767 --- /dev/null +++ b/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java @@ -0,0 +1,241 @@ +// Copyright (C) 2020 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.acceptance.testsuite.change; + +import com.google.auto.value.AutoValue; +import com.google.gerrit.acceptance.testsuite.ThrowingFunction; +import com.google.gerrit.acceptance.testsuite.change.TestRange.Position; +import com.google.gerrit.common.Nullable; +import com.google.gerrit.entities.Patch; +import java.util.Optional; +import java.util.function.Function; +import java.util.function.IntFunction; + +/** Attributes of the comment. If not provided, arbitrary values will be used. */ +@AutoValue +public abstract class TestCommentCreation { + + public abstract Optional message(); + + public abstract Optional file(); + + public abstract Optional line(); + + public abstract Optional range(); + + public abstract Optional side(); + + public abstract Optional unresolved(); + + public abstract Optional parentUuid(); + + abstract ThrowingFunction commentCreator(); + + public static TestCommentCreation.Builder builder( + ThrowingFunction commentCreator) { + return new AutoValue_TestCommentCreation.Builder().commentCreator(commentCreator); + } + + @AutoValue.Builder + public abstract static class Builder { + + public Builder noMessage() { + return message(""); + } + + /** Message text of the comment. */ + public abstract Builder message(String message); + + /** Indicates a patchset-level comment. */ + public Builder onPatchsetLevel() { + return file(Patch.PATCHSET_LEVEL); + } + + /** Indicates a file comment. The comment will be on the specified file. */ + public Builder onFileLevelOf(String filePath) { + return file(filePath).line(null).range(null); + } + + /** + * Starts the fluent change to create a line comment. The line comment will be at the indicated + * line. Lines start with 1. + */ + public FileBuilder onLine(int line) { + return new FileBuilder(file -> file(file).line(line).range(null)); + } + + /** + * Starts the fluent chain to create a range comment. The range begins at the specified line. + * Lines start at 1. The start position (line, charOffset) is inclusive, the end position (line, + * charOffset) is exclusive. + */ + public PositionBuilder fromLine(int startLine) { + return new PositionBuilder<>( + startCharOffset -> { + Position start = Position.builder().line(startLine).charOffset(startCharOffset).build(); + TestRange.Builder testRangeBuilder = TestRange.builder().setStart(start); + return new StartAwarePositionBuilder(this, testRangeBuilder); + }); + } + + /** File on which the comment should be added. */ + abstract Builder file(String filePath); + + /** Line on which the comment should be added. */ + abstract Builder line(@Nullable Integer line); + + /** Range on which the comment should be added. */ + abstract Builder range(@Nullable TestRange range); + + /** + * Indicates that the comment refers to a file, line, range, ... in the commit of the patchset. + * + *

On the UI, such comments are shown on the right side of a diff view when a diff against + * base is selected. See {@link #onParentCommit()} for comments shown on the left side. + */ + public Builder onPatchsetCommit() { + return side(CommentSide.PATCHSET_COMMIT); + } + + /** + * Indicates that the comment refers to a file, line, range, ... in the parent commit of the + * patchset. + * + *

On the UI, such comments are shown on the left side of a diff view when a diff against + * base is selected. See {@link #onPatchsetCommit()} for comments shown on the right side. + * + *

For merge commits, this indicates the first parent commit. + */ + public Builder onParentCommit() { + return side(CommentSide.PARENT_COMMIT); + } + + /** Like {@link #onParentCommit()} but for the second parent of a merge commit. */ + public Builder onSecondParentCommit() { + return side(CommentSide.SECOND_PARENT_COMMIT); + } + + /** + * Like {@link #onParentCommit()} but for the AutoMerge commit created from the parents of a + * merge commit. + */ + public Builder onAutoMergeCommit() { + return side(CommentSide.AUTO_MERGE_COMMIT); + } + + abstract Builder side(CommentSide side); + + /** Indicates a resolved comment. */ + public Builder resolved() { + return unresolved(false); + } + + /** Indicates an unresolved comment. */ + public Builder unresolved() { + return unresolved(true); + } + + abstract Builder unresolved(boolean unresolved); + + /** + * UUID of another comment to which this comment is a reply. This comment must have similar + * attributes (e.g. file, line, side) as the parent comment. + */ + public abstract Builder parentUuid(String parentUuid); + + abstract TestCommentCreation.Builder commentCreator( + ThrowingFunction commentCreator); + + abstract TestCommentCreation autoBuild(); + + /** + * Creates the comment. + * + * @return the UUID of the created comment + */ + public String create() { + TestCommentCreation commentCreation = autoBuild(); + return commentCreation.commentCreator().applyAndThrowSilently(commentCreation); + } + } + + /** Builder for the file specification of line/range comments. */ + public static class FileBuilder { + private final Function nextStepProvider; + + private FileBuilder(Function nextStepProvider) { + this.nextStepProvider = nextStepProvider; + } + + /** File on which the comment should be added. */ + public Builder ofFile(String file) { + return nextStepProvider.apply(file); + } + } + + /** Builder to simplify a position specification. */ + public static class PositionBuilder { + private final IntFunction nextStepProvider; + + private PositionBuilder(IntFunction nextStepProvider) { + this.nextStepProvider = nextStepProvider; + } + + /** Character offset within the line. A value of 0 indicates the beginning of the line. */ + public T charOffset(int characterOffset) { + return nextStepProvider.apply(characterOffset); + } + } + + /** Builder for the end position of a range. */ + public static class StartAwarePositionBuilder { + private final TestCommentCreation.Builder testCommentCreationBuilder; + private final TestRange.Builder testRangeBuilder; + + private StartAwarePositionBuilder( + Builder testCommentCreationBuilder, TestRange.Builder testRangeBuilder) { + this.testCommentCreationBuilder = testCommentCreationBuilder; + this.testRangeBuilder = testRangeBuilder; + } + + /** Line of the end position of the range. */ + public PositionBuilder toLine(int endLine) { + return new PositionBuilder<>( + endCharOffset -> { + Position end = Position.builder().line(endLine).charOffset(endCharOffset).build(); + TestRange range = testRangeBuilder.setEnd(end).build(); + testCommentCreationBuilder.range(range); + return new FileBuilder(testCommentCreationBuilder::file); + }); + } + } + + enum CommentSide { + PATCHSET_COMMIT(1), + AUTO_MERGE_COMMIT(0), + PARENT_COMMIT(-1), + SECOND_PARENT_COMMIT(-2); + + private final short numericSide; + + CommentSide(int numericSide) { + this.numericSide = (short) numericSide; + } + + public short getNumericSide() { + return numericSide; + } + } +} diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestRange.java b/java/com/google/gerrit/acceptance/testsuite/change/TestRange.java new file mode 100644 index 0000000000..f5cb7db754 --- /dev/null +++ b/java/com/google/gerrit/acceptance/testsuite/change/TestRange.java @@ -0,0 +1,67 @@ +// Copyright (C) 2020 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.acceptance.testsuite.change; + +import com.google.auto.value.AutoValue; + +/** Representation of a range used for testing purposes. */ +@AutoValue +public abstract class TestRange { + + /** Start position of the range. (inclusive) */ + public abstract Position start(); + + /** End position of the range. (exclusive) */ + public abstract Position end(); + + static Builder builder() { + return new AutoValue_TestRange.Builder(); + } + + @AutoValue.Builder + abstract static class Builder { + + abstract Builder setStart(Position start); + + abstract Builder setEnd(Position end); + + abstract TestRange build(); + } + + /** Position (start/end) of a range. */ + @AutoValue + public abstract static class Position { + + /** 1-based line. */ + public abstract int line(); + + /** 0-based character offset within the line. */ + public abstract int charOffset(); + + static Builder builder() { + return new AutoValue_TestRange_Position.Builder(); + } + + @AutoValue.Builder + abstract static class Builder { + + abstract Builder line(int line); + + abstract Builder charOffset(int characterOffset); + + abstract Position build(); + } + } +} diff --git a/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java b/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java new file mode 100644 index 0000000000..869c58c6c7 --- /dev/null +++ b/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java @@ -0,0 +1,93 @@ +// Copyright (C) 2020 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.common.testing; + +import static com.google.common.truth.Truth.assertAbout; +import static com.google.gerrit.extensions.common.testing.RangeSubject.ranges; + +import com.google.common.truth.BooleanSubject; +import com.google.common.truth.ComparableSubject; +import com.google.common.truth.FailureMetadata; +import com.google.common.truth.IntegerSubject; +import com.google.common.truth.StringSubject; +import com.google.common.truth.Subject; +import com.google.gerrit.extensions.client.Side; +import com.google.gerrit.extensions.common.CommentInfo; +import com.google.gerrit.truth.ListSubject; +import java.util.List; + +public class CommentInfoSubject extends Subject { + + public static ListSubject assertThatList( + List commentInfos) { + return ListSubject.assertThat(commentInfos, comments()); + } + + public static CommentInfoSubject assertThat(CommentInfo commentInfo) { + return assertAbout(comments()).that(commentInfo); + } + + private static Factory comments() { + return CommentInfoSubject::new; + } + + private final CommentInfo commentInfo; + + private CommentInfoSubject(FailureMetadata failureMetadata, CommentInfo commentInfo) { + super(failureMetadata, commentInfo); + this.commentInfo = commentInfo; + } + + public IntegerSubject patchSet() { + return check("patchSet").that(commentInfo().patchSet); + } + + public StringSubject path() { + return check("path").that(commentInfo().path); + } + + public IntegerSubject line() { + return check("line").that(commentInfo().line); + } + + public RangeSubject range() { + return check("range").about(ranges()).that(commentInfo().range); + } + + public StringSubject message() { + return check("message").that(commentInfo().message); + } + + public ComparableSubject side() { + return check("side").that(commentInfo().side); + } + + public IntegerSubject parent() { + return check("parent").that(commentInfo().parent); + } + + public BooleanSubject unresolved() { + return check("unresolved").that(commentInfo().unresolved); + } + + public StringSubject inReplyTo() { + return check("inReplyTo").that(commentInfo().inReplyTo); + } + + private CommentInfo commentInfo() { + isNotNull(); + return commentInfo; + } +} diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java new file mode 100644 index 0000000000..998a012e94 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java @@ -0,0 +1,300 @@ +// Copyright (C) 2020 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.acceptance.testsuite.change; + +import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThat; +import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThatList; + +import com.google.common.truth.Correspondence; +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.Patch; +import com.google.gerrit.entities.PatchSet; +import com.google.gerrit.extensions.client.Side; +import com.google.gerrit.extensions.common.CommentInfo; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.truth.NullAwareCorrespondence; +import com.google.inject.Inject; +import java.util.List; +import org.junit.Test; + +public class PatchsetOperationsImplTest extends AbstractDaemonTest { + + @Inject private ChangeOperations changeOperations; + + @Test + public void commentCanBeCreatedWithoutSpecifyingAnyParameters() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = changeOperations.change(changeId).currentPatchset().newComment().create(); + + List comments = getCommentsFromServer(changeId); + assertThatList(comments).comparingElementsUsing(hasUuid()).containsExactly(commentUuid); + } + + @Test + public void commentCanBeCreatedOnOlderPatchset() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + PatchSet.Id previousPatchsetId = + changeOperations.change(changeId).currentPatchset().get().patchsetId(); + changeOperations.change(changeId).newPatchset().create(); + + String commentUuid = + changeOperations.change(changeId).patchset(previousPatchsetId).newComment().create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).patchSet().isEqualTo(previousPatchsetId.get()); + } + + @Test + public void commentIsCreatedWithSpecifiedMessage() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = + changeOperations + .change(changeId) + .currentPatchset() + .newComment() + .message("Test comment message") + .create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).message().isEqualTo("Test comment message"); + } + + @Test + public void commentCanBeCreatedWithEmptyMessage() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = + changeOperations.change(changeId).currentPatchset().newComment().noMessage().create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).message().isNull(); + } + + @Test + public void patchsetLevelCommentCanBeCreated() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = + changeOperations.change(changeId).currentPatchset().newComment().onPatchsetLevel().create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).path().isEqualTo(Patch.PATCHSET_LEVEL); + } + + @Test + public void fileCommentCanBeCreated() throws Exception { + Change.Id changeId = changeOperations.newChange().file("file1").content("Line 1").create(); + + String commentUuid = + changeOperations + .change(changeId) + .currentPatchset() + .newComment() + .onFileLevelOf("file1") + .create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).path().isEqualTo("file1"); + assertThat(comment).line().isNull(); + assertThat(comment).range().isNull(); + } + + @Test + public void lineCommentCanBeCreated() throws Exception { + Change.Id changeId = + changeOperations + .newChange() + .file("file1") + .content("Line 1\nLine 2\nLine 3\nLine 4\n") + .create(); + + String commentUuid = + changeOperations + .change(changeId) + .currentPatchset() + .newComment() + .onLine(3) + .ofFile("file1") + .create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).line().isEqualTo(3); + assertThat(comment).range().isNull(); + } + + @Test + public void rangeCommentCanBeCreated() throws Exception { + Change.Id changeId = + changeOperations + .newChange() + .file("file1") + .content("Line 1\nLine 2\nLine 3\nLine 4\n") + .create(); + + String commentUuid = + changeOperations + .change(changeId) + .currentPatchset() + .newComment() + .fromLine(2) + .charOffset(4) + .toLine(3) + .charOffset(5) + .ofFile("file1") + .create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).range().startLine().isEqualTo(2); + assertThat(comment).range().startCharacter().isEqualTo(4); + assertThat(comment).range().endLine().isEqualTo(3); + assertThat(comment).range().endCharacter().isEqualTo(5); + // Line is automatically filled from specified range. It's the end line. + assertThat(comment).line().isEqualTo(3); + } + + @Test + public void commentCanBeCreatedOnPatchsetCommit() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = + changeOperations + .change(changeId) + .currentPatchset() + .newComment() + .onPatchsetCommit() + .create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + // Null is often used instead of Side.REVISION as Side.REVISION is the default. + assertThat(comment).side().isAnyOf(Side.REVISION, null); + assertThat(comment).parent().isNull(); + } + + @Test + public void commentCanBeCreatedOnParentCommit() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = + changeOperations.change(changeId).currentPatchset().newComment().onParentCommit().create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).side().isEqualTo(Side.PARENT); + assertThat(comment).parent().isEqualTo(1); + } + + @Test + public void commentCanBeCreatedOnSecondParentCommit() throws Exception { + // Second parents only exist for merge commits. The test API currently doesn't support the + // creation of changes with merge commits yet, though. As there's no explicit validation keeping + // us from adding comments on the non-existing second parent of a regular commit, just use the + // latter. That's still better than not having this test at all. + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = + changeOperations + .change(changeId) + .currentPatchset() + .newComment() + .onSecondParentCommit() + .create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).side().isEqualTo(Side.PARENT); + assertThat(comment).parent().isEqualTo(2); + } + + @Test + public void commentCanBeCreatedOnAutoMergeCommit() throws Exception { + // Second parents only exist for merge commits. The test API currently doesn't support the + // creation of changes with merge commits yet, though. As there's no explicit validation keeping + // us from adding comments on the non-existing second parent of a regular commit, just use the + // latter. That's still better than not having this test at all. + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = + changeOperations + .change(changeId) + .currentPatchset() + .newComment() + .onAutoMergeCommit() + .create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).side().isEqualTo(Side.PARENT); + assertThat(comment).parent().isNull(); + } + + @Test + public void commentCanBeCreatedAsResolved() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = + changeOperations.change(changeId).currentPatchset().newComment().resolved().create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).unresolved().isFalse(); + } + + @Test + public void commentCanBeCreatedAsUnresolved() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + + String commentUuid = + changeOperations.change(changeId).currentPatchset().newComment().unresolved().create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).unresolved().isTrue(); + } + + @Test + public void replyToCommentCanBeCreated() throws Exception { + Change.Id changeId = changeOperations.newChange().create(); + String parentCommentUuid = + changeOperations.change(changeId).currentPatchset().newComment().create(); + + String commentUuid = + changeOperations + .change(changeId) + .currentPatchset() + .newComment() + .parentUuid(parentCommentUuid) + .create(); + + CommentInfo comment = getCommentFromServer(changeId, commentUuid); + assertThat(comment).inReplyTo().isEqualTo(parentCommentUuid); + } + + private List getCommentsFromServer(Change.Id changeId) throws RestApiException { + return gApi.changes().id(changeId.get()).commentsAsList(); + } + + private CommentInfo getCommentFromServer(Change.Id changeId, String uuid) + throws RestApiException { + return gApi.changes().id(changeId.get()).commentsAsList().stream() + .filter(comment -> comment.id.equals(uuid)) + .findAny() + .orElseThrow( + () -> + new IllegalStateException( + String.format("Comment %s not found on change %d", uuid, changeId.get()))); + } + + private Correspondence hasUuid() { + return NullAwareCorrespondence.transforming(comment -> comment.id, "hasUuid"); + } +}