diff options
Diffstat (limited to 'spec/services/projects')
14 files changed, 112 insertions, 931 deletions
diff --git a/spec/services/projects/after_rename_service_spec.rb b/spec/services/projects/after_rename_service_spec.rb index 411ff5662d4..4b2569f6b2d 100644 --- a/spec/services/projects/after_rename_service_spec.rb +++ b/spec/services/projects/after_rename_service_spec.rb @@ -21,183 +21,88 @@ RSpec.describe Projects::AfterRenameService, feature_category: :groups_and_proje end describe '#execute' do - context 'using legacy storage' do - let(:project) { create(:project, :repository, :wiki_repo, :legacy_storage) } - let(:project_storage) { project.send(:storage) } - let(:gitlab_shell) { Gitlab::Shell.new } - - before do - # Project#gitlab_shell returns a new instance of Gitlab::Shell on every - # call. This makes testing a bit easier. - allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - - stub_application_setting(hashed_storage_enabled: false) - end - - it 'renames a repository' do - stub_container_registry_config(enabled: false) - - expect_any_instance_of(SystemHooksService) - .to receive(:execute_hooks_for) - .with(project, :rename) - - expect_any_instance_of(Gitlab::UploadsTransfer) - .to receive(:rename_project) - .with(path_before_rename, path_after_rename, project.namespace.full_path) - - expect(repo_before_rename).to exist - expect(wiki_repo_before_rename).to exist - - service_execute - - expect(repo_before_rename).not_to exist - expect(wiki_repo_before_rename).not_to exist - expect(repo_after_rename).to exist - expect(wiki_repo_after_rename).to exist - end - - context 'container registry with images' do - let(:container_repository) { create(:container_repository) } - - before do - stub_container_registry_config(enabled: true) - stub_container_registry_tags(repository: :any, tags: ['tag']) - project.container_repositories << container_repository - end - - it 'raises a RenameFailedError' do - expect { service_execute }.to raise_error(described_class::RenameFailedError) - end - end - - context 'attachments' do - before do - expect(project_storage).to receive(:rename_repo) { true } - end - - it 'moves uploads folder to new location' do - expect_any_instance_of(Gitlab::UploadsTransfer).to receive(:rename_project) - - service_execute - end - end - - it 'updates project full path in gitaly' do - service_execute - - expect(project.repository.full_path).to eq(project.full_path) - end - - it 'updates storage location' do - allow(project_storage).to receive(:rename_repo).and_return(true) - - service_execute + let(:project) { create(:project, :repository, skip_disk_validation: true) } + let(:gitlab_shell) { Gitlab::Shell.new } + let(:hash) { Digest::SHA2.hexdigest(project.id.to_s) } + let(:hashed_prefix) { File.join('@hashed', hash[0..1], hash[2..3]) } + let(:hashed_path) { File.join(hashed_prefix, hash) } + + before do + # Project#gitlab_shell returns a new instance of Gitlab::Shell on every + # call. This makes testing a bit easier. + allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) + + stub_application_setting(hashed_storage_enabled: true) + end - expect(project.project_repository).to have_attributes( - disk_path: project.disk_path, - shard_name: project.repository_storage - ) - end + it 'renames a repository' do + stub_container_registry_config(enabled: false) - context 'with hashed storage upgrade when renaming enabled' do - it 'calls HashedStorage::MigrationService with correct options' do - stub_application_setting(hashed_storage_enabled: true) + expect_any_instance_of(SystemHooksService) + .to receive(:execute_hooks_for) + .with(project, :rename) - expect_next_instance_of(::Projects::HashedStorage::MigrationService) do |service| - expect(service).to receive(:execute).and_return(true) - end + expect(project).to receive(:expire_caches_before_rename) - service_execute - end - end + service_execute end - context 'using hashed storage' do - let(:project) { create(:project, :repository, skip_disk_validation: true) } - let(:gitlab_shell) { Gitlab::Shell.new } - let(:hash) { Digest::SHA2.hexdigest(project.id.to_s) } - let(:hashed_prefix) { File.join('@hashed', hash[0..1], hash[2..3]) } - let(:hashed_path) { File.join(hashed_prefix, hash) } + context 'container registry with images' do + let(:container_repository) { create(:container_repository) } before do - # Project#gitlab_shell returns a new instance of Gitlab::Shell on every - # call. This makes testing a bit easier. - allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - - stub_application_setting(hashed_storage_enabled: true) + stub_container_registry_config(enabled: true) + stub_container_registry_tags(repository: :any, tags: ['tag']) + project.container_repositories << container_repository end - it 'renames a repository' do - stub_container_registry_config(enabled: false) - - expect(gitlab_shell).not_to receive(:mv_repository) - - expect_any_instance_of(SystemHooksService) - .to receive(:execute_hooks_for) - .with(project, :rename) - - expect(project).to receive(:expire_caches_before_rename) - - service_execute + it 'raises a RenameFailedError' do + expect { service_execute } + .to raise_error(described_class::RenameFailedError) end + end - context 'container registry with images' do - let(:container_repository) { create(:container_repository) } + context 'attachments' do + let(:uploader) { create(:upload, :issuable_upload, :with_file, model: project) } + let(:file_uploader) { build(:file_uploader, project: project) } + let(:legacy_storage_path) { File.join(file_uploader.root, legacy_storage.disk_path) } + let(:hashed_storage_path) { File.join(file_uploader.root, hashed_storage.disk_path) } - before do - stub_container_registry_config(enabled: true) - stub_container_registry_tags(repository: :any, tags: ['tag']) - project.container_repositories << container_repository - end + it 'keeps uploads folder location unchanged' do + expect_any_instance_of(Gitlab::UploadsTransfer).not_to receive(:rename_project) - it 'raises a RenameFailedError' do - expect { service_execute } - .to raise_error(described_class::RenameFailedError) - end + service_execute end - context 'attachments' do - let(:uploader) { create(:upload, :issuable_upload, :with_file, model: project) } - let(:file_uploader) { build(:file_uploader, project: project) } - let(:legacy_storage_path) { File.join(file_uploader.root, legacy_storage.disk_path) } - let(:hashed_storage_path) { File.join(file_uploader.root, hashed_storage.disk_path) } + context 'when not rolled out' do + let(:project) { create(:project, :repository, storage_version: 1, skip_disk_validation: true) } - it 'keeps uploads folder location unchanged' do - expect_any_instance_of(Gitlab::UploadsTransfer).not_to receive(:rename_project) + it 'moves attachments folder to hashed storage' do + expect(File.directory?(legacy_storage_path)).to be_truthy + expect(File.directory?(hashed_storage_path)).to be_falsey service_execute - end - - context 'when not rolled out' do - let(:project) { create(:project, :repository, storage_version: 1, skip_disk_validation: true) } - - it 'moves attachments folder to hashed storage' do - expect(File.directory?(legacy_storage_path)).to be_truthy - expect(File.directory?(hashed_storage_path)).to be_falsey + expect(project.reload.hashed_storage?(:attachments)).to be_truthy - service_execute - expect(project.reload.hashed_storage?(:attachments)).to be_truthy - - expect(File.directory?(legacy_storage_path)).to be_falsey - expect(File.directory?(hashed_storage_path)).to be_truthy - end + expect(File.directory?(legacy_storage_path)).to be_falsey + expect(File.directory?(hashed_storage_path)).to be_truthy end end + end - it 'updates project full path in gitaly' do - service_execute + it 'updates project full path in gitaly' do + service_execute - expect(project.repository.full_path).to eq(project.full_path) - end + expect(project.repository.full_path).to eq(project.full_path) + end - it 'updates storage location' do - service_execute + it 'updates storage location' do + service_execute - expect(project.project_repository).to have_attributes( - disk_path: project.disk_path, - shard_name: project.repository_storage - ) - end + expect(project.project_repository).to have_attributes( + disk_path: project.disk_path, + shard_name: project.repository_storage + ) end context 'EventStore' do diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb deleted file mode 100644 index e21d8b6fa83..00000000000 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ /dev/null @@ -1,152 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::HashedStorage::MigrateRepositoryService, feature_category: :groups_and_projects do - let(:gitlab_shell) { Gitlab::Shell.new } - let(:project) { create(:project, :legacy_storage, :repository, :wiki_repo, :design_repo) } - let(:legacy_storage) { Storage::LegacyProject.new(project) } - let(:hashed_storage) { Storage::Hashed.new(project) } - - subject(:service) { described_class.new(project: project, old_disk_path: project.disk_path) } - - describe '#execute' do - let(:old_disk_path) { legacy_storage.disk_path } - let(:new_disk_path) { hashed_storage.disk_path } - - before do - allow(service).to receive(:gitlab_shell) { gitlab_shell } - end - - context 'repository lock' do - it 'tries to lock the repository' do - expect(service).to receive(:try_to_set_repository_read_only!) - - service.execute - end - - it 'fails when a git operation is in progress' do - allow(project).to receive(:git_transfer_in_progress?) { true } - - expect { service.execute }.to raise_error(Projects::HashedStorage::RepositoryInUseError) - end - end - - context 'when repository doesnt exist on disk' do - let(:project) { create(:project, :legacy_storage) } - - it 'skips the disk change but increase the version' do - service.execute - - expect(project.hashed_storage?(:repository)).to be_truthy - end - end - - context 'when succeeds' do - it 'renames project, wiki and design repositories' do - service.execute - - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.design.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 all repositories' do - expect_move_repository(old_disk_path, new_disk_path) - expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") - expect_move_repository("#{old_disk_path}.design", "#{new_disk_path}.design") - - service.execute - end - - it 'writes project full path to gitaly' do - service.execute - - expect(project.repository.full_path).to eq project.full_path - end - end - - context 'when exception happens' do - it 'handles OpenSSL::Cipher::CipherError' do - expect(project).to receive(:ensure_runners_token).and_raise(OpenSSL::Cipher::CipherError) - - expect { service.execute }.not_to raise_exception - end - - it 'ensures rollback when OpenSSL::Cipher::CipherError' do - expect(project).to receive(:ensure_runners_token).and_raise(OpenSSL::Cipher::CipherError) - expect(service).to receive(:rollback_folder_move).and_call_original - - service.execute - project.reload - - expect(project.legacy_storage?).to be_truthy - expect(project.repository_read_only?).to be_falsey - end - - it 'handles Gitlab::Git::CommandError' do - expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError) - - expect { service.execute }.not_to raise_exception - end - - it 'ensures rollback when Gitlab::Git::CommandError' do - expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError) - expect(service).to receive(:rollback_folder_move).and_call_original - - service.execute - project.reload - - expect(project.legacy_storage?).to be_truthy - expect(project.repository_read_only?).to be_falsey - end - end - - context 'when one move fails' do - it 'rollsback repositories to original name' do - allow(service).to receive(:move_repository).and_call_original - allow(service).to receive(:move_repository).with(old_disk_path, new_disk_path).once { false } # will disable first move only - - expect(service).to receive(:rollback_folder_move).and_call_original - - service.execute - - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.design.git")).to be_falsey - expect(project.repository_read_only?).to be_falsey - end - - context 'when rollback fails' do - before do - gitlab_shell.mv_repository(project.repository_storage, old_disk_path, new_disk_path) - end - - it 'does not try to move nil repository over existing' do - expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage, old_disk_path, new_disk_path) - expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") - expect_move_repository("#{old_disk_path}.design", "#{new_disk_path}.design") - - service.execute - end - end - end - - it 'works even when project validation fails' do - allow(project).to receive(:valid?) { false } - - expect { service.execute }.to change { project.hashed_storage?(:repository) }.to(true) - end - - def expect_move_repository(from_name, to_name) - expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage, 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 ffbd5c2500a..d5b04688322 100644 --- a/spec/services/projects/hashed_storage/migration_service_spec.rb +++ b/spec/services/projects/hashed_storage/migration_service_spec.rb @@ -14,43 +14,6 @@ RSpec.describe Projects::HashedStorage::MigrationService, feature_category: :gro subject(:service) { described_class.new(project, project.full_path, logger: logger) } describe '#execute' do - context 'repository migration' do - let(:repository_service) do - Projects::HashedStorage::MigrateRepositoryService.new( - project: project, - old_disk_path: project.full_path, - logger: logger - ) - end - - it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do - expect(service).to receive(:migrate_repository_service).and_return(repository_service) - expect(repository_service).to receive(:execute) - - service.execute - end - - 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 - - it 'migrates legacy repositories to hashed storage' do - legacy_attachments_path = FileUploader.absolute_base_dir(project) - hashed_project = project.dup.tap { |p| p.id = project.id } - hashed_project.storage_version = ::Project::HASHED_STORAGE_FEATURES[:attachments] - hashed_attachments_path = FileUploader.absolute_base_dir(hashed_project) - - expect(logger).to receive(:info).with(/Repository moved from '#{project_legacy_path}' to '#{project_hashed_path}'/) - expect(logger).to receive(:info).with(/Repository moved from '#{wiki_legacy_path}' to '#{wiki_hashed_path}'/) - expect(logger).to receive(:info).with(/Project attachments moved from '#{legacy_attachments_path}' to '#{hashed_attachments_path}'/) - - expect { service.execute }.to change { project.storage_version }.from(nil).to(2) - end - end - context 'attachments migration' do let(:project) { create(:project, :empty_repo, :wiki_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) } @@ -62,13 +25,6 @@ RSpec.describe Projects::HashedStorage::MigrationService, feature_category: :gro ) end - it 'delegates migration to Projects::HashedStorage::MigrateRepositoryService' do - expect(service).to receive(:migrate_attachments_service).and_return(attachments_service) - expect(attachments_service).to receive(:execute) - - service.execute - end - 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) diff --git a/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb b/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb deleted file mode 100644 index d1a68503fa3..00000000000 --- a/spec/services/projects/hashed_storage/rollback_attachments_service_spec.rb +++ /dev/null @@ -1,106 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::HashedStorage::RollbackAttachmentsService, feature_category: :groups_and_projects do - subject(:service) { described_class.new(project: project, old_disk_path: project.disk_path, logger: nil) } - - let(:project) { create(:project, :repository, skip_disk_validation: true) } - let(:legacy_storage) { Storage::LegacyProject.new(project) } - let(:hashed_storage) { Storage::Hashed.new(project) } - - let!(:upload) { Upload.find_by(path: file_uploader.upload_path) } - let(:file_uploader) { build(:file_uploader, project: project) } - let(:old_disk_path) { File.join(base_path(hashed_storage), upload.path) } - let(:new_disk_path) { File.join(base_path(legacy_storage), upload.path) } - - describe '#execute' do - context 'when succeeds' do - it 'moves attachments to legacy storage layout' do - expect(File.file?(old_disk_path)).to be_truthy - expect(File.file?(new_disk_path)).to be_falsey - expect(File.exist?(base_path(hashed_storage))).to be_truthy - expect(File.exist?(base_path(legacy_storage))).to be_falsey - expect(FileUtils).to receive(:mv).with(base_path(hashed_storage), base_path(legacy_storage)).and_call_original - - service.execute - - expect(File.exist?(base_path(legacy_storage))).to be_truthy - expect(File.exist?(base_path(hashed_storage))).to be_falsey - expect(File.file?(old_disk_path)).to be_falsey - expect(File.file?(new_disk_path)).to be_truthy - end - - it 'returns true' do - expect(service.execute).to be_truthy - end - - it 'sets skipped to false' do - service.execute - - expect(service.skipped?).to be_falsey - end - end - - context 'when original folder does not exist anymore' do - before do - FileUtils.rm_rf(base_path(hashed_storage)) - end - - it 'skips moving folders and go to next' do - expect(FileUtils).not_to receive(:mv).with(base_path(hashed_storage), base_path(legacy_storage)) - - service.execute - - expect(File.exist?(base_path(legacy_storage))).to be_falsey - expect(File.file?(new_disk_path)).to be_falsey - end - - it 'returns true' do - expect(service.execute).to be_truthy - end - - it 'sets skipped to true' do - service.execute - - expect(service.skipped?).to be_truthy - end - end - - context 'when target folder already exists' do - before do - FileUtils.mkdir_p(base_path(legacy_storage)) - end - - it 'raises AttachmentCannotMoveError' do - expect(FileUtils).not_to receive(:mv).with(base_path(legacy_storage), base_path(hashed_storage)) - - expect { service.execute }.to raise_error(Projects::HashedStorage::AttachmentCannotMoveError) - end - end - - it 'works even when project validation fails' do - allow(project).to receive(:valid?) { false } - - expect { service.execute }.to change { project.hashed_storage?(:attachments) }.to(false) - end - end - - describe '#old_disk_path' do - it 'returns old disk_path for project' do - expect(service.old_disk_path).to eq(project.disk_path) - end - end - - describe '#new_disk_path' do - it 'returns new disk_path for project' do - service.execute - - expect(service.new_disk_path).to eq(project.full_path) - end - end - - def base_path(storage) - File.join(FileUploader.root, storage.disk_path) - end -end diff --git a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb deleted file mode 100644 index 1e5d4ae4d20..00000000000 --- a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb +++ /dev/null @@ -1,152 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis_shared_state, feature_category: :groups_and_projects do - let(:gitlab_shell) { Gitlab::Shell.new } - let(:project) { create(:project, :repository, :wiki_repo, :design_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) } - let(:legacy_storage) { Storage::LegacyProject.new(project) } - let(:hashed_storage) { Storage::Hashed.new(project) } - - subject(:service) { described_class.new(project: project, old_disk_path: project.disk_path) } - - describe '#execute' do - let(:old_disk_path) { hashed_storage.disk_path } - let(:new_disk_path) { legacy_storage.disk_path } - - before do - allow(service).to receive(:gitlab_shell) { gitlab_shell } - end - - context 'repository lock' do - it 'tries to lock the repository' do - expect(service).to receive(:try_to_set_repository_read_only!) - - service.execute - end - - it 'fails when a git operation is in progress' do - allow(project).to receive(:git_transfer_in_progress?) { true } - - expect { service.execute }.to raise_error(Projects::HashedStorage::RepositoryInUseError) - end - end - - context 'when repository doesnt exist on disk' do - let(:project) { create(:project) } - - it 'skips the disk change but decrease the version' do - service.execute - - expect(project.legacy_storage?).to be_truthy - end - end - - context 'when succeeds' do - it 'renames project, wiki and design repositories' do - service.execute - - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_truthy - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_truthy - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.design.git")).to be_truthy - end - - it 'updates project to be legacy and not read-only' do - service.execute - - expect(project.legacy_storage?).to be_truthy - expect(project.repository_read_only).to be_falsey - end - - it 'move operation is called for both repositories' do - expect_move_repository(old_disk_path, new_disk_path) - expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") - expect_move_repository("#{old_disk_path}.design", "#{new_disk_path}.design") - - service.execute - end - - it 'writes project full path to gitaly' do - service.execute - - expect(project.repository.full_path).to eq project.full_path - end - end - - context 'when exception happens' do - it 'handles OpenSSL::Cipher::CipherError' do - expect(project).to receive(:ensure_runners_token).and_raise(OpenSSL::Cipher::CipherError) - - expect { service.execute }.not_to raise_exception - end - - it 'ensures rollback when OpenSSL::Cipher::CipherError' do - expect(project).to receive(:ensure_runners_token).and_raise(OpenSSL::Cipher::CipherError) - expect(service).to receive(:rollback_folder_move).and_call_original - - service.execute - project.reload - - expect(project.hashed_storage?(:repository)).to be_truthy - expect(project.repository_read_only?).to be_falsey - end - - it 'handles Gitlab::Git::CommandError' do - expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError) - - expect { service.execute }.not_to raise_exception - end - - it 'ensures rollback when Gitlab::Git::CommandError' do - expect(project).to receive(:set_full_path).and_raise(Gitlab::Git::CommandError) - expect(service).to receive(:rollback_folder_move).and_call_original - - service.execute - project.reload - - expect(project.hashed_storage?(:repository)).to be_truthy - expect(project.repository_read_only?).to be_falsey - end - end - - context 'when one move fails' do - it 'rolls repositories back to original name' do - allow(service).to receive(:move_repository).and_call_original - allow(service).to receive(:move_repository).with(old_disk_path, new_disk_path).once { false } # will disable first move only - - expect(service).to receive(:rollback_folder_move).and_call_original - - service.execute - - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.git")).to be_falsey - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.wiki.git")).to be_falsey - expect(gitlab_shell.repository_exists?(project.repository_storage, "#{new_disk_path}.design.git")).to be_falsey - expect(project.repository_read_only?).to be_falsey - end - - context 'when rollback fails' do - before do - gitlab_shell.mv_repository(project.repository_storage, old_disk_path, new_disk_path) - end - - it 'does not try to move nil repository over existing' do - expect(gitlab_shell).not_to receive(:mv_repository).with(project.repository_storage, old_disk_path, new_disk_path) - expect_move_repository("#{old_disk_path}.wiki", "#{new_disk_path}.wiki") - expect_move_repository("#{old_disk_path}.design", "#{new_disk_path}.design") - - service.execute - end - end - end - - it 'works even when project validation fails' do - allow(project).to receive(:valid?) { false } - - expect { service.execute }.to change { project.legacy_storage? }.to(true) - end - - def expect_move_repository(from_name, to_name) - expect(gitlab_shell).to receive(:mv_repository).with(project.repository_storage, from_name, to_name).and_call_original - end - end -end diff --git a/spec/services/projects/hashed_storage/rollback_service_spec.rb b/spec/services/projects/hashed_storage/rollback_service_spec.rb deleted file mode 100644 index 088eb9d2734..00000000000 --- a/spec/services/projects/hashed_storage/rollback_service_spec.rb +++ /dev/null @@ -1,78 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::HashedStorage::RollbackService, feature_category: :groups_and_projects do - let(:project) { create(:project, :empty_repo, :wiki_repo) } - let(:logger) { double } - let!(:project_attachment) { build(:file_uploader, project: project) } - let(:project_hashed_path) { Storage::Hashed.new(project).disk_path } - let(:project_legacy_path) { Storage::LegacyProject.new(project).disk_path } - let(:wiki_hashed_path) { "#{project_hashed_path}.wiki" } - let(:wiki_legacy_path) { "#{project_legacy_path}.wiki" } - - subject(:service) { described_class.new(project, project.disk_path, logger: logger) } - - describe '#execute' do - context 'attachments rollback' do - let(:attachments_service_class) { Projects::HashedStorage::RollbackAttachmentsService } - let(:attachments_service) { attachments_service_class.new(project: project, old_disk_path: project.disk_path, logger: logger) } - - it 'delegates rollback to Projects::HashedStorage::RollbackAttachmentsService' do - expect(service).to receive(:rollback_attachments_service).and_return(attachments_service) - expect(attachments_service).to receive(:execute) - - service.execute - end - - it 'does not delegate rollback if repository is in legacy storage already' do - project.storage_version = nil - expect(attachments_service_class).not_to receive(:new) - - service.execute - end - - it 'rollbacks to legacy storage' do - hashed_attachments_path = FileUploader.absolute_base_dir(project) - legacy_project = project.dup - legacy_project.storage_version = nil - legacy_attachments_path = FileUploader.absolute_base_dir(legacy_project) - - expect(logger).to receive(:info).with(/Project attachments moved from '#{hashed_attachments_path}' to '#{legacy_attachments_path}'/) - - expect(logger).to receive(:info).with(/Repository moved from '#{project_hashed_path}' to '#{project_legacy_path}'/) - expect(logger).to receive(:info).with(/Repository moved from '#{wiki_hashed_path}' to '#{wiki_legacy_path}'/) - - expect { service.execute }.to change { project.storage_version }.from(2).to(nil) - end - end - - context 'repository rollback' do - let(:project) { create(:project, :empty_repo, :wiki_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) } - let(:repository_service_class) { Projects::HashedStorage::RollbackRepositoryService } - let(:repository_service) { repository_service_class.new(project: project, old_disk_path: project.disk_path, logger: logger) } - - it 'delegates rollback to RollbackRepositoryService' do - expect(service).to receive(:rollback_repository_service).and_return(repository_service) - expect(repository_service).to receive(:execute) - - service.execute - end - - it 'does not delegate rollback if repository is in legacy storage already' do - project.storage_version = nil - - expect(repository_service_class).not_to receive(:new) - - service.execute - end - - it 'rollbacks to legacy storage' do - expect(logger).to receive(:info).with(/Repository moved from '#{project_hashed_path}' to '#{project_legacy_path}'/) - expect(logger).to receive(:info).with(/Repository moved from '#{wiki_hashed_path}' to '#{wiki_legacy_path}'/) - - expect { service.execute }.to change { project.storage_version }.from(1).to(nil) - end - end - end -end diff --git a/spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb b/spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb deleted file mode 100644 index fab8cafd1a0..00000000000 --- a/spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb +++ /dev/null @@ -1,136 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::InProductMarketingCampaignEmailsService, feature_category: :experimentation_adoption do - describe '#execute' do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:campaign) { Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE } - - before do - allow(Notify) - .to receive(:build_ios_app_guide_email) - .and_return(instance_double(ActionMailer::MessageDelivery, deliver_later: true)) - end - - subject(:execute) do - described_class.new(project, campaign).execute - end - - context 'users can receive marketing emails' do - let(:maintainer) { create(:user) } - let(:developer) { create(:user) } - - before do - project.add_developer(developer) - project.add_maintainer(maintainer) - end - - it 'sends the email to all project members with access_level >= Developer', :aggregate_failures do - [project.owner, maintainer, developer].each do |user| - email = user.notification_email_or_default - - expect(Notify).to receive(:build_ios_app_guide_email).with(email) - end - - execute - end - - it 'records sent emails', :aggregate_failures do - expect { execute }.to change { Users::InProductMarketingEmail.count }.from(0).to(3) - - [project.owner, maintainer, developer].each do |user| - expect( - Users::InProductMarketingEmail.where( - user: user, - campaign: campaign - ) - ).to exist - end - end - - it 'tracks experiment :email_sent event', :experiment do - expect(experiment(:build_ios_app_guide_email)).to track(:email_sent) - .on_next_instance - .with_context(project: project) - - execute - end - end - - shared_examples 'does not send the email' do - it do - email = user.notification_email_or_default - expect(Notify).not_to receive(:build_ios_app_guide_email).with(email) - execute - end - end - - shared_examples 'does not create a record of the sent email' do - it do - expect( - Users::InProductMarketingEmail.where( - user: user, - campaign: campaign - ) - ).not_to exist - - execute - end - end - - context "when user can't receive marketing emails" do - before do - project.add_developer(user) - end - - context 'when user.can?(:receive_notifications) is false' do - it 'does not send the email' do - allow_next_found_instance_of(User) do |user| - allow(user).to receive(:can?).with(:receive_notifications) { false } - - email = user.notification_email_or_default - expect(Notify).not_to receive(:build_ios_app_guide_email).with(email) - - expect( - Users::InProductMarketingEmail.where( - user: user, - campaign: campaign - ) - ).not_to exist - end - - execute - end - end - end - - context 'when campaign email has already been sent to the user' do - before do - project.add_developer(user) - create(:in_product_marketing_email, :campaign, user: user, campaign: campaign) - end - - it_behaves_like 'does not send the email' - end - - context "when user is a reporter" do - before do - project.add_reporter(user) - end - - it_behaves_like 'does not send the email' - it_behaves_like 'does not create a record of the sent email' - end - - context "when user is a guest" do - before do - project.add_guest(user) - end - - it_behaves_like 'does not send the email' - it_behaves_like 'does not create a record of the sent email' - end - end -end diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb index 00c156ba538..ef2a89a15b1 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -94,7 +94,7 @@ RSpec.describe Projects::LfsPointers::LfsDownloadService, feature_category: :sou it 'streams the download' do expected_options = { headers: anything, stream_body: true } - expect(Gitlab::HTTP).to receive(:perform_request).with(Net::HTTP::Get, anything, expected_options) + expect(Gitlab::HTTP).to receive(:get).with(anything, expected_options) subject.execute end diff --git a/spec/services/projects/participants_service_spec.rb b/spec/services/projects/participants_service_spec.rb index b01e64439ec..692f43eb205 100644 --- a/spec/services/projects/participants_service_spec.rb +++ b/spec/services/projects/participants_service_spec.rb @@ -18,6 +18,14 @@ RSpec.describe Projects::ParticipantsService, feature_category: :groups_and_proj described_class.new(project, user).execute(noteable) end + it 'returns results in correct order' do + group = create(:group).tap { |g| g.add_owner(user) } + + expect(run_service.pluck(:username)).to eq([ + noteable.author.username, 'all', user.username, group.full_path + ]) + end + it 'includes `All Project and Group Members`' do expect(run_service).to include(a_hash_including({ username: "all", name: "All Project and Group Members" })) end @@ -104,6 +112,24 @@ RSpec.describe Projects::ParticipantsService, feature_category: :groups_and_proj expect(group_items.first[:avatar_url]).to eq("/gitlab/uploads/-/system/group/avatar/#{group.id}/dk.png") end end + + context 'with subgroups' do + let(:group_1) { create(:group, path: 'bb') } + let(:group_2) { create(:group, path: 'zz') } + let(:subgroup) { create(:group, path: 'aa', parent: group_1) } + + before do + group_1.add_owner(user) + group_2.add_owner(user) + subgroup.add_owner(user) + end + + it 'returns results ordered by full path' do + expect(group_items.pluck(:username)).to eq([ + group_1.full_path, subgroup.full_path, group_2.full_path + ]) + end + end end context 'when `disable_all_mention` FF is enabled' do diff --git a/spec/services/projects/record_target_platforms_service_spec.rb b/spec/services/projects/record_target_platforms_service_spec.rb index 7c6907c7a95..bf87b763341 100644 --- a/spec/services/projects/record_target_platforms_service_spec.rb +++ b/spec/services/projects/record_target_platforms_service_spec.rb @@ -51,52 +51,6 @@ RSpec.describe Projects::RecordTargetPlatformsService, '#execute', feature_categ end end end - - describe 'Build iOS guide email experiment' do - shared_examples 'tracks experiment assignment event' do - it 'tracks the assignment event', :experiment do - expect(experiment(:build_ios_app_guide_email)) - .to track(:assignment) - .with_context(project: project) - .on_next_instance - - execute - end - end - - context 'experiment candidate' do - before do - stub_experiments(build_ios_app_guide_email: :candidate) - end - - it 'executes a Projects::InProductMarketingCampaignEmailsService' do - service_double = instance_double(Projects::InProductMarketingCampaignEmailsService, execute: true) - - expect(Projects::InProductMarketingCampaignEmailsService) - .to receive(:new).with(project, Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE) - .and_return service_double - expect(service_double).to receive(:execute) - - execute - end - - it_behaves_like 'tracks experiment assignment event' - end - - context 'experiment control' do - before do - stub_experiments(build_ios_app_guide_email: :control) - end - - it 'does not execute a Projects::InProductMarketingCampaignEmailsService' do - expect(Projects::InProductMarketingCampaignEmailsService).not_to receive(:new) - - execute - end - - it_behaves_like 'tracks experiment assignment event' - end - end end context 'when project is not an XCode project' do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 1ddf6168c07..22264819e3b 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -425,28 +425,6 @@ RSpec.describe Projects::TransferService, feature_category: :groups_and_projects end end - context 'namespace which contains orphan repository with same projects path name' do - let(:raw_fake_repo) { Gitlab::Git::Repository.new('default', File.join(group.full_path, "#{project.path}.git"), nil, nil) } - - before do - group.add_owner(user) - - raw_fake_repo.create_repository - end - - after do - raw_fake_repo.remove - end - - it 'does not allow the project transfer' do - transfer_result = execute_transfer - - expect(transfer_result).to eq false - expect(project.namespace).to eq(user.namespace) - expect(project.errors[:new_namespace]).to include('Cannot move project') - end - end - context 'target namespace containing the same project name' do before do group.add_owner(user) diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index 6c767876d05..0ad7693a047 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -22,6 +22,20 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do subject(:service) { described_class.new(project, build) } + RSpec.shared_examples 'old deployments' do + it 'deactivates old deployments from the same project with the same path prefix', :freeze_time do + other_project = create(:pages_deployment) + same_project_other_path_prefix = create(:pages_deployment, project: project, path_prefix: 'other') + same_project = create(:pages_deployment, project: project) + + expect { expect(service.execute[:status]).to eq(:success) } + .to not_change { other_project.reload.deleted_at } + .and not_change { same_project_other_path_prefix.reload.deleted_at } + .and change { same_project.reload.deleted_at } + .from(nil).to(described_class::OLD_DEPLOYMENTS_DESTRUCTION_DELAY.from_now) + end + end + RSpec.shared_examples 'pages size limit is' do |size_limit| context "when size is below the limit" do before do @@ -36,6 +50,8 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do expect(deploy_status.description).not_to be_present expect(project.pages_metadatum).to be_deployed end + + it_behaves_like 'old deployments' end context "when size is above the limit" do @@ -95,6 +111,8 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do build.reload end + it_behaves_like 'old deployments' + it "doesn't delete artifacts after deploying" do expect(service.execute[:status]).to eq(:success) @@ -146,31 +164,6 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do expect(project.pages_metadatum.reload.pages_deployment).to eq(project.pages_deployments.last) end - context 'when there is an old pages deployment' do - let!(:old_deployment_from_another_project) { create(:pages_deployment) } - let!(:old_deployment) { create(:pages_deployment, project: project) } - - it 'schedules a destruction of older deployments' do - expect(DestroyPagesDeploymentsWorker).to( - receive(:perform_in).with( - described_class::OLD_DEPLOYMENTS_DESTRUCTION_DELAY, - project.id, - instance_of(Integer) - ) - ) - - service.execute - end - - it 'removes older deployments', :sidekiq_inline do - expect do - service.execute - end.not_to change { PagesDeployment.count } # it creates one and deletes one - - expect(PagesDeployment.find_by_id(old_deployment.id)).to be_nil - end - end - context 'when archive does not have pages directory' do let(:file) { empty_file } let(:metadata_filename) { empty_metadata_filename } @@ -291,20 +284,7 @@ RSpec.describe Projects::UpdatePagesService, feature_category: :pages do expect(deployment.ci_build_id).to eq(build.id) end - context 'when old deployment present' do - let!(:old_build) { create(:ci_build, name: 'pages', pipeline: old_pipeline, ref: 'HEAD') } - let!(:old_deployment) { create(:pages_deployment, ci_build: old_build, project: project) } - - before do - project.update_pages_deployment!(old_deployment) - end - - it 'deactivates old deployments' do - expect(service.execute[:status]).to eq(:success) - - expect(old_deployment.reload.deleted_at).not_to be_nil - end - end + it_behaves_like 'old deployments' context 'when newer deployment present' do before do diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index b30c1d30044..d173d23a1d6 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -103,6 +103,7 @@ RSpec.describe Projects::UpdateRepositoryStorageService, feature_category: :sour expect(project_repository_double).to receive(:replicate) .with(project.repository.raw) .and_raise(Gitlab::Git::CommandError) + expect(project_repository_double).to receive(:remove) expect do subject.execute @@ -140,10 +141,11 @@ RSpec.describe Projects::UpdateRepositoryStorageService, feature_category: :sour .with(project.repository.raw) expect(project_repository_double).to receive(:checksum) .and_return('not matching checksum') + expect(project_repository_double).to receive(:remove) expect do subject.execute - end.to raise_error(UpdateRepositoryStorageMethods::Error, /Failed to verify project repository checksum/) + end.to raise_error(Repositories::ReplicateService::Error, /Failed to verify project repository checksum/) expect(project).not_to be_repository_read_only expect(project.repository_storage).to eq('default') @@ -316,10 +318,14 @@ RSpec.describe Projects::UpdateRepositoryStorageService, feature_category: :sour context 'when object pool checksum does not match' do let(:new_object_pool_checksum) { 'not_match' } - it 'raises an error and does not change state' do + it 'raises an error and removes the new object pool repository' do + expect(object_pool_repository_double).to receive(:remove) + original_count = PoolRepository.count - expect { subject.execute }.to raise_error(UpdateRepositoryStorageMethods::Error) + expect do + subject.execute + end.to raise_error(Repositories::ReplicateService::Error, /Failed to verify object_pool repository/) project.reload diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 195cfe78b3f..7ab85d8253a 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -356,7 +356,7 @@ RSpec.describe Projects::UpdateService, feature_category: :groups_and_projects d context 'when changes project features' do # Using some sample features for testing. # Not using all the features because some of them must be enabled/disabled together - %w[issues wiki forking].each do |feature_name| + %w[issues wiki forking model_experiments].each do |feature_name| context "with feature_name:#{feature_name}" do let(:feature) { "#{feature_name}_access_level" } let(:params) do |