mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Removed support for deprecated finder_sql
in associations.
This commit is contained in:
parent
f875d319ad
commit
d80334488f
9 changed files with 10 additions and 197 deletions
|
@ -1,3 +1,7 @@
|
|||
* Removed support for deprecated `finder_sql` in associations.
|
||||
|
||||
*Neeraj Singh*
|
||||
|
||||
* Support array as root element in JSON fields.
|
||||
|
||||
*Alexey Noskov & Francesco Rodriguez*
|
||||
|
|
|
@ -8,7 +8,7 @@ module ActiveRecord::Associations::Builder
|
|||
CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove]
|
||||
|
||||
def valid_options
|
||||
super + [:table_name, :finder_sql, :before_add,
|
||||
super + [:table_name, :before_add,
|
||||
:after_add, :before_remove, :after_remove, :extend]
|
||||
end
|
||||
|
||||
|
|
|
@ -44,7 +44,7 @@ module ActiveRecord
|
|||
|
||||
# Implements the ids reader method, e.g. foo.item_ids for Foo.has_many :items
|
||||
def ids_reader
|
||||
if loaded? || options[:finder_sql]
|
||||
if loaded?
|
||||
load_target.map do |record|
|
||||
record.send(reflection.association_primary_key)
|
||||
end
|
||||
|
@ -79,9 +79,7 @@ module ActiveRecord
|
|||
if block_given?
|
||||
load_target.find(*args) { |*block_args| yield(*block_args) }
|
||||
else
|
||||
if options[:finder_sql]
|
||||
find_by_scan(*args)
|
||||
elsif options[:inverse_of] && loaded?
|
||||
if options[:inverse_of] && loaded?
|
||||
args = args.flatten
|
||||
raise RecordNotFound, "Couldn't find #{scope.klass.name} without an ID" if args.blank?
|
||||
|
||||
|
@ -197,19 +195,11 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
# Count all records using SQL. If the +:finder_sql+ option is set for the
|
||||
# association, it will be used for the query. Otherwise, construct options and pass them with
|
||||
# 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)
|
||||
|
||||
if options[:finder_sql]
|
||||
unless count_options.blank?
|
||||
raise ArgumentError, "If finder_sql is used then options cannot be passed"
|
||||
end
|
||||
|
||||
reflection.klass.count_by_sql(custom_counter_sql)
|
||||
else
|
||||
relation = scope
|
||||
if association_scope.distinct_value
|
||||
# This is needed because 'SELECT count(DISTINCT *)..' is not valid SQL.
|
||||
|
@ -227,7 +217,6 @@ module ActiveRecord
|
|||
else
|
||||
value
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
# Removes +records+ from this association calling +before_remove+ and
|
||||
|
@ -361,7 +350,6 @@ module ActiveRecord
|
|||
if record.new_record?
|
||||
include_in_memory?(record)
|
||||
else
|
||||
load_target if options[:finder_sql]
|
||||
loaded? ? target.include?(record) : scope.exists?(record)
|
||||
end
|
||||
else
|
||||
|
@ -406,27 +394,8 @@ module ActiveRecord
|
|||
|
||||
private
|
||||
|
||||
def custom_counter_sql
|
||||
# replace the SELECT clause with COUNT(SELECTS), preserving any hints within /* ... */
|
||||
interpolate(options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) do
|
||||
count_with = $2.to_s
|
||||
count_with = '*' if count_with.blank? || count_with =~ /,/ || count_with =~ /\.\*/
|
||||
"SELECT #{$1}COUNT(#{count_with}) FROM"
|
||||
end
|
||||
end
|
||||
|
||||
def custom_finder_sql
|
||||
interpolate(options[:finder_sql])
|
||||
end
|
||||
|
||||
def find_target
|
||||
records =
|
||||
if options[:finder_sql]
|
||||
reflection.klass.find_by_sql(custom_finder_sql)
|
||||
else
|
||||
scope.to_a
|
||||
end
|
||||
|
||||
records = scope.to_a
|
||||
records.each { |record| set_inverse_instance(record) }
|
||||
records
|
||||
end
|
||||
|
@ -565,7 +534,6 @@ module ActiveRecord
|
|||
# Otherwise, go to the database only if none of the following are true:
|
||||
# * target already loaded
|
||||
# * owner is new record
|
||||
# * custom :finder_sql exists
|
||||
# * target contains new or changed record(s)
|
||||
# * the first arg is an integer (which indicates the number of records to be returned)
|
||||
def fetch_first_or_last_using_find?(args)
|
||||
|
@ -574,7 +542,6 @@ module ActiveRecord
|
|||
else
|
||||
!(loaded? ||
|
||||
owner.new_record? ||
|
||||
options[:finder_sql] ||
|
||||
target.any? { |record| record.new_record? || record.changed? } ||
|
||||
args.first.kind_of?(Integer))
|
||||
end
|
||||
|
@ -591,7 +558,7 @@ module ActiveRecord
|
|||
end
|
||||
end
|
||||
|
||||
# If using a custom finder_sql or if the :inverse_of option has been
|
||||
# If the :inverse_of option has been
|
||||
# specified, then #find scans the entire collection.
|
||||
def find_by_scan(*args)
|
||||
expects_array = args.first.kind_of?(Array)
|
||||
|
|
|
@ -58,8 +58,6 @@ module ActiveRecord
|
|||
def count_records
|
||||
count = if has_cached_counter?
|
||||
owner.send(:read_attribute, cached_counter_attribute_name)
|
||||
elsif options[:finder_sql]
|
||||
reflection.klass.count_by_sql(custom_counter_sql)
|
||||
else
|
||||
scope.count
|
||||
end
|
||||
|
|
|
@ -524,25 +524,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
|
|||
assert ! project.developers.include?(developer)
|
||||
end
|
||||
|
||||
def test_find_in_association_with_custom_finder_sql
|
||||
assert_equal developers(:david), projects(:active_record).developers_with_finder_sql.find(developers(:david).id), "SQL find"
|
||||
|
||||
active_record = projects(:active_record)
|
||||
active_record.developers_with_finder_sql.reload
|
||||
assert_equal developers(:david), active_record.developers_with_finder_sql.find(developers(:david).id), "Ruby find"
|
||||
end
|
||||
|
||||
def test_find_in_association_with_custom_finder_sql_and_multiple_interpolations
|
||||
# interpolate once:
|
||||
assert_equal [developers(:david), developers(:jamis), developers(:poor_jamis)], projects(:active_record).developers_with_finder_sql, "first interpolation"
|
||||
# interpolate again, for a different project id
|
||||
assert_equal [developers(:david)], projects(:action_controller).developers_with_finder_sql, "second interpolation"
|
||||
end
|
||||
|
||||
def test_find_in_association_with_custom_finder_sql_and_string_id
|
||||
assert_equal developers(:david), projects(:active_record).developers_with_finder_sql.find(developers(:david).id.to_s), "SQL find"
|
||||
end
|
||||
|
||||
def test_find_with_merged_options
|
||||
assert_equal 1, projects(:active_record).limited_developers.size
|
||||
assert_equal 1, projects(:active_record).limited_developers.to_a.size
|
||||
|
@ -778,13 +759,6 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
|
|||
assert_equal 2, david.projects.count
|
||||
end
|
||||
|
||||
unless current_adapter?(:PostgreSQLAdapter)
|
||||
def test_count_with_finder_sql
|
||||
assert_equal 3, projects(:active_record).developers_with_finder_sql.count
|
||||
assert_equal 3, projects(:active_record).developers_with_multiline_finder_sql.count
|
||||
end
|
||||
end
|
||||
|
||||
def test_association_proxy_transaction_method_starts_transaction_in_association_class
|
||||
Post.expects(:transaction)
|
||||
Category.first.posts.transaction do
|
||||
|
|
|
@ -23,66 +23,6 @@ require 'models/categorization'
|
|||
require 'models/minivan'
|
||||
require 'models/speedometer'
|
||||
|
||||
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
|
||||
end
|
||||
def test_should_fail
|
||||
assert_raise(ArgumentError) do
|
||||
Invoice.create.custom_line_items.count(:conditions => {:amount => 0})
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
class HasManyAssociationsTestForCountWithVariousFinderSqls < 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"
|
||||
has_many :custom_full_line_items, :class_name => 'LineItem', :finder_sql => "SELECT line_items.invoice_id, line_items.amount from line_items"
|
||||
has_many :custom_star_line_items, :class_name => 'LineItem', :finder_sql => "SELECT * from line_items"
|
||||
has_many :custom_qualified_star_line_items, :class_name => 'LineItem', :finder_sql => "SELECT line_items.* from line_items"
|
||||
end
|
||||
end
|
||||
|
||||
def test_should_count_distinct_results
|
||||
invoice = Invoice.new
|
||||
invoice.custom_line_items << LineItem.new(:amount => 0)
|
||||
invoice.custom_line_items << LineItem.new(:amount => 0)
|
||||
invoice.save!
|
||||
|
||||
assert_equal 1, invoice.custom_line_items.count
|
||||
end
|
||||
|
||||
def test_should_count_results_with_multiple_fields
|
||||
invoice = Invoice.new
|
||||
invoice.custom_full_line_items << LineItem.new(:amount => 0)
|
||||
invoice.custom_full_line_items << LineItem.new(:amount => 0)
|
||||
invoice.save!
|
||||
|
||||
assert_equal 2, invoice.custom_full_line_items.count
|
||||
end
|
||||
|
||||
def test_should_count_results_with_star
|
||||
invoice = Invoice.new
|
||||
invoice.custom_star_line_items << LineItem.new(:amount => 0)
|
||||
invoice.custom_star_line_items << LineItem.new(:amount => 0)
|
||||
invoice.save!
|
||||
|
||||
assert_equal 2, invoice.custom_star_line_items.count
|
||||
end
|
||||
|
||||
def test_should_count_results_with_qualified_star
|
||||
invoice = Invoice.new
|
||||
invoice.custom_qualified_star_line_items << LineItem.new(:amount => 0)
|
||||
invoice.custom_qualified_star_line_items << LineItem.new(:amount => 0)
|
||||
invoice.save!
|
||||
|
||||
assert_equal 2, invoice.custom_qualified_star_line_items.count
|
||||
end
|
||||
end
|
||||
|
||||
class HasManyAssociationsTestForReorderWithJoinDependency < ActiveRecord::TestCase
|
||||
fixtures :authors, :posts, :comments
|
||||
|
||||
|
@ -352,26 +292,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
|
|||
assert_equal "Summit", Firm.all.merge!(:order => "id").first.clients_using_primary_key.first.name
|
||||
end
|
||||
|
||||
def test_finding_using_sql
|
||||
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.order("id").first.clients_using_sql.size
|
||||
end
|
||||
|
||||
def test_finding_using_sql_take_into_account_only_uniq_ids
|
||||
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_finder_sql
|
||||
assert_equal 2, Firm.find(4).clients_using_sql.count
|
||||
end
|
||||
|
||||
def test_belongs_to_sanity
|
||||
c = Client.new
|
||||
assert_nil c.firm
|
||||
|
@ -399,22 +319,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
|
|||
assert_raise(ActiveRecord::RecordNotFound) { firm.clients.find(2, 99) }
|
||||
end
|
||||
|
||||
def test_find_string_ids_when_using_finder_sql
|
||||
firm = Firm.order("id").first
|
||||
|
||||
client = firm.clients_using_finder_sql.find("2")
|
||||
assert_kind_of Client, client
|
||||
|
||||
client_ary = firm.clients_using_finder_sql.find(["2"])
|
||||
assert_kind_of Array, client_ary
|
||||
assert_equal client, client_ary.first
|
||||
|
||||
client_ary = firm.clients_using_finder_sql.find("2", "3")
|
||||
assert_kind_of Array, client_ary
|
||||
assert_equal 2, client_ary.size
|
||||
assert_equal true, client_ary.include?(client)
|
||||
end
|
||||
|
||||
def test_find_all
|
||||
firm = Firm.all.merge!(:order => "id").first
|
||||
assert_equal 2, firm.clients.where("#{QUOTED_TYPE} = 'Client'").to_a.length
|
||||
|
@ -1324,13 +1228,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
|
|||
assert_equal [readers(:michael_welcome).id], posts(:welcome).readers_with_person_ids
|
||||
end
|
||||
|
||||
def test_get_ids_for_unloaded_finder_sql_associations_loads_them
|
||||
company = companies(:first_firm)
|
||||
assert !company.clients_using_sql.loaded?
|
||||
assert_equal [companies(:second_client).id], company.clients_using_sql_ids
|
||||
assert company.clients_using_sql.loaded?
|
||||
end
|
||||
|
||||
def test_get_ids_for_ordered_association
|
||||
assert_equal [companies(:second_client).id, companies(:first_client).id], companies(:first_firm).clients_ordered_by_name_ids
|
||||
end
|
||||
|
@ -1418,17 +1315,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
|
|||
assert ! firm.clients.loaded?
|
||||
end
|
||||
|
||||
def test_include_loads_collection_if_target_uses_finder_sql
|
||||
firm = companies(:first_firm)
|
||||
client = firm.clients_using_sql.first
|
||||
|
||||
firm.reload
|
||||
assert ! firm.clients_using_sql.loaded?
|
||||
assert_equal true, firm.clients_using_sql.include?(client)
|
||||
assert firm.clients_using_sql.loaded?
|
||||
end
|
||||
|
||||
|
||||
def test_include_returns_false_for_non_matching_record_to_verify_scoping
|
||||
firm = companies(:first_firm)
|
||||
client = Client.create!(:name => 'Not Associated')
|
||||
|
|
|
@ -48,10 +48,6 @@ 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"
|
||||
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_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',
|
||||
|
|
|
@ -10,10 +10,6 @@ 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"
|
||||
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
|
||||
|
||||
|
|
|
@ -9,14 +9,6 @@ class Project < ActiveRecord::Base
|
|||
has_and_belongs_to_many :salaried_developers, -> { where "salary > 0" }, :class_name => "Developer"
|
||||
|
||||
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
|
||||
|
||||
|
|
Loading…
Reference in a new issue