diff --git a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java index ff9fb3c559..9064b97a5a 100644 --- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java +++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java @@ -106,6 +106,8 @@ public interface RevisionApi { List robotCommentsAsList() throws RestApiException; + Map> portedComments() throws RestApiException; + /** * Applies the indicated fix by creating a new change edit or integrating the fix with the * existing change edit. If no change edit exists before this call, the fix must refer to the @@ -293,6 +295,11 @@ public interface RevisionApi { throw new NotImplementedException(); } + @Override + public Map> portedComments() throws RestApiException { + throw new NotImplementedException(); + } + @Override public EditInfo applyFix(String fixId) throws RestApiException { throw new NotImplementedException(); diff --git a/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java b/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java index 923cfc31f7..c34e4398f1 100644 --- a/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java +++ b/java/com/google/gerrit/extensions/common/testing/CommentInfoSubject.java @@ -27,6 +27,7 @@ import com.google.common.truth.Subject; import com.google.gerrit.extensions.client.Side; import com.google.gerrit.extensions.common.CommentInfo; import com.google.gerrit.truth.ListSubject; +import java.sql.Timestamp; import java.util.List; public class CommentInfoSubject extends Subject { @@ -91,6 +92,10 @@ public class CommentInfoSubject extends Subject { return check("inReplyTo").that(commentInfo().inReplyTo); } + public StringSubject commitId() { + return check("commitId").that(commentInfo().commitId); + } + public AccountInfoSubject author() { return check("author").about(accounts()).that(commentInfo().author); } @@ -99,6 +104,14 @@ public class CommentInfoSubject extends Subject { return check("tag").that(commentInfo().tag); } + public ComparableSubject updated() { + return check("updated").that(commentInfo().updated); + } + + public StringSubject changeMessageId() { + return check("changeMessageId").that(commentInfo().changeMessageId); + } + private CommentInfo commentInfo() { isNotNull(); return commentInfo; diff --git a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java index b515dfe1bc..df01d8a92b 100644 --- a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java +++ b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java @@ -79,6 +79,7 @@ import com.google.gerrit.server.restapi.change.GetMergeList; import com.google.gerrit.server.restapi.change.GetPatch; import com.google.gerrit.server.restapi.change.GetRelated; import com.google.gerrit.server.restapi.change.GetRevisionActions; +import com.google.gerrit.server.restapi.change.ListPortedComments; import com.google.gerrit.server.restapi.change.ListRevisionComments; import com.google.gerrit.server.restapi.change.ListRevisionDrafts; import com.google.gerrit.server.restapi.change.ListRobotComments; @@ -130,6 +131,7 @@ class RevisionApiImpl implements RevisionApi { private final FileApiImpl.Factory fileApi; private final ListRevisionComments listComments; private final ListRobotComments listRobotComments; + private final ListPortedComments listPortedComments; private final ApplyFix applyFix; private final GetFixPreview getFixPreview; private final Fixes fixes; @@ -175,6 +177,7 @@ class RevisionApiImpl implements RevisionApi { FileApiImpl.Factory fileApi, ListRevisionComments listComments, ListRobotComments listRobotComments, + ListPortedComments listPortedComments, ApplyFix applyFix, GetFixPreview getFixPreview, Fixes fixes, @@ -219,6 +222,7 @@ class RevisionApiImpl implements RevisionApi { this.listComments = listComments; this.robotComments = robotComments; this.listRobotComments = listRobotComments; + this.listPortedComments = listPortedComments; this.applyFix = applyFix; this.getFixPreview = getFixPreview; this.fixes = fixes; @@ -453,6 +457,15 @@ class RevisionApiImpl implements RevisionApi { } } + @Override + public Map> portedComments() throws RestApiException { + try { + return listPortedComments.apply(revision).value(); + } catch (Exception e) { + throw asRestApiException("Cannot retrieve ported comments", e); + } + } + @Override public EditInfo applyFix(String fixId) throws RestApiException { try { diff --git a/java/com/google/gerrit/server/restapi/change/ListPortedComments.java b/java/com/google/gerrit/server/restapi/change/ListPortedComments.java new file mode 100644 index 0000000000..3f9e127e84 --- /dev/null +++ b/java/com/google/gerrit/server/restapi/change/ListPortedComments.java @@ -0,0 +1,189 @@ +// 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.server.restapi.change; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static java.util.stream.Collectors.groupingBy; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.gerrit.entities.Comment; +import com.google.gerrit.entities.Comment.Range; +import com.google.gerrit.entities.HumanComment; +import com.google.gerrit.entities.PatchSet; +import com.google.gerrit.entities.Project; +import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace; +import com.google.gerrit.extensions.common.CommentInfo; +import com.google.gerrit.extensions.restapi.Response; +import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.server.CommentsUtil; +import com.google.gerrit.server.change.RevisionResource; +import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.patch.DiffMappings; +import com.google.gerrit.server.patch.GitPositionTransformer; +import com.google.gerrit.server.patch.GitPositionTransformer.Mapping; +import com.google.gerrit.server.patch.GitPositionTransformer.Position; +import com.google.gerrit.server.patch.GitPositionTransformer.PositionedEntity; +import com.google.gerrit.server.patch.PatchList; +import com.google.gerrit.server.patch.PatchListCache; +import com.google.gerrit.server.patch.PatchListKey; +import com.google.gerrit.server.patch.PatchListNotAvailableException; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.inject.Inject; +import com.google.inject.Provider; +import com.google.inject.Singleton; +import java.util.List; +import java.util.Map; + +@Singleton +public class ListPortedComments implements RestReadView { + + private final CommentsUtil commentsUtil; + private final Provider commentJson; + private final PatchListCache patchListCache; + + @Inject + public ListPortedComments( + Provider commentJson, CommentsUtil commentsUtil, PatchListCache patchListCache) { + this.commentJson = commentJson; + this.commentsUtil = commentsUtil; + this.patchListCache = patchListCache; + } + + @Override + public Response>> apply(RevisionResource revisionResource) + throws PermissionBackendException, PatchListNotAvailableException { + PatchSet targetPatchset = revisionResource.getPatchSet(); + + List allComments = loadAllPublishedComments(revisionResource); + ImmutableList relevantComments = filterToRelevant(allComments, targetPatchset); + ImmutableList portedComments = + portToTargetPatchset( + revisionResource.getChangeResource().getNotes(), targetPatchset, relevantComments); + return Response.ok(format(portedComments)); + } + + private List loadAllPublishedComments(RevisionResource revisionResource) { + return commentsUtil.publishedHumanCommentsByChange(revisionResource.getNotes()); + } + + private ImmutableList filterToRelevant( + List allComments, PatchSet targetPatchset) { + return allComments.stream() + .filter(comment -> comment.key.patchSetId < targetPatchset.number()) + // TODO(aliceks): Also support comments on parent revisions. + .filter(comment -> comment.side > 0) + .collect(toImmutableList()); + } + + private ImmutableList portToTargetPatchset( + ChangeNotes notes, PatchSet targetPatchset, List comments) + throws PatchListNotAvailableException { + Map> commentsPerPatchset = + comments.stream().collect(groupingBy(comment -> comment.key.patchSetId, toImmutableList())); + + ImmutableList.Builder portedComments = + ImmutableList.builderWithExpectedSize(comments.size()); + for (Integer originalPatchsetId : commentsPerPatchset.keySet()) { + ImmutableList patchsetComments = commentsPerPatchset.get(originalPatchsetId); + PatchSet originalPatchset = + notes.getPatchSets().get(PatchSet.id(notes.getChangeId(), originalPatchsetId)); + portedComments.addAll( + portToTargetPatchset( + notes.getProjectName(), originalPatchset, targetPatchset, patchsetComments)); + } + return portedComments.build(); + } + + private ImmutableList portToTargetPatchset( + Project.NameKey project, + PatchSet originalPatchset, + PatchSet targetPatchset, + ImmutableList comments) + throws PatchListNotAvailableException { + ImmutableSet mappings = + loadPatchsetMappings(project, originalPatchset, targetPatchset); + ImmutableList> positionedComments = + comments.stream().map(this::toPositionedEntity).collect(toImmutableList()); + return GitPositionTransformer.transform(positionedComments, mappings).stream() + .map(PositionedEntity::getEntityAtUpdatedPosition) + .collect(toImmutableList()); + } + + private ImmutableSet loadPatchsetMappings( + Project.NameKey project, PatchSet originalPatchset, PatchSet targetPatchset) + throws PatchListNotAvailableException { + PatchList patchList = + patchListCache.get( + PatchListKey.againstCommit( + originalPatchset.commitId(), targetPatchset.commitId(), Whitespace.IGNORE_NONE), + project); + return patchList.getPatches().stream().map(DiffMappings::toMapping).collect(toImmutableSet()); + } + + private PositionedEntity toPositionedEntity(HumanComment comment) { + return PositionedEntity.create( + comment, + ListPortedComments::extractPosition, + ListPortedComments::createCommentAtNewPosition); + } + + private static Position extractPosition(HumanComment comment) { + Position.Builder positionBuilder = Position.builder().filePath(comment.key.filename); + return positionBuilder.lineRange(extractLineRange(comment)).build(); + } + + private static GitPositionTransformer.Range extractLineRange(HumanComment comment) { + // Line specifications in comment are 1-based. Line specifications in Position are 0-based. + if (comment.range != null) { + // The combination of (line, charOffset) is exclusive and must be mapped to an exclusive line. + int exclusiveEndLine = + comment.range.endChar > 0 ? comment.range.endLine : comment.range.endLine - 1; + return GitPositionTransformer.Range.create(comment.range.startLine - 1, exclusiveEndLine); + } + return GitPositionTransformer.Range.create(comment.lineNbr - 1, comment.lineNbr); + } + + private static HumanComment createCommentAtNewPosition( + HumanComment originalComment, Position newPosition) { + HumanComment portedComment = new HumanComment(originalComment); + portedComment.key.filename = newPosition.filePath(); + portedComment.lineNbr = newPosition.lineRange().start() + 1; + if (portedComment.range != null) { + portedComment.range = + toRange( + newPosition.lineRange(), portedComment.range.startChar, portedComment.range.endChar); + portedComment.lineNbr = portedComment.range.endLine; + } + return portedComment; + } + + private static Comment.Range toRange( + GitPositionTransformer.Range lineRange, int originalStartChar, int originalEndChar) { + int adjustedEndLine = originalEndChar > 0 ? lineRange.end() : lineRange.end() + 1; + return new Range(lineRange.start() + 1, originalStartChar, adjustedEndLine, originalEndChar); + } + + private Map> format(List comments) + throws PermissionBackendException { + return commentJson + .get() + .setFillAccounts(true) + .setFillPatchSet(true) + .newHumanCommentFormatter() + .format(comments); + } +} diff --git a/java/com/google/gerrit/server/restapi/change/Module.java b/java/com/google/gerrit/server/restapi/change/Module.java index 466ea3c3bf..1a4f270057 100644 --- a/java/com/google/gerrit/server/restapi/change/Module.java +++ b/java/com/google/gerrit/server/restapi/change/Module.java @@ -172,6 +172,8 @@ public class Module extends RestApiModule { post(FIX_KIND, "apply").to(ApplyFix.class); get(FIX_KIND, "preview").to(GetFixPreview.class); + get(REVISION_KIND, "ported_comments").to(ListPortedComments.class); + child(REVISION_KIND, "files").to(Files.class); put(FILE_KIND, "reviewed").to(PutReviewed.class); delete(FILE_KIND, "reviewed").to(DeleteReviewed.class); diff --git a/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java b/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java new file mode 100644 index 0000000000..7e844b6d9d --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/api/revision/PortedCommentsIT.java @@ -0,0 +1,1547 @@ +// 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.api.revision; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.Iterables.getOnlyElement; +import static com.google.common.collect.MoreCollectors.onlyElement; +import static com.google.common.truth.Truth.assertThat; +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.truth.MapSubject.assertThatMap; + +import com.google.common.collect.ImmutableList; +import com.google.common.truth.Correspondence; +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.testsuite.account.AccountOperations; +import com.google.gerrit.acceptance.testsuite.change.ChangeOperations; +import com.google.gerrit.acceptance.testsuite.change.TestPatchset; +import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations; +import com.google.gerrit.entities.Account; +import com.google.gerrit.entities.Change; +import com.google.gerrit.entities.Patch; +import com.google.gerrit.entities.PatchSet; +import com.google.gerrit.extensions.common.CommentInfo; +import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.truth.NullAwareCorrespondence; +import com.google.inject.Inject; +import java.util.Collection; +import java.util.List; +import java.util.Map; +import org.junit.Ignore; +import org.junit.Test; + +public class PortedCommentsIT extends AbstractDaemonTest { + + @Inject private ChangeOperations changeOps; + @Inject private AccountOperations accountOps; + @Inject private RequestScopeOperations requestScopeOps; + + @Test + public void onlyCommentsBeforeTargetPatchsetArePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + PatchSet.Id patchset3Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + String comment1Uuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create(); + changeOps.change(changeId).patchset(patchset2Id).newComment().create(); + changeOps.change(changeId).patchset(patchset3Id).newComment().create(); + + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThatList(portedComments).comparingElementsUsing(hasUuid()).containsExactly(comment1Uuid); + } + + @Test + public void commentsOnAnyPatchsetBeforeTargetPatchsetArePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + changeOps.change(changeId).newPatchset().create(); + PatchSet.Id patchset3Id = changeOps.change(changeId).newPatchset().create(); + PatchSet.Id patchset4Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + String comment1Uuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create(); + String comment3Uuid = changeOps.change(changeId).patchset(patchset3Id).newComment().create(); + + List portedComments = flatten(getPortedComments(patchset4Id)); + + assertThat(portedComments) + .comparingElementsUsing(hasUuid()) + .containsExactly(comment1Uuid, comment3Uuid); + } + + @Test + public void severalCommentsFromEarlierPatchsetArePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + String comment1Uuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create(); + String comment2Uuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create(); + + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThat(portedComments) + .comparingElementsUsing(hasUuid()) + .containsExactly(comment1Uuid, comment2Uuid); + } + + @Test + public void completeCommentThreadIsPorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + String rootCommentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create(); + String child1CommentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .parentUuid(rootCommentUuid) + .create(); + String child2CommentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .parentUuid(child1CommentUuid) + .create(); + + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThat(portedComments) + .comparingElementsUsing(hasUuid()) + .containsExactly(rootCommentUuid, child1CommentUuid, child2CommentUuid); + } + + @Test + // TODO(aliceks): Filter out unresolved comment threads. + @Ignore + public void onlyUnresolvedCommentsArePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + changeOps.change(changeId).patchset(patchset1Id).newComment().resolved().create(); + String comment2Uuid = + changeOps.change(changeId).patchset(patchset1Id).newComment().unresolved().create(); + + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThat(portedComments).comparingElementsUsing(hasUuid()).containsExactly(comment2Uuid); + } + + @Test + // TODO(aliceks): Filter out unresolved comment threads. + @Ignore + public void unresolvedStateOfLastCommentInThreadMatters() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + String rootComment1Uuid = + changeOps.change(changeId).patchset(patchset1Id).newComment().resolved().create(); + String childComment1Uuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .parentUuid(rootComment1Uuid) + .unresolved() + .create(); + String rootComment2Uuid = + changeOps.change(changeId).patchset(patchset1Id).newComment().unresolved().create(); + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .parentUuid(rootComment2Uuid) + .resolved() + .create(); + + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThat(portedComments) + .comparingElementsUsing(hasUuid()) + .containsExactly(rootComment1Uuid, childComment1Uuid); + } + + @Test + public void unresolvedStateOfLastCommentByDateMattersForBranchedThreads() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + String rootCommentUuid = + changeOps.change(changeId).patchset(patchset1Id).newComment().resolved().create(); + String childComment1Uuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .parentUuid(rootCommentUuid) + .resolved() + .create(); + String childComment2Uuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .parentUuid(rootCommentUuid) + .unresolved() + .create(); + + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThat(portedComments) + .comparingElementsUsing(hasUuid()) + .containsExactly(rootCommentUuid, childComment1Uuid, childComment2Uuid); + } + + @Test + public void draftCommentsAreNotPortedViaApiForPublishedComments() throws Exception { + Account.Id accountId = accountOps.newAccount().create(); + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add draft comment. + changeOps.change(changeId).patchset(patchset1Id).newDraftComment().author(accountId).create(); + + // Draft comments are only visible to their author. + requestScopeOps.setApiUser(accountId); + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThatList(portedComments).isEmpty(); + } + + @Test + public void publishedCommentsOfAllTypesArePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().file("myFile").content("Line 1\nLine 2\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + String rangeCommentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .message("Range comment") + .fromLine(1) + .charOffset(2) + .toLine(2) + .charOffset(1) + .ofFile("myFile") + .create(); + String lineCommentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .message("Line comment") + .onLine(1) + .ofFile("myFile") + .create(); + String fileCommentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .message("File comment") + .onFileLevelOf("myFile") + .create(); + String patchsetLevelCommentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .message("Patchset-level comment") + .onPatchsetLevel() + .create(); + + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThat(portedComments) + .comparingElementsUsing(hasUuid()) + .containsExactly( + rangeCommentUuid, lineCommentUuid, fileCommentUuid, patchsetLevelCommentUuid); + } + + // This is not the desired behavior but at least a current state which doesn't throw exceptions + // or has wrong behavior. + @Test + public void commentOnParentCommitIsIgnored() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + changeOps.change(changeId).patchset(patchset1Id).newComment().onParentCommit().create(); + + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThat(portedComments).isEmpty(); + } + + @Test + public void portedCommentHasOriginalUuid() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comment. + String commentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create(); + + List portedComments = flatten(getPortedComments(patchset2Id)); + + assertThatList(portedComments).onlyElement().uuid().isEqualTo(commentUuid); + } + + @Test + public void portedCommentHasOriginalPatchset() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comment. + String commentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).patchSet().isEqualTo(patchset1Id.get()); + } + + @Test + public void portedCommentHasOriginalPatchsetCommitId() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + TestPatchset patchset1 = changeOps.change(changeId).currentPatchset().get(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comment. + String commentUuid = + changeOps.change(changeId).patchset(patchset1.patchsetId()).newComment().create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).commitId().isEqualTo(patchset1.commitId().name()); + } + + @Test + public void portedCommentHasOriginalMessage() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + TestPatchset patchset1 = changeOps.change(changeId).currentPatchset().get(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1.patchsetId()) + .newComment() + .message("My comment text") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).message().isEqualTo("My comment text"); + } + + @Test + public void portedReplyStillRefersToParentComment() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + TestPatchset patchset1 = changeOps.change(changeId).currentPatchset().get(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comments. + String rootCommentUuid = + changeOps.change(changeId).patchset(patchset1.patchsetId()).newComment().create(); + String childCommentUuid = + changeOps + .change(changeId) + .patchset(patchset1.patchsetId()) + .newComment() + .parentUuid(rootCommentUuid) + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, childCommentUuid); + + assertThat(portedComment).inReplyTo().isEqualTo(rootCommentUuid); + } + + @Test + public void portedCommentHasOriginalAuthor() throws Exception { + // Set up change and patchsets. + Account.Id authorId = accountOps.newAccount().create(); + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comment. + String commentUuid = + changeOps.change(changeId).patchset(patchset1Id).newComment().author(authorId).create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).author().id().isEqualTo(authorId.get()); + } + + @Test + public void portedCommentHasOriginalTag() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + TestPatchset patchset1 = changeOps.change(changeId).currentPatchset().get(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1.patchsetId()) + .newComment() + .tag("My comment tag") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).tag().isEqualTo("My comment tag"); + } + + @Test + public void portedCommentHasUpdatedTimestamp() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comment. + String commentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).updated().isNotNull(); + } + + @Test + public void portedCommentDoesNotHaveChangeMessageId() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comment. + String commentUuid = changeOps.change(changeId).patchset(patchset1Id).newComment().create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + // There's currently no use case for linking ported comments to specific change messages. Hence, + // there's no reason to fill this field, which requires additional computations. + // Besides, we also don't fill this field for the comments requested for a specific patchset. + assertThat(portedComment).changeMessageId().isNull(); + } + + @Test + public void pathOfPortedCommentIsOnlyIndicatedInMap() throws Exception { + // Set up change and patchsets. + Change.Id changeId = changeOps.newChange().file("myFile").content("Line 1").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = changeOps.change(changeId).newPatchset().create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onFileLevelOf("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("myFile"); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).path().isNull(); + } + + @Test + public void portedRangeCommentCanHandleAddedLines() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(3) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).range().startLine().isEqualTo(5); + assertThat(portedComment).range().startCharacter().isEqualTo(2); + assertThat(portedComment).range().endLine().isEqualTo(6); + assertThat(portedComment).range().endCharacter().isEqualTo(5); + } + + @Test + public void portedRangeCommentCanHandleDeletedLines() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(3) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).range().startLine().isEqualTo(2); + assertThat(portedComment).range().startCharacter().isEqualTo(2); + assertThat(portedComment).range().endLine().isEqualTo(3); + assertThat(portedComment).range().endCharacter().isEqualTo(5); + } + + @Test + public void portedRangeCommentCanHandlePureRename() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(3) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("newFileName"); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).range().startLine().isEqualTo(3); + assertThat(portedComment).range().startCharacter().isEqualTo(2); + assertThat(portedComment).range().endLine().isEqualTo(4); + assertThat(portedComment).range().endCharacter().isEqualTo(5); + } + + @Test + public void portedRangeCommentCanHandleRenameWithLineShift() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .delete() + .file("newFileName") + .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(3) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("newFileName"); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).range().startLine().isEqualTo(5); + assertThat(portedComment).range().startCharacter().isEqualTo(2); + assertThat(portedComment).range().endLine().isEqualTo(6); + assertThat(portedComment).range().endCharacter().isEqualTo(5); + } + + @Test + public void portedRangeCommentAdditionallyAppearsOnCopyAtIndependentPosition() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + // Gerrit currently only identifies a copy if a rename also happens at the same time. Modify the + // renamed file slightly different than the copied file so that the end location of the comment + // is different. Modify the renamed file less so that Gerrit/Git picks it as the renamed one. + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .delete() + .file("renamedFiled") + .content("Line 1\nLine 1.1\nLine 2\nLine 3\nLine 4\n") + .file("copiedFile") + .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(3) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("renamedFiled", "copiedFile"); + CommentInfo portedCommentOnRename = getOnlyElement(portedComments.get("renamedFiled")); + assertThat(portedCommentOnRename).uuid().isEqualTo(commentUuid); + assertThat(portedCommentOnRename).range().startLine().isEqualTo(4); + assertThat(portedCommentOnRename).range().startCharacter().isEqualTo(2); + assertThat(portedCommentOnRename).range().endLine().isEqualTo(5); + assertThat(portedCommentOnRename).range().endCharacter().isEqualTo(5); + CommentInfo portedCommentOnCopy = getOnlyElement(portedComments.get("copiedFile")); + assertThat(portedCommentOnCopy).uuid().isEqualTo(commentUuid); + assertThat(portedCommentOnCopy).range().startLine().isEqualTo(5); + assertThat(portedCommentOnCopy).range().startCharacter().isEqualTo(2); + assertThat(portedCommentOnCopy).range().endLine().isEqualTo(6); + assertThat(portedCommentOnCopy).range().endCharacter().isEqualTo(5); + } + + @Test + public void lineOfPortedRangeCommentFollowsContract() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(3) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + // Line is equal to the end line, which is at 6 when ported. + assertThat(portedComment).line().isEqualTo(6); + } + + @Test + // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments. + @Ignore + public void portedRangeCommentBecomesFileCommentOnConflict() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine two\nLine three\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(2) + .charOffset(2) + .toLine(3) + .charOffset(5) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("myFile"); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).range().isNull(); + assertThat(portedComment).line().isNull(); + } + + @Test + public void portedRangeCommentEndingOnLineJustBeforeModificationCanBePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 2\nLine three\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(1) + .charOffset(2) + .toLine(2) + .charOffset(5) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).range().startLine().isEqualTo(1); + assertThat(portedComment).range().startCharacter().isEqualTo(2); + assertThat(portedComment).range().endLine().isEqualTo(2); + assertThat(portedComment).range().endCharacter().isEqualTo(5); + } + + @Test + public void portedRangeCommentEndingAtStartOfModifiedLineCanBePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 2\nLine three\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(1) + .charOffset(2) + .toLine(3) + .charOffset(0) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).range().startLine().isEqualTo(1); + assertThat(portedComment).range().startCharacter().isEqualTo(2); + assertThat(portedComment).range().endLine().isEqualTo(3); + assertThat(portedComment).range().endCharacter().isEqualTo(0); + } + + @Test + // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments. + @Ignore + public void portedRangeCommentEndingWithinModifiedLineBecomesFileComment() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 2\nLine three\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(1) + .charOffset(2) + .toLine(3) + .charOffset(4) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).range().isNull(); + assertThat(portedComment).line().isNull(); + } + + @Test + // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments. + @Ignore + public void portedRangeCommentWithinModifiedLineBecomesFileComment() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 2\nLine three\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(3) + .charOffset(2) + .toLine(3) + .charOffset(5) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).range().isNull(); + assertThat(portedComment).line().isNull(); + } + + @Test + // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments. + @Ignore + public void portedRangeCommentStartingWithinLastModifiedLineBecomesFileComment() + throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line one\nLine two\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(2) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).range().isNull(); + assertThat(portedComment).line().isNull(); + } + + @Test + public void portedRangeCommentStartingOnLineJustAfterModificationCanBePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine two\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(3) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).range().startLine().isEqualTo(3); + assertThat(portedComment).range().startCharacter().isEqualTo(2); + assertThat(portedComment).range().endLine().isEqualTo(4); + assertThat(portedComment).range().endCharacter().isEqualTo(5); + } + + @Test + // TODO(aliceks): Adjust code to not ignore conflicts and only map start/end conflicts to file + // comments. + @Ignore + public void portedRangeCommentStartingBeforeButEndingAfterModifiedLineCanBePorted() + throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 2\nLine three\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(2) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).range().startLine().isEqualTo(3); + assertThat(portedComment).range().startCharacter().isEqualTo(2); + assertThat(portedComment).range().endLine().isEqualTo(4); + assertThat(portedComment).range().endCharacter().isEqualTo(5); + } + + @Test + // TODO(aliceks): Correct handling of deleted files. + @Ignore + public void portedRangeCommentBecomesPatchsetLevelCommentOnFileDeletion() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps.change(changeId).newPatchset().file("myFile").delete().create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .fromLine(3) + .charOffset(2) + .toLine(4) + .charOffset(5) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).range().isNull(); + assertThat(portedComment).line().isNull(); + } + + @Test + public void portedLineCommentCanHandleAddedLines() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(3) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).line().isEqualTo(5); + } + + @Test + public void portedLineCommentCanHandleDeletedLines() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(3) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).line().isEqualTo(2); + } + + @Test + public void portedLineCommentCanHandlePureRename() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(3) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("newFileName"); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).line().isEqualTo(3); + } + + @Test + public void portedLineCommentCanHandleRenameWithLineShift() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n") + .create(); + PatchSet.Id patchset3Id = + changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(3) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset3Id); + + assertThatMap(portedComments).keys().containsExactly("newFileName"); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).line().isEqualTo(5); + } + + @Test + public void portedLineCommentAdditionallyAppearsOnCopyAtIndependentPosition() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + // Gerrit currently only identifies a copy if a rename also happens at the same time. Modify the + // renamed file slightly different than the copied file so that the end location of the comment + // is different. Modify the renamed file less so that Gerrit/Git picks it as the renamed one. + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .delete() + .file("renamedFiled") + .content("Line 1\nLine 1.1\nLine 2\nLine 3\nLine 4\n") + .file("copiedFile") + .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(3) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("renamedFiled", "copiedFile"); + CommentInfo portedCommentOnRename = getOnlyElement(portedComments.get("renamedFiled")); + assertThat(portedCommentOnRename).uuid().isEqualTo(commentUuid); + assertThat(portedCommentOnRename).line().isEqualTo(4); + CommentInfo portedCommentOnCopy = getOnlyElement(portedComments.get("copiedFile")); + assertThat(portedCommentOnCopy).uuid().isEqualTo(commentUuid); + assertThat(portedCommentOnCopy).line().isEqualTo(5); + } + + @Test + // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments. + @Ignore + public void portedLineCommentBecomesFileCommentOnConflict() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine two\nLine three\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(2) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("myFile"); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).line().isNull(); + } + + @Test + public void portedLineCommentOnLineJustBeforeModificationCanBePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 2\nLine three\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(2) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).line().isEqualTo(2); + } + + @Test + // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments. + @Ignore + public void portedLineCommentOnStartLineOfModificationBecomesFileComment() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 2\nSome completely\ndifferent\ncontent\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(3) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).line().isNull(); + } + + @Test + // TODO(aliceks): Adjust code to not ignore conflicts and add mapping to file comments. + @Ignore + public void portedLineCommentOnLastLineOfModificationBecomesFileComment() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 2\nSome completely\ndifferent\ncontent\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(4) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).line().isNull(); + } + + @Test + public void portedLineCommentOnLineJustAfterModificationCanBePorted() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 2\nLine three\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(4) + .ofFile("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).line().isEqualTo(4); + } + + @Test + // TODO(aliceks): Correct handling of deleted files. + @Ignore + public void portedLineCommentBecomesPatchsetLevelCommentOnFileDeletion() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps.change(changeId).newPatchset().file("myFile").delete().create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onLine(3) + .ofFile("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).range().isNull(); + assertThat(portedComment).line().isNull(); + } + + @Test + public void portedFileCommentIsObliviousToAdjustedFileContent() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onFileLevelOf("myFile") + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).line().isNull(); + } + + @Test + public void portedFileCommentCanHandleRename() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onFileLevelOf("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("newFileName"); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).line().isNull(); + } + + @Test + public void portedFileCommentAdditionallyAppearsOnCopy() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .renameTo("renamedFiled") + .file("copiedFile") + .content("Line 1\nLine 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + changeOps.change(changeId).patchset(patchset1Id).newComment().onFileLevelOf("myFile").create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly("renamedFiled", "copiedFile"); + CommentInfo portedCommentOnCopy = getOnlyElement(portedComments.get("copiedFile")); + assertThat(portedCommentOnCopy).line().isNull(); + } + + @Test + // TODO(aliceks): Correct handling of deleted files. + @Ignore + public void portedFileCommentBecomesPatchsetLevelCommentOnFileDeletion() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps.change(changeId).newPatchset().file("myFile").delete().create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + .onFileLevelOf("myFile") + .create(); + + Map> portedComments = getPortedComments(patchset2Id); + assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL); + CommentInfo portedComment = extractSpecificComment(portedComments, commentUuid); + assertThat(portedComment).range().isNull(); + assertThat(portedComment).line().isNull(); + } + + @Test + public void portedPatchsetLevelCommentIsObliviousToAdjustedFileContent() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .file("myFile") + .content("Line 1\nLine 1.1\nLine 1.2\nLine 2\nLine 3\nLine 4\n") + .create(); + // Add comment. + changeOps.change(changeId).patchset(patchset1Id).newComment().onPatchsetLevel().create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL); + } + + @Test + public void portedPatchsetLevelCommentIsObliviousToRename() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().file("myFile").content("Line 1\nLine 2\nLine 3\nLine 4\n").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps.change(changeId).newPatchset().file("myFile").renameTo("newFileName").create(); + // Add comment. + changeOps.change(changeId).patchset(patchset1Id).newComment().onPatchsetLevel().create(); + + Map> portedComments = getPortedComments(patchset2Id); + + assertThatMap(portedComments).keys().containsExactly(Patch.PATCHSET_LEVEL); + } + + @Test + public void lineCommentOnCommitMessageIsPortedToNewPosition() throws Exception { + // Set up change and patchsets. + Change.Id changeId = + changeOps.newChange().commitMessage("Summary line\n\nText 1\nText 2").create(); + PatchSet.Id patchset1Id = changeOps.change(changeId).currentPatchset().get().patchsetId(); + PatchSet.Id patchset2Id = + changeOps + .change(changeId) + .newPatchset() + .commitMessage("Summary line\n\nText 1\nText 1.1\nText 2") + .create(); + // Add comment. + String commentUuid = + changeOps + .change(changeId) + .patchset(patchset1Id) + .newComment() + // The /COMMIT_MSG file has a header of 6 lines, so the summary line is in line 7. + // Place comment on 'Text 2' which is line 10. + .onLine(10) + .ofFile(Patch.COMMIT_MSG) + .create(); + + CommentInfo portedComment = getPortedComment(patchset2Id, commentUuid); + + assertThat(portedComment).line().isEqualTo(11); + } + + private CommentInfo getPortedComment(PatchSet.Id patchsetId, String commentUuid) + throws RestApiException { + Map> portedComments = getPortedComments(patchsetId); + return extractSpecificComment(portedComments, commentUuid); + } + + private Map> getPortedComments(PatchSet.Id patchsetId) + throws RestApiException { + return gApi.changes() + .id(patchsetId.changeId().get()) + .revision(patchsetId.get()) + .portedComments(); + } + + private static CommentInfo extractSpecificComment( + Map> portedComments, String commentUuid) { + return portedComments.values().stream() + .flatMap(Collection::stream) + .filter(comment -> comment.id.equals(commentUuid)) + .collect(onlyElement()); + } + + /** + * Returns all comments in one list. The map keys (= file paths) are simply ignored. The returned + * comments won't have the file path attribute set for them as they came from a map with that + * attribute as key (= established Gerrit behavior). + */ + private static ImmutableList flatten( + Map> commentsPerFile) { + return commentsPerFile.values().stream() + .flatMap(Collection::stream) + .collect((toImmutableList())); + } + + // Unfortunately, we don't get an absolutely helpful error message when using this correspondence + // as CommentInfo doesn't have a toString() implementation. Even if we added it, the string + // representation would be quite unwieldy due to the huge number of comment attributes. + // Interestingly, using Correspondence#formattingDiffsUsing didn't improve anything. + private static Correspondence hasUuid() { + return NullAwareCorrespondence.transforming(comment -> comment.id, "hasUuid"); + } +}