Fix hashed storage for attachments bugs

4 files changed, 122 insertions, 2 deletions

diff --git a/changelogs/unreleased/fix-import-uploads-hashed-storage.yml b/changelogs/unreleased/fix-import-uploads-hashed-storage.yml
new file mode 100644
index 00000000000..d43cabbfb8f
--- /dev/null
+++ b/changelogs/unreleased/fix-import-uploads-hashed-storage.yml
@@ -0,0 +1,5 @@
+---
+title: Fix hashed storage for Import/Export uploads
+merge_request: 15482
+author:
+type: fixed
diff --git a/lib/gitlab/import_export/uploads_saver.rb b/lib/gitlab/import_export/uploads_saver.rb
index f9ae5079d7c..627a487d577 100644
--- a/lib/gitlab/import_export/uploads_saver.rb
+++ b/lib/gitlab/import_export/uploads_saver.rb
@@ -24,8 +24,7 @@ module Gitlab
       end
 
       def uploads_path
-        # TODO: decide what to do with uploads. We will use UUIDs here too?
-        File.join(Rails.root.join('public/uploads'), @project.path_with_namespace)
+        FileUploader.dynamic_path_segment(@project)
       end
     end
   end
diff --git a/spec/lib/gitlab/import_export/uploads_restorer_spec.rb b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb
new file mode 100644
index 00000000000..63992ea8ab8
--- /dev/null
+++ b/spec/lib/gitlab/import_export/uploads_restorer_spec.rb
@@ -0,0 +1,55 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::UploadsRestorer do
+  describe 'bundle a project Git repo' do
+    let(:export_path) { "#{Dir.tmpdir}/uploads_saver_spec" }
+    let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) }
+    let(:uploads_path) { FileUploader.dynamic_path_segment(project) }
+
+    before do
+      allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+      FileUtils.mkdir_p(File.join(shared.export_path, 'uploads/random'))
+      FileUtils.touch(File.join(shared.export_path, 'uploads/random', "dummy.txt"))
+    end
+
+    after do
+      FileUtils.rm_rf(export_path)
+    end
+
+    describe 'legacy storage' do
+      let(:project) { create(:project) }
+
+      subject(:restorer) { described_class.new(project: project, shared: shared) }
+
+      it 'saves the uploads successfully' do
+        expect(restorer.restore).to be true
+      end
+
+      it 'copies the uploads to the project path' do
+        restorer.restore
+
+        uploads = Dir.glob(File.join(uploads_path, '**/*')).map { |file| File.basename(file) }
+
+        expect(uploads).to include('dummy.txt')
+      end
+    end
+
+    describe 'hashed storage' do
+      let(:project) { create(:project, :hashed) }
+
+      subject(:restorer) { described_class.new(project: project, shared: shared) }
+
+      it 'saves the uploads successfully' do
+        expect(restorer.restore).to be true
+      end
+
+      it 'copies the uploads to the project path' do
+        restorer.restore
+
+        uploads = Dir.glob(File.join(uploads_path, '**/*')).map { |file| File.basename(file) }
+
+        expect(uploads).to include('dummy.txt')
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/import_export/uploads_saver_spec.rb b/spec/lib/gitlab/import_export/uploads_saver_spec.rb
new file mode 100644
index 00000000000..e8948de1f3a
--- /dev/null
+++ b/spec/lib/gitlab/import_export/uploads_saver_spec.rb
@@ -0,0 +1,61 @@
+require 'spec_helper'
+
+describe Gitlab::ImportExport::UploadsSaver do
+  describe 'bundle a project Git repo' do
+    let(:export_path) { "#{Dir.tmpdir}/uploads_saver_spec" }
+    let(:file) { fixture_file_upload(Rails.root + 'spec/fixtures/banana_sample.gif', 'image/gif') }
+    let(:shared) { Gitlab::ImportExport::Shared.new(relative_path: project.full_path) }
+
+    before do
+      allow_any_instance_of(Gitlab::ImportExport).to receive(:storage_path).and_return(export_path)
+    end
+
+    after do
+      FileUtils.rm_rf(export_path)
+    end
+
+    describe 'legacy storage' do
+      let(:project) { create(:project) }
+
+      subject(:saver) { described_class.new(shared: shared, project: project) }
+
+      before do
+        UploadService.new(project, file, FileUploader).execute
+      end
+
+      it 'saves the uploads successfully' do
+        expect(saver.save).to be true
+      end
+
+      it 'copies the uploads to the export path' do
+        saver.save
+
+        uploads = Dir.glob(File.join(shared.export_path, 'uploads', '**/*')).map { |file| File.basename(file) }
+
+        expect(uploads).to include('banana_sample.gif')
+      end
+    end
+
+    describe 'hashed storage' do
+      let(:project) { create(:project, :hashed) }
+
+      subject(:saver) { described_class.new(shared: shared, project: project) }
+
+      before do
+        UploadService.new(project, file, FileUploader).execute
+      end
+
+      it 'saves the uploads successfully' do
+        expect(saver.save).to be true
+      end
+
+      it 'copies the uploads to the export path' do
+        saver.save
+
+        uploads = Dir.glob(File.join(shared.export_path, 'uploads', '**/*')).map { |file| File.basename(file) }
+
+        expect(uploads).to include('banana_sample.gif')
+      end
+    end
+  end
+end