Merge branch '320775-remove-legacy-storage-pages-removal-logic' into 'master'

Remove legacy storage pages removal logic

See merge request gitlab-org/gitlab!69382

app/models/project.rb

@@ -120,7 +120,6 @@ class Project < ApplicationRecord
 
   use_fast_destroy :build_trace_chunks
 
-  after_destroy -> { run_after_commit { legacy_remove_pages } }
   after_destroy :remove_exports
 
   after_validation :check_pending_delete
@@ -1903,27 +1902,6 @@ class Project < ApplicationRecord
                .delete_all
   end
 
-  # TODO: remove this method https://gitlab.com/gitlab-org/gitlab/-/issues/320775
-  # rubocop: disable CodeReuse/ServiceClass
-  def legacy_remove_pages
-    return unless ::Settings.pages.local_store.enabled
-
-    # Projects with a missing namespace cannot have their pages removed
-    return unless namespace
-
-    mark_pages_as_not_deployed unless destroyed?
-
-    # 1. We rename pages to temporary directory
-    # 2. We wait 5 minutes, due to NFS caching
-    # 3. We asynchronously remove pages with force
-    temp_path = "#{path}.#{SecureRandom.hex}.deleted"
-
-    if Gitlab::PagesTransfer.new.rename_project(path, temp_path, namespace.full_path)
-      PagesWorker.perform_in(5.minutes, :remove, namespace.full_path, temp_path)
-    end
-  end
-  # rubocop: enable CodeReuse/ServiceClass
-
   def mark_pages_as_deployed(artifacts_archive: nil)
     ensure_pages_metadatum.update!(deployed: true, artifacts_archive: artifacts_archive)
   end

app/services/pages/delete_service.rb

@@ -12,9 +12,6 @@ module Pages
       PagesDomain.for_project(project).delete_all
 
       DestroyPagesDeploymentsWorker.perform_async(project.id)
-
-      # TODO: remove this call https://gitlab.com/gitlab-org/gitlab/-/issues/320775
-      PagesRemoveWorker.perform_async(project.id) if ::Settings.pages.local_store.enabled
     end
   end
 end

app/workers/pages_remove_worker.rb

 # frozen_string_literal: true
 
-# TODO: remove this worker https://gitlab.com/gitlab-org/gitlab/-/issues/320775
+# TODO: remove this worker https://gitlab.com/gitlab-org/gitlab/-/issues/340641
 class PagesRemoveWorker # rubocop:disable Scalability/IdempotentWorker
   include ApplicationWorker
 
@@ -11,9 +11,6 @@ class PagesRemoveWorker # rubocop:disable Scalability/IdempotentWorker
   loggable_arguments 0
 
   def perform(project_id)
-    project = Project.find_by_id(project_id)
-    return unless project
-
-    project.legacy_remove_pages
+    # no-op
   end
 end

spec/models/project_spec.rb

@@ -4556,44 +4556,6 @@ RSpec.describe Project, factory_default: :keep do
     end
   end
 
-  describe '#legacy_remove_pages' do
-    let(:project) { create(:project).tap { |project| project.mark_pages_as_deployed } }
-    let(:pages_metadatum) { project.pages_metadatum }
-    let(:namespace) { project.namespace }
-    let(:pages_path) { project.pages_path }
-
-    around do |example|
-      FileUtils.mkdir_p(pages_path)
-      begin
-        example.run
-      ensure
-        FileUtils.rm_rf(pages_path)
-      end
-    end
-
-    it 'removes the pages directory and marks the project as not having pages deployed' do
-      expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return(true)
-      expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, namespace.full_path, anything)
-
-      expect { project.legacy_remove_pages }.to change { pages_metadatum.reload.deployed }.from(true).to(false)
-    end
-
-    it 'does nothing if updates on legacy storage are disabled' do
-      allow(Settings.pages.local_store).to receive(:enabled).and_return(false)
-
-      expect(Gitlab::PagesTransfer).not_to receive(:new)
-      expect(PagesWorker).not_to receive(:perform_in)
-
-      project.legacy_remove_pages
-    end
-
-    it 'is run when the project is destroyed' do
-      expect(project).to receive(:legacy_remove_pages).and_call_original
-
-      expect { project.destroy! }.not_to raise_error
-    end
-  end
-
   describe '#remove_export' do
     let(:project) { create(:project, :with_export) }
 

spec/requests/api/pages/pages_spec.rb

@@ -36,12 +36,7 @@ RSpec.describe API::Pages do
         end
 
         it 'removes the pages' do
-          expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return true
-          expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything)
-
-          Sidekiq::Testing.inline! do
           delete api("/projects/#{project.id}/pages", admin )
-          end
 
           expect(project.reload.pages_metadatum.deployed?).to be(false)
         end

spec/services/pages/delete_service_spec.rb

@@ -12,27 +12,6 @@ RSpec.describe Pages::DeleteService do
     project.mark_pages_as_deployed
   end
 
-  it 'deletes published pages', :sidekiq_inline do
-    expect_next_instance_of(Gitlab::PagesTransfer) do |pages_transfer|
-      expect(pages_transfer).to receive(:rename_project).and_return true
-    end
-
-    expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything)
-
-    service.execute
-  end
-
-  it "doesn't remove anything from the legacy storage if local_store is disabled", :sidekiq_inline do
-    allow(Settings.pages.local_store).to receive(:enabled).and_return(false)
-
-    expect(project.pages_deployed?).to be(true)
-    expect(PagesWorker).not_to receive(:perform_in)
-
-    service.execute
-
-    expect(project.pages_deployed?).to be(false)
-  end
-
   it 'marks pages as not deployed' do
     expect do
       service.execute

spec/services/projects/update_pages_service_spec.rb

@@ -18,10 +18,6 @@ RSpec.describe Projects::UpdatePagesService do
 
   subject { described_class.new(project, build) }
 
-  before do
-    project.legacy_remove_pages
-  end
-
   context 'for new artifacts' do
     context "for a valid job" do
       let!(:artifacts_archive) { create(:ci_job_artifact, :correct_checksum, file: file, job: build) }

spec/workers/namespaceless_project_destroy_worker_spec.rb

@@ -48,12 +48,6 @@ RSpec.describe NamespacelessProjectDestroyWorker do
 
           subject.perform(project.id)
         end
-
-        it 'does not do anything in Project#legacy_remove_pages method' do
-          expect(Gitlab::PagesTransfer).not_to receive(:new)
-
-          subject.perform(project.id)
-        end
       end
 
       context 'project forked from another' do

spec/workers/pages_remove_worker_spec.rb

@@ -3,23 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe PagesRemoveWorker do
-  let(:project) { create(:project, path: "my.project")}
-  let!(:domain) { create(:pages_domain, project: project) }
-
-  subject { described_class.new.perform(project.id) }
-
-  before do
-    project.mark_pages_as_deployed
-  end
-
-  it 'deletes published pages' do
-    expect(project.pages_deployed?).to be(true)
-
-    expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return true
-    expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, project.namespace.full_path, anything)
-
-    subject
-
-    expect(project.reload.pages_deployed?).to be(false)
+  it 'does not raise error' do
+    expect do
+      described_class.new.perform(create(:project).id)
+    end.not_to raise_error
   end
 end