refactoring to_xml for SignedRetraction and RelayableRetraction

lib/diaspora_federation/entities/relayable_retraction.rb

@@ -53,31 +53,6 @@ module DiasporaFederation
       #   @return [String] target author signature
       property :target_author_signature, default: nil
 
-      # Generates XML and updates signatures
-      # @see Entity#to_xml
-      # @return [Nokogiri::XML::Element] root element containing properties as child elements
-      def to_xml
-        super.tap do |xml|
-          hash = to_h
-          xml.at_xpath("target_author_signature").content = hash[:target_author_signature]
-          xml.at_xpath("parent_author_signature").content = hash[:parent_author_signature]
-        end
-      end
-
-      # Adds signatures to the 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.
-      #
-      # @see Entity#to_h
-      # @return [Hash] entity data hash with updated signatures
-      def to_h
-        target_author = DiasporaFederation.callbacks.trigger(:fetch_entity_author_id_by_guid, target_type, target_guid)
-        privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_diaspora_id, author)
-
-        super.tap do |hash|
-          fill_required_signature(target_author, privkey, hash) unless privkey.nil?
-        end
-      end
-
       # use only {Retraction} for receive
       # @return [Retraction] instance as normal retraction
       def to_retraction
@@ -86,6 +61,19 @@ module DiasporaFederation
 
       private
 
+      # It updates also the signatures with the keys of the author and the parent
+      # if the signatures are not there yet and if the keys are available.
+      #
+      # @return [Hash] xml elements with updated signatures
+      def xml_elements
+        target_author = DiasporaFederation.callbacks.trigger(:fetch_entity_author_id_by_guid, target_type, target_guid)
+        privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_diaspora_id, author)
+
+        super.tap do |xml_elements|
+          fill_required_signature(target_author, privkey, xml_elements) unless privkey.nil?
+        end
+      end
+
       # @param [String] target_author the author of the entity to retract
       # @param [OpenSSL::PKey::RSA] privkey private key of sender
       # @param [Hash] hash hash given for a signing

lib/diaspora_federation/entities/signed_retraction.rb

@@ -30,29 +30,6 @@ module DiasporaFederation
       #   @return [String] author signature
       property :target_author_signature, default: nil
 
-      # Generates XML and updates signatures
-      # @see Entity#to_xml
-      # @return [Nokogiri::XML::Element] root element containing properties as child elements
-      def to_xml
-        super.tap do |xml|
-          xml.at_xpath("target_author_signature").content = to_h[:target_author_signature]
-        end
-      end
-
-      # Adds signature to the hash with the key of the author
-      # if the signature is not in the hash yet and if the key is available.
-      #
-      # @see Entity#to_h
-      # @return [Hash] entity data hash with updated signatures
-      def to_h
-        super.tap do |hash|
-          if target_author_signature.nil?
-            privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_diaspora_id, author)
-            hash[:target_author_signature] = SignedRetraction.sign_with_key(privkey, self) unless privkey.nil?
-          end
-        end
-      end
-
       # use only {Retraction} for receive
       # @return [Retraction] instance as normal retraction
       def to_retraction
@@ -66,6 +43,23 @@ module DiasporaFederation
       def self.sign_with_key(privkey, ret)
         Base64.strict_encode64(privkey.sign(Relayable::DIGEST, [ret.target_guid, ret.target_type].join(";")))
       end
+
+      private
+
+      # It updates also the signatures with the keys of the author and the parent
+      # if the signatures are not there yet and if the keys are available.
+      #
+      # @return [Hash] xml elements with updated signatures
+      def xml_elements
+        super.tap do |xml_elements|
+          xml_elements[:target_author_signature] = target_author_signature || sign_with_author.to_s
+        end
+      end
+
+      def sign_with_author
+        privkey = DiasporaFederation.callbacks.trigger(:fetch_private_key_by_diaspora_id, author)
+        SignedRetraction.sign_with_key(privkey, self) unless privkey.nil?
+      end
     end
   end
 end

spec/lib/diaspora_federation/entities/relayable_retraction_spec.rb

@@ -1,6 +1,10 @@
 module DiasporaFederation
   describe Entities::RelayableRetraction do
