From 14575893010add4db926c3e8517c360ea62625c2 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Sat, 10 Aug 2013 22:21:25 +0530 Subject: [PATCH 01/44] Remove sqlite specific`supports_autoincrement?` which always defaults to true --- .../connection_adapters/sqlite3_adapter.rb | 11 +---------- activerecord/test/schema/sqlite_specific_schema.rb | 7 ++----- 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index df489a5b1f..472a96400d 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -180,11 +180,6 @@ def supports_count_distinct? #:nodoc: true end - # Returns true - def supports_autoincrement? #:nodoc: - true - end - def supports_index_sort_order? true end @@ -606,11 +601,7 @@ def sqlite_version end def default_primary_key_type - if supports_autoincrement? - 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL' - else - 'INTEGER PRIMARY KEY NOT NULL' - end + 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL' end def translate_exception(exception, message) diff --git a/activerecord/test/schema/sqlite_specific_schema.rb b/activerecord/test/schema/sqlite_specific_schema.rb index e9ddeb32cf..b7aff4f47d 100644 --- a/activerecord/test/schema/sqlite_specific_schema.rb +++ b/activerecord/test/schema/sqlite_specific_schema.rb @@ -1,9 +1,6 @@ ActiveRecord::Schema.define do - # For sqlite 3.1.0+, make a table with an autoincrement column - if supports_autoincrement? - create_table :table_with_autoincrement, :force => true do |t| - t.column :name, :string - end + create_table :table_with_autoincrement, :force => true do |t| + t.column :name, :string end execute "DROP TABLE fk_test_has_fk" rescue nil From 49f513c25415f94452af1bb8fe3f55f090a6d0f2 Mon Sep 17 00:00:00 2001 From: Zachary Scott Date: Sat, 28 Sep 2013 19:31:46 -0400 Subject: [PATCH 02/44] fixed a doc bug in the CHANGELOG.md s/does no longer depend on/no longer depends on/ --- activerecord/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 810a2c668d..dc549240c2 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -519,7 +519,7 @@ *Neeraj Singh* -* Fixture setup does no longer depend on `ActiveRecord::Base.configurations`. +* Fixture setup no longer depends on `ActiveRecord::Base.configurations`. This is relevant when `ENV["DATABASE_URL"]` is used in place of a `database.yml`. *Yves Senn* From ac53d6e94287c63b5c105bc8a571c593864230c2 Mon Sep 17 00:00:00 2001 From: Zachary Scott Date: Sat, 28 Sep 2013 19:32:48 -0400 Subject: [PATCH 03/44] Use ActiveRecord::Base#update! instead of #update_attributes! --- guides/source/action_controller_overview.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index 8dfecd0190..cd4a1a0792 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -209,7 +209,7 @@ class PeopleController < ActionController::Base # Request reply. def update person = current_account.people.find(params[:id]) - person.update_attributes!(person_params) + person.update!(person_params) redirect_to person end From afd0a8ab5f690b9bcc5f49371abd82d41a364f58 Mon Sep 17 00:00:00 2001 From: Federico Ravasio Date: Mon, 7 Oct 2013 10:15:23 +0200 Subject: [PATCH 04/44] Just change ENV and restore it afterwards. Stubbing ENV[] is not safe outside MRI. At some point after the stubbing has occurred a backtrace is printed to the ActiveSupport warning log: there Rubinius accesses ENV['RBX_NOCOLOR'] to determine if it should print the backtrace with colors or not, causing the stub to fail. Other implementations might access ENV in a different way too, we just can't predict it. The only thing we can do here is to actually set the ENV with what we want and restore it afterwards. --- activerecord/test/cases/fixtures_test.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index e61deb1080..bffff07089 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -253,7 +253,8 @@ def test_serialized_fixtures end def test_fixtures_are_set_up_with_database_env_variable - ENV.stubs(:[]).with("DATABASE_URL").returns("sqlite3:///:memory:") + db_url_tmp = ENV['DATABASE_URL'] + ENV['DATABASE_URL'] = "sqlite3:///:memory:" ActiveRecord::Base.stubs(:configurations).returns({}) test_case = Class.new(ActiveRecord::TestCase) do fixtures :accounts @@ -266,6 +267,8 @@ def test_fixtures result = test_case.new(:test_fixtures).run assert result.passed?, "Expected #{result.name} to pass:\n#{result}" + ensure + ENV['DATABASE_URL'] = db_url_tmp end end From d8537ef1ec1ba51eb08087b4c929474f267bdf10 Mon Sep 17 00:00:00 2001 From: Federico Ravasio Date: Mon, 7 Oct 2013 10:20:05 +0200 Subject: [PATCH 05/44] Assert presence of "frozen" in error message, not the full MRI message. Related to all the other issues regarding message independent assertions to make Rails compatible with other Ruby implementations other than MRI. The best way here would be to have a specific error raised when modifying frozen objects, like FrozenObjectError or something. But since Ruby doesn't provide such a thing, we must limit the assertion to the lowest common denominator, which is word "frozen". --- activerecord/test/cases/transactions_test.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 17206ffe99..980981903a 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -421,7 +421,9 @@ def test_rollback_when_saving_a_frozen_record topic = Topic.new(:title => 'test') topic.freeze e = assert_raise(RuntimeError) { topic.save } - assert_equal "can't modify frozen Hash", e.message + assert_match(/frozen/i, e.message) # Not good enough, but we can't do much + # about it since there is no specific error + # for frozen objects. assert !topic.persisted?, 'not persisted' assert_nil topic.id assert topic.frozen?, 'not frozen' From 0aeb11e5751c5c998f4d52b0084754d848e2865e Mon Sep 17 00:00:00 2001 From: Federico Ravasio Date: Mon, 7 Oct 2013 10:24:14 +0200 Subject: [PATCH 06/44] Allow methods arity below -1 in assert_responds. Every method from MRI's core classes is written in C. This means Method#arity always returns -1 for methods with a variable number of arguments. This is not the case with Rubinius, where, for example Array#slice! is implemented in Ruby and has arity -2, since is defined as def slice!(start, length = undefined) --- activerecord/test/cases/relation/delegation_test.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/activerecord/test/cases/relation/delegation_test.rb b/activerecord/test/cases/relation/delegation_test.rb index 71ade0bcc2..c171c5e14e 100644 --- a/activerecord/test/cases/relation/delegation_test.rb +++ b/activerecord/test/cases/relation/delegation_test.rb @@ -9,10 +9,11 @@ class DelegationTest < ActiveRecord::TestCase def assert_responds(target, method) assert target.respond_to?(method) assert_nothing_raised do - case target.to_a.method(method).arity - when 0 + method_arity = target.to_a.method(method).arity + + if method_arity.zero? target.send(method) - when -1 + elsif method_arity < 0 if method == :shuffle! target.send(method) else From 7cab255a97fceb48e3b59b3d39b5fe58c6dad54b Mon Sep 17 00:00:00 2001 From: Edo Balvers Date: Fri, 9 Aug 2013 12:58:46 +0200 Subject: [PATCH 07/44] Fixes #11773 when using includes combined with select, the select statement was overwritten. --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/lib/active_record/relation/finder_methods.rb | 2 +- activerecord/test/cases/relations_test.rb | 8 ++++++++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 83c94caa53..c3c4ae7862 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -199,6 +199,12 @@ *Yves Senn* +* Fixes bug when using includes combined with select, the select statement was overwritten. + + Fixes #11773 + + *Edo Balvers* + * Load fixtures from linked folders. *Kassio Borges* diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 0132a02f83..fb8f44b188 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -261,7 +261,7 @@ def construct_relation_for_association_calculations end def construct_relation_for_association_find(join_dependency) - relation = except(:select).select(join_dependency.columns) + relation = except(:select).select(join_dependency.columns + select_values) apply_join_dependency(relation, join_dependency) end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index f814947ab2..396d4ef1e9 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -486,6 +486,14 @@ def test_default_scoping_finder_methods assert_equal Developer.where(name: 'David').map(&:id).sort, developers end + def test_includes_with_select + query = Post.select('comments_count AS ranking').order('ranking').includes(:comments) + .where(comments: { id: 1 }) + + assert_equal ['comments_count AS ranking'], query.select_values + assert_equal 1, query.to_a.size + end + def test_loading_with_one_association posts = Post.preload(:comments) post = posts.find { |p| p.id == 1 } From 97943f19b764e10c655846a47530122d861007b8 Mon Sep 17 00:00:00 2001 From: Jefferson Queiroz Venerando Date: Thu, 10 Oct 2013 10:34:14 -0300 Subject: [PATCH 08/44] Fix wrong variable name used in the select_day method documentation The variable name created in the example is `my_date`, the methods were using `my_time` instead. --- actionview/lib/action_view/helpers/date_helper.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionview/lib/action_view/helpers/date_helper.rb b/actionview/lib/action_view/helpers/date_helper.rb index 8e1aea50a9..d7e8e99200 100644 --- a/actionview/lib/action_view/helpers/date_helper.rb +++ b/actionview/lib/action_view/helpers/date_helper.rb @@ -531,7 +531,7 @@ def select_hour(datetime, options = {}, html_options = {}) # my_date = Time.now + 2.days # # # Generates a select field for days that defaults to the day for the date in my_date. - # select_day(my_time) + # select_day(my_date) # # # Generates a select field for days that defaults to the number given. # select_day(5) @@ -541,7 +541,7 @@ def select_hour(datetime, options = {}, html_options = {}) # # # Generates a select field for days that defaults to the day for the date in my_date # # that is named 'due' rather than 'day'. - # select_day(my_time, field_name: 'due') + # select_day(my_date, field_name: 'due') # # # Generates a select field for days with a custom prompt. Use prompt: true for a # # generic prompt. From 723c3881b9d77e3dd14b17d8b3403cd9b578e063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Rodr=C3=ADguez=20Troiti=C3=B1o?= Date: Thu, 10 Oct 2013 18:38:04 +0200 Subject: [PATCH 09/44] Keep code consistent with previous code blocks. [ci skip] --- guides/source/getting_started.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/guides/source/getting_started.md b/guides/source/getting_started.md index bb2e8e906f..2f322d15da 100644 --- a/guides/source/getting_started.md +++ b/guides/source/getting_started.md @@ -1490,8 +1490,8 @@ So first, we'll wire up the Post show template

