From 1f1653e0392622e8cf827ddb1e28c5f8de50b3d6 Mon Sep 17 00:00:00 2001 From: David Roetzel Date: Tue, 28 Apr 2026 13:10:25 +0200 Subject: [PATCH] Remove rejected and revoked collection items (#38792) --- app/lib/activitypub/activity/reject.rb | 2 +- app/models/collection_item.rb | 7 +++--- .../collection_item_cleanup_scheduler.rb | 16 +++++++++++++ config/sidekiq.yml | 4 ++++ ...1611_add_index_to_collection_item_state.rb | 9 ++++++++ db/schema.rb | 3 ++- spec/lib/activitypub/activity/reject_spec.rb | 7 ++++-- .../collection_item_cleanup_scheduler_spec.rb | 23 +++++++++++++++++++ 8 files changed, 63 insertions(+), 8 deletions(-) create mode 100644 app/workers/scheduler/collection_item_cleanup_scheduler.rb create mode 100644 db/migrate/20260423141611_add_index_to_collection_item_state.rb create mode 100644 spec/workers/scheduler/collection_item_cleanup_scheduler_spec.rb diff --git a/app/lib/activitypub/activity/reject.rb b/app/lib/activitypub/activity/reject.rb index b6195cd199..e5684367a2 100644 --- a/app/lib/activitypub/activity/reject.rb +++ b/app/lib/activitypub/activity/reject.rb @@ -51,7 +51,7 @@ class ActivityPub::Activity::Reject < ActivityPub::Activity collection_item = feature_request_from_object return unless collection_item.account == @account && collection_item.local? - collection_item.destroy! + collection_item.reject! end def relay diff --git a/app/models/collection_item.rb b/app/models/collection_item.rb index cc77583674..bbb0343ebe 100644 --- a/app/models/collection_item.rb +++ b/app/models/collection_item.rb @@ -25,6 +25,9 @@ class CollectionItem < ApplicationRecord { pending: 0, accepted: 1, rejected: 2, revoked: 3 }, validate: true + alias reject! rejected! + alias revoke! revoked! + delegate :local?, :remote?, to: :collection validates :account_id, uniqueness: { scope: :collection_id } @@ -45,10 +48,6 @@ class CollectionItem < ApplicationRecord scope :accepted_partial, ->(account) { joins(:account).merge(Account.local).accepted.where(uri: nil, account_id: account.id) } scope :pending_or_accepted, -> { where(state: [:pending, :accepted]) } - def revoke! - update!(state: :revoked) - end - def with_local_account? account&.local? end diff --git a/app/workers/scheduler/collection_item_cleanup_scheduler.rb b/app/workers/scheduler/collection_item_cleanup_scheduler.rb new file mode 100644 index 0000000000..f70b3ee8f0 --- /dev/null +++ b/app/workers/scheduler/collection_item_cleanup_scheduler.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class Scheduler::CollectionItemCleanupScheduler + include Sidekiq::Worker + + RETENTION_PERIOD = 24.hours + + sidekiq_options retry: 0, lock: :until_executed, lock_ttl: 1.day.to_i + + def perform + CollectionItem + .where(state: [:rejected, :revoked]) + .where(updated_at: ...(RETENTION_PERIOD.ago)) + .destroy_all + end +end diff --git a/config/sidekiq.yml b/config/sidekiq.yml index fb85343d94..5beb95a3f8 100644 --- a/config/sidekiq.yml +++ b/config/sidekiq.yml @@ -72,3 +72,7 @@ interval: 1 day class: Scheduler::Fasp::FollowRecommendationCleanupScheduler queue: scheduler + collection_item_cleanup_scheduler: + interval: 1 hour + class: Scheduler::CollectionItemCleanupScheduler + queue: scheduler diff --git a/db/migrate/20260423141611_add_index_to_collection_item_state.rb b/db/migrate/20260423141611_add_index_to_collection_item_state.rb new file mode 100644 index 0000000000..498dbc7eb8 --- /dev/null +++ b/db/migrate/20260423141611_add_index_to_collection_item_state.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class AddIndexToCollectionItemState < ActiveRecord::Migration[8.1] + disable_ddl_transaction! + + def change + add_index :collection_items, :state, where: 'state IN (2, 3)', algorithm: :concurrently + end +end diff --git a/db/schema.rb b/db/schema.rb index 8523182b5e..62eff610c6 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_04_20_124030) do +ActiveRecord::Schema[8.1].define(version: 2026_04_23_141611) do # These are extensions that must be enabled in order to support this database enable_extension "pg_catalog.plpgsql" @@ -373,6 +373,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_04_20_124030) do t.index ["account_id", "collection_id"], name: "index_collection_items_on_account_id_and_collection_id", unique: true t.index ["approval_uri"], name: "index_collection_items_on_approval_uri", unique: true, where: "(approval_uri IS NOT NULL)" t.index ["collection_id"], name: "index_collection_items_on_collection_id" + t.index ["state"], name: "index_collection_items_on_state", where: "(state = ANY (ARRAY[2, 3]))" t.index ["uri"], name: "index_collection_items_on_uri", unique: true, where: "(uri IS NOT NULL)" end diff --git a/spec/lib/activitypub/activity/reject_spec.rb b/spec/lib/activitypub/activity/reject_spec.rb index 8f58f02c71..d4ef898c25 100644 --- a/spec/lib/activitypub/activity/reject_spec.rb +++ b/spec/lib/activitypub/activity/reject_spec.rb @@ -161,8 +161,11 @@ RSpec.describe ActivityPub::Activity::Reject do } end - it 'deletes the collection item' do - expect { subject.perform }.to change(collection.collection_items, :count).by(-1) + it 'sets the collection item state to `rejected`' do + expect do + subject.perform + collection_item.reload + end.to change(collection_item, :state).to('rejected') end end end diff --git a/spec/workers/scheduler/collection_item_cleanup_scheduler_spec.rb b/spec/workers/scheduler/collection_item_cleanup_scheduler_spec.rb new file mode 100644 index 0000000000..0a13053f81 --- /dev/null +++ b/spec/workers/scheduler/collection_item_cleanup_scheduler_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Scheduler::CollectionItemCleanupScheduler do + let(:worker) { described_class.new } + + describe '#perform' do + let!(:old_rejected_item) { Fabricate(:collection_item, state: :rejected, updated_at: 25.hours.ago) } + let!(:old_revoked_item) { Fabricate(:collection_item, state: :revoked, updated_at: 26.hours.ago) } + let!(:new_revoked_item) { Fabricate(:collection_item, state: :revoked, updated_at: 2.hours.ago) } + let!(:accepted_item) { Fabricate(:collection_item, state: :accepted, updated_at: 30.hours.ago) } + + it 'deletes the rejected and revoked items older than 24 hours' do + expect { subject.perform }.to change(CollectionItem, :count).by(-2) + + expect { old_rejected_item.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { old_revoked_item.reload }.to raise_error(ActiveRecord::RecordNotFound) + expect { new_revoked_item.reload }.to_not raise_error + expect { accepted_item.reload }.to_not raise_error + end + end +end