Permalink
Browse files

Changed ActiveRecord to use new callbacks and speed up observers by o…

…nly notifying events that are actually being consumed.

Signed-off-by: Joshua Peek <josh@joshpeek.com>
  • Loading branch information...
1 parent aeab739 commit 4f37b97033f596ec2c95eb53e9964e051c224981 @josevalim josevalim committed with josh Sep 8, 2009
Showing with 376 additions and 461 deletions.
  1. +10 −6 activemodel/lib/active_model/validations.rb
  2. +1 −1 activemodel/lib/active_model/validations/presence.rb
  3. +1 −1 activemodel/lib/active_model/validations/with.rb
  4. +16 −26 activemodel/lib/active_model/validations_repair_helper.rb
  5. +1 −7 activemodel/test/cases/validations/i18n_generate_message_validation_test.rb
  6. +3 −10 activemodel/test/cases/validations/i18n_validation_test.rb
  7. +3 −3 activemodel/test/cases/validations_test.rb
  8. +3 −3 activemodel/test/models/reply.rb
  9. +36 −17 activerecord/lib/active_record/associations.rb
  10. +3 −8 activerecord/lib/active_record/base.rb
  11. +126 −129 activerecord/lib/active_record/callbacks.rb
  12. +15 −5 activerecord/lib/active_record/observer.rb
  13. +5 −30 activerecord/lib/active_record/validations.rb
  14. +1 −1 activerecord/test/cases/associations/has_many_associations_test.rb
  15. +57 −61 activerecord/test/cases/callbacks_test.rb
  16. +0 −2 activerecord/test/cases/helper.rb
  17. +6 −13 activerecord/test/cases/lifecycle_test.rb
  18. +0 −46 activerecord/test/cases/repair_helper.rb
  19. +5 −5 activerecord/test/cases/transactions_test.rb
  20. +2 −10 activerecord/test/cases/validations/i18n_generate_message_validation_test.rb
  21. +4 −20 activerecord/test/cases/validations/i18n_validation_test.rb
  22. +3 −6 activerecord/test/cases/validations_test.rb
  23. +5 −0 activerecord/test/models/company.rb
  24. +4 −4 activerecord/test/models/reply.rb
  25. +1 −1 activerecord/test/models/topic.rb
  26. +53 −46 activesupport/lib/active_support/new_callbacks.rb
  27. +12 −0 activesupport/test/new_callbacks_test.rb
