Allow Controllers to have multiple view_paths instead of a single template_root. Closes #2754 [John Long]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@6120 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Allow Controllers to have multiple view_paths instead of a single template_root.  Closes #2754 [John Long]
+
 * Add much-needed html-scanner tests.  Fixed CDATA parsing bug. [Rick]
 
 * improve error message for Routing for named routes.  Closes #7346 [Rob Sanheim]

actionpack/README

@@ -382,7 +382,7 @@ methods:
     end
   end
 
-  WeblogController::Base.template_root = File.dirname(__FILE__)
+  WeblogController::Base.view_paths = [ File.dirname(__FILE__) ]
   WeblogController.process_cgi if $0 == __FILE__
 
 The last two lines are responsible for telling ActionController where the

actionpack/examples/address_book_controller.rb

@@ -42,7 +42,7 @@ class AddressBookController < ActionController::Base
     end
 end
 
-ActionController::Base.template_root = File.dirname(__FILE__)
+ActionController::Base.view_paths = [ File.dirname(__FILE__) ]
 # ActionController::Base.logger = Logger.new("debug.log") # Remove first comment to turn on logging in current dir
 
 begin

actionpack/examples/benchmark.rb

@@ -24,7 +24,7 @@ class BenchmarkController < ActionController::Base
   end
 end
 
-#ActionController::Base.template_root = File.dirname(__FILE__)
+#ActionController::Base.view_paths = [ File.dirname(__FILE__) ]
 
 require "benchmark"
 

actionpack/examples/blog_controller.cgi

@@ -43,7 +43,7 @@ class BlogController < ActionController::Base
     end
 end
 
-ActionController::Base.template_root = File.dirname(__FILE__)
+ActionController::Base.view_paths = [ File.dirname(__FILE__) ]
 # ActionController::Base.logger = Logger.new("debug.log") # Remove first comment to turn on logging in current dir
 
 begin

actionpack/examples/debate_controller.cgi

@@ -47,7 +47,7 @@ class DebateController < ActionController::Base
     end
 end
 
-ActionController::Base.template_root = File.dirname(__FILE__)
+ActionController::Base.view_paths = [ File.dirname(__FILE__) ]
 # ActionController::Base.logger = Logger.new("debug.log") # Remove first comment to turn on logging in current dir
 
 begin

actionpack/lib/action_controller/base.rb

@@ -277,11 +277,7 @@ module ActionController #:nodoc:
     # Controls the default charset for all renders.
     @@default_charset = "utf-8"
     cattr_accessor :default_charset
-
-    # Template root determines the base from which template references will be made. So a call to render("test/template")
-    # will be converted to "#{template_root}/test/template.rhtml".
-    class_inheritable_accessor :template_root
-
+    
     # The logger is used for generating information on the action run-time (including benchmarking) if available.
     # Can be set to nil for no logging. Compatible with both Ruby's own Logger and Log4r loggers.
     cattr_accessor :logger
@@ -357,7 +353,41 @@ module ActionController #:nodoc:
       def hide_action(*names)
         write_inheritable_attribute(:hidden_actions, hidden_actions | names.collect { |n| n.to_s })
       end
-
+      
+      # Deprecated. Use view_paths instead.
+      def template_root=(path)
+        view_paths.unshift(path)
+      end
+      deprecate :template_root= => :view_paths
+      
+      # Deprecated. Use view_paths instead.
+      def template_root
+        view_paths.first
+      end
+      deprecate :template_root => :view_paths
+      
+      @@view_paths = {}
+      
+      # View load paths determine the bases from which template references can be made. So a call to
+      # render("test/template") will be looked up in the view load paths array and the closest match will be
+      # returned.
+      def view_paths=(value)
+        @@view_paths[name] = value
+      end
+      
+      # View load paths for controller.
+      def view_paths
+        if paths = @@view_paths[name]
+          paths
+        else
+          if superclass.respond_to?(:view_paths)
+            superclass.view_paths.dup.freeze
+          else
+            @@view_paths[name] = []
+          end
+        end
+      end
+      
       # Replace sensitive paramater data from the request log.
       # Filters paramaters that have any of the arguments as a substring.
       # Looks in all subhashes of the param hash for keys to filter.
