From 5258a37ce5cdd03f68349a594ebb0b43a90d0181 Mon Sep 17 00:00:00 2001
From: Maxwell Salzberg <maxwell@joindiaspora.com>
Date: Mon, 12 Sep 2011 19:29:48 -0700
Subject: [PATCH 1/3] MS DG clean up Postzord::Dispatcher::Private

---
 app/controllers/status_messages_controller.rb |   3 +-
 lib/postzord/dispatcher/private.rb            | 100 ++++++++++++------
 spec/lib/postzord/dispatcher/private_spec.rb  |  58 +++++-----
 3 files changed, 95 insertions(+), 66 deletions(-)

diff --git a/app/controllers/status_messages_controller.rb b/app/controllers/status_messages_controller.rb
index fbe07f57d..6f9b084b6 100644
--- a/app/controllers/status_messages_controller.rb
+++ b/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
diff --git a/lib/postzord/dispatcher/private.rb b/lib/postzord/dispatcher/private.rb
index 140f63129..370c6f5c4 100644
--- a/lib/postzord/dispatcher/private.rb
+++ b/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)
-  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
-
-      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 [Object]
+  def post(opts={})
+    self.post_to_subscribers if @subscribers.present?
     self.deliver_to_services(opts[:url], opts[:services] || [])
-    @object.after_dispatch(@sender)
+    self.process_after_dispatch_hooks
+    @object
   end
 
   protected
 
-  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 [Object]
+  def process_after_dispatch_hooks
+    @object.after_dispatch(@sender)
+    @object
   end
 
+  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)
+    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|
diff --git a/spec/lib/postzord/dispatcher/private_spec.rb b/spec/lib/postzord/dispatcher/private_spec.rb
index 85ae8bc52..de95844a5 100644
--- a/spec/lib/postzord/dispatcher/private_spec.rb
+++ b/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

From dd1b869705d69807571c6267a5803c1a8b82eb9b Mon Sep 17 00:00:00 2001
From: Maxwell Salzberg <maxwell@joindiaspora.com>
Date: Mon, 12 Sep 2011 20:29:18 -0700
Subject: [PATCH 2/3] wip

---
 lib/diaspora/relayable.rb        | 9 ++++++++-
 lib/postzord/dispatcher.rb       | 1 -
 lib/postzord/receiver/private.rb | 2 ++
 3 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/lib/diaspora/relayable.rb b/lib/diaspora/relayable.rb
index d5b85263a..cc80a3001 100644
--- a/lib/diaspora/relayable.rb
+++ b/lib/diaspora/relayable.rb
@@ -35,10 +35,14 @@ module Diaspora
         self.parent.subscribers(user)
       elsif user.owns?(self)
         [self.parent.author]
+      else
+        raise "What are you doing with a relayable that you have nothing to do with?"
+        #[] 
       end
     end
 
     def receive(user, person)
+
       self.class.transaction do
         comment_or_like = self.class.where(:guid => self.guid).first || self
 
@@ -57,11 +61,14 @@ module Diaspora
 
         #dispatch object DOWNSTREAM, received it via UPSTREAM
         unless user.owns?(comment_or_like)
-          comment_or_like.save 
+          puts "i am #{user.username}, I am reiveiving and object for #{person.owner.username}"
+          pp self
+          comment_or_like.save!
           Postzord::Dispatcher.new(user, comment_or_like).post
         end
 
         comment_or_like.socket_to_user(user) if comment_or_like.respond_to? :socket_to_user
+
         if comment_or_like.after_receive(user, person)
           comment_or_like 
         end
diff --git a/lib/postzord/dispatcher.rb b/lib/postzord/dispatcher.rb
index d1178258d..7b3c5255b 100644
--- a/lib/postzord/dispatcher.rb
+++ b/lib/postzord/dispatcher.rb
@@ -21,6 +21,5 @@ class Postzord::Dispatcher
      @zord =  Postzord::Dispatcher::Private.new(user, object, opts)
     #end
   end
-
 end
 
diff --git a/lib/postzord/receiver/private.rb b/lib/postzord/receiver/private.rb
index 18ba4dfb9..521be7b79 100644
--- a/lib/postzord/receiver/private.rb
+++ b/lib/postzord/receiver/private.rb
@@ -33,6 +33,8 @@ module Postzord
         Rails.logger.info("event=receive status=start recipient=#{@user_person.diaspora_handle} payload_type=#{@object.class} sender=#{@sender.diaspora_handle}")
         if self.validate_object
           receive_object
+        else
+          raise 'not a valid object'
         end
       end
 

