Merge pull request #1617 from internetee/epp-contact-update-extract-xml-parsing

Split contact update into XML and Business action parts

app/controllers/epp/contacts_controller.rb

@@ -1,3 +1,5 @@
+require 'deserializers/xml/contact_update'
+
 module Epp
   class ContactsController < BaseController
     before_action :find_contact, only: [:info, :update, :delete]
@@ -43,9 +45,14 @@ module Epp
     def update
       authorize! :update, @contact, @password
 
-      frame = params[:parsed_frame]
+      collected_data = ::Deserializers::Xml::ContactUpdate.new(params[:parsed_frame])
+      action = Actions::ContactUpdate.new(@contact,
+                                          collected_data.contact,
+                                          collected_data.legal_document,
+                                          collected_data.ident,
+                                          current_user)
 
-      if @contact.update_attributes(frame, current_user)
+      if action.call
         if !address_processing? && address_given?
           @response_code = 1100
           @response_description = t('epp.contacts.completed_without_address')

app/models/actions/contact_update.rb

@@ -0,0 +1,105 @@
+module Actions
+  class ContactUpdate
+    attr_reader :contact
+    attr_reader :new_attributes
+    attr_reader :legal_document
+    attr_reader :ident
+    attr_reader :user
+
+    def initialize(contact, new_attributes, legal_document, ident, user)
+      @contact = contact
+      @new_attributes = new_attributes
+      @legal_document = legal_document
+      @ident = ident
+      @user = user
+    end
+
+    def call
+      maybe_remove_address
+      maybe_update_statuses
+      maybe_update_ident
+      maybe_attach_legal_doc
+      commit
+    end
+
+    def maybe_remove_address
+      return if Setting.address_processing?
+
+      new_attributes.delete(:city)
+      new_attributes.delete(:zip)
+      new_attributes.delete(:street)
+      new_attributes.delete(:state)
+      new_attributes.delete(:country_code)
+    end
+
+    def maybe_update_statuses
+      return unless Setting.client_status_editing_enabled
+
+      new_statuses =
+        contact.statuses - new_attributes[:statuses_to_remove] + new_attributes[:statuses_to_add]
+
+      new_attributes[:statuses] = new_statuses
+    end
+
+    def maybe_attach_legal_doc
+      return unless legal_document
+
+      document = contact.legal_documents.create(
+        document_type: legal_document[:type],
+        body: legal_document[:body]
+      )
+
+      contact.legal_document_id = document.id
+    end
+
+    def maybe_update_ident
+      return unless ident[:ident]
+
+      if contact.identifier.valid?
+        submitted_ident = ::Contact::Ident.new(code: ident[:ident],
+                                               type: ident[:ident_type],
+                                               country_code: ident[:ident_country_code])
+
+        if submitted_ident != contact.identifier
+          contact.add_epp_error('2308', nil, nil, I18n.t('epp.contacts.errors.valid_ident'))
+          @error = true
+        end
+      else
+        ident_update_attempt = ident[:ident] != contact.ident
+
+        if ident_update_attempt
+          contact.add_epp_error('2308', nil, nil, I18n.t('epp.contacts.errors.ident_update'))
+          @error = true
+        end
+
+        identifier = ::Contact::Ident.new(code: ident[:ident],
+                                          type: ident[:ident_type],
+                                          country_code: ident[:ident_country_code])
+
+        identifier.validate
+
+        contact.identifier = identifier
+        contact.ident_updated_at ||= Time.zone.now
+      end
+    end
+
+    def commit
+      return false if @error
+
+      contact.upid = user.registrar&.id
+      contact.up_date = Time.zone.now
+
+      contact.attributes = new_attributes
+
+      email_changed = contact.will_save_change_to_email?
+      old_email = contact.email_was
+      updated = contact.save
+
+      if updated && email_changed && contact.registrant?
+        ContactMailer.email_changed(contact: contact, old_email: old_email).deliver_now
+      end
+
+      updated
+    end
+  end
+end

