From 412ea873060da4dc73236fdd63a2931d27dbfa40 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sat, 19 Aug 2017 18:44:48 +0200 Subject: [PATCH] Improve ActivityPub/OStatus compatibility (#4632) *Note: OStatus URIs are invalid for ActivityPub. But we have them for as long as we want to keep old OStatus-sourced content and as long as we remain OStatus-compatible.* - In Announce handling, if object URI is not a URL, fallback to object URL - Do not use specialized ThreadResolveWorker, rely on generalized handling - When serializing notes, if parent's URI is not a URL, use parent's URL --- app/lib/activitypub/activity/announce.rb | 14 ++++++++++++-- app/lib/activitypub/activity/create.rb | 2 +- app/serializers/activitypub/note_serializer.rb | 8 +++++++- .../activitypub/thread_resolve_worker.rb | 17 ----------------- .../activitypub/thread_resolve_worker_spec.rb | 16 ---------------- 5 files changed, 20 insertions(+), 37 deletions(-) delete mode 100644 app/workers/activitypub/thread_resolve_worker.rb delete mode 100644 spec/workers/activitypub/thread_resolve_worker_spec.rb diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb index decf8f9601..09fec28a08 100644 --- a/app/lib/activitypub/activity/announce.rb +++ b/app/lib/activitypub/activity/announce.rb @@ -2,8 +2,8 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity def perform - original_status = status_from_uri(object_uri) - original_status = ActivityPub::FetchRemoteStatusService.new.call(object_uri) if original_status.nil? + original_status = status_from_uri(object_uri) + original_status ||= fetch_remote_original_status return if original_status.nil? || delete_arrived_first?(@json['id']) @@ -11,4 +11,14 @@ class ActivityPub::Activity::Announce < ActivityPub::Activity distribute(status) status end + + private + + def fetch_remote_original_status + if object_uri.start_with?('http') + ActivityPub::FetchRemoteStatusService.new.call(object_uri) + elsif @object['url'].present? + ::FetchRemoteStatusService.new.call(@object['url']) + end + end end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 77d66fba3c..1541257590 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -91,7 +91,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity def resolve_thread(status) return unless status.reply? && status.thread.nil? - ActivityPub::ThreadResolveWorker.perform_async(status.id, @object['inReplyTo']) + ThreadResolveWorker.perform_async(status.id, @object['inReplyTo']) end def conversation_from_uri(uri) diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index bc8eb8a356..4061b9ce45 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -27,7 +27,13 @@ class ActivityPub::NoteSerializer < ActiveModel::Serializer end def in_reply_to - ActivityPub::TagManager.instance.uri_for(object.thread) if object.reply? + return unless object.reply? + + if object.thread.uri.nil? || object.thread.uri.start_with?('http') + ActivityPub::TagManager.instance.uri_for(object.thread) + else + object.thread.url + end end def published diff --git a/app/workers/activitypub/thread_resolve_worker.rb b/app/workers/activitypub/thread_resolve_worker.rb deleted file mode 100644 index 4ef762d064..0000000000 --- a/app/workers/activitypub/thread_resolve_worker.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -class ActivityPub::ThreadResolveWorker - include Sidekiq::Worker - - sidekiq_options queue: 'pull', retry: false - - def perform(child_status_id, parent_uri) - child_status = Status.find(child_status_id) - parent_status = ActivityPub::FetchRemoteStatusService.new.call(parent_uri) - - return if parent_status.nil? - - child_status.thread = parent_status - child_status.save! - end -end diff --git a/spec/workers/activitypub/thread_resolve_worker_spec.rb b/spec/workers/activitypub/thread_resolve_worker_spec.rb deleted file mode 100644 index b954cb62ca..0000000000 --- a/spec/workers/activitypub/thread_resolve_worker_spec.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'rails_helper' - -describe ActivityPub::ThreadResolveWorker do - subject { described_class.new } - - let(:status) { Fabricate(:status) } - let(:parent) { Fabricate(:status) } - - describe '#perform' do - it 'gets parent from ActivityPub::FetchRemoteStatusService and glues them together' do - allow(ActivityPub::FetchRemoteStatusService).to receive(:new).and_return(double(:service, call: parent)) - subject.perform(status.id, 'http://example.com/123') - expect(status.reload.in_reply_to_id).to eq parent.id - end - end -end