diff options
47 files changed, 865 insertions, 791 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 72bbeeec61..1f343f690c 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,41 @@ *Rails 3.1.0 (unreleased)* +* ActiveRecord::Associations::AssociationProxy has been split. There is now an Association class + (and subclasses) which are responsible for operating on associations, and then a separate, + thin wrapper called CollectionProxy, which proxies collection associations. + + This prevents namespace pollution, separates concerns, and will allow further refactorings. + + Singular associations (has_one, belongs_to) no longer have a proxy at all. They simply return + the associated record or nil. This means that you should not use undocumented methods such + as bob.mother.create - use bob.create_mother instead. + + [Jon Leighton] + +* Make has_many :through associations work correctly when you build a record and then save it. This + requires you to set the :inverse_of option on the source reflection on the join model, like so: + + class Post < ActiveRecord::Base + has_many :taggings + has_many :tags, :through => :taggings + end + + class Tagging < ActiveRecord::Base + belongs_to :post + belongs_to :tag, :inverse_of => :tagging # :inverse_of must be set! + end + + class Tag < ActiveRecord::Base + has_many :taggings + has_many :posts, :through => :taggings + end + + post = Post.first + tag = post.tags.build :name => "ruby" + tag.save # will save a Taggable linking to the post + + [Jon Leighton] + * Support the :dependent option on has_many :through associations. For historical and practical reasons, :delete_all is the default deletion strategy employed by association.delete(*records), despite the fact that the default strategy is :nullify for regular has_many. Also, this only diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 52d2c49836..a34a73cf5d 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -124,16 +124,16 @@ module ActiveRecord def add_preloaded_records_to_collection(parent_records, reflection_name, associated_record) parent_records.each do |parent_record| - association_proxy = parent_record.send(reflection_name) - association_proxy.loaded! - association_proxy.target.concat(Array.wrap(associated_record)) - association_proxy.send(:set_inverse_instance, associated_record) + association = parent_record.association(reflection_name) + association.loaded! + association.target.concat(Array.wrap(associated_record)) + association.set_inverse_instance(associated_record) end end def add_preloaded_record_to_collection(parent_records, reflection_name, associated_record) parent_records.each do |parent_record| - parent_record.send(:association_proxy, reflection_name).target = associated_record + parent_record.association(reflection_name).target = associated_record end end @@ -158,7 +158,7 @@ module ActiveRecord seen_keys[seen_key] = true mapped_records = id_to_record_map[seen_key] mapped_records.each do |mapped_record| - association_proxy = mapped_record.send(:association_proxy, reflection_name) + association_proxy = mapped_record.association(reflection_name) association_proxy.target = associated_record association_proxy.send(:set_inverse_instance, associated_record) end @@ -187,7 +187,7 @@ module ActiveRecord id_to_record_map = construct_id_map(records) - records.each { |record| record.send(reflection.name).loaded! } + records.each { |record| record.association(reflection.name).loaded! } options = reflection.options right = Arel::Table.new(options[:join_table]).alias('t0') @@ -233,7 +233,7 @@ module ActiveRecord end def preload_has_one_association(records, reflection, preload_options={}) - return if records.first.send(:association_proxy, reflection.name).loaded? + return if records.first.association(reflection.name).loaded? id_to_record_map = construct_id_map(records, reflection.options[:primary_key]) options = reflection.options @@ -268,7 +268,7 @@ module ActiveRecord foreign_key = reflection.through_reflection_foreign_key id_to_record_map = construct_id_map(records, foreign_key || reflection.options[:primary_key]) - records.each { |record| record.send(reflection.name).loaded! } + records.each { |record| record.association(reflection.name).loaded! } if options[:through] through_records = preload_through_records(records, reflection, options[:through]) @@ -298,7 +298,7 @@ module ActiveRecord # Dont cache the association - we would only be caching a subset records.map { |record| - proxy = record.send(through_association) + proxy = record.association(through_association) if proxy.respond_to?(:target) Array.wrap(proxy.target).tap { proxy.reset } @@ -320,7 +320,7 @@ module ActiveRecord end def preload_belongs_to_association(records, reflection, preload_options={}) - return if records.first.send(:association_proxy, reflection.name).loaded? + return if records.first.association(reflection.name).loaded? options = reflection.options klasses_and_ids = {} diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index a2db592c7d..77110c721b 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -118,17 +118,19 @@ module ActiveRecord # These classes will be loaded when associations are created. # So there is no need to eager load them. - autoload :AssociationCollection, 'active_record/associations/association_collection' - autoload :SingularAssociation, 'active_record/associations/singular_association' - autoload :AssociationProxy, 'active_record/associations/association_proxy' - autoload :ThroughAssociation, 'active_record/associations/through_association' - autoload :BelongsToAssociation, 'active_record/associations/belongs_to_association' + autoload :Association, 'active_record/associations/association' + autoload :SingularAssociation, 'active_record/associations/singular_association' + autoload :CollectionAssociation, 'active_record/associations/collection_association' + autoload :CollectionProxy, 'active_record/associations/collection_proxy' + + 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' - autoload :HasManyAssociation, 'active_record/associations/has_many_association' - autoload :HasManyThroughAssociation, 'active_record/associations/has_many_through_association' - autoload :HasOneAssociation, 'active_record/associations/has_one_association' - autoload :HasOneThroughAssociation, 'active_record/associations/has_one_through_association' + autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association' + autoload :HasManyAssociation, 'active_record/associations/has_many_association' + autoload :HasManyThroughAssociation, 'active_record/associations/has_many_through_association' + autoload :HasOneAssociation, 'active_record/associations/has_one_association' + autoload :HasOneThroughAssociation, 'active_record/associations/has_one_through_association' + autoload :ThroughAssociation, 'active_record/associations/through_association' # Clears out the association cache. def clear_association_cache #:nodoc: @@ -138,29 +140,23 @@ module ActiveRecord # :nodoc: attr_reader :association_cache - protected - - # Returns the proxy for the given association name, instantiating it if it doesn't - # already exist - def association_proxy(name) - association = association_instance_get(name) - - if association.nil? - reflection = self.class.reflect_on_association(name) - association = reflection.proxy_class.new(self, reflection) - association_instance_set(name, association) - end + # Returns the association instance for the given name, instantiating it if it doesn't already exist + def association(name) #:nodoc: + association = association_instance_get(name) - association + if association.nil? + reflection = self.class.reflect_on_association(name) + association = reflection.association_class.new(self, reflection) + association_instance_set(name, association) end + association + end + private # Returns the specified association instance if it responds to :loaded?, nil otherwise. def association_instance_get(name) - if @association_cache.key? name - association = @association_cache[name] - association if association.respond_to?(:loaded?) - end + @association_cache[name] end # Set the specified association instance. @@ -523,6 +519,22 @@ module ActiveRecord # @group.avatars << Avatar.new # this would work if User belonged_to Avatar rather than the other way around # @group.avatars.delete(@group.avatars.last) # so would this # + # If you are using a +belongs_to+ on the join model, it is a good idea to set the + # <tt>:inverse_of</tt> option on the +belongs_to+, which will mean that the following example + # works correctly (where <tt>tags</tt> is a +has_many+ <tt>:through</tt> association): + # + # @post = Post.first + # @tag = @post.tags.build :name => "ruby" + # @tag.save + # + # The last line ought to save the through record (a <tt>Taggable</tt>). This will only work if the + # <tt>:inverse_of</tt> is set: + # + # class Taggable < ActiveRecord::Base + # belongs_to :post + # belongs_to :tag, :inverse_of => :taggings + # end + # # === Polymorphic Associations # # Polymorphic associations on models are not restricted on what types of models they @@ -1043,13 +1055,21 @@ module ActiveRecord # [:as] # Specifies a polymorphic interface (See <tt>belongs_to</tt>). # [:through] - # Specifies a join model through which to perform the query. Options for <tt>:class_name</tt> - # and <tt>:foreign_key</tt> are ignored, as the association uses the source reflection. You - # can only use a <tt>:through</tt> query through a <tt>belongs_to</tt>, <tt>has_one</tt> - # or <tt>has_many</tt> association on the join model. The collection of join models - # can be managed via the collection API. For example, new join models are created for - # newly associated objects, and if some are gone their rows are deleted (directly, - # no destroy callbacks are triggered). + # Specifies a join model through which to perform the query. Options for <tt>:class_name</tt>, + # <tt>:primary_key</tt> and <tt>:foreign_key</tt> are ignored, as the association uses the + # source reflection. You can only use a <tt>:through</tt> query through a <tt>belongs_to</tt>, + # <tt>has_one</tt> or <tt>has_many</tt> association on the join model. + # + # If the association on the join model is a +belongs_to+, the collection can be modified + # and the records on the <tt>:through</tt> model will be automatically created and removed + # as appropriate. Otherwise, the collection is read-only, so you should manipulate the + # <tt>:through</tt> association directly. + # + # If you are going to modify the association (rather than just read from it), then it is + # a good idea to set the <tt>:inverse_of</tt> option on the source association on the + # join model. This allows associated records to be built which will automatically create + # the appropriate join model records when they are saved. (See the 'Association Join Models' + # section above.) # [:source] # Specifies the source association name used by <tt>has_many :through</tt> queries. # Only use it if the name cannot be inferred from the association. @@ -1550,7 +1570,7 @@ module ActiveRecord def association_accessor_methods(reflection) redefine_method(reflection.name) do |*params| force_reload = params.first unless params.empty? - association = association_proxy(reflection.name) + association = association(reflection.name) if force_reload reflection.klass.uncached { association.reload } @@ -1558,18 +1578,18 @@ module ActiveRecord association.reload end - association.target.nil? ? nil : association + association.target end redefine_method("#{reflection.name}=") do |record| - association_proxy(reflection.name).replace(record) + association(reflection.name).replace(record) end end def collection_reader_method(reflection) redefine_method(reflection.name) do |*params| force_reload = params.first unless params.empty? - association = association_proxy(reflection.name) + association = association(reflection.name) if force_reload reflection.klass.uncached { association.reload } @@ -1577,14 +1597,17 @@ module ActiveRecord association.reload end - association + association.proxy end redefine_method("#{reflection.name.to_s.singularize}_ids") do if send(reflection.name).loaded? || reflection.options[:finder_sql] - send(reflection.name).map { |r| r.id } + records = send(reflection.name) + records.map { |r| r.send(reflection.association_primary_key) } else - send(reflection.name).select("#{reflection.quoted_table_name}.#{reflection.klass.primary_key}").except(:includes).map! { |r| r.id } + column = "#{reflection.quoted_table_name}.#{reflection.association_primary_key}" + records = send(reflection.name).select(column).except(:includes) + records.map! { |r| r.send(reflection.association_primary_key) } end end end @@ -1594,7 +1617,7 @@ module ActiveRecord if writer redefine_method("#{reflection.name}=") do |new_value| - association_proxy(reflection.name).replace(new_value) + association(reflection.name).replace(new_value) end redefine_method("#{reflection.name.to_s.singularize}_ids=") do |new_value| @@ -1616,7 +1639,7 @@ module ActiveRecord constructors.each do |name, proxy_name| redefine_method(name) do |*params| attributes = params.first unless params.empty? - association_proxy(reflection.name).send(proxy_name, attributes) + association(reflection.name).send(proxy_name, attributes) end end end diff --git a/activerecord/lib/active_record/associations/association.rb b/activerecord/lib/active_record/associations/association.rb new file mode 100644 index 0000000000..ae745ea7c2 --- /dev/null +++ b/activerecord/lib/active_record/associations/association.rb @@ -0,0 +1,262 @@ +require 'active_support/core_ext/array/wrap' + +module ActiveRecord + module Associations + # = Active Record Associations + # + # This is the root class of all associations ('+ Foo' signifies an included module Foo): + # + # Association + # SingularAssociaton + # HasOneAssociation + # HasOneThroughAssociation + ThroughAssociation + # BelongsToAssociation + # BelongsToPolymorphicAssociation + # CollectionAssociation + # HasAndBelongsToManyAssociation + # HasManyAssociation + # HasManyThroughAssociation + ThroughAssociation + class Association #:nodoc: + attr_reader :owner, :target, :reflection + + delegate :options, :klass, :to => :reflection + + def initialize(owner, reflection) + reflection.check_validity! + + @owner, @reflection = owner, reflection + @updated = false + + reset + construct_scope + end + + # Returns the name of the table of the related class: + # + # post.comments.aliased_table_name # => "comments" + # + def aliased_table_name + @reflection.klass.table_name + end + + # Resets the \loaded flag to +false+ and sets the \target to +nil+. + def reset + @loaded = false + IdentityMap.remove(@target) if defined?(@target) && @target && IdentityMap.enabled? + @target = nil + end + + # Reloads the \target and returns +self+ on success. + def reload + reset + construct_scope + load_target + self unless @target.nil? + end + + # Has the \target been already \loaded? + def loaded? + @loaded + end + + # Asserts the \target has been loaded setting the \loaded flag to +true+. + def loaded! + @loaded = true + @stale_state = stale_state + end + + # The target is stale if the target no longer points to the record(s) that the + # relevant foreign_key(s) refers to. If stale, the association accessor method + # on the owner will reload the target. It's up to subclasses to implement the + # state_state method if relevant. + # + # Note that if the target has not been loaded, it is not considered stale. + def stale_target? + loaded? && @stale_state != stale_state + end + + # Sets the target of this association to <tt>\target</tt>, and the \loaded flag to +true+. + def target=(target) + @target = target + loaded! + end + + def scoped + target_scope.merge(@association_scope) + end + + # Construct the scope for this association. + # + # Note that the association_scope is merged into the targed_scope only when the + # scoped method is called. This is because at that point the call may be surrounded + # by scope.scoping { ... } or with_scope { ... } etc, which affects the scope which + # actually gets built. + def construct_scope + @association_scope = association_scope if target_klass + end + + def association_scope + scope = target_klass.unscoped + scope = scope.create_with(creation_attributes) + 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 + scope = scope.extending(*Array.wrap(@reflection.options[:extend])) + scope.where(construct_owner_conditions) + end + + def aliased_table + target_klass.arel_table + end + + # Set the inverse association, if possible + def set_inverse_instance(record) + if record && invertible_for?(record) + inverse = record.association(inverse_reflection_for(record).name) + inverse.target = @owner + end + end + + # This class of the target. belongs_to polymorphic overrides this to look at the + # polymorphic_type field on the owner. + def target_klass + @reflection.klass + end + + # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the + # through association's scope) + def target_scope + target_klass.scoped + end + + # Loads the \target if needed and returns it. + # + # This method is abstract in the sense that it relies on +find_target+, + # which is expected to be provided by descendants. + # + # If the \target is already \loaded it is just returned. Thus, you can call + # +load_target+ unconditionally to get the \target. + # + # ActiveRecord::RecordNotFound is rescued within the method, and it is + # not reraised. The proxy is \reset and +nil+ is the return value. + def load_target + if find_target? + begin + if IdentityMap.enabled? && association_class && association_class.respond_to?(:base_class) + @target = IdentityMap.get(association_class, @owner[@reflection.foreign_key]) + end + rescue NameError + nil + ensure + @target ||= find_target + end + end + @target = find_target if find_target? + loaded! + target + rescue ActiveRecord::RecordNotFound + reset + end + + private + + def find_target? + !loaded? && (!@owner.new_record? || foreign_key_present?) && target_klass + end + + def interpolate(sql, record = nil) + if sql.respond_to?(:to_proc) + @owner.send(:instance_exec, record, &sql) + else + sql + end + end + + def select_value + @reflection.options[:select] + end + + # Implemented by (some) subclasses + def creation_attributes + { } + 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[reflection.foreign_key] + else + attributes[reflection.foreign_key] = @owner[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) + conditions = construct_owner_attributes(reflection).map do |attr, value| + table[attr].eq(value) + end + table.create_and(conditions) + end + + # 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 + + # Should be true if there is a foreign key present on the @owner which + # references the target. This is used to determine whether we can load + # the target if the @owner is currently a new record (and therefore + # without a key). + # + # Currently implemented by belongs_to (vanilla and polymorphic) and + # has_one/has_many :through associations which go through a belongs_to + def foreign_key_present? + false + end + + # Raises ActiveRecord::AssociationTypeMismatch unless +record+ is of + # the kind of the class of the associated objects. Meant to be used as + # a sanity check when you are about to assign an associated record. + def raise_on_type_mismatch(record) + unless record.is_a?(@reflection.klass) || record.is_a?(@reflection.class_name.constantize) + message = "#{@reflection.class_name}(##{@reflection.klass.object_id}) expected, got #{record.class}(##{record.class.object_id})" + raise ActiveRecord::AssociationTypeMismatch, message + end + 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 + + # Is this association invertible? Can be redefined by subclasses. + def invertible_for?(record) + inverse_reflection_for(record) + end + + # This should be implemented to return the values of the relevant key(s) on the owner, + # so that when state_state is different from the value stored on the last find_target, + # the target is stale. + # + # This is only relevant to certain associations, which is why it returns nil by default. + def stale_state + end + + def association_class + @reflection.klass + end + end + end +end diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb deleted file mode 100644 index d16cda5585..0000000000 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ /dev/null @@ -1,348 +0,0 @@ -require 'active_support/core_ext/array/wrap' - -module ActiveRecord - module Associations - # = Active Record Associations - # - # This is the root class of all association proxies ('+ Foo' signifies an included module Foo): - # - # AssociationProxy - # SingularAssociaton - # HasOneAssociation - # HasOneThroughAssociation + ThroughAssociation - # BelongsToAssociation - # BelongsToPolymorphicAssociation - # AssociationCollection - # HasAndBelongsToManyAssociation - # HasManyAssociation - # HasManyThroughAssociation + 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 - # object, known as the <tt>@target</tt>. The kind of association any proxy is - # about is available in <tt>@reflection</tt>. That's an instance of the class - # ActiveRecord::Reflection::AssociationReflection. - # - # For example, given - # - # class Blog < ActiveRecord::Base - # has_many :posts - # end - # - # blog = Blog.find(:first) - # - # the association proxy in <tt>blog.posts</tt> has the object in +blog+ as - # <tt>@owner</tt>, the collection of its posts as <tt>@target</tt>, and - # the <tt>@reflection</tt> object represents a <tt>:has_many</tt> macro. - # - # This class has most of the basic instance methods removed, and delegates - # unknown methods to <tt>@target</tt> via <tt>method_missing</tt>. As a - # corner case, it even removes the +class+ method and that's why you get - # - # blog.posts.class # => Array - # - # though the object behind <tt>blog.posts</tt> is not an Array, but an - # ActiveRecord::Associations::HasManyAssociation. - # - # The <tt>@target</tt> object is not \loaded until needed. For example, - # - # blog.posts.count - # - # is computed directly through SQL and does not trigger by itself the - # instantiation of the actual post records. - class AssociationProxy #:nodoc: - alias_method :proxy_extend, :extend - - instance_methods.each { |m| undef_method m unless m.to_s =~ /^(?:nil\?|send|object_id|to_a)$|^__|^respond_to|proxy_/ } - - def initialize(owner, reflection) - @owner, @reflection = owner, reflection - @updated = false - reflection.check_validity! - Array.wrap(reflection.options[:extend]).each { |ext| proxy_extend(ext) } - reset - construct_scope - end - - def to_param - proxy_target.to_param - end - - # Returns the owner of the proxy. - def proxy_owner - @owner - end - - # Returns the reflection object that represents the association handled - # by the proxy. - def proxy_reflection - @reflection - end - - # Does the proxy or its \target respond to +symbol+? - def respond_to?(*args) - super || (load_target && @target.respond_to?(*args)) - end - - # Forwards any missing method call to the \target. - def method_missing(method, *args, &block) - if load_target - return super unless @target.respond_to?(method) - @target.send(method, *args, &block) - end - rescue NoMethodError => e - raise e, e.message.sub(/ for #<.*$/, " via proxy for #{@target}") - end - - # Forwards <tt>===</tt> explicitly to the \target because the instance method - # removal above doesn't catch it. Loads the \target if needed. - def ===(other) - other === load_target - end - - # Returns the name of the table of the related class: - # - # post.comments.aliased_table_name # => "comments" - # - def aliased_table_name - @reflection.klass.table_name - end - - # Resets the \loaded flag to +false+ and sets the \target to +nil+. - def reset - @loaded = false - IdentityMap.remove(@target) if defined?(@target) && @target && IdentityMap.enabled? - @target = nil - end - - # Reloads the \target and returns +self+ on success. - def reload - reset - construct_scope - load_target - self unless @target.nil? - end - - # Has the \target been already \loaded? - def loaded? - @loaded - end - - # Asserts the \target has been loaded setting the \loaded flag to +true+. - def loaded! - @loaded = true - @stale_state = stale_state - end - - # The target is stale if the target no longer points to the record(s) that the - # relevant foreign_key(s) refers to. If stale, the association accessor method - # on the owner will reload the target. It's up to subclasses to implement the - # state_state method if relevant. - # - # Note that if the target has not been loaded, it is not considered stale. - def stale_target? - loaded? && @stale_state != stale_state - end - - # Returns the target of this proxy, same as +proxy_target+. - attr_reader :target - - # Returns the \target of the proxy, same as +target+. - alias :proxy_target :target - - # Sets the target of this proxy to <tt>\target</tt>, and the \loaded flag to +true+. - def target=(target) - @target = target - loaded! - end - - # Forwards the call to the target. Loads the \target if needed. - def inspect - load_target.inspect - end - - def send(method, *args) - return super if respond_to?(method) - load_target.send(method, *args) - end - - def scoped - target_scope.merge(@association_scope) - end - - protected - - # Construct the scope for this association. - # - # Note that the association_scope is merged into the targed_scope only when the - # scoped method is called. This is because at that point the call may be surrounded - # by scope.scoping { ... } or with_scope { ... } etc, which affects the scope which - # actually gets built. - def construct_scope - @association_scope = association_scope if target_klass - end - - def association_scope - scope = target_klass.unscoped - scope = scope.create_with(creation_attributes) - 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 - scope = scope.extending(*Array.wrap(@reflection.options[:extend])) - scope.where(construct_owner_conditions) - end - - def aliased_table - target_klass.arel_table - end - - # Set the inverse association, if possible - def set_inverse_instance(record) - if record && invertible_for?(record) - inverse = record.send(:association_proxy, inverse_reflection_for(record).name) - inverse.target = @owner - end - end - - # This class of the target. belongs_to polymorphic overrides this to look at the - # polymorphic_type field on the owner. - def target_klass - @reflection.klass - end - - # Can be overridden (i.e. in ThroughAssociation) to merge in other scopes (i.e. the - # through association's scope) - def target_scope - target_klass.scoped - end - - # Loads the \target if needed and returns it. - # - # This method is abstract in the sense that it relies on +find_target+, - # which is expected to be provided by descendants. - # - # If the \target is already \loaded it is just returned. Thus, you can call - # +load_target+ unconditionally to get the \target. - # - # ActiveRecord::RecordNotFound is rescued within the method, and it is - # not reraised. The proxy is \reset and +nil+ is the return value. - def load_target - if find_target? - begin - if IdentityMap.enabled? && association_class && association_class.respond_to?(:base_class) - @target = IdentityMap.get(association_class, @owner[@reflection.foreign_key]) - end - rescue NameError - nil - ensure - @target ||= find_target - end - end - loaded! - target - rescue ActiveRecord::RecordNotFound - reset - end - - private - - def find_target? - !loaded? && (!@owner.new_record? || foreign_key_present?) && target_klass - end - - def interpolate(sql, record = nil) - if sql.respond_to?(:to_proc) - @owner.send(:instance_exec, record, &sql) - else - sql - end - end - - def select_value - @reflection.options[:select] - end - - # Implemented by (some) subclasses - def creation_attributes - { } - 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[reflection.foreign_key] - else - attributes[reflection.foreign_key] = @owner[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) - conditions = construct_owner_attributes(reflection).map do |attr, value| - table[attr].eq(value) - end - table.create_and(conditions) - end - - # 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 - - # Should be true if there is a foreign key present on the @owner which - # references the target. This is used to determine whether we can load - # the target if the @owner is currently a new record (and therefore - # without a key). - # - # Currently implemented by belongs_to (vanilla and polymorphic) and - # has_one/has_many :through associations which go through a belongs_to - def foreign_key_present? - false - end - - # Raises ActiveRecord::AssociationTypeMismatch unless +record+ is of - # the kind of the class of the associated objects. Meant to be used as - # a sanity check when you are about to assign an associated record. - def raise_on_type_mismatch(record) - unless record.is_a?(@reflection.klass) || record.is_a?(@reflection.class_name.constantize) - message = "#{@reflection.class_name}(##{@reflection.klass.object_id}) expected, got #{record.class}(##{record.class.object_id})" - raise ActiveRecord::AssociationTypeMismatch, message - end - 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 - - # Is this association invertible? Can be redefined by subclasses. - def invertible_for?(record) - inverse_reflection_for(record) - end - - # This should be implemented to return the values of the relevant key(s) on the owner, - # so that when state_state is different from the value stored on the last find_target, - # the target is stale. - # - # This is only relevant to certain associations, which is why it returns nil by default. - def stale_state - end - - def association_class - @reflection.klass - end - end - end -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 ca6467d911..b711ff35ca 100644 --- a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb @@ -209,10 +209,10 @@ module ActiveRecord association = join_part.instantiate(row) case macro when :has_many, :has_and_belongs_to_many - collection = record.send(join_part.reflection.name) - collection.loaded! - collection.target.push(association) - collection.send(:set_inverse_instance, association) + other = record.association(join_part.reflection.name) + other.loaded! + other.target.push(association) + other.set_inverse_instance(association) when :belongs_to set_target_and_inverse(join_part, association, record) else @@ -223,9 +223,9 @@ module ActiveRecord end def set_target_and_inverse(join_part, association, record) - association_proxy = record.send(:association_proxy, join_part.reflection.name) - association_proxy.target = association - association_proxy.send(:set_inverse_instance, association) + other = record.association(join_part.reflection.name) + other.target = association + other.set_inverse_instance(association) end end end diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/collection_association.rb index ca350f51c9..68631681e4 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -17,8 +17,26 @@ 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: - delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :to => :scoped + class CollectionAssociation < Association #:nodoc: + attr_reader :proxy + + def initialize(owner, reflection) + # When scopes are created via method_missing on the proxy, they are stored so that + # any records fetched from the database are kept around for future use. + @scopes_cache = Hash.new do |hash, method| + hash[method] = { } + end + + super + + @proxy = CollectionProxy.new(self) + end + + def reset + @loaded = false + @target = [] + @scopes_cache.clear + end def select(select = nil) if block_given? @@ -44,17 +62,6 @@ module ActiveRecord first_or_last(:last, *args) end - def to_ary - load_target.dup - end - alias_method :to_a, :to_ary - - def reset - @_scopes_cache = {} - @loaded = false - @target = [] - end - def build(attributes = {}, &block) build_or_create(attributes, :build, &block) end @@ -75,7 +82,7 @@ module ActiveRecord # Add +records+ to this association. Returns +self+ so method calls may be chained. # Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically. - def <<(*records) + def concat(*records) result = true load_target if @owner.new_record? @@ -88,12 +95,9 @@ module ActiveRecord end end - result && self + result && records end - alias_method :push, :<< - alias_method :concat, :<< - # Starts a transaction in the association class's database connection. # # class Author < ActiveRecord::Base @@ -119,13 +123,6 @@ module ActiveRecord end end - # Identical to delete_all, except that the return value is the association (for chaining) - # rather than the records which have been removed. - def clear - delete_all - self - end - # Destroy all the records from this association. # # See destroy for more info. @@ -254,7 +251,7 @@ module ActiveRecord end end - def uniq(collection = self) + def uniq(collection = load_target) seen = {} collection.find_all do |record| seen[record.id] = true unless seen.key?(record.id) @@ -291,70 +288,50 @@ module ActiveRecord end end - def respond_to?(method, include_private = false) - super || @reflection.klass.respond_to?(method, include_private) + def cached_scope(method, args) + @scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args) end - def method_missing(method, *args, &block) - match = DynamicFinderMatch.match(method) - if match && match.creator? - attributes = match.attribute_names - return send(:"find_by_#{attributes.join('_and_')}", *args) || create(Hash[attributes.zip(args)]) - end - - if @target.respond_to?(method) || (!@reflection.klass.respond_to?(method) && Class.respond_to?(method)) - super - elsif @reflection.klass.scopes[method] - @_scopes_cache ||= {} - @_scopes_cache[method] ||= {} - @_scopes_cache[method][args] ||= scoped.readonly(nil).send(method, *args) - else - scoped.readonly(nil).send(method, *args, &block) - end + def association_scope + options = @reflection.options.slice(:order, :limit, :joins, :group, :having, :offset) + super.apply_finder_options(options) end - protected - - def association_scope - options = @reflection.options.slice(:order, :limit, :joins, :group, :having, :offset) - super.apply_finder_options(options) - end - - def load_target - if find_target? - targets = [] + def load_target + if find_target? + targets = [] - begin - targets = find_target - rescue ActiveRecord::RecordNotFound - reset - end - - @target = merge_target_lists(targets, @target) + begin + targets = find_target + rescue ActiveRecord::RecordNotFound + reset end - loaded! - target + @target = merge_target_lists(targets, @target) end - def add_to_target(record) - transaction do - callback(:before_add, record) - yield(record) if block_given? + loaded! + target + end - if @reflection.options[:uniq] && index = @target.index(record) - @target[index] = record - else - @target << record - end + def add_to_target(record) + transaction do + callback(:before_add, record) + yield(record) if block_given? - callback(:after_add, record) - set_inverse_instance(record) + if @reflection.options[:uniq] && index = @target.index(record) + @target[index] = record + else + @target << record end - record + callback(:after_add, record) + set_inverse_instance(record) end + record + end + private def select_value @@ -498,8 +475,8 @@ module ActiveRecord def include_in_memory?(record) if @reflection.is_a?(ActiveRecord::Reflection::ThroughReflection) - @owner.send(proxy_reflection.through_reflection.name).any? { |source| - target = source.send(proxy_reflection.source_reflection.name) + @owner.send(@reflection.through_reflection.name).any? { |source| + target = source.send(@reflection.source_reflection.name) target.respond_to?(:include?) ? target.include?(record) : target == record } || @target.include?(record) else diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb new file mode 100644 index 0000000000..cf77d770c9 --- /dev/null +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -0,0 +1,127 @@ +module ActiveRecord + module Associations + # Association proxies in Active Record are middlemen between the object that + # holds the association, known as the <tt>@owner</tt>, and the actual associated + # object, known as the <tt>@target</tt>. The kind of association any proxy is + # about is available in <tt>@reflection</tt>. That's an instance of the class + # ActiveRecord::Reflection::AssociationReflection. + # + # For example, given + # + # class Blog < ActiveRecord::Base + # has_many :posts + # end + # + # blog = Blog.find(:first) + # + # the association proxy in <tt>blog.posts</tt> has the object in +blog+ as + # <tt>@owner</tt>, the collection of its posts as <tt>@target</tt>, and + # the <tt>@reflection</tt> object represents a <tt>:has_many</tt> macro. + # + # This class has most of the basic instance methods removed, and delegates + # unknown methods to <tt>@target</tt> via <tt>method_missing</tt>. As a + # corner case, it even removes the +class+ method and that's why you get + # + # blog.posts.class # => Array + # + # though the object behind <tt>blog.posts</tt> is not an Array, but an + # ActiveRecord::Associations::HasManyAssociation. + # + # The <tt>@target</tt> object is not \loaded until needed. For example, + # + # blog.posts.count + # + # is computed directly through SQL and does not trigger by itself the + # instantiation of the actual post records. + class CollectionProxy # :nodoc: + alias :proxy_extend :extend + + instance_methods.each { |m| undef_method m unless m.to_s =~ /^(?:nil\?|send|object_id|to_a)$|^__|^respond_to|proxy_/ } + + delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, + :lock, :readonly, :having, :to => :scoped + + delegate :target, :load_target, :loaded?, :scoped, + :to => :@association + + delegate :select, :find, :first, :last, + :build, :create, :create!, + :concat, :delete_all, :destroy_all, :delete, :destroy, :uniq, + :sum, :count, :size, :length, :empty?, + :any?, :many?, :include?, + :to => :@association + + def initialize(association) + @association = association + Array.wrap(association.options[:extend]).each { |ext| proxy_extend(ext) } + end + + def respond_to?(*args) + super || + (load_target && target.respond_to?(*args)) || + @association.klass.respond_to?(*args) + end + + def method_missing(method, *args, &block) + match = DynamicFinderMatch.match(method) + if match && match.creator? + attributes = match.attribute_names + return send(:"find_by_#{attributes.join('_and_')}", *args) || create(Hash[attributes.zip(args)]) + end + + if target.respond_to?(method) || (!@association.klass.respond_to?(method) && Class.respond_to?(method)) + if load_target + if target.respond_to?(method) + target.send(method, *args, &block) + else + begin + super + rescue NoMethodError => e + raise e, e.message.sub(/ for #<.*$/, " via proxy for #{target}") + end + end + end + + elsif @association.klass.scopes[method] + @association.cached_scope(method, args) + else + scoped.readonly(nil).send(method, *args, &block) + end + end + + # Forwards <tt>===</tt> explicitly to the \target because the instance method + # removal above doesn't catch it. Loads the \target if needed. + def ===(other) + other === load_target + end + + def to_ary + load_target.dup + end + alias_method :to_a, :to_ary + + def <<(*records) + @association.concat(records) && self + end + alias_method :push, :<< + + def clear + delete_all + self + end + + def reload + @association.reload + self + end + + def new(*args, &block) + if @association.is_a?(HasManyThroughAssociation) + @association.build(*args, &block) + else + method_missing(:new, *args, &block) + end + end + 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 b9c9919e7a..bcaea5ded4 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 @@ -1,7 +1,7 @@ module ActiveRecord # = Active Record Has And Belongs To Many Association module Associations - class HasAndBelongsToManyAssociation < AssociationCollection #:nodoc: + class HasAndBelongsToManyAssociation < CollectionAssociation #:nodoc: attr_reader :join_table def initialize(owner, reflection) @@ -9,28 +9,26 @@ module ActiveRecord super end - protected + def insert_record(record, validate = true) + return if record.new_record? && !record.save(:validate => validate) - def insert_record(record, validate = true) - return if record.new_record? && !record.save(:validate => validate) + if @reflection.options[:insert_sql] + @owner.connection.insert(interpolate(@reflection.options[:insert_sql], record)) + else + stmt = join_table.compile_insert( + join_table[@reflection.foreign_key] => @owner.id, + join_table[@reflection.association_foreign_key] => record.id + ) - if @reflection.options[:insert_sql] - @owner.connection.insert(interpolate(@reflection.options[:insert_sql], record)) - else - stmt = join_table.compile_insert( - join_table[@reflection.foreign_key] => @owner.id, - join_table[@reflection.association_foreign_key] => record.id - ) - - @owner.connection.insert stmt.to_sql - end - - record + @owner.connection.insert stmt.to_sql end - def association_scope - super.joins(construct_joins) - end + record + end + + def association_scope + super.joins(construct_joins) + end private diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 543b073393..91565b247a 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -5,13 +5,11 @@ module ActiveRecord # # If the association has a <tt>:through</tt> option further specialization # is provided by its child HasManyThroughAssociation. - class HasManyAssociation < AssociationCollection #:nodoc: - protected - - def insert_record(record, validate = true) - set_owner_attributes(record) - record.save(:validate => validate) - end + class HasManyAssociation < CollectionAssociation #:nodoc: + def insert_record(record, validate = true) + set_owner_attributes(record) + record.save(:validate => validate) + end private 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 9f74d57c4d..664c284d45 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -22,7 +22,7 @@ module ActiveRecord end end - def <<(*records) + def concat(*records) unless @owner.new_record? records.flatten.each do |record| raise_on_type_mismatch(record) @@ -33,19 +33,45 @@ module ActiveRecord super end - protected + def insert_record(record, validate = true) + return if record.new_record? && !record.save(:validate => validate) + through_record(record).save! + update_counter(1) + record + end - def insert_record(record, validate = true) - return if record.new_record? && !record.save(:validate => validate) + private - through_association = @owner.send(@reflection.through_reflection.name) - through_association.create!(construct_join_attributes(record)) + def through_record(record) + through_association = @owner.association(@reflection.through_reflection.name) + attributes = construct_join_attributes(record) - update_counter(1) - record + through_record = Array.wrap(through_association.target).find { |candidate| + candidate.attributes.slice(*attributes.keys) == attributes + } + + unless through_record + through_record = through_association.build(attributes) + through_record.send("#{@reflection.source_reflection.name}=", record) + end + + through_record end - private + def build_record(attributes) + record = super(attributes) + + inverse = @reflection.source_reflection.inverse_of + if inverse + if inverse.macro == :has_many + record.send(inverse.name) << through_record(record) + elsif inverse.macro == :has_one + record.send("#{inverse.name}=", through_record(record)) + end + end + + record + end def target_reflection_has_associated_record? if @reflection.through_reflection.macro == :belongs_to && @owner[@reflection.through_reflection.foreign_key].blank? @@ -67,7 +93,7 @@ module ActiveRecord end def delete_records(records, method) - through = @owner.send(:association_proxy, @reflection.through_reflection.name) + through = @owner.association(@reflection.through_reflection.name) scope = through.scoped.where(construct_join_attributes(*records)) case method @@ -79,6 +105,8 @@ module ActiveRecord count = scope.delete_all end + delete_through_records(through, records) + if @reflection.through_reflection.macro == :has_many && update_through_counter?(method) update_counter(-count, @reflection.through_reflection) end @@ -86,6 +114,18 @@ module ActiveRecord update_counter(-count) end + def delete_through_records(through, records) + if @reflection.through_reflection.macro == :has_many + records.each do |record| + through.target.delete(through_record(record)) + end + else + records.each do |record| + through.target = nil if through.target == through_record(record) + end + end + end + def find_target return [] unless target_reflection_has_associated_record? scoped.all 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 69771afe50..112b773ec4 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -1,7 +1,7 @@ module ActiveRecord # = Active Record Has One Through Association module Associations - class HasOneThroughAssociation < HasOneAssociation + class HasOneThroughAssociation < HasOneAssociation #:nodoc: include ThroughAssociation def replace(record) @@ -12,7 +12,7 @@ module ActiveRecord private def create_through_record(record) - through_proxy = @owner.send(:association_proxy, @reflection.through_reflection.name) + through_proxy = @owner.association(@reflection.through_reflection.name) through_record = through_proxy.send(:load_target) if through_record && !record diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index 7f92d9712a..0aa647c63d 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -1,6 +1,6 @@ module ActiveRecord module Associations - class SingularAssociation < AssociationProxy #:nodoc: + class SingularAssociation < Association #:nodoc: def create(attributes = {}) new_record(:create, attributes) end @@ -29,7 +29,7 @@ module ActiveRecord end def check_record(record) - record = record.target if AssociationProxy === record + record = record.target if Association === record raise_on_type_mismatch(record) if record record end diff --git a/activerecord/lib/active_record/associations/through_association.rb b/activerecord/lib/active_record/associations/through_association.rb index 0550bae408..8db8068295 100644 --- a/activerecord/lib/active_record/associations/through_association.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -1,7 +1,7 @@ module ActiveRecord # = Active Record Through Association module Associations - module ThroughAssociation + module ThroughAssociation #:nodoc: protected diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index ec27fd2e63..476598bf88 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -244,7 +244,7 @@ module ActiveRecord # unless the parent is/was a new record itself. def associated_records_to_validate_or_save(association, new_record, autosave) if new_record - association + association && association.target elsif autosave association.target.find_all { |record| record.changed_for_autosave? } else @@ -264,9 +264,9 @@ module ActiveRecord # Validate the association if <tt>:validate</tt> or <tt>:autosave</tt> is # turned on for the association. def validate_single_association(reflection) - if (association = association_instance_get(reflection.name)) && !association.target.nil? - association_valid?(reflection, association) - end + association = association_instance_get(reflection.name) + record = association && association.target + association_valid?(reflection, record) if record end # Validate the associated records if <tt>:validate</tt> or @@ -283,12 +283,12 @@ module ActiveRecord # Returns whether or not the association is valid and applies any errors to # the parent, <tt>self</tt>, if it wasn't. Skips any <tt>:autosave</tt> # enabled records if they're marked_for_destruction? or destroyed. - def association_valid?(reflection, association) - return true if association.destroyed? || association.marked_for_destruction? + def association_valid?(reflection, record) + return true if record.destroyed? || record.marked_for_destruction? - unless valid = association.valid? + unless valid = record.valid? if reflection.options[:autosave] - association.errors.each do |attribute, message| + record.errors.each do |attribute, message| attribute = "#{reflection.name}.#{attribute}" errors[attribute] << message errors[attribute].uniq! @@ -327,12 +327,12 @@ module ActiveRecord saved = true if autosave && record.marked_for_destruction? - association.destroy(record) + association.proxy.destroy(record) elsif autosave != false && (@new_record_before_save || record.new_record?) if autosave - saved = association.send(:insert_record, record, false) + saved = association.insert_record(record, false) else - association.send(:insert_record, record) + association.insert_record(record) end elsif autosave saved = record.save(:validate => false) @@ -361,16 +361,18 @@ module ActiveRecord # This all happens inside a transaction, _if_ the Transactions module is included into # ActiveRecord::Base after the AutosaveAssociation module, which it does by default. def save_has_one_association(reflection) - if (association = association_instance_get(reflection.name)) && !association.target.nil? && !association.destroyed? + association = association_instance_get(reflection.name) + record = association && association.load_target + if record && !record.destroyed? autosave = reflection.options[:autosave] - if autosave && association.marked_for_destruction? - association.destroy + if autosave && record.marked_for_destruction? + record.destroy else key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id - if autosave != false && (new_record? || association.new_record? || association[reflection.foreign_key] != key || autosave) - association[reflection.foreign_key] = key - saved = association.save(:validate => !autosave) + if autosave != false && (new_record? || record.new_record? || record[reflection.foreign_key] != key || autosave) + record[reflection.foreign_key] = key + saved = record.save(:validate => !autosave) raise ActiveRecord::Rollback if !saved && autosave saved end @@ -382,16 +384,18 @@ module ActiveRecord # # In addition, it will destroy the association if it was marked for destruction. def save_belongs_to_association(reflection) - if (association = association_instance_get(reflection.name)) && !association.destroyed? + association = association_instance_get(reflection.name) + record = association && association.load_target + if record && !record.destroyed? autosave = reflection.options[:autosave] - if autosave && association.marked_for_destruction? - association.destroy + if autosave && record.marked_for_destruction? + record.destroy elsif autosave != false - saved = association.save(:validate => !autosave) if association.new_record? || (autosave && association.changed_for_autosave?) + saved = record.save(:validate => !autosave) if record.new_record? || (autosave && record.changed_for_autosave?) if association.updated? - association_id = association.send(reflection.options[:primary_key] || :id) + association_id = record.send(reflection.options[:primary_key] || :id) self[reflection.foreign_key] = association_id association.loaded! end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 20be4a22ea..576450bc3a 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -845,14 +845,18 @@ module ActiveRecord # Adds a new column to the named table. # See TableDefinition#column for details of the options you can use. def add_column(table_name, column_name, type, options = {}) - default = options[:default] - notnull = options[:null] == false + add_column_sql = "ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}" + add_column_options!(add_column_sql, options) - # Add the column. - execute("ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}") + begin + execute add_column_sql + rescue ActiveRecord::StatementInvalid => e + raise e if postgresql_version > 80000 - change_column_default(table_name, column_name, default) if options_include_default?(options) - change_column_null(table_name, column_name, false, default) if notnull + execute("ALTER TABLE #{quote_table_name(table_name)} ADD COLUMN #{quote_column_name(column_name)} #{type_to_sql(type, options[:limit], options[:precision], options[:scale])}") + change_column_default(table_name, column_name, options[:default]) if options_include_default?(options) + change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + end end # Changes the column of a table. diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index f065ef7753..d523c643ba 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -446,7 +446,6 @@ class FixturesFileNotFound < StandardError; end class Fixtures MAX_ID = 2 ** 30 - 1 - DEFAULT_FILTER_RE = /\.ya?ml$/ @@all_cached_fixtures = Hash.new { |h,k| h[k] = {} } @@ -480,8 +479,7 @@ class Fixtures cache_for_connection(connection).update(fixtures_map) end - def self.instantiate_fixtures(object, table_name, fixtures, load_instances = true) - object.instance_variable_set "@#{table_name.to_s.gsub('.','_')}", fixtures + def self.instantiate_fixtures(object, fixture_name, fixtures, load_instances = true) if load_instances fixtures.each do |name, fixture| begin @@ -567,11 +565,10 @@ class Fixtures attr_reader :table_name, :name, :fixtures, :model_class - def initialize(connection, table_name, class_name, fixture_path, file_filter = DEFAULT_FILTER_RE) + def initialize(connection, table_name, class_name, fixture_path) @connection = connection @table_name = table_name @fixture_path = fixture_path - @file_filter = file_filter @name = table_name # preserve fixture base name @class_name = class_name @@ -842,17 +839,17 @@ module ActiveRecord self.fixture_class_names = self.fixture_class_names.merge(class_names) end - def fixtures(*table_names) - if table_names.first == :all - table_names = Dir["#{fixture_path}/**/*.{yml,csv}"] - table_names.map! { |f| f[(fixture_path.size + 1)..-5] } + def fixtures(*fixture_names) + if fixture_names.first == :all + fixture_names = Dir["#{fixture_path}/**/*.{yml,csv}"] + fixture_names.map! { |f| f[(fixture_path.size + 1)..-5] } else - table_names = table_names.flatten.map { |n| n.to_s } + fixture_names = fixture_names.flatten.map { |n| n.to_s } end - self.fixture_table_names |= table_names - require_fixture_classes(table_names) - setup_fixture_accessors(table_names) + self.fixture_table_names |= fixture_names + require_fixture_classes(fixture_names) + setup_fixture_accessors(fixture_names) end def try_to_load_dependency(file_name) @@ -867,40 +864,43 @@ module ActiveRecord end end - def require_fixture_classes(table_names = nil) - (table_names || fixture_table_names).each do |table_name| - file_name = table_name.to_s + def require_fixture_classes(fixture_names = nil) + (fixture_names || fixture_table_names).each do |fixture_name| + file_name = fixture_name.to_s file_name = file_name.singularize if ActiveRecord::Base.pluralize_table_names try_to_load_dependency(file_name) end end - def setup_fixture_accessors(table_names = nil) - table_names = Array.wrap(table_names || fixture_table_names) - table_names.each do |table_name| - table_name = table_name.to_s.tr('./', '_') + def setup_fixture_accessors(fixture_names = nil) + fixture_names = Array.wrap(fixture_names || fixture_table_names) + methods = Module.new do + fixture_names.each do |fixture_name| + fixture_name = fixture_name.to_s.tr('./', '_') - redefine_method(table_name) do |*fixtures| - force_reload = fixtures.pop if fixtures.last == true || fixtures.last == :reload + define_method(fixture_name) do |*fixtures| + force_reload = fixtures.pop if fixtures.last == true || fixtures.last == :reload - @fixture_cache[table_name] ||= {} + @fixture_cache[fixture_name] ||= {} - instances = fixtures.map do |fixture| - @fixture_cache[table_name].delete(fixture) if force_reload + instances = fixtures.map do |fixture| + @fixture_cache[fixture_name].delete(fixture) if force_reload - if @loaded_fixtures[table_name][fixture.to_s] - ActiveRecord::IdentityMap.without do - @fixture_cache[table_name][fixture] ||= @loaded_fixtures[table_name][fixture.to_s].find + if @loaded_fixtures[fixture_name][fixture.to_s] + ActiveRecord::IdentityMap.without do + @fixture_cache[fixture_name][fixture] ||= @loaded_fixtures[fixture_name][fixture.to_s].find + end + else + raise StandardError, "No fixture with name '#{fixture}' found for table '#{fixture_name}'" end - else - raise StandardError, "No fixture with name '#{fixture}' found for table '#{table_name}'" end - end - instances.size == 1 ? instances.first : instances + instances.size == 1 ? instances.first : instances + end + private fixture_name end - private table_name end + include methods end def uses_transaction(*methods) @@ -920,7 +920,7 @@ module ActiveRecord end def setup_fixtures - return unless defined?(ActiveRecord) && !ActiveRecord::Base.configurations.blank? + return unless !ActiveRecord::Base.configurations.blank? if pre_loaded_fixtures && !use_transactional_fixtures raise RuntimeError, 'pre_loaded_fixtures requires use_transactional_fixtures' @@ -985,8 +985,8 @@ module ActiveRecord Fixtures.instantiate_all_loaded_fixtures(self, load_instances?) else raise RuntimeError, 'Load fixtures before instantiating them.' if @loaded_fixtures.nil? - @loaded_fixtures.each do |table_name, fixtures| - Fixtures.instantiate_fixtures(self, table_name, fixtures, load_instances?) + @loaded_fixtures.each do |fixture_name, fixtures| + Fixtures.instantiate_fixtures(self, fixture_name, fixtures, load_instances?) end end end diff --git a/activerecord/lib/active_record/locking/pessimistic.rb b/activerecord/lib/active_record/locking/pessimistic.rb index d900831e13..557b277d6b 100644 --- a/activerecord/lib/active_record/locking/pessimistic.rb +++ b/activerecord/lib/active_record/locking/pessimistic.rb @@ -40,7 +40,7 @@ module ActiveRecord # # Database-specific information on row locking: # MySQL: http://dev.mysql.com/doc/refman/5.1/en/innodb-locking-reads.html - # PostgreSQL: http://www.postgresql.org/docs/8.1/interactive/sql-select.html#SQL-FOR-UPDATE-SHARE + # PostgreSQL: http://www.postgresql.org/docs/current/interactive/sql-select.html#SQL-FOR-UPDATE-SHARE module Pessimistic # Obtain a row lock on this record. Reloads the record to obtain the requested # lock. Pass an SQL locking clause to append the end of the SELECT statement diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 52bdadcbb4..522c0cfc9f 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -387,13 +387,13 @@ module ActiveRecord end end - association = send(association_name) + association = association(association_name) existing_records = if association.loaded? - association.to_a + association.target 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.scoped.where(association.klass.primary_key => attribute_ids) end attributes_collection.each do |attributes| @@ -413,12 +413,12 @@ module ActiveRecord unless association.loaded? || call_reject_if(association_name, attributes) # Make sure we are operating on the actual object which is in the association's # proxy_target array (either by finding it, or adding it if not found) - target_record = association.proxy_target.detect { |record| record == existing_record } + target_record = association.target.detect { |record| record == existing_record } if target_record existing_record = target_record else - association.send(:add_to_target, existing_record) + association.add_to_target(existing_record) end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index ceeb0ec39d..4093a1a209 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -313,7 +313,7 @@ module ActiveRecord macro == :belongs_to end - def proxy_class + def association_class case macro when :belongs_to if options[:polymorphic] @@ -394,6 +394,10 @@ module ActiveRecord @source_reflection_names ||= (options[:source] ? [options[:source]] : [name.to_s.singularize, name]).collect { |n| n.to_sym } end + def association_primary_key + source_reflection.association_primary_key + end + def check_validity! if through_reflection.nil? raise HasManyThroughAssociationNotFoundError.new(active_record.name, self) diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 61d9974570..9633fd3d82 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -18,7 +18,10 @@ module ActiveRecord attribute = table[column.to_sym] case value - when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::Relation + when ActiveRecord::Relation + value.select_values = [value.klass.arel_table['id']] if value.select_values.empty? + attribute.in(value.arel.ast) + when Array, ActiveRecord::Associations::CollectionProxy values = value.to_a.map { |x| x.is_a?(ActiveRecord::Base) ? x.id : x } diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6a905b8588..f76681e880 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -209,16 +209,7 @@ module ActiveRecord def collapse_wheres(arel, wheres) equalities = wheres.grep(Arel::Nodes::Equality) - groups = equalities.group_by do |equality| - equality.left - end - - groups.each do |_, eqls| - test = eqls.inject(eqls.shift) do |memo, expr| - memo.or(expr) - end - arel.where(test) - end + arel.where(Arel::Nodes::And.new(equalities)) unless equalities.empty? (wheres - equalities).each do |where| where = Arel.sql(where) if String === where diff --git a/activerecord/test/cases/associations/association_proxy_test.rb b/activerecord/test/cases/associations/association_proxy_test.rb deleted file mode 100644 index 55d8da4c4e..0000000000 --- a/activerecord/test/cases/associations/association_proxy_test.rb +++ /dev/null @@ -1,52 +0,0 @@ -require "cases/helper" - -module ActiveRecord - module Associations - class AsssociationProxyTest < ActiveRecord::TestCase - class FakeOwner - attr_accessor :new_record - alias :new_record? :new_record - - def initialize - @new_record = false - end - end - - class FakeReflection < Struct.new(:options, :klass) - def initialize options = {}, klass = nil - super - end - - def check_validity! - true - end - end - - class FakeTarget - end - - class FakeTargetProxy < AssociationProxy - def association_scope - true - end - - def find_target - FakeTarget.new - end - end - - def test_method_missing_error - reflection = FakeReflection.new({}, Object.new) - owner = FakeOwner.new - proxy = FakeTargetProxy.new(owner, reflection) - - exception = assert_raises(NoMethodError) do - proxy.omg - end - - assert_match('omg', exception.message) - assert_match(FakeTarget.name, exception.message) - end - end - end -end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 01073bca3d..9006914508 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -50,11 +50,6 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_nothing_raised { account.firm = account.firm } end - def test_triple_equality - assert Client.find(3).firm === Firm - assert Firm === Client.find(3).firm - end - def test_type_mismatch assert_raise(ActiveRecord::AssociationTypeMismatch) { Account.find(1).firm = 1 } assert_raise(ActiveRecord::AssociationTypeMismatch) { Account.find(1).firm = Project.find(1) } @@ -569,13 +564,15 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase def test_reloading_association_with_key_change client = companies(:second_client) - firm = client.firm # note this is a proxy object + firm = client.association(:firm) client.firm = companies(:another_firm) - assert_equal companies(:another_firm), firm.reload + firm.reload + assert_equal companies(:another_firm), firm.target client.client_of = companies(:first_firm).id - assert_equal companies(:first_firm), firm.reload + firm.reload + assert_equal companies(:first_firm), firm.target end def test_polymorphic_counter_cache diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 73b1d6c500..efdecd4b09 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -113,6 +113,24 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert posts(:thinking).reload.people(true).collect(&:first_name).include?("Ted") end + def test_build_then_save_with_has_many_inverse + post = posts(:thinking) + person = post.people.build(:first_name => "Bob") + person.save + post.reload + + assert post.people.include?(person) + end + + def test_build_then_save_with_has_one_inverse + post = posts(:thinking) + person = post.single_people.build(:first_name => "Bob") + person.save + post.reload + + assert post.single_people.include?(person) + end + def test_delete_association assert_queries(2){posts(:welcome);people(:michael); } @@ -718,4 +736,14 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_equal post.tags, post.interpolated_tags assert_equal post.tags, post.interpolated_tags_2 end + + def test_primary_key_option_on_source + post = posts(:welcome) + category = categories(:general) + categorization = Categorization.create!(:post_id => post.id, :named_category_name => category.name) + + assert_equal [category], post.named_categories + assert_equal [category.name], post.named_category_ids # checks when target loaded + assert_equal [category.name], post.reload.named_category_ids # checks when target no loaded + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 6b73a35905..c1dad5e246 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -66,11 +66,6 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_nothing_raised { company.account = company.account } end - def test_triple_equality - assert Account === companies(:first_firm).account - assert companies(:first_firm).account === Account - end - def test_type_mismatch assert_raise(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = 1 } assert_raise(ActiveRecord::AssociationTypeMismatch) { companies(:first_firm).account = Project.find(1) } @@ -320,7 +315,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_creation_failure_without_dependent_option pirate = pirates(:blackbeard) - orig_ship = pirate.ship.target + orig_ship = pirate.ship assert_equal ships(:black_pearl), orig_ship new_ship = pirate.create_ship @@ -333,7 +328,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase def test_creation_failure_with_dependent_option pirate = pirates(:blackbeard).becomes(DestructivePirate) - orig_ship = pirate.dependent_ship.target + orig_ship = pirate.dependent_ship new_ship = pirate.create_dependent_ship assert new_ship.new_record? diff --git a/activerecord/test/cases/associations/has_one_through_associations_test.rb b/activerecord/test/cases/associations/has_one_through_associations_test.rb index c1abba9dbf..bfc5ddc747 100644 --- a/activerecord/test/cases/associations/has_one_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_through_associations_test.rb @@ -139,7 +139,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase def test_assigning_association_correctly_assigns_target new_member = Member.create(:name => "Chris") new_member.club = new_club = Club.create(:name => "LRUG") - assert_equal new_club, new_member.club.target + assert_equal new_club, new_member.association(:club).target end def test_has_one_through_proxy_should_not_respond_to_private_methods @@ -197,7 +197,7 @@ class HasOneThroughAssociationsTest < ActiveRecord::TestCase MemberDetail.find(:all, :include => :member_type) end @new_detail = @member_details[0] - assert @new_detail.send(:association_proxy, :member_type).loaded? + assert @new_detail.send(:association, :member_type).loaded? assert_not_nil assert_no_queries { @new_detail.member_type } end diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index e9a57a00a0..76282213d8 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -137,7 +137,7 @@ class InverseHasOneTests < ActiveRecord::TestCase def test_parent_instance_should_be_shared_with_newly_created_child_via_bang_method m = Man.find(:first) - f = m.face.create!(:description => 'haunted') + f = m.create_face!(:description => 'haunted') assert_not_nil f.man assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" m.name = 'Bongo' @@ -158,18 +158,6 @@ class InverseHasOneTests < ActiveRecord::TestCase assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance" end - def test_parent_instance_should_be_shared_with_replaced_via_method_child - m = Man.find(:first) - f = Face.new(:description => 'haunted') - m.face.replace(f) - assert_not_nil f.man - assert_equal m.name, f.man.name, "Name of man should be the same before changes to parent instance" - m.name = 'Bongo' - assert_equal m.name, f.man.name, "Name of man should be the same after changes to parent instance" - f.man.name = 'Mungo' - assert_equal m.name, f.man.name, "Name of man should be the same after changes to replaced-child-owned instance" - end - def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).dirty_face } end @@ -271,18 +259,6 @@ class InverseHasManyTests < ActiveRecord::TestCase assert_equal m.name, i.man.name, "Name of man should be the same after changes to replaced-child-owned instance" end - def test_parent_instance_should_be_shared_with_replaced_via_method_children - m = Man.find(:first) - i = Interest.new(:topic => 'Industrial Revolution Re-enactment') - m.interests.replace([i]) - assert_not_nil i.man - assert_equal m.name, i.man.name, "Name of man should be the same before changes to parent instance" - m.name = 'Bongo' - assert_equal m.name, i.man.name, "Name of man should be the same after changes to parent instance" - i.man.name = 'Mungo' - assert_equal m.name, i.man.name, "Name of man should be the same after changes to replaced-child-owned instance" - end - def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Man.find(:first).secret_interests } end @@ -366,19 +342,6 @@ class InverseBelongsToTests < ActiveRecord::TestCase assert_equal f.description, m.face.description, "Description of face should be the same after changes to replaced-parent-owned instance" end - def test_child_instance_should_be_shared_with_replaced_via_method_parent - f = faces(:trusting) - assert_not_nil f.man - m = Man.new(:name => 'Charles') - f.man.replace(m) - assert_not_nil m.face - assert_equal f.description, m.face.description, "Description of face should be the same before changes to child instance" - f.description = 'gormless' - assert_equal f.description, m.face.description, "Description of face should be the same after changes to child instance" - m.face.description = 'pleasing' - assert_equal f.description, m.face.description, "Description of face should be the same after changes to replaced-parent-owned instance" - end - def test_trying_to_use_inverses_that_dont_exist_should_raise_an_error assert_raise(ActiveRecord::InverseOfAssociationNotFoundError) { Face.find(:first).horrible_man } end @@ -434,7 +397,7 @@ class InversePolymorphicBelongsToTests < ActiveRecord::TestCase new_man = Man.new assert_not_nil face.polymorphic_man - face.polymorphic_man.replace(new_man) + face.polymorphic_man = new_man assert_equal face.description, new_man.polymorphic_face.description, "Description of face should be the same before changes to parent instance" face.description = 'Bongo' diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index 34f57d3155..6d7f905dc5 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -153,7 +153,7 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase def test_create_polymorphic_has_one_with_scope old_count = Tagging.count - tagging = posts(:welcome).tagging.create(:tag => tags(:misc)) + tagging = posts(:welcome).create_tagging(:tag => tags(:misc)) assert_equal "Post", tagging.taggable_type assert_equal old_count+1, Tagging.count end diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index 83c605d2bb..a1996fcf27 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -133,25 +133,6 @@ end class AssociationProxyTest < ActiveRecord::TestCase fixtures :authors, :posts, :categorizations, :categories, :developers, :projects, :developers_projects - def test_proxy_accessors - welcome = posts(:welcome) - assert_equal welcome, welcome.author.proxy_owner - assert_equal welcome.class.reflect_on_association(:author), welcome.author.proxy_reflection - welcome.author.class # force load target - assert_equal welcome.author, welcome.author.proxy_target - - david = authors(:david) - assert_equal david, david.posts.proxy_owner - assert_equal david.class.reflect_on_association(:posts), david.posts.proxy_reflection - david.posts.class # force load target - assert_equal david.posts, david.posts.proxy_target - - assert_equal david, david.posts_with_extension.testing_proxy_owner - assert_equal david.class.reflect_on_association(:posts_with_extension), david.posts_with_extension.testing_proxy_reflection - david.posts_with_extension.class # force load target - assert_equal david.posts_with_extension, david.posts_with_extension.testing_proxy_target - end - def test_push_does_not_load_target david = authors(:david) @@ -216,16 +197,6 @@ class AssociationProxyTest < ActiveRecord::TestCase assert_equal post.body, "More cool stuff!" end - def test_failed_reload_returns_nil - p = setup_dangling_association - assert_nil p.author.reload - end - - def test_failed_reset_returns_nil - p = setup_dangling_association - assert_nil p.author.reset - end - def test_reload_returns_assocition david = developers(:david) assert_nothing_raised do @@ -240,13 +211,6 @@ class AssociationProxyTest < ActiveRecord::TestCase [*author] end end - - def setup_dangling_association - josh = Author.create(:name => "Josh") - p = Post.create(:title => "New on Edge", :body => "More cool stuff!", :author => josh) - josh.destroy - p - end end class OverridingAssociationsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/autosave_association_test.rb b/activerecord/test/cases/autosave_association_test.rb index 8035cd2c6f..ca59b3d6de 100644 --- a/activerecord/test/cases/autosave_association_test.rb +++ b/activerecord/test/cases/autosave_association_test.rb @@ -112,7 +112,7 @@ class TestDefaultAutosaveAssociationOnAHasOneAssociation < ActiveRecord::TestCas def test_build_before_child_saved firm = Firm.find(1) - account = firm.account.build("credit_limit" => 1000) + account = firm.build_account("credit_limit" => 1000) assert_equal account, firm.account assert !account.persisted? assert firm.save diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index ee51692f93..fa40fad56d 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -132,7 +132,6 @@ class FixturesTest < ActiveRecord::TestCase end def test_complete_instantiation - assert_equal 4, @topics.size assert_equal "The First Topic", @first.title end @@ -142,7 +141,6 @@ class FixturesTest < ActiveRecord::TestCase end def test_erb_in_fixtures - assert_equal 11, @developers.size assert_equal "fixture_5", @dev_5.name end @@ -199,7 +197,6 @@ class FixturesTest < ActiveRecord::TestCase end def test_binary_in_fixtures - assert_equal 1, @binaries.size data = File.open(ASSETS_ROOT + "/flowers.jpg", 'rb') { |f| f.read } data.force_encoding('ASCII-8BIT') if data.respond_to?(:force_encoding) data.freeze @@ -304,9 +301,6 @@ class FixturesWithoutInstanceInstantiationTest < ActiveRecord::TestCase def test_without_instance_instantiation assert !defined?(@first), "@first is not defined" - assert_not_nil @topics - assert_not_nil @developers - assert_not_nil @accounts end end @@ -384,6 +378,21 @@ class ForeignKeyFixturesTest < ActiveRecord::TestCase end end +class OverRideFixtureMethodTest < ActiveRecord::TestCase + fixtures :topics + + def topics(name) + topic = super + topic.title = 'omg' + topic + end + + def test_fixture_methods_can_be_overridden + x = topics :first + assert_equal 'omg', x.title + end +end + class CheckSetTableNameFixturesTest < ActiveRecord::TestCase set_fixture_class :funny_jokes => 'Joke' fixtures :funny_jokes diff --git a/activerecord/test/cases/named_scope_test.rb b/activerecord/test/cases/named_scope_test.rb index ed5e1e0cba..d05b0ff947 100644 --- a/activerecord/test/cases/named_scope_test.rb +++ b/activerecord/test/cases/named_scope_test.rb @@ -448,7 +448,7 @@ class NamedScopeTest < ActiveRecord::TestCase [:destroy_all, :reset, :delete_all].each do |method| before = post.comments.containing_the_letter_e - post.comments.send(method) + post.association(:comments).send(method) assert before.object_id != post.comments.containing_the_letter_e.object_id, "AssociationCollection##{method} should reset the named scopes cache" end end diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index d1afe7376a..c57ab7ed28 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -155,7 +155,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase man = Man.find man.id man.interests_attributes = [{:id => interest.id, :topic => 'gardening'}] assert_equal man.interests.first.topic, man.interests[0].topic - end + end end class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase @@ -918,16 +918,16 @@ class TestHasManyAutosaveAssociationWhichItselfHasAutosaveAssociations < ActiveR test "if association is not loaded and association record is saved and then in memory record attributes should be saved" do @ship.parts_attributes=[{:id => @part.id,:name =>'Deck'}] - assert_equal 1, @ship.parts.proxy_target.size + assert_equal 1, @ship.association(:parts).target.size assert_equal 'Deck', @ship.parts[0].name end test "if association is not loaded and child doesn't change and I am saving a grandchild then in memory record should be used" do @ship.parts_attributes=[{:id => @part.id,:trinkets_attributes =>[{:id => @trinket.id, :name => 'Ruby'}]}] - assert_equal 1, @ship.parts.proxy_target.size + assert_equal 1, @ship.association(:parts).target.size assert_equal 'Mast', @ship.parts[0].name - assert_no_difference("@ship.parts[0].trinkets.proxy_target.size") do - @ship.parts[0].trinkets.proxy_target.size + assert_no_difference("@ship.parts[0].association(:trinkets).target.size") do + @ship.parts[0].association(:trinkets).target.size end assert_equal 'Ruby', @ship.parts[0].trinkets[0].name @ship.save diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 0e45f5a374..37bbb17e74 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -474,10 +474,17 @@ class RelationTest < ActiveRecord::TestCase relation = relation.where(:name => david.name) relation = relation.where(:name => 'Santiago') relation = relation.where(:id => david.id) - assert_equal [david], relation.all + assert_equal [], relation.all end - def test_find_all_with_multiple_ors + def test_multi_where_ands_queries + relation = Author.unscoped + david = authors(:david) + sql = relation.where(:name => david.name).where(:name => 'Santiago').to_sql + assert_match('AND', sql) + end + + def test_find_all_with_multiple_should_use_and david = authors(:david) relation = [ { :name => david.name }, @@ -486,7 +493,34 @@ class RelationTest < ActiveRecord::TestCase ].inject(Author.unscoped) do |memo, param| memo.where(param) end - assert_equal [david], relation.all + assert_equal [], relation.all + end + + def test_find_all_using_where_with_relation + david = authors(:david) + # switching the lines below would succeed in current rails + # assert_queries(2) { + assert_queries(1) { + relation = Author.where(:id => Author.where(:id => david.id)) + assert_equal [david], relation.all + } + end + + def test_find_all_using_where_with_relation_with_joins + david = authors(:david) + assert_queries(1) { + relation = Author.where(:id => Author.joins(:posts).where(:id => david.id)) + assert_equal [david], relation.all + } + end + + + def test_find_all_using_where_with_relation_with_select_to_build_subquery + david = authors(:david) + assert_queries(1) { + relation = Author.where(:name => Author.where(:id => david.id).select(:name)) + assert_equal [david], relation.all + } end def test_exists diff --git a/activerecord/test/models/person.rb b/activerecord/test/models/person.rb index a18b9e44df..cc3a4f5f9d 100644 --- a/activerecord/test/models/person.rb +++ b/activerecord/test/models/person.rb @@ -1,5 +1,7 @@ class Person < ActiveRecord::Base has_many :readers + has_one :reader + has_many :posts, :through => :readers has_many :posts_with_no_comments, :through => :readers, :source => :post, :include => :comments, :conditions => 'comments.id is null' diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 5f29196790..a342aaf60b 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -90,10 +90,12 @@ class Post < ActiveRecord::Base has_many :standard_categorizations, :class_name => 'Categorization', :foreign_key => :post_id has_many :author_using_custom_pk, :through => :standard_categorizations has_many :authors_using_custom_pk, :through => :standard_categorizations + has_many :named_categories, :through => :standard_categorizations has_many :readers has_many :readers_with_person, :include => :person, :class_name => "Reader" has_many :people, :through => :readers + has_many :single_people, :through => :readers has_many :people_with_callbacks, :source=>:person, :through => :readers, :before_add => lambda {|owner, reader| log(:added, :before, reader.first_name) }, :after_add => lambda {|owner, reader| log(:added, :after, reader.first_name) }, diff --git a/activerecord/test/models/reader.rb b/activerecord/test/models/reader.rb index 27527bf566..0207a2bd92 100644 --- a/activerecord/test/models/reader.rb +++ b/activerecord/test/models/reader.rb @@ -1,4 +1,5 @@ class Reader < ActiveRecord::Base belongs_to :post - belongs_to :person + belongs_to :person, :inverse_of => :readers + belongs_to :single_person, :class_name => 'Person', :foreign_key => :person_id, :inverse_of => :reader end diff --git a/activeresource/test/cases/validations_test.rb b/activeresource/test/cases/validations_test.rb index bd79fdd952..671d1ea8f0 100644 --- a/activeresource/test/cases/validations_test.rb +++ b/activeresource/test/cases/validations_test.rb @@ -48,6 +48,12 @@ class ValidationsTest < ActiveModel::TestCase assert p.save, "should have saved after fixing the validation, but had: #{p.errors.inspect}" end + def test_client_side_validation_maximum + project = Project.new(:description => '123456789012345') + assert ! project.valid? + assert_equal ['is too long (maximum is 10 characters)'], project.errors[:description] + end + protected # quickie helper to create a new project with all the required diff --git a/activeresource/test/fixtures/project.rb b/activeresource/test/fixtures/project.rb index e15fa6f620..53de666601 100644 --- a/activeresource/test/fixtures/project.rb +++ b/activeresource/test/fixtures/project.rb @@ -1,25 +1,18 @@ # used to test validations class Project < ActiveResource::Base self.site = "http://37s.sunrise.i:3000" + schema do + string :email + string :name + end - validates_presence_of :name + validates :name, :presence => true + validates :description, :presence => false, :length => {:maximum => 10} validate :description_greater_than_three_letters # to test the validate *callback* works def description_greater_than_three_letters errors.add :description, 'must be greater than three letters long' if description.length < 3 unless description.blank? end - - - # stop-gap accessor to default this attribute to nil - # Otherwise the validations fail saying that the method does not exist. - # In future, method_missing will be updated to not explode on a known - # attribute. - def name - attributes['name'] || nil - end - def description - attributes['description'] || nil - end end diff --git a/activesupport/lib/active_support/inflections.rb b/activesupport/lib/active_support/inflections.rb index e7b5387ed7..06ceccdb22 100644 --- a/activesupport/lib/active_support/inflections.rb +++ b/activesupport/lib/active_support/inflections.rb @@ -4,10 +4,12 @@ module ActiveSupport inflect.plural(/s$/i, 's') inflect.plural(/(ax|test)is$/i, '\1es') inflect.plural(/(octop|vir)us$/i, '\1i') + inflect.plural(/(octop|vir)i$/i, '\1i') inflect.plural(/(alias|status)$/i, '\1es') inflect.plural(/(bu)s$/i, '\1ses') inflect.plural(/(buffal|tomat)o$/i, '\1oes') inflect.plural(/([ti])um$/i, '\1a') + inflect.plural(/([ti])a$/i, '\1a') inflect.plural(/sis$/i, 'ses') inflect.plural(/(?:([^f])fe|([lr])f)$/i, '\1\2ves') inflect.plural(/(hive)$/i, '\1s') @@ -15,7 +17,9 @@ module ActiveSupport inflect.plural(/(x|ch|ss|sh)$/i, '\1es') inflect.plural(/(matr|vert|ind)(?:ix|ex)$/i, '\1ices') inflect.plural(/([m|l])ouse$/i, '\1ice') + inflect.plural(/([m|l])ice$/i, '\1ice') inflect.plural(/^(ox)$/i, '\1en') + inflect.plural(/^(oxen)$/i, '\1') inflect.plural(/(quiz)$/i, '\1zes') inflect.singular(/s$/i, '') diff --git a/activesupport/lib/active_support/testing/performance.rb b/activesupport/lib/active_support/testing/performance.rb index 64b436ba8c..8c91a061fb 100644 --- a/activesupport/lib/active_support/testing/performance.rb +++ b/activesupport/lib/active_support/testing/performance.rb @@ -401,7 +401,7 @@ begin Mode = RubyProf::GC_TIME if RubyProf.const_defined?(:GC_TIME) # Ruby 1.9 with GC::Profiler - if GC.respond_to?(:total_time) + if defined?(GC::Profiler) && GC::Profiler.respond_to?(:total_time) def measure GC::Profiler.total_time end diff --git a/activesupport/test/core_ext/module_test.rb b/activesupport/test/core_ext/module_test.rb index 75404ec0e1..a95cf1591f 100644 --- a/activesupport/test/core_ext/module_test.rb +++ b/activesupport/test/core_ext/module_test.rb @@ -26,14 +26,10 @@ module Yz end end -class De -end - Somewhere = Struct.new(:street, :city) Someone = Struct.new(:name, :place) do delegate :street, :city, :to_f, :to => :place - delegate :state, :to => :@place delegate :upcase, :to => "place.city" end diff --git a/activesupport/test/inflector_test.rb b/activesupport/test/inflector_test.rb index 60714a152d..f55116dfab 100644 --- a/activesupport/test/inflector_test.rb +++ b/activesupport/test/inflector_test.rb @@ -63,6 +63,13 @@ class InflectorTest < Test::Unit::TestCase assert_equal(singular.capitalize, ActiveSupport::Inflector.singularize(plural.capitalize)) end end + + SingularToPlural.each do |singular, plural| + define_method "test_pluralize_#{plural}" do + assert_equal(plural, ActiveSupport::Inflector.pluralize(plural)) + assert_equal(plural.capitalize, ActiveSupport::Inflector.pluralize(plural.capitalize)) + end + end def test_overwrite_previous_inflectors assert_equal("series", ActiveSupport::Inflector.singularize("series")) diff --git a/activesupport/test/inflector_test_cases.rb b/activesupport/test/inflector_test_cases.rb index 59515dad32..2b144e5931 100644 --- a/activesupport/test/inflector_test_cases.rb +++ b/activesupport/test/inflector_test_cases.rb @@ -44,6 +44,7 @@ module InflectorTestCases "datum" => "data", "medium" => "media", + "stadium" => "stadia", "analysis" => "analyses", "node_child" => "node_children", diff --git a/railties/lib/rails/generators/named_base.rb b/railties/lib/rails/generators/named_base.rb index 44a2639488..2af7f85463 100644 --- a/railties/lib/rails/generators/named_base.rb +++ b/railties/lib/rails/generators/named_base.rb @@ -118,11 +118,11 @@ module Rails end def singular_table_name - @singular_table_name ||= table_name.singularize + @singular_table_name ||= (pluralize_table_names? ? table_name.singularize : table_name) end def plural_table_name - @plural_table_name ||= table_name.pluralize + @plural_table_name ||= (pluralize_table_names? ? table_name : table_name.pluralize) end def plural_file_name diff --git a/railties/test/generators/named_base_test.rb b/railties/test/generators/named_base_test.rb index 1badae0713..f23701e99e 100644 --- a/railties/test/generators/named_base_test.rb +++ b/railties/test/generators/named_base_test.rb @@ -98,6 +98,11 @@ class NamedBaseTest < Rails::Generators::TestCase assert_name g, 'posts', :index_helper end + def test_index_helper_to_pluralize_once + g = generator ['Stadium'] + assert_name g, 'stadia', :index_helper + end + def test_index_helper_with_uncountable g = generator ['Sheep'] assert_name g, 'sheep_index', :index_helper |