diff options
Diffstat (limited to 'spec/models/concerns')
-rw-r--r-- | spec/models/concerns/after_commit_queue_spec.rb | 128 | ||||
-rw-r--r-- | spec/models/concerns/calloutable_spec.rb | 26 | ||||
-rw-r--r-- | spec/models/concerns/case_sensitivity_spec.rb | 5 | ||||
-rw-r--r-- | spec/models/concerns/group_descendant_spec.rb | 6 | ||||
-rw-r--r-- | spec/models/concerns/loose_foreign_key_spec.rb | 66 | ||||
-rw-r--r-- | spec/models/concerns/participable_spec.rb | 74 | ||||
-rw-r--r-- | spec/models/concerns/routable_spec.rb | 117 | ||||
-rw-r--r-- | spec/models/concerns/sha_attribute_spec.rb | 2 | ||||
-rw-r--r-- | spec/models/concerns/transactions_spec.rb | 21 |
9 files changed, 278 insertions, 167 deletions
diff --git a/spec/models/concerns/after_commit_queue_spec.rb b/spec/models/concerns/after_commit_queue_spec.rb new file mode 100644 index 00000000000..40cddde333e --- /dev/null +++ b/spec/models/concerns/after_commit_queue_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe AfterCommitQueue do + describe '#run_after_commit' do + it 'runs after record is saved' do + called = false + test_proc = proc { called = true } + + project = build(:project) + project.run_after_commit(&test_proc) + + expect(called).to be false + + # save! is run in its own transaction + project.save! + + expect(called).to be true + end + + it 'runs after transaction is committed' do + called = false + test_proc = proc { called = true } + + project = build(:project) + + Project.transaction do + project.run_after_commit(&test_proc) + + project.save! + + expect(called).to be false + end + + expect(called).to be true + end + end + + describe '#run_after_commit_or_now' do + it 'runs immediately if not within a transction' do + called = false + test_proc = proc { called = true } + + project = build(:project) + + project.run_after_commit_or_now(&test_proc) + + expect(called).to be true + end + + it 'runs after transaction has completed' do + called = false + test_proc = proc { called = true } + + project = build(:project) + + Project.transaction do + # Add this record to the current transaction so that after commit hooks + # are called + Project.connection.add_transaction_record(project) + + project.run_after_commit_or_now(&test_proc) + + project.save! + + expect(called).to be false + end + + expect(called).to be true + end + + context 'multiple databases - Ci::ApplicationRecord models' do + before do + skip_if_multiple_databases_not_setup + + table_sql = <<~SQL + CREATE TABLE _test_ci_after_commit_queue ( + id serial NOT NULL PRIMARY KEY); + SQL + + ::Ci::ApplicationRecord.connection.execute(table_sql) + end + + let(:ci_klass) do + Class.new(Ci::ApplicationRecord) do + self.table_name = '_test_ci_after_commit_queue' + + include AfterCommitQueue + + def self.name + 'TestCiAfterCommitQueue' + end + end + end + + let(:ci_record) { ci_klass.new } + + it 'runs immediately if not within a transaction' do + called = false + test_proc = proc { called = true } + + ci_record.run_after_commit_or_now(&test_proc) + + expect(called).to be true + end + + it 'runs after transaction has completed' do + called = false + test_proc = proc { called = true } + + Ci::ApplicationRecord.transaction do + # Add this record to the current transaction so that after commit hooks + # are called + Ci::ApplicationRecord.connection.add_transaction_record(ci_record) + + ci_record.run_after_commit_or_now(&test_proc) + + ci_record.save! + + expect(called).to be false + end + + expect(called).to be true + end + end + end +end diff --git a/spec/models/concerns/calloutable_spec.rb b/spec/models/concerns/calloutable_spec.rb deleted file mode 100644 index d847413de88..00000000000 --- a/spec/models/concerns/calloutable_spec.rb +++ /dev/null @@ -1,26 +0,0 @@ -# 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/case_sensitivity_spec.rb b/spec/models/concerns/case_sensitivity_spec.rb index 269f9577267..6e624c687c4 100644 --- a/spec/models/concerns/case_sensitivity_spec.rb +++ b/spec/models/concerns/case_sensitivity_spec.rb @@ -9,11 +9,12 @@ RSpec.describe CaseSensitivity do Class.new(ActiveRecord::Base) do include CaseSensitivity self.table_name = 'namespaces' + self.inheritance_column = :_type_disabled end end - let_it_be(:model_1) { model.create!(path: 'mOdEl-1', name: 'mOdEl 1') } - let_it_be(:model_2) { model.create!(path: 'mOdEl-2', name: 'mOdEl 2') } + let_it_be(:model_1) { model.create!(path: 'mOdEl-1', name: 'mOdEl 1', type: Namespaces::UserNamespace.sti_name) } + let_it_be(:model_2) { model.create!(path: 'mOdEl-2', name: 'mOdEl 2', type: Group.sti_name) } it 'finds a single instance by a single attribute regardless of case' do expect(model.iwhere(path: 'MODEL-1')).to contain_exactly(model_1) diff --git a/spec/models/concerns/group_descendant_spec.rb b/spec/models/concerns/group_descendant_spec.rb index b29fa910ee6..d593d829dca 100644 --- a/spec/models/concerns/group_descendant_spec.rb +++ b/spec/models/concerns/group_descendant_spec.rb @@ -19,14 +19,16 @@ RSpec.describe GroupDescendant do query_count = ActiveRecord::QueryRecorder.new { test_group.hierarchy }.count - expect(query_count).to eq(1) + # use_traversal_ids_for_ancestors_upto actor based feature flag check adds an extra query. + expect(query_count).to eq(2) end it 'only queries once for the ancestors when a top is given' do test_group = create(:group, parent: subsub_group).reload recorder = ActiveRecord::QueryRecorder.new { test_group.hierarchy(subgroup) } - expect(recorder.count).to eq(1) + # use_traversal_ids_for_ancestors_upto actor based feature flag check adds an extra query. + expect(recorder.count).to eq(2) end it 'builds a hierarchy for a group' do diff --git a/spec/models/concerns/loose_foreign_key_spec.rb b/spec/models/concerns/loose_foreign_key_spec.rb deleted file mode 100644 index 42da69eb75e..00000000000 --- a/spec/models/concerns/loose_foreign_key_spec.rb +++ /dev/null @@ -1,66 +0,0 @@ -# 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 - loose_foreign_key 'merge_requests', 'project_id', 'on_delete' => 'async_nullify' - 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) - 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 - loose_foreign_key :merge_requests, :project_id, on_delete: :async_nullify - loose_foreign_key :merge_requests, :project_id, on_delete: :destroy - 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 'inheritance validation' do - let(:inherited_project_class) do - Class.new(Project) do - include LooseForeignKey - - loose_foreign_key :issues, :project_id, on_delete: :async_delete - 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/participable_spec.rb b/spec/models/concerns/participable_spec.rb index 903c7ae16b6..50cf7377b99 100644 --- a/spec/models/concerns/participable_spec.rb +++ b/spec/models/concerns/participable_spec.rb @@ -51,7 +51,9 @@ RSpec.describe Participable do end it 'supports attributes returning another Participable' do - other_model = Class.new { include Participable } + other_model = Class.new do + include Participable + end other_model.participant(:bar) model.participant(:foo) @@ -115,6 +117,76 @@ RSpec.describe Participable do end end + describe '#visible_participants' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?).with(anything, :read_class, anything) { readable } + end + + let(:readable) { true } + + it 'returns the list of participants' do + model.participant(:foo) + model.participant(:bar) + + user1 = build(:user) + user2 = build(:user) + user3 = build(:user) + project = build(:project, :public) + instance = model.new + + allow(instance).to receive_message_chain(:model_name, :element) { 'class' } + expect(instance).to receive(:foo).and_return(user2) + expect(instance).to receive(:bar).and_return(user3) + expect(instance).to receive(:project).thrice.and_return(project) + + participants = instance.visible_participants(user1) + + expect(participants).to include(user2) + expect(participants).to include(user3) + end + + context 'when Participable is not readable by the user' do + let(:readable) { false } + + it 'does not return unavailable participants' do + model.participant(:bar) + + instance = model.new + user1 = build(:user) + user2 = build(:user) + project = build(:project, :public) + + allow(instance).to receive_message_chain(:model_name, :element) { 'class' } + allow(instance).to receive(:bar).and_return(user2) + expect(instance).to receive(:project).thrice.and_return(project) + + expect(instance.visible_participants(user1)).to be_empty + end + + context 'when feature flag is disabled' do + before do + stub_feature_flags(verify_participants_access: false) + end + + it 'returns unavailable participants' do + model.participant(:bar) + + instance = model.new + user1 = build(:user) + user2 = build(:user) + project = build(:project, :public) + + allow(instance).to receive_message_chain(:model_name, :element) { 'class' } + allow(instance).to receive(:bar).and_return(user2) + expect(instance).to receive(:project).thrice.and_return(project) + + expect(instance.visible_participants(user1)).to match_array([user2]) + end + end + end + end + describe '#participant?' do let(:instance) { model.new } diff --git a/spec/models/concerns/routable_spec.rb b/spec/models/concerns/routable_spec.rb index 0a433a8cf4f..2330147b376 100644 --- a/spec/models/concerns/routable_spec.rb +++ b/spec/models/concerns/routable_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.shared_examples '.find_by_full_path' do +RSpec.shared_examples 'routable resource' do describe '.find_by_full_path', :aggregate_failures do it 'finds records by their full path' do expect(described_class.find_by_full_path(record.full_path)).to eq(record) @@ -52,13 +52,27 @@ RSpec.shared_examples '.find_by_full_path' do end end -RSpec.describe Routable do - it_behaves_like '.find_by_full_path' do - let_it_be(:record) { create(:group) } +RSpec.shared_examples 'routable resource with parent' do + it_behaves_like 'routable resource' + + describe '#full_path' do + it { expect(record.full_path).to eq "#{record.parent.full_path}/#{record.path}" } + + it 'hits the cache when not preloaded' do + forcibly_hit_cached_lookup(record, :full_path) + + expect(record.full_path).to eq("#{record.parent.full_path}/#{record.path}") + end end - it_behaves_like '.find_by_full_path' do - let_it_be(:record) { create(:project) } + describe '#full_name' do + it { expect(record.full_name).to eq "#{record.parent.human_name} / #{record.name}" } + + it 'hits the cache when not preloaded' do + forcibly_hit_cached_lookup(record, :full_name) + + expect(record.full_name).to eq("#{record.parent.human_name} / #{record.name}") + end end end @@ -66,6 +80,14 @@ RSpec.describe Group, 'Routable', :with_clean_rails_cache do let_it_be_with_reload(:group) { create(:group, name: 'foo') } let_it_be(:nested_group) { create(:group, parent: group) } + it_behaves_like 'routable resource' do + let_it_be(:record) { group } + end + + it_behaves_like 'routable resource with parent' do + let_it_be(:record) { nested_group } + end + describe 'Validations' do it { is_expected.to validate_presence_of(:route) } end @@ -119,24 +141,6 @@ RSpec.describe Group, 'Routable', :with_clean_rails_cache do end end - describe '.find_by_full_path' do - it_behaves_like '.find_by_full_path' do - let_it_be(:record) { group } - end - - it_behaves_like '.find_by_full_path' do - let_it_be(:record) { nested_group } - end - - it 'does not find projects with a matching path' do - project = create(:project) - redirect_route = create(:redirect_route, source: project) - - expect(described_class.find_by_full_path(project.full_path)).to be_nil - expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to be_nil - end - end - describe '.where_full_path_in' do context 'without any paths' do it 'returns an empty relation' do @@ -195,64 +199,39 @@ RSpec.describe Group, 'Routable', :with_clean_rails_cache do expect(group.route_loaded?).to be_truthy end end - - describe '#full_path' do - it { expect(group.full_path).to eq(group.path) } - it { expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") } - - it 'hits the cache when not preloaded' do - forcibly_hit_cached_lookup(nested_group, :full_path) - - expect(nested_group.full_path).to eq("#{group.full_path}/#{nested_group.path}") - end - end - - describe '#full_name' do - it { expect(group.full_name).to eq(group.name) } - it { expect(nested_group.full_name).to eq("#{group.name} / #{nested_group.name}") } - - it 'hits the cache when not preloaded' do - forcibly_hit_cached_lookup(nested_group, :full_name) - - expect(nested_group.full_name).to eq("#{group.name} / #{nested_group.name}") - end - end end RSpec.describe Project, 'Routable', :with_clean_rails_cache do let_it_be(:namespace) { create(:namespace) } let_it_be(:project) { create(:project, namespace: namespace) } - it_behaves_like '.find_by_full_path' do + it_behaves_like 'routable resource with parent' do let_it_be(:record) { project } end +end - it 'does not find groups with a matching path' do - group = create(:group) - redirect_route = create(:redirect_route, source: group) - - expect(described_class.find_by_full_path(group.full_path)).to be_nil - expect(described_class.find_by_full_path(redirect_route.path, follow_redirects: true)).to be_nil - end - - describe '#full_path' do - it { expect(project.full_path).to eq "#{namespace.full_path}/#{project.path}" } - - it 'hits the cache when not preloaded' do - forcibly_hit_cached_lookup(project, :full_path) - - expect(project.full_path).to eq("#{namespace.full_path}/#{project.path}") +RSpec.describe Namespaces::ProjectNamespace, 'Routable', :with_clean_rails_cache do + let_it_be(:group) { create(:group) } + let_it_be(:project_namespace) do + # For now we create only project namespace w/o project, otherwise same path + # would be used for project and project namespace. + # This can be removed when route is created automatically for project namespaces. + # https://gitlab.com/gitlab-org/gitlab/-/issues/346448 + create(:project_namespace, project: nil, parent: group, + visibility_level: Gitlab::VisibilityLevel::PUBLIC, + path: 'foo', name: 'foo').tap do |project_namespace| + Route.create!(source: project_namespace, path: project_namespace.full_path, + name: project_namespace.full_name) end end - describe '#full_name' do - it { expect(project.full_name).to eq "#{namespace.human_name} / #{project.name}" } - - it 'hits the cache when not preloaded' do - forcibly_hit_cached_lookup(project, :full_name) + # we have couple of places where we use generic Namespace, in that case + # we don't want to include ProjectNamespace routes yet + it 'ignores project namespace when searching for generic namespace' do + redirect_route = create(:redirect_route, source: project_namespace) - expect(project.full_name).to eq("#{namespace.human_name} / #{project.name}") - end + expect(Namespace.find_by_full_path(project_namespace.full_path)).to be_nil + expect(Namespace.find_by_full_path(redirect_route.path, follow_redirects: true)).to be_nil end end diff --git a/spec/models/concerns/sha_attribute_spec.rb b/spec/models/concerns/sha_attribute_spec.rb index 220eadfab92..1bcf3dc8b61 100644 --- a/spec/models/concerns/sha_attribute_spec.rb +++ b/spec/models/concerns/sha_attribute_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe ShaAttribute do - let(:model) { Class.new(ApplicationRecord) { include ShaAttribute } } + let(:model) { Class.new(ActiveRecord::Base) { include ShaAttribute } } before do columns = [ diff --git a/spec/models/concerns/transactions_spec.rb b/spec/models/concerns/transactions_spec.rb new file mode 100644 index 00000000000..404a33196e6 --- /dev/null +++ b/spec/models/concerns/transactions_spec.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Transactions do + let(:model) { build(:project) } + + it 'is not in a transaction' do + expect(model.class).not_to be_inside_transaction + end + + it 'is in a transaction', :aggregate_failures do + Project.transaction do + expect(model.class).to be_inside_transaction + end + + ApplicationRecord.transaction do + expect(model.class).to be_inside_transaction + end + end +end |