Fix diffs being collapsed unnecessarily

4 files changed, 88 insertions, 17 deletions

diff --git a/changelogs/unreleased/diff-collapsing.yml b/changelogs/unreleased/diff-collapsing.yml
new file mode 100644
index 000000000..4edbd3618
--- /dev/null
+++ b/changelogs/unreleased/diff-collapsing.yml
@@ -0,0 +1,5 @@
+---
+title: Fix diffs being collapsed unnecessarily
+merge_request: 854
+author:
+type: fixed
diff --git a/internal/diff/diff.go b/internal/diff/diff.go
index 9365ef1ac..acc3e574f 100644
--- a/internal/diff/diff.go
+++ b/internal/diff/diff.go
@@ -43,7 +43,6 @@ type Parser struct {
 	linesProcessed    int
 	bytesProcessed    int
 	finished          bool
-	overSafeLimits    bool
 	err               error
 }
 
@@ -149,7 +148,7 @@ func (parser *Parser) Parse() bool {
 		}
 	}
 
-	if parser.overSafeLimits && parser.currentDiff.lineCount > 0 {
+	if parser.limits.CollapseDiffs && parser.isOverSafeLimits() && parser.currentDiff.lineCount > 0 {
 		parser.linesProcessed -= parser.currentDiff.lineCount
 		parser.bytesProcessed -= len(parser.currentDiff.Patch)
 		parser.currentDiff.Collapsed = true
@@ -166,8 +165,6 @@ func (parser *Parser) Parse() bool {
 		}
 	}
 
-	parser.setOverSafeLimits()
-
 	return true
 }
 
@@ -183,16 +180,10 @@ func (parser *Parser) Err() error {
 	return parser.err
 }
 
-func (parser *Parser) setOverSafeLimits() {
-	if parser.overSafeLimits || !parser.limits.CollapseDiffs {
-		return
-	}
-
-	if parser.filesProcessed >= parser.limits.SafeMaxFiles ||
-		parser.linesProcessed >= parser.limits.SafeMaxLines ||
-		parser.bytesProcessed >= parser.limits.SafeMaxBytes {
-		parser.overSafeLimits = true
-	}
+func (parser *Parser) isOverSafeLimits() bool {
+	return parser.filesProcessed > parser.limits.SafeMaxFiles ||
+		parser.linesProcessed > parser.limits.SafeMaxLines ||
+		parser.bytesProcessed > parser.limits.SafeMaxBytes
 }
 
 func (parser *Parser) cacheRawLines(reader *bufio.Reader) {
diff --git a/internal/diff/diff_test.go b/internal/diff/diff_test.go
new file mode 100644
index 000000000..a052719d9
--- /dev/null
+++ b/internal/diff/diff_test.go
@@ -0,0 +1,75 @@
+package diff
+
+import (
+	"fmt"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestDiffParserWithLargeDiff(t *testing.T) {
+	bigPatch := strings.Repeat("+Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.\n", 100000)
+	rawDiff := fmt.Sprintf(`:000000 100644 0000000000000000000000000000000000000000 4cc7061661b8f52891bc1b39feb4d856b21a1067 A	big.txt
+:000000 100644 0000000000000000000000000000000000000000 3be11c69355948412925fa5e073d76d58ff3afd2 A	file-00.txt
+
+diff --git a/big.txt b/big.txt
+new file mode 100644
+index 0000000000000000000000000000000000000000..4cc7061661b8f52891bc1b39feb4d856b21a1067
+--- /dev/null
++++ b/big.txt
+@@ -0,0 +1,100000 @@
+%sdiff --git a/file-00.txt b/file-00.txt
+new file mode 100644
+index 0000000000000000000000000000000000000000..3be11c69355948412925fa5e073d76d58ff3afd2
+--- /dev/null
++++ b/file-00.txt
+@@ -0,0 +1 @@
++Lorem ipsum
+`, bigPatch)
+
+	limits := Limits{
+		EnforceLimits: true,
+		SafeMaxFiles:  3,
+		SafeMaxBytes:  200,
+		SafeMaxLines:  200,
+		MaxFiles:      5,
+		MaxBytes:      10000000,
+		MaxLines:      10000000,
+		CollapseDiffs: true,
+	}
+	diffParser := NewDiffParser(strings.NewReader(rawDiff), limits)
+
+	diffs := []*Diff{}
+	for diffParser.Parse() {
+		diffs = append(diffs, diffParser.Diff())
+	}
+
+	expectedDiffs := []*Diff{
+		&Diff{
+			OldMode:   0,
+			NewMode:   0100644,
+			FromID:    "0000000000000000000000000000000000000000",
+			ToID:      "4cc7061661b8f52891bc1b39feb4d856b21a1067",
+			FromPath:  []byte("big.txt"),
+			ToPath:    []byte("big.txt"),
+			Status:    65,
+			Collapsed: true,
+			lineCount: 100003,
+		},
+		&Diff{
+			OldMode:   0,
+			NewMode:   0100644,
+			FromID:    "0000000000000000000000000000000000000000",
+			ToID:      "3be11c69355948412925fa5e073d76d58ff3afd2",
+			FromPath:  []byte("file-00.txt"),
+			ToPath:    []byte("file-00.txt"),
+			Status:    65,
+			Collapsed: false,
+			Patch:     []byte("@@ -0,0 +1 @@\n+Lorem ipsum\n"),
+			lineCount: 4,
+		},
+	}
+
+	require.Equal(t, expectedDiffs, diffs)
+}
diff --git a/internal/service/diff/commit_test.go b/internal/service/diff/commit_test.go
index 967e4d4d1..551f4e151 100644
--- a/internal/service/diff/commit_test.go
+++ b/internal/service/diff/commit_test.go
@@ -554,11 +554,11 @@ func TestSuccessfulCommitDiffRequestWithLimits(t *testing.T) {
 				SafeMaxBytes:  5 * 5 * 1024,
 			},
 			result: []diffAttributes{
-				{path: "CHANGELOG"},
+				{path: "CHANGELOG", collapsed: true},
 				{path: "CONTRIBUTING.md", collapsed: true},
-				{path: "LICENSE", collapsed: true},
+				{path: "LICENSE"},
 				{path: "PROCESS.md", collapsed: true},
-				{path: "VERSION", collapsed: true},
+				{path: "VERSION"},
 			},
 		},
 		{