add additional parsed xml properties to the entity-instance

allow mapping with `name` and `xml_name`

lib/diaspora_federation/entities/relayable.rb

@@ -56,8 +56,8 @@ module DiasporaFederation
       #
       # @see Entity#to_h
       # @return [Hash] entity data hash with updated signatures
-      def to_h
+      def to_signed_h
-        super.tap do |hash|
+        to_h.tap do |hash|
           if author_signature.nil?
             privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_diaspora_id, diaspora_id)
             raise AuthorPrivateKeyNotFound, "author=#{diaspora_id} guid=#{guid}" if privkey.nil?
@@ -74,7 +74,7 @@ module DiasporaFederation
       # @return [Nokogiri::XML::Element] root element containing properties as child elements
       def to_xml
         entity_xml.tap do |xml|
-          hash = to_h
+          hash = to_signed_h
           xml.at_xpath("author_signature").content = hash[:author_signature]
           xml.at_xpath("parent_author_signature").content = hash[:parent_author_signature]
         end
@@ -135,7 +135,7 @@ module DiasporaFederation
           return false
         end
 
-        validity = pubkey.verify(DIGEST, Base64.decode64(signature), legacy_signature_data(data))
+        validity = pubkey.verify(DIGEST, Base64.decode64(signature), legacy_signature_data(to_h))
         logger.info "event=verify_signature status=complete guid=#{guid} validity=#{validity}"
         validity
       end

lib/diaspora_federation/entity.rb

@@ -35,9 +35,9 @@ module DiasporaFederation
   class Entity
     extend PropertiesDSL
 
-    # the original data hash with which the entity was created
+    # additional properties from parsed xml
-    # @return [Hash] original data
+    # @return [Hash] additional xml elements
-    attr_reader :data
+    attr_reader :additional_xml_elements
 
     # Initializes the Entity with the given attribute hash and freezes the created
     # instance it returns.
@@ -51,16 +51,17 @@ module DiasporaFederation
     # @note Attributes not defined as part of the class definition ({PropertiesDSL#property},
     #       {PropertiesDSL#entity}) get discarded silently.
     #
-    # @param [Hash] data
+    # @param [Hash] data entity data
+    # @param [Hash] additional_xml_elements additional xml elements
     # @return [Entity] new instance
-    def initialize(data)
+    def initialize(data, additional_xml_elements=nil)
       raise ArgumentError, "expected a Hash" unless data.is_a?(Hash)
       missing_props = self.class.missing_props(data)
       unless missing_props.empty?
         raise ArgumentError, "missing required properties: #{missing_props.join(', ')}"
       end
 
-      @data = data
+      @additional_xml_elements = nilify(additional_xml_elements)
 
       self.class.default_values.merge(data).each do |k, v|
         instance_variable_set("@#{k}", nilify(v)) if setable?(k, v)

lib/diaspora_federation/properties_dsl.rb

@@ -66,6 +66,13 @@ module DiasporaFederation
       @class_prop_names ||= class_props.map {|p| p[:name] }
     end
 
+    # finds a property by +xml_name+ or +name+
+    # @param [String] xml_name name of the property from the received xml
+    # @return [Hash] the property data
+    def find_property_for_xml_name(xml_name)
+      class_props.find {|prop| prop[:xml_name].to_s == xml_name || prop[:name].to_s == xml_name }
+    end
+
     private
 
     def determine_xml_name(name, type, opts={})

lib/diaspora_federation/salmon/xml_payload.rb

@@ -83,38 +83,41 @@ module DiasporaFederation
       # @param [Nokogiri::XML::Element] root_node xml nodes
       # @return [Entity] instance
       def self.populate_entity(klass, root_node)
-        # Use all known properties to build the Entity. All other elements are respected
+        # Use all known properties to build the Entity (entity_data). All additional xml elements
-        # and attached to resulted hash as string. It is intended to build a hash
+        # are respected and attached to a hash as string (additional_xml_elements). It is intended
-        # invariable of an Entity definition, in order to support receiving objects
+        # 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.
-        data = Hash[root_node.element_children.map {|child|
+        entity_data = {}
-          xml_name = child.name
+        additional_xml_elements = {}
-          property = klass.class_props.find {|prop| prop[:xml_name].to_s == xml_name }
-          if property
-            parse_element_from_node(property[:name], property[:type], xml_name, root_node)
-          else
-            [xml_name, child.text]
-          end
-        }]
 
-        klass.new(data).tap do |entity|
+        root_node.element_children.each do |child|
+          xml_name = child.name
+          property = klass.find_property_for_xml_name(xml_name)
+
+          if property
+            entity_data[property[:name]] = parse_element_from_node(property[:type], xml_name, root_node)
+          else
+            additional_xml_elements[xml_name] = child.text
+          end
+        end
+
+        klass.new(entity_data, additional_xml_elements).tap do |entity|
           entity.verify_signatures if entity.respond_to? :verify_signatures
         end
       end
       private_class_method :populate_entity
 
