Skip to content

Commit

Permalink
Clear the cache for the key when set
Browse files Browse the repository at this point in the history
If setting a cache value is called within a TX and then the value is
accessed, then the old value is retrieved from the cache.

The fix is to clear the cached value for the var name immediately after
calling save!. Note, it is still appropriate to clear cache values upon
commit due to race conditions.
  • Loading branch information
justin808 committed Mar 1, 2016
1 parent 16cfcca commit 3e812a2
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 0 deletions.
13 changes: 13 additions & 0 deletions lib/rails-settings/cached_settings.rb
Expand Up @@ -24,6 +24,10 @@ def cache_key(var_name, scope_object)
scope << "#{var_name}"
end

def remove_cache_key(var_name, scope_object)
Rails.cache.delete(cache_key(var_name, scope_object))
end

def [](var_name)
value = Rails.cache.fetch(cache_key(var_name, @object)) do
super(var_name)
Expand All @@ -36,6 +40,15 @@ def [](var_name)
end
end

# set a setting value by [] notation
def []=(var_name, value)
super

Rails.cache.write(cache_key(var_name, @object),value)

value
end

def save_default(key, value)
return false unless self[key].nil?
self[key] = value
Expand Down
35 changes: 35 additions & 0 deletions spec/rails-settings-cached/cached_setting_spec.rb
@@ -1,6 +1,7 @@
require 'spec_helper'

describe RailsSettings::CachedSettings do
before(:each) { Rails.cache.clear }
describe '.cache_key' do
it 'should work with instance method' do
obj = Setting.unscoped.first
Expand Down Expand Up @@ -43,12 +44,27 @@
queries_count = count_queries do
expect(described_class["gender"]).to be nil
described_class["gender"] = "female"

# Call 4 times, make sure value is cached by fact number of queries does not go up.
expect(described_class["gender"]).to eq("female")
expect(described_class["gender"]).to eq("female")
expect(described_class["gender"]).to eq("female")
expect(described_class["gender"]).to eq("female")
end
expect(queries_count).to eq(5)
end

it "caches unscoped settings" do
expect(described_class["gender"]).to eq("female")
ActiveRecord::Base.transaction do
described_class["gender"] = "trans"
expect(described_class["gender"]).to eq("trans")
end

expect(described_class["gender"]).to eq("trans")
end


it "caches scoped settings" do
user = User.create!(login: 'another_test', password: 'foobar')

Expand All @@ -61,6 +77,25 @@
expect(queries_count).to eq(6)
end

it "caches scoped settings in transaction" do
user = User.create!(login: 'another_test2', password: 'foobar')

queries_count = count_queries do
expect(user.settings["gender"]).to be nil
ActiveRecord::Base.transaction do
user.settings["gender"] = "male"
expect(user.settings["gender"]).to eq("male")
end

# Call 4 times, make sure value is cached by fact number of queries does not go up.
expect(user.settings["gender"]).to eq("male")
expect(user.settings["gender"]).to eq("male")
expect(user.settings["gender"]).to eq("male")
expect(user.settings["gender"]).to eq("male")
end
expect(queries_count).to eq(6)
end

it "caches values from db" do
described_class["some_random_key"] = "asd"
Rails.cache.clear
Expand Down

0 comments on commit 3e812a2

Please sign in to comment.