checks: Convert PushRuleChecks to allow for batching

The PushRuleChecks are implemented as BaseSingleChecker, where they
always operate on a single change only. This is inefficient for a subset
of push rules, most namely for the file size rules: we could load all
blobs via a single RPC call and inspect their sizes instead of doing an
RPC call per change.

Prepare for this change by converting the PushRules to be implemented as
BaseBulkChecker. For now, nothing really changes: we just deconstruct
the bulk changes into SingleChangeAccesses and then pass those into our
subrules. But with this change in place, we can easily convert subsets
of our push rules to use batching.

ee/lib/ee/gitlab/checks/single_change_access.rb → ee/lib/ee/gitlab/checks/changes_access.rb

@@ -3,12 +3,12 @@
 module EE
   module Gitlab
     module Checks
-      module SingleChangeAccess
+      module ChangesAccess
         extend ActiveSupport::Concern
         extend ::Gitlab::Utils::Override
 
-        override :ref_level_checks
-        def ref_level_checks
+        override :bulk_access_checks!
+        def bulk_access_checks!
           super
 
           PushRuleCheck.new(self).validate!

ee/lib/ee/gitlab/checks/push_rule_check.rb

@@ -3,7 +3,7 @@
 module EE
   module Gitlab
     module Checks
-      class PushRuleCheck < ::Gitlab::Checks::BaseSingleChecker
+      class PushRuleCheck < ::Gitlab::Checks::BaseBulkChecker
         def validate!
           return unless push_rule
 
@@ -19,17 +19,25 @@ module EE
         # @return [Nil] returns nil unless an error is raised
         # @raise [Gitlab::GitAccess::ForbiddenError] if check fails
         def check_tag_or_branch!
-          if tag_name
-            PushRules::TagCheck.new(change_access).validate!
-          else
-            PushRules::BranchCheck.new(change_access).validate!
+          changes_access.single_change_accesses.each do |single_change_access|
+            if single_change_access.tag_name
+              PushRules::TagCheck.new(single_change_access).validate!
+            else
+              PushRules::BranchCheck.new(single_change_access).validate!
+            end
           end
+
+          nil
         end
 
         # @return [Nil] returns nil unless an error is raised
         # @raise [Gitlab::GitAccess::ForbiddenError] if check fails
         def check_file_size!
-          PushRules::FileSizeCheck.new(change_access).validate!
+          changes_access.single_change_accesses.each do |single_change_access|
+            PushRules::FileSizeCheck.new(single_change_access).validate!
+          end
+
+          nil
         end
 
         # Run the checks one after the other.

ee/spec/lib/ee/gitlab/checks/push_rule_check_spec.rb

@@ -3,9 +3,16 @@
 require 'spec_helper'
 
 RSpec.describe EE::Gitlab::Checks::PushRuleCheck do
-  include_context 'push rules checks context'
+  include_context 'changes access checks context'
 
   let(:push_rule) { create(:push_rule, :commit_message) }
+  let(:project) { create(:project, :public, :repository, push_rule: push_rule) }
+
+  before do
+    allow(project.repository).to receive(:new_commits).and_return(
+      project.repository.commits_between('be93687618e4b132087f430a4d8fc3a609c9b77c', '54fcc214b94e78d7a41a9a8fe6d87a5e59500e51')
+    )
+  end
 
   shared_examples "push checks" do
     before do
@@ -28,8 +35,11 @@ RSpec.describe EE::Gitlab::Checks::PushRuleCheck do
     end
 
     context 'when tag name exists' do
-      before do
-        allow(change_access).to receive(:tag_name).and_return(true)
+      let(:changes) do
+        [
+          # Update of a tag.
+          { oldrev: oldrev, newrev: newrev, ref: 'refs/tags/name' }
+        ]
       end
 
       it 'validates tags push rules' do
@@ -43,8 +53,11 @@ RSpec.describe EE::Gitlab::Checks::PushRuleCheck do
     end
 
     context 'when tag name does not exists' do
-      before do
-        allow(change_access).to receive(:tag_name).and_return(false)
+      let(:changes) do
+        [
+          # Update of a branch.
+          { oldrev: oldrev, newrev: newrev, ref: 'refs/heads/name' }
+        ]
       end
 
       it 'validates branches push rules' do
@@ -56,6 +69,24 @@ RSpec.describe EE::Gitlab::Checks::PushRuleCheck do
         subject.validate!
       end
     end
+
+    context 'when tag and branch exist' do
+      let(:changes) do
+        [
+          { oldrev: oldrev, newrev: newrev, ref: 'refs/heads/name' },
+          { oldrev: oldrev, newrev: newrev, ref: 'refs/tags/name' }
+        ]
+      end
+
+      it 'validates tag and branch push rules' do
+        expect_any_instance_of(EE::Gitlab::Checks::PushRules::TagCheck)
+          .to receive(:validate!)
+        expect_any_instance_of(EE::Gitlab::Checks::PushRules::BranchCheck)
+          .to receive(:validate!)
+
+        subject.validate!
+      end
+    end
   end
 
   describe '#validate!' do

ee/spec/lib/gitlab/checks/single_change_access_spec.rb → ee/spec/lib/gitlab/checks/changes_access_spec.rb

@@ -2,13 +2,28 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Checks::SingleChangeAccess do
+RSpec.describe Gitlab::Checks::ChangesAccess do
   describe '#validate!' do
     include_context 'push rules checks context'
 
     let(:push_rule) { create(:push_rule, deny_delete_tag: true) }
+    let(:changes) do
+      [
+        { oldrev: oldrev, newrev: newrev, ref: ref }
+      ]
+    end
+
+    let(:changes_access) do
+      Gitlab::Checks::ChangesAccess.new(
+        changes,
+        project: project,
+        user_access: user_access,
+        protocol: protocol,
+        logger: logger
+      )
+    end
 
-    subject { change_access }
+    subject { changes_access }
 
     it_behaves_like 'check ignored when push rule unlicensed'
 

lib/gitlab/checks/changes_access.rb

@@ -112,3 +112,5 @@ module Gitlab
     end
   end
 end
+
+Gitlab::Checks::ChangesAccess.prepend_mod_with('Gitlab::Checks::ChangesAccess')