From e9e01e965e8c31cd7b901e16d07325fcf22925e9 Mon Sep 17 00:00:00 2001 From: zhitomirskiyi Date: Mon, 24 Jan 2011 17:23:51 -0800 Subject: [PATCH] fixed error with comments saving twice on the same pod --- app/models/comment.rb | 22 +++--- app/models/post.rb | 2 +- lib/postzord/receiver.rb | 6 +- lib/webfinger.rb | 2 +- spec/intergration/receiving_spec.rb | 102 +++++++++++++++++----------- spec/lib/postzord/receiver_spec.rb | 3 +- 6 files changed, 81 insertions(+), 56 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 10027719b..57429e55c 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -61,26 +61,28 @@ class Comment < ActiveRecord::Base end def receive(user, person) - commenter = self.person - unless self.post.person == user.person || self.verify_post_creator_signature + local_comment = Comment.where(:guid => self.guid).first + comment = local_comment || self + + unless comment.post.person == user.person || comment.verify_post_creator_signature Rails.logger.info("event=receive status=abort reason='comment signature not valid' recipient=#{user.diaspora_handle} sender=#{self.post.person.diaspora_handle} payload_type=#{self.class} post_id=#{self.post_id}") return end #sign comment as the post creator if you've been hit UPSTREAM - if user.owns? self.post - self.post_creator_signature = self.sign_with_key(user.encryption_key) - self.save + if user.owns? comment.post + comment.post_creator_signature = comment.sign_with_key(user.encryption_key) + comment.save end #dispatch comment DOWNSTREAM, received it via UPSTREAM - unless user.owns?(self) - self.save - user.dispatch_comment(self) + unless user.owns?(comment) + comment.save + user.dispatch_comment(comment) end - self.socket_to_user(user, :aspect_ids => self.post.aspect_ids) - self + comment.socket_to_user(user, :aspect_ids => comment.post.aspect_ids) + comment end #ENCRYPTION diff --git a/app/models/post.rb b/app/models/post.rb index 456fce36e..66bb866fe 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -82,7 +82,7 @@ class Post < ActiveRecord::Base else user.add_post_to_aspects(local_post) Rails.logger.info("event=receive payload_type=#{self.class} update=true status=complete sender=#{self.diaspora_handle} existing_post=#{local_post.id}") - self + local_post end elsif !local_post self.save diff --git a/lib/postzord/receiver.rb b/lib/postzord/receiver.rb index 55b49fb2b..ade93bb6e 100644 --- a/lib/postzord/receiver.rb +++ b/lib/postzord/receiver.rb @@ -28,8 +28,8 @@ module Postzord end def parse_and_receive(xml) - Rails.logger.info("event=receive status=start recipient=#{@user_person.diaspora_handle} payload_type=#{@object.class} sender=#{@sender.diaspora_handle}") @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 end @@ -37,8 +37,8 @@ module Postzord def receive_object obj = @object.receive(@user, @author) - Notification.notify(@user, @object, @author) if @object.respond_to?(:notification_type) - Rails.logger.info("event=receive status=complete recipient=#{@user_person.diaspora_handle} sender=#{@sender.diaspora_handle} payload_type#{@object.class}") + Notification.notify(@user, obj, @author) if obj.respond_to?(:notification_type) + Rails.logger.info("event=receive status=complete recipient=#{@user_person.diaspora_handle} sender=#{@sender.diaspora_handle} payload_type#{obj.class}") obj end diff --git a/lib/webfinger.rb b/lib/webfinger.rb index d060bce06..60d6f66b5 100644 --- a/lib/webfinger.rb +++ b/lib/webfinger.rb @@ -31,7 +31,7 @@ class Webfinger raise WebfingerFailedError.new(@account) end rescue - Rails.logger.info("event=receive status=abort recipient=#{self.diaspora_handle} sender=#{salmon.author_email} reason='#{e.message}'") + Rails.logger.info("event=receive status=abort recipient=#{@account} sender=#{salmon.author_email} reason='#{e.message}'") nil end end diff --git a/spec/intergration/receiving_spec.rb b/spec/intergration/receiving_spec.rb index afe49755f..dd2ad340b 100644 --- a/spec/intergration/receiving_spec.rb +++ b/spec/intergration/receiving_spec.rb @@ -154,55 +154,77 @@ describe 'a user receives a post' do end describe 'comments' do - before do - connect_users(@user1, @aspect, @user3, @aspect3) - @post = @user1.post :status_message, :message => "hello", :to => @aspect.id - xml = @post.to_diaspora_xml + context 'remote' do + before do + connect_users(@user1, @aspect, @user3, @aspect3) + @post = @user1.post :status_message, :message => "hello", :to => @aspect.id - receive_with_zord(@user2, @user1.person, xml) - receive_with_zord(@user3, @user1.person, xml) + xml = @post.to_diaspora_xml - @comment = @user3.comment('tada',:on => @post) - @comment.post_creator_signature = @comment.sign_with_key(@user1.encryption_key) - @xml = @comment.to_diaspora_xml - @comment.delete + receive_with_zord(@user2, @user1.person, xml) + receive_with_zord(@user3, @user1.person, xml) + + @comment = @user3.comment('tada',:on => @post) + @comment.post_creator_signature = @comment.sign_with_key(@user1.encryption_key) + @xml = @comment.to_diaspora_xml + @comment.delete + end + + it 'should correctly attach the user already on the pod' do + @user2.reload.raw_visible_posts.size.should == 1 + post_in_db = StatusMessage.find(@post.id) + post_in_db.comments.should == [] + receive_with_zord(@user2, @user1.person, @xml) + + post_in_db.comments(true).first.person.should == @user3.person + end + + it 'should correctly marshal a stranger for the downstream user' do + remote_person = @user3.person.dup + @user3.person.delete + @user3.delete + remote_person.id = nil + + #stubs async webfinger + Person.should_receive(:by_account_identifier).twice.and_return{ |handle| + if handle == @user1.person.diaspora_handle + @user1.person.save + @user1.person + else + remote_person.profile = Factory(:profile) + remote_person.save! + remote_person + end + } + + @user2.reload.raw_visible_posts.size.should == 1 + post_in_db = StatusMessage.find(@post.id) + post_in_db.comments.should == [] + + receive_with_zord(@user2, @user1.person, @xml) + + post_in_db.comments(true).first.person.should == remote_person + end end - it 'should correctly attach the user already on the pod' do - @user2.reload.raw_visible_posts.size.should == 1 - post_in_db = StatusMessage.find(@post.id) - post_in_db.comments.should == [] - receive_with_zord(@user2, @user1.person, @xml) + context 'local' do + before do + @post = @user1.post :status_message, :message => "hello", :to => @aspect.id - post_in_db.comments(true).first.person.should == @user3.person - end + xml = @post.to_diaspora_xml - it 'should correctly marshal a stranger for the downstream user' do - remote_person = @user3.person.dup - @user3.person.delete - @user3.delete - remote_person.id = nil + receive_with_zord(@user2, @user1.person, xml) - #stubs async webfinger - Person.should_receive(:by_account_identifier).twice.and_return{ |handle| - if handle == @user1.person.diaspora_handle - @user1.person.save - @user1.person - else - remote_person.profile = Factory(:profile) - remote_person.save! - remote_person - end - } + @comment = @user2.comment('tada',:on => @post) + @xml = @comment.to_diaspora_xml + end - @user2.reload.raw_visible_posts.size.should == 1 - post_in_db = StatusMessage.find(@post.id) - post_in_db.comments.should == [] - - receive_with_zord(@user2, @user1.person, @xml) - - post_in_db.comments(true).first.person.should == remote_person + it 'does not raise a `Mysql2::Error: Duplicate entry...` exception on save' do + #lambda { + receive_with_zord(@user1, @user2.person, @xml) + #}.should_not raise_exception + end end end diff --git a/spec/lib/postzord/receiver_spec.rb b/spec/lib/postzord/receiver_spec.rb index 088c4ac0d..fb6f0ba57 100644 --- a/spec/lib/postzord/receiver_spec.rb +++ b/spec/lib/postzord/receiver_spec.rb @@ -86,7 +86,8 @@ describe Postzord::Receiver do it 'calls Notification.notify if object responds to notification_type' do cm = Comment.new - cm.stub!(:receive) + cm.stub!(:receive).and_return(cm) + Notification.should_receive(:notify).with(@user, cm, @person2) zord = Postzord::Receiver.new(@user, :person => @person2, :object => cm) zord.receive_object