aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorVictor Costan <costan@gmail.com>2013-11-24 12:52:53 -0500
committerVictor Costan <costan@gmail.com>2013-12-03 02:52:26 -0500
commitddf27acbc285a892842866cde04951cdad52c5c9 (patch)
tree9497d0ae23ce1217adb5d21143587c54825dc564
parentabc1e5831cae724cfcdf524f62abb71be02d7e86 (diff)
downloadrails-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.md6
-rw-r--r--activerecord/lib/active_record/fixture_set/file.rb3
-rw-r--r--activerecord/lib/active_record/fixtures.rb32
-rw-r--r--activerecord/test/cases/fixture_set/file_test.rb55
-rw-r--r--guides/source/4_1_release_notes.md4
-rw-r--r--guides/source/upgrading_ruby_on_rails.md17
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
-------------------------------------