From 64016542a869f326f01d788eb67e32a347b30d06 Mon Sep 17 00:00:00 2001 From: Ryan Bates Date: Sat, 9 Aug 2008 17:49:32 -0700 Subject: [PATCH] support marshal in MemoryStore and FileStore, configure with :raw option --- .../lib/action_controller/caching/fragments.rb | 4 +- actionpack/test/controller/caching_test.rb | 50 +++++----- .../lib/active_support/cache/file_store.rb | 16 +++- .../lib/active_support/cache/memory_store.rb | 26 +++++- activesupport/test/caching_test.rb | 97 ++++++++++++++++++++ railties/lib/initializer.rb | 9 +- 6 files changed, 164 insertions(+), 38 deletions(-) diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index e9b434d..411b017 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -81,7 +81,7 @@ module ActionController #:nodoc: key = fragment_cache_key(key) self.class.benchmark "Cached fragment miss: #{key}" do - cache_store.write(key, content, options) + cache_store.write(key, content, (options || {}).reverse_merge(:raw => true)) end content @@ -94,7 +94,7 @@ module ActionController #:nodoc: key = fragment_cache_key(key) self.class.benchmark "Cached fragment hit: #{key}" do - cache_store.read(key, options) + cache_store.read(key, (options || {}).reverse_merge(:raw => true)) end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 47a0fcf..1d422c4 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -493,64 +493,64 @@ class FragmentCachingTest < Test::Unit::TestCase end def test_read_fragment_with_caching_enabled - @store.write('views/name', 'value') + @store.write('views/name', 'value', :raw => true) assert_equal 'value', @controller.read_fragment('name') end def test_read_fragment_with_caching_disabled ActionController::Base.perform_caching = false - @store.write('views/name', 'value') + @store.write('views/name', 'value', :raw => true) assert_nil @controller.read_fragment('name') end def test_fragment_exist_with_caching_enabled - @store.write('views/name', 'value') + @store.write('views/name', 'value', :raw => true) assert @controller.fragment_exist?('name') assert !@controller.fragment_exist?('other_name') end def test_fragment_exist_with_caching_disabled ActionController::Base.perform_caching = false - @store.write('views/name', 'value') + @store.write('views/name', 'value', :raw => true) assert !@controller.fragment_exist?('name') assert !@controller.fragment_exist?('other_name') end def test_write_fragment_with_caching_enabled - assert_nil @store.read('views/name') + assert_nil @store.read('views/name', :raw => true) assert_equal 'value', @controller.write_fragment('name', 'value') - assert_equal 'value', @store.read('views/name') + assert_equal 'value', @store.read('views/name', :raw => true) end def test_write_fragment_with_caching_disabled - assert_nil @store.read('views/name') + assert_nil @store.read('views/name', :raw => true) ActionController::Base.perform_caching = false assert_equal nil, @controller.write_fragment('name', 'value') - assert_nil @store.read('views/name') + assert_nil @store.read('views/name', :raw => true) end def test_expire_fragment_with_simple_key - @store.write('views/name', 'value') + @store.write('views/name', 'value', :raw => true) @controller.expire_fragment 'name' - assert_nil @store.read('views/name') + assert_nil @store.read('views/name', :raw => true) end def test_expire_fragment_with_regexp - @store.write('views/name', 'value') - @store.write('views/another_name', 'another_value') - @store.write('views/primalgrasp', 'will not expire ;-)') + @store.write('views/name', 'value', :raw => true) + @store.write('views/another_name', 'another_value', :raw => true) + @store.write('views/primalgrasp', 'will not expire ;-)', :raw => true) @controller.expire_fragment /name/ - assert_nil @store.read('views/name') - assert_nil @store.read('views/another_name') - assert_equal 'will not expire ;-)', @store.read('views/primalgrasp') + assert_nil @store.read('views/name', :raw => true) + assert_nil @store.read('views/another_name', :raw => true) + assert_equal 'will not expire ;-)', @store.read('views/primalgrasp', :raw => true) end def test_fragment_for_with_disabled_caching ActionController::Base.perform_caching = false - @store.write('views/expensive', 'fragment content') + @store.write('views/expensive', 'fragment content', :raw => true) fragment_computed = false buffer = 'generated till now -> ' @@ -561,7 +561,7 @@ class FragmentCachingTest < Test::Unit::TestCase end def test_fragment_for - @store.write('views/expensive', 'fragment content') + @store.write('views/expensive', 'fragment content', :raw => true) fragment_computed = false buffer = 'generated till now -> ' @@ -620,14 +620,14 @@ This bit's fragment cached CACHED assert_equal expected_body, @response.body - assert_equal "This bit's fragment cached", @store.read('views/test.host/functional_caching/fragment_cached') + assert_equal "This bit's fragment cached", @store.read('views/test.host/functional_caching/fragment_cached', :raw => true) end def test_fragment_caching_in_partials get :html_fragment_cached_with_partial assert_response :success assert_match /Fragment caching in a partial/, @response.body - assert_match "Fragment caching in a partial", @store.read('views/test.host/functional_caching/html_fragment_cached_with_partial') + assert_match "Fragment caching in a partial", @store.read('views/test.host/functional_caching/html_fragment_cached_with_partial', :raw => true) end def test_render_inline_before_fragment_caching @@ -635,14 +635,14 @@ CACHED assert_response :success assert_match /Some inline content/, @response.body assert_match /Some cached content/, @response.body - assert_match "Some cached content", @store.read('views/test.host/functional_caching/inline_fragment_cached') + assert_match "Some cached content", @store.read('views/test.host/functional_caching/inline_fragment_cached', :raw => true) end def test_fragment_caching_in_rjs_partials xhr :get, :js_fragment_cached_with_partial assert_response :success assert_match /Fragment caching in a partial/, @response.body - assert_match "Fragment caching in a partial", @store.read('views/test.host/functional_caching/js_fragment_cached_with_partial') + assert_match "Fragment caching in a partial", @store.read('views/test.host/functional_caching/js_fragment_cached_with_partial', :raw => true) end def test_html_formatted_fragment_caching @@ -652,7 +652,7 @@ CACHED assert_equal expected_body, @response.body - assert_equal "

