From d4d4140c17e409b3a41824e3888427aec80afebf Mon Sep 17 00:00:00 2001
From: James Edwards-Jones <jedwardsjones@gitlab.com>
Date: Thu, 15 Feb 2018 05:21:17 +0000
Subject: Avoid slow File Lock checks when not used

Also avoid double commit lookup during file lock check by reusing
memoized commits.
---
 changelogs/unreleased/jej-avoid-slow-file-lock-checks.yml | 5 +++++
 lib/gitlab/checks/change_access.rb                        | 7 ++++++-
 lib/gitlab/checks/commit_check.rb                         | 4 ++--
 spec/lib/gitlab/checks/change_access_spec.rb              | 4 ++--
 4 files changed, 15 insertions(+), 5 deletions(-)
 create mode 100644 changelogs/unreleased/jej-avoid-slow-file-lock-checks.yml

diff --git a/changelogs/unreleased/jej-avoid-slow-file-lock-checks.yml b/changelogs/unreleased/jej-avoid-slow-file-lock-checks.yml
new file mode 100644
index 00000000000..5d1d9fc3deb
--- /dev/null
+++ b/changelogs/unreleased/jej-avoid-slow-file-lock-checks.yml
@@ -0,0 +1,5 @@
+---
+title: Avoid timeout on push by skipping slow File Lock checks where possible
+merge_request: 17140
+author:
+type: performance
diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb
index d75e73dac10..94d45c17ca0 100644
--- a/lib/gitlab/checks/change_access.rb
+++ b/lib/gitlab/checks/change_access.rb
@@ -120,6 +120,7 @@ module Gitlab
 
       def commits_check
         return if deletion? || newrev.nil?
+        return unless should_run_commit_validations?
 
         # n+1: https://gitlab.com/gitlab-org/gitlab-ee/issues/3593
         ::Gitlab::GitalyClient.allow_n_plus_1_calls do
@@ -138,6 +139,10 @@ module Gitlab
 
       private
 
+      def should_run_commit_validations?
+        commit_check.validate_lfs_file_locks?
+      end
+
       def updated_from_web?
         protocol == 'web'
       end
@@ -175,7 +180,7 @@ module Gitlab
       end
 
       def commits
-        project.repository.new_commits(newrev)
+        @commits ||= project.repository.new_commits(newrev)
       end
     end
   end
diff --git a/lib/gitlab/checks/commit_check.rb b/lib/gitlab/checks/commit_check.rb
index ae0cd142378..43a52b493bb 100644
--- a/lib/gitlab/checks/commit_check.rb
+++ b/lib/gitlab/checks/commit_check.rb
@@ -35,14 +35,14 @@ module Gitlab
         end
       end
 
-      private
-
       def validate_lfs_file_locks?
         strong_memoize(:validate_lfs_file_locks) do
           project.lfs_enabled? && project.lfs_file_locks.any? && newrev && oldrev
         end
       end
 
+      private
+
       def lfs_file_locks_validation
         lambda do |paths|
           lfs_lock = project.lfs_file_locks.where(path: paths).where.not(user_id: user.id).first
diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb
index 475b5c5cfb2..b49ddbfc780 100644
--- a/spec/lib/gitlab/checks/change_access_spec.rb
+++ b/spec/lib/gitlab/checks/change_access_spec.rb
@@ -190,7 +190,7 @@ describe Gitlab::Checks::ChangeAccess do
 
       context 'with LFS not enabled' do
         it 'skips the validation' do
-          expect_any_instance_of(described_class).not_to receive(:lfs_file_locks_validation)
+          expect_any_instance_of(Gitlab::Checks::CommitCheck).not_to receive(:validate)
 
           subject.exec
         end
@@ -207,7 +207,7 @@ describe Gitlab::Checks::ChangeAccess do
           end
         end
 
-        context 'when change is sent by the author od the lock' do
+        context 'when change is sent by the author of the lock' do
           let(:user) { owner }
 
           it "doesn't raise any error" do
-- 
cgit v1.2.1