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 <santiago@wyeworks.com>

activerecord/lib/active_record/aggregations.rb

@@ -6,7 +6,7 @@ module Aggregations # :nodoc:
     def clear_aggregation_cache #:nodoc:
       self.class.reflect_on_all_aggregations.to_a.each do |assoc|
         instance_variable_set "@#{assoc.name}", nil
-      end unless self.new_record?
+      end if self.persisted?
     end
 
     # Active Record implements aggregation through a macro-like class method called +composed_of+

activerecord/lib/active_record/associations.rb

@@ -118,7 +118,7 @@ module Associations # :nodoc:
     def clear_association_cache #:nodoc:
       self.class.reflect_on_all_associations.to_a.each do |assoc|
         instance_variable_set "@#{assoc.name}", nil
-      end unless self.new_record?
+      end if self.persisted?
     end
 
     private

activerecord/lib/active_record/associations/association_collection.rb

@@ -120,13 +120,13 @@ def build(attributes = {}, &block)
       # 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 @@ def create!(attrs = {})
       # 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 @@ def replace(other_array)
 
       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 @@ def construct_find_options!(options)
         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 @@ def remove_records(*records)
 
           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 @@ def callbacks_for(callback_name)
         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)

activerecord/lib/active_record/associations/association_proxy.rb

@@ -175,10 +175,10 @@ def sanitize_sql(sql, table_name = @reflection.klass.table_name)
         # 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 @@ def method_missing(method, *args)
         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
 

activerecord/lib/active_record/associations/belongs_to_association.rb

@@ -14,21 +14,21 @@ def replace(record)
         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
 
           @target = @owner[@reflection.primary_key_name] = nil
         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
 

activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb

@@ -34,7 +34,7 @@ def count_records
         end
 
         def insert_record(record, force = true, validate = true)
-          if record.new_record?
+          unless record.persisted?
             if force
               record.save!
             else

activerecord/lib/active_record/associations/has_many_through_association.rb

@@ -59,7 +59,7 @@ def construct_find_options!(options)
         end
 
         def insert_record(record, force = true, validate = true)
-          if record.new_record?
+          unless record.persisted?
             if force
               record.save!
             else

activerecord/lib/active_record/associations/has_one_association.rb

@@ -30,18 +30,18 @@ def replace(obj, dont_save = false)
           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 @@ def replace(obj, dont_save = false)
         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 @@ def new_record(replace_existing)
           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

activerecord/lib/active_record/associations/has_one_through_association.rb

@@ -21,7 +21,7 @@ def create_through_record(new_value) #nodoc:
         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))

activerecord/lib/active_record/attribute_methods/primary_key.rb

@@ -3,10 +3,11 @@ module AttributeMethods
     module PrimaryKey
       extend ActiveSupport::Concern
 
-      # Returns this record's primary key value wrapped in an Array
-      # or nil if the record is a new_record?
+      # Returns this record's primary key value wrapped in an Array or nil if
+      # the record is not persisted? or has just been destroyed.
       def to_key
-        new_record? ? nil : [ id ]
+        key = send(self.class.primary_key)
+        [key] if key
       end
 
       module ClassMethods

activerecord/lib/active_record/autosave_association.rb

@@ -217,7 +217,7 @@ def marked_for_destruction?
     # Returns whether or not this record has been changed in any way (including whether
     # any of its nested autosave associations are likewise changed)
     def changed_for_autosave?
-      new_record? || changed? || marked_for_destruction? || nested_records_changed_for_autosave?
+      !persisted? || changed? || marked_for_destruction? || nested_records_changed_for_autosave?
     end
 
     private
@@ -231,7 +231,7 @@ def associated_records_to_validate_or_save(association, new_record, autosave)
       elsif autosave
         association.target.find_all { |record| record.changed_for_autosave? }
       else
-        association.target.find_all { |record| record.new_record? }
+        association.target.find_all { |record| !record.persisted? }
       end
     end
 
@@ -257,7 +257,7 @@ def validate_single_association(reflection)
     # +reflection+.
     def validate_collection_association(reflection)
       if association = association_instance_get(reflection.name)
-        if records = associated_records_to_validate_or_save(association, new_record?, reflection.options[:autosave])
+        if records = associated_records_to_validate_or_save(association, !persisted?, reflection.options[:autosave])
           records.each { |record| association_valid?(reflection, record) }
         end
       end
@@ -286,7 +286,7 @@ def association_valid?(reflection, association)
     # Is used as a before_save callback to check while saving a collection
     # association whether or not the parent was a new record before saving.
     def before_save_collection_association
-      @new_record_before_save = new_record?
+      @new_record_before_save = !persisted?
       true
     end
 
@@ -308,7 +308,7 @@ def save_collection_association(reflection)
 
             if autosave && record.marked_for_destruction?
               association.destroy(record)