From adaefd0cb548d26661a8cd43fba7f0d96f92281d Mon Sep 17 00:00:00 2001
From: Maxwell Salzberg <maxwell@joindiaspora.com>
Date: Tue, 13 Sep 2011 12:48:22 -0700
Subject: [PATCH 3/3] MS DG rspec is green update gemfile

---
 Gemfile.lock                                 |   3 +
 lib/diaspora/relayable.rb                    |  10 +-
 lib/diaspora/webhooks.rb                     |  17 +-
 lib/postzord/receiver/private.rb             |   2 +
 spec/controllers/comments_controller_spec.rb |  74 ++++----
 spec/integration/attack_vectors_spec.rb      | 186 +++++++++----------
 spec/mailers/notifier_spec.rb                |  66 +++----
 spec/models/comment_spec.rb                  |  21 +--
 8 files changed, 189 insertions(+), 190 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 3b934cdab..912d92173 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -241,6 +241,8 @@ GEM
       spruz (~> 0.2.8)
     jwt (0.1.3)
       json (>= 1.2.4)
+    kaminari (0.12.4)
+      rails (>= 3.0.0)
     linecache (0.43)
     linecache19 (0.5.12)
       ruby_core_source (>= 0.1.4)
@@ -504,6 +506,7 @@ DEPENDENCIES
   jasmine (= 1.1.0.rc3)
   json (= 1.4.6)
   jwt (= 0.1.3)
+  kaminari
   linecache (= 0.43)
   mini_magick (= 3.2)
   mobile-fu
diff --git a/lib/diaspora/relayable.rb b/lib/diaspora/relayable.rb
index cc80a3001..430518218 100644
--- a/lib/diaspora/relayable.rb
+++ b/lib/diaspora/relayable.rb
@@ -18,10 +18,12 @@ module Diaspora
       end
     end
 
+    # @return [Boolean] true
     def relayable?
       true
     end
 
+    # @return [String]
     def parent_guid
       self.parent.guid
     end
@@ -30,14 +32,14 @@ module Diaspora
       self.parent = parent_class.where(:guid => new_parent_guid).first
     end
 
+    # @return [Array<Person>]
     def subscribers(user)
       if user.owns?(self.parent)
         self.parent.subscribers(user)
       elsif user.owns?(self)
         [self.parent.author]
       else
-        raise "What are you doing with a relayable that you have nothing to do with?"
-        #[] 
+        []
       end
     end
 
@@ -55,14 +57,11 @@ module Diaspora
         #as the owner of the post being liked or commented on, you need to add your own signature in order to pass it to the people who received your original post
         if user.owns? comment_or_like.parent
           comment_or_like.parent_author_signature = comment_or_like.sign_with_key(user.encryption_key)
-
           comment_or_like.save!
         end
 
         #dispatch object DOWNSTREAM, received it via UPSTREAM
         unless user.owns?(comment_or_like)
-          puts "i am #{user.username}, I am reiveiving and object for #{person.owner.username}"
-          pp self
           comment_or_like.save!
           Postzord::Dispatcher.new(user, comment_or_like).post
         end
@@ -75,6 +74,7 @@ module Diaspora
       end
     end
 
+    # @return [Object]
     def after_receive(user, person)
       self
     end
diff --git a/lib/diaspora/webhooks.rb b/lib/diaspora/webhooks.rb
index d342d8ef8..083899e52 100644
--- a/lib/diaspora/webhooks.rb
+++ b/lib/diaspora/webhooks.rb
@@ -1,4 +1,4 @@
-#   Copyright (c) 2010, Diaspora Inc.  This file is
+#   Copyright (c) 2011, Diaspora Inc.  This file is
 #   licensed under the Affero General Public License version 3 or later.  See
 #   the COPYRIGHT file.
 
@@ -7,9 +7,11 @@ module Diaspora
     require 'builder/xchar'
 
     def to_diaspora_xml
-      xml = "<XML>"
-      xml += "<post>#{to_xml.to_s}</post>"
-      xml += "</XML>"
+      <<XML
+      <XML>
+      <post>#{to_xml.to_s}</post>
+      </XML>
+XML
     end
 
     def x(input)
@@ -17,16 +19,19 @@ module Diaspora
     end
 
     # @abstract
+    # @note this must return [Array<Person>]
+    # @return [Array<Person>]
     def subscribers(user)
-      raise 'you must override subscribers in order to enable federation on this model'
+      raise 'You must override subscribers in order to enable federation on this model'
     end
 
     # @abstract
     def receive(user, person)
