Permalink
Browse files

Merge remote branch 'eloy/2-3-stable' into 2-3-stable

  • Loading branch information...
2 parents 94de32b + 51e6124 commit c50609c9f04e482680cf4e14fb0f4aa079753868 @josevalim josevalim committed Jan 8, 2010
@@ -1783,7 +1783,7 @@ def references_eager_loaded_tables?(options)
end
def using_limitable_reflections?(reflections)
- reflections.collect(&:collection_association?).length.zero?
+ reflections.collect(&:collection?).length.zero?
end
def column_aliases(join_dependency)
@@ -1860,7 +1860,7 @@ def remove_duplicate_results!(base, records, associations)
case associations
when Symbol, String
reflection = base.reflections[associations]
- if reflection && reflection.collection_association?
+ if reflection && reflection.collection?
records.each { |record| record.send(reflection.name).target.uniq! }
end
when Array
@@ -1874,7 +1874,7 @@ def remove_duplicate_results!(base, records, associations)
parent_records = records.map do |record|
descendant = record.send(reflection.name)
next unless descendant
- descendant.target.uniq! if reflection.collection_association?
+ descendant.target.uniq! if reflection.collection?
descendant
end.flatten.compact
@@ -167,7 +167,7 @@ def #{type}(name, options = {})
def add_autosave_association_callbacks(reflection)
save_method = :"autosave_associated_records_for_#{reflection.name}"
validation_method = :"validate_associated_records_for_#{reflection.name}"
- collection = reflection.collection_association?
+ collection = reflection.collection?
unless method_defined?(save_method)
if collection
@@ -225,10 +225,10 @@ def marked_for_destruction?
def associated_records_to_validate_or_save(association, new_record, autosave)
if new_record
association
- elsif association.loaded?
- autosave ? association : association.select { |record| record.new_record? }
+ elsif autosave
+ association.target.select { |record| record.new_record? || record.changed? || record.marked_for_destruction? }
else
- autosave ? association.target : association.target.select { |record| record.new_record? }
+ association.target.select { |record| record.new_record? }
end
end
@@ -297,13 +297,15 @@ def save_collection_association(reflection)
association.destroy(record)
elsif autosave != false && (@new_record_before_save || record.new_record?)
if autosave
- association.send(:insert_record, record, false, false)
+ saved = association.send(:insert_record, record, false, false)
else
association.send(:insert_record, record)
end
elsif autosave
- record.save(false)
+ saved = record.save(false)
end
+
+ raise ActiveRecord::Rollback if saved == false
end
end
@@ -330,7 +332,9 @@ def save_has_one_association(reflection)
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)
association[reflection.primary_key_name] = key
- association.save(!autosave)
+ saved = association.save(!autosave)
+ raise ActiveRecord::Rollback if !saved && autosave
+ saved
end
end
end
@@ -351,7 +355,7 @@ def save_belongs_to_association(reflection)
if autosave && association.marked_for_destruction?
association.destroy
elsif autosave != false
- association.save(!autosave) if association.new_record? || autosave
+ saved = association.save(!autosave) if association.new_record? || autosave
if association.updated?
association_id = association.send(reflection.options[:primary_key] || :id)
@@ -361,6 +365,8 @@ def save_belongs_to_association(reflection)
self[reflection.options[:foreign_type]] = association.class.base_class.name.to_s
end
end
+
+ saved if autosave
end
end
end
@@ -234,7 +234,7 @@ def accepts_nested_attributes_for(*attr_names)
reflection.options[:autosave] = true
add_autosave_association_callbacks(reflection)
nested_attributes_options[association_name.to_sym] = options
- type = (reflection.collection_association? ? :collection : :one_to_one)
+ type = (reflection.collection? ? :collection : :one_to_one)
# def pirate_attributes=(attributes)
# assign_nested_attributes_for_one_to_one_association(:pirate, attributes)
@@ -257,11 +257,11 @@ def polymorphic_inverse_of(associated_class)
# Returns whether or not this association reflection is for a collection
# association. Returns +true+ if the +macro+ is one of +has_many+ or
# +has_and_belongs_to_many+, +false+ otherwise.
- def collection_association?
- if @collection_association.nil?
- @collection_association = [:has_many, :has_and_belongs_to_many].include?(macro)
+ def collection?
+ if @collection.nil?
+ @collection = [:has_many, :has_and_belongs_to_many].include?(macro)
end
- @collection_association
+ @collection
end
# Returns whether or not the association should be validated as part of
@@ -280,7 +280,7 @@ def validate?
private
def derive_class_name
class_name = name.to_s.camelize
- class_name = class_name.singularize if collection_association?
+ class_name = class_name.singularize if collection?
class_name
end
@@ -3,6 +3,8 @@
require 'models/company'
require 'models/customer'
require 'models/developer'
+require 'models/invoice'
+require 'models/line_item'
require 'models/order'
require 'models/parrot'
require 'models/person'
@@ -692,23 +694,18 @@ def save(*args)
define_method("test_should_rollback_destructions_if_an_exception_occurred_while_saving_#{association_name}") do
2.times { |i| @pirate.send(association_name).create!(:name => "#{association_name}_#{i}") }
- before = @pirate.send(association_name).map { |c| c }
+ before = @pirate.send(association_name).map { |c| c.mark_for_destruction ; c }
- # Stub the save method of the first child to destroy and the second to raise an exception
- class << before.first
- def save(*args)
- super
- destroy
- end
- end
+ # Stub the destroy method of the the second child to raise an exception
class << before.last
- def save(*args)
+ def destroy(*args)
super
raise 'Oh noes!'
end
end
assert_raise(RuntimeError) { assert !@pirate.save }
+ assert before.first.frozen? # the first child was indeed destroyed
assert_equal before, @pirate.reload.send(association_name)
end
@@ -816,6 +813,18 @@ def test_should_still_raise_an_ActiveRecordRecord_Invalid_exception_if_we_want_t
end
end
+ def test_should_not_save_and_return_false_if_a_callback_cancelled_saving
+ pirate = Pirate.new(:catchphrase => 'Arr')
+ ship = pirate.build_ship(:name => 'The Vile Insanity')
+ ship.cancel_save_from_callback = true
+
+ assert_no_difference 'Pirate.count' do
+ assert_no_difference 'Ship.count' do
+ assert !pirate.save
+ end
+ end
+ end
+
def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
before = [@pirate.catchphrase, @pirate.ship.name]
@@ -894,6 +903,18 @@ def test_should_still_raise_an_ActiveRecordRecord_Invalid_exception_if_we_want_t
end
end
+ def test_should_not_save_and_return_false_if_a_callback_cancelled_saving
+ ship = Ship.new(:name => 'The Vile Insanity')
+ pirate = ship.build_pirate(:catchphrase => 'Arr')
+ pirate.cancel_save_from_callback = true
+
+ assert_no_difference 'Ship.count' do
+ assert_no_difference 'Pirate.count' do
+ assert !ship.save
+ end
+ end
+ end
+
def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
before = [@ship.pirate.catchphrase, @ship.name]
@@ -909,7 +930,6 @@ def save(*args)
end
assert_raise(RuntimeError) { assert !@ship.save }
- # TODO: Why does using reload on @ship looses the associated pirate?
assert_equal before, [@ship.pirate.reload.catchphrase, @ship.reload.name]
end
@@ -986,6 +1006,26 @@ def test_should_allow_to_bypass_validations_on_the_associated_models_on_create
end
end
+ def test_should_not_save_and_return_false_if_a_callback_cancelled_saving_in_either_create_or_update
+ @pirate.catchphrase = 'Changed'
+ @child_1.name = 'Changed'
+ @child_1.cancel_save_from_callback = true
+
+ assert !@pirate.save
+ assert_equal "Don' botharrr talkin' like one, savvy?", @pirate.reload.catchphrase
+ assert_equal "Posideons Killer", @child_1.reload.name
+
+ new_pirate = Pirate.new(:catchphrase => 'Arr')
+ new_child = new_pirate.send(@association_name).build(:name => 'Grace OMalley')
+ new_child.cancel_save_from_callback = true
+
+ assert_no_difference 'Pirate.count' do
+ assert_no_difference "#{new_child.class.name}.count" do
+ assert !new_pirate.save
+ end
+ end
+ end
+
def test_should_rollback_any_changes_if_an_exception_occurred_while_saving
before = [@pirate.catchphrase, *@pirate.send(@association_name).map(&:name)]
new_names = ['Grace OMalley', 'Privateers Greed']
@@ -1169,3 +1209,10 @@ def setup
assert !@pirate.respond_to?(:validate_associated_records_for_non_validated_parrots)
end
end
+
+class TestAutosaveAssociationWithTouch < ActiveRecord::TestCase
+ def test_autosave_with_touch_should_not_raise_system_stack_error
+ invoice = Invoice.create
+ assert_nothing_raised { invoice.line_items.create(:amount => 10) }
+ end
+end
@@ -188,11 +188,11 @@ def test_has_many_through_reflection
end
def test_collection_association
- assert Pirate.reflect_on_association(:birds).collection_association?
- assert Pirate.reflect_on_association(:parrots).collection_association?
+ assert Pirate.reflect_on_association(:birds).collection?
+ assert Pirate.reflect_on_association(:parrots).collection?
- assert !Pirate.reflect_on_association(:ship).collection_association?
- assert !Ship.reflect_on_association(:pirate).collection_association?
+ assert !Pirate.reflect_on_association(:ship).collection?
+ assert !Ship.reflect_on_association(:pirate).collection?
end
def test_default_association_validation
@@ -1,3 +1,9 @@
class Bird < ActiveRecord::Base
validates_presence_of :name
+
+ attr_accessor :cancel_save_from_callback
+ before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+ def cancel_save_callback_method
+ false
+ end
end
@@ -0,0 +1,4 @@
+class Invoice < ActiveRecord::Base
+ has_many :line_items, :autosave => true
+ before_save {|record| record.balance = record.line_items.map(&:amount).sum }
+end
@@ -0,0 +1,3 @@
+class LineItem < ActiveRecord::Base
+ belongs_to :invoice, :touch => true
+end
@@ -6,6 +6,12 @@ class Parrot < ActiveRecord::Base
alias_attribute :title, :name
validates_presence_of :name
+
+ attr_accessor :cancel_save_from_callback
+ before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+ def cancel_save_callback_method
+ false
+ end
end
class LiveParrot < Parrot
@@ -51,6 +51,12 @@ def reject_empty_ships_on_create(attributes)
attributes.delete('_reject_me_if_new').present? && new_record?
end
+ attr_accessor :cancel_save_from_callback
+ before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+ def cancel_save_callback_method
+ false
+ end
+
private
def log_before_add(record)
log(record, "before_adding_method")
@@ -9,4 +9,10 @@ class Ship < ActiveRecord::Base
accepts_nested_attributes_for :update_only_pirate, :update_only => true
validates_presence_of :name
+
+ attr_accessor :cancel_save_from_callback
+ before_save :cancel_save_callback_method, :if => :cancel_save_from_callback
+ def cancel_save_callback_method
+ false
+ end
end
@@ -192,6 +192,11 @@ def create_table(*args, &block)
t.string :info
end
+ create_table :invoices, :force => true do |t|
+ t.integer :balance
+ t.datetime :updated_at
+ end
+
create_table :items, :force => true do |t|
t.column :name, :integer
end
@@ -217,6 +222,11 @@ def create_table(*args, &block)
t.integer :version, :null => false, :default => 0
end
+ create_table :line_items, :force => true do |t|
+ t.integer :invoice_id
+ t.integer :amount
+ end
+
create_table :lock_without_defaults, :force => true do |t|
t.column :lock_version, :integer
end

0 comments on commit c50609c

Please sign in to comment.