diff options
Diffstat (limited to 'activerecord')
9 files changed, 118 insertions, 45 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 037131d610..a58e761bd4 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,16 @@ ## Rails 4.0.0 (unreleased) ## +* `:finder_sql` and `:counter_sql` options on collection associations + are deprecated. Please transition to using scopes. + + *Jon Leighton* + +* `:insert_sql` and `:delete_sql` options on `has_and_belongs_to_many` + associations are deprecated. Please transition to using `has_many + :through` + + *Jon Leighton* + * The migration generator now creates a join table with (commented) indexes every time the migration name contains the word `join_table`: diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index af81af4ad2..dbddae0876 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -1,3 +1,5 @@ +require 'active_support/deprecation' + module ActiveRecord::Associations::Builder class CollectionAssociation < Association #:nodoc: CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] @@ -14,6 +16,7 @@ module ActiveRecord::Associations::Builder end def build + show_deprecation_warnings wrap_block_extension reflection = super CALLBACKS.each { |callback_name| define_callback(callback_name) } @@ -24,6 +27,14 @@ module ActiveRecord::Associations::Builder 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 7f79ef25f2..8df28ad876 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 @@ -14,6 +14,16 @@ module ActiveRecord::Associations::Builder 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/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 14a24e02dc..b38a206bef 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -380,7 +380,7 @@ module ActiveRecord if options[:finder_sql] reflection.klass.find_by_sql(custom_finder_sql) else - scoped.all + scoped.to_a end records.each { |record| set_inverse_instance(record) } 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 9846d81a9b..53bc60445c 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,12 +67,15 @@ end class DeveloperWithCounterSQL < ActiveRecord::Base self.table_name = 'developers' - 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}" } + + 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 end class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase @@ -841,4 +844,16 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase 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 4d6d6a24fb..bceec80b6b 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -22,7 +22,9 @@ require 'models/engine' class HasManyAssociationsTestForCountWithFinderSql < ActiveRecord::TestCase class Invoice < ActiveRecord::Base - has_many :custom_line_items, :class_name => 'LineItem', :finder_sql => "SELECT line_items.* from line_items" + ActiveSupport::Deprecation.silence do + has_many :custom_line_items, :class_name => 'LineItem', :finder_sql => "SELECT line_items.* from line_items" + end end def test_should_fail assert_raise(ArgumentError) do @@ -33,7 +35,9 @@ end class HasManyAssociationsTestForCountWithCountSql < ActiveRecord::TestCase class Invoice < ActiveRecord::Base - has_many :custom_line_items, :class_name => 'LineItem', :counter_sql => "SELECT COUNT(*) line_items.* from line_items" + ActiveSupport::Deprecation.silence do + has_many :custom_line_items, :class_name => 'LineItem', :counter_sql => "SELECT COUNT(*) line_items.* from line_items" + end end def test_should_fail assert_raise(ArgumentError) do @@ -44,7 +48,9 @@ end class HasManyAssociationsTestForCountDistinctWithFinderSql < ActiveRecord::TestCase class Invoice < ActiveRecord::Base - has_many :custom_line_items, :class_name => 'LineItem', :finder_sql => "SELECT DISTINCT line_items.amount from line_items" + ActiveSupport::Deprecation.silence do + has_many :custom_line_items, :class_name => 'LineItem', :finder_sql => "SELECT DISTINCT line_items.amount from line_items" + end end def test_should_count_distinct_results @@ -308,30 +314,30 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_finding_using_sql - firm = Firm.scoped(:order => "id").first + firm = Firm.order("id").first first_client = firm.clients_using_sql.first assert_not_nil first_client assert_equal "Microsoft", first_client.name assert_equal 1, firm.clients_using_sql.size - assert_equal 1, Firm.scoped(:order => "id").first.clients_using_sql.size + assert_equal 1, Firm.order("id").first.clients_using_sql.size end def test_finding_using_sql_take_into_account_only_uniq_ids - firm = Firm.scoped(:order => "id").first + firm = Firm.order("id").first client = firm.clients_using_sql.first assert_equal client, firm.clients_using_sql.find(client.id, client.id) assert_equal client, firm.clients_using_sql.find(client.id, client.id.to_s) end def test_counting_using_sql - assert_equal 1, Firm.scoped(:order => "id").first.clients_using_counter_sql.size - assert Firm.scoped(:order => "id").first.clients_using_counter_sql.any? - assert_equal 0, Firm.scoped(:order => "id").first.clients_using_zero_counter_sql.size - assert !Firm.scoped(:order => "id").first.clients_using_zero_counter_sql.any? + assert_equal 1, Firm.order("id").first.clients_using_counter_sql.size + assert Firm.order("id").first.clients_using_counter_sql.any? + assert_equal 0, Firm.order("id").first.clients_using_zero_counter_sql.size + assert !Firm.order("id").first.clients_using_zero_counter_sql.any? end def test_counting_non_existant_items_using_sql - assert_equal 0, Firm.scoped(:order => "id").first.no_clients_using_counter_sql.size + assert_equal 0, Firm.order("id").first.no_clients_using_counter_sql.size end def test_counting_using_finder_sql @@ -366,7 +372,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_find_string_ids_when_using_finder_sql - firm = Firm.scoped(:order => "id").first + firm = Firm.order("id").first client = firm.clients_using_finder_sql.find("2") assert_kind_of Client, client @@ -1638,4 +1644,14 @@ class HasManyAssociationsTest < ActiveRecord::TestCase post.taggings_with_delete_all.delete_all end 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 4c57cf1050..5bfbb5e855 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -36,11 +36,13 @@ module Namespaced end class Firm < Company - 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 + 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 :unsorted_clients, :class_name => "Client" has_many :unsorted_clients_with_symbol, :class_name => :Client has_many :clients_sorted_desc, -> { order "id DESC" }, :class_name => "Client" @@ -53,17 +55,19 @@ class Firm < Company has_many :clients_with_interpolated_conditions, ->(firm) { where "rating > #{firm.rating}" }, :class_name => "Client" has_many :clients_like_ms, -> { where("name = 'Microsoft'").order("id") }, :class_name => "Client" has_many :clients_like_ms_with_hash_conditions, -> { where(:name => 'Microsoft').order("id") }, :class_name => "Client" - 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' + 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 :plain_clients, :class_name => 'Client' has_many :readonly_clients, -> { readonly }, :class_name => 'Client' 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 e5ceda2928..eb2aedc425 100644 --- a/activerecord/test/models/company_in_module.rb +++ b/activerecord/test/models/company_in_module.rb @@ -11,7 +11,9 @@ module MyApplication has_many :clients_sorted_desc, -> { order("id DESC") }, :class_name => "Client" has_many :clients_of_firm, -> { order "id" }, :foreign_key => "client_of", :class_name => "Client" has_many :clients_like_ms, -> { where("name = 'Microsoft'").order("id") }, :class_name => "Client" - has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}' + ActiveSupport::Deprecation.silence do + has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}' + end 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 afe06f4a4c..af3ec4be83 100644 --- a/activerecord/test/models/project.rb +++ b/activerecord/test/models/project.rb @@ -7,15 +7,19 @@ class Project < ActiveRecord::Base has_and_belongs_to_many :developers_named_david, -> { where("name = 'David'").uniq }, :class_name => "Developer" has_and_belongs_to_many :developers_named_david_with_hash_conditions, -> { where(:name => 'David').uniq }, :class_name => "Developer" has_and_belongs_to_many :salaried_developers, -> { where "salary > 0" }, :class_name => "Developer" - 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}" } + + 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_callbacks, :class_name => "Developer", :before_add => Proc.new {|o, r| o.developers_log << "before_adding#{r.id || '<new>'}"}, :after_add => Proc.new {|o, r| o.developers_log << "after_adding#{r.id || '<new>'}"}, :before_remove => Proc.new {|o, r| o.developers_log << "before_removing#{r.id}"}, |