From e16b9af7e613116af4ae6accd14b00574dbe25bf Mon Sep 17 00:00:00 2001
From: Benjamin Neff <benjamin@coding4coffee.ch>
Date: Sun, 7 Jun 2015 03:18:48 +0200
Subject: [PATCH] make public and private receiver work similar ...

---
 lib/postzord/receiver.rb                   | 19 ++++++---
 lib/postzord/receiver/private.rb           | 47 +++++++++------------
 lib/postzord/receiver/public.rb            | 41 +++++++++---------
 spec/lib/postzord/receiver/private_spec.rb |  8 ++--
 spec/lib/postzord/receiver/public_spec.rb  |  9 ++--
 spec/lib/postzord/receiver_spec.rb         | 49 ++++++++++++++++++++--
 spec/models/poll_participation_spec.rb     |  4 +-
 7 files changed, 111 insertions(+), 66 deletions(-)

diff --git a/lib/postzord/receiver.rb b/lib/postzord/receiver.rb
index deb44216fd..ecdf02f6cf 100644
--- a/lib/postzord/receiver.rb
+++ b/lib/postzord/receiver.rb
@@ -14,12 +14,19 @@ class Postzord::Receiver
     self.receive!
   end
 
+  private
+
   def author_does_not_match_xml_author?
-    if (@author.diaspora_handle != xml_author)
-      logger.warn "event=receive status=abort reason='author in xml does not match retrieved person' " \
-                  "payload_type=#{@object.class} sender=#{@author.diaspora_handle}"
-      return true
-    end
+    return false unless @author.diaspora_handle != xml_author
+    logger.error "event=receive status=abort reason='author in xml does not match retrieved person' " \
+                 "type=#{@object.class} sender=#{@author.diaspora_handle}"
+    true
   end
-end
 
+  def relayable_without_parent?
+    return false unless @object.respond_to?(:relayable?) && @object.parent.nil?
+    logger.error "event=receive status=abort reason='no corresponding post' type=#{@object.class} " \
+                 "sender=#{@author.diaspora_handle}"
+    true
+  end
+end
diff --git a/lib/postzord/receiver/private.rb b/lib/postzord/receiver/private.rb
index 98f211d91f..378380b857 100644
--- a/lib/postzord/receiver/private.rb
+++ b/lib/postzord/receiver/private.rb
@@ -9,30 +9,28 @@ class Postzord::Receiver::Private < Postzord::Receiver
     @user_person = @user.person
     @salmon_xml = opts[:salmon_xml]
 
-    @sender = opts[:person] || Webfinger.new(self.salmon.author_id).fetch
-    @author = @sender
+    @author = opts[:person] || Webfinger.new(salmon.author_id).fetch
 
     @object = opts[:object]
   end
 
   def receive!
-    if @sender && self.salmon.verified_for_key?(@sender.public_key)
+    if @author && salmon.verified_for_key?(@author.public_key)
       parse_and_receive(salmon.parsed_data)
     else
       logger.error "event=receive status=abort reason='not_verified for key' " \
                    "recipient=#{@user.diaspora_handle} sender=#{@salmon.author_id}"
     end
   rescue => e
-    logger.error "failed to receive #{@object.class} from sender:#{@sender.id} for user:#{@user.id}: #{e.message}\n" \
+    logger.error "failed to receive #{@object.class} from sender:#{@author.id} for user:#{@user.id}: #{e.message}\n" \
                  "#{@object.inspect}"
     raise e
   end
 
   def parse_and_receive(xml)
     @object ||= Diaspora::Parser.from_xml(xml)
-    return if @object.nil?
 
-    logger.info "user:#{@user.id} starting private receive from person:#{@sender.guid}"
+    logger.info "user:#{@user.id} starting private receive from person:#{@author.guid}"
 
     validate_object
     set_author!
@@ -43,27 +41,17 @@ class Postzord::Receiver::Private < Postzord::Receiver
   def receive_object
     obj = @object.receive(@user, @author)
     Notification.notify(@user, obj, @author) if obj.respond_to?(:notification_type)
-    logger.info "user:#{@user.id} successfully received #{@object.class} from person #{@sender.guid}" \
+    logger.info "user:#{@user.id} successfully received #{@object.class} from person #{@author.guid}" \
                 "#{": #{@object.guid}" if @object.respond_to?(:guid)}"
     logger.debug "received: #{@object.inspect}"
   end
 
   protected
