From 5d318400c95f6a4d02af271969d0b980dbfad1e7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jonne=20Ha=C3=9F?= <me@jhass.eu>
Date: Sun, 26 Jun 2016 17:01:33 +0200
Subject: [PATCH] Diaspora::Federation::Receive::ignore_existing_guid returns
 nil when ignoring the error message

Previously it returned the return value of Logging::Logger#warn, which
is true for loggers that log the warn level. However
Diaspora::Federation::Receive::receive_relayable checks the return value
for truthiness in order to decide whether to attempt to relay it,
resulting in a NoMethodError: undefined method `parent' for
true:TrueClass in Diaspora::Federation::Receive::relay_relayable

This change is cosmetic as the exception raised prevented any action
that shouldn't happen anyway, so there's no actual logic change.
---
 lib/diaspora/federation/receive.rb |  1 +
 spec/shared_behaviors/receiving.rb | 25 +++++++++++++++++--------
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb
index 3aee007c02..827aa28031 100644
--- a/lib/diaspora/federation/receive.rb
+++ b/lib/diaspora/federation/receive.rb
@@ -285,6 +285,7 @@ module Diaspora
       rescue => e
         raise e unless load_from_database(klass, guid, author)
         logger.warn "ignoring error on receive #{klass}:#{guid}: #{e.class}: #{e.message}"
+        nil
       end
 
       # try to load the object first from the DB and if not available, save it.
diff --git a/spec/shared_behaviors/receiving.rb b/spec/shared_behaviors/receiving.rb
index d649ab51a3..78898dc8c7 100644
--- a/spec/shared_behaviors/receiving.rb
+++ b/spec/shared_behaviors/receiving.rb
@@ -2,22 +2,22 @@ require "spec_helper"
 
 shared_examples_for "it ignores existing object received twice" do |klass|
   it "return nil if the #{klass} already exists" do
-    expect(Diaspora::Federation::Receive.public_send(:perform, entity)).not_to be_nil
-    expect(Diaspora::Federation::Receive.public_send(:perform, entity)).to be_nil
+    expect(Diaspora::Federation::Receive.perform(entity)).not_to be_nil
+    expect(Diaspora::Federation::Receive.perform(entity)).to be_nil
   end
 
   it "does not change anything if the #{klass} already exists" do
-    Diaspora::Federation::Receive.public_send(:perform, entity)
+    Diaspora::Federation::Receive.perform(entity)
 
     expect_any_instance_of(klass).not_to receive(:create_or_update)
 
-    Diaspora::Federation::Receive.public_send(:perform, entity)
+    Diaspora::Federation::Receive.perform(entity)
   end
 end
 
 shared_examples_for "it rejects if the parent author ignores the author" do |klass|
   it "saves the relayable if the author is not ignored" do
-    Diaspora::Federation::Receive.public_send(:perform, entity)
+    Diaspora::Federation::Receive.perform(entity)
 
     expect(klass.find_by!(guid: entity.guid)).to be_instance_of(klass)
   end
@@ -29,7 +29,7 @@ shared_examples_for "it rejects if the parent author ignores the author" do |kla
 
     it "raises an error and does not save the relayable" do
       expect {
-        Diaspora::Federation::Receive.public_send(:perform, entity)
+        Diaspora::Federation::Receive.perform(entity)
       }.to raise_error Diaspora::Federation::AuthorIgnored
 
       expect(klass.find_by(guid: entity.guid)).to be_nil
@@ -47,7 +47,7 @@ shared_examples_for "it rejects if the parent author ignores the author" do |kla
       expect(dispatcher).to receive(:dispatch)
 
       expect {
-        Diaspora::Federation::Receive.public_send(:perform, entity)
+        Diaspora::Federation::Receive.perform(entity)
       }.to raise_error Diaspora::Federation::AuthorIgnored
     end
   end
@@ -61,6 +61,15 @@ shared_examples_for "it relays relayables" do |klass|
       expect(relayable.guid).to eq(entity.guid)
     end
 
-    Diaspora::Federation::Receive.public_send(:perform, entity)
+    Diaspora::Federation::Receive.perform(entity)
+  end
+
+  it "does not dispatch the received relayable if there was an error saving it and it exists already" do
+    allow_any_instance_of(klass).to receive(:save!).and_raise(RuntimeError, "something went wrong")
+    allow(Diaspora::Federation::Receive).to receive(:load_from_database).and_return(true)
+
+    expect(Diaspora::Federation::Dispatcher).to_not receive(:defer_dispatch)
+
+    Diaspora::Federation::Receive.perform(entity)
   end
 end
-- 
GitLab