Use prosopite to find & fix more n+1 queries

Gemfile

@@ -96,6 +96,10 @@ group :development, :test do
 
   gem "brakeman", "~> 6.1", require: false
 
+  # used to find n+1 queries
+  gem "prosopite", "~> 1.4"
+  gem "pg_query", "~> 5.1"
+
   # bundle show | rg rubocop | cut -d' ' -f4 | xargs bundle update
   gem "rubocop", "~> 1.48", require: false
   gem "rubocop-rails", "~> 2.18", require: false

Gemfile.lock

@@ -469,6 +469,8 @@ GEM
       ast (~> 2.4.1)
       racc
     pg (1.5.6)
+    pg_query (5.1.0)
+      google-protobuf (>= 3.22.3)
     pghero (3.4.1)
       activerecord (>= 6)
     phlex (1.9.1)
@@ -482,6 +484,7 @@ GEM
     pp (0.5.0)
       prettyprint
     prettyprint (0.2.0)
+    prosopite (1.4.2)
     pry (0.14.1)
       coderay (~> 1.1)
       method_source (~> 1.0)
@@ -812,9 +815,11 @@ DEPENDENCIES
   openid_connect (~> 2.3)
   opensearch-ruby (~> 3.1)
   pg (~> 1.5)
+  pg_query (~> 5.1)
   pghero (~> 3.4)
   phlex-rails (~> 1.1)
   pp (= 0.5.0)
+  prosopite (~> 1.4)
   pry-byebug (~> 3.10)
   puma (~> 6.4)
   pundit (~> 2.3)
@@ -1037,11 +1042,13 @@ CHECKSUMS
   parallel (1.24.0) sha256=5bf38efb9b37865f8e93d7a762727f8c5fc5deb19949f4040c76481d5eee9397
   parser (3.3.0.5) sha256=7748313e505ca87045dc0465c776c802043f777581796eb79b1654c5d19d2687
   pg (1.5.6) sha256=4bc3ad2438825eea68457373555e3fd4ea1a82027b8a6be98ef57c0d57292b1c
+  pg_query (5.1.0) sha256=b7f7f47c864f08ccbed46a8244906fb6ee77ee344fd27250717963928c93145d
   pghero (3.4.1) sha256=7f949828119ab17de22ebaef1854ab8c738106bdc31bee3ac0acd0462c0efa88
   phlex (1.9.1) sha256=cd1500f75ae075930c1f141de0d5c1456c27aa8731796e74548061f742b24d06
   phlex-rails (1.1.1) sha256=b0e82d0ba541eca55ca39051de8be2817a7ed400f8a630e21b261239c8f812d0
   pp (0.5.0) sha256=f8f40bc2ba9e1ab351b9458151da3a89f46034f7f599a8e0a06abb9b9f4411dd
   prettyprint (0.2.0) sha256=2bc9e15581a94742064a3cc8b0fb9d45aae3d03a1baa6ef80922627a0766f193
+  prosopite (1.4.2) sha256=b2e422e2d9dbf3ce20130ded3252fe14adb3833555eacd451f5015d8c9177e79
   pry (0.14.1) sha256=99b6df0665875dd5a39d85e0150aa5a12e2bb4fef401b6c4f64d32ee502f8454
   pry-byebug (3.10.1) sha256=c8f975c32255bfdb29e151f5532130be64ff3d0042dc858d0907e849125581f8
   psych (5.1.2) sha256=337322f58fc2bf24827d2b9bd5ab595f6a72971867d151bb39980060ea40a368

app/controllers/adoptions_controller.rb

@@ -8,7 +8,7 @@ class AdoptionsController < ApplicationController
   def index
     @ownership_call     = @rubygem.ownership_call
     @user_request       = @rubygem.ownership_requests.find_by(user: current_user)
-    @ownership_requests = @rubygem.ownership_requests.includes(:user)
+    @ownership_requests = @rubygem.ownership_requests.preload(:user)
   end
 
   private

app/controllers/api/v1/github_secret_scanning_controller.rb

