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

16 files changed, 103 insertions, 91 deletions

diff --git a/railties/lib/generators/rails/app/templates/app/controllers/application_controller.rb b/railties/lib/generators/rails/app/templates/app/controllers/application_controller.rb
index 2cdf4eae54..e8065d9505 100644
--- a/railties/lib/generators/rails/app/templates/app/controllers/application_controller.rb
+++ b/railties/lib/generators/rails/app/templates/app/controllers/application_controller.rb
@@ -1,4 +1,3 @@
 class ApplicationController < ActionController::Base
   protect_from_forgery
-  filter_parameter_logging :password
 end
diff --git a/railties/lib/generators/rails/app/templates/config/application.rb b/railties/lib/generators/rails/app/templates/config/application.rb
index 334820826f..78a355d2f4 100644
--- a/railties/lib/generators/rails/app/templates/config/application.rb
+++ b/railties/lib/generators/rails/app/templates/config/application.rb
@@ -30,5 +30,8 @@ module <%= app_const_base %>
     #   g.template_engine :erb
     #   g.test_framework  :test_unit, :fixture => true
     # end
+
+    # Configure sensitive parameters which will be filtered from the log file.
+    config.filter_parameters << :password
   end
 end
diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb
index 743681359c..5c9892c630 100644
--- a/railties/lib/rails/application.rb
+++ b/railties/lib/rails/application.rb
@@ -110,6 +110,7 @@ module Rails
     end
 
     def call(env)
+      env["action_dispatch.parameter_filter"] = config.filter_parameters
       app.call(env)
     end
 
diff --git a/railties/lib/rails/commands/server.rb b/railties/lib/rails/commands/server.rb
index 115499db05..b21ae2a17b 100644
--- a/railties/lib/rails/commands/server.rb
+++ b/railties/lib/rails/commands/server.rb
@@ -46,7 +46,6 @@ module Rails
       trap(:INT) { exit }
       puts "=> Ctrl-C to shutdown server" unless options[:daemonize]
 
-      initialize_log_tailer! unless options[:daemonize]
       super
     ensure
       puts 'Exiting' unless options[:daemonize]
@@ -54,6 +53,7 @@ module Rails
 
     def middleware
       middlewares = []
+      middlewares << [Rails::Rack::LogTailer, log_path] unless options[:daemonize]
       middlewares << [Rails::Rack::Debugger]  if options[:debugger]
       Hash.new(middlewares)
     end
@@ -71,14 +71,5 @@ module Rails
         :pid         => "tmp/pids/server.pid"
       })
     end
-
-  protected
-
-    # LogTailer should not be used as a middleware since the logging happens
-    # async in a request and the middleware calls are sync. So we send it
-    # to subscriber which will be responsible for calling tail! in the log tailer.
-    def initialize_log_tailer!
-      Rails::Subscriber.log_tailer = Rails::Rack::LogTailer.new(nil, log_path)
-    end
   end
 end
diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb
index b76a7ac2d8..7f1783a6b9 100644
--- a/railties/lib/rails/configuration.rb
+++ b/railties/lib/rails/configuration.rb
@@ -10,15 +10,15 @@ module Rails
 
     def self.default_middleware_stack
       ActionDispatch::MiddlewareStack.new.tap do |middleware|
-        middleware.use('ActionDispatch::Static', lambda { Rails.public_path }, :if => lambda { Rails.application.config.serve_static_assets })
+        middleware.use('::ActionDispatch::Static', lambda { Rails.public_path }, :if => lambda { Rails.application.config.serve_static_assets })
         middleware.use('::Rack::Lock', :if => lambda { !ActionController::Base.allow_concurrency })
         middleware.use('::Rack::Runtime')
-        middleware.use('ActionDispatch::ShowExceptions', lambda { ActionController::Base.consider_all_requests_local })
-        middleware.use('ActionDispatch::Notifications')
-        middleware.use('ActionDispatch::Callbacks', lambda { !Rails.application.config.cache_classes })
-        middleware.use('ActionDispatch::Cookies')
+        middleware.use('::Rails::Rack::Logger')
+        middleware.use('::ActionDispatch::ShowExceptions', lambda { ActionController::Base.consider_all_requests_local })
+        middleware.use('::ActionDispatch::Callbacks', lambda { !Rails.application.config.cache_classes })
+        middleware.use('::ActionDispatch::Cookies')
         middleware.use(lambda { ActionController::Base.session_store }, lambda { ActionController::Base.session_options })
-        middleware.use('ActionDispatch::Flash', :if => lambda { ActionController::Base.session_store })
+        middleware.use('::ActionDispatch::Flash', :if => lambda { ActionController::Base.session_store })
         middleware.use(lambda { Rails::Rack::Metal.new(Rails.application.config.paths.app.metals.to_a, Rails.application.config.metals) })
         middleware.use('ActionDispatch::ParamsParser')
         middleware.use('::Rack::MethodOverride')
@@ -69,7 +69,7 @@ module Rails
 
   class Configuration < Railtie::Configuration
     attr_accessor :after_initialize_blocks, :cache_classes, :colorize_logging,
-                  :consider_all_requests_local, :dependency_loading,
+                  :consider_all_requests_local, :dependency_loading, :filter_parameters,
                   :load_once_paths, :logger, :metals, :plugins,
                   :preload_frameworks, :reload_plugins, :serve_static_assets,
                   :time_zone, :whiny_nils
@@ -83,6 +83,7 @@ module Rails
       super
       @load_once_paths              = []
       @after_initialize_blocks      = []
+      @filter_parameters            = []
       @dependency_loading           = true
       @serve_static_assets          = true
     end
diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb
index d3175e6a9d..83b8c74966 100644
--- a/railties/lib/rails/generators.rb
+++ b/railties/lib/rails/generators.rb
@@ -134,9 +134,14 @@ module Rails
       lookups = []
       lookups << "#{base}:#{name}"    if base
       lookups << "#{name}:#{context}" if context
-      lookups << "#{name}:#{name}"    unless name.to_s.include?(?:)
-      lookups << "#{name}"
-      lookups << "rails:#{name}"      unless base || context || name.to_s.include?(?:)
+
+      unless base || context
+        unless name.to_s.include?(?:)
+          lookups << "#{name}:#{name}"
+          lookups << "rails:#{name}"
+        end
+        lookups << "#{name}"
+      end
 
       lookup(lookups)
 
@@ -232,9 +237,9 @@ module Rails
         load_generators_from_railties!
         paths = namespaces_to_paths(namespaces)
 
-        paths.each do |path|
-          ["generators", "rails_generators"].each do |base|
-            path = "#{base}/#{path}_generator"
+        paths.each do |raw_path|
+          ["rails_generators", "generators"].each do |base|
+            path = "#{base}/#{raw_path}_generator"
 
             begin
               require path
@@ -243,7 +248,9 @@ module Rails
               raise unless e.message =~ /#{Regexp.escape(path)}$/
             rescue NameError => e
               raise unless e.message =~ /Rails::Generator([\s(::)]|$)/
-              warn "[WARNING] Could not load generator #{path.inspect} because it's a Rails 2.x generator, which is not supported anymore. Error: #{e.message}"
+              warn "[WARNING] Could not load generator #{path.inspect} because it's a Rails 2.x generator, which is not supported anymore. Error: #{e.message}.\n#{e.backtrace.join("\n")}"
+            rescue Exception => e
+              warn "[WARNING] Could not load generator #{path.inspect}. Error: #{e.message}.\n#{e.backtrace.join("\n")}"
             end
           end
         end
@@ -278,7 +285,7 @@ module Rails
         paths = []
         namespaces.each do |namespace|
           pieces = namespace.split(":")
-          paths << pieces.dup.push(pieces.last).join("/")
+          paths << pieces.dup.push(pieces.last).join("/") unless pieces.uniq.size == 1
           paths << pieces.join("/")
         end
         paths.uniq!
diff --git a/railties/lib/rails/rack.rb b/railties/lib/rails/rack.rb
index 36945a6b0f..4bc0c2c88b 100644
--- a/railties/lib/rails/rack.rb
+++ b/railties/lib/rails/rack.rb
@@ -1,6 +1,7 @@
 module Rails
   module Rack
     autoload :Debugger,  "rails/rack/debugger"
+    autoload :Logger,    "rails/rack/logger"
     autoload :LogTailer, "rails/rack/log_tailer"
     autoload :Metal,     "rails/rack/metal"
     autoload :Static,    "rails/rack/static"
diff --git a/railties/lib/rails/rack/logger.rb b/railties/lib/rails/rack/logger.rb
new file mode 100644
index 0000000000..91a613092f
--- /dev/null
+++ b/railties/lib/rails/rack/logger.rb
@@ -0,0 +1,38 @@
+require 'rails/subscriber'
+
+module Rails
+  module Rack
+    # Log the request started and flush all loggers after it.
+    class Logger < Rails::Subscriber
+      def initialize(app)
+        @app = app
+      end
+
+      def call(env)
+        @env = env
+        before_dispatch
+        result = @app.call(@env)
+        after_dispatch
+        result
+      end
+
+      protected
+
+        def request
+          @request ||= ActionDispatch::Request.new(@env) 
+        end
+
+        def before_dispatch
+          path = request.request_uri.inspect rescue "unknown"
+
+          info "\n\nStarted #{request.method.to_s.upcase} #{path} " <<
+                      "for #{request.remote_ip} at #{Time.now.to_s(:db)}"
+        end
+
+        def after_dispatch
+          Rails::Subscriber.flush_all!
+        end
+
+    end
+  end
+end
diff --git a/railties/lib/rails/subscriber.rb b/railties/lib/rails/subscriber.rb
index 9965786d86..8c62f562d9 100644
--- a/railties/lib/rails/subscriber.rb
+++ b/railties/lib/rails/subscriber.rb
@@ -33,7 +33,7 @@ module Rails
   # Subscriber also has some helpers to deal with logging and automatically flushes
   # all logs when the request finishes (via action_dispatch.callback notification).
   class Subscriber
-    mattr_accessor :colorize_logging, :log_tailer
+    mattr_accessor :colorize_logging
     self.colorize_logging = true
 
     # Embed in a String to clear all previous ANSI sequences.
@@ -69,11 +69,6 @@ module Rails
           Rails.logger.error "Could not log #{args[0].inspect} event. #{e.class}: #{e.message}"
         end
       end
-
-      if args[0] == "action_dispatch.after_dispatch" && !subscribers.empty?
-        flush_all!
-        log_tailer.tail! if log_tailer
-      end
     end
 
     # Flush all subscribers' logger.
diff --git a/railties/lib/rails/subscriber/test_helper.rb b/railties/lib/rails/subscriber/test_helper.rb
index 1464767ed9..39b4117372 100644
--- a/railties/lib/rails/subscriber/test_helper.rb
+++ b/railties/lib/rails/subscriber/test_helper.rb
@@ -1,12 +1,12 @@
 require 'rails/subscriber'
-require 'active_support/notifications'
 
 module Rails
   class Subscriber
     # Provides some helpers to deal with testing subscribers by setting up
     # notifications. Take for instance ActiveRecord subscriber tests:
     #
-    #   module SubscriberTest
+    #   class SyncSubscriberTest < ActiveSupport::TestCase
+    #     include Rails::Subscriber::TestHelper
     #     Rails::Subscriber.add(:active_record, ActiveRecord::Railties::Subscriber.new)
     # 
     #     def test_basic_query_logging
@@ -39,8 +39,6 @@ module Rails
     #
     module TestHelper
       def setup
-        Thread.abort_on_exception = true
-
         @logger   = MockLogger.new
         @notifier = ActiveSupport::Notifications::Notifier.new(queue)
 
@@ -54,7 +52,6 @@ module Rails
       def teardown
         set_logger(nil)
         ActiveSupport::Notifications.notifier = nil
-        Thread.abort_on_exception = false
       end
 
       class MockLogger
@@ -92,26 +89,9 @@ module Rails
       def set_logger(logger)
         Rails.logger = logger
       end
-    end
-    
-    module SyncTestHelper
-      include TestHelper
-
-      def queue
-        ActiveSupport::Notifications::Fanout.new(true)
-      end
-    end
-
-    module AsyncTestHelper
-      include TestHelper
 
       def queue
-        ActiveSupport::Notifications::Fanout.new(false)
-      end
-
-      def wait
-        sleep(0.01)
-        super
+        ActiveSupport::Notifications::Fanout.new
       end
     end
   end
diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb
index 79dfacdcdb..6968e87986 100644
--- a/railties/test/application/configuration_test.rb
+++ b/railties/test/application/configuration_test.rb
@@ -122,5 +122,17 @@ module ApplicationTests
         require "#{app_path}/config/environment"
       end
     end
+    
+    test "filter_parameters should be able to set via config.filter_parameters" do
+      add_to_config <<-RUBY
+        config.filter_parameters += [ :foo, 'bar', lambda { |key, value|
+          value = value.reverse if key =~ /baz/
+        }]
+      RUBY
+      
+      assert_nothing_raised do
+        require "#{app_path}/config/application"
+      end
+    end
   end
 end
diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb
index 1c5cc62ecd..8c4247e840 100644
--- a/railties/test/application/middleware_test.rb
+++ b/railties/test/application/middleware_test.rb
@@ -17,8 +17,8 @@ module ApplicationTests
         "ActionDispatch::Static",
         "Rack::Lock",
         "Rack::Runtime",
+        "Rails::Rack::Logger",
         "ActionDispatch::ShowExceptions",
-        "ActionDispatch::Notifications",
         "ActionDispatch::Callbacks",
         "ActionDispatch::Cookies",
         "ActionDispatch::Session::CookieStore",