-    let(:data) { FactoryGirl.build(:relayable_retraction_entity, author: alice.diaspora_id).to_h }
+    let(:data) {
+      FactoryGirl.build(:relayable_retraction_entity, author: alice.diaspora_id).send(:xml_elements).tap do |data|
+        data[:target_author_signature] = nil
+      end
+    }
 
     let(:xml) {
       <<-XML
@@ -18,7 +22,7 @@ XML
 
     it_behaves_like "an XML Entity"
 
-    describe "#to_h" do
+    describe "#to_xml" do
       let(:author_pkey) { OpenSSL::PKey::RSA.generate(1024) }
       let(:hash) { FactoryGirl.attributes_for(:relayable_retraction_entity) }
 
@@ -33,9 +37,9 @@ XML
 
         signed_string = "#{hash[:target_guid]};#{hash[:target_type]}"
 
-        signed_hash = Entities::RelayableRetraction.new(hash).to_h
+        xml = Entities::RelayableRetraction.new(hash).to_xml
 
-        signature = Base64.decode64(signed_hash[:target_author_signature])
+        signature = Base64.decode64(xml.at_xpath("target_author_signature").text)
         expect(author_pkey.verify(OpenSSL::Digest::SHA256.new, signature, signed_string)).to be_truthy
       end
 
@@ -50,16 +54,19 @@ XML
 
         signed_string = "#{hash[:target_guid]};#{hash[:target_type]}"
 
-        signed_hash = Entities::RelayableRetraction.new(hash).to_h
+        xml = Entities::RelayableRetraction.new(hash).to_xml
 
-        signature = Base64.decode64(signed_hash[:parent_author_signature])
+        signature = Base64.decode64(xml.at_xpath("parent_author_signature").text)
         expect(author_pkey.verify(OpenSSL::Digest::SHA256.new, signature, signed_string)).to be_truthy
       end
 
       it "doesn't change signatures if they are already set" do
         hash.merge!(target_author_signature: "aa", parent_author_signature: "bb")
 
-        expect(Entities::RelayableRetraction.new(hash).to_h).to eq(hash)
+        xml = Entities::RelayableRetraction.new(hash).to_xml
+
+        expect(xml.at_xpath("target_author_signature").text).to eq("aa")
+        expect(xml.at_xpath("parent_author_signature").text).to eq("bb")
       end
 
       it "doesn't change signatures if keys weren't supplied" do
@@ -71,8 +78,9 @@ XML
           :fetch_entity_author_id_by_guid, "Comment", hash[:target_guid]
         ).and_return(hash[:author])
 
-        signed_hash = Entities::RelayableRetraction.new(hash).to_h
+        xml = Entities::RelayableRetraction.new(hash).to_xml
-        expect(signed_hash[:target_author_signature]).to eq(nil)
+        expect(xml.at_xpath("target_author_signature").text).to eq("")
+        expect(xml.at_xpath("parent_author_signature").text).to eq("")
       end
     end
 

spec/lib/diaspora_federation/entities/signed_retraction_spec.rb

@@ -1,6 +1,6 @@
 module DiasporaFederation
   describe Entities::SignedRetraction do
-    let(:data) { FactoryGirl.build(:signed_retraction_entity, author: alice.diaspora_id).to_h }
+    let(:data) { FactoryGirl.build(:signed_retraction_entity, author: alice.diaspora_id).send(:xml_elements) }
 
     let(:xml) {
       <<-XML
@@ -17,7 +17,7 @@ XML
 
     it_behaves_like "an XML Entity"
 
-    describe "#to_h" do
+    describe "#to_xml" do
       let(:author_pkey) { OpenSSL::PKey::RSA.generate(1024) }
       let(:hash) { FactoryGirl.attributes_for(:signed_retraction_entity) }
 
@@ -28,18 +28,18 @@ XML
 
         signed_string = "#{hash[:target_guid]};#{hash[:target_type]}"
 
-        signed_hash = Entities::SignedRetraction.new(hash).to_h
+        xml = Entities::SignedRetraction.new(hash).to_xml
 
-        valid = author_pkey.verify(
+        signature = Base64.decode64(xml.at_xpath("target_author_signature").text)
-          OpenSSL::Digest::SHA256.new, Base64.decode64(signed_hash[:target_author_signature]), signed_string
+        expect(author_pkey.verify(OpenSSL::Digest::SHA256.new, signature, signed_string)).to be_truthy
-        )
-        expect(valid).to be_truthy
       end
 
       it "doesn't change signature if it is already set" do
         hash[:target_author_signature] = "aa"
 
-        expect(Entities::SignedRetraction.new(hash).to_h).to eq(hash)
+        xml = Entities::SignedRetraction.new(hash).to_xml
+
+        expect(xml.at_xpath("target_author_signature").text).to eq("aa")
       end
 
       it "doesn't change signature if a key wasn't supplied" do
@@ -47,8 +47,8 @@ XML
           :fetch_private_key_by_diaspora_id, hash[:author]
         ).and_return(nil)
 
-        signed_hash = Entities::SignedRetraction.new(hash).to_h
+        xml = Entities::SignedRetraction.new(hash).to_xml
-        expect(signed_hash[:author_signature]).to eq(nil)
+        expect(xml.at_xpath("target_author_signature").text).to eq("")
       end
     end