Refactor PostReceive worker to limit merge conflicts

app/workers/ee/post_receive.rb

+module EE
+  # PostReceive EE mixin
+  #
+  # This module is intended to encapsulate EE-specific model logic
+  # and be prepended in the `PostReceive` worker
+  module PostReceive
+    extend ActiveSupport::Concern
+    extend ::Gitlab::CurrentSettings
+
+    private
+
+    def after_project_changes_hooks(post_received, user, refs, changes)
+      super
+
+      # Generate repository updated event on Geo event log when Geo is enabled
+      ::Geo::PushEventStore.new(post_received.project, refs: refs, changes: changes).create
+    end
+
+    def process_wiki_changes(post_received)
+      super
+
+      update_wiki_es_indexes(post_received)
+
+      if ::Gitlab::Geo.enabled?
+        # Create wiki repository updated event on Geo event log
+        ::Geo::PushEventStore.new(post_received.project, source: Geo::PushEvent::WIKI).create
+
+        # Triggers repository update on secondary nodes
+        ::Gitlab::Geo.notify_wiki_update(post_received.project)
+      end
+    end
+
+    def update_wiki_es_indexes(post_received)
+      return unless current_application_settings.elasticsearch_indexing?
+
+      post_received.project.wiki.index_blobs
+    end
+  end
+end

app/workers/post_receive.rb

 class PostReceive
   include Sidekiq::Worker
   include DedicatedSidekiqQueue
-  extend Gitlab::CurrentSettings
+  prepend EE::PostReceive
 
   def perform(project_identifier, identifier, changes)
     project, is_wiki = parse_project_identifier(project_identifier)
@@ -18,49 +18,18 @@ class PostReceive
     post_received = Gitlab::GitPostReceive.new(project, identifier, changes)
 
     if is_wiki
-      process_wiki_repository_update(post_received)
+      process_wiki_changes(post_received)
     else
       process_project_changes(post_received)
-      process_repository_update(post_received)
     end
   end
 
-  def process_repository_update(post_received)
+  private
+
+  def process_project_changes(post_received)
     changes = []
     refs = Set.new
 
-    post_received.changes_refs do |oldrev, newrev, ref|
-      @user ||= post_received.identify(newrev)
-
-      unless @user
-        log("Triggered hook for non-existing user \"#{post_received.identifier}\"")
-        return false
-      end
-
-      changes << Gitlab::DataBuilder::Repository.single_change(oldrev, newrev, ref)
-      refs << ref
-    end
-
-    # Generate repository update event on Geo event log when Geo is enabled
-    Geo::PushEventStore.new(post_received.project, refs: refs.to_a, changes: changes).create
-
-    hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, @user, changes, refs.to_a)
-    SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks)
-  end
-
-  def process_wiki_repository_update(post_received)
-    update_wiki_es_indexes(post_received)
-
-    if Gitlab::Geo.enabled?
-      # Generate wiki update event on Geo event log
-      Geo::PushEventStore.new(post_received.project, source: Geo::PushEvent::WIKI).create
-
-      # Triggers repository update on secondary nodes
-      Gitlab::Geo.notify_wiki_update(post_received.project)
-    end
-  end
-
-  def process_project_changes(post_received)
     post_received.changes_refs do |oldrev, newrev, ref|
       @user ||= post_received.identify(newrev)
 
@@ -74,16 +43,22 @@ class PostReceive
       elsif Gitlab::Git.branch_ref?(ref)
         GitPushService.new(post_received.project, @user, oldrev: oldrev, newrev: newrev, ref: ref).execute
       end
+
+      changes << Gitlab::DataBuilder::Repository.single_change(oldrev, newrev, ref)
+      refs << ref
     end
-  end
 
-  def update_wiki_es_indexes(post_received)
-    return unless current_application_settings.elasticsearch_indexing?
+    after_project_changes_hooks(post_received, @user, refs.to_a, changes)
+  end
 
-    post_received.project.wiki.index_blobs
+  def after_project_changes_hooks(post_received, user, refs, changes)
+    hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, user, changes, refs)
+    SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks)
   end
 
-  private
+  def process_wiki_changes(post_received)
+    # Nothing defined here yet.
+  end
 
   # To maintain backwards compatibility, we accept both gl_repository or
   # repository paths as project identifiers. Our plan is to migrate to

spec/workers/ee/post_receive_spec.rb

