Fix retraction related failures in mysql

app/controllers/status_messages_controller.rb

@@ -22,7 +22,7 @@ class StatusMessagesController < ApplicationController
     @status_message = current_user.build_post(:status_message, params[:status_message])
 
 
-    if photos || @status_message.save!(:safe => true)
+    if photos || @status_message.save
       raise 'MongoMapper failed to catch a failed save' unless @status_message.id
 
       @status_message.photos += photos unless photos.nil?

app/helpers/sockets_helper.rb

@@ -6,7 +6,13 @@ module SocketsHelper
   include ApplicationHelper
 
   def obj_id(object)
-    object.respond_to?(:post_id) ? object.post_id : object.id
+    if object.respond_to?(:post_id)
+      object.post_id
+    elsif object.respond_to?(:post_guid)
+      object.post_guid
+    else
+      object.id
+    end
   end
 
   def action_hash(uid, object, opts={})

app/models/retraction.rb

@@ -6,7 +6,7 @@ class Retraction
   include ROXML
   include Diaspora::Webhooks
 
-  xml_accessor :post_id
+  xml_accessor :post_guid
   xml_accessor :diaspora_handle
   xml_accessor :type
 
@@ -15,31 +15,30 @@ class Retraction
   def self.for(object)
     retraction = self.new
     if object.is_a? User
-      retraction.post_id = object.person.id
+      retraction.post_guid = object.person.guid
       retraction.type = object.person.class.to_s
     else
-      retraction.post_id = object.id
+      retraction.post_guid = object.guid
       retraction.type = object.class.to_s
     end
     retraction.diaspora_handle = object.diaspora_handle
     retraction
   end
 
-  def perform receiving_user_id
+  def target
-    Rails.logger.debug "Performing retraction for #{post_id}"
+    @target ||= self.type.constantize.where(:guid => post_guid).first
-    if self.type.constantize.find_by_id(post_id)
+  end
-      unless Post.where(:diaspora_handle => person.diaspora_handle, :id => post_id).first
-        Rails.logger.info("event=retraction status=abort reason='no post found authored by retractor' sender=#{person.diaspora_handle} post_id=#{post_id}")
-        return
-      end
 
-      begin
+  def perform receiving_user_id
-        Rails.logger.debug("Retracting #{self.type} id: #{self.post_id}")
+    Rails.logger.debug "Performing retraction for #{post_guid}"
-        target = self.type.constantize.where(:id => self.post_id).first
+    if self.target
-        target.unsocket_from_uid receiving_user_id if target.respond_to? :unsocket_from_uid
+      if  self.target.person != self.person
-        target.delete
+        Rails.logger.info("event=retraction status=abort reason='no post found authored by retractor' sender=#{person.diaspora_handle} post_id=#{post_guid}")
-      rescue NameError
+        return
-        Rails.logger.info("event=retraction status=abort reason='unknown type'")
+      else
+        Rails.logger.info("event=retraction status=complete type=#{self.type} guid=#{self.post_guid}")
+        self.target.unsocket_from_uid receiving_user_id if target.respond_to? :unsocket_from_uid
+        self.target.delete
       end
     end
   end

lib/diaspora/user/receiving.rb

@@ -85,15 +85,14 @@ module Diaspora
 
       def receive_retraction retraction
         if retraction.type == 'Person'
-          unless retraction.person.id.to_s == retraction.post_id.to_s
+          unless retraction.person.guid.to_s == retraction.post_guid.to_s
             Rails.logger.info("event=receive status=abort reason='sender is not the person he is trying to retract' recipient=#{self.diaspora_handle} sender=#{retraction.person.diaspora_handle} payload_type=#{retraction.class} retraction_type=person")
             return
           end
-          disconnected_by Person.where(:id => retraction.post_id).first
+          disconnected_by Person.where(:guid => retraction.post_guid).first
-        else
+        elsif retraction.perform self.id
-          retraction.perform self.id
           aspects = self.aspects_with_person(retraction.person)
