Unverified Commit e531a0c1 authored by Rémy Coutable's avatar Rémy Coutable
Browse files

Use the new CacheableAttributes concern in the ApplicationSetting and Appearance models


Signed-off-by: default avatarRémy Coutable <remy@rymai.me>
parent 02f17a09
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
......
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
......
......@@ -103,6 +103,7 @@ def changed_mounts
end
included do
include AfterCommitQueue
after_save on: [:create, :update] do
background_upload(changed_mounts)
end
......
......@@ -3,34 +3,11 @@
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)
......
......@@ -3,6 +3,9 @@
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 @@ def expect_invalid
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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment