diff --git a/actionmailer/test/subscriber_test.rb b/actionmailer/test/subscriber_test.rb index 01a71f481d3f37ebf7b284cfca4e5f0f47085f27..aed5d2ca7e92f88c747e6b9fbf64626f29f9781a 100644 --- a/actionmailer/test/subscriber_test.rb +++ b/actionmailer/test/subscriber_test.rb @@ -2,7 +2,8 @@ require "rails/subscriber/test_helper" require "action_mailer/railties/subscriber" -module SubscriberTest +class AMSubscriberTest < ActionMailer::TestCase + include Rails::Subscriber::TestHelper Rails::Subscriber.add(:action_mailer, ActionMailer::Railties::Subscriber.new) class TestMailer < ActionMailer::Base @@ -40,14 +41,4 @@ def test_receive_is_notified assert_equal 1, @logger.logged(:debug).size assert_match /Jamis/, @logger.logged(:debug).first end - - class SyncSubscriberTest < ActionMailer::TestCase - include Rails::Subscriber::SyncTestHelper - include SubscriberTest - end - - class AsyncSubscriberTest < ActionMailer::TestCase - include Rails::Subscriber::AsyncTestHelper - include SubscriberTest - end end \ No newline at end of file diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index fa4a253ec1a7b6b4d4587faf3e86fdf4ff1ef8db..33e107d21657eeb73884118a019ceaa3a751b39a 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -16,7 +16,6 @@ module ActionController autoload :ConditionalGet autoload :Configuration autoload :Cookies - autoload :FilterParameterLogging autoload :Flash autoload :Head autoload :Helpers diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index 21a811c004276a022faaaac1ec666fb11186281e..a66aafd80e7d3ac1e82908121d71ad13714f9abc 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -27,7 +27,6 @@ class Base < Metal include ActionController::Compatibility include ActionController::Cookies - include ActionController::FilterParameterLogging include ActionController::Flash include ActionController::Verification include ActionController::RequestForgeryProtection @@ -74,6 +73,15 @@ def self.subclasses @subclasses ||= [] end + # This method has been moved to ActionDispatch::Request.filter_parameters + def self.filter_parameter_logging(*args, &block) + ActiveSupport::Deprecation.warn("Setting filter_parameter_logging in ActionController is deprecated and has no longer effect, please set 'config.filter_parameters' in config/application.rb instead", caller) + filter = Rails.application.config.filter_parameters + filter.concat(args) + filter << block if block + filter + end + def _normalize_options(action=nil, options={}, &blk) case action when NilClass diff --git a/actionpack/lib/action_controller/metal/filter_parameter_logging.rb b/actionpack/lib/action_controller/metal/filter_parameter_logging.rb deleted file mode 100644 index 9e03f507596ff98a8a5a918f31d4e1992ecbb8f1..0000000000000000000000000000000000000000 --- a/actionpack/lib/action_controller/metal/filter_parameter_logging.rb +++ /dev/null @@ -1,66 +0,0 @@ -module ActionController - module FilterParameterLogging - extend ActiveSupport::Concern - - INTERNAL_PARAMS = %w(controller action format _method only_path) - - module ClassMethods - # Replace sensitive parameter data from the request log. - # Filters parameters that have any of the arguments as a substring. - # Looks in all subhashes of the param hash for keys to filter. - # If a block is given, each key and value of the parameter hash and all - # subhashes is passed to it, the value or key - # can be replaced using String#replace or similar method. - # - # Examples: - # - # filter_parameter_logging :password - # => replaces the value to all keys matching /password/i with "[FILTERED]" - # - # filter_parameter_logging :foo, "bar" - # => replaces the value to all keys matching /foo|bar/i with "[FILTERED]" - # - # filter_parameter_logging { |k,v| v.reverse! if k =~ /secret/i } - # => reverses the value to all keys matching /secret/i - # - # filter_parameter_logging(:foo, "bar") { |k,v| v.reverse! if k =~ /secret/i } - # => reverses the value to all keys matching /secret/i, and - # replaces the value to all keys matching /foo|bar/i with "[FILTERED]" - def filter_parameter_logging(*filter_words, &block) - raise "You must filter at least one word from logging" if filter_words.empty? - - parameter_filter = Regexp.new(filter_words.join('|'), true) - - define_method(:filter_parameters) do |original_params| - filtered_params = {} - - original_params.each do |key, value| - if key =~ parameter_filter - value = '[FILTERED]' - elsif value.is_a?(Hash) - value = filter_parameters(value) - elsif value.is_a?(Array) - value = value.map { |item| filter_parameters(item) } - elsif block_given? - key = key.dup - value = value.dup if value.duplicable? - yield key, value - end - - filtered_params[key] = value - end - - filtered_params.except!(*INTERNAL_PARAMS) - end - protected :filter_parameters - end - end - - protected - - def filter_parameters(params) - params.dup.except!(*INTERNAL_PARAMS) - end - - end -end diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 19c962bafa7681e8baf99fde03365bebcd9ff3d4..7f9a7c068b19e8f97bac36720edf13ea728dfdb5 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -10,7 +10,6 @@ module Instrumentation extend ActiveSupport::Concern include AbstractController::Logger - include ActionController::FilterParameterLogging attr_internal :view_runtime @@ -18,7 +17,7 @@ def process_action(action, *args) raw_payload = { :controller => self.class.name, :action => self.action_name, - :params => filter_parameters(params), + :params => request.filtered_parameters, :formats => request.formats.map(&:to_sym) } diff --git a/actionpack/lib/action_controller/railties/subscriber.rb b/actionpack/lib/action_controller/railties/subscriber.rb index d257d6ac2c45eb318eb79b5851abceed6b5a7b09..1f0e6bf51abecff0b661c0db9c5011d563125730 100644 --- a/actionpack/lib/action_controller/railties/subscriber.rb +++ b/actionpack/lib/action_controller/railties/subscriber.rb @@ -1,10 +1,14 @@ module ActionController module Railties class Subscriber < Rails::Subscriber + INTERNAL_PARAMS = %w(controller action format _method only_path) + def start_processing(event) payload = event.payload + params = payload[:params].except(*INTERNAL_PARAMS) + info " Processing by #{payload[:controller]}##{payload[:action]} as #{payload[:formats].first.to_s.upcase}" - info " Parameters: #{payload[:params].inspect}" unless payload[:params].blank? + info " Parameters: #{params.inspect}" unless params.empty? end def process_action(event) diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 082562d9211952db38b1009d7b5c0318bc2d7f6c..f0490a561949557981bd5d7145f6f6a8e45cd165 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -46,7 +46,6 @@ module ActionDispatch autoload :Cookies autoload :Flash autoload :Head - autoload :Notifications autoload :ParamsParser autoload :Rescue autoload :ShowExceptions @@ -63,6 +62,7 @@ module Http autoload :Headers autoload :MimeNegotiation autoload :Parameters + autoload :FilterParameters autoload :Upload autoload :UploadedFile, 'action_dispatch/http/upload' autoload :URL diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb new file mode 100644 index 0000000000000000000000000000000000000000..1958e1668d1102a9a8fa650a0aa8f8d1751ddd39 --- /dev/null +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -0,0 +1,98 @@ +require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/hash/keys' + +module ActionDispatch + module Http + # Allows you to specify sensitive parameters which will be replaced from + # the request log by looking in all subhashes of the param hash for keys + # to filter. If a block is given, each key and value of the parameter + # hash and all subhashes is passed to it, the value or key can be replaced + # using String#replace or similar method. + # + # Examples: + # + # env["action_dispatch.parameter_filter"] = [:password] + # => replaces the value to all keys matching /password/i with "[FILTERED]" + # + # env["action_dispatch.parameter_filter"] = [:foo, "bar"] + # => replaces the value to all keys matching /foo|bar/i with "[FILTERED]" + # + # env["action_dispatch.parameter_filter"] = lambda do |k,v| + # v.reverse! if k =~ /secret/i + # end + # => reverses the value to all keys matching /secret/i + # + module FilterParameters + extend ActiveSupport::Concern + + # Return a hash of parameters with all sensitive data replaced. + def filtered_parameters + @filtered_parameters ||= process_parameter_filter(parameters) + end + alias :fitered_params :filtered_parameters + + # Return a hash of request.env with all sensitive data replaced. + def filtered_env + filtered_env = @env.dup + filtered_env.each do |key, value| + if (key =~ /RAW_POST_DATA/i) + filtered_env[key] = '[FILTERED]' + elsif value.is_a?(Hash) + filtered_env[key] = process_parameter_filter(value) + end + end + filtered_env + end + + protected + + def compile_parameter_filter #:nodoc: + strings, regexps, blocks = [], [], [] + + Array(@env["action_dispatch.parameter_filter"]).each do |item| + case item + when NilClass + when Proc + blocks << item + when Regexp + regexps << item + else + strings << item.to_s + end + end + + regexps << Regexp.new(strings.join('|'), true) unless strings.empty? + [regexps, blocks] + end + + def filtering_parameters? #:nodoc: + @env["action_dispatch.parameter_filter"].present? + end + + def process_parameter_filter(original_params) #:nodoc: + return original_params.dup unless filtering_parameters? + + filtered_params = {} + regexps, blocks = compile_parameter_filter + + original_params.each do |key, value| + if regexps.find { |r| key =~ r } + value = '[FILTERED]' + elsif value.is_a?(Hash) + value = process_parameter_filter(value) + elsif value.is_a?(Array) + value = value.map { |i| process_parameter_filter(i) } + elsif blocks.present? + key = key.dup + value = value.dup if value.duplicable? + blocks.each { |b| b.call(key, value) } + end + + filtered_params[key] = value + end + + filtered_params + end + end + end +end \ No newline at end of file diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index 68ba3637bf944a1ba1e709de46795e8000caa1c5..40b40ea94ece107d9162c9315bf10db7adfb7384 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -30,29 +30,6 @@ def path_parameters @env["action_dispatch.request.path_parameters"] ||= {} end - def filter_parameters - # TODO: Remove dependency on controller - if controller = @env['action_controller.instance'] - controller.send(:filter_parameters, params) - else - params - end - end - - def filter_env - if controller = @env['action_controller.instance'] - @env.map do |key, value| - if (key =~ /RAW_POST_DATA/i) - '[FILTERED]' - else - controller.send(:filter_parameters, {key => value}).values[0] - end - end - else - env - end - end - private # Convert nested Hashs to HashWithIndifferentAccess def normalize_parameters(value) diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 187ce7c15d411b72725da4dd1bee481b7d44dc86..7a17023ed25643de7b521d1215039907e54f68e0 100755 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -11,6 +11,7 @@ class Request < Rack::Request include ActionDispatch::Http::Cache::Request include ActionDispatch::Http::MimeNegotiation include ActionDispatch::Http::Parameters + include ActionDispatch::Http::FilterParameters include ActionDispatch::Http::Upload include ActionDispatch::Http::URL diff --git a/actionpack/lib/action_dispatch/middleware/notifications.rb b/actionpack/lib/action_dispatch/middleware/notifications.rb deleted file mode 100644 index ce3732b7401c6c5d2f91584aa30cf7e2622e1926..0000000000000000000000000000000000000000 --- a/actionpack/lib/action_dispatch/middleware/notifications.rb +++ /dev/null @@ -1,32 +0,0 @@ -module ActionDispatch - # Provide notifications in the middleware stack. Notice that for the before_dispatch - # and after_dispatch notifications, we just send the original env, so we don't pile - # up large env hashes in the queue. However, in exception cases, the whole env hash - # is actually useful, so we send it all. - class Notifications - def initialize(app) - @app = app - end - - def call(env) - request = Request.new(env) - payload = retrieve_payload_from_env(request.filter_env) - - ActiveSupport::Notifications.instrument("action_dispatch.before_dispatch", payload) - - ActiveSupport::Notifications.instrument!("action_dispatch.after_dispatch", payload) do - @app.call(env) - end - rescue Exception => exception - ActiveSupport::Notifications.instrument('action_dispatch.exception', - :env => env, :exception => exception) - raise exception - end - - protected - # Remove any rack related constants from the env, like rack.input. - def retrieve_payload_from_env(env) - Hash[:env => env.except(*env.keys.select { |k| k.to_s.index("rack.") == 0 })] - end - end -end diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 18978bfb39ad2fcea8a68f256b0a65fe6d258c43..e4bd143e78cc543643d936a36981dbbe1a2ec280 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -5,9 +5,6 @@ module ActionDispatch class Railtie < Rails::Railtie plugin_name :action_dispatch - require "action_dispatch/railties/subscriber" - subscriber ActionDispatch::Railties::Subscriber.new - # Prepare dispatcher callbacks and run 'prepare' callbacks initializer "action_dispatch.prepare_dispatcher" do |app| # TODO: This used to say unless defined?(Dispatcher). Find out why and fix. diff --git a/actionpack/lib/action_dispatch/railties/subscriber.rb b/actionpack/lib/action_dispatch/railties/subscriber.rb deleted file mode 100644 index cdb1162eac77e8b2ff706fab7f22478bc9d30367..0000000000000000000000000000000000000000 --- a/actionpack/lib/action_dispatch/railties/subscriber.rb +++ /dev/null @@ -1,17 +0,0 @@ -module ActionDispatch - module Railties - class Subscriber < Rails::Subscriber - def before_dispatch(event) - request = Request.new(event.payload[:env]) - path = request.request_uri.inspect rescue "unknown" - - info "\n\nStarted #{request.method.to_s.upcase} #{path} " << - "for #{request.remote_ip} at #{event.time.to_s(:db)}" - end - - def logger - ActionController::Base.logger - end - end - end -end \ No newline at end of file diff --git a/actionpack/test/abstract/render_test.rb b/actionpack/test/abstract/render_test.rb index be0478b638705dea30f012225a47a68e7aa38596..4bec44c9ae3f3777483f525027bb778f374f365b 100644 --- a/actionpack/test/abstract/render_test.rb +++ b/actionpack/test/abstract/render_test.rb @@ -33,6 +33,10 @@ def default render end + def shortcut + render "template" + end + def template_name render :_template_name => :template_name end @@ -73,6 +77,11 @@ def test_render_default assert_equal "With Default", @controller.response_body end + def test_render_template_through_shortcut + @controller.process(:shortcut) + assert_equal "With Template", @controller.response_body + end + def test_render_template_name @controller.process(:template_name) assert_equal "With Template Name", @controller.response_body diff --git a/actionpack/test/activerecord/controller_runtime_test.rb b/actionpack/test/activerecord/controller_runtime_test.rb index 37c7738301e8b6a85780090004827252eb364638..ed8e32493851f74169199c740234218adb91da98 100644 --- a/actionpack/test/activerecord/controller_runtime_test.rb +++ b/actionpack/test/activerecord/controller_runtime_test.rb @@ -6,16 +6,15 @@ ActionController::Base.send :include, ActiveRecord::Railties::ControllerRuntime -module ControllerRuntimeSubscriberTest +class ControllerRuntimeSubscriberTest < ActionController::TestCase class SubscriberController < ActionController::Base def show render :inline => "<%= Project.all %>" end end - - def self.included(base) - base.tests SubscriberController - end + + include Rails::Subscriber::TestHelper + tests SubscriberController def setup @old_logger = ActionController::Base.logger @@ -40,14 +39,4 @@ def test_log_with_active_record assert_equal 2, @logger.logged(:info).size assert_match /\(Views: [\d\.]+ms | ActiveRecord: [\d\.]+ms\)/, @logger.logged(:info)[1] end - - class SyncSubscriberTest < ActionController::TestCase - include Rails::Subscriber::SyncTestHelper - include ControllerRuntimeSubscriberTest - end - - class AsyncSubscriberTest < ActionController::TestCase - include Rails::Subscriber::AsyncTestHelper - include ControllerRuntimeSubscriberTest - end end \ No newline at end of file diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 1510a6a7e0a6165848c1d2b76df6d92d0006e975..4fcfbacf4ef607c536b130ca3c6ae80daf0795a1 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -2,6 +2,9 @@ require 'logger' require 'pp' # require 'pp' early to prevent hidden_methods from not picking up the pretty-print methods until too late +module Rails +end + # Provide some controller to run the tests on. module Submodule class ContainedEmptyController < ActionController::Base @@ -63,7 +66,7 @@ def default_url_options(options = nil) end end -class ControllerClassTests < Test::Unit::TestCase +class ControllerClassTests < ActiveSupport::TestCase def test_controller_path assert_equal 'empty', EmptyController.controller_path assert_equal EmptyController.controller_path, EmptyController.new.controller_path @@ -74,7 +77,21 @@ def test_controller_path def test_controller_name assert_equal 'empty', EmptyController.controller_name assert_equal 'contained_empty', Submodule::ContainedEmptyController.controller_name - end + end + + def test_filter_parameter_logging + parameters = [] + config = mock(:config => mock(:filter_parameters => parameters)) + Rails.expects(:application).returns(config) + + assert_deprecated do + Class.new(ActionController::Base) do + filter_parameter_logging :password + end + end + + assert_equal [:password], parameters + end end class ControllerInstanceTests < Test::Unit::TestCase diff --git a/actionpack/test/controller/filter_params_test.rb b/actionpack/test/controller/filter_params_test.rb deleted file mode 100644 index 45949636c37a3d45acf00ae88ec21438ece37652..0000000000000000000000000000000000000000 --- a/actionpack/test/controller/filter_params_test.rb +++ /dev/null @@ -1,51 +0,0 @@ -require 'abstract_unit' - -class FilterParamController < ActionController::Base - def payment - head :ok - end -end - -class FilterParamTest < ActionController::TestCase - tests FilterParamController - - def test_filter_parameters_must_have_one_word - assert_raises RuntimeError do - FilterParamController.filter_parameter_logging - end - end - - def test_filter_parameters - assert FilterParamController.respond_to?(:filter_parameter_logging) - - test_hashes = [ - [{'foo'=>'bar'},{'foo'=>'bar'},%w'food'], - [{'foo'=>'bar'},{'foo'=>'[FILTERED]'},%w'foo'], - [{'foo'=>'bar', 'bar'=>'foo'},{'foo'=>'[FILTERED]', 'bar'=>'foo'},%w'foo baz'], - [{'foo'=>'bar', 'baz'=>'foo'},{'foo'=>'[FILTERED]', 'baz'=>'[FILTERED]'},%w'foo baz'], - [{'bar'=>{'foo'=>'bar','bar'=>'foo'}},{'bar'=>{'foo'=>'[FILTERED]','bar'=>'foo'}},%w'fo'], - [{'foo'=>{'foo'=>'bar','bar'=>'foo'}},{'foo'=>'[FILTERED]'},%w'f banana'], - [{'baz'=>[{'foo'=>'baz'}]}, {'baz'=>[{'foo'=>'[FILTERED]'}]}, %w(foo)]] - - test_hashes.each do |before_filter, after_filter, filter_words| - FilterParamController.filter_parameter_logging(*filter_words) - assert_equal after_filter, @controller.__send__(:filter_parameters, before_filter) - - filter_words.push('blah') - FilterParamController.filter_parameter_logging(*filter_words) do |key, value| - value.reverse! if key =~ /bargain/ - end - - before_filter['barg'] = {'bargain'=>'gain', 'blah'=>'bar', 'bar'=>{'bargain'=>{'blah'=>'foo'}}} - after_filter['barg'] = {'bargain'=>'niag', 'blah'=>'[FILTERED]', 'bar'=>{'bargain'=>{'blah'=>'[FILTERED]'}}} - - assert_equal after_filter, @controller.__send__(:filter_parameters, before_filter) - end - end - - def test_filter_parameters_is_protected - FilterParamController.filter_parameter_logging(:foo) - assert !FilterParamController.action_methods.include?('filter_parameters') - assert_raise(NoMethodError) { @controller.filter_parameters([{'password' => '[FILTERED]'}]) } - end -end diff --git a/actionpack/test/controller/subscriber_test.rb b/actionpack/test/controller/subscriber_test.rb index 950eecaf6ff7bd7e84d80b88e6b97340faafcdb8..152a0d0c0488c3e6e8199a070b8b0fa4e5d10cff 100644 --- a/actionpack/test/controller/subscriber_test.rb +++ b/actionpack/test/controller/subscriber_test.rb @@ -35,11 +35,9 @@ def with_page_cache end end -module ActionControllerSubscriberTest - - def self.included(base) - base.tests Another::SubscribersController - end +class ACSubscriberTest < ActionController::TestCase + tests Another::SubscribersController + include Rails::Subscriber::TestHelper def setup @old_logger = ActionController::Base.logger @@ -99,7 +97,7 @@ def test_process_action_with_view_runtime end def test_process_action_with_filter_parameters - Another::SubscribersController.filter_parameter_logging(:lifo, :amount) + @request.env["action_dispatch.parameter_filter"] = [:lifo, :amount] get :show, :lifo => 'Pratik', :amount => '420', :step => '1' wait @@ -171,14 +169,4 @@ def test_with_page_cache def logs @logs ||= @logger.logged(:info) end - - class SyncSubscriberTest < ActionController::TestCase - include Rails::Subscriber::SyncTestHelper - include ActionControllerSubscriberTest - end - - class AsyncSubscriberTest < ActionController::TestCase - include Rails::Subscriber::AsyncTestHelper - include ActionControllerSubscriberTest - end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index cb95ecea50793982e36c18a9cac282fa48d78b29..2b5c19361a4bdfb6bcd59677e8d318577d730cac 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -454,6 +454,56 @@ def teardown assert_equal Mime::XML, request.negotiate_mime([Mime::XML, Mime::CSV]) end + test "process parameter filter" do + test_hashes = [ + [{'foo'=>'bar'},{'foo'=>'bar'},%w'food'], + [{'foo'=>'bar'},{'foo'=>'[FILTERED]'},%w'foo'], + [{'foo'=>'bar', 'bar'=>'foo'},{'foo'=>'[FILTERED]', 'bar'=>'foo'},%w'foo baz'], + [{'foo'=>'bar', 'baz'=>'foo'},{'foo'=>'[FILTERED]', 'baz'=>'[FILTERED]'},%w'foo baz'], + [{'bar'=>{'foo'=>'bar','bar'=>'foo'}},{'bar'=>{'foo'=>'[FILTERED]','bar'=>'foo'}},%w'fo'], + [{'foo'=>{'foo'=>'bar','bar'=>'foo'}},{'foo'=>'[FILTERED]'},%w'f banana'], + [{'baz'=>[{'foo'=>'baz'}]}, {'baz'=>[{'foo'=>'[FILTERED]'}]}, [/foo/]]] + + test_hashes.each do |before_filter, after_filter, filter_words| + request = stub_request('action_dispatch.parameter_filter' => filter_words) + assert_equal after_filter, request.send(:process_parameter_filter, before_filter) + + filter_words << 'blah' + filter_words << lambda { |key, value| + value.reverse! if key =~ /bargain/ + } + + request = stub_request('action_dispatch.parameter_filter' => filter_words) + before_filter['barg'] = {'bargain'=>'gain', 'blah'=>'bar', 'bar'=>{'bargain'=>{'blah'=>'foo'}}} + after_filter['barg'] = {'bargain'=>'niag', 'blah'=>'[FILTERED]', 'bar'=>{'bargain'=>{'blah'=>'[FILTERED]'}}} + + assert_equal after_filter, request.send(:process_parameter_filter, before_filter) + end + end + + test "filtered_parameters returns params filtered" do + request = stub_request('action_dispatch.request.parameters' => + { 'lifo' => 'Pratik', 'amount' => '420', 'step' => '1' }, + 'action_dispatch.parameter_filter' => [:lifo, :amount]) + + params = request.filtered_parameters + assert_equal "[FILTERED]", params["lifo"] + assert_equal "[FILTERED]", params["amount"] + assert_equal "1", params["step"] + end + + test "filtered_env filters env as a whole" do + request = stub_request('action_dispatch.request.parameters' => + { 'amount' => '420', 'step' => '1' }, "RAW_POST_DATA" => "yada yada", + 'action_dispatch.parameter_filter' => [:lifo, :amount]) + + request = stub_request(request.filtered_env) + + assert_equal "[FILTERED]", request.raw_post + assert_equal "[FILTERED]", request.params["amount"] + assert_equal "1", request.params["step"] + end + protected def stub_request(env={}) diff --git a/actionpack/test/dispatch/subscriber_test.rb b/actionpack/test/dispatch/subscriber_test.rb deleted file mode 100644 index 3096c132e7e0e9db43f8a793b25176b6abe0da26..0000000000000000000000000000000000000000 --- a/actionpack/test/dispatch/subscriber_test.rb +++ /dev/null @@ -1,111 +0,0 @@ -require "abstract_unit" -require "rails/subscriber/test_helper" -require "action_dispatch/railties/subscriber" - -module DispatcherSubscriberTest - Boomer = lambda do |env| - req = ActionDispatch::Request.new(env) - case req.path - when "/" - [200, {}, []] - else - raise "puke!" - end - end - - App = ActionDispatch::Notifications.new(Boomer) - - def setup - Rails::Subscriber.add(:action_dispatch, ActionDispatch::Railties::Subscriber.new) - @app = App - super - - @events = [] - ActiveSupport::Notifications.subscribe do |*args| - @events << args - end - end - - def set_logger(logger) - ActionController::Base.logger = logger - end - - def test_publishes_notifications - get "/" - wait - - assert_equal 2, @events.size - before, after = @events - - assert_equal 'action_dispatch.before_dispatch', before[0] - assert_kind_of Hash, before[4][:env] - assert_equal 'GET', before[4][:env]["REQUEST_METHOD"] - - assert_equal 'action_dispatch.after_dispatch', after[0] - assert_kind_of Hash, after[4][:env] - assert_equal 'GET', after[4][:env]["REQUEST_METHOD"] - end - - def test_publishes_notifications_even_on_failures - begin - get "/puke" - rescue - end - - wait - - assert_equal 3, @events.size - before, after, exception = @events - - assert_equal 'action_dispatch.before_dispatch', before[0] - assert_kind_of Hash, before[4][:env] - assert_equal 'GET', before[4][:env]["REQUEST_METHOD"] - - assert_equal 'action_dispatch.after_dispatch', after[0] - assert_kind_of Hash, after[4][:env] - assert_equal 'GET', after[4][:env]["REQUEST_METHOD"] - - assert_equal 'action_dispatch.exception', exception[0] - assert_kind_of Hash, exception[4][:env] - assert_equal 'GET', exception[4][:env]["REQUEST_METHOD"] - assert_kind_of RuntimeError, exception[4][:exception] - end - - def test_subscriber_logs_notifications - get "/" - wait - - log = @logger.logged(:info).first - assert_equal 1, @logger.logged(:info).size - - assert_match %r{^Started GET "/"}, log - assert_match %r{for 127\.0\.0\.1}, log - end - - def test_subscriber_has_its_logged_flushed_after_request - assert_equal 0, @logger.flush_count - get "/" - wait - assert_equal 1, @logger.flush_count - end - - def test_subscriber_has_its_logged_flushed_even_after_busted_requests - assert_equal 0, @logger.flush_count - begin - get "/puke" - rescue - end - wait - assert_equal 1, @logger.flush_count - end - - class SyncSubscriberTest < ActionController::IntegrationTest - include Rails::Subscriber::SyncTestHelper - include DispatcherSubscriberTest - end - - class AsyncSubscriberTest < ActionController::IntegrationTest - include Rails::Subscriber::AsyncTestHelper - include DispatcherSubscriberTest - end -end \ No newline at end of file diff --git a/actionpack/test/template/subscriber_test.rb b/actionpack/test/template/subscriber_test.rb index af0b3102cf54e4a6dca80a071162e8943d2e9c62..5db2b16ac1f7c23323054bf627e117a92015e080 100644 --- a/actionpack/test/template/subscriber_test.rb +++ b/actionpack/test/template/subscriber_test.rb @@ -3,7 +3,8 @@ require "action_view/railties/subscriber" require "controller/fake_models" -module ActionViewSubscriberTest +class AVSubscriberTest < ActiveSupport::TestCase + include Rails::Subscriber::TestHelper def setup @old_logger = ActionController::Base.logger @@ -89,14 +90,4 @@ def test_render_collection_template_without_path assert_equal 1, @logger.logged(:info).size assert_match /Rendered collection/, @logger.logged(:info).last end - - class SyncSubscriberTest < ActiveSupport::TestCase - include Rails::Subscriber::SyncTestHelper - include ActionViewSubscriberTest - end - - class AsyncSubscriberTest < ActiveSupport::TestCase - include Rails::Subscriber::AsyncTestHelper - include ActionViewSubscriberTest - end end \ No newline at end of file diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 58673ab7bd385fd92535b1d38b57519da3b1dff6..cc0accf90e0c9e31a1b9ab7ae61856ebc5a84a59 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -45,7 +45,6 @@ module ActiveRecord autoload :AssociationPreload autoload :Associations autoload :AttributeMethods - autoload :Attributes autoload :AutosaveAssociation autoload :Relation @@ -77,7 +76,6 @@ module ActiveRecord autoload :StateMachine autoload :Timestamp autoload :Transactions - autoload :Types autoload :Validations end @@ -95,28 +93,6 @@ module AttributeMethods end end - module Attributes - extend ActiveSupport::Autoload - - eager_autoload do - autoload :Aliasing - autoload :Store - autoload :Typecasting - end - end - - module Type - extend ActiveSupport::Autoload - - eager_autoload do - autoload :Number, 'active_record/types/number' - autoload :Object, 'active_record/types/object' - autoload :Serialize, 'active_record/types/serialize' - autoload :TimeWithZone, 'active_record/types/time_with_zone' - autoload :Unknown, 'active_record/types/unknown' - end - end - module Locking extend ActiveSupport::Autoload diff --git a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb index 74921241f751be9a3ae1f2dd3a3194aa460e77de..a4e144f233a43f559681ef2169668bacb43b3093 100644 --- a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb +++ b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb @@ -8,18 +8,25 @@ module BeforeTypeCast end def read_attribute_before_type_cast(attr_name) - _attributes.without_typecast[attr_name] + @attributes[attr_name] end # Returns a hash of attributes before typecasting and deserialization. def attributes_before_type_cast - _attributes.without_typecast + self.attribute_names.inject({}) do |attrs, name| + attrs[name] = read_attribute_before_type_cast(name) + attrs + end end private # Handle *_before_type_cast for method_missing. def attribute_before_type_cast(attribute_name) - read_attribute_before_type_cast(attribute_name) + if attribute_name == 'id' + read_attribute_before_type_cast(self.class.primary_key) + else + read_attribute_before_type_cast(attribute_name) + end end end end diff --git a/activerecord/lib/active_record/attribute_methods/query.rb b/activerecord/lib/active_record/attribute_methods/query.rb index 0154ee35f8c21bd782b7c63e61018803de75d509..a949d801202ba3b7a2c155e27d94b3c5360a719d 100644 --- a/activerecord/lib/active_record/attribute_methods/query.rb +++ b/activerecord/lib/active_record/attribute_methods/query.rb @@ -8,7 +8,23 @@ module Query end def query_attribute(attr_name) - _attributes.has?(attr_name) + unless value = read_attribute(attr_name) + false + else + column = self.class.columns_hash[attr_name] + if column.nil? + if Numeric === value || value !~ /[^0-9]/ + !value.to_i.zero? + else + return false if ActiveRecord::ConnectionAdapters::Column::FALSE_VALUES.include?(value) + !value.blank? + end + elsif column.number? + !value.zero? + else + !value.blank? + end + end end private @@ -19,5 +35,3 @@ def attribute?(attribute_name) end end end - - diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 97caec77446fdd0d0682b41cdde126cb5cdf20d4..3da3d9d8cca86589f0271516b80a13fd0be828ff 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -37,7 +37,11 @@ def cache_attribute?(attr_name) protected def define_method_attribute(attr_name) - define_read_method(attr_name.to_sym, attr_name, columns_hash[attr_name]) + if self.serialized_attributes[attr_name] + define_read_method_for_serialized_attribute(attr_name) + else + define_read_method(attr_name.to_sym, attr_name, columns_hash[attr_name]) + end if attr_name == primary_key && attr_name != "id" define_read_method(:id, attr_name, columns_hash[attr_name]) @@ -45,12 +49,18 @@ def define_method_attribute(attr_name) end private + # Define read method for serialized attribute. + def define_read_method_for_serialized_attribute(attr_name) + generated_attribute_methods.module_eval("def #{attr_name}; unserialize_attribute('#{attr_name}'); end", __FILE__, __LINE__) + end # Define an attribute reader method. Cope with nil column. def define_read_method(symbol, attr_name, column) - access_code = "_attributes['#{attr_name}']" + cast_code = column.type_cast_code('v') if column + access_code = cast_code ? "(v=@attributes['#{attr_name}']) && #{cast_code}" : "@attributes['#{attr_name}']" + unless attr_name.to_s == self.primary_key.to_s - access_code = access_code.insert(0, "missing_attribute('#{attr_name}', caller) unless _attributes.key?('#{attr_name}'); ") + access_code = access_code.insert(0, "missing_attribute('#{attr_name}', caller) unless @attributes.has_key?('#{attr_name}'); ") end if cache_attribute?(attr_name) @@ -63,7 +73,38 @@ def define_read_method(symbol, attr_name, column) # Returns the value of the attribute identified by attr_name after it has been typecast (for example, # "2004-12-12" in a data column is cast to a date object, like Date.new(2004, 12, 12)). def read_attribute(attr_name) - _attributes[attr_name] + attr_name = attr_name.to_s + attr_name = self.class.primary_key if attr_name == 'id' + if !(value = @attributes[attr_name]).nil? + if column = column_for_attribute(attr_name) + if unserializable_attribute?(attr_name, column) + unserialize_attribute(attr_name) + else + column.type_cast(value) + end + else + value + end + else + nil + end + end + + # Returns true if the attribute is of a text column and marked for serialization. + def unserializable_attribute?(attr_name, column) + column.text? && self.class.serialized_attributes[attr_name] + end + + # Returns the unserialized object of the attribute. + def unserialize_attribute(attr_name) + unserialized_object = object_from_yaml(@attributes[attr_name]) + + if unserialized_object.is_a?(self.class.serialized_attributes[attr_name]) || unserialized_object.nil? + @attributes.frozen? ? unserialized_object : @attributes[attr_name] = unserialized_object + else + raise SerializationTypeMismatch, + "#{attr_name} was supposed to be a #{self.class.serialized_attributes[attr_name]}, but was a #{unserialized_object.class.to_s}" + end end private diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 4ac0c7f608b9fc61b11ad831fd512edef5bece99..a8e3e28a7a626bafcde9813cca22720e615e8e3a 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -12,20 +12,48 @@ module TimeZoneConversion end module ClassMethods - - def cache_attribute?(attr_name) - time_zone_aware?(attr_name) || super - end - protected + # Defined for all +datetime+ and +timestamp+ attributes when +time_zone_aware_attributes+ are enabled. + # This enhanced read method automatically converts the UTC time stored in the database to the time zone stored in Time.zone. + def define_method_attribute(attr_name) + if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) + method_body = <<-EOV + def #{attr_name}(reload = false) + cached = @attributes_cache['#{attr_name}'] + return cached if cached && !reload + time = read_attribute('#{attr_name}') + @attributes_cache['#{attr_name}'] = time.acts_like?(:time) ? time.in_time_zone : time + end + EOV + generated_attribute_methods.module_eval(method_body, __FILE__, __LINE__) + else + super + end + end - def time_zone_aware?(attr_name) - column = columns_hash[attr_name] - time_zone_aware_attributes && - !skip_time_zone_conversion_for_attributes.include?(attr_name.to_sym) && - [:datetime, :timestamp].include?(column.type) + # Defined for all +datetime+ and +timestamp+ attributes when +time_zone_aware_attributes+ are enabled. + # This enhanced write method will automatically convert the time passed to it to the zone stored in Time.zone. + def define_method_attribute=(attr_name) + if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) + method_body = <<-EOV + def #{attr_name}=(time) + unless time.acts_like?(:time) + time = time.is_a?(String) ? Time.zone.parse(time) : time.to_time rescue time + end + time = time.in_time_zone rescue nil if time + write_attribute(:#{attr_name}, time) + end + EOV + generated_attribute_methods.module_eval(method_body, __FILE__, __LINE__) + else + super + end end + private + def create_time_zone_conversion_attribute?(name, column) + time_zone_aware_attributes && !skip_time_zone_conversion_for_attributes.include?(name.to_sym) && [:datetime, :timestamp].include?(column.type) + end end end end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 37eadbe0a9564f20d6660dcb9f3d589beddf33b1..e31acac05041b3362065bb001a3f3595cb6d785c 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -17,9 +17,14 @@ def define_method_attribute=(attr_name) # Updates the attribute identified by attr_name with the specified +value+. Empty strings for fixnum and float # columns are turned into +nil+. def write_attribute(attr_name, value) - attr_name = _attributes.unalias(attr_name) + attr_name = attr_name.to_s + attr_name = self.class.primary_key if attr_name == 'id' @attributes_cache.delete(attr_name) - _attributes[attr_name] = value + if (column = column_for_attribute(attr_name)) && column.number? + @attributes[attr_name] = convert_number_column_value(value) + else + @attributes[attr_name] = value + end end private diff --git a/activerecord/lib/active_record/attributes.rb b/activerecord/lib/active_record/attributes.rb deleted file mode 100644 index e4d9e89821de3ba30a29a3600dbe5085059a7386..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/attributes.rb +++ /dev/null @@ -1,37 +0,0 @@ -module ActiveRecord - module Attributes - - # Returns true if the given attribute is in the attributes hash - def has_attribute?(attr_name) - _attributes.key?(attr_name) - end - - # Returns an array of names for the attributes available on this object sorted alphabetically. - def attribute_names - _attributes.keys.sort! - end - - # Returns a hash of all the attributes with their names as keys and the values of the attributes as values. - def attributes - attributes = _attributes.dup - attributes.typecast! unless _attributes.frozen? - attributes.to_h - end - - protected - - # Not to be confused with the public #attributes method, which returns a typecasted Hash. - def _attributes - @attributes - end - - def initialize_attribute_store(merge_attributes = nil) - @attributes = ActiveRecord::Attributes::Store.new - @attributes.merge!(merge_attributes) if merge_attributes - @attributes.types.merge!(self.class.attribute_types) - @attributes.aliases.merge!('id' => self.class.primary_key) unless 'id' == self.class.primary_key - @attributes - end - - end -end diff --git a/activerecord/lib/active_record/attributes/aliasing.rb b/activerecord/lib/active_record/attributes/aliasing.rb deleted file mode 100644 index db77739d1f5c846d4b9881d96097b82da2c7ee06..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/attributes/aliasing.rb +++ /dev/null @@ -1,42 +0,0 @@ -module ActiveRecord - module Attributes - module Aliasing - # Allows access to keys using aliased names. - # - # Example: - # class Attributes < Hash - # include Aliasing - # end - # - # attributes = Attributes.new - # attributes.aliases['id'] = 'fancy_primary_key' - # attributes['fancy_primary_key'] = 2020 - # - # attributes['id'] - # => 2020 - # - # Additionally, symbols are always aliases of strings: - # attributes[:fancy_primary_key] - # => 2020 - # - def [](key) - super(unalias(key)) - end - - def []=(key, value) - super(unalias(key), value) - end - - def aliases - @aliases ||= {} - end - - def unalias(key) - key = key.to_s - aliases[key] || key - end - - end - end -end - diff --git a/activerecord/lib/active_record/attributes/store.rb b/activerecord/lib/active_record/attributes/store.rb deleted file mode 100644 index 61109f4acc8be375349f47b95566fe43d7a472ea..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/attributes/store.rb +++ /dev/null @@ -1,15 +0,0 @@ -module ActiveRecord - module Attributes - class Store < Hash - include ActiveRecord::Attributes::Typecasting - include ActiveRecord::Attributes::Aliasing - - # Attributes not mapped to a column are handled using Type::Unknown, - # which enables boolean typecasting for unmapped keys. - def types - @types ||= Hash.new(Type::Unknown.new) - end - - end - end -end diff --git a/activerecord/lib/active_record/attributes/typecasting.rb b/activerecord/lib/active_record/attributes/typecasting.rb deleted file mode 100644 index 56c32f989568f8de418f37030a46b4d63cbf6985..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/attributes/typecasting.rb +++ /dev/null @@ -1,117 +0,0 @@ -module ActiveRecord - module Attributes - module Typecasting - # Typecasts values during access based on their key mapping to a Type. - # - # Example: - # class Attributes < Hash - # include Typecasting - # end - # - # attributes = Attributes.new - # attributes.types['comments_count'] = Type::Integer - # attributes['comments_count'] = '5' - # - # attributes['comments_count'] - # => 5 - # - # To support keys not mapped to a typecaster, add a default to types. - # attributes.types.default = Type::Unknown - # attributes['age'] = '25' - # attributes['age'] - # => '25' - # - # A valid type supports #cast, #precast, #boolean, and #appendable? methods. - # - def [](key) - value = super(key) - typecast_read(key, value) - end - - def []=(key, value) - super(key, typecast_write(key, value)) - end - - def to_h - hash = {} - hash.merge!(self) - hash - end - - def dup # :nodoc: - copy = super - copy.types = types.dup - copy - end - - # Provides a duplicate with typecasting disabled. - # - # Example: - # attributes = Attributes.new - # attributes.types['comments_count'] = Type::Integer - # attributes['comments_count'] = '5' - # - # attributes.without_typecast['comments_count'] - # => '5' - # - def without_typecast - dup.without_typecast! - end - - def without_typecast! - types.clear - self - end - - def typecast! - keys.each { |key| self[key] = self[key] } - self - end - - # Check if key has a value that typecasts to true. - # - # attributes = Attributes.new - # attributes.types['comments_count'] = Type::Integer - # - # attributes['comments_count'] = 0 - # attributes.has?('comments_count') - # => false - # - # attributes['comments_count'] = 1 - # attributes.has?('comments_count') - # => true - # - def has?(key) - value = self[key] - boolean_typecast(key, value) - end - - def types - @types ||= {} - end - - protected - - def types=(other_types) - @types = other_types - end - - def boolean_typecast(key, value) - value ? types[key].boolean(value) : false - end - - def typecast_read(key, value) - type = types[key] - value = type.cast(value) - self[key] = value if type.appendable? && !frozen? - - value - end - - def typecast_write(key, value) - types[key].precast(value) - end - - end - end -end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index bc1b0bde311d567f3b140efcb143dbc4e22a4c61..f1b2b3b979228f4777d6832d3d2d7f29faf3ae3c 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1200,7 +1200,7 @@ def arel_engine def instantiate(record) object = find_sti_class(record[inheritance_column]).allocate - object.send(:initialize_attribute_store, record) + object.instance_variable_set(:'@attributes', record) object.instance_variable_set(:'@attributes_cache', {}) object.send(:_run_find_callbacks) @@ -1663,7 +1663,7 @@ def encode_quoted_value(value) #:nodoc: # In both instances, valid attribute keys are determined by the column names of the associated table -- # hence you can't have attributes that aren't part of the table columns. def initialize(attributes = nil) - initialize_attribute_store(attributes_from_column_definition) + @attributes = attributes_from_column_definition @attributes_cache = {} @new_record = true ensure_proper_type @@ -1694,7 +1694,7 @@ def initialize_copy(other) callback(:after_initialize) if respond_to_without_attributes?(:after_initialize) cloned_attributes = other.clone_attributes(:read_attribute_before_type_cast) cloned_attributes.delete(self.class.primary_key) - initialize_attribute_store(cloned_attributes) + @attributes = cloned_attributes clear_aggregation_cache @attributes_cache = {} @new_record = true @@ -1924,11 +1924,21 @@ def toggle!(attribute) def reload(options = nil) clear_aggregation_cache clear_association_cache - _attributes.update(self.class.find(self.id, options).instance_variable_get('@attributes')) + @attributes.update(self.class.find(self.id, options).instance_variable_get('@attributes')) @attributes_cache = {} self end + # Returns true if the given attribute is in the attributes hash + def has_attribute?(attr_name) + @attributes.has_key?(attr_name.to_s) + end + + # Returns an array of names for the attributes available on this object sorted alphabetically. + def attribute_names + @attributes.keys.sort + end + # Returns the value of the attribute identified by attr_name after it has been typecast (for example, # "2004-12-12" in a data column is cast to a date object, like Date.new(2004, 12, 12)). # (Alias for the protected read_attribute method). @@ -2262,7 +2272,7 @@ def assign_multiparameter_attributes(pairs) end def instantiate_time_object(name, values) - if self.class.send(:time_zone_aware?, name) + if self.class.send(:create_time_zone_conversion_attribute?, name, column_for_attribute(name)) Time.zone.local(*values) else Time.time_with_datetime_fallback(@@default_timezone, *values) @@ -2345,6 +2355,22 @@ def quoted_comma_pair_list(quoter, hash) comma_pair_list(quote_columns(quoter, hash)) end + def convert_number_column_value(value) + if value == false + 0 + elsif value == true + 1 + elsif value.is_a?(String) && value.blank? + nil + else + value + end + end + + def object_from_yaml(string) + return string unless string.is_a?(String) && string =~ /^---/ + YAML::load(string) rescue string + end end Base.class_eval do @@ -2359,7 +2385,6 @@ def quoted_comma_pair_list(quoter, hash) include AttributeMethods::PrimaryKey include AttributeMethods::TimeZoneConversion include AttributeMethods::Dirty - include Attributes, Types include Callbacks, ActiveModel::Observing, Timestamp include Associations, AssociationPreload, NamedScope include ActiveModel::Conversion diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index c9fff15199ebeedd74288dde36f9363d676fcd6f..04b85119cbd492f034df982aa0d79997ddd1ff9c 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -282,8 +282,11 @@ def to_sql def scope_for_create @scope_for_create ||= begin - @create_with_value || wheres.inject({}) do |hash, where| - hash[where.operand1.name] = where.operand2.value if where.is_a?(Arel::Predicates::Equality) + @create_with_value || @where_values.inject({}) do |hash, where| + if where.is_a?(Arel::Predicates::Equality) + hash[where.operand1.name] = where.operand2.respond_to?(:value) ? where.operand2.value : where.operand2 + end + hash end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index d0689cd93ea8e97b9bff23c36453b36d38fe610d..8954f2d12b19917713aecbd724f61a29ab953265 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -8,11 +8,10 @@ module QueryMethods class_eval <<-CEVAL def #{query_method}(*args) - spawn.tap do |new_relation| - new_relation.#{query_method}_values ||= [] - value = Array.wrap(args.flatten).reject {|x| x.blank? } - new_relation.#{query_method}_values += value if value.present? - end + new_relation = spawn + value = Array.wrap(args.flatten).reject {|x| x.blank? } + new_relation.#{query_method}_values += value if value.present? + new_relation end CEVAL end @@ -20,11 +19,10 @@ def #{query_method}(*args) [:where, :having].each do |query_method| class_eval <<-CEVAL def #{query_method}(*args) - spawn.tap do |new_relation| - new_relation.#{query_method}_values ||= [] - value = build_where(*args) - new_relation.#{query_method}_values += [*value] if value.present? - end + new_relation = spawn + value = build_where(*args) + new_relation.#{query_method}_values += [*value] if value.present? + new_relation end CEVAL end @@ -34,9 +32,9 @@ def #{query_method}(*args) class_eval <<-CEVAL def #{query_method}(value = true) - spawn.tap do |new_relation| - new_relation.#{query_method}_value = value - end + new_relation = spawn + new_relation.#{query_method}_value = value + new_relation end CEVAL end @@ -119,8 +117,16 @@ def build_arel end end - @where_values.uniq.each do |w| - arel = w.is_a?(String) ? arel.where(w) : arel.where(*w) + @where_values.uniq.each do |where| + next if where.blank? + + case where + when Arel::SqlLiteral + arel = arel.where(where) + else + sql = where.is_a?(String) ? where : where.to_sql + arel = arel.where(Arel::SqlLiteral.new("(#{sql})")) + end end @having_values.uniq.each do |h| @@ -135,7 +141,7 @@ def build_arel end @order_values.uniq.each do |o| - arel = arel.order(o) if o.present? + arel = arel.order(Arel::SqlLiteral.new(o.to_s)) if o.present? end selects = @select_values.uniq @@ -169,8 +175,7 @@ def build_where(*args) builder = PredicateBuilder.new(table.engine) conditions = if [String, Array].include?(args.first.class) - sql = @klass.send(:sanitize_sql, args.size > 1 ? args : args.first) - Arel::SqlLiteral.new("(#{sql})") if sql.present? + @klass.send(:sanitize_sql, args.size > 1 ? args : args.first) elsif args.first.is_a?(Hash) attributes = @klass.send(:expand_hash_conditions_for_aggregates, args.first) builder.build_from_hash(attributes, table) diff --git a/activerecord/lib/active_record/types.rb b/activerecord/lib/active_record/types.rb deleted file mode 100644 index 74f569352b0e7add21ed84d254d97e3e3c667bd4..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/types.rb +++ /dev/null @@ -1,38 +0,0 @@ -module ActiveRecord - module Types - extend ActiveSupport::Concern - - module ClassMethods - - def attribute_types - attribute_types = {} - columns.each do |column| - options = {} - options[:time_zone_aware] = time_zone_aware?(column.name) - options[:serialize] = serialized_attributes[column.name] - - attribute_types[column.name] = to_type(column, options) - end - attribute_types - end - - private - - def to_type(column, options = {}) - type_class = if options[:time_zone_aware] - Type::TimeWithZone - elsif options[:serialize] - Type::Serialize - elsif [ :integer, :float, :decimal ].include?(column.type) - Type::Number - else - Type::Object - end - - type_class.new(column, options) - end - - end - - end -end diff --git a/activerecord/lib/active_record/types/number.rb b/activerecord/lib/active_record/types/number.rb deleted file mode 100644 index cfbe87757544708e18c34f399f0a5112200a103b..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/types/number.rb +++ /dev/null @@ -1,30 +0,0 @@ -module ActiveRecord - module Type - class Number < Object - - def boolean(value) - value = cast(value) - !(value.nil? || value.zero?) - end - - def precast(value) - convert_number_column_value(value) - end - - private - - def convert_number_column_value(value) - if value == false - 0 - elsif value == true - 1 - elsif value.is_a?(String) && value.blank? - nil - else - value - end - end - - end - end -end \ No newline at end of file diff --git a/activerecord/lib/active_record/types/object.rb b/activerecord/lib/active_record/types/object.rb deleted file mode 100644 index ec3f861abd6c43491b54ad29e3fcd07c2c13e754..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/types/object.rb +++ /dev/null @@ -1,37 +0,0 @@ -module ActiveRecord - module Type - module Casting - - def cast(value) - typecaster.type_cast(value) - end - - def precast(value) - value - end - - def boolean(value) - cast(value).present? - end - - # Attributes::Typecasting stores appendable? types (e.g. serialized Arrays) when typecasting reads. - def appendable? - false - end - - end - - class Object - include Casting - - attr_reader :name, :options - attr_reader :typecaster - - def initialize(typecaster = nil, options = {}) - @typecaster, @options = typecaster, options - end - - end - - end -end \ No newline at end of file diff --git a/activerecord/lib/active_record/types/serialize.rb b/activerecord/lib/active_record/types/serialize.rb deleted file mode 100644 index 7b6af1981f9121ebf1f55744e404a7fc8f31af6d..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/types/serialize.rb +++ /dev/null @@ -1,33 +0,0 @@ -module ActiveRecord - module Type - class Serialize < Object - - def cast(value) - unserialize(value) - end - - def appendable? - true - end - - protected - - def unserialize(value) - unserialized_object = object_from_yaml(value) - - if unserialized_object.is_a?(@options[:serialize]) || unserialized_object.nil? - unserialized_object - else - raise SerializationTypeMismatch, - "#{name} was supposed to be a #{@options[:serialize]}, but was a #{unserialized_object.class.to_s}" - end - end - - def object_from_yaml(string) - return string unless string.is_a?(String) && string =~ /^---/ - YAML::load(string) rescue string - end - - end - end -end \ No newline at end of file diff --git a/activerecord/lib/active_record/types/time_with_zone.rb b/activerecord/lib/active_record/types/time_with_zone.rb deleted file mode 100644 index 3a8b9292f9474a072f8c03a81f5b0d5d5f4e5046..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/types/time_with_zone.rb +++ /dev/null @@ -1,20 +0,0 @@ -module ActiveRecord - module Type - class TimeWithZone < Object - - def cast(time) - time = super(time) - time.acts_like?(:time) ? time.in_time_zone : time - end - - def precast(time) - unless time.acts_like?(:time) - time = time.is_a?(String) ? ::Time.zone.parse(time) : time.to_time rescue time - end - time = time.in_time_zone rescue nil if time - super(time) - end - - end - end -end diff --git a/activerecord/lib/active_record/types/unknown.rb b/activerecord/lib/active_record/types/unknown.rb deleted file mode 100644 index f832c7b3045e21c6a2e278ba882898b131471068..0000000000000000000000000000000000000000 --- a/activerecord/lib/active_record/types/unknown.rb +++ /dev/null @@ -1,37 +0,0 @@ -module ActiveRecord - module Type - # Useful for handling attributes not mapped to types. Performs some boolean typecasting, - # but otherwise leaves the value untouched. - class Unknown - - def cast(value) - value - end - - def precast(value) - value - end - - # Attempts typecasting to handle numeric, false and blank values. - def boolean(value) - empty = (numeric?(value) && value.to_i.zero?) || false?(value) || value.blank? - !empty - end - - def appendable? - false - end - - protected - - def false?(value) - ActiveRecord::ConnectionAdapters::Column::FALSE_VALUES.include?(value) - end - - def numeric?(value) - Numeric === value || value !~ /[^0-9]/ - end - - end - end -end \ No newline at end of file diff --git a/activerecord/test/cases/attributes/aliasing_test.rb b/activerecord/test/cases/attributes/aliasing_test.rb deleted file mode 100644 index 7ee25779f17837e70e2c77a81bf42c999387ad4f..0000000000000000000000000000000000000000 --- a/activerecord/test/cases/attributes/aliasing_test.rb +++ /dev/null @@ -1,20 +0,0 @@ -require "cases/helper" - -class AliasingTest < ActiveRecord::TestCase - - class AliasingAttributes < Hash - include ActiveRecord::Attributes::Aliasing - end - - test "attribute access with aliasing" do - attributes = AliasingAttributes.new - attributes[:name] = 'Batman' - attributes.aliases['nickname'] = 'name' - - assert_equal 'Batman', attributes[:name], "Symbols should point to Strings" - assert_equal 'Batman', attributes['name'] - assert_equal 'Batman', attributes['nickname'] - assert_equal 'Batman', attributes[:nickname] - end - -end diff --git a/activerecord/test/cases/attributes/typecasting_test.rb b/activerecord/test/cases/attributes/typecasting_test.rb deleted file mode 100644 index 8a3b5513757ebe78cb937c275e57bbf72b6aefd6..0000000000000000000000000000000000000000 --- a/activerecord/test/cases/attributes/typecasting_test.rb +++ /dev/null @@ -1,120 +0,0 @@ -require "cases/helper" - -class TypecastingTest < ActiveRecord::TestCase - - class TypecastingAttributes < Hash - include ActiveRecord::Attributes::Typecasting - end - - module MockType - class Object - - def cast(value) - value - end - - def precast(value) - value - end - - def boolean(value) - !value.blank? - end - - def appendable? - false - end - - end - - class Integer < Object - - def cast(value) - value.to_i - end - - def precast(value) - value ? value : 0 - end - - def boolean(value) - !Float(value).zero? - end - - end - - class Serialize < Object - - def cast(value) - YAML::load(value) rescue value - end - - def precast(value) - value - end - - def appendable? - true - end - - end - end - - def setup - @attributes = TypecastingAttributes.new - @attributes.types.default = MockType::Object.new - @attributes.types['comments_count'] = MockType::Integer.new - end - - test "typecast on read" do - attributes = @attributes.merge('comments_count' => '5') - assert_equal 5, attributes['comments_count'] - end - - test "typecast on write" do - @attributes['comments_count'] = false - - assert_equal 0, @attributes.to_h['comments_count'] - end - - test "serialized objects" do - attributes = @attributes.merge('tags' => [ 'peanut butter' ].to_yaml) - attributes.types['tags'] = MockType::Serialize.new - attributes['tags'] << 'jelly' - - assert_equal [ 'peanut butter', 'jelly' ], attributes['tags'] - end - - test "without typecasting" do - @attributes.merge!('comments_count' => '5') - attributes = @attributes.without_typecast - - assert_equal '5', attributes['comments_count'] - assert_equal 5, @attributes['comments_count'], "Original attributes should typecast" - end - - - test "typecast all attributes" do - attributes = @attributes.merge('title' => 'I love sandwiches', 'comments_count' => '5') - attributes.typecast! - - assert_equal({ 'title' => 'I love sandwiches', 'comments_count' => 5 }, attributes) - end - - test "query for has? value" do - attributes = @attributes.merge('comments_count' => '1') - - assert_equal true, attributes.has?('comments_count') - attributes['comments_count'] = '0' - assert_equal false, attributes.has?('comments_count') - end - - test "attributes to Hash" do - attributes_hash = { 'title' => 'I love sandwiches', 'comments_count' => '5' } - attributes = @attributes.merge(attributes_hash) - - assert_equal Hash, attributes.to_h.class - assert_equal attributes_hash, attributes.to_h - end - -end diff --git a/activerecord/test/cases/subscriber_test.rb b/activerecord/test/cases/subscriber_test.rb index ce91d9385d32bd1ce137d0358feecfad8d35d7cd..5328d4468b6fb52d562cb7d20b1114e636d11ed9 100644 --- a/activerecord/test/cases/subscriber_test.rb +++ b/activerecord/test/cases/subscriber_test.rb @@ -3,7 +3,8 @@ require "rails/subscriber/test_helper" require "active_record/railties/subscriber" -module SubscriberTest +class SubscriberTest < ActiveSupport::TestCase + include Rails::Subscriber::TestHelper Rails::Subscriber.add(:active_record, ActiveRecord::Railties::Subscriber.new) def setup @@ -38,14 +39,4 @@ def test_cached_queries assert_match /CACHE/, @logger.logged(:debug).last assert_match /SELECT .*?FROM .?developers.?/, @logger.logged(:debug).last end - - class SyncSubscriberTest < ActiveSupport::TestCase - include Rails::Subscriber::SyncTestHelper - include SubscriberTest - end - - class AsyncSubscriberTest < ActiveSupport::TestCase - include Rails::Subscriber::AsyncTestHelper - include SubscriberTest - end end \ No newline at end of file diff --git a/activerecord/test/cases/types/number_test.rb b/activerecord/test/cases/types/number_test.rb deleted file mode 100644 index ee7216a0f1879e4f95e4bf2d83d05549ddcd1512..0000000000000000000000000000000000000000 --- a/activerecord/test/cases/types/number_test.rb +++ /dev/null @@ -1,30 +0,0 @@ -require "cases/helper" - -class NumberTest < ActiveRecord::TestCase - - def setup - @column = ActiveRecord::ConnectionAdapters::Column.new('comments_count', 0, 'integer') - @number = ActiveRecord::Type::Number.new(@column) - end - - test "typecast" do - assert_equal 1, @number.cast(1) - assert_equal 1, @number.cast('1') - assert_equal 0, @number.cast('') - - assert_equal 0, @number.precast(false) - assert_equal 1, @number.precast(true) - assert_equal nil, @number.precast('') - assert_equal 0, @number.precast(0) - end - - test "cast as boolean" do - assert_equal true, @number.boolean('1') - assert_equal true, @number.boolean(1) - - assert_equal false, @number.boolean(0) - assert_equal false, @number.boolean('0') - assert_equal false, @number.boolean(nil) - end - -end diff --git a/activerecord/test/cases/types/object_test.rb b/activerecord/test/cases/types/object_test.rb deleted file mode 100644 index f2667a9b00c0b32ee8a0e06416b6ca372cf476ee..0000000000000000000000000000000000000000 --- a/activerecord/test/cases/types/object_test.rb +++ /dev/null @@ -1,24 +0,0 @@ -require "cases/helper" - -class ObjectTest < ActiveRecord::TestCase - - def setup - @column = ActiveRecord::ConnectionAdapters::Column.new('name', '', 'date') - @object = ActiveRecord::Type::Object.new(@column) - end - - test "typecast with column" do - date = Date.new(2009, 7, 10) - assert_equal date, @object.cast('10-07-2009') - assert_equal nil, @object.cast('') - - assert_equal date, @object.precast(date) - end - - test "cast as boolean" do - assert_equal false, @object.boolean(nil) - assert_equal false, @object.boolean('false') - assert_equal true, @object.boolean('10-07-2009') - end - -end diff --git a/activerecord/test/cases/types/serialize_test.rb b/activerecord/test/cases/types/serialize_test.rb deleted file mode 100644 index e9423a5b9d45025d48c8f4d60657de27523ded0b..0000000000000000000000000000000000000000 --- a/activerecord/test/cases/types/serialize_test.rb +++ /dev/null @@ -1,20 +0,0 @@ -require "cases/helper" - -class SerializeTest < ActiveRecord::TestCase - - test "typecast" do - serializer = ActiveRecord::Type::Serialize.new(column = nil, :serialize => Array) - - assert_equal [], serializer.cast([].to_yaml) - assert_equal ['1'], serializer.cast(['1'].to_yaml) - assert_equal nil, serializer.cast(nil.to_yaml) - end - - test "cast as boolean" do - serializer = ActiveRecord::Type::Serialize.new(column = nil, :serialize => Array) - - assert_equal true, serializer.boolean(['1'].to_yaml) - assert_equal false, serializer.boolean([].to_yaml) - end - -end \ No newline at end of file diff --git a/activerecord/test/cases/types/time_with_zone_test.rb b/activerecord/test/cases/types/time_with_zone_test.rb deleted file mode 100644 index b3de79a6c84f6ba5b460fc1c80f982e8a69de15f..0000000000000000000000000000000000000000 --- a/activerecord/test/cases/types/time_with_zone_test.rb +++ /dev/null @@ -1,42 +0,0 @@ -require "cases/helper" - -class TimeWithZoneTest < ActiveRecord::TestCase - - def setup - @column = ActiveRecord::ConnectionAdapters::Column.new('created_at', 0, 'datetime') - @time_with_zone = ActiveRecord::Type::TimeWithZone.new(@column) - end - - test "typecast" do - Time.use_zone("Pacific Time (US & Canada)") do - time_string = "2009-10-07 21:29:10" - time = Time.zone.parse(time_string) - - # assert_equal time, @time_with_zone.cast(time_string) - assert_equal nil, @time_with_zone.cast('') - assert_equal nil, @time_with_zone.cast(nil) - - assert_equal time, @time_with_zone.precast(time) - assert_equal time, @time_with_zone.precast(time_string) - assert_equal time, @time_with_zone.precast(time.to_time) - # assert_equal "#{time.to_date.to_s} 00:00:00 -0700", @time_with_zone.precast(time.to_date).to_s - end - end - - test "cast as boolean" do - Time.use_zone('Central Time (US & Canada)') do - time = Time.zone.now - - assert_equal true, @time_with_zone.boolean(time) - assert_equal true, @time_with_zone.boolean(time.to_date) - assert_equal true, @time_with_zone.boolean(time.to_time) - - assert_equal true, @time_with_zone.boolean(time.to_s) - assert_equal true, @time_with_zone.boolean(time.to_date.to_s) - assert_equal true, @time_with_zone.boolean(time.to_time.to_s) - - assert_equal false, @time_with_zone.boolean('') - end - end - -end diff --git a/activerecord/test/cases/types/unknown_test.rb b/activerecord/test/cases/types/unknown_test.rb deleted file mode 100644 index 230d67b2fb37ca5d8d8a6ffd8ca2821d6de7baf0..0000000000000000000000000000000000000000 --- a/activerecord/test/cases/types/unknown_test.rb +++ /dev/null @@ -1,29 +0,0 @@ -require "cases/helper" - -class UnknownTest < ActiveRecord::TestCase - - test "typecast attributes does't modify values" do - unkown = ActiveRecord::Type::Unknown.new - person = { 'name' => '0' } - - assert_equal person['name'], unkown.cast(person['name']) - assert_equal person['name'], unkown.precast(person['name']) - end - - test "cast as boolean" do - person = { 'id' => 0, 'name' => ' ', 'admin' => 'false', 'votes' => '0' } - unkown = ActiveRecord::Type::Unknown.new - - assert_equal false, unkown.boolean(person['votes']) - assert_equal false, unkown.boolean(person['admin']) - assert_equal false, unkown.boolean(person['name']) - assert_equal false, unkown.boolean(person['id']) - - person = { 'id' => 5, 'name' => 'Eric', 'admin' => 'true', 'votes' => '25' } - assert_equal true, unkown.boolean(person['votes']) - assert_equal true, unkown.boolean(person['admin']) - assert_equal true, unkown.boolean(person['name']) - assert_equal true, unkown.boolean(person['id']) - end - -end \ No newline at end of file diff --git a/activerecord/test/cases/types_test.rb b/activerecord/test/cases/types_test.rb deleted file mode 100644 index 403a9a6e022de7f0354e7379cd3b538d0a2ccd68..0000000000000000000000000000000000000000 --- a/activerecord/test/cases/types_test.rb +++ /dev/null @@ -1,32 +0,0 @@ -require "cases/helper" -require 'models/topic' - -class TypesTest < ActiveRecord::TestCase - - test "attribute types from columns" do - begin - ActiveRecord::Base.time_zone_aware_attributes = true - attribute_type_classes = {} - Topic.attribute_types.each { |key, type| attribute_type_classes[key] = type.class } - - expected = { "id" => ActiveRecord::Type::Number, - "replies_count" => ActiveRecord::Type::Number, - "parent_id" => ActiveRecord::Type::Number, - "content" => ActiveRecord::Type::Serialize, - "written_on" => ActiveRecord::Type::TimeWithZone, - "title" => ActiveRecord::Type::Object, - "author_name" => ActiveRecord::Type::Object, - "approved" => ActiveRecord::Type::Object, - "parent_title" => ActiveRecord::Type::Object, - "bonus_time" => ActiveRecord::Type::Object, - "type" => ActiveRecord::Type::Object, - "last_read" => ActiveRecord::Type::Object, - "author_email_address" => ActiveRecord::Type::Object } - - assert_equal expected, attribute_type_classes - ensure - ActiveRecord::Base.time_zone_aware_attributes = false - end - end - -end diff --git a/activeresource/test/cases/subscriber_test.rb b/activeresource/test/cases/subscriber_test.rb index 7100b02872f90e7e3a8e357d176c40e238ebbd55..3556fbf7cb32043a6d76eb030178d11a243e4f65 100644 --- a/activeresource/test/cases/subscriber_test.rb +++ b/activeresource/test/cases/subscriber_test.rb @@ -3,7 +3,8 @@ require "rails/subscriber/test_helper" require "active_resource/railties/subscriber" -module SubscriberTest +class SubscriberTest < ActiveSupport::TestCase + include Rails::Subscriber::TestHelper Rails::Subscriber.add(:active_resource, ActiveResource::Railties::Subscriber.new) def setup @@ -26,14 +27,4 @@ def test_request_notification assert_equal "GET http://somewhere.else:80/people/1.xml", @logger.logged(:info)[0] assert_match /\-\-\> 200 200 106/, @logger.logged(:info)[1] end - - class SyncSubscriberTest < ActiveSupport::TestCase - include Rails::Subscriber::SyncTestHelper - include SubscriberTest - end - - class AsyncSubscriberTest < ActiveSupport::TestCase - include Rails::Subscriber::AsyncTestHelper - include SubscriberTest - end end \ No newline at end of file diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index bb07e4765c7d93c773e07888bc9688303e00ad43..ac482a2ec82f9507e8df79e86f6aba28c1d6d749 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -3,11 +3,9 @@ module ActiveSupport module Notifications # This is a default queue implementation that ships with Notifications. It - # consumes events in a thread and publish them to all registered subscribers. - # + # just pushes events to all registered subscribers. class Fanout - def initialize(sync = false) - @subscriber_klass = sync ? Subscriber : AsyncSubscriber + def initialize @subscribers = [] end @@ -16,7 +14,7 @@ def bind(pattern) end def subscribe(pattern = nil, &block) - @subscribers << @subscriber_klass.new(pattern, &block) + @subscribers << Subscriber.new(pattern, &block) end def publish(*args) @@ -68,34 +66,6 @@ def push(*args) @block.call(*args) end end - - # Used for internal implementation only. - class AsyncSubscriber < Subscriber #:nodoc: - def initialize(pattern, &block) - super - @events = Queue.new - start_consumer - end - - def drained? - @events.empty? - end - - private - def start_consumer - Thread.new { consume } - end - - def consume - while args = @events.shift - @block.call(*args) - end - end - - def push(*args) - @events << args - end - end end end end diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index c41d81fe7e9dfaf08f4050e9996ee273e5d9525f..d3af535c26c38146b14a4afe6e7eb914e2b4b2fc 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -3,18 +3,12 @@ module Notifications class TestCase < ActiveSupport::TestCase def setup - Thread.abort_on_exception = true - ActiveSupport::Notifications.notifier = nil @notifier = ActiveSupport::Notifications.notifier @events = [] @notifier.subscribe { |*args| @events << event(*args) } end - def teardown - Thread.abort_on_exception = false - end - private def event(*args) ActiveSupport::Notifications::Event.new(*args) @@ -25,7 +19,7 @@ def drain end end - class PubSubTest < TestCase + class SyncPubSubTest < TestCase def test_events_are_published_to_a_listener @notifier.publish :foo @notifier.wait @@ -72,16 +66,6 @@ def event(*args) end end - class SyncPubSubTest < PubSubTest - def setup - Thread.abort_on_exception = true - - @notifier = ActiveSupport::Notifications::Notifier.new(ActiveSupport::Notifications::Fanout.new(true)) - @events = [] - @notifier.subscribe { |*args| @events << event(*args) } - end - end - class InstrumentationTest < TestCase delegate :instrument, :instrument!, :to => ActiveSupport::Notifications 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 2cdf4eae5470cb262788e75e1db8c9a8f72a4c0f..e8065d9505d7ec6f727021c827de18a0a95737de 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 334820826ff8a88fb72e2dd0a5bddcc4ca8ada59..78a355d2f4ad84768f3a566adb5b51596fee3bd0 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 @@ class Application < Rails::Application # 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 743681359cb2fc22d5d492a6bbbd2d881d48b6b9..5c9892c6305fd944b2fbc594dfd58cdbea3d6cc5 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -110,6 +110,7 @@ def app 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 115499db05cb3a17992128fd31169dec9d9b81b9..b21ae2a17b572d07fa67e5f4f02e0abe0a300dc6 100644 --- a/railties/lib/rails/commands/server.rb +++ b/railties/lib/rails/commands/server.rb @@ -46,7 +46,6 @@ def start 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 @@ def start 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 @@ def default_options :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 b76a7ac2d87cd5548bf4320775360bb5118239f8..7f1783a6b9951361f7725966d71a66553277a9ad 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -10,15 +10,15 @@ def self.default 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 @@ def config_keys 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 @@ def initialize(base = nil) 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 d3175e6a9d7200457231d01bd60af41c7e66d579..83b8c74966e296f0ba9b1e865aea74f6cb526234 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -134,9 +134,14 @@ def self.find_by_namespace(name, base=nil, context=nil) #:nodoc: 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 @@ def self.lookup(namespaces) #:nodoc: 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 @@ def self.lookup(namespaces) #:nodoc: 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 @@ def self.namespaces_to_paths(namespaces) #:nodoc: 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 36945a6b0fd2d3e235106e4842547032b3155725..4bc0c2c88b31618bc156dc20e1becf14467d7b5f 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 0000000000000000000000000000000000000000..91a613092fde657bf434f04ec320ee1c4e63e112 --- /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 9965786d86488071cccaf81c6adfa74ebc2d25b3..8c62f562d9b633182485f34efdc247b3eae9dbbf 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 @@ def self.dispatch(args) 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 1464767ed9ecaa71ad45bd2ce48fae2cf1baae0e..39b411737259b72c36956a2057698101f9c8d144 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 @@ class Subscriber # module TestHelper def setup - Thread.abort_on_exception = true - @logger = MockLogger.new @notifier = ActiveSupport::Notifications::Notifier.new(queue) @@ -54,7 +52,6 @@ def setup def teardown set_logger(nil) ActiveSupport::Notifications.notifier = nil - Thread.abort_on_exception = false end class MockLogger @@ -92,26 +89,9 @@ def wait 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 79dfacdcdbcc7eab3d1eb206de63e02297372094..6968e87986df9f411e4b3ff9d691531897e426ee 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -122,5 +122,17 @@ def setup 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 1c5cc62ecd1184f0a4fa23116f36697d0c72d882..8c4247e8409de774919d87c308b7464f24009748 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -17,8 +17,8 @@ def setup "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 0000000000000000000000000000000000000000..9098a8a3548ad44b61521d9ec9500a23e2247068 --- /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 similarity index 100% rename from railties/test/fixtures/lib/generators/foobar/foobar_generator.rb rename to 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 664d1e5670e7de35383beb40677ec20674e5711b..f37b684f73667cda59570faf3209bb3c5be7f6e4 100644 --- a/railties/test/generators_test.rb +++ b/railties/test/generators_test.rb @@ -16,6 +16,7 @@ def teardown 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 @@ def test_help_when_a_generator_with_required_arguments_is_invoked_without_argume 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 724e8a75d6cffc1b6859e75bc5eb5a1c26bca30f..f6c895093f3288be2c5102478fd0ead75d0a271c 100644 --- a/railties/test/subscriber_test.rb +++ b/railties/test/subscriber_test.rb @@ -24,7 +24,9 @@ def puke(event) end end -module SubscriberTest +class SyncSubscriberTest < ActiveSupport::TestCase + include Rails::Subscriber::TestHelper + def setup super @subscriber = MySubscriber.new @@ -94,51 +96,24 @@ def test_flushes_loggers 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