From 200fda1cdc7457b539a7c4e1050f6901bb3245bd Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Tue, 9 Jun 2026 16:22:21 +0200 Subject: [PATCH] Add collection reach finder (#39346) --- app/lib/activitypub/activity/accept.rb | 2 +- app/lib/collection_reach_finder.rb | 18 ++++++++++ app/models/collection.rb | 1 + .../add_account_to_collection_service.rb | 2 +- app/services/create_collection_service.rb | 2 +- .../delete_collection_item_service.rb | 2 +- app/services/delete_collection_service.rb | 8 +++-- .../revoke_collection_item_service.rb | 2 +- app/services/update_collection_service.rb | 2 +- .../collection_raw_distribution_worker.rb | 15 ++++++++ spec/lib/activitypub/activity/accept_spec.rb | 4 +-- spec/lib/activitypub/activity/delete_spec.rb | 2 +- spec/lib/collection_reach_finder_spec.rb | 35 +++++++++++++++++++ .../add_account_to_collection_service_spec.rb | 4 +-- .../create_collection_service_spec.rb | 2 +- .../delete_collection_item_service_spec.rb | 4 +-- .../delete_collection_service_spec.rb | 7 +++- .../revoke_collection_item_service_spec.rb | 2 +- .../update_collection_service_spec.rb | 4 +-- 19 files changed, 98 insertions(+), 20 deletions(-) create mode 100644 app/lib/collection_reach_finder.rb create mode 100644 app/workers/activitypub/collection_raw_distribution_worker.rb create mode 100644 spec/lib/collection_reach_finder_spec.rb diff --git a/app/lib/activitypub/activity/accept.rb b/app/lib/activitypub/activity/accept.rb index a76b79a6d8..67f211f8a1 100644 --- a/app/lib/activitypub/activity/accept.rb +++ b/app/lib/activitypub/activity/accept.rb @@ -53,7 +53,7 @@ class ActivityPub::Activity::Accept < ActivityPub::Activity collection_item.update!(approval_uri:, state: :accepted) activity_json = ActiveModelSerializers::SerializableResource.new(collection_item, serializer: ActivityPub::AddFeaturedItemSerializer, adapter: ActivityPub::Adapter).to_json - ActivityPub::AccountRawDistributionWorker.perform_async(activity_json, collection_item.collection.account_id) + ActivityPub::CollectionRawDistributionWorker.perform_async(activity_json, collection_item.collection_id) end def accept_quote!(quote) diff --git a/app/lib/collection_reach_finder.rb b/app/lib/collection_reach_finder.rb new file mode 100644 index 0000000000..3f2829c55e --- /dev/null +++ b/app/lib/collection_reach_finder.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CollectionReachFinder < AccountReachFinder + def initialize(collection) + @collection = collection + super(@collection.account) + end + + def inboxes + (super + collection_member_inboxes).uniq + end + + private + + def collection_member_inboxes + @collection.accounts.inboxes + end +end diff --git a/app/models/collection.rb b/app/models/collection.rb index ac76331659..337126b145 100644 --- a/app/models/collection.rb +++ b/app/models/collection.rb @@ -32,6 +32,7 @@ class Collection < ApplicationRecord has_many :collection_items, dependent: :delete_all has_many :accepted_collection_items, -> { accepted }, class_name: 'CollectionItem', inverse_of: :collection # rubocop:disable Rails/HasManyOrHasOneDependent has_many :collection_reports, dependent: :delete_all + has_many :accounts, -> { merge(CollectionItem.pending_or_accepted) }, through: :collection_items validates :name, presence: true validates :name, length: { maximum: 40 }, if: :local? diff --git a/app/services/add_account_to_collection_service.rb b/app/services/add_account_to_collection_service.rb index 3c2ecacf4f..f5438a6f9e 100644 --- a/app/services/add_account_to_collection_service.rb +++ b/app/services/add_account_to_collection_service.rb @@ -30,7 +30,7 @@ class AddAccountToCollectionService end def distribute_add_activity - ActivityPub::AccountRawDistributionWorker.perform_async(add_activity_json, @collection.account_id) + ActivityPub::CollectionRawDistributionWorker.perform_async(add_activity_json, @collection.id) end def distribute_feature_request_activity diff --git a/app/services/create_collection_service.rb b/app/services/create_collection_service.rb index 1c46b40530..4e88108ca0 100644 --- a/app/services/create_collection_service.rb +++ b/app/services/create_collection_service.rb @@ -19,7 +19,7 @@ class CreateCollectionService private def distribute_add_activity - ActivityPub::AccountRawDistributionWorker.perform_async(activity_json, @account.id) + ActivityPub::CollectionRawDistributionWorker.perform_async(activity_json, @collection.id) end def distribute_feature_request_activities diff --git a/app/services/delete_collection_item_service.rb b/app/services/delete_collection_item_service.rb index 3f91ea0c3b..04ff4b1263 100644 --- a/app/services/delete_collection_item_service.rb +++ b/app/services/delete_collection_item_service.rb @@ -16,7 +16,7 @@ class DeleteCollectionItemService private def distribute_remove_activity - ActivityPub::AccountRawDistributionWorker.perform_async(activity_json, @collection.account.id) + ActivityPub::CollectionRawDistributionWorker.perform_async(activity_json, @collection.id) end def activity_json diff --git a/app/services/delete_collection_service.rb b/app/services/delete_collection_service.rb index c2cc6ebd6c..0414d26980 100644 --- a/app/services/delete_collection_service.rb +++ b/app/services/delete_collection_service.rb @@ -3,6 +3,7 @@ class DeleteCollectionService def call(collection) @collection = collection + @account_ids = @collection.account_ids @collection.destroy! distribute_remove_activity @@ -11,10 +12,13 @@ class DeleteCollectionService private def distribute_remove_activity - ActivityPub::AccountRawDistributionWorker.perform_async(activity_json, @collection.account.id) + @account_ids.each do |account_id| + ActivityPub::DeliveryWorker.perform_async(activity_json, account_id, @collection.account.inbox_url) + end + ActivityPub::AccountRawDistributionWorker.perform_async(activity_json, @collection.account_id) end def activity_json - ActiveModelSerializers::SerializableResource.new(@collection, serializer: ActivityPub::RemoveFeaturedCollectionSerializer, adapter: ActivityPub::Adapter).to_json + @activity_json ||= ActiveModelSerializers::SerializableResource.new(@collection, serializer: ActivityPub::RemoveFeaturedCollectionSerializer, adapter: ActivityPub::Adapter).to_json end end diff --git a/app/services/revoke_collection_item_service.rb b/app/services/revoke_collection_item_service.rb index 0b3c2c709e..a5222d2b62 100644 --- a/app/services/revoke_collection_item_service.rb +++ b/app/services/revoke_collection_item_service.rb @@ -17,7 +17,7 @@ class RevokeCollectionItemService < BaseService def distribute_stamp_deletion! ActivityPub::DeliveryWorker.perform_async(signed_activity_json, @account.id, @collection.account.inbox_url) - ActivityPub::AccountRawDistributionWorker.perform_async(signed_activity_json, @collection.account_id) + ActivityPub::CollectionRawDistributionWorker.perform_async(signed_activity_json, @collection.id) end def signed_activity_json diff --git a/app/services/update_collection_service.rb b/app/services/update_collection_service.rb index 5ffb4bad81..041666d081 100644 --- a/app/services/update_collection_service.rb +++ b/app/services/update_collection_service.rb @@ -16,7 +16,7 @@ class UpdateCollectionService def distribute_update_activity return unless relevant_attributes_changed? - ActivityPub::AccountRawDistributionWorker.perform_async(activity_json, @collection.account.id) + ActivityPub::CollectionRawDistributionWorker.perform_async(activity_json, @collection.id) end def notify_about_update diff --git a/app/workers/activitypub/collection_raw_distribution_worker.rb b/app/workers/activitypub/collection_raw_distribution_worker.rb new file mode 100644 index 0000000000..6ddb3807dd --- /dev/null +++ b/app/workers/activitypub/collection_raw_distribution_worker.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class ActivityPub::CollectionRawDistributionWorker < ActivityPub::RawDistributionWorker + def perform(json, collection_id, exclude_inboxes = []) + @collection = Collection.find(collection_id) + + super(json, @collection.account_id, exclude_inboxes) + end + + private + + def inboxes + @inboxes ||= CollectionReachFinder.new(@collection).inboxes + end +end diff --git a/spec/lib/activitypub/activity/accept_spec.rb b/spec/lib/activitypub/activity/accept_spec.rb index 7775143e88..6e176597e9 100644 --- a/spec/lib/activitypub/activity/accept_spec.rb +++ b/spec/lib/activitypub/activity/accept_spec.rb @@ -194,7 +194,7 @@ RSpec.describe ActivityPub::Activity::Accept do expect(collection_item.reload).to be_accepted expect(collection_item.approval_uri).to eq 'https://example.com/stamps/1' - expect(ActivityPub::AccountRawDistributionWorker) + expect(ActivityPub::CollectionRawDistributionWorker) .to have_enqueued_sidekiq_job end end @@ -206,7 +206,7 @@ RSpec.describe ActivityPub::Activity::Accept do expect(collection_item.reload).to_not be_accepted expect(collection_item.approval_uri).to be_nil - expect(ActivityPub::AccountRawDistributionWorker) + expect(ActivityPub::CollectionRawDistributionWorker) .to_not have_enqueued_sidekiq_job end end diff --git a/spec/lib/activitypub/activity/delete_spec.rb b/spec/lib/activitypub/activity/delete_spec.rb index 7e5d5f8574..260aadb54e 100644 --- a/spec/lib/activitypub/activity/delete_spec.rb +++ b/spec/lib/activitypub/activity/delete_spec.rb @@ -139,7 +139,7 @@ RSpec.describe ActivityPub::Activity::Delete do subject.perform expect(collection_item.reload).to be_revoked - expect(ActivityPub::AccountRawDistributionWorker).to have_enqueued_sidekiq_job + expect(ActivityPub::CollectionRawDistributionWorker).to have_enqueued_sidekiq_job end end end diff --git a/spec/lib/collection_reach_finder_spec.rb b/spec/lib/collection_reach_finder_spec.rb new file mode 100644 index 0000000000..74c6b67538 --- /dev/null +++ b/spec/lib/collection_reach_finder_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe CollectionReachFinder do + let(:account) { Fabricate(:account) } + let(:collection) { Fabricate(:collection, account:) } + + let(:follower_example_com) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.com/inbox-1', domain: 'example.com') } + let(:follower_with_shared) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://foo.bar/users/a/inbox', domain: 'foo.bar', shared_inbox_url: 'https://foo.bar/inbox') } + + let(:collection_member_with_shared) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://foo.bar/users/b/inbox', domain: 'foo.bar', shared_inbox_url: 'https://foo.bar/inbox') } + let(:collection_member_example_org) { Fabricate(:account, protocol: :activitypub, inbox_url: 'https://example.org/inbox-2', domain: 'example.org') } + + before do + follower_example_com.follow!(account) + follower_with_shared.follow!(account) + + [follower_example_com, collection_member_with_shared, collection_member_example_org].each do |collection_member| + Fabricate(:collection_item, collection:, account: collection_member, activity_uri: "https://#{collection_member.domain}/activity", approval_uri: "https://#{collection_member.domain}/approval") + end + end + + describe '#inboxes' do + subject { described_class.new(collection).inboxes } + + it 'includes unique inbox URIs of followers and collection members respecting shared inbox URIs where present' do + expect(subject).to contain_exactly( + 'https://example.com/inbox-1', + 'https://foo.bar/inbox', + 'https://example.org/inbox-2' + ) + end + end +end diff --git a/spec/services/add_account_to_collection_service_spec.rb b/spec/services/add_account_to_collection_service_spec.rb index dd7983ca62..2e033ea239 100644 --- a/spec/services/add_account_to_collection_service_spec.rb +++ b/spec/services/add_account_to_collection_service_spec.rb @@ -25,9 +25,9 @@ RSpec.describe AddAccountToCollectionService do it 'federates an `Add` activity and schedules a notification' do subject.call(collection, account) - expect(ActivityPub::AccountRawDistributionWorker) + expect(ActivityPub::CollectionRawDistributionWorker) .to have_enqueued_sidekiq_job - .with(anything, collection.account_id) + .with(anything, collection.id) expect(LocalNotificationWorker) .to have_enqueued_sidekiq_job .with(account.id, anything, 'CollectionItem', 'added_to_collection') diff --git a/spec/services/create_collection_service_spec.rb b/spec/services/create_collection_service_spec.rb index 67acb3b7ae..4e93134034 100644 --- a/spec/services/create_collection_service_spec.rb +++ b/spec/services/create_collection_service_spec.rb @@ -32,7 +32,7 @@ RSpec.describe CreateCollectionService do it 'federates an `Add` activity' do subject.call(base_params, author) - expect(ActivityPub::AccountRawDistributionWorker).to have_enqueued_sidekiq_job + expect(ActivityPub::CollectionRawDistributionWorker).to have_enqueued_sidekiq_job end context 'when given account ids' do diff --git a/spec/services/delete_collection_item_service_spec.rb b/spec/services/delete_collection_item_service_spec.rb index f9a235b902..2f42584158 100644 --- a/spec/services/delete_collection_item_service_spec.rb +++ b/spec/services/delete_collection_item_service_spec.rb @@ -17,7 +17,7 @@ RSpec.describe DeleteCollectionItemService do it 'federates a `Remove` activity' do subject.call(collection_item) - expect(ActivityPub::AccountRawDistributionWorker).to have_enqueued_sidekiq_job + expect(ActivityPub::CollectionRawDistributionWorker).to have_enqueued_sidekiq_job end context 'when `revoke` is set to true' do @@ -36,7 +36,7 @@ RSpec.describe DeleteCollectionItemService do it 'destroys the collection withouth federating anything' do expect { subject.call(collection_item, revoke: true) }.to change(collection.collection_items, :count).by(-1) - expect(ActivityPub::AccountRawDistributionWorker).to_not have_enqueued_sidekiq_job + expect(ActivityPub::CollectionRawDistributionWorker).to_not have_enqueued_sidekiq_job end end end diff --git a/spec/services/delete_collection_service_spec.rb b/spec/services/delete_collection_service_spec.rb index 06ed82b21d..e2b85fe924 100644 --- a/spec/services/delete_collection_service_spec.rb +++ b/spec/services/delete_collection_service_spec.rb @@ -7,15 +7,20 @@ RSpec.describe DeleteCollectionService do let!(:collection) { Fabricate(:collection) } + before do + Fabricate.times(2, :collection_item, collection:) + end + describe '#call' do it 'destroys the collection' do expect { subject.call(collection) }.to change(Collection, :count).by(-1) end - it 'federates a `Remove` activity' do + it "federates a `Remove` activity to the account's reach plus each collection member" do subject.call(collection) expect(ActivityPub::AccountRawDistributionWorker).to have_enqueued_sidekiq_job + expect(ActivityPub::DeliveryWorker).to have_enqueued_sidekiq_job.exactly(2).times end end end diff --git a/spec/services/revoke_collection_item_service_spec.rb b/spec/services/revoke_collection_item_service_spec.rb index 18357f53dd..760c9038b9 100644 --- a/spec/services/revoke_collection_item_service_spec.rb +++ b/spec/services/revoke_collection_item_service_spec.rb @@ -21,7 +21,7 @@ RSpec.describe RevokeCollectionItemService do subject.call(collection_item) expect(ActivityPub::DeliveryWorker).to have_enqueued_sidekiq_job.with(instance_of(String), collection_item.account_id, 'https://example.com/actor/1/inbox') - expect(ActivityPub::AccountRawDistributionWorker).to have_enqueued_sidekiq_job + expect(ActivityPub::CollectionRawDistributionWorker).to have_enqueued_sidekiq_job end end end diff --git a/spec/services/update_collection_service_spec.rb b/spec/services/update_collection_service_spec.rb index 088d5768ed..2899b301b2 100644 --- a/spec/services/update_collection_service_spec.rb +++ b/spec/services/update_collection_service_spec.rb @@ -14,14 +14,14 @@ RSpec.describe UpdateCollectionService do expect { subject.call(collection, { name: 'Newly updated name' }) } .to change(collection, :name).to('Newly updated name') .and enqueue_sidekiq_job(LocalNotificationWorker).with(collection_item.account_id, collection.id, collection.class.name, 'collection_update') - .and enqueue_sidekiq_job(ActivityPub::AccountRawDistributionWorker) + .and enqueue_sidekiq_job(ActivityPub::CollectionRawDistributionWorker) end context 'when nothing changed' do it 'does not federate an activity' do subject.call(collection, { name: collection.name }) - expect(ActivityPub::AccountRawDistributionWorker).to_not have_enqueued_sidekiq_job + expect(ActivityPub::CollectionRawDistributionWorker).to_not have_enqueued_sidekiq_job expect(LocalNotificationWorker).to_not have_enqueued_sidekiq_job end end