xml_order and additional_xml_elements only for Relayables

lib/diaspora_federation/entities/relayable.rb

@@ -10,6 +10,14 @@ module DiasporaFederation
       # digest instance used for signing
       DIGEST = OpenSSL::Digest::SHA256.new
 
+      # order from the parsed xml for signature
+      # @return [Array] order from xml
+      attr_reader :xml_order
+
+      # additional properties from parsed xml
+      # @return [Hash] additional xml elements
+      attr_reader :additional_xml_elements
+
       # on inclusion of this module the required properties for a relayable are added to the object that includes it
       #
       # @!attribute [r] author
@@ -49,6 +57,21 @@ module DiasporaFederation
           property :author_signature, default: nil
           property :parent_author_signature, default: nil
         end
+
+        entity.extend ParseXML
+      end
+
+      # Initializes a new relayable Entity with order and additional xml elements
+      #
+      # @param [Hash] data entity data
+      # @param [Array] xml_order order from xml
+      # @param [Hash] additional_xml_elements additional xml elements
+      # @see DiasporaFederation::Entity#initialize
+      def initialize(data, xml_order=nil, additional_xml_elements={})
+        @xml_order = xml_order
+        @additional_xml_elements = additional_xml_elements
+
+        super(data)
       end
 
       # verifies the signatures (+author_signature+ and +parent_author_signature+ if needed)
@@ -147,6 +170,37 @@ module DiasporaFederation
         signature_order.map {|name| data[name] }.join(";")
       end
 
+      # override class methods from {Entity} to parse the xml
+      module ParseXML
+        private
+
+        # @param [Nokogiri::XML::Element] root_node xml nodes
+        # @return [Entity] instance
+        def populate_entity(root_node)
+          # Use all known properties to build the Entity (entity_data). All additional xml elements
+          # are respected and attached to a hash as string (additional_xml_elements). It also remembers
+          # the order of the xml-nodes (xml_order). This is needed to support receiving objects from
+          # the future versions of Diaspora, where new elements may have been added.
+          entity_data = {}
+          additional_xml_elements = {}
+
+          xml_order = root_node.element_children.map do |child|
+            xml_name = child.name
+            property = find_property_for_xml_name(xml_name)
+
+            if property
+              entity_data[property] = parse_element_from_node(xml_name, class_props[property], root_node)
+              property
+            else
+              additional_xml_elements[xml_name] = child.text
+              xml_name
+            end
+          end
+
+          new(entity_data, xml_order, additional_xml_elements).tap(&:verify_signatures)
+        end
+      end
+
       # Exception raised when creating the author_signature failes, because the private key was not found
       class AuthorPrivateKeyNotFound < RuntimeError
       end

lib/diaspora_federation/entity.rb

@@ -35,12 +35,6 @@ module DiasporaFederation
   class Entity
     extend PropertiesDSL
 
-    attr_reader :xml_order
-
-    # additional properties from parsed xml
-    # @return [Hash] additional xml elements
-    attr_reader :additional_xml_elements
-
     # Initializes the Entity with the given attribute hash and freezes the created
     # instance it returns.
     #
@@ -54,9 +48,8 @@ module DiasporaFederation
     #       {PropertiesDSL#entity}) get discarded silently.
     #
     # @param [Hash] data entity data
-    # @param [Hash] additional_xml_elements additional xml elements
     # @return [Entity] new instance
-    def initialize(data, xml_order=nil, additional_xml_elements={})
+    def initialize(data)
       raise ArgumentError, "expected a Hash" unless data.is_a?(Hash)
       entity_data = self.class.resolv_aliases(data)
       missing_props = self.class.missing_props(entity_data)
@@ -64,9 +57,6 @@ module DiasporaFederation
         raise ArgumentError, "missing required properties: #{missing_props.join(', ')}"
       end
 
-      @xml_order = xml_order
-      @additional_xml_elements = additional_xml_elements
-
       self.class.default_values.merge(entity_data).each do |name, value|
         instance_variable_set("@#{name}", nilify(value)) if setable?(name, value)
       end
