From 5f9e47be34fcf42ff7fcd5668c7555d4a38e289a Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 4 Nov 2022 13:21:06 +0100 Subject: Add caching for payload serialization during fan-out (#19642) --- app/lib/status_cache_hydrator.rb | 47 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 app/lib/status_cache_hydrator.rb (limited to 'app/lib/status_cache_hydrator.rb') diff --git a/app/lib/status_cache_hydrator.rb b/app/lib/status_cache_hydrator.rb new file mode 100644 index 000000000..01e92b385 --- /dev/null +++ b/app/lib/status_cache_hydrator.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +class StatusCacheHydrator + def initialize(status) + @status = status + end + + def hydrate(account_id) + # 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) } + + # If we're delivering to the author who disabled the display of the application used to create the + # status, we need to hydrate the application, since it was not rendered for the basic payload + payload[:application] = ActiveModelSerializers::SerializableResource.new(@status.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:application].nil? && @status.account_id == account_id + + # 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 + if payload[:reblog] + payload[:favourited] = false + payload[:reblogged] = false + payload[:muted] = false + payload[:bookmarked] = false + payload[:pinned] = false + 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 + # 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][: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][: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][:pinned] = StatusPin.where(account_id: account_id, status_id: @status.reblog_of_id).exists? + payload[:reblog][:filtered] = payload[:filtered] + else + 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[:muted] = ConversationMute.where(account_id: account_id, conversation_id: @status.conversation_id).exists? + payload[:bookmarked] = Bookmark.where(account_id: account_id, status_id: @status.id).exists? + payload[:pinned] = StatusPin.where(account_id: account_id, status_id: @status.id).exists? + payload[:filtered] = CustomFilter.apply_cached_filters(CustomFilter.cached_filters_for(@status.id), @status).map { |filter| ActiveModelSerializers::SerializableResource.new(filter, serializer: REST::FilterResultSerializer).as_json } + end + + payload + end +end -- cgit From 03b991de6c5874f3e7d1b6f6ff70b853b8daf639 Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 4 Nov 2022 19:33:16 +0100 Subject: 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 --- app/lib/status_cache_hydrator.rb | 22 +++++-- spec/lib/status_cache_hydrator_spec.rb | 111 +++++++++++++++++++++++++-------- 2 files changed, 101 insertions(+), 32 deletions(-) (limited to 'app/lib/status_cache_hydrator.rb') diff --git a/app/lib/status_cache_hydrator.rb b/app/lib/status_cache_hydrator.rb index 01e92b385..552419fff 100644 --- a/app/lib/status_cache_hydrator.rb +++ b/app/lib/status_cache_hydrator.rb @@ -16,23 +16,35 @@ class StatusCacheHydrator # 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 if payload[:reblog] - payload[:favourited] = false - payload[:reblogged] = false payload[:muted] = 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 } # 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 - 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][: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][: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] + + 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 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? diff --git a/spec/lib/status_cache_hydrator_spec.rb b/spec/lib/status_cache_hydrator_spec.rb index ad9940a85..873d58464 100644 --- a/spec/lib/status_cache_hydrator_spec.rb +++ b/spec/lib/status_cache_hydrator_spec.rb @@ -7,48 +7,105 @@ describe StatusCacheHydrator do let(:account) { Fabricate(:account) } describe '#hydrate' do - subject { described_class.new(status).hydrate(account.id) } - let(:compare_to_hash) { InlineRenderer.render(status, account, :status) } - context 'when cache is warm' do - before do - Rails.cache.write("fan-out/#{status.id}", InlineRenderer.render(status, nil, :status)) + shared_examples 'shared behavior' do + context 'when handling a new status' do + it 'renders the same attributes as a full render' do + expect(subject).to include(compare_to_hash) + end end - it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) - end - end + context 'when handling a reblog' do + let(:reblog) { Fabricate(:status) } + let(:status) { Fabricate(:status, reblog: reblog) } - context 'when cache is cold' do - before do - Rails.cache.delete("fan-out/#{status.id}") - end + 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 - it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + 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 - context 'when account has favourited status' do - before do - FavouriteService.new.call(account, status) + 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 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) - end + it_behaves_like 'shared behavior' end - context 'when account has reblogged status' do - before do - ReblogService.new.call(account, status) + context 'when cache is cold' do + subject do + Rails.cache.delete("fan-out/#{status.id}") + described_class.new(status).hydrate(account.id) end - it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) - end + it_behaves_like 'shared behavior' end end end -- cgit From bb89f83cc06b9665205c62db21163f2ce43d97ed Mon Sep 17 00:00:00 2001 From: Claire Date: Fri, 4 Nov 2022 20:01:33 +0100 Subject: Fix additional issues with status cache hydration (#19747) * Spare one SQL query when hydrating polls * Improve tests * Fix more discrepancies * Fix possible crash when the status has no application set --- app/lib/status_cache_hydrator.rb | 13 +++++++++---- spec/lib/status_cache_hydrator_spec.rb | 26 +++++++++++++++++++------- 2 files changed, 28 insertions(+), 11 deletions(-) (limited to 'app/lib/status_cache_hydrator.rb') diff --git a/app/lib/status_cache_hydrator.rb b/app/lib/status_cache_hydrator.rb index 552419fff..ffe813ee9 100644 --- a/app/lib/status_cache_hydrator.rb +++ b/app/lib/status_cache_hydrator.rb @@ -11,7 +11,7 @@ class StatusCacheHydrator # If we're delivering to the author who disabled the display of the application used to create the # status, we need to hydrate the application, since it was not rendered for the basic payload - payload[:application] = ActiveModelSerializers::SerializableResource.new(@status.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:application].nil? && @status.account_id == account_id + payload[:application] = ActiveModelSerializers::SerializableResource.new(@status.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:application].nil? && @status.account_id == account_id && @status.application.present? # 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 @@ -23,7 +23,7 @@ class StatusCacheHydrator # 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 - 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][: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.present? 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? @@ -37,7 +37,7 @@ class StatusCacheHydrator payload[:reblog][:poll][:voted] = true payload[:reblog][:poll][:own_votes] = [] else - own_votes = @status.reblog.poll.votes.where(account_id: account_id).pluck(:choice) + own_votes = PollVote.where(poll_id: @status.reblog.poll_id, account_id: account_id).pluck(:choice) payload[:reblog][:poll][:voted] = !own_votes.empty? payload[:reblog][:poll][:own_votes] = own_votes end @@ -50,8 +50,13 @@ class StatusCacheHydrator payload[:reblogged] = Status.where(account_id: account_id, reblog_of_id: @status.id).exists? payload[:muted] = ConversationMute.where(account_id: account_id, conversation_id: @status.conversation_id).exists? payload[:bookmarked] = Bookmark.where(account_id: account_id, status_id: @status.id).exists? - payload[:pinned] = StatusPin.where(account_id: account_id, status_id: @status.id).exists? + payload[:pinned] = StatusPin.where(account_id: account_id, status_id: @status.id).exists? if @status.account_id == account_id payload[:filtered] = CustomFilter.apply_cached_filters(CustomFilter.cached_filters_for(@status.id), @status).map { |filter| ActiveModelSerializers::SerializableResource.new(filter, serializer: REST::FilterResultSerializer).as_json } + + if payload[:poll] + payload[:poll][:voted] = @status.account_id == account_id + payload[:poll][:own_votes] = [] + end end payload diff --git a/spec/lib/status_cache_hydrator_spec.rb b/spec/lib/status_cache_hydrator_spec.rb index 873d58464..c9d8d0fe1 100644 --- a/spec/lib/status_cache_hydrator_spec.rb +++ b/spec/lib/status_cache_hydrator_spec.rb @@ -11,8 +11,20 @@ describe StatusCacheHydrator do shared_examples 'shared behavior' do context 'when handling a new status' do + let(:poll) { Fabricate(:poll) } + let(:status) { Fabricate(:status, poll: poll) } + + it 'renders the same attributes as a full render' do + expect(subject).to eql(compare_to_hash) + end + end + + context 'when handling a new status with own poll' do + let(:poll) { Fabricate(:poll, account: account) } + let(:status) { Fabricate(:status, poll: poll, account: account) } + it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -26,7 +38,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -36,7 +48,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -48,7 +60,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -62,7 +74,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end @@ -71,7 +83,7 @@ describe StatusCacheHydrator do 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) + expect(subject).to eql(compare_to_hash) end end @@ -84,7 +96,7 @@ describe StatusCacheHydrator do end it 'renders the same attributes as a full render' do - expect(subject).to include(compare_to_hash) + expect(subject).to eql(compare_to_hash) end end end -- cgit From 21d6bc16893a9dd24c5ef54cf2f554846c8eca5b Mon Sep 17 00:00:00 2001 From: Claire Date: Sun, 6 Nov 2022 16:56:27 +0100 Subject: Fix status cache hydration discrepancy --- app/lib/status_cache_hydrator.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'app/lib/status_cache_hydrator.rb') diff --git a/app/lib/status_cache_hydrator.rb b/app/lib/status_cache_hydrator.rb index ffe813ee9..cde75ae8b 100644 --- a/app/lib/status_cache_hydrator.rb +++ b/app/lib/status_cache_hydrator.rb @@ -11,7 +11,7 @@ class StatusCacheHydrator # If we're delivering to the author who disabled the display of the application used to create the # status, we need to hydrate the application, since it was not rendered for the basic payload - payload[:application] = ActiveModelSerializers::SerializableResource.new(@status.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json if payload[:application].nil? && @status.account_id == account_id && @status.application.present? + payload[:application] = @status.application.present? ? ActiveModelSerializers::SerializableResource.new(@status.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json : nil if payload[:application].nil? && @status.account_id == account_id # 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 @@ -23,7 +23,7 @@ class StatusCacheHydrator # 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 - 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.present? + payload[:reblog][:application] = @status.reblog.application.present? ? ActiveModelSerializers::SerializableResource.new(@status.reblog.application, serializer: REST::StatusSerializer::ApplicationSerializer).as_json : nil if payload[:reblog][:application].nil? && @status.reblog.account_id == account_id 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? -- cgit