-      raise 'you must override receive in order to enable federation on this model'
+      raise 'You must override receive in order to enable federation on this model'
     end
 
     # @param [User] sender
+    # @note this is a hook
     def after_dispatch sender
     end
   end
diff --git a/lib/postzord/receiver/private.rb b/lib/postzord/receiver/private.rb
index 521be7b79..6363e0d62 100644
--- a/lib/postzord/receiver/private.rb
+++ b/lib/postzord/receiver/private.rb
@@ -30,7 +30,9 @@ module Postzord
 
       def parse_and_receive(xml)
         @object ||= Diaspora::Parser.from_xml(xml)
+
         Rails.logger.info("event=receive status=start recipient=#{@user_person.diaspora_handle} payload_type=#{@object.class} sender=#{@sender.diaspora_handle}")
+
         if self.validate_object
           receive_object
         else
diff --git a/spec/controllers/comments_controller_spec.rb b/spec/controllers/comments_controller_spec.rb
index cf7a67189..2c3a562e1 100644
--- a/spec/controllers/comments_controller_spec.rb
+++ b/spec/controllers/comments_controller_spec.rb
@@ -6,9 +6,6 @@ require 'spec_helper'
 
 describe CommentsController do
   before do
-    @aspect1 = alice.aspects.where(:name => "generic").first
-    @aspect2 = bob.aspects.where(:name => "generic").first
-
     @controller.stub(:current_user).and_return(alice)
     sign_in :user, alice
   end
@@ -21,7 +18,8 @@ describe CommentsController do
 
     context "on my own post" do
       before do
-        @post = alice.post :status_message, :text => 'GIANTS', :to => @aspect1.id
+        aspect_to_post = alice.aspects.where(:name => "generic").first
+        @post = alice.post :status_message, :text => 'GIANTS', :to => aspect_to_post
       end
 
       it 'responds to format js' do
@@ -37,7 +35,8 @@ describe CommentsController do
 
     context "on a post from a contact" do
       before do
-        @post = bob.post :status_message, :text => 'GIANTS', :to => @aspect2.id
+        aspect_to_post = bob.aspects.where(:name => "generic").first
+        @post = bob.post :status_message, :text => 'GIANTS', :to => aspect_to_post
       end
 
       it 'comments' do
@@ -60,61 +59,64 @@ describe CommentsController do
       end
     end
 
-    context 'on a post from a stranger' do
-      before do
-        @post = eve.post :status_message, :text => 'GIANTS', :to => eve.aspects.first.id
-      end
+    it 'posts no comment on a post from a stranger' do
+      aspect_to_post = eve.aspects.where(:name => "generic").first
+      @post = eve.post :status_message, :text => 'GIANTS', :to => aspect_to_post
 
-      it 'posts no comment' do
-        alice.should_not_receive(:comment)
-        post :create, comment_hash
-        response.code.should == '422'
-      end
+      alice.should_not_receive(:comment)
+      post :create, comment_hash
+      response.code.should == '422'
     end
   end
 
   describe '#destroy' do
+    before do
+      aspect_to_post = bob.aspects.where(:name => "generic").first
+      @message = bob.post(:status_message, :text => "hey", :to => aspect_to_post)
+    end
+
     context 'your post' do
       before do
-        @message = alice.post(:status_message, :text => "hey", :to => @aspect1.id)
-        @comment = alice.comment("hey", :post => @message)
-        @comment2 = bob.comment("hey", :post => @message)
-        @comment3 = eve.comment("hey", :post => @message)
+        @controller.stub(:current_user).and_return(bob)
+        sign_in :user, bob
       end
 
       it 'lets the user delete his comment' do
-        alice.should_receive(:retract).with(@comment)
-        delete :destroy, :format => "js", :post_id => 1,  :id => @comment.id
+        comment = bob.comment("hey", :post => @message)
+
+        bob.should_receive(:retract).with(comment)
+        delete :destroy, :format => "js", :post_id => 1,  :id => comment.id
         response.status.should == 204
       end
 
       it "lets the user destroy other people's comments" do
-        alice.should_receive(:retract).with(@comment2)
-        delete :destroy, :format => "js", :post_id => 1,  :id => @comment2.id
+        comment = alice.comment("hey", :post => @message)
+
+        bob.should_receive(:retract).with(comment)
+        delete :destroy, :format => "js", :post_id => 1,  :id => comment.id
         response.status.should == 204
       end
     end
 
     context "another user's post" do
