From 1c7a5ad3e6bc99ddfc89ad23aa36f6a7bb922b3d Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Fri, 5 Feb 2016 00:16:24 +0100 Subject: [PATCH] add additional parsed xml properties to the entity-instance allow mapping with `name` and `xml_name` --- lib/diaspora_federation/entities/relayable.rb | 8 +-- lib/diaspora_federation/entity.rb | 13 ++-- lib/diaspora_federation/properties_dsl.rb | 7 +++ lib/diaspora_federation/salmon/xml_payload.rb | 41 +++++++------ .../entities/comment_spec.rb | 4 +- .../entities/conversation_spec.rb | 12 ++-- .../diaspora_federation/entities/like_spec.rb | 2 +- .../entities/message_spec.rb | 4 +- .../entities/participation_spec.rb | 2 +- .../entities/poll_participation_spec.rb | 6 +- .../entities/relayable_spec.rb | 10 ++-- .../properties_dsl_spec.rb | 17 ++++++ .../salmon/xml_payload_spec.rb | 59 +++++++++++++++---- 13 files changed, 129 insertions(+), 56 deletions(-) diff --git a/lib/diaspora_federation/entities/relayable.rb b/lib/diaspora_federation/entities/relayable.rb index c67990e..bcec828 100644 --- a/lib/diaspora_federation/entities/relayable.rb +++ b/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 - super.tap do |hash| + def to_signed_h + 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 diff --git a/lib/diaspora_federation/entity.rb b/lib/diaspora_federation/entity.rb index db9fa2c..0f5a819 100644 --- a/lib/diaspora_federation/entity.rb +++ b/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 - # @return [Hash] original data - attr_reader :data + # 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. @@ -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) diff --git a/lib/diaspora_federation/properties_dsl.rb b/lib/diaspora_federation/properties_dsl.rb index 98dc074..ade42ef 100644 --- a/lib/diaspora_federation/properties_dsl.rb +++ b/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={}) diff --git a/lib/diaspora_federation/salmon/xml_payload.rb b/lib/diaspora_federation/salmon/xml_payload.rb index 6a0deba..7439be3 100644 --- a/lib/diaspora_federation/salmon/xml_payload.rb +++ b/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 - # and attached to resulted hash as string. It is intended to build a hash - # invariable of an Entity definition, in order to support receiving objects + # 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 is intended + # 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| - xml_name = child.name - 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 - }] + entity_data = {} + additional_xml_elements = {} - 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] parsed data - def self.parse_element_from_node(name, type, xml_name, node) + # @return [Object] parsed data + 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 diff --git a/spec/lib/diaspora_federation/entities/comment_spec.rb b/spec/lib/diaspora_federation/entities/comment_spec.rb index a64b859..4f92afb 100644 --- a/spec/lib/diaspora_federation/entities/comment_spec.rb +++ b/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 diff --git a/spec/lib/diaspora_federation/entities/conversation_spec.rb b/spec/lib/diaspora_federation/entities/conversation_spec.rb index 9e7e493..dbe684a 100644 --- a/spec/lib/diaspora_federation/entities/conversation_spec.rb +++ b/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(:msg2) { FactoryGirl.build(:message_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_h } - let(:signed_msg1) { Entities::Message.new(msg1) } - let(:signed_msg2) { Entities::Message.new(msg2) } + let(:signed_msg1) { + msg = FactoryGirl.build(:message_entity, diaspora_id: alice.diaspora_id, parent_guid: parent.guid).to_signed_h + Entities::Message.new(msg) + } + 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], diff --git a/spec/lib/diaspora_federation/entities/like_spec.rb b/spec/lib/diaspora_federation/entities/like_spec.rb index 11d2e2a..d13bd32 100644 --- a/spec/lib/diaspora_federation/entities/like_spec.rb +++ b/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) { diff --git a/spec/lib/diaspora_federation/entities/message_spec.rb b/spec/lib/diaspora_federation/entities/message_spec.rb index 92510b2..a07b789 100644 --- a/spec/lib/diaspora_federation/entities/message_spec.rb +++ b/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 diff --git a/spec/lib/diaspora_federation/entities/participation_spec.rb b/spec/lib/diaspora_federation/entities/participation_spec.rb index 30ef57b..fc09ab9 100644 --- a/spec/lib/diaspora_federation/entities/participation_spec.rb +++ b/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) { diff --git a/spec/lib/diaspora_federation/entities/poll_participation_spec.rb b/spec/lib/diaspora_federation/entities/poll_participation_spec.rb index d1bc5f2..4e1b316 100644 --- a/spec/lib/diaspora_federation/entities/poll_participation_spec.rb +++ b/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) { diff --git a/spec/lib/diaspora_federation/entities/relayable_spec.rb b/spec/lib/diaspora_federation/entities/relayable_spec.rb index db7bfd2..1af4d53 100644 --- a/spec/lib/diaspora_federation/entities/relayable_spec.rb +++ b/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 diff --git a/spec/lib/diaspora_federation/properties_dsl_spec.rb b/spec/lib/diaspora_federation/properties_dsl_spec.rb index cd8a287..90a51bd 100644 --- a/spec/lib/diaspora_federation/properties_dsl_spec.rb +++ b/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 diff --git a/spec/lib/diaspora_federation/salmon/xml_payload_spec.rb b/spec/lib/diaspora_federation/salmon/xml_payload_spec.rb index 5b0fd94..8165483 100644 --- a/spec/lib/diaspora_federation/salmon/xml_payload_spec.rb +++ b/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 + + + + asdf + + + +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 - - - - asdf - - - -XML - expect(subject.to_xml).to eq(xml) + expect(subject.to_xml).to eq(entity_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 @@ -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 + + + + asdf + qwer + + + +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 @@ -134,12 +156,23 @@ 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