Fix various issues with store hydration (#19746)

- Improve tests
- Fix possible crash when application of a reblogged post isn't set
- Fix discrepancies around favourited and reblogged attributes
- Fix discrepancies around pinned attribute
- Fix polls not being hydrated
This commit is contained in:
Claire 2022-11-04 19:33:16 +01:00 committed by GitHub
parent 0165449e3a
commit 03b991de6c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 105 additions and 36 deletions

View file

@ -16,23 +16,35 @@ 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]
payload[:favourited] = false
payload[:reblogged] = false
payload[:muted] = false payload[:muted] = false
payload[:bookmarked] = false payload[:bookmarked] = false
payload[:pinned] = false payload[:pinned] = false if @status.account_id == account_id
payload[:filtered] = CustomFilter.apply_cached_filters(CustomFilter.cached_filters_for(@status.reblog_of_id), @status.reblog).map { |filter| ActiveModelSerializers::SerializableResource.new(filter, serializer: REST::FilterResultSerializer).as_json } payload[:filtered] = CustomFilter.apply_cached_filters(CustomFilter.cached_filters_for(@status.reblog_of_id), @status.reblog).map { |filter| ActiveModelSerializers::SerializableResource.new(filter, serializer: REST::FilterResultSerializer).as_json }
# If the reblogged status is being delivered to the author who disabled the display of the application # If the reblogged status is being delivered to the author who disabled the display of the application
# 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] = ActiveModelSerializers::SerializableResource.new(@status.reblog.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:reblog][:application].nil? && @status.reblog.account_id == account_id payload[:reblog][:application] = ActiveModelSerializers::SerializableResource.new(@status.reblog.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:reblog][:application].nil? && @status.reblog.account_id == account_id && @status.reblog.application_id.present?
payload[:reblog][:favourited] = Favourite.where(account_id: account_id, status_id: @status.reblog_of_id).exists? payload[:reblog][:favourited] = Favourite.where(account_id: account_id, status_id: @status.reblog_of_id).exists?
payload[:reblog][:reblogged] = Status.where(account_id: account_id, reblog_of_id: @status.reblog_of_id).exists? payload[:reblog][:reblogged] = Status.where(account_id: account_id, reblog_of_id: @status.reblog_of_id).exists?
payload[:reblog][:muted] = ConversationMute.where(account_id: account_id, conversation_id: @status.reblog.conversation_id).exists? payload[:reblog][:muted] = ConversationMute.where(account_id: account_id, conversation_id: @status.reblog.conversation_id).exists?
payload[:reblog][:bookmarked] = Bookmark.where(account_id: account_id, status_id: @status.reblog_of_id).exists? payload[:reblog][:bookmarked] = Bookmark.where(account_id: account_id, status_id: @status.reblog_of_id).exists?
payload[:reblog][:pinned] = StatusPin.where(account_id: account_id, status_id: @status.reblog_of_id).exists? payload[:reblog][:pinned] = StatusPin.where(account_id: account_id, status_id: @status.reblog_of_id).exists? if @status.reblog.account_id == account_id
payload[:reblog][:filtered] = payload[:filtered] payload[:reblog][:filtered] = payload[:filtered]
if payload[:reblog][:poll]
if @status.reblog.account_id == account_id
payload[:reblog][:poll][:voted] = true
payload[:reblog][:poll][:own_votes] = []
else
own_votes = @status.reblog.poll.votes.where(account_id: account_id).pluck(:choice)
payload[:reblog][:poll][:voted] = !own_votes.empty?
payload[:reblog][:poll][:own_votes] = own_votes
end
end
payload[:favourited] = payload[:reblog][:favourited]
payload[:reblogged] = payload[:reblog][:reblogged]
else else
payload[:favourited] = Favourite.where(account_id: account_id, status_id: @status.id).exists? payload[:favourited] = Favourite.where(account_id: account_id, status_id: @status.id).exists?
payload[:reblogged] = Status.where(account_id: account_id, reblog_of_id: @status.id).exists? payload[:reblogged] = Status.where(account_id: account_id, reblog_of_id: @status.id).exists?

View file

@ -7,48 +7,105 @@ describe StatusCacheHydrator do
let(:account) { Fabricate(:account) } let(:account) { Fabricate(:account) }
describe '#hydrate' do describe '#hydrate' do
subject { described_class.new(status).hydrate(account.id) }
let(:compare_to_hash) { InlineRenderer.render(status, account, :status) } let(:compare_to_hash) { InlineRenderer.render(status, account, :status) }
context 'when cache is warm' do shared_examples 'shared behavior' do
before do context 'when handling a new status' do
Rails.cache.write("fan-out/#{status.id}", InlineRenderer.render(status, nil, :status)) it 'renders the same attributes as a full render' do
expect(subject).to include(compare_to_hash)
end
end end
it 'renders the same attributes as a full render' do context 'when handling a reblog' do
expect(subject).to include(compare_to_hash) let(:reblog) { Fabricate(:status) }
let(:status) { Fabricate(:status, reblog: reblog) }
context 'that has been favourited' do
before do
FavouriteService.new.call(account, reblog)
end
it 'renders the same attributes as a full render' do
expect(subject).to include(compare_to_hash)
end
end
context 'that has been reblogged' do
before do
ReblogService.new.call(account, reblog)
end
it 'renders the same attributes as a full render' do
expect(subject).to include(compare_to_hash)
end
end
context 'that has been pinned' do
let(:reblog) { Fabricate(:status, account: account) }
before do
StatusPin.create!(account: account, status: reblog)
end
it 'renders the same attributes as a full render' do
expect(subject).to include(compare_to_hash)
end
end
context 'that has been followed tags' do
let(:followed_tag) { Fabricate(:tag) }
before do
reblog.tags << Fabricate(:tag)
reblog.tags << followed_tag
TagFollow.create!(tag: followed_tag, account: account, rate_limit: false)
end
it 'renders the same attributes as a full render' do
expect(subject).to include(compare_to_hash)
end
end
context 'that has a poll authored by the user' do
let(:poll) { Fabricate(:poll, account: account) }
let(:reblog) { Fabricate(:status, poll: poll, account: account) }
it 'renders the same attributes as a full render' do
expect(subject).to include(compare_to_hash)
end
end
context 'that has been voted in' do
let(:poll) { Fabricate(:poll, options: %w(Yellow Blue)) }
let(:reblog) { Fabricate(:status, poll: poll) }
before do
VoteService.new.call(account, poll, [0])
end
it 'renders the same attributes as a full render' do
expect(subject).to include(compare_to_hash)
end
end
end end
end end
context 'when cache is warm' do
subject do
Rails.cache.write("fan-out/#{status.id}", InlineRenderer.render(status, nil, :status))
described_class.new(status).hydrate(account.id)
end
it_behaves_like 'shared behavior'
end
context 'when cache is cold' do context 'when cache is cold' do
before do subject do
Rails.cache.delete("fan-out/#{status.id}") Rails.cache.delete("fan-out/#{status.id}")
described_class.new(status).hydrate(account.id)
end end
it 'renders the same attributes as a full render' do it_behaves_like 'shared behavior'
expect(subject).to include(compare_to_hash)
end
end
context 'when account has favourited status' do
before do
FavouriteService.new.call(account, status)
end
it 'renders the same attributes as a full render' do
expect(subject).to include(compare_to_hash)
end
end
context 'when account has reblogged status' do
before do
ReblogService.new.call(account, status)
end
it 'renders the same attributes as a full render' do
expect(subject).to include(compare_to_hash)
end
end end
end end
end end