Merge changes I45ee1aaf,I6c5574ed
* changes:
Convert ChangeType to non-optional in FileDiffOutput and GitFileDiff
Add the whitespace parameter to the DiffOperations interface methods
diff --git a/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java b/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
index ed046826..0d3dcff 100644
--- a/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
+++ b/java/com/google/gerrit/server/change/FileInfoJsonNewImpl.java
@@ -89,8 +89,8 @@
FileDiffOutput fileDiff = fileDiffs.get(path);
FileInfo fileInfo = new FileInfo();
fileInfo.status =
- fileDiff.changeType().get() != Patch.ChangeType.MODIFIED
- ? fileDiff.changeType().get().getCode()
+ fileDiff.changeType() != Patch.ChangeType.MODIFIED
+ ? fileDiff.changeType().getCode()
: null;
fileInfo.oldPath = fileDiff.oldPath().orElse(null);
fileInfo.sizeDelta = fileDiff.sizeDelta();
diff --git a/java/com/google/gerrit/server/patch/DiffOperations.java b/java/com/google/gerrit/server/patch/DiffOperations.java
index 1d8438e..87a43c0 100644
--- a/java/com/google/gerrit/server/patch/DiffOperations.java
+++ b/java/com/google/gerrit/server/patch/DiffOperations.java
@@ -17,6 +17,7 @@
import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.server.patch.filediff.FileDiffOutput;
import java.util.Map;
import org.eclipse.jgit.lib.ObjectId;
@@ -80,12 +81,17 @@
* @param parentNum integer specifying which parent to use as base. If null, the only parent will
* be used or the auto-merge if {@code newCommit} is a merge commit.
* @param fileName the file name for which the diff should be evaluated.
+ * @param whitespace preference controlling whitespace effect in diff computation.
* @return the diff for the single file between the two commits.
* @throws DiffNotAvailableException if an internal error occurred in Git while evaluating the
* diff, or if an exception happened while parsing the base commit.
*/
FileDiffOutput getModifiedFileAgainstParent(
- Project.NameKey project, ObjectId newCommit, @Nullable Integer parentNum, String fileName)
+ Project.NameKey project,
+ ObjectId newCommit,
+ @Nullable Integer parentNum,
+ String fileName,
+ @Nullable DiffPreferencesInfo.Whitespace whitespace)
throws DiffNotAvailableException;
/**
@@ -96,11 +102,16 @@
* @param oldCommit 20 bytes SHA-1 of the old commit used in the diff.
* @param newCommit 20 bytes SHA-1 of the new commit used in the diff.
* @param fileName the file name for which the diff should be evaluated.
+ * @param whitespace preference controlling whitespace effect in diff computation.
* @return the diff for the single file between the two commits.
* @throws DiffNotAvailableException if an internal error occurred in Git while evaluating the
* diff.
*/
FileDiffOutput getModifiedFile(
- Project.NameKey project, ObjectId oldCommit, ObjectId newCommit, String fileName)
+ Project.NameKey project,
+ ObjectId oldCommit,
+ ObjectId newCommit,
+ String fileName,
+ @Nullable DiffPreferencesInfo.Whitespace whitespace)
throws DiffNotAvailableException;
}
diff --git a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
index 2d98d42..45ba789 100644
--- a/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
+++ b/java/com/google/gerrit/server/patch/DiffOperationsImpl.java
@@ -25,6 +25,7 @@
import com.google.gerrit.entities.Patch;
import com.google.gerrit.entities.Patch.ChangeType;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.extensions.client.DiffPreferencesInfo;
import com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.patch.diff.ModifiedFilesCache;
@@ -55,6 +56,7 @@
private static final int RENAME_SCORE = 60;
private static final DiffAlgorithm DEFAULT_DIFF_ALGORITHM = DiffAlgorithm.HISTOGRAM;
+ private static final Whitespace DEFAULT_WHITESPACE = Whitespace.IGNORE_NONE;
private final ModifiedFilesCache modifiedFilesCache;
private final FileDiffCache fileDiffCache;
@@ -112,12 +114,16 @@
@Override
public FileDiffOutput getModifiedFileAgainstParent(
- Project.NameKey project, ObjectId newCommit, @Nullable Integer parent, String fileName)
+ Project.NameKey project,
+ ObjectId newCommit,
+ @Nullable Integer parent,
+ String fileName,
+ @Nullable DiffPreferencesInfo.Whitespace whitespace)
throws DiffNotAvailableException {
try {
DiffParameters diffParams = computeDiffParameters(project, newCommit, parent);
FileDiffCacheKey key =
- createFileDiffCacheKey(project, diffParams.baseCommit(), newCommit, fileName);
+ createFileDiffCacheKey(project, diffParams.baseCommit(), newCommit, fileName, whitespace);
return getModifiedFilesForKeys(ImmutableList.of(key)).get(fileName);
} catch (IOException e) {
throw new DiffNotAvailableException(
@@ -127,9 +133,14 @@
@Override
public FileDiffOutput getModifiedFile(
- Project.NameKey project, ObjectId oldCommit, ObjectId newCommit, String fileName)
+ Project.NameKey project,
+ ObjectId oldCommit,
+ ObjectId newCommit,
+ String fileName,
+ @Nullable DiffPreferencesInfo.Whitespace whitespace)
throws DiffNotAvailableException {
- FileDiffCacheKey key = createFileDiffCacheKey(project, oldCommit, newCommit, fileName);
+ FileDiffCacheKey key =
+ createFileDiffCacheKey(project, oldCommit, newCommit, fileName, whitespace);
return getModifiedFilesForKeys(ImmutableList.of(key)).get(fileName);
}
@@ -144,10 +155,14 @@
modifiedFilesCache.get(createModifiedFilesKey(project, oldCommit, newCommit));
List<FileDiffCacheKey> fileCacheKeys = new ArrayList<>();
- fileCacheKeys.add(createFileDiffCacheKey(project, oldCommit, newCommit, COMMIT_MSG));
+ fileCacheKeys.add(
+ createFileDiffCacheKey(
+ project, oldCommit, newCommit, COMMIT_MSG, /* whitespace= */ null));
if (cmp.isAgainstAutoMerge() || isMergeAgainstParent(cmp, project, newCommit)) {
- fileCacheKeys.add(createFileDiffCacheKey(project, oldCommit, newCommit, MERGE_LIST));
+ fileCacheKeys.add(
+ createFileDiffCacheKey(
+ project, oldCommit, newCommit, MERGE_LIST, /*whitespace = */ null));
}
if (diffParams.skipFiles() == null) {
@@ -160,7 +175,8 @@
newCommit,
entity.newPath().isPresent()
? entity.newPath().get()
- : entity.oldPath().get()))
+ : entity.oldPath().get(),
+ /* whitespace= */ null))
.forEach(fileCacheKeys::add);
}
return getModifiedFilesForKeys(fileCacheKeys);
@@ -178,7 +194,7 @@
if (fileDiffOutput.isEmpty() || allDueToRebase(fileDiffOutput)) {
continue;
}
- if (fileDiffOutput.changeType().get() == Patch.ChangeType.DELETED) {
+ if (fileDiffOutput.changeType() == ChangeType.DELETED) {
files.put(fileDiffOutput.oldPath().get(), fileDiffOutput);
} else {
files.put(fileDiffOutput.newPath().get(), fileDiffOutput);
@@ -189,8 +205,8 @@
private static boolean allDueToRebase(FileDiffOutput fileDiffOutput) {
return fileDiffOutput.allEditsDueToRebase()
- && (!(fileDiffOutput.changeType().get() == ChangeType.RENAMED
- || fileDiffOutput.changeType().get() == ChangeType.COPIED));
+ && (!(fileDiffOutput.changeType() == ChangeType.RENAMED
+ || fileDiffOutput.changeType() == ChangeType.COPIED));
}
private boolean isMergeAgainstParent(ComparisonType cmp, Project.NameKey project, ObjectId commit)
@@ -209,7 +225,12 @@
}
private static FileDiffCacheKey createFileDiffCacheKey(
- Project.NameKey project, ObjectId aCommit, ObjectId bCommit, String newPath) {
+ Project.NameKey project,
+ ObjectId aCommit,
+ ObjectId bCommit,
+ String newPath,
+ @Nullable Whitespace whitespace) {
+ whitespace = whitespace == null ? DEFAULT_WHITESPACE : whitespace;
return FileDiffCacheKey.builder()
.project(project)
.oldCommit(aCommit)
@@ -217,7 +238,7 @@
.newFilePath(newPath)
.renameScore(RENAME_SCORE)
.diffAlgorithm(DEFAULT_DIFF_ALGORITHM)
- .whitespace(Whitespace.IGNORE_NONE)
+ .whitespace(whitespace)
.build();
}
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
index d945da3..fb2aa3d 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffCacheImpl.java
@@ -288,7 +288,7 @@
return FileDiffOutput.builder()
.oldPath(FileHeaderUtil.getOldPath(fileHeader))
.newPath(FileHeaderUtil.getNewPath(fileHeader))
- .changeType(Optional.of(changeType))
+ .changeType(changeType)
.patchType(Optional.of(FileHeaderUtil.getPatchType(fileHeader)))
.headerLines(FileHeaderUtil.getHeaderLines(fileHeader))
.edits(
diff --git a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
index 46ce28b..3348033 100644
--- a/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
+++ b/java/com/google/gerrit/server/patch/filediff/FileDiffOutput.java
@@ -47,7 +47,7 @@
public abstract Optional<String> newPath();
/** The change type of the underlying file, e.g. added, deleted, renamed, etc... */
- public abstract Optional<Patch.ChangeType> changeType();
+ public abstract Patch.ChangeType changeType();
/** The patch type of the underlying file, e.g. unified, binary , etc... */
public abstract Optional<Patch.PatchType> patchType();
@@ -95,10 +95,11 @@
}
/** Returns an entity representing an unchanged file between two commits. */
- static FileDiffOutput empty(String filePath) {
+ public static FileDiffOutput empty(String filePath) {
return builder()
.oldPath(Optional.empty())
.newPath(Optional.of(filePath))
+ .changeType(ChangeType.MODIFIED)
.headerLines(ImmutableList.of())
.edits(ImmutableList.of())
.size(0)
@@ -123,9 +124,7 @@
if (newPath().isPresent()) {
result += stringSize(newPath().get());
}
- if (changeType().isPresent()) {
- result += 4;
- }
+ result += 4; // changeType
if (patchType().isPresent()) {
result += 4;
}
@@ -145,7 +144,7 @@
public abstract Builder newPath(Optional<String> value);
- public abstract Builder changeType(Optional<ChangeType> value);
+ public abstract Builder changeType(ChangeType value);
public abstract Builder patchType(Optional<PatchType> value);
@@ -169,9 +168,6 @@
private static final FieldDescriptor NEW_PATH_DESCRIPTOR =
FileDiffOutputProto.getDescriptor().findFieldByNumber(2);
- private static final FieldDescriptor CHANGE_TYPE_DESCRIPTOR =
- FileDiffOutputProto.getDescriptor().findFieldByNumber(3);
-
private static final FieldDescriptor PATCH_TYPE_DESCRIPTOR =
FileDiffOutputProto.getDescriptor().findFieldByNumber(4);
@@ -182,6 +178,7 @@
.setSize(fileDiff.size())
.setSizeDelta(fileDiff.sizeDelta())
.addAllHeaderLines(fileDiff.headerLines())
+ .setChangeType(fileDiff.changeType().name())
.addAllEdits(
fileDiff.edits().stream()
.map(
@@ -206,10 +203,6 @@
builder.setNewPath(fileDiff.newPath().get());
}
- if (fileDiff.changeType().isPresent()) {
- builder.setChangeType(fileDiff.changeType().get().name());
- }
-
if (fileDiff.patchType().isPresent()) {
builder.setPatchType(fileDiff.patchType().get().name());
}
@@ -225,6 +218,7 @@
.size(proto.getSize())
.sizeDelta(proto.getSizeDelta())
.headerLines(proto.getHeaderLinesList().stream().collect(ImmutableList.toImmutableList()))
+ .changeType(ChangeType.valueOf(proto.getChangeType()))
.edits(
proto.getEditsList().stream()
.map(
@@ -244,9 +238,6 @@
if (proto.hasField(NEW_PATH_DESCRIPTOR)) {
builder.newPath(Optional.of(proto.getNewPath()));
}
- if (proto.hasField(CHANGE_TYPE_DESCRIPTOR)) {
- builder.changeType(Optional.of(Patch.ChangeType.valueOf(proto.getChangeType())));
- }
if (proto.hasField(PATCH_TYPE_DESCRIPTOR)) {
builder.patchType(Optional.of(Patch.PatchType.valueOf(proto.getPatchType())));
}
diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
index 81c0e5d..a01d447 100644
--- a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
+++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
@@ -77,7 +77,7 @@
.fileHeader(FileHeaderUtil.toString(fileHeader))
.oldPath(FileHeaderUtil.getOldPath(fileHeader))
.newPath(FileHeaderUtil.getNewPath(fileHeader))
- .changeType(Optional.of(FileHeaderUtil.getChangeType(fileHeader)))
+ .changeType(FileHeaderUtil.getChangeType(fileHeader))
.patchType(Optional.of(FileHeaderUtil.getPatchType(fileHeader)))
.oldMode(Optional.of(mapFileMode(diffEntry.getOldMode())))
.newMode(Optional.of(mapFileMode(diffEntry.getNewMode())))
@@ -96,6 +96,7 @@
.oldId(oldId)
.newId(newId)
.newPath(Optional.of(newFilePath))
+ .changeType(ChangeType.MODIFIED)
.edits(ImmutableList.of())
.fileHeader("")
.build();
@@ -126,7 +127,7 @@
public abstract Optional<Patch.FileMode> newMode();
/** The change type associated with the file. */
- public abstract Optional<ChangeType> changeType();
+ public abstract ChangeType changeType();
/** The patch type associated with the file. */
public abstract Optional<PatchType> patchType();
@@ -150,9 +151,7 @@
if (newPath().isPresent()) {
result += stringSize(newPath().get());
}
- if (changeType().isPresent()) {
- result += 4;
- }
+ result += 4;
if (patchType().isPresent()) {
result += 4;
}
@@ -188,7 +187,7 @@
public abstract Builder newMode(Optional<Patch.FileMode> value);
- public abstract Builder changeType(Optional<ChangeType> value);
+ public abstract Builder changeType(ChangeType value);
public abstract Builder patchType(Optional<PatchType> value);
@@ -223,7 +222,8 @@
GitFileDiffProto.newBuilder()
.setFileHeader(gitFileDiff.fileHeader())
.setOldId(idConverter.toByteString(gitFileDiff.oldId().toObjectId()))
- .setNewId(idConverter.toByteString(gitFileDiff.newId().toObjectId()));
+ .setNewId(idConverter.toByteString(gitFileDiff.newId().toObjectId()))
+ .setChangeType(gitFileDiff.changeType().name());
gitFileDiff
.edits()
.forEach(
@@ -246,9 +246,6 @@
if (gitFileDiff.newMode().isPresent()) {
builder.setNewMode(gitFileDiff.newMode().get().name());
}
- if (gitFileDiff.changeType().isPresent()) {
- builder.setChangeType(gitFileDiff.changeType().get().name());
- }
if (gitFileDiff.patchType().isPresent()) {
builder.setPatchType(gitFileDiff.patchType().get().name());
}
@@ -267,7 +264,8 @@
.collect(toImmutableList()))
.fileHeader(proto.getFileHeader())
.oldId(AbbreviatedObjectId.fromObjectId(idConverter.fromByteString(proto.getOldId())))
- .newId(AbbreviatedObjectId.fromObjectId(idConverter.fromByteString(proto.getNewId())));
+ .newId(AbbreviatedObjectId.fromObjectId(idConverter.fromByteString(proto.getNewId())))
+ .changeType(ChangeType.valueOf(proto.getChangeType()));
if (proto.hasField(OLD_PATH_DESCRIPTOR)) {
builder.oldPath(Optional.of(proto.getOldPath()));
@@ -281,9 +279,6 @@
if (proto.hasField(NEW_MODE_DESCRIPTOR)) {
builder.newMode(Optional.of(Patch.FileMode.valueOf(proto.getNewMode())));
}
- if (proto.hasField(CHANGE_TYPE_DESCRIPTOR)) {
- builder.changeType(Optional.of(Patch.ChangeType.valueOf(proto.getChangeType())));
- }
if (proto.hasField(PATCH_TYPE_DESCRIPTOR)) {
builder.patchType(Optional.of(Patch.PatchType.valueOf(proto.getPatchType())));
}
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffOutputSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffOutputSerializerTest.java
index 4307954..44ea55a 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffOutputSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/FileDiffOutputSerializerTest.java
@@ -37,7 +37,7 @@
FileDiffOutput.builder()
.oldPath(Optional.of("old_file_path.txt"))
.newPath(Optional.empty())
- .changeType(Optional.of(ChangeType.DELETED))
+ .changeType(ChangeType.DELETED)
.patchType(Optional.of(PatchType.UNIFIED))
.size(23)
.sizeDelta(10)
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffSerializerTest.java
index 8030818..93441a4 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/GitFileDiffSerializerTest.java
@@ -47,7 +47,7 @@
.newPath(Optional.empty())
.oldId(AbbreviatedObjectId.fromObjectId(OLD_ID))
.newId(AbbreviatedObjectId.fromObjectId(NEW_ID))
- .changeType(Optional.of(ChangeType.DELETED))
+ .changeType(ChangeType.DELETED)
.patchType(Optional.of(PatchType.UNIFIED))
.oldMode(Optional.of(FileMode.REGULAR_FILE))
.newMode(Optional.of(FileMode.REGULAR_FILE))