Fix multiple issues in quoted posts hydration (#34864)

This commit is contained in:
Claire 2025-05-31 17:20:31 +02:00 committed by GitHub
parent 44d71d59ef
commit 32b521b7f4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 67 additions and 16 deletions

View file

@ -5,7 +5,7 @@ class StatusCacheHydrator
@status = status @status = status
end end
def hydrate(account_id) def hydrate(account_id, nested: false)
# The cache of the serialized hash is generated by the fan-out-on-write service # The cache of the serialized hash is generated by the fan-out-on-write service
payload = Rails.cache.fetch("fan-out/#{@status.id}") { InlineRenderer.render(@status, nil, :status) } payload = Rails.cache.fetch("fan-out/#{@status.id}") { InlineRenderer.render(@status, nil, :status) }
@ -16,17 +16,17 @@ class StatusCacheHydrator
# We take advantage of the fact that some relationships can only occur with an original status, not # We take advantage of the fact that some relationships can only occur with an original status, not
# the reblog that wraps it, so we can assume that some values are always false # the reblog that wraps it, so we can assume that some values are always false
if payload[:reblog] if payload[:reblog]
hydrate_reblog_payload(payload, account_id) hydrate_reblog_payload(payload, account_id, nested:)
else else
hydrate_non_reblog_payload(payload, account_id) hydrate_non_reblog_payload(payload, account_id, nested:)
end end
end end
private private
def hydrate_non_reblog_payload(empty_payload, account_id) def hydrate_non_reblog_payload(empty_payload, account_id, nested: false)
empty_payload.tap do |payload| empty_payload.tap do |payload|
fill_status_payload(payload, @status, account_id) fill_status_payload(payload, @status, account_id, nested:)
if payload[:poll] if payload[:poll]
payload[:poll][:voted] = @status.account_id == account_id payload[:poll][:voted] = @status.account_id == account_id
@ -35,7 +35,7 @@ class StatusCacheHydrator
end end
end end
def hydrate_reblog_payload(empty_payload, account_id) def hydrate_reblog_payload(empty_payload, account_id, nested: false)
empty_payload.tap do |payload| empty_payload.tap do |payload|
payload[:muted] = false payload[:muted] = false
payload[:bookmarked] = false payload[:bookmarked] = false
@ -45,7 +45,7 @@ class StatusCacheHydrator
# used to create the status, we need to hydrate it here too # used to create the status, we need to hydrate it here too
payload[:reblog][:application] = payload_reblog_application if payload[:reblog][:application].nil? && @status.reblog.account_id == account_id payload[:reblog][:application] = payload_reblog_application if payload[:reblog][:application].nil? && @status.reblog.account_id == account_id
fill_status_payload(payload[:reblog], @status.reblog, account_id) fill_status_payload(payload[:reblog], @status.reblog, account_id, nested:)
if payload[:reblog][:poll] if payload[:reblog][:poll]
if @status.reblog.account_id == account_id if @status.reblog.account_id == account_id
@ -64,28 +64,36 @@ class StatusCacheHydrator
end end
end end
def fill_status_payload(payload, status, account_id) def fill_status_payload(payload, status, account_id, nested: false)
payload[:favourited] = Favourite.exists?(account_id: account_id, status_id: status.id) payload[:favourited] = Favourite.exists?(account_id: account_id, status_id: status.id)
payload[:reblogged] = Status.exists?(account_id: account_id, reblog_of_id: status.id) payload[:reblogged] = Status.exists?(account_id: account_id, reblog_of_id: status.id)
payload[:muted] = ConversationMute.exists?(account_id: account_id, conversation_id: status.conversation_id) payload[:muted] = ConversationMute.exists?(account_id: account_id, conversation_id: status.conversation_id)
payload[:bookmarked] = Bookmark.exists?(account_id: account_id, status_id: status.id) payload[:bookmarked] = Bookmark.exists?(account_id: account_id, status_id: status.id)
payload[:pinned] = StatusPin.exists?(account_id: account_id, status_id: status.id) if status.account_id == account_id payload[:pinned] = StatusPin.exists?(account_id: account_id, status_id: status.id) if status.account_id == account_id
payload[:filtered] = mapped_applied_custom_filter(account_id, status) payload[:filtered] = mapped_applied_custom_filter(account_id, status)
payload[:quote] = hydrate_quote_payload(payload[:quote], status.quote, account_id) if payload[:quote] payload[:quote] = hydrate_quote_payload(payload[:quote], status.quote, account_id, nested:) if payload[:quote]
end end
def hydrate_quote_payload(empty_payload, quote, account_id) def hydrate_quote_payload(empty_payload, quote, account_id, nested: false)
# TODO: properly handle quotes, including visibility and access control
empty_payload.tap do |payload| empty_payload.tap do |payload|
# Nothing to do if we're in the shallow (depth limit) case # Nothing to do if we're in the shallow (depth limit) case
next unless payload.key?(:quoted_status) next unless payload.key?(:quoted_status)
# TODO: handle hiding a rendered status or showing a non-rendered status according to visibility payload.delete(:quoted_status) if nested
# TODO: performance improvements
if quote&.quoted_status.nil? if quote&.quoted_status.nil?
payload[:quoted_status] = nil payload[nested ? :quoted_status_id : :quoted_status] = nil
elsif payload[:quoted_status].present? payload[:state] = 'deleted'
payload[:quoted_status] = StatusCacheHydrator.new(quote.quoted_status).hydrate(account_id) elsif StatusFilter.new(quote.quoted_status, Account.find_by(id: account_id)).filtered?
payload[nested ? :quoted_status_id : :quoted_status] = nil
payload[:state] = 'unauthorized'
elsif payload[:state] == 'accepted'
if nested
payload[:quoted_status_id] = quote.quoted_status_id&.to_s
else
payload[:quoted_status] = StatusCacheHydrator.new(quote.quoted_status).hydrate(account_id, nested: true)
end
end end
end end
end end

View file

@ -50,6 +50,7 @@ RSpec.describe StatusCacheHydrator do
it 'renders the same attributes as full render' do it 'renders the same attributes as full render' do
expect(subject).to eql(compare_to_hash) expect(subject).to eql(compare_to_hash)
expect(subject[:quote]).to_not be_nil expect(subject[:quote]).to_not be_nil
expect(subject[:quote_status]).to be_nil
end end
end end
@ -65,6 +66,48 @@ RSpec.describe StatusCacheHydrator do
expect(subject[:quote]).to_not be_nil expect(subject[:quote]).to_not be_nil
end end
context 'when the quote post is recursive' do
let(:quoted_status) { status }
it 'renders the same attributes as full render' do
expect(subject).to eql(compare_to_hash)
expect(subject[:quote]).to_not be_nil
end
end
context 'when the quoted post has been deleted' do
let(:quoted_status) { nil }
it 'returns the same attributes as full render' do
expect(subject).to eql(compare_to_hash)
expect(subject[:quote]).to_not be_nil
expect(subject[:quote_status]).to be_nil
end
end
context 'when the quoted post author has blocked the viewer' do
before do
quoted_status.account.block!(account)
end
it 'returns the same attributes as full render' do
expect(subject).to eql(compare_to_hash)
expect(subject[:quote]).to_not be_nil
expect(subject[:quote_status]).to be_nil
end
end
context 'when the viewer has blocked the quoted post author' do
before do
account.block!(quoted_status.account)
end
it 'returns the same attributes as full render' do
expect(subject).to eql(compare_to_hash)
expect(subject[:quote]).to_not be_nil
end
end
context 'when the quoted post has been favourited' do context 'when the quoted post has been favourited' do
before do before do
FavouriteService.new.call(account, quoted_status) FavouriteService.new.call(account, quoted_status)