Story #107192666 - refactor, add support for remove all, bug fix overly visible nodes

app/models/epp/domain.rb

@@ -316,49 +316,26 @@ class Epp::Domain < Domain
   def dnskeys_attrs(frame, action)
     keys = []
     return keys if frame.blank?
-
-    if frame.xpath('dnsSec:all').present?
-      # support delete all or all of
-    else
     inf_data = DnsSecKeys.new(frame)
-      if Setting.key_data_allowed
-        if inf_data.ds_data.present?
-          errors.add(:base, :ds_data_not_allowed)
-          return
+
+    if  action == 'rem' &&
+        frame.css('rem > all').first.try(:text) == 'true'
+      keys = inf_data.mark_destroy_all dnskeys
     else
+      if Setting.key_data_allowed
+        errors.add(:base, :ds_data_not_allowed) if inf_data.ds_data.present?
         keys = inf_data.key_data
       end
-      end
       if Setting.ds_data_allowed
-        if inf_data.key_data.present?
-          errors.add(:base, :key_data_not_allowed)
-          return
-        else
+        errors.add(:base, :key_data_not_allowed) if inf_data.key_data.present?
         keys = inf_data.ds_data
       end
-      end
-    end
-
       if action == 'rem'
-      to_destroy = []
-      keys.each do |x|
-        dk = dnskeys.find_by(public_key: x[:public_key])
-
-        unless dk
-          add_epp_error('2303', 'publicKey', x[:public_key], [:dnskeys, :not_found])
-          next
+        keys = inf_data.mark_destroy(dnskeys)
+        add_epp_error('2303', nil, nil, [:dnskeys, :not_found]) if keys.include? nil
       end
-
-        to_destroy << {
-          id: dk.id,
-          _destroy: 1
-        }
-      end
-
-      return to_destroy
-    else
-      return keys
     end
+    errors.any? ? [] : keys
   end
   # rubocop: enable Metrics/PerceivedComplexity
   # rubocop: enable Metrics/CyclomaticComplexity
@@ -367,19 +344,28 @@ class Epp::Domain < Domain
     def initialize(frame)
       @key_data = []
       @ds_data = []
+      # schema validation prevents both in the same parent node
       if frame.css('dsData').present?
         ds_data_from frame
-      end
+      else
         frame.css('keyData').each do |key|
           @key_data.append key_data_from(key)
         end
       end
+    end
 
     attr_reader :key_data
     attr_reader :ds_data
 
-    def error
-      ds_data.present? && key_data.present?
+    def mark_destroy_all(dns_keys)
+      # if transition support required mark_destroy dns_keys when has ds/key values otherwise ...
+      dns_keys.map { |inf_data| mark inf_data }
+    end
+
+    def mark_destroy(dns_keys)
+      (ds_data.present? ? ds_filter(dns_keys) : kd_filter(dns_keys)).map do |inf_data|
+        inf_data.blank? ? nil : mark(inf_data)
+      end
     end
 
     private
@@ -395,7 +381,6 @@ class Epp::Domain < Domain
     def xm_copy(frame, map)
       result = {}
       map.each do |key, elem|
-        # content validation might happen later in Dnskey, if we get that far; or not. TODO: check handling
         result[key] = frame.css(elem).first.try(:text)
       end
       result
@@ -405,8 +390,8 @@ class Epp::Domain < Domain
       result = xm_copy frame, KEY_INTERFACE
       # TODO: can these defaults go where they belong?
       result.merge({
-                       ds_alg: 3,
-                       ds_digest_type: Setting.ds_algorithm
+                       ds_alg: 3,      # DSA/SHA-1 [DSA] RFC2536
+                       ds_digest_type: Setting.ds_algorithm     # only 1
                    })
     end
 
@@ -414,12 +399,27 @@ class Epp::Domain < Domain
       frame.css('dsData').each do |ds_data|
         key = ds_data.css('keyData')
         ds = xm_copy ds_data, DS_INTERFACE
-        ds.merge! (key_data_from key) if key.present?
+        ds.merge(key_data_from key) if key.present?
         @ds_data << ds
       end
     end
+
+    def ds_filter(dns_keys)
+      @ds_data.map do |ds|
+        dns_keys.find_by(ds.slice(*DS_INTERFACE.keys))
+      end
      end
 
+    def kd_filter(dns_keys)
+      @key_data.map do |key|
+        dns_keys.find_by(key)
+      end
+    end
+
+    def mark(inf_data)
+      { id: inf_data.id, _destroy: 1 }
+    end
+  end
   
   def domain_statuses_attrs(frame, action)
     status_list = domain_status_list_from(frame)