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

diff --git a/app/lib/activitypub/activity/announce.rb b/app/lib/activitypub/activity/announce.rb
index decf8f9601c46266ff8bf701afdb5e2e0f74c5a2..09fec28a080715998c7ab2b1c8e918be7393cb03 100644 (file)
--- 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 77d66fba3c5e8c81ff2f32150599ebbf5f25062d..1541257590d1b607d02afea4bc186274b05d38d4 100644 (file)
--- 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 bc8eb8a35616ee4fab2af3877b1558b34bb3caee..4061b9ce4560e146bc31c57cb4306699cf9567be 100644 (file)
--- 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 (file)
index 4ef762d..0000000
--- 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 (file)
index b954cb6..0000000
--- 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