Moving friendships into a 'Contact' proxy object.

app/models/aspect.rb

@@ -6,11 +6,10 @@ class Aspect
   include MongoMapper::Document
 
   key :name,        String
-  key :person_ids,  Array
   key :request_ids, Array
   key :post_ids,    Array
 
-  many :people,   :in => :person_ids,  :class_name => 'Person'
+  many :people,   :foreign_key => 'aspect_ids', :class_name => 'Contact'
   many :requests, :in => :request_ids, :class_name => 'Request'
   many :posts,    :in => :post_ids,    :class_name => 'Post'
 

app/models/contact.rb

@@ -0,0 +1,16 @@
+#   Copyright (c) 2010, Diaspora Inc.  This file is
+#   licensed under the Affero General Public License version 3 or later.  See
+#   the COPYRIGHT file.
+
+class Contact
+  include MongoMapper::Document
+  belongs_to :user
+  validates_presence_of :user
+
+  belongs_to :person
+  validates_presence_of :person
+
+  key :aspect_ids, Array, :typecast => 'ObjectId'  
+  many :aspects, :in => :aspect_ids, :class_name => 'Aspect'
+  validates_presence_of :aspects
+end

app/models/user.rb

@@ -55,7 +55,7 @@ class User
   end
 
   many :inviters, :in => :inviter_ids, :class_name => 'User'
-  many :friends, :in => :friend_ids, :class_name => 'Person'
+  many :friends, :in => :friend_ids, :class_name => 'Contact'
   many :visible_people, :in => :visible_person_ids, :class_name => 'Person' # One of these needs to go
   many :pending_requests, :in => :pending_request_ids, :class_name => 'Request'
   many :raw_visible_posts, :in => :visible_post_ids, :class_name => 'Post'
@@ -119,22 +119,27 @@ class User
   end
 
   def add_person_to_aspect(person_id, aspect_id, opts = {})
-    raise "Can not add person to an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id) 
+    contact = contact_for(Person.find(person_id))
-    raise "Can not add person you are not friends with" unless person = self.find_friend_by_id(person_id)
+    raise "Can not add person to an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id)
-    raise 'Can not add person who is already in the aspect' if aspect.person_ids.include?(person_id)
+    raise "Can not add person you are not friends with" unless contact
-    aspect.people << person 
+    raise 'Can not add person who is already in the aspect' if aspect.people.include?(contact)
+    contact.aspects << aspect
     opts[:posts] ||= self.raw_visible_posts.all(:person_id => person_id)
     
     aspect.posts += opts[:posts]
+    contact.save
     aspect.save
   end
 
   def delete_person_from_aspect(person_id, aspect_id, opts = {})
-    raise "Can not delete a person from an aspect you do not own" unless aspect = self.aspects.find_by_id(aspect_id)
+    aspect = Aspect.find(aspect_id)
-    aspect.person_ids.delete(person_id.to_id)
+    raise "Can not delete a person from an aspect you do not own" unless aspect.user == self
+    contact = contact_for Person.find(person_id)
+    contact.aspects.delete aspect
     opts[:posts] ||= aspect.posts.all(:person_id => person_id)
     aspect.posts -= opts[:posts]
     aspect.save
+    contact.save
   end
 
   ######## Posting ########
@@ -211,17 +216,17 @@ class User
       aspects = self.aspects.find_all_by_id(aspect_ids)
     end
     #send to the aspects
-    target_people = []
+    target_contacts = []
 
     aspects.each { |aspect|
       aspect.posts << post
       aspect.save
-      target_people = target_people | aspect.people
+      target_contacts = target_contacts | aspect.people
     }
 
     push_to_hub(post) if post.respond_to?(:public) && post.public
 
-    push_to_people(post, target_people)
+    push_to_people(post, target_contacts.map{|c|c.person})
   end
 
   def push_to_people(post, people)
@@ -274,7 +279,7 @@ class User
     if owns? comment.post
       comment.post_creator_signature = comment.sign_with_key(encryption_key)
       comment.save