-      before do
-        @message = bob.post(:status_message, :text => "hey", :to => bob.aspects.first.id)
-        @comment = alice.comment("hey", :post => @message)
-        @comment2 = bob.comment("hey", :post => @message)
-        @comment3 = eve.comment("hey", :post => @message)
-      end
-
       it 'let the user delete his comment' do
-        alice.should_receive(:retract).with(@comment)
-        delete :destroy, :format => "js", :post_id => 1,  :id => @comment.id
+        comment = alice.comment("hey", :post => @message)
+
+        alice.should_receive(:retract).with(comment)
+        delete :destroy, :format => "js", :post_id => 1,  :id => comment.id
         response.status.should == 204
       end
 
       it 'does not let the user destroy comments he does not own' do
-        alice.should_not_receive(:retract).with(@comment2)
-        delete :destroy, :format => "js", :post_id => 1,  :id => @comment3.id
+        comment1 = bob.comment("hey", :post => @message)
+        comment2 = eve.comment("hey", :post => @message)
+
+        alice.should_not_receive(:retract).with(comment1)
+        delete :destroy, :format => "js", :post_id => 1,  :id => comment2.id
         response.status.should == 403
       end
     end
+
     it 'renders nothing and 404 on a nonexistent comment' do
       delete :destroy, :post_id => 1, :id => 343415
       response.status.should == 404
@@ -126,7 +128,6 @@ describe CommentsController do
     before do
       aspect_to_post = bob.aspects.where(:name => "generic").first
       @message = bob.post(:status_message, :text => "hey", :to => aspect_to_post.id)
-      @comments = [alice, bob, eve].map{ |u| u.comment("hey", :post => @message) }
     end
 
     it 'generates a jasmine fixture', :fixture => true do
@@ -142,9 +143,12 @@ describe CommentsController do
     end
 
     it 'returns all the comments for a post' do
+      comments = [alice, bob, eve].map{ |u| u.comment("hey", :post => @message) }
+
       get :index, :post_id => @message.id, :format => 'js'
-      assigns[:comments].should == @comments
+      assigns[:comments].should == comments
     end
+
     it 'returns a 404 on a nonexistent post' do
       get :index, :post_id => 235236, :format => 'js'
       response.status.should == 404
diff --git a/spec/integration/attack_vectors_spec.rb b/spec/integration/attack_vectors_spec.rb
index 48843e6a6..eb0c38ac1 100644
--- a/spec/integration/attack_vectors_spec.rb
+++ b/spec/integration/attack_vectors_spec.rb
@@ -6,221 +6,215 @@ require 'spec_helper'
 
 describe "attack vectors" do
 
-  let(:user) { Factory.create(:user_with_aspect) }
-  let(:aspect) { user.aspects.first }
-
-  let(:bad_user) { Factory.create(:user)}
-
-  let(:user2) { eve }
-  let(:aspect2) { user2.aspects.first }
-
-  let(:user3) { Factory.create(:user) }
-  let(:aspect3) { user3.aspects.create(:name => 'heroes') }
+  let(:eves_aspect) { eve.aspects.find_by_name("generic") }
+  let(:alices_aspect) { alice.aspects.find_by_name("generic") }
 
   context 'non-contact valid user' do
-
     it 'does not save a post from a non-contact' do
+      bad_user = Factory(:user)
+
       post_from_non_contact = bad_user.build_post( :status_message, :text => 'hi')
-      salmon_xml = bad_user.salmon(post_from_non_contact).xml_for(user.person)
+      salmon_xml = bad_user.salmon(post_from_non_contact).xml_for(bob.person)
 
       post_from_non_contact.delete
       bad_user.delete
       post_count = Post.count
 
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
-      zord.perform
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
+      expect {
+        zord.perform
+      }.should raise_error /not a valid object/
 
-      user.visible_posts.include?(post_from_non_contact).should be_false
+      bob.visible_posts.include?(post_from_non_contact).should be_false
       Post.count.should == post_count
     end
-
   end
 
   it 'does not let a user attach to posts previously in the db unless its received from the author' do
-    connect_users(user, aspect, user3, aspect3)
+    original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
+    original_message.diaspora_handle = bob.diaspora_handle
 
-    original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+    alice.contacts.create(:person => eve.person, :aspects => [alice.aspects.first])
 
-    original_message.diaspora_handle = user.diaspora_handle
+    salmon_xml = bob.salmon(original_message).xml_for(alice.person)
+    zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
+    expect {
+      zord.perform
+    }.should raise_error /not a valid object/
 
