refactored post receiving; only update cache on verfied received objects

app/models/post.rb

@@ -96,46 +96,6 @@ class Post < ActiveRecord::Base
     end
   end
 
-  # @param [User] user The user that is receiving this post.
-  # @param [Person] person The person who dispatched this post to the
-  # @return [void]
-  def receive(user, person)
-    #exists locally, but you dont know about it
-    #does not exsist locally, and you dont know about it
-    #exists_locally?
-    #you know about it, and it is mutable
-    #you know about it, and it is not mutable
-    self.class.transaction do
-      local_post = self.class.where(:guid => self.guid).first
-      if local_post && local_post.author_id == self.author_id
-        known_post = user.find_visible_post_by_id(self.guid, :key => :guid)
-        if known_post
-          if known_post.mutable?
-            known_post.update_attributes(self.attributes)
-          else
-            Rails.logger.info("event=receive payload_type=#{self.class} update=true status=abort sender=#{self.diaspora_handle} reason=immutable existing_post=#{known_post.id}")
-          end
-        else
-          user.contact_for(person).receive_post(local_post)
-          user.notify_if_mentioned(local_post)
-          Rails.logger.info("event=receive payload_type=#{self.class} update=true status=complete sender=#{self.diaspora_handle} existing_post=#{local_post.id}")
-          return local_post
-        end
-      elsif !local_post
-        if self.save
-          user.contact_for(person).receive_post(self)
-          user.notify_if_mentioned(self)
-          Rails.logger.info("event=receive payload_type=#{self.class} update=false status=complete sender=#{self.diaspora_handle}")
-          return self
-        else
-          Rails.logger.info("event=receive payload_type=#{self.class} update=false status=abort sender=#{self.diaspora_handle} reason=#{self.errors.full_messages}")
-        end
-      else
-        Rails.logger.info("event=receive payload_type=#{self.class} update=true status=abort sender=#{self.diaspora_handle} reason='update not from post owner' existing_post=#{self.id}")
-      end
-    end
-  end
-
   def activity_streams?
     false
   end
@@ -173,4 +133,72 @@ class Post < ActiveRecord::Base
     self.triggers_caching? && RedisCache.configured? &&
       RedisCache.acceptable_types.include?(self.type) && user = self.author.owner
   end
+
+
+  # @param [User] user The user that is receiving this post.
+  # @param [Person] person The person who dispatched this post to the
+  # @return [void]
+  def receive(user, person)
+    #exists locally, but you dont know about it
+    #does not exsist locally, and you dont know about it
+    #exists_locally?
+    #you know about it, and it is mutable
+    #you know about it, and it is not mutable
+    self.class.transaction do
+      local_post = persisted_post
+
+      if local_post && verify_persisted_post(local_post)
+        self.receive_persisted(user, person, local_post)
+
+      elsif !local_post
+        self.receive_non_persisted(user, person)
+
+      else
+        Rails.logger.info("event=receive payload_type=#{self.class} update=true status=abort sender=#{self.diaspora_handle} reason='update not from post owner' existing_post=#{self.id}")
+        false
+      end
+    end
+  end
+
+  protected
+
+  # @return [Post,void]
+  def persisted_post
+    self.class.where(:guid => self.guid).first
+  end
+
+  # @return [Boolean]
+  def verify_persisted_post(persisted_post)
+    persisted_post.author_id == self.author_id
+  end
+
+  def receive_persisted(user, person, local_post)
+    known_post = user.find_visible_post_by_id(self.guid, :key => :guid)
+    if known_post
+      if known_post.mutable?
+        known_post.update_attributes(self.attributes)
+        true
+      else
+        Rails.logger.info("event=receive payload_type=#{self.class} update=true status=abort sender=#{self.diaspora_handle} reason=immutable") #existing_post=#{known_post.id}")
+        false
+      end
+    else
+      user.contact_for(person).receive_post(local_post)
+      user.notify_if_mentioned(local_post)
+      Rails.logger.info("event=receive payload_type=#{self.class} update=true status=complete sender=#{self.diaspora_handle}") #existing_post=#{local_post.id}")
+      true
+    end
+  end
+
+  def receive_non_persisted(user, person)
+    if self.save
+      user.contact_for(person).receive_post(self)
+      user.notify_if_mentioned(self)
+      Rails.logger.info("event=receive payload_type=#{self.class} update=false status=complete sender=#{self.diaspora_handle}")
+      true
+    else
+      Rails.logger.info("event=receive payload_type=#{self.class} update=false status=abort sender=#{self.diaspora_handle} reason=#{self.errors.full_messages}")
+      false
+    end
+  end
 end

