From 8620bf90c5e486e1ec44b9aabb63f8c848668ed2 Mon Sep 17 00:00:00 2001 From: Bogdan Gusiev Date: Wed, 17 Aug 2011 17:26:00 +0300 Subject: Implemented strict validation concept In order to deliver debug information to dev team instead of display error message to end user Implemented strict validation concept that suppose to define validation that always raise exception when fails --- activemodel/lib/active_model/errors.rb | 8 +++++++- activemodel/lib/active_model/validations/acceptance.rb | 2 ++ .../lib/active_model/validations/confirmation.rb | 2 ++ activemodel/lib/active_model/validations/exclusion.rb | 2 ++ activemodel/lib/active_model/validations/format.rb | 2 ++ activemodel/lib/active_model/validations/inclusion.rb | 2 ++ activemodel/lib/active_model/validations/length.rb | 2 ++ .../lib/active_model/validations/numericality.rb | 2 ++ activemodel/lib/active_model/validations/presence.rb | 2 ++ activemodel/lib/active_model/validations/validates.rb | 18 +++++++++++++++--- activemodel/lib/active_model/validations/with.rb | 6 ++++-- 11 files changed, 42 insertions(+), 6 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 36819553ee..843c0c3cb5 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -63,7 +63,7 @@ module ActiveModel class Errors include Enumerable - CALLBACKS_OPTIONS = [:if, :unless, :on, :allow_nil, :allow_blank] + CALLBACKS_OPTIONS = [:if, :unless, :on, :allow_nil, :allow_blank, :strict] attr_reader :messages @@ -218,6 +218,9 @@ module ActiveModel elsif message.is_a?(Proc) message = message.call end + if options[:strict] + raise ActiveModel::StrictValidationFailed, message + end self[attribute] << message end @@ -319,4 +322,7 @@ module ActiveModel I18n.translate(key, options) end end + + class StrictValidationFailed < StandardError + end end diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb index 01907ac9da..e628c6f306 100644 --- a/activemodel/lib/active_model/validations/acceptance.rb +++ b/activemodel/lib/active_model/validations/acceptance.rb @@ -58,6 +58,8 @@ module ActiveModel # :unless => Proc.new { |user| user.signup_step <= 2 }). # The method, proc or string should return or evaluate to a true or # false value. + # * :strict - Specifies whether validation should be strict. + # See ActiveModel::Validation#validates! for more information def validates_acceptance_of(*attr_names) validates_with AcceptanceValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/confirmation.rb b/activemodel/lib/active_model/validations/confirmation.rb index a9dcb0b505..6573a7d264 100644 --- a/activemodel/lib/active_model/validations/confirmation.rb +++ b/activemodel/lib/active_model/validations/confirmation.rb @@ -58,6 +58,8 @@ module ActiveModel # :unless => :skip_validation, or # :unless => Proc.new { |user| user.signup_step <= 2 }). The # method, proc or string should return or evaluate to a true or false value. + # * :strict - Specifies whether validation should be strict. + # See ActiveModel::Validation#validates! for more information def validates_confirmation_of(*attr_names) validates_with ConfirmationValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/exclusion.rb b/activemodel/lib/active_model/validations/exclusion.rb index d3b8d31502..644cc814a7 100644 --- a/activemodel/lib/active_model/validations/exclusion.rb +++ b/activemodel/lib/active_model/validations/exclusion.rb @@ -59,6 +59,8 @@ module ActiveModel # * :unless - Specifies a method, proc or string to call to determine if the validation should # not occur (e.g. :unless => :skip_validation, or :unless => Proc.new { |user| user.signup_step <= 2 }). The # method, proc or string should return or evaluate to a true or false value. + # * :strict - Specifies whether validation should be strict. + # See ActiveModel::Validation#validates! for more information def validates_exclusion_of(*attr_names) validates_with ExclusionValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/format.rb b/activemodel/lib/active_model/validations/format.rb index 090e8cfbae..d3faa8c6a6 100644 --- a/activemodel/lib/active_model/validations/format.rb +++ b/activemodel/lib/active_model/validations/format.rb @@ -84,6 +84,8 @@ module ActiveModel # * :unless - Specifies a method, proc or string to call to determine if the validation should # not occur (e.g. :unless => :skip_validation, or :unless => Proc.new { |user| user.signup_step <= 2 }). The # method, proc or string should return or evaluate to a true or false value. + # * :strict - Specifies whether validation should be strict. + # See ActiveModel::Validation#validates! for more information def validates_format_of(*attr_names) validates_with FormatValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/inclusion.rb b/activemodel/lib/active_model/validations/inclusion.rb index 9a9270d615..147e2ecb69 100644 --- a/activemodel/lib/active_model/validations/inclusion.rb +++ b/activemodel/lib/active_model/validations/inclusion.rb @@ -59,6 +59,8 @@ module ActiveModel # * :unless - Specifies a method, proc or string to call to determine if the validation should # not occur (e.g. :unless => :skip_validation, or :unless => Proc.new { |user| user.signup_step <= 2 }). The # method, proc or string should return or evaluate to a true or false value. + # * :strict - Specifies whether validation should be strict. + # See ActiveModel::Validation#validates! for more information def validates_inclusion_of(*attr_names) validates_with InclusionValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index 144e73904e..eb7aac709d 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -96,6 +96,8 @@ module ActiveModel # * :tokenizer - Specifies how to split up the attribute string. (e.g. :tokenizer => lambda {|str| str.scan(/\w+/)} to # count words as in above example.) # Defaults to lambda{ |value| value.split(//) } which counts individual characters. + # * :strict - Specifies whether validation should be strict. + # See ActiveModel::Validation#validates! for more information def validates_length_of(*attr_names) validates_with LengthValidator, _merge_attributes(attr_names) end diff --git a/activemodel/lib/active_model/validations/numericality.rb b/activemodel/lib/active_model/validations/numericality.rb index 0d1903362c..34d447a0fa 100644 --- a/activemodel/lib/active_model/validations/numericality.rb +++ b/activemodel/lib/active_model/validations/numericality.rb @@ -107,6 +107,8 @@ module ActiveModel # * :unless - Specifies a method, proc or string to call to determine if the validation should # not occur (e.g. :unless => :skip_validation, or :unless => Proc.new { |user| user.signup_step <= 2 }). The # method, proc or string should return or evaluate to a true or false value. + # * :strict - Specifies whether validation should be strict. + # See ActiveModel::Validation#validates! for more information # # The following checks can also be supplied with a proc or a symbol which corresponds to a method: # * :greater_than diff --git a/activemodel/lib/active_model/validations/presence.rb b/activemodel/lib/active_model/validations/presence.rb index cfb4c33dcc..35af7152db 100644 --- a/activemodel/lib/active_model/validations/presence.rb +++ b/activemodel/lib/active_model/validations/presence.rb @@ -35,6 +35,8 @@ module ActiveModel # * unless - Specifies a method, proc or string to call to determine if the validation should # not occur (e.g. :unless => :skip_validation, or :unless => Proc.new { |user| user.signup_step <= 2 }). # The method, proc or string should return or evaluate to a true or false value. + # * :strict - Specifies whether validation should be strict. + # See ActiveModel::Validation#validates! for more information # def validates_presence_of(*attr_names) validates_with PresenceValidator, _merge_attributes(attr_names) diff --git a/activemodel/lib/active_model/validations/validates.rb b/activemodel/lib/active_model/validations/validates.rb index 7ff42de00b..43b095d11a 100644 --- a/activemodel/lib/active_model/validations/validates.rb +++ b/activemodel/lib/active_model/validations/validates.rb @@ -70,8 +70,8 @@ module ActiveModel # validator's initializer as +options[:in]+ while other types including # regular expressions and strings are passed as +options[:with]+ # - # Finally, the options +:if+, +:unless+, +:on+, +:allow_blank+ and +:allow_nil+ can be given - # to one specific validator, as a hash: + # Finally, the options +:if+, +:unless+, +:on+, +:allow_blank+, +:allow_nil+ and +:strict+ + # can be given to one specific validator, as a hash: # # validates :password, :presence => { :if => :password_required? }, :confirmation => true # @@ -101,12 +101,24 @@ module ActiveModel end end + # This method is used to define validation that can not be correcterized by end user + # and is considered exceptional. + # So each validator defined with bang or :strict option set to true + # will always raise ActiveModel::InternalValidationFailed instead of adding error + # when validation fails + # See validates for more information about validation itself. + def validates!(*attributes) + options = attributes.extract_options! + options[:strict] = true + validates(*(attributes << options)) + end + protected # When creating custom validators, it might be useful to be able to specify # additional default keys. This can be done by overwriting this method. def _validates_default_keys - [ :if, :unless, :on, :allow_blank, :allow_nil ] + [ :if, :unless, :on, :allow_blank, :allow_nil , :strict] end def _parse_validates_options(options) #:nodoc: diff --git a/activemodel/lib/active_model/validations/with.rb b/activemodel/lib/active_model/validations/with.rb index a87b213fe4..83aae206a6 100644 --- a/activemodel/lib/active_model/validations/with.rb +++ b/activemodel/lib/active_model/validations/with.rb @@ -61,7 +61,9 @@ module ActiveModel # (e.g. :unless => :skip_validation, or # :unless => Proc.new { |user| user.signup_step <= 2 }). # The method, proc or string should return or evaluate to a true or false value. - # + # * :strict - Specifies whether validation should be strict. + # See ActiveModel::Validation#validates! for more information + # If you pass any additional configuration options, they will be passed # to the class and available as options: # @@ -140,4 +142,4 @@ module ActiveModel end end end -end \ No newline at end of file +end -- cgit v1.2.3 From 1be3442a0a6df37525ff706fecfd65de7eae65cf Mon Sep 17 00:00:00 2001 From: Anand Date: Tue, 23 Aug 2011 16:30:02 +0530 Subject: added missing require array/wrap in serialization --- activemodel/lib/active_model/serialization.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/serialization.rb b/activemodel/lib/active_model/serialization.rb index 9260c5082d..b9f6f6cbbf 100644 --- a/activemodel/lib/active_model/serialization.rb +++ b/activemodel/lib/active_model/serialization.rb @@ -1,5 +1,7 @@ require 'active_support/core_ext/hash/except' require 'active_support/core_ext/hash/slice' +require 'active_support/core_ext/array/wrap' + module ActiveModel # == Active Model Serialization -- cgit v1.2.3 From cbb147931b2f547ad76a7ed233430642490c6000 Mon Sep 17 00:00:00 2001 From: Bogdan Gusiev Date: Thu, 25 Aug 2011 16:43:43 +0300 Subject: Typo fix --- activemodel/lib/active_model/validations/validates.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/validations/validates.rb b/activemodel/lib/active_model/validations/validates.rb index 43b095d11a..b85c2453fb 100644 --- a/activemodel/lib/active_model/validations/validates.rb +++ b/activemodel/lib/active_model/validations/validates.rb @@ -101,7 +101,7 @@ module ActiveModel end end - # This method is used to define validation that can not be correcterized by end user + # This method is used to define validation that can not be corrected by end user # and is considered exceptional. # So each validator defined with bang or :strict option set to true # will always raise ActiveModel::InternalValidationFailed instead of adding error -- cgit v1.2.3 From d0f9f4e66467f7ab6a75c16e1a5b9d00912870cc Mon Sep 17 00:00:00 2001 From: Josh Nesbitt Date: Fri, 2 Sep 2011 13:28:38 +0100 Subject: Fix typo in ActiveModel::Dirty comment. define_attribute_methods is a class method, not attribute. --- activemodel/lib/active_model/dirty.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index e3e71525fa..166cccf161 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -29,7 +29,7 @@ module ActiveModel # # include ActiveModel::Dirty # - # define_attribute_methods = [:name] + # define_attribute_methods [:name] # # def name # @name -- cgit v1.2.3 From 67790644372ad3a771810f1d6d99687d795789ea Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Thu, 1 Sep 2011 23:54:17 -0500 Subject: Remove hard dependency on bcrypt. --- activemodel/lib/active_model/secure_password.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 63380d6ffd..a73276199a 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -1,5 +1,3 @@ -require 'bcrypt' - module ActiveModel module SecurePassword extend ActiveSupport::Concern @@ -30,6 +28,9 @@ module ActiveModel # User.find_by_name("david").try(:authenticate, "notright") # => nil # User.find_by_name("david").try(:authenticate, "mUc3m00RsqyRe") # => user def has_secure_password + gem 'bcrypt-ruby', '~> 3.0.0' + require 'bcrypt' + attr_reader :password validates_confirmation_of :password -- cgit v1.2.3 From 9b02f3f41f9be96bcc61222a8dcd197c1a2edf79 Mon Sep 17 00:00:00 2001 From: Guillermo Iguaran Date: Sat, 3 Sep 2011 00:27:07 -0500 Subject: Add comments about bcrypt-ruby gem to SecurePassword --- activemodel/lib/active_model/secure_password.rb | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index a73276199a..7a109d9a52 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -10,6 +10,10 @@ module ActiveModel # a "password_confirmation" attribute) are automatically added. # You can add more validations by hand if need be. # + # You need to add bcrypt-ruby (~> 3.0.0) to Gemfile to use has_secure_password: + # + # gem 'bcrypt-ruby', '~> 3.0.0' + # # Example using Active Record (which automatically includes ActiveModel::SecurePassword): # # # Schema: User(name:string, password_digest:string) @@ -28,6 +32,8 @@ module ActiveModel # User.find_by_name("david").try(:authenticate, "notright") # => nil # User.find_by_name("david").try(:authenticate, "mUc3m00RsqyRe") # => user def has_secure_password + # Load bcrypt-ruby only when has_secured_password is used to avoid make ActiveModel + # (and by extension the entire framework) dependent on a binary library. gem 'bcrypt-ruby', '~> 3.0.0' require 'bcrypt' -- cgit v1.2.3 From e0335e2ccbdbbd14715877c2d9c07792622e883a Mon Sep 17 00:00:00 2001 From: Damien Mathieu <42@dmathieu.com> Date: Tue, 6 Sep 2011 18:35:12 +0200 Subject: add has_key? to ActiveModel::Errors --- activemodel/lib/active_model/errors.rb | 1 + 1 file changed, 1 insertion(+) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 843c0c3cb5..7828434927 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -88,6 +88,7 @@ module ActiveModel def include?(error) (v = messages[error]) && v.any? end + alias :has_key? :include? # Get messages for +key+ def get(key) -- cgit v1.2.3 From f5a944f662d3236f7bf3162d3b61850c61339b50 Mon Sep 17 00:00:00 2001 From: Lawrence Pit Date: Fri, 9 Sep 2011 18:28:25 +1000 Subject: Add ability to get an individual full error message + test for full_messages. --- activemodel/lib/active_model/errors.rb | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 7828434927..d91e4a2b6a 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -180,6 +180,7 @@ module ActiveModel all? { |k, v| v && v.empty? } end alias_method :blank?, :empty? + # Returns an xml formatted representation of the Errors hash. # # p.errors.add(:name, "can't be blank") @@ -254,20 +255,22 @@ module ActiveModel # company.errors.full_messages # => # ["Name is too short (minimum is 5 characters)", "Name can't be blank", "Email can't be blank"] def full_messages - map { |attribute, message| - if attribute == :base - message - else - attr_name = attribute.to_s.gsub('.', '_').humanize - attr_name = @base.class.human_attribute_name(attribute, :default => attr_name) - - I18n.t(:"errors.format", { - :default => "%{attribute} %{message}", - :attribute => attr_name, - :message => message - }) - end - } + map { |attribute, message| full_message(attribute, message) } + end + + # Returns a full message for a given attribute. + # + # company.errors.full_message(:name, "is invalid") # => + # "Name is invalid" + def full_message(attribute, message) + return message if attribute == :base + attr_name = attribute.to_s.gsub('.', '_').humanize + attr_name = @base.class.human_attribute_name(attribute, :default => attr_name) + I18n.t(:"errors.format", { + :default => "%{attribute} %{message}", + :attribute => attr_name, + :message => message + }) end # Translates an error message in its default scope -- cgit v1.2.3 From 8b8b7143efe2e0bac5bcfe90264e4baa66bdb532 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Sun, 4 Sep 2011 22:07:07 +0100 Subject: Use an empty AttributeMethodMatcher by default. This means that attribute methods which don't exist will get generated when define_attribute_methods is called, so we don't have to use hacks like `attribute_method_suffix ''`. --- activemodel/lib/active_model/attribute_methods.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index bdc0eb4a0d..f058d3aa79 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -60,7 +60,7 @@ module ActiveModel included do class_attribute :attribute_method_matchers, :instance_writer => false - self.attribute_method_matchers = [] + self.attribute_method_matchers = [ClassMethods::AttributeMethodMatcher.new] end module ClassMethods @@ -357,8 +357,11 @@ module ActiveModel if attribute_method_matchers_cache.key?(method_name) attribute_method_matchers_cache[method_name] else + # Must try to match prefixes/suffixes first, or else the matcher with no prefix/suffix + # will match every time. + matchers = attribute_method_matchers.partition(&:plain?).reverse.flatten(1) match = nil - attribute_method_matchers.detect { |method| match = method.match(method_name) } + matchers.detect { |method| match = method.match(method_name) } attribute_method_matchers_cache[method_name] = match end end @@ -387,6 +390,10 @@ module ActiveModel def method_name(attr_name) @method_name % attr_name end + + def plain? + prefix.empty? && suffix.empty? + end end end -- cgit v1.2.3 From 93d574c9627ade0b0bdf4ff05471dabe18cafedc Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 9 Sep 2011 09:02:40 +0100 Subject: refactoring --- activemodel/lib/active_model/attribute_methods.rb | 30 +++++++++-------------- 1 file changed, 11 insertions(+), 19 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index f058d3aa79..b1724c277a 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -284,33 +284,25 @@ module ActiveModel def define_attribute_method(attr_name) attribute_method_matchers.each do |matcher| - unless instance_method_already_implemented?(matcher.method_name(attr_name)) - generate_method = "define_method_#{matcher.prefix}attribute#{matcher.suffix}" + method_name = matcher.method_name(attr_name) + + unless instance_method_already_implemented?(method_name) + generate_method = "define_method_#{matcher.method_missing_target}" if respond_to?(generate_method) send(generate_method, attr_name) else - method_name = matcher.method_name(attr_name) + if method_name =~ COMPILABLE_REGEXP + defn = "def #{method_name}(*args)" + else + defn = "define_method(:'#{method_name}') do |*args|" + end generated_attribute_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1 - if method_defined?('#{method_name}') - undef :'#{method_name}' + #{defn} + send(:#{matcher.method_missing_target}, '#{attr_name}', *args) end RUBY - - if method_name.to_s =~ COMPILABLE_REGEXP - generated_attribute_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method_name}(*args) - send(:#{matcher.method_missing_target}, '#{attr_name}', *args) - end - RUBY - else - generated_attribute_methods.module_eval <<-RUBY, __FILE__, __LINE__ + 1 - define_method('#{method_name}') do |*args| - send('#{matcher.method_missing_target}', '#{attr_name}', *args) - end - RUBY - end end end end -- cgit v1.2.3 From 99bd6b53da9555450afb1e050324007868e0768c Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 9 Sep 2011 09:08:27 +0100 Subject: Add deprecation for doing `attribute_method_suffix ''` --- activemodel/lib/active_model/attribute_methods.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index b1724c277a..c5f7a21d3f 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -1,5 +1,6 @@ require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/class/attribute' +require 'active_support/deprecation' module ActiveModel class MissingAttributeError < NoMethodError @@ -365,6 +366,16 @@ module ActiveModel def initialize(options = {}) options.symbolize_keys! + + if options[:prefix] == '' || options[:suffix] == '' + ActiveSupport::Deprecation.warn( + "Specifying an empty prefix/suffix for an attribute method is no longer " \ + "necessary. If the un-prefixed/suffixed version of the method has not been " \ + "defined when `define_attribute_methods` is called, it will be defined " \ + "automatically." + ) + end + @prefix, @suffix = options[:prefix] || '', options[:suffix] || '' @regex = /^(#{Regexp.escape(@prefix)})(.+?)(#{Regexp.escape(@suffix)})$/ @method_missing_target = "#{@prefix}attribute#{@suffix}" -- cgit v1.2.3 From ac687ed651773fccecbc22cd6d8b07d5439ceb76 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 12 Sep 2011 22:12:12 +0100 Subject: Let Ruby deal with method visibility. Check respond_to_without_attributes? in method_missing. If there is any method that responds (even private), let super handle it and raise NoMethodError if necessary. --- activemodel/lib/active_model/attribute_methods.rb | 24 +++++++++++------------ 1 file changed, 11 insertions(+), 13 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index c5f7a21d3f..baebc91192 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -411,13 +411,18 @@ module ActiveModel # It's also possible to instantiate related objects, so a Client class # belonging to the clients table with a +master_id+ foreign key can # instantiate master through Client#master. - def method_missing(method_id, *args, &block) - method_name = method_id.to_s - if match = match_attribute_method?(method_name) - guard_private_attribute_method!(method_name, args) - return __send__(match.target, match.attr_name, *args, &block) + def method_missing(method, *args, &block) + if respond_to_without_attributes?(method, true) + super + else + match = match_attribute_method?(method.to_s) + + if match + __send__(match.target, match.attr_name, *args, &block) + else + super + end end - super end # A Person object with a name attribute can ask person.respond_to?(:name), @@ -450,13 +455,6 @@ module ActiveModel match && attribute_method?(match.attr_name) ? match : nil end - # prevent method_missing from calling private methods with #send - def guard_private_attribute_method!(method_name, args) - if self.class.private_method_defined?(method_name) - raise NoMethodError.new("Attempt to call private method `#{method_name}'", method_name, args) - end - end - def missing_attribute(attr_name, stack) raise ActiveModel::MissingAttributeError, "missing attribute: #{attr_name}", stack end -- cgit v1.2.3 From 6d8dbeca6b0e676145ecdbba38f2fe56b74b4f8f Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 12 Sep 2011 22:29:45 +0100 Subject: Avoid double super call in some cases. If super was false earlier, it is still going to be false, so we don't need to call it again at the end of the method. --- activemodel/lib/active_model/attribute_methods.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index baebc91192..39ece6d3b3 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -431,15 +431,14 @@ module ActiveModel alias :respond_to_without_attributes? :respond_to? def respond_to?(method, include_private_methods = false) if super - return true + true elsif !include_private_methods && super(method, true) # If we're here then we haven't found among non-private methods # but found among all methods. Which means that the given method is private. - return false - elsif match_attribute_method?(method.to_s) - return true + false + else + !match_attribute_method?(method.to_s).nil? end - super end protected -- cgit v1.2.3 From c89e1c7bdefa2489f6ebd04862a426b7200bf494 Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Mon, 12 Sep 2011 23:58:20 +0100 Subject: Add an attribute_missing method to ActiveModel::AttributeMethods. This can be overloaded by implementors if necessary. --- activemodel/lib/active_model/attribute_methods.rb | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 39ece6d3b3..539e0bbdda 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -362,7 +362,7 @@ module ActiveModel class AttributeMethodMatcher attr_reader :prefix, :suffix, :method_missing_target - AttributeMethodMatch = Struct.new(:target, :attr_name) + AttributeMethodMatch = Struct.new(:target, :attr_name, :method_name) def initialize(options = {}) options.symbolize_keys! @@ -384,7 +384,7 @@ module ActiveModel def match(method_name) if @regex =~ method_name - AttributeMethodMatch.new(method_missing_target, $2) + AttributeMethodMatch.new(method_missing_target, $2, method_name) else nil end @@ -416,15 +416,18 @@ module ActiveModel super else match = match_attribute_method?(method.to_s) - - if match - __send__(match.target, match.attr_name, *args, &block) - else - super - end + match ? attribute_missing(match, *args, &block) : super end end + # attribute_missing is like method_missing, but for attributes. When method_missing is + # called we check to see if there is a matching attribute method. If so, we call + # attribute_missing to dispatch the attribute. This method can be overloaded to + # customise the behaviour. + def attribute_missing(match, *args, &block) + __send__(match.target, match.attr_name, *args, &block) + end + # A Person object with a name attribute can ask person.respond_to?(:name), # person.respond_to?(:name=), and person.respond_to?(:name?) # which will all return +true+. -- cgit v1.2.3 From 778c82bea69eb15908a8bb77999ceac0a749242d Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Tue, 13 Sep 2011 23:46:43 +0100 Subject: Generate attribute method unless it's already in the module. There's no harm in generating a method name that's already defined on the host class, since we're generating the attribute methods in a module that gets included. In fact, this is desirable as it allows the host class to call super. --- activemodel/lib/active_model/attribute_methods.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 539e0bbdda..a201e983cd 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -329,7 +329,7 @@ module ActiveModel protected def instance_method_already_implemented?(method_name) - method_defined?(method_name) + generated_attribute_methods.method_defined?(method_name) end private -- cgit v1.2.3 From 51bef9d8fb0b4da7a104425ab8545e9331387743 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Sun, 18 Sep 2011 09:09:01 -0700 Subject: to_xml should also rely on serializable hash. --- activemodel/lib/active_model/serializers/xml.rb | 46 ++++++++----------------- 1 file changed, 15 insertions(+), 31 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb index 64dda3bcee..d61d9d7119 100644 --- a/activemodel/lib/active_model/serializers/xml.rb +++ b/activemodel/lib/active_model/serializers/xml.rb @@ -15,10 +15,10 @@ module ActiveModel class Attribute #:nodoc: attr_reader :name, :value, :type - def initialize(name, serializable, raw_value=nil) + def initialize(name, serializable, value) @name, @serializable = name, serializable - raw_value = raw_value.in_time_zone if raw_value.respond_to?(:in_time_zone) - @value = raw_value || @serializable.send(name) + value = value.in_time_zone if value.respond_to?(:in_time_zone) + @value = value @type = compute_type end @@ -49,40 +49,24 @@ module ActiveModel def initialize(serializable, options = nil) @serializable = serializable @options = options ? options.dup : {} - - @options[:only] = Array.wrap(@options[:only]).map { |n| n.to_s } - @options[:except] = Array.wrap(@options[:except]).map { |n| n.to_s } end - # To replicate the behavior in ActiveRecord#attributes, :except - # takes precedence over :only. If :only is not set - # for a N level model but is set for the N+1 level models, - # then because :except is set to a default value, the second - # level model can have both :except and :only set. So if - # :only is set, always delete :except. - def attributes_hash - attributes = @serializable.attributes - if options[:only].any? - attributes.slice(*options[:only]) - elsif options[:except].any? - attributes.except(*options[:except]) - else - attributes - end + def serializable_hash + @serializable.serializable_hash(@options.except(:include)) end - def serializable_attributes - attributes_hash.map do |name, value| - self.class::Attribute.new(name, @serializable, value) + def serializable_collection + methods = Array.wrap(options[:methods]).map(&:to_s) + serializable_hash.map do |name, value| + name = name.to_s + if methods.include?(name) + self.class::MethodAttribute.new(name, @serializable, value) + else + self.class::Attribute.new(name, @serializable, value) + end end end - def serializable_methods - Array.wrap(options[:methods]).map do |name| - self.class::MethodAttribute.new(name.to_s, @serializable) if @serializable.respond_to?(name.to_s) - end.compact - end - def serialize require 'builder' unless defined? ::Builder @@ -114,7 +98,7 @@ module ActiveModel end def add_attributes_and_methods - (serializable_attributes + serializable_methods).each do |attribute| + serializable_collection.each do |attribute| key = ActiveSupport::XmlMini.rename_key(attribute.name, options) ActiveSupport::XmlMini.to_tag(key, attribute.value, options.merge(attribute.decorations)) -- cgit v1.2.3