Commit e9b5b10a authored by Francisco Javier López's avatar Francisco Javier López Committed by Douwe Maan

Skip per-commit validations which have already passed on another change/branch

parent cf200b64
......@@ -469,6 +469,10 @@ class Commit
!!merged_merge_request(user)
end
def cache_key
"commit:#{sha}"
end
private
def commit_reference(from, referable_commit_id, full: false)
......
---
title: Skip per-commit validations already evaluated
merge_request: 23984
author:
type: performance
......@@ -33,6 +33,22 @@ module Gitlab
def tag_exists?
project.repository.tag_exists?(tag_name)
end
def validate_once(resource)
Gitlab::SafeRequestStore.fetch(cache_key_for_resource(resource)) do
yield(resource)
true
end
end
def cache_key_for_resource(resource)
"git_access:#{checker_cache_key}:#{resource.cache_key}"
end
def checker_cache_key
self.class.name.demodulize.underscore
end
end
end
end
......@@ -14,13 +14,17 @@ module Gitlab
return if deletion? || newrev.nil?
return unless should_run_diff_validations?
return if commits.empty?
return unless uses_raw_delta_validations?
file_paths = []
process_raw_deltas do |diff|
file_paths << (diff.new_path || diff.old_path)
validate_diff(diff)
process_commits do |commit|
validate_once(commit) do
commit.raw_deltas.each do |diff|
file_paths << (diff.new_path || diff.old_path)
validate_diff(diff)
end
end
end
validate_file_paths(file_paths)
......@@ -28,17 +32,13 @@ module Gitlab
private
def should_run_diff_validations?
validate_lfs_file_locks?
end
def validate_lfs_file_locks?
strong_memoize(:validate_lfs_file_locks) do
project.lfs_enabled? && project.any_lfs_file_locks?
end
end
def uses_raw_delta_validations?
def should_run_diff_validations?
validations_for_diff.present? || path_validations.present?
end
......@@ -59,16 +59,14 @@ module Gitlab
validate_lfs_file_locks? ? [lfs_file_locks_validation] : []
end
def process_raw_deltas
def process_commits
logger.log_timed(LOG_MESSAGES[:diff_content_check]) do
# n+1: https://gitlab.com/gitlab-org/gitlab-ee/issues/3593
::Gitlab::GitalyClient.allow_n_plus_1_calls do
commits.each do |commit|
logger.check_timeout_reached
commit.raw_deltas.each do |diff|
yield(diff)
end
yield(commit)
end
end
end
......
......@@ -7,6 +7,7 @@ module Gitlab
ERROR_MESSAGE = 'LFS objects are missing. Ensure LFS is properly set up or try a manual "git lfs push --all".'.freeze
def validate!
return unless project.lfs_enabled?
return if skip_lfs_integrity_check
logger.log_timed(LOG_MESSAGE) do
......
......@@ -47,5 +47,43 @@ describe Gitlab::Checks::DiffCheck do
end
end
end
context 'commit diff validations' do
before do
allow(subject).to receive(:validations_for_diff).and_return([lambda { |diff| return }])
expect_any_instance_of(Commit).to receive(:raw_deltas).and_call_original
subject.validate!
end
context 'when request store is inactive' do
it 'are run for every commit' do
expect_any_instance_of(Commit).to receive(:raw_deltas).and_call_original
subject.validate!
end
end
context 'when request store is active', :request_store do
it 'are cached for every commit' do
expect_any_instance_of(Commit).not_to receive(:raw_deltas)
subject.validate!
end
it 'are run for not cached commits' do
allow(project.repository).to receive(:new_commits).and_return(
project.repository.commits_between('be93687618e4b132087f430a4d8fc3a609c9b77c', 'a5391128b0ef5d21df5dd23d98557f4ef12fae20')
)
change_access.instance_variable_set(:@commits, project.repository.new_commits)
expect(project.repository.new_commits.first).not_to receive(:raw_deltas).and_call_original
expect(project.repository.new_commits.last).to receive(:raw_deltas).and_call_original
subject.validate!
end
end
end
end
end
......@@ -709,10 +709,22 @@ describe Gitlab::GitAccess do
project.add_developer(user)
end
it 'checks LFS integrity only for first change' do
expect_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).exactly(1).times
context 'when LFS is not enabled' do
it 'does not run LFSIntegrity check' do
expect(Gitlab::Checks::LfsIntegrity).not_to receive(:new)
push_access_check
push_access_check
end
end
context 'when LFS is enabled' do
it 'checks LFS integrity only for first change' do
allow(project).to receive(:lfs_enabled?).and_return(true)
expect_any_instance_of(Gitlab::Checks::LfsIntegrity).to receive(:objects_missing?).exactly(1).times
push_access_check
end
end
end
......
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