-      push_to_people comment, people_in_aspects(aspects_with_post(comment.post.id))
+      push_to_people comment, contacts_in_aspects(aspects_with_post(comment.post.id)).map{|c|c.person}
     elsif owns? comment
       comment.save
       push_to_people comment, [comment.post.person]
@@ -288,7 +293,7 @@ class User
 
     post.unsocket_from_uid(self.id, :aspect_ids => aspect_ids) if post.respond_to? :unsocket_from_uid
     retraction = Retraction.for(post)
-    push_to_people retraction, people_in_aspects(aspects_with_post(post.id))
+    push_to_people retraction, contacts_in_aspects(aspects_with_post(post.id)).map{|c| c.person}
     retraction
   end
 
@@ -439,11 +444,11 @@ class User
   end
 
   def unfriend_everyone
-    friends.each { |friend|
+    friends.each { |contact|
-      if friend.owner?
+      if contact.person.owner?
-        friend.owner.unfriended_by self.person
+        contact.person.owner.unfriended_by self.person
       else
-        self.unfriend friend
+        self.unfriend contact
       end
     }
   end

lib/diaspora/user/friending.rb

@@ -11,7 +11,7 @@ module Diaspora
         raise "You have already sent a friend request to that person!" if self.pending_requests.detect{
           |x| x.destination_url == desired_friend.receive_url }
         raise "You are already friends with that person!" if self.friends.detect{
-          |x| x.receive_url == desired_friend.receive_url}
+          |x| x.person.receive_url == desired_friend.receive_url}
         request = Request.instantiate(
           :to => desired_friend.receive_url,
           :from => self.person,
@@ -91,13 +91,15 @@ module Diaspora
       end
 
       def remove_friend(bad_friend)
-        raise "Friend not deleted" unless self.friend_ids.delete( bad_friend.id )
+        contact = contact_for(bad_friend)
-        aspects.each{|aspect|
+        raise "Friend not deleted" unless self.friend_ids.delete(contact.id)
-          if aspect.person_ids.delete( bad_friend.id )
+        contact.aspects.each{|aspect|
-            aspect.posts.delete_if { |post| 
+          contact.aspects.delete(aspect)
-              post.person_id == bad_friend.id}
+          aspect.posts.delete_if { |post| 
-          end}
+            post.person_id == bad_friend.id
-        self.save
+          }
+          aspect.save
+        }
 
         self.raw_visible_posts.find_all_by_person_id( bad_friend.id ).each{|post|
           self.visible_post_ids.delete( post.id )
@@ -105,7 +107,7 @@ module Diaspora
           (post.user_refs > 0 || post.person.owner.nil? == false) ?  post.save : post.destroy
         }
         self.save
-
+        contact.destroy
         bad_friend.save
       end
 
@@ -115,12 +117,17 @@ module Diaspora
       end
 
       def activate_friend(person, aspect)
-        aspect.people << person
+        new_contact = Contact.create(:user => self, :person => person, :aspects => [aspect])
-        friends << person
+        new_contact.aspects << aspect
+        friends << new_contact
         save
         aspect.save
       end
 
+      def contact_for(person)
+        friends.first(:person_id => person.id)
+      end
+
       def request_from_me?(request)
         (pending_request_ids.include?(request.id.to_id)) && (request.callback_url == person.receive_url) 
       end

lib/diaspora/user/querying.rb

@@ -24,7 +24,7 @@ module Diaspora
       def visible_person_by_id( id )
         id = id.to_id
         return self.person if id == self.person.id
-        result = friends.detect{|x| x.id == id }
+        result = friends.first(:person_id => id).person
         result = visible_people.detect{|x| x.id == id } unless result
         result
       end
@@ -38,22 +38,17 @@ module Diaspora
         aspects.detect{|x| x.id == id }
       end
 
-      def find_friend_by_id(id)
-        id = id.to_id
-        friends.detect{|x| x.id == id }
-      end
-
       def aspects_with_post( id )
         self.aspects.find_all_by_post_ids( id.to_id )
       end
 
       def aspects_with_person person
-        aspects.all(:person_ids => person.id)
+        contact_for(person).aspects
       end
 
-      def people_in_aspects aspects
+      def contacts_in_aspects aspects
-        aspects.inject([]) do |found_people,aspect|
+        aspects.inject([]) do |contacts,aspect|
-          found_people | aspect.people
+          contacts | aspect.people
         end
       end
 

lib/diaspora/user/receiving.rb

@@ -16,27 +16,24 @@ module Diaspora
 
         sender_in_xml = sender(object, xml)
 
-        if (salmon_author == sender_in_xml)
+        if (salmon_author != sender_in_xml)
-          
-          if object.is_a? Request
-            receive_request object, sender_in_xml
-          elsif self.friend_ids.include? salmon_author.id
-            if object.is_a? Retraction
-              receive_retraction object, xml
-            elsif object.is_a? Profile
-              receive_profile object, xml
-            elsif object.is_a?(Comment)
-              receive_comment object, xml
-            else
-              receive_post object, xml
-            end
-          else
-            raise "Not friends with that person"
-          end
-
-        else
           raise "Malicious Post, #{salmon_author.real_name} with id #{salmon_author.id} is sending a #{object.class} as #{sender_in_xml.real_name} with id #{sender_in_xml.id} "
         end
+
+        if object.is_a? Request
+          return receive_request object, sender_in_xml
+        end
+        raise "Not friends with that person" unless self.contact_for(salmon_author)
+
+        if object.is_a? Retraction
+          receive_retraction object, xml
+        elsif object.is_a? Profile
+          receive_profile object, xml
+        elsif object.is_a?(Comment)
+          receive_comment object, xml
+        else
+          receive_post object, xml
+        end
       end
 
       def sender(object, xml)

spec/lib/diaspora/parser_spec.rb

@@ -98,8 +98,9 @@ describe Diaspora::Parser do
 
       user.reload
       aspect.reload
-      aspect.people.include?(new_person).should be true
+      new_contact = user.contact_for(new_person)
-      user.friends.include?(new_person).should be true
+      aspect.people.include?(new_contact).should be true
+      user.friends.include?(new_contact).should be true
     end
 
     it 'should process retraction for a person' do

spec/misc_spec.rb

@@ -18,18 +18,20 @@ describe 'making sure the spec runner works' do
       @user2 = Factory.create(:user)
       @aspect2 = @user2.aspect(:name => "bruisers")
       friend_users(@user1, @aspect1, @user2, @aspect2)
-      @user1.reload
-      @aspect1.reload
-      @user2.reload
-      @aspect2.reload
     end
 
     it 'makes the first user friends with the second' do
-      @aspect1.people.include?(@user2.person).should be_true
+      contact = @user1.contact_for @user2.person
+      @user1.friends.should include contact
+      @aspect1.people.should include contact
+      contact.aspects.include?( @aspect1 ).should be true
     end
 
     it 'makes the second user friends with the first' do
-      @aspect2.people.include?(@user1.person).should be_true
+      contact = @user2.contact_for @user1.person
+      @user2.friends.should include contact
+      @aspect2.people.should include contact
+      contact.aspects.include?( @aspect2 ).should be true
     end
   end
 end

spec/models/album_spec.rb

@@ -20,7 +20,7 @@ describe Album do
   end
 
   it 'has many photos' do
-    album.associations[:photos].type == :many
+    album.associations[:photos].type.should == :many
   end
 
   context 'when an album has two attached images' do

spec/models/aspect_spec.rb

@@ -29,15 +29,17 @@ describe Aspect do
     end
 
     it 'should be able to have other users' do
-      aspect.people << user2.person
+      Contact.create(:user => user, :person => user2.person, :aspects => [aspect])
-      aspect.people.include?(user.person).should be false
+      aspect.people.first(:person_id => user.person.id).should be_nil
-      aspect.people.include?(user2.person).should be true
+      aspect.people.first(:person_id => user2.person.id).should_not be_nil
       aspect.people.size.should == 1
     end
 
     it 'should be able to have users and people' do
-      aspect.people << user2.person
+      contact1 = Contact.create(:user => user, :person => user2.person, :aspects => [aspect])
-      aspect.people << friend_2
+      contact2 = Contact.create(:user => user, :person => friend_2, :aspects => [aspect])
+      aspect.people.include?(contact1).should be_true
+      aspect.people.include?(contact2).should be_true
       aspect.save.should be_true
     end
   end
@@ -69,7 +71,7 @@ describe Aspect do
     end
 
     it 'should have people' do
-      aspect.people.all.include?(friend).should be true
+      aspect.people.first(:person_id => friend.id).should be_true
       aspect.people.size.should == 1
     end
 
@@ -87,8 +89,9 @@ describe Aspect do
         user.add_person_to_aspect(friend.id, aspect1.id)
         aspects = user.aspects_with_person(friend)
         aspects.count.should == 2
-        aspects.each{ |asp| asp.people.include?(friend) }
+        contact = user.contact_for(friend)
-        aspects.should_not include aspect_without_friend
+        aspects.each{ |asp| asp.people.include?(contact).should be_true }
+        aspects.include?(aspect_without_friend).should be_false
       end
     end
   end
@@ -140,6 +143,7 @@ describe Aspect do
   end
 
   context "aspect management" do
+    let(:contact){user.contact_for(user2.person)}
     before do
       friend_users(user, aspect, user2, aspect2)
       aspect.reload
@@ -149,10 +153,10 @@ describe Aspect do
 
     describe "#add_person_to_aspect" do
       it 'adds the user to the aspect' do
-        aspect1.people.should_not include user2.person
+        aspect1.people.should_not include contact
         user.add_person_to_aspect(user2.person.id, aspect1.id)
         aspect1.reload
-        aspect1.people.should include user2.person
+        aspect1.people.should include contact
       end
 
       it 'raises if its an aspect that the user does not own'do
@@ -172,10 +176,10 @@ describe Aspect do
       it 'deletes a user from the aspect' do
          user.add_person_to_aspect(user2.person.id, aspect1.id)
          user.reload
-         user.aspects.find_by_id(aspect1.id).people.include?(user2.person).should be true
+         aspect1.reload.people.include?(contact).should be true
          user.delete_person_from_aspect(user2.person.id, aspect1.id)
          user.reload
-         user.aspects.find_by_id(aspect1.id).people.include?(user2.person).should be false
+         aspect1.reload.people.include?(contact).should be false
       end
 
       it 'should check to make sure you have the aspect ' do
@@ -189,9 +193,7 @@ describe Aspect do
       let(:message2){user3.post(:status_message, :message => "other post", :to => aspect3.id)}
 
       before do
-        friend_users(user, aspect, user3, aspect3)
         user.receive message.to_diaspora_xml, user2.person
-        user.receive message2.to_diaspora_xml, user3.person
         aspect.reload
         @post_count  = aspect.posts.count
         @post_count1 = aspect1.posts.count
@@ -213,6 +215,8 @@ describe Aspect do
       end
 
       it 'should not delete other peoples posts' do
+        friend_users(user, aspect, user3, aspect3)
+        user.receive message2.to_diaspora_xml, user3.person
         user.delete_person_from_aspect(user2.person.id, aspect.id)
         aspect.reload
         aspect.posts.should == [message2]
@@ -224,24 +228,24 @@ describe Aspect do
           aspect.reload
           aspect1.reload
 
-          aspect.person_ids.include?(user2.person.id).should be false
+          aspect.people.include?(contact).should be false
-          aspect1.people.include?(user2.person).should be true
+          aspect1.people.include?(contact).should be true
         end
 
         it "should not move a person who is not a friend" do
           proc{ user.move_friend(:friend_id => friend.id, :from => aspect.id, :to => aspect1.id) }.should raise_error /Can not add person you are not friends with/
           aspect.reload
           aspect1.reload
-          aspect.people.include?(friend).should be false
+          aspect.people.first(:person_id => friend.id).should be_nil
-          aspect1.people.include?(friend).should be false
+          aspect1.people.first(:person_id => friend.id).should be_nil
         end
 
         it "should not move a person to a aspect that's not his" do
           proc {user.move_friend(:friend_id => user2.person.id, :from => aspect.id, :to => aspect2.id )}.should raise_error /Can not add person to an aspect you do not own/
           aspect.reload
           aspect2.reload
-          aspect.people.include?(user2.person).should be true
+          aspect.people.include?(contact).should be true
-          aspect2.people.include?(user2.person).should be false
+          aspect2.people.include?(contact).should be false
         end
 
         it 'should move all posts by that user to the new aspect' do

spec/models/contact_spec.rb

@@ -0,0 +1,31 @@
+#   Copyright (c) 2010, Diaspora Inc.  This file is
+#   licensed under the Affero General Public License version 3 or later.  See
+#   the COPYRIGHT file.
+
+require 'spec_helper'
+
+describe Contact do
+  describe 'validations' do
+    let(:contact){Contact.new}
+    
+    it 'requires a user' do
+      contact.valid?
+      contact.errors.full_messages.should include "User can't be blank"
+    end
+    
+    it 'requires a person' do
+      contact.valid?
+      contact.errors.full_messages.should include "Person can't be blank"
+    end
+
+    it 'has many aspects' do
+      contact.associations[:aspects].type.should == :many
+    end
+
+    it 'has at least one aspect' do
+      contact.valid?
+      contact.errors.full_messages.should include "Aspects can't be blank"
+    end
+
+  end
+end

spec/models/user/receive_spec.rb

@@ -23,10 +23,9 @@ describe User do
     status_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id
 
     xml = status_message.to_diaspora_xml
-    user2.destroy
+    user2.delete
     status_message.destroy
 
-    user
     lambda {user.receive xml , user2.person}.should change(Post,:count).by(1)
   end
 

spec/models/user/user_friending_spec.rb

@@ -13,6 +13,7 @@ describe Diaspora::UserModules::Friending do
 
   let(:person_one) { Factory.create :person }
   let(:person_two) { Factory.create :person }
+  let(:person_three) { Factory.create :person }
 
   let(:user2) { Factory.create :user }
   let(:aspect2) { user2.aspect(:name => "aspect two") }
@@ -24,6 +25,41 @@ describe Diaspora::UserModules::Friending do
     Notifier.stub!(:request_accepted).and_return(deliverable)
   end
 
+
+  describe '#contact_for' do
+
+    it 'returns a contact' do
+      contact = Contact.create(:user => user, :person => person_one, :aspects => [aspect])
+      user.friends << contact
+      user.contact_for(person_one).should be_true
+    end
+
+    it 'returns the correct contact' do
+      contact = Contact.create(:user => user, :person => person_one, :aspects => [aspect])
+      user.friends << contact
+
+      contact2 = Contact.create(:user => user, :person => person_two, :aspects => [aspect])
+      user.friends << contact2
+
+      contact3 = Contact.create(:user => user, :person => person_three, :aspects => [aspect])
+      user.friends << contact3
+
+      user.contact_for(person_two).person.should == person_two
+    end
+
+    it 'returns nil for a non-contact' do
+      user.contact_for(person_one).should be_nil
+    end
+
+    it 'returns nil when someone else has contact with the target' do
+      contact = Contact.create(:user => user, :person => person_one, :aspects => [aspect])
+      user.friends << contact
+      user2.contact_for(person_one).should be_nil
+    end
+
+  end
+
+
   context 'friend requesting' do
     it "should assign a request to a aspect" do
       aspect.requests.size.should == 0
@@ -52,10 +88,9 @@ describe Diaspora::UserModules::Friending do
     end
 
     it 'should not be able to friend request an existing friend' do
-      user.friends << friend
+      friend_users(user, aspect, user2, aspect2)
-      user.save
 
-      proc { user.send_friend_request_to(friend, aspect) }.should raise_error
+      proc { user.send_friend_request_to(user2.person, aspect1) }.should raise_error
     end
 
     it 'should not be able to friend request yourself' do
@@ -99,14 +134,14 @@ describe Diaspora::UserModules::Friending do
           proc {
             user2.accept_friend_request @request_three.id, aspect2.id
           }.should_not change(Person, :count)
-          user2.friends.include?(user.person).should be true
+          user2.contact_for(user.person).should_not be_nil
         end
 
         it 'should not delete the ignored user on the same pod' do
           proc {
             user2.ignore_friend_request @request_three.id
           }.should_not change(Person, :count)
-          user2.friends.include?(user.person).should be false
+          user2.contact_for(user.person).should be_nil
         end
 
         it 'sends an email to the receiving user' do
@@ -118,6 +153,7 @@ describe Diaspora::UserModules::Friending do
 
 
       end
+
       context 'Two users receiving requests from one person' do
         before do
           user.receive @req_xml, person_one
@@ -127,28 +163,28 @@ describe Diaspora::UserModules::Friending do
         describe '#accept_friend_request' do
           it 'should both users should befriend the same person' do
             user.accept_friend_request @request.id, aspect.id
-            user.friends.include?(person_one).should be true
+            user.contact_for(person_one).should_not be_nil
 
             user2.accept_friend_request @request_two.id, aspect2.id
-            user2.friends.include?(person_one).should be true
+            user2.contact_for(person_one).should_not be_nil
           end
 
           it 'should keep the person around if one of the users rejects him' do
             user.accept_friend_request @request.id, aspect.id
-            user.friends.include?(person_one).should be true
+            user.contact_for(person_one).should_not be_nil
 
             user2.ignore_friend_request @request_two.id
-            user2.friends.include?(person_one).should be false
+            user2.contact_for(person_one).should be_nil
           end
         end
 
 
         it 'should keep the person around if the users ignores them' do
           user.ignore_friend_request user.pending_requests.first.id
-          user.friends.include?(person_one).should be false
+          user.contact_for(person_one).should be_nil
 
           user2.ignore_friend_request user2.pending_requests.first.id #@request_two.id
-          user2.friends.include?(person_one).should be false
+          user2.contact_for(person_one).should be_nil
         end
       end
 
@@ -178,12 +214,12 @@ describe Diaspora::UserModules::Friending do
         user.accept_friend_request @request.id, aspect.id
         user.pending_requests.size.should be 1
         user.friends.size.should be 1
-        user.friends.include?(person_one).should be true
+        user.contact_for(person_one).should_not be_nil
 
         user.ignore_friend_request @request_two.id
         user.pending_requests.size.should be 0
         user.friends.size.should be 1
-        user.friends.include?(person_two).should be false
+        user.contact_for(person_two).should be_nil
       end
     end
 
@@ -193,8 +229,9 @@ describe Diaspora::UserModules::Friending do
       end
 
       it 'should unfriend the other user on the same seed' do
-        lambda { user2.unfriend user.person }.should change {
+        lambda { 
-          user2.friends.count }.by(-1)
+          user2.unfriend user.person }.should change {
+          user2.reload.friends.count }.by(-1)
         aspect2.reload.people.count.should == 0
       end
 
@@ -206,6 +243,8 @@ describe Diaspora::UserModules::Friending do
 
       it 'should remove the friend from all aspects they are in' do
         user.add_person_to_aspect(user2.person.id, aspect1.id)
+        aspect.reload.people.count.should == 1
+        aspect1.reload.people.count.should == 1
         lambda { user.unfriended_by user2.person }.should change {
           user.friends.count }.by(-1)
         aspect.reload.people.count.should == 0

spec/models/user/visible_posts_spec.rb

@@ -73,17 +73,6 @@ describe User do
       end
     end
 
-    describe '#find_friend_by_id' do
-      it 'should find a friend' do
-        friend_users(user, first_aspect, user2, user2.aspects.first)
-        user.find_friend_by_id(user2.person.id).should == user2.person
-      end
-
-      it 'should not find a non-friend' do
-        user = Factory :user
-        user.find_friend_by_id(user2.person.id).should be nil
-      end
-    end
   end
   describe '#albums_by_aspect' do
     let!(:first_aspect)  {user2.aspect(:name => 'bruisers')}