-            elsif autosave != false && (@new_record_before_save || record.new_record?)
+            elsif autosave != false && (@new_record_before_save || !record.persisted?)
               if autosave
                 saved = association.send(:insert_record, record, false, false)
               else
@@ -343,7 +343,7 @@ def save_has_one_association(reflection)
           association.destroy
         else
           key = reflection.options[:primary_key] ? send(reflection.options[:primary_key]) : id
-          if autosave != false && (new_record? || association.new_record? || association[reflection.primary_key_name] != key || autosave)
+          if autosave != false && (!persisted? || !association.persisted? || association[reflection.primary_key_name] != key || autosave)
             association[reflection.primary_key_name] = key
             saved = association.save(:validate => !autosave)
             raise ActiveRecord::Rollback if !saved && autosave
@@ -363,7 +363,7 @@ def save_belongs_to_association(reflection)
         if autosave && association.marked_for_destruction?
           association.destroy
         elsif autosave != false
-          saved = association.save(:validate => !autosave) if association.new_record? || autosave
+          saved = association.save(:validate => !autosave) if !association.persisted? || autosave
 
           if association.updated?
             association_id = association.send(reflection.options[:primary_key] || :id)

activerecord/lib/active_record/base.rb

@@ -204,7 +204,7 @@ module ActiveRecord #:nodoc:
   #
   #   # No 'Winter' tag exists
   #   winter = Tag.find_or_initialize_by_name("Winter")
-  #   winter.new_record? # true
+  #   winter.persisted? # false
   #
   # To find by a subset of the attributes to be used for instantiating a new object, pass a hash instead of
   # a list of parameters.
@@ -1368,7 +1368,7 @@ def encode_quoted_value(value) #:nodoc:
       def initialize(attributes = nil)
         @attributes = attributes_from_column_definition
         @attributes_cache = {}
-        @new_record = true
+        @persited = false
         @readonly = false
         @destroyed = false
         @marked_for_destruction = false
@@ -1403,7 +1403,7 @@ def initialize_copy(other)
         clear_aggregation_cache
         clear_association_cache
         @attributes_cache = {}
-        @new_record = true
+        @persisted = false
         ensure_proper_type
 
         populate_with_current_scope_attributes
@@ -1422,7 +1422,8 @@ def initialize_copy(other)
       def init_with(coder)
         @attributes = coder['attributes']
         @attributes_cache, @previously_changed, @changed_attributes = {}, {}, {}
-        @new_record = @readonly = @destroyed = @marked_for_destruction = false
+        @readonly = @destroyed = @marked_for_destruction = false
+        @persisted = true
         _run_find_callbacks
         _run_initialize_callbacks
       end
@@ -1463,7 +1464,7 @@ def to_param
       #   Person.find(5).cache_key  # => "people/5-20071224150000" (updated_at available)
       def cache_key
         case
-        when new_record?
+        when !persisted?
           "#{self.class.model_name.cache_key}/new"
         when timestamp = self[:updated_at]
           "#{self.class.model_name.cache_key}/#{id}-#{timestamp.to_s(:number)}"
@@ -1584,8 +1585,9 @@ def column_for_attribute(name)
       # Returns true if the +comparison_object+ is the same object, or is of the same type and has the same id.
       def ==(comparison_object)
         comparison_object.equal?(self) ||
-          (comparison_object.instance_of?(self.class) &&
-            comparison_object.id == id && !comparison_object.new_record?)
+          persisted? &&
+            (comparison_object.instance_of?(self.class) &&
+              comparison_object.id == id)
       end
 
       # Delegates to ==
@@ -1630,7 +1632,7 @@ def readonly!
       # Returns the contents of the record as a nicely formatted string.
       def inspect
         attributes_as_nice_string = self.class.column_names.collect { |name|
-          if has_attribute?(name) || new_record?
+          if has_attribute?(name) || !persisted?
             "#{name}: #{attribute_for_inspect(name)}"
           end
         }.compact.join(", ")

activerecord/lib/active_record/locking/optimistic.rb

@@ -109,7 +109,7 @@ def update(attribute_names = @attributes.keys) #:nodoc:
         def destroy #:nodoc:
           return super unless locking_enabled?
 
-          unless new_record?
+          if persisted?
             lock_col = self.class.locking_column
             previous_value = send(lock_col).to_i
 

activerecord/lib/active_record/locking/pessimistic.rb

@@ -47,7 +47,7 @@ module Pessimistic
       # or pass true for "FOR UPDATE" (the default, an exclusive row lock).  Returns
       # the locked record.
       def lock!(lock = true)
-        reload(:lock => lock) unless new_record?
+        reload(:lock => lock) if persisted?
         self
       end
     end