+
   def salmon
     @salmon ||= Salmon::EncryptedSlap.from_xml(@salmon_xml, @user)
   end
 
-  def validate_object
-    raise Diaspora::ContactRequiredUnlessRequest if contact_required_unless_request
-    raise Diaspora::RelayableObjectWithoutParent if relayable_without_parent?
-
-    assign_sender_handle_if_request
-
-    raise Diaspora::AuthorXMLAuthorMismatch if author_does_not_match_xml_author?
-
-    @object
-  end
-
   def xml_author
     if @object.respond_to?(:relayable?)
       #if A and B are friends, and A sends B a comment from C, we delegate the validation to the owner of the post being commented on
@@ -84,19 +72,22 @@ class Postzord::Receiver::Private < Postzord::Receiver
 
   private
 
-  #validations
-  def relayable_without_parent?
-    if @object.respond_to?(:relayable?) && @object.parent.nil?
-      logger.error "event=receive status=abort reason='no corresponding post' type=#{@object.class} " \
-                   "recipient=#{@user_person.diaspora_handle} sender=#{@sender.diaspora_handle}"
-      return true
-    end
+  # validations
+
+  def validate_object
+    raise Diaspora::XMLNotParseable if @object.nil?
+    raise Diaspora::ContactRequiredUnlessRequest if contact_required_unless_request
+    raise Diaspora::RelayableObjectWithoutParent if relayable_without_parent?
+
+    assign_sender_handle_if_request
+
+    raise Diaspora::AuthorXMLAuthorMismatch if author_does_not_match_xml_author?
   end
 
   def contact_required_unless_request
-    unless @object.is_a?(Request) || @user.contact_for(@sender)
+    unless @object.is_a?(Request) || @user.contact_for(@author)
       logger.error "event=receive status=abort reason='sender not connected to recipient' type=#{@object.class} " \
-                   "recipient=#{@user_person.diaspora_handle} sender=#{@sender.diaspora_handle}"
+                   "recipient=#{@user_person.diaspora_handle} sender=#{@author.diaspora_handle}"
       return true
     end
   end
@@ -104,7 +95,7 @@ class Postzord::Receiver::Private < Postzord::Receiver
   def assign_sender_handle_if_request
     #special casey
     if @object.is_a?(Request)
-      @object.sender_handle = @sender.diaspora_handle
+      @object.sender_handle = @author.diaspora_handle
     end
   end
 end
diff --git a/lib/postzord/receiver/public.rb b/lib/postzord/receiver/public.rb
index afd9622af5..29ba0491b3 100644
--- a/lib/postzord/receiver/public.rb
+++ b/lib/postzord/receiver/public.rb
@@ -9,8 +9,6 @@ class Postzord::Receiver::Public < Postzord::Receiver
   def initialize(xml)
     @salmon = Salmon::Slap.from_xml(xml)
     @author = Webfinger.new(@salmon.author_id).fetch
-
-    logger.info "Receiving public object from person:#{@author.id}"
   end
 
   # @return [Boolean]
@@ -23,7 +21,7 @@ class Postzord::Receiver::Public < Postzord::Receiver
     return unless verified_signature?
     # return false unless account_deletion_is_from_author
 
-    return unless save_object
+    parse_and_receive(@salmon.parsed_data)
 
     logger.info "received a #{@object.inspect}"
     if @object.is_a?(SignedRetraction) # feels like a hack
@@ -51,15 +49,19 @@ class Postzord::Receiver::Public < Postzord::Receiver
     receiver.notify_users
   end
 
-  # @return [Object]
-  def save_object
-    @object = Diaspora::Parser.from_xml(@salmon.parsed_data)
-    raise Diaspora::XMLNotParseable if @object.nil?
-    raise Diaspora::NonPublic if object_can_be_public_and_it_is_not?
-    raise Diaspora::RelayableObjectWithoutParent if object_must_have_parent_and_does_not?
-    raise Diaspora::AuthorXMLAuthorMismatch if author_does_not_match_xml_author?
+  # @return [void]
+  def parse_and_receive(xml)
+    @object = Diaspora::Parser.from_xml(xml)
+
+    logger.info "starting public receive from person:#{@author.guid}"
+
+    validate_object
+    receive_object
+  end
+
+  # @return [void]
+  def receive_object
     @object.save! if @object.respond_to?(:save!)
