Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Clean up view path cruft and split path implementations into Template…

…::Path and Template::EagerPath
  • Loading branch information...
commit 558ab327b733717f4a8de3ed62b8dcd62e9ff9c3 1 parent 490c26c
@josh josh authored
View
1  actionmailer/test/abstract_unit.rb
@@ -17,7 +17,6 @@
FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures')
ActionMailer::Base.template_root = FIXTURE_LOAD_PATH
-ActionMailer::Base.template_root.load
class MockSMTP
def self.deliveries
View
12 actionpack/lib/action_controller/base.rb
@@ -869,7 +869,7 @@ def render(options = nil, extra_options = {}, &block) #:doc:
validate_render_arguments(options, extra_options, block_given?)
if options.nil?
- options = { :template => default_template.filename, :layout => true }
+ options = { :template => default_template, :layout => true }
elsif options == :update
options = extra_options.merge({ :update => true })
elsif options.is_a?(String) || options.is_a?(Symbol)
@@ -1125,7 +1125,7 @@ def stale?(options)
end
# Sets the etag, last_modified, or both on the response and renders a
- # "304 Not Modified" response if the request is already fresh.
+ # "304 Not Modified" response if the request is already fresh.
#
# Example:
#
@@ -1133,8 +1133,8 @@ def stale?(options)
# @article = Article.find(params[:id])
# fresh_when(:etag => @article, :last_modified => @article.created_at.utc)
# end
- #
- # This will render the show template if the request isn't sending a matching etag or
+ #
+ # This will render the show template if the request isn't sending a matching etag or
# If-Modified-Since header and just a "304 Not Modified" response if there's a match.
def fresh_when(options)
options.assert_valid_keys(:etag, :last_modified)
@@ -1239,7 +1239,7 @@ def log_processing
log_processing_for_parameters
end
end
-
+
def log_processing_for_request_id
request_id = "\n\nProcessing #{self.class.name}\##{action_name} "
request_id << "to #{params[:format]} " if params[:format]
@@ -1251,7 +1251,7 @@ def log_processing_for_request_id
def log_processing_for_parameters
parameters = respond_to?(:filter_parameters) ? filter_parameters(params) : params.dup
parameters = parameters.except!(:controller, :action, :format, :_method)
-
+
logger.info " Parameters: #{parameters.inspect}" unless parameters.empty?
end
View
4 actionpack/lib/action_controller/rescue.rb
@@ -38,8 +38,8 @@ module Rescue
'ActionView::TemplateError' => 'template_error'
}
- RESCUES_TEMPLATE_PATH = ActionView::PathSet::Path.new(
- File.join(File.dirname(__FILE__), "templates"), true)
+ RESCUES_TEMPLATE_PATH = ActionView::Template::EagerPath.new(
+ File.join(File.dirname(__FILE__), "templates"))
def self.included(base) #:nodoc:
base.cattr_accessor :rescue_responses
View
2  actionpack/lib/action_view/base.rb
@@ -225,7 +225,7 @@ def view_paths=(paths)
end
# Returns the result of a render that's dictated by the options hash. The primary options are:
- #
+ #
# * <tt>:partial</tt> - See ActionView::Partials.
# * <tt>:update</tt> - Calls update_page with the block given.
# * <tt>:file</tt> - Renders an explicit template file (this used to be the old default), add :locals to pass in those.
View
2  actionpack/lib/action_view/inline_template.rb
@@ -12,7 +12,7 @@ def initialize(source, type = nil)
private
# Always recompile inline templates
- def recompile?(local_assigns)
+ def recompile?
true
end
end
View
107 actionpack/lib/action_view/paths.rb
@@ -2,7 +2,7 @@ module ActionView #:nodoc:
class PathSet < Array #:nodoc:
def self.type_cast(obj)
if obj.is_a?(String)
- Path.new(obj)
+ Template::EagerPath.new(obj)
else
obj
end
@@ -32,111 +32,6 @@ def unshift(*objs)
super(*objs.map { |obj| self.class.type_cast(obj) })
end
- class Path #:nodoc:
- attr_reader :path, :paths
- delegate :hash, :inspect, :to => :path
-
- def initialize(path, load = false)
- raise ArgumentError, "path already is a Path class" if path.is_a?(Path)
- @path = path.freeze
- reload! if load
- end
-
- def to_s
- if defined?(RAILS_ROOT)
- path.to_s.sub(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}\//, '')
- else
- path.to_s
- end
- end
-
- def to_str
- path.to_str
- end
-
- def ==(path)
- to_str == path.to_str
- end
-
- def eql?(path)
- to_str == path.to_str
- end
-
- # Returns a ActionView::Template object for the given path string. The
- # input path should be relative to the view path directory,
- # +hello/index.html.erb+. This method also has a special exception to
- # match partial file names without a handler extension. So
- # +hello/index.html+ will match the first template it finds with a
- # known template extension, +hello/index.html.erb+. Template extensions
- # should not be confused with format extensions +html+, +js+, +xml+,
- # etc. A format must be supplied to match a formated file. +hello/index+
- # will never match +hello/index.html.erb+.
- #
- # This method also has two different implementations, one that is "lazy"
- # and makes file system calls every time and the other is cached,
- # "eager" which looks up the template in an in memory index. The "lazy"
- # version is designed for development where you want to automatically
- # find new templates between requests. The "eager" version is designed
- # for production mode and it is much faster but requires more time
- # upfront to build the file index.
- def [](path)
- if loaded?
- @paths[path]
- else
- Dir.glob("#{@path}/#{path}*").each do |file|
- template = create_template(file)
- if template.accessible_paths.include?(path)
- return template
- end
- end
- nil
- end
- end
-
- def loaded?
- @loaded ? true : false
- end
-
- def load
- reload! unless loaded?
- self
- end
-
- # Rebuild load path directory cache
- def reload!
- @paths = {}
-
- templates_in_path do |template|
- template.load!
- template.accessible_paths.each do |path|
- @paths[path] = template
- end
- end
-
- @paths.freeze
- @loaded = true
- end
-
- private
- def templates_in_path
- (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**")).each do |file|
- yield create_template(file) unless File.directory?(file)
- end
- end
-
- def create_template(file)
- Template.new(file.split("#{self}/").last, self)
- end
- end
-
- def load
- each { |path| path.load }
- end
-
- def reload!
- each { |path| path.reload! }
- end
-
def find_template(original_template_path, format = nil)
return original_template_path if original_template_path.respond_to?(:render)
template_path = original_template_path.sub(/^\//, '')
View
9 actionpack/lib/action_view/renderable.rb
@@ -60,7 +60,7 @@ def method_name(local_assigns)
def compile(local_assigns)
render_symbol = method_name(local_assigns)
- if recompile?(render_symbol)
+ if !Base::CompiledTemplates.method_defined?(render_symbol) || recompile?
compile!(render_symbol, local_assigns)
end
end
@@ -89,11 +89,8 @@ def #{render_symbol}(local_assigns)
end
end
- # Method to check whether template compilation is necessary.
- # The template will be compiled if the file has not been compiled yet, or
- # if local_assigns has a new key, which isn't supported by the compiled code yet.
- def recompile?(symbol)
- !Base::CompiledTemplates.method_defined?(symbol) || !loaded?
+ def recompile?
+ false
end
end
end
View
85 actionpack/lib/action_view/template.rb
@@ -1,5 +1,83 @@
module ActionView #:nodoc:
class Template
+ class Path
+ attr_reader :path, :paths
+ delegate :hash, :inspect, :to => :path
+
+ def initialize(path)
+ raise ArgumentError, "path already is a Path class" if path.is_a?(Path)
+ @path = path.freeze
+ end
+
+ def to_s
+ if defined?(RAILS_ROOT)
+ path.to_s.sub(/^#{Regexp.escape(File.expand_path(RAILS_ROOT))}\//, '')
+ else
+ path.to_s
+ end
+ end
+
+ def to_str
+ path.to_str
+ end
+
+ def ==(path)
+ to_str == path.to_str
+ end
+
+ def eql?(path)
+ to_str == path.to_str
+ end
+
+ # Returns a ActionView::Template object for the given path string. The
+ # input path should be relative to the view path directory,
+ # +hello/index.html.erb+. This method also has a special exception to
+ # match partial file names without a handler extension. So
+ # +hello/index.html+ will match the first template it finds with a
+ # known template extension, +hello/index.html.erb+. Template extensions
+ # should not be confused with format extensions +html+, +js+, +xml+,
+ # etc. A format must be supplied to match a formated file. +hello/index+
+ # will never match +hello/index.html.erb+.
+ def [](path)
+ templates_in_path do |template|
+ if template.accessible_paths.include?(path)
+ return template
+ end
+ end
+ nil
+ end
+
+ private
+ def templates_in_path
+ (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**")).each do |file|
+ yield create_template(file) unless File.directory?(file)
+ end
+ end
+
+ def create_template(file)
+ Template.new(file.split("#{self}/").last, self)
+ end
+ end
+
+ class EagerPath < Path
+ def initialize(path)
+ super
+
+ @paths = {}
+ templates_in_path do |template|
+ template.load!
+ template.accessible_paths.each do |path|
+ @paths[path] = template
+ end
+ end
+ @paths.freeze
+ end
+
+ def [](path)
+ @paths[path]
+ end
+ end
+
extend TemplateHandlers
extend ActiveSupport::Memoizable
include Renderable
@@ -115,13 +193,12 @@ def stale?
File.mtime(filename) > mtime
end
- def loaded?
- @loaded
+ def recompile?
+ !@cached
end
def load!
- @loaded = true
- compile({})
+ @cached = true
freeze
end
View
1  actionpack/test/abstract_unit.rb
@@ -34,7 +34,6 @@
FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures')
ActionController::Base.view_paths = FIXTURE_LOAD_PATH
-ActionController::Base.view_paths.load
def uses_mocha(test_name)
yield
View
9 actionpack/test/template/compiled_templates_test.rb
@@ -31,7 +31,7 @@ def test_compiled_template_will_not_be_recompiled_when_rendered_with_identical_l
end
def test_compiled_template_will_always_be_recompiled_when_template_is_not_cached
- ActionView::Template.any_instance.expects(:loaded?).times(3).returns(false)
+ 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)
@@ -62,13 +62,14 @@ def render(*args)
def render_with_cache(*args)
view_paths = ActionController::Base.view_paths
- assert view_paths.first.loaded?
+ assert_equal ActionView::Template::EagerPath, view_paths.first.class
ActionView::Base.new(view_paths, {}).render(*args)
end
def render_without_cache(*args)
- view_paths = ActionView::Base.process_view_paths(FIXTURE_LOAD_PATH)
- assert !view_paths.first.loaded?
+ path = ActionView::Template::Path.new(FIXTURE_LOAD_PATH)
+ view_paths = ActionView::Base.process_view_paths(path)
+ assert_equal ActionView::Template::Path, view_paths.first.class
ActionView::Base.new(view_paths, {}).render(*args)
end
View
7 actionpack/test/template/render_test.rb
@@ -197,7 +197,7 @@ class CachedViewRenderTest < Test::Unit::TestCase
# Ensure view path cache is primed
def setup
view_paths = ActionController::Base.view_paths
- assert view_paths.first.loaded?
+ assert_equal ActionView::Template::EagerPath, view_paths.first.class
setup_view(view_paths)
end
end
@@ -208,8 +208,9 @@ class LazyViewRenderTest < Test::Unit::TestCase
# Test the same thing as above, but make sure the view path
# is not eager loaded
def setup
- view_paths = ActionView::Base.process_view_paths(FIXTURE_LOAD_PATH)
- assert !view_paths.first.loaded?
+ path = ActionView::Template::Path.new(FIXTURE_LOAD_PATH)
+ view_paths = ActionView::Base.process_view_paths(path)
+ assert_equal ActionView::Template::Path, view_paths.first.class
setup_view(view_paths)
end
end
View
7 railties/lib/initializer.rb
@@ -370,8 +370,9 @@ def load_observers
def load_view_paths
if configuration.frameworks.include?(:action_view)
if configuration.cache_classes
- ActionController::Base.view_paths.load if configuration.frameworks.include?(:action_controller)
- ActionMailer::Base.template_root.load if configuration.frameworks.include?(:action_mailer)
+ view_path = ActionView::Template::EagerPath.new(configuration.view_path)
+ ActionController::Base.view_paths = view_path if configuration.frameworks.include?(:action_controller)
+ ActionMailer::Base.template_root = view_path if configuration.frameworks.include?(:action_mailer)
end
end
end
@@ -473,7 +474,7 @@ def initialize_framework_logging
# set to use Configuration#view_path.
def initialize_framework_views
if configuration.frameworks.include?(:action_view)
- view_path = ActionView::PathSet::Path.new(configuration.view_path, false)
+ view_path = ActionView::Template::Path.new(configuration.view_path)
ActionMailer::Base.template_root ||= view_path if configuration.frameworks.include?(:action_mailer)
ActionController::Base.view_paths = view_path if configuration.frameworks.include?(:action_controller) && ActionController::Base.view_paths.empty?
end

3 comments on commit 558ab32

@rsanheim

more action pack refactoring? not that it probably isn’t needed, it just feels like this area of the framework has been in constant churn since 2.0.

@josh
Collaborator

blame wycats! :p

@stephankaag

This commit is causing troubles with engines.

http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/1712-template-is-missing-error-for-engine-in-production-environment#ticket-1712-5

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