Eliminate n+1 queries in multiple problematic actions (#4520)

app/controllers/api/v1/owners_controller.rb

@@ -53,7 +53,10 @@ def destroy
   def gems
     user = User.find_by_slug(params[:handle])
     if user
-      rubygems = user.rubygems.with_versions
+      rubygems = user.rubygems.with_versions.includes(
+        :linkset, :gem_download,
+        most_recent_version: %i[dependencies gem_download]
+      ).strict_loading
       respond_to do |format|
         format.json { render json: rubygems }
         format.yaml { render yaml: rubygems }

app/controllers/api/v1/rubygems_controller.rb

@@ -21,7 +21,7 @@ def show
     cache_expiry_headers
     set_surrogate_key "gem/#{@rubygem.name}"
 
-    if @rubygem.hosted? && @rubygem.public_versions.indexed.count.nonzero?
+    if @rubygem.hosted? && @rubygem.public_versions.indexed.present?
       respond_to do |format|
         format.json { render json: @rubygem }
         format.yaml { render yaml: @rubygem }

app/controllers/api/v1/versions_controller.rb

@@ -7,7 +7,7 @@ def show
     cache_expiry_headers
     set_surrogate_key "gem/#{@rubygem.name}"
 
-    if @rubygem.public_versions.count.nonzero?
+    if @rubygem.public_versions.present?
       respond_to do |format|
         format.json { render json: @rubygem.public_versions }
         format.yaml { render yaml: @rubygem.public_versions }
@@ -24,7 +24,7 @@ def latest
     set_surrogate_key "gem/#{params[:id]}"
 
     version = nil
-    version = rubygem.versions.most_recent if rubygem&.public_versions&.indexed&.count&.nonzero?
+    version = rubygem.most_recent_version if rubygem&.public_versions.present?
     number = version.number if version
     render json: { "version" => number || "unknown" }, callback: params["callback"]
   end

app/controllers/api/v1/web_hooks_controller.rb

@@ -34,7 +34,7 @@ def fire
     @rubygem ||= Rubygem.find_by_name("gemcutter")
 
     if webhook.fire(request.protocol.delete("://"), request.host_with_port,
-                    @rubygem.versions.most_recent, delayed: false)
+                    @rubygem.most_recent_version, delayed: false)
       render plain: webhook.deployed_message(@rubygem)
     else
       render plain: webhook.failed_message(@rubygem), status: :bad_request

app/controllers/concerns/latest_version.rb

@@ -3,7 +3,7 @@ module LatestVersion
 
   included do
     def latest_version
-      @latest_version ||= @rubygem.versions.most_recent
+      @latest_version ||= @rubygem.most_recent_version
     end
 
     def latest_version_by_slug

app/controllers/dashboards_controller.rb

@@ -8,11 +8,11 @@ def show
     respond_to do |format|
       format.html do
         @my_gems         = current_user.rubygems.with_versions.by_name
-        @latest_updates  = Version.subscribed_to_by(current_user).published(Gemcutter::DEFAULT_PAGINATION)
+        @latest_updates  = Version.subscribed_to_by(current_user).published.limit(Gemcutter::DEFAULT_PAGINATION)
         @subscribed_gems = current_user.subscribed_gems.with_versions
       end
       format.atom do
-        @versions = Version.subscribed_to_by(api_or_logged_in_user).published(Gemcutter::DEFAULT_PAGINATION)
+        @versions = Version.subscribed_to_by(api_or_logged_in_user).published.limit(Gemcutter::DEFAULT_PAGINATION)
         render "versions/feed"
       end
     end

app/controllers/dependencies_controller.rb

@@ -6,6 +6,7 @@ class DependenciesController < ApplicationController
   def show
     @dependencies = Hash.new { |h, k| h[k] = [] }
     resolvable_dependencies = @latest_version.dependencies.where(unresolved_name: nil)
+      .strict_loading.preload(rubygem: :public_versions)
 
     resolvable_dependencies.each do |dependency|
       gem_name = dependency.rubygem.name
@@ -44,6 +45,6 @@ def json_return
 
   def render_str_call(scope)
     local_var = { scope: scope, dependencies: @dependencies, gem_name: @latest_version.rubygem.name }
-    ActionController::Base.new.render_to_string(partial: "dependencies/dependencies", formats: [:html], locals: local_var)
+    self.class.renderer.new(request.env).render(partial: "dependencies/dependencies", formats: [:html], locals: local_var)
   end
 end

app/controllers/profiles_controller.rb

@@ -8,8 +8,9 @@ class ProfilesController < ApplicationController
   before_action :disable_cache, only: :edit
 
   def show
