diff options
Diffstat (limited to 'activerecord/lib/active_record')
20 files changed, 241 insertions, 260 deletions
diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 6d905fe6fe..ecf7b6c210 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -127,8 +127,7 @@ module ActiveRecord association_proxy = parent_record.send(reflection_name) association_proxy.loaded association_proxy.target.push(*Array.wrap(associated_record)) - - association_proxy.__send__(:set_inverse_instance, associated_record, parent_record) + association_proxy.send(:set_inverse_instance, associated_record) end end @@ -148,16 +147,19 @@ module ActiveRecord def set_association_single_records(id_to_record_map, reflection_name, associated_records, key) seen_keys = {} associated_records.each do |associated_record| + seen_key = associated_record[key].to_s + #this is a has_one or belongs_to: there should only be one record. #Unfortunately we can't (in portable way) ask the database for #'all records where foo_id in (x,y,z), but please # only one row per distinct foo_id' so this where we enforce that - next if seen_keys[associated_record[key].to_s] - seen_keys[associated_record[key].to_s] = true - mapped_records = id_to_record_map[associated_record[key].to_s] + next if seen_keys.key? seen_key + + seen_keys[seen_key] = true + mapped_records = id_to_record_map[seen_key] mapped_records.each do |mapped_record| association_proxy = mapped_record.send("set_#{reflection_name}_target", associated_record) - association_proxy.__send__(:set_inverse_instance, associated_record, mapped_record) + association_proxy.send(:set_inverse_instance, associated_record) end end @@ -188,19 +190,17 @@ module ActiveRecord left = reflection.klass.arel_table - table_name = reflection.klass.quoted_table_name id_to_record_map, ids = construct_id_map(records) records.each {|record| record.send(reflection.name).loaded} options = reflection.options - conditions = "t0.#{reflection.primary_key_name} #{in_or_equals_for_ids(ids)}" - conditions << append_conditions(reflection, preload_options) - right = Arel::Table.new(options[:join_table]).alias('t0') - condition = left[reflection.klass.primary_key].eq( + + + join_condition = left[reflection.klass.primary_key].eq( right[reflection.association_foreign_key]) - join = left.create_join(right, left.create_on(condition)) + join = left.create_join(right, left.create_on(join_condition)) select = [ # FIXME: options[:select] is always nil in the tests. Do we really # need it? @@ -216,8 +216,16 @@ module ActiveRecord associated_records_proxy.joins_values = [join] associated_records_proxy.select_values = select + custom_conditions = append_conditions(reflection, preload_options) + all_associated_records = associated_records(ids) do |some_ids| - associated_records_proxy.where([conditions, some_ids]).to_a + method = in_or_equal(some_ids) + conditions = right[reflection.primary_key_name].send(*method) + conditions = custom_conditions.inject(conditions) do |ast, cond| + ast.and cond + end + + associated_records_proxy.where(conditions).to_a end set_association_collection_records(id_to_record_map, reflection.name, all_associated_records, 'the_parent_record_id') @@ -324,61 +332,54 @@ module ActiveRecord if klass = record.send(polymorph_type) klass_id = record.send(primary_key_name) if klass_id - id_map = klasses_and_ids[klass] ||= {} + id_map = klasses_and_ids[klass.constantize] ||= {} (id_map[klass_id.to_s] ||= []) << record end end end else - id_map = {} - records.each do |record| + id_map = records.group_by do |record| key = record.send(primary_key_name) - (id_map[key.to_s] ||= []) << record if key + key && key.to_s end - klasses_and_ids[reflection.klass.name] = id_map unless id_map.empty? + id_map.delete nil + klasses_and_ids[reflection.klass] = id_map unless id_map.empty? end - klasses_and_ids.each do |klass_name, _id_map| - klass = klass_name.constantize - - table_name = klass.quoted_table_name + klasses_and_ids.each do |klass, _id_map| + table = klass.arel_table primary_key = (reflection.options[:primary_key] || klass.primary_key).to_s - column_type = klass.columns.detect{|c| c.name == primary_key}.type - - ids = _id_map.keys.map do |id| - if column_type == :integer - id.to_i - elsif column_type == :float - id.to_f - else - id - end - end + method = in_or_equal(_id_map.keys) + conditions = table[primary_key].send(*method) - conditions = "#{table_name}.#{connection.quote_column_name(primary_key)} #{in_or_equals_for_ids(ids)}" - conditions << append_conditions(reflection, preload_options) + custom_conditions = append_conditions(reflection, preload_options) + conditions = custom_conditions.inject(conditions) do |ast, cond| + ast.and cond + end - associated_records = klass.unscoped.where([conditions, ids]).apply_finder_options(options.slice(:include, :select, :joins, :order)).to_a + associated_records = klass.unscoped.where(conditions).apply_finder_options(options.slice(:include, :select, :joins, :order)).to_a set_association_single_records(_id_map, reflection.name, associated_records, primary_key) end end def find_associated_records(ids, reflection, preload_options) - options = reflection.options - table_name = reflection.klass.quoted_table_name + options = reflection.options + table = reflection.klass.arel_table + + conditions = [] + + key = reflection.primary_key_name if interface = reflection.options[:as] - conditions = "#{reflection.klass.quoted_table_name}.#{connection.quote_column_name "#{interface}_id"} #{in_or_equals_for_ids(ids)} and #{reflection.klass.quoted_table_name}.#{connection.quote_column_name "#{interface}_type"} = '#{self.base_class.sti_name}'" - else - foreign_key = reflection.primary_key_name - conditions = "#{reflection.klass.quoted_table_name}.#{foreign_key} #{in_or_equals_for_ids(ids)}" + key = "#{interface}_id" + conditions << table["#{interface}_type"].eq(base_class.sti_name) end - conditions << append_conditions(reflection, preload_options) + conditions += append_conditions(reflection, preload_options) find_options = { - :select => preload_options[:select] || options[:select] || Arel::SqlLiteral.new("#{table_name}.*"), + :select => preload_options[:select] || options[:select] || table[Arel.star], :include => preload_options[:include] || options[:include], :joins => options[:joins], :group => preload_options[:group] || options[:group], @@ -386,19 +387,24 @@ module ActiveRecord } associated_records(ids) do |some_ids| - reflection.klass.scoped.apply_finder_options(find_options.merge(:conditions => [conditions, some_ids])).to_a + method = in_or_equal(some_ids) + where = conditions.inject(table[key].send(*method)) do |ast, cond| + ast.and cond + end + + reflection.klass.scoped.apply_finder_options(find_options.merge(:conditions => where)).to_a end end def append_conditions(reflection, preload_options) - sql = "" - sql << " AND (#{reflection.sanitized_conditions})" if reflection.sanitized_conditions - sql << " AND (#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions] - sql + [ + ("(#{reflection.sanitized_conditions})" if reflection.sanitized_conditions), + ("(#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions]), + ].compact.map { |x| Arel.sql x } end - def in_or_equals_for_ids(ids) - ids.size > 1 ? "IN (?)" : "= ?" + def in_or_equal(ids) + ids.length == 1 ? ['eq', ids.first] : ['in', ids] end # Some databases impose a limit on the number of ids in a list (in Oracle its 1000) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 1056c51a3d..b49cf8de95 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -120,6 +120,8 @@ module ActiveRecord # So there is no need to eager load them. autoload :AssociationCollection, 'active_record/associations/association_collection' autoload :AssociationProxy, 'active_record/associations/association_proxy' + autoload :HasAssociation, 'active_record/associations/has_association' + autoload :ThroughAssociation, 'active_record/associations/through_association' autoload :BelongsToAssociation, 'active_record/associations/belongs_to_association' autoload :BelongsToPolymorphicAssociation, 'active_record/associations/belongs_to_polymorphic_association' autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association' diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 7964f4fa2b..108e316672 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -18,6 +18,8 @@ module ActiveRecord # If you need to work on all current children, new and existing records, # +load_target+ and the +loaded+ flag are your friends. class AssociationCollection < AssociationProxy #:nodoc: + include HasAssociation + delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :to => :scoped def select(select = nil) @@ -111,7 +113,7 @@ module ActiveRecord else build_record(attributes) do |record| block.call(record) if block_given? - set_belongs_to_association_for(record) + set_owner_attributes(record) end end end @@ -123,7 +125,7 @@ module ActiveRecord load_target if @owner.new_record? transaction do - flatten_deeper(records).each do |record| + records.flatten.each do |record| raise_on_type_mismatch(record) add_record_to_target_with_callbacks(record) do |r| result &&= insert_record(record) unless @owner.new_record? @@ -452,9 +454,7 @@ module ActiveRecord end records = @reflection.options[:uniq] ? uniq(records) : records - records.each do |record| - set_inverse_instance(record, @owner) - end + records.each { |record| set_inverse_instance(record) } records end @@ -468,7 +468,7 @@ module ActiveRecord @target << record end callback(:after_add, record) - set_inverse_instance(record, @owner) + set_inverse_instance(record) record end @@ -488,7 +488,7 @@ module ActiveRecord ensure_owner_is_persisted! transaction do - with_scope(:create => @scope[:create].merge(scoped.where_values_hash || {})) do + with_scope(:create => @scope[:create].merge(scoped.where_values_hash)) do build_record(attrs, &block) end end @@ -501,7 +501,7 @@ module ActiveRecord end def remove_records(*records) - records = flatten_deeper(records) + records = records.flatten records.each { |record| raise_on_type_mismatch(record) } transaction do diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index f4eceeed8c..6720d83199 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -4,17 +4,17 @@ module ActiveRecord module Associations # = Active Record Associations # - # This is the root class of all association proxies: + # This is the root class of all association proxies ('+ Foo' signifies an included module Foo): # # AssociationProxy # BelongsToAssociation - # HasOneAssociation # BelongsToPolymorphicAssociation - # AssociationCollection + # AssociationCollection + HasAssociation # HasAndBelongsToManyAssociation # HasManyAssociation - # HasManyThroughAssociation - # HasOneThroughAssociation + # HasManyThroughAssociation + ThroughAssociation + # HasOneAssociation + HasAssociation + # HasOneThroughAssociation + ThroughAssociation # # Association proxies in Active Record are middlemen between the object that # holds the association, known as the <tt>@owner</tt>, and the actual associated @@ -167,11 +167,6 @@ module ActiveRecord end protected - # Does the association have a <tt>:dependent</tt> option? - def dependent? - @reflection.options[:dependent] - end - def interpolate_sql(sql, record = nil) @owner.send(:interpolate_sql, sql, record) end @@ -181,20 +176,6 @@ module ActiveRecord @reflection.klass.send(:sanitize_sql, sql, table_name) end - # Assigns the ID of the owner to the corresponding foreign key in +record+. - # If the association is polymorphic the type of the owner is also set. - def set_belongs_to_association_for(record) - if @reflection.options[:as] - record["#{@reflection.options[:as]}_id"] = @owner.id if @owner.persisted? - record["#{@reflection.options[:as]}_type"] = @owner.class.base_class.name.to_s - else - if @owner.persisted? - primary_key = @reflection.options[:primary_key] || :id - record[@reflection.primary_key_name] = @owner.send(primary_key) - end - end - end - # Merges into +options+ the ones coming from the reflection. def merge_options_from_reflection!(options) options.reverse_merge!( @@ -232,6 +213,17 @@ module ActiveRecord {} end + def aliased_table + @reflection.klass.arel_table + end + + # Set the inverse association, if possible + def set_inverse_instance(record) + if record && invertible_for?(record) + record.send("set_#{inverse_reflection_for(record).name}_target", @owner) + end + end + private # Forwards any missing method call to the \target. def method_missing(method, *args) @@ -290,34 +282,21 @@ module ActiveRecord end end - if RUBY_VERSION < '1.9.2' - # Array#flatten has problems with recursive arrays before Ruby 1.9.2. - # Going one level deeper solves the majority of the problems. - def flatten_deeper(array) - array.collect { |element| (element.respond_to?(:flatten) && !element.is_a?(Hash)) ? element.flatten : element }.flatten - end - else - def flatten_deeper(array) - array.flatten - end - end - # Returns the ID of the owner, quoted if needed. def owner_quoted_id @owner.quoted_id end - def set_inverse_instance(record, instance) - return if record.nil? || !we_can_set_the_inverse_on_this?(record) - inverse_relationship = @reflection.inverse_of - unless inverse_relationship.nil? - record.send(:"set_#{inverse_relationship.name}_target", instance) - end + # Can be redefined by subclasses, notably polymorphic belongs_to + # The record parameter is necessary to support polymorphic inverses as we must check for + # the association in the specific class of the record. + def inverse_reflection_for(record) + @reflection.inverse_of end - # Override in subclasses - def we_can_set_the_inverse_on_this?(record) - false + # Is this association invertible? Can be redefined by subclasses. + def invertible_for?(record) + inverse_reflection_for(record) end end end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index bbfe18f9fb..98c1c13524 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -32,7 +32,7 @@ module ActiveRecord @updated = true end - set_inverse_instance(record, @owner) + set_inverse_instance(record) loaded record @@ -69,7 +69,7 @@ module ActiveRecord options ) if @owner[@reflection.primary_key_name] end - set_inverse_instance(the_target, @owner) + set_inverse_instance(the_target) the_target end @@ -83,8 +83,9 @@ module ActiveRecord # NOTE - for now, we're only supporting inverse setting from belongs_to back onto # has_one associations. - def we_can_set_the_inverse_on_this?(record) - @reflection.has_inverse? && @reflection.inverse_of.macro == :has_one + def invertible_for?(record) + inverse = inverse_reflection_for(record) + inverse && inverse.macro == :has_one end def record_id(record) diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index c580de7fbe..90eff7399c 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -14,7 +14,7 @@ module ActiveRecord @updated = true end - set_inverse_instance(record, @owner) + set_inverse_instance(record) loaded record end @@ -38,23 +38,13 @@ module ActiveRecord private - # NOTE - for now, we're only supporting inverse setting from belongs_to back onto - # has_one associations. - def we_can_set_the_inverse_on_this?(record) - if @reflection.has_inverse? - inverse_association = @reflection.polymorphic_inverse_of(record.class) - inverse_association && inverse_association.macro == :has_one - else - false - end + def inverse_reflection_for(record) + @reflection.polymorphic_inverse_of(record.class) end - def set_inverse_instance(record, instance) - return if record.nil? || !we_can_set_the_inverse_on_this?(record) - inverse_relationship = @reflection.polymorphic_inverse_of(record.class) - if inverse_relationship - record.send(:"set_#{inverse_relationship.name}_target", instance) - end + def invertible_for?(record) + inverse = inverse_reflection_for(record) + inverse && inverse.macro == :has_one end def construct_find_scope @@ -71,7 +61,7 @@ module ActiveRecord :include => @reflection.options[:include] ) end - set_inverse_instance(target, @owner) + set_inverse_instance(target) target end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb index a74d0a4ca8..bb6145656e 100644 --- a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb @@ -212,7 +212,7 @@ module ActiveRecord collection = record.send(join_part.reflection.name) collection.loaded collection.target.push(association) - collection.__send__(:set_inverse_instance, association, record) + collection.send(:set_inverse_instance, association) when :belongs_to set_target_and_inverse(join_part, association, record) else @@ -224,7 +224,7 @@ module ActiveRecord def set_target_and_inverse(join_part, association, record) association_proxy = record.send("set_#{join_part.reflection.name}_target", association) - association_proxy.__send__(:set_inverse_instance, association, record) + association_proxy.send(:set_inverse_instance, association) end end end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index e17ac6f2cc..b1d454545f 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -75,10 +75,12 @@ module ActiveRecord "INNER JOIN #{@owner.connection.quote_table_name @reflection.options[:join_table]} ON #{@reflection.quoted_table_name}.#{@reflection.klass.primary_key} = #{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.association_foreign_key}" end - def construct_conditions - sql = "#{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.primary_key_name} = #{owner_quoted_id} " - sql << " AND (#{conditions})" if conditions - sql + def join_table + Arel::Table.new(@reflection.options[:join_table]) + end + + def construct_owner_conditions + super(join_table) end def construct_find_scope @@ -108,6 +110,10 @@ module ActiveRecord [] end end + + def invertible_for?(record) + false + end end end end diff --git a/activerecord/lib/active_record/associations/has_association.rb b/activerecord/lib/active_record/associations/has_association.rb new file mode 100644 index 0000000000..0ecdb696ea --- /dev/null +++ b/activerecord/lib/active_record/associations/has_association.rb @@ -0,0 +1,42 @@ +module ActiveRecord + module Associations + # Included in all has_* associations (i.e. everything except belongs_to) + module HasAssociation #:nodoc: + protected + # Sets the owner attributes on the given record + def set_owner_attributes(record) + if @owner.persisted? + construct_owner_attributes.each { |key, value| record[key] = value } + end + end + + # Returns a hash linking the owner to the association represented by the reflection + def construct_owner_attributes(reflection = @reflection) + attributes = {} + if reflection.macro == :belongs_to + attributes[reflection.association_primary_key] = @owner.send(reflection.primary_key_name) + else + attributes[reflection.primary_key_name] = @owner.send(reflection.active_record_primary_key) + + if reflection.options[:as] + attributes["#{reflection.options[:as]}_type"] = @owner.class.base_class.name + end + end + attributes + end + + # Builds an array of arel nodes from the owner attributes hash + def construct_owner_conditions(table = aliased_table, reflection = @reflection) + construct_owner_attributes(reflection).map do |attr, value| + table[attr].eq(value) + end + end + + def construct_conditions + conditions = construct_owner_conditions + conditions << Arel.sql(sql_conditions) if sql_conditions + aliased_table.create_and(conditions) + end + end + end +end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 4b05cf6e81..c3360463cd 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -54,7 +54,7 @@ module ActiveRecord end def insert_record(record, force = false, validate = true) - set_belongs_to_association_for(record) + set_owner_attributes(record) save_record(record, force, validate) end @@ -79,22 +79,6 @@ module ActiveRecord end end - def target_obsolete? - false - end - - def construct_conditions - if @reflection.options[:as] - sql = - "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " + - "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}" - else - sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}" - end - sql << " AND (#{conditions})" if conditions - sql - end - def construct_find_scope { :conditions => construct_conditions, @@ -106,13 +90,7 @@ module ActiveRecord end def construct_create_scope - create_scoping = {} - set_belongs_to_association_for(create_scoping) - create_scoping - end - - def we_can_set_the_inverse_on_this?(record) - @reflection.inverse_of + construct_owner_attributes end end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 5f4667b4d8..e2b008034e 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -1,17 +1,16 @@ -require "active_record/associations/through_association_scope" require 'active_support/core_ext/object/blank' module ActiveRecord # = Active Record Has Many Through Association module Associations class HasManyThroughAssociation < HasManyAssociation #:nodoc: - include ThroughAssociationScope + include ThroughAssociation alias_method :new, :build def destroy(*records) transaction do - delete_records(flatten_deeper(records)) + delete_records(records.flatten) super end end @@ -21,9 +20,13 @@ module ActiveRecord # have a size larger than zero, and you need to fetch that collection afterwards, it'll take one fewer # SELECT query if you use #length. def size - return @owner.send(:read_attribute, cached_counter_attribute_name) if has_cached_counter? - return @target.size if loaded? - return count + if has_cached_counter? + @owner.send(:read_attribute, cached_counter_attribute_name) + elsif loaded? + @target.size + else + count + end end protected @@ -51,9 +54,9 @@ module ActiveRecord # TODO - add dependent option support def delete_records(records) - klass = @reflection.through_reflection.klass + through_association = @owner.send(@reflection.through_reflection.name) records.each do |associate| - klass.delete_all(construct_join_attributes(associate)) + through_association.where(construct_join_attributes(associate)).delete_all end end @@ -63,16 +66,8 @@ module ActiveRecord scoped.all end - def has_cached_counter? - @owner.attribute_present?(cached_counter_attribute_name) - end - - def cached_counter_attribute_name - "#{@reflection.name}_count" - end - # NOTE - not sure that we can actually cope with inverses here - def we_can_set_the_inverse_on_this?(record) + def invertible_for?(record) false end end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index c49fd6e66a..c32aaf986e 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -2,6 +2,8 @@ module ActiveRecord # = Active Record Belongs To Has One Association module Associations class HasOneAssociation < AssociationProxy #:nodoc: + include HasAssociation + def create(attrs = {}, replace_existing = true) new_record(replace_existing) do |reflection| attrs = merge_with_conditions(attrs) @@ -27,7 +29,7 @@ module ActiveRecord load_target unless @target.nil? || @target == obj - if dependent? && !dont_save + if @reflection.options[:dependent] && !dont_save case @reflection.options[:dependent] when :delete @target.delete if @target.persisted? @@ -49,11 +51,11 @@ module ActiveRecord @target = nil else raise_on_type_mismatch(obj) - set_belongs_to_association_for(obj) + set_owner_attributes(obj) @target = (AssociationProxy === obj ? obj.target : obj) end - set_inverse_instance(obj, @owner) + set_inverse_instance(obj) @loaded = true unless !@owner.persisted? || obj.nil? || dont_save @@ -79,26 +81,16 @@ module ActiveRecord the_target = with_scope(:find => @scope[:find]) do @reflection.klass.find(:first, options) end - set_inverse_instance(the_target, @owner) + set_inverse_instance(the_target) the_target end def construct_find_scope - if @reflection.options[:as] - sql = - "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " + - "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}" - else - sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}" - end - sql << " AND (#{conditions})" if conditions - { :conditions => sql } + { :conditions => construct_conditions } end def construct_create_scope - create_scoping = {} - set_belongs_to_association_for(create_scoping) - create_scoping + construct_owner_attributes end def new_record(replace_existing) @@ -115,17 +107,12 @@ module ActiveRecord else record[@reflection.primary_key_name] = @owner.id if @owner.persisted? self.target = record - set_inverse_instance(record, @owner) + set_inverse_instance(record) end record end - def we_can_set_the_inverse_on_this?(record) - inverse = @reflection.inverse_of - return !inverse.nil? - end - def merge_with_conditions(attrs={}) attrs ||= {} attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash) diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb index eb17935d6a..c9ae930e93 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -1,10 +1,8 @@ -require "active_record/associations/through_association_scope" - module ActiveRecord # = Active Record Has One Through Association module Associations class HasOneThroughAssociation < HasOneAssociation - include ThroughAssociationScope + include ThroughAssociation def replace(new_value) create_through_record(new_value) @@ -13,25 +11,26 @@ module ActiveRecord private - def create_through_record(new_value) #nodoc: - klass = @reflection.through_reflection.klass - - current_object = @owner.send(@reflection.through_reflection.name) + def create_through_record(new_value) + proxy = @owner.send(@reflection.through_reflection.name) || + @owner.send(:association_instance_get, @reflection.through_reflection.name) + record = proxy.target - if current_object - new_value ? current_object.update_attributes(construct_join_attributes(new_value)) : current_object.destroy + if record && !new_value + record.destroy elsif new_value - if @owner.new_record? - self.target = new_value - through_association = @owner.send(:association_instance_get, @reflection.through_reflection.name) - through_association.build(construct_join_attributes(new_value)) + attributes = construct_join_attributes(new_value) + + if record + record.update_attributes(attributes) + elsif @owner.new_record? + proxy.build(attributes) else - @owner.send(@reflection.through_reflection.name, klass.create(construct_join_attributes(new_value))) + proxy.create(attributes) end end end - private def find_target update_stale_state scoped.first diff --git a/activerecord/lib/active_record/associations/through_association_scope.rb b/activerecord/lib/active_record/associations/through_association.rb index e57de84f66..57718285f8 100644 --- a/activerecord/lib/active_record/associations/through_association_scope.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -1,7 +1,7 @@ module ActiveRecord - # = Active Record Through Association Scope + # = Active Record Through Association module Associations - module ThroughAssociationScope + module ThroughAssociation def scoped with_scope(@scope) do @@ -35,8 +35,12 @@ module ActiveRecord } end + # This scope affects the creation of the associated records (not the join records). At the + # moment we only support creating on a :through association when the source reflection is a + # belongs_to. Thus it's not necessary to set a foreign key on the associated record(s), so + # this scope has can legitimately be empty. def construct_create_scope - construct_owner_attributes(@reflection) + { } end def aliased_through_table @@ -47,35 +51,13 @@ module ActiveRecord @reflection.through_reflection.klass.arel_table end - # Build SQL conditions from attributes, qualified by table name. - def construct_conditions - table = aliased_through_table - conditions = construct_owner_attributes(@reflection.through_reflection).map do |attr, value| - table[attr].eq(value) - end - conditions << Arel.sql(sql_conditions) if sql_conditions - table.create_and(conditions) - end - - # Associate attributes pointing to owner - def construct_owner_attributes(reflection) - if as = reflection.options[:as] - { "#{as}_id" => @owner[reflection.active_record_primary_key], - "#{as}_type" => @owner.class.base_class.name } - elsif reflection.macro == :belongs_to - { reflection.klass.primary_key => @owner[reflection.primary_key_name] } - else - { reflection.primary_key_name => @owner[reflection.active_record_primary_key] } - end + def construct_owner_conditions + super(aliased_through_table, @reflection.through_reflection) end - def construct_from - @reflection.table_name - end - - def construct_select(custom_select = nil) - distinct = "DISTINCT #{@reflection.quoted_table_name}.*" if @reflection.options[:uniq] - custom_select || @reflection.options[:select] || distinct + def construct_select + @reflection.options[:select] || + @reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*" end def construct_joins @@ -117,12 +99,10 @@ module ActiveRecord # TODO: revisit this to allow it for deletion, supposing dependent option is supported raise ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) if [:has_one, :has_many].include?(@reflection.source_reflection.macro) - join_attributes = construct_owner_attributes(@reflection.through_reflection) - - join_attributes.merge!( + join_attributes = { @reflection.source_reflection.primary_key_name => associate.send(@reflection.source_reflection.association_primary_key) - ) + } if @reflection.options[:source_type] join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name) diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index b891b2c50e..978cd7fbe3 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -14,11 +14,13 @@ module ActiveRecord # Defines the primary key field -- can be overridden in subclasses. Overwriting will negate any effect of the # primary_key_prefix_type setting, though. def primary_key - reset_primary_key + @primary_key ||= reset_primary_key end def reset_primary_key #:nodoc: - key = get_primary_key(base_class.name) + key = self == base_class ? get_primary_key(base_class.name) : + base_class.primary_key + set_primary_key(key) key end @@ -40,6 +42,9 @@ module ActiveRecord end end + attr_accessor :original_primary_key + attr_writer :primary_key + # Sets the name of the primary key column to use to the given value, # or (if the value is nil or false) to the value returned by the given # block. @@ -48,9 +53,11 @@ module ActiveRecord # set_primary_key "sysid" # end def set_primary_key(value = nil, &block) - define_attr_method :primary_key, value, &block + @primary_key ||= '' + self.original_primary_key = @primary_key + value &&= value.to_s + self.primary_key = block_given? ? instance_eval(&block) : value end - alias :primary_key= :set_primary_key end end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 6aac25ba9b..396ab226a9 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1490,8 +1490,10 @@ MSG attributes.each do |k, v| if k.include?("(") multi_parameter_attributes << [ k, v ] + elsif respond_to?("#{k}=") + send("#{k}=", v) else - respond_to?(:"#{k}=") ? send(:"#{k}=", v) : raise(UnknownAttributeError, "unknown attribute: #{k}") + raise(UnknownAttributeError, "unknown attribute: #{k}") end end @@ -1628,7 +1630,7 @@ MSG # Returns the contents of the record as a nicely formatted string. def inspect attributes_as_nice_string = self.class.column_names.collect { |name| - if has_attribute?(name) || new_record? + if has_attribute?(name) "#{name}: #{attribute_for_inspect(name)}" end }.compact.join(", ") @@ -1816,7 +1818,7 @@ MSG if scope = self.class.send(:current_scoped_methods) create_with = scope.scope_for_create create_with.each { |att,value| - respond_to?(:"#{att}=") && send("#{att}=", value) + respond_to?("#{att}=") && send("#{att}=", value) } end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index a7a12faac2..7489e88eef 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -33,8 +33,9 @@ module ActiveRecord when BigDecimal then value.to_s('F') when Numeric then value.to_s when Date, Time then "'#{quoted_date(value)}'" + when Symbol then "'#{quote_string(value.to_s)}'" else - "'#{quote_string(value.to_s)}'" + "'#{quote_string(value.to_yaml)}'" end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 9ac8fcb176..b05957981d 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -258,11 +258,11 @@ module ActiveRecord # Creates a record with values matching those of the instance attributes # and returns its id. def create - if self.id.nil? && connection.prefetch_primary_key?(self.class.table_name) + if id.nil? && connection.prefetch_primary_key?(self.class.table_name) self.id = connection.next_sequence_value(self.class.sequence_name) end - attributes_values = arel_attributes_values + attributes_values = arel_attributes_values(!id.nil?) new_id = if attributes_values.empty? self.class.unscoped.insert connection.empty_insert_statement_value @@ -282,7 +282,7 @@ module ActiveRecord # that instances loaded from the database would. def attributes_from_column_definition Hash[self.class.columns.map do |column| - [column.name, column.default] unless column.name == self.class.primary_key + [column.name, column.default] end] end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index d4c5c85594..20e983b5f7 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -166,13 +166,19 @@ module ActiveRecord if conditions || options.present? where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates) else + limit = nil + order = [] # Apply limit and order only if they're both present if @limit_value.present? == @order_values.present? - stmt = arel.compile_update(Arel::SqlLiteral.new(@klass.send(:sanitize_sql_for_assignment, updates))) - @klass.connection.update stmt.to_sql - else - except(:limit, :order).update_all(updates) + limit = arel.limit + order = arel.orders end + + stmt = arel.compile_update(Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates))) + stmt.take limit + stmt.order(*order) + stmt.key = @klass.arel_table[@klass.primary_key] + @klass.connection.update stmt.to_sql end end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 443f318067..868f761a33 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -301,8 +301,8 @@ module ActiveRecord # Save the new record state and id of a record so it can be restored later if a transaction fails. def remember_transaction_record_state #:nodoc @_start_transaction_state ||= {} + @_start_transaction_state[:id] = id if has_attribute?(self.class.primary_key) unless @_start_transaction_state.include?(:new_record) - @_start_transaction_state[:id] = id if has_attribute?(self.class.primary_key) @_start_transaction_state[:new_record] = @new_record end unless @_start_transaction_state.include?(:destroyed) @@ -329,7 +329,7 @@ module ActiveRecord @attributes = @attributes.dup if @attributes.frozen? @new_record = restore_state[:new_record] @destroyed = restore_state[:destroyed] - if restore_state[:id] + if restore_state.has_key?(:id) self.id = restore_state[:id] else @attributes.delete(self.class.primary_key) |