summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJames Edwards-Jones <jedwardsjones@gitlab.com>2018-02-15 05:21:17 +0000
committerJames Edwards-Jones <jedwardsjones@gitlab.com>2018-02-19 10:01:25 +0000
commitd4dfa342c1f5a916d325e198ff19d3f702bfb3d9 (patch)
treed41ef1cd19f694aaedd0f5fc168684b6fda18317
parentf7c662c733a2a24c044c1f0bad03cfc63f704258 (diff)
downloadgitlab-ce-d4dfa342c1f5a916d325e198ff19d3f702bfb3d9.tar.gz
Avoid slow File Lock checks when not used
Also avoid double commit lookup during file lock check by reusing memoized commits.
-rw-r--r--lib/gitlab/checks/change_access.rb7
-rw-r--r--lib/gitlab/checks/commit_check.rb4
-rw-r--r--spec/lib/gitlab/checks/change_access_spec.rb4
3 files changed, 10 insertions, 5 deletions
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