-    @user           = User.find_by_slug!(params[:id])
-    rubygems        = @user.rubygems_downloaded
+    @user = User.includes(rubygems_downloaded: %i[most_recent_version gem_download]).strict_loading
+      .find_by_slug!(params[:id])
+    rubygems        = @user.rubygems_downloaded.to_a
     @rubygems       = rubygems.slice!(0, 10)
     @extra_rubygems = rubygems
   end
@@ -40,7 +41,7 @@ def update
 
   def delete
     @only_owner_gems = current_user.only_owner_gems
-    @multi_owner_gems = current_user.rubygems_downloaded - @only_owner_gems
+    @multi_owner_gems = current_user.rubygems_downloaded.to_a - @only_owner_gems
   end
 
   def destroy

app/controllers/rubygems_controller.rb

@@ -15,7 +15,7 @@ def index
         @gems   = Rubygem.letter(@letter).includes(:latest_version, :gem_download).page(@page)
       end
       format.atom do
-        @versions = Version.published(Gemcutter::DEFAULT_PAGINATION)
+        @versions = Version.published.limit(Gemcutter::DEFAULT_PAGINATION)
         render "versions/feed"
       end
     end
@@ -25,7 +25,7 @@ def show
     if @reserved_gem
       render "reserved"
     else
-      @versions = @rubygem.public_versions(5)
+      @versions = @rubygem.public_versions.limit(5)
       @adoption = @rubygem.ownership_call
       if @versions.to_a.any?
         render "show"

app/models/concerns/rubygem_searchable.rb

@@ -30,7 +30,7 @@ module RubygemSearchable
       }
 
     def search_data # rubocop:disable Metrics/MethodLength
-      if (latest_version = versions.most_recent)
+      if (latest_version = most_recent_version)
         deps = latest_version.dependencies.to_a
         versioned_links = links(latest_version)
       end

app/models/gem_typo.rb

@@ -35,6 +35,6 @@ def matched_protected_gem_name
 
   def not_protected?(rubygem)
     return true unless rubygem
-    rubygem.downloads < DOWNLOADS_THRESHOLD && rubygem.versions.most_recent.created_at < LAST_RELEASE_TIME
+    rubygem.downloads < DOWNLOADS_THRESHOLD && rubygem.most_recent_version.created_at < LAST_RELEASE_TIME
   end
 end

app/models/rubygem.rb

@@ -27,6 +27,14 @@ class Rubygem < ApplicationRecord
   has_many :reverse_development_dependencies, -> { merge(Dependency.development) }, through: :incoming_dependencies, source: :version_rubygem
   has_many :reverse_runtime_dependencies, -> { merge(Dependency.runtime) }, through: :incoming_dependencies, source: :version_rubygem
 
+  has_one :most_recent_version,
+    lambda {
+      order(Arel.sql("case when #{quoted_table_name}.latest AND #{quoted_table_name}.platform = 'ruby' then 2 else 1 end desc"))
+        .order(Arel.sql("case when #{quoted_table_name}.latest then #{quoted_table_name}.number else NULL end desc"))
+        .order(id: :desc)
+    },
+    class_name: "Version", inverse_of: :rubygem
+
   validates :name,
     length: { maximum: Gemcutter::MAX_FIELD_LENGTH },
     presence: true,
@@ -127,12 +135,10 @@ def all_errors(version = nil)
     end.flatten.join(", ")
   end
 
-  def public_versions(limit = nil)
-    versions.includes(:gem_download).by_position.published(limit)
-  end
+  has_many :public_versions, -> { by_position.published }, class_name: "Version", inverse_of: :rubygem
 
   def public_versions_with_extra_version(extra_version)
-    versions = public_versions(5).to_a
+    versions = public_versions.limit(5).to_a
     versions << extra_version
     versions.uniq.sort_by(&:position)
   end
@@ -195,17 +201,13 @@ def downloads
     gem_download&.count || 0
   end
 
-  def most_recent_version
-    versions.most_recent
-  end
-
   def links(version = most_recent_version)
     Links.new(self, version)
   end
 
   def payload(version = most_recent_version, protocol = Gemcutter::PROTOCOL, host_with_port = Gemcutter::HOST)
     versioned_links = links(version)
