From 239262fee03096d1e52acf8fe69de736726d87e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Thu, 8 Dec 2011 16:37:56 +0100 Subject: Optimize layout lookup to avoid double calls. --- actionpack/lib/abstract_controller/layouts.rb | 50 +++++++++++----------- actionpack/lib/abstract_controller/view_paths.rb | 2 +- actionpack/lib/action_view/lookup_context.rb | 1 - .../lib/action_view/renderer/template_renderer.rb | 21 ++++++--- 4 files changed, 41 insertions(+), 33 deletions(-) (limited to 'actionpack/lib') diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index 79edd5418e..6b6b38c64f 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -168,11 +168,12 @@ module AbstractController included do class_attribute :_layout_conditions remove_possible_method :_layout_conditions - delegate :_layout_conditions, :to => :'self.class' self._layout_conditions = {} _write_layout_method end + delegate :_layout_conditions, :to => "self.class" + module ClassMethods def inherited(klass) super @@ -188,7 +189,7 @@ module AbstractController # # ==== Returns # * Boolean - True if the action has a layout, false otherwise. - def action_has_layout? + def conditional_layout? return unless super conditions = _layout_conditions @@ -244,7 +245,13 @@ module AbstractController def _write_layout_method remove_possible_method(:_layout) - prefixes = _implied_layout_name =~ /\blayouts/ ? [] : ["layouts"] + prefixes = _implied_layout_name =~ /\blayouts/ ? [] : ["layouts"] + name_clause = if name + <<-RUBY + lookup_context.find_all("#{_implied_layout_name}", #{prefixes.inspect}).first || super + RUBY + end + layout_definition = case defined?(@_layout) ? @_layout : nil when String @_layout.inspect @@ -265,27 +272,15 @@ module AbstractController when true raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil" when nil - if name - <<-RUBY - if template_exists?("#{_implied_layout_name}", #{prefixes.inspect}) - "#{_implied_layout_name}" - else - super - end - RUBY - end + name_clause end self.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def _layout - if action_has_layout? + if conditional_layout? #{layout_definition} - elsif self.class.name - if template_exists?("#{_implied_layout_name}", #{prefixes.inspect}) - "#{_implied_layout_name}" - else - super - end + else + #{name_clause} end end RUBY @@ -298,8 +293,7 @@ module AbstractController if _include_layout?(options) layout = options.key?(:layout) ? options.delete(:layout) : :default - value = _layout_for_option(layout) - options[:layout] = (value =~ /\blayouts/ ? value : "layouts/#{value}") if value + options[:layout] = Proc.new { _normalize_layout(_layout_for_option(layout)) } end end @@ -314,6 +308,10 @@ module AbstractController @_action_has_layout end + def conditional_layout? + true + end + private # This will be overwritten by _write_layout_method @@ -335,6 +333,10 @@ module AbstractController end end + def _normalize_layout(value) + value.is_a?(String) && value !~ /\blayouts/ ? "layouts/#{value}" : value + end + # Returns the default layout for this controller. # Optionally raises an exception if the layout could not be found. # @@ -346,17 +348,17 @@ module AbstractController # * template - The template object for the default layout (or nil) def _default_layout(require_layout = false) begin - layout_name = _layout + value = _layout if action_has_layout? rescue NameError => e raise e, "Could not render layout: #{e.message}" end - if require_layout && action_has_layout? && !layout_name + if require_layout && action_has_layout? && !value raise ArgumentError, "There was no default layout for #{self.class} in #{view_paths.inspect}" end - layout_name + value end def _include_layout?(options) diff --git a/actionpack/lib/abstract_controller/view_paths.rb b/actionpack/lib/abstract_controller/view_paths.rb index e8394447a7..96118b940f 100644 --- a/actionpack/lib/abstract_controller/view_paths.rb +++ b/actionpack/lib/abstract_controller/view_paths.rb @@ -10,7 +10,7 @@ module AbstractController self._view_paths.freeze end - delegate :find_template, :template_exists?, :view_paths, :formats, :formats=, + delegate :template_exists?, :view_paths, :formats, :formats=, :locale, :locale=, :to => :lookup_context module ClassMethods diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index d26158cefe..f586708d54 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -227,7 +227,6 @@ module ActionView end # A method which only uses the first format in the formats array for layout lookup. - # This method plays straight with instance variables for performance reasons. def with_layout_format if formats.size == 1 yield diff --git a/actionpack/lib/action_view/renderer/template_renderer.rb b/actionpack/lib/action_view/renderer/template_renderer.rb index ac91d333ba..6bf91de05a 100644 --- a/actionpack/lib/action_view/renderer/template_renderer.rb +++ b/actionpack/lib/action_view/renderer/template_renderer.rb @@ -58,14 +58,21 @@ module ActionView # context object. If no layout is found, it checks if at least a layout with # the given name exists across all details before raising the error. def find_layout(layout, keys) - begin - with_layout_format do - layout =~ /^\// ? - with_fallbacks { find_template(layout, nil, false, keys, @details) } : find_template(layout, nil, false, keys, @details) + with_layout_format { resolve_layout(layout, keys) } + end + + def resolve_layout(layout, keys) + case layout + when String + if layout =~ /^\// + with_fallbacks { find_template(layout, nil, false, keys, @details) } + else + find_template(layout, nil, false, keys, @details) end - rescue ActionView::MissingTemplate - all_details = @details.merge(:formats => @lookup_context.default_formats) - raise unless template_exists?(layout, nil, false, keys, all_details) + when Proc + resolve_layout(layout.call, keys) + else + layout end end end -- cgit v1.2.3