Merge "Allow linking robot comments to auto-generated change messages."

java/com/google/gerrit/server/CommentsUtil.java

@@ -242,7 +242,9 @@ public class CommentsUtil {
    * @param changeMessages list of change messages
    */
   public static void linkCommentsToChangeMessages(
-      List<? extends CommentInfo> comments, List<ChangeMessage> changeMessages) {
+      List<? extends CommentInfo> comments,
+      List<ChangeMessage> changeMessages,
+      boolean skipAutoGeneratedMessages) {
     ArrayList<ChangeMessage> sortedChangeMessages =
         changeMessages.stream()
             .sorted(comparing(ChangeMessage::getWrittenOn))
@@ -257,7 +259,7 @@ public class CommentsUtil {
       // message in timestamp
       while (cmItr < sortedChangeMessages.size()) {
         ChangeMessage cm = sortedChangeMessages.get(cmItr);
-        if (isAfter(comment, cm) || skipChangeMessage(cm)) {
+        if (isAfter(comment, cm) || (skipAutoGeneratedMessages && isAutoGenerated(cm))) {
           cmItr += 1;
         } else {
           break;
@@ -269,7 +271,7 @@ public class CommentsUtil {
     }
   }
 
-  private static boolean skipChangeMessage(ChangeMessage cm) {
+  private static boolean isAutoGenerated(ChangeMessage cm) {
     return ChangeMessagesUtil.isAutogenerated(cm.getTag());
   }
 

java/com/google/gerrit/server/restapi/change/ListChangeComments.java

@@ -73,7 +73,7 @@ public class ListChangeComments implements RestReadView<ChangeResource> {
       throws PermissionBackendException {
     ImmutableList<CommentInfo> commentInfos = getCommentFormatter().formatAsList(comments);
     List<ChangeMessage> changeMessages = changeMessagesUtil.byChange(rsrc.getNotes());
-    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages);
+    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages, true);
     return commentInfos;
   }
 
@@ -83,7 +83,7 @@ public class ListChangeComments implements RestReadView<ChangeResource> {
     List<CommentInfo> commentInfos =
         commentInfosMap.values().stream().flatMap(List::stream).collect(toList());
     List<ChangeMessage> changeMessages = changeMessagesUtil.byChange(rsrc.getNotes());
-    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages);
+    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages, true);
     return commentInfosMap;
   }
 

java/com/google/gerrit/server/restapi/change/ListChangeRobotComments.java

@@ -63,7 +63,7 @@ public class ListChangeRobotComments implements RestReadView<ChangeResource> {
     List<RobotCommentInfo> commentInfos =
         robotCommentsMap.values().stream().flatMap(List::stream).collect(toList());
     List<ChangeMessage> changeMessages = changeMessagesUtil.byChange(rsrc.getNotes());
-    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages);
+    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages, false);
     return Response.ok(robotCommentsMap);
   }
 }

java/com/google/gerrit/server/restapi/change/ListRobotComments.java

@@ -64,7 +64,7 @@ public class ListRobotComments implements RestReadView<RevisionResource> {
       Iterable<RobotComment> comments, RevisionResource rsrc) throws PermissionBackendException {
     ImmutableList<RobotCommentInfo> commentInfos = getCommentFormatter().formatAsList(comments);
     List<ChangeMessage> changeMessages = changeMessagesUtil.byChange(rsrc.getNotes());
-    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages);
+    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages, false);
     return commentInfos;
   }
 
@@ -74,7 +74,7 @@ public class ListRobotComments implements RestReadView<RevisionResource> {
     List<RobotCommentInfo> commentInfos =
         commentInfosMap.values().stream().flatMap(List::stream).collect(toList());
     List<ChangeMessage> changeMessages = changeMessagesUtil.byChange(rsrc.getNotes());
-    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages);
+    CommentsUtil.linkCommentsToChangeMessages(commentInfos, changeMessages, false);
     return commentInfosMap;
   }
 

java/com/google/gerrit/testing/TestCommentHelper.java

@@ -26,6 +26,7 @@ import com.google.gerrit.extensions.client.Comment.Range;
 import com.google.gerrit.extensions.client.Side;
 import com.google.gerrit.extensions.common.CommentInfo;
 import com.google.gerrit.extensions.common.FixSuggestionInfo;
+import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.inject.Inject;
 import java.util.Arrays;
 import java.util.Collection;
@@ -144,6 +145,7 @@ public class TestCommentHelper {
     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);
   }
 }

javatests/com/google/gerrit/acceptance/api/revision/RobotCommentsIT.java

