do some refactorings

- use more "tap"
- fix rubocop issues

diaspora_federation.gemspec

@@ -16,7 +16,8 @@ Gem::Specification.new do |s|
                   "among the various installations of Diaspora*"
   s.license     = "AGPL 3.0 - http://www.gnu.org/licenses/agpl-3.0.html"
 
-  s.files       = Dir["lib/**/*", "LICENSE", "README.md"] - Dir["lib/diaspora_federation/{engine,rails,test}.rb", "lib/diaspora_federation/test/*"]
+  s.files       = Dir["lib/**/*", "LICENSE", "README.md"] -
+                    Dir["lib/diaspora_federation/{engine,rails,test}.rb", "lib/diaspora_federation/test/*"]
 
   s.required_ruby_version = "~> 2.0"
 

lib/diaspora_federation/entities/relayable.rb

@@ -33,19 +33,22 @@ module DiasporaFederation
 
       # Generates XML and updates signatures
       def to_xml
-        xml = entity_xml
+        entity_xml.tap do |xml|
-        hash = to_h
+          hash = to_h
-        Relayable.update_signatures!(hash)
+          Relayable.update_signatures!(hash)
 
-        xml.at_xpath("author_signature").content = hash[:author_signature]
+          xml.at_xpath("author_signature").content = hash[:author_signature]
-        xml.at_xpath("parent_author_signature").content = hash[:parent_author_signature]
+          xml.at_xpath("parent_author_signature").content = hash[:parent_author_signature]
-        xml
+        end
       end
 
       # Exception raised when verify_signatures fails to verify signatures (signatures are wrong)
       class SignatureVerificationFailed < ArgumentError
       end
 
+      # verifies the signatures (+author_signature+ and +parent_author_signature+ if needed)
+      # @param [Hash] data hash with data to verify
+      # @raise [SignatureVerificationFailed] if the signature is not valid or no public key is found
       def self.verify_signatures(data)
         pkey = DiasporaFederation.callbacks.trigger(:fetch_public_key_by_id, data[:diaspora_id])
         raise SignatureVerificationFailed, "failed to fetch public key for #{data[:diaspora_id]}" if pkey.nil?
@@ -53,22 +56,26 @@ module DiasporaFederation
           data, data[:author_signature], pkey
         )
 
-        unless DiasporaFederation.callbacks.trigger(:post_author_is_local?, data[:parent_guid])
+        author_is_local = DiasporaFederation.callbacks.trigger(:post_author_is_local?, data[:parent_guid])
-          # this happens only on downstream federation
+        verify_parent_signature(data) unless author_is_local
-          pkey = DiasporaFederation.callbacks.trigger(:fetch_public_key_by_post_guid, data[:parent_guid])
-          raise SignatureVerificationFailed,
-                "failed to fetch public key for parent of #{data[:parent_guid]}" if pkey.nil?
-          raise SignatureVerificationFailed, "wrong parent_author_signature" unless Signing.verify_signature(
-            data, data[:parent_author_signature], pkey
-          )
-        end
       end
 
+      # this happens only on downstream federation
+      # @param [Hash] data hash with data to verify
+      def self.verify_parent_signature(data)
+        pkey = DiasporaFederation.callbacks.trigger(:fetch_public_key_by_post_guid, data[:parent_guid])
+        raise SignatureVerificationFailed,
+              "failed to fetch public key for parent of #{data[:parent_guid]}" if pkey.nil?
+        raise SignatureVerificationFailed, "wrong parent_author_signature" unless Signing.verify_signature(
+          data, data[:parent_author_signature], pkey
+        )
+      end
+      private_class_method :verify_parent_signature
+
       # Adds signatures to a given hash with the keys of the author and the parent
       # if the signatures are not in the hash yet and if the keys are available.
       #
       # @param [Hash] data hash given for a signing
-      # @return [Hash] reference to the input hash
       def self.update_signatures!(data)
         if data[:author_signature].nil?
           pkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_id, data[:diaspora_id])
@@ -79,8 +86,6 @@ module DiasporaFederation
           pkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_post_guid, data[:parent_guid])
           data[:parent_author_signature] = Signing.sign_with_key(data, pkey) unless pkey.nil?
         end
-
-        data
       end
     end
   end

lib/diaspora_federation/salmon/xml_payload.rb

@@ -85,25 +85,15 @@ module DiasporaFederation
       # @param [Nokogiri::XML::Element] node xml nodes
       # @return [Entity] instance
       def self.populate_entity(klass, node)
-        # Build a hash of attributes basing on XML tree. If elements are known in "props" they respect the Entity logic.
+        # Use all known properties to build the Entity. All other elements are respected
-        # All other elemnts are respected and attached to resulted hash as string.
+        # and attached to resulted hash as string. It is intended to build a hash
-        # It is intended to build a hash invariable of an Entity definition, in order to support receiving objects
+        # invariable of an Entity definition, in order to support receiving objects
         # from the future versions of Diaspora, where new elements may have been added.
-        xml_names = klass.class_props.map {|prop_def| prop_def[:xml_name].to_s }
-
         data = node.element_children.map { |child|
           xml_name = child.name
-          if xml_names.include?(xml_name)
+          property = klass.class_props.find {|prop| prop[:xml_name].to_s == xml_name }
-            prop = klass.class_props.find {|prop| prop[:xml_name].to_s == xml_name }
+          if property
-            type = prop[:type]
+            parse_element_from_node(property[:name], property[:type], xml_name, node)
-
-            if type == String
-              [prop[:name], parse_string_from_node(xml_name, node)]
-            elsif type.instance_of?(Array)
-              [prop[:name], parse_array_from_node(type, node)]
-            elsif type.ancestors.include?(Entity)
-              [prop[:name], parse_entity_from_node(type, node)]
-            end
           else
             [xml_name, child.text]
           end
@@ -115,6 +105,17 @@ module DiasporaFederation
       end
       private_class_method :populate_entity
 
+      def self.parse_element_from_node(name, type, xml_name, node)
+        if type == String
+          [name, parse_string_from_node(xml_name, node)]
+        elsif type.instance_of?(Array)
+          [name, parse_array_from_node(type, node)]
+        elsif type.ancestors.include?(Entity)
+          [name, parse_entity_from_node(type, node)]
+        end
+      end
+      private_class_method :parse_element_from_node
+
       # create simple entry in data hash
       # @return [String] data
       def self.parse_string_from_node(name, node)

lib/diaspora_federation/test.rb

@@ -18,12 +18,13 @@ module DiasporaFederation
 
     # Generates attributes for entity constructor with correct signatures in it
     #
-    # @param [Symbol] entity_type type to generate attributes for
+    # @param [Symbol] factory_name the factory to generate attributes for (normally entity name)
     # @return [Hash] hash with correct signatures
-    def self.relayable_attributes_with_signatures(entity_type)
+    def self.relayable_attributes_with_signatures(factory_name)
-      DiasporaFederation::Entities::Relayable.update_signatures!(
+      klass = FactoryGirl.factory_by_name(factory_name).build_class
-        sort_hash(FactoryGirl.attributes_for(entity_type), FactoryGirl.factory_by_name(entity_type).build_class)
+      sort_hash(FactoryGirl.attributes_for(factory_name), klass).tap do |data|
-      )
+        DiasporaFederation::Entities::Relayable.update_signatures!(data)
+      end
     end
   end
 end