diff options
author | Douwe Maan <douwe@gitlab.com> | 2017-12-04 15:45:43 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2017-12-04 15:45:43 +0000 |
commit | 65b7a7a0632280c77de1dc9d6c93dfb5df0c2fc9 (patch) | |
tree | b86d2e81c4d550d89906654d5bd378b93d5654d0 /lib/api | |
parent | 6a27f9b1b234470a994b0b43460bd9c8097ae12f (diff) | |
parent | 264171f72d4ef3e5dfafaf32d3d267ab279469e1 (diff) | |
download | gitlab-ce-65b7a7a0632280c77de1dc9d6c93dfb5df0c2fc9.tar.gz |
Merge branch 'sh-optimize-groups-api' into 'master'
Optimize API /groups/:id/projects by preloading assocations
Closes #40308
See merge request gitlab-org/gitlab-ce!15475
Diffstat (limited to 'lib/api')
-rw-r--r-- | lib/api/entities.rb | 56 | ||||
-rw-r--r-- | lib/api/groups.rb | 14 | ||||
-rw-r--r-- | lib/api/projects.rb | 8 | ||||
-rw-r--r-- | lib/api/projects_relation_builder.rb | 34 |
4 files changed, 98 insertions, 14 deletions
diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ce332fe85d2..7cec8da013d 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -88,13 +88,29 @@ module API end class BasicProjectDetails < ProjectIdentity - expose :default_branch, :tag_list + include ::API::ProjectsRelationBuilder + + expose :default_branch + # Avoids an N+1 query: https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770 + expose :tag_list do |project| + # project.tags.order(:name).pluck(:name) is the most suitable option + # to avoid loading all the ActiveRecord objects but, if we use it here + # it override the preloaded associations and makes a query + # (fixed in https://github.com/rails/rails/pull/25976). + project.tags.map(&:name).sort + end expose :ssh_url_to_repo, :http_url_to_repo, :web_url expose :avatar_url do |project, options| project.avatar_url(only_path: false) end expose :star_count, :forks_count expose :last_activity_at + + def self.preload_relation(projects_relation, options = {}) + projects_relation.preload(:project_feature, :route) + .preload(namespace: [:route, :owner], + tags: :taggings) + end end class Project < BasicProjectDetails @@ -146,7 +162,7 @@ module API expose :shared_runners_enabled expose :lfs_enabled?, as: :lfs_enabled expose :creator_id - expose :namespace, using: 'API::Entities::Namespace' + expose :namespace, using: 'API::Entities::NamespaceBasic' expose :forked_from_project, using: Entities::BasicProjectDetails, if: lambda { |project, options| project.forked? } expose :import_status expose :import_error, if: lambda { |_project, options| options[:user_can_admin_project] } @@ -156,7 +172,7 @@ module API expose :public_builds, as: :public_jobs expose :ci_config_path expose :shared_with_groups do |project, options| - SharedGroup.represent(project.project_group_links.all, options) + SharedGroup.represent(project.project_group_links, options) end expose :only_allow_merge_if_pipeline_succeeds expose :request_access_enabled @@ -164,6 +180,18 @@ module API expose :printing_merge_request_link_enabled expose :statistics, using: 'API::Entities::ProjectStatistics', if: :statistics + + def self.preload_relation(projects_relation, options = {}) + super(projects_relation).preload(:group) + .preload(project_group_links: :group, + fork_network: :root_project, + forked_project_link: :forked_from_project, + forked_from_project: [:route, :forks, namespace: :route, tags: :taggings]) + end + + def self.forks_counting_projects(projects_relation) + projects_relation + projects_relation.map(&:forked_from_project).compact + end end class ProjectStatistics < Grape::Entity @@ -618,9 +646,11 @@ module API expose :created_at end - class Namespace < Grape::Entity + class NamespaceBasic < Grape::Entity expose :id, :name, :path, :kind, :full_path, :parent_id + end + class Namespace < NamespaceBasic expose :members_count_with_descendants, if: -> (namespace, opts) { expose_members_count_with_descendants?(namespace, opts) } do |namespace, _| namespace.users_with_descendants.count end @@ -680,7 +710,7 @@ module API if options.key?(:project_members) (options[:project_members] || []).find { |member| member.source_id == project.id } else - project.project_members.find_by(user_id: options[:current_user].id) + project.project_member(options[:current_user]) end end @@ -689,11 +719,25 @@ module API if options.key?(:group_members) (options[:group_members] || []).find { |member| member.source_id == project.namespace_id } else - project.group.group_members.find_by(user_id: options[:current_user].id) + project.group.group_member(options[:current_user]) end end end end + + def self.preload_relation(projects_relation, options = {}) + relation = super(projects_relation, options) + + unless options.key?(:group_members) + relation = relation.preload(group: [group_members: [:source, user: [notification_settings: :source]]]) + end + + unless options.key?(:project_members) + relation = relation.preload(project_members: [:source, user: [notification_settings: :source]]) + end + + relation + end end class LabelBasic < Grape::Entity diff --git a/lib/api/groups.rb b/lib/api/groups.rb index bcf2e6dae1d..b81f07a1770 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -52,6 +52,13 @@ module API groups end + def find_group_projects(params) + group = find_group!(params[:id]) + projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute + projects = reorder_projects(projects) + paginate(projects) + end + def present_groups(params, groups) options = { with: Entities::Group, @@ -170,11 +177,10 @@ module API use :pagination end get ":id/projects" do - group = find_group!(params[:id]) - projects = GroupProjectsFinder.new(group: group, current_user: current_user, params: project_finder_params).execute - projects = reorder_projects(projects) + projects = find_group_projects(params) entity = params[:simple] ? Entities::BasicProjectDetails : Entities::Project - present paginate(projects), with: entity, current_user: current_user + + present entity.prepare_relation(projects), with: entity, current_user: current_user end desc 'Get a list of subgroups in this group.' do diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 4cd7e714aa2..14a4fc6f025 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -79,11 +79,11 @@ module API projects = projects.with_statistics if params[:statistics] projects = projects.with_issues_enabled if params[:with_issues_enabled] projects = projects.with_merge_requests_enabled if params[:with_merge_requests_enabled] + projects = paginate(projects) if current_user - projects = projects.includes(:route, :taggings, namespace: :route) - project_members = current_user.project_members - group_members = current_user.group_members + project_members = current_user.project_members.preload(:source, user: [notification_settings: :source]) + group_members = current_user.group_members.preload(:source, user: [notification_settings: :source]) end options = options.reverse_merge( @@ -95,7 +95,7 @@ module API ) options[:with] = Entities::BasicProjectDetails if params[:simple] - present paginate(projects), options + present options[:with].prepare_relation(projects, options), options end end diff --git a/lib/api/projects_relation_builder.rb b/lib/api/projects_relation_builder.rb new file mode 100644 index 00000000000..6482fd94ab8 --- /dev/null +++ b/lib/api/projects_relation_builder.rb @@ -0,0 +1,34 @@ +module API + module ProjectsRelationBuilder + extend ActiveSupport::Concern + + module ClassMethods + def prepare_relation(projects_relation, options = {}) + projects_relation = preload_relation(projects_relation, options) + execute_batch_counting(projects_relation) + projects_relation + end + + def preload_relation(projects_relation, options = {}) + projects_relation + end + + def forks_counting_projects(projects_relation) + projects_relation + end + + def batch_forks_counting(projects_relation) + ::Projects::BatchForksCountService.new(forks_counting_projects(projects_relation)).refresh_cache + end + + def batch_open_issues_counting(projects_relation) + ::Projects::BatchOpenIssuesCountService.new(projects_relation).refresh_cache + end + + def execute_batch_counting(projects_relation) + batch_forks_counting(projects_relation) + batch_open_issues_counting(projects_relation) + end + end + end +end |