Merge branch 'master' of git@github.com:rails/rails

14 files changed, 129 insertions, 22 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index c469564eb5..06d73819fb 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -1,5 +1,7 @@
 *2.3.0 [Edge]*
 
+* Added support for multiple routes.rb files (useful for plugin engines). This also means that draw will no longer clear the route set, you have to do that by hand (shouldn't make a difference to you unless you're doing some funky stuff) [DHH]
+
 * Dropped formatted_* routes in favor of just passing in :format as an option. This cuts resource routes generation in half #1359 [aaronbatalion]
 
 * Remove support for old double-encoded cookies from the cookie store.  These values haven't been generated since before 2.1.0, and any users who have visited the app in the intervening 6 months will have had their cookie upgraded. [Koz]
diff --git a/actionpack/lib/action_controller/routing/route_set.rb b/actionpack/lib/action_controller/routing/route_set.rb
index 89cdf9d0f5..a9690d1807 100644
--- a/actionpack/lib/action_controller/routing/route_set.rb
+++ b/actionpack/lib/action_controller/routing/route_set.rb
@@ -200,9 +200,11 @@ module ActionController
           end
       end
 
-      attr_accessor :routes, :named_routes, :configuration_file
+      attr_accessor :routes, :named_routes, :configuration_files
 
       def initialize
+        self.configuration_files = []
+
         self.routes = []
         self.named_routes = NamedRouteCollection.new
 
@@ -216,7 +218,6 @@ module ActionController
       end
 
       def draw
-        clear!
         yield Mapper.new(self)
         install_helpers
       end
@@ -240,8 +241,22 @@ module ActionController
         routes.empty?
       end
 
+      def add_configuration_file(path)
+        self.configuration_files << path
+      end
+
+      # Deprecated accessor
+      def configuration_file=(path)
+        add_configuration_file(path)
+      end
+      
+      # Deprecated accessor
+      def configuration_file
+        configuration_files
+      end
+
       def load!
-        Routing.use_controllers! nil # Clear the controller cache so we may discover new ones
+        Routing.use_controllers!(nil) # Clear the controller cache so we may discover new ones
         clear!
         load_routes!
       end
@@ -250,24 +265,39 @@ module ActionController
       alias reload! load!
 
       def reload
-        if @routes_last_modified && configuration_file
-          mtime = File.stat(configuration_file).mtime
-          # if it hasn't been changed, then just return
-          return if mtime == @routes_last_modified
-          # if it has changed then record the new time and fall to the load! below
-          @routes_last_modified = mtime
+        if configuration_files.any? && @routes_last_modified
+          if routes_changed_at == @routes_last_modified
+            return # routes didn't change, don't reload
+          else
+            @routes_last_modified = routes_changed_at
+          end
         end
+
         load!
       end
 
       def load_routes!
-        if configuration_file
-          load configuration_file
-          @routes_last_modified = File.stat(configuration_file).mtime
+        if configuration_files.any?
+          configuration_files.each { |config| load(config) }
+          @routes_last_modified = routes_changed_at
         else
           add_route ":controller/:action/:id"
         end
       end
+      
+      def routes_changed_at
+        routes_changed_at = nil
+        
+        configuration_files.each do |config|
+          config_changed_at = File.stat(config).mtime
+
+          if routes_changed_at.nil? || config_changed_at > routes_changed_at
+            routes_changed_at = config_changed_at 
+          end
+        end
+        
+        routes_changed_at
+      end
 
       def add_route(path, options = {})
         route = builder.build(path, options)
diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb
index d62c7a1743..d5b6bd6b2a 100644
--- a/actionpack/test/controller/routing_test.rb
+++ b/actionpack/test/controller/routing_test.rb
@@ -747,12 +747,16 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do
       ActionController::Base.optimise_named_routes = true
 
       @rs = ::ActionController::Routing::RouteSet.new
-      @rs.draw {|m| m.connect ':controller/:action/:id' }
 
       ActionController::Routing.use_controllers! %w(content admin/user admin/news_feed)
     end
+    
+    def teardown
+      @rs.clear!
+    end
 
     def test_default_setup
+      @rs.draw {|m| m.connect ':controller/:action/:id' }
       assert_equal({:controller => "content", :action => 'index'}, rs.recognize_path("/content"))
       assert_equal({:controller => "content", :action => 'list'}, rs.recognize_path("/content/list"))
       assert_equal({:controller => "content", :action => 'show', :id => '10'}, rs.recognize_path("/content/show/10"))
@@ -769,6 +773,7 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do
     end
 
     def test_ignores_leading_slash
+      @rs.clear!
       @rs.draw {|m| m.connect '/:controller/:action/:id'}
       test_default_setup
     end
@@ -1002,6 +1007,8 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do
     end
 
     def test_changing_controller
