diff options
| -rw-r--r-- | actionpack/CHANGELOG | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_view.rb | 1 | ||||
| -rw-r--r-- | actionpack/lib/action_view/base.rb | 173 | ||||
| -rw-r--r-- | actionpack/lib/action_view/compiled_templates.rb | 70 | ||||
| -rw-r--r-- | actionpack/lib/action_view/template_error.rb | 44 | ||||
| -rw-r--r-- | actionpack/test/controller/new_render_test.rb | 1 | ||||
| -rw-r--r-- | actionpack/test/template/compiled_templates_tests.rb | 63 |
7 files changed, 232 insertions, 122 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 24eec5f9f4..3835cb51e7 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *SVN* +* Rewrote compiled templates to decrease code complexity. Removed template load caching in favour of compiled caching. Fixed template error messages. [Nicholas Seckar] + * Fix Routing to handle :some_param => nil better. [Nicholas Seckar, Luminas] * Add support for :include with pagination (subject to existing constraints for :include with :limit and :offset) #1478 [michael@schubert.cx] diff --git a/actionpack/lib/action_view.rb b/actionpack/lib/action_view.rb index 3c6e109caf..45acbd5ff7 100644 --- a/actionpack/lib/action_view.rb +++ b/actionpack/lib/action_view.rb @@ -24,6 +24,7 @@ $:.unshift(File.dirname(__FILE__) + "/action_view/vendor") require 'action_view/vendor/builder' +require 'action_view/compiled_templates' require 'action_view/base' require 'action_view/partials' diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 097e4f8b7f..56d174e124 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -1,6 +1,7 @@ require 'erb' module ActionView #:nodoc: + class ActionViewError < StandardError #:nodoc: end @@ -59,10 +60,8 @@ module ActionView #:nodoc: # # == Template caching # - # The parsing of ERb templates are cached by default, but the reading of them are not. This means that the application by default - # will reflect changes to the templates immediatly. If you'd like to sacrifice that immediacy for the speed gain given by also - # caching the loading of templates (reading from the file system), you can turn that on with - # <tt>ActionView::Base.cache_template_loading = true</tt>. + # By default, Rails will compile each template to a method in order to render it. When you alter a template, Rails will + # check the file's modification time and recompile it. # # == Builder # @@ -125,23 +124,16 @@ module ActionView #:nodoc: attr_reader :logger, :params, :response, :session, :headers, :flash - # Turn on to cache the reading of templates from the file system. - # Doing so means that you have to restart the server when changing - # templates, but it will save checking whether the file has changed - # on disk. - @@cache_template_loading = false - cattr_accessor :cache_template_loading - # Specify trim mode for the ERB compiler. Defaults to '-'. # See ERB documentation for suitable values. @@erb_trim_mode = '-' cattr_accessor :erb_trim_mode - @@compiled_templates = {} - @@template_count = 0 - @@loaded_templates = {} @@template_handlers = {} + @@compiled_templates = CompiledTemplates.new + include @@compiled_templates + def self.load_helpers(helper_dir)#:nodoc: Dir.foreach(helper_dir) do |helper_file| next unless helper_file =~ /_helper.rb$/ @@ -177,7 +169,7 @@ module ActionView #:nodoc: template_extension = template_path.split('.').last end - template_source = read_template_file(template_file_name, template_extension) + template_source = nil # Don't read the source until we know that it is required begin render_template(template_extension, template_source, template_file_name, local_assigns) @@ -214,14 +206,41 @@ module ActionView #:nodoc: # Renders the +template+ which is given as a string as either rhtml or rxml depending on <tt>template_extension</tt>. # The hash in <tt>local_assigns</tt> is made available as local variables. - def render_template(template_extension, template, file_name = nil, local_assigns = {}) - if handler = @@template_handlers[template_extension] + def render_template(template_extension, template, file_path = nil, local_assigns = {}) + if handler = @@template_handlers[template_extension] + template ||= read_template_file(file_path, template_extension) # Make sure that a lazyily-read template is loaded. delegate_render(handler, template, local_assigns) - elsif template_extension == 'rxml' - rxml_render(template_extension, template, file_name, local_assigns) else - rhtml_render(template_extension, template, file_name, local_assigns) + compile_and_render_template(template_extension, template, file_path, local_assigns) + end + end + + # Render the privded template with the given local assigns. If the template has not been rendered with the provided + # local assigns yet, or if the template has been updated on disk, then the template will be compiled to a method. + # + # Either, but not both, of template and file_path may be nil. If file_path is given but template is nil, the template + # will only be read if it has to be compiled. + # + def compile_and_render_template(extension, template = nil, file_path = nil, local_assigns = {}) + file_path = File.expand_path(file_path) if file_path + identifier = file_path || template # either might be nil. Prefer to use the file_path as a key + names, params = split_locals(local_assigns) + + compile = ! @@compiled_templates.compiled?(identifier, names) # Compile the template if it hasn't been done + if ! compile && file_path # If the file path is given, recompile if the mtime is new. + compiled_at = @@compiled_templates.mtime(identifier, names) + compile = compiled_at.nil? || (mtime = File.mtime(file_path)).nil? || compiled_at < mtime + end + + if compile + template ||= read_template_file(file_path, extension) + compile_template(extension, file_path || 'inline-template', identifier, template, names) end + + # Get the selector for this template and names, then call the method. + selector = @@compiled_templates.selector(identifier, names) + evaluate_assigns + send(selector, *params) end def pick_template_extension(template_path)#:nodoc: @@ -263,103 +282,49 @@ module ActionView #:nodoc: end def template_exists?(template_path, extension) - fp = full_template_path(template_path, extension) - (@@cache_template_loading && @@loaded_templates.has_key?(fp)) || FileTest.exists?(fp) + File.file?(full_template_path(template_path, extension)) end + # This method reads a template file. No, it doesn't check mtimes, look to check if the template + # has been compiled, or check your date of birth. It reads the template file. Crazy, I know. def read_template_file(template_path, extension) - info = @@loaded_templates[template_path] - # info is either the template source code, or the its compile time, or nil - # if nil, we need to read it from the file system - # if it is a time, we need to reread it if it has changed on disk - # if @@cache_template_loading is true, we will never reread - unless read_file = info.nil? - read_file = !@@cache_template_loading && info.is_a?(Time) && info < File.stat(template_path).mtime - end - if read_file - info = @@loaded_templates[template_path] = File.read(template_path) - @@compiled_templates[template_path] = nil - end + File.read(template_path) + end - info + # Split the provided hash of local assigns into two arrays, one of the names, and another of the value + # The arrays are guarenteed to be in matching order, and also ordered the same for different hashes. + def split_locals(assigns) + names, values = [], [] + assigns.to_a.sort_by {|pair| pair.first.to_s}.each do |name, value| + names << name + values << value + end + return [names, values] end - def evaluate_assigns(local_assigns = {}) + def evaluate_assigns unless @assigns_added assign_variables_from_controller @assigns_added = true end - saved_locals = {} - - local_assigns.each do |key, value| - varstr = "@_#{key}_" - saved_locals[varstr] = instance_variable_get(varstr) - instance_variable_set(varstr, value) - - unless self.respond_to?(key) - self.class.class_eval("def #{key}; #{varstr}; end") - self.class.class_eval("def #{key}=(v); #{varstr} = v; end") - end - end - - saved_locals end - def compile_template(extension, template, file_name) - cache_name = file_name || template - - unless @@compiled_templates[cache_name] - case extension - when :rhtml - t_name = 'run_html_' - t_arg = '' - t_code = ERB.new(template, nil, @@erb_trim_mode).src - when :rxml - t_name = 'run_xml_' - t_arg = '(xml)' - t_code = template - end - - if file_name - i = file_name.index(@base_path) - l = @base_path.length - s_file_name = i ? file_name[i+l+1,file_name.length-l-1] : file_name - s_file_name.sub!(/(.rhtml|.rxml)$/,'') - s_file_name.tr!('/:-', '_') - s_file_name.gsub!(/[^a-zA-Z0-9_]/){|s| s[0].to_s} - t_name += s_file_name - else - @@template_count += 1 - t_name += @@template_count.to_s - end - - @@loaded_templates[cache_name] = Time.now if file_name - - t_def = "def #{t_name}#{t_arg}; #{t_code}; end" - self.class.class_eval(t_def) rescue raise ActionViewError, "ERROR defining #{t_name}: #{t_def}" - - @@compiled_templates[cache_name] = t_name.intern - - logger.debug "Compiled template #{cache_name}\n ==> #{t_name}" if logger + # Compile the template to a method using a CompiledTemplates instance. + def compile_template(extension, file_path, identifier, template, local_names = []) + line_no = 0 + + case extension && extension.to_sym + when :rxml + # Initialize the xml variable to the builder instance. + source_code = \ +"xml = Builder::XmlMarkup.new(:indent => 2) +@controller.headers['Content-Type'] ||= 'text/xml'\n" + template + line_no = -2 # offset extra line. + else # Assume rhtml + source_code = ERB.new(template, nil, @@erb_trim_mode).src end - @@compiled_templates[cache_name] - end - - def rhtml_render(extension, template, file_name, local_assigns) - render_sym = compile_template(:rhtml, template, file_name) - saved_locals = evaluate_assigns(local_assigns) - result = self.send(render_sym) - saved_locals.each { |k,v| instance_variable_set(k, v) } - result - end - def rxml_render(extension, template, file_name, local_assigns) - @controller.headers["Content-Type"] ||= 'text/xml' - render_sym = compile_template(:rxml, template, file_name) - saved_locals = evaluate_assigns(local_assigns) - result = self.send(render_sym, Builder::XmlMarkup.new(:indent => 2)) - saved_locals.each { |k,v| instance_variable_set(k, v) } - result + @@compiled_templates.compile_source(identifier, local_names, source_code, line_no, file_path) end def delegate_render(handler, template, local_assigns) @@ -372,4 +337,4 @@ module ActionView #:nodoc: end end -require 'action_view/template_error'
\ No newline at end of file +require 'action_view/template_error' diff --git a/actionpack/lib/action_view/compiled_templates.rb b/actionpack/lib/action_view/compiled_templates.rb new file mode 100644 index 0000000000..9488432ba2 --- /dev/null +++ b/actionpack/lib/action_view/compiled_templates.rb @@ -0,0 +1,70 @@ + +module ActionView + + # CompiledTemplates modules hold methods that have been compiled. + # Templates are compiled into these methods so that they do not need to be + # re-read and re-parsed each request. + # + # Each template may be compiled into one or more methods. Each method accepts a given + # set of parameters which is used to implement local assigns passing. + # + # To use a compiled template module, create a new instance and include it into the class + # in which you want the template to be rendered. + class CompiledTemplates < Module + attr_reader :method_names + + def initialize + @method_names = Hash.new do |hash, key| + hash[key] = "__compiled_method_#{(hash.length + 1)}" + end + @mtimes = {} + end + + # Return the full key for the given identifier and argument names + def full_key(identifier, arg_names) + [identifier, arg_names] + end + + # Return the selector for this method or nil if it has not been compiled + def selector(identifier, arg_names) + key = full_key(identifier, arg_names) + method_names.key?(key) ? method_names[key] : nil + end + alias :compiled? :selector + + # Return the time at which the method for the given identifier and argument names was compiled. + def mtime(identifier, arg_names) + @mtimes[full_key(identifier, arg_names)] + end + + # Compile the provided source code for the given argument names and with the given initial line number. + # The identifier should be unique to this source. + # + # The file_name, if provided will appear in backtraces. If not provded, the file_name defaults + # to the identifier. + # + # This method will return the selector for the compiled version of this method. + def compile_source(identifier, arg_names, source, initial_line_number = 0, file_name = nil) + file_name ||= identifier + name = method_names[full_key(identifier, arg_names)] + arg_desc = arg_names.empty? ? '' : "(#{arg_names * ', '})" + fake_file_name = "#{file_name}#{arg_desc}" # Include the arguments for this version (for now) + + method_def = wrap_source(name, arg_names, source) + + begin + module_eval(method_def, fake_file_name, initial_line_number) + @mtimes[full_key(identifier, arg_names)] = Time.now + rescue Object => e + e.blame_file! identifier + raise + end + name + end + + # Wrap the provided source in a def ... end block. + def wrap_source(name, arg_names, source) + "def #{name}(#{arg_names * ', '})\n#{source}\nend" + end + end +end diff --git a/actionpack/lib/action_view/template_error.rb b/actionpack/lib/action_view/template_error.rb index 5fa7f2db5e..28f11675cf 100644 --- a/actionpack/lib/action_view/template_error.rb +++ b/actionpack/lib/action_view/template_error.rb @@ -7,16 +7,13 @@ module ActionView attr_reader :original_exception def initialize(base_path, file_name, assigns, source, original_exception) - @base_path, @file_name, @assigns, @source, @original_exception = - base_path, file_name, assigns, source, original_exception + @base_path, @assigns, @source, @original_exception = + base_path, assigns, source, original_exception + @file_name = File.expand_path file_name end def message - if original_exception.message.include?("(eval):") - original_exception.message.scan(/\(eval\):(?:[0-9]*):in `.*'(.*)/).first.first - else - original_exception.message - end + original_exception.message end def sub_template_message @@ -42,21 +39,20 @@ module ActionView extract.join end - + def sub_template_of(file_name) @sub_templates ||= [] @sub_templates << file_name end def line_number - trace = @original_exception.backtrace.join - if trace.include?("erb):") - trace.scan(/\((?:erb)\):([0-9]*)/).first.first.to_i - elsif trace.include?("eval):") - trace.scan(/\((?:eval)\):([0-9]*)/).first.first.to_i - else - 1 + if @file_name + regexp = /#{Regexp.escape @file_name}(?:\(.*?\))?:(\d+)/ # A regexp to match a line number in our file + [@original_exception.message, @original_exception.backtrace].flatten.each do |line| + return $1.to_i if regexp =~ line + end end + 0 end def file_name @@ -79,12 +75,24 @@ module ActionView private def strip_base_path(file_name) + file_name = File.expand_path(file_name).gsub(/^#{Regexp.escape File.expand_path(RAILS_ROOT)}/, '') file_name.gsub(@base_path, "") end + if defined?(RAILS_ROOT) + RailsRootRegexp = %r((#{Regexp.escape RAILS_ROOT}|#{Regexp.escape File.expand_path(RAILS_ROOT)})/(.*)?) + else + RailsRootRegexp = /^()(.*)$/ + end + def clean_backtrace(exception) - base_dir = File.expand_path(File.dirname(__FILE__) + "/../../../../") - exception.backtrace.collect { |line| line.gsub(base_dir, "").gsub("/public/../config/environments/../../", "").gsub("/public/../", "") } + exception.backtrace.collect do |line| + line.gsub %r{^(\s*)(/[-\w\d\./]+)} do + leading, path = $1, $2 + path = $2 if RailsRootRegexp =~ path + leading + path + end + end end end -end
\ No newline at end of file +end diff --git a/actionpack/test/controller/new_render_test.rb b/actionpack/test/controller/new_render_test.rb index 1e077bb902..a29a6b9cd2 100644 --- a/actionpack/test/controller/new_render_test.rb +++ b/actionpack/test/controller/new_render_test.rb @@ -132,6 +132,7 @@ class NewRenderTestController < ActionController::Base def rendering_with_conflicting_local_vars @name = "David" + def @template.name() nil end render :action => "potential_conflicts" end diff --git a/actionpack/test/template/compiled_templates_tests.rb b/actionpack/test/template/compiled_templates_tests.rb new file mode 100644 index 0000000000..4734e6e81e --- /dev/null +++ b/actionpack/test/template/compiled_templates_tests.rb @@ -0,0 +1,63 @@ +require 'test/unit' +require File.dirname(__FILE__) + '/../../lib/action_view/helpers/date_helper' +require File.dirname(__FILE__) + "/../abstract_unit" + +class CompiledTemplateTests < Test::Unit::TestCase + + def setup + @ct = ActionView::CompiledTemplates.new + @v = Class.new + @v.send :include, @ct + end + attr_reader :ct, :v + + def test_name_allocation + hi_world = ct.method_names['hi world'] + hi_sexy = ct.method_names['hi sexy'] + wish_upon_a_star = ct.method_names['I love seeing decent error messages'] + + assert_equal hi_world, ct.method_names['hi world'] + assert_equal hi_sexy, ct.method_names['hi sexy'] + assert_equal wish_upon_a_star, ct.method_names['I love seeing decent error messages'] + assert_equal 3, [hi_world, hi_sexy, wish_upon_a_star].uniq.length + end + + def test_wrap_source + assert_equal( + "def aliased_assignment(value)\nself.value = value\nend", + @ct.wrap_source(:aliased_assignment, [:value], 'self.value = value') + ) + + assert_equal( + "def simple()\nnil\nend", + @ct.wrap_source(:simple, [], 'nil') + ) + end + + def test_compile_source_single_method + selector = ct.compile_source('doubling method', [:a], 'a + a') + assert_equal 2, @v.new.send(selector, 1) + assert_equal 4, @v.new.send(selector, 2) + assert_equal -4, @v.new.send(selector, -2) + assert_equal 0, @v.new.send(selector, 0) + selector + end + + def test_compile_source_two_method + sel1 = test_compile_source_single_method # compile the method in the other test + sel2 = ct.compile_source('doubling method', [:a, :b], 'a + b + a + b') + assert_not_equal sel1, sel2 + + assert_equal 2, @v.new.send(sel1, 1) + assert_equal 4, @v.new.send(sel1, 2) + + assert_equal 6, @v.new.send(sel2, 1, 2) + assert_equal 32, @v.new.send(sel2, 15, 1) + end + + def test_mtime + t1 = Time.now + test_compile_source_single_method + assert (t1..Time.now).include?(ct.mtime('doubling method', [:a])) + end +end |