From 1730926c9ddb122cb55c7269e9ef46f8942c31ab Mon Sep 17 00:00:00 2001 From: Ryan Bigg Date: Thu, 10 Oct 2013 11:03:49 +1100 Subject: [PATCH] Ensure that preference keys are not prefixed with '/' if ENV['RAILS_CACHE_KEY'] is not set Fixes #3831 --- .../models/spree/preferences/configuration.rb | 6 +++++- .../models/spree/preferences/preferable.rb | 6 +++++- .../spree/preferences/configuration_spec.rb | 17 ++++++++++++++++ .../spree/preferences/preferable_spec.rb | 20 +++++++++++++++++++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/core/app/models/spree/preferences/configuration.rb b/core/app/models/spree/preferences/configuration.rb index 8636c96ed4f..ee8929ba8b2 100644 --- a/core/app/models/spree/preferences/configuration.rb +++ b/core/app/models/spree/preferences/configuration.rb @@ -29,7 +29,11 @@ def configure end def preference_cache_key(name) - [ENV['RAILS_CACHE_ID'], self.class.name, name].flatten.join('::').underscore + [rails_cache_id, self.class.name, name].compact.join('::').underscore + end + + def rails_cache_id + ENV['RAILS_CACHE_ID'] end def reset diff --git a/core/app/models/spree/preferences/preferable.rb b/core/app/models/spree/preferences/preferable.rb index a02ab9265d3..4a5d8cbf53e 100644 --- a/core/app/models/spree/preferences/preferable.rb +++ b/core/app/models/spree/preferences/preferable.rb @@ -78,7 +78,11 @@ def prefers?(name) def preference_cache_key(name) return unless id - [ENV["RAILS_CACHE_ID"], self.class.name, name, id].join('::').underscore + [rails_cache_id, self.class.name, name, id].compact.join('::').underscore + end + + def rails_cache_id + ENV['RAILS_CACHE_ID'] end def save_pending_preferences diff --git a/core/spec/models/spree/preferences/configuration_spec.rb b/core/spec/models/spree/preferences/configuration_spec.rb index a1b45d05341..f1650de318e 100644 --- a/core/spec/models/spree/preferences/configuration_spec.rb +++ b/core/spec/models/spree/preferences/configuration_spec.rb @@ -9,6 +9,23 @@ class AppConfig < Spree::Preferences::Configuration @config = AppConfig.new end + # Regression test for #3831 + context "with a rails cache id set" do + before do + @config.stub :rails_cache_id => "cache" + end + + it "can access the preference cache key" do + expect(@config.preference_cache_key("foo")).to eql("cache/app_config/foo") + end + end + + context "with no rails cache id set" do + it "does not prefix the preference cache key with a slash" do + expect(@config.preference_cache_key("foo")).to eql("app_config/foo") + end + end + it "has named methods to access preferences" do @config.color = 'orange' @config.color.should eq 'orange' diff --git a/core/spec/models/spree/preferences/preferable_spec.rb b/core/spec/models/spree/preferences/preferable_spec.rb index e01fcf81141..f5f5dc61d79 100644 --- a/core/spec/models/spree/preferences/preferable_spec.rb +++ b/core/spec/models/spree/preferences/preferable_spec.rb @@ -31,6 +31,26 @@ class B < A store.persistence = true end + # Regression test for #3831 + describe "preference cache key" do + context "with a rails_cache_id set" do + before do + @a.stub(:rails_cache_id => 'cache') + end + + it "includes the cache id within the key" do + expect(@a.preference_cache_key('foo')).to eql("cache/a/foo/#{@a.id}") + end + end + + context "without a rails_cache_id set" do + it "includes the cache id within the key" do + expect(@a.preference_cache_key('foo')).to eql("a/foo/#{@a.id}") + end + end + + end + describe "preference definitions" do it "parent should not see child definitions" do @a.has_preference?(:color).should be_true