Commit d873214e authored by Tiger Watson's avatar Tiger Watson

Merge branch...

Merge branch '235912-fork-interface-with-prevent-forking-outside-a-group-is-extremely-slow' into 'master'

Fix slow group loading on forking page

Closes #235912

See merge request gitlab-org/gitlab!39640
parents c3b2acb1 da7d73e7
...@@ -43,9 +43,10 @@ class Projects::ForksController < Projects::ApplicationController ...@@ -43,9 +43,10 @@ class Projects::ForksController < Projects::ApplicationController
end end
format.json do format.json do
namespaces = fork_service.valid_fork_targets - [current_user.namespace, project.namespace] namespaces = load_namespaces_with_associations - [project.namespace]
render json: { render json: {
namespaces: ForkNamespaceSerializer.new.represent(namespaces, project: project, current_user: current_user) namespaces: ForkNamespaceSerializer.new.represent(namespaces, project: project, current_user: current_user, memberships: memberships_hash)
} }
end end
end end
...@@ -100,6 +101,14 @@ class Projects::ForksController < Projects::ApplicationController ...@@ -100,6 +101,14 @@ class Projects::ForksController < Projects::ApplicationController
def whitelist_query_limiting def whitelist_query_limiting
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42335') Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-foss/issues/42335')
end end
def load_namespaces_with_associations
@load_namespaces_with_associations ||= fork_service.valid_fork_targets(only_groups: true).preload(:route)
end
def memberships_hash
current_user.members.where(source: load_namespaces_with_associations).index_by(&:source_id)
end
end end
Projects::ForksController.prepend_if_ee('EE::Projects::ForksController') Projects::ForksController.prepend_if_ee('EE::Projects::ForksController')
...@@ -7,8 +7,10 @@ class ForkTargetsFinder ...@@ -7,8 +7,10 @@ class ForkTargetsFinder
end end
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def execute def execute(options = {})
::Namespace.where(id: user.manageable_namespaces).sort_by_type return ::Namespace.where(id: user.manageable_namespaces).sort_by_type unless options[:only_groups]
::Group.where(id: user.manageable_groups)
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -19,7 +19,7 @@ class ForkNamespaceEntity < Grape::Entity ...@@ -19,7 +19,7 @@ class ForkNamespaceEntity < Grape::Entity
end end
expose :permission do |namespace, options| expose :permission do |namespace, options|
membership(options[:current_user], namespace)&.human_access membership(options[:current_user], namespace, options[:memberships])&.human_access
end end
expose :relative_path do |namespace| expose :relative_path do |namespace|
...@@ -37,10 +37,10 @@ class ForkNamespaceEntity < Grape::Entity ...@@ -37,10 +37,10 @@ class ForkNamespaceEntity < Grape::Entity
private private
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def membership(user, object) def membership(user, object, memberships)
return unless user return unless user
@membership ||= user.members.find_by(source: object) memberships[object.id]
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -10,8 +10,8 @@ module Projects ...@@ -10,8 +10,8 @@ module Projects
forked_project forked_project
end end
def valid_fork_targets def valid_fork_targets(options = {})
@valid_fork_targets ||= ForkTargetsFinder.new(@project, current_user).execute @valid_fork_targets ||= ForkTargetsFinder.new(@project, current_user).execute(options)
end end
def valid_fork_target?(namespace = target_namespace) def valid_fork_target?(namespace = target_namespace)
......
---
title: Fix slow group loading on forking page
merge_request: 39640
author:
type: performance
...@@ -11,6 +11,11 @@ module EE ...@@ -11,6 +11,11 @@ module EE
def load_forks def load_forks
super.with_compliance_framework_settings super.with_compliance_framework_settings
end end
override :load_namespaces_with_associations
def load_namespaces_with_associations
super.with_deletion_schedule_only
end
end end
end end
end end
...@@ -7,7 +7,7 @@ module EE ...@@ -7,7 +7,7 @@ module EE
override :execute override :execute
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def execute def execute(options = {})
targets = super targets = super
root_group = project.group&.root_ancestor root_group = project.group&.root_ancestor
......
...@@ -69,6 +69,7 @@ module EE ...@@ -69,6 +69,7 @@ module EE
scope :aimed_for_deletion, -> (date) { joins(:deletion_schedule).where('group_deletion_schedules.marked_for_deletion_on <= ?', date) } scope :aimed_for_deletion, -> (date) { joins(:deletion_schedule).where('group_deletion_schedules.marked_for_deletion_on <= ?', date) }
scope :with_deletion_schedule, -> { preload(deletion_schedule: :deleting_user) } scope :with_deletion_schedule, -> { preload(deletion_schedule: :deleting_user) }
scope :with_deletion_schedule_only, -> { preload(:deletion_schedule) }
scope :where_group_links_with_provider, ->(provider) do scope :where_group_links_with_provider, ->(provider) do
joins(:ldap_group_links).where(ldap_group_links: { provider: provider }) joins(:ldap_group_links).where(ldap_group_links: { provider: provider })
......
...@@ -7,7 +7,11 @@ RSpec.describe ForkNamespaceEntity do ...@@ -7,7 +7,11 @@ RSpec.describe ForkNamespaceEntity do
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let(:namespace) { create(:group_with_deletion_schedule, :with_avatar, description: 'test', marked_for_deletion_on: 1.day.ago) } let(:namespace) { create(:group_with_deletion_schedule, :with_avatar, description: 'test', marked_for_deletion_on: 1.day.ago) }
let(:entity) { described_class.new(namespace, current_user: user, project: project) } let(:memberships) do
user.members.index_by(&:source_id)
end
let(:entity) { described_class.new(namespace, current_user: user, project: project, memberships: memberships) }
subject(:json) { entity.as_json } subject(:json) { entity.as_json }
......
...@@ -35,5 +35,13 @@ RSpec.describe ForkTargetsFinder do ...@@ -35,5 +35,13 @@ RSpec.describe ForkTargetsFinder do
it 'returns all user manageable namespaces' do it 'returns all user manageable namespaces' do
expect(finder.execute).to match_array([user.namespace, maintained_group, owned_group, project.namespace]) expect(finder.execute).to match_array([user.namespace, maintained_group, owned_group, project.namespace])
end end
it 'returns only groups when only_groups option is passed' do
expect(finder.execute(only_groups: true)).to match_array([maintained_group, owned_group, project.namespace])
end
it 'returns groups relation when only_groups option is passed' do
expect(finder.execute(only_groups: true)).to include(a_kind_of(Group))
end
end end
end end
...@@ -8,13 +8,17 @@ RSpec.describe ForkNamespaceEntity do ...@@ -8,13 +8,17 @@ RSpec.describe ForkNamespaceEntity do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project) } let_it_be(:project) { create(:project) }
let_it_be(:namespace) { create(:group, :with_avatar, description: 'test') }
let(:memberships) do
user.members.index_by(&:source_id)
end
let(:namespace) { create(:group, :with_avatar, description: 'test') } let(:entity) { described_class.new(namespace, current_user: user, project: project, memberships: memberships) }
let(:entity) { described_class.new(namespace, current_user: user, project: project) }
subject(:json) { entity.as_json } subject(:json) { entity.as_json }
before do before do
namespace.add_developer(user)
project.add_maintainer(user) project.add_maintainer(user)
end end
...@@ -52,7 +56,6 @@ RSpec.describe ForkNamespaceEntity do ...@@ -52,7 +56,6 @@ RSpec.describe ForkNamespaceEntity do
end end
it 'exposes human readable permission level' do it 'exposes human readable permission level' do
namespace.add_developer(user)
expect(json[:permission]).to eql 'Developer' expect(json[:permission]).to eql 'Developer'
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