diff --git a/railties/test/fixtures/lib/generators/model_generator.rb b/railties/test/fixtures/lib/generators/model_generator.rb
new file mode 100644
index 0000000000..9098a8a354
--- /dev/null
+++ b/railties/test/fixtures/lib/generators/model_generator.rb
@@ -0,0 +1 @@
+raise "I should never be loaded"
\ No newline at end of file
diff --git a/railties/test/fixtures/lib/generators/foobar/foobar_generator.rb b/railties/test/fixtures/lib/rails_generators/foobar/foobar_generator.rb
index d1de8c56fa..d1de8c56fa 100644
--- a/railties/test/fixtures/lib/generators/foobar/foobar_generator.rb
+++ b/railties/test/fixtures/lib/rails_generators/foobar/foobar_generator.rb
diff --git a/railties/test/generators_test.rb b/railties/test/generators_test.rb
index 664d1e5670..f37b684f73 100644
--- a/railties/test/generators_test.rb
+++ b/railties/test/generators_test.rb
@@ -16,6 +16,7 @@ class GeneratorsTest < Rails::Generators::TestCase
   end
 
   def test_simple_invoke
+    assert File.exists?(File.join(@path, 'generators', 'model_generator.rb'))
     TestUnit::Generators::ModelGenerator.expects(:start).with(["Account"], {})
     Rails::Generators.invoke("test_unit:model", ["Account"])
   end
@@ -30,6 +31,13 @@ class GeneratorsTest < Rails::Generators::TestCase
     assert_match /Description:/, output
   end
 
+  def test_should_give_higher_preference_to_rails_generators
+    assert File.exists?(File.join(@path, 'generators', 'model_generator.rb'))
+    Rails::Generators::ModelGenerator.expects(:start).with(["Account"], {})
+    warnings = capture(:stderr){ Rails::Generators.invoke :model, ["Account"] }
+    assert warnings.empty?
+  end
+
   def test_invoke_with_default_values
     Rails::Generators::ModelGenerator.expects(:start).with(["Account"], {})
     Rails::Generators.invoke :model, ["Account"]
diff --git a/railties/test/subscriber_test.rb b/railties/test/subscriber_test.rb
index 724e8a75d6..f6c895093f 100644
--- a/railties/test/subscriber_test.rb
+++ b/railties/test/subscriber_test.rb
@@ -24,7 +24,9 @@ class MySubscriber < Rails::Subscriber
   end
 end
 
-module SubscriberTest
+class SyncSubscriberTest < ActiveSupport::TestCase
+  include Rails::Subscriber::TestHelper
+
   def setup
     super
     @subscriber = MySubscriber.new
@@ -94,51 +96,24 @@ module SubscriberTest
     assert_equal 1, @logger.flush_count
   end
 
-  def test_flushes_loggers_when_action_dispatch_callback_is_received
-    Rails::Subscriber.add :my_subscriber, @subscriber
-    instrument "action_dispatch.after_dispatch"
-    wait
-    assert_equal 1, @logger.flush_count
-  end
-
   def test_flushes_the_same_logger_just_once
     Rails::Subscriber.add :my_subscriber, @subscriber
     Rails::Subscriber.add :another, @subscriber
-    instrument "action_dispatch.after_dispatch"
+    Rails::Subscriber.flush_all!
     wait
     assert_equal 1, @logger.flush_count
   end
 
-  def test_logging_thread_does_not_die_on_failures
+  def test_logging_does_not_die_on_failures
     Rails::Subscriber.add :my_subscriber, @subscriber
     instrument "my_subscriber.puke"
-    instrument "action_dispatch.after_dispatch"
-    wait
-    assert_equal 1, @logger.flush_count
-    assert_equal 1, @logger.logged(:error).size
-    assert_equal 'Could not log "my_subscriber.puke" event. RuntimeError: puke', @logger.logged(:error).last
-  end
-
-  def test_tails_logs_when_action_dispatch_callback_is_received
-    log_tailer = mock()
-    log_tailer.expects(:tail!)
-    Rails::Subscriber.log_tailer = log_tailer
-
-    Rails::Subscriber.add :my_subscriber, @subscriber
-    instrument "action_dispatch.after_dispatch"
+    instrument "my_subscriber.some_event"
     wait
-  ensure
-    Rails::Subscriber.log_tailer = nil
-  end
 
-  class SyncSubscriberTest < ActiveSupport::TestCase
-    include Rails::Subscriber::SyncTestHelper
-    include SubscriberTest
-  end
+    assert_equal 1, @logger.logged(:info).size
+    assert_equal 'my_subscriber.some_event', @logger.logged(:info).last
 
-  class AsyncSubscriberTest < ActiveSupport::TestCase
-    include Rails::Subscriber::AsyncTestHelper
-    include SubscriberTest
+    assert_equal 1, @logger.logged(:error).size
+    assert_equal 'Could not log "my_subscriber.puke" event. RuntimeError: puke', @logger.logged(:error).last
   end
-
 end
\ No newline at end of file