diff --git a/lib/gitlab/diff/highlight_cache.rb b/lib/gitlab/diff/highlight_cache.rb
index 0eb22e6b3cbd8c965bcc1929d0ef5d22ce529b00..e873e9c17d57cbe5af03f66ad607a974042345bf 100644
--- a/lib/gitlab/diff/highlight_cache.rb
+++ b/lib/gitlab/diff/highlight_cache.rb
@@ -20,10 +20,23 @@ def initialize(diff_collection)
# - Assigns DiffFile#highlighted_diff_lines for cached files
#
def decorate(diff_file)
- if content = read_file(diff_file)
- diff_file.highlighted_diff_lines = content.map do |line|
- Gitlab::Diff::Line.safe_init_from_hash(line)
- end
+ content = read_file(diff_file)
+
+ return [] unless content
+
+ if content.empty? && recache_due_to_size?(diff_file)
+ # If the file is missing from the cache and there's reason to believe
+ # it is uncached due to a size issue around changing the values for
+ # max patch size, manually populate the hash and then set the value.
+ #
+ new_cache_content = {}
+ new_cache_content[diff_file.file_path] = diff_file.highlighted_diff_lines.map(&:to_hash)
+
+ write_to_redis_hash(new_cache_content)
+
+ set_highlighted_diff_lines(diff_file, read_file(diff_file))
+ else
+ set_highlighted_diff_lines(diff_file, content)
end
end
@@ -58,6 +71,28 @@ def key
private
+ def set_highlighted_diff_lines(diff_file, content)
+ diff_file.highlighted_diff_lines = content.map do |line|
+ Gitlab::Diff::Line.safe_init_from_hash(line)
+ end
+ end
+
+ def recache_due_to_size?(diff_file)
+ diff_file_class = diff_file.diff.class
+
+ current_patch_safe_limit_bytes = diff_file_class.patch_safe_limit_bytes
+ default_patch_safe_limit_bytes = diff_file_class.patch_safe_limit_bytes(diff_file_class::DEFAULT_MAX_PATCH_BYTES)
+
+ # If the diff is >= than the default limit, but less than the current
+ # limit, it is likely uncached due to having hit the default limit,
+ # making it eligible for recalculating.
+ #
+ diff_file.diff.diff_bytesize.between?(
+ default_patch_safe_limit_bytes,
+ current_patch_safe_limit_bytes
+ )
+ end
+
def cacheable_files
strong_memoize(:cacheable_files) do
diff_files.select { |file| cacheable?(file) && read_file(file).nil? }
diff --git a/lib/gitlab/git/diff.rb b/lib/gitlab/git/diff.rb
index 09a49b6c1ca647a0da9399ca52f9519f8244468c..78c47023c081fa31d54f697ff8cbb53c0c227873 100644
--- a/lib/gitlab/git/diff.rb
+++ b/lib/gitlab/git/diff.rb
@@ -120,8 +120,8 @@ def binary_message(old_path, new_path)
# default.
#
# Patches surpassing this limit should still be persisted in the database.
- def patch_safe_limit_bytes
- patch_hard_limit_bytes / 10
+ def patch_safe_limit_bytes(limit = patch_hard_limit_bytes)
+ limit / 10
end
# Returns the limit for a single diff file (patch).
@@ -174,9 +174,13 @@ def line_count
@line_count ||= Util.count_lines(@diff)
end
+ def diff_bytesize
+ @diff_bytesize ||= @diff.bytesize
+ end
+
def too_large?
if @too_large.nil?
- @too_large = @diff.bytesize >= self.class.patch_hard_limit_bytes
+ @too_large = diff_bytesize >= self.class.patch_hard_limit_bytes
else
@too_large
end
@@ -194,7 +198,7 @@ def too_large!
def collapsed?
return @collapsed if defined?(@collapsed)
- @collapsed = !expanded && @diff.bytesize >= self.class.patch_safe_limit_bytes
+ @collapsed = !expanded && diff_bytesize >= self.class.patch_safe_limit_bytes
end
def collapse!
diff --git a/spec/lib/gitlab/diff/highlight_cache_spec.rb b/spec/lib/gitlab/diff/highlight_cache_spec.rb
index 7e926f8609682689ce77c89a26621af0f57f2fa4..f6810d7a96670486025dd7bb45a2fcbf63865f3e 100644
--- a/spec/lib/gitlab/diff/highlight_cache_spec.rb
+++ b/spec/lib/gitlab/diff/highlight_cache_spec.rb
@@ -43,7 +43,8 @@
describe '#decorate' do
# Manually creates a Diff::File object to avoid triggering the cache on
- # the FileCollection::MergeRequestDiff
+ # the FileCollection::MergeRequestDiff
+ #
let(:diff_file) do
diffs = merge_request.diffs
raw_diff = diffs.diffable.raw_diffs(diffs.diff_options.merge(paths: ['CHANGELOG'])).first
@@ -73,6 +74,37 @@
expect(rich_texts).to all(be_html_safe)
end
+
+ context "when diff_file is uncached due to default_max_patch_bytes change" do
+ before do
+ expect(cache).to receive(:read_file).at_least(:once).and_return([])
+
+ # Stub out the application's default and current patch size limits. We
+ # want them to be different, and the diff file to be sized between
+ # the 2 values.
+ #
+ diff_file_size_kb = (diff_file.diff.diff.bytesize * 10)
+
+ stub_const("#{diff_file.diff.class}::DEFAULT_MAX_PATCH_BYTES", diff_file_size_kb - 1 )
+ expect(diff_file.diff.class).to receive(:patch_safe_limit_bytes).and_return(diff_file_size_kb + 1)
+ expect(diff_file.diff.class)
+ .to receive(:patch_safe_limit_bytes)
+ .with(diff_file.diff.class::DEFAULT_MAX_PATCH_BYTES)
+ .and_call_original
+ end
+
+ it "manually writes highlighted lines to the cache" do
+ expect(cache).to receive(:write_to_redis_hash).and_call_original
+
+ cache.decorate(diff_file)
+ end
+
+ it "assigns highlighted diff lines to the DiffFile" do
+ expect(diff_file.highlighted_diff_lines.size).to be > 5
+
+ cache.decorate(diff_file)
+ end
+ end
end
shared_examples 'caches missing entries' do
diff --git a/spec/lib/gitlab/git/diff_spec.rb b/spec/lib/gitlab/git/diff_spec.rb
index 117c519e98de6deb2366bcf8607a778f0f7afff9..980a52bb61e867cd5438dd1937ad14a62851c7f5 100644
--- a/spec/lib/gitlab/git/diff_spec.rb
+++ b/spec/lib/gitlab/git/diff_spec.rb
@@ -284,13 +284,21 @@
end
describe '#line_count' do
- it 'returns the correct number of lines' do
- diff = described_class.new(gitaly_diff)
+ let(:diff) { described_class.new(gitaly_diff) }
+ it 'returns the correct number of lines' do
expect(diff.line_count).to eq(7)
end
end
+ describe "#diff_bytesize" do
+ let(:diff) { described_class.new(gitaly_diff) }
+
+ it "returns the size of the diff in bytes" do
+ expect(diff.diff_bytesize).to eq(diff.diff.bytesize)
+ end
+ end
+
describe '#too_large?' do
it 'returns true for a diff that is too large' do
diff = described_class.new(diff: 'a' * 204800)