提交 0123ceb9 编写于 作者: A Aaron Patterson

Merge remote branch 'jonleighton/association_fixes'

* jonleighton/association_fixes:
  Add a transaction wrapper in add_to_target. This means that #build will now also use a transaction. IMO this is reasonable given that the before_add and after_add callbacks might do anything, and this great consistency allows us to abstract out the duplicate code from #build and #create.
  Inline ensure_owner_is_persisted! as it is only called from one place
  @target should always be an array
  Rename add_record_to_target_with_callbacks to add_to_target
  Don't pass the block through build_record
  Move create and create! next to build
  Get rid of create_record as it is not only used in one place
  Get rid of AssociationCollection#save_record
  Fix test/cases/connection_pool_test.rb for sqlite3 in-memory db
  Add interpolation of association conditions back in, in the form of proc { ... } rather than instance_eval-ing strings
......@@ -50,8 +50,27 @@
(for example, add_name_to_users) use the reversible migration's `change`
method instead of the ordinary `up` and `down` methods. [Prem Sichanugrist]
* Removed support for interpolated SQL conditions. Please use scoping
along with attribute conditionals as a replacement.
* Removed support for interpolating string SQL conditions on associations. Instead, you should
use a proc, like so:
Before:
has_many :things, :conditions => 'foo = #{bar}'
After:
has_many :things, :conditions => proc { "foo = #{bar}" }
Inside the proc, 'self' is the object which is the owner of the association, unless you are
eager loading the association, in which case 'self' is the class which the association is within.
You can have any "normal" conditions inside the proc, so the following will work too:
has_many :things, :conditions => proc { ["foo = ?", bar] }
Previously :insert_sql and :delete_sql on has_and_belongs_to_many association allowed you to call
'record' to get the record being inserted or deleted. This is now passed as an argument to
the proc.
* Added ActiveRecord::Base#has_secure_password (via ActiveModel::SecurePassword) to encapsulate dead-simple password usage with BCrypt encryption and salting [DHH]. Example:
......
......@@ -399,10 +399,18 @@ def find_associated_records(ids, reflection, preload_options)
end
end
def process_conditions(conditions, klass = self)
if conditions.respond_to?(:to_proc)
conditions = instance_eval(&conditions)
end
klass.send(:sanitize_sql, conditions)
end
def append_conditions(reflection, preload_options)
[
("(#{reflection.sanitized_conditions})" if reflection.sanitized_conditions),
("(#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions]),
('(' + process_conditions(reflection.options[:conditions], reflection.klass) + ')' if reflection.options[:conditions]),
('(' + process_conditions(preload_options[:conditions]) + ')' if preload_options[:conditions]),
].compact.map { |x| Arel.sql x }
end
......
......@@ -56,14 +56,21 @@ def reset
end
def build(attributes = {}, &block)
if attributes.is_a?(Array)
attributes.collect { |attr| build(attr, &block) }
else
build_record(attributes) do |record|
block.call(record) if block_given?
set_owner_attributes(record)
end
build_or_create(attributes, :build, &block)
end
def create(attributes = {}, &block)
unless @owner.persisted?
raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
end
build_or_create(attributes, :create, &block)
end
def create!(attrs = {}, &block)
record = create(attrs, &block)
Array.wrap(record).each(&:save!)
record
end
# Add +records+ to this association. Returns +self+ so method calls may be chained.
......@@ -75,7 +82,7 @@ def <<(*records)
transaction do
records.flatten.each do |record|
raise_on_type_mismatch(record)
add_record_to_target_with_callbacks(record) do |r|
add_to_target(record) do |r|
result &&= insert_record(record) unless @owner.new_record?
end
end
......@@ -191,24 +198,6 @@ def destroy(*records)
delete_or_destroy(records, :destroy)
end
def create(attrs = {})
if attrs.is_a?(Array)
attrs.collect { |attr| create(attr) }
else
create_record(attrs) do |record|
yield(record) if block_given?
insert_record(record, false)
end
end
end
def create!(attrs = {})
create_record(attrs) do |record|
yield(record) if block_given?
insert_record(record, true)
end
end
# Returns the size of the collection by executing a SELECT COUNT(*)
# query if the collection hasn't been loaded, and calling
# <tt>collection.size</tt> if it has.
......@@ -348,17 +337,21 @@ def load_target
target
end
def add_record_to_target_with_callbacks(record)
callback(:before_add, record)
yield(record) if block_given?
@target ||= [] unless loaded?
if @reflection.options[:uniq] && index = @target.index(record)
@target[index] = record
else
@target << record
def add_to_target(record)
transaction do
callback(:before_add, record)
yield(record) if block_given?
if @reflection.options[:uniq] && index = @target.index(record)
@target[index] = record
else
@target << record
end
callback(:after_add, record)
set_inverse_instance(record)
end
callback(:after_add, record)
set_inverse_instance(record)
record
end
......@@ -374,17 +367,15 @@ def uniq_select_value
def custom_counter_sql
if @reflection.options[:counter_sql]
counter_sql = @reflection.options[:counter_sql]
interpolate(@reflection.options[:counter_sql])
else
# replace the SELECT clause with COUNT(*), preserving any hints within /* ... */
counter_sql = @reflection.options[:finder_sql].sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
interpolate(@reflection.options[:finder_sql]).sub(/SELECT\b(\/\*.*?\*\/ )?(.*)\bFROM\b/im) { "SELECT #{$1}COUNT(*) FROM" }
end
interpolate_sql(counter_sql)
end
def custom_finder_sql
interpolate_sql(@reflection.options[:finder_sql])
interpolate(@reflection.options[:finder_sql])
end
def find_target
......@@ -421,26 +412,26 @@ def merge_target_lists(loaded, existing)
end + existing
end
# Do the relevant stuff to insert the given record into the association collection. The
# force param specifies whether or not an exception should be raised on failure. The
# validate param specifies whether validation should be performed (if force is false).
def insert_record(record, force = true, validate = true)
raise NotImplementedError
end
def build_or_create(attributes, method)
records = Array.wrap(attributes).map do |attrs|
record = build_record(attrs)
add_to_target(record) do
yield(record) if block_given?
insert_record(record) if method == :create
end
end
def save_record(record, force, validate)
force ? record.save! : record.save(:validate => validate)
attributes.is_a?(Array) ? records : records.first
end
def create_record(attributes, &block)
ensure_owner_is_persisted!
transaction { build_record(attributes, &block) }
# Do the relevant stuff to insert the given record into the association collection.
def insert_record(record, validate = true)
raise NotImplementedError
end
def build_record(attributes, &block)
attributes = scoped.scope_for_create.merge(attributes)
record = @reflection.build_association(attributes)
add_record_to_target_with_callbacks(record, &block)
def build_record(attributes)
@reflection.build_association(scoped.scope_for_create.merge(attributes))
end
def delete_or_destroy(records, method)
......@@ -482,12 +473,6 @@ def callbacks_for(callback_name)
@owner.class.send(full_callback_name.to_sym) || []
end
def ensure_owner_is_persisted!
unless @owner.persisted?
raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved"
end
end
# Should we deal with assoc.first or assoc.last by issuing an independent query to
# the database, or by getting the target, and then taking the first/last item from that?
#
......
......@@ -184,7 +184,8 @@ def construct_scope
def association_scope
scope = target_klass.unscoped
scope = scope.create_with(creation_attributes)
scope = scope.apply_finder_options(@reflection.options.slice(:conditions, :readonly, :include))
scope = scope.apply_finder_options(@reflection.options.slice(:readonly, :include))
scope = scope.where(interpolate(@reflection.options[:conditions]))
if select = select_value
scope = scope.select(select)
end
......@@ -240,8 +241,12 @@ def find_target?
!loaded? && (!@owner.new_record? || foreign_key_present?) && target_klass
end
def interpolate_sql(sql, record = nil)
@owner.send(:interpolate_sql, sql, record)
def interpolate(sql, record = nil)
if sql.respond_to?(:to_proc)
@owner.send(:instance_exec, record, &sql)
else
sql
end
end
def select_value
......
......@@ -108,6 +108,10 @@ def allocate_aliases
end
def process_conditions(conditions, table_name)
if conditions.respond_to?(:to_proc)
conditions = instance_eval(&conditions)
end
Arel.sql(sanitize_sql(conditions, table_name))
end
......
......@@ -11,13 +11,11 @@ def initialize(owner, reflection)
protected
def insert_record(record, force = true, validate = true)
if record.new_record?
return false unless save_record(record, force, validate)
end
def insert_record(record, validate = true)
return if record.new_record? && !record.save(:validate => validate)
if @reflection.options[:insert_sql]
@owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record))
@owner.connection.insert(interpolate(@reflection.options[:insert_sql], record))
else
stmt = join_table.compile_insert(
join_table[@reflection.foreign_key] => @owner.id,
......@@ -27,7 +25,7 @@ def insert_record(record, force = true, validate = true)
@owner.connection.insert stmt.to_sql
end
true
record
end
def association_scope
......@@ -42,7 +40,7 @@ def count_records
def delete_records(records, method)
if sql = @reflection.options[:delete_sql]
records.each { |record| @owner.connection.delete(interpolate_sql(sql, record)) }
records.each { |record| @owner.connection.delete(interpolate(sql, record)) }
else
relation = join_table
stmt = relation.where(relation[@reflection.foreign_key].eq(@owner.id).
......
......@@ -8,9 +8,9 @@ module Associations
class HasManyAssociation < AssociationCollection #:nodoc:
protected
def insert_record(record, force = false, validate = true)
def insert_record(record, validate = true)
set_owner_attributes(record)
save_record(record, force, validate)
record.save(:validate => validate)
end
private
......
......@@ -22,12 +22,21 @@ def size
end
end
def <<(*records)
unless @owner.new_record?
records.flatten.each do |record|
raise_on_type_mismatch(record)
record.save! if record.new_record?
end
end
super
end
protected
def insert_record(record, force = true, validate = true)
if record.new_record?
return unless save_record(record, force, validate)
end
def insert_record(record, validate = true)
return if record.new_record? && !record.save(:validate => validate)
through_association = @owner.send(@reflection.through_reflection.name)
through_association.create!(construct_join_attributes(record))
......
......@@ -119,14 +119,14 @@ def add_conditions(scope)
scope = scope.where(@reflection.through_reflection.klass.send(:type_condition))
end
scope = scope.where(@reflection.source_reflection.options[:conditions])
scope = scope.where(interpolate(@reflection.source_reflection.options[:conditions]))
scope.where(through_conditions)
end
# If there is a hash of conditions then we make sure the keys are scoped to the
# through table name if left ambiguous.
def through_conditions
conditions = @reflection.through_reflection.options[:conditions]
conditions = interpolate(@reflection.through_reflection.options[:conditions])
if conditions.is_a?(Hash)
Hash[conditions.map { |key, value|
......
......@@ -312,7 +312,7 @@ def save_collection_association(reflection)
association.destroy(record)
elsif autosave != false && (@new_record_before_save || record.new_record?)
if autosave
saved = association.send(:insert_record, record, false, false)
saved = association.send(:insert_record, record, false)
else
association.send(:insert_record, record)
end
......
......@@ -1790,12 +1790,6 @@ def quote_value(value, column = nil)
self.class.connection.quote(value, column)
end
# Interpolate custom SQL string in instance context.
# Optional record argument is meant for custom insert_sql.
def interpolate_sql(sql, record = nil)
instance_eval("%@#{sql.gsub('@', '\@')}@", __FILE__, __LINE__)
end
# Instantiates objects for all attribute classes that needs more than one constructor parameter. This is done
# by calling new on the column type or aggregation type (through composed_of) object with these parameters.
# So having the pairs written_on(1) = "2004", written_on(2) = "6", written_on(3) = "24", will instantiate
......
......@@ -413,7 +413,7 @@ def assign_nested_attributes_for_collection_association(association_name, attrib
if target_record
existing_record = target_record
else
association.send(:add_record_to_target_with_callbacks, existing_record)
association.send(:add_to_target, existing_record)
end
end
......
......@@ -668,6 +668,14 @@ def test_limited_eager_with_numeric_in_association
assert_equal people(:david, :susan), Person.find(:all, :include => [:readers, :primary_contact, :number1_fan], :conditions => "number1_fans_people.first_name like 'M%'", :order => 'people.id', :limit => 2, :offset => 0)
end
def test_preload_with_interpolation
post = Post.includes(:comments_with_interpolated_conditions).find(posts(:welcome).id)
assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
post = Post.joins(:comments_with_interpolated_conditions).find(posts(:welcome).id)
assert_equal [comments(:greetings)], post.comments_with_interpolated_conditions
end
def test_polymorphic_type_condition
post = Post.find(posts(:thinking).id, :include => :taggings)
assert post.taggings.include?(taggings(:thinking_general))
......
......@@ -72,7 +72,7 @@ class DeveloperWithCounterSQL < ActiveRecord::Base
:join_table => "developers_projects",
:association_foreign_key => "project_id",
:foreign_key => "developer_id",
:counter_sql => 'SELECT COUNT(*) AS count_all FROM projects INNER JOIN developers_projects ON projects.id = developers_projects.project_id WHERE developers_projects.developer_id =#{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
......
......@@ -279,7 +279,6 @@ def test_counting_non_existant_items_using_sql
def test_counting_using_finder_sql
assert_equal 2, Firm.find(4).clients_using_sql.count
assert_equal 2, Firm.find(4).clients_using_multiline_sql.count
end
def test_belongs_to_sanity
......
......@@ -711,4 +711,11 @@ def test_deleting_from_has_many_through_a_belongs_to_should_not_try_to_update_co
post.author_addresses.delete(address)
assert post[:author_count].nil?
end
def test_interpolated_conditions
post = posts(:welcome)
assert !post.tags.empty?
assert_equal post.tags, post.interpolated_tags
assert_equal post.tags, post.interpolated_tags_2
end
end
......@@ -243,6 +243,14 @@ def test_dependence_with_missing_association_and_nullify
firm.destroy
end
def test_finding_with_interpolated_condition
firm = Firm.find(:first)
superior = firm.clients.create(:name => 'SuperiorCo')
superior.rating = 10
superior.save
assert_equal 10, firm.clients_with_interpolated_conditions.first.rating
end
def test_assignment_before_child_saved
firm = Firm.find(1)
firm.account = a = Account.new("credit_limit" => 1000)
......
......@@ -1295,12 +1295,6 @@ def test_count_with_join
assert_equal res6, res7
end
def test_interpolate_sql
assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo@bar') }
assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo bar) baz') }
assert_nothing_raised { Category.new.send(:interpolate_sql, 'foo bar} baz') }
end
def test_scoped_find_conditions
scoped_developers = Developer.send(:with_scope, :find => { :conditions => 'salary > 90000' }) do
Developer.find(:all, :conditions => 'id < 5')
......
......@@ -6,6 +6,16 @@ class ConnectionPoolTest < ActiveRecord::TestCase
def setup
# Keep a duplicate pool so we do not bother others
@pool = ConnectionPool.new ActiveRecord::Base.connection_pool.spec
if in_memory_db?
# Separate connections to an in-memory database create an entirely new database,
# with an empty schema etc, so we just stub out this schema on the fly.
@pool.with_connection do |connection|
connection.create_table :posts do |t|
t.integer :cololumn
end
end
end
end
def test_pool_caches_columns
......
......@@ -48,19 +48,16 @@ class Firm < Company
has_many :dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :destroy
has_many :exclusively_dependent_clients_of_firm, :foreign_key => "client_of", :class_name => "Client", :order => "id", :dependent => :delete_all
has_many :limited_clients, :class_name => "Client", :limit => 1
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"
has_many :clients_using_sql, :class_name => "Client", :finder_sql => 'SELECT * FROM companies WHERE client_of = #{id}'
has_many :clients_using_multiline_sql, :class_name => "Client", :finder_sql => '
SELECT
companies.*
FROM companies WHERE companies.client_of = #{id}'
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 => 'SELECT * FROM companies WHERE client_of = #{id}',
:counter_sql => 'SELECT COUNT(*) FROM companies WHERE client_of = #{id}'
: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 => 'SELECT * FROM companies WHERE client_of = #{id}',
:counter_sql => 'SELECT 0 FROM companies WHERE client_of = #{id}'
: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'
......
......@@ -41,6 +41,9 @@ def find_most_recent
has_many :author_categorizations, :through => :author, :source => :categorizations
has_many :author_addresses, :through => :author
has_many :comments_with_interpolated_conditions, :class_name => 'Comment',
:conditions => proc { ["#{"#{aliased_table_name}." rescue ""}body = ?", 'Thank you for the welcome'] }
has_one :very_special_comment
has_one :very_special_comment_with_post, :class_name => "VerySpecialComment", :include => :post
has_many :special_comments
......@@ -57,6 +60,10 @@ def add_joins_and_select
end
end
has_many :interpolated_taggings, :class_name => 'Tagging', :as => :taggable, :conditions => proc { "1 = #{1}" }
has_many :interpolated_tags, :through => :taggings
has_many :interpolated_tags_2, :through => :interpolated_taggings, :source => :tag
has_many :taggings_with_delete_all, :class_name => 'Tagging', :as => :taggable, :dependent => :delete_all
has_many :taggings_with_destroy, :class_name => 'Tagging', :as => :taggable, :dependent => :destroy
......
......@@ -7,14 +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"
has_and_belongs_to_many :developers_with_finder_sql, :class_name => "Developer", :finder_sql => '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 => '
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 => "DELETE FROM developers_projects WHERE project_id = \#{id} AND developer_id = \#{record.id}"
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 || '<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}"},
......
......@@ -6,6 +6,7 @@ class Tagging < ActiveRecord::Base
belongs_to :tag, :include => :tagging
belongs_to :super_tag, :class_name => 'Tag', :foreign_key => 'super_tag_id'
belongs_to :invalid_tag, :class_name => 'Tag', :foreign_key => 'tag_id'
belongs_to :interpolated_tag, :class_name => 'Tag', :foreign_key => :tag_id, :conditions => proc { "1 = #{1}" }
belongs_to :taggable, :polymorphic => true, :counter_cache => true
has_many :things, :through => :taggable
end
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册