Add comment creation to test API of changes

Adding a new comment is as simple as:
- changeOperations.change(changeId).patchset(patchsetId)
    .newComment().create()
- changeOperations.change(changeId).currentPatchset()
    .newComment().message("Some text").create()
- changeOperations.change(changeId).currentPatchset().newComment()
     .fromLine(2).charOffset(4).toLine(3).charOffset(5).ofFile("foo")
     .create()

Change-Id: I94e9dd3090aabf12678d0c3dd094e8593b4443c6

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.
+   *
+   * <p>Example:
+   *
+   * <pre>
+   * String createdCommentUuid = changeOperations
+   *     .change(changeId)
+   *     .currentPatchset()
+   *     .onLine(2)
+   *     .ofFile("file1")
+   *     .create();
+   * </pre>
+   *
+   * @return builder to create a new comment
+   */
+  TestCommentCreation.Builder newComment();
 }

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;
+    }
+  }
 }

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<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();
+
+  abstract ThrowingFunction<TestCommentCreation, String> commentCreator();
+
+  public static TestCommentCreation.Builder builder(
+      ThrowingFunction<TestCommentCreation, String> 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<StartAwarePositionBuilder> 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.
+     *
+     * <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.
+     */
+    public abstract Builder parentUuid(String parentUuid);
+
+    abstract TestCommentCreation.Builder commentCreator(
+        ThrowingFunction<TestCommentCreation, String> 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<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;
+    }
+  }
+}

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();
+    }
+  }
+}