-    user3.contacts.create(:person => user2.person, :aspects => [user3.aspects.first])
-
-    salmon_xml = user.salmon(original_message).xml_for(user3.person)
-    zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
-    zord.perform
-
-    user3.reload.visible_posts.should_not include(StatusMessage.find(original_message.id))
+    alice.reload.visible_posts.should_not include(StatusMessage.find(original_message.id))
   end
 
   context 'malicious contact attack vector' do
-    before do
-      connect_users(user, aspect, user2, aspect2)
-      connect_users(user, aspect, user3, aspect3)
-    end
-
     describe 'mass assignment on id' do
       it "does not save a message over an old message with a different author" do
-        original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+        original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
 
-        salmon_xml = user2.salmon(original_message).xml_for(user.person)
+        salmon_xml = eve.salmon(original_message).xml_for(bob.person)
 
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
 
-        malicious_message = Factory.build(:status_message, :id => original_message.id, :text => 'BAD!!!', :author => user3.person)
-        salmon_xml = user3.salmon(malicious_message).xml_for(user.person)
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        malicious_message = Factory.build(:status_message, :id => original_message.id, :text => 'BAD!!!', :author => alice.person)
+        salmon_xml = alice.salmon(malicious_message).xml_for(bob.person)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
 
         original_message.reload.text.should == "store this!"
       end
 
       it 'does not save a message over an old message with the same author' do
-        original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+        original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
 
-        salmon_xml =  user2.salmon(original_message).xml_for(user.person)
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        salmon_xml =  eve.salmon(original_message).xml_for(bob.person)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
 
         lambda {
-          malicious_message = Factory.build( :status_message, :id => original_message.id, :text => 'BAD!!!', :author => user2.person)
+          malicious_message = Factory.build( :status_message, :id => original_message.id, :text => 'BAD!!!', :author => eve.person)
 
-          salmon_xml2 = user3.salmon(malicious_message).xml_for(user.person)
-          zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+          salmon_xml2 = alice.salmon(malicious_message).xml_for(bob.person)
+          zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
           zord.perform
 
-        }.should_not change{user.reload.visible_posts.count}
+        }.should_not change{
+          bob.reload.visible_posts.count
+        }
 
         original_message.reload.text.should == "store this!"
-        user.visible_posts.first.text.should == "store this!"
+        bob.visible_posts.first.text.should == "store this!"
       end
     end
+
     it 'should not overwrite another persons profile profile' do
-      profile = user2.profile.clone
+      profile = eve.profile.clone
       profile.first_name = "Not BOB"
 
-      user2.reload
+      eve.reload
 
-      first_name = user2.profile.first_name
-      salmon_xml = user3.salmon(profile).xml_for(user.person)
+      first_name = eve.profile.first_name
+      salmon_xml = alice.salmon(profile).xml_for(bob.person)
 
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
-      zord.perform
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
+      expect {
+        zord.perform
+      }.should raise_error /not a valid object/
 
-      user2.reload
-      user2.profile.first_name.should == first_name
+      eve.reload.profile.first_name.should == first_name
     end
 
     it "ignores retractions on a post not owned by the retraction's sender" do
       StatusMessage.delete_all
-      original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+      original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
 
-      salmon_xml = user2.salmon(original_message).xml_for(user.person)
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      salmon_xml = eve.salmon(original_message).xml_for(bob.person)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
-      user.visible_posts.count.should == 1
+      bob.visible_posts.count.should == 1
       StatusMessage.count.should == 1
 
       ret = Retraction.new
       ret.post_guid = original_message.guid
-      ret.diaspora_handle = user3.person.diaspora_handle
+      ret.diaspora_handle = alice.person.diaspora_handle
       ret.type = original_message.class.to_s
 
-      salmon_xml = user3.salmon(ret).xml_for(user.person)
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      salmon_xml = alice.salmon(ret).xml_for(bob.person)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
       StatusMessage.count.should == 1
-      user.visible_posts.count.should == 1
+      bob.visible_posts.count.should == 1
     end
 
     it "disregards retractions for non-existent posts that are from someone other than the post's author" do
       StatusMessage.delete_all
-      original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+      original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
       id = original_message.reload.id
 
       ret = Retraction.new
       ret.post_guid = original_message.guid
-      ret.diaspora_handle = user3.person.diaspora_handle
+      ret.diaspora_handle = alice.person.diaspora_handle
       ret.type = original_message.class.to_s
 
       original_message.delete
 
       StatusMessage.count.should == 0
       proc {
-        salmon_xml = user3.salmon(ret).xml_for(user.person)
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        salmon_xml = alice.salmon(ret).xml_for(bob.person)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
       }.should_not raise_error
     end
 
     it 'should not receive retractions where the retractor and the salmon author do not match' do
