From 0c527026308627652c42ed8a6f26a21d3bbc7a5b Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Wed, 1 Aug 2012 19:11:46 +0100 Subject: [PATCH] Revert "Deprecate :finder_sql, :counter_sql, :insert_sql, :delete_sql." This reverts commit a79bfa92e7bdc31b346d13ee5447d3fdac382bfb. Conflicts: activerecord/CHANGELOG.md We shouldn't introducing deprecations in point releases. It will be deprecated in 4.0 instead. --- activerecord/CHANGELOG.md | 3 ++ .../builder/collection_association.rb | 11 ------ .../builder/has_and_belongs_to_many.rb | 10 ------ ...s_and_belongs_to_many_associations_test.rb | 27 ++++---------- .../has_many_associations_test.rb | 22 ++---------- activerecord/test/models/company.rb | 36 +++++++++---------- activerecord/test/models/company_in_module.rb | 4 +-- activerecord/test/models/project.rb | 22 +++++------- 8 files changed, 38 insertions(+), 97 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 8da51a64c3e34..63baa7a389e22 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,8 @@ ## Rails 3.2.8 ## +* Reverted the deprecation of `*_sql` association options. They will + be deprecated in 4.0 instead. + * Do not eager load AR session store. ActiveRecord::SessionStore depends on the abstract store in Action Pack. Eager loading this class would break client code that eager loads Active Record standalone. Fixes #7160 *Xavier Noria* * Do not set RAILS_ENV to "development" when using `db:test:prepare` and related rake tasks. diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index f67fe17260507..35f9a3ae8e0d9 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -1,5 +1,3 @@ -require 'active_support/deprecation' - module ActiveRecord::Associations::Builder class CollectionAssociation < Association #:nodoc: CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] @@ -21,7 +19,6 @@ def initialize(model, name, options, &extension) end def build - show_deprecation_warnings wrap_block_extension reflection = super CALLBACKS.each { |callback_name| define_callback(callback_name) } @@ -32,14 +29,6 @@ def writable? true end - def show_deprecation_warnings - [:finder_sql, :counter_sql].each do |name| - if options.include? name - ActiveSupport::Deprecation.warn("The :#{name} association option is deprecated. Please find an alternative (such as using scopes).") - end - end - end - private def wrap_block_extension diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index b997b2c3cf0a4..0b634ab944f73 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -11,16 +11,6 @@ def build reflection end - def show_deprecation_warnings - super - - [:delete_sql, :insert_sql].each do |name| - if options.include? name - ActiveSupport::Deprecation.warn("The :#{name} association option is deprecated. Please find an alternative (such as using has_many :through).") - end - end - end - private def define_destroy_hook diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index dc7d10452451b..bacab8d2f237f 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -67,15 +67,12 @@ class DeveloperWithSymbolsForKeys < ActiveRecord::Base class DeveloperWithCounterSQL < ActiveRecord::Base self.table_name = 'developers' - - ActiveSupport::Deprecation.silence do - has_and_belongs_to_many :projects, - :class_name => "DeveloperWithCounterSQL", - :join_table => "developers_projects", - :association_foreign_key => "project_id", - :foreign_key => "developer_id", - :counter_sql => proc { "SELECT COUNT(*) AS count_all FROM projects INNER JOIN developers_projects ON projects.id = developers_projects.project_id WHERE developers_projects.developer_id =#{id}" } - end + has_and_belongs_to_many :projects, + :class_name => "DeveloperWithCounterSQL", + :join_table => "developers_projects", + :association_foreign_key => "project_id", + :foreign_key => "developer_id", + :counter_sql => proc { "SELECT COUNT(*) AS count_all FROM projects INNER JOIN developers_projects ON projects.id = developers_projects.project_id WHERE developers_projects.developer_id =#{id}" } end class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase @@ -857,16 +854,4 @@ def test_include_method_in_has_and_belongs_to_many_association_should_return_tru developer = project.developers.build assert project.developers.include?(developer) end - - test ":insert_sql is deprecated" do - klass = Class.new(ActiveRecord::Base) - def klass.name; 'Foo'; end - assert_deprecated { klass.has_and_belongs_to_many :posts, :insert_sql => 'lol' } - end - - test ":delete_sql is deprecated" do - klass = Class.new(ActiveRecord::Base) - def klass.name; 'Foo'; end - assert_deprecated { klass.has_and_belongs_to_many :posts, :delete_sql => 'lol' } - end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 94fc48276f5f2..a51ce261178fe 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -22,9 +22,7 @@ class HasManyAssociationsTestForCountWithFinderSql < ActiveRecord::TestCase class Invoice < ActiveRecord::Base - ActiveSupport::Deprecation.silence do - has_many :custom_line_items, :class_name => 'LineItem', :finder_sql => "SELECT line_items.* from line_items" - end + has_many :custom_line_items, :class_name => 'LineItem', :finder_sql => "SELECT line_items.* from line_items" end def test_should_fail assert_raise(ArgumentError) do @@ -35,9 +33,7 @@ def test_should_fail class HasManyAssociationsTestForCountWithCountSql < ActiveRecord::TestCase class Invoice < ActiveRecord::Base - ActiveSupport::Deprecation.silence do - has_many :custom_line_items, :class_name => 'LineItem', :counter_sql => "SELECT COUNT(*) line_items.* from line_items" - end + has_many :custom_line_items, :class_name => 'LineItem', :counter_sql => "SELECT COUNT(*) line_items.* from line_items" end def test_should_fail assert_raise(ArgumentError) do @@ -48,9 +44,7 @@ def test_should_fail class HasManyAssociationsTestForCountDistinctWithFinderSql < ActiveRecord::TestCase class Invoice < ActiveRecord::Base - ActiveSupport::Deprecation.silence do - has_many :custom_line_items, :class_name => 'LineItem', :finder_sql => "SELECT DISTINCT line_items.amount from line_items" - end + has_many :custom_line_items, :class_name => 'LineItem', :finder_sql => "SELECT DISTINCT line_items.amount from line_items" end def test_should_count_distinct_results @@ -1704,14 +1698,4 @@ def test_replace_returns_target assert_equal [bulb1, bulb3], car.bulbs assert_equal [bulb1, bulb3], result end - - test ":finder_sql is deprecated" do - klass = Class.new(ActiveRecord::Base) - assert_deprecated { klass.has_many :foo, :finder_sql => 'lol' } - end - - test ":counter_sql is deprecated" do - klass = Class.new(ActiveRecord::Base) - assert_deprecated { klass.has_many :foo, :counter_sql => 'lol' } - end end diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 04ec1ab2351ec..f8e259fdfee1e 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -36,13 +36,11 @@ class Client < ::Company end class Firm < Company - ActiveSupport::Deprecation.silence do - has_many :clients, :order => "id", :dependent => :destroy, :counter_sql => - "SELECT COUNT(*) FROM companies WHERE firm_id = 1 " + - "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )", - :before_remove => :log_before_remove, - :after_remove => :log_after_remove - end + has_many :clients, :order => "id", :dependent => :destroy, :counter_sql => + "SELECT COUNT(*) FROM companies WHERE firm_id = 1 " + + "AND (#{QUOTED_TYPE} = 'Client' OR #{QUOTED_TYPE} = 'SpecialClient' OR #{QUOTED_TYPE} = 'VerySpecialClient' )", + :before_remove => :log_before_remove, + :after_remove => :log_after_remove has_many :unsorted_clients, :class_name => "Client" has_many :unsorted_clients_with_symbol, :class_name => :Client has_many :clients_sorted_desc, :class_name => "Client", :order => "id DESC" @@ -55,19 +53,17 @@ class Firm < Company has_many :clients_with_interpolated_conditions, :class_name => "Client", :conditions => proc { "rating > #{rating}" } has_many :clients_like_ms, :conditions => "name = 'Microsoft'", :class_name => "Client", :order => "id" has_many :clients_like_ms_with_hash_conditions, :conditions => { :name => 'Microsoft' }, :class_name => "Client", :order => "id" - ActiveSupport::Deprecation.silence do - has_many :clients_using_sql, :class_name => "Client", :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" } - has_many :clients_using_counter_sql, :class_name => "Client", - :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id} " }, - :counter_sql => proc { "SELECT COUNT(*) FROM companies WHERE client_of = #{id}" } - has_many :clients_using_zero_counter_sql, :class_name => "Client", - :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" }, - :counter_sql => proc { "SELECT 0 FROM companies WHERE client_of = #{id}" } - has_many :no_clients_using_counter_sql, :class_name => "Client", - :finder_sql => 'SELECT * FROM companies WHERE client_of = 1000', - :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = 1000' - has_many :clients_using_finder_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE 1=1' - end + has_many :clients_using_sql, :class_name => "Client", :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" } + has_many :clients_using_counter_sql, :class_name => "Client", + :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id} " }, + :counter_sql => proc { "SELECT COUNT(*) FROM companies WHERE client_of = #{id}" } + has_many :clients_using_zero_counter_sql, :class_name => "Client", + :finder_sql => proc { "SELECT * FROM companies WHERE client_of = #{id}" }, + :counter_sql => proc { "SELECT 0 FROM companies WHERE client_of = #{id}" } + has_many :no_clients_using_counter_sql, :class_name => "Client", + :finder_sql => 'SELECT * FROM companies WHERE client_of = 1000', + :counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = 1000' + has_many :clients_using_finder_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE 1=1' has_many :plain_clients, :class_name => 'Client' has_many :readonly_clients, :class_name => 'Client', :readonly => true has_many :clients_using_primary_key, :class_name => 'Client', diff --git a/activerecord/test/models/company_in_module.rb b/activerecord/test/models/company_in_module.rb index 1a1cffd1f2bf0..2c8c30efb4e4d 100644 --- a/activerecord/test/models/company_in_module.rb +++ b/activerecord/test/models/company_in_module.rb @@ -11,9 +11,7 @@ class Firm < Company has_many :clients_sorted_desc, :class_name => "Client", :order => "id DESC" has_many :clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id" has_many :clients_like_ms, :conditions => "name = 'Microsoft'", :class_name => "Client", :order => "id" - ActiveSupport::Deprecation.silence do - has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}' - end + has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}' has_one :account, :class_name => 'MyApplication::Billing::Account', :dependent => :destroy end diff --git a/activerecord/test/models/project.rb b/activerecord/test/models/project.rb index f8d432df1fd25..efe1ce67da893 100644 --- a/activerecord/test/models/project.rb +++ b/activerecord/test/models/project.rb @@ -7,19 +7,15 @@ class Project < ActiveRecord::Base has_and_belongs_to_many :developers_named_david, :class_name => "Developer", :conditions => "name = 'David'", :uniq => true has_and_belongs_to_many :developers_named_david_with_hash_conditions, :class_name => "Developer", :conditions => { :name => 'David' }, :uniq => true has_and_belongs_to_many :salaried_developers, :class_name => "Developer", :conditions => "salary > 0" - - ActiveSupport::Deprecation.silence do - has_and_belongs_to_many :developers_with_finder_sql, :class_name => "Developer", :finder_sql => proc { "SELECT t.*, j.* FROM developers_projects j, developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id" } - has_and_belongs_to_many :developers_with_multiline_finder_sql, :class_name => "Developer", :finder_sql => proc { - "SELECT - t.*, j.* - FROM - developers_projects j, - developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id" - } - has_and_belongs_to_many :developers_by_sql, :class_name => "Developer", :delete_sql => proc { |record| "DELETE FROM developers_projects WHERE project_id = #{id} AND developer_id = #{record.id}" } - end - + has_and_belongs_to_many :developers_with_finder_sql, :class_name => "Developer", :finder_sql => proc { "SELECT t.*, j.* FROM developers_projects j, developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id" } + has_and_belongs_to_many :developers_with_multiline_finder_sql, :class_name => "Developer", :finder_sql => proc { + "SELECT + t.*, j.* + FROM + developers_projects j, + developers t WHERE t.id = j.developer_id AND j.project_id = #{id} ORDER BY t.id" + } + has_and_belongs_to_many :developers_by_sql, :class_name => "Developer", :delete_sql => proc { |record| "DELETE FROM developers_projects WHERE project_id = #{id} AND developer_id = #{record.id}" } has_and_belongs_to_many :developers_with_callbacks, :class_name => "Developer", :before_add => Proc.new {|o, r| o.developers_log << "before_adding#{r.id || ''}"}, :after_add => Proc.new {|o, r| o.developers_log << "after_adding#{r.id || ''}"}, :before_remove => Proc.new {|o, r| o.developers_log << "before_removing#{r.id}"},