diff options
149 files changed, 2925 insertions, 2044 deletions
@@ -1,5 +1,6 @@ gem "rake", ">= 0.8.7" gem "mocha", ">= 0.9.8" +gem "ruby-debug", ">= 0.10.3" if RUBY_VERSION < '1.9' gem "rails", "3.0.pre", :path => "railties" %w(activesupport activemodel actionpack actionmailer activerecord activeresource).each do |lib| @@ -24,8 +24,15 @@ task :default => %w(test test:isolated) end end -spec = eval(File.read('rails.gemspec')) +desc "Smoke-test all projects" +task :smoke do + (PROJECTS - %w(activerecord)).each do |project| + system %(cd #{project} && #{env} #{$0} test:isolated) + end + system %(cd activerecord && #{env} #{$0} sqlite3:isolated_test) +end +spec = eval(File.read('rails.gemspec')) Rake::GemPackageTask.new(spec) do |pkg| pkg.gem_spec = spec end diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index 46760bba7c..4073e9b386 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -20,7 +20,7 @@ module AbstractController end def clear_template_caches! - @found_layouts.clear if @found_layouts + @found_layouts.clear if defined? @found_layouts super end diff --git a/actionpack/lib/abstract_controller/localized_cache.rb b/actionpack/lib/abstract_controller/localized_cache.rb index ee7b43cb9f..bf648af60a 100644 --- a/actionpack/lib/abstract_controller/localized_cache.rb +++ b/actionpack/lib/abstract_controller/localized_cache.rb @@ -1,6 +1,6 @@ module AbstractController class HashKey - @hash_keys = Hash.new {|h,k| h[k] = Hash.new {|h,k| h[k] = {} } } + @hash_keys = Hash.new {|h,k| h[k] = Hash.new {|sh,sk| sh[sk] = {} } } def self.get(klass, formats, locale) @hash_keys[klass][formats][locale] ||= new(klass, formats, locale) diff --git a/actionpack/lib/abstract_controller/logger.rb b/actionpack/lib/abstract_controller/logger.rb index e3bcd28da7..a23a13e1d6 100644 --- a/actionpack/lib/abstract_controller/logger.rb +++ b/actionpack/lib/abstract_controller/logger.rb @@ -9,25 +9,5 @@ module AbstractController cattr_accessor :logger extend ActiveSupport::Benchmarkable end - - # A class that allows you to defer expensive processing - # until the logger actually tries to log. Otherwise, you are - # forced to do the processing in advance, and send the - # entire processed String to the logger, which might - # just discard the String if the log level is too low. - # - # TODO: Require that Rails loggers accept a block. - class DelayedLog < ActiveSupport::BasicObject - def initialize(&block) - @str, @block = nil, block - end - - def method_missing(*args, &block) - unless @str - @str, @block = @block.call, nil - end - @str.send(*args, &block) - end - end end end diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb index dbba69f637..67656110c4 100644 --- a/actionpack/lib/action_controller/base.rb +++ b/actionpack/lib/action_controller/base.rb @@ -15,7 +15,6 @@ module ActionController include ActionController::ConditionalGet include ActionController::RackDelegation include ActionController::Logger - include ActionController::Benchmarking include ActionController::Configuration # Legacy modules diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index d784138ebe..69ed84da95 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -60,6 +60,17 @@ module ActionController #:nodoc: def cache_configured? perform_caching && cache_store end + + def log_event(name, before, after, instrumenter_id, payload) + if name.to_s =~ /(read|write|cache|expire|exist)_(fragment|page)\??/ + key_or_path = payload[:key] || payload[:path] + human_name = name.to_s.humanize + duration = (after - before) * 1000 + logger.info("#{human_name} #{key_or_path.inspect} (%.1fms)" % duration) + else + super + end + end end def caching_allowed? diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 8c1167d526..f569d0dd8b 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -74,7 +74,7 @@ module ActionController #:nodoc: return unless cache_configured? key = fragment_cache_key(key) - ActiveSupport::Notifications.instrument(:fragment_exist?, :key => key) do + ActiveSupport::Notifications.instrument(:exist_fragment?, :key => key) do cache_store.exist?(key, options) end end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index b436de9878..f445ca70ee 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -85,7 +85,7 @@ module ActionController end class ActionEndpoint - @@endpoints = Hash.new {|h,k| h[k] = Hash.new {|h,k| h[k] = {} } } + @@endpoints = Hash.new {|h,k| h[k] = Hash.new {|sh,sk| sh[sk] = {} } } def self.for(controller, action, stack) @@endpoints[controller][action][stack] ||= begin diff --git a/actionpack/lib/action_controller/metal/benchmarking.rb b/actionpack/lib/action_controller/metal/benchmarking.rb deleted file mode 100644 index f73f635b0d..0000000000 --- a/actionpack/lib/action_controller/metal/benchmarking.rb +++ /dev/null @@ -1,72 +0,0 @@ -require 'active_support/core_ext/benchmark' - -module ActionController #:nodoc: - # The benchmarking module times the performance of actions and reports to the logger. If the Active Record - # package has been included, a separate timing section for database calls will be added as well. - module Benchmarking #:nodoc: - extend ActiveSupport::Concern - - protected - def render(*args, &block) - if logger - if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected? - db_runtime = ActiveRecord::Base.connection.reset_runtime - end - - render_output = nil - @view_runtime = Benchmark.ms { render_output = super } - - if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected? - @db_rt_before_render = db_runtime - @db_rt_after_render = ActiveRecord::Base.connection.reset_runtime - @view_runtime -= @db_rt_after_render - end - - render_output - else - super - end - end - - private - def process_action(*args) - if logger - ms = [Benchmark.ms { super }, 0.01].max - logging_view = defined?(@view_runtime) - logging_active_record = Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected? - - log_message = 'Completed in %.0fms' % ms - - if logging_view || logging_active_record - log_message << " (" - log_message << view_runtime if logging_view - - if logging_active_record - log_message << ", " if logging_view - log_message << active_record_runtime + ")" - else - ")" - end - end - - log_message << " | #{response.status}" - log_message << " [#{complete_request_uri rescue "unknown"}]" - - logger.info(log_message) - else - super - end - end - - def view_runtime - "View: %.0f" % @view_runtime - end - - def active_record_runtime - db_runtime = ActiveRecord::Base.connection.reset_runtime - db_runtime += @db_rt_before_render if @db_rt_before_render - db_runtime += @db_rt_after_render if @db_rt_after_render - "DB: %.0f" % db_runtime - end - end -end diff --git a/actionpack/lib/action_controller/metal/cookies.rb b/actionpack/lib/action_controller/metal/cookies.rb index 8d5f0d7199..5b51bd21d0 100644 --- a/actionpack/lib/action_controller/metal/cookies.rb +++ b/actionpack/lib/action_controller/metal/cookies.rb @@ -58,140 +58,140 @@ module ActionController #:nodoc: def cookies @cookies ||= CookieJar.build(request, response) end - end + end - class CookieJar < Hash #:nodoc: - def self.build(request, response) - new.tap do |hash| - hash.update(request.cookies) - hash.response = response - end + class CookieJar < Hash #:nodoc: + def self.build(request, response) + new.tap do |hash| + hash.update(request.cookies) + hash.response = response end + end - attr_accessor :response + attr_accessor :response - # Returns the value of the cookie by +name+, or +nil+ if no such cookie exists. - def [](name) - super(name.to_s) - end + # Returns the value of the cookie by +name+, or +nil+ if no such cookie exists. + def [](name) + super(name.to_s) + end - # Sets the cookie named +name+. The second argument may be the very cookie - # value, or a hash of options as documented above. - def []=(key, options) - if options.is_a?(Hash) - options.symbolize_keys! - value = options[:value] - else - value = options - options = { :value => value } - end - - super(key.to_s, value) - - options[:path] ||= "/" - response.set_cookie(key, options) + # Sets the cookie named +name+. The second argument may be the very cookie + # value, or a hash of options as documented above. + def []=(key, options) + if options.is_a?(Hash) + options.symbolize_keys! + value = options[:value] + else + value = options + options = { :value => value } end - # Removes the cookie on the client machine by setting the value to an empty string - # and setting its expiration date into the past. Like <tt>[]=</tt>, you can pass in - # an options hash to delete cookies with extra data such as a <tt>:path</tt>. - def delete(key, options = {}) + super(key.to_s, value) + + options[:path] ||= "/" + response.set_cookie(key, options) + end + + # Removes the cookie on the client machine by setting the value to an empty string + # and setting its expiration date into the past. Like <tt>[]=</tt>, you can pass in + # an options hash to delete cookies with extra data such as a <tt>:path</tt>. + def delete(key, options = {}) + options.symbolize_keys! + options[:path] ||= "/" + value = super(key.to_s) + response.delete_cookie(key, options) + value + end + + # Returns a jar that'll automatically set the assigned cookies to have an expiration date 20 years from now. Example: + # + # cookies.permanent[:prefers_open_id] = true + # # => Set-Cookie: prefers_open_id=true; path=/; expires=Sun, 16-Dec-2029 03:24:16 GMT + # + # This jar is only meant for writing. You'll read permanent cookies through the regular accessor. + # + # This jar allows chaining with the signed jar as well, so you can set permanent, signed cookies. Examples: + # + # cookies.permanent.signed[:remember_me] = current_user.id + # # => Set-Cookie: discount=BAhU--848956038e692d7046deab32b7131856ab20e14e; path=/; expires=Sun, 16-Dec-2029 03:24:16 GMT + def permanent + @permanent ||= PermanentCookieJar.new(self) + end + + # Returns a jar that'll automatically generate a signed representation of cookie value and verify it when reading from + # the cookie again. This is useful for creating cookies with values that the user is not supposed to change. If a signed + # cookie was tampered with by the user (or a 3rd party), an ActiveSupport::MessageVerifier::InvalidSignature exception will + # be raised. + # + # This jar requires that you set a suitable secret for the verification on ActionController::Base.cookie_verifier_secret. + # + # Example: + # + # cookies.signed[:discount] = 45 + # # => Set-Cookie: discount=BAhpMg==--2c1c6906c90a3bc4fd54a51ffb41dffa4bf6b5f7; path=/ + # + # cookies.signed[:discount] # => 45 + def signed + @signed ||= SignedCookieJar.new(self) + end + end + + class PermanentCookieJar < CookieJar #:nodoc: + def initialize(parent_jar) + @parent_jar = parent_jar + end + + def []=(key, options) + if options.is_a?(Hash) options.symbolize_keys! - options[:path] ||= "/" - value = super(key.to_s) - response.delete_cookie(key, options) - value + else + options = { :value => options } end - # Returns a jar that'll automatically set the assigned cookies to have an expiration date 20 years from now. Example: - # - # cookies.permanent[:prefers_open_id] = true - # # => Set-Cookie: prefers_open_id=true; path=/; expires=Sun, 16-Dec-2029 03:24:16 GMT - # - # This jar is only meant for writing. You'll read permanent cookies through the regular accessor. - # - # This jar allows chaining with the signed jar as well, so you can set permanent, signed cookies. Examples: - # - # cookies.permanent.signed[:remember_me] = current_user.id - # # => Set-Cookie: discount=BAhU--848956038e692d7046deab32b7131856ab20e14e; path=/; expires=Sun, 16-Dec-2029 03:24:16 GMT - def permanent - @permanent ||= PermanentCookieJar.new(self) - end - - # Returns a jar that'll automatically generate a signed representation of cookie value and verify it when reading from - # the cookie again. This is useful for creating cookies with values that the user is not supposed to change. If a signed - # cookie was tampered with by the user (or a 3rd party), an ActiveSupport::MessageVerifier::InvalidSignature exception will - # be raised. - # - # This jar requires that you set a suitable secret for the verification on ActionController::Base.cookie_verifier_secret. - # - # Example: - # - # cookies.signed[:discount] = 45 - # # => Set-Cookie: discount=BAhpMg==--2c1c6906c90a3bc4fd54a51ffb41dffa4bf6b5f7; path=/ - # - # cookies.signed[:discount] # => 45 - def signed - @signed ||= SignedCookieJar.new(self) - end + options[:expires] = 20.years.from_now + @parent_jar[key] = options end - - class PermanentCookieJar < CookieJar #:nodoc: - def initialize(parent_jar) - @parent_jar = parent_jar - end - - def []=(key, options) - if options.is_a?(Hash) - options.symbolize_keys! - else - options = { :value => options } - end - - options[:expires] = 20.years.from_now - @parent_jar[key] = options - end - - def signed - @signed ||= SignedCookieJar.new(self) - end - - def controller - @parent_jar.controller - end - - def method_missing(method, *arguments, &block) - @parent_jar.send(method, *arguments, &block) - end + + def signed + @signed ||= SignedCookieJar.new(self) end - - class SignedCookieJar < CookieJar #:nodoc: - def initialize(parent_jar) - unless ActionController::Base.cookie_verifier_secret - raise "You must set ActionController::Base.cookie_verifier_secret to use signed cookies" - end - - @parent_jar = parent_jar - @verifier = ActiveSupport::MessageVerifier.new(ActionController::Base.cookie_verifier_secret) - end - - def [](name) - @verifier.verify(@parent_jar[name]) - end - - def []=(key, options) - if options.is_a?(Hash) - options.symbolize_keys! - options[:value] = @verifier.generate(options[:value]) - else - options = { :value => @verifier.generate(options) } - end - - @parent_jar[key] = options + + def controller + @parent_jar.controller + end + + def method_missing(method, *arguments, &block) + @parent_jar.send(method, *arguments, &block) + end + end + + class SignedCookieJar < CookieJar #:nodoc: + def initialize(parent_jar) + unless ActionController::Base.cookie_verifier_secret + raise "You must set ActionController::Base.cookie_verifier_secret to use signed cookies" end - - def method_missing(method, *arguments, &block) - @parent_jar.send(method, *arguments, &block) + + @parent_jar = parent_jar + @verifier = ActiveSupport::MessageVerifier.new(ActionController::Base.cookie_verifier_secret) + end + + def [](name) + @verifier.verify(@parent_jar[name]) + end + + def []=(key, options) + if options.is_a?(Hash) + options.symbolize_keys! + options[:value] = @verifier.generate(options[:value]) + else + options = { :value => @verifier.generate(options) } end + + @parent_jar[key] = options + end + + def method_missing(method, *arguments, &block) + @parent_jar.send(method, *arguments, &block) + end end end diff --git a/actionpack/lib/action_controller/metal/filter_parameter_logging.rb b/actionpack/lib/action_controller/metal/filter_parameter_logging.rb index a53c052075..59e200396a 100644 --- a/actionpack/lib/action_controller/metal/filter_parameter_logging.rb +++ b/actionpack/lib/action_controller/metal/filter_parameter_logging.rb @@ -2,8 +2,6 @@ module ActionController module FilterParameterLogging extend ActiveSupport::Concern - include AbstractController::Logger - module ClassMethods # Replace sensitive parameter data from the request log. # Filters parameters that have any of the arguments as a substring. @@ -54,23 +52,25 @@ module ActionController end protected :filter_parameters end - end - INTERNAL_PARAMS = [:controller, :action, :format, :_method, :only_path] + protected - def process(*) - response = super - if logger - parameters = filter_parameters(params).except!(*INTERNAL_PARAMS) - logger.info { " Parameters: #{parameters.inspect}" } unless parameters.empty? + # Overwrite log_process_action to include parameters information. + # If this method is invoked, it means logger is defined, so don't + # worry with such scenario here. + def log_process_action(controller) #:nodoc: + params = controller.send(:filter_parameters, controller.request.params) + logger.info " Parameters: #{params.inspect}" unless params.empty? + super end - response end + INTERNAL_PARAMS = [:controller, :action, :format, :_method, :only_path] + protected def filter_parameters(params) - params.dup + params.dup.except!(*INTERNAL_PARAMS) end end diff --git a/actionpack/lib/action_controller/metal/flash.rb b/actionpack/lib/action_controller/metal/flash.rb index 581ff6109e..682f90e23b 100644 --- a/actionpack/lib/action_controller/metal/flash.rb +++ b/actionpack/lib/action_controller/metal/flash.rb @@ -123,80 +123,79 @@ module ActionController #:nodoc: session["flash"] = self end - private - # Used internally by the <tt>keep</tt> and <tt>discard</tt> methods - # use() # marks the entire flash as used - # use('msg') # marks the "msg" entry as used - # use(nil, false) # marks the entire flash as unused (keeps it around for one more action) - # use('msg', false) # marks the "msg" entry as unused (keeps it around for one more action) - # Returns the single value for the key you asked to be marked (un)used or the FlashHash itself - # if no key is passed. - def use(key = nil, used = true) - Array(key || keys).each { |k| used ? @used << k : @used.delete(k) } - return key ? self[key] : self - end - end - - # Access the contents of the flash. Use <tt>flash["notice"]</tt> to - # read a notice you put there or <tt>flash["notice"] = "hello"</tt> - # to put a new one. - def flash #:doc: - unless @_flash - @_flash = session["flash"] || FlashHash.new - @_flash.sweep + private + # Used internally by the <tt>keep</tt> and <tt>discard</tt> methods + # use() # marks the entire flash as used + # use('msg') # marks the "msg" entry as used + # use(nil, false) # marks the entire flash as unused (keeps it around for one more action) + # use('msg', false) # marks the "msg" entry as unused (keeps it around for one more action) + # Returns the single value for the key you asked to be marked (un)used or the FlashHash itself + # if no key is passed. + def use(key = nil, used = true) + Array(key || keys).each { |k| used ? @used << k : @used.delete(k) } + return key ? self[key] : self + end end - @_flash - end - - # Convenience accessor for flash[:alert] - def alert - flash[:alert] - end - - # Convenience accessor for flash[:alert]= - def alert=(message) - flash[:alert] = message - end + # Access the contents of the flash. Use <tt>flash["notice"]</tt> to + # read a notice you put there or <tt>flash["notice"] = "hello"</tt> + # to put a new one. + def flash #:doc: + unless @_flash + @_flash = session["flash"] || FlashHash.new + @_flash.sweep + end - # Convenience accessor for flash[:notice] - def notice - flash[:notice] - end + @_flash + end - # Convenience accessor for flash[:notice]= - def notice=(message) - flash[:notice] = message - end + # Convenience accessor for flash[:alert] + def alert + flash[:alert] + end + # Convenience accessor for flash[:alert]= + def alert=(message) + flash[:alert] = message + end - protected - def process_action(method_name) - @_flash = nil - super - @_flash.store(session) if @_flash - @_flash = nil + # Convenience accessor for flash[:notice] + def notice + flash[:notice] end - def reset_session - super - @_flash = nil + # Convenience accessor for flash[:notice]= + def notice=(message) + flash[:notice] = message end - def redirect_to(options = {}, response_status_and_flash = {}) #:doc: - if alert = response_status_and_flash.delete(:alert) - flash[:alert] = alert + protected + def process_action(method_name) + @_flash = nil + super + @_flash.store(session) if @_flash + @_flash = nil end - if notice = response_status_and_flash.delete(:notice) - flash[:notice] = notice + def reset_session + super + @_flash = nil end - if other_flashes = response_status_and_flash.delete(:flash) - flash.update(other_flashes) - end + def redirect_to(options = {}, response_status_and_flash = {}) #:doc: + if alert = response_status_and_flash.delete(:alert) + flash[:alert] = alert + end - super(options, response_status_and_flash) - end + if notice = response_status_and_flash.delete(:notice) + flash[:notice] = notice + end + + if other_flashes = response_status_and_flash.delete(:flash) + flash.update(other_flashes) + end + + super(options, response_status_and_flash) + end end end diff --git a/actionpack/lib/action_controller/metal/logger.rb b/actionpack/lib/action_controller/metal/logger.rb index 956d7dd371..e71f77fbb2 100644 --- a/actionpack/lib/action_controller/metal/logger.rb +++ b/actionpack/lib/action_controller/metal/logger.rb @@ -1,34 +1,85 @@ require 'abstract_controller/logger' module ActionController + # Adds instrumentation to <tt>process_action</tt> and a <tt>log_event</tt> method + # responsible to log events from ActiveSupport::Notifications. This module handles + # :process_action and :render_template events but allows any other module to hook + # into log_event and provide its own logging facilities (as in ActionController::Caching). module Logger - # Override process_action in the AbstractController::Base - # to log details about the method. + extend ActiveSupport::Concern + + attr_internal :view_runtime + def process_action(action) - result = ActiveSupport::Notifications.instrument(:process_action, - :controller => self, :action => action) do + ActiveSupport::Notifications.instrument(:process_action, :controller => self, :action => action) do super end + end + def render(*args, &block) if logger - log = AbstractController::Logger::DelayedLog.new do - "\n\nProcessing #{self.class.name}\##{action_name} " \ - "to #{request.formats} (for #{request_origin}) " \ - "[#{request.method.to_s.upcase}]" + render_output = nil + + self.view_runtime = cleanup_view_runtime do + Benchmark.ms { render_output = super } end - logger.info(log) + render_output + else + super end + end - result + # If you want to remove any time taken into account in :view_runtime + # wrongly, you can do it here: + # + # def cleanup_view_runtime + # super - time_taken_in_something_expensive + # end + # + # :api: plugin + def cleanup_view_runtime #:nodoc: + yield end - private + module ClassMethods + # This is the hook invoked by ActiveSupport::Notifications.subscribe. + # If you need to log any event, overwrite the method and do it here. + def log_event(name, before, after, instrumenter_id, payload) #:nodoc: + if name == :process_action + duration = [(after - before) * 1000, 0.01].max + controller = payload[:controller] + request = controller.request + + logger.info "\n\nProcessed #{controller.class.name}##{payload[:action]} " \ + "to #{request.formats} (for #{request.remote_ip} at #{before.to_s(:db)}) " \ + "[#{request.method.to_s.upcase}]" - # Returns the request origin with the IP and time. This needs to be cached, - # otherwise we would get different results for each time it calls. - def request_origin - @request_origin ||= "#{request.remote_ip} at #{Time.now.to_s(:db)}" + log_process_action(controller) + + message = "Completed in %.0fms" % duration + message << " | #{controller.response.status}" + message << " [#{request.request_uri rescue "unknown"}]" + + logger.info(message) + elsif name == :render_template + # TODO Make render_template logging work if you are using just ActionView + duration = (after - before) * 1000 + message = "Rendered #{payload[:identifier]}" + message << " within #{payload[:layout]}" if payload[:layout] + message << (" (%.1fms)" % duration) + logger.info(message) + end + end + + protected + + # A hook which allows logging what happened during controller process action. + # :api: plugin + def log_process_action(controller) #:nodoc: + view_runtime = controller.send :view_runtime + logger.info(" View runtime: %.1fms" % view_runtime.to_f) if view_runtime + end end end -end +end
\ No newline at end of file diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 2826b1e34c..f1fb4d7ce5 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -13,7 +13,7 @@ module ActionController #:nodoc: cattr_accessor :request_forgery_protection_token # Controls whether request forgergy protection is turned on or not. Turned off by default only in test mode. - class_inheritable_accessor :allow_forgery_protection + extlib_inheritable_accessor :allow_forgery_protection self.allow_forgery_protection = true helper_method :form_authenticity_token diff --git a/actionpack/lib/action_controller/rails.rb b/actionpack/lib/action_controller/rails.rb index 36a52b3149..6ebb50887b 100644 --- a/actionpack/lib/action_controller/rails.rb +++ b/actionpack/lib/action_controller/rails.rb @@ -1,6 +1,7 @@ module ActionController class Plugin < Rails::Plugin plugin_name :action_controller + include_modules_in "ActionController::Base" initializer "action_controller.set_configs" do |app| app.config.action_controller.each do |k,v| @@ -14,10 +15,6 @@ module ActionController end # Routing must be initialized after plugins to allow the former to extend the routes - # --- - # If Action Controller is not one of the loaded frameworks (Configuration#frameworks) - # this does nothing. Otherwise, it loads the routing definitions and sets up - # loading module used to lazily load controllers (Configuration#controller_paths). initializer "action_controller.initialize_routing" do |app| app.route_configuration_files << app.config.routes_configuration_file app.route_configuration_files << app.config.builtin_routes_configuration_file @@ -88,13 +85,8 @@ module ActionController initializer "action_controller.notifications" do |app| require 'active_support/notifications' - ActiveSupport::Notifications.subscribe(/(read|write|cache|expire|exist)_(fragment|page)\??/) do |*args| - event = ActiveSupport::Notifications::Event.new(*args) - - if logger = ActionController::Base.logger - human_name = event.name.to_s.humanize - logger.info("#{human_name} (%.1fms)" % event.duration) - end + ActiveSupport::Notifications.subscribe do |*args| + ActionController::Base.log_event(*args) if ActionController::Base.logger end end diff --git a/actionpack/lib/action_dispatch.rb b/actionpack/lib/action_dispatch.rb index 0696cb017c..7f61ff5657 100644 --- a/actionpack/lib/action_dispatch.rb +++ b/actionpack/lib/action_dispatch.rb @@ -24,6 +24,7 @@ activesupport_path = File.expand_path('../../../activesupport/lib', __FILE__) $:.unshift(activesupport_path) if File.directory?(activesupport_path) && !$:.include?(activesupport_path) require 'active_support/ruby/shim' +require 'active_support/dependencies/autoload' require 'rack' diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 2a41b4dbad..1e43104f0a 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -6,13 +6,13 @@ module ActionDispatch extend ActiveSupport::Memoizable def initialize(*args) - if args.size == 1 && args[0].is_a?(Hash) - super() - update(args[0]) - else - super - end - end + if args.size == 1 && args[0].is_a?(Hash) + super() + update(args[0]) + else + super + end + end def [](header_name) if include?(header_name) diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index c30897b32a..13c0f2bad0 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -10,8 +10,8 @@ module Mime %w(<< concat shift unshift push pop []= clear compact! collect! delete delete_at delete_if flatten! map! insert reject! reverse! replace slice! sort! uniq!).each do |method| - module_eval <<-CODE - def #{method}(*args) + module_eval <<-CODE, __FILE__, __LINE__ + 1 + def #{method}(*) @symbols = nil super end @@ -104,7 +104,7 @@ module Mime SET << Mime.const_get(symbol.to_s.upcase) - ([string] + mime_type_synonyms).each { |string| LOOKUP[string] = SET.last } unless skip_lookup + ([string] + mime_type_synonyms).each { |str| LOOKUP[str] = SET.last } unless skip_lookup ([symbol.to_s] + extension_synonyms).each { |ext| EXTENSION_LOOKUP[ext] = SET.last } end diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index 3b27309f58..24be4fee55 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -93,8 +93,9 @@ module ActionDispatch alias_method :insert_before, :insert def insert_after(index, *args, &block) - index = self.index(index) unless index.is_a?(Integer) - insert(index + 1, *args, &block) + i = index.is_a?(Integer) ? index : self.index(index) + raise "No such middleware to insert after: #{index.inspect}" unless i + insert(i + 1, *args, &block) end def swap(target, *args, &block) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index e655d6a708..8f33346a4f 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -43,9 +43,13 @@ module ActionDispatch def extract_path_and_options(args) options = args.extract_options! - if args.empty? + case + when using_to_shorthand?(args, options) path, to = options.find { |name, value| name.is_a?(String) } options.merge!(:to => to).delete(path) if path + when using_match_shorthand?(args, options) + path = args.first + options = { :to => path.gsub("/", "#"), :as => path.gsub("/", "_") } else path = args.first end @@ -53,6 +57,16 @@ module ActionDispatch [ normalize_path(path), options ] end + # match "account" => "account#index" + def using_to_shorthand?(args, options) + args.empty? && options.present? + end + + # match "account/overview" + def using_match_shorthand?(args, options) + args.present? && options.except(:via).empty? && !args.first.include?(':') + end + def normalize_path(path) path = nil if path == "" path = "#{@scope[:path]}#{path}" if @scope[:path] diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index aeaf1ee4ff..5158415c20 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -191,45 +191,60 @@ module ActionView def setup(options, block) partial = options[:partial] - @options = options - @locals = options[:locals] || {} - @block = block + @options = options + @locals = options[:locals] || {} + @block = block if String === partial - @object = options[:object] - @path = partial + @object = options[:object] + @path = partial + @collection = collection else @object = partial - @path = partial_path(partial) + + if @collection = collection + paths = @collection_paths = @collection.map { |o| partial_path(o) } + @path = paths.uniq.size == 1 ? paths.first : nil + else + @path = partial_path + end end end def render - if @collection = collection - render_collection + options = @options + + if @collection + ActiveSupport::Notifications.instrument(:render_collection, :path => @path, + :count => @collection.size) do + render_collection + end else - @template = template = find_template - render_template(template, @object || @locals[template.variable_name]) + content = ActiveSupport::Notifications.instrument(:render_partial, :path => @path) do + render_partial + end + + if !@block && options[:layout] + content = @view._render_layout(find_template(options[:layout]), @locals){ content } + end + content end end def render_collection @template = template = find_template - return nil if @collection.blank? if @options.key?(:spacer_template) spacer = find_template(@options[:spacer_template]).render(@view, @locals) end - result = template ? collection_with_template(template) : collection_without_template + result = template ? collection_with_template : collection_without_template result.join(spacer).html_safe! end - def collection_with_template(template) - options = @options - - segments, locals, as = [], @locals, options[:as] || template.variable_name + def collection_with_template(template = @template) + segments, locals, as = [], @locals, @options[:as] || template.variable_name counter_name = template.counter_name locals[counter_name] = -1 @@ -245,16 +260,14 @@ module ActionView segments end - def collection_without_template - options = @options - - segments, locals, as = [], @locals, options[:as] + def collection_without_template(collection_paths = @collection_paths) + segments, locals, as = [], @locals, @options[:as] index, template = -1, nil - @collection.each do |object| - template = find_template(partial_path(object)) + @collection.each_with_index do |object, i| + template = find_template(collection_paths[i]) locals[template.counter_name] = (index += 1) - locals[template.variable_name] = object + locals[as || template.variable_name] = object segments << template.render(@view, locals) end @@ -263,18 +276,15 @@ module ActionView segments end - def render_template(template, object = @object) - options, locals, view = @options, @locals, @view - locals[options[:as] || template.variable_name] = object + def render_partial(object = @object) + @template = template = find_template + locals, view = @locals, @view - content = template.render(view, locals) do |*name| - @view._layout_for(*name, &@block) - end + object ||= locals[template.variable_name] + locals[@options[:as] || template.variable_name] = object - if @block || !options[:layout] - content - else - find_template(options[:layout]).render(@view, @locals) { content } + template.render(view, locals) do |*name| + view._layout_for(*name, &@block) end end @@ -305,9 +315,9 @@ module ActionView def partial_path(object = @object) @partial_names[object.class] ||= begin - return nil unless object.respond_to?(:to_model) + object = object.to_model if object.respond_to?(:to_model) - object.to_model.class.model_name.partial_path.dup.tap do |partial| + object.class.model_name.partial_path.dup.tap do |partial| path = @view.controller_path partial.insert(0, "#{File.dirname(path)}/") if path.include?(?/) end diff --git a/actionpack/lib/action_view/render/rendering.rb b/actionpack/lib/action_view/render/rendering.rb index d4d16b4d98..0302e44b4e 100644 --- a/actionpack/lib/action_view/render/rendering.rb +++ b/actionpack/lib/action_view/render/rendering.rb @@ -73,25 +73,24 @@ module ActionView # would be <html>Hello David</html>. def _layout_for(name = nil, &block) return @_content_for[name || :layout] if !block_given? || name - capture(&block) end def _render_inline(inline, layout, options) - handler = Template.handler_class_for_extension(options[:type] || "erb") - template = Template.new(options[:inline], "inline template", handler, {}) + locals = options[:locals] - locals = options[:locals] - content = template.render(self, locals) + content = ActiveSupport::Notifications.instrument(:render_inline) do + handler = Template.handler_class_for_extension(options[:type] || "erb") + template = Template.new(options[:inline], "inline template", handler, {}) + template.render(self, locals) + end _render_text(content, layout, locals) end def _render_text(content, layout, locals) - content = layout.render(self, locals) do |*name| - _layout_for(*name) { content } - end if layout - + ActiveSupport::Notifications.instrument(:render_text) + content = _render_layout(layout, locals){ content } if layout content end @@ -108,23 +107,27 @@ module ActionView end def _render_template(template, layout = nil, options = {}, partial = nil) - logger && logger.info do - msg = "Rendering #{template.inspect}" - msg << " (#{options[:status]})" if options[:status] - msg + locals = options[:locals] || {} + + content = ActiveSupport::Notifications.instrument(:render_template, + :identifier => template.identifier, :layout => (layout ? layout.identifier : nil)) do + partial ? _render_partial_object(template, options) : template.render(self, locals) end - locals = options[:locals] || {} - content = partial ? _render_partial_object(template, options) : template.render(self, locals) @_content_for[:layout] = content if layout @_layout = layout.identifier - logger.info("Rendering template within #{layout.inspect}") if logger - content = layout.render(self, locals) { |*name| _layout_for(*name) } + content = _render_layout(layout, locals) end content end + + def _render_layout(layout, locals, &block) + ActiveSupport::Notifications.instrument(:render_layout, :identifier => layout.identifier) do + layout.render(self, locals){ |*name| _layout_for(*name, &block) } + end + end end end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index d46c989d11..adaf6544a7 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -36,10 +36,8 @@ module ActionView end def render(view, locals, &block) - ActiveSupport::Notifications.instrument(:render_template, :identifier => identifier) do - method_name = compile(locals, view) - view.send(method_name, locals, &block) - end + method_name = compile(locals, view) + view.send(method_name, locals, &block) rescue Exception => e if e.is_a?(Template::Error) e.sub_template_of(self) diff --git a/actionpack/lib/action_view/template/handler.rb b/actionpack/lib/action_view/template/handler.rb index 5a46a27893..221d1bd5c5 100644 --- a/actionpack/lib/action_view/template/handler.rb +++ b/actionpack/lib/action_view/template/handler.rb @@ -17,8 +17,8 @@ module ActionView end def compile(template) - raise "Need to implement #{self.class.name}#compile(template)" - end + raise "Need to implement #{self.class.name}#compile(template)" + end end end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index a2f4ab2ef5..c6a17907ff 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -108,13 +108,9 @@ module ActionView query << '{' << ext.map {|e| e && ".#{e}" }.join(',') << '}' end - Dir[query].map do |path| - next if File.directory?(path) - source = File.read(path) - identifier = Pathname.new(path).expand_path.to_s - - Template.new(source, identifier, *path_to_details(path)) - end.compact + Dir[query].reject { |p| File.directory?(p) }.map do |p| + Template.new(File.read(p), File.expand_path(p), *path_to_details(p)) + end end # # TODO: fix me @@ -162,4 +158,4 @@ module ActionView @paths.first.to_s end end -end
\ No newline at end of file +end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index a9341b60df..8c65087898 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -50,6 +50,14 @@ ORIGINAL_LOCALES = I18n.available_locales.map {|locale| locale.to_s }.sort FIXTURE_LOAD_PATH = File.join(File.dirname(__FILE__), 'fixtures') FIXTURES = Pathname.new(FIXTURE_LOAD_PATH) +# Turn on notifications +require 'active_support/notifications' +Thread.abort_on_exception = true + +ActiveSupport::Notifications.subscribe do |*args| + ActionController::Base.log_event(*args) if ActionController::Base.logger +end + module SetupOnce extend ActiveSupport::Concern @@ -87,6 +95,21 @@ class ActiveSupport::TestCase end end +class MockLogger + attr_reader :logged + attr_accessor :level + + def initialize + @level = Logger::DEBUG + @logged = [] + end + + def method_missing(method, *args, &blk) + @logged << args.first + @logged << blk.call if block_given? + end +end + class ActionController::IntegrationTest < ActiveSupport::TestCase def self.build_app(routes = nil) ActionDispatch::MiddlewareStack.new { |middleware| diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index 8f8ada8d8c..65118f9bc9 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -158,13 +158,6 @@ class PerformActionTest < ActionController::TestCase assert_raise(ActionController::UnknownAction) { get :hidden_action } assert_raise(ActionController::UnknownAction) { get :another_hidden_action } end - - def test_namespaced_action_should_log_module_name - use_controller Submodule::ContainedNonEmptyController - @controller.logger = MockLogger.new - get :public_action - assert_match /Processing\sSubmodule::ContainedNonEmptyController#public_action/, @controller.logger.logged[1] - end end class DefaultUrlOptionsTest < ActionController::TestCase diff --git a/actionpack/test/controller/benchmark_test.rb b/actionpack/test/controller/benchmark_test.rb deleted file mode 100644 index 66ebfcf20a..0000000000 --- a/actionpack/test/controller/benchmark_test.rb +++ /dev/null @@ -1,33 +0,0 @@ -require 'abstract_unit' - -# Provide some static controllers. -class BenchmarkedController < ActionController::Base - def public_action - render :nothing => true - end - - def rescue_action(e) - raise e - end -end - -class BenchmarkTest < ActionController::TestCase - tests BenchmarkedController - - class MockLogger - def method_missing(*args) - end - end - - def setup - super - # benchmark doesn't do anything unless a logger is set - @controller.logger = MockLogger.new - @request.host = "test.actioncontroller.i" - end - - def test_with_http_1_0_request - @request.host = nil - assert_nothing_raised { get :public_action } - end -end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 4ea2e57741..679eaf7b38 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -640,7 +640,7 @@ class FragmentCachingTest < ActionController::TestCase assert fragment_computed assert_equal 'generated till now -> ', buffer ActiveSupport::Notifications.notifier.wait - assert_equal [:fragment_exist?, :write_fragment], events.map(&:first) + assert_equal [:exist_fragment?, :write_fragment], events.map(&:first) end end diff --git a/actionpack/test/controller/filter_params_test.rb b/actionpack/test/controller/filter_params_test.rb index 43bef34885..420ebeacf4 100644 --- a/actionpack/test/controller/filter_params_test.rb +++ b/actionpack/test/controller/filter_params_test.rb @@ -70,9 +70,9 @@ class FilterParamTest < ActionController::TestCase FilterParamController.filter_parameter_logging(:lifo, :amount) get :payment, :lifo => 'Pratik', :amount => '420', :step => '1' + ActiveSupport::Notifications.notifier.wait filtered_params_logs = logs.detect {|l| l =~ /\AParameters/ } - assert filtered_params_logs.index('"amount"=>"[FILTERED]"') assert filtered_params_logs.index('"lifo"=>"[FILTERED]"') assert filtered_params_logs.index('"step"=>"1"') diff --git a/actionpack/test/controller/logging_test.rb b/actionpack/test/controller/logging_test.rb index 2b5e8d8bde..4206dffa7e 100644 --- a/actionpack/test/controller/logging_test.rb +++ b/actionpack/test/controller/logging_test.rb @@ -1,48 +1,74 @@ require 'abstract_unit' -class LoggingController < ActionController::Base - def show - render :nothing => true - end -end - -class LoggingTest < ActionController::TestCase - tests LoggingController - - class MockLogger - attr_reader :logged - attr_accessor :level +module Another + class LoggingController < ActionController::Base + layout "layouts/standard" - def initialize - @level = Logger::DEBUG + def show + render :nothing => true end - def method_missing(method, *args, &blk) - @logged ||= [] - @logged << args.first - @logged << blk.call if block_given? + def with_layout + render :template => "test/hello_world", :layout => true end end +end + +class LoggingTest < ActionController::TestCase + tests Another::LoggingController def setup super set_logger end + def get(*args) + super + wait + end + + def wait + ActiveSupport::Notifications.notifier.wait + end + def test_logging_without_parameters get :show - assert_equal 3, logs.size + assert_equal 4, logs.size assert_nil logs.detect {|l| l =~ /Parameters/ } end def test_logging_with_parameters get :show, :id => '10' - assert_equal 4, logs.size + assert_equal 5, logs.size params = logs.detect {|l| l =~ /Parameters/ } assert_equal 'Parameters: {"id"=>"10"}', params end + def test_log_controller_with_namespace_and_action + get :show + assert_match /Processed\sAnother::LoggingController#show/, logs[1] + end + + def test_log_view_runtime + get :show + assert_match /View runtime/, logs[2] + end + + def test_log_completed_status_and_request_uri + get :show + last = logs.last + assert_match /Completed/, last + assert_match /200/, last + assert_match /another\/logging\/show/, last + end + + def test_logger_prints_layout_and_template_rendering_info + get :with_layout + logged = logs.find {|l| l =~ /render/i } + assert_match /Rendered (.*)test\/hello_world.erb within (.*)layouts\/standard.html.erb/, logged + end + private def set_logger @controller.logger = MockLogger.new diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 54f2739d38..2c3dc2a72d 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -10,19 +10,6 @@ module Fun end end -class MockLogger - attr_reader :logged - - def initialize - @logged = [] - end - - def method_missing(method, *args, &blk) - @logged << args.first - @logged << blk.call if block_given? - end -end - class TestController < ActionController::Base protect_from_forgery @@ -1500,21 +1487,4 @@ class LastModifiedRenderTest < ActionController::TestCase get :conditional_hello_with_bangs assert_response :success end -end - -class RenderingLoggingTest < ActionController::TestCase - tests TestController - - def setup - super - @request.host = "www.nextangle.com" - end - - def test_logger_prints_layout_and_template_rendering_info - @controller.logger = MockLogger.new - get :layout_test - logged = @controller.logger.logged.find_all {|l| l =~ /render/i } - assert logged[0] =~ %r{Rendering.*test/hello_world} - assert logged[1] =~ %r{Rendering template within.*layouts/standard} - end -end +end
\ No newline at end of file diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 360ffe977b..c4b0b9cdbf 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -16,25 +16,27 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest Routes = routes Routes.draw do controller :sessions do - get 'login', :to => :new, :as => :login - post 'login', :to => :create + get 'login' => :new, :as => :login + post 'login' => :create - delete 'logout', :to => :destroy, :as => :logout + delete 'logout' => :destroy, :as => :logout end match 'account/logout' => redirect("/logout"), :as => :logout_redirect match 'account/login', :to => redirect("/login") + match 'account/overview' + match 'account/modulo/:name', :to => redirect("/%{name}s") match 'account/proc/:name', :to => redirect {|params| "/#{params[:name].pluralize}" } match 'openid/login', :via => [:get, :post], :to => "openid#login" controller(:global) do - match 'global/:action' + get 'global/hide_notice' match 'global/export', :to => :export, :as => :export_request - match 'global/hide_notice', :to => :hide_notice, :as => :hide_notice match '/export/:id/:file', :to => :export, :as => :export_download, :constraints => { :file => /.*/ } + match 'global/:action' end constraints(:ip => /192\.168\.1\.\d\d\d/) do @@ -221,7 +223,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal 'global#export', @response.body assert_equal '/global/export', export_request_path - assert_equal '/global/hide_notice', hide_notice_path + assert_equal '/global/hide_notice', global_hide_notice_path assert_equal '/export/123/foo.txt', export_download_path(:id => 123, :file => 'foo.txt') end end @@ -486,6 +488,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal 'projects#info', @response.body end end + + def test_convention_match_with_no_scope + with_test_routes do + assert_equal '/account/overview', account_overview_path + get '/account/overview' + assert_equal 'account#overview', @response.body + end + end private def with_test_routes diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb index e0de27b96d..f14016027c 100644 --- a/activemodel/lib/active_model.rb +++ b/activemodel/lib/active_model.rb @@ -30,21 +30,23 @@ module ActiveModel extend ActiveSupport::Autoload autoload :AttributeMethods + autoload :Callbacks autoload :Conversion autoload :DeprecatedErrorMethods autoload :Dirty autoload :Errors autoload :Lint - autoload :Name, 'active_model/naming' + autoload :Name, 'active_model/naming' autoload :Naming - autoload :Observer, 'active_model/observing' + autoload :Observer, 'active_model/observing' autoload :Observing autoload :Serialization autoload :StateMachine autoload :Translation autoload :Validations - autoload :ValidationsRepairHelper autoload :Validator + autoload :EachValidator, 'active_model/validator' + autoload :BlockValidator, 'active_model/validator' autoload :VERSION module Serializers @@ -55,4 +57,5 @@ module ActiveModel end end +require 'active_support/i18n' I18n.load_path << File.dirname(__FILE__) + '/active_model/locale/en.yml' diff --git a/activemodel/lib/active_model/callbacks.rb b/activemodel/lib/active_model/callbacks.rb new file mode 100644 index 0000000000..f66a1ddcaa --- /dev/null +++ b/activemodel/lib/active_model/callbacks.rb @@ -0,0 +1,91 @@ +require 'active_support/callbacks' + +module ActiveModel + module Callbacks + def self.extended(base) + base.class_eval do + include ActiveSupport::Callbacks + end + end + + # Define callbacks similar to ActiveRecord ones. It means: + # + # * The callback chain is aborted whenever the block given to + # _run_callbacks returns false. + # + # * If a class is given to the fallback, it will search for + # before_create, around_create and after_create methods. + # + # == Usage + # + # First you need to define which callbacks your model will have: + # + # class MyModel + # define_model_callbacks :create + # end + # + # This will define three class methods: before_create, around_create, + # and after_create. They accept a symbol, a string, an object or a block. + # + # After you create a callback, you need to tell when they are executed. + # For example, you could do: + # + # def create + # _run_create_callbacks do + # super + # end + # end + # + # == Options + # + # define_model_callbacks accepts all options define_callbacks does, in + # case you want to overwrite a default. Besides that, it also accepts + # an :only option, where you can choose if you want all types (before, + # around or after) or just some: + # + # define_model_callbacks :initializer, :only => :after + # + def define_model_callbacks(*callbacks) + options = callbacks.extract_options! + options = { :terminator => "result == false", :scope => [:kind, :name] }.merge(options) + + types = Array(options.delete(:only)) + types = [:before, :around, :after] if types.empty? + + callbacks.each do |callback| + define_callbacks(callback, options) + + types.each do |type| + send(:"_define_#{type}_model_callback", self, callback) + end + end + end + + def _define_before_model_callback(klass, callback) #:nodoc: + klass.class_eval <<-CALLBACK, __FILE__, __LINE__ + def self.before_#{callback}(*args, &block) + set_callback(:#{callback}, :before, *args, &block) + end + CALLBACK + end + + def _define_around_model_callback(klass, callback) #:nodoc: + klass.class_eval <<-CALLBACK, __FILE__, __LINE__ + def self.around_#{callback}(*args, &block) + set_callback(:#{callback}, :around, *args, &block) + end + CALLBACK + end + + def _define_after_model_callback(klass, callback) #:nodoc: + klass.class_eval <<-CALLBACK, __FILE__, __LINE__ + def self.after_#{callback}(*args, &block) + options = args.extract_options! + options[:prepend] = true + options[:if] = Array(options[:if]) << "!halted && value != false" + set_callback(:#{callback}, :after, *(args << options), &block) + end + CALLBACK + end + end +end
\ No newline at end of file diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index 675d62b9a6..4cd68a0c89 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -2,11 +2,11 @@ require 'active_support/inflector' module ActiveModel class Name < String - attr_reader :singular, :plural, :element, :collection, :partial_path, :human + attr_reader :singular, :plural, :element, :collection, :partial_path alias_method :cache_key, :collection - def initialize(klass, name) - super(name) + def initialize(klass) + super(klass.name) @klass = klass @singular = ActiveSupport::Inflector.underscore(self).tr('/', '_').freeze @plural = ActiveSupport::Inflector.pluralize(@singular).freeze @@ -15,13 +15,31 @@ module ActiveModel @collection = ActiveSupport::Inflector.tableize(self).freeze @partial_path = "#{@collection}/#{@element}".freeze end + + # Transform the model name into a more humane format, using I18n. By default, + # it will underscore then humanize the class name (BlogPost.model_name.human #=> "Blog post"). + # Specify +options+ with additional translating options. + def human(options={}) + return @human unless @klass.respond_to?(:lookup_ancestors) && + @klass.respond_to?(:i18n_scope) + + defaults = @klass.lookup_ancestors.map do |klass| + klass.model_name.underscore.to_sym + end + + defaults << options.delete(:default) if options[:default] + defaults << @human + + options.reverse_merge! :scope => [@klass.i18n_scope, :models], :count => 1, :default => defaults + I18n.translate(defaults.shift, options) + end end module Naming # Returns an ActiveModel::Name object for module. It can be # used to retrieve all kinds of naming-related information. def model_name - @_model_name ||= ActiveModel::Name.new(self, name) + @_model_name ||= ActiveModel::Name.new(self) end end end diff --git a/activemodel/lib/active_model/translation.rb b/activemodel/lib/active_model/translation.rb index 42ca463f82..e5ef1e6114 100644 --- a/activemodel/lib/active_model/translation.rb +++ b/activemodel/lib/active_model/translation.rb @@ -37,28 +37,8 @@ module ActiveModel # Model.human_name is deprecated. Use Model.model_name.human instead. def human_name(*args) - ActiveSupport::Deprecation.warn("human_name has been deprecated, please use model_name.human instead", caller[0,1]) + ActiveSupport::Deprecation.warn("human_name has been deprecated, please use model_name.human instead", caller[0,5]) model_name.human(*args) end end - - class Name < String - # Transform the model name into a more humane format, using I18n. By default, - # it will underscore then humanize the class name (BlogPost.human_name #=> "Blog post"). - # Specify +options+ with additional translating options. - def human(options={}) - return @human unless @klass.respond_to?(:lookup_ancestors) && - @klass.respond_to?(:i18n_scope) - - defaults = @klass.lookup_ancestors.map do |klass| - klass.model_name.underscore.to_sym - end - - defaults << options.delete(:default) if options[:default] - defaults << @human - - options.reverse_merge! :scope => [@klass.i18n_scope, :models], :count => 1, :default => defaults - I18n.translate(defaults.shift, options) - end - end end diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index a0d64507f9..d5460a58bd 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -13,6 +13,29 @@ module ActiveModel end module ClassMethods + # Validates each attribute against a block. + # + # class Person < ActiveRecord::Base + # validates_each :first_name, :last_name do |record, attr, value| + # record.errors.add attr, 'starts with z.' if value[0] == ?z + # end + # end + # + # Options: + # * <tt>:on</tt> - Specifies when this validation is active (default is <tt>:save</tt>, other options <tt>:create</tt>, <tt>:update</tt>). + # * <tt>:allow_nil</tt> - Skip validation if attribute is +nil+. + # * <tt>:allow_blank</tt> - Skip validation if attribute is blank. + # * <tt>:if</tt> - Specifies a method, proc or string to call to determine if the validation should + # occur (e.g. <tt>:if => :allow_validation</tt>, or <tt>:if => Proc.new { |user| user.signup_step > 2 }</tt>). The + # method, proc or string should return or evaluate to a true or false value. + # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine if the validation should + # not occur (e.g. <tt>:unless => :skip_validation</tt>, or <tt>:unless => Proc.new { |user| user.signup_step <= 2 }</tt>). The + # method, proc or string should return or evaluate to a true or false value. + def validates_each(*attr_names, &block) + options = attr_names.extract_options!.symbolize_keys + validates_with BlockValidator, options.merge(:attributes => attr_names.flatten), &block + end + # Adds a validation method or block to the class. This is useful when # overriding the +validate+ instance method becomes too unwieldly and # you're looking for more descriptive declaration of your validations. @@ -40,39 +63,6 @@ module ActiveModel # end # # This usage applies to +validate_on_create+ and +validate_on_update as well+. - - # Validates each attribute against a block. - # - # class Person < ActiveRecord::Base - # validates_each :first_name, :last_name do |record, attr, value| - # record.errors.add attr, 'starts with z.' if value[0] == ?z - # end - # end - # - # Options: - # * <tt>:on</tt> - Specifies when this validation is active (default is <tt>:save</tt>, other options <tt>:create</tt>, <tt>:update</tt>). - # * <tt>:allow_nil</tt> - Skip validation if attribute is +nil+. - # * <tt>:allow_blank</tt> - Skip validation if attribute is blank. - # * <tt>:if</tt> - Specifies a method, proc or string to call to determine if the validation should - # occur (e.g. <tt>:if => :allow_validation</tt>, or <tt>:if => Proc.new { |user| user.signup_step > 2 }</tt>). The - # method, proc or string should return or evaluate to a true or false value. - # * <tt>:unless</tt> - Specifies a method, proc or string to call to determine if the validation should - # not occur (e.g. <tt>:unless => :skip_validation</tt>, or <tt>:unless => Proc.new { |user| user.signup_step <= 2 }</tt>). The - # method, proc or string should return or evaluate to a true or false value. - def validates_each(*attrs) - options = attrs.extract_options!.symbolize_keys - attrs = attrs.flatten - - # Declare the validation. - validate options do |record| - attrs.each do |attr| - value = record.send(:read_attribute_for_validation, attr) - next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank]) - yield record, attr, value - end - end - end - def validate(*args, &block) options = args.last if options.is_a?(Hash) && options.key?(:on) diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb index b65c9b933d..bd9463ed27 100644 --- a/activemodel/lib/active_model/validations/acceptance.rb +++ b/activemodel/lib/active_model/validations/acceptance.rb @@ -1,5 +1,17 @@ module ActiveModel module Validations + class AcceptanceValidator < EachValidator + def initialize(options) + super(options.reverse_merge(:allow_nil => true, :accept => "1")) + end + + def validate_each(record, attribute, value) + unless value == options[:accept] + record.errors.add(attribute, :accepted, :default => options[:message]) + end + end + end + module ClassMethods # Encapsulates the pattern of wanting to validate the acceptance of a terms of service check box (or similar agreement). Example: # @@ -25,8 +37,7 @@ module ActiveModel # not occur (e.g. <tt>:unless => :skip_validation</tt>, or <tt>:unless => Proc.new { |user| user.signup_step <= 2 }</tt>). The # method, proc or string should return or evaluate to a true or false value. def validates_acceptance_of(*attr_names) - configuration = { :allow_nil => true, :accept => "1" } - configuration.update(attr_names.extract_options!) + options = attr_names.extract_options! db_cols = begin column_names @@ -37,11 +48,7 @@ module ActiveModel names = attr_names.reject { |name| db_cols.include?(name.to_s) } attr_accessor(*names) - validates_each(attr_names,configuration) do |record, attr_name, value| - unless value == configuration[:accept] - record.errors.add(attr_name, :accepted, :default => configuration[:message]) - end - end + validates_with AcceptanceValidator, options.merge(:attributes => attr_names) end end end diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb index d414224dd2..b06effdceb 100644 --- a/activemodel/lib/active_model/validations/confirmation.rb +++ b/activemodel/lib/active_model/validations/confirmation.rb @@ -1,5 +1,13 @@ module ActiveModel module Validations + class ConfirmationValidator < EachValidator + def validate_each(record, attribute, value) + confirmed = record.send(:"#{attribute}_confirmation") + return if confirmed.nil? || value == confirmed + record.errors.add(attribute, :confirmation, :default => options[:message]) + end + end + module ClassMethods # Encapsulates the pattern of wanting to validate a password or email address field with a confirmation. Example: # @@ -30,15 +38,9 @@ module ActiveModel # not occur (e.g. <tt>:unless => :skip_validation</tt>, or <tt>:unless => Proc.new { |user| user.signup_step <= 2 }</tt>). The # method, proc or string should return or evaluate to a true or false value. def validates_confirmation_of(*attr_names) - configuration = attr_names.extract_options! - - attr_accessor(*(attr_names.map { |n| "#{n}_confirmation" })) - - validates_each(attr_names, configuration) do |record, attr_name, value| - unless record.send("#{attr_name}_confirmation").nil? or value == record.send("#{attr_name}_confirmation") - record.errors.add(attr_name, :confirmation, :default => configuration[:message]) - end - end + options = attr_names.extract_options! + attr_accessor(*(attr_names.map { |n| :"#{n}_confirmation" })) + validates_with ConfirmationValidator, options.merge(:attributes => attr_names) end end end diff --git a/activemodel/lib/active_model/validations/exclusion.rb b/activemodel/lib/active_model/validations/exclusion.rb index 2cfdec97a5..f8759f253b 100644 --- a/activemodel/lib/active_model/validations/exclusion.rb +++ b/activemodel/lib/active_model/validations/exclusion.rb @@ -1,5 +1,17 @@ module ActiveModel module Validations + class ExclusionValidator < EachValidator + def check_validity! + raise ArgumentError, "An object with the method include? is required must be supplied as the " << + ":in option of the configuration hash" unless options[:in].respond_to?(:include?) + end + + def validate_each(record, attribute, value) + return unless options[:in].include?(value) + record.errors.add(attribute, :exclusion, :default => options[:message], :value => value) + end + end + module ClassMethods # Validates that the value of the specified attribute is not in a particular enumerable object. # @@ -21,17 +33,9 @@ module ActiveModel # not occur (e.g. <tt>:unless => :skip_validation</tt>, or <tt>:unless => Proc.new { |user| user.signup_step <= 2 }</tt>). The # method, proc or string should return or evaluate to a true or false value. def validates_exclusion_of(*attr_names) - configuration = attr_names.extract_options! - - enum = configuration[:in] || configuration[:within] - - raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?(:include?) - - validates_each(attr_names, configuration) do |record, attr_name, value| - if enum.include?(value) - record.errors.add(attr_name, :exclusion, :default => configuration[:message], :value => value) - end - end + options = attr_names.extract_options! + options[:in] ||= options.delete(:within) + validates_with ExclusionValidator, options.merge(:attributes => attr_names) end end end diff --git a/activemodel/lib/active_model/validations/format.rb b/activemodel/lib/active_model/validations/format.rb index c670dafc7c..d5427c2b03 100644 --- a/activemodel/lib/active_model/validations/format.rb +++ b/activemodel/lib/active_model/validations/format.rb @@ -1,5 +1,15 @@ module ActiveModel module Validations + class FormatValidator < EachValidator + def validate_each(record, attribute, value) + if options[:with] && value.to_s !~ options[:with] + record.errors.add(attribute, :invalid, :default => options[:message], :value => value) + elsif options[:without] && value.to_s =~ options[:without] + record.errors.add(attribute, :invalid, :default => options[:message], :value => value) + end + end + end + module ClassMethods # Validates whether the value of the specified attribute is of the correct form, going by the regular expression provided. # You can require that the attribute matches the regular expression: @@ -33,29 +43,21 @@ module ActiveModel # not occur (e.g. <tt>:unless => :skip_validation</tt>, or <tt>:unless => Proc.new { |user| user.signup_step <= 2 }</tt>). The # method, proc or string should return or evaluate to a true or false value. def validates_format_of(*attr_names) - configuration = attr_names.extract_options! + options = attr_names.extract_options! - unless configuration.include?(:with) ^ configuration.include?(:without) # ^ == xor, or "exclusive or" + unless options.include?(:with) ^ options.include?(:without) # ^ == xor, or "exclusive or" raise ArgumentError, "Either :with or :without must be supplied (but not both)" end - if configuration[:with] && !configuration[:with].is_a?(Regexp) + if options[:with] && !options[:with].is_a?(Regexp) raise ArgumentError, "A regular expression must be supplied as the :with option of the configuration hash" end - if configuration[:without] && !configuration[:without].is_a?(Regexp) + if options[:without] && !options[:without].is_a?(Regexp) raise ArgumentError, "A regular expression must be supplied as the :without option of the configuration hash" end - if configuration[:with] - validates_each(attr_names, configuration) do |record, attr_name, value| - record.errors.add(attr_name, :invalid, :default => configuration[:message], :value => value) if value.to_s !~ configuration[:with] - end - elsif configuration[:without] - validates_each(attr_names, configuration) do |record, attr_name, value| - record.errors.add(attr_name, :invalid, :default => configuration[:message], :value => value) if value.to_s =~ configuration[:without] - end - end + validates_with FormatValidator, options.merge(:attributes => attr_names) end end end diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb index 0d7dc5cd64..a122e9e737 100644 --- a/activemodel/lib/active_model/validations/inclusion.rb +++ b/activemodel/lib/active_model/validations/inclusion.rb @@ -1,5 +1,17 @@ module ActiveModel module Validations + class InclusionValidator < EachValidator + def check_validity! + raise ArgumentError, "An object with the method include? is required must be supplied as the " << + ":in option of the configuration hash" unless options[:in].respond_to?(:include?) + end + + def validate_each(record, attribute, value) + return if options[:in].include?(value) + record.errors.add(attribute, :inclusion, :default => options[:message], :value => value) + end + end + module ClassMethods # Validates whether the value of the specified attribute is available in a particular enumerable object. # @@ -21,17 +33,9 @@ module ActiveModel # not occur (e.g. <tt>:unless => :skip_validation</tt>, or <tt>:unless => Proc.new { |user| user.signup_step <= 2 }</tt>). The # method, proc or string should return or evaluate to a true or false value. def validates_inclusion_of(*attr_names) - configuration = attr_names.extract_options! - - enum = configuration[:in] || configuration[:within] - - raise(ArgumentError, "An object with the method include? is required must be supplied as the :in option of the configuration hash") unless enum.respond_to?(:include?) - - validates_each(attr_names, configuration) do |record, attr_name, value| - unless enum.include?(value) - record.errors.add(attr_name, :inclusion, :default => configuration[:message], :value => value) - end - end + options = attr_names.extract_options! + options[:in] ||= options.delete(:within) + validates_with InclusionValidator, options.merge(:attributes => attr_names) end end end diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index e91841bd1c..6e90a75c17 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -1,7 +1,75 @@ module ActiveModel module Validations + class LengthValidator < EachValidator + OPTIONS = [ :is, :within, :in, :minimum, :maximum ].freeze + MESSAGES = { :is => :wrong_length, :minimum => :too_short, :maximum => :too_long }.freeze + CHECKS = { :is => :==, :minimum => :>=, :maximum => :<= }.freeze + + DEFAULT_TOKENIZER = lambda { |value| value.split(//) } + attr_reader :type + + def initialize(options) + @type = (OPTIONS & options.keys).first + super(options.reverse_merge(:tokenizer => DEFAULT_TOKENIZER)) + end + + def check_validity! + ensure_one_range_option! + ensure_argument_types! + end + + def validate_each(record, attribute, value) + checks = options.slice(:minimum, :maximum, :is) + value = options[:tokenizer].call(value) if value.kind_of?(String) + + if [:within, :in].include?(type) + range = options[type] + checks[:minimum], checks[:maximum] = range.begin, range.end + checks[:maximum] -= 1 if range.exclude_end? + end + + checks.each do |key, check_value| + custom_message = options[:message] || options[MESSAGES[key]] + validity_check = CHECKS[key] + + valid_value = if key == :maximum + value.nil? || value.size.send(validity_check, check_value) + else + value && value.size.send(validity_check, check_value) + end + + record.errors.add(attribute, MESSAGES[key], :default => custom_message, :count => check_value) unless valid_value + end + end + + protected + + def ensure_one_range_option! #:nodoc: + range_options = OPTIONS & options.keys + + case range_options.size + when 0 + raise ArgumentError, 'Range unspecified. Specify the :within, :maximum, :minimum, or :is option.' + when 1 + # Valid number of options; do nothing. + else + raise ArgumentError, 'Too many range options specified. Choose only one.' + end + end + + def ensure_argument_types! #:nodoc: + value = options[type] + + case type + when :within, :in + raise ArgumentError, ":#{type} must be a Range" unless value.is_a?(Range) + when :is, :minimum, :maximum + raise ArgumentError, ":#{type} must be a nonnegative Integer" unless value.is_a?(Integer) && value >= 0 + end + end + end + module ClassMethods - ALL_RANGE_OPTIONS = [ :is, :within, :in, :minimum, :maximum ].freeze # Validates that the specified attribute matches the length restrictions supplied. Only one option can be used at a time: # @@ -38,62 +106,9 @@ module ActiveModel # * <tt>:tokenizer</tt> - Specifies how to split up the attribute string. (e.g. <tt>:tokenizer => lambda {|str| str.scan(/\w+/)}</tt> to # count words as in above example.) # Defaults to <tt>lambda{ |value| value.split(//) }</tt> which counts individual characters. - def validates_length_of(*attrs) - # Merge given options with defaults. - options = { :tokenizer => lambda {|value| value.split(//)} } - options.update(attrs.extract_options!.symbolize_keys) - - # Ensure that one and only one range option is specified. - range_options = ALL_RANGE_OPTIONS & options.keys - case range_options.size - when 0 - raise ArgumentError, 'Range unspecified. Specify the :within, :maximum, :minimum, or :is option.' - when 1 - # Valid number of options; do nothing. - else - raise ArgumentError, 'Too many range options specified. Choose only one.' - end - - # Get range option and value. - option = range_options.first - option_value = options[range_options.first] - key = {:is => :wrong_length, :minimum => :too_short, :maximum => :too_long}[option] - custom_message = options[:message] || options[key] - - case option - when :within, :in - raise ArgumentError, ":#{option} must be a Range" unless option_value.is_a?(Range) - - validates_each(attrs, options) do |record, attr, value| - value = options[:tokenizer].call(value) if value.kind_of?(String) - - min, max = option_value.begin, option_value.end - max = max - 1 if option_value.exclude_end? - - if value.nil? || value.size < min - record.errors.add(attr, :too_short, :default => custom_message || options[:too_short], :count => min) - elsif value.size > max - record.errors.add(attr, :too_long, :default => custom_message || options[:too_long], :count => max) - end - end - when :is, :minimum, :maximum - raise ArgumentError, ":#{option} must be a nonnegative Integer" unless option_value.is_a?(Integer) and option_value >= 0 - - # Declare different validations per option. - validity_checks = { :is => "==", :minimum => ">=", :maximum => "<=" } - - validates_each(attrs, options) do |record, attr, value| - value = options[:tokenizer].call(value) if value.kind_of?(String) - - valid_value = if option == :maximum - value.nil? || value.size.send(validity_checks[option], option_value) - else - value && value.size.send(validity_checks[option], option_value) - end - - record.errors.add(attr, key, :default => custom_message, :count => option_value) unless valid_value - end - end + def validates_length_of(*attr_names) + options = attr_names.extract_options! + validates_with LengthValidator, options.merge(:attributes => attr_names) end alias_method :validates_size_of, :validates_length_of diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 32dbcd82d0..f2aab8c5b8 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -1,10 +1,68 @@ module ActiveModel module Validations - module ClassMethods - ALL_NUMERICALITY_CHECKS = { :greater_than => '>', :greater_than_or_equal_to => '>=', - :equal_to => '==', :less_than => '<', :less_than_or_equal_to => '<=', - :odd => 'odd?', :even => 'even?' }.freeze + class NumericalityValidator < EachValidator + CHECKS = { :greater_than => :>, :greater_than_or_equal_to => :>=, + :equal_to => :==, :less_than => :<, :less_than_or_equal_to => :<=, + :odd => :odd?, :even => :even? }.freeze + + def initialize(options) + super(options.reverse_merge(:only_integer => false, :allow_nil => false)) + end + + def check_validity! + options.slice(*CHECKS.keys) do |option, value| + next if [:odd, :even].include?(option) + raise ArgumentError, ":#{option} must be a number, a symbol or a proc" unless value.is_a?(Numeric) || value.is_a?(Proc) || value.is_a?(Symbol) + end + end + + def validate_each(record, attr_name, value) + before_type_cast = "#{attr_name}_before_type_cast" + + raw_value = record.send("#{attr_name}_before_type_cast") if record.respond_to?(before_type_cast.to_sym) + raw_value ||= value + + return if options[:allow_nil] && raw_value.nil? + + unless value = parse_raw_value(raw_value, options) + record.errors.add(attr_name, :not_a_number, :value => raw_value, :default => options[:message]) + return + end + + options.slice(*CHECKS.keys).each do |option, option_value| + case option + when :odd, :even + unless value.to_i.send(CHECKS[option]) + record.errors.add(attr_name, option, :value => value, :default => options[:message]) + end + else + option_value = option_value.call(record) if option_value.is_a?(Proc) + option_value = record.send(option_value) if option_value.is_a?(Symbol) + + unless value.send(CHECKS[option], option_value) + record.errors.add(attr_name, option, :default => options[:message], :value => value, :count => option_value) + end + end + end + end + + protected + + def parse_raw_value(raw_value, options) + if options[:only_integer] + raw_value.to_i if raw_value.to_s =~ /\A[+-]?\d+\Z/ + else + begin + Kernel.Float(raw_value) + rescue ArgumentError, TypeError + nil + end + end + end + end + + module ClassMethods # Validates whether the value of the specified attribute is numeric by trying to convert it to # a float with Kernel.Float (if <tt>only_integer</tt> is false) or applying it to the regular expression # <tt>/\A[\+\-]?\d+\Z/</tt> (if <tt>only_integer</tt> is set to true). @@ -44,61 +102,9 @@ module ActiveModel # validates_numericality_of :width, :greater_than => :minimum_weight # end # - # - def validates_numericality_of(*attr_names) - configuration = { :only_integer => false, :allow_nil => false } - configuration.update(attr_names.extract_options!) - - numericality_options = ALL_NUMERICALITY_CHECKS.keys & configuration.keys - - (numericality_options - [ :odd, :even ]).each do |option| - value = configuration[option] - raise ArgumentError, ":#{option} must be a number, a symbol or a proc" unless value.is_a?(Numeric) || value.is_a?(Proc) || value.is_a?(Symbol) - end - - validates_each(attr_names,configuration) do |record, attr_name, value| - before_type_cast = "#{attr_name}_before_type_cast" - - if record.respond_to?(before_type_cast.to_sym) - raw_value = record.send("#{attr_name}_before_type_cast") || value - else - raw_value = value - end - - next if configuration[:allow_nil] and raw_value.nil? - - if configuration[:only_integer] - unless raw_value.to_s =~ /\A[+-]?\d+\Z/ - record.errors.add(attr_name, :not_a_number, :value => raw_value, :default => configuration[:message]) - next - end - raw_value = raw_value.to_i - else - begin - raw_value = Kernel.Float(raw_value) - rescue ArgumentError, TypeError - record.errors.add(attr_name, :not_a_number, :value => raw_value, :default => configuration[:message]) - next - end - end - - numericality_options.each do |option| - case option - when :odd, :even - unless raw_value.to_i.method(ALL_NUMERICALITY_CHECKS[option])[] - record.errors.add(attr_name, option, :value => raw_value, :default => configuration[:message]) - end - else - configuration[option] = configuration[option].call(record) if configuration[option].is_a? Proc - configuration[option] = record.method(configuration[option]).call if configuration[option].is_a? Symbol - - unless raw_value.method(ALL_NUMERICALITY_CHECKS[option])[configuration[option]] - record.errors.add(attr_name, option, :default => configuration[:message], :value => raw_value, :count => configuration[option]) - end - end - end - end + options = attr_names.extract_options! + validates_with NumericalityValidator, options.merge(:attributes => attr_names) end end end diff --git a/activemodel/lib/active_model/validations/presence.rb b/activemodel/lib/active_model/validations/presence.rb index 3ff677c137..a4c6f866a7 100644 --- a/activemodel/lib/active_model/validations/presence.rb +++ b/activemodel/lib/active_model/validations/presence.rb @@ -2,6 +2,12 @@ require 'active_support/core_ext/object/blank' module ActiveModel module Validations + class PresenceValidator < EachValidator + def validate(record) + record.errors.add_on_blank(attributes, options[:message]) + end + end + module ClassMethods # Validates that the specified attributes are not blank (as defined by Object#blank?). Happens by default on save. Example: # @@ -28,13 +34,8 @@ module ActiveModel # The method, proc or string should return or evaluate to a true or false value. # def validates_presence_of(*attr_names) - configuration = attr_names.extract_options! - - # can't use validates_each here, because it cannot cope with nonexistent attributes, - # while errors.add_on_empty can - validate configuration do |record| - record.errors.add_on_blank(attr_names, configuration[:message]) - end + options = attr_names.extract_options! + validates_with PresenceValidator, options.merge(:attributes => attr_names) end end end diff --git a/activemodel/lib/active_model/validations/with.rb b/activemodel/lib/active_model/validations/with.rb index edc2133ddc..8d521173c6 100644 --- a/activemodel/lib/active_model/validations/with.rb +++ b/activemodel/lib/active_model/validations/with.rb @@ -48,14 +48,9 @@ module ActiveModel # end # end # - def validates_with(*args) - configuration = args.extract_options! - - validate configuration do |record| - args.each do |klass| - klass.new(record, configuration.except(:on, :if, :unless)).validate - end - end + def validates_with(*args, &block) + options = args.extract_options! + args.each { |klass| validate(klass.new(options, &block), options) } end end end diff --git a/activemodel/lib/active_model/validator.rb b/activemodel/lib/active_model/validator.rb index 09de72b757..8c9f9c7fb3 100644 --- a/activemodel/lib/active_model/validator.rb +++ b/activemodel/lib/active_model/validator.rb @@ -1,5 +1,4 @@ module ActiveModel #:nodoc: - # A simple base class that can be used along with ActiveModel::Base.validates_with # # class Person < ActiveModel::Base @@ -52,17 +51,59 @@ module ActiveModel #:nodoc: # @my_custom_field = options[:field_name] || :first_name # end # end - # class Validator - attr_reader :record, :options + attr_reader :options - def initialize(record, options) - @record = record + def initialize(options) @options = options end - def validate - raise "You must override this method" + def validate(record) + raise NotImplementedError + end + end + + # EachValidator is a validator which iterates through the attributes given + # in the options hash invoking the validate_each method passing in the + # record, attribute and value. + # + # All ActiveModel validations are built on top of this Validator. + class EachValidator < Validator + attr_reader :attributes + + def initialize(options) + @attributes = options.delete(:attributes) + super + check_validity! + end + + def validate(record) + attributes.each do |attribute| + value = record.send(:read_attribute_for_validation, attribute) + next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank]) + validate_each(record, attribute, value) + end + end + + def validate_each(record, attribute, value) + raise NotImplementedError + end + + def check_validity! + end + end + + # BlockValidator is a special EachValidator which receives a block on initialization + # and call this block for each attribute being validated. +validates_each+ uses this + # Validator. + class BlockValidator < EachValidator + def initialize(options, &block) + @block = block + super + end + + def validate_each(record, attribute, value) + @block.call(record, attribute, value) end end end diff --git a/activemodel/test/cases/callbacks_test.rb b/activemodel/test/cases/callbacks_test.rb new file mode 100644 index 0000000000..b996f51d6b --- /dev/null +++ b/activemodel/test/cases/callbacks_test.rb @@ -0,0 +1,70 @@ +require "cases/helper" + +class CallbacksTest < ActiveModel::TestCase + + class CallbackValidator + def around_create(model) + model.callbacks << :before_around_create + yield + model.callbacks << :after_around_create + end + end + + class ModelCallbacks + attr_reader :callbacks + extend ActiveModel::Callbacks + + define_model_callbacks :create + define_model_callbacks :initialize, :only => :after + + before_create :before_create + around_create CallbackValidator.new + + after_create do |model| + model.callbacks << :after_create + end + + after_create "@callbacks << :final_callback" + + def initialize(valid=true) + @callbacks, @valid = [], valid + end + + def before_create + @callbacks << :before_create + end + + def create + _run_create_callbacks do + @callbacks << :create + @valid + end + end + end + + test "complete callback chain" do + model = ModelCallbacks.new + model.create + assert_equal model.callbacks, [ :before_create, :before_around_create, :create, + :after_around_create, :after_create, :final_callback] + end + + test "after callbacks are always appended" do + model = ModelCallbacks.new + model.create + assert_equal model.callbacks.last, :final_callback + end + + test "after callbacks are not executed if the block returns false" do + model = ModelCallbacks.new(false) + model.create + assert_equal model.callbacks, [ :before_create, :before_around_create, + :create, :after_around_create] + end + + test "only selects which types of callbacks should be created" do + assert !ModelCallbacks.respond_to?(:before_initialize) + assert !ModelCallbacks.respond_to?(:around_initialize) + assert ModelCallbacks.respond_to?(:after_initialize) + end +end diff --git a/activemodel/test/cases/naming_test.rb b/activemodel/test/cases/naming_test.rb index fe1ea36450..dc39b84ed8 100644 --- a/activemodel/test/cases/naming_test.rb +++ b/activemodel/test/cases/naming_test.rb @@ -1,8 +1,9 @@ require 'cases/helper' +require 'models/track_back' class NamingTest < ActiveModel::TestCase def setup - @model_name = ActiveModel::Name.new(self, 'Post::TrackBack') + @model_name = ActiveModel::Name.new(Post::TrackBack) end def test_singular diff --git a/activemodel/test/cases/translation_test.rb b/activemodel/test/cases/translation_test.rb index d171784963..bfc1ca12e6 100644 --- a/activemodel/test/cases/translation_test.rb +++ b/activemodel/test/cases/translation_test.rb @@ -1,11 +1,5 @@ require 'cases/helper' - -class SuperUser - extend ActiveModel::Translation -end - -class User < SuperUser -end +require 'models/person' class ActiveModelI18nTests < ActiveModel::TestCase @@ -14,38 +8,38 @@ class ActiveModelI18nTests < ActiveModel::TestCase end def test_translated_model_attributes - I18n.backend.store_translations 'en', :activemodel => {:attributes => {:super_user => {:name => 'super_user name attribute'} } } - assert_equal 'super_user name attribute', SuperUser.human_attribute_name('name') + I18n.backend.store_translations 'en', :activemodel => {:attributes => {:person => {:name => 'person name attribute'} } } + assert_equal 'person name attribute', Person.human_attribute_name('name') end def test_translated_model_attributes_with_symbols - I18n.backend.store_translations 'en', :activemodel => {:attributes => {:super_user => {:name => 'super_user name attribute'} } } - assert_equal 'super_user name attribute', SuperUser.human_attribute_name(:name) + I18n.backend.store_translations 'en', :activemodel => {:attributes => {:person => {:name => 'person name attribute'} } } + assert_equal 'person name attribute', Person.human_attribute_name(:name) end def test_translated_model_attributes_with_ancestor - I18n.backend.store_translations 'en', :activemodel => {:attributes => {:user => {:name => 'user name attribute'} } } - assert_equal 'user name attribute', User.human_attribute_name('name') + I18n.backend.store_translations 'en', :activemodel => {:attributes => {:child => {:name => 'child name attribute'} } } + assert_equal 'child name attribute', Child.human_attribute_name('name') end def test_translated_model_attributes_with_ancestors_fallback - I18n.backend.store_translations 'en', :activemodel => {:attributes => {:super_user => {:name => 'super_user name attribute'} } } - assert_equal 'super_user name attribute', User.human_attribute_name('name') + I18n.backend.store_translations 'en', :activemodel => {:attributes => {:person => {:name => 'person name attribute'} } } + assert_equal 'person name attribute', Child.human_attribute_name('name') end def test_translated_model_names - I18n.backend.store_translations 'en', :activemodel => {:models => {:super_user => 'super_user model'} } - assert_equal 'super_user model', SuperUser.model_name.human + I18n.backend.store_translations 'en', :activemodel => {:models => {:person => 'person model'} } + assert_equal 'person model', Person.model_name.human end def test_translated_model_names_with_sti - I18n.backend.store_translations 'en', :activemodel => {:models => {:user => 'user model'} } - assert_equal 'user model', User.model_name.human + I18n.backend.store_translations 'en', :activemodel => {:models => {:child => 'child model'} } + assert_equal 'child model', Child.model_name.human end def test_translated_model_names_with_ancestors_fallback - I18n.backend.store_translations 'en', :activemodel => {:models => {:super_user => 'super_user model'} } - assert_equal 'super_user model', User.model_name.human + I18n.backend.store_translations 'en', :activemodel => {:models => {:person => 'person model'} } + assert_equal 'person model', Child.model_name.human end end diff --git a/activemodel/test/cases/validations/acceptance_validation_test.rb b/activemodel/test/cases/validations/acceptance_validation_test.rb index 88e5fdb358..11c9c1edfd 100644 --- a/activemodel/test/cases/validations/acceptance_validation_test.rb +++ b/activemodel/test/cases/validations/acceptance_validation_test.rb @@ -9,9 +9,10 @@ require 'models/person' class AcceptanceValidationTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end def test_terms_of_service_agreement_no_acceptance Topic.validates_acceptance_of(:terms_of_service, :on => :create) @@ -53,28 +54,18 @@ class AcceptanceValidationTest < ActiveModel::TestCase assert t.save end - def test_validates_acceptance_of_with_custom_error_using_quotes - repair_validations(Developer) do - Developer.validates_acceptance_of :salary, :message=> "This string contains 'single' and \"double\" quotes" - d = Developer.new - d.salary = "0" - assert !d.valid? - assert_equal "This string contains 'single' and \"double\" quotes", d.errors[:salary].last - end - end - def test_validates_acceptance_of_for_ruby_class - repair_validations(Person) do - Person.validates_acceptance_of :karma + Person.validates_acceptance_of :karma - p = Person.new - p.karma = "" + p = Person.new + p.karma = "" - assert p.invalid? - assert_equal ["must be accepted"], p.errors[:karma] + assert p.invalid? + assert_equal ["must be accepted"], p.errors[:karma] - p.karma = "1" - assert p.valid? - end + p.karma = "1" + assert p.valid? + ensure + Person.reset_callbacks(:validate) end end diff --git a/activemodel/test/cases/validations/conditional_validation_test.rb b/activemodel/test/cases/validations/conditional_validation_test.rb index 4c716d5d48..5260162a58 100644 --- a/activemodel/test/cases/validations/conditional_validation_test.rb +++ b/activemodel/test/cases/validations/conditional_validation_test.rb @@ -6,9 +6,10 @@ require 'models/topic' class ConditionalValidationTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end def test_if_validation_using_method_true # When the method returns true diff --git a/activemodel/test/cases/validations/confirmation_validation_test.rb b/activemodel/test/cases/validations/confirmation_validation_test.rb index 1d6f2a6ec5..55554d5054 100644 --- a/activemodel/test/cases/validations/confirmation_validation_test.rb +++ b/activemodel/test/cases/validations/confirmation_validation_test.rb @@ -8,9 +8,10 @@ require 'models/person' class ConfirmationValidationTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end def test_no_title_confirmation Topic.validates_confirmation_of(:title) @@ -39,30 +40,19 @@ class ConfirmationValidationTest < ActiveModel::TestCase assert t.save end - def test_validates_confirmation_of_with_custom_error_using_quotes - repair_validations(Developer) do - Developer.validates_confirmation_of :name, :message=> "confirm 'single' and \"double\" quotes" - d = Developer.new - d.name = "John" - d.name_confirmation = "Johnny" - assert !d.valid? - assert_equal ["confirm 'single' and \"double\" quotes"], d.errors[:name] - end - end - def test_validates_confirmation_of_for_ruby_class - repair_validations(Person) do - Person.validates_confirmation_of :karma + Person.validates_confirmation_of :karma - p = Person.new - p.karma_confirmation = "None" - assert p.invalid? + p = Person.new + p.karma_confirmation = "None" + assert p.invalid? - assert_equal ["doesn't match confirmation"], p.errors[:karma] + assert_equal ["doesn't match confirmation"], p.errors[:karma] - p.karma = "None" - assert p.valid? - end + p.karma = "None" + assert p.valid? + ensure + Person.reset_callbacks(:validate) end end diff --git a/activemodel/test/cases/validations/exclusion_validation_test.rb b/activemodel/test/cases/validations/exclusion_validation_test.rb index 584f009e84..7d851f546c 100644 --- a/activemodel/test/cases/validations/exclusion_validation_test.rb +++ b/activemodel/test/cases/validations/exclusion_validation_test.rb @@ -7,9 +7,10 @@ require 'models/person' class ExclusionValidationTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end def test_validates_exclusion_of Topic.validates_exclusion_of( :title, :in => %w( abe monkey ) ) @@ -30,17 +31,17 @@ class ExclusionValidationTest < ActiveModel::TestCase end def test_validates_exclusion_of_for_ruby_class - repair_validations(Person) do - Person.validates_exclusion_of :karma, :in => %w( abe monkey ) + Person.validates_exclusion_of :karma, :in => %w( abe monkey ) - p = Person.new - p.karma = "abe" - assert p.invalid? + p = Person.new + p.karma = "abe" + assert p.invalid? - assert_equal ["is reserved"], p.errors[:karma] + assert_equal ["is reserved"], p.errors[:karma] - p.karma = "Lifo" - assert p.valid? - end + p.karma = "Lifo" + assert p.valid? + ensure + Person.reset_callbacks(:validate) end end diff --git a/activemodel/test/cases/validations/format_validation_test.rb b/activemodel/test/cases/validations/format_validation_test.rb index e19e4bf7b3..e10089208a 100644 --- a/activemodel/test/cases/validations/format_validation_test.rb +++ b/activemodel/test/cases/validations/format_validation_test.rb @@ -8,9 +8,10 @@ require 'models/person' class PresenceValidationTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end def test_validate_format Topic.validates_format_of(:title, :content, :with => /^Validation\smacros \w+!$/, :message => "is bad data") @@ -100,28 +101,18 @@ class PresenceValidationTest < ActiveModel::TestCase assert_raise(ArgumentError) { Topic.validates_format_of(:title, :without => "clearly not a regexp") } end - def test_validates_format_of_with_custom_error_using_quotes - repair_validations(Developer) do - Developer.validates_format_of :name, :with => /^(A-Z*)$/, :message=> "format 'single' and \"double\" quotes" - d = Developer.new - d.name = d.name_confirmation = "John 32" - assert !d.valid? - assert_equal ["format 'single' and \"double\" quotes"], d.errors[:name] - end - end - def test_validates_format_of_for_ruby_class - repair_validations(Person) do - Person.validates_format_of :karma, :with => /\A\d+\Z/ + Person.validates_format_of :karma, :with => /\A\d+\Z/ - p = Person.new - p.karma = "Pixies" - assert p.invalid? + p = Person.new + p.karma = "Pixies" + assert p.invalid? - assert_equal ["is invalid"], p.errors[:karma] + assert_equal ["is invalid"], p.errors[:karma] - p.karma = "1234" - assert p.valid? - end + p.karma = "1234" + assert p.valid? + ensure + Person.reset_callbacks(:validate) end end diff --git a/activemodel/test/cases/validations/i18n_validation_test.rb b/activemodel/test/cases/validations/i18n_validation_test.rb index 68b1b27f75..2717a09331 100644 --- a/activemodel/test/cases/validations/i18n_validation_test.rb +++ b/activemodel/test/cases/validations/i18n_validation_test.rb @@ -1,6 +1,5 @@ require "cases/helper" require 'cases/tests_database' - require 'models/person' class I18nValidationTest < ActiveModel::TestCase diff --git a/activemodel/test/cases/validations/inclusion_validation_test.rb b/activemodel/test/cases/validations/inclusion_validation_test.rb index bc1b0365d2..6b2bcd9c60 100644 --- a/activemodel/test/cases/validations/inclusion_validation_test.rb +++ b/activemodel/test/cases/validations/inclusion_validation_test.rb @@ -8,9 +8,10 @@ require 'models/person' class InclusionValidationTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end def test_validates_inclusion_of Topic.validates_inclusion_of( :title, :in => %w( a b c d e f g ) ) @@ -53,28 +54,18 @@ class InclusionValidationTest < ActiveModel::TestCase assert_equal ["option uhoh is not in the list"], t.errors[:title] end - def test_validates_inclusion_of_with_custom_error_using_quotes - repair_validations(Developer) do - Developer.validates_inclusion_of :salary, :in => 1000..80000, :message=> "This string contains 'single' and \"double\" quotes" - d = Developer.new - d.salary = "90,000" - assert !d.valid? - assert_equal "This string contains 'single' and \"double\" quotes", d.errors[:salary].last - end - end - def test_validates_inclusion_of_for_ruby_class - repair_validations(Person) do - Person.validates_inclusion_of :karma, :in => %w( abe monkey ) + Person.validates_inclusion_of :karma, :in => %w( abe monkey ) - p = Person.new - p.karma = "Lifo" - assert p.invalid? + p = Person.new + p.karma = "Lifo" + assert p.invalid? - assert_equal ["is not included in the list"], p.errors[:karma] + assert_equal ["is not included in the list"], p.errors[:karma] - p.karma = "monkey" - assert p.valid? - end + p.karma = "monkey" + assert p.valid? + ensure + Person.reset_callbacks(:validate) end end diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb index 2c97b762f1..f3ef5e648a 100644 --- a/activemodel/test/cases/validations/length_validation_test.rb +++ b/activemodel/test/cases/validations/length_validation_test.rb @@ -8,9 +8,10 @@ require 'models/person' class LengthValidationTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end def test_validates_length_of_with_allow_nil Topic.validates_length_of( :title, :is => 5, :allow_nil=>true ) @@ -419,48 +420,18 @@ class LengthValidationTest < ActiveModel::TestCase assert_equal ["Your essay must be at least 5 words."], t.errors[:content] end - def test_validates_length_of_with_custom_too_long_using_quotes - repair_validations(Developer) do - Developer.validates_length_of :name, :maximum => 4, :too_long=> "This string contains 'single' and \"double\" quotes" - d = Developer.new - d.name = "Jeffrey" - assert !d.valid? - assert_equal ["This string contains 'single' and \"double\" quotes"], d.errors[:name] - end - end - - def test_validates_length_of_with_custom_too_short_using_quotes - repair_validations(Developer) do - Developer.validates_length_of :name, :minimum => 4, :too_short=> "This string contains 'single' and \"double\" quotes" - d = Developer.new - d.name = "Joe" - assert !d.valid? - assert_equal ["This string contains 'single' and \"double\" quotes"], d.errors[:name] - end - end - - def test_validates_length_of_with_custom_message_using_quotes - repair_validations(Developer) do - Developer.validates_length_of :name, :minimum => 4, :message=> "This string contains 'single' and \"double\" quotes" - d = Developer.new - d.name = "Joe" - assert !d.valid? - assert_equal ["This string contains 'single' and \"double\" quotes"], d.errors[:name] - end - end - def test_validates_length_of_for_ruby_class - repair_validations(Person) do - Person.validates_length_of :karma, :minimum => 5 + Person.validates_length_of :karma, :minimum => 5 - p = Person.new - p.karma = "Pix" - assert p.invalid? + p = Person.new + p.karma = "Pix" + assert p.invalid? - assert_equal ["is too short (minimum is 5 characters)"], p.errors[:karma] + assert_equal ["is too short (minimum is 5 characters)"], p.errors[:karma] - p.karma = "The Smiths" - assert p.valid? - end + p.karma = "The Smiths" + assert p.valid? + ensure + Person.reset_callbacks(:validate) end end diff --git a/activemodel/test/cases/validations/numericality_validation_test.rb b/activemodel/test/cases/validations/numericality_validation_test.rb index d3201966dc..75cd654f98 100644 --- a/activemodel/test/cases/validations/numericality_validation_test.rb +++ b/activemodel/test/cases/validations/numericality_validation_test.rb @@ -8,9 +8,10 @@ require 'models/person' class NumericalityValidationTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end NIL = [nil] BLANK = ["", " ", " \t \r \n"] @@ -138,37 +139,19 @@ class NumericalityValidationTest < ActiveModel::TestCase assert_equal ["greater than 4"], topic.errors[:approved] end - def test_numericality_with_getter_method - repair_validations(Developer) do - Developer.validates_numericality_of( :salary ) - developer = Developer.new("name" => "michael", "salary" => nil) - developer.instance_eval("def salary; read_attribute('salary') ? read_attribute('salary') : 100000; end") - assert developer.valid? - end - end - - def test_numericality_with_allow_nil_and_getter_method - repair_validations(Developer) do - Developer.validates_numericality_of( :salary, :allow_nil => true) - developer = Developer.new("name" => "michael", "salary" => nil) - developer.instance_eval("def salary; read_attribute('salary') ? read_attribute('salary') : 100000; end") - assert developer.valid? - end - end - def test_validates_numericality_of_for_ruby_class - repair_validations(Person) do - Person.validates_numericality_of :karma, :allow_nil => false + Person.validates_numericality_of :karma, :allow_nil => false - p = Person.new - p.karma = "Pix" - assert p.invalid? + p = Person.new + p.karma = "Pix" + assert p.invalid? - assert_equal ["is not a number"], p.errors[:karma] + assert_equal ["is not a number"], p.errors[:karma] - p.karma = "1234" - assert p.valid? - end + p.karma = "1234" + assert p.valid? + ensure + Person.reset_callbacks(:validate) end private diff --git a/activemodel/test/cases/validations/presence_validation_test.rb b/activemodel/test/cases/validations/presence_validation_test.rb index 90b0951a77..8b9795a90c 100644 --- a/activemodel/test/cases/validations/presence_validation_test.rb +++ b/activemodel/test/cases/validations/presence_validation_test.rb @@ -9,9 +9,6 @@ require 'models/custom_reader' class PresenceValidationTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - - repair_validations(Topic) def test_validate_presences Topic.validates_presence_of(:title, :content) @@ -30,43 +27,44 @@ class PresenceValidationTest < ActiveModel::TestCase t.content = "like stuff" assert t.save + ensure + Topic.reset_callbacks(:validate) end - # def test_validates_presence_of_with_custom_message_using_quotes - # repair_validations(Developer) do - # Developer.validates_presence_of :non_existent, :message=> "This string contains 'single' and \"double\" quotes" - # d = Developer.new - # d.name = "Joe" - # assert !d.valid? - # assert_equal ["This string contains 'single' and \"double\" quotes"], d.errors[:non_existent] - # end - # end + def test_validates_acceptance_of_with_custom_error_using_quotes + Person.validates_presence_of :karma, :message=> "This string contains 'single' and \"double\" quotes" + p = Person.new + assert !p.valid? + assert_equal "This string contains 'single' and \"double\" quotes", p.errors[:karma].last + ensure + Person.reset_callbacks(:validate) + end def test_validates_presence_of_for_ruby_class - repair_validations(Person) do - Person.validates_presence_of :karma + Person.validates_presence_of :karma - p = Person.new - assert p.invalid? + p = Person.new + assert p.invalid? - assert_equal ["can't be blank"], p.errors[:karma] + assert_equal ["can't be blank"], p.errors[:karma] - p.karma = "Cold" - assert p.valid? - end + p.karma = "Cold" + assert p.valid? + ensure + Person.reset_callbacks(:validate) end def test_validates_presence_of_for_ruby_class_with_custom_reader - repair_validations(Person) do - CustomReader.validates_presence_of :karma + CustomReader.validates_presence_of :karma - p = CustomReader.new - assert p.invalid? + p = CustomReader.new + assert p.invalid? - assert_equal ["can't be blank"], p.errors[:karma] + assert_equal ["can't be blank"], p.errors[:karma] - p[:karma] = "Cold" - assert p.valid? - end + p[:karma] = "Cold" + assert p.valid? + ensure + CustomReader.reset_callbacks(:validate) end end diff --git a/activemodel/test/cases/validations/with_validation_test.rb b/activemodel/test/cases/validations/with_validation_test.rb index fae87a6188..9e9b925df2 100644 --- a/activemodel/test/cases/validations/with_validation_test.rb +++ b/activemodel/test/cases/validations/with_validation_test.rb @@ -6,32 +6,33 @@ require 'models/topic' class ValidatesWithTest < ActiveRecord::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end ERROR_MESSAGE = "Validation error from validator" OTHER_ERROR_MESSAGE = "Validation error from other validator" class ValidatorThatAddsErrors < ActiveModel::Validator - def validate() + def validate(record) record.errors[:base] << ERROR_MESSAGE end end class OtherValidatorThatAddsErrors < ActiveModel::Validator - def validate() + def validate(record) record.errors[:base] << OTHER_ERROR_MESSAGE end end class ValidatorThatDoesNotAddErrors < ActiveModel::Validator - def validate() + def validate(record) end end class ValidatorThatValidatesOptions < ActiveModel::Validator - def validate() + def validate(record) if options[:field] == :first_name record.errors[:base] << ERROR_MESSAGE end @@ -98,11 +99,11 @@ class ValidatesWithTest < ActiveRecord::TestCase assert topic.errors[:base].include?(ERROR_MESSAGE) end - test "passes all non-standard configuration options to the validator class" do + test "passes all configuration options to the validator class" do topic = Topic.new validator = mock() - validator.expects(:new).with(topic, {:foo => :bar}).returns(validator) - validator.expects(:validate) + validator.expects(:new).with(:foo => :bar, :if => "1 == 1").returns(validator) + validator.expects(:validate).with(topic) Topic.validates_with(validator, :if => "1 == 1", :foo => :bar) assert topic.valid? diff --git a/activemodel/test/cases/validations_test.rb b/activemodel/test/cases/validations_test.rb index 78565177d8..61910395b5 100644 --- a/activemodel/test/cases/validations_test.rb +++ b/activemodel/test/cases/validations_test.rb @@ -9,11 +9,12 @@ require 'models/custom_reader' class ValidationsTest < ActiveModel::TestCase include ActiveModel::TestsDatabase - include ActiveModel::ValidationsRepairHelper # Most of the tests mess with the validations of Topic, so lets repair it all the time. # Other classes we mess with will be dealt with in the specific tests - repair_validations(Topic) + def teardown + Topic.reset_callbacks(:validate) + end def test_single_field_validation r = Reply.new diff --git a/activemodel/test/models/person.rb b/activemodel/test/models/person.rb index d98420f900..c83d768379 100644 --- a/activemodel/test/models/person.rb +++ b/activemodel/test/models/person.rb @@ -1,5 +1,9 @@ class Person include ActiveModel::Validations + extend ActiveModel::Translation - attr_accessor :title, :karma + attr_accessor :title, :karma, :salary +end + +class Child < Person end diff --git a/activemodel/test/models/track_back.rb b/activemodel/test/models/track_back.rb new file mode 100644 index 0000000000..d137e4ff8f --- /dev/null +++ b/activemodel/test/models/track_back.rb @@ -0,0 +1,4 @@ +class Post + class TrackBack + end +end
\ No newline at end of file diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index a2c5528860..4846774deb 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,48 @@ *Edge* +* Add Relation#delete_all. [Pratik Naik] + + Item.where(:colour => 'red').delete_all + +* Add Model.having and Relation#having. [Pratik Naik] + + Developer.group("salary").having("sum(salary) > 10000").select("salary") + +* Add Relation#count. [Pratik Naik] + + legends = People.where("age > 100") + legends.count + legends.count(:age, :distinct => true) + legends.select('id').count + +* Add Model.readonly and association_collection#readonly finder method. [Pratik Naik] + + Post.readonly.to_a # Load all posts in readonly mode + @user.items.readonly(false).to_a # Load all the user items in writable mode + +* Add .lock finder method [Pratik Naik] + + User.lock.where(:name => 'lifo').to_a + + old_items = Item.where("age > 100") + old_items.lock.each {|i| .. } + +* Add Model.from and association_collection#from finder methods [Pratik Naik] + + user = User.scoped + user.select('*').from('users, items') + +* Add relation.destroy_all [Pratik Naik] + + old_items = Item.where("age > 100") + old_items.destroy_all + +* Add relation.exists? [Pratik Naik] + + red_items = Item.where(:colours => 'red') + red_items.exists? + red_items.exists?(1) + * Add find(ids) to relations. [Pratik Naik] old_users = User.order("age DESC") diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 196b87c0ac..7031c67539 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -48,10 +48,12 @@ module ActiveRecord autoload :Attributes autoload :AutosaveAssociation autoload :Relation + autoload :RelationalCalculations autoload :Base autoload :Batches autoload :Calculations autoload :Callbacks + autoload :ControllerRuntime autoload :DynamicFinderMatch autoload :DynamicScopeMatch autoload :Migration diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 2735bc5141..f0bad6c3ba 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -3,8 +3,8 @@ require 'active_support/core_ext/enumerable' module ActiveRecord class InverseOfAssociationNotFoundError < ActiveRecordError #:nodoc: - def initialize(reflection) - super("Could not find the inverse association for #{reflection.name} (#{reflection.options[:inverse_of].inspect} in #{reflection.class_name})") + def initialize(reflection, associated_class = nil) + super("Could not find the inverse association for #{reflection.name} (#{reflection.options[:inverse_of].inspect} in #{associated_class.nil? ? reflection.class_name : associated_class.name})") end end @@ -1466,11 +1466,10 @@ module ActiveRecord end def find_with_associations(options = {}, join_dependency = nil) - catch :invalid_query do - join_dependency ||= JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins]) - rows = select_all_rows(options, join_dependency) - return join_dependency.instantiate(rows) - end + join_dependency ||= JoinDependency.new(self, merge_includes(scope(:find, :include), options[:include]), options[:joins]) + rows = select_all_rows(options, join_dependency) + join_dependency.instantiate(rows) + rescue ThrowResult [] end @@ -1707,7 +1706,7 @@ module ActiveRecord def construct_finder_arel_with_included_associations(options, join_dependency) scope = scope(:find) - relation = arel_table((scope && scope[:from]) || options[:from]) + relation = arel_table for association in join_dependency.join_associations relation = association.join_relation(relation) @@ -1715,9 +1714,11 @@ module ActiveRecord relation = relation.joins(construct_join(options[:joins], scope)). select(column_aliases(join_dependency)). - group(construct_group(options[:group], options[:having], scope)). + group(options[:group] || (scope && scope[:group])). + having(options[:having] || (scope && scope[:having])). order(construct_order(options[:order], scope)). - where(construct_conditions(options[:conditions], scope)) + where(construct_conditions(options[:conditions], scope)). + from((scope && scope[:from]) || options[:from]) relation = relation.where(construct_arel_limited_ids_condition(options, join_dependency)) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) relation = relation.limit(construct_limit(options[:limit], scope)) if using_limitable_reflections?(join_dependency.reflections) @@ -1731,7 +1732,7 @@ module ActiveRecord def construct_arel_limited_ids_condition(options, join_dependency) if (ids_array = select_limited_ids_array(options, join_dependency)).empty? - throw :invalid_query + raise ThrowResult else Arel::Predicates::In.new( Arel::SqlLiteral.new("#{connection.quote_table_name table_name}.#{primary_key}"), @@ -1758,7 +1759,8 @@ module ActiveRecord relation = relation.joins(construct_join(options[:joins], scope)). where(construct_conditions(options[:conditions], scope)). - group(construct_group(options[:group], options[:having], scope)). + group(options[:group] || (scope && scope[:group])). + having(options[:having] || (scope && scope[:having])). order(construct_order(options[:order], scope)). limit(construct_limit(options[:limit], scope)). offset(construct_limit(options[:offset], scope)). diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index b85d76e8d3..b2b3a9789c 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -21,7 +21,7 @@ module ActiveRecord construct_sql end - delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :to => :scoped + delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :from, :lock, :readonly, :having, :to => :scoped def select(select = nil, &block) if block_given? @@ -52,27 +52,21 @@ module ActiveRecord load_target.select { |r| ids.include?(r.id) } end else - conditions = "#{@finder_sql}" - if sanitized_conditions = sanitize_sql(options[:conditions]) - conditions << " AND (#{sanitized_conditions})" - end - - options[:conditions] = conditions + merge_options_from_reflection!(options) + construct_find_options!(options) + + find_scope = construct_scope[:find].slice(:conditions, :order) + + with_scope(:find => find_scope) do + relation = @reflection.klass.send(:construct_finder_arel_with_includes, options) - if options[:order] && @reflection.options[:order] - options[:order] = "#{options[:order]}, #{@reflection.options[:order]}" - elsif @reflection.options[:order] - options[:order] = @reflection.options[:order] + case args.first + when :first, :last, :all + relation.send(args.first) + else + relation.find(*args) + end end - - # Build options specific to association - construct_find_options!(options) - - merge_options_from_reflection!(options) - - # Pass through args exactly as we received them. - args << options - @reflection.klass.find(*args) end end @@ -183,7 +177,7 @@ module ActiveRecord if @reflection.options[:counter_sql] @reflection.klass.count_by_sql(@counter_sql) else - column_name, options = @reflection.klass.send(:construct_count_options_from_args, *args) + column_name, options = @reflection.klass.scoped.send(:construct_count_options_from_args, *args) 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 diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index 67e18d692d..f6edd6383c 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -13,6 +13,7 @@ module ActiveRecord @updated = true end + set_inverse_instance(record, @owner) loaded record end @@ -22,21 +23,44 @@ module ActiveRecord end private - def find_target - return nil if association_class.nil? - if @reflection.options[:conditions] - association_class.find( - @owner[@reflection.primary_key_name], - :select => @reflection.options[:select], - :conditions => conditions, - :include => @reflection.options[:include] - ) + # NOTE - for now, we're only supporting inverse setting from belongs_to back onto + # has_one associations. + def we_can_set_the_inverse_on_this?(record) + if @reflection.has_inverse? + inverse_association = @reflection.polymorphic_inverse_of(record.class) + inverse_association && inverse_association.macro == :has_one else - association_class.find(@owner[@reflection.primary_key_name], :select => @reflection.options[:select], :include => @reflection.options[:include]) + false + end + end + + def set_inverse_instance(record, instance) + return if record.nil? || !we_can_set_the_inverse_on_this?(record) + inverse_relationship = @reflection.polymorphic_inverse_of(record.class) + unless inverse_relationship.nil? + record.send(:"set_#{inverse_relationship.name}_target", instance) end end + def find_target + return nil if association_class.nil? + + target = + if @reflection.options[:conditions] + association_class.find( + @owner[@reflection.primary_key_name], + :select => @reflection.options[:select], + :conditions => conditions, + :include => @reflection.options[:include] + ) + else + association_class.find(@owner[@reflection.primary_key_name], :select => @reflection.options[:select], :include => @reflection.options[:include]) + end + set_inverse_instance(target, @owner) + target + end + def foreign_key_present !@owner[@reflection.primary_key_name].nil? end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index b85a40b2e5..ea769fd48b 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -57,6 +57,7 @@ module ActiveRecord @target = (AssociationProxy === obj ? obj.target : obj) end + set_inverse_instance(obj, @owner) @loaded = true unless @owner.new_record? or obj.nil? or dont_save @@ -120,10 +121,9 @@ module ActiveRecord else record[@reflection.primary_key_name] = @owner.id unless @owner.new_record? self.target = record + set_inverse_instance(record, @owner) end - set_inverse_instance(record, @owner) - record end diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index c0d8904bc8..44c668b619 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -155,6 +155,13 @@ module ActiveRecord # Adds a validate and save callback for the association as specified by # the +reflection+. + # + # For performance reasons, we don't check whether to validate at runtime, + # but instead only define the method and callback when needed. However, + # this can change, for instance, when using nested attributes. Since we + # don't want the callbacks to get defined multiple times, there are + # guards that check if the save or validation methods have already been + # defined before actually defining them. def add_autosave_association_callbacks(reflection) save_method = "autosave_associated_records_for_#{reflection.name}" validation_method = "validate_associated_records_for_#{reflection.name}" @@ -162,28 +169,33 @@ module ActiveRecord case reflection.macro when :has_many, :has_and_belongs_to_many - before_save :before_save_collection_association + unless method_defined?(save_method) + before_save :before_save_collection_association - define_method(save_method) { save_collection_association(reflection) } - # Doesn't use after_save as that would save associations added in after_create/after_update twice - after_create save_method - after_update save_method + define_method(save_method) { save_collection_association(reflection) } + # Doesn't use after_save as that would save associations added in after_create/after_update twice + after_create save_method + after_update save_method + end - if force_validation || (reflection.macro == :has_many && reflection.options[:validate] != false) + if !method_defined?(validation_method) && + (force_validation || (reflection.macro == :has_many && reflection.options[:validate] != false)) define_method(validation_method) { validate_collection_association(reflection) } validate validation_method end else - case reflection.macro - when :has_one - define_method(save_method) { save_has_one_association(reflection) } - after_save save_method - when :belongs_to - define_method(save_method) { save_belongs_to_association(reflection) } - before_save save_method + unless method_defined?(save_method) + case reflection.macro + when :has_one + define_method(save_method) { save_has_one_association(reflection) } + after_save save_method + when :belongs_to + define_method(save_method) { save_belongs_to_association(reflection) } + before_save save_method + end end - if force_validation + if !method_defined?(validation_method) && force_validation define_method(validation_method) { validate_single_association(reflection) } validate validation_method end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2008dea5e9..07c5545171 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -69,6 +69,10 @@ module ActiveRecord #:nodoc: class StatementInvalid < ActiveRecordError end + # Raised when SQL statement is invalid and the application gets a blank result. + class ThrowResult < ActiveRecordError + end + # Parent class for all specific exceptions which wrap database driver exceptions # provides access to the original exception also. class WrappedDatabaseException < StatementInvalid @@ -640,18 +644,19 @@ module ActiveRecord #:nodoc: # end def find(*args) options = args.extract_options! - validate_find_options(options) set_readonly_option!(options) + relation = construct_finder_arel_with_includes(options) + case args.first - when :first then find_initial(options) - when :last then find_last(options) - when :all then find_every(options) - else find_from_ids(args, options) + when :first, :last, :all + relation.send(args.first) + else + relation.find(*args) end end - delegate :select, :group, :order, :limit, :joins, :where, :preload, :eager_load, :to => :scoped + delegate :select, :group, :order, :limit, :joins, :where, :preload, :eager_load, :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>. @@ -665,16 +670,10 @@ module ActiveRecord #:nodoc: find(:last, *args) end - # Returns an ActiveRecord::Relation object. You can pass in all the same arguments to this method as you can - # to find(:all). + # 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) - options = args.extract_options! - - if options.empty? && !scoped?(:find) - arel_table.to_a - else - construct_finder_arel_with_includes(options).to_a - end + find(:all, *args) end # Executes a custom SQL query against your database and returns all the results. The results will @@ -728,10 +727,13 @@ module ActiveRecord #:nodoc: # Person.exists?(:name => "David") # Person.exists?(['name LIKE ?', "%#{query}%"]) # Person.exists? - def exists?(id_or_conditions = {}) - find_initial( - :select => "#{quoted_table_name}.#{primary_key}", - :conditions => expand_id_conditions(id_or_conditions)) ? true : false + 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. @@ -916,7 +918,7 @@ module ActiveRecord #:nodoc: # Person.destroy_all("last_login < '2004-04-04'") # Person.destroy_all(:status => "inactive") def destroy_all(conditions = nil) - find(:all, :conditions => conditions).each { |object| object.destroy } + where(conditions).destroy_all end # Deletes the records matching +conditions+ without instantiating the records first, and hence not @@ -937,11 +939,7 @@ module ActiveRecord #:nodoc: # 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) - if conditions - arel_table.where(Arel::SqlLiteral.new(construct_conditions(conditions, scope(:find)))).delete - else - arel_table.delete - end + arel_table.where(construct_conditions(conditions, scope(:find))).delete_all end # Returns the result of an SQL statement that should only include a COUNT(*) in the SELECT part. @@ -1512,120 +1510,6 @@ module ActiveRecord #:nodoc: end private - def find_initial(options) - options.update(:limit => 1) - find_every(options).first - end - - def find_last(options) - order = options[:order] - - if order - order = reverse_sql_order(order) - elsif !scoped?(:find, :order) - order = "#{table_name}.#{primary_key} DESC" - end - - if scoped?(:find, :order) - scope = scope(:find) - original_scoped_order = scope[:order] - scope[:order] = reverse_sql_order(original_scoped_order) - end - - begin - find_initial(options.merge({ :order => order })) - ensure - scope[:order] = original_scoped_order if original_scoped_order - end - end - - def reverse_sql_order(order_query) - order_query.to_s.split(/,/).each { |s| - if s.match(/\s(asc|ASC)$/) - s.gsub!(/\s(asc|ASC)$/, ' DESC') - elsif s.match(/\s(desc|DESC)$/) - s.gsub!(/\s(desc|DESC)$/, ' ASC') - else - s.concat(' DESC') - end - }.join(',') - end - - def find_every(options) - include_associations = merge_includes(scope(:find, :include), options[:include]) - - if include_associations.any? && references_eager_loaded_tables?(options) - records = find_with_associations(options) - else - records = find_by_sql(construct_finder_sql(options)) - if include_associations.any? - preload_associations(records, include_associations) - end - end - - records.each { |record| record.readonly! } if options[:readonly] - - records - end - - def find_from_ids(ids, options) - 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 #{name} without an ID" - when 1 - result = find_one(ids.first, options) - expects_array ? [ result ] : result - else - find_some(ids, options) - end - end - - def find_one(id, options) - conditions = " AND (#{sanitize_sql(options[:conditions])})" if options[:conditions] - options.update :conditions => "#{quoted_table_name}.#{connection.quote_column_name(primary_key)} = #{quote_value(id,columns_hash[primary_key])}#{conditions}" - - # Use find_every(options).first since the primary key condition - # already ensures we have a single record. Using find_initial adds - # a superfluous :limit => 1. - if result = find_every(options).first - result - else - raise RecordNotFound, "Couldn't find #{name} with ID=#{id}#{conditions}" - end - end - - def find_some(ids, options) - conditions = " AND (#{sanitize_sql(options[:conditions])})" if options[:conditions] - ids_list = ids.map { |id| quote_value(id,columns_hash[primary_key]) }.join(',') - options.update :conditions => "#{quoted_table_name}.#{connection.quote_column_name(primary_key)} IN (#{ids_list})#{conditions}" - - result = find_every(options) - - # Determine expected size from limit and offset, not just ids.size. - expected_size = - if options[:limit] && ids.size > options[:limit] - options[:limit] - else - ids.size - end - - # 11 ids with limit 3, offset 9 should give 2 results. - if options[:offset] && (ids.size - options[:offset] < expected_size) - expected_size = ids.size - options[:offset] - end - - if result.size == expected_size - result - else - raise RecordNotFound, "Couldn't find all #{name.pluralize} with IDs (#{ids_list})#{conditions} (found #{result.size} results, but was looking for #{expected_size})" - end - end - # Finder methods must instantiate through this method to work with the # single-table inheritance model that makes it possible to create # objects of different types from the same table. @@ -1676,17 +1560,21 @@ module ActiveRecord #:nodoc: end def construct_finder_arel(options = {}, scope = scope(:find)) - # TODO add lock to Arel validate_find_options(options) - relation = arel_table(options[:from]). + relation = arel_table. joins(construct_join(options[:joins], scope)). where(construct_conditions(options[:conditions], scope)). select(options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))). - group(construct_group(options[:group], options[:having], scope)). + group(options[:group] || (scope && scope[:group])). + having(options[:having] || (scope && scope[:having])). order(construct_order(options[:order], scope)). limit(construct_limit(options[:limit], scope)). - offset(construct_offset(options[:offset], scope)) + offset(construct_offset(options[:offset], scope)). + from(options[:from]) + + lock = (scope && scope[:lock]) || options[:lock] + relation = relation.lock if lock.present? relation = relation.readonly if options[:readonly] @@ -1708,10 +1596,6 @@ module ActiveRecord #:nodoc: relation end - def construct_finder_sql(options, scope = scope(:find)) - construct_finder_arel(options, scope).to_sql - end - def construct_join(joins, scope) merged_joins = scope && scope[:joins] && joins ? merge_joins(scope[:joins], joins) : (joins || scope && scope[:joins]) case merged_joins @@ -1728,20 +1612,9 @@ module ActiveRecord #:nodoc: end end - def construct_group(group, having, scope) - sql = '' - if group - sql << group.to_s - sql << " HAVING #{sanitize_sql_for_conditions(having)}" if having - elsif scope && (scoped_group = scope[:group]) - sql << scoped_group.to_s - sql << " HAVING #{sanitize_sql_for_conditions(scope[:having])}" if scope[:having] - end - sql - end - def construct_order(order, scope) orders = [] + scoped_order = scope[:order] if scope if order orders << order @@ -1749,7 +1622,8 @@ module ActiveRecord #:nodoc: elsif scoped_order orders << scoped_order end - orders + + orders.reject {|o| o.blank?} end def construct_limit(limit, scope) @@ -1771,7 +1645,7 @@ module ActiveRecord #:nodoc: # Merges includes so that the result is a valid +include+ def merge_includes(first, second) - (safe_to_array(first) + safe_to_array(second)).uniq + (Array.wrap(first) + Array.wrap(second)).uniq end def merge_joins(*joins) @@ -1783,7 +1657,7 @@ module ActiveRecord #:nodoc: end joins.flatten.map{|j| j.strip}.uniq else - joins.collect{|j| safe_to_array(j)}.flatten.uniq + joins.collect{|j| Array.wrap(j)}.flatten.uniq end end @@ -1800,30 +1674,10 @@ module ActiveRecord #:nodoc: }.join(" ") end - # Object#to_a is deprecated, though it does have the desired behavior - def safe_to_array(o) - case o - when NilClass - [] - when Array - o - else - [o] - end - end - def array_of_strings?(o) o.is_a?(Array) && o.all?{|obj| obj.is_a?(String)} end - # The optional scope argument is for the current <tt>:find</tt> scope. - # The <tt>:lock</tt> option has precedence over a scoped <tt>:lock</tt>. - def add_lock!(sql, options, scope = :auto) - scope = scope(:find) if :auto == scope - options = options.reverse_merge(:lock => scope[:lock]) if scope - connection.add_lock!(sql, options) - end - def type_condition(table_alias=nil) quoted_table_alias = self.connection.quote_table_name(table_alias || table_name) quoted_inheritance_column = connection.quote_column_name(inheritance_column) @@ -1925,14 +1779,6 @@ module ActiveRecord #:nodoc: end end - # Interpret Array and Hash as conditions and anything else as an id. - def expand_id_conditions(id_or_conditions) - case id_or_conditions - when Array, Hash then id_or_conditions - else sanitize_sql(primary_key => id_or_conditions) - end - end - protected # Scope parameters to method calls within the block. Takes a hash of method_name => parameters hash. # method_name may be <tt>:find</tt> or <tt>:create</tt>. <tt>:find</tt> parameters may include the <tt>:conditions</tt>, <tt>:joins</tt>, diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index fcba23dc0d..d51d9f2159 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -44,7 +44,26 @@ module ActiveRecord # # 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) - calculate(:count, *construct_count_options_from_args(*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 @@ -122,168 +141,63 @@ module ActiveRecord # 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 = {}) - validate_calculation_options(operation, options) - operation = operation.to_s.downcase - - scope = scope(:find) + construct_calculation_arel(options).calculate(operation, column_name, options.slice(:distinct)) + rescue ThrowResult + 0 + end - merged_includes = merge_includes(scope ? scope[:include] : [], options[:include]) + private + def validate_calculation_options(options = {}) + options.assert_valid_keys(CALCULATIONS_OPTIONS) + end - if operation == "count" - if merged_includes.any? - distinct = true - column_name = options[:select] || primary_key - end + def construct_calculation_arel(options = {}) + validate_calculation_options(options) + options = options.except(:distinct) - distinct = nil if column_name.to_s =~ /\s*DISTINCT\s+/i - distinct ||= options[:distinct] - else - distinct = nil - end + scope = scope(:find) + includes = merge_includes(scope ? scope[:include] : [], options[:include]) - catch :invalid_query do - relation = if merged_includes.any? - join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, construct_join(options[:joins], scope)) - construct_finder_arel_with_included_associations(options, join_dependency) + if includes.any? + join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, includes, construct_join(options[:joins], scope)) + construct_calculation_arel_with_included_associations(options, join_dependency) else - relation = arel_table(options[:from]). + arel_table. joins(construct_join(options[:joins], scope)). + from((scope && scope[:from]) || options[:from]). where(construct_conditions(options[:conditions], scope)). order(options[:order]). limit(options[:limit]). - offset(options[:offset]) - end - if options[:group] - return execute_grouped_calculation(operation, column_name, options, relation) - else - return execute_simple_calculation(operation, column_name, options.merge(:distinct => distinct), relation) + offset(options[:offset]). + group(options[:group]). + having(options[:having]). + select(options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))) end end - 0 - end - - def execute_simple_calculation(operation, column_name, options, relation) #:nodoc: - column = if column_names.include?(column_name.to_s) - Arel::Attribute.new(arel_table(options[:from] || table_name), - options[:select] || column_name) - else - Arel::SqlLiteral.new(options[:select] || - (column_name == :all ? "*" : column_name.to_s)) - end - - relation = relation.select(operation == 'count' ? column.count(options[:distinct]) : column.send(operation)) - - type_cast_calculated_value(connection.select_value(relation.to_sql), column_for(column_name), operation) - end - def execute_grouped_calculation(operation, column_name, options, relation) #:nodoc: - group_attr = options[:group].to_s - association = 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 + def construct_calculation_arel_with_included_associations(options, join_dependency) + scope = scope(:find) - options[:group] = connection.adapter_name == 'FrontBase' ? group_alias : group_field + relation = arel_table - aggregate_alias = column_alias_for(operation, column_name) - - options[:select] = (operation == 'count' && column_name == :all) ? - "COUNT(*) AS count_all" : - Arel::Attribute.new(arel_table, column_name).send(operation).as(aggregate_alias).to_sql - - options[:select] << ", #{group_field} AS #{group_alias}" - - relation = relation.select(options[:select]).group(construct_group(options[:group], options[:having], nil)) - - calculated_data = 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 - - protected - def construct_count_options_from_args(*args) - options = {} - column_name = :all - - # We need to handle - # count() - # count(:column_name=:all) - # count(options={}) - # count(column_name=:all, options={}) - # selects specified by scopes - case args.size - when 0 - column_name = scope(:find)[:select] if scope(:find) - when 1 - if args[0].is_a?(Hash) - column_name = scope(:find)[:select] if scope(:find) - 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}" + for association in join_dependency.join_associations + relation = association.join_relation(relation) end - [column_name || :all, options] - end - - private - def validate_calculation_options(operation, options = {}) - options.assert_valid_keys(CALCULATIONS_OPTIONS) - end + relation = relation.joins(construct_join(options[:joins], scope)). + select(column_aliases(join_dependency)). + group(options[:group]). + having(options[:having]). + order(options[:order]). + where(construct_conditions(options[:conditions], scope)). + from((scope && scope[:from]) || options[:from]) - # 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!(/ +/, '_') + relation = relation.where(construct_arel_limited_ids_condition(options, join_dependency)) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) + relation = relation.limit(construct_limit(options[:limit], scope)) if using_limitable_reflections?(join_dependency.reflections) - connection.table_alias_for(table_name) + relation end - def column_for(field) - field_name = field.to_s.split('.').last - 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 end end end diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb index b25893a1c3..e1d772bd95 100644 --- a/activerecord/lib/active_record/callbacks.rb +++ b/activerecord/lib/active_record/callbacks.rb @@ -1,5 +1,3 @@ -require 'observer' - module ActiveRecord # Callbacks are hooks into the lifecycle of an Active Record object that allow you to trigger logic # before or after an alteration of the object state. This can be used to make sure that associated and @@ -210,7 +208,6 @@ module ActiveRecord # instead of quietly returning +false+. module Callbacks extend ActiveSupport::Concern - include ActiveSupport::Callbacks CALLBACKS = [ :after_initialize, :after_find, :before_validation, :after_validation, @@ -224,60 +221,14 @@ module ActiveRecord alias_method_chain method, :callbacks end - define_callbacks :initialize, :find, :save, :create, :update, :destroy, - :validation, :terminator => "result == false", :scope => [:kind, :name] + extend ActiveModel::Callbacks + + define_model_callbacks :initialize, :find, :only => :after + define_model_callbacks :save, :create, :update, :destroy + define_model_callbacks :validation, :only => [:before, :after] end module ClassMethods - def after_initialize(*args, &block) - options = args.extract_options! - options[:prepend] = true - set_callback(:initialize, :after, *(args << options), &block) - end - - def after_find(*args, &block) - options = args.extract_options! - options[:prepend] = true - set_callback(:find, :after, *(args << options), &block) - end - - [:save, :create, :update, :destroy].each do |callback| - module_eval <<-CALLBACKS, __FILE__, __LINE__ - def before_#{callback}(*args, &block) - set_callback(:#{callback}, :before, *args, &block) - end - - def around_#{callback}(*args, &block) - set_callback(:#{callback}, :around, *args, &block) - end - - def after_#{callback}(*args, &block) - options = args.extract_options! - options[:prepend] = true - options[:if] = Array(options[:if]) << "!halted && value != false" - set_callback(:#{callback}, :after, *(args << options), &block) - end - CALLBACKS - end - - def before_validation(*args, &block) - options = args.extract_options! - if options[:on] - options[:if] = Array(options[:if]) - options[:if] << "@_on_validate == :#{options[:on]}" - end - set_callback(:validation, :before, *(args << options), &block) - end - - def after_validation(*args, &block) - options = args.extract_options! - options[:if] = Array(options[:if]) - options[:if] << "!halted" - options[:if] << "@_on_validate == :#{options[:on]}" if options[:on] - options[:prepend] = true - set_callback(:validation, :after, *(args << options), &block) - end - def method_added(meth) super if CALLBACKS.include?(meth.to_sym) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index be89873632..027d736484 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -181,18 +181,6 @@ module ActiveRecord # done if the transaction block raises an exception or returns false. def rollback_db_transaction() end - # Appends a locking clause to an SQL statement. - # This method *modifies* the +sql+ parameter. - # # SELECT * FROM suppliers FOR UPDATE - # add_lock! 'SELECT * FROM suppliers', :lock => true - # add_lock! 'SELECT * FROM suppliers', :lock => ' FOR UPDATE' - def add_lock!(sql, options) - case lock = options[:lock] - when true; sql << ' FOR UPDATE' - when String; sql << " #{lock}" - end - end - def default_sequence_name(table, column) nil end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index c9c2892ba4..78b897add6 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -183,12 +183,6 @@ module ActiveRecord catch_schema_changes { @connection.rollback } end - # SELECT ... FOR UPDATE is redundant since the table is locked. - def add_lock!(sql, options) #:nodoc: - sql - end - - # SCHEMA STATEMENTS ======================================== def tables(name = nil) #:nodoc: diff --git a/activerecord/lib/active_record/controller_runtime.rb b/activerecord/lib/active_record/controller_runtime.rb new file mode 100644 index 0000000000..1281901ae8 --- /dev/null +++ b/activerecord/lib/active_record/controller_runtime.rb @@ -0,0 +1,27 @@ +module ActiveRecord + module ControllerRuntime + extend ActiveSupport::Concern + + attr_internal :db_runtime + + def cleanup_view_runtime + if Object.const_defined?("ActiveRecord") && ActiveRecord::Base.connected? + db_rt_before_render = ActiveRecord::Base.connection.reset_runtime + runtime = super + db_rt_after_render = ActiveRecord::Base.connection.reset_runtime + self.db_runtime = db_rt_before_render + db_rt_after_render + runtime - db_rt_after_render + else + super + end + end + + module ClassMethods + def process_log_action(controller) + super + db_runtime = controller.send :db_runtime + logger.info(" ActiveRecord runtime: %.1fms" % db_runtime.to_f) if db_runtime + end + end + end +end
\ No newline at end of file diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index ca3110a374..ff3a51d5c0 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -212,6 +212,11 @@ module ActiveRecord # nested attributes array exceeds the specified limit, NestedAttributes::TooManyRecords # exception is raised. If omitted, any number associations can be processed. # Note that the :limit option is only applicable to one-to-many associations. + # [:update_only] + # Allows you to specify that an existing record may only be updated. + # A new record may only be created when there is no existing record. + # This option only works for one-to-one associations and is ignored for + # collection associations. This option is off by default. # # Examples: # # creates avatar_attributes= @@ -221,9 +226,9 @@ module ActiveRecord # # creates avatar_attributes= and posts_attributes= # accepts_nested_attributes_for :avatar, :posts, :allow_destroy => true def accepts_nested_attributes_for(*attr_names) - options = { :allow_destroy => false } + options = { :allow_destroy => false, :update_only => false } options.update(attr_names.extract_options!) - options.assert_valid_keys(:allow_destroy, :reject_if, :limit) + options.assert_valid_keys(:allow_destroy, :reject_if, :limit, :update_only) attr_names.each do |association_name| if reflection = reflect_on_association(association_name) @@ -235,7 +240,7 @@ module ActiveRecord end reflection.options[:autosave] = true - + add_autosave_association_callbacks(reflection) self.nested_attributes_options[association_name.to_sym] = options if options[:reject_if] == :all_blank @@ -243,15 +248,13 @@ module ActiveRecord end # def pirate_attributes=(attributes) - # assign_nested_attributes_for_one_to_one_association(:pirate, attributes, false) + # assign_nested_attributes_for_one_to_one_association(:pirate, attributes) # end class_eval %{ def #{association_name}_attributes=(attributes) assign_nested_attributes_for_#{type}_association(:#{association_name}, attributes) end }, __FILE__, __LINE__ - - add_autosave_association_callbacks(reflection) else raise ArgumentError, "No association found for name `#{association_name}'. Has it been defined yet?" end @@ -286,28 +289,29 @@ module ActiveRecord # Assigns the given attributes to the association. # - # If the given attributes include an <tt>:id</tt> that matches the existing - # record’s id, then the existing record will be modified. Otherwise a new - # record will be built. + # If update_only is false and the given attributes include an <tt>:id</tt> + # that matches the existing record’s id, then the existing record will be + # modified. If update_only is true, a new record is only created when no + # object exists. Otherwise a new record will be built. # - # If the given attributes include a matching <tt>:id</tt> attribute _and_ a - # <tt>:_destroy</tt> key set to a truthy value, then the existing record - # will be marked for destruction. + # If the given attributes include a matching <tt>:id</tt> attribute, or + # update_only is true, and a <tt>:_destroy</tt> key set to a truthy value, + # then the existing record will be marked for destruction. def assign_nested_attributes_for_one_to_one_association(association_name, attributes) options = self.nested_attributes_options[association_name] attributes = attributes.with_indifferent_access + check_existing_record = (options[:update_only] || !attributes['id'].blank?) - if attributes['id'].blank? - unless reject_new_record?(association_name, attributes) - method = "build_#{association_name}" - if respond_to?(method) - send(method, attributes.except(*UNASSIGNABLE_KEYS)) - else - raise ArgumentError, "Cannot build association #{association_name}. Are you trying to build a polymorphic one-to-one association?" - end + if check_existing_record && (record = send(association_name)) && + (options[:update_only] || record.id.to_s == attributes['id'].to_s) + assign_to_or_mark_for_destruction(record, attributes, options[:allow_destroy]) + elsif !reject_new_record?(association_name, attributes) + method = "build_#{association_name}" + if respond_to?(method) + send(method, attributes.except(*UNASSIGNABLE_KEYS)) + else + raise ArgumentError, "Cannot build association #{association_name}. Are you trying to build a polymorphic one-to-one association?" end - elsif (existing_record = send(association_name)) && existing_record.id.to_s == attributes['id'].to_s - assign_to_or_mark_for_destruction(existing_record, attributes, options[:allow_destroy]) end end diff --git a/activerecord/lib/active_record/rails.rb b/activerecord/lib/active_record/rails.rb index ddbc555113..e7de699974 100644 --- a/activerecord/lib/active_record/rails.rb +++ b/activerecord/lib/active_record/rails.rb @@ -7,6 +7,13 @@ require "action_controller/rails" module ActiveRecord class Plugin < Rails::Plugin plugin_name :active_record + include_modules_in "ActiveRecord::Base" + + config.action_controller.include "ActiveRecord::ControllerRuntime" + + rake_tasks do + load "active_record/rails/databases.rake" + end initializer "active_record.set_configs" do |app| app.config.active_record.each do |k,v| @@ -50,8 +57,8 @@ module ActiveRecord initializer "active_record.notifications" do require 'active_support/notifications' - ActiveSupport::Notifications.subscribe("sql") do |name, before, after, result, instrumenter_id, payload| - ActiveRecord::Base.connection.log_info(payload[:sql], name, after - before) + ActiveSupport::Notifications.subscribe("sql") do |name, before, after, instrumenter_id, payload| + ActiveRecord::Base.connection.log_info(payload[:sql], payload[:name], (after - before) * 1000) end end diff --git a/railties/lib/rails/tasks/databases.rake b/activerecord/lib/active_record/rails/databases.rake index a35a6c156b..a35a6c156b 100644 --- a/railties/lib/rails/tasks/databases.rake +++ b/activerecord/lib/active_record/rails/databases.rake diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index db5d2b25ed..b751c9ad68 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -214,8 +214,10 @@ module ActiveRecord end def check_validity_of_inverse! - if has_inverse? && inverse_of.nil? - raise InverseOfAssociationNotFoundError.new(self) + unless options[:polymorphic] + if has_inverse? && inverse_of.nil? + raise InverseOfAssociationNotFoundError.new(self) + end end end @@ -237,8 +239,16 @@ module ActiveRecord def inverse_of if has_inverse? @inverse_of ||= klass.reflect_on_association(options[:inverse_of]) - else - nil + end + end + + def polymorphic_inverse_of(associated_class) + if has_inverse? + if inverse_relationship = associated_class.reflect_on_association(options[:inverse_of]) + inverse_relationship + else + raise InverseOfAssociationNotFoundError.new(self, associated_class) + end end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index c927270eee..64261db809 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -1,55 +1,120 @@ module ActiveRecord class Relation + include RelationalCalculations + delegate :to_sql, :to => :relation - delegate :length, :collect, :find, :map, :each, :to => :to_a - attr_reader :relation, :klass + delegate :length, :collect, :map, :each, :all?, :to => :to_a + + attr_reader :relation, :klass, :preload_associations, :eager_load_associations + attr_writer :readonly, :preload_associations, :eager_load_associations - def initialize(klass, relation, readonly = false, preload = [], eager_load = []) + def initialize(klass, relation) @klass, @relation = klass, relation - @readonly = readonly - @associations_to_preload = preload - @eager_load_associations = eager_load - @loaded = false + @preload_associations = [] + @eager_load_associations = [] + @loaded, @readonly = false + end + + def merge(r) + raise ArgumentError, "Cannot merge a #{r.klass.name} relation with #{@klass.name} relation" if r.klass != @klass + + joins(r.relation.joins(r.relation)). + group(r.send(:group_clauses).join(', ')). + order(r.send(:order_clauses).join(', ')). + where(r.send(:where_clause)). + limit(r.taken). + offset(r.skipped). + select(r.send(:select_clauses).join(', ')). + eager_load(r.eager_load_associations). + preload(r.preload_associations). + from(r.send(:sources).present? ? r.send(:from_clauses) : nil) end + alias :& :merge + def preload(*associations) - create_new_relation(@relation, @readonly, @associations_to_preload + Array.wrap(associations)) + spawn.tap {|r| r.preload_associations += Array.wrap(associations) } end def eager_load(*associations) - create_new_relation(@relation, @readonly, @associations_to_preload, @eager_load_associations + Array.wrap(associations)) + spawn.tap {|r| r.eager_load_associations += Array.wrap(associations) } end - def readonly - create_new_relation(@relation, true) + def readonly(status = true) + spawn.tap {|r| r.readonly = status } end def select(selects) - create_new_relation(@relation.project(selects)) + if selects.present? + relation = spawn(@relation.project(selects)) + relation.readonly = @relation.joins(relation).present? ? false : @readonly + relation + else + spawn + end + end + + def from(from) + from.present? ? spawn(@relation.from(from)) : spawn + end + + def having(*args) + return spawn if args.blank? + + if [String, Hash, Array].include?(args.first.class) + havings = @klass.send(:merge_conditions, args.size > 1 ? Array.wrap(args) : args.first) + else + havings = args.first + end + + spawn(@relation.having(havings)) end def group(groups) - create_new_relation(@relation.group(groups)) + groups.present? ? spawn(@relation.group(groups)) : spawn end def order(orders) - create_new_relation(@relation.order(orders)) + orders.present? ? spawn(@relation.order(orders)) : spawn + end + + def lock(locks = true) + case locks + when String + spawn(@relation.lock(locks)) + when TrueClass, NilClass + spawn(@relation.lock) + else + spawn + end + end + + def reverse_order + relation = spawn + relation.instance_variable_set(:@orders, nil) + + order_clause = @relation.send(:order_clauses).join(', ') + if order_clause.present? + relation.order(reverse_sql_order(order_clause)) + else + relation.order("#{@klass.table_name}.#{@klass.primary_key} DESC") + end end def limit(limits) - create_new_relation(@relation.take(limits)) + limits.present? ? spawn(@relation.take(limits)) : spawn end def offset(offsets) - create_new_relation(@relation.skip(offsets)) + offsets.present? ? spawn(@relation.skip(offsets)) : spawn end def on(join) - create_new_relation(@relation.on(join)) + spawn(@relation.on(join)) end def joins(join, join_type = nil) - return self if join.blank? + return spawn if join.blank? join_relation = case join when String @@ -64,45 +129,58 @@ module ActiveRecord @relation.join(join, join_type) end - create_new_relation(join_relation) + spawn(join_relation).tap { |r| r.readonly = true } end def where(*args) + return spawn if args.blank? + if [String, Hash, Array].include?(args.first.class) conditions = @klass.send(:merge_conditions, args.size > 1 ? Array.wrap(args) : args.first) + conditions = Arel::SqlLiteral.new(conditions) if conditions else conditions = args.first end - create_new_relation(@relation.where(conditions)) + spawn(@relation.where(conditions)) end - def respond_to?(method) - @relation.respond_to?(method) || Array.method_defined?(method) || super + def respond_to?(method, include_private = false) + return true if @relation.respond_to?(method, include_private) || Array.method_defined?(method) + + if match = DynamicFinderMatch.match(method) + return true if @klass.send(:all_attributes_exists?, match.attribute_names) + elsif match = DynamicScopeMatch.match(method) + return true if @klass.send(:all_attributes_exists?, match.attribute_names) + else + super + end end def to_a return @records if loaded? @records = if @eager_load_associations.any? - catch :invalid_query do - return @klass.send(:find_with_associations, { + begin + @klass.send(:find_with_associations, { :select => @relation.send(:select_clauses).join(', '), :joins => @relation.joins(relation), :group => @relation.send(:group_clauses).join(', '), :order => @relation.send(:order_clauses).join(', '), :conditions => where_clause, :limit => @relation.taken, - :offset => @relation.skipped + :offset => @relation.skipped, + :from => (@relation.send(:from_clauses) if @relation.send(:sources).present?) }, ActiveRecord::Associations::ClassMethods::JoinDependency.new(@klass, @eager_load_associations, nil)) + rescue ThrowResult + [] end - [] else @klass.find_by_sql(@relation.to_sql) end - @associations_to_preload.each {|associations| @klass.send(:preload_associations, @records, associations) } + @preload_associations.each {|associations| @klass.send(:preload_associations, @records, associations) } @records.each { |record| record.readonly! } if @readonly @loaded = true @@ -130,6 +208,12 @@ module ActiveRecord end end + def exists?(id = nil) + relation = select("#{@klass.quoted_table_name}.#{@klass.primary_key}").limit(1) + relation = relation.where(@klass.primary_key => id) if id + relation.first ? true : false + end + def first if loaded? @records.first @@ -138,16 +222,54 @@ module ActiveRecord end end + def last + if loaded? + @records.last + else + @last ||= reverse_order.limit(1).to_a[0] + end + end + + def size + loaded? ? @records.length : count + end + + def empty? + loaded? ? @records.empty? : count.zero? + end + + def destroy_all + to_a.each {|object| object.destroy} + reset + end + + def delete_all + @relation.delete.tap { reset } + end + def loaded? @loaded end def reload @loaded = false - @records = @first = nil + reset + end + + def reset + @first = @last = nil + @records = [] self end + def spawn(relation = @relation) + relation = self.class.new(@klass, relation) + relation.readonly = @readonly + relation.preload_associations = @preload_associations + relation.eager_load_associations = @eager_load_associations + relation + end + protected def method_missing(method, *args, &block) @@ -241,12 +363,21 @@ module ActiveRecord end end - def create_new_relation(relation, readonly = @readonly, preload = @associations_to_preload, eager_load = @eager_load_associations) - Relation.new(@klass, relation, readonly, preload, eager_load) - end - def where_clause(join_string = "\n\tAND ") @relation.send(:where_clauses).join(join_string) end + + def reverse_sql_order(order_query) + order_query.to_s.split(/,/).each { |s| + if s.match(/\s(asc|ASC)$/) + s.gsub!(/\s(asc|ASC)$/, ' DESC') + elsif s.match(/\s(desc|DESC)$/) + s.gsub!(/\s(desc|DESC)$/, ' ASC') + else + s.concat(' DESC') + end + }.join(',') + end + end end diff --git a/activerecord/lib/active_record/relational_calculations.rb b/activerecord/lib/active_record/relational_calculations.rb new file mode 100644 index 0000000000..f42ffe0460 --- /dev/null +++ b/activerecord/lib/active_record/relational_calculations.rb @@ -0,0 +1,177 @@ +module ActiveRecord + module RelationalCalculations + + 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 = @relation.joins(relation) + 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 @relation.send(:groupings).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.arel_table, 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 = @relation.send(:groupings).first.value + 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.arel_table, 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) + # TODO : relation.projections only works when .select() was last in the chain. Fix it! + 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(relation = @relation) + if relation.respond_to?(:projections) && relation.projections.present? + relation.send(:select_clauses).join(', ') + elsif relation.respond_to?(:relation) + get_projection_name_from_chained_relations(relation.relation) + end + end + + end +end diff --git a/activerecord/lib/active_record/validations/associated.rb b/activerecord/lib/active_record/validations/associated.rb index 92f47d770f..66b78682ad 100644 --- a/activerecord/lib/active_record/validations/associated.rb +++ b/activerecord/lib/active_record/validations/associated.rb @@ -1,5 +1,12 @@ module ActiveRecord module Validations + class AssociatedValidator < ActiveModel::EachValidator + def validate_each(record, attribute, value) + return if (value.is_a?(Array) ? value : [value]).collect{ |r| r.nil? || r.valid? }.all? + record.errors.add(attribute, :invalid, :default => options[:message], :value => value) + end + end + module ClassMethods # Validates whether the associated object or objects are all valid themselves. Works with any kind of association. # @@ -33,13 +40,8 @@ module ActiveRecord # not occur (e.g. <tt>:unless => :skip_validation</tt>, or <tt>:unless => Proc.new { |user| user.signup_step <= 2 }</tt>). The # method, proc or string should return or evaluate to a true or false value. def validates_associated(*attr_names) - configuration = attr_names.extract_options! - - validates_each(attr_names, configuration) do |record, attr_name, value| - unless (value.is_a?(Array) ? value : [value]).collect { |r| r.nil? || r.valid? }.all? - record.errors.add(attr_name, :invalid, :default => configuration[:message], :value => value) - end - end + options = attr_names.extract_options! + validates_with AssociatedValidator, options.merge(:attributes => attr_names) end end end diff --git a/activerecord/lib/active_record/validations/uniqueness.rb b/activerecord/lib/active_record/validations/uniqueness.rb index 711086dc2c..ffbe1b5c40 100644 --- a/activerecord/lib/active_record/validations/uniqueness.rb +++ b/activerecord/lib/active_record/validations/uniqueness.rb @@ -1,5 +1,77 @@ module ActiveRecord module Validations + class UniquenessValidator < ActiveModel::EachValidator + def initialize(options) + @klass = options.delete(:klass) + super(options.reverse_merge(:case_sensitive => true)) + end + + def validate_each(record, attribute, value) + finder_class = find_finder_class_for(record) + table_name = record.class.quoted_table_name + sql, params = mount_sql_and_params(finder_class, table_name, attribute, value) + + Array(options[:scope]).each do |scope_item| + scope_value = record.send(scope_item) + sql << " AND " << record.class.send(:attribute_condition, "#{table_name}.#{scope_item}", scope_value) + params << scope_value + end + + unless record.new_record? + sql << " AND #{record.class.quoted_table_name}.#{record.class.primary_key} <> ?" + params << record.send(:id) + end + + finder_class.send(:with_exclusive_scope) do + if finder_class.exists?([sql, *params]) + record.errors.add(attribute, :taken, :default => options[:message], :value => value) + end + end + end + + protected + + # The check for an existing value should be run from a class that + # isn't abstract. This means working down from the current class + # (self), to the first non-abstract class. Since classes don't know + # their subclasses, we have to build the hierarchy between self and + # the record's class. + def find_finder_class_for(record) #:nodoc: + class_hierarchy = [record.class] + + while class_hierarchy.first != @klass + class_hierarchy.insert(0, class_hierarchy.first.superclass) + end + + class_hierarchy.detect { |klass| !klass.abstract_class? } + end + + def mount_sql_and_params(klass, table_name, attribute, value) #:nodoc: + column = klass.columns_hash[attribute.to_s] + + operator = if value.nil? + "IS ?" + elsif column.text? + value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s + "#{klass.connection.case_sensitive_equality_operator} ?" + else + "= ?" + end + + sql_attribute = "#{table_name}.#{klass.connection.quote_column_name(attribute)}" + + if value.nil? || (options[:case_sensitive] || !column.text?) + sql = "#{sql_attribute} #{operator}" + params = [value] + else + sql = "LOWER(#{sql_attribute}) #{operator}" + params = [value.mb_chars.downcase] + end + + [sql, params] + end + end + module ClassMethods # Validates whether the value of the specified attributes are unique across the system. Useful for making sure that only one user # can be named "davidhh". @@ -69,6 +141,7 @@ module ActiveRecord # # This could even happen if you use transactions with the 'serializable' # isolation level. There are several ways to get around this problem: + # # - By locking the database table before validating, and unlocking it after # saving. However, table locking is very expensive, and thus not # recommended. @@ -94,65 +167,10 @@ module ActiveRecord # index constraint errors from other types of database errors, so you # will have to parse the (database-specific) exception message to detect # such a case. + # def validates_uniqueness_of(*attr_names) - configuration = { :case_sensitive => true } - configuration.update(attr_names.extract_options!) - - validates_each(attr_names,configuration) do |record, attr_name, value| - # The check for an existing value should be run from a class that - # isn't abstract. This means working down from the current class - # (self), to the first non-abstract class. Since classes don't know - # their subclasses, we have to build the hierarchy between self and - # the record's class. - class_hierarchy = [record.class] - while class_hierarchy.first != self - class_hierarchy.insert(0, class_hierarchy.first.superclass) - end - - # Now we can work our way down the tree to the first non-abstract - # class (which has a database table to query from). - finder_class = class_hierarchy.detect { |klass| !klass.abstract_class? } - - column = finder_class.columns_hash[attr_name.to_s] - - if value.nil? - comparison_operator = "IS ?" - elsif column.text? - comparison_operator = "#{connection.case_sensitive_equality_operator} ?" - value = column.limit ? value.to_s.mb_chars[0, column.limit] : value.to_s - else - comparison_operator = "= ?" - end - - sql_attribute = "#{record.class.quoted_table_name}.#{connection.quote_column_name(attr_name)}" - - if value.nil? || (configuration[:case_sensitive] || !column.text?) - condition_sql = "#{sql_attribute} #{comparison_operator}" - condition_params = [value] - else - condition_sql = "LOWER(#{sql_attribute}) #{comparison_operator}" - condition_params = [value.mb_chars.downcase] - end - - if scope = configuration[:scope] - Array(scope).map do |scope_item| - scope_value = record.send(scope_item) - condition_sql << " AND " << attribute_condition("#{record.class.quoted_table_name}.#{scope_item}", scope_value) - condition_params << scope_value - end - end - - unless record.new_record? - condition_sql << " AND #{record.class.quoted_table_name}.#{record.class.primary_key} <> ?" - condition_params << record.send(:id) - end - - finder_class.with_exclusive_scope do - if finder_class.exists?([condition_sql, *condition_params]) - record.errors.add(attr_name, :taken, :default => configuration[:message], :value => value) - end - end - end + options = attr_names.extract_options! + validates_with UniquenessValidator, options.merge(:attributes => attr_names, :klass => self) end end end diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index d5a4d9007b..ffa6d45948 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -61,14 +61,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_with_two_tables_in_from_without_getting_double_quoted - posts = Post.find(:all, - :select => "posts.*", - :from => "authors, posts", - :include => :comments, - :conditions => "posts.author_id = authors.id", - :order => "posts.id" - ) - + posts = Post.select("posts.*").from("authors, posts").eager_load(:comments).where("posts.author_id = authors.id").order("posts.id").to_a assert_equal 2, posts.first.comments.size end @@ -469,7 +462,7 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_eager_with_has_many_and_limit_and_scoped_conditions_on_the_eagers posts = nil - Post.with_scope(:find => { + Post.send(:with_scope, :find => { :include => :comments, :conditions => "comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment'" }) do @@ -477,7 +470,7 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal 2, posts.size end - Post.with_scope(:find => { + Post.send(:with_scope, :find => { :include => [ :comments, :author ], :conditions => "authors.name = 'David' AND (comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment')" }) do @@ -487,7 +480,7 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_eager_with_has_many_and_limit_and_scoped_and_explicit_conditions_on_the_eagers - Post.with_scope(:find => { :conditions => "1=1" }) do + Post.send(:with_scope, :find => { :conditions => "1=1" }) do posts = authors(:david).posts.find(:all, :include => :comments, :conditions => "comments.body like 'Normal%' OR comments.#{QUOTED_TYPE}= 'SpecialComment'", @@ -506,7 +499,7 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_eager_with_scoped_order_using_association_limiting_without_explicit_scope posts_with_explicit_order = Post.find(:all, :conditions => 'comments.id is not null', :include => :comments, :order => 'posts.id DESC', :limit => 2) - posts_with_scoped_order = Post.with_scope(:find => {:order => 'posts.id DESC'}) do + posts_with_scoped_order = Post.send(:with_scope, :find => {:order => 'posts.id DESC'}) do Post.find(:all, :conditions => 'comments.id is not null', :include => :comments, :limit => 2) end assert_equal posts_with_explicit_order, posts_with_scoped_order diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index 5f08c40005..18a1cd3cd0 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -9,84 +9,84 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase fixtures :authors, :posts, :comments, :categories, :categories_posts, :categorizations def test_construct_finder_sql_creates_inner_joins - sql = Author.send(:construct_finder_sql, :joins => :posts) + sql = Author.joins(:posts).to_sql assert_match /INNER JOIN .?posts.? ON .?posts.?.author_id = authors.id/, sql end def test_construct_finder_sql_cascades_inner_joins - sql = Author.send(:construct_finder_sql, :joins => {:posts => :comments}) + sql = Author.joins(:posts => :comments).to_sql assert_match /INNER JOIN .?posts.? ON .?posts.?.author_id = authors.id/, sql assert_match /INNER JOIN .?comments.? ON .?comments.?.post_id = posts.id/, sql end def test_construct_finder_sql_inner_joins_through_associations - sql = Author.send(:construct_finder_sql, :joins => :categorized_posts) + sql = Author.joins(:categorized_posts).to_sql assert_match /INNER JOIN .?categorizations.?.*INNER JOIN .?posts.?/, sql end def test_construct_finder_sql_applies_association_conditions - sql = Author.send(:construct_finder_sql, :joins => :categories_like_general, :conditions => "TERMINATING_MARKER") + sql = Author.joins(:categories_like_general).where("TERMINATING_MARKER").to_sql assert_match /INNER JOIN .?categories.? ON.*AND.*.?General.?(.|\n)*TERMINATING_MARKER/, sql end def test_construct_finder_sql_applies_aliases_tables_on_association_conditions - result = Author.find(:all, :joins => [:thinking_posts, :welcome_posts]) + result = Author.joins(:thinking_posts, :welcome_posts).to_a assert_equal authors(:david), result.first end def test_construct_finder_sql_unpacks_nested_joins - sql = Author.send(:construct_finder_sql, :joins => {:posts => [[:comments]]}) + sql = Author.joins(:posts => [[:comments]]).to_sql assert_no_match /inner join.*inner join.*inner join/i, sql, "only two join clauses should be present" assert_match /INNER JOIN .?posts.? ON .?posts.?.author_id = authors.id/, sql assert_match /INNER JOIN .?comments.? ON .?comments.?.post_id = .?posts.?.id/, sql end def test_construct_finder_sql_ignores_empty_joins_hash - sql = Author.send(:construct_finder_sql, :joins => {}) + sql = Author.joins({}).to_sql assert_no_match /JOIN/i, sql end def test_construct_finder_sql_ignores_empty_joins_array - sql = Author.send(:construct_finder_sql, :joins => []) + sql = Author.joins([]).to_sql assert_no_match /JOIN/i, sql end def test_find_with_implicit_inner_joins_honors_readonly_without_select - authors = Author.find(:all, :joins => :posts) + authors = Author.joins(:posts).to_a assert !authors.empty?, "expected authors to be non-empty" assert authors.all? {|a| a.readonly? }, "expected all authors to be readonly" end def test_find_with_implicit_inner_joins_honors_readonly_with_select - authors = Author.find(:all, :select => 'authors.*', :joins => :posts) + authors = Author.joins(:posts).select('authors.*').to_a assert !authors.empty?, "expected authors to be non-empty" assert authors.all? {|a| !a.readonly? }, "expected no authors to be readonly" end def test_find_with_implicit_inner_joins_honors_readonly_false - authors = Author.find(:all, :joins => :posts, :readonly => false) + authors = Author.joins(:posts).readonly(false).to_a assert !authors.empty?, "expected authors to be non-empty" assert authors.all? {|a| !a.readonly? }, "expected no authors to be readonly" end def test_find_with_implicit_inner_joins_does_not_set_associations - authors = Author.find(:all, :select => 'authors.*', :joins => :posts) + authors = Author.joins(:posts).select('authors.*') assert !authors.empty?, "expected authors to be non-empty" assert authors.all? {|a| !a.send(:instance_variable_names).include?("@posts")}, "expected no authors to have the @posts association loaded" end def test_count_honors_implicit_inner_joins - real_count = Author.find(:all).sum{|a| a.posts.count } + real_count = Author.scoped.to_a.sum{|a| a.posts.count } assert_equal real_count, Author.count(:joins => :posts), "plain inner join count should match the number of referenced posts records" end def test_calculate_honors_implicit_inner_joins - real_count = Author.find(:all).sum{|a| a.posts.count } + real_count = Author.scoped.to_a.sum{|a| a.posts.count } assert_equal real_count, Author.calculate(:count, 'authors.id', :joins => :posts), "plain inner join count should match the number of referenced posts records" end def test_calculate_honors_implicit_inner_joins_and_distinct_and_conditions - real_count = Author.find(:all).select {|a| a.posts.any? {|p| p.title =~ /^Welcome/} }.length + real_count = Author.scoped.to_a.select {|a| a.posts.any? {|p| p.title =~ /^Welcome/} }.length authors_with_welcoming_post_titles = Author.calculate(:count, 'authors.id', :joins => :posts, :distinct => true, :conditions => "posts.title like 'Welcome%'") assert_equal real_count, authors_with_welcoming_post_titles, "inner join and conditions should have only returned authors posting titles starting with 'Welcome'" end diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 47f83db112..1d7604f52b 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -85,7 +85,7 @@ class InverseHasOneTests < ActiveRecord::TestCase fixtures :men, :faces def test_parent_instance_should_be_shared_with_child_on_find - m = Man.find(:first) + m = men(:gordon) f = m.face assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" m.name = 'Bongo' @@ -96,7 +96,7 @@ class InverseHasOneTests < ActiveRecord::TestCase def test_parent_instance_should_be_shared_with_eager_loaded_child_on_find - m = Man.find(:first, :include => :face) + m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :face) f = m.face assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" m.name = 'Bongo' @@ -104,7 +104,7 @@ class InverseHasOneTests < ActiveRecord::TestCase f.man.name = 'Mungo' assert_equal m.name, f.man.name, "Name of man should be the same after changes to child-owned instance" - m = Man.find(:first, :include => :face, :order => 'faces.id') + m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :face, :order => 'faces.id') f = m.face assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" m.name = 'Bongo' @@ -114,7 +114,7 @@ class InverseHasOneTests < ActiveRecord::TestCase end def test_parent_instance_should_be_shared_with_newly_built_child - m = Man.find(:first) + m = men(:gordon) f = m.build_face(:description => 'haunted') assert_not_nil f.man assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" @@ -125,7 +125,7 @@ class InverseHasOneTests < ActiveRecord::TestCase end def test_parent_instance_should_be_shared_with_newly_created_child - m = Man.find(:first) + m = men(:gordon) f = m.create_face(:description => 'haunted') assert_not_nil f.man assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" @@ -135,6 +135,86 @@ class InverseHasOneTests < ActiveRecord::TestCase assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" end + def test_parent_instance_should_be_shared_with_newly_created_child_via_bang_method + m = Man.find(:first) + f = m.face.create!(:description => 'haunted') + assert_not_nil f.man + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_built_child_when_we_dont_replace_existing + m = Man.find(:first) + f = m.build_face({:description => 'haunted'}, false) + assert_not_nil f.man + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to just-built-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_created_child_when_we_dont_replace_existing + m = Man.find(:first) + f = m.create_face({:description => 'haunted'}, false) + assert_not_nil f.man + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_created_child_via_bang_method_when_we_dont_replace_existing + m = Man.find(:first) + f = m.face.create!({:description => 'haunted'}, false) + assert_not_nil f.man + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_replaced_via_accessor_child + m = Man.find(:first) + f = Face.new(:description => 'haunted') + m.face = f + assert_not_nil f.man + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_replaced_via_method_child + m = Man.find(:first) + f = Face.new(:description => 'haunted') + m.face.replace(f) + assert_not_nil f.man + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_replaced_via_method_child_when_we_dont_replace_existing + m = Man.find(:first) + f = Face.new(:description => 'haunted') + m.face.replace(f, false) + assert_not_nil f.man + assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" + f.man.name = 'Mungo' + assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance" + end + def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).dirty_face } end @@ -144,7 +224,7 @@ class InverseHasManyTests < ActiveRecord::TestCase fixtures :men, :interests def test_parent_instance_should_be_shared_with_every_child_on_find - m = Man.find(:first) + m = men(:gordon) is = m.interests is.each do |i| assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" @@ -156,7 +236,7 @@ class InverseHasManyTests < ActiveRecord::TestCase end def test_parent_instance_should_be_shared_with_eager_loaded_children - m = Man.find(:first, :include => :interests) + m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :interests) is = m.interests is.each do |i| assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" @@ -166,7 +246,7 @@ class InverseHasManyTests < ActiveRecord::TestCase assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance" end - m = Man.find(:first, :include => :interests, :order => 'interests.id') + m = Man.find(:first, :conditions => {:name => 'Gordon'}, :include => :interests, :order => 'interests.id') is = m.interests is.each do |i| assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" @@ -175,11 +255,10 @@ class InverseHasManyTests < ActiveRecord::TestCase i.man.name = 'Mungo' assert_equal m.name, i.man.name, "Name of man should be the same after changes to child-owned instance" end - end def test_parent_instance_should_be_shared_with_newly_built_child - m = Man.find(:first) + m = men(:gordon) i = m.interests.build(:topic => 'Industrial Revolution Re-enactment') assert_not_nil i.man assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" @@ -189,8 +268,20 @@ class InverseHasManyTests < ActiveRecord::TestCase assert_equal m.name, i.man.name, "Name of man should be the same after changes to just-built-child-owned instance" end - def test_parent_instance_should_be_shared_with_newly_created_child + def test_parent_instance_should_be_shared_with_newly_block_style_built_child m = Man.find(:first) + i = m.interests.build {|ii| ii.topic = 'Industrial Revolution Re-enactment'} + assert_not_nil i.topic, "Child attributes supplied to build via blocks should be populated" + assert_not_nil i.man + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to just-built-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_created_child + m = men(:gordon) i = m.interests.create(:topic => 'Industrial Revolution Re-enactment') assert_not_nil i.man assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" @@ -200,8 +291,31 @@ class InverseHasManyTests < ActiveRecord::TestCase assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" end - def test_parent_instance_should_be_shared_with_poked_in_child + def test_parent_instance_should_be_shared_with_newly_created_via_bang_method_child m = Man.find(:first) + i = m.interests.create!(:topic => 'Industrial Revolution Re-enactment') + assert_not_nil i.man + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_newly_block_style_created_child + m = Man.find(:first) + i = m.interests.create {|ii| ii.topic = 'Industrial Revolution Re-enactment'} + assert_not_nil i.topic, "Child attributes supplied to create via blocks should be populated" + assert_not_nil i.man + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_poked_in_child + m = men(:gordon) i = Interest.create(:topic => 'Industrial Revolution Re-enactment') m.interests << i assert_not_nil i.man @@ -212,6 +326,30 @@ class InverseHasManyTests < ActiveRecord::TestCase assert_equal m.name, i.man.name, "Name of man should be the same after changes to newly-created-child-owned instance" end + def test_parent_instance_should_be_shared_with_replaced_via_accessor_children + m = Man.find(:first) + i = Interest.new(:topic => 'Industrial Revolution Re-enactment') + m.interests = [i] + assert_not_nil i.man + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to replaced-child-owned instance" + end + + def test_parent_instance_should_be_shared_with_replaced_via_method_children + m = Man.find(:first) + i = Interest.new(:topic => 'Industrial Revolution Re-enactment') + m.interests.replace([i]) + assert_not_nil i.man + assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" + m.name = 'Bongo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" + i.man.name = 'Mungo' + assert_equal m.name, i.man.name, "Name of man should be the same after changes to replaced-child-owned instance" + end + def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).secret_interests } end @@ -221,7 +359,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase fixtures :men, :faces, :interests def test_child_instance_should_be_shared_with_parent_on_find - f = Face.find(:first) + f = faces(:trusting) m = f.man assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" f.description = 'gormless' @@ -231,7 +369,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase end def test_eager_loaded_child_instance_should_be_shared_with_parent_on_find - f = Face.find(:first, :include => :man) + f = Face.find(:first, :include => :man, :conditions => {:description => 'trusting'}) m = f.man assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" f.description = 'gormless' @@ -239,8 +377,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase m.face.description = 'pleasing' assert_equal f.description, m.face.description, "Description of face should be the same after changes to parent-owned instance" - - f = Face.find(:first, :include => :man, :order => 'men.id') + f = Face.find(:first, :include => :man, :order => 'men.id', :conditions => {:description => 'trusting'}) m = f.man assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" f.description = 'gormless' @@ -250,7 +387,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase end def test_child_instance_should_be_shared_with_newly_built_parent - f = Face.find(:first) + f = faces(:trusting) m = f.build_man(:name => 'Charles') assert_not_nil m.face assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" @@ -261,7 +398,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase end def test_child_instance_should_be_shared_with_newly_created_parent - f = Face.find(:first) + f = faces(:trusting) m = f.create_man(:name => 'Charles') assert_not_nil m.face assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" @@ -272,7 +409,7 @@ class InverseBelongsToTests < ActiveRecord::TestCase end def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many - i = Interest.find(:first) + i = interests(:trainspotting) m = i.man assert_not_nil m.interests iz = m.interests.detect {|iz| iz.id == i.id} @@ -284,11 +421,128 @@ class InverseBelongsToTests < ActiveRecord::TestCase assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to parent-owned instance" end + def test_child_instance_should_be_shared_with_replaced_via_accessor_parent + f = Face.find(:first) + m = Man.new(:name => 'Charles') + f.man = m + assert_not_nil m.face + assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" + m.face.description = 'pleasing' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to replaced-parent-owned instance" + end + + def test_child_instance_should_be_shared_with_replaced_via_method_parent + f = faces(:trusting) + assert_not_nil f.man + m = Man.new(:name => 'Charles') + f.man.replace(m) + assert_not_nil m.face + assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" + m.face.description = 'pleasing' + assert_equal f.description, m.face.description, "Description of face should be the same after changes to replaced-parent-owned instance" + end + def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_man } end end +class InversePolymorphicBelongsToTests < ActiveRecord::TestCase + fixtures :men, :faces, :interests + + def test_child_instance_should_be_shared_with_parent_on_find + f = Face.find(:first, :conditions => {:description => 'confused'}) + m = f.polymorphic_man + assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to child instance" + m.polymorphic_face.description = 'pleasing' + assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance" + end + + def test_eager_loaded_child_instance_should_be_shared_with_parent_on_find + f = Face.find(:first, :conditions => {:description => 'confused'}, :include => :man) + m = f.polymorphic_man + assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to child instance" + m.polymorphic_face.description = 'pleasing' + assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance" + + f = Face.find(:first, :conditions => {:description => 'confused'}, :include => :man, :order => 'men.id') + m = f.polymorphic_man + assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same before changes to child instance" + f.description = 'gormless' + assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to child instance" + m.polymorphic_face.description = 'pleasing' + assert_equal f.description, m.polymorphic_face.description, "Description of face should be the same after changes to parent-owned instance" + end + + def test_child_instance_should_be_shared_with_replaced_via_accessor_parent + face = faces(:confused) + old_man = face.polymorphic_man + new_man = Man.new + + assert_not_nil face.polymorphic_man + face.polymorphic_man = new_man + + assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same before changes to parent instance" + face.description = 'Bongo' + assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to parent instance" + new_man.polymorphic_face.description = 'Mungo' + assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to replaced-parent-owned instance" + end + + def test_child_instance_should_be_shared_with_replaced_via_method_parent + face = faces(:confused) + old_man = face.polymorphic_man + new_man = Man.new + + assert_not_nil face.polymorphic_man + face.polymorphic_man.replace(new_man) + + assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same before changes to parent instance" + face.description = 'Bongo' + assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to parent instance" + new_man.polymorphic_face.description = 'Mungo' + assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to replaced-parent-owned instance" + end + + def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many + i = interests(:llama_wrangling) + m = i.polymorphic_man + assert_not_nil m.polymorphic_interests + iz = m.polymorphic_interests.detect {|iz| iz.id == i.id} + assert_not_nil iz + assert_equal i.topic, iz.topic, "Interest topics should be the same before changes to child" + i.topic = 'Eating cheese with a spoon' + assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to child" + iz.topic = 'Cow tipping' + assert_not_equal i.topic, iz.topic, "Interest topics should not be the same after changes to parent-owned instance" + end + + def test_trying_to_access_inverses_that_dont_exist_shouldnt_raise_an_error + # Ideally this would, if only for symmetry's sake with other association types + assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_polymorphic_man } + end + + def test_trying_to_set_polymorphic_inverses_that_dont_exist_at_all_should_raise_an_error + # fails because no class has the correct inverse_of for horrible_polymorphic_man + assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_polymorphic_man = Man.first } + end + + def test_trying_to_set_polymorphic_inverses_that_dont_exist_on_the_instance_being_set_should_raise_an_error + # passes because Man does have the correct inverse_of + assert_nothing_raised(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).polymorphic_man = Man.first } + # fails because Interest does have the correct inverse_of + assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).polymorphic_man = Interest.first } + end +end + # NOTE - these tests might not be meaningful, ripped as they were from the parental_control plugin # which would guess the inverse rather than look for an explicit configuration option. class InverseMultipleHasManyInversesForSameModel < ActiveRecord::TestCase diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 9164701601..803e5b25b1 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -31,11 +31,40 @@ class TestAutosaveAssociationsInGeneral < ActiveRecord::TestCase assert base.valid_keys_for_has_and_belongs_to_many_association.include?(:autosave) end + def test_should_not_add_the_same_callbacks_multiple_times_for_has_one + assert_no_difference_when_adding_callbacks_twice_for Pirate, :ship + end + + def test_should_not_add_the_same_callbacks_multiple_times_for_belongs_to + assert_no_difference_when_adding_callbacks_twice_for Ship, :pirate + end + + def test_should_not_add_the_same_callbacks_multiple_times_for_has_many + assert_no_difference_when_adding_callbacks_twice_for Pirate, :birds + end + + def test_should_not_add_the_same_callbacks_multiple_times_for_has_and_belongs_to_many + assert_no_difference_when_adding_callbacks_twice_for Pirate, :parrots + end + private def base ActiveRecord::Base end + + def assert_no_difference_when_adding_callbacks_twice_for(model, association_name) + reflection = model.reflect_on_association(association_name) + assert_no_difference "callbacks_for_model(#{model.name}).length" do + model.send(:add_autosave_association_callbacks, reflection) + end + end + + def callbacks_for_model(model) + model.instance_variables.grep(/_callbacks$/).map do |ivar| + model.instance_variable_get(ivar) + end.flatten + end end class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCase diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index b51c9f0cb3..ebb717812d 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1825,7 +1825,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_scoped_find_conditions - scoped_developers = Developer.with_scope(:find => { :conditions => 'salary > 90000' }) do + scoped_developers = Developer.send(:with_scope, :find => { :conditions => 'salary > 90000' }) do Developer.find(:all, :conditions => 'id < 5') end assert !scoped_developers.include?(developers(:david)) # David's salary is less than 90,000 @@ -1833,7 +1833,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_scoped_find_limit_offset - scoped_developers = Developer.with_scope(:find => { :limit => 3, :offset => 2 }) do + scoped_developers = Developer.send(:with_scope, :find => { :limit => 3, :offset => 2 }) do Developer.find(:all, :order => 'id') end assert !scoped_developers.include?(developers(:david)) @@ -1847,17 +1847,17 @@ class BasicsTest < ActiveRecord::TestCase def test_scoped_find_order # Test order in scope - scoped_developers = Developer.with_scope(:find => { :limit => 1, :order => 'salary DESC' }) do + scoped_developers = Developer.send(:with_scope, :find => { :limit => 1, :order => 'salary DESC' }) do Developer.find(:all) end assert_equal 'Jamis', scoped_developers.first.name assert scoped_developers.include?(developers(:jamis)) # Test scope without order and order in find - scoped_developers = Developer.with_scope(:find => { :limit => 1 }) do + scoped_developers = Developer.send(:with_scope, :find => { :limit => 1 }) do Developer.find(:all, :order => 'salary DESC') end # Test scope order + find order, find has priority - scoped_developers = Developer.with_scope(:find => { :limit => 3, :order => 'id DESC' }) do + scoped_developers = Developer.send(:with_scope, :find => { :limit => 3, :order => 'id DESC' }) do Developer.find(:all, :order => 'salary ASC') end assert scoped_developers.include?(developers(:poor_jamis)) @@ -1869,7 +1869,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_scoped_find_limit_offset_including_has_many_association - topics = Topic.with_scope(:find => {:limit => 1, :offset => 1, :include => :replies}) do + topics = Topic.send(:with_scope, :find => {:limit => 1, :offset => 1, :include => :replies}) do Topic.find(:all, :order => "topics.id") end assert_equal 1, topics.size @@ -1877,7 +1877,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_scoped_find_order_including_has_many_association - developers = Developer.with_scope(:find => { :order => 'developers.salary DESC', :include => :projects }) do + developers = Developer.send(:with_scope, :find => { :order => 'developers.salary DESC', :include => :projects }) do Developer.find(:all) end assert developers.size >= 2 @@ -1887,7 +1887,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_scoped_find_with_group_and_having - developers = Developer.with_scope(:find => { :group => 'developers.salary', :having => "SUM(salary) > 10000", :select => "SUM(salary) as salary" }) do + developers = Developer.send(:with_scope, :find => { :group => 'developers.salary', :having => "SUM(salary) > 10000", :select => "SUM(salary) as salary" }) do Developer.find(:all) end assert_equal 3, developers.size @@ -1933,7 +1933,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_find_scoped_ordered_last - last_developer = Developer.with_scope(:find => { :order => 'developers.salary ASC' }) do + last_developer = Developer.send(:with_scope, :find => { :order => 'developers.salary ASC' }) do Developer.find(:last) end assert_equal last_developer, Developer.find(:all, :order => 'developers.salary ASC').last diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 004f4d0ea6..bd2d471fc7 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -29,8 +29,8 @@ class CalculationsTest < ActiveRecord::TestCase end def test_type_cast_calculated_value_should_convert_db_averages_of_fixnum_class_to_decimal - assert_equal 0, NumericData.send(:type_cast_calculated_value, 0, nil, 'avg') - assert_equal 53.0, NumericData.send(:type_cast_calculated_value, 53, nil, 'avg') + assert_equal 0, NumericData.scoped.send(:type_cast_calculated_value, 0, nil, 'avg') + assert_equal 53.0, NumericData.scoped.send(:type_cast_calculated_value, 53, nil, 'avg') end def test_should_get_maximum_of_field @@ -42,7 +42,7 @@ class CalculationsTest < ActiveRecord::TestCase end def test_should_get_maximum_of_field_with_scoped_include - Account.with_scope :find => { :include => :firm, :conditions => "companies.name != 'Summit'" } do + Account.send :with_scope, :find => { :include => :firm, :conditions => "companies.name != 'Summit'" } do assert_equal 50, Account.maximum(:credit_limit) end end @@ -248,17 +248,15 @@ class CalculationsTest < ActiveRecord::TestCase def test_should_reject_invalid_options assert_nothing_raised do - [:count, :sum].each do |func| - # empty options are valid - Company.send(:validate_calculation_options, func) - # these options are valid for all calculations - [:select, :conditions, :joins, :order, :group, :having, :distinct].each do |opt| - Company.send(:validate_calculation_options, func, opt => true) - end + # 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, :count, :include => true) + Company.send(:validate_calculation_options, :include => true) end assert_raise(ArgumentError) { Company.send(:validate_calculation_options, :sum, :foo => :bar) } diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index c531a2dec1..d2451f24c1 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -120,7 +120,7 @@ class FinderTest < ActiveRecord::TestCase end def test_exists_with_scoped_include - Developer.with_scope(:find => { :include => :projects, :order => "projects.name" }) do + Developer.send(:with_scope, :find => { :include => :projects, :order => "projects.name" }) do assert Developer.exists? end end @@ -1022,8 +1022,8 @@ class FinderTest < ActiveRecord::TestCase def test_finder_with_scoped_from all_topics = Topic.find(:all) - Topic.with_scope(:find => { :from => 'fake_topics' }) do - assert_equal all_topics, Topic.all(:from => 'topics').to_a + Topic.send(:with_scope, :find => { :from => 'fake_topics' }) do + assert_equal all_topics, Topic.from('topics').to_a end end diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 307320b964..479970b2fa 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -47,11 +47,6 @@ ActiveRecord::Base.connection.class.class_eval do alias_method_chain :execute, :query_record end -# Make with_scope public for tests -class << ActiveRecord::Base - public :with_scope, :with_exclusive_scope -end - unless ENV['FIXTURE_DEBUG'] module ActiveRecord::TestFixtures::ClassMethods def try_to_load_dependency_with_silence(*args) @@ -62,9 +57,10 @@ unless ENV['FIXTURE_DEBUG'] end end +require "cases/validations_repair_helper" class ActiveSupport::TestCase include ActiveRecord::TestFixtures - include ActiveModel::ValidationsRepairHelper + include ActiveRecord::ValidationsRepairHelper self.fixture_path = FIXTURES_ROOT self.use_instantiated_fixtures = false diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index a64c01292f..dfaecf35cf 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -225,7 +225,7 @@ unless current_adapter?(:SybaseAdapter, :OpenBaseAdapter) def test_sane_find_with_scoped_lock assert_nothing_raised do Person.transaction do - Person.with_scope(:find => { :lock => true }) do + Person.send(:with_scope, :find => { :lock => true }) do Person.find 1 end end diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index eb4ce0e774..cfc6f8772c 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -10,19 +10,19 @@ class MethodScopingTest < ActiveRecord::TestCase fixtures :authors, :developers, :projects, :comments, :posts, :developers_projects def test_set_conditions - Developer.with_scope(:find => { :conditions => 'just a test...' }) do + Developer.send(:with_scope, :find => { :conditions => 'just a test...' }) do assert_equal 'just a test...', Developer.send(:current_scoped_methods)[:find][:conditions] end end def test_scoped_find - Developer.with_scope(:find => { :conditions => "name = 'David'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do assert_nothing_raised { Developer.find(1) } end end def test_scoped_find_first - Developer.with_scope(:find => { :conditions => "salary = 100000" }) do + Developer.send(:with_scope, :find => { :conditions => "salary = 100000" }) do assert_equal Developer.find(10), Developer.find(:first, :order => 'name') end end @@ -30,7 +30,7 @@ class MethodScopingTest < ActiveRecord::TestCase def test_scoped_find_last highest_salary = Developer.find(:first, :order => "salary DESC") - Developer.with_scope(:find => { :order => "salary" }) do + Developer.send(:with_scope, :find => { :order => "salary" }) do assert_equal highest_salary, Developer.last end end @@ -39,38 +39,38 @@ class MethodScopingTest < ActiveRecord::TestCase lowest_salary = Developer.find(:first, :order => "salary ASC") highest_salary = Developer.find(:first, :order => "salary DESC") - Developer.with_scope(:find => { :order => "salary" }) do + Developer.send(:with_scope, :find => { :order => "salary" }) do assert_equal highest_salary, Developer.last assert_equal lowest_salary, Developer.first end end def test_scoped_find_combines_conditions - Developer.with_scope(:find => { :conditions => "salary = 9000" }) do + Developer.send(:with_scope, :find => { :conditions => "salary = 9000" }) do assert_equal developers(:poor_jamis), Developer.find(:first, :conditions => "name = 'Jamis'") end end def test_scoped_find_sanitizes_conditions - Developer.with_scope(:find => { :conditions => ['salary = ?', 9000] }) do + Developer.send(:with_scope, :find => { :conditions => ['salary = ?', 9000] }) do assert_equal developers(:poor_jamis), Developer.find(:first) end end def test_scoped_find_combines_and_sanitizes_conditions - Developer.with_scope(:find => { :conditions => ['salary = ?', 9000] }) do + Developer.send(:with_scope, :find => { :conditions => ['salary = ?', 9000] }) do assert_equal developers(:poor_jamis), Developer.find(:first, :conditions => ['name = ?', 'Jamis']) end end def test_scoped_find_all - Developer.with_scope(:find => { :conditions => "name = 'David'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do assert_equal [developers(:david)], Developer.find(:all) end end def test_scoped_find_select - Developer.with_scope(:find => { :select => "id, name" }) do + Developer.send(:with_scope, :find => { :select => "id, name" }) do developer = Developer.find(:first, :conditions => "name = 'David'") assert_equal "David", developer.name assert !developer.has_attribute?(:salary) @@ -78,7 +78,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_options_select_replaces_scope_select - Developer.with_scope(:find => { :select => "id, name" }) do + Developer.send(:with_scope, :find => { :select => "id, name" }) do developer = Developer.find(:first, :select => 'id, salary', :conditions => "name = 'David'") assert_equal 80000, developer.salary assert !developer.has_attribute?(:name) @@ -86,11 +86,11 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_count - Developer.with_scope(:find => { :conditions => "name = 'David'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do assert_equal 1, Developer.count end - Developer.with_scope(:find => { :conditions => 'salary = 100000' }) do + Developer.send(:with_scope, :find => { :conditions => 'salary = 100000' }) do assert_equal 8, Developer.count assert_equal 1, Developer.count(:conditions => "name LIKE 'fixture_1%'") end @@ -98,7 +98,7 @@ class MethodScopingTest < ActiveRecord::TestCase def test_scoped_find_include # with the include, will retrieve only developers for the given project - scoped_developers = Developer.with_scope(:find => { :include => :projects }) do + scoped_developers = Developer.send(:with_scope, :find => { :include => :projects }) do Developer.find(:all, :conditions => 'projects.id = 2') end assert scoped_developers.include?(developers(:david)) @@ -107,7 +107,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_find_joins - scoped_developers = Developer.with_scope(:find => { :joins => 'JOIN developers_projects ON id = developer_id' } ) do + scoped_developers = Developer.send(:with_scope, :find => { :joins => 'JOIN developers_projects ON id = developer_id' } ) do Developer.find(:all, :conditions => 'developers_projects.project_id = 2') end assert scoped_developers.include?(developers(:david)) @@ -117,7 +117,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_find_using_new_style_joins - scoped_developers = Developer.with_scope(:find => { :joins => :projects }) do + scoped_developers = Developer.send(:with_scope, :find => { :joins => :projects }) do Developer.find(:all, :conditions => 'projects.id = 2') end assert scoped_developers.include?(developers(:david)) @@ -127,7 +127,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_find_merges_old_style_joins - scoped_authors = Author.with_scope(:find => { :joins => 'INNER JOIN posts ON authors.id = posts.author_id ' }) do + scoped_authors = Author.send(:with_scope, :find => { :joins => 'INNER JOIN posts ON authors.id = posts.author_id ' }) do Author.find(:all, :select => 'DISTINCT authors.*', :joins => 'INNER JOIN comments ON posts.id = comments.post_id', :conditions => 'comments.id = 1') end assert scoped_authors.include?(authors(:david)) @@ -137,7 +137,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_find_merges_new_style_joins - scoped_authors = Author.with_scope(:find => { :joins => :posts }) do + scoped_authors = Author.send(:with_scope, :find => { :joins => :posts }) do Author.find(:all, :select => 'DISTINCT authors.*', :joins => :comments, :conditions => 'comments.id = 1') end assert scoped_authors.include?(authors(:david)) @@ -147,7 +147,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_find_merges_new_and_old_style_joins - scoped_authors = Author.with_scope(:find => { :joins => :posts }) do + scoped_authors = Author.send(:with_scope, :find => { :joins => :posts }) do Author.find(:all, :select => 'DISTINCT authors.*', :joins => 'JOIN comments ON posts.id = comments.post_id', :conditions => 'comments.id = 1') end assert scoped_authors.include?(authors(:david)) @@ -157,7 +157,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_find_merges_string_array_style_and_string_style_joins - scoped_authors = Author.with_scope(:find => { :joins => ["INNER JOIN posts ON posts.author_id = authors.id"]}) do + scoped_authors = Author.send(:with_scope, :find => { :joins => ["INNER JOIN posts ON posts.author_id = authors.id"]}) do Author.find(:all, :select => 'DISTINCT authors.*', :joins => 'INNER JOIN comments ON posts.id = comments.post_id', :conditions => 'comments.id = 1') end assert scoped_authors.include?(authors(:david)) @@ -167,7 +167,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_find_merges_string_array_style_and_hash_style_joins - scoped_authors = Author.with_scope(:find => { :joins => :posts}) do + scoped_authors = Author.send(:with_scope, :find => { :joins => :posts}) do Author.find(:all, :select => 'DISTINCT authors.*', :joins => ['INNER JOIN comments ON posts.id = comments.post_id'], :conditions => 'comments.id = 1') end assert scoped_authors.include?(authors(:david)) @@ -177,7 +177,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_find_merges_joins_and_eliminates_duplicate_string_joins - scoped_authors = Author.with_scope(:find => { :joins => 'INNER JOIN posts ON posts.author_id = authors.id'}) do + scoped_authors = Author.send(:with_scope, :find => { :joins => 'INNER JOIN posts ON posts.author_id = authors.id'}) do Author.find(:all, :select => 'DISTINCT authors.*', :joins => ["INNER JOIN posts ON posts.author_id = authors.id", "INNER JOIN comments ON posts.id = comments.post_id"], :conditions => 'comments.id = 1') end assert scoped_authors.include?(authors(:david)) @@ -187,7 +187,7 @@ class MethodScopingTest < ActiveRecord::TestCase end def test_scoped_find_strips_spaces_from_string_joins_and_eliminates_duplicate_string_joins - scoped_authors = Author.with_scope(:find => { :joins => ' INNER JOIN posts ON posts.author_id = authors.id '}) do + scoped_authors = Author.send(:with_scope, :find => { :joins => ' INNER JOIN posts ON posts.author_id = authors.id '}) do Author.find(:all, :select => 'DISTINCT authors.*', :joins => ['INNER JOIN posts ON posts.author_id = authors.id'], :conditions => 'posts.id = 1') end assert scoped_authors.include?(authors(:david)) @@ -198,7 +198,7 @@ class MethodScopingTest < ActiveRecord::TestCase def test_scoped_count_include # with the include, will retrieve only developers for the given project - Developer.with_scope(:find => { :include => :projects }) do + Developer.send(:with_scope, :find => { :include => :projects }) do assert_equal 1, Developer.count(:conditions => 'projects.id = 2') end end @@ -206,7 +206,7 @@ class MethodScopingTest < ActiveRecord::TestCase def test_scoped_create new_comment = nil - VerySpecialComment.with_scope(:create => { :post_id => 1 }) do + VerySpecialComment.send(:with_scope, :create => { :post_id => 1 }) do assert_equal({ :post_id => 1 }, VerySpecialComment.send(:current_scoped_methods)[:create]) new_comment = VerySpecialComment.create :body => "Wonderful world" end @@ -216,14 +216,14 @@ class MethodScopingTest < ActiveRecord::TestCase def test_immutable_scope options = { :conditions => "name = 'David'" } - Developer.with_scope(:find => options) do + Developer.send(:with_scope, :find => options) do assert_equal %w(David), Developer.find(:all).map { |d| d.name } options[:conditions] = "name != 'David'" assert_equal %w(David), Developer.find(:all).map { |d| d.name } end scope = { :find => { :conditions => "name = 'David'" }} - Developer.with_scope(scope) do + Developer.send(:with_scope, scope) do assert_equal %w(David), Developer.find(:all).map { |d| d.name } scope[:find][:conditions] = "name != 'David'" assert_equal %w(David), Developer.find(:all).map { |d| d.name } @@ -232,7 +232,7 @@ class MethodScopingTest < ActiveRecord::TestCase def test_scoped_with_duck_typing scoping = Struct.new(:method_scoping).new(:find => { :conditions => ["name = ?", 'David'] }) - Developer.with_scope(scoping) do + Developer.send(:with_scope, scoping) do assert_equal %w(David), Developer.find(:all).map { |d| d.name } end end @@ -241,7 +241,7 @@ class MethodScopingTest < ActiveRecord::TestCase scoped_methods = Developer.instance_eval('current_scoped_methods') begin - Developer.with_scope(:find => { :conditions => "name = 'Jamis'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'Jamis'" }) do raise "an exception" end rescue @@ -254,8 +254,8 @@ class NestedScopingTest < ActiveRecord::TestCase fixtures :authors, :developers, :projects, :comments, :posts def test_merge_options - Developer.with_scope(:find => { :conditions => 'salary = 80000' }) do - Developer.with_scope(:find => { :limit => 10 }) do + Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do + Developer.send(:with_scope, :find => { :limit => 10 }) do merged_option = Developer.instance_eval('current_scoped_methods')[:find] assert_equal({ :conditions => 'salary = 80000', :limit => 10 }, merged_option) end @@ -263,8 +263,8 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_merge_inner_scope_has_priority - Developer.with_scope(:find => { :limit => 5 }) do - Developer.with_scope(:find => { :limit => 10 }) do + Developer.send(:with_scope, :find => { :limit => 5 }) do + Developer.send(:with_scope, :find => { :limit => 10 }) do merged_option = Developer.instance_eval('current_scoped_methods')[:find] assert_equal({ :limit => 10 }, merged_option) end @@ -272,8 +272,8 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_replace_options - Developer.with_scope(:find => { :conditions => "name = 'David'" }) do - Developer.with_exclusive_scope(:find => { :conditions => "name = 'Jamis'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do + Developer.send(:with_exclusive_scope, :find => { :conditions => "name = 'Jamis'" }) do assert_equal({:find => { :conditions => "name = 'Jamis'" }}, Developer.instance_eval('current_scoped_methods')) assert_equal({:find => { :conditions => "name = 'Jamis'" }}, Developer.send(:scoped_methods)[-1]) end @@ -281,21 +281,21 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_append_conditions - Developer.with_scope(:find => { :conditions => "name = 'David'" }) do - Developer.with_scope(:find => { :conditions => 'salary = 80000' }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do + Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do appended_condition = Developer.instance_eval('current_scoped_methods')[:find][:conditions] assert_equal("(name = 'David') AND (salary = 80000)", appended_condition) assert_equal(1, Developer.count) end - Developer.with_scope(:find => { :conditions => "name = 'Maiha'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'Maiha'" }) do assert_equal(0, Developer.count) end end end def test_merge_and_append_options - Developer.with_scope(:find => { :conditions => 'salary = 80000', :limit => 10 }) do - Developer.with_scope(:find => { :conditions => "name = 'David'" }) do + Developer.send(:with_scope, :find => { :conditions => 'salary = 80000', :limit => 10 }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do merged_option = Developer.instance_eval('current_scoped_methods')[:find] assert_equal({ :conditions => "(salary = 80000) AND (name = 'David')", :limit => 10 }, merged_option) end @@ -303,8 +303,8 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_nested_scoped_find - Developer.with_scope(:find => { :conditions => "name = 'Jamis'" }) do - Developer.with_exclusive_scope(:find => { :conditions => "name = 'David'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'Jamis'" }) do + Developer.send(:with_exclusive_scope, :find => { :conditions => "name = 'David'" }) do assert_nothing_raised { Developer.find(1) } assert_equal('David', Developer.find(:first).name) end @@ -313,8 +313,8 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_nested_scoped_find_include - Developer.with_scope(:find => { :include => :projects }) do - Developer.with_scope(:find => { :conditions => "projects.id = 2" }) do + Developer.send(:with_scope, :find => { :include => :projects }) do + Developer.send(:with_scope, :find => { :conditions => "projects.id = 2" }) do assert_nothing_raised { Developer.find(1) } assert_equal('David', Developer.find(:first).name) end @@ -323,24 +323,24 @@ class NestedScopingTest < ActiveRecord::TestCase def test_nested_scoped_find_merged_include # :include's remain unique and don't "double up" when merging - Developer.with_scope(:find => { :include => :projects, :conditions => "projects.id = 2" }) do - Developer.with_scope(:find => { :include => :projects }) do + Developer.send(:with_scope, :find => { :include => :projects, :conditions => "projects.id = 2" }) do + Developer.send(:with_scope, :find => { :include => :projects }) do assert_equal 1, Developer.instance_eval('current_scoped_methods')[:find][:include].length assert_equal('David', Developer.find(:first).name) end end # the nested scope doesn't remove the first :include - Developer.with_scope(:find => { :include => :projects, :conditions => "projects.id = 2" }) do - Developer.with_scope(:find => { :include => [] }) do + Developer.send(:with_scope, :find => { :include => :projects, :conditions => "projects.id = 2" }) do + Developer.send(:with_scope, :find => { :include => [] }) do assert_equal 1, Developer.instance_eval('current_scoped_methods')[:find][:include].length assert_equal('David', Developer.find(:first).name) end end # mixing array and symbol include's will merge correctly - Developer.with_scope(:find => { :include => [:projects], :conditions => "projects.id = 2" }) do - Developer.with_scope(:find => { :include => :projects }) do + Developer.send(:with_scope, :find => { :include => [:projects], :conditions => "projects.id = 2" }) do + Developer.send(:with_scope, :find => { :include => :projects }) do assert_equal 1, Developer.instance_eval('current_scoped_methods')[:find][:include].length assert_equal('David', Developer.find(:first).name) end @@ -348,21 +348,21 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_nested_scoped_find_replace_include - Developer.with_scope(:find => { :include => :projects }) do - Developer.with_exclusive_scope(:find => { :include => [] }) do + Developer.send(:with_scope, :find => { :include => :projects }) do + Developer.send(:with_exclusive_scope, :find => { :include => [] }) do assert_equal 0, Developer.instance_eval('current_scoped_methods')[:find][:include].length end end end def test_three_level_nested_exclusive_scoped_find - Developer.with_scope(:find => { :conditions => "name = 'Jamis'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'Jamis'" }) do assert_equal('Jamis', Developer.find(:first).name) - Developer.with_exclusive_scope(:find => { :conditions => "name = 'David'" }) do + Developer.send(:with_exclusive_scope, :find => { :conditions => "name = 'David'" }) do assert_equal('David', Developer.find(:first).name) - Developer.with_exclusive_scope(:find => { :conditions => "name = 'Maiha'" }) do + Developer.send(:with_exclusive_scope, :find => { :conditions => "name = 'Maiha'" }) do assert_equal(nil, Developer.find(:first)) end @@ -377,8 +377,8 @@ class NestedScopingTest < ActiveRecord::TestCase def test_merged_scoped_find poor_jamis = developers(:poor_jamis) - Developer.with_scope(:find => { :conditions => "salary < 100000" }) do - Developer.with_scope(:find => { :offset => 1, :order => 'id asc' }) do + Developer.send(:with_scope, :find => { :conditions => "salary < 100000" }) do + Developer.send(:with_scope, :find => { :offset => 1, :order => 'id asc' }) do # Oracle adapter does not generated space after asc therefore trailing space removed from regex assert_sql /ORDER BY id asc/ do assert_equal(poor_jamis, Developer.find(:first, :order => 'id asc')) @@ -388,16 +388,16 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_merged_scoped_find_sanitizes_conditions - Developer.with_scope(:find => { :conditions => ["name = ?", 'David'] }) do - Developer.with_scope(:find => { :conditions => ['salary = ?', 9000] }) do + Developer.send(:with_scope, :find => { :conditions => ["name = ?", 'David'] }) do + Developer.send(:with_scope, :find => { :conditions => ['salary = ?', 9000] }) do assert_raise(ActiveRecord::RecordNotFound) { developers(:poor_jamis) } end end end def test_nested_scoped_find_combines_and_sanitizes_conditions - Developer.with_scope(:find => { :conditions => ["name = ?", 'David'] }) do - Developer.with_exclusive_scope(:find => { :conditions => ['salary = ?', 9000] }) do + Developer.send(:with_scope, :find => { :conditions => ["name = ?", 'David'] }) do + Developer.send(:with_exclusive_scope, :find => { :conditions => ['salary = ?', 9000] }) do assert_equal developers(:poor_jamis), Developer.find(:first) assert_equal developers(:poor_jamis), Developer.find(:first, :conditions => ['name = ?', 'Jamis']) end @@ -405,8 +405,8 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_merged_scoped_find_combines_and_sanitizes_conditions - Developer.with_scope(:find => { :conditions => ["name = ?", 'David'] }) do - Developer.with_scope(:find => { :conditions => ['salary > ?', 9000] }) do + Developer.send(:with_scope, :find => { :conditions => ["name = ?", 'David'] }) do + Developer.send(:with_scope, :find => { :conditions => ['salary > ?', 9000] }) do assert_equal %w(David), Developer.find(:all).map { |d| d.name } end end @@ -414,8 +414,8 @@ class NestedScopingTest < ActiveRecord::TestCase def test_nested_scoped_create comment = nil - Comment.with_scope(:create => { :post_id => 1}) do - Comment.with_scope(:create => { :post_id => 2}) do + Comment.send(:with_scope, :create => { :post_id => 1}) do + Comment.send(:with_scope, :create => { :post_id => 2}) do assert_equal({ :post_id => 2 }, Comment.send(:current_scoped_methods)[:create]) comment = Comment.create :body => "Hey guys, nested scopes are broken. Please fix!" end @@ -425,8 +425,8 @@ class NestedScopingTest < ActiveRecord::TestCase def test_nested_exclusive_scope_for_create comment = nil - Comment.with_scope(:create => { :body => "Hey guys, nested scopes are broken. Please fix!" }) do - Comment.with_exclusive_scope(:create => { :post_id => 1 }) do + Comment.send(:with_scope, :create => { :body => "Hey guys, nested scopes are broken. Please fix!" }) do + Comment.send(:with_exclusive_scope, :create => { :post_id => 1 }) do assert_equal({ :post_id => 1 }, Comment.send(:current_scoped_methods)[:create]) comment = Comment.create :body => "Hey guys" end @@ -437,8 +437,8 @@ class NestedScopingTest < ActiveRecord::TestCase def test_merged_scoped_find_on_blank_conditions [nil, " ", [], {}].each do |blank| - Developer.with_scope(:find => {:conditions => blank}) do - Developer.with_scope(:find => {:conditions => blank}) do + Developer.send(:with_scope, :find => {:conditions => blank}) do + Developer.send(:with_scope, :find => {:conditions => blank}) do assert_nothing_raised { Developer.find(:first) } end end @@ -447,8 +447,8 @@ class NestedScopingTest < ActiveRecord::TestCase def test_merged_scoped_find_on_blank_bind_conditions [ [""], ["",{}] ].each do |blank| - Developer.with_scope(:find => {:conditions => blank}) do - Developer.with_scope(:find => {:conditions => blank}) do + Developer.send(:with_scope, :find => {:conditions => blank}) do + Developer.send(:with_scope, :find => {:conditions => blank}) do assert_nothing_raised { Developer.find(:first) } end end @@ -458,8 +458,8 @@ class NestedScopingTest < ActiveRecord::TestCase def test_immutable_nested_scope options1 = { :conditions => "name = 'Jamis'" } options2 = { :conditions => "name = 'David'" } - Developer.with_scope(:find => options1) do - Developer.with_exclusive_scope(:find => options2) do + Developer.send(:with_scope, :find => options1) do + Developer.send(:with_exclusive_scope, :find => options2) do assert_equal %w(David), Developer.find(:all).map { |d| d.name } options1[:conditions] = options2[:conditions] = nil assert_equal %w(David), Developer.find(:all).map { |d| d.name } @@ -470,8 +470,8 @@ class NestedScopingTest < ActiveRecord::TestCase def test_immutable_merged_scope options1 = { :conditions => "name = 'Jamis'" } options2 = { :conditions => "salary > 10000" } - Developer.with_scope(:find => options1) do - Developer.with_scope(:find => options2) do + Developer.send(:with_scope, :find => options1) do + Developer.send(:with_scope, :find => options2) do assert_equal %w(Jamis), Developer.find(:all).map { |d| d.name } options1[:conditions] = options2[:conditions] = nil assert_equal %w(Jamis), Developer.find(:all).map { |d| d.name } @@ -480,10 +480,10 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_ensure_that_method_scoping_is_correctly_restored - Developer.with_scope(:find => { :conditions => "name = 'David'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do scoped_methods = Developer.instance_eval('current_scoped_methods') begin - Developer.with_scope(:find => { :conditions => "name = 'Maiha'" }) do + Developer.send(:with_scope, :find => { :conditions => "name = 'Maiha'" }) do raise "an exception" end rescue @@ -493,8 +493,8 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_nested_scoped_find_merges_old_style_joins - scoped_authors = Author.with_scope(:find => { :joins => 'INNER JOIN posts ON authors.id = posts.author_id' }) do - Author.with_scope(:find => { :joins => 'INNER JOIN comments ON posts.id = comments.post_id' }) do + scoped_authors = Author.send(:with_scope, :find => { :joins => 'INNER JOIN posts ON authors.id = posts.author_id' }) do + Author.send(:with_scope, :find => { :joins => 'INNER JOIN comments ON posts.id = comments.post_id' }) do Author.find(:all, :select => 'DISTINCT authors.*', :conditions => 'comments.id = 1') end end @@ -505,8 +505,8 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_nested_scoped_find_merges_new_style_joins - scoped_authors = Author.with_scope(:find => { :joins => :posts }) do - Author.with_scope(:find => { :joins => :comments }) do + scoped_authors = Author.send(:with_scope, :find => { :joins => :posts }) do + Author.send(:with_scope, :find => { :joins => :comments }) do Author.find(:all, :select => 'DISTINCT authors.*', :conditions => 'comments.id = 1') end end @@ -517,8 +517,8 @@ class NestedScopingTest < ActiveRecord::TestCase end def test_nested_scoped_find_merges_new_and_old_style_joins - scoped_authors = Author.with_scope(:find => { :joins => :posts }) do - Author.with_scope(:find => { :joins => 'INNER JOIN comments ON posts.id = comments.post_id' }) do + scoped_authors = Author.send(:with_scope, :find => { :joins => :posts }) do + Author.send(:with_scope, :find => { :joins => 'INNER JOIN comments ON posts.id = comments.post_id' }) do Author.find(:all, :select => 'DISTINCT authors.*', :joins => '', :conditions => 'comments.id = 1') end end @@ -552,7 +552,7 @@ class HasManyScopingTest< ActiveRecord::TestCase end def test_nested_scope - Comment.with_scope(:find => { :conditions => '1=1' }) do + Comment.send(:with_scope, :find => { :conditions => '1=1' }) do assert_equal 'a comment...', @welcome.comments.what_are_you end end @@ -577,7 +577,7 @@ class HasAndBelongsToManyScopingTest< ActiveRecord::TestCase end def test_nested_scope - Category.with_scope(:find => { :conditions => '1=1' }) do + Category.send(:with_scope, :find => { :conditions => '1=1' }) do assert_equal 'a comment...', @welcome.comments.what_are_you end end @@ -633,7 +633,7 @@ class DefaultScopingTest < ActiveRecord::TestCase def test_nested_scope expected = Developer.find(:all, :order => 'name DESC').collect { |dev| dev.salary } - received = DeveloperOrderedBySalary.with_scope(:find => { :order => 'name DESC'}) do + received = DeveloperOrderedBySalary.send(:with_scope, :find => { :order => 'name DESC'}) do DeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } end assert_equal expected, received @@ -647,7 +647,7 @@ class DefaultScopingTest < ActiveRecord::TestCase def test_nested_exclusive_scope expected = Developer.find(:all, :limit => 100).collect { |dev| dev.salary } - received = DeveloperOrderedBySalary.with_exclusive_scope(:find => { :limit => 100 }) do + received = DeveloperOrderedBySalary.send(:with_exclusive_scope, :find => { :limit => 100 }) do DeveloperOrderedBySalary.find(:all).collect { |dev| dev.salary } end assert_equal expected, received diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 53fd168e1b..60c5bad225 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -245,6 +245,27 @@ class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase def test_should_automatically_enable_autosave_on_the_association assert Pirate.reflect_on_association(:ship).options[:autosave] end + + def test_should_accept_update_only_option + @pirate.update_attribute(:update_only_ship_attributes, { :id => @pirate.ship.id, :name => 'Mayflower' }) + end + + def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true + @ship.delete + assert_difference('Ship.count', 1) do + @pirate.reload.update_attribute(:update_only_ship_attributes, { :name => 'Mayflower' }) + end + end + + def test_should_update_existing_when_update_only_is_true_and_no_id_is_given + @ship.delete + @ship = @pirate.create_update_only_ship(:name => 'Nights Dirty Lightning') + + assert_no_difference('Ship.count') do + @pirate.update_attributes(:update_only_ship_attributes => { :name => 'Mayflower' }) + end + assert_equal 'Mayflower', @ship.reload.name + end end class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase @@ -362,6 +383,27 @@ class TestNestedAttributesOnABelongsToAssociation < ActiveRecord::TestCase def test_should_automatically_enable_autosave_on_the_association assert Ship.reflect_on_association(:pirate).options[:autosave] end + + def test_should_accept_update_only_option + @ship.update_attribute(:update_only_pirate_attributes, { :id => @pirate.ship.id, :catchphrase => 'Arr' }) + end + + def test_should_create_new_model_when_nothing_is_there_and_update_only_is_true + @pirate.delete + assert_difference('Pirate.count', 1) do + @ship.reload.update_attribute(:update_only_pirate_attributes, { :catchphrase => 'Arr' }) + end + end + + def test_should_update_existing_when_update_only_is_true_and_no_id_is_given + @pirate.delete + @pirate = @ship.create_update_only_pirate(:catchphrase => 'Aye') + + assert_no_difference('Pirate.count') do + @ship.update_attributes(:update_only_pirate_attributes => { :catchphrase => 'Arr' }) + end + assert_equal 'Arr', @pirate.reload.catchphrase + end end module NestedAttributesOnACollectionAssociationTests @@ -371,6 +413,15 @@ module NestedAttributesOnACollectionAssociationTests assert_respond_to @pirate, association_setter end + def test_should_save_only_one_association_on_create + pirate = Pirate.create!({ + :catchphrase => 'Arr', + association_getter => { 'foo' => { :name => 'Grace OMalley' } } + }) + + assert_equal 1, pirate.reload.send(@association_name).count + end + def test_should_take_a_hash_with_string_keys_and_assign_the_attributes_to_the_associated_models @alternate_params[association_getter].stringify_keys! @pirate.update_attributes @alternate_params diff --git a/activerecord/test/cases/readonly_test.rb b/activerecord/test/cases/readonly_test.rb index b921cbdc9c..98011f40a4 100644 --- a/activerecord/test/cases/readonly_test.rb +++ b/activerecord/test/cases/readonly_test.rb @@ -33,19 +33,20 @@ class ReadOnlyTest < ActiveRecord::TestCase def test_find_with_readonly_option Developer.find(:all).each { |d| assert !d.readonly? } - Developer.find(:all, :readonly => false).each { |d| assert !d.readonly? } - Developer.find(:all, :readonly => true).each { |d| assert d.readonly? } + Developer.readonly(false).each { |d| assert !d.readonly? } + Developer.readonly(true).each { |d| assert d.readonly? } + Developer.readonly.each { |d| assert d.readonly? } end def test_find_with_joins_option_implies_readonly # Blank joins don't count. - Developer.find(:all, :joins => ' ').each { |d| assert !d.readonly? } - Developer.find(:all, :joins => ' ', :readonly => false).each { |d| assert !d.readonly? } + Developer.joins(' ').each { |d| assert !d.readonly? } + Developer.joins(' ').readonly(false).each { |d| assert !d.readonly? } # Others do. - Developer.find(:all, :joins => ', projects').each { |d| assert d.readonly? } - Developer.find(:all, :joins => ', projects', :readonly => false).each { |d| assert !d.readonly? } + Developer.joins(', projects').each { |d| assert d.readonly? } + Developer.joins(', projects').readonly(false).each { |d| assert !d.readonly? } end @@ -54,7 +55,7 @@ class ReadOnlyTest < ActiveRecord::TestCase assert !dev.projects.empty? assert dev.projects.all?(&:readonly?) assert dev.projects.find(:all).all?(&:readonly?) - assert dev.projects.find(:all, :readonly => true).all?(&:readonly?) + assert dev.projects.readonly(true).all?(&:readonly?) end def test_has_many_find_readonly @@ -62,7 +63,7 @@ class ReadOnlyTest < ActiveRecord::TestCase assert !post.comments.empty? assert !post.comments.any?(&:readonly?) assert !post.comments.find(:all).any?(&:readonly?) - assert post.comments.find(:all, :readonly => true).all?(&:readonly?) + assert post.comments.readonly(true).all?(&:readonly?) end def test_has_many_with_through_is_not_implicitly_marked_readonly @@ -71,32 +72,32 @@ class ReadOnlyTest < ActiveRecord::TestCase end def test_readonly_scoping - Post.with_scope(:find => { :conditions => '1=1' }) do + Post.send(:with_scope, :find => { :conditions => '1=1' }) do assert !Post.find(1).readonly? - assert Post.find(1, :readonly => true).readonly? - assert !Post.find(1, :readonly => false).readonly? + assert Post.readonly(true).find(1).readonly? + assert !Post.readonly(false).find(1).readonly? end - Post.with_scope(:find => { :joins => ' ' }) do + Post.send(:with_scope, :find => { :joins => ' ' }) do assert !Post.find(1).readonly? - assert Post.find(1, :readonly => true).readonly? - assert !Post.find(1, :readonly => false).readonly? + assert Post.readonly.find(1).readonly? + assert !Post.readonly(false).find(1).readonly? end # Oracle barfs on this because the join includes unqualified and # conflicting column names unless current_adapter?(:OracleAdapter) - Post.with_scope(:find => { :joins => ', developers' }) do + Post.send(:with_scope, :find => { :joins => ', developers' }) do assert Post.find(1).readonly? - assert Post.find(1, :readonly => true).readonly? - assert !Post.find(1, :readonly => false).readonly? + assert Post.readonly.find(1).readonly? + assert !Post.readonly(false).find(1).readonly? end end - Post.with_scope(:find => { :readonly => true }) do + Post.send(:with_scope, :find => { :readonly => true }) do assert Post.find(1).readonly? - assert Post.find(1, :readonly => true).readonly? - assert !Post.find(1, :readonly => false).readonly? + assert Post.readonly.find(1).readonly? + assert !Post.readonly(false).find(1).readonly? end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 9c5a38a399..f0ef3e22c5 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -38,7 +38,7 @@ class RelationTest < ActiveRecord::TestCase end def test_scoped_first - topics = Topic.scoped + topics = Topic.scoped.order('id ASC') assert_queries(1) do 2.times { assert_equal "The First Topic", topics.first.title } @@ -48,7 +48,7 @@ class RelationTest < ActiveRecord::TestCase end def test_loaded_first - topics = Topic.scoped + topics = Topic.scoped.order('id ASC') assert_queries(1) do topics.all # force load @@ -81,7 +81,7 @@ class RelationTest < ActiveRecord::TestCase def test_finding_with_order topics = Topic.order('id') - assert_equal 4, topics.size + assert_equal 4, topics.to_a.size assert_equal topics(:first).title, topics.first.title end @@ -95,11 +95,11 @@ class RelationTest < ActiveRecord::TestCase def test_finding_with_order_limit_and_offset entrants = Entrant.order("id ASC").limit(2).offset(1) - assert_equal 2, entrants.size + assert_equal 2, entrants.to_a.size assert_equal entrants(:second).name, entrants.first.name entrants = Entrant.order("id ASC").limit(2).offset(2) - assert_equal 1, entrants.size + assert_equal 1, entrants.to_a.size assert_equal entrants(:third).name, entrants.first.name end @@ -142,7 +142,22 @@ class RelationTest < ActiveRecord::TestCase relation = Topic.scoped ["map", "uniq", "sort", "insert", "delete", "update"].each do |method| - assert relation.respond_to?(method), "Topic.all should respond to #{method.inspect}" + assert relation.respond_to?(method), "Topic.scoped should respond to #{method.inspect}" + end + end + + def test_respond_to_private_arel_methods + relation = Topic.scoped + + assert ! relation.respond_to?(:matching_attributes) + assert relation.respond_to?(:matching_attributes, true) + end + + def test_respond_to_dynamic_finders + relation = Topic.scoped + + ["find_by_title", "find_by_title_and_author_name", "find_or_create_by_title", "find_or_initialize_by_title_and_author_name"].each do |method| + assert relation.respond_to?(method), "Topic.scoped should respond to #{method.inspect}" end end @@ -244,7 +259,7 @@ class RelationTest < ActiveRecord::TestCase author = Author.scoped.find_by_id!(authors(:david).id) assert_equal "David", author.name - assert_raises(ActiveRecord::RecordNotFound) { Author.scoped.find_by_id_and_name!('invalid', 'wt') } + assert_raises(ActiveRecord::RecordNotFound) { Author.scoped.find_by_id_and_name!(20, 'invalid') } end def test_dynamic_find_all_by_attributes @@ -281,7 +296,7 @@ class RelationTest < ActiveRecord::TestCase david = authors.find(authors(:david).id) assert_equal 'David', david.name - assert_raises(ActiveRecord::RecordNotFound) { authors.where(:name => 'lifo').find('invalid') } + assert_raises(ActiveRecord::RecordNotFound) { authors.where(:name => 'lifo').find('42') } end def test_find_ids @@ -294,8 +309,140 @@ class RelationTest < ActiveRecord::TestCase assert_equal 'Mary', results[1].name assert_equal results, authors.find([authors(:david).id, authors(:mary).id]) - assert_raises(ActiveRecord::RecordNotFound) { authors.where(:name => 'lifo').find(authors(:david).id, 'invalid') } - assert_raises(ActiveRecord::RecordNotFound) { authors.find(['invalid', 'oops']) } + assert_raises(ActiveRecord::RecordNotFound) { authors.where(:name => 'lifo').find(authors(:david).id, '42') } + assert_raises(ActiveRecord::RecordNotFound) { authors.find(['42', 43]) } + end + + def test_exists + davids = Author.where(:name => 'David') + assert davids.exists? + assert davids.exists?(authors(:david).id) + assert ! davids.exists?(authors(:mary).id) + assert ! davids.exists?("42") + assert ! davids.exists?(42) + + fake = Author.where(:name => 'fake author') + assert ! fake.exists? + assert ! fake.exists?(authors(:david).id) + end + + def test_last + authors = Author.scoped + assert_equal authors(:mary), authors.last + end + + def test_destroy_all + davids = Author.where(:name => 'David') + + # Force load + assert_equal [authors(:david)], davids.to_a + assert davids.loaded? + + assert_difference('Author.count', -1) { davids.destroy_all } + + assert_equal [], davids.to_a + assert davids.loaded? + end + + def test_delete_all + davids = Author.where(:name => 'David') + + assert_difference('Author.count', -1) { davids.delete_all } + assert ! davids.loaded? + end + + def test_delete_all_loaded + davids = Author.where(:name => 'David') + + # Force load + assert_equal [authors(:david)], davids.to_a + assert davids.loaded? + + assert_difference('Author.count', -1) { davids.delete_all } + + assert_equal [], davids.to_a + assert davids.loaded? + end + + def test_relation_merging + devs = Developer.where("salary >= 80000") & Developer.limit(2) & Developer.order('id ASC').where("id < 3") + assert_equal [developers(:david), developers(:jamis)], devs.to_a + + dev_with_count = Developer.limit(1) & Developer.order('id DESC') & Developer.select('developers.*') + assert_equal [developers(:poor_jamis)], dev_with_count.to_a + end + + def test_relation_merging_with_eager_load + relations = [] + relations << (Post.order('comments.id DESC') & Post.eager_load(:last_comment) & Post.scoped) + relations << (Post.eager_load(:last_comment) & Post.order('comments.id DESC') & Post.scoped) + + relations.each do |posts| + post = posts.find { |p| p.id == 1 } + assert_equal Post.find(1).last_comment, post.last_comment + end + end + + def test_relation_merging_with_preload + [Post.scoped & Post.preload(:author), Post.preload(:author) & Post.scoped].each do |posts| + assert_queries(2) { assert posts.first.author } + end + end + + def test_invalid_merge + assert_raises(ArgumentError) { Post.scoped & Developer.scoped } + end + + def test_count + posts = Post.scoped + + assert_equal 7, posts.count + assert_equal 7, posts.count(:all) + assert_equal 7, posts.count(:id) + + assert_equal 1, posts.where('comments_count > 1').count + assert_equal 5, posts.where(:comments_count => 0).count + end + + def test_count_with_distinct + posts = Post.scoped + + assert_equal 3, posts.count(:comments_count, :distinct => true) + assert_equal 7, posts.count(:comments_count, :distinct => false) + + assert_equal 3, posts.select(:comments_count).count(:distinct => true) + assert_equal 7, posts.select(:comments_count).count(:distinct => false) + end + + def test_count_explicit_columns + Post.update_all(:comments_count => nil) + posts = Post.scoped + + assert_equal 0, posts.select('comments_count').where('id is not null').order('id').count + assert_equal 0, posts.where('id is not null').select('comments_count').count + + assert_equal 7, posts.select('comments_count').count('id') + assert_equal 0, posts.select('comments_count').count + assert_equal 0, posts.count(:comments_count) + assert_equal 0, posts.count('comments_count') + end + + def test_size + posts = Post.scoped + + assert_queries(1) { assert_equal 7, posts.size } + assert ! posts.loaded? + + best_posts = posts.where(:comments_count => 0) + best_posts.to_a # force load + assert_no_queries { assert_equal 5, best_posts.size } + end + + def test_count_complex_chained_relations + posts = Post.select('comments_count').where('id is not null').group("author_id").where("comments_count > 0") + + expected = { 1 => 2 } + assert_equal expected, posts.count end end diff --git a/activerecord/test/cases/validations/uniqueness_validation_test.rb b/activerecord/test/cases/validations/uniqueness_validation_test.rb index 17ba4e2f8a..db633339f3 100644 --- a/activerecord/test/cases/validations/uniqueness_validation_test.rb +++ b/activerecord/test/cases/validations/uniqueness_validation_test.rb @@ -213,7 +213,7 @@ class UniquenessValidationTest < ActiveRecord::TestCase def test_validates_uniqueness_inside_with_scope Topic.validates_uniqueness_of(:title) - Topic.with_scope(:find => { :conditions => { :author_name => "David" } }) do + Topic.send(:with_scope, :find => { :conditions => { :author_name => "David" } }) do t1 = Topic.new("title" => "I'm unique!", "author_name" => "Mary") assert t1.save t2 = Topic.new("title" => "I'm unique!", "author_name" => "David") diff --git a/activemodel/lib/active_model/validations_repair_helper.rb b/activerecord/test/cases/validations_repair_helper.rb index 40741e6dbe..e04738d209 100644 --- a/activemodel/lib/active_model/validations_repair_helper.rb +++ b/activerecord/test/cases/validations_repair_helper.rb @@ -1,4 +1,4 @@ -module ActiveModel +module ActiveRecord module ValidationsRepairHelper extend ActiveSupport::Concern diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 130231c622..7462d944e0 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -98,14 +98,14 @@ class ValidationsTest < ActiveRecord::TestCase end def test_scoped_create_without_attributes - Reply.with_scope(:create => {}) do + Reply.send(:with_scope, :create => {}) do assert_raise(ActiveRecord::RecordInvalid) { Reply.create! } end end def test_create_with_exceptions_using_scope_for_protected_attributes assert_nothing_raised do - ProtectedPerson.with_scope( :create => { :first_name => "Mary" } ) do + ProtectedPerson.send(:with_scope, :create => { :first_name => "Mary" } ) do person = ProtectedPerson.create! :addon => "Addon" assert_equal person.first_name, "Mary", "scope should ignore attr_protected" end @@ -114,7 +114,7 @@ class ValidationsTest < ActiveRecord::TestCase def test_create_with_exceptions_using_scope_and_empty_attributes assert_nothing_raised do - ProtectedPerson.with_scope( :create => { :first_name => "Mary" } ) do + ProtectedPerson.send(:with_scope, :create => { :first_name => "Mary" } ) do person = ProtectedPerson.create! assert_equal person.first_name, "Mary", "should be ok when no attributes are passed to create!" end diff --git a/activerecord/test/fixtures/faces.yml b/activerecord/test/fixtures/faces.yml index 1dd2907cf7..c8e4a34484 100644 --- a/activerecord/test/fixtures/faces.yml +++ b/activerecord/test/fixtures/faces.yml @@ -5,3 +5,7 @@ trusting: weather_beaten: description: weather beaten man: steve + +confused: + description: confused + polymorphic_man: gordon (Man) diff --git a/activerecord/test/fixtures/interests.yml b/activerecord/test/fixtures/interests.yml index ec71890ab6..9200a19d5a 100644 --- a/activerecord/test/fixtures/interests.yml +++ b/activerecord/test/fixtures/interests.yml @@ -23,7 +23,11 @@ woodsmanship: zine: going_out man: steve -survial: +survival: topic: Survival zine: going_out man: steve + +llama_wrangling: + topic: Llama Wrangling + polymorphic_man: gordon (Man) diff --git a/activerecord/test/models/face.rb b/activerecord/test/models/face.rb index 1540dbf741..edb75d333f 100644 --- a/activerecord/test/models/face.rb +++ b/activerecord/test/models/face.rb @@ -1,5 +1,7 @@ class Face < ActiveRecord::Base belongs_to :man, :inverse_of => :face - # This is a "broken" inverse_of for the purposes of testing + belongs_to :polymorphic_man, :polymorphic => true, :inverse_of => :polymorphic_face + # These is a "broken" inverse_of for the purposes of testing belongs_to :horrible_man, :class_name => 'Man', :inverse_of => :horrible_face + belongs_to :horrible_polymorphic_man, :polymorphic => true, :inverse_of => :horrible_polymorphic_face end diff --git a/activerecord/test/models/interest.rb b/activerecord/test/models/interest.rb index d8291d00cc..d5d9226204 100644 --- a/activerecord/test/models/interest.rb +++ b/activerecord/test/models/interest.rb @@ -1,4 +1,5 @@ class Interest < ActiveRecord::Base belongs_to :man, :inverse_of => :interests + belongs_to :polymorphic_man, :polymorphic => true, :inverse_of => :polymorphic_interests belongs_to :zine, :inverse_of => :interests end diff --git a/activerecord/test/models/man.rb b/activerecord/test/models/man.rb index f40bc9d0fc..4bff92dc98 100644 --- a/activerecord/test/models/man.rb +++ b/activerecord/test/models/man.rb @@ -1,6 +1,8 @@ class Man < ActiveRecord::Base has_one :face, :inverse_of => :man + has_one :polymorphic_face, :class_name => 'Face', :as => :polymorphic_man, :inverse_of => :polymorphic_man has_many :interests, :inverse_of => :man + has_many :polymorphic_interests, :class_name => 'Interest', :as => :polymorphic_man, :inverse_of => :polymorphic_man # These are "broken" inverse_of associations for the purposes of testing has_one :dirty_face, :class_name => 'Face', :inverse_of => :dirty_man has_many :secret_interests, :class_name => 'Interest', :inverse_of => :secret_man diff --git a/activerecord/test/models/pirate.rb b/activerecord/test/models/pirate.rb index f2c05dd48f..88c1634717 100644 --- a/activerecord/test/models/pirate.rb +++ b/activerecord/test/models/pirate.rb @@ -19,6 +19,7 @@ class Pirate < ActiveRecord::Base # These both have :autosave enabled because accepts_nested_attributes_for is used on them. has_one :ship + has_one :update_only_ship, :class_name => 'Ship' has_one :non_validated_ship, :class_name => 'Ship' has_many :birds has_many :birds_with_method_callbacks, :class_name => "Bird", @@ -35,6 +36,7 @@ class Pirate < ActiveRecord::Base accepts_nested_attributes_for :parrots, :birds, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } accepts_nested_attributes_for :ship, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } + accepts_nested_attributes_for :update_only_ship, :update_only => true accepts_nested_attributes_for :parrots_with_method_callbacks, :parrots_with_proc_callbacks, :birds_with_method_callbacks, :birds_with_proc_callbacks, :allow_destroy => true accepts_nested_attributes_for :birds_with_reject_all_blank, :reject_if => :all_blank diff --git a/activerecord/test/models/ship.rb b/activerecord/test/models/ship.rb index 06759d64b8..a96e38ab41 100644 --- a/activerecord/test/models/ship.rb +++ b/activerecord/test/models/ship.rb @@ -2,9 +2,11 @@ class Ship < ActiveRecord::Base self.record_timestamps = false belongs_to :pirate + belongs_to :update_only_pirate, :class_name => 'Pirate' has_many :parts, :class_name => 'ShipPart', :autosave => true accepts_nested_attributes_for :pirate, :allow_destroy => true, :reject_if => proc { |attributes| attributes.empty? } + accepts_nested_attributes_for :update_only_pirate, :update_only => true validates_presence_of :name end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 0dd9da4c11..1ec36e7832 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -520,11 +520,15 @@ ActiveRecord::Schema.define do create_table :faces, :force => true do |t| t.string :description t.integer :man_id + t.integer :polymorphic_man_id + t.string :polymorphic_man_type end create_table :interests, :force => true do |t| t.string :topic t.integer :man_id + t.integer :polymorphic_man_id + t.string :polymorphic_man_type t.integer :zine_id end diff --git a/activesupport/CHANGELOG b/activesupport/CHANGELOG index cc4a2ff90e..9b0a84678a 100644 --- a/activesupport/CHANGELOG +++ b/activesupport/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Added Object#presence that returns the object if it's #present? otherwise returns nil [DHH/Colin Kelley] + * Add Enumerable#exclude? to bring parity to Enumerable#include? and avoid if !x.include?/else calls [DHH] * Update Edinburgh TimeZone to use "Europe/London" instead of "Europe/Dublin" #3310 [Phil Ross] diff --git a/activesupport/activesupport.gemspec b/activesupport/activesupport.gemspec index cabda2b073..2d2cbf6448 100644 --- a/activesupport/activesupport.gemspec +++ b/activesupport/activesupport.gemspec @@ -6,6 +6,8 @@ Gem::Specification.new do |s| s.summary = "Support and utility classes used by the Rails framework." s.description = %q{Utility library which carries commonly used classes and goodies from the Rails framework} + s.add_dependency('i18n', '>= 0.1.3') + s.files = Dir['CHANGELOG', 'README', 'lib/**/*'] s.require_path = 'lib' s.has_rdoc = true diff --git a/activesupport/lib/active_support.rb b/activesupport/lib/active_support.rb index f2baa5a56a..8215cfaf0d 100644 --- a/activesupport/lib/active_support.rb +++ b/activesupport/lib/active_support.rb @@ -70,6 +70,3 @@ module ActiveSupport end require 'active_support/vendor' - -require 'i18n' -I18n.load_path << "#{File.dirname(__FILE__)}/active_support/locale/en.yml" diff --git a/activesupport/lib/active_support/all.rb b/activesupport/lib/active_support/all.rb index f537818300..64600575d9 100644 --- a/activesupport/lib/active_support/all.rb +++ b/activesupport/lib/active_support/all.rb @@ -1,3 +1,4 @@ require 'active_support' +require 'active_support/i18n' require 'active_support/time' require 'active_support/core_ext' diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 67e9b0103f..97e2a478b5 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -467,8 +467,8 @@ module ActiveSupport # method that took into consideration the per_key conditions. This # is a speed improvement for ActionPack. # - def set_callback(name, *filters, &block) - __update_callbacks(name, filters, block) do |chain, type, filters, options| + def set_callback(name, *filter_list, &block) + __update_callbacks(name, filter_list, block) do |chain, type, filters, options| filters.map! do |filter| chain.delete_if {|c| c.matches?(type, filter) } Callback.new(chain, filter, type, options.dup, self) @@ -480,8 +480,8 @@ module ActiveSupport # Skip a previously defined callback for a given type. # - def skip_callback(name, *filters, &block) - __update_callbacks(name, filters, block) do |chain, type, filters, options| + def skip_callback(name, *filter_list, &block) + __update_callbacks(name, filter_list, block) do |chain, type, filters, options| chain = send("_#{name}_callbacks=", chain.clone(self)) filters.each do |filter| diff --git a/activesupport/lib/active_support/core_ext/array/conversions.rb b/activesupport/lib/active_support/core_ext/array/conversions.rb index db140225e8..7fcef38372 100644 --- a/activesupport/lib/active_support/core_ext/array/conversions.rb +++ b/activesupport/lib/active_support/core_ext/array/conversions.rb @@ -1,6 +1,7 @@ require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/hash/reverse_merge' require 'active_support/inflector' +require 'active_support/i18n' class Array # Converts the array to a comma-separated sentence where the last element is joined by the connector word. Options: diff --git a/activesupport/lib/active_support/core_ext/object/blank.rb b/activesupport/lib/active_support/core_ext/object/blank.rb index 9a1f663bf3..eb99bb1a36 100644 --- a/activesupport/lib/active_support/core_ext/object/blank.rb +++ b/activesupport/lib/active_support/core_ext/object/blank.rb @@ -2,11 +2,11 @@ class Object # An object is blank if it's false, empty, or a whitespace string. # For example, "", " ", +nil+, [], and {} are blank. # - # This simplifies + # This simplifies: # # if !address.nil? && !address.empty? # - # to + # ...to: # # if !address.blank? def blank? @@ -17,6 +17,24 @@ class Object def present? !blank? end + + # Returns object if it's #present? otherwise returns nil. + # object.presence is equivalent to object.present? ? object : nil. + # + # This is handy for any representation of objects where blank is the same + # as not present at all. For example, this simplifies a common check for + # HTTP POST/query parameters: + # + # state = params[:state] if params[:state].present? + # country = params[:country] if params[:country].present? + # region = state || country || 'US' + # + # ...becomes: + # + # region = params[:state].presence || params[:country].presence || 'US' + def presence + self if present? + end end class NilClass #:nodoc: diff --git a/activesupport/lib/active_support/core_ext/string.rb b/activesupport/lib/active_support/core_ext/string.rb index 0365b6af1c..411ea0f016 100644 --- a/activesupport/lib/active_support/core_ext/string.rb +++ b/activesupport/lib/active_support/core_ext/string.rb @@ -7,4 +7,5 @@ require 'active_support/core_ext/string/access' require 'active_support/core_ext/string/xchar' require 'active_support/core_ext/string/behavior' require 'active_support/core_ext/string/interpolation' -require 'active_support/core_ext/string/output_safety'
\ No newline at end of file +require 'active_support/core_ext/string/output_safety' +require 'active_support/core_ext/string/exclude' diff --git a/activesupport/lib/active_support/core_ext/string/exclude.rb b/activesupport/lib/active_support/core_ext/string/exclude.rb new file mode 100644 index 0000000000..5ca268b953 --- /dev/null +++ b/activesupport/lib/active_support/core_ext/string/exclude.rb @@ -0,0 +1,6 @@ +class String + # The inverse of String#include?. Returns true if the string does not include the other string. + def exclude?(string) + !include?(string) + end +end diff --git a/activesupport/lib/active_support/core_ext/string/output_safety.rb b/activesupport/lib/active_support/core_ext/string/output_safety.rb index 3e6ab0ebd2..ceed90ce79 100644 --- a/activesupport/lib/active_support/core_ext/string/output_safety.rb +++ b/activesupport/lib/active_support/core_ext/string/output_safety.rb @@ -1,3 +1,15 @@ +class Object + def html_safe? + false + end +end + +class Fixnum + def html_safe? + true + end +end + class String attr_accessor :_rails_html_safe alias html_safe? _rails_html_safe diff --git a/activesupport/lib/active_support/i18n.rb b/activesupport/lib/active_support/i18n.rb new file mode 100644 index 0000000000..854c60eec1 --- /dev/null +++ b/activesupport/lib/active_support/i18n.rb @@ -0,0 +1,2 @@ +require 'i18n' +I18n.load_path << "#{File.dirname(__FILE__)}/locale/en.yml"
\ No newline at end of file diff --git a/activesupport/lib/active_support/notifications/instrumenter.rb b/activesupport/lib/active_support/notifications/instrumenter.rb index fb95422af2..0655dd0cb6 100644 --- a/activesupport/lib/active_support/notifications/instrumenter.rb +++ b/activesupport/lib/active_support/notifications/instrumenter.rb @@ -10,10 +10,10 @@ module ActiveSupport end def instrument(name, payload={}) - time = Time.now - result = yield if block_given? + time = Time.now + yield if block_given? ensure - @notifier.publish(name, time, Time.now, result, @id, payload) + @notifier.publish(name, time, Time.now, @id, payload) end private @@ -23,15 +23,14 @@ module ActiveSupport end class Event - attr_reader :name, :time, :end, :transaction_id, :result, :payload + attr_reader :name, :time, :end, :transaction_id, :payload - def initialize(name, start, ending, result, transaction_id, payload) + def initialize(name, start, ending, transaction_id, payload) @name = name @payload = payload.dup @time = start @transaction_id = transaction_id @end = ending - @result = result end def duration diff --git a/activesupport/test/core_ext/blank_test.rb b/activesupport/test/core_ext/blank_test.rb index 1dbbf3ff30..ed6c625a0a 100644 --- a/activesupport/test/core_ext/blank_test.rb +++ b/activesupport/test/core_ext/blank_test.rb @@ -14,12 +14,17 @@ class BlankTest < Test::Unit::TestCase NOT = [ EmptyFalse.new, Object.new, true, 0, 1, 'a', [nil], { nil => 0 } ] def test_blank - BLANK.each { |v| assert v.blank?, "#{v.inspect} should be blank" } + BLANK.each { |v| assert v.blank?, "#{v.inspect} should be blank" } NOT.each { |v| assert !v.blank?, "#{v.inspect} should not be blank" } end def test_present BLANK.each { |v| assert !v.present?, "#{v.inspect} should not be present" } - NOT.each { |v| assert v.present?, "#{v.inspect} should be present" } + NOT.each { |v| assert v.present?, "#{v.inspect} should be present" } + end + + def test_presence + BLANK.each { |v| assert_equal nil, v.presence, "#{v.inspect}.presence should return nil" } + NOT.each { |v| assert_equal v, v.presence, "#{v.inspect}.presence should return self" } end end diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb index 56ed296dac..9a805bc010 100644 --- a/activesupport/test/core_ext/string_ext_test.rb +++ b/activesupport/test/core_ext/string_ext_test.rb @@ -350,6 +350,24 @@ class OutputSafetyTest < ActiveSupport::TestCase assert_equal @string, @string.html_safe! end + test "A fixnum is safe by default" do + assert 5.html_safe? + end + + test "An object is unsafe by default" do + klass = Class.new(Object) do + def to_str + "other" + end + end + + @string.html_safe! + @string << klass.new + + assert_equal "helloother", @string + assert !@string.html_safe? + end + test "Adding a safe string to another safe string returns a safe string" do @other_string = "other".html_safe! @string.html_safe! @@ -416,4 +434,17 @@ class OutputSafetyTest < ActiveSupport::TestCase @other_string << @string assert @other_string.html_safe? end + + test "Concatting a fixnum to safe always yields safe" do + @string.html_safe! + @string.concat(13) + assert @string.html_safe? + end +end + +class StringExcludeTest < ActiveSupport::TestCase + test 'inverse of #include' do + assert_equal false, 'foo'.exclude?('o') + assert_equal true, 'foo'.exclude?('p') + end end diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index 4f880d0db7..3ba77ae135 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -134,27 +134,25 @@ module Notifications class EventTest < TestCase def test_events_are_initialized_with_details - event = event(:foo, Time.now, Time.now + 1, 1, random_id, :payload => :bar) - assert_equal :foo, event.name - assert_equal Hash[:payload => :bar], event.payload - end - - def test_events_consumes_information_given_as_payload time = Time.now - event = event(:foo, time, time + 0.01, 1, random_id, {}) + event = event(:foo, time, time + 0.01, random_id, {}) - assert_equal Hash.new, event.payload + assert_equal :foo, event.name assert_equal time, event.time - assert_equal 1, event.result assert_equal 10.0, event.duration end + def test_events_consumes_information_given_as_payload + event = event(:foo, Time.now, Time.now + 1, random_id, :payload => :bar) + assert_equal Hash[:payload => :bar], event.payload + end + def test_event_is_parent_based_on_time_frame time = Time.utc(2009, 01, 01, 0, 0, 1) - parent = event(:foo, Time.utc(2009), Time.utc(2009) + 100, nil, random_id, {}) - child = event(:foo, time, time + 10, nil, random_id, {}) - not_child = event(:foo, time, time + 100, nil, random_id, {}) + parent = event(:foo, Time.utc(2009), Time.utc(2009) + 100, random_id, {}) + child = event(:foo, time, time + 10, random_id, {}) + not_child = event(:foo, time, time + 100, random_id, {}) assert parent.parent_of?(child) assert !child.parent_of?(parent) diff --git a/rack b/rack -Subproject adf996587aecdd604eff441b8b69e4c47a8c261 +Subproject c6805fb93da30e0056b38e0fa6015c3d1bca587 diff --git a/railties/CHANGELOG b/railties/CHANGELOG index 9ef2922133..0bc1ea32bc 100644 --- a/railties/CHANGELOG +++ b/railties/CHANGELOG @@ -1,5 +1,7 @@ *Edge* +* Added default .gitignore (this is just recognizing Git market share, don't throw a hissy if you use another SCM) [DHH] + * Added cookies.permanent, cookies.signed, and cookies.permanent.signed accessor for common cookie actions [DHH]. Examples: cookies.permanent[:prefers_open_id] = true diff --git a/railties/Rakefile b/railties/Rakefile index 07c2ff84a0..eff59f1f40 100644 --- a/railties/Rakefile +++ b/railties/Rakefile @@ -15,7 +15,7 @@ $LOAD_PATH.unshift "#{File.dirname(__FILE__)}/lib" require 'rails/version' PKG_BUILD = ENV['PKG_BUILD'] ? '.' + ENV['PKG_BUILD'] : '' -PKG_NAME = ENV['PKG_NAME'] || 'railties' +PKG_NAME = 'railties' PKG_VERSION = Rails::VERSION::STRING + PKG_BUILD PKG_FILE_NAME = "#{PKG_NAME}-#{PKG_VERSION}" PKG_DESTINATION = ENV["RAILS_PKG_DESTINATION"] || "../#{PKG_NAME}" @@ -25,7 +25,6 @@ RELEASE_NAME = "REL #{PKG_VERSION}" RUBY_FORGE_PROJECT = "rails" RUBY_FORGE_USER = "webster132" - task :default => :test task :test => 'test:isolated' @@ -50,33 +49,6 @@ Rake::TestTask.new('test:regular') do |t| t.verbose = true end -VENDOR_LIBS = %w( actionpack activerecord actionmailer activesupport activeresource railties ) - -desc "Generates a fresh Rails package with documentation" -task :fresh_rails => [ :clean, :create_rails, :copy_vendor_libraries, :generate_documentation ] - -desc "Generates a fresh Rails package using GEMs with documentation" -task :fresh_gem_rails => [ :clean, :create_rails ] - -desc "Generates a fresh Rails package without documentation (faster)" -task :fresh_rails_without_docs => [ :clean, :create_rails, :copy_vendor_libraries ] - -desc "Generates a fresh Rails package without documentation using links (faster)" -task :fresh_rails_without_docs_using_links => [ :clean, :create_rails, :link_vendor_libraries ] - -desc "Generates minimal Rails package using symlinks" -task :dev => [ :clean, :create_rails, :link_vendor_libraries ] - -desc "Packages the fresh Rails package with documentation" -task :package => [ :clean, :fresh_rails ] do - system %{cd ..; tar -czvf #{PKG_NAME}-#{PKG_VERSION}.tgz #{PKG_NAME}} - system %{cd ..; zip -r #{PKG_NAME}-#{PKG_VERSION}.zip #{PKG_NAME}} -end - -task :clean do - rm_rf PKG_DESTINATION -end - # Update spinoffs ------------------------------------------------------------------- desc "Updates application README to the latest version Railties README" @@ -86,51 +58,14 @@ task :update_readme do cp "./README", readme end -# Run application generator ------------------------------------------------------------- - -task :create_rails do - require 'rails/generators' - require 'rails/generators/rails/app/app_generator' - Rails::Generators::AppGenerator.start [ File.basename(PKG_DESTINATION), "--quiet" ], - :destination_root => File.expand_path(File.dirname(PKG_DESTINATION)) -end - -# Copy Vendors ---------------------------------------------------------------------------- - -desc "Copy in all the Rails packages to vendor" -task :copy_vendor_libraries do - mkdir File.join(PKG_DESTINATION, 'vendor', 'rails') - VENDOR_LIBS.each { |dir| cp_r File.join('..', dir), File.join(PKG_DESTINATION, 'vendor', 'rails', dir) } - FileUtils.rm_r(Dir.glob(File.join(PKG_DESTINATION, 'vendor', 'rails', "**", ".git"))) -end - -desc "Link in all the Rails packages to vendor" -task :link_vendor_libraries do - mkdir File.join(PKG_DESTINATION, 'vendor', 'rails') - VENDOR_LIBS.each { |dir| ln_s File.join('..', '..', '..', dir), File.join(PKG_DESTINATION, 'vendor', 'rails', dir) } -end - - desc 'Generate guides (for authors), use ONLY=foo to process just "foo.textile"' task :generate_guides do ENV["WARN_BROKEN_LINKS"] = "1" # authors can't disable this ruby "guides/rails_guides.rb" end - # Generate documentation ------------------------------------------------------------------ -desc "Generate documentation for the framework and for the empty application" -task :generate_documentation => [ :generate_app_doc, :generate_rails_framework_doc ] - -task :generate_rails_framework_doc do - system %{cd #{PKG_DESTINATION}; rake doc:rails} -end - -task :generate_app_doc do - system %{cd #{PKG_DESTINATION}; rake doc:app} -end - Rake::RDocTask.new { |rdoc| rdoc.rdoc_dir = 'doc' rdoc.title = "Railties -- Gluing the Engine to the Rails" @@ -152,8 +87,8 @@ Rake::GemPackageTask.new(spec) do |pkg| pkg.gem_spec = spec end - # Publishing ------------------------------------------------------- + desc "Publish the rails gem" task :pgem => [:gem] do require 'rake/contrib/sshpublisher' diff --git a/railties/bin/rails b/railties/bin/rails index 0f51d5739f..b8b2d6188f 100755 --- a/railties/bin/rails +++ b/railties/bin/rails @@ -20,7 +20,7 @@ end ARGV << "--help" if ARGV.empty? -require 'rails' + require 'rails/generators' require 'rails/generators/rails/app/app_generator' diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index cd579a1c0d..d714c5ac41 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -72,7 +72,9 @@ module Rails def load_tasks require "rails/tasks" - Dir["#{root}/vendor/plugins/*/**/tasks/**/*.rake"].sort.each { |ext| load ext } + plugins.each { |p| p.load_tasks } + # Load all application tasks + # TODO: extract out the path to the rake tasks Dir["#{root}/lib/tasks/**/*.rake"].sort.each { |ext| load ext } task :environment do $rails_rake_task = true @@ -90,7 +92,7 @@ module Rails def plugins @plugins ||= begin plugin_names = config.plugins || [:all] - Plugin.plugins.select { |p| plugin_names.include?(:all) || plugin_names.include?(p.plugin_name) } + + Plugin.plugins.select { |p| plugin_names.include?(:all) || plugin_names.include?(p.plugin_name) }.map { |p| p.new } + Plugin::Vendored.all(config.plugins || [:all], config.paths.vendor.plugins) end end diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index 086f67a419..cb321536d2 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -1,6 +1,24 @@ require 'active_support/ordered_options' module Rails + # Create a Plugin::Options from ActiveSuppot::OrderedOptions, + # which support the following syntax: + # + # controller.action_controller.include FooBar + # + class Plugin::Options < ActiveSupport::OrderedOptions #:nodoc: + attr_reader :includes + + def initialize(*args) + @includes = [] + super + end + + def include(*args) + @includes.concat(args) + end + end + # Temporarily separate the plugin configuration class from the main # configuration class while this bit is being cleaned up. class Plugin::Configuration @@ -16,7 +34,7 @@ module Rails @options = base.options.dup @middleware = base.middleware.dup else - @options = Hash.new { |h,k| h[k] = ActiveSupport::OrderedOptions.new } + @options = Hash.new { |h,k| h[k] = Rails::Plugin::Options.new } @middleware = ActionDispatch::MiddlewareStack.new end end diff --git a/railties/lib/rails/core.rb b/railties/lib/rails/core.rb index da16c5816c..c819e90dd7 100644 --- a/railties/lib/rails/core.rb +++ b/railties/lib/rails/core.rb @@ -15,7 +15,6 @@ require 'rails/paths' require 'rails/core' require 'rails/configuration' require 'rails/deprecation' -require 'rails/initializer' require 'rails/ruby_version_check' # For Ruby 1.8, this initialization sets $KCODE to 'u' to enable the diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 0e66c9f58f..2ba56bc3c5 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -4,8 +4,8 @@ $:.unshift(activesupport_path) if File.directory?(activesupport_path) && !$:.inc require 'active_support' require 'active_support/core_ext/object/blank' require 'active_support/core_ext/object/metaclass' -require 'active_support/core_ext/array' -require 'active_support/core_ext/hash' +require 'active_support/core_ext/array/extract_options' +require 'active_support/core_ext/hash/deep_merge' require 'active_support/core_ext/module/attribute_accessors' require 'active_support/core_ext/string/inflections' @@ -95,7 +95,7 @@ module Rails end def self.plugins_generators_paths #:nodoc: - return [] unless Rails.root + return [] unless defined?(Rails.root) && Rails.root Dir[File.join(Rails.root, "vendor", "plugins", "*", "lib", "{generators,rails_generators}")] end @@ -136,7 +136,7 @@ module Rails def self.load_paths @load_paths ||= begin paths = [] - paths += Dir[File.join(Rails.root, "lib", "{generators,rails_generators}")] if Rails.root + paths += Dir[File.join(Rails.root, "lib", "{generators,rails_generators}")] if defined?(Rails.root) && Rails.root paths += Dir[File.join(Thor::Util.user_home, ".rails", "{generators,rails_generators}")] paths += self.plugins_generators_paths paths += self.gems_generators_paths diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index 30272ed9b2..22a27fdac2 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -3,6 +3,10 @@ require 'active_support/secure_random' require 'rails/version' unless defined?(Rails::VERSION) module Rails::Generators + # We need to store the RAILS_DEV_PATH in a constant, otherwise the path + # can change in Ruby 1.8.7 when we FileUtils.cd. + RAILS_DEV_PATH = File.expand_path("../../../../../..", File.dirname(__FILE__)) + class AppGenerator < Base DATABASES = %w( mysql oracle postgresql sqlite3 frontbase ibm_db ) add_shebang_option! @@ -15,17 +19,22 @@ module Rails::Generators class_option :template, :type => :string, :aliases => "-m", :desc => "Path to an application template (can be a filesystem path or URL)." + class_option :dev, :type => :boolean, :default => false, + :desc => "Setup the application with Gemfile pointing to your Rails checkout" + + class_option :edge, :type => :boolean, :default => false, + :desc => "Setup the application with Gemfile pointing to Rails repository" + class_option :skip_activerecord, :type => :boolean, :aliases => "-O", :default => false, - :desc => "Skip ActiveRecord files" + :desc => "Skip ActiveRecord files" class_option :skip_testunit, :type => :boolean, :aliases => "-T", :default => false, - :desc => "Skip TestUnit files" + :desc => "Skip TestUnit files" class_option :skip_prototype, :type => :boolean, :aliases => "-J", :default => false, - :desc => "Skip Prototype files" + :desc => "Skip Prototype files" - # Add Rails options - # + # Add bin/rails options class_option :version, :type => :boolean, :aliases => "-v", :group => :rails, :desc => "Show Rails version number and quit" @@ -49,6 +58,7 @@ module Rails::Generators def create_root_files copy_file "README" + copy_file "gitignore", ".gitignore" template "Rakefile" template "config.ru" template "Gemfile" @@ -172,7 +182,6 @@ module Rails::Generators end # Define file as an alias to create_file for backwards compatibility. - # def file(*args, &block) create_file(*args, &block) end @@ -189,6 +198,10 @@ module Rails::Generators ActiveSupport::SecureRandom.hex(64) end + def dev_or_edge? + options.dev? || options.edge? + end + def self.banner "#{$0} #{self.arguments.map(&:usage).join(' ')} [options]" end diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 8e851a64e7..59c6d333e2 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -1,9 +1,14 @@ # Edit this Gemfile to bundle your application's dependencies. - +<% if !dev_or_edge? %> gem "rails", "<%= Rails::VERSION::STRING %>" +<% end -%> ## Bundle edge rails: -# gem "rails", :git => "git://github.com/rails/rails.git" +<%- if options.dev? -%> +gem "rails", :path => "<%= Rails::Generators::RAILS_DEV_PATH %>" +<%- else -%> +<%= "# " unless options.edge? %>gem "rails", :git => "git://github.com/rails/rails.git" +<%- end -%> ## Bundle the gems you use: # gem "bj" diff --git a/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb b/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb index 9889b52893..95b6a0af6d 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb +++ b/railties/lib/rails/generators/rails/app/templates/app/controllers/application_controller.rb @@ -4,5 +4,6 @@ class ApplicationController < ActionController::Base helper :all protect_from_forgery + respond_to :html, :js, :xml, :json filter_parameter_logging :password end diff --git a/railties/lib/rails/generators/rails/app/templates/gitignore b/railties/lib/rails/generators/rails/app/templates/gitignore new file mode 100644 index 0000000000..a4f05d101d --- /dev/null +++ b/railties/lib/rails/generators/rails/app/templates/gitignore @@ -0,0 +1,3 @@ +db/*.sqlite3 +log/*.log +tmp/**/* diff --git a/railties/lib/rails/initializer.rb b/railties/lib/rails/initializer.rb deleted file mode 100644 index 95478428ec..0000000000 --- a/railties/lib/rails/initializer.rb +++ /dev/null @@ -1,15 +0,0 @@ -require "rails" # In case people require this file directly - -module Rails - class Initializer - class Error < StandardError ; end - def self.run(initializer = nil, config = nil) - if initializer - # Deprecated - else - Rails.application = Class.new(Application) - yield Rails.application.config if block_given? - end - end - end -end diff --git a/railties/lib/rails/plugin.rb b/railties/lib/rails/plugin.rb index 0699affea7..c64042cf7d 100644 --- a/railties/lib/rails/plugin.rb +++ b/railties/lib/rails/plugin.rb @@ -25,6 +25,39 @@ module Rails Configuration.default end + def self.rake_tasks(&blk) + @rake_tasks ||= [] + @rake_tasks << blk if blk + @rake_tasks + end + + def rake_tasks + self.class.rake_tasks + end + + def load_tasks + return unless rake_tasks + rake_tasks.each { |blk| blk.call } + end + + # Creates an initializer which includes all given modules to the given class. + # + # module Rails + # class ActionController < Rails::Plugin + # plugin_name :action_controller + # include_modules_in "ActionController::Base" + # end + # end + # + def self.include_modules_in(klass, from=plugin_name) + self.initializer :"#{from}.include_modules" do |app| + klass = klass.constantize if klass.is_a?(String) + app.config.send(from).includes.each do |mod| + klass.send(:include, mod.is_a?(String) ? mod.constantize : mod) + end + end + end + class Vendored < Plugin def self.all(list, paths) plugins = [] @@ -52,6 +85,10 @@ module Rails Dir["#{path}/{lib}", "#{path}/app/{models,controllers,helpers}"] end + def load_tasks + Dir["#{path}/**/tasks/**/*.rake"].sort.each { |ext| load ext } + end + initializer :add_to_load_path, :after => :set_autoload_paths do |app| load_paths.each do |path| $LOAD_PATH << path diff --git a/railties/lib/rails/tasks.rb b/railties/lib/rails/tasks.rb index dc886f4a4d..44c014efe8 100644 --- a/railties/lib/rails/tasks.rb +++ b/railties/lib/rails/tasks.rb @@ -3,7 +3,6 @@ $VERBOSE = nil # Load Rails rakefile extensions %w( annotations - databases documentation framework log diff --git a/railties/test/abstract_unit.rb b/railties/test/abstract_unit.rb index 47013d7797..66ab5a08c3 100644 --- a/railties/test/abstract_unit.rb +++ b/railties/test/abstract_unit.rb @@ -22,6 +22,8 @@ require 'active_support/test_case' require 'action_controller' require 'rails' -Rails::Initializer.run do |config| +# TODO: Remove these hacks +class TestApp < Rails::Application config.root = File.dirname(__FILE__) end +Rails.application = TestApp diff --git a/railties/test/application/notifications_test.rb b/railties/test/application/notifications_test.rb index 71e406f2c1..8229e83147 100644 --- a/railties/test/application/notifications_test.rb +++ b/railties/test/application/notifications_test.rb @@ -1,27 +1,35 @@ require "isolation/abstract_unit" module ApplicationTests - class NotificationsTest < Test::Unit::TestCase - include ActiveSupport::Testing::Isolation + class MyQueue + def publish(name, *args) + raise name + end - class MyQueue - def publish(name, *args) - raise name - end + # Not a full queue implementation + def method_missing(name, *args, &blk) + self end + end + + class NotificationsTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation def setup build_app boot_rails - require "rails" require "active_support/notifications" @events = [] - Rails::Initializer.run do |c| - c.notifications.notifier = ActiveSupport::Notifications::Notifier.new(MyQueue.new) - end + + add_to_config <<-RUBY + config.notifications.notifier = ActiveSupport::Notifications::Notifier.new(ApplicationTests::MyQueue.new) + RUBY end test "new queue is set" do + use_frameworks [] + require "#{app_path}/config/environment" + assert_raise RuntimeError do ActiveSupport::Notifications.publish('foo') end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 10d0bc6bc2..567555fdc5 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -149,6 +149,19 @@ class AppGeneratorTest < GeneratorsTestCase assert_file 'lib/test_file.rb', 'heres test data' end + def test_dev_option + run_generator ["--dev"] + rails_path = File.expand_path('../../..', Rails.root) + dev_gem = %(gem "rails", :path => #{rails_path.inspect}) + assert_file 'Gemfile', /^#{Regexp.escape(dev_gem)}$/ + end + + def test_edge_option + run_generator ["--edge"] + edge_gem = %(gem "rails", :git => "git://github.com/rails/rails.git") + assert_file 'Gemfile', /^#{Regexp.escape(edge_gem)}$/ + end + protected def run_generator(args=[]) diff --git a/railties/test/initializer/check_ruby_version_test.rb b/railties/test/initializer/check_ruby_version_test.rb index 0691caad9d..a2c07ece75 100644 --- a/railties/test/initializer/check_ruby_version_test.rb +++ b/railties/test/initializer/check_ruby_version_test.rb @@ -1,7 +1,7 @@ require "isolation/abstract_unit" module InitializerTests - class PathsTest < Test::Unit::TestCase + class CheckRubyVersionTest < Test::Unit::TestCase include ActiveSupport::Testing::Isolation def setup @@ -9,52 +9,21 @@ module InitializerTests boot_rails end - test "rails does not initialize with ruby version 1.8.1" do - assert_rails_does_not_boot "1.8.1" - end - - test "rails does not initialize with ruby version 1.8.2" do - assert_rails_does_not_boot "1.8.2" - end - - test "rails does not initialize with ruby version 1.8.3" do - assert_rails_does_not_boot "1.8.3" - end - - test "rails does not initialize with ruby version 1.8.4" do - assert_rails_does_not_boot "1.8.4" - end - - test "rails does not initializes with ruby version 1.8.5" do - assert_rails_does_not_boot "1.8.5" - end - - test "rails does not initialize with ruby version 1.8.6" do - assert_rails_does_not_boot "1.8.6" - end - - test "rails initializes with ruby version 1.8.7" do - assert_rails_boots "1.8.7" - end - - test "rails initializes with the current version of Ruby" do - assert_rails_boots - end - - def set_ruby_version(version) - $-w = nil - Object.const_set(:RUBY_VERSION, version.freeze) + test "rails initializes with ruby 1.8.7 or later" do + if RUBY_VERSION < '1.8.7' + assert_rails_does_not_boot + else + assert_rails_boots + end end - def assert_rails_boots(version = nil) - set_ruby_version(version) if version + def assert_rails_boots assert_nothing_raised "It appears that rails does not boot" do require "rails" end end - def assert_rails_does_not_boot(version) - set_ruby_version(version) + def assert_rails_does_not_boot $stderr = File.open("/dev/null", "w") assert_raises(SystemExit) do require "rails" diff --git a/railties/test/initializer/path_test.rb b/railties/test/initializer/path_test.rb index fa66ebcd83..3bbf9617a0 100644 --- a/railties/test/initializer/path_test.rb +++ b/railties/test/initializer/path_test.rb @@ -1,101 +1,103 @@ require "isolation/abstract_unit" -class PathsTest < Test::Unit::TestCase - include ActiveSupport::Testing::Isolation - - def setup - build_app - boot_rails - require "rails" - add_to_config <<-RUBY - config.root = "#{app_path}" - config.frameworks = [:action_controller, :action_view, :action_mailer, :active_record] - config.after_initialize do - ActionController::Base.session_store = nil - end - RUBY - require "#{app_path}/config/environment" - @paths = Rails.application.config.paths - end - - def root(*path) - app_path(*path).to_s - end - - def assert_path(paths, *dir) - assert_equal [root(*dir)], paths.paths - end - - def assert_in_load_path(*path) - assert $:.any? { |p| File.expand_path(p) == root(*path) }, "Load path does not include '#{root(*path)}'. They are:\n-----\n #{$:.join("\n")}\n-----" - end - - def assert_not_in_load_path(*path) - assert !$:.any? { |p| File.expand_path(p) == root(*path) }, "Load path includes '#{root(*path)}'. They are:\n-----\n #{$:.join("\n")}\n-----" - end - - test "booting up Rails yields a valid paths object" do - assert_path @paths.app, "app" - assert_path @paths.app.metals, "app", "metal" - assert_path @paths.app.models, "app", "models" - assert_path @paths.app.helpers, "app", "helpers" - assert_path @paths.app.services, "app", "services" - assert_path @paths.lib, "lib" - assert_path @paths.vendor, "vendor" - assert_path @paths.vendor.plugins, "vendor", "plugins" - assert_path @paths.tmp, "tmp" - assert_path @paths.tmp.cache, "tmp", "cache" - assert_path @paths.config, "config" - assert_path @paths.config.locales, "config", "locales" - assert_path @paths.config.environments, "config", "environments" - - assert_equal root("app", "controllers"), @paths.app.controllers.to_a.first - assert_equal Pathname.new(File.dirname(__FILE__)).join("..", "..", "builtin", "rails_info").expand_path, - Pathname.new(@paths.app.controllers.to_a[1]).expand_path - end +module InitializerTests + class PathTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation + + def setup + build_app + boot_rails + require "rails" + add_to_config <<-RUBY + config.root = "#{app_path}" + config.frameworks = [:action_controller, :action_view, :action_mailer, :active_record] + config.after_initialize do + ActionController::Base.session_store = nil + end + RUBY + require "#{app_path}/config/environment" + @paths = Rails.application.config.paths + end + + def root(*path) + app_path(*path).to_s + end + + def assert_path(paths, *dir) + assert_equal [root(*dir)], paths.paths + end + + def assert_in_load_path(*path) + assert $:.any? { |p| File.expand_path(p) == root(*path) }, "Load path does not include '#{root(*path)}'. They are:\n-----\n #{$:.join("\n")}\n-----" + end + + def assert_not_in_load_path(*path) + assert !$:.any? { |p| File.expand_path(p) == root(*path) }, "Load path includes '#{root(*path)}'. They are:\n-----\n #{$:.join("\n")}\n-----" + end + + test "booting up Rails yields a valid paths object" do + assert_path @paths.app, "app" + assert_path @paths.app.metals, "app", "metal" + assert_path @paths.app.models, "app", "models" + assert_path @paths.app.helpers, "app", "helpers" + assert_path @paths.app.services, "app", "services" + assert_path @paths.lib, "lib" + assert_path @paths.vendor, "vendor" + assert_path @paths.vendor.plugins, "vendor", "plugins" + assert_path @paths.tmp, "tmp" + assert_path @paths.tmp.cache, "tmp", "cache" + assert_path @paths.config, "config" + assert_path @paths.config.locales, "config", "locales" + assert_path @paths.config.environments, "config", "environments" + + assert_equal root("app", "controllers"), @paths.app.controllers.to_a.first + assert_equal Pathname.new(File.dirname(__FILE__)).join("..", "..", "builtin", "rails_info").expand_path, + Pathname.new(@paths.app.controllers.to_a[1]).expand_path + end + + test "booting up Rails yields a list of paths that are eager" do + assert @paths.app.models.eager_load? + assert @paths.app.controllers.eager_load? + assert @paths.app.helpers.eager_load? + assert @paths.app.metals.eager_load? + end + + test "environments has a glob equal to the current environment" do + assert_equal "#{RAILS_ENV}.rb", @paths.config.environments.glob + end + + test "load path includes each of the paths in config.paths as long as the directories exist" do + assert_in_load_path "app" + assert_in_load_path "app", "controllers" + assert_in_load_path "app", "models" + assert_in_load_path "app", "helpers" + assert_in_load_path "lib" + assert_in_load_path "vendor" + + assert_not_in_load_path "app", "views" + assert_not_in_load_path "app", "metal" + assert_not_in_load_path "app", "services" + assert_not_in_load_path "config" + assert_not_in_load_path "config", "locales" + assert_not_in_load_path "config", "environments" + assert_not_in_load_path "tmp" + assert_not_in_load_path "tmp", "cache" + end + + test "controller paths include builtin in development mode" do + RAILS_ENV.replace "development" + assert Rails::Configuration.new.paths.app.controllers.paths.any? { |p| p =~ /builtin/ } + end + + test "controller paths does not have builtin_directories in test mode" do + RAILS_ENV.replace "test" + assert !Rails::Configuration.new.paths.app.controllers.paths.any? { |p| p =~ /builtin/ } + end + + test "controller paths does not have builtin_directories in production mode" do + RAILS_ENV.replace "production" + assert !Rails::Configuration.new.paths.app.controllers.paths.any? { |p| p =~ /builtin/ } + end - test "booting up Rails yields a list of paths that are eager" do - assert @paths.app.models.eager_load? - assert @paths.app.controllers.eager_load? - assert @paths.app.helpers.eager_load? - assert @paths.app.metals.eager_load? end - - test "environments has a glob equal to the current environment" do - assert_equal "#{RAILS_ENV}.rb", @paths.config.environments.glob - end - - test "load path includes each of the paths in config.paths as long as the directories exist" do - assert_in_load_path "app" - assert_in_load_path "app", "controllers" - assert_in_load_path "app", "models" - assert_in_load_path "app", "helpers" - assert_in_load_path "lib" - assert_in_load_path "vendor" - - assert_not_in_load_path "app", "views" - assert_not_in_load_path "app", "metal" - assert_not_in_load_path "app", "services" - assert_not_in_load_path "config" - assert_not_in_load_path "config", "locales" - assert_not_in_load_path "config", "environments" - assert_not_in_load_path "tmp" - assert_not_in_load_path "tmp", "cache" - end - - test "controller paths include builtin in development mode" do - RAILS_ENV.replace "development" - assert Rails::Configuration.new.paths.app.controllers.paths.any? { |p| p =~ /builtin/ } - end - - test "controller paths does not have builtin_directories in test mode" do - RAILS_ENV.replace "test" - assert !Rails::Configuration.new.paths.app.controllers.paths.any? { |p| p =~ /builtin/ } - end - - test "controller paths does not have builtin_directories in production mode" do - RAILS_ENV.replace "production" - assert !Rails::Configuration.new.paths.app.controllers.paths.any? { |p| p =~ /builtin/ } - end - -end
\ No newline at end of file +end diff --git a/railties/test/metal_test.rb b/railties/test/metal_test.rb index 2256b191e2..91f55c2b5e 100644 --- a/railties/test/metal_test.rb +++ b/railties/test/metal_test.rb @@ -1,5 +1,4 @@ require 'abstract_unit' -require 'rails/initializer' class MetalTest < Test::Unit::TestCase def test_metals_should_return_list_of_found_metal_apps diff --git a/railties/test/plugins/configuration_test.rb b/railties/test/plugins/configuration_test.rb index 5786316d1d..0843d05577 100644 --- a/railties/test/plugins/configuration_test.rb +++ b/railties/test/plugins/configuration_test.rb @@ -8,6 +8,10 @@ module PluginsTest require "rails" end + module Bar; end + module Baz; end + module All; end + test "config is available to plugins" do class Foo < Rails::Plugin ; end assert_nil Foo.config.action_controller.foo @@ -24,6 +28,18 @@ module PluginsTest assert_equal "hello", AppTemplate::Application.config.foo.greetings end + test "plugin configurations allow modules to be given" do + class Foo < Rails::Plugin ; config.foo.include(Bar, Baz) ; end + assert_equal [Bar, Baz], Foo.config.foo.includes + end + + test "plugin includes given modules in given class" do + class Foo < Rails::Plugin ; config.foo.include(Bar, "PluginsTest::ConfigurationTest::Baz") ; include_modules_in All ; end + Foo.new.run_initializers(Foo) + assert All.ancestors.include?(Bar) + assert All.ancestors.include?(Baz) + end + test "plugin config merges are deep" do class Foo < Rails::Plugin ; config.foo.greetings = 'hello' ; end class MyApp < Rails::Application diff --git a/railties/test/plugins/framework_extension_test.rb b/railties/test/plugins/framework_extension_test.rb index 87e19cadce..a6c7b753f8 100644 --- a/railties/test/plugins/framework_extension_test.rb +++ b/railties/test/plugins/framework_extension_test.rb @@ -2,6 +2,38 @@ require "isolation/abstract_unit" module PluginsTest class FrameworkExtensionTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation + + def setup + build_app + boot_rails + require "rails" + end + + test "rake_tasks block is executed when MyApp.load_tasks is called" do + $ran_block = false + + class MyPlugin < Rails::Plugin + rake_tasks do + $ran_block = true + end + end + + require "#{app_path}/config/environment" + + assert !$ran_block + require 'rake' + require 'rake/testtask' + require 'rake/rdoctask' + + AppTemplate::Application.load_tasks + assert $ran_block + end + end + + class ActiveRecordExtensionTest < Test::Unit::TestCase + include ActiveSupport::Testing::Isolation + def setup build_app boot_rails |