Merge branch 'hashed-storage-attachments-migration-path' into 'master'

Hashed storage attachments migration path

See merge request gitlab-org/gitlab-ce!15352

3 files changed, 157 insertions, 48 deletions

diff --git a/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb
new file mode 100644
index 00000000000..50e59954f73
--- /dev/null
+++ b/spec/services/projects/hashed_storage/migrate_attachments_service_spec.rb
@@ -0,0 +1,63 @@
+require 'spec_helper'
+
+describe Projects::HashedStorage::MigrateAttachmentsService do
+  subject(:service) { described_class.new(project) }
+  let(:project) { create(:project) }
+  let(:legacy_storage) { Storage::LegacyProject.new(project) }
+  let(:hashed_storage) { Storage::HashedProject.new(project) }
+
+  let!(:upload) { Upload.find_by(path: file_uploader.relative_path) }
+  let(:file_uploader) { build(:file_uploader, project: project) }
+  let(:old_path) { File.join(base_path(legacy_storage), upload.path) }
+  let(:new_path) { File.join(base_path(hashed_storage), upload.path) }
+
+  context '#execute' do
+    context 'when succeeds' do
+      it 'moves attachments to hashed storage layout' do
+        expect(File.file?(old_path)).to be_truthy
+        expect(File.file?(new_path)).to be_falsey
+        expect(File.exist?(base_path(legacy_storage))).to be_truthy
+        expect(File.exist?(base_path(hashed_storage))).to be_falsey
+        expect(FileUtils).to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)).and_call_original
+
+        service.execute
+
+        expect(File.exist?(base_path(hashed_storage))).to be_truthy
+        expect(File.exist?(base_path(legacy_storage))).to be_falsey
+        expect(File.file?(old_path)).to be_falsey
+        expect(File.file?(new_path)).to be_truthy
+      end
+    end
+
+    context 'when original folder does not exist anymore' do
+      before do
+        FileUtils.rm_rf(base_path(legacy_storage))
+      end
+
+      it 'skips moving folders and go to next' do
+        expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage))
+
+        service.execute
+
+        expect(File.exist?(base_path(hashed_storage))).to be_falsey
+        expect(File.file?(new_path)).to be_falsey
+      end
+    end
+
+    context 'when target folder already exists' do
+      before do
+        FileUtils.mkdir_p(base_path(hashed_storage))
+      end
+
+      it 'raises AttachmentMigrationError' do
+        expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage))
+
+        expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentMigrationError)
+      end
+    end
+  end
+
+  def base_path(storage)
+    FileUploader.dynamic_path_builder(storage.disk_path)
+  end
+end
diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb
new file mode 100644
index 00000000000..3a3e47fd9c0
--- /dev/null
+++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb
@@ -0,0 +1,76 @@
+require 'spec_helper'
+
+describe Projects::HashedStorage::MigrateRepositoryService do
+  let(:gitlab_shell) { Gitlab::Shell.new }
+  let(:project) { create(:project, :empty_repo, :wiki_repo) }
+  let(:service) { described_class.new(project) }
+  let(:legacy_storage) { Storage::LegacyProject.new(project) }
+  let(:hashed_storage) { Storage::HashedProject.new(project) }
+
+  describe '#execute' do
+    before do
+      allow(service).to receive(:gitlab_shell) { gitlab_shell }
+    end
+
+    context 'when succeeds' do
+      it 'renames project and wiki repositories' do
+        service.execute
+
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_truthy
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy
+      end
+
+      it 'updates project to be hashed and not read-only' do
+        service.execute
+
+        expect(project.hashed_storage?(:repository)).to be_truthy
+        expect(project.repository_read_only).to be_falsey
+      end
+
+      it 'move operation is called for both repositories' do
+        expect_move_repository(project.disk_path, hashed_storage.disk_path)
+        expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki")
+
+        service.execute
+      end
+    end
+
+    context 'when one move fails' do
+      it 'rollsback repositories to original name' do
+        from_name = project.disk_path
+        to_name = hashed_storage.disk_path
+        allow(service).to receive(:move_repository).and_call_original
+        allow(service).to receive(:move_repository).with(from_name, to_name).once { false } # will disable first move only
+
+        expect(service).to receive(:rollback_folder_move).and_call_original
+
+        service.execute
+
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_falsey
+        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_falsey
+        expect(project.repository_read_only?).to be_falsey
+      end
+
+      context 'when rollback fails' do
+        let(:from_name) { legacy_storage.disk_path }
+        let(:to_name) { hashed_storage.disk_path }
+
+        before do
+          hashed_storage.ensure_storage_path_exists
+          gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name)
+        end
+
+        it 'does not try to move nil repository over hashed' do
+          expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage_path, from_name, to_name)
+          expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki")
+
+          service.execute
+        end
+      end
+    end
+
+    def expect_move_repository(from_name, to_name)
+      expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage_path, from_name, to_name).and_call_original
+    end
+  end
+end
diff --git a/spec/services/projects/hashed_storage_migration_service_spec.rb b/spec/services/projects/hashed_storage_migration_service_spec.rb
index b71b47c59b6..466f0b5d7c2 100644
--- a/spec/services/projects/hashed_storage_migration_service_spec.rb
+++ b/spec/services/projects/hashed_storage_migration_service_spec.rb
@@ -1,74 +1,44 @@
 require 'spec_helper'
 
 describe Projects::HashedStorageMigrationService do
