Use errors as first-class objects

app/controllers/epp/base_controller.rb

@@ -28,27 +28,24 @@ module Epp
     protected
 
     def respond_with_command_failed_error(exception)
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2400',
-        msg: 'Command failed',
-      }
+                     message: 'Command failed')
       handle_errors
       log_exception(exception)
     end
 
     def respond_with_object_does_not_exist_error
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2303',
-        msg: 'Object does not exist',
-      }
+                     msg: 'Object does not exist')
       handle_errors
     end
 
     def respond_with_authorization_error
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2201',
-        msg: 'Authorization error',
-      }
+                     msg: 'Authorization error')
       handle_errors
     end
 
@@ -63,10 +60,9 @@ module Epp
     def validate_against_schema
       return if %w[hello error].include?(params[:action])
       schema.validate(params[:nokogiri_frame]).each do |error|
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: 2001,
-          msg: error
-        }
+                       msg: error)
       end
       handle_errors and return if epp_errors.any?
     end
@@ -94,7 +90,7 @@ module Epp
 
     # ERROR + RESPONSE HANDLING
     def epp_errors
-      @errors ||= []
+      @errors ||= ActiveModel::Errors.new(self)
     end
 
     def handle_errors(obj = nil)
@@ -102,21 +98,9 @@ module Epp
 
       if obj
         obj.construct_epp_errors
-        @errors += obj.errors.where[:epp_errors].flatten
+        obj.errors.each { |error| @errors.import error }
       end
 
-      if params[:parsed_frame]&.at_css('update')
-        @errors.each_with_index do |errors, index|
-          if errors[:code] == '2304' &&
-            errors[:value].present? &&
-            errors[:value][:val] == DomainStatus::SERVER_DELETE_PROHIBITED &&
-            errors[:value][:obj] == 'status'
-            @errors[index][:value][:val] = DomainStatus::PENDING_UPDATE
-          end
-        end
-      end
-      @errors.uniq!
-
       render_epp_response '/epp/error'
     end
 
@@ -133,10 +117,9 @@ module Epp
         return true
       end
 
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      msg: 'Parameter value policy error. Allowed only Latin characters.',
-        code: '2306'
-      }
+                     code: '2306')
 
       handle_errors and return false
     end
@@ -180,10 +163,9 @@ module Epp
         else
           missing = el.present? ? el.text.blank? : true
         end
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2003',
-          msg: I18n.t('errors.messages.required_parameter_missing', key: "#{full_selector} [#{attr}]")
-        } if missing
+                       message: I18n.t('errors.messages.required_parameter_missing', key: "#{full_selector} [#{attr}]")) if missing
       end
 
       missing ? false : el # return last selector if it was present
@@ -201,25 +183,22 @@ module Epp
       attribute = element[attribute_selector]
 
       unless attribute
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2003',
-          msg: I18n.t('errors.messages.required_parameter_missing', key: attribute_selector)
-        }
+                       msg: I18n.t('errors.messages.required_parameter_missing', key: attribute_selector))
         return
       end
 
       return if options[:values].include?(attribute)
 
       if options[:policy]
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2306',
-          msg: I18n.t('attribute_is_invalid', attribute: attribute_selector)
-        }
+                       msg: I18n.t('attribute_is_invalid', attribute: attribute_selector))
       else
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2004',
-          msg: I18n.t('parameter_value_range_error', key: attribute_selector)
-        }
+                       msg: I18n.t('parameter_value_range_error', key: attribute_selector))
       end
     end
 
@@ -231,30 +210,27 @@ module Epp
       attribute = element[attribute_selector]
       return if (attribute && options[:values].include?(attribute)) || !attribute
 
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2306',
-        msg: I18n.t('attribute_is_invalid', attribute: attribute_selector)
-      }
+                     msg: I18n.t('attribute_is_invalid', attribute: attribute_selector))
     end
 
     def exactly_one_of(*selectors)
       full_selectors = create_full_selectors(*selectors)
       return if element_count(*full_selectors, use_prefix: false) == 1
 
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2306',
-        msg: I18n.t(:exactly_one_parameter_required, params: full_selectors.join(' OR '))
-      }
+                     msg: I18n.t(:exactly_one_parameter_required, params: full_selectors.join(' OR ')))
     end
 
     def mutually_exclusive(*selectors)
       full_selectors = create_full_selectors(*selectors)
       return if element_count(*full_selectors, use_prefix: false) <= 1
 
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2306',
-        msg: I18n.t(:mutally_exclusive_params, params: full_selectors.join(', '))
-      }
+                     msg: I18n.t(:mutally_exclusive_params, params: full_selectors.join(', ')))
     end
 
     def optional(selector, *validations)