app/models/epp/contact.rb

@@ -1,5 +1,6 @@
 require 'deserializers/xml/legal_document'
 require 'deserializers/xml/ident'
+require 'deserializers/xml/contact'
 
 class Epp::Contact < Contact
   include EppErrors
@@ -23,25 +24,8 @@ class Epp::Contact < Contact
     end
 
     def attrs_from(frame, new_record: false)
-      f = frame
+      at = ::Deserializers::Xml::Contact.new(frame).call
-      at = {}.with_indifferent_access
+      ident_attrs = ::Deserializers::Xml::Ident.new(frame).call
-      at[:name]       = f.css('postalInfo name').text        if f.css('postalInfo name').present?
-      at[:org_name]   = f.css('postalInfo org').text         if f.css('postalInfo org').present?
-      at[:email]      = f.css('email').text                  if f.css('email').present?
-      at[:fax]        = f.css('fax').text                    if f.css('fax').present?
-      at[:phone]      = f.css('voice').text                  if f.css('voice').present?
-
-      if address_processing?
-        at[:city] = f.css('postalInfo addr city').text if f.css('postalInfo addr city').present?
-        at[:zip] = f.css('postalInfo addr pc').text if f.css('postalInfo addr pc').present?
-        at[:street] = f.css('postalInfo addr street').text if f.css('postalInfo addr street').present?
-        at[:state] = f.css('postalInfo addr sp').text if f.css('postalInfo addr sp').present?
-        at[:country_code] = f.css('postalInfo addr cc').text if f.css('postalInfo addr cc').present?
-      end
-
-      at[:auth_info]    = f.css('authInfo pw').text            if f.css('authInfo pw').present?
-
-      ident_attrs = ::Deserializers::Xml::Ident.new(f).call
       at.merge!(ident_attrs) if new_record
       at
     end
@@ -72,8 +56,8 @@ class Epp::Contact < Contact
 
       res
     end
-
   end
+
   delegate :ident_attr_valid?, to: :class
 
   def epp_code_map
@@ -104,102 +88,6 @@ class Epp::Contact < Contact
     }
   end
 
-  def update_attributes(frame, current_user)
-    return super if frame.blank?
-    at = {}.with_indifferent_access
-    at.deep_merge!(self.class.attrs_from(frame.css('chg'), new_record: false))
-
-    if Setting.client_status_editing_enabled
-      at[:statuses] = statuses - statuses_attrs(frame.css('rem'), 'rem') + statuses_attrs(frame.css('add'), 'add')
-    end
-
-    if doc = attach_legal_document(::Deserializers::Xml::LegalDocument.new(frame).call)
-      frame.css("legalDocument").first.content = doc.path if doc&.persisted?
-      self.legal_document_id = doc.id
-    end
-
-    ident_frame = frame.css('ident').first
-
-    # https://github.com/internetee/registry/issues/576
-    if ident_frame
-      if identifier.valid?
-        submitted_ident = Ident.new(code: ident_frame.text,
-                                    type: ident_frame.attr('type'),
-                                    country_code: ident_frame.attr('cc'))
-
-        if submitted_ident != identifier
-          add_epp_error('2308', nil, nil, I18n.t('epp.contacts.errors.valid_ident'))
-          return
-        end
-      else
-        ident_update_attempt = ident_frame.text.present? && (ident_frame.text != ident)
-
-        if ident_update_attempt
-          add_epp_error('2308', nil, nil, I18n.t('epp.contacts.errors.ident_update'))
-          return
-        end
-
-        identifier = Ident.new(code: ident,
-                               type: ident_frame.attr('type'),
-                               country_code: ident_frame.attr('cc'))
-
-        identifier.validate
-
-        self.identifier = identifier
-        self.ident_updated_at ||= Time.zone.now
-      end
-    end
-
-    self.upid = current_user.registrar.id if current_user.registrar
-    self.up_date = Time.zone.now
-
-    self.attributes = at
-
-    email_changed = will_save_change_to_email?
-    old_email = email_was
-    updated = save
-
-    if updated && email_changed && registrant?
-      ContactMailer.email_changed(contact: self, old_email: old_email).deliver_now
-    end
-
-    updated
-  end
-
-  def statuses_attrs(frame, action)
-    status_list = status_list_from(frame)
-
-    if action == 'rem'
-      to_destroy = []
-      status_list.each do |status|
-        if statuses.include?(status)
-          to_destroy << status
-        else
-          add_epp_error('2303', 'status', status, [:contact_statuses, :not_found])
-        end
-      end
-
-      return to_destroy
-    else
-      return status_list
-    end
-  end
-
-  def status_list_from(frame)
-    status_list = []
-
-    frame.css('status').each do |status|
-      unless Contact::CLIENT_STATUSES.include?(status['s'])
-        add_epp_error('2303', 'status', status['s'], [:domain_statuses, :not_found])
-        next
-      end
-
-      status_list << status['s']
-    end
-
-    status_list
-  end
-
   def attach_legal_document(legal_document_data)
     return unless legal_document_data
 

