From 9f46488805e86b1bc341ea1620b866016c2ce5ed Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 20 May 2020 14:34:42 +0000 Subject: Add latest changes from gitlab-org/gitlab@13-0-stable-ee --- spec/models/concerns/awardable_spec.rb | 41 +++++++++ .../concerns/blocks_json_serialization_spec.rb | 7 +- spec/models/concerns/cache_markdown_field_spec.rb | 52 +++++++++++ spec/models/concerns/cacheable_attributes_spec.rb | 4 +- spec/models/concerns/has_user_type_spec.rb | 86 ++++++++++++++++++ spec/models/concerns/mentionable_spec.rb | 52 +++++++++-- spec/models/concerns/noteable_spec.rb | 2 +- spec/models/concerns/reactive_caching_spec.rb | 100 ++++++++++++++------- spec/models/concerns/redis_cacheable_spec.rb | 6 +- spec/models/concerns/spammable_spec.rb | 91 +++++++++++++++---- 10 files changed, 377 insertions(+), 64 deletions(-) create mode 100644 spec/models/concerns/has_user_type_spec.rb (limited to 'spec/models/concerns') diff --git a/spec/models/concerns/awardable_spec.rb b/spec/models/concerns/awardable_spec.rb index 76da42cf243..29f911fcb04 100644 --- a/spec/models/concerns/awardable_spec.rb +++ b/spec/models/concerns/awardable_spec.rb @@ -91,4 +91,45 @@ describe Awardable do expect(issue.award_emoji).to eq issue.award_emoji.sort_by(&:id) end end + + describe "#grouped_awards" do + context 'default award emojis' do + let(:issue_without_downvote) { create(:issue) } + let(:issue_with_downvote) do + issue_with_downvote = create(:issue) + create(:award_emoji, :downvote, awardable: issue_with_downvote) + issue_with_downvote + end + + it "includes unused thumbs buttons by default" do + expect(issue_without_downvote.grouped_awards.keys.sort).to eq %w(thumbsdown thumbsup) + end + + it "doesn't include unused thumbs buttons when disabled in project" do + issue_without_downvote.project.show_default_award_emojis = false + + expect(issue_without_downvote.grouped_awards.keys.sort).to eq [] + end + + it "includes unused thumbs buttons when enabled in project" do + issue_without_downvote.project.show_default_award_emojis = true + + expect(issue_without_downvote.grouped_awards.keys.sort).to eq %w(thumbsdown thumbsup) + end + + it "doesn't include unused thumbs buttons in summary" do + expect(issue_without_downvote.grouped_awards(with_thumbs: false).keys).to eq [] + end + + it "includes used thumbs buttons when disabled in project" do + issue_with_downvote.project.show_default_award_emojis = false + + expect(issue_with_downvote.grouped_awards.keys).to eq %w(thumbsdown) + end + + it "includes used thumbs buttons in summary" do + expect(issue_with_downvote.grouped_awards(with_thumbs: false).keys).to eq %w(thumbsdown) + end + end + end end diff --git a/spec/models/concerns/blocks_json_serialization_spec.rb b/spec/models/concerns/blocks_json_serialization_spec.rb index 0ef5be3cb61..32870461019 100644 --- a/spec/models/concerns/blocks_json_serialization_spec.rb +++ b/spec/models/concerns/blocks_json_serialization_spec.rb @@ -3,8 +3,11 @@ require 'spec_helper' describe BlocksJsonSerialization do - DummyModel = Class.new do - include BlocksJsonSerialization + before do + stub_const('DummyModel', Class.new) + DummyModel.class_eval do + include BlocksJsonSerialization + end end it 'blocks as_json' do diff --git a/spec/models/concerns/cache_markdown_field_spec.rb b/spec/models/concerns/cache_markdown_field_spec.rb index 697a9e98505..193144fcb0e 100644 --- a/spec/models/concerns/cache_markdown_field_spec.rb +++ b/spec/models/concerns/cache_markdown_field_spec.rb @@ -223,6 +223,10 @@ describe CacheMarkdownField, :clean_gitlab_redis_cache do end context 'when the markdown cache is up to date' do + before do + thing.try(:save) + end + it 'does not call #refresh_markdown_cache' do expect(thing).not_to receive(:refresh_markdown_cache) @@ -256,6 +260,54 @@ describe CacheMarkdownField, :clean_gitlab_redis_cache do let(:klass) { ar_class } it_behaves_like 'a class with cached markdown fields' + + describe '#attribute_invalidated?' do + let(:thing) { klass.create(description: markdown, description_html: html, cached_markdown_version: cache_version) } + + it 'returns true when cached_markdown_version is different' do + thing.cached_markdown_version += 1 + + expect(thing.attribute_invalidated?(:description_html)).to eq(true) + end + + it 'returns true when markdown is changed' do + thing.description = updated_markdown + + expect(thing.attribute_invalidated?(:description_html)).to eq(true) + end + + it 'returns true when both markdown and HTML are changed' do + thing.description = updated_markdown + thing.description_html = updated_html + + expect(thing.attribute_invalidated?(:description_html)).to eq(true) + end + + it 'returns false when there are no changes' do + expect(thing.attribute_invalidated?(:description_html)).to eq(false) + end + end + + context 'when cache version is updated' do + let(:old_version) { cache_version - 1 } + let(:old_html) { '

Foo

' } + + let(:thing) do + # This forces the record to have outdated HTML. We can't use `create` because the `before_create` hook + # would re-render the HTML to the latest version + klass.create.tap do |thing| + thing.update_columns(description: markdown, description_html: old_html, cached_markdown_version: old_version) + end + end + + it 'correctly updates cached HTML even if refresh_markdown_cache is called before updating the attribute' do + thing.refresh_markdown_cache + + thing.update(description: updated_markdown) + + expect(thing.description_html).to eq(updated_html) + end + end end context 'for other classes' do diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb index d8f940a808e..56e0d044247 100644 --- a/spec/models/concerns/cacheable_attributes_spec.rb +++ b/spec/models/concerns/cacheable_attributes_spec.rb @@ -205,11 +205,11 @@ describe CacheableAttributes do end end - it 'uses RequestStore in addition to Thread memory cache', :request_store do + it 'uses RequestStore in addition to process memory cache', :request_store do # Warm up the cache create(:application_setting).cache! - expect(ApplicationSetting.cache_backend).to eq(Gitlab::ThreadMemoryCache.cache_backend) + expect(ApplicationSetting.cache_backend).to eq(Gitlab::ProcessMemoryCache.cache_backend) expect(ApplicationSetting.cache_backend).to receive(:read).with(ApplicationSetting.cache_key).once.and_call_original 2.times { ApplicationSetting.current } diff --git a/spec/models/concerns/has_user_type_spec.rb b/spec/models/concerns/has_user_type_spec.rb new file mode 100644 index 00000000000..f12eee414f9 --- /dev/null +++ b/spec/models/concerns/has_user_type_spec.rb @@ -0,0 +1,86 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe User do + specify 'types consistency checks', :aggregate_failures do + expect(described_class::USER_TYPES.keys) + .to match_array(%w[human ghost alert_bot project_bot support_bot service_user visual_review_bot migration_bot]) + expect(described_class::USER_TYPES).to include(*described_class::BOT_USER_TYPES) + expect(described_class::USER_TYPES).to include(*described_class::NON_INTERNAL_USER_TYPES) + expect(described_class::USER_TYPES).to include(*described_class::INTERNAL_USER_TYPES) + end + + describe 'scopes & predicates' do + User::USER_TYPES.keys.each do |type| + let_it_be(type) { create(:user, username: type, user_type: type) } + end + let(:bots) { User::BOT_USER_TYPES.map { |type| public_send(type) } } + let(:non_internal) { User::NON_INTERNAL_USER_TYPES.map { |type| public_send(type) } } + let(:everyone) { User::USER_TYPES.keys.map { |type| public_send(type) } } + + describe '.humans' do + it 'includes humans only' do + expect(described_class.humans).to match_array([human]) + end + end + + describe '.bots' do + it 'includes all bots' do + expect(described_class.bots).to match_array(bots) + end + end + + describe '.bots_without_project_bot' do + it 'includes all bots except project_bot' do + expect(described_class.bots_without_project_bot).to match_array(bots - [project_bot]) + end + end + + describe '.non_internal' do + it 'includes all non_internal users' do + expect(described_class.non_internal).to match_array(non_internal) + end + end + + describe '.without_ghosts' do + it 'includes everyone except ghosts' do + expect(described_class.without_ghosts).to match_array(everyone - [ghost]) + end + end + + describe '.without_project_bot' do + it 'includes everyone except project_bot' do + expect(described_class.without_project_bot).to match_array(everyone - [project_bot]) + end + end + + describe '#bot?' do + it 'is true for all bot user types and false for others' do + expect(bots).to all(be_bot) + + (everyone - bots).each do |user| + expect(user).not_to be_bot + end + end + end + + describe '#human?' do + it 'is true for humans only' do + expect(human).to be_human + expect(alert_bot).not_to be_human + expect(User.new).to be_human + end + end + + describe '#internal?' do + it 'is true for all internal user types and false for others' do + expect(everyone - non_internal).to all(be_internal) + + non_internal.each do |user| + expect(user).not_to be_internal + end + end + end + end +end diff --git a/spec/models/concerns/mentionable_spec.rb b/spec/models/concerns/mentionable_spec.rb index 13a3d1cdd82..03fd1c69654 100644 --- a/spec/models/concerns/mentionable_spec.rb +++ b/spec/models/concerns/mentionable_spec.rb @@ -3,14 +3,17 @@ require 'spec_helper' describe Mentionable do - class Example - include Mentionable + before do + stub_const('Example', Class.new) + Example.class_eval do + include Mentionable - attr_accessor :project, :message - attr_mentionable :message + attr_accessor :project, :message + attr_mentionable :message - def author - nil + def author + nil + end end end @@ -28,11 +31,11 @@ describe Mentionable do end describe '#any_mentionable_attributes_changed?' do - Message = Struct.new(:text) + message = Struct.new(:text) let(:mentionable) { Example.new } let(:changes) do - msg = Message.new('test') + msg = message.new('test') changes = {} changes[msg] = ['', 'some message'] @@ -325,3 +328,36 @@ describe Snippet, 'Mentionable' do end end end + +describe PersonalSnippet, 'Mentionable' do + describe '#store_mentions!' do + it_behaves_like 'mentions in description', :personal_snippet + it_behaves_like 'mentions in notes', :personal_snippet do + let(:note) { create(:note_on_personal_snippet) } + let(:mentionable) { note.noteable } + end + end + + describe 'load mentions' do + it_behaves_like 'load mentions from DB', :personal_snippet do + let(:note) { create(:note_on_personal_snippet) } + let(:mentionable) { note.noteable } + end + end +end + +describe DesignManagement::Design do + describe '#store_mentions!' do + it_behaves_like 'mentions in notes', :design do + let(:note) { create(:diff_note_on_design) } + let(:mentionable) { note.noteable } + end + end + + describe 'load mentions' do + it_behaves_like 'load mentions from DB', :design do + let(:note) { create(:diff_note_on_design) } + let(:mentionable) { note.noteable } + end + end +end diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index 097bc24d90f..5c8c5425ca7 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -241,7 +241,7 @@ describe Noteable do describe '.resolvable_types' do it 'exposes the replyable types' do - expect(described_class.resolvable_types).to include('MergeRequest') + expect(described_class.resolvable_types).to include('MergeRequest', 'DesignManagement::Design') end end diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 96a9c317fb8..cfca383e0b0 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -6,39 +6,47 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do include ExclusiveLeaseHelpers include ReactiveCachingHelpers - class CacheTest - include ReactiveCaching + let(:cache_class_test) do + Class.new do + include ReactiveCaching - self.reactive_cache_key = ->(thing) { ["foo", thing.id] } + self.reactive_cache_key = ->(thing) { ["foo", thing.id] } - self.reactive_cache_lifetime = 5.minutes - self.reactive_cache_refresh_interval = 15.seconds + self.reactive_cache_lifetime = 5.minutes + self.reactive_cache_refresh_interval = 15.seconds - attr_reader :id + attr_reader :id - def self.primary_key - :id - end + def self.primary_key + :id + end - def initialize(id, &blk) - @id = id - @calculator = blk - end + def initialize(id, &blk) + @id = id + @calculator = blk + end - def calculate_reactive_cache - @calculator.call - end + def calculate_reactive_cache + @calculator.call + end - def result - with_reactive_cache do |data| - data + def result + with_reactive_cache do |data| + data + end end end end + let(:external_dependency_cache_class_test) do + Class.new(cache_class_test) do + self.reactive_cache_work_type = :external_dependency + end + end + let(:calculation) { -> { 2 + 2 } } let(:cache_key) { "foo:666" } - let(:instance) { CacheTest.new(666, &calculation) } + let(:instance) { cache_class_test.new(666, &calculation) } describe '#with_reactive_cache' do before do @@ -47,6 +55,18 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do subject(:go!) { instance.result } + shared_examples 'reactive worker call' do |worker_class| + let(:instance) do + test_class.new(666, &calculation) + end + + it 'performs caching with correct worker' do + expect(worker_class).to receive(:perform_async).with(test_class, 666) + + go! + end + end + shared_examples 'a cacheable value' do |cached_value| before do stub_reactive_cache(instance, cached_value) @@ -73,10 +93,12 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do it { is_expected.to be_nil } - it 'refreshes cache' do - expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666) + it_behaves_like 'reactive worker call', ReactiveCachingWorker do + let(:test_class) { cache_class_test } + end - instance.with_reactive_cache { raise described_class::InvalidateReactiveCache } + it_behaves_like 'reactive worker call', ExternalServiceReactiveCachingWorker do + let(:test_class) { external_dependency_cache_class_test } end end end @@ -84,10 +106,12 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do context 'when cache is empty' do it { is_expected.to be_nil } - it 'enqueues a background worker to bootstrap the cache' do - expect(ReactiveCachingWorker).to receive(:perform_async).with(CacheTest, 666) + it_behaves_like 'reactive worker call', ReactiveCachingWorker do + let(:test_class) { cache_class_test } + end - go! + it_behaves_like 'reactive worker call', ExternalServiceReactiveCachingWorker do + let(:test_class) { external_dependency_cache_class_test } end it 'updates the cache lifespan' do @@ -168,12 +192,14 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do context 'with custom reactive_cache_worker_finder' do let(:args) { %w(arg1 arg2) } - let(:instance) { CustomFinderCacheTest.new(666, &calculation) } + let(:instance) { custom_finder_cache_test.new(666, &calculation) } - class CustomFinderCacheTest < CacheTest - self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } + let(:custom_finder_cache_test) do + Class.new(cache_class_test) do + self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } - def self.from_cache(*args); end + def self.from_cache(*args); end + end end before do @@ -234,6 +260,18 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do go! end + context 'when :external_dependency cache' do + let(:instance) do + external_dependency_cache_class_test.new(666, &calculation) + end + + it 'enqueues a repeat worker' do + expect_reactive_cache_update_queued(instance, worker_klass: ExternalServiceReactiveCachingWorker) + + go! + end + end + it 'calls a reactive_cache_updated only once if content did not change on subsequent update' do expect(instance).to receive(:calculate_reactive_cache).twice expect(instance).to receive(:reactive_cache_updated).once @@ -262,7 +300,7 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do it_behaves_like 'ExceededReactiveCacheLimit' context 'when reactive_cache_hard_limit is overridden' do - let(:test_class) { Class.new(CacheTest) { self.reactive_cache_hard_limit = 3.megabytes } } + let(:test_class) { Class.new(cache_class_test) { self.reactive_cache_hard_limit = 3.megabytes } } let(:instance) { test_class.new(666, &calculation) } it_behaves_like 'successful cache' diff --git a/spec/models/concerns/redis_cacheable_spec.rb b/spec/models/concerns/redis_cacheable_spec.rb index f88d64e2013..1cf6afcc167 100644 --- a/spec/models/concerns/redis_cacheable_spec.rb +++ b/spec/models/concerns/redis_cacheable_spec.rb @@ -31,7 +31,7 @@ describe RedisCacheable do subject { instance.cached_attribute(payload.each_key.first) } it 'gets the cache attribute' do - Gitlab::Redis::SharedState.with do |redis| + Gitlab::Redis::Cache.with do |redis| expect(redis).to receive(:get).with(cache_key) .and_return(payload.to_json) end @@ -44,7 +44,7 @@ describe RedisCacheable do subject { instance.cache_attributes(payload) } it 'sets the cache attributes' do - Gitlab::Redis::SharedState.with do |redis| + Gitlab::Redis::Cache.with do |redis| expect(redis).to receive(:set).with(cache_key, payload.to_json, anything) end @@ -52,7 +52,7 @@ describe RedisCacheable do end end - describe '#cached_attr_reader', :clean_gitlab_redis_shared_state do + describe '#cached_attr_reader', :clean_gitlab_redis_cache do subject { instance.name } before do diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index b8537dd39f6..a8d27e174b7 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -39,43 +39,100 @@ describe Spammable do describe '#invalidate_if_spam' do using RSpec::Parameterized::TableSyntax + before do + stub_application_setting(recaptcha_enabled: true) + end + context 'when the model is spam' do - where(:recaptcha_enabled, :error) do - true | /solve the reCAPTCHA to proceed/ - false | /has been discarded/ + subject { invalidate_if_spam(is_spam: true) } + + it 'has an error related to spam on the model' do + expect(subject.errors.messages[:base]).to match_array /has been discarded/ end + end - with_them do - subject { invalidate_if_spam(true, recaptcha_enabled) } + context 'when the model needs recaptcha' do + subject { invalidate_if_spam(needs_recaptcha: true) } - it 'has an error related to spam on the model' do - expect(subject.errors.messages[:base]).to match_array error - end + it 'has an error related to spam on the model' do + expect(subject.errors.messages[:base]).to match_array /solve the reCAPTCHA/ end end - context 'when the model is not spam' do - [true, false].each do |enabled| - let(:recaptcha_enabled) { enabled } + context 'if the model is spam and also needs recaptcha' do + subject { invalidate_if_spam(is_spam: true, needs_recaptcha: true) } + + it 'has an error related to spam on the model' do + expect(subject.errors.messages[:base]).to match_array /solve the reCAPTCHA/ + end + end - subject { invalidate_if_spam(false, recaptcha_enabled) } + context 'when the model is not spam nor needs recaptcha' do + subject { invalidate_if_spam } - it 'returns no error' do - expect(subject.errors.messages[:base]).to be_empty - end + it 'returns no error' do + expect(subject.errors.messages[:base]).to be_empty end end - def invalidate_if_spam(is_spam, recaptcha_enabled) - stub_application_setting(recaptcha_enabled: recaptcha_enabled) + context 'if recaptcha is not enabled and the model needs recaptcha' do + before do + stub_application_setting(recaptcha_enabled: false) + end + + subject { invalidate_if_spam(needs_recaptcha: true) } + it 'has no errors' do + expect(subject.errors.messages[:base]).to match_array /has been discarded/ + end + end + + def invalidate_if_spam(is_spam: false, needs_recaptcha: false) issue.tap do |i| i.spam = is_spam + i.needs_recaptcha = needs_recaptcha i.invalidate_if_spam end end end + describe 'spam flags' do + before do + issue.spam = false + issue.needs_recaptcha = false + end + + describe '#spam!' do + it 'adds only `spam` flag' do + issue.spam! + + expect(issue.spam).to be_truthy + expect(issue.needs_recaptcha).to be_falsey + end + end + + describe '#needs_recaptcha!' do + it 'adds `needs_recaptcha` flag' do + issue.needs_recaptcha! + + expect(issue.spam).to be_falsey + expect(issue.needs_recaptcha).to be_truthy + end + end + + describe '#clear_spam_flags!' do + it 'clears spam and recaptcha flags' do + issue.spam = true + issue.needs_recaptcha = true + + issue.clear_spam_flags! + + expect(issue).not_to be_spam + expect(issue.needs_recaptcha).to be_falsey + end + end + end + describe '#submittable_as_spam_by?' do let(:admin) { build(:admin) } let(:user) { build(:user) } -- cgit v1.2.1