From 1f06652a57e727700c3a673dc1f86e3b1e07ce1f Mon Sep 17 00:00:00 2001 From: David Chelimsky Date: Sun, 7 Nov 2010 08:05:18 -0600 Subject: use persisted? instead of new_record? wherever possible - persisted? is the API defined in ActiveModel - makes it easier for extension libraries to conform to ActiveModel APIs without concern for whether the extended object is specifically ActiveRecord [#5927 state:committed] Signed-off-by: Santiago Pastorino --- .../associations/association_collection.rb | 21 +++++++++++---------- .../active_record/associations/association_proxy.rb | 6 +++--- .../associations/belongs_to_association.rb | 6 +++--- .../has_and_belongs_to_many_association.rb | 2 +- .../associations/has_many_through_association.rb | 2 +- .../associations/has_one_association.rb | 12 ++++++------ .../associations/has_one_through_association.rb | 2 +- 7 files changed, 26 insertions(+), 25 deletions(-) (limited to 'activerecord/lib/active_record/associations') diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 896e18af01..398c41f3e9 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -120,13 +120,13 @@ module ActiveRecord # Since << flattens its argument list and inserts each record, +push+ and +concat+ behave identically. def <<(*records) result = true - load_target if @owner.new_record? + load_target unless @owner.persisted? transaction do flatten_deeper(records).each do |record| raise_on_type_mismatch(record) add_record_to_target_with_callbacks(record) do |r| - result &&= insert_record(record) unless @owner.new_record? + result &&= insert_record(record) if @owner.persisted? end end end @@ -285,12 +285,12 @@ module ActiveRecord # This method is abstract in the sense that it relies on # +count_records+, which is a method descendants have to provide. def size - if @owner.new_record? || (loaded? && !@reflection.options[:uniq]) + if !@owner.persisted? || (loaded? && !@reflection.options[:uniq]) @target.size elsif !loaded? && @reflection.options[:group] load_target.size elsif !loaded? && !@reflection.options[:uniq] && @target.is_a?(Array) - unsaved_records = @target.select { |r| r.new_record? } + unsaved_records = @target.reject { |r| r.persisted? } unsaved_records.size + count_records else count_records @@ -357,7 +357,7 @@ module ActiveRecord def include?(record) return false unless record.is_a?(@reflection.klass) - return include_in_memory?(record) if record.new_record? + return include_in_memory?(record) unless record.persisted? load_target if @reflection.options[:finder_sql] && !loaded? return @target.include?(record) if loaded? exists?(record) @@ -372,7 +372,7 @@ module ActiveRecord end def load_target - if !@owner.new_record? || foreign_key_present + if @owner.persisted? || foreign_key_present begin if !loaded? if @target.is_a?(Array) && @target.any? @@ -513,7 +513,7 @@ module ActiveRecord transaction do records.each { |record| callback(:before_remove, record) } - old_records = records.reject { |r| r.new_record? } + old_records = records.select { |r| r.persisted? } yield(records, old_records) records.each { |record| callback(:after_remove, record) } end @@ -538,14 +538,15 @@ module ActiveRecord end def ensure_owner_is_not_new - if @owner.new_record? + unless @owner.persisted? raise ActiveRecord::RecordNotSaved, "You cannot call create unless the parent is saved" end end def fetch_first_or_last_using_find?(args) - args.first.kind_of?(Hash) || !(loaded? || @owner.new_record? || @reflection.options[:finder_sql] || - @target.any? { |record| record.new_record? } || args.first.kind_of?(Integer)) + args.first.kind_of?(Hash) || !(loaded? || !@owner.persisted? || @reflection.options[:finder_sql] || + @target.any? { |record| !record.persisted? } || args.first.kind_of?(Integer)) + # TODO - would prefer @target.none? { |r| r.persisted? } end def include_in_memory?(record) diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index ac2aa46edf..09bba213ce 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -175,10 +175,10 @@ module ActiveRecord # If the association is polymorphic the type of the owner is also set. def set_belongs_to_association_for(record) if @reflection.options[:as] - record["#{@reflection.options[:as]}_id"] = @owner.id unless @owner.new_record? + record["#{@reflection.options[:as]}_id"] = @owner.id if @owner.persisted? record["#{@reflection.options[:as]}_type"] = @owner.class.base_class.name.to_s else - unless @owner.new_record? + if @owner.persisted? primary_key = @reflection.options[:primary_key] || :id record[@reflection.primary_key_name] = @owner.send(primary_key) end @@ -252,7 +252,7 @@ module ActiveRecord def load_target return nil unless defined?(@loaded) - if !loaded? and (!@owner.new_record? || foreign_key_present) + if !loaded? and (@owner.persisted? || foreign_key_present) @target = find_target end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 34b6cd5576..b624951cd9 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -14,7 +14,7 @@ module ActiveRecord counter_cache_name = @reflection.counter_cache_column if record.nil? - if counter_cache_name && !@owner.new_record? + if counter_cache_name && @owner.persisted? @reflection.klass.decrement_counter(counter_cache_name, previous_record_id) if @owner[@reflection.primary_key_name] end @@ -22,13 +22,13 @@ module ActiveRecord else raise_on_type_mismatch(record) - if counter_cache_name && !@owner.new_record? && record.id != @owner[@reflection.primary_key_name] + if counter_cache_name && @owner.persisted? && record.id != @owner[@reflection.primary_key_name] @reflection.klass.increment_counter(counter_cache_name, record.id) @reflection.klass.decrement_counter(counter_cache_name, @owner[@reflection.primary_key_name]) if @owner[@reflection.primary_key_name] end @target = (AssociationProxy === record ? record.target : record) - @owner[@reflection.primary_key_name] = record_id(record) unless record.new_record? + @owner[@reflection.primary_key_name] = record_id(record) if record.persisted? @updated = true 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 1fc9aba5cf..da742fa668 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 @@ -34,7 +34,7 @@ module ActiveRecord end def insert_record(record, force = true, validate = true) - if record.new_record? + unless record.persisted? if force record.save! else 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 437c8b1fd6..51770cff0f 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -59,7 +59,7 @@ module ActiveRecord end def insert_record(record, force = true, validate = true) - if record.new_record? + unless record.persisted? if force record.save! else diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 17901387e9..e6e037441f 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -30,18 +30,18 @@ module ActiveRecord if dependent? && !dont_save case @reflection.options[:dependent] when :delete - @target.delete unless @target.new_record? + @target.delete if @target.persisted? @owner.clear_association_cache when :destroy - @target.destroy unless @target.new_record? + @target.destroy if @target.persisted? @owner.clear_association_cache when :nullify @target[@reflection.primary_key_name] = nil - @target.save unless @owner.new_record? || @target.new_record? + @target.save if @owner.persisted? && @target.persisted? end else @target[@reflection.primary_key_name] = nil - @target.save unless @owner.new_record? || @target.new_record? + @target.save if @owner.persisted? && @target.persisted? end end @@ -56,7 +56,7 @@ module ActiveRecord set_inverse_instance(obj, @owner) @loaded = true - unless @owner.new_record? or obj.nil? or dont_save + unless !@owner.persisted? or obj.nil? or dont_save return (obj.save ? self : false) else return (obj.nil? ? nil : self) @@ -113,7 +113,7 @@ module ActiveRecord if replace_existing replace(record, true) else - record[@reflection.primary_key_name] = @owner.id unless @owner.new_record? + record[@reflection.primary_key_name] = @owner.id if @owner.persisted? self.target = record set_inverse_instance(record, @owner) end 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 7f28abf464..6e98f7dffb 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -21,7 +21,7 @@ module ActiveRecord if current_object new_value ? current_object.update_attributes(construct_join_attributes(new_value)) : current_object.destroy elsif new_value - if @owner.new_record? + unless @owner.persisted? self.target = new_value through_association = @owner.send(:association_instance_get, @reflection.through_reflection.name) through_association.build(construct_join_attributes(new_value)) -- cgit v1.2.3