This instance will be upgraded to Heptapod 0.26.0rc1 on 2021-10-26 at 14:30 UTC+2 (a few minutes of down time)

Commit 9ba13c0a authored by Nick Thomas's avatar Nick Thomas Committed by Stan Hu
Browse files

Speed up loading and filtering deploy keys and their projects

This commit changes how we eager-load projects, routes, and namespaces
required by the deploy keys endpoint, getting a 10x improvement in my
local testing.

The endpoint still doesn't scale in-general, but going from ~13 seconds
to dump a 63K result to generating the same thing in ~1.6 seconds seems
like a good improvement to me.
parent a66e0413deaf
......@@ -9,6 +9,7 @@ class DeployKey < Key
scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
scope :are_public, -> { where(public: true) }
scope :with_projects, -> { includes(deploy_keys_projects: { project: [:route, :namespace] }) }
ignore_column :can_push
......@@ -23,7 +24,7 @@ def orphaned?
end
def almost_orphaned?
self.deploy_keys_projects.length == 1
self.deploy_keys_projects.count == 1
end
def destroyed_when_orphaned?
......@@ -47,6 +48,6 @@ def deploy_keys_project_for(project)
end
def projects_with_write_access
Project.preload(:route).where(id: deploy_keys_projects.with_write_access.select(:project_id))
Project.with_route.where(id: deploy_keys_projects.with_write_access.select(:project_id))
end
end
# frozen_string_literal: true
class DeployKeysProject < ApplicationRecord
belongs_to :project
belongs_to :project, inverse_of: :deploy_keys_projects
belongs_to :deploy_key, inverse_of: :deploy_keys_projects
scope :without_project_deleted, -> { joins(:project).where(projects: { pending_delete: false }) }
scope :in_project, ->(project) { where(project: project) }
scope :with_write_access, -> { where(can_push: true) }
......
......@@ -214,7 +214,7 @@ class Project < ApplicationRecord
as: :source, class_name: 'ProjectMember', dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
has_many :members_and_requesters, as: :source, class_name: 'ProjectMember'
has_many :deploy_keys_projects
has_many :deploy_keys_projects, inverse_of: :project
has_many :deploy_keys, through: :deploy_keys_projects
has_many :users_star_projects
has_many :starrers, through: :users_star_projects, source: :user
......
......@@ -12,48 +12,38 @@ def new_key
@key ||= DeployKey.new.tap { |dk| dk.deploy_keys_projects.build }
end
# rubocop: disable CodeReuse/ActiveRecord
def enabled_keys
@enabled_keys ||= project.deploy_keys.includes(:projects)
end
# rubocop: enable CodeReuse/ActiveRecord
def any_keys_enabled?
enabled_keys.any?
project.deploy_keys
end
def available_keys
@available_keys ||= current_user.accessible_deploy_keys - enabled_keys
current_user
.accessible_deploy_keys
.id_not_in(enabled_keys.select(:id))
.with_projects
end
# rubocop: disable CodeReuse/ActiveRecord
def available_project_keys
@available_project_keys ||= current_user.project_deploy_keys.includes(:projects) - enabled_keys
end
# rubocop: enable CodeReuse/ActiveRecord
def key_available?(deploy_key)
available_keys.include?(deploy_key)
current_user
.project_deploy_keys
.id_not_in(enabled_keys.select(:id))
.with_projects
end
# rubocop: disable CodeReuse/ActiveRecord
def available_public_keys
return @available_public_keys if defined?(@available_public_keys)
@available_public_keys ||= DeployKey.are_public.includes(:projects) - enabled_keys
# Public keys that are already used by another accessible project are already
# in @available_project_keys.
@available_public_keys -= available_project_keys
DeployKey
.are_public
.id_not_in(enabled_keys.select(:id))
.id_not_in(available_project_keys.select(:id))
.with_projects
end
# rubocop: enable CodeReuse/ActiveRecord
def as_json
serializer = DeployKeySerializer.new # rubocop: disable CodeReuse/Serializer
opts = { user: current_user }
{
enabled_keys: serializer.represent(enabled_keys, opts),
enabled_keys: serializer.represent(enabled_keys.with_projects, opts),
available_project_keys: serializer.represent(available_project_keys, opts),
public_keys: serializer.represent(available_public_keys, opts)
}
......
......@@ -10,9 +10,10 @@ class DeployKeyEntity < Grape::Entity
expose :created_at
expose :updated_at
expose :deploy_keys_projects, using: DeployKeysProjectEntity do |deploy_key|
deploy_key.deploy_keys_projects
.without_project_deleted
.select { |deploy_key_project| Ability.allowed?(options[:user], :read_project, deploy_key_project.project) }
deploy_key.deploy_keys_projects.select do |deploy_key_project|
!deploy_key_project.project&.pending_delete? &&
Ability.allowed?(options[:user], :read_project, deploy_key_project.project)
end
end
expose :can_edit
......
---
title: Speed up loading and filtering deploy keys and their projects
merge_request: 31384
author:
type: performance
......@@ -29,10 +29,6 @@
it 'returns the enabled_keys size' do
expect(presenter.enabled_keys_size).to eq(1)
end
it 'returns true if there is any enabled_keys' do
expect(presenter.any_keys_enabled?).to eq(true)
end
end
describe '#available_keys/#available_project_keys' do
......@@ -54,9 +50,5 @@
it 'returns the available_project_keys size' do
expect(presenter.available_project_keys_size).to eq(1)
end
it 'shows if there is an available key' do
expect(presenter.key_available?(deploy_key)).to eq(false)
end
end
end
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment