diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 12d81436ffbd1e0451eb1f6b811c4d0f65c53eed..450669968b94cfa89737e264834b4140c8c37f0a 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,5 +1,10 @@ ## Rails 4.0.0 (unreleased) ## +* Patched Marshal#load to work with constant autoloading. + Fixes autoloading with cache stores that relay on Marshal(MemCacheStore and FileStore). [fixes #8167] + + *Uriel Katz* + * Make `Time.zone.parse` to work with JavaScript format date strings. *Andrew White* * Add `DateTime#seconds_until_end_of_day` and `Time#seconds_until_end_of_day` diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 2c1ad60d449ee072bd28bbe44cc0c9a873f9569f..8e265ad8637b928e5df026b3c019b8af38abddf5 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -1,3 +1,4 @@ +require 'active_support/core_ext/marshal' require 'active_support/core_ext/file/atomic' require 'active_support/core_ext/string/conversions' require 'uri/common' diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index 17450fe4d06518822763d02e9aeae2127dfd7d5d..712db2c75a35ceeb4d5b4714e2a7af85c13a01a9 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -6,6 +6,7 @@ end require 'digest/md5' +require 'active_support/core_ext/marshal' module ActiveSupport module Cache diff --git a/activesupport/lib/active_support/core_ext/marshal.rb b/activesupport/lib/active_support/core_ext/marshal.rb new file mode 100644 index 0000000000000000000000000000000000000000..fec3051c0c4da83a20353cbf0b294cf4054f405f --- /dev/null +++ b/activesupport/lib/active_support/core_ext/marshal.rb @@ -0,0 +1,21 @@ +module Marshal + class << self + def load_with_autoloading(source) + begin + load_without_autoloading(source) + rescue ArgumentError, NameError => exc + if exc.message.match(%r|undefined class/module (.+)|) + # try loading the class/module + $1.constantize + # if it is a IO we need to go back to read the object + source.rewind if source.respond_to?(:rewind) + retry + else + raise exc + end + end + end + + alias_method_chain :load, :autoloading + end +end \ No newline at end of file diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index ed903746c8b1812e06d24d3eaa636795c6438507..a2e2c51283805fa77db1373e0cfaf3bf1489f4c7 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -1,6 +1,7 @@ require 'logger' require 'abstract_unit' require 'active_support/cache' +require 'dependecies_test_helpers' class CacheKeyTest < ActiveSupport::TestCase def test_entry_legacy_optional_ivars @@ -562,6 +563,45 @@ def test_middleware end end +module AutoloadingCacheBehavior + include DependeciesTestHelpers + def test_simple_autoloading + with_autoloading_fixtures do + @cache.write('foo', E.new) + end + + remove_constants(:E) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_kind_of E, @cache.read('foo') + end + + remove_constants(:E) + ActiveSupport::Dependencies.clear + end + + def test_two_classes_autoloading + with_autoloading_fixtures do + @cache.write('foo', [E.new, ClassFolder.new]) + end + + remove_constants(:E, :ClassFolder) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + loaded = @cache.read('foo') + assert_kind_of Array, loaded + assert_equal 2, loaded.size + assert_kind_of E, loaded[0] + assert_kind_of ClassFolder, loaded[1] + end + + remove_constants(:E, :ClassFolder) + ActiveSupport::Dependencies.clear + end +end + class FileStoreTest < ActiveSupport::TestCase def setup Dir.mkdir(cache_dir) unless File.exist?(cache_dir) @@ -585,6 +625,7 @@ def cache_dir include LocalCacheBehavior include CacheDeleteMatchedBehavior include CacheIncrementDecrementBehavior + include AutoloadingCacheBehavior def test_clear filepath = File.join(cache_dir, ".gitkeep") @@ -745,6 +786,7 @@ def setup include LocalCacheBehavior include CacheIncrementDecrementBehavior include EncodedKeyCacheBehavior + include AutoloadingCacheBehavior def test_raw_values cache = ActiveSupport::Cache.lookup_store(:mem_cache_store, :raw => true) diff --git a/activesupport/test/core_ext/marshal_test.rb b/activesupport/test/core_ext/marshal_test.rb new file mode 100644 index 0000000000000000000000000000000000000000..ac79b15fa8775781b7198aa97e9dc518a77d9457 --- /dev/null +++ b/activesupport/test/core_ext/marshal_test.rb @@ -0,0 +1,124 @@ +require 'abstract_unit' +require 'active_support/core_ext/marshal' +require 'dependecies_test_helpers' + +class MarshalTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + include DependeciesTestHelpers + + def teardown + ActiveSupport::Dependencies.clear + remove_constants(:E, :ClassFolder) + end + + test "that Marshal#load still works" do + sanity_data = ["test", [1, 2, 3], {a: [1, 2, 3]}, ActiveSupport::TestCase] + sanity_data.each do |obj| + dumped = Marshal.dump(obj) + assert_equal Marshal.load_without_autoloading(dumped), Marshal.load(dumped) + end + end + + test "that a missing class is autoloaded from string" do + dumped = nil + with_autoloading_fixtures do + dumped = Marshal.dump(E.new) + end + + remove_constants(:E) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_kind_of E, Marshal.load(dumped) + end + end + + test "that classes in sub modules work" do + dumped = nil + with_autoloading_fixtures do + dumped = Marshal.dump(ClassFolder::ClassFolderSubclass.new) + end + + remove_constants(:ClassFolder) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_kind_of ClassFolder::ClassFolderSubclass, Marshal.load(dumped) + end + end + + test "that more than one missing class is autoloaded" do + dumped = nil + with_autoloading_fixtures do + dumped = Marshal.dump([E.new, ClassFolder.new]) + end + + remove_constants(:E, :ClassFolder) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + loaded = Marshal.load(dumped) + assert_equal 2, loaded.size + assert_kind_of E, loaded[0] + assert_kind_of ClassFolder, loaded[1] + end + end + + test "that a real missing class is causing an exception" do + dumped = nil + with_autoloading_fixtures do + dumped = Marshal.dump(E.new) + end + + remove_constants(:E) + ActiveSupport::Dependencies.clear + + assert_raise(NameError) do + Marshal.load(dumped) + end + end + + test "when first class is autoloaded and second not" do + dumped = nil + class SomeClass + end + + with_autoloading_fixtures do + dumped = Marshal.dump([E.new, SomeClass.new]) + end + + remove_constants(:E) + self.class.send(:remove_const, :SomeClass) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_raise(NameError) do + Marshal.load(dumped) + end + + assert_nothing_raised("E failed to load while we expect only SomeClass to fail loading") do + E.new + end + + assert_raise(NameError, "We expected SomeClass to not be loaded but it is!") do + SomeClass.new + end + end + end + + test "loading classes from files trigger autoloading" do + Tempfile.open("object_serializer_test") do |f| + with_autoloading_fixtures do + Marshal.dump(E.new, f) + end + + f.rewind + remove_constants(:E) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_kind_of E, Marshal.load(f) + end + end + end +end \ No newline at end of file diff --git a/activesupport/test/dependecies_test_helpers.rb b/activesupport/test/dependecies_test_helpers.rb new file mode 100644 index 0000000000000000000000000000000000000000..4b46d32fb43ee1892b50e6849c5f08655df3c85a --- /dev/null +++ b/activesupport/test/dependecies_test_helpers.rb @@ -0,0 +1,27 @@ +module DependeciesTestHelpers + def with_loading(*from) + old_mechanism, ActiveSupport::Dependencies.mechanism = ActiveSupport::Dependencies.mechanism, :load + this_dir = File.dirname(__FILE__) + parent_dir = File.dirname(this_dir) + path_copy = $LOAD_PATH.dup + $LOAD_PATH.unshift(parent_dir) unless $LOAD_PATH.include?(parent_dir) + prior_autoload_paths = ActiveSupport::Dependencies.autoload_paths + ActiveSupport::Dependencies.autoload_paths = from.collect { |f| "#{this_dir}/#{f}" } + yield + ensure + $LOAD_PATH.replace(path_copy) + ActiveSupport::Dependencies.autoload_paths = prior_autoload_paths + ActiveSupport::Dependencies.mechanism = old_mechanism + ActiveSupport::Dependencies.explicitly_unloadable_constants = [] + end + + def with_autoloading_fixtures(&block) + with_loading 'autoloading_fixtures', &block + end + + def remove_constants(*constants) + constants.each do |constant| + Object.send(:remove_const, constant) if Object.const_defined?(constant) + end + end +end \ No newline at end of file diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 7fea72dab59b37979a53abba7029c8933f6a4a5b..952c82f9d8eb52f18b70c751661c7a075e7bbbee 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -1,6 +1,7 @@ require 'abstract_unit' require 'pp' require 'active_support/dependencies' +require 'dependecies_test_helpers' module ModuleWithMissing mattr_accessor :missing_count @@ -19,25 +20,7 @@ def teardown ActiveSupport::Dependencies.clear end - def with_loading(*from) - old_mechanism, ActiveSupport::Dependencies.mechanism = ActiveSupport::Dependencies.mechanism, :load - this_dir = File.dirname(__FILE__) - parent_dir = File.dirname(this_dir) - path_copy = $LOAD_PATH.dup - $LOAD_PATH.unshift(parent_dir) unless $LOAD_PATH.include?(parent_dir) - prior_autoload_paths = ActiveSupport::Dependencies.autoload_paths - ActiveSupport::Dependencies.autoload_paths = from.collect { |f| "#{this_dir}/#{f}" } - yield - ensure - $LOAD_PATH.replace(path_copy) - ActiveSupport::Dependencies.autoload_paths = prior_autoload_paths - ActiveSupport::Dependencies.mechanism = old_mechanism - ActiveSupport::Dependencies.explicitly_unloadable_constants = [] - end - - def with_autoloading_fixtures(&block) - with_loading 'autoloading_fixtures', &block - end + include DependeciesTestHelpers def test_depend_on_path skip "LoadError#path does not exist" if RUBY_VERSION < '2.0.0' diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index e6f2db2a2df26ea87a596f04d2f0c685ac50a74d..9fca3d585b0f98f19c0fec954b6242120d11ee56 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -3720,6 +3720,27 @@ The auxiliary file is written in a standard directory for temporary files, but y NOTE: Defined in `active_support/core_ext/file/atomic.rb`. +Extensions to `Marshal` +-------------------- + +### `load` + +Unpatched Marshal#load doesn't call constant_missing so in order to support ActiveSupport constant autoloading load is patched using alias_method_chain. + +The method accepts the same arguments as unpatched Marshal#load and the result of calling it will be the same. + +For example, ActiveSupport uses this method to read from cache(in FileStore): + +```ruby +File.open(file_name) { |f| Marshal.load(f) } +``` + +If Marshal#load didn't support constant autoloading then various caching stores wouldn't too. + +WARNING. If a IO (e.g. a file) is used as source it needs to respond to rewind (which a normal file does) because if an exception is raised calling the original Marshal#load the file will be exhausted. + +NOTE: Defined in `active_support/core_ext/marshal.rb`. + Extensions to `Logger` ----------------------