<% end %> -<%= link_to 'Edit Post', edit_post_path(@post) %> | -<%= link_to 'Back to Posts', posts_path %> +<%= link_to 'Back', posts_path %> +| <%= link_to 'Edit', edit_post_path(@post) %> ``` This adds a form on the `Post` show page that creates a new comment by From 365110196afcf952bc22729d4467d579b708328f Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 11 Oct 2013 13:05:29 -0700 Subject: [PATCH 10/44] Revert "Merge pull request #12480 from iwiznia/master" This reverts commit e5f5a838b96a362534d9bb60d02334439ed9784c, reversing changes made to d7567f3290a50952494e9213556a1f283a6cf3a0. --- activesupport/CHANGELOG.md | 7 ------- activesupport/lib/active_support/duration.rb | 20 ------------------- activesupport/test/core_ext/duration_test.rb | 15 -------------- .../source/active_support_core_extensions.md | 15 -------------- 4 files changed, 57 deletions(-) diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 478fef2baf..f1dd7c312d 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,10 +1,3 @@ -* Add `flatten` and `flatten!` methods to Duration objects. - - Example: - Date.today + (1.month + 1.month).flatten == Date.today + 2.months - - *Ionatan Wiznia* - * `require_dependency` accepts objects that respond to `to_path`, in particular `Pathname` instances. diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index 1db0ca51ae..87b6407038 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -81,18 +81,6 @@ def as_json(options = nil) #:nodoc: to_i end - # Flattens all the +parts+ of the duration, and returns a - # new duration object. - def flatten - Duration.new(@value, flatten_parts) - end - - # Flattens all the +parts+ of this duration. - def flatten! - @parts = flatten_parts - self - end - protected def sum(sign, time = ::Time.current) #:nodoc: @@ -109,14 +97,6 @@ def sum(sign, time = ::Time.current) #:nodoc: end end - def flatten_parts - @parts.inject({}) do |result, (name, value)| - result[name] ||= 0 - result[name] += value - result - end.to_a - end - private def method_missing(method, *args, &block) #:nodoc: diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index cc24d7c74c..ed267cf4b9 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -145,21 +145,6 @@ def test_to_json assert_equal '172800', 2.days.to_json end - def test_flatten - a = 2.months - b = (1.month + 1.month).flatten - - assert_equal a.parts, b.parts - end - - def test_flatten! - a = (1.month + 1.month) - b = a.flatten - a.flatten! - - assert_equal a.parts, b.parts - end - protected def with_env_tz(new_tz = 'US/Eastern') old_tz, ENV['TZ'] = ENV['TZ'], new_tz diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index d11c17117e..d3f49b19fa 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -3699,21 +3699,6 @@ Time.utc(1582, 10, 3) + 5.days # => Mon Oct 18 00:00:00 UTC 1582 ``` -When addinng or substracting durations, the resulting duration will be equivalent to subsequent calls to since or advance, so: - -```ruby -Date.new(2013,1,28) + 1.month + 1.month -# => Thu, 28 Mar 2013 -``` - -If you want to add durations and then use them as just one call to since or advance, you can use the flatten or flatten! method: - -```ruby -Date.new(2013,1,31) + (1.month + 1.month).flatten -# => Sun, 31 Mar 2013 -``` - - Extensions to `File` -------------------- From ce46f20d13ca1b45dfd499c359ee8bce7abce1ae Mon Sep 17 00:00:00 2001 From: Malav Bhavsar Date: Thu, 10 Oct 2013 01:56:42 -0700 Subject: [PATCH 11/44] Some fixes in docs [ci skip] Update a link to point to right section in api docs Fix a typo --- guides/source/action_view_overview.md | 2 +- guides/source/active_record_basics.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md index 94aa0f8502..d19dd11181 100644 --- a/guides/source/action_view_overview.md +++ b/guides/source/action_view_overview.md @@ -269,7 +269,7 @@ Rails will render the `_product_ruler` partial (with no data passed to it) betwe ### Layouts -Layouts can be used to render a common view template around the results of Rails controller actions. Typically, every Rails has a couple of overall layouts that most pages are rendered within. For example, a site might have a layout for a logged in user, and a layout for the marketing or sales side of the site. The logged in user layout might include top-level navigation that should be present across many controller actions. The sales layout for a SaaS app might include top-level navigation for things like "Pricing" and "Contact Us." You would expect each layout to have a different look and feel. You can read more details about Layouts in the [Layouts and Rendering in Rails](layouts_and_rendering.html) guide. +Layouts can be used to render a common view template around the results of Rails controller actions. Typically, every Rails application has a couple of overall layouts that most pages are rendered within. For example, a site might have a layout for a logged in user, and a layout for the marketing or sales side of the site. The logged in user layout might include top-level navigation that should be present across many controller actions. The sales layout for a SaaS app might include top-level navigation for things like "Pricing" and "Contact Us." You would expect each layout to have a different look and feel. You can read more details about Layouts in the [Layouts and Rendering in Rails](layouts_and_rendering.html) guide. Partial Layouts --------------- diff --git a/guides/source/active_record_basics.md b/guides/source/active_record_basics.md index 34baae509b..a184f0753d 100644 --- a/guides/source/active_record_basics.md +++ b/guides/source/active_record_basics.md @@ -116,7 +116,7 @@ to Active Record instances: locking](http://api.rubyonrails.org/classes/ActiveRecord/Locking.html) to a model. * `type` - Specifies that the model uses [Single Table - Inheritance](http://api.rubyonrails.org/classes/ActiveRecord/Base.html). + Inheritance](http://api.rubyonrails.org/classes/ActiveRecord/Base.html#label-Single+table+inheritance). * `(association_name)_type` - Stores the type for [polymorphic associations](association_basics.html#polymorphic-associations). * `(table_name)_count` - Used to cache the number of belonging objects on From 464def3ecf79247834bd1d5053c3a76aeee03084 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C4=B1tk=C4=B1=20Ba=C4=9Fdat?= Date: Sat, 12 Oct 2013 16:47:17 +0300 Subject: [PATCH 12/44] Fix a writing mistake A small mistake found in the line of ```The default error is "can't be empty"``` for ```:presence``` helper. ```empty``` word changed to ```blank```. --- guides/source/active_record_validations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index 339612ebc5..797b996357 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -528,7 +528,7 @@ If you validate the presence of an object associated via a `has_one` or Since `false.blank?` is true, if you want to validate the presence of a boolean field you should use `validates :field_name, inclusion: { in: [true, false] }`. -The default error message is _"can't be empty"_. +The default error message is _"can't be blank"_. ### `absence` From 73ec210e4c6c25b589bb220169e78be566ad55a0 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Sun, 13 Oct 2013 20:54:09 +0530 Subject: [PATCH 13/44] Drop unused iterator var --- .../active_record/associations/preloader/through_association.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index ea21836c65..3166df57eb 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -15,7 +15,7 @@ def associated_records_by_owner(preloader) through_reflection.name, through_scope) - through_records = owners.map do |owner, h| + through_records = owners.map do |owner| association = owner.association through_reflection.name [owner, Array(association.reader)] From 78c176e2fabff6629fd695b4292bd60e42852ec4 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Sun, 13 Oct 2013 20:58:50 +0530 Subject: [PATCH 14/44] Change `map` to `map!` to save extra array creation on new array --- actionview/lib/action_view/helpers/url_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 8a4918a8c0..2f5246f42a 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -455,7 +455,7 @@ def mail_to(email_address, name = nil, html_options = {}, &block) html_options, name = name, nil if block_given? html_options = (html_options || {}).stringify_keys - extras = %w{ cc bcc body subject }.map { |item| + extras = %w{ cc bcc body subject }.map! { |item| option = html_options.delete(item) || next "#{item}=#{Rack::Utils.escape_path(option)}" }.compact From 4ce643dbb5bd37e1f4662b37abc80a18078feba1 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Sun, 13 Oct 2013 21:27:21 +0530 Subject: [PATCH 15/44] `Relation#count` doesn't use options anymore. --- .../lib/active_record/associations/collection_association.rb | 4 +--- .../lib/active_record/associations/collection_proxy.rb | 4 ++-- activerecord/lib/active_record/relation/calculations.rb | 5 ++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 75f8990cf0..6b06a5f7fd 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -195,9 +195,7 @@ def destroy_all # Count all records using SQL. Construct options and pass them with # scope to the target class's +count+. - def count(column_name = nil, count_options = {}) - column_name, count_options = nil, column_name if column_name.is_a?(Hash) - + def count(column_name = nil) relation = scope if association_scope.distinct_value # This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL. diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index ea7f768a68..0b6cdf5217 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -669,8 +669,8 @@ def distinct # # #, # # # # # ] - def count(column_name = nil, options = {}) - @association.count(column_name, options) + def count(column_name = nil) + @association.count(column_name) end # Returns the size of the collection. If the collection hasn't been loaded, diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 27c04b0952..a4fe3bd3b7 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -19,9 +19,8 @@ module Calculations # # Person.group(:city).count # # => { 'Rome' => 5, 'Paris' => 3 } - def count(column_name = nil, options = {}) - column_name, options = nil, column_name if column_name.is_a?(Hash) - calculate(:count, column_name, options) + def count(column_name = nil) + calculate(:count, column_name) end # Calculates the average value on a given column. Returns +nil+ if there's From 16c0023a9a14688bbc96d81e809bd26ccf651438 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Sch=C3=BCtz?= Date: Sat, 12 Oct 2013 20:27:13 +0200 Subject: [PATCH 16/44] Make logging to stdout work again with implicit `development` env --- railties/lib/rails/commands/server.rb | 3 +- railties/test/commands/server_test.rb | 60 +++++++++++++++++++++------ railties/test/env_helpers.rb | 4 ++ 3 files changed, 54 insertions(+), 13 deletions(-) diff --git a/railties/lib/rails/commands/server.rb b/railties/lib/rails/commands/server.rb index 485bd1eb09..4201dac42f 100644 --- a/railties/lib/rails/commands/server.rb +++ b/railties/lib/rails/commands/server.rb @@ -1,6 +1,7 @@ require 'fileutils' require 'optparse' require 'action_dispatch' +require 'rails' module Rails class Server < ::Rack::Server @@ -32,7 +33,7 @@ def parse!(args) opt_parser.parse! args - options[:log_stdout] = options[:daemonize].blank? && options[:environment] == "development" + options[:log_stdout] = options[:daemonize].blank? && (options[:environment] || Rails.env) == "development" options[:server] = args.shift options end diff --git a/railties/test/commands/server_test.rb b/railties/test/commands/server_test.rb index 20afca2618..ba688f1e9e 100644 --- a/railties/test/commands/server_test.rb +++ b/railties/test/commands/server_test.rb @@ -27,26 +27,62 @@ def test_server_option_without_environment end def test_environment_with_rails_env - with_rails_env 'production' do - server = Rails::Server.new - assert_equal 'production', server.options[:environment] + with_rack_env nil do + with_rails_env 'production' do + server = Rails::Server.new + assert_equal 'production', server.options[:environment] + end end end def test_environment_with_rack_env - with_rack_env 'production' do - server = Rails::Server.new - assert_equal 'production', server.options[:environment] + with_rails_env nil do + with_rack_env 'production' do + server = Rails::Server.new + assert_equal 'production', server.options[:environment] + end end end def test_log_stdout - args = ["-e", "development"] - options = Rails::Server::Options.new.parse!(args) - assert_equal true, options[:log_stdout] + with_rack_env nil do + with_rails_env nil do + args = [] + options = Rails::Server::Options.new.parse!(args) + assert_equal true, options[:log_stdout] - args = ["-e", "production"] - options = Rails::Server::Options.new.parse!(args) - assert_equal false, options[:log_stdout] + args = ["-e", "development"] + options = Rails::Server::Options.new.parse!(args) + assert_equal true, options[:log_stdout] + + args = ["-e", "production"] + options = Rails::Server::Options.new.parse!(args) + assert_equal false, options[:log_stdout] + + with_rack_env 'development' do + args = [] + options = Rails::Server::Options.new.parse!(args) + assert_equal true, options[:log_stdout] + end + + with_rack_env 'production' do + args = [] + options = Rails::Server::Options.new.parse!(args) + assert_equal false, options[:log_stdout] + end + + with_rails_env 'development' do + args = [] + options = Rails::Server::Options.new.parse!(args) + assert_equal true, options[:log_stdout] + end + + with_rails_env 'production' do + args = [] + options = Rails::Server::Options.new.parse!(args) + assert_equal false, options[:log_stdout] + end + end + end end end diff --git a/railties/test/env_helpers.rb b/railties/test/env_helpers.rb index 6223c85bbf..330fe150ca 100644 --- a/railties/test/env_helpers.rb +++ b/railties/test/env_helpers.rb @@ -1,7 +1,10 @@ +require 'rails' + module EnvHelpers private def with_rails_env(env) + Rails.instance_variable_set :@_env, nil switch_env 'RAILS_ENV', env do switch_env 'RACK_ENV', nil do yield @@ -10,6 +13,7 @@ def with_rails_env(env) end def with_rack_env(env) + Rails.instance_variable_set :@_env, nil switch_env 'RACK_ENV', env do switch_env 'RAILS_ENV', nil do yield From 42d3d3c217d158873409c50c429ae711a0306d99 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Sun, 13 Oct 2013 23:55:25 +0530 Subject: [PATCH 17/44] Stop accepting `options` for `Relation#average`, `Relation#minimum`, `Relation#maximum`, `Relation#calculate`, `perform_calculation`, `NullRelation#calculate` as they isn't used anymore. --- .../lib/active_record/null_relation.rb | 2 +- .../active_record/relation/calculations.rb | 20 +++++++++---------- activerecord/test/cases/relations_test.rb | 2 +- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 716020e7e7..1f3d377e53 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -50,7 +50,7 @@ def sum(*) 0 end - def calculate(_operation, _column_name, _options = {}) + def calculate(_operation, _column_name) if _operation == :count 0 else diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index a4fe3bd3b7..2d267183ce 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -27,8 +27,8 @@ def count(column_name = nil) # no row. See +calculate+ for examples with options. # # Person.average(:age) # => 35.8 - def average(column_name, options = {}) - calculate(:average, column_name, options) + def average(column_name) + calculate(:average, column_name) end # Calculates the minimum value on a given column. The value is returned @@ -36,8 +36,8 @@ def average(column_name, options = {}) # +calculate+ for examples with options. # # Person.minimum(:age) # => 7 - def minimum(column_name, options = {}) - calculate(:minimum, column_name, options) + def minimum(column_name) + calculate(:minimum, column_name) end # Calculates the maximum value on a given column. The value is returned @@ -45,8 +45,8 @@ def minimum(column_name, options = {}) # +calculate+ for examples with options. # # Person.maximum(:age) # => 93 - def maximum(column_name, options = {}) - calculate(:maximum, column_name, options) + def maximum(column_name) + calculate(:maximum, column_name) end # Calculates the sum of values on a given column. The value is returned @@ -89,15 +89,15 @@ def sum(*args) # Person.group(:last_name).having("min(age) > 17").minimum(:age) # # Person.sum("2 * age") - def calculate(operation, column_name, options = {}) + def calculate(operation, column_name) if column_name.is_a?(Symbol) && attribute_alias?(column_name) column_name = attribute_alias(column_name) end if has_include?(column_name) - construct_relation_for_association_calculations.calculate(operation, column_name, options) + construct_relation_for_association_calculations.calculate(operation, column_name) else - perform_calculation(operation, column_name, options) + perform_calculation(operation, column_name) end end @@ -180,7 +180,7 @@ def has_include?(column_name) eager_loading? || (includes_values.present? && (column_name || references_eager_loaded_tables?)) end - def perform_calculation(operation, column_name, options = {}) + def perform_calculation(operation, column_name) operation = operation.to_s.downcase # If #count is used with #distinct / #uniq it is considered distinct. (eg. relation.distinct.count) diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 396d4ef1e9..03504ce0c7 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -295,7 +295,7 @@ def test_null_relation_content_size_methods def test_null_relation_calculations_methods assert_no_queries do assert_equal 0, Developer.none.count - assert_equal 0, Developer.none.calculate(:count, nil, {}) + assert_equal 0, Developer.none.calculate(:count, nil) assert_equal nil, Developer.none.calculate(:average, 'salary') end end From 625cd69a8b74a49fe2831983e89536f872c5abe7 Mon Sep 17 00:00:00 2001 From: Paul Nikitochkin Date: Sat, 12 Oct 2013 16:39:09 +0300 Subject: [PATCH 18/44] Make missed association exception message more informative Add target class name, which should have missed association on preload, into exception message to simplify detecting problem part. --- activerecord/CHANGELOG.md | 6 ++++++ activerecord/lib/active_record/associations/preloader.rb | 6 +++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 83c94caa53..661f3aed07 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* For missed association exception message + which is raised in `ActiveRecord::Associations::Preloader` class + added owner record class name in order to simplify to find problem code. + + *Paul Nikitochkin* + * `has_and_belongs_to_many` is now transparently implemented in terms of `has_many :through`. Behavior should remain the same, if not, it is a bug. diff --git a/activerecord/lib/active_record/associations/preloader.rb b/activerecord/lib/active_record/associations/preloader.rb index 713ff80d47..2393667ac8 100644 --- a/activerecord/lib/active_record/associations/preloader.rb +++ b/activerecord/lib/active_record/associations/preloader.rb @@ -153,13 +153,13 @@ def records_by_reflection(association, records) records.group_by do |record| reflection = record.class.reflect_on_association(association) - reflection || raise_config_error(association) + reflection || raise_config_error(record, association) end end - def raise_config_error(association) + def raise_config_error(record, association) raise ActiveRecord::ConfigurationError, - "Association named '#{association}' was not found; " \ + "Association named '#{association}' was not found on #{record.class.name}; " \ "perhaps you misspelled it?" end From bc293ff690e5478b99a55594f9fa8fe0e709941b Mon Sep 17 00:00:00 2001 From: Paul Nikitochkin Date: Sat, 12 Oct 2013 14:38:37 +0300 Subject: [PATCH 19/44] Generate subquery for Relation passed as array condition for where Instead of executing 2 queries for fetching records filtered by array condition with Relation, added generation of subquery to current query. This behaviour will be consistent when passes Relation as hash condition to where Closes: #12415 --- activerecord/CHANGELOG.md | 17 +++++++++++++++++ activerecord/lib/active_record/sanitization.rb | 14 ++++++++++++-- activerecord/test/cases/relations_test.rb | 5 +++++ activerecord/test/cases/sanitize_test.rb | 6 ++++++ 4 files changed, 40 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 83c94caa53..77ce30359c 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,20 @@ +* Generate subquery for `Relation` if it passed as array condition for `where` method + + Example: + # Before + Blog.where('id in (?)', Blog.where(id: 1)) + # => SELECT "blogs".* FROM "blogs" WHERE "blogs"."id" = 1 + # => SELECT "blogs".* FROM "blogs" WHERE (id IN (1)) + + # After + Blog.where('id in (?)', Blog.where(id: 1).select(:id)) + # => SELECT "blogs".* FROM "blogs" + # WHERE "blogs"."id" IN (SELECT "blogs"."id" FROM "blogs" WHERE "blogs"."id" = 1) + + Fixes: #12415 + + *Paul Nikitochkin* + * `has_and_belongs_to_many` is now transparently implemented in terms of `has_many :through`. Behavior should remain the same, if not, it is a bug. diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index 0b87ab9926..af4ea0efec 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -127,7 +127,17 @@ def replace_bind_variables(statement, values) #:nodoc: raise_if_bind_arity_mismatch(statement, statement.count('?'), values.size) bound = values.dup c = connection - statement.gsub('?') { quote_bound_value(bound.shift, c) } + statement.gsub('?') do + replace_bind_variable(bound.shift, c) + end + end + + def replace_bind_variable(value, c = connection) + if ActiveRecord::Relation === value + value.to_sql + else + quote_bound_value(value, c) + end end def replace_named_bind_variables(statement, bind_vars) #:nodoc: @@ -135,7 +145,7 @@ def replace_named_bind_variables(statement, bind_vars) #:nodoc: if $1 == ':' # skip postgresql casts $& # return the whole match elsif bind_vars.include?(match = $2.to_sym) - quote_bound_value(bind_vars[match]) + replace_bind_variable(bind_vars[match]) else raise PreparedStatementInvalid, "missing value for :#{match} in #{statement}" end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index f814947ab2..0cd8595ba6 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -626,6 +626,11 @@ def test_find_all_using_where_with_relation relation = Author.where(:id => Author.where(:id => david.id)) assert_equal [david], relation.to_a } + + assert_queries(1) { + relation = Author.where('id in (?)', Author.where(id: david).select(:id)) + assert_equal [david], relation.to_a + } end def test_find_all_using_where_with_relation_and_alternate_primary_key diff --git a/activerecord/test/cases/sanitize_test.rb b/activerecord/test/cases/sanitize_test.rb index 082570c55b..4c0762deca 100644 --- a/activerecord/test/cases/sanitize_test.rb +++ b/activerecord/test/cases/sanitize_test.rb @@ -31,4 +31,10 @@ def test_sanitize_sql_array_handles_bind_variables assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi\nand\nThumper"]) assert_equal "name=#{quoted_bambi_and_thumper}", Binary.send(:sanitize_sql_array, ["name=?", "Bambi\nand\nThumper".mb_chars]) end + + def test_sanitize_sql_array_handles_relations + assert_match(/\(\bselect\b.*?\bwhere\b.*?\)/i, + Binary.send(:sanitize_sql_array, ["id in (?)", Binary.where(id: 1)]), + "should sanitize `Relation` as subquery") + end end From 3a2093984ff49d86db1efeff0c7581e788ecfb9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Sun, 13 Oct 2013 16:54:04 -0300 Subject: [PATCH 20/44] Add nodoc to method --- activerecord/lib/active_record/sanitization.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb index af4ea0efec..cab8fd745a 100644 --- a/activerecord/lib/active_record/sanitization.rb +++ b/activerecord/lib/active_record/sanitization.rb @@ -132,7 +132,7 @@ def replace_bind_variables(statement, values) #:nodoc: end end - def replace_bind_variable(value, c = connection) + def replace_bind_variable(value, c = connection) #:nodoc: if ActiveRecord::Relation === value value.to_sql else From 05b178085a2ad3bf3f2c0dcd1ea40fb5e8c8dc0d Mon Sep 17 00:00:00 2001 From: Dmitry Polushkin Date: Fri, 1 Mar 2013 01:08:27 +0000 Subject: [PATCH 21/44] inversed instance should not be reloaded after stale state was changed check at association reader that record is inverted and should not be reloaded because of stale was changed at target record --- .../lib/active_record/associations/association.rb | 7 +++++-- .../cases/associations/inverse_associations_test.rb | 12 ++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 04c36d5740..2f33ecb71f 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -42,6 +42,7 @@ def reset @loaded = false @target = nil @stale_state = nil + @inversed = false end # Reloads the \target and returns +self+ on success. @@ -59,8 +60,9 @@ def loaded? # Asserts the \target has been loaded setting the \loaded flag to +true+. def loaded! - @loaded = true + @loaded = true @stale_state = stale_state + @inversed = false end # The target is stale if the target no longer points to the record(s) that the @@ -70,7 +72,7 @@ def loaded! # # Note that if the target has not been loaded, it is not considered stale. def stale_target? - loaded? && @stale_state != stale_state + !@inversed && loaded? && @stale_state != stale_state end # Sets the target of this association to \target, and the \loaded flag to +true+. @@ -104,6 +106,7 @@ def set_inverse_instance(record) if record && invertible_for?(record) inverse = record.association(inverse_reflection_for(record).name) inverse.target = owner + inverse.instance_variable_set(:@inversed, true) end end diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index 8c81e00865..893030345f 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -603,6 +603,18 @@ def test_child_instance_should_be_shared_with_replaced_via_method_parent assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same after changes to replaced-parent-owned instance" end + def test_inversed_instance_should_not_be_reloaded_after_stale_state_changed + new_man = Man.new + face = Face.new + new_man.face = face + + old_inversed_man = face.man + new_man.save! + new_inversed_man = face.man + + assert_equal old_inversed_man.object_id, new_inversed_man.object_id + end + def test_should_not_try_to_set_inverse_instances_when_the_inverse_is_a_has_many i = interests(:llama_wrangling) m = i.polymorphic_man From 3a6cf83b9a0d9481d9bfa87afea2155e524e0596 Mon Sep 17 00:00:00 2001 From: Dmitry Polushkin Date: Sun, 12 May 2013 22:19:19 +0100 Subject: [PATCH 22/44] add inversed accessor to association class --- activerecord/lib/active_record/associations/association.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb index 2f33ecb71f..e6a45487d0 100644 --- a/activerecord/lib/active_record/associations/association.rb +++ b/activerecord/lib/active_record/associations/association.rb @@ -17,6 +17,7 @@ module Associations # HasManyThroughAssociation + ThroughAssociation class Association #:nodoc: attr_reader :owner, :target, :reflection + attr_accessor :inversed delegate :options, :to => :reflection @@ -72,7 +73,7 @@ def loaded! # # Note that if the target has not been loaded, it is not considered stale. def stale_target? - !@inversed && loaded? && @stale_state != stale_state + !inversed && loaded? && @stale_state != stale_state end # Sets the target of this association to \target, and the \loaded flag to +true+. @@ -106,7 +107,7 @@ def set_inverse_instance(record) if record && invertible_for?(record) inverse = record.association(inverse_reflection_for(record).name) inverse.target = owner - inverse.instance_variable_set(:@inversed, true) + inverse.inversed = true end end From 04e3b41e2fc834c842ef50f25feea829aab0bbbc Mon Sep 17 00:00:00 2001 From: Dmitry Polushkin Date: Sun, 13 Oct 2013 21:54:07 +0100 Subject: [PATCH 23/44] Add a note to the changelog for #9499 --- activerecord/CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index e122b0181e..00681c13f1 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,9 @@ +* Prevent the inversed association from being reloaded on save. + + Fixes #9499. + + *Dmitry Polushkin* + * Generate subquery for `Relation` if it passed as array condition for `where` method Example: From 3430dafbc855b20c78d9ea9a105823b17fa16df3 Mon Sep 17 00:00:00 2001 From: Chad Jolly Date: Fri, 6 Sep 2013 14:01:00 -0600 Subject: [PATCH 24/44] newline at end of structure.sql file --- activerecord/lib/active_record/railties/databases.rake | 1 + 1 file changed, 1 insertion(+) diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index daccab762f..ecadb95a5d 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -287,6 +287,7 @@ db_namespace = namespace :db do if ActiveRecord::Base.connection.supports_migrations? File.open(filename, "a") do |f| f.puts ActiveRecord::Base.connection.dump_schema_information + f.print "\n" end end db_namespace['structure:dump'].reenable From 9541a72858989acfd482a9b7775c6eac10f75c02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 14 Oct 2013 00:56:30 -0300 Subject: [PATCH 25/44] Dump the default function when the primary key is uuid Fixes #12489 --- activerecord/lib/active_record/schema_dumper.rb | 1 + activerecord/test/cases/adapters/postgresql/uuid_test.rb | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 8986d255cd..811babfc31 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -123,6 +123,7 @@ def table(table, stream) tbl.print %Q(, primary_key: "#{pk}") elsif pkcol.sql_type == 'uuid' tbl.print ", id: :uuid" + tbl.print %Q(, default: "#{pkcol.default_function}") if pkcol.respond_to?(:default_function) && pkcol.default_function end else tbl.print ", id: false" diff --git a/activerecord/test/cases/adapters/postgresql/uuid_test.rb b/activerecord/test/cases/adapters/postgresql/uuid_test.rb index 0cd5b420fc..a753a23c09 100644 --- a/activerecord/test/cases/adapters/postgresql/uuid_test.rb +++ b/activerecord/test/cases/adapters/postgresql/uuid_test.rb @@ -24,7 +24,7 @@ def setup @connection.reconnect! @connection.transaction do - @connection.create_table('pg_uuids', id: :uuid) do |t| + @connection.create_table('pg_uuids', id: :uuid, default: 'uuid_generate_v1()') do |t| t.string 'name' t.uuid 'other_uuid', default: 'uuid_generate_v4()' end @@ -60,7 +60,7 @@ def test_pk_and_sequence_for_uuid_primary_key def test_schema_dumper_for_uuid_primary_key schema = StringIO.new ActiveRecord::SchemaDumper.dump(@connection, schema) - assert_match(/\bcreate_table "pg_uuids", id: :uuid\b/, schema.string) + assert_match(/\bcreate_table "pg_uuids", id: :uuid, default: "uuid_generate_v1\(\)"/, schema.string) assert_match(/t\.uuid "other_uuid", default: "uuid_generate_v4\(\)"/, schema.string) end end From c921db9484b9c2d0b791d5a174371b788be8b6e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 14 Oct 2013 01:02:41 -0300 Subject: [PATCH 26/44] Push default_function to superclass to avoid method check --- .../connection_adapters/column.rb | 23 ++++++++++--------- .../connection_adapters/postgresql_adapter.rb | 8 ++++--- .../lib/active_record/schema_dumper.rb | 2 +- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/column.rb b/activerecord/lib/active_record/connection_adapters/column.rb index 2596c221bc..f2fbd5a8f2 100644 --- a/activerecord/lib/active_record/connection_adapters/column.rb +++ b/activerecord/lib/active_record/connection_adapters/column.rb @@ -13,7 +13,7 @@ module Format ISO_DATETIME = /\A(\d{4})-(\d\d)-(\d\d) (\d\d):(\d\d):(\d\d)(\.\d+)?\z/ end - attr_reader :name, :default, :type, :limit, :null, :sql_type, :precision, :scale + attr_reader :name, :default, :type, :limit, :null, :sql_type, :precision, :scale, :default_function attr_accessor :primary, :coder alias :encoded? :coder @@ -27,16 +27,17 @@ module Format # It will be mapped to one of the standard Rails SQL types in the type attribute. # +null+ determines if this column allows +NULL+ values. def initialize(name, default, sql_type = nil, null = true) - @name = name - @sql_type = sql_type - @null = null - @limit = extract_limit(sql_type) - @precision = extract_precision(sql_type) - @scale = extract_scale(sql_type) - @type = simplified_type(sql_type) - @default = extract_default(default) - @primary = nil - @coder = nil + @name = name + @sql_type = sql_type + @null = null + @limit = extract_limit(sql_type) + @precision = extract_precision(sql_type) + @scale = extract_scale(sql_type) + @type = simplified_type(sql_type) + @default = extract_default(default) + @default_function = nil + @primary = nil + @coder = nil end # Returns +true+ if the column is either of type string or text. diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0c4d005e63..1b4ef92b8a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -45,12 +45,12 @@ def postgresql_connection(config) module ConnectionAdapters # PostgreSQL-specific extensions to column definitions in a table. class PostgreSQLColumn < Column #:nodoc: - attr_accessor :array, :default_function + attr_accessor :array # Instantiates a new PostgreSQL column definition in a table. def initialize(name, default, oid_type, sql_type = nil, null = true) @oid_type = oid_type default_value = self.class.extract_value_from_default(default) - @default_function = default if !default_value && default && default =~ /.+\(.*\)/ + if sql_type =~ /\[\]$/ @array = true super(name, default_value, sql_type[0..sql_type.length - 3], null) @@ -58,6 +58,8 @@ def initialize(name, default, oid_type, sql_type = nil, null = true) @array = false super(name, default_value, sql_type, null) end + + @default_function = default if !default_value && default && default =~ /.+\(.*\)/ end # :stopdoc: @@ -442,7 +444,7 @@ def adapter_name def prepare_column_options(column, types) spec = super spec[:array] = 'true' if column.respond_to?(:array) && column.array - spec[:default] = "\"#{column.default_function}\"" if column.respond_to?(:default_function) && column.default_function + spec[:default] = "\"#{column.default_function}\"" if column.default_function spec end diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 811babfc31..e055d571ab 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -123,7 +123,7 @@ def table(table, stream) tbl.print %Q(, primary_key: "#{pk}") elsif pkcol.sql_type == 'uuid' tbl.print ", id: :uuid" - tbl.print %Q(, default: "#{pkcol.default_function}") if pkcol.respond_to?(:default_function) && pkcol.default_function + tbl.print %Q(, default: "#{pkcol.default_function}") if pkcol.default_function end else tbl.print ", id: false" From 5b144233e9ac3405b029d296a5f231b4caa73859 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafael=20Mendon=C3=A7a=20Fran=C3=A7a?= Date: Mon, 14 Oct 2013 01:25:59 -0300 Subject: [PATCH 27/44] Extract a function to determine if the default value is a function --- .../active_record/connection_adapters/postgresql_adapter.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 1b4ef92b8a..771a150eae 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -59,7 +59,7 @@ def initialize(name, default, oid_type, sql_type = nil, null = true) super(name, default_value, sql_type, null) end - @default_function = default if !default_value && default && default =~ /.+\(.*\)/ + @default_function = default if has_default_function?(default_value, default) end # :stopdoc: @@ -150,6 +150,10 @@ def type_cast(value) private + def has_default_function?(default_value, default) + !default_value && (%r{\w+(.*)} === default) + end + def extract_limit(sql_type) case sql_type when /^bigint/i; 8 From a11ddbe55fd27f38e0085ee1210947e3d8f47220 Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 14 Oct 2013 09:47:21 +0200 Subject: [PATCH 28/44] cleanup changelog entry format. [ci skip] --- activerecord/CHANGELOG.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index e122b0181e..4e6d7900ff 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,6 +1,8 @@ -* Generate subquery for `Relation` if it passed as array condition for `where` method +* Generate subquery for `Relation` if it passed as array condition for `where` + method. Example: + # Before Blog.where('id in (?)', Blog.where(id: 1)) # => SELECT "blogs".* FROM "blogs" WHERE "blogs"."id" = 1 @@ -11,7 +13,7 @@ # => SELECT "blogs".* FROM "blogs" # WHERE "blogs"."id" IN (SELECT "blogs"."id" FROM "blogs" WHERE "blogs"."id" = 1) - Fixes: #12415 + Fixes #12415. *Paul Nikitochkin* From 4f381734e22cbb59498fb8c779b7ca253e9fc23b Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Mon, 14 Oct 2013 20:54:29 +0530 Subject: [PATCH 29/44] Remove `default_primary_key_type` and extract contains of `native_database_types` to a constant since they aren't conditional now in SQLite3Adapter. Makes it more like other adapters. --- .../connection_adapters/sqlite3_adapter.rb | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index af38514e43..e5ad08b6b0 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -55,6 +55,21 @@ def binary_to_string(value) class SQLite3Adapter < AbstractAdapter include Savepoints + NATIVE_DATABASE_TYPES = { + primary_key: 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL', + string: { name: "varchar", limit: 255 }, + text: { name: "text" }, + integer: { name: "integer" }, + float: { name: "float" }, + decimal: { name: "decimal" }, + datetime: { name: "datetime" }, + timestamp: { name: "datetime" }, + time: { name: "time" }, + date: { name: "date" }, + binary: { name: "blob" }, + boolean: { name: "boolean" } + } + class Version include Comparable @@ -195,20 +210,7 @@ def allowed_index_name_length end def native_database_types #:nodoc: - { - :primary_key => default_primary_key_type, - :string => { :name => "varchar", :limit => 255 }, - :text => { :name => "text" }, - :integer => { :name => "integer" }, - :float => { :name => "float" }, - :decimal => { :name => "decimal" }, - :datetime => { :name => "datetime" }, - :timestamp => { :name => "datetime" }, - :time => { :name => "time" }, - :date => { :name => "date" }, - :binary => { :name => "blob" }, - :boolean => { :name => "boolean" } - } + NATIVE_DATABASE_TYPES end # Returns the current database encoding format as a string, eg: 'UTF-8' @@ -591,10 +593,6 @@ def sqlite_version @sqlite_version ||= SQLite3Adapter::Version.new(select_value('select sqlite_version(*)')) end - def default_primary_key_type - 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL' - end - def translate_exception(exception, message) case exception.message when /column(s)? .* (is|are) not unique/ From cd9592959f9f16b228ac3f4bc6fc92932253f8dc Mon Sep 17 00:00:00 2001 From: Neeraj Singh Date: Thu, 9 May 2013 13:08:44 -0400 Subject: [PATCH 30/44] scope_chain should not be mutated for other reflections Currently `scope_chain` uses same array for building different `scope_chain` for different associations. During processing these arrays are sometimes mutated and because of in-place mutation the changed `scope_chain` impacts other reflections. Fix is to dup the value before adding to the `scope_chain`. Fixes #3882. --- activerecord/CHANGELOG.md | 13 +++++++++++++ activerecord/lib/active_record/reflection.rb | 2 +- activerecord/test/cases/reflection_test.rb | 16 ++++++++++++++++ activerecord/test/models/cake_designer.rb | 3 +++ activerecord/test/models/chef.rb | 3 +++ activerecord/test/models/department.rb | 4 ++++ activerecord/test/models/drink_designer.rb | 3 +++ activerecord/test/models/hotel.rb | 6 ++++++ activerecord/test/schema/schema.rb | 16 ++++++++++++++++ 9 files changed, 65 insertions(+), 1 deletion(-) create mode 100644 activerecord/test/models/cake_designer.rb create mode 100644 activerecord/test/models/chef.rb create mode 100644 activerecord/test/models/department.rb create mode 100644 activerecord/test/models/drink_designer.rb create mode 100644 activerecord/test/models/hotel.rb diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 90b5cb3b8f..0b7fbe5a48 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,16 @@ +* `scope_chain` should not be mutated for other reflections. + + Currently `scope_chain` uses same array for building different + `scope_chain` for different associations. During processing + these arrays are sometimes mutated and because of in-place + mutation the changed `scope_chain` impacts other reflections. + + Fix is to dup the value before adding to the `scope_chain`. + + Fixes #3882. + + *Neeraj Singh* + * Prevent the inversed association from being reloaded on save. Fixes #9499. diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index e88c5d17cb..bce7766501 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -574,7 +574,7 @@ def scope_chain # Add to it the scope from this reflection (if any) scope_chain.first << scope if scope - through_scope_chain = through_reflection.scope_chain + through_scope_chain = through_reflection.scope_chain.map(&:dup) if options[:source_type] through_scope_chain.first << diff --git a/activerecord/test/cases/reflection_test.rb b/activerecord/test/cases/reflection_test.rb index 0e5c7df2cc..d7ad5ed29f 100644 --- a/activerecord/test/cases/reflection_test.rb +++ b/activerecord/test/cases/reflection_test.rb @@ -18,6 +18,11 @@ require 'models/tag' require 'models/sponsor' require 'models/edge' +require 'models/hotel' +require 'models/chef' +require 'models/department' +require 'models/cake_designer' +require 'models/drink_designer' class ReflectionTest < ActiveRecord::TestCase include ActiveRecord::Reflection @@ -227,6 +232,17 @@ def test_scope_chain assert_equal expected, actual end + def test_scope_chain_does_not_interfere_with_hmt_with_polymorphic_case + @hotel = Hotel.create! + @department = @hotel.departments.create! + @department.chefs.create!(employable: CakeDesigner.create!) + @department.chefs.create!(employable: DrinkDesigner.create!) + + assert_equal 1, @hotel.cake_designers.size + assert_equal 1, @hotel.drink_designers.size + assert_equal 2, @hotel.chefs.size + end + def test_nested? assert !Author.reflect_on_association(:comments).nested? assert Author.reflect_on_association(:tags).nested? diff --git a/activerecord/test/models/cake_designer.rb b/activerecord/test/models/cake_designer.rb new file mode 100644 index 0000000000..9c57ef573a --- /dev/null +++ b/activerecord/test/models/cake_designer.rb @@ -0,0 +1,3 @@ +class CakeDesigner < ActiveRecord::Base + has_one :chef, as: :employable +end diff --git a/activerecord/test/models/chef.rb b/activerecord/test/models/chef.rb new file mode 100644 index 0000000000..67a4e54f06 --- /dev/null +++ b/activerecord/test/models/chef.rb @@ -0,0 +1,3 @@ +class Chef < ActiveRecord::Base + belongs_to :employable, polymorphic: true +end diff --git a/activerecord/test/models/department.rb b/activerecord/test/models/department.rb new file mode 100644 index 0000000000..08004a0ed3 --- /dev/null +++ b/activerecord/test/models/department.rb @@ -0,0 +1,4 @@ +class Department < ActiveRecord::Base + has_many :chefs + belongs_to :hotel +end diff --git a/activerecord/test/models/drink_designer.rb b/activerecord/test/models/drink_designer.rb new file mode 100644 index 0000000000..2db968ef11 --- /dev/null +++ b/activerecord/test/models/drink_designer.rb @@ -0,0 +1,3 @@ +class DrinkDesigner < ActiveRecord::Base + has_one :chef, as: :employable +end diff --git a/activerecord/test/models/hotel.rb b/activerecord/test/models/hotel.rb new file mode 100644 index 0000000000..b352cd22f3 --- /dev/null +++ b/activerecord/test/models/hotel.rb @@ -0,0 +1,6 @@ +class Hotel < ActiveRecord::Base + has_many :departments + has_many :chefs, through: :departments + has_many :cake_designers, source_type: 'CakeDesigner', source: :employable, through: :chefs + has_many :drink_designers, source_type: 'DrinkDesigner', source: :employable, through: :chefs +end diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 75711673a7..88a686d436 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -787,6 +787,22 @@ def create_table(*args, &block) t.string 'from' end + create_table :hotels, force: true do |t| + end + create_table :departments, force: true do |t| + t.integer :hotel_id + end + create_table :cake_designers, force: true do |t| + end + create_table :drink_designers, force: true do |t| + end + create_table :chefs, force: true do |t| + t.integer :employable_id + t.string :employable_type + t.integer :department_id + end + + except 'SQLite' do # fk_test_has_fk should be before fk_test_has_pk create_table :fk_test_has_fk, :force => true do |t| From ec620e0984ce3159dc4c2fc8626982be3cfaca2a Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Mon, 14 Oct 2013 21:52:25 +0530 Subject: [PATCH 31/44] `$SAFE = 4;` has been removed with Ruby 2.1 For background - https://bugs.ruby-lang.org/issues/8468 Changset - https://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/41259/diff/test/ruby/test_thread.rb --- activesupport/test/core_ext/thread_test.rb | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/activesupport/test/core_ext/thread_test.rb b/activesupport/test/core_ext/thread_test.rb index 54d2dcd8dd..6a7c6e0604 100644 --- a/activesupport/test/core_ext/thread_test.rb +++ b/activesupport/test/core_ext/thread_test.rb @@ -72,17 +72,4 @@ def test_thread_variable_frozen_after_set end end - def test_thread_variable_security - rubinius_skip "$SAFE is not supported on Rubinius." - - t = Thread.new { sleep } - - assert_raises(SecurityError) do - Thread.new { $SAFE = 4; t.thread_variable_get(:foo) }.join - end - - assert_raises(SecurityError) do - Thread.new { $SAFE = 4; t.thread_variable_set(:foo, :baz) }.join - end - end end From 67c2525d59f8136c648bc59c5c39dbab77713fe2 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Mon, 14 Oct 2013 23:52:06 +0530 Subject: [PATCH 32/44] Minor Refactoring to `NumberHelper#number_to_human` * Use destructive `map` and `sort_by` to save extra object creation. * Create `INVERTED_DECIMAL_UNITS.invert` constant instead of repeatedly doing `DECIMAL_UNITS.invert` --- activesupport/lib/active_support/number_helper.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index c9c0eff2bf..e0151baa36 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -108,7 +108,7 @@ module NumberHelper DECIMAL_UNITS = { 0 => :unit, 1 => :ten, 2 => :hundred, 3 => :thousand, 6 => :million, 9 => :billion, 12 => :trillion, 15 => :quadrillion, -1 => :deci, -2 => :centi, -3 => :mili, -6 => :micro, -9 => :nano, -12 => :pico, -15 => :femto } - + INVERTED_DECIMAL_UNITS = DECIMAL_UNITS.invert STORAGE_UNITS = [:byte, :kb, :mb, :gb, :tb] # Formats a +number+ into a US phone number (e.g., (555) @@ -561,8 +561,6 @@ def number_to_human(number, options = {}) #for backwards compatibility with those that didn't add strip_insignificant_zeros to their locale files options[:strip_insignificant_zeros] = true if not options.key?(:strip_insignificant_zeros) - inverted_du = DECIMAL_UNITS.invert - units = options.delete :units unit_exponents = case units when Hash @@ -573,7 +571,7 @@ def number_to_human(number, options = {}) translate_number_value_with_default("human.decimal_units.units", :locale => options[:locale], :raise => true) else raise ArgumentError, ":units must be a Hash or String translation scope." - end.keys.map{|e_name| inverted_du[e_name] }.sort_by{|e| -e} + end.keys.map!{|e_name| INVERTED_DECIMAL_UNITS[e_name] }.sort_by!{|e| -e} number_exponent = number != 0 ? Math.log10(number.abs).floor : 0 display_exponent = unit_exponents.find{ |e| number_exponent >= e } || 0 From 8b14a6b76898e34ceb6da7e683155dfd3f8c85c0 Mon Sep 17 00:00:00 2001 From: Vipul A M Date: Mon, 14 Oct 2013 22:40:21 +0530 Subject: [PATCH 33/44] Fix `singleton_class?` Due to changes from http://bugs.ruby-lang.org/projects/ruby-trunk/repository/revisions/39628 current `singleton_class?` implementation fails. Changed based on reference from http://bugs.ruby-lang.org/issues/7609 --- .../lib/active_support/core_ext/class/attribute.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/activesupport/lib/active_support/core_ext/class/attribute.rb b/activesupport/lib/active_support/core_ext/class/attribute.rb index 83038f9da5..f2a221c396 100644 --- a/activesupport/lib/active_support/core_ext/class/attribute.rb +++ b/activesupport/lib/active_support/core_ext/class/attribute.rb @@ -118,7 +118,10 @@ def class_attribute(*attrs) end private - def singleton_class? - ancestors.first != self + + unless respond_to?(:singleton_class?) + def singleton_class? + ancestors.first != self + end end end From a94186f836044d5ddaf9b185e01a99dc1c2569f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C4=B1tk=C4=B1=20Ba=C4=9Fdat?= Date: Mon, 14 Oct 2013 22:14:30 +0300 Subject: [PATCH 34/44] Remove size alias for length validation Removed ```The `size` helper is an alias for `length`.``` line. If you use this "nonexist" helper, you will get an error message like this: ``` ArgumentError: Unknown validator: 'SizeValidator' ... ``` Maybe wanted to mean ```validates_size_of``` helper as an alias for ```validates_length_of``` helper. --- guides/source/active_record_validations.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/guides/source/active_record_validations.md b/guides/source/active_record_validations.md index 797b996357..0df52a655f 100644 --- a/guides/source/active_record_validations.md +++ b/guides/source/active_record_validations.md @@ -438,8 +438,6 @@ provide a personalized message or use `presence: true` instead. When `:in` or `:within` have a lower limit of 1, you should either provide a personalized message or call `presence` prior to `length`. -The `size` helper is an alias for `length`. - ### `numericality` This helper validates that your attributes have only numeric values. By From 19b871a0902c4ec3e460a38f41583a7855edd81a Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 14 Oct 2013 13:04:57 -0700 Subject: [PATCH 35/44] only calculate offset index once. #12537 --- .../preloader/through_association.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 3166df57eb..63d4253e6a 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -35,6 +35,15 @@ def associated_records_by_owner(preloader) } end + pl_indexes = preloaders.each_with_object({}) do |pl,hash| + i = 0 + loaded_records = pl.preloaded_records + hash[pl] = loaded_records.each_with_object({}) { |r,indexes| + indexes[r] = i + i += 1 + } + end + through_records.each_with_object({}) { |(lhs,center),records_by_owner| pl_to_middle = center.group_by { |record| middle_to_pl[record] } @@ -43,12 +52,7 @@ def associated_records_by_owner(preloader) r.send(source_reflection.name) }.compact - loaded_records = pl.preloaded_records - i = 0 - record_index = loaded_records.each_with_object({}) { |r,indexes| - indexes[r] = i - i += 1 - } + record_index = pl_indexes[pl] records = rhs_records.sort_by { |rhs| record_index[rhs] } @preloaded_records.concat rhs_records records From 43a63f6d5ec2026dc7c610394aab37613181fff1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 14 Oct 2013 13:23:03 -0700 Subject: [PATCH 36/44] the preloader for the RHS has all the preloaded records, so ask it --- .../associations/preloader/through_association.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 63d4253e6a..edce1d80c5 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -29,6 +29,8 @@ def associated_records_by_owner(preloader) source_reflection.name, reflection_scope) + @preloaded_records.concat preloaders.flat_map(&:preloaded_records) + middle_to_pl = preloaders.each_with_object({}) do |pl,h| pl.owners.each { |middle| h[middle] = pl @@ -53,9 +55,7 @@ def associated_records_by_owner(preloader) }.compact record_index = pl_indexes[pl] - records = rhs_records.sort_by { |rhs| record_index[rhs] } - @preloaded_records.concat rhs_records - records + rhs_records.sort_by { |rhs| record_index[rhs] } end } end From 03c98bddfd22d8df9daa0ea74f04f1ee72cddf89 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 14 Oct 2013 13:27:52 -0700 Subject: [PATCH 37/44] simplify populating the ordering hash --- .../associations/preloader/through_association.rb | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index edce1d80c5..21f2fb7cc7 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -29,7 +29,7 @@ def associated_records_by_owner(preloader) source_reflection.name, reflection_scope) - @preloaded_records.concat preloaders.flat_map(&:preloaded_records) + @preloaded_records = preloaders.flat_map(&:preloaded_records) middle_to_pl = preloaders.each_with_object({}) do |pl,h| pl.owners.each { |middle| @@ -37,13 +37,9 @@ def associated_records_by_owner(preloader) } end - pl_indexes = preloaders.each_with_object({}) do |pl,hash| - i = 0 - loaded_records = pl.preloaded_records - hash[pl] = loaded_records.each_with_object({}) { |r,indexes| - indexes[r] = i - i += 1 - } + record_offset = {} + @preloaded_records.each_with_index do |record,i| + record_offset[record] = i end through_records.each_with_object({}) { |(lhs,center),records_by_owner| @@ -54,8 +50,7 @@ def associated_records_by_owner(preloader) r.send(source_reflection.name) }.compact - record_index = pl_indexes[pl] - rhs_records.sort_by { |rhs| record_index[rhs] } + rhs_records.sort_by { |rhs| record_offset[rhs] } end } end From e95bc57208ed5a88ce30b92fb6a5ff4969a6de02 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 14 Oct 2013 15:27:31 -0700 Subject: [PATCH 38/44] we should have unique sponsorable ids in the fixtures at least --- activerecord/test/fixtures/sponsors.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/test/fixtures/sponsors.yml b/activerecord/test/fixtures/sponsors.yml index bfc6b238b1..2da541c539 100644 --- a/activerecord/test/fixtures/sponsors.yml +++ b/activerecord/test/fixtures/sponsors.yml @@ -8,5 +8,5 @@ boring_club_sponsor_for_groucho: sponsorable_type: Member crazy_club_sponsor_for_groucho: sponsor_club: crazy_club - sponsorable_id: 2 + sponsorable_id: 3 sponsorable_type: Member From 396b1951fc91e5aecf499ca5602995f61cda59a3 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 14 Oct 2013 15:49:08 -0700 Subject: [PATCH 39/44] read the association instead of sending --- .../associations/preloader/through_association.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations/preloader/through_association.rb b/activerecord/lib/active_record/associations/preloader/through_association.rb index 21f2fb7cc7..2a8530af62 100644 --- a/activerecord/lib/active_record/associations/preloader/through_association.rb +++ b/activerecord/lib/active_record/associations/preloader/through_association.rb @@ -47,7 +47,9 @@ def associated_records_by_owner(preloader) records_by_owner[lhs] = pl_to_middle.flat_map do |pl, middles| rhs_records = middles.flat_map { |r| - r.send(source_reflection.name) + association = r.association source_reflection.name + + association.reader }.compact rhs_records.sort_by { |rhs| record_offset[rhs] } From 4f642dcb39cea155ddb50978a38fb3a2f2290ea3 Mon Sep 17 00:00:00 2001 From: yalab Date: Sat, 13 Jul 2013 14:38:00 +0900 Subject: [PATCH 40/44] Added --model-name option scaffold_controller_generator. --- railties/CHANGELOG.md | 4 +++ .../scaffold_controller_generator.rb | 2 +- .../lib/rails/generators/resource_helpers.rb | 25 +++++++++++++++---- railties/test/generators/named_base_test.rb | 19 ++++++++++++++ .../scaffold_controller_generator_test.rb | 9 +++++++ 5 files changed, 53 insertions(+), 6 deletions(-) diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index 4e6bf899af..8a0e0ff3f6 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,7 @@ +* Added `--model-name` scaffld\_controller\_generator option. + + *yalab* + * Expose MiddlewareStack#unshift to environment configuration. *Ben Pickles* diff --git a/railties/lib/rails/generators/rails/scaffold_controller/scaffold_controller_generator.rb b/railties/lib/rails/generators/rails/scaffold_controller/scaffold_controller_generator.rb index 4f36b612ae..6bf0a33a5f 100644 --- a/railties/lib/rails/generators/rails/scaffold_controller/scaffold_controller_generator.rb +++ b/railties/lib/rails/generators/rails/scaffold_controller/scaffold_controller_generator.rb @@ -13,7 +13,7 @@ class ScaffoldControllerGenerator < NamedBase # :nodoc: argument :attributes, type: :array, default: [], banner: "field:type field:type" def create_controller_files - template "controller.rb", File.join('app/controllers', class_path, "#{controller_file_name}_controller.rb") + template "controller.rb", File.join('app/controllers', controller_class_path, "#{controller_file_name}_controller.rb") end hook_for :template_engine, :test_framework, as: :scaffold diff --git a/railties/lib/rails/generators/resource_helpers.rb b/railties/lib/rails/generators/resource_helpers.rb index 7fd5c00768..a01eb57651 100644 --- a/railties/lib/rails/generators/resource_helpers.rb +++ b/railties/lib/rails/generators/resource_helpers.rb @@ -9,11 +9,19 @@ module ResourceHelpers # :nodoc: def self.included(base) #:nodoc: base.class_option :force_plural, type: :boolean, desc: "Forces the use of a plural ModelName" + base.class_option :model_name, type: :string, desc: "ModelName to be used" end # Set controller variables on initialization. def initialize(*args) #:nodoc: super + if options[:model_name] + controller_name = name + self.name = options[:model_name] + assign_names!(self.name) + else + controller_name = name + end if name == name.pluralize && name.singularize != name.pluralize && !options[:force_plural] unless ResourceHelpers.skip_warn @@ -24,19 +32,26 @@ def initialize(*args) #:nodoc: assign_names!(name) end - @controller_name = name.pluralize + assign_controller_names!(controller_name.pluralize) end protected - attr_reader :controller_name + attr_reader :controller_name, :controller_file_name def controller_class_path - class_path + if options[:model_name] + @controller_class_path + else + class_path + end end - def controller_file_name - @controller_file_name ||= file_name.pluralize + def assign_controller_names!(name) + @controller_name = name + @controller_class_path = name.include?('/') ? name.split('/') : name.split('::') + @controller_class_path.map! { |m| m.underscore } + @controller_file_name = @controller_class_path.pop end def controller_file_path diff --git a/railties/test/generators/named_base_test.rb b/railties/test/generators/named_base_test.rb index 2bc2c33a72..ac5cfff229 100644 --- a/railties/test/generators/named_base_test.rb +++ b/railties/test/generators/named_base_test.rb @@ -117,6 +117,25 @@ def test_hide_namespace assert Rails::Generators.hidden_namespaces.include?('hidden') end + def test_scaffold_plural_names_with_model_name_option + g = generator ['Admin::Foo'], model_name: 'User' + assert_name g, 'user', :singular_name + assert_name g, 'User', :name + assert_name g, 'user', :file_path + assert_name g, 'User', :class_name + assert_name g, 'user', :file_name + assert_name g, 'User', :human_name + assert_name g, 'users', :plural_name + assert_name g, 'user', :i18n_scope + assert_name g, 'users', :table_name + assert_name g, 'Admin::Foos', :controller_name + assert_name g, %w(admin), :controller_class_path + assert_name g, 'Admin::Foos', :controller_class_name + assert_name g, 'admin/foos', :controller_file_path + assert_name g, 'foos', :controller_file_name + assert_name g, 'admin.foos', :controller_i18n_scope + end + protected def assert_name(generator, value, method) diff --git a/railties/test/generators/scaffold_controller_generator_test.rb b/railties/test/generators/scaffold_controller_generator_test.rb index 013cb78252..26e56a162c 100644 --- a/railties/test/generators/scaffold_controller_generator_test.rb +++ b/railties/test/generators/scaffold_controller_generator_test.rb @@ -166,4 +166,13 @@ def test_new_hash_style assert_match(/render action: 'new'/, content) end end + + def test_model_name_option + run_generator ["Admin::User", "--model-name=User"] + assert_file "app/controllers/admin/users_controller.rb" do |content| + assert_instance_method :index, content do |m| + assert_match("@users = User.all", m) + end + end + end end From 8581953e5b4907001384815f8787f38c832c34df Mon Sep 17 00:00:00 2001 From: Kir Shatrov Date: Mon, 14 Oct 2013 14:25:44 +0200 Subject: [PATCH 41/44] Prepare generated Gemfile for Capistrano 3 Capistrano 3 is now rails-less, so we need to encourage developers to use capistrano-rails gem. More details: http://www.capistranorb.com/documentation/frameworks/ruby-on-rails/ [ci skip] --- railties/lib/rails/generators/rails/app/templates/Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index edc76e6c34..4048930c8d 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -25,7 +25,7 @@ end # gem 'unicorn' # Use Capistrano for deployment -# gem 'capistrano', group: :development +# gem 'capistrano-rails', group: :development <% unless defined?(JRUBY_VERSION) -%> # Use debugger From 6d6300bcad230b78b2fcc6df6b993454fa0f389d Mon Sep 17 00:00:00 2001 From: Dmitry Vorotilin Date: Tue, 15 Oct 2013 15:00:06 +0400 Subject: [PATCH 42/44] Add missed require making `enable_warnings` available --- activesupport/lib/active_support/dependencies.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index df13fef0a4..19d4ff51d7 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -8,6 +8,7 @@ require 'active_support/core_ext/module/anonymous' require 'active_support/core_ext/module/qualified_const' require 'active_support/core_ext/object/blank' +require 'active_support/core_ext/kernel/reporting' require 'active_support/core_ext/load_error' require 'active_support/core_ext/name_error' require 'active_support/core_ext/string/starts_ends_with' From e9a1ecd583f3993c696a1cc95e182dcbe346553b Mon Sep 17 00:00:00 2001 From: Vijay Dev Date: Tue, 15 Oct 2013 23:27:34 +0530 Subject: [PATCH 43/44] Revert "fixed a doc bug in the CHANGELOG.md s/does no longer depend on/no longer depends on/" This reverts commit 49f513c25415f94452af1bb8fe3f55f090a6d0f2. Reason: Changelogs aren't supposed to be edited in docrails. --- activerecord/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index dc549240c2..810a2c668d 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -519,7 +519,7 @@ *Neeraj Singh* -* Fixture setup no longer depends on `ActiveRecord::Base.configurations`. +* Fixture setup does no longer depend on `ActiveRecord::Base.configurations`. This is relevant when `ENV["DATABASE_URL"]` is used in place of a `database.yml`. *Yves Senn* From e3e3851ed662fc55ee88c16a3cd9bb87f24e6bd6 Mon Sep 17 00:00:00 2001 From: Vijay Dev Date: Tue, 15 Oct 2013 23:34:49 +0530 Subject: [PATCH 44/44] grammar fix (reverted in e9a1ecd) --- activerecord/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 0b7fbe5a48..e1eb3e4113 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -588,7 +588,7 @@ *Neeraj Singh* -* Fixture setup does no longer depend on `ActiveRecord::Base.configurations`. +* Fixture setup no longer depends on `ActiveRecord::Base.configurations`. This is relevant when `ENV["DATABASE_URL"]` is used in place of a `database.yml`. *Yves Senn*