diff options
Diffstat (limited to 'spec/services/projects')
18 files changed, 659 insertions, 536 deletions
diff --git a/spec/services/projects/after_import_service_spec.rb b/spec/services/projects/after_import_service_spec.rb deleted file mode 100644 index a16aec891a9..00000000000 --- a/spec/services/projects/after_import_service_spec.rb +++ /dev/null @@ -1,131 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::AfterImportService do - include GitHelpers - - subject { described_class.new(project) } - - let(:project) { create(:project, :repository) } - let(:repository) { project.repository } - let(:sha) { project.commit.sha } - let(:housekeeping_service) { double(:housekeeping_service) } - - describe '#execute' do - before do - allow(Repositories::HousekeepingService) - .to receive(:new).with(project).and_return(housekeeping_service) - - allow(housekeeping_service) - .to receive(:execute).and_yield - - allow(housekeeping_service).to receive(:increment!) - end - - it 'performs housekeeping' do - subject.execute - - expect(housekeeping_service).to have_received(:execute) - end - - context 'with some refs in refs/pull/**/*' do - before do - repository.write_ref('refs/pull/1/head', sha) - repository.write_ref('refs/pull/1/merge', sha) - - subject.execute - end - - it 'removes refs/pull/**/*' do - expect(rugged.references.map(&:name)) - .not_to include(%r{\Arefs/pull/}) - end - end - - Repository::RESERVED_REFS_NAMES.each do |name| - context "with a ref in refs/#{name}/tmp" do - before do - repository.write_ref("refs/#{name}/tmp", sha) - - subject.execute - end - - it "does not remove refs/#{name}/tmp" do - expect(rugged.references.map(&:name)) - .to include("refs/#{name}/tmp") - end - end - end - - context 'when after import action throw non-retriable exception' do - let(:exception) { StandardError.new('after import error') } - - before do - allow(repository) - .to receive(:delete_all_refs_except) - .and_raise(exception) - end - - it 'throws after import error' do - expect { subject.execute }.to raise_exception('after import error') - end - end - - context 'when housekeeping service lease is taken' do - let(:exception) { Repositories::HousekeepingService::LeaseTaken.new } - - it 'logs the error message' do - allow_next_instance_of(Repositories::HousekeepingService) do |instance| - expect(instance).to receive(:execute).and_raise(exception) - end - - expect(Gitlab::Import::Logger).to receive(:info).with( - { - message: 'Project housekeeping failed', - project_full_path: project.full_path, - project_id: project.id, - 'error.message' => exception.to_s - }).and_call_original - - subject.execute - end - end - - context 'when after import action throw retriable exception one time' do - let(:exception) { GRPC::DeadlineExceeded.new } - - before do - expect(repository) - .to receive(:delete_all_refs_except) - .and_raise(exception) - expect(repository) - .to receive(:delete_all_refs_except) - .and_call_original - - subject.execute - end - - it 'removes refs/pull/**/*' do - expect(rugged.references.map(&:name)) - .not_to include(%r{\Arefs/pull/}) - end - - it 'records the failures in the database', :aggregate_failures do - import_failure = ImportFailure.last - - expect(import_failure.source).to eq('delete_all_refs') - expect(import_failure.project_id).to eq(project.id) - expect(import_failure.relation_key).to be_nil - expect(import_failure.relation_index).to be_nil - expect(import_failure.exception_class).to eq('GRPC::DeadlineExceeded') - expect(import_failure.exception_message).to be_present - expect(import_failure.correlation_id_value).not_to be_empty - end - end - - def rugged - rugged_repo(repository) - end - end -end diff --git a/spec/services/projects/android_target_platform_detector_service_spec.rb b/spec/services/projects/android_target_platform_detector_service_spec.rb new file mode 100644 index 00000000000..74fd320bb48 --- /dev/null +++ b/spec/services/projects/android_target_platform_detector_service_spec.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::AndroidTargetPlatformDetectorService do + let_it_be(:project) { build(:project) } + + subject { described_class.new(project).execute } + + before do + allow(Gitlab::FileFinder).to receive(:new) { finder } + end + + context 'when project is not an Android project' do + let(:finder) { instance_double(Gitlab::FileFinder, find: []) } + + it { is_expected.to be_nil } + end + + context 'when project is an Android project' do + let(:finder) { instance_double(Gitlab::FileFinder) } + + before do + query = described_class::MANIFEST_FILE_SEARCH_QUERY + allow(finder).to receive(:find).with(query) { [instance_double(Gitlab::Search::FoundBlob)] } + end + + it { is_expected.to eq :android } + end +end diff --git a/spec/services/projects/batch_open_issues_count_service_spec.rb b/spec/services/projects/batch_open_issues_count_service_spec.rb index 17bd5f7a37b..89a4abbf9c9 100644 --- a/spec/services/projects/batch_open_issues_count_service_spec.rb +++ b/spec/services/projects/batch_open_issues_count_service_spec.rb @@ -5,7 +5,6 @@ require 'spec_helper' RSpec.describe Projects::BatchOpenIssuesCountService do let!(:project_1) { create(:project) } let!(:project_2) { create(:project) } - let!(:banned_user) { create(:user, :banned) } let(:subject) { described_class.new([project_1, project_2]) } @@ -13,41 +12,32 @@ RSpec.describe Projects::BatchOpenIssuesCountService do before do create(:issue, project: project_1) create(:issue, project: project_1, confidential: true) - create(:issue, project: project_1, author: banned_user) + create(:issue, project: project_2) create(:issue, project: project_2, confidential: true) - create(:issue, project: project_2, author: banned_user) end - context 'when cache is clean', :aggregate_failures do + context 'when cache is clean' do it 'refreshes cache keys correctly' do - expect(get_cache_key(project_1)).to eq(nil) - expect(get_cache_key(project_2)).to eq(nil) - - subject.count_service.new(project_1).refresh_cache - subject.count_service.new(project_2).refresh_cache - - expect(get_cache_key(project_1)).to eq(1) - expect(get_cache_key(project_2)).to eq(1) + subject.refresh_cache_and_retrieve_data - expect(get_cache_key(project_1, true)).to eq(2) - expect(get_cache_key(project_2, true)).to eq(2) + # It does not update total issues cache + expect(Rails.cache.read(get_cache_key(subject, project_1))).to eq(nil) + expect(Rails.cache.read(get_cache_key(subject, project_2))).to eq(nil) - expect(get_cache_key(project_1, true, true)).to eq(3) - expect(get_cache_key(project_2, true, true)).to eq(3) + expect(Rails.cache.read(get_cache_key(subject, project_1, true))).to eq(1) + expect(Rails.cache.read(get_cache_key(subject, project_1, true))).to eq(1) end end end - def get_cache_key(project, with_confidential = false, with_hidden = false) + def get_cache_key(subject, project, public_key = false) service = subject.count_service.new(project) - if with_confidential && with_hidden - Rails.cache.read(service.cache_key(service.class::TOTAL_COUNT_KEY)) - elsif with_confidential - Rails.cache.read(service.cache_key(service.class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY)) + if public_key + service.cache_key(service.class::PUBLIC_COUNT_KEY) else - Rails.cache.read(service.cache_key(service.class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY)) + service.cache_key(service.class::TOTAL_COUNT_KEY) end end end diff --git a/spec/services/projects/blame_service_spec.rb b/spec/services/projects/blame_service_spec.rb new file mode 100644 index 00000000000..40b2bc869dc --- /dev/null +++ b/spec/services/projects/blame_service_spec.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::BlameService, :aggregate_failures do + subject(:service) { described_class.new(blob, commit, params) } + + let_it_be(:project) { create(:project, :repository) } + let_it_be(:commit) { project.repository.commit } + let_it_be(:blob) { project.repository.blob_at('HEAD', 'README.md') } + + let(:params) { { page: page } } + let(:page) { nil } + + before do + stub_const("#{described_class.name}::PER_PAGE", 2) + end + + describe '#blame' do + subject { service.blame } + + it 'returns a correct Gitlab::Blame object' do + is_expected.to be_kind_of(Gitlab::Blame) + + expect(subject.blob).to eq(blob) + expect(subject.commit).to eq(commit) + expect(subject.range).to eq(1..2) + end + + describe 'Pagination range calculation' do + subject { service.blame.range } + + context 'with page = 1' do + let(:page) { 1 } + + it { is_expected.to eq(1..2) } + end + + context 'with page = 2' do + let(:page) { 2 } + + it { is_expected.to eq(3..4) } + end + + context 'with page = 3 (overlimit)' do + let(:page) { 3 } + + it { is_expected.to eq(1..2) } + end + + context 'with page = 0 (incorrect)' do + let(:page) { 0 } + + it { is_expected.to eq(1..2) } + end + + context 'when feature flag disabled' do + before do + stub_feature_flags(blame_page_pagination: false) + end + + it { is_expected.to be_nil } + end + end + end + + describe '#pagination' do + subject { service.pagination } + + it 'returns a pagination object' do + is_expected.to be_kind_of(Kaminari::PaginatableArray) + + expect(subject.current_page).to eq(1) + expect(subject.total_pages).to eq(2) + expect(subject.total_count).to eq(4) + end + + context 'when feature flag disabled' do + before do + stub_feature_flags(blame_page_pagination: false) + end + + it { is_expected.to be_nil } + end + + context 'when per_page is above the global max per page limit' do + before do + stub_const("#{described_class.name}::PER_PAGE", 1000) + allow(blob).to receive_message_chain(:data, :lines, :count) { 500 } + end + + it 'returns a correct pagination object' do + is_expected.to be_kind_of(Kaminari::PaginatableArray) + + expect(subject.current_page).to eq(1) + expect(subject.total_pages).to eq(1) + expect(subject.total_count).to eq(500) + end + end + + describe 'Current page' do + subject { service.pagination.current_page } + + context 'with page = 1' do + let(:page) { 1 } + + it { is_expected.to eq(1) } + end + + context 'with page = 2' do + let(:page) { 2 } + + it { is_expected.to eq(2) } + end + + context 'with page = 3 (overlimit)' do + let(:page) { 3 } + + it { is_expected.to eq(1) } + end + + context 'with page = 0 (incorrect)' do + let(:page) { 0 } + + it { is_expected.to eq(1) } + end + end + end +end diff --git a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb index 86c0ba4222c..79904e2bf72 100644 --- a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb @@ -34,8 +34,6 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_ stub_digest_config('sha256:configB', 5.days.ago) stub_digest_config('sha256:configC', 1.month.ago) stub_digest_config('sha256:configD', nil) - - stub_feature_flags(container_registry_expiration_policies_throttling: false) end describe '#execute' do @@ -334,24 +332,17 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_ end end - where(:feature_flag_enabled, :max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do - false | 10 | :success | :success | false - false | 10 | :error | :error | false - false | 3 | :success | :success | false - false | 3 | :error | :error | false - false | 0 | :success | :success | false - false | 0 | :error | :error | false - true | 10 | :success | :success | false - true | 10 | :error | :error | false - true | 3 | :success | :error | true - true | 3 | :error | :error | true - true | 0 | :success | :success | false - true | 0 | :error | :error | false + where(:max_list_size, :delete_tags_service_status, :expected_status, :expected_truncated) do + 10 | :success | :success | false + 10 | :error | :error | false + 3 | :success | :error | true + 3 | :error | :error | true + 0 | :success | :success | false + 0 | :error | :error | false end with_them do before do - stub_feature_flags(container_registry_expiration_policies_throttling: feature_flag_enabled) stub_application_setting(container_registry_cleanup_tags_service_max_list_size: max_list_size) allow_next_instance_of(Projects::ContainerRepository::DeleteTagsService) do |service| expect(service).to receive(:execute).and_return(status: delete_tags_service_status) @@ -429,10 +420,10 @@ RSpec.describe Projects::ContainerRepository::CleanupTagsService, :clean_gitlab_ end # We will ping the container registry for all tags *except* for C because it's cached - expect(ContainerRegistry::Blob).to receive(:new).with(repository, "digest" => "sha256:configA").and_call_original - expect(ContainerRegistry::Blob).to receive(:new).with(repository, "digest" => "sha256:configB").twice.and_call_original - expect(ContainerRegistry::Blob).not_to receive(:new).with(repository, "digest" => "sha256:configC") - expect(ContainerRegistry::Blob).to receive(:new).with(repository, "digest" => "sha256:configD").and_call_original + expect(ContainerRegistry::Blob).to receive(:new).with(repository, { "digest" => "sha256:configA" }).and_call_original + expect(ContainerRegistry::Blob).to receive(:new).with(repository, { "digest" => "sha256:configB" }).twice.and_call_original + expect(ContainerRegistry::Blob).not_to receive(:new).with(repository, { "digest" => "sha256:configC" }) + expect(ContainerRegistry::Blob).to receive(:new).with(repository, { "digest" => "sha256:configD" }).and_call_original expect(subject).to include(cached_tags_count: 1) end 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 246ca301cfa..9e6849aa514 100644 --- a/spec/services/projects/container_repository/delete_tags_service_spec.rb +++ b/spec/services/projects/container_repository/delete_tags_service_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' RSpec.describe Projects::ContainerRepository::DeleteTagsService do + using RSpec::Parameterized::TableSyntax include_context 'container repository delete tags service shared context' let(:service) { described_class.new(project, user, params) } @@ -17,11 +18,13 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do shared_examples 'logging a success response' do it 'logs an info message' do expect(service).to receive(:log_info).with( - service_class: 'Projects::ContainerRepository::DeleteTagsService', - message: 'deleted tags', - container_repository_id: repository.id, - project_id: repository.project_id, - deleted_tags_count: tags.size + { + service_class: 'Projects::ContainerRepository::DeleteTagsService', + message: 'deleted tags', + container_repository_id: repository.id, + project_id: repository.project_id, + deleted_tags_count: tags.size + } ) subject @@ -131,10 +134,6 @@ 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 @@ -157,11 +156,39 @@ RSpec.describe Projects::ContainerRepository::DeleteTagsService do end context 'when the repository is importing' do - before do - repository.update_columns(migration_state: 'importing', migration_import_started_at: Time.zone.now) + where(:migration_state, :called_by_policy, :error_expected) do + 'default' | false | false + 'default' | true | false + 'pre_importing' | false | false + 'pre_importing' | true | true + 'pre_import_done' | false | false + 'pre_import_done' | true | true + 'importing' | false | true + 'importing' | true | true + 'import_done' | false | false + 'import_done' | true | false + 'import_aborted' | false | false + 'import_aborted' | true | false + 'import_skipped' | false | false + 'import_skipped' | true | false end - it { is_expected.to include(status: :error, message: 'repository importing') } + with_them do + let(:params) { { tags: tags, container_expiration_policy: called_by_policy ? true : nil } } + + before do + repository.update_columns(migration_state: migration_state, migration_import_started_at: Time.zone.now, migration_pre_import_started_at: Time.zone.now, migration_pre_import_done_at: Time.zone.now) + end + + it 'returns an error response if expected' do + if error_expected + expect(subject).to include(status: :error, message: 'repository importing') + else + expect(service).to receive(:delete_tags).and_return(status: :success) + expect(subject).not_to include(status: :error) + end + end + end end end 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 74f782538c5..8d8907119f0 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,10 +12,6 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do subject { service.execute } - 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) @@ -26,6 +22,8 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do end context 'with tags to delete' do + let(:timeout) { 10 } + it_behaves_like 'deleting tags' it 'succeeds when tag delete returns 404' do @@ -50,59 +48,52 @@ RSpec.describe Projects::ContainerRepository::Gitlab::DeleteTagsService do 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' + before do + stub_application_setting(container_registry_delete_tags_service_timeout: timeout) + end - 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 + 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 eq(status: :error, message: 'error while deleting tags', deleted: ['A'], exception_class_name: Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError.name) } + it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: ['A'], exception_class_name: Projects::ContainerRepository::Gitlab::DeleteTagsService::TimeoutError.name) } - 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) + 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 + subject end + end - context 'set to 0' do - let(:timeout) { 0 } + context 'set to 0' do + let(:timeout) { 0 } - it_behaves_like 'deleting tags' - end + it_behaves_like 'deleting tags' + end - context 'set to nil' do - let(:timeout) { nil } + context 'set to nil' do + let(:timeout) { nil } - it_behaves_like 'deleting tags' - end + it_behaves_like 'deleting tags' end + end - context 'with a network error' do - before do - expect(service).to receive(:delete_tags).and_raise(::Faraday::TimeoutError) - end + context 'with a network error' do + before do + expect(service).to receive(:delete_tags).and_raise(::Faraday::TimeoutError) + end - it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: [], exception_class_name: ::Faraday::TimeoutError.name) } + it { is_expected.to eq(status: :error, message: 'error while deleting tags', deleted: [], exception_class_name: ::Faraday::TimeoutError.name) } - it 'tracks the exception' do - expect(::Gitlab::ErrorTracking) - .to receive(:track_exception).with(::Faraday::TimeoutError, tags_count: tags.size, container_repository_id: repository.id) + it 'tracks the exception' do + expect(::Gitlab::ErrorTracking) + .to receive(:track_exception).with(::Faraday::TimeoutError, tags_count: tags.size, container_repository_id: repository.id) - subject - end + subject end end end diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index c5c5af3cb01..cd1e629e1d2 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -141,18 +141,6 @@ RSpec.describe Projects::CreateService, '#execute' do expect(project.project_setting).to be_persisted end - context 'create_project_settings feature flag is disabled' do - before do - stub_feature_flags(create_project_settings: false) - end - - it 'builds associated project settings' do - project = create_project(user, opts) - - expect(project.project_setting).to be_new_record - end - end - it_behaves_like 'storing arguments in the application context' do let(:expected_params) { { project: subject.full_path } } @@ -780,6 +768,45 @@ RSpec.describe Projects::CreateService, '#execute' do create_project(user, opts) end + context 'when import source is enabled' do + before do + stub_application_setting(import_sources: ['github']) + end + + it 'does not raise an error when import_source is string' do + opts[:import_type] = 'github' + + project = create_project(user, opts) + + expect(project).to be_persisted + expect(project.errors).to be_blank + end + + it 'does not raise an error when import_source is symbol' do + opts[:import_type] = :github + + project = create_project(user, opts) + + expect(project).to be_persisted + expect(project.errors).to be_blank + end + end + + context 'when import source is disabled' do + before do + stub_application_setting(import_sources: []) + opts[:import_type] = 'git' + end + + it 'raises an error' do + project = create_project(user, opts) + + expect(project).to respond_to(:errors) + expect(project.errors).to have_key(:import_source_disabled) + expect(project.saved?).to be_falsey + end + end + context 'with external authorization enabled' do before do enable_external_authorization_service_check @@ -797,7 +824,7 @@ RSpec.describe Projects::CreateService, '#execute' do it 'saves the project when the user has access to the label' do expect(::Gitlab::ExternalAuthorization) - .to receive(:access_allowed?).with(user, 'new-label', any_args) { true } + .to receive(:access_allowed?).with(user, 'new-label', any_args) { true }.at_least(1).time project = create_project(user, opts.merge({ external_authorization_classification_label: 'new-label' })) diff --git a/spec/services/projects/group_links/create_service_spec.rb b/spec/services/projects/group_links/create_service_spec.rb index 4ea5f2b3a53..65d3085a850 100644 --- a/spec/services/projects/group_links/create_service_spec.rb +++ b/spec/services/projects/group_links/create_service_spec.rb @@ -5,65 +5,104 @@ require 'spec_helper' RSpec.describe Projects::GroupLinks::CreateService, '#execute' do let_it_be(:user) { create :user } let_it_be(:group) { create :group } - let_it_be(:project) { create :project } + let_it_be(:project) { create(:project, namespace: create(:namespace, :with_namespace_settings)) } - let(:group_access) { Gitlab::Access::DEVELOPER } let(:opts) do { - link_group_access: group_access, + link_group_access: Gitlab::Access::DEVELOPER, expires_at: nil } end - subject { described_class.new(project, user, opts) } + subject { described_class.new(project, group, user, opts) } - before do - group.add_developer(user) - end + shared_examples_for 'not shareable' do + it 'does not share and returns an error' do + expect do + result = subject.execute - it 'adds group to project' do - expect { subject.execute(group) }.to change { project.project_group_links.count }.from(0).to(1) + expect(result[:status]).to eq(:error) + expect(result[:http_status]).to eq(404) + end.not_to change { project.project_group_links.count } + end end - it 'updates authorization', :sidekiq_inline do - expect { subject.execute(group) }.to( - change { Ability.allowed?(user, :read_project, project) } - .from(false).to(true)) - end + shared_examples_for 'shareable' do + it 'adds group to project' do + expect do + result = subject.execute - it 'returns false if group is blank' do - expect { subject.execute(nil) }.not_to change { project.project_group_links.count } + expect(result[:status]).to eq(:success) + end.to change { project.project_group_links.count }.from(0).to(1) + end end - it 'returns error if user is not allowed to share with a group' do - expect { subject.execute(create(:group)) }.not_to change { project.project_group_links.count } - end + context 'when user has proper membership to share a group' do + before do + group.add_guest(user) + end - context 'with specialized project_authorization workers' do - let_it_be(:other_user) { create(:user) } + it_behaves_like 'shareable' - before do - group.add_developer(other_user) + it 'updates authorization', :sidekiq_inline do + expect { subject.execute }.to( + change { Ability.allowed?(user, :read_project, project) } + .from(false).to(true)) + end + + context 'with specialized project_authorization workers' do + let_it_be(:other_user) { create(:user) } + + before do + group.add_developer(other_user) + end + + it 'schedules authorization update for users with access to group' do + expect(AuthorizedProjectsWorker).not_to( + receive(:bulk_perform_async) + ) + expect(AuthorizedProjectUpdate::ProjectRecalculateWorker).to( + receive(:perform_async) + .with(project.id) + .and_call_original + ) + expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( + receive(:bulk_perform_in) + .with(1.hour, + array_including([user.id], [other_user.id]), + batch_delay: 30.seconds, batch_size: 100) + .and_call_original + ) + + subject.execute + end end - it 'schedules authorization update for users with access to group' do - expect(AuthorizedProjectsWorker).not_to( - receive(:bulk_perform_async) - ) - expect(AuthorizedProjectUpdate::ProjectRecalculateWorker).to( - receive(:perform_async) - .with(project.id) - .and_call_original - ) - expect(AuthorizedProjectUpdate::UserRefreshFromReplicaWorker).to( - receive(:bulk_perform_in) - .with(1.hour, - array_including([user.id], [other_user.id]), - batch_delay: 30.seconds, batch_size: 100) - .and_call_original - ) - - subject.execute(group) + context 'when sharing outside the hierarchy is disabled' do + let_it_be(:shared_group_parent) do + create(:group, + namespace_settings: create(:namespace_settings, prevent_sharing_groups_outside_hierarchy: true)) + end + + let_it_be(:project, reload: true) { create(:project, group: shared_group_parent) } + + it_behaves_like 'not shareable' + + context 'when group is inside hierarchy' do + let(:group) { create(:group, :private, parent: shared_group_parent) } + + it_behaves_like 'shareable' + end end end + + context 'when user does not have permissions for the group' do + it_behaves_like 'not shareable' + end + + context 'when group is blank' do + let(:group) { nil } + + it_behaves_like 'not shareable' + end end diff --git a/spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb b/spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb new file mode 100644 index 00000000000..4c51c8a4ac8 --- /dev/null +++ b/spec/services/projects/in_product_marketing_campaign_emails_service_spec.rb @@ -0,0 +1,130 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::InProductMarketingCampaignEmailsService do + describe '#execute' do + let(:user) { create(:user, email_opted_in: true) } + let(:project) { create(:project) } + let(:campaign) { Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE } + + subject(:execute) do + described_class.new(project, campaign).execute + end + + context 'users can receive marketing emails' do + let(:owner) { create(:user, email_opted_in: true) } + let(:maintainer) { create(:user, email_opted_in: true) } + let(:developer) { create(:user, email_opted_in: true) } + + before do + project.add_owner(owner) + project.add_developer(developer) + project.add_maintainer(maintainer) + end + + it 'sends the email to all project members with access_level >= Developer', :aggregate_failures do + double = instance_double(ActionMailer::MessageDelivery, deliver_later: true) + + [owner, maintainer, developer].each do |user| + email = user.notification_email_or_default + + expect(Notify).to receive(:build_ios_app_guide_email).with(email) { double } + expect(double).to receive(:deliver_later) + end + + execute + end + + it 'records sent emails', :aggregate_failures do + expect { execute }.to change { Users::InProductMarketingEmail.count }.from(0).to(3) + + [owner, maintainer, developer].each do |user| + expect( + Users::InProductMarketingEmail.where( + user: user, + campaign: campaign + ) + ).to exist + end + end + + it 'tracks experiment :email_sent event', :experiment do + expect(experiment(:build_ios_app_guide_email)).to track(:email_sent) + .on_next_instance + .with_context(project: project) + + execute + end + end + + shared_examples 'does nothing' do + it 'does not send the email' do + email = user.notification_email_or_default + expect(Notify).not_to receive(:build_ios_app_guide_email).with(email) + execute + end + + it 'does not create a record of the sent email' do + expect { execute }.not_to change { Users::InProductMarketingEmail.count } + end + end + + context "when user can't receive marketing emails" do + before do + project.add_developer(user) + end + + context 'when user.can?(:receive_notifications) is false' do + it 'does not send the email' do + allow_next_found_instance_of(User) do |user| + allow(user).to receive(:can?).with(:receive_notifications) { false } + + email = user.notification_email_or_default + expect(Notify).not_to receive(:build_ios_app_guide_email).with(email) + + expect( + Users::InProductMarketingEmail.where( + user: user, + campaign: campaign + ) + ).not_to exist + end + + execute + end + end + + context 'when user is not opted in to receive marketing emails' do + let(:user) { create(:user, email_opted_in: false) } + + it_behaves_like 'does nothing' + end + end + + context 'when campaign email has already been sent to the user' do + before do + project.add_developer(user) + create(:in_product_marketing_email, :campaign, user: user, campaign: campaign) + end + + it_behaves_like 'does nothing' + end + + context "when user is a reporter" do + before do + project.add_reporter(user) + end + + it_behaves_like 'does nothing' + end + + context "when user is a guest" do + before do + project.add_guest(user) + end + + it_behaves_like 'does nothing' + end + end +end diff --git a/spec/services/projects/open_issues_count_service_spec.rb b/spec/services/projects/open_issues_count_service_spec.rb index 8710d0c0267..c739fea5ecf 100644 --- a/spec/services/projects/open_issues_count_service_spec.rb +++ b/spec/services/projects/open_issues_count_service_spec.rb @@ -4,102 +4,89 @@ require 'spec_helper' RSpec.describe Projects::OpenIssuesCountService, :use_clean_rails_memory_store_caching do let(:project) { create(:project) } - let(:user) { create(:user) } - let(:banned_user) { create(:user, :banned) } - subject { described_class.new(project, user) } + subject { described_class.new(project) } it_behaves_like 'a counter caching service' - before do - create(:issue, :opened, project: project) - create(:issue, :opened, confidential: true, project: project) - create(:issue, :opened, author: banned_user, project: project) - create(:issue, :closed, project: project) - - described_class.new(project).refresh_cache - end - describe '#count' do - shared_examples 'counts public issues, does not count hidden or confidential' do - it 'counts only public issues' do - expect(subject.count).to eq(1) - end - - it 'uses PUBLIC_COUNT_WITHOUT_HIDDEN_KEY cache key' do - expect(subject.cache_key).to include('project_open_public_issues_without_hidden_count') - end - end - context 'when user is nil' do - let(:user) { nil } + it 'does not include confidential issues in the issue count' do + create(:issue, :opened, project: project) + create(:issue, :opened, confidential: true, project: project) - it_behaves_like 'counts public issues, does not count hidden or confidential' + expect(described_class.new(project).count).to eq(1) + end end context 'when user is provided' do + let(:user) { create(:user) } + context 'when user can read confidential issues' do before do project.add_reporter(user) end - it 'includes confidential issues and does not include hidden issues in count' do - expect(subject.count).to eq(2) + it 'returns the right count with confidential issues' do + create(:issue, :opened, project: project) + create(:issue, :opened, confidential: true, project: project) + + expect(described_class.new(project, user).count).to eq(2) end - it 'uses TOTAL_COUNT_WITHOUT_HIDDEN_KEY cache key' do - expect(subject.cache_key).to include('project_open_issues_without_hidden_count') + it 'uses total_open_issues_count cache key' do + expect(described_class.new(project, user).cache_key_name).to eq('total_open_issues_count') end end - context 'when user cannot read confidential or hidden issues' do + context 'when user cannot read confidential issues' do before do project.add_guest(user) end - it_behaves_like 'counts public issues, does not count hidden or confidential' - end - - context 'when user is an admin' do - let_it_be(:user) { create(:user, :admin) } - - context 'when admin mode is enabled', :enable_admin_mode do - it 'includes confidential and hidden issues in count' do - expect(subject.count).to eq(3) - end + it 'does not include confidential issues' do + create(:issue, :opened, project: project) + create(:issue, :opened, confidential: true, project: project) - it 'uses TOTAL_COUNT_KEY cache key' do - expect(subject.cache_key).to include('project_open_issues_including_hidden_count') - end + expect(described_class.new(project, user).count).to eq(1) end - context 'when admin mode is disabled' do - it_behaves_like 'counts public issues, does not count hidden or confidential' + it 'uses public_open_issues_count cache key' do + expect(described_class.new(project, user).cache_key_name).to eq('public_open_issues_count') end end end - end - - describe '#refresh_cache', :aggregate_failures do - context 'when cache is empty' do - it 'refreshes cache keys correctly' do - expect(Rails.cache.read(described_class.new(project).cache_key(described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY))).to eq(1) - expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY))).to eq(2) - expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_KEY))).to eq(3) - end - end - context 'when cache is outdated' do - it 'refreshes cache keys correctly' do + describe '#refresh_cache' do + before do + create(:issue, :opened, project: project) create(:issue, :opened, project: project) create(:issue, :opened, confidential: true, project: project) - create(:issue, :opened, author: banned_user, project: project) + end - described_class.new(project).refresh_cache + context 'when cache is empty' do + it 'refreshes cache keys correctly' do + subject.refresh_cache - expect(Rails.cache.read(described_class.new(project).cache_key(described_class::PUBLIC_COUNT_WITHOUT_HIDDEN_KEY))).to eq(2) - expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_WITHOUT_HIDDEN_KEY))).to eq(4) - expect(Rails.cache.read(described_class.new(project).cache_key(described_class::TOTAL_COUNT_KEY))).to eq(6) + expect(Rails.cache.read(subject.cache_key(described_class::PUBLIC_COUNT_KEY))).to eq(2) + expect(Rails.cache.read(subject.cache_key(described_class::TOTAL_COUNT_KEY))).to eq(3) + end + end + + context 'when cache is outdated' do + before do + subject.refresh_cache + end + + it 'refreshes cache keys correctly' do + create(:issue, :opened, project: project) + create(:issue, :opened, confidential: true, project: project) + + subject.refresh_cache + + expect(Rails.cache.read(subject.cache_key(described_class::PUBLIC_COUNT_KEY))).to eq(3) + expect(Rails.cache.read(subject.cache_key(described_class::TOTAL_COUNT_KEY))).to eq(5) + end end end end diff --git a/spec/services/projects/prometheus/alerts/create_service_spec.rb b/spec/services/projects/prometheus/alerts/create_service_spec.rb deleted file mode 100644 index 6b9d43e4e81..00000000000 --- a/spec/services/projects/prometheus/alerts/create_service_spec.rb +++ /dev/null @@ -1,52 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::Prometheus::Alerts::CreateService do - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } - - let(:service) { described_class.new(project: project, current_user: user, params: params) } - - subject { service.execute } - - describe '#execute' do - context 'with params' do - let_it_be(:environment) { create(:environment, project: project) } - - let_it_be(:metric) do - create(:prometheus_metric, project: project) - end - - let(:params) do - { - environment_id: environment.id, - prometheus_metric_id: metric.id, - operator: '<', - threshold: 1.0 - } - end - - it 'creates an alert' do - expect(subject).to be_persisted - - expect(subject).to have_attributes( - project: project, - environment: environment, - prometheus_metric: metric, - operator: 'lt', - threshold: 1.0 - ) - end - end - - context 'without params' do - let(:params) { {} } - - it 'fails to create' do - expect(subject).to be_new_record - expect(subject).to be_invalid - end - end - end -end diff --git a/spec/services/projects/prometheus/alerts/destroy_service_spec.rb b/spec/services/projects/prometheus/alerts/destroy_service_spec.rb deleted file mode 100644 index a3e9c3516c2..00000000000 --- a/spec/services/projects/prometheus/alerts/destroy_service_spec.rb +++ /dev/null @@ -1,21 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::Prometheus::Alerts::DestroyService do - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } - let_it_be(:alert) { create(:prometheus_alert, project: project) } - - let(:service) { described_class.new(project: project, current_user: user, params: nil) } - - describe '#execute' do - subject { service.execute(alert) } - - it 'deletes the alert' do - expect(subject).to be_truthy - - expect(alert).to be_destroyed - end - end -end diff --git a/spec/services/projects/prometheus/alerts/update_service_spec.rb b/spec/services/projects/prometheus/alerts/update_service_spec.rb deleted file mode 100644 index ec6766221f6..00000000000 --- a/spec/services/projects/prometheus/alerts/update_service_spec.rb +++ /dev/null @@ -1,53 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::Prometheus::Alerts::UpdateService do - let_it_be(:project) { create(:project) } - let_it_be(:user) { create(:user) } - let_it_be(:environment) { create(:environment, project: project) } - - let_it_be(:alert) do - create(:prometheus_alert, project: project, environment: environment) - end - - let(:service) { described_class.new(project: project, current_user: user, params: params) } - - let(:params) do - { - environment_id: alert.environment_id, - prometheus_metric_id: alert.prometheus_metric_id, - operator: '==', - threshold: 2.0 - } - end - - describe '#execute' do - subject { service.execute(alert) } - - context 'with valid params' do - it 'updates the alert' do - expect(subject).to be_truthy - - expect(alert.reload).to have_attributes( - operator: 'eq', - threshold: 2.0 - ) - end - end - - context 'with invalid params' do - let(:other_environment) { create(:environment) } - - before do - params[:environment_id] = other_environment.id - end - - it 'fails to update' do - expect(subject).to be_falsey - - expect(alert).to be_invalid - end - end - end -end diff --git a/spec/services/projects/prometheus/metrics/destroy_service_spec.rb b/spec/services/projects/prometheus/metrics/destroy_service_spec.rb index 17cc88b27b6..b4af81f2c87 100644 --- a/spec/services/projects/prometheus/metrics/destroy_service_spec.rb +++ b/spec/services/projects/prometheus/metrics/destroy_service_spec.rb @@ -12,17 +12,4 @@ RSpec.describe Projects::Prometheus::Metrics::DestroyService do expect(PrometheusMetric.find_by(id: metric.id)).to be_nil end - - context 'when metric has a prometheus alert associated' do - it 'schedules a prometheus alert update' do - create(:prometheus_alert, project: metric.project, prometheus_metric: metric) - - schedule_update_service = spy - allow(::Clusters::Applications::ScheduleUpdateService).to receive(:new).and_return(schedule_update_service) - - subject.execute - - expect(schedule_update_service).to have_received(:execute) - end - end end diff --git a/spec/services/projects/prometheus/metrics/update_service_spec.rb b/spec/services/projects/prometheus/metrics/update_service_spec.rb deleted file mode 100644 index bf87093150c..00000000000 --- a/spec/services/projects/prometheus/metrics/update_service_spec.rb +++ /dev/null @@ -1,44 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Projects::Prometheus::Metrics::UpdateService do - let(:metric) { create(:prometheus_metric) } - - it 'updates the prometheus metric' do - expect do - described_class.new(metric, { title: "bar" }).execute - end.to change { metric.reload.title }.to("bar") - end - - context 'when metric has a prometheus alert associated' do - let(:schedule_update_service) { spy } - - before do - create(:prometheus_alert, project: metric.project, prometheus_metric: metric) - allow(::Clusters::Applications::ScheduleUpdateService).to receive(:new).and_return(schedule_update_service) - end - - context 'when updating title' do - it 'schedules a prometheus alert update' do - described_class.new(metric, { title: "bar" }).execute - - expect(schedule_update_service).to have_received(:execute) - end - end - - context 'when updating query' do - it 'schedules a prometheus alert update' do - described_class.new(metric, { query: "sum(bar)" }).execute - - expect(schedule_update_service).to have_received(:execute) - end - end - - it 'does not schedule a prometheus alert update without title nor query being changed' do - described_class.new(metric, { y_label: "bar" }).execute - - expect(schedule_update_service).not_to have_received(:execute) - end - end -end diff --git a/spec/services/projects/record_target_platforms_service_spec.rb b/spec/services/projects/record_target_platforms_service_spec.rb index 85311f36428..22ff325a62e 100644 --- a/spec/services/projects/record_target_platforms_service_spec.rb +++ b/spec/services/projects/record_target_platforms_service_spec.rb @@ -5,21 +5,38 @@ require 'spec_helper' RSpec.describe Projects::RecordTargetPlatformsService, '#execute' do let_it_be(:project) { create(:project) } - subject(:execute) { described_class.new(project).execute } + let(:detector_service) { Projects::AppleTargetPlatformDetectorService } + + subject(:execute) { described_class.new(project, detector_service).execute } + + context 'when detector returns target platform values' do + let(:detector_result) { [:ios, :osx] } + let(:service_result) { detector_result.map(&:to_s) } - context 'when project is an XCode project' do before do - double = instance_double(Projects::AppleTargetPlatformDetectorService, execute: [:ios, :osx]) - allow(Projects::AppleTargetPlatformDetectorService).to receive(:new) { double } + double = instance_double(detector_service, execute: detector_result) + allow(detector_service).to receive(:new) { double } end - it 'creates a new setting record for the project', :aggregate_failures do - expect { execute }.to change { ProjectSetting.count }.from(0).to(1) - expect(ProjectSetting.last.target_platforms).to match_array(%w(ios osx)) + shared_examples 'saves and returns detected target platforms' do + it 'creates a new setting record for the project', :aggregate_failures do + expect { execute }.to change { ProjectSetting.count }.from(0).to(1) + expect(ProjectSetting.last.target_platforms).to match_array(service_result) + end + + it 'returns the array of stored target platforms' do + expect(execute).to match_array service_result + end end - it 'returns array of detected target platforms' do - expect(execute).to match_array %w(ios osx) + it_behaves_like 'saves and returns detected target platforms' + + context 'when detector returns a non-array value' do + let(:detector_service) { Projects::AndroidTargetPlatformDetectorService } + let(:detector_result) { :android } + let(:service_result) { [detector_result.to_s] } + + it_behaves_like 'saves and returns detected target platforms' end context 'when a project has an existing setting record' do @@ -49,9 +66,76 @@ RSpec.describe Projects::RecordTargetPlatformsService, '#execute' do end end end + + describe 'Build iOS guide email experiment' do + shared_examples 'tracks experiment assignment event' do + it 'tracks the assignment event', :experiment do + expect(experiment(:build_ios_app_guide_email)) + .to track(:assignment) + .with_context(project: project) + .on_next_instance + + execute + end + end + + context 'experiment candidate' do + before do + stub_experiments(build_ios_app_guide_email: :candidate) + end + + it 'executes a Projects::InProductMarketingCampaignEmailsService' do + service_double = instance_double(Projects::InProductMarketingCampaignEmailsService, execute: true) + + expect(Projects::InProductMarketingCampaignEmailsService) + .to receive(:new).with(project, Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE) + .and_return service_double + expect(service_double).to receive(:execute) + + execute + end + + it_behaves_like 'tracks experiment assignment event' + end + + shared_examples 'does not send email' do + it 'does not execute a Projects::InProductMarketingCampaignEmailsService' do + expect(Projects::InProductMarketingCampaignEmailsService).not_to receive(:new) + + execute + end + end + + context 'experiment control' do + before do + stub_experiments(build_ios_app_guide_email: :control) + end + + it_behaves_like 'does not send email' + it_behaves_like 'tracks experiment assignment event' + end + + context 'when project is not an iOS project' do + let(:detector_service) { Projects::AppleTargetPlatformDetectorService } + let(:detector_result) { :android } + + before do + stub_experiments(build_ios_app_guide_email: :candidate) + end + + it_behaves_like 'does not send email' + + it 'does not track experiment assignment event', :experiment do + expect(experiment(:build_ios_app_guide_email)) + .not_to track(:assignment) + + execute + end + end + end end - context 'when project is not an XCode project' do + context 'when detector does not return any target platform values' do before do double = instance_double(Projects::AppleTargetPlatformDetectorService, execute: []) allow(Projects::AppleTargetPlatformDetectorService).to receive(:new).with(project) { double } diff --git a/spec/services/projects/update_pages_service_spec.rb b/spec/services/projects/update_pages_service_spec.rb index 6407b8d3940..777162b6196 100644 --- a/spec/services/projects/update_pages_service_spec.rb +++ b/spec/services/projects/update_pages_service_spec.rb @@ -13,6 +13,7 @@ RSpec.describe Projects::UpdatePagesService do let(:file) { fixture_file_upload("spec/fixtures/pages.zip") } let(:empty_file) { fixture_file_upload("spec/fixtures/pages_empty.zip") } + let(:empty_metadata_filename) { "spec/fixtures/pages_empty.zip.meta" } let(:metadata_filename) { "spec/fixtures/pages.zip.meta" } let(:metadata) { fixture_file_upload(metadata_filename) if File.exist?(metadata_filename) } @@ -91,6 +92,17 @@ RSpec.describe Projects::UpdatePagesService do end end + context 'when archive does not have pages directory' do + let(:file) { empty_file } + let(:metadata_filename) { empty_metadata_filename } + + it 'returns an error' do + expect(execute).not_to eq(:success) + + expect(GenericCommitStatus.last.description).to eq("Error: The `public/` folder is missing, or not declared in `.gitlab-ci.yml`.") + end + end + it 'limits pages size' do stub_application_setting(max_pages_size: 1) expect(execute).not_to eq(:success) |