lib/postzord/receiver.rb

@@ -8,8 +8,9 @@ class Postzord::Receiver
   require File.join(Rails.root, 'lib/postzord/receiver/public')
 
   def perform!
-    receive!
-    update_cache! if cache?
+    if self.receive!
+      self.update_cache! if cache?
+    end
   end
 
   # @return [Boolean]

lib/postzord/receiver/local_batch.rb

@@ -23,6 +23,8 @@ class Postzord::Receiver::LocalBatch < Postzord::Receiver
     # 09/27/11 this is slow
     #socket_to_users if @object.respond_to?(:socket_to_user)
     notify_users
+
+    true
   end
 
   def update_cache!

lib/postzord/receiver/private.rb

@@ -23,7 +23,7 @@ class Postzord::Receiver::Private < Postzord::Receiver
       parse_and_receive(salmon.parsed_data)
     else
       Rails.logger.info("event=receive status=abort recipient=#{@user.diaspora_handle} sender=#{@salmon.author_id} reason='not_verified for key'")
-      nil
+      false
     end
   end
 

lib/postzord/receiver/public.rb

@@ -19,12 +19,13 @@ class Postzord::Receiver::Public < Postzord::Receiver
   # @return [void]
   def receive!
     return false unless verified_signature?
-    return unless save_object
+    return false unless save_object
 
     if @object.respond_to?(:relayable?)
       receive_relayable
     else
       Resque.enqueue(Jobs::ReceiveLocalBatch, @object.class.to_s, @object.id, self.recipient_user_ids)
+      true
     end
   end
 

spec/lib/postzord/receiver/private_spec.rb

@@ -53,28 +53,28 @@ describe Postzord::Receiver::Private do
       @salmon = @zord.instance_variable_get(:@salmon)
     end
 
-    context 'returns nil' do
+    context 'returns false' do
       it 'if the salmon author does not exist' do
         @zord.instance_variable_set(:@sender, nil)
-        @zord.perform!.should be_nil
+        @zord.receive!.should == false
       end
 
       it 'if the author does not match the signature' do
         @zord.instance_variable_set(:@sender, Factory(:person))
-        @zord.perform!.should be_nil
+        @zord.receive!.should == false
       end
     end
 
     context 'returns the sent object' do
       it 'returns the received object on success' do
-        @zord.perform!
+        @zord.receive!
         @zord.instance_variable_get(:@object).should respond_to(:to_diaspora_xml)
       end
     end
 
     it 'parses the salmon object' do
       Diaspora::Parser.should_receive(:from_xml).with(@salmon.parsed_data).and_return(@original_post)
-      @zord.perform!
+      @zord.receive!
     end
   end
 

spec/lib/postzord/receiver/public_spec.rb

@@ -44,6 +44,11 @@ describe Postzord::Receiver::Public do
       @receiver.perform!
     end
 
+    it 'returns false if signature is not verified' do
+      @receiver.should_receive(:verified_signature?).and_return(false)
+      @receiver.perform!.should be_false
+    end
+
     context 'if signature is valid' do
       it 'calls recipient_user_ids' do
         @receiver.should_receive(:recipient_user_ids)

spec/lib/postzord/receiver_spec.rb

@@ -13,7 +13,7 @@ describe Postzord::Receiver do
 
   describe "#perform!" do
     before do