@@ -209,44 +199,25 @@ module DiasporaFederation
     # @param [Nokogiri::XML::Element] root_node xml nodes
     # @return [Entity] instance
     def self.populate_entity(root_node)
-      # Use all known properties to build the Entity (entity_data). All additional xml elements
+      entity_data = Hash[class_props.map {|name, type|
-      # are respected and attached to a hash as string (additional_xml_elements). It is intended
+        [name, parse_element_from_node(name, type, root_node)]
-      # to build a hash invariable of an Entity definition, in order to support receiving objects
+      }]
-      # from the future versions of Diaspora, where new elements may have been added.
-      entity_data = {}
-      xml_order = []
-      additional_xml_elements = {}
 
-      root_node.element_children.each do |child|
+      new(entity_data)
-        xml_name = child.name
-        property = find_property_for_xml_name(xml_name)
-
-        if property
-          entity_data[property] = parse_element_from_node(class_props[property], xml_name, root_node)
-          xml_order << property
-        else
-          additional_xml_elements[xml_name] = child.text
-          xml_order << xml_name
-        end
-      end
-
-      new(entity_data, xml_order, additional_xml_elements).tap do |entity|
-        entity.verify_signatures if entity.respond_to? :verify_signatures
-      end
     end
     private_class_method :populate_entity
 
+    # @param [String] name property name to parse
     # @param [Class] type target type to parse
-    # @param [String] xml_name xml tag to parse
+    # @param [Nokogiri::XML::Element] root_node XML node to parse
-    # @param [Nokogiri::XML::Element] node XML node to parse
     # @return [Object] parsed data
-    def self.parse_element_from_node(type, xml_name, node)
+    def self.parse_element_from_node(name, type, root_node)
       if type == String
-        parse_string_from_node(xml_name, node)
+        parse_string_from_node(name, root_node)
       elsif type.instance_of?(Array)
-        parse_array_from_node(type.first, node)
+        parse_array_from_node(type.first, root_node)
       elsif type.ancestors.include?(Entity)
-        parse_entity_from_node(type, node)
+        parse_entity_from_node(type, root_node)
       end
     end
     private_class_method :parse_element_from_node
@@ -258,6 +229,7 @@ module DiasporaFederation
     # @return [String] data
     def self.parse_string_from_node(name, root_node)
       node = root_node.xpath(name.to_s)
+      node = root_node.xpath(xml_names[name].to_s) if node.empty?
       node.first.text if node.any?
     end
     private_class_method :parse_string_from_node

spec/lib/diaspora_federation/entities/relayable_spec.rb

@@ -24,15 +24,15 @@ module DiasporaFederation
       end
     end
 
+    def sign_with_key(privkey, signature_data)
+      Base64.strict_encode64(privkey.sign(OpenSSL::Digest::SHA256.new, signature_data))
+    end
+
     def verify_signature(pubkey, signature, signed_string)
       pubkey.verify(OpenSSL::Digest::SHA256.new, Base64.decode64(signature), signed_string)
     end
 
     describe "#verify_signatures" do
-      def sign_with_key(privkey, signature_data)
-        Base64.strict_encode64(privkey.sign(OpenSSL::Digest::SHA256.new, signature_data))
-      end
-
       it "doesn't raise anything if correct signatures with legacy-string were passed" do
         hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
         hash[:parent_author_signature] = sign_with_key(parent_pkey, legacy_signature_data)
@@ -285,5 +285,85 @@ XML
         expect(xml.at_xpath("parent_author_signature").text).to eq("")
       end
     end
