diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-09-20 13:18:24 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-09-20 13:18:24 +0000 |
commit | 0653e08efd039a5905f3fa4f6e9cef9f5d2f799c (patch) | |
tree | 4dcc884cf6d81db44adae4aa99f8ec1233a41f55 /spec/models | |
parent | 744144d28e3e7fddc117924fef88de5d9674fe4c (diff) | |
download | gitlab-ce-0653e08efd039a5905f3fa4f6e9cef9f5d2f799c.tar.gz |
Add latest changes from gitlab-org/gitlab@14-3-stable-eev14.3.0-rc42
Diffstat (limited to 'spec/models')
79 files changed, 2952 insertions, 1430 deletions
diff --git a/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb b/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb new file mode 100644 index 00000000000..3e6d4ebd0a2 --- /dev/null +++ b/spec/models/analytics/cycle_analytics/issue_stage_event_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Analytics::CycleAnalytics::IssueStageEvent do + it { is_expected.to validate_presence_of(:stage_event_hash_id) } + it { is_expected.to validate_presence_of(:issue_id) } + it { is_expected.to validate_presence_of(:group_id) } + it { is_expected.to validate_presence_of(:project_id) } + it { is_expected.to validate_presence_of(:start_event_timestamp) } +end diff --git a/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb b/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb new file mode 100644 index 00000000000..244c5c70286 --- /dev/null +++ b/spec/models/analytics/cycle_analytics/merge_request_stage_event_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Analytics::CycleAnalytics::MergeRequestStageEvent do + it { is_expected.to validate_presence_of(:stage_event_hash_id) } + it { is_expected.to validate_presence_of(:merge_request_id) } + it { is_expected.to validate_presence_of(:group_id) } + it { is_expected.to validate_presence_of(:project_id) } + it { is_expected.to validate_presence_of(:start_event_timestamp) } +end diff --git a/spec/models/application_record_spec.rb b/spec/models/application_record_spec.rb index f9a05c720a3..efb92ddaea0 100644 --- a/spec/models/application_record_spec.rb +++ b/spec/models/application_record_spec.rb @@ -39,7 +39,7 @@ RSpec.describe ApplicationRecord do let(:suggestion_attributes) { attributes_for(:suggestion).merge!(note_id: note.id) } - shared_examples '.safe_find_or_create_by' do + describe '.safe_find_or_create_by' do it 'creates the suggestion avoiding race conditions' do existing_suggestion = double(:Suggestion) @@ -63,7 +63,7 @@ RSpec.describe ApplicationRecord do end end - shared_examples '.safe_find_or_create_by!' do + describe '.safe_find_or_create_by!' do it 'creates a record using safe_find_or_create_by' do expect(Suggestion.safe_find_or_create_by!(suggestion_attributes)) .to be_a(Suggestion) @@ -88,24 +88,6 @@ RSpec.describe ApplicationRecord do .to raise_error(ActiveRecord::RecordNotFound) end end - - context 'when optimized_safe_find_or_create_by is enabled' do - before do - stub_feature_flags(optimized_safe_find_or_create_by: true) - end - - it_behaves_like '.safe_find_or_create_by' - it_behaves_like '.safe_find_or_create_by!' - end - - context 'when optimized_safe_find_or_create_by is disabled' do - before do - stub_feature_flags(optimized_safe_find_or_create_by: false) - end - - it_behaves_like '.safe_find_or_create_by' - it_behaves_like '.safe_find_or_create_by!' - end end describe '.underscore' do @@ -164,6 +146,23 @@ RSpec.describe ApplicationRecord do end end end + + # rubocop:disable Database/MultipleDatabases + it 'increments a counter when a transaction is created in ActiveRecord' do + expect(described_class.connection.transaction_open?).to be false + + expect(::Gitlab::Database::Metrics) + .to receive(:subtransactions_increment) + .with('ActiveRecord::Base') + .once + + ActiveRecord::Base.transaction do + ActiveRecord::Base.transaction(requires_new: true) do + expect(ActiveRecord::Base.connection.transaction_open?).to be true + end + end + end + # rubocop:enable Database/MultipleDatabases end describe '.with_fast_read_statement_timeout' do @@ -236,4 +235,46 @@ RSpec.describe ApplicationRecord do end end end + + describe '.default_select_columns' do + shared_examples_for 'selects identically to the default' do + it 'generates the same sql as the default' do + expected_sql = test_model.all.to_sql + generated_sql = test_model.all.select(test_model.default_select_columns).to_sql + + expect(expected_sql).to eq(generated_sql) + end + end + + before do + ApplicationRecord.connection.execute(<<~SQL) + create table tests ( + id bigserial primary key not null, + ignore_me text + ) + SQL + end + context 'without an ignored column' do + let(:test_model) do + Class.new(ApplicationRecord) do + self.table_name = 'tests' + end + end + + it_behaves_like 'selects identically to the default' + end + + context 'with an ignored column' do + let(:test_model) do + Class.new(ApplicationRecord) do + include IgnorableColumns + self.table_name = 'tests' + + ignore_columns :ignore_me, remove_after: '2100-01-01', remove_with: '99.12' + end + end + + it_behaves_like 'selects identically to the default' + end + end end diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index e9c5ffef210..3e264867703 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -7,6 +7,8 @@ RSpec.describe ApplicationSetting do subject(:setting) { described_class.create_from_defaults } + it_behaves_like 'sanitizable', :application_setting, %i[default_branch_name] + it { include(CacheableAttributes) } it { include(ApplicationSettingImplementation) } it { expect(described_class.current_without_cache).to eq(described_class.last) } @@ -79,12 +81,19 @@ RSpec.describe ApplicationSetting do it { is_expected.to validate_numericality_of(:wiki_page_max_content_bytes).only_integer.is_greater_than_or_equal_to(1024) } it { is_expected.to validate_presence_of(:max_artifacts_size) } it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } + it { is_expected.to validate_presence_of(:max_yaml_size_bytes) } + it { is_expected.to validate_numericality_of(:max_yaml_size_bytes).only_integer.is_greater_than(0) } + it { is_expected.to validate_presence_of(:max_yaml_depth) } + it { is_expected.to validate_numericality_of(:max_yaml_depth).only_integer.is_greater_than(0) } it { is_expected.to validate_presence_of(:max_pages_size) } it 'ensures max_pages_size is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do is_expected.to validate_numericality_of(:max_pages_size).only_integer.is_greater_than_or_equal_to(0) .is_less_than(::Gitlab::Pages::MAX_SIZE / 1.megabyte) end + it { is_expected.to validate_presence_of(:jobs_per_stage_page_size) } + it { is_expected.to validate_numericality_of(:jobs_per_stage_page_size).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.not_to allow_value(7).for(:minimum_password_length) } it { is_expected.not_to allow_value(129).for(:minimum_password_length) } it { is_expected.not_to allow_value(nil).for(:minimum_password_length) } @@ -921,6 +930,8 @@ RSpec.describe ApplicationSetting do context 'throttle_* settings' do where(:throttle_setting) do %i[ + throttle_unauthenticated_api_requests_per_period + throttle_unauthenticated_api_period_in_seconds throttle_unauthenticated_requests_per_period throttle_unauthenticated_period_in_seconds throttle_authenticated_api_requests_per_period @@ -931,6 +942,12 @@ RSpec.describe ApplicationSetting do throttle_unauthenticated_packages_api_period_in_seconds throttle_authenticated_packages_api_requests_per_period throttle_authenticated_packages_api_period_in_seconds + throttle_unauthenticated_files_api_requests_per_period + throttle_unauthenticated_files_api_period_in_seconds + throttle_authenticated_files_api_requests_per_period + throttle_authenticated_files_api_period_in_seconds + throttle_authenticated_git_lfs_requests_per_period + throttle_authenticated_git_lfs_period_in_seconds ] end @@ -942,6 +959,20 @@ RSpec.describe ApplicationSetting do it { is_expected.not_to allow_value(nil).for(throttle_setting) } end end + + context 'sidekiq job limiter settings' do + it 'has the right defaults', :aggregate_failures do + expect(setting.sidekiq_job_limiter_mode).to eq('compress') + expect(setting.sidekiq_job_limiter_compression_threshold_bytes) + .to eq(Gitlab::SidekiqMiddleware::SizeLimiter::Validator::DEFAULT_COMPRESSION_THRESHOLD_BYTES) + expect(setting.sidekiq_job_limiter_limit_bytes) + .to eq(Gitlab::SidekiqMiddleware::SizeLimiter::Validator::DEFAULT_SIZE_LIMIT) + end + + it { is_expected.to allow_value('track').for(:sidekiq_job_limiter_mode) } + it { is_expected.to validate_numericality_of(:sidekiq_job_limiter_compression_threshold_bytes).only_integer.is_greater_than_or_equal_to(0) } + it { is_expected.to validate_numericality_of(:sidekiq_job_limiter_limit_bytes).only_integer.is_greater_than_or_equal_to(0) } + end end context 'restrict creating duplicates' do diff --git a/spec/models/bulk_imports/entity_spec.rb b/spec/models/bulk_imports/entity_spec.rb index 11a3e53dd16..c1cbe61885f 100644 --- a/spec/models/bulk_imports/entity_spec.rb +++ b/spec/models/bulk_imports/entity_spec.rb @@ -154,4 +154,57 @@ RSpec.describe BulkImports::Entity, type: :model do expect(described_class.all_human_statuses).to contain_exactly('created', 'started', 'finished', 'failed') end end + + describe '#pipelines' do + context 'when entity is group' do + it 'returns group pipelines' do + entity = build(:bulk_import_entity, :group_entity) + + expect(entity.pipelines.flatten).to include(BulkImports::Groups::Pipelines::GroupPipeline) + end + end + + context 'when entity is project' do + it 'returns project pipelines' do + entity = build(:bulk_import_entity, :project_entity) + + expect(entity.pipelines.flatten).to include(BulkImports::Projects::Pipelines::ProjectPipeline) + end + end + end + + describe '#create_pipeline_trackers!' do + context 'when entity is group' do + it 'creates trackers for group entity' do + entity = create(:bulk_import_entity, :group_entity) + entity.create_pipeline_trackers! + + expect(entity.trackers.count).to eq(BulkImports::Groups::Stage.pipelines.count) + expect(entity.trackers.map(&:pipeline_name)).to include(BulkImports::Groups::Pipelines::GroupPipeline.to_s) + end + end + + context 'when entity is project' do + it 'creates trackers for project entity' do + entity = create(:bulk_import_entity, :project_entity) + entity.create_pipeline_trackers! + + expect(entity.trackers.count).to eq(BulkImports::Projects::Stage.pipelines.count) + expect(entity.trackers.map(&:pipeline_name)).to include(BulkImports::Projects::Pipelines::ProjectPipeline.to_s) + end + end + end + + describe '#pipeline_exists?' do + let_it_be(:entity) { create(:bulk_import_entity, :group_entity) } + + it 'returns true when the given pipeline name exists in the pipelines list' do + expect(entity.pipeline_exists?(BulkImports::Groups::Pipelines::GroupPipeline)).to eq(true) + expect(entity.pipeline_exists?('BulkImports::Groups::Pipelines::GroupPipeline')).to eq(true) + end + + it 'returns false when the given pipeline name exists in the pipelines list' do + expect(entity.pipeline_exists?('BulkImports::Groups::Pipelines::InexistentPipeline')).to eq(false) + end + end end diff --git a/spec/models/bulk_imports/tracker_spec.rb b/spec/models/bulk_imports/tracker_spec.rb index 0f00aeb9c1d..7f0a7d4f1ae 100644 --- a/spec/models/bulk_imports/tracker_spec.rb +++ b/spec/models/bulk_imports/tracker_spec.rb @@ -66,7 +66,7 @@ RSpec.describe BulkImports::Tracker, type: :model do describe '#pipeline_class' do it 'returns the pipeline class' do - pipeline_class = BulkImports::Stage.pipelines.first[1] + pipeline_class = BulkImports::Groups::Stage.pipelines.first[1] tracker = create(:bulk_import_tracker, pipeline_name: pipeline_class) expect(tracker.pipeline_class).to eq(pipeline_class) @@ -77,7 +77,7 @@ RSpec.describe BulkImports::Tracker, type: :model do expect { tracker.pipeline_class } .to raise_error( - NameError, + BulkImports::Error, "'InexistingPipeline' is not a valid BulkImport Pipeline" ) end diff --git a/spec/models/ci/bridge_spec.rb b/spec/models/ci/bridge_spec.rb index db956b26b6b..6dd3c40f228 100644 --- a/spec/models/ci/bridge_spec.rb +++ b/spec/models/ci/bridge_spec.rb @@ -74,18 +74,18 @@ RSpec.describe Ci::Bridge do it "schedules downstream pipeline creation when the status is #{status}" do bridge.status = status - expect(bridge).to receive(:schedule_downstream_pipeline!) - bridge.enqueue! + + expect(::Ci::CreateCrossProjectPipelineWorker.jobs.last['args']).to eq([bridge.id]) end end it "schedules downstream pipeline creation when the status is waiting for resource" do bridge.status = :waiting_for_resource - expect(bridge).to receive(:schedule_downstream_pipeline!) - bridge.enqueue_waiting_for_resource! + + expect(::Ci::CreateCrossProjectPipelineWorker.jobs.last['args']).to eq([bridge.id]) end it 'raises error when the status is failed' do diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 26abc98656e..1e06d566c80 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1307,7 +1307,9 @@ RSpec.describe Ci::Build do shared_examples_for 'avoid deadlock' do it 'executes UPDATE in the right order' do - recorded = ActiveRecord::QueryRecorder.new { subject } + recorded = with_cross_database_modification_prevented do + ActiveRecord::QueryRecorder.new { subject } + end index_for_build = recorded.log.index { |l| l.include?("UPDATE \"ci_builds\"") } index_for_deployment = recorded.log.index { |l| l.include?("UPDATE \"deployments\"") } @@ -1322,7 +1324,9 @@ RSpec.describe Ci::Build do it_behaves_like 'avoid deadlock' it 'transits deployment status to running' do - subject + with_cross_database_modification_prevented do + subject + end expect(deployment).to be_running end @@ -1340,7 +1344,9 @@ RSpec.describe Ci::Build do it_behaves_like 'calling proper BuildFinishedWorker' it 'transits deployment status to success' do - subject + with_cross_database_modification_prevented do + subject + end expect(deployment).to be_success end @@ -1353,7 +1359,9 @@ RSpec.describe Ci::Build do it_behaves_like 'calling proper BuildFinishedWorker' it 'transits deployment status to failed' do - subject + with_cross_database_modification_prevented do + subject + end expect(deployment).to be_failed end @@ -1365,7 +1373,9 @@ RSpec.describe Ci::Build do it_behaves_like 'avoid deadlock' it 'transits deployment status to skipped' do - subject + with_cross_database_modification_prevented do + subject + end expect(deployment).to be_skipped end @@ -1378,7 +1388,9 @@ RSpec.describe Ci::Build do it_behaves_like 'calling proper BuildFinishedWorker' it 'transits deployment status to canceled' do - subject + with_cross_database_modification_prevented do + subject + end expect(deployment).to be_canceled end @@ -2632,6 +2644,10 @@ RSpec.describe Ci::Build do value: "#{Gitlab.host_with_port}/#{project.namespace.root_ancestor.path.downcase}#{DependencyProxy::URL_SUFFIX}", public: true, masked: false }, + { key: 'CI_DEPENDENCY_PROXY_DIRECT_GROUP_IMAGE_PREFIX', + value: "#{Gitlab.host_with_port}/#{project.namespace.full_path.downcase}#{DependencyProxy::URL_SUFFIX}", + public: true, + masked: false }, { key: 'CI_API_V4_URL', value: 'http://localhost/api/v4', public: true, masked: false }, { key: 'CI_PIPELINE_IID', value: pipeline.iid.to_s, public: true, masked: false }, { key: 'CI_PIPELINE_SOURCE', value: pipeline.source, public: true, masked: false }, @@ -5243,4 +5259,33 @@ RSpec.describe Ci::Build do expect(described_class.with_coverage_regex).to eq([build_with_coverage_regex]) end end + + describe '#ensure_trace_metadata!' do + it 'delegates to Ci::BuildTraceMetadata' do + expect(Ci::BuildTraceMetadata) + .to receive(:find_or_upsert_for!) + .with(build.id) + + build.ensure_trace_metadata! + end + end + + describe '#doom!' do + subject { build.doom! } + + let_it_be(:build) { create(:ci_build, :queued) } + + it 'updates status and failure_reason', :aggregate_failures do + subject + + expect(build.status).to eq("failed") + expect(build.failure_reason).to eq("data_integrity_failure") + end + + it 'drops associated pending build' do + subject + + expect(build.reload.queuing_entry).not_to be_present + end + end end diff --git a/spec/models/ci/build_trace_chunks/fog_spec.rb b/spec/models/ci/build_trace_chunks/fog_spec.rb index 21dab6fad60..bbf04ef9430 100644 --- a/spec/models/ci/build_trace_chunks/fog_spec.rb +++ b/spec/models/ci/build_trace_chunks/fog_spec.rb @@ -107,37 +107,22 @@ RSpec.describe Ci::BuildTraceChunks::Fog do let(:model) { create(:ci_build_trace_chunk, :fog_with_data, initial_data: initial_data) } let(:data) { data_store.data(model) } - context 'when ci_job_trace_force_encode is enabled' do - it 'appends ASCII data' do - data_store.append_data(model, +'hello world', 4) + it 'appends ASCII data' do + data_store.append_data(model, +'hello world', 4) - expect(data.encoding).to eq(Encoding::ASCII_8BIT) - expect(data.force_encoding(Encoding::UTF_8)).to eq('😺hello world') - end - - it 'appends UTF-8 data' do - data_store.append_data(model, +'Résumé', 4) - - expect(data.encoding).to eq(Encoding::ASCII_8BIT) - expect(data.force_encoding(Encoding::UTF_8)).to eq("😺Résumé") - end - - context 'when initial data is UTF-8' do - let(:initial_data) { +'😺' } + expect(data.encoding).to eq(Encoding::ASCII_8BIT) + expect(data.force_encoding(Encoding::UTF_8)).to eq('😺hello world') + end - it 'appends ASCII data' do - data_store.append_data(model, +'hello world', 4) + it 'appends UTF-8 data' do + data_store.append_data(model, +'Résumé', 4) - expect(data.encoding).to eq(Encoding::ASCII_8BIT) - expect(data.force_encoding(Encoding::UTF_8)).to eq('😺hello world') - end - end + expect(data.encoding).to eq(Encoding::ASCII_8BIT) + expect(data.force_encoding(Encoding::UTF_8)).to eq("😺Résumé") end - context 'when ci_job_trace_force_encode is disabled' do - before do - stub_feature_flags(ci_job_trace_force_encode: false) - end + context 'when initial data is UTF-8' do + let(:initial_data) { +'😺' } it 'appends ASCII data' do data_store.append_data(model, +'hello world', 4) @@ -145,11 +130,6 @@ RSpec.describe Ci::BuildTraceChunks::Fog do expect(data.encoding).to eq(Encoding::ASCII_8BIT) expect(data.force_encoding(Encoding::UTF_8)).to eq('😺hello world') end - - it 'throws an exception when appending UTF-8 data' do - expect(Gitlab::ErrorTracking).to receive(:track_and_raise_exception).and_call_original - expect { data_store.append_data(model, +'Résumé', 4) }.to raise_exception(Encoding::CompatibilityError) - end end end diff --git a/spec/models/ci/build_trace_metadata_spec.rb b/spec/models/ci/build_trace_metadata_spec.rb index 42b9d5d34b6..5e4645c5dc4 100644 --- a/spec/models/ci/build_trace_metadata_spec.rb +++ b/spec/models/ci/build_trace_metadata_spec.rb @@ -7,4 +7,128 @@ RSpec.describe Ci::BuildTraceMetadata do it { is_expected.to belong_to(:trace_artifact) } it { is_expected.to validate_presence_of(:build) } + it { is_expected.to validate_presence_of(:archival_attempts) } + + describe '#can_attempt_archival_now?' do + let(:metadata) do + build(:ci_build_trace_metadata, + archival_attempts: archival_attempts, + last_archival_attempt_at: last_archival_attempt_at) + end + + subject { metadata.can_attempt_archival_now? } + + context 'when archival_attempts is over the limit' do + let(:archival_attempts) { described_class::MAX_ATTEMPTS + 1 } + let(:last_archival_attempt_at) {} + + it { is_expected.to be_falsey } + end + + context 'when last_archival_attempt_at is not set' do + let(:archival_attempts) { described_class::MAX_ATTEMPTS } + let(:last_archival_attempt_at) {} + + it { is_expected.to be_truthy } + end + + context 'when last_archival_attempt_at is set' do + let(:archival_attempts) { described_class::MAX_ATTEMPTS } + let(:last_archival_attempt_at) { 6.days.ago } + + it { is_expected.to be_truthy } + end + + context 'when last_archival_attempt_at is too close' do + let(:archival_attempts) { described_class::MAX_ATTEMPTS } + let(:last_archival_attempt_at) { 1.hour.ago } + + it { is_expected.to be_falsey } + end + end + + describe '#archival_attempts_available?' do + let(:metadata) do + build(:ci_build_trace_metadata, archival_attempts: archival_attempts) + end + + subject { metadata.archival_attempts_available? } + + context 'when archival_attempts is over the limit' do + let(:archival_attempts) { described_class::MAX_ATTEMPTS + 1 } + + it { is_expected.to be_falsey } + end + + context 'when archival_attempts is at the limit' do + let(:archival_attempts) { described_class::MAX_ATTEMPTS } + + it { is_expected.to be_truthy } + end + end + + describe '#increment_archival_attempts!' do + let_it_be(:metadata) do + create(:ci_build_trace_metadata, + archival_attempts: 2, + last_archival_attempt_at: 1.day.ago) + end + + it 'increments the attempts' do + expect { metadata.increment_archival_attempts! } + .to change { metadata.reload.archival_attempts } + end + + it 'updates the last_archival_attempt_at timestamp' do + expect { metadata.increment_archival_attempts! } + .to change { metadata.reload.last_archival_attempt_at } + end + end + + describe '#track_archival!' do + let(:trace_artifact) { create(:ci_job_artifact) } + let(:metadata) { create(:ci_build_trace_metadata) } + + it 'stores the artifact id and timestamp' do + expect(metadata.trace_artifact_id).to be_nil + + metadata.track_archival!(trace_artifact.id) + metadata.reload + + expect(metadata.trace_artifact_id).to eq(trace_artifact.id) + expect(metadata.archived_at).to be_like_time(Time.current) + end + end + + describe '.find_or_upsert_for!' do + let_it_be(:build) { create(:ci_build) } + + subject(:execute) do + described_class.find_or_upsert_for!(build.id) + end + + it 'creates a new record' do + metadata = execute + + expect(metadata).to be_a(described_class) + expect(metadata.id).to eq(build.id) + expect(metadata.archival_attempts).to eq(0) + end + + context 'with existing records' do + before do + create(:ci_build_trace_metadata, + build: build, + archival_attempts: described_class::MAX_ATTEMPTS) + end + + it 'returns the existing record' do + metadata = execute + + expect(metadata).to be_a(described_class) + expect(metadata.id).to eq(build.id) + expect(metadata.archival_attempts).to eq(described_class::MAX_ATTEMPTS) + end + end + end end diff --git a/spec/models/ci/pending_build_spec.rb b/spec/models/ci/pending_build_spec.rb index 0518c9a1652..ad711f5622f 100644 --- a/spec/models/ci/pending_build_spec.rb +++ b/spec/models/ci/pending_build_spec.rb @@ -34,6 +34,47 @@ RSpec.describe Ci::PendingBuild do end end end + + describe '.for_tags' do + subject(:pending_builds) { described_class.for_tags(tag_ids) } + + let_it_be(:pending_build_with_tags) { create(:ci_pending_build, tag_ids: [1, 2]) } + let_it_be(:pending_build_without_tags) { create(:ci_pending_build) } + + context 'when tag_ids match pending builds' do + let(:tag_ids) { [1, 2] } + + it 'returns matching pending builds' do + expect(pending_builds).to contain_exactly(pending_build_with_tags, pending_build_without_tags) + end + end + + context 'when tag_ids does not match pending builds' do + let(:tag_ids) { [non_existing_record_id] } + + it 'returns matching pending builds without tags' do + expect(pending_builds).to contain_exactly(pending_build_without_tags) + end + end + + context 'when tag_ids is not provided' do + context 'with a nil value' do + let(:tag_ids) { nil } + + it 'returns matching pending builds without tags' do + expect(pending_builds).to contain_exactly(pending_build_without_tags) + end + end + + context 'with an empty array' do + let(:tag_ids) { [] } + + it 'returns matching pending builds without tags' do + expect(pending_builds).to contain_exactly(pending_build_without_tags) + end + end + end + end end describe '.upsert_from_build!' do @@ -58,7 +99,11 @@ RSpec.describe Ci::PendingBuild do end end - context 'when project does not have shared runner' do + context 'when project does not have shared runners enabled' do + before do + project.shared_runners_enabled = false + end + it 'sets instance_runners_enabled to false' do described_class.upsert_from_build!(build) @@ -69,6 +114,10 @@ RSpec.describe Ci::PendingBuild do context 'when project has shared runner' do let_it_be(:runner) { create(:ci_runner, :instance) } + before do + project.shared_runners_enabled = true + end + context 'when ci_pending_builds_maintain_shared_runners_data is enabled' do it 'sets instance_runners_enabled to true' do described_class.upsert_from_build!(build) @@ -113,5 +162,65 @@ RSpec.describe Ci::PendingBuild do end end end + + context 'when build has tags' do + let!(:build) { create(:ci_build, :tags) } + + subject(:ci_pending_build) { described_class.last } + + context 'when ci_pending_builds_maintain_tags_data is enabled' do + it 'sets tag_ids' do + described_class.upsert_from_build!(build) + + expect(ci_pending_build.tag_ids).to eq(build.tags_ids) + end + end + + context 'when ci_pending_builds_maintain_tags_data is disabled' do + before do + stub_feature_flags(ci_pending_builds_maintain_tags_data: false) + end + + it 'does not set tag_ids' do + described_class.upsert_from_build!(build) + + expect(ci_pending_build.tag_ids).to be_empty + end + end + end + + context 'when a build project is nested in a subgroup' do + let(:group) { create(:group, :with_hierarchy, depth: 2, children: 1) } + let(:project) { create(:project, namespace: group.descendants.first) } + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, :created, pipeline: pipeline) } + + subject { described_class.last } + + context 'when build can be picked by a group runner' do + before do + project.group_runners_enabled = true + end + + it 'denormalizes namespace traversal ids' do + described_class.upsert_from_build!(build) + + expect(subject.namespace_traversal_ids).not_to be_empty + expect(subject.namespace_traversal_ids).to eq [group.id, project.namespace.id] + end + end + + context 'when build can not be picked by a group runner' do + before do + project.group_runners_enabled = false + end + + it 'creates an empty namespace traversal ids array' do + described_class.upsert_from_build!(build) + + expect(subject.namespace_traversal_ids).to be_empty + end + end + end end end diff --git a/spec/models/ci/pipeline_schedule_spec.rb b/spec/models/ci/pipeline_schedule_spec.rb index 8de3ebb18b9..c7e1fe91b1e 100644 --- a/spec/models/ci/pipeline_schedule_spec.rb +++ b/spec/models/ci/pipeline_schedule_spec.rb @@ -107,31 +107,24 @@ RSpec.describe Ci::PipelineSchedule do describe '#set_next_run_at' do using RSpec::Parameterized::TableSyntax - where(:worker_cron, :schedule_cron, :plan_limit, :ff_enabled, :now, :result) do - '0 1 2 3 *' | '0 1 * * *' | nil | true | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) - '0 1 2 3 *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) - '0 1 2 3 *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | false | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) - '*/5 * * * *' | '*/1 * * * *' | nil | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5) - '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) - '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | false | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5) - '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 10).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10) - '*/5 * * * *' | '*/1 * * * *' | 200 | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10) - '*/5 * * * *' | '*/1 * * * *' | 200 | false | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5) - '*/5 * * * *' | '0 * * * *' | nil | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) - '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 10).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) - '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 10).to_i | false | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) - '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) - '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 2.hours.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) - '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) - '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 10).to_i | true | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) - '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 8).to_i | true | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) - '*/5 * * * *' | '0 1 1 * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 1, 1, 0) | Time.zone.local(2021, 6, 1, 1, 0) - '*/9 * * * *' | '0 1 1 * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 1, 1, 9) | Time.zone.local(2021, 6, 1, 1, 0) - '*/9 * * * *' | '0 1 1 * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | false | Time.zone.local(2021, 5, 1, 1, 9) | Time.zone.local(2021, 6, 1, 1, 9) - '*/5 * * * *' | '59 14 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | true | Time.zone.local(2021, 5, 1, 15, 0) | Time.zone.local(2021, 5, 2, 15, 0) - '*/5 * * * *' | '59 14 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | false | Time.zone.local(2021, 5, 1, 15, 0) | Time.zone.local(2021, 5, 2, 15, 0) - '*/5 * * * *' | '45 21 1 2 *' | (1.day.in_minutes / 5).to_i | true | Time.zone.local(2021, 2, 1, 21, 45) | Time.zone.local(2022, 2, 1, 21, 45) - '*/5 * * * *' | '45 21 1 2 *' | (1.day.in_minutes / 5).to_i | false | Time.zone.local(2021, 2, 1, 21, 45) | Time.zone.local(2022, 2, 1, 21, 50) + where(:worker_cron, :schedule_cron, :plan_limit, :now, :result) do + '0 1 2 3 *' | '0 1 * * *' | nil | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) + '0 1 2 3 *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 3, 2, 1, 0) | Time.zone.local(2022, 3, 2, 1, 0) + '*/5 * * * *' | '*/1 * * * *' | nil | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 5) + '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) + '*/5 * * * *' | '*/1 * * * *' | (1.day.in_minutes / 10).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10) + '*/5 * * * *' | '*/1 * * * *' | 200 | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 11, 10) + '*/5 * * * *' | '0 * * * *' | nil | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) + '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 10).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) + '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 0) + '*/5 * * * *' | '0 * * * *' | (1.day.in_minutes / 2.hours.in_minutes).to_i | Time.zone.local(2021, 5, 27, 11, 0) | Time.zone.local(2021, 5, 27, 12, 5) + '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) + '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 10).to_i | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) + '*/5 * * * *' | '0 1 * * *' | (1.day.in_minutes / 8).to_i | Time.zone.local(2021, 5, 27, 1, 0) | Time.zone.local(2021, 5, 28, 1, 0) + '*/5 * * * *' | '0 1 1 * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 1, 1, 0) | Time.zone.local(2021, 6, 1, 1, 0) + '*/9 * * * *' | '0 1 1 * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 1, 1, 9) | Time.zone.local(2021, 6, 1, 1, 0) + '*/5 * * * *' | '59 14 * * *' | (1.day.in_minutes / 1.hour.in_minutes).to_i | Time.zone.local(2021, 5, 1, 15, 0) | Time.zone.local(2021, 5, 2, 15, 0) + '*/5 * * * *' | '45 21 1 2 *' | (1.day.in_minutes / 5).to_i | Time.zone.local(2021, 2, 1, 21, 45) | Time.zone.local(2022, 2, 1, 21, 45) end with_them do @@ -143,7 +136,6 @@ RSpec.describe Ci::PipelineSchedule do end create(:plan_limits, :default_plan, ci_daily_pipeline_schedule_triggers: plan_limit) if plan_limit - stub_feature_flags(ci_daily_limit_for_pipeline_schedules: false) unless ff_enabled # Setting this here to override initial save with the current time pipeline_schedule.next_run_at = now diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index da89eccc3b2..1007d64438f 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -183,6 +183,28 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end end + describe '.where_not_sha' do + let_it_be(:pipeline) { create(:ci_pipeline, sha: 'abcx') } + let_it_be(:pipeline_2) { create(:ci_pipeline, sha: 'abc') } + + let(:sha) { 'abc' } + + subject { described_class.where_not_sha(sha) } + + it 'returns the pipeline without the specified sha' do + is_expected.to contain_exactly(pipeline) + end + + context 'when argument is array' do + let(:sha) { %w[abc abcx] } + + it 'returns the pipelines without the specified shas' do + pipeline_3 = create(:ci_pipeline, sha: 'abcy') + is_expected.to contain_exactly(pipeline_3) + end + end + end + describe '.for_source_sha' do subject { described_class.for_source_sha(source_sha) } @@ -2015,16 +2037,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do it 'returns external pull request modified paths' do expect(pipeline.modified_paths).to match(external_pull_request.modified_paths) end - - context 'when the FF ci_modified_paths_of_external_prs is disabled' do - before do - stub_feature_flags(ci_modified_paths_of_external_prs: false) - end - - it 'returns nil' do - expect(pipeline.modified_paths).to be_nil - end - end end end @@ -4524,51 +4536,6 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do subject(:reset_bridge) { pipeline.reset_source_bridge!(project.owner) } - # This whole block will be removed by https://gitlab.com/gitlab-org/gitlab/-/issues/329194 - # It contains some duplicate checks. - context 'when the FF ci_reset_bridge_with_subsequent_jobs is disabled' do - before do - stub_feature_flags(ci_reset_bridge_with_subsequent_jobs: false) - end - - context 'when the pipeline is a child pipeline and the bridge is depended' do - let!(:parent_pipeline) { create(:ci_pipeline) } - let!(:bridge) { create_bridge(parent_pipeline, pipeline, true) } - - it 'marks source bridge as pending' do - reset_bridge - - expect(bridge.reload).to be_pending - end - - context 'when the parent pipeline has subsequent jobs after the bridge' do - let!(:after_bridge_job) { create(:ci_build, :skipped, pipeline: parent_pipeline, stage_idx: bridge.stage_idx + 1) } - - it 'does not touch subsequent jobs of the bridge' do - reset_bridge - - expect(after_bridge_job.reload).to be_skipped - end - end - - context 'when the parent pipeline has a dependent upstream pipeline' do - let(:upstream_pipeline) { create(:ci_pipeline, project: create(:project)) } - let!(:upstream_bridge) { create_bridge(upstream_pipeline, parent_pipeline, true) } - - let(:upstream_upstream_pipeline) { create(:ci_pipeline, project: create(:project)) } - let!(:upstream_upstream_bridge) { create_bridge(upstream_upstream_pipeline, upstream_pipeline, true) } - - it 'marks all source bridges as pending' do - reset_bridge - - expect(bridge.reload).to be_pending - expect(upstream_bridge.reload).to be_pending - expect(upstream_upstream_bridge.reload).to be_pending - end - end - end - end - context 'when the pipeline is a child pipeline and the bridge is depended' do let!(:parent_pipeline) { create(:ci_pipeline) } let!(:bridge) { create_bridge(parent_pipeline, pipeline, true) } diff --git a/spec/models/ci/pipeline_variable_spec.rb b/spec/models/ci/pipeline_variable_spec.rb index 04fcaab4c2d..4e8d49585d0 100644 --- a/spec/models/ci/pipeline_variable_spec.rb +++ b/spec/models/ci/pipeline_variable_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Ci::PipelineVariable do it_behaves_like "CI variable" - it { is_expected.to validate_uniqueness_of(:key).scoped_to(:pipeline_id) } + it { is_expected.to validate_presence_of(:key) } describe '#hook_attrs' do let(:variable) { create(:ci_pipeline_variable, key: 'foo', value: 'bar') } diff --git a/spec/models/ci/runner_spec.rb b/spec/models/ci/runner_spec.rb index ffc8ab4cf8b..31e854c852e 100644 --- a/spec/models/ci/runner_spec.rb +++ b/spec/models/ci/runner_spec.rb @@ -531,6 +531,10 @@ RSpec.describe Ci::Runner do it 'can handle builds' do expect(runner.can_pick?(build)).to be_truthy end + + it 'knows namespace id it is assigned to' do + expect(runner.namespace_ids).to eq [group.id] + end end end diff --git a/spec/models/clusters/agent_spec.rb b/spec/models/clusters/agent_spec.rb index ea7a55480a8..f9df84e8ff4 100644 --- a/spec/models/clusters/agent_spec.rb +++ b/spec/models/clusters/agent_spec.rb @@ -9,6 +9,10 @@ RSpec.describe Clusters::Agent do it { is_expected.to belong_to(:project).class_name('::Project') } it { is_expected.to have_many(:agent_tokens).class_name('Clusters::AgentToken') } it { is_expected.to have_many(:last_used_agent_tokens).class_name('Clusters::AgentToken') } + it { is_expected.to have_many(:group_authorizations).class_name('Clusters::Agents::GroupAuthorization') } + it { is_expected.to have_many(:authorized_groups).through(:group_authorizations) } + it { is_expected.to have_many(:project_authorizations).class_name('Clusters::Agents::ProjectAuthorization') } + it { is_expected.to have_many(:authorized_projects).through(:project_authorizations).class_name('::Project') } it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_length_of(:name).is_at_most(63) } diff --git a/spec/models/clusters/agents/group_authorization_spec.rb b/spec/models/clusters/agents/group_authorization_spec.rb new file mode 100644 index 00000000000..2a99fb26e3f --- /dev/null +++ b/spec/models/clusters/agents/group_authorization_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::Agents::GroupAuthorization do + it { is_expected.to belong_to(:agent).class_name('Clusters::Agent').required } + it { is_expected.to belong_to(:group).class_name('::Group').required } + + it { expect(described_class).to validate_jsonb_schema(['config']) } +end diff --git a/spec/models/clusters/agents/implicit_authorization_spec.rb b/spec/models/clusters/agents/implicit_authorization_spec.rb new file mode 100644 index 00000000000..69aa55a350e --- /dev/null +++ b/spec/models/clusters/agents/implicit_authorization_spec.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::Agents::ImplicitAuthorization do + let_it_be(:agent) { create(:cluster_agent) } + + subject { described_class.new(agent: agent) } + + it { expect(subject.agent).to eq(agent) } + it { expect(subject.agent_id).to eq(agent.id) } + it { expect(subject.project).to eq(agent.project) } + it { expect(subject.config).to be_nil } +end diff --git a/spec/models/clusters/agents/project_authorization_spec.rb b/spec/models/clusters/agents/project_authorization_spec.rb new file mode 100644 index 00000000000..134c70739ac --- /dev/null +++ b/spec/models/clusters/agents/project_authorization_spec.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Clusters::Agents::ProjectAuthorization do + it { is_expected.to belong_to(:agent).class_name('Clusters::Agent').required } + it { is_expected.to belong_to(:project).class_name('Project').required } + + it { expect(described_class).to validate_jsonb_schema(['config']) } +end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index 278e200b05c..9d305e31bad 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -268,6 +268,16 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do it { is_expected.to contain_exactly(cluster) } end + describe '.with_name' do + subject { described_class.with_name(name) } + + let(:name) { 'this-cluster' } + let!(:cluster) { create(:cluster, :project, name: name) } + let!(:another_cluster) { create(:cluster, :project) } + + it { is_expected.to contain_exactly(cluster) } + end + describe 'validations' do subject { cluster.valid? } @@ -902,8 +912,8 @@ RSpec.describe Clusters::Cluster, :use_clean_rails_memory_store_caching do subject { cluster.kubernetes_namespace_for(environment, deployable: build) } let(:environment_name) { 'the-environment-name' } - let(:environment) { create(:environment, name: environment_name, project: cluster.project, last_deployable: build) } - let(:build) { create(:ci_build, environment: environment_name, project: cluster.project) } + let(:environment) { create(:environment, name: environment_name, project: cluster.project) } + let(:build) { create(:ci_build, environment: environment, project: cluster.project) } let(:cluster) { create(:cluster, :project, managed: managed_cluster) } let(:managed_cluster) { true } let(:default_namespace) { Gitlab::Kubernetes::DefaultNamespace.new(cluster, project: cluster.project).from_environment_slug(environment.slug) } diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index a951af4cc4f..7134a387e65 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -88,6 +88,15 @@ RSpec.describe CommitStatus do end end + describe '.created_at_before' do + it 'finds the relevant records' do + status = create(:commit_status, created_at: 1.day.ago, project: project) + create(:commit_status, created_at: 1.day.since, project: project) + + expect(described_class.created_at_before(Time.current)).to eq([status]) + end + end + describe '.updated_before' do let!(:lookback) { 5.days.ago } let!(:timeout) { 1.day.ago } diff --git a/spec/models/concerns/approvable_base_spec.rb b/spec/models/concerns/approvable_base_spec.rb index c7ea2631a24..79053e98db7 100644 --- a/spec/models/concerns/approvable_base_spec.rb +++ b/spec/models/concerns/approvable_base_spec.rb @@ -60,6 +60,34 @@ RSpec.describe ApprovableBase do end end + describe '#can_be_unapproved_by?' do + subject { merge_request.can_be_unapproved_by?(user) } + + before do + merge_request.project.add_developer(user) + end + + it 'returns false' do + is_expected.to be_falsy + end + + context 'when a user has approved' do + let!(:approval) { create(:approval, merge_request: merge_request, user: user) } + + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'when a user is nil' do + let(:user) { nil } + + it 'returns false' do + is_expected.to be_falsy + end + end + end + describe '.not_approved_by_users_with_usernames' do subject { MergeRequest.not_approved_by_users_with_usernames([user.username, user2.username]) } diff --git a/spec/models/concerns/calloutable_spec.rb b/spec/models/concerns/calloutable_spec.rb new file mode 100644 index 00000000000..d847413de88 --- /dev/null +++ b/spec/models/concerns/calloutable_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Calloutable do + subject { build(:user_callout) } + + describe "Associations" do + it { is_expected.to belong_to(:user) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:user) } + end + + describe '#dismissed_after?' do + let(:some_feature_name) { UserCallout.feature_names.keys.second } + let(:callout_dismissed_month_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.month.ago )} + let(:callout_dismissed_day_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.day.ago )} + + it 'returns whether a callout dismissed after specified date' do + expect(callout_dismissed_month_ago.dismissed_after?(15.days.ago)).to eq(false) + expect(callout_dismissed_day_ago.dismissed_after?(15.days.ago)).to eq(true) + end + end +end diff --git a/spec/models/concerns/featurable_spec.rb b/spec/models/concerns/featurable_spec.rb index 295f3523dd5..453b6f7f29a 100644 --- a/spec/models/concerns/featurable_spec.rb +++ b/spec/models/concerns/featurable_spec.rb @@ -30,8 +30,11 @@ RSpec.describe Featurable do describe '.set_available_features' do let!(:klass) do - Class.new do + Class.new(ApplicationRecord) do include Featurable + + self.table_name = 'project_features' + set_available_features %i(feature1 feature2) def feature1_access_level diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index 071e0dcba44..2a3f639a8ac 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -368,6 +368,23 @@ RSpec.describe Issuable do expect(sorted_issue_ids).to eq(sorted_issue_ids.uniq) end end + + context 'by title' do + let!(:issue1) { create(:issue, project: project, title: 'foo') } + let!(:issue2) { create(:issue, project: project, title: 'bar') } + let!(:issue3) { create(:issue, project: project, title: 'baz') } + let!(:issue4) { create(:issue, project: project, title: 'Baz 2') } + + it 'sorts asc' do + issues = project.issues.sort_by_attribute('title_asc') + expect(issues).to eq([issue2, issue3, issue4, issue1]) + end + + it 'sorts desc' do + issues = project.issues.sort_by_attribute('title_desc') + expect(issues).to eq([issue1, issue4, issue3, issue2]) + end + end end describe '#subscribed?' do diff --git a/spec/models/concerns/loose_foreign_key_spec.rb b/spec/models/concerns/loose_foreign_key_spec.rb new file mode 100644 index 00000000000..ce5e33261a9 --- /dev/null +++ b/spec/models/concerns/loose_foreign_key_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe LooseForeignKey do + let(:project_klass) do + Class.new(ApplicationRecord) do + include LooseForeignKey + + self.table_name = 'projects' + + loose_foreign_key :issues, :project_id, on_delete: :async_delete, gitlab_schema: :gitlab_main + loose_foreign_key 'merge_requests', 'project_id', 'on_delete' => 'async_nullify', 'gitlab_schema' => :gitlab_main + end + end + + it 'exposes the loose foreign key definitions' do + definitions = project_klass.loose_foreign_key_definitions + + tables = definitions.map(&:to_table) + expect(tables).to eq(%w[issues merge_requests]) + end + + it 'casts strings to symbol' do + definition = project_klass.loose_foreign_key_definitions.last + + expect(definition.from_table).to eq('projects') + expect(definition.to_table).to eq('merge_requests') + expect(definition.column).to eq('project_id') + expect(definition.on_delete).to eq(:async_nullify) + expect(definition.options[:gitlab_schema]).to eq(:gitlab_main) + end + + context 'validation' do + context 'on_delete validation' do + let(:invalid_class) do + Class.new(ApplicationRecord) do + include LooseForeignKey + + self.table_name = 'projects' + + loose_foreign_key :issues, :project_id, on_delete: :async_delete, gitlab_schema: :gitlab_main + loose_foreign_key :merge_requests, :project_id, on_delete: :async_nullify, gitlab_schema: :gitlab_main + loose_foreign_key :merge_requests, :project_id, on_delete: :destroy, gitlab_schema: :gitlab_main + end + end + + it 'raises error when invalid `on_delete` option was given' do + expect { invalid_class }.to raise_error /Invalid on_delete option given: destroy/ + end + end + + context 'gitlab_schema validation' do + let(:invalid_class) do + Class.new(ApplicationRecord) do + include LooseForeignKey + + self.table_name = 'projects' + + loose_foreign_key :merge_requests, :project_id, on_delete: :async_nullify, gitlab_schema: :unknown + end + end + + it 'raises error when invalid `gitlab_schema` option was given' do + expect { invalid_class }.to raise_error /Invalid gitlab_schema option given: unknown/ + end + end + + context 'inheritance validation' do + let(:inherited_project_class) do + Class.new(Project) do + include LooseForeignKey + + loose_foreign_key :issues, :project_id, on_delete: :async_delete, gitlab_schema: :gitlab_main + end + end + + it 'raises error when loose_foreign_key is defined in a child ActiveRecord model' do + expect { inherited_project_class }.to raise_error /Please define the loose_foreign_key on the Project class/ + end + end + end +end diff --git a/spec/models/concerns/partitioned_table_spec.rb b/spec/models/concerns/partitioned_table_spec.rb index c37fb81a1cf..714db4e21bd 100644 --- a/spec/models/concerns/partitioned_table_spec.rb +++ b/spec/models/concerns/partitioned_table_spec.rb @@ -35,11 +35,5 @@ RSpec.describe PartitionedTable do expect(my_class.partitioning_strategy.partitioning_key).to eq(key) end - - it 'registers itself with the PartitionCreator' do - expect(Gitlab::Database::Partitioning::PartitionManager).to receive(:register).with(my_class) - - subject - end end end diff --git a/spec/models/concerns/sanitizable_spec.rb b/spec/models/concerns/sanitizable_spec.rb new file mode 100644 index 00000000000..4a1d463d666 --- /dev/null +++ b/spec/models/concerns/sanitizable_spec.rb @@ -0,0 +1,101 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Sanitizable do + let_it_be(:klass) do + Class.new do + include ActiveModel::Model + include ActiveModel::Attributes + include ActiveModel::Validations + include ActiveModel::Validations::Callbacks + include Sanitizable + + attribute :id, :integer + attribute :name, :string + attribute :description, :string + attribute :html_body, :string + + sanitizes! :name, :description + + def self.model_name + ActiveModel::Name.new(self, nil, 'SomeModel') + end + end + end + + shared_examples 'noop' do + it 'has no effect' do + expect(subject).to eq(input) + end + end + + shared_examples 'a sanitizable field' do |field| + let(:record) { klass.new(id: 1, name: input, description: input, html_body: input) } + + before do + record.valid? + end + + subject { record.public_send(field) } + + describe field do + context 'when input is nil' do + let_it_be(:input) { nil } + + it_behaves_like 'noop' + end + + context 'when input does not contain any html' do + let_it_be(:input) { 'hello, world!' } + + it_behaves_like 'noop' + end + + context 'when input contains html' do + let_it_be(:input) { 'hello<script>alert(1)</script>' } + + it 'sanitizes the input' do + expect(subject).to eq('hello') + end + + context 'when input includes html entities' do + let(:input) { '<div>hello&world</div>' } + + it 'does not escape them' do + expect(subject).to eq(' hello&world ') + end + end + end + + context 'when input contains pre-escaped html entities' do + let_it_be(:input) { '<script>alert(1)</script>' } + + it_behaves_like 'noop' + + it 'is not valid', :aggregate_failures do + expect(record).not_to be_valid + expect(record.errors.full_messages).to include('Name cannot contain escaped HTML entities') + end + end + end + end + + shared_examples 'a non-sanitizable field' do |field, input| + describe field do + subject { klass.new(field => input).valid? } + + it 'has no effect' do + expect(Sanitize).not_to receive(:fragment) + + subject + end + end + end + + it_behaves_like 'a non-sanitizable field', :id, 1 + it_behaves_like 'a non-sanitizable field', :html_body, 'hello<script>alert(1)</script>' + + it_behaves_like 'a sanitizable field', :name + it_behaves_like 'a sanitizable field', :description +end diff --git a/spec/models/concerns/taggable_queries_spec.rb b/spec/models/concerns/taggable_queries_spec.rb new file mode 100644 index 00000000000..0d248c4636e --- /dev/null +++ b/spec/models/concerns/taggable_queries_spec.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe TaggableQueries do + it 'keeps MAX_TAGS_IDS in sync with TAGS_LIMIT' do + expect(described_class::MAX_TAGS_IDS).to eq(Gitlab::Ci::Config::Entry::Tags::TAGS_LIMIT) + end +end diff --git a/spec/models/customer_relations/contact_spec.rb b/spec/models/customer_relations/contact_spec.rb new file mode 100644 index 00000000000..b19554dd67e --- /dev/null +++ b/spec/models/customer_relations/contact_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe CustomerRelations::Contact, type: :model do + describe 'associations' do + it { is_expected.to belong_to(:group) } + it { is_expected.to belong_to(:organization).optional } + end + + describe 'validations' do + subject { build(:contact) } + + it { is_expected.to validate_presence_of(:group) } + it { is_expected.to validate_presence_of(:first_name) } + it { is_expected.to validate_presence_of(:last_name) } + + it { is_expected.to validate_length_of(:phone).is_at_most(32) } + it { is_expected.to validate_length_of(:first_name).is_at_most(255) } + it { is_expected.to validate_length_of(:last_name).is_at_most(255) } + it { is_expected.to validate_length_of(:email).is_at_most(255) } + it { is_expected.to validate_length_of(:description).is_at_most(1024) } + + it_behaves_like 'an object with RFC3696 compliant email-formatted attributes', :email + end + + describe '#before_validation' do + it 'strips leading and trailing whitespace' do + contact = described_class.new(first_name: ' First ', last_name: ' Last ', phone: ' 123456 ') + contact.valid? + + expect(contact.first_name).to eq('First') + expect(contact.last_name).to eq('Last') + expect(contact.phone).to eq('123456') + end + end +end diff --git a/spec/models/customer_relations/organization_spec.rb b/spec/models/customer_relations/organization_spec.rb index b79b5748156..71b455ae8c8 100644 --- a/spec/models/customer_relations/organization_spec.rb +++ b/spec/models/customer_relations/organization_spec.rb @@ -8,7 +8,7 @@ RSpec.describe CustomerRelations::Organization, type: :model do end describe 'validations' do - subject { create(:organization) } + subject { build(:organization) } it { is_expected.to validate_presence_of(:group) } it { is_expected.to validate_presence_of(:name) } diff --git a/spec/models/dependency_proxy/blob_spec.rb b/spec/models/dependency_proxy/blob_spec.rb index 7c8a1eb95e8..3797f6184fe 100644 --- a/spec/models/dependency_proxy/blob_spec.rb +++ b/spec/models/dependency_proxy/blob_spec.rb @@ -6,10 +6,13 @@ RSpec.describe DependencyProxy::Blob, type: :model do it { is_expected.to belong_to(:group) } end + it_behaves_like 'having unique enum values' + describe 'validations' do it { is_expected.to validate_presence_of(:group) } it { is_expected.to validate_presence_of(:file) } it { is_expected.to validate_presence_of(:file_name) } + it { is_expected.to validate_presence_of(:status) } end describe '.total_size' do diff --git a/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb new file mode 100644 index 00000000000..2906ea7b774 --- /dev/null +++ b/spec/models/dependency_proxy/image_ttl_group_policy_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe DependencyProxy::ImageTtlGroupPolicy, type: :model do + describe 'relationships' do + it { is_expected.to belong_to(:group) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:group) } + + describe '#enabled' do + it { is_expected.to allow_value(true).for(:enabled) } + it { is_expected.to allow_value(false).for(:enabled) } + it { is_expected.not_to allow_value(nil).for(:enabled) } + end + + describe '#ttl' do + it { is_expected.to validate_numericality_of(:ttl).allow_nil.is_greater_than(0) } + end + end +end diff --git a/spec/models/dependency_proxy/manifest_spec.rb b/spec/models/dependency_proxy/manifest_spec.rb index 4203644c003..2a085b3613b 100644 --- a/spec/models/dependency_proxy/manifest_spec.rb +++ b/spec/models/dependency_proxy/manifest_spec.rb @@ -6,11 +6,14 @@ RSpec.describe DependencyProxy::Manifest, type: :model do it { is_expected.to belong_to(:group) } end + it_behaves_like 'having unique enum values' + describe 'validations' do it { is_expected.to validate_presence_of(:group) } it { is_expected.to validate_presence_of(:file) } it { is_expected.to validate_presence_of(:file_name) } it { is_expected.to validate_presence_of(:digest) } + it { is_expected.to validate_presence_of(:status) } end describe 'file is being stored' do diff --git a/spec/models/design_management/action_spec.rb b/spec/models/design_management/action_spec.rb index 59c58191718..0a8bbc8d26e 100644 --- a/spec/models/design_management/action_spec.rb +++ b/spec/models/design_management/action_spec.rb @@ -8,37 +8,55 @@ RSpec.describe DesignManagement::Action do end describe 'scopes' do - describe '.most_recent' do - let_it_be(:design_a) { create(:design) } - let_it_be(:design_b) { create(:design) } - let_it_be(:design_c) { create(:design) } + let_it_be(:issue) { create(:issue) } + let_it_be(:design_a) { create(:design, issue: issue) } + let_it_be(:design_b) { create(:design, issue: issue) } - let(:designs) { [design_a, design_b, design_c] } + context 'with 3 designs' do + let_it_be(:design_c) { create(:design, issue: issue) } - before_all do - create(:design_version, designs: [design_a, design_b, design_c]) - create(:design_version, designs: [design_a, design_b]) - create(:design_version, designs: [design_a]) - end + let_it_be(:action_a_1) { create(:design_action, design: design_a) } + let_it_be(:action_a_2) { create(:design_action, design: design_a, event: :deletion) } + let_it_be(:action_b) { create(:design_action, design: design_b) } + let_it_be(:action_c) { create(:design_action, design: design_c, event: :deletion) } + + describe '.most_recent' do + let(:designs) { [design_a, design_b, design_c] } + + before_all do + create(:design_version, designs: [design_a, design_b, design_c]) + create(:design_version, designs: [design_a, design_b]) + create(:design_version, designs: [design_a]) + end + + it 'finds the correct version for each design' do + dvs = described_class.where(design: designs) + + expected = designs + .map(&:id) + .zip(dvs.order("version_id DESC").pluck(:version_id).uniq) - it 'finds the correct version for each design' do - dvs = described_class.where(design: designs) + actual = dvs.most_recent.map { |dv| [dv.design_id, dv.version_id] } - expected = designs - .map(&:id) - .zip(dvs.order("version_id DESC").pluck(:version_id).uniq) + expect(actual).to eq(expected) + end + end - actual = dvs.most_recent.map { |dv| [dv.design_id, dv.version_id] } + describe '.by_design' do + it 'returns the actions by design_id' do + expect(described_class.by_design([design_a.id, design_b.id])) + .to match_array([action_a_1, action_a_2, action_b]) + end + end - expect(actual).to eq(expected) + describe '.by_event' do + it 'returns the actions by event type' do + expect(described_class.by_event(:deletion)).to match_array([action_a_2, action_c]) + end end end describe '.up_to_version' do - let_it_be(:issue) { create(:issue) } - let_it_be(:design_a) { create(:design, issue: issue) } - let_it_be(:design_b) { create(:design, issue: issue) } - # let bindings are not available in before(:all) contexts, # so we need to redefine the array on each construction. let_it_be(:oldest) { create(:design_version, designs: [design_a, design_b]) } diff --git a/spec/models/diff_note_spec.rb b/spec/models/diff_note_spec.rb index 11652d9841b..f377b34679c 100644 --- a/spec/models/diff_note_spec.rb +++ b/spec/models/diff_note_spec.rb @@ -558,4 +558,31 @@ RSpec.describe DiffNote do it { is_expected.to eq('note') } end + + describe '#shas' do + it 'returns list of SHAs based on original_position' do + expect(subject.shas).to match_array([ + position.base_sha, + position.start_sha, + position.head_sha + ]) + end + + context 'when position changes' do + before do + subject.position = new_position + end + + it 'includes the new position SHAs' do + expect(subject.shas).to match_array([ + position.base_sha, + position.start_sha, + position.head_sha, + new_position.base_sha, + new_position.start_sha, + new_position.head_sha + ]) + end + end + end end diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index 53561586d61..e3e9d1f7a71 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -135,6 +135,20 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do environment.stop end + + context 'when environment has auto stop period' do + let!(:environment) { create(:environment, :available, :auto_stoppable, project: project) } + + it 'clears auto stop period when the environment has stopped' do + environment.stop! + + expect(environment.auto_stop_at).to be_nil + end + + it 'does not clear auto stop period when the environment has not stopped' do + expect(environment.auto_stop_at).to be_present + end + end end describe '.for_name_like' do @@ -233,55 +247,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end - describe '.stop_actions' do - subject { environments.stop_actions } - - let_it_be(:project) { create(:project, :repository) } - let_it_be(:user) { create(:user) } - - let(:environments) { Environment.all } - - before_all do - project.add_developer(user) - project.repository.add_branch(user, 'review/feature-1', 'master') - project.repository.add_branch(user, 'review/feature-2', 'master') - end - - shared_examples_for 'correct filtering' do - it 'returns stop actions for available environments only' do - expect(subject.count).to eq(1) - expect(subject.first.name).to eq('stop_review_app') - expect(subject.first.ref).to eq('review/feature-1') - end - end - - before do - create_review_app(user, project, 'review/feature-1') - create_review_app(user, project, 'review/feature-2') - end - - it 'returns stop actions for environments' do - expect(subject.count).to eq(2) - expect(subject).to match_array(Ci::Build.where(name: 'stop_review_app')) - end - - context 'when one of the stop actions has already been executed' do - before do - Ci::Build.where(ref: 'review/feature-2').find_by_name('stop_review_app').enqueue! - end - - it_behaves_like 'correct filtering' - end - - context 'when one of the deployments does not have stop action' do - before do - Deployment.where(ref: 'review/feature-2').update_all(on_stop: nil) - end - - it_behaves_like 'correct filtering' - end - end - describe '.pluck_names' do subject { described_class.pluck_names } @@ -726,6 +691,28 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe '#last_deployable' do + subject { environment.last_deployable } + + context 'does not join across databases' do + let(:pipeline_a) { create(:ci_pipeline, project: project) } + let(:pipeline_b) { create(:ci_pipeline, project: project) } + let(:ci_build_a) { create(:ci_build, project: project, pipeline: pipeline_a) } + let(:ci_build_b) { create(:ci_build, project: project, pipeline: pipeline_b) } + + before do + create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b) + end + + it 'when called' do + with_cross_joins_prevented do + expect(subject.id).to eq(ci_build_a.id) + end + end + end + end + describe '#last_visible_deployment' do subject { environment.last_visible_deployment } @@ -768,6 +755,86 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end + describe '#last_visible_deployable' do + subject { environment.last_visible_deployable } + + context 'does not join across databases' do + let(:pipeline_a) { create(:ci_pipeline, project: project) } + let(:pipeline_b) { create(:ci_pipeline, project: project) } + let(:ci_build_a) { create(:ci_build, project: project, pipeline: pipeline_a) } + let(:ci_build_b) { create(:ci_build, project: project, pipeline: pipeline_b) } + + before do + create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b) + end + + it 'for direct call' do + with_cross_joins_prevented do + expect(subject.id).to eq(ci_build_b.id) + end + end + + it 'for preload' do + environment.reload + + with_cross_joins_prevented do + ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_deployable: []]) + expect(subject.id).to eq(ci_build_b.id) + end + end + end + + context 'call after preload' do + it 'fetches from association cache' do + pipeline = create(:ci_pipeline, project: project) + ci_build = create(:ci_build, project: project, pipeline: pipeline) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build) + + environment.reload + ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_deployable: []]) + + query_count = ActiveRecord::QueryRecorder.new do + expect(subject.id).to eq(ci_build.id) + end.count + + expect(query_count).to eq(0) + end + end + + context 'when the feature for disable_join is disabled' do + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:ci_build) { create(:ci_build, project: project, pipeline: pipeline) } + + before do + stub_feature_flags(environment_last_visible_pipeline_disable_joins: false) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build) + end + + context 'for preload' do + it 'executes the original association instead of override' do + environment.reload + ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_deployable: []]) + + expect_any_instance_of(Deployment).not_to receive(:deployable) + + query_count = ActiveRecord::QueryRecorder.new do + expect(subject.id).to eq(ci_build.id) + end.count + + expect(query_count).to eq(0) + end + end + + context 'for direct call' do + it 'executes the original association instead of override' do + expect_any_instance_of(Deployment).not_to receive(:deployable) + expect(subject.id).to eq(ci_build.id) + end + end + end + end + describe '#last_visible_pipeline' do let(:user) { create(:user) } let_it_be(:project) { create(:project, :repository) } @@ -812,6 +879,35 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do expect(last_pipeline).to eq(failed_pipeline) end + context 'does not join across databases' do + let(:pipeline_a) { create(:ci_pipeline, project: project) } + let(:pipeline_b) { create(:ci_pipeline, project: project) } + let(:ci_build_a) { create(:ci_build, project: project, pipeline: pipeline_a) } + let(:ci_build_b) { create(:ci_build, project: project, pipeline: pipeline_b) } + + before do + create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b) + end + + subject { environment.last_visible_pipeline } + + it 'for direct call' do + with_cross_joins_prevented do + expect(subject.id).to eq(pipeline_b.id) + end + end + + it 'for preload' do + environment.reload + + with_cross_joins_prevented do + ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_pipeline: []]) + expect(subject.id).to eq(pipeline_b.id) + end + end + end + context 'for the environment' do it 'returns the last pipeline' do pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha) @@ -850,6 +946,57 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do end end end + + context 'call after preload' do + it 'fetches from association cache' do + pipeline = create(:ci_pipeline, project: project) + ci_build = create(:ci_build, project: project, pipeline: pipeline) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build) + + environment.reload + ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_pipeline: []]) + + query_count = ActiveRecord::QueryRecorder.new do + expect(environment.last_visible_pipeline.id).to eq(pipeline.id) + end.count + + expect(query_count).to eq(0) + end + end + + context 'when the feature for disable_join is disabled' do + let(:pipeline) { create(:ci_pipeline, project: project) } + let(:ci_build) { create(:ci_build, project: project, pipeline: pipeline) } + + before do + stub_feature_flags(environment_last_visible_pipeline_disable_joins: false) + create(:deployment, :failed, project: project, environment: environment, deployable: ci_build) + end + + subject { environment.last_visible_pipeline } + + context 'for preload' do + it 'executes the original association instead of override' do + environment.reload + ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_pipeline: []]) + + expect_any_instance_of(Ci::Build).not_to receive(:pipeline) + + query_count = ActiveRecord::QueryRecorder.new do + expect(subject.id).to eq(pipeline.id) + end.count + + expect(query_count).to eq(0) + end + end + + context 'for direct call' do + it 'executes the original association instead of override' do + expect_any_instance_of(Ci::Build).not_to receive(:pipeline) + expect(subject.id).to eq(pipeline.id) + end + end + end end describe '#upcoming_deployment' do diff --git a/spec/models/error_tracking/error_spec.rb b/spec/models/error_tracking/error_spec.rb index 57899985daf..5543392b624 100644 --- a/spec/models/error_tracking/error_spec.rb +++ b/spec/models/error_tracking/error_spec.rb @@ -16,6 +16,62 @@ RSpec.describe ErrorTracking::Error, type: :model do it { is_expected.to validate_presence_of(:actor) } end + describe '.report_error' do + it 'updates existing record with a new timestamp' do + timestamp = Time.zone.now + + reported_error = described_class.report_error( + name: error.name, + description: 'Lorem ipsum', + actor: error.actor, + platform: error.platform, + timestamp: timestamp + ) + + expect(reported_error.id).to eq(error.id) + expect(reported_error.last_seen_at).to eq(timestamp) + expect(reported_error.description).to eq('Lorem ipsum') + end + end + + describe '.sort_by_attribute' do + let!(:error2) { create(:error_tracking_error, first_seen_at: Time.zone.now - 2.weeks, last_seen_at: Time.zone.now - 1.week) } + let!(:error3) { create(:error_tracking_error, first_seen_at: Time.zone.now - 3.weeks, last_seen_at: Time.zone.now.yesterday) } + let!(:errors) { [error, error2, error3] } + + subject { described_class.where(id: errors).sort_by_attribute(sort) } + + context 'id desc by default' do + let(:sort) { nil } + + it { is_expected.to eq([error3, error2, error]) } + end + + context 'first_seen' do + let(:sort) { 'first_seen' } + + it { is_expected.to eq([error, error2, error3]) } + end + + context 'last_seen' do + let(:sort) { 'last_seen' } + + it { is_expected.to eq([error, error3, error2]) } + end + + context 'frequency' do + let(:sort) { 'frequency' } + + before do + create(:error_tracking_error_event, error: error2) + create(:error_tracking_error_event, error: error2) + create(:error_tracking_error_event, error: error3) + end + + it { is_expected.to eq([error2, error3, error]) } + end + end + describe '#title' do it { expect(error.title).to eq('ActionView::MissingTemplate Missing template posts/edit') } end diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 7be61f4950e..29255e53fcf 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -478,18 +478,17 @@ RSpec.describe ErrorTracking::ProjectErrorTrackingSetting do describe '#sentry_enabled' do using RSpec::Parameterized::TableSyntax - where(:enabled, :integrated, :feature_flag, :sentry_enabled) do - true | false | false | true - true | true | false | true - true | true | true | false - false | false | false | false + where(:enabled, :integrated, :sentry_enabled) do + true | false | true + true | true | false + true | true | false + false | false | false end with_them do before do subject.enabled = enabled subject.integrated = integrated - stub_feature_flags(integrated_error_tracking: feature_flag) end it { expect(subject.sentry_enabled).to eq(sentry_enabled) } diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index ddf12c8e4c4..d536a0783bc 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -30,10 +30,12 @@ RSpec.describe Group do it { is_expected.to have_many(:group_deploy_keys) } it { is_expected.to have_many(:integrations) } it { is_expected.to have_one(:dependency_proxy_setting) } + it { is_expected.to have_one(:dependency_proxy_image_ttl_policy) } it { is_expected.to have_many(:dependency_proxy_blobs) } it { is_expected.to have_many(:dependency_proxy_manifests) } it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::GroupDistribution').dependent(:destroy) } it { is_expected.to have_many(:daily_build_group_report_results).class_name('Ci::DailyBuildGroupReportResult') } + it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout').with_foreign_key(:group_id) } describe '#members & #requesters' do let(:requester) { create(:user) } @@ -80,7 +82,7 @@ RSpec.describe Group do group = build(:group, parent: build(:namespace)) expect(group).not_to be_valid - expect(group.errors[:parent_id].first).to eq('a group cannot have a user namespace as its parent') + expect(group.errors[:parent_id].first).to eq('user namespace cannot be the parent of another namespace') end it 'allows a group to have another group as its parent' do @@ -2273,19 +2275,27 @@ RSpec.describe Group do end describe '.groups_including_descendants_by' do - it 'returns the expected groups for a group and its descendants' do - parent_group1 = create(:group) - child_group1 = create(:group, parent: parent_group1) - child_group2 = create(:group, parent: parent_group1) + let_it_be(:parent_group1) { create(:group) } + let_it_be(:parent_group2) { create(:group) } + let_it_be(:extra_group) { create(:group) } + let_it_be(:child_group1) { create(:group, parent: parent_group1) } + let_it_be(:child_group2) { create(:group, parent: parent_group1) } + let_it_be(:child_group3) { create(:group, parent: parent_group2) } - parent_group2 = create(:group) - child_group3 = create(:group, parent: parent_group2) + subject { described_class.groups_including_descendants_by([parent_group2.id, parent_group1.id]) } - create(:group) + shared_examples 'returns the expected groups for a group and its descendants' do + specify { is_expected.to contain_exactly(parent_group1, parent_group2, child_group1, child_group2, child_group3) } + end + + it_behaves_like 'returns the expected groups for a group and its descendants' - groups = described_class.groups_including_descendants_by([parent_group2.id, parent_group1.id]) + context 'when :linear_group_including_descendants_by feature flag is disabled' do + before do + stub_feature_flags(linear_group_including_descendants_by: false) + end - expect(groups).to contain_exactly(parent_group1, parent_group2, child_group1, child_group2, child_group3) + it_behaves_like 'returns the expected groups for a group and its descendants' end end @@ -2477,6 +2487,12 @@ RSpec.describe Group do end end + describe '#membership_locked?' do + it 'returns false' do + expect(build(:group)).not_to be_membership_locked + end + end + describe '#default_owner' do let(:group) { build(:group) } @@ -2619,6 +2635,26 @@ RSpec.describe Group do end end + describe '.organizations' do + it 'returns organizations belonging to the group' do + organization1 = create(:organization, group: group) + create(:organization) + organization3 = create(:organization, group: group) + + expect(group.organizations).to contain_exactly(organization1, organization3) + end + end + + describe '.contacts' do + it 'returns contacts belonging to the group' do + contact1 = create(:contact, group: group) + create(:contact) + contact3 = create(:contact, group: group) + + expect(group.contacts).to contain_exactly(contact1, contact3) + end + end + describe '#to_ability_name' do it 'returns group' do group = build(:group) @@ -2696,4 +2732,40 @@ RSpec.describe Group do group.open_merge_requests_count end end + + describe '#dependency_proxy_image_prefix' do + let_it_be(:group) { build_stubbed(:group, path: 'GroupWithUPPERcaseLetters') } + + it 'converts uppercase letters to lowercase' do + expect(group.dependency_proxy_image_prefix).to end_with("/groupwithuppercaseletters#{DependencyProxy::URL_SUFFIX}") + end + + it 'removes the protocol' do + expect(group.dependency_proxy_image_prefix).not_to include('http') + end + end + + describe '#dependency_proxy_image_ttl_policy' do + subject(:ttl_policy) { group.dependency_proxy_image_ttl_policy } + + it 'builds a new policy if one does not exist', :aggregate_failures do + expect(ttl_policy.ttl).to eq(90) + expect(ttl_policy.enabled).to eq(false) + expect(ttl_policy.created_at).to be_nil + expect(ttl_policy.updated_at).to be_nil + end + + context 'with existing policy' do + before do + group.dependency_proxy_image_ttl_policy.update!(ttl: 30, enabled: true) + end + + it 'returns the policy if it already exists', :aggregate_failures do + expect(ttl_policy.ttl).to eq(30) + expect(ttl_policy.enabled).to eq(true) + expect(ttl_policy.created_at).not_to be_nil + expect(ttl_policy.updated_at).not_to be_nil + end + end + end end diff --git a/spec/models/hooks/web_hook_spec.rb b/spec/models/hooks/web_hook_spec.rb index c68ad3bf0c4..59f4533a6c1 100644 --- a/spec/models/hooks/web_hook_spec.rb +++ b/spec/models/hooks/web_hook_spec.rb @@ -10,7 +10,11 @@ RSpec.describe WebHook do let(:hook) { build(:project_hook, project: project) } around do |example| - freeze_time { example.run } + if example.metadata[:skip_freeze_time] + example.run + else + freeze_time { example.run } + end end describe 'associations' do @@ -326,10 +330,28 @@ RSpec.describe WebHook do expect { hook.backoff! }.to change(hook, :backoff_count).by(1) end - it 'does not let the backoff count exceed the maximum failure count' do - hook.backoff_count = described_class::MAX_FAILURES + context 'when we have backed off MAX_FAILURES times' do + before do + stub_const("#{described_class}::MAX_FAILURES", 5) + 5.times { hook.backoff! } + end + + it 'does not let the backoff count exceed the maximum failure count' do + expect { hook.backoff! }.not_to change(hook, :backoff_count) + end + + it 'does not change disabled_until', :skip_freeze_time do + travel_to(hook.disabled_until - 1.minute) do + expect { hook.backoff! }.not_to change(hook, :disabled_until) + end + end - expect { hook.backoff! }.not_to change(hook, :backoff_count) + it 'changes disabled_until when it has elapsed', :skip_freeze_time do + travel_to(hook.disabled_until + 1.minute) do + expect { hook.backoff! }.to change { hook.disabled_until } + expect(hook.backoff_count).to eq(described_class::MAX_FAILURES) + end + end end include_examples 'is tolerant of invalid records' do diff --git a/spec/models/instance_configuration_spec.rb b/spec/models/instance_configuration_spec.rb index 9544f0fe6ec..551e6e7572c 100644 --- a/spec/models/instance_configuration_spec.rb +++ b/spec/models/instance_configuration_spec.rb @@ -76,24 +76,46 @@ RSpec.describe InstanceConfiguration do end end - describe '#gitlab_ci' do - let(:gitlab_ci) { subject.settings[:gitlab_ci] } + describe '#size_limits' do + before do + Gitlab::CurrentSettings.current_application_settings.update!( + max_attachment_size: 10, + receive_max_input_size: 20, + max_import_size: 30, + diff_max_patch_bytes: 409600, + max_artifacts_size: 50, + max_pages_size: 60, + snippet_size_limit: 70 + ) + end - it 'returns Settings.gitalb_ci' do - gitlab_ci.delete(:artifacts_max_size) + it 'returns size limits from application settings' do + size_limits = subject.settings[:size_limits] - expect(gitlab_ci).to eq(Settings.gitlab_ci.symbolize_keys) + expect(size_limits[:max_attachment_size]).to eq(10.megabytes) + expect(size_limits[:receive_max_input_size]).to eq(20.megabytes) + expect(size_limits[:max_import_size]).to eq(30.megabytes) + expect(size_limits[:diff_max_patch_bytes]).to eq(400.kilobytes) + expect(size_limits[:max_artifacts_size]).to eq(50.megabytes) + expect(size_limits[:max_pages_size]).to eq(60.megabytes) + expect(size_limits[:snippet_size_limit]).to eq(70.bytes) end - it 'returns the key artifacts_max_size' do - expect(gitlab_ci.keys).to include(:artifacts_max_size) + it 'returns nil if receive_max_input_size not set' do + Gitlab::CurrentSettings.current_application_settings.update!(receive_max_input_size: nil) + + size_limits = subject.settings[:size_limits] + + expect(size_limits[:receive_max_input_size]).to be_nil end - it 'returns the key artifacts_max_size with values' do - stub_application_setting(max_artifacts_size: 200) + it 'returns nil if set to 0 (unlimited)' do + Gitlab::CurrentSettings.current_application_settings.update!(max_import_size: 0, max_pages_size: 0) + + size_limits = subject.settings[:size_limits] - expect(gitlab_ci[:artifacts_max_size][:default]).to eq(100.megabytes) - expect(gitlab_ci[:artifacts_max_size][:value]).to eq(200.megabytes) + expect(size_limits[:max_import_size]).to be_nil + expect(size_limits[:max_pages_size]).to be_nil end end diff --git a/spec/models/integration_spec.rb b/spec/models/integration_spec.rb index f5f6a425fdd..8a06f7fac99 100644 --- a/spec/models/integration_spec.rb +++ b/spec/models/integration_spec.rb @@ -825,4 +825,20 @@ RSpec.describe Integration do .to include(*described_class::PROJECT_SPECIFIC_INTEGRATION_NAMES) end end + + describe '#password_fields' do + it 'returns all fields with type `password`' do + allow(subject).to receive(:fields).and_return([ + { name: 'password', type: 'password' }, + { name: 'secret', type: 'password' }, + { name: 'public', type: 'text' } + ]) + + expect(subject.password_fields).to match_array(%w[password secret]) + end + + it 'returns an empty array if no password fields exist' do + expect(subject.password_fields).to eq([]) + end + end end diff --git a/spec/models/integrations/datadog_spec.rb b/spec/models/integrations/datadog_spec.rb index 7049e64c2ce..9c3ff7aa35b 100644 --- a/spec/models/integrations/datadog_spec.rb +++ b/spec/models/integrations/datadog_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Integrations::Datadog do let(:active) { true } let(:dd_site) { 'datadoghq.com' } - let(:default_url) { 'https://webhooks-http-intake.logs.datadoghq.com/api/v2/webhook' } + let(:default_url) { 'https://webhook-intake.datadoghq.com/api/v2/webhook' } let(:api_url) { '' } let(:api_key) { SecureRandom.hex(32) } let(:dd_env) { 'ci' } @@ -66,7 +66,7 @@ RSpec.describe Integrations::Datadog do context 'with custom api_url' do let(:dd_site) { '' } - let(:api_url) { 'https://webhooks-http-intake.logs.datad0g.com/api/v2/webhook' } + let(:api_url) { 'https://webhook-intake.datad0g.com/api/v2/webhook' } it { is_expected.not_to validate_presence_of(:datadog_site) } it { is_expected.to validate_presence_of(:api_url) } @@ -108,7 +108,7 @@ RSpec.describe Integrations::Datadog do end context 'with custom URL' do - let(:api_url) { 'https://webhooks-http-intake.logs.datad0g.com/api/v2/webhook' } + let(:api_url) { 'https://webhook-intake.datad0g.com/api/v2/webhook' } it { is_expected.to eq(api_url + "?dd-api-key=#{api_key}&env=#{dd_env}&service=#{dd_service}") } diff --git a/spec/models/integrations/pipelines_email_spec.rb b/spec/models/integrations/pipelines_email_spec.rb index 761049f25fe..afd9d71ebc4 100644 --- a/spec/models/integrations/pipelines_email_spec.rb +++ b/spec/models/integrations/pipelines_email_spec.rb @@ -48,7 +48,7 @@ RSpec.describe Integrations::PipelinesEmail, :mailer do end it 'sends email' do - emails = receivers.map { |r| double(notification_email: r) } + emails = receivers.map { |r| double(notification_email_or_default: r) } should_only_email(*emails, kind: :bcc) end diff --git a/spec/models/integrations/prometheus_spec.rb b/spec/models/integrations/prometheus_spec.rb index f6f242bf58e..76e20f20a00 100644 --- a/spec/models/integrations/prometheus_spec.rb +++ b/spec/models/integrations/prometheus_spec.rb @@ -516,7 +516,7 @@ RSpec.describe Integrations::Prometheus, :use_clean_rails_memory_store_caching, name: 'google_iap_audience_client_id', title: 'Google IAP Audience Client ID', placeholder: s_('PrometheusService|IAP_CLIENT_ID.apps.googleusercontent.com'), - help: s_('PrometheusService|PrometheusService|The ID of the IAP-secured resource.'), + help: s_('PrometheusService|The ID of the IAP-secured resource.'), autocomplete: 'off', required: false }, diff --git a/spec/models/integrations/zentao_spec.rb b/spec/models/integrations/zentao_spec.rb new file mode 100644 index 00000000000..a1503ecc092 --- /dev/null +++ b/spec/models/integrations/zentao_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::Zentao do + let(:url) { 'https://jihudemo.zentao.net' } + let(:api_url) { 'https://jihudemo.zentao.net' } + let(:api_token) { 'ZENTAO_TOKEN' } + let(:zentao_product_xid) { '3' } + let(:zentao_integration) { create(:zentao_integration) } + + describe '#create' do + let(:project) { create(:project, :repository) } + let(:params) do + { + project: project, + url: url, + api_url: api_url, + api_token: api_token, + zentao_product_xid: zentao_product_xid + } + end + + it 'stores data in data_fields correctly' do + tracker_data = described_class.create!(params).zentao_tracker_data + + expect(tracker_data.url).to eq(url) + expect(tracker_data.api_url).to eq(api_url) + expect(tracker_data.api_token).to eq(api_token) + expect(tracker_data.zentao_product_xid).to eq(zentao_product_xid) + end + end + + describe '#fields' do + it 'returns custom fields' do + expect(zentao_integration.fields.pluck(:name)).to eq(%w[url api_url api_token zentao_product_xid]) + end + end + + describe '#test' do + let(:test_response) { { success: true } } + + before do + allow_next_instance_of(Gitlab::Zentao::Client) do |client| + allow(client).to receive(:ping).and_return(test_response) + end + end + + it 'gets response from Gitlab::Zentao::Client#ping' do + expect(zentao_integration.test).to eq(test_response) + end + end +end diff --git a/spec/models/integrations/zentao_tracker_data_spec.rb b/spec/models/integrations/zentao_tracker_data_spec.rb new file mode 100644 index 00000000000..b078c57830b --- /dev/null +++ b/spec/models/integrations/zentao_tracker_data_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Integrations::ZentaoTrackerData do + describe 'factory available' do + let(:zentao_tracker_data) { create(:zentao_tracker_data) } + + it { expect(zentao_tracker_data.valid?).to eq true } + end + + describe 'associations' do + it { is_expected.to belong_to(:integration) } + end + + describe 'encrypted attributes' do + subject { described_class.encrypted_attributes.keys } + + it { is_expected.to contain_exactly(:url, :api_url, :zentao_product_xid, :api_token) } + end +end diff --git a/spec/models/internal_id_spec.rb b/spec/models/internal_id_spec.rb index 6aba91d9471..51b27151ba2 100644 --- a/spec/models/internal_id_spec.rb +++ b/spec/models/internal_id_spec.rb @@ -39,266 +39,199 @@ RSpec.describe InternalId do end end - shared_examples_for 'a monotonically increasing id generator' do - describe '.generate_next' do - subject { described_class.generate_next(id_subject, scope, usage, init) } + describe '.generate_next' do + subject { described_class.generate_next(id_subject, scope, usage, init) } - context 'in the absence of a record' do - it 'creates a record if not yet present' do - expect { subject }.to change { described_class.count }.from(0).to(1) - end - - it 'stores record attributes' do - subject - - described_class.first.tap do |record| - expect(record.project).to eq(project) - expect(record.usage).to eq(usage.to_s) - end - end - - context 'with existing issues' do - before do - create_list(:issue, 2, project: project) - described_class.delete_all - end - - it 'calculates last_value values automatically' do - expect(subject).to eq(project.issues.size + 1) - end - end - end - - it 'generates a strictly monotone, gapless sequence' do - seq = Array.new(10).map do - described_class.generate_next(issue, scope, usage, init) - end - normalized = seq.map { |i| i - seq.min } - - expect(normalized).to eq((0..seq.size - 1).to_a) + context 'in the absence of a record' do + it 'creates a record if not yet present' do + expect { subject }.to change { described_class.count }.from(0).to(1) end - context 'there are no instances to pass in' do - let(:id_subject) { Issue } + it 'stores record attributes' do + subject - it 'accepts classes instead' do - expect(subject).to eq(1) + described_class.first.tap do |record| + expect(record.project).to eq(project) + expect(record.usage).to eq(usage.to_s) end end - context 'when executed outside of transaction' do - it 'increments counter with in_transaction: "false"' do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases - - expect(InternalId.internal_id_transactions_total).to receive(:increment) - .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original - - subject + context 'with existing issues' do + before do + create_list(:issue, 2, project: project) + described_class.delete_all end - end - context 'when executed within transaction' do - it 'increments counter with in_transaction: "true"' do - expect(InternalId.internal_id_transactions_total).to receive(:increment) - .with(operation: :generate, usage: 'issues', in_transaction: 'true').and_call_original - - InternalId.transaction { subject } + it 'calculates last_value values automatically' do + expect(subject).to eq(project.issues.size + 1) end end end - describe '.reset' do - subject { described_class.reset(issue, scope, usage, value) } - - context 'in the absence of a record' do - let(:value) { 2 } - - it 'does not revert back the value' do - expect { subject }.not_to change { described_class.count } - expect(subject).to be_falsey - end + it 'generates a strictly monotone, gapless sequence' do + seq = Array.new(10).map do + described_class.generate_next(issue, scope, usage, init) end + normalized = seq.map { |i| i - seq.min } - context 'when valid iid is used to reset' do - let!(:value) { generate_next } - - context 'and iid is a latest one' do - it 'does rewind and next generated value is the same' do - expect(subject).to be_truthy - expect(generate_next).to eq(value) - end - end + expect(normalized).to eq((0..seq.size - 1).to_a) + end - context 'and iid is not a latest one' do - it 'does not rewind' do - generate_next + context 'there are no instances to pass in' do + let(:id_subject) { Issue } - expect(subject).to be_falsey - expect(generate_next).to be > value - end - end - - def generate_next - described_class.generate_next(issue, scope, usage, init) - end + it 'accepts classes instead' do + expect(subject).to eq(1) end + end - context 'when executed outside of transaction' do - let(:value) { 2 } - - it 'increments counter with in_transaction: "false"' do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases + context 'when executed outside of transaction' do + it 'increments counter with in_transaction: "false"' do + allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases - expect(InternalId.internal_id_transactions_total).to receive(:increment) - .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original + expect(InternalId.internal_id_transactions_total).to receive(:increment) + .with(operation: :generate, usage: 'issues', in_transaction: 'false').and_call_original - subject - end + subject end + end - context 'when executed within transaction' do - let(:value) { 2 } - - it 'increments counter with in_transaction: "true"' do - expect(InternalId.internal_id_transactions_total).to receive(:increment) - .with(operation: :reset, usage: 'issues', in_transaction: 'true').and_call_original + context 'when executed within transaction' do + it 'increments counter with in_transaction: "true"' do + expect(InternalId.internal_id_transactions_total).to receive(:increment) + .with(operation: :generate, usage: 'issues', in_transaction: 'true').and_call_original - InternalId.transaction { subject } - end + InternalId.transaction { subject } end end + end - describe '.track_greatest' do - let(:value) { 9001 } - - subject { described_class.track_greatest(id_subject, scope, usage, value, init) } - - context 'in the absence of a record' do - it 'creates a record if not yet present' do - expect { subject }.to change { described_class.count }.from(0).to(1) - end - end + describe '.reset' do + subject { described_class.reset(issue, scope, usage, value) } - it 'stores record attributes' do - subject + context 'in the absence of a record' do + let(:value) { 2 } - described_class.first.tap do |record| - expect(record.project).to eq(project) - expect(record.usage).to eq(usage.to_s) - expect(record.last_value).to eq(value) - end + it 'does not revert back the value' do + expect { subject }.not_to change { described_class.count } + expect(subject).to be_falsey end + end - context 'with existing issues' do - before do - create(:issue, project: project) - described_class.delete_all - end + context 'when valid iid is used to reset' do + let!(:value) { generate_next } - it 'still returns the last value to that of the given value' do - expect(subject).to eq(value) + context 'and iid is a latest one' do + it 'does rewind and next generated value is the same' do + expect(subject).to be_truthy + expect(generate_next).to eq(value) end end - context 'when value is less than the current last_value' do - it 'returns the current last_value' do - described_class.create!(**scope, usage: usage, last_value: 10_001) + context 'and iid is not a latest one' do + it 'does not rewind' do + generate_next - expect(subject).to eq 10_001 + expect(subject).to be_falsey + expect(generate_next).to be > value end end - context 'there are no instances to pass in' do - let(:id_subject) { Issue } - - it 'accepts classes instead' do - expect(subject).to eq(value) - end + def generate_next + described_class.generate_next(issue, scope, usage, init) end + end - context 'when executed outside of transaction' do - it 'increments counter with in_transaction: "false"' do - allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases - - expect(InternalId.internal_id_transactions_total).to receive(:increment) - .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original + context 'when executed outside of transaction' do + let(:value) { 2 } - subject - end - end + it 'increments counter with in_transaction: "false"' do + allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases - context 'when executed within transaction' do - it 'increments counter with in_transaction: "true"' do - expect(InternalId.internal_id_transactions_total).to receive(:increment) - .with(operation: :track_greatest, usage: 'issues', in_transaction: 'true').and_call_original + expect(InternalId.internal_id_transactions_total).to receive(:increment) + .with(operation: :reset, usage: 'issues', in_transaction: 'false').and_call_original - InternalId.transaction { subject } - end + subject end end - end - context 'when the feature flag is disabled' do - stub_feature_flags(generate_iids_without_explicit_locking: false) + context 'when executed within transaction' do + let(:value) { 2 } - it_behaves_like 'a monotonically increasing id generator' - end - - context 'when the feature flag is enabled' do - stub_feature_flags(generate_iids_without_explicit_locking: true) + it 'increments counter with in_transaction: "true"' do + expect(InternalId.internal_id_transactions_total).to receive(:increment) + .with(operation: :reset, usage: 'issues', in_transaction: 'true').and_call_original - it_behaves_like 'a monotonically increasing id generator' + InternalId.transaction { subject } + end + end end - describe '#increment_and_save!' do - let(:id) { create(:internal_id) } - - subject { id.increment_and_save! } + describe '.track_greatest' do + let(:value) { 9001 } - it 'returns incremented iid' do - value = id.last_value + subject { described_class.track_greatest(id_subject, scope, usage, value, init) } - expect(subject).to eq(value + 1) + context 'in the absence of a record' do + it 'creates a record if not yet present' do + expect { subject }.to change { described_class.count }.from(0).to(1) + end end - it 'saves the record' do + it 'stores record attributes' do subject - expect(id.changed?).to be_falsey + described_class.first.tap do |record| + expect(record.project).to eq(project) + expect(record.usage).to eq(usage.to_s) + expect(record.last_value).to eq(value) + end end - context 'with last_value=nil' do - let(:id) { build(:internal_id, last_value: nil) } + context 'with existing issues' do + before do + create(:issue, project: project) + described_class.delete_all + end - it 'returns 1' do - expect(subject).to eq(1) + it 'still returns the last value to that of the given value' do + expect(subject).to eq(value) end end - end - - describe '#track_greatest_and_save!' do - let(:id) { create(:internal_id) } - let(:new_last_value) { 9001 } - subject { id.track_greatest_and_save!(new_last_value) } + context 'when value is less than the current last_value' do + it 'returns the current last_value' do + described_class.create!(**scope, usage: usage, last_value: 10_001) - it 'returns new last value' do - expect(subject).to eq new_last_value + expect(subject).to eq 10_001 + end end - it 'saves the record' do - subject + context 'there are no instances to pass in' do + let(:id_subject) { Issue } - expect(id.changed?).to be_falsey + it 'accepts classes instead' do + expect(subject).to eq(value) + end end - context 'when new last value is lower than the max' do - it 'does not update the last value' do - id.update!(last_value: 10_001) + context 'when executed outside of transaction' do + it 'increments counter with in_transaction: "false"' do + allow(ActiveRecord::Base.connection).to receive(:transaction_open?) { false } # rubocop: disable Database/MultipleDatabases + + expect(InternalId.internal_id_transactions_total).to receive(:increment) + .with(operation: :track_greatest, usage: 'issues', in_transaction: 'false').and_call_original subject + end + end + + context 'when executed within transaction' do + it 'increments counter with in_transaction: "true"' do + expect(InternalId.internal_id_transactions_total).to receive(:increment) + .with(operation: :track_greatest, usage: 'issues', in_transaction: 'true').and_call_original - expect(id.reload.last_value).to eq 10_001 + InternalId.transaction { subject } end end end diff --git a/spec/models/issue/metrics_spec.rb b/spec/models/issue/metrics_spec.rb index 49c891c20da..2fdf1f09f80 100644 --- a/spec/models/issue/metrics_spec.rb +++ b/spec/models/issue/metrics_spec.rb @@ -34,7 +34,7 @@ RSpec.describe Issue::Metrics do end end - describe "when recording the default set of issue metrics on issue save" do + context "when recording the default set of issue metrics on issue save" do context "milestones" do it "records the first time an issue is associated with a milestone" do time = Time.current @@ -80,20 +80,5 @@ RSpec.describe Issue::Metrics do expect(metrics.first_added_to_board_at).to be_like_time(time) end end - - describe "#record!" do - it "does not cause an N+1 query" do - label = create(:label) - subject.update!(label_ids: [label.id]) - - control_count = ActiveRecord::QueryRecorder.new { Issue::Metrics.find_by(issue: subject).record! }.count - - additional_labels = create_list(:label, 4) - - subject.update!(label_ids: additional_labels.map(&:id)) - - expect { Issue::Metrics.find_by(issue: subject).record! }.not_to exceed_query_limit(control_count) - end - end end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 116bda7a18b..1747972e8ae 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -102,7 +102,7 @@ RSpec.describe Issue do end it 'records current metrics' do - expect_any_instance_of(Issue::Metrics).to receive(:record!) + expect(Issue::Metrics).to receive(:record!) create(:issue, project: reusable_project) end @@ -111,7 +111,6 @@ RSpec.describe Issue do before do subject.metrics.delete subject.reload - subject.metrics # make sure metrics association is cached (currently nil) end it 'creates the metrics record' do @@ -166,8 +165,8 @@ RSpec.describe Issue do expect(described_class.simple_sorts.keys).to include( *%w(created_asc created_at_asc created_date created_desc created_at_desc closest_future_date closest_future_date_asc due_date due_date_asc due_date_desc - id_asc id_desc relative_position relative_position_asc - updated_desc updated_asc updated_at_asc updated_at_desc)) + id_asc id_desc relative_position relative_position_asc updated_desc updated_asc + updated_at_asc updated_at_desc title_asc title_desc)) end end @@ -204,6 +203,25 @@ RSpec.describe Issue do end end + describe '.order_title' do + let_it_be(:issue1) { create(:issue, title: 'foo') } + let_it_be(:issue2) { create(:issue, title: 'bar') } + let_it_be(:issue3) { create(:issue, title: 'baz') } + let_it_be(:issue4) { create(:issue, title: 'Baz 2') } + + context 'sorting ascending' do + subject { described_class.order_title_asc } + + it { is_expected.to eq([issue2, issue3, issue4, issue1]) } + end + + context 'sorting descending' do + subject { described_class.order_title_desc } + + it { is_expected.to eq([issue1, issue4, issue3, issue2]) } + end + end + describe '#order_by_position_and_priority' do let(:project) { reusable_project } let(:p1) { create(:label, title: 'P1', project: project, priority: 1) } @@ -1177,18 +1195,33 @@ RSpec.describe Issue do it 'refreshes the number of open issues of the project' do project = subject.project - expect { subject.destroy! } - .to change { project.open_issues_count }.from(1).to(0) + expect do + subject.destroy! + + BatchLoader::Executor.clear_current + end.to change { project.open_issues_count }.from(1).to(0) end end describe '.public_only' do - it 'only returns public issues' do - public_issue = create(:issue, project: reusable_project) - create(:issue, project: reusable_project, confidential: true) + let_it_be(:banned_user) { create(:user, :banned) } + let_it_be(:public_issue) { create(:issue, project: reusable_project) } + let_it_be(:confidential_issue) { create(:issue, project: reusable_project, confidential: true) } + let_it_be(:hidden_issue) { create(:issue, project: reusable_project, author: banned_user) } + it 'only returns public issues' do expect(described_class.public_only).to eq([public_issue]) end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(ban_user_feature_flag: false) + end + + it 'returns public and hidden issues' do + expect(described_class.public_only).to eq([public_issue, hidden_issue]) + end + end end describe '.confidential_only' do @@ -1402,19 +1435,19 @@ RSpec.describe Issue do describe 'scheduling rebalancing' do before do allow_next_instance_of(RelativePositioning::Mover) do |mover| - allow(mover).to receive(:move) { raise ActiveRecord::QueryCanceled } + allow(mover).to receive(:move) { raise RelativePositioning::NoSpaceLeft } end end shared_examples 'schedules issues rebalancing' do let(:issue) { build_stubbed(:issue, relative_position: 100, project: project) } - it 'schedules rebalancing if we time-out when moving' do + it 'schedules rebalancing if there is no space left' do lhs = build_stubbed(:issue, relative_position: 99, project: project) to_move = build(:issue, project: project) expect(IssueRebalancingWorker).to receive(:perform_async).with(nil, project_id, namespace_id) - expect { to_move.move_between(lhs, issue) }.to raise_error(ActiveRecord::QueryCanceled) + expect { to_move.move_between(lhs, issue) }.to raise_error(RelativePositioning::NoSpaceLeft) end end diff --git a/spec/models/loose_foreign_keys/deleted_record_spec.rb b/spec/models/loose_foreign_keys/deleted_record_spec.rb new file mode 100644 index 00000000000..db2f8b4d2d3 --- /dev/null +++ b/spec/models/loose_foreign_keys/deleted_record_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe LooseForeignKeys::DeletedRecord do + let_it_be(:deleted_record_1) { described_class.create!(created_at: 1.day.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 5) } + let_it_be(:deleted_record_2) { described_class.create!(created_at: 3.days.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 1) } + let_it_be(:deleted_record_3) { described_class.create!(created_at: 5.days.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 3) } + let_it_be(:deleted_record_4) { described_class.create!(created_at: 10.days.ago, deleted_table_name: 'projects', deleted_table_primary_key_value: 1) } # duplicate + + # skip created_at because it gets truncated after insert + def map_attributes(records) + records.pluck(:deleted_table_name, :deleted_table_primary_key_value) + end + + describe 'partitioning strategy' do + it 'has retain_non_empty_partitions option' do + expect(described_class.partitioning_strategy.retain_non_empty_partitions).to eq(true) + end + end + + describe '.load_batch' do + it 'loads records and orders them by creation date' do + records = described_class.load_batch(4) + + expect(map_attributes(records)).to eq([['projects', 1], ['projects', 3], ['projects', 1], ['projects', 5]]) + end + + it 'supports configurable batch size' do + records = described_class.load_batch(2) + + expect(map_attributes(records)).to eq([['projects', 1], ['projects', 3]]) + end + end + + describe '.delete_records' do + it 'deletes exactly one record' do + described_class.delete_records([deleted_record_2]) + + expect(described_class.count).to eq(3) + expect(described_class.find_by(created_at: deleted_record_2.created_at)).to eq(nil) + end + + it 'deletes two records' do + described_class.delete_records([deleted_record_2, deleted_record_4]) + + expect(described_class.count).to eq(2) + end + + it 'deletes all records' do + described_class.delete_records([deleted_record_1, deleted_record_2, deleted_record_3, deleted_record_4]) + + expect(described_class.count).to eq(0) + end + end +end diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb index 067b3c25645..3f7f69ff34e 100644 --- a/spec/models/member_spec.rb +++ b/spec/models/member_spec.rb @@ -645,6 +645,16 @@ RSpec.describe Member do expect(user.authorized_projects.reload).to include(project) end + + it 'does not accept the invite if saving a new user fails' do + invalid_user = User.new(first_name: '', last_name: '') + + member.accept_invite! invalid_user + + expect(member.invite_accepted_at).to be_nil + expect(member.invite_token).not_to be_nil + expect_any_instance_of(Member).not_to receive(:after_accept_invite) + end end describe "#decline_invite!" do diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 4a8a2909891..06ca88644b7 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -151,43 +151,6 @@ RSpec.describe MergeRequest, factory_default: :keep do end end - describe '#squash_in_progress?' do - let(:repo_path) do - Gitlab::GitalyClient::StorageSettings.allow_disk_access do - subject.source_project.repository.path - end - end - - let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") } - - before do - system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{squash_path} master)) - end - - it 'returns true when there is a current squash directory' do - expect(subject.squash_in_progress?).to be_truthy - end - - it 'returns false when there is no squash directory' do - FileUtils.rm_rf(squash_path) - - expect(subject.squash_in_progress?).to be_falsey - end - - it 'returns false when the squash directory has expired' do - time = 20.minutes.ago.to_time - File.utime(time, time, squash_path) - - expect(subject.squash_in_progress?).to be_falsey - end - - it 'returns false when the source project has been removed' do - allow(subject).to receive(:source_project).and_return(nil) - - expect(subject.squash_in_progress?).to be_falsey - end - end - describe '#squash?' do let(:merge_request) { build(:merge_request, squash: squash) } diff --git a/spec/models/milestone_spec.rb b/spec/models/milestone_spec.rb index f14b9c57eb1..bc592acc80f 100644 --- a/spec/models/milestone_spec.rb +++ b/spec/models/milestone_spec.rb @@ -538,15 +538,6 @@ RSpec.describe Milestone do it { is_expected.to match('gitlab-org/gitlab-ce%123') } it { is_expected.to match('gitlab-org/gitlab-ce%"my-milestone"') } - - context 'when milestone_reference_pattern feature flag is false' do - before do - stub_feature_flags(milestone_reference_pattern: false) - end - - it { is_expected.to match('gitlab-org/gitlab-ce%123') } - it { is_expected.to match('gitlab-org/gitlab-ce%"my-milestone"') } - end end describe '.link_reference_pattern' do diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index e8ed6f1a460..c1cc8fc3e88 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' RSpec.describe NamespaceSetting, type: :model do + it_behaves_like 'sanitizable', :namespace_settings, %i[default_branch_name] + # Relationships # describe "Associations" do @@ -41,14 +43,6 @@ RSpec.describe NamespaceSetting, type: :model do it_behaves_like "doesn't return an error" end - - context "when it contains javascript tags" do - it "gets sanitized properly" do - namespace_settings.update!(default_branch_name: "hello<script>alert(1)</script>") - - expect(namespace_settings.default_branch_name).to eq('hello') - end - end end describe '#allow_mfa_for_group' do diff --git a/spec/models/namespace_spec.rb b/spec/models/namespace_spec.rb index e2700378f5f..51a26d82daa 100644 --- a/spec/models/namespace_spec.rb +++ b/spec/models/namespace_spec.rb @@ -36,27 +36,34 @@ RSpec.describe Namespace do it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) } context 'validating the parent of a namespace' do - context 'when the namespace has no parent' do - it 'allows a namespace to have no parent associated with it' do - namespace = build(:namespace) - - expect(namespace).to be_valid - end + using RSpec::Parameterized::TableSyntax + + where(:parent_type, :child_type, :error) do + nil | 'User' | nil + nil | 'Group' | nil + nil | 'Project' | 'must be set for a project namespace' + 'Project' | 'User' | 'project namespace cannot be the parent of another namespace' + 'Project' | 'Group' | 'project namespace cannot be the parent of another namespace' + 'Project' | 'Project' | 'project namespace cannot be the parent of another namespace' + 'Group' | 'User' | 'cannot not be used for user namespace' + 'Group' | 'Group' | nil + 'Group' | 'Project' | nil + 'User' | 'User' | 'cannot not be used for user namespace' + 'User' | 'Group' | 'user namespace cannot be the parent of another namespace' + 'User' | 'Project' | nil end - context 'when the namespace has a parent' do - it 'does not allow a namespace to have a group as its parent' do - namespace = build(:namespace, parent: build(:group)) - - expect(namespace).not_to be_valid - expect(namespace.errors[:parent_id].first).to eq('a user namespace cannot have a parent') - end - - it 'does not allow a namespace to have another namespace as its parent' do - namespace = build(:namespace, parent: build(:namespace)) - - expect(namespace).not_to be_valid - expect(namespace.errors[:parent_id].first).to eq('a user namespace cannot have a parent') + with_them do + it 'validates namespace parent' do + parent = build(:namespace, type: parent_type) if parent_type + namespace = build(:namespace, type: child_type, parent: parent) + + if error + expect(namespace).not_to be_valid + expect(namespace.errors[:parent_id].first).to eq(error) + else + expect(namespace).to be_valid + end end end @@ -157,6 +164,65 @@ RSpec.describe Namespace do end end + describe 'handling STI', :aggregate_failures do + let(:namespace_type) { nil } + let(:parent) { nil } + let(:namespace) { Namespace.find(create(:namespace, type: namespace_type, parent: parent).id) } + + context 'creating a Group' do + let(:namespace_type) { 'Group' } + + it 'is valid' do + expect(namespace).to be_a(Group) + expect(namespace.kind).to eq('group') + expect(namespace.group?).to be_truthy + end + end + + context 'creating a ProjectNamespace' do + let(:namespace_type) { 'Project' } + let(:parent) { create(:group) } + + it 'is valid' do + expect(Namespace.find(namespace.id)).to be_a(Namespaces::ProjectNamespace) + expect(namespace.kind).to eq('project') + expect(namespace.project?).to be_truthy + end + end + + context 'creating a UserNamespace' do + let(:namespace_type) { 'User' } + + it 'is valid' do + # TODO: We create a normal Namespace until + # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68894 is ready + expect(Namespace.find(namespace.id)).to be_a(Namespace) + expect(namespace.kind).to eq('user') + expect(namespace.user?).to be_truthy + end + end + + context 'creating a default Namespace' do + let(:namespace_type) { nil } + + it 'is valid' do + expect(Namespace.find(namespace.id)).to be_a(Namespace) + expect(namespace.kind).to eq('user') + expect(namespace.user?).to be_truthy + end + end + + context 'creating an unknown Namespace type' do + let(:namespace_type) { 'One' } + + it 'defaults to a Namespace' do + expect(Namespace.find(namespace.id)).to be_a(Namespace) + expect(namespace.kind).to eq('user') + expect(namespace.user?).to be_truthy + end + end + end + describe 'scopes', :aggregate_failures do let_it_be(:namespace1) { create(:group, name: 'Namespace 1', path: 'namespace-1') } let_it_be(:namespace2) { create(:group, name: 'Namespace 2', path: 'namespace-2') } @@ -287,6 +353,12 @@ RSpec.describe Namespace do end end + describe '#owner_required?' do + specify { expect(build(:project_namespace).owner_required?).to be_falsey } + specify { expect(build(:group).owner_required?).to be_falsey } + specify { expect(build(:namespace).owner_required?).to be_truthy } + end + describe '#visibility_level_field' do it { expect(namespace.visibility_level_field).to eq(:visibility_level) } end @@ -1377,6 +1449,13 @@ RSpec.describe Namespace do expect { root_group.root_ancestor }.not_to exceed_query_limit(0) end + it 'returns root_ancestor for nested group with a single query' do + nested_group = create(:group, parent: root_group) + nested_group.reload + + expect { nested_group.root_ancestor }.not_to exceed_query_limit(1) + end + it 'returns the top most ancestor' do nested_group = create(:group, parent: root_group) deep_nested_group = create(:group, parent: nested_group) diff --git a/spec/models/namespaces/project_namespace_spec.rb b/spec/models/namespaces/project_namespace_spec.rb new file mode 100644 index 00000000000..f38e8aa85d0 --- /dev/null +++ b/spec/models/namespaces/project_namespace_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Namespaces::ProjectNamespace, type: :model do + describe 'relationships' do + it { is_expected.to have_one(:project).with_foreign_key(:project_namespace_id).inverse_of(:project_namespace) } + end + + describe 'validations' do + it { is_expected.not_to validate_presence_of :owner } + end + + context 'when deleting project namespace' do + # using delete rather than destroy due to `delete` skipping AR hooks/callbacks + # so it's ensured to work at the DB level. Uses ON DELETE CASCADE on foreign key + let_it_be(:project) { create(:project) } + let_it_be(:project_namespace) { create(:project_namespace, project: project) } + + it 'also deletes the associated project' do + project_namespace.delete + + expect { project_namespace.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end +end diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 0afdae2fc93..5e3773513f1 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -500,15 +500,15 @@ RSpec.describe Note do let_it_be(:ext_issue) { create(:issue, project: ext_proj) } shared_examples "checks references" do - it "returns true" do + it "returns false" do expect(note.system_note_with_references_visible_for?(ext_issue.author)).to be_falsy end - it "returns false" do + it "returns true" do expect(note.system_note_with_references_visible_for?(private_user)).to be_truthy end - it "returns false if user visible reference count set" do + it "returns true if user visible reference count set" do note.user_visible_reference_count = 1 note.total_reference_count = 1 @@ -516,7 +516,15 @@ RSpec.describe Note do expect(note.system_note_with_references_visible_for?(ext_issue.author)).to be_truthy end - it "returns true if ref count is 0" do + it "returns false if user visible reference count set but does not match total reference count" do + note.user_visible_reference_count = 1 + note.total_reference_count = 2 + + expect(note).not_to receive(:reference_mentionables) + expect(note.system_note_with_references_visible_for?(ext_issue.author)).to be_falsy + end + + it "returns false if ref count is 0" do note.user_visible_reference_count = 0 expect(note).not_to receive(:reference_mentionables) @@ -562,13 +570,35 @@ RSpec.describe Note do end it_behaves_like "checks references" + end - it "returns true if user visible reference count set and there is a private reference" do - note.user_visible_reference_count = 1 - note.total_reference_count = 2 + context "when there is a private issue and user reference" do + let_it_be(:ext_issue2) { create(:issue, project: ext_proj) } - expect(note).not_to receive(:reference_mentionables) - expect(note.system_note_with_references_visible_for?(ext_issue.author)).to be_falsy + let(:note) do + create :note, + noteable: ext_issue2, project: ext_proj, + note: "mentioned in #{private_issue.to_reference(ext_proj)} and pinged user #{private_user.to_reference}", + system: true + end + + it_behaves_like "checks references" + end + + context "when there is a publicly visible user reference" do + let(:note) do + create :note, + noteable: ext_issue, project: ext_proj, + note: "mentioned in #{ext_proj.owner.to_reference}", + system: true + end + + it "returns true for other users" do + expect(note.system_note_with_references_visible_for?(ext_issue.author)).to be_truthy + end + + it "returns true for anonymous users" do + expect(note.system_note_with_references_visible_for?(nil)).to be_truthy end end end @@ -1543,7 +1573,15 @@ RSpec.describe Note do let(:note) { build(:note) } it 'returns cache key and author cache key by default' do - expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}") + expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}:#{note.project.team.human_max_access(note.author_id)}") + end + + context 'when note has no author' do + let(:note) { build(:note, author: nil) } + + it 'returns cache key only' do + expect(note.post_processed_cache_key).to eq("#{note.cache_key}:") + end end context 'when note has redacted_note_html' do @@ -1554,7 +1592,7 @@ RSpec.describe Note do end it 'returns cache key with redacted_note_html sha' do - expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}:#{Digest::SHA1.hexdigest(redacted_note_html)}") + expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}:#{note.project.team.human_max_access(note.author_id)}:#{Digest::SHA1.hexdigest(redacted_note_html)}") end end end diff --git a/spec/models/operations/feature_flag_scope_spec.rb b/spec/models/operations/feature_flag_scope_spec.rb deleted file mode 100644 index dc83789fade..00000000000 --- a/spec/models/operations/feature_flag_scope_spec.rb +++ /dev/null @@ -1,391 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Operations::FeatureFlagScope do - describe 'associations' do - it { is_expected.to belong_to(:feature_flag) } - end - - describe 'validations' do - context 'when duplicate environment scope is going to be created' do - let!(:existing_feature_flag_scope) do - create(:operations_feature_flag_scope) - end - - let(:new_feature_flag_scope) do - build(:operations_feature_flag_scope, - feature_flag: existing_feature_flag_scope.feature_flag, - environment_scope: existing_feature_flag_scope.environment_scope) - end - - it 'validates uniqueness of environment scope' do - new_feature_flag_scope.save - - expect(new_feature_flag_scope.errors[:environment_scope]) - .to include("(#{existing_feature_flag_scope.environment_scope})" \ - " has already been taken") - end - end - - context 'when environment scope of a default scope is updated' do - let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag) } - let!(:scope_default) { feature_flag.default_scope } - - it 'keeps default scope intact' do - scope_default.update(environment_scope: 'review/*') - - expect(scope_default.errors[:environment_scope]) - .to include("cannot be changed from default scope") - end - end - - context 'when a default scope is destroyed' do - let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag) } - let!(:scope_default) { feature_flag.default_scope } - - it 'prevents from destroying the default scope' do - expect { scope_default.destroy! }.to raise_error(ActiveRecord::ReadOnlyRecord) - end - end - - describe 'strategy validations' do - it 'handles null strategies which can occur while adding the column during migration' do - scope = create(:operations_feature_flag_scope, active: true) - allow(scope).to receive(:strategies).and_return(nil) - - scope.active = false - scope.save - - expect(scope.errors[:strategies]).to be_empty - end - - it 'validates multiple strategies' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: "default", parameters: {} }, - { name: "invalid", parameters: {} }]) - - expect(scope.errors[:strategies]).not_to be_empty - end - - where(:invalid_value) do - [{}, 600, "bad", [{ name: 'default', parameters: {} }, 300]] - end - with_them do - it 'must be an array of strategy hashes' do - scope = create(:operations_feature_flag_scope) - - scope.strategies = invalid_value - scope.save - - expect(scope.errors[:strategies]).to eq(['must be an array of strategy hashes']) - end - end - - describe 'name' do - using RSpec::Parameterized::TableSyntax - - where(:name, :params, :expected) do - 'default' | {} | [] - 'gradualRolloutUserId' | { groupId: 'mygroup', percentage: '50' } | [] - 'userWithId' | { userIds: 'sam' } | [] - 5 | nil | ['strategy name is invalid'] - nil | nil | ['strategy name is invalid'] - "nothing" | nil | ['strategy name is invalid'] - "" | nil | ['strategy name is invalid'] - 40.0 | nil | ['strategy name is invalid'] - {} | nil | ['strategy name is invalid'] - [] | nil | ['strategy name is invalid'] - end - with_them do - it 'must be one of "default", "gradualRolloutUserId", or "userWithId"' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: name, parameters: params }]) - - expect(scope.errors[:strategies]).to eq(expected) - end - end - end - - describe 'parameters' do - context 'when the strategy name is gradualRolloutUserId' do - it 'must have parameters' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'gradualRolloutUserId' }]) - - expect(scope.errors[:strategies]).to eq(['parameters are invalid']) - end - - where(:invalid_parameters) do - [nil, {}, { percentage: '40', groupId: 'mygroup', userIds: '4' }, { percentage: '40' }, - { percentage: '40', groupId: 'mygroup', extra: nil }, { groupId: 'mygroup' }] - end - with_them do - it 'must have valid parameters for the strategy' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'gradualRolloutUserId', - parameters: invalid_parameters }]) - - expect(scope.errors[:strategies]).to eq(['parameters are invalid']) - end - end - - it 'allows the parameters in any order' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'gradualRolloutUserId', - parameters: { percentage: '10', groupId: 'mygroup' } }]) - - expect(scope.errors[:strategies]).to be_empty - end - - describe 'percentage' do - where(:invalid_value) do - [50, 40.0, { key: "value" }, "garbage", "00", "01", "101", "-1", "-10", "0100", - "1000", "10.0", "5%", "25%", "100hi", "e100", "30m", " ", "\r\n", "\n", "\t", - "\n10", "20\n", "\n100", "100\n", "\n ", nil] - end - with_them do - it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'gradualRolloutUserId', - parameters: { groupId: 'mygroup', percentage: invalid_value } }]) - - expect(scope.errors[:strategies]).to eq(['percentage must be a string between 0 and 100 inclusive']) - end - end - - where(:valid_value) do - %w[0 1 10 38 100 93] - end - with_them do - it 'must be a string value between 0 and 100 inclusive and without a percentage sign' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'gradualRolloutUserId', - parameters: { groupId: 'mygroup', percentage: valid_value } }]) - - expect(scope.errors[:strategies]).to eq([]) - end - end - end - - describe 'groupId' do - where(:invalid_value) do - [nil, 4, 50.0, {}, 'spaces bad', 'bad$', '%bad', '<bad', 'bad>', '!bad', - '.bad', 'Bad', 'bad1', "", " ", "b" * 33, "ba_d", "ba\nd"] - end - with_them do - it 'must be a string value of up to 32 lowercase characters' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'gradualRolloutUserId', - parameters: { groupId: invalid_value, percentage: '40' } }]) - - expect(scope.errors[:strategies]).to eq(['groupId parameter is invalid']) - end - end - - where(:valid_value) do - ["somegroup", "anothergroup", "okay", "g", "a" * 32] - end - with_them do - it 'must be a string value of up to 32 lowercase characters' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'gradualRolloutUserId', - parameters: { groupId: valid_value, percentage: '40' } }]) - - expect(scope.errors[:strategies]).to eq([]) - end - end - end - end - - context 'when the strategy name is userWithId' do - it 'must have parameters' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'userWithId' }]) - - expect(scope.errors[:strategies]).to eq(['parameters are invalid']) - end - - where(:invalid_parameters) do - [nil, { userIds: 'sam', percentage: '40' }, { userIds: 'sam', some: 'param' }, { percentage: '40' }, {}] - end - with_them do - it 'must have valid parameters for the strategy' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'userWithId', parameters: invalid_parameters }]) - - expect(scope.errors[:strategies]).to eq(['parameters are invalid']) - end - end - - describe 'userIds' do - where(:valid_value) do - ["", "sam", "1", "a", "uuid-of-some-kind", "sam,fred,tom,jane,joe,mike", - "gitlab@example.com", "123,4", "UPPER,Case,charActeRS", "0", - "$valid$email#2345#$%..{}+=-)?\\/@example.com", "spaces allowed", - "a" * 256, "a,#{'b' * 256},ccc", "many spaces"] - end - with_them do - it 'is valid with a string of comma separated values' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'userWithId', parameters: { userIds: valid_value } }]) - - expect(scope.errors[:strategies]).to be_empty - end - end - - where(:invalid_value) do - [1, 2.5, {}, [], nil, "123\n456", "1,2,3,12\t3", "\n", "\n\r", - "joe\r,sam", "1,2,2", "1,,2", "1,2,,,,", "b" * 257, "1, ,2", "tim, ,7", " ", - " ", " ,1", "1, ", " leading,1", "1,trailing ", "1, both ,2"] - end - with_them do - it 'is invalid' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'userWithId', parameters: { userIds: invalid_value } }]) - - expect(scope.errors[:strategies]).to include( - 'userIds must be a string of unique comma separated values each 256 characters or less' - ) - end - end - end - end - - context 'when the strategy name is default' do - it 'must have parameters' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'default' }]) - - expect(scope.errors[:strategies]).to eq(['parameters are invalid']) - end - - where(:invalid_value) do - [{ groupId: "hi", percentage: "7" }, "", "nothing", 7, nil, [], 2.5] - end - with_them do - it 'must be empty' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'default', - parameters: invalid_value }]) - - expect(scope.errors[:strategies]).to eq(['parameters are invalid']) - end - end - - it 'must be empty' do - feature_flag = create(:operations_feature_flag) - scope = described_class.create(feature_flag: feature_flag, - environment_scope: 'production', active: true, - strategies: [{ name: 'default', - parameters: {} }]) - - expect(scope.errors[:strategies]).to be_empty - end - end - end - end - end - - describe '.enabled' do - subject { described_class.enabled } - - let!(:feature_flag_scope) do - create(:operations_feature_flag_scope, active: active) - end - - context 'when scope is active' do - let(:active) { true } - - it 'returns the scope' do - is_expected.to include(feature_flag_scope) - end - end - - context 'when scope is inactive' do - let(:active) { false } - - it 'returns an empty array' do - is_expected.not_to include(feature_flag_scope) - end - end - end - - describe '.disabled' do - subject { described_class.disabled } - - let!(:feature_flag_scope) do - create(:operations_feature_flag_scope, active: active) - end - - context 'when scope is active' do - let(:active) { true } - - it 'returns an empty array' do - is_expected.not_to include(feature_flag_scope) - end - end - - context 'when scope is inactive' do - let(:active) { false } - - it 'returns the scope' do - is_expected.to include(feature_flag_scope) - end - end - end - - describe '.for_unleash_client' do - it 'returns scopes for the specified project' do - project1 = create(:project) - project2 = create(:project) - expected_feature_flag = create(:operations_feature_flag, project: project1) - create(:operations_feature_flag, project: project2) - - scopes = described_class.for_unleash_client(project1, 'sandbox').to_a - - expect(scopes).to contain_exactly(*expected_feature_flag.scopes) - end - - it 'returns a scope that matches exactly over a match with a wild card' do - project = create(:project) - feature_flag = create(:operations_feature_flag, project: project) - create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production*') - expected_scope = create(:operations_feature_flag_scope, feature_flag: feature_flag, environment_scope: 'production') - - scopes = described_class.for_unleash_client(project, 'production').to_a - - expect(scopes).to contain_exactly(expected_scope) - end - end -end diff --git a/spec/models/operations/feature_flag_spec.rb b/spec/models/operations/feature_flag_spec.rb index cb9da2aea34..d689632e2b4 100644 --- a/spec/models/operations/feature_flag_spec.rb +++ b/spec/models/operations/feature_flag_spec.rb @@ -49,28 +49,7 @@ RSpec.describe Operations::FeatureFlag do it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_presence_of(:name) } it { is_expected.to validate_uniqueness_of(:name).scoped_to(:project_id) } - it { is_expected.to define_enum_for(:version).with_values(legacy_flag: 1, new_version_flag: 2) } - - context 'a version 1 feature flag' do - it 'is valid if associated with Operations::FeatureFlagScope models' do - project = create(:project) - feature_flag = described_class.create!({ name: 'test', project: project, version: 1, - scopes_attributes: [{ environment_scope: '*', active: false }] }) - - expect(feature_flag).to be_valid - end - - it 'is invalid if associated with Operations::FeatureFlags::Strategy models' do - project = create(:project) - feature_flag = described_class.new({ name: 'test', project: project, version: 1, - strategies_attributes: [{ name: 'default', parameters: {} }] }) - - expect(feature_flag.valid?).to eq(false) - expect(feature_flag.errors.messages).to eq({ - version_associations: ["version 1 feature flags may not have strategies"] - }) - end - end + it { is_expected.to define_enum_for(:version).with_values(new_version_flag: 2) } context 'a version 2 feature flag' do it 'is invalid if associated with Operations::FeatureFlagScope models' do @@ -102,64 +81,9 @@ RSpec.describe Operations::FeatureFlag do end end - describe 'feature flag version' do - it 'defaults to 1 if unspecified' do - project = create(:project) - - feature_flag = described_class.create!(name: 'my_flag', project: project, active: true) - - expect(feature_flag).to be_valid - expect(feature_flag.version_before_type_cast).to eq(1) - end - end - - describe 'Scope creation' do - subject { described_class.new(**params) } - - let(:project) { create(:project) } - - let(:params) do - { name: 'test', project: project, scopes_attributes: scopes_attributes } - end - - let(:scopes_attributes) do - [{ environment_scope: '*', active: false }, - { environment_scope: 'review/*', active: true }] - end - - it { is_expected.to be_valid } - - context 'when the first scope is not wildcard' do - let(:scopes_attributes) do - [{ environment_scope: 'review/*', active: true }, - { environment_scope: '*', active: false }] - end - - it { is_expected.not_to be_valid } - end - end - describe 'the default scope' do let_it_be(:project) { create(:project) } - context 'with a version 1 feature flag' do - it 'creates a default scope' do - feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 1 }) - - expect(feature_flag.scopes.count).to eq(1) - expect(feature_flag.scopes.first.environment_scope).to eq('*') - end - - it 'allows specifying the default scope in the parameters' do - feature_flag = described_class.create!({ name: 'test', project: project, - scopes_attributes: [{ environment_scope: '*', active: false }, - { environment_scope: 'review/*', active: true }], version: 1 }) - - expect(feature_flag.scopes.count).to eq(2) - expect(feature_flag.scopes.first.environment_scope).to eq('*') - end - end - context 'with a version 2 feature flag' do it 'does not create a default scope' do feature_flag = described_class.create!({ name: 'test', project: project, scopes_attributes: [], version: 2 }) @@ -180,16 +104,6 @@ RSpec.describe Operations::FeatureFlag do end end - context 'when the feature flag is active and all scopes are inactive' do - let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: true) } - - it 'returns the flag' do - feature_flag.default_scope.update!(active: false) - - is_expected.to eq([feature_flag]) - end - end - context 'when the feature flag is inactive' do let!(:feature_flag) { create(:operations_feature_flag, active: false) } @@ -197,16 +111,6 @@ RSpec.describe Operations::FeatureFlag do is_expected.to be_empty end end - - context 'when the feature flag is inactive and all scopes are active' do - let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: false) } - - it 'does not return the flag' do - feature_flag.default_scope.update!(active: true) - - is_expected.to be_empty - end - end end describe '.disabled' do @@ -220,16 +124,6 @@ RSpec.describe Operations::FeatureFlag do end end - context 'when the feature flag is active and all scopes are inactive' do - let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: true) } - - it 'does not return the flag' do - feature_flag.default_scope.update!(active: false) - - is_expected.to be_empty - end - end - context 'when the feature flag is inactive' do let!(:feature_flag) { create(:operations_feature_flag, active: false) } @@ -237,16 +131,6 @@ RSpec.describe Operations::FeatureFlag do is_expected.to eq([feature_flag]) end end - - context 'when the feature flag is inactive and all scopes are active' do - let!(:feature_flag) { create(:operations_feature_flag, :legacy_flag, active: false) } - - it 'returns the flag' do - feature_flag.default_scope.update!(active: true) - - is_expected.to eq([feature_flag]) - end - end end describe '.for_unleash_client' do diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index 90910fcb7ce..450656e3e9c 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -2,6 +2,8 @@ require 'spec_helper' RSpec.describe Packages::PackageFile, type: :model do + using RSpec::Parameterized::TableSyntax + let_it_be(:project) { create(:project) } let_it_be(:package_file1) { create(:package_file, :xml, file_name: 'FooBar') } let_it_be(:package_file2) { create(:package_file, :xml, file_name: 'ThisIsATest') } @@ -139,6 +141,71 @@ RSpec.describe Packages::PackageFile, type: :model do end end + describe '.most_recent!' do + it { expect(described_class.most_recent!).to eq(debian_package.package_files.last) } + end + + describe '.most_recent_for' do + let_it_be(:package1) { create(:npm_package) } + let_it_be(:package2) { create(:npm_package) } + let_it_be(:package3) { create(:npm_package) } + let_it_be(:package4) { create(:npm_package) } + + let_it_be(:package_file2_2) { create(:package_file, :npm, package: package2) } + + let_it_be(:package_file3_2) { create(:package_file, :npm, package: package3) } + let_it_be(:package_file3_3) { create(:package_file, :npm, package: package3) } + + let_it_be(:package_file4_2) { create(:package_file, :npm, package: package2) } + let_it_be(:package_file4_3) { create(:package_file, :npm, package: package2) } + let_it_be(:package_file4_4) { create(:package_file, :npm, package: package2) } + + let(:most_recent_package_file1) { package1.package_files.recent.first } + let(:most_recent_package_file2) { package2.package_files.recent.first } + let(:most_recent_package_file3) { package3.package_files.recent.first } + let(:most_recent_package_file4) { package4.package_files.recent.first } + + subject { described_class.most_recent_for(packages) } + + where( + package_input1: [1, nil], + package_input2: [2, nil], + package_input3: [3, nil], + package_input4: [4, nil] + ) + + with_them do + let(:compact_inputs) { [package_input1, package_input2, package_input3, package_input4].compact } + let(:packages) do + ::Packages::Package.id_in( + compact_inputs.map { |pkg_number| public_send("package#{pkg_number}") } + .map(&:id) + ) + end + + let(:expected_package_files) { compact_inputs.map { |pkg_number| public_send("most_recent_package_file#{pkg_number}") } } + + it { is_expected.to contain_exactly(*expected_package_files) } + end + + context 'extra join and extra where' do + let_it_be(:helm_package) { create(:helm_package, without_package_files: true) } + let_it_be(:helm_package_file1) { create(:helm_package_file, channel: 'alpha') } + let_it_be(:helm_package_file2) { create(:helm_package_file, channel: 'alpha', package: helm_package) } + let_it_be(:helm_package_file3) { create(:helm_package_file, channel: 'beta', package: helm_package) } + let_it_be(:helm_package_file4) { create(:helm_package_file, channel: 'beta', package: helm_package) } + + let(:extra_join) { :helm_file_metadatum } + let(:extra_where) { { packages_helm_file_metadata: { channel: 'alpha' } } } + + subject { described_class.most_recent_for(Packages::Package.id_in(helm_package.id), extra_join: extra_join, extra_where: extra_where) } + + it 'returns the most recent package for the selected channel' do + expect(subject).to contain_exactly(helm_package_file2) + end + end + end + describe '#update_file_store callback' do let_it_be(:package_file) { build(:package_file, :nuget, size: nil) } diff --git a/spec/models/packages/package_spec.rb b/spec/models/packages/package_spec.rb index 4d4d4ad4fa9..99e5769fc1f 100644 --- a/spec/models/packages/package_spec.rb +++ b/spec/models/packages/package_spec.rb @@ -1165,4 +1165,47 @@ RSpec.describe Packages::Package, type: :model do it_behaves_like 'not enqueuing a sync worker job' end end + + describe '#create_build_infos!' do + let_it_be(:package) { create(:package) } + let_it_be(:pipeline) { create(:ci_pipeline) } + + let(:build) { double(pipeline: pipeline) } + + subject { package.create_build_infos!(build) } + + context 'with a valid build' do + it 'creates a build info' do + expect { subject }.to change { ::Packages::BuildInfo.count }.by(1) + + last_build = ::Packages::BuildInfo.last + expect(last_build.package).to eq(package) + expect(last_build.pipeline).to eq(pipeline) + end + + context 'with an already existing build info' do + let_it_be(:build_info) { create(:packages_build_info, package: package, pipeline: pipeline) } + + it 'does not create a build info' do + expect { subject }.not_to change { ::Packages::BuildInfo.count } + end + end + end + + context 'with a nil build' do + let(:build) { nil } + + it 'does not create a build info' do + expect { subject }.not_to change { ::Packages::BuildInfo.count } + end + end + + context 'with a build without a pipeline' do + let(:build) { double(pipeline: nil) } + + it 'does not create a build info' do + expect { subject }.not_to change { ::Packages::BuildInfo.count } + end + end + end end diff --git a/spec/models/preloaders/commit_status_preloader_spec.rb b/spec/models/preloaders/commit_status_preloader_spec.rb new file mode 100644 index 00000000000..85ea784335c --- /dev/null +++ b/spec/models/preloaders/commit_status_preloader_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::CommitStatusPreloader do + let_it_be(:pipeline) { create(:ci_pipeline) } + + let_it_be(:build1) { create(:ci_build, :tags, pipeline: pipeline) } + let_it_be(:build2) { create(:ci_build, :tags, pipeline: pipeline) } + let_it_be(:bridge1) { create(:ci_bridge, pipeline: pipeline) } + let_it_be(:bridge2) { create(:ci_bridge, pipeline: pipeline) } + let_it_be(:generic_commit_status1) { create(:generic_commit_status, pipeline: pipeline) } + let_it_be(:generic_commit_status2) { create(:generic_commit_status, pipeline: pipeline) } + + describe '#execute' do + let(:relations) { %i[pipeline metadata tags job_artifacts_archive downstream_pipeline] } + let(:statuses) { CommitStatus.where(commit_id: pipeline.id).all } + + subject(:execute) { described_class.new(statuses).execute(relations) } + + it 'prevents N+1 for specified relations', :use_sql_query_cache do + execute + + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + call_each_relation(statuses.sample(3)) + end + + expect do + call_each_relation(statuses) + end.to issue_same_number_of_queries_as(control_count) + end + + private + + def call_each_relation(statuses) + statuses.each do |status| + relations.each { |relation| status.public_send(relation) if status.respond_to?(relation) } + end + end + end +end diff --git a/spec/models/preloaders/merge_requests_preloader_spec.rb b/spec/models/preloaders/merge_requests_preloader_spec.rb new file mode 100644 index 00000000000..7108de2e491 --- /dev/null +++ b/spec/models/preloaders/merge_requests_preloader_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::MergeRequestsPreloader do + describe '#execute' do + let_it_be_with_refind(:merge_requests) { create_list(:merge_request, 3) } + let_it_be(:upvotes) { merge_requests.each { |m| create(:award_emoji, :upvote, awardable: m) } } + + it 'does not make n+1 queries' do + described_class.new(merge_requests).execute + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + # expectations make sure the queries execute + merge_requests.each do |m| + expect(m.target_project.project_feature).not_to be_nil + expect(m.lazy_upvotes_count).to eq(1) + end + end + + # 1 query for BatchLoader to load all upvotes at once + expect(control.count).to eq(1) + end + + it 'runs extra queries without preloading' do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + # expectations make sure the queries execute + merge_requests.each do |m| + expect(m.target_project.project_feature).not_to be_nil + expect(m.lazy_upvotes_count).to eq(1) + end + end + + # 4 queries per merge request = + # 1 to load merge request + # 1 to load project + # 1 to load project_feature + # 1 to load upvotes count + expect(control.count).to eq(4 * merge_requests.size) + end + end +end diff --git a/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb new file mode 100644 index 00000000000..8144e1ad233 --- /dev/null +++ b/spec/models/preloaders/user_max_access_level_in_groups_preloader_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Preloaders::UserMaxAccessLevelInGroupsPreloader do + let_it_be(:user) { create(:user) } + let_it_be(:group1) { create(:group, :private).tap { |g| g.add_developer(user) } } + let_it_be(:group2) { create(:group, :private).tap { |g| g.add_developer(user) } } + let_it_be(:group3) { create(:group, :private) } + + let(:max_query_regex) { /SELECT MAX\("members"\."access_level"\).+/ } + let(:groups) { [group1, group2, group3] } + + shared_examples 'executes N max member permission queries to the DB' do + it 'executes the specified max membership queries' do + queries = ActiveRecord::QueryRecorder.new do + groups.each { |group| user.can?(:read_group, group) } + end + + max_queries = queries.log.grep(max_query_regex) + + expect(max_queries.count).to eq(expected_query_count) + end + end + + context 'when the preloader is used', :request_store do + before do + described_class.new(groups, user).execute + end + + it_behaves_like 'executes N max member permission queries to the DB' do + # Will query all groups where the user is not already a member + let(:expected_query_count) { 1 } + end + + context 'when user has access but is not a direct member of the group' do + let(:groups) { [group1, group2, group3, create(:group, :private, parent: group1)] } + + it_behaves_like 'executes N max member permission queries to the DB' do + # One query for group with no access and another one where the user is not a direct member + let(:expected_query_count) { 2 } + end + end + end + + context 'when the preloader is not used', :request_store do + it_behaves_like 'executes N max member permission queries to the DB' do + let(:expected_query_count) { groups.count } + end + end +end diff --git a/spec/models/project_ci_cd_setting_spec.rb b/spec/models/project_ci_cd_setting_spec.rb index caab182cda8..406485d8cc8 100644 --- a/spec/models/project_ci_cd_setting_spec.rb +++ b/spec/models/project_ci_cd_setting_spec.rb @@ -21,12 +21,6 @@ RSpec.describe ProjectCiCdSetting do end end - describe '#job_token_scope_enabled' do - it 'is false by default' do - expect(described_class.new.job_token_scope_enabled).to be_falsey - end - end - describe '#default_git_depth' do let(:default_value) { described_class::DEFAULT_GIT_DEPTH } diff --git a/spec/models/project_feature_spec.rb b/spec/models/project_feature_spec.rb index 5f720f8c4f8..75e43ed9a67 100644 --- a/spec/models/project_feature_spec.rb +++ b/spec/models/project_feature_spec.rb @@ -41,18 +41,15 @@ RSpec.describe ProjectFeature do end end - context 'public features' do - features = ProjectFeature::FEATURES - %i(pages) + it_behaves_like 'access level validation', ProjectFeature::FEATURES - %i(pages) do + let(:container_features) { project.project_feature } + end - features.each do |feature| - it "does not allow public access level for #{feature}" do - project_feature = project.project_feature - field = "#{feature}_access_level".to_sym - project_feature.update_attribute(field, ProjectFeature::PUBLIC) + it 'allows public access level for :pages feature' do + project_feature = project.project_feature + project_feature.pages_access_level = ProjectFeature::PUBLIC - expect(project_feature.valid?).to be_falsy, "#{field} failed" - end - end + expect(project_feature.valid?).to be_truthy end describe 'default pages access level' do diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index d8f3a63d221..3989ddc31e8 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -16,6 +16,7 @@ RSpec.describe Project, factory_default: :keep do describe 'associations' do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:namespace) } + it { is_expected.to belong_to(:project_namespace).class_name('Namespaces::ProjectNamespace').with_foreign_key('project_namespace_id').inverse_of(:project) } it { is_expected.to belong_to(:creator).class_name('User') } it { is_expected.to belong_to(:pool_repository) } it { is_expected.to have_many(:users) } @@ -137,6 +138,8 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:timelogs) } it { is_expected.to have_many(:error_tracking_errors).class_name('ErrorTracking::Error') } it { is_expected.to have_many(:error_tracking_client_keys).class_name('ErrorTracking::ClientKey') } + it { is_expected.to have_many(:pending_builds).class_name('Ci::PendingBuild') } + it { is_expected.to have_many(:ci_feature_usages).class_name('Projects::CiFeatureUsage') } # GitLab Pages it { is_expected.to have_many(:pages_domains) } @@ -183,6 +186,20 @@ RSpec.describe Project, factory_default: :keep do end end + context 'when deleting project' do + # using delete rather than destroy due to `delete` skipping AR hooks/callbacks + # so it's ensured to work at the DB level. Uses AFTER DELETE trigger. + let_it_be(:project) { create(:project) } + let_it_be(:project_namespace) { create(:project_namespace, project: project) } + + it 'also deletes the associated ProjectNamespace' do + project.delete + + expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { project_namespace.reload }.to raise_error(ActiveRecord::RecordNotFound) + end + end + context 'when creating a new project' do let_it_be(:project) { create(:project) } @@ -602,6 +619,12 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#membership_locked?' do + it 'returns false' do + expect(build(:project)).not_to be_membership_locked + end + end + describe '#autoclose_referenced_issues' do context 'when DB entry is nil' do let(:project) { build(:project, autoclose_referenced_issues: nil) } @@ -1051,12 +1074,12 @@ RSpec.describe Project, factory_default: :keep do project.open_issues_count(user) end - it 'invokes the count service with no current_user' do - count_service = instance_double(Projects::OpenIssuesCountService) - expect(Projects::OpenIssuesCountService).to receive(:new).with(project, nil).and_return(count_service) - expect(count_service).to receive(:count) + it 'invokes the batch count service with no current_user' do + count_service = instance_double(Projects::BatchOpenIssuesCountService) + expect(Projects::BatchOpenIssuesCountService).to receive(:new).with([project]).and_return(count_service) + expect(count_service).to receive(:refresh_cache_and_retrieve_data).and_return({}) - project.open_issues_count + project.open_issues_count.to_s end end @@ -1257,19 +1280,19 @@ RSpec.describe Project, factory_default: :keep do end it 'returns an active external wiki' do - create(:service, project: project, type: 'ExternalWikiService', active: true) + create(:external_wiki_integration, project: project, active: true) is_expected.to be_kind_of(Integrations::ExternalWiki) end it 'does not return an inactive external wiki' do - create(:service, project: project, type: 'ExternalWikiService', active: false) + create(:external_wiki_integration, project: project, active: false) is_expected.to eq(nil) end it 'sets Project#has_external_wiki when it is nil' do - create(:service, project: project, type: 'ExternalWikiService', active: true) + create(:external_wiki_integration, project: project, active: true) project.update_column(:has_external_wiki, nil) expect { subject }.to change { project.has_external_wiki }.from(nil).to(true) @@ -1279,36 +1302,40 @@ RSpec.describe Project, factory_default: :keep do describe '#has_external_wiki' do let_it_be(:project) { create(:project) } - def subject + def has_external_wiki project.reload.has_external_wiki end - specify { is_expected.to eq(false) } + specify { expect(has_external_wiki).to eq(false) } - context 'when there is an active external wiki service' do - let!(:service) do - create(:service, project: project, type: 'ExternalWikiService', active: true) + context 'when there is an active external wiki integration' do + let(:active) { true } + + let!(:integration) do + create(:external_wiki_integration, project: project, active: active) end - specify { is_expected.to eq(true) } + specify { expect(has_external_wiki).to eq(true) } it 'becomes false if the external wiki service is destroyed' do expect do - Integration.find(service.id).delete - end.to change { subject }.to(false) + Integration.find(integration.id).delete + end.to change { has_external_wiki }.to(false) end it 'becomes false if the external wiki service becomes inactive' do expect do - service.update_column(:active, false) - end.to change { subject }.to(false) + integration.update_column(:active, false) + end.to change { has_external_wiki }.to(false) end - end - it 'is false when external wiki service is not active' do - create(:service, project: project, type: 'ExternalWikiService', active: false) + context 'when created as inactive' do + let(:active) { false } - is_expected.to eq(false) + it 'is false' do + expect(has_external_wiki).to eq(false) + end + end end end @@ -2536,7 +2563,7 @@ RSpec.describe Project, factory_default: :keep do end describe '#uses_default_ci_config?' do - let(:project) { build(:project)} + let(:project) { build(:project) } it 'has a custom ci config path' do project.ci_config_path = 'something_custom' @@ -2557,6 +2584,44 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#uses_external_project_ci_config?' do + subject(:uses_external_project_ci_config) { project.uses_external_project_ci_config? } + + let(:project) { build(:project) } + + context 'when ci_config_path is configured with external project' do + before do + project.ci_config_path = '.gitlab-ci.yml@hello/world' + end + + it { is_expected.to eq(true) } + end + + context 'when ci_config_path is nil' do + before do + project.ci_config_path = nil + end + + it { is_expected.to eq(false) } + end + + context 'when ci_config_path is configured with a file in the project' do + before do + project.ci_config_path = 'hello/world/gitlab-ci.yml' + end + + it { is_expected.to eq(false) } + end + + context 'when ci_config_path is configured with remote file' do + before do + project.ci_config_path = 'https://example.org/file.yml' + end + + it { is_expected.to eq(false) } + end + end + describe '#latest_successful_build_for_ref' do let_it_be(:project) { create(:project, :repository) } let_it_be(:pipeline) { create_pipeline(project) } @@ -3260,6 +3325,16 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#after_change_head_branch_does_not_exist' do + let_it_be(:project) { create(:project) } + + it 'adds an error to container if branch does not exist' do + expect do + project.after_change_head_branch_does_not_exist('unexisted-branch') + end.to change { project.errors.size }.from(0).to(1) + end + end + describe '#lfs_objects_for_repository_types' do let(:project) { create(:project) } @@ -4496,44 +4571,6 @@ RSpec.describe Project, factory_default: :keep do end end - describe '#legacy_remove_pages' do - let(:project) { create(:project).tap { |project| project.mark_pages_as_deployed } } - let(:pages_metadatum) { project.pages_metadatum } - let(:namespace) { project.namespace } - let(:pages_path) { project.pages_path } - - around do |example| - FileUtils.mkdir_p(pages_path) - begin - example.run - ensure - FileUtils.rm_rf(pages_path) - end - end - - it 'removes the pages directory and marks the project as not having pages deployed' do - expect_any_instance_of(Gitlab::PagesTransfer).to receive(:rename_project).and_return(true) - expect(PagesWorker).to receive(:perform_in).with(5.minutes, :remove, namespace.full_path, anything) - - expect { project.legacy_remove_pages }.to change { pages_metadatum.reload.deployed }.from(true).to(false) - end - - it 'does nothing if updates on legacy storage are disabled' do - allow(Settings.pages.local_store).to receive(:enabled).and_return(false) - - expect(Gitlab::PagesTransfer).not_to receive(:new) - expect(PagesWorker).not_to receive(:perform_in) - - project.legacy_remove_pages - end - - it 'is run when the project is destroyed' do - expect(project).to receive(:legacy_remove_pages).and_call_original - - expect { project.destroy! }.not_to raise_error - end - end - describe '#remove_export' do let(:project) { create(:project, :with_export) } @@ -7037,6 +7074,15 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#ci_config_external_project' do + subject(:ci_config_external_project) { project.ci_config_external_project } + + let(:other_project) { create(:project) } + let(:project) { build(:project, ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}") } + + it { is_expected.to eq(other_project) } + end + describe '#enabled_group_deploy_keys' do let_it_be(:project) { create(:project) } @@ -7131,15 +7177,96 @@ RSpec.describe Project, factory_default: :keep do end describe 'topics' do - let_it_be(:project) { create(:project, topic_list: 'topic1, topic2, topic3') } + let_it_be(:project) { create(:project, name: 'topic-project', topic_list: 'topic1, topic2, topic3') } it 'topic_list returns correct string array' do - expect(project.topic_list).to match_array(%w[topic1 topic2 topic3]) + expect(project.topic_list).to eq(%w[topic1 topic2 topic3]) + end + + it 'topics returns correct topic records' do + expect(project.topics.first.class.name).to eq('Projects::Topic') + expect(project.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) + end + + context 'topic_list=' do + using RSpec::Parameterized::TableSyntax + + where(:topic_list, :expected_result) do + ['topicA', 'topicB'] | %w[topicA topicB] # rubocop:disable Style/WordArray, Lint/BinaryOperatorWithIdenticalOperands + ['topicB', 'topicA'] | %w[topicB topicA] # rubocop:disable Style/WordArray, Lint/BinaryOperatorWithIdenticalOperands + [' topicC ', ' topicD '] | %w[topicC topicD] + ['topicE', 'topicF', 'topicE'] | %w[topicE topicF] # rubocop:disable Style/WordArray + ['topicE ', 'topicF', ' topicE'] | %w[topicE topicF] + 'topicA, topicB' | %w[topicA topicB] + 'topicB, topicA' | %w[topicB topicA] + ' topicC , topicD ' | %w[topicC topicD] + 'topicE, topicF, topicE' | %w[topicE topicF] + 'topicE , topicF, topicE' | %w[topicE topicF] + end + + with_them do + it 'set topics' do + project.topic_list = topic_list + project.save! + + expect(project.topics.map(&:name)).to eq(expected_result) + end + end + + it 'set topics if only the order is changed' do + project.topic_list = 'topicA, topicB' + project.save! + + expect(project.reload.topics.map(&:name)).to eq(%w[topicA topicB]) + + project.topic_list = 'topicB, topicA' + project.save! + + expect(project.reload.topics.map(&:name)).to eq(%w[topicB topicA]) + end + + it 'does not persist topics before project is saved' do + project.topic_list = 'topicA, topicB' + + expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) + end + + it 'does not update topics if project is not valid' do + project.name = nil + project.topic_list = 'topicA, topicB' + + expect(project.save).to be_falsy + expect(project.reload.topics.map(&:name)).to eq(%w[topic1 topic2 topic3]) + end end - it 'topics returns correct tag records' do - expect(project.topics.first.class.name).to eq('ActsAsTaggableOn::Tag') - expect(project.topics.map(&:name)).to match_array(%w[topic1 topic2 topic3]) + context 'during ExtractProjectTopicsIntoSeparateTable migration' do + before do + topic_a = ActsAsTaggableOn::Tag.find_or_create_by!(name: 'topicA') + topic_b = ActsAsTaggableOn::Tag.find_or_create_by!(name: 'topicB') + + project.reload.topics_acts_as_taggable = [topic_a, topic_b] + project.save! + project.reload + end + + it 'topic_list returns correct string array' do + expect(project.topic_list).to eq(%w[topicA topicB topic1 topic2 topic3]) + end + + it 'topics returns correct topic records' do + expect(project.topics.map(&:class)).to eq([ActsAsTaggableOn::Tag, ActsAsTaggableOn::Tag, Projects::Topic, Projects::Topic, Projects::Topic]) + expect(project.topics.map(&:name)).to eq(%w[topicA topicB topic1 topic2 topic3]) + end + + it 'topic_list= sets new topics and removes old topics' do + project.topic_list = 'new-topic1, new-topic2' + project.save! + project.reload + + expect(project.topics.map(&:class)).to eq([Projects::Topic, Projects::Topic]) + expect(project.topics.map(&:name)).to eq(%w[new-topic1 new-topic2]) + end end end diff --git a/spec/models/projects/project_topic_spec.rb b/spec/models/projects/project_topic_spec.rb new file mode 100644 index 00000000000..c7a989040c7 --- /dev/null +++ b/spec/models/projects/project_topic_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::ProjectTopic do + let_it_be(:project_topic, reload: true) { create(:project_topic) } + + subject { project_topic } + + it { expect(subject).to be_valid } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to belong_to(:topic) } + end +end diff --git a/spec/models/projects/topic_spec.rb b/spec/models/projects/topic_spec.rb new file mode 100644 index 00000000000..409dc932709 --- /dev/null +++ b/spec/models/projects/topic_spec.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::Topic do + let_it_be(:topic, reload: true) { create(:topic) } + + subject { topic } + + it { expect(subject).to be_valid } + + describe 'associations' do + it { is_expected.to have_many(:project_topics) } + it { is_expected.to have_many(:projects) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:name) } + it { is_expected.to validate_uniqueness_of(:name) } + it { is_expected.to validate_length_of(:name).is_at_most(255) } + end +end diff --git a/spec/models/protected_branch_spec.rb b/spec/models/protected_branch_spec.rb index a173ab48f17..019c01af672 100644 --- a/spec/models/protected_branch_spec.rb +++ b/spec/models/protected_branch_spec.rb @@ -162,6 +162,30 @@ RSpec.describe ProtectedBranch do expect(described_class.protected?(project, 'staging/some-branch')).to eq(false) end + + context 'with caching', :use_clean_rails_memory_store_caching do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:protected_branch) { create(:protected_branch, project: project, name: "jawn") } + + before do + allow(described_class).to receive(:matching).once.and_call_original + # the original call works and warms the cache + described_class.protected?(project, 'jawn') + end + + it 'correctly invalidates a cache' do + expect(described_class).to receive(:matching).once.and_call_original + + create(:protected_branch, project: project, name: "bar") + # the cache is invalidated because the project has been "updated" + expect(described_class.protected?(project, 'jawn')).to eq(true) + end + + it 'correctly uses the cached version' do + expect(described_class).not_to receive(:matching) + expect(described_class.protected?(project, 'jawn')).to eq(true) + end + end end context 'new project' do diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 211e448b6cf..dc55214c1dd 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -68,51 +68,69 @@ RSpec.describe Repository do describe 'tags_sorted_by' do let(:tags_to_compare) { %w[v1.0.0 v1.1.0] } + let(:feature_flag) { true } + + before do + stub_feature_flags(gitaly_tags_finder: feature_flag) + end context 'name_desc' do subject { repository.tags_sorted_by('name_desc').map(&:name) & tags_to_compare } it { is_expected.to eq(['v1.1.0', 'v1.0.0']) } + + context 'when feature flag is disabled' do + let(:feature_flag) { false } + + it { is_expected.to eq(['v1.1.0', 'v1.0.0']) } + end end context 'name_asc' do subject { repository.tags_sorted_by('name_asc').map(&:name) & tags_to_compare } it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } + + context 'when feature flag is disabled' do + let(:feature_flag) { false } + + it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } + end end context 'updated' do - let(:tag_a) { repository.find_tag('v1.0.0') } - let(:tag_b) { repository.find_tag('v1.1.0') } + let(:latest_tag) { 'v0.0.0' } + + before do + rugged_repo(repository).tags.create(latest_tag, repository.commit.id) + end + + after do + rugged_repo(repository).tags.delete(latest_tag) + end context 'desc' do - subject { repository.tags_sorted_by('updated_desc').map(&:name) } + subject { repository.tags_sorted_by('updated_desc').map(&:name) & (tags_to_compare + [latest_tag]) } - before do - double_first = double(committed_date: Time.current) - double_last = double(committed_date: Time.current - 1.second) + it { is_expected.to eq([latest_tag, 'v1.1.0', 'v1.0.0']) } - allow(tag_a).to receive(:dereferenced_target).and_return(double_first) - allow(tag_b).to receive(:dereferenced_target).and_return(double_last) - allow(repository).to receive(:tags).and_return([tag_a, tag_b]) - end + context 'when feature flag is disabled' do + let(:feature_flag) { false } - it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } + it { is_expected.to eq([latest_tag, 'v1.1.0', 'v1.0.0']) } + end end context 'asc' do - subject { repository.tags_sorted_by('updated_asc').map(&:name) } + subject { repository.tags_sorted_by('updated_asc').map(&:name) & (tags_to_compare + [latest_tag]) } - before do - double_first = double(committed_date: Time.current - 1.second) - double_last = double(committed_date: Time.current) + it { is_expected.to eq(['v1.0.0', 'v1.1.0', latest_tag]) } - allow(tag_a).to receive(:dereferenced_target).and_return(double_last) - allow(tag_b).to receive(:dereferenced_target).and_return(double_first) - allow(repository).to receive(:tags).and_return([tag_a, tag_b]) - end + context 'when feature flag is disabled' do + let(:feature_flag) { false } - it { is_expected.to eq(['v1.1.0', 'v1.0.0']) } + it { is_expected.to eq(['v1.0.0', 'v1.1.0', latest_tag]) } + end end context 'annotated tag pointing to a blob' do @@ -125,29 +143,32 @@ RSpec.describe Repository do tagger: { name: 'John Smith', email: 'john@gmail.com' } } rugged_repo(repository).tags.create(annotated_tag_name, 'a48e4fc218069f68ef2e769dd8dfea3991362175', **options) + end - double_first = double(committed_date: Time.current - 1.second) - double_last = double(committed_date: Time.current) + it { is_expected.to eq(['v1.0.0', 'v1.1.0', annotated_tag_name]) } - allow(tag_a).to receive(:dereferenced_target).and_return(double_last) - allow(tag_b).to receive(:dereferenced_target).and_return(double_first) - end + context 'when feature flag is disabled' do + let(:feature_flag) { false } - it { is_expected.to eq(['v1.1.0', 'v1.0.0', annotated_tag_name]) } + it { is_expected.to eq(['v1.0.0', 'v1.1.0', annotated_tag_name]) } + end after do rugged_repo(repository).tags.delete(annotated_tag_name) end end end - end - describe '#ref_name_for_sha' do - it 'returns the ref' do - allow(repository.raw_repository).to receive(:ref_name_for_sha) - .and_return('refs/environments/production/77') + context 'unknown option' do + subject { repository.tags_sorted_by('unknown_desc').map(&:name) & tags_to_compare } + + it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } - expect(repository.ref_name_for_sha('bla', '0' * 40)).to eq 'refs/environments/production/77' + context 'when feature flag is disabled' do + let(:feature_flag) { false } + + it { is_expected.to eq(['v1.0.0', 'v1.1.0']) } + end end end @@ -479,6 +500,29 @@ RSpec.describe Repository do end end + describe '#commits_between' do + let(:commit) { project.commit } + + it 'delegates to Gitlab::Git::Commit#between, returning decorated commits' do + expect(Gitlab::Git::Commit) + .to receive(:between) + .with(repository.raw_repository, commit.parent_id, commit.id, limit: 5) + .and_call_original + + result = repository.commits_between(commit.parent_id, commit.id, limit: 5) + + expect(result).to contain_exactly(instance_of(Commit), instance_of(Commit)) + end + + it 'defaults to no limit' do + expect(Gitlab::Git::Commit) + .to receive(:between) + .with(repository.raw_repository, commit.parent_id, commit.id, limit: nil) + + repository.commits_between(commit.parent_id, commit.id) + end + end + describe '#find_commits_by_message' do it 'returns commits with messages containing a given string' do commit_ids = repository.find_commits_by_message('submodule').map(&:id) @@ -1294,6 +1338,15 @@ RSpec.describe Repository do expect(repository.license).to be_nil end + it 'returns nil when license_key is not recognized' do + expect(repository).to receive(:license_key).twice.and_return('not-recognized') + expect(Gitlab::ErrorTracking).to receive(:track_exception) do |ex| + expect(ex).to be_a(Licensee::InvalidLicense) + end + + expect(repository.license).to be_nil + end + it 'returns other when the content is not recognizable' do license = Licensee::License.new('other') repository.create_file(user, 'LICENSE', 'Gitlab B.V.', @@ -1773,7 +1826,7 @@ RSpec.describe Repository do expect(merge_commit.message).to eq('Custom message') expect(merge_commit.author_name).to eq(user.name) - expect(merge_commit.author_email).to eq(user.commit_email) + expect(merge_commit.author_email).to eq(user.commit_email_or_default) expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present end end @@ -2313,6 +2366,42 @@ RSpec.describe Repository do end end + describe '#find_tag' do + before do + allow(Gitlab::GitalyClient).to receive(:call).and_call_original + end + + it 'finds a tag with specified name by performing FindTag request' do + expect(Gitlab::GitalyClient) + .to receive(:call).with(anything, :ref_service, :find_tag, anything, anything).and_call_original + + expect(repository.find_tag('v1.1.0').name).to eq('v1.1.0') + end + + it 'does not perform Gitaly call when tags are preloaded' do + repository.tags + + expect(Gitlab::GitalyClient).not_to receive(:call) + + expect(repository.find_tag('v1.1.0').name).to eq('v1.1.0') + end + + it 'returns nil when tag does not exists' do + expect(repository.find_tag('does-not-exist')).to be_nil + end + + context 'when find_tag_via_gitaly is disabled' do + it 'fetches all tags' do + stub_feature_flags(find_tag_via_gitaly: false) + + expect(Gitlab::GitalyClient) + .to receive(:call).with(anything, :ref_service, :find_all_tags, anything, anything).and_call_original + + expect(repository.find_tag('v1.1.0').name).to eq('v1.1.0') + end + end + end + describe '#avatar' do it 'returns nil if repo does not exist' do allow(repository).to receive(:root_ref).and_raise(Gitlab::Git::Repository::NoRepository) @@ -3230,26 +3319,54 @@ RSpec.describe Repository do describe '#change_head' do let(:branch) { repository.container.default_branch } - it 'adds an error to container if branch does not exist' do - expect(repository.change_head('unexisted-branch')).to be false - expect(repository.container.errors.size).to eq(1) - end + context 'when the branch exists' do + it 'returns truthy' do + expect(repository.change_head(branch)).to be_truthy + end - it 'calls the before_change_head and after_change_head methods' do - expect(repository).to receive(:before_change_head) - expect(repository).to receive(:after_change_head) + it 'does not call container.after_change_head_branch_does_not_exist' do + expect(repository.container).not_to receive(:after_change_head_branch_does_not_exist) - repository.change_head(branch) - end + repository.change_head(branch) + end + + it 'calls repository hooks' do + expect(repository).to receive(:before_change_head) + expect(repository).to receive(:after_change_head) - it 'copies the gitattributes' do - expect(repository).to receive(:copy_gitattributes).with(branch) - repository.change_head(branch) + repository.change_head(branch) + end + + it 'copies the gitattributes' do + expect(repository).to receive(:copy_gitattributes).with(branch) + repository.change_head(branch) + end + + it 'reloads the default branch' do + expect(repository.container).to receive(:reload_default_branch) + repository.change_head(branch) + end end - it 'reloads the default branch' do - expect(repository.container).to receive(:reload_default_branch) - repository.change_head(branch) + context 'when the branch does not exist' do + let(:branch) { 'non-existent-branch' } + + it 'returns falsey' do + expect(repository.change_head(branch)).to be_falsey + end + + it 'calls container.after_change_head_branch_does_not_exist' do + expect(repository.container).to receive(:after_change_head_branch_does_not_exist).with(branch) + + repository.change_head(branch) + end + + it 'does not call repository hooks' do + expect(repository).not_to receive(:before_change_head) + expect(repository).not_to receive(:after_change_head) + + repository.change_head(branch) + end end end end diff --git a/spec/models/shard_spec.rb b/spec/models/shard_spec.rb index a9d11f4290c..38729fa1758 100644 --- a/spec/models/shard_spec.rb +++ b/spec/models/shard_spec.rb @@ -33,19 +33,21 @@ RSpec.describe Shard do expect(result.name).to eq('foo') end - it 'retries if creation races' do + it 'returns existing record if creation races' do + shard_created_by_others = double(described_class) + expect(described_class) - .to receive(:find_or_create_by) - .with(name: 'default') - .and_raise(ActiveRecord::RecordNotUnique, 'fail') - .once + .to receive(:find_by) + .with(name: 'new_shard') + .and_return(nil, shard_created_by_others) expect(described_class) - .to receive(:find_or_create_by) - .with(name: 'default') - .and_call_original + .to receive(:create) + .with(name: 'new_shard') + .and_raise(ActiveRecord::RecordNotUnique, 'fail') + .once - expect(described_class.by_name('default')).to eq(default_shard) + expect(described_class.by_name('new_shard')).to eq(shard_created_by_others) end end end diff --git a/spec/models/user_callout_spec.rb b/spec/models/user_callout_spec.rb index eb66f074293..5b36c8450ea 100644 --- a/spec/models/user_callout_spec.rb +++ b/spec/models/user_callout_spec.rb @@ -3,29 +3,12 @@ require 'spec_helper' RSpec.describe UserCallout do - let!(:callout) { create(:user_callout) } + let_it_be(:callout) { create(:user_callout) } it_behaves_like 'having unique enum values' - describe 'relationships' do - it { is_expected.to belong_to(:user) } - end - describe 'validations' do - it { is_expected.to validate_presence_of(:user) } - it { is_expected.to validate_presence_of(:feature_name) } it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id).ignoring_case_sensitivity } end - - describe '#dismissed_after?' do - let(:some_feature_name) { described_class.feature_names.keys.second } - let(:callout_dismissed_month_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.month.ago )} - let(:callout_dismissed_day_ago) { create(:user_callout, feature_name: some_feature_name, dismissed_at: 1.day.ago )} - - it 'returns whether a callout dismissed after specified date' do - expect(callout_dismissed_month_ago.dismissed_after?(15.days.ago)).to eq(false) - expect(callout_dismissed_day_ago.dismissed_after?(15.days.ago)).to eq(true) - end - end end diff --git a/spec/models/user_detail_spec.rb b/spec/models/user_detail_spec.rb index 3c87dcdcbd9..ba7ea3f7ce2 100644 --- a/spec/models/user_detail_spec.rb +++ b/spec/models/user_detail_spec.rb @@ -25,29 +25,4 @@ RSpec.describe UserDetail do it { is_expected.to validate_length_of(:bio).is_at_most(255) } end end - - describe '#bio_html' do - let(:user) { create(:user, bio: 'some **bio**') } - - subject { user.user_detail.bio_html } - - it 'falls back to #bio when the html representation is missing' do - user.user_detail.update!(bio_html: nil) - - expect(subject).to eq(user.user_detail.bio) - end - - it 'stores rendered html' do - expect(subject).to include('some <strong>bio</strong>') - end - - it 'does not try to set the value when the column is not there' do - without_bio_html_column = UserDetail.column_names - ['bio_html'] - - expect(described_class).to receive(:column_names).at_least(:once).and_return(without_bio_html_column) - expect(user.user_detail).not_to receive(:bio_html=) - - subject - end - end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index d73bc95a2f2..334f9b4ae30 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -65,9 +65,6 @@ RSpec.describe User do it { is_expected.to delegate_method(:render_whitespace_in_code).to(:user_preference) } it { is_expected.to delegate_method(:render_whitespace_in_code=).to(:user_preference).with_arguments(:args) } - it { is_expected.to delegate_method(:experience_level).to(:user_preference) } - it { is_expected.to delegate_method(:experience_level=).to(:user_preference).with_arguments(:args) } - it { is_expected.to delegate_method(:markdown_surround_selection).to(:user_preference) } it { is_expected.to delegate_method(:markdown_surround_selection=).to(:user_preference).with_arguments(:args) } @@ -82,7 +79,6 @@ RSpec.describe User do it { is_expected.to delegate_method(:bio).to(:user_detail).allow_nil } it { is_expected.to delegate_method(:bio=).to(:user_detail).with_arguments(:args).allow_nil } - it { is_expected.to delegate_method(:bio_html).to(:user_detail).allow_nil } end describe 'associations' do @@ -110,7 +106,6 @@ RSpec.describe User do it { is_expected.to have_many(:spam_logs).dependent(:destroy) } it { is_expected.to have_many(:todos) } it { is_expected.to have_many(:award_emoji).dependent(:destroy) } - it { is_expected.to have_many(:triggers).dependent(:destroy) } it { is_expected.to have_many(:builds).dependent(:nullify) } it { is_expected.to have_many(:pipelines).dependent(:nullify) } it { is_expected.to have_many(:chat_names).dependent(:destroy) } @@ -125,6 +120,8 @@ RSpec.describe User do it { is_expected.to have_many(:created_custom_emoji).inverse_of(:creator) } it { is_expected.to have_many(:in_product_marketing_emails) } it { is_expected.to have_many(:timelogs) } + it { is_expected.to have_many(:callouts).class_name('UserCallout') } + it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout') } describe "#user_detail" do it 'does not persist `user_detail` by default' do @@ -404,7 +401,7 @@ RSpec.describe User do user = build(:user, username: "test.#{type}") expect(user).not_to be_valid - expect(user.errors.full_messages).to include('Username ending with MIME type format is not allowed.') + expect(user.errors.full_messages).to include('Username ending with a file extension is not allowed.') expect(build(:user, username: "test#{type}")).to be_valid end end @@ -438,12 +435,12 @@ RSpec.describe User do subject { create(:user).tap { |user| user.emails << build(:email, email: email_value, confirmed_at: Time.current) } } end - describe '#commit_email' do + describe '#commit_email_or_default' do subject(:user) { create(:user) } it 'defaults to the primary email' do expect(user.email).to be_present - expect(user.commit_email).to eq(user.email) + expect(user.commit_email_or_default).to eq(user.email) end it 'defaults to the primary email when the column in the database is null' do @@ -451,38 +448,37 @@ RSpec.describe User do found_user = described_class.find_by(id: user.id) - expect(found_user.commit_email).to eq(user.email) + expect(found_user.commit_email_or_default).to eq(user.email) end it 'returns the private commit email when commit_email has _private' do user.update_column(:commit_email, Gitlab::PrivateCommitEmail::TOKEN) - expect(user.commit_email).to eq(user.private_commit_email) + expect(user.commit_email_or_default).to eq(user.private_commit_email) end + end + + describe '#commit_email=' do + subject(:user) { create(:user) } it 'can be set to a confirmed email' do confirmed = create(:email, :confirmed, user: user) user.commit_email = confirmed.email expect(user).to be_valid - expect(user.commit_email).to eq(confirmed.email) end it 'can not be set to an unconfirmed email' do unconfirmed = create(:email, user: user) user.commit_email = unconfirmed.email - # This should set the commit_email attribute to the primary email - expect(user).to be_valid - expect(user.commit_email).to eq(user.email) + expect(user).not_to be_valid end it 'can not be set to a non-existent email' do user.commit_email = 'non-existent-email@nonexistent.nonexistent' - # This should set the commit_email attribute to the primary email - expect(user).to be_valid - expect(user.commit_email).to eq(user.email) + expect(user).not_to be_valid end it 'can not be set to an invalid email, even if confirmed' do @@ -692,70 +688,26 @@ RSpec.describe User do end end - context 'owns_notification_email' do - it 'accepts temp_oauth_email emails' do - user = build(:user, email: "temp-email-for-oauth@example.com") - expect(user).to be_valid - end - - it 'does not accept not verified emails' do - email = create(:email) - user = email.user - user.notification_email = email.email + context 'when secondary email is same as primary' do + let(:user) { create(:user, email: 'user@example.com') } - expect(user).to be_invalid - expect(user.errors[:notification_email]).to include(_('must be an email you have verified')) - end - end + it 'lets user change primary email without failing validations' do + user.commit_email = user.email + user.notification_email = user.email + user.public_email = user.email + user.save! - context 'owns_public_email' do - it 'accepts verified emails' do - email = create(:email, :confirmed, email: 'test@test.com') - user = email.user - user.notification_email = email.email + user.email = 'newemail@example.com' + user.confirm expect(user).to be_valid end - - it 'does not accept not verified emails' do - email = create(:email) - user = email.user - user.public_email = email.email - - expect(user).to be_invalid - expect(user.errors[:public_email]).to include(_('must be an email you have verified')) - end end - context 'set_commit_email' do - it 'keeps commit email when private commit email is being used' do - user = create(:user, commit_email: Gitlab::PrivateCommitEmail::TOKEN) - - expect(user.read_attribute(:commit_email)).to eq(Gitlab::PrivateCommitEmail::TOKEN) - end - - it 'keeps the commit email when nil' do - user = create(:user, commit_email: nil) - - expect(user.read_attribute(:commit_email)).to be_nil - end - - it 'reverts to nil when email is not verified' do - user = create(:user, commit_email: "foo@bar.com") - - expect(user.read_attribute(:commit_email)).to be_nil - end - end - - context 'owns_commit_email' do - it 'accepts private commit email' do - user = build(:user, commit_email: Gitlab::PrivateCommitEmail::TOKEN) - - expect(user).to be_valid - end - - it 'accepts nil commit email' do - user = build(:user, commit_email: nil) + context 'when commit_email is changed to _private' do + it 'passes user validations' do + user = create(:user) + user.commit_email = '_private' expect(user).to be_valid end @@ -931,12 +883,8 @@ RSpec.describe User do end context 'maximum value' do - before do - allow(Devise.password_length).to receive(:max).and_return(201) - end - it 'is determined by the current value of `Devise.password_length.max`' do - expect(password_length.max).to eq(201) + expect(password_length.max).to eq(Devise.password_length.max) end end end @@ -1311,53 +1259,57 @@ RSpec.describe User do end end - describe '#update_notification_email' do - # Regression: https://gitlab.com/gitlab-org/gitlab-foss/issues/22846 - context 'when changing :email' do - let(:user) { create(:user) } - let(:new_email) { 'new-email@example.com' } + describe 'when changing email' do + let(:user) { create(:user) } + let(:new_email) { 'new-email@example.com' } + context 'if notification_email was nil' do it 'sets :unconfirmed_email' do expect do user.tap { |u| u.update!(email: new_email) }.reload end.to change(user, :unconfirmed_email).to(new_email) end - it 'does not change :notification_email' do + + it 'does not change notification_email or notification_email_or_default before email is confirmed' do expect do user.tap { |u| u.update!(email: new_email) }.reload - end.not_to change(user, :notification_email) + end.not_to change(user, :notification_email_or_default) + + expect(user.notification_email).to be_nil end - it 'updates :notification_email to the new email once confirmed' do + it 'updates notification_email_or_default to the new email once confirmed' do user.update!(email: new_email) expect do user.tap(&:confirm).reload - end.to change(user, :notification_email).to eq(new_email) + end.to change(user, :notification_email_or_default).to eq(new_email) + + expect(user.notification_email).to be_nil end + end - context 'and :notification_email is set to a secondary email' do - let!(:email_attrs) { attributes_for(:email, :confirmed, user: user) } - let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) } + context 'when notification_email is set to a secondary email' do + let!(:email_attrs) { attributes_for(:email, :confirmed, user: user) } + let(:secondary) { create(:email, :confirmed, email: 'secondary@example.com', user: user) } - before do - user.emails.create!(email_attrs) - user.tap { |u| u.update!(notification_email: email_attrs[:email]) }.reload - end + before do + user.emails.create!(email_attrs) + user.tap { |u| u.update!(notification_email: email_attrs[:email]) }.reload + end - it 'does not change :notification_email to :email' do - expect do - user.tap { |u| u.update!(email: new_email) }.reload - end.not_to change(user, :notification_email) - end + it 'does not change notification_email to email before email is confirmed' do + expect do + user.tap { |u| u.update!(email: new_email) }.reload + end.not_to change(user, :notification_email) + end - it 'does not change :notification_email to :email once confirmed' do - user.update!(email: new_email) + it 'does not change notification_email to email once confirmed' do + user.update!(email: new_email) - expect do - user.tap(&:confirm).reload - end.not_to change(user, :notification_email) - end + expect do + user.tap(&:confirm).reload + end.not_to change(user, :notification_email) end end end @@ -1833,14 +1785,26 @@ RSpec.describe User do end describe '#manageable_groups' do - it 'includes all the namespaces the user can manage' do - expect(user.manageable_groups).to contain_exactly(group, subgroup) + shared_examples 'manageable groups examples' do + it 'includes all the namespaces the user can manage' do + expect(user.manageable_groups).to contain_exactly(group, subgroup) + end + + it 'does not include duplicates if a membership was added for the subgroup' do + subgroup.add_owner(user) + + expect(user.manageable_groups).to contain_exactly(group, subgroup) + end end - it 'does not include duplicates if a membership was added for the subgroup' do - subgroup.add_owner(user) + it_behaves_like 'manageable groups examples' + + context 'when feature flag :linear_user_manageable_groups is disabled' do + before do + stub_feature_flags(linear_user_manageable_groups: false) + end - expect(user.manageable_groups).to contain_exactly(group, subgroup) + it_behaves_like 'manageable groups examples' end end @@ -1925,12 +1889,25 @@ RSpec.describe User do expect(user.deactivated?).to be_truthy end - it 'sends deactivated user an email' do - expect_next_instance_of(NotificationService) do |notification| - allow(notification).to receive(:user_deactivated).with(user.name, user.notification_email) + context "when user deactivation emails are disabled" do + before do + stub_application_setting(user_deactivation_emails_enabled: false) end + it 'does not send deactivated user an email' do + expect(NotificationService).not_to receive(:new) - user.deactivate + user.deactivate + end + end + + context "when user deactivation emails are enabled" do + it 'sends deactivated user an email' do + expect_next_instance_of(NotificationService) do |notification| + allow(notification).to receive(:user_deactivated).with(user.name, user.notification_email_or_default) + end + + user.deactivate + end end end @@ -1997,15 +1974,15 @@ RSpec.describe User do user.ban! end - it 'activates the user' do - user.activate + it 'unbans the user' do + user.unban expect(user.banned?).to eq(false) expect(user.active?).to eq(true) end it 'deletes the BannedUser record' do - expect { user.activate }.to change { Users::BannedUser.count }.by(-1) + expect { user.unban }.to change { Users::BannedUser.count }.by(-1) expect(Users::BannedUser.where(user_id: user.id)).not_to exist end end @@ -3125,15 +3102,15 @@ RSpec.describe User do end end - describe '#notification_email' do + describe '#notification_email_or_default' do let(:email) { 'gonzo@muppets.com' } context 'when the column in the database is null' do subject { create(:user, email: email, notification_email: nil) } it 'defaults to the primary email' do - expect(subject.read_attribute(:notification_email)).to be nil - expect(subject.notification_email).to eq(email) + expect(subject.notification_email).to be nil + expect(subject.notification_email_or_default).to eq(email) end end end @@ -3467,17 +3444,32 @@ RSpec.describe User do end describe '#membership_groups' do - let!(:user) { create(:user) } - let!(:parent_group) { create(:group) } - let!(:child_group) { create(:group, parent: parent_group) } + let_it_be(:user) { create(:user) } - before do - parent_group.add_user(user, Gitlab::Access::MAINTAINER) + let_it_be(:parent_group) do + create(:group).tap do |g| + g.add_user(user, Gitlab::Access::MAINTAINER) + end end + let_it_be(:child_group) { create(:group, parent: parent_group) } + let_it_be(:other_group) { create(:group) } + subject { user.membership_groups } - it { is_expected.to contain_exactly parent_group, child_group } + shared_examples 'returns groups where the user is a member' do + specify { is_expected.to contain_exactly(parent_group, child_group) } + end + + it_behaves_like 'returns groups where the user is a member' + + context 'when feature flag :linear_user_membership_groups is disabled' do + before do + stub_feature_flags(linear_user_membership_groups: false) + end + + it_behaves_like 'returns groups where the user is a member' + end end describe '#authorizations_for_projects' do @@ -3686,6 +3678,11 @@ RSpec.describe User do it 'loads all the runners in the tree of groups' do expect(user.ci_owned_runners).to contain_exactly(runner, group_runner) end + + it 'returns true for owns_runner?' do + expect(user.owns_runner?(runner)).to eq(true) + expect(user.owns_runner?(group_runner)).to eq(true) + end end end @@ -3698,6 +3695,10 @@ RSpec.describe User do it 'loads the runners in the group' do expect(user.ci_owned_runners).to contain_exactly(group_runner) end + + it 'returns true for owns_runner?' do + expect(user.owns_runner?(group_runner)).to eq(true) + end end end @@ -3706,6 +3707,10 @@ RSpec.describe User do it 'loads the runner belonging to the project' do expect(user.ci_owned_runners).to contain_exactly(runner) end + + it 'returns true for owns_runner?' do + expect(user.owns_runner?(runner)).to eq(true) + end end end @@ -3718,6 +3723,10 @@ RSpec.describe User do it 'loads the runners of the project' do expect(user.ci_owned_runners).to contain_exactly(project_runner) end + + it 'returns true for owns_runner?' do + expect(user.owns_runner?(project_runner)).to eq(true) + end end context 'when the user is a developer' do @@ -3728,6 +3737,10 @@ RSpec.describe User do it 'does not load any runner' do expect(user.ci_owned_runners).to be_empty end + + it 'returns false for owns_runner?' do + expect(user.owns_runner?(project_runner)).to eq(false) + end end context 'when the user is a reporter' do @@ -3738,6 +3751,10 @@ RSpec.describe User do it 'does not load any runner' do expect(user.ci_owned_runners).to be_empty end + + it 'returns false for owns_runner?' do + expect(user.owns_runner?(project_runner)).to eq(false) + end end context 'when the user is a guest' do @@ -3748,6 +3765,10 @@ RSpec.describe User do it 'does not load any runner' do expect(user.ci_owned_runners).to be_empty end + + it 'returns false for owns_runner?' do + expect(user.owns_runner?(project_runner)).to eq(false) + end end end @@ -3760,6 +3781,10 @@ RSpec.describe User do it 'does not load the runners of the group' do expect(user.ci_owned_runners).to be_empty end + + it 'returns false for owns_runner?' do + expect(user.owns_runner?(runner)).to eq(false) + end end context 'when the user is a developer' do @@ -3770,6 +3795,10 @@ RSpec.describe User do it 'does not load any runner' do expect(user.ci_owned_runners).to be_empty end + + it 'returns false for owns_runner?' do + expect(user.owns_runner?(runner)).to eq(false) + end end context 'when the user is a reporter' do @@ -3780,6 +3809,10 @@ RSpec.describe User do it 'does not load any runner' do expect(user.ci_owned_runners).to be_empty end + + it 'returns false for owns_runner?' do + expect(user.owns_runner?(runner)).to eq(false) + end end context 'when the user is a guest' do @@ -3790,6 +3823,10 @@ RSpec.describe User do it 'does not load any runner' do expect(user.ci_owned_runners).to be_empty end + + it 'returns false for owns_runner?' do + expect(user.owns_runner?(runner)).to eq(false) + end end end @@ -3797,6 +3834,10 @@ RSpec.describe User do it 'does not load any runner' do expect(user.ci_owned_runners).to be_empty end + + it 'returns false for owns_runner?' do + expect(user.owns_runner?(create(:ci_runner))).to eq(false) + end end context 'with runner in a personal project' do @@ -5312,7 +5353,7 @@ RSpec.describe User do let(:group) { nil } it 'returns global notification email' do - is_expected.to eq(user.notification_email) + is_expected.to eq(user.notification_email_or_default) end end @@ -5320,7 +5361,7 @@ RSpec.describe User do it 'returns global notification email' do create(:notification_setting, user: user, source: group, notification_email: '') - is_expected.to eq(user.notification_email) + is_expected.to eq(user.notification_email_or_default) end end @@ -5521,22 +5562,17 @@ RSpec.describe User do end describe '#dismissed_callout?' do - subject(:user) { create(:user) } - - let(:feature_name) { UserCallout.feature_names.each_key.first } + let_it_be(:user, refind: true) { create(:user) } + let_it_be(:feature_name) { UserCallout.feature_names.each_key.first } context 'when no callout dismissal record exists' do it 'returns false when no ignore_dismissal_earlier_than provided' do expect(user.dismissed_callout?(feature_name: feature_name)).to eq false end - - it 'returns false when ignore_dismissal_earlier_than provided' do - expect(user.dismissed_callout?(feature_name: feature_name, ignore_dismissal_earlier_than: 3.months.ago)).to eq false - end end context 'when dismissed callout exists' do - before do + before_all do create(:user_callout, user: user, feature_name: feature_name, dismissed_at: 4.months.ago) end @@ -5554,6 +5590,123 @@ RSpec.describe User do end end + describe '#find_or_initialize_callout' do + let_it_be(:user, refind: true) { create(:user) } + let_it_be(:feature_name) { UserCallout.feature_names.each_key.first } + + subject(:find_or_initialize_callout) { user.find_or_initialize_callout(feature_name) } + + context 'when callout exists' do + let!(:callout) { create(:user_callout, user: user, feature_name: feature_name) } + + it 'returns existing callout' do + expect(find_or_initialize_callout).to eq(callout) + end + end + + context 'when callout does not exist' do + context 'when feature name is valid' do + it 'initializes a new callout' do + expect(find_or_initialize_callout).to be_a_new(UserCallout) + end + + it 'is valid' do + expect(find_or_initialize_callout).to be_valid + end + end + + context 'when feature name is not valid' do + let(:feature_name) { 'notvalid' } + + it 'initializes a new callout' do + expect(find_or_initialize_callout).to be_a_new(UserCallout) + end + + it 'is not valid' do + expect(find_or_initialize_callout).not_to be_valid + end + end + end + end + + describe '#dismissed_callout_for_group?' do + let_it_be(:user, refind: true) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:feature_name) { Users::GroupCallout.feature_names.each_key.first } + + context 'when no callout dismissal record exists' do + it 'returns false when no ignore_dismissal_earlier_than provided' do + expect(user.dismissed_callout_for_group?(feature_name: feature_name, group: group)).to eq false + end + end + + context 'when dismissed callout exists' do + before_all do + create(:group_callout, + user: user, + group_id: group.id, + feature_name: feature_name, + dismissed_at: 4.months.ago) + end + + it 'returns true when no ignore_dismissal_earlier_than provided' do + expect(user.dismissed_callout_for_group?(feature_name: feature_name, group: group)).to eq true + end + + it 'returns true when ignore_dismissal_earlier_than is earlier than dismissed_at' do + expect(user.dismissed_callout_for_group?(feature_name: feature_name, group: group, ignore_dismissal_earlier_than: 6.months.ago)).to eq true + end + + it 'returns false when ignore_dismissal_earlier_than is later than dismissed_at' do + expect(user.dismissed_callout_for_group?(feature_name: feature_name, group: group, ignore_dismissal_earlier_than: 3.months.ago)).to eq false + end + end + end + + describe '#find_or_initialize_group_callout' do + let_it_be(:user, refind: true) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:feature_name) { Users::GroupCallout.feature_names.each_key.first } + + subject(:callout_with_source) do + user.find_or_initialize_group_callout(feature_name, group.id) + end + + context 'when callout exists' do + let!(:callout) do + create(:group_callout, user: user, feature_name: feature_name, group_id: group.id) + end + + it 'returns existing callout' do + expect(callout_with_source).to eq(callout) + end + end + + context 'when callout does not exist' do + context 'when feature name is valid' do + it 'initializes a new callout' do + expect(callout_with_source).to be_a_new(Users::GroupCallout) + end + + it 'is valid' do + expect(callout_with_source).to be_valid + end + end + + context 'when feature name is not valid' do + let(:feature_name) { 'notvalid' } + + it 'initializes a new callout' do + expect(callout_with_source).to be_a_new(Users::GroupCallout) + end + + it 'is not valid' do + expect(callout_with_source).not_to be_valid + end + end + end + end + describe '#hook_attrs' do it 'includes id, name, username, avatar_url, and email' do user = create(:user) @@ -5916,45 +6069,6 @@ RSpec.describe User do end end - describe '#find_or_initialize_callout' do - subject(:find_or_initialize_callout) { user.find_or_initialize_callout(feature_name) } - - let(:user) { create(:user) } - let(:feature_name) { UserCallout.feature_names.each_key.first } - - context 'when callout exists' do - let!(:callout) { create(:user_callout, user: user, feature_name: feature_name) } - - it 'returns existing callout' do - expect(find_or_initialize_callout).to eq(callout) - end - end - - context 'when callout does not exist' do - context 'when feature name is valid' do - it 'initializes a new callout' do - expect(find_or_initialize_callout).to be_a_new(UserCallout) - end - - it 'is valid' do - expect(find_or_initialize_callout).to be_valid - end - end - - context 'when feature name is not valid' do - let(:feature_name) { 'notvalid' } - - it 'initializes a new callout' do - expect(find_or_initialize_callout).to be_a_new(UserCallout) - end - - it 'is not valid' do - expect(find_or_initialize_callout).not_to be_valid - end - end - end - end - describe '#default_dashboard?' do it 'is the default dashboard' do user = build(:user) @@ -6024,4 +6138,75 @@ RSpec.describe User do expect(described_class.by_provider_and_extern_uid(:github, 'my_github_id')).to match_array([expected_user]) end end + + describe '#unset_secondary_emails_matching_deleted_email!' do + let(:deleted_email) { 'kermit@muppets.com' } + + subject { build(:user, commit_email: commit_email) } + + context 'when no secondary email matches the deleted email' do + let(:commit_email) { 'fozzie@muppets.com' } + + it 'does nothing' do + expect(subject).not_to receive(:save) + subject.unset_secondary_emails_matching_deleted_email!(deleted_email) + expect(subject.commit_email).to eq commit_email + end + end + + context 'when a secondary email matches the deleted_email' do + let(:commit_email) { deleted_email } + + it 'un-sets the secondary email' do + expect(subject).to receive(:save) + subject.unset_secondary_emails_matching_deleted_email!(deleted_email) + expect(subject.commit_email).to be nil + end + end + end + + describe '#groups_with_developer_maintainer_project_access' do + let_it_be(:user) { create(:user) } + let_it_be(:group1) { create(:group) } + + let_it_be(:developer_group1) do + create(:group).tap do |g| + g.add_developer(user) + end + end + + let_it_be(:developer_group2) do + create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS).tap do |g| + g.add_developer(user) + end + end + + let_it_be(:guest_group1) do + create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS).tap do |g| + g.add_guest(user) + end + end + + let_it_be(:developer_group1) do + create(:group, project_creation_level: ::Gitlab::Access::DEVELOPER_MAINTAINER_PROJECT_ACCESS).tap do |g| + g.add_maintainer(user) + end + end + + subject { user.send(:groups_with_developer_maintainer_project_access) } + + shared_examples 'groups_with_developer_maintainer_project_access examples' do + specify { is_expected.to contain_exactly(developer_group2) } + end + + it_behaves_like 'groups_with_developer_maintainer_project_access examples' + + context 'when feature flag :linear_user_groups_with_developer_maintainer_project_access is disabled' do + before do + stub_feature_flags(linear_user_groups_with_developer_maintainer_project_access: false) + end + + it_behaves_like 'groups_with_developer_maintainer_project_access examples' + end + end end diff --git a/spec/models/users/group_callout_spec.rb b/spec/models/users/group_callout_spec.rb new file mode 100644 index 00000000000..461b5fd7715 --- /dev/null +++ b/spec/models/users/group_callout_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Users::GroupCallout do + let_it_be(:user) { create_default(:user) } + let_it_be(:group) { create_default(:group) } + let_it_be(:callout) { create(:group_callout) } + + it_behaves_like 'having unique enum values' + + describe 'relationships' do + it { is_expected.to belong_to(:group) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:group) } + it { is_expected.to validate_presence_of(:feature_name) } + it { is_expected.to validate_uniqueness_of(:feature_name).scoped_to(:user_id, :group_id).ignoring_case_sensitivity } + end + + describe '#source_feature_name' do + it 'provides string based off source and feature' do + expect(callout.source_feature_name).to eq "#{callout.feature_name}_#{callout.group_id}" + end + end +end diff --git a/spec/models/work_item/type_spec.rb b/spec/models/work_item/type_spec.rb index 90f551b7d63..dd5324d63a0 100644 --- a/spec/models/work_item/type_spec.rb +++ b/spec/models/work_item/type_spec.rb @@ -19,8 +19,10 @@ RSpec.describe WorkItem::Type do it 'deletes type but not unrelated issues' do type = create(:work_item_type) + expect(WorkItem::Type.count).to eq(5) + expect { type.destroy! }.not_to change(Issue, :count) - expect(WorkItem::Type.count).to eq 0 + expect(WorkItem::Type.count).to eq(4) end end @@ -28,7 +30,7 @@ RSpec.describe WorkItem::Type do type = create(:work_item_type, work_items: [work_item]) expect { type.destroy! }.to raise_error(ActiveRecord::InvalidForeignKey) - expect(Issue.count).to eq 1 + expect(Issue.count).to eq(1) end end |