ERB

", @store.read('views/test.host/functional_caching/formatted_fragment_cached') + assert_equal "

ERB

", @store.read('views/test.host/functional_caching/formatted_fragment_cached', :raw => true) end def test_xml_formatted_fragment_caching @@ -662,7 +662,7 @@ CACHED assert_equal expected_body, @response.body - assert_equal "

Builder

\n", @store.read('views/test.host/functional_caching/formatted_fragment_cached') + assert_equal "

Builder

\n", @store.read('views/test.host/functional_caching/formatted_fragment_cached', :raw => true) end def test_js_formatted_fragment_caching @@ -673,6 +673,6 @@ CACHED assert_equal expected_body, @response.body assert_equal ['$("element_1").visualEffect("highlight");', '$("element_2").visualEffect("highlight");'], - @store.read('views/test.host/functional_caching/formatted_fragment_cached') + @store.read('views/test.host/functional_caching/formatted_fragment_cached', :raw => true) end end diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 7b6ca39..ddab9d3 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -9,13 +9,25 @@ module ActiveSupport def read(name, options = nil) super - File.open(real_file_path(name), 'rb') { |f| f.read } rescue nil + File.open(real_file_path(name), 'rb') do |f| + if options && options[:raw] + f.read + else + Marshal.load(f.read) + end + end rescue nil end def write(name, value, options = nil) super ensure_cache_path(File.dirname(real_file_path(name))) - File.atomic_write(real_file_path(name), cache_path) { |f| f.write(value) } + File.atomic_write(real_file_path(name), cache_path) do |f| + if options && options[:raw] + f.write(value) + else + f.write(Marshal.dump(value)) + end + end rescue => e RAILS_DEFAULT_LOGGER.error "Couldn't create cache directory: #{name} (#{e.message})" if RAILS_DEFAULT_LOGGER end diff --git a/activesupport/lib/active_support/cache/memory_store.rb b/activesupport/lib/active_support/cache/memory_store.rb index a44f877..fc2e8fd 100644 --- a/activesupport/lib/active_support/cache/memory_store.rb +++ b/activesupport/lib/active_support/cache/memory_store.rb @@ -1,7 +1,8 @@ module ActiveSupport module Cache class MemoryStore < Store - def initialize + def initialize(options = {}) + @raw = options.has_key?(:raw) ? options[:raw] : true @data = {} @mutex = Mutex.new end @@ -14,12 +15,20 @@ module ActiveSupport def read(name, options = nil) super - @data[name] + if raw?(options) + dup_value(@data[name]) + elsif @data[name] + Marshal.load(@data[name]) + end end def write(name, value, options = nil) super - @data[name] = value + if raw?(options) + @data[name] = dup_value(value) + else + @data[name] = Marshal.dump(value) + end end def delete(name, options = nil) @@ -49,6 +58,17 @@ module ActiveSupport def clear @data.clear end + + protected + def raw?(options) + options && options.has_key?(:raw) ? options[:raw] : @raw + end + + def dup_value(value) + value.dup + rescue TypeError + value + end end end end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index f3220d2..64f92c9 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -70,3 +70,100 @@ uses_mocha 'high-level cache store tests' do end end end + +class MemoryStoreTest < Test::Unit::TestCase + def setup + @cache = ActiveSupport::Cache.lookup_store(:memory_store) + end + + def test_should_not_change_state_after_cache + hash = {:foo => 1} + @cache.write('hash', hash) + hash[:foo] = 2 + assert_not_equal hash, @cache.read('hash') + end + + def test_should_be_raw_by_default + @cache.write('foo', 'bar') + assert_equal @cache.read('foo', :raw => true), @cache.read('foo') + end + + def test_should_return_nil_for_non_existing_key + assert_equal nil, @cache.read('doesntexist', :raw => true) + assert_equal nil, @cache.read('doesntexist', :raw => false) + end +end + +class MemoryStoreRawTest < Test::Unit::TestCase + def test_should_not_be_raw_when_specified + @cache = ActiveSupport::Cache.lookup_store(:memory_store, :raw => false) + @cache.write('foo', 'bar') + assert_equal @cache.read('foo', :raw => false), @cache.read('foo') + end + + def test_should_be_raw_when_specified + @cache = ActiveSupport::Cache.lookup_store(:memory_store, :raw => true) + @cache.write('foo', 'bar') + assert_equal @cache.read('foo', :raw => true), @cache.read('foo') + end +end + +uses_mocha 'memory store marshal tests' do + class MemoryStoreMarshalTest < Test::Unit::TestCase + def setup + @cache = ActiveSupport::Cache.lookup_store(:memory_store) + end + + def test_should_not_use_marshal_with_raw + Marshal.expects(:dump).never + @cache.write('foo', 'bar', :raw => true) + Marshal.expects(:load).never + assert_equal 'bar', @cache.read('foo', :raw => true) + end + + def test_should_use_marshal_without_raw + Marshal.expects(:dump).with('bar').returns(:marshal_dump) + @cache.write('foo', 'bar', :raw => false) + Marshal.expects(:load).with(:marshal_dump).returns(:marshal_load) + assert_equal :marshal_load, @cache.read('foo', :raw => false) + end + end +end + +uses_mocha 'file based cache store tests' do + class FileStoreTest < Test::Unit::TestCase + def setup + # we don't want to touch the file system at all here + @file = StringIO.new + File.stubs(:open).yields(@file) + File.stubs(:atomic_write).yields(@file) + @cache = ActiveSupport::Cache.lookup_store(:file_store, '/dev/null') + end + + def test_should_write_marshalled_data_to_file + @cache.write('foo', 'bar') + @file.rewind + assert_equal Marshal.dump('bar'), @file.read + end + + def test_should_read_marshalled_data_from_file + @file.write('bar') + @file.rewind + Marshal.expects(:load).with('bar') + @cache.read('foo') + end + + def test_should_write_raw_data_to_file_with_raw_option + @cache.write('foo', 'bar', :raw => true) + @file.rewind + assert_equal 'bar', @file.read + end + + def test_should_read_raw_data_to_file_with_raw_option + @file.write('bar') + @file.rewind + Marshal.expects(:load).never + @cache.read('foo', :raw => true) + end + end +end diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb index 6576cd3..9c1de79 100644 --- a/railties/lib/initializer.rb +++ b/railties/lib/initializer.rb @@ -622,7 +622,7 @@ Run `rake gems:install` to install the missing gems. # used directly. attr_accessor :logger - # The specific cache store to use. By default, the ActiveSupport::Cache::Store will be used. + # The specific cache store to use. By default, the ActiveSupport::Cache::MemoryStore will be used. attr_accessor :cache_store # The root of the application's views. (Defaults to app/views.) @@ -745,6 +745,7 @@ Run `rake gems:install` to install the missing gems. self.database_configuration_file = default_database_configuration_file self.routes_configuration_file = default_routes_configuration_file self.gems = default_gems + self.cache_store = default_cache_store for framework in default_frameworks self.send("#{framework}=", Rails::OrderedOptions.new) @@ -949,11 +950,7 @@ Run `rake gems:install` to install the missing gems. end def default_cache_store - if File.exist?("#{root_path}/tmp/cache/") - [ :file_store, "#{root_path}/tmp/cache/" ] - else - :memory_store - end + [:memory_store, {:raw => (environment != 'development')}] end def default_gems -- 1.5.5.1