@@ -534,10 +564,15 @@ module ActionController #:nodoc:
       def controller_path
         self.class.controller_path
       end
-
+      
       def session_enabled?
         request.session_options[:disabled] != false
       end
+      
+      # View load paths for controller.
+      def view_paths
+        self.class.view_paths
+      end
 
     protected
       # Renders the content that will be returned to the browser as the response body.
@@ -1030,14 +1065,10 @@ module ActionController #:nodoc:
           end
       end
 
-      def self.view_root
-        @view_root ||= template_root
-      end
-
       def initialize_template_class(response)
         raise "You must assign a template class through ActionController.template_class= before processing a request" unless @@template_class
 
-        response.template = self.class.view_class.new(self.class.view_root, {}, self)
+        response.template = self.class.view_class.new(view_paths, {}, self)
         response.redirected_to = nil
         @performed_render = @performed_redirect = false
       end
@@ -1057,7 +1088,6 @@ module ActionController #:nodoc:
         assign_deprecated_shortcuts(request, response)
       end
 
-
       # TODO: assigns cookies headers params request response template
       DEPRECATED_INSTANCE_VARIABLES = %w(cookies flash headers params request response session)
 
@@ -1151,7 +1181,7 @@ module ActionController #:nodoc:
       end
 
       def add_class_variables_to_assigns
-        %w(template_root logger template_class ignore_missing_templates).each do |cvar|
+        %w(view_paths logger template_class ignore_missing_templates).each do |cvar|
           @assigns[cvar] = self.send(cvar)
         end
       end

actionpack/lib/action_controller/layout.rb

@@ -179,16 +179,18 @@ module ActionController #:nodoc:
       def default_layout #:nodoc:
         @default_layout ||= read_inheritable_attribute("layout")
       end
