From bec4b69a3b65c3696edad3c880207e8c476b0937 Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Sun, 15 Jun 2008 21:22:27 -0500 Subject: [PATCH] Replaced TemplateFinder abstraction with ViewLoadPaths --- actionmailer/lib/action_mailer/base.rb | 5 +- actionmailer/test/mail_service_test.rb | 6 +- actionpack/CHANGELOG | 2 + actionpack/lib/action_controller/base.rb | 24 ++- .../lib/action_controller/dispatcher.rb | 2 +- actionpack/lib/action_controller/layout.rb | 2 +- actionpack/lib/action_view.rb | 3 +- actionpack/lib/action_view/base.rb | 105 +++++++++---- actionpack/lib/action_view/inline_template.rb | 9 +- .../lib/action_view/partial_template.rb | 2 +- actionpack/lib/action_view/template.rb | 44 +++--- actionpack/lib/action_view/template_file.rb | 88 +++++++++++ actionpack/lib/action_view/template_finder.rb | 147 ------------------ .../lib/action_view/template_handlers.rb | 1 - actionpack/lib/action_view/view_load_paths.rb | 103 ++++++++++++ actionpack/test/controller/view_paths_test.rb | 111 ++++++++----- .../test/template/template_file_test.rb | 95 +++++++++++ .../test/template/template_finder_test.rb | 60 ------- .../test/template/template_object_test.rb | 37 ++--- 19 files changed, 494 insertions(+), 352 deletions(-) create mode 100644 actionpack/lib/action_view/template_file.rb delete mode 100644 actionpack/lib/action_view/template_finder.rb create mode 100644 actionpack/lib/action_view/view_load_paths.rb create mode 100644 actionpack/test/template/template_file_test.rb delete mode 100644 actionpack/test/template/template_finder_test.rb diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 51fc6032cc5dc..1518e23dfedf1 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -426,8 +426,7 @@ def register_template_extension(extension) end def template_root=(root) - write_inheritable_attribute(:template_root, root) - ActionView::TemplateFinder.process_view_paths(root) + write_inheritable_attribute(:template_root, ActionView::ViewLoadPaths.new(Array(root))) end end @@ -547,7 +546,7 @@ def template_path end def initialize_template_class(assigns) - ActionView::Base.new([template_root], assigns, self) + ActionView::Base.new(template_root, assigns, self) end def sort_parts(parts, order = []) diff --git a/actionmailer/test/mail_service_test.rb b/actionmailer/test/mail_service_test.rb index e5ecb0e254851..7f4a8817cae3e 100755 --- a/actionmailer/test/mail_service_test.rb +++ b/actionmailer/test/mail_service_test.rb @@ -942,13 +942,13 @@ def test_return_path_with_deliver class InheritableTemplateRootTest < Test::Unit::TestCase def test_attr expected = "#{File.dirname(__FILE__)}/fixtures/path.with.dots" - assert_equal expected, FunkyPathMailer.template_root + assert_equal [expected], FunkyPathMailer.template_root.map(&:to_s) sub = Class.new(FunkyPathMailer) sub.template_root = 'test/path' - assert_equal 'test/path', sub.template_root - assert_equal expected, FunkyPathMailer.template_root + assert_equal ['test/path'], sub.template_root.map(&:to_s) + assert_equal [expected], FunkyPathMailer.template_root.map(&:to_s) end end diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 3e977e1519519..4a6e70a153def 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Replaced TemplateFinder abstraction with ViewLoadPaths [Josh Peek] + * Added block-call style to link_to [Sam Stephenson/DHH]. Example: <% link_to(@profile) do %> diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index de1a2662a6aaa..bf34edcd855e4 100755 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -421,8 +421,7 @@ def view_paths end def view_paths=(value) - @view_paths = value - ActionView::TemplateFinder.process_view_paths(value) + @view_paths = ActionView::ViewLoadPaths.new(Array(value)) if value end # Adds a view_path to the front of the view_paths array. @@ -434,8 +433,7 @@ def view_paths=(value) # def prepend_view_path(path) @view_paths = superclass.view_paths.dup if @view_paths.nil? - view_paths.unshift(*path) - ActionView::TemplateFinder.process_view_paths(path) + @view_paths.unshift(*path) end # Adds a view_path to the end of the view_paths array. @@ -447,8 +445,7 @@ def prepend_view_path(path) # def append_view_path(path) @view_paths = superclass.view_paths.dup if @view_paths.nil? - view_paths.push(*path) - ActionView::TemplateFinder.process_view_paths(path) + @view_paths.push(*path) end # Replace sensitive parameter data from the request log. @@ -640,11 +637,11 @@ def session_enabled? # View load paths for controller. def view_paths - @template.finder.view_paths + @template.view_paths end def view_paths=(value) - @template.finder.view_paths = value # Mutex needed + @template.view_paths = ViewLoadPaths.new(value) end # Adds a view_path to the front of the view_paths array. @@ -654,7 +651,7 @@ def view_paths=(value) # self.prepend_view_path(["views/default", "views/custom"]) # def prepend_view_path(path) - @template.finder.prepend_view_path(path) # Mutex needed + @template.view_paths.unshift(*path) end # Adds a view_path to the end of the view_paths array. @@ -664,7 +661,7 @@ def prepend_view_path(path) # self.append_view_path(["views/default", "views/custom"]) # def append_view_path(path) - @template.finder.append_view_path(path) # Mutex needed + @template.view_paths.push(*path) end protected @@ -1225,7 +1222,7 @@ def close_session end def template_exists?(template_name = default_template_name) - @template.finder.file_exists?(template_name) + @template.file_exists?(template_name) end def template_public?(template_name = default_template_name) @@ -1233,9 +1230,8 @@ def template_public?(template_name = default_template_name) end def template_exempt_from_layout?(template_name = default_template_name) - extension = @template && @template.finder.pick_template_extension(template_name) - name_with_extension = !template_name.include?('.') && extension ? "#{template_name}.#{extension}" : template_name - @@exempt_from_layout.any? { |ext| name_with_extension =~ ext } + template_name = @template.send(:template_file_from_name, template_name) if @template + @@exempt_from_layout.any? { |ext| template_name.to_s =~ ext } end def default_template_name(action_name = self.action_name) diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index fe4f6b4a7ea29..7df987d525534 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -134,7 +134,7 @@ def reload_application run_callbacks :prepare_dispatch Routing::Routes.reload - ActionView::TemplateFinder.reload! unless ActionView::Base.cache_template_loading + ActionController::Base.view_paths.reload! end # Cleanup the application by clearing out loaded classes so they can diff --git a/actionpack/lib/action_controller/layout.rb b/actionpack/lib/action_controller/layout.rb index b5b59f2d7c2c6..0721f71498efe 100644 --- a/actionpack/lib/action_controller/layout.rb +++ b/actionpack/lib/action_controller/layout.rb @@ -304,7 +304,7 @@ def action_has_layout? end def layout_directory?(layout_name) - @template.finder.find_template_extension_from_handler(File.join('layouts', layout_name)) + @template.view_paths.find_template_file_for_path("#{File.join('layouts', layout_name)}.#{@template.template_format}.erb") ? true : false end end end diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 2f6894a8f93a6..973020a7682b5 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -22,7 +22,8 @@ #++ require 'action_view/template_handlers' -require 'action_view/template_finder' +require 'action_view/template_file' +require 'action_view/view_load_paths' require 'action_view/template' require 'action_view/partial_template' require 'action_view/inline_template' diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index c417cc07acbf6..4f3cc46a14c3f 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -1,17 +1,17 @@ module ActionView #:nodoc: class ActionViewError < StandardError #:nodoc: end - + class MissingTemplate < ActionViewError #:nodoc: end - # Action View templates can be written in three ways. If the template file has a .erb (or .rhtml) extension then it uses a mixture of ERb - # (included in Ruby) and HTML. If the template file has a .builder (or .rxml) extension then Jim Weirich's Builder::XmlMarkup library is used. + # Action View templates can be written in three ways. If the template file has a .erb (or .rhtml) extension then it uses a mixture of ERb + # (included in Ruby) and HTML. If the template file has a .builder (or .rxml) extension then Jim Weirich's Builder::XmlMarkup library is used. # If the template file has a .rjs extension then it will use ActionView::Helpers::PrototypeHelper::JavaScriptGenerator. - # + # # = ERb - # - # You trigger ERb by using embeddings such as <% %>, <% -%>, and <%= %>. The <%= %> tag set is used when you want output. Consider the + # + # You trigger ERb by using embeddings such as <% %>, <% -%>, and <%= %>. The <%= %> tag set is used when you want output. Consider the # following loop for names: # # Names of all the people @@ -51,7 +51,7 @@ class MissingTemplate < ActionViewError #:nodoc: # <%= @page_title %> # # == Passing local variables to sub templates - # + # # You can pass local variables to sub templates by using a hash with the variable names as keys and the objects as values: # # <%= render "shared/header", { :headline => "Welcome", :person => person } %> @@ -77,8 +77,8 @@ class MissingTemplate < ActionViewError #:nodoc: # # == Builder # - # Builder templates are a more programmatic alternative to ERb. They are especially useful for generating XML content. An XmlMarkup object - # named +xml+ is automatically made available to templates with a .builder extension. + # Builder templates are a more programmatic alternative to ERb. They are especially useful for generating XML content. An XmlMarkup object + # named +xml+ is automatically made available to templates with a .builder extension. # # Here are some basic examples: # @@ -87,7 +87,7 @@ class MissingTemplate < ActionViewError #:nodoc: # xml.a("A Link", "href"=>"http://onestepback.org") # => A Link # xml.target("name"=>"compile", "option"=>"fast") # => # # NOTE: order of attributes is not specified. - # + # # Any method with a block will be treated as an XML markup tag with nested markup in the block. For example, the following: # # xml.div { @@ -111,7 +111,7 @@ class MissingTemplate < ActionViewError #:nodoc: # xml.description "Basecamp: Recent items" # xml.language "en-us" # xml.ttl "40" - # + # # for item in @recent_items # xml.item do # xml.title(item_title(item)) @@ -119,7 +119,7 @@ class MissingTemplate < ActionViewError #:nodoc: # xml.pubDate(item_pubDate(item)) # xml.guid(@person.firm.account.url + @recent_items.url(item)) # xml.link(@person.firm.account.url + @recent_items.url(item)) - # + # # xml.tag!("dc:creator", item.author_name) if item_has_creator?(item) # end # end @@ -130,12 +130,12 @@ class MissingTemplate < ActionViewError #:nodoc: # # == JavaScriptGenerator # - # JavaScriptGenerator templates end in .rjs. Unlike conventional templates which are used to - # render the results of an action, these templates generate instructions on how to modify an already rendered page. This makes it easy to - # modify multiple elements on your page in one declarative Ajax response. Actions with these templates are called in the background with Ajax + # JavaScriptGenerator templates end in .rjs. Unlike conventional templates which are used to + # render the results of an action, these templates generate instructions on how to modify an already rendered page. This makes it easy to + # modify multiple elements on your page in one declarative Ajax response. Actions with these templates are called in the background with Ajax # and make updates to the page where the request originated from. - # - # An instance of the JavaScriptGenerator object named +page+ is automatically made available to your template, which is implicitly wrapped in an ActionView::Helpers::PrototypeHelper#update_page block. + # + # An instance of the JavaScriptGenerator object named +page+ is automatically made available to your template, which is implicitly wrapped in an ActionView::Helpers::PrototypeHelper#update_page block. # # When an .rjs action is called with +link_to_remote+, the generated JavaScript is automatically evaluated. Example: # @@ -145,15 +145,14 @@ class MissingTemplate < ActionViewError #:nodoc: # # page.replace_html 'sidebar', :partial => 'sidebar' # page.remove "person-#{@person.id}" - # page.visual_effect :highlight, 'user-list' + # page.visual_effect :highlight, 'user-list' # # This refreshes the sidebar, removes a person element and highlights the user list. - # + # # See the ActionView::Helpers::PrototypeHelper::GeneratorMethods documentation for more details. class Base include ERB::Util - attr_reader :finder attr_accessor :base_path, :assigns, :template_extension, :first_render attr_accessor :controller @@ -170,14 +169,14 @@ class Base # Specify whether file modification times should be checked to see if a template needs recompilation @@cache_template_loading = false cattr_accessor :cache_template_loading - + def self.cache_template_extensions=(*args) ActiveSupport::Deprecation.warn("config.action_view.cache_template_extensions option has been deprecated and has no affect. " << "Please remove it from your config files.", caller) end # Specify whether RJS responses should be wrapped in a try/catch block - # that alert()s the caught exception (and then re-raises it). + # that alert()s the caught exception (and then re-raises it). @@debug_rjs = false cattr_accessor :debug_rjs @@ -185,7 +184,7 @@ def self.cache_template_extensions=(*args) delegate :request_forgery_protection_token, :template, :params, :session, :cookies, :response, :headers, :flash, :logger, :action_name, :controller_name, :to => :controller - + module CompiledTemplates #:nodoc: # holds compiled template code end @@ -221,11 +220,17 @@ def initialize(view_paths = [], assigns_for_first_render = {}, controller = nil) @assigns = assigns_for_first_render @assigns_added = nil @controller = controller - @finder = TemplateFinder.new(self, view_paths) + self.view_paths = view_paths end - # Renders the template present at template_path. If use_full_path is set to true, - # it's relative to the view_paths array, otherwise it's absolute. The hash in local_assigns + attr_reader :view_paths + + def view_paths=(paths) + @view_paths = ViewLoadPaths.new(Array(paths)) + end + + # Renders the template present at template_path. If use_full_path is set to true, + # it's relative to the view_paths array, otherwise it's absolute. The hash in local_assigns # is made available as local variables. def render_file(template_path, use_full_path = true, local_assigns = {}) #:nodoc: if defined?(ActionMailer) && defined?(ActionMailer::Base) && controller.is_a?(ActionMailer::Base) && !template_path.include?("/") @@ -240,11 +245,11 @@ def render_file(template_path, use_full_path = true, local_assigns = {}) #:nodoc render :partial => 'signup' # no mailer_name necessary END_ERROR end - + Template.new(self, template_path, use_full_path, local_assigns).render_template end - - # Renders the template present at template_path (relative to the view_paths array). + + # Renders the template present at template_path (relative to the view_paths array). # The hash in local_assigns is made available as local variables. def render(options = {}, local_assigns = {}, &block) #:nodoc: if options.is_a?(String) @@ -257,7 +262,7 @@ def render(options = {}, local_assigns = {}, &block) #:nodoc: if partial_layout = options.delete(:layout) if block_given? - wrap_content_for_layout capture(&block) do + wrap_content_for_layout capture(&block) do concat(render(options.merge(:partial => partial_layout))) end else @@ -314,6 +319,10 @@ def template_format end end + def file_exists?(template_path) + view_paths.template_exists?(template_file_from_name(template_path)) + end + private def wrap_content_for_layout(content) original_content_for_layout, @content_for_layout = @content_for_layout, content @@ -334,11 +343,43 @@ def evaluate_assigns def assign_variables_from_controller @assigns.each { |key, value| instance_variable_set("@#{key}", value) } end - + def execute(template) send(template.method, template.locals) do |*names| instance_variable_get "@content_for_#{names.first || 'layout'}" - end + end + end + + def template_file_from_name(template_name) + template_name = TemplateFile.from_path(template_name) + pick_template_extension(template_name) unless template_name.extension + end + + # Gets the extension for an existing template with the given template_path. + # Returns the format with the extension if that template exists. + # + # pick_template_extension('users/show') + # # => 'html.erb' + # + # pick_template_extension('users/legacy') + # # => "rhtml" + # + def pick_template_extension(file) + if f = self.view_paths.find_template_file_for_path(file.dup_with_extension(template_format)) || file_from_first_render(file) + f + elsif template_format == :js && f = self.view_paths.find_template_file_for_path(file.dup_with_extension(:html)) + @template_format = :html + f + else + nil + end + end + + # Determine the template extension from the @first_render filename + def file_from_first_render(file) + if extension = File.basename(@first_render.to_s)[/^[^.]+\.(.+)$/, 1] + file.dup_with_extension(extension) + end end end end diff --git a/actionpack/lib/action_view/inline_template.rb b/actionpack/lib/action_view/inline_template.rb index 87c012d18168c..fd0ad483020b4 100644 --- a/actionpack/lib/action_view/inline_template.rb +++ b/actionpack/lib/action_view/inline_template.rb @@ -1,20 +1,17 @@ module ActionView #:nodoc: class InlineTemplate < Template #:nodoc: - def initialize(view, source, locals = {}, type = nil) @view = view - @finder = @view.finder - + @source = source @extension = type @locals = locals || {} - + @handler = self.class.handler_class_for_extension(@extension).new(@view) end - + def method_key @source end - end end diff --git a/actionpack/lib/action_view/partial_template.rb b/actionpack/lib/action_view/partial_template.rb index b271a73388cd6..0cf996ca0464a 100644 --- a/actionpack/lib/action_view/partial_template.rb +++ b/actionpack/lib/action_view/partial_template.rb @@ -16,7 +16,7 @@ def initialize(view, partial_path, object = nil, locals = {}) end def render - ActionController::Base.benchmark("Rendered #{@path}", Logger::DEBUG, false) do + ActionController::Base.benchmark("Rendered #{@path.path_without_format_and_extension}", Logger::DEBUG, false) do @handler.render(self) end end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 500ff713bb30c..4c3f252c10035 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -3,15 +3,15 @@ class Template #:nodoc: extend TemplateHandlers attr_accessor :locals - attr_reader :handler, :path, :extension, :filename, :path_without_extension, :method + attr_reader :handler, :path, :extension, :filename, :method def initialize(view, path, use_full_path, locals = {}) @view = view - @finder = @view.finder + @paths = view.view_paths - # Clear the forward slash at the beginning if exists - @path = use_full_path ? path.sub(/^\//, '') : path - @view.first_render ||= @path + @original_path = path + @path = TemplateFile.from_path(path, !use_full_path) + @view.first_render ||= @path.to_s @source = nil # Don't read the source until we know that it is required set_extension_and_file_name(use_full_path) @@ -36,6 +36,10 @@ def render @handler.render(self) end + def path_without_extension + @path.path_without_extension + end + def source @source ||= File.read(self.filename) end @@ -45,7 +49,7 @@ def method_key end def base_path_for_exception - @finder.find_base_path_for("#{@path_without_extension}.#{@extension}") || @finder.view_paths.first + (@paths.find_load_path_for_path(@path) || @paths.first).to_s end def prepare! @@ -60,28 +64,30 @@ def prepare! private def set_extension_and_file_name(use_full_path) - @path_without_extension, @extension = @finder.path_and_extension(@path) + @extension = @path.extension + if use_full_path - if @extension - @filename = @finder.pick_template(@path_without_extension, @extension) - else - @extension = @finder.pick_template_extension(@path).to_s - raise_missing_template_exception unless @extension - - @filename = @finder.pick_template(@path, @extension) - @extension = @extension.gsub(/^.+\./, '') # strip off any formats + unless @extension + @path = @view.send(:template_file_from_name, @path) + raise_missing_template_exception unless @path + @extension = @path.extension + end + + if @path = @paths.find_template_file_for_path(path) + @filename = @path.full_path + @extension = @path.extension end else - @filename = @path + @filename = @path.full_path end raise_missing_template_exception if @filename.blank? end def raise_missing_template_exception - full_template_path = @path.include?('.') ? @path : "#{@path}.#{@view.template_format}.erb" - display_paths = @finder.view_paths.join(':') - template_type = (@path =~ /layouts/i) ? 'layout' : 'template' + full_template_path = @original_path.include?('.') ? @original_path : "#{@original_path}.#{@view.template_format}.erb" + display_paths = @paths.join(':') + template_type = (@original_path =~ /layouts/i) ? 'layout' : 'template' raise(MissingTemplate, "Missing #{template_type} #{full_template_path} in view path #{display_paths}") end end diff --git a/actionpack/lib/action_view/template_file.rb b/actionpack/lib/action_view/template_file.rb new file mode 100644 index 0000000000000..dd66482b3cca9 --- /dev/null +++ b/actionpack/lib/action_view/template_file.rb @@ -0,0 +1,88 @@ +module ActionView #:nodoc: + # TemplateFile abstracts the pattern of querying a file path for its + # path with or without its extension. The path is only the partial path + # from the load path root e.g. "hello/index.html.erb" not + # "app/views/hello/index.html.erb" + class TemplateFile + def self.from_path(path, use_full_path = false) + path.is_a?(self) ? path : new(path, use_full_path) + end + + def self.from_full_path(load_path, full_path) + file = new(full_path.split(load_path).last) + file.load_path = load_path + file.freeze + end + + attr_accessor :load_path, :base_path, :name, :format, :extension + delegate :to_s, :inspect, :to => :path + + def initialize(path, use_full_path = false) + path = path.dup + + # Clear the forward slash in the beginning unless using full path + trim_forward_slash!(path) unless use_full_path + + @base_path, @name, @format, @extension = split(path) + end + + def freeze + @load_path.freeze + @base_path.freeze + @name.freeze + @format.freeze + @extension.freeze + super + end + + def format_and_extension + extensions = [format, extension].compact.join(".") + extensions.blank? ? nil : extensions + end + + def full_path + if load_path + "#{load_path}/#{path}" + else + path + end + end + + def path + base_path.to_s + [name, format, extension].compact.join(".") + end + + def path_without_extension + base_path.to_s + [name, format].compact.join(".") + end + + def path_without_format_and_extension + "#{base_path}#{name}" + end + + def dup_with_extension(extension) + file = dup + file.extension = extension ? extension.to_s : nil + file + end + + private + def trim_forward_slash!(path) + path.sub!(/^\//, '') + end + + # Returns file split into an array + # [base_path, name, format, extension] + def split(file) + if m = file.match(/^(.*\/)?(\w+)\.?(\w+)?\.?(\w+)?\.?(\w+)?$/) + if m[5] # Mulipart formats + [m[1], m[2], "#{m[3]}.#{m[4]}", m[5]] + elsif m[4] # Single format + [m[1], m[2], m[3], m[4]] + else # No format + [m[1], m[2], nil, m[3]] + end + end + end + end +end diff --git a/actionpack/lib/action_view/template_finder.rb b/actionpack/lib/action_view/template_finder.rb deleted file mode 100644 index 7e9a310810dbb..0000000000000 --- a/actionpack/lib/action_view/template_finder.rb +++ /dev/null @@ -1,147 +0,0 @@ -module ActionView #:nodoc: - class TemplateFinder #:nodoc: - cattr_reader :processed_view_paths - @@processed_view_paths = Hash.new {|hash, key| hash[key] = []} - - cattr_reader :file_extension_cache - @@file_extension_cache = Hash.new {|hash, key| - hash[key] = Hash.new {|hash, key| hash[key] = []} - } - - class << self #:nodoc: - # This method is not thread safe. Mutex should be used whenever this is accessed from an instance method - def process_view_paths(*view_paths) - view_paths.flatten.compact.each do |dir| - next if @@processed_view_paths.has_key?(dir) - @@processed_view_paths[dir] = [] - - # - # Dir.glob("#{dir}/**/*/**") reads all the directories in view path and templates inside those directories - # Dir.glob("#{dir}/**") reads templates residing at top level of view path - # - (Dir.glob("#{dir}/**/*/**") | Dir.glob("#{dir}/**")).each do |file| - unless File.directory?(file) - @@processed_view_paths[dir] << file.split(dir).last.sub(/^\//, '') - - # Build extension cache - extension = file.split(".").last - if ActionView::Template.template_handler_extensions.include?(extension) - key = file.split(dir).last.sub(/^\//, '').sub(/\.(\w+)$/, '') - @@file_extension_cache[dir][key] << extension - end - end - end - end - end - - def reload! - view_paths = @@processed_view_paths.keys - - @@processed_view_paths = Hash.new {|hash, key| hash[key] = []} - @@file_extension_cache = Hash.new {|hash, key| - hash[key] = Hash.new {|hash, key| hash[key] = []} - } - - process_view_paths(view_paths) - end - end - - attr_accessor :view_paths - - def initialize(*args) - @template = args.shift - - @view_paths = args.flatten - @view_paths = @view_paths.respond_to?(:find) ? @view_paths.dup : [*@view_paths].compact - self.class.process_view_paths(@view_paths) - end - - def prepend_view_path(path) - @view_paths.unshift(*path) - - self.class.process_view_paths(path) - end - - def append_view_path(path) - @view_paths.push(*path) - - self.class.process_view_paths(path) - end - - def view_paths=(path) - @view_paths = path - self.class.process_view_paths(path) - end - - def pick_template(template_path, extension) - file_name = "#{template_path}.#{extension}" - base_path = find_base_path_for(file_name) - base_path.blank? ? false : "#{base_path}/#{file_name}" - end - alias_method :template_exists?, :pick_template - - def file_exists?(template_path) - # Clear the forward slash in the beginning if exists - template_path = template_path.sub(/^\//, '') - - template_file_name, template_file_extension = path_and_extension(template_path) - - if template_file_extension - template_exists?(template_file_name, template_file_extension) - else - template_exists?(template_file_name, pick_template_extension(template_path)) - end - end - - def find_base_path_for(template_file_name) - @view_paths.find { |path| @@processed_view_paths[path].include?(template_file_name) } - end - - # Returns the view path that the full path resides in. - def extract_base_path_from(full_path) - @view_paths.find { |p| full_path[0..p.size - 1] == p } - end - - # Gets the extension for an existing template with the given template_path. - # Returns the format with the extension if that template exists. - # - # pick_template_extension('users/show') - # # => 'html.erb' - # - # pick_template_extension('users/legacy') - # # => "rhtml" - # - def pick_template_extension(template_path) - if extension = find_template_extension_from_handler(template_path, @template.template_format) || find_template_extension_from_first_render - extension - elsif @template.template_format == :js && extension = find_template_extension_from_handler(template_path, :html) - @template.template_format = :html - extension - end - end - - def find_template_extension_from_handler(template_path, template_format = @template.template_format) - formatted_template_path = "#{template_path}.#{template_format}" - - view_paths.each do |path| - if (extensions = @@file_extension_cache[path][formatted_template_path]).any? - return "#{template_format}.#{extensions.first}" - elsif (extensions = @@file_extension_cache[path][template_path]).any? - return extensions.first.to_s - end - end - nil - end - - # Splits the path and extension from the given template_path and returns as an array. - def path_and_extension(template_path) - template_path_without_extension = template_path.sub(/\.(\w+)$/, '') - [ template_path_without_extension, $1 ] - end - - # Determine the template extension from the @first_render filename - def find_template_extension_from_first_render - File.basename(@template.first_render.to_s)[/^[^.]+\.(.+)$/, 1] - end - end -end diff --git a/actionpack/lib/action_view/template_handlers.rb b/actionpack/lib/action_view/template_handlers.rb index 02945adddaccf..1471e99e01cfb 100644 --- a/actionpack/lib/action_view/template_handlers.rb +++ b/actionpack/lib/action_view/template_handlers.rb @@ -28,7 +28,6 @@ def self.extended(base) # return the rendered template as a string. def register_template_handler(extension, klass) @@template_handlers[extension.to_sym] = klass - ActionView::TemplateFinder.reload! end def template_handler_extensions diff --git a/actionpack/lib/action_view/view_load_paths.rb b/actionpack/lib/action_view/view_load_paths.rb new file mode 100644 index 0000000000000..e873d96aa0661 --- /dev/null +++ b/actionpack/lib/action_view/view_load_paths.rb @@ -0,0 +1,103 @@ +module ActionView #:nodoc: + class ViewLoadPaths < Array #:nodoc: + def self.type_cast(obj) + obj.is_a?(String) ? LoadPath.new(obj) : obj + end + + class LoadPath #:nodoc: + attr_reader :path, :paths + delegate :to_s, :inspect, :to => :path + + def initialize(path) + @path = path.freeze + reload! + end + + def eql?(view_path) + view_path.is_a?(ViewPath) && @path == view_path.path + end + + def hash + @path.hash + end + + # Rebuild load path directory cache + def reload! + @paths = {} + + files.each do |file| + @paths[file.path] = file + @paths[file.path_without_extension] ||= file + end + + @paths.freeze + end + + # Tries to find the extension for the template name. + # If it does not it exist, tries again without the format extension + # find_template_file_for_partial_path('users/show') => 'html.erb' + # find_template_file_for_partial_path('users/legacy') => 'rhtml' + def find_template_file_for_partial_path(file) + @paths[file.path] || @paths[file.path_without_extension] || @paths[file.path_without_format_and_extension] + end + + private + # Get all the files and directories in the path + def files_in_path + Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**") + end + + # Create an array of all the files within the path + def files + files_in_path.map do |file| + TemplateFile.from_full_path(@path, file) unless File.directory?(file) + end.compact + end + end + + def initialize(*args) + super(*args).map! { |obj| self.class.type_cast(obj) } + end + + def reload! + each { |path| path.reload! } + end + + def <<(obj) + super(self.class.type_cast(obj)) + end + + def push(*objs) + delete_paths!(objs) + super(*objs.map { |obj| self.class.type_cast(obj) }) + end + + def unshift(*objs) + delete_paths!(objs) + super(*objs.map { |obj| self.class.type_cast(obj) }) + end + + def template_exists?(file) + find_load_path_for_path(file) ? true : false + end + + def find_load_path_for_path(file) + find { |path| path.paths[file.to_s] } + end + + def find_template_file_for_path(file) + file = TemplateFile.from_path(file) + each do |path| + if f = path.find_template_file_for_partial_path(file) + return f + end + end + nil + end + + private + def delete_paths!(paths) + paths.each { |p1| delete_if { |p2| p1.to_s == p2.to_s } } + end + end +end diff --git a/actionpack/test/controller/view_paths_test.rb b/actionpack/test/controller/view_paths_test.rb index 0516da32d8480..1b4c1fae2f0db 100644 --- a/actionpack/test/controller/view_paths_test.rb +++ b/actionpack/test/controller/view_paths_test.rb @@ -1,30 +1,30 @@ require 'abstract_unit' class ViewLoadPathsTest < Test::Unit::TestCase - LOAD_PATH_ROOT = File.join(File.dirname(__FILE__), '..', 'fixtures') - ActionController::Base.view_paths = [ LOAD_PATH_ROOT ] + ActionController::Base.view_paths = [LOAD_PATH_ROOT] class TestController < ActionController::Base def self.controller_path() "test" end def rescue_action(e) raise end - + before_filter :add_view_path, :only => :hello_world_at_request_time - + def hello_world() end def hello_world_at_request_time() render(:action => 'hello_world') end + private - def add_view_path - prepend_view_path "#{LOAD_PATH_ROOT}/override" - end + def add_view_path + prepend_view_path "#{LOAD_PATH_ROOT}/override" + end end - + class Test::SubController < ActionController::Base layout 'test/sub' def hello_world; render(:template => 'test/hello_world'); end end - + def setup TestController.view_paths = nil @@ -41,68 +41,80 @@ def setup @last_message = nil ActiveSupport::Deprecation.behavior = Proc.new { |message, callback| @last_message = message } end - + def teardown ActiveSupport::Deprecation.behavior = @old_behavior end - + def test_template_load_path_was_set_correctly - assert_equal [ LOAD_PATH_ROOT ], @controller.view_paths + assert_equal [ LOAD_PATH_ROOT ], @controller.view_paths.map(&:to_s) end - + def test_controller_appends_view_path_correctly @controller.append_view_path 'foo' - assert_equal [LOAD_PATH_ROOT, 'foo'], @controller.view_paths - + assert_equal [LOAD_PATH_ROOT, 'foo'], @controller.view_paths.map(&:to_s) + @controller.append_view_path(%w(bar baz)) - assert_equal [LOAD_PATH_ROOT, 'foo', 'bar', 'baz'], @controller.view_paths + assert_equal [LOAD_PATH_ROOT, 'foo', 'bar', 'baz'], @controller.view_paths.map(&:to_s) + + @controller.append_view_path(LOAD_PATH_ROOT) + assert_equal ['foo', 'bar', 'baz', LOAD_PATH_ROOT], @controller.view_paths.map(&:to_s) + + @controller.append_view_path([LOAD_PATH_ROOT]) + assert_equal ['foo', 'bar', 'baz', LOAD_PATH_ROOT], @controller.view_paths.map(&:to_s) end - + def test_controller_prepends_view_path_correctly @controller.prepend_view_path 'baz' - assert_equal ['baz', LOAD_PATH_ROOT], @controller.view_paths - + assert_equal ['baz', LOAD_PATH_ROOT], @controller.view_paths.map(&:to_s) + @controller.prepend_view_path(%w(foo bar)) - assert_equal ['foo', 'bar', 'baz', LOAD_PATH_ROOT], @controller.view_paths + assert_equal ['foo', 'bar', 'baz', LOAD_PATH_ROOT], @controller.view_paths.map(&:to_s) + + @controller.prepend_view_path(LOAD_PATH_ROOT) + assert_equal [LOAD_PATH_ROOT, 'foo', 'bar', 'baz'], @controller.view_paths.map(&:to_s) + + @controller.prepend_view_path([LOAD_PATH_ROOT]) + assert_equal [LOAD_PATH_ROOT, 'foo', 'bar', 'baz'], @controller.view_paths.map(&:to_s) end - + def test_template_appends_view_path_correctly @controller.instance_variable_set :@template, ActionView::Base.new(TestController.view_paths, {}, @controller) class_view_paths = TestController.view_paths @controller.append_view_path 'foo' - assert_equal [LOAD_PATH_ROOT, 'foo'], @controller.view_paths - + assert_equal [LOAD_PATH_ROOT, 'foo'], @controller.view_paths.map(&:to_s) + @controller.append_view_path(%w(bar baz)) - assert_equal [LOAD_PATH_ROOT, 'foo', 'bar', 'baz'], @controller.view_paths + assert_equal [LOAD_PATH_ROOT, 'foo', 'bar', 'baz'], @controller.view_paths.map(&:to_s) assert_equal class_view_paths, TestController.view_paths end - + def test_template_prepends_view_path_correctly @controller.instance_variable_set :@template, ActionView::Base.new(TestController.view_paths, {}, @controller) class_view_paths = TestController.view_paths - + @controller.prepend_view_path 'baz' - assert_equal ['baz', LOAD_PATH_ROOT], @controller.view_paths - + assert_equal ['baz', LOAD_PATH_ROOT], @controller.view_paths.map(&:to_s) + @controller.prepend_view_path(%w(foo bar)) - assert_equal ['foo', 'bar', 'baz', LOAD_PATH_ROOT], @controller.view_paths + assert_equal ['foo', 'bar', 'baz', LOAD_PATH_ROOT], @controller.view_paths.map(&:to_s) assert_equal class_view_paths, TestController.view_paths end - + def test_view_paths get :hello_world assert_response :success assert_equal "Hello world!", @response.body end - + def test_view_paths_override TestController.prepend_view_path "#{LOAD_PATH_ROOT}/override" get :hello_world assert_response :success assert_equal "Hello overridden world!", @response.body end - + def test_view_paths_override_for_layouts_in_controllers_with_a_module @controller = Test::SubController.new Test::SubController.view_paths = [ "#{LOAD_PATH_ROOT}/override", LOAD_PATH_ROOT, "#{LOAD_PATH_ROOT}/override2" ] @@ -110,31 +122,44 @@ def test_view_paths_override_for_layouts_in_controllers_with_a_module assert_response :success assert_equal "layout: Hello overridden world!", @response.body end - + def test_view_paths_override_at_request_time get :hello_world_at_request_time assert_response :success assert_equal "Hello overridden world!", @response.body end - + def test_inheritance original_load_paths = ActionController::Base.view_paths - + self.class.class_eval %{ class A < ActionController::Base; end class B < A; end class C < ActionController::Base; end } - - A.view_paths = [ 'a/path' ] - - assert_equal [ 'a/path' ], A.view_paths - assert_equal A.view_paths, B.view_paths + + A.view_paths = ['a/path'] + + assert_equal ['a/path'], A.view_paths.map(&:to_s) + assert_equal A.view_paths, B.view_paths assert_equal original_load_paths, C.view_paths - + C.view_paths = [] assert_nothing_raised { C.view_paths << 'c/path' } - assert_equal ['c/path'], C.view_paths + assert_equal ['c/path'], C.view_paths.map(&:to_s) + end + + def test_find_template_file_for_path + assert_equal "test/hello_world.erb", @controller.view_paths.find_template_file_for_path("test/hello_world.erb").to_s + assert_equal "test/hello.builder", @controller.view_paths.find_template_file_for_path("test/hello.builder").to_s + assert_equal nil, @controller.view_paths.find_template_file_for_path("test/missing.erb") + end + + def test_view_paths_find_template_file_for_path + assert_equal "test/formatted_html_erb.html.erb", @controller.view_paths.find_template_file_for_path("test/formatted_html_erb.html").to_s + assert_equal "test/formatted_xml_erb.xml.erb", @controller.view_paths.find_template_file_for_path("test/formatted_xml_erb.xml").to_s + assert_equal "test/hello_world.erb", @controller.view_paths.find_template_file_for_path("test/hello_world.html").to_s + assert_equal "test/hello_world.erb", @controller.view_paths.find_template_file_for_path("test/hello_world.xml").to_s + assert_equal nil, @controller.view_paths.find_template_file_for_path("test/missing.html") end - end diff --git a/actionpack/test/template/template_file_test.rb b/actionpack/test/template/template_file_test.rb new file mode 100644 index 0000000000000..d14a966c1cf3a --- /dev/null +++ b/actionpack/test/template/template_file_test.rb @@ -0,0 +1,95 @@ +require 'abstract_unit' + +class TemplateFileTest < Test::Unit::TestCase + LOAD_PATH_ROOT = File.join(File.dirname(__FILE__), '..', 'fixtures') + + def setup + @template = ActionView::TemplateFile.new("test/hello_world.html.erb") + @another_template = ActionView::TemplateFile.new("test/hello_world.erb") + @file_only = ActionView::TemplateFile.new("hello_world.erb") + @full_path = ActionView::TemplateFile.new("/u/app/scales/config/../app/views/test/hello_world.erb", true) + @layout = ActionView::TemplateFile.new("layouts/hello") + @multipart = ActionView::TemplateFile.new("test_mailer/implicitly_multipart_example.text.html.erb") + end + + def test_path + assert_equal "test/hello_world.html.erb", @template.path + assert_equal "test/hello_world.erb", @another_template.path + assert_equal "hello_world.erb", @file_only.path + assert_equal "/u/app/scales/config/../app/views/test/hello_world.erb", @full_path.path + assert_equal "layouts/hello", @layout.path + assert_equal "test_mailer/implicitly_multipart_example.text.html.erb", @multipart.path + end + + def test_path_without_extension + assert_equal "test/hello_world.html", @template.path_without_extension + assert_equal "test/hello_world", @another_template.path_without_extension + assert_equal "hello_world", @file_only.path_without_extension + assert_equal "layouts/hello", @layout.path_without_extension + assert_equal "test_mailer/implicitly_multipart_example.text.html", @multipart.path_without_extension + end + + def test_path_without_format_and_extension + assert_equal "test/hello_world", @template.path_without_format_and_extension + assert_equal "test/hello_world", @another_template.path_without_format_and_extension + assert_equal "hello_world", @file_only.path_without_format_and_extension + assert_equal "layouts/hello", @layout.path_without_format_and_extension + assert_equal "test_mailer/implicitly_multipart_example", @multipart.path_without_format_and_extension + end + + def test_name + assert_equal "hello_world", @template.name + assert_equal "hello_world", @another_template.name + assert_equal "hello_world", @file_only.name + assert_equal "hello_world", @full_path.name + assert_equal "hello", @layout.name + assert_equal "implicitly_multipart_example", @multipart.name + end + + def test_format + assert_equal "html", @template.format + assert_equal nil, @another_template.format + assert_equal nil, @layout.format + assert_equal "text.html", @multipart.format + end + + def test_extension + assert_equal "erb", @template.extension + assert_equal "erb", @another_template.extension + assert_equal nil, @layout.extension + assert_equal "erb", @multipart.extension + end + + def test_format_and_extension + assert_equal "html.erb", @template.format_and_extension + assert_equal "erb", @another_template.format_and_extension + assert_equal nil, @layout.format_and_extension + assert_equal "text.html.erb", @multipart.format_and_extension + end + + def test_new_file_with_extension + file = @template.dup_with_extension(:haml) + assert_equal "test/hello_world.html", file.path_without_extension + assert_equal "haml", file.extension + assert_equal "test/hello_world.html.haml", file.path + + file = @another_template.dup_with_extension(:haml) + assert_equal "test/hello_world", file.path_without_extension + assert_equal "haml", file.extension + assert_equal "test/hello_world.haml", file.path + + file = @another_template.dup_with_extension(nil) + assert_equal "test/hello_world", file.path_without_extension + assert_equal nil, file.extension + assert_equal "test/hello_world", file.path + end + + def test_freezes_entire_contents + @template.freeze + assert @template.frozen? + assert @template.base_path.frozen? + assert @template.name.frozen? + assert @template.format.frozen? + assert @template.extension.frozen? + end +end diff --git a/actionpack/test/template/template_finder_test.rb b/actionpack/test/template/template_finder_test.rb deleted file mode 100644 index 07fc4b8c56535..0000000000000 --- a/actionpack/test/template/template_finder_test.rb +++ /dev/null @@ -1,60 +0,0 @@ -require 'abstract_unit' - -class TemplateFinderTest < Test::Unit::TestCase - - LOAD_PATH_ROOT = File.join(File.dirname(__FILE__), '..', 'fixtures') - - def setup - ActionView::TemplateFinder.process_view_paths(LOAD_PATH_ROOT) - ActionView::Template::register_template_handler :mab, Class.new(ActionView::TemplateHandler) - @template = ActionView::Base.new - @finder = ActionView::TemplateFinder.new(@template, LOAD_PATH_ROOT) - end - - def test_should_cache_file_extension_properly - assert_equal ["builder", "erb", "rhtml", "rjs", "rxml", "mab"].sort, - ActionView::TemplateFinder.file_extension_cache[LOAD_PATH_ROOT].values.flatten.uniq.sort - - assert_equal (Dir.glob("#{LOAD_PATH_ROOT}/**/*/*.{erb,rjs,rhtml,builder,rxml,mab}") | - Dir.glob("#{LOAD_PATH_ROOT}/**.{erb,rjs,rhtml,builder,rxml,mab}")).size, - ActionView::TemplateFinder.file_extension_cache[LOAD_PATH_ROOT].keys.size - end - - def test_should_cache_dir_content_properly - assert ActionView::TemplateFinder.processed_view_paths[LOAD_PATH_ROOT] - assert_equal (Dir.glob("#{LOAD_PATH_ROOT}/**/*/**") | Dir.glob("#{LOAD_PATH_ROOT}/**")).find_all {|f| !File.directory?(f) }.size, - ActionView::TemplateFinder.processed_view_paths[LOAD_PATH_ROOT].size - end - - def test_find_template_extension_from_first_render - assert_nil @finder.send(:find_template_extension_from_first_render) - - { - nil => nil, - '' => nil, - 'foo' => nil, - '/foo' => nil, - 'foo.rb' => 'rb', - 'foo.bar.rb' => 'bar.rb', - 'baz/foo.rb' => 'rb', - 'baz/foo.bar.rb' => 'bar.rb', - 'baz/foo.o/foo.rb' => 'rb', - 'baz/foo.o/foo.bar.rb' => 'bar.rb', - }.each do |input,expectation| - @template.instance_variable_set('@first_render', input) - assert_equal expectation, @finder.send(:find_template_extension_from_first_render) - end - end - - def test_should_report_file_exists_correctly - assert_nil @finder.send(:find_template_extension_from_first_render) - assert_equal false, @finder.send(:file_exists?, 'test.rhtml') - assert_equal false, @finder.send(:file_exists?, 'test.rb') - - @template.instance_variable_set('@first_render', 'foo.rb') - - assert_equal 'rb', @finder.send(:find_template_extension_from_first_render) - assert_equal false, @finder.send(:file_exists?, 'baz') - assert_equal false, @finder.send(:file_exists?, 'baz.rb') - end -end diff --git a/actionpack/test/template/template_object_test.rb b/actionpack/test/template/template_object_test.rb index afb5c5cc16976..2cfc4523c6370 100644 --- a/actionpack/test/template/template_object_test.rb +++ b/actionpack/test/template/template_object_test.rb @@ -2,60 +2,57 @@ class TemplateObjectTest < Test::Unit::TestCase LOAD_PATH_ROOT = File.join(File.dirname(__FILE__), '..', 'fixtures') - ActionView::TemplateFinder.process_view_paths(LOAD_PATH_ROOT) - + class TemplateTest < Test::Unit::TestCase def setup @view = ActionView::Base.new(LOAD_PATH_ROOT) @path = "test/hello_world.erb" end - + def test_should_create_valid_template template = ActionView::Template.new(@view, @path, true) - + assert_kind_of ActionView::TemplateHandlers::ERB, template.handler - assert_equal "test/hello_world.erb", template.path + assert_equal "test/hello_world.erb", template.path.to_s assert_nil template.instance_variable_get(:"@source") assert_equal "erb", template.extension end - + uses_mocha 'Template preparation tests' do - def test_should_prepare_template_properly template = ActionView::Template.new(@view, @path, true) view = template.instance_variable_get(:"@view") - + view.expects(:evaluate_assigns) template.handler.expects(:compile_template).with(template) view.expects(:method_names).returns({}) - + template.prepare! end - end end - + class PartialTemplateTest < Test::Unit::TestCase def setup @view = ActionView::Base.new(LOAD_PATH_ROOT) @path = "test/partial_only" end - + def test_should_create_valid_partial_template template = ActionView::PartialTemplate.new(@view, @path, nil) - - assert_equal "test/_partial_only", template.path + + assert_equal "test/_partial_only", template.path.path_without_format_and_extension assert_equal :partial_only, template.variable_name - + assert template.locals.has_key?(:object) assert template.locals.has_key?(:partial_only) end - + def test_partial_with_errors template = ActionView::PartialTemplate.new(@view, 'test/raise', nil) assert_raise(ActionView::TemplateError) { template.render_template } end - + uses_mocha 'Partial template preparation tests' do def test_should_prepare_on_initialization ActionView::PartialTemplate.any_instance.expects(:prepare!) @@ -63,7 +60,7 @@ def test_should_prepare_on_initialization end end end - + class PartialTemplateFallbackTest < Test::Unit::TestCase def setup @view = ActionView::Base.new(LOAD_PATH_ROOT) @@ -72,7 +69,7 @@ def setup def test_default template = ActionView::PartialTemplate.new(@view, @path, nil) - assert_equal 'test/_layout_for_partial', template.path + assert_equal 'test/_layout_for_partial', template.path.path_without_format_and_extension assert_equal 'erb', template.extension assert_equal :html, @view.template_format end @@ -80,7 +77,7 @@ def test_default def test_js @view.template_format = :js template = ActionView::PartialTemplate.new(@view, @path, nil) - assert_equal 'test/_layout_for_partial', template.path + assert_equal 'test/_layout_for_partial', template.path.path_without_format_and_extension assert_equal 'erb', template.extension assert_equal :html, @view.template_format end