From b26345dde4454995e7aff93d21f13405d81fbf28 Mon Sep 17 00:00:00 2001 From: Martin Lensment Date: Mon, 11 Aug 2014 16:41:31 +0300 Subject: [PATCH] Domain creating refactor --- app/controllers/concerns/epp/common.rb | 7 +- app/helpers/epp/domains_helper.rb | 81 +++++++-------------- app/models/domain.rb | 86 ++++++++++++++++------- app/models/domain_contact.rb | 2 + spec/fabricators/contact_fabricator.rb | 4 +- spec/fabricators/domain_fabricator.rb | 2 + spec/fabricators/nameserver_fabricator.rb | 3 + spec/models/domain_spec.rb | 4 +- 8 files changed, 103 insertions(+), 86 deletions(-) create mode 100644 spec/fabricators/nameserver_fabricator.rb diff --git a/app/controllers/concerns/epp/common.rb b/app/controllers/concerns/epp/common.rb index af2f10fc3..0fef45510 100644 --- a/app/controllers/concerns/epp/common.rb +++ b/app/controllers/concerns/epp/common.rb @@ -35,11 +35,16 @@ module Epp::Common def handle_errors(obj=nil) if obj obj.construct_epp_errors - @errors = obj.errors[:epp_errors] + @errors += obj.errors[:epp_errors] end render '/epp/error' end + def append_errors(obj) + obj.construct_epp_errors + @errors += obj.errors[:epp_errors] + end + def xml_attrs_present?(ph, attributes) attributes.each do |x| epp_errors << {code: '2003', msg: I18n.t('errors.messages.required_parameter_missing', key: x.last)} unless has_attribute(ph, x) diff --git a/app/helpers/epp/domains_helper.rb b/app/helpers/epp/domains_helper.rb index cfd9a468d..6e32660b7 100644 --- a/app/helpers/epp/domains_helper.rb +++ b/app/helpers/epp/domains_helper.rb @@ -1,27 +1,12 @@ module Epp::DomainsHelper def create_domain - ph = params_hash['epp']['command']['create']['create'] - - unless xml_attrs_present?(ph, [['name'], ['ns'], ['authInfo'], ['contact'], ['registrant']]) - render '/epp/error' and return - end - - @domain = Domain.new(domain_create_params(ph)) - - if owner_contact_id = Contact.find_by(code: ph[:registrant]).try(:id) - @domain.owner_contact_id = owner_contact_id - else - epp_errors << {code: '2303', msg: I18n.t('errors.messages.epp_registrant_not_found'), value: {obj: 'registrant', val: ph[:registrant]}} - render '/epp/error' and return - end - Domain.transaction do - if @domain.save && @domain.attach_contacts(domain_contacts) && @domain.attach_nameservers(domain_nameservers) - render '/epp/domains/create' - else - handle_errors(@domain) - raise ActiveRecord::Rollback - end + @domain = Domain.new(domain_create_params) + + handle_errors(@domain) and return unless @domain.attach_objects(@ph, params[:frame]) + handle_errors(@domain) and return unless @domain.save + + render '/epp/domains/create' end end @@ -44,11 +29,31 @@ module Epp::DomainsHelper ### HELPER METHODS ### private + ## CREATE + def validate_domain_create_request + @ph = params_hash['epp']['command']['create']['create'] + xml_attrs_present?(@ph, [['name'], ['ns'], ['authInfo'], ['contact'], ['registrant']]) + end + + def domain_create_params + { + name: @ph[:name], + registrar_id: current_epp_user.registrar.try(:id), + registered_at: Time.now, + period: @ph[:period].to_i, + valid_from: Date.today, + valid_to: Date.today + @ph[:period].to_i.years, + auth_info: @ph[:authInfo][:pw] + } + end + + ## RENEW def validate_domain_renew_request @ph = params_hash['epp']['command']['renew']['renew'] xml_attrs_present?(@ph, [['name'], ['curExpDate'], ['period']]) end + ## SHARED def find_domain domain = Domain.find_by(name: @ph[:name]) unless domain @@ -56,38 +61,4 @@ module Epp::DomainsHelper end domain end - - def domain_create_params(ph) - { - name: ph[:name], - registrar_id: current_epp_user.registrar.try(:id), - registered_at: Time.now, - period: ph[:period].to_i, - valid_from: Date.today, - valid_to: Date.today + ph[:period].to_i.years, - auth_info: ph[:authInfo][:pw] - } - end - - def domain_contacts - parsed_frame = Nokogiri::XML(params[:frame]).remove_namespaces! - - res = {} - Contact::CONTACT_TYPES.each do |ct| - res[ct.to_sym] ||= [] - parsed_frame.css("contact[type='#{ct}']").each do |x| - res[ct.to_sym] << Hash.from_xml(x.to_s).with_indifferent_access - end - end - - res - end - - def domain_nameservers - ph = params_hash['epp']['command']['create']['create']['ns'] - return [] unless ph - return ph[:hostObj] if ph[:hostObj] - return ph[:hostAttr] if ph[:hostAttr] - [] - end end diff --git a/app/models/domain.rb b/app/models/domain.rb index ab52e62dd..7a3eb7086 100644 --- a/app/models/domain.rb +++ b/app/models/domain.rb @@ -34,9 +34,13 @@ class Domain < ActiveRecord::Base validates :name_dirty, domain_name: true, uniqueness: true validates :period, numericality: { only_integer: true, greater_than: 0, less_than: 100 } validates :name, :owner_contact, presence: true - validate :validate_period + validates_associated :nameservers + validate :validate_period + validate :validate_nameservers_count + validate :validate_admin_contacts_count + def name=(value) value.strip! write_attribute(:name, SimpleIDN.to_unicode(value)) @@ -44,18 +48,37 @@ class Domain < ActiveRecord::Base write_attribute(:name_dirty, value) end + def attach_objects(ph, frame) + attach_owner_contact(ph[:registrant]) + attach_contacts(self.class.parse_contacts_from_frame(frame)) + attach_nameservers(self.class.parse_nameservers_from_params(ph[:ns])) + + errors.empty? + end + + def attach_owner_contact(code) + self.owner_contact = Contact.find_by(code: code) + + errors.add(:owner_contact, { + obj: 'registrant', + val: code, + msg: I18n.t('errors.messages.epp_registrant_not_found') + }) unless owner_contact + end + def attach_contacts(contacts) contacts.each do |k, v| v.each do |x| - contact = Contact.find_by(code: x[:contact]) - attach_contact(k, contact) and next if contact - - # Detailed error message with value to display in EPP response - errors.add(:domain_contacts, { - obj: 'contact', - val: x[:contact], - msg: errors.generate_message(:domain_contacts, :not_found) - }) + if contact = Contact.find_by(code: x[:contact]) + attach_contact(k, contact) + else + # Detailed error message with value to display in EPP response + errors.add(:domain_contacts, { + obj: 'contact', + val: x[:contact], + msg: errors.generate_message(:domain_contacts, :not_found) + }) + end end end @@ -64,29 +87,17 @@ class Domain < ActiveRecord::Base if owner_contact.citizen? attach_contact(Contact::CONTACT_TYPE_ADMIN, owner_contact) if admin_contacts.empty? end - - validate_admin_contacts_count - - errors.empty? end def attach_contact(type, contact) - domain_contacts.create( - contact: contact, - contact_type: type - ) + tech_contacts << contact if type.to_sym == :tech + admin_contacts << contact if type.to_sym == :admin end def attach_nameservers(ns_list) ns_list.each do |ns| attach_nameserver(ns) end - - save - - validate_nameservers_count - - errors.empty? end def attach_nameserver(ns) @@ -107,12 +118,12 @@ class Domain < ActiveRecord::Base self.nameservers.build(attrs) end + ### VALIDATIONS ### + def validate_nameservers_count - unless nameservers.count.between?(1, 13) + unless nameservers.length.between?(1, 13) errors.add(:nameservers, :out_of_range, {min: 1, max: 13}) end - - errors.empty? end def validate_admin_contacts_count @@ -156,6 +167,27 @@ class Domain < ActiveRecord::Base end class << self + def parse_contacts_from_frame(frame) + parsed_frame = Nokogiri::XML(frame).remove_namespaces! + + res = {} + Contact::CONTACT_TYPES.each do |ct| + res[ct.to_sym] ||= [] + parsed_frame.css("contact[type='#{ct}']").each do |x| + res[ct.to_sym] << Hash.from_xml(x.to_s).with_indifferent_access + end + end + + res + end + + def parse_nameservers_from_params(ph) + return [] unless ph + return ph[:hostObj] if ph[:hostObj] + return ph[:hostAttr] if ph[:hostAttr] + [] + end + def check_availability(domains) domains = [domains] if domains.is_a?(String) diff --git a/app/models/domain_contact.rb b/app/models/domain_contact.rb index 6d21a6843..5389595b4 100644 --- a/app/models/domain_contact.rb +++ b/app/models/domain_contact.rb @@ -1,4 +1,6 @@ class DomainContact < ActiveRecord::Base belongs_to :contact belongs_to :domain + + scope :tech, -> {where(contact_type: :tech)} end diff --git a/spec/fabricators/contact_fabricator.rb b/spec/fabricators/contact_fabricator.rb index 6d0e5a46a..775d7e510 100644 --- a/spec/fabricators/contact_fabricator.rb +++ b/spec/fabricators/contact_fabricator.rb @@ -2,8 +2,8 @@ Fabricator(:contact) do name Faker::Name.name phone '+372.12345678' email Faker::Internet.email - ident '37605030299' - code "sh#{Faker::Number.number(4)}" + ident '37605030299' + code { "sh#{Faker::Number.number(4)}" } ident_type 'op' address end diff --git a/spec/fabricators/domain_fabricator.rb b/spec/fabricators/domain_fabricator.rb index 24113039d..da7e755ee 100644 --- a/spec/fabricators/domain_fabricator.rb +++ b/spec/fabricators/domain_fabricator.rb @@ -4,4 +4,6 @@ Fabricator(:domain) do period 1 period_unit 'y' owner_contact(fabricator: :contact) + nameservers(count: 3) + admin_contacts(count: 1) { Fabricate(:contact) } end diff --git a/spec/fabricators/nameserver_fabricator.rb b/spec/fabricators/nameserver_fabricator.rb new file mode 100644 index 000000000..5008ac422 --- /dev/null +++ b/spec/fabricators/nameserver_fabricator.rb @@ -0,0 +1,3 @@ +Fabricator(:nameserver) do + hostname { "ns.#{Faker::Internet.domain_word}.ee" } +end diff --git a/spec/models/domain_spec.rb b/spec/models/domain_spec.rb index 2c0ffa78e..fd2278fa9 100644 --- a/spec/models/domain_spec.rb +++ b/spec/models/domain_spec.rb @@ -41,7 +41,9 @@ describe Domain do expect(d.errors.messages).to match_array({ name: ['is missing'], period: ['is not a number'], - owner_contact: ["Registrant is missing"] + owner_contact: ["Registrant is missing"], + admin_contacts: ["Admin contact is missing"], + nameservers: ["Nameservers count must be between 1-13"] }) end