-    @object
   end
 
   # @return [Array<Integer>] User ids
@@ -78,6 +80,15 @@ class Postzord::Receiver::Public < Postzord::Receiver
 
   private
 
+  # validations
+
+  def validate_object
+    raise Diaspora::XMLNotParseable if @object.nil?
+    raise Diaspora::NonPublic if object_can_be_public_and_it_is_not?
+    raise Diaspora::RelayableObjectWithoutParent if relayable_without_parent?
+    raise Diaspora::AuthorXMLAuthorMismatch if author_does_not_match_xml_author?
+  end
+
   def account_deletion_is_from_author
     return true unless @object.is_a?(AccountDeletion)
     return false if @object.diaspora_handle != @author.diaspora_handle
@@ -88,12 +99,4 @@ class Postzord::Receiver::Public < Postzord::Receiver
   def object_can_be_public_and_it_is_not?
     @object.respond_to?(:public) && !@object.public?
   end
-
-  def object_must_have_parent_and_does_not?
-    if @object.respond_to?(:relayable?) # comment, like
-      @object.parent.nil?
-    else
-      false
-    end
-  end
 end
diff --git a/spec/lib/postzord/receiver/private_spec.rb b/spec/lib/postzord/receiver/private_spec.rb
index 2e54f861c3..717dbe8c13 100644
--- a/spec/lib/postzord/receiver/private_spec.rb
+++ b/spec/lib/postzord/receiver/private_spec.rb
@@ -18,7 +18,7 @@ describe Postzord::Receiver::Private do
 
       zord = Postzord::Receiver::Private.new(bob, :person => alice.person, :object => @alices_post)
       expect(zord.instance_variable_get(:@user)).not_to be_nil
-      expect(zord.instance_variable_get(:@sender)).not_to be_nil
+      expect(zord.instance_variable_get(:@author)).not_to be_nil
       expect(zord.instance_variable_get(:@object)).not_to be_nil
     end
 
@@ -32,7 +32,7 @@ describe Postzord::Receiver::Private do
 
       zord = Postzord::Receiver::Private.new(bob, :salmon_xml => @salmon_xml)
       expect(zord.instance_variable_get(:@user)).not_to be_nil
-      expect(zord.instance_variable_get(:@sender)).not_to be_nil
+      expect(zord.instance_variable_get(:@author)).not_to be_nil
       expect(zord.instance_variable_get(:@salmon_xml)).not_to be_nil
     end
   end
@@ -45,13 +45,13 @@ describe Postzord::Receiver::Private do
 
     context "does not parse and receive" do
       it "if the salmon author does not exist" do
-        @zord.instance_variable_set(:@sender, nil)
+        @zord.instance_variable_set(:@author, nil)
         expect(@zord).not_to receive(:parse_and_receive)
         @zord.receive!
       end
 
       it "if the author does not match the signature" do
-        @zord.instance_variable_set(:@sender, FactoryGirl.create(:person))
+        @zord.instance_variable_set(:@author, FactoryGirl.create(:person))
         expect(@zord).not_to receive(:parse_and_receive)
         @zord.receive!
       end
diff --git a/spec/lib/postzord/receiver/public_spec.rb b/spec/lib/postzord/receiver/public_spec.rb
index f4aadcc0f5..090d67358a 100644
--- a/spec/lib/postzord/receiver/public_spec.rb
+++ b/spec/lib/postzord/receiver/public_spec.rb
@@ -47,7 +47,7 @@ describe Postzord::Receiver::Public do
 
     it "does not save the object if signature is not verified" do
       expect(@receiver).to receive(:verified_signature?).and_return(false)
-      expect(@receiver).not_to receive(:save_object)
+      expect(@receiver).not_to receive(:parse_and_receive)
       @receiver.perform!
     end
 
@@ -58,7 +58,7 @@ describe Postzord::Receiver::Public do
       end
 
       it 'saves the parsed object' do
-        expect(@receiver).to receive(:save_object)
+        expect(@receiver).to receive(:parse_and_receive).and_call_original
         @receiver.perform!
       end
 
