diff options
| -rw-r--r-- | app/models/appearance.rb | 15 | ||||
| -rw-r--r-- | app/models/application_setting.rb | 36 | ||||
| -rw-r--r-- | app/uploaders/object_storage.rb | 1 | ||||
| -rw-r--r-- | spec/models/appearance_spec.rb | 27 | ||||
| -rw-r--r-- | spec/models/application_setting_spec.rb | 30 |
5 files changed, 11 insertions, 98 deletions
diff --git a/app/models/appearance.rb b/app/models/appearance.rb index f8713138a93..67cc84a9140 100644 --- a/app/models/appearance.rb +++ b/app/models/appearance.rb @@ -1,6 +1,6 @@ class Appearance < ActiveRecord::Base + include CacheableAttributes include CacheMarkdownField - include AfterCommitQueue include ObjectStorage::BackgroundMove include WithUploads @@ -15,16 +15,9 @@ class Appearance < ActiveRecord::Base mount_uploader :logo, AttachmentUploader mount_uploader :header_logo, AttachmentUploader - CACHE_KEY = "current_appearance:#{Gitlab::VERSION}".freeze - - after_commit :flush_redis_cache - - def self.current - Rails.cache.fetch(CACHE_KEY) { first } - end - - def flush_redis_cache - Rails.cache.delete(CACHE_KEY) + # Overrides CacheableAttributes.current_without_cache + def self.current_without_cache + first end def single_appearance_row diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 451e512aef7..e8ccb320fae 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -1,11 +1,11 @@ class ApplicationSetting < ActiveRecord::Base + include CacheableAttributes include CacheMarkdownField include TokenAuthenticatable add_authentication_token_field :runners_registration_token add_authentication_token_field :health_check_access_token - CACHE_KEY = 'application_setting.last'.freeze DOMAIN_LIST_SEPARATOR = %r{\s*[,;]\s* # comma or semicolon, optionally surrounded by whitespace | # or \s # any whitespace character @@ -229,40 +229,6 @@ class ApplicationSetting < ActiveRecord::Base after_commit do reset_memoized_terms - Rails.cache.write(CACHE_KEY, self) - end - - def self.current - ensure_cache_setup - - Rails.cache.fetch(CACHE_KEY) do - ApplicationSetting.last.tap do |settings| - # do not cache nils - raise 'missing settings' unless settings - end - end - rescue - # Fall back to an uncached value if there are any problems (e.g. redis down) - ApplicationSetting.last - end - - def self.expire - Rails.cache.delete(CACHE_KEY) - rescue - # Gracefully handle when Redis is not available. For example, - # omnibus may fail here during gitlab:assets:compile. - end - - def self.cached - value = Rails.cache.read(CACHE_KEY) - ensure_cache_setup if value.present? - value - end - - def self.ensure_cache_setup - # This is a workaround for a Rails bug that causes attribute methods not - # to be loaded when read from cache: https://github.com/rails/rails/issues/27348 - ApplicationSetting.define_attribute_methods end def self.defaults diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index a3549cada95..f2a8afccdeb 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -103,6 +103,7 @@ module ObjectStorage end included do + include AfterCommitQueue after_save on: [:create, :update] do background_upload(changed_mounts) end diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index 5489c17bd82..77b07cf1ac9 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -3,34 +3,11 @@ require 'rails_helper' describe Appearance do subject { build(:appearance) } - it { is_expected.to be_valid } + it { include(CacheableAttributes) } + it { expect(described_class.current_without_cache).to eq(described_class.first) } it { is_expected.to have_many(:uploads) } - describe '.current', :use_clean_rails_memory_store_caching do - let!(:appearance) { create(:appearance) } - - it 'returns the current appearance row' do - expect(described_class.current).to eq(appearance) - end - - it 'caches the result' do - expect(described_class).to receive(:first).once - - 2.times { described_class.current } - end - end - - describe '#flush_redis_cache' do - it 'flushes the cache in Redis' do - appearance = create(:appearance) - - expect(Rails.cache).to receive(:delete).with(described_class::CACHE_KEY) - - appearance.flush_redis_cache - end - end - describe '#single_appearance_row' do it 'adds an error when more than 1 row exists' do create(:appearance) diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index 10d6109cae7..7e47043a1cb 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -3,6 +3,9 @@ require 'spec_helper' describe ApplicationSetting do let(:setting) { described_class.create_from_defaults } + it { include(CacheableAttributes) } + it { expect(described_class.current_without_cache).to eq(described_class.last) } + it { expect(setting).to be_valid } it { expect(setting.uuid).to be_present } it { expect(setting).to have_db_column(:auto_devops_enabled) } @@ -318,33 +321,6 @@ describe ApplicationSetting do end end - describe '.current' do - context 'redis unavailable' do - it 'returns an ApplicationSetting' do - allow(Rails.cache).to receive(:fetch).and_call_original - allow(described_class).to receive(:last).and_return(:last) - expect(Rails.cache).to receive(:fetch).with(ApplicationSetting::CACHE_KEY).and_raise(ArgumentError) - - expect(described_class.current).to eq(:last) - end - end - - context 'when an ApplicationSetting is not yet present' do - it 'does not cache nil object' do - # when missing settings a nil object is returned, but not cached - allow(described_class).to receive(:last).and_return(nil).twice - expect(described_class.current).to be_nil - - # when the settings are set the method returns a valid object - allow(described_class).to receive(:last).and_return(:last) - expect(described_class.current).to eq(:last) - - # subsequent calls get everything from cache - expect(described_class.current).to eq(:last) - end - end - end - context 'restrict creating duplicates' do before do described_class.create_from_defaults |