From e1867c38fc5a4b931b4b2256d4909182e94f1051 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 6 Dec 2019 18:07:44 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- lib/api/entities.rb | 6 +- lib/api/groups.rb | 2 +- lib/api/helpers/pagination.rb | 27 +++++++- lib/api/pipelines.rb | 2 + lib/api/projects.rb | 34 +++++++--- lib/api/projects_batch_counting.rb | 27 ++++++++ lib/api/projects_relation_builder.rb | 36 ---------- lib/gitlab/auth/user_auth_finders.rb | 6 ++ lib/gitlab/pagination/keyset.rb | 21 ++++++ lib/gitlab/pagination/keyset/page.rb | 47 +++++++++++++ lib/gitlab/pagination/keyset/pager.rb | 56 ++++++++++++++++ lib/gitlab/pagination/keyset/request_context.rb | 89 +++++++++++++++++++++++++ lib/sentry/client.rb | 2 +- 13 files changed, 304 insertions(+), 51 deletions(-) create mode 100644 lib/api/projects_batch_counting.rb delete mode 100644 lib/api/projects_relation_builder.rb create mode 100644 lib/gitlab/pagination/keyset.rb create mode 100644 lib/gitlab/pagination/keyset/page.rb create mode 100644 lib/gitlab/pagination/keyset/pager.rb create mode 100644 lib/gitlab/pagination/keyset/request_context.rb (limited to 'lib') diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 1e41e28c449..ead2ea34227 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -176,7 +176,7 @@ module API end class BasicProjectDetails < ProjectIdentity - include ::API::ProjectsRelationBuilder + include ::API::ProjectsBatchCounting expose :default_branch, if: -> (project, options) { Ability.allowed?(options[:current_user], :download_code, project) } # Avoids an N+1 query: https://github.com/mbleigh/acts-as-taggable-on/issues/91#issuecomment-168273770 @@ -418,7 +418,7 @@ module API options: { only_owned: true } ).execute - Entities::Project.prepare_relation(projects) + Entities::Project.preload_and_batch_count!(projects) end expose :shared_projects, using: Entities::Project do |group, options| @@ -428,7 +428,7 @@ module API options: { only_shared: true } ).execute - Entities::Project.prepare_relation(projects) + Entities::Project.preload_and_batch_count!(projects) end end diff --git a/lib/api/groups.rb b/lib/api/groups.rb index 6c88b61eee8..b9cfc16fb23 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -231,7 +231,7 @@ module API projects, options = with_custom_attributes(projects, options) - present options[:with].prepare_relation(projects), options + present options[:with].preload_and_batch_count!(projects), options end desc 'Get a list of subgroups in this group.' do diff --git a/lib/api/helpers/pagination.rb b/lib/api/helpers/pagination.rb index 9c5b355e823..1b63e450a12 100644 --- a/lib/api/helpers/pagination.rb +++ b/lib/api/helpers/pagination.rb @@ -3,8 +3,33 @@ module API module Helpers module Pagination + # This returns an ActiveRecord relation def paginate(relation) - ::Gitlab::Pagination::OffsetPagination.new(self).paginate(relation) + Gitlab::Pagination::OffsetPagination.new(self).paginate(relation) + end + + # This applies pagination and executes the query + # It always returns an array instead of an ActiveRecord relation + def paginate_and_retrieve!(relation) + offset_or_keyset_pagination(relation).to_a + end + + private + + def offset_or_keyset_pagination(relation) + return paginate(relation) unless keyset_pagination_enabled? + + request_context = Gitlab::Pagination::Keyset::RequestContext.new(self) + + unless Gitlab::Pagination::Keyset.available?(request_context, relation) + return error!('Keyset pagination is not yet available for this type of request', 405) + end + + Gitlab::Pagination::Keyset.paginate(request_context, relation) + end + + def keyset_pagination_enabled? + params[:pagination] == 'keyset' && Feature.enabled?(:api_keyset_pagination, default_enabled: true) end end end diff --git a/lib/api/pipelines.rb b/lib/api/pipelines.rb index 7c87a9878bf..66a183173af 100644 --- a/lib/api/pipelines.rb +++ b/lib/api/pipelines.rb @@ -25,6 +25,8 @@ module API optional :yaml_errors, type: Boolean, desc: 'Returns pipelines with invalid configurations' optional :name, type: String, desc: 'The name of the user who triggered pipelines' optional :username, type: String, desc: 'The username of the user who triggered pipelines' + optional :updated_before, type: DateTime, desc: 'Return pipelines updated before the specified datetime. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ' + optional :updated_after, type: DateTime, desc: 'Return pipelines updated after the specified datetime. Format: ISO 8601 YYYY-MM-DDTHH:MM:SSZ' optional :order_by, type: String, values: PipelinesFinder::ALLOWED_INDEXED_COLUMNS, default: 'id', desc: 'Order pipelines' optional :sort, type: String, values: %w[asc desc], default: 'desc', diff --git a/lib/api/projects.rb b/lib/api/projects.rb index a1fce9e8b20..ea7087d2f46 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -75,15 +75,17 @@ module API mutually_exclusive :import_url, :template_name, :template_project_id end - def load_projects + def find_projects ProjectsFinder.new(current_user: current_user, params: project_finder_params).execute end - def present_projects(projects, options = {}) + # Prepare the full projects query + # None of this is supposed to actually execute any database query + def prepare_query(projects) projects = reorder_projects(projects) projects = apply_filters(projects) - projects = paginate(projects) - projects, options = with_custom_attributes(projects, options) + + projects, options = with_custom_attributes(projects) options = options.reverse_merge( with: current_user ? Entities::ProjectWithAccess : Entities::BasicProjectDetails, @@ -91,9 +93,23 @@ module API current_user: current_user, license: false ) + options[:with] = Entities::BasicProjectDetails if params[:simple] - present options[:with].prepare_relation(projects, options), options + projects = options[:with].preload_relation(projects, options) + + [projects, options] + end + + def prepare_and_present(project_relation) + projects, options = prepare_query(project_relation) + + projects = paginate_and_retrieve!(projects) + + # Refresh count caches + options[:with].execute_batch_counting(projects) + + present projects, options end def translate_params_for_compatibility(params) @@ -118,7 +134,7 @@ module API params[:user] = user - present_projects load_projects + prepare_and_present find_projects end desc 'Get projects starred by a user' do @@ -134,7 +150,7 @@ module API not_found!('User') unless user starred_projects = StarredProjectsFinder.new(user, params: project_finder_params, current_user: current_user).execute - present_projects starred_projects + prepare_and_present starred_projects end end @@ -150,7 +166,7 @@ module API use :with_custom_attributes end get do - present_projects load_projects + prepare_and_present find_projects end desc 'Create new project' do @@ -287,7 +303,7 @@ module API get ':id/forks' do forks = ForkProjectsFinder.new(user_project, params: project_finder_params, current_user: current_user).execute - present_projects forks + prepare_and_present forks end desc 'Check pages access of this project' diff --git a/lib/api/projects_batch_counting.rb b/lib/api/projects_batch_counting.rb new file mode 100644 index 00000000000..4e5124c8791 --- /dev/null +++ b/lib/api/projects_batch_counting.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module API + module ProjectsBatchCounting + extend ActiveSupport::Concern + + class_methods do + # This adds preloading to the query and executes batch counting + # Side-effect: The query will be executed during batch counting + def preload_and_batch_count!(projects_relation) + preload_relation(projects_relation).tap do |projects| + execute_batch_counting(projects) + end + end + + def execute_batch_counting(projects) + ::Projects::BatchForksCountService.new(forks_counting_projects(projects)).refresh_cache + + ::Projects::BatchOpenIssuesCountService.new(projects).refresh_cache + end + + def forks_counting_projects(projects) + projects + end + end + end +end diff --git a/lib/api/projects_relation_builder.rb b/lib/api/projects_relation_builder.rb deleted file mode 100644 index 263468c9aa6..00000000000 --- a/lib/api/projects_relation_builder.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -module API - module ProjectsRelationBuilder - extend ActiveSupport::Concern - - class_methods do - 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 diff --git a/lib/gitlab/auth/user_auth_finders.rb b/lib/gitlab/auth/user_auth_finders.rb index e2f562c0843..a8869f907e6 100644 --- a/lib/gitlab/auth/user_auth_finders.rb +++ b/lib/gitlab/auth/user_auth_finders.rb @@ -169,6 +169,8 @@ module Gitlab case request_format when :archive archive_request? + when :blob + blob_request? else false end @@ -189,6 +191,10 @@ module Gitlab def archive_request? current_request.path.include?('/-/archive/') end + + def blob_request? + current_request.path.include?('/raw/') + end end end end diff --git a/lib/gitlab/pagination/keyset.rb b/lib/gitlab/pagination/keyset.rb new file mode 100644 index 00000000000..5bd45fa9b56 --- /dev/null +++ b/lib/gitlab/pagination/keyset.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Gitlab + module Pagination + module Keyset + def self.paginate(request_context, relation) + Gitlab::Pagination::Keyset::Pager.new(request_context).paginate(relation) + end + + def self.available?(request_context, relation) + order_by = request_context.page.order_by + + # This is only available for Project and order-by id (asc/desc) + return false unless relation.klass == Project + return false unless order_by.size == 1 && order_by[:id] + + true + end + end + end +end diff --git a/lib/gitlab/pagination/keyset/page.rb b/lib/gitlab/pagination/keyset/page.rb new file mode 100644 index 00000000000..735f54faf0f --- /dev/null +++ b/lib/gitlab/pagination/keyset/page.rb @@ -0,0 +1,47 @@ +# frozen_string_literal: true + +module Gitlab + module Pagination + module Keyset + # A Page models the pagination information for a particular page of the collection + class Page + # Default number of records for a page + DEFAULT_PAGE_SIZE = 20 + + # Maximum number of records for a page + MAXIMUM_PAGE_SIZE = 100 + + attr_accessor :lower_bounds, :end_reached + attr_reader :order_by + + def initialize(order_by: {}, lower_bounds: nil, per_page: DEFAULT_PAGE_SIZE, end_reached: false) + @order_by = order_by.symbolize_keys + @lower_bounds = lower_bounds&.symbolize_keys + @per_page = per_page + @end_reached = end_reached + end + + # Number of records to return per page + def per_page + return DEFAULT_PAGE_SIZE if @per_page <= 0 + + [@per_page, MAXIMUM_PAGE_SIZE].min + end + + # Determine whether this page indicates the end of the collection + def end_reached? + @end_reached + end + + # Construct a Page for the next page + # Uses identical order_by/per_page information for the next page + def next(lower_bounds, end_reached) + dup.tap do |next_page| + next_page.lower_bounds = lower_bounds&.symbolize_keys + next_page.end_reached = end_reached + end + end + end + end + end +end diff --git a/lib/gitlab/pagination/keyset/pager.rb b/lib/gitlab/pagination/keyset/pager.rb new file mode 100644 index 00000000000..99b125cc2a0 --- /dev/null +++ b/lib/gitlab/pagination/keyset/pager.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module Gitlab + module Pagination + module Keyset + class Pager + attr_reader :request + + def initialize(request) + @request = request + end + + def paginate(relation) + # Validate assumption: The last two columns must match the page order_by + validate_order!(relation) + + # This performs the database query and retrieves records + # We retrieve one record more to check if we have data beyond this page + all_records = relation.limit(page.per_page + 1).to_a # rubocop: disable CodeReuse/ActiveRecord + + records_for_page = all_records.first(page.per_page) + + # If we retrieved more records than belong on this page, + # we know there's a next page + there_is_more = all_records.size > records_for_page.size + apply_headers(records_for_page.last, there_is_more) + + records_for_page + end + + private + + def apply_headers(last_record_in_page, there_is_more) + end_reached = last_record_in_page.nil? || !there_is_more + lower_bounds = last_record_in_page&.slice(page.order_by.keys) + + next_page = page.next(lower_bounds, end_reached) + + request.apply_headers(next_page) + end + + def page + @page ||= request.page + end + + def validate_order!(rel) + present_order = rel.order_values.map { |val| [val.expr.name.to_sym, val.direction] }.last(2).to_h + + unless page.order_by == present_order + raise ArgumentError, "Page's order_by does not match the relation's order: #{present_order} vs #{page.order_by}" + end + end + end + end + end +end diff --git a/lib/gitlab/pagination/keyset/request_context.rb b/lib/gitlab/pagination/keyset/request_context.rb new file mode 100644 index 00000000000..aeaed7587b3 --- /dev/null +++ b/lib/gitlab/pagination/keyset/request_context.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +module Gitlab + module Pagination + module Keyset + class RequestContext + attr_reader :request + + DEFAULT_SORT_DIRECTION = :desc + PRIMARY_KEY = :id + + # A tie breaker is added as an additional order-by column + # to establish a well-defined order. We use the primary key + # column here. + TIE_BREAKER = { PRIMARY_KEY => DEFAULT_SORT_DIRECTION }.freeze + + def initialize(request) + @request = request + end + + # extracts Paging information from request parameters + def page + @page ||= Page.new(order_by: order_by, per_page: params[:per_page]) + end + + def apply_headers(next_page) + request.header('Links', pagination_links(next_page)) + end + + private + + def order_by + return TIE_BREAKER.dup unless params[:order_by] + + order_by = { params[:order_by].to_sym => params[:sort]&.to_sym || DEFAULT_SORT_DIRECTION } + + # Order by an additional unique key, we use the primary key here + order_by = order_by.merge(TIE_BREAKER) unless order_by[PRIMARY_KEY] + + order_by + end + + def params + @params ||= request.params + end + + def lower_bounds_params(page) + page.lower_bounds.each_with_object({}) do |(column, value), params| + filter = filter_with_comparator(page, column) + params[filter] = value + end + end + + def filter_with_comparator(page, column) + direction = page.order_by[column] + + if direction&.to_sym == :desc + "#{column}_before" + else + "#{column}_after" + end + end + + def page_href(page) + base_request_uri.tap do |uri| + uri.query = query_params_for(page).to_query + end.to_s + end + + def pagination_links(next_page) + return if next_page.end_reached? + + %(<#{page_href(next_page)}>; rel="next") + end + + def base_request_uri + @base_request_uri ||= URI.parse(request.request.url).tap do |uri| + uri.host = Gitlab.config.gitlab.host + uri.port = Gitlab.config.gitlab.port + end + end + + def query_params_for(page) + request.params.merge(lower_bounds_params(page)) + end + end + end + end +end diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb index 708ace53f5b..450695aa545 100644 --- a/lib/sentry/client.rb +++ b/lib/sentry/client.rb @@ -206,7 +206,7 @@ module Sentry uri = URI(url) uri.path.squeeze!('/') - # Remove trailing spaces + # Remove trailing slash uri = uri.to_s.gsub(/\/\z/, '') uri -- cgit v1.2.1