+require 'spec_helper'
+
+describe PostReceive do
+  let(:changes) { "123456 789012 refs/heads/tést\n654321 210987 refs/tags/tag" }
+  let(:wrongly_encoded_changes) { changes.encode("ISO-8859-1").force_encoding("UTF-8") }
+  let(:base64_changes) { Base64.encode64(wrongly_encoded_changes) }
+  let(:project_identifier) { "project-#{project.id}" }
+  let(:key) { create(:key, user: project.owner) }
+  let(:key_id) { key.shell_id }
+  let(:project) { create(:project, :repository) }
+
+  describe "#process_project_changes" do
+    before do
+      allow_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(project.owner)
+    end
+
+    context 'after project changes hooks' do
+      let(:fake_hook_data) { Hash.new(event_name: 'repository_update') }
+
+      before do
+        allow_any_instance_of(Gitlab::DataBuilder::Repository).to receive(:update).and_return(fake_hook_data)
+        # silence hooks so we can isolate
+        allow_any_instance_of(Key).to receive(:post_create_hook).and_return(true)
+        allow_any_instance_of(GitTagPushService).to receive(:execute).and_return(true)
+        allow_any_instance_of(GitPushService).to receive(:execute).and_return(true)
+      end
+
+      it 'calls Geo::PushEventStore' do
+        expect_any_instance_of(Geo::PushEventStore).to receive(:create)
+
+        described_class.new.perform(project_identifier, key_id, base64_changes)
+      end
+    end
+  end
+
+  describe '#process_wiki_changes' do
+    let(:project_identifier) { "#{pwd(project)}.wiki" }
+
+    it 'triggers Geo::PushEventStore when Geo is enabled' do
+      allow(Gitlab::Geo).to receive(:enabled?) { true }
+
+      expect(Geo::PushEventStore).to receive(:new).with(instance_of(Project), source: Geo::PushEvent::WIKI).and_call_original
+      expect_any_instance_of(Geo::PushEventStore).to receive(:create)
+
+      described_class.new.perform(project_identifier, key_id, base64_changes)
+    end
+
+    it 'triggers wiki index update when ElasticSearch is enabled' do
+      expect(Project).to receive(:find_by_full_path).with("#{project.full_path}.wiki").and_return(nil)
+      expect(Project).to receive(:find_by_full_path).with(project.full_path).and_return(project)
+      stub_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
+
+      expect_any_instance_of(ProjectWiki).to receive(:index_blobs)
+
+      described_class.new.perform(project_identifier, key_id, base64_changes)
+    end
+  end
+
+  def pwd(project)
+    File.join(Gitlab.config.repositories.storages.default['path'], project.path_with_namespace)
+  end
+end

spec/workers/post_receive_spec.rb

@@ -94,53 +94,23 @@ describe PostReceive do
         it { expect{ subject }.not_to change{ Ci::Pipeline.count } }
       end
     end
-  end
-
-  describe '#process_repository_update' do
-    let(:changes) {'123456 789012 refs/heads/tést'}
-    let(:fake_hook_data) do
-      { event_name: 'repository_update' }
-    end
-
-    before do
-      allow_any_instance_of(Gitlab::GitPostReceive).to receive(:identify).and_return(project.owner)
-      allow_any_instance_of(Gitlab::DataBuilder::Repository).to receive(:update).and_return(fake_hook_data)
-      # silence hooks so we can isolate
-      allow_any_instance_of(Key).to receive(:post_create_hook).and_return(true)
-      allow(subject).to receive(:process_project_changes).and_return(true)
-    end
-
-    it 'calls Geo::PushEventStore' do
-      expect_any_instance_of(Geo::PushEventStore).to receive(:create)
-
-      subject.perform(pwd(project), key_id, base64_changes)
-    end
 
-    it 'calls SystemHooksService' do
-      expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true)
+    context 'after project changes hooks' do
+      let(:changes) { '123456 789012 refs/heads/tést' }
+      let(:fake_hook_data) { Hash.new(event_name: 'repository_update') }
 
-      subject.perform(pwd(project), key_id, base64_changes)
-    end
-  end
-
-  describe '#process_wiki_update' do
-    it 'triggers Geo::PushEventStore when Geo is enabled' do
-      allow(Gitlab::Geo).to receive(:enabled?) { true }
-
-      expect(Geo::PushEventStore).to receive(:new).with(instance_of(Project), source: Geo::PushEvent::WIKI).and_call_original
-      expect_any_instance_of(Geo::PushEventStore).to receive(:create)
-
-      described_class.new.perform("#{pwd(project)}.wiki", key_id, base64_changes)
-    end
-
-    it 'triggers wiki index update when ElasticSearch is enabled' do
-      expect(Project).to receive(:find_by_full_path).with("#{project.full_path}.wiki").and_return(nil)
-      expect(Project).to receive(:find_by_full_path).with(project.full_path).and_return(project)
-      stub_application_setting(elasticsearch_search: true, elasticsearch_indexing: true)
+      before do
+        allow_any_instance_of(Gitlab::DataBuilder::Repository).to receive(:update).and_return(fake_hook_data)
+        # silence hooks so we can isolate
+        allow_any_instance_of(Key).to receive(:post_create_hook).and_return(true)
+        allow_any_instance_of(GitPushService).to receive(:execute).and_return(true)
+      end
 
-      expect_any_instance_of(ProjectWiki).to receive(:index_blobs)
+      it 'calls SystemHooksService' do
+        expect_any_instance_of(SystemHooksService).to receive(:execute_hooks).with(fake_hook_data, :repository_update_hooks).and_return(true)
 
-      described_class.new.perform("#{pwd(project)}.wiki", key_id, base64_changes)
+        described_class.new.perform(project_identifier, key_id, base64_changes)
+      end
     end
   end