MS DG clean up Postzord::Dispatcher::Private

app/controllers/status_messages_controller.rb

@@ -50,10 +50,9 @@ class StatusMessagesController < ApplicationController
 
       aspects = current_user.aspects_from_ids(params[:aspect_ids])
       current_user.add_to_streams(@status_message, aspects)
-      receiving_services = current_user.services.where( :type => params[:services].map{|s| "Services::"+s.titleize}) if params[:services]
+      receiving_services = current_user.services.where(:type => params[:services].map{|s| "Services::"+s.titleize}) if params[:services]
       current_user.dispatch_post(@status_message, :url => short_post_url(@status_message.guid), :services => receiving_services)
 
-
       if request.env['HTTP_REFERER'].include?("people") # if this is a post coming from a profile page
         flash[:notice] = t('status_messages.create.success', :names => @status_message.mentions.includes(:person => :profile).map{ |mention| mention.person.name }.join(', '))
       end

lib/postzord/dispatcher/private.rb

@@ -3,65 +3,86 @@
 #   the COPYRIGHT file.
 
 class Postzord::Dispatcher::Private
-  # @note Takes :additional_subscribers param to add to subscribers to dispatch to
+
+  attr_reader :sender, :object, :xml, :subscribers
+
+  # @param user [User] User dispatching the object in question
+  # @param object [Object] The object to be sent to other Diaspora installations
+  # @opt additional_subscribers [Array<Person>] Additional subscribers
   def initialize(user, object, opts={})
-    unless object.respond_to? :to_diaspora_xml
-      raise 'this object does not respond_to? to_diaspora xml.  try including Diaspora::Webhooks into your object'
-    end
     @sender = user
-    @sender_person = @sender.person
     @object = object
     @xml = @object.to_diaspora_xml
-    @subscribers = @object.subscribers(@sender)
-    @subscribers = @subscribers | [*opts[:additional_subscribers]] if opts[:additional_subscribers]
+
+    additional_subscribers = opts[:additional_subscribers] || []
+    @subscribers = subscribers_from_object | [*additional_subscribers]
   end
 
-  def salmon
-    @salmon_factory ||= Salmon::EncryptedSlap.create_by_user_and_activity(@sender, @xml)
+  # @return [Object]
+  def post(opts={})
+    self.post_to_subscribers if @subscribers.present?
+    self.deliver_to_services(opts[:url], opts[:services] || [])
+    self.process_after_dispatch_hooks
+    @object
   end
 
-  def post(opts = {})
-    unless @subscribers == nil
-      remote_people, local_people = @subscribers.partition{ |person| person.owner_id.nil? }
-
-      if @object.respond_to?(:relayable?) && @sender.owns?(@object.parent)
-        user_ids = [*local_people].map{|x| x.owner_id }
-        local_users = User.where(:id => user_ids)
-        self.notify_users(local_users)
-        local_users << @sender if @object.author.local?
-        self.socket_to_users(local_users)
-      else
-        self.deliver_to_local(local_people)
-      end
+  protected
 
-      self.deliver_to_remote(remote_people) unless @sender.username == 'diasporahq' #NOTE: 09/08/11 this is temporary (~3days max) till we fix fanout in federation
-    end
-    self.deliver_to_services(opts[:url], opts[:services] || [])
+  # @return [Object]
+  def process_after_dispatch_hooks
     @object.after_dispatch(@sender)
+    @object
   end
 
-  protected
+  def post_to_subscribers
+    remote_people, local_people = @subscribers.partition{ |person| person.owner_id.nil? }
 
+    if @object.respond_to?(:relayable?) && @sender.owns?(@object.parent)
+      self.socket_and_notify_local_users(local_people)
+    else
+      self.deliver_to_local(local_people)
+    end
+
+    self.deliver_to_remote(remote_people) unless @sender.username == 'diasporahq' #NOTE: 09/08/11 this is temporary (~3days max) till we fix fanout in federation
+  end
+
+  # @return [Array<Person>] Recipients of the object, minus any additional subscribers
+  def subscribers_from_object
+    @object.subscribers(@sender)
+  end
+
+  # @param local_people [Array<People>]
+  # @return [ActiveRecord::Association<User>]
+  def fetch_local_users(people)
+    return if people.blank?
+    user_ids = people.map{|x| x.owner_id }
+    User.where(:id => user_ids)
+  end
+
+  # @param people [Array<Person>] Recipients of the post
   def deliver_to_remote(people)
-    Resque.enqueue(Job::HttpMulti, @sender.id, Base64.encode64(@object.to_diaspora_xml), people.map{|p| p.id}) unless people.empty?
+    return if people.blank?
+    Resque.enqueue(Job::HttpMulti, @sender.id, Base64.encode64(@object.to_diaspora_xml), people.map{|p| p.id}) 
   end
 