lib/deserializers/xml/contact.rb

@@ -0,0 +1,61 @@
+module Deserializers
+  module Xml
+    class Contact
+      attr_reader :frame
+
+      def initialize(frame)
+        @frame = frame
+      end
+
+      def call
+        attributes = {
+          name: if_present('postalInfo name'),
+          org_name: if_present('postalInfo org'),
+          email: if_present('email'),
+          fax: if_present('fax'),
+          phone: if_present('voice'),
+
+          # Address fields
+          city: if_present('postalInfo addr city'),
+          zip: if_present('postalInfo addr pc'),
+          street: if_present('postalInfo addr street'),
+          state: if_present('postalInfo addr sp'),
+          country_code: if_present('postalInfo addr cc'),
+
+          # Auth info
+          auth_info: if_present('authInfo pw'),
+
+          # statuses
+          statuses_to_add: statuses_to_add,
+          statuses_to_remove: statuses_to_remove,
+        }
+
+        attributes.compact
+      end
+
+      def if_present(css_path)
+        return if frame.css(css_path).blank?
+
+        frame.css(css_path).text
+      end
+
+      def statuses_to_add
+        statuses_frame = frame.css('add')
+        return if statuses_frame.blank?
+
+        statuses_frame.css('status').map do |status|
+          status['s']
+        end
+      end
+
+      def statuses_to_remove
+        statuses_frame = frame.css('rem')
+        return if statuses_frame.blank?
+
+        statuses_frame.css('status').map do |status|
+          status['s']
+        end
+      end
+    end
+  end
+end

lib/deserializers/xml/contact_update.rb

@@ -0,0 +1,27 @@
+require 'deserializers/xml/legal_document'
+require 'deserializers/xml/ident'
+require 'deserializers/xml/contact'
+
+module Deserializers
+  module Xml
+    class ContactUpdate
+      attr_reader :frame
+
+      def initialize(frame)
+        @frame = frame
+      end
+
+      def contact
+        @contact ||= ::Deserializers::Xml::Contact.new(frame).call
+      end
+
+      def ident
+        @ident ||= ::Deserializers::Xml::Ident.new(frame).call
+      end
+
+      def legal_document
+        @legal_document ||= ::Deserializers::Xml::LegalDocument.new(frame).call
+      end
+    end
+  end
+end

test/lib/deserializers/xml/contact_test.rb