-      original_message = user2.post :status_message, :text => 'store this!', :to => aspect2.id
+      original_message = eve.post :status_message, :text => 'store this!', :to => eves_aspect.id
 
-      salmon_xml = user2.salmon(original_message).xml_for(user.person)
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      salmon_xml = eve.salmon(original_message).xml_for(bob.person)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
-
-      user.visible_posts.count.should == 1
+      bob.visible_posts.count.should == 1
 
       ret = Retraction.new
       ret.post_guid = original_message.guid
-      ret.diaspora_handle = user2.person.diaspora_handle
+      ret.diaspora_handle = eve.person.diaspora_handle
       ret.type = original_message.class.to_s
 
-      lambda {
-
-        salmon_xml = user3.salmon(ret).xml_for(user.person)
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      salmon_xml = alice.salmon(ret).xml_for(bob.person)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
+      expect {
         zord.perform
+      }.should raise_error /not a valid object/
 
-      }.should_not change(StatusMessage, :count)
-      user.reload.visible_posts.count.should == 1
+      bob.reload.visible_posts.count.should == 1
     end
 
     it 'it should not allow you to send retractions for other people' do
       ret = Retraction.new
-      ret.post_guid = user2.person.guid
-      ret.diaspora_handle = user3.person.diaspora_handle
-      ret.type = user2.person.class.to_s
+      ret.post_guid = eve.person.guid
+      ret.diaspora_handle = alice.person.diaspora_handle
+      ret.type = eve.person.class.to_s
 
       proc{
-        salmon_xml = user3.salmon(ret).xml_for(user.person)
+        salmon_xml = alice.salmon(ret).xml_for(bob.person)
 
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+        zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
         zord.perform
 
-      }.should_not change{user.reload.contacts.count}
+      }.should_not change{bob.reload.contacts.count}
     end
 
     it 'it should not allow you to send retractions with xml and salmon handle mismatch' do
       ret = Retraction.new
-      ret.post_guid = user2.person.guid
-      ret.diaspora_handle = user2.person.diaspora_handle
-      ret.type = user2.person.class.to_s
+      ret.post_guid = eve.person.guid
+      ret.diaspora_handle = eve.person.diaspora_handle
+      ret.type = eve.person.class.to_s
 
-      proc{
-        salmon_xml = user3.salmon(ret).xml_for(user.person)
-        zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      bob.contacts.count.should == 2
+
+      salmon_xml = alice.salmon(ret).xml_for(bob.person)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
+      expect {
         zord.perform
-      }.should_not change{user.reload.contacts.count}
+      }.should raise_error /not a valid object/
+
+      bob.reload.contacts.count.should == 2
     end
 
     it 'does not let me update other persons post' do
-      original_message = user2.post(:photo, :user_file => uploaded_photo, :text => "store this!", :to => aspect2.id)
+      original_message = eve.post(:photo, :user_file => uploaded_photo, :text => "store this!", :to => eves_aspect.id)
 
-      salmon_xml = user2.salmon(original_message).xml_for(user.person)
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      salmon_xml = eve.salmon(original_message).xml_for(bob.person)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
-      original_message.diaspora_handle = user3.diaspora_handle
+      original_message.diaspora_handle = alice.diaspora_handle
       original_message.text= "bad bad bad"
 
-      salmon_xml = user3.salmon(original_message).xml_for(user.person)
+      salmon_xml = alice.salmon(original_message).xml_for(bob.person)
 
-      zord = Postzord::Receiver::Private.new(user, :salmon_xml => salmon_xml)
+      zord = Postzord::Receiver::Private.new(bob, :salmon_xml => salmon_xml)
       zord.perform
 
       original_message.reload.text.should == "store this!"
diff --git a/spec/mailers/notifier_spec.rb b/spec/mailers/notifier_spec.rb
index 58e04355d..a4eb08d11 100644
--- a/spec/mailers/notifier_spec.rb
+++ b/spec/mailers/notifier_spec.rb
@@ -3,24 +3,20 @@ require 'spec_helper'
 describe Notifier do
   include ActionView::Helpers::TextHelper
 
-  let!(:user) {alice}
-  let!(:user2) {eve}
-
-  let!(:aspect) {user.aspects.create(:name => "win")}
-  let!(:aspect2) {user2.aspects.create(:name => "win")}
-  let!(:person) {Factory.create :person}
+  let(:person) { Factory(:person) }
 
   before do
     Notifier.deliveries = []
   end
