From 63394c2b26a0c2c012aa82b6d89532055b66fa06 Mon Sep 17 00:00:00 2001 From: Bilka Date: Fri, 17 Apr 2026 16:06:01 +0200 Subject: [PATCH 1/3] AO3-7405 Add tests --- .../other_b/subscriptions_fandoms.feature | 4 ++ .../step_definitions/subscription_steps.rb | 5 ++ spec/requests/tags_n_plus_one_spec.rb | 47 +++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 spec/requests/tags_n_plus_one_spec.rb diff --git a/features/other_b/subscriptions_fandoms.feature b/features/other_b/subscriptions_fandoms.feature index 72f84f8944b..ac36d195144 100644 --- a/features/other_b/subscriptions_fandoms.feature +++ b/features/other_b/subscriptions_fandoms.feature @@ -17,13 +17,17 @@ Feature: Subscriptions Scenario: Subscribe to a test fandom when there are works in it When I am logged in as "author" + And time is frozen at 2026-04-11 16:00 And I post a work "My Work Title" with category "F/F" + And time is frozen at 2026-04-11 17:00 + And I post a work "Silly Games" with category "F/F" When I am logged in as "reader" And I view the "F/F" works index Then I should see "RSS Feed" When I follow "RSS Feed" Then I should see "My Work Title" And I should see "Stargate SG-1" + And the feed updated date should be should be the created date of "Silly Games" Scenario: Subscribe to a non-test fandom diff --git a/features/step_definitions/subscription_steps.rb b/features/step_definitions/subscription_steps.rb index b99bfaee3b2..47241556bea 100644 --- a/features/step_definitions/subscription_steps.rb +++ b/features/step_definitions/subscription_steps.rb @@ -37,6 +37,11 @@ expect(page).to have_selector(:xpath, "//author").exactly(int) end +Then "the feed updated date should be should be the created date of {string}" do |work| + w = Work.find_by(title: work) + expect(find(:xpath, "//feed/updated")).to have_content(w.created_at.xmlschema) +end + # rubocop:disable Cucumber/RegexStepName Then /^the (\d+)(?:st|nd|rd|th) feed author should contain "([^"]*)"$/ do |n, text| within(:xpath, "//author[#{n}]") do diff --git a/spec/requests/tags_n_plus_one_spec.rb b/spec/requests/tags_n_plus_one_spec.rb new file mode 100644 index 00000000000..7307c6b8026 --- /dev/null +++ b/spec/requests/tags_n_plus_one_spec.rb @@ -0,0 +1,47 @@ +require "spec_helper" + +describe "n+1 queries in the tags controller" do + describe "#feed", n_plus_one: true do + context "when creating a tag's feed" do + let!(:tag) { create(:canonical_fandom, name: "Hermitcraft SMP") } + + subject do + proc do + get feed_tag_path({ id: tag.id, format: :atom }) + end + end + + context "when all bylines are cached" do + populate do |n| + create_list(:work, n, fandom_string: "Hermitcraft SMP") + subject.call # this caches the bylines + end + + it "produces a constant number of queries" do + expect do + subject.call + expect(response.body.scan("").size).to eq(current_scale.to_i) + end.to perform_constant_number_of_queries + end + end + + context "when nothing is cached" do + populate do |n| + create_list(:work, n, fandom_string: "Hermitcraft SMP") + end + + it "performs around 4 queries per work" do + # TODO: Ideally, we'd like the uncached tag feed to also have a + # constant number of queries, instead of the linear number of queries + # we're checking for here. But we also don't want to put too much + # unnecessary load on the databases when we have a bunch of cache hits, + # so this requires some care & tuning. + expect do + subject.call + expect(response.body.scan("").size).to eq(current_scale.to_i) + end.to perform_linear_number_of_queries(slope: 4).with_warming_up + end + end + end + end +end From c7f2a32f77eec86d2df93c3fa2f7d83ea6cbf980 Mon Sep 17 00:00:00 2001 From: Bilka Date: Fri, 17 Apr 2026 16:10:25 +0200 Subject: [PATCH 2/3] AO3-7405 Prevent N+1 queries on tags#feed --- app/controllers/tags_controller.rb | 18 ++++++++---------- app/helpers/works_helper.rb | 2 +- app/views/tags/feed.atom.builder | 5 +++-- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 3bd9829fe40..8185d03efd9 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -72,19 +72,17 @@ def show end def feed - begin - @tag = Tag.find(params[:id]) - rescue ActiveRecord::RecordNotFound - raise ActiveRecord::RecordNotFound, "Couldn't find tag with id '#{params[:id]}'" - end + @tag = Tag.find(params[:id]) @tag = @tag.merger if !@tag.canonical? && @tag.merger # Temp for testing if %w(Fandom Character Relationship).include?(@tag.type.to_s) || @tag.name == 'F/F' - if @tag.canonical? - @works = @tag.filtered_works.visible_to_all.order('created_at DESC').limit(25) - else - @works = @tag.works.visible_to_all.order('created_at DESC').limit(25) - end + @works = if @tag.canonical? + @tag.filtered_works + else + @tag.works + end + + @works = @works.visible_to_all.order(created_at: :desc).limit(25).includes(:tags, :language, :series) else redirect_to(tag_works_path(tag_id: @tag.to_param)) && return end diff --git a/app/helpers/works_helper.rb b/app/helpers/works_helper.rb index 36ab1e42a94..fe788bba8f1 100644 --- a/app/helpers/works_helper.rb +++ b/app/helpers/works_helper.rb @@ -174,7 +174,7 @@ def feed_summary(work) text = +"

