Commit 327b64b7 authored by Luke Duncalfe's avatar Luke Duncalfe Committed by Jan Provaznik

Add concern to add preloading to group resolvers

parent 1b3a4e6e
# frozen_string_literal: true
# Mixin for all resolver classes for type `Types::GroupType.connection_type`.
module ResolvesGroups
extend ActiveSupport::Concern
include LooksAhead
def resolve_with_lookahead(**args)
apply_lookahead(resolve_groups(**args))
end
private
# The resolver should implement this method.
def resolve_groups(**args)
raise NotImplementedError
end
def preloads
{
contacts: [:contacts],
container_repositories_count: [:container_repositories],
custom_emoji: [:custom_emoji],
full_path: [:route],
organizations: [:organizations],
path: [:route],
dependency_proxy_blob_count: [:dependency_proxy_blobs],
dependency_proxy_blobs: [:dependency_proxy_blobs],
dependency_proxy_image_count: [:dependency_proxy_manifests],
dependency_proxy_image_ttl_policy: [:dependency_proxy_image_ttl_policy],
dependency_proxy_setting: [:dependency_proxy_setting]
}
end
end
...@@ -2,6 +2,8 @@ ...@@ -2,6 +2,8 @@
module Resolvers module Resolvers
class GroupsResolver < BaseResolver class GroupsResolver < BaseResolver
include ResolvesGroups
type Types::GroupType, null: true type Types::GroupType, null: true
argument :include_parent_descendants, GraphQL::Types::Boolean, argument :include_parent_descendants, GraphQL::Types::Boolean,
...@@ -19,16 +21,12 @@ module Resolvers ...@@ -19,16 +21,12 @@ module Resolvers
alias_method :parent, :object alias_method :parent, :object
def resolve(**args)
return [] unless parent.present?
find_groups(args)
end
private private
# rubocop: disable CodeReuse/ActiveRecord # rubocop: disable CodeReuse/ActiveRecord
def find_groups(args) def resolve_groups(args)
return Group.none unless parent.present?
GroupsFinder GroupsFinder
.new(context[:current_user], args.merge(parent: parent)) .new(context[:current_user], args.merge(parent: parent))
.execute .execute
......
...@@ -3,8 +3,8 @@ ...@@ -3,8 +3,8 @@
module Resolvers module Resolvers
module Users module Users
class GroupsResolver < BaseResolver class GroupsResolver < BaseResolver
include ResolvesGroups
include Gitlab::Graphql::Authorize::AuthorizeResource include Gitlab::Graphql::Authorize::AuthorizeResource
include LooksAhead
type Types::GroupType.connection_type, null: true type Types::GroupType.connection_type, null: true
...@@ -23,19 +23,14 @@ module Resolvers ...@@ -23,19 +23,14 @@ module Resolvers
Preloaders::UserMaxAccessLevelInGroupsPreloader.new(nodes, current_user).execute Preloaders::UserMaxAccessLevelInGroupsPreloader.new(nodes, current_user).execute
end end
def resolve_with_lookahead(**args) def ready?(**args)
return unless Feature.enabled?(:paginatable_namespace_drop_down_for_project_creation, current_user, default_enabled: :yaml) Feature.enabled?(:paginatable_namespace_drop_down_for_project_creation, current_user, default_enabled: :yaml)
apply_lookahead(Groups::UserGroupsFinder.new(current_user, object, args).execute)
end end
private private
def preloads def resolve_groups(**args)
{ Groups::UserGroupsFinder.new(current_user, object, args).execute
path: [:route],
full_path: [:route]
}
end end
end end
end end
......
...@@ -34,6 +34,7 @@ module Types ...@@ -34,6 +34,7 @@ module Types
null: true, null: true,
method: :project_creation_level_str, method: :project_creation_level_str,
description: 'Permission level required to create projects in the group.' description: 'Permission level required to create projects in the group.'
field :subgroup_creation_level, field :subgroup_creation_level,
type: GraphQL::Types::String, type: GraphQL::Types::String,
null: true, null: true,
...@@ -44,6 +45,7 @@ module Types ...@@ -44,6 +45,7 @@ module Types
type: GraphQL::Types::Boolean, type: GraphQL::Types::Boolean,
null: true, null: true,
description: 'Indicates if all users in this group are required to set up two-factor authentication.' description: 'Indicates if all users in this group are required to set up two-factor authentication.'
field :two_factor_grace_period, field :two_factor_grace_period,
type: GraphQL::Types::Int, type: GraphQL::Types::Int,
null: true, null: true,
...@@ -225,11 +227,11 @@ module Types ...@@ -225,11 +227,11 @@ module Types
end end
def dependency_proxy_image_count def dependency_proxy_image_count
group.dependency_proxy_manifests.count group.dependency_proxy_manifests.size
end end
def dependency_proxy_blob_count def dependency_proxy_blob_count
group.dependency_proxy_blobs.count group.dependency_proxy_blobs.size
end end
def dependency_proxy_total_size def dependency_proxy_total_size
......
...@@ -56,6 +56,9 @@ class Group < Namespace ...@@ -56,6 +56,9 @@ class Group < Namespace
has_many :boards has_many :boards
has_many :badges, class_name: 'GroupBadge' has_many :badges, class_name: 'GroupBadge'
has_many :organizations, class_name: 'CustomerRelations::Organization', inverse_of: :group
has_many :contacts, class_name: 'CustomerRelations::Contact', inverse_of: :group
has_many :cluster_groups, class_name: 'Clusters::Group' has_many :cluster_groups, class_name: 'Clusters::Group'
has_many :clusters, through: :cluster_groups, class_name: 'Clusters::Cluster' has_many :clusters, through: :cluster_groups, class_name: 'Clusters::Cluster'
...@@ -757,14 +760,6 @@ class Group < Namespace ...@@ -757,14 +760,6 @@ class Group < Namespace
Timelog.in_group(self) Timelog.in_group(self)
end end
def organizations
::CustomerRelations::Organization.where(group_id: self.id)
end
def contacts
::CustomerRelations::Contact.where(group_id: self.id)
end
def dependency_proxy_image_ttl_policy def dependency_proxy_image_ttl_policy
super || build_dependency_proxy_image_ttl_policy super || build_dependency_proxy_image_ttl_policy
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ResolvesGroups do
include GraphqlHelpers
include AfterNextHelpers
let_it_be(:user) { create(:user) }
let_it_be(:groups) { create_pair(:group) }
let_it_be(:resolver) do
Class.new(Resolvers::BaseResolver) do
include ResolvesGroups
type Types::GroupType, null: true
end
end
let_it_be(:query_type) do
query_factory do |query|
query.field :groups,
Types::GroupType.connection_type,
null: true,
resolver: resolver
end
end
let_it_be(:lookahead_fields) do
<<~FIELDS
contacts { nodes { id } }
containerRepositoriesCount
customEmoji { nodes { id } }
fullPath
organizations { nodes { id } }
path
dependencyProxyBlobCount
dependencyProxyBlobs { nodes { fileName } }
dependencyProxyImageCount
dependencyProxyImageTtlPolicy { enabled }
dependencyProxySetting { enabled }
FIELDS
end
it 'avoids N+1 queries on the fields marked with lookahead' do
group_ids = groups.map(&:id)
allow_next(resolver).to receive(:resolve_groups).and_return(Group.id_in(group_ids))
# Prevent authorization queries from affecting the test.
allow(Ability).to receive(:allowed?).and_return(true)
single_group_query = ActiveRecord::QueryRecorder.new do
data = query_groups(limit: 1)
expect(data.size).to eq(1)
end
multi_group_query = -> {
data = query_groups(limit: 2)
expect(data.size).to eq(2)
}
expect { multi_group_query.call }.not_to exceed_query_limit(single_group_query)
end
def query_groups(limit:)
query_string = "{ groups(first: #{limit}) { nodes { id #{lookahead_fields} } } }"
data = execute_query(query_type, graphql: query_string)
graphql_dig_at(data, :data, :groups, :nodes)
end
end
...@@ -37,6 +37,8 @@ RSpec.describe Group do ...@@ -37,6 +37,8 @@ RSpec.describe Group do
it { is_expected.to have_many(:daily_build_group_report_results).class_name('Ci::DailyBuildGroupReportResult') } it { is_expected.to have_many(:daily_build_group_report_results).class_name('Ci::DailyBuildGroupReportResult') }
it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout').with_foreign_key(:group_id) } it { is_expected.to have_many(:group_callouts).class_name('Users::GroupCallout').with_foreign_key(:group_id) }
it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') } it { is_expected.to have_many(:bulk_import_exports).class_name('BulkImports::Export') }
it { is_expected.to have_many(:contacts).class_name('CustomerRelations::Contact') }
it { is_expected.to have_many(:organizations).class_name('CustomerRelations::Organization') }
describe '#members & #requesters' do describe '#members & #requesters' do
let(:requester) { create(:user) } let(:requester) { create(:user) }
......
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