diff options
Diffstat (limited to 'spec/services/projects')
14 files changed, 556 insertions, 145 deletions
diff --git a/spec/services/projects/alerting/notify_service_spec.rb b/spec/services/projects/alerting/notify_service_spec.rb index f08ecd397ec..b88f0ef5149 100644 --- a/spec/services/projects/alerting/notify_service_spec.rb +++ b/spec/services/projects/alerting/notify_service_spec.rb @@ -12,11 +12,16 @@ describe Projects::Alerting::NotifyService do shared_examples 'processes incident issues' do |amount| let(:create_incident_service) { spy } + let(:new_alert) { instance_double(AlertManagement::Alert, id: 503, persisted?: true) } it 'processes issues' do + expect(AlertManagement::Alert) + .to receive(:create) + .and_return(new_alert) + expect(IncidentManagement::ProcessAlertWorker) .to receive(:perform_async) - .with(project.id, kind_of(Hash)) + .with(project.id, kind_of(Hash), new_alert.id) .exactly(amount).times Sidekiq::Testing.inline! do @@ -59,15 +64,26 @@ describe Projects::Alerting::NotifyService do end end + shared_examples 'NotifyService does not create alert' do + it 'does not create alert' do + expect { subject }.not_to change(AlertManagement::Alert, :count) + end + end + describe '#execute' do let(:token) { 'invalid-token' } - let(:starts_at) { Time.now.change(usec: 0) } + let(:starts_at) { Time.current.change(usec: 0) } let(:service) { described_class.new(project, nil, payload) } let(:payload_raw) do { - 'title' => 'alert title', - 'start_time' => starts_at.rfc3339 - } + title: 'alert title', + start_time: starts_at.rfc3339, + severity: 'low', + monitoring_tool: 'GitLab RSpec', + service: 'GitLab Test Suite', + description: 'Very detailed description', + hosts: ['1.1.1.1', '2.2.2.2'] + }.with_indifferent_access end let(:payload) { ActionController::Parameters.new(payload_raw).permit! } @@ -88,6 +104,73 @@ describe Projects::Alerting::NotifyService do .and_return(incident_management_setting) end + context 'with valid payload' do + let(:last_alert_attributes) do + AlertManagement::Alert.last.attributes + .except('id', 'iid', 'created_at', 'updated_at') + .with_indifferent_access + end + + it 'creates AlertManagement::Alert' do + expect { subject }.to change(AlertManagement::Alert, :count).by(1) + end + + it 'created alert has all data properly assigned' do + subject + + expect(last_alert_attributes).to match( + project_id: project.id, + title: payload_raw.fetch(:title), + started_at: Time.zone.parse(payload_raw.fetch(:start_time)), + severity: payload_raw.fetch(:severity), + status: AlertManagement::Alert::STATUSES[:triggered], + events: 1, + hosts: payload_raw.fetch(:hosts), + payload: payload_raw.with_indifferent_access, + issue_id: nil, + description: payload_raw.fetch(:description), + monitoring_tool: payload_raw.fetch(:monitoring_tool), + service: payload_raw.fetch(:service), + fingerprint: nil, + ended_at: nil + ) + end + + context 'with a minimal payload' do + let(:payload_raw) do + { + title: 'alert title', + start_time: starts_at.rfc3339 + } + end + + it 'creates AlertManagement::Alert' do + expect { subject }.to change(AlertManagement::Alert, :count).by(1) + end + + it 'created alert has all data properly assigned' do + subject + + expect(last_alert_attributes).to match( + project_id: project.id, + title: payload_raw.fetch(:title), + started_at: Time.zone.parse(payload_raw.fetch(:start_time)), + severity: 'critical', + status: AlertManagement::Alert::STATUSES[:triggered], + events: 1, + hosts: [], + payload: payload_raw.with_indifferent_access, + issue_id: nil, + description: nil, + monitoring_tool: nil, + service: nil, + fingerprint: nil, + ended_at: nil + ) + end + end + end + it_behaves_like 'does not process incident issues' context 'issue enabled' do @@ -103,6 +186,7 @@ describe Projects::Alerting::NotifyService do end it_behaves_like 'does not process incident issues due to error', http_status: :bad_request + it_behaves_like 'NotifyService does not create alert' end end @@ -115,12 +199,14 @@ describe Projects::Alerting::NotifyService do context 'with invalid token' do it_behaves_like 'does not process incident issues due to error', http_status: :unauthorized + it_behaves_like 'NotifyService does not create alert' end context 'with deactivated Alerts Service' do let!(:alerts_service) { create(:alerts_service, :inactive, project: project) } it_behaves_like 'does not process incident issues due to error', http_status: :forbidden + it_behaves_like 'NotifyService does not create alert' end end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 1feea27eebc..e542f1e9108 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -489,6 +489,104 @@ describe Projects::CreateService, '#execute' do end end + it_behaves_like 'measurable service' do + before do + opts.merge!( + current_user: user, + path: 'foo' + ) + end + + let(:base_log_data) do + { + class: Projects::CreateService.name, + current_user: user.name, + project_full_path: "#{user.namespace.full_path}/#{opts[:path]}" + } + end + + after do + create_project(user, opts) + end + end + + context 'with specialized_project_authorization_workers' do + let_it_be(:other_user) { create(:user) } + let_it_be(:group) { create(:group) } + + let(:opts) do + { + name: 'GitLab', + namespace_id: group.id + } + end + + before do + group.add_maintainer(user) + group.add_developer(other_user) + end + + it 'updates authorization for current_user' do + expect(Users::RefreshAuthorizedProjectsService).to( + receive(:new).with(user).and_call_original + ) + + project = create_project(user, opts) + + expect( + Ability.allowed?(user, :read_project, project) + ).to be_truthy + end + + it 'schedules authorization update for users with access to group' do + expect(AuthorizedProjectsWorker).not_to( + receive(:bulk_perform_async) + ) + expect(AuthorizedProjectUpdate::ProjectCreateWorker).to( + receive(:perform_async).and_call_original + ) + expect(AuthorizedProjectUpdate::UserRefreshWithLowUrgencyWorker).to( + receive(:bulk_perform_in) + .with(1.hour, array_including([user.id], [other_user.id])) + .and_call_original + ) + + create_project(user, opts) + end + + context 'when feature is disabled' do + before do + stub_feature_flags(specialized_project_authorization_workers: false) + end + + it 'updates authorization for current_user' do + expect(Users::RefreshAuthorizedProjectsService).to( + receive(:new).with(user).and_call_original + ) + + project = create_project(user, opts) + + expect( + Ability.allowed?(user, :read_project, project) + ).to be_truthy + end + + it 'uses AuthorizedProjectsWorker' do + expect(AuthorizedProjectsWorker).to( + receive(:bulk_perform_async).with(array_including([user.id], [other_user.id])).and_call_original + ) + expect(AuthorizedProjectUpdate::ProjectCreateWorker).not_to( + receive(:perform_async) + ) + expect(AuthorizedProjectUpdate::UserRefreshWithLowUrgencyWorker).not_to( + receive(:bulk_perform_in) + ) + + create_project(user, opts) + end + end + end + def create_project(user, opts) Projects::CreateService.new(user, opts).execute end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index c8354f6ba4e..112a41c773b 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -320,7 +320,13 @@ describe Projects::ForkService do allow_any_instance_of(Gitlab::Git::Repository).to receive(:checksum) .and_return(::Gitlab::Git::BLANK_SHA) - Projects::UpdateRepositoryStorageService.new(project).execute('test_second_storage') + storage_move = create( + :project_repository_storage_move, + :scheduled, + project: project, + destination_storage_name: 'test_second_storage' + ) + Projects::UpdateRepositoryStorageService.new(storage_move).execute fork_after_move = fork_project(project) pool_repository_before_move = PoolRepository.joins(:shard) .find_by(source_project: project, shards: { name: 'default' }) diff --git a/spec/services/projects/hashed_storage/base_attachment_service_spec.rb b/spec/services/projects/hashed_storage/base_attachment_service_spec.rb index 34c37be6703..070dd5fc1b8 100644 --- a/spec/services/projects/hashed_storage/base_attachment_service_spec.rb +++ b/spec/services/projects/hashed_storage/base_attachment_service_spec.rb @@ -31,7 +31,7 @@ describe Projects::HashedStorage::BaseAttachmentService do expect(Dir.exist?(target_path)).to be_truthy Timecop.freeze do - suffix = Time.now.utc.to_i + suffix = Time.current.utc.to_i subject.send(:discard_path!, target_path) expected_renamed_path = "#{target_path}-#{suffix}" diff --git a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb index 71be335c11d..f1eaf8324e0 100644 --- a/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/migrate_repository_service_spec.rb @@ -6,7 +6,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do include GitHelpers let(:gitlab_shell) { Gitlab::Shell.new } - let(:project) { create(:project, :legacy_storage, :repository, :wiki_repo) } + 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) } @@ -45,11 +45,12 @@ describe Projects::HashedStorage::MigrateRepositoryService do end context 'when succeeds' do - it 'renames project and wiki repositories' 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 @@ -59,9 +60,10 @@ describe Projects::HashedStorage::MigrateRepositoryService do expect(project.repository_read_only).to be_falsey end - it 'move operation is called for both repositories' do + 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 @@ -86,6 +88,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do 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 @@ -97,6 +100,7 @@ describe Projects::HashedStorage::MigrateRepositoryService do 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 diff --git a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb index 6dcd2ff4555..1c0f446d9cf 100644 --- a/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb +++ b/spec/services/projects/hashed_storage/rollback_repository_service_spec.rb @@ -6,7 +6,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis include GitHelpers let(:gitlab_shell) { Gitlab::Shell.new } - let(:project) { create(:project, :repository, :wiki_repo, storage_version: ::Project::HASHED_STORAGE_FEATURES[:repository]) } + 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) } @@ -45,11 +45,12 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis end context 'when succeeds' do - it 'renames project and wiki repositories' 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 @@ -62,6 +63,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis 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 @@ -86,6 +88,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis 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 @@ -97,6 +100,7 @@ describe Projects::HashedStorage::RollbackRepositoryService, :clean_gitlab_redis 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 diff --git a/spec/services/projects/import_export/export_service_spec.rb b/spec/services/projects/import_export/export_service_spec.rb index e00507d1827..5f496cb1e56 100644 --- a/spec/services/projects/import_export/export_service_spec.rb +++ b/spec/services/projects/import_export/export_service_spec.rb @@ -7,9 +7,10 @@ describe Projects::ImportExport::ExportService do let!(:user) { create(:user) } let(:project) { create(:project) } let(:shared) { project.import_export_shared } - let(:service) { described_class.new(project, user) } let!(:after_export_strategy) { Gitlab::ImportExport::AfterExportStrategies::DownloadNotificationStrategy.new } + subject(:service) { described_class.new(project, user) } + before do project.add_maintainer(user) end @@ -46,8 +47,8 @@ describe Projects::ImportExport::ExportService do # in the corresponding EE spec. skip if Gitlab.ee? - # once for the normal repo, once for the wiki - expect(Gitlab::ImportExport::RepoSaver).to receive(:new).twice.and_call_original + # once for the normal repo, once for the wiki repo, and once for the design repo + expect(Gitlab::ImportExport::RepoSaver).to receive(:new).exactly(3).times.and_call_original service.execute end @@ -58,6 +59,12 @@ describe Projects::ImportExport::ExportService do service.execute end + it 'saves the design repo' do + expect(Gitlab::ImportExport::DesignRepoSaver).to receive(:new).and_call_original + + service.execute + end + it 'saves the lfs objects' do expect(Gitlab::ImportExport::LfsSaver).to receive(:new).and_call_original @@ -177,5 +184,20 @@ describe Projects::ImportExport::ExportService do expect { service.execute }.to raise_error(Gitlab::ImportExport::Error).with_message(expected_message) end end + + it_behaves_like 'measurable service' do + let(:base_log_data) do + { + class: described_class.name, + current_user: user.name, + project_full_path: project.full_path, + file_path: shared.export_path + } + end + + after do + service.execute(after_export_strategy) + end + end end end diff --git a/spec/services/projects/import_service_spec.rb b/spec/services/projects/import_service_spec.rb index af8118f9b11..ca6750b373d 100644 --- a/spec/services/projects/import_service_spec.rb +++ b/spec/services/projects/import_service_spec.rb @@ -264,13 +264,33 @@ describe Projects::ImportService do it 'fails with port 25' do project.import_url = "https://github.com:25/vim/vim.git" - result = described_class.new(project, user).execute + result = subject.execute expect(result[:status]).to eq :error expect(result[:message]).to include('Only allowed ports are 80, 443') end end + it_behaves_like 'measurable service' do + let(:base_log_data) do + { + class: described_class.name, + current_user: user.name, + project_full_path: project.full_path, + import_type: project.import_type, + file_path: project.import_source + } + end + + before do + project.import_type = 'github' + end + + after do + subject.execute + end + end + def stub_github_omniauth_provider provider = OpenStruct.new( 'name' => 'github', diff --git a/spec/services/projects/prometheus/alerts/create_events_service_spec.rb b/spec/services/projects/prometheus/alerts/create_events_service_spec.rb index 1d726db6ce3..35f23afd7a2 100644 --- a/spec/services/projects/prometheus/alerts/create_events_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/create_events_service_spec.rb @@ -50,7 +50,7 @@ describe Projects::Prometheus::Alerts::CreateEventsService do let(:events) { service.execute } context 'with a firing payload' do - let(:started_at) { truncate_to_second(Time.now) } + let(:started_at) { truncate_to_second(Time.current) } let(:firing_event) { alert_payload(status: 'firing', started_at: started_at) } let(:alerts_payload) { { 'alerts' => [firing_event] } } @@ -87,7 +87,7 @@ describe Projects::Prometheus::Alerts::CreateEventsService do end context 'with a resolved payload' do - let(:started_at) { truncate_to_second(Time.now) } + let(:started_at) { truncate_to_second(Time.current) } let(:ended_at) { started_at + 1 } let(:payload_key) { PrometheusAlertEvent.payload_key_for(alert.prometheus_metric_id, utc_rfc3339(started_at)) } let(:resolved_event) { alert_payload(status: 'resolved', started_at: started_at, ended_at: ended_at) } @@ -285,7 +285,7 @@ describe Projects::Prometheus::Alerts::CreateEventsService do private - def alert_payload(status: 'firing', started_at: Time.now, ended_at: Time.now, gitlab_alert_id: alert.prometheus_metric_id, title: nil, environment: nil) + def alert_payload(status: 'firing', started_at: Time.current, ended_at: Time.current, gitlab_alert_id: alert.prometheus_metric_id, title: nil, environment: nil) payload = {} payload['status'] = status if status diff --git a/spec/services/projects/prometheus/alerts/notify_service_spec.rb b/spec/services/projects/prometheus/alerts/notify_service_spec.rb index dce96dda1e3..009543f9016 100644 --- a/spec/services/projects/prometheus/alerts/notify_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/notify_service_spec.rb @@ -217,6 +217,32 @@ describe Projects::Prometheus::Alerts::NotifyService do end end + context 'process Alert Management alerts' do + let(:process_service) { instance_double(AlertManagement::ProcessPrometheusAlertService) } + + before do + create(:prometheus_service, project: project) + create(:project_alerting_setting, project: project, token: token) + end + + context 'with multiple firing alerts and resolving alerts' do + let(:payload_raw) do + payload_for(firing: [alert_firing, alert_firing], resolved: [alert_resolved]) + end + + it 'processes Prometheus alerts' do + expect(AlertManagement::ProcessPrometheusAlertService) + .to receive(:new) + .with(project, nil, kind_of(Hash)) + .exactly(3).times + .and_return(process_service) + expect(process_service).to receive(:execute).exactly(3).times + + subject + end + end + end + context 'process incident issues' do before do create(:prometheus_service, project: project) @@ -286,6 +312,13 @@ describe Projects::Prometheus::Alerts::NotifyService do it_behaves_like 'no notifications', http_status: :bad_request + it 'does not process Prometheus alerts' do + expect(AlertManagement::ProcessPrometheusAlertService) + .not_to receive(:new) + + subject + end + it 'does not process issues' do expect(IncidentManagement::ProcessPrometheusAlertWorker) .not_to receive(:perform_async) diff --git a/spec/services/projects/propagate_service_template_spec.rb b/spec/services/projects/propagate_service_template_spec.rb index 2c3effec617..7188ac5f733 100644 --- a/spec/services/projects/propagate_service_template_spec.rb +++ b/spec/services/projects/propagate_service_template_spec.rb @@ -8,16 +8,19 @@ describe Projects::PropagateServiceTemplate do PushoverService.create( template: true, active: true, + push_events: false, properties: { device: 'MyDevice', sound: 'mic', priority: 4, user_key: 'asdf', api_key: '123456789' - }) + } + ) end let!(:project) { create(:project) } + let(:excluded_attributes) { %w[id project_id template created_at updated_at title description] } it 'creates services for projects' do expect(project.pushover_service).to be_nil @@ -35,7 +38,7 @@ describe Projects::PropagateServiceTemplate do properties: { bamboo_url: 'http://gitlab.com', username: 'mic', - password: "password", + password: 'password', build_key: 'build' } ) @@ -54,7 +57,7 @@ describe Projects::PropagateServiceTemplate do properties: { bamboo_url: 'http://gitlab.com', username: 'mic', - password: "password", + password: 'password', build_key: 'build' } ) @@ -70,6 +73,33 @@ describe Projects::PropagateServiceTemplate do described_class.propagate(service_template) expect(project.pushover_service.properties).to eq(service_template.properties) + + expect(project.pushover_service.attributes.except(*excluded_attributes)) + .to eq(service_template.attributes.except(*excluded_attributes)) + end + + context 'service with data fields' do + let(:service_template) do + JiraService.create!( + template: true, + active: true, + push_events: false, + url: 'http://jira.instance.com', + username: 'user', + password: 'secret' + ) + end + + it 'creates the service containing the template attributes' do + described_class.propagate(service_template) + + expect(project.jira_service.attributes.except(*excluded_attributes)) + .to eq(service_template.attributes.except(*excluded_attributes)) + + excluded_attributes = %w[id service_id created_at updated_at] + expect(project.jira_service.data_fields.attributes.except(*excluded_attributes)) + .to eq(service_template.data_fields.attributes.except(*excluded_attributes)) + end end describe 'bulk update', :use_sql_query_cache do diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index f17ddb22d22..0e2431c0e44 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -9,18 +9,26 @@ describe Projects::TransferService do let(:group) { create(:group) } let(:project) { create(:project, :repository, :legacy_storage, namespace: user.namespace) } + subject(:execute_transfer) { described_class.new(project, user).execute(group) } + context 'namespace -> namespace' do before do - allow_any_instance_of(Gitlab::UploadsTransfer) - .to receive(:move_project).and_return(true) - allow_any_instance_of(Gitlab::PagesTransfer) - .to receive(:move_project).and_return(true) + allow_next_instance_of(Gitlab::UploadsTransfer) do |service| + allow(service).to receive(:move_project).and_return(true) + end + allow_next_instance_of(Gitlab::PagesTransfer) do |service| + allow(service).to receive(:move_project).and_return(true) + end + group.add_owner(user) - @result = transfer_project(project, user, group) end - it { expect(@result).to be_truthy } - it { expect(project.namespace).to eq(group) } + it 'updates the namespace' do + transfer_result = execute_transfer + + expect(transfer_result).to be_truthy + expect(project.namespace).to eq(group) + end end context 'when transfer succeeds' do @@ -31,26 +39,29 @@ describe Projects::TransferService do it 'sends notifications' do expect_any_instance_of(NotificationService).to receive(:project_was_moved) - transfer_project(project, user, group) + execute_transfer end it 'invalidates the user\'s personal_project_count cache' do expect(user).to receive(:invalidate_personal_projects_count) - transfer_project(project, user, group) + execute_transfer end it 'executes system hooks' do - transfer_project(project, user, group) do |service| + expect_next_instance_of(described_class) do |service| expect(service).to receive(:execute_system_hooks) end + + execute_transfer end it 'moves the disk path', :aggregate_failures do old_path = project.repository.disk_path old_full_path = project.repository.full_path - transfer_project(project, user, group) + execute_transfer + project.reload_repository! expect(project.repository.disk_path).not_to eq(old_path) @@ -60,13 +71,13 @@ describe Projects::TransferService do end it 'updates project full path in .git/config' do - transfer_project(project, user, group) + execute_transfer expect(rugged_config['gitlab.fullpath']).to eq "#{group.full_path}/#{project.path}" end it 'updates storage location' do - transfer_project(project, user, group) + execute_transfer expect(project.project_repository).to have_attributes( disk_path: "#{group.full_path}/#{project.path}", @@ -80,7 +91,7 @@ describe Projects::TransferService do def attempt_project_transfer(&block) expect do - transfer_project(project, user, group, &block) + execute_transfer end.to raise_error(ActiveRecord::ActiveRecordError) end @@ -138,13 +149,15 @@ describe Projects::TransferService do end context 'namespace -> no namespace' do - before do - @result = transfer_project(project, user, nil) - end + let(:group) { nil } + + it 'does not allow the project transfer' do + transfer_result = execute_transfer - it { expect(@result).to eq false } - it { expect(project.namespace).to eq(user.namespace) } - it { expect(project.errors.messages[:new_namespace].first).to eq 'Please select a new namespace for your project.' } + expect(transfer_result).to eq false + expect(project.namespace).to eq(user.namespace) + expect(project.errors.messages[:new_namespace].first).to eq 'Please select a new namespace for your project.' + end end context 'disallow transferring of project with tags' do @@ -156,18 +169,18 @@ describe Projects::TransferService do project.container_repositories << container_repository end - subject { transfer_project(project, user, group) } - - it { is_expected.to be_falsey } + it 'does not allow the project transfer' do + expect(execute_transfer).to eq false + end end context 'namespace -> not allowed namespace' do - before do - @result = transfer_project(project, user, group) - end + it 'does not allow the project transfer' do + transfer_result = execute_transfer - it { expect(@result).to eq false } - it { expect(project.namespace).to eq(user.namespace) } + expect(transfer_result).to eq false + expect(project.namespace).to eq(user.namespace) + end end context 'namespace which contains orphan repository with same projects path name' do @@ -177,99 +190,94 @@ describe Projects::TransferService do group.add_owner(user) TestEnv.create_bare_repository(fake_repo_path) - - @result = transfer_project(project, user, group) end after do FileUtils.rm_rf(fake_repo_path) end - it { expect(@result).to eq false } - it { expect(project.namespace).to eq(user.namespace) } - it { expect(project.errors[:new_namespace]).to include('Cannot move project') } + 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) - project.update(name: 'new_name') + create(:project, name: project.name, group: group, path: 'other') + end - create(:project, name: 'new_name', group: group, path: 'other') + it 'does not allow the project transfer' do + transfer_result = execute_transfer - @result = transfer_project(project, user, group) + expect(transfer_result).to eq false + expect(project.namespace).to eq(user.namespace) + expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists') end - - it { expect(@result).to eq false } - it { expect(project.namespace).to eq(user.namespace) } - it { expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists') } end context 'target namespace containing the same project path' do before do group.add_owner(user) - create(:project, name: 'other-name', path: project.path, group: group) - - @result = transfer_project(project, user, group) end - it { expect(@result).to eq false } - it { expect(project.namespace).to eq(user.namespace) } - it { expect(project.errors[:new_namespace]).to include('Project with same name or path in target namespace already exists') } + 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('Project with same name or path in target namespace already exists') + end end context 'target namespace allows developers to create projects' do let(:group) { create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS) } context 'the user is a member of the target namespace with developer permissions' do - subject(:transfer_project_result) { transfer_project(project, user, group) } - before do group.add_developer(user) end it 'does not allow project transfer to the target namespace' do - expect(transfer_project_result).to eq false + transfer_result = execute_transfer + + expect(transfer_result).to eq false expect(project.namespace).to eq(user.namespace) expect(project.errors[:new_namespace]).to include('Transfer failed, please contact an admin.') end end end - def transfer_project(project, user, new_namespace) - service = Projects::TransferService.new(project, user) - - yield(service) if block_given? - - service.execute(new_namespace) - end - context 'visibility level' do - let(:internal_group) { create(:group, :internal) } + let(:group) { create(:group, :internal) } before do - internal_group.add_owner(user) + group.add_owner(user) end context 'when namespace visibility level < project visibility level' do - let(:public_project) { create(:project, :public, :repository, namespace: user.namespace) } + let(:project) { create(:project, :public, :repository, namespace: user.namespace) } before do - transfer_project(public_project, user, internal_group) + execute_transfer end - it { expect(public_project.visibility_level).to eq(internal_group.visibility_level) } + it { expect(project.visibility_level).to eq(group.visibility_level) } end context 'when namespace visibility level > project visibility level' do - let(:private_project) { create(:project, :private, :repository, namespace: user.namespace) } + let(:project) { create(:project, :private, :repository, namespace: user.namespace) } before do - transfer_project(private_project, user, internal_group) + execute_transfer end - it { expect(private_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) } + it { expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) } end end @@ -277,9 +285,11 @@ describe Projects::TransferService do it 'delegates transfer to Labels::TransferService' do group.add_owner(user) - expect_any_instance_of(Labels::TransferService).to receive(:execute).once.and_call_original + expect_next_instance_of(Labels::TransferService, user, project.group, project) do |labels_transfer_service| + expect(labels_transfer_service).to receive(:execute).once.and_call_original + end - transfer_project(project, user, group) + execute_transfer end end @@ -287,49 +297,52 @@ describe Projects::TransferService do it 'delegates transfer to Milestones::TransferService' do group.add_owner(user) - expect(Milestones::TransferService).to receive(:new).with(user, project.group, project).and_call_original - expect_any_instance_of(Milestones::TransferService).to receive(:execute).once + expect_next_instance_of(Milestones::TransferService, user, project.group, project) do |milestones_transfer_service| + expect(milestones_transfer_service).to receive(:execute).once.and_call_original + end - transfer_project(project, user, group) + execute_transfer end end context 'when hashed storage in use' do - let!(:hashed_project) { create(:project, :repository, namespace: user.namespace) } - let!(:old_disk_path) { hashed_project.repository.disk_path } + let!(:project) { create(:project, :repository, namespace: user.namespace) } + let!(:old_disk_path) { project.repository.disk_path } before do group.add_owner(user) end it 'does not move the disk path', :aggregate_failures do - new_full_path = "#{group.full_path}/#{hashed_project.path}" + new_full_path = "#{group.full_path}/#{project.path}" - transfer_project(hashed_project, user, group) - hashed_project.reload_repository! + execute_transfer - expect(hashed_project.repository).to have_attributes( + project.reload_repository! + + expect(project.repository).to have_attributes( disk_path: old_disk_path, full_path: new_full_path ) - expect(hashed_project.disk_path).to eq(old_disk_path) + expect(project.disk_path).to eq(old_disk_path) end it 'does not move the disk path when the transfer fails', :aggregate_failures do - old_full_path = hashed_project.full_path + old_full_path = project.full_path expect_next_instance_of(described_class) do |service| allow(service).to receive(:execute_system_hooks).and_raise('foo') end - expect { transfer_project(hashed_project, user, group) }.to raise_error('foo') - hashed_project.reload_repository! + expect { execute_transfer }.to raise_error('foo') + + project.reload_repository! - expect(hashed_project.repository).to have_attributes( + expect(project.repository).to have_attributes( disk_path: old_disk_path, full_path: old_full_path ) - expect(hashed_project.disk_path).to eq(old_disk_path) + expect(project.disk_path).to eq(old_disk_path) end end @@ -344,18 +357,102 @@ describe Projects::TransferService do end it 'refreshes the permissions of the old and new namespace' do - transfer_project(project, owner, group) + execute_transfer expect(group_member.authorized_projects).to include(project) expect(owner.authorized_projects).to include(project) end it 'only schedules a single job for every user' do - expect(UserProjectAccessChangedService).to receive(:new) - .with([owner.id, group_member.id]) - .and_call_original + expect_next_instance_of(UserProjectAccessChangedService, [owner.id, group_member.id]) do |service| + expect(service).to receive(:execute).once.and_call_original + end + + execute_transfer + end + end + + describe 'transferring a design repository' do + subject { described_class.new(project, user) } + + before do + group.add_owner(user) + end + + def design_repository + project.design_repository + end + + it 'does not create a design repository' do + expect(subject.execute(group)).to be true + + project.clear_memoization(:design_repository) + + expect(design_repository.exists?).to be false + end - transfer_project(project, owner, group) + describe 'when the project has a design repository' do + let(:project_repo_path) { "#{project.path}#{::Gitlab::GlRepository::DESIGN.path_suffix}" } + let(:old_full_path) { "#{user.namespace.full_path}/#{project_repo_path}" } + let(:new_full_path) { "#{group.full_path}/#{project_repo_path}" } + + context 'with legacy storage' do + let(:project) { create(:project, :repository, :legacy_storage, :design_repo, namespace: user.namespace) } + + it 'moves the repository' do + expect(subject.execute(group)).to be true + + project.clear_memoization(:design_repository) + + expect(design_repository).to have_attributes( + disk_path: new_full_path, + full_path: new_full_path + ) + end + + it 'does not move the repository when an error occurs', :aggregate_failures do + allow(subject).to receive(:execute_system_hooks).and_raise('foo') + expect { subject.execute(group) }.to raise_error('foo') + + project.clear_memoization(:design_repository) + + expect(design_repository).to have_attributes( + disk_path: old_full_path, + full_path: old_full_path + ) + end + end + + context 'with hashed storage' do + let(:project) { create(:project, :repository, namespace: user.namespace) } + + it 'does not move the repository' do + old_disk_path = design_repository.disk_path + + expect(subject.execute(group)).to be true + + project.clear_memoization(:design_repository) + + expect(design_repository).to have_attributes( + disk_path: old_disk_path, + full_path: new_full_path + ) + end + + it 'does not move the repository when an error occurs' do + old_disk_path = design_repository.disk_path + + allow(subject).to receive(:execute_system_hooks).and_raise('foo') + expect { subject.execute(group) }.to raise_error('foo') + + project.clear_memoization(:design_repository) + + expect(design_repository).to have_attributes( + disk_path: old_disk_path, + full_path: old_full_path + ) + end + end end end diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index 4396ccab584..38c2dc0780e 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' describe Projects::UpdateRemoteMirrorService do let(:project) { create(:project, :repository) } let(:remote_project) { create(:forked_project_with_submodules) } - let(:remote_mirror) { project.remote_mirrors.create!(url: remote_project.http_url_to_repo, enabled: true, only_protected_branches: false) } + let(:remote_mirror) { create(:remote_mirror, project: project, enabled: true) } let(:remote_name) { remote_mirror.remote_name } subject(:service) { described_class.new(project, project.creator) } @@ -16,7 +16,9 @@ describe Projects::UpdateRemoteMirrorService do before do project.repository.add_branch(project.owner, 'existing-branch', 'master') - allow(remote_mirror).to receive(:update_repository).and_return(true) + allow(remote_mirror) + .to receive(:update_repository) + .and_return(double(divergent_refs: [])) end it 'ensures the remote exists' do @@ -53,7 +55,7 @@ describe Projects::UpdateRemoteMirrorService do it 'marks the mirror as failed and raises the error when an unexpected error occurs' do allow(project.repository).to receive(:fetch_remote).and_raise('Badly broken') - expect { execute! }.to raise_error /Badly broken/ + expect { execute! }.to raise_error(/Badly broken/) expect(remote_mirror).to be_failed expect(remote_mirror.last_error).to include('Badly broken') @@ -83,32 +85,21 @@ describe Projects::UpdateRemoteMirrorService do end end - context 'when syncing all branches' do - it 'push all the branches the first time' do + context 'when there are divergent refs' do + before do stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) - - expect(remote_mirror).to receive(:update_repository).with({}) - - execute! end - end - context 'when only syncing protected branches' do - it 'sync updated protected branches' do - stub_fetch_remote(project, remote_name: remote_name, ssh_auth: remote_mirror) - protected_branch = create_protected_branch(project) - remote_mirror.only_protected_branches = true - - expect(remote_mirror) - .to receive(:update_repository) - .with(only_branches_matching: [protected_branch.name]) + it 'marks the mirror as failed and sets an error message' do + response = double(divergent_refs: %w[refs/heads/master refs/heads/develop]) + expect(remote_mirror).to receive(:update_repository).and_return(response) execute! - end - def create_protected_branch(project) - branch_name = project.repository.branch_names.find { |n| n != 'existing-branch' } - create(:protected_branch, project: project, name: branch_name) + expect(remote_mirror).to be_failed + expect(remote_mirror.last_error).to include("Some refs have diverged") + expect(remote_mirror.last_error).to include("refs/heads/master\n") + expect(remote_mirror.last_error).to include("refs/heads/develop") end end end diff --git a/spec/services/projects/update_repository_storage_service_spec.rb b/spec/services/projects/update_repository_storage_service_spec.rb index 05555fa76f7..28b79bc61d9 100644 --- a/spec/services/projects/update_repository_storage_service_spec.rb +++ b/spec/services/projects/update_repository_storage_service_spec.rb @@ -5,17 +5,20 @@ require 'spec_helper' describe Projects::UpdateRepositoryStorageService do include Gitlab::ShellAdapter - subject { described_class.new(project) } + subject { described_class.new(repository_storage_move) } describe "#execute" do - let(:time) { Time.now } + let(:time) { Time.current } before do allow(Time).to receive(:now).and_return(time) + allow(Gitlab.config.repositories.storages).to receive(:keys).and_return(%w[default test_second_storage]) end context 'without wiki and design repository' do let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: false) } + let(:destination) { 'test_second_storage' } + let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) } let!(:checksum) { project.repository.checksum } let(:project_repository_double) { double(:repository) } @@ -41,9 +44,9 @@ describe Projects::UpdateRepositoryStorageService do expect(project_repository_double).to receive(:checksum) .and_return(checksum) - result = subject.execute('test_second_storage') + result = subject.execute - expect(result[:status]).to eq(:success) + expect(result).to be_success expect(project).not_to be_repository_read_only expect(project.repository_storage).to eq('test_second_storage') expect(gitlab_shell.repository_exists?('default', old_path)).to be(false) @@ -52,11 +55,13 @@ describe Projects::UpdateRepositoryStorageService do end context 'when the filesystems are the same' do + let(:destination) { project.repository_storage } + it 'bails out and does nothing' do - result = subject.execute(project.repository_storage) + result = subject.execute - expect(result[:status]).to eq(:error) - expect(result[:message]).to match(/SameFilesystemError/) + expect(result).to be_error + expect(result.message).to match(/SameFilesystemError/) end end @@ -72,9 +77,9 @@ describe Projects::UpdateRepositoryStorageService do .and_raise(Gitlab::Git::CommandError) expect(GitlabShellWorker).not_to receive(:perform_async) - result = subject.execute('test_second_storage') + result = subject.execute - expect(result[:status]).to eq(:error) + expect(result).to be_error expect(project).not_to be_repository_read_only expect(project.repository_storage).to eq('default') end @@ -93,9 +98,9 @@ describe Projects::UpdateRepositoryStorageService do .and_return('not matching checksum') expect(GitlabShellWorker).not_to receive(:perform_async) - result = subject.execute('test_second_storage') + result = subject.execute - expect(result[:status]).to eq(:error) + expect(result).to be_error expect(project).not_to be_repository_read_only expect(project.repository_storage).to eq('default') end @@ -115,9 +120,9 @@ describe Projects::UpdateRepositoryStorageService do expect(project_repository_double).to receive(:checksum) .and_return(checksum) - result = subject.execute('test_second_storage') + result = subject.execute - expect(result[:status]).to eq(:success) + expect(result).to be_success expect(project.repository_storage).to eq('test_second_storage') expect(project.reload_pool_repository).to be_nil end @@ -128,11 +133,26 @@ describe Projects::UpdateRepositoryStorageService do include_examples 'moves repository to another storage', 'wiki' do let(:project) { create(:project, :repository, repository_read_only: true, wiki_enabled: true) } let(:repository) { project.wiki.repository } + let(:destination) { 'test_second_storage' } + let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) } before do project.create_wiki end end end + + context 'with design repository' do + include_examples 'moves repository to another storage', 'design' do + let(:project) { create(:project, :repository, repository_read_only: true) } + let(:repository) { project.design_repository } + let(:destination) { 'test_second_storage' } + let(:repository_storage_move) { create(:project_repository_storage_move, :scheduled, project: project, destination_storage_name: destination) } + + before do + project.design_repository.create_if_not_exists + end + end + end end end |