Skip to content

Commit

Permalink
[PATCH] Optimize Error I18n to avoid unecessary lookups and just retr…
Browse files Browse the repository at this point in the history
…ieve values when needed [#3477 status:resolved].

Signed-off-by: Joshua Peek <josh@joshpeek.com>
  • Loading branch information
josevalim authored and josh committed Nov 10, 2009
1 parent 6c0028d commit a454012
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 15 deletions.
36 changes: 21 additions & 15 deletions activerecord/lib/active_record/validations.rb
Expand Up @@ -22,21 +22,18 @@ def initialize(base, attribute, type = nil, options = {})
self.base = base
self.attribute = attribute
self.type = type || :invalid
self.options = options
self.message = options.delete(:message) || self.type
self.options = {
:scope => [:activerecord, :errors],
:model => @base.class.human_name,
:attribute => @base.class.human_attribute_name(attribute.to_s),
:value => value
}.merge!(options)
end

def message
generate_message(@message, options.dup)
# When type is a string, it means that we do not have to do a lookup, because
# the user already sent the "final" message.
type.is_a?(String) ? type : generate_message(default_options)
end

def full_message
attribute.to_s == 'base' ? message : generate_full_message(message, options.dup)
attribute.to_s == 'base' ? message : generate_full_message(default_options)
end

alias :to_s :message
Expand Down Expand Up @@ -65,16 +62,16 @@ def value
# <li><tt>activerecord.errors.messages.blank</tt></li>
# <li>any default you provided through the +options+ hash (in the activerecord.errors scope)</li>
# </ol>
def generate_message(message, options = {})
def generate_message(options = {})
keys = @base.class.self_and_descendants_from_active_record.map do |klass|
[ :"models.#{klass.name.underscore}.attributes.#{attribute}.#{message}",
:"models.#{klass.name.underscore}.#{message}" ]
[ :"models.#{klass.name.underscore}.attributes.#{attribute}.#{@message}",
:"models.#{klass.name.underscore}.#{@message}" ]
end.flatten

keys << options.delete(:default)
keys << :"messages.#{message}"
keys << message if message.is_a?(String)
keys << @type unless @type == message
keys << :"messages.#{@message}"
keys << @message if @message.is_a?(String)
keys << @type unless @type == @message
keys.compact!

options.merge!(:default => keys)
Expand Down Expand Up @@ -108,7 +105,7 @@ def generate_message(message, options = {})
# full_messages:
# title:
# blank: This title is screwed!
def generate_full_message(message, options = {})
def generate_full_message(options = {})
keys = [
:"full_messages.#{@message}",
:'full_messages.format',
Expand All @@ -118,6 +115,15 @@ def generate_full_message(message, options = {})
options.merge!(:default => keys, :message => self.message)
I18n.translate(keys.shift, options)
end

# Return user options with default options.
#
def default_options
options.reverse_merge :scope => [:activerecord, :errors],
:model => @base.class.human_name,
:attribute => @base.class.human_attribute_name(attribute.to_s),
:value => value
end
end

# Active Record validation is reported to and from this object, which is used by Base#save to
Expand Down
14 changes: 14 additions & 0 deletions activerecord/test/cases/validations_test.rb
Expand Up @@ -28,6 +28,12 @@ class ProtectedPerson < ActiveRecord::Base
set_table_name 'people'
attr_accessor :addon
attr_protected :first_name

def special_error
this_method_does_not_exist!
rescue
errors.add(:special_error, "This method does not exist")
end
end

class UniqueReply < Reply
Expand Down Expand Up @@ -172,6 +178,14 @@ def test_create_with_exceptions_using_scope_and_empty_attributes
end
end

def test_values_are_not_retrieved_unless_needed
assert_nothing_raised do
person = ProtectedPerson.new
person.special_error
assert_equal "This method does not exist", person.errors[:special_error]
end
end

def test_single_error_per_attr_iteration
r = Reply.new
r.save
Expand Down

0 comments on commit a454012

Please sign in to comment.