From d4dfa342c1f5a916d325e198ff19d3f702bfb3d9 Mon Sep 17 00:00:00 2001 From: James Edwards-Jones 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. --- lib/gitlab/checks/change_access.rb | 7 ++++++- lib/gitlab/checks/commit_check.rb | 4 ++-- spec/lib/gitlab/checks/change_access_spec.rb | 4 ++-- 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 -- cgit v1.2.1