+
   describe '.administrative' do
     it 'mails a user' do
-      mails = Notifier.admin("Welcome to bureaucracy!", [user])
+      mails = Notifier.admin("Welcome to bureaucracy!", [bob])
       mails.length.should == 1
       mail = mails.first
-      mail.to.should == [user.email]
+      mail.to.should == [bob.email]
       mail.body.encoded.should match /Welcome to bureaucracy!/
-      mail.body.encoded.should match /#{user.username}/
+      mail.body.encoded.should match /#{bob.username}/
     end
     it 'mails a bunch of users' do
       users = []
@@ -39,23 +35,23 @@ describe Notifier do
 
   describe '.single_admin' do
     it 'mails a user' do
-      mail = Notifier.single_admin("Welcome to bureaucracy!", user)
-      mail.to.should == [user.email]
+      mail = Notifier.single_admin("Welcome to bureaucracy!", bob)
+      mail.to.should == [bob.email]
       mail.body.encoded.should match /Welcome to bureaucracy!/
-      mail.body.encoded.should match /#{user.username}/
+      mail.body.encoded.should match /#{bob.username}/
     end
 
     it 'has the layout' do
-
-      mail = Notifier.single_admin("Welcome to bureaucracy!", user)
+      mail = Notifier.single_admin("Welcome to bureaucracy!", bob)
       mail.body.encoded.should match /change your notification settings/
     end
   end
 
   describe ".started_sharing" do
-    let!(:request_mail) {Notifier.started_sharing(user.id, person.id)}
+    let!(:request_mail) {Notifier.started_sharing(bob.id, person.id)}
+
     it 'goes to the right person' do
-      request_mail.to.should == [user.email]
+      request_mail.to.should == [bob.email]
     end
 
     it 'has the name of person sending the request' do
@@ -168,11 +164,11 @@ describe Notifier do
 
       @cnv = Conversation.create(@create_hash)
 
-      @mail = Notifier.private_message(user.id, @cnv.author.id, @cnv.messages.first.id)
+      @mail = Notifier.private_message(bob.id, @cnv.author.id, @cnv.messages.first.id)
     end
 
     it 'TO: goes to the right person' do
-      @mail.to.should == [user.email]
+      @mail.to.should == [bob.email]
     end
 
     it "FROM: contains the sender's name" do
@@ -186,7 +182,7 @@ describe Notifier do
 
     it 'SUBJECT: has "Re:" if not the first message in a conversation' do
       @cnv.messages << Message.new(:text => 'yo', :author => eve.person)
-      @mail = Notifier.private_message(user.id, @cnv.author.id, @cnv.messages.last.id)
+      @mail = Notifier.private_message(bob.id, @cnv.author.id, @cnv.messages.last.id)
 
       @mail.subject.should == "Re: #{@cnv.subject}"
     end
@@ -201,15 +197,14 @@ describe Notifier do
   end
 
   context "comments" do
-    let(:connect) { connect_users(user, aspect, user2, aspect2)}
-    let(:commented_post) {user.post(:status_message, :text => "It's really sunny outside today, and this is a super long status message!  #notreally", :to => :all)}
-    let(:comment) { user2.comment("Totally is", :post => commented_post)}
+    let(:commented_post) {bob.post(:status_message, :text => "It's really sunny outside today, and this is a super long status message!  #notreally", :to => :all)}
+    let(:comment) { eve.comment("Totally is", :post => commented_post)}
 
     describe ".comment_on_post" do
-      let(:comment_mail) {Notifier.comment_on_post(user.id, person.id, comment.id).deliver}
+      let(:comment_mail) {Notifier.comment_on_post(bob.id, person.id, comment.id).deliver}
 
       it 'TO: goes to the right person' do
-        comment_mail.to.should == [user.email]
+        comment_mail.to.should == [bob.email]
       end
 
       it "FROM: contains the sender's name" do
@@ -237,7 +232,7 @@ describe Notifier do
 
       [:reshare, :activity_streams_photo].each do |post_type|
         context post_type.to_s do
