Fix export removal for hashed-storage projects within a renamed or deleted namespace

4 files changed, 87 insertions, 25 deletions

diff --git a/spec/factories/projects.rb b/spec/factories/projects.rb
index 16d328a5bc2..20976977f21 100644
--- a/spec/factories/projects.rb
+++ b/spec/factories/projects.rb
@@ -93,6 +93,12 @@ FactoryBot.define do
       avatar { fixture_file_upload('spec/fixtures/dk.png') }
     end
 
+    trait :with_export do
+      after(:create) do |project, evaluator|
+        ProjectExportWorker.new.perform(project.creator.id, project.id)
+      end
+    end
+
     trait :broken_storage do
       after(:create) do |project|
         project.update_column(:repository_storage, 'broken')
diff --git a/spec/features/projects/import_export/namespace_export_file_spec.rb b/spec/features/projects/import_export/namespace_export_file_spec.rb
index e76bc6f1220..c1fccf4a40b 100644
--- a/spec/features/projects/import_export/namespace_export_file_spec.rb
+++ b/spec/features/projects/import_export/namespace_export_file_spec.rb
@@ -1,44 +1,37 @@
 require 'spec_helper'
 
 feature 'Import/Export - Namespace export file cleanup', :js do
-  let(:export_path) { "#{Dir.tmpdir}/import_file_spec" }
-  let(:config_hash) { YAML.load_file(Gitlab::ImportExport.config_file).deep_stringify_keys }
+  let(:export_path) { Dir.mktmpdir('namespace_export_file_spec') }
 
-  let(:project) { create(:project) }
-
-  background do
-    allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+  before do
+    allow(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
   end
 
   after do
     FileUtils.rm_rf(export_path, secure: true)
   end
 
-  context 'admin user' do
+  shared_examples_for 'handling project exports on namespace change' do
+    let!(:old_export_path) { project.export_path }
+
     before do
       sign_in(create(:admin))
+
+      setup_export_project
     end
 
     context 'moving the namespace' do
-      scenario 'removes the export file' do
-        setup_export_project
-
-        old_export_path = project.export_path.dup
-
+      it 'removes the export file' do
         expect(File).to exist(old_export_path)
 
-        project.namespace.update(path: 'new_path')
+        project.namespace.update!(path: build(:namespace).path)
 
         expect(File).not_to exist(old_export_path)
       end
     end
 
     context 'deleting the namespace' do
-      scenario 'removes the export file' do
-        setup_export_project
-
-        old_export_path = project.export_path.dup
-
+      it 'removes the export file' do
         expect(File).to exist(old_export_path)
 
         project.namespace.destroy
@@ -46,17 +39,29 @@ feature 'Import/Export - Namespace export file cleanup', :js do
         expect(File).not_to exist(old_export_path)
       end
     end
+  end
 
-    def setup_export_project
-      visit edit_project_path(project)
+  describe 'legacy storage' do
+    let(:project) { create(:project) }
 
-      expect(page).to have_content('Export project')
+    it_behaves_like 'handling project exports on namespace change'
+  end
+
+  describe 'hashed storage' do
+    let(:project) { create(:project, :hashed) }
 
-      find(:link, 'Export project').send_keys(:return)
+    it_behaves_like 'handling project exports on namespace change'
+  end
 
-      visit edit_project_path(project)
+  def setup_export_project
+    visit edit_project_path(project)
 
-      expect(page).to have_content('Download export')
-    end
+    expect(page).to have_content('Export project')
+
+    find(:link, 'Export project').send_keys(:return)
+
+    visit edit_project_path(project)
+
+    expect(page).to have_content('Download export')
   end
 end
diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb
index 6b7dbad128c..824cca66fb4 100644
--- a/spec/models/namespace_spec.rb
+++ b/spec/models/namespace_spec.rb
@@ -596,4 +596,24 @@ describe Namespace do
       end
     end
   end
+
+  describe '#remove_exports' do
+    let(:legacy_project) { create(:project, :with_export, namespace: namespace) }
+    let(:hashed_project) { create(:project, :with_export, :hashed, namespace: namespace) }
+    let(:export_path) { Dir.mktmpdir('namespace_remove_exports_spec') }
+    let(:legacy_export) { legacy_project.export_project_path }
+    let(:hashed_export) { hashed_project.export_project_path }
+
+    it 'removes exports for legacy and hashed projects' do
+      allow(Gitlab::ImportExport).to receive(:storage_path) { export_path }
+
+      expect(File.exist?(legacy_export)).to be_truthy
+      expect(File.exist?(hashed_export)).to be_truthy
+
+      namespace.remove_exports!
+
+      expect(File.exist?(legacy_export)).to be_falsy
+      expect(File.exist?(hashed_export)).to be_falsy
+    end
+  end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index df4c796c61e..da940571bc1 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -2503,6 +2503,37 @@ describe Project do
     end
   end
 
+  describe '#remove_exports' do
+    let(:project) { create(:project, :with_export) }
+
+    it 'removes the exports directory for the project' do
+      expect(File.exist?(project.export_path)).to be_truthy
+
+      allow(FileUtils).to receive(:rm_rf).and_call_original
+      expect(FileUtils).to receive(:rm_rf).with(project.export_path).and_call_original
+      project.remove_exports
+
+      expect(File.exist?(project.export_path)).to be_falsy
+    end
+
+    it 'is a no-op when there is no namespace' do
+      export_path = project.export_path
+      project.update_column(:namespace_id, nil)
+
+      expect(FileUtils).not_to receive(:rm_rf).with(export_path)
+
+      project.remove_exports
+
+      expect(File.exist?(export_path)).to be_truthy
+    end
+
+    it 'is run when the project is destroyed' do
+      expect(project).to receive(:remove_exports).and_call_original
+
+      project.destroy
+    end
+  end
+
   describe '#forks_count' do
     it 'returns the number of forks' do
       project = build(:project)