Merge changes I343425ab,Ieaa18566
* changes:
Do not allow creations of comments with invalid inReplyTo
Add robot comments creation and retrieval to the test api
diff --git a/java/com/google/gerrit/acceptance/GerritServer.java b/java/com/google/gerrit/acceptance/GerritServer.java
index 50d11a1..5942c0f 100644
--- a/java/com/google/gerrit/acceptance/GerritServer.java
+++ b/java/com/google/gerrit/acceptance/GerritServer.java
@@ -35,6 +35,7 @@
import com.google.gerrit.acceptance.testsuite.change.PerCommentOperationsImpl;
import com.google.gerrit.acceptance.testsuite.change.PerDraftCommentOperationsImpl;
import com.google.gerrit.acceptance.testsuite.change.PerPatchsetOperationsImpl;
+import com.google.gerrit.acceptance.testsuite.change.PerRobotCommentOperationsImpl;
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.group.GroupOperationsImpl;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
@@ -518,6 +519,7 @@
factory(PerPatchsetOperationsImpl.Factory.class);
factory(PerCommentOperationsImpl.Factory.class);
factory(PerDraftCommentOperationsImpl.Factory.class);
+ factory(PerRobotCommentOperationsImpl.Factory.class);
factory(PushOneCommit.Factory.class);
install(InProcessProtocol.module());
install(new NoSshModule());
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java
index aa0391d..c4e4192 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperations.java
@@ -127,5 +127,13 @@
* @return an aggregation of operations on a specific draft comment
*/
PerDraftCommentOperations draftComment(String commentUuid);
+
+ /**
+ * Starts the fluent chain for querying or modifying a robot comment. Please see the methods of
+ * {@link PerRobotCommentOperations} for details on possible operations.
+ *
+ * @return an aggregation of operations on a specific robot comment
+ */
+ PerRobotCommentOperations robotComment(String commentUuid);
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
index f04de17..3b15b57 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImpl.java
@@ -87,6 +87,7 @@
private final PerPatchsetOperationsImpl.Factory perPatchsetOperationsFactory;
private final PerCommentOperationsImpl.Factory perCommentOperationsFactory;
private final PerDraftCommentOperationsImpl.Factory perDraftCommentOperationsFactory;
+ private final PerRobotCommentOperationsImpl.Factory perRobotCommentOperationsFactory;
@Inject
public ChangeOperationsImpl(
@@ -102,7 +103,8 @@
ChangeFinder changeFinder,
PerPatchsetOperationsImpl.Factory perPatchsetOperationsFactory,
PerCommentOperationsImpl.Factory perCommentOperationsFactory,
- PerDraftCommentOperationsImpl.Factory perDraftCommentOperationsFactory) {
+ PerDraftCommentOperationsImpl.Factory perDraftCommentOperationsFactory,
+ PerRobotCommentOperationsImpl.Factory perRobotCommentOperationsFactory) {
this.seq = seq;
this.changeInserterFactory = changeInserterFactory;
this.patchsetInserterFactory = patchsetInserterFactory;
@@ -116,6 +118,7 @@
this.perPatchsetOperationsFactory = perPatchsetOperationsFactory;
this.perCommentOperationsFactory = perCommentOperationsFactory;
this.perDraftCommentOperationsFactory = perDraftCommentOperationsFactory;
+ this.perRobotCommentOperationsFactory = perRobotCommentOperationsFactory;
}
@Override
@@ -555,5 +558,11 @@
ChangeNotes changeNotes = getChangeNotes();
return perDraftCommentOperationsFactory.create(changeNotes, commentUuid);
}
+
+ @Override
+ public PerRobotCommentOperations robotComment(String commentUuid) {
+ ChangeNotes changeNotes = getChangeNotes();
+ return perRobotCommentOperationsFactory.create(changeNotes, commentUuid);
+ }
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/CommentSide.java b/java/com/google/gerrit/acceptance/testsuite/change/CommentSide.java
new file mode 100644
index 0000000..b7e720b
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/CommentSide.java
@@ -0,0 +1,36 @@
+// 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;
+
+/**
+ * Marks the commit that contains the comment (also known as side). Used by {@link
+ * TestCommentCreation} and {@link TestRobotCommentCreation}.
+ */
+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/FileBuilder.java b/java/com/google/gerrit/acceptance/testsuite/change/FileBuilder.java
new file mode 100644
index 0000000..c8514a7
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/FileBuilder.java
@@ -0,0 +1,33 @@
+// 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 java.util.function.Function;
+
+/**
+ * Builder for the file specification of line/range comments. Used by {@link TestCommentCreation}
+ * and {@link TestRobotCommentCreation}.
+ */
+public class FileBuilder<T> {
+ private final Function<String, T> nextStepProvider;
+
+ public FileBuilder(Function<String, T> nextStepProvider) {
+ this.nextStepProvider = nextStepProvider;
+ }
+ /** File on which the comment should be added. */
+ public T ofFile(String file) {
+ return nextStepProvider.apply(file);
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerCommentOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/PerCommentOperationsImpl.java
index 5f046ca..0218731 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/PerCommentOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerCommentOperationsImpl.java
@@ -56,6 +56,11 @@
}
static TestHumanComment toTestComment(HumanComment comment) {
- return TestHumanComment.builder().uuid(comment.key.uuid).parentUuid(comment.parentUuid).build();
+ return TestHumanComment.builder()
+ .uuid(comment.key.uuid)
+ .parentUuid(comment.parentUuid)
+ .tag(comment.tag)
+ .unresolved(comment.unresolved)
+ .build();
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java
index 33d2d43..f4c70bd 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperations.java
@@ -67,4 +67,25 @@
* @return builder to create a new comment
*/
TestCommentCreation.Builder newDraftComment();
+
+ /**
+ * Starts the fluent chain to create a new robot comment. The returned builder can be used to
+ * specify the attributes of the robot comment. To create the robot comment for real, {@link
+ * TestRobotCommentCreation.Builder#create()} must be called.
+ *
+ * <p>Example:
+ *
+ * <pre>
+ * String createdRobotCommentUuid = changeOperations
+ * .change(changeId)
+ * .currentPatchset()
+ * .newRobotComment()
+ * .onLine(2)
+ * .ofFile("file1")
+ * .create();
+ * </pre>
+ *
+ * @return builder to create a new comment
+ */
+ TestRobotCommentCreation.Builder newRobotComment();
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java
index d39f1e1..9f2e1f4 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerPatchsetOperationsImpl.java
@@ -16,13 +16,13 @@
import static com.google.gerrit.server.CommentsUtil.setCommentCommitId;
-import com.google.gerrit.acceptance.testsuite.change.TestCommentCreation.CommentSide;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Comment.Status;
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.entities.RobotComment;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.Comment.Range;
import com.google.gerrit.extensions.restapi.RestApiException;
@@ -101,6 +101,11 @@
return TestCommentCreation.builder(this::createComment, Status.DRAFT);
}
+ @Override
+ public TestRobotCommentCreation.Builder newRobotComment() {
+ return TestRobotCommentCreation.builder(this::createRobotComment);
+ }
+
private String createComment(TestCommentCreation commentCreation)
throws IOException, RestApiException, UpdateException {
Project.NameKey project = changeNotes.getProjectName();
@@ -126,6 +131,33 @@
return userFactory.create(authorId);
}
+ /**
+ * Both this and {@code toEntitiesCommentRange} is needed since there are two Comment.Range
+ * entities, in different packages: {@code com.google.gerrit.entities.Comment.Range}, and {@code
+ * com.google.gerrit.extensions.Comment.Range}
+ */
+ private static 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;
+ }
+
+ /**
+ * Both this and {@code toCommentRange} is needed since there are two Comment.Range entities, in
+ * different packages: {@code com.google.gerrit.entities.Comment.Range}, and {@code
+ * com.google.gerrit.extensions.Comment.Range}
+ */
+ private static com.google.gerrit.entities.Comment.Range toEntitiesCommentRange(TestRange range) {
+ return new com.google.gerrit.entities.Comment.Range(
+ range.start().line(),
+ range.start().charOffset(),
+ range.end().line(),
+ range.end().charOffset());
+ }
+
private class CommentAdditionOp implements BatchUpdateOp {
private String createdCommentUuid;
private final TestCommentCreation commentCreation;
@@ -173,7 +205,7 @@
// Specification of range trumps explicit line specification.
commentCreation
.range()
- .map(this::toCommentRange)
+ .map(PerPatchsetOperationsImpl::toCommentRange)
.ifPresent(range -> newComment.setLineNbrAndRange(null, range));
setCommentCommitId(
@@ -183,14 +215,94 @@
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;
+ private String createRobotComment(TestRobotCommentCreation robotCommentCreation)
+ 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();
+
+ IdentifiedUser author = getAuthor(robotCommentCreation);
+ RobotCommentAdditionOp robotCommentAdditionOp =
+ new RobotCommentAdditionOp(robotCommentCreation);
+ try (BatchUpdate batchUpdate = batchUpdateFactory.create(project, author, now)) {
+ batchUpdate.setRepository(repository, revWalk, objectInserter);
+ batchUpdate.addOp(changeNotes.getChangeId(), robotCommentAdditionOp);
+ batchUpdate.execute();
+ }
+ return robotCommentAdditionOp.createdRobotCommentUuid;
+ }
+ }
+
+ private IdentifiedUser getAuthor(TestRobotCommentCreation robotCommentCreation) {
+ Account.Id authorId = robotCommentCreation.author().orElse(changeNotes.getChange().getOwner());
+ return userFactory.create(authorId);
+ }
+
+ private class RobotCommentAdditionOp implements BatchUpdateOp {
+ private String createdRobotCommentUuid;
+ private final TestRobotCommentCreation robotCommentCreation;
+
+ public RobotCommentAdditionOp(TestRobotCommentCreation robotCommentCreation) {
+ this.robotCommentCreation = robotCommentCreation;
+ }
+
+ @Override
+ public boolean updateChange(ChangeContext context) throws Exception {
+ RobotComment robotComment = toNewRobotComment(context, robotCommentCreation);
+ ChangeUpdate changeUpdate = context.getUpdate(patchsetId);
+ changeUpdate.putRobotComment(robotComment);
+ // For robot comments, only the tag set on the ChangeUpdate (and not on the RobotComment)
+ // matters.
+ robotCommentCreation.tag().ifPresent(changeUpdate::setTag);
+ createdRobotCommentUuid = robotComment.key.uuid;
+ return true;
+ }
+
+ private RobotComment toNewRobotComment(
+ ChangeContext context, TestRobotCommentCreation robotCommentCreation)
+ throws PatchListNotAvailableException {
+ String message = robotCommentCreation.message().orElse("The text of a test robot comment.");
+
+ String filePath = robotCommentCreation.file().orElse(Patch.PATCHSET_LEVEL);
+ short side = robotCommentCreation.side().orElse(CommentSide.PATCHSET_COMMIT).getNumericSide();
+ String robotId = robotCommentCreation.robotId().orElse("robot");
+ String robotRunId = robotCommentCreation.robotId().orElse("1");
+ RobotComment newRobotComment =
+ commentsUtil.newRobotComment(
+ context, filePath, patchsetId, side, message, robotId, robotRunId);
+
+ // TODO(paiking): This should not be needed, as the tag only matters in ChangeUpdate.
+ robotCommentCreation.tag().ifPresent(tag -> newRobotComment.tag = tag);
+
+ robotCommentCreation.line().ifPresent(line -> newRobotComment.setLineNbrAndRange(line, null));
+ // Specification of range trumps explicit line specification.
+ robotCommentCreation
+ .range()
+ .map(PerPatchsetOperationsImpl::toCommentRange)
+ .ifPresent(range -> newRobotComment.setLineNbrAndRange(null, range));
+
+ robotCommentCreation
+ .unresolved()
+ .ifPresent(unresolved -> newRobotComment.unresolved = unresolved);
+ robotCommentCreation
+ .parentUuid()
+ .ifPresent(parentUuid -> newRobotComment.parentUuid = parentUuid);
+ robotCommentCreation.url().ifPresent(url -> newRobotComment.url = url);
+ if (!robotCommentCreation.properties().isEmpty()) {
+ newRobotComment.properties = robotCommentCreation.properties();
+ }
+
+ setCommentCommitId(
+ newRobotComment,
+ patchListCache,
+ context.getChange(),
+ changeNotes.getPatchSets().get(patchsetId));
+ return newRobotComment;
}
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperations.java b/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperations.java
new file mode 100644
index 0000000..c9718aa
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperations.java
@@ -0,0 +1,29 @@
+// 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;
+
+/** An aggregation of methods on a specific, robot comment. */
+public interface PerRobotCommentOperations {
+
+ /**
+ * Retrieves the robot comment.
+ *
+ * <p><strong>Note:</strong> This call will fail with an exception if the requested comment
+ * doesn't exist or if it is a comment of another type.
+ *
+ * @return the corresponding {@code TestRobotComment}
+ */
+ TestRobotComment get();
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperationsImpl.java
new file mode 100644
index 0000000..075c451
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PerRobotCommentOperationsImpl.java
@@ -0,0 +1,64 @@
+// 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.common.collect.MoreCollectors.onlyElement;
+
+import com.google.gerrit.entities.RobotComment;
+import com.google.gerrit.server.CommentsUtil;
+import com.google.gerrit.server.notedb.ChangeNotes;
+import com.google.inject.Inject;
+import com.google.inject.assistedinject.Assisted;
+
+/**
+ * The implementation of {@link PerRobotCommentOperations}.
+ *
+ * <p>There is only one implementation of {@link PerRobotCommentOperations}. Nevertheless, we keep
+ * the separation between interface and implementation to enhance clarity.
+ */
+public class PerRobotCommentOperationsImpl implements PerRobotCommentOperations {
+ private final CommentsUtil commentsUtil;
+
+ private final ChangeNotes changeNotes;
+ private final String commentUuid;
+
+ public interface Factory {
+ PerRobotCommentOperationsImpl create(ChangeNotes changeNotes, String commentUuid);
+ }
+
+ @Inject
+ public PerRobotCommentOperationsImpl(
+ CommentsUtil commentsUtil, @Assisted ChangeNotes changeNotes, @Assisted String commentUuid) {
+ this.commentsUtil = commentsUtil;
+ this.changeNotes = changeNotes;
+ this.commentUuid = commentUuid;
+ }
+
+ @Override
+ public TestRobotComment get() {
+ RobotComment comment =
+ commentsUtil.robotCommentsByChange(changeNotes).stream()
+ .filter(foundComment -> foundComment.key.uuid.equals(commentUuid))
+ .collect(onlyElement());
+ return toTestRobotComment(comment);
+ }
+
+ static TestRobotComment toTestRobotComment(RobotComment robotComment) {
+ return TestRobotComment.builder()
+ .uuid(robotComment.key.uuid)
+ .parentUuid(robotComment.parentUuid)
+ .build();
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/PositionBuilder.java b/java/com/google/gerrit/acceptance/testsuite/change/PositionBuilder.java
new file mode 100644
index 0000000..b061c81
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/PositionBuilder.java
@@ -0,0 +1,34 @@
+// 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 java.util.function.IntFunction;
+
+/**
+ * Builder to simplify a position specification. Used by {@link TestCommentCreation} and {@link
+ * TestRobotCommentCreation}.
+ */
+public class PositionBuilder<T> {
+ private final IntFunction<T> nextStepProvider;
+
+ public PositionBuilder(IntFunction<T> 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);
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/StartAwarePositionBuilder.java b/java/com/google/gerrit/acceptance/testsuite/change/StartAwarePositionBuilder.java
new file mode 100644
index 0000000..c9a0eff
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/StartAwarePositionBuilder.java
@@ -0,0 +1,49 @@
+// 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 java.util.function.Consumer;
+import java.util.function.Function;
+
+/**
+ * Builder for the end position of a range. Used by {@link TestCommentCreation} and {@link
+ * TestRobotCommentCreation}.
+ */
+public class StartAwarePositionBuilder<T> {
+ private final TestRange.Builder testRangeBuilder;
+ private final Consumer<TestRange> rangeConsumer;
+ private final Function<String, T> fileFunction;
+
+ public StartAwarePositionBuilder(
+ TestRange.Builder testRangeBuilder,
+ Consumer<TestRange> rangeConsumer,
+ Function<String, T> fileFunction) {
+ this.testRangeBuilder = testRangeBuilder;
+ this.rangeConsumer = rangeConsumer;
+ this.fileFunction = fileFunction;
+ }
+
+ /** Line of the end position of the range. */
+ public PositionBuilder<FileBuilder<T>> toLine(int endLine) {
+ return new PositionBuilder<>(
+ endCharOffset -> {
+ TestRange.Position end =
+ TestRange.Position.builder().line(endLine).charOffset(endCharOffset).build();
+ TestRange range = testRangeBuilder.setEnd(end).build();
+ rangeConsumer.accept(range);
+ return new FileBuilder<T>(fileFunction);
+ });
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java
index d1d1567..2f4ddd0 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestCommentCreation.java
@@ -22,10 +22,12 @@
import com.google.gerrit.entities.Comment;
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. */
+/**
+ * Attributes of the human comment. If not provided, arbitrary values will be used. This class is
+ * very similar to {@link TestRobotCommentCreation} to allow separation between robot and human
+ * comments.
+ */
@AutoValue
public abstract class TestCommentCreation {
@@ -51,7 +53,7 @@
abstract ThrowingFunction<TestCommentCreation, String> commentCreator();
- public static TestCommentCreation.Builder builder(
+ public static Builder builder(
ThrowingFunction<TestCommentCreation, String> commentCreator, Comment.Status commentStatus) {
return new AutoValue_TestCommentCreation.Builder()
.commentCreator(commentCreator)
@@ -82,8 +84,8 @@
* 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));
+ public FileBuilder<Builder> onLine(int line) {
+ return new FileBuilder<>(file -> file(file).line(line).range(null));
}
/**
@@ -91,12 +93,12 @@
* Lines start at 1. The start position (line, charOffset) is inclusive, the end position (line,
* charOffset) is exclusive.
*/
- public PositionBuilder<StartAwarePositionBuilder> fromLine(int startLine) {
+ public PositionBuilder<StartAwarePositionBuilder<Builder>> 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);
+ return new StartAwarePositionBuilder<>(testRangeBuilder, this::range, this::file);
});
}
@@ -179,8 +181,7 @@
*/
abstract Builder status(Comment.Status value);
- abstract TestCommentCreation.Builder commentCreator(
- ThrowingFunction<TestCommentCreation, String> commentCreator);
+ abstract Builder commentCreator(ThrowingFunction<TestCommentCreation, String> commentCreator);
abstract TestCommentCreation autoBuild();
@@ -194,72 +195,4 @@
return commentCreation.commentCreator().applyAndThrowSilently(commentCreation);
}
}
-
- /** Builder for the file specification of line/range comments. */
- public static class FileBuilder {
- private final Function<String, Builder> nextStepProvider;
-
- private FileBuilder(Function<String, Builder> 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<T> {
- private final IntFunction<T> nextStepProvider;
-
- private PositionBuilder(IntFunction<T> 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<FileBuilder> 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/TestHumanComment.java b/java/com/google/gerrit/acceptance/testsuite/change/TestHumanComment.java
index 9bb026f..3a7f2ae 100644
--- a/java/com/google/gerrit/acceptance/testsuite/change/TestHumanComment.java
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestHumanComment.java
@@ -28,6 +28,12 @@
/** UUID of another comment to which this comment is a reply. */
public abstract Optional<String> parentUuid();
+ /** Tag of a comment. */
+ public abstract Optional<String> tag();
+
+ /** Unresolved state of a comment. */
+ public abstract boolean unresolved();
+
static Builder builder() {
return new AutoValue_TestHumanComment.Builder();
}
@@ -38,6 +44,10 @@
abstract Builder parentUuid(@Nullable String parentUuid);
+ abstract Builder tag(@Nullable String tag);
+
+ abstract Builder unresolved(boolean unresolved);
+
abstract TestHumanComment build();
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestRobotComment.java b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotComment.java
new file mode 100644
index 0000000..76fb52f
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotComment.java
@@ -0,0 +1,43 @@
+// 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.common.Nullable;
+import java.util.Optional;
+
+/** Representation of a robot comment used for testing purposes. */
+@AutoValue
+public abstract class TestRobotComment {
+
+ /** The UUID of the comment. Should be unique. */
+ public abstract String uuid();
+
+ /** UUID of another comment to which this comment is a reply. */
+ public abstract Optional<String> parentUuid();
+
+ static Builder builder() {
+ return new AutoValue_TestRobotComment.Builder();
+ }
+
+ @AutoValue.Builder
+ abstract static class Builder {
+ abstract Builder uuid(String uuid);
+
+ abstract Builder parentUuid(@Nullable String parentUuid);
+
+ abstract TestRobotComment build();
+ }
+}
diff --git a/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java
new file mode 100644
index 0000000..809190d
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/change/TestRobotCommentCreation.java
@@ -0,0 +1,214 @@
+// 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.common.collect.ImmutableMap;
+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.Account;
+import com.google.gerrit.entities.Patch;
+import java.util.Map;
+import java.util.Optional;
+
+/**
+ * Attributes of the robot comment. If not provided, arbitrary values will be used. This class is
+ * very similar to {@link TestCommentCreation} to allow separation between robot and human comments.
+ */
+@AutoValue
+public abstract class TestRobotCommentCreation {
+
+ public abstract Optional<String> message();
+
+ public abstract Optional<String> file();
+
+ public abstract Optional<Integer> line();
+
+ public abstract Optional<TestRange> range();
+
+ public abstract Optional<CommentSide> side();
+
+ public abstract Optional<Boolean> unresolved();
+
+ public abstract Optional<String> parentUuid();
+
+ public abstract Optional<String> tag();
+
+ public abstract Optional<Account.Id> author();
+
+ public abstract Optional<String> robotId();
+
+ public abstract Optional<String> robotRunId();
+
+ public abstract Optional<String> url();
+
+ public abstract ImmutableMap<String, String> properties();
+
+ abstract ThrowingFunction<TestRobotCommentCreation, String> commentCreator();
+
+ public static Builder builder(ThrowingFunction<TestRobotCommentCreation, String> commentCreator) {
+ return new AutoValue_TestRobotCommentCreation.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<Builder> 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<StartAwarePositionBuilder<Builder>> 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<>(testRangeBuilder, this::range, this::file);
+ });
+ }
+
+ /** 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.
+ *
+ * <p>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.
+ *
+ * <p>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.
+ *
+ * <p>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. The parent comment must be a
+ * published comment.
+ */
+ public abstract Builder parentUuid(String parentUuid);
+
+ /** Tag to attach to the comment. */
+ public abstract Builder tag(String value);
+
+ /** Author of the comment. Must be an existing user account. */
+ public abstract Builder author(Account.Id accountId);
+
+ /** Id of the robot that created the comment. */
+ public abstract Builder robotId(String robotId);
+
+ /** An ID of the run of the robot that created the comment. */
+ public abstract Builder robotRunId(String robotRunId);
+
+ /** Url for more information for the robot comment. */
+ public abstract Builder url(String url);
+
+ /** Robot specific properties as map that maps arbitrary keys to values. */
+ public abstract Builder properties(Map<String, String> properties);
+
+ abstract ImmutableMap.Builder<String, String> propertiesBuilder();
+
+ public Builder addProperty(String key, String value) {
+ propertiesBuilder().put(key, value);
+ return this;
+ }
+
+ abstract Builder commentCreator(
+ ThrowingFunction<TestRobotCommentCreation, String> commentCreator);
+
+ abstract TestRobotCommentCreation autoBuild();
+
+ /**
+ * Creates the robot comment.
+ *
+ * @return the UUID of the created robot comment
+ */
+ public String create() {
+ TestRobotCommentCreation commentCreation = autoBuild();
+ return commentCreation.commentCreator().applyAndThrowSilently(commentCreation);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java b/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java
index dd226ed..5176145 100644
--- a/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java
+++ b/java/com/google/gerrit/extensions/common/testing/RobotCommentInfoSubject.java
@@ -18,6 +18,7 @@
import static com.google.gerrit.truth.ListSubject.elements;
import com.google.common.truth.FailureMetadata;
+import com.google.common.truth.MapSubject;
import com.google.common.truth.StringSubject;
import com.google.common.truth.Subject;
import com.google.gerrit.extensions.common.FixSuggestionInfo;
@@ -59,6 +60,26 @@
return check("path").that(robotCommentInfo.path);
}
+ public StringSubject robotId() {
+ isNotNull();
+ return check("robotId").that(robotCommentInfo.robotId);
+ }
+
+ public StringSubject robotRunId() {
+ isNotNull();
+ return check("robotRunId").that(robotCommentInfo.robotRunId);
+ }
+
+ public StringSubject url() {
+ isNotNull();
+ return check("url").that(robotCommentInfo.url);
+ }
+
+ public MapSubject properties() {
+ isNotNull();
+ return check("property").that(robotCommentInfo.properties);
+ }
+
public FixSuggestionInfoSubject onlyFixSuggestion() {
return fixSuggestions().onlyElement();
}
diff --git a/java/com/google/gerrit/server/CommentsUtil.java b/java/com/google/gerrit/server/CommentsUtil.java
index 30913f7..b83eae8 100644
--- a/java/com/google/gerrit/server/CommentsUtil.java
+++ b/java/com/google/gerrit/server/CommentsUtil.java
@@ -189,6 +189,12 @@
.findFirst();
}
+ public Optional<HumanComment> getPublishedHumanComment(ChangeNotes notes, String uuid) {
+ return publishedHumanCommentsByChange(notes).stream()
+ .filter(c -> c.key.uuid.equals(uuid))
+ .findFirst();
+ }
+
public Optional<HumanComment> getDraft(ChangeNotes notes, IdentifiedUser user, Comment.Key key) {
return draftByChangeAuthor(notes, user.getAccountId()).stream()
.filter(c -> key.equals(c.key))
@@ -205,6 +211,10 @@
return sort(Lists.newArrayList(notes.getRobotComments().values()));
}
+ public Optional<RobotComment> getRobotComment(ChangeNotes notes, String uuid) {
+ return robotCommentsByChange(notes).stream().filter(c -> c.key.uuid.equals(uuid)).findFirst();
+ }
+
public List<HumanComment> draftByChange(ChangeNotes notes) {
List<HumanComment> comments = new ArrayList<>();
for (Ref ref : getDraftRefs(notes.getChangeId())) {
diff --git a/java/com/google/gerrit/server/change/DraftCommentResource.java b/java/com/google/gerrit/server/change/DraftCommentResource.java
index e0648cf..19a495d 100644
--- a/java/com/google/gerrit/server/change/DraftCommentResource.java
+++ b/java/com/google/gerrit/server/change/DraftCommentResource.java
@@ -20,6 +20,7 @@
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.inject.TypeLiteral;
public class DraftCommentResource implements RestResource {
@@ -50,6 +51,10 @@
return comment;
}
+ public ChangeNotes getNotes() {
+ return rev.getNotes();
+ }
+
public String getId() {
return comment.key.uuid;
}
diff --git a/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java b/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
index b749e6a..399da8f 100644
--- a/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/CreateDraftComment.java
@@ -81,6 +81,11 @@
throw new BadRequestException("line must be >= 0");
} 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");
+ } else if (in.inReplyTo != null
+ && !commentsUtil.getPublishedHumanComment(rsrc.getNotes(), in.inReplyTo).isPresent()
+ && !commentsUtil.getRobotComment(rsrc.getNotes(), in.inReplyTo).isPresent()) {
+ throw new BadRequestException(
+ String.format("Invalid inReplyTo, comment %s not found", in.inReplyTo));
}
try (BatchUpdate bu =
diff --git a/java/com/google/gerrit/server/restapi/change/PostReview.java b/java/com/google/gerrit/server/restapi/change/PostReview.java
index 9064dc8..32c1656 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReview.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReview.java
@@ -616,6 +616,7 @@
ensureCommentNotOnMagicFilesOfAutoMerge(path, comment);
ensureRangeIsValid(path, comment.range);
ensureValidPatchsetLevelComment(path, comment);
+ ensureValidInReplyTo(revision.getNotes(), comment.inReplyTo);
}
}
}
@@ -662,6 +663,16 @@
}
}
+ private void ensureValidInReplyTo(ChangeNotes changeNotes, String inReplyTo)
+ throws BadRequestException {
+ if (inReplyTo != null
+ && !commentsUtil.getPublishedHumanComment(changeNotes, inReplyTo).isPresent()
+ && !commentsUtil.getRobotComment(changeNotes, inReplyTo).isPresent()) {
+ throw new BadRequestException(
+ String.format("Invalid inReplyTo, comment %s not found", inReplyTo));
+ }
+ }
+
private void checkRobotComments(
RevisionResource revision, Map<String, List<RobotCommentInput>> in)
throws BadRequestException, PatchListNotAvailableException {
diff --git a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
index f327f16..17a8ff4 100644
--- a/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
+++ b/java/com/google/gerrit/server/restapi/change/PutDraftComment.java
@@ -31,6 +31,7 @@
import com.google.gerrit.server.CommentsUtil;
import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.DraftCommentResource;
+import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.patch.PatchListCache;
import com.google.gerrit.server.patch.PatchListNotAvailableException;
@@ -55,6 +56,7 @@
private final PatchSetUtil psUtil;
private final Provider<CommentJson> commentJson;
private final PatchListCache patchListCache;
+ private final ChangeNotes.Factory changeNotesFactory;
@Inject
PutDraftComment(
@@ -63,13 +65,15 @@
CommentsUtil commentsUtil,
PatchSetUtil psUtil,
Provider<CommentJson> commentJson,
- PatchListCache patchListCache) {
+ PatchListCache patchListCache,
+ ChangeNotes.Factory changeNotesFactory) {
this.updateFactory = updateFactory;
this.delete = delete;
this.commentsUtil = commentsUtil;
this.psUtil = psUtil;
this.commentJson = commentJson;
this.patchListCache = patchListCache;
+ this.changeNotesFactory = changeNotesFactory;
}
@Override
@@ -86,8 +90,12 @@
throw new BadRequestException("patchset-level comments can't have side, range, or line");
} 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");
+ } else if (in.inReplyTo != null
+ && !commentsUtil.getPublishedHumanComment(rsrc.getNotes(), in.inReplyTo).isPresent()
+ && !commentsUtil.getRobotComment(rsrc.getNotes(), in.inReplyTo).isPresent()) {
+ throw new BadRequestException(
+ String.format("Invalid inReplyTo, comment %s not found", in.inReplyTo));
}
-
try (BatchUpdate bu =
updateFactory.create(rsrc.getChange().getProject(), rsrc.getUser(), TimeUtil.nowTs())) {
Op op = new Op(rsrc.getComment().key, in);
diff --git a/java/com/google/gerrit/testing/TestCommentHelper.java b/java/com/google/gerrit/testing/TestCommentHelper.java
index 889bc18..76a5521 100644
--- a/java/com/google/gerrit/testing/TestCommentHelper.java
+++ b/java/com/google/gerrit/testing/TestCommentHelper.java
@@ -17,6 +17,7 @@
import static java.util.stream.Collectors.toList;
import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.extensions.api.GerritApi;
import com.google.gerrit.extensions.api.changes.DraftInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
@@ -148,13 +149,30 @@
addRobotComment(targetChangeId, robotCommentInput, "robot comment test");
}
+ public void addRobotComment(Change.Id targetChangeId, RobotCommentInput robotCommentInput)
+ throws Exception {
+ addRobotComment(targetChangeId, robotCommentInput, "robot comment test");
+ }
+
public void addRobotComment(
String targetChangeId, RobotCommentInput robotCommentInput, String message) throws Exception {
+ ReviewInput reviewInput = createReviewInput(robotCommentInput, message);
+ gApi.changes().id(targetChangeId).current().review(reviewInput);
+ }
+
+ public void addRobotComment(
+ Change.Id targetChangeId, RobotCommentInput robotCommentInput, String message)
+ throws Exception {
+ ReviewInput reviewInput = createReviewInput(robotCommentInput, message);
+ gApi.changes().id(targetChangeId.get()).current().review(reviewInput);
+ }
+
+ private ReviewInput createReviewInput(RobotCommentInput robotCommentInput, String message) {
ReviewInput reviewInput = new ReviewInput();
reviewInput.robotComments =
Collections.singletonMap(robotCommentInput.path, ImmutableList.of(robotCommentInput));
reviewInput.message = message;
reviewInput.tag = ChangeMessagesUtil.AUTOGENERATED_TAG_PREFIX;
- gApi.changes().id(targetChangeId).current().review(reviewInput);
+ return reviewInput;
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
index 27b866b..1ad952c 100644
--- a/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java
@@ -14,9 +14,11 @@
package com.google.gerrit.acceptance.api.revision;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.collect.MoreCollectors.onlyElement;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
+import static com.google.gerrit.entities.Patch.COMMIT_MSG;
import static com.google.gerrit.entities.Patch.PATCHSET_LEVEL;
import static com.google.gerrit.extensions.client.ListChangesOption.MESSAGES;
import static com.google.gerrit.extensions.common.testing.DiffInfoSubject.assertThat;
@@ -32,8 +34,11 @@
import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.acceptance.UseClockStep;
import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.testsuite.change.ChangeOperations;
+import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.extensions.api.changes.PublishChangeEditInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.api.changes.ReviewInput.RobotCommentInput;
import com.google.gerrit.extensions.client.Comment;
import com.google.gerrit.extensions.client.Side;
@@ -67,6 +72,7 @@
public class RobotCommentsIT extends AbstractDaemonTest {
@Inject private TestCommentHelper testCommentHelper;
+ @Inject private ChangeOperations changeOperations;
private static final String PLAIN_TEXT_CONTENT_TYPE = "text/plain";
private static final String GERRIT_COMMIT_MESSAGE_TYPE = "text/x-gerrit-commit-message";
@@ -320,6 +326,58 @@
}
@Test
+ public void robotCommentInvalidInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ RobotCommentInput input = TestCommentHelper.createRobotCommentInput(PATCHSET_LEVEL);
+ input.inReplyTo = "invalid";
+ BadRequestException ex =
+ assertThrows(
+ BadRequestException.class, () -> testCommentHelper.addRobotComment(changeId, input));
+ assertThat(ex.getMessage()).contains("inReplyTo");
+ }
+
+ @Test
+ public void canCreateRobotCommentWithRobotCommentAsParent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ ReviewInput.RobotCommentInput robotCommentInput =
+ TestCommentHelper.createRobotCommentInputWithMandatoryFields(COMMIT_MSG);
+ robotCommentInput.message = "comment reply";
+ robotCommentInput.inReplyTo = parentRobotCommentUuid;
+ testCommentHelper.addRobotComment(changeId, robotCommentInput);
+
+ RobotCommentInfo resultComment =
+ Iterables.getOnlyElement(
+ gApi.changes().id(changeId.get()).current().robotCommentsAsList().stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
+ assertThat(resultComment.inReplyTo).isEqualTo(parentRobotCommentUuid);
+ }
+
+ @Test
+ public void canCreateRobotCommentWithHumanCommentAsParent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String changeIdString = changeOperations.change(changeId).get().changeId();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().create();
+
+ ReviewInput.RobotCommentInput robotCommentInput =
+ TestCommentHelper.createRobotCommentInputWithMandatoryFields(COMMIT_MSG);
+ robotCommentInput.message = "comment reply";
+ robotCommentInput.inReplyTo = parentCommentUuid;
+ testCommentHelper.addRobotComment(changeIdString, robotCommentInput);
+
+ RobotCommentInfo resultComment =
+ Iterables.getOnlyElement(
+ gApi.changes().id(changeIdString).current().robotCommentsAsList().stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
+ assertThat(resultComment.inReplyTo).isEqualTo(parentCommentUuid);
+ }
+
+ @Test
public void hugeRobotCommentIsRejected() {
int defaultSizeLimit = 1 << 20;
fixReplacementInfo.replacement = getStringFor(defaultSizeLimit + 1);
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
index 88705e1..0cdff4e 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -14,10 +14,12 @@
package com.google.gerrit.acceptance.server.change;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth8.assertThat;
import static com.google.gerrit.acceptance.PushOneCommit.FILE_NAME;
import static com.google.gerrit.acceptance.PushOneCommit.SUBJECT;
+import static com.google.gerrit.entities.Patch.COMMIT_MSG;
import static com.google.gerrit.entities.Patch.PATCHSET_LEVEL;
import static com.google.gerrit.testing.GerritJUnit.assertThrows;
import static com.google.gerrit.truth.MapSubject.assertThatMap;
@@ -51,7 +53,6 @@
import com.google.gerrit.extensions.client.Side;
import com.google.gerrit.extensions.common.ChangeInfo;
import com.google.gerrit.extensions.common.CommentInfo;
-import com.google.gerrit.extensions.common.RobotCommentInfo;
import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
@@ -658,6 +659,34 @@
}
@Test
+ public void putDraft_humanInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().create();
+
+ DraftInput draft = newDraft(COMMIT_MSG, Side.REVISION, 0, "foo");
+ draft.inReplyTo = parentCommentUuid;
+ String createdDraftUuid = addDraft(changeId, draft).id;
+ TestHumanComment actual =
+ changeOperations.change(changeId).draftComment(createdDraftUuid).get();
+ assertThat(actual.parentUuid()).hasValue(parentCommentUuid);
+ }
+
+ @Test
+ public void putDraft_robotInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ DraftInput draft = newDraft(COMMIT_MSG, Side.REVISION, 0, "foo");
+ draft.inReplyTo = parentRobotCommentUuid;
+ String createdDraftUuid = addDraft(changeId, draft).id;
+ TestHumanComment actual =
+ changeOperations.change(changeId).draftComment(createdDraftUuid).get();
+ assertThat(actual.parentUuid()).hasValue(parentRobotCommentUuid);
+ }
+
+ @Test
public void putDraft_idMismatch() throws Exception {
String file = "file";
PushOneCommit.Result r = createChange();
@@ -702,6 +731,16 @@
}
@Test
+ public void putDraft_invalidInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ DraftInput draft = newDraft(COMMIT_MSG, Side.REVISION, 0, "foo");
+ draft.inReplyTo = "invalid";
+ BadRequestException exception =
+ assertThrows(BadRequestException.class, () -> addDraft(changeId, draft));
+ assertThat(exception.getMessage()).contains(String.format("%s not found", draft.inReplyTo));
+ }
+
+ @Test
public void putDraft_updatePath() throws Exception {
PushOneCommit.Result r = createChange();
String changeId = r.getChangeId();
@@ -715,22 +754,62 @@
}
@Test
- public void putDraft_updateInReplyToAndTag() throws Exception {
- PushOneCommit.Result r = createChange();
- String changeId = r.getChangeId();
- String revId = r.getCommit().getName();
- DraftInput draftInput1 = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
- CommentInfo commentInfo = addDraft(changeId, revId, draftInput1);
- DraftInput draftInput2 = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
- String inReplyTo = "in_reply_to";
+ public void putDraft_updateInvalidInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ DraftInput originalDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
+ CommentInfo originalDraft = addDraft(changeId, originalDraftInput);
+
+ DraftInput updatedDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
+ updatedDraftInput.inReplyTo = "invalid";
+ BadRequestException exception =
+ assertThrows(
+ BadRequestException.class,
+ () -> updateDraft(changeId, updatedDraftInput, originalDraft.id));
+ assertThat(exception.getMessage()).contains(String.format("Invalid inReplyTo"));
+ }
+
+ @Test
+ public void putDraft_updateHumanInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().create();
+ DraftInput originalDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
+ CommentInfo originalDraft = addDraft(changeId, originalDraftInput);
+
+ DraftInput updateDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
+ updateDraftInput.inReplyTo = parentCommentUuid;
+ updateDraft(changeId, updateDraftInput, originalDraft.id);
+ assertThat(changeOperations.change(changeId).draftComment(originalDraft.id).get().parentUuid())
+ .hasValue(parentCommentUuid);
+ }
+
+ @Test
+ public void putDraft_updateRobotInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+ DraftInput originalDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
+ CommentInfo originalDraft = addDraft(changeId, originalDraftInput);
+
+ DraftInput updateDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
+ updateDraftInput.inReplyTo = parentRobotCommentUuid;
+ updateDraft(changeId, updateDraftInput, originalDraft.id);
+ assertThat(changeOperations.change(changeId).draftComment(originalDraft.id).get().parentUuid())
+ .hasValue(parentRobotCommentUuid);
+ }
+
+ @Test
+ public void putDraft_updateTag() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ DraftInput originalDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "foo");
+ CommentInfo originalDraft = addDraft(changeId, originalDraftInput);
+
+ DraftInput updateDraftInput = newDraft(FILE_NAME, Side.REVISION, 0, "bar");
String tag = "täg";
- draftInput2.inReplyTo = inReplyTo;
- draftInput2.tag = tag;
- updateDraft(changeId, revId, draftInput2, commentInfo.id);
- com.google.gerrit.entities.Comment comment =
- Iterables.getOnlyElement(commentsUtil.draftByChange(r.getChange().notes()));
- assertThat(comment.parentUuid).isEqualTo(inReplyTo);
- assertThat(comment.tag).isEqualTo(tag);
+ updateDraftInput.tag = tag;
+ updateDraft(changeId, updateDraftInput, originalDraft.id);
+ assertThat(changeOperations.change(changeId).draftComment(originalDraft.id).get().tag())
+ .hasValue(tag);
}
@Test
@@ -1474,26 +1553,73 @@
@Test
public void canCreateHumanCommentWithRobotCommentAsParentAndUnsetUnresolved() throws Exception {
- PushOneCommit.Result result = createChange();
- String changeId = result.getChangeId();
- String ps1 = result.getCommit().name();
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
- testCommentHelper.addRobotComment(
- result.getChangeId(),
- TestCommentHelper.createRobotCommentInputWithMandatoryFields(FILE_NAME));
- RobotCommentInfo robotCommentInfo =
- Iterables.getOnlyElement(gApi.changes().id(changeId).current().robotCommentsAsList());
+ CommentInput createdCommentInput = newComment(COMMIT_MSG, "comment reply");
+ createdCommentInput.inReplyTo = parentRobotCommentUuid;
+ createdCommentInput.unresolved = null;
+ addComments(changeId, createdCommentInput);
- CommentInput comment = newComment(FILE_NAME, "comment 1 reply");
- comment.inReplyTo = robotCommentInfo.id;
- comment.unresolved = null;
- addComments(changeId, ps1, comment);
+ CommentInfo resultNewComment =
+ Iterables.getOnlyElement(
+ getPublishedCommentsAsList(changeId).stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
- CommentInfo resultComment = Iterables.getOnlyElement(getPublishedCommentsAsList(changeId));
- assertThat(resultComment.inReplyTo).isEqualTo(robotCommentInfo.id);
+ assertThat(resultNewComment.inReplyTo).isEqualTo(parentRobotCommentUuid);
// Default unresolved is false.
- assertThat(resultComment.unresolved).isFalse();
+ assertThat(resultNewComment.unresolved).isFalse();
+ }
+
+ @Test
+ public void canCreateHumanCommentWithHumanCommentAsParent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().create();
+
+ CommentInput createdCommentInput = newComment(COMMIT_MSG, "comment reply");
+ createdCommentInput.inReplyTo = parentCommentUuid;
+ addComments(changeId, createdCommentInput);
+
+ CommentInfo resultNewComment =
+ Iterables.getOnlyElement(
+ getPublishedCommentsAsList(changeId).stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
+ assertThat(resultNewComment.inReplyTo).isEqualTo(parentCommentUuid);
+ }
+
+ @Test
+ public void canCreateHumanCommentWithRobotCommentAsParent() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentRobotCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ CommentInput createdCommentInput = newComment(COMMIT_MSG, "comment reply");
+ createdCommentInput.inReplyTo = parentRobotCommentUuid;
+ addComments(changeId, createdCommentInput);
+
+ CommentInfo resultNewComment =
+ Iterables.getOnlyElement(
+ getPublishedCommentsAsList(changeId).stream()
+ .filter(c -> c.message.equals("comment reply"))
+ .collect(toImmutableSet()));
+ assertThat(resultNewComment.inReplyTo).isEqualTo(parentRobotCommentUuid);
+ }
+
+ @Test
+ public void cannotCreateCommentWithInvalidInReplyTo() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ CommentInput comment = newComment(COMMIT_MSG, "comment 1 reply");
+ comment.inReplyTo = "invalid";
+
+ BadRequestException exception =
+ assertThrows(BadRequestException.class, () -> addComments(changeId, comment));
+ assertThat(exception.getMessage()).contains(String.format("%s not found", comment.inReplyTo));
}
private List<CommentInfo> getRevisionComments(String changeId, String revId) throws Exception {
@@ -1510,6 +1636,12 @@
return comment;
}
+ private void addComments(Change.Id changeId, CommentInput... commentInputs) throws Exception {
+ ReviewInput input = new ReviewInput();
+ input.comments = Arrays.stream(commentInputs).collect(groupingBy(c -> c.path));
+ gApi.changes().id(changeId.get()).current().review(input);
+ }
+
private void addComments(String changeId, String revision, CommentInput... commentInputs)
throws Exception {
ReviewInput input = new ReviewInput();
@@ -1517,6 +1649,12 @@
gApi.changes().id(changeId).revision(revision).review(input);
}
+ private void addComments(String changeId, CommentInput... commentInputs) throws Exception {
+ ReviewInput input = new ReviewInput();
+ input.comments = Arrays.stream(commentInputs).collect(groupingBy(c -> c.path));
+ gApi.changes().id(changeId).current().review(input);
+ }
+
/**
* All the commits, which contain the target comment before, should still contain the comment with
* the updated message. All the other metas of the commits should be exactly the same.
@@ -1610,11 +1748,31 @@
return gApi.changes().id(changeId).revision(revId).createDraft(in).get();
}
+ private CommentInfo addDraft(Change.Id changeId, String revId, DraftInput in) throws Exception {
+ return gApi.changes().id(changeId.get()).revision(revId).createDraft(in).get();
+ }
+
+ private CommentInfo addDraft(String changeId, DraftInput in) throws Exception {
+ return gApi.changes().id(changeId).current().createDraft(in).get();
+ }
+
+ private CommentInfo addDraft(Change.Id changeId, DraftInput in) throws Exception {
+ return gApi.changes().id(changeId.get()).current().createDraft(in).get();
+ }
+
private void updateDraft(String changeId, String revId, DraftInput in, String uuid)
throws Exception {
gApi.changes().id(changeId).revision(revId).draft(uuid).update(in);
}
+ private void updateDraft(String changeId, DraftInput in, String uuid) throws Exception {
+ gApi.changes().id(changeId).current().draft(uuid).update(in);
+ }
+
+ private void updateDraft(Change.Id changeId, DraftInput in, String uuid) throws Exception {
+ gApi.changes().id(changeId.get()).current().draft(uuid).update(in);
+ }
+
private void deleteDraft(String changeId, String revId, String uuid) throws Exception {
gApi.changes().id(changeId).revision(revId).draft(uuid).delete();
}
@@ -1633,6 +1791,10 @@
return gApi.changes().id(changeId).commentsAsList();
}
+ private List<CommentInfo> getPublishedCommentsAsList(Change.Id changeId) throws Exception {
+ return gApi.changes().id(changeId.get()).commentsAsList();
+ }
+
private Map<String, List<CommentInfo>> getDraftComments(String changeId, String revId)
throws Exception {
return gApi.changes().id(changeId).revision(revId).drafts();
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
index 975d7ec..0bd6554 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/ChangeOperationsImplTest.java
@@ -1175,6 +1175,39 @@
}
@Test
+ public void tagOfPublishedCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String childCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().tag("tag").create();
+
+ TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
+
+ assertThat(comment.tag()).value().isEqualTo("tag");
+ }
+
+ @Test
+ public void unresolvedOfUnresolvedPublishedCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String childCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().unresolved().create();
+
+ TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
+
+ assertThat(comment.unresolved()).isTrue();
+ }
+
+ @Test
+ public void unresolvedOfResolvedPublishedCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String childCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newComment().resolved().create();
+
+ TestHumanComment comment = changeOperations.change(changeId).comment(childCommentUuid).get();
+
+ assertThat(comment.unresolved()).isFalse();
+ }
+
+ @Test
public void draftCommentCanBeRetrieved() {
Change.Id changeId = changeOperations.newChange().create();
String commentUuid = changeOperations.change(changeId).currentPatchset().newComment().create();
@@ -1212,6 +1245,36 @@
assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
}
+ @Test
+ public void robotCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ TestRobotComment comment = changeOperations.change(changeId).robotComment(commentUuid).get();
+
+ assertThat(comment.uuid()).isEqualTo(commentUuid);
+ }
+
+ @Test
+ public void parentUuidOfRobotCommentCanBeRetrieved() {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+ String childCommentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .parentUuid(parentCommentUuid)
+ .create();
+
+ TestRobotComment comment =
+ changeOperations.change(changeId).robotComment(childCommentUuid).get();
+
+ assertThat(comment.parentUuid()).value().isEqualTo(parentCommentUuid);
+ }
+
private ChangeInfo getChangeFromServer(Change.Id changeId) throws RestApiException {
return gApi.changes().id(changeId.get()).get();
}
diff --git a/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java
index cf687aa..fa28396 100644
--- a/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java
+++ b/javatests/com/google/gerrit/acceptance/testsuite/change/PatchsetOperationsImplTest.java
@@ -16,6 +16,8 @@
import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThat;
import static com.google.gerrit.extensions.common.testing.CommentInfoSubject.assertThatList;
+import static com.google.gerrit.extensions.common.testing.RobotCommentInfoSubject.assertThat;
+import static com.google.gerrit.extensions.common.testing.RobotCommentInfoSubject.assertThatList;
import com.google.common.truth.Correspondence;
import com.google.gerrit.acceptance.AbstractDaemonTest;
@@ -27,6 +29,7 @@
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.common.RobotCommentInfo;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.truth.NullAwareCorrespondence;
import com.google.inject.Inject;
@@ -44,7 +47,6 @@
Change.Id changeId = changeOperations.newChange().create();
String commentUuid = changeOperations.change(changeId).currentPatchset().newComment().create();
-
List<CommentInfo> comments = getCommentsFromServer(changeId);
assertThatList(comments).comparingElementsUsing(hasUuid()).containsExactly(commentUuid);
}
@@ -643,10 +645,393 @@
assertThatList(comments).isEmpty();
}
+ @Test
+ public void robotCommentCanBeCreatedWithoutSpecifyingAnyParameters() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+ List<RobotCommentInfo> robotComments = getRobotCommentsFromServerFromCurrentPatchset(changeId);
+ assertThatList(robotComments).comparingElementsUsing(hasUuid()).containsExactly(commentUuid);
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnOlderPatchset() 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).newRobotComment().create();
+
+ CommentInfo comment = getRobotCommentFromServer(previousPatchsetId, commentUuid);
+ assertThat(comment).uuid().isEqualTo(commentUuid);
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithSpecifiedMessage() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .message("Test comment message")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).message().isEqualTo("Test comment message");
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedWithEmptyMessage() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().noMessage().create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).message().isNull();
+ }
+
+ @Test
+ public void patchsetLevelRobotCommentCanBeCreated() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onPatchsetLevel()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).path().isEqualTo(Patch.PATCHSET_LEVEL);
+ }
+
+ @Test
+ public void fileRobotCommentCanBeCreated() throws Exception {
+ Change.Id changeId = changeOperations.newChange().file("file1").content("Line 1").create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onFileLevelOf("file1")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).path().isEqualTo("file1");
+ assertThat(comment).line().isNull();
+ assertThat(comment).range().isNull();
+ }
+
+ @Test
+ public void lineRobotCommentCanBeCreated() 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()
+ .newRobotComment()
+ .onLine(3)
+ .ofFile("file1")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).line().isEqualTo(3);
+ assertThat(comment).range().isNull();
+ }
+
+ @Test
+ public void rangeRobotCommentCanBeCreated() 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()
+ .newRobotComment()
+ .fromLine(2)
+ .charOffset(4)
+ .toLine(3)
+ .charOffset(5)
+ .ofFile("file1")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(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 robotCommentCanBeCreatedOnPatchsetCommit() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onPatchsetCommit()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(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 robotCommentCanBeCreatedOnParentCommit() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onParentCommit()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).side().isEqualTo(Side.PARENT);
+ assertThat(comment).parent().isEqualTo(1);
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnSecondParentCommit() throws Exception {
+ Change.Id parent1ChangeId = changeOperations.newChange().create();
+ Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onSecondParentCommit()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).side().isEqualTo(Side.PARENT);
+ assertThat(comment).parent().isEqualTo(2);
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnNonExistingSecondParentCommit() throws Exception {
+ Change.Id parentChangeId = changeOperations.newChange().create();
+ Change.Id changeId = changeOperations.newChange().childOf().change(parentChangeId).create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onSecondParentCommit()
+ .create();
+
+ // We want to be able to create such invalid robot comments for testing purposes (e.g. testing
+ // error handling or resilience of an endpoint) and hence we need to allow such invalid robot
+ // comments in the test API.
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).side().isEqualTo(Side.PARENT);
+ assertThat(comment).parent().isEqualTo(2);
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedOnAutoMergeCommit() throws Exception {
+ Change.Id parent1ChangeId = changeOperations.newChange().create();
+ Change.Id parent2ChangeId = changeOperations.newChange().create();
+ Change.Id changeId =
+ changeOperations
+ .newChange()
+ .mergeOf()
+ .change(parent1ChangeId)
+ .and()
+ .change(parent2ChangeId)
+ .create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .onAutoMergeCommit()
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).side().isEqualTo(Side.PARENT);
+ assertThat(comment).parent().isNull();
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedAsResolved() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().resolved().create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).unresolved().isFalse();
+ }
+
+ @Test
+ public void robotCommentCanBeCreatedAsUnresolved() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().unresolved().create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).unresolved().isTrue();
+ }
+
+ @Test
+ public void replyToRobotCommentCanBeCreated() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ String parentCommentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .parentUuid(parentCommentUuid)
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).inReplyTo().isEqualTo(parentCommentUuid);
+ }
+
+ @Test
+ public void tagCanBeAttachedToARobotComment() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .tag("my special tag")
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).tag().isEqualTo("my special tag");
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithSpecifiedAuthor() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+ Account.Id accountId = accountOperations.newAccount().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .author(accountId)
+ .create();
+
+ CommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).author().id().isEqualTo(accountId.get());
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithRobotId() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .robotId("robot-id")
+ .create();
+
+ RobotCommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).robotId().isEqualTo("robot-id");
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithRobotRunId() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .robotId("robot-run-id")
+ .create();
+
+ RobotCommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).robotId().isEqualTo("robot-run-id");
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithUrl() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations.change(changeId).currentPatchset().newRobotComment().url("url").create();
+
+ RobotCommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).url().isEqualTo("url");
+ }
+
+ @Test
+ public void robotCommentIsCreatedWithProperty() throws Exception {
+ Change.Id changeId = changeOperations.newChange().create();
+
+ String commentUuid =
+ changeOperations
+ .change(changeId)
+ .currentPatchset()
+ .newRobotComment()
+ .addProperty("key", "value")
+ .create();
+
+ RobotCommentInfo comment = getRobotCommentFromServerInCurrentPatchset(changeId, commentUuid);
+ assertThat(comment).properties().containsExactly("key", "value");
+ }
+
private List<CommentInfo> getCommentsFromServer(Change.Id changeId) throws RestApiException {
return gApi.changes().id(changeId.get()).commentsAsList();
}
+ private List<RobotCommentInfo> getRobotCommentsFromServerFromCurrentPatchset(Change.Id changeId)
+ throws RestApiException {
+ return gApi.changes().id(changeId.get()).current().robotCommentsAsList();
+ }
+
private List<CommentInfo> getDraftCommentsFromServer(Change.Id changeId) throws RestApiException {
return gApi.changes().id(changeId.get()).draftsAsList();
}
@@ -662,6 +1047,33 @@
String.format("Comment %s not found on change %d", uuid, changeId.get())));
}
+ private RobotCommentInfo getRobotCommentFromServerInCurrentPatchset(
+ Change.Id changeId, String uuid) throws RestApiException {
+ return gApi.changes().id(changeId.get()).current().robotCommentsAsList().stream()
+ .filter(comment -> comment.id.equals(uuid))
+ .findAny()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "Robot Comment %s not found on change %d on the latest patchset",
+ uuid, changeId.get())));
+ }
+
+ private RobotCommentInfo getRobotCommentFromServer(PatchSet.Id patchsetId, String uuid)
+ throws RestApiException {
+ return gApi.changes().id(patchsetId.changeId().toString())
+ .revision(patchsetId.getId().toString()).robotCommentsAsList().stream()
+ .filter(comment -> comment.id.equals(uuid))
+ .findAny()
+ .orElseThrow(
+ () ->
+ new IllegalStateException(
+ String.format(
+ "Robot Comment %s not found on change %d on patchset %d",
+ uuid, patchsetId.changeId().get(), patchsetId.get())));
+ }
+
private CommentInfo getDraftCommentFromServer(Change.Id changeId, String uuid)
throws RestApiException {
return gApi.changes().id(changeId.get()).draftsAsList().stream()
