Line data Source code
1 : // Copyright (C) 2020 The Android Open Source Project
2 : //
3 : // Licensed under the Apache License, Version 2.0 (the "License");
4 : // you may not use this file except in compliance with the License.
5 : // You may obtain a copy of the License at
6 : //
7 : // http://www.apache.org/licenses/LICENSE-2.0
8 : //
9 : // Unless required by applicable law or agreed to in writing, software
10 : // distributed under the License is distributed on an "AS IS" BASIS,
11 : // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12 : // See the License for the specific language governing permissions and
13 : // limitations under the License.
14 :
15 : package com.google.gerrit.server.restapi.change;
16 :
17 : import static com.google.common.collect.ImmutableList.toImmutableList;
18 : import static com.google.common.collect.ImmutableSet.toImmutableSet;
19 : import static java.util.stream.Collectors.groupingBy;
20 :
21 : import com.google.common.annotations.VisibleForTesting;
22 : import com.google.common.collect.ImmutableList;
23 : import com.google.common.collect.ImmutableMap;
24 : import com.google.common.collect.ImmutableSet;
25 : import com.google.common.flogger.FluentLogger;
26 : import com.google.gerrit.entities.Change;
27 : import com.google.gerrit.entities.Comment.Range;
28 : import com.google.gerrit.entities.HumanComment;
29 : import com.google.gerrit.entities.Patch;
30 : import com.google.gerrit.entities.PatchSet;
31 : import com.google.gerrit.entities.Project;
32 : import com.google.gerrit.metrics.Counter0;
33 : import com.google.gerrit.metrics.Description;
34 : import com.google.gerrit.metrics.MetricMaker;
35 : import com.google.gerrit.server.CommentsUtil;
36 : import com.google.gerrit.server.logging.Metadata;
37 : import com.google.gerrit.server.logging.TraceContext;
38 : import com.google.gerrit.server.logging.TraceContext.TraceTimer;
39 : import com.google.gerrit.server.notedb.ChangeNotes;
40 : import com.google.gerrit.server.patch.DiffMappings;
41 : import com.google.gerrit.server.patch.DiffNotAvailableException;
42 : import com.google.gerrit.server.patch.DiffOperations;
43 : import com.google.gerrit.server.patch.DiffOptions;
44 : import com.google.gerrit.server.patch.GitPositionTransformer;
45 : import com.google.gerrit.server.patch.GitPositionTransformer.BestPositionOnConflict;
46 : import com.google.gerrit.server.patch.GitPositionTransformer.FileMapping;
47 : import com.google.gerrit.server.patch.GitPositionTransformer.Mapping;
48 : import com.google.gerrit.server.patch.GitPositionTransformer.Position;
49 : import com.google.gerrit.server.patch.GitPositionTransformer.PositionedEntity;
50 : import com.google.gerrit.server.patch.filediff.FileDiffOutput;
51 : import com.google.gerrit.server.patch.filediff.FileEdits;
52 : import com.google.gerrit.server.patch.filediff.TaggedEdit;
53 : import com.google.inject.Inject;
54 : import com.google.inject.Singleton;
55 : import java.util.List;
56 : import java.util.Map;
57 : import java.util.Optional;
58 : import java.util.function.Function;
59 : import java.util.stream.Stream;
60 : import org.eclipse.jgit.lib.ObjectId;
61 :
62 : /**
63 : * Container for all logic necessary to port comments to a target patchset.
64 : *
65 : * <p>A ported comment is a comment which was left on an earlier patchset and is shown on a later
66 : * patchset. If a comment eligible for porting (e.g. before target patchset) can't be matched to its
67 : * exact position in the target patchset, we'll map it to its next best location. This can also
68 : * include a transformation of a line comment into a file comment.
69 : */
70 : @Singleton
71 : public class CommentPorter {
72 145 : private static final FluentLogger logger = FluentLogger.forEnclosingClass();
73 :
74 : @VisibleForTesting
75 : @Singleton
76 : static class Metrics {
77 : final Counter0 portedAsPatchsetLevel;
78 : final Counter0 portedAsFileLevel;
79 : final Counter0 portedAsRangeComments;
80 :
81 : @Inject
82 145 : Metrics(MetricMaker metricMaker) {
83 145 : portedAsPatchsetLevel =
84 145 : metricMaker.newCounter(
85 : "ported_comments/as_patchset_level",
86 : new Description("Total number of comments ported as patchset-level comments.")
87 145 : .setRate()
88 145 : .setUnit("count"));
89 145 : portedAsFileLevel =
90 145 : metricMaker.newCounter(
91 : "ported_comments/as_file_level",
92 : new Description("Total number of comments ported as file-level comments.")
93 145 : .setRate()
94 145 : .setUnit("count"));
95 145 : portedAsRangeComments =
96 145 : metricMaker.newCounter(
97 : "ported_comments/as_range_comments",
98 : new Description(
99 : "Total number of comments having line/range values in the ported patchset.")
100 145 : .setRate()
101 145 : .setUnit("count"));
102 145 : }
103 : }
104 :
105 : private final DiffOperations diffOperations;
106 145 : private final GitPositionTransformer positionTransformer =
107 : new GitPositionTransformer(BestPositionOnConflict.INSTANCE);
108 : private final CommentsUtil commentsUtil;
109 : private final Metrics metrics;
110 :
111 : @Inject
112 145 : public CommentPorter(DiffOperations diffOperations, CommentsUtil commentsUtil, Metrics metrics) {
113 145 : this.diffOperations = diffOperations;
114 145 : this.commentsUtil = commentsUtil;
115 145 : this.metrics = metrics;
116 145 : }
117 :
118 : /**
119 : * Ports the given comments to the target patchset.
120 : *
121 : * <p>Not all given comments are ported. Only those fulfilling some criteria (e.g. before target
122 : * patchset) are considered eligible for porting.
123 : *
124 : * <p>The returned comments represent the ported version. They don't bear any indication to which
125 : * patchset they were ported. This is intentional as the target patchset should be obvious from
126 : * the API or the used REST resources. The returned comments still have the patchset field filled.
127 : * It contains the reference to the patchset on which the comment was originally left. That
128 : * patchset number can vary among the returned comments as all comments before the target patchset
129 : * are potentially eligible for porting.
130 : *
131 : * <p>The number of returned comments can be smaller (-> only eligible ones are ported!) or larger
132 : * compared to the provided comments. The latter happens when files appear as copied in the target
133 : * patchset. In such a situation, the same comment UUID will occur more than once in the returned
134 : * comments.
135 : *
136 : * @param changeNotes the {@link ChangeNotes} of the change to which the comments belong
137 : * @param targetPatchset the patchset to which the comments should be ported
138 : * @param comments the original comments
139 : * @param filters additional filters to apply to the comments before porting. Only the remaining
140 : * comments will be ported.
141 : * @return the ported comments, in no particular order
142 : */
143 : public ImmutableList<HumanComment> portComments(
144 : ChangeNotes changeNotes,
145 : PatchSet targetPatchset,
146 : List<HumanComment> comments,
147 : List<HumanCommentFilter> filters) {
148 3 : try (TraceTimer ignored =
149 3 : TraceContext.newTimer(
150 3 : "Porting comments", Metadata.builder().patchSetId(targetPatchset.number()).build())) {
151 3 : ImmutableList<HumanCommentFilter> allFilters = addDefaultFilters(filters, targetPatchset);
152 3 : ImmutableList<HumanComment> relevantComments = filter(comments, allFilters);
153 3 : return port(changeNotes, targetPatchset, relevantComments);
154 : }
155 : }
156 :
157 : private ImmutableList<HumanCommentFilter> addDefaultFilters(
158 : List<HumanCommentFilter> filters, PatchSet targetPatchset) {
159 : // Apply the EarlierPatchsetCommentFilter first as it reduces the amount of comments before
160 : // more expensive filters are applied.
161 3 : HumanCommentFilter earlierPatchsetFilter =
162 3 : new EarlierPatchsetCommentFilter(targetPatchset.id());
163 3 : return Stream.concat(Stream.of(earlierPatchsetFilter), filters.stream())
164 3 : .collect(toImmutableList());
165 : }
166 :
167 : private ImmutableList<HumanComment> filter(
168 : List<HumanComment> allComments, ImmutableList<HumanCommentFilter> filters) {
169 3 : ImmutableList<HumanComment> filteredComments = ImmutableList.copyOf(allComments);
170 3 : for (HumanCommentFilter filter : filters) {
171 3 : filteredComments = filter.filter(filteredComments);
172 3 : }
173 3 : return filteredComments;
174 : }
175 :
176 : private ImmutableList<HumanComment> port(
177 : ChangeNotes notes, PatchSet targetPatchset, List<HumanComment> comments) {
178 3 : Map<Integer, ImmutableList<HumanComment>> commentsPerPatchset =
179 3 : comments.stream().collect(groupingBy(comment -> comment.key.patchSetId, toImmutableList()));
180 :
181 3 : ImmutableList.Builder<HumanComment> portedComments =
182 3 : ImmutableList.builderWithExpectedSize(comments.size());
183 3 : for (Integer originalPatchsetId : commentsPerPatchset.keySet()) {
184 2 : ImmutableList<HumanComment> patchsetComments = commentsPerPatchset.get(originalPatchsetId);
185 2 : PatchSet originalPatchset =
186 2 : notes.getPatchSets().get(PatchSet.id(notes.getChangeId(), originalPatchsetId));
187 2 : if (originalPatchset != null) {
188 2 : portedComments.addAll(
189 2 : portSamePatchset(
190 2 : notes.getProjectName(),
191 2 : notes.getChange(),
192 : originalPatchset,
193 : targetPatchset,
194 : patchsetComments));
195 : } else {
196 1 : logger.atWarning().log(
197 : "Some comments which should be ported refer to the non-existent patchset %s of"
198 : + " change %d. Omitting %d affected comments.",
199 1 : originalPatchsetId, notes.getChangeId().get(), patchsetComments.size());
200 : }
201 2 : }
202 3 : return portedComments.build();
203 : }
204 :
205 : private ImmutableList<HumanComment> portSamePatchset(
206 : Project.NameKey project,
207 : Change change,
208 : PatchSet originalPatchset,
209 : PatchSet targetPatchset,
210 : ImmutableList<HumanComment> comments) {
211 2 : try (TraceTimer ignored =
212 2 : TraceContext.newTimer(
213 : "Porting comments same patchset",
214 2 : Metadata.builder()
215 2 : .projectName(project.get())
216 2 : .changeId(change.getChangeId())
217 2 : .patchSetId(originalPatchset.number())
218 2 : .build())) {
219 2 : Map<Short, List<HumanComment>> commentsPerSide =
220 2 : comments.stream().collect(groupingBy(comment -> comment.side));
221 2 : ImmutableList.Builder<HumanComment> portedComments = ImmutableList.builder();
222 2 : for (Map.Entry<Short, List<HumanComment>> sideAndComments : commentsPerSide.entrySet()) {
223 2 : portedComments.addAll(
224 2 : portSamePatchsetAndSide(
225 : project,
226 : change,
227 : originalPatchset,
228 : targetPatchset,
229 2 : sideAndComments.getValue(),
230 2 : sideAndComments.getKey()));
231 2 : }
232 2 : return portedComments.build();
233 : }
234 : }
235 :
236 : private ImmutableList<HumanComment> portSamePatchsetAndSide(
237 : Project.NameKey project,
238 : Change change,
239 : PatchSet originalPatchset,
240 : PatchSet targetPatchset,
241 : List<HumanComment> comments,
242 : short side) {
243 2 : try (TraceTimer ignored =
244 2 : TraceContext.newTimer(
245 : "Porting comments same patchset and side",
246 2 : Metadata.builder()
247 2 : .projectName(project.get())
248 2 : .changeId(change.getChangeId())
249 2 : .patchSetId(originalPatchset.number())
250 2 : .commentSide(side)
251 2 : .build())) {
252 : ImmutableSet<Mapping> mappings;
253 : try {
254 2 : mappings = loadMappings(project, change, originalPatchset, targetPatchset, side);
255 2 : } catch (Exception e) {
256 2 : logger.atWarning().withCause(e).log(
257 : "Could not determine some necessary diff mappings for porting comments on change %s"
258 : + " from patchset %s to patchset %s. Mapping %d affected comments to the fallback"
259 : + " destination.",
260 2 : change.getChangeId(),
261 2 : originalPatchset.id().getId(),
262 2 : targetPatchset.id().getId(),
263 2 : comments.size());
264 2 : mappings = getFallbackMappings(comments);
265 2 : }
266 :
267 2 : ImmutableList<PositionedEntity<HumanComment>> positionedComments =
268 2 : comments.stream().map(this::toPositionedEntity).collect(toImmutableList());
269 2 : ImmutableMap<PositionedEntity<HumanComment>, HumanComment> origToPortedMap =
270 2 : positionTransformer.transform(positionedComments, mappings).stream()
271 2 : .collect(
272 2 : ImmutableMap.toImmutableMap(
273 2 : Function.identity(), PositionedEntity::getEntityAtUpdatedPosition));
274 2 : collectMetrics(origToPortedMap);
275 2 : return ImmutableList.copyOf(origToPortedMap.values());
276 : }
277 : }
278 :
279 : private ImmutableSet<Mapping> loadMappings(
280 : Project.NameKey project,
281 : Change change,
282 : PatchSet originalPatchset,
283 : PatchSet targetPatchset,
284 : short side)
285 : throws DiffNotAvailableException {
286 2 : try (TraceTimer ignored =
287 2 : TraceContext.newTimer(
288 : "Loading commit mappings",
289 2 : Metadata.builder()
290 2 : .projectName(project.get())
291 2 : .changeId(change.getChangeId())
292 2 : .patchSetId(originalPatchset.number())
293 2 : .build())) {
294 2 : ObjectId originalCommit = determineCommitId(change, originalPatchset, side);
295 2 : ObjectId targetCommit = determineCommitId(change, targetPatchset, side);
296 2 : return loadCommitMappings(project, originalCommit, targetCommit);
297 : }
298 : }
299 :
300 : private ObjectId determineCommitId(Change change, PatchSet patchset, short side) {
301 2 : return commentsUtil
302 2 : .determineCommitId(change, patchset, side)
303 2 : .orElseThrow(
304 : () ->
305 1 : new IllegalStateException(
306 1 : String.format(
307 : "Commit indicated by change %d, patchset %d, side %d doesn't exist.",
308 1 : change.getId().get(), patchset.id().get(), side)));
309 : }
310 :
311 : private ImmutableSet<Mapping> loadCommitMappings(
312 : Project.NameKey project, ObjectId originalCommit, ObjectId targetCommit)
313 : throws DiffNotAvailableException {
314 2 : try (TraceTimer ignored =
315 2 : TraceContext.newTimer(
316 2 : "Computing diffs", Metadata.builder().commit(originalCommit.name()).build())) {
317 2 : Map<String, FileDiffOutput> modifiedFiles =
318 2 : diffOperations.listModifiedFiles(
319 : project,
320 : originalCommit,
321 : targetCommit,
322 2 : DiffOptions.builder().skipFilesWithAllEditsDueToRebase(false).build());
323 2 : return modifiedFiles.values().stream()
324 2 : .map(CommentPorter::getFileEdits)
325 2 : .map(DiffMappings::toMapping)
326 2 : .collect(toImmutableSet());
327 : }
328 : }
329 :
330 : private static FileEdits getFileEdits(FileDiffOutput fileDiffOutput) {
331 1 : return FileEdits.create(
332 1 : fileDiffOutput.edits().stream().map(TaggedEdit::edit).collect(toImmutableList()),
333 1 : fileDiffOutput.oldPath(),
334 1 : fileDiffOutput.newPath());
335 : }
336 :
337 : private ImmutableSet<Mapping> getFallbackMappings(List<HumanComment> comments) {
338 : // Consider all files as deleted. -> Comments will be ported to the fallback destination, which
339 : // currently are patchset-level comments.
340 2 : return comments.stream()
341 2 : .map(comment -> comment.key.filename)
342 2 : .distinct()
343 2 : .map(FileMapping::forDeletedFile)
344 2 : .map(fileMapping -> Mapping.create(fileMapping, ImmutableSet.of()))
345 2 : .collect(toImmutableSet());
346 : }
347 :
348 : private PositionedEntity<HumanComment> toPositionedEntity(HumanComment comment) {
349 2 : return PositionedEntity.create(
350 : comment, CommentPorter::extractPosition, CommentPorter::createCommentAtNewPosition);
351 : }
352 :
353 : private static Position extractPosition(HumanComment comment) {
354 2 : Position.Builder positionBuilder = Position.builder();
355 : // Patchset-level comments don't have a file path. The transformation logic still works when
356 : // using the magic file path but it doesn't hurt to use the actual representation for "no file"
357 : // internally.
358 2 : if (!Patch.PATCHSET_LEVEL.equals(comment.key.filename)) {
359 2 : positionBuilder.filePath(comment.key.filename);
360 : }
361 2 : return positionBuilder.lineRange(extractLineRange(comment)).build();
362 : }
363 :
364 : /**
365 : * Returns {@link Optional#empty()} if the {@code comment} parameter is a file comment, or the
366 : * comment range {start_line, end_line} otherwise.
367 : */
368 : private static Optional<GitPositionTransformer.Range> extractLineRange(HumanComment comment) {
369 : // Line specifications in comment are 1-based. Line specifications in Position are 0-based.
370 2 : if (comment.range != null) {
371 : // The combination of (line, charOffset) is exclusive and must be mapped to an exclusive line.
372 : int exclusiveEndLine =
373 1 : comment.range.endChar > 0 ? comment.range.endLine : comment.range.endLine - 1;
374 1 : return Optional.of(
375 1 : GitPositionTransformer.Range.create(comment.range.startLine - 1, exclusiveEndLine));
376 : }
377 2 : if (comment.lineNbr > 0) {
378 1 : return Optional.of(GitPositionTransformer.Range.create(comment.lineNbr - 1, comment.lineNbr));
379 : }
380 : // File comment -> no range.
381 2 : return Optional.empty();
382 : }
383 :
384 : private static HumanComment createCommentAtNewPosition(
385 : HumanComment originalComment, Position newPosition) {
386 2 : HumanComment portedComment = new HumanComment(originalComment);
387 2 : portedComment.key.filename = newPosition.filePath().orElse(Patch.PATCHSET_LEVEL);
388 2 : if (portedComment.range != null && newPosition.lineRange().isPresent()) {
389 : // Comment was a range comment and also stayed one.
390 1 : portedComment.range =
391 1 : toRange(
392 1 : newPosition.lineRange().get(),
393 : portedComment.range.startChar,
394 : portedComment.range.endChar);
395 1 : portedComment.lineNbr = portedComment.range.endLine;
396 : } else {
397 2 : portedComment.range = null;
398 : // No line -> use 0 = file comment or any other comment type without an explicit line.
399 2 : portedComment.lineNbr = newPosition.lineRange().map(range -> range.start() + 1).orElse(0);
400 : }
401 2 : if (Patch.PATCHSET_LEVEL.equals(portedComment.key.filename)) {
402 : // Correct the side of the comment to Side.REVISION (= 1) if the comment was changed to
403 : // patchset level.
404 2 : portedComment.side = 1;
405 : }
406 2 : return portedComment;
407 : }
408 :
409 : private static Range toRange(
410 : GitPositionTransformer.Range lineRange, int originalStartChar, int originalEndChar) {
411 1 : int adjustedEndLine = originalEndChar > 0 ? lineRange.end() : lineRange.end() + 1;
412 1 : return new Range(lineRange.start() + 1, originalStartChar, adjustedEndLine, originalEndChar);
413 : }
414 :
415 : /**
416 : * Collect metrics from the original and ported comments.
417 : *
418 : * @param portMap map of the ported comments. The keys contain a {@link PositionedEntity} of the
419 : * original comment, and the values contain the transformed comments.
420 : */
421 : private void collectMetrics(ImmutableMap<PositionedEntity<HumanComment>, HumanComment> portMap) {
422 2 : for (Map.Entry<PositionedEntity<HumanComment>, HumanComment> entry : portMap.entrySet()) {
423 2 : HumanComment original = entry.getKey().getEntity();
424 2 : HumanComment transformed = entry.getValue();
425 :
426 2 : if (!Patch.isMagic(original.key.filename)) {
427 2 : if (Patch.PATCHSET_LEVEL.equals(transformed.key.filename)) {
428 2 : metrics.portedAsPatchsetLevel.increment();
429 2 : } else if (extractLineRange(original).isPresent()) {
430 1 : if (extractLineRange(transformed).isPresent()) {
431 1 : metrics.portedAsRangeComments.increment();
432 : } else {
433 : // line range was present in the original comment, but the ported comment is a file
434 : // level comment.
435 1 : metrics.portedAsFileLevel.increment();
436 : }
437 : }
438 : }
439 2 : }
440 2 : }
441 :
442 : /** A filter which just keeps those comments which are before the given patchset. */
443 : private static class EarlierPatchsetCommentFilter implements HumanCommentFilter {
444 :
445 : private final PatchSet.Id patchsetId;
446 :
447 3 : public EarlierPatchsetCommentFilter(PatchSet.Id patchsetId) {
448 3 : this.patchsetId = patchsetId;
449 3 : }
450 :
451 : @Override
452 : public ImmutableList<HumanComment> filter(ImmutableList<HumanComment> comments) {
453 3 : return comments.stream()
454 3 : .filter(comment -> comment.key.patchSetId < patchsetId.get())
455 3 : .collect(toImmutableList());
456 : }
457 : }
458 : }
|
