diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2017-10-04 23:26:51 +0300 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2017-10-05 12:10:57 +0300 |
commit | dda023d66d09b8a3a43a5599bde42ac52eb6fd06 (patch) | |
tree | aa585d3884c5f577bd605e21c1554d02bf5f1ca5 /app/finders | |
parent | 57bd3bb34a19bf812fd6a74f394a69c491b05dd0 (diff) |
Optimize queries and pagination in `GroupDescendantsFinder`
Diffstat (limited to 'app/finders')
-rw-r--r-- | app/finders/group_descendants_finder.rb | 93 |
1 files changed, 32 insertions, 61 deletions
diff --git a/app/finders/group_descendants_finder.rb b/app/finders/group_descendants_finder.rb index bde93da3a3e..4e7ae6f499b 100644 --- a/app/finders/group_descendants_finder.rb +++ b/app/finders/group_descendants_finder.rb @@ -36,10 +36,13 @@ class GroupDescendantsFinder def execute # The children array might be extended with the ancestors of projects when # filtering. In that case, take the maximum so the aray does not get limited - # Otherwise, allow paginating through the search results + # Otherwise, allow paginating through all results # - total_count = [children.size, subgroup_count + project_count].max - Kaminari.paginate_array(children, total_count: total_count) + all_required_elements = children + all_required_elements |= ancestors_for_projects if params[:filter] + total_count = [all_required_elements.size, paginator.total_count].max + + Kaminari.paginate_array(all_required_elements, total_count: total_count) end def subgroup_count @@ -53,50 +56,15 @@ class GroupDescendantsFinder private def children - return @children if @children - - subgroups_with_counts = subgroups.with_route - .page(params[:page]).per(per_page) - .select(GROUP_SELECTS) - - paginated_projects = paginate_projects_after_groups(subgroups_with_counts) - - subgroups_with_counts = add_project_ancestors_when_searching(subgroups_with_counts, paginated_projects) - - @children = subgroups_with_counts + paginated_projects + @children ||= paginator.paginate(params[:page]) end - def add_project_ancestors_when_searching(groups, projects) - return groups unless params[:filter] - - project_ancestors = ancestors_for_projects(projects) - .with_route.select(GROUP_SELECTS) - groups | project_ancestors + def collections + [subgroups.with_route.select(GROUP_SELECTS), projects] end - def paginate_projects_after_groups(loaded_subgroups) - # We adjust the pagination for projects for the combination with groups: - # - We limit the first page (page 0) where we show projects: - # Page size = 20: 17 groups, 3 projects - # - We ofset the page to start at 0 after the group pages: - # 3 pages of projects: - # - currently on page 3: Show page 0 (first page) limited to the number of - # projects that still fit the page (no offset) - # - currently on page 4: Show page 1 show all projects, offset by the number - # of projects shown on project-page 0. - group_page_count = loaded_subgroups.total_pages - subgroup_page = loaded_subgroups.current_page - group_last_page_count = subgroups.page(group_page_count).count - project_page = subgroup_page - group_page_count - offset = if project_page.zero? || group_page_count.zero? - 0 - else - per_page - group_last_page_count - end - - projects.with_route.page(project_page) - .per(per_page - loaded_subgroups.size) - .padding(offset) + def paginator + Gitlab::MultiCollectionPaginator.new(*collections, per_page: params[:per_page]) end def direct_child_groups @@ -107,22 +75,22 @@ class GroupDescendantsFinder def all_visible_descendant_groups groups_table = Group.arel_table + visible_for_user = groups_table[:visibility_level] + .in(Gitlab::VisibilityLevel.levels_for_user(current_user)) visible_for_user = if current_user - groups_table[:id].in( - Arel::Nodes::SqlLiteral.new(GroupsFinder.new(current_user, all_available: true).execute.select(:id).to_sql) - ) - else - groups_table[:visibility_level].eq(Gitlab::VisibilityLevel::PUBLIC) + visible_projects = GroupsFinder.new(current_user).execute.as('visible') + authorized = groups_table.project(1).from(visible_projects) + .where(visible_projects[:id].eq(groups_table[:id])) + .exists + visible_for_user.or(authorized) end - - Gitlab::GroupHierarchy.new(Group.where(id: parent_group)) - .base_and_descendants + hierarchy_for_parent + .descendants .where(visible_for_user) end def subgroups_matching_filter all_visible_descendant_groups - .where.not(id: parent_group) .search(params[:filter]) end @@ -137,14 +105,15 @@ class GroupDescendantsFinder # So when searching 'project', on the 'subgroup' page we want to preload # 'nested-group' but not 'subgroup' or 'root' def ancestors_for_groups(base_for_ancestors) - ancestors_for_parent = Gitlab::GroupHierarchy.new(Group.where(id: parent_group)) - .base_and_ancestors Gitlab::GroupHierarchy.new(base_for_ancestors) - .base_and_ancestors.where.not(id: ancestors_for_parent) + .base_and_ancestors(upto: parent_group.id) end - def ancestors_for_projects(projects) - ancestors_for_groups(Group.where(id: projects.select(:namespace_id))) + def ancestors_for_projects + projects_to_load_ancestors_of = projects.where.not(namespace: parent_group) + groups_to_load_ancestors_of = Group.where(id: projects_to_load_ancestors_of.select(:namespace_id)) + ancestors_for_groups(groups_to_load_ancestors_of) + .with_route.select(GROUP_SELECTS) end def subgroups @@ -169,9 +138,11 @@ class GroupDescendantsFinder projects_for_user.where(namespace: parent_group) end + # Finds all projects nested under `parent_group` or any of it's descendant + # groups def projects_matching_filter projects_for_user.search(params[:filter]) - .where(namespace: all_visible_descendant_groups) + .where(namespace_id: hierarchy_for_parent.base_and_descendants.select(:id)) end def projects @@ -182,14 +153,14 @@ class GroupDescendantsFinder else direct_child_projects end - projects.order_by(sort) + projects.with_route.order_by(sort) end def sort params.fetch(:sort, 'id_asc') end - def per_page - params.fetch(:per_page, Kaminari.config.default_per_page) + def hierarchy_for_parent + @hierarchy ||= Gitlab::GroupHierarchy.new(Group.where(id: parent_group.id)) end end |