From 8272630ce8af0546e7d1aa9211a9d91b80700cbd Mon Sep 17 00:00:00 2001 From: Bruce Krysiak Date: Tue, 10 Mar 2009 11:17:16 +0000 Subject: Ensure ActiveRecord#to_xml respects :skip_types for included associations [#1632 state:resolved] Signed-off-by: Pratik Naik --- .../lib/active_record/serializers/xml_serializer.rb | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/serializers/xml_serializer.rb b/activerecord/lib/active_record/serializers/xml_serializer.rb index 4749823b94..fa75874603 100644 --- a/activerecord/lib/active_record/serializers/xml_serializer.rb +++ b/activerecord/lib/active_record/serializers/xml_serializer.rb @@ -231,16 +231,22 @@ module ActiveRecord #:nodoc: def add_associations(association, records, opts) if records.is_a?(Enumerable) tag = reformat_name(association.to_s) + type = options[:skip_types] ? {} : {:type => "array"} + if records.empty? - builder.tag!(tag, :type => :array) + builder.tag!(tag, type) else - builder.tag!(tag, :type => :array) do + builder.tag!(tag, type) do association_name = association.to_s.singularize records.each do |record| - record.to_xml opts.merge( - :root => association_name, - :type => (record.class.to_s.underscore == association_name ? nil : record.class.name) - ) + if options[:skip_types] + record_type = {} + else + record_class = (record.class.to_s.underscore == association_name) ? nil : record.class.name + record_type = {:type => record_class} + end + + record.to_xml opts.merge(:root => association_name).merge(record_type) end end end -- cgit v1.2.3 From c3aa2bcdcffb42f578b0e89fe08e1c4e234ccf3b Mon Sep 17 00:00:00 2001 From: Manfred Stienstra Date: Tue, 10 Mar 2009 11:49:58 +0000 Subject: Ensure nested with_scope merges conditions inside out [#2193 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/base.rb | 19 ++++++++----------- activerecord/lib/active_record/named_scope.rb | 4 ++-- 2 files changed, 10 insertions(+), 13 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index edc145985d..4f39761672 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -416,7 +416,7 @@ module ActiveRecord #:nodoc: end @@subclasses = {} - + ## # :singleton-method: # Contains the database configuration - as is typically stored in config/database.yml - @@ -661,7 +661,6 @@ module ActiveRecord #:nodoc: connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) } end - # Returns true if a record exists in the table that matches the +id+ or # conditions given, or false otherwise. The argument can take five forms: # @@ -1003,7 +1002,6 @@ module ActiveRecord #:nodoc: update_counters(id, counter_name => -1) end - # Attributes named in this macro are protected from mass-assignment, # such as new(attributes), # update_attributes(attributes), or @@ -1104,7 +1102,6 @@ module ActiveRecord #:nodoc: read_inheritable_attribute(:attr_serialized) or write_inheritable_attribute(:attr_serialized, {}) end - # Guesses the table name (in forced lower-case) based on the name of the class in the inheritance hierarchy descending # directly from ActiveRecord::Base. So if the hierarchy looks like: Reply < Message < ActiveRecord::Base, then Message is used # to guess the table name even when called on Reply. The rules used to do the guess are handled by the Inflector class @@ -1417,7 +1414,6 @@ module ActiveRecord #:nodoc: end end - def quote_value(value, column = nil) #:nodoc: connection.quote(value,column) end @@ -1486,7 +1482,7 @@ module ActiveRecord #:nodoc: elsif match = DynamicScopeMatch.match(method_id) return true if all_attributes_exists?(match.attribute_names) end - + super end @@ -2014,7 +2010,6 @@ module ActiveRecord #:nodoc: end end - # Defines an "attribute" method (like +inheritance_column+ or # +table_name+). A new (class) method will be created with the # given name. If a value is specified, the new method will @@ -2111,7 +2106,7 @@ module ActiveRecord #:nodoc: end # Merge scopings - if action == :merge && current_scoped_methods + if [:merge, :reverse_merge].include?(action) && current_scoped_methods method_scoping = current_scoped_methods.inject(method_scoping) do |hash, (method, params)| case hash[method] when Hash @@ -2133,7 +2128,11 @@ module ActiveRecord #:nodoc: end end else - hash[method] = hash[method].merge(params) + if action == :reverse_merge + hash[method] = hash[method].merge(params) + else + hash[method] = params.merge(hash[method]) + end end else hash[method] = params @@ -2143,7 +2142,6 @@ module ActiveRecord #:nodoc: end self.scoped_methods << method_scoping - begin yield ensure @@ -2749,7 +2747,6 @@ module ActiveRecord #:nodoc: assign_multiparameter_attributes(multi_parameter_attributes) end - # Returns a hash of all the attributes with their names as keys and the values of the attributes as values. def attributes self.attribute_names.inject({}) do |attrs, name| diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 43411dfb55..519941dd94 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -104,7 +104,7 @@ module ActiveRecord end end end - + class Scope attr_reader :proxy_scope, :proxy_options, :current_scoped_methods_when_defined NON_DELEGATE_METHODS = %w(nil? send object_id class extend find size count sum average maximum minimum paginate first last empty? any? respond_to?).to_set @@ -175,7 +175,7 @@ module ActiveRecord if scopes.include?(method) scopes[method].call(self, *args) else - with_scope :find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {} do + with_scope({:find => proxy_options, :create => proxy_options[:conditions].is_a?(Hash) ? proxy_options[:conditions] : {}}, :reverse_merge) do method = :new if method == :build if current_scoped_methods_when_defined with_scope current_scoped_methods_when_defined do -- cgit v1.2.3 From 19ad375e7afa99dc8bc5d859c478bae19c5ddc18 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Tue, 10 Mar 2009 23:11:05 -0700 Subject: Don't duplicate :order from scope and options, it makes mysql do extra work --- activerecord/lib/active_record/base.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 4f39761672..62bdf0483a 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1741,7 +1741,9 @@ module ActiveRecord #:nodoc: scoped_order = scope[:order] if scope if order sql << " ORDER BY #{order}" - sql << ", #{scoped_order}" if scoped_order + if scoped_order && scoped_order != order + sql << ", #{scoped_order}" + end else sql << " ORDER BY #{scoped_order}" if scoped_order end -- cgit v1.2.3 From 04333482bd665e4d4ced167ff4f566ecfc0cc919 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Wed, 11 Mar 2009 14:08:40 +0000 Subject: Rename ActiveRecord::Base.each to ActiveRecord::Base.find_each --- activerecord/lib/active_record/batches.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/batches.rb b/activerecord/lib/active_record/batches.rb index 9e9c8fbbf4..4ea932f34d 100644 --- a/activerecord/lib/active_record/batches.rb +++ b/activerecord/lib/active_record/batches.rb @@ -11,14 +11,14 @@ module ActiveRecord # # Example: # - # Person.each(:conditions => "age > 21") do |person| + # Person.find_each(:conditions => "age > 21") do |person| # person.party_all_night! # end # # Note: This method is only intended to use for batch processing of large amounts of records that wouldn't fit in # memory all at once. If you just need to loop over less than 1000 records, it's probably better just to use the # regular find methods. - def each(options = {}) + def find_each(options = {}) find_in_batches(options) do |records| records.each { |record| yield record } end -- cgit v1.2.3 From f23adf0ed4bd596c9e7688455489a79f8b35eb3f Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Wed, 11 Mar 2009 14:52:51 +0000 Subject: Add tests for AssociationCollection#find_each and AssociationCollection#find_in_batches --- activerecord/lib/active_record/associations/association_collection.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 0fefec1216..f024f99a34 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -60,7 +60,7 @@ module ActiveRecord @reflection.klass.find(*args) end end - + # Fetches the first one using SQL if possible. def first(*args) if fetch_first_or_last_using_find?(args) -- cgit v1.2.3 From 106976df0911e423042ec4abc165fd561766a047 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Wed, 11 Mar 2009 15:24:30 +0000 Subject: Ensure ActiveRecord::Base.find_in_batches fires doesnt fire an extra query unless needed --- activerecord/lib/active_record/batches.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/batches.rb b/activerecord/lib/active_record/batches.rb index 4ea932f34d..03bd4f9f93 100644 --- a/activerecord/lib/active_record/batches.rb +++ b/activerecord/lib/active_record/batches.rb @@ -49,12 +49,15 @@ module ActiveRecord raise "You can't specify a limit, it's forced to be the batch_size" if options[:limit] start = options.delete(:start).to_i + batch_size = options.delete(:batch_size) || 1000 - with_scope(:find => options.merge(:order => batch_order, :limit => options.delete(:batch_size) || 1000)) do + with_scope(:find => options.merge(:order => batch_order, :limit => batch_size)) do records = find(:all, :conditions => [ "#{table_name}.#{primary_key} >= ?", start ]) while records.any? yield records + + break if records.size < batch_size records = find(:all, :conditions => [ "#{table_name}.#{primary_key} > ?", records.last.id ]) end end -- cgit v1.2.3 From 92dadf6d7927fde1482ba1d96d0916093bfb83ca Mon Sep 17 00:00:00 2001 From: Will Bryant Date: Thu, 12 Mar 2009 02:13:26 +1300 Subject: Fixed autosave checks on objects with hm:t in :include [#2213 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/autosave_association.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 1c3d0567c1..6dcc5005d1 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -310,7 +310,7 @@ 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) + if (association = association_instance_get(reflection.name)) && !association.target.nil? if reflection.options[:autosave] && association.marked_for_destruction? association.destroy elsif new_record? || association.new_record? || association[reflection.primary_key_name] != id || reflection.options[:autosave] -- cgit v1.2.3 From db26ace030f6704da6fc80bcc6cd00a2aee664ce Mon Sep 17 00:00:00 2001 From: Murray Steele Date: Thu, 12 Mar 2009 13:49:16 +0000 Subject: Ensure NoMethodError isn't raised when some of the nested eager loaded associations are empty [#1696 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/associations.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 301b3a3b58..6d25b36aea 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1856,9 +1856,10 @@ module ActiveRecord def construct(parent, associations, joins, row) case associations when Symbol, String - while (join = joins.shift).reflection.name.to_s != associations.to_s - raise ConfigurationError, "Not Enough Associations" if joins.empty? - end + join = joins.detect{|j| j.reflection.name.to_s == associations.to_s && j.parent_table_name == parent.class.table_name } + raise(ConfigurationError, "No such association") if join.nil? + + joins.delete(join) construct_association(parent, join, row) when Array associations.each do |association| @@ -1866,7 +1867,11 @@ module ActiveRecord end when Hash associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name| - association = construct_association(parent, joins.shift, row) + join = joins.detect{|j| j.reflection.name.to_s == name.to_s && j.parent_table_name == parent.class.table_name } + raise(ConfigurationError, "No such association") if join.nil? + + association = construct_association(parent, join, row) + joins.delete(join) construct(association, associations[name], joins, row) if association end else -- cgit v1.2.3 From 91b98cf0a5417ce4042a0b3cd1930d5a221b737f Mon Sep 17 00:00:00 2001 From: Elijah Miller Date: Thu, 12 Mar 2009 15:06:19 +0000 Subject: Returning nil from named scope lambda is equivalent to an empty hash [#1773 state:resolved] Signed-off-by: Pratik Naik --- activerecord/lib/active_record/named_scope.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 519941dd94..1f3ef300f2 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -117,6 +117,7 @@ module ActiveRecord delegate :scopes, :with_scope, :to => :proxy_scope def initialize(proxy_scope, options, &block) + options ||= {} [options[:extend]].flatten.each { |extension| extend extension } if options[:extend] extend Module.new(&block) if block_given? unless Scope === proxy_scope -- cgit v1.2.3 From 47bdf3bf40ec17e1f8ca1c0e3d7f697d0c4cd1bf Mon Sep 17 00:00:00 2001 From: Luca Guidi Date: Thu, 12 Mar 2009 15:24:37 +0000 Subject: Ensure AutosaveAssociation runs remove callbacks [#2146 state:resolved] Signed-off-by: Eloy Duran Signed-off-by: Pratik Naik --- .../associations/association_collection.rb | 51 ++++++++++++++-------- .../lib/active_record/autosave_association.rb | 2 +- 2 files changed, 33 insertions(+), 20 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index f024f99a34..ad375be184 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -186,7 +186,6 @@ module ActiveRecord end end - # Removes +records+ from this association calling +before_remove+ and # +after_remove+ callbacks. # @@ -195,22 +194,25 @@ module ActiveRecord # are actually removed from the database, that depends precisely on # +delete_records+. They are in any case removed from the collection. def delete(*records) - records = flatten_deeper(records) - records.each { |record| raise_on_type_mismatch(record) } - - transaction do - records.each { |record| callback(:before_remove, record) } - - old_records = records.reject {|r| r.new_record? } + remove_records(records) do |records, old_records| delete_records(old_records) if old_records.any? - - records.each do |record| - @target.delete(record) - callback(:after_remove, record) - end + records.each { |record| @target.delete(record) } end end + # Destroy +records+ and remove from this association calling +before_remove+ + # and +after_remove+ callbacks. + # + # Note this method will always remove records from database ignoring the + # +:dependent+ option. + def destroy(*records) + remove_records(records) do |records, old_records| + old_records.each { |record| record.destroy } + end + + load_target + end + # Removes all records from this association. Returns +self+ so method calls may be chained. def clear return self if length.zero? # forces load_target if it hasn't happened already @@ -223,15 +225,14 @@ module ActiveRecord self end - - def destroy_all - transaction do - each { |record| record.destroy } - end + # Destory all the records from this association + def destroy_all + load_target + destroy(@target) reset_target! end - + def create(attrs = {}) if attrs.is_a?(Array) attrs.collect { |attr| create(attr) } @@ -431,6 +432,18 @@ module ActiveRecord record end + def remove_records(*records) + records = flatten_deeper(records) + records.each { |record| raise_on_type_mismatch(record) } + + transaction do + records.each { |record| callback(:before_remove, record) } + old_records = records.reject { |r| r.new_record? } + yield(records, old_records) + records.each { |record| callback(:after_remove, record) } + end + end + def callback(method, record) callbacks_for(method).each do |callback| ActiveSupport::Callbacks::Callback.new(method, callback, record).call(@owner, record) diff --git a/activerecord/lib/active_record/autosave_association.rb b/activerecord/lib/active_record/autosave_association.rb index 6dcc5005d1..741aa2acbe 100644 --- a/activerecord/lib/active_record/autosave_association.rb +++ b/activerecord/lib/active_record/autosave_association.rb @@ -283,7 +283,7 @@ module ActiveRecord if records = associated_records_to_validate_or_save(association, @new_record_before_save, autosave) records.each do |record| if autosave && record.marked_for_destruction? - record.destroy + association.destroy(record) elsif @new_record_before_save || record.new_record? if autosave association.send(:insert_record, record, false, false) -- cgit v1.2.3