@@ -24,18 +24,18 @@ def revoke
     return render plain: "Can't fetch public key from GitHub", status: :unauthorized if key.empty_public_key?
     return render plain: "Invalid GitHub Signature", status: :unauthorized unless key.valid_github_signature?(signature, request.body.read.chomp)
 
-    tokens = params.require(:_json).map { |t| t.permit(:token, :type, :url) }
-    resp = []
-    tokens.each do |t|
-      api_key = ApiKey.find_by(hashed_key: hashed_key(t[:token]))
+    tokens = params.require(:_json).map { |t| t.permit(:token, :type, :url) }.index_by { |t| hashed_key(t.require(:token)) }
+    api_keys = ApiKey.where(hashed_key: tokens.keys).index_by(&:hashed_key)
+    resp = tokens.map do |hashed_key, t|
+      api_key = api_keys[hashed_key]
       label = if api_key&.expire!
                 schedule_revoke_email(api_key, t[:url])
                 "true_positive"
               else
                 "false_positive"
               end
 
-      resp << {
+      {
         token_raw: t[:token],
         token_type: t[:type],
         label: label

app/controllers/api/v1/owners_controller.rb

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

app/controllers/api/v1/rubygems_controller.rb

@@ -11,6 +11,7 @@ def index
     return render_forbidden unless @api_key.can_index_rubygems?
 
     @rubygems = @api_key.user.rubygems.with_versions
+      .preload(:linkset, :gem_download, most_recent_version: { dependencies: :rubygem, gem_download: nil })
     respond_to do |format|
       format.json { render json: @rubygems }
       format.yaml { render yaml: @rubygems }

app/controllers/api/v1/timeframe_versions_controller.rb

@@ -40,7 +40,7 @@ def to_time
   end
 
   def render_rubygems(versions)
-    rubygems = versions.includes(:dependencies, rubygem: :linkset).map do |version|
+    rubygems = versions.includes(:dependencies, :gem_download, rubygem: %i[linkset gem_download]).map do |version|
       payload = version.rubygem.payload(version)
       payload.merge(version.as_json)
     end

app/controllers/api/v1/versions_controller.rb

@@ -7,10 +7,11 @@ def show
     cache_expiry_headers
     set_surrogate_key "gem/#{@rubygem.name}"
 
-    if @rubygem.public_versions.present?
+    versions = @rubygem.public_versions.includes(:gem_download)
+    if versions.present?
       respond_to do |format|
-        format.json { render json: @rubygem.public_versions }
-        format.yaml { render yaml: @rubygem.public_versions }
+        format.json { render json: versions }
+        format.yaml { render yaml: versions }
       end
     else
       render plain: t(:this_rubygem_could_not_be_found), status: :not_found

app/controllers/api_keys_controller.rb

@@ -6,7 +6,7 @@ class ApiKeysController < ApplicationController
 
   def index
     @api_key  = session.delete(:api_key)
-    @api_keys = current_user.api_keys.unexpired.not_oidc
+    @api_keys = current_user.api_keys.unexpired.not_oidc.preload(ownership: :rubygem)
     redirect_to new_profile_api_key_path if @api_keys.empty?
   end
 

app/controllers/dashboards_controller.rb

@@ -7,7 +7,7 @@ class DashboardsController < ApplicationController
   def show
     respond_to do |format|
       format.html do
-        @my_gems         = current_user.rubygems.with_versions.by_name
+        @my_gems         = current_user.rubygems.with_versions.by_name.preload(:most_recent_version)
         @latest_updates  = Version.subscribed_to_by(current_user).published.limit(Gemcutter::DEFAULT_PAGINATION)
         @subscribed_gems = current_user.subscribed_gems.with_versions
       end

app/controllers/notifiers_controller.rb

@@ -4,7 +4,7 @@ class NotifiersController < ApplicationController
   before_action :redirect_to_settings_strong_mfa_required, if: :mfa_required_weak_level_enabled?
 
   def show
-    @ownerships = current_user.ownerships.by_indexed_gem_name
+    @ownerships = current_user.ownerships.by_indexed_gem_name.includes(:rubygem)
   end
 
   def update

app/controllers/sendgrid_events_controller.rb

@@ -17,7 +17,10 @@ class SendgridEventsController < ApplicationController
   )
 
   def create
+    existing = SendgridEvent.where(sendgrid_id: events_params.pluck(:sg_event_id)).pluck(:sendgrid_id).to_set
     events_params.each do |event_payload|
+      next unless existing.add?(event_payload.require(:sg_event_id))
+
       SendgridEvent.process_later(event_payload)
     end
     head :ok

app/models/rubygem.rb

@@ -41,7 +41,7 @@ class Rubygem < ApplicationRecord
     uniqueness: { case_sensitive: false },
     name_format: true,
     if: :needs_name_validation?
-  validate :reserved_names_exclusion
+  validate :reserved_names_exclusion, if: :needs_name_validation?
   validate :protected_gem_typo, on: :create, unless: -> { Array(validation_context).include?(:typo_exception) }
 
   after_create :update_unresolved
@@ -207,7 +207,7 @@ def links(version = most_recent_version)
 
   def payload(version = most_recent_version, protocol = Gemcutter::PROTOCOL, host_with_port = Gemcutter::HOST)
     versioned_links = links(version)
-    deps = version.dependencies.where.associated(:rubygem).to_a
+    deps = version.dependencies.to_a.select(&:rubygem)
     {
       "name"               => name,
       "downloads"          => downloads,
@@ -311,9 +311,7 @@ def reorder_versions
       .indexed
       .group_by(&:platform)
 
-    versions_of_platforms.each_value do |platforms|
-      Version.default_scoped.find(platforms.max.id).update_column(:latest, true)
-    end
+    Version.default_scoped.where(id: versions_of_platforms.values.map! { |v| v.max.id }).update_all(latest: true)
   end
 
   def refresh_indexed!
@@ -422,7 +420,7 @@ def mark_unresolved
   end
 
   def bulk_reorder_versions
-    numbers = reload.versions.sort.reverse.map(&:number).uniq
+    numbers = reload.versions.pluck(:number).uniq.sort_by { |n| Gem::Version.new(n) }.reverse
 
     ids = []
     positions = []

app/models/sendgrid_event.rb

@@ -25,8 +25,6 @@ def self.fails_since_last_delivery(email)
   end
 
   def self.process_later(payload)
-    return if exists?(sendgrid_id: payload[:sg_event_id])
-
     transaction do
       sendgrid_event = create!(
         sendgrid_id: payload[:sg_event_id],
@@ -38,6 +36,8 @@ def self.process_later(payload)
       )
       ProcessSendgridEventJob.perform_later(sendgrid_event:)
     end
+  rescue ActiveRecord::RecordNotUnique
+    nil
   end
 
   def self.process(id)

app/models/version.rb

@@ -95,6 +95,10 @@ def self.subscribed_to_by(user)
       .by_created_at
   end
 
+  def self.created_after(time)
+    where(arel_table[:created_at].gt(Arel::Nodes::BindParam.new(time)))
+  end
+
   def self.latest
     where(latest: true)
   end
@@ -159,18 +163,14 @@ def self.new_pushed_versions(limit = 5)
   end
 
   def self.just_updated(limit = 5)
-    six_months_ago_ts = 6.months.ago
-    subquery = <<~SQL.squish
-      versions.rubygem_id IN (SELECT versions.rubygem_id
-                                FROM versions
-                            WHERE versions.indexed = 'true' AND
-                                  versions.created_at > '#{six_months_ago_ts}'
-                            GROUP BY versions.rubygem_id
-                              HAVING COUNT(versions.id) > 1
-                              ORDER BY MAX(created_at) DESC LIMIT :limit)
-    SQL
-
-    where(subquery, limit: limit)
+    where(rubygem_id: Version.default_scoped
+                        .select(:rubygem_id)
+                        .indexed
+                        .created_after(6.months.ago)
+                        .group(:rubygem_id)
+                        .having(arel_table["id"].count.gt(1))
+                        .order(arel_table["created_at"].maximum.desc)
+                        .limit(limit))
       .joins(:rubygem)
       .indexed
       .by_created_at
@@ -490,10 +490,11 @@ def no_dashes_in_version_number
   end
 
   def create_link_verifications
-    Links::LINKS.each_value do |long|
-      uri = metadata[long]
-      next if uri.blank?
-      rubygem.link_verifications.find_or_create_by!(uri:).retry_if_needed
+    uris = metadata.values_at(*Links::LINKS.values).compact_blank.uniq
+    verifications = rubygem.link_verifications.where(uri: uris).index_by(&:uri)
+    uris.each do |uri|
+      verification = verifications.fetch(uri) { rubygem.link_verifications.create_or_find_by!(uri:) }
+      verification.retry_if_needed
     end
   end
 

config/initializers/prosopite.rb

@@ -0,0 +1,25 @@
+if Rails.env.local?
+  require 'prosopite/middleware/rack'
+  Rails.configuration.middleware.use(Prosopite::Middleware::Rack)
+
+  Rails.application.config.after_initialize do
+    Prosopite.custom_logger = SemanticLogger[Prosopite]
+    Prosopite.raise = true
+    Prosopite.ignore_queries = [
+      # dependency api intentionally loads 1 by 1, so each gem can be stored in the cache
+      # plus the API is deprecated
+      Regexp.new(Regexp.quote("SELECT rv.name, rv.number, rv.platform, d.requirements, for_dep_name.name dep_name"))
+    ]
+    Prosopite.allow_stack_paths = [
+      # mailers need refactoring to not find based on IDs when we already have objects in memory
+      "app/mailers/",
+
+      # avo auditing potentially loads things multiple times, but it will be bounded by the size of the audit
+      "app/avo/actions/base_action.rb",
+      "app/components/avo/fields/audited_changes_field/show_component.html.erb",
+
+      # calls count for each owner, AR doesn't yet allow preloading aggregates
+      "app/views/ownership_requests/_ownership_request.html.erb"
+    ]
+  end
+end

test/functional/api/v1/activities_controller_test.rb

@@ -12,7 +12,8 @@ class Api::V1::ActivitiesControllerTest < ActionController::TestCase
         create(:version, rubygem: @sinatra)
 
         @foobar = create(:rubygem, name: "foobar")
-        create(:version, rubygem: @foobar)
+        create(:version, rubygem: @foobar,
+               dependencies: [build(:dependency, rubygem: @rails), build(:dependency, rubygem: @sinatra)])
       end
 
       should "return correct JSON for latest gems" do

test/functional/api/v1/owners_controller_test.rb

@@ -72,7 +72,13 @@ def self.should_respond_to(format)
     setup do
       @user = create(:user)
       rubygem = create(:rubygem, owners: [@user])
-      create(:version, rubygem: rubygem)
+      version = create(:version, rubygem: rubygem)
+      rubygem2 = create(:rubygem, owners: [@user])
+      rubygem3 = create(:rubygem, owners: [@user])
+      version2 = create(:version, rubygem: rubygem2)
+      create(:dependency, version: version, rubygem: rubygem2, requirements: ">= 0", scope: "runtime")
+      create(:dependency, version: version, rubygem: rubygem3, requirements: ">= 0", scope: "development")
+      create(:dependency, version: version2, rubygem: rubygem3, requirements: ">= 0", scope: "runtime")
       get :gems, params: { handle: @user.id }, format: :json
     end
 

test/test_helper.rb

@@ -187,6 +187,14 @@ def teardown_rstuf
   end
 end
 
+class ActionController::TestCase
+  def process(...)
+    Prosopite.scan do
+      super
+    end
+  end
+end
+
 class ActionDispatch::IntegrationTest
   include OauthHelpers
   setup { host! Gemcutter::HOST }