From c2afa055614f15edfbd2f4c97f9254425286fc6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Britto?= Date: Mon, 16 Dec 2013 00:51:35 -0200 Subject: Improve ERB dependency detection. The current implementation can't handle some special cases of oddly-formatted Ruby. Now we are able to detect them: * Multi-line arguments on the `render` call * Strings containing quotes, e.g. `"something's wrong"` * Multiple kinds of identifiers - instance variables, class variables and globals * Method chains as arguments for the `render` call Also, this fix reduces the rate of "false positives" which showed up when we had calls/access to identifiers containing `render`, like `surrender` and `rendering`. --- actionview/lib/action_view/dependency_tracker.rb | 74 +++++++++++++---- .../test/template/dependency_tracker_test.rb | 92 +++++++++++++++++++++- 2 files changed, 145 insertions(+), 21 deletions(-) diff --git a/actionview/lib/action_view/dependency_tracker.rb b/actionview/lib/action_view/dependency_tracker.rb index b2e8334077..748fa81221 100644 --- a/actionview/lib/action_view/dependency_tracker.rb +++ b/actionview/lib/action_view/dependency_tracker.rb @@ -1,7 +1,7 @@ require 'thread_safe' module ActionView - class DependencyTracker + class DependencyTracker # :nodoc: @trackers = ThreadSafe::Cache.new def self.find_dependencies(name, template) @@ -23,9 +23,36 @@ module ActionView @trackers.delete(handler) end - class ERBTracker + class ERBTracker # :nodoc: EXPLICIT_DEPENDENCY = /# Template Dependency: (\S+)/ + # A valid ruby identifier - suitable for class, method and specially variable names + IDENTIFIER = / + [[:alpha:]_] # at least one uppercase letter, lowercase letter or underscore + [[:word:]]* # followed by optional letters, numbers or underscores + /x + + # Any kind of variable name. e.g. @instance, @@class, $global or local. + # Possibly following a method call chain + VARIABLE_OR_METHOD_CHAIN = / + (?:\$|@{1,2})? # optional global, instance or class variable indicator + (?:#{IDENTIFIER}\.)* # followed by an optional chain of zero-argument method calls + (?#{IDENTIFIER}) # and a final valid identifier, captured as DYNAMIC + /x + + # A simple string literal. e.g. "School's out!" + STRING = / + (?['"]) # an opening quote + (?.*?) # with anything inside, captured as STATIC + \k # and a matching closing quote + /x + + # Part of any hash containing the :partial key + PARTIAL_HASH_KEY = / + (?:\bpartial:|:partial\s*=>) # partial key in either old or new style hash syntax + \s* # followed by optional spaces + /x + # Matches: # render partial: "comments/comment", collection: commentable.comments # render "comments/comments" @@ -36,11 +63,13 @@ module ActionView # render(topics) => render("topics/topic") # render(message.topics) => render("topics/topic") RENDER_DEPENDENCY = / - render\s* # render, followed by optional whitespace - \(? # start an optional parenthesis for the render call - (partial:|:partial\s+=>)?\s* # naming the partial, used with collection -- 1st capture - ([@a-z"'][@\w\/\."']+) # the template name itself -- 2nd capture - /x + \brender\b # render, the whole word + \s*\(?\s* # optional opening paren surrounded by spaces + (?: + (?:.*?#{PARTIAL_HASH_KEY})? # optional hash, up to the partial key declaration + (?:#{STRING}|#{VARIABLE_OR_METHOD_CHAIN}) # finally, the dependency name of interest + ) + /xm def self.call(name, template) new(name, template).dependencies @@ -68,19 +97,30 @@ module ActionView end def render_dependencies - source.scan(RENDER_DEPENDENCY). - collect(&:second).uniq. + render_dependencies = [] - # render(@topic) => render("topics/topic") - # render(topics) => render("topics/topic") - # render(message.topics) => render("topics/topic") - collect { |name| name.sub(/\A@?([a-z_]+\.)*([a-z_]+)\z/) { "#{$2.pluralize}/#{$2.singularize}" } }. + source.scan(RENDER_DEPENDENCY) do + add_dynamic_dependency(render_dependencies, Regexp.last_match[:dynamic]) + add_static_dependency(render_dependencies, Regexp.last_match[:static]) + end - # render("headline") => render("message/headline") - collect { |name| name.include?("/") ? name : "#{directory}/#{name}" }. + render_dependencies.uniq + end + + def add_dynamic_dependency(dependencies, dependency) + if dependency + dependencies << "#{dependency.pluralize}/#{dependency.singularize}" + end + end - # replace quotes from string renders - collect { |name| name.gsub(/["']/, "") } + def add_static_dependency(dependencies, dependency) + if dependency + if dependency.include?('/') + dependencies << dependency + else + dependencies << "#{directory}/#{dependency}" + end + end end def explicit_dependencies diff --git a/actionview/test/template/dependency_tracker_test.rb b/actionview/test/template/dependency_tracker_test.rb index 7a9b4b26ac..6540394c14 100644 --- a/actionview/test/template/dependency_tracker_test.rb +++ b/actionview/test/template/dependency_tracker_test.rb @@ -1,3 +1,5 @@ +# encoding: utf-8 + require 'abstract_unit' require 'action_view/dependency_tracker' @@ -52,23 +54,105 @@ class ERBTrackerTest < Minitest::Test def test_dependency_of_erb_template_with_number_in_filename template = FakeTemplate.new("<%# render 'messages/message123' %>", :erb) - tracker = make_tracker('messages/_message123', template) + tracker = make_tracker("messages/_message123", template) assert_equal ["messages/message123"], tracker.dependencies end def test_finds_dependency_in_correct_directory template = FakeTemplate.new("<%# render(message.topic) %>", :erb) - tracker = make_tracker('messages/_message', template) + tracker = make_tracker("messages/_message", template) assert_equal ["topics/topic"], tracker.dependencies end def test_finds_dependency_in_correct_directory_with_underscore template = FakeTemplate.new("<%# render(message_type.messages) %>", :erb) - tracker = make_tracker('message_types/_message_type', template) + tracker = make_tracker("message_types/_message_type", template) assert_equal ["messages/message"], tracker.dependencies end -end + def test_dependency_of_erb_template_with_no_spaces_after_render + template = FakeTemplate.new("<%# render'messages/message' %>", :erb) + tracker = make_tracker("messages/_message", template) + + assert_equal ["messages/message"], tracker.dependencies + end + + def test_finds_no_dependency_when_render_begins_the_name_of_an_identifier + template = FakeTemplate.new("<%# rendering 'it useless' %>", :erb) + tracker = make_tracker("resources/_resource", template) + + assert_equal [], tracker.dependencies + end + + def test_finds_no_dependency_when_render_ends_the_name_of_another_method + template = FakeTemplate.new("<%# surrender 'to reason' %>", :erb) + tracker = make_tracker("resources/_resource", template) + + assert_equal [], tracker.dependencies + end + + def test_finds_dependency_on_multiline_render_calls + template = FakeTemplate.new("<%# + render :object => @all_posts, + :partial => 'posts' %>", :erb) + + tracker = make_tracker("some/_little_posts", template) + + assert_equal ["some/posts"], tracker.dependencies + end + + def test_finds_multiple_unrelated_odd_dependencies + template = FakeTemplate.new(" + <%# render('shared/header', title: 'Title') %> +