@@ -6,7 +6,7 @@
module ActiveModel
module Validations
extend ActiveSupport::Concern
- include ActiveSupport::Callbacks
+ include ActiveSupport::NewCallbacks
included do
define_callbacks :validate
@@ -64,7 +64,7 @@ def validates_each(*attrs)
attrs = attrs.flatten
# Declare the validation.
- send(validation_method(options[:on]), options) do |record|
+ validate options do |record|
attrs.each do |attr|
value = record.send(:read_attribute_for_validation, attr)
next if (value.nil? && options[:allow_nil]) || (value.blank? && options[:allow_blank])
@@ -73,10 +73,14 @@ def validates_each(*attrs)
end
end
- private
- def validation_method(on)
- :validate
+ def validate(*args, &block)
+ options = args.last
+ if options.is_a?(Hash) && options.key?(:on)
+ options[:if] = Array(options[:if])
+ options[:if] << "@_on_validate == :#{options[:on]}"
end
+ set_callback(:validate, :before, *args, &block)
+ end
end
# Returns the Errors object that holds all information about attribute error messages.
@@ -87,7 +91,7 @@ def errors
# Runs all the specified validations and returns true if no errors were added otherwise false.
def valid?
errors.clear
- run_callbacks(:validate)
+ _run_validate_callbacks
errors.empty?
end
@@ -32,7 +32,7 @@ def validates_presence_of(*attr_names)
# can't use validates_each here, because it cannot cope with nonexistent attributes,
# while errors.add_on_empty can
- send(validation_method(configuration[:on]), configuration) do |record|
+ validate configuration do |record|
record.errors.add_on_blank(attr_names, configuration[:message])
end
end
@@ -51,7 +51,7 @@ module ClassMethods
def validates_with(*args)
configuration = args.extract_options!
- send(validation_method(configuration[:on]), configuration) do |record|
+ validate configuration do |record|
args.each do |klass|
klass.new(record, configuration.except(:on, :if, :unless)).validate
end
@@ -2,44 +2,34 @@ module ActiveModel
module ValidationsRepairHelper
extend ActiveSupport::Concern
- module Toolbox
- def self.record_validations(*model_classes)
- model_classes.inject({}) do |repair, klass|
- repair[klass] ||= {}
- [:validate, :validate_on_create, :validate_on_update].each do |callback|
- ivar = "@#{callback.to_s}_callbacks"
- the_callback = klass.instance_variable_get(ivar) if klass.instance_variable_defined?(ivar)
- repair[klass][callback] = (the_callback.nil? ? nil : the_callback.dup)
- end
- repair
- end
- end
-
- def self.reset_validations(recorded)
- recorded.each do |klass, repairs|
- [:validate, :validate_on_create, :validate_on_update].each do |callback|
- klass.instance_variable_set("@#{callback.to_s}_callbacks", repairs[callback])
- end
- end
- end
- end
-
module ClassMethods
def repair_validations(*model_classes)
setup do
- @validation_repairs = Toolbox.record_validations(*model_classes)
+ @_stored_callbacks = {}
+ model_classes.each do |k|
+ @_stored_callbacks[k] = k._validate_callbacks.dup
+ end
end
teardown do
- Toolbox.reset_validations(@validation_repairs)
+ model_classes.each do |k|
+ k._validate_callbacks = @_stored_callbacks[k]
+ k.__update_callbacks(:validate)
+ end
end
end
end
def repair_validations(*model_classes, &block)
- validation_repairs = Toolbox.record_validations(*model_classes)
+ @__stored_callbacks = {}
+ model_classes.each do |k|
+ @__stored_callbacks[k] = k._validate_callbacks.dup
+ end
return block.call
ensure
- Toolbox.reset_validations(validation_repairs)
+ model_classes.each do |k|
+ k._validate_callbacks = @__stored_callbacks[k]
+ k.__update_callbacks(:validate)
+ end
end
end
end
@@ -5,7 +5,7 @@
class I18nGenerateMessageValidationTest < Test::Unit::TestCase
def setup
- reset_callbacks Person
+ Person.reset_callbacks(:validate)
@person = Person.new
@old_load_path, @old_backend = I18n.load_path, I18n.backend
@@ -45,12 +45,6 @@ def teardown
I18n.backend = @old_backend
end
- def reset_callbacks(*models)
- models.each do |model|
- model.instance_variable_set("@validate_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
- end
- end
-
# validates_inclusion_of: generate_message(attr_name, :inclusion, :default => configuration[:message], :value => value)
def test_generate_message_inclusion_with_default_message
assert_equal 'is not included in the list', @person.errors.generate_message(:title, :inclusion, :default => nil, :value => 'title')
@@ -7,8 +7,7 @@ class I18nValidationTest < ActiveModel::TestCase
include ActiveModel::TestsDatabase
def setup
- reset_callbacks Person
-
+ Person.reset_callbacks(:validate)
@person = Person.new
@old_load_path, @old_backend = I18n.load_path, I18n.backend
@@ -18,17 +17,11 @@ def setup
end
def teardown
- reset_callbacks Person
+ Person.reset_callbacks(:validate)
I18n.load_path.replace @old_load_path
I18n.backend = @old_backend
end
- def reset_callbacks(*models)
- models.each do |model|
- model.instance_variable_set("@validate_callbacks", ActiveSupport::Callbacks::CallbackChain.new)
- end
- end
-
def test_percent_s_interpolation_syntax_in_error_messages_was_deprecated
assert_not_deprecated do
default = "%s interpolation syntax was deprecated"
@@ -532,4 +525,4 @@ def test_validates_with_message_string
assert_equal ["I am a custom error"], @person.errors[:title]
end
-end
+end
@@ -121,8 +121,8 @@ def test_validate_block
end
def test_invalid_validator
- Topic.validate 3
- assert_raise(ArgumentError) { t = Topic.create }
+ Topic.validate :i_dont_exist
+ assert_raise(NameError) { t = Topic.create }
end
def test_errors_to_xml
@@ -189,4 +189,4 @@ def test_deprecated_errors_on_base_and_each
all_errors = t.errors.to_a
assert_deprecated { assert_equal all_errors, t.errors.each_full{|err| err} }
end
-end
+end
@@ -2,11 +2,11 @@
class Reply < Topic
validate :errors_on_empty_content
- validate_on_create :title_is_wrong_create
+ validate :title_is_wrong_create, :on => :create
validate :check_empty_title
- validate_on_create :check_content_mismatch
- validate_on_update :check_wrong_update
+ validate :check_content_mismatch, :on => :create
+ validate :check_wrong_update, :on => :update
attr_accessible :title, :author_name, :author_email_address, :written_on, :content, :last_read
@@ -1491,24 +1491,43 @@ def configure_dependency_for_has_many(reflection, extra_conditions = nil)
end
before_destroy method_name
when :delete_all
- module_eval %Q{
- before_destroy do |record| # before_destroy do |record|
- delete_all_has_many_dependencies(record, # delete_all_has_many_dependencies(record,
- "#{reflection.name}", # "posts",
- #{reflection.class_name}, # Post,
- %@#{dependent_conditions}@) # %@...@) # this is a string literal like %(...)
- end # end
- }
+ # before_destroy do |record|
+ # self.class.send(:delete_all_has_many_dependencies,
+ # record,
+ # "posts",
+ # Post,
+ # %@...@) # this is a string literal like %(...)
+ # end
+ # end
+ module_eval <<-CALLBACK
+ before_destroy do |record|
+ self.class.send(:delete_all_has_many_dependencies,
+ record,
+ "#{reflection.name}",
+ #{reflection.class_name},
+ %@#{dependent_conditions}@)
+ end
+ CALLBACK
when :nullify
- module_eval %Q{
- before_destroy do |record| # before_destroy do |record|
- nullify_has_many_dependencies(record, # nullify_has_many_dependencies(record,
- "#{reflection.name}", # "posts",
- #{reflection.class_name}, # Post,
- "#{reflection.primary_key_name}", # "user_id",
- %@#{dependent_conditions}@) # %@...@) # this is a string literal like %(...)
- end # end
- }
+ # before_destroy do |record|
+ # self.class.send(:nullify_has_many_dependencies,
+ # record,
+ # "posts",
+ # Post,
+ # "user_id",
+ # %@...@) # this is a string literal like %(...)
+ # end
+ # end
+ module_eval <<-CALLBACK
+ before_destroy do |record|
+ self.class.send(:nullify_has_many_dependencies,
+ record,
+ "#{reflection.name}",
+ #{reflection.class_name},
+ "#{reflection.primary_key_name}",
+ %@#{dependent_conditions}@)
+ end
+ CALLBACK
else
raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, or :nullify (#{reflection.options[:dependent].inspect})"
end
@@ -1679,13 +1679,8 @@ def instantiate(record)
object.instance_variable_set("@attributes", record)
object.instance_variable_set("@attributes_cache", Hash.new)
- if object.respond_to_without_attributes?(:after_find)
- object.send(:callback, :after_find)
- end
-
- if object.respond_to_without_attributes?(:after_initialize)
- object.send(:callback, :after_initialize)
- end
+ object.send(:_run_find_callbacks)
+ object.send(:_run_initialize_callbacks)
object
end
@@ -2438,7 +2433,7 @@ def initialize(attributes = nil)
self.attributes = attributes unless attributes.nil?
self.class.send(:scope, :create).each { |att,value| self.send("#{att}=", value) } if self.class.send(:scoped?, :create)
result = yield self if block_given?
- callback(:after_initialize) if respond_to_without_attributes?(:after_initialize)
+ _run_initialize_callbacks
result
end
Oops, something went wrong.

0 comments on commit 4f37b97

Please sign in to comment.