From ef68d3e35cb58f9f491993eeec6e7de99442dd06 Mon Sep 17 00:00:00 2001 From: lulalala Date: Thu, 15 Mar 2018 16:13:18 +0800 Subject: Add ActiveModel::Error and NestedError Add initialize_dup to deep dup. Move proc eval and flexible message position out to Errors, because proc eval is needed for Errors#added? and Errors#delete --- activemodel/lib/active_model/error.rb | 60 ++++++++++++++++++++++++++++ activemodel/lib/active_model/errors.rb | 16 -------- activemodel/lib/active_model/nested_error.rb | 33 +++++++++++++++ 3 files changed, 93 insertions(+), 16 deletions(-) create mode 100644 activemodel/lib/active_model/error.rb create mode 100644 activemodel/lib/active_model/nested_error.rb (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/error.rb b/activemodel/lib/active_model/error.rb new file mode 100644 index 0000000000..214a0b356d --- /dev/null +++ b/activemodel/lib/active_model/error.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module ActiveModel + # == Active \Model \Error + # + # Represents one single error + class Error + CALLBACKS_OPTIONS = [:if, :unless, :on, :allow_nil, :allow_blank, :strict] + MESSAGE_OPTIONS = [:message] + + def initialize(base, attribute, type, **options) + @base = base + @attribute = attribute + @raw_type = type + @type = type || :invalid + @options = options + end + + def initialize_dup(other) + @attribute = @attribute.dup + @raw_type = @raw_type.dup + @type = @type.dup + @options = @options.deep_dup + end + + attr_reader :base, :attribute, :type, :raw_type, :options + + def message + case raw_type + when Symbol + base.errors.generate_message(attribute, raw_type, options.except(*CALLBACKS_OPTIONS)) + else + raw_type + end + end + + def detail + { error: raw_type }.merge(options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS)) + end + + def full_message + base.errors.full_message(attribute, message) + end + + # See if error matches provided +attribute+, +type+ and +options+. + def match?(attribute, type = nil, **options) + if @attribute != attribute || (type && @type != type) + return false + end + + options.each do |key, value| + if @options[key] != value + return false + end + end + + true + end + end +end diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 3a692a3e64..7eff374ce3 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -59,9 +59,6 @@ module ActiveModel class Errors include Enumerable - CALLBACKS_OPTIONS = [:if, :unless, :on, :allow_nil, :allow_blank, :strict] - MESSAGE_OPTIONS = [:message] - class << self attr_accessor :i18n_customize_full_message # :nodoc: end @@ -532,19 +529,6 @@ module ActiveModel end private - def normalize_message(attribute, message, options) - case message - when Symbol - generate_message(attribute, message, options.except(*CALLBACKS_OPTIONS)) - else - message - end - end - - def normalize_detail(message, options) - { error: message }.merge(options.except(*CALLBACKS_OPTIONS + MESSAGE_OPTIONS)) - end - def without_default_proc(hash) hash.dup.tap do |new_h| new_h.default_proc = nil diff --git a/activemodel/lib/active_model/nested_error.rb b/activemodel/lib/active_model/nested_error.rb new file mode 100644 index 0000000000..b01447ac75 --- /dev/null +++ b/activemodel/lib/active_model/nested_error.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +require "active_model/error" +require "forwardable" + +module ActiveModel + # Represents one single error + # @!attribute [r] base + # @return [ActiveModel::Base] the object which the error belongs to + # @!attribute [r] attribute + # @return [Symbol] attribute of the object which the error belongs to + # @!attribute [r] type + # @return [Symbol] error's type + # @!attribute [r] options + # @return [Hash] additional options + # @!attribute [r] inner_error + # @return [Error] inner error + class NestedError < Error + def initialize(base, inner_error, override_options = {}) + @base = base + @inner_error = inner_error + @attribute = override_options.fetch(:attribute) { inner_error.attribute } + @type = override_options.fetch(:type) { inner_error.type } + @raw_type = inner_error.raw_type + @options = inner_error.options + end + + attr_reader :inner_error + + extend Forwardable + def_delegators :@inner_error, :full_message, :message + end +end -- cgit v1.2.3 From d9011e39357300fe78720227af4c13b4bc4ac4dd Mon Sep 17 00:00:00 2001 From: lulalala Date: Tue, 20 Mar 2018 22:39:44 +0800 Subject: Change errors Allow `each` to behave in new way if block arity is 1 Ensure dumped marshal from Rails 5 can be loaded Make errors compatible with marshal and YAML dumps from previous versions of Rails Add deprecation warnings Ensure each behave like the past, sorted by attribute --- activemodel/lib/active_model/error.rb | 8 +- activemodel/lib/active_model/errors.rb | 291 +++++++++++++++++++++------------ 2 files changed, 190 insertions(+), 109 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/error.rb b/activemodel/lib/active_model/error.rb index 214a0b356d..b1912f2604 100644 --- a/activemodel/lib/active_model/error.rb +++ b/activemodel/lib/active_model/error.rb @@ -8,7 +8,7 @@ module ActiveModel CALLBACKS_OPTIONS = [:if, :unless, :on, :allow_nil, :allow_blank, :strict] MESSAGE_OPTIONS = [:message] - def initialize(base, attribute, type, **options) + def initialize(base, attribute, type = :invalid, **options) @base = base @attribute = attribute @raw_type = type @@ -56,5 +56,11 @@ module ActiveModel true end + + def strict_match?(attribute, type, **options) + return false unless match?(attribute, type, **options) + + full_message == Error.new(@base, attribute, type, **options).full_message + end end end diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 7eff374ce3..a0b3b0ab54 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -4,6 +4,10 @@ require "active_support/core_ext/array/conversions" require "active_support/core_ext/string/inflections" require "active_support/core_ext/object/deep_dup" require "active_support/core_ext/string/filters" +require "active_support/deprecation" +require "active_model/error" +require "active_model/nested_error" +require "forwardable" module ActiveModel # == Active \Model \Errors @@ -59,12 +63,17 @@ module ActiveModel class Errors include Enumerable + extend Forwardable + def_delegators :@errors, :size, :clear, :blank?, :empty?, *(Enumerable.instance_methods(false) - [:to_a, :include?]) + + LEGACY_ATTRIBUTES = [:messages, :details].freeze + class << self attr_accessor :i18n_customize_full_message # :nodoc: end self.i18n_customize_full_message = false - attr_reader :messages, :details + attr_reader :errors # Pass in the instance of the object that is using the errors object. # @@ -74,18 +83,17 @@ module ActiveModel # end # end def initialize(base) - @base = base - @messages = apply_default_array({}) - @details = apply_default_array({}) + @base = base + @errors = [] end def initialize_dup(other) # :nodoc: - @messages = other.messages.dup - @details = other.details.deep_dup + @errors = other.errors.deep_dup super end # Copies the errors from other. + # For copying errors but keep @base as is. # # other - The ActiveModel::Errors instance. # @@ -93,11 +101,26 @@ module ActiveModel # # person.errors.copy!(other) def copy!(other) # :nodoc: - @messages = other.messages.dup - @details = other.details.dup + @errors = other.errors.deep_dup + @errors.each { |error| + error.instance_variable_set("@base", @base) + } end - # Merges the errors from other. + # Imports one error + # Imported errors are wrapped as a NestedError, + # providing access to original error object. + # If attribute or type needs to be overriden, use `override_options`. + # + # override_options - Hash + # @option override_options [Symbol] :attribute Override the attribute the error belongs to + # @option override_options [Symbol] :type Override type of the error. + def import(error, override_options = {}) + @errors.append(NestedError.new(@base, error, override_options)) + end + + # Merges the errors from other, + # each Error wrapped as NestedError. # # other - The ActiveModel::Errors instance. # @@ -105,8 +128,9 @@ module ActiveModel # # person.errors.merge!(other) def merge!(other) - @messages.merge!(other.messages) { |_, ary1, ary2| ary1 + ary2 } - @details.merge!(other.details) { |_, ary1, ary2| ary1 + ary2 } + other.errors.each { |error| + import(error) + } end # Removes all errors except the given keys. Returns a hash containing the removed errors. @@ -116,18 +140,28 @@ module ActiveModel # person.errors.keys # => [:age, :gender] def slice!(*keys) keys = keys.map(&:to_sym) - @details.slice!(*keys) - @messages.slice!(*keys) + + results = messages.slice!(*keys) + + @errors.keep_if do |error| + keys.include?(error.attribute) + end + + results end - # Clear the error messages. + # Search for errors matching +attribute+, +type+ or +options+. # - # person.errors.full_messages # => ["name cannot be nil"] - # person.errors.clear - # person.errors.full_messages # => [] - def clear - messages.clear - details.clear + # Only supplied params will be matched. + # + # person.errors.where(:name) # => all name errors. + # person.errors.where(:name, :too_short) # => all name errors being too short + # person.errors.where(:name, :too_short, minimum: 2) # => all name errors being too short and minimum is 2 + def where(attribute, type = nil, **options) + attribute, type, options = normalize_arguments(attribute, type, options) + @errors.select { |error| + error.match?(attribute, type, options) + } end # Returns +true+ if the error messages include an error for the given key @@ -137,8 +171,9 @@ module ActiveModel # person.errors.include?(:name) # => true # person.errors.include?(:age) # => false def include?(attribute) - attribute = attribute.to_sym - messages.key?(attribute) && messages[attribute].present? + @errors.any? { |error| + error.match?(attribute.to_sym) + } end alias :has_key? :include? alias :key? :include? @@ -148,10 +183,13 @@ module ActiveModel # person.errors[:name] # => ["cannot be nil"] # person.errors.delete(:name) # => ["cannot be nil"] # person.errors[:name] # => [] - def delete(key) - attribute = key.to_sym - details.delete(attribute) - messages.delete(attribute) + def delete(attribute, type = nil, **options) + attribute, type, options = normalize_arguments(attribute, type, options) + matches = where(attribute, type, options) + matches.each do |error| + @errors.delete(error) + end + matches.map(&:message) end # When passed a symbol or a name of a method, returns an array of errors @@ -160,7 +198,7 @@ module ActiveModel # person.errors[:name] # => ["cannot be nil"] # person.errors['name'] # => ["cannot be nil"] def [](attribute) - messages[attribute.to_sym] + where(attribute.to_sym).map { |error| error.message } end # Iterates through each error key, value pair in the error messages hash. @@ -177,31 +215,37 @@ module ActiveModel # # Will yield :name and "can't be blank" # # then yield :name and "must be specified" # end - def each - messages.each_key do |attribute| - messages[attribute].each { |error| yield attribute, error } - end - end + def each(&block) + if block.arity == 1 + @errors.each(&block) + else + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Enumerating ActiveModel::Errors as a hash has been deprecated. + In Rails 6, `errors` is an array of Error objects, + therefore it should be accessed by a block with a single block + parameter like this: + + person.errors.each do |error| + error.full_message + end - # Returns the number of error messages. - # - # person.errors.add(:name, :blank, message: "can't be blank") - # person.errors.size # => 1 - # person.errors.add(:name, :not_specified, message: "must be specified") - # person.errors.size # => 2 - def size - values.flatten.size + You are passing a block expecting 2 parameters, + so the old hash behavior is simulated. As this is deprecated, + this will result in an ArgumentError in Rails 6.1. + MSG + @errors. + sort { |a, b| a.attribute <=> b.attribute }. + each { |error| yield error.attribute, error.message } + end end - alias :count :size # Returns all message values. # # person.errors.messages # => {:name=>["cannot be nil", "must be specified"]} # person.errors.values # => [["cannot be nil", "must be specified"]] def values - messages.select do |key, value| - !value.empty? - end.values + deprecation_removal_warning(:values) + @errors.map(&:message).freeze end # Returns all message keys. @@ -209,20 +253,11 @@ module ActiveModel # person.errors.messages # => {:name=>["cannot be nil", "must be specified"]} # person.errors.keys # => [:name] def keys - messages.select do |key, value| - !value.empty? - end.keys - end - - # Returns +true+ if no errors are found, +false+ otherwise. - # If the error message is a string it can be empty. - # - # person.errors.full_messages # => ["name cannot be nil"] - # person.errors.empty? # => false - def empty? - size.zero? + deprecation_removal_warning(:keys) + keys = @errors.map(&:attribute) + keys.uniq! + keys.freeze end - alias :blank? :empty? # Returns an xml formatted representation of the Errors hash. # @@ -236,6 +271,7 @@ module ActiveModel # # name must be specified # # def to_xml(options = {}) + deprecation_removal_warning(:to_xml) to_a.to_xml({ root: "errors", skip_types: true }.merge!(options)) end @@ -255,13 +291,36 @@ module ActiveModel # person.errors.to_hash # => {:name=>["cannot be nil"]} # person.errors.to_hash(true) # => {:name=>["name cannot be nil"]} def to_hash(full_messages = false) - if full_messages - messages.each_with_object({}) do |(attribute, array), messages| - messages[attribute] = array.map { |message| full_message(attribute, message) } + hash = {} + @errors.each do |error| + if full_messages + message = error.full_message + else + message = error.message + end + + if hash.has_key?(error.attribute) + hash[error.attribute] << message + else + hash[error.attribute] = [message] + end + end + hash + end + alias :messages :to_hash + + def details + hash = {} + @errors.each do |error| + detail = error.detail + + if hash.has_key?(error.attribute) + hash[error.attribute] << detail + else + hash[error.attribute] = [detail] end - else - without_default_proc(messages) end + hash end # Adds +message+ to the error messages and used validator type to +details+ on +attribute+. @@ -305,17 +364,20 @@ module ActiveModel # # => {:base=>["either name or email must be present"]} # person.errors.details # # => {:base=>[{error: :name_or_email_blank}]} - def add(attribute, message = :invalid, options = {}) - message = message.call if message.respond_to?(:call) - detail = normalize_detail(message, options) - message = normalize_message(attribute, message, options) + def add(attribute, type = :invalid, **options) + error = Error.new( + @base, + *normalize_arguments(attribute, type, options) + ) + if exception = options[:strict] exception = ActiveModel::StrictValidationFailed if exception == true - raise exception, full_message(attribute, message) + raise exception, error.full_message end - details[attribute.to_sym] << detail - messages[attribute.to_sym] << message + @errors.append(error) + + error end # Returns +true+ if an error on the attribute with the given message is @@ -334,13 +396,15 @@ module ActiveModel # person.errors.added? :name, :too_long, count: 24 # => false # person.errors.added? :name, :too_long # => false # person.errors.added? :name, "is too long" # => false - def added?(attribute, message = :invalid, options = {}) - message = message.call if message.respond_to?(:call) + def added?(attribute, type = :invalid, options = {}) + attribute, type, options = normalize_arguments(attribute, type, options) - if message.is_a? Symbol - details[attribute.to_sym].include? normalize_detail(message, options) + if type.is_a? Symbol + @errors.any? { |error| + error.strict_match?(attribute, type, options) + } else - self[attribute].include? message + messages_for(attribute).include?(type) end end @@ -356,12 +420,12 @@ module ActiveModel # person.errors.of_kind? :name, :not_too_long # => false # person.errors.of_kind? :name, "is too long" # => false def of_kind?(attribute, message = :invalid) - message = message.call if message.respond_to?(:call) + attribute, message = normalize_arguments(attribute, message) if message.is_a? Symbol - details[attribute.to_sym].map { |e| e[:error] }.include? message + !where(attribute, message).empty? else - self[attribute].include? message + messages_for(attribute).include?(message) end end @@ -376,7 +440,7 @@ module ActiveModel # person.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| full_message(attribute, message) } + @errors.map(&:full_message) end alias :to_a :full_messages @@ -391,21 +455,16 @@ module ActiveModel # person.errors.full_messages_for(:name) # # => ["Name is too short (minimum is 5 characters)", "Name can't be blank"] def full_messages_for(attribute) - attribute = attribute.to_sym - messages[attribute].map { |message| full_message(attribute, message) } + where(attribute).map(&:full_message).freeze + end + + def messages_for(attribute) + where(attribute).map(&:message).freeze end # Returns a full message for a given attribute. # # person.errors.full_message(:name, 'is invalid') # => "Name is invalid" - # - # The `"%{attribute} %{message}"` error format can be overridden with either - # - # * activemodel.errors.models.person/contacts/addresses.attributes.street.format - # * activemodel.errors.models.person/contacts/addresses.format - # * activemodel.errors.models.person.attributes.name.format - # * activemodel.errors.models.person.format - # * errors.format def full_message(attribute, message) return message if attribute == :base attribute = attribute.to_s @@ -511,34 +570,50 @@ module ActiveModel I18n.translate(key, options) end - def marshal_dump # :nodoc: - [@base, without_default_proc(@messages), without_default_proc(@details)] - end - def marshal_load(array) # :nodoc: - @base, @messages, @details = array - apply_default_array(@messages) - apply_default_array(@details) + # Rails 5 + @errors = [] + @base = array[0] + add_from_legacy_details_hash(array[2]) end def init_with(coder) # :nodoc: - coder.map.each { |k, v| instance_variable_set(:"@#{k}", v) } - @details ||= {} - apply_default_array(@messages) - apply_default_array(@details) + data = coder.map + + data.each { |k, v| + next if LEGACY_ATTRIBUTES.include?(k.to_sym) + instance_variable_set(:"@#{k}", v) + } + + @errors ||= [] + + # Legacy support Rails 5.x details hash + add_from_legacy_details_hash(data["details"]) if data.key?("details") end - private - def without_default_proc(hash) - hash.dup.tap do |new_h| - new_h.default_proc = nil + private + + def normalize_arguments(attribute, type, **options) + # Evaluate proc first + if type.respond_to?(:call) + type = type.call(@base, options) + end + + [attribute.to_sym, type, options] end - end - def apply_default_array(hash) - hash.default_proc = proc { |h, key| h[key] = [] } - hash - end + def add_from_legacy_details_hash(details) + details.each { |attribute, errors| + errors.each { |error| + type = error.delete(:error) + add(attribute, type, error) + } + } + end + + def deprecation_removal_warning(method_name) + ActiveSupport::Deprecation.warn("ActiveModel::Errors##{method_name} is deprecated and will be removed in Rails 6.1") + end end # Raised when a validation cannot be corrected by end users and are considered -- cgit v1.2.3 From ea77205a9ff71ebd0dc7cf29e598ef126c9807f5 Mon Sep 17 00:00:00 2001 From: lulalala Date: Mon, 26 Mar 2018 13:09:59 +0800 Subject: Add convenience method group_by_attribute Many operations need grouping of errors by attributes, e.g. ActiveRecord::AutosaveAssociation#association_valid? Refactor other methods using group_by_attribute --- activemodel/lib/active_model/errors.rb | 29 +++++++++-------------------- 1 file changed, 9 insertions(+), 20 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index a0b3b0ab54..9800d9305e 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -292,18 +292,9 @@ module ActiveModel # person.errors.to_hash(true) # => {:name=>["name cannot be nil"]} def to_hash(full_messages = false) hash = {} - @errors.each do |error| - if full_messages - message = error.full_message - else - message = error.message - end - - if hash.has_key?(error.attribute) - hash[error.attribute] << message - else - hash[error.attribute] = [message] - end + message_method = full_messages ? :full_message : :message + group_by_attribute.each do |attribute, errors| + hash[attribute] = errors.map(&message_method) end hash end @@ -311,18 +302,16 @@ module ActiveModel def details hash = {} - @errors.each do |error| - detail = error.detail - - if hash.has_key?(error.attribute) - hash[error.attribute] << detail - else - hash[error.attribute] = [detail] - end + group_by_attribute.each do |attribute, errors| + hash[attribute] = errors.map(&:detail) end hash end + def group_by_attribute + group_by(&:attribute) + end + # Adds +message+ to the error messages and used validator type to +details+ on +attribute+. # More than one error can be added to the same +attribute+. # If no +message+ is supplied, :invalid is assumed. -- cgit v1.2.3 From 86b4aa1175b23deca15981fbc19cf7f02b13b25d Mon Sep 17 00:00:00 2001 From: lulalala Date: Mon, 2 Apr 2018 22:40:09 +0800 Subject: Backward compatibility for errors.collect/select etc. All enumerable methods must go through the `each` so it retain old hash behavior. Revert this after Rails 6.1 in order to speed up enumerable methods. --- activemodel/lib/active_model/errors.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 9800d9305e..98e16ea455 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -64,7 +64,7 @@ module ActiveModel include Enumerable extend Forwardable - def_delegators :@errors, :size, :clear, :blank?, :empty?, *(Enumerable.instance_methods(false) - [:to_a, :include?]) + def_delegators :@errors, :size, :clear, :blank?, :empty? LEGACY_ATTRIBUTES = [:messages, :details].freeze @@ -309,7 +309,7 @@ module ActiveModel end def group_by_attribute - group_by(&:attribute) + @errors.group_by(&:attribute) end # Adds +message+ to the error messages and used validator type to +details+ on +attribute+. -- cgit v1.2.3 From cccbac6df6de18b98e300fdd973758447446dbee Mon Sep 17 00:00:00 2001 From: lulalala Date: Tue, 3 Apr 2018 00:17:45 +0800 Subject: Add a transitional method `objects`, for accessing the array directly. This is because we try to accommodate old hash behavior, so `first` and `last` now does not return Error object. --- 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 98e16ea455..805d042cac 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -74,6 +74,7 @@ module ActiveModel self.i18n_customize_full_message = false attr_reader :errors + alias :objects :errors # Pass in the instance of the object that is using the errors object. # -- cgit v1.2.3 From 2a06f13099b3344e93198728795209bc69501d4a Mon Sep 17 00:00:00 2001 From: lulalala Date: Tue, 3 Apr 2018 10:52:36 +0800 Subject: Add messages_for --- activemodel/lib/active_model/errors.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 805d042cac..bb64bc6264 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -65,6 +65,8 @@ module ActiveModel extend Forwardable def_delegators :@errors, :size, :clear, :blank?, :empty? + # TODO: forward all enumerable methods after `each` deprecation is removed. + def_delegators :@errors, :count LEGACY_ATTRIBUTES = [:messages, :details].freeze @@ -199,7 +201,7 @@ module ActiveModel # person.errors[:name] # => ["cannot be nil"] # person.errors['name'] # => ["cannot be nil"] def [](attribute) - where(attribute.to_sym).map { |error| error.message } + messages_for(attribute) end # Iterates through each error key, value pair in the error messages hash. @@ -604,6 +606,10 @@ module ActiveModel def deprecation_removal_warning(method_name) ActiveSupport::Deprecation.warn("ActiveModel::Errors##{method_name} is deprecated and will be removed in Rails 6.1") end + + def deprecation_rename_warning(old_method_name, new_method_name) + ActiveSupport::Deprecation.warn("ActiveModel::Errors##{old_method_name} is deprecated. Please call ##{new_method_name} instead.") + end end # Raised when a validation cannot be corrected by end users and are considered -- cgit v1.2.3 From 86620cc3aa8e2630bc8d934b1a86453276b9eee9 Mon Sep 17 00:00:00 2001 From: lulalala Date: Tue, 3 Apr 2018 13:06:04 +0800 Subject: Allow errors to remove duplicates, and ensure cyclic associations w/ autosave duplicate errors can be removed See SHA 7550f0a016ee6647aaa76c0c0ae30bebc3867288 --- activemodel/lib/active_model/error.rb | 15 +++++++++++++++ activemodel/lib/active_model/errors.rb | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/error.rb b/activemodel/lib/active_model/error.rb index b1912f2604..9731fa74df 100644 --- a/activemodel/lib/active_model/error.rb +++ b/activemodel/lib/active_model/error.rb @@ -62,5 +62,20 @@ module ActiveModel full_message == Error.new(@base, attribute, type, **options).full_message end + + def ==(other) + attributes_for_hash == other.attributes_for_hash + end + alias eql? == + + def hash + attributes_for_hash.hash + end + + protected + + def attributes_for_hash + [@base, @attribute, @raw_type, @options] + end end end diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index bb64bc6264..2d559e06d4 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -64,7 +64,7 @@ module ActiveModel include Enumerable extend Forwardable - def_delegators :@errors, :size, :clear, :blank?, :empty? + def_delegators :@errors, :size, :clear, :blank?, :empty?, :uniq! # TODO: forward all enumerable methods after `each` deprecation is removed. def_delegators :@errors, :count -- cgit v1.2.3 From 582a8e2f9473af9942c00d9c41b6ca109d6ca7d7 Mon Sep 17 00:00:00 2001 From: lulalala Date: Tue, 3 Apr 2018 14:14:42 +0800 Subject: String override options in #import to convert to symbol --- activemodel/lib/active_model/errors.rb | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 2d559e06d4..3ba33bee4d 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -119,6 +119,11 @@ module ActiveModel # @option override_options [Symbol] :attribute Override the attribute the error belongs to # @option override_options [Symbol] :type Override type of the error. def import(error, override_options = {}) + [:attribute, :type].each do |key| + if override_options.key?(key) + override_options[key] = override_options[key].to_sym + end + end @errors.append(NestedError.new(@base, error, override_options)) end -- cgit v1.2.3 From be1585fca07c0823009014c5539e49dc7396524c Mon Sep 17 00:00:00 2001 From: lulalala Date: Tue, 3 Apr 2018 14:48:11 +0800 Subject: Nested attribute error's attribute name to use different key: To keep the same as SHA dcafe995bfe51e53dd04607956be9b54073e9cb6 --- activemodel/lib/active_model/nested_error.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/nested_error.rb b/activemodel/lib/active_model/nested_error.rb index b01447ac75..93348c7771 100644 --- a/activemodel/lib/active_model/nested_error.rb +++ b/activemodel/lib/active_model/nested_error.rb @@ -28,6 +28,6 @@ module ActiveModel attr_reader :inner_error extend Forwardable - def_delegators :@inner_error, :full_message, :message + def_delegators :@inner_error, :message end end -- cgit v1.2.3 From 67d262f70f47154b2476b5fcadf21dd63ebc2597 Mon Sep 17 00:00:00 2001 From: lulalala Date: Sat, 22 Dec 2018 20:25:02 +0800 Subject: Add deprecation to slice! --- activemodel/lib/active_model/errors.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 3ba33bee4d..7c6346f577 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -147,6 +147,8 @@ module ActiveModel # person.errors.slice!(:age, :gender) # => { :name=>["cannot be nil"], :city=>["cannot be nil"] } # person.errors.keys # => [:age, :gender] def slice!(*keys) + deprecation_removal_warning(:slice!) + keys = keys.map(&:to_sym) results = messages.slice!(*keys) -- cgit v1.2.3 From abee0343686b476139c476952b1c68f1fba6b3d0 Mon Sep 17 00:00:00 2001 From: lulalala Date: Sat, 31 Mar 2018 22:42:40 +0800 Subject: Raise deprecation for calling `[:f] = 'b'` or `[:f] << 'b'` Revert some tests to ensure back compatibility --- activemodel/lib/active_model/errors.rb | 50 +++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 4 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 7c6346f577..5440988d27 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -302,11 +302,11 @@ module ActiveModel # person.errors.to_hash(true) # => {:name=>["name cannot be nil"]} def to_hash(full_messages = false) hash = {} - message_method = full_messages ? :full_message : :message + message_method = full_messages ? :full_messages_for : :messages_for group_by_attribute.each do |attribute, errors| - hash[attribute] = errors.map(&message_method) + hash[attribute] = public_send(message_method, attribute) end - hash + DeprecationHandlingMessageHash.new(hash, self) end alias :messages :to_hash @@ -458,7 +458,7 @@ module ActiveModel end def messages_for(attribute) - where(attribute).map(&:message).freeze + DeprecationHandlingMessageArray.new(where(attribute).map(&:message).freeze, self, attribute) end # Returns a full message for a given attribute. @@ -619,6 +619,48 @@ module ActiveModel end end + class DeprecationHandlingMessageHash < SimpleDelegator + def initialize(content, errors) + @errors = errors + super(content) + end + + def []=(attribute, value) + ActiveSupport::Deprecation.warn("Calling `[]=` to an ActiveModel::Errors is deprecated. Please call `ActiveModel::Errors#add` instead.") + + Array(value).each do |message| + @errors.add(attribute, message) + end + __setobj__ @errors.messages + value + end + + def [](attribute) + messages = super(attribute) + + return messages if messages + + __getobj__[attribute]= DeprecationHandlingMessageArray.new([], @errors, attribute) + end + end + + class DeprecationHandlingMessageArray < SimpleDelegator + def initialize(content, errors, attribute) + @errors = errors + @attribute = attribute + super(content) + end + + def <<(message) + ActiveSupport::Deprecation.warn("Calling `<<` to an ActiveModel::Errors message array in order to add an error is deprecated. Please call `ActiveModel::Errors#add` instead.") + + @errors.add(@attribute, message) + __setobj__ @errors[@attribute] + self + end + end + + # Raised when a validation cannot be corrected by end users and are considered # exceptional. # -- cgit v1.2.3 From ba38b40e83302cb9da17c95086d93e5071426668 Mon Sep 17 00:00:00 2001 From: lulalala Date: Sun, 23 Dec 2018 10:33:57 +0800 Subject: Split messages and to_hash Fix double wrapping issue Revert messages_for wrapping. It's a new method so no need to put deprecation warnings. --- activemodel/lib/active_model/errors.rb | 39 ++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 18 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 5440988d27..4b8b601b4b 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -208,7 +208,7 @@ module ActiveModel # person.errors[:name] # => ["cannot be nil"] # person.errors['name'] # => ["cannot be nil"] def [](attribute) - messages_for(attribute) + DeprecationHandlingMessageArray.new(messages_for(attribute), self, attribute) end # Iterates through each error key, value pair in the error messages hash. @@ -302,13 +302,16 @@ module ActiveModel # person.errors.to_hash(true) # => {:name=>["name cannot be nil"]} def to_hash(full_messages = false) hash = {} - message_method = full_messages ? :full_messages_for : :messages_for + message_method = full_messages ? :full_message : :message group_by_attribute.each do |attribute, errors| - hash[attribute] = public_send(message_method, attribute) + hash[attribute] = errors.map(&message_method) end - DeprecationHandlingMessageHash.new(hash, self) + hash + end + + def messages + DeprecationHandlingMessageHash.new(self) end - alias :messages :to_hash def details hash = {} @@ -458,7 +461,7 @@ module ActiveModel end def messages_for(attribute) - DeprecationHandlingMessageArray.new(where(attribute).map(&:message).freeze, self, attribute) + where(attribute).map(&:message) end # Returns a full message for a given attribute. @@ -620,8 +623,17 @@ module ActiveModel end class DeprecationHandlingMessageHash < SimpleDelegator - def initialize(content, errors) + def initialize(errors) @errors = errors + + content = @errors.to_hash + content.each do |attribute, value| + content[attribute] = DeprecationHandlingMessageArray.new(value, @errors, attribute) + end + content.default_proc = proc do |hash, attribute| + hash[attribute] = DeprecationHandlingMessageArray.new([], @errors, attribute) + end + super(content) end @@ -631,16 +643,8 @@ module ActiveModel Array(value).each do |message| @errors.add(attribute, message) end - __setobj__ @errors.messages - value - end - - def [](attribute) - messages = super(attribute) - - return messages if messages - __getobj__[attribute]= DeprecationHandlingMessageArray.new([], @errors, attribute) + super(attribute, DeprecationHandlingMessageArray.new(@errors.messages_for(attribute), @errors, attribute)) end end @@ -655,12 +659,11 @@ module ActiveModel ActiveSupport::Deprecation.warn("Calling `<<` to an ActiveModel::Errors message array in order to add an error is deprecated. Please call `ActiveModel::Errors#add` instead.") @errors.add(@attribute, message) - __setobj__ @errors[@attribute] + __setobj__ @errors.messages_for(@attribute) self end end - # Raised when a validation cannot be corrected by end users and are considered # exceptional. # -- cgit v1.2.3 From 90815b12c51284b497d55ef52ce4e962f5d33f7f Mon Sep 17 00:00:00 2001 From: lulalala Date: Wed, 26 Dec 2018 22:36:02 +0800 Subject: Fix spec --- activemodel/lib/active_model/lint.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/lint.rb b/activemodel/lib/active_model/lint.rb index b7ceabb59a..f9bfed95f1 100644 --- a/activemodel/lib/active_model/lint.rb +++ b/activemodel/lib/active_model/lint.rb @@ -101,7 +101,7 @@ module ActiveModel # locale. If no error is present, the method should return an empty array. def test_errors_aref assert_respond_to model, :errors - assert model.errors[:hello].is_a?(Array), "errors#[] should return an Array" + assert_equal [], model.errors[:hello], "errors#[] should return an empty Array" end private -- cgit v1.2.3 From e7834214a668cde0a4f7757f7f4a3d78f73f2fd8 Mon Sep 17 00:00:00 2001 From: lulalala Date: Sat, 12 Jan 2019 21:03:22 +0800 Subject: Fix equality comparison raising error bug --- activemodel/lib/active_model/error.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/error.rb b/activemodel/lib/active_model/error.rb index 9731fa74df..5a1298e27f 100644 --- a/activemodel/lib/active_model/error.rb +++ b/activemodel/lib/active_model/error.rb @@ -64,7 +64,7 @@ module ActiveModel end def ==(other) - attributes_for_hash == other.attributes_for_hash + other.is_a?(self.class) && attributes_for_hash == other.attributes_for_hash end alias eql? == -- cgit v1.2.3 From 514c4b4d5319f10a275813f1af5a21ed3f41263d Mon Sep 17 00:00:00 2001 From: lulalala Date: Sat, 19 Jan 2019 09:53:25 +0800 Subject: Freeze DeprecationHandling array and hash --- activemodel/lib/active_model/errors.rb | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 4b8b601b4b..81a65a60a7 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -151,7 +151,7 @@ module ActiveModel keys = keys.map(&:to_sym) - results = messages.slice!(*keys) + results = messages.dup.slice!(*keys) @errors.keep_if do |error| keys.include?(error.attribute) @@ -625,16 +625,7 @@ module ActiveModel class DeprecationHandlingMessageHash < SimpleDelegator def initialize(errors) @errors = errors - - content = @errors.to_hash - content.each do |attribute, value| - content[attribute] = DeprecationHandlingMessageArray.new(value, @errors, attribute) - end - content.default_proc = proc do |hash, attribute| - hash[attribute] = DeprecationHandlingMessageArray.new([], @errors, attribute) - end - - super(content) + super(prepare_content) end def []=(attribute, value) @@ -644,15 +635,31 @@ module ActiveModel @errors.add(attribute, message) end - super(attribute, DeprecationHandlingMessageArray.new(@errors.messages_for(attribute), @errors, attribute)) + __setobj__ prepare_content end + + private + + def prepare_content + content = @errors.to_hash + content.each do |attribute, value| + content[attribute] = DeprecationHandlingMessageArray.new(value, @errors, attribute) + end + content.default_proc = proc do |hash, attribute| + hash = hash.dup + hash[attribute] = DeprecationHandlingMessageArray.new([], @errors, attribute) + __setobj__ hash.freeze + hash[attribute] + end + content.freeze + end end class DeprecationHandlingMessageArray < SimpleDelegator def initialize(content, errors, attribute) @errors = errors @attribute = attribute - super(content) + super(content.freeze) end def <<(message) -- cgit v1.2.3 From f7f42a2d0e7154f30d3f1f6cbedf14fc2c3f5b52 Mon Sep 17 00:00:00 2001 From: lulalala Date: Sun, 17 Mar 2019 10:25:08 +0800 Subject: Fix messages[]= does not override value --- 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 81a65a60a7..ac19b4625e 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -631,6 +631,7 @@ module ActiveModel def []=(attribute, value) ActiveSupport::Deprecation.warn("Calling `[]=` to an ActiveModel::Errors is deprecated. Please call `ActiveModel::Errors#add` instead.") + @errors.delete(attribute) Array(value).each do |message| @errors.add(attribute, message) end -- cgit v1.2.3 From aaa0c3279745e3405bc3279924e41cb641e1af8e Mon Sep 17 00:00:00 2001 From: lulalala Date: Sun, 17 Mar 2019 15:57:00 +0800 Subject: Set default array to details maintaining behavior errors.details[:foo].any? --- activemodel/lib/active_model/errors.rb | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'activemodel/lib') diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index ac19b4625e..a9af426fb1 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -318,7 +318,7 @@ module ActiveModel group_by_attribute.each do |attribute, errors| hash[attribute] = errors.map(&:detail) end - hash + DeprecationHandlingDetailsHash.new(hash) end def group_by_attribute @@ -672,6 +672,14 @@ module ActiveModel end end + class DeprecationHandlingDetailsHash < SimpleDelegator + def initialize(details) + details.default = [] + details.freeze + super(details) + end + end + # Raised when a validation cannot be corrected by end users and are considered # exceptional. # -- cgit v1.2.3