diff options
42 files changed, 356 insertions, 171 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 0f1e1f303f..85a83ed7d9 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,39 @@ ## Rails 4.0.0 (unreleased) ## +* Allow setting a symbol as path in scope on routes. This is now allowed: + + scope :api do + resources :users + end + + It is also possible to pass multiple symbols to scope to shorten multiple nested scopes: + + scope :api do + scope :v1 do + resources :users + end + end + + can be rewritten as: + + scope :api, :v1 do + resources :users + end + + *Guillermo Iguaran* + +* Fix error when using a non-hash query argument named "params" in `url_for`. + + Before: + + url_for(params: "") # => undefined method `reject!' for "":String + + After: + + url_for(params: "") # => http://www.example.com?params= + + *tumayun + Carlos Antonio da Silva* + * Render every partial with a new `ActionView::PartialRenderer`. This resolves issues when rendering nested partials. Fix #8197 diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 5aecb59df9..be8055955d 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -509,7 +509,7 @@ module ActionController @request.assign_parameters(@routes, controller_class_name, action.to_s, parameters) @request.session.update(session) if session - @request.session["flash"] = @request.flash.update(flash || {}) + @request.flash.update(flash || {}) @controller.request = @request @controller.response = @response @@ -526,6 +526,7 @@ module ActionController @response.prepare! @assigns = @controller.respond_to?(:view_assigns) ? @controller.view_assigns : {} + @request.session['flash'] = @request.flash.to_session_value @request.session.delete('flash') if @request.session['flash'].blank? @response end diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 9a7e8a5a9c..bced7d84c0 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -28,7 +28,7 @@ module ActionDispatch path = options.delete(:script_name).to_s.chomp("/") path << options.delete(:path).to_s - params = options[:params] || {} + params = options[:params].is_a?(Hash) ? options[:params] : options.slice(:params) params.reject! { |_,v| v.to_param.nil? } result = build_host_url(options) diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index 9928b7cc3a..7b18c57420 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -4,7 +4,7 @@ module ActionDispatch # read a notice you put there or <tt>flash["notice"] = "hello"</tt> # to put a new one. def flash - @env[Flash::KEY] ||= (session["flash"] || Flash::FlashHash.new).tap(&:sweep) + @env[Flash::KEY] ||= Flash::FlashHash.from_session_value(session["flash"]) end end @@ -70,16 +70,30 @@ module ActionDispatch end end - # Implementation detail: please do not change the signature of the - # FlashHash class. Doing that will likely affect all Rails apps in - # production as the FlashHash currently stored in their sessions will - # become invalid. class FlashHash include Enumerable - def initialize #:nodoc: - @discard = Set.new - @flashes = {} + def self.from_session_value(value) + flash = case value + when FlashHash # Rails 3.1, 3.2 + new(value.instance_variable_get(:@flashes), value.instance_variable_get(:@used)) + when Hash # Rails 4.0 + new(value['flashes'], value['discard']) + else + new + end + + flash.tap(&:sweep) + end + + def to_session_value + return nil if empty? + {'discard' => @discard.to_a, 'flashes' => @flashes} + end + + def initialize(flashes = {}, discard = []) #:nodoc: + @discard = Set.new(discard) + @flashes = flashes @now = nil end @@ -223,7 +237,7 @@ module ActionDispatch if flash_hash if !flash_hash.empty? || session.key?('flash') - session["flash"] = flash_hash + session["flash"] = flash_hash.to_session_value new_hash = flash_hash.dup else new_hash = flash_hash @@ -233,7 +247,7 @@ module ActionDispatch end if (!session.respond_to?(:loaded?) || session.loaded?) && # (reset_session uses {}, which doesn't implement #loaded?) - session.key?('flash') && session['flash'].empty? + session.key?('flash') && session['flash'].nil? session.delete('flash') end end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index d6fe436b68..0c19b493ab 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -152,7 +152,7 @@ module ActionDispatch end def conditions - { :path_info => @path }.merge(constraints).merge(request_method_condition) + { :path_info => @path }.merge!(constraints).merge!(request_method_condition) end def requirements @@ -252,7 +252,7 @@ module ActionDispatch def defaults_from_constraints(constraints) url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.slice(*url_keys).select{ |k, v| v.is_a?(String) || v.is_a?(Fixnum) } + constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } end end @@ -285,7 +285,7 @@ module ActionDispatch # of most Rails applications, this is beneficial. def root(options = {}) options = { :to => options } if options.is_a?(String) - match '/', { :as => :root, :via => :get }.merge(options) + match '/', { :as => :root, :via => :get }.merge!(options) end # Matches a url pattern to one or more routes. Any symbols in a pattern @@ -641,19 +641,16 @@ module ActionDispatch # resources :posts # end def scope(*args) - options = args.extract_options! - options = options.dup - - options[:path] = args.first if args.first.is_a?(String) + options = args.extract_options!.dup recover = {} + options[:path] = args.flatten.join('/') if args.any? options[:constraints] ||= {} - unless options[:constraints].is_a?(Hash) - block, options[:constraints] = options[:constraints], {} - end if options[:constraints].is_a?(Hash) (options[:defaults] ||= {}).reverse_merge!(defaults_from_constraints(options[:constraints])) + else + block, options[:constraints] = options[:constraints], {} end scope_options.each do |option| @@ -663,8 +660,8 @@ module ActionDispatch end end - recover[:block] = @scope[:blocks] - @scope[:blocks] = merge_blocks_scope(@scope[:blocks], block) + recover[:blocks] = @scope[:blocks] + @scope[:blocks] = merge_blocks_scope(@scope[:blocks], block) recover[:options] = @scope[:options] @scope[:options] = merge_options_scope(@scope[:options], options) @@ -672,12 +669,7 @@ module ActionDispatch yield self ensure - scope_options.each do |option| - @scope[option] = recover[option] if recover.has_key?(option) - end - - @scope[:options] = recover[:options] - @scope[:blocks] = recover[:block] + @scope.merge!(recover) end # Scopes routes to a specific controller @@ -853,7 +845,7 @@ module ActionDispatch end def merge_options_scope(parent, child) #:nodoc: - (parent || {}).except(*override_keys(child)).merge(child) + (parent || {}).except(*override_keys(child)).merge!(child) end def merge_shallow_scope(parent, child) #:nodoc: @@ -866,7 +858,7 @@ module ActionDispatch def defaults_from_constraints(constraints) url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.slice(*url_keys).select{ |k, v| v.is_a?(String) || v.is_a?(Fixnum) } + constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } end end diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index d70063d0e9..d751e04e6a 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -75,7 +75,7 @@ module ActionDispatch :port => request.optional_port, :path => request.path, :params => request.query_parameters - }.merge options + }.merge! options if !params.empty? && url_options[:path].match(/%\{\w*\}/) url_options[:path] = (url_options[:path] % escape_path(params)) diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb index 36d557e1a3..f5fdf766ad 100644 --- a/actionpack/lib/action_view/renderer/partial_renderer.rb +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -1,4 +1,3 @@ - module ActionView # = Action View Partials # diff --git a/actionpack/lib/action_view/renderer/renderer.rb b/actionpack/lib/action_view/renderer/renderer.rb index dfef43bc9d..30a0c4be70 100644 --- a/actionpack/lib/action_view/renderer/renderer.rb +++ b/actionpack/lib/action_view/renderer/renderer.rb @@ -33,22 +33,12 @@ module ActionView # Direct accessor to template rendering. def render_template(context, options) #:nodoc: - _template_renderer.render(context, options) + TemplateRenderer.new(@lookup_context).render(context, options) end # Direct access to partial rendering. def render_partial(context, options, &block) #:nodoc: - _partial_renderer.render(context, options, block) - end - - private - - def _template_renderer #:nodoc: - TemplateRenderer.new(@lookup_context) - end - - def _partial_renderer #:nodoc: - PartialRenderer.new(@lookup_context) + PartialRenderer.new(@lookup_context).render(context, options, block) end end end diff --git a/actionpack/lib/action_view/renderer/template_renderer.rb b/actionpack/lib/action_view/renderer/template_renderer.rb index 2a5ea5a711..4d5c5db80c 100644 --- a/actionpack/lib/action_view/renderer/template_renderer.rb +++ b/actionpack/lib/action_view/renderer/template_renderer.rb @@ -41,7 +41,7 @@ module ActionView # Renders the given template. A string representing the layout can be # supplied as well. - def render_template(template, layout_name = nil, locals = {}) #:nodoc: + def render_template(template, layout_name = nil, locals = nil) #:nodoc: view, locals = @view, locals || {} render_with_layout(layout_name, locals) do |layout| diff --git a/actionpack/test/controller/flash_hash_test.rb b/actionpack/test/controller/flash_hash_test.rb index ccca0dac17..5490d9394b 100644 --- a/actionpack/test/controller/flash_hash_test.rb +++ b/actionpack/test/controller/flash_hash_test.rb @@ -46,6 +46,27 @@ module ActionDispatch assert_equal({'foo' => 'bar'}, @hash.to_hash) end + def test_to_session_value + @hash['foo'] = 'bar' + assert_equal({'flashes' => {'foo' => 'bar'}, 'discard' => []}, @hash.to_session_value) + + @hash.discard('foo') + assert_equal({'flashes' => {'foo' => 'bar'}, 'discard' => %w[foo]}, @hash.to_session_value) + + @hash.now['qux'] = 1 + assert_equal({'flashes' => {'foo' => 'bar', 'qux' => 1}, 'discard' => %w[foo qux]}, @hash.to_session_value) + + @hash.sweep + assert_equal(nil, @hash.to_session_value) + end + + def test_from_session_value + rails_3_2_cookie = 'BAh7B0kiD3Nlc3Npb25faWQGOgZFRkkiJWY4ZTFiODE1MmJhNzYwOWMyOGJiYjE3ZWM5MjYzYmE3BjsAVEkiCmZsYXNoBjsARm86JUFjdGlvbkRpc3BhdGNoOjpGbGFzaDo6Rmxhc2hIYXNoCToKQHVzZWRvOghTZXQGOgpAaGFzaHsAOgxAY2xvc2VkRjoNQGZsYXNoZXN7BkkiDG1lc3NhZ2UGOwBGSSIKSGVsbG8GOwBGOglAbm93MA==' + session = Marshal.load(Base64.decode64(rails_3_2_cookie)) + hash = Flash::FlashHash.from_session_value(session['flash']) + assert_equal({'flashes' => {'message' => 'Hello'}, 'discard' => %w[message]}, hash.to_session_value) + end + def test_empty? assert @hash.empty? @hash['zomg'] = 'bears' diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index e2964f9071..f2bacf3e20 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -3,7 +3,7 @@ require 'abstract_unit' class RequestTest < ActiveSupport::TestCase def url_for(options = {}) - options.reverse_merge!(:host => 'www.example.com') + options = { host: 'www.example.com' }.merge!(options) ActionDispatch::Http::URL.url_for(options) end @@ -25,6 +25,8 @@ class RequestTest < ActiveSupport::TestCase assert_equal 'http://www.example.com/', url_for(:trailing_slash => true) assert_equal 'http://dhh:supersecret@www.example.com', url_for(:user => 'dhh', :password => 'supersecret') assert_equal 'http://www.example.com?search=books', url_for(:params => { :search => 'books' }) + assert_equal 'http://www.example.com?params=', url_for(:params => '') + assert_equal 'http://www.example.com?params=1', url_for(:params => 1) end test "remote ip" do @@ -355,7 +357,6 @@ class RequestTest < ActiveSupport::TestCase assert_equal "/of/some/uri", request.path_info end - test "host with default port" do request = stub_request 'HTTP_HOST' => 'rubyonrails.org:80' assert_equal "rubyonrails.org", request.host_with_port @@ -577,16 +578,16 @@ class RequestTest < ActiveSupport::TestCase test "formats with accept header" do request = stub_request 'HTTP_ACCEPT' => 'text/html' request.expects(:parameters).at_least_once.returns({}) - assert_equal [ Mime::HTML ], request.formats + assert_equal [Mime::HTML], request.formats request = stub_request 'CONTENT_TYPE' => 'application/xml; charset=UTF-8', 'HTTP_X_REQUESTED_WITH' => "XMLHttpRequest" request.expects(:parameters).at_least_once.returns({}) - assert_equal with_set(Mime::XML), request.formats + assert_equal [Mime::XML], request.formats request = stub_request request.expects(:parameters).at_least_once.returns({ :format => :txt }) - assert_equal with_set(Mime::TEXT), request.formats + assert_equal [Mime::TEXT], request.formats request = stub_request request.expects(:parameters).at_least_once.returns({ :format => :unknown }) @@ -811,8 +812,4 @@ protected ActionDispatch::Http::URL.tld_length = tld_length ActionDispatch::Request.new(env) end - - def with_set(*args) - args - end end diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 34606512dc..0a59d3cf9e 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -370,6 +370,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest scope :path => 'api' do resource :me get '/' => 'mes#index' + scope :v2 do + resource :me, as: 'v2_me' + get '/' => 'mes#index' + end + + scope :v3, :admin do + resource :me, as: 'v3_me' + end end get "(/:username)/followers" => "followers#index" @@ -1467,6 +1475,18 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal 'mes#index', @response.body end + def test_symbol_scope + get '/api/v2/me' + assert_equal 'mes#show', @response.body + assert_equal '/api/v2/me', v2_me_path + + get '/api/v2' + assert_equal 'mes#index', @response.body + + get '/api/v3/admin/me' + assert_equal 'mes#show', @response.body + end + def test_url_generator_for_generic_route get 'whatever/foo/bar' assert_equal 'foo#bar', @response.body diff --git a/activemodel/test/cases/secure_password_test.rb b/activemodel/test/cases/secure_password_test.rb index c7e93370ec..7783bb25d5 100644 --- a/activemodel/test/cases/secure_password_test.rb +++ b/activemodel/test/cases/secure_password_test.rb @@ -5,13 +5,18 @@ require 'models/visitor' require 'models/administrator' class SecurePasswordTest < ActiveModel::TestCase - setup do + ActiveModel::SecurePassword.min_cost = true + @user = User.new @visitor = Visitor.new @oauthed_user = OauthedUser.new end + teardown do + ActiveModel::SecurePassword.min_cost = false + end + test "blank password" do @user.password = @visitor.password = '' assert !@user.valid?(:create), 'user should be invalid' @@ -70,13 +75,16 @@ class SecurePasswordTest < ActiveModel::TestCase end end - test "Password digest cost defaults to bcrypt default cost" do + test "Password digest cost defaults to bcrypt default cost when min_cost is false" do + ActiveModel::SecurePassword.min_cost = false + @user.password = "secret" assert_equal BCrypt::Engine::DEFAULT_COST, @user.password_digest.cost end test "Password digest cost can be set to bcrypt min cost to speed up tests" do ActiveModel::SecurePassword.min_cost = true + @user.password = "secret" assert_equal BCrypt::Engine::MIN_COST, @user.password_digest.cost end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 082e3d1089..201eddbcae 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,26 @@ ## Rails 4.0.0 (unreleased) ## +* Prevent mass assignment to the type column of polymorphic associations when using `build` + Fix #8265 + + *Yves Senn* + +* Deprecate calling `Relation#sum` with a block. To perform a calculation over + the array result of the relation, use `to_a.sum(&block)`. + + *Carlos Antonio da Silva* + +* Fix postgresql adapter to handle BC timestamps correctly + + HistoryEvent.create!(:name => "something", :occured_at => Date.new(0) - 5.years) + + *Bogdan Gusiev* + +* When running migrations on Postgresql, the `:limit` option for `binary` and `text` columns is silently dropped. + Previously, these migrations caused sql exceptions, because Postgresql doesn't support limits on these types. + + *Victor Costan* + * Don't change STI type when calling `ActiveRecord::Base#becomes`. Add `ActiveRecord::Base#becomes!` with the previous behavior. @@ -431,11 +452,11 @@ *kennyj* -* Use inversed parent for first and last child of has_many association. +* Use inversed parent for first and last child of `has_many` association. *Ravil Bayramgalin* -* Fix Column.microseconds and Column.fast_string_to_date to avoid converting +* Fix `Column.microseconds` and `Column.fast_string_to_time` to avoid converting timestamp seconds to a float, since it occasionally results in inaccuracies with microsecond-precision times. Fixes #7352. @@ -725,13 +746,6 @@ *Marc-André Lafortune* -* Allow blocks for `count` with `ActiveRecord::Relation`, to work similar as - `Array#count`: - - Person.where("age > 26").count { |person| person.gender == 'female' } - - *Chris Finne & Carlos Antonio da Silva* - * Added support to `CollectionAssociation#delete` for passing `fixnum` or `string` values as record ids. This finds the records responding to the `id` and executes delete on them. diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 651b17920c..d8b6d7a86b 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -234,7 +234,7 @@ module ActiveRecord # others.size | X | X | X # others.length | X | X | X # others.count | X | X | X - # others.sum(args*,&block) | X | X | X + # others.sum(*args) | X | X | X # others.empty? | X | X | X # others.clear | X | X | X # others.delete(other,other,...) | X | X | X diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 99e7383d42..3f0e4ca999 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -232,7 +232,8 @@ module ActiveRecord def build_record(attributes) reflection.build_association(attributes) do |record| - attributes = create_scope.except(*(record.changed - [reflection.foreign_key])) + skip_assign = [reflection.foreign_key, reflection.type].compact + attributes = create_scope.except(*(record.changed - skip_assign)) record.assign_attributes(attributes) end end diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 862ff201de..1548e68cea 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -161,15 +161,6 @@ module ActiveRecord end end - # Calculate sum using SQL, not Enumerable. - def sum(*args) - if block_given? - scope.sum(*args) { |*block_args| yield(*block_args) } - else - scope.sum(*args) - end - end - # Count all records using SQL. If the +:counter_sql+ or +:finder_sql+ option is set for the # association, it will be used for the query. Otherwise, construct options and pass them with # scope to the target class's +count+. diff --git a/activerecord/lib/active_record/attribute_methods/read.rb b/activerecord/lib/active_record/attribute_methods/read.rb index 90701938e5..3c03cce838 100644 --- a/activerecord/lib/active_record/attribute_methods/read.rb +++ b/activerecord/lib/active_record/attribute_methods/read.rb @@ -32,21 +32,29 @@ module ActiveRecord protected - # We want to generate the methods via module_eval rather than define_method, - # because define_method is slower on dispatch and uses more memory (because it - # creates a closure). + # We want to generate the methods via module_eval rather than + # define_method, because define_method is slower on dispatch and + # uses more memory (because it creates a closure). # - # But sometimes the database might return columns with characters that are not - # allowed in normal method names (like 'my_column(omg)'. So to work around this - # we first define with the __temp__ identifier, and then use alias method to - # rename it to what we want. - def define_method_attribute(attr_name) + # But sometimes the database might return columns with + # characters that are not allowed in normal method names (like + # 'my_column(omg)'. So to work around this we first define with + # the __temp__ identifier, and then use alias method to rename + # it to what we want. + # + # We are also defining a constant to hold the frozen string of + # the attribute name. Using a constant means that we do not have + # to allocate an object on each call to the attribute method. + # Making it frozen means that it doesn't get duped when used to + # key the @attributes_cache in read_attribute. + def define_method_attribute(name) + safe_name = name.unpack('h*').first generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1 - def __temp__ - read_attribute('#{attr_name}') { |n| missing_attribute(n, caller) } + def __temp__#{safe_name} + read_attribute(AttrNames::ATTR_#{safe_name}) { |n| missing_attribute(n, caller) } end - alias_method '#{attr_name}', :__temp__ - undef_method :__temp__ + alias_method #{name.inspect}, :__temp__#{safe_name} + undef_method :__temp__#{safe_name} STR end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index fa9097db1f..cd33494cc3 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -9,15 +9,19 @@ module ActiveRecord module ClassMethods protected - def define_method_attribute=(attr_name) - if attr_name =~ ActiveModel::AttributeMethods::NAME_COMPILABLE_REGEXP - generated_attribute_methods.module_eval("def #{attr_name}=(new_value); write_attribute('#{attr_name}', new_value); end", __FILE__, __LINE__) - else - generated_attribute_methods.send(:define_method, "#{attr_name}=") do |new_value| - write_attribute(attr_name, new_value) - end + + # See define_method_attribute in read.rb for an explanation of + # this code. + def define_method_attribute=(name) + safe_name = name.unpack('h*').first + generated_attribute_methods.module_eval <<-STR, __FILE__, __LINE__ + 1 + def __temp__#{safe_name}=(value) + write_attribute(AttrNames::ATTR_#{safe_name}, value) end - end + alias_method #{(name + '=').inspect}, :__temp__#{safe_name}= + undef_method :__temp__#{safe_name}= + STR + end end # Updates the attribute identified by <tt>attr_name</tt> with the diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 17dd71e898..f1e42dfbbe 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -540,7 +540,7 @@ module ActiveRecord column_type_sql << "(#{precision})" end elsif scale - raise ArgumentError, "Error adding decimal column: precision cannot be empty if scale if specified" + raise ArgumentError, "Error adding decimal column: precision cannot be empty if scale is specified" end elsif (type != :primary_key) && (limit ||= native.is_a?(Hash) && native[:limit]) diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index 9627aaae3a..e9677415cc 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -2,7 +2,7 @@ require 'active_record/connection_adapters/abstract_mysql_adapter' require 'active_record/connection_adapters/statement_pool' require 'active_support/core_ext/hash/keys' -gem 'mysql', '~> 2.9.0' +gem 'mysql', '~> 2.9' require 'mysql' class Mysql diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb index 62d091357d..c04a799b8d 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/cast.rb @@ -8,6 +8,8 @@ module ActiveRecord case string when 'infinity'; 1.0 / 0.0 when '-infinity'; -1.0 / 0.0 + when / BC$/ + super("-" + string.sub(/ BC$/, "")) else super end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb index 9d3fa18e3a..62a4d76928 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb @@ -129,11 +129,15 @@ module ActiveRecord # Quote date/time values for use in SQL input. Includes microseconds # if the value is a Time responding to usec. def quoted_date(value) #:nodoc: + result = super if value.acts_like?(:time) && value.respond_to?(:usec) - "#{super}.#{sprintf("%06d", value.usec)}" - else - super + result = "#{result}.#{sprintf("%06d", value.usec)}" + end + + if value.year < 0 + result = result.sub(/^-/, "") + " BC" end + result end end end 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 82a0b662f4..7c561b6f82 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -396,6 +396,13 @@ module ActiveRecord when nil, 0..0x3fffffff; super(type) else raise(ActiveRecordError, "No binary type has byte size #{limit}.") end + when 'text' + # PostgreSQL doesn't support limits on text columns. + # The hard limit is 1Gb, according to section 8.3 in the manual. + case limit + when nil, 0..0x3fffffff; super(type) + else raise(ActiveRecordError, "The limit on text can be at most 1GB - 1byte.") + end when 'integer' return 'integer' unless limit @@ -422,20 +429,17 @@ module ActiveRecord # PostgreSQL requires the ORDER BY columns in the select list for distinct queries, and # requires that the ORDER BY include the distinct column. # - # distinct("posts.id", "posts.created_at desc") + # distinct("posts.id", ["posts.created_at desc"]) + # # => "DISTINCT posts.id, posts.created_at AS alias_0" def distinct(columns, orders) #:nodoc: - return "DISTINCT #{columns}" if orders.empty? - - # Construct a clean list of column names from the ORDER BY clause, removing - # any ASC/DESC modifiers - order_columns = orders.collect do |s| - s = s.to_sql unless s.is_a?(String) - s.gsub(/\s+(ASC|DESC)\s*(NULLS\s+(FIRST|LAST)\s*)?/i, '') - end - order_columns.delete_if { |c| c.blank? } - order_columns = order_columns.zip((0...order_columns.size).to_a).map { |s,i| "#{s} AS alias_#{i}" } - - "DISTINCT #{columns}, #{order_columns * ', '}" + order_columns = orders.map{ |s| + # Convert Arel node to string + s = s.to_sql unless s.is_a?(String) + # Remove any ASC/DESC modifiers + s.gsub(/\s+(ASC|DESC)\s*(NULLS\s+(FIRST|LAST)\s*)?/i, '') + }.reject(&:blank?).map.with_index { |column, i| "#{column} AS alias_#{i}" } + + [super].concat(order_columns).join(', ') end end end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 957027c1ee..94c6684700 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -83,7 +83,12 @@ module ActiveRecord @attribute_methods_mutex = Mutex.new # force attribute methods to be higher in inheritance hierarchy than other generated methods - generated_attribute_methods + generated_attribute_methods.const_set(:AttrNames, Module.new { + def self.const_missing(name) + const_set(name, [name.to_s.sub(/ATTR_/, '')].pack('h*').freeze) + end + }) + generated_feature_methods end diff --git a/activerecord/lib/active_record/migration/join_table.rb b/activerecord/lib/active_record/migration/join_table.rb index e880ae97bb..ebf64cbcdc 100644 --- a/activerecord/lib/active_record/migration/join_table.rb +++ b/activerecord/lib/active_record/migration/join_table.rb @@ -8,7 +8,7 @@ module ActiveRecord end def join_table_name(table_1, table_2) - [table_1, table_2].sort.join("_").to_sym + [table_1.to_s, table_2.to_s].sort.join("_").to_sym end end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 1c17c578e9..eed49e17b1 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -180,7 +180,7 @@ module ActiveRecord name = name.to_s verify_readonly_attribute(name) send("#{name}=", value) - save(:validate => false) + save(validate: false) end # Updates the attributes of the model from the passed-in hash and saves the @@ -235,8 +235,8 @@ module ActiveRecord updated_count = self.class.where(self.class.primary_key => id).update_all(attributes) - attributes.each do |k,v| - raw_write_attribute(k,v) + attributes.each do |k, v| + raw_write_attribute(k, v) end updated_count == 1 @@ -388,10 +388,14 @@ module ActiveRecord # Returns the number of affected rows. def update(attribute_names = @attributes.keys) attributes_with_values = arel_attributes_with_values_for_update(attribute_names) - return 0 if attributes_with_values.empty? - klass = self.class - stmt = klass.unscoped.where(klass.arel_table[klass.primary_key].eq(id)).arel.compile_update(attributes_with_values) - klass.connection.update stmt + + if attributes_with_values.empty? + 0 + else + klass = self.class + stmt = klass.unscoped.where(klass.arel_table[klass.primary_key].eq(id)).arel.compile_update(attributes_with_values) + klass.connection.update stmt + end end # Creates a record with values matching those of the instance attributes diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 3ee55c580e..f0f170b684 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -31,6 +31,14 @@ module ActiveRecord @default_scoped = false end + def initialize_copy(other) + # This method is a hot spot, so for now, use Hash[] to dup the hash. + # https://bugs.ruby-lang.org/issues/7166 + @values = Hash[@values] + @values[:bind] = @values[:bind].dup if @values.key? :bind + reset + end + def insert(values) primary_key_value = nil @@ -90,14 +98,6 @@ module ActiveRecord scoping { @klass.new(*args, &block) } end - def initialize_copy(other) - # This method is a hot spot, so for now, use Hash[] to dup the hash. - # https://bugs.ruby-lang.org/issues/7166 - @values = Hash[@values] - @values[:bind] = @values[:bind].dup if @values.key? :bind - reset - end - alias build new # Tries to create a new record with the same scoped attributes diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 741f94f777..72b035a023 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -1,3 +1,5 @@ +require 'active_support/deprecation' + module ActiveRecord module Calculations # Count the records. @@ -13,16 +15,9 @@ module ActiveRecord # # Person.count(:age, distinct: true) # # => counts the number of different age values - # - # Person.where("age > 26").count { |person| person.gender == 'female' } - # # => queries people where "age > 26" then count the loaded results filtering by gender def count(column_name = nil, options = {}) - if block_given? - self.to_a.count { |item| yield item } - else - column_name, options = nil, column_name if column_name.is_a?(Hash) - calculate(:count, column_name, options) - end + column_name, options = nil, column_name if column_name.is_a?(Hash) + calculate(:count, column_name, options) end # Calculates the average value on a given column. Returns +nil+ if there's @@ -56,13 +51,13 @@ module ActiveRecord # +calculate+ for examples with options. # # Person.sum('age') # => 4562 - # # => returns the total sum of all people's age - # - # Person.where('age > 100').sum { |person| person.age - 100 } - # # queries people where "age > 100" then perform a sum calculation with the block returns def sum(*args) if block_given? - self.to_a.sum(*args) { |item| yield item } + ActiveSupport::Deprecation.warn( + "Calling #sum with a block is deprecated and will be removed in Rails 4.1. " \ + "If you want to perform sum calculation over the array of elements, use `to_a.sum(&block)`." + ) + self.to_a.sum(*args) {|*block_args| yield(*block_args)} else calculate(:sum, *args) end diff --git a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb index f1362dd15f..872204c644 100644 --- a/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb +++ b/activerecord/test/cases/adapters/postgresql/postgresql_adapter_test.rb @@ -216,6 +216,35 @@ module ActiveRecord assert_equal "(number > 100)", index.where end + def test_distinct_zero_orders + assert_equal "DISTINCT posts.id", + @connection.distinct("posts.id", []) + end + + def test_distinct_one_order + assert_equal "DISTINCT posts.id, posts.created_at AS alias_0", + @connection.distinct("posts.id", ["posts.created_at desc"]) + end + + def test_distinct_few_orders + assert_equal "DISTINCT posts.id, posts.created_at AS alias_0, posts.position AS alias_1", + @connection.distinct("posts.id", ["posts.created_at desc", "posts.position asc"]) + end + + def test_distinct_blank_not_nil_orders + assert_equal "DISTINCT posts.id, posts.created_at AS alias_0", + @connection.distinct("posts.id", ["posts.created_at desc", "", " "]) + end + + def test_distinct_with_arel_order + order = Object.new + def order.to_sql + "posts.created_at desc" + end + assert_equal "DISTINCT posts.id, posts.created_at AS alias_0", + @connection.distinct("posts.id", [order]) + end + def test_distinct_with_nulls assert_equal "DISTINCT posts.title, posts.updater_id AS alias_0", @connection.distinct("posts.title", ["posts.updater_id desc nulls first"]) assert_equal "DISTINCT posts.title, posts.updater_id AS alias_0", @connection.distinct("posts.title", ["posts.updater_id desc nulls last"]) diff --git a/activerecord/test/cases/adapters/postgresql/sql_types_test.rb b/activerecord/test/cases/adapters/postgresql/sql_types_test.rb new file mode 100644 index 0000000000..d7d40f6385 --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/sql_types_test.rb @@ -0,0 +1,18 @@ +require "cases/helper" + +class SqlTypesTest < ActiveRecord::TestCase + def test_binary_types + assert_equal 'bytea', type_to_sql(:binary, 100_000) + assert_raise ActiveRecord::ActiveRecordError do + type_to_sql :binary, 4294967295 + end + assert_equal 'text', type_to_sql(:text, 100_000) + assert_raise ActiveRecord::ActiveRecordError do + type_to_sql :text, 4294967295 + end + end + + def type_to_sql(*args) + ActiveRecord::Base.connection.type_to_sql(*args) + end +end diff --git a/activerecord/test/cases/adapters/postgresql/timestamp_test.rb b/activerecord/test/cases/adapters/postgresql/timestamp_test.rb index 26507ad654..630bdeec67 100644 --- a/activerecord/test/cases/adapters/postgresql/timestamp_test.rb +++ b/activerecord/test/cases/adapters/postgresql/timestamp_test.rb @@ -75,6 +75,15 @@ class TimestampTest < ActiveRecord::TestCase assert_equal '4', pg_datetime_precision('foos', 'updated_at') end + def test_bc_timestamp + unless current_adapter?(:PostgreSQLAdapter) + return skip("only tested on postgresql") + end + date = Date.new(0) - 1.second + Developer.create!(:name => "aaron", :updated_at => date) + assert_equal date, Developer.find_by_name("aaron").updated_at + end + private def pg_datetime_precision(table_name, column_name) diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 01afa087be..2ded97582d 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1579,6 +1579,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [tagging], post.taggings end + def test_build_with_polymotphic_has_many_does_not_allow_to_override_type_and_id + welcome = posts(:welcome) + tagging = welcome.taggings.build(:taggable_id => 99, :taggable_type => 'ShouldNotChange') + + assert_equal welcome.id, tagging.taggable_id + assert_equal 'Post', tagging.taggable_type + end + def test_dont_call_save_callbacks_twice_on_has_many firm = companies(:first_firm) contract = firm.contracts.create! @@ -1658,6 +1666,12 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_deprecated { klass.has_many :foo, :counter_sql => 'lol' } end + test "sum calculation with block for array compatibility is deprecated" do + assert_deprecated do + posts(:welcome).comments.sum { |c| c.id } + end + end + test "has many associations on new records use null relations" do post = Post.new diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 65d28ea028..5cb7eabf0e 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -383,30 +383,16 @@ class CalculationsTest < ActiveRecord::TestCase Company.where(:type => "Firm").from('companies').count(:type) end - def test_count_with_block_acts_as_array - accounts = Account.where('id > 0') - assert_equal Account.count, accounts.count { true } - assert_equal 0, accounts.count { false } - assert_equal Account.where('credit_limit > 50').size, accounts.count { |account| account.credit_limit > 50 } - assert_equal Account.count, Account.count { true } - assert_equal 0, Account.count { false } - end - - def test_sum_with_block_acts_as_array - accounts = Account.where('id > 0') - assert_equal Account.sum(:credit_limit), accounts.sum { |account| account.credit_limit } - assert_equal Account.sum(:credit_limit) + Account.count, accounts.sum{ |account| account.credit_limit + 1 } - assert_equal 0, accounts.sum { |account| 0 } - end - def test_sum_with_from_option assert_equal Account.sum(:credit_limit), Account.from('accounts').sum(:credit_limit) assert_equal Account.where("credit_limit > 50").sum(:credit_limit), Account.where("credit_limit > 50").from('accounts').sum(:credit_limit) end - def test_sum_array_compatibility - assert_equal Account.sum(:credit_limit), Account.sum(&:credit_limit) + def test_sum_array_compatibility_deprecation + assert_deprecated do + assert_equal Account.sum(:credit_limit), Account.sum(&:credit_limit) + end end def test_average_with_from_option diff --git a/activerecord/test/cases/migration/create_join_table_test.rb b/activerecord/test/cases/migration/create_join_table_test.rb index cd1b0e8b47..f262bbaad7 100644 --- a/activerecord/test/cases/migration/create_join_table_test.rb +++ b/activerecord/test/cases/migration/create_join_table_test.rb @@ -35,6 +35,12 @@ module ActiveRecord assert_equal %w(artist_id music_id), connection.columns(:artists_musics).map(&:name).sort end + def test_create_join_table_with_symbol_and_string + connection.create_join_table :artists, 'musics' + + assert_equal %w(artist_id music_id), connection.columns(:artists_musics).map(&:name).sort + end + def test_create_join_table_with_the_proper_order connection.create_join_table :videos, :musics diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index d0e7338f15..0cfde83778 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -192,5 +192,10 @@ end _SQL rescue #This version of PostgreSQL either has no XML support or is was not compiled with XML support: skipping table end + + create_table :limitless_fields, force: true do |t| + t.binary :binary, limit: 100_000 + t.text :text, limit: 100_000 + end end diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 79d00ded0a..32c139df99 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -985,7 +985,7 @@ SELECT categories.* FROM categories ### Specifying Conditions on the Joined Tables -You can specify conditions on the joined tables using the regular [Array](array-conditions) and [String](#pure-string-conditions) conditions. [Hash conditions](#hash-conditions) provides a special syntax for specifying conditions for the joined tables: +You can specify conditions on the joined tables using the regular [Array](#array-conditions) and [String](#pure-string-conditions) conditions. [Hash conditions](#hash-conditions) provides a special syntax for specifying conditions for the joined tables: ```ruby time_range = (Time.now.midnight - 1.day)..Time.now.midnight diff --git a/guides/source/active_record_validations_callbacks.md b/guides/source/active_record_validations_callbacks.md index 5c27ccbf9e..0f4140b650 100644 --- a/guides/source/active_record_validations_callbacks.md +++ b/guides/source/active_record_validations_callbacks.md @@ -953,8 +953,12 @@ Below is a simple example where we change the Rails behavior to always display t ```ruby ActionView::Base.field_error_proc = Proc.new do |html_tag, instance| - errors = Array(instance.error_message).join(',') - %(#{html_tag}<span class="validation-error"> #{errors}</span>).html_safe + if html_tag =~ /\<label/ + html_tag + else + errors = Array(instance.error_message).join(',') + %(#{html_tag}<span class="validation-error"> #{errors}</span>).html_safe + end end ``` diff --git a/guides/source/caching_with_rails.md b/guides/source/caching_with_rails.md index 4cb76bfe5f..826ddd264a 100644 --- a/guides/source/caching_with_rails.md +++ b/guides/source/caching_with_rails.md @@ -465,14 +465,14 @@ end Instead of a options hash, you can also simply pass in a model, Rails will use the `updated_at` and `cache_key` methods for setting `last_modified` and `etag`: -<ruby> +```ruby class ProductsController < ApplicationController def show @product = Product.find(params[:id]) respond_with(@product) if stale?(@product) end end -</ruby> +``` If you don't have any special response processing and are using the default rendering mechanism (i.e. you're not using respond_to or calling render yourself) then you’ve got an easy helper in fresh_when: diff --git a/guides/source/engines.md b/guides/source/engines.md index f9bbff1c4c..d8f5796dde 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -699,7 +699,7 @@ This section explains how to add and/or override engine MVC functionality in the ### Overriding Models and Controllers -Engine model and controller classes can be extended by open classing them in the main Rails application (since model and controller classes are just Ruby classes that inherit Rails specific functionality). Open classing an Engine class redefines it for use in the main applicaiton. This is usually implemented by using the decorator pattern. +Engine model and controller classes can be extended by open classing them in the main Rails application (since model and controller classes are just Ruby classes that inherit Rails specific functionality). Open classing an Engine class redefines it for use in the main application. This is usually implemented by using the decorator pattern. For simple class modifications use `Class#class_eval`, and for complex class modifications, consider using `ActiveSupport::Concern`. diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index f97e66985c..89afeaeec5 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -105,6 +105,10 @@ module Rails def database_configuration require 'erb' YAML.load ERB.new(IO.read(paths["config/database"].first)).result + rescue Psych::SyntaxError => e + raise "YAML syntax error occurred while parsing #{paths["config/database"].first}. " \ + "Please note that YAML must be consistently indented using spaces. Tabs are not allowed. " \ + "Error: #{e.message}" end def log_level diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 2c2bb1c714..f6721c617f 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -407,6 +407,8 @@ module Rails end end + self.isolated = false + delegate :middleware, :root, :paths, to: :config delegate :engine_name, :isolated?, to: "self.class" |