Skip to content

Commit

Permalink
use persisted? instead of new_record? wherever possible
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
dchelimsky committed Nov 9, 2010
1 parent 4e0477c commit 787407c
Show file tree
Hide file tree
Showing 35 changed files with 203 additions and 196 deletions.
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/aggregations.rb
Expand Up @@ -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+
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/associations.rb
Expand Up @@ -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
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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?
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
Expand Up @@ -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

Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -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
Expand Down
Expand Up @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Expand Up @@ -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))
Expand Down
Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions activerecord/lib/active_record/autosave_association.rb
Expand Up @@ -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
Expand All @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
18 changes: 10 additions & 8 deletions activerecord/lib/active_record/base.rb
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)}"
Expand Down Expand Up @@ -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 ==
Expand Down Expand Up @@ -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(", ")
Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/locking/optimistic.rb
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion activerecord/lib/active_record/locking/pessimistic.rb
Expand Up @@ -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
Expand Down

0 comments on commit 787407c

Please sign in to comment.