@@ -151,7 +151,7 @@ public class RobotCommentsIT extends AbstractDaemonTest {
     TestTimeUtil.resetWithClockStep(0, TimeUnit.SECONDS);
     createChange();
     /* Advancing the time after creating the change so that the first robot comment is not in the same timestamp as with the change creation */
-    TestTimeUtil.incrementClock(5, TimeUnit.SECONDS);
+    TestTimeUtil.incrementClock(10, TimeUnit.SECONDS);
 
     RobotCommentInput c1 = TestCommentHelper.createRobotCommentInput(FILE_NAME);
     RobotCommentInput c2 = TestCommentHelper.createRobotCommentInput(FILE_NAME);
@@ -220,9 +220,9 @@ public class RobotCommentsIT extends AbstractDaemonTest {
             .changeMessageId;
 
     /**
-     * Upload PS message, robot message 1 & robot comment 1 all have the same timestamp. The robot
+     * All change messages have the auto-generated tag. Robot comments can be linked to
-     * comment is matched to robot message 1 because the PS upload message is auto-generated and is
+     * auto-generated messages where each comment is linked to the next nearest change message in
-     * ignored in matching
+     * timestamp
      */
     assertThat(message1ChangeId).isEqualTo(comment1MessageId);
     assertThat(message2ChangeId).isEqualTo(comment2MessageId);

javatests/com/google/gerrit/server/restapi/change/ListChangeCommentsTest.java

@@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.ChangeMessage;
 import com.google.gerrit.extensions.common.CommentInfo;
+import com.google.gerrit.server.ChangeMessagesUtil;
 import com.google.gerrit.server.CommentsUtil;
 import java.sql.Timestamp;
 import org.junit.Test;
@@ -33,27 +34,38 @@ public class ListChangeCommentsTest {
 
   @Test
   public void commentsLinkedToChangeMessages() {
+    /**
+     * Human comments should not be linked to auto-generated messages. A comment is linked to the
+     * nearest next change message in timestamp
+     */
     CommentInfo c1 = getNewCommentInfo("c1", Timestamp.valueOf("2018-01-01 09:01:00"));
     CommentInfo c2 = getNewCommentInfo("c2", Timestamp.valueOf("2018-01-01 09:01:15"));
     CommentInfo c3 = getNewCommentInfo("c3", Timestamp.valueOf("2018-01-01 09:01:25"));
 
     ChangeMessage cm1 =
-        getNewChangeMessage("cm1key", "cm1", Timestamp.valueOf("2018-01-01 00:00:00"));
+        getNewChangeMessage("cm1key", "cm1", Timestamp.valueOf("2018-01-01 00:00:00"), null);
+    ChangeMessage cmIgnore =
+        getNewChangeMessage(
+            "cm2key",
+            "cm2",
+            Timestamp.valueOf("2018-01-01 09:01:15"),
+            ChangeMessagesUtil.AUTOGENERATED_TAG_PREFIX);
     ChangeMessage cm2 =
-        getNewChangeMessage("cm2key", "cm2", Timestamp.valueOf("2018-01-01 09:01:15"));
+        getNewChangeMessage("cm2key", "cm2", Timestamp.valueOf("2018-01-01 09:01:16"), null);
     ChangeMessage cm3 =
-        getNewChangeMessage("cm3key", "cm3", Timestamp.valueOf("2018-01-01 09:01:27"));
+        getNewChangeMessage("cm3key", "cm3", Timestamp.valueOf("2018-01-01 09:01:27"), null);
 
     assertThat(c1.changeMessageId).isNull();
     assertThat(c2.changeMessageId).isNull();
     assertThat(c3.changeMessageId).isNull();
 
     ImmutableList<CommentInfo> comments = ImmutableList.of(c1, c2, c3);
-    ImmutableList<ChangeMessage> changeMessages = ImmutableList.of(cm1, cm2, cm3);
+    ImmutableList<ChangeMessage> changeMessages = ImmutableList.of(cm1, cmIgnore, cm2, cm3);
 
-    CommentsUtil.linkCommentsToChangeMessages(comments, changeMessages);
+    CommentsUtil.linkCommentsToChangeMessages(comments, changeMessages, true);
 
     assertThat(c1.changeMessageId).isEqualTo(changeMessageKey(cm2));
+    /** comment 2 ignored the auto-generated change message */
     assertThat(c2.changeMessageId).isEqualTo(changeMessageKey(cm2));
     assertThat(c3.changeMessageId).isEqualTo(changeMessageKey(cm3));
   }
@@ -65,10 +77,12 @@ public class ListChangeCommentsTest {
     return c;
   }
 
-  private static ChangeMessage getNewChangeMessage(String id, String message, Timestamp ts) {
+  private static ChangeMessage getNewChangeMessage(
+      String id, String message, Timestamp ts, String tag) {
     ChangeMessage.Key key = ChangeMessage.key(Change.id(1), id);
     ChangeMessage cm = new ChangeMessage(key, null, ts, null);
     cm.setMessage(message);
+    cm.setTag(tag);
     return cm;
   }