diff options
129 files changed, 1697 insertions, 723 deletions
diff --git a/.travis.yml b/.travis.yml index 9e7a449010..e5aaf57f93 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,8 @@ before_install: rvm: - 1.9.3 - 2.0.0 - - 2.1.1 + - 2.1 + - ruby-head - rbx-2 - jruby env: @@ -19,6 +20,7 @@ matrix: allow_failures: - rvm: rbx-2 - rvm: jruby + - rvm: ruby-head fast_finish: true notifications: email: false @@ -13,6 +13,7 @@ gem 'turbolinks' gem 'coffee-rails', '~> 4.0.0' gem 'arel', github: 'rails/arel', branch: 'master' gem 'sprockets-rails', github: 'rails/sprockets-rails', branch: '2-1-stable' +gem 'i18n', github: 'svenfuchs/i18n', branch: 'master' # require: false so bcrypt is loaded only when has_secure_password is used. # This is to avoid ActiveModel (and by extension the entire framework) @@ -25,7 +26,7 @@ gem 'uglifier', '>= 1.3.0', require: false group :doc do gem 'sdoc', '~> 0.4.0' - gem 'redcarpet', '~> 3.1.0', platforms: :ruby + gem 'redcarpet', '~> 3.1.2', platforms: :ruby gem 'w3c_validators' gem 'kindlerb' end @@ -38,6 +39,9 @@ local_gemfile = File.dirname(__FILE__) + "/.Gemfile" instance_eval File.read local_gemfile if File.exist? local_gemfile group :test do + # FIX: Our test suite isn't ready to run in random order yet + gem 'minitest', '< 5.3.4' + platforms :mri_19 do gem 'ruby-prof', '~> 0.11.2' end @@ -85,7 +89,7 @@ end # gems that are necessary for ActiveRecord tests with Oracle database if ENV['ORACLE_ENHANCED'] platforms :ruby do - gem 'ruby-oci8', '>= 2.0.4' + gem 'ruby-oci8', '~> 2.1' end gem 'activerecord-oracle_enhanced-adapter', github: 'rsim/oracle-enhanced', branch: 'master' end diff --git a/actionmailer/test/delivery_methods_test.rb b/actionmailer/test/delivery_methods_test.rb index 609903620b..16e8638542 100644 --- a/actionmailer/test/delivery_methods_test.rb +++ b/actionmailer/test/delivery_methods_test.rb @@ -47,12 +47,12 @@ class DefaultsDeliveryMethodsTest < ActiveSupport::TestCase end class CustomDeliveryMethodsTest < ActiveSupport::TestCase - def setup + setup do @old_delivery_method = ActionMailer::Base.delivery_method ActionMailer::Base.add_delivery_method :custom, MyCustomDelivery end - def teardown + teardown do ActionMailer::Base.delivery_method = @old_delivery_method new = ActionMailer::Base.delivery_methods.dup new.delete(:custom) @@ -93,18 +93,16 @@ class MailDeliveryTest < ActiveSupport::TestCase end end - def setup - ActionMailer::Base.delivery_method = :smtp + setup do + @old_delivery_method = DeliveryMailer.delivery_method end - def teardown - DeliveryMailer.delivery_method = :smtp - DeliveryMailer.perform_deliveries = true - DeliveryMailer.raise_delivery_errors = true + teardown do + DeliveryMailer.delivery_method = @old_delivery_method + DeliveryMailer.deliveries.clear end test "ActionMailer should be told when Mail gets delivered" do - DeliveryMailer.deliveries.clear DeliveryMailer.expects(:deliver_mail).once DeliveryMailer.welcome.deliver end @@ -176,22 +174,29 @@ class MailDeliveryTest < ActiveSupport::TestCase end test "does not perform deliveries if requested" do - DeliveryMailer.perform_deliveries = false - DeliveryMailer.deliveries.clear - Mail::Message.any_instance.expects(:deliver!).never - DeliveryMailer.welcome.deliver + old_perform_deliveries = DeliveryMailer.perform_deliveries + begin + DeliveryMailer.perform_deliveries = false + Mail::Message.any_instance.expects(:deliver!).never + DeliveryMailer.welcome.deliver + ensure + DeliveryMailer.perform_deliveries = old_perform_deliveries + end end test "does not append the deliveries collection if told not to perform the delivery" do - DeliveryMailer.perform_deliveries = false - DeliveryMailer.deliveries.clear - DeliveryMailer.welcome.deliver - assert_equal(0, DeliveryMailer.deliveries.length) + old_perform_deliveries = DeliveryMailer.perform_deliveries + begin + DeliveryMailer.perform_deliveries = false + DeliveryMailer.welcome.deliver + assert_equal [], DeliveryMailer.deliveries + ensure + DeliveryMailer.perform_deliveries = old_perform_deliveries + end end test "raise errors on bogus deliveries" do DeliveryMailer.delivery_method = BogusDelivery - DeliveryMailer.deliveries.clear assert_raise RuntimeError do DeliveryMailer.welcome.deliver end @@ -199,27 +204,34 @@ class MailDeliveryTest < ActiveSupport::TestCase test "does not increment the deliveries collection on error" do DeliveryMailer.delivery_method = BogusDelivery - DeliveryMailer.deliveries.clear assert_raise RuntimeError do DeliveryMailer.welcome.deliver end - assert_equal(0, DeliveryMailer.deliveries.length) + assert_equal [], DeliveryMailer.deliveries end test "does not raise errors on bogus deliveries if set" do - DeliveryMailer.delivery_method = BogusDelivery - DeliveryMailer.raise_delivery_errors = false - assert_nothing_raised do - DeliveryMailer.welcome.deliver + old_raise_delivery_errors = DeliveryMailer.raise_delivery_errors + begin + DeliveryMailer.delivery_method = BogusDelivery + DeliveryMailer.raise_delivery_errors = false + assert_nothing_raised do + DeliveryMailer.welcome.deliver + end + ensure + DeliveryMailer.raise_delivery_errors = old_raise_delivery_errors end end test "does not increment the deliveries collection on bogus deliveries" do - DeliveryMailer.delivery_method = BogusDelivery - DeliveryMailer.raise_delivery_errors = false - DeliveryMailer.deliveries.clear - DeliveryMailer.welcome.deliver - assert_equal(0, DeliveryMailer.deliveries.length) + old_raise_delivery_errors = DeliveryMailer.raise_delivery_errors + begin + DeliveryMailer.delivery_method = BogusDelivery + DeliveryMailer.raise_delivery_errors = false + DeliveryMailer.welcome.deliver + assert_equal [], DeliveryMailer.deliveries + ensure + DeliveryMailer.raise_delivery_errors = old_raise_delivery_errors + end end - end diff --git a/actionmailer/test/i18n_with_controller_test.rb b/actionmailer/test/i18n_with_controller_test.rb index 14a1b11b6d..d502d42ffd 100644 --- a/actionmailer/test/i18n_with_controller_test.rb +++ b/actionmailer/test/i18n_with_controller_test.rb @@ -10,15 +10,15 @@ class I18nTestMailer < ActionMailer::Base def mail_with_i18n_subject(recipient) @recipient = recipient I18n.locale = :de - mail(to: recipient, subject: "#{I18n.t :email_subject} #{recipient}", + mail(to: recipient, subject: I18n.t(:email_subject), from: "system@loudthinking.com", date: Time.local(2004, 12, 12)) end end class TestController < ActionController::Base def send_mail - I18nTestMailer.mail_with_i18n_subject("test@localhost").deliver - render text: 'Mail sent' + email = I18nTestMailer.mail_with_i18n_subject("test@localhost").deliver + render text: "Mail sent - Subject: #{email.subject}" end end @@ -32,16 +32,19 @@ class ActionMailerI18nWithControllerTest < ActionDispatch::IntegrationTest Routes end - def setup - I18n.backend.store_translations('de', email_subject: '[Signed up] Welcome') + def test_send_mail + with_translation 'de', email_subject: '[Anmeldung] Willkommen' do + get '/test/send_mail' + assert_equal "Mail sent - Subject: [Anmeldung] Willkommen", @response.body + end end - def teardown - I18n.locale = :en - end + protected - def test_send_mail - get '/test/send_mail' - assert_equal "Mail sent", @response.body + def with_translation(locale, data) + I18n.backend.store_translations(locale, data) + yield + ensure + I18n.backend.reload! end end diff --git a/actionmailer/test/log_subscriber_test.rb b/actionmailer/test/log_subscriber_test.rb index 5f0bee88fd..e7a73d6c8e 100644 --- a/actionmailer/test/log_subscriber_test.rb +++ b/actionmailer/test/log_subscriber_test.rb @@ -1,7 +1,7 @@ -require "abstract_unit" +require 'abstract_unit' require 'mailers/base_mailer' -require "active_support/log_subscriber/test_helper" -require "action_mailer/log_subscriber" +require 'active_support/log_subscriber/test_helper' +require 'action_mailer/log_subscriber' class AMLogSubscriberTest < ActionMailer::TestCase include ActiveSupport::LogSubscriber::TestHelper @@ -31,6 +31,8 @@ class AMLogSubscriberTest < ActionMailer::TestCase assert_equal(2, @logger.logged(:debug).size) assert_match(/BaseMailer#welcome: processed outbound mail in [\d.]+ms/, @logger.logged(:debug).first) assert_match(/Welcome/, @logger.logged(:debug).second) + ensure + BaseMailer.deliveries.clear end def test_receive_is_notified diff --git a/actionmailer/test/mail_layout_test.rb b/actionmailer/test/mail_layout_test.rb index 7f959282cb..166dd096d4 100644 --- a/actionmailer/test/mail_layout_test.rb +++ b/actionmailer/test/mail_layout_test.rb @@ -44,16 +44,6 @@ class ExplicitLayoutMailer < ActionMailer::Base end class LayoutMailerTest < ActiveSupport::TestCase - def setup - set_delivery_method :test - ActionMailer::Base.perform_deliveries = true - ActionMailer::Base.deliveries.clear - end - - def teardown - restore_delivery_method - end - def test_should_pickup_default_layout mail = AutoLayoutMailer.hello assert_equal "Hello from layout Inside", mail.body.to_s.strip diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index d52ccd3d5e..be1f53faf5 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,28 @@ +* Fix 'Stack level too deep' when rendering `head :ok` in an action method + called 'status' in a controller. + + Fixes #13905. + + *Christiaan Van den Poel* + +* Add MKCALENDAR HTTP method (RFC 4791). + + *Sergey Karpesh* + +* Instrument fragment cache metrics. + + Adds `:controller`: and `:action` keys to the instrumentation payload + for the `*_fragment.action_controller` notifications. This allows tracking + e.g. the fragment cache hit rates for each controller action. + + *Daniel Schierbeck* + +* Always use the provided port if the protocol is relative. + + Fixes #15043. + + *Guilherme Cavalcanti*, *Andrew White* + * Moved `params[request_forgery_protection_token]` into its own method and improved tests. diff --git a/actionpack/lib/abstract_controller/base.rb b/actionpack/lib/abstract_controller/base.rb index 6d200a0333..c00f0d0c6f 100644 --- a/actionpack/lib/abstract_controller/base.rb +++ b/actionpack/lib/abstract_controller/base.rb @@ -8,7 +8,8 @@ module AbstractController class Error < StandardError #:nodoc: end - class ActionNotFound < StandardError #:nodoc: + # Raised when a non-existing controller action is triggered. + class ActionNotFound < StandardError end # <tt>AbstractController::Base</tt> is a low-level API. Nobody should be @@ -120,7 +121,7 @@ module AbstractController # # The actual method that is called is determined by calling # #method_for_action. If no method can handle the action, then an - # ActionNotFound error is raised. + # AbstractController::ActionNotFound error is raised. # # ==== Returns # * <tt>self</tt> @@ -215,7 +216,8 @@ module AbstractController # # ==== Returns # * <tt>string</tt> - The name of the method that handles the action - # * false - No valid method name could be found. Raise ActionNotFound. + # * false - No valid method name could be found. + # Raise AbstractController::ActionNotFound. def _find_action_name(action_name) _valid_action_name?(action_name) && method_for_action(action_name) end @@ -235,7 +237,7 @@ module AbstractController # the case. # # If none of these conditions are true, and method_for_action - # returns nil, an ActionNotFound exception will be raised. + # returns nil, an AbstractController::ActionNotFound exception will be raised. # # ==== Parameters # * <tt>action_name</tt> - An action name to find a method name for diff --git a/actionpack/lib/abstract_controller/url_for.rb b/actionpack/lib/abstract_controller/url_for.rb index 4a95e1f276..72d07b0927 100644 --- a/actionpack/lib/abstract_controller/url_for.rb +++ b/actionpack/lib/abstract_controller/url_for.rb @@ -11,7 +11,7 @@ module AbstractController def _routes raise "In order to use #url_for, you must include routing helpers explicitly. " \ - "For instance, `include Rails.application.routes.url_helpers" + "For instance, `include Rails.application.routes.url_helpers`." end module ClassMethods diff --git a/actionpack/lib/action_controller/caching/fragments.rb b/actionpack/lib/action_controller/caching/fragments.rb index 879d5fdd94..2694d4c12f 100644 --- a/actionpack/lib/action_controller/caching/fragments.rb +++ b/actionpack/lib/action_controller/caching/fragments.rb @@ -90,7 +90,13 @@ module ActionController end def instrument_fragment_cache(name, key) # :nodoc: - ActiveSupport::Notifications.instrument("#{name}.action_controller", :key => key){ yield } + payload = { + controller: controller_name, + action: action_name, + key: key + } + + ActiveSupport::Notifications.instrument("#{name}.action_controller", payload) { yield } end end end diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 0f4cc7a8f5..696fbf6e09 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -232,5 +232,9 @@ module ActionController new.dispatch(name, klass.new(env)) end end + + def _status_code + @_status + end end end diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index 43407f5b78..84a9112144 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -27,7 +27,7 @@ module ActionController self.status = status self.location = url_for(location) if location - if include_content?(self.status) + if include_content?(self._status_code) self.content_type = content_type || (Mime[formats.first] if formats) self.response.charset = false if self.response self.response_body = " " diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index acf40b2e16..4c0554d27b 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -102,7 +102,8 @@ module ActionController end end - @stream.write "data: #{json}\n\n" + message = json.gsub("\n", "\ndata: ") + @stream.write "data: #{message}\n\n" end end diff --git a/actionpack/lib/action_controller/metal/rack_delegation.rb b/actionpack/lib/action_controller/metal/rack_delegation.rb index bdf6e88699..6921834044 100644 --- a/actionpack/lib/action_controller/metal/rack_delegation.rb +++ b/actionpack/lib/action_controller/metal/rack_delegation.rb @@ -6,7 +6,7 @@ module ActionController extend ActiveSupport::Concern delegate :headers, :status=, :location=, :content_type=, - :status, :location, :content_type, :to => "@_response" + :status, :location, :content_type, :_status_code, :to => "@_response" def dispatch(action, request) set_response!(request) diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index 788eed8b89..3e607bbde1 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -1,5 +1,10 @@ module ActionDispatch module Http + # Provides access to the request's HTTP headers from the environment. + # + # env = { "CONTENT_TYPE" => "text/plain" } + # headers = ActionDispatch::Http::Headers.new(env) + # headers["Content-Type"] # => "text/plain" class Headers CGI_VARIABLES = %w( CONTENT_TYPE CONTENT_LENGTH @@ -14,14 +19,16 @@ module ActionDispatch include Enumerable attr_reader :env - def initialize(env = {}) + def initialize(env = {}) # :nodoc: @env = env end + # Returns the value for the given key mapped to @env. def [](key) @env[env_name(key)] end + # Sets the given value for the key mapped to @env. def []=(key, value) @env[env_name(key)] = value end @@ -31,6 +38,13 @@ module ActionDispatch end alias :include? :key? + # Returns the value for the given key mapped to @env. + # + # If the key is not found and an optional code block is not provided, + # raises a <tt>KeyError</tt> exception. + # + # If the code block is provided, then it will be run and + # its result returned. def fetch(key, *args, &block) @env.fetch env_name(key), *args, &block end @@ -39,12 +53,17 @@ module ActionDispatch @env.each(&block) end + # Returns a new Http::Headers instance containing the contents of + # <tt>headers_or_env</tt> and the original instance. def merge(headers_or_env) headers = Http::Headers.new(env.dup) headers.merge!(headers_or_env) headers end + # Adds the contents of <tt>headers_or_env</tt> to original instance + # entries; duplicate keys are overwritten with the values from + # <tt>headers_or_env</tt>. def merge!(headers_or_env) headers_or_env.each do |key, value| self[env_name(key)] = value @@ -52,6 +71,8 @@ module ActionDispatch end private + # Converts a HTTP header name to an environment variable name if it is + # not contained within the headers hash. def env_name(key) key = key.to_s if key =~ HTTP_HEADER diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index daa06e96e6..cdb3e44b3a 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -64,6 +64,7 @@ module ActionDispatch # Ordered Collections Protocol (WebDAV) (http://www.ietf.org/rfc/rfc3648.txt) # Web Distributed Authoring and Versioning (WebDAV) Access Control Protocol (http://www.ietf.org/rfc/rfc3744.txt) # Web Distributed Authoring and Versioning (WebDAV) SEARCH (http://www.ietf.org/rfc/rfc5323.txt) + # Calendar Extensions to WebDAV (http://www.ietf.org/rfc/rfc4791.txt) # PATCH Method for HTTP (http://www.ietf.org/rfc/rfc5789.txt) RFC2616 = %w(OPTIONS GET HEAD POST PUT DELETE TRACE CONNECT) RFC2518 = %w(PROPFIND PROPPATCH MKCOL COPY MOVE LOCK UNLOCK) @@ -71,9 +72,10 @@ module ActionDispatch RFC3648 = %w(ORDERPATCH) RFC3744 = %w(ACL) RFC5323 = %w(SEARCH) + RFC4791 = %w(MKCALENDAR) RFC5789 = %w(PATCH) - HTTP_METHODS = RFC2616 + RFC2518 + RFC3253 + RFC3648 + RFC3744 + RFC5323 + RFC5789 + HTTP_METHODS = RFC2616 + RFC2518 + RFC3253 + RFC3648 + RFC3744 + RFC5323 + RFC4791 + RFC5789 HTTP_METHOD_LOOKUP = {} diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index 3d27ff2b24..eaea93b730 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -296,6 +296,9 @@ module ActionDispatch # :nodoc: cookies end + def _status_code + @status + end private def before_committed diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 6f5a52c568..c9860af909 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -29,15 +29,12 @@ module ActionDispatch extract_subdomains(host, tld_length).join('.') end - def url_for(options = {}) - options = options.dup - path = options.delete(:script_name).to_s.chomp("/") - path << options.delete(:path).to_s - - params = options[:params].is_a?(Hash) ? options[:params] : options.slice(:params) - params.reject! { |_,v| v.to_param.nil? } + def url_for(options) + path = options[:script_name].to_s.chomp("/") + path << options[:path].to_s result = build_host_url(options) + if options[:trailing_slash] if path.include?('?') result << path.sub(/\?/, '/\&') @@ -47,7 +44,16 @@ module ActionDispatch else result << path end - result << "?#{params.to_query}" unless params.empty? + + if options.key? :params + params = options[:params].is_a?(Hash) ? + options[:params] : + { params: options[:params] } + + params.reject! { |_,v| v.to_param.nil? } + result << "?#{params.to_query}" unless params.empty? + end + result << "##{Journey::Router::Utils.escape_fragment(options[:anchor].to_param.to_s)}" if options[:anchor] result end @@ -55,7 +61,7 @@ module ActionDispatch private def build_host_url(options) - if options[:host].blank? && options[:only_path].blank? + unless options[:host] || options[:only_path] raise ArgumentError, 'Missing host to link to! Please provide the :host parameter, set default_url_options[:host], or set :only_path to true' end @@ -130,7 +136,7 @@ module ActionDispatch case options[:protocol] when "//" - nil + options[:port] when "https://" options[:port].to_i == 443 ? nil : options[:port] else diff --git a/actionpack/lib/action_dispatch/routing.rb b/actionpack/lib/action_dispatch/routing.rb index 9cd884daa3..ce03164ca9 100644 --- a/actionpack/lib/action_dispatch/routing.rb +++ b/actionpack/lib/action_dispatch/routing.rb @@ -1,6 +1,7 @@ # encoding: UTF-8 require 'active_support/core_ext/object/to_param' require 'active_support/core_ext/regexp' +require 'active_support/dependencies/autoload' module ActionDispatch # The routing module provides URL rewriting in native Ruby. It's a way to diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 4c20974ac7..58c7f5330e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -578,18 +578,17 @@ module ActionDispatch _route = @set.named_routes.routes[name.to_sym] _routes = @set app.routes.define_mounted_helper(name) - app.routes.singleton_class.class_eval do - redefine_method :mounted? do - true - end - - redefine_method :_generate_prefix do |options| + app.routes.extend Module.new { + def mounted?; true; end + define_method :find_script_name do |options| + super(options) || begin prefix_options = options.slice(*_route.segment_keys) # we must actually delete prefix segment keys to avoid passing them to next url_for _route.segment_keys.each { |k| options.delete(k) } _routes.url_helpers.send("#{name}_path", prefix_options) + end end - end + } end end diff --git a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb index b800ee6448..bd3696cda1 100644 --- a/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb +++ b/actionpack/lib/action_dispatch/routing/polymorphic_routes.rb @@ -101,58 +101,45 @@ module ActionDispatch # polymorphic_url(Comment) # same as comments_url() # def polymorphic_url(record_or_hash_or_array, options = {}) - recipient = self - - if record_or_hash_or_array.kind_of?(Array) - if record_or_hash_or_array.include? nil - raise ArgumentError, "Nil location provided. Can't build URI." - end - record_or_hash_or_array = record_or_hash_or_array.dup - if record_or_hash_or_array.first.is_a?(ActionDispatch::Routing::RoutesProxy) - recipient = record_or_hash_or_array.shift - end - record_or_hash_or_array = record_or_hash_or_array[0] if record_or_hash_or_array.size == 1 + if Hash === record_or_hash_or_array + options = record_or_hash_or_array.merge(options) + record = options.delete :id + return polymorphic_url record, options end - record = extract_record(record_or_hash_or_array) - record = convert_to_model(record) - - args = Array === record_or_hash_or_array ? - record_or_hash_or_array.dup : - [ record_or_hash_or_array ] + opts = options.dup + action = opts.delete :action + type = opts.delete(:routing_type) || :url - inflection = if options[:action] && options[:action].to_s == "new" - args.pop - :singular - elsif (record.respond_to?(:persisted?) && !record.persisted?) - args.pop - :plural - elsif record.is_a?(Class) - args.pop - :plural - else - :singular - end - - args.delete_if {|arg| arg.is_a?(Symbol) || arg.is_a?(String)} - named_route = build_named_route_call(record_or_hash_or_array, record, inflection, options) - - url_options = options.except(:action, :routing_type) - unless url_options.empty? - args << url_options - end + HelperMethodBuilder.polymorphic_method self, + record_or_hash_or_array, + action, + type, + opts - args.collect! { |a| convert_to_model(a) } - - recipient.send(named_route, *args) end # Returns the path component of a URL for the given record. It uses # <tt>polymorphic_url</tt> with <tt>routing_type: :path</tt>. def polymorphic_path(record_or_hash_or_array, options = {}) - polymorphic_url(record_or_hash_or_array, options.merge(:routing_type => :path)) + if Hash === record_or_hash_or_array + options = record_or_hash_or_array.merge(options) + record = options.delete :id + return polymorphic_path record, options + end + + opts = options.dup + action = opts.delete :action + type = :path + + HelperMethodBuilder.polymorphic_method self, + record_or_hash_or_array, + action, + type, + opts end + %w(edit new).each do |action| module_eval <<-EOT, __FILE__, __LINE__ + 1 def #{action}_polymorphic_url(record_or_hash, options = {}) # def edit_polymorphic_url(record_or_hash, options = {}) @@ -170,53 +157,169 @@ module ActionDispatch end private - def action_prefix(options) - options[:action] ? "#{options[:action]}_" : '' + + class HelperMethodBuilder # :nodoc: + CACHE = { 'path' => {}, 'url' => {} } + + def self.get(action, type) + type = type.to_s + CACHE[type].fetch(action) { build action, type } + end + + def self.url; CACHE['url'.freeze][nil]; end + def self.path; CACHE['path'.freeze][nil]; end + + def self.build(action, type) + prefix = action ? "#{action}_" : "" + suffix = type + if action.to_s == 'new' + HelperMethodBuilder.singular prefix, suffix + else + HelperMethodBuilder.plural prefix, suffix + end end - def routing_type(options) - options[:routing_type] || :url + def self.singular(prefix, suffix) + new(->(name) { name.singular_route_key }, prefix, suffix) end - def build_named_route_call(records, record, inflection, options = {}) - if records.is_a?(Array) - record = records.pop - route = records.map do |parent| - if parent.is_a?(Symbol) || parent.is_a?(String) - parent - else - model_name_from_record_or_class(parent).singular_route_key - end + def self.plural(prefix, suffix) + new(->(name) { name.route_key }, prefix, suffix) + end + + def self.polymorphic_method(recipient, record_or_hash_or_array, action, type, options) + builder = get action, type + + case record_or_hash_or_array + when Array + if record_or_hash_or_array.empty? || record_or_hash_or_array.include?(nil) + raise ArgumentError, "Nil location provided. Can't build URI." + end + if record_or_hash_or_array.first.is_a?(ActionDispatch::Routing::RoutesProxy) + recipient = record_or_hash_or_array.shift end + + method, args = builder.handle_list record_or_hash_or_array + when String, Symbol + method, args = builder.handle_string record_or_hash_or_array + when Class + method, args = builder.handle_class record_or_hash_or_array + + when nil + raise ArgumentError, "Nil location provided. Can't build URI." + else + method, args = builder.handle_model record_or_hash_or_array + end + + + if options.empty? + recipient.send(method, *args) else - route = [] + recipient.send(method, *args, options) end + end - if record.is_a?(Symbol) || record.is_a?(String) - route << record - elsif record - if inflection == :singular - route << model_name_from_record_or_class(record).singular_route_key + attr_reader :suffix, :prefix + + def initialize(key_strategy, prefix, suffix) + @key_strategy = key_strategy + @prefix = prefix + @suffix = suffix + end + + def handle_string(record) + [get_method_for_string(record), []] + end + + def handle_string_call(target, str) + target.send get_method_for_string str + end + + def handle_class(klass) + [get_method_for_class(klass), []] + end + + def handle_class_call(target, klass) + target.send get_method_for_class klass + end + + def handle_model(record) + args = [] + + model = record.to_model + name = if record.persisted? + args << model + model.class.model_name.singular_route_key + else + @key_strategy.call model.class.model_name + end + + named_route = prefix + "#{name}_#{suffix}" + + [named_route, args] + end + + def handle_model_call(target, model) + method, args = handle_model model + target.send(method, *args) + end + + def handle_list(list) + record_list = list.dup + record = record_list.pop + + args = [] + + route = record_list.map { |parent| + case parent + when Symbol, String + parent.to_s + when Class + args << parent + parent.model_name.singular_route_key else - route << model_name_from_record_or_class(record).route_key + args << parent.to_model + parent.to_model.class.model_name.singular_route_key end + } + + route << + case record + when Symbol, String + record.to_s + when Class + @key_strategy.call record.model_name else - raise ArgumentError, "Nil location provided. Can't build URI." + if record.persisted? + args << record.to_model + record.to_model.class.model_name.singular_route_key + else + @key_strategy.call record.to_model.class.model_name + end end - route << routing_type(options) + route << suffix - action_prefix(options) + route.join("_") + named_route = prefix + route.join("_") + [named_route, args] end - def extract_record(record_or_hash_or_array) - case record_or_hash_or_array - when Array; record_or_hash_or_array.last - when Hash; record_or_hash_or_array[:id] - else record_or_hash_or_array - end + private + + def get_method_for_class(klass) + name = @key_strategy.call klass.model_name + prefix + "#{name}_#{suffix}" + end + + def get_method_for_string(str) + prefix + "#{str}_#{suffix}" end + + [nil, 'new', 'edit'].each do |action| + CACHE['url'][action] = build action, 'url' + CACHE['path'][action] = build action, 'path' + end + end end end end - diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 1ec6fa674b..e699419f23 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -1,11 +1,13 @@ require 'action_dispatch/journey' require 'forwardable' require 'thread_safe' +require 'active_support/concern' require 'active_support/core_ext/object/to_query' require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/module/remove_method' require 'active_support/core_ext/array/extract_options' require 'action_controller/metal/exceptions' +require 'action_dispatch/http/request' module ActionDispatch module Routing @@ -155,7 +157,7 @@ module ActionDispatch end def self.optimize_helper?(route) - !route.glob? && route.requirements.except(:controller, :action).empty? + !route.glob? && route.path.requirements.empty? end class OptimizedUrlHelper < UrlHelper # :nodoc: @@ -171,8 +173,7 @@ module ActionDispatch def call(t, args) if args.size == arg_size && !args.last.is_a?(Hash) && optimize_routes_generation?(t) - options = @options.dup - options.merge!(t.url_options) if t.respond_to?(:url_options) + options = t.url_options.merge @options options[:path] = optimized_helper(args) ActionDispatch::Http::URL.url_for(options) else @@ -225,17 +226,18 @@ module ActionDispatch end def call(t, args) - t.url_for(handle_positional_args(t, args, @options, @segment_keys)) + options = t.url_options.merge @options + hash = handle_positional_args(t, args, options, @segment_keys) + t._routes.url_for(hash) end - def handle_positional_args(t, args, options, keys) + def handle_positional_args(t, args, result, keys) inner_options = args.extract_options! - result = options.dup if args.size > 0 if args.size < keys.size - 1 # take format into account - keys -= t.url_options.keys if t.respond_to?(:url_options) - keys -= options.keys + keys -= t.url_options.keys + keys -= result.keys end keys -= inner_options.keys result.merge!(Hash[keys.zip(args)]) @@ -393,6 +395,8 @@ module ActionDispatch @_routes = routes class << self delegate :url_for, :optimize_routes_generation?, :to => '@_routes' + attr_reader :_routes + def url_options; {}; end end # Make named_routes available in the module singleton @@ -639,28 +643,34 @@ module ActionDispatch !mounted? && default_url_options.empty? end - def _generate_prefix(options = {}) - nil + def find_script_name(options) + options.delete :script_name end - # The +options+ argument must be +nil+ or a hash whose keys are *symbols*. + # The +options+ argument must be a hash whose keys are *symbols*. def url_for(options) - options = default_url_options.merge(options || {}) + options = default_url_options.merge options + + user = password = nil - user, password = extract_authentication(options) - recall = options.delete(:_recall) + if options[:user] && options[:password] + user = options.delete :user + password = options.delete :password + end - original_script_name = options.delete(:original_script_name).presence - script_name = options.delete(:script_name).presence || _generate_prefix(options) + recall = options.delete(:_recall) { {} } + + original_script_name = options.delete(:original_script_name) + script_name = find_script_name options if script_name && original_script_name script_name = original_script_name + script_name end - path_options = options.except(*RESERVED_OPTIONS) - path_options = yield(path_options) if block_given? + path_options = options.dup + RESERVED_OPTIONS.each { |ro| path_options.delete ro } - path, params = generate(path_options, recall || {}) + path, params = generate(path_options, recall) params.merge!(options[:params] || {}) ActionDispatch::Http::URL.url_for(options.merge!({ @@ -715,17 +725,6 @@ module ActionDispatch raise ActionController::RoutingError, "No route matches #{path.inspect}" end - - private - - def extract_authentication(options) - if options[:user] && options[:password] - [options.delete(:user), options.delete(:password)] - else - nil - end - end - end end end diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 4a0ef40873..e624fe3c4a 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -155,10 +155,14 @@ module ActionDispatch _routes.url_for(options.symbolize_keys.reverse_merge!(url_options)) when String options + when Symbol + HelperMethodBuilder.url.handle_string_call self, options when Array polymorphic_url(options, options.extract_options!) + when Class + HelperMethodBuilder.url.handle_class_call self, options else - polymorphic_url(options) + HelperMethodBuilder.url.handle_model_call self, options end end diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index 68feb26936..0adc6c84ff 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -73,7 +73,13 @@ module ActionDispatch if Regexp === fragment fragment else - @controller._compute_redirect_to_location(fragment) + handle = @controller || Class.new(ActionController::Metal) do + include ActionController::Redirecting + def initialize(request) + @_request = request + end + end.new(@request) + handle._compute_redirect_to_location(fragment) end end end diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 03a4741f42..46de36317e 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -320,8 +320,8 @@ module ActionDispatch end module RoutingTestHelpers - def url_for(set, options, recall = nil) - set.send(:url_for, options.merge(:only_path => true, :_recall => recall)) + def url_for(set, options, recall = {}) + set.url_for options.merge(:only_path => true, :_recall => recall) end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 58a86ce9af..c0e6a2ebd1 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -227,6 +227,22 @@ CACHED @store.read("views/test.host/functional_caching/inline_fragment_cached/#{template_digest("functional_caching/inline_fragment_cached")}")) end + def test_fragment_cache_instrumentation + payload = nil + + subscriber = proc do |*args| + event = ActiveSupport::Notifications::Event.new(*args) + payload = event.payload + end + + ActiveSupport::Notifications.subscribed(subscriber, "read_fragment.action_controller") do + get :inline_fragment_cached + end + + assert_equal "functional_caching", payload[:controller] + assert_equal "inline_fragment_cached", payload[:action] + end + def test_html_formatted_fragment_caching get :formatted_fragment_cached, :format => "html" assert_response :success diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index e851cc6a63..214eab2f0d 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -374,6 +374,10 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest follow_redirect! assert_response :success assert_equal "/get", path + + get '/moved' + assert_response :redirect + assert_redirected_to '/method' end end @@ -511,6 +515,8 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end set.draw do + get 'moved' => redirect('/method') + match ':action', :to => controller, :via => [:get, :post], :as => :action get 'get/:action', :to => controller, :as => :get_action end @@ -769,3 +775,34 @@ class UrlOptionsIntegrationTest < ActionDispatch::IntegrationTest assert_equal "/foo/1/edit", url_for(:action => 'edit', :only_path => true) end end + +class HeadWithStatusActionIntegrationTest < ActionDispatch::IntegrationTest + class FooController < ActionController::Base + def status + head :ok + end + end + + def self.routes + @routes ||= ActionDispatch::Routing::RouteSet.new + end + + def self.call(env) + routes.call(env) + end + + def app + self.class + end + + routes.draw do + get "/foo/status" => 'head_with_status_action_integration_test/foo#status' + end + + test "get /foo/status with head result does not cause stack overflow error" do + assert_nothing_raised do + get '/foo/status' + end + assert_response :ok + end +end diff --git a/actionpack/test/controller/live_stream_test.rb b/actionpack/test/controller/live_stream_test.rb index 947f64176b..1dca36374a 100644 --- a/actionpack/test/controller/live_stream_test.rb +++ b/actionpack/test/controller/live_stream_test.rb @@ -39,6 +39,13 @@ module ActionController ensure sse.close end + + def sse_with_multiple_line_message + sse = SSE.new(response.stream) + sse.write("first line.\nsecond line.") + ensure + sse.close + end end tests SSETestController @@ -87,6 +94,15 @@ module ActionController assert_match(/data: {\"name\":\"Ryan\"}/, second_response) assert_match(/id: 2/, second_response) end + + def test_sse_with_multiple_line_message + get :sse_with_multiple_line_message + + wait_for_response_stream_close + first_response, second_response = response.body.split("\n") + assert_match(/data: first line/, first_response) + assert_match(/data: second line/, second_response) + end end class LiveStreamTest < ActionController::TestCase diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index df453a0251..b22bc2dc25 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -419,14 +419,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase get 'page' => 'content#show_page', :as => 'pages', :host => 'foo.com' end routes = setup_for_named_route - routes.expects(:url_for).with({ - :host => 'foo.com', - :only_path => false, - :controller => 'content', - :action => 'show_page', - :use_route => 'pages' - }).once - routes.send(:pages_url) + assert_equal "http://foo.com/page", routes.pages_url end def setup_for_named_route(options = {}) diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index b0983a5252..aee139b95e 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -200,8 +200,6 @@ class SendFileTest < ActionController::TestCase end end - tests SendFileWithActionControllerLive - def test_send_file_with_action_controller_live @controller = SendFileWithActionControllerLive.new @controller.options = { :content_type => "application/x-ruby" } diff --git a/actionpack/test/controller/url_for_test.rb b/actionpack/test/controller/url_for_test.rb index a8035e5bd7..0c6df16325 100644 --- a/actionpack/test/controller/url_for_test.rb +++ b/actionpack/test/controller/url_for_test.rb @@ -169,6 +169,18 @@ module AbstractController ) end + def test_without_protocol_and_with_port + add_host! + add_port! + + assert_equal('//www.basecamphq.com:3000/c/a/i', + W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => '//') + ) + assert_equal('//www.basecamphq.com:3000/c/a/i', + W.new.url_for(:controller => 'c', :action => 'a', :id => 'i', :protocol => false) + ) + end + def test_trailing_slash add_host! options = {:controller => 'foo', :trailing_slash => true, :action => 'bar', :id => '33'} diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index 08501d19c0..cd31e8e326 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -15,6 +15,9 @@ module TestGenerationPrefix ActiveModel::Name.new(klass) end + + def to_model; self; end + def persisted?; true; end end class WithMountedEngine < ActionDispatch::IntegrationTest diff --git a/actionpack/test/dispatch/routing/route_set_test.rb b/actionpack/test/dispatch/routing/route_set_test.rb index 0e488d2b88..c465d56bde 100644 --- a/actionpack/test/dispatch/routing/route_set_test.rb +++ b/actionpack/test/dispatch/routing/route_set_test.rb @@ -81,10 +81,6 @@ module ActionDispatch end private - def clear! - @set.clear! - end - def draw(&block) @set.draw(&block) end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 0a13dcfad4..cae6b312b6 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -3553,6 +3553,7 @@ class TestHttpMethods < ActionDispatch::IntegrationTest RFC3648 = %w(ORDERPATCH) RFC3744 = %w(ACL) RFC5323 = %w(SEARCH) + RFC4791 = %w(MKCALENDAR) RFC5789 = %w(PATCH) def simple_app(response) @@ -3564,13 +3565,13 @@ class TestHttpMethods < ActionDispatch::IntegrationTest @app = ActionDispatch::Routing::RouteSet.new @app.draw do - (RFC2616 + RFC2518 + RFC3253 + RFC3648 + RFC3744 + RFC5323 + RFC5789).each do |method| + (RFC2616 + RFC2518 + RFC3253 + RFC3648 + RFC3744 + RFC5323 + RFC4791 + RFC5789).each do |method| match '/' => s.simple_app(method), :via => method.underscore.to_sym end end end - (RFC2616 + RFC2518 + RFC3253 + RFC3648 + RFC3744 + RFC5323 + RFC5789).each do |method| + (RFC2616 + RFC2518 + RFC3253 + RFC3648 + RFC3744 + RFC5323 + RFC4791 + RFC5789).each do |method| test "request method #{method.underscore} can be matched" do get '/', nil, 'REQUEST_METHOD' => method assert_equal method, @response.body diff --git a/actionpack/test/dispatch/url_generation_test.rb b/actionpack/test/dispatch/url_generation_test.rb index fdea27e2d2..910ff8a80f 100644 --- a/actionpack/test/dispatch/url_generation_test.rb +++ b/actionpack/test/dispatch/url_generation_test.rb @@ -64,18 +64,30 @@ module TestUrlGeneration test "port is extracted from the host" do assert_equal "http://www.example.com:8080/foo", foo_url(host: "www.example.com:8080", protocol: "http://") + assert_equal "//www.example.com:8080/foo", foo_url(host: "www.example.com:8080", protocol: "//") + assert_equal "//www.example.com:80/foo", foo_url(host: "www.example.com:80", protocol: "//") + end + + test "port option is used" do + assert_equal "http://www.example.com:8080/foo", foo_url(host: "www.example.com", protocol: "http://", port: 8080) + assert_equal "//www.example.com:8080/foo", foo_url(host: "www.example.com", protocol: "//", port: 8080) + assert_equal "//www.example.com:80/foo", foo_url(host: "www.example.com", protocol: "//", port: 80) end test "port option overrides the host" do assert_equal "http://www.example.com:8080/foo", foo_url(host: "www.example.com:8443", protocol: "http://", port: 8080) + assert_equal "//www.example.com:8080/foo", foo_url(host: "www.example.com:8443", protocol: "//", port: 8080) + assert_equal "//www.example.com:80/foo", foo_url(host: "www.example.com:443", protocol: "//", port: 80) end test "port option disables the host when set to nil" do assert_equal "http://www.example.com/foo", foo_url(host: "www.example.com:8443", protocol: "http://", port: nil) + assert_equal "//www.example.com/foo", foo_url(host: "www.example.com:8443", protocol: "//", port: nil) end test "port option disables the host when set to false" do assert_equal "http://www.example.com/foo", foo_url(host: "www.example.com:8443", protocol: "http://", port: false) + assert_equal "//www.example.com/foo", foo_url(host: "www.example.com:8443", protocol: "//", port: false) end test "keep subdomain when key is true" do diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 36028122d1..147e5b47db 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,4 +1,15 @@ -* Take label values into account when doing I18n lookups for model attributes. +* Allow custom `:host` option to be passed to `asset_url` helper that + overwrites `config.action_controller.asset_host` for particular asset. + + *Hubert Łępicki* + +* Deprecate `AbstractController::Base.parent_prefixes`. + Override `AbstractController::Base.local_prefixes` when you want to change + where to find views. + + *Nick Sutterer* + +* Take label values into account when doing I18n lookups for model attributes. The following: @@ -20,9 +31,11 @@ * Change `asset_path` to use File.join to create proper paths: + Before: + https://some.host.com//assets/some.js - becomes + After: https://some.host.com/assets/some.js @@ -87,9 +100,4 @@ *Piotr Chmolowski, Łukasz Strzałkowski* -* Allow custom `:host` option to be passed to `asset_url` helper that - overwrites `config.action_controller.asset_host` for particular asset. - - *Hubert Łępicki* - Please check [4-1-stable](https://github.com/rails/rails/blob/4-1-stable/actionview/CHANGELOG.md) for previous changes. diff --git a/actionview/lib/action_view/helpers/form_helper.rb b/actionview/lib/action_view/helpers/form_helper.rb index 22bfd87d85..180c4a62bf 100644 --- a/actionview/lib/action_view/helpers/form_helper.rb +++ b/actionview/lib/action_view/helpers/form_helper.rb @@ -449,7 +449,11 @@ module ActionView method: method ) - options[:url] ||= polymorphic_path(record, format: options.delete(:format)) + options[:url] ||= if options.key?(:format) + polymorphic_path(record, format: options.delete(:format)) + else + polymorphic_path(record, {}) + end end private :apply_form_for_options! diff --git a/actionview/lib/action_view/helpers/translation_helper.rb b/actionview/lib/action_view/helpers/translation_helper.rb index 0bc40874d9..17ec6a40bf 100644 --- a/actionview/lib/action_view/helpers/translation_helper.rb +++ b/actionview/lib/action_view/helpers/translation_helper.rb @@ -7,7 +7,7 @@ module ActionView module TranslationHelper # Delegates to <tt>I18n#translate</tt> but also performs three additional functions. # - # First, it will ensure that any thrown +MissingTranslation+ messages will be turned + # First, it will ensure that any thrown +MissingTranslation+ messages will be turned # into inline spans that: # # * have a "translation-missing" class set, @@ -34,6 +34,7 @@ module ActionView # naming convention helps to identify translations that include HTML tags so that # you know what kind of output to expect when you call translate in a template. def translate(key, options = {}) + options = options.dup options[:default] = wrap_translate_defaults(options[:default]) if options[:default] # If the user has specified rescue_format then pass it all through, otherwise use diff --git a/actionview/lib/action_view/routing_url_for.rb b/actionview/lib/action_view/routing_url_for.rb index b9e4b590e7..881a123572 100644 --- a/actionview/lib/action_view/routing_url_for.rb +++ b/actionview/lib/action_view/routing_url_for.rb @@ -1,3 +1,5 @@ +require 'action_dispatch/routing/polymorphic_routes' + module ActionView module RoutingUrlFor @@ -83,10 +85,14 @@ module ActionView super({ :only_path => options[:host].nil? }.merge!(options.symbolize_keys)) when :back _back_url + when Symbol + ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_string_call self, options when Array polymorphic_path(options, options.extract_options!) + when Class + ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_class_call self, options else - polymorphic_path(options) + ActionDispatch::Routing::PolymorphicRoutes::HelperMethodBuilder.path.handle_model_call self, options end end diff --git a/actionview/lib/action_view/template/resolver.rb b/actionview/lib/action_view/template/resolver.rb index 05f0c301e7..189086132e 100644 --- a/actionview/lib/action_view/template/resolver.rb +++ b/actionview/lib/action_view/template/resolver.rb @@ -181,13 +181,7 @@ module ActionView def query(path, details, formats) query = build_query(path, details) - # deals with case-insensitive file systems. - sanitizer = Hash.new { |h,dir| h[dir] = Dir["#{dir}/*"] } - - template_paths = Dir[query].reject { |filename| - File.directory?(filename) || - !sanitizer[File.dirname(filename)].include?(filename) - } + template_paths = find_template_paths query template_paths.map { |template| handler, format, variant = extract_handler_and_format_and_variant(template, formats) @@ -202,6 +196,26 @@ module ActionView } end + if File.const_defined? :FNM_EXTGLOB + def find_template_paths(query) + Dir[query].reject { |filename| + File.directory?(filename) || + # deals with case-insensitive file systems. + !File.fnmatch(query, filename, File::FNM_EXTGLOB) + } + end + else + def find_template_paths(query) + # deals with case-insensitive file systems. + sanitizer = Hash.new { |h,dir| h[dir] = Dir["#{dir}/*"] } + + Dir[query].reject { |filename| + File.directory?(filename) || + !sanitizer[File.dirname(filename)].include?(filename) + } + end + end + # Helper for building query glob string based on resolver's pattern. def build_query(path, details) query = @pattern.dup diff --git a/actionview/lib/action_view/view_paths.rb b/actionview/lib/action_view/view_paths.rb index 6c349feb1d..80a41f2418 100644 --- a/actionview/lib/action_view/view_paths.rb +++ b/actionview/lib/action_view/view_paths.rb @@ -14,27 +14,38 @@ module ActionView :locale, :locale=, :to => :lookup_context module ClassMethods - def parent_prefixes - @parent_prefixes ||= begin - parent_controller = superclass - prefixes = [] - - until parent_controller.abstract? - prefixes << parent_controller.controller_path - parent_controller = parent_controller.superclass - end + def _prefixes # :nodoc: + @_prefixes ||= begin + deprecated_prefixes = handle_deprecated_parent_prefixes + if deprecated_prefixes + deprecated_prefixes + else + return local_prefixes if superclass.abstract? - prefixes + local_prefixes + superclass._prefixes + end end end + + private + + # Override this method in your controller if you want to change paths prefixes for finding views. + # Prefixes defined here will still be added to parents' <tt>._prefixes</tt>. + def local_prefixes + [controller_path] + end + + def handle_deprecated_parent_prefixes # TODO: remove in 4.3/5.0. + return unless respond_to?(:parent_prefixes) + + ActiveSupport::Deprecation.warn "Overriding ActionController::Base::parent_prefixes is deprecated, override .local_prefixes instead." + local_prefixes + parent_prefixes + end end # The prefixes used in render "foo" shortcuts. - def _prefixes - @_prefixes ||= begin - parent_prefixes = self.class.parent_prefixes - parent_prefixes.dup.unshift(controller_path) - end + def _prefixes # :nodoc: + self.class._prefixes end # LookupContext is the object responsible to hold all information required to lookup diff --git a/actionview/test/actionpack/abstract/abstract_controller_test.rb b/actionview/test/actionpack/abstract/abstract_controller_test.rb index 40d3b17131..e653b12d32 100644 --- a/actionview/test/actionpack/abstract/abstract_controller_test.rb +++ b/actionview/test/actionpack/abstract/abstract_controller_test.rb @@ -150,6 +150,54 @@ module AbstractController end end + class OverridingLocalPrefixes < AbstractController::Base + include AbstractController::Rendering + include ActionView::Rendering + append_view_path File.expand_path(File.join(File.dirname(__FILE__), "views")) + + def index + render + end + + def self.local_prefixes + # this would usually return "abstract_controller/testing/overriding_local_prefixes" + super + ["abstract_controller/testing/me3"] + end + + class Inheriting < self + end + end + + class OverridingLocalPrefixesTest < ActiveSupport::TestCase # TODO: remove me in 5.0/4.3. + test "overriding .local_prefixes adds prefix" do + @controller = OverridingLocalPrefixes.new + @controller.process(:index) + assert_equal "Hello from me3/index.erb", @controller.response_body + end + + test ".local_prefixes is inherited" do + @controller = OverridingLocalPrefixes::Inheriting.new + @controller.process(:index) + assert_equal "Hello from me3/index.erb", @controller.response_body + end + end + + class DeprecatedParentPrefixes < OverridingLocalPrefixes + def self.parent_prefixes + ["abstract_controller/testing/me3"] + end + end + + class DeprecatedParentPrefixesTest < ActiveSupport::TestCase # TODO: remove me in 5.0/4.3. + test "overriding .parent_prefixes is deprecated" do + @controller = DeprecatedParentPrefixes.new + assert_deprecated do + @controller.process(:index) + end + assert_equal "Hello from me3/index.erb", @controller.response_body + end + end + # Test rendering with layouts # ==== # self._layout is used when defined diff --git a/actionview/test/activerecord/polymorphic_routes_test.rb b/actionview/test/activerecord/polymorphic_routes_test.rb index a42888d873..fef27ef492 100644 --- a/actionview/test/activerecord/polymorphic_routes_test.rb +++ b/actionview/test/activerecord/polymorphic_routes_test.rb @@ -75,6 +75,9 @@ class PolymorphicRoutesTest < ActionController::TestCase end def assert_url(url, args) + host = self.class.default_url_options[:host] + + assert_equal url.sub(/http:\/\/#{host}/, ''), polymorphic_path(args) assert_equal url, polymorphic_url(args) assert_equal url, url_for(args) end @@ -96,8 +99,7 @@ class PolymorphicRoutesTest < ActionController::TestCase def test_symbol with_test_routes do - assert_equal "http://example.com/projects", polymorphic_url(:projects) - assert_equal "http://example.com/projects", url_for(:projects) + assert_url "http://example.com/projects", :projects end end @@ -129,6 +131,23 @@ class PolymorphicRoutesTest < ActionController::TestCase end end + def test_polymorphic_url_with_2_objects + with_namespaced_routes(:blog) do + @blog_blog.save + @blog_post.save + assert_equal "http://example.com/blogs/#{@blog_blog.id}/posts/#{@blog_post.id}", polymorphic_url([@blog_blog, @blog_post]) + end + end + + def test_polymorphic_url_with_3_objects + with_namespaced_routes(:blog) do + @blog_blog.save + @blog_post.save + @fax.save + assert_equal "http://example.com/blogs/#{@blog_blog.id}/posts/#{@blog_post.id}/faxes/#{@fax.id}", polymorphic_url([@blog_blog, @blog_post, @fax]) + end + end + def test_namespaced_model_with_nested_resources with_namespaced_routes(:blog) do @blog_post.save @@ -183,6 +202,19 @@ class PolymorphicRoutesTest < ActionController::TestCase end end + def test_with_class_list_of_one + with_test_routes do + assert_url "http://example.com/projects", [@project.class] + end + end + + def test_class_with_options + with_test_routes do + assert_equal "http://example.com/projects?foo=bar", polymorphic_url(@project.class, { :foo => :bar }) + assert_equal "/projects?foo=bar", polymorphic_path(@project.class, { :foo => :bar }) + end + end + def test_with_new_record with_test_routes do assert_url "http://example.com/projects", @project @@ -191,14 +223,20 @@ class PolymorphicRoutesTest < ActionController::TestCase def test_new_record_arguments params = nil - extend Module.new { - define_method("projects_url") { |*args| - params = args - super(*args) - } - } with_test_routes do + extend Module.new { + define_method("projects_url") { |*args| + params = args + super(*args) + } + + define_method("projects_path") { |*args| + params = args + super(*args) + } + } + assert_url "http://example.com/projects", @project assert_equal [], params end @@ -389,6 +427,12 @@ class PolymorphicRoutesTest < ActionController::TestCase end end + def test_with_array_containing_single_string_name + with_test_routes do + assert_url "http://example.com/projects", ["projects"] + end + end + def test_with_array_containing_symbols with_test_routes do assert_url "http://example.com/series/new", [:new, :series] @@ -543,13 +587,15 @@ class PolymorphicRoutesTest < ActionController::TestCase set.draw do scope(:module => name) do resources :blogs do - resources :posts + resources :posts do + resources :faxes + end end resources :posts end end - self.class.send(:include, @routes.url_helpers) + extend @routes.url_helpers yield end end @@ -571,7 +617,7 @@ class PolymorphicRoutesTest < ActionController::TestCase resources :model_delegates end - self.class.send(:include, @routes.url_helpers) + extend @routes.url_helpers yield end end @@ -593,7 +639,7 @@ class PolymorphicRoutesTest < ActionController::TestCase end end - self.class.send(:include, @routes.url_helpers) + extend @routes.url_helpers yield end end @@ -612,8 +658,21 @@ class PolymorphicRoutesTest < ActionController::TestCase end end - self.class.send(:include, @routes.url_helpers) + extend @routes.url_helpers yield end end end + +class PolymorphicPathRoutesTest < PolymorphicRoutesTest + include ActionView::RoutingUrlFor + include ActionView::Context + + attr_accessor :controller + + def assert_url(url, args) + host = self.class.default_url_options[:host] + + assert_equal url.sub(/http:\/\/#{host}/, ''), url_for(args) + end +end diff --git a/actionview/test/template/sanitize_helper_test.rb b/actionview/test/template/sanitize_helper_test.rb index 12d5260a9d..f7c8f36b78 100644 --- a/actionview/test/template/sanitize_helper_test.rb +++ b/actionview/test/template/sanitize_helper_test.rb @@ -1,6 +1,6 @@ require 'abstract_unit' -# The exhaustive tests are in test/controller/html/sanitizer_test.rb. +# The exhaustive tests are in test/template/html-scanner/sanitizer_test.rb # This tests the that the helpers hook up correctly to the sanitizer classes. class SanitizeHelperTest < ActionView::TestCase tests ActionView::Helpers::SanitizeHelper diff --git a/actionview/test/template/test_test.rb b/actionview/test/template/test_test.rb index 5721ee6c6f..88bac85039 100644 --- a/actionview/test/template/test_test.rb +++ b/actionview/test/template/test_test.rb @@ -39,6 +39,8 @@ class PeopleHelperTest < ActionView::TestCase with_test_route_set do person = Struct.new(:name) { extend ActiveModel::Naming + def to_model; self; end + def persisted?; true; end def self.name; 'Mocha::Mock'; end }.new "David" diff --git a/actionview/test/template/translation_helper_test.rb b/actionview/test/template/translation_helper_test.rb index c4770840fb..a9d5ea7345 100644 --- a/actionview/test/template/translation_helper_test.rb +++ b/actionview/test/template/translation_helper_test.rb @@ -151,4 +151,10 @@ class TranslationHelperTest < ActiveSupport::TestCase translation = translate(:'translations.missing', default: ['A Generic String', 'Second generic string']) assert_equal 'A Generic String', translation end + + def test_translate_does_not_change_options + options = {} + translate(:'translations.missing', options) + assert_equal({}, options) + end end diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 68cc874ca3..b94558b65c 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,4 +1,4 @@ -* Add plural and singular form for length validator's default messages +* Add plural and singular form for length validator's default messages. *Abd ar-Rahman Hamid* diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index b684703923..5647204859 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,13 +1,91 @@ -* Return a non zero status when running `rake db:migrate:status` and migration table does - not exist. +* Floats with limit >= 25 that get turned into doubles in MySQL no longer have + their limit dropped from the schema. - *Paul B.* + Fixes #14135. + + *Aaron Nelson* + +* Fix how to calculate associated class name when using namespaced `has_and_belongs_to_many` + association. + + Fixes #14709. + + *Kassio Borges* + +* `ActiveRecord::Relation::Merger#filter_binds` now compares equivalent symbols and + strings in column names as equal. + + This fixes a rare case in which more bind values are passed than there are + placeholders for them in the generated SQL statement, which can make PostgreSQL + throw a `StatementInvalid` exception. + + *Nat Budin* + +* Fix `stored_attributes` to correctly merge the details of stored + attributes defined in parent classes. + + Fixes #14672. + + *Brad Bennett*, *Jessica Yao*, *Lakshmi Parthasarathy* + +* `change_column_default` allows `[]` as argument to `change_column_default`. + + Fixes #11586. + + *Yves Senn* + +* Handle `name` and `"char"` column types in the PostgreSQL adapter. + + `name` and `"char"` are special character types used internally by + PostgreSQL and are used by internal system catalogs. These field types + can sometimes show up in structure-sniffing queries that feature internal system + structures or with certain PostgreSQL extensions. + + *J Smith*, *Yves Senn* -* Keep track of dirty attributes after transaction is rollback. +* Fix `PostgreSQLAdapter::OID::Float#type_cast` to convert Infinity and + NaN PostgreSQL values into a native Ruby `Float::INFINITY` and `Float::NAN` - Related #13166. + Before: + + Point.create(value: 1.0/0) + Point.last.value # => 0.0 + + After: + + Point.create(value: 1.0/0) + Point.last.value # => Infinity + + *Innokenty Mikhailov* + +* Allow the PostgreSQL adapter to handle bigserial pk types again. + + Fixes #10410. + + *Patrick Robertson* + +* Deprecate joining, eager loading and preloading of instance dependent + associations without replacement. These operations happen before instances + are created. The current behavior is unexpected and can result in broken + behavior. + + Fixes #15024. - *Bogdan Gusiev* *arthurnn* + *Yves Senn* + +* Fixed HABTM's CollectionAssociation size calculation. + + HABTM should fall back to using the normal CollectionAssociation's size + calculation if the collection is not cached or loaded. + + Fixes #14913, #14914. + + *Fred Wu* + +* Return a non zero status when running `rake db:migrate:status` and migration table does + not exist. + + *Paul B.* * Add support for module-level `table_name_suffix` in models. @@ -202,7 +280,7 @@ * Auto-generate stable fixture UUIDs on PostgreSQL. - Fixes: #11524 + Fixes #11524. *Roderick van Domburg* diff --git a/activerecord/README.rdoc b/activerecord/README.rdoc index e04abe9b37..e5b68750e4 100644 --- a/activerecord/README.rdoc +++ b/activerecord/README.rdoc @@ -1,4 +1,4 @@ -= Active Record -- Object-relational mapping put on rails += Active Record -- Object-relational mapping in Rails Active Record connects classes to relational database tables to establish an almost zero-configuration persistence layer for applications. The library @@ -19,9 +19,11 @@ A short rundown of some of the major features: class Product < ActiveRecord::Base end - - The Product class is automatically mapped to the table named "products", - which might look like this: + + {Learn more}[link:classes/ActiveRecord/Base.html] + +The Product class is automatically mapped to the table named "products", +which might look like this: CREATE TABLE products ( id int(11) NOT NULL auto_increment, @@ -29,11 +31,9 @@ A short rundown of some of the major features: PRIMARY KEY (id) ); - This would also define the following accessors: `Product#name` and - `Product#name=(new_name)` - - {Learn more}[link:classes/ActiveRecord/Base.html] - +This would also define the following accessors: `Product#name` and +`Product#name=(new_name)`. + * Associations between objects defined by simple class methods. diff --git a/activerecord/Rakefile b/activerecord/Rakefile index 6f8948f987..84856774f2 100644 --- a/activerecord/Rakefile +++ b/activerecord/Rakefile @@ -142,53 +142,7 @@ task :drop_postgresql_databases => 'postgresql:drop_databases' task :rebuild_postgresql_databases => 'postgresql:rebuild_databases' -namespace :frontbase do - desc 'Build the FrontBase test databases' - task :build_databases => :rebuild_frontbase_databases - - desc 'Rebuild the FrontBase test databases' - task :rebuild_databases do - build_frontbase_database = Proc.new do |db_name, sql_definition_file| - %( - STOP DATABASE #{db_name}; - DELETE DATABASE #{db_name}; - CREATE DATABASE #{db_name}; - - CONNECT TO #{db_name} AS SESSION_NAME USER _SYSTEM; - SET COMMIT FALSE; - - CREATE USER RAILS; - CREATE SCHEMA RAILS AUTHORIZATION RAILS; - COMMIT; - - SET SESSION AUTHORIZATION RAILS; - SCRIPT '#{sql_definition_file}'; - - COMMIT; - - DISCONNECT ALL; - ) - end - config = ARTest.config['connections']['frontbase'] - create_activerecord_unittest = build_frontbase_database[config['arunit']['database'], File.join(SCHEMA_ROOT, 'frontbase.sql')] - create_activerecord_unittest2 = build_frontbase_database[config['arunit2']['database'], File.join(SCHEMA_ROOT, 'frontbase2.sql')] - execute_frontbase_sql = Proc.new do |sql| - system(<<-SHELL) - /Library/FrontBase/bin/sql92 <<-SQL - #{sql} - SQL - SHELL - end - execute_frontbase_sql[create_activerecord_unittest] - execute_frontbase_sql[create_activerecord_unittest2] - end -end - -task :build_frontbase_databases => 'frontbase:build_databases' -task :rebuild_frontbase_databases => 'frontbase:rebuild_databases' - spec = eval(File.read('activerecord.gemspec')) - Gem::PackageTask.new(spec) do |p| p.gem_spec = spec end diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index ac1479ad8f..727ee5f65f 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -419,6 +419,10 @@ module ActiveRecord # has_many :birthday_events, ->(user) { where starts_on: user.birthday }, class_name: 'Event' # end # + # Note: Joining, eager loading and preloading of these associations is not fully possible. + # These operations happen before instance creation and the scope will be called with a +nil+ argument. + # This can lead to unexpected behavior and is deprecated. + # # == Association callbacks # # Similar to the normal callbacks that hook into the life cycle of an Active Record object, @@ -1576,6 +1580,11 @@ module ActiveRecord join_model = builder.through_model + # FIXME: we should move this to the internal constants. Also people + # should never directly access this constant so I'm not happy about + # setting it. + const_set join_model.name, join_model + middle_reflection = builder.middle_reflection join_model Builder::HasMany.define_callbacks self, middle_reflection diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index e472277374..a297439214 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -23,7 +23,13 @@ module ActiveRecord::Associations::Builder KnownTable.new options[:join_table].to_s else class_name = options.fetch(:class_name) { - name.to_s.camelize.singularize + model_name = name.to_s.camelize.singularize + + if parent_name = lhs_class.parent_name + model_name = model_name.prepend("#{parent_name}::") + end + + model_name } KnownClass.new lhs_class, class_name end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 48628230c7..caf4e612f9 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -194,7 +194,7 @@ module ActiveRecord options[:dependent] end - delete_records(:all, dependent).tap do + delete_or_nullify_all_records(dependent).tap do reset loaded! end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index aac85a36c8..f5e911c739 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -105,23 +105,27 @@ module ActiveRecord } end + def delete_count(method, scope) + if method == :delete_all + scope.delete_all + else + scope.update_all(reflection.foreign_key => nil) + end + end + + def delete_or_nullify_all_records(method) + count = delete_count(method, self.scope) + update_counter(-count) + end + # Deletes the records according to the <tt>:dependent</tt> option. def delete_records(records, method) if method == :destroy records.each(&:destroy!) update_counter(-records.length) unless inverse_updates_counter_cache? else - if records == :all || !reflection.klass.primary_key - scope = self.scope - else - scope = self.scope.where(reflection.klass.primary_key => records) - end - - if method == :delete_all - update_counter(-scope.delete_all) - else - update_counter(-scope.update_all(reflection.foreign_key => nil)) - end + scope = self.scope.where(reflection.klass.primary_key => records) + update_counter(-delete_count(method, scope)) end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 0b122d2070..35ad512537 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -23,7 +23,7 @@ module ActiveRecord elsif loaded? target.size else - count + super end end @@ -130,13 +130,13 @@ module ActiveRecord end end + def delete_or_nullify_all_records(method) + delete_records(load_target, method) + end + def delete_records(records, method) ensure_not_nested - # This is unoptimised; it will load all the target records - # even when we just want to delete everything. - records = load_target if records == :all - scope = through_association.scope scope.where! construct_join_attributes(*records) diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index b7dc037a65..5842be3a7b 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -215,6 +215,7 @@ module ActiveRecord associations.map do |name, right| reflection = find_reflection base_klass, name reflection.check_validity! + reflection.check_eager_loadable! if reflection.options[:polymorphic] raise EagerLoadPolymorphicError.new(reflection) diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 311684d886..20bd4947dc 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -175,6 +175,7 @@ module ActiveRecord if owners.first.association(reflection.name).loaded? return AlreadyLoaded end + reflection.check_preloadable! case reflection.macro when :has_many diff --git a/activerecord/lib/active_record/attribute_assignment.rb b/activerecord/lib/active_record/attribute_assignment.rb index 30fa2c8ba5..87ecbe54f1 100644 --- a/activerecord/lib/active_record/attribute_assignment.rb +++ b/activerecord/lib/active_record/attribute_assignment.rb @@ -11,6 +11,15 @@ module ActiveRecord # If the passed hash responds to <tt>permitted?</tt> method and the return value # of this method is +false+ an <tt>ActiveModel::ForbiddenAttributesError</tt> # exception is raised. + # + # cat = Cat.new(name: "Gorby", status: "yawning") + # cat.attributes # => { "name" => "Gorby", "status" => "yawning" } + # cat.assign_attributes(status: "sleeping") + # cat.attributes # => { "name" => "Gorby", "status" => "sleeping" } + # + # New attributes will be persisted in the database when the object is saved. + # + # Aliased to <tt>attributes=</tt>. def assign_attributes(new_attributes) if !new_attributes.respond_to?(:stringify_keys) raise ArgumentError, "When assigning attributes, you must pass a hash as an argument." diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 4b1733619a..8bd51dc71f 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -326,13 +326,13 @@ module ActiveRecord # class Task < ActiveRecord::Base # end # - # person = Task.new(title: '', is_done: false) - # person.attribute_present?(:title) # => false - # person.attribute_present?(:is_done) # => true - # person.name = 'Francesco' - # person.is_done = true - # person.attribute_present?(:title) # => true - # person.attribute_present?(:is_done) # => true + # task = Task.new(title: '', is_done: false) + # task.attribute_present?(:title) # => false + # task.attribute_present?(:is_done) # => true + # task.title = 'Buy milk' + # task.is_done = true + # task.attribute_present?(:title) # => true + # task.attribute_present?(:is_done) # => true def attribute_present?(attribute) value = read_attribute(attribute) !value.nil? && !(value.respond_to?(:empty?) && value.empty?) @@ -359,6 +359,8 @@ module ActiveRecord # "2004-12-12" in a date column is cast to a date object, like Date.new(2004, 12, 12)). It raises # <tt>ActiveModel::MissingAttributeError</tt> if the identified attribute is missing. # + # Note: +:id+ is always present. + # # Alias for the <tt>read_attribute</tt> method. # # class Person < ActiveRecord::Base diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index 4184fad81c..35045b5258 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -129,6 +129,8 @@ module ActiveRecord when /^mediumint/i; 3 when /^smallint/i; 2 when /^tinyint/i; 1 + when /^float/i; 24 + when /^double/i; 53 else super end diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index 187eefb9e4..38efebeaf3 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -72,6 +72,8 @@ module ActiveRecord end # Casts a Ruby value to something appropriate for writing to the database. + # Numeric columns will typecast boolean and string to appropriate numeric + # values. def type_cast_for_write(value) return value unless number? diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/array_parser.rb b/activerecord/lib/active_record/connection_adapters/postgresql/array_parser.rb index 0b218f2bfd..743bf68fe6 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/array_parser.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/array_parser.rb @@ -1,6 +1,6 @@ module ActiveRecord module ConnectionAdapters - class PostgreSQLColumn < Column + module PostgreSQL module ArrayParser DOUBLE_QUOTE = '"' @@ -9,35 +9,23 @@ module ActiveRecord BRACKET_OPEN = '{' BRACKET_CLOSE = '}' - private - # Loads pg_array_parser if available. String parsing can be - # performed quicker by a native extension, which will not create - # a large amount of Ruby objects that will need to be garbage - # collected. pg_array_parser has a C and Java extension - begin - require 'pg_array_parser' - include PgArrayParser - rescue LoadError - def parse_pg_array(string) - parse_data(string) + def parse_pg_array(string) # :nodoc: + local_index = 0 + array = [] + while(local_index < string.length) + case string[local_index] + when BRACKET_OPEN + local_index,array = parse_array_contents(array, string, local_index + 1) + when BRACKET_CLOSE + return array end + local_index += 1 end - def parse_data(string) - local_index = 0 - array = [] - while(local_index < string.length) - case string[local_index] - when BRACKET_OPEN - local_index,array = parse_array_contents(array, string, local_index + 1) - when BRACKET_CLOSE - return array - end - local_index += 1 - end + array + end - array - end + private def parse_array_contents(array, string, index) is_escaping = false diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb index 551a9289c3..b612602216 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb @@ -1,19 +1,19 @@ module ActiveRecord module ConnectionAdapters - class PostgreSQLColumn < Column + module PostgreSQL module Cast - def point_to_string(point) + def point_to_string(point) # :nodoc: "(#{point[0]},#{point[1]})" end - def string_to_point(string) + def string_to_point(string) # :nodoc: if string[0] == '(' && string[-1] == ')' string = string[1...-1] end string.split(',').map{ |v| Float(v) } end - def string_to_time(string) + def string_to_time(string) # :nodoc: return string unless String === string case string @@ -26,7 +26,7 @@ module ActiveRecord end end - def string_to_bit(value) + def string_to_bit(value) # :nodoc: case value when /^0x/i value[2..-1].hex.to_s(2) # Hexadecimal notation @@ -35,7 +35,7 @@ module ActiveRecord end end - def hstore_to_string(object, array_member = false) + def hstore_to_string(object, array_member = false) # :nodoc: if Hash === object string = object.map { |k, v| "#{escape_hstore(k)}=>#{escape_hstore(v)}" }.join(',') string = escape_hstore(string) if array_member @@ -45,7 +45,7 @@ module ActiveRecord end end - def string_to_hstore(string) + def string_to_hstore(string) # :nodoc: if string.nil? nil elsif String === string @@ -59,7 +59,7 @@ module ActiveRecord end end - def json_to_string(object) + def json_to_string(object) # :nodoc: if Hash === object || Array === object ActiveSupport::JSON.encode(object) else @@ -67,7 +67,7 @@ module ActiveRecord end end - def array_to_string(value, column, adapter) + def array_to_string(value, column, adapter) # :nodoc: casted_values = value.map do |val| if String === val if val == "NULL" @@ -82,13 +82,13 @@ module ActiveRecord "{#{casted_values.join(',')}}" end - def range_to_string(object) + def range_to_string(object) # :nodoc: from = object.begin.respond_to?(:infinite?) && object.begin.infinite? ? '' : object.begin to = object.end.respond_to?(:infinite?) && object.end.infinite? ? '' : object.end "[#{from},#{to}#{object.exclude_end? ? ')' : ']'}" end - def string_to_json(string) + def string_to_json(string) # :nodoc: if String === string ActiveSupport::JSON.decode(string) else @@ -96,7 +96,7 @@ module ActiveRecord end end - def string_to_cidr(string) + def string_to_cidr(string) # :nodoc: if string.nil? nil elsif String === string @@ -110,7 +110,7 @@ module ActiveRecord end end - def cidr_to_string(object) + def cidr_to_string(object) # :nodoc: if IPAddr === object "#{object.to_s}/#{object.instance_variable_get(:@mask_addr).to_s(2).count('1')}" else @@ -118,7 +118,7 @@ module ActiveRecord end end - def string_to_array(string, oid) + def string_to_array(string, oid) # :nodoc: parse_pg_array(string).map {|val| type_cast_array(oid, val)} end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb index 2cbcd5fd50..97a93ce87a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb @@ -1,3 +1,5 @@ +require 'active_record/connection_adapters/postgresql/cast' + module ActiveRecord module ConnectionAdapters # PostgreSQL-specific extensions to column definitions in a table. @@ -29,8 +31,20 @@ module ActiveRecord # :stopdoc: class << self - include ConnectionAdapters::PostgreSQLColumn::Cast - include ConnectionAdapters::PostgreSQLColumn::ArrayParser + include PostgreSQL::Cast + + # Loads pg_array_parser if available. String parsing can be + # performed quicker by a native extension, which will not create + # a large amount of Ruby objects that will need to be garbage + # collected. pg_array_parser has a C and Java extension + begin + require 'pg_array_parser' + include PgArrayParser + rescue LoadError + require 'active_record/connection_adapters/postgresql/array_parser' + include PostgreSQL::ArrayParser + end + attr_accessor :money_precision end # :startdoc: @@ -98,6 +112,9 @@ module ActiveRecord end end + # Casts a Ruby value to something appropriate for writing to PostgreSQL. + # see ActiveRecord::ConnectionAdapters::Class#type_cast_for_write + # see ActiveRecord::ConnectionAdapters::PostgreSQLAdapter::OID::Type def type_cast_for_write(value) if @oid_type.respond_to?(:type_cast_for_write) @oid_type.type_cast_for_write(value) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb index 168b08ba75..89a7257d77 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/database_statements.rb @@ -1,6 +1,6 @@ module ActiveRecord module ConnectionAdapters - class PostgreSQLAdapter < AbstractAdapter + module PostgreSQL module DatabaseStatements def explain(arel, binds = []) sql = "EXPLAIN #{to_sql(arel, binds)}" @@ -94,6 +94,11 @@ module ActiveRecord super.insert end + # The internal PostgreSQL identifier of the money data type. + MONEY_COLUMN_TYPE_OID = 790 #:nodoc: + # The internal PostgreSQL identifier of the BYTEA data type. + BYTEA_COLUMN_TYPE_OID = 17 #:nodoc: + # create a 2D array representing the result set def result_as_array(res) #:nodoc: # check if we have any binary column and if they need escaping diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb index 540b3694b5..cf6a375704 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/oid.rb @@ -1,8 +1,6 @@ -require 'active_record/connection_adapters/abstract_adapter' - module ActiveRecord module ConnectionAdapters - class PostgreSQLAdapter < AbstractAdapter + module PostgreSQL module OID class Type def type; end @@ -249,9 +247,14 @@ This is not reliable and will be removed in the future. def type; :float end def type_cast(value) - return if value.nil? - - value.to_f + case value + when nil; nil + when 'Infinity'; ::Float::INFINITY + when '-Infinity'; -::Float::INFINITY + when 'NaN'; ::Float::NAN + else + value.to_f + end end end @@ -369,6 +372,77 @@ This is not reliable and will be removed in the future. end end + # This class uses the data from PostgreSQL pg_type table to build + # the OID -> Type mapping. + # - OID is and integer representing the type. + # - Type is an OID::Type object. + # This class has side effects on the +store+ passed during initialization. + class TypeMapInitializer # :nodoc: + def initialize(store) + @store = store + end + + def run(records) + mapped, nodes = records.partition { |row| OID.registered_type? row['typname'] } + ranges, nodes = nodes.partition { |row| row['typtype'] == 'r' } + enums, nodes = nodes.partition { |row| row['typtype'] == 'e' } + domains, nodes = nodes.partition { |row| row['typtype'] == 'd' } + arrays, nodes = nodes.partition { |row| row['typinput'] == 'array_in' } + composites, nodes = nodes.partition { |row| row['typelem'] != '0' } + + mapped.each { |row| register_mapped_type(row) } + enums.each { |row| register_enum_type(row) } + domains.each { |row| register_domain_type(row) } + arrays.each { |row| register_array_type(row) } + ranges.each { |row| register_range_type(row) } + composites.each { |row| register_composite_type(row) } + end + + private + def register_mapped_type(row) + register row['oid'], OID::NAMES[row['typname']] + end + + def register_enum_type(row) + register row['oid'], OID::Enum.new + end + + def register_array_type(row) + if subtype = @store[row['typelem'].to_i] + register row['oid'], OID::Array.new(subtype) + end + end + + def register_range_type(row) + if subtype = @store[row['rngsubtype'].to_i] + register row['oid'], OID::Range.new(subtype) + end + end + + def register_domain_type(row) + if base_type = @store[row["typbasetype"].to_i] + register row['oid'], base_type + else + warn "unknown base type (OID: #{row["typbasetype"]}) for domain #{row["typname"]}." + end + end + + def register_composite_type(row) + if subtype = @store[row['typelem'].to_i] + register row['oid'], OID::Vector.new(row['typdelim'], subtype) + end + end + + def register(oid, oid_type) + oid = oid.to_i + + raise ArgumentError, "can't register nil type for OID #{oid}" if oid_type.nil? + return if @store.key?(oid) + + @store[oid] = oid_type + end + end + # When the PG adapter connects, the pg_type table is queried. The # key of this hash maps to the `typname` column from the table. # type_map is then dynamically built with oids as the key and type @@ -404,6 +478,7 @@ This is not reliable and will be removed in the future. register_type 'text', OID::Text.new register_type 'varchar', OID::String.new alias_type 'char', 'varchar' + alias_type 'name', 'varchar' alias_type 'bpchar', 'varchar' register_type 'bool', OID::Boolean.new register_type 'bit', OID::Bit.new diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index 403e37fde9..0883b02a35 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -1,6 +1,6 @@ module ActiveRecord module ConnectionAdapters - class PostgreSQLAdapter < AbstractAdapter + module PostgreSQL module Quoting # Escapes binary strings for bytea input to the database. def escape_bytea(value) @@ -187,8 +187,8 @@ module ActiveRecord def quote_default_value(value, column) #:nodoc: if column.type == :uuid && value =~ /\(\)/ value - else - quote(value) + else + quote(value, column) end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb b/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb index bc775394a6..98dcf441ff 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/referential_integrity.rb @@ -1,6 +1,6 @@ module ActiveRecord module ConnectionAdapters - class PostgreSQLAdapter < AbstractAdapter + module PostgreSQL module ReferentialIntegrity def supports_disable_referential_integrity? #:nodoc: true diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index e7169bd357..dd983562fb 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -1,6 +1,6 @@ module ActiveRecord module ConnectionAdapters - class PostgreSQLAdapter < AbstractAdapter + module PostgreSQL class SchemaCreation < AbstractAdapter::SchemaCreation private @@ -12,7 +12,7 @@ module ActiveRecord def visit_ColumnDefinition(o) sql = super - if o.primary_key? && o.type == :uuid + if o.primary_key? && o.type != :primary_key sql << " PRIMARY KEY " add_column_options!(sql, column_options(o)) end @@ -33,10 +33,6 @@ module ActiveRecord end end - def schema_creation - SchemaCreation.new self - end - module SchemaStatements # Drops the database specified on the +name+ attribute # and creates it again using the provided +options+. @@ -101,7 +97,7 @@ module ActiveRecord # If the schema is not specified as part of +name+ then it will only find tables within # the current schema search path (regardless of permissions to access tables in other schemas) def table_exists?(name) - schema, table = Utils.extract_schema_and_table(name.to_s) + schema, table = extract_schema_and_table(name.to_s) return false unless table exec_query(<<-SQL, 'SCHEMA').rows.first[0].to_i > 0 @@ -405,6 +401,7 @@ module ActiveRecord def change_column_default(table_name, column_name, default) clear_cache! column = column_for(table_name, column_name) + execute "ALTER TABLE #{quote_table_name(table_name)} ALTER COLUMN #{quote_column_name(column_name)} SET DEFAULT #{quote_default_value(default, column)}" if column end @@ -491,6 +488,23 @@ module ActiveRecord [super, *order_columns].join(', ') end + + private + + # Returns an array of <tt>[schema_name, table_name]</tt> extracted from +name+. + # +schema_name+ is nil if not specified in +name+. + # +schema_name+ and +table_name+ exclude surrounding quotes (regardless of whether provided in +name+) + # +name+ supports the range of schema/table references understood by PostgreSQL, for example: + # + # * <tt>table_name</tt> + # * <tt>"table.name"</tt> + # * <tt>schema_name.table_name</tt> + # * <tt>schema_name."table.name"</tt> + # * <tt>"schema.name"."table name"</tt> + def extract_schema_and_table(name) + table, schema = name.scan(/[^".\s]+|"[^"]*"/)[0..1].collect{|m| m.gsub(/(^"|"$)/,'') }.reverse + [schema, table] + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 764cb576d9..23b91be0f3 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -1,13 +1,13 @@ require 'active_record/connection_adapters/abstract_adapter' require 'active_record/connection_adapters/statement_pool' + +require 'active_record/connection_adapters/postgresql/column' require 'active_record/connection_adapters/postgresql/oid' -require 'active_record/connection_adapters/postgresql/cast' -require 'active_record/connection_adapters/postgresql/array_parser' require 'active_record/connection_adapters/postgresql/quoting' +require 'active_record/connection_adapters/postgresql/referential_integrity' require 'active_record/connection_adapters/postgresql/schema_statements' require 'active_record/connection_adapters/postgresql/database_statements' -require 'active_record/connection_adapters/postgresql/referential_integrity' -require 'active_record/connection_adapters/postgresql/column' + require 'arel/visitors/bind_visitor' # Make sure we're using pg high enough for PGResult#values @@ -238,10 +238,12 @@ module ActiveRecord citext: { name: "citext" } } - include Quoting - include ReferentialIntegrity - include SchemaStatements - include DatabaseStatements + OID = PostgreSQL::OID #:nodoc: + + include PostgreSQL::Quoting + include PostgreSQL::ReferentialIntegrity + include PostgreSQL::SchemaStatements + include PostgreSQL::DatabaseStatements include Savepoints # Returns 'PostgreSQL' as adapter name for identification purposes. @@ -249,6 +251,10 @@ module ActiveRecord ADAPTER_NAME end + def schema_creation + PostgreSQL::SchemaCreation.new self + end + # Adds `:array` option to the default set provided by the # AbstractAdapter def prepare_column_options(column, types) @@ -494,25 +500,6 @@ module ActiveRecord exec_query "SET SESSION AUTHORIZATION #{user}" end - module Utils - extend self - - # Returns an array of <tt>[schema_name, table_name]</tt> extracted from +name+. - # +schema_name+ is nil if not specified in +name+. - # +schema_name+ and +table_name+ exclude surrounding quotes (regardless of whether provided in +name+) - # +name+ supports the range of schema/table references understood by PostgreSQL, for example: - # - # * <tt>table_name</tt> - # * <tt>"table.name"</tt> - # * <tt>schema_name.table_name</tt> - # * <tt>schema_name."table.name"</tt> - # * <tt>"schema.name"."table name"</tt> - def extract_schema_and_table(name) - table, schema = name.scan(/[^".\s]+|"[^"]*"/)[0..1].collect{|m| m.gsub(/(^"|"$)/,'') }.reverse - [schema, table] - end - end - def use_insert_returning? @use_insert_returning end @@ -571,25 +558,6 @@ module ActiveRecord initialize_type_map(type_map) end - def add_oid(row, records_by_oid, type_map) - return type_map if type_map.key? row['type_elem'].to_i - - if OID.registered_type? row['typname'] - # this composite type is explicitly registered - vector = OID::NAMES[row['typname']] - else - # use the default for composite types - unless type_map.key? row['typelem'].to_i - add_oid records_by_oid[row['typelem']], records_by_oid, type_map - end - - vector = OID::Vector.new row['typdelim'], type_map[row['typelem'].to_i] - end - - type_map[row['oid'].to_i] = vector - type_map - end - def initialize_type_map(type_map, oids = nil) if supports_ranges? query = <<-SQL @@ -608,52 +576,9 @@ module ActiveRecord query += "WHERE t.oid::integer IN (%s)" % oids.join(", ") end - result = execute(query, 'SCHEMA') - ranges, nodes = result.partition { |row| row['typtype'] == 'r' } - enums, nodes = nodes.partition { |row| row['typtype'] == 'e' } - domains, nodes = nodes.partition { |row| row['typtype'] == 'd' } - arrays, nodes = nodes.partition { |row| row['typinput'] == 'array_in' } - leaves, nodes = nodes.partition { |row| row['typelem'] == '0' } - - # populate the enum types - enums.each do |row| - type_map[row['oid'].to_i] = OID::Enum.new - end - - # populate the base types - leaves.find_all { |row| OID.registered_type? row['typname'] }.each do |row| - type_map[row['oid'].to_i] = OID::NAMES[row['typname']] - end - - records_by_oid = result.group_by { |row| row['oid'] } - - # populate composite types - nodes.each do |row| - add_oid row, records_by_oid, type_map - end - - # populate array types - arrays.find_all { |row| type_map.key? row['typelem'].to_i }.each do |row| - array = OID::Array.new type_map[row['typelem'].to_i] - type_map[row['oid'].to_i] = array - end - - # populate range types - ranges.find_all { |row| type_map.key? row['rngsubtype'].to_i }.each do |row| - subtype = type_map[row['rngsubtype'].to_i] - range = OID::Range.new subtype - type_map[row['oid'].to_i] = range - end - - # populate domain types - domains.each do |row| - base_type_oid = row["typbasetype"].to_i - if base_type = type_map[base_type_oid] - type_map[row['oid'].to_i] = base_type - else - warn "unknown base type (OID: #{base_type_oid}) for domain #{row["typname"]}." - end - end + initializer = OID::TypeMapInitializer.new(type_map) + records = execute(query, 'SCHEMA') + initializer.run(records) end FEATURE_NOT_SUPPORTED = "0A000" #:nodoc: @@ -725,11 +650,6 @@ module ActiveRecord @statements[sql_key] end - # The internal PostgreSQL identifier of the money data type. - MONEY_COLUMN_TYPE_OID = 790 #:nodoc: - # The internal PostgreSQL identifier of the BYTEA data type. - BYTEA_COLUMN_TYPE_OID = 17 #:nodoc: - # Connects to a PostgreSQL server and sets up the adapter depending on the # connected server's characteristics. def connect diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 18f1ca26de..c7ec093824 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -27,8 +27,10 @@ module ActiveRecord # conversation.status # => nil # # Scopes based on the allowed values of the enum field will be provided - # as well. With the above example, it will create an +active+ and +archived+ - # scope. + # as well. With the above example: + # + # Conversation.active + # Conversation.archived # # You can set the default value from the database declaration, like: # diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 7f4d77849a..0eec6774a0 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -278,6 +278,20 @@ module ActiveRecord end end + def check_preloadable! + return unless scope + + if scope.arity > 0 + ActiveSupport::Deprecation.warn <<-WARNING +The association scope '#{name}' is instance dependent (the scope block takes an argument). +Preloading happens before the individual instances are created. This means that there is no instance +being passed to the association scope. This will most likely result in broken or incorrect behavior. +Joining, Preloading and eager loading of these associations is deprecated and will be removed in the future. + WARNING + end + end + alias :check_eager_loadable! :check_preloadable! + def join_id_for(owner) #:nodoc: key = (source_macro == :belongs_to) ? foreign_key : active_record_primary_key owner[key] diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 42c9881b48..56cf9bcd27 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -19,6 +19,14 @@ module ActiveRecord # # Person.group(:city).count # # => { 'Rome' => 5, 'Paris' => 3 } + # + # If +count+ is used with +select+, it will count the selected columns: + # + # Person.select(:age).count + # # => counts the number of different age values + # + # Note: not all valid +select+ expressions are valid +count+ expressions. The specifics differ + # between databases. In invalid cases, an error from the databsae is thrown. def count(column_name = nil, options = {}) # TODO: Remove options argument as soon we remove support to # activerecord-deprecated_finders. diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index fcb28a18f6..ac41d0aa80 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -156,7 +156,7 @@ module ActiveRecord def filter_binds(lhs_binds, removed_wheres) return lhs_binds if removed_wheres.empty? - set = Set.new removed_wheres.map { |x| x.left.name } + set = Set.new removed_wheres.map { |x| x.left.name.to_s } lhs_binds.dup.delete_if { |col,_| set.include? col.name } end diff --git a/activerecord/lib/active_record/store.rb b/activerecord/lib/active_record/store.rb index 79a6ccbda0..7014bc6d45 100644 --- a/activerecord/lib/active_record/store.rb +++ b/activerecord/lib/active_record/store.rb @@ -66,8 +66,9 @@ module ActiveRecord extend ActiveSupport::Concern included do - class_attribute :stored_attributes, instance_accessor: false - self.stored_attributes = {} + class << self + attr_accessor :local_stored_attributes + end end module ClassMethods @@ -93,9 +94,9 @@ module ActiveRecord # assign new store attribute and create new hash to ensure that each class in the hierarchy # has its own hash of stored attributes. - self.stored_attributes = {} if self.stored_attributes.blank? - self.stored_attributes[store_attribute] ||= [] - self.stored_attributes[store_attribute] |= keys + self.local_stored_attributes ||= {} + self.local_stored_attributes[store_attribute] ||= [] + self.local_stored_attributes[store_attribute] |= keys end def _store_accessors_module @@ -105,6 +106,14 @@ module ActiveRecord mod end end + + def stored_attributes + parent = superclass.respond_to?(:stored_attributes) ? superclass.stored_attributes : {} + if self.local_stored_attributes + parent.merge!(self.local_stored_attributes) { |k, a, b| a | b } + end + parent + end end protected diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index e31d28cfd1..17f76b63b3 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -350,7 +350,6 @@ module ActiveRecord end @_start_transaction_state[:level] = (@_start_transaction_state[:level] || 0) + 1 @_start_transaction_state[:frozen?] = @attributes.frozen? - @_start_transaction_state[:changed_attributes] ||= changed_attributes.dup end # Clear the new record state and id of a record. @@ -369,9 +368,6 @@ module ActiveRecord @attributes = @attributes.dup if @attributes.frozen? @new_record = restore_state[:new_record] @destroyed = restore_state[:destroyed] - changed_attributes.replace(restore_state[:changed_attributes]).delete_if do |attribute, old_value| - old_value == @attributes[attribute] - end if restore_state.has_key?(:id) write_attribute(self.class.primary_key, restore_state[:id]) else diff --git a/activerecord/test/cases/adapters/mysql/schema_test.rb b/activerecord/test/cases/adapters/mysql/schema_test.rb index 807a7a155e..87c5277e64 100644 --- a/activerecord/test/cases/adapters/mysql/schema_test.rb +++ b/activerecord/test/cases/adapters/mysql/schema_test.rb @@ -17,6 +17,44 @@ module ActiveRecord self.table_name = "#{db}.#{table}" def self.name; 'Post'; end end + + @connection.create_table "mysql_doubles" + end + + teardown do + @connection.execute "drop table if exists mysql_doubles" + end + + class MysqlDouble < ActiveRecord::Base + self.table_name = "mysql_doubles" + end + + def test_float_limits + @connection.add_column :mysql_doubles, :float_no_limit, :float + @connection.add_column :mysql_doubles, :float_short, :float, limit: 5 + @connection.add_column :mysql_doubles, :float_long, :float, limit: 53 + + @connection.add_column :mysql_doubles, :float_23, :float, limit: 23 + @connection.add_column :mysql_doubles, :float_24, :float, limit: 24 + @connection.add_column :mysql_doubles, :float_25, :float, limit: 25 + MysqlDouble.reset_column_information + + column_no_limit = MysqlDouble.columns.find { |c| c.name == 'float_no_limit' } + column_short = MysqlDouble.columns.find { |c| c.name == 'float_short' } + column_long = MysqlDouble.columns.find { |c| c.name == 'float_long' } + + column_23 = MysqlDouble.columns.find { |c| c.name == 'float_23' } + column_24 = MysqlDouble.columns.find { |c| c.name == 'float_24' } + column_25 = MysqlDouble.columns.find { |c| c.name == 'float_25' } + + # Mysql floats are precision 0..24, Mysql doubles are precision 25..53 + assert_equal 24, column_no_limit.limit + assert_equal 24, column_short.limit + assert_equal 53, column_long.limit + + assert_equal 24, column_23.limit + assert_equal 24, column_24.limit + assert_equal 53, column_25.limit end def test_schema diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 18dd4a6de8..c20030ca64 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -16,7 +16,7 @@ class PostgresqlArrayTest < ActiveRecord::TestCase t.integer 'ratings', array: true end end - @column = PgArray.columns.find { |c| c.name == 'tags' } + @column = PgArray.columns_hash['tags'] end teardown do @@ -61,10 +61,10 @@ class PostgresqlArrayTest < ActiveRecord::TestCase def test_change_column_with_array @connection.add_column :pg_arrays, :snippets, :string, array: true, default: [] - @connection.change_column :pg_arrays, :snippets, :text, array: true, default: "{}" + @connection.change_column :pg_arrays, :snippets, :text, array: true, default: [] PgArray.reset_column_information - column = PgArray.columns.find { |c| c.name == 'snippets' } + column = PgArray.columns_hash['snippets'] assert_equal :text, column.type assert_equal [], column.default @@ -80,6 +80,14 @@ class PostgresqlArrayTest < ActiveRecord::TestCase end end + def test_change_column_default_with_array + @connection.change_column_default :pg_arrays, :tags, [] + + PgArray.reset_column_information + column = PgArray.columns_hash['tags'] + assert_equal [], column.default + end + def test_type_cast_array data = '{1,2,3}' oid_type = @column.instance_variable_get('@oid_type').subtype @@ -101,17 +109,32 @@ class PostgresqlArrayTest < ActiveRecord::TestCase assert_equal(['1', '2'], x.ratings) end - def test_rewrite + def test_select_with_strings @connection.execute "insert into pg_arrays (tags) VALUES ('{1,2,3}')" x = PgArray.first - x.tags = ['1','2','3','4'] - assert x.save! + assert_equal(['1','2','3'], x.tags) end - def test_select + def test_rewrite_with_strings @connection.execute "insert into pg_arrays (tags) VALUES ('{1,2,3}')" x = PgArray.first - assert_equal(['1','2','3'], x.tags) + x.tags = ['1','2','3','4'] + x.save! + assert_equal ['1','2','3','4'], x.reload.tags + end + + def test_select_with_integers + @connection.execute "insert into pg_arrays (ratings) VALUES ('{1,2,3}')" + x = PgArray.first + assert_equal([1, 2, 3], x.ratings) + end + + def test_rewrite_with_integers + @connection.execute "insert into pg_arrays (ratings) VALUES ('{1,2,3}')" + x = PgArray.first + x.ratings = [2, '3', 4] + x.save! + assert_equal [2, 3, 4], x.reload.ratings end def test_multi_dimensional_with_strings @@ -175,6 +198,14 @@ class PostgresqlArrayTest < ActiveRecord::TestCase assert_equal [], pg_array.reload.tags end + def test_escaping + unknown = 'foo\\",bar,baz,\\' + tags = ["hello_#{unknown}"] + ar = PgArray.create!(tags: tags) + ar.reload + assert_equal tags, ar.tags + end + private def assert_cycle field, array # test creation diff --git a/activerecord/test/cases/adapters/postgresql/bytea_test.rb b/activerecord/test/cases/adapters/postgresql/bytea_test.rb index e3478856c8..fadadfa57c 100644 --- a/activerecord/test/cases/adapters/postgresql/bytea_test.rb +++ b/activerecord/test/cases/adapters/postgresql/bytea_test.rb @@ -19,7 +19,7 @@ class PostgresqlByteaTest < ActiveRecord::TestCase end end end - @column = ByteaDataType.columns.find { |c| c.name == 'payload' } + @column = ByteaDataType.columns_hash['payload'] assert(@column.is_a?(ActiveRecord::ConnectionAdapters::PostgreSQLColumn)) end diff --git a/activerecord/test/cases/adapters/postgresql/citext_test.rb b/activerecord/test/cases/adapters/postgresql/citext_test.rb index 948bf49a54..8493050726 100644 --- a/activerecord/test/cases/adapters/postgresql/citext_test.rb +++ b/activerecord/test/cases/adapters/postgresql/citext_test.rb @@ -50,7 +50,7 @@ if ActiveRecord::Base.connection.supports_extensions? t.citext 'username' end Citext.reset_column_information - column = Citext.columns.find { |c| c.name == 'username' } + column = Citext.columns_hash['username'] assert_equal :citext, column.type raise ActiveRecord::Rollback # reset the schema change diff --git a/activerecord/test/cases/adapters/postgresql/composite_test.rb b/activerecord/test/cases/adapters/postgresql/composite_test.rb index 224b1b770b..68b9e6daf7 100644 --- a/activerecord/test/cases/adapters/postgresql/composite_test.rb +++ b/activerecord/test/cases/adapters/postgresql/composite_test.rb @@ -1,19 +1,19 @@ # -*- coding: utf-8 -*- require "cases/helper" +require 'support/connection_helper' require 'active_record/base' require 'active_record/connection_adapters/postgresql_adapter' -class PostgresqlCompositeTest < ActiveRecord::TestCase +module PostgresqlCompositeBehavior + include ConnectionHelper + class PostgresqlComposite < ActiveRecord::Base self.table_name = "postgresql_composites" end - teardown do - @connection.execute 'DROP TABLE IF EXISTS postgresql_composites' - @connection.execute 'DROP TYPE IF EXISTS full_address' - end - def setup + super + @connection = ActiveRecord::Base.connection @connection.transaction do @connection.execute <<-SQL @@ -29,9 +29,27 @@ class PostgresqlCompositeTest < ActiveRecord::TestCase end end + def teardown + super + + @connection.execute 'DROP TABLE IF EXISTS postgresql_composites' + @connection.execute 'DROP TYPE IF EXISTS full_address' + reset_connection + PostgresqlComposite.reset_column_information + end +end + +# Composites are mapped to `OID::Identity` by default. The user is informed by a warning like: +# "unknown OID 5653508: failed to recognize type of 'address'. It will be treated as String." +# To take full advantage of composite types, we suggest you register your own +OID::Type+. +# See PostgresqlCompositeWithCustomOIDTest +class PostgresqlCompositeTest < ActiveRecord::TestCase + include PostgresqlCompositeBehavior + def test_column + ensure_warning_is_issued + column = PostgresqlComposite.columns_hash["address"] - # TODO: Composite columns should have a type assert_nil column.type assert_equal "full_address", column.sql_type assert_not column.number? @@ -41,6 +59,8 @@ class PostgresqlCompositeTest < ActiveRecord::TestCase end def test_composite_mapping + ensure_warning_is_issued + @connection.execute "INSERT INTO postgresql_composites VALUES (1, ROW('Paris', 'Champs-Élysées'));" composite = PostgresqlComposite.first assert_equal "(Paris,Champs-Élysées)", composite.address @@ -50,4 +70,71 @@ class PostgresqlCompositeTest < ActiveRecord::TestCase assert_equal '(Paris,"Rue Basse")', composite.reload.address end + + private + def ensure_warning_is_issued + warning = capture(:stderr) do + PostgresqlComposite.columns_hash + end + assert_match(/unknown OID \d+: failed to recognize type of 'address'\. It will be treated as String\./, warning) + end +end + +class PostgresqlCompositeWithCustomOIDTest < ActiveRecord::TestCase + include PostgresqlCompositeBehavior + + class FullAddressType + def type; :full_address end + def simplified_type(sql_type); type end + + def type_cast(value) + if value =~ /\("?([^",]*)"?,"?([^",]*)"?\)/ + FullAddress.new($1, $2) + end + end + + def type_cast_for_write(value) + "(#{value.city},#{value.street})" + end + end + + FullAddress = Struct.new(:city, :street) + + def setup + super + + @registration = ActiveRecord::ConnectionAdapters::PostgreSQLAdapter::OID + @registration.register_type "full_address", FullAddressType.new + end + + def teardown + super + + # there is currently no clean way to unregister a OID::Type + @registration::NAMES.delete("full_address") + end + + def test_column + column = PostgresqlComposite.columns_hash["address"] + assert_equal :full_address, column.type + assert_equal "full_address", column.sql_type + assert_not column.number? + assert_not column.text? + assert_not column.binary? + assert_not column.array + end + + def test_composite_mapping + @connection.execute "INSERT INTO postgresql_composites VALUES (1, ROW('Paris', 'Champs-Élysées'));" + composite = PostgresqlComposite.first + assert_equal "Paris", composite.address.city + assert_equal "Champs-Élysées", composite.address.street + + composite.address = FullAddress.new("Paris", "Rue Basse") + skip "Saving with custom OID type is currently not supported." + composite.save! + + assert_equal 'Paris', composite.reload.address.city + assert_equal 'Rue Basse', composite.reload.address.street + end end diff --git a/activerecord/test/cases/adapters/postgresql/datatype_test.rb b/activerecord/test/cases/adapters/postgresql/datatype_test.rb index e7dda1a1af..ea433d391f 100644 --- a/activerecord/test/cases/adapters/postgresql/datatype_test.rb +++ b/activerecord/test/cases/adapters/postgresql/datatype_test.rb @@ -1,4 +1,5 @@ require "cases/helper" +require 'support/ddl_helper' class PostgresqlArray < ActiveRecord::Base end @@ -37,9 +38,6 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase @connection = ActiveRecord::Base.connection @connection.execute("set lc_monetary = 'C'") - @connection.execute("INSERT INTO postgresql_arrays (id, commission_by_quarter, nicknames) VALUES (1, '{35000,21000,18000,17000}', '{foo,bar,baz}')") - @first_array = PostgresqlArray.find(1) - @connection.execute("INSERT INTO postgresql_tsvectors (id, text_vector) VALUES (1, ' ''text'' ''vector'' ')") @first_tsvector = PostgresqlTsvector.find(1) @@ -50,7 +48,11 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase @second_money = PostgresqlMoney.find(2) @connection.execute("INSERT INTO postgresql_numbers (id, single, double) VALUES (1, 123.456, 123456.789)") + @connection.execute("INSERT INTO postgresql_numbers (id, single, double) VALUES (2, '-Infinity', 'Infinity')") + @connection.execute("INSERT INTO postgresql_numbers (id, single, double) VALUES (3, 123.456, 'NaN')") @first_number = PostgresqlNumber.find(1) + @second_number = PostgresqlNumber.find(2) + @third_number = PostgresqlNumber.find(3) @connection.execute("INSERT INTO postgresql_times (id, time_interval, scaled_time_interval) VALUES (1, '1 year 2 days ago', '3 weeks ago')") @first_time = PostgresqlTime.find(1) @@ -68,23 +70,10 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase end teardown do - [PostgresqlArray, PostgresqlTsvector, PostgresqlMoney, PostgresqlNumber, PostgresqlTime, PostgresqlNetworkAddress, + [PostgresqlTsvector, PostgresqlMoney, PostgresqlNumber, PostgresqlTime, PostgresqlNetworkAddress, PostgresqlBitString, PostgresqlOid, PostgresqlTimestampWithZone].each(&:delete_all) end - def test_array_escaping - unknown = %(foo\\",bar,baz,\\) - nicknames = ["hello_#{unknown}"] - ar = PostgresqlArray.create!(nicknames: nicknames, id: 100) - ar.reload - assert_equal nicknames, ar.nicknames - end - - def test_data_type_of_array_types - assert_equal :integer, @first_array.column_for_attribute(:commission_by_quarter).type - assert_equal :text, @first_array.column_for_attribute(:nicknames).type - end - def test_data_type_of_tsvector_types assert_equal :tsvector, @first_tsvector.column_for_attribute(:text_vector).type end @@ -118,11 +107,6 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase assert_equal :integer, @first_oid.column_for_attribute(:obj_id).type end - def test_array_values - assert_equal [35000,21000,18000,17000], @first_array.commission_by_quarter - assert_equal ['foo','bar','baz'], @first_array.nicknames - end - def test_tsvector_values assert_equal "'text' 'vector'", @first_tsvector.text_vector end @@ -133,7 +117,7 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase end def test_money_type_cast - column = PostgresqlMoney.columns.find { |c| c.name == 'wealth' } + column = PostgresqlMoney.columns_hash['wealth'] assert_equal(12345678.12, column.type_cast("$12,345,678.12")) assert_equal(12345678.12, column.type_cast("$12.345.678,12")) assert_equal(-1.15, column.type_cast("-$1.15")) @@ -154,6 +138,9 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase def test_number_values assert_equal 123.456, @first_number.single assert_equal 123456.789, @first_number.double + assert_equal(-::Float::INFINITY, @second_number.single) + assert_equal ::Float::INFINITY, @second_number.double + assert_same ::Float::NAN, @third_number.double end def test_time_values @@ -179,30 +166,6 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase assert_equal 1234, @first_oid.obj_id end - def test_update_integer_array - new_value = [32800,95000,29350,17000] - @first_array.commission_by_quarter = new_value - assert @first_array.save - assert @first_array.reload - assert_equal new_value, @first_array.commission_by_quarter - @first_array.commission_by_quarter = new_value - assert @first_array.save - assert @first_array.reload - assert_equal new_value, @first_array.commission_by_quarter - end - - def test_update_text_array - new_value = ['robby','robert','rob','robbie'] - @first_array.nicknames = new_value - assert @first_array.save - assert @first_array.reload - assert_equal new_value, @first_array.nicknames - @first_array.nicknames = new_value - assert @first_array.save - assert @first_array.reload - assert_equal new_value, @first_array.nicknames - end - def test_update_money new_value = BigDecimal.new('123.45') @first_money.wealth = new_value @@ -308,3 +271,25 @@ class PostgresqlDataTypeTest < ActiveRecord::TestCase @connection.reconnect! end end + +class PostgresqlInternalDataTypeTest < ActiveRecord::TestCase + include DdlHelper + + setup do + @connection = ActiveRecord::Base.connection + end + + def test_name_column_type + with_example_table @connection, 'ex', 'data name' do + column = @connection.columns('ex').find { |col| col.name == 'data' } + assert_equal :string, column.type + end + end + + def test_char_column_type + with_example_table @connection, 'ex', 'data "char"' do + column = @connection.columns('ex').find { |col| col.name == 'data' } + assert_equal :string, column.type + end + end +end diff --git a/activerecord/test/cases/adapters/postgresql/hstore_test.rb b/activerecord/test/cases/adapters/postgresql/hstore_test.rb index c24c4b0d56..67a610b459 100644 --- a/activerecord/test/cases/adapters/postgresql/hstore_test.rb +++ b/activerecord/test/cases/adapters/postgresql/hstore_test.rb @@ -28,7 +28,7 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase t.hstore 'settings' end end - @column = Hstore.columns.find { |c| c.name == 'tags' } + @column = Hstore.columns_hash['tags'] end teardown do @@ -78,7 +78,7 @@ class PostgresqlHstoreTest < ActiveRecord::TestCase t.hstore 'users', default: '' end Hstore.reset_column_information - column = Hstore.columns.find { |c| c.name == 'users' } + column = Hstore.columns_hash['users'] assert_equal :hstore, column.type raise ActiveRecord::Rollback # reset the schema change diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index ee793ffff2..d25f8bf958 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -23,7 +23,7 @@ class PostgresqlJSONTest < ActiveRecord::TestCase rescue ActiveRecord::StatementInvalid skip "do not test on PG without json" end - @column = JsonDataType.columns.find { |c| c.name == 'payload' } + @column = JsonDataType.columns_hash['payload'] end teardown do @@ -57,7 +57,7 @@ class PostgresqlJSONTest < ActiveRecord::TestCase t.json 'users', default: '{}' end JsonDataType.reset_column_information - column = JsonDataType.columns.find { |c| c.name == 'users' } + column = JsonDataType.columns_hash['users'] assert_equal :json, column.type raise ActiveRecord::Rollback # reset the schema change diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index b7791078db..49f5ec250f 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -396,6 +396,23 @@ module ActiveRecord reset_connection end + def test_unparsed_defaults_are_at_least_set_when_saving + with_example_table "id SERIAL PRIMARY KEY, number INTEGER NOT NULL DEFAULT (4 + 4) * 2 / 4" do + number_klass = Class.new(ActiveRecord::Base) do + self.table_name = 'ex' + end + column = number_klass.columns_hash["number"] + assert_nil column.default + assert_nil column.default_function + + first_number = number_klass.new + assert_nil first_number.number + + first_number.save! + assert_equal 4, first_number.reload.number + end + end + private def insert(ctx, data) binds = data.map { |name, value| diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index 11ec7599a3..9d4d79c0c6 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -352,6 +352,21 @@ class SchemaTest < ActiveRecord::TestCase end end + def test_extract_schema_and_table + { + %(table_name) => [nil,'table_name'], + %("table.name") => [nil,'table.name'], + %(schema.table_name) => %w{schema table_name}, + %("schema".table_name) => %w{schema table_name}, + %(schema."table_name") => %w{schema table_name}, + %("schema"."table_name") => %w{schema table_name}, + %("even spaces".table) => ['even spaces','table'], + %(schema."table.name") => ['schema', 'table.name'] + }.each do |given, expect| + assert_equal expect, @connection.send(:extract_schema_and_table, given) + end + end + private def columns(table_name) @connection.send(:column_definitions, table_name).map do |name, type, default| diff --git a/activerecord/test/cases/adapters/postgresql/utils_test.rb b/activerecord/test/cases/adapters/postgresql/utils_test.rb deleted file mode 100644 index 9e7b08ef34..0000000000 --- a/activerecord/test/cases/adapters/postgresql/utils_test.rb +++ /dev/null @@ -1,20 +0,0 @@ -require 'cases/helper' - -class PostgreSQLUtilsTest < ActiveSupport::TestCase - include ActiveRecord::ConnectionAdapters::PostgreSQLAdapter::Utils - - def test_extract_schema_and_table - { - %(table_name) => [nil,'table_name'], - %("table.name") => [nil,'table.name'], - %(schema.table_name) => %w{schema table_name}, - %("schema".table_name) => %w{schema table_name}, - %(schema."table_name") => %w{schema table_name}, - %("schema"."table_name") => %w{schema table_name}, - %("even spaces".table) => ['even spaces','table'], - %(schema."table.name") => ['schema', 'table.name'] - }.each do |given, expect| - assert_equal expect, extract_schema_and_table(given) - end - end -end diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index bdf8e15e3e..40ed0f64a4 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -43,14 +43,16 @@ class PostgresqlUUIDTest < ActiveRecord::TestCase def test_change_column_default @connection.add_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v1()" UUIDType.reset_column_information - column = UUIDType.columns.find { |c| c.name == 'thingy' } + column = UUIDType.columns_hash['thingy'] assert_equal "uuid_generate_v1()", column.default_function - + @connection.change_column :uuid_data_type, :thingy, :uuid, null: false, default: "uuid_generate_v4()" - + UUIDType.reset_column_information - column = UUIDType.columns.find { |c| c.name == 'thingy' } + column = UUIDType.columns_hash['thingy'] assert_equal "uuid_generate_v4()", column.default_function + ensure + UUIDType.reset_column_information end def test_data_type_of_uuid_types diff --git a/activerecord/test/cases/adapters/postgresql/xml_test.rb b/activerecord/test/cases/adapters/postgresql/xml_test.rb index ae299697b1..c1c85f8c92 100644 --- a/activerecord/test/cases/adapters/postgresql/xml_test.rb +++ b/activerecord/test/cases/adapters/postgresql/xml_test.rb @@ -20,7 +20,7 @@ class PostgresqlXMLTest < ActiveRecord::TestCase rescue ActiveRecord::StatementInvalid skip "do not test on PG without xml" end - @column = XmlDataType.columns.find { |c| c.name == 'payload' } + @column = XmlDataType.columns_hash['payload'] end teardown do diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 7eaa5adc86..07903a3441 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -826,11 +826,15 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_preload_with_interpolation - post = Post.includes(:comments_with_interpolated_conditions).find(posts(:welcome).id) - assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions + assert_deprecated do + post = Post.includes(:comments_with_interpolated_conditions).find(posts(:welcome).id) + assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions + end - post = Post.joins(:comments_with_interpolated_conditions).find(posts(:welcome).id) - assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions + assert_deprecated do + post = Post.joins(:comments_with_interpolated_conditions).find(posts(:welcome).id) + assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions + end end def test_polymorphic_type_condition @@ -1232,4 +1236,23 @@ class EagerAssociationTest < ActiveRecord::TestCase assert_equal 2, author.posts.size } end + + test "include instance dependent associations is deprecated" do + message = "association scope 'posts_with_signature' is" + assert_deprecated message do + begin + Author.includes(:posts_with_signature).to_a + rescue NoMethodError + # it's expected that preloading of this association fails + end + end + + assert_deprecated message do + Author.preload(:posts_with_signature).to_a rescue NoMethodError + end + + assert_deprecated message do + Author.eager_load(:posts_with_signature).to_a + end + end end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 5d33634da2..878f1877db 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -22,6 +22,9 @@ require 'models/sponsor' require 'models/country' require 'models/treaty' require 'models/vertex' +require 'models/publisher' +require 'models/publisher/article' +require 'models/publisher/magazine' require 'active_support/core_ext/string/conversions' class ProjectWithAfterCreateHook < ActiveRecord::Base @@ -83,6 +86,12 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase country.treaties << treaty end + def test_marshal_dump + post = posts :welcome + preloaded = Post.includes(:categories).find post.id + assert_equal preloaded, Marshal.load(Marshal.dump(preloaded)) + end + def test_should_property_quote_string_primary_keys setup_data_for_habtm_case @@ -217,6 +226,24 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal developers(:poor_jamis, :jamis, :david), projects(:active_record).developers end + def test_habtm_collection_size_from_build + devel = Developer.create("name" => "Fred Wu") + devel.projects << Project.create("name" => "Grimetime") + devel.projects.build + + assert_equal 2, devel.projects.size + end + + def test_habtm_collection_size_from_params + devel = Developer.new({ + projects_attributes: { + '0' => {} + } + }) + + assert_equal 1, devel.projects.size + end + def test_build devel = Developer.find(1) proj = assert_no_queries { devel.projects.build("name" => "Projekt") } @@ -824,4 +851,13 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase def test_custom_join_table assert_equal 'edges', Vertex.reflect_on_association(:sources).join_table end + + def test_namespaced_habtm + magazine = Publisher::Magazine.create + article = Publisher::Article.create + magazine.articles << article + magazine.save + + assert_includes magazine.articles, article + end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 2453d6cf58..5f01352ab4 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1894,4 +1894,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_not pirate.valid?(:conference) assert_equal "can't be blank", ship.errors[:name].first end + + test 'association with instance dependent scope' do + bob = authors(:bob) + Post.create!(title: "signed post by bob", body: "stuff", author: authors(:bob)) + Post.create!(title: "anonymous post", body: "more stuff", author: authors(:bob)) + assert_equal ["misc post by bob", "other post by bob", + "signed post by bob"], bob.posts_with_signature.map(&:title).sort + + assert_equal [], authors(:david).posts_with_signature.map(&:title) + end end diff --git a/activerecord/test/cases/primary_keys_test.rb b/activerecord/test/cases/primary_keys_test.rb index 51ddd406ed..56d0dd6a77 100644 --- a/activerecord/test/cases/primary_keys_test.rb +++ b/activerecord/test/cases/primary_keys_test.rb @@ -219,3 +219,29 @@ if current_adapter?(:MysqlAdapter, :Mysql2Adapter) end end end + +if current_adapter?(:PostgreSQLAdapter) + class PrimaryKeyBigSerialTest < ActiveRecord::TestCase + self.use_transactional_fixtures = false + + class Widget < ActiveRecord::Base + end + + setup do + @connection = ActiveRecord::Base.connection + @connection.create_table(:widgets, id: :bigserial) { |t| } + end + + teardown do + @connection.drop_table :widgets + end + + def test_bigserial_primary_key + assert_equal "id", Widget.primary_key + assert_equal :integer, Widget.columns_hash[Widget.primary_key].type + + widget = Widget.create! + assert_not_nil widget.id + end + end +end diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index ff1c2a0d82..2b5c2fd5a4 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -17,8 +17,9 @@ class RelationMergingTest < ActiveRecord::TestCase end def test_relation_to_sql - sql = Post.first.comments.to_sql - assert_no_match(/\?/, sql) + post = Post.first + sql = post.comments.to_sql + assert_match(/.?post_id.? = #{post.id}\Z/i, sql) end def test_relation_merging_with_arel_equalities_keeps_last_equality @@ -107,6 +108,11 @@ class RelationMergingTest < ActiveRecord::TestCase merged = left.merge(right) assert_equal post, merged.first end + + def test_merging_compares_symbols_and_strings_as_equal + post = PostThatLoadsCommentsInAnAfterSaveHook.create!(title: "First Post", body: "Blah blah blah.") + assert_equal "First comment!", post.comments.where(body: "First comment!").first_or_create.body + end end class MergingDifferentRelationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/store_test.rb b/activerecord/test/cases/store_test.rb index 978cee9cfb..6a34c55011 100644 --- a/activerecord/test/cases/store_test.rb +++ b/activerecord/test/cases/store_test.rb @@ -163,6 +163,22 @@ class StoreTest < ActiveRecord::TestCase assert_equal [:width, :height], second_model.stored_attributes[:data] end + test "stored_attributes are tracked per subclass" do + first_model = Class.new(ActiveRecord::Base) do + store_accessor :data, :color + end + second_model = Class.new(first_model) do + store_accessor :data, :width, :height + end + third_model = Class.new(first_model) do + store_accessor :data, :area, :volume + end + + assert_equal [:color], first_model.stored_attributes[:data] + assert_equal [:color, :width, :height], second_model.stored_attributes[:data] + assert_equal [:color, :area, :volume], third_model.stored_attributes[:data] + end + test "YAML coder initializes the store when a Nil value is given" do assert_equal({}, @john.params) end diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 7f2e830083..e6ed85394b 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -274,48 +274,6 @@ class TransactionTest < ActiveRecord::TestCase end end - def test_rollback_when_changing_inside_transaction - assert !@first.approved? - Topic.transaction do - @first.approved = true - @first.save! - raise ActiveRecord::Rollback - end - assert @first.approved - assert @first.changes["approved"] - @first.save! - assert @first.reload.approved - end - - def test_rollback_when_changing_outside_transaction - assert !@first.approved? - @first.approved = true - Topic.transaction do - @first.save! - raise ActiveRecord::Rollback - end - assert @first.changes["approved"] - assert @first.approved - @first.save! - assert @first.reload.approved - end - - def test_rollback_when_changing_back_to_prev_stage - assert !@first.approved? - Topic.transaction do - @first.approved = true - @first.save! - @first.approved = false - @first.save! - raise ActiveRecord::Rollback - end - assert !@first.approved - assert !@first.changes["approved"] - @first.save! - assert !@first.reload.approved - end - - def test_force_savepoint_in_nested_transaction Topic.transaction do @first.approved = true diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index d80da06e27..a6e1dc72e5 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -14,28 +14,28 @@ class ValidationsTest < ActiveRecord::TestCase # Other classes we mess with will be dealt with in the specific tests repair_validations(Topic) - def test_error_on_create + def test_valid_uses_create_context_when_new r = WrongReply.new r.title = "Wrong Create" - assert !r.save + assert_not r.valid? assert r.errors[:title].any?, "A reply with a bad title should mark that attribute as invalid" assert_equal ["is Wrong Create"], r.errors[:title], "A reply with a bad content should contain an error" end - def test_error_on_update + def test_valid_uses_update_context_when_persisted r = WrongReply.new r.title = "Bad" r.content = "Good" - assert r.save, "First save should be successful" + assert r.save, "First validation should be successful" r.title = "Wrong Update" - assert !r.save, "Second save should fail" + assert_not r.valid?, "Second validation should fail" assert r.errors[:title].any?, "A reply with a bad title should mark that attribute as invalid" assert_equal ["is Wrong Update"], r.errors[:title], "A reply with a bad content should contain an error" end - def test_error_on_given_context + def test_valid_using_special_context r = WrongReply.new(:title => "Valid title") assert !r.valid?(:special_case) assert_equal "Invalid", r.errors[:author_name].join @@ -45,11 +45,11 @@ class ValidationsTest < ActiveRecord::TestCase assert r.valid?(:special_case) r.author_name = nil - assert !r.save(:context => :special_case) + assert_not r.valid?(:special_case) assert_equal "Invalid", r.errors[:author_name].join r.author_name = "secret" - assert r.save(:context => :special_case) + assert r.valid?(:special_case) end def test_validate @@ -100,7 +100,7 @@ class ValidationsTest < ActiveRecord::TestCase end end - def test_create_without_validation + def test_save_without_validation reply = WrongReply.new assert !reply.save assert reply.save(:validate => false) diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index c197951c71..8949cf5826 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -140,6 +140,8 @@ class Author < ActiveRecord::Base has_many :posts_with_default_include, :class_name => 'PostWithDefaultInclude' has_many :comments_on_posts_with_default_include, :through => :posts_with_default_include, :source => :comments + has_many :posts_with_signature, ->(record) { where("posts.title LIKE ?", "%by #{record.name.downcase}%") }, class_name: "Post" + scope :relation_include_posts, -> { includes(:posts) } scope :relation_include_tags, -> { includes(:tags) } diff --git a/activerecord/test/models/comment.rb b/activerecord/test/models/comment.rb index f82df417ce..bf0162d09b 100644 --- a/activerecord/test/models/comment.rb +++ b/activerecord/test/models/comment.rb @@ -40,3 +40,11 @@ end class VerySpecialComment < Comment end + +class CommentThatAutomaticallyAltersPostBody < Comment + belongs_to :post, class_name: "PostThatLoadsCommentsInAnAfterSaveHook", foreign_key: :post_id + + after_save do |comment| + comment.post.update_attributes(body: "Automatically altered") + end +end diff --git a/activerecord/test/models/developer.rb b/activerecord/test/models/developer.rb index 762259ffa3..0a614c3bfd 100644 --- a/activerecord/test/models/developer.rb +++ b/activerecord/test/models/developer.rb @@ -13,6 +13,8 @@ class Developer < ActiveRecord::Base end end + accepts_nested_attributes_for :projects + has_and_belongs_to_many :projects_extended_by_name, -> { extending(DeveloperProjectsAssociationExtension) }, :class_name => "Project", diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index b1e56c14d1..5f01ab0a82 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -208,3 +208,12 @@ class SpecialPostWithDefaultScope < ActiveRecord::Base self.table_name = 'posts' default_scope { where(:id => [1, 5,6]) } end + +class PostThatLoadsCommentsInAnAfterSaveHook < ActiveRecord::Base + self.table_name = 'posts' + has_many :comments, class_name: "CommentThatAutomaticallyAltersPostBody", foreign_key: :post_id + + after_save do |post| + post.comments.load + end +end diff --git a/activerecord/test/models/publisher.rb b/activerecord/test/models/publisher.rb new file mode 100644 index 0000000000..0d4a7f9235 --- /dev/null +++ b/activerecord/test/models/publisher.rb @@ -0,0 +1,2 @@ +module Publisher +end diff --git a/activerecord/test/models/publisher/article.rb b/activerecord/test/models/publisher/article.rb new file mode 100644 index 0000000000..03a277bbdd --- /dev/null +++ b/activerecord/test/models/publisher/article.rb @@ -0,0 +1,3 @@ +class Publisher::Article < ActiveRecord::Base + has_and_belongs_to_many :magazines +end diff --git a/activerecord/test/models/publisher/magazine.rb b/activerecord/test/models/publisher/magazine.rb new file mode 100644 index 0000000000..82e1a14008 --- /dev/null +++ b/activerecord/test/models/publisher/magazine.rb @@ -0,0 +1,3 @@ +class Publisher::Magazine < ActiveRecord::Base + has_and_belongs_to_many :articles +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index da3074e90f..a78074d530 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -62,6 +62,14 @@ ActiveRecord::Schema.define do t.string :name end + create_table :articles, force: true do |t| + end + + create_table :articles_magazines, force: true do |t| + t.references :article + t.references :magazine + end + create_table :audit_logs, force: true do |t| t.column :message, :string, null: false t.column :developer_id, :integer, null: false @@ -385,6 +393,9 @@ ActiveRecord::Schema.define do t.column :custom_lock_version, :integer end + create_table :magazines, force: true do |t| + end + create_table :mateys, id: false, force: true do |t| t.column :pirate_id, :integer t.column :target_id, :integer diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 8e63273271..866a5a958d 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,22 @@ +* Remove deprecated string based terminators for `ActiveSupport::Callbacks`. + + *Eileen M. Uchitelle* + +* Fixed an issue when using + `ActiveSupport::NumberHelper::NumberToDelimitedConverter` to + convert a value that is an `ActiveSupport::SafeBuffer` introduced + in 2da9d67. + + See #15064. + + *Mark J. Titorenko* + +* `TimeZone#parse` defaults the day of the month to '1' if any other date + components are specified. This is more consistent with the behavior of + `Time#parse`. + + *Ulysse Carion* + * `humanize` strips leading underscores, if any. Before: diff --git a/activesupport/lib/active_support/backtrace_cleaner.rb b/activesupport/lib/active_support/backtrace_cleaner.rb index d58578b7bc..1fec1bea0d 100644 --- a/activesupport/lib/active_support/backtrace_cleaner.rb +++ b/activesupport/lib/active_support/backtrace_cleaner.rb @@ -65,14 +65,14 @@ module ActiveSupport @silencers << block end - # Will remove all silencers, but leave in the filters. This is useful if - # your context of debugging suddenly expands as you suspect a bug in one of + # Removes all silencers, but leaves in the filters. Useful if your + # context of debugging suddenly expands as you suspect a bug in one of # the libraries you use. def remove_silencers! @silencers = [] end - # Removes all filters, but leaves in silencers. Useful if you suddenly + # Removes all filters, but leaves in the silencers. Useful if you suddenly # need to see entire filepaths in the backtrace that you had already # filtered out. def remove_filters! diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index 05ca943776..06505bddf9 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -297,14 +297,14 @@ module ActiveSupport target = env.target value = env.value - unless env.halted + if env.halted + next_callback.call env + else user_callback.call(target, value) { env = next_callback.call env env.value } env - else - next_callback.call env end } end @@ -724,12 +724,6 @@ module ActiveSupport # would call <tt>Audit#save</tt>. def define_callbacks(*names) options = names.extract_options! - if options.key?(:terminator) && String === options[:terminator] - ActiveSupport::Deprecation.warn "String based terminators are deprecated, please use a lambda" - value = options[:terminator] - line = class_eval "lambda { |result| #{value} }", __FILE__, __LINE__ - options[:terminator] = lambda { |target, result| target.instance_exec(result, &line) } - end names.each do |name| class_attribute "_#{name}_callbacks" diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index 6c3e48a3ca..2149d4439d 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -221,7 +221,7 @@ module ActiveSupport def garbage?(value) # If the type is the only element which makes it then # this still makes the value nil, except if type is - # a XML node(where type['value'] is a Hash) + # an XML node(where type['value'] is a Hash) value['type'] && !value['type'].is_a?(::Hash) && value.size == 1 end diff --git a/activesupport/lib/active_support/core_ext/object/to_query.rb b/activesupport/lib/active_support/core_ext/object/to_query.rb index 37352fa608..172f06ed64 100644 --- a/activesupport/lib/active_support/core_ext/object/to_query.rb +++ b/activesupport/lib/active_support/core_ext/object/to_query.rb @@ -1,4 +1,5 @@ require 'active_support/core_ext/object/to_param' +require 'cgi' class Object # Converts an object into a string suitable for use as a URL query string, using the given <tt>key</tt> as the @@ -6,7 +7,6 @@ class Object # # Note: This method is defined as a default implementation for all Objects for Hash#to_query to work. def to_query(key) - require 'cgi' unless defined?(CGI) && defined?(CGI::escape) "#{CGI.escape(key.to_param)}=#{CGI.escape(to_param.to_s)}" end end diff --git a/activesupport/lib/active_support/number_helper/number_to_delimited_converter.rb b/activesupport/lib/active_support/number_helper/number_to_delimited_converter.rb index 6405afc9a6..d85cc086d7 100644 --- a/activesupport/lib/active_support/number_helper/number_to_delimited_converter.rb +++ b/activesupport/lib/active_support/number_helper/number_to_delimited_converter.rb @@ -13,7 +13,9 @@ module ActiveSupport def parts left, right = number.to_s.split('.') - left.gsub!(DELIMITED_REGEX) { "#{$1}#{options[:delimiter]}" } + left.gsub!(DELIMITED_REGEX) do |digit_to_delimit| + "#{digit_to_delimit}#{options[:delimiter]}" + end [left, right].compact end end diff --git a/activesupport/lib/active_support/time_with_zone.rb b/activesupport/lib/active_support/time_with_zone.rb index c25c97cfa8..f2a2f3c3db 100644 --- a/activesupport/lib/active_support/time_with_zone.rb +++ b/activesupport/lib/active_support/time_with_zone.rb @@ -185,8 +185,11 @@ module ActiveSupport end alias_method :rfc822, :rfc2822 - # <tt>:db</tt> format outputs time in UTC; all others output time in local. - # Uses TimeWithZone's +strftime+, so <tt>%Z</tt> and <tt>%z</tt> work correctly. + # Returns a string of the object's date and time. + # Accepts an optional <tt>format</tt>: + # * <tt>:default</tt> - default value, mimics Ruby 1.9 Time#to_s format. + # * <tt>:db</tt> - format outputs time in UTC :db time. See Time#to_formatted_s(:db). + # * Any key in <tt>Time::DATE_FORMATS</tt> can be used. See active_support/core_ext/time/conversions.rb. def to_s(format = :default) if format == :db utc.to_s(format) diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index 38f0d268f4..72efb09fbe 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -282,6 +282,11 @@ module ActiveSupport # # Time.zone.now # => Fri, 31 Dec 1999 14:00:00 HST -10:00 # Time.zone.parse('22:30:00') # => Fri, 31 Dec 1999 22:30:00 HST -10:00 + # + # However, if the date component is not provided, but any other upper + # components are supplied, then the day of the month defaults to 1: + # + # Time.zone.parse('Mar 2000') # => Wed, 01 Mar 2000 00:00:00 HST -10:00 def parse(str, now=now()) parts = Date._parse(str, false) return if parts.empty? @@ -289,7 +294,7 @@ module ActiveSupport time = Time.new( parts.fetch(:year, now.year), parts.fetch(:mon, now.month), - parts.fetch(:mday, now.day), + parts.fetch(:mday, parts[:year] || parts[:mon] ? 1 : now.day), parts.fetch(:hour, 0), parts.fetch(:min, 0), parts.fetch(:sec, 0) + parts.fetch(:sec_fraction, 0), diff --git a/activesupport/test/core_ext/array_ext_test.rb b/activesupport/test/core_ext/array_ext_test.rb index 57722fd52a..e0e54f47e4 100644 --- a/activesupport/test/core_ext/array_ext_test.rb +++ b/activesupport/test/core_ext/array_ext_test.rb @@ -1,10 +1,9 @@ require 'abstract_unit' require 'active_support/core_ext/array' require 'active_support/core_ext/big_decimal' +require 'active_support/core_ext/hash' require 'active_support/core_ext/object/conversions' - -require 'active_support/core_ext' # FIXME: pulling in all to_xml extensions -require 'active_support/hash_with_indifferent_access' +require 'active_support/core_ext/string' class ArrayExtAccessTests < ActiveSupport::TestCase def test_from @@ -234,7 +233,7 @@ class ArraySplitTests < ActiveSupport::TestCase end class ArrayToXmlTests < ActiveSupport::TestCase - def test_to_xml + def test_to_xml_with_hash_elements xml = [ { :name => "David", :age => 26, :age_in_millis => 820497600000 }, { :name => "Jason", :age => 31, :age_in_millis => BigDecimal.new('1.0') } @@ -249,6 +248,22 @@ class ArrayToXmlTests < ActiveSupport::TestCase assert xml.include?(%(<name>Jason</name>)), xml end + def test_to_xml_with_non_hash_elements + xml = [1, 2, 3].to_xml(:skip_instruct => true, :indent => 0) + + assert_equal '<fixnums type="array"><fixnum', xml.first(29) + assert xml.include?(%(<fixnum type="integer">2</fixnum>)), xml + end + + def test_to_xml_with_non_hash_different_type_elements + xml = [1, 2.0, '3'].to_xml(:skip_instruct => true, :indent => 0) + + assert_equal '<objects type="array"><object', xml.first(29) + assert xml.include?(%(<object type="integer">1</object>)), xml + assert xml.include?(%(<object type="float">2.0</object>)), xml + assert xml.include?(%(object>3</object>)), xml + end + def test_to_xml_with_dedicated_name xml = [ { :name => "David", :age => 26, :age_in_millis => 820497600000 }, { :name => "Jason", :age => 31 } @@ -269,6 +284,18 @@ class ArrayToXmlTests < ActiveSupport::TestCase assert xml.include?(%(<name>Jason</name>)) end + def test_to_xml_with_indent_set + xml = [ + { :name => "David", :street_address => "Paulina" }, { :name => "Jason", :street_address => "Evergreen" } + ].to_xml(:skip_instruct => true, :skip_types => true, :indent => 4) + + assert_equal "<objects>\n <object>", xml.first(22) + assert xml.include?(%(\n <street-address>Paulina</street-address>)) + assert xml.include?(%(\n <name>David</name>)) + assert xml.include?(%(\n <street-address>Evergreen</street-address>)) + assert xml.include?(%(\n <name>Jason</name>)) + end + def test_to_xml_with_dasherize_false xml = [ { :name => "David", :street_address => "Paulina" }, { :name => "Jason", :street_address => "Evergreen" } @@ -289,7 +316,7 @@ class ArrayToXmlTests < ActiveSupport::TestCase assert xml.include?(%(<street-address>Evergreen</street-address>)) end - def test_to_with_instruct + def test_to_xml_with_instruct xml = [ { :name => "David", :age => 26, :age_in_millis => 820497600000 }, { :name => "Jason", :age => 31, :age_in_millis => BigDecimal.new('1.0') } diff --git a/activesupport/test/number_helper_test.rb b/activesupport/test/number_helper_test.rb index 9bdb92024e..a7a0ae02e7 100644 --- a/activesupport/test/number_helper_test.rb +++ b/activesupport/test/number_helper_test.rb @@ -1,5 +1,6 @@ require 'abstract_unit' require 'active_support/number_helper' +require 'active_support/core_ext/string/output_safety' module ActiveSupport module NumberHelper @@ -97,6 +98,7 @@ module ActiveSupport assert_equal("123,456,789.78901", number_helper.number_to_delimited(123456789.78901)) assert_equal("0.78901", number_helper.number_to_delimited(0.78901)) assert_equal("123,456.78", number_helper.number_to_delimited("123456.78")) + assert_equal("123,456.78", number_helper.number_to_delimited("123456.78".html_safe)) end end diff --git a/activesupport/test/time_zone_test.rb b/activesupport/test/time_zone_test.rb index 79ec57af2b..127bcc2b4d 100644 --- a/activesupport/test/time_zone_test.rb +++ b/activesupport/test/time_zone_test.rb @@ -254,6 +254,15 @@ class TimeZoneTest < ActiveSupport::TestCase assert_equal Time.utc(1999,12,31,19), twz.time end + def test_parse_with_day_omitted + with_env_tz 'US/Eastern' do + zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)'] + assert_equal Time.local(2000, 2, 1), zone.parse('Feb', Time.local(2000, 1, 1)) + assert_equal Time.local(2005, 2, 1), zone.parse('Feb 2005', Time.local(2000, 1, 1)) + assert_equal Time.local(2005, 2, 2), zone.parse('2 Feb 2005', Time.local(2000, 1, 1)) + end + end + def test_parse_should_not_black_out_system_timezone_dst_jump with_env_tz('EET') do zone = ActiveSupport::TimeZone['Pacific Time (US & Canada)'] diff --git a/guides/rails_guides.rb b/guides/rails_guides.rb index 1bdeef2947..9d1d5567f6 100644 --- a/guides/rails_guides.rb +++ b/guides/rails_guides.rb @@ -24,11 +24,11 @@ begin require 'redcarpet' rescue LoadError # This can happen if doc:guides is executed in an application. - $stderr.puts('Generating guides requires Redcarpet 3.1.0+.') + $stderr.puts('Generating guides requires Redcarpet 3.1.2+.') $stderr.puts(<<ERROR) if bundler? Please add - gem 'redcarpet', '~> 3.1.0' + gem 'redcarpet', '~> 3.1.2' to the Gemfile, run diff --git a/guides/rails_guides/markdown/renderer.rb b/guides/rails_guides/markdown/renderer.rb index 3791ed6fd5..2eb7ca17a3 100644 --- a/guides/rails_guides/markdown/renderer.rb +++ b/guides/rails_guides/markdown/renderer.rb @@ -15,7 +15,7 @@ module RailsGuides HTML end - def header(text, header_level, anchor) + def header(text, header_level) # Always increase the heading level by, so we can use h1, h2 heading in the document header_level += 1 diff --git a/guides/source/active_record_postgresql.md b/guides/source/active_record_postgresql.md index ae767769dd..169a48afb9 100644 --- a/guides/source/active_record_postgresql.md +++ b/guides/source/active_record_postgresql.md @@ -12,10 +12,9 @@ It describes how to properly setup Active Record for PostgreSQL. After reading this guide, you will know: - * How to use PostgreSQL's datatypes. -* How to use UUID Primary keys. -* How to implement Full text search with PostgreSQL. +* How to use UUID primary keys. +* How to implement full text search with PostgreSQL. -------------------------------------------------------------------------------- @@ -99,7 +98,7 @@ Profile.create(settings: { "color" => "blue", "resolution" => "800x600" }) profile = Profile.first profile.settings # => {"color"=>"blue", "resolution"=>"800x600"} -profile.settings = {"color" => "yellow", "resulution" => "1280x1024"} +profile.settings = {"color" => "yellow", "resolution" => "1280x1024"} profile.save! ## you need to call _will_change! if you are editing the store in place @@ -291,6 +290,33 @@ user.save! The types `inet` and `cidr` are mapped to Ruby [`IPAddr`](http://www.ruby-doc.org/stdlib-2.1.1/libdoc/ipaddr/rdoc/IPAddr.html) objects. The `macaddr` type is mapped to normal text. +```ruby +# db/migrate/20140508144913_create_devices.rb +create_table(:devices, force: true) do |t| + t.inet 'ip' + t.cidr 'network' + t.macaddr 'address' +end + +# app/models/device.rb +class Device < ActiveRecord::Base +end + +# Usage +macbook = Device.create(ip: "192.168.1.12", + network: "192.168.2.0/24", + address: "32:01:16:6d:05:ef") + +macbook.ip +# => #<IPAddr: IPv4:192.168.1.12/255.255.255.255> + +macbook.network +# => #<IPAddr: IPv4:192.168.2.0/255.255.255.0> + +macbook.address +# => "32:01:16:6d:05:ef" +``` + ### Geometric Types * [type definition](http://www.postgresql.org/docs/9.3/static/datatype-geometric.html) @@ -345,3 +371,62 @@ Document.where("to_tsvector('english', title || ' ' || body) @@ to_tsquery(?)", Views ----- + +* [view creation](http://www.postgresql.org/docs/9.3/static/sql-createview.html) + +Imagine you need to work with a legacy database containing the following table: + +``` +rails_pg_guide=# \d "TBL_ART" + Table "public.TBL_ART" + Column | Type | Modifiers +------------+-----------------------------+------------------------------------------------------------ + INT_ID | integer | not null default nextval('"TBL_ART_INT_ID_seq"'::regclass) + STR_TITLE | character varying | + STR_STAT | character varying | default 'draft'::character varying + DT_PUBL_AT | timestamp without time zone | + BL_ARCH | boolean | default false +Indexes: + "TBL_ART_pkey" PRIMARY KEY, btree ("INT_ID") +``` + +This table does not follow the Rails conventions at all. +Because simple PostgreSQL views are updateable by default, +we can wrap it as follows: + +```ruby +# db/migrate/20131220144913_create_articles_view.rb +execute <<-SQL +CREATE VIEW articles AS + SELECT "INT_ID" AS id, + "STR_TITLE" AS title, + "STR_STAT" AS status, + "DT_PUBL_AT" AS published_at, + "BL_ARCH" AS archived + FROM "TBL_ART" + WHERE "BL_ARCH" = 'f' + SQL + +# app/models/article.rb +class Article < ActiveRecord::Base + self.primary_key = "id" + def archive! + update_attribute :archived, true + end +end + +# Usage +first = Article.create! title: "Winter is coming", + status: "published", + published_at: 1.year.ago +second = Article.create! title: "Brace yourself", + status: "draft", + published_at: 1.month.ago + +Article.count # => 1 +first.archive! +p Article.count # => 2 +``` + +Note: This application only cares about non-archived `Articles`. A view also +allows for conditions so we can exclude the archived `Articles` directly. diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index 329db7cf29..8d0d6d260d 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -1106,7 +1106,7 @@ end A model may find it useful to set `:instance_accessor` to `false` as a way to prevent mass-assignment from setting the attribute. -NOTE: Defined in `active_support/core_ext/module/attribute_accessors.rb`. `active_support/core_ext/class/attribute_accessors.rb` is deprecated and will be removed in Ruby on Rails 4.2. +NOTE: Defined in `active_support/core_ext/module/attribute_accessors.rb`. ### Subclasses & Descendants diff --git a/guides/source/generators.md b/guides/source/generators.md index 4a5377c206..a1ba97fd35 100644 --- a/guides/source/generators.md +++ b/guides/source/generators.md @@ -248,7 +248,7 @@ end end ``` -We can try out our new generator by creating a helper for users: +We can try out our new generator by creating a helper for products: ```bash $ rails generate my_helper products @@ -507,7 +507,7 @@ Replaces text inside a file. gsub_file 'name_of_file.rb', 'method.to_be_replaced', 'method.the_replacing_code' ``` -Regular Expressions can be used to make this method more precise. You can also use append_file and prepend_file in the same way to place code at the beginning and end of a file respectively. +Regular Expressions can be used to make this method more precise. You can also use `append_file` and `prepend_file` in the same way to place code at the beginning and end of a file respectively. ### `application` diff --git a/guides/source/migrations.md b/guides/source/migrations.md index c61ccfe94a..fe1a6a4697 100644 --- a/guides/source/migrations.md +++ b/guides/source/migrations.md @@ -902,6 +902,11 @@ schema into a RDBMS other than the one used to create it. Because schema dumps are the authoritative source for your database schema, it is strongly recommended that you check them into source control. +`db/schema.rb` contains the current version number of the database. This +ensures conflicts are going to happen in the case of a merge where both +branches touched the schema. When that happens, solve conflicts manually, +keeping the highest version number of the two. + Active Record and Referential Integrity --------------------------------------- diff --git a/guides/source/routing.md b/guides/source/routing.md index 0783bce442..9dab946c72 100644 --- a/guides/source/routing.md +++ b/guides/source/routing.md @@ -662,7 +662,7 @@ get 'photos/:id', to: 'photos#show', id: /[A-Z]\d{5}/ `:constraints` takes regular expressions with the restriction that regexp anchors can't be used. For example, the following route will not work: ```ruby -get '/:id', to: 'posts#show', constraints: {id: /^\d/} +get '/:id', to: 'posts#show', constraints: { id: /^\d/ } ``` However, note that you don't need to use anchors because all routes are anchored at the start. @@ -676,12 +676,12 @@ get '/:username', to: 'users#show' ### Request-Based Constraints -You can also constrain a route based on any method on the <a href="action_controller_overview.html#the-request-object">Request</a> object that returns a `String`. +You can also constrain a route based on any method on the [Request object](action_controller_overview.html#the-request-object) that returns a `String`. You specify a request-based constraint the same way that you specify a segment constraint: ```ruby -get 'photos', constraints: {subdomain: 'admin'} +get 'photos', constraints: { subdomain: 'admin' } ``` You can also specify constraints in a block form: @@ -694,7 +694,7 @@ namespace :admin do end ``` -NOTE: Request constraints work by calling a method on the <a href="action_controller_overview.html#the-request-object">Request object</a> with the same name as the hash key and then compare the return value with the hash value. Therefore, constraint values should match the corresponding Request object method return type. For example: `constraints: { subdomain: 'api' }` will match an `api` subdomain as expected, however using a symbol `constraints: { subdomain: :api }` will not, because `request.subdomain` returns `'api'` as a String. +NOTE: Request constraints work by calling a method on the [Request object](action_controller_overview.html#the-request-object) with the same name as the hash key and then compare the return value with the hash value. Therefore, constraint values should match the corresponding Request object method return type. For example: `constraints: { subdomain: 'api' }` will match an `api` subdomain as expected, however using a symbol `constraints: { subdomain: :api }` will not, because `request.subdomain` returns `'api'` as a String. ### Advanced Constraints @@ -783,8 +783,8 @@ get '/stories/:name', to: redirect('/posts/%{name}') You can also provide a block to redirect, which receives the symbolized path parameters and the request object: ```ruby -get '/stories/:name', to: redirect {|path_params, req| "/posts/#{path_params[:name].pluralize}" } -get '/stories', to: redirect {|path_params, req| "/posts/#{req.subdomain}" } +get '/stories/:name', to: redirect { |path_params, req| "/posts/#{path_params[:name].pluralize}" } +get '/stories', to: redirect { |path_params, req| "/posts/#{req.subdomain}" } ``` Please note that this redirection is a 301 "Moved Permanently" redirect. Keep in mind that some web browsers or proxy servers will cache this type of redirect, making the old page inaccessible. @@ -793,7 +793,7 @@ In all of these cases, if you don't provide the leading host (`http://www.exampl ### Routing to Rack Applications -Instead of a String like `'posts#index'`, which corresponds to the `index` action in the `PostsController`, you can specify any <a href="rails_on_rack.html">Rack application</a> as the endpoint for a matcher: +Instead of a String like `'posts#index'`, which corresponds to the `index` action in the `PostsController`, you can specify any [Rack application](rails_on_rack.html) as the endpoint for a matcher: ```ruby match '/application.js', to: Sprockets, via: :all @@ -879,7 +879,7 @@ a warning. You can use the `:constraints` option to specify a required format on the implicit `id`. For example: ```ruby -resources :photos, constraints: {id: /[A-Z][A-Z][0-9]+/} +resources :photos, constraints: { id: /[A-Z][A-Z][0-9]+/ } ``` This declaration constrains the `:id` parameter to match the supplied regular expression. So, in this case, the router would no longer match `/photos/1` to this route. Instead, `/photos/RR27` would match. @@ -919,7 +919,7 @@ will recognize incoming paths beginning with `/photos` and route the requests to ### Overriding the `new` and `edit` Segments -The `:path_names` option lets you override the automatically-generated "new" and "edit" segments in paths: +The `:path_names` option lets you override the automatically-generated `new` and `edit` segments in paths: ```ruby resources :photos, path_names: { new: 'make', edit: 'change' } @@ -954,7 +954,7 @@ end resources :photos ``` -This will provide route helpers such as `admin_photos_path`, `new_admin_photo_path` etc. +This will provide route helpers such as `admin_photos_path`, `new_admin_photo_path`, etc. To prefix a group of route helpers, use `:as` with `scope`: @@ -982,7 +982,7 @@ This will provide you with URLs such as `/bob/posts/1` and will allow you to ref ### Restricting the Routes Created -By default, Rails creates routes for the seven default actions (index, show, new, create, edit, update, and destroy) for every RESTful route in your application. You can use the `:only` and `:except` options to fine-tune this behavior. The `:only` option tells Rails to create only the specified routes: +By default, Rails creates routes for the seven default actions (`index`, `show`, `new`, `create`, `edit`, `update`, and `destroy`) for every RESTful route in your application. You can use the `:only` and `:except` options to fine-tune this behavior. The `:only` option tells Rails to create only the specified routes: ```ruby resources :photos, only: [:index, :show] diff --git a/railties/lib/rails/generators/base.rb b/railties/lib/rails/generators/base.rb index 67bab96a22..9af6435f23 100644 --- a/railties/lib/rails/generators/base.rb +++ b/railties/lib/rails/generators/base.rb @@ -83,7 +83,7 @@ module Rails # # The first and last part used to find the generator to be invoked are # guessed based on class invokes hook_for, as noticed in the example above. - # This can be customized with two options: :base and :as. + # This can be customized with two options: :in and :as. # # Let's suppose you are creating a generator that needs to invoke the # controller generator from test unit. Your first attempt is: @@ -108,7 +108,7 @@ module Rails # "test_unit:controller", "test_unit" # # Similarly, if you want it to also lookup in the rails namespace, you just - # need to provide the :base value: + # need to provide the :in value: # # class AwesomeGenerator < Rails::Generators::Base # hook_for :test_framework, in: :rails, as: :controller diff --git a/railties/lib/rails/generators/rails/plugin/templates/Gemfile b/railties/lib/rails/generators/rails/plugin/templates/Gemfile index 1f704db510..796587f316 100644 --- a/railties/lib/rails/generators/rails/plugin/templates/Gemfile +++ b/railties/lib/rails/generators/rails/plugin/templates/Gemfile @@ -1,7 +1,7 @@ -source "https://rubygems.org" +source 'https://rubygems.org' <% if options[:skip_gemspec] -%> -<%= '# ' if options.dev? || options.edge? -%>gem "rails", "~> <%= Rails::VERSION::STRING %>" +<%= '# ' if options.dev? || options.edge? -%>gem 'rails', '~> <%= Rails::VERSION::STRING %>' <% else -%> # Declare your gem's dependencies in <%= name %>.gemspec. # Bundler will treat runtime dependencies like base dependencies, and @@ -11,7 +11,7 @@ gemspec <% if options[:skip_gemspec] -%> group :development do - gem "<%= gem_for_database %>" + gem '<%= gem_for_database %>' end <% else -%> # Declare any dependencies that are still in development here instead of in diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 19912805a8..e95c3fa20d 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -360,7 +360,7 @@ module ApplicationTests test "default method for update can be changed" do app_file 'app/models/post.rb', <<-RUBY class Post - extend ActiveModel::Naming + include ActiveModel::Model def to_key; [1]; end def persisted?; true; end end diff --git a/railties/test/generators/plugin_generator_test.rb b/railties/test/generators/plugin_generator_test.rb index 69ff23eb95..7180efee41 100644 --- a/railties/test/generators/plugin_generator_test.rb +++ b/railties/test/generators/plugin_generator_test.rb @@ -312,7 +312,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_no_file "bukkits.gemspec" assert_file "Gemfile" do |contents| assert_no_match('gemspec', contents) - assert_match(/gem "rails", "~> #{Rails.version}"/, contents) + assert_match(/gem 'rails', '~> #{Rails.version}'/, contents) assert_match_sqlite3(contents) assert_no_match(/# gem "jquery-rails"/, contents) end @@ -323,7 +323,7 @@ class PluginGeneratorTest < Rails::Generators::TestCase assert_no_file "bukkits.gemspec" assert_file "Gemfile" do |contents| assert_no_match('gemspec', contents) - assert_match(/gem "rails", "~> #{Rails.version}"/, contents) + assert_match(/gem 'rails', '~> #{Rails.version}'/, contents) assert_match_sqlite3(contents) end end @@ -416,9 +416,9 @@ protected def assert_match_sqlite3(contents) unless defined?(JRUBY_VERSION) - assert_match(/group :development do\n gem "sqlite3"\nend/, contents) + assert_match(/group :development do\n gem 'sqlite3'\nend/, contents) else - assert_match(/group :development do\n gem "activerecord-jdbcsqlite3-adapter"\nend/, contents) + assert_match(/group :development do\n gem 'activerecord-jdbcsqlite3-adapter'\nend/, contents) end end end diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index c4b18e9ea5..28e5b2ff1e 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -592,11 +592,15 @@ YAML @plugin.write "app/models/bukkits/post.rb", <<-RUBY module Bukkits class Post - extend ActiveModel::Naming + include ActiveModel::Model def to_param "1" end + + def persisted? + true + end end end RUBY @@ -704,8 +708,7 @@ YAML @plugin.write "app/models/bukkits/post.rb", <<-RUBY module Bukkits class Post - extend ActiveModel::Naming - include ActiveModel::Conversion + include ActiveModel::Model attr_accessor :title def to_param diff --git a/railties/test/railties/mounted_engine_test.rb b/railties/test/railties/mounted_engine_test.rb index 0ef2ff2e2e..fb2071c7c3 100644 --- a/railties/test/railties/mounted_engine_test.rb +++ b/railties/test/railties/mounted_engine_test.rb @@ -88,18 +88,14 @@ module ApplicationTests @plugin.write "app/models/blog/post.rb", <<-RUBY module Blog class Post - extend ActiveModel::Naming + include ActiveModel::Model def id 44 end - def to_param - id.to_s - end - - def new_record? - false + def persisted? + true end end end |