Commit d3a3db42 authored by Nick Thomas's avatar Nick Thomas Committed by Stan Hu

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 ded3b757
...@@ -9,6 +9,7 @@ class DeployKey < Key ...@@ -9,6 +9,7 @@ class DeployKey < Key
scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) } scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
scope :are_public, -> { where(public: true) } scope :are_public, -> { where(public: true) }
scope :with_projects, -> { includes(deploy_keys_projects: { project: [:route, :namespace] }) }
ignore_column :can_push ignore_column :can_push
...@@ -23,7 +24,7 @@ class DeployKey < Key ...@@ -23,7 +24,7 @@ class DeployKey < Key
end end
def almost_orphaned? def almost_orphaned?
self.deploy_keys_projects.length == 1 self.deploy_keys_projects.count == 1
end end
def destroyed_when_orphaned? def destroyed_when_orphaned?
...@@ -47,6 +48,6 @@ class DeployKey < Key ...@@ -47,6 +48,6 @@ class DeployKey < Key
end end
def projects_with_write_access 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
end end
# frozen_string_literal: true # frozen_string_literal: true
class DeployKeysProject < ApplicationRecord class DeployKeysProject < ApplicationRecord
belongs_to :project belongs_to :project, inverse_of: :deploy_keys_projects
belongs_to :deploy_key, 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 :without_project_deleted, -> { joins(:project).where(projects: { pending_delete: false }) }
scope :in_project, ->(project) { where(project: project) } scope :in_project, ->(project) { where(project: project) }
scope :with_write_access, -> { where(can_push: true) } scope :with_write_access, -> { where(can_push: true) }
......
...@@ -214,7 +214,7 @@ class Project < ApplicationRecord ...@@ -214,7 +214,7 @@ class Project < ApplicationRecord
as: :source, class_name: 'ProjectMember', dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent as: :source, class_name: 'ProjectMember', dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
has_many :members_and_requesters, as: :source, class_name: 'ProjectMember' 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 :deploy_keys, through: :deploy_keys_projects
has_many :users_star_projects has_many :users_star_projects
has_many :starrers, through: :users_star_projects, source: :user has_many :starrers, through: :users_star_projects, source: :user
......
...@@ -12,48 +12,38 @@ module Projects ...@@ -12,48 +12,38 @@ module Projects
@key ||= DeployKey.new.tap { |dk| dk.deploy_keys_projects.build } @key ||= DeployKey.new.tap { |dk| dk.deploy_keys_projects.build }
end end
# rubocop: disable CodeReuse/ActiveRecord
def enabled_keys def enabled_keys
@enabled_keys ||= project.deploy_keys.includes(:projects) project.deploy_keys
end
# rubocop: enable CodeReuse/ActiveRecord
def any_keys_enabled?
enabled_keys.any?
end end
def available_keys 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 end
# rubocop: disable CodeReuse/ActiveRecord
def available_project_keys def available_project_keys
@available_project_keys ||= current_user.project_deploy_keys.includes(:projects) - enabled_keys current_user
end .project_deploy_keys
# rubocop: enable CodeReuse/ActiveRecord .id_not_in(enabled_keys.select(:id))
.with_projects
def key_available?(deploy_key)
available_keys.include?(deploy_key)
end end
# rubocop: disable CodeReuse/ActiveRecord
def available_public_keys def available_public_keys
return @available_public_keys if defined?(@available_public_keys) DeployKey
.are_public
@available_public_keys ||= DeployKey.are_public.includes(:projects) - enabled_keys .id_not_in(enabled_keys.select(:id))
.id_not_in(available_project_keys.select(:id))
# Public keys that are already used by another accessible project are already .with_projects
# in @available_project_keys.
@available_public_keys -= available_project_keys
end end
# rubocop: enable CodeReuse/ActiveRecord
def as_json def as_json
serializer = DeployKeySerializer.new # rubocop: disable CodeReuse/Serializer serializer = DeployKeySerializer.new # rubocop: disable CodeReuse/Serializer
opts = { user: current_user } 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), available_project_keys: serializer.represent(available_project_keys, opts),
public_keys: serializer.represent(available_public_keys, opts) public_keys: serializer.represent(available_public_keys, opts)
} }
......
...@@ -10,9 +10,10 @@ class DeployKeyEntity < Grape::Entity ...@@ -10,9 +10,10 @@ class DeployKeyEntity < Grape::Entity
expose :created_at expose :created_at
expose :updated_at expose :updated_at
expose :deploy_keys_projects, using: DeployKeysProjectEntity do |deploy_key| expose :deploy_keys_projects, using: DeployKeysProjectEntity do |deploy_key|
deploy_key.deploy_keys_projects deploy_key.deploy_keys_projects.select do |deploy_key_project|
.without_project_deleted !deploy_key_project.project&.pending_delete? &&
.select { |deploy_key_project| Ability.allowed?(options[:user], :read_project, deploy_key_project.project) } Ability.allowed?(options[:user], :read_project, deploy_key_project.project)
end
end end
expose :can_edit expose :can_edit
......
---
title: Speed up loading and filtering deploy keys and their projects
merge_request: 31384
author:
type: performance
...@@ -29,10 +29,6 @@ describe Projects::Settings::DeployKeysPresenter do ...@@ -29,10 +29,6 @@ describe Projects::Settings::DeployKeysPresenter do
it 'returns the enabled_keys size' do it 'returns the enabled_keys size' do
expect(presenter.enabled_keys_size).to eq(1) expect(presenter.enabled_keys_size).to eq(1)
end end
it 'returns true if there is any enabled_keys' do
expect(presenter.any_keys_enabled?).to eq(true)
end
end end
describe '#available_keys/#available_project_keys' do describe '#available_keys/#available_project_keys' do
...@@ -54,9 +50,5 @@ describe Projects::Settings::DeployKeysPresenter do ...@@ -54,9 +50,5 @@ describe Projects::Settings::DeployKeysPresenter do
it 'returns the available_project_keys size' do it 'returns the available_project_keys size' do
expect(presenter.available_project_keys_size).to eq(1) expect(presenter.available_project_keys_size).to eq(1)
end end
it 'shows if there is an available key' do
expect(presenter.key_available?(deploy_key)).to eq(false)
end
end 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