Refactor PatchScriptBuilder and PatchScriptFactory
In this change:
1) Some code from PatchScriptBuilder is moved to DiffContentCalculator
2) The PatchScriptFactory.loadCommentsAndHistory method is splitted.
Instead CommentsLoader and HistoryLoader classes are added.
3) Use immutable objects instead of mutable in some cases
4) Remove unused fields/methods from PatchScript class
5) Remove dead code for the unused 'hugeFile' parameter
Change-Id: Ibdf0385fefa3fac20602632b9e8a3f4f48d964f6
diff --git a/java/com/google/gerrit/common/data/PatchScript.java b/java/com/google/gerrit/common/data/PatchScript.java
index 73e301b..d8ec6a9 100644
--- a/java/com/google/gerrit/common/data/PatchScript.java
+++ b/java/com/google/gerrit/common/data/PatchScript.java
@@ -14,6 +14,8 @@
package com.google.gerrit.common.data;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.Patch.ChangeType;
@@ -45,7 +47,7 @@
public final String mimeType;
public final String commitId;
- public PatchScriptFileInfo(
+ PatchScriptFileInfo(
String name,
FileMode mode,
SparseFileContent content,
@@ -61,20 +63,18 @@
}
}
- private Change.Key changeId;
- private ChangeType changeType;
- private List<String> header;
- private DiffPreferencesInfo diffPrefs;
- private List<Edit> edits;
- private Set<Edit> editsDueToRebase;
- private CommentDetail comments;
- private List<Patch> history;
- private boolean hugeFile;
- private boolean intralineFailure;
- private boolean intralineTimeout;
- private boolean binary;
- private PatchScriptFileInfo fileInfoA;
- private PatchScriptFileInfo fileInfoB;
+ private final Change.Key changeId;
+ private final ChangeType changeType;
+ private final ImmutableList<String> header;
+ private final DiffPreferencesInfo diffPrefs;
+ private final ImmutableList<Edit> edits;
+ private final ImmutableSet<Edit> editsDueToRebase;
+ private final ImmutableList<Patch> history;
+ private final boolean intralineFailure;
+ private final boolean intralineTimeout;
+ private final boolean binary;
+ private final PatchScriptFileInfo fileInfoA;
+ private final PatchScriptFileInfo fileInfoB;
public PatchScript(
Change.Key ck,
@@ -83,19 +83,17 @@
String nn,
FileMode om,
FileMode nm,
- List<String> h,
+ ImmutableList<String> h,
DiffPreferencesInfo dp,
SparseFileContent ca,
SparseFileContent cb,
- List<Edit> e,
- Set<Edit> editsDueToRebase,
+ ImmutableList<Edit> e,
+ ImmutableSet<Edit> editsDueToRebase,
DisplayMethod ma,
DisplayMethod mb,
String mta,
String mtb,
- CommentDetail cd,
- List<Patch> hist,
- boolean hf,
+ ImmutableList<Patch> hist,
boolean idf,
boolean idt,
boolean bin,
@@ -107,9 +105,7 @@
diffPrefs = dp;
edits = e;
this.editsDueToRebase = editsDueToRebase;
- comments = cd;
history = hist;
- hugeFile = hf;
intralineFailure = idf;
intralineTimeout = idt;
binary = bin;
@@ -122,22 +118,6 @@
return changeId;
}
- public DisplayMethod getDisplayMethodA() {
- return fileInfoA.displayMethod;
- }
-
- public DisplayMethod getDisplayMethodB() {
- return fileInfoB.displayMethod;
- }
-
- public FileMode getFileModeA() {
- return fileInfoA.mode;
- }
-
- public FileMode getFileModeB() {
- return fileInfoB.mode;
- }
-
public List<String> getPatchHeader() {
return header;
}
@@ -154,10 +134,6 @@
return fileInfoB.name;
}
- public CommentDetail getCommentDetail() {
- return comments;
- }
-
public List<Patch> getHistory() {
return history;
}
@@ -166,14 +142,6 @@
return diffPrefs;
}
- public void setDiffPrefs(DiffPreferencesInfo dp) {
- diffPrefs = dp;
- }
-
- public boolean isHugeFile() {
- return hugeFile;
- }
-
public boolean isIgnoreWhitespace() {
return diffPrefs.ignoreWhitespace != Whitespace.IGNORE_NONE;
}
@@ -186,10 +154,6 @@
return intralineTimeout;
}
- public boolean isExpandAllComments() {
- return diffPrefs.expandAllComments;
- }
-
public SparseFileContent getA() {
return fileInfoA.content;
}
@@ -198,14 +162,6 @@
return fileInfoB.content;
}
- public String getMimeTypeA() {
- return fileInfoA.mimeType;
- }
-
- public String getMimeTypeB() {
- return fileInfoB.mimeType;
- }
-
public List<Edit> getEdits() {
return edits;
}
@@ -218,14 +174,6 @@
return binary;
}
- public String getCommitIdA() {
- return fileInfoA.commitId;
- }
-
- public String getCommitIdB() {
- return fileInfoB.commitId;
- }
-
public PatchScriptFileInfo getFileInfoA() {
return fileInfoA;
}
diff --git a/java/com/google/gerrit/server/patch/DiffContentCalculator.java b/java/com/google/gerrit/server/patch/DiffContentCalculator.java
new file mode 100644
index 0000000..53f7ca6
--- /dev/null
+++ b/java/com/google/gerrit/server/patch/DiffContentCalculator.java
@@ -0,0 +1,417 @@
+// Copyright (C) 2019 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.patch;
+
+import static java.util.Comparator.comparing;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.common.data.CommentDetail;
+import com.google.gerrit.entities.Comment;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
+import com.google.gerrit.jgit.diff.ReplaceEdit;
+import com.google.gerrit.prettify.common.EditList;
+import com.google.gerrit.prettify.common.SparseFileContent;
+import com.google.gerrit.prettify.common.SparseFileContentBuilder;
+import java.util.Comparator;
+import java.util.List;
+import java.util.Optional;
+import org.eclipse.jgit.diff.Edit;
+
+/** Collects all lines and their content to be displayed in diff view. */
+class DiffContentCalculator {
+ private static final int MAX_CONTEXT = 5000000;
+
+ private static final Comparator<Edit> EDIT_SORT = comparing(Edit::getBeginA);
+
+ private final DiffPreferencesInfo diffPrefs;
+
+ DiffContentCalculator(DiffPreferencesInfo diffPrefs) {
+ this.diffPrefs = diffPrefs;
+ }
+
+ /**
+ * Gather information necessary to display line-by-line difference between 2 texts.
+ *
+ * <p>The method returns instance of {@link DiffCalculatorResult} with the following data:
+ *
+ * <ul>
+ * <li>All changed lines
+ * <li>Additional lines to be displayed above and below the changed lines
+ * <li>All changed and unchanged lines with comments
+ * <li>Additional lines to be displayed above and below lines with commentsEdits with special
+ * "fake" edits for unchanged lines with comments
+ * </ul>
+ *
+ * <p>More details can be found in {@link DiffCalculatorResult}.
+ *
+ * @param srcA Original text content
+ * @param srcB New text content
+ * @param edits List of edits which was applied to srcA to produce srcB
+ * @param comments Existing comments for srcA and srcB
+ * @return an instance of {@link DiffCalculatorResult}.
+ */
+ DiffCalculatorResult calculateDiffContent(
+ TextSource srcA, TextSource srcB, ImmutableList<Edit> edits, CommentDetail comments) {
+ int context = getContext();
+ if (srcA.src == srcB.src && srcA.size() <= context && edits.isEmpty()) {
+ // Odd special case; the files are identical (100% rename or copy)
+ // and the user has asked for context that is larger than the file.
+ // Send them the entire file, with an empty edit after the last line.
+ //
+ SparseFileContentBuilder diffA = new SparseFileContentBuilder(srcA.size());
+ for (int i = 0; i < srcA.size(); i++) {
+ srcA.copyLineTo(diffA, i);
+ }
+ DiffContent diffContent =
+ new DiffContent(diffA.build(), SparseFileContent.create(ImmutableList.of(), srcB.size()));
+ Edit emptyEdit = new Edit(srcA.size(), srcA.size());
+ return new DiffCalculatorResult(diffContent, ImmutableList.of(emptyEdit));
+ }
+ ImmutableList.Builder<Edit> builder = ImmutableList.builder();
+
+ builder.addAll(correctForDifferencesInNewlineAtEnd(srcA, srcB, edits));
+
+ boolean nonsortedEdits = false;
+ if (comments != null) {
+ ImmutableList<Edit> commentEdits = ensureCommentsVisible(comments, edits);
+ builder.addAll(commentEdits);
+ nonsortedEdits = !commentEdits.isEmpty();
+ }
+
+ ImmutableList<Edit> sortedEdits = builder.build();
+ if (nonsortedEdits) {
+ sortedEdits = ImmutableList.sortedCopyOf(EDIT_SORT, sortedEdits);
+ }
+
+ // In order to expand the skipped common lines or syntax highlight the
+ // file properly we need to give the client the complete file contents.
+ // So force our context temporarily to the complete file size.
+ //
+ DiffContent diffContent =
+ packContent(
+ srcA,
+ srcB,
+ diffPrefs.ignoreWhitespace != Whitespace.IGNORE_NONE,
+ sortedEdits,
+ MAX_CONTEXT);
+ return new DiffCalculatorResult(diffContent, sortedEdits);
+ }
+
+ private int getContext() {
+ if (diffPrefs.context == DiffPreferencesInfo.WHOLE_FILE_CONTEXT) {
+ return MAX_CONTEXT;
+ }
+ return Math.min(diffPrefs.context, MAX_CONTEXT);
+ }
+
+ private ImmutableList<Edit> correctForDifferencesInNewlineAtEnd(
+ TextSource a, TextSource b, ImmutableList<Edit> edits) {
+ // a.src.size() is the size ignoring a newline at the end whereas a.size() considers it.
+ int aSize = a.src.size();
+ int bSize = b.src.size();
+
+ if (edits.isEmpty() && (aSize == 0 || bSize == 0)) {
+ // The diff was requested for a file which was either added or deleted but which JGit doesn't
+ // consider a file addition/deletion (e.g. requesting a diff for the old file name of a
+ // renamed file looks like a deletion).
+ return edits;
+ }
+
+ if (edits.isEmpty() && (aSize != bSize)) {
+ // Only edits due to rebase were present. If we now added the edits for the newlines, the
+ // code which later assembles the file contents would fail.
+ return edits;
+ }
+
+ Optional<Edit> lastEdit = getLast(edits);
+ if (isNewlineAtEndDeleted(a, b)) {
+ Optional<Edit> lastLineEdit = lastEdit.filter(edit -> edit.getEndA() == aSize);
+
+ if (lastLineEdit.isPresent()) {
+ Edit edit = lastLineEdit.get();
+ Edit updatedLastLineEdit =
+ edit instanceof ReplaceEdit
+ ? new ReplaceEdit(
+ edit.getBeginA(),
+ edit.getEndA() + 1,
+ edit.getBeginB(),
+ edit.getEndB(),
+ ((ReplaceEdit) edit).getInternalEdits())
+ : new Edit(edit.getBeginA(), edit.getEndA() + 1, edit.getBeginB(), edit.getEndB());
+
+ ImmutableList.Builder<Edit> newEditsBuilder =
+ ImmutableList.builderWithExpectedSize(edits.size());
+ return newEditsBuilder
+ .addAll(edits.subList(0, edits.size() - 1))
+ .add(updatedLastLineEdit)
+ .build();
+ }
+ ImmutableList.Builder<Edit> newEditsBuilder =
+ ImmutableList.builderWithExpectedSize(edits.size() + 1);
+ Edit newlineEdit = new Edit(aSize, aSize + 1, bSize, bSize);
+ return newEditsBuilder.addAll(edits).add(newlineEdit).build();
+
+ } else if (isNewlineAtEndAdded(a, b)) {
+ Optional<Edit> lastLineEdit = lastEdit.filter(edit -> edit.getEndB() == bSize);
+ if (lastLineEdit.isPresent()) {
+ Edit edit = lastLineEdit.get();
+ Edit updatedLastLineEdit =
+ edit instanceof ReplaceEdit
+ ? new ReplaceEdit(
+ edit.getBeginA(),
+ edit.getEndA(),
+ edit.getBeginB(),
+ edit.getEndB() + 1,
+ ((ReplaceEdit) edit).getInternalEdits())
+ : new Edit(edit.getBeginA(), edit.getEndA(), edit.getBeginB(), edit.getEndB() + 1);
+
+ ImmutableList.Builder<Edit> newEditsBuilder =
+ ImmutableList.builderWithExpectedSize(edits.size());
+ return newEditsBuilder
+ .addAll(edits.subList(0, edits.size() - 1))
+ .add(updatedLastLineEdit)
+ .build();
+ }
+ ImmutableList.Builder<Edit> newEditsBuilder =
+ ImmutableList.builderWithExpectedSize(edits.size() + 1);
+ Edit newlineEdit = new Edit(aSize, aSize, bSize, bSize + 1);
+ return newEditsBuilder.addAll(edits).add(newlineEdit).build();
+ }
+ return edits;
+ }
+
+ private static <T> Optional<T> getLast(List<T> list) {
+ return list.isEmpty() ? Optional.empty() : Optional.ofNullable(list.get(list.size() - 1));
+ }
+
+ private boolean isNewlineAtEndDeleted(TextSource a, TextSource b) {
+ return !a.src.isMissingNewlineAtEnd() && b.src.isMissingNewlineAtEnd();
+ }
+
+ private boolean isNewlineAtEndAdded(TextSource a, TextSource b) {
+ return a.src.isMissingNewlineAtEnd() && !b.src.isMissingNewlineAtEnd();
+ }
+
+ private ImmutableList<Edit> ensureCommentsVisible(
+ CommentDetail comments, ImmutableList<Edit> edits) {
+ if (comments.getCommentsA().isEmpty() && comments.getCommentsB().isEmpty()) {
+ // No comments, no additional dummy edits are required.
+ //
+ return ImmutableList.of();
+ }
+
+ // Construct empty Edit blocks around each location where a comment is.
+ // This will force the later packContent method to include the regions
+ // containing comments, potentially combining those regions together if
+ // they have overlapping contexts. UI renders will also be able to make
+ // correct hunks from this, but because the Edit is empty they will not
+ // style it specially.
+ //
+ final ImmutableList.Builder<Edit> commmentEdits = ImmutableList.builder();
+ int lastLine;
+
+ lastLine = -1;
+ for (Comment c : comments.getCommentsA()) {
+ final int a = c.lineNbr;
+ if (lastLine != a) {
+ final int b = mapA2B(a - 1, edits);
+ if (0 <= b) {
+ getNewEditForComment(edits, new Edit(a - 1, b)).ifPresent(commmentEdits::add);
+ }
+ lastLine = a;
+ }
+ }
+
+ lastLine = -1;
+ for (Comment c : comments.getCommentsB()) {
+ int b = c.lineNbr;
+ if (lastLine != b) {
+ final int a = mapB2A(b - 1, edits);
+ if (0 <= a) {
+ getNewEditForComment(edits, new Edit(a, b - 1)).ifPresent(commmentEdits::add);
+ }
+ lastLine = b;
+ }
+ }
+ return commmentEdits.build();
+ }
+
+ private Optional<Edit> getNewEditForComment(ImmutableList<Edit> edits, Edit toAdd) {
+ final int a = toAdd.getBeginA();
+ final int b = toAdd.getBeginB();
+ for (Edit e : edits) {
+ if (e.getBeginA() <= a && a <= e.getEndA()) {
+ return Optional.empty();
+ }
+ if (e.getBeginB() <= b && b <= e.getEndB()) {
+ return Optional.empty();
+ }
+ }
+ return Optional.of(toAdd);
+ }
+
+ private int mapA2B(int a, ImmutableList<Edit> edits) {
+ if (edits.isEmpty()) {
+ // Magic special case of an unmodified file.
+ //
+ return a;
+ }
+
+ for (int i = 0; i < edits.size(); i++) {
+ final Edit e = edits.get(i);
+ if (a < e.getBeginA()) {
+ if (i == 0) {
+ // Special case of context at start of file.
+ //
+ return a;
+ }
+ return e.getBeginB() - (e.getBeginA() - a);
+ }
+ if (e.getBeginA() <= a && a <= e.getEndA()) {
+ return -1;
+ }
+ }
+
+ final Edit last = edits.get(edits.size() - 1);
+ return last.getEndB() + (a - last.getEndA());
+ }
+
+ private int mapB2A(int b, ImmutableList<Edit> edits) {
+ if (edits.isEmpty()) {
+ // Magic special case of an unmodified file.
+ //
+ return b;
+ }
+
+ for (int i = 0; i < edits.size(); i++) {
+ final Edit e = edits.get(i);
+ if (b < e.getBeginB()) {
+ if (i == 0) {
+ // Special case of context at start of file.
+ //
+ return b;
+ }
+ return e.getBeginA() - (e.getBeginB() - b);
+ }
+ if (e.getBeginB() <= b && b <= e.getEndB()) {
+ return -1;
+ }
+ }
+
+ final Edit last = edits.get(edits.size() - 1);
+ return last.getEndA() + (b - last.getEndB());
+ }
+
+ private DiffContent packContent(
+ TextSource a,
+ TextSource b,
+ boolean ignoredWhitespace,
+ ImmutableList<Edit> edits,
+ int context) {
+ SparseFileContentBuilder diffA = new SparseFileContentBuilder(a.size());
+ SparseFileContentBuilder diffB = new SparseFileContentBuilder(b.size());
+ EditList list = new EditList(edits, context, a.size(), b.size());
+ for (EditList.Hunk hunk : list.getHunks()) {
+ while (hunk.next()) {
+ if (hunk.isContextLine()) {
+ String lineA = a.getSourceLine(hunk.getCurA());
+ diffA.addLine(hunk.getCurA(), lineA);
+
+ if (ignoredWhitespace) {
+ // If we ignored whitespace in some form, also get the line
+ // from b when it does not exactly match the line from a.
+ //
+ String lineB = b.getSourceLine(hunk.getCurB());
+ if (!lineA.equals(lineB)) {
+ diffB.addLine(hunk.getCurB(), lineB);
+ }
+ }
+ hunk.incBoth();
+ continue;
+ }
+
+ if (hunk.isDeletedA()) {
+ a.copyLineTo(diffA, hunk.getCurA());
+ hunk.incA();
+ }
+
+ if (hunk.isInsertedB()) {
+ b.copyLineTo(diffB, hunk.getCurB());
+ hunk.incB();
+ }
+ }
+ }
+ return new DiffContent(diffA.build(), diffB.build());
+ }
+
+ /** Contains information to be displayed in line-by-line diff view. */
+ static class DiffCalculatorResult {
+ // This class is not @AutoValue, because Edit is mutable
+
+ /** Lines to be displayed */
+ final DiffContent diffContent;
+ /** List of edits including "fake" edits for unchanged lines with comments. */
+ final ImmutableList<Edit> edits;
+
+ DiffCalculatorResult(DiffContent diffContent, ImmutableList<Edit> edits) {
+ this.diffContent = diffContent;
+ this.edits = edits;
+ }
+ }
+
+ /** Lines to be displayed in line-by-line diff view. */
+ static class DiffContent {
+ /* All lines from the original text (i.e. srcA) to be displayed. */
+ final SparseFileContent a;
+ /**
+ * All lines from the new text (i.e. srcB) which are different than in original text. Lines are:
+ * a) All changed lines (i.e. if the content of the line was replaced with the new line) b) All
+ * inserted lines Note, that deleted lines are added to the a and are not added to b
+ */
+ final SparseFileContent b;
+
+ DiffContent(SparseFileContent a, SparseFileContent b) {
+ this.a = a;
+ this.b = b;
+ }
+ }
+
+ static class TextSource {
+ final Text src;
+
+ TextSource(Text src) {
+ this.src = src;
+ }
+
+ int size() {
+ if (src == null) {
+ return 0;
+ }
+ if (src.isMissingNewlineAtEnd()) {
+ return src.size();
+ }
+ return src.size() + 1;
+ }
+
+ void copyLineTo(SparseFileContentBuilder target, int lineNumber) {
+ target.addLine(lineNumber, getSourceLine(lineNumber));
+ }
+
+ private String getSourceLine(int lineNumber) {
+ return lineNumber >= src.size() ? "" : src.getString(lineNumber);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/patch/PatchListEntry.java b/java/com/google/gerrit/server/patch/PatchListEntry.java
index 625f56c..f37b925 100644
--- a/java/com/google/gerrit/server/patch/PatchListEntry.java
+++ b/java/com/google/gerrit/server/patch/PatchListEntry.java
@@ -37,7 +37,6 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
-import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Set;
@@ -214,9 +213,10 @@
return sizeDelta;
}
- public List<String> getHeaderLines() {
+ public ImmutableList<String> getHeaderLines() {
final IntList m = RawParseUtils.lineMap(header, 0, header.length);
- final List<String> headerLines = new ArrayList<>(m.size() - 1);
+ final ImmutableList.Builder<String> headerLines =
+ ImmutableList.builderWithExpectedSize(m.size() - 1);
for (int i = 1; i < m.size() - 1; i++) {
final int b = m.get(i);
int e = m.get(i + 1);
@@ -225,7 +225,7 @@
}
headerLines.add(RawParseUtils.decode(UTF_8, header, b, e));
}
- return headerLines;
+ return headerLines.build();
}
Patch toPatch(PatchSet.Id setId) {
diff --git a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
index 0a17303..b5842a9 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptBuilder.java
@@ -15,7 +15,6 @@
package com.google.gerrit.server.patch;
import static java.nio.charset.StandardCharsets.UTF_8;
-import static java.util.Comparator.comparing;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -23,21 +22,16 @@
import com.google.gerrit.common.data.PatchScript;
import com.google.gerrit.common.data.PatchScript.DisplayMethod;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.Comment;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.Patch.ChangeType;
import com.google.gerrit.extensions.client.DiffPreferencesInfo;
-import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
-import com.google.gerrit.prettify.common.EditList;
-import com.google.gerrit.prettify.common.SparseFileContentBuilder;
import com.google.gerrit.server.mime.FileTypeRegistry;
+import com.google.gerrit.server.patch.DiffContentCalculator.DiffCalculatorResult;
+import com.google.gerrit.server.patch.DiffContentCalculator.TextSource;
import com.google.inject.Inject;
import eu.medsea.mimeutil.MimeType;
import eu.medsea.mimeutil.MimeUtil2;
import java.io.IOException;
-import java.util.ArrayList;
-import java.util.Comparator;
-import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
@@ -53,18 +47,10 @@
class PatchScriptBuilder {
- static final int MAX_CONTEXT = 5000000;
- static final int BIG_FILE = 9000;
-
- private static final Comparator<Edit> EDIT_SORT = comparing(Edit::getBeginA);
-
private Change change;
private DiffPreferencesInfo diffPrefs;
- private List<Edit> edits;
private final FileTypeRegistry registry;
- private int context;
private IntraLineDiffCalculator intralineDiffCalculator;
- private SidesResolver sidesResolver;
@Inject
PatchScriptBuilder(FileTypeRegistry ftr) {
@@ -77,34 +63,60 @@
void setDiffPrefs(DiffPreferencesInfo dp) {
diffPrefs = dp;
-
- context = diffPrefs.context;
- if (context == DiffPreferencesInfo.WHOLE_FILE_CONTEXT) {
- context = MAX_CONTEXT;
- } else if (context > MAX_CONTEXT) {
- context = MAX_CONTEXT;
- }
}
void setIntraLineDiffCalculator(IntraLineDiffCalculator calculator) {
intralineDiffCalculator = calculator;
}
- void setSidesResolver(SidesResolver resolver) {
- sidesResolver = resolver;
- }
-
- PatchScript toPatchScript(PatchFileChange content, CommentDetail comments, List<Patch> history)
- throws IOException {
- return build(content, comments, history);
- }
-
- private PatchScript build(PatchFileChange content, CommentDetail comments, List<Patch> history)
+ PatchScript toPatchScript(
+ Repository git,
+ PatchList list,
+ PatchListEntry content,
+ CommentDetail comments,
+ ImmutableList<Patch> history)
throws IOException {
- ResolvedSides sides = sidesResolver.resolveSides(registry, oldName(content), newName(content));
+ PatchFileChange change =
+ new PatchFileChange(
+ content.getEdits(),
+ content.getEditsDueToRebase(),
+ content.getHeaderLines(),
+ content.getOldName(),
+ content.getNewName(),
+ content.getChangeType(),
+ content.getPatchType());
+ SidesResolver sidesResolver = new SidesResolver(git, list.getComparisonType());
+ ResolvedSides sides =
+ resolveSides(
+ git, sidesResolver, oldName(change), newName(change), list.getOldId(), list.getNewId());
PatchSide a = sides.a;
PatchSide b = sides.b;
+ return build(a, b, change, comments, history);
+ }
+
+ private ResolvedSides resolveSides(
+ Repository git,
+ SidesResolver sidesResolver,
+ String oldName,
+ String newName,
+ ObjectId aId,
+ ObjectId bId)
+ throws IOException {
+ try (ObjectReader reader = git.newObjectReader()) {
+ PatchSide a = sidesResolver.resolve(registry, reader, oldName, null, aId, true);
+ PatchSide b =
+ sidesResolver.resolve(registry, reader, newName, a, bId, Objects.equals(aId, bId));
+ return new ResolvedSides(a, b);
+ }
+ }
+
+ private PatchScript build(
+ PatchSide a,
+ PatchSide b,
+ PatchFileChange content,
+ CommentDetail comments,
+ ImmutableList<Patch> history) {
ImmutableList<Edit> contentEdits = content.getEdits();
ImmutableSet<Edit> editsDueToRebase = content.getEditsDueToRebase();
@@ -116,43 +128,11 @@
intralineDiffCalculator.calculateIntraLineDiff(
contentEdits, editsDueToRebase, a.id, b.id, a.src, b.src, b.treeId, b.path);
}
-
- edits = new ArrayList<>(intralineResult.edits.orElse(contentEdits));
-
- correctForDifferencesInNewlineAtEnd(a, b);
-
- if (comments != null) {
- ensureCommentsVisible(comments);
- }
-
- boolean hugeFile = false;
- if (a.src == b.src && a.size() <= context && content.getEdits().isEmpty()) {
- // Odd special case; the files are identical (100% rename or copy)
- // and the user has asked for context that is larger than the file.
- // Send them the entire file, with an empty edit after the last line.
- //
- for (int i = 0; i < a.size(); i++) {
- a.addLine(i);
- }
- edits = new ArrayList<>(1);
- edits.add(new Edit(a.size(), a.size()));
-
- } else {
- if (BIG_FILE < Math.max(a.size(), b.size())) {
- // IF the file is really large, we disable things to avoid choking
- // the browser client.
- //
- hugeFile = true;
- }
-
- // In order to expand the skipped common lines or syntax highlight the
- // file properly we need to give the client the complete file contents.
- // So force our context temporarily to the complete file size.
- //
- context = MAX_CONTEXT;
-
- packContent(a, b, diffPrefs.ignoreWhitespace != Whitespace.IGNORE_NONE);
- }
+ ImmutableList<Edit> finalEdits = intralineResult.edits.orElse(contentEdits);
+ DiffContentCalculator calculator = new DiffContentCalculator(diffPrefs);
+ DiffCalculatorResult diffCalculatorResult =
+ calculator.calculateDiffContent(
+ new TextSource(a.src), new TextSource(b.src), finalEdits, comments);
return new PatchScript(
change.getKey(),
@@ -163,17 +143,15 @@
b.fileMode,
content.getHeaderLines(),
diffPrefs,
- a.dst.build(),
- b.dst.build(),
- edits,
+ diffCalculatorResult.diffContent.a,
+ diffCalculatorResult.diffContent.b,
+ diffCalculatorResult.edits,
editsDueToRebase,
a.displayMethod,
b.displayMethod,
- a.mimeType.toString(),
- b.mimeType.toString(),
- comments,
+ a.mimeType,
+ b.mimeType,
history,
- hugeFile,
intralineResult.failure,
intralineResult.timeout,
content.getPatchType() == Patch.PatchType.BINARY,
@@ -235,204 +213,6 @@
return mode.getObjectType() == Constants.OBJ_COMMIT;
}
- private void correctForDifferencesInNewlineAtEnd(PatchSide a, PatchSide b) {
- // a.src.size() is the size ignoring a newline at the end whereas a.size() considers it.
- int aSize = a.src.size();
- int bSize = b.src.size();
-
- if (edits.isEmpty() && (aSize == 0 || bSize == 0)) {
- // The diff was requested for a file which was either added or deleted but which JGit doesn't
- // consider a file addition/deletion (e.g. requesting a diff for the old file name of a
- // renamed file looks like a deletion).
- return;
- }
-
- if (edits.isEmpty() && (aSize != bSize)) {
- // Only edits due to rebase were present. If we now added the edits for the newlines, the
- // code which later assembles the file contents would fail.
- return;
- }
-
- Optional<Edit> lastEdit = getLast(edits);
- if (isNewlineAtEndDeleted(a, b)) {
- Optional<Edit> lastLineEdit = lastEdit.filter(edit -> edit.getEndA() == aSize);
- if (lastLineEdit.isPresent()) {
- lastLineEdit.get().extendA();
- } else {
- Edit newlineEdit = new Edit(aSize, aSize + 1, bSize, bSize);
- edits.add(newlineEdit);
- }
- } else if (isNewlineAtEndAdded(a, b)) {
- Optional<Edit> lastLineEdit = lastEdit.filter(edit -> edit.getEndB() == bSize);
- if (lastLineEdit.isPresent()) {
- lastLineEdit.get().extendB();
- } else {
- Edit newlineEdit = new Edit(aSize, aSize, bSize, bSize + 1);
- edits.add(newlineEdit);
- }
- }
- }
-
- private static <T> Optional<T> getLast(List<T> list) {
- return list.isEmpty() ? Optional.empty() : Optional.ofNullable(list.get(list.size() - 1));
- }
-
- private boolean isNewlineAtEndDeleted(PatchSide a, PatchSide b) {
- return !a.src.isMissingNewlineAtEnd() && b.src.isMissingNewlineAtEnd();
- }
-
- private boolean isNewlineAtEndAdded(PatchSide a, PatchSide b) {
- return a.src.isMissingNewlineAtEnd() && !b.src.isMissingNewlineAtEnd();
- }
-
- private void ensureCommentsVisible(CommentDetail comments) {
- if (comments.getCommentsA().isEmpty() && comments.getCommentsB().isEmpty()) {
- // No comments, no additional dummy edits are required.
- //
- return;
- }
-
- // Construct empty Edit blocks around each location where a comment is.
- // This will force the later packContent method to include the regions
- // containing comments, potentially combining those regions together if
- // they have overlapping contexts. UI renders will also be able to make
- // correct hunks from this, but because the Edit is empty they will not
- // style it specially.
- //
- final List<Edit> empty = new ArrayList<>();
- int lastLine;
-
- lastLine = -1;
- for (Comment c : comments.getCommentsA()) {
- final int a = c.lineNbr;
- if (lastLine != a) {
- final int b = mapA2B(a - 1);
- if (0 <= b) {
- safeAdd(empty, new Edit(a - 1, b));
- }
- lastLine = a;
- }
- }
-
- lastLine = -1;
- for (Comment c : comments.getCommentsB()) {
- int b = c.lineNbr;
- if (lastLine != b) {
- final int a = mapB2A(b - 1);
- if (0 <= a) {
- safeAdd(empty, new Edit(a, b - 1));
- }
- lastLine = b;
- }
- }
-
- // Sort the final list by the index in A, so packContent can combine
- // them correctly later.
- //
- edits.addAll(empty);
- edits.sort(EDIT_SORT);
- }
-
- private void safeAdd(List<Edit> empty, Edit toAdd) {
- final int a = toAdd.getBeginA();
- final int b = toAdd.getBeginB();
- for (Edit e : edits) {
- if (e.getBeginA() <= a && a <= e.getEndA()) {
- return;
- }
- if (e.getBeginB() <= b && b <= e.getEndB()) {
- return;
- }
- }
- empty.add(toAdd);
- }
-
- private int mapA2B(int a) {
- if (edits.isEmpty()) {
- // Magic special case of an unmodified file.
- //
- return a;
- }
-
- for (int i = 0; i < edits.size(); i++) {
- final Edit e = edits.get(i);
- if (a < e.getBeginA()) {
- if (i == 0) {
- // Special case of context at start of file.
- //
- return a;
- }
- return e.getBeginB() - (e.getBeginA() - a);
- }
- if (e.getBeginA() <= a && a <= e.getEndA()) {
- return -1;
- }
- }
-
- final Edit last = edits.get(edits.size() - 1);
- return last.getEndB() + (a - last.getEndA());
- }
-
- private int mapB2A(int b) {
- if (edits.isEmpty()) {
- // Magic special case of an unmodified file.
- //
- return b;
- }
-
- for (int i = 0; i < edits.size(); i++) {
- final Edit e = edits.get(i);
- if (b < e.getBeginB()) {
- if (i == 0) {
- // Special case of context at start of file.
- //
- return b;
- }
- return e.getBeginA() - (e.getBeginB() - b);
- }
- if (e.getBeginB() <= b && b <= e.getEndB()) {
- return -1;
- }
- }
-
- final Edit last = edits.get(edits.size() - 1);
- return last.getEndA() + (b - last.getEndB());
- }
-
- private void packContent(PatchSide a, PatchSide b, boolean ignoredWhitespace) {
- EditList list = new EditList(edits, context, a.size(), b.size());
- for (EditList.Hunk hunk : list.getHunks()) {
- while (hunk.next()) {
- if (hunk.isContextLine()) {
- String lineA = a.getSourceLine(hunk.getCurA());
- a.dst.addLine(hunk.getCurA(), lineA);
-
- if (ignoredWhitespace) {
- // If we ignored whitespace in some form, also get the line
- // from b when it does not exactly match the line from a.
- //
- String lineB = b.getSourceLine(hunk.getCurB());
- if (!lineA.equals(lineB)) {
- b.dst.addLine(hunk.getCurB(), lineB);
- }
- }
- hunk.incBoth();
- continue;
- }
-
- if (hunk.isDeletedA()) {
- a.addLine(hunk.getCurA());
- hunk.incA();
- }
-
- if (hunk.isInsertedB()) {
- b.addLine(hunk.getCurB());
- hunk.incB();
- }
- }
- }
- }
-
private static class PatchSide {
final ObjectId treeId;
final String path;
@@ -440,10 +220,9 @@
final FileMode mode;
final byte[] srcContent;
final Text src;
- final MimeType mimeType;
+ final String mimeType;
final DisplayMethod displayMethod;
final PatchScript.FileMode fileMode;
- final SparseFileContentBuilder dst;
private PatchSide(
ObjectId treeId,
@@ -452,7 +231,7 @@
FileMode mode,
byte[] srcContent,
Text src,
- MimeType mimeType,
+ String mimeType,
DisplayMethod displayMethod,
PatchScript.FileMode fileMode) {
this.treeId = treeId;
@@ -464,33 +243,7 @@
this.mimeType = mimeType;
this.displayMethod = displayMethod;
this.fileMode = fileMode;
- dst = new SparseFileContentBuilder(size());
}
-
- int size() {
- if (src == null) {
- return 0;
- }
- if (src.isMissingNewlineAtEnd()) {
- return src.size();
- }
- return src.size() + 1;
- }
-
- void addLine(int lineNumber) {
- String lineContent = getSourceLine(lineNumber);
- dst.addLine(lineNumber, lineContent);
- }
-
- String getSourceLine(int lineNumber) {
- return lineNumber >= src.size() ? "" : src.getString(lineNumber);
- }
- }
-
- interface SidesResolver {
-
- ResolvedSides resolveSides(FileTypeRegistry typeRegistry, String oldName, String newName)
- throws IOException;
}
private static class ResolvedSides {
@@ -504,31 +257,14 @@
}
}
- static class SidesResolverImpl implements SidesResolver {
+ static class SidesResolver {
private final Repository db;
- private ComparisonType comparisonType;
- private ObjectId aId;
- private ObjectId bId;
+ private final ComparisonType comparisonType;
- SidesResolverImpl(Repository db) {
+ SidesResolver(Repository db, ComparisonType comparisonType) {
this.db = db;
- }
-
- void setTrees(ComparisonType comparisonType, ObjectId a, ObjectId b) {
this.comparisonType = comparisonType;
- this.aId = a;
- this.bId = b;
- }
-
- @Override
- public ResolvedSides resolveSides(FileTypeRegistry typeRegistry, String oldName, String newName)
- throws IOException {
- try (ObjectReader reader = db.newObjectReader()) {
- PatchSide a = resolve(typeRegistry, reader, oldName, null, aId);
- PatchSide b = resolve(typeRegistry, reader, newName, a, bId);
- return new ResolvedSides(a, b);
- }
}
PatchSide resolve(
@@ -536,13 +272,14 @@
final ObjectReader reader,
final String path,
final PatchSide other,
- final ObjectId within)
+ final ObjectId within,
+ final boolean isWithinEqualsA)
throws IOException {
try {
boolean isCommitMsg = Patch.COMMIT_MSG.equals(path);
boolean isMergeList = Patch.MERGE_LIST.equals(path);
if (isCommitMsg || isMergeList) {
- if (comparisonType.isAgainstParentOrAutoMerge() && Objects.equals(aId, within)) {
+ if (comparisonType.isAgainstParentOrAutoMerge() && isWithinEqualsA) {
return createSide(
within,
path,
@@ -550,7 +287,7 @@
FileMode.MISSING,
Text.NO_BYTES,
Text.EMPTY,
- MimeUtil2.UNKNOWN_MIME_TYPE,
+ MimeUtil2.UNKNOWN_MIME_TYPE.toString(),
DisplayMethod.NONE,
false);
}
@@ -575,7 +312,7 @@
mode,
srcContent,
src,
- MimeUtil2.UNKNOWN_MIME_TYPE,
+ MimeUtil2.UNKNOWN_MIME_TYPE.toString(),
displayMethod,
false);
}
@@ -601,7 +338,7 @@
} else {
srcContent = Text.NO_BYTES;
}
- MimeType mimeType = MimeUtil2.UNKNOWN_MIME_TYPE;
+ String mimeType = MimeUtil2.UNKNOWN_MIME_TYPE.toString();
DisplayMethod displayMethod = DisplayMethod.DIFF;
if (reuse) {
mimeType = other.mimeType;
@@ -609,10 +346,12 @@
src = other.src;
} else if (srcContent.length > 0 && FileMode.SYMLINK != mode) {
- mimeType = registry.getMimeType(path, srcContent);
- if ("image".equals(mimeType.getMediaType()) && registry.isSafeInline(mimeType)) {
+ MimeType registryMimeType = registry.getMimeType(path, srcContent);
+ if ("image".equals(registryMimeType.getMediaType())
+ && registry.isSafeInline(registryMimeType)) {
displayMethod = DisplayMethod.IMG;
}
+ mimeType = registryMimeType.toString();
}
return createSide(within, path, id, mode, srcContent, src, mimeType, displayMethod, reuse);
@@ -628,7 +367,7 @@
FileMode mode,
byte[] srcContent,
Text src,
- MimeType mimeType,
+ String mimeType,
DisplayMethod displayMethod,
boolean reuse) {
if (!reuse) {
@@ -705,20 +444,58 @@
String bPath);
}
- interface PatchFileChange {
+ static class PatchFileChange {
+ private final ImmutableList<Edit> edits;
+ private final ImmutableSet<Edit> editsDueToRebase;
+ private final ImmutableList<String> headerLines;
+ private final String oldName;
+ private final String newName;
+ private final ChangeType changeType;
+ private final Patch.PatchType patchType;
- ImmutableList<Edit> getEdits();
+ public PatchFileChange(
+ ImmutableList<Edit> edits,
+ ImmutableSet<Edit> editsDueToRebase,
+ ImmutableList<String> headerLines,
+ String oldName,
+ String newName,
+ ChangeType changeType,
+ Patch.PatchType patchType) {
+ this.edits = edits;
+ this.editsDueToRebase = editsDueToRebase;
+ this.headerLines = headerLines;
+ this.oldName = oldName;
+ this.newName = newName;
+ this.changeType = changeType;
+ this.patchType = patchType;
+ }
- ImmutableSet<Edit> getEditsDueToRebase();
+ ImmutableList<Edit> getEdits() {
+ return edits;
+ }
- List<String> getHeaderLines();
+ ImmutableSet<Edit> getEditsDueToRebase() {
+ return editsDueToRebase;
+ }
- String getNewName();
+ ImmutableList<String> getHeaderLines() {
+ return headerLines;
+ }
- String getOldName();
+ String getNewName() {
+ return newName;
+ }
- ChangeType getChangeType();
+ String getOldName() {
+ return oldName;
+ }
- Patch.PatchType getPatchType();
+ ChangeType getChangeType() {
+ return changeType;
+ }
+
+ Patch.PatchType getPatchType() {
+ return patchType;
+ }
}
}
diff --git a/java/com/google/gerrit/server/patch/PatchScriptFactory.java b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
index 2c8de1d..afc1ab1 100644
--- a/java/com/google/gerrit/server/patch/PatchScriptFactory.java
+++ b/java/com/google/gerrit/server/patch/PatchScriptFactory.java
@@ -18,7 +18,6 @@
import static com.google.common.base.Preconditions.checkState;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableSet;
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.CommentDetail;
@@ -41,10 +40,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.LargeObjectException;
import com.google.gerrit.server.notedb.ChangeNotes;
-import com.google.gerrit.server.patch.PatchScriptBuilder.IntraLineDiffCalculator;
import com.google.gerrit.server.patch.PatchScriptBuilder.IntraLineDiffCalculatorResult;
-import com.google.gerrit.server.patch.PatchScriptBuilder.PatchFileChange;
-import com.google.gerrit.server.patch.PatchScriptBuilder.SidesResolverImpl;
import com.google.gerrit.server.permissions.ChangePermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -55,9 +51,7 @@
import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject;
import java.io.IOException;
-import java.util.ArrayList;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
@@ -103,17 +97,12 @@
private final Provider<CurrentUser> userProvider;
private final PermissionBackend permissionBackend;
private final ProjectCache projectCache;
- private Optional<ChangeEdit> edit;
private final Change.Id changeId;
private boolean loadHistory = true;
private boolean loadComments = true;
private ChangeNotes notes;
- private ObjectId aId;
- private ObjectId bId;
- private List<Patch> history;
- private CommentDetail comments;
@AssistedInject
PatchScriptFactory(
@@ -200,23 +189,6 @@
public PatchScript call()
throws LargeObjectException, AuthException, InvalidChangeOperationException, IOException,
PermissionBackendException {
- validatePatchSetId(psa);
- validatePatchSetId(psb);
-
- if (psa != null) {
- checkState(parentNum < 0, "expected no parentNum when psa is present");
- checkArgument(psa.get() != 0, "edit not supported for left side");
- aId = getCommitId(psa);
- } else {
- aId = null;
- }
-
- if (psb.get() != 0) {
- bId = getCommitId(psb);
- } else {
- // Change edit: create synthetic PatchSet corresponding to the edit.
- bId = getEditRev();
- }
try {
permissionBackend.currentUser().change(notes).check(ChangePermission.READ);
@@ -230,13 +202,31 @@
try (Repository git = repoManager.openRepository(notes.getProjectName())) {
try {
- final PatchList list = listFor(keyFor(diffPrefs.ignoreWhitespace));
- final PatchScriptBuilder b = newBuilder(list, git);
+ validatePatchSetId(psa);
+ validatePatchSetId(psb);
+
+ ObjectId aId = getAId().orElse(null);
+ ObjectId bId = getBId().orElse(null);
+ boolean changeEdit = false;
+ if (bId == null) {
+ // Change edit: create synthetic PatchSet corresponding to the edit.
+ Optional<ChangeEdit> edit = editReader.byChange(notes);
+ if (!edit.isPresent()) {
+ throw new NoSuchChangeException(notes.getChangeId());
+ }
+ bId = edit.get().getEditCommit();
+ changeEdit = true;
+ }
+
+ final PatchList list = listFor(keyFor(aId, bId, diffPrefs.ignoreWhitespace));
+ final PatchScriptBuilder b = newBuilder();
final PatchListEntry content = list.get(fileName);
- loadCommentsAndHistory(content.getChangeType(), content.getOldName(), content.getNewName());
+ Optional<ImmutableList<Patch>> history = loadHistory(content, changeEdit);
+ Optional<CommentDetail> comments =
+ loadComments(content, changeEdit, history.orElse(ImmutableList.of()));
- return b.toPatchScript(new PatchListFileChange(content), comments, history);
+ return b.toPatchScript(git, list, content, comments.orElse(null), history.orElse(null));
} catch (PatchListNotAvailableException e) {
throw new NoSuchChangeException(changeId, e);
} catch (IOException e) {
@@ -254,7 +244,46 @@
}
}
- private PatchListKey keyFor(Whitespace whitespace) {
+ private Optional<CommentDetail> loadComments(
+ PatchListEntry content, boolean changeEdit, ImmutableList<Patch> history) {
+ if (!loadComments) {
+ return Optional.empty();
+ }
+ return new CommentsLoader(psa, psb, userProvider, notes, commentsUtil)
+ .load(
+ changeEdit,
+ content.getChangeType(),
+ content.getOldName(),
+ content.getNewName(),
+ history);
+ }
+
+ private Optional<ImmutableList<Patch>> loadHistory(PatchListEntry content, boolean changeEdit) {
+ if (!loadHistory) {
+ return Optional.empty();
+ }
+ HistoryLoader loader = new HistoryLoader(psa, psb, psUtil, notes, fileName);
+ return Optional.of(loader.load(changeEdit, content.getChangeType(), content.getOldName()));
+ }
+
+ private Optional<ObjectId> getAId() {
+ if (psa == null) {
+ return Optional.empty();
+ }
+ checkState(parentNum < 0, "expected no parentNum when psa is present");
+ checkArgument(psa.get() != 0, "edit not supported for left side");
+ return Optional.of(getCommitId(psa));
+ }
+
+ private Optional<ObjectId> getBId() {
+ if (psb.get() == 0) {
+ // Change edit
+ return Optional.empty();
+ }
+ return Optional.of(getCommitId(psb));
+ }
+
+ private PatchListKey keyFor(ObjectId aId, ObjectId bId, Whitespace whitespace) {
if (parentNum < 0) {
return PatchListKey.againstCommit(aId, bId, whitespace);
}
@@ -265,17 +294,14 @@
return patchListCache.get(key, notes.getProjectName());
}
- private PatchScriptBuilder newBuilder(PatchList list, Repository git) {
+ private PatchScriptBuilder newBuilder() {
final PatchScriptBuilder b = builderFactory.get();
b.setChange(notes.getChange());
b.setDiffPrefs(diffPrefs);
if (diffPrefs.intralineDifference) {
b.setIntraLineDiffCalculator(
- new IntraLineDiffCalculatorImpl(patchListCache, notes.getProjectName(), diffPrefs));
+ new IntraLineDiffCalculator(patchListCache, notes.getProjectName(), diffPrefs));
}
- SidesResolverImpl sidesResolver = new SidesResolverImpl(git);
- sidesResolver.setTrees(list.getComparisonType(), list.getOldId(), list.getNewId());
- b.setSidesResolver(sidesResolver);
return b;
}
@@ -287,14 +313,6 @@
return ps.commitId();
}
- private ObjectId getEditRev() throws AuthException, IOException {
- edit = editReader.byChange(notes);
- if (edit.isPresent()) {
- return edit.get().getEditCommit();
- }
- throw new NoSuchChangeException(notes.getChangeId());
- }
-
private void validatePatchSetId(PatchSet.Id psId) throws NoSuchChangeException {
if (psId == null) { // OK, means use base;
} else if (changeId.equals(psId.changeId())) { // OK, same change;
@@ -303,16 +321,29 @@
}
}
- private void loadCommentsAndHistory(ChangeType changeType, String oldName, String newName) {
- Map<Patch.Key, Patch> byKey = new HashMap<>();
+ private static class HistoryLoader {
+ private final PatchSet.Id psa;
+ private final PatchSet.Id psb;
+ private final PatchSetUtil psUtil;
+ private final ChangeNotes notes;
+ private final String fileName;
- if (loadHistory) {
+ HistoryLoader(
+ PatchSet.Id psa, PatchSet.Id psb, PatchSetUtil psUtil, ChangeNotes notes, String fileName) {
+ this.psa = psa;
+ this.psb = psb;
+ this.psUtil = psUtil;
+ this.notes = notes;
+ this.fileName = fileName;
+ }
+
+ private ImmutableList<Patch> load(boolean changeEdit, ChangeType changeType, String oldName) {
// This seems like a cheap trick. It doesn't properly account for a
// file that gets renamed between patch set 1 and patch set 2. We
// will wind up packing the wrong Patch object because we didn't do
// proper rename detection between the patch sets.
//
- history = new ArrayList<>();
+ ImmutableList.Builder<Patch> historyBuilder = ImmutableList.builder();
for (PatchSet ps : psUtil.byChange(notes)) {
String name = fileName;
if (psa != null) {
@@ -333,17 +364,51 @@
}
Patch p = new Patch(Patch.key(ps.id(), name));
- history.add(p);
- byKey.put(p.getKey(), p);
+ historyBuilder.add(p);
}
- if (edit != null && edit.isPresent()) {
+ if (changeEdit) {
Patch p = new Patch(Patch.key(PatchSet.id(psb.changeId(), 0), fileName));
- history.add(p);
- byKey.put(p.getKey(), p);
+ historyBuilder.add(p);
}
+ return historyBuilder.build();
+ }
+ }
+
+ private static class CommentsLoader {
+ private final PatchSet.Id psa;
+ private final PatchSet.Id psb;
+ private final Provider<CurrentUser> userProvider;
+ private final ChangeNotes notes;
+ private final CommentsUtil commentsUtil;
+ private CommentDetail comments;
+
+ CommentsLoader(
+ PatchSet.Id psa,
+ PatchSet.Id psb,
+ Provider<CurrentUser> userProvider,
+ ChangeNotes notes,
+ CommentsUtil commentsUtil) {
+ this.psa = psa;
+ this.psb = psb;
+ this.userProvider = userProvider;
+ this.notes = notes;
+ this.commentsUtil = commentsUtil;
}
- if (loadComments && edit == null) {
+ private Optional<CommentDetail> load(
+ boolean changeEdit,
+ ChangeType changeType,
+ String oldName,
+ String newName,
+ ImmutableList<Patch> history) {
+ // TODO: Implement this method with CommentDetailBuilder (this class doesn't exists yet).
+ // This is a legacy code which create final object and populate it and then returns it.
+ if (changeEdit) {
+ return Optional.empty();
+ }
+ Map<Patch.Key, Patch> byKey = new HashMap<>();
+ history.forEach(p -> byKey.put(p.getKey(), p));
+
comments = new CommentDetail(psa, psb);
switch (changeType) {
case ADDED:
@@ -392,40 +457,42 @@
break;
}
}
+ return Optional.of(comments);
}
- }
- private void loadPublished(Map<Patch.Key, Patch> byKey, String file) {
- for (Comment c : commentsUtil.publishedByChangeFile(notes, file)) {
- comments.include(notes.getChangeId(), c);
- PatchSet.Id psId = PatchSet.id(notes.getChangeId(), c.key.patchSetId);
- Patch.Key pKey = Patch.key(psId, c.key.filename);
- Patch p = byKey.get(pKey);
- if (p != null) {
- p.setCommentCount(p.getCommentCount() + 1);
+ private void loadPublished(Map<Patch.Key, Patch> byKey, String file) {
+ for (Comment c : commentsUtil.publishedByChangeFile(notes, file)) {
+ comments.include(notes.getChangeId(), c);
+ PatchSet.Id psId = PatchSet.id(notes.getChangeId(), c.key.patchSetId);
+ Patch.Key pKey = Patch.key(psId, c.key.filename);
+ Patch p = byKey.get(pKey);
+ if (p != null) {
+ p.setCommentCount(p.getCommentCount() + 1);
+ }
+ }
+ }
+
+ private void loadDrafts(Map<Patch.Key, Patch> byKey, Account.Id me, String file) {
+ for (Comment c : commentsUtil.draftByChangeFileAuthor(notes, file, me)) {
+ comments.include(notes.getChangeId(), c);
+ PatchSet.Id psId = PatchSet.id(notes.getChangeId(), c.key.patchSetId);
+ Patch.Key pKey = Patch.key(psId, c.key.filename);
+ Patch p = byKey.get(pKey);
+ if (p != null) {
+ p.setDraftCount(p.getDraftCount() + 1);
+ }
}
}
}
- private void loadDrafts(Map<Patch.Key, Patch> byKey, Account.Id me, String file) {
- for (Comment c : commentsUtil.draftByChangeFileAuthor(notes, file, me)) {
- comments.include(notes.getChangeId(), c);
- PatchSet.Id psId = PatchSet.id(notes.getChangeId(), c.key.patchSetId);
- Patch.Key pKey = Patch.key(psId, c.key.filename);
- Patch p = byKey.get(pKey);
- if (p != null) {
- p.setDraftCount(p.getDraftCount() + 1);
- }
- }
- }
-
- private static class IntraLineDiffCalculatorImpl implements IntraLineDiffCalculator {
+ private static class IntraLineDiffCalculator
+ implements PatchScriptBuilder.IntraLineDiffCalculator {
private final PatchListCache patchListCache;
private final Project.NameKey projectKey;
private final DiffPreferencesInfo diffPrefs;
- public IntraLineDiffCalculatorImpl(
+ IntraLineDiffCalculator(
PatchListCache patchListCache, Project.NameKey projectKey, DiffPreferencesInfo diffPrefs) {
this.patchListCache = patchListCache;
this.projectKey = projectKey;
@@ -454,62 +521,16 @@
case EDIT_LIST:
return IntraLineDiffCalculatorResult.success(d.getEdits());
- case DISABLED:
- return IntraLineDiffCalculatorResult.NO_RESULT;
-
case ERROR:
return IntraLineDiffCalculatorResult.FAILURE;
case TIMEOUT:
return IntraLineDiffCalculatorResult.TIMEOUT;
+ case DISABLED:
default:
return IntraLineDiffCalculatorResult.NO_RESULT;
}
}
}
-
- private static class PatchListFileChange implements PatchFileChange {
-
- private final PatchListEntry patchListEntry;
-
- PatchListFileChange(PatchListEntry patchListEntry) {
- this.patchListEntry = patchListEntry;
- }
-
- @Override
- public ImmutableList<Edit> getEdits() {
- return patchListEntry.getEdits();
- }
-
- @Override
- public ImmutableSet<Edit> getEditsDueToRebase() {
- return patchListEntry.getEditsDueToRebase();
- }
-
- @Override
- public String getNewName() {
- return patchListEntry.getNewName();
- }
-
- @Override
- public String getOldName() {
- return patchListEntry.getOldName();
- }
-
- @Override
- public ChangeType getChangeType() {
- return patchListEntry.getChangeType();
- }
-
- @Override
- public List<String> getHeaderLines() {
- return patchListEntry.getHeaderLines();
- }
-
- @Override
- public Patch.PatchType getPatchType() {
- return patchListEntry.getPatchType();
- }
- }
}
