From 002d4a87dd79436bd0613d6fa0c792be3b73337f Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 19 Oct 2023 14:12:56 +0800 Subject: [PATCH] Improve topic serialization performance --- plugin.rb | 47 +++++++---- spec/requests/list_controller_spec.rb | 80 ++++++++++++++++++ spec/requests/topics_controller_spec.rb | 107 ++++++++++++++++++++++++ 3 files changed, 220 insertions(+), 14 deletions(-) create mode 100644 spec/requests/list_controller_spec.rb create mode 100644 spec/requests/topics_controller_spec.rb diff --git a/plugin.rb b/plugin.rb index fc21df34..de73e892 100644 --- a/plugin.rb +++ b/plugin.rb @@ -315,14 +315,18 @@ updated_at visibility ] - activity_pub_post_custom_fields.each do |field_name| - register_post_custom_field_type("activity_pub_#{field_name.to_s}", :string) + activity_pub_post_custom_field_names = + activity_pub_post_custom_fields.map do |field_name| + "activity_pub_#{field_name.to_s}" + end + activity_pub_post_custom_field_names.each do |field_name| + register_post_custom_field_type(field_name, :string) end add_permitted_post_create_param(:activity_pub_visibility) add_to_class(:post, :activity_pub_url) do - self.activity_pub_object&.url + activity_pub_local? ? activity_pub_full_url : activity_pub_object&.url end add_to_class(:post, :activity_pub_full_url) do "#{DiscourseActivityPub.base_url}#{self.url}" @@ -375,9 +379,9 @@ add_to_class(:post, :activity_pub_after_scheduled) do |args = {}| activity_pub_update_custom_fields(args) end - activity_pub_post_custom_fields.each do |field_name| - add_to_class(:post, "activity_pub_#{field_name}".to_sym) do - custom_fields["activity_pub_#{field_name}"] + activity_pub_post_custom_field_names.each do |field_name| + add_to_class(:post, field_name.to_sym) do + custom_fields[field_name] end end add_to_class(:post, :activity_pub_updated_at) do @@ -404,8 +408,8 @@ type: "post" } - activity_pub_post_custom_fields.each do |field_name| - model[field_name.to_sym] = self.send("activity_pub_#{field_name.to_s}") + activity_pub_post_custom_fields.each do |field| + model[field.to_sym] = self.send("activity_pub_#{field.to_s}") end group_ids =[ @@ -416,8 +420,8 @@ MessageBus.publish("/activity-pub", { model: model }, { group_ids: group_ids }) end add_to_class(:post, :before_clear_all_activity_pub_objects) do - activity_pub_post_custom_fields.each do |field_name| - self.custom_fields["activity_pub_#{field_name.to_s}"] = nil + activity_pub_post_custom_field_names.each do |field_name| + self.custom_fields[field_name] = nil end self.save_custom_fields(true) end @@ -509,10 +513,12 @@ add_to_serializer(:post, :activity_pub_enabled) do object.activity_pub_enabled end - activity_pub_post_custom_fields.each do |field_name| - add_to_serializer(:post, "activity_pub_#{field_name}".to_sym) do - object.send("activity_pub_#{field_name}") - end + activity_pub_post_custom_field_names.each do |field_name| + add_to_serializer( + :post, + field_name.to_sym, + include_condition: -> { object.activity_pub_enabled } + ) { object.send(field_name) } end add_to_serializer( :post, @@ -545,6 +551,19 @@ include_condition: -> { object.activity_pub_enabled } ) { object.activity_pub_is_first_post? } + TopicView.on_preload do |topic_view| + if topic_view.topic.activity_pub_enabled + Post.preload_custom_fields( + topic_view.posts, + activity_pub_post_custom_field_names + ) + ActiveRecord::Associations::Preloader.new( + records: topic_view.posts, + associations: [:activity_pub_object] + ).call + end + end + PostAction.include DiscourseActivityPub::AP::ModelCallbacks add_to_class(:post_action, :activity_pub_enabled) do diff --git a/spec/requests/list_controller_spec.rb b/spec/requests/list_controller_spec.rb new file mode 100644 index 00000000..895f4ea8 --- /dev/null +++ b/spec/requests/list_controller_spec.rb @@ -0,0 +1,80 @@ +# frozen_string_literal: true + +RSpec.describe ListController do + describe "#index" do + context "with activity pub topics" do + fab!(:category1) { Fabricate(:category) } + fab!(:category2) { Fabricate(:category) } + fab!(:category3) { Fabricate(:category) } + fab!(:topic_ids) { + topic_ids = [] + [category1, category2].each do |category| + 5.times do + topic = Fabricate(:topic, category: category) + topic_ids << topic.id + collection = Fabricate(:discourse_activity_pub_ordered_collection, model: topic) + post = Fabricate(:post, topic: topic) + note = Fabricate(:discourse_activity_pub_object_note, model: post, collection_id: collection.id) + activity = Fabricate(:discourse_activity_pub_activity_create, object: note) + end + end + topic_ids + } + + def track_index_queries + track_sql_queries do + get "/latest.json" + expect(response.status).to eq(200) + body = response.parsed_body + expect(body["topic_list"]["topics"].map { |t| t["id"] }).to contain_exactly(*topic_ids) + end + end + + context "without a logged in user" do + it "does not increase the number of queries" do + SiteSetting.activity_pub_enabled = false + + # prime caches + get "/latest.json" + expect(response.status).to eq(200) + + disabled_queries = track_index_queries + + SiteSetting.activity_pub_enabled = true + toggle_activity_pub(category1, callbacks: true) + toggle_activity_pub(category2, callbacks: true) + + enabled_queries = track_index_queries + + expect(enabled_queries.count).to eq(disabled_queries.count) + end + end + + context "with a logged in user" do + let!(:user) { Fabricate(:user) } + + before do + sign_in(user) + end + + it "does not increase the number of queries" do + SiteSetting.activity_pub_enabled = false + + # prime caches + get "/latest.json" + expect(response.status).to eq(200) + + disabled_queries = track_index_queries + + SiteSetting.activity_pub_enabled = true + toggle_activity_pub(category1, callbacks: true) + toggle_activity_pub(category2, callbacks: true) + + enabled_queries = track_index_queries + + expect(enabled_queries.count).to eq(disabled_queries.count) + end + end + end + end +end \ No newline at end of file diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb new file mode 100644 index 00000000..cdde0d62 --- /dev/null +++ b/spec/requests/topics_controller_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +RSpec.describe TopicsController do + ADDITIONAL_QUERY_LIMIT = 6 + + describe "#show" do + fab!(:category) { Fabricate(:category) } + fab!(:topic) { Fabricate(:topic, category: category) } + fab!(:collection) { Fabricate(:discourse_activity_pub_ordered_collection, model: topic) } + fab!(:posts) { + posts = [] + 10.times do + post = Fabricate(:post, topic: topic) + posts << post + note = Fabricate(:discourse_activity_pub_object_note, model: post, collection_id: collection.id) + Fabricate(:discourse_activity_pub_activity_create, object: note) + end + posts + } + + def track_topic_show_queries + track_sql_queries do + get "/t/#{topic.id}.json" + expect(response.status).to eq(200) + end + end + + context "without a logged in user" do + context "with activity pub first_post enabled" do + it "does not increase the number of queries beyond the limit" do + SiteSetting.activity_pub_enabled = false + + get "/t/#{topic.id}.json" + expect(response.status).to eq(200) + + disabled_queries = track_topic_show_queries + + SiteSetting.activity_pub_enabled = true + toggle_activity_pub(category, callbacks: true, publication_type: 'first_post') + + enabled_queries = track_topic_show_queries + expect(enabled_queries.count).to be <= (disabled_queries.count + ADDITIONAL_QUERY_LIMIT) + end + end + + context "with activity pub full_topic enabled" do + it "does not increase the number of queries beyond the limit" do + SiteSetting.activity_pub_enabled = false + + get "/t/#{topic.id}.json" + expect(response.status).to eq(200) + + disabled_queries = track_topic_show_queries + + SiteSetting.activity_pub_enabled = true + toggle_activity_pub(category, callbacks: true, publication_type: 'full_topic') + + enabled_queries = track_topic_show_queries + expect(enabled_queries.count).to be <= (disabled_queries.count + ADDITIONAL_QUERY_LIMIT) + end + end + end + + context "with a logged in user" do + let!(:user) { Fabricate(:user) } + + before do + sign_in(user) + end + + context "with activity pub first_post enabled" do + it "does not increase the number of queries beyond the limit" do + SiteSetting.activity_pub_enabled = false + + get "/t/#{topic.id}.json" + expect(response.status).to eq(200) + + disabled_queries = track_topic_show_queries + + SiteSetting.activity_pub_enabled = true + toggle_activity_pub(category, callbacks: true, publication_type: 'first_post') + + enabled_queries = track_topic_show_queries + + expect(enabled_queries.count).to be <= (disabled_queries.count + ADDITIONAL_QUERY_LIMIT) + end + end + + context "with activity pub full_topic enabled" do + it "does not increase the number of queries beyond the limit" do + SiteSetting.activity_pub_enabled = false + + get "/t/#{topic.id}.json" + expect(response.status).to eq(200) + + disabled_queries = track_topic_show_queries + + SiteSetting.activity_pub_enabled = true + toggle_activity_pub(category, callbacks: true, publication_type: 'full_topic') + + enabled_queries = track_topic_show_queries + expect(enabled_queries.count).to be <= (disabled_queries.count + ADDITIONAL_QUERY_LIMIT) + end + end + end + end +end \ No newline at end of file