-  let(:gitlab_shell) { Gitlab::Shell.new }
   let(:project) { create(:project, :empty_repo, :wiki_repo) }
-  let(:service) { described_class.new(project) }
-  let(:legacy_storage) { Storage::LegacyProject.new(project) }
-  let(:hashed_storage) { Storage::HashedProject.new(project) }
+  subject(:service) { described_class.new(project) }
 
   describe '#execute' do
-    before do
-      allow(service).to receive(:gitlab_shell) { gitlab_shell }
-    end
-
-    context 'when succeeds' do
-      it 'renames project and wiki repositories' do
-        service.execute
+    context 'repository migration' do
+      let(:repository_service) { Projects::HashedStorage::MigrateRepositoryService.new(project, subject.logger) }
 
-        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_truthy
-        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_truthy
-      end
+      it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do
+        expect(Projects::HashedStorage::MigrateRepositoryService).to receive(:new).with(project, subject.logger).and_return(repository_service)
+        expect(repository_service).to receive(:execute)
 
-      it 'updates project to be hashed and not read-only' do
         service.execute
-
-        expect(project.hashed_storage?(:repository)).to be_truthy
-        expect(project.repository_read_only).to be_falsey
       end
 
-      it 'move operation is called for both repositories' do
-        expect_move_repository(project.disk_path, hashed_storage.disk_path)
-        expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki")
+      it 'does not delegate migration if repository is already migrated' do
+        project.storage_version = ::Project::LATEST_STORAGE_VERSION
+        expect(Projects::HashedStorage::MigrateRepositoryService).not_to receive(:new)
 
         service.execute
       end
     end
 
-    context 'when one move fails' do
-      it 'rollsback repositories to original name' do
-        from_name = project.disk_path
-        to_name = hashed_storage.disk_path
-        allow(service).to receive(:move_repository).and_call_original
-        allow(service).to receive(:move_repository).with(from_name, to_name).once { false } # will disable first move only
+    context 'attachments migration' do
+      let(:attachments_service) { Projects::HashedStorage::MigrateAttachmentsService.new(project, subject.logger) }
 
-        expect(service).to receive(:rollback_folder_move).and_call_original
+      it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do
+        expect(Projects::HashedStorage::MigrateAttachmentsService).to receive(:new).with(project, subject.logger).and_return(attachments_service)
+        expect(attachments_service).to receive(:execute)
 
         service.execute
-
-        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.git")).to be_falsey
-        expect(gitlab_shell.exists?(project.repository_storage_path, "#{hashed_storage.disk_path}.wiki.git")).to be_falsey
       end
 
-      context 'when rollback fails' do
-        before do
-          from_name = legacy_storage.disk_path
-          to_name = hashed_storage.disk_path
+      it 'does not delegate migration if attachments are already migrated' do
+        project.storage_version = ::Project::LATEST_STORAGE_VERSION
+        expect(Projects::HashedStorage::MigrateAttachmentsService).not_to receive(:new)
 
-          hashed_storage.ensure_storage_path_exists
-          gitlab_shell.mv_repository(project.repository_storage_path, from_name, to_name)
-        end
-
-        it 'does not try to move nil repository over hashed' do
-          expect_move_repository("#{project.disk_path}.wiki", "#{hashed_storage.disk_path}.wiki")
-
-          service.execute
-        end
+        service.execute
       end
     end
-
-    def expect_move_repository(from_name, to_name)
-      expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage_path, from_name, to_name).and_call_original
-    end
   end
 end