From 043037225b9b282b25d1e35d57dca8667266e39e Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 5 Jun 2020 09:56:30 +0300 Subject: [PATCH 1/4] Extract legal doc parsing away from Epp::Domain class --- app/models/contact.rb | 4 +- app/models/epp/contact.rb | 6 +- app/models/epp/domain.rb | 26 +++----- lib/deserializers/xml/legal_document.rb | 22 +++++++ .../deserializers/xml/legal_document_test.rb | 62 +++++++++++++++++++ 5 files changed, 99 insertions(+), 21 deletions(-) create mode 100644 lib/deserializers/xml/legal_document.rb create mode 100644 test/lib/deserializers/xml/legal_document_test.rb diff --git a/app/models/contact.rb b/app/models/contact.rb index 2ee45e716..33cc3ff0f 100644 --- a/app/models/contact.rb +++ b/app/models/contact.rb @@ -1,3 +1,5 @@ +require 'deserializers/xml/legal_document' + class Contact < ApplicationRecord include Versions # version/contact_version.rb include EppErrors @@ -351,7 +353,7 @@ class Contact < ApplicationRecord return false end - legal_document_data = Epp::Domain.parse_legal_document_from_frame(frame) + legal_document_data = ::Deserializers::Xml::LegalDocument.new(frame).call if legal_document_data diff --git a/app/models/epp/contact.rb b/app/models/epp/contact.rb index fa488d9e1..10a74426a 100644 --- a/app/models/epp/contact.rb +++ b/app/models/epp/contact.rb @@ -1,3 +1,5 @@ +require 'deserializers/xml/legal_document' + class Epp::Contact < Contact include EppErrors @@ -140,7 +142,7 @@ class Epp::Contact < Contact at[:statuses] = statuses - statuses_attrs(frame.css('rem'), 'rem') + statuses_attrs(frame.css('add'), 'add') end - if doc = attach_legal_document(Epp::Domain.parse_legal_document_from_frame(frame)) + 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 @@ -237,7 +239,7 @@ class Epp::Contact < Contact end def add_legal_file_to_new frame - legal_document_data = Epp::Domain.parse_legal_document_from_frame(frame) + legal_document_data = ::Deserializers::Xml::LegalDocument.new(frame).call return unless legal_document_data doc = LegalDocument.create( diff --git a/app/models/epp/domain.rb b/app/models/epp/domain.rb index 6eab37079..8c3ffec48 100644 --- a/app/models/epp/domain.rb +++ b/app/models/epp/domain.rb @@ -1,3 +1,5 @@ +require 'deserializers/xml/legal_document' + class Epp::Domain < Domain include EppErrors @@ -180,7 +182,7 @@ class Epp::Domain < Domain # Adding legal doc to domain and # if something goes wrong - raise Rollback error def add_legal_file_to_new frame - legal_document_data = Epp::Domain.parse_legal_document_from_frame(frame) + legal_document_data = ::Deserializers::Xml::LegalDocument.new(frame).call return unless legal_document_data doc = LegalDocument.create( @@ -457,7 +459,7 @@ class Epp::Domain < Domain at.deep_merge!(attrs_from(frame.css('chg'), current_user, 'chg')) at.deep_merge!(attrs_from(frame.css('rem'), current_user, 'rem')) - if doc = attach_legal_document(Epp::Domain.parse_legal_document_from_frame(frame)) + 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 @@ -554,7 +556,7 @@ class Epp::Domain < Domain return end - if doc = attach_legal_document(Epp::Domain.parse_legal_document_from_frame(frame)) + if doc = attach_legal_document(::Deserializers::Xml::LegalDocument.new(frame).call) frame.css("legalDocument").first.content = doc.path if doc&.persisted? end @@ -665,7 +667,7 @@ class Epp::Domain < Domain self.registrar = current_user.registrar end - attach_legal_document(self.class.parse_legal_document_from_frame(frame)) + attach_legal_document(::Deserializers::Xml::LegalDocument.new(frame).call) save!(validate: false) return dt @@ -690,7 +692,7 @@ class Epp::Domain < Domain regenerate_transfer_code self.registrar = pt.new_registrar - attach_legal_document(self.class.parse_legal_document_from_frame(frame)) + attach_legal_document(::Deserializers::Xml::LegalDocument.new(frame).call) save!(validate: false) end @@ -710,7 +712,7 @@ class Epp::Domain < Domain status: DomainTransfer::CLIENT_REJECTED ) - attach_legal_document(self.class.parse_legal_document_from_frame(frame)) + attach_legal_document(::Deserializers::Xml::LegalDocument.new(frame).call) save!(validate: false) end @@ -759,18 +761,6 @@ class Epp::Domain < Domain p[:unit] end - def parse_legal_document_from_frame(parsed_frame) - ld = parsed_frame.css('legalDocument').first - return nil unless ld - return nil if ld.text.starts_with?(ENV['legal_documents_dir']) # escape reloading - return nil if ld.text.starts_with?('/home/') # escape reloading - - { - body: ld.text, - type: ld['type'] - } - end - def check_availability(domain_names) domain_names = [domain_names] if domain_names.is_a?(String) diff --git a/lib/deserializers/xml/legal_document.rb b/lib/deserializers/xml/legal_document.rb new file mode 100644 index 000000000..53d162bd1 --- /dev/null +++ b/lib/deserializers/xml/legal_document.rb @@ -0,0 +1,22 @@ +module Deserializers + module Xml + # Given a nokogiri frame, extract information about legal document from it. + class LegalDocument + attr_reader :frame + + def initialize(frame) + @frame = frame + end + + def call + ld = frame.css('legalDocument').first + return unless ld + + { + body: ld.text, + type: ld['type'] + } + end + end + end +end diff --git a/test/lib/deserializers/xml/legal_document_test.rb b/test/lib/deserializers/xml/legal_document_test.rb new file mode 100644 index 000000000..ac173ba4b --- /dev/null +++ b/test/lib/deserializers/xml/legal_document_test.rb @@ -0,0 +1,62 @@ +require 'test_helper' +require 'deserializers/xml/legal_document' + +class DeserializersXmlLegalDocumentTest < ActiveSupport::TestCase + def test_returns_nil_when_required_fields_not_present + xml_string = <<-XML + + + + + + john-001 + + + new name + + +123.4 + new-email@inbox.test + + + + + + XML + + nokogiri_frame = Nokogiri::XML(xml_string).remove_namespaces! + instance = ::Deserializers::Xml::LegalDocument.new(nokogiri_frame) + + assert_nil instance.call + end + + def test_returns_hash_when_document_exists + xml_string = <<-XML + + + + + + FIRST0:SH2027223711 + + wrong password + + + + + + 37605030299 + dGVzdCBmYWlsCg== + + + ABC-12345 + + + XML + + nokogiri_frame = Nokogiri::XML(xml_string).remove_namespaces! + instance = ::Deserializers::Xml::LegalDocument.new(nokogiri_frame) + expected_result = { body: "dGVzdCBmYWlsCg==", type: "pdf" } + + assert_equal expected_result, instance.call + end +end From bacdebd17c435723ea1802a245aacf55364b389d Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 5 Jun 2020 10:31:51 +0300 Subject: [PATCH 2/4] Pull out ident parsing from Epp::Contact class --- app/models/epp/contact.rb | 24 +----- lib/deserializers/xml/ident.rb | 34 +++++++++ test/lib/deserializers/xml/ident_test.rb | 95 ++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 21 deletions(-) create mode 100644 lib/deserializers/xml/ident.rb create mode 100644 test/lib/deserializers/xml/ident_test.rb diff --git a/app/models/epp/contact.rb b/app/models/epp/contact.rb index 10a74426a..398c5715f 100644 --- a/app/models/epp/contact.rb +++ b/app/models/epp/contact.rb @@ -1,4 +1,5 @@ require 'deserializers/xml/legal_document' +require 'deserializers/xml/ident' class Epp::Contact < Contact include EppErrors @@ -40,8 +41,8 @@ class Epp::Contact < Contact at[:auth_info] = f.css('authInfo pw').text if f.css('authInfo pw').present? - - at.merge!(ident_attrs(f.css('ident').first)) if new_record + ident_attrs = ::Deserializers::Xml::Ident.new(f).call + at.merge!(ident_attrs) if new_record at end @@ -56,25 +57,6 @@ class Epp::Contact < Contact ) end - def ident_attrs(ident_frame) - return {} unless ident_attr_valid?(ident_frame) - - { - ident: ident_frame.text, - ident_type: ident_frame.attr('type'), - ident_country_code: ident_frame.attr('cc') - } - end - - def ident_attr_valid?(ident_frame) - return false if ident_frame.blank? - return false if ident_frame.try('text').blank? - return false if ident_frame.attr('type').blank? - return false if ident_frame.attr('cc').blank? - - true - end - def legal_document_attrs(legal_frame) return [] if legal_frame.blank? return [] if legal_frame.try('text').blank? diff --git a/lib/deserializers/xml/ident.rb b/lib/deserializers/xml/ident.rb new file mode 100644 index 000000000..966b3727f --- /dev/null +++ b/lib/deserializers/xml/ident.rb @@ -0,0 +1,34 @@ +module Deserializers + module Xml + class Ident + attr_reader :frame + + def initialize(frame) + @frame = frame.css('ident').first + end + + def call + if valid? + { + ident: frame.text, + ident_type: frame.attr('type'), + ident_country_code: frame.attr('cc') + } + else + {} + end + end + + private + + def valid? + return false if frame.blank? + return false if frame.try('text').blank? + return false if frame.attr('type').blank? + return false if frame.attr('cc').blank? + + true + end + end + end +end diff --git a/test/lib/deserializers/xml/ident_test.rb b/test/lib/deserializers/xml/ident_test.rb new file mode 100644 index 000000000..beff7aef5 --- /dev/null +++ b/test/lib/deserializers/xml/ident_test.rb @@ -0,0 +1,95 @@ +require 'test_helper' +require 'deserializers/xml/ident' + +class DeserializersXmlIdentTest < ActiveSupport::TestCase + def test_returns_empty_hash_when_not_present + xml_string = <<-XML + + + + + + john-001 + + + new name + + +123.4 + new-email@inbox.test + + + + + + XML + + nokogiri_frame = Nokogiri::XML(xml_string).remove_namespaces! + instance = ::Deserializers::Xml::Ident.new(nokogiri_frame) + assert_equal instance.call, {} + end + + def test_returns_empty_hash_when_not_valid + xml_string = <<-XML + + + + + + FIRST0:SH2027223711 + + wrong password + + + + + + 37605030299 + dGVzdCBmYWlsCg== + + + ABC-12345 + + + XML + + nokogiri_frame = Nokogiri::XML(xml_string).remove_namespaces! + instance = ::Deserializers::Xml::Ident.new(nokogiri_frame) + assert_equal instance.call, {} + end + + def test_returns_complete_hash_when_valid + xml_string = <<-XML + + + + + + FIRST0:SH2027223711 + + wrong password + + + + + + 37605030299 + dGVzdCBmYWlsCg== + + + ABC-12345 + + + XML + + nokogiri_frame = Nokogiri::XML(xml_string).remove_namespaces! + instance = ::Deserializers::Xml::Ident.new(nokogiri_frame) + + expected_result = { + ident: '37605030299', + ident_type: 'priv', + ident_country_code: 'EE' + } + + assert_equal instance.call, expected_result + end +end From 94078fe71eaae68c4c21ce9e2c6b129b75631510 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 5 Jun 2020 10:33:00 +0300 Subject: [PATCH 3/4] Remove dead code --- app/models/epp/contact.rb | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/app/models/epp/contact.rb b/app/models/epp/contact.rb index 398c5715f..d14bf1e1b 100644 --- a/app/models/epp/contact.rb +++ b/app/models/epp/contact.rb @@ -57,17 +57,6 @@ class Epp::Contact < Contact ) end - def legal_document_attrs(legal_frame) - return [] if legal_frame.blank? - return [] if legal_frame.try('text').blank? - return [] if legal_frame.attr('type').blank? - - [{ - body: legal_frame.text, - document_type: legal_frame.attr('type') - }] - end - def check_availability(codes) codes = [codes] if codes.is_a?(String) From 1666ef288a9ecc8ffd0edcbfa60abf6a40ab8a77 Mon Sep 17 00:00:00 2001 From: Maciej Szlosarczyk Date: Fri, 5 Jun 2020 10:44:58 +0300 Subject: [PATCH 4/4] Make rubocop more happy --- lib/deserializers/xml/ident.rb | 2 +- lib/deserializers/xml/legal_document.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/deserializers/xml/ident.rb b/lib/deserializers/xml/ident.rb index 966b3727f..0dc65cbb6 100644 --- a/lib/deserializers/xml/ident.rb +++ b/lib/deserializers/xml/ident.rb @@ -12,7 +12,7 @@ module Deserializers { ident: frame.text, ident_type: frame.attr('type'), - ident_country_code: frame.attr('cc') + ident_country_code: frame.attr('cc'), } else {} diff --git a/lib/deserializers/xml/legal_document.rb b/lib/deserializers/xml/legal_document.rb index 53d162bd1..b75267f2d 100644 --- a/lib/deserializers/xml/legal_document.rb +++ b/lib/deserializers/xml/legal_document.rb @@ -14,7 +14,7 @@ module Deserializers { body: ld.text, - type: ld['type'] + type: ld['type'], } end end