-    deps = version.dependencies.to_a
+    deps = version.dependencies.where.associated(:rubygem).to_a
     {
       "name"               => name,
       "downloads"          => downloads,
@@ -230,8 +232,8 @@ def payload(version = most_recent_version, protocol = Gemcutter::PROTOCOL, host_
       "changelog_uri"      => versioned_links.changelog_uri,
       "funding_uri"        => versioned_links.funding_uri,
       "dependencies"       => {
-        "development" => deps.select { |r| r.rubygem && r.scope == "development" },
-        "runtime"     => deps.select { |r| r.rubygem && r.scope == "runtime" }
+        "development" => deps.select { |r| r.scope == "development" },
+        "runtime"     => deps.select { |r| r.scope == "runtime" }
       }
     }
   end

app/models/user.rb

@@ -34,6 +34,11 @@ class User < ApplicationRecord
   has_many :subscriptions, dependent: :destroy
   has_many :subscribed_gems, -> { order("name ASC") }, through: :subscriptions, source: :rubygem
 
+  has_many :rubygems_downloaded,
+    -> { with_versions.joins(:gem_download).order(GemDownload.arel_table["count"].desc) },
+    through: :ownerships,
+    source: :rubygem
+
   has_many :pushed_versions, -> { by_created_at }, dependent: :nullify, inverse_of: :pusher, class_name: "Version", foreign_key: :pusher_id
   has_many :yanked_versions, through: :deletions, source: :version, inverse_of: :yanker
 
@@ -190,11 +195,7 @@ def generate_api_key
   end
 
   def total_downloads_count
-    rubygems.to_a.sum(&:downloads)
-  end
-
-  def rubygems_downloaded
-    rubygems.with_versions.sort_by { |rubygem| -rubygem.downloads }
+    rubygems.joins(:gem_download).sum(:count)
   end
 
   def total_rubygems_count

app/models/version.rb

@@ -4,7 +4,9 @@ class Version < ApplicationRecord # rubocop:disable Metrics/ClassLength
   RUBYGEMS_IMPORT_DATE = Date.parse("2009-07-25")
 
   belongs_to :rubygem, touch: true
-  has_many :dependencies, -> { order("rubygems.name ASC").includes(:rubygem) }, dependent: :destroy, inverse_of: "version"
+  has_many :dependencies, lambda {
+                            order(Rubygem.arel_table["name"].asc).includes(:rubygem).references(:rubygem)
+                          }, dependent: :destroy, inverse_of: "version"
   has_many :audits, as: :auditable, inverse_of: :auditable, dependent: :nullify
   has_one :gem_download, inverse_of: :version, dependent: :destroy
   belongs_to :pusher, class_name: "User", inverse_of: :pushed_versions, optional: true
@@ -145,10 +147,6 @@ def self.rows_for_prerelease_index
       .pluck("rubygems.name", :number, :platform)
   end
 
-  def self.most_recent
-    latest.find_by(platform: "ruby") || latest.order(number: :desc).first || last
-  end
-
   # This method returns the new versions for brand new rubygems
   def self.new_pushed_versions(limit = 5)
     subquery = <<~SQL.squish
@@ -179,8 +177,8 @@ def self.just_updated(limit = 5)
       .limit(limit)
   end
 
-  def self.published(limit)
-    indexed.by_created_at.limit(limit)
+  def self.published
+    indexed.by_created_at
   end
 
   def self.rubygem_name_for(full_name)
@@ -362,7 +360,7 @@ def to_install
     latest = if prerelease
                rubygem.versions.by_position.prerelease.first
              else
-               rubygem.versions.most_recent
+               rubygem.most_recent_version
              end
     command << " -v #{number}" if latest != self
     command << " --pre" if prerelease

app/views/dashboards/show.html.erb

@@ -46,7 +46,7 @@
         <ul>
           <% @my_gems.each do |rubygem| %>
             <li>
-              <%= link_to rubygem.name, rubygem_path(rubygem.slug), :title => short_info(rubygem.versions.most_recent), :class => 't-link' %>
+              <%= link_to rubygem.name, rubygem_path(rubygem.slug), :title => short_info(rubygem.most_recent_version), :class => 't-link' %>
             </li>
           <% end %>
         </ul>
@@ -57,7 +57,7 @@
         <ul>
           <% current_user.subscribed_gems.each do |gem| %>
             <li>
-              <%= link_to gem, rubygem_path(gem.slug), :title => short_info(gem.versions.most_recent), :class => 't-link' %>
+              <%= link_to gem, rubygem_path(gem.slug), :title => short_info(gem.most_recent_version), :class => 't-link' %>
             </li>
           <% end %>
         </ul>

app/views/dependencies/_dependencies.html.erb

@@ -7,10 +7,10 @@
         <span>
           <span class="deps_expanded deps_expanded-link" data-gem_id="<%= name %>" data-version="<%= version %>"></span>
         </span>
-        <a target="_blank" href="/gems/<%= name %>/versions/<%= version %>">
+        <%= link_to rubygem_version_path(name, version), target: :_blank do %>
           <span class="deps_item"><%= name %> <%= version %>
           <span class='deps_item--details'> <%= req %></span></span>
-        </a>
+        <% end %>
 
         <div><div class="deps_scope"></div></div>
         <div><div class="deps_scope"></div></div>

app/views/profiles/_rubygem.html.erb

@@ -3,7 +3,7 @@
     <span class="gems__gem__info">
       <a href="<%= rubygem_path(rubygem.slug) %>" class="gems__gem__name">
         <%= rubygem.name %>
-        <span class="gems__gem__version"><%= rubygem.versions.most_recent %></span>
+        <span class="gems__gem__version"><%= rubygem.most_recent_version %></span>
       </a>
     </span>
     <p class="gems__gem__downloads__count">

test/components/previews/oidc/trusted_publisher/github_action/table_component_preview.rb

@@ -1,7 +1,7 @@
 class OIDC::TrustedPublisher::GitHubAction::TableComponentPreview < Lookbook::Preview
   # @param environment text The environment for the GitHub Action
-  def default(environment: nil, repository_name: "rubygem2", workflow_filename: "push_gem.yml")
-    github_action = FactoryBot.build(:oidc_trusted_publisher_github_action, environment:, repository_name:, workflow_filename:)
+  def default(environment: nil, repository_name: nil, workflow_filename: nil)
+    github_action = FactoryBot.build(:oidc_trusted_publisher_github_action, **{ environment:, repository_name:, workflow_filename: }.compact)
     render OIDC::TrustedPublisher::GitHubAction::TableComponent.new(github_action:)
   end
 end

test/functional/api/v1/owners_controller_test.rb

@@ -71,6 +71,8 @@ def self.should_respond_to(format)
   context "on GET to owner gems with id" do
     setup do
       @user = create(:user)
+      rubygem = create(:rubygem, owners: [@user])
+      create(:version, rubygem: rubygem)
       get :gems, params: { handle: @user.id }, format: :json
     end
 

test/functional/api/v1/rubygems_controller_test.rb

@@ -362,7 +362,7 @@ def self.should_respond_to(format)
       end
       should respond_with :conflict
       should "not register new version" do
-        version = Rubygem.last.reload.versions.most_recent
+        version = Rubygem.last.reload.most_recent_version
 
         assert_equal @date.to_fs(:db), version.built_at.to_fs(:db), "(date)"
         assert_equal "Freewill", version.summary, "(summary)"

test/functional/dashboards_controller_test.rb

@@ -53,7 +53,7 @@ class DashboardsControllerTest < ActionController::TestCase
       should "render links" do
         @gems.each do |g|
           assert page.has_content?(g.name)
-          selector = "a[href='#{rubygem_path(g.slug)}'][title='#{g.versions.most_recent.info}']"
+          selector = "a[href='#{rubygem_path(g.slug)}'][title='#{g.most_recent_version.info}']"
 
           page.assert_selector(selector)
         end

test/functional/dependencies_controller_test.rb

@@ -8,7 +8,7 @@ class DependenciesControllerTest < ActionController::TestCase
     @dep_rubygem = create(:rubygem)
 
     ["1.0.2", "2.4.3", "4.5.6"].map do |ver_number|
-      create(:version, number: ver_number, rubygem: @dep_rubygem)
+      create(:version, number: ver_number, rubygem: @dep_rubygem, indexed: true)
     end
 
     create(:dependency,
@@ -24,7 +24,7 @@ def request_endpoint(rubygem, version, format = "html")
 
   def render_str_call(scope, dependencies)
     local_var = { scope: scope, dependencies: dependencies, gem_name: @rubygem.name }
-    ActionController::Base.new.render_to_string(partial: "dependencies/dependencies", formats: [:html], locals: local_var)
+    DependenciesController.renderer.new({}).render(partial: "dependencies/dependencies", formats: [:html], locals: local_var)
   end
 
   context "GET to show in html" do

test/functional/searches_controller_test.rb

@@ -18,7 +18,7 @@ class SearchesControllerTest < ActionController::TestCase
       @sinatra = create(:rubygem, name: "sinatra")
       import_and_refresh
 
-      assert_nil @sinatra.versions.most_recent
+      assert_nil @sinatra.most_recent_version
       assert_predicate @sinatra.reload.versions.count, :zero?
       get :show, params: { query: "sinatra" }
     end

test/models/gem_download_test.rb

@@ -117,7 +117,7 @@ def test_update_the_count_atomically
 
       should "set version_downloads of ES record with most_recent version downloads" do
         2.times.each do |i|
-          most_recent_version = @gems[i].versions.most_recent
+          most_recent_version = @gems[i].most_recent_version
 
           assert_equal most_recent_version.downloads_count, es_version_downloads(@gems[i].id)
         end
@@ -128,7 +128,7 @@ def test_update_the_count_atomically
       setup do
         @rubygem = create(:rubygem, number: "0.0.1.rc")
         import_and_refresh
-        most_recent_version = @rubygem.versions.most_recent
+        most_recent_version = @rubygem.most_recent_version
         version_downloads = [most_recent_version.full_name, 40]
         GemDownload.bulk_update([version_downloads])
       end