-      @receiver.stub(:receive!)
+      @receiver.stub(:receive!).and_return(true)
     end
 
     it 'calls receive!' do
@@ -22,8 +22,11 @@ describe Postzord::Receiver do
     end
 
     context 'update_cache!' do
-      it "gets called if cache?" do
+      before do
         @receiver.stub(:cache?).and_return(true)
+      end
+
+      it "gets called if cache?" do
         @receiver.should_receive(:update_cache!)
         @receiver.perform!
       end
@@ -33,6 +36,12 @@ describe Postzord::Receiver do
         @receiver.should_not_receive(:update_cache!)
         @receiver.perform!
       end
+
+      it 'does not get called if receive! is false' do
+        @receiver.stub(:receive!).and_return(false)
+        @receiver.should_not_receive(:update_cache!)
+        @receiver.perform!
+      end
     end
   end
 

spec/models/post_spec.rb

@@ -199,9 +199,83 @@ describe Post do
       @post.should_cache_for_author?.should be_false
     end
   end
+  
+  describe "#receive" do
+    it 'returns false if the post does not verify' do
+      @post = Factory(:status_message, :author => bob.person)
+      @post.should_receive(:verify_persisted_post).and_return(false)
+      @post.receive(bob, eve.person).should == false
+    end
+  end
 
- #user exists,
- #  cache configured,
- #  triggers caching,
- #  of the cachable type
+  describe "#receive_persisted" do
+    before do
+      @post = Factory.build(:status_message, :author => bob.person)
+      @known_post = Post.new
+      bob.stub(:contact_for).with(eve.person).and_return(stub(:receive_post => true))
+    end
+
+    context "user knows about the post" do
+      before do
+        bob.stub(:find_visible_post_by_id).and_return(@known_post)
+      end
+
+      it 'updates attributes only if mutable' do
+        @known_post.stub(:mutable?).and_return(true)
+        @known_post.should_receive(:update_attributes)
+        @post.send(:receive_persisted, bob, eve.person, @known_post).should == true
+      end
+      
+      it 'returns false if trying to update a non-mutable object' do
+        @known_post.stub(:mutable?).and_return(false)
+        @known_post.should_not_receive(:update_attributes)
+        @post.send(:receive_persisted, bob, eve.person, @known_post).should == false
+      end
+    end
+
+    context "the user does not know about the post" do
+      before do
+        bob.stub(:find_visible_post_by_id).and_return(nil)
+        bob.stub(:notify_if_mentioned).and_return(true)
+      end
+
+      it "receives the post from the contact of the author" do
+        @post.send(:receive_persisted, bob, eve.person, @known_post).should == true
+      end
+      
+      it 'notifies the user if they are mentioned' do
+        bob.stub(:contact_for).with(eve.person).and_return(stub(:receive_post => true))
+        bob.should_receive(:notify_if_mentioned).and_return(true)
+
+        @post.send(:receive_persisted, bob, eve.person, @known_post).should == true
+      end
+    end
+  end
+
+  describe '#receive_non_persisted' do
+    context "the user does not know about the post" do
+      before do
+        @post = Factory.build(:status_message, :author => bob.person)
+        bob.stub(:find_visible_post_by_id).and_return(nil)
+        bob.stub(:notify_if_mentioned).and_return(true)
+      end
+
+      it "it receives the post from the contact of the author" do
+        bob.should_receive(:contact_for).with(eve.person).and_return(stub(:receive_post => true))
+        @post.send(:receive_non_persisted, bob, eve.person).should == true
+      end
+      
+      it 'notifies the user if they are mentioned' do
+        bob.stub(:contact_for).with(eve.person).and_return(stub(:receive_post => true))
+        bob.should_receive(:notify_if_mentioned).and_return(true)
+
+        @post.send(:receive_non_persisted, bob, eve.person).should == true
+      end
+
+      it 'returns false if the post does not save' do
+        @post.stub(:save).and_return(false)
+        @post.send(:receive_non_persisted, bob, eve.person).should == false
+      end
+    end
+  end
 end