1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Let AssociationCollection#find use #scoped to do its finding. Note that I am removing test_polymorphic_has_many_going_through_join_model_with_disabled_include, since this specifies different behaviour for an association than for a regular scope. It seems reasonable to expect scopes and association proxies to behave in roughly the same way rather than having subtle differences.

This commit is contained in:
Jon Leighton 2011-01-02 20:33:18 +00:00 committed by Aaron Patterson
parent d6289aadce
commit 3103296a61
14 changed files with 90 additions and 114 deletions

View file

@ -32,37 +32,10 @@ module ActiveRecord
end
def find(*args)
options = args.extract_options!
# If using a custom finder_sql, scan the entire collection.
if @reflection.options[:finder_sql]
expects_array = args.first.kind_of?(Array)
ids = args.flatten.compact.uniq.map { |arg| arg.to_i }
if ids.size == 1
id = ids.first
record = load_target.detect { |r| id == r.id }
expects_array ? [ record ] : record
else
load_target.select { |r| ids.include?(r.id) }
end
find_by_scan(*args)
else
merge_options_from_reflection!(options)
construct_find_options!(options)
with_scope(:find => @scope[:find].slice(:conditions, :order)) do
relation = @reflection.klass.send(:construct_finder_arel, options, @reflection.klass.send(:current_scoped_methods))
case args.first
when :first, :last
relation.send(args.first)
when :all
records = relation.all
@reflection.options[:uniq] ? uniq(records) : records
else
relation.find(*args)
end
end
find_by_sql(*args)
end
end
@ -360,7 +333,25 @@ module ActiveRecord
end
protected
def construct_find_options!(options)
def construct_find_scope
{
:conditions => construct_conditions,
:select => construct_select,
:readonly => @reflection.options[:readonly],
:order => @reflection.options[:order],
:limit => @reflection.options[:limit],
:include => @reflection.options[:include],
:joins => @reflection.options[:joins],
:group => @reflection.options[:group],
:having => @reflection.options[:having],
:offset => @reflection.options[:offset]
}
end
def construct_select
@reflection.options[:select] ||
@reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*"
end
def load_target
@ -394,7 +385,7 @@ module ActiveRecord
target
end
def method_missing(method, *args)
def method_missing(method, *args, &block)
match = DynamicFinderMatch.match(method)
if match && match.creator?
attributes = match.attribute_names
@ -406,15 +397,9 @@ module ActiveRecord
elsif @reflection.klass.scopes[method]
@_scopes_cache ||= {}
@_scopes_cache[method] ||= {}
@_scopes_cache[method][args] ||= with_scope(@scope) { @reflection.klass.send(method, *args) }
@_scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args)
else
with_scope(@scope) do
if block_given?
@reflection.klass.send(method, *args) { |*block_args| yield(*block_args) }
else
@reflection.klass.send(method, *args)
end
end
scoped.readonly(nil).send(method, *args, &block)
end
end
@ -547,6 +532,24 @@ module ActiveRecord
@target.include?(record)
end
end
# If using a custom finder_sql, #find scans the entire collection.
def find_by_scan(*args)
expects_array = args.first.kind_of?(Array)
ids = args.flatten.compact.uniq.map { |arg| arg.to_i }
if ids.size == 1
id = ids.first
record = load_target.detect { |r| id == r.id }
expects_array ? [ record ] : record
else
load_target.select { |r| ids.include?(r.id) }
end
end
def find_by_sql(*args)
scoped.find(*args)
end
end
end
end

View file

@ -182,20 +182,6 @@ module ActiveRecord
@reflection.klass.send(:sanitize_sql, sql, table_name)
end
# Merges into +options+ the ones coming from the reflection.
def merge_options_from_reflection!(options)
options.reverse_merge!(
:group => @reflection.options[:group],
:having => @reflection.options[:having],
:limit => @reflection.options[:limit],
:offset => @reflection.options[:offset],
:joins => @reflection.options[:joins],
:include => @reflection.options[:include],
:select => @reflection.options[:select],
:readonly => @reflection.options[:readonly]
)
end
# Forwards +with_scope+ to the reflection.
def with_scope(*args, &block)
target_klass.send :with_scope, *args, &block

View file

@ -2,6 +2,7 @@ module ActiveRecord
# = Active Record Has And Belongs To Many Association
module Associations
class HasAndBelongsToManyAssociation < AssociationCollection #:nodoc:
def columns
@reflection.columns(@reflection.options[:join_table], "#{@reflection.options[:join_table]} Columns")
end
@ -15,11 +16,6 @@ module ActiveRecord
end
protected
def construct_find_options!(options)
options[:joins] = Arel::SqlLiteral.new(@scope[:find][:joins])
options[:readonly] = finding_with_ambiguous_select?(options[:select] || @reflection.options[:select])
options[:select] ||= (@reflection.options[:select] || Arel::SqlLiteral.new('*'))
end
def count_records
load_target.size
@ -84,22 +80,23 @@ module ActiveRecord
end
def construct_find_scope
{
:conditions => construct_conditions,
:joins => construct_joins,
:readonly => false,
:order => @reflection.options[:order],
:include => @reflection.options[:include],
:limit => @reflection.options[:limit]
}
super.merge(
:joins => construct_joins,
:readonly => ambiguous_select?(@reflection.options[:select]),
:select => @reflection.options[:select] || Arel.star
)
end
# Join tables with additional columns on top of the two foreign keys must be considered
# ambiguous unless a select clause has been explicitly defined. Otherwise you can get
# broken records back, if, for example, the join column also has an id column. This will
# then overwrite the id column of the records coming back.
def finding_with_ambiguous_select?(select_clause)
!select_clause && columns.size != 2
def ambiguous_select?(select)
extra_join_columns? && select.nil?
end
def extra_join_columns?
columns.size > 2
end
private
@ -114,6 +111,13 @@ module ActiveRecord
def invertible_for?(record)
false
end
def find_by_sql(*args)
options = args.extract_options!
ambiguous = ambiguous_select?(@reflection.options[:select] || options[:select])
scoped.readonly(ambiguous).find(*(args << options))
end
end
end
end

