Skip to content

Commit

Permalink
[WIP] Use prosopite to find & fix more n+1 queries
Browse files Browse the repository at this point in the history
  • Loading branch information
segiddins committed Mar 13, 2024
1 parent 8d62809 commit 578d1e7
Show file tree
Hide file tree
Showing 13 changed files with 57 additions and 14 deletions.
4 changes: 4 additions & 0 deletions Gemfile
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions Gemfile.lock
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/adoptions_controller.rb
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions app/controllers/api/v1/owners_controller.rb
Expand Up @@ -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 }
Expand Down
1 change: 1 addition & 0 deletions app/controllers/api/v1/rubygems_controller.rb
Expand Up @@ -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 }
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/api/v1/timeframe_versions_controller.rb
Expand Up @@ -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
Expand Down
7 changes: 4 additions & 3 deletions app/controllers/api/v1/versions_controller.rb
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/dashboards_controller.rb
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion app/controllers/notifiers_controller.rb
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions app/models/rubygem.rb
Expand Up @@ -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,
Expand Down Expand Up @@ -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!
Expand Down
17 changes: 17 additions & 0 deletions config/initializers/prosopite.rb
@@ -0,0 +1,17 @@
if Rails.env.local?
require 'prosopite/middleware/rack'
Rails.configuration.middleware.use(Prosopite::Middleware::Rack)

Rails.application.config.after_initialize do
Prosopite.rails_logger = true
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 = [
%(app/mailers/)
]
end
end
8 changes: 7 additions & 1 deletion test/functional/api/v1/owners_controller_test.rb
Expand Up @@ -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

Expand Down
9 changes: 9 additions & 0 deletions test/test_helper.rb
Expand Up @@ -187,6 +187,15 @@ def teardown_rstuf
end
end

class ActionController::TestCase
def process(...)
Prosopite.scan
super
ensure
Prosopite.finish
end
end

class ActionDispatch::IntegrationTest
include OauthHelpers
setup { host! Gemcutter::HOST }
Expand Down

0 comments on commit 578d1e7

Please sign in to comment.