+      @rs.draw {|m| m.connect ':controller/:action/:id' }
+
       assert_equal '/admin/stuff/show/10', rs.generate(
         {:controller => 'stuff', :action => 'show', :id => 10},
         {:controller => 'admin/user', :action => 'index'}
@@ -1155,10 +1162,12 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do
     end
 
     def test_action_expiry
+      @rs.draw {|m| m.connect ':controller/:action/:id' }
       assert_equal '/content', rs.generate({:controller => 'content'}, {:controller => 'content', :action => 'show'})
     end
 
     def test_recognition_with_uppercase_controller_name
+      @rs.draw {|m| m.connect ':controller/:action/:id' }
       assert_equal({:controller => "content", :action => 'index'}, rs.recognize_path("/Content"))
       assert_equal({:controller => "content", :action => 'list'}, rs.recognize_path("/ConTent/list"))
       assert_equal({:controller => "content", :action => 'show', :id => '10'}, rs.recognize_path("/CONTENT/show/10"))
@@ -2399,13 +2408,13 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do
     def setup
       routes.instance_variable_set '@routes_last_modified', nil
       silence_warnings { Object.const_set :RAILS_ROOT, '.' }
-      ActionController::Routing::Routes.configuration_file = File.join(RAILS_ROOT, 'config', 'routes.rb')
+      routes.add_configuration_file(File.join(RAILS_ROOT, 'config', 'routes.rb'))
 
       @stat = stub_everything
     end
 
     def teardown
-      ActionController::Routing::Routes.configuration_file = nil
+      ActionController::Routing::Routes.configuration_files.clear
       Object.send :remove_const, :RAILS_ROOT
     end
 
@@ -2448,12 +2457,24 @@ uses_mocha 'LegacyRouteSet, Route, RouteSet and RouteLoading' do
     end
 
     def test_load_with_configuration
-      routes.configuration_file = "foobarbaz"
+      routes.configuration_files.clear
+      routes.add_configuration_file("foobarbaz")
       File.expects(:stat).returns(@stat)
       routes.expects(:load).with("foobarbaz")
 
       routes.reload
     end
+    
+    def test_load_multiple_configurations
+      routes.add_configuration_file("engines.rb")
+      
+      File.expects(:stat).at_least_once.returns(@stat)
+
+      routes.expects(:load).with('./config/routes.rb')
+      routes.expects(:load).with('engines.rb')
+
+      routes.reload
+    end
 
     private
       def routes
diff --git a/actionpack/test/controller/url_rewriter_test.rb b/actionpack/test/controller/url_rewriter_test.rb
index bb714a0245..e9d372544e 100644
--- a/actionpack/test/controller/url_rewriter_test.rb
+++ b/actionpack/test/controller/url_rewriter_test.rb
@@ -302,6 +302,7 @@ class UrlWriterTests < ActionController::TestCase
   end
 
   def test_named_routes_with_nil_keys
+    ActionController::Routing::Routes.clear!
     add_host!
     ActionController::Routing::Routes.draw do |map|
       map.main '', :controller => 'posts'
diff --git a/railties/CHANGELOG b/railties/CHANGELOG
index 6cd57e295b..5324a7c73b 100644
--- a/railties/CHANGELOG
+++ b/railties/CHANGELOG
@@ -1,5 +1,7 @@
 *2.3.0 [Edge]*
 
+* Added that config/routes.rb files in engine plugins are automatically loaded (and reloaded when they change in dev mode) [DHH]
+
 * Added app/[models|controllers|helpers] to the load path for plugins that has an app directory (go engines ;)) [DHH]
 
 * Add config.preload_frameworks to load all frameworks at startup. Default to false so Rails autoloads itself as it's used. Turn this on for Passenger and JRuby. Also turned on by config.threadsafe!  [Jeremy Kemper]
diff --git a/railties/lib/initializer.rb b/railties/lib/initializer.rb
index 038288dc88..b0abf3379c 100644
--- a/railties/lib/initializer.rb
+++ b/railties/lib/initializer.rb
@@ -486,8 +486,13 @@ Run `rake gems:install` to install the missing gems.
     # loading module used to lazily load controllers (Configuration#controller_paths).
     def initialize_routing
       return unless configuration.frameworks.include?(:action_controller)
-      ActionController::Routing.controller_paths = configuration.controller_paths
-      ActionController::Routing::Routes.configuration_file = configuration.routes_configuration_file
+
+      ActionController::Routing.controller_paths = configuration.controller_paths + plugin_loader.controller_paths
+
+      ([ configuration.routes_configuration_file ] + plugin_loader.routing_files).each do |routing_file|
+        ActionController::Routing::Routes.add_configuration_file(routing_file)
+      end
+
       ActionController::Routing::Routes.reload
     end
 
diff --git a/railties/lib/rails/plugin.rb b/railties/lib/rails/plugin.rb
index 3b9e7dec2d..2b1e877e2b 100644
--- a/railties/lib/rails/plugin.rb
+++ b/railties/lib/rails/plugin.rb
@@ -40,7 +40,7 @@ module Rails
         load_paths << app_paths if has_app_directory?
       end.flatten
     end
-
+    
     # Evaluates a plugin's init.rb file.
     def load(initializer)
       return if loaded?
@@ -60,7 +60,26 @@ module Rails
     def about
       @about ||= load_about_information
     end
+
+    # Engines are plugins with an app/ directory.
+    def engine?
+      has_app_directory?
+    end
     