by #{byline(work, { visibility: 'public', only_path: false })}

" text << work.summary if work.summary text << "

Words: #{work.word_count}, Chapters: #{chapter_total_display(work)}, Language: #{work.language ? work.language.name : 'English'}

" - text << "

Series: #{series_list_with_work_position(work)}

" unless work.series.count.zero? + text << "

Series: #{series_list_with_work_position(work)}

" unless work.series.size.zero? # Create list of tags text << "
    " %w(Fandom Rating ArchiveWarning Category Character Relationship Freeform).each do |type| diff --git a/app/views/tags/feed.atom.builder b/app/views/tags/feed.atom.builder index f8a614d9932..6b4be56ea31 100644 --- a/app/views/tags/feed.atom.builder +++ b/app/views/tags/feed.atom.builder @@ -1,10 +1,11 @@ atom_feed do |feed| feed.title "AO3 works tagged '#{@tag.name}'" - feed.updated @works.first.created_at if @works.respond_to?(:first) && @works.first.present? - @works.each do |work| + @works.each_with_index do |work, index| next if work.unrevealed? + feed.updated work.created_at if index == 0 + feed.entry work do |entry| entry.title work.title entry.summary feed_summary(work), type: "html" From e2b4568d368b230e4e38bf66a661721c29234f84 Mon Sep 17 00:00:00 2001 From: Bilka Date: Fri, 17 Apr 2026 17:14:16 +0200 Subject: [PATCH 3/3] AO3-7405 Rubocop --- app/controllers/tags_controller.rb | 1 - app/views/tags/feed.atom.builder | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 8185d03efd9..3c291f060e7 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -81,7 +81,6 @@ def feed else @tag.works end - @works = @works.visible_to_all.order(created_at: :desc).limit(25).includes(:tags, :language, :series) else redirect_to(tag_works_path(tag_id: @tag.to_param)) && return diff --git a/app/views/tags/feed.atom.builder b/app/views/tags/feed.atom.builder index 6b4be56ea31..395a6ad4e2d 100644 --- a/app/views/tags/feed.atom.builder +++ b/app/views/tags/feed.atom.builder @@ -4,7 +4,7 @@ atom_feed do |feed| @works.each_with_index do |work, index| next if work.unrevealed? - feed.updated work.created_at if index == 0 + feed.updated work.created_at if index.zero? feed.entry work do |entry| entry.title work.title