@@ -265,8 +241,8 @@ module Epp
 
       validations.each do |x|
         validator = "#{x.first[0]}_validator".camelize.constantize
-        err = validator.validate_epp(selector.split(' ').last, value)
-        epp_errors << err if err
+        result = validator.validate_epp(selector.split(' ').last, value)
+        epp_errors.add(:epp_errors, result) if result
       end
     end
 
@@ -297,10 +273,9 @@ module Epp
 
     def xml_attrs_present?(ph, attributes) # TODO: THIS IS DEPRECATED AND WILL BE REMOVED IN FUTURE
       attributes.each do |x|
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2003',
-          msg: I18n.t('errors.messages.required_parameter_missing', key: x.last)
-        } unless has_attribute(ph, x)
+                       msg: I18n.t('errors.messages.required_parameter_missing', key: x.last)) unless has_attribute(ph, x)
       end
       epp_errors.empty?
     end
@@ -355,10 +330,9 @@ module Epp
 
     def enforce_epp_session_timeout
       if epp_session.timed_out?
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2201',
-          msg: 'Authorization error: Session timeout',
-        }
+                       msg: 'Authorization error: Session timeout')
         handle_errors
         epp_session.destroy!
       else

app/controllers/epp/contacts_controller.rb

@@ -72,9 +72,10 @@ module Epp
     end
 
     def action_call_response(action:)
-      # rubocop:disable Style/AndOr
-      (handle_errors(@contact) and return) unless action.call
-      # rubocop:enable Style/AndOr
+      unless action.call
+        handle_errors(@contact)
+        return
+      end
 
       if opt_addr?
         @response_code = 1100
@@ -134,24 +135,16 @@ module Epp
       ident = params[:parsed_frame].css('ident')
 
       if ident.present? && ident.attr('type').blank?
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2003',
-          msg: I18n.t('errors.messages.required_ident_attribute_missing', key: 'type')
-        }
+                       msg: I18n.t('errors.messages.required_ident_attribute_missing', key: 'type'))
       end
 
       if ident.present? && ident.text != 'birthday' && ident.attr('cc').blank?
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2003',
-          msg: I18n.t('errors.messages.required_ident_attribute_missing', key: 'cc')
-        }
+                       msg: I18n.t('errors.messages.required_ident_attribute_missing', key: 'cc'))
       end
-      # if ident.present? && ident.attr('cc').blank?
-      # epp_errors << {
-      # code: '2003',
-      # msg: I18n.t('errors.messages.required_ident_attribute_missing', key: 'cc')
-      # }
-      # end
       contact_org_disabled
       fax_disabled
       status_editing_disabled
@@ -178,28 +171,27 @@ module Epp
       return true if ENV['contact_org_enabled'] == 'true'
       return true if params[:parsed_frame].css('postalInfo org').text.blank?
 
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2306',
                      msg: "#{I18n.t(:contact_org_error)}: postalInfo > org [org]"
-      }
+      )
     end
 
     def fax_disabled
       return true if ENV['fax_enabled'] == 'true'
       return true if params[:parsed_frame].css('fax').text.blank?
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2306',
-        msg: "#{I18n.t(:contact_fax_error)}: fax [fax]"
-      }
+                     msg: "#{I18n.t(:contact_fax_error)}: fax [fax]")
     end
 
     def status_editing_disabled
       return true if Setting.client_status_editing_enabled
       return true if params[:parsed_frame].css('status').empty?
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2306',
                      msg: "#{I18n.t(:client_side_status_editing_error)}: status [status]"
-      }
+      )
     end
 
     def address_given?

app/controllers/epp/domains_controller.rb

@@ -90,10 +90,9 @@ module Epp
       action = params[:parsed_frame].css('transfer').first[:op]
 
       if @domain.non_transferable?
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2304',
-          msg: I18n.t(:object_status_prohibits_operation),
-        }
+                       msg: I18n.t(:object_status_prohibits_operation))
         handle_errors
         return
       end