@@ -120,14 +120,15 @@ describe Postzord::Receiver::Public do
     end
   end
 
-  describe "#save_object" do
+  describe "#parse_and_receive" do
     before do
       @receiver = Postzord::Receiver::Public.new(@xml)
+      @parsed_salmon = Salmon::Slap.from_xml(@xml)
     end
 
     it "should raise a Diaspora::XMLNotParseable when the parsed object is nil" do
       expect(Diaspora::Parser).to receive(:from_xml).and_return(nil)
-      expect { @receiver.save_object }.to raise_error(Diaspora::XMLNotParseable)
+      expect { @receiver.parse_and_receive(@parsed_salmon.parsed_data) }.to raise_error(Diaspora::XMLNotParseable)
     end
   end
 end
diff --git a/spec/lib/postzord/receiver_spec.rb b/spec/lib/postzord/receiver_spec.rb
index a7dca6b1de..9dc2ceac71 100644
--- a/spec/lib/postzord/receiver_spec.rb
+++ b/spec/lib/postzord/receiver_spec.rb
@@ -2,7 +2,7 @@
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 
-require 'spec_helper'
+require "spec_helper"
 
 describe Postzord::Receiver do
   before do
@@ -14,10 +14,53 @@ describe Postzord::Receiver do
       allow(@receiver).to receive(:receive!).and_return(true)
     end
 
-    it 'calls receive!' do
+    it "calls receive!" do
       expect(@receiver).to receive(:receive!)
       @receiver.perform!
     end
   end
-end
 
+  describe "#author_does_not_match_xml_author?" do
+    before do
+      @receiver.instance_variable_set(:@author, alice.person)
+      allow(@receiver).to receive(:xml_author).and_return(alice.diaspora_handle)
+    end
+
+    it "should return false if the author matches" do
+      allow(@receiver).to receive(:xml_author).and_return(alice.diaspora_handle)
+      expect(@receiver.send(:author_does_not_match_xml_author?)).to be_falsey
+    end
+
+    it "should return true if the author does not match" do
+      allow(@receiver).to receive(:xml_author).and_return(bob.diaspora_handle)
+      expect(@receiver.send(:author_does_not_match_xml_author?)).to be_truthy
+    end
+  end
+
+  describe "#relayable_without_parent?" do
+    before do
+      @receiver.instance_variable_set(:@author, alice.person)
+    end
+
+    it "should return false if object is not relayable" do
+      @receiver.instance_variable_set(:@object, nil)
+      expect(@receiver.send(:relayable_without_parent?)).to be_falsey
+    end
+
+    context "if object is relayable" do
+      before do
+        @comment = bob.build_comment(text: "yo", post: FactoryGirl.create(:status_message))
+        @receiver.instance_variable_set(:@object, @comment)
+      end
+
+      it "should return false if object has parent" do
+        expect(@receiver.send(:relayable_without_parent?)).to be_falsey
+      end
+
+      it "should return true if object has no parent" do
+        @comment.parent = nil
+        expect(@receiver.send(:relayable_without_parent?)).to be_truthy
+      end
+    end
+  end
+end
diff --git a/spec/models/poll_participation_spec.rb b/spec/models/poll_participation_spec.rb
index 3caa16e250..d56d25e27a 100644
--- a/spec/models/poll_participation_spec.rb
+++ b/spec/models/poll_participation_spec.rb
@@ -2,7 +2,6 @@ require 'spec_helper'
 require Rails.root.join("spec", "shared_behaviors", "relayable")
 
 describe PollParticipation, :type => :model do
-  
   before do
     @alices_aspect = alice.aspects.first
     @status = bob.post(:status_message, :text => "hello", :to => bob.aspects.first.id)
@@ -98,7 +97,8 @@ describe PollParticipation, :type => :model do
       allow_any_instance_of(Person).to receive(:local?).and_return(false)
       expect{
         salmon = Salmon::Slap.create_by_user_and_activity(alice, @poll_participation_alice.to_diaspora_xml).xml_for(@poll_participant)
-        Postzord::Receiver::Public.new(salmon).save_object
+        parsed_salmon = Salmon::Slap.from_xml(salmon)
+        Postzord::Receiver::Public.new(salmon).parse_and_receive(parsed_salmon.parsed_data)
       }.to_not raise_error
     end
   end
-- 
GitLab