diff options
135 files changed, 1632 insertions, 2525 deletions
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index a8233512ab..356861b591 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -524,14 +524,16 @@ module ActionMailer #:nodoc: def deliver!(mail = @mail) raise "no mail object available for delivery!" unless mail - begin - ActiveSupport::Notifications.instrument("action_mailer.deliver", - :template => template, :mailer => self.class.name) do |payload| - self.class.set_payload_for_mail(payload, mail) + ActiveSupport::Notifications.instrument("action_mailer.deliver", + :template => template, :mailer => self.class.name) do |payload| + + self.class.set_payload_for_mail(payload, mail) + + begin self.delivery_method.perform_delivery(mail) if perform_deliveries + rescue Exception => e # Net::SMTP errors or sendmail pipe errors + raise e if raise_delivery_errors end - rescue Exception => e # Net::SMTP errors or sendmail pipe errors - raise e if raise_delivery_errors end mail diff --git a/actionmailer/lib/action_mailer/railties/subscriber.rb b/actionmailer/lib/action_mailer/railties/subscriber.rb index af9c477237..cff852055c 100644 --- a/actionmailer/lib/action_mailer/railties/subscriber.rb +++ b/actionmailer/lib/action_mailer/railties/subscriber.rb @@ -3,13 +3,13 @@ module ActionMailer class Subscriber < Rails::Subscriber def deliver(event) recipients = Array(event.payload[:to]).join(', ') - info("Sent mail to #{recipients} (%1.fms)" % event.duration) - debug("\n#{event.payload[:mail]}") + info("\nSent mail to #{recipients} (%1.fms)" % event.duration) + debug(event.payload[:mail]) end def receive(event) - info("Received mail (%.1fms)" % event.duration) - debug("\n#{event.payload[:mail]}") + info("\nReceived mail (%.1fms)" % event.duration) + debug(event.payload[:mail]) end def logger diff --git a/actionmailer/lib/action_mailer/test_case.rb b/actionmailer/lib/action_mailer/test_case.rb index 318a1e46d1..0ca4f5494e 100644 --- a/actionmailer/lib/action_mailer/test_case.rb +++ b/actionmailer/lib/action_mailer/test_case.rb @@ -56,7 +56,7 @@ module ActionMailer end def read_fixture(action) - IO.readlines(File.join(RAILS_ROOT, 'test', 'fixtures', self.class.mailer_class.name.underscore, action)) + IO.readlines(File.join(Rails.root, 'test', 'fixtures', self.class.mailer_class.name.underscore, action)) end end end diff --git a/actionmailer/test/subscriber_test.rb b/actionmailer/test/subscriber_test.rb index 01a71f481d..aed5d2ca7e 100644 --- a/actionmailer/test/subscriber_test.rb +++ b/actionmailer/test/subscriber_test.rb @@ -2,7 +2,8 @@ require "abstract_unit" 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 @@ module SubscriberTest 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/abstract_controller.rb b/actionpack/lib/abstract_controller.rb index efc35a7e56..725d8fb8fc 100644 --- a/actionpack/lib/abstract_controller.rb +++ b/actionpack/lib/abstract_controller.rb @@ -15,5 +15,6 @@ module AbstractController autoload :LocalizedCache autoload :Logger autoload :Rendering + autoload :Translation autoload :UrlFor end diff --git a/actionpack/lib/abstract_controller/localized_cache.rb b/actionpack/lib/abstract_controller/localized_cache.rb index bf648af60a..5e3efa002c 100644 --- a/actionpack/lib/abstract_controller/localized_cache.rb +++ b/actionpack/lib/abstract_controller/localized_cache.rb @@ -34,7 +34,7 @@ module AbstractController end end - def render(options) + def render(*args) Thread.current[:format_locale_key] = HashKey.get(self.class, formats, I18n.locale) super end diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index d57136dbb8..a168b1b4c5 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -40,12 +40,13 @@ module AbstractController # Mostly abstracts the fact that calling render twice is a DoubleRenderError. # Delegates render_to_body and sticks the result in self.response_body. - def render(*args) + def render(*args, &block) if response_body - raise AbstractController::DoubleRenderError, "OMG" + raise AbstractController::DoubleRenderError end - self.response_body = render_to_body(*args) + options = _normalize_options(*args, &block) + self.response_body = render_to_body(options) end # Raw rendering of a template to a Rack-compatible body. @@ -69,7 +70,8 @@ module AbstractController # render_to_body into a String. # # :api: plugin - def render_to_string(options = {}) + def render_to_string(*args) + options = _normalize_options(*args) AbstractController::Rendering.body_to_s(render_to_body(options)) end @@ -96,6 +98,11 @@ module AbstractController _view_paths end + # The prefix used in render "foo" shortcuts. + def _prefix + controller_path + end + # Return a string representation of a Rack-compatible response body. def self.body_to_s(body) if body.respond_to?(:to_str) @@ -110,6 +117,28 @@ module AbstractController private + # Normalize options, by converting render "foo" to render :template => "prefix/foo" + # and render "/foo" to render :file => "/foo". + def _normalize_options(action=nil, options={}) + case action + when Hash + options, action = action, nil + when String, Symbol + action = action.to_s + case action.index("/") + when NilClass + options[:_prefix] = _prefix + options[:_template_name] = action + when 0 + options[:file] = action + else + options[:template] = action + end + end + + options + end + # Take in a set of options and determine the template to render # # ==== Options diff --git a/actionpack/lib/action_controller/translation.rb b/actionpack/lib/abstract_controller/translation.rb index 65e9eddb0a..6d68cf4944 100644 --- a/actionpack/lib/action_controller/translation.rb +++ b/actionpack/lib/abstract_controller/translation.rb @@ -1,4 +1,4 @@ -module ActionController +module AbstractController module Translation def translate(*args) I18n.translate(*args) diff --git a/actionpack/lib/action_controller.rb b/actionpack/lib/action_controller.rb index 8bc2cc79d2..33e107d216 100644 --- a/actionpack/lib/action_controller.rb +++ b/actionpack/lib/action_controller.rb @@ -8,7 +8,6 @@ module ActionController autoload :Base autoload :Caching autoload :PolymorphicRoutes - autoload :Translation autoload :Metal autoload :Middleware @@ -17,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 260e5da336..f46231d72b 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -4,6 +4,7 @@ module ActionController include AbstractController::Callbacks include AbstractController::Layouts + include AbstractController::Translation include ActionController::Helpers helper :all # By default, all helpers should be included @@ -32,12 +33,10 @@ module ActionController include ActionController::Streaming include ActionController::HttpAuthentication::Basic::ControllerMethods include ActionController::HttpAuthentication::Digest::ControllerMethods - include ActionController::Translation # Add instrumentations hooks at the bottom, to ensure they instrument # all the methods properly. include ActionController::Instrumentation - include ActionController::FilterParameterLogging # TODO: Extract into its own module # This should be moved together with other normalizing behavior @@ -74,40 +73,13 @@ module ActionController @subclasses ||= [] end - def _normalize_options(action = nil, options = {}, &blk) - if action.is_a?(Hash) - options, action = action, nil - elsif action.is_a?(String) || action.is_a?(Symbol) - key = case action = action.to_s - when %r{^/} then :file - when %r{/} then :template - else :action - end - options.merge! key => action - elsif action - options.merge! :partial => action - end - - if options.key?(:action) && options[:action].to_s.index("/") - options[:template] = options.delete(:action) - end - - if options[:status] - options[:status] = Rack::Utils.status_code(options[:status]) - end - - options[:update] = blk if block_given? - options - end - - def render(action = nil, options = {}, &blk) - options = _normalize_options(action, options, &blk) - super(options) - end - - def render_to_string(action = nil, options = {}, &blk) - options = _normalize_options(action, options, &blk) - super(options) + # 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 end end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 1819c0f886..57627a6f0b 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -60,6 +60,7 @@ module ActionController # :api: private def dispatch(name, env) @_env = env + @_env['action_controller.instance'] = self process(name) to_a end 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 0b1e1ee6ab..0000000000 --- a/actionpack/lib/action_controller/metal/filter_parameter_logging.rb +++ /dev/null @@ -1,71 +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 append_info_to_payload(payload) - super - payload[:params] = filter_parameters(request.params) - end - - def filter_parameters(params) - params.dup.except!(*INTERNAL_PARAMS) - end - - end -end diff --git a/actionpack/lib/action_controller/metal/helpers.rb b/actionpack/lib/action_controller/metal/helpers.rb index d0402e5bad..cdd14560e1 100644 --- a/actionpack/lib/action_controller/metal/helpers.rb +++ b/actionpack/lib/action_controller/metal/helpers.rb @@ -100,7 +100,7 @@ module ActionController module_path = module_name.underscore helper module_path rescue MissingSourceFile => e - raise e unless e.is_missing? "#{module_path}_helper" + raise e unless e.is_missing? "helpers/#{module_path}_helper" rescue NameError => e raise e unless e.missing_name? "#{module_name}Helper" end diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 876f778751..7f9a7c068b 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -9,35 +9,34 @@ module ActionController module Instrumentation extend ActiveSupport::Concern - included do - include AbstractController::Logger - end + include AbstractController::Logger attr_internal :view_runtime def process_action(action, *args) - ActiveSupport::Notifications.instrument("action_controller.process_action") do |payload| + raw_payload = { + :controller => self.class.name, + :action => self.action_name, + :params => request.filtered_parameters, + :formats => request.formats.map(&:to_sym) + } + + ActiveSupport::Notifications.instrument("action_controller.start_processing", raw_payload.dup) + + ActiveSupport::Notifications.instrument("action_controller.process_action", raw_payload) do |payload| result = super - payload[:controller] = self.class.name - payload[:action] = self.action_name - payload[:status] = response.status + payload[:status] = response.status append_info_to_payload(payload) result end end - def render(*args, &block) - if logger - render_output = nil - - self.view_runtime = cleanup_view_runtime do - Benchmark.ms { render_output = super } - end - - render_output - else - super + def render(*args) + render_output = nil + self.view_runtime = cleanup_view_runtime do + Benchmark.ms { render_output = super } end + render_output end def send_file(path, options={}) diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index 74e50bb032..72e2bbd00e 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -12,9 +12,10 @@ module ActionController super end - def render(options) - super - self.content_type ||= options[:_template].mime_type.to_s + def render(*args) + args << {} unless args.last.is_a?(Hash) + super(*args) + self.content_type ||= args.last[:_template].mime_type.to_s response_body end @@ -24,18 +25,6 @@ module ActionController end private - def _prefix - controller_path - end - - def _determine_template(options) - if (options.keys & [:partial, :file, :template, :text, :inline]).empty? - options[:_template_name] ||= options[:action] - options[:_prefix] = _prefix - end - - super - end def _render_partial(options) options[:partial] = action_name if options[:partial] == true @@ -53,5 +42,29 @@ module ActionController self.content_type = content_type if content_type self.headers["Location"] = url_for(location) if location end + + def _normalize_options(action=nil, options={}, &blk) + case action + when NilClass + when Hash + options = super(action.delete(:action), action) + when String, Symbol + options = super + else + options.merge! :partial => action + end + + if (options.keys & [:partial, :file, :template, :text, :inline]).empty? + options[:_template_name] ||= options[:action] + options[:_prefix] = _prefix + end + + if options[:status] + options[:status] = Rack::Utils.status_code(options[:status]) + end + + options[:update] = blk if block_given? + options + end end end diff --git a/actionpack/lib/action_controller/railties/subscriber.rb b/actionpack/lib/action_controller/railties/subscriber.rb index 6659e5df47..1f0e6bf51a 100644 --- a/actionpack/lib/action_controller/railties/subscriber.rb +++ b/actionpack/lib/action_controller/railties/subscriber.rb @@ -1,15 +1,23 @@ module ActionController module Railties class Subscriber < Rails::Subscriber - def process_action(event) + INTERNAL_PARAMS = %w(controller action format _method only_path) + + def start_processing(event) payload = event.payload - info " Parameters: #{payload[:params].inspect}" unless payload[:params].blank? + params = payload[:params].except(*INTERNAL_PARAMS) + info " Processing by #{payload[:controller]}##{payload[:action]} as #{payload[:formats].first.to_s.upcase}" + info " Parameters: #{params.inspect}" unless params.empty? + end + + def process_action(event) + payload = event.payload additions = ActionController::Base.log_process_action(payload) message = "Completed in %.0fms" % event.duration message << " (#{additions.join(" | ")})" unless additions.blank? - message << " by #{payload[:controller]}##{payload[:action]} [#{payload[:status]}]" + message << " with #{payload[:status]}" info(message) end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 082562d921..f0490a5619 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 ActionDispatch 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 0000000000..1958e1668d --- /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 97546d5f93..40b40ea94e 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -29,9 +29,8 @@ module ActionDispatch def path_parameters @env["action_dispatch.request.path_parameters"] ||= {} end - - private + private # Convert nested Hashs to HashWithIndifferentAccess def normalize_parameters(value) case value diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 187ce7c15d..7a17023ed2 100755 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -11,6 +11,7 @@ module ActionDispatch 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 01d2cbb435..0000000000 --- a/actionpack/lib/action_dispatch/middleware/notifications.rb +++ /dev/null @@ -1,24 +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(stack_env) - env = stack_env.dup - ActiveSupport::Notifications.instrument("action_dispatch.before_dispatch", :env => env) - - ActiveSupport::Notifications.instrument!("action_dispatch.after_dispatch", :env => env) do - @app.call(stack_env) - end - rescue Exception => exception - ActiveSupport::Notifications.instrument('action_dispatch.exception', - :env => stack_env, :exception => exception) - raise exception - end - end -end
\ No newline at end of file diff --git a/actionpack/lib/action_dispatch/middleware/params_parser.rb b/actionpack/lib/action_dispatch/middleware/params_parser.rb index 534390d4aa..522982e202 100644 --- a/actionpack/lib/action_dispatch/middleware/params_parser.rb +++ b/actionpack/lib/action_dispatch/middleware/params_parser.rb @@ -35,14 +35,14 @@ module ActionDispatch when Proc strategy.call(request.raw_post) when :xml_simple, :xml_node - request.body.size == 0 ? {} : Hash.from_xml(request.body).with_indifferent_access + request.body.size == 0 ? {} : Hash.from_xml(request.raw_post).with_indifferent_access when :yaml - YAML.load(request.body) + YAML.load(request.raw_post) when :json if request.body.size == 0 {} else - data = ActiveSupport::JSON.decode(request.body) + data = ActiveSupport::JSON.decode(request.raw_post) data = {:_json => data} unless data.is_a?(Hash) data.with_indifferent_access end diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index 24be4fee55..0dc1d70e37 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -60,9 +60,7 @@ module ActionDispatch end def inspect - str = klass.to_s - args.each { |arg| str += ", #{build_args.inspect}" } - str + klass.to_s end def build(app) diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 18978bfb39..e4bd143e78 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 c08a844c6a..0000000000 --- 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\nProcessing #{path} to #{request.formats.join(', ')} " << - "(for #{request.remote_ip} at #{event.time.to_s(:db)}) [#{request.method.to_s.upcase}]" - end - - def logger - ActionController::Base.logger - end - end - end -end
\ No newline at end of file diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 811c287355..fcbb70749f 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -375,6 +375,15 @@ module ActionDispatch end end + def action_type(action) + case action + when :index, :create + :collection + when :show, :update, :destroy + :member + end + end + def name options[:as] || plural end @@ -391,6 +400,15 @@ module ActionDispatch plural end + def name_for_action(action) + case action_type(action) + when :collection + collection_name + when :member + member_name + end + end + def id_segment ":#{singular}_id" end @@ -405,6 +423,13 @@ module ActionDispatch super end + def action_type(action) + case action + when :show, :create, :update, :destroy + :member + end + end + def name options[:as] || singular end @@ -428,7 +453,7 @@ module ActionDispatch with_scope_level(:resource, resource) do yield if block_given? - get :show, :as => resource.member_name if resource.actions.include?(:show) + get :show if resource.actions.include?(:show) post :create if resource.actions.include?(:create) put :update if resource.actions.include?(:update) delete :destroy if resource.actions.include?(:destroy) @@ -454,14 +479,14 @@ module ActionDispatch yield if block_given? with_scope_level(:collection) do - get :index, :as => resource.collection_name if resource.actions.include?(:index) + get :index if resource.actions.include?(:index) post :create if resource.actions.include?(:create) get :new, :as => resource.singular if resource.actions.include?(:new) end with_scope_level(:member) do scope(':id') do - get :show, :as => resource.member_name if resource.actions.include?(:show) + get :show if resource.actions.include?(:show) put :update if resource.actions.include?(:update) delete :destroy if resource.actions.include?(:destroy) get :edit, :as => resource.singular if resource.actions.include?(:edit) @@ -525,7 +550,10 @@ module ActionDispatch begin old_path = @scope[:path] @scope[:path] = "#{@scope[:path]}(.:format)" - return match(options.reverse_merge(:to => action)) + return match(options.reverse_merge( + :to => action, + :as => parent_resource.name_for_action(action) + )) ensure @scope[:path] = old_path end diff --git a/actionpack/lib/action_dispatch/routing/route.rb b/actionpack/lib/action_dispatch/routing/route.rb index f1431e7a37..e6e44d3546 100644 --- a/actionpack/lib/action_dispatch/routing/route.rb +++ b/actionpack/lib/action_dispatch/routing/route.rb @@ -44,6 +44,12 @@ module ActionDispatch def to_a [@app, @conditions, @defaults, @name] end + + def to_s + @to_s ||= begin + "%-6s %-40s %s" % [(verb || :any).to_s.upcase, path, requirements.inspect] + end + end end end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 4ec47d146c..2093bb3a0e 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -240,9 +240,9 @@ module ActionDispatch path = location.query ? "#{location.path}?#{location.query}" : location.path end - [ControllerCapture, ActionController::Testing].each do |mod| - unless ActionController::Base < mod - ActionController::Base.class_eval { include mod } + unless ActionController::Base < ActionController::Testing + ActionController::Base.class_eval do + include ActionController::Testing end end @@ -259,7 +259,9 @@ module ActionDispatch "HTTP_HOST" => host, "REMOTE_ADDR" => remote_addr, "CONTENT_TYPE" => "application/x-www-form-urlencoded", - "HTTP_ACCEPT" => accept + "HTTP_ACCEPT" => accept, + + "action_dispatch.show_exceptions" => false } (rack_environment || {}).each do |key, value| @@ -267,16 +269,15 @@ module ActionDispatch end session = Rack::Test::Session.new(@mock_session) - - @controller = ActionController::Base.capture_instantiation do - session.request(path, env) - end + session.request(path, env) @request_count += 1 @request = ActionDispatch::Request.new(session.last_request.env) @response = ActionDispatch::TestResponse.from_response(@mock_session.last_response) @html_document = nil + @controller = session.last_request.env['action_controller.instance'] + return response.status end @@ -294,31 +295,6 @@ module ActionDispatch end end - # A module used to extend ActionController::Base, so that integration tests - # can capture the controller used to satisfy a request. - module ControllerCapture #:nodoc: - extend ActiveSupport::Concern - - included do - alias_method_chain :initialize, :capture - end - - def initialize_with_capture(*args) - initialize_without_capture - self.class.last_instantiation ||= self - end - - module ClassMethods #:nodoc: - mattr_accessor :last_instantiation - - def capture_instantiation - self.last_instantiation = nil - yield - return last_instantiation - end - end - end - module Runner def app @app diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index 4970c768e8..c4b0455c2a 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -181,7 +181,6 @@ module ActionView #:nodoc: extend ActiveSupport::Memoizable attr_accessor :base_path, :assigns, :template_extension, :formats - attr_accessor :controller attr_internal :captures def reset_formats(formats) @@ -277,13 +276,13 @@ module ActionView #:nodoc: @config = nil @formats = formats @assigns = assigns_for_first_render.each { |key, value| instance_variable_set("@#{key}", value) } - @controller = controller + @_controller = controller @helpers = self.class.helpers || Module.new @_content_for = Hash.new {|h,k| h[k] = ActionView::SafeBuffer.new } self.view_paths = view_paths end - attr_internal :template + attr_internal :controller, :template attr_reader :view_paths def view_paths=(paths) @@ -298,12 +297,11 @@ module ActionView #:nodoc: # Evaluates the local assigns and controller ivars, pushes them to the view. def _evaluate_assigns_and_ivars #:nodoc: - if @controller - variables = @controller.instance_variable_names - variables -= @controller.protected_instance_variables if @controller.respond_to?(:protected_instance_variables) - variables.each { |name| instance_variable_set(name, @controller.instance_variable_get(name)) } + if controller + variables = controller.instance_variable_names + variables -= controller.protected_instance_variables if controller.respond_to?(:protected_instance_variables) + variables.each { |name| instance_variable_set(name, controller.instance_variable_get(name)) } end end - end end diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index 15b70ecff5..83357dd76f 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -634,8 +634,8 @@ module ActionView # Prefix with <tt>/dir/</tt> if lacking a leading +/+. Account for relative URL # roots. Rewrite the asset path for cache-busting asset ids. Include # asset host, if configured, with the correct request protocol. - def compute_public_path(source, dir, ext = nil, include_host = true) - has_request = @controller.respond_to?(:request) + def compute_public_path(source, dir, ext = nil, include_host = true) + has_request = controller.respond_to?(:request) source_ext = File.extname(source)[1..-1] if ext && !is_uri?(source) && (source_ext.blank? || (ext != source_ext && File.exist?(File.join(config.assets_dir, dir, "#{source}.#{ext}")))) @@ -658,7 +658,7 @@ module ActionView host = compute_asset_host(source) if has_request && !host.blank? && !is_uri?(host) - host = "#{@controller.request.protocol}#{host}" + host = "#{controller.request.protocol}#{host}" end "#{host}#{source}" @@ -681,7 +681,7 @@ module ActionView if host.is_a?(Proc) || host.respond_to?(:call) case host.is_a?(Proc) ? host.arity : host.method(:call).arity when 2 - request = @controller.respond_to?(:request) && @controller.request + request = controller.respond_to?(:request) && controller.request host.call(source, request) else host.call(source) diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb index 64d1ad2715..d5cc14b29a 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/actionpack/lib/action_view/helpers/cache_helper.rb @@ -32,7 +32,7 @@ module ActionView # <i>Topics listed alphabetically</i> # <% end %> def cache(name = {}, options = nil, &block) - @controller.fragment_for(output_buffer, name, options, &block) + controller.fragment_for(output_buffer, name, options, &block) end end end diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index 14628c5404..511386fede 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -13,7 +13,7 @@ module ActionView # Need to map default url options to controller one. def default_url_options(*args) #:nodoc: - @controller.send(:default_url_options, *args) + controller.send(:default_url_options, *args) end # Returns the URL for the set of +options+ provided. This takes the @@ -89,10 +89,10 @@ module ActionView when Hash options = { :only_path => options[:host].nil? }.update(options.symbolize_keys) escape = options.key?(:escape) ? options.delete(:escape) : false - @controller.send(:url_for, options) + controller.send(:url_for, options) when :back escape = false - @controller.request.env["HTTP_REFERER"] || 'javascript:history.back()' + controller.request.env["HTTP_REFERER"] || 'javascript:history.back()' else escape = false polymorphic_path(options) @@ -546,10 +546,10 @@ module ActionView # # => false def current_page?(options) url_string = CGI.unescapeHTML(url_for(options)) - request = @controller.request - # We ignore any extra parameters in the request_uri if the + request = controller.request + # We ignore any extra parameters in the request_uri if the # submitted url doesn't have any either. This lets the function - # work with things like ?order=asc + # work with things like ?order=asc if url_string.index("?") request_uri = request.request_uri else diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index ec278ca783..7c33f1334a 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -1,3 +1,5 @@ +require 'active_support/core_ext/object/try' + module ActionView module Rendering # Returns the result of a render that's dictated by the options hash. The primary options are: diff --git a/actionpack/test/abstract/render_test.rb b/actionpack/test/abstract/render_test.rb index be0478b638..ffd430fa86 100644 --- a/actionpack/test/abstract/render_test.rb +++ b/actionpack/test/abstract/render_test.rb @@ -6,9 +6,16 @@ module AbstractController class ControllerRenderer < AbstractController::Base include AbstractController::Rendering + def _prefix + "renderer" + end + self.view_paths = [ActionView::FixtureResolver.new( "default.erb" => "With Default", "template.erb" => "With Template", + "renderer/string.erb" => "With String", + "renderer/symbol.erb" => "With Symbol", + "string/with_path.erb" => "With String With Path", "some/file.erb" => "With File", "template_name.erb" => "With Template Name" )] @@ -33,6 +40,18 @@ module AbstractController render end + def string + render "string" + end + + def string_with_path + render "string/with_path" + end + + def symbol + render :symbol + end + def template_name render :_template_name => :template_name end @@ -73,6 +92,21 @@ module AbstractController assert_equal "With Default", @controller.response_body end + def test_render_string + @controller.process(:string) + assert_equal "With String", @controller.response_body + end + + def test_render_symbol + @controller.process(:symbol) + assert_equal "With Symbol", @controller.response_body + end + + def test_render_string_with_path + @controller.process(:string_with_path) + assert_equal "With String With Path", @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/controller/translation_test.rb b/actionpack/test/abstract/translation_test.rb index 0bf61a6556..0bf61a6556 100644 --- a/actionpack/test/controller/translation_test.rb +++ b/actionpack/test/abstract/translation_test.rb diff --git a/actionpack/test/activerecord/controller_runtime_test.rb b/actionpack/test/activerecord/controller_runtime_test.rb index d6f7cd80ab..ed8e324938 100644 --- a/actionpack/test/activerecord/controller_runtime_test.rb +++ b/actionpack/test/activerecord/controller_runtime_test.rb @@ -6,16 +6,15 @@ require 'action_controller/railties/subscriber' 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 @@ -37,17 +36,7 @@ module ControllerRuntimeSubscriberTest get :show wait - assert_equal 1, @logger.logged(:info).size - assert_match /\(Views: [\d\.]+ms | ActiveRecord: [\d\.]+ms\)/, @logger.logged(:info)[0] - end - - class SyncSubscriberTest < ActionController::TestCase - include Rails::Subscriber::SyncTestHelper - include ControllerRuntimeSubscriberTest - end - - class AsyncSubscriberTest < ActionController::TestCase - include Rails::Subscriber::AsyncTestHelper - include ControllerRuntimeSubscriberTest + assert_equal 2, @logger.logged(:info).size + assert_match /\(Views: [\d\.]+ms | ActiveRecord: [\d\.]+ms\)/, @logger.logged(:info)[1] 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 1510a6a7e0..4fcfbacf4e 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -2,6 +2,9 @@ require 'abstract_unit' 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 @@ class DefaultUrlOptionsController < ActionController::Base 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 @@ class ControllerClassTests < Test::Unit::TestCase 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 45949636c3..0000000000 --- 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/new_base/base_test.rb b/actionpack/test/controller/new_base/base_test.rb index 964780eaf2..0b40f8ce95 100644 --- a/actionpack/test/controller/new_base/base_test.rb +++ b/actionpack/test/controller/new_base/base_test.rb @@ -22,7 +22,7 @@ module Dispatching end def show_actions - render :text => "actions: #{action_methods.to_a.join(', ')}" + render :text => "actions: #{action_methods.to_a.sort.join(', ')}" end protected @@ -77,9 +77,9 @@ module Dispatching test "action methods" do assert_equal Set.new(%w( + index modify_response_headers modify_response_body_twice - index modify_response_body show_actions )), SimpleController.action_methods @@ -88,7 +88,7 @@ module Dispatching assert_equal Set.new, Submodule::ContainedEmptyController.action_methods get "/dispatching/simple/show_actions" - assert_body "actions: modify_response_headers, modify_response_body_twice, index, modify_response_body, show_actions" + assert_body "actions: index, modify_response_body, modify_response_body_twice, modify_response_headers, show_actions" end end end diff --git a/actionpack/test/controller/subscriber_test.rb b/actionpack/test/controller/subscriber_test.rb index 24132ee928..152a0d0c04 100644 --- a/actionpack/test/controller/subscriber_test.rb +++ b/actionpack/test/controller/subscriber_test.rb @@ -35,11 +35,9 @@ module Another 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 @@ -63,13 +61,19 @@ module ActionControllerSubscriberTest ActionController::Base.logger = logger end + def test_start_processing + get :show + wait + assert_equal 2, logs.size + assert_equal "Processing by Another::SubscribersController#show as HTML", logs.first + end + def test_process_action get :show wait - assert_equal 1, logs.size - assert_match /Completed/, logs.first - assert_match /\[200\]/, logs.first - assert_match /Another::SubscribersController#show/, logs.first + assert_equal 2, logs.size + assert_match /Completed/, logs.last + assert_match /with 200/, logs.last end def test_process_action_without_parameters @@ -82,23 +86,23 @@ module ActionControllerSubscriberTest get :show, :id => '10' wait - assert_equal 2, logs.size - assert_equal 'Parameters: {"id"=>"10"}', logs[0] + assert_equal 3, logs.size + assert_equal 'Parameters: {"id"=>"10"}', logs[1] end def test_process_action_with_view_runtime get :show wait - assert_match /\(Views: [\d\.]+ms\)/, logs[0] + assert_match /\(Views: [\d\.]+ms\)/, logs[1] 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 - params = logs[0] + params = logs[1] assert_match /"amount"=>"\[FILTERED\]"/, params assert_match /"lifo"=>"\[FILTERED\]"/, params assert_match /"step"=>"1"/, params @@ -108,34 +112,34 @@ module ActionControllerSubscriberTest get :redirector wait - assert_equal 2, logs.size - assert_equal "Redirected to http://foo.bar/", logs[0] + assert_equal 3, logs.size + assert_equal "Redirected to http://foo.bar/", logs[1] end def test_send_data get :data_sender wait - assert_equal 2, logs.size - assert_match /Sent data omg\.txt/, logs[0] + assert_equal 3, logs.size + assert_match /Sent data omg\.txt/, logs[1] end def test_send_file get :file_sender wait - assert_equal 2, logs.size - assert_match /Sent file/, logs[0] - assert_match /test\/fixtures\/company\.rb/, logs[0] + assert_equal 3, logs.size + assert_match /Sent file/, logs[1] + assert_match /test\/fixtures\/company\.rb/, logs[1] end def test_send_xfile get :xfile_sender wait - assert_equal 2, logs.size - assert_match /Sent X\-Sendfile header/, logs[0] - assert_match /test\/fixtures\/company\.rb/, logs[0] + assert_equal 3, logs.size + assert_match /Sent X\-Sendfile header/, logs[1] + assert_match /test\/fixtures\/company\.rb/, logs[1] end def test_with_fragment_cache @@ -143,9 +147,9 @@ module ActionControllerSubscriberTest get :with_fragment_cache wait - assert_equal 3, logs.size - assert_match /Exist fragment\? views\/foo/, logs[0] - assert_match /Write fragment views\/foo/, logs[1] + assert_equal 4, logs.size + assert_match /Exist fragment\? views\/foo/, logs[1] + assert_match /Write fragment views\/foo/, logs[2] ensure ActionController::Base.perform_caching = true end @@ -155,9 +159,9 @@ module ActionControllerSubscriberTest get :with_page_cache wait - assert_equal 2, logs.size - assert_match /Write page/, logs[0] - assert_match /\/index\.html/, logs[0] + assert_equal 3, logs.size + assert_match /Write page/, logs[1] + assert_match /\/index\.html/, logs[1] ensure ActionController::Base.perform_caching = true end @@ -165,14 +169,4 @@ module ActionControllerSubscriberTest 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/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index d3308f73cc..0faa99a912 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -35,7 +35,7 @@ class JsonParamsParsingTest < ActionController::IntegrationTest begin $stderr = StringIO.new json = "[\"person]\": {\"name\": \"David\"}}" - post "/parse", json, {'CONTENT_TYPE' => 'application/json'} + post "/parse", json, {'CONTENT_TYPE' => 'application/json', 'action_dispatch.show_exceptions' => true} assert_response :error $stderr.rewind && err = $stderr.read assert err =~ /Error occurred while parsing request parameters/ diff --git a/actionpack/test/dispatch/request/xml_params_parsing_test.rb b/actionpack/test/dispatch/request/xml_params_parsing_test.rb index 96189e4ca2..488799ac2a 100644 --- a/actionpack/test/dispatch/request/xml_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/xml_params_parsing_test.rb @@ -43,7 +43,7 @@ class XmlParamsParsingTest < ActionController::IntegrationTest begin $stderr = StringIO.new xml = "<person><name>David</name><avatar type='file' name='me.jpg' content_type='image/jpg'>#{ActiveSupport::Base64.encode64('ABC')}</avatar></pineapple>" - post "/parse", xml, default_headers + post "/parse", xml, default_headers.merge('action_dispatch.show_exceptions' => true) assert_response :error $stderr.rewind && err = $stderr.read assert err =~ /Error occurred while parsing request parameters/ diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index cb95ecea50..2b5c19361a 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -454,6 +454,56 @@ class RequestTest < ActiveSupport::TestCase 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/show_exceptions_test.rb b/actionpack/test/dispatch/show_exceptions_test.rb index def86c8323..97da680f17 100644 --- a/actionpack/test/dispatch/show_exceptions_test.rb +++ b/actionpack/test/dispatch/show_exceptions_test.rb @@ -38,15 +38,15 @@ class ShowExceptionsTest < ActionController::IntegrationTest @app = ProductionApp self.remote_addr = '208.77.188.166' - get "/" + get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 assert_equal "500 error fixture\n", body - get "/not_found" + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 assert_equal "404 error fixture\n", body - get "/method_not_allowed" + get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} assert_response 405 assert_equal "", body end @@ -56,15 +56,15 @@ class ShowExceptionsTest < ActionController::IntegrationTest ['127.0.0.1', '::1'].each do |ip_address| self.remote_addr = ip_address - get "/" + get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 assert_match /puke/, body - get "/not_found" + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 assert_match /#{ActionController::UnknownAction.name}/, body - get "/method_not_allowed" + get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} assert_response 405 assert_match /ActionController::MethodNotAllowed/, body end @@ -78,11 +78,11 @@ class ShowExceptionsTest < ActionController::IntegrationTest @app = ProductionApp self.remote_addr = '208.77.188.166' - get "/" + get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 assert_equal "500 localized error fixture\n", body - get "/not_found" + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 assert_equal "404 error fixture\n", body ensure @@ -94,15 +94,15 @@ class ShowExceptionsTest < ActionController::IntegrationTest @app = DevelopmentApp self.remote_addr = '208.77.188.166' - get "/" + get "/", {}, {'action_dispatch.show_exceptions' => true} assert_response 500 assert_match /puke/, body - get "/not_found" + get "/not_found", {}, {'action_dispatch.show_exceptions' => true} assert_response 404 assert_match /#{ActionController::UnknownAction.name}/, body - get "/method_not_allowed" + get "/method_not_allowed", {}, {'action_dispatch.show_exceptions' => true} assert_response 405 assert_match /ActionController::MethodNotAllowed/, body end diff --git a/actionpack/test/dispatch/subscriber_test.rb b/actionpack/test/dispatch/subscriber_test.rb deleted file mode 100644 index a7f1a2659a..0000000000 --- a/actionpack/test/dispatch/subscriber_test.rb +++ /dev/null @@ -1,112 +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{^Processing "/" to text/html}, log - assert_match %r{\(for 127\.0\.0\.1}, log - assert_match %r{\[GET\]}, 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 af0b3102cf..5db2b16ac1 100644 --- a/actionpack/test/template/subscriber_test.rb +++ b/actionpack/test/template/subscriber_test.rb @@ -3,7 +3,8 @@ require "rails/subscriber/test_helper" 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 @@ module ActionViewSubscriberTest 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/activemodel/lib/active_model/lint.rb b/activemodel/lib/active_model/lint.rb index eabd542485..e8a39130a6 100644 --- a/activemodel/lib/active_model/lint.rb +++ b/activemodel/lib/active_model/lint.rb @@ -50,6 +50,8 @@ module ActiveModel assert_kind_of String, model_name assert_kind_of String, model_name.human assert_kind_of String, model_name.partial_path + assert_kind_of String, model_name.singular + assert_kind_of String, model_name.plural end # == Errors Testing diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index d5b6d40514..cc0accf90e 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 @@ -53,14 +52,13 @@ module ActiveRecord autoload_under 'relation' do autoload :QueryMethods autoload :FinderMethods - autoload :CalculationMethods + autoload :Calculations autoload :PredicateBuilder autoload :SpawnMethods end autoload :Base autoload :Batches - autoload :Calculations autoload :Callbacks autoload :DynamicFinderMatch autoload :DynamicScopeMatch @@ -78,7 +76,6 @@ module ActiveRecord autoload :StateMachine autoload :Timestamp autoload :Transactions - autoload :Types autoload :Validations end @@ -96,28 +93,6 @@ module ActiveRecord 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/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index a43c4d09d6..a5b06460fe 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -187,13 +187,12 @@ module ActiveRecord conditions = "t0.#{reflection.primary_key_name} #{in_or_equals_for_ids(ids)}" conditions << append_conditions(reflection, preload_options) - associated_records = reflection.klass.with_exclusive_scope do - reflection.klass.where([conditions, ids]). + associated_records = reflection.klass.unscoped.where([conditions, ids]). includes(options[:include]). joins("INNER JOIN #{connection.quote_table_name options[:join_table]} t0 ON #{reflection.klass.quoted_table_name}.#{reflection.klass.primary_key} = t0.#{reflection.association_foreign_key}"). select("#{options[:select] || table_name+'.*'}, t0.#{reflection.primary_key_name} as the_parent_record_id"). order(options[:order]).to_a - end + set_association_collection_records(id_to_record_map, reflection.name, associated_records, 'the_parent_record_id') end @@ -341,9 +340,7 @@ module ActiveRecord conditions = "#{table_name}.#{connection.quote_column_name(primary_key)} #{in_or_equals_for_ids(ids)}" conditions << append_conditions(reflection, preload_options) - associated_records = klass.with_exclusive_scope do - klass.where([conditions, ids]).apply_finder_options(options.slice(:include, :select, :joins, :order)).to_a - end + associated_records = klass.unscoped.where([conditions, ids]).apply_finder_options(options.slice(:include, :select, :joins, :order)).to_a set_association_single_records(id_map, reflection.name, associated_records, primary_key) end @@ -362,14 +359,16 @@ module ActiveRecord conditions << append_conditions(reflection, preload_options) - reflection.klass.with_exclusive_scope do - reflection.klass.select(preload_options[:select] || options[:select] || "#{table_name}.*"). - includes(preload_options[:include] || options[:include]). - where([conditions, ids]). - joins(options[:joins]). - group(preload_options[:group] || options[:group]). - order(preload_options[:order] || options[:order]) - end + find_options = { + :select => preload_options[:select] || options[:select] || "#{table_name}.*", + :include => preload_options[:include] || options[:include], + :conditions => [conditions, ids], + :joins => options[:joins], + :group => preload_options[:group] || options[:group], + :order => preload_options[:order] || options[:order] + } + + reflection.klass.unscoped.apply_finder_options(find_options).to_a end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index ebf1a41e85..57785b4c93 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1463,13 +1463,6 @@ module ActiveRecord after_destroy(method_name) end - def find_with_associations(options, join_dependency) - rows = select_all_rows(options, join_dependency) - join_dependency.instantiate(rows) - rescue ThrowResult - [] - end - # Creates before_destroy callback methods that nullify, delete or destroy # has_many associated objects, according to the defined :dependent rule. # @@ -1693,66 +1686,6 @@ module ActiveRecord reflection end - def select_all_rows(options, join_dependency) - connection.select_all( - construct_finder_sql_with_included_associations(options, join_dependency), - "#{name} Load Including Associations" - ) - end - - def construct_finder_arel_with_included_associations(options, join_dependency) - relation = scoped - - for association in join_dependency.join_associations - relation = association.join_relation(relation) - end - - relation = relation.apply_finder_options(options).select(column_aliases(join_dependency)) - - if !using_limitable_reflections?(join_dependency.reflections) && relation.limit_value - relation = relation.where(construct_arel_limited_ids_condition(options, join_dependency)) - end - - relation = relation.except(:limit, :offset) unless using_limitable_reflections?(join_dependency.reflections) - - relation - end - - def construct_finder_sql_with_included_associations(options, join_dependency) - construct_finder_arel_with_included_associations(options, join_dependency).to_sql - end - - def construct_arel_limited_ids_condition(options, join_dependency) - if (ids_array = select_limited_ids_array(options, join_dependency)).empty? - raise ThrowResult - else - Arel::Predicates::In.new( - Arel::SqlLiteral.new("#{connection.quote_table_name table_name}.#{primary_key}"), - ids_array - ) - end - end - - def select_limited_ids_array(options, join_dependency) - connection.select_all( - construct_finder_sql_for_association_limiting(options, join_dependency), - "#{name} Load IDs For Limited Eager Loading" - ).collect { |row| row[primary_key] } - end - - def construct_finder_sql_for_association_limiting(options, join_dependency) - relation = scoped - - for association in join_dependency.join_associations - relation = association.join_relation(relation) - end - - relation = relation.apply_finder_options(options).except(:select) - relation = relation.select(connection.distinct("#{connection.quote_table_name table_name}.#{primary_key}", relation.order_values.join(", "))) - - relation.to_sql - end - def using_limitable_reflections?(reflections) reflections.collect(&:collection?).length.zero? end diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index e9402d3547..9487d16123 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -176,14 +176,15 @@ module ActiveRecord # be used for the query. If no +:counter_sql+ was supplied, but +:finder_sql+ was set, the # descendant's +construct_sql+ method will have set :counter_sql automatically. # Otherwise, construct options and pass them with scope to the target class's +count+. - def count(*args) + def count(column_name = nil, options = {}) if @reflection.options[:counter_sql] @reflection.klass.count_by_sql(@counter_sql) else - column_name, options = @reflection.klass.scoped.send(:construct_count_options_from_args, *args) + column_name, options = nil, column_name if column_name.is_a?(Hash) + if @reflection.options[:uniq] # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL. - column_name = "#{@reflection.quoted_table_name}.#{@reflection.klass.primary_key}" if column_name == :all + column_name = "#{@reflection.quoted_table_name}.#{@reflection.klass.primary_key}" unless column_name options.merge!(:distinct => true) end 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 74921241f7..a4e144f233 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 ActiveRecord 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 0154ee35f8..a949d80120 100644 --- a/activerecord/lib/active_record/attribute_methods/query.rb +++ b/activerecord/lib/active_record/attribute_methods/query.rb @@ -8,7 +8,23 @@ module ActiveRecord 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 @@ module ActiveRecord end end end - - diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 97caec7744..3da3d9d8cc 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -37,7 +37,11 @@ module ActiveRecord 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 @@ module ActiveRecord 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 @@ module ActiveRecord # Returns the value of the attribute identified by <tt>attr_name</tt> 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 4ac0c7f608..a8e3e28a7a 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 ActiveRecord 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 37eadbe0a9..e31acac050 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -17,9 +17,14 @@ module ActiveRecord # Updates the attribute identified by <tt>attr_name</tt> 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 e4d9e89821..0000000000 --- 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 db77739d1f..0000000000 --- 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 61109f4acc..0000000000 --- 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 56c32f9895..0000000000 --- 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 06244d1132..12feef4849 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -556,122 +556,9 @@ module ActiveRecord #:nodoc: end alias :colorize_logging= :colorize_logging - # Find operates with four different retrieval approaches: - # - # * Find by id - This can either be a specific id (1), a list of ids (1, 5, 6), or an array of ids ([5, 6, 10]). - # If no record can be found for all of the listed ids, then RecordNotFound will be raised. - # * Find first - This will return the first record matched by the options used. These options can either be specific - # conditions or merely an order. If no record can be matched, +nil+ is returned. Use - # <tt>Model.find(:first, *args)</tt> or its shortcut <tt>Model.first(*args)</tt>. - # * Find last - This will return the last record matched by the options used. These options can either be specific - # conditions or merely an order. If no record can be matched, +nil+ is returned. Use - # <tt>Model.find(:last, *args)</tt> or its shortcut <tt>Model.last(*args)</tt>. - # * Find all - This will return all the records matched by the options used. - # If no records are found, an empty array is returned. Use - # <tt>Model.find(:all, *args)</tt> or its shortcut <tt>Model.all(*args)</tt>. - # - # All approaches accept an options hash as their last parameter. - # - # ==== Parameters - # - # * <tt>:conditions</tt> - An SQL fragment like "administrator = 1", <tt>[ "user_name = ?", username ]</tt>, or <tt>["user_name = :user_name", { :user_name => user_name }]</tt>. See conditions in the intro. - # * <tt>:order</tt> - An SQL fragment like "created_at DESC, name". - # * <tt>:group</tt> - An attribute name by which the result should be grouped. Uses the <tt>GROUP BY</tt> SQL-clause. - # * <tt>:having</tt> - Combined with +:group+ this can be used to filter the records that a <tt>GROUP BY</tt> returns. Uses the <tt>HAVING</tt> SQL-clause. - # * <tt>:limit</tt> - An integer determining the limit on the number of rows that should be returned. - # * <tt>:offset</tt> - An integer determining the offset from where the rows should be fetched. So at 5, it would skip rows 0 through 4. - # * <tt>:joins</tt> - Either an SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id" (rarely needed), - # named associations in the same form used for the <tt>:include</tt> option, which will perform an <tt>INNER JOIN</tt> on the associated table(s), - # or an array containing a mixture of both strings and named associations. - # If the value is a string, then the records will be returned read-only since they will have attributes that do not correspond to the table's columns. - # Pass <tt>:readonly => false</tt> to override. - # * <tt>:include</tt> - Names associations that should be loaded alongside. The symbols named refer - # to already defined associations. See eager loading under Associations. - # * <tt>:select</tt> - By default, this is "*" as in "SELECT * FROM", but can be changed if you, for example, want to do a join but not - # include the joined columns. Takes a string with the SELECT SQL fragment (e.g. "id, name"). - # * <tt>:from</tt> - By default, this is the table name of the class, but can be changed to an alternate table name (or even the name - # of a database view). - # * <tt>:readonly</tt> - Mark the returned records read-only so they cannot be saved or updated. - # * <tt>:lock</tt> - An SQL fragment like "FOR UPDATE" or "LOCK IN SHARE MODE". - # <tt>:lock => true</tt> gives connection's default exclusive lock, usually "FOR UPDATE". - # - # ==== Examples - # - # # find by id - # Person.find(1) # returns the object for ID = 1 - # Person.find(1, 2, 6) # returns an array for objects with IDs in (1, 2, 6) - # Person.find([7, 17]) # returns an array for objects with IDs in (7, 17) - # Person.find([1]) # returns an array for the object with ID = 1 - # Person.find(1, :conditions => "administrator = 1", :order => "created_on DESC") - # - # Note that returned records may not be in the same order as the ids you - # provide since database rows are unordered. Give an explicit <tt>:order</tt> - # to ensure the results are sorted. - # - # ==== Examples - # - # # find first - # Person.find(:first) # returns the first object fetched by SELECT * FROM people - # Person.find(:first, :conditions => [ "user_name = ?", user_name]) - # Person.find(:first, :conditions => [ "user_name = :u", { :u => user_name }]) - # Person.find(:first, :order => "created_on DESC", :offset => 5) - # - # # find last - # Person.find(:last) # returns the last object fetched by SELECT * FROM people - # Person.find(:last, :conditions => [ "user_name = ?", user_name]) - # Person.find(:last, :order => "created_on DESC", :offset => 5) - # - # # find all - # Person.find(:all) # returns an array of objects for all the rows fetched by SELECT * FROM people - # Person.find(:all, :conditions => [ "category IN (?)", categories], :limit => 50) - # Person.find(:all, :conditions => { :friends => ["Bob", "Steve", "Fred"] } - # Person.find(:all, :offset => 10, :limit => 10) - # Person.find(:all, :include => [ :account, :friends ]) - # Person.find(:all, :group => "category") - # - # Example for find with a lock: Imagine two concurrent transactions: - # each will read <tt>person.visits == 2</tt>, add 1 to it, and save, resulting - # in two saves of <tt>person.visits = 3</tt>. By locking the row, the second - # transaction has to wait until the first is finished; we get the - # expected <tt>person.visits == 4</tt>. - # - # Person.transaction do - # person = Person.find(1, :lock => true) - # person.visits += 1 - # person.save! - # end - def find(*args) - options = args.extract_options! - - relation = construct_finder_arel(options, current_scoped_methods) - - case args.first - when :first, :last, :all - relation.send(args.first) - else - relation.find(*args) - end - end - + delegate :find, :first, :last, :all, :destroy, :destroy_all, :exists?, :delete, :delete_all, :update, :update_all, :to => :scoped delegate :select, :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :to => :scoped - - # A convenience wrapper for <tt>find(:first, *args)</tt>. You can pass in all the - # same arguments to this method as you can to <tt>find(:first)</tt>. - def first(*args) - find(:first, *args) - end - - # A convenience wrapper for <tt>find(:last, *args)</tt>. You can pass in all the - # same arguments to this method as you can to <tt>find(:last)</tt>. - def last(*args) - find(:last, *args) - end - - # A convenience wrapper for <tt>find(:all, *args)</tt>. You can pass in all the - # same arguments to this method as you can to <tt>find(:all)</tt>. - def all(*args) - find(:all, *args) - end + delegate :count, :average, :minimum, :maximum, :sum, :calculate, :to => :scoped # Executes a custom SQL query against your database and returns all the results. The results will # be returned as an array with columns requested encapsulated as attributes of the model you call @@ -699,40 +586,6 @@ module ActiveRecord #:nodoc: connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) } end - # Returns true if a record exists in the table that matches the +id+ or - # conditions given, or false otherwise. The argument can take five forms: - # - # * Integer - Finds the record with this primary key. - # * String - Finds the record with a primary key corresponding to this - # string (such as <tt>'5'</tt>). - # * Array - Finds the record that matches these +find+-style conditions - # (such as <tt>['color = ?', 'red']</tt>). - # * Hash - Finds the record that matches these +find+-style conditions - # (such as <tt>{:color => 'red'}</tt>). - # * No args - Returns false if the table is empty, true otherwise. - # - # For more information about specifying conditions as a Hash or Array, - # see the Conditions section in the introduction to ActiveRecord::Base. - # - # Note: You can't pass in a condition as a string (like <tt>name = - # 'Jamie'</tt>), since it would be sanitized and then queried against - # the primary key column, like <tt>id = 'name = \'Jamie\''</tt>. - # - # ==== Examples - # Person.exists?(5) - # Person.exists?('5') - # Person.exists?(:name => "David") - # Person.exists?(['name LIKE ?', "%#{query}%"]) - # Person.exists? - def exists?(id_or_conditions = nil) - case id_or_conditions - when Array, Hash - where(id_or_conditions).exists? - else - scoped.exists?(id_or_conditions) - end - end - # Creates an object (or multiple objects) and saves it to the database, if validations pass. # The resulting object is returned whether the object was saved successfully to the database or not. # @@ -766,177 +619,6 @@ module ActiveRecord #:nodoc: end end - # Updates an object (or multiple objects) and saves it to the database, if validations pass. - # The resulting object is returned whether the object was saved successfully to the database or not. - # - # ==== Parameters - # - # * +id+ - This should be the id or an array of ids to be updated. - # * +attributes+ - This should be a hash of attributes to be set on the object, or an array of hashes. - # - # ==== Examples - # - # # Updating one record: - # Person.update(15, :user_name => 'Samuel', :group => 'expert') - # - # # Updating multiple records: - # people = { 1 => { "first_name" => "David" }, 2 => { "first_name" => "Jeremy" } } - # Person.update(people.keys, people.values) - def update(id, attributes) - if id.is_a?(Array) - idx = -1 - id.collect { |one_id| idx += 1; update(one_id, attributes[idx]) } - else - object = find(id) - object.update_attributes(attributes) - object - end - end - - # Deletes the row with a primary key matching the +id+ argument, using a - # SQL +DELETE+ statement, and returns the number of rows deleted. Active - # Record objects are not instantiated, so the object's callbacks are not - # executed, including any <tt>:dependent</tt> association options or - # Observer methods. - # - # You can delete multiple rows at once by passing an Array of <tt>id</tt>s. - # - # Note: Although it is often much faster than the alternative, - # <tt>#destroy</tt>, skipping callbacks might bypass business logic in - # your application that ensures referential integrity or performs other - # essential jobs. - # - # ==== Examples - # - # # Delete a single row - # Todo.delete(1) - # - # # Delete multiple rows - # Todo.delete([2,3,4]) - def delete(id_or_array) - scoped.delete(id_or_array) - end - - # Destroy an object (or multiple objects) that has the given id, the object is instantiated first, - # therefore all callbacks and filters are fired off before the object is deleted. This method is - # less efficient than ActiveRecord#delete but allows cleanup methods and other actions to be run. - # - # This essentially finds the object (or multiple objects) with the given id, creates a new object - # from the attributes, and then calls destroy on it. - # - # ==== Parameters - # - # * +id+ - Can be either an Integer or an Array of Integers. - # - # ==== Examples - # - # # Destroy a single object - # Todo.destroy(1) - # - # # Destroy multiple objects - # todos = [1,2,3] - # Todo.destroy(todos) - def destroy(id) - if id.is_a?(Array) - id.map { |one_id| destroy(one_id) } - else - find(id).destroy - end - end - - # Updates all records with details given if they match a set of conditions supplied, limits and order can - # also be supplied. This method constructs a single SQL UPDATE statement and sends it straight to the - # database. It does not instantiate the involved models and it does not trigger Active Record callbacks - # or validations. - # - # ==== Parameters - # - # * +updates+ - A string, array, or hash representing the SET part of an SQL statement. - # * +conditions+ - A string, array, or hash representing the WHERE part of an SQL statement. See conditions in the intro. - # * +options+ - Additional options are <tt>:limit</tt> and <tt>:order</tt>, see the examples for usage. - # - # ==== Examples - # - # # Update all customers with the given attributes - # Customer.update_all :wants_email => true - # - # # Update all books with 'Rails' in their title - # Book.update_all "author = 'David'", "title LIKE '%Rails%'" - # - # # Update all avatars migrated more than a week ago - # Avatar.update_all ['migrated_at = ?', Time.now.utc], ['migrated_at > ?', 1.week.ago] - # - # # Update all books that match our conditions, but limit it to 5 ordered by date - # Book.update_all "author = 'David'", "title LIKE '%Rails%'", :order => 'created_at', :limit => 5 - def update_all(updates, conditions = nil, options = {}) - relation = unscoped - - relation = relation.where(conditions) if conditions - relation = relation.limit(options[:limit]) if options[:limit].present? - relation = relation.order(options[:order]) if options[:order].present? - - if current_scoped_methods && current_scoped_methods.limit_value.present? && current_scoped_methods.order_values.present? - # Only take order from scope if limit is also provided by scope, this - # is useful for updating a has_many association with a limit. - relation = current_scoped_methods.merge(relation) if current_scoped_methods - else - relation = current_scoped_methods.except(:limit, :order).merge(relation) if current_scoped_methods - end - - relation.update(sanitize_sql_for_assignment(updates)) - end - - # Destroys the records matching +conditions+ by instantiating each - # record and calling its +destroy+ method. Each object's callbacks are - # executed (including <tt>:dependent</tt> association options and - # +before_destroy+/+after_destroy+ Observer methods). Returns the - # collection of objects that were destroyed; each will be frozen, to - # reflect that no changes should be made (since they can't be - # persisted). - # - # Note: Instantiation, callback execution, and deletion of each - # record can be time consuming when you're removing many records at - # once. It generates at least one SQL +DELETE+ query per record (or - # possibly more, to enforce your callbacks). If you want to delete many - # rows quickly, without concern for their associations or callbacks, use - # +delete_all+ instead. - # - # ==== Parameters - # - # * +conditions+ - A string, array, or hash that specifies which records - # to destroy. If omitted, all records are destroyed. See the - # Conditions section in the introduction to ActiveRecord::Base for - # more information. - # - # ==== Examples - # - # Person.destroy_all("last_login < '2004-04-04'") - # Person.destroy_all(:status => "inactive") - def destroy_all(conditions = nil) - where(conditions).destroy_all - end - - # Deletes the records matching +conditions+ without instantiating the records first, and hence not - # calling the +destroy+ method nor invoking callbacks. This is a single SQL DELETE statement that - # goes straight to the database, much more efficient than +destroy_all+. Be careful with relations - # though, in particular <tt>:dependent</tt> rules defined on associations are not honored. Returns - # the number of rows affected. - # - # ==== Parameters - # - # * +conditions+ - Conditions are specified the same way as with +find+ method. - # - # ==== Example - # - # Post.delete_all("person_id = 5 AND (category = 'Something' OR category = 'Else')") - # Post.delete_all(["person_id = ? AND (category = ? OR category = ?)", 5, 'Something', 'Else']) - # - # Both calls delete the affected posts all at once with a single DELETE statement. If you need to destroy dependent - # associations or call your <tt>before_*</tt> or +after_destroy+ callbacks, use the +destroy_all+ method instead. - def delete_all(conditions = nil) - where(conditions).delete_all - end - # Returns the result of an SQL statement that should only include a COUNT(*) in the SELECT part. # The use of this method should be restricted to complicated SQL queries that can't be executed # using the ActiveRecord::Calculations class methods. Look into those before using this. @@ -1224,6 +906,10 @@ module ActiveRecord #:nodoc: reset_table_name end + def quoted_table_name + @quoted_table_name ||= connection.quote_table_name(table_name) + end + def reset_table_name #:nodoc: base = base_class @@ -1241,6 +927,7 @@ module ActiveRecord #:nodoc: name = "#{table_name_prefix}#{contained}#{undecorated_table_name(base.name)}#{table_name_suffix}" end + @quoted_table_name = nil set_table_name(name) name end @@ -1487,20 +1174,6 @@ module ActiveRecord #:nodoc: store_full_sti_class ? name : name.demodulize end - # Merges conditions so that the result is a valid +condition+ - def merge_conditions(*conditions) - segments = [] - - conditions.each do |condition| - unless condition.blank? - sql = sanitize_sql(condition) - segments << sql unless sql.blank? - end - end - - "(#{segments.join(') AND (')})" unless segments.empty? - end - def unscoped @unscoped ||= Relation.new(self, arel_table) finder_needs_type_condition? ? @unscoped.where(type_condition) : @unscoped @@ -1527,7 +1200,7 @@ module ActiveRecord #:nodoc: 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) @@ -1563,43 +1236,11 @@ module ActiveRecord #:nodoc: end def construct_finder_arel(options = {}, scope = nil) - relation = unscoped.apply_finder_options(options) + relation = options.is_a?(Hash) ? unscoped.apply_finder_options(options) : unscoped.merge(options) relation = scope.merge(relation) if scope relation end - def construct_join(joins) - case joins - when Symbol, Hash, Array - if array_of_strings?(joins) - joins.join(' ') + " " - else - build_association_joins(joins) - end - when String - " #{joins} " - else - "" - end - end - - def build_association_joins(joins) - join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, joins, nil) - relation = unscoped.table - join_dependency.join_associations.map { |association| - if (association_relation = association.relation).is_a?(Array) - [Arel::InnerJoin.new(relation, association_relation.first, *association.association_join.first).joins(relation), - Arel::InnerJoin.new(relation, association_relation.last, *association.association_join.last).joins(relation)].join() - else - Arel::InnerJoin.new(relation, association_relation, *association.association_join).joins(relation) - end - }.join(" ") - end - - def array_of_strings?(o) - o.is_a?(Array) && o.all?{|obj| obj.is_a?(String)} - end - def type_condition sti_column = arel_table[inheritance_column] condition = sti_column.eq(sti_name) @@ -1762,11 +1403,8 @@ module ActiveRecord #:nodoc: relation = construct_finder_arel(method_scoping[:find] || {}) if current_scoped_methods && current_scoped_methods.create_with_value && method_scoping[:create] - scope_for_create = case action - when :merge + scope_for_create = if action == :merge current_scoped_methods.create_with_value.merge(method_scoping[:create]) - when :reverse_merge - method_scoping[:create].merge(current_scoped_methods.create_with_value) else method_scoping[:create] end @@ -1781,15 +1419,7 @@ module ActiveRecord #:nodoc: method_scoping = relation end - if current_scoped_methods - case action - when :merge - method_scoping = current_scoped_methods.merge(method_scoping) - when :reverse_merge - method_scoping = current_scoped_methods.except(:where).merge(method_scoping) - method_scoping = method_scoping.merge(current_scoped_methods.only(:where)) - end - end + method_scoping = current_scoped_methods.merge(method_scoping) if current_scoped_methods && action == :merge self.scoped_methods << method_scoping begin @@ -1820,7 +1450,8 @@ module ActiveRecord #:nodoc: end def scoped_methods #:nodoc: - Thread.current[:"#{self}_scoped_methods"] ||= self.default_scoping.dup + key = :"#{self}_scoped_methods" + Thread.current[key] = Thread.current[key].presence || self.default_scoping.dup end def current_scoped_methods #:nodoc: @@ -2033,7 +1664,7 @@ module ActiveRecord #: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 @@ -2064,7 +1695,7 @@ module ActiveRecord #:nodoc: 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 @@ -2294,11 +1925,21 @@ module ActiveRecord #:nodoc: 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 <tt>attr_name</tt> 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). @@ -2480,7 +2121,7 @@ module ActiveRecord #:nodoc: def update(attribute_names = @attributes.keys) attributes_with_values = arel_attributes_values(false, false, attribute_names) return 0 if attributes_with_values.empty? - self.class.unscoped.where(self.class.arel_table[self.class.primary_key].eq(id)).update(attributes_with_values) + self.class.unscoped.where(self.class.arel_table[self.class.primary_key].eq(id)).arel.update(attributes_with_values) end # Creates a record with values matching those of the instance attributes @@ -2632,7 +2273,7 @@ module ActiveRecord #:nodoc: 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) @@ -2704,10 +2345,6 @@ module ActiveRecord #:nodoc: hash.inject([]) { |list, pair| list << "#{pair.first} = #{pair.last}" }.join(", ") end - def self.quoted_table_name - self.connection.quote_table_name(self.table_name) - end - def quote_columns(quoter, hash) hash.inject({}) do |quoted, (name, value)| quoted[quoter.quote_column_name(name)] = value @@ -2719,6 +2356,22 @@ module ActiveRecord #:nodoc: 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 @@ -2733,7 +2386,6 @@ module ActiveRecord #:nodoc: include AttributeMethods::PrimaryKey include AttributeMethods::TimeZoneConversion include AttributeMethods::Dirty - include Attributes, Types include Callbacks, ActiveModel::Observing, Timestamp include Associations, AssociationPreload, NamedScope include ActiveModel::Conversion @@ -2742,7 +2394,7 @@ module ActiveRecord #:nodoc: # #save_with_autosave_associations to be wrapped inside a transaction. include AutosaveAssociation, NestedAttributes - include Aggregations, Transactions, Reflection, Batches, Calculations, Serialization + include Aggregations, Transactions, Reflection, Batches, Serialization end end diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb deleted file mode 100644 index 8a44dc7df1..0000000000 --- a/activerecord/lib/active_record/calculations.rb +++ /dev/null @@ -1,173 +0,0 @@ -module ActiveRecord - module Calculations #:nodoc: - extend ActiveSupport::Concern - - CALCULATIONS_OPTIONS = [:conditions, :joins, :order, :select, :group, :having, :distinct, :limit, :offset, :include, :from] - - module ClassMethods - # Count operates using three different approaches. - # - # * Count all: By not passing any parameters to count, it will return a count of all the rows for the model. - # * Count using column: By passing a column name to count, it will return a count of all the rows for the model with supplied column present - # * Count using options will find the row count matched by the options used. - # - # The third approach, count using options, accepts an option hash as the only parameter. The options are: - # - # * <tt>:conditions</tt>: An SQL fragment like "administrator = 1" or [ "user_name = ?", username ]. See conditions in the intro to ActiveRecord::Base. - # * <tt>:joins</tt>: Either an SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id" (rarely needed) - # or named associations in the same form used for the <tt>:include</tt> option, which will perform an INNER JOIN on the associated table(s). - # If the value is a string, then the records will be returned read-only since they will have attributes that do not correspond to the table's columns. - # Pass <tt>:readonly => false</tt> to override. - # * <tt>:include</tt>: Named associations that should be loaded alongside using LEFT OUTER JOINs. The symbols named refer - # to already defined associations. When using named associations, count returns the number of DISTINCT items for the model you're counting. - # See eager loading under Associations. - # * <tt>:order</tt>: An SQL fragment like "created_at DESC, name" (really only used with GROUP BY calculations). - # * <tt>:group</tt>: An attribute name by which the result should be grouped. Uses the GROUP BY SQL-clause. - # * <tt>:select</tt>: By default, this is * as in SELECT * FROM, but can be changed if you, for example, want to do a join but not - # include the joined columns. - # * <tt>:distinct</tt>: Set this to true to make this a distinct calculation, such as SELECT COUNT(DISTINCT posts.id) ... - # * <tt>:from</tt> - By default, this is the table name of the class, but can be changed to an alternate table name (or even the name - # of a database view). - # - # Examples for counting all: - # Person.count # returns the total count of all people - # - # Examples for counting by column: - # Person.count(:age) # returns the total count of all people whose age is present in database - # - # Examples for count with options: - # Person.count(:conditions => "age > 26") - # Person.count(:conditions => "age > 26 AND job.salary > 60000", :include => :job) # because of the named association, it finds the DISTINCT count using LEFT OUTER JOIN. - # Person.count(:conditions => "age > 26 AND job.salary > 60000", :joins => "LEFT JOIN jobs on jobs.person_id = person.id") # finds the number of rows matching the conditions and joins. - # Person.count('id', :conditions => "age > 26") # Performs a COUNT(id) - # Person.count(:all, :conditions => "age > 26") # Performs a COUNT(*) (:all is an alias for '*') - # - # Note: <tt>Person.count(:all)</tt> will not work because it will use <tt>:all</tt> as the condition. Use Person.count instead. - def count(*args) - case args.size - when 0 - construct_calculation_arel.count - when 1 - if args[0].is_a?(Hash) - options = args[0] - distinct = options.has_key?(:distinct) ? options.delete(:distinct) : false - construct_calculation_arel(options).count(options[:select], :distinct => distinct) - else - construct_calculation_arel.count(args[0]) - end - when 2 - column_name, options = args - distinct = options.has_key?(:distinct) ? options.delete(:distinct) : false - construct_calculation_arel(options).count(column_name, :distinct => distinct) - else - raise ArgumentError, "Unexpected parameters passed to count(): #{args.inspect}" - end - rescue ThrowResult - 0 - end - - # Calculates the average value on a given column. The value is returned as - # a float, or +nil+ if there's no row. See +calculate+ for examples with - # options. - # - # Person.average('age') # => 35.8 - def average(column_name, options = {}) - calculate(:average, column_name, options) - end - - # Calculates the minimum value on a given column. The value is returned - # with the same data type of the column, or +nil+ if there's no row. See - # +calculate+ for examples with options. - # - # Person.minimum('age') # => 7 - def minimum(column_name, options = {}) - calculate(:minimum, column_name, options) - end - - # Calculates the maximum value on a given column. The value is returned - # with the same data type of the column, or +nil+ if there's no row. See - # +calculate+ for examples with options. - # - # Person.maximum('age') # => 93 - def maximum(column_name, options = {}) - calculate(:maximum, column_name, options) - end - - # Calculates the sum of values on a given column. The value is returned - # with the same data type of the column, 0 if there's no row. See - # +calculate+ for examples with options. - # - # Person.sum('age') # => 4562 - def sum(column_name, options = {}) - calculate(:sum, column_name, options) - end - - # This calculates aggregate values in the given column. Methods for count, sum, average, minimum, and maximum have been added as shortcuts. - # Options such as <tt>:conditions</tt>, <tt>:order</tt>, <tt>:group</tt>, <tt>:having</tt>, and <tt>:joins</tt> can be passed to customize the query. - # - # There are two basic forms of output: - # * Single aggregate value: The single value is type cast to Fixnum for COUNT, Float for AVG, and the given column's type for everything else. - # * Grouped values: This returns an ordered hash of the values and groups them by the <tt>:group</tt> option. It takes either a column name, or the name - # of a belongs_to association. - # - # values = Person.maximum(:age, :group => 'last_name') - # puts values["Drake"] - # => 43 - # - # drake = Family.find_by_last_name('Drake') - # values = Person.maximum(:age, :group => :family) # Person belongs_to :family - # puts values[drake] - # => 43 - # - # values.each do |family, max_age| - # ... - # end - # - # Options: - # * <tt>:conditions</tt> - An SQL fragment like "administrator = 1" or [ "user_name = ?", username ]. See conditions in the intro to ActiveRecord::Base. - # * <tt>:include</tt>: Eager loading, see Associations for details. Since calculations don't load anything, the purpose of this is to access fields on joined tables in your conditions, order, or group clauses. - # * <tt>:joins</tt> - An SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id". (Rarely needed). - # The records will be returned read-only since they will have attributes that do not correspond to the table's columns. - # * <tt>:order</tt> - An SQL fragment like "created_at DESC, name" (really only used with GROUP BY calculations). - # * <tt>:group</tt> - An attribute name by which the result should be grouped. Uses the GROUP BY SQL-clause. - # * <tt>:select</tt> - By default, this is * as in SELECT * FROM, but can be changed if you for example want to do a join, but not - # include the joined columns. - # * <tt>:distinct</tt> - Set this to true to make this a distinct calculation, such as SELECT COUNT(DISTINCT posts.id) ... - # - # Examples: - # Person.calculate(:count, :all) # The same as Person.count - # Person.average(:age) # SELECT AVG(age) FROM people... - # Person.minimum(:age, :conditions => ['last_name != ?', 'Drake']) # Selects the minimum age for everyone with a last name other than 'Drake' - # Person.minimum(:age, :having => 'min(age) > 17', :group => :last_name) # Selects the minimum age for any family without any minors - # Person.sum("2 * age") - def calculate(operation, column_name, options = {}) - construct_calculation_arel(options).calculate(operation, column_name, options.slice(:distinct)) - rescue ThrowResult - 0 - end - - private - def validate_calculation_options(options = {}) - options.assert_valid_keys(CALCULATIONS_OPTIONS) - end - - def construct_calculation_arel(options = {}) - validate_calculation_options(options) - options = options.except(:distinct) - - merge_with_includes = current_scoped_methods ? current_scoped_methods.includes_values : [] - includes = (merge_with_includes + Array.wrap(options[:include])).uniq - - if includes.any? - merge_with_joins = current_scoped_methods ? current_scoped_methods.joins_values : [] - joins = (merge_with_joins + Array.wrap(options[:joins])).uniq - join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, includes, construct_join(joins)) - construct_finder_arel_with_included_associations(options, join_dependency) - else - scoped.apply_finder_options(options) - end - end - - end - end -end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 9fcdabdb44..9044ca418b 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -81,10 +81,10 @@ module ActiveRecord relation = self.class.unscoped affected_rows = relation.where( - relation[self.class.primary_key].eq(quoted_id).and( - relation[self.class.locking_column].eq(quote_value(previous_value)) + relation.table[self.class.primary_key].eq(quoted_id).and( + relation.table[self.class.locking_column].eq(quote_value(previous_value)) ) - ).update(arel_attributes_values(false, false, attribute_names)) + ).arel.update(arel_attributes_values(false, false, attribute_names)) unless affected_rows == 1 diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 92030e5bfd..ff6c041ef4 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -148,18 +148,6 @@ module ActiveRecord relation end - def find(*args) - options = args.extract_options! - relation = options.present? ? apply_finder_options(options) : self - - case args.first - when :first, :last, :all - relation.send(args.first) - else - options.present? ? relation.find(*args) : super - end - end - def first(*args) if args.first.kind_of?(Integer) || (loaded? && !args.first.kind_of?(Hash)) to_a.first(*args) @@ -176,13 +164,8 @@ module ActiveRecord end end - def count(*args) - options = args.extract_options! - options.present? ? apply_finder_options(options).count(*args) : super - end - def ==(other) - to_a == other.to_a + other.respond_to?(:to_ary) ? to_a == other.to_a : false end private diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index e37e692a97..1a96cdad17 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -5,9 +5,10 @@ module ActiveRecord MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having] SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from] - include FinderMethods, CalculationMethods, SpawnMethods, QueryMethods + include FinderMethods, Calculations, SpawnMethods, QueryMethods delegate :length, :collect, :map, :each, :all?, :include?, :to => :to_a + delegate :insert, :to => :arel attr_reader :table, :klass @@ -31,7 +32,7 @@ module ActiveRecord end def respond_to?(method, include_private = false) - return true if arel.respond_to?(method, include_private) || Array.method_defined?(method) + return true if arel.respond_to?(method, include_private) || Array.method_defined?(method) || @klass.respond_to?(method, include_private) if match = DynamicFinderMatch.match(method) return true if @klass.send(:all_attributes_exists?, match.attribute_names) @@ -45,12 +46,10 @@ module ActiveRecord def to_a return @records if loaded? - eager_loading = @eager_load_values.any? || (@includes_values.any? && references_eager_loaded_tables?) - - @records = eager_loading ? find_with_associations : @klass.find_by_sql(arel.to_sql) + @records = eager_loading? ? find_with_associations : @klass.find_by_sql(arel.to_sql) preload = @preload_values - preload += @includes_values unless eager_loading + preload += @includes_values unless eager_loading? preload.each {|associations| @klass.send(:preload_associations, @records, associations) } # @readonly_value is true only if set explicity. @implicit_readonly is true if there are JOINS and no explicit SELECT. @@ -61,8 +60,6 @@ module ActiveRecord @records end - alias all to_a - def size loaded? ? @records.length : count end @@ -83,19 +80,177 @@ module ActiveRecord if block_given? to_a.many? { |*block_args| yield(*block_args) } else - arel.send(:taken).present? ? to_a.many? : size > 1 + @limit_value.present? ? to_a.many? : size > 1 end end - def destroy_all - to_a.each {|object| object.destroy} - reset + # Updates all records with details given if they match a set of conditions supplied, limits and order can + # also be supplied. This method constructs a single SQL UPDATE statement and sends it straight to the + # database. It does not instantiate the involved models and it does not trigger Active Record callbacks + # or validations. + # + # ==== Parameters + # + # * +updates+ - A string, array, or hash representing the SET part of an SQL statement. + # * +conditions+ - A string, array, or hash representing the WHERE part of an SQL statement. See conditions in the intro. + # * +options+ - Additional options are <tt>:limit</tt> and <tt>:order</tt>, see the examples for usage. + # + # ==== Examples + # + # # Update all customers with the given attributes + # Customer.update_all :wants_email => true + # + # # Update all books with 'Rails' in their title + # Book.update_all "author = 'David'", "title LIKE '%Rails%'" + # + # # Update all avatars migrated more than a week ago + # Avatar.update_all ['migrated_at = ?', Time.now.utc], ['migrated_at > ?', 1.week.ago] + # + # # Update all books that match our conditions, but limit it to 5 ordered by date + # Book.update_all "author = 'David'", "title LIKE '%Rails%'", :order => 'created_at', :limit => 5 + def update_all(updates, conditions = nil, options = {}) + if conditions || options.present? + where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates) + else + # Apply limit and order only if they're both present + if @limit_value.present? == @order_values.present? + arel.update(@klass.send(:sanitize_sql_for_assignment, updates)) + else + except(:limit, :order).update_all(updates) + end + end end - def delete_all - arel.delete.tap { reset } + # Updates an object (or multiple objects) and saves it to the database, if validations pass. + # The resulting object is returned whether the object was saved successfully to the database or not. + # + # ==== Parameters + # + # * +id+ - This should be the id or an array of ids to be updated. + # * +attributes+ - This should be a hash of attributes to be set on the object, or an array of hashes. + # + # ==== Examples + # + # # Updating one record: + # Person.update(15, :user_name => 'Samuel', :group => 'expert') + # + # # Updating multiple records: + # people = { 1 => { "first_name" => "David" }, 2 => { "first_name" => "Jeremy" } } + # Person.update(people.keys, people.values) + def update(id, attributes) + if id.is_a?(Array) + idx = -1 + id.collect { |one_id| idx += 1; update(one_id, attributes[idx]) } + else + object = find(id) + object.update_attributes(attributes) + object + end + end + + # Destroys the records matching +conditions+ by instantiating each + # record and calling its +destroy+ method. Each object's callbacks are + # executed (including <tt>:dependent</tt> association options and + # +before_destroy+/+after_destroy+ Observer methods). Returns the + # collection of objects that were destroyed; each will be frozen, to + # reflect that no changes should be made (since they can't be + # persisted). + # + # Note: Instantiation, callback execution, and deletion of each + # record can be time consuming when you're removing many records at + # once. It generates at least one SQL +DELETE+ query per record (or + # possibly more, to enforce your callbacks). If you want to delete many + # rows quickly, without concern for their associations or callbacks, use + # +delete_all+ instead. + # + # ==== Parameters + # + # * +conditions+ - A string, array, or hash that specifies which records + # to destroy. If omitted, all records are destroyed. See the + # Conditions section in the introduction to ActiveRecord::Base for + # more information. + # + # ==== Examples + # + # Person.destroy_all("last_login < '2004-04-04'") + # Person.destroy_all(:status => "inactive") + def destroy_all(conditions = nil) + if conditions + where(conditions).destroy_all + else + to_a.each {|object| object.destroy} + reset + end end + # Destroy an object (or multiple objects) that has the given id, the object is instantiated first, + # therefore all callbacks and filters are fired off before the object is deleted. This method is + # less efficient than ActiveRecord#delete but allows cleanup methods and other actions to be run. + # + # This essentially finds the object (or multiple objects) with the given id, creates a new object + # from the attributes, and then calls destroy on it. + # + # ==== Parameters + # + # * +id+ - Can be either an Integer or an Array of Integers. + # + # ==== Examples + # + # # Destroy a single object + # Todo.destroy(1) + # + # # Destroy multiple objects + # todos = [1,2,3] + # Todo.destroy(todos) + def destroy(id) + if id.is_a?(Array) + id.map { |one_id| destroy(one_id) } + else + find(id).destroy + end + end + + # Deletes the records matching +conditions+ without instantiating the records first, and hence not + # calling the +destroy+ method nor invoking callbacks. This is a single SQL DELETE statement that + # goes straight to the database, much more efficient than +destroy_all+. Be careful with relations + # though, in particular <tt>:dependent</tt> rules defined on associations are not honored. Returns + # the number of rows affected. + # + # ==== Parameters + # + # * +conditions+ - Conditions are specified the same way as with +find+ method. + # + # ==== Example + # + # Post.delete_all("person_id = 5 AND (category = 'Something' OR category = 'Else')") + # Post.delete_all(["person_id = ? AND (category = ? OR category = ?)", 5, 'Something', 'Else']) + # + # Both calls delete the affected posts all at once with a single DELETE statement. If you need to destroy dependent + # associations or call your <tt>before_*</tt> or +after_destroy+ callbacks, use the +destroy_all+ method instead. + def delete_all(conditions = nil) + conditions ? where(conditions).delete_all : arel.delete.tap { reset } + end + + # Deletes the row with a primary key matching the +id+ argument, using a + # SQL +DELETE+ statement, and returns the number of rows deleted. Active + # Record objects are not instantiated, so the object's callbacks are not + # executed, including any <tt>:dependent</tt> association options or + # Observer methods. + # + # You can delete multiple rows at once by passing an Array of <tt>id</tt>s. + # + # Note: Although it is often much faster than the alternative, + # <tt>#destroy</tt>, skipping callbacks might bypass business logic in + # your application that ensures referential integrity or performs other + # essential jobs. + # + # ==== Examples + # + # # Delete a single row + # Todo.delete(1) + # + # # Delete multiple rows + # Todo.delete([2,3,4]) def delete(id_or_array) where(@klass.primary_key => id_or_array).delete_all end @@ -112,6 +267,7 @@ module ActiveRecord def reset @first = @last = @to_sql = @order_clause = @scope_for_create = @arel = @loaded = nil + @should_eager_load = @join_dependency = nil @records = [] self end @@ -126,20 +282,29 @@ module ActiveRecord 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 end + def eager_loading? + @should_eager_load ||= (@eager_load_values.any? || (@includes_values.any? && references_eager_loaded_tables?)) + end + protected def method_missing(method, *args, &block) - if arel.respond_to?(method) - arel.send(method, *args, &block) - elsif Array.method_defined?(method) + if Array.method_defined?(method) to_a.send(method, *args, &block) + elsif @klass.respond_to?(method) + @klass.send(:with_scope, self) { @klass.send(method, *args, &block) } + elsif arel.respond_to?(method) + arel.send(method, *args, &block) elsif match = DynamicFinderMatch.match(method) attributes = match.attribute_names super unless @klass.send(:all_attributes_exists?, attributes) @@ -160,10 +325,6 @@ module ActiveRecord @klass.send(:with_scope, :create => scope_for_create, :find => {}) { yield } end - def where_clause(join_string = " AND ") - arel.send(:where_clauses).join(join_string) - end - def references_eager_loaded_tables? joined_tables = (tables_in_string(arel.joins(arel)) + [table.name, table.table_alias]).compact.uniq (tables_in_string(to_sql) - joined_tables).any? diff --git a/activerecord/lib/active_record/relation/calculation_methods.rb b/activerecord/lib/active_record/relation/calculation_methods.rb deleted file mode 100644 index 91de89e607..0000000000 --- a/activerecord/lib/active_record/relation/calculation_methods.rb +++ /dev/null @@ -1,172 +0,0 @@ -module ActiveRecord - module CalculationMethods - - def count(*args) - calculate(:count, *construct_count_options_from_args(*args)) - end - - def average(column_name) - calculate(:average, column_name) - end - - def minimum(column_name) - calculate(:minimum, column_name) - end - - def maximum(column_name) - calculate(:maximum, column_name) - end - - def sum(column_name) - calculate(:sum, column_name) - end - - def calculate(operation, column_name, options = {}) - operation = operation.to_s.downcase - - if operation == "count" - joins = arel.joins(arel) - if joins.present? && joins =~ /LEFT OUTER/i - distinct = true - column_name = @klass.primary_key if column_name == :all - end - - distinct = nil if column_name.to_s =~ /\s*DISTINCT\s+/i - distinct ||= options[:distinct] - else - distinct = nil - end - - distinct = options[:distinct] || distinct - column_name = :all if column_name.blank? && operation == "count" - - if @group_values.any? - return execute_grouped_calculation(operation, column_name) - else - return execute_simple_calculation(operation, column_name, distinct) - end - rescue ThrowResult - 0 - end - - private - - def execute_simple_calculation(operation, column_name, distinct) #:nodoc: - column = if @klass.column_names.include?(column_name.to_s) - Arel::Attribute.new(@klass.unscoped, column_name) - else - Arel::SqlLiteral.new(column_name == :all ? "*" : column_name.to_s) - end - - relation = select(operation == 'count' ? column.count(distinct) : column.send(operation)) - type_cast_calculated_value(@klass.connection.select_value(relation.to_sql), column_for(column_name), operation) - end - - def execute_grouped_calculation(operation, column_name) #:nodoc: - group_attr = @group_values.first - association = @klass.reflect_on_association(group_attr.to_sym) - associated = association && association.macro == :belongs_to # only count belongs_to associations - group_field = associated ? association.primary_key_name : group_attr - group_alias = column_alias_for(group_field) - group_column = column_for(group_field) - - group = @klass.connection.adapter_name == 'FrontBase' ? group_alias : group_field - - aggregate_alias = column_alias_for(operation, column_name) - - select_statement = if operation == 'count' && column_name == :all - "COUNT(*) AS count_all" - else - Arel::Attribute.new(@klass.unscoped, column_name).send(operation).as(aggregate_alias).to_sql - end - - select_statement << ", #{group_field} AS #{group_alias}" - - relation = select(select_statement).group(group) - - calculated_data = @klass.connection.select_all(relation.to_sql) - - if association - key_ids = calculated_data.collect { |row| row[group_alias] } - key_records = association.klass.base_class.find(key_ids) - key_records = key_records.inject({}) { |hsh, r| hsh.merge(r.id => r) } - end - - calculated_data.inject(ActiveSupport::OrderedHash.new) do |all, row| - key = type_cast_calculated_value(row[group_alias], group_column) - key = key_records[key] if associated - value = row[aggregate_alias] - all[key] = type_cast_calculated_value(value, column_for(column_name), operation) - all - end - end - - def construct_count_options_from_args(*args) - options = {} - column_name = :all - - # Handles count(), count(:column), count(:distinct => true), count(:column, :distinct => true) - case args.size - when 0 - select = get_projection_name_from_chained_relations - column_name = select if select !~ /(,|\*)/ - when 1 - if args[0].is_a?(Hash) - select = get_projection_name_from_chained_relations - column_name = select if select !~ /(,|\*)/ - options = args[0] - else - column_name = args[0] - end - when 2 - column_name, options = args - else - raise ArgumentError, "Unexpected parameters passed to count(): #{args.inspect}" - end - - [column_name || :all, options] - end - - # Converts the given keys to the value that the database adapter returns as - # a usable column name: - # - # column_alias_for("users.id") # => "users_id" - # column_alias_for("sum(id)") # => "sum_id" - # column_alias_for("count(distinct users.id)") # => "count_distinct_users_id" - # column_alias_for("count(*)") # => "count_all" - # column_alias_for("count", "id") # => "count_id" - def column_alias_for(*keys) - table_name = keys.join(' ') - table_name.downcase! - table_name.gsub!(/\*/, 'all') - table_name.gsub!(/\W+/, ' ') - table_name.strip! - table_name.gsub!(/ +/, '_') - - @klass.connection.table_alias_for(table_name) - end - - def column_for(field) - field_name = field.to_s.split('.').last - @klass.columns.detect { |c| c.name.to_s == field_name } - end - - def type_cast_calculated_value(value, column, operation = nil) - case operation - when 'count' then value.to_i - when 'sum' then type_cast_using_column(value || '0', column) - when 'average' then value && (value.is_a?(Fixnum) ? value.to_f : value).to_d - else type_cast_using_column(value, column) - end - end - - def type_cast_using_column(value, column) - column ? column.type_cast(value) : value - end - - def get_projection_name_from_chained_relations - @select_values.join(", ") if @select_values.present? - end - - end -end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb new file mode 100644 index 0000000000..e77424a64b --- /dev/null +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -0,0 +1,259 @@ +module ActiveRecord + module Calculations + # Count operates using three different approaches. + # + # * Count all: By not passing any parameters to count, it will return a count of all the rows for the model. + # * Count using column: By passing a column name to count, it will return a count of all the rows for the model with supplied column present + # * Count using options will find the row count matched by the options used. + # + # The third approach, count using options, accepts an option hash as the only parameter. The options are: + # + # * <tt>:conditions</tt>: An SQL fragment like "administrator = 1" or [ "user_name = ?", username ]. See conditions in the intro to ActiveRecord::Base. + # * <tt>:joins</tt>: Either an SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id" (rarely needed) + # or named associations in the same form used for the <tt>:include</tt> option, which will perform an INNER JOIN on the associated table(s). + # If the value is a string, then the records will be returned read-only since they will have attributes that do not correspond to the table's columns. + # Pass <tt>:readonly => false</tt> to override. + # * <tt>:include</tt>: Named associations that should be loaded alongside using LEFT OUTER JOINs. The symbols named refer + # to already defined associations. When using named associations, count returns the number of DISTINCT items for the model you're counting. + # See eager loading under Associations. + # * <tt>:order</tt>: An SQL fragment like "created_at DESC, name" (really only used with GROUP BY calculations). + # * <tt>:group</tt>: An attribute name by which the result should be grouped. Uses the GROUP BY SQL-clause. + # * <tt>:select</tt>: By default, this is * as in SELECT * FROM, but can be changed if you, for example, want to do a join but not + # include the joined columns. + # * <tt>:distinct</tt>: Set this to true to make this a distinct calculation, such as SELECT COUNT(DISTINCT posts.id) ... + # * <tt>:from</tt> - By default, this is the table name of the class, but can be changed to an alternate table name (or even the name + # of a database view). + # + # Examples for counting all: + # Person.count # returns the total count of all people + # + # Examples for counting by column: + # Person.count(:age) # returns the total count of all people whose age is present in database + # + # Examples for count with options: + # Person.count(:conditions => "age > 26") + # Person.count(:conditions => "age > 26 AND job.salary > 60000", :include => :job) # because of the named association, it finds the DISTINCT count using LEFT OUTER JOIN. + # Person.count(:conditions => "age > 26 AND job.salary > 60000", :joins => "LEFT JOIN jobs on jobs.person_id = person.id") # finds the number of rows matching the conditions and joins. + # Person.count('id', :conditions => "age > 26") # Performs a COUNT(id) + # Person.count(:all, :conditions => "age > 26") # Performs a COUNT(*) (:all is an alias for '*') + # + # Note: <tt>Person.count(:all)</tt> will not work because it will use <tt>:all</tt> as the condition. Use Person.count instead. + def count(column_name = nil, options = {}) + column_name, options = nil, column_name if column_name.is_a?(Hash) + calculate(:count, column_name, options) + end + + # Calculates the average value on a given column. The value is returned as + # a float, or +nil+ if there's no row. See +calculate+ for examples with + # options. + # + # Person.average('age') # => 35.8 + def average(column_name, options = {}) + calculate(:average, column_name, options) + end + + # Calculates the minimum value on a given column. The value is returned + # with the same data type of the column, or +nil+ if there's no row. See + # +calculate+ for examples with options. + # + # Person.minimum('age') # => 7 + def minimum(column_name, options = {}) + calculate(:minimum, column_name, options) + end + + # Calculates the maximum value on a given column. The value is returned + # with the same data type of the column, or +nil+ if there's no row. See + # +calculate+ for examples with options. + # + # Person.maximum('age') # => 93 + def maximum(column_name, options = {}) + calculate(:maximum, column_name, options) + end + + # Calculates the sum of values on a given column. The value is returned + # with the same data type of the column, 0 if there's no row. See + # +calculate+ for examples with options. + # + # Person.sum('age') # => 4562 + def sum(column_name, options = {}) + calculate(:sum, column_name, options) + end + + # This calculates aggregate values in the given column. Methods for count, sum, average, minimum, and maximum have been added as shortcuts. + # Options such as <tt>:conditions</tt>, <tt>:order</tt>, <tt>:group</tt>, <tt>:having</tt>, and <tt>:joins</tt> can be passed to customize the query. + # + # There are two basic forms of output: + # * Single aggregate value: The single value is type cast to Fixnum for COUNT, Float for AVG, and the given column's type for everything else. + # * Grouped values: This returns an ordered hash of the values and groups them by the <tt>:group</tt> option. It takes either a column name, or the name + # of a belongs_to association. + # + # values = Person.maximum(:age, :group => 'last_name') + # puts values["Drake"] + # => 43 + # + # drake = Family.find_by_last_name('Drake') + # values = Person.maximum(:age, :group => :family) # Person belongs_to :family + # puts values[drake] + # => 43 + # + # values.each do |family, max_age| + # ... + # end + # + # Options: + # * <tt>:conditions</tt> - An SQL fragment like "administrator = 1" or [ "user_name = ?", username ]. See conditions in the intro to ActiveRecord::Base. + # * <tt>:include</tt>: Eager loading, see Associations for details. Since calculations don't load anything, the purpose of this is to access fields on joined tables in your conditions, order, or group clauses. + # * <tt>:joins</tt> - An SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id". (Rarely needed). + # The records will be returned read-only since they will have attributes that do not correspond to the table's columns. + # * <tt>:order</tt> - An SQL fragment like "created_at DESC, name" (really only used with GROUP BY calculations). + # * <tt>:group</tt> - An attribute name by which the result should be grouped. Uses the GROUP BY SQL-clause. + # * <tt>:select</tt> - By default, this is * as in SELECT * FROM, but can be changed if you for example want to do a join, but not + # include the joined columns. + # * <tt>:distinct</tt> - Set this to true to make this a distinct calculation, such as SELECT COUNT(DISTINCT posts.id) ... + # + # Examples: + # Person.calculate(:count, :all) # The same as Person.count + # Person.average(:age) # SELECT AVG(age) FROM people... + # Person.minimum(:age, :conditions => ['last_name != ?', 'Drake']) # Selects the minimum age for everyone with a last name other than 'Drake' + # Person.minimum(:age, :having => 'min(age) > 17', :group => :last_name) # Selects the minimum age for any family without any minors + # Person.sum("2 * age") + def calculate(operation, column_name, options = {}) + if options.except(:distinct).present? + apply_finder_options(options.except(:distinct)).calculate(operation, column_name, :distinct => options[:distinct]) + else + if eager_loading? || includes_values.present? + construct_relation_for_association_calculations.calculate(operation, column_name, options) + else + perform_calculation(operation, column_name, options) + end + end + rescue ThrowResult + 0 + end + + private + + def perform_calculation(operation, column_name, options = {}) + operation = operation.to_s.downcase + + if operation == "count" + column_name ||= (select_for_count || :all) + + joins = arel.joins(arel) + if joins.present? && joins =~ /LEFT OUTER/i + distinct = true + column_name = @klass.primary_key if column_name == :all + end + + distinct = nil if column_name.to_s =~ /\s*DISTINCT\s+/i + distinct ||= options[:distinct] + else + distinct = nil + end + + distinct = options[:distinct] || distinct + column_name = :all if column_name.blank? && operation == "count" + + if @group_values.any? + return execute_grouped_calculation(operation, column_name) + else + return execute_simple_calculation(operation, column_name, distinct) + end + end + + def execute_simple_calculation(operation, column_name, distinct) #:nodoc: + column = if @klass.column_names.include?(column_name.to_s) + Arel::Attribute.new(@klass.unscoped, column_name) + else + Arel::SqlLiteral.new(column_name == :all ? "*" : column_name.to_s) + end + + # Postgresql doesn't like ORDER BY when there are no GROUP BY + relation = except(:order).select(operation == 'count' ? column.count(distinct) : column.send(operation)) + type_cast_calculated_value(@klass.connection.select_value(relation.to_sql), column_for(column_name), operation) + end + + def execute_grouped_calculation(operation, column_name) #:nodoc: + group_attr = @group_values.first + association = @klass.reflect_on_association(group_attr.to_sym) + associated = association && association.macro == :belongs_to # only count belongs_to associations + group_field = associated ? association.primary_key_name : group_attr + group_alias = column_alias_for(group_field) + group_column = column_for(group_field) + + group = @klass.connection.adapter_name == 'FrontBase' ? group_alias : group_field + + aggregate_alias = column_alias_for(operation, column_name) + + select_statement = if operation == 'count' && column_name == :all + "COUNT(*) AS count_all" + else + Arel::Attribute.new(@klass.unscoped, column_name).send(operation).as(aggregate_alias).to_sql + end + + select_statement << ", #{group_field} AS #{group_alias}" + + relation = select(select_statement).group(group) + + calculated_data = @klass.connection.select_all(relation.to_sql) + + if association + key_ids = calculated_data.collect { |row| row[group_alias] } + key_records = association.klass.base_class.find(key_ids) + key_records = key_records.inject({}) { |hsh, r| hsh.merge(r.id => r) } + end + + calculated_data.inject(ActiveSupport::OrderedHash.new) do |all, row| + key = type_cast_calculated_value(row[group_alias], group_column) + key = key_records[key] if associated + value = row[aggregate_alias] + all[key] = type_cast_calculated_value(value, column_for(column_name), operation) + all + end + end + + # Converts the given keys to the value that the database adapter returns as + # a usable column name: + # + # column_alias_for("users.id") # => "users_id" + # column_alias_for("sum(id)") # => "sum_id" + # column_alias_for("count(distinct users.id)") # => "count_distinct_users_id" + # column_alias_for("count(*)") # => "count_all" + # column_alias_for("count", "id") # => "count_id" + def column_alias_for(*keys) + table_name = keys.join(' ') + table_name.downcase! + table_name.gsub!(/\*/, 'all') + table_name.gsub!(/\W+/, ' ') + table_name.strip! + table_name.gsub!(/ +/, '_') + + @klass.connection.table_alias_for(table_name) + end + + def column_for(field) + field_name = field.to_s.split('.').last + @klass.columns.detect { |c| c.name.to_s == field_name } + end + + def type_cast_calculated_value(value, column, operation = nil) + case operation + when 'count' then value.to_i + when 'sum' then type_cast_using_column(value || '0', column) + when 'average' then value && (value.is_a?(Fixnum) ? value.to_f : value).to_d + else type_cast_using_column(value, column) + end + end + + def type_cast_using_column(value, column) + column ? column.type_cast(value) : value + end + + def select_for_count + if @select_values.present? + select = @select_values.join(", ") + select if select !~ /(,|\*)/ + end + end + end +end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 980c5796f3..d6d3d66642 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -1,44 +1,157 @@ module ActiveRecord module FinderMethods - - def find(*ids, &block) + # Find operates with four different retrieval approaches: + # + # * Find by id - This can either be a specific id (1), a list of ids (1, 5, 6), or an array of ids ([5, 6, 10]). + # If no record can be found for all of the listed ids, then RecordNotFound will be raised. + # * Find first - This will return the first record matched by the options used. These options can either be specific + # conditions or merely an order. If no record can be matched, +nil+ is returned. Use + # <tt>Model.find(:first, *args)</tt> or its shortcut <tt>Model.first(*args)</tt>. + # * Find last - This will return the last record matched by the options used. These options can either be specific + # conditions or merely an order. If no record can be matched, +nil+ is returned. Use + # <tt>Model.find(:last, *args)</tt> or its shortcut <tt>Model.last(*args)</tt>. + # * Find all - This will return all the records matched by the options used. + # If no records are found, an empty array is returned. Use + # <tt>Model.find(:all, *args)</tt> or its shortcut <tt>Model.all(*args)</tt>. + # + # All approaches accept an options hash as their last parameter. + # + # ==== Parameters + # + # * <tt>:conditions</tt> - An SQL fragment like "administrator = 1", <tt>[ "user_name = ?", username ]</tt>, or <tt>["user_name = :user_name", { :user_name => user_name }]</tt>. See conditions in the intro. + # * <tt>:order</tt> - An SQL fragment like "created_at DESC, name". + # * <tt>:group</tt> - An attribute name by which the result should be grouped. Uses the <tt>GROUP BY</tt> SQL-clause. + # * <tt>:having</tt> - Combined with +:group+ this can be used to filter the records that a <tt>GROUP BY</tt> returns. Uses the <tt>HAVING</tt> SQL-clause. + # * <tt>:limit</tt> - An integer determining the limit on the number of rows that should be returned. + # * <tt>:offset</tt> - An integer determining the offset from where the rows should be fetched. So at 5, it would skip rows 0 through 4. + # * <tt>:joins</tt> - Either an SQL fragment for additional joins like "LEFT JOIN comments ON comments.post_id = id" (rarely needed), + # named associations in the same form used for the <tt>:include</tt> option, which will perform an <tt>INNER JOIN</tt> on the associated table(s), + # or an array containing a mixture of both strings and named associations. + # If the value is a string, then the records will be returned read-only since they will have attributes that do not correspond to the table's columns. + # Pass <tt>:readonly => false</tt> to override. + # * <tt>:include</tt> - Names associations that should be loaded alongside. The symbols named refer + # to already defined associations. See eager loading under Associations. + # * <tt>:select</tt> - By default, this is "*" as in "SELECT * FROM", but can be changed if you, for example, want to do a join but not + # include the joined columns. Takes a string with the SELECT SQL fragment (e.g. "id, name"). + # * <tt>:from</tt> - By default, this is the table name of the class, but can be changed to an alternate table name (or even the name + # of a database view). + # * <tt>:readonly</tt> - Mark the returned records read-only so they cannot be saved or updated. + # * <tt>:lock</tt> - An SQL fragment like "FOR UPDATE" or "LOCK IN SHARE MODE". + # <tt>:lock => true</tt> gives connection's default exclusive lock, usually "FOR UPDATE". + # + # ==== Examples + # + # # find by id + # Person.find(1) # returns the object for ID = 1 + # Person.find(1, 2, 6) # returns an array for objects with IDs in (1, 2, 6) + # Person.find([7, 17]) # returns an array for objects with IDs in (7, 17) + # Person.find([1]) # returns an array for the object with ID = 1 + # Person.find(1, :conditions => "administrator = 1", :order => "created_on DESC") + # + # Note that returned records may not be in the same order as the ids you + # provide since database rows are unordered. Give an explicit <tt>:order</tt> + # to ensure the results are sorted. + # + # ==== Examples + # + # # find first + # Person.find(:first) # returns the first object fetched by SELECT * FROM people + # Person.find(:first, :conditions => [ "user_name = ?", user_name]) + # Person.find(:first, :conditions => [ "user_name = :u", { :u => user_name }]) + # Person.find(:first, :order => "created_on DESC", :offset => 5) + # + # # find last + # Person.find(:last) # returns the last object fetched by SELECT * FROM people + # Person.find(:last, :conditions => [ "user_name = ?", user_name]) + # Person.find(:last, :order => "created_on DESC", :offset => 5) + # + # # find all + # Person.find(:all) # returns an array of objects for all the rows fetched by SELECT * FROM people + # Person.find(:all, :conditions => [ "category IN (?)", categories], :limit => 50) + # Person.find(:all, :conditions => { :friends => ["Bob", "Steve", "Fred"] } + # Person.find(:all, :offset => 10, :limit => 10) + # Person.find(:all, :include => [ :account, :friends ]) + # Person.find(:all, :group => "category") + # + # Example for find with a lock: Imagine two concurrent transactions: + # each will read <tt>person.visits == 2</tt>, add 1 to it, and save, resulting + # in two saves of <tt>person.visits = 3</tt>. By locking the row, the second + # transaction has to wait until the first is finished; we get the + # expected <tt>person.visits == 4</tt>. + # + # Person.transaction do + # person = Person.find(1, :lock => true) + # person.visits += 1 + # person.save! + # end + def find(*args, &block) return to_a.find(&block) if block_given? - expects_array = ids.first.kind_of?(Array) - return ids.first if expects_array && ids.first.empty? - - ids = ids.flatten.compact.uniq + options = args.extract_options! - case ids.size - when 0 - raise RecordNotFound, "Couldn't find #{@klass.name} without an ID" - when 1 - result = find_one(ids.first) - expects_array ? [ result ] : result + if options.present? + apply_finder_options(options).find(*args) else - find_some(ids) + case args.first + when :first, :last, :all + send(args.first) + else + find_with_ids(*args) + end end end - def exists?(id = nil) - relation = select(primary_key).limit(1) - relation = relation.where(primary_key.eq(id)) if id - relation.first ? true : false + # A convenience wrapper for <tt>find(:first, *args)</tt>. You can pass in all the + # same arguments to this method as you can to <tt>find(:first)</tt>. + def first(*args) + args.any? ? apply_finder_options(args.first).first : find_first end - def first - if loaded? - @records.first - else - @first ||= limit(1).to_a[0] - end + # A convenience wrapper for <tt>find(:last, *args)</tt>. You can pass in all the + # same arguments to this method as you can to <tt>find(:last)</tt>. + def last(*args) + args.any? ? apply_finder_options(args.first).last : find_last end - def last - if loaded? - @records.last + # A convenience wrapper for <tt>find(:all, *args)</tt>. You can pass in all the + # same arguments to this method as you can to <tt>find(:all)</tt>. + def all(*args) + args.any? ? apply_finder_options(args.first).to_a : to_a + end + + # Returns true if a record exists in the table that matches the +id+ or + # conditions given, or false otherwise. The argument can take five forms: + # + # * Integer - Finds the record with this primary key. + # * String - Finds the record with a primary key corresponding to this + # string (such as <tt>'5'</tt>). + # * Array - Finds the record that matches these +find+-style conditions + # (such as <tt>['color = ?', 'red']</tt>). + # * Hash - Finds the record that matches these +find+-style conditions + # (such as <tt>{:color => 'red'}</tt>). + # * No args - Returns false if the table is empty, true otherwise. + # + # For more information about specifying conditions as a Hash or Array, + # see the Conditions section in the introduction to ActiveRecord::Base. + # + # Note: You can't pass in a condition as a string (like <tt>name = + # 'Jamie'</tt>), since it would be sanitized and then queried against + # the primary key column, like <tt>id = 'name = \'Jamie\''</tt>. + # + # ==== Examples + # Person.exists?(5) + # Person.exists?('5') + # Person.exists?(:name => "David") + # Person.exists?(['name LIKE ?', "%#{query}%"]) + # Person.exists? + def exists?(id = nil) + case id + when Array, Hash + where(id).exists? else - @last ||= reverse_order.limit(1).to_a[0] + relation = select(primary_key).limit(1) + relation = relation.where(primary_key.eq(id)) if id + relation.first ? true : false end end @@ -53,9 +166,20 @@ module ActiveRecord [] end + def construct_relation_for_association_calculations + including = (@eager_load_values + @includes_values).uniq + join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(@klass, including, arel.joins(arel)) + + relation = except(:includes, :eager_load, :preload) + apply_join_dependency(relation, join_dependency) + end + def construct_relation_for_association_find(join_dependency) relation = except(:includes, :eager_load, :preload, :select).select(@klass.send(:column_aliases, join_dependency)) + apply_join_dependency(relation, join_dependency) + end + def apply_join_dependency(relation, join_dependency) for association in join_dependency.join_associations relation = association.join_relation(relation) end @@ -113,11 +237,30 @@ module ActiveRecord record end + def find_with_ids(*ids, &block) + return to_a.find(&block) if block_given? + + expects_array = ids.first.kind_of?(Array) + return ids.first if expects_array && ids.first.empty? + + ids = ids.flatten.compact.uniq + + case ids.size + when 0 + raise RecordNotFound, "Couldn't find #{@klass.name} without an ID" + when 1 + result = find_one(ids.first) + expects_array ? [ result ] : result + else + find_some(ids) + end + end + def find_one(id) record = where(primary_key.eq(id)).first unless record - conditions = where_clause(', ') + conditions = arel.send(:where_clauses).join(', ') conditions = " [WHERE #{conditions}]" if conditions.present? raise RecordNotFound, "Couldn't find #{@klass.name} with ID=#{id}#{conditions}" end @@ -129,21 +272,21 @@ module ActiveRecord result = where(primary_key.in(ids)).all expected_size = - if arel.taken && ids.size > arel.taken - arel.taken + if @limit_value && ids.size > @limit_value + @limit_value else ids.size end # 11 ids with limit 3, offset 9 should give 2 results. - if arel.skipped && (ids.size - arel.skipped < expected_size) - expected_size = ids.size - arel.skipped + if @offset_value && (ids.size - @offset_value < expected_size) + expected_size = ids.size - @offset_value end if result.size == expected_size result else - conditions = where_clause(', ') + conditions = arel.send(:where_clauses).join(', ') conditions = " [WHERE #{conditions}]" if conditions.present? error = "Couldn't find all #{@klass.name.pluralize} with IDs " @@ -152,5 +295,21 @@ module ActiveRecord end end + def find_first + if loaded? + @records.first + else + @first ||= limit(1).to_a[0] + end + end + + def find_last + if loaded? + @records.last + else + @last ||= reverse_order.limit(1).to_a[0] + end + end + end end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index a3ac58bc81..8954f2d12b 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -8,11 +8,10 @@ module ActiveRecord 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 @@ module ActiveRecord [: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 @@ module ActiveRecord 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 @@ -77,7 +75,7 @@ module ActiveRecord # Build association joins first joins.each do |join| - association_joins << join if [Hash, Array, Symbol].include?(join.class) && !@klass.send(:array_of_strings?, join) + association_joins << join if [Hash, Array, Symbol].include?(join.class) && !array_of_strings?(join) end if association_joins.any? @@ -110,7 +108,7 @@ module ActiveRecord when Relation::JoinOperation arel = arel.join(join.relation, join.join_class).on(*join.on) when Hash, Array, Symbol - if @klass.send(:array_of_strings?, join) + if array_of_strings?(join) join_string = join.join(' ') arel = arel.join(join_string) end @@ -119,8 +117,16 @@ module ActiveRecord 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,21 +141,23 @@ module ActiveRecord 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 + quoted_table_name = @klass.quoted_table_name + if selects.present? selects.each do |s| @implicit_readonly = false arel = arel.project(s) if s.present? end - elsif joins.present? - arel = arel.project(@klass.quoted_table_name + '.*') + else + arel = arel.project(quoted_table_name + '.*') end - arel = arel.from(@from_value) if @from_value.present? + arel = @from_value.present? ? arel.from(@from_value) : arel.from(quoted_table_name) case @lock_value when TrueClass @@ -167,8 +175,7 @@ module ActiveRecord builder = PredicateBuilder.new(table.engine) conditions = if [String, Array].include?(args.first.class) - merged = @klass.send(:merge_conditions, args.size > 1 ? Array.wrap(args) : args.first) - Arel::SqlLiteral.new(merged) if merged + @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) @@ -193,5 +200,9 @@ module ActiveRecord }.join(',') end + def array_of_strings?(o) + o.is_a?(Array) && o.all?{|obj| obj.is_a?(String)} + end + end end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index d5b13c6100..cccf413e67 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -1,17 +1,7 @@ module ActiveRecord module SpawnMethods - def spawn(arel_table = self.table) - relation = self.class.new(@klass, arel_table) - - (Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS).each do |query_method| - relation.send(:"#{query_method}_values=", send(:"#{query_method}_values")) - end - - Relation::SINGLE_VALUE_METHODS.each do |query_method| - relation.send(:"#{query_method}_value=", send(:"#{query_method}_value")) - end - - relation + def spawn + clone.reset end def merge(r) @@ -98,19 +88,12 @@ module ActiveRecord options.assert_valid_keys(VALID_FIND_OPTIONS) - relation = relation.joins(options[:joins]). - where(options[:conditions]). - select(options[:select]). - group(options[:group]). - having(options[:having]). - order(options[:order]). - limit(options[:limit]). - offset(options[:offset]). - from(options[:from]). - includes(options[:include]) - - relation = relation.lock(options[:lock]) if options[:lock].present? - relation = relation.readonly(options[:readonly]) if options.has_key?(:readonly) + [:joins, :select, :group, :having, :order, :limit, :offset, :from, :lock, :readonly].each do |finder| + relation = relation.send(finder, options[finder]) if options.has_key?(finder) + end + + relation = relation.where(options[:conditions]) if options.has_key?(:conditions) + relation = relation.includes(options[:include]) if options.has_key?(:include) relation end diff --git a/activerecord/lib/active_record/types.rb b/activerecord/lib/active_record/types.rb deleted file mode 100644 index 74f569352b..0000000000 --- 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 cfbe877575..0000000000 --- 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 ec3f861abd..0000000000 --- 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 7b6af1981f..0000000000 --- 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 3a8b9292f9..0000000000 --- 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 f832c7b304..0000000000 --- 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/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 1bce45865f..004d0156e1 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -732,21 +732,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal [projects(:active_record), projects(:action_controller)].map(&:id).sort, developer.project_ids.sort end - def test_select_limited_ids_array - # Set timestamps - Developer.transaction do - Developer.find(:all, :order => 'id').each_with_index do |record, i| - record.update_attributes(:created_at => 5.years.ago + (i * 5.minutes)) - end - end - - join_base = ActiveRecord::Associations::ClassMethods::JoinDependency::JoinBase.new(Project) - join_dep = ActiveRecord::Associations::ClassMethods::JoinDependency.new(join_base, :developers, nil) - projects = Project.send(:select_limited_ids_array, {:order => 'developers.created_at'}, join_dep) - assert !projects.include?("'"), projects - assert_equal ["1", "2"], projects.sort - end - def test_scoped_find_on_through_association_doesnt_return_read_only_records tag = Post.find(1).tags.find_by_name("General") diff --git a/activerecord/test/cases/attributes/aliasing_test.rb b/activerecord/test/cases/attributes/aliasing_test.rb deleted file mode 100644 index 7ee25779f1..0000000000 --- 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 8a3b551375..0000000000 --- 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/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index bd2d471fc7..c3b2e56387 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -246,23 +246,6 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal 8, c['Jadedpixel'] end - def test_should_reject_invalid_options - assert_nothing_raised do - # empty options are valid - Company.send(:validate_calculation_options) - # these options are valid for all calculations - [:select, :conditions, :joins, :order, :group, :having, :distinct].each do |opt| - Company.send(:validate_calculation_options, opt => true) - end - - # :include is only valid on :count - Company.send(:validate_calculation_options, :include => true) - end - - assert_raise(ArgumentError) { Company.send(:validate_calculation_options, :sum, :foo => :bar) } - assert_raise(ArgumentError) { Company.send(:validate_calculation_options, :count, :foo => :bar) } - end - def test_should_count_selected_field_with_include assert_equal 6, Account.count(:distinct => true, :include => :firm) assert_equal 4, Account.count(:distinct => true, :include => :firm, :select => :credit_limit) diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index 7ca5b5a988..1081aa40a9 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -11,7 +11,7 @@ class MethodScopingTest < ActiveRecord::TestCase def test_set_conditions Developer.send(:with_scope, :find => { :conditions => 'just a test...' }) do - assert_equal '(just a test...)', Developer.scoped.send(:where_clause) + assert_equal '(just a test...)', Developer.scoped.arel.send(:where_clauses).join(' AND ') end end @@ -257,7 +257,7 @@ class NestedScopingTest < ActiveRecord::TestCase Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do Developer.send(:with_scope, :find => { :limit => 10 }) do devs = Developer.scoped - assert_equal '(salary = 80000)', devs.send(:where_clause) + assert_equal '(salary = 80000)', devs.arel.send(:where_clauses).join(' AND ') assert_equal 10, devs.taken end end @@ -285,7 +285,7 @@ class NestedScopingTest < ActiveRecord::TestCase Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do devs = Developer.scoped - assert_equal "(name = 'David') AND (salary = 80000)", devs.send(:where_clause) + assert_equal "(name = 'David') AND (salary = 80000)", devs.arel.send(:where_clauses).join(' AND ') assert_equal(1, Developer.count) end Developer.send(:with_scope, :find => { :conditions => "name = 'Maiha'" }) do @@ -298,7 +298,7 @@ class NestedScopingTest < ActiveRecord::TestCase Developer.send(:with_scope, :find => { :conditions => 'salary = 80000', :limit => 10 }) do Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do devs = Developer.scoped - assert_equal "(salary = 80000) AND (name = 'David')", devs.send(:where_clause) + assert_equal "(salary = 80000) AND (name = 'David')", devs.arel.send(:where_clauses).join(' AND ') assert_equal 10, devs.taken end end @@ -588,7 +588,7 @@ class HasAndBelongsToManyScopingTest< ActiveRecord::TestCase end class DefaultScopingTest < ActiveRecord::TestCase - fixtures :developers + fixtures :developers, :posts def test_default_scope expected = Developer.find(:all, :order => 'salary DESC').collect { |dev| dev.salary } @@ -657,6 +657,12 @@ class DefaultScopingTest < ActiveRecord::TestCase received = DeveloperOrderedBySalary.find(:all, :order => 'salary').collect { |dev| dev.salary } assert_equal expected, received end + + def test_default_scope_using_relation + posts = PostWithComment.scoped + assert_equal 2, posts.count + assert_equal posts(:thinking), posts.first + end end =begin diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index 3e2bd58f9a..894d96346e 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -379,6 +379,21 @@ class NamedScopeTest < ActiveRecord::TestCase def test_deprecated_named_scope_method assert_deprecated('named_scope has been deprecated') { Topic.named_scope :deprecated_named_scope } end + + def test_named_scopes_on_relations + # Topic.replied + approved_topics = Topic.scoped.approved.order('id DESC') + assert_equal topics(:fourth), approved_topics.first + + replied_approved_topics = approved_topics.replied + assert_equal topics(:third), replied_approved_topics.first + end + + def test_index_on_named_scope + approved = Topic.approved.order('id ASC') + assert_equal topics(:second), approved[0] + assert approved.loaded? + end end class DynamicScopeMatchTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index d34c9b4895..1e345399f5 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -164,6 +164,11 @@ class RelationTest < ActiveRecord::TestCase end end + def test_respond_to_class_methods_and_named_scopes + assert DeveloperOrderedBySalary.scoped.respond_to?(:all_ordered_by_name) + assert Topic.scoped.respond_to?(:by_lifo) + end + def test_find_with_readonly_option Developer.scoped.each { |d| assert !d.readonly? } Developer.scoped.readonly.each { |d| assert d.readonly? } diff --git a/activerecord/test/cases/subscriber_test.rb b/activerecord/test/cases/subscriber_test.rb index ce91d9385d..5328d4468b 100644 --- a/activerecord/test/cases/subscriber_test.rb +++ b/activerecord/test/cases/subscriber_test.rb @@ -3,7 +3,8 @@ require "models/developer" 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 @@ module SubscriberTest 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 ee7216a0f1..0000000000 --- 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 f2667a9b00..0000000000 --- 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 e9423a5b9d..0000000000 --- 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 b3de79a6c8..0000000000 --- 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 230d67b2fb..0000000000 --- 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 403a9a6e02..0000000000 --- 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/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index f48b35486c..704313649a 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -100,3 +100,8 @@ end class SubStiPost < StiPost self.table_name = Post.table_name end + +class PostWithComment < ActiveRecord::Base + self.table_name = 'posts' + default_scope where("posts.comments_count > 0").order("posts.comments_count ASC") +end diff --git a/activeresource/test/cases/subscriber_test.rb b/activeresource/test/cases/subscriber_test.rb index 7100b02872..3556fbf7cb 100644 --- a/activeresource/test/cases/subscriber_test.rb +++ b/activeresource/test/cases/subscriber_test.rb @@ -3,7 +3,8 @@ require "fixtures/person" 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 @@ module SubscriberTest 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/core_ext/load_error.rb b/activesupport/lib/active_support/core_ext/load_error.rb index cc6287b100..615ebe9588 100644 --- a/activesupport/lib/active_support/core_ext/load_error.rb +++ b/activesupport/lib/active_support/core_ext/load_error.rb @@ -1,36 +1,22 @@ -class MissingSourceFile < LoadError #:nodoc: - attr_reader :path - def initialize(message, path) - super(message) - @path = path - end - - def is_missing?(path) - path.gsub(/\.rb$/, '') == self.path.gsub(/\.rb$/, '') - end +class LoadError + REGEXPS = [ + /^no such file to load -- (.+)$/i, + /^Missing \w+ (?:file\s*)?([^\s]+.rb)$/i, + /^Missing API definition file in (.+)$/i, + ] - def self.from_message(message) - REGEXPS.each do |regexp, capture| - match = regexp.match(message) - return MissingSourceFile.new(message, match[capture]) unless match.nil? + def path + @path ||= begin + REGEXPS.find do |regex| + message =~ regex + end + $1 end - nil end - REGEXPS = [ - [/^no such file to load -- (.+)$/i, 1], - [/^Missing \w+ (file\s*)?([^\s]+.rb)$/i, 2], - [/^Missing API definition file in (.+)$/i, 1], - [/win32/, 0] - ] unless defined?(REGEXPS) -end - -class LoadError - def self.new(*args) - if self == LoadError - MissingSourceFile.from_message(args.first) - else - super - end + def is_missing?(location) + location.sub(/\.rb$/, '') == path.sub(/\.rb$/, '') end end + +MissingSourceFile = LoadError
\ No newline at end of file diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index e858bcdc80..8ded9f8b2d 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -236,7 +236,7 @@ module ActiveSupport #:nodoc: rescue LoadError => load_error unless swallow_load_errors if file_name = load_error.message[/ -- (.*?)(\.rb)?$/, 1] - raise MissingSourceFile.new(message % file_name, load_error.path).copy_blame!(load_error) + raise LoadError.new(message % file_name).copy_blame!(load_error) end raise end diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index bb07e4765c..ac482a2ec8 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -3,11 +3,9 @@ require 'thread' 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 @@ module ActiveSupport 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 @@ module ActiveSupport @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/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index cbb8e890ae..245d3ce051 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -172,7 +172,7 @@ module ActiveSupport MAPPING.freeze end - UTC_OFFSET_WITH_COLON = '%+03d:%02d' + UTC_OFFSET_WITH_COLON = '%s%02d:%02d' UTC_OFFSET_WITHOUT_COLON = UTC_OFFSET_WITH_COLON.sub(':', '') # Assumes self represents an offset from UTC in seconds (as returned from Time#utc_offset) @@ -181,9 +181,10 @@ module ActiveSupport # TimeZone.seconds_to_utc_offset(-21_600) # => "-06:00" def self.seconds_to_utc_offset(seconds, colon = true) format = colon ? UTC_OFFSET_WITH_COLON : UTC_OFFSET_WITHOUT_COLON - hours = seconds / 3600 + sign = (seconds < 0 ? '-' : '+') + hours = seconds.abs / 3600 minutes = (seconds.abs % 3600) / 60 - format % [hours, minutes] + format % [sign, hours, minutes] end include Comparable diff --git a/activesupport/test/core_ext/load_error_test.rb b/activesupport/test/core_ext/load_error_test.rb index b775b65f9f..d7b8f602ca 100644 --- a/activesupport/test/core_ext/load_error_test.rb +++ b/activesupport/test/core_ext/load_error_test.rb @@ -15,3 +15,18 @@ class TestMissingSourceFile < Test::Unit::TestCase end end end + +class TestLoadError < Test::Unit::TestCase + def test_with_require + assert_raise(LoadError) { require 'no_this_file_don\'t_exist' } + end + def test_with_load + assert_raise(LoadError) { load 'nor_does_this_one' } + end + def test_path + begin load 'nor/this/one.rb' + rescue LoadError => e + assert_equal 'nor/this/one.rb', e.path + end + end +end
\ No newline at end of file diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index c41d81fe7e..d3af535c26 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -3,18 +3,12 @@ require 'abstract_unit' 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 @@ module Notifications 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 @@ module Notifications 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/activesupport/test/time_zone_test.rb b/activesupport/test/time_zone_test.rb index 99c4310854..ce43c6507d 100644 --- a/activesupport/test/time_zone_test.rb +++ b/activesupport/test/time_zone_test.rb @@ -208,6 +208,12 @@ class TimeZoneTest < Test::Unit::TestCase assert_equal "+0000", ActiveSupport::TimeZone.seconds_to_utc_offset(0, false) assert_equal "+0500", ActiveSupport::TimeZone.seconds_to_utc_offset(18_000, false) end + + def test_seconds_to_utc_offset_with_negative_offset + assert_equal "-01:00", ActiveSupport::TimeZone.seconds_to_utc_offset(-3_600) + assert_equal "-00:59", ActiveSupport::TimeZone.seconds_to_utc_offset(-3_599) + assert_equal "-05:30", ActiveSupport::TimeZone.seconds_to_utc_offset(-19_800) + end def test_formatted_offset_positive zone = ActiveSupport::TimeZone['Moscow'] diff --git a/railties/builtin/rails_info/rails/info.rb b/railties/builtin/rails_info/rails/info.rb index 269fe488b0..90c9015fcf 100644 --- a/railties/builtin/rails_info/rails/info.rb +++ b/railties/builtin/rails_info/rails/info.rb @@ -114,7 +114,7 @@ module Rails end property 'Middleware' do - Rails.configuration.middleware.active.map { |middle| middle.inspect } + Rails.configuration.middleware.active.map(&:inspect) end # The Rails Git revision, if it's checked out into vendor/rails. 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.ru b/railties/lib/generators/rails/app/templates/config.ru index 2ab821e38d..fcfbc6b07a 100644 --- a/railties/lib/generators/rails/app/templates/config.ru +++ b/railties/lib/generators/rails/app/templates/config.ru @@ -1,4 +1,4 @@ # This file is used by Rack-based servers to start the application. require ::File.expand_path('../config/environment', __FILE__) -run <%= app_const %>.instance +run <%= app_const %> 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/generators/rails/app/templates/config/databases/ibm_db.yml b/railties/lib/generators/rails/app/templates/config/databases/ibm_db.yml index a9716ddb44..2cecb5c879 100644 --- a/railties/lib/generators/rails/app/templates/config/databases/ibm_db.yml +++ b/railties/lib/generators/rails/app/templates/config/databases/ibm_db.yml @@ -59,4 +59,4 @@ production: #account: my_account #app_user: my_app_user #application: my_application - #workstation: my_workstation
\ No newline at end of file + #workstation: my_workstation diff --git a/railties/lib/generators/rails/app/templates/config/environments/development.rb.tt b/railties/lib/generators/rails/app/templates/config/environments/development.rb.tt index b10103b436..177ce44d41 100644 --- a/railties/lib/generators/rails/app/templates/config/environments/development.rb.tt +++ b/railties/lib/generators/rails/app/templates/config/environments/development.rb.tt @@ -16,4 +16,4 @@ # Don't care if the mailer can't send config.action_mailer.raise_delivery_errors = false -end
\ No newline at end of file +end diff --git a/railties/lib/generators/rails/app/templates/config/environments/production.rb.tt b/railties/lib/generators/rails/app/templates/config/environments/production.rb.tt index 543a40108c..ee071df63b 100644 --- a/railties/lib/generators/rails/app/templates/config/environments/production.rb.tt +++ b/railties/lib/generators/rails/app/templates/config/environments/production.rb.tt @@ -30,4 +30,4 @@ # Enable threaded mode # config.threadsafe! -end
\ No newline at end of file +end diff --git a/railties/lib/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/generators/rails/app/templates/config/environments/test.rb.tt index 428fa35633..f6c38f3c0d 100644 --- a/railties/lib/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/generators/rails/app/templates/config/environments/test.rb.tt @@ -26,4 +26,4 @@ # This is necessary if your schema can't be completely dumped by the schema dumper, # like if you have constraints or database-specific column types # config.active_record.schema_format = :sql -end
\ No newline at end of file +end diff --git a/railties/lib/generators/rails/app/templates/config/initializers/backtrace_silencers.rb b/railties/lib/generators/rails/app/templates/config/initializers/backtrace_silencers.rb index 839d4cde19..59385cdf37 100644 --- a/railties/lib/generators/rails/app/templates/config/initializers/backtrace_silencers.rb +++ b/railties/lib/generators/rails/app/templates/config/initializers/backtrace_silencers.rb @@ -4,4 +4,4 @@ # Rails.backtrace_cleaner.add_silencer { |line| line =~ /my_noisy_library/ } # You can also remove all the silencers if you're trying to debug a problem that might stem from framework code. -# Rails.backtrace_cleaner.remove_silencers!
\ No newline at end of file +# Rails.backtrace_cleaner.remove_silencers! diff --git a/railties/lib/generators/rails/app/templates/config/locales/en.yml b/railties/lib/generators/rails/app/templates/config/locales/en.yml index f265c068d8..a747bfa698 100644 --- a/railties/lib/generators/rails/app/templates/config/locales/en.yml +++ b/railties/lib/generators/rails/app/templates/config/locales/en.yml @@ -2,4 +2,4 @@ # See http://github.com/svenfuchs/rails-i18n/tree/master/rails%2Flocale for starting points. en: - hello: "Hello world"
\ No newline at end of file + hello: "Hello world" diff --git a/railties/lib/generators/rails/app/templates/db/seeds.rb b/railties/lib/generators/rails/app/templates/db/seeds.rb index bc8695e6f0..664d8c74c8 100644 --- a/railties/lib/generators/rails/app/templates/db/seeds.rb +++ b/railties/lib/generators/rails/app/templates/db/seeds.rb @@ -2,6 +2,6 @@ # The data can then be loaded with the rake db:seed (or created alongside the db with db:setup). # # Examples: -# +# # cities = City.create([{ :name => 'Chicago' }, { :name => 'Copenhagen' }]) # Mayor.create(:name => 'Daley', :city => cities.first) diff --git a/railties/lib/generators/rails/app/templates/script/console.tt b/railties/lib/generators/rails/app/templates/script/console.tt index daba8ba2f1..915c5b8294 100755 --- a/railties/lib/generators/rails/app/templates/script/console.tt +++ b/railties/lib/generators/rails/app/templates/script/console.tt @@ -2,4 +2,4 @@ require File.expand_path('../../config/boot', __FILE__) require 'rails/commands/console' require File.expand_path('../../config/application', __FILE__) -Rails::Console.start(<%= app_const %>.instance) +Rails::Console.start(<%= app_const %>) diff --git a/railties/lib/generators/rails/app/templates/script/dbconsole.tt b/railties/lib/generators/rails/app/templates/script/dbconsole.tt index a7f114a97f..a92f6f2844 100755 --- a/railties/lib/generators/rails/app/templates/script/dbconsole.tt +++ b/railties/lib/generators/rails/app/templates/script/dbconsole.tt @@ -2,4 +2,4 @@ require File.expand_path('../../config/boot', __FILE__) require 'rails/commands/dbconsole' require File.expand_path('../../config/application', __FILE__) -Rails::DBConsole.start(<%= app_const %>.instance) +Rails::DBConsole.start(<%= app_const %>) diff --git a/railties/lib/generators/rails/metal/templates/metal.rb b/railties/lib/generators/rails/metal/templates/metal.rb index e94982b69a..2f5d4e7593 100644 --- a/railties/lib/generators/rails/metal/templates/metal.rb +++ b/railties/lib/generators/rails/metal/templates/metal.rb @@ -1,12 +1,12 @@ # Allow the metal piece to run in isolation -require(File.dirname(__FILE__) + "/../../config/environment") unless defined?(Rails) +require File.expand_path('../../../config/environment', __FILE__) class <%= class_name %> def self.call(env) if env["PATH_INFO"] =~ /^\/<%= file_name %>/ [200, {"Content-Type" => "text/html"}, ["Hello, World!"]] else - [404, {"Content-Type" => "text/html"}, ["Not Found"]] + [404, {"Content-Type" => "text/html", "X-Cascade" => "pass"}, ["Not Found"]] end end end diff --git a/railties/lib/generators/rails/plugin/templates/Rakefile.tt b/railties/lib/generators/rails/plugin/templates/Rakefile.tt index 23c2245a41..e94c0bfc77 100644 --- a/railties/lib/generators/rails/plugin/templates/Rakefile.tt +++ b/railties/lib/generators/rails/plugin/templates/Rakefile.tt @@ -1,22 +1,10 @@ -require 'rake' require 'rake/testtask' -require 'rake/rdoctask' desc 'Default: run unit tests.' task :default => :test desc 'Test the <%= file_name %> plugin.' Rake::TestTask.new(:test) do |t| - t.libs << 'lib' t.libs << 'test' t.pattern = 'test/**/*_test.rb' end - -desc 'Generate documentation for the <%= file_name %> plugin.' -Rake::RDocTask.new(:rdoc) do |rdoc| - rdoc.rdoc_dir = 'rdoc' - rdoc.title = '<%= class_name %>' - rdoc.options << '--line-numbers' << '--inline-source' - rdoc.rdoc_files.include('README') - rdoc.rdoc_files.include('lib/**/*.rb') -end diff --git a/railties/lib/generators/rails/stylesheets/templates/scaffold.css b/railties/lib/generators/rails/stylesheets/templates/scaffold.css index d9fa2cf2dc..de6669ad9e 100644 --- a/railties/lib/generators/rails/stylesheets/templates/scaffold.css +++ b/railties/lib/generators/rails/stylesheets/templates/scaffold.css @@ -59,4 +59,3 @@ div.field, div.actions { font-size: 12px; list-style: square; } - diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 4d05f8115c..8366127476 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -8,7 +8,13 @@ module Rails class << self attr_writer :config alias configure class_eval - delegate :initialize!, :load_tasks, :root, :to => :instance + delegate :call, + :initialize!, + :load_generators, + :load_tasks, + :middleware, + :root, + :to => :instance private :new def instance @@ -82,6 +88,10 @@ module Rails end end + def load_generators + plugins.each { |p| p.load_generators } + end + def initializers initializers = Bootstrap.new(self).initializers plugins.each { |p| initializers += p.initializers } @@ -89,13 +99,12 @@ module Rails initializers end - # TODO: Fix this method + # TODO: Fix this method. It loads all railties independent if :all is given + # or not, otherwise frameworks are never loaded. def plugins @plugins ||= begin plugin_names = (config.plugins || [:all]).map { |p| p.to_sym } - Railtie.plugins.select { |p| - plugin_names.include?(:all) || plugin_names.include?(p.plugin_name) - }.map { |p| p.new } + Plugin.all(plugin_names, config.paths.vendor.plugins) + Railtie.plugins.map(&:new) + Plugin.all(plugin_names, config.paths.vendor.plugins) end end @@ -107,6 +116,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 736c36c0dc..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) @@ -168,7 +173,7 @@ module Rails # Show help message with available generators. def self.help - traverse_load_paths! + lookup! namespaces = subclasses.map{ |k| k.namespace } namespaces.sort! @@ -226,27 +231,15 @@ module Rails nil end - # This will try to load any generator in the load path to show in help. - def self.traverse_load_paths! #:nodoc: - $LOAD_PATH.each do |base| - Dir[File.join(base, "{generators,rails_generators}", "**", "*_generator.rb")].each do |path| - begin - require path - rescue Exception => e - # No problem - end - end - end - end - # Receives namespaces in an array and tries to find matching generators # in the load path. 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 @@ -255,12 +248,36 @@ 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 end + # This will try to load any generator in the load path to show in help. + def self.lookup! #:nodoc: + load_generators_from_railties! + + $LOAD_PATH.each do |base| + Dir[File.join(base, "{generators,rails_generators}", "**", "*_generator.rb")].each do |path| + begin + require path + rescue Exception => e + # No problem + end + end + end + end + + # Allow generators to be loaded from custom paths. + def self.load_generators_from_railties! #:nodoc: + return if defined?(@generators_from_railties) || Rails.application.nil? + @generators_from_railties = true + Rails.application.load_generators + end + # Convert namespaces to paths by replacing ":" for "/" and adding # an extra lookup. For example, "rails:model" should be searched # in both: "rails/model/model_generator" and "rails/model_generator". @@ -268,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/plugin.rb b/railties/lib/rails/plugin.rb index 0c09730963..c3b81bcd3e 100644 --- a/railties/lib/rails/plugin.rb +++ b/railties/lib/rails/plugin.rb @@ -27,7 +27,7 @@ module Rails end def load_tasks - Dir["#{path}/**/tasks/**/*.rake"].sort.each { |ext| load ext } + Dir["#{path}/{tasks,lib/tasks,rails/tasks}/**/*.rake"].sort.each { |ext| load ext } end initializer :add_to_load_path, :after => :set_autoload_paths do |app| 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/railtie.rb b/railties/lib/rails/railtie.rb index 49a4d22319..611688cef1 100644 --- a/railties/lib/rails/railtie.rb +++ b/railties/lib/rails/railtie.rb @@ -150,13 +150,28 @@ module Rails @rake_tasks end + def self.generators(&blk) + @generators ||= [] + @generators << blk if blk + @generators + end + def rake_tasks self.class.rake_tasks end + def generators + self.class.generators + end + def load_tasks return unless rake_tasks rake_tasks.each { |blk| blk.call } end + + def load_generators + return unless generators + generators.each { |blk| blk.call } + end end end diff --git a/railties/lib/rails/subscriber.rb b/railties/lib/rails/subscriber.rb index a0cc5b86df..6638ff28c1 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. @@ -63,12 +63,11 @@ module Rails subscriber = subscribers[namespace.to_sym] if subscriber.respond_to?(name) && subscriber.logger - subscriber.send(name, ActiveSupport::Notifications::Event.new(*args)) - end - - if args[0] == "action_dispatch.after_dispatch" && !subscribers.empty? - flush_all! - log_tailer.tail! if log_tailer + begin + subscriber.send(name, ActiveSupport::Notifications::Event.new(*args)) + rescue Exception => e + Rails.logger.error "Could not log #{args[0].inspect} event. #{e.class}: #{e.message}" + end end end 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/lib/rails/tasks/middleware.rake b/railties/lib/rails/tasks/middleware.rake index e1ab309157..5a5bd7a7e9 100644 --- a/railties/lib/rails/tasks/middleware.rake +++ b/railties/lib/rails/tasks/middleware.rake @@ -3,5 +3,5 @@ task :middleware => :environment do Rails.configuration.middleware.active.each do |middleware| puts "use #{middleware.inspect}" end - puts "run ActionController::Routing::Routes" + puts "run #{Rails.application.class.name}" 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..31696598ce 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", @@ -76,7 +76,7 @@ module ApplicationTests end def middleware - AppTemplate::Application.instance.middleware.active.map(&:klass).map(&:name) + AppTemplate::Application.middleware.active.map(&:klass).map(&:name) end end end diff --git a/railties/test/application/routing_test.rb b/railties/test/application/routing_test.rb index 725dd06929..50cb9e3acc 100644 --- a/railties/test/application/routing_test.rb +++ b/railties/test/application/routing_test.rb @@ -176,5 +176,33 @@ module ApplicationTests get '/foo' assert_equal 'baz', last_response.body end + + test 'resource routing with irrigular inflection' do + app_file 'config/initializers/inflection.rb', <<-RUBY + ActiveSupport::Inflector.inflections do |inflect| + inflect.irregular 'yazi', 'yazilar' + end + RUBY + + app_file 'config/routes.rb', <<-RUBY + AppTemplate::Application.routes.draw do |map| + resources :yazilar + end + RUBY + + controller 'yazilar', <<-RUBY + class YazilarController < ActionController::Base + def index + render :text => 'yazilar#index' + end + end + RUBY + + get '/yazilars' + assert_equal 404, last_response.status + + get '/yazilar' + assert_equal 200, last_response.status + end end end 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/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 4bfe210efb..0a79e2cfb8 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -17,11 +17,11 @@ class PluginGeneratorTest < Rails::Generators::TestCase vendor/plugins/plugin_fu/uninstall.rb vendor/plugins/plugin_fu/lib vendor/plugins/plugin_fu/lib/plugin_fu.rb + vendor/plugins/plugin_fu/Rakefile ).each{ |path| assert_file path } %w( vendor/plugins/plugin_fu/README - vendor/plugins/plugin_fu/Rakefile ).each{ |path| assert_file path, /PluginFu/ } %w( diff --git a/railties/test/generators_test.rb b/railties/test/generators_test.rb index 60c81a813f..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"] @@ -148,6 +156,13 @@ class GeneratorsTest < Rails::Generators::TestCase Rails::Generators.subclasses.delete(klass) end + def test_load_generators_from_railties + Rails::Generators::ModelGenerator.expects(:start).with(["Account"], {}) + Rails::Generators.send(:remove_instance_variable, :@generators_from_railties) + Rails.application.expects(:load_generators) + Rails::Generators.invoke("model", ["Account"]) + end + def test_rails_root_templates template = File.join(Rails.root, "lib", "templates", "active_record", "model", "model.rb") diff --git a/railties/test/plugins/framework_extension_test.rb b/railties/test/plugins/framework_extension_test.rb index c920db77aa..d57fd4e635 100644 --- a/railties/test/plugins/framework_extension_test.rb +++ b/railties/test/plugins/framework_extension_test.rb @@ -30,6 +30,22 @@ module PluginsTest AppTemplate::Application.load_tasks assert $ran_block end + + test "generators block is executed when MyApp.load_generators is called" do + $ran_block = false + + class MyTie < Rails::Railtie + generators do + $ran_block = true + end + end + + require "#{app_path}/config/environment" + + assert !$ran_block + AppTemplate::Application.load_generators + assert $ran_block + end end class ActiveRecordExtensionTest < Test::Unit::TestCase diff --git a/railties/test/subscriber_test.rb b/railties/test/subscriber_test.rb index fa3f7bfabb..f6c895093f 100644 --- a/railties/test/subscriber_test.rb +++ b/railties/test/subscriber_test.rb @@ -18,9 +18,15 @@ class MySubscriber < Rails::Subscriber def bar(event) info "#{color("cool", :red)}, #{color("isn't it?", :blue, true)}" end + + def puke(event) + raise "puke" + end end -module SubscriberTest +class SyncSubscriberTest < ActiveSupport::TestCase + include Rails::Subscriber::TestHelper + def setup super @subscriber = MySubscriber.new @@ -90,41 +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_tails_logs_when_action_dispatch_callback_is_received - log_tailer = mock() - log_tailer.expects(:tail!) - Rails::Subscriber.log_tailer = log_tailer - + def test_logging_does_not_die_on_failures Rails::Subscriber.add :my_subscriber, @subscriber - instrument "action_dispatch.after_dispatch" + instrument "my_subscriber.puke" + 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 |