From a81bdac38c5da390199ebb9f4dde4b3f7d542733 Mon Sep 17 00:00:00 2001 From: Benjamin Neff Date: Sat, 18 Jun 2016 21:33:46 +0200 Subject: [PATCH] don't send relayables back to sender pod send retraction for relayable to target author if retracted by parent author --- lib/diaspora/federated/retraction.rb | 8 ++++- lib/diaspora/federation/receive.rb | 2 +- lib/diaspora/relayable.rb | 2 +- .../lib/diaspora/federated/retraction_spec.rb | 25 ++++++++++++++++ spec/lib/diaspora/federation/receive_spec.rb | 2 +- spec/shared_behaviors/relayable.rb | 29 ++++++++++++------- 6 files changed, 54 insertions(+), 14 deletions(-) diff --git a/lib/diaspora/federated/retraction.rb b/lib/diaspora/federated/retraction.rb index 7c7ce2a26..7c71c8591 100644 --- a/lib/diaspora/federated/retraction.rb +++ b/lib/diaspora/federated/retraction.rb @@ -27,7 +27,8 @@ class Retraction new(federation_retraction.to_h, target.subscribers.select(&:remote?), target) end - def defer_dispatch(user) + def defer_dispatch(user, include_target_author=true) + subscribers = dispatch_subscribers(include_target_author) Workers::DeferredRetraction.perform_async(user.id, data, subscribers.map(&:id), service_opts(user)) end @@ -46,6 +47,11 @@ class Retraction attr_reader :target + def dispatch_subscribers(include_target_author) + subscribers << target.author if target.is_a?(Diaspora::Relayable) && include_target_author && target.author.remote? + subscribers + end + def service_opts(user) return {} unless target.is_a?(StatusMessage) diff --git a/lib/diaspora/federation/receive.rb b/lib/diaspora/federation/receive.rb index 9de2c79e5..c31f21ba6 100644 --- a/lib/diaspora/federation/receive.rb +++ b/lib/diaspora/federation/receive.rb @@ -143,7 +143,7 @@ module Diaspora if object.parent.author.local? parent_author = object.parent.author.owner retraction = Retraction.for(object, parent_author) - retraction.defer_dispatch(parent_author) + retraction.defer_dispatch(parent_author, false) retraction.perform else object.destroy! diff --git a/lib/diaspora/relayable.rb b/lib/diaspora/relayable.rb index 247fd827b..e3a481d05 100644 --- a/lib/diaspora/relayable.rb +++ b/lib/diaspora/relayable.rb @@ -33,7 +33,7 @@ module Diaspora if author.local? parent.subscribers else - parent.subscribers.select(&:remote?) + parent.subscribers.select(&:remote?).reject {|person| person.pod_id == author.pod_id } end else [parent.author, author] diff --git a/spec/lib/diaspora/federated/retraction_spec.rb b/spec/lib/diaspora/federated/retraction_spec.rb index ecc7e3cd7..c7b6fb941 100644 --- a/spec/lib/diaspora/federated/retraction_spec.rb +++ b/spec/lib/diaspora/federated/retraction_spec.rb @@ -98,6 +98,31 @@ describe Retraction do Retraction.for(comment, local_luke).defer_dispatch(local_luke) end + + context "relayable" do + let(:post) { local_luke.post(:status_message, text: "hello", public: true) } + let(:comment) { FactoryGirl.create(:comment, post: post, author: remote_raphael) } + + it "sends retraction to target author if deleted by parent author" do + federation_retraction = Diaspora::Federation::Entities.relayable_retraction(comment, local_luke) + + expect(Workers::DeferredRetraction).to receive(:perform_async).with( + local_luke.id, federation_retraction.to_h, [remote_raphael.id], {} + ) + + Retraction.for(comment, local_luke).defer_dispatch(local_luke) + end + + it "don't sends retraction back to target author if relayed by parent author" do + federation_retraction = Diaspora::Federation::Entities.relayable_retraction(comment, local_luke) + + expect(Workers::DeferredRetraction).to receive(:perform_async).with( + local_luke.id, federation_retraction.to_h, [], {} + ) + + Retraction.for(comment, local_luke).defer_dispatch(local_luke, false) + end + end end describe "#perform" do diff --git a/spec/lib/diaspora/federation/receive_spec.rb b/spec/lib/diaspora/federation/receive_spec.rb index 3d2b7022a..b123a4811 100644 --- a/spec/lib/diaspora/federation/receive_spec.rb +++ b/spec/lib/diaspora/federation/receive_spec.rb @@ -468,7 +468,7 @@ describe Diaspora::Federation::Receive do comment_retraction = Retraction.for(remote_comment, alice) expect(Retraction).to receive(:for).with(instance_of(Comment), alice).and_return(comment_retraction) - expect(comment_retraction).to receive(:defer_dispatch).with(alice) + expect(comment_retraction).to receive(:defer_dispatch).with(alice, false) expect(comment_retraction).to receive(:perform).and_call_original expect_any_instance_of(Comment).to receive(:destroy!).and_call_original diff --git a/spec/shared_behaviors/relayable.rb b/spec/shared_behaviors/relayable.rb index 912ce1073..1043ca463 100644 --- a/spec/shared_behaviors/relayable.rb +++ b/spec/shared_behaviors/relayable.rb @@ -45,18 +45,27 @@ shared_examples_for "it is relayable" do end describe "#subscribers" do - it "returns the parents original audience, if the parent is local" do - expect(object_on_local_parent.subscribers.map(&:id)) - .to match_array([local_leia.person, remote_raphael].map(&:id)) + context "parent is local" do + it "returns the parents original audience, if author is local" do + expect(object_on_local_parent.subscribers.map(&:id)) + .to match_array([local_leia.person, remote_raphael].map(&:id)) + end + + it "returns remote persons of the parents original audience not on same pod as the author, if author is remote" do + person1 = FactoryGirl.create(:person, pod: remote_raphael.pod) + person2 = FactoryGirl.create(:person, pod: FactoryGirl.create(:pod)) + local_luke.share_with(person1, local_luke.aspects.first) + local_luke.share_with(person2, local_luke.aspects.first) + + expect(remote_object_on_local_parent.subscribers.map(&:id)).to match_array([person2].map(&:id)) + end end - it "returns remote persons of the parents original audience, if the parent is local, but the author is remote" do - expect(remote_object_on_local_parent.subscribers.map(&:id)).to match_array([remote_raphael].map(&:id)) - end - - it "returns the author of parent and author of relayable (for local delivery), if the parent is not local" do - expect(object_on_remote_parent.subscribers.map(&:id)) - .to match_array([remote_raphael, local_luke.person].map(&:id)) + context "parent is remote" do + it "returns the author of parent and author of relayable (for local delivery)" do + expect(object_on_remote_parent.subscribers.map(&:id)) + .to match_array([remote_raphael, local_luke.person].map(&:id)) + end end end end