Fix cross-server conversation tracking (#37559)

app/lib/activitypub/activity/create.rb

@@ -379,6 +379,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
   def conversation_from_uri(uri)
     return nil if uri.nil?
     return Conversation.find_by(id: OStatus::TagManager.instance.unique_tag_to_local_id(uri, 'Conversation')) if OStatus::TagManager.instance.local_id?(uri)
+    return ActivityPub::TagManager.instance.uri_to_resource(uri, Conversation) if ActivityPub::TagManager.instance.local_uri?(uri)
 
     begin
       Conversation.find_or_create_by!(uri: uri)

app/lib/activitypub/tag_manager.rb

@@ -241,12 +241,6 @@ class ActivityPub::TagManager
     !host.nil? && (::TagManager.instance.local_domain?(host) || ::TagManager.instance.web_domain?(host))
   end
 
-  def uri_to_local_id(uri, param = :id)
-    path_params = Rails.application.routes.recognize_path(uri)
-    path_params[:username] = Rails.configuration.x.local_domain if path_params[:controller] == 'instance_actors'
-    path_params[param]
-  end
-
   def uris_to_local_accounts(uris)
     usernames = []
     ids = []
@@ -264,6 +258,14 @@ class ActivityPub::TagManager
     uri_to_resource(uri, Account)
   end
 
+  def uri_to_local_conversation(uri)
+    path_params = Rails.application.routes.recognize_path(uri)
+    return unless path_params[:controller] == 'activitypub/contexts'
+
+    account_id, conversation_id = path_params[:id].split('-')
+    Conversation.find_by(parent_account_id: account_id, id: conversation_id)
+  end
+
   def uri_to_resource(uri, klass)
     return if uri.nil?
 
@@ -271,6 +273,8 @@ class ActivityPub::TagManager
       case klass.name
       when 'Account'
         uris_to_local_accounts([uri]).first
+      when 'Conversation'
+        uri_to_local_conversation(uri)
       else
         StatusFinder.new(uri).status
       end

app/lib/ostatus/tag_manager.rb

@@ -11,16 +11,12 @@ class OStatus::TagManager
   def unique_tag_to_local_id(tag, expected_type)
     return nil unless local_id?(tag)
 
-    if ActivityPub::TagManager.instance.local_uri?(tag)
-      ActivityPub::TagManager.instance.uri_to_local_id(tag)
-    else
     matches = Regexp.new("objectId=([\\d]+):objectType=#{expected_type}").match(tag)
     matches[1] unless matches.nil?
   end
-  end
 
   def local_id?(id)
-    id.start_with?("tag:#{Rails.configuration.x.local_domain}") || ActivityPub::TagManager.instance.local_uri?(id)
+    id.start_with?("tag:#{Rails.configuration.x.local_domain}")
   end
 
   def uri_for(target)

spec/lib/activitypub/activity/create_spec.rb

@@ -471,7 +471,7 @@ RSpec.describe ActivityPub::Activity::Create do
         end
       end
 
-      context 'with a reply' do
+      context 'with a reply without explicitly setting a conversation' do
         let(:original_status) { Fabricate(:status) }
 
         let(:object_json) do
@@ -493,6 +493,30 @@ RSpec.describe ActivityPub::Activity::Create do
         end
       end
 
+      context 'with a reply explicitly setting a conversation' do
+        let(:original_status) { Fabricate(:status) }
+
+        let(:object_json) do
+          build_object(
+            inReplyTo: ActivityPub::TagManager.instance.uri_for(original_status),
+            conversation: ActivityPub::TagManager.instance.uri_for(original_status.conversation),
+            context: ActivityPub::TagManager.instance.uri_for(original_status.conversation)
+          )
+        end
+
+        it 'creates status' do
+          expect { subject.perform }.to change(sender.statuses, :count).by(1)
+
+          status = sender.statuses.first
+
+          expect(status).to_not be_nil
+          expect(status.thread).to eq original_status
+          expect(status.reply?).to be true
+          expect(status.in_reply_to_account).to eq original_status.account
+          expect(status.conversation).to eq original_status.conversation
+        end
+      end
+
       context 'with mentions' do
         let(:recipient) { Fabricate(:account) }
 

spec/lib/activitypub/tag_manager_spec.rb

@@ -612,14 +612,6 @@ RSpec.describe ActivityPub::TagManager do
     end
   end
 
-  describe '#uri_to_local_id' do
-    let(:account) { Fabricate(:account, id_scheme: :username_ap_id) }
-
-    it 'returns the local ID' do
-      expect(subject.uri_to_local_id(subject.uri_for(account), :username)).to eq account.username
-    end
-  end
-
   describe '#uris_to_local_accounts' do
     it 'returns the expected local accounts' do
       account = Fabricate(:account)