-          PostVisibility.where(:post_id => retraction.post_id,
+          PostVisibility.where(:post_id => retraction.target.id,
                                :aspect_id => aspects.map{|a| a.id}).delete_all
         end
         retraction
@@ -144,6 +143,9 @@ module Diaspora
       def post_visible?(post)
         raw_visible_posts.where(:guid => post.guid).first
       end
+      def posts_match(first, second)
+        (first.person == second.person) && (first.guid == second.guid) && (first.diaspora_handle == second.diaspora_handle)
+      end
 
       def receive_post post
         #exsists locally, but you dont know about it
@@ -156,7 +158,10 @@ module Diaspora
         on_pod = existing_post(post)
         log_string = "event=receive payload_type=#{post.class} sender=#{post.diaspora_handle} "
         if on_pod
-          if post_visible?(post)
+          if !posts_match(on_pod, post)
+            Rails.logger.info(log_string << "update=false status=abort reason='incoming post does not patch existing post' existing_post=#{on_pod.id}")
+            return
+          elsif post_visible?(post)
             if post.mutable?
               on_pod.caption = post.caption
               on_pod.save!

spec/models/aspect_spec.rb

@@ -18,29 +18,29 @@ describe Aspect do
 
   describe 'creation' do
     let!(:aspect){user.aspects.create(:name => 'losers')}
-    it 'should have a name' do
+    it 'has a name' do
       aspect.name.should == "losers"
     end
 
-    it 'should not allow duplicate names' do
+    it 'does not allow duplicate names' do
       lambda {
         invalid_aspect = user.aspects.create(:name => "losers ")
       }.should_not change(Aspect, :count)
     end
 
-    it 'should have a limit of 20 characters' do
+    it 'has a 20 character limit on names' do
       aspect = Aspect.new(:name => "this name is really too too too too too long")
       aspect.valid?.should == false
     end
 
-    it 'should be able to have other users' do
+    it 'is able to have other users as contacts' do
       Contact.create(:user => user, :person => user2.person, :aspects => [aspect])
       aspect.contacts.where(:person_id => user.person.id).should be_empty
       aspect.contacts.where(:person_id => user2.person.id).should_not be_empty
       aspect.contacts.size.should == 1
     end
 
-    it 'should be able to have users and people' do
+    it 'is able to have users and people and contacts' do
       contact1 = Contact.create(:user => user, :person => user2.person, :aspects => [aspect])
       contact2 = Contact.create(:user => user, :person => connected_person_2, :aspects => [aspect])
       aspect.contacts.include?(contact1).should be_true

spec/models/user/attack_vectors_spec.rb

@@ -40,8 +40,9 @@ describe "attack vectors" do
     original_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id
 
     original_message.diaspora_handle = user.diaspora_handle
+    user3.activate_contact(user2.person, user3.aspects.first)
     user3.receive_salmon(user.salmon(original_message).xml_for(user3.person))
-    user3.reload.visible_posts.should_not include(original_message)
+    user3.reload.visible_posts.should_not include(StatusMessage.find(original_message.id))
   end
 
   context 'malicious contact attack vector' do
@@ -56,10 +57,8 @@ describe "attack vectors" do
 
         user.receive_salmon(user2.salmon(original_message).xml_for(user.person))
 
-        lambda {
+        malicious_message = Factory.build( :status_message, :id => original_message.id, :message => 'BAD!!!', :person => user3.person)
-          malicious_message = Factory.build( :status_message, :id => original_message.id, :message => 'BAD!!!', :person => user3.person)
+        user.receive_salmon(user3.salmon(malicious_message).xml_for(user.person))
-          user.receive_salmon(user3.salmon(malicious_message).xml_for(user.person))
-        }.should_not change{user.reload.raw_visible_posts.count}
 
         original_message.reload.message.should == "store this!"
         user.raw_visible_posts.first.message.should == "store this!"
@@ -94,15 +93,16 @@ describe "attack vectors" do
       original_message = user2.post :status_message, :message => 'store this!', :to => aspect2.id
       user.receive_salmon(user2.salmon(original_message).xml_for(user.person))
       user.raw_visible_posts.count.should be 1
+      StatusMessage.count.should == 1
 
       ret = Retraction.new
-      ret.post_id = original_message.id
+      ret.post_guid = original_message.guid
       ret.diaspora_handle = user3.person.diaspora_handle
       ret.type = original_message.class.to_s
 
       user.receive_salmon(user3.salmon(ret).xml_for(user.person))
       StatusMessage.count.should be 1
-      user.reload.raw_visible_posts.count.should be 1
+      user.raw_visible_posts.count.should be 1
     end
 
     it "disregards retractions for non-existent posts that are from someone other than the post's author" do
@@ -110,14 +110,16 @@ describe "attack vectors" do
       id = original_message.reload.id
 
       ret = Retraction.new
-      ret.post_id = original_message.id
+      ret.post_guid = original_message.guid
       ret.diaspora_handle = user3.person.diaspora_handle
       ret.type = original_message.class.to_s
 
       original_message.delete
 
       StatusMessage.count.should be 0
-      proc{ user.receive_salmon(user3.salmon(ret).xml_for(user.person)) }.should_not raise_error
+      proc{
+        user.receive_salmon(user3.salmon(ret).xml_for(user.person))
+      }.should_not raise_error
     end
 
     it 'should not receive retractions where the retractor and the salmon author do not match' do
@@ -126,7 +128,7 @@ describe "attack vectors" do
       user.raw_visible_posts.count.should == 1
 
       ret = Retraction.new
-      ret.post_id = original_message.id
+      ret.post_guid = original_message.guid
       ret.diaspora_handle = user2.person.diaspora_handle
       ret.type = original_message.class.to_s
 
@@ -138,7 +140,7 @@ describe "attack vectors" do
 
     it 'it should not allow you to send retractions for other people' do
       ret = Retraction.new
-      ret.post_id = user2.person.id
+      ret.post_guid = user2.person.guid
       ret.diaspora_handle = user3.person.diaspora_handle
       ret.type = user2.person.class.to_s
 
@@ -149,7 +151,7 @@ describe "attack vectors" do
 
     it 'it should not allow you to send retractions with xml and salmon handle mismatch' do
       ret = Retraction.new
-      ret.post_id = user2.person.id
+      ret.post_guid = user2.person.guid
       ret.diaspora_handle = user2.person.diaspora_handle
       ret.type = user2.person.class.to_s