-      # @param [Symbol] name property name
       # @param [Class] type target type to parse
       # @param [String] xml_name xml tag to parse
       # @param [Nokogiri::XML::Element] node XML node to parse
-      # @return [Array<Symbol, Object>] parsed data
+      # @return [Object] parsed data
-      def self.parse_element_from_node(name, type, xml_name, node)
+      def self.parse_element_from_node(type, xml_name, node)
         if type == String
-          [name, parse_string_from_node(xml_name, node)]
+          parse_string_from_node(xml_name, node)
         elsif type.instance_of?(Array)
-          [name, parse_array_from_node(type, node)]
+          parse_array_from_node(type, node)
         elsif type.ancestors.include?(Entity)
-          [name, parse_entity_from_node(type, node)]
+          parse_entity_from_node(type, node)
         end
       end
       private_class_method :parse_element_from_node

spec/lib/diaspora_federation/entities/comment_spec.rb

@@ -1,7 +1,9 @@
 module DiasporaFederation
   describe Entities::Comment do
     let(:parent) { FactoryGirl.create(:post, author: bob) }
-    let(:data) { FactoryGirl.build(:comment_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_h }
+    let(:data) {
+      FactoryGirl.build(:comment_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_signed_h
+    }
 
     let(:xml) {
       <<-XML

spec/lib/diaspora_federation/entities/conversation_spec.rb

@@ -1,10 +1,14 @@
 module DiasporaFederation
   describe Entities::Conversation do
     let(:parent) { FactoryGirl.create(:conversation, author: bob) }
-    let(:msg1) { FactoryGirl.build(:message_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_h }
+    let(:signed_msg1) {
-    let(:msg2) { FactoryGirl.build(:message_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_h }
+      msg = FactoryGirl.build(:message_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_signed_h
-    let(:signed_msg1) { Entities::Message.new(msg1) }
+      Entities::Message.new(msg)
-    let(:signed_msg2) { Entities::Message.new(msg2) }
+    }
+    let(:signed_msg2) {
+      msg = FactoryGirl.build(:message_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_signed_h
+      Entities::Message.new(msg)
+    }
     let(:data) {
       FactoryGirl.attributes_for(:conversation_entity).merge!(
         messages:        [signed_msg1, signed_msg2],

spec/lib/diaspora_federation/entities/like_spec.rb

@@ -7,7 +7,7 @@ module DiasporaFederation
         diaspora_id: alice.diaspora_id,
         parent_guid: parent.guid,
         parent_type: parent.entity_type
-      ).to_h
+      ).to_signed_h
     }
 
     let(:xml) {

spec/lib/diaspora_federation/entities/message_spec.rb

@@ -1,7 +1,9 @@
 module DiasporaFederation
   describe Entities::Message do
     let(:parent) { FactoryGirl.create(:conversation, author: bob) }
-    let(:data) { FactoryGirl.build(:message_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_h }
+    let(:data) {
+      FactoryGirl.build(:message_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_signed_h
+    }
 
     let(:xml) {
       <<-XML

spec/lib/diaspora_federation/entities/participation_spec.rb

@@ -7,7 +7,7 @@ module DiasporaFederation
         diaspora_id: alice.diaspora_id,
         parent_guid: parent.guid,
         parent_type: parent.entity_type
-      ).to_h
+      ).to_signed_h
     }
 
     let(:xml) {

spec/lib/diaspora_federation/entities/poll_participation_spec.rb

@@ -2,7 +2,11 @@ module DiasporaFederation
   describe Entities::PollParticipation do
     let(:parent) { FactoryGirl.create(:poll, author: bob) }
     let(:data) {
-      FactoryGirl.build(:poll_participation_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_h
+      FactoryGirl.build(
+        :poll_participation_entity,
+        diaspora_id: alice.diaspora_id,
+        parent_guid: parent.guid
+      ).to_signed_h
     }
 
     let(:xml) {

spec/lib/diaspora_federation/entities/relayable_spec.rb

@@ -132,7 +132,7 @@ module DiasporaFederation
       end
     end
 
-    describe "#to_h" do
+    describe "#to_signed_h" do
       it "updates signatures when they were nil and keys were supplied" do
         expect(DiasporaFederation.callbacks).to receive(:trigger).with(
           :fetch_private_key_by_diaspora_id, hash[:diaspora_id]
@@ -144,7 +144,7 @@ module DiasporaFederation
 
         signed_string = hash.reject {|key, _| key == :some_other_data }.values.join(";")
 
-        signed_hash = SomeRelayable.new(hash).to_h
+        signed_hash = SomeRelayable.new(hash).to_signed_h
 
         def verify_signature(pubkey, signature, signed_string)
           pubkey.verify(OpenSSL::Digest::SHA256.new, Base64.decode64(signature), signed_string)
@@ -157,7 +157,7 @@ module DiasporaFederation
       it "doesn't change signatures if they are already set" do
         hash.merge!(author_signature: "aa", parent_author_signature: "bb").delete(:some_other_data)
 
-        expect(SomeRelayable.new(hash).to_h).to eq(hash)
+        expect(SomeRelayable.new(hash).to_signed_h).to eq(hash)
       end
 
       it "raises when author_signature not set and key isn't supplied" do
@@ -166,7 +166,7 @@ module DiasporaFederation
         ).and_return(nil)
 
         expect {
-          SomeRelayable.new(hash).to_h
+          SomeRelayable.new(hash).to_signed_h
         }.to raise_error Entities::Relayable::AuthorPrivateKeyNotFound
       end
 
@@ -179,7 +179,7 @@ module DiasporaFederation
           :fetch_author_private_key_by_entity_guid, "Parent", hash[:parent_guid]
         ).and_return(nil)
 
-        signed_hash = SomeRelayable.new(hash).to_h
+        signed_hash = SomeRelayable.new(hash).to_signed_h
 
         expect(signed_hash[:parent_author_signature]).to eq(nil)
       end

spec/lib/diaspora_federation/properties_dsl_spec.rb

@@ -123,5 +123,22 @@ module DiasporaFederation
         expect(Entities::TestDefaultEntity.class_prop_names).to include(:test1, :test2, :test3, :test4)
       end
     end
+
+    describe ".find_property_for_xml_name" do
+      it "finds property by xml_name" do
+        dsl.property :test, xml_name: :xml_test
+        expect(dsl.find_property_for_xml_name("xml_test")).to eq(dsl.class_props.first)
+      end
+
+      it "finds property by name" do
+        dsl.property :test, xml_name: :xml_test
+        expect(dsl.find_property_for_xml_name("test")).to eq(dsl.class_props.first)
+      end
+
+      it "returns nil if property is not defined" do
+        dsl.property :test, xml_name: :xml_test
+        expect(dsl.find_property_for_xml_name("unknown")).to be_nil
+      end
+    end
   end
 end

spec/lib/diaspora_federation/salmon/xml_payload_spec.rb

@@ -2,6 +2,17 @@ module DiasporaFederation
   describe Salmon::XmlPayload do
     let(:entity) { Entities::TestEntity.new(test: "asdf") }
     let(:payload) { Salmon::XmlPayload.pack(entity) }
+    let(:entity_xml) {
+      <<-XML.strip
+<XML>
+  <post>
+    <test_entity>
+      <test>asdf</test>
+    </test_entity>
+  </post>
+</XML>
+XML
+    }
 
     describe ".pack" do
       it "expects an Entity as param" do
@@ -35,16 +46,7 @@ module DiasporaFederation
         end
 
         it "produces the expected XML" do
-          xml = <<-XML.strip
+          expect(subject.to_xml).to eq(entity_xml)
-<XML>
-  <post>
-    <test_entity>
-      <test>asdf</test>
-    </test_entity>
-  </post>
-</XML>
-XML
-          expect(subject.to_xml).to eq(xml)
         end
       end
     end
@@ -103,7 +105,9 @@ XML
           expect(subject).to be_an_instance_of Entities::TestEntity
           expect(subject.test).to eq("asdf")
         end
+      end
 
+      context "parsing" do
         it "uses xml_name for parsing" do
           xml = <<-XML.strip
 <XML>
@@ -122,6 +126,24 @@ XML
           expect(entity.qwer).to eq("qwer")
         end
 
+        it "allows name for parsing even when property has a xml_name" do
+          xml = <<-XML.strip
+<XML>
+  <post>
+    <test_entity_with_xml_name>
+      <test>asdf</test>
+      <qwer>qwer</qwer>
+    </test_entity>
+  </post>
+</XML>
+XML
+          entity = Salmon::XmlPayload.unpack(Nokogiri::XML::Document.parse(xml).root)
+
+          expect(entity).to be_an_instance_of Entities::TestEntityWithXmlName
+          expect(entity.test).to eq("asdf")
+          expect(entity.qwer).to eq("qwer")
+        end
+
         it "doesn't drop unknown properties" do
           xml = <<-XML
 <XML>
@@ -134,12 +156,23 @@ XML
   </post>
 </XML>
 XML
-          expect(Entities::TestEntity).to receive(:new).with(
+
+          entity = Salmon::XmlPayload.unpack(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",
-            :test                                => "asdf",
             "some_random_property"               => "another value"
           )
-          Salmon::XmlPayload.unpack(Nokogiri::XML::Document.parse(xml).root)
+        end
+
+        it "creates Entity with nil 'additional_xml_elements' if the xml has only known properties" do
+          entity = Salmon::XmlPayload.unpack(Nokogiri::XML::Document.parse(entity_xml).root)
+
+          expect(entity).to be_an_instance_of Entities::TestEntity
+          expect(entity.test).to eq("asdf")
+          expect(entity.additional_xml_elements).to be_nil
         end
       end