diff options
author | Rémy Coutable <remy@rymai.me> | 2018-05-29 18:49:52 +0300 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2018-06-01 14:58:42 +0300 |
commit | a886532cc04bfa7ec6885ea883889f6d138961bd (patch) | |
tree | 17630f8544d7cf862d374a511e889ff6e5a6210e /spec | |
parent | e206e32881e4fbfcbe647d7b2ee713c99ef1bf99 (diff) |
Revert to caching the AR object in CacheableAttributes
Caching the attributes as JSON and manually instantiating the record
ended-up very complex since the edge-cases such as upload fields,
serialized fields, and fields with custom accessors had to be handled.
To ensure 3 points out of 4 are checked from https://gitlab.com/gitlab-org/gitlab-ce/issues/45175
we now include the Rails version in the cache key.
Signed-off-by: Rémy Coutable <remy@rymai.me>
Diffstat (limited to 'spec')
-rw-r--r-- | spec/models/concerns/cacheable_attributes_spec.rb | 124 |
1 files changed, 96 insertions, 28 deletions
diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb index 49e4b23ebc7..77f46518d47 100644 --- a/spec/models/concerns/cacheable_attributes_spec.rb +++ b/spec/models/concerns/cacheable_attributes_spec.rb @@ -22,7 +22,7 @@ describe CacheableAttributes do attr_accessor :attributes - def initialize(attrs = {}) + def initialize(attrs = {}, *) @attributes = attrs end end @@ -52,7 +52,7 @@ describe CacheableAttributes do describe '.cache_key' do it 'excludes cache attributes' do - expect(minimal_test_class.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:json") + expect(minimal_test_class.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Gitlab.migrations_hash}:#{Rails.version}") end end @@ -75,47 +75,106 @@ describe CacheableAttributes do context 'without any attributes given' do it 'intializes a new object with the defaults' do - expect(minimal_test_class.build_from_defaults).not_to be_persisted + expect(minimal_test_class.build_from_defaults.attributes).to eq(minimal_test_class.defaults) end end - context 'without attributes given' do + context 'with attributes given' do it 'intializes a new object with the given attributes merged into the defaults' do expect(minimal_test_class.build_from_defaults(foo: 'd').attributes[:foo]).to eq('d') end end + + describe 'edge cases on concrete implementations' do + describe '.build_from_defaults' do + context 'without any attributes given' do + it 'intializes all attributes even if they are nil' do + record = ApplicationSetting.build_from_defaults + + expect(record).not_to be_persisted + expect(record.sign_in_text).to be_nil + end + end + end + end end describe '.current', :use_clean_rails_memory_store_caching do context 'redis unavailable' do - it 'returns an uncached record' do + before do allow(minimal_test_class).to receive(:last).and_return(:last) - expect(Rails.cache).to receive(:read).and_raise(Redis::BaseError) + expect(Rails.cache).to receive(:read).with(minimal_test_class.cache_key).and_raise(Redis::BaseError) + end + + context 'in production environment' do + before do + expect(Rails.env).to receive(:production?).and_return(true) + end - expect(minimal_test_class.current).to eq(:last) + it 'returns an uncached record and logs a warning' do + expect(Rails.logger).to receive(:warn).with("Cached record for TestClass couldn't be loaded, falling back to uncached record: Redis::BaseError") + + expect(minimal_test_class.current).to eq(:last) + end + end + + context 'in other environments' do + before do + expect(Rails.env).to receive(:production?).and_return(false) + end + + it 'returns an uncached record and logs a warning' do + expect(Rails.logger).not_to receive(:warn) + + expect { minimal_test_class.current }.to raise_error(Redis::BaseError) + end end end context 'when a record is not yet present' do it 'does not cache nil object' do # when missing settings a nil object is returned, but not cached - allow(minimal_test_class).to receive(:last).twice.and_return(nil) + allow(ApplicationSetting).to receive(:current_without_cache).twice.and_return(nil) - expect(minimal_test_class.current).to be_nil - expect(Rails.cache.exist?(minimal_test_class.cache_key)).to be(false) + expect(ApplicationSetting.current).to be_nil + expect(Rails.cache.exist?(ApplicationSetting.cache_key)).to be(false) end - it 'cache non-nil object' do - # when the settings are set the method returns a valid object - allow(minimal_test_class).to receive(:last).and_call_original + it 'caches non-nil object' do + create(:application_setting) - expect(minimal_test_class.current).to eq(minimal_test_class.last) - expect(Rails.cache.exist?(minimal_test_class.cache_key)).to be(true) + expect(ApplicationSetting.current).to eq(ApplicationSetting.last) + expect(Rails.cache.exist?(ApplicationSetting.cache_key)).to be(true) # subsequent calls retrieve the record from the cache - last_record = minimal_test_class.last - expect(minimal_test_class).not_to receive(:last) - expect(minimal_test_class.current.attributes).to eq(last_record.attributes) + last_record = ApplicationSetting.last + expect(ApplicationSetting).not_to receive(:current_without_cache) + expect(ApplicationSetting.current.attributes).to eq(last_record.attributes) + end + end + + describe 'edge cases' do + describe 'caching behavior', :use_clean_rails_memory_store_caching do + it 'retrieves upload fields properly' do + ar_record = create(:appearance, :with_logo) + ar_record.cache! + + cache_record = Appearance.current + + expect(cache_record).to be_persisted + expect(cache_record.logo).to be_an(AttachmentUploader) + expect(cache_record.logo.url).to end_with('/dk.png') + end + + it 'retrieves markdown fields properly' do + ar_record = create(:appearance, description: '**Hello**') + ar_record.cache! + + cache_record = Appearance.current + + expect(cache_record.description).to eq('**Hello**') + expect(cache_record.description_html).to eq('<p dir="auto"><strong>Hello</strong></p>') + end end end end @@ -127,27 +186,36 @@ describe CacheableAttributes do end end - context 'when cached settings do not include the latest defaults' do + context 'when cached is warm' do before do - Rails.cache.write(minimal_test_class.cache_key, { bar: 'b', baz: 'c' }.to_json) - minimal_test_class.define_singleton_method(:defaults) do - { foo: 'a', bar: 'b', baz: 'c' } - end + # Warm up the cache + create(:appearance).cache! end - it 'includes attributes from defaults' do - expect(minimal_test_class.cached.attributes[:foo]).to eq(minimal_test_class.defaults[:foo]) + it 'retrieves the record from cache' do + expect(ActiveRecord::QueryRecorder.new { Appearance.cached }.count).to eq(0) + expect(Appearance.cached).to eq(Appearance.current_without_cache) end end end describe '#cache!', :use_clean_rails_memory_store_caching do - let(:appearance_record) { create(:appearance) } + let(:record) { create(:appearance) } it 'caches the attributes' do - appearance_record.cache! + record.cache! + + expect(Rails.cache.read(Appearance.cache_key)).to eq(record) + end + + describe 'edge cases' do + let(:record) { create(:appearance) } - expect(Rails.cache.read(Appearance.cache_key)).to eq(appearance_record.attributes.to_json) + it 'caches the attributes' do + record.cache! + + expect(Rails.cache.read(Appearance.cache_key)).to eq(record) + end end end end |