Make Gitlab::PagesTransfer possible to call async

When we rename a project, we call Gitlab::PagesTransfer to move any
pages directory on disk. This makes it convenient to use that class
through Sidekiq instead, so that Puma no longer needs access to Pages
NFS mounts.

app/workers/all_queues.yml

@@ -1540,6 +1540,14 @@
   :weight: 1
   :idempotent:
   :tags: []
+- :name: pages_transfer
+  :feature_category: :pages
+  :has_external_dependencies:
+  :urgency: :low
+  :resource_boundary: :unknown
+  :weight: 1
+  :idempotent:
+  :tags: []
 - :name: pages_update_configuration
   :feature_category: :pages
   :has_external_dependencies:

app/workers/pages_transfer_worker.rb

+# frozen_string_literal: true
+
+class PagesTransferWorker # rubocop:disable Scalability/IdempotentWorker
+  include ApplicationWorker
+
+  TransferFailedError = Class.new(StandardError)
+
+  feature_category :pages
+  loggable_arguments 0, 1
+
+  def perform(method, args)
+    return unless Gitlab::PagesTransfer::Async::METHODS.include?(method)
+
+    result = Gitlab::PagesTransfer.new.public_send(method, *args) # rubocop:disable GitlabSecurity/PublicSend
+
+    # If result isn't truthy, the move failed. Promote this to an
+    # exception so that it will be logged and retried appropriately
+    raise TransferFailedError unless result
+  end
+end

config/sidekiq_queues.yml

@@ -180,6 +180,8 @@
   - 1
 - - pages_remove
   - 1
+- - pages_transfer
+  - 1
 - - pages_update_configuration
   - 1
 - - personal_access_tokens

lib/gitlab/pages_transfer.rb

 # frozen_string_literal: true
 
+# To make a call happen in a new Sidekiq job, add `.async` before the call. For
+# instance:
+#
+#   PagesTransfer.new.async.move_namespace(...)
+#
 module Gitlab
   class PagesTransfer < ProjectTransfer
+    class Async
+      METHODS = %w[move_namespace move_project rename_project rename_namespace].freeze
+
+      METHODS.each do |meth|
+        define_method meth do |*args|
+          PagesTransferWorker.perform_async(meth, args)
+        end
+      end
+    end
+
+    def async
+      @async ||= Async.new
+    end
+
     def root_dir
       Gitlab.config.pages.path
     end

spec/lib/gitlab/pages_transfer_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::PagesTransfer do
+  describe '#async' do
+    let(:async) { subject.async }
+
+    context 'when receiving an allowed method' do
+      it 'schedules a PagesTransferWorker', :aggregate_failures do
+        described_class::Async::METHODS.each do |meth|
+          expect(PagesTransferWorker)
+            .to receive(:perform_async).with(meth, %w[foo bar])
+
+          async.public_send(meth, 'foo', 'bar')
+        end
+      end
+    end
+
+    context 'when receiving a private method' do
+      it 'raises NoMethodError' do
+        expect { async.move('foo', 'bar') }.to raise_error(NoMethodError)
+      end
+    end
+
+    context 'when receiving a non-existent method' do
+      it 'raises NoMethodError' do
+        expect { async.foo('bar') }.to raise_error(NoMethodError)
+      end
+    end
+  end
+end

spec/workers/pages_transfer_worker_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe PagesTransferWorker do
+  describe '#perform' do
+    RSpec.shared_examples 'moving a pages directory' do |parameter|
+      let!(:pages_path_before) { project.pages_path }
+      let(:config_path_before) { File.join(pages_path_before, 'config.json') }
+      let(:pages_path_after) { project.reload.pages_path }
+      let(:config_path_after) { File.join(pages_path_after, 'config.json') }
+
+      before do
+        FileUtils.mkdir_p(pages_path_before)
+        FileUtils.touch(config_path_before)
+
+        setup!
+      end
+
+      after do
+        FileUtils.remove_entry(pages_path_before, true)
+        FileUtils.remove_entry(pages_path_after, true)
+      end
+
+      it 'calls Gitlab::PagesTransfer to move the directory' do
+        expect_next_instance_of(Gitlab::PagesTransfer) do |service|
+          expect(service).to receive(meth).with(*args).and_call_original
+        end
+
+        subject.perform(meth, args)
+
+        expect(File.exist?(config_path_before)).to be(false)
+        expect(File.exist?(config_path_after)).to be(true)
+      end
+
+      it 'raises an exception if the service fails to move the directory' do
+        # Move the directory once, so it can't be moved again
+        subject.perform(meth, args)
+
+        expect { subject.perform(meth, args) }
+          .to raise_error(described_class::TransferFailedError)
+      end
+    end
+
+    describe 'when method is move_namespace' do
+      # Can't use let_it_be because we change the path
+      let(:group_1) { create(:group) }
+      let(:group_2) { create(:group) }
+      let(:subgroup) { create(:group, parent: group_1) }
+      let(:project) { create(:project, group: subgroup) }
+      let(:new_path) { "#{group_2.path}/#{subgroup.path}" }
+      let(:meth) { 'move_namespace' }
+
+      # Store the path before we change it
+      let!(:args) { [project.path, subgroup.full_path, new_path] }
+
+      def setup!
+        # We need to skip hooks, otherwise the directory will be moved
+        # via an ActiveRecord callback
+        subgroup.update_columns(parent_id: group_2.id)
+        subgroup.route.update!(path: new_path)
+      end
+
+      include_examples 'moving a pages directory'
+    end
+
+    describe 'when method is move_project' do
+      # Can't use let_it_be because we change the path
+      let(:group_1) { create(:group) }
+      let(:group_2) { create(:group) }
+      let(:project) { create(:project, group: group_1) }
+      let(:new_path) { group_2.path }
+      let(:meth) { 'move_project' }
+      let(:args) { [project.path, group_1.full_path, group_2.full_path] }
+
+      def setup!
+        project.update!(group: group_2)
+      end
+
+      include_examples 'moving a pages directory'
+    end
+
+    describe 'when method is rename_project' do
+      # Can't use let_it_be because we change the path
+      let(:project) { create(:project) }
+      let(:new_path) { project.path.succ }
+      let(:meth) { 'rename_project' }
+
+      # Store the path before we change it
+      let!(:args) { [project.path, new_path, project.namespace.full_path] }
+
+      def setup!
+        project.update!(path: new_path)
+      end
+
+      include_examples 'moving a pages directory'
+    end
+
+    describe 'when method is rename_namespace' do
+      # Can't use let_it_be because we change the path
+      let(:group) { create(:group) }
+      let(:project) { create(:project, group: group) }
+      let(:new_path) { project.namespace.full_path.succ }
+      let(:meth) { 'rename_namespace' }
+
+      # Store the path before we change it
+      let!(:args) { [project.namespace.full_path, new_path] }
+
+      def setup!
+        # We need to skip hooks, otherwise the directory will be moved
+        # via an ActiveRecord callback
+        group.update_columns(path: new_path)
+        group.route.update!(path: new_path)
+      end
+
+      include_examples 'moving a pages directory'
+    end
+
+    describe 'when method is not allowed' do
+      it 'does nothing' do
+        expect(Gitlab::PagesTransfer).not_to receive(:new)
+
+        subject.perform('object_id', [])
+      end
+    end
+  end
+end