Commit d4dfa342 authored by James Edwards-Jones's avatar James Edwards-Jones

Avoid slow File Lock checks when not used

Also avoid double commit lookup during file lock check by reusing
memoized commits.
parent f7c662c7
...@@ -120,6 +120,7 @@ module Gitlab ...@@ -120,6 +120,7 @@ module Gitlab
def commits_check def commits_check
return if deletion? || newrev.nil? return if deletion? || newrev.nil?
return unless should_run_commit_validations?
# n+1: https://gitlab.com/gitlab-org/gitlab-ee/issues/3593 # n+1: https://gitlab.com/gitlab-org/gitlab-ee/issues/3593
::Gitlab::GitalyClient.allow_n_plus_1_calls do ::Gitlab::GitalyClient.allow_n_plus_1_calls do
...@@ -138,6 +139,10 @@ module Gitlab ...@@ -138,6 +139,10 @@ module Gitlab
private private
def should_run_commit_validations?
commit_check.validate_lfs_file_locks?
end
def updated_from_web? def updated_from_web?
protocol == 'web' protocol == 'web'
end end
...@@ -175,7 +180,7 @@ module Gitlab ...@@ -175,7 +180,7 @@ module Gitlab
end end
def commits def commits
project.repository.new_commits(newrev) @commits ||= project.repository.new_commits(newrev)
end end
end end
end end
......
...@@ -35,14 +35,14 @@ module Gitlab ...@@ -35,14 +35,14 @@ module Gitlab
end end
end end
private
def validate_lfs_file_locks? def validate_lfs_file_locks?
strong_memoize(:validate_lfs_file_locks) do strong_memoize(:validate_lfs_file_locks) do
project.lfs_enabled? && project.lfs_file_locks.any? && newrev && oldrev project.lfs_enabled? && project.lfs_file_locks.any? && newrev && oldrev
end end
end end
private
def lfs_file_locks_validation def lfs_file_locks_validation
lambda do |paths| lambda do |paths|
lfs_lock = project.lfs_file_locks.where(path: paths).where.not(user_id: user.id).first lfs_lock = project.lfs_file_locks.where(path: paths).where.not(user_id: user.id).first
......
...@@ -190,7 +190,7 @@ describe Gitlab::Checks::ChangeAccess do ...@@ -190,7 +190,7 @@ describe Gitlab::Checks::ChangeAccess do
context 'with LFS not enabled' do context 'with LFS not enabled' do
it 'skips the validation' 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 subject.exec
end end
...@@ -207,7 +207,7 @@ describe Gitlab::Checks::ChangeAccess do ...@@ -207,7 +207,7 @@ describe Gitlab::Checks::ChangeAccess do
end end
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 } let(:user) { owner }
it "doesn't raise any error" do it "doesn't raise any error" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment