Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Fix some edge cases when the same template is called with different l…

…ocal assigns

Signed-off-by: Joshua Peek <josh@joshpeek.com>
  • Loading branch information...
commit 199e750d46c04970b5e7684998d09405648ecbd4 1 parent 1dab1d3
@pixeltrix pixeltrix authored josh committed
View
30 actionpack/lib/action_view/renderable.rb
@@ -16,8 +16,18 @@ def handler
memoize :handler
def compiled_source
+ @compiled_at = Time.now
handler.call(self)
end
+ memoize :compiled_source
+
+ def compiled_at
+ @compiled_at
+ end
+
+ def defined_at
+ @defined_at ||= {}
+ end
def method_name_without_locals
['_run', extension, method_segment].compact.join('_')
@@ -61,8 +71,12 @@ def method_name(local_assigns)
def compile(local_assigns)
render_symbol = method_name(local_assigns)
- if !Base::CompiledTemplates.method_defined?(render_symbol) || recompile?
+ if self.is_a?(InlineTemplate)
compile!(render_symbol, local_assigns)
+ else
+ if !Base::CompiledTemplates.method_defined?(render_symbol) || recompile?(render_symbol)
+ recompile!(render_symbol, local_assigns)
+ end
end
end
@@ -79,6 +93,7 @@ def #{render_symbol}(local_assigns)
begin
ActionView::Base::CompiledTemplates.module_eval(source, filename, 0)
+ defined_at[render_symbol] = Time.now if respond_to?(:reloadable?) && reloadable?
rescue Errno::ENOENT => e
raise e # Missing template file, re-raise for Base to rescue
rescue Exception => e # errors from template code
@@ -91,5 +106,18 @@ def #{render_symbol}(local_assigns)
raise ActionView::TemplateError.new(self, {}, e)
end
end
+
+ def recompile?(render_symbol)
+ !cached? || redefine?(render_symbol) || stale?
+ end
+
+ def recompile!(render_symbol, local_assigns)
+ compiled_source(:reload) if compiled_at.nil? || compiled_at < mtime
+ compile!(render_symbol, local_assigns)
+ end
+
+ def redefine?(render_symbol)
+ compiled_at && defined_at[render_symbol] && compiled_at > defined_at[render_symbol]
+ end
end
end
View
23 actionpack/lib/action_view/template.rb
@@ -7,7 +7,9 @@ class Path
def initialize(path)
raise ArgumentError, "path already is a Path class" if path.is_a?(Path)
@path = path.freeze
+ end
+ def load!
@paths = {}
templates_in_path do |template|
load_template(template)
@@ -44,6 +46,7 @@ def eql?(path)
# etc. A format must be supplied to match a formated file. +hello/index+
# will never match +hello/index.html.erb+.
def [](path)
+ load! if @paths.nil?
@paths[path] || find_template(path)
end
@@ -197,26 +200,22 @@ def render_template(view, local_assigns = {})
end
def stale?
- !frozen? && mtime < mtime(:reload)
+ reloadable? && (mtime < mtime(:reload))
end
def load!
reloadable? ? memoize_all : freeze
end
- private
- def cached?
- Base.cache_template_loading || ActionController::Base.allow_concurrency
- end
-
- def reloadable?
- !cached?
- end
+ def reloadable?
+ !(Base.cache_template_loading || ActionController::Base.allow_concurrency)
+ end
- def recompile?
- reloadable? ? stale? : false
- end
+ def cached?
+ ActionController::Base.perform_caching || !reloadable?
+ end
+ private
def valid_extension?(extension)
!Template.registered_template_handler(extension).nil?
end
View
86 actionpack/test/template/compiled_templates_test.rb
@@ -10,52 +10,64 @@ def setup
end
def test_template_gets_compiled
- assert_equal 0, @compiled_templates.instance_methods.size
- assert_equal "Hello world!", render(:file => "test/hello_world.erb")
- assert_equal 1, @compiled_templates.instance_methods.size
+ with_caching(true) do
+ assert_equal 0, @compiled_templates.instance_methods.size
+ assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+ assert_equal 1, @compiled_templates.instance_methods.size
+ end
end
def test_template_gets_recompiled_when_using_different_keys_in_local_assigns
- assert_equal 0, @compiled_templates.instance_methods.size
- assert_equal "Hello world!", render(:file => "test/hello_world.erb")
- assert_equal "Hello world!", render(:file => "test/hello_world.erb", :locals => {:foo => "bar"})
- assert_equal 2, @compiled_templates.instance_methods.size
+ with_caching(true) do
+ assert_equal 0, @compiled_templates.instance_methods.size
+ assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+ assert_equal "Hello world!", render(:file => "test/hello_world.erb", :locals => {:foo => "bar"})
+ assert_equal 2, @compiled_templates.instance_methods.size
+ end
end
def test_compiled_template_will_not_be_recompiled_when_rendered_with_identical_local_assigns
- assert_equal 0, @compiled_templates.instance_methods.size
- assert_equal "Hello world!", render(:file => "test/hello_world.erb")
- ActionView::Template.any_instance.expects(:compile!).never
- assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+ with_caching(true) do
+ assert_equal 0, @compiled_templates.instance_methods.size
+ assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+ ActionView::Template.any_instance.expects(:compile!).never
+ assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+ end
end
def test_compiled_template_will_always_be_recompiled_when_template_is_not_cached
- ActionView::Template.any_instance.expects(:recompile?).times(3).returns(true)
- assert_equal 0, @compiled_templates.instance_methods.size
- assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb")
- ActionView::Template.any_instance.expects(:compile!).times(3)
- 3.times { assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb") }
- assert_equal 1, @compiled_templates.instance_methods.size
+ with_caching(false) do
+ ActionView::Template.any_instance.expects(:recompile?).times(3).returns(true)
+ assert_equal 0, @compiled_templates.instance_methods.size
+ assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb")
+ ActionView::Template.any_instance.expects(:compile!).times(3)
+ 3.times { assert_equal "Hello world!", render(:file => "#{FIXTURE_LOAD_PATH}/test/hello_world.erb") }
+ assert_equal 1, @compiled_templates.instance_methods.size
+ end
end
- def test_template_changes_are_not_reflected_with_cached_templates
+ def test_template_changes_are_not_reflected_with_cached_template_loading
with_caching(true) do
- assert_equal "Hello world!", render(:file => "test/hello_world.erb")
- modify_template "test/hello_world.erb", "Goodbye world!" do
+ with_reloading(false) do
+ assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+ modify_template "test/hello_world.erb", "Goodbye world!" do
+ assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+ end
assert_equal "Hello world!", render(:file => "test/hello_world.erb")
end
- assert_equal "Hello world!", render(:file => "test/hello_world.erb")
end
end
- def test_template_changes_are_reflected_without_cached_templates
- with_caching(false) do
- assert_equal "Hello world!", render(:file => "test/hello_world.erb")
- modify_template "test/hello_world.erb", "Goodbye world!" do
- assert_equal "Goodbye world!", render(:file => "test/hello_world.erb")
- sleep(1) # Need to sleep so that the timestamp actually changes
+ def test_template_changes_are_reflected_without_cached_template_loading
+ with_caching(true) do
+ with_reloading(true) do
+ assert_equal "Hello world!", render(:file => "test/hello_world.erb")
+ modify_template "test/hello_world.erb", "Goodbye world!" do
+ assert_equal "Goodbye world!", render(:file => "test/hello_world.erb")
+ sleep(1) # Need to sleep so that the timestamp actually changes
+ end
+ assert_equal "Hello world!", render(:file => "test/hello_world.erb")
end
- assert_equal "Hello world!", render(:file => "test/hello_world.erb")
end
end
@@ -77,13 +89,23 @@ def modify_template(template, content)
end
end
- def with_caching(caching_enabled)
- old_caching_enabled = ActionView::Base.cache_template_loading
+ def with_caching(perform_caching)
+ old_perform_caching = ActionController::Base.perform_caching
+ begin
+ ActionController::Base.perform_caching = perform_caching
+ yield
+ ensure
+ ActionController::Base.perform_caching = old_perform_caching
+ end
+ end
+
+ def with_reloading(reload_templates)
+ old_cache_template_loading = ActionView::Base.cache_template_loading
begin
- ActionView::Base.cache_template_loading = caching_enabled
+ ActionView::Base.cache_template_loading = !reload_templates
yield
ensure
- ActionView::Base.cache_template_loading = old_caching_enabled
+ ActionView::Base.cache_template_loading = old_cache_template_loading
end
end
end
View
6 railties/lib/initializer.rb
@@ -369,10 +369,8 @@ def load_observers
def load_view_paths
if configuration.frameworks.include?(:action_view)
- if configuration.cache_classes
- ActionController::Base.view_paths = configuration.view_path if configuration.frameworks.include?(:action_controller)
- ActionMailer::Base.template_root = configuration.view_path if configuration.frameworks.include?(:action_mailer)
- end
+ ActionController::Base.view_paths.each { |path| path.load! } if configuration.frameworks.include?(:action_controller)
+ ActionMailer::Base.template_root.load! if configuration.frameworks.include?(:action_mailer)
end
end

5 comments on commit 199e750

@mattly

- def test_template_changes_are_reflected_without_cached_templates 0 - with_caching(false) do 0 - assert_equal “Hello world!”, render(:file => “test/hello_world.erb”) 0 - modify_template “test/hello_world.erb”, “Goodbye world!” do 0 - assert_equal “Goodbye world!”, render(:file => “test/hello_world.erb”) 0 - sleep(1) # Need to sleep so that the timestamp actually changes 0 + def test_template_changes_are_reflected_without_cached_template_loading 0 + with_caching(true) do

um, are you sure you wanted to turn caching on there? Templates are no longer reloading in development mode.

@mattly

gah, compiled_templates_test_diff, line 51-62 are reflected there.

@pixeltrix
Owner

If you’re referring to the the test test_template_changes_are_reflected_without_cached_template_loading then with_caching(true) sets AC::Base.perform_caching = true and with_reloading(true) set AV::Base.cache_template_loading = false. This simulates template reloading in production mode.

@mattly

perhaps it’s supposed to simulate that in production mode; the tests have changed quite a bit and I don’t find the intention of the new tests as clear as the intention of the old tests.

However, in development mode against 1dab1d380377f1a2a60da43bc22989d55632d246 template reloading works; in development mode against this commit it doesn’t.

@masterkain

I can confirm, templates doesn’t reload in dev mode.

Please sign in to comment.
Something went wrong with that request. Please try again.