Change ActivityPub path generation to all happen in ActivityPub::TagManager (#33527)

app/controllers/activitypub/collections_controller.rb

@@ -49,7 +49,7 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController
 
   def collection_presenter
     ActivityPub::CollectionPresenter.new(
-      id: account_collection_url(@account, params[:id]),
+      id: ActivityPub::TagManager.instance.collection_uri_for(@account, params[:id]),
       type: @type,
       size: @size,
       items: @items

app/controllers/activitypub/outboxes_controller.rb

@@ -41,12 +41,8 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController
     end
   end
 
-  def outbox_url(**)
+  def outbox_url(...)
-    if params[:account_username].present?
+    ActivityPub::TagManager.instance.outbox_uri_for(@account, ...)
-      account_outbox_url(@account, **)
-    else
-      instance_actor_outbox_url(**)
-    end
   end
 
   def next_page

app/controllers/follower_accounts_controller.rb

@@ -46,7 +46,7 @@ class FollowerAccountsController < ApplicationController
   end
 
   def page_url(page)
-    account_followers_url(@account, page: page) unless page.nil?
+    ActivityPub::TagManager.instance.followers_uri_for(@account, page: page) unless page.nil?
   end
 
   def next_page_url

app/lib/activitypub/tag_manager.rb

@@ -86,8 +86,34 @@ class ActivityPub::TagManager
     account_status_shares_url(target.account, target)
   end
 
-  def followers_uri_for(target)
+  def following_uri_for(target, ...)
-    target.local? ? account_followers_url(target) : target.followers_url.presence
+    raise ArgumentError, 'target must be a local account' unless target.local?
+
+    account_following_index_url(target, ...)
+  end
+
+  def followers_uri_for(target, ...)
+    return target.followers_url.presence unless target.local?
+
+    account_followers_url(target, ...)
+  end
+
+  def collection_uri_for(target, ...)
+    raise NotImplementedError unless target.local?
+
+    account_collection_url(target, ...)
+  end
+
+  def inbox_uri_for(target)
+    raise NotImplementedError unless target.local?
+
+    target.instance_actor? ? instance_actor_inbox_url : account_inbox_url(target)
+  end
+
+  def outbox_uri_for(target, ...)
+    raise NotImplementedError unless target.local?
+
+    target.instance_actor? ? instance_actor_outbox_url(...) : account_outbox_url(target, ...)
   end
 
   # Primary audience of a status
@@ -99,7 +125,7 @@ class ActivityPub::TagManager
     when 'public'
       [COLLECTIONS[:public]]
     when 'unlisted', 'private'
-      [account_followers_url(status.account)]
+      [followers_uri_for(status.account)]
     when 'direct', 'limited'
       if status.account.silenced?
         # Only notify followers if the account is locally silenced
@@ -133,7 +159,7 @@ class ActivityPub::TagManager
 
     case status.visibility
     when 'public'
-      cc << account_followers_url(status.account)
+      cc << followers_uri_for(status.account)
     when 'unlisted'
       cc << COLLECTIONS[:public]
     end

app/serializers/activitypub/actor_serializer.rb

@@ -44,7 +44,7 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer
   delegate :suspended?, :instance_actor?, to: :object
 
   def id
-    object.instance_actor? ? instance_actor_url : account_url(object)
+    ActivityPub::TagManager.instance.uri_for(object)
   end
 
   def type
@@ -60,27 +60,27 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer
   end
 
   def following
-    account_following_index_url(object)
+    ActivityPub::TagManager.instance.following_uri_for(object)
   end
 
   def followers
-    account_followers_url(object)
+    ActivityPub::TagManager.instance.followers_uri_for(object)
   end
 
   def inbox
-    object.instance_actor? ? instance_actor_inbox_url : account_inbox_url(object)
+    ActivityPub::TagManager.instance.inbox_uri_for(object)
   end
 
   def outbox
-    object.instance_actor? ? instance_actor_outbox_url : account_outbox_url(object)
+    ActivityPub::TagManager.instance.outbox_uri_for(object)
   end
 
   def featured
-    account_collection_url(object, :featured)
+    ActivityPub::TagManager.instance.collection_uri_for(object, :featured)
   end
 
   def featured_tags
-    account_collection_url(object, :tags)
+    ActivityPub::TagManager.instance.collection_uri_for(object, :tags)
   end
 
   def endpoints

app/serializers/activitypub/add_serializer.rb

@@ -38,6 +38,6 @@ class ActivityPub::AddSerializer < ActivityPub::Serializer
   end
 
   def target
-    account_collection_url(object.account, :featured)
+    ActivityPub::TagManager.instance.collection_uri_for(object.account, :featured)
   end
 end

app/serializers/activitypub/remove_serializer.rb

@@ -38,6 +38,6 @@ class ActivityPub::RemoveSerializer < ActivityPub::Serializer
   end
 
   def target
-    account_collection_url(object.account, :featured)
+    ActivityPub::TagManager.instance.collection_uri_for(object.account, :featured)
   end
 end

app/serializers/webfinger_serializer.rb

@@ -13,7 +13,7 @@ class WebfingerSerializer < ActiveModel::Serializer
     if object.instance_actor?
       [instance_actor_url]
     else
-      [short_account_url(object), account_url(object)]
+      [short_account_url(object), ActivityPub::TagManager.instance.uri_for(object)]
     end
   end
 
@@ -43,6 +43,6 @@ class WebfingerSerializer < ActiveModel::Serializer
   end
 
   def self_href
-    object.instance_actor? ? instance_actor_url : account_url(object)
+    ActivityPub::TagManager.instance.uri_for(object)
   end
 end

spec/lib/activitypub/tag_manager_spec.rb

@@ -7,17 +7,50 @@ RSpec.describe ActivityPub::TagManager do
 
   subject { described_class.instance }
 
+  let(:domain) { "#{Rails.configuration.x.use_https ? 'https' : 'http'}://#{Rails.configuration.x.web_domain}" }
+
+  describe '#public_collection?' do
+    it 'returns true for the special public collection and common shorthands' do
+      expect(subject.public_collection?('https://www.w3.org/ns/activitystreams#Public')).to be true
+      expect(subject.public_collection?('as:Public')).to be true
+      expect(subject.public_collection?('Public')).to be true
+    end
+
+    it 'returns false for other URIs' do
+      expect(subject.public_collection?('https://example.com/foo/bar')).to be false
+    end
+  end
+
   describe '#url_for' do
-    it 'returns a string' do
+    it 'returns a string starting with web domain' do
       account = Fabricate(:account)
-      expect(subject.url_for(account)).to be_a String
+      expect(subject.url_for(account)).to be_a(String)
+        .and start_with(domain)
     end
   end
 
   describe '#uri_for' do
-    it 'returns a string' do
+    it 'returns a string starting with web domain' do
       account = Fabricate(:account)
-      expect(subject.uri_for(account)).to be_a String
+      expect(subject.uri_for(account)).to be_a(String)
+        .and start_with(domain)
+    end
+  end
+
+  describe '#activity_uri_for' do
+    context 'when given an account' do
+      it 'raises an exception' do
+        account = Fabricate(:account)
+        expect { subject.activity_uri_for(account) }.to raise_error(ArgumentError)
+      end
+    end
+
+    context 'when given a local activity' do
+      it 'returns a string starting with web domain' do
+        status = Fabricate(:status)
+        expect(subject.uri_for(status)).to be_a(String)
+          .and start_with(domain)
+      end
     end
   end