From 13c16b4e95c134d883b54e796e842c7f72cbbe21 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 4 May 2017 04:34:57 +0200 Subject: [PATCH] Likely fix #2458, fix #2031 - handle out-of-order deletes for statuses (#2734) * Likely fix #2458, fix #2031 - handle out-of-order deletes for statuses If a delete arrives before the original status, cache that information for 6h, and if the original status arrives in that window, ignore it * Add test case --- app/services/process_feed_service.rb | 17 ++++++++- spec/services/process_feed_service_spec.rb | 44 ++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/app/services/process_feed_service.rb b/app/services/process_feed_service.rb index 4d23a6262..70c05c938 100644 --- a/app/services/process_feed_service.rb +++ b/app/services/process_feed_service.rb @@ -42,6 +42,11 @@ class ProcessFeedService < BaseService private def create_status + if redis.exists("delete_upon_arrival:#{id}") + Rails.logger.debug "Delete for status #{id} was queued, ignoring" + return + end + Rails.logger.debug "Creating remote status #{id}" status, just_created = status_from_xml(@xml) @@ -84,7 +89,13 @@ class ProcessFeedService < BaseService def delete_status Rails.logger.debug "Deleting remote status #{id}" status = Status.find_by(uri: id) - RemoveStatusService.new.call(status) unless status.nil? + + if status.nil? + redis.setex("delete_upon_arrival:#{id}", 6 * 3_600, id) + else + RemoveStatusService.new.call(status) + end + nil end @@ -273,5 +284,9 @@ class ProcessFeedService < BaseService def account?(xml = @xml) !xml.at_xpath('./xmlns:author', xmlns: TagManager::XMLNS).nil? end + + def redis + Redis.current + end end end diff --git a/spec/services/process_feed_service_spec.rb b/spec/services/process_feed_service_spec.rb index 19054ecd4..f16dda4d4 100644 --- a/spec/services/process_feed_service_spec.rb +++ b/spec/services/process_feed_service_spec.rb @@ -154,4 +154,48 @@ XML expect(created_statuses.first.reblog.account_id).to eq good_actor.id expect(created_statuses.first.reblog.text).to eq 'Overwatch rocks' end + + it 'ignores statuses with an out-of-order delete' do + sender = Fabricate(:account, username: 'tracer', domain: 'overwatch.com') + + delete_body = < + + tag:overwatch.com,2017-04-27:objectId=4487555:objectType=Status + 2017-04-27T13:49:25Z + 2017-04-27T13:49:25Z + http://activitystrea.ms/schema/1.0/note + http://activitystrea.ms/schema/1.0/delete + + https://overwatch.com/users/tracer + http://activitystrea.ms/schema/1.0/person + https://overwatch.com/users/tracer + tracer + + +XML + + status_body = < + + tag:overwatch.com,2017-04-27:objectId=4487555:objectType=Status + 2017-04-27T13:49:25Z + 2017-04-27T13:49:25Z + http://activitystrea.ms/schema/1.0/note + http://activitystrea.ms/schema/1.0/post + + https://overwatch.com/users/tracer + http://activitystrea.ms/schema/1.0/person + https://overwatch.com/users/tracer + tracer + + Overwatch rocks + +XML + + subject.call(delete_body, sender) + created_statuses = subject.call(status_body, sender) + + expect(created_statuses).to be_empty + end end