@@ -102,10 +101,9 @@ module Epp
       wrong_transfer_code = provided_transfer_code != @domain.transfer_code
 
       if wrong_transfer_code
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2202',
-          msg: 'Invalid authorization information',
-        }
+                       msg: 'Invalid authorization information')
         handle_errors
         return
       end
@@ -120,10 +118,9 @@ module Epp
       if @domain_transfer
         render_epp_response '/epp/domains/transfer'
       else
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2303',
-          msg: I18n.t('no_transfers_found')
-        }
+                       msg: I18n.t('no_transfers_found'))
         handle_errors
       end
     end
@@ -184,11 +181,10 @@ module Epp
     def validate_transfer
       # period element is disabled for now
       if params[:parsed_frame].css('period').any?
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2307',
                        msg: I18n.t(:unimplemented_object_service),
-          value: { obj: 'period' }
-        }
+                       value: { obj: 'period' })
       end
 
       requires 'transfer > transfer'
@@ -217,10 +213,10 @@ module Epp
       return true if Setting.client_status_editing_enabled
       return true if check_client_hold
       return true if params[:parsed_frame].css('status').empty?
-      epp_errors << {
+      epp_errors.add(:epp_errors,
                      code: '2306',
                      msg: "#{I18n.t(:client_side_status_editing_error)}: status [status]"
-      }
+      )
     end
 
     def check_client_hold
@@ -232,17 +228,15 @@ module Epp
       @domain_pricelist = @domain.pricelist(operation, period.try(:to_i), unit)
       if @domain_pricelist.try(:price) # checking if price list is not found
         if current_user.registrar.balance < @domain_pricelist.price.amount
-          epp_errors << {
+          epp_errors.add(:epp_errors,
                          code: '2104',
-            msg: I18n.t('billing_failure_credit_balance_low')
-          }
+                         msg: I18n.t('billing_failure_credit_balance_low'))
           return false
         end
       else
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2104',
-          msg: I18n.t(:active_price_missing_for_this_operation)
-        }
+                       msg: I18n.t(:active_price_missing_for_this_operation))
         return false
       end
       true

app/controllers/epp/errors_controller.rb

@@ -3,12 +3,12 @@ module Epp
     skip_authorization_check
 
     def error
-      epp_errors << { code: params[:code], msg: params[:msg] }
+      epp_errors.add(:epp_errors, code: params[:code], msg: params[:msg] )
       render_epp_response '/epp/error'
     end
 
     def command_handler
-      epp_errors << { code: '2000', msg: 'Unknown command' }
+      epp_errors.add(:epp_errors, code: '2000', msg: 'Unknown command' )
       render_epp_response '/epp/error'
     end
   end

app/controllers/epp/polls_controller.rb

@@ -43,11 +43,11 @@ module Epp
       @notification = current_user.unread_notifications.find_by(id: params[:parsed_frame].css('poll').first['msgID'])
 
       unless @notification
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: '2303',
                        msg: I18n.t('message_was_not_found'),
-          value: { obj: 'msgID', val: params[:parsed_frame].css('poll').first['msgID'] }
-        }
+                       value: { obj: 'msgID',
+                                val: params[:parsed_frame].css('poll').first['msgID'] })
         handle_errors and return
       end
 

app/controllers/epp/sessions_controller.rb

@@ -20,10 +20,9 @@ module Epp
 
         server_md5 = Certificate.parse_md_from_string(File.read(ENV['cert_path']))
         if client_md5 != server_md5
-          epp_errors << {
+          epp_errors.add(:epp_errors,
                          msg: 'Authentication error; server closing connection (certificate is not valid)',
-            code: '2501'
-          }
+                         code: '2501')
 
           success = false
         end
@@ -32,56 +31,50 @@ module Epp
       if !Rails.env.development? && (!webclient_request && @api_user)
         unless @api_user.pki_ok?(request.env['HTTP_SSL_CLIENT_CERT'],
                                  request.env['HTTP_SSL_CLIENT_S_DN_CN'])
