diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-05-06 09:10:11 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-05-06 09:10:11 +0300 |
commit | 0a0e8803b0e3e2fb83d74c9bafc32f4e9d825bcc (patch) | |
tree | 12ca59ec66c2ec7d405101869a7384d2034d308d /app/finders | |
parent | 806b829e76120085d80759dabc110f0328cfb7ac (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'app/finders')
-rw-r--r-- | app/finders/deployments_finder.rb | 95 |
1 files changed, 89 insertions, 6 deletions
diff --git a/app/finders/deployments_finder.rb b/app/finders/deployments_finder.rb index 8fec7932926..a54408e7216 100644 --- a/app/finders/deployments_finder.rb +++ b/app/finders/deployments_finder.rb @@ -16,14 +16,25 @@ class DeploymentsFinder attr_reader :params + # Warning: + # These const are directly used in Deployment Rest API, thus + # modifying these values could implicity change the API interface or introduce a breaking change. + # Also, if you add a sort value, make sure that the new query will stay + # performant with the other filtering/sorting parameters. + # The composed query could be significantly slower when the filtering and sorting columns are different. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/325627 for example. ALLOWED_SORT_VALUES = %w[id iid created_at updated_at ref finished_at].freeze DEFAULT_SORT_VALUE = 'id' ALLOWED_SORT_DIRECTIONS = %w[asc desc].freeze DEFAULT_SORT_DIRECTION = 'asc' + InefficientQueryError = Class.new(StandardError) + def initialize(params = {}) @params = params + + validate! end def execute @@ -38,6 +49,32 @@ class DeploymentsFinder private + def validate! + if filter_by_updated_at? && filter_by_finished_at? + raise InefficientQueryError, 'Both `updated_at` filter and `finished_at` filter can not be specified' + end + + # Currently, the inefficient parameters are allowed in order to avoid breaking changes in Deployment API. + # We'll switch to a hard error in https://gitlab.com/gitlab-org/gitlab/-/issues/328500. + if (filter_by_updated_at? && !order_by_updated_at?) || (!filter_by_updated_at? && order_by_updated_at?) + Gitlab::ErrorTracking.track_and_raise_for_dev_exception( + InefficientQueryError.new('`updated_at` filter and `updated_at` sorting must be paired') + ) + end + + if (filter_by_finished_at? && !order_by_finished_at?) || (!filter_by_finished_at? && order_by_finished_at?) + raise InefficientQueryError, '`finished_at` filter and `finished_at` sorting must be paired' + end + + if filter_by_finished_at? && !filter_by_successful_deployment? + raise InefficientQueryError, '`finished_at` filter must be combined with `success` status filter.' + end + + if params[:environment].present? && !params[:project].present? + raise InefficientQueryError, '`environment` filter must be combined with `project` scope.' + end + end + def init_collection if params[:project].present? params[:project].deployments @@ -49,6 +86,8 @@ class DeploymentsFinder end def sort(items) + sort_params = build_sort_params + optimize_sort_params!(sort_params) items.order(sort_params) # rubocop: disable CodeReuse/ActiveRecord end @@ -67,8 +106,8 @@ class DeploymentsFinder end def by_environment(items) - if params[:environment].present? - items.for_environment_name(params[:environment]) + if params[:project].present? && params[:environment].present? + items.for_environment_name(params[:project], params[:environment]) else items end @@ -84,14 +123,58 @@ class DeploymentsFinder items.for_status(params[:status]) end - def sort_params + def build_sort_params order_by = ALLOWED_SORT_VALUES.include?(params[:order_by]) ? params[:order_by] : DEFAULT_SORT_VALUE order_direction = ALLOWED_SORT_DIRECTIONS.include?(params[:sort]) ? params[:sort] : DEFAULT_SORT_DIRECTION - { order_by => order_direction }.tap do |sort_values| - sort_values['id'] = 'desc' if sort_values['updated_at'] - sort_values['id'] = sort_values.delete('created_at') if sort_values['created_at'] # Sorting by `id` produces the same result as sorting by `created_at` + { order_by => order_direction } + end + + def optimize_sort_params!(sort_params) + # Implicitly enforce the ordering when filtered by `updated_at` column for performance optimization. + # See https://gitlab.com/gitlab-org/gitlab/-/issues/325627#note_552417509. + # We remove this in https://gitlab.com/gitlab-org/gitlab/-/issues/328500. + if filter_by_updated_at? && implicitly_enforce_ordering_for_updated_at_filter? + sort_params.replace('updated_at' => sort_params.each_value.first) end + + if sort_params['created_at'] || sort_params['iid'] + # Sorting by `id` produces the same result as sorting by `created_at` or `iid` + sort_params.replace(id: sort_params.each_value.first) + elsif sort_params['updated_at'] + # This adds the order as a tie-breaker when multiple rows have the same updated_at value. + # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/20848. + sort_params.merge!(id: :desc) + end + end + + def filter_by_updated_at? + params[:updated_before].present? || params[:updated_after].present? + end + + def filter_by_finished_at? + params[:finished_before].present? || params[:finished_after].present? + end + + def filter_by_successful_deployment? + params[:status].to_s == 'success' + end + + def order_by_updated_at? + params[:order_by].to_s == 'updated_at' + end + + def order_by_finished_at? + params[:order_by].to_s == 'finished_at' + end + + def implicitly_enforce_ordering_for_updated_at_filter? + return false unless params[:project].present? + + ::Feature.enabled?( + :deployments_finder_implicitly_enforce_ordering_for_updated_at_filter, + params[:project], + default_enabled: :yaml) end # rubocop: disable CodeReuse/ActiveRecord |