+
+    describe ".from_xml" do
+      context "parsing" do
+        before do
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :fetch_public_key_by_diaspora_id, author
+          ).and_return(author_pkey.public_key)
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :fetch_author_public_key_by_entity_guid, "Parent", parent_guid
+          ).and_return(parent_pkey.public_key)
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :entity_author_is_local?, "Parent", parent_guid
+          ).and_return(false)
+        end
+
+        let(:new_signature_data) { "#{author};#{guid};#{parent_guid};#{new_property};#{property}" }
+        let(:new_xml) {
+          <<-XML
+<some_relayable>
+  <diaspora_handle>#{author}</diaspora_handle>
+  <guid>#{guid}</guid>
+  <parent_guid>#{parent_guid}</parent_guid>
+  <new_property>#{new_property}</new_property>
+  <property>#{property}</property>
+  <author_signature>#{sign_with_key(author_pkey, new_signature_data)}</author_signature>
+  <parent_author_signature>#{sign_with_key(parent_pkey, new_signature_data)}</parent_author_signature>
+</some_relayable>
+XML
+        }
+
+        it "doesn't drop unknown properties" do
+          entity = SomeRelayable.from_xml(Nokogiri::XML::Document.parse(new_xml).root)
+
+          expect(entity).to be_an_instance_of SomeRelayable
+          expect(entity.property).to eq(property)
+          expect(entity.additional_xml_elements).to eq(
+            "new_property" => new_property
+          )
+        end
+
+        it "hand over the order in the xml to the instance" do
+          entity = SomeRelayable.from_xml(Nokogiri::XML::Document.parse(new_xml).root)
+
+          expect(entity.xml_order).to eq(
+            [:author, :guid, :parent_guid, "new_property", :property, :author_signature, :parent_author_signature]
+          )
+        end
+
+        it "creates Entity with empty 'additional_xml_elements' if the xml has only known properties" do
+          hash[:author_signature] = sign_with_key(author_pkey, legacy_signature_data)
+          hash[:parent_author_signature] = sign_with_key(parent_pkey, legacy_signature_data)
+
+          xml = SomeRelayable.new(hash).to_xml
+
+          entity = SomeRelayable.from_xml(xml)
+
+          expect(entity).to be_an_instance_of SomeRelayable
+          expect(entity.property).to eq(property)
+          expect(entity.additional_xml_elements).to be_empty
+        end
+      end
+
+      context "relayable signature verification feature support" do
+        it "calls signatures verification on relayable unpack" do
+          hash.merge!(author_signature: "aa", parent_author_signature: "bb")
+
+          xml = SomeRelayable.new(hash).to_xml
+
+          expect(DiasporaFederation.callbacks).to receive(:trigger).with(
+            :fetch_public_key_by_diaspora_id, author
+          ).and_return(author_pkey.public_key)
+
+          expect {
+            SomeRelayable.from_xml(xml)
+          }.to raise_error DiasporaFederation::Entities::Relayable::SignatureVerificationFailed
+        end
+      end
+    end
   end
 end

spec/lib/diaspora_federation/entity_spec.rb

@@ -168,45 +168,6 @@ XML
           expect(entity.test).to eq("asdf")
           expect(entity.qwer).to eq("qwer")
         end
-
-        it "doesn't drop unknown properties" do
-          xml = <<-XML
-<test_entity>
-  <a_prop_from_newer_diaspora_version>some value</a_prop_from_newer_diaspora_version>
-  <test>asdf</test>
-  <some_random_property>another value</some_random_property>
-</test_entity>
-XML
-
-          entity = Entities::TestEntity.from_xml(Nokogiri::XML::Document.parse(xml).root)
-
-          expect(entity).to be_an_instance_of Entities::TestEntity
-          expect(entity.test).to eq("asdf")
-          expect(entity.additional_xml_elements).to eq(
-            "a_prop_from_newer_diaspora_version" => "some value",
-            "some_random_property"               => "another value"
-          )
-        end
-
-        it "creates Entity with nil 'additional_xml_elements' if the xml has only known properties" do
-          entity = Entities::TestEntity.from_xml(entity_xml)
-
-          expect(entity).to be_an_instance_of Entities::TestEntity
-          expect(entity.test).to eq("asdf")
-          expect(entity.additional_xml_elements).to be_empty
-        end
-      end
-
-      context "relayable signature verification feature support" do
-        it "calls signatures verification on relayable unpack" do
-          entity = FactoryGirl.build(:comment_entity, author: alice.diaspora_id)
-          entity_xml = entity.to_xml
-          entity_xml.at_xpath("author_signature").content = nil
-
-          expect {
-            Entities::Comment.from_xml(entity_xml)
-          }.to raise_error DiasporaFederation::Entities::Relayable::SignatureVerificationFailed
-        end
       end
 
       context "nested entities" do