-
+      
+      def layout_list #:nodoc:
+        view_paths.collect do |path|
+          Dir["#{path}/layouts/**/*"]
+        end.flatten
+      end
+      
       private
         def inherited_with_layout(child)
           inherited_without_layout(child)
           layout_match = child.name.underscore.sub(/_controller$/, '').sub(/^controllers\//, '')
-          child.layout(layout_match) unless layout_list.grep(%r{layouts/#{layout_match}\.[a-z][0-9a-z]*$}).empty?
-        end
-
-        def layout_list
-          Dir.glob("#{template_root}/layouts/**/*")
+          child.layout(layout_match) unless child.layout_list.grep(%r{layouts/#{layout_match}\.[a-z][0-9a-z]*$}).empty?
         end
 
         def add_layout_conditions(conditions)
@@ -305,7 +307,10 @@ module ActionController #:nodoc:
       # Does a layout directory for this class exist?
       # we cache this info in a class level hash
       def layout_directory?(layout_name)
-        template_path = File.join(self.class.view_root, 'layouts', layout_name)
+        view_paths.find do |path|
+          File.file?(File.join(path, 'layouts', layout_name))
+        end
+        template_path ||= File.join(view_paths.first, 'layouts', layout_name)
         dirname = File.dirname(template_path)
         self.class.send(:layout_directory_exists_cache)[dirname]
       end

actionpack/lib/action_view/base.rb

@@ -232,15 +232,16 @@ module ActionView #:nodoc:
       @@template_handlers[extension] = klass
     end
 
-    def initialize(base_path = nil, assigns_for_first_render = {}, controller = nil)#:nodoc:
-      @base_path, @assigns = base_path, assigns_for_first_render
+    def initialize(view_paths = [], assigns_for_first_render = {}, controller = nil)#:nodoc:
+      @view_paths = [*view_paths].compact
+      @assigns = assigns_for_first_render
       @assigns_added = nil
       @controller = controller
       @logger = controller && controller.logger 
     end
 
     # Renders the template present at <tt>template_path</tt>. If <tt>use_full_path</tt> is set to true, 
-    # it's relative to the template_root, otherwise it's absolute. The hash in <tt>local_assigns</tt> 
+    # it's relative to the view_paths array, otherwise it's absolute. The hash in <tt>local_assigns</tt> 
     # is made available as local variables.
     def render_file(template_path, use_full_path = true, local_assigns = {}) #:nodoc:
       @first_render ||= template_path
@@ -268,12 +269,12 @@ module ActionView #:nodoc:
           e.sub_template_of(template_file_name)
           raise e
         else
-          raise TemplateError.new(@base_path, template_file_name, @assigns, template_source, e)
+          raise TemplateError.new(find_base_path_for(template_file_name), template_file_name, @assigns, template_source, e)
         end
       end
     end
-
-    # Renders the template present at <tt>template_path</tt> (relative to the template_root). 
+    
+    # Renders the template present at <tt>template_path</tt> (relative to the view_paths array). 
     # The hash in <tt>local_assigns</tt> is made available as local variables.
     def render(options = {}, old_local_assigns = {}, &block) #:nodoc:
       if options.is_a?(String)
@@ -358,12 +359,11 @@ module ActionView #:nodoc:
 
     def file_exists?(template_path)#:nodoc:
       template_file_name, template_file_extension = path_and_extension(template_path)
-
       if template_file_extension
         template_exists?(template_file_name, template_file_extension)
       else
         cached_template_extension(template_path) ||
-           %w(erb builder javascript delegate).any? do |template_type| 
+           %w(erb builder javascript delegate).any? do |template_type|
              send("#{template_type}_template_exists?", template_path)
            end
       end
@@ -376,7 +376,9 @@ module ActionView #:nodoc:
 
     private
       def full_template_path(template_path, extension)
-        "#{@base_path}/#{template_path}.#{extension}"
+        file_name = "#{template_path}.#{extension}"
+        base_path = find_base_path_for(file_name)
+        "#{base_path}/#{file_name}"
       end
 
       def template_exists?(template_path, extension)
@@ -392,7 +394,11 @@ module ActionView #:nodoc:
       def cached_template_extension(template_path)
         @@cache_template_extensions && @@cached_template_extension[template_path]
       end
-
+      
+      def find_base_path_for(template_file_name)
+        @view_paths.find { |p| File.file?(File.join(p, template_file_name)) }
+      end
+      
       def find_template_extension_for(template_path)
         if match = delegate_template_exists?(template_path)
           match.first.to_sym
@@ -400,7 +406,7 @@ module ActionView #:nodoc:
         elsif builder_template_exists?(template_path):    :rxml
         elsif javascript_template_exists?(template_path): :rjs
         else
-          raise ActionViewError, "No rhtml, rxml, rjs or delegate template found for #{template_path} in #{@base_path}"
+          raise ActionViewError, "No rhtml, rxml, rjs or delegate template found for #{template_path} in #{@view_paths.inspect}"
         end
       end
 
@@ -536,7 +542,7 @@ module ActionView #:nodoc:
             logger.debug "Backtrace: #{e.backtrace.join("\n")}"
           end
 
-          raise TemplateError.new(@base_path, file_name || template, @assigns, template, e)
+          raise TemplateError.new(lookup_template_base_path_for(file_name || template), file_name || template, @assigns, template, e)
         end
 
         @@compile_time[render_symbol] = Time.now
@@ -545,4 +551,4 @@ module ActionView #:nodoc:
   end
 end
 
-require 'action_view/template_error'
+require 'action_view/template_error'

actionpack/test/activerecord/pagination_test.rb

@@ -4,7 +4,7 @@ class PaginationTest < ActiveRecordTestCase
   fixtures :topics, :replies, :developers, :projects, :developers_projects
   
   class PaginationController < ActionController::Base
-    self.template_root = "#{File.dirname(__FILE__)}/../fixtures/"
+    self.view_paths = [ "#{File.dirname(__FILE__)}/../fixtures/" ]
     
     def simple_paginate
       @topic_pages, @topics = paginate(:topics)

actionpack/test/controller/action_pack_assertions_test.rb

@@ -152,7 +152,7 @@ end
 # tell the controller where to find its templates but start from parent
 # directory of test_request_response to simulate the behaviour of a
 # production environment
-ActionPackAssertionsController.template_root = File.dirname(__FILE__) + "/../fixtures/"
+ActionPackAssertionsController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
 
 # a test case to exercise the new capabilities TestRequest & TestResponse
 class ActionPackAssertionsControllerTest < Test::Unit::TestCase

actionpack/test/controller/addresses_render_test.rb

@@ -22,7 +22,7 @@ class AddressesTestController < ActionController::Base
   def self.controller_path; "addresses"; end
 end
 
-AddressesTestController.template_root = File.dirname(__FILE__) + "/../fixtures/"
+AddressesTestController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
 
 class AddressesTest < Test::Unit::TestCase
   def setup

actionpack/test/controller/capture_test.rb

@@ -23,7 +23,7 @@ class CaptureController < ActionController::Base
   def rescue_action(e) raise end
 end
 
-CaptureController.template_root = File.dirname(__FILE__) + "/../fixtures/"
+CaptureController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
 
 class CaptureTest < Test::Unit::TestCase
   def setup

actionpack/test/controller/content_type_test.rb

@@ -45,7 +45,7 @@ class ContentTypeController < ActionController::Base
   def rescue_action(e) raise end
 end
 
-ContentTypeController.template_root = File.dirname(__FILE__) + "/../fixtures/"
+ContentTypeController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
 
 class ContentTypeTest < Test::Unit::TestCase
   def setup

actionpack/test/controller/deprecation/deprecated_base_methods_test.rb

@@ -21,7 +21,7 @@ class DeprecatedBaseMethodsTest < Test::Unit::TestCase
     def rescue_action(e) raise e end
   end
 
-  Target.template_root = File.dirname(__FILE__) + "/../../fixtures"
+  Target.view_paths = [ File.dirname(__FILE__) + "/../../fixtures" ]
 
   def setup
     @request    = ActionController::TestRequest.new

actionpack/test/controller/layout_test.rb

@@ -1,15 +1,17 @@
 require File.dirname(__FILE__) + '/../abstract_unit'
 
-# The template_root must be set on Base and not LayoutTest so that LayoutTest's inherited method has access to
-# the template_root when looking for a layout
-ActionController::Base.template_root = File.dirname(__FILE__) + '/../fixtures/layout_tests/'
+# 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
+ActionController::Base.view_paths = [ File.dirname(__FILE__) + '/../fixtures/layout_tests/' ]
 
 class LayoutTest < ActionController::Base
   def self.controller_path; 'views' end
+  self.view_paths = ActionController::Base.view_paths.dup
 end
 
-# Restore template root to be unset
-ActionController::Base.template_root = nil
+# Restore view_paths to previous value
+ActionController::Base.view_paths = old_load_paths
 
 class ProductController < LayoutTest
 end

actionpack/test/controller/mime_responds_test.rb

@@ -118,7 +118,7 @@ class RespondToController < ActionController::Base
     end
 end
 
-RespondToController.template_root = File.dirname(__FILE__) + "/../fixtures/"
+RespondToController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
 
 class MimeControllerTest < Test::Unit::TestCase
   def setup

actionpack/test/controller/new_render_test.rb

@@ -350,8 +350,8 @@ class NewRenderTestController < ActionController::Base
     end
 end
 
-NewRenderTestController.template_root = File.dirname(__FILE__) + "/../fixtures/"
-Fun::GamesController.template_root = File.dirname(__FILE__) + "/../fixtures/"
+NewRenderTestController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
+Fun::GamesController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
 
 class NewRenderTest < Test::Unit::TestCase
   def setup

actionpack/test/controller/render_test.rb

@@ -134,8 +134,8 @@ class TestController < ActionController::Base
     end
 end
 
-TestController.template_root = File.dirname(__FILE__) + "/../fixtures/"
-Fun::GamesController.template_root = File.dirname(__FILE__) + "/../fixtures/"
+TestController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
+Fun::GamesController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
 
 class RenderTest < Test::Unit::TestCase
   def setup

actionpack/test/controller/send_file_test.rb

@@ -21,7 +21,7 @@ class SendFileController < ActionController::Base
   def rescue_action(e) raise end
 end
 
-SendFileController.template_root = File.dirname(__FILE__) + "/../fixtures/"
+SendFileController.view_paths = [ File.dirname(__FILE__) + "/../fixtures/" ]
 
 class SendFileTest < Test::Unit::TestCase
   include TestFileUtils

actionpack/test/controller/view_paths_test.rb

@@ -0,0 +1,78 @@
+require File.dirname(__FILE__) + '/../abstract_unit'
+
+class ViewLoadPathsTest < Test::Unit::TestCase
+  
+  LOAD_PATH_ROOT = File.join(File.dirname(__FILE__), '..', 'fixtures')
+
+  class TestController < ActionController::Base
+    def self.controller_path() "test" end
+    def rescue_action(e) raise end
+      
+    def hello_world() end
+  end
+  
+  def setup
+    TestController.view_paths = [ LOAD_PATH_ROOT ]
+    @controller = TestController.new
+    @request  = ActionController::TestRequest.new
+    @response = ActionController::TestResponse.new
+  
+    # Track the last warning.
+    @old_behavior = ActiveSupport::Deprecation.behavior
+    @last_message = nil
+    ActiveSupport::Deprecation.behavior = Proc.new { |message| @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
+  end
+  
+  def test_view_paths
+    get :hello_world
+    assert_response :success
+    assert_equal "Hello world!", @response.body
+  end
+  
+  def test_view_paths_override
+    TestController.view_paths.unshift "#{LOAD_PATH_ROOT}/override"
+    get :hello_world
+    assert_response :success
+    assert_equal "Hello overridden world!", @response.body
+  end
+  
+  def test_template_root_deprecated
+    assert_deprecated(/template_root.*view_paths/) do
+      TestController.template_root = LOAD_PATH_ROOT
+    end
+    assert_deprecated(/template_root.*view_paths/) do
+      assert_equal LOAD_PATH_ROOT, TestController.template_root
+    end
+  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
+    assert_equal original_load_paths, C.view_paths
+    
+    e = assert_raises(TypeError) { C.view_paths << 'c/path' }
+    assert_equal "can't modify frozen array", e.message
+    
+    C.view_paths = []
+    assert_nothing_raised { C.view_paths << 'c/path' }
+  end
+  
+end

actionpack/test/fixtures/override/test/hello_world.rhtml

@@ -0,0 +1 @@
+Hello overridden world!

actionpack/test/template/deprecated_instance_variables_test.rb

@@ -2,7 +2,7 @@ require "#{File.dirname(__FILE__)}/../abstract_unit"
 
 class DeprecatedViewInstanceVariablesTest < Test::Unit::TestCase
   class DeprecatedInstanceVariablesController < ActionController::Base
-    self.template_root = "#{File.dirname(__FILE__)}/../fixtures/"
+    self.view_paths = [ "#{File.dirname(__FILE__)}/../fixtures/" ]
 
     def self.controller_path; 'deprecated_instance_variables' end
 

actionpack/test/template/url_helper_test.rb

@@ -250,7 +250,7 @@ end
 
 class UrlHelperWithControllerTest < Test::Unit::TestCase
   class UrlHelperController < ActionController::Base
-    self.template_root = "#{File.dirname(__FILE__)}/../fixtures/"
+    self.view_paths = [ "#{File.dirname(__FILE__)}/../fixtures/" ]
 
     def self.controller_path; 'url_helper_with_controller' end
 
@@ -305,7 +305,7 @@ end
 
 class LinkToUnlessCurrentWithControllerTest < Test::Unit::TestCase
   class TasksController < ActionController::Base
-    self.template_root = "#{File.dirname(__FILE__)}/../fixtures/"
+    self.view_paths = ["#{File.dirname(__FILE__)}/../fixtures/"]
 
     def self.controller_path; 'tasks' end