aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG4
-rw-r--r--activerecord/MIT-LICENSE2
-rw-r--r--activerecord/lib/active_record.rb2
-rwxr-xr-xactiverecord/lib/active_record/associations.rb4
-rw-r--r--activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb12
-rw-r--r--activerecord/lib/active_record/callbacks.rb5
-rw-r--r--activerecord/lib/active_record/dirty.rb4
-rw-r--r--activerecord/lib/active_record/reflection.rb8
-rw-r--r--activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb11
-rw-r--r--activerecord/test/cases/associations/has_many_associations_test.rb3
-rw-r--r--activerecord/test/cases/callbacks_test.rb44
-rw-r--r--activerecord/test/cases/dirty_test.rb26
-rw-r--r--activerecord/test/cases/helper.rb2
-rw-r--r--activerecord/test/cases/json_serialization_test.rb2
-rw-r--r--activerecord/test/schema/schema.rb1
15 files changed, 112 insertions, 18 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index fbc71cddf1..507e37ac3b 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,9 @@
*2.3.0/3.0*
+* Make after_save callbacks fire only if the record was successfully saved. #1735 [Michael Lovitt]
+
+ Previously the callbacks would fire if a before_save cancelled saving.
+
* Support nested transactions using database savepoints. #383 [Jonathan Viney, Hongli Lai]
* Added dynamic scopes ala dynamic finders #1648 [Yaroslav Markin]
diff --git a/activerecord/MIT-LICENSE b/activerecord/MIT-LICENSE
index 93be57f683..e6df48772f 100644
--- a/activerecord/MIT-LICENSE
+++ b/activerecord/MIT-LICENSE
@@ -1,4 +1,4 @@
-Copyright (c) 2004-2008 David Heinemeier Hansson
+Copyright (c) 2004-2009 David Heinemeier Hansson
Permission is hereby granted, free of charge, to any person obtaining
a copy of this software and associated documentation files (the
diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb
index 390c005785..e1265b7e1e 100644
--- a/activerecord/lib/active_record.rb
+++ b/activerecord/lib/active_record.rb
@@ -1,5 +1,5 @@
#--
-# Copyright (c) 2004-2008 David Heinemeier Hansson
+# Copyright (c) 2004-2009 David Heinemeier Hansson
#
# Permission is hereby granted, free of charge, to any person obtaining
# a copy of this software and associated documentation files (the
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 86616abf52..8b51a38f48 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1531,14 +1531,14 @@ module ActiveRecord
association = send(reflection.name)
association.destroy unless association.nil?
end
- before_destroy method_name
+ after_destroy method_name
when :delete
method_name = "belongs_to_dependent_delete_for_#{reflection.name}".to_sym
define_method(method_name) do
association = send(reflection.name)
association.delete unless association.nil?
end
- before_destroy method_name
+ after_destroy method_name
else
raise ArgumentError, "The :dependent option expects either :destroy or :delete (#{reflection.options[:dependent].inspect})"
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 3d689098b5..a5cc3bf091 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
@@ -9,6 +9,14 @@ module ActiveRecord
create_record(attributes) { |record| insert_record(record, true) }
end
+ def columns
+ @reflection.columns(@reflection.options[:join_table], "#{@reflection.options[:join_table]} Columns")
+ end
+
+ def reset_column_information
+ @reflection.reset_column_information
+ end
+
protected
def construct_find_options!(options)
options[:joins] = @join_sql
@@ -32,8 +40,6 @@ module ActiveRecord
if @reflection.options[:insert_sql]
@owner.connection.insert(interpolate_sql(@reflection.options[:insert_sql], record))
else
- columns = @owner.connection.columns(@reflection.options[:join_table], "#{@reflection.options[:join_table]} Columns")
-
attributes = columns.inject({}) do |attrs, column|
case column.name.to_s
when @reflection.primary_key_name.to_s
@@ -103,7 +109,7 @@ module ActiveRecord
# clause has been explicitly defined. Otherwise you can get broken records back, if, for example, the join column also has
# an id column. This will then overwrite the id column of the records coming back.
def finding_with_ambiguous_select?(select_clause)
- !select_clause && @owner.connection.columns(@reflection.options[:join_table], "Join Table Columns").size != 2
+ !select_clause && columns.size != 2
end
private
diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index dc33f8c2c7..88958f4583 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -219,8 +219,9 @@ module ActiveRecord
def after_save() end
def create_or_update_with_callbacks #:nodoc:
return false if callback(:before_save) == false
- result = create_or_update_without_callbacks
- callback(:after_save)
+ if result = create_or_update_without_callbacks
+ callback(:after_save)
+ end
result
end
private :create_or_update_with_callbacks
diff --git a/activerecord/lib/active_record/dirty.rb b/activerecord/lib/active_record/dirty.rb
index 4c899f58e5..4a2510aa63 100644
--- a/activerecord/lib/active_record/dirty.rb
+++ b/activerecord/lib/active_record/dirty.rb
@@ -151,8 +151,8 @@ module ActiveRecord
def field_changed?(attr, old, value)
if column = column_for_attribute(attr)
- if column.type == :integer && column.null && (old.nil? || old == 0) && value.blank?
- # For nullable integer columns, NULL gets stored in database for blank (i.e. '') values.
+ if column.number? && column.null && (old.nil? || old == 0) && value.blank?
+ # For nullable numeric columns, NULL gets stored in database for blank (i.e. '') values.
# Hence we don't record it as a change if the value changes from nil to ''.
# If an old value of 0 is set to '' we want this to get changed to nil as otherwise it'll
# be typecast back to 0 (''.to_i => 0)
diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb
index dbff4f24d6..1937abdc83 100644
--- a/activerecord/lib/active_record/reflection.rb
+++ b/activerecord/lib/active_record/reflection.rb
@@ -198,6 +198,14 @@ module ActiveRecord
end
end
+ def columns(tbl_name, log_msg)
+ @columns ||= klass.connection.columns(tbl_name, log_msg)
+ end
+
+ def reset_column_information
+ @columns = nil
+ end
+
def check_validity!
end
diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
index 2f08e09d43..1e3b423471 100644
--- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb
@@ -775,4 +775,15 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase
end
end
end
+
+ def test_caching_of_columns
+ david = Developer.find(1)
+ # clear cache possibly created by other tests
+ david.projects.reset_column_information
+ assert_queries(0) { david.projects.columns; david.projects.columns }
+ # and again to verify that reset_column_information clears the cache correctly
+ david.projects.reset_column_information
+ assert_queries(0) { david.projects.columns; david.projects.columns }
+ end
+
end
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index a5ae5cd8ec..428fb50013 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -698,7 +698,8 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
authors(:david).destroy
end
- assert_equal [author_address.id], AuthorAddress.destroyed_author_address_ids[authors(:david).id]
+ assert_equal nil, AuthorAddress.find_by_id(authors(:david).author_address_id)
+ assert_equal nil, AuthorAddress.find_by_id(authors(:david).author_address_extra_id)
end
def test_invalid_belongs_to_dependent_option_raises_exception
diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb
index 11830a2ef6..33b1ea034d 100644
--- a/activerecord/test/cases/callbacks_test.rb
+++ b/activerecord/test/cases/callbacks_test.rb
@@ -121,9 +121,19 @@ end
class CallbackCancellationDeveloper < ActiveRecord::Base
set_table_name 'developers'
- def before_create
- false
- end
+
+ attr_reader :after_save_called, :after_create_called, :after_update_called, :after_destroy_called
+ attr_accessor :cancel_before_save, :cancel_before_create, :cancel_before_update, :cancel_before_destroy
+
+ def before_save; !@cancel_before_save; end
+ def before_create; !@cancel_before_create; end
+ def before_update; !@cancel_before_update; end
+ def before_destroy; !@cancel_before_destroy; end
+
+ def after_save; @after_save_called = true; end
+ def after_update; @after_update_called = true; end
+ def after_create; @after_create_called = true; end
+ def after_destroy; @after_destroy_called = true; end
end
class CallbacksTest < ActiveRecord::TestCase
@@ -349,19 +359,47 @@ class CallbacksTest < ActiveRecord::TestCase
assert !david.valid?
assert !david.save
assert_raises(ActiveRecord::RecordInvalid) { david.save! }
+
+ someone = CallbackCancellationDeveloper.find(1)
+ someone.cancel_before_save = true
+ assert someone.valid?
+ assert !someone.save
+ assert_save_callbacks_not_called(someone)
end
def test_before_create_returning_false
someone = CallbackCancellationDeveloper.new
+ someone.cancel_before_create = true
assert someone.valid?
assert !someone.save
+ assert_save_callbacks_not_called(someone)
+ end
+
+ def test_before_update_returning_false
+ someone = CallbackCancellationDeveloper.find(1)
+ someone.cancel_before_update = true
+ assert someone.valid?
+ assert !someone.save
+ assert_save_callbacks_not_called(someone)
end
def test_before_destroy_returning_false
david = ImmutableDeveloper.find(1)
assert !david.destroy
assert_not_nil ImmutableDeveloper.find_by_id(1)
+
+ someone = CallbackCancellationDeveloper.find(1)
+ someone.cancel_before_destroy = true
+ assert !someone.destroy
+ assert !someone.after_destroy_called
+ end
+
+ def assert_save_callbacks_not_called(someone)
+ assert !someone.after_save_called
+ assert !someone.after_create_called
+ assert !someone.after_update_called
end
+ private :assert_save_callbacks_not_called
def test_zzz_callback_returning_false # must be run last since we modify CallbackDeveloper
david = CallbackDeveloper.find(1)
diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb
index 10cdbdc622..1c9e281cc0 100644
--- a/activerecord/test/cases/dirty_test.rb
+++ b/activerecord/test/cases/dirty_test.rb
@@ -21,6 +21,10 @@ private
end
end
+class NumericData < ActiveRecord::Base
+ self.table_name = 'numeric_data'
+end
+
class DirtyTest < ActiveRecord::TestCase
def test_attribute_changes
# New record - no changes.
@@ -58,7 +62,7 @@ class DirtyTest < ActiveRecord::TestCase
assert_equal parrot.name_change, parrot.title_change
end
- def test_nullable_integer_not_marked_as_changed_if_new_value_is_blank
+ def test_nullable_number_not_marked_as_changed_if_new_value_is_blank
pirate = Pirate.new
["", nil].each do |value|
@@ -68,6 +72,26 @@ class DirtyTest < ActiveRecord::TestCase
end
end
+ def test_nullable_decimal_not_marked_as_changed_if_new_value_is_blank
+ numeric_data = NumericData.new
+
+ ["", nil].each do |value|
+ numeric_data.bank_balance = value
+ assert !numeric_data.bank_balance_changed?
+ assert_nil numeric_data.bank_balance_change
+ end
+ end
+
+ def test_nullable_float_not_marked_as_changed_if_new_value_is_blank
+ numeric_data = NumericData.new
+
+ ["", nil].each do |value|
+ numeric_data.temperature = value
+ assert !numeric_data.temperature_changed?
+ assert_nil numeric_data.temperature_change
+ end
+ end
+
def test_nullable_integer_zero_to_string_zero_not_marked_as_changed
pirate = Pirate.new
pirate.parrot_id = 0
diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb
index ccd04b67f6..24ce35e2e2 100644
--- a/activerecord/test/cases/helper.rb
+++ b/activerecord/test/cases/helper.rb
@@ -34,7 +34,7 @@ rescue LoadError
end
ActiveRecord::Base.connection.class.class_eval do
- IGNORED_SQL = [/^PRAGMA/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/]
+ IGNORED_SQL = [/^PRAGMA/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/, /SHOW FIELDS/]
def execute_with_query_record(sql, name = nil, &block)
$queries_executed ||= []
diff --git a/activerecord/test/cases/json_serialization_test.rb b/activerecord/test/cases/json_serialization_test.rb
index 3446e5e7f0..975acde096 100644
--- a/activerecord/test/cases/json_serialization_test.rb
+++ b/activerecord/test/cases/json_serialization_test.rb
@@ -200,6 +200,6 @@ class DatabaseConnectedJsonEncodingTest < ActiveRecord::TestCase
2 => @mary
}
- assert_equal %({1: {"name": "David"}}), authors_hash.to_json(:only => [1, :name])
+ assert_equal %({"1": {"name": "David"}}), authors_hash.to_json(:only => [1, :name])
end
end
diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb
index 8199cb8fc7..094932d375 100644
--- a/activerecord/test/schema/schema.rb
+++ b/activerecord/test/schema/schema.rb
@@ -252,6 +252,7 @@ ActiveRecord::Schema.define do
t.decimal :world_population, :precision => 10, :scale => 0
t.decimal :my_house_population, :precision => 2, :scale => 0
t.decimal :decimal_number_with_default, :precision => 3, :scale => 2, :default => 2.78
+ t.float :temperature
end
create_table :orders, :force => true do |t|
generated by cgit v1.2.3 (git 2.25.1) at 2025-03-15 16:13:28 +0000