Merge branch 'commit-limits' into 'master'

Commit limits

Prevent timeouts when creating/viewing Merge Requests with many
commits. Also, reduce the number of commits shown in the UI from 500
to 100.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/14031

This MR avoids Unicorn timeouts in some places and takes 4.5 seconds
off load times in others, when manually testing with a 4500-commit
compare/MR in the linux repo. It does not limit the number of Commit
objects instantiated in memory, just the amount of HTML generated. It
seems that having 4500 commits objects is not a problem in itself. If
it ever becomes one we could do something for Commit objects like we
did for Diff (introduce a CommitCollection or something).

See merge request !3095

4 files changed, 26 insertions, 6 deletions

diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb
index a09e91578b6..f994c9e6170 100644
--- a/app/helpers/commits_helper.rb
+++ b/app/helpers/commits_helper.rb
@@ -211,4 +211,15 @@ module CommitsHelper
   def clean(string)
     Sanitize.clean(string, remove_contents: true)
   end
+
+  def limited_commits(commits)
+    if commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
+      [
+        commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE),
+        commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE
+      ]
+    else
+      [commits, 0]
+    end
+  end
 end
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index df08d3a6dfb..33884118595 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -17,7 +17,7 @@ class MergeRequestDiff < ActiveRecord::Base
   include Sortable
 
   # Prevent store of diff if commits amount more then 500
-  COMMITS_SAFE_SIZE = 500
+  COMMITS_SAFE_SIZE = 100
 
   belongs_to :merge_request
 
diff --git a/app/views/projects/commits/_commit_list.html.haml b/app/views/projects/commits/_commit_list.html.haml
index ce60fbdf032..bac9e244d36 100644
--- a/app/views/projects/commits/_commit_list.html.haml
+++ b/app/views/projects/commits/_commit_list.html.haml
@@ -1,11 +1,14 @@
+- commits, hidden = limited_commits(@commits)
+- commits = Commit.decorate(commits, @project)
+
 %div.panel.panel-default
   .panel-heading
     Commits (#{@commits.count})
-  - if @commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
+  - if hidden > 0
     %ul.well-list
-      - Commit.decorate(@commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE), @project).each do |commit|
+      - commits.each do |commit|
         = render "projects/commits/inline_commit", commit: commit, project: @project
       %li.warning-row.unstyled
-        other #{@commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE} commits hidden to prevent performance issues.
+        #{number_with_delimiter(hidden)} additional commits have been omitted to prevent performance issues.
   - else
-    %ul.well-list= render Commit.decorate(@commits, @project), project: @project
+    %ul.well-list= render commits, project: @project
diff --git a/app/views/projects/commits/_commits.html.haml b/app/views/projects/commits/_commits.html.haml
index 6c631228002..a7e3c2478c2 100644
--- a/app/views/projects/commits/_commits.html.haml
+++ b/app/views/projects/commits/_commits.html.haml
@@ -1,7 +1,9 @@
 - unless defined?(project)
   - project = @project
 
-- @commits.group_by { |c| c.committed_date.to_date }.sort.reverse.each do |day, commits|
+- commits, hidden = limited_commits(@commits)
+
+- commits.group_by { |c| c.committed_date.to_date }.sort.reverse.each do |day, commits|
   .row.commits-row
     .col-md-2.hidden-xs.hidden-sm
       %h5.commits-row-date
@@ -13,3 +15,7 @@
       %ul.bordered-list
         = render commits, project: project
   %hr.lists-separator
+
+- if hidden > 0
+  .alert.alert-warning
+    #{number_with_delimiter(hidden)} additional commits have been omitted to prevent performance issues.