Merge "Respect .gitattributes diff overrides in diff operations"

java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java

diff --git a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
index 580aef5..23b0024 100644
--- a/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
+++ b/java/com/google/gerrit/server/patch/gitfilediff/GitFileDiff.java
@@ -32,6 +32,7 @@
 import com.google.gerrit.server.patch.filediff.Edit;
 import com.google.protobuf.Descriptors.FieldDescriptor;
 import java.util.Optional;
+import org.eclipse.jgit.attributes.Attribute;
 import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.lib.AbbreviatedObjectId;
 import org.eclipse.jgit.lib.FileMode;
@@ -66,8 +67,17 @@
    * parameters.
    */
   static GitFileDiff create(DiffEntry diffEntry, FileHeader fileHeader) {
+    Attribute diffAttr = diffEntry.getDiffAttribute();
+    // Treat the file as binary if .gitattributes explicitly unsets diffing (e.g. "-diff"
+    // or the "binary" macro which JGit expands to "-diff -merge -text").
+    boolean isBinary = diffAttr != null && diffAttr.getState() == Attribute.State.UNSET;
+
     ImmutableList<Edit> edits =
-        fileHeader.toEditList().stream().map(Edit::fromJGitEdit).collect(toImmutableList());
+        isBinary
+            ? ImmutableList.of()
+            : fileHeader.toEditList().stream().map(Edit::fromJGitEdit).collect(toImmutableList());
+
+    PatchType patchType = isBinary ? PatchType.BINARY : FileHeaderUtil.getPatchType(fileHeader);
 
     return builder()
         .edits(edits)
@@ -77,7 +87,7 @@
         .oldPath(FileHeaderUtil.getOldPath(fileHeader))
         .newPath(FileHeaderUtil.getNewPath(fileHeader))
         .changeType(FileHeaderUtil.getChangeType(fileHeader))
-        .patchType(Optional.of(FileHeaderUtil.getPatchType(fileHeader)))
+        .patchType(Optional.of(patchType))
         .oldMode(Optional.of(mapFileMode(diffEntry.getOldMode())))
         .newMode(Optional.of(mapFileMode(diffEntry.getNewMode())))
         .build();

javatests/com/google/gerrit/server/patch/DiffOperationsTest.java

diff --git a/javatests/com/google/gerrit/server/patch/DiffOperationsTest.java b/javatests/com/google/gerrit/server/patch/DiffOperationsTest.java
index 7c8555e..1234262 100644
--- a/javatests/com/google/gerrit/server/patch/DiffOperationsTest.java
+++ b/javatests/com/google/gerrit/server/patch/DiffOperationsTest.java
@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.entities.Patch.ChangeType;
+import com.google.gerrit.entities.Patch.PatchType;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.RefNames;
 import com.google.gerrit.server.git.GitRepositoryManager;
@@ -423,6 +424,66 @@
     }
   }
 
+  @Test
+  public void gitattributesDiffOverride() throws Exception {
+    String jsonFile = "file_2.json";
+    // 1. First, let's create a commit where the file is modified but there is no .gitattributes.
+    // It should be diffed as UNIFIED.
+    ObjectId oldCommitId1 =
+        createCommit(repo, null, ImmutableList.of(new FileEntity(jsonFile, "{}")));
+    ObjectId newCommitId1 =
+        createCommit(
+            repo, oldCommitId1, ImmutableList.of(new FileEntity(jsonFile, "{\"foo\": \"bar\"}")));
+    FileDiffOutput diffOutput1 =
+        diffOperations.getModifiedFileAgainstParent(
+            testProjectName, newCommitId1, 0, jsonFile, null);
+    assertThat(diffOutput1.patchType()).hasValue(PatchType.UNIFIED);
+    assertThat(diffOutput1.edits()).isNotEmpty();
+
+    // 2. Now let's create a commit where .gitattributes specifies "-diff" for the file.
+    // It should be treated as BINARY.
+    ObjectId oldCommitId2 =
+        createCommit(
+            repo,
+            null,
+            ImmutableList.of(
+                new FileEntity(".gitattributes", jsonFile + " -diff"),
+                new FileEntity(jsonFile, "{}")));
+    ObjectId newCommitId2 =
+        createCommit(
+            repo,
+            oldCommitId2,
+            ImmutableList.of(
+                new FileEntity(".gitattributes", jsonFile + " -diff"),
+                new FileEntity(jsonFile, "{\"foo\": \"bar\"}")));
+    FileDiffOutput diffOutput2 =
+        diffOperations.getModifiedFileAgainstParent(
+            testProjectName, newCommitId2, 0, jsonFile, null);
+    assertThat(diffOutput2.patchType()).hasValue(PatchType.BINARY);
+    assertThat(diffOutput2.edits()).isEmpty();
+
+    // 3. Let's also test with "binary" macro attribute.
+    ObjectId oldCommitId3 =
+        createCommit(
+            repo,
+            null,
+            ImmutableList.of(
+                new FileEntity(".gitattributes", jsonFile + " binary"),
+                new FileEntity(jsonFile, "{}")));
+    ObjectId newCommitId3 =
+        createCommit(
+            repo,
+            oldCommitId3,
+            ImmutableList.of(
+                new FileEntity(".gitattributes", jsonFile + " binary"),
+                new FileEntity(jsonFile, "{\"foo\": \"bar\"}")));
+    FileDiffOutput diffOutput3 =
+        diffOperations.getModifiedFileAgainstParent(
+            testProjectName, newCommitId3, 0, jsonFile, null);
+    assertThat(diffOutput3.patchType()).hasValue(PatchType.BINARY);
+    assertThat(diffOutput3.edits()).isEmpty();
+  }
+
   static class FileEntity {
     String name;
     String content;