diff options
Diffstat (limited to 'spec/services/repositories')
4 files changed, 59 insertions, 174 deletions
diff --git a/spec/services/repositories/changelog_service_spec.rb b/spec/services/repositories/changelog_service_spec.rb index ddb8e7e1182..82546ae810b 100644 --- a/spec/services/repositories/changelog_service_spec.rb +++ b/spec/services/repositories/changelog_service_spec.rb @@ -78,7 +78,7 @@ RSpec.describe Repositories::ChangelogService do recorder = ActiveRecord::QueryRecorder.new { service.execute(commit_to_changelog: commit_to_changelog) } changelog = project.repository.blob_at('master', 'CHANGELOG.md')&.data - expect(recorder.count).to eq(9) + expect(recorder.count).to eq(10) expect(changelog).to include('Title 1', 'Title 2') end @@ -148,6 +148,52 @@ RSpec.describe Repositories::ChangelogService do expect(changelog).to include('Title 1', 'Title 2') end end + + it 'avoids N+1 queries', :request_store do + RequestStore.clear! + + request = ->(to) do + described_class + .new(project, creator, version: '1.0.0', from: sha1, to: to) + .execute(commit_to_changelog: false) + end + + control = ActiveRecord::QueryRecorder.new { request.call(sha2) } + + RequestStore.clear! + + expect { request.call(sha3) }.not_to exceed_query_limit(control.count) + end + + context 'when one of commits does not exist' do + let(:service) { described_class.new(project, creator, version: '1.0.0', from: 'master', to: '54321') } + + it 'raises an exception' do + expect { service.execute(commit_to_changelog: false) }.to raise_error(Gitlab::Changelog::Error) + end + end + + context 'when commit range exceeds the limit' do + let(:service) { described_class.new(project, creator, version: '1.0.0', from: sha1) } + + before do + stub_const("#{described_class.name}::COMMITS_LIMIT", 2) + end + + it 'raises an exception' do + expect { service.execute(commit_to_changelog: false) }.to raise_error(Gitlab::Changelog::Error) + end + + context 'when feature flag is off' do + before do + stub_feature_flags(changelog_commits_limitation: false) + end + + it 'returns the changelog' do + expect(service.execute(commit_to_changelog: false)).to include('Title 1', 'Title 2', 'Title 3') + end + end + end end describe '#start_of_commit_range' do diff --git a/spec/services/repositories/destroy_rollback_service_spec.rb b/spec/services/repositories/destroy_rollback_service_spec.rb deleted file mode 100644 index a52dff62760..00000000000 --- a/spec/services/repositories/destroy_rollback_service_spec.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Repositories::DestroyRollbackService do - let_it_be(:user) { create(:user) } - - let!(:project) { create(:project, :repository, namespace: user.namespace) } - let(:repository) { project.repository } - let(:path) { repository.disk_path } - let(:remove_path) { "#{path}+#{project.id}#{described_class::DELETED_FLAG}" } - - subject { described_class.new(repository).execute } - - before do - # Dont run sidekiq to check if renamed repository exists - Sidekiq::Testing.fake! { destroy_project(project, user) } - end - - it 'moves the repository from the +deleted folder' do - expect(project.gitlab_shell.repository_exists?(project.repository_storage, remove_path + '.git')).to be_truthy - expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_falsey - - subject - - expect(project.gitlab_shell.repository_exists?(project.repository_storage, remove_path + '.git')).to be_falsey - expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_truthy - end - - it 'logs the successful action' do - expect(Gitlab::AppLogger).to receive(:info) - - subject - end - - it 'flushes the repository cache' do - expect(repository).to receive(:before_delete) - - subject - end - - it 'returns success and does not perform any action if repository path does not exist' do - expect(repository).to receive(:disk_path).and_return('foo') - expect(repository).not_to receive(:before_delete) - - expect(subject[:status]).to eq :success - end - - it 'gracefully handles exception if the repository does not exist on disk' do - expect(repository).to receive(:before_delete).and_raise(Gitlab::Git::Repository::NoRepository) - expect(subject[:status]).to eq :success - end - - context 'when move operation cannot be performed' do - let(:service) { described_class.new(repository) } - - before do - expect(service).to receive(:mv_repository).and_return(false) - end - - it 'returns error' do - result = service.execute - - expect(result[:status]).to eq :error - end - - it 'logs the error' do - expect(Gitlab::AppLogger).to receive(:error) - - service.execute - end - - context 'when repository does not exist' do - it 'returns success' do - allow(service).to receive(:repo_exists?).and_return(true, false) - - expect(service.execute[:status]).to eq :success - end - end - end - - def destroy_project(project, user) - Projects::DestroyService.new(project, user, {}).execute - end -end diff --git a/spec/services/repositories/destroy_service_spec.rb b/spec/services/repositories/destroy_service_spec.rb index 3766467d708..565a18d501a 100644 --- a/spec/services/repositories/destroy_service_spec.rb +++ b/spec/services/repositories/destroy_service_spec.rb @@ -8,31 +8,19 @@ RSpec.describe Repositories::DestroyService do let!(:project) { create(:project, :repository, namespace: user.namespace) } let(:repository) { project.repository } let(:path) { repository.disk_path } - let(:remove_path) { "#{path}+#{project.id}#{described_class::DELETED_FLAG}" } subject { described_class.new(repository).execute } - it 'moves the repository to a +deleted folder' do + it 'removes the repository' do expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_truthy - expect(project.gitlab_shell.repository_exists?(project.repository_storage, remove_path + '.git')).to be_falsey subject - 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_truthy - end - - it 'schedules the repository deletion' do - subject - - expect(Repositories::ShellDestroyService).to receive(:new).with(repository).and_call_original - - expect(GitlabShellWorker).to receive(:perform_in) - .with(Repositories::ShellDestroyService::REPO_REMOVAL_DELAY, :remove_repository, project.repository_storage, remove_path) - - # Because GitlabShellWorker is inside a run_after_commit callback we need to + # Because the removal happens inside a run_after_commit callback we need to # trigger the callback project.touch + + expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_falsey end context 'on a read-only instance' do @@ -41,22 +29,12 @@ RSpec.describe Repositories::DestroyService do end it 'schedules the repository deletion' do - expect(Repositories::ShellDestroyService).to receive(:new).with(repository).and_call_original - - expect(GitlabShellWorker).to receive(:perform_in) - .with(Repositories::ShellDestroyService::REPO_REMOVAL_DELAY, :remove_repository, project.repository_storage, remove_path) + expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_truthy subject - end - end - - it 'removes the repository', :sidekiq_inline do - subject - project.touch - - 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.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_falsey + end end it 'flushes the repository cache' do @@ -77,48 +55,20 @@ RSpec.describe Repositories::DestroyService do expect(subject[:status]).to eq :success end - context 'when move operation cannot be performed' do - let(:service) { described_class.new(repository) } - - before do - expect(service).to receive(:mv_repository).and_return(false) - end - - it 'returns error' do - expect(service.execute[:status]).to eq :error - end - - it 'logs the error' do - expect(Gitlab::AppLogger).to receive(:error) - - service.execute - end - - context 'when repository does not exist' do - it 'returns success' do - allow(service).to receive(:repo_exists?).and_return(true, false) - - expect(Repositories::ShellDestroyService).not_to receive(:new) - expect(service.execute[:status]).to eq :success - end - end - end - context 'with a project wiki repository' do let(:project) { create(:project, :wiki_repo) } let(:repository) { project.wiki.repository } it 'schedules the repository deletion' do - subject - - expect(Repositories::ShellDestroyService).to receive(:new).with(repository).and_call_original + expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_truthy - expect(GitlabShellWorker).to receive(:perform_in) - .with(Repositories::ShellDestroyService::REPO_REMOVAL_DELAY, :remove_repository, project.repository_storage, remove_path) + subject - # Because GitlabShellWorker is inside a run_after_commit callback we need to + # Because the removal happens inside a run_after_commit callback we need to # trigger the callback project.touch + + expect(project.gitlab_shell.repository_exists?(project.repository_storage, path + '.git')).to be_falsey end end end diff --git a/spec/services/repositories/shell_destroy_service_spec.rb b/spec/services/repositories/shell_destroy_service_spec.rb deleted file mode 100644 index 65168a1784a..00000000000 --- a/spec/services/repositories/shell_destroy_service_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Repositories::ShellDestroyService do - let_it_be(:user) { create(:user) } - - let!(:project) { create(:project, :repository, namespace: user.namespace) } - let(:path) { project.repository.disk_path } - let(:remove_path) { "#{path}+#{project.id}#{described_class::DELETED_FLAG}" } - - it 'returns success if the repository is nil' do - expect(GitlabShellWorker).not_to receive(:perform_in) - - result = described_class.new(nil).execute - - expect(result[:status]).to eq :success - end - - it 'schedules the repository deletion' do - expect(GitlabShellWorker).to receive(:perform_in) - .with(described_class::REPO_REMOVAL_DELAY, :remove_repository, project.repository_storage, remove_path) - - described_class.new(project.repository).execute - end -end |