From 0ccd15cd53b921a85af93c8ee25cd5c22962cbe1 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Tue, 29 Oct 2019 02:54:07 +0100 Subject: [PATCH] Remove legacy xml unwrap code Closes #28 --- lib/diaspora_federation/salmon.rb | 1 - .../salmon/magic_envelope.rb | 3 +- lib/diaspora_federation/salmon/xml_payload.rb | 43 -------- spec/integration/comment_integration_spec.rb | 100 ++---------------- .../entities/account_migration_spec.rb | 3 +- .../entities/conversation_spec.rb | 3 +- .../entities/event_spec.rb | 3 +- .../entities/photo_spec.rb | 3 +- .../entities/profile_spec.rb | 3 +- .../entities/status_message_spec.rb | 3 +- .../salmon/xml_payload_spec.rb | 60 ----------- spec/support/shared_entity_specs.rb | 2 +- 12 files changed, 21 insertions(+), 206 deletions(-) delete mode 100644 lib/diaspora_federation/salmon/xml_payload.rb delete mode 100644 spec/lib/diaspora_federation/salmon/xml_payload_spec.rb diff --git a/lib/diaspora_federation/salmon.rb b/lib/diaspora_federation/salmon.rb index 0267598..6bcd6dc 100644 --- a/lib/diaspora_federation/salmon.rb +++ b/lib/diaspora_federation/salmon.rb @@ -13,6 +13,5 @@ require "base64" require "diaspora_federation/salmon/aes" require "diaspora_federation/salmon/exceptions" -require "diaspora_federation/salmon/xml_payload" require "diaspora_federation/salmon/magic_envelope" require "diaspora_federation/salmon/encrypted_magic_envelope" diff --git a/lib/diaspora_federation/salmon/magic_envelope.rb b/lib/diaspora_federation/salmon/magic_envelope.rb index fcc3f53..fafa1cd 100644 --- a/lib/diaspora_federation/salmon/magic_envelope.rb +++ b/lib/diaspora_federation/salmon/magic_envelope.rb @@ -114,7 +114,8 @@ module DiasporaFederation logger.debug "unenvelop message from #{sender}:\n#{data}" - new(XmlPayload.unpack(Nokogiri::XML(data).root), sender) + xml = Nokogiri::XML(data).root + new(Entity.entity_class(xml.name).from_xml(xml), sender) end private diff --git a/lib/diaspora_federation/salmon/xml_payload.rb b/lib/diaspora_federation/salmon/xml_payload.rb deleted file mode 100644 index 32ea453..0000000 --- a/lib/diaspora_federation/salmon/xml_payload.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - module Salmon - # +XmlPayload+ provides methods to wrap a XML-serialized {Entity} inside a - # common XML structure that will become the payload for federation messages. - # - # The wrapper looks like so: - # - # - # {data} - # - # - # - # (The +post+ element is there for historic reasons...) - # @deprecated - module XmlPayload - # Extracts the Entity XML from the wrapping XML structure, parses the entity - # XML and returns a new instance of the Entity that was packed inside the - # given payload. - # - # @param [Nokogiri::XML::Element] xml payload XML root node - # @return [Entity] re-constructed Entity instance - # @raise [ArgumentError] if the argument is not an - # {http://www.rubydoc.info/gems/nokogiri/Nokogiri/XML/Element Nokogiri::XML::Element} - # @raise [UnknownEntity] if the class for the entity contained inside the - # XML can't be found - def self.unpack(xml) - raise ArgumentError, "only Nokogiri::XML::Element allowed" unless xml.instance_of?(Nokogiri::XML::Element) - - data = xml_wrapped?(xml) ? xml.at_xpath("post/*[1]") : xml - - Entity.entity_class(data.name).from_xml(data) - end - - # @param [Nokogiri::XML::Element] element - private_class_method def self.xml_wrapped?(element) - (element.name == "XML" && !element.at_xpath("post").nil? && - !element.at_xpath("post").children.empty?) - end - end - end -end diff --git a/spec/integration/comment_integration_spec.rb b/spec/integration/comment_integration_spec.rb index f713699..2966631 100644 --- a/spec/integration/comment_integration_spec.rb +++ b/spec/integration/comment_integration_spec.rb @@ -65,20 +65,6 @@ module DiasporaFederation ) } - let(:legacy_format_comment_xml_alice) { <<~XML } - - - - e21589b0b41101333b870f77ba60fa73 - 9e269ae0b41201333b8c0f77ba60fa73 - XU5X1uqTh8SY6JMG9uhEVR5Rg7FURV6lpRwl/HYOu6DJ3Hd9tpA2aSFFibUxxsMgJXKNrrc5SykrrEdTiQoEei+j0QqZf3B5R7r84qgK7M46KazwIpqRPwVl2MdA/0DdQyYJLA/oavNj1nwll9vtR87M7e/C94qG6P+iQTMBQzo= - - this is a very informative comment - alice@pod-a.org - - - - XML let(:new_format_comment_xml_alice) { <<~XML } alice@pod-a.org @@ -101,20 +87,6 @@ module DiasporaFederation XML - let(:legacy_format_comment_xml_bob) { <<~XML } - - - - e21589b0b41101333b870f77ba60fa73 - 9e269ae0b41201333b8c0f77ba60fa73 - this is a very informative comment - alice@pod-a.org - XU5X1uqTh8SY6JMG9uhEVR5Rg7FURV6lpRwl/HYOu6DJ3Hd9tpA2aSFFibUxxsMgJXKNrrc5SykrrEdTiQoEei+j0QqZf3B5R7r84qgK7M46KazwIpqRPwVl2MdA/0DdQyYJLA/oavNj1nwll9vtR87M7e/C94qG6P+iQTMBQzo= - QqWSdwpb+/dcJUxuKKVe7aiz1NivXzlIdWZ71xyrxnhFxFYd+7EIittyTcp1cVehjg96pwDbn++P/rWyCffqenWu025DHvUfSmQkC93Z0dX6r3OIUlZqwEggtOdbunybiE++F3BVsGt5wC4YbAESB5ZFuhFVhBXh1X+EaZ/qoKo= - - - - XML let(:legacy_order_new_format_comment_xml_bob) { <<~XML } e21589b0b41101333b870f77ba60fa73 @@ -125,20 +97,6 @@ module DiasporaFederation QqWSdwpb+/dcJUxuKKVe7aiz1NivXzlIdWZ71xyrxnhFxFYd+7EIittyTcp1cVehjg96pwDbn++P/rWyCffqenWu025DHvUfSmQkC93Z0dX6r3OIUlZqwEggtOdbunybiE++F3BVsGt5wC4YbAESB5ZFuhFVhBXh1X+EaZ/qoKo= XML - let(:new_order_legacy_format_comment_xml_bob) { <<~XML } - - - - alice@pod-a.org - e21589b0b41101333b870f77ba60fa73 - 9e269ae0b41201333b8c0f77ba60fa73 - this is a very informative comment - SQbLeqsEpFmSl74G1fFJXKQcsq6jp5B2ZjmfEOF/LbBccYP2oZEyEqOq18K3Fa71OYTp6Nddb38hCmHWWHvnGUltGfxKBnQ0WHafJUi40VM4VmeRoU8cac6m+1hslwe5SNmK6oh47EV3mRCXlgGGjLIrw7iEwjKL2g9x1gkNp2s= - hWsagsczmZD6d36d6MFdTt3hKAdnRtupSIU6464G2kkMJ+WlExxMgbF6kWR+jVCBTeKipWCYK3Arnj0YkuIZM9d14bJGVMTsW/ZzNfJ69bXZhsyawI8dPnZnLVydo+hU/XmGJBEuh2TOj9Emq6/HCYiWzPTF5qhYAtyJ1oxJ4Yk= - - - - XML let(:new_format_comment_xml_bob) { <<~XML } alice@pod-a.org @@ -149,21 +107,6 @@ module DiasporaFederation hWsagsczmZD6d36d6MFdTt3hKAdnRtupSIU6464G2kkMJ+WlExxMgbF6kWR+jVCBTeKipWCYK3Arnj0YkuIZM9d14bJGVMTsW/ZzNfJ69bXZhsyawI8dPnZnLVydo+hU/XmGJBEuh2TOj9Emq6/HCYiWzPTF5qhYAtyJ1oxJ4Yk= XML - let(:legacy_format_new_data_comment_xml_bob) { <<~XML } - - - - alice@pod-a.org - e21589b0b41101333b870f77ba60fa73 - 9e269ae0b41201333b8c0f77ba60fa73 - this is a very informative comment - foobar - SFYXSvCX/DhTFiOUALp2Nf1kfNkGKXrnoBPikAyhaIogGydVBm+8tIlu1U/vsnpyKO3yfC3JReJ00/UBd4J16VO1IxStntq8NUqbSv4me5A/6kdK9Xg6eYbXrqQGm8fUQ5Xuh2UzeB71p7SVySXX3OZHVe0dvHCxH/lsfSDpEjc= - NxXuEUVeXwUMR77osIbaNlp2oB3bpl8rBEFgQoO6cnoN5ewDbiGADK0x6EhcmJptjwhGVcZiNJNpq7k3/pjJtKaH++3ToCAtcuZoIKwPDsneLnjPhVjE2GXM1TiZKwoHrq41qSp/8Vl5UPbtC6sPiOzIvPKaILXUG8XCiVWuB0M= - - - - XML let(:new_data_comment_xml_bob) { <<~XML } alice@pod-a.org @@ -191,7 +134,7 @@ module DiasporaFederation expect_callback(:fetch_related_entity, "Post", parent_guid).and_return(parent) xml = Nokogiri::XML(new_data_comment_xml_alice).root - Salmon::XmlPayload.unpack(xml).to_xml + Entity.entity_class(xml.name).from_xml(xml).to_xml end end @@ -202,21 +145,15 @@ module DiasporaFederation expect_callback(:fetch_related_entity, "Post", parent_guid).and_return(parent) end - it "relays legacy order" do - xml = Nokogiri::XML(legacy_format_comment_xml_alice).root - entity = Salmon::XmlPayload.unpack(xml) - expect(entity.to_xml.to_xml).to eq(legacy_order_new_format_comment_xml_bob.strip) - end - it "relays new order" do xml = Nokogiri::XML(new_format_comment_xml_alice).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.to_xml.to_xml).to eq(new_format_comment_xml_bob.strip) end it "relays new data" do xml = Nokogiri::XML(new_data_comment_xml_alice).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.to_xml.to_xml).to eq(new_data_comment_xml_bob.strip) end end @@ -229,25 +166,9 @@ module DiasporaFederation expect_callback(:fetch_related_entity, "Post", parent_guid).and_return(parent) end - it "parses legacy format" do - xml = Nokogiri::XML(legacy_format_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) - - expect(entity.author).to eq(author) - expect(entity.text).to eq(text) - end - it "parses legacy order with new xml format" do xml = Nokogiri::XML(legacy_order_new_format_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) - - expect(entity.author).to eq(author) - expect(entity.text).to eq(text) - end - - it "parses new order with legacy xml format" do - xml = Nokogiri::XML(new_order_legacy_format_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.author).to eq(author) expect(entity.text).to eq(text) @@ -255,24 +176,15 @@ module DiasporaFederation it "parses new xml format" do xml = Nokogiri::XML(new_format_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.author).to eq(author) expect(entity.text).to eq(text) end - it "parses new data with legacy xml format" do - xml = Nokogiri::XML(legacy_format_new_data_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) - - expect(entity.author).to eq(author) - expect(entity.text).to eq(text) - expect(entity.additional_data["new_data"]).to eq(new_data) - end - it "parses new data with new xml format" do xml = Nokogiri::XML(new_data_comment_xml_bob).root - entity = Salmon::XmlPayload.unpack(xml) + entity = Entity.entity_class(xml.name).from_xml(xml) expect(entity.author).to eq(author) expect(entity.text).to eq(text) diff --git a/spec/lib/diaspora_federation/entities/account_migration_spec.rb b/spec/lib/diaspora_federation/entities/account_migration_spec.rb index cf79218..423ab35 100644 --- a/spec/lib/diaspora_federation/entities/account_migration_spec.rb +++ b/spec/lib/diaspora_federation/entities/account_migration_spec.rb @@ -215,7 +215,8 @@ module DiasporaFederation it "fails validation on parsing" do expect { - DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(xml).root) + parsed_xml = Nokogiri::XML(xml).root + Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) }.to raise_error Entity::ValidationError end end diff --git a/spec/lib/diaspora_federation/entities/conversation_spec.rb b/spec/lib/diaspora_federation/entities/conversation_spec.rb index 9522873..ace9e08 100644 --- a/spec/lib/diaspora_federation/entities/conversation_spec.rb +++ b/spec/lib/diaspora_federation/entities/conversation_spec.rb @@ -58,7 +58,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.messages).to eq([]) end end diff --git a/spec/lib/diaspora_federation/entities/event_spec.rb b/spec/lib/diaspora_federation/entities/event_spec.rb index 78568ef..8b892a9 100644 --- a/spec/lib/diaspora_federation/entities/event_spec.rb +++ b/spec/lib/diaspora_federation/entities/event_spec.rb @@ -43,7 +43,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.end).to be_nil expect(parsed_instance.all_day).to be_falsey expect(parsed_instance.timezone).to be_nil diff --git a/spec/lib/diaspora_federation/entities/photo_spec.rb b/spec/lib/diaspora_federation/entities/photo_spec.rb index 92d55e9..b6804c0 100644 --- a/spec/lib/diaspora_federation/entities/photo_spec.rb +++ b/spec/lib/diaspora_federation/entities/photo_spec.rb @@ -62,7 +62,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.public).to be_falsey expect(parsed_instance.text).to be_nil end diff --git a/spec/lib/diaspora_federation/entities/profile_spec.rb b/spec/lib/diaspora_federation/entities/profile_spec.rb index 4cc7d59..ef594b2 100644 --- a/spec/lib/diaspora_federation/entities/profile_spec.rb +++ b/spec/lib/diaspora_federation/entities/profile_spec.rb @@ -63,7 +63,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.full_name).to be_nil expect(parsed_instance.first_name).to be_nil expect(parsed_instance.last_name).to be_nil diff --git a/spec/lib/diaspora_federation/entities/status_message_spec.rb b/spec/lib/diaspora_federation/entities/status_message_spec.rb index 9424030..bd64130 100644 --- a/spec/lib/diaspora_federation/entities/status_message_spec.rb +++ b/spec/lib/diaspora_federation/entities/status_message_spec.rb @@ -136,7 +136,8 @@ module DiasporaFederation XML - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root) + parsed_xml = Nokogiri::XML(minimal_xml).root + parsed_instance = Entity.entity_class(parsed_xml.name).from_xml(parsed_xml) expect(parsed_instance.photos).to eq([]) expect(parsed_instance.location).to be_nil expect(parsed_instance.poll).to be_nil diff --git a/spec/lib/diaspora_federation/salmon/xml_payload_spec.rb b/spec/lib/diaspora_federation/salmon/xml_payload_spec.rb deleted file mode 100644 index 180949b..0000000 --- a/spec/lib/diaspora_federation/salmon/xml_payload_spec.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -module DiasporaFederation - describe Salmon::XmlPayload do - let(:entity) { Entities::TestEntity.new(test: "asdf") } - let(:entity_xml) { <<~XML.strip } - - - - asdf - - - - XML - - describe ".unpack" do - context "sanity" do - it "expects an Nokogiri::XML::Element as param" do - expect { - Salmon::XmlPayload.unpack(Nokogiri::XML(entity_xml).root) - }.not_to raise_error - end - - it "raises and error when the param is not an Nokogiri::XML::Element" do - ["asdf", 1234, true, :test, entity].each do |val| - expect { - Salmon::XmlPayload.unpack(val) - }.to raise_error ArgumentError, "only Nokogiri::XML::Element allowed" - end - end - end - - context "returned object" do - subject { Salmon::XmlPayload.unpack(Nokogiri::XML(entity_xml).root) } - - it "#to_h should match entity.to_h" do - expect(subject.to_h).to eq(entity.to_h) - end - - it "returns an entity instance of the original class" do - expect(subject).to be_an_instance_of Entities::TestEntity - expect(subject.test).to eq("asdf") - end - - it "allows unwrapped entities" do - xml = <<~XML - - asdf - - XML - - entity = Salmon::XmlPayload.unpack(Nokogiri::XML(xml).root) - - expect(entity).to be_an_instance_of Entities::TestEntity - expect(entity.test).to eq("asdf") - end - end - end - end -end diff --git a/spec/support/shared_entity_specs.rb b/spec/support/shared_entity_specs.rb index 4c7ad42..4d1e256 100644 --- a/spec/support/shared_entity_specs.rb +++ b/spec/support/shared_entity_specs.rb @@ -67,7 +67,7 @@ shared_examples "an XML Entity" do |ignored_props=[]| context "parsing" do it "reads its own output" do packed_xml = instance.to_xml - parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(packed_xml) + parsed_instance = DiasporaFederation::Entity.entity_class(packed_xml.name).from_xml(packed_xml) check_entity(instance, parsed_instance, ignored_props) end