diff options
| -rw-r--r-- | Gemfile | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_controller/test_case.rb | 2 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/middleware/exception_wrapper.rb | 10 | ||||
| -rw-r--r-- | actionpack/lib/action_dispatch/middleware/flash.rb | 40 | ||||
| -rw-r--r-- | actionpack/test/controller/flash_hash_test.rb | 25 | ||||
| -rw-r--r-- | actionpack/test/controller/test_case_test.rb | 12 | ||||
| -rw-r--r-- | actionpack/test/dispatch/exception_wrapper_test.rb | 17 | ||||
| -rw-r--r-- | activemodel/CHANGELOG.md | 5 | ||||
| -rw-r--r-- | activemodel/lib/active_model/locale/en.yml | 3 | ||||
| -rw-r--r-- | activerecord/CHANGELOG.md | 9 | ||||
| -rw-r--r-- | activerecord/lib/active_record/associations/join_dependency.rb | 7 | ||||
| -rw-r--r-- | activerecord/lib/active_record/locale/en.yml | 1 | ||||
| -rw-r--r-- | activerecord/test/cases/associations/eager_test.rb | 8 |
13 files changed, 102 insertions, 39 deletions
@@ -42,7 +42,7 @@ group :job do gem 'sidekiq', require: false gem 'sucker_punch', require: false gem 'delayed_job', require: false - gem 'queue_classic', "< 3.0.0", require: false, platforms: :ruby + gem 'queue_classic', require: false, platforms: :ruby gem 'sneakers', '0.1.1.pre', require: false gem 'que', require: false gem 'backburner', require: false diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index afe95e3fec..b05700aace 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -677,6 +677,8 @@ module ActionController if flash_value = @request.flash.to_session_value @request.session['flash'] = flash_value + else + @request.session.delete('flash') end @response diff --git a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb index a4862e33aa..5595a73887 100644 --- a/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb +++ b/actionpack/lib/action_dispatch/middleware/exception_wrapper.rb @@ -87,8 +87,7 @@ module ActionDispatch def source_extracts backtrace.map do |trace| - file, line = trace.split(":") - line_number = line.to_i + file, line_number = extract_file_and_line_number(trace) { code: source_fragment(file, line_number), @@ -139,6 +138,13 @@ module ActionDispatch end end + def extract_file_and_line_number(trace) + # Split by the first colon followed by some digits, which works for both + # Windows and Unix path styles. + file, line = trace.match(/^(.+?):(\d+).*$/, &:captures) || trace + [file, line.to_i] + end + def expand_backtrace @exception.backtrace.unshift( @exception.to_s.split("\n") diff --git a/actionpack/lib/action_dispatch/middleware/flash.rb b/actionpack/lib/action_dispatch/middleware/flash.rb index a7f95150a4..59639a010e 100644 --- a/actionpack/lib/action_dispatch/middleware/flash.rb +++ b/actionpack/lib/action_dispatch/middleware/flash.rb @@ -80,24 +80,30 @@ module ActionDispatch include Enumerable def self.from_session_value(value) #:nodoc: - 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 - - # Builds a hash containing the discarded values and the hashes - # representing the flashes. - # If there are no values in @flashes, returns nil. + case value + when FlashHash # Rails 3.1, 3.2 + flashes = value.instance_variable_get(:@flashes) + if discard = value.instance_variable_get(:@used) + flashes.except!(*discard) + end + new(flashes, flashes.keys) + when Hash # Rails 4.0 + flashes = value['flashes'] + if discard = value['discard'] + flashes.except!(*discard) + end + new(flashes, flashes.keys) + else + new + end + end + + # Builds a hash containing the flashes to keep for the next request. + # If there are none to keep, returns nil. def to_session_value #:nodoc: - return nil if empty? - {'discard' => @discard.to_a, 'flashes' => @flashes} + flashes_to_keep = @flashes.except(*@discard) + return nil if flashes_to_keep.empty? + {'flashes' => flashes_to_keep} end def initialize(flashes = {}, discard = []) #:nodoc: diff --git a/actionpack/test/controller/flash_hash_test.rb b/actionpack/test/controller/flash_hash_test.rb index d979b561f2..081288ef21 100644 --- a/actionpack/test/controller/flash_hash_test.rb +++ b/actionpack/test/controller/flash_hash_test.rb @@ -57,33 +57,36 @@ module ActionDispatch 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) + assert_equal({'flashes' => {'foo' => 'bar'}}, @hash.to_session_value) @hash.now['qux'] = 1 - assert_equal({'flashes' => {'foo' => 'bar', 'qux' => 1}, 'discard' => %w[foo qux]}, @hash.to_session_value) + assert_equal({'flashes' => {'foo' => 'bar'}}, @hash.to_session_value) + + @hash.discard('foo') + assert_equal(nil, @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_id"=>"f8e1b8152ba7609c28bbb17ec9263ba7", "flash"=>#<ActionDispatch::Flash::FlashHash:0x00000000000000 @used=#<Set: {"farewell"}>, @closed=false, @flashes={"greeting"=>"Hello", "farewell"=>"Goodbye"}, @now=nil>} + rails_3_2_cookie = 'BAh7B0kiD3Nlc3Npb25faWQGOgZFRkkiJWY4ZTFiODE1MmJhNzYwOWMyOGJiYjE3ZWM5MjYzYmE3BjsAVEkiCmZsYXNoBjsARm86JUFjdGlvbkRpc3BhdGNoOjpGbGFzaDo6Rmxhc2hIYXNoCToKQHVzZWRvOghTZXQGOgpAaGFzaHsGSSINZmFyZXdlbGwGOwBUVDoMQGNsb3NlZEY6DUBmbGFzaGVzewdJIg1ncmVldGluZwY7AFRJIgpIZWxsbwY7AFRJIg1mYXJld2VsbAY7AFRJIgxHb29kYnllBjsAVDoJQG5vdzA=' 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) + assert_equal({'greeting' => 'Hello'}, hash.to_hash) + assert_equal(nil, hash.to_session_value) end def test_from_session_value_on_json_serializer - decrypted_data = "{ \"session_id\":\"d98bdf6d129618fc2548c354c161cfb5\", \"flash\":{\"discard\":[], \"flashes\":{\"message\":\"hey you\"}} }" + decrypted_data = "{ \"session_id\":\"d98bdf6d129618fc2548c354c161cfb5\", \"flash\":{\"discard\":[\"farewell\"], \"flashes\":{\"greeting\":\"Hello\",\"farewell\":\"Goodbye\"}} }" session = ActionDispatch::Cookies::JsonSerializer.load(decrypted_data) hash = Flash::FlashHash.from_session_value(session['flash']) - assert_equal({'discard' => %w[message], 'flashes' => { 'message' => 'hey you'}}, hash.to_session_value) - assert_equal "hey you", hash[:message] - assert_equal "hey you", hash["message"] + assert_equal({'greeting' => 'Hello'}, hash.to_hash) + assert_equal(nil, hash.to_session_value) + assert_equal "Hello", hash[:greeting] + assert_equal "Hello", hash["greeting"] end def test_empty? diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 901db63e39..25337a7dd4 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -14,6 +14,11 @@ class TestCaseTest < ActionController::TestCase render text: 'ignore me' end + def delete_flash + flash.delete("test") + render :text => 'ignore me' + end + def set_flash_now flash.now["test_now"] = ">#{flash["test_now"]}<" render text: 'ignore me' @@ -290,6 +295,13 @@ XML assert_equal '>value_now<', flash['test_now'] end + def test_process_delete_flash + process :set_flash + process :delete_flash + assert_empty flash + assert_empty session + end + def test_process_with_session process :set_session assert_equal 'A wonder', session['string'], "A value stored in the session should be available by string key" diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb index d7408164ba..7a29a7ff97 100644 --- a/actionpack/test/dispatch/exception_wrapper_test.rb +++ b/actionpack/test/dispatch/exception_wrapper_test.rb @@ -34,6 +34,23 @@ module ActionDispatch assert_equal [ code: 'foo', line_number: 42 ], wrapper.source_extracts end + test '#source_extracts works with Windows paths' do + exc = TestError.new("c:/path/to/rails/app/controller.rb:27:in 'index':") + + wrapper = ExceptionWrapper.new({}, exc) + wrapper.expects(:source_fragment).with('c:/path/to/rails/app/controller.rb', 27).returns('nothing') + + assert_equal [ code: 'nothing', line_number: 27 ], wrapper.source_extracts + end + + test '#source_extracts works with non standard backtrace' do + exc = TestError.new('invalid') + + wrapper = ExceptionWrapper.new({}, exc) + wrapper.expects(:source_fragment).with('invalid', 0).returns('nothing') + + assert_equal [ code: 'nothing', line_number: 0 ], wrapper.source_extracts + end test '#application_trace returns traces only from the application' do exception = TestError.new(caller.prepend("lib/file.rb:42:in `index'")) diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 4c78fc91a5..5830a00bc5 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,8 +1,3 @@ -* Change the default error message from `can't be blank` to `must exist` for - the presence validator of the `:required` option on `belongs_to`/`has_one` associations. - - *Henrik Nygren* - * Assigning an unknown attribute key to an `ActiveModel` instance during initialization will now raise `ActiveModel::AttributeAssignment::UnknownAttributeError` instead of `NoMethodError` diff --git a/activemodel/lib/active_model/locale/en.yml b/activemodel/lib/active_model/locale/en.yml index b0b20ee0bf..b11c8f53b4 100644 --- a/activemodel/lib/active_model/locale/en.yml +++ b/activemodel/lib/active_model/locale/en.yml @@ -14,10 +14,9 @@ en: empty: "can't be empty" blank: "can't be blank" present: "must be blank" - required: "must exist" too_long: one: "is too long (maximum is 1 character)" - other: "is too long (maximum is %{count} characters)" + other: "is too long (maximum is %{count} characters)" too_short: one: "is too short (minimum is 1 character)" other: "is too short (minimum is %{count} characters)" diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 4cc62346cb..a17148f1f7 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,12 @@ +* Fix n+1 query problem when eager loading nil associations (fixes #18312) + + *Sammy Larbi* + +* Change the default error message from `can't be blank` to `must exist` for + the presence validator of the `:required` option on `belongs_to`/`has_one` associations. + + *Henrik Nygren* + * Fixed ActiveRecord::Relation#group method when argument is SQL reserved key word: Example: diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 4b75370171..fcf06323e6 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -232,6 +232,7 @@ module ActiveRecord end def construct(ar_parent, parent, row, rs, seen, model_cache, aliases) + return if ar_parent.nil? primary_id = ar_parent.id parent.children.each do |node| @@ -248,7 +249,11 @@ module ActiveRecord key = aliases.column_alias(node, node.primary_key) id = row[key] - next if id.nil? + if id.nil? + nil_association = ar_parent.association(node.reflection.name) + nil_association.loaded! + next + end model = seen[parent.base_klass][primary_id][node.base_klass][id] diff --git a/activerecord/lib/active_record/locale/en.yml b/activerecord/lib/active_record/locale/en.yml index b1fbd38622..8a3c27e6da 100644 --- a/activerecord/lib/active_record/locale/en.yml +++ b/activerecord/lib/active_record/locale/en.yml @@ -7,6 +7,7 @@ en: # Default error messages errors: messages: + required: "must exist" taken: "has already been taken" # Active Record models configuration diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 371635d20a..7d8b933992 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -77,9 +77,17 @@ class EagerAssociationTest < ActiveRecord::TestCase def test_has_many_through_with_order authors = Author.includes(:favorite_authors).to_a + assert authors.count > 0 assert_no_queries { authors.map(&:favorite_authors) } end + def test_eager_loaded_has_one_association_with_references_does_not_run_additional_queries + Post.update_all(author_id: nil) + authors = Author.includes(:post).references(:post).to_a + assert authors.count > 0 + assert_no_queries { authors.map(&:post) } + end + def test_with_two_tables_in_from_without_getting_double_quoted posts = Post.select("posts.*").from("authors, posts").eager_load(:comments).where("posts.author_id = authors.id").order("posts.id").to_a assert_equal 2, posts.first.comments.size |