From 01ff084a4df76ba0856a513aca9bdf8f1d550365 Mon Sep 17 00:00:00 2001 From: Boyan Tabakov Date: Wed, 4 Sep 2013 10:33:09 +0300 Subject: Improved large commit handling. Previously, only number of changed files mattered. Now, number of lines to render in the diff are also taken into account. A hard limit is set, above which diffs are not rendered and users are not allowed to override that. This prevents high server resource usage with huge commits. Related to #1745, #2259 In addition, handle large commits for MergeRequests and Compare controllers. Also fixes a bug where diffs are loaded twice, if user goes directly to merge_requests/:id/diffs URL. --- app/models/commit.rb | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) (limited to 'app/models/commit.rb') diff --git a/app/models/commit.rb b/app/models/commit.rb index f8ca6a5fe82..dd1f9801878 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -6,15 +6,41 @@ class Commit attr_mentionable :safe_message - # Safe amount of files with diffs in one commit to render + # Safe amount of changes (files and lines) in one commit to render # Used to prevent 500 error on huge commits by suppressing diff # - DIFF_SAFE_SIZE = 100 + # User can force display of diff above this size + DIFF_SAFE_FILES = 100 + DIFF_SAFE_LINES = 5000 + # Commits above this size will not be rendered in HTML + DIFF_HARD_LIMIT_FILES = 500 + DIFF_HARD_LIMIT_LINES = 10000 def self.decorate(commits) commits.map { |c| self.new(c) } end + # Calculate number of lines to render for diffs + def self.diff_line_count(diffs) + diffs.reduce(0){|sum, d| sum + d.diff.lines.count} + end + + def self.diff_suppress?(diffs, line_count = nil) + # optimize - check file count first + return true if diffs.size > DIFF_SAFE_FILES + + line_count ||= Commit::diff_line_count(diffs) + line_count > DIFF_SAFE_LINES + end + + def self.diff_force_suppress?(diffs, line_count = nil) + # optimize - check file count first + return true if diffs.size > DIFF_HARD_LIMIT_FILES + + line_count ||= Commit::diff_line_count(diffs) + line_count > DIFF_HARD_LIMIT_LINES + end + attr_accessor :raw def initialize(raw_commit) @@ -27,6 +53,19 @@ class Commit @raw.id end + def diff_line_count + @diff_line_count ||= Commit::diff_line_count(self.diffs) + @diff_line_count + end + + def diff_suppress? + Commit::diff_suppress?(self.diffs, diff_line_count) + end + + def diff_force_suppress? + Commit::diff_force_suppress?(self.diffs, diff_line_count) + end + # Returns a string describing the commit for use in a link title # # Example -- cgit v1.2.1