diff options
Diffstat (limited to 'spec/services/projects')
19 files changed, 621 insertions, 548 deletions
diff --git a/spec/services/projects/after_rename_service_spec.rb b/spec/services/projects/after_rename_service_spec.rb index 52136b37c66..f03e1ed0e22 100644 --- a/spec/services/projects/after_rename_service_spec.rb +++ b/spec/services/projects/after_rename_service_spec.rb @@ -22,7 +22,6 @@ RSpec.describe Projects::AfterRenameService do # call. This makes testing a bit easier. allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - stub_feature_flags(skip_hashed_storage_upgrade: false) stub_application_setting(hashed_storage_enabled: false) end @@ -62,13 +61,28 @@ RSpec.describe Projects::AfterRenameService do context 'gitlab pages' do before do - expect(project_storage).to receive(:rename_repo) { true } + allow(project_storage).to receive(:rename_repo) { true } end - it 'moves pages folder to new location' do - expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project) + context 'when the project has pages deployed' do + it 'schedules a move of the pages directory' do + allow(project).to receive(:pages_deployed?).and_return(true) - service_execute + expect(PagesTransferWorker).to receive(:perform_async).with('rename_project', anything) + + service_execute + end + end + + context 'when the project does not have pages deployed' do + it 'does nothing with the pages directory' do + allow(project).to receive(:pages_deployed?).and_return(false) + + expect(PagesTransferWorker).not_to receive(:perform_async) + expect(Gitlab::PagesTransfer).not_to receive(:new) + + service_execute + end end end @@ -126,7 +140,6 @@ RSpec.describe Projects::AfterRenameService do # call. This makes testing a bit easier. allow(project).to receive(:gitlab_shell).and_return(gitlab_shell) - stub_feature_flags(skip_hashed_storage_upgrade: false) stub_application_setting(hashed_storage_enabled: true) end @@ -160,10 +173,25 @@ RSpec.describe Projects::AfterRenameService do end context 'gitlab pages' do - it 'moves pages folder to new location' do - expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project) + context 'when the project has pages deployed' do + it 'schedules a move of the pages directory' do + allow(project).to receive(:pages_deployed?).and_return(true) - service_execute + expect(PagesTransferWorker).to receive(:perform_async).with('rename_project', anything) + + service_execute + end + end + + context 'when the project does not have pages deployed' do + it 'does nothing with the pages directory' do + allow(project).to receive(:pages_deployed?).and_return(false) + + expect(PagesTransferWorker).not_to receive(:perform_async) + expect(Gitlab::PagesTransfer).not_to receive(:new) + + service_execute + end end end diff --git a/spec/services/projects/alerting/notify_service_spec.rb b/spec/services/projects/alerting/notify_service_spec.rb index 3e74a15c3c0..77a0e330109 100644 --- a/spec/services/projects/alerting/notify_service_spec.rb +++ b/spec/services/projects/alerting/notify_service_spec.rb @@ -3,67 +3,32 @@ require 'spec_helper' RSpec.describe Projects::Alerting::NotifyService do - let_it_be(:project, reload: true) { create(:project) } + let_it_be_with_reload(:project) { create(:project, :repository) } before do - # We use `let_it_be(:project)` so we make sure to clear caches - project.clear_memoization(:licensed_feature_available) allow(ProjectServiceWorker).to receive(:perform_async) end - shared_examples 'processes incident issues' do - let(:create_incident_service) { spy } - - before do - allow_any_instance_of(AlertManagement::Alert).to receive(:execute_services) - end - - it 'processes issues' do - expect(IncidentManagement::ProcessAlertWorker) - .to receive(:perform_async) - .with(nil, nil, kind_of(Integer)) - .once - - Sidekiq::Testing.inline! do - expect(subject).to be_success - end - end - end - - shared_examples 'does not process incident issues' do - it 'does not process issues' do - expect(IncidentManagement::ProcessAlertWorker) - .not_to receive(:perform_async) - - expect(subject).to be_success - end - end - - shared_examples 'does not process incident issues due to error' do |http_status:| - it 'does not process issues' do - expect(IncidentManagement::ProcessAlertWorker) - .not_to receive(:perform_async) - - expect(subject).to be_error - expect(subject.http_status).to eq(http_status) - end - end - describe '#execute' do let(:token) { 'invalid-token' } let(:starts_at) { Time.current.change(usec: 0) } let(:fingerprint) { 'testing' } let(:service) { described_class.new(project, nil, payload) } + let_it_be(:environment) { create(:environment, project: project) } + let(:environment) { create(:environment, project: project) } + let(:ended_at) { nil } let(:payload_raw) do { title: 'alert title', start_time: starts_at.rfc3339, + end_time: ended_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'], - fingerprint: fingerprint + fingerprint: fingerprint, + gitlab_environment_name: environment.name }.with_indifferent_access end @@ -72,13 +37,14 @@ RSpec.describe Projects::Alerting::NotifyService do subject { service.execute(token) } context 'with activated Alerts Service' do - let!(:alerts_service) { create(:alerts_service, project: project) } + let_it_be_with_reload(:alerts_service) { create(:alerts_service, project: project) } context 'with valid token' do let(:token) { alerts_service.token } - let(:incident_management_setting) { double(send_email?: email_enabled, create_issue?: issue_enabled) } + let(:incident_management_setting) { double(send_email?: email_enabled, create_issue?: issue_enabled, auto_close_incident?: auto_close_enabled) } let(:email_enabled) { false } let(:issue_enabled) { false } + let(:auto_close_enabled) { false } before do allow(service) @@ -105,9 +71,9 @@ RSpec.describe Projects::Alerting::NotifyService do monitoring_tool: payload_raw.fetch(:monitoring_tool), service: payload_raw.fetch(:service), fingerprint: Digest::SHA1.hexdigest(fingerprint), + environment_id: environment.id, ended_at: nil, - prometheus_alert_id: nil, - environment_id: nil + prometheus_alert_id: nil ) end end @@ -121,12 +87,67 @@ RSpec.describe Projects::Alerting::NotifyService do it_behaves_like 'creates an alert management alert' it_behaves_like 'assigns the alert properties' + it 'creates a system note corresponding to alert creation' do + expect { subject }.to change(Note, :count).by(1) + end + context 'existing alert with same fingerprint' do let(:fingerprint_sha) { Digest::SHA1.hexdigest(fingerprint) } let!(:alert) { create(:alert_management_alert, project: project, fingerprint: fingerprint_sha) } it_behaves_like 'adds an alert management alert event' + context 'end time given' do + let(:ended_at) { Time.current.change(nsec: 0) } + + it 'does not resolve the alert' do + expect { subject }.not_to change { alert.reload.status } + end + + it 'does not set the ended at' do + subject + + expect(alert.reload.ended_at).to be_nil + end + + it_behaves_like 'does not an create alert management alert' + + context 'auto_close_enabled setting enabled' do + let(:auto_close_enabled) { true } + + it 'resolves the alert and sets the end time', :aggregate_failures do + subject + alert.reload + + expect(alert.resolved?).to eq(true) + expect(alert.ended_at).to eql(ended_at) + end + + context 'related issue exists' do + let(:alert) { create(:alert_management_alert, :with_issue, project: project, fingerprint: fingerprint_sha) } + let(:issue) { alert.issue } + + context 'state_tracking is enabled' do + before do + stub_feature_flags(track_resource_state_change_events: true) + end + + it { expect { subject }.to change { issue.reload.state }.from('opened').to('closed') } + it { expect { subject }.to change(ResourceStateEvent, :count).by(1) } + end + + context 'state_tracking is disabled' do + before do + stub_feature_flags(track_resource_state_change_events: false) + end + + it { expect { subject }.to change { issue.reload.state }.from('opened').to('closed') } + it { expect { subject }.to change(Note, :count).by(1) } + end + end + end + end + context 'existing alert is resolved' do let!(:alert) { create(:alert_management_alert, :resolved, project: project, fingerprint: fingerprint_sha) } @@ -148,6 +169,13 @@ RSpec.describe Projects::Alerting::NotifyService do end end + context 'end time given' do + let(:ended_at) { Time.current } + + it_behaves_like 'creates an alert management alert' + it_behaves_like 'assigns the alert properties' + end + context 'with a minimal payload' do let(:payload_raw) do { @@ -183,6 +211,18 @@ RSpec.describe Projects::Alerting::NotifyService do end end + context 'with overlong payload' do + let(:payload_raw) do + { + title: 'a' * Gitlab::Utils::DeepSize::DEFAULT_MAX_SIZE, + start_time: starts_at.rfc3339 + } + end + + it_behaves_like 'does not process incident issues due to error', http_status: :bad_request + it_behaves_like 'does not an create alert management alert' + end + it_behaves_like 'does not process incident issues' context 'issue enabled' do @@ -230,7 +270,9 @@ RSpec.describe Projects::Alerting::NotifyService do end context 'with deactivated Alerts Service' do - let!(:alerts_service) { create(:alerts_service, :inactive, project: project) } + before do + alerts_service.update!(active: false) + end it_behaves_like 'does not process incident issues due to error', http_status: :forbidden it_behaves_like 'does not an create alert management alert' diff --git a/spec/services/projects/container_repository/delete_tags_service_spec.rb b/spec/services/projects/container_repository/delete_tags_service_spec.rb index 3014ccbd7ba..5116427dad2 100644 --- a/spec/services/projects/container_repository/delete_tags_service_spec.rb +++ b/spec/services/projects/container_repository/delete_tags_service_spec.rb @@ -90,6 +90,10 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do subject { service.execute(repository) } + before do + stub_feature_flags(container_registry_expiration_policies_throttling: false) + end + context 'without permissions' do it { is_expected.to include(status: :error) } end @@ -119,6 +123,18 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do it_behaves_like 'logging a success response' end + + context 'with a timeout error' do + before do + expect_next_instance_of(::Projects::ContainerRepository::Gitlab::DeleteTagsService) do |delete_service| + expect(delete_service).to receive(:delete_tags).and_raise(::Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError) + end + end + + it { is_expected.to include(status: :error, message: 'timeout while deleting tags') } + + it_behaves_like 'logging an error response', message: 'timeout while deleting tags' + end end context 'and the feature is disabled' do diff --git a/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb b/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb index 68c232e5d83..3bbcec8775e 100644 --- a/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb +++ b/spec/services/projects/container_repository/gitlab/delete_tags_service_spec.rb @@ -12,13 +12,21 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do subject { service.execute } - context 'with tags to delete' do + before do + stub_feature_flags(container_registry_expiration_policies_throttling: false) + end + + RSpec.shared_examples 'deleting tags' do it 'deletes the tags by name' do stub_delete_reference_requests(tags) expect_delete_tag_by_names(tags) is_expected.to eq(status: :success, deleted: tags) end + end + + context 'with tags to delete' do + it_behaves_like 'deleting tags' it 'succeeds when tag delete returns 404' do stub_delete_reference_requests('A' => 200, 'Ba' => 404) @@ -41,6 +49,47 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do it { is_expected.to eq(status: :error, message: 'could not delete tags') } end end + + context 'with throttling enabled' do + let(:timeout) { 10 } + + before do + stub_feature_flags(container_registry_expiration_policies_throttling: true) + stub_application_setting(container_registry_delete_tags_service_timeout: timeout) + end + + it_behaves_like 'deleting tags' + + context 'with timeout' do + context 'set to a valid value' do + before do + allow(Time.zone).to receive(:now).and_return(10, 15, 25) # third call to Time.zone.now will be triggering the timeout + stub_delete_reference_requests('A' => 200) + end + + it { is_expected.to include(status: :error, message: 'timeout while deleting tags') } + + it 'tracks the exception' do + expect(::Gitlab::ErrorTracking) + .to receive(:track_exception).with(::Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError, tags_count: tags.size, container_repository_id: repository.id) + + subject + end + end + + context 'set to 0' do + let(:timeout) { 0 } + + it_behaves_like 'deleting tags' + end + + context 'set to nil' do + let(:timeout) { nil } + + it_behaves_like 'deleting tags' + end + end + end end context 'with empty tags' do diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb index 56b19c33ece..a3711c9e17f 100644 --- a/spec/services/projects/destroy_service_spec.rb +++ b/spec/services/projects/destroy_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Projects::DestroyService do +RSpec.describe Projects::DestroyService, :aggregate_failures do include ProjectForksHelper let_it_be(:user) { create(:user) } @@ -60,317 +60,353 @@ RSpec.describe Projects::DestroyService do end end - it_behaves_like 'deleting the project' - - it 'invalidates personal_project_count cache' do - expect(user).to receive(:invalidate_personal_projects_count) - - destroy_project(project, user, {}) - end - - context 'when project has remote mirrors' do - let!(:project) do - create(:project, :repository, namespace: user.namespace).tap do |project| - project.remote_mirrors.create(url: 'http://test.com') - end - end + shared_examples 'project destroy' do + it_behaves_like 'deleting the project' - it 'destroys them' do - expect(RemoteMirror.count).to eq(1) + it 'invalidates personal_project_count cache' do + expect(user).to receive(:invalidate_personal_projects_count) destroy_project(project, user, {}) - - expect(RemoteMirror.count).to eq(0) end - end - context 'when project has exports' do - let!(:project_with_export) do - create(:project, :repository, namespace: user.namespace).tap do |project| - create(:import_export_upload, - project: project, - export_file: fixture_file_upload('spec/fixtures/project_export.tar.gz')) + context 'when project has remote mirrors' do + let!(:project) do + create(:project, :repository, namespace: user.namespace).tap do |project| + project.remote_mirrors.create(url: 'http://test.com') + end end - end - it 'destroys project and export' do - expect do - destroy_project(project_with_export, user, {}) - end.to change(ImportExportUpload, :count).by(-1) + it 'destroys them' do + expect(RemoteMirror.count).to eq(1) - expect(Project.all).not_to include(project_with_export) - end - end + destroy_project(project, user, {}) - context 'Sidekiq fake' do - before do - # Dont run sidekiq to check if renamed repository exists - Sidekiq::Testing.fake! { destroy_project(project, user, {}) } + expect(RemoteMirror.count).to eq(0) + end end - it { expect(Project.all).not_to include(project) } - - it do - expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_falsey - end + context 'when project has exports' do + let!(:project_with_export) do + create(:project, :repository, namespace: user.namespace).tap do |project| + create(:import_export_upload, + project: project, + export_file: fixture_file_upload('spec/fixtures/project_export.tar.gz')) + end + end - it do - expect(project.gitlab_shell.repository_exists?(project.repository_storage, remove_path + '.git')).to be_truthy - end - end + it 'destroys project and export' do + expect do + destroy_project(project_with_export, user, {}) + end.to change(ImportExportUpload, :count).by(-1) - context 'when flushing caches fail due to Git errors' do - before do - allow(project.repository).to receive(:before_delete).and_raise(::Gitlab::Git::CommandError) - allow(Gitlab::GitLogger).to receive(:warn).with( - class: Repositories::DestroyService.name, - container_id: project.id, - disk_path: project.disk_path, - message: 'Gitlab::Git::CommandError').and_call_original + expect(Project.all).not_to include(project_with_export) + end end - it_behaves_like 'deleting the project' - end + context 'Sidekiq fake' do + before do + # Dont run sidekiq to check if renamed repository exists + Sidekiq::Testing.fake! { destroy_project(project, user, {}) } + end - context 'when flushing caches fail due to Redis' do - before do - new_user = create(:user) - project.team.add_user(new_user, Gitlab::Access::DEVELOPER) - allow_any_instance_of(described_class).to receive(:flush_caches).and_raise(::Redis::CannotConnectError) - end + it { expect(Project.all).not_to include(project) } - it 'keeps project team intact upon an error' do - perform_enqueued_jobs do - destroy_project(project, user, {}) - rescue ::Redis::CannotConnectError + it do + expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_falsey end - expect(project.team.members.count).to eq 2 + it do + expect(project.gitlab_shell.repository_exists?(project.repository_storage, remove_path + '.git')).to be_truthy + end end - end - context 'with async_execute', :sidekiq_inline do - let(:async) { true } - - context 'async delete of project with private issue visibility' do + context 'when flushing caches fail due to Git errors' do before do - project.project_feature.update_attribute("issues_access_level", ProjectFeature::PRIVATE) + allow(project.repository).to receive(:before_delete).and_raise(::Gitlab::Git::CommandError) + allow(Gitlab::GitLogger).to receive(:warn).with( + class: Repositories::DestroyService.name, + container_id: project.id, + disk_path: project.disk_path, + message: 'Gitlab::Git::CommandError').and_call_original end it_behaves_like 'deleting the project' end - it_behaves_like 'deleting the project with pipeline and build' + context 'when flushing caches fail due to Redis' do + before do + new_user = create(:user) + project.team.add_user(new_user, Gitlab::Access::DEVELOPER) + allow_any_instance_of(described_class).to receive(:flush_caches).and_raise(::Redis::CannotConnectError) + end - context 'errors' do - context 'when `remove_legacy_registry_tags` fails' do - before do - expect_any_instance_of(described_class) - .to receive(:remove_legacy_registry_tags).and_return(false) + it 'keeps project team intact upon an error' do + perform_enqueued_jobs do + destroy_project(project, user, {}) + rescue ::Redis::CannotConnectError end - it_behaves_like 'handles errors thrown during async destroy', "Failed to remove some tags" + expect(project.team.members.count).to eq 2 end + end + + context 'with async_execute', :sidekiq_inline do + let(:async) { true } - context 'when `remove_repository` fails' do + context 'async delete of project with private issue visibility' do before do - expect_any_instance_of(described_class) - .to receive(:remove_repository).and_return(false) + project.project_feature.update_attribute("issues_access_level", ProjectFeature::PRIVATE) end - it_behaves_like 'handles errors thrown during async destroy', "Failed to remove project repository" + it_behaves_like 'deleting the project' end - context 'when `execute` raises expected error' do - before do - expect_any_instance_of(Project) - .to receive(:destroy!).and_raise(StandardError.new("Other error message")) + it_behaves_like 'deleting the project with pipeline and build' + + context 'errors' do + context 'when `remove_legacy_registry_tags` fails' do + before do + expect_any_instance_of(described_class) + .to receive(:remove_legacy_registry_tags).and_return(false) + end + + it_behaves_like 'handles errors thrown during async destroy', "Failed to remove some tags" end - it_behaves_like 'handles errors thrown during async destroy', "Other error message" - end + context 'when `remove_repository` fails' do + before do + expect_any_instance_of(described_class) + .to receive(:remove_repository).and_return(false) + end - context 'when `execute` raises unexpected error' do - before do - expect_any_instance_of(Project) - .to receive(:destroy!).and_raise(Exception.new('Other error message')) + it_behaves_like 'handles errors thrown during async destroy', "Failed to remove project repository" end - it 'allows error to bubble up and rolls back project deletion' do - expect do - destroy_project(project, user, {}) - end.to raise_error(Exception, 'Other error message') + context 'when `execute` raises expected error' do + before do + expect_any_instance_of(Project) + .to receive(:destroy!).and_raise(StandardError.new("Other error message")) + end - expect(project.reload.pending_delete).to be(false) - expect(project.delete_error).to include("Other error message") + it_behaves_like 'handles errors thrown during async destroy', "Other error message" end - end - end - end - describe 'container registry' do - context 'when there are regular container repositories' do - let(:container_repository) { create(:container_repository) } + context 'when `execute` raises unexpected error' do + before do + expect_any_instance_of(Project) + .to receive(:destroy!).and_raise(Exception.new('Other error message')) + end - before do - stub_container_registry_tags(repository: project.full_path + '/image', - tags: ['tag']) - project.container_repositories << container_repository + it 'allows error to bubble up and rolls back project deletion' do + expect do + destroy_project(project, user, {}) + end.to raise_error(Exception, 'Other error message') + + expect(project.reload.pending_delete).to be(false) + expect(project.delete_error).to include("Other error message") + end + end end + end - context 'when image repository deletion succeeds' do - it 'removes tags' do - expect_any_instance_of(ContainerRepository) - .to receive(:delete_tags!).and_return(true) + describe 'container registry' do + context 'when there are regular container repositories' do + let(:container_repository) { create(:container_repository) } - destroy_project(project, user) + before do + stub_container_registry_tags(repository: project.full_path + '/image', + tags: ['tag']) + project.container_repositories << container_repository end - end - context 'when image repository deletion fails' do - it 'raises an exception' do - expect_any_instance_of(ContainerRepository) - .to receive(:delete_tags!).and_raise(RuntimeError) + context 'when image repository deletion succeeds' do + it 'removes tags' do + expect_any_instance_of(ContainerRepository) + .to receive(:delete_tags!).and_return(true) - expect(destroy_project(project, user)).to be false + destroy_project(project, user) + end end - end - context 'when registry is disabled' do - before do - stub_container_registry_config(enabled: false) + context 'when image repository deletion fails' do + it 'raises an exception' do + expect_any_instance_of(ContainerRepository) + .to receive(:delete_tags!).and_raise(RuntimeError) + + expect(destroy_project(project, user)).to be false + end end - it 'does not attempting to remove any tags' do - expect(Projects::ContainerRepository::DestroyService).not_to receive(:new) + context 'when registry is disabled' do + before do + stub_container_registry_config(enabled: false) + end - destroy_project(project, user) + it 'does not attempting to remove any tags' do + expect(Projects::ContainerRepository::DestroyService).not_to receive(:new) + + destroy_project(project, user) + end end end - end - context 'when there are tags for legacy root repository' do - before do - stub_container_registry_tags(repository: project.full_path, - tags: ['tag']) - end + context 'when there are tags for legacy root repository' do + before do + stub_container_registry_tags(repository: project.full_path, + tags: ['tag']) + end - context 'when image repository tags deletion succeeds' do - it 'removes tags' do - expect_any_instance_of(ContainerRepository) - .to receive(:delete_tags!).and_return(true) + context 'when image repository tags deletion succeeds' do + it 'removes tags' do + expect_any_instance_of(ContainerRepository) + .to receive(:delete_tags!).and_return(true) - destroy_project(project, user) + destroy_project(project, user) + end end - end - context 'when image repository tags deletion fails' do - it 'raises an exception' do - expect_any_instance_of(ContainerRepository) - .to receive(:delete_tags!).and_return(false) + context 'when image repository tags deletion fails' do + it 'raises an exception' do + expect_any_instance_of(ContainerRepository) + .to receive(:delete_tags!).and_return(false) - expect(destroy_project(project, user)).to be false + expect(destroy_project(project, user)).to be false + end end end end - end - context 'for a forked project with LFS objects' do - let(:forked_project) { fork_project(project, user) } + context 'for a forked project with LFS objects' do + let(:forked_project) { fork_project(project, user) } - before do - project.lfs_objects << create(:lfs_object) - forked_project.reload - end + before do + project.lfs_objects << create(:lfs_object) + forked_project.reload + end - it 'destroys the fork' do - expect { destroy_project(forked_project, user) } - .not_to raise_error + it 'destroys the fork' do + expect { destroy_project(forked_project, user) } + .not_to raise_error + end end - end - context 'as the root of a fork network' do - let!(:fork_1) { fork_project(project, user) } - let!(:fork_2) { fork_project(project, user) } + context 'as the root of a fork network' do + let!(:fork_1) { fork_project(project, user) } + let!(:fork_2) { fork_project(project, user) } - it 'updates the fork network with the project name' do - fork_network = project.fork_network + it 'updates the fork network with the project name' do + fork_network = project.fork_network - destroy_project(project, user) + destroy_project(project, user) - fork_network.reload + fork_network.reload - expect(fork_network.deleted_root_project_name).to eq(project.full_name) - expect(fork_network.root_project).to be_nil + expect(fork_network.deleted_root_project_name).to eq(project.full_name) + expect(fork_network.root_project).to be_nil + end end - end - context 'repository +deleted path removal' do - context 'regular phase' do - it 'schedules +deleted removal of existing repos' do - service = described_class.new(project, user, {}) - allow(service).to receive(:schedule_stale_repos_removal) + context 'repository +deleted path removal' do + context 'regular phase' do + it 'schedules +deleted removal of existing repos' do + service = described_class.new(project, user, {}) + allow(service).to receive(:schedule_stale_repos_removal) - expect(Repositories::ShellDestroyService).to receive(:new).and_call_original - expect(GitlabShellWorker).to receive(:perform_in) - .with(5.minutes, :remove_repository, project.repository_storage, removal_path(project.disk_path)) + expect(Repositories::ShellDestroyService).to receive(:new).and_call_original + expect(GitlabShellWorker).to receive(:perform_in) + .with(5.minutes, :remove_repository, project.repository_storage, removal_path(project.disk_path)) - service.execute + service.execute + end end - end - context 'stale cleanup' do - let(:async) { true } + context 'stale cleanup' do + let(:async) { true } - it 'schedules +deleted wiki and repo removal' do - allow(ProjectDestroyWorker).to receive(:perform_async) + it 'schedules +deleted wiki and repo removal' do + allow(ProjectDestroyWorker).to receive(:perform_async) - expect(Repositories::ShellDestroyService).to receive(:new).with(project.repository).and_call_original - expect(GitlabShellWorker).to receive(:perform_in) - .with(10.minutes, :remove_repository, project.repository_storage, removal_path(project.disk_path)) + expect(Repositories::ShellDestroyService).to receive(:new).with(project.repository).and_call_original + expect(GitlabShellWorker).to receive(:perform_in) + .with(10.minutes, :remove_repository, project.repository_storage, removal_path(project.disk_path)) - expect(Repositories::ShellDestroyService).to receive(:new).with(project.wiki.repository).and_call_original - expect(GitlabShellWorker).to receive(:perform_in) - .with(10.minutes, :remove_repository, project.repository_storage, removal_path(project.wiki.disk_path)) + expect(Repositories::ShellDestroyService).to receive(:new).with(project.wiki.repository).and_call_original + expect(GitlabShellWorker).to receive(:perform_in) + .with(10.minutes, :remove_repository, project.repository_storage, removal_path(project.wiki.disk_path)) - destroy_project(project, user, {}) + destroy_project(project, user, {}) + end end end - end - context 'snippets' do - let!(:snippet1) { create(:project_snippet, project: project, author: user) } - let!(:snippet2) { create(:project_snippet, project: project, author: user) } + context 'snippets' do + let!(:snippet1) { create(:project_snippet, project: project, author: user) } + let!(:snippet2) { create(:project_snippet, project: project, author: user) } - it 'does not include snippets when deleting in batches' do - expect(project).to receive(:destroy_dependent_associations_in_batches).with({ exclude: [:container_repositories, :snippets] }) + it 'does not include snippets when deleting in batches' do + expect(project).to receive(:destroy_dependent_associations_in_batches).with({ exclude: [:container_repositories, :snippets] }) - destroy_project(project, user) - end + destroy_project(project, user) + end - it 'calls the bulk snippet destroy service' do - expect(project.snippets.count).to eq 2 + it 'calls the bulk snippet destroy service' do + expect(project.snippets.count).to eq 2 - expect(Snippets::BulkDestroyService).to receive(:new) - .with(user, project.snippets).and_call_original + expect(Snippets::BulkDestroyService).to receive(:new) + .with(user, project.snippets).and_call_original - expect do - destroy_project(project, user) - end.to change(Snippet, :count).by(-2) - end + expect do + destroy_project(project, user) + end.to change(Snippet, :count).by(-2) + end - context 'when an error is raised deleting snippets' do - it 'does not delete project' do - allow_next_instance_of(Snippets::BulkDestroyService) do |instance| - allow(instance).to receive(:execute).and_return(ServiceResponse.error(message: 'foo')) + context 'when an error is raised deleting snippets' do + it 'does not delete project' do + allow_next_instance_of(Snippets::BulkDestroyService) do |instance| + allow(instance).to receive(:execute).and_return(ServiceResponse.error(message: 'foo')) + end + + expect(destroy_project(project, user)).to be_falsey + expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_truthy end + end + end - expect(destroy_project(project, user)).to be_falsey - expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_truthy + context 'error while destroying', :sidekiq_inline do + let!(:pipeline) { create(:ci_pipeline, project: project) } + let!(:builds) { create_list(:ci_build, 2, :artifacts, pipeline: pipeline) } + let!(:build_trace) { create(:ci_build_trace_chunk, build: builds[0]) } + + it 'deletes on retry' do + # We can expect this to timeout for very large projects + # TODO: remove allow_next_instance_of: https://gitlab.com/gitlab-org/gitlab/-/issues/220440 + allow_any_instance_of(Ci::Build).to receive(:destroy).and_raise('boom') + destroy_project(project, user, {}) + + allow_any_instance_of(Ci::Build).to receive(:destroy).and_call_original + destroy_project(project, user, {}) + + expect(Project.unscoped.all).not_to include(project) + expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_falsey + expect(project.gitlab_shell.repository_exists?(project.repository_storage, remove_path + '.git')).to be_falsey + expect(project.all_pipelines).to be_empty + expect(project.builds).to be_empty end end end + context 'when project_transactionless_destroy enabled' do + it_behaves_like 'project destroy' + end + + context 'when project_transactionless_destroy disabled', :sidekiq_inline do + before do + stub_feature_flags(project_transactionless_destroy: false) + end + + it_behaves_like 'project destroy' + end + def destroy_project(project, user, params = {}) described_class.new(project, user, params).public_send(async ? :async_execute : :execute) end diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index 925c2ff5d88..166a2dae55b 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -9,7 +9,7 @@ RSpec.describe Projects::ForkService do it 'flushes the forks count cache of the source project', :clean_gitlab_redis_cache do expect(from_project.forks_count).to be_zero - fork_project(from_project, to_user) + fork_project(from_project, to_user, using_service: true) BatchLoader::Executor.clear_current expect(from_project.forks_count).to eq(1) @@ -40,7 +40,7 @@ RSpec.describe Projects::ForkService do @guest = create(:user) @from_project.add_user(@guest, :guest) end - subject { fork_project(@from_project, @guest) } + subject { fork_project(@from_project, @guest, using_service: true) } it { is_expected.not_to be_persisted } it { expect(subject.errors[:forked_from_project_id]).to eq(['is forbidden']) } @@ -56,7 +56,7 @@ RSpec.describe Projects::ForkService do end describe "successfully creates project in the user namespace" do - let(:to_project) { fork_project(@from_project, @to_user, namespace: @to_user.namespace) } + let(:to_project) { fork_project(@from_project, @to_user, namespace: @to_user.namespace, using_service: true) } it { expect(to_project).to be_persisted } it { expect(to_project.errors).to be_empty } @@ -92,21 +92,21 @@ RSpec.describe Projects::ForkService do end it 'imports the repository of the forked project', :sidekiq_might_not_need_inline do - to_project = fork_project(@from_project, @to_user, repository: true) + to_project = fork_project(@from_project, @to_user, repository: true, using_service: true) expect(to_project.empty_repo?).to be_falsy end end context 'creating a fork of a fork' do - let(:from_forked_project) { fork_project(@from_project, @to_user) } + let(:from_forked_project) { fork_project(@from_project, @to_user, using_service: true) } let(:other_namespace) do group = create(:group) group.add_owner(@to_user) group end - let(:to_project) { fork_project(from_forked_project, @to_user, namespace: other_namespace) } + let(:to_project) { fork_project(from_forked_project, @to_user, namespace: other_namespace, using_service: true) } it 'sets the root of the network to the root project' do expect(to_project.fork_network.root_project).to eq(@from_project) @@ -126,7 +126,7 @@ RSpec.describe Projects::ForkService do context 'project already exists' do it "fails due to validation, not transaction failure" do @existing_project = create(:project, :repository, creator_id: @to_user.id, name: @from_project.name, namespace: @to_namespace) - @to_project = fork_project(@from_project, @to_user, namespace: @to_namespace) + @to_project = fork_project(@from_project, @to_user, namespace: @to_namespace, using_service: true) expect(@existing_project).to be_persisted expect(@to_project).not_to be_persisted @@ -137,7 +137,7 @@ RSpec.describe Projects::ForkService do context 'repository in legacy storage already exists' do let(:fake_repo_path) { File.join(TestEnv.repos_path, @to_user.namespace.full_path, "#{@from_project.path}.git") } - let(:params) { { namespace: @to_user.namespace } } + let(:params) { { namespace: @to_user.namespace, using_service: true } } before do stub_application_setting(hashed_storage_enabled: false) @@ -169,13 +169,13 @@ RSpec.describe Projects::ForkService do context 'GitLab CI is enabled' do it "forks and enables CI for fork" do @from_project.enable_ci - @to_project = fork_project(@from_project, @to_user) + @to_project = fork_project(@from_project, @to_user, using_service: true) expect(@to_project.builds_enabled?).to be_truthy end end context "CI/CD settings" do - let(:to_project) { fork_project(@from_project, @to_user) } + let(:to_project) { fork_project(@from_project, @to_user, using_service: true) } context "when origin has git depth specified" do before do @@ -206,7 +206,7 @@ RSpec.describe Projects::ForkService do end it "creates fork with lowest level" do - forked_project = fork_project(@from_project, @to_user) + forked_project = fork_project(@from_project, @to_user, using_service: true) expect(forked_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end @@ -218,7 +218,7 @@ RSpec.describe Projects::ForkService do end it "creates fork with private visibility levels" do - forked_project = fork_project(@from_project, @to_user) + forked_project = fork_project(@from_project, @to_user, using_service: true) expect(forked_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end @@ -232,7 +232,7 @@ RSpec.describe Projects::ForkService do end it 'fails' do - to_project = fork_project(@from_project, @to_user, namespace: @to_user.namespace) + to_project = fork_project(@from_project, @to_user, namespace: @to_user.namespace, using_service: true) expect(to_project.errors[:forked_from_project_id]).to eq(['is forbidden']) end @@ -253,7 +253,7 @@ RSpec.describe Projects::ForkService do @group.add_user(@developer, GroupMember::DEVELOPER) @project.add_user(@developer, :developer) @project.add_user(@group_owner, :developer) - @opts = { namespace: @group } + @opts = { namespace: @group, using_service: true } end context 'fork project for group' do @@ -299,7 +299,7 @@ RSpec.describe Projects::ForkService do group_owner = create(:user) private_group.add_owner(group_owner) - forked_project = fork_project(public_project, group_owner, namespace: private_group) + forked_project = fork_project(public_project, group_owner, namespace: private_group, using_service: true) expect(forked_project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE) end @@ -310,7 +310,7 @@ RSpec.describe Projects::ForkService do context 'when a project is already forked' do it 'creates a new poolresository after the project is moved to a new shard' do project = create(:project, :public, :repository) - fork_before_move = fork_project(project) + fork_before_move = fork_project(project, nil, using_service: true) # Stub everything required to move a project to a Gitaly shard that does not exist allow(Gitlab::GitalyClient).to receive(:filesystem_id).with('default').and_call_original @@ -329,7 +329,7 @@ RSpec.describe Projects::ForkService do destination_storage_name: 'test_second_storage' ) Projects::UpdateRepositoryStorageService.new(storage_move).execute - fork_after_move = fork_project(project.reload) + fork_after_move = fork_project(project.reload, nil, using_service: true) pool_repository_before_move = PoolRepository.joins(:shard) .find_by(source_project: project, shards: { name: 'default' }) pool_repository_after_move = PoolRepository.joins(:shard) @@ -350,7 +350,7 @@ RSpec.describe Projects::ForkService do context 'when no pool exists' do it 'creates a new object pool' do - forked_project = fork_project(fork_from_project, forker) + forked_project = fork_project(fork_from_project, forker, using_service: true) expect(forked_project.pool_repository).to eq(fork_from_project.pool_repository) end @@ -360,7 +360,7 @@ RSpec.describe Projects::ForkService do let!(:pool_repository) { create(:pool_repository, source_project: fork_from_project) } it 'joins the object pool' do - forked_project = fork_project(fork_from_project, forker) + forked_project = fork_project(fork_from_project, forker, using_service: true) expect(forked_project.pool_repository).to eq(fork_from_project.pool_repository) end 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 5e1b6f2e404..969381b8748 100644 --- a/spec/services/projects/hashed_storage/base_attachment_service_spec.rb +++ b/spec/services/projects/hashed_storage/base_attachment_service_spec.rb @@ -30,7 +30,7 @@ RSpec.describe Projects::HashedStorage::BaseAttachmentService do target_path = Dir.mktmpdir expect(Dir.exist?(target_path)).to be_truthy - Timecop.freeze do + freeze_time do suffix = Time.current.utc.to_i subject.send(:discard_path!, target_path) 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 a606371099d..cfe8e863223 100644 --- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb @@ -4,7 +4,8 @@ require 'spec_helper' RSpec.describe Projects::LfsPointers::LfsDownloadService do include StubRequests - let(:project) { create(:project) } + let_it_be(:project) { create(:project) } + let(:lfs_content) { SecureRandom.random_bytes(10) } let(:oid) { Digest::SHA256.hexdigest(lfs_content) } let(:download_link) { "http://gitlab.com/#{oid}" } @@ -14,9 +15,11 @@ RSpec.describe Projects::LfsPointers::LfsDownloadService do subject { described_class.new(project, lfs_object) } - before do + before_all do ApplicationSetting.create_from_defaults + end + before do stub_application_setting(allow_local_requests_from_web_hooks_and_services: local_request_setting) allow(project).to receive(:lfs_enabled?).and_return(true) end @@ -226,5 +229,55 @@ RSpec.describe Projects::LfsPointers::LfsDownloadService do subject.execute end end + + context 'when a large lfs object with the same oid already exists' do + let!(:existing_lfs_object) { create(:lfs_object, :with_file, :correct_oid) } + + before do + stub_const("#{described_class}::LARGE_FILE_SIZE", 500) + stub_full_request(download_link).to_return(body: lfs_content) + end + + context 'and first fragments are the same' do + let(:lfs_content) { existing_lfs_object.file.read } + + context 'when lfs_link_existing_object feature flag disabled' do + before do + stub_feature_flags(lfs_link_existing_object: false) + end + + it 'does not call link_existing_lfs_object!' do + expect(subject).not_to receive(:link_existing_lfs_object!) + + subject.execute + end + end + + it 'returns success' do + expect(subject.execute).to eq({ status: :success }) + end + + it 'links existing lfs object to the project' do + expect { subject.execute } + .to change { project.lfs_objects.include?(existing_lfs_object) }.from(false).to(true) + end + end + + context 'and first fragments diverges' do + let(:lfs_content) { SecureRandom.random_bytes(1000) } + let(:oid) { existing_lfs_object.oid } + + it 'raises oid mismatch error' do + expect(subject.execute).to eq({ + status: :error, + message: "LFS file with oid #{oid} cannot be linked with an existing LFS object" + }) + end + + it 'does not change lfs objects' do + expect { subject.execute }.not_to change { project.lfs_objects } + end + end + end end end diff --git a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb index d59f5dbae19..0e7d16f18e8 100644 --- a/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb +++ b/spec/services/projects/lfs_pointers/lfs_link_service_spec.rb @@ -24,11 +24,11 @@ RSpec.describe Projects::LfsPointers::LfsLinkService do end it 'links existing lfs objects to the project' do - expect(project.all_lfs_objects.count).to eq 2 + expect(project.lfs_objects.count).to eq 2 linked = subject.execute(new_oid_list.keys) - expect(project.all_lfs_objects.count).to eq 3 + expect(project.lfs_objects.count).to eq 3 expect(linked.size).to eq 3 end @@ -52,7 +52,7 @@ RSpec.describe Projects::LfsPointers::LfsLinkService do lfs_objects = create_list(:lfs_object, 7) linked = subject.execute(lfs_objects.pluck(:oid)) - expect(project.all_lfs_objects.count).to eq 9 + expect(project.lfs_objects.count).to eq 9 expect(linked.size).to eq 7 end diff --git a/spec/services/projects/open_issues_count_service_spec.rb b/spec/services/projects/open_issues_count_service_spec.rb index c739fea5ecf..294c9adcc92 100644 --- a/spec/services/projects/open_issues_count_service_spec.rb +++ b/spec/services/projects/open_issues_count_service_spec.rb @@ -10,6 +10,14 @@ RSpec.describe Projects::OpenIssuesCountService, :use_clean_rails_memory_store_c it_behaves_like 'a counter caching service' describe '#count' do + it 'does not count test cases' do + create(:issue, :opened, project: project) + create(:incident, :opened, project: project) + create(:quality_test_case, :opened, project: project) + + expect(described_class.new(project).count).to eq(2) + end + context 'when user is nil' do it 'does not include confidential issues in the issue count' do create(:issue, :opened, project: project) diff --git a/spec/services/projects/overwrite_project_service_spec.rb b/spec/services/projects/overwrite_project_service_spec.rb index e4495da9807..a03746d0271 100644 --- a/spec/services/projects/overwrite_project_service_spec.rb +++ b/spec/services/projects/overwrite_project_service_spec.rb @@ -16,6 +16,8 @@ RSpec.describe Projects::OverwriteProjectService do subject { described_class.new(project_to, user) } before do + project_to.project_feature.reload + allow(project_to).to receive(:import_data).and_return(double(data: { 'original_path' => project_from.path })) end diff --git a/spec/services/projects/prometheus/alerts/notify_service_spec.rb b/spec/services/projects/prometheus/alerts/notify_service_spec.rb index efe8e8b9243..0e5ac7c69e3 100644 --- a/spec/services/projects/prometheus/alerts/notify_service_spec.rb +++ b/spec/services/projects/prometheus/alerts/notify_service_spec.rb @@ -16,11 +16,6 @@ RSpec.describe Projects::Prometheus::Alerts::NotifyService do let(:subject) { service.execute(token_input) } - before do - # We use `let_it_be(:project)` so we make sure to clear caches - project.clear_memoization(:licensed_feature_available) - end - context 'with valid payload' do let_it_be(:alert_firing) { create(:prometheus_alert, project: project) } let_it_be(:alert_resolved) { create(:prometheus_alert, project: project) } diff --git a/spec/services/projects/propagate_service_template_spec.rb b/spec/services/projects/propagate_service_template_spec.rb deleted file mode 100644 index df69e5a29fb..00000000000 --- a/spec/services/projects/propagate_service_template_spec.rb +++ /dev/null @@ -1,139 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::PropagateServiceTemplate do - describe '.propagate' do - let!(:service_template) 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 default] } - - it 'creates services for projects' do - expect(project.pushover_service).to be_nil - - described_class.propagate(service_template) - - expect(project.reload.pushover_service).to be_present - end - - it 'creates services for a project that has another service' do - BambooService.create( - template: true, - active: true, - project: project, - properties: { - bamboo_url: 'http://gitlab.com', - username: 'mic', - password: 'password', - build_key: 'build' - } - ) - - expect(project.pushover_service).to be_nil - - described_class.propagate(service_template) - - expect(project.reload.pushover_service).to be_present - end - - it 'does not create the service if it exists already' do - other_service = BambooService.create( - template: true, - active: true, - properties: { - bamboo_url: 'http://gitlab.com', - username: 'mic', - password: 'password', - build_key: 'build' - } - ) - - Service.build_from_integration(project.id, service_template).save! - Service.build_from_integration(project.id, other_service).save! - - expect { described_class.propagate(service_template) } - .not_to change { Service.count } - end - - it 'creates the service containing the template attributes' 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 - let(:project_total) { 5 } - - before do - stub_const 'Projects::PropagateServiceTemplate::BATCH_SIZE', 3 - - project_total.times { create(:project) } - - described_class.propagate(service_template) - end - - it 'creates services for all projects' do - expect(Service.all.reload.count).to eq(project_total + 2) - end - end - - describe 'external tracker' do - it 'updates the project external tracker' do - service_template.update!(category: 'issue_tracker') - - expect { described_class.propagate(service_template) } - .to change { project.reload.has_external_issue_tracker }.to(true) - end - end - - describe 'external wiki' do - it 'updates the project external tracker' do - service_template.update!(type: 'ExternalWikiService') - - expect { described_class.propagate(service_template) } - .to change { project.reload.has_external_wiki }.to(true) - end - end - end -end diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb index 3362b333c6e..a0e83fb4a21 100644 --- a/spec/services/projects/transfer_service_spec.rb +++ b/spec/services/projects/transfer_service_spec.rb @@ -5,8 +5,8 @@ require 'spec_helper' RSpec.describe Projects::TransferService do include GitHelpers - let(:user) { create(:user) } - let(:group) { create(:group) } + let_it_be(:user) { create(:user) } + let_it_be(:group) { create(:group) } let(:project) { create(:project, :repository, :legacy_storage, namespace: user.namespace) } subject(:execute_transfer) { described_class.new(project, user).execute(group) } @@ -489,6 +489,29 @@ RSpec.describe Projects::TransferService do end end + context 'moving pages' do + let_it_be(:project) { create(:project, namespace: user.namespace) } + + before do + group.add_owner(user) + end + + it 'schedules a job when pages are deployed' do + project.mark_pages_as_deployed + + expect(PagesTransferWorker).to receive(:perform_async) + .with("move_project", [project.path, user.namespace.full_path, group.full_path]) + + execute_transfer + end + + it 'does not schedule a job when no pages are deployed' do + expect(PagesTransferWorker).not_to receive(:perform_async) + + execute_transfer + end + end + def rugged_config rugged_repo(project.repository).config end diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb index 6a2c55a5e55..073e2e09397 100644 --- a/spec/services/projects/unlink_fork_service_spec.rb +++ b/spec/services/projects/unlink_fork_service_spec.rb @@ -58,26 +58,6 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin expect(source.forks_count).to be_zero end - context 'when the source has LFS objects' do - let(:lfs_object) { create(:lfs_object) } - - before do - lfs_object.projects << project - end - - it 'links the fork to the lfs object before unlinking' do - subject.execute - - expect(lfs_object.projects).to include(forked_project) - end - - it 'does not fail if the lfs objects were already linked' do - lfs_object.projects << forked_project - - expect { subject.execute }.not_to raise_error - end - end - context 'when the original project was deleted' do it 'does not fail when the original project is deleted' do source = forked_project.forked_from_project @@ -152,24 +132,6 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin expect(project.forks_count).to be_zero end - context 'when given project is a fork of an unlinked parent' do - let!(:fork_of_fork) { fork_project(forked_project, user) } - let(:lfs_object) { create(:lfs_object) } - - before do - lfs_object.projects << project - end - - it 'saves lfs objects to the root project' do - # Remove parent from network - described_class.new(forked_project, user).execute - - described_class.new(fork_of_fork, user).execute - - expect(lfs_object.projects).to include(fork_of_fork) - end - end - context 'and is node with a parent' do subject { described_class.new(forked_project, user) } diff --git a/spec/services/projects/update_pages_configuration_service_spec.rb b/spec/services/projects/update_pages_configuration_service_spec.rb index 9f7ebd40df6..294de813e02 100644 --- a/spec/services/projects/update_pages_configuration_service_spec.rb +++ b/spec/services/projects/update_pages_configuration_service_spec.rb @@ -48,15 +48,6 @@ RSpec.describe Projects::UpdatePagesConfigurationService do expect(subject).to include(status: :success) end end - - context 'when an error occurs' do - it 'returns an error object' do - e = StandardError.new("Failure") - allow(service).to receive(:reload_daemon).and_raise(e) - - expect(subject).to eq(status: :error, message: "Failure", exception: e) - end - end end context 'when pages are not deployed' do diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index 374ce4f4ce2..bfb3cbb0131 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -29,8 +29,9 @@ RSpec.describe Projects::UpdatePagesService do context 'for new artifacts' do context "for a valid job" do + let!(:artifacts_archive) { create(:ci_job_artifact, file: file, job: build) } + before do - create(:ci_job_artifact, file: file, job: build) create(:ci_job_artifact, file_type: :metadata, file_format: :gzip, file: metadata, job: build) build.reload @@ -49,6 +50,7 @@ RSpec.describe Projects::UpdatePagesService do expect(project.pages_deployed?).to be_falsey expect(execute).to eq(:success) expect(project.pages_metadatum).to be_deployed + expect(project.pages_metadatum.artifacts_archive).to eq(artifacts_archive) expect(project.pages_deployed?).to be_truthy # Check that all expected files are extracted diff --git a/spec/services/projects/update_remote_mirror_service_spec.rb b/spec/services/projects/update_remote_mirror_service_spec.rb index 6785b71fcc0..1de04888e0a 100644 --- a/spec/services/projects/update_remote_mirror_service_spec.rb +++ b/spec/services/projects/update_remote_mirror_service_spec.rb @@ -3,9 +3,10 @@ require 'spec_helper' RSpec.describe Projects::UpdateRemoteMirrorService do - let(:project) { create(:project, :repository) } - let(:remote_project) { create(:forked_project_with_submodules) } - let(:remote_mirror) { create(:remote_mirror, project: project, enabled: true) } + let_it_be(:project) { create(:project, :repository, lfs_enabled: true) } + let_it_be(:remote_project) { create(:forked_project_with_submodules) } + let_it_be(:remote_mirror) { create(:remote_mirror, project: project, enabled: true) } + let(:remote_name) { remote_mirror.remote_name } subject(:service) { described_class.new(project, project.creator) } @@ -79,7 +80,6 @@ RSpec.describe Projects::UpdateRemoteMirrorService do with_them do before do allow(remote_mirror).to receive(:url).and_return(url) - allow(service).to receive(:update_mirror).with(remote_mirror).and_return(true) end it "returns expected status" do @@ -128,5 +128,63 @@ RSpec.describe Projects::UpdateRemoteMirrorService do expect(remote_mirror.last_error).to include("refs/heads/develop") end end + + context "sending lfs objects" do + let_it_be(:lfs_pointer) { create(:lfs_objects_project, project: project) } + + before do + stub_lfs_setting(enabled: true) + end + + context 'feature flag enabled' do + before do + stub_feature_flags(push_mirror_syncs_lfs: true) + end + + it 'pushes LFS objects to a HTTP repository' do + expect_next_instance_of(Lfs::PushService) do |service| + expect(service).to receive(:execute) + end + + execute! + end + + it 'does nothing to an SSH repository' do + remote_mirror.update!(url: 'ssh://example.com') + + expect_any_instance_of(Lfs::PushService).not_to receive(:execute) + + execute! + end + + it 'does nothing if LFS is disabled' do + expect(project).to receive(:lfs_enabled?) { false } + + expect_any_instance_of(Lfs::PushService).not_to receive(:execute) + + execute! + end + + it 'does nothing if non-password auth is specified' do + remote_mirror.update!(auth_method: 'ssh_public_key') + + expect_any_instance_of(Lfs::PushService).not_to receive(:execute) + + execute! + end + end + + context 'feature flag disabled' do + before do + stub_feature_flags(push_mirror_syncs_lfs: false) + end + + it 'does nothing' do + expect_any_instance_of(Lfs::PushService).not_to receive(:execute) + + execute! + end + end + end end end diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 4a613f42556..7832d727220 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -272,7 +272,7 @@ RSpec.describe Projects::UpdateService do result = update_project(project, user, project_feature_attributes: { issues_access_level: ProjectFeature::PRIVATE } - ) + ) expect(result).to eq({ status: :success }) expect(project.project_feature.issues_access_level).to be(ProjectFeature::PRIVATE) @@ -325,20 +325,9 @@ RSpec.describe Projects::UpdateService do expect(project.errors.messages[:base]).to include('There is already a repository with that name on disk') end - it 'renames the project without upgrading it' do - result = update_project(project, admin, path: 'new-path') - - expect(result).not_to include(status: :error) - expect(project).to be_valid - expect(project.errors).to be_empty - expect(project.disk_path).to include('new-path') - expect(project.reload.hashed_storage?(:repository)).to be_falsey - end - context 'when hashed storage is enabled' do before do stub_application_setting(hashed_storage_enabled: true) - stub_feature_flags(skip_hashed_storage_upgrade: false) end it 'migrates project to a hashed storage instead of renaming the repo to another legacy name' do @@ -349,22 +338,6 @@ RSpec.describe Projects::UpdateService do expect(project.errors).to be_empty expect(project.reload.hashed_storage?(:repository)).to be_truthy end - - context 'when skip_hashed_storage_upgrade feature flag is enabled' do - before do - stub_feature_flags(skip_hashed_storage_upgrade: true) - end - - it 'renames the project without upgrading it' do - result = update_project(project, admin, path: 'new-path') - - expect(result).not_to include(status: :error) - expect(project).to be_valid - expect(project.errors).to be_empty - expect(project.disk_path).to include('new-path') - expect(project.reload.hashed_storage?(:repository)).to be_falsey - end - end end end @@ -412,32 +385,6 @@ RSpec.describe Projects::UpdateService do subject end - - context 'when `async_update_pages_config` is disabled' do - before do - stub_feature_flags(async_update_pages_config: false) - end - - it 'calls Projects::UpdatePagesConfigurationService when pages are deployed' do - project.mark_pages_as_deployed - - expect(Projects::UpdatePagesConfigurationService) - .to receive(:new) - .with(project) - .and_call_original - - subject - end - - it "does not update pages config when pages aren't deployed" do - project.mark_pages_as_not_deployed - - expect(Projects::UpdatePagesConfigurationService) - .not_to receive(:new) - - subject - end - end end context 'when updating #pages_https_only', :https_pages_enabled do @@ -532,14 +479,14 @@ RSpec.describe Projects::UpdateService do attributes_for(:prometheus_service, project: project, properties: { api_url: "http://new.prometheus.com", manual_configuration: "0" } - ) + ) end let!(:prometheus_service) do create(:prometheus_service, project: project, properties: { api_url: "http://old.prometheus.com", manual_configuration: "0" } - ) + ) end it 'updates existing record' do @@ -556,7 +503,7 @@ RSpec.describe Projects::UpdateService do attributes_for(:prometheus_service, project: project, properties: { api_url: "http://example.prometheus.com", manual_configuration: "0" } - ) + ) end it 'creates new record' do @@ -572,7 +519,7 @@ RSpec.describe Projects::UpdateService do attributes_for(:prometheus_service, project: project, properties: { api_url: nil, manual_configuration: "1" } - ) + ) end it 'does not create new record' do |