handle existing guids on receive

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

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
 

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,14 +11,17 @@ module Diaspora
       end
 
       def self.comment(entity)
+        author = author_of(entity)
+        ignore_existing_guid(Comment, entity.guid, author) do
           Comment.new(
-          author:      author_of(entity),
+            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)
         recipient = Person.find_by(diaspora_handle: entity.recipient).owner
@@ -31,8 +36,10 @@ module Diaspora
       end
 
       def self.conversation(entity)
+        author = author_of(entity)
+        try_load_existing_guid(Conversation, entity.guid, author) do
           Conversation.new(
-          author:              author_of(entity),
+            author:              author,
             guid:                entity.guid,
             subject:             entity.subject,
             created_at:          entity.created_at,
@@ -42,19 +49,25 @@ module Diaspora
             conversation.save!
           end
         end
+      end
 
       def self.like(entity)
+        author = author_of(entity)
+        ignore_existing_guid(Like, entity.guid, author) do
           Like.new(
-          author:   author_of(entity),
+            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)
+        ignore_existing_guid(Message, entity.guid, author_of(entity)) do
           build_message(entity).tap(&:save!)
         end
+      end
 
       def self.participation(entity)
         parent = entity.parent_type.constantize.find_by(guid: entity.parent_guid)
@@ -73,8 +86,10 @@ module Diaspora
       end
 
       def self.poll_participation(entity)
+        author = author_of(entity)
+        ignore_existing_guid(PollParticipation, entity.guid, author) do
           PollParticipation.new(
-          author: author_of(entity),
+            author: author,
             guid:   entity.guid,
             poll:   Poll.find_by(guid: entity.parent_guid)
           ).tap do |poll_participation|
@@ -83,6 +98,7 @@ module Diaspora
             save_relayable(poll_participation, entity)
           end
         end
+      end
 
       def self.profile(entity)
         author_of(entity).profile.tap do |profile|
@@ -115,8 +131,10 @@ module Diaspora
       end
 
       def self.status_message(entity)
+        author = author_of(entity)
+        try_load_existing_guid(StatusMessage, entity.guid, author) do
           StatusMessage.new(
-          author:                author_of(entity),
+            author:                author,
             guid:                  entity.guid,
             raw_message:           entity.raw_message,
             public:                entity.public,
@@ -130,12 +148,12 @@ module Diaspora
             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