+    # Returns true if the engine ships with a routing file
+    def routed?
+      File.exist?(routing_file)
+    end
+
+    def controller_path
+      File.join(directory, 'app', 'controllers')
+    end
+
+    def routing_file
+      File.join(directory, 'config', 'routes.rb')
+    end
+    
+
     private
       def load_about_information
         about_yml_path = File.join(@directory, "about.yml")
@@ -82,7 +101,7 @@ module Rails
           File.join(directory, 'app', 'helpers')
         ]
       end
-    
+      
       def lib_path
         File.join(directory, 'lib')
       end
diff --git a/railties/lib/rails/plugin/loader.rb b/railties/lib/rails/plugin/loader.rb
index 8d7eac53c5..f08d9b74e2 100644
--- a/railties/lib/rails/plugin/loader.rb
+++ b/railties/lib/rails/plugin/loader.rb
@@ -22,6 +22,11 @@ module Rails
         @plugins ||= all_plugins.select { |plugin| should_load?(plugin) }.sort { |p1, p2| order_plugins(p1, p2) }
       end
 
+      # Returns the plugins that are in engine-form (have an app/ directory)
+      def engines
+        @engines ||= plugins.select(&:engine?)
+      end
+
       # Returns all the plugins that could be found by the current locators.
       def all_plugins
         @all_plugins ||= locate_plugins
@@ -56,7 +61,18 @@ module Rails
         end
 
         $LOAD_PATH.uniq!
-      end      
+      end
+      
+      # Returns an array of all the controller paths found inside engine-type plugins.
+      def controller_paths
+        engines.collect(&:controller_path)
+      end
+      
+      # Returns an array of routing.rb files from all the plugins that include config/routes.rb
+      def routing_files
+        plugins.select(&:routed?).collect(&:routing_file)
+      end
+      
       
       protected
       
diff --git a/railties/test/fixtures/plugins/engines/engine/app/controllers/engine_controller.rb b/railties/test/fixtures/plugins/engines/engine/app/controllers/engine_controller.rb
new file mode 100644
index 0000000000..f08373de40
--- /dev/null
+++ b/railties/test/fixtures/plugins/engines/engine/app/controllers/engine_controller.rb
@@ -0,0 +1,2 @@
+class EngineController < ActionController::Base
+end
\ No newline at end of file
diff --git a/railties/test/fixtures/plugins/engines/engine/app/models/engine_model.rb b/railties/test/fixtures/plugins/engines/engine/app/models/engine_model.rb
new file mode 100644
index 0000000000..e265712185
--- /dev/null
+++ b/railties/test/fixtures/plugins/engines/engine/app/models/engine_model.rb
@@ -0,0 +1,2 @@
+class EngineModel
+end
\ No newline at end of file
diff --git a/railties/test/fixtures/plugins/engines/engine/config/routes.rb b/railties/test/fixtures/plugins/engines/engine/config/routes.rb
new file mode 100644
index 0000000000..cca8d1b146
--- /dev/null
+++ b/railties/test/fixtures/plugins/engines/engine/config/routes.rb
@@ -0,0 +1,3 @@
+ActionController::Routing::Routes.draw do |map|
+  map.connect '/engine', :controller => "engine"
+end
diff --git a/railties/test/fixtures/plugins/engines/engine/init.rb b/railties/test/fixtures/plugins/engines/engine/init.rb
new file mode 100644
index 0000000000..f4b00c0fa4
--- /dev/null
+++ b/railties/test/fixtures/plugins/engines/engine/init.rb
@@ -0,0 +1,3 @@
+# My app/models dir must be in the load path.
+require 'engine_model'
+raise 'missing model from my app/models dir' unless defined?(EngineModel)
diff --git a/railties/test/initializer_test.rb b/railties/test/initializer_test.rb
index 88c267b58e..2104412c54 100644
--- a/railties/test/initializer_test.rb
+++ b/railties/test/initializer_test.rb
@@ -252,6 +252,7 @@ uses_mocha "Initializer plugin loading tests" do
       assert $LOAD_PATH.include?(File.join(plugin_fixture_path('default/acts/acts_as_chunky_bacon'), 'lib'))
     end
 
+
     private
 
       def load_plugins!
diff --git a/railties/test/plugin_locator_test.rb b/railties/test/plugin_locator_test.rb
index 363fa27f15..5a8c651e5a 100644
--- a/railties/test/plugin_locator_test.rb
+++ b/railties/test/plugin_locator_test.rb
@@ -47,7 +47,7 @@ uses_mocha "Plugin Locator Tests" do
     end
   
     def test_should_return_all_plugins_found_under_the_set_plugin_paths
-      assert_equal ["a", "acts_as_chunky_bacon", "gemlike", "plugin_with_no_lib_dir", "stubby"].sort, @locator.plugins.map(&:name).sort
+      assert_equal ["a", "acts_as_chunky_bacon", "engine", "gemlike", "plugin_with_no_lib_dir", "stubby"].sort, @locator.plugins.map(&:name).sort
     end
   
     def test_should_find_plugins_only_under_the_plugin_paths_set_in_configuration