diff options
author | Victor Costan <costan@gmail.com> | 2013-11-24 12:52:53 -0500 |
---|---|---|
committer | Victor Costan <costan@gmail.com> | 2013-12-03 02:52:26 -0500 |
commit | ddf27acbc285a892842866cde04951cdad52c5c9 (patch) | |
tree | 9497d0ae23ce1217adb5d21143587c54825dc564 | |
parent | abc1e5831cae724cfcdf524f62abb71be02d7e86 (diff) | |
download | rails-ddf27acbc285a892842866cde04951cdad52c5c9.tar.gz rails-ddf27acbc285a892842866cde04951cdad52c5c9.tar.bz2 rails-ddf27acbc285a892842866cde04951cdad52c5c9.zip |
Introduce a context for rendering fixtures ERB.
Fixture files are passed through an ERB renderer before being read as
YAML. The rendering is currently done in the context of the main object,
so method definitons leak into other fixtures, and there is no clean
place to define fixture helpers.
After this commit, the ERB renderer will use a new subclass of
ActiveRecord::FixtureSet.context_class each time a fixture is rendered.
-rw-r--r-- | activerecord/CHANGELOG.md | 6 | ||||
-rw-r--r-- | activerecord/lib/active_record/fixture_set/file.rb | 3 | ||||
-rw-r--r-- | activerecord/lib/active_record/fixtures.rb | 32 | ||||
-rw-r--r-- | activerecord/test/cases/fixture_set/file_test.rb | 55 | ||||
-rw-r--r-- | guides/source/4_1_release_notes.md | 4 | ||||
-rw-r--r-- | guides/source/upgrading_ruby_on_rails.md | 17 |
6 files changed, 116 insertions, 1 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 34603b7f23..58222e8111 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* The ERB in fixture files is no longer evaluated in the context of the main + object. Helper methods used by multiple fixtures should be defined on the + class object returned by `ActiveRecord::FixtureSet.context_class`. + + *Victor Costan* + * Previously, the `has_one` macro incorrectly accepted the `counter_cache` option, but never actually supported it. Now it will raise an `ArgumentError` when using `has_one` with `counter_cache`. diff --git a/activerecord/lib/active_record/fixture_set/file.rb b/activerecord/lib/active_record/fixture_set/file.rb index fbd7a4d891..8132310c91 100644 --- a/activerecord/lib/active_record/fixture_set/file.rb +++ b/activerecord/lib/active_record/fixture_set/file.rb @@ -38,7 +38,8 @@ module ActiveRecord end def render(content) - ERB.new(content).result + context = ActiveRecord::FixtureSet::RenderContext.create_subclass.new + ERB.new(content).result(context.get_binding) end # Validate our unmarshalled data. diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 8601414209..d241788a9b 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -119,6 +119,23 @@ module ActiveRecord # perhaps you should reexamine whether your application is properly testable. Hence, dynamic values # in fixtures are to be considered a code smell. # + # Helper methods defined in a fixture will not be available in other fixtures, to prevent against + # unwanted inter-test dependencies. Methods used by multiple fixtures should be defined in a module + # that is included in <tt>ActiveRecord::FixtureSet.context_class</tt>. + # + # - define a helper method in `test_helper.rb` + # class FixtureFileHelpers + # def file_sha(path) + # Digest::SHA2.hexdigest(File.read(Rails.root.join('test/fixtures', path))) + # end + # end + # ActiveRecord::FixtureSet.context_class.send :include, FixtureFileHelpers + # + # - use the helper method in a fixture + # photo: + # name: kitten.png + # sha: <%= file_sha 'files/kitten.png' %> + # # = Transactional Fixtures # # Test cases can use begin+rollback to isolate their changes to the database instead of having to @@ -529,6 +546,11 @@ module ActiveRecord Zlib.crc32(label.to_s) % MAX_ID end + # Superclass for the evaluation contexts used by ERB fixtures. + def self.context_class + @context_class ||= Class.new + end + attr_reader :table_name, :name, :fixtures, :model_class, :config def initialize(connection, name, class_name, path, config = ActiveRecord::Base) @@ -989,3 +1011,13 @@ module ActiveRecord end end end + +class ActiveRecord::FixtureSet::RenderContext # :nodoc: + def self.create_subclass + Class.new ActiveRecord::FixtureSet.context_class do + def get_binding + binding() + end + end + end +end diff --git a/activerecord/test/cases/fixture_set/file_test.rb b/activerecord/test/cases/fixture_set/file_test.rb index a029fedbd3..92efa8aca7 100644 --- a/activerecord/test/cases/fixture_set/file_test.rb +++ b/activerecord/test/cases/fixture_set/file_test.rb @@ -68,6 +68,61 @@ module ActiveRecord end end + def test_render_context_helper + ActiveRecord::FixtureSet.context_class.class_eval do + def fixture_helper + "Fixture helper" + end + end + yaml = "one:\n name: <%= fixture_helper %>\n" + tmp_yaml ['curious', 'yml'], yaml do |t| + golden = + [["one", {"name" => "Fixture helper"}]] + assert_equal golden, File.open(t.path) { |fh| fh.to_a } + end + ActiveRecord::FixtureSet.context_class.class_eval do + remove_method :fixture_helper + end + end + + def test_render_context_lookup_scope + yaml = <<END +one: + ActiveRecord: <%= defined? ActiveRecord %> + ActiveRecord_FixtureSet: <%= defined? ActiveRecord::FixtureSet %> + FixtureSet: <%= defined? FixtureSet %> + ActiveRecord_FixtureSet_File: <%= defined? ActiveRecord::FixtureSet::File %> + File: <%= File.name %> +END + + golden = [['one', { + 'ActiveRecord' => 'constant', + 'ActiveRecord_FixtureSet' => 'constant', + 'FixtureSet' => nil, + 'ActiveRecord_FixtureSet_File' => 'constant', + 'File' => 'File' + }]] + + tmp_yaml ['curious', 'yml'], yaml do |t| + assert_equal golden, File.open(t.path) { |fh| fh.to_a } + end + end + + # Make sure that each fixture gets its own rendering context so that + # fixtures are independent. + def test_independent_render_contexts + yaml1 = "<% def leaked_method; 'leak'; end %>\n" + yaml2 = "one:\n name: <%= leaked_method %>\n" + tmp_yaml ['leaky', 'yml'], yaml1 do |t1| + tmp_yaml ['curious', 'yml'], yaml2 do |t2| + File.open(t1.path) { |fh| fh.to_a } + assert_raises(NameError) do + File.open(t2.path) { |fh| fh.to_a } + end + end + end + end + private def tmp_yaml(name, contents) t = Tempfile.new name diff --git a/guides/source/4_1_release_notes.md b/guides/source/4_1_release_notes.md index f278aa9ea5..de29b2e58e 100644 --- a/guides/source/4_1_release_notes.md +++ b/guides/source/4_1_release_notes.md @@ -348,6 +348,10 @@ for detailed changes. ActiveRecord will now translate aliased attribute names to the actual column name used in the database. ([Pull Request](https://github.com/rails/rails/pull/7839)) +* The ERB in fixture files is no longer evaluated in the context of the main + object. Helper methods used by multiple fixtures should be defined on modules + included in `ActiveRecord::FixtureSet.context_class`. ([Pull Request](https://github.com/rails/rails/pull/13022)) + Credits ------- diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md index ef5f6ac024..3fbc913d8b 100644 --- a/guides/source/upgrading_ruby_on_rails.md +++ b/guides/source/upgrading_ruby_on_rails.md @@ -27,6 +27,23 @@ Upgrading from Rails 4.0 to Rails 4.1 NOTE: This section is a work in progress. +### Methods defined in Active Record fixtures + +Rails 4.1 evaluates each fixture's ERB in a separate context, so helper methods +defined in a fixture will not be available in other fixtures. + +Helper methods that are used in multiple fixtures should be defined on modules +included in the newly introduced `ActiveRecord::FixtureSet.context_class`, in +`test_helper.rb`. + +```ruby +class FixtureFileHelpers + def file_sha(path) + Digest::SHA2.hexdigest(File.read(Rails.root.join('test/fixtures', path))) + end +end +ActiveRecord::FixtureSet.context_class.send :include, FixtureFileHelpers +``` Upgrading from Rails 3.2 to Rails 4.0 ------------------------------------- |