-          epp_errors << {
+          epp_errors.add(:epp_errors,
                          msg: 'Authentication error; server closing connection (certificate is not valid)',
-            code: '2501'
-          }
+                         code: '2501')
 
           success = false
         end
       end
 
       if success && !@api_user
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        msg: 'Authentication error; server closing connection (API user not found)',
-          code: '2501'
-        }
+                       code: '2501')
 
         success = false
       end
 
       if success && !@api_user.try(:active)
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        msg: 'Authentication error; server closing connection (API user is not active)',
-          code: '2501'
-        }
+                       code: '2501')
 
         success = false
       end
 
       if success && @api_user.cannot?(:create, :epp_login)
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        msg: 'Authentication error; server closing connection (API user does not have epp role)',
-          code: '2501'
-        }
+                       code: '2501')
 
         success = false
       end
 
       if success && !ip_white?
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        msg: 'Authentication error; server closing connection (IP is not whitelisted)',
-          code: '2501'
-        }
+                       code: '2501')
 
         success = false
       end
 
       if success && EppSession.limit_reached?(@api_user.registrar)
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        msg: 'Session limit exceeded; server closing connection (connection limit reached)',
-          code: '2502',
-        }
+                       code: '2502')
 
         success = false
       end
@@ -98,10 +91,9 @@ module Epp
         already_authenticated = EppSession.exists?(session_id: epp_session_id)
 
         if already_authenticated
-          epp_errors << {
+          epp_errors.add(:epp_errors,
                          msg: 'Command use error; Already authenticated',
-            code: 2002,
-          }
+                         code: 2002)
           handle_errors
           return
         end
@@ -127,10 +119,9 @@ module Epp
 
     def logout
       unless signed_in?
-        epp_errors << {
+        epp_errors.add(:epp_errors,
                        code: 2201,
-          msg: 'Authorization error'
-        }
+                       msg: 'Authorization error')
         handle_errors
         return
       end

app/models/concerns/epp_errors.rb

@@ -5,35 +5,41 @@ module EppErrors
   end
 
   def construct_epp_errors
-    epp_errors = []
+    epp_errors = ActiveModel::Errors.new(self)
     errors.each do |error|
       attr = error.attribute.to_s.split('.')[0].to_sym
       next if attr == :epp_errors
 
       if self.class.reflect_on_association(attr)
-        epp_errors << collect_child_errors(attr)
+        collect_child_errors(attr).each do |child_error|
+          epp_errors.import child_error
+        end
       end
 
       if self.class.reflect_on_aggregation(attr)
         aggregation = send(attr)
-        epp_errors << collect_aggregation_errors(aggregation)
+        collect_aggregation_errors(aggregation).each do |aggregation_error|
+          epp_errors.import aggregation_error
+        end
         next
       end
-
-      epp_errors << collect_parent_errors(attr, error.message)
+      collect_parent_errors(attr, error.message).each do |parent_error|
+        epp_errors.import parent_error
       end
-    errors.add(:epp_errors, epp_errors.flatten) unless epp_errors.empty?
+    end
+    epp_errors.each { |epp_error| errors.import epp_error}
+    errors
   end
 
   def collect_parent_errors(attr, errors)
     errors = [errors] if errors.is_a?(String)
 
-    epp_errors = []
+    epp_errors = ActiveModel::Errors.new(self)
     errors.each do |err|
       code, value = find_epp_code_and_value(err)
       next unless code
       msg = attr.to_sym == :base ? err : "#{err} [#{attr}]"
-      epp_errors << { code: code, msg: msg, value: value }
+      epp_errors.add(attr, code: code, msg: msg, value: value)
     end
     epp_errors
   end
@@ -41,12 +47,13 @@ module EppErrors
   def collect_child_errors(attr)
     macro = self.class.reflect_on_association(attr).macro
     multi = [:has_and_belongs_to_many, :has_many]
-    # single = [:belongs_to, :has_one]
 
-    epp_errors = []
+    epp_errors = ActiveModel::Errors.new(self)
     send(attr).each do |x|
-      x.errors.messages.each do |attribute, errors|
-        epp_errors << x.collect_parent_errors(attribute, errors)
+      x.errors.each do |error|
+        x.collect_parent_errors(error.attribute, error.message).each do |parent_error|
+          epp_errors.import parent_error
+        end
       end
     end if multi.include?(macro)
 

app/views/epp/error.xml.builder

@@ -1,6 +1,8 @@
 xml.epp_head do
   xml.response do
-    @errors.each do |x|
+    @errors.each do |error|
+      x = error&.options
+      next if x.empty?
       xml.result('code' => x[:code]) do
         xml.msg(x[:msg], 'lang' => 'en')
         model_name = resource ? resource.model_name.singular.sub('epp_','') : controller.controller_name.singularize