From d55be67df17d303419d1d906f40e0556c6652246 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 7 May 2016 18:59:40 +0200 Subject: [PATCH] handle existing guids on receive --- app/workers/receive_base.rb | 4 +- lib/diaspora/federation.rb | 4 + lib/diaspora/federation/receive.rb | 150 ++++++++++++++++++++--------- 3 files changed, 111 insertions(+), 47 deletions(-) diff --git a/app/workers/receive_base.rb b/app/workers/receive_base.rb index 1fbccefa6..e2ccc8690 100644 --- a/app/workers/receive_base.rb +++ b/app/workers/receive_base.rb @@ -20,15 +20,13 @@ module Workers DiasporaFederation::Salmon::InvalidAlgorithm, DiasporaFederation::Salmon::InvalidEncoding, Diaspora::Federation::AuthorIgnored, + Diaspora::Federation::InvalidAuthor, # TODO: deprecated DiasporaFederation::Salmon::MissingMagicEnvelope, DiasporaFederation::Salmon::MissingAuthor, DiasporaFederation::Salmon::MissingHeader, DiasporaFederation::Salmon::InvalidHeader => e logger.warn "don't retry for error: #{e.class}" - rescue ActiveRecord::RecordInvalid => e - logger.warn "failed to save received object: #{e.record.errors.full_messages}" - raise e unless e.message.include? "already been taken" end end end diff --git a/lib/diaspora/federation.rb b/lib/diaspora/federation.rb index a3baae637..e84d8348b 100644 --- a/lib/diaspora/federation.rb +++ b/lib/diaspora/federation.rb @@ -8,6 +8,10 @@ module Diaspora # Raised, if author is ignored by the relayable parent author class AuthorIgnored < RuntimeError end + + # Raised, if the author of the existing object doesn't match the received author + class InvalidAuthor < RuntimeError + end end end diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index a7dbf4219..8e42b21fc 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -1,6 +1,8 @@ module Diaspora module Federation module Receive + extend Diaspora::Logging + def self.account_deletion(entity) AccountDeletion.new( person: author_of(entity), @@ -9,13 +11,16 @@ module Diaspora end def self.comment(entity) - Comment.new( - author: author_of(entity), - guid: entity.guid, - created_at: entity.created_at, - text: entity.text, - commentable: Post.find_by(guid: entity.parent_guid) - ).tap {|comment| save_relayable(comment, entity) } + author = author_of(entity) + ignore_existing_guid(Comment, entity.guid, author) do + Comment.new( + author: author, + guid: entity.guid, + created_at: entity.created_at, + text: entity.text, + commentable: Post.find_by(guid: entity.parent_guid) + ).tap {|comment| save_relayable(comment, entity) } + end end def self.contact(entity) @@ -31,29 +36,37 @@ module Diaspora end def self.conversation(entity) - Conversation.new( - author: author_of(entity), - guid: entity.guid, - subject: entity.subject, - created_at: entity.created_at, - participant_handles: entity.participants - ).tap do |conversation| - conversation.messages = entity.messages.map {|message| build_message(message) } - conversation.save! + author = author_of(entity) + try_load_existing_guid(Conversation, entity.guid, author) do + Conversation.new( + author: author, + guid: entity.guid, + subject: entity.subject, + created_at: entity.created_at, + participant_handles: entity.participants + ).tap do |conversation| + conversation.messages = entity.messages.map {|message| build_message(message) } + conversation.save! + end end end def self.like(entity) - Like.new( - author: author_of(entity), - guid: entity.guid, - positive: entity.positive, - target: entity.parent_type.constantize.find_by(guid: entity.parent_guid) - ).tap {|like| save_relayable(like, entity) } + author = author_of(entity) + ignore_existing_guid(Like, entity.guid, author) do + Like.new( + author: author, + guid: entity.guid, + positive: entity.positive, + target: entity.parent_type.constantize.find_by(guid: entity.parent_guid) + ).tap {|like| save_relayable(like, entity) } + end end def self.message(entity) - build_message(entity).tap(&:save!) + ignore_existing_guid(Message, entity.guid, author_of(entity)) do + build_message(entity).tap(&:save!) + end end def self.participation(entity) @@ -73,14 +86,17 @@ module Diaspora end def self.poll_participation(entity) - PollParticipation.new( - author: author_of(entity), - guid: entity.guid, - poll: Poll.find_by(guid: entity.parent_guid) - ).tap do |poll_participation| - poll_participation.poll_answer_guid = entity.poll_answer_guid + author = author_of(entity) + ignore_existing_guid(PollParticipation, entity.guid, author) do + PollParticipation.new( + author: author, + guid: entity.guid, + poll: Poll.find_by(guid: entity.parent_guid) + ).tap do |poll_participation| + poll_participation.poll_answer_guid = entity.poll_answer_guid - save_relayable(poll_participation, entity) + save_relayable(poll_participation, entity) + end end end @@ -115,27 +131,29 @@ module Diaspora end def self.status_message(entity) - StatusMessage.new( - author: author_of(entity), - guid: entity.guid, - raw_message: entity.raw_message, - public: entity.public, - created_at: entity.created_at, - provider_display_name: entity.provider_display_name - ).tap do |status_message| - status_message.photos = entity.photos.map {|photo| build_photo(photo) } - status_message.location = build_location(entity.location) if entity.location - status_message.poll = build_poll(entity.poll) if entity.poll + author = author_of(entity) + try_load_existing_guid(StatusMessage, entity.guid, author) do + StatusMessage.new( + author: author, + guid: entity.guid, + raw_message: entity.raw_message, + public: entity.public, + created_at: entity.created_at, + provider_display_name: entity.provider_display_name + ).tap do |status_message| + status_message.photos = entity.photos.map {|photo| build_photo(photo) } + status_message.location = build_location(entity.location) if entity.location + status_message.poll = build_poll(entity.poll) if entity.poll - status_message.save! + status_message.save! + end end end - private - def self.author_of(entity) Person.find_by(diaspora_handle: entity.author) end + private_class_method :author_of def self.build_location(entity) Location.new( @@ -144,6 +162,7 @@ module Diaspora lng: entity.lng ) end + private_class_method :build_location def self.build_message(entity) Message.new( @@ -154,6 +173,7 @@ module Diaspora conversation_guid: entity.conversation_guid ) end + private_class_method :build_message def self.build_photo(entity) Photo.new( @@ -169,6 +189,7 @@ module Diaspora width: entity.width ) end + private_class_method :build_photo def self.build_poll(entity) Poll.new( @@ -183,6 +204,7 @@ module Diaspora end end end + private_class_method :build_poll def self.save_relayable(relayable, entity) retract_if_author_ignored(relayable) @@ -190,6 +212,7 @@ module Diaspora relayable.author_signature = entity.author_signature if relayable.parent.author.local? relayable.save! end + private_class_method :save_relayable def self.retract_if_author_ignored(relayable) parent_author = relayable.parent.author @@ -199,6 +222,45 @@ module Diaspora raise Diaspora::Federation::AuthorIgnored end + private_class_method :retract_if_author_ignored + + # check if the object already exists, otherwise save it. + # if save fails (probably because of a second object received parallel), + # check again if an object with the same guid already exists, but maybe has a different author. + # @raise [InvalidAuthor] if the author of the existing object doesn't match + def self.ignore_existing_guid(klass, guid, author) + yield unless klass.where(guid: guid, author_id: author.id).exists? + rescue => e + raise e unless load_from_database(klass, guid, author) + logger.warn "ignoring error on receive #{klass}:#{guid}: #{e.class}: #{e.message}" + end + private_class_method :ignore_existing_guid + + # try to load the object first from the DB and if not available, save it. + # if save fails (probably because of a second object received parallel), + # try again to load it, because it is possibly there now. + # @raise [InvalidAuthor] if the author of the existing object doesn't match + def self.try_load_existing_guid(klass, guid, author) + load_from_database(klass, guid, author) || yield + rescue Diaspora::Federation::InvalidAuthor => e + raise e # don't try loading from db twice + rescue => e + logger.warn "failed to save #{klass}:#{guid} (#{e.class}: #{e.message}) - try loading it from DB" + load_from_database(klass, guid, author).tap do |object| + raise e unless object + end + end + private_class_method :try_load_existing_guid + + # @raise [InvalidAuthor] if the author of the loaded object doesn't match + def self.load_from_database(klass, guid, author) + klass.find_by(guid: guid).tap do |object| + if object && object.author_id != author.id + raise Diaspora::Federation::InvalidAuthor, "#{klass}:#{guid}: #{author.diaspora_handle}" + end + end + end + private_class_method :load_from_database end end end