summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/deploy_key.rb6
-rw-r--r--app/models/deploy_keys_project.rb3
-rw-r--r--app/models/project.rb2
-rw-r--r--app/models/user.rb11
-rw-r--r--app/presenters/projects/settings/deploy_keys_presenter.rb40
-rw-r--r--app/serializers/deploy_key_entity.rb7
-rw-r--r--changelogs/unreleased/43080-speed-up-deploy-keys.yml5
-rw-r--r--db/post_migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb23
-rw-r--r--db/schema.rb3
-rw-r--r--spec/models/user_spec.rb18
-rw-r--r--spec/presenters/projects/settings/deploy_keys_presenter_spec.rb8
11 files changed, 78 insertions, 48 deletions
diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb
index db501b4b506..0bd90bd28e3 100644
--- a/app/models/deploy_key.rb
+++ b/app/models/deploy_key.rb
@@ -2,12 +2,14 @@
class DeployKey < Key
include IgnorableColumn
+ include FromUnion
has_many :deploy_keys_projects, inverse_of: :deploy_key, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :projects, through: :deploy_keys_projects
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
@@ -22,7 +24,7 @@ class DeployKey < Key
end
def almost_orphaned?
- self.deploy_keys_projects.length == 1
+ self.deploy_keys_projects.count == 1
end
def destroyed_when_orphaned?
@@ -46,6 +48,6 @@ class DeployKey < Key
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
diff --git a/app/models/deploy_keys_project.rb b/app/models/deploy_keys_project.rb
index 15906ed8e06..40c66d5bc4c 100644
--- a/app/models/deploy_keys_project.rb
+++ b/app/models/deploy_keys_project.rb
@@ -1,9 +1,8 @@
# 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) }
diff --git a/app/models/project.rb b/app/models/project.rb
index 8f234fba04f..33f1077e982 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -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
diff --git a/app/models/user.rb b/app/models/user.rb
index b439d1c0c16..4630552e02e 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -933,7 +933,7 @@ class User < ApplicationRecord
end
def project_deploy_keys
- DeployKey.unscoped.in_projects(authorized_projects.pluck(:id)).distinct(:id)
+ DeployKey.in_projects(authorized_projects.select(:id)).distinct(:id)
end
def highest_role
@@ -941,11 +941,10 @@ class User < ApplicationRecord
end
def accessible_deploy_keys
- @accessible_deploy_keys ||= begin
- key_ids = project_deploy_keys.pluck(:id)
- key_ids.push(*DeployKey.are_public.pluck(:id))
- DeployKey.where(id: key_ids)
- end
+ DeployKey.from_union([
+ DeployKey.where(id: project_deploy_keys.select(:deploy_key_id)),
+ DeployKey.are_public
+ ])
end
def created_by
diff --git a/app/presenters/projects/settings/deploy_keys_presenter.rb b/app/presenters/projects/settings/deploy_keys_presenter.rb
index 85518c9a3a4..6f8c4e1f902 100644
--- a/app/presenters/projects/settings/deploy_keys_presenter.rb
+++ b/app/presenters/projects/settings/deploy_keys_presenter.rb
@@ -12,48 +12,38 @@ module Projects
@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)
}
diff --git a/app/serializers/deploy_key_entity.rb b/app/serializers/deploy_key_entity.rb
index 54bf030aba1..e47d6454780 100644
--- a/app/serializers/deploy_key_entity.rb
+++ b/app/serializers/deploy_key_entity.rb
@@ -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
diff --git a/changelogs/unreleased/43080-speed-up-deploy-keys.yml b/changelogs/unreleased/43080-speed-up-deploy-keys.yml
new file mode 100644
index 00000000000..73c9a9e5f82
--- /dev/null
+++ b/changelogs/unreleased/43080-speed-up-deploy-keys.yml
@@ -0,0 +1,5 @@
+---
+title: Speed up loading and filtering deploy keys and their projects
+merge_request: 31384
+author:
+type: performance
diff --git a/db/post_migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb b/db/post_migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb
new file mode 100644
index 00000000000..9b4d74b4bea
--- /dev/null
+++ b/db/post_migrate/20190802235445_add_index_on_id_and_type_and_public_to_keys.rb
@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+class AddIndexOnIdAndTypeAndPublicToKeys < ActiveRecord::Migration[5.2]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ INDEX_NAME = "index_on_deploy_keys_id_and_type_and_public"
+
+ def up
+ add_concurrent_index(:keys,
+ [:id, :type],
+ where: "public = 't'",
+ unique: true,
+ name: INDEX_NAME)
+ end
+
+ def down
+ remove_concurrent_index_by_name(:keys, INDEX_NAME)
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 90d93ddc736..8d5da4a07dd 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 2019_08_02_012622) do
+ActiveRecord::Schema.define(version: 2019_08_02_235445) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
@@ -1770,6 +1770,7 @@ ActiveRecord::Schema.define(version: 2019_08_02_012622) do
t.boolean "public", default: false, null: false
t.datetime "last_used_at"
t.index ["fingerprint"], name: "index_keys_on_fingerprint", unique: true
+ t.index ["id", "type"], name: "index_on_deploy_keys_id_and_type_and_public", unique: true, where: "(public = true)"
t.index ["user_id"], name: "index_keys_on_user_id"
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 35c335c5b5c..46b86e8393d 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -794,6 +794,24 @@ describe User do
end
end
+ describe '#accessible_deploy_keys' do
+ let(:user) { create(:user) }
+ let(:project) { create(:project) }
+ let!(:private_deploy_keys_project) { create(:deploy_keys_project) }
+ let!(:public_deploy_keys_project) { create(:deploy_keys_project) }
+ let!(:accessible_deploy_keys_project) { create(:deploy_keys_project, project: project) }
+
+ before do
+ public_deploy_keys_project.deploy_key.update(public: true)
+ project.add_developer(user)
+ end
+
+ it 'user can only see deploy keys accessible to right projects' do
+ expect(user.accessible_deploy_keys).to match_array([public_deploy_keys_project.deploy_key,
+ accessible_deploy_keys_project.deploy_key])
+ end
+ end
+
describe '#deploy_keys' do
include_context 'user keys'
diff --git a/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb b/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb
index 001545bb5df..b4bf39f3cdb 100644
--- a/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb
+++ b/spec/presenters/projects/settings/deploy_keys_presenter_spec.rb
@@ -29,10 +29,6 @@ describe Projects::Settings::DeployKeysPresenter do
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 @@ describe Projects::Settings::DeployKeysPresenter do
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
View Lorry Controller Status