Fix incorrect URL being used when cache busting (#34189)

This commit is contained in:
Claire 2025-03-17 17:40:28 +01:00 committed by GitHub
parent 2a5853989f
commit e30001bc80
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 20 additions and 24 deletions

View File

@ -421,7 +421,7 @@ class MediaAttachment < ApplicationRecord
@paths_to_cache_bust = MediaAttachment.attachment_definitions.keys.flat_map do |attachment_name| @paths_to_cache_bust = MediaAttachment.attachment_definitions.keys.flat_map do |attachment_name|
attachment = public_send(attachment_name) attachment = public_send(attachment_name)
styles = DEFAULT_STYLES | attachment.styles.keys styles = DEFAULT_STYLES | attachment.styles.keys
styles.map { |style| attachment.path(style) } styles.map { |style| attachment.url(style) }
end.compact end.compact
rescue => e rescue => e
# We really don't want any error here preventing media deletion # We really don't want any error here preventing media deletion

View File

@ -95,7 +95,7 @@ class SuspendAccountService < BaseService
end end
end end
CacheBusterWorker.perform_async(attachment.path(style)) if Rails.configuration.x.cache_buster_enabled CacheBusterWorker.perform_async(attachment.url(style)) if Rails.configuration.x.cache_buster_enabled
end end
end end
end end

View File

@ -91,7 +91,7 @@ class UnsuspendAccountService < BaseService
end end
end end
CacheBusterWorker.perform_async(attachment.path(style)) if Rails.configuration.x.cache_buster_enabled CacheBusterWorker.perform_async(attachment.url(style)) if Rails.configuration.x.cache_buster_enabled
end end
end end
end end

View File

@ -295,12 +295,12 @@ RSpec.describe MediaAttachment, :attachment_processing do
end end
it 'queues CacheBusterWorker jobs' do it 'queues CacheBusterWorker jobs' do
original_path = media.file.path(:original) original_url = media.file.url(:original)
small_path = media.file.path(:small) small_url = media.file.url(:small)
expect { media.destroy } expect { media.destroy }
.to enqueue_sidekiq_job(CacheBusterWorker).with(original_path) .to enqueue_sidekiq_job(CacheBusterWorker).with(original_url)
.and enqueue_sidekiq_job(CacheBusterWorker).with(small_path) .and enqueue_sidekiq_job(CacheBusterWorker).with(small_url)
end end
end end

View File

@ -2,7 +2,7 @@
require 'rails_helper' require 'rails_helper'
RSpec.describe SuspendAccountService, :inline_jobs do RSpec.describe SuspendAccountService do
shared_examples 'common behavior' do shared_examples 'common behavior' do
subject { described_class.new.call(account) } subject { described_class.new.call(account) }
@ -11,6 +11,7 @@ RSpec.describe SuspendAccountService, :inline_jobs do
before do before do
allow(FeedManager.instance).to receive_messages(unmerge_from_home: nil, unmerge_from_list: nil) allow(FeedManager.instance).to receive_messages(unmerge_from_home: nil, unmerge_from_list: nil)
allow(Rails.configuration.x).to receive(:cache_buster_enabled).and_return(true)
local_follower.follow!(account) local_follower.follow!(account)
list.accounts << account list.accounts << account
@ -23,6 +24,7 @@ RSpec.describe SuspendAccountService, :inline_jobs do
it 'unmerges from feeds of local followers and changes file mode and preserves suspended flag' do it 'unmerges from feeds of local followers and changes file mode and preserves suspended flag' do
expect { subject } expect { subject }
.to change_file_mode .to change_file_mode
.and enqueue_sidekiq_job(CacheBusterWorker).with(account.media_attachments.first.file.url(:original))
.and not_change_suspended_flag .and not_change_suspended_flag
expect(FeedManager.instance).to have_received(:unmerge_from_home).with(account, local_follower) expect(FeedManager.instance).to have_received(:unmerge_from_home).with(account, local_follower)
expect(FeedManager.instance).to have_received(:unmerge_from_list).with(account, list) expect(FeedManager.instance).to have_received(:unmerge_from_list).with(account, list)
@ -38,17 +40,12 @@ RSpec.describe SuspendAccountService, :inline_jobs do
end end
describe 'suspending a local account' do describe 'suspending a local account' do
def match_update_actor_request(req, account) def match_update_actor_request(json, account)
json = JSON.parse(req.body) json = JSON.parse(json)
actor_id = ActivityPub::TagManager.instance.uri_for(account) actor_id = ActivityPub::TagManager.instance.uri_for(account)
json['type'] == 'Update' && json['actor'] == actor_id && json['object']['id'] == actor_id && json['object']['suspended'] json['type'] == 'Update' && json['actor'] == actor_id && json['object']['id'] == actor_id && json['object']['suspended']
end end
before do
stub_request(:post, 'https://alice.com/inbox').to_return(status: 201)
stub_request(:post, 'https://bob.com/inbox').to_return(status: 201)
end
include_examples 'common behavior' do include_examples 'common behavior' do
let!(:account) { Fabricate(:account) } let!(:account) { Fabricate(:account) }
let!(:remote_follower) { Fabricate(:account, uri: 'https://alice.com', inbox_url: 'https://alice.com/inbox', protocol: :activitypub, domain: 'alice.com') } let!(:remote_follower) { Fabricate(:account, uri: 'https://alice.com', inbox_url: 'https://alice.com/inbox', protocol: :activitypub, domain: 'alice.com') }
@ -61,22 +58,20 @@ RSpec.describe SuspendAccountService, :inline_jobs do
it 'sends an Update actor activity to followers and reporters' do it 'sends an Update actor activity to followers and reporters' do
subject subject
expect(a_request(:post, remote_follower.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once
expect(a_request(:post, remote_reporter.inbox_url).with { |req| match_update_actor_request(req, account) }).to have_been_made.once expect(ActivityPub::DeliveryWorker)
.to have_enqueued_sidekiq_job(satisfying { |json| match_update_actor_request(json, account) }, account.id, remote_follower.inbox_url).once
.and have_enqueued_sidekiq_job(satisfying { |json| match_update_actor_request(json, account) }, account.id, remote_reporter.inbox_url).once
end end
end end
end end
describe 'suspending a remote account' do describe 'suspending a remote account' do
def match_reject_follow_request(req, account, followee) def match_reject_follow_request(json, account, followee)
json = JSON.parse(req.body) json = JSON.parse(json)
json['type'] == 'Reject' && json['actor'] == ActivityPub::TagManager.instance.uri_for(followee) && json['object']['actor'] == account.uri json['type'] == 'Reject' && json['actor'] == ActivityPub::TagManager.instance.uri_for(followee) && json['object']['actor'] == account.uri
end end
before do
stub_request(:post, 'https://bob.com/inbox').to_return(status: 201)
end
include_examples 'common behavior' do include_examples 'common behavior' do
let!(:account) { Fabricate(:account, domain: 'bob.com', uri: 'https://bob.com', inbox_url: 'https://bob.com/inbox', protocol: :activitypub) } let!(:account) { Fabricate(:account, domain: 'bob.com', uri: 'https://bob.com', inbox_url: 'https://bob.com/inbox', protocol: :activitypub) }
let!(:local_followee) { Fabricate(:account) } let!(:local_followee) { Fabricate(:account) }
@ -88,7 +83,8 @@ RSpec.describe SuspendAccountService, :inline_jobs do
it 'sends a Reject Follow activity', :aggregate_failures do it 'sends a Reject Follow activity', :aggregate_failures do
subject subject
expect(a_request(:post, account.inbox_url).with { |req| match_reject_follow_request(req, account, local_followee) }).to have_been_made.once expect(ActivityPub::DeliveryWorker)
.to have_enqueued_sidekiq_job(satisfying { |json| match_reject_follow_request(json, account, local_followee) }, local_followee.id, account.inbox_url).once
end end
end end
end end