Section title

+ <%# render@section %> + ", :erb) + + tracker = make_tracker("multiple/_dependencies", template) + + assert_equal ["shared/header", "sections/section"], tracker.dependencies + end + + def test_finds_dependencies_for_all_kinds_of_identifiers + template = FakeTemplate.new(" + <%# render $globals %> + <%# render @instance_variables %> + <%# render @@class_variables %> + ", :erb) + + tracker = make_tracker("identifiers/_all", template) + + assert_equal [ + "globals/global", + "instance_variables/instance_variable", + "class_variables/class_variable" + ], tracker.dependencies + end + + def test_finds_dependencies_on_method_chains + template = FakeTemplate.new("<%# render @parent.child.grandchildren %>", :erb) + tracker = make_tracker("method/_chains", template) + + assert_equal ["grandchildren/grandchild"], tracker.dependencies + end + + def test_finds_dependencies_with_special_characters + template = FakeTemplate.new("<%# render @pokémon, partial: 'ピカチュウ' %>", :erb) + tracker = make_tracker("special/_characters", template) + + assert_equal ["special/ピカチュウ"], tracker.dependencies + end + + def test_finds_dependencies_with_quotes_within + template = FakeTemplate.new(" + <%# render \"single/quote's\" %> + <%# render 'double/quote\"s' %> + ", :erb) + tracker = make_tracker("quotes/_single_and_double", template) + + assert_equal ["single/quote's", 'double/quote"s'], tracker.dependencies + end +end -- cgit v1.2.3 From ccbba3ff50a7c2d6523f55f7821aabdb89fc5d45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Britto?= Date: Sat, 21 Dec 2013 22:14:07 -0200 Subject: Avoid scanning multiple render calls as a single match. Each chunk of text coming after `render` is now handled individually as a possible list of arguments. --- actionview/lib/action_view/dependency_tracker.rb | 36 ++++++++++++---------- .../test/template/dependency_tracker_test.rb | 30 +++++++++++++++--- 2 files changed, 45 insertions(+), 21 deletions(-) diff --git a/actionview/lib/action_view/dependency_tracker.rb b/actionview/lib/action_view/dependency_tracker.rb index 748fa81221..0ccf2515c5 100644 --- a/actionview/lib/action_view/dependency_tracker.rb +++ b/actionview/lib/action_view/dependency_tracker.rb @@ -54,21 +54,20 @@ module ActionView /x # Matches: - # render partial: "comments/comment", collection: commentable.comments - # render "comments/comments" - # render 'comments/comments' - # render('comments/comments') + # partial: "comments/comment", collection: @all_comments => "comments/comment" + # (object: @single_comment, partial: "comments/comment") => "comments/comment" # - # render(@topic) => render("topics/topic") - # render(topics) => render("topics/topic") - # render(message.topics) => render("topics/topic") - RENDER_DEPENDENCY = / - \brender\b # render, the whole word - \s*\(?\s* # optional opening paren surrounded by spaces - (?: - (?:.*?#{PARTIAL_HASH_KEY})? # optional hash, up to the partial key declaration - (?:#{STRING}|#{VARIABLE_OR_METHOD_CHAIN}) # finally, the dependency name of interest - ) + # "comments/comments" + # 'comments/comments' + # ('comments/comments') + # + # (@topic) => "topics/topic" + # topics => "topics/topic" + # (message.topics) => "topics/topic" + RENDER_ARGUMENTS = /\A + (?:\s*\(?\s*) # optional opening paren surrounded by spaces + (?:.*?#{PARTIAL_HASH_KEY})? # optional hash, up to the partial key declaration + (?:#{STRING}|#{VARIABLE_OR_METHOD_CHAIN}) # finally, the dependency name of interest /xm def self.call(name, template) @@ -98,10 +97,13 @@ module ActionView def render_dependencies render_dependencies = [] + render_calls = source.split(/\brender\b/).drop(1) - source.scan(RENDER_DEPENDENCY) do - add_dynamic_dependency(render_dependencies, Regexp.last_match[:dynamic]) - add_static_dependency(render_dependencies, Regexp.last_match[:static]) + render_calls.each do |arguments| + arguments.scan(RENDER_ARGUMENTS) do + add_dynamic_dependency(render_dependencies, Regexp.last_match[:dynamic]) + add_static_dependency(render_dependencies, Regexp.last_match[:static]) + end end render_dependencies.uniq diff --git a/actionview/test/template/dependency_tracker_test.rb b/actionview/test/template/dependency_tracker_test.rb index 6540394c14..df3a0602d1 100644 --- a/actionview/test/template/dependency_tracker_test.rb +++ b/actionview/test/template/dependency_tracker_test.rb @@ -147,12 +147,34 @@ class ERBTrackerTest < Minitest::Test end def test_finds_dependencies_with_quotes_within - template = FakeTemplate.new(" - <%# render \"single/quote's\" %> - <%# render 'double/quote\"s' %> - ", :erb) + template = FakeTemplate.new(%{ + <%# render "single/quote's" %> + <%# render 'double/quote"s' %> + }, :erb) + tracker = make_tracker("quotes/_single_and_double", template) assert_equal ["single/quote's", 'double/quote"s'], tracker.dependencies end + + def test_finds_dependencies_with_extra_spaces + template = FakeTemplate.new(%{ + <%= render "header" %> + <%= render partial: "form" %> + <%= render @message %> + <%= render ( @message.events ) %> + <%= render :collection => @message.comments, + :partial => "comments/comment" %> + }, :erb) + + tracker = make_tracker("spaces/_extra", template) + + assert_equal [ + "spaces/header", + "spaces/form", + "messages/message", + "events/event", + "comments/comment" + ], tracker.dependencies + end end -- cgit v1.2.3 From e987dcd78f6fa21d4a6c3a10f2347c80145bac40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Britto?= Date: Thu, 9 Jan 2014 20:47:14 -0200 Subject: Update changelog --- actionview/CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index a57da72d8c..19877ca8cb 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,10 @@ +* Improved ERB dependency detection. New argument types and formattings for the `render` + calls can be matched. + + Fixes #13074 and #13116 + + *João Britto* + * Use `display:none` instead of `display:inline` for hidden fields Fixes #6403 -- cgit v1.2.3 From e47203dfdebf797d3325a240dccf4ab604915179 Mon Sep 17 00:00:00 2001 From: "T.J. Schuck" Date: Thu, 16 Jan 2014 14:57:22 -0500 Subject: Update deprecation warning to give more information about caller --- activerecord/lib/active_record/fixtures.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index fee3f51d9e..297792aeec 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -462,7 +462,7 @@ module ActiveRecord @class_names.delete_if { |k,klass| unless klass.is_a? Class klass = klass.safe_constantize - ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name will be removed in Rails 4.2, consider using the class itself instead.") + ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name to `set_fixture_class` will be removed in Rails 4.2. Use the class itself instead.") end !insert_class(@class_names, k, klass) } @@ -568,7 +568,7 @@ module ActiveRecord @model_class = nil if class_name.is_a?(String) - ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name will be removed in Rails 4.2, consider using the class itself instead.") + ActiveSupport::Deprecation.warn("The ability to pass in strings as a class name to `FixtureSet.new` will be removed in Rails 4.2. Use the class itself instead.") end if class_name.is_a?(Class) # TODO: Should be an AR::Base type class, or any? -- cgit v1.2.3 From 7b11b06e6d880edae2da092ed82b722f1a9b5c3d Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Wed, 15 Jan 2014 21:43:20 -0200 Subject: No need to use #send with public methods --- .../test/cases/transaction_callbacks_test.rb | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index fe0781b7ec..5f30125e92 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -30,14 +30,14 @@ class TransactionCallbacksTest < ActiveRecord::TestCase has_many :replies, class_name: "ReplyWithCallbacks", foreign_key: "parent_id" - after_commit{|record| record.send(:do_after_commit, nil)} - after_commit(:on => :create){|record| record.send(:do_after_commit, :create)} - after_commit(:on => :update){|record| record.send(:do_after_commit, :update)} - after_commit(:on => :destroy){|record| record.send(:do_after_commit, :destroy)} - after_rollback{|record| record.send(:do_after_rollback, nil)} - after_rollback(:on => :create){|record| record.send(:do_after_rollback, :create)} - after_rollback(:on => :update){|record| record.send(:do_after_rollback, :update)} - after_rollback(:on => :destroy){|record| record.send(:do_after_rollback, :destroy)} + after_commit { |record| record.do_after_commit(nil) } + after_commit(on: :create) { |record| record.do_after_commit(:create) } + after_commit(on: :update) { |record| record.do_after_commit(:update) } + after_commit(on: :destroy) { |record| record.do_after_commit(:destroy) } + after_rollback { |record| record.do_after_rollback(nil) } + after_rollback(on: :create) { |record| record.do_after_rollback(:create) } + after_rollback(on: :update) { |record| record.do_after_rollback(:update) } + after_rollback(on: :destroy) { |record| record.do_after_rollback(:destroy) } def history @history ||= [] @@ -303,11 +303,11 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_after_rollback_callbacks_should_validate_on_condition - assert_raise(ArgumentError) { Topic.send(:after_rollback, :on => :save) } + assert_raise(ArgumentError) { Topic.after_rollback(on: :save) } end def test_after_commit_callbacks_should_validate_on_condition - assert_raise(ArgumentError) { Topic.send(:after_commit, :on => :save) } + assert_raise(ArgumentError) { Topic.after_commit(on: :save) } end def test_saving_a_record_with_a_belongs_to_that_specifies_touching_the_parent_should_call_callbacks_on_the_parent_object -- cgit v1.2.3 From da4b5e8ecbfc684850e23afe2b3522d32e48bcac Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Wed, 15 Jan 2014 21:58:29 -0200 Subject: Cleanup duplicated setup of callbacks in transactions tests --- .../test/cases/transaction_callbacks_test.rb | 67 ++++++---------------- 1 file changed, 19 insertions(+), 48 deletions(-) diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 5f30125e92..256b043011 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -79,24 +79,14 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record - @first.after_commit_block(:create){|r| r.history << :commit_on_create} - @first.after_commit_block(:update){|r| r.history << :commit_on_update} - @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} - @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} - @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} - @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + add_transaction_execution_blocks @first @first.save! assert_equal [:commit_on_update], @first.history end def test_only_call_after_commit_on_destroy_after_transaction_commits_for_destroyed_record - @first.after_commit_block(:create){|r| r.history << :commit_on_create} - @first.after_commit_block(:update){|r| r.history << :commit_on_update} - @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} - @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} - @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} - @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + add_transaction_execution_blocks @first @first.destroy assert_equal [:commit_on_destroy], @first.history @@ -104,12 +94,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today) - @new_record.after_commit_block(:create){|r| r.history << :commit_on_create} - @new_record.after_commit_block(:update){|r| r.history << :commit_on_update} - @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} - @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create} - @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update} - @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + add_transaction_execution_blocks @new_record @new_record.save! assert_equal [:commit_on_create], @new_record.history @@ -123,12 +108,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_only_call_after_commit_on_update_after_transaction_commits_for_existing_record_on_touch - @first.after_commit_block(:create){|r| r.history << :commit_on_create} - @first.after_commit_block(:update){|r| r.history << :commit_on_update} - @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} - @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} - @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} - @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + add_transaction_execution_blocks @first @first.touch assert_equal [:commit_on_update], @first.history @@ -147,12 +127,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record - @first.after_commit_block(:create){|r| r.history << :commit_on_create} - @first.after_commit_block(:update){|r| r.history << :commit_on_update} - @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} - @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} - @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} - @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + add_transaction_execution_blocks @first Topic.transaction do @first.save! @@ -163,12 +138,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_only_call_after_rollback_on_update_after_transaction_rollsback_for_existing_record_on_touch - @first.after_commit_block(:create){|r| r.history << :commit_on_create} - @first.after_commit_block(:update){|r| r.history << :commit_on_update} - @first.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} - @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} - @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} - @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + add_transaction_execution_blocks @first Topic.transaction do @first.touch @@ -179,12 +149,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_only_call_after_rollback_on_destroy_after_transaction_rollsback_for_destroyed_record - @first.after_commit_block(:create){|r| r.history << :commit_on_create} - @first.after_commit_block(:update){|r| r.history << :commit_on_update} - @first.after_commit_block(:destroy){|r| r.history << :commit_on_update} - @first.after_rollback_block(:create){|r| r.history << :rollback_on_create} - @first.after_rollback_block(:update){|r| r.history << :rollback_on_update} - @first.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + add_transaction_execution_blocks @first Topic.transaction do @first.destroy @@ -196,12 +161,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase def test_only_call_after_rollback_on_create_after_transaction_rollsback_for_new_record @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today) - @new_record.after_commit_block(:create){|r| r.history << :commit_on_create} - @new_record.after_commit_block(:update){|r| r.history << :commit_on_update} - @new_record.after_commit_block(:destroy){|r| r.history << :commit_on_destroy} - @new_record.after_rollback_block(:create){|r| r.history << :rollback_on_create} - @new_record.after_rollback_block(:update){|r| r.history << :rollback_on_update} - @new_record.after_rollback_block(:destroy){|r| r.history << :rollback_on_destroy} + add_transaction_execution_blocks @new_record Topic.transaction do @new_record.save! @@ -324,6 +284,17 @@ class TransactionCallbacksTest < ActiveRecord::TestCase assert flag end + + private + + def add_transaction_execution_blocks(record) + record.after_commit_block(:create) { |r| r.history << :commit_on_create } + record.after_commit_block(:update) { |r| r.history << :commit_on_update } + record.after_commit_block(:destroy) { |r| r.history << :commit_on_destroy } + record.after_rollback_block(:create) { |r| r.history << :rollback_on_create } + record.after_rollback_block(:update) { |r| r.history << :rollback_on_update } + record.after_rollback_block(:destroy) { |r| r.history << :rollback_on_destroy } + end end class CallbacksOnMultipleActionsTest < ActiveRecord::TestCase -- cgit v1.2.3 From 07ff3525d5275de47960c3a012a1fa261bbfdfc8 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Wed, 15 Jan 2014 21:59:33 -0200 Subject: No need for instance vars on single tests --- activerecord/test/cases/transaction_callbacks_test.rb | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 256b043011..2da0754040 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -93,11 +93,11 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record - @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today) - add_transaction_execution_blocks @new_record + new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today) + add_transaction_execution_blocks new_record - @new_record.save! - assert_equal [:commit_on_create], @new_record.history + new_record.save! + assert_equal [:commit_on_create], new_record.history end def test_only_call_after_commit_on_create_after_transaction_commits_for_new_record_if_create_succeeds_creating_through_association @@ -160,15 +160,15 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def test_only_call_after_rollback_on_create_after_transaction_rollsback_for_new_record - @new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today) - add_transaction_execution_blocks @new_record + new_record = TopicWithCallbacks.new(:title => "New topic", :written_on => Date.today) + add_transaction_execution_blocks new_record Topic.transaction do - @new_record.save! + new_record.save! raise ActiveRecord::Rollback end - assert_equal [:rollback_on_create], @new_record.history + assert_equal [:rollback_on_create], new_record.history end def test_call_after_rollback_when_commit_fails -- cgit v1.2.3 From c9c4a427e13dab240ab1cc35e0a3b3555c01b37f Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Wed, 15 Jan 2014 22:03:53 -0200 Subject: Do not set up a variable used only in two tests Just create a local variable whenever we need the record, rather than doing an extra find for every test on the setup method. --- .../test/cases/transaction_callbacks_test.rb | 33 ++++++++++++---------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 2da0754040..7e7d95841b 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -67,7 +67,7 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end def setup - @first, @second = TopicWithCallbacks.find(1, 3).sort_by { |t| t.id } + @first = TopicWithCallbacks.find(1) end def test_call_after_commit_after_transaction_commits @@ -195,23 +195,24 @@ class TransactionCallbacksTest < ActiveRecord::TestCase @first.after_rollback_block{|r| r.rollbacks(1)} @first.after_commit_block{|r| r.commits(1)} - def @second.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end - def @second.commits(i=0); @commits ||= 0; @commits += i if i; end - @second.after_rollback_block{|r| r.rollbacks(1)} - @second.after_commit_block{|r| r.commits(1)} + second = TopicWithCallbacks.find(3) + def second.rollbacks(i=0); @rollbacks ||= 0; @rollbacks += i if i; end + def second.commits(i=0); @commits ||= 0; @commits += i if i; end + second.after_rollback_block{|r| r.rollbacks(1)} + second.after_commit_block{|r| r.commits(1)} Topic.transaction do @first.save! Topic.transaction(:requires_new => true) do - @second.save! + second.save! raise ActiveRecord::Rollback end end assert_equal 1, @first.commits assert_equal 0, @first.rollbacks - assert_equal 0, @second.commits - assert_equal 1, @second.rollbacks + assert_equal 0, second.commits + assert_equal 1, second.rollbacks end def test_only_call_after_rollback_on_records_rolled_back_to_a_savepoint_when_release_savepoint_fails @@ -242,24 +243,26 @@ class TransactionCallbacksTest < ActiveRecord::TestCase def @first.last_after_transaction_error; @last_transaction_error; end @first.after_commit_block{|r| r.last_after_transaction_error = :commit; raise "fail!";} @first.after_rollback_block{|r| r.last_after_transaction_error = :rollback; raise "fail!";} - @second.after_commit_block{|r| r.history << :after_commit} - @second.after_rollback_block{|r| r.history << :after_rollback} + + second = TopicWithCallbacks.find(3) + second.after_commit_block{|r| r.history << :after_commit} + second.after_rollback_block{|r| r.history << :after_rollback} Topic.transaction do @first.save! - @second.save! + second.save! end assert_equal :commit, @first.last_after_transaction_error - assert_equal [:after_commit], @second.history + assert_equal [:after_commit], second.history - @second.history.clear + second.history.clear Topic.transaction do @first.save! - @second.save! + second.save! raise ActiveRecord::Rollback end assert_equal :rollback, @first.last_after_transaction_error - assert_equal [:after_rollback], @second.history + assert_equal [:after_rollback], second.history end def test_after_rollback_callbacks_should_validate_on_condition -- cgit v1.2.3 From 44e55510547f5aaea78f5f91b82dd3dc5e9bef54 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Thu, 16 Jan 2014 18:45:03 -0200 Subject: Move AR test classes inside the test case --- .../associations/eager_singularization_test.rb | 84 ++++++++++++---------- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/activerecord/test/cases/associations/eager_singularization_test.rb b/activerecord/test/cases/associations/eager_singularization_test.rb index 634f6b63ba..2dee6845b0 100644 --- a/activerecord/test/cases/associations/eager_singularization_test.rb +++ b/activerecord/test/cases/associations/eager_singularization_test.rb @@ -1,45 +1,53 @@ require "cases/helper" -class Virus < ActiveRecord::Base - belongs_to :octopus -end -class Octopus < ActiveRecord::Base - has_one :virus -end -class Pass < ActiveRecord::Base - belongs_to :bus -end -class Bus < ActiveRecord::Base - has_many :passes -end -class Mess < ActiveRecord::Base - has_and_belongs_to_many :crises -end -class Crisis < ActiveRecord::Base - has_and_belongs_to_many :messes - has_many :analyses, :dependent => :destroy - has_many :successes, :through => :analyses - has_many :dresses, :dependent => :destroy - has_many :compresses, :through => :dresses -end -class Analysis < ActiveRecord::Base - belongs_to :crisis - belongs_to :success -end -class Success < ActiveRecord::Base - has_many :analyses, :dependent => :destroy - has_many :crises, :through => :analyses -end -class Dress < ActiveRecord::Base - belongs_to :crisis - has_many :compresses -end -class Compress < ActiveRecord::Base - belongs_to :dress -end - class EagerSingularizationTest < ActiveRecord::TestCase + class Virus < ActiveRecord::Base + belongs_to :octopus + end + + class Octopus < ActiveRecord::Base + has_one :virus + end + + class Pass < ActiveRecord::Base + belongs_to :bus + end + + class Bus < ActiveRecord::Base + has_many :passes + end + + class Mess < ActiveRecord::Base + has_and_belongs_to_many :crises + end + + class Crisis < ActiveRecord::Base + has_and_belongs_to_many :messes + has_many :analyses, :dependent => :destroy + has_many :successes, :through => :analyses + has_many :dresses, :dependent => :destroy + has_many :compresses, :through => :dresses + end + + class Analysis < ActiveRecord::Base + belongs_to :crisis + belongs_to :success + end + + class Success < ActiveRecord::Base + has_many :analyses, :dependent => :destroy + has_many :crises, :through => :analyses + end + + class Dress < ActiveRecord::Base + belongs_to :crisis + has_many :compresses + end + + class Compress < ActiveRecord::Base + belongs_to :dress + end def setup if ActiveRecord::Base.connection.supports_migrations? -- cgit v1.2.3 From 740cf33b5ee9156429e68c19fa0fcf78b0bef877 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Thu, 16 Jan 2014 18:48:51 -0200 Subject: Use minitest's skip rather than conditionals + early returns --- .../associations/eager_singularization_test.rb | 85 ++++++++++------------ 1 file changed, 38 insertions(+), 47 deletions(-) diff --git a/activerecord/test/cases/associations/eager_singularization_test.rb b/activerecord/test/cases/associations/eager_singularization_test.rb index 2dee6845b0..e74c2ba3b0 100644 --- a/activerecord/test/cases/associations/eager_singularization_test.rb +++ b/activerecord/test/cases/associations/eager_singularization_test.rb @@ -50,47 +50,44 @@ class EagerSingularizationTest < ActiveRecord::TestCase end def setup - if ActiveRecord::Base.connection.supports_migrations? - ActiveRecord::Base.connection.create_table :viri do |t| - t.column :octopus_id, :integer - t.column :species, :string - end - ActiveRecord::Base.connection.create_table :octopi do |t| - t.column :species, :string - end - ActiveRecord::Base.connection.create_table :passes do |t| - t.column :bus_id, :integer - t.column :rides, :integer - end - ActiveRecord::Base.connection.create_table :buses do |t| - t.column :name, :string - end - ActiveRecord::Base.connection.create_table :crises_messes, :id => false do |t| - t.column :crisis_id, :integer - t.column :mess_id, :integer - end - ActiveRecord::Base.connection.create_table :messes do |t| - t.column :name, :string - end - ActiveRecord::Base.connection.create_table :crises do |t| - t.column :name, :string - end - ActiveRecord::Base.connection.create_table :successes do |t| - t.column :name, :string - end - ActiveRecord::Base.connection.create_table :analyses do |t| - t.column :crisis_id, :integer - t.column :success_id, :integer - end - ActiveRecord::Base.connection.create_table :dresses do |t| - t.column :crisis_id, :integer - end - ActiveRecord::Base.connection.create_table :compresses do |t| - t.column :dress_id, :integer - end - @have_tables = true - else - @have_tables = false + skip 'Does not support migrations' unless ActiveRecord::Base.connection.supports_migrations? + + ActiveRecord::Base.connection.create_table :viri do |t| + t.column :octopus_id, :integer + t.column :species, :string + end + ActiveRecord::Base.connection.create_table :octopi do |t| + t.column :species, :string + end + ActiveRecord::Base.connection.create_table :passes do |t| + t.column :bus_id, :integer + t.column :rides, :integer + end + ActiveRecord::Base.connection.create_table :buses do |t| + t.column :name, :string + end + ActiveRecord::Base.connection.create_table :crises_messes, :id => false do |t| + t.column :crisis_id, :integer + t.column :mess_id, :integer + end + ActiveRecord::Base.connection.create_table :messes do |t| + t.column :name, :string + end + ActiveRecord::Base.connection.create_table :crises do |t| + t.column :name, :string + end + ActiveRecord::Base.connection.create_table :successes do |t| + t.column :name, :string + end + ActiveRecord::Base.connection.create_table :analyses do |t| + t.column :crisis_id, :integer + t.column :success_id, :integer + end + ActiveRecord::Base.connection.create_table :dresses do |t| + t.column :crisis_id, :integer + end + ActiveRecord::Base.connection.create_table :compresses do |t| + t.column :dress_id, :integer end end @@ -109,28 +106,24 @@ class EagerSingularizationTest < ActiveRecord::TestCase end def test_eager_no_extra_singularization_belongs_to - return unless @have_tables assert_nothing_raised do Virus.all.merge!(:includes => :octopus).to_a end end def test_eager_no_extra_singularization_has_one - return unless @have_tables assert_nothing_raised do Octopus.all.merge!(:includes => :virus).to_a end end def test_eager_no_extra_singularization_has_many - return unless @have_tables assert_nothing_raised do Bus.all.merge!(:includes => :passes).to_a end end def test_eager_no_extra_singularization_has_and_belongs_to_many - return unless @have_tables assert_nothing_raised do Crisis.all.merge!(:includes => :messes).to_a Mess.all.merge!(:includes => :crises).to_a @@ -138,14 +131,12 @@ class EagerSingularizationTest < ActiveRecord::TestCase end def test_eager_no_extra_singularization_has_many_through_belongs_to - return unless @have_tables assert_nothing_raised do Crisis.all.merge!(:includes => :successes).to_a end end def test_eager_no_extra_singularization_has_many_through_has_many - return unless @have_tables assert_nothing_raised do Crisis.all.merge!(:includes => :compresses).to_a end -- cgit v1.2.3 From 49223c9bc9bab1d827e98763ee49892b8c4faa72 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Thu, 16 Jan 2014 18:52:43 -0200 Subject: Extract a method to simplify setup code --- .../associations/eager_singularization_test.rb | 50 ++++++++++++---------- 1 file changed, 27 insertions(+), 23 deletions(-) diff --git a/activerecord/test/cases/associations/eager_singularization_test.rb b/activerecord/test/cases/associations/eager_singularization_test.rb index e74c2ba3b0..669569a774 100644 --- a/activerecord/test/cases/associations/eager_singularization_test.rb +++ b/activerecord/test/cases/associations/eager_singularization_test.rb @@ -50,59 +50,63 @@ class EagerSingularizationTest < ActiveRecord::TestCase end def setup - skip 'Does not support migrations' unless ActiveRecord::Base.connection.supports_migrations? + skip 'Does not support migrations' unless connection.supports_migrations? - ActiveRecord::Base.connection.create_table :viri do |t| + connection.create_table :viri do |t| t.column :octopus_id, :integer t.column :species, :string end - ActiveRecord::Base.connection.create_table :octopi do |t| + connection.create_table :octopi do |t| t.column :species, :string end - ActiveRecord::Base.connection.create_table :passes do |t| + connection.create_table :passes do |t| t.column :bus_id, :integer t.column :rides, :integer end - ActiveRecord::Base.connection.create_table :buses do |t| + connection.create_table :buses do |t| t.column :name, :string end - ActiveRecord::Base.connection.create_table :crises_messes, :id => false do |t| + connection.create_table :crises_messes, :id => false do |t| t.column :crisis_id, :integer t.column :mess_id, :integer end - ActiveRecord::Base.connection.create_table :messes do |t| + connection.create_table :messes do |t| t.column :name, :string end - ActiveRecord::Base.connection.create_table :crises do |t| + connection.create_table :crises do |t| t.column :name, :string end - ActiveRecord::Base.connection.create_table :successes do |t| + connection.create_table :successes do |t| t.column :name, :string end - ActiveRecord::Base.connection.create_table :analyses do |t| + connection.create_table :analyses do |t| t.column :crisis_id, :integer t.column :success_id, :integer end - ActiveRecord::Base.connection.create_table :dresses do |t| + connection.create_table :dresses do |t| t.column :crisis_id, :integer end - ActiveRecord::Base.connection.create_table :compresses do |t| + connection.create_table :compresses do |t| t.column :dress_id, :integer end end def teardown - ActiveRecord::Base.connection.drop_table :viri - ActiveRecord::Base.connection.drop_table :octopi - ActiveRecord::Base.connection.drop_table :passes - ActiveRecord::Base.connection.drop_table :buses - ActiveRecord::Base.connection.drop_table :crises_messes - ActiveRecord::Base.connection.drop_table :messes - ActiveRecord::Base.connection.drop_table :crises - ActiveRecord::Base.connection.drop_table :successes - ActiveRecord::Base.connection.drop_table :analyses - ActiveRecord::Base.connection.drop_table :dresses - ActiveRecord::Base.connection.drop_table :compresses + connection.drop_table :viri + connection.drop_table :octopi + connection.drop_table :passes + connection.drop_table :buses + connection.drop_table :crises_messes + connection.drop_table :messes + connection.drop_table :crises + connection.drop_table :successes + connection.drop_table :analyses + connection.drop_table :dresses + connection.drop_table :compresses + end + + def connection + ActiveRecord::Base.connection end def test_eager_no_extra_singularization_belongs_to -- cgit v1.2.3 From bcd6def32b0970b33a49c721ea247c8360bf8344 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 16 Jan 2014 20:36:38 -0200 Subject: Remove warning --- activesupport/test/core_ext/module_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index 4680fe5dfe..ff6e21854e 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -246,12 +246,12 @@ class ModuleTest < ActiveSupport::TestCase end def test_delegation_line_number - file, line = Someone.instance_method(:foo).source_location + _, line = Someone.instance_method(:foo).source_location assert_equal Someone::FAILED_DELEGATE_LINE, line end def test_delegate_line_with_nil - file, line = Someone.instance_method(:bar).source_location + _, line = Someone.instance_method(:bar).source_location assert_equal Someone::FAILED_DELEGATE_LINE_2, line end -- cgit v1.2.3 From beeb8969e0ec623b5221d5b8aa6713d9139c4545 Mon Sep 17 00:00:00 2001 From: Rex Feng Date: Thu, 16 Jan 2014 18:52:44 -0500 Subject: clean up security guide: his => their [ci skip] --- guides/source/security.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guides/source/security.md b/guides/source/security.md index c367604d6f..cffe7c85f1 100644 --- a/guides/source/security.md +++ b/guides/source/security.md @@ -81,7 +81,7 @@ Here are some general guidelines on sessions. * _Do not store large objects in a session_. Instead you should store them in the database and save their id in the session. This will eliminate synchronization headaches and it won't fill up your session storage space (depending on what session storage you chose, see below). This will also be a good idea, if you modify the structure of an object and old versions of it are still in some user's cookies. With server-side session storages you can clear out the sessions, but with client-side storages, this is hard to mitigate. -* _Critical data should not be stored in session_. If the user clears his cookies or closes the browser, they will be lost. And with a client-side session storage, the user can read the data. +* _Critical data should not be stored in session_. If the user clears their cookies or closes the browser, they will be lost. And with a client-side session storage, the user can read the data. ### Session Storage -- cgit v1.2.3 From 7252b43c6ecd31a023399085165b1cea9d1306f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Thu, 16 Jan 2014 22:09:37 -0200 Subject: Only some dynamic finders are deprecated. find_by_* and find_by_*! are not deprecated for example, so lets add a note only where it is needed [ci skip] --- guides/source/active_record_querying.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 4725e2c8a2..3783be50c0 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -1338,11 +1338,6 @@ Client.unscoped { Dynamic Finders --------------- -NOTE: Dynamic finders have been deprecated in Rails 4.0 and will be -removed in Rails 4.1. The best practice is to use Active Record scopes -instead. You can find the deprecation gem at -https://github.com/rails/activerecord-deprecated_finders - For every field (also known as an attribute) you define in your table, Active Record provides a finder method. If you have a field called `first_name` on your `Client` model for example, you get `find_by_first_name` for free from Active Record. If you have a `locked` field on the `Client` model, you also get `find_by_locked` and methods. You can specify an exclamation point (`!`) on the end of the dynamic finders to get them to raise an `ActiveRecord::RecordNotFound` error if they do not return any records, like `Client.find_by_name!("Ryan")` @@ -1352,6 +1347,11 @@ If you want to find both by name and locked, you can chain these finders togethe Find or Build a New Object -------------------------- +NOTE: Some dynamic finders have been deprecated in Rails 4.0 and will be +removed in Rails 4.1. The best practice is to use Active Record scopes +instead. You can find the deprecation gem at +https://github.com/rails/activerecord-deprecated_finders + It's common that you need to find a record or create it if it doesn't exist. You can do that with the `find_or_create_by` and `find_or_create_by!` methods. ### `find_or_create_by` -- cgit v1.2.3 From 5402b72faafecfa1eda8afce0e0f194fcf385fe3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Fri, 17 Jan 2014 00:10:25 -0200 Subject: Remove warnings on Ruby 2.1 --- activesupport/lib/active_support/hash_with_indifferent_access.rb | 6 +++++- activesupport/lib/active_support/ordered_hash.rb | 4 ++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index f690eab604..42f5e64306 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -228,7 +228,11 @@ module ActiveSupport def to_options!; self end def select(*args, &block) - dup.tap {|hash| hash.select!(*args, &block)} + dup.tap { |hash| hash.select!(*args, &block)} + end + + def reject(*args, &block) + dup.tap { |hash| hash.reject!(*args, &block)} end # Convert to a regular hash with string keys. diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index 1a3693f766..8b320f6d05 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -28,6 +28,10 @@ module ActiveSupport coder.represent_seq '!omap', map { |k,v| { k => v } } end + def reject(*args, &block) + dup.tap { |hash| hash.reject!(*args, &block)} + end + def nested_under_indifferent_access self end -- cgit v1.2.3 From a49727213c3f5906a3fbbfa552256312e0cc319e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Fri, 17 Jan 2014 00:30:41 -0200 Subject: Consistence in the block style --- activesupport/lib/active_support/hash_with_indifferent_access.rb | 8 ++++---- activesupport/lib/active_support/ordered_hash.rb | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/activesupport/lib/active_support/hash_with_indifferent_access.rb b/activesupport/lib/active_support/hash_with_indifferent_access.rb index 42f5e64306..594a4ca938 100644 --- a/activesupport/lib/active_support/hash_with_indifferent_access.rb +++ b/activesupport/lib/active_support/hash_with_indifferent_access.rb @@ -159,7 +159,7 @@ module ActiveSupport # # counters.fetch('foo') # => 1 # counters.fetch(:bar, 0) # => 0 - # counters.fetch(:bar) {|key| 0} # => 0 + # counters.fetch(:bar) { |key| 0 } # => 0 # counters.fetch(:zoo) # => KeyError: key not found: "zoo" def fetch(key, *extras) super(convert_key(key), *extras) @@ -172,7 +172,7 @@ module ActiveSupport # hash[:b] = 'y' # hash.values_at('a', 'b') # => ["x", "y"] def values_at(*indices) - indices.collect {|key| self[convert_key(key)]} + indices.collect { |key| self[convert_key(key)] } end # Returns an exact copy of the hash. @@ -228,11 +228,11 @@ module ActiveSupport def to_options!; self end def select(*args, &block) - dup.tap { |hash| hash.select!(*args, &block)} + dup.tap { |hash| hash.select!(*args, &block) } end def reject(*args, &block) - dup.tap { |hash| hash.reject!(*args, &block)} + dup.tap { |hash| hash.reject!(*args, &block) } end # Convert to a regular hash with string keys. diff --git a/activesupport/lib/active_support/ordered_hash.rb b/activesupport/lib/active_support/ordered_hash.rb index 8b320f6d05..58a2ce2105 100644 --- a/activesupport/lib/active_support/ordered_hash.rb +++ b/activesupport/lib/active_support/ordered_hash.rb @@ -29,7 +29,7 @@ module ActiveSupport end def reject(*args, &block) - dup.tap { |hash| hash.reject!(*args, &block)} + dup.tap { |hash| hash.reject!(*args, &block) } end def nested_under_indifferent_access -- cgit v1.2.3 From 486db21f919c39cef76b487be45290b7561307e8 Mon Sep 17 00:00:00 2001 From: Carlos Antonio da Silva Date: Fri, 17 Jan 2014 15:24:48 -0200 Subject: Fix eager load of Serializers on Active Model --- activemodel/lib/active_model.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb index 469f137d03..3e49c34182 100644 --- a/activemodel/lib/active_model.rb +++ b/activemodel/lib/active_model.rb @@ -59,9 +59,9 @@ module ActiveModel end end - def eager_load! + def self.eager_load! super - ActiveModel::Serializer.eager_load! + ActiveModel::Serializers.eager_load! end end -- cgit v1.2.3