+  # @param people [Array<Person>] Recipients of the post
   def deliver_to_local(people)
     return if people.blank? || @object.is_a?(Profile)
     if @object.is_a?(Post)
       batch_deliver_to_local(people)
     else
       people.each do |person|
-        Rails.logger.info("event=push route=local sender=#{@sender_person.diaspora_handle} recipient=#{person.diaspora_handle} payload_type=#{@object.class}")
-        Resque.enqueue(Job::Receive, person.owner_id, @xml, @sender_person.id)
+        Rails.logger.info("event=push route=local sender=#{@sender.person.diaspora_handle} recipient=#{person.diaspora_handle} payload_type=#{@object.class}")
+        Resque.enqueue(Job::Receive, person.owner_id, @xml, @sender.person.id)
       end
     end
   end
 
+  # @param people [Array<Person>] Recipients of the post
   def batch_deliver_to_local(people)
     ids = people.map{ |p| p.owner_id }
     Resque.enqueue(Job::ReceiveLocalBatch, @object.id, ids)
-    Rails.logger.info("event=push route=local sender=#{@sender_person.diaspora_handle} recipients=#{ids.join(',')} payload_type=#{@object.class}")
+    Rails.logger.info("event=push route=local sender=#{@sender.person.diaspora_handle} recipients=#{ids.join(',')} payload_type=#{@object.class}")
   end
 
   def deliver_to_hub
@@ -69,6 +90,8 @@ class Postzord::Dispatcher::Private
     Resque.enqueue(Job::PublishToHub, @sender.public_url)
   end
 
+  # @param url [String]
+  # @param services [Array<Service>]
   def deliver_to_services(url, services)
     if @object.respond_to?(:public) && @object.public
       deliver_to_hub
@@ -80,15 +103,26 @@ class Postzord::Dispatcher::Private
     end
   end
 
+  # @param services [Array<User>]
   def socket_and_notify_users(users)
     notify_users(users)
     socket_to_users(users)
   end
 
+  # @param local_people [Array<People>]
+  def socket_and_notify_local_users(local_people)
+    local_users = fetch_local_users(local_people)
+    self.notify_users(local_users)
+    local_users << @sender if @object.author.local?
+    self.socket_to_users(local_users)
+  end
 
+  # @param services [Array<User>]
   def notify_users(users)
     Resque.enqueue(Job::NotifyLocalUsers, users.map{|u| u.id}, @object.class.to_s, @object.id, @object.author.id)
   end
+
+  # @param services [Array<User>]
   def socket_to_users(users)
     return unless @object.respond_to?(:socket_to_user)
     users.each do |user|

spec/lib/postzord/dispatcher/private_spec.rb

@@ -11,22 +11,23 @@ describe Postzord::Dispatcher::Private do
     @sm = Factory(:status_message, :public => true, :author => alice.person)
     @subscribers = []
     5.times{@subscribers << Factory(:person)}
-    @sm.stub!(:subscribers)
+    @sm.stub(:subscribers).and_return(@subscribers)
     @xml = @sm.to_diaspora_xml
   end
 
   describe '.initialize' do
-    it 'takes an sender(User) and object (responds_to #subscibers) and sets then to @sender and @object' do
+    it 'sets @sender, @object, @xml' do
       zord = Postzord::Dispatcher::Private.new(alice, @sm)
-      zord.instance_variable_get(:@sender).should == alice
-      zord.instance_variable_get(:@object).should == @sm
+      zord.sender.should == alice
+      zord.object.should == @sm
+      zord.xml.should == @sm.to_diaspora_xml
     end
 
     context 'setting @subscribers' do 
       it 'sets @subscribers from object' do
         @sm.should_receive(:subscribers).and_return(@subscribers)
         zord = Postzord::Dispatcher::Private.new(alice, @sm)
-        zord.instance_variable_get(:@subscribers).should == @subscribers
+        zord.subscribers.should == @subscribers
       end
 
       it 'accepts additional subscribers from opts' do
@@ -34,32 +35,24 @@ describe Postzord::Dispatcher::Private do
 
         @sm.should_receive(:subscribers).and_return(@subscribers)
         zord = Postzord::Dispatcher::Private.new(alice, @sm, :additional_subscribers => new_person)
-        zord.instance_variable_get(:@subscribers).should == @subscribers | [new_person]
+        zord.subscribers.should == @subscribers | [new_person]
       end
     end
 
-    it 'sets the @sender_person object' do
-      zord = Postzord::Dispatcher::Private.new(alice, @sm)
-      zord.instance_variable_get(:@sender_person).should == alice.person
-    end
-
     it 'raises and gives you a helpful message if the object can not federate' do