View file

@ -71,16 +71,6 @@ module ActiveRecord
end
end
def construct_find_scope
{
:conditions => construct_conditions,
:readonly => false,
:order => @reflection.options[:order],
:limit => @reflection.options[:limit],
:include => @reflection.options[:include]
}
end
def construct_create_scope
construct_owner_attributes
end

View file

@ -38,11 +38,6 @@ module ActiveRecord
end
end
def construct_find_options!(options)
options[:joins] = [construct_joins] + Array.wrap(options[:joins])
options[:include] = @reflection.source_reflection.options[:include] if options[:include].nil? && @reflection.source_reflection.options[:include]
end
def insert_record(record, force = true, validate = true)
if record.new_record?
return false unless save_record(record, force, validate)

View file

@ -13,15 +13,11 @@ module ActiveRecord
protected
def construct_find_scope
{
:conditions => construct_conditions,
:joins => construct_joins,
:include => @reflection.options[:include] || @reflection.source_reflection.options[:include],
:select => construct_select,
:order => @reflection.options[:order],
:limit => @reflection.options[:limit],
:readonly => @reflection.options[:readonly]
}
super.merge(
:joins => construct_joins,
:include => @reflection.options[:include] ||
@reflection.source_reflection.options[:include]
)
end
# This scope affects the creation of the associated records (not the join records). At the
@ -44,11 +40,6 @@ module ActiveRecord
super(aliased_through_table, @reflection.through_reflection)
end
def construct_select
@reflection.options[:select] ||
@reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*"
end
def construct_joins
right = aliased_through_table
left = @reflection.klass.arel_table

View file

@ -393,7 +393,7 @@ module ActiveRecord
association.to_a
else
attribute_ids = attributes_collection.map {|a| a['id'] || a[:id] }.compact
attribute_ids.empty? ? [] : association.all(:conditions => {association.primary_key => attribute_ids})
attribute_ids.empty? ? [] : association.all(:conditions => {association.klass.primary_key => attribute_ids})
end
attributes_collection.each do |attributes|

View file

@ -102,7 +102,7 @@ module ActiveRecord
options.assert_valid_keys(VALID_FIND_OPTIONS)
finders = options.dup
finders.delete_if { |key, value| value.nil? }
finders.delete_if { |key, value| value.nil? && key != :limit }
([:joins, :select, :group, :order, :having, :limit, :offset, :from, :lock, :readonly] & finders.keys).each do |finder|
relation = relation.send(finder, finders[finder])

View file

@ -85,13 +85,6 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase
end
end
def test_polymorphic_has_many_going_through_join_model_with_disabled_include
assert_equal tags(:general), tag = posts(:welcome).tags.add_joins_and_select.first
assert_queries 1 do
tag.tagging
end
end
def test_polymorphic_has_many_going_through_join_model_with_custom_select_and_joins
assert_equal tags(:general), tag = posts(:welcome).tags.add_joins_and_select.first
tag.author_id

View file

@ -6,11 +6,6 @@ require 'models/project'
require 'models/reader'
require 'models/person'
# Dummy class methods to test implicit association scoping.
def Comment.foo() find :first end
def Project.foo() find :first end
class ReadOnlyTest < ActiveRecord::TestCase
fixtures :posts, :comments, :developers, :projects, :developers_projects, :people, :readers
@ -114,7 +109,13 @@ class ReadOnlyTest < ActiveRecord::TestCase
end
def test_association_collection_method_missing_scoping_not_readonly
assert !Developer.find(1).projects.foo.readonly?
assert !Post.find(1).comments.foo.readonly?
developer = Developer.find(1)
project = Post.find(1)
assert !developer.projects.all_as_method.first.readonly?
assert !developer.projects.all_as_scope.first.readonly?
assert !project.comments.all_as_method.first.readonly?
assert !project.comments.all_as_scope.first.readonly?
end
end

View file

@ -806,4 +806,8 @@ class RelationTest < ActiveRecord::TestCase
assert_equal [rails_author], [rails_author] & relation
assert_equal [rails_author], relation & [rails_author]
end
def test_removing_limit_with_options
assert_not_equal 1, Post.limit(1).all(:limit => nil).count
end
end

View file

@ -15,6 +15,11 @@ class Comment < ActiveRecord::Base
def self.search_by_type(q)
self.find(:all, :conditions => ["#{QUOTED_TYPE} = ?", q])
end
def self.all_as_method
all
end
scope :all_as_scope, {}
end
class SpecialComment < Comment

View file

@ -51,7 +51,7 @@ class Post < ActiveRecord::Base
has_many :taggings, :as => :taggable
has_many :tags, :through => :taggings do
def add_joins_and_select
find :all, :select => 'tags.*, authors.id as author_id', :include => false,
find :all, :select => 'tags.*, authors.id as author_id',
:joins => 'left outer join posts on taggings.taggable_id = posts.id left outer join authors on posts.author_id = authors.id'
end
end

View file

@ -28,6 +28,10 @@ class Project < ActiveRecord::Base
@developers_log = []
end
def self.all_as_method
all
end
scope :all_as_scope, {}
end
class SpecialProject < Project