-          let(:commented_post) { Factory(post_type, :author => user.person) }
+          let(:commented_post) { Factory(post_type, :author => bob.person) }
           it 'succeeds' do
             proc {
               comment_mail
@@ -248,10 +243,10 @@ describe Notifier do
     end
 
     describe ".also_commented" do
-      let(:comment_mail) {Notifier.also_commented(user.id, person.id, comment.id)}
+      let(:comment_mail) {Notifier.also_commented(bob.id, person.id, comment.id)}
 
       it 'TO: goes to the right person' do
-        comment_mail.to.should == [user.email]
+        comment_mail.to.should == [bob.email]
       end
 
       it 'FROM: has the name of person commenting as the sender' do
@@ -278,7 +273,7 @@ describe Notifier do
       end
       [:reshare, :activity_streams_photo].each do |post_type|
         context post_type.to_s do
-          let(:commented_post) { Factory(post_type, :author => user.person) }
+          let(:commented_post) { Factory(post_type, :author => bob.person) }
           it 'succeeds' do
             proc {
               comment_mail
@@ -290,29 +285,28 @@ describe Notifier do
 
     describe ".confirm_email" do
       before do
-        user.update_attribute(:unconfirmed_email, "my@newemail.com")
+        bob.update_attribute(:unconfirmed_email, "my@newemail.com")
+        @confirm_email = Notifier.confirm_email(bob.id)
       end
 
-      let!(:confirm_email) { Notifier.confirm_email(user.id) }
-
       it 'goes to the right person' do
-        confirm_email.to.should == [user.unconfirmed_email]
+        @confirm_email.to.should == [bob.unconfirmed_email]
       end
 
       it 'has the unconfirmed emil in the subject' do
-        confirm_email.subject.should include(user.unconfirmed_email)
+        @confirm_email.subject.should include(bob.unconfirmed_email)
       end
 
       it 'has the unconfirmed emil in the body' do
-        confirm_email.body.encoded.should include(user.unconfirmed_email)
+        @confirm_email.body.encoded.should include(bob.unconfirmed_email)
       end
 
       it 'has the receivers name in the body' do
-        confirm_email.body.encoded.should include(user.person.profile.first_name)
+        @confirm_email.body.encoded.should include(bob.person.profile.first_name)
       end
 
       it 'has the activation link in the body' do
-        confirm_email.body.encoded.should include(confirm_email_url(:token => user.confirm_email_token))
+        @confirm_email.body.encoded.should include(confirm_email_url(:token => bob.confirm_email_token))
       end
     end
   end
diff --git a/spec/models/comment_spec.rb b/spec/models/comment_spec.rb
index 7d855360a..7d6135886 100644
--- a/spec/models/comment_spec.rb
+++ b/spec/models/comment_spec.rb
@@ -8,36 +8,32 @@ require File.join(Rails.root, "spec", "shared_behaviors", "relayable")
 describe Comment do
   before do
     @alices_aspect = alice.aspects.first
-    @bobs_aspect = bob.aspects.first
-
-    @bob = bob
-    @eve = eve
-    @status = alice.post(:status_message, :text => "hello", :to => @alices_aspect.id)
+    @status = bob.post(:status_message, :text => "hello", :to => bob.aspects.first.id)
   end
 
   describe 'comment#notification_type' do
     it "returns 'comment_on_post' if the comment is on a post you own" do
-      comment = bob.comment("why so formal?", :post => @status)
-      comment.notification_type(alice, bob.person).should == Notifications::CommentOnPost
+      comment = alice.comment("why so formal?", :post => @status)
+      comment.notification_type(bob, alice.person).should == Notifications::CommentOnPost
     end
 
     it 'returns false if the comment is not on a post you own and no one "also_commented"' do
       comment = alice.comment("I simply felt like issuing a greeting.  Do step off.", :post => @status)
-      comment.notification_type(@bob, alice.person).should == false
+      comment.notification_type(eve, alice.person).should be_false
     end
 
     context "also commented" do
       before do
-        @bob.comment("a-commenta commenta", :post => @status)
-        @comment = @eve.comment("I also commented on the first user's post", :post => @status)
+        alice.comment("a-commenta commenta", :post => @status)
+        @comment = eve.comment("I also commented on the first user's post", :post => @status)
       end
 
       it 'does not return also commented if the user commented' do
-        @comment.notification_type(@eve, alice.person).should == false
+        @comment.notification_type(eve, alice.person).should == false
       end
 
       it "returns 'also_commented' if another person commented on a post you commented on" do
-        @comment.notification_type(@bob, alice.person).should == Notifications::AlsoCommented
+        @comment.notification_type(alice, alice.person).should == Notifications::AlsoCommented
       end
     end
   end
@@ -103,6 +99,7 @@ describe Comment do
     end
   end
 
+  # NOTE(move this to the youtube module spec)
   describe 'youtube' do
     before do
       @message = alice.post :status_message, :text => "hi", :to => @alices_aspect.id