-      proc{ Postzord::Dispatcher::Private.new(alice, [])
+      pending "put this in the base class!"
+      expect {
+        Postzord::Dispatcher::Private.new(alice, [])
       }.should raise_error /Diaspora::Webhooks/
     end
   end
 
-  it 'creates a salmon base object' do
-    zord = Postzord::Dispatcher::Private.new(alice, @sm)
-    zord.salmon.should_not be nil
-  end
-
   context 'instance methods' do
     before do
       @subscribers << bob.person
       @remote_people, @local_people = @subscribers.partition{ |person| person.owner_id.nil? }
-      @sm.stub!(:subscribers).and_return @subscribers
-      @zord =  Postzord::Dispatcher::Private.new(alice, @sm)
+
+      @zord = Postzord::Dispatcher::Private.new(alice, @sm)
     end
 
     describe '#post' do
@@ -67,6 +60,7 @@ describe Postzord::Dispatcher::Private do
         @zord.stub!(:socket_and_notify_users)
       end
       it 'calls Array#partition on subscribers' do
+        @zord.instance_variable_set(:@subscribers, @subscribers)
         @subscribers.should_receive(:partition).and_return([@remote_people, @local_people])
         @zord.post
       end
@@ -138,49 +132,58 @@ describe Postzord::Dispatcher::Private do
               @mailman.post
             end
           end
-
         end
+
         context "remote raphael" do
           before do
             @comment = Factory.build(:comment, :author => @remote_raphael, :post => @post)
             @comment.save
             @mailman = Postzord::Dispatcher::Private.new(@local_luke, @comment)
           end
+
           it 'does not call deliver_to_local' do
             @mailman.should_not_receive(:deliver_to_local)
             @mailman.post
           end
+
           it 'calls deliver_to_remote with remote_raphael' do
             @mailman.should_receive(:deliver_to_remote).with([@remote_raphael])
             @mailman.post
           end
+
           it 'calls socket_to_users' do
             @mailman.should_receive(:socket_to_users).with([@local_leia])
             @mailman.post
           end
+
           it 'calls notify_users' do
             @mailman.should_receive(:notify_users).with([@local_leia])
             @mailman.post
           end
         end
+
         context "local luke" do
           before do
             @comment = @local_luke.build_comment :text => "yo", :post => @post
             @comment.save
             @mailman = Postzord::Dispatcher::Private.new(@local_luke, @comment)
           end
+
           it 'does not call deliver_to_local' do
             @mailman.should_not_receive(:deliver_to_local)
             @mailman.post
           end
+
           it 'calls deliver_to_remote with remote_raphael' do
             @mailman.should_receive(:deliver_to_remote).with([@remote_raphael])
             @mailman.post
           end
+
           it 'calls socket_to_users' do
             @mailman.should_receive(:socket_to_users).with([@local_leia, @local_luke])
             @mailman.post
           end
+
           it 'calls notify_users' do
             @mailman.should_receive(:notify_users).with([@local_leia])
             @mailman.post
@@ -195,18 +198,22 @@ describe Postzord::Dispatcher::Private do
           @comment.save
           @mailman = Postzord::Dispatcher::Private.new(@local_luke, @comment)
         end
+
         it 'calls deliver_to_remote with remote_raphael' do
           @mailman.should_receive(:deliver_to_remote).with([@remote_raphael])
           @mailman.post
         end
+
         it 'calls deliver_to_local with nobody' do
           @mailman.should_receive(:deliver_to_local).with([])
           @mailman.post
         end
+
         it 'does not call socket_to_users' do
           @mailman.should_not_receive(:socket_to_users)
           @mailman.post
         end
+
         it 'does not call notify_users' do
           @mailman.should_not_receive(:notify_users)
           @mailman.post
@@ -227,17 +234,6 @@ describe Postzord::Dispatcher::Private do
         Resque.should_receive(:enqueue).with(Job::HttpMulti, alice.id, anything, @remote_people.map{|p| p.id}).once
         @mailman.send(:deliver_to_remote, @remote_people)
       end
-
-      it 'calls salmon_for each remote person' do
-       salmon = @mailman.salmon
-       Salmon::EncryptedSlap.stub(:create_by_user_and_activity).and_return(salmon)
-       salmon.should_receive(:xml_for).with(alice.person).and_return('what')
-       @hydra.stub!(:queue)
-       @hydra.stub!(:run)
-       fantasy_resque do
-         @mailman.send(:deliver_to_remote, @remote_people)
-       end
-      end
     end
 
     describe '#deliver_to_local' do