@@ -0,0 +1,114 @@
+require 'test_helper'
+require 'deserializers/xml/contact'
+
+class DeserializersXmlContactTest < ActiveSupport::TestCase
+  def test_trims_empty_values
+    xml_string = <<-XML
+    XML
+
+    nokogiri_frame = Nokogiri::XML(xml_string).remove_namespaces!
+    instance = ::Deserializers::Xml::Contact.new(nokogiri_frame)
+    assert_equal instance.call, {}
+  end
+
+  def test_handles_update
+    xml_string = <<-XML
+      <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+      <epp xmlns="https://epp.tld.ee/schema/epp-ee-1.0.xsd">
+        <command>
+          <update>
+            <contact:update xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">
+              <contact:id>john-001</contact:id>
+              <contact:chg>
+                <contact:postalInfo>
+                  <contact:name>new name</contact:name>
+                  <contact:org>Org</contact:org>
+                </contact:postalInfo>
+                <contact:voice>+123.4</contact:voice>
+                <contact:email>new-email@inbox.test</contact:email>
+              </contact:chg>
+            </contact:update>
+          </update>
+        </command>
+      </epp>
+    XML
+
+    nokogiri_frame = Nokogiri::XML(xml_string).remove_namespaces!
+    instance = ::Deserializers::Xml::Contact.new(nokogiri_frame)
+    assert_equal instance.call, { name: 'new name',
+                                  org_name: 'Org',
+                                  email: 'new-email@inbox.test',
+                                  phone: '+123.4' }
+  end
+
+  def test_handles_create
+    name = 'new'
+    email = 'new@registrar.test'
+    phone = '+1.2'
+
+    xml_string = <<-XML
+      <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+      <epp xmlns="https://epp.tld.ee/schema/epp-ee-1.0.xsd">
+        <command>
+          <create>
+            <contact:create xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">
+              <contact:postalInfo>
+                <contact:name>#{name}</contact:name>
+              </contact:postalInfo>
+              <contact:voice>#{phone}</contact:voice>
+              <contact:email>#{email}</contact:email>
+            </contact:create>
+          </create>
+          <extension>
+            <eis:extdata xmlns:eis="https://epp.tld.ee/schema/eis-1.0.xsd">
+              <eis:ident type="priv" cc="US">any</eis:ident>
+            </eis:extdata>
+          </extension>
+        </command>
+      </epp>
+    XML
+
+    nokogiri_frame = Nokogiri::XML(xml_string).remove_namespaces!
+    instance = ::Deserializers::Xml::Contact.new(nokogiri_frame)
+    assert_equal instance.call, { name: 'new', email: 'new@registrar.test', phone: '+1.2' }
+  end
+
+  def test_handles_statuses
+    xml_string = <<-XML
+      <?xml version="1.0" encoding="UTF-8" standalone="no"?>
+      <epp xmlns="https://epp.tld.ee/schema/epp-ee-1.0.xsd">
+        <command>
+          <update>
+            <contact:update xmlns:contact="https://epp.tld.ee/schema/contact-ee-1.1.xsd">
+              <contact:id>john-001</contact:id>
+              <contact:chg>
+                <contact:postalInfo>
+                  <contact:name>new name</contact:name>
+                </contact:postalInfo>
+                <contact:voice>+123.4</contact:voice>
+                <contact:email>new-email@inbox.test</contact:email>
+              </contact:chg>
+              <contact:add>
+                <contact:status s="clientDeleteProhibited" lang="en">Payment overdue.</contact:status>
+                <contact:status s="clientUpdateProhibited"/>
+              </contact:add>
+              <contact:rem>
+                <contact:status s="pendingDelete"/>
+              </contact:rem>
+            </contact:update>
+          </update>
+        </command>
+      </epp>
+    XML
+
+    nokogiri_frame = Nokogiri::XML(xml_string).remove_namespaces!
+    instance = ::Deserializers::Xml::Contact.new(nokogiri_frame)
+    assert_equal instance.call, { name: 'new name',
+                                  email: 'new-email@inbox.test',
+                                  phone: '+123.4',
+                                  statuses_to_add: ['clientDeleteProhibited',
+                                                    'clientUpdateProhibited'],
+                                  statuses_to_remove: ['pendingDelete']
+                                }
+  end
+end