From 4d910b033379727e5e7355590c50c72fc75e56db Mon Sep 17 00:00:00 2001 From: Joshua Peek Date: Wed, 26 Nov 2008 20:54:47 -0600 Subject: Super lazy load view paths in development mode (no indexing or caching at all). Switch layout finders to use view path api to take advantage of cache. --- actionpack/lib/action_controller/base.rb | 5 +- actionpack/lib/action_controller/dispatcher.rb | 1 - actionpack/lib/action_controller/layout.rb | 38 ++++-------- actionpack/lib/action_view/base.rb | 4 +- actionpack/lib/action_view/paths.rb | 67 ++++++++++++++++------ actionpack/lib/action_view/renderable.rb | 2 +- actionpack/test/abstract_unit.rb | 2 +- actionpack/test/controller/layout_test.rb | 14 ++--- .../test/template/compiled_templates_test.rb | 9 --- actionpack/test/template/render_test.rb | 17 +++++- 10 files changed, 88 insertions(+), 71 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 7e38f95076..dca66ff0a5 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -867,8 +867,9 @@ module ActionController #:nodoc: end end - response.layout = layout = pick_layout(options) - logger.info("Rendering template within #{layout}") if logger && layout + layout = pick_layout(options) + response.layout = layout.path_without_format_and_extension if layout + logger.info("Rendering template within #{layout.path_without_format_and_extension}") if logger && layout if content_type = options[:content_type] response.content_type = content_type.to_s diff --git a/actionpack/lib/action_controller/dispatcher.rb b/actionpack/lib/action_controller/dispatcher.rb index 1c7a4b0545..6e4aba2280 100644 --- a/actionpack/lib/action_controller/dispatcher.rb +++ b/actionpack/lib/action_controller/dispatcher.rb @@ -137,7 +137,6 @@ module ActionController run_callbacks :prepare_dispatch Routing::Routes.reload - ActionController::Base.view_paths.reload! ActionView::Helpers::AssetTagHelper::AssetTag::Cache.clear end diff --git a/actionpack/lib/action_controller/layout.rb b/actionpack/lib/action_controller/layout.rb index 3631ce86af..54108df06d 100644 --- a/actionpack/lib/action_controller/layout.rb +++ b/actionpack/lib/action_controller/layout.rb @@ -175,13 +175,12 @@ module ActionController #:nodoc: def default_layout(format) #:nodoc: layout = read_inheritable_attribute(:layout) return layout unless read_inheritable_attribute(:auto_layout) - @default_layout ||= {} - @default_layout[format] ||= default_layout_with_format(format, layout) - @default_layout[format] + find_layout(layout, format) end - def layout_list #:nodoc: - Array(view_paths).sum([]) { |path| Dir["#{path}/layouts/**/*"] } + def find_layout(layout, *formats) #:nodoc: + return layout if layout.respond_to?(:render) + view_paths.find_template(layout.to_s =~ /layouts\// ? layout : "layouts/#{layout}", *formats) end private @@ -189,7 +188,7 @@ module ActionController #:nodoc: inherited_without_layout(child) unless child.name.blank? layout_match = child.name.underscore.sub(/_controller$/, '').sub(/^controllers\//, '') - child.layout(layout_match, {}, true) unless child.layout_list.grep(%r{layouts/#{layout_match}(\.[a-z][0-9a-z]*)+$}).empty? + child.layout(layout_match, {}, true) if child.find_layout(layout_match, :all) end end @@ -200,15 +199,6 @@ module ActionController #:nodoc: def normalize_conditions(conditions) conditions.inject({}) {|hash, (key, value)| hash.merge(key => [value].flatten.map {|action| action.to_s})} end - - def default_layout_with_format(format, layout) - list = layout_list - if list.grep(%r{layouts/#{layout}\.#{format}(\.[a-z][0-9a-z]*)+$}).empty? - (!list.grep(%r{layouts/#{layout}\.([a-z][0-9a-z]*)+$}).empty? && format == :html) ? layout : nil - else - layout - end - end end # Returns the name of the active layout. If the layout was specified as a method reference (through a symbol), this method @@ -217,20 +207,18 @@ module ActionController #:nodoc: # weblog/standard, but layout "standard" will return layouts/standard. def active_layout(passed_layout = nil) layout = passed_layout || self.class.default_layout(default_template_format) + active_layout = case layout - when String then layout when Symbol then __send__(layout) when Proc then layout.call(self) + else layout end - # Explicitly passed layout names with slashes are looked up relative to the template root, - # but auto-discovered layouts derived from a nested controller will contain a slash, though be relative - # to the 'layouts' directory so we have to check the file system to infer which case the layout name came from. if active_layout - if active_layout.include?('/') && ! layout_directory?(active_layout) - active_layout + if layout = self.class.find_layout(active_layout, @template.template_format) + layout else - "layouts/#{active_layout}" + raise ActionView::MissingTemplate.new(self.class.view_paths, active_layout) end end end @@ -271,12 +259,6 @@ module ActionController #:nodoc: end end - def layout_directory?(layout_name) - @template.__send__(:_pick_template, "#{File.join('layouts', layout_name)}.#{@template.template_format}") ? true : false - rescue ActionView::MissingTemplate - false - end - def default_template_format response.template.template_format end diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 7697848713..da1f283deb 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -322,9 +322,7 @@ module ActionView #:nodoc: end # OPTIMIZE: Checks to lookup template in view path - if template = self.view_paths["#{template_file_name}.#{template_format}"] - template - elsif template = self.view_paths[template_file_name] + if template = self.view_paths.find_template(template_file_name, template_format) template elsif (first_render = @_render_stack.first) && first_render.respond_to?(:format_and_extension) && (template = self.view_paths["#{template_file_name}.#{first_render.format_and_extension}"]) diff --git a/actionpack/lib/action_view/paths.rb b/actionpack/lib/action_view/paths.rb index d6bf2137af..f01ed9e6e0 100644 --- a/actionpack/lib/action_view/paths.rb +++ b/actionpack/lib/action_view/paths.rb @@ -40,18 +40,10 @@ module ActionView #:nodoc: end class Path #:nodoc: - def self.eager_load_templates! - @eager_load_templates = true - end - - def self.eager_load_templates? - @eager_load_templates || false - end - attr_reader :path, :paths delegate :to_s, :to_str, :hash, :inspect, :to => :path - def initialize(path, load = true) + def initialize(path, load = false) raise ArgumentError, "path already is a Path class" if path.is_a?(Path) @path = path.freeze reload! if load @@ -65,9 +57,35 @@ module ActionView #:nodoc: 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) - raise "Unloaded view path! #{@path}" unless @loaded - @paths[path] + if loaded? + @paths[path] + else + Dir.glob("#{@path}/#{path}*").each do |file| + template = create_template(file) + if path == template.path_without_extension || path == template.path + return template + end + end + nil + end end def loaded? @@ -84,9 +102,7 @@ module ActionView #:nodoc: @paths = {} templates_in_path do |template| - # Eager load memoized methods and freeze cached template - template.freeze if self.class.eager_load_templates? - + template.freeze @paths[template.path] = template @paths[template.path_without_extension] ||= template end @@ -98,11 +114,13 @@ module ActionView #:nodoc: private def templates_in_path (Dir.glob("#{@path}/**/*/**") | Dir.glob("#{@path}/**")).each do |file| - unless File.directory?(file) - yield Template.new(file.split("#{self}/").last, self) - end + 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 @@ -121,5 +139,20 @@ module ActionView #:nodoc: end nil end + + def find_template(path, *formats) + if formats && formats.first == :all + formats = Mime::EXTENSION_LOOKUP.values.map(&:to_sym) + end + formats.each do |format| + if template = self["#{path}.#{format}"] + return template + end + end + if template = self[path] + return template + end + nil + end end end diff --git a/actionpack/lib/action_view/renderable.rb b/actionpack/lib/action_view/renderable.rb index 5ff5569db6..c04399f2c9 100644 --- a/actionpack/lib/action_view/renderable.rb +++ b/actionpack/lib/action_view/renderable.rb @@ -96,7 +96,7 @@ module ActionView # 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) - !(ActionView::PathSet::Path.eager_load_templates? && Base::CompiledTemplates.method_defined?(symbol)) + !(frozen? && Base::CompiledTemplates.method_defined?(symbol)) end end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index bee598e1ef..24fdc03507 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -30,8 +30,8 @@ ActionController::Base.logger = nil ActionController::Routing::Routes.reload rescue nil FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures') -ActionView::PathSet::Path.eager_load_templates! ActionController::Base.view_paths = FIXTURE_LOAD_PATH +ActionController::Base.view_paths.load def uses_mocha(test_name) yield diff --git a/actionpack/test/controller/layout_test.rb b/actionpack/test/controller/layout_test.rb index 61c20f8299..18c01f755c 100644 --- a/actionpack/test/controller/layout_test.rb +++ b/actionpack/test/controller/layout_test.rb @@ -3,6 +3,10 @@ require 'abstract_unit' # The view_paths array must be set on Base and not LayoutTest so that LayoutTest's inherited # method has access to the view_paths array when looking for a layout to automatically assign. old_load_paths = ActionController::Base.view_paths + +ActionView::Template::register_template_handler :mab, + lambda { |template| template.source.inspect } + ActionController::Base.view_paths = [ File.dirname(__FILE__) + '/../fixtures/layout_tests/' ] class LayoutTest < ActionController::Base @@ -31,9 +35,6 @@ end class MultipleExtensions < LayoutTest end -ActionView::Template::register_template_handler :mab, - lambda { |template| template.source.inspect } - class LayoutAutoDiscoveryTest < ActionController::TestCase def setup @request.host = "www.nextangle.com" @@ -52,10 +53,9 @@ class LayoutAutoDiscoveryTest < ActionController::TestCase end def test_third_party_template_library_auto_discovers_layout - ThirdPartyTemplateLibraryController.view_paths.reload! @controller = ThirdPartyTemplateLibraryController.new get :hello - assert_equal 'layouts/third_party_template_library', @controller.active_layout + assert_equal 'layouts/third_party_template_library.mab', @controller.active_layout.to_s assert_equal 'layouts/third_party_template_library', @response.layout assert_response :success assert_equal 'Mab', @response.body @@ -64,14 +64,14 @@ class LayoutAutoDiscoveryTest < ActionController::TestCase def test_namespaced_controllers_auto_detect_layouts @controller = ControllerNameSpace::NestedController.new get :hello - assert_equal 'layouts/controller_name_space/nested', @controller.active_layout + assert_equal 'layouts/controller_name_space/nested', @controller.active_layout.to_s assert_equal 'controller_name_space/nested.rhtml hello.rhtml', @response.body end def test_namespaced_controllers_auto_detect_layouts @controller = MultipleExtensions.new get :hello - assert_equal 'layouts/multiple_extensions', @controller.active_layout + assert_equal 'layouts/multiple_extensions.html.erb', @controller.active_layout.to_s assert_equal 'multiple_extensions.html.erb hello.rhtml', @response.body.strip end end diff --git a/actionpack/test/template/compiled_templates_test.rb b/actionpack/test/template/compiled_templates_test.rb index f7688b2072..0b3d039409 100644 --- a/actionpack/test/template/compiled_templates_test.rb +++ b/actionpack/test/template/compiled_templates_test.rb @@ -30,15 +30,6 @@ uses_mocha 'TestTemplateRecompilation' do assert_equal "Hello world!", render(:file => "test/hello_world.erb") end - def test_compiled_template_will_always_be_recompiled_when_eager_loaded_templates_is_off - ActionView::PathSet::Path.expects(:eager_load_templates?).times(4).returns(false) - 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 - private def render(*args) ActionView::Base.new(ActionController::Base.view_paths, {}).render(*args) diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index b316d5c23e..28d38b0c76 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -4,7 +4,9 @@ require 'controller/fake_models' class ViewRenderTest < Test::Unit::TestCase def setup @assigns = { :secret => 'in the sauce' } - @view = ActionView::Base.new(ActionController::Base.view_paths, @assigns) + view_paths = ActionController::Base.view_paths + @view = ActionView::Base.new(view_paths, @assigns) + assert view_paths.first.loaded? end def test_render_file @@ -157,7 +159,7 @@ class ViewRenderTest < Test::Unit::TestCase end def test_render_fallbacks_to_erb_for_unknown_types - assert_equal "Hello, World!", @view.render(:inline => "Hello, World!", :type => :foo) + assert_equal "Hello, World!", @view.render(:inline => "Hello, World!", :type => :bar) end CustomHandler = lambda do |template| @@ -196,3 +198,14 @@ class ViewRenderTest < Test::Unit::TestCase @view.render(:file => "test/nested_layout.erb", :layout => "layouts/yield") end end + +class LazyViewRenderTest < ViewRenderTest + # Test the same thing as above, but make sure the view path + # is not eager loaded + def setup + @assigns = { :secret => 'in the sauce' } + view_paths = ActionView::Base.process_view_paths(FIXTURE_LOAD_PATH) + @view = ActionView::Base.new(view_paths, @assigns) + assert !view_paths.first.loaded? + end +end -- cgit v1.2.3