Try fetching missing parent of relayables

* Extract post fetching logic from Reshare into
  its own module
* raise proper error message when fetching fails
* raise proper error message when parent is still missing

We can't skip fetch failures or missing parents and
still need to retry them in case we're sent the parent
later on

app/models/reshare.rb

@@ -74,35 +74,16 @@ class Reshare < Post
   private
 
   def after_parse
-    root_author = Webfinger.new(@root_diaspora_id).fetch
+    if root.blank?
-    root_author.save! unless root_author.persisted?
+      self.root = Diaspora::Fetcher::Single.find_or_fetch_from_remote root_guid, @root_diaspora_id do |fetched_post, author|
-
+        # why do we check this?
-    return if Post.exists?(:guid => self.root_guid)
+        if fetched_post.diaspora_handle != author.diaspora_handle
-
+          raise Diaspora::PostNotFetchable, "Diaspora ID (#{fetched_post.diaspora_handle}) in the root does not match the Diaspora ID (#{author.diaspora_handle}) specified in the reshare!"
-    fetched_post = self.class.fetch_post(root_author, self.root_guid)
+        end
-
-    if fetched_post
-      #Why are we checking for this?
-      if root_author.diaspora_handle != fetched_post.diaspora_handle
-        raise "Diaspora ID (#{fetched_post.diaspora_handle}) in the root does not match the Diaspora ID (#{root_author.diaspora_handle}) specified in the reshare!"
       end
-
-      fetched_post.save!
     end
   end
 
-  # Fetch a remote public post, used for receiving reshares of unknown posts
-  # @param [Person] author the remote post's author
-  # @param [String] guid the remote post's guid
-  # @return [Post] an unsaved remote post or false if the post was not found
-  def self.fetch_post author, guid
-    url = author.url + "/p/#{guid}.xml"
-    response = Faraday.get(url)
-    return false if response.status == 404 # Old pod, friendika
-    raise "Failed to get #{url}" unless response.success? # Other error, N/A for example
-    Diaspora::Parser.from_xml(response.body)
-  end
-
   def root_must_be_public
     if self.root && !self.root.public
       errors[:base] << "Only posts which are public may be reshared."

lib/diaspora/exceptions.rb

@@ -16,7 +16,6 @@ module Diaspora
   # that prevents further execution
   class NotMine < StandardError
   end
-  
 
   # Received a message without having a contact
   class ContactRequiredUnlessRequest < StandardError
@@ -30,4 +29,10 @@ module Diaspora
   # original XML message
   class AuthorXMLAuthorMismatch < StandardError
   end
+
+  # Tried to fetch a post but it was deleted, not valid
+  # or the remote end doesn't support post fetching
+  class PostNotFetchable < StandardError
+  end
+
 end

lib/diaspora/fetcher.rb

@@ -1,5 +1,6 @@
 module Diaspora
   module Fetcher
     require 'diaspora/fetcher/public'
+    require 'diaspora/fetcher/single'
   end
 end

lib/diaspora/fetcher/single.rb

@@ -0,0 +1,41 @@
+module Diaspora
+  module Fetcher
+    module Single
+      module_function
+
+      # Fetch and store a remote public post
+      # @param [String] guid the remote posts guid
+      # @param [String] author_id Diaspora ID of a user known to have the post,
+      #                 preferably the author
+      # @yield [Post, Person] If a block is given it is yielded the post
+      #                       and the author prior save
+      # @return a saved post
+      def find_or_fetch_from_remote guid, author_id
+        post = Post.where(guid: guid).first
+        return post if post
+
+        post_author = Webfinger.new(author_id).fetch
+        post_author.save! unless post_author.persisted?
+
+        if fetched_post = fetch_post(post_author, guid)
+          yield fetched_post, post_author if block_given?
+          raise Diaspora::PostNotFetchable unless fetched_post.save
+        end
+
+        fetched_post
+      end
+
+      # Fetch a remote public post, used for receiving of unknown public posts
+      # @param [Person] author the remote post's author
+      # @param [String] guid the remote post's guid
+      # @return [Post] an unsaved remote post or false if the post was not found
+      def fetch_post author, guid
+        url = author.url + "/p/#{guid}.xml"
+        response = Faraday.get(url)
+        raise Diaspora::PostNotFetchable if response.status == 404 # Old pod, Friendika, deleted
+        raise "Failed to get #{url}" unless response.success? # Other error, N/A for example
+        Diaspora::Parser.from_xml(response.body)
+      end
+    end
+  end
+end

lib/diaspora/relayable.rb

@@ -51,7 +51,8 @@ module Diaspora
     end
 
     def parent_guid= new_parent_guid
-      self.parent = parent_class.where(:guid => new_parent_guid).first
+      @parent_guid = new_parent_guid
+      self.parent = parent_class.where(guid: new_parent_guid).first
     end
 
     # @return [Array<Person>]
@@ -66,7 +67,7 @@ module Diaspora
     end
 
     def receive(user, person=nil)
-      comment_or_like = self.class.where(:guid => self.guid).first || self
+      comment_or_like = self.class.where(guid: self.guid).first || self
 
       # Check to make sure the signature of the comment or like comes from the person claiming to author it
       unless comment_or_like.parent_author == user.person || comment_or_like.verify_parent_author_signature
@@ -134,5 +135,17 @@ module Diaspora
     def parent= parent
       raise NotImplementedError.new('you must override parent= in order to enable relayable on this model')
     end
+
+    # ROXML hook ensuring our own hooks are called
+    def after_parse
+      if @parent_guid
+        self.parent ||= fetch_parent(@parent_guid)
+      end
+    end
+
+    # Childs should override this to support fetching a missing parent
+    # @param guid the parents guid
+    def fetch_parent guid
+    end
   end
 end

lib/federated/relayable.rb

@@ -38,5 +38,9 @@ module Federated
     def parent= parent
       self.target = parent
     end
+
+    def fetch_parent guid
+      Diaspora::Fetcher::Single.find_or_fetch_from_remote guid, diaspora_handle
+    end
   end
-end
+end

lib/postzord/receiver/public.rb

@@ -57,6 +57,7 @@ class Postzord::Receiver::Public < Postzord::Receiver
   def save_object
     @object = Diaspora::Parser::from_xml(@salmon.parsed_data)
     raise "Object is not public" if object_can_be_public_and_it_is_not?
+    raise Diaspora::RelayableObjectWithoutParent if object_must_have_parent_and_does_not?
     raise Diaspora::AuthorXMLAuthorMismatch if author_does_not_match_xml_author?
     @object.save! if @object && @object.respond_to?(:save!)
     @object
@@ -88,4 +89,12 @@ class Postzord::Receiver::Public < Postzord::Receiver
   def object_can_be_public_and_it_is_not?
     @object.respond_to?(:public) && !@object.public?
   end
+
+  def object_must_have_parent_and_does_not?
+    if @object.respond_to?(:relayable?) # comment, like
+      @object.parent.nil?
+    else
+      false
+    end
+  end
 end

spec/models/comment_spec.rb

@@ -96,6 +96,13 @@ describe Comment, :type => :model do
       it 'marshals the post' do
         expect(@marshalled_comment.post).to eq(@post)
       end
+
+      it 'tries to fetch a missing parent' do
+        guid = @post.guid
+        @post.destroy
+        expect_any_instance_of(Comment).to receive(:fetch_parent).with(guid).and_return(nil)
+        Comment.from_xml(@xml)
+      end
     end
   end
 

spec/models/reshare_spec.rb

@@ -83,7 +83,7 @@ describe Reshare, :type => :model do
       rs1 = FactoryGirl.build(:reshare, :root=>@sm)
       rs2 = FactoryGirl.build(:reshare, :root=>rs1)
       @rs3 = FactoryGirl.build(:reshare, :root=>rs2)
-      
+
      sm = FactoryGirl.create(:status_message, :author => alice.person, :public => true)
      rs1 = FactoryGirl.create(:reshare, :root => sm)
      @of_deleted = FactoryGirl.build(:reshare, :root => rs1)
@@ -194,13 +194,13 @@ describe Reshare, :type => :model do
         end
 
         context "fetching post" do
-          it "doesn't error out if the post is not found" do
+          it "raises if the post is not found" do
             allow(@response).to receive(:status).and_return(404)
             expect(Faraday.default_connection).to receive(:get).and_return(@response)
 
             expect {
               Reshare.from_xml(@xml)
-            }.to_not raise_error
+            }.to raise_error(Diaspora::PostNotFetchable)
           end
 
           it "raises if there's another error receiving the post" do