diff --git a/app/assets/javascripts/behaviors/markdown/render_math.js b/app/assets/javascripts/behaviors/markdown/render_math.js index 8238f5523f3..12f47255bdf 100644 --- a/app/assets/javascripts/behaviors/markdown/render_math.js +++ b/app/assets/javascripts/behaviors/markdown/render_math.js @@ -114,6 +114,12 @@ class SafeMathRenderer { throwOnError: true, maxSize: 20, maxExpand: 20, + trust: (context) => + // this config option restores the KaTeX pre-v0.11.0 + // behavior of allowing certain commands and protocols + // eslint-disable-next-line @gitlab/require-i18n-strings + ['\\url', '\\href'].includes(context.command) && + ['http', 'https', 'mailto', '_relative'].includes(context.protocol), }); } catch (e) { // Don't show a flash for now because it would override an existing flash message diff --git a/app/assets/javascripts/jobs/components/table/cells/actions_cell.vue b/app/assets/javascripts/jobs/components/table/cells/actions_cell.vue new file mode 100644 index 00000000000..376482b0319 --- /dev/null +++ b/app/assets/javascripts/jobs/components/table/cells/actions_cell.vue @@ -0,0 +1,14 @@ + + + + + diff --git a/app/assets/javascripts/jobs/components/table/cells/duration_cell.vue b/app/assets/javascripts/jobs/components/table/cells/duration_cell.vue new file mode 100644 index 00000000000..ba5732d3d43 --- /dev/null +++ b/app/assets/javascripts/jobs/components/table/cells/duration_cell.vue @@ -0,0 +1,49 @@ + + + + + + + {{ durationTimeFormatted(duration) }} + + + + + {{ timeFormatted(finishedTime) }} + + + + diff --git a/app/assets/javascripts/jobs/components/table/cells/job_cell.vue b/app/assets/javascripts/jobs/components/table/cells/job_cell.vue new file mode 100644 index 00000000000..cea873b64a0 --- /dev/null +++ b/app/assets/javascripts/jobs/components/table/cells/job_cell.vue @@ -0,0 +1,131 @@ + + + + + + {{ jobId }} + + + + + + {{ job.refName }} + + + {{ __('none') }} + + + + {{ job.shortSha }} + + + + + + {{ tag }} + + + {{ s__('Job|triggered') }} + + {{ s__('Job|allowed to fail') }} + + {{ s__('Job|delayed') }} + + + {{ s__('Job|manual') }} + + + + diff --git a/app/assets/javascripts/jobs/components/table/cells/pipeline_cell.vue b/app/assets/javascripts/jobs/components/table/cells/pipeline_cell.vue new file mode 100644 index 00000000000..71f9397f5f5 --- /dev/null +++ b/app/assets/javascripts/jobs/components/table/cells/pipeline_cell.vue @@ -0,0 +1,50 @@ + + + + + + {{ pipelineId }} + + + {{ __('created by') }} + + + + {{ __('API') }} + + + diff --git a/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql b/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql index d9e51b0345a..7025bc97c53 100644 --- a/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql +++ b/app/assets/javascripts/jobs/components/table/graphql/queries/get_jobs.query.graphql @@ -8,7 +8,20 @@ query getJobs($fullPath: ID!, $statuses: [CiJobStatus!]) { startCursor } nodes { + artifacts { + nodes { + downloadPath + } + } + allowFailure + status + scheduledAt + manualJob + triggered + createdByTag detailedStatus { + detailsPath + group icon label text diff --git a/app/assets/javascripts/jobs/components/table/jobs_table.vue b/app/assets/javascripts/jobs/components/table/jobs_table.vue index 32b26d45dfe..f52f5163dda 100644 --- a/app/assets/javascripts/jobs/components/table/jobs_table.vue +++ b/app/assets/javascripts/jobs/components/table/jobs_table.vue @@ -1,6 +1,11 @@ - + + + + + + + + + + + + + + + + + + + + {{ item.stage.name }} + + + + + + {{ item.name }} + + + + + + + + + {{ + formatCoverage(item.coverage) + }} + + + + + + diff --git a/app/assets/javascripts/jobs/components/table/jobs_table_tabs.vue b/app/assets/javascripts/jobs/components/table/jobs_table_tabs.vue index 95d265fce60..26791e4284d 100644 --- a/app/assets/javascripts/jobs/components/table/jobs_table_tabs.vue +++ b/app/assets/javascripts/jobs/components/table/jobs_table_tabs.vue @@ -50,7 +50,7 @@ export default { - + import { + GlAlert, GlDropdown, GlDropdownItem, GlDropdownSectionHeader, + GlLoadingIcon, GlSprintf, GlTooltipDirective, } from '@gitlab/ui'; -import { __ } from '~/locale'; +import axios from '~/lib/utils/axios_utils'; +import { __, s__ } from '~/locale'; + +export const i18n = { + artifacts: __('Artifacts'), + downloadArtifact: __('Download %{name} artifact'), + artifactSectionHeader: __('Download artifacts'), + artifactsFetchErrorMessage: s__('Pipelines|Could not load artifacts.'), +}; export default { - i18n: { - artifacts: __('Artifacts'), - downloadArtifact: __('Download %{name} artifact'), - artifactSectionHeader: __('Download artifacts'), - }, + i18n, directives: { GlTooltip: GlTooltipDirective, }, components: { + GlAlert, GlDropdown, GlDropdownItem, GlDropdownSectionHeader, + GlLoadingIcon, GlSprintf, }, + inject: { + artifactsEndpoint: { + default: '', + }, + artifactsEndpointPlaceholder: { + default: '', + }, + }, props: { - artifacts: { - type: Array, + pipelineId: { + type: Number, required: true, }, }, + data() { + return { + artifacts: [], + hasError: false, + isLoading: false, + }; + }, + methods: { + fetchArtifacts() { + this.isLoading = true; + // Replace the placeholder with the ID of the pipeline we are viewing + const endpoint = this.artifactsEndpoint.replace( + this.artifactsEndpointPlaceholder, + this.pipelineId, + ); + return axios + .get(endpoint) + .then(({ data }) => { + this.artifacts = data.artifacts; + }) + .catch(() => { + this.hasError = true; + }) + .finally(() => { + this.isLoading = false; + }); + }, + }, }; @@ -43,11 +87,18 @@ export default { lazy text-sr-only no-caret + @show.once="fetchArtifacts" > {{ $options.i18n.artifactSectionHeader }} + + {{ $options.i18n.artifactsFetchErrorMessage }} + + + + - + diff --git a/app/assets/javascripts/pipelines/pipelines_index.js b/app/assets/javascripts/pipelines/pipelines_index.js index 9ed4365ad75..7cdc53a83bb 100644 --- a/app/assets/javascripts/pipelines/pipelines_index.js +++ b/app/assets/javascripts/pipelines/pipelines_index.js @@ -22,6 +22,8 @@ export const initPipelinesIndex = (selector = '#pipelines-list-vue') => { const { endpoint, + artifactsEndpoint, + artifactsEndpointPlaceholder, pipelineScheduleUrl, emptyStateSvgPath, errorStateSvgPath, @@ -41,6 +43,8 @@ export const initPipelinesIndex = (selector = '#pipelines-list-vue') => { el, provide: { addCiYmlPath, + artifactsEndpoint, + artifactsEndpointPlaceholder, suggestedCiTemplates: JSON.parse(suggestedCiTemplates), }, data() { diff --git a/app/assets/javascripts/vue_shared/mixins/timeago.js b/app/assets/javascripts/vue_shared/mixins/timeago.js index af14c6d9486..45452f2ea35 100644 --- a/app/assets/javascripts/vue_shared/mixins/timeago.js +++ b/app/assets/javascripts/vue_shared/mixins/timeago.js @@ -14,5 +14,25 @@ export default { tooltipTitle(time) { return formatDate(time); }, + + durationTimeFormatted(duration) { + const date = new Date(duration * 1000); + + let hh = date.getUTCHours(); + let mm = date.getUTCMinutes(); + let ss = date.getSeconds(); + + if (hh < 10) { + hh = `0${hh}`; + } + if (mm < 10) { + mm = `0${mm}`; + } + if (ss < 10) { + ss = `0${ss}`; + } + + return `${hh}:${mm}:${ss}`; + }, }, }; diff --git a/app/models/concerns/from_set_operator.rb b/app/models/concerns/from_set_operator.rb index 593fd251c5c..c6d63631c84 100644 --- a/app/models/concerns/from_set_operator.rb +++ b/app/models/concerns/from_set_operator.rb @@ -10,8 +10,8 @@ module FromSetOperator raise "Trying to redefine method '#{method(method_name)}'" if methods.include?(method_name) - define_method(method_name) do |members, remove_duplicates: true, alias_as: table_name| - operator_sql = operator.new(members, remove_duplicates: remove_duplicates).to_sql + define_method(method_name) do |members, remove_duplicates: true, remove_order: true, alias_as: table_name| + operator_sql = operator.new(members, remove_duplicates: remove_duplicates, remove_order: remove_order).to_sql from(Arel.sql("(#{operator_sql}) #{alias_as}")) end diff --git a/app/models/issue.rb b/app/models/issue.rb index 743e8417ced..a0b939d0122 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -109,6 +109,7 @@ class Issue < ApplicationRecord scope :order_due_date_desc, -> { reorder(::Gitlab::Database.nulls_last_order('due_date', 'DESC')) } scope :order_closest_future_date, -> { reorder(Arel.sql('CASE WHEN issues.due_date >= CURRENT_DATE THEN 0 ELSE 1 END ASC, ABS(CURRENT_DATE - issues.due_date) ASC')) } scope :order_relative_position_asc, -> { reorder(::Gitlab::Database.nulls_last_order('relative_position', 'ASC')) } + scope :order_relative_position_desc, -> { reorder(::Gitlab::Database.nulls_first_order('relative_position', 'DESC')) } scope :order_closed_date_desc, -> { reorder(closed_at: :desc) } scope :order_created_at_desc, -> { reorder(created_at: :desc) } scope :order_severity_asc, -> { includes(:issuable_severity).order('issuable_severities.severity ASC NULLS FIRST') } diff --git a/app/models/project.rb b/app/models/project.rb index 17e53d9e0c2..5033eb43979 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -367,6 +367,8 @@ class Project < ApplicationRecord has_one :operations_feature_flags_client, class_name: 'Operations::FeatureFlagsClient' has_many :operations_feature_flags_user_lists, class_name: 'Operations::FeatureFlags::UserList' + has_many :timelogs + accepts_nested_attributes_for :variables, allow_destroy: true accepts_nested_attributes_for :project_feature, update_only: true accepts_nested_attributes_for :project_setting, update_only: true @@ -615,7 +617,7 @@ class Project < ApplicationRecord mount_uploader :bfg_object_map, AttachmentUploader def self.with_api_entity_associations - preload(:project_feature, :route, :tags, :group, namespace: [:route, :owner]) + preload(:project_feature, :route, :tags, :group, :timelogs, namespace: [:route, :owner]) end def self.with_web_entity_associations diff --git a/app/models/timelog.rb b/app/models/timelog.rb index c1aa84cbbcd..b8ca6e1fdd0 100644 --- a/app/models/timelog.rb +++ b/app/models/timelog.rb @@ -3,11 +3,14 @@ class Timelog < ApplicationRecord include Importable + before_save :set_project + validates :time_spent, :user, presence: true validate :issuable_id_is_present, unless: :importing? belongs_to :issue, touch: true belongs_to :merge_request, touch: true + belongs_to :project belongs_to :user belongs_to :note @@ -37,6 +40,10 @@ class Timelog < ApplicationRecord end end + def set_project + self.project_id = issuable.project_id + end + # Rails5 defaults to :touch_later, overwrite for normal touch def belongs_to_touch_method :touch diff --git a/app/views/layouts/nav/_breadcrumbs.html.haml b/app/views/layouts/nav/_breadcrumbs.html.haml index b25f651aa64..c111714f552 100644 --- a/app/views/layouts/nav/_breadcrumbs.html.haml +++ b/app/views/layouts/nav/_breadcrumbs.html.haml @@ -9,7 +9,7 @@ = button_tag class: 'toggle-mobile-nav', type: 'button' do %span.sr-only= _("Open sidebar") = sprite_icon('hamburger', size: 18) - .breadcrumbs-links.overflow-auto{ data: { testid: 'breadcrumb-links', qa_selector: 'breadcrumb_links_content' } } + .breadcrumbs-links{ data: { testid: 'breadcrumb-links', qa_selector: 'breadcrumb_links_content' } } %ul.list-unstyled.breadcrumbs-list.js-breadcrumbs-list - unless hide_top_links = header_title diff --git a/app/views/projects/pipelines/index.html.haml b/app/views/projects/pipelines/index.html.haml index 4b0487f4685..731c30e9be9 100644 --- a/app/views/projects/pipelines/index.html.haml +++ b/app/views/projects/pipelines/index.html.haml @@ -1,12 +1,15 @@ - page_title _('Pipelines') - add_page_specific_style 'page_bundles/pipelines' - add_page_specific_style 'page_bundles/ci_status' +- artifacts_endpoint_placeholder = ':pipeline_artifacts_id' = render_if_exists "shared/shared_runners_minutes_limit_flash_message" #pipelines-list-vue{ data: { endpoint: project_pipelines_path(@project, format: :json), project_id: @project.id, params: params.to_json, + "artifacts-endpoint" => downloadable_artifacts_project_pipeline_path(@project, artifacts_endpoint_placeholder, format: :json), + "artifacts-endpoint-placeholder" => artifacts_endpoint_placeholder, "pipeline-schedule-url" => pipeline_schedules_path(@project), "empty-state-svg-path" => image_path('illustrations/pipelines_empty.svg'), "error-state-svg-path" => image_path('illustrations/pipelines_failed.svg'), diff --git a/changelogs/unreleased/25301-breadcrumbs-list-overflow-on-compare-with-sha1-on-mobile.yml b/changelogs/unreleased/25301-breadcrumbs-list-overflow-on-compare-with-sha1-on-mobile.yml deleted file mode 100644 index 6677a464768..00000000000 --- a/changelogs/unreleased/25301-breadcrumbs-list-overflow-on-compare-with-sha1-on-mobile.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Fix overflow in breadcrumbs list mainly on mobile -merge_request: 59552 -author: Takuya Noguchi -type: other diff --git a/changelogs/unreleased/325938-drop-jira-proxy-setting-columns.yml b/changelogs/unreleased/325938-drop-jira-proxy-setting-columns.yml new file mode 100644 index 00000000000..cd5abd45c30 --- /dev/null +++ b/changelogs/unreleased/325938-drop-jira-proxy-setting-columns.yml @@ -0,0 +1,5 @@ +--- +title: Drop Jira proxy setting columns +merge_request: 60123 +author: +type: other diff --git a/changelogs/unreleased/326987-update-katex-to-v0-13-0.yml b/changelogs/unreleased/326987-update-katex-to-v0-13-0.yml new file mode 100644 index 00000000000..a34e9cc6b3c --- /dev/null +++ b/changelogs/unreleased/326987-update-katex-to-v0-13-0.yml @@ -0,0 +1,5 @@ +--- +title: Update KaTeX integration to v0.13.0 +merge_request: 60071 +author: +type: other diff --git a/changelogs/unreleased/add-prpoject_id-to-timelogs.yml b/changelogs/unreleased/add-prpoject_id-to-timelogs.yml new file mode 100644 index 00000000000..548fb46c107 --- /dev/null +++ b/changelogs/unreleased/add-prpoject_id-to-timelogs.yml @@ -0,0 +1,5 @@ +--- +title: Add project_id foreign key to timelogs +merge_request: 60040 +author: Lee Tickett @leetickett +type: added diff --git a/changelogs/unreleased/sh-lazy-load-pipeline-artifacts.yml b/changelogs/unreleased/sh-lazy-load-pipeline-artifacts.yml new file mode 100644 index 00000000000..a4b5aebc945 --- /dev/null +++ b/changelogs/unreleased/sh-lazy-load-pipeline-artifacts.yml @@ -0,0 +1,5 @@ +--- +title: Lazy load artifacts on pipelines list page +merge_request: 60058 +author: +type: added diff --git a/db/migrate/20210422181809_add_project_to_timelogs.rb b/db/migrate/20210422181809_add_project_to_timelogs.rb new file mode 100644 index 00000000000..1f98e440d15 --- /dev/null +++ b/db/migrate/20210422181809_add_project_to_timelogs.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class AddProjectToTimelogs < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + with_lock_retries do + add_column :timelogs, :project_id, :integer + end + end + + def down + with_lock_retries do + remove_column :timelogs, :project_id + end + end +end diff --git a/db/migrate/20210424163400_add_project_id_fk_to_timelogs.rb b/db/migrate/20210424163400_add_project_id_fk_to_timelogs.rb new file mode 100644 index 00000000000..69542e7627d --- /dev/null +++ b/db/migrate/20210424163400_add_project_id_fk_to_timelogs.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class AddProjectIdFkToTimelogs < ActiveRecord::Migration[6.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + INDEX_NAME = 'index_timelogs_on_project_id_and_spent_at' + + disable_ddl_transaction! + + def up + add_concurrent_index :timelogs, [:project_id, :spent_at], name: INDEX_NAME + add_concurrent_foreign_key :timelogs, :projects, column: :project_id, on_delete: :cascade + end + + def down + with_lock_retries do + remove_foreign_key_if_exists :timelogs, column: :project_id + end + remove_concurrent_index_by_name :timelogs, INDEX_NAME + end +end diff --git a/db/post_migrate/20210423124223_remove_proxy_settings_to_jira_tracker_data.rb b/db/post_migrate/20210423124223_remove_proxy_settings_to_jira_tracker_data.rb new file mode 100644 index 00000000000..43ab965d79c --- /dev/null +++ b/db/post_migrate/20210423124223_remove_proxy_settings_to_jira_tracker_data.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class RemoveProxySettingsToJiraTrackerData < ActiveRecord::Migration[6.0] + def change + remove_column :jira_tracker_data, :encrypted_proxy_address, :text + remove_column :jira_tracker_data, :encrypted_proxy_address_iv, :text + remove_column :jira_tracker_data, :encrypted_proxy_port, :text + remove_column :jira_tracker_data, :encrypted_proxy_port_iv, :text + remove_column :jira_tracker_data, :encrypted_proxy_username, :text + remove_column :jira_tracker_data, :encrypted_proxy_username_iv, :text + remove_column :jira_tracker_data, :encrypted_proxy_password, :text + remove_column :jira_tracker_data, :encrypted_proxy_password_iv, :text + end +end diff --git a/db/schema_migrations/20210422181809 b/db/schema_migrations/20210422181809 new file mode 100644 index 00000000000..547e44f87b3 --- /dev/null +++ b/db/schema_migrations/20210422181809 @@ -0,0 +1 @@ +870589d3a4b4bc139ac29b0d87b0f9e777de21e854e5692c0dedd6683c83649a \ No newline at end of file diff --git a/db/schema_migrations/20210423124223 b/db/schema_migrations/20210423124223 new file mode 100644 index 00000000000..d5b07602553 --- /dev/null +++ b/db/schema_migrations/20210423124223 @@ -0,0 +1 @@ +6b508f1a48402aa2db3862e2e31ee4ccb851f535ed59f9b949ac1bad0ff2f0e1 \ No newline at end of file diff --git a/db/schema_migrations/20210424163400 b/db/schema_migrations/20210424163400 new file mode 100644 index 00000000000..ebc4b319aca --- /dev/null +++ b/db/schema_migrations/20210424163400 @@ -0,0 +1 @@ +808e4c1b0bb4f44afea57cce84820ef1371ae852d7cbc79ef454c04219ea956d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index e27a8c0403b..df77bef9631 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14016,14 +14016,6 @@ CREATE TABLE jira_tracker_data ( deployment_type smallint DEFAULT 0 NOT NULL, vulnerabilities_issuetype text, vulnerabilities_enabled boolean DEFAULT false NOT NULL, - encrypted_proxy_address text, - encrypted_proxy_address_iv text, - encrypted_proxy_port text, - encrypted_proxy_port_iv text, - encrypted_proxy_username text, - encrypted_proxy_username_iv text, - encrypted_proxy_password text, - encrypted_proxy_password_iv text, jira_issue_transition_automatic boolean DEFAULT false NOT NULL, CONSTRAINT check_0bf84b76e9 CHECK ((char_length(vulnerabilities_issuetype) <= 255)), CONSTRAINT check_214cf6a48b CHECK ((char_length(project_key) <= 255)) @@ -18015,7 +18007,8 @@ CREATE TABLE timelogs ( issue_id integer, merge_request_id integer, spent_at timestamp without time zone, - note_id integer + note_id integer, + project_id integer ); CREATE SEQUENCE timelogs_id_seq @@ -24149,6 +24142,8 @@ CREATE INDEX index_timelogs_on_merge_request_id ON timelogs USING btree (merge_r CREATE INDEX index_timelogs_on_note_id ON timelogs USING btree (note_id); +CREATE INDEX index_timelogs_on_project_id_and_spent_at ON timelogs USING btree (project_id, spent_at); + CREATE INDEX index_timelogs_on_spent_at ON timelogs USING btree (spent_at) WHERE (spent_at IS NOT NULL); CREATE INDEX index_timelogs_on_user_id ON timelogs USING btree (user_id); @@ -25324,6 +25319,9 @@ ALTER TABLE ONLY geo_event_log ALTER TABLE ONLY vulnerability_exports ADD CONSTRAINT fk_c3d3cb5d0f FOREIGN KEY (group_id) REFERENCES namespaces(id) ON DELETE CASCADE; +ALTER TABLE ONLY timelogs + ADD CONSTRAINT fk_c49c83dd77 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY geo_event_log ADD CONSTRAINT fk_c4b1c1f66e FOREIGN KEY (repository_deleted_event_id) REFERENCES geo_repository_deleted_events(id) ON DELETE CASCADE; diff --git a/doc/operations/incident_management/alerts.md b/doc/operations/incident_management/alerts.md index 0c7a87d3087..c49684954d9 100644 --- a/doc/operations/incident_management/alerts.md +++ b/doc/operations/incident_management/alerts.md @@ -33,9 +33,10 @@ The alert list displays the following information: - **Event count**: The number of times that an alert has fired. - **Issue**: A link to the incident issue that has been created for the alert. - **Status**: The current status of the alert: - - **Triggered**: No one has begun investigation. + - **Triggered**: Investigation has not started. - **Acknowledged**: Someone is actively investigating the problem. - **Resolved**: No further work is required. + - **Ignored**: No action will be taken on the alert. NOTE: Check out a live example available from the diff --git a/doc/user/application_security/container_scanning/index.md b/doc/user/application_security/container_scanning/index.md index c0d47df0f2a..8a6033ee307 100644 --- a/doc/user/application_security/container_scanning/index.md +++ b/doc/user/application_security/container_scanning/index.md @@ -24,10 +24,8 @@ displays them in a merge request, you can use GitLab to audit your Docker-based GitLab provides integration with two different open-source tools for vulnerability static analysis in containers: -| GitLab Project | Open-source tool | -| --- | --- | -|[Klar Analyzer](https://gitlab.com/gitlab-org/security-products/analyzers/klar/)| [Klar](https://github.com/optiopay/klar)| -|[Container-Scanning](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning)|[Trivy](https://github.com/aquasecurity/trivy)| +- [Clair](https://github.com/quay/claircore) +- [Trivy](https://github.com/aquasecurity/trivy) To integrate GitLab with security scanners other than those listed here, see [Security scanner integration](../../../development/integrations/secure.md). @@ -57,10 +55,10 @@ To enable container scanning in your pipeline, you need the following: shared runners on GitLab.com, then this is already the case. - An image matching the following supported distributions (depending on the analyzer being used): - | GitLab Analyzer | Supported distributions | + | Scanning Engine | Supported distributions | | --- | --- | - |[Klar](https://gitlab.com/gitlab-org/security-products/analyzers/klar/)| [Claircore](https://quay.github.io/claircore/)| - |[Container-Scanning](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning)|[OS](https://aquasecurity.github.io/trivy/latest/vuln-detection/os/) & [Application](https://aquasecurity.github.io/trivy/latest/vuln-detection/library/)| + | [Clair](https://github.com/quay/claircore) | [Supported operating systems and languages](https://quay.github.io/claircore/) | + | [Trivy](https://github.com/aquasecurity/trivy) | Supported [operating systems](https://aquasecurity.github.io/trivy/latest/vuln-detection/os/) and [languages](https://aquasecurity.github.io/trivy/latest/vuln-detection/library/) | - [Build and push](../../packages/container_registry/index.md#build-and-push-by-using-gitlab-cicd) your Docker image to your project's container registry. The name of the Docker image should use @@ -168,7 +166,7 @@ The variables you set in your `.gitlab-ci.yml` overwrite those in This example [includes](../../../ci/yaml/README.md#include) the container scanning template and enables verbose output for both analyzers: -Klar: +Clair: ```yaml include: @@ -178,7 +176,7 @@ variables: CLAIR_TRACE: true ``` -Container-Scanning: +Trivy: ```yaml include: @@ -210,27 +208,27 @@ You can [configure](#customizing-the-container-scanning-settings) both analyzers | CI/CD Variable | Default | Description | Supported by| | ------------------------------ | ------------- | ----------- | ------------ | | `ADDITIONAL_CA_CERT_BUNDLE` | `""` | Bundle of CA certs that you want to trust. See [Using a custom SSL CA certificate authority](#using-a-custom-ssl-ca-certificate-authority) for more details. | Both | -| `CLAIR_DB_CONNECTION_STRING` | `postgresql://postgres:password@clair-vulnerabilities-db:5432/postgres?sslmode=disable&statement_timeout=60000` | This variable represents the [connection string](https://www.postgresql.org/docs/9.3/libpq-connect.html#AEN39692) to the [PostgreSQL server hosting the vulnerability definitions](https://hub.docker.com/r/arminc/clair-db) database. **Do not change this** unless you're running the image locally as described in [Running the standalone container scanning tool](#running-the-standalone-container-scanning-tool). The host value for the connection string must match the [alias](https://gitlab.com/gitlab-org/gitlab/-/blob/898c5da43504eba87b749625da50098d345b60d6/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml#L23) value of the `Container-Scanning.gitlab-ci.yml` template file, which defaults to `clair-vulnerabilities-db`. | Klar | -| `CLAIR_DB_IMAGE` | `arminc/clair-db:latest` | The Docker image name and tag for the [PostgreSQL server hosting the vulnerability definitions](https://hub.docker.com/r/arminc/clair-db). It can be useful to override this value with a specific version (for example, to provide a consistent set of vulnerabilities for integration testing purposes, or to refer to a locally hosted vulnerability database for an on-premise offline installation). | Klar | -| `CLAIR_DB_IMAGE_TAG` | `latest` | (**DEPRECATED - use `CLAIR_DB_IMAGE` instead**) The Docker image tag for the [PostgreSQL server hosting the vulnerability definitions](https://hub.docker.com/r/arminc/clair-db). It can be useful to override this value with a specific version (for example, to provide a consistent set of vulnerabilities for integration testing purposes). | Klar | -| `CLAIR_OUTPUT` | `Unknown` | Severity level threshold. Vulnerabilities with severity level higher than or equal to this threshold are output. Supported levels are `Unknown`, `Negligible`, `Low`, `Medium`, `High`, `Critical`, and `Defcon1`. | Klar | -| `CLAIR_TRACE` | `"false"` | Set to true to enable more verbose output from the Clair server process. | Klar | -| `CLAIR_VULNERABILITIES_DB_URL` | `clair-vulnerabilities-db` | (**DEPRECATED - use `CLAIR_DB_CONNECTION_STRING` instead**) This variable is explicitly set in the [services section](https://gitlab.com/gitlab-org/gitlab/-/blob/898c5da43504eba87b749625da50098d345b60d6/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml#L23) of the `Container-Scanning.gitlab-ci.yml` file and defaults to `clair-vulnerabilities-db`. This value represents the address that the [PostgreSQL server hosting the vulnerability definitions](https://hub.docker.com/r/arminc/clair-db) is running on. **Do not change this** unless you're running the image locally as described in [Running the standalone container scanning tool](#running-the-standalone-container-scanning-tool). | Klar | +| `CLAIR_DB_CONNECTION_STRING` | `postgresql://postgres:password@clair-vulnerabilities-db:5432/postgres?sslmode=disable&statement_timeout=60000` | This variable represents the [connection string](https://www.postgresql.org/docs/9.3/libpq-connect.html#AEN39692) to the [PostgreSQL server hosting the vulnerability definitions](https://hub.docker.com/r/arminc/clair-db) database. **Do not change this** unless you're running the image locally as described in [Running the standalone container scanning tool](#running-the-standalone-container-scanning-tool). The host value for the connection string must match the [alias](https://gitlab.com/gitlab-org/gitlab/-/blob/898c5da43504eba87b749625da50098d345b60d6/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml#L23) value of the `Container-Scanning.gitlab-ci.yml` template file, which defaults to `clair-vulnerabilities-db`. | Clair | +| `CLAIR_DB_IMAGE` | `arminc/clair-db:latest` | The Docker image name and tag for the [PostgreSQL server hosting the vulnerability definitions](https://hub.docker.com/r/arminc/clair-db). It can be useful to override this value with a specific version (for example, to provide a consistent set of vulnerabilities for integration testing purposes, or to refer to a locally hosted vulnerability database for an on-premise offline installation). | Clair | +| `CLAIR_DB_IMAGE_TAG` | `latest` | (**DEPRECATED - use `CLAIR_DB_IMAGE` instead**) The Docker image tag for the [PostgreSQL server hosting the vulnerability definitions](https://hub.docker.com/r/arminc/clair-db). It can be useful to override this value with a specific version (for example, to provide a consistent set of vulnerabilities for integration testing purposes). | Clair | +| `CLAIR_OUTPUT` | `Unknown` | Severity level threshold. Vulnerabilities with severity level higher than or equal to this threshold are output. Supported levels are `Unknown`, `Negligible`, `Low`, `Medium`, `High`, `Critical`, and `Defcon1`. | Clair | +| `CLAIR_TRACE` | `"false"` | Set to true to enable more verbose output from the Clair server process. | Clair | +| `CLAIR_VULNERABILITIES_DB_URL` | `clair-vulnerabilities-db` | (**DEPRECATED - use `CLAIR_DB_CONNECTION_STRING` instead**) This variable is explicitly set in the [services section](https://gitlab.com/gitlab-org/gitlab/-/blob/898c5da43504eba87b749625da50098d345b60d6/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml#L23) of the `Container-Scanning.gitlab-ci.yml` file and defaults to `clair-vulnerabilities-db`. This value represents the address that the [PostgreSQL server hosting the vulnerability definitions](https://hub.docker.com/r/arminc/clair-db) is running on. **Do not change this** unless you're running the image locally as described in [Running the standalone container scanning tool](#running-the-standalone-container-scanning-tool). | Clair | | `CI_APPLICATION_REPOSITORY` | `$CI_REGISTRY_IMAGE/$CI_COMMIT_REF_SLUG` | Docker repository URL for the image to be scanned. | Both | | `CI_APPLICATION_TAG` | `$CI_COMMIT_SHA` | Docker repository tag for the image to be scanned. | Both | | `CS_ANALYZER_IMAGE` | `$SECURE_ANALYZERS_PREFIX/$CS_PROJECT:$CS_MAJOR_VERSION` | Docker image of the analyzer. | Both | | `CS_MAJOR_VERSION` | `3` | The major version of the Docker image tag. | Both | | `CS_PROJECT` | Depends on `$CS_MAJOR_VERSION`. `klar` if `$CS_MAJOR_VERSION` is set to `1`, `2` or `3`, and `container-scanning` otherwise. | Analyzer project to be used. | Both | | `DOCKER_IMAGE` | `$CI_APPLICATION_REPOSITORY:$CI_APPLICATION_TAG` | The Docker image to be scanned. If set, this variable overrides the `$CI_APPLICATION_REPOSITORY` and `$CI_APPLICATION_TAG` variables. | Both | -| `DOCKER_INSECURE` | `"false"` | Allow [Klar](https://github.com/optiopay/klar) to access secure Docker registries using HTTPS with bad (or self-signed) SSL certificates. | Klar | -| `DOCKER_PASSWORD` | `$CI_REGISTRY_PASSWORD` | Password for accessing a Docker registry requiring authentication. | Klar | -| `DOCKER_USER` | `$CI_REGISTRY_USER` | Username for accessing a Docker registry requiring authentication. | Klar | +| `DOCKER_INSECURE` | `"false"` | Allow [Klar](https://github.com/optiopay/klar) to access secure Docker registries using HTTPS with bad (or self-signed) SSL certificates. | Clair | +| `DOCKER_PASSWORD` | `$CI_REGISTRY_PASSWORD` | Password for accessing a Docker registry requiring authentication. | Clair | +| `DOCKER_USER` | `$CI_REGISTRY_USER` | Username for accessing a Docker registry requiring authentication. | Clair | | `DOCKERFILE_PATH` | `Dockerfile` | The path to the `Dockerfile` to use for generating remediations. By default, the scanner looks for a file named `Dockerfile` in the root directory of the project. You should configure this variable only if your `Dockerfile` is in a non-standard location, such as a subdirectory. See [Solutions for vulnerabilities](#solutions-for-vulnerabilities-auto-remediation) for more details. | Both | -| `KLAR_TRACE` | `"false"` | Set to true to enable more verbose output from Klar. | Klar | -| `REGISTRY_INSECURE` | `"false"` | Allow [Klar](https://github.com/optiopay/klar) to access insecure registries (HTTP only). Should only be set to `true` when testing the image locally. | Klar | +| `KLAR_TRACE` | `"false"` | Set to true to enable more verbose output from Klar. | Clair | +| `REGISTRY_INSECURE` | `"false"` | Allow [Klar](https://github.com/optiopay/klar) to access insecure registries (HTTP only). Should only be set to `true` when testing the image locally. | Clair | | `SECURE_ANALYZERS_PREFIX` | `"registry.gitlab.com/gitlab-org/security-products/analyzers"` | Set the Docker registry base address from which to download the analyzer. | Both | | `SECURE_LOG_LEVEL` | `info` | Set the minimum logging level. Messages of this logging level or higher are output. From highest to lowest severity, the logging levels are: `fatal`, `error`, `warn`, `info`, `debug`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/10880) in GitLab 13.1. | Both | -| `TRIVY_DEBUG` | `"false"` | Set to true to enable more verbose output from the Trivy process. | Container-Scanning | +| `TRIVY_DEBUG` | `"false"` | Set to true to enable more verbose output from the Trivy process. | Trivy | ### Overriding the container scanning template @@ -238,7 +236,7 @@ If you want to override the job definition (for example, to change properties li must declare and override a job after the template inclusion, and then specify any additional keys. -This example sets `GIT_STRATEGY` to `fetch` to be considered by both Klar and Container-Scanning: +This example sets `GIT_STRATEGY` to `fetch` to be considered by both Clair and Trivy: ```yaml include: @@ -249,7 +247,7 @@ include: GIT_STRATEGY: fetch ``` -This example sets `KLAR_TRACE` to `true`, which is specific to Klar: +This example sets `KLAR_TRACE` to `true`, which is specific to Clair: ```yaml include: @@ -260,7 +258,7 @@ container_scanning: CLAIR_TRACE: true ``` -This example sets `TRIVY_DEBUG` to `true`, which is specific to Container-Scanning: +This example sets `TRIVY_DEBUG` to `true`, which is specific to Trivy: ```yaml include: @@ -290,16 +288,16 @@ taking the following steps: - Remove the `CS_ANALYZER_IMAGE` variable from your CI file. The job scope is `.cs_common`. Note that instead of overriding this variable, you can use `CS_MAJOR_VERSION`. -1. Remove any variables that are only applicable to Klar. For a complete list of these variables, +1. Remove any variables that are only applicable to Clair. For a complete list of these variables, see the [available variables](#available-variables). 1. Make any [necessary customizations](#customizing-the-container-scanning-settings) to the - `Container-Scanning` scanner. We strongly recommended that you minimize customizations, as they + `Trivy` scanner. We strongly recommended that you minimize customizations, as they might require changes in future GitLab major releases. **Troubleshooting** Prior to the GitLab 14.0 release, any variable defined under the scope `container_scanning` is not -considered for Container-Scanning. Verify that all variables for the Container-Scanning analyzer are +considered for the Trivy scanner. Verify that all variables for Trivy are either defined as a global variable, or under `.cs_common` and `container_scanning_new`. ### Using a custom SSL CA certificate authority @@ -422,8 +420,8 @@ To use container scanning in an offline environment, you need: | GitLab Analyzer | Container Registry | | --- | --- | -|[Klar](https://gitlab.com/gitlab-org/security-products/analyzers/klar/)| [registry URL](https://gitlab.com/gitlab-org/security-products/analyzers/klar/container_registry) | -|[Container-Scanning](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning)|[registry URL](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning/container_registry/1741162)| +| [Klar](https://gitlab.com/gitlab-org/security-products/analyzers/klar/) (used to run Clair) | [Klar container registry](https://gitlab.com/gitlab-org/security-products/analyzers/klar/container_registry) | +| [Container-Scanning](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning) (used to run Trivy) | [Container-Scanning container registry](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning/container_registry/1741162) | Note that GitLab Runner has a [default `pull policy` of `always`](https://docs.gitlab.com/runner/executors/docker.html#using-the-always-pull-policy), meaning the runner tries to pull Docker images from the GitLab container registry even if a local @@ -436,24 +434,24 @@ enables the use of updated scanners in your CI/CD pipelines. Support for custom certificate authorities was introduced in the following versions: -| Analyzer | Version | +| Scanner | Version | | -------- | ------- | -| `klar` | [v2.3.0](https://gitlab.com/gitlab-org/security-products/analyzers/klar/-/releases/v2.3.0) | -| `container-scanning` | [4.0.0](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning/-/releases/4.0.0) | +| `Clair` | [v2.3.0](https://gitlab.com/gitlab-org/security-products/analyzers/klar/-/releases/v2.3.0) | +| `Trivy` | [4.0.0](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning/-/releases/4.0.0) | #### Make GitLab container scanning analyzer images available inside your Docker registry For container scanning, import the following default images from `registry.gitlab.com` into your [local Docker container registry](../../packages/container_registry/index.md): -Klar: +Clair: ```plaintext registry.gitlab.com/gitlab-org/security-products/analyzers/klar https://hub.docker.com/r/arminc/clair-db ``` -Container-Scanning: +Trivy: ```plaintext registry.gitlab.com/gitlab-org/security-products/analyzers/container-scanning @@ -475,7 +473,7 @@ For details on saving and transporting Docker images as a file, see Docker's doc 1. [Override the container scanning template](#overriding-the-container-scanning-template) in your `.gitlab-ci.yml` file to refer to the Docker images hosted on your local Docker container registry: - Klar: + Clair: ```yaml include: @@ -487,7 +485,7 @@ For details on saving and transporting Docker images as a file, see Docker's doc CLAIR_DB_IMAGE: $CI_REGISTRY/namespace/clair-vulnerabilities-db ``` - Container-Scanning: + Trivy: ```yaml include: @@ -499,11 +497,11 @@ For details on saving and transporting Docker images as a file, see Docker's doc 1. If your local Docker container registry is running securely over `HTTPS`, but you're using a self-signed certificate, then you must set `DOCKER_INSECURE: "true"` in the above - `container_scanning` section of your `.gitlab-ci.yml`. This only applies to Klar. + `container_scanning` section of your `.gitlab-ci.yml`. This only applies to Clair. #### Automating container scanning vulnerability database updates with a pipeline -For those using Klar, it can be worthwhile to set up a [scheduled pipeline](../../../ci/pipelines/schedules.md) +For those using Clair, it can be worthwhile to set up a [scheduled pipeline](../../../ci/pipelines/schedules.md) to build a new version of the vulnerabilities database on a preset schedule. Automating this with a pipeline means you do not have to do it manually each time. You can use the following `.gitlab-yml.ci` as a template: @@ -529,7 +527,7 @@ The above template works for a GitLab Docker registry running on a local install ## Running the standalone container scanning tool -### Klar +### Clair It's possible to run [Klar](https://gitlab.com/gitlab-org/security-products/analyzers/klar) against a Docker container without needing to run it within the context of a CI job. To scan an @@ -563,7 +561,7 @@ image directly, follow these steps: The results are stored in `gl-container-scanning-report.json`. -### Container-Scanning +### Trivy It's possible to run the [GitLab container scanning tool](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning) against a Docker container without needing to run it within the context of a CI job. To scan an @@ -691,7 +689,7 @@ the security vulnerabilities in your groups, projects and pipelines. ## Vulnerabilities database update -If you're using Klar and want more information about the vulnerabilities database update, see the +If you're using Clair and want more information about the vulnerabilities database update, see the [maintenance table](../index.md#maintenance-and-update-of-the-vulnerabilities-database). ## Interacting with the vulnerabilities diff --git a/doc/user/application_security/dast/index.md b/doc/user/application_security/dast/index.md index 73da4d4c832..ae09a47ca1b 100644 --- a/doc/user/application_security/dast/index.md +++ b/doc/user/application_security/dast/index.md @@ -658,6 +658,7 @@ DAST can be [configured](#customizing-the-dast-settings) using CI/CD variables. | `DAST_AUTO_UPDATE_ADDONS` | boolean | ZAP add-ons are pinned to specific versions in the DAST Docker image. Set to `true` to download the latest versions when the scan starts. Default: `false` | | `DAST_API_HOST_OVERRIDE` | string | Used to override domains defined in API specification files. Only supported when importing the API specification from a URL. Example: `example.com:8080` | | `DAST_EXCLUDE_RULES` | string | Set to a comma-separated list of Vulnerability Rule IDs to exclude them from running during the scan. Rule IDs are numbers and can be found from the DAST log or on the [ZAP project](https://github.com/zaproxy/zaproxy/blob/develop/docs/scanners.md). For example, `HTTP Parameter Override` has a rule ID of `10026`. **Note:** In earlier versions of GitLab the excluded rules were executed but alerts they generated were suppressed. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/118641) in GitLab 12.10. | +| `DAST_ONLY_INCLUDE_RULES` | string | Set to a comma-separated list of Vulnerability Rule IDs to configure the scan to run only them. Rule IDs are numbers and can be found from the DAST log or on the [ZAP project](https://github.com/zaproxy/zaproxy/blob/develop/docs/scanners.md). [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/250651) in GitLab 13.12. | | `DAST_REQUEST_HEADERS` | string | Set to a comma-separated list of request header names and values. Headers are added to every request made by DAST. For example, `Cache-control: no-cache,User-Agent: DAST/1.0` | | `DAST_DEBUG` | boolean | Enable debug message output. Default: `false`. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12652) in GitLab 13.1. | | `DAST_SPIDER_MINS` | number | The maximum duration of the spider scan in minutes. Set to `0` for unlimited. Default: One minute, or unlimited when the scan is a full scan. [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/12652) in GitLab 13.1. | diff --git a/doc/user/application_security/vulnerabilities/severities.md b/doc/user/application_security/vulnerabilities/severities.md index 7576d08bcbe..5b24afa807e 100644 --- a/doc/user/application_security/vulnerabilities/severities.md +++ b/doc/user/application_security/vulnerabilities/severities.md @@ -61,10 +61,10 @@ the following tables: ## Container Scanning -| GitLab analyzer | Outputs severity levels? | Native severity level type | Native severity level example | +| GitLab scanner | Outputs severity levels? | Native severity level type | Native severity level example | |------------------------------------------------------------------------|--------------------------|----------------------------|--------------------------------------------------------------| -| [`klar`](https://gitlab.com/gitlab-org/security-products/analyzers/klar) | **{check-circle}** Yes | String | `Negligible`, `Low`, `Medium`, `High`, `Critical`, `Defcon1` | -| [`container-scanning`](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning)| **{check-circle}** Yes | String | `Unknown`, `Low`, `Medium`, `High`, `Critical` | +| [`clair`](https://gitlab.com/gitlab-org/security-products/analyzers/klar) | **{check-circle}** Yes | String | `Negligible`, `Low`, `Medium`, `High`, `Critical`, `Defcon1` | +| [`trivy`](https://gitlab.com/gitlab-org/security-products/analyzers/container-scanning)| **{check-circle}** Yes | String | `Unknown`, `Low`, `Medium`, `High`, `Critical` | ## Fuzz Testing diff --git a/doc/user/feature_flags.md b/doc/user/feature_flags.md index 5be28de4101..e05fc582dc0 100644 --- a/doc/user/feature_flags.md +++ b/doc/user/feature_flags.md @@ -3,6 +3,7 @@ stage: none group: Development info: "See the Technical Writers assigned to Development Guidelines: https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments-to-development-guidelines" description: "Understand what 'GitLab features deployed behind flags' means." +layout: 'feature_flags' --- # GitLab functionality may be limited by feature flags diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 263fc614f59..2f6c3cc46fb 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -123,6 +123,16 @@ module Gitlab # ignore - happens when Rake tasks yet have to create a database, e.g. for testing end + def self.nulls_order(field, direction = :asc, nulls_order = :nulls_last) + raise ArgumentError unless [:nulls_last, :nulls_first].include?(nulls_order) + raise ArgumentError unless [:asc, :desc].include?(direction) + + case nulls_order + when :nulls_last then nulls_last_order(field, direction) + when :nulls_first then nulls_first_order(field, direction) + end + end + def self.nulls_last_order(field, direction = 'ASC') Arel.sql("#{field} #{direction} NULLS LAST") end diff --git a/lib/gitlab/pagination/keyset/iterator.rb b/lib/gitlab/pagination/keyset/iterator.rb new file mode 100644 index 00000000000..3bc8c0bf616 --- /dev/null +++ b/lib/gitlab/pagination/keyset/iterator.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Gitlab + module Pagination + module Keyset + class Iterator + def initialize(scope:, use_union_optimization: false) + @scope = scope + @order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(scope) + @use_union_optimization = use_union_optimization + end + + # rubocop: disable CodeReuse/ActiveRecord + def each_batch(of: 1000) + cursor_attributes = {} + + loop do + current_scope = scope.dup.limit(of) + relation = order + .apply_cursor_conditions(current_scope, cursor_attributes, { use_union_optimization: @use_union_optimization }) + .reorder(order) + .limit(of) + + yield relation + + last_record = relation.last + break unless last_record + + cursor_attributes = order.cursor_attributes_for_node(last_record) + end + end + # rubocop: enable CodeReuse/ActiveRecord + + private + + attr_reader :scope, :order + end + end + end +end diff --git a/lib/gitlab/pagination/keyset/order.rb b/lib/gitlab/pagination/keyset/order.rb index 648b371ee9b..cef3a7b291a 100644 --- a/lib/gitlab/pagination/keyset/order.rb +++ b/lib/gitlab/pagination/keyset/order.rb @@ -135,7 +135,7 @@ module Gitlab # # (id < 3 AND created_at IS NULL) OR (created_at IS NOT NULL) def build_where_values(values) - return if values.blank? + return [] if values.blank? verify_incoming_values!(values) @@ -156,13 +156,26 @@ module Gitlab end end - build_or_query(where_values) + where_values + end + + def where_values_with_or_query(values) + build_or_query(build_where_values(values.with_indifferent_access)) end # rubocop: disable CodeReuse/ActiveRecord - def apply_cursor_conditions(scope, values = {}) + def apply_cursor_conditions(scope, values = {}, options = { use_union_optimization: false }) + values ||= {} + transformed_values = values.with_indifferent_access scope = apply_custom_projections(scope) - scope.where(build_where_values(values.with_indifferent_access)) + + where_values = build_where_values(transformed_values) + + if options[:use_union_optimization] && where_values.size > 1 + build_union_query(scope, where_values).reorder(self) + else + scope.where(build_or_query(where_values)) # rubocop: disable CodeReuse/ActiveRecord + end end # rubocop: enable CodeReuse/ActiveRecord @@ -212,11 +225,19 @@ module Gitlab end def build_or_query(expressions) - or_expression = expressions.reduce { |or_expression, expression| Arel::Nodes::Or.new(or_expression, expression) } + return [] if expressions.blank? + or_expression = expressions.reduce { |or_expression, expression| Arel::Nodes::Or.new(or_expression, expression) } Arel::Nodes::Grouping.new(or_expression) end + def build_union_query(scope, where_values) + scopes = where_values.map do |where_value| + scope.dup.where(where_value).reorder(self) # rubocop: disable CodeReuse/ActiveRecord + end + scope.model.from_union(scopes, remove_duplicates: false, remove_order: false) + end + def to_sql_literal(column_definitions) column_definitions.map do |column_definition| if column_definition.order_expression.respond_to?(:to_sql) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 276737cdfef..c49963aa14b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1487,6 +1487,9 @@ msgstr "" msgid "ACTION REQUIRED: Something went wrong while obtaining the Let's Encrypt certificate for GitLab Pages domain '%{domain}'" msgstr "" +msgid "API" +msgstr "" + msgid "API Fuzzing" msgstr "" @@ -18543,12 +18546,24 @@ msgstr "" msgid "Job|This job is stuck because you don't have any active runners that can run this job." msgstr "" +msgid "Job|allowed to fail" +msgstr "" + +msgid "Job|delayed" +msgstr "" + msgid "Job|for" msgstr "" msgid "Job|into" msgstr "" +msgid "Job|manual" +msgstr "" + +msgid "Job|triggered" +msgstr "" + msgid "Job|with" msgstr "" @@ -23641,6 +23656,9 @@ msgstr "" msgid "Pipelines|Copy trigger token" msgstr "" +msgid "Pipelines|Could not load artifacts." +msgstr "" + msgid "Pipelines|Could not load merged YAML content" msgstr "" @@ -37581,6 +37599,9 @@ msgstr "" msgid "created %{timeAgo}" msgstr "" +msgid "created by" +msgstr "" + msgid "data" msgstr "" diff --git a/package.json b/package.json index dcdae406c0f..31e20d88981 100644 --- a/package.json +++ b/package.json @@ -109,7 +109,7 @@ "js-yaml": "^3.13.1", "jszip": "^3.1.3", "jszip-utils": "^0.0.2", - "katex": "^0.10.0", + "katex": "^0.13.2", "lodash": "^4.17.20", "marked": "^0.3.12", "mathjax": "3", diff --git a/spec/factories/issues.rb b/spec/factories/issues.rb index 5c62de4d08d..2d52747dece 100644 --- a/spec/factories/issues.rb +++ b/spec/factories/issues.rb @@ -13,6 +13,14 @@ FactoryBot.define do confidential { true } end + trait :with_asc_relative_position do + sequence(:relative_position) { |n| n * 1000 } + end + + trait :with_desc_relative_position do + sequence(:relative_position) { |n| -n * 1000 } + end + trait :opened do state_id { Issue.available_states[:opened] } end diff --git a/spec/features/markdown/math_spec.rb b/spec/features/markdown/math_spec.rb index 441cff7045f..fa23fac2f96 100644 --- a/spec/features/markdown/math_spec.rb +++ b/spec/features/markdown/math_spec.rb @@ -13,14 +13,24 @@ RSpec.describe 'Math rendering', :js do ```math a^2+b^2=c^2 ``` + + This math is aligned + + ```math + \\begin{align*} + a&=b+c \\\\ + d+e&=f + \\end{align*} + ``` MATH issue = create(:issue, project: project, description: description) visit project_issue_path(project, issue) - expect(page).to have_selector('.katex .mord.mathdefault', text: 'b') - expect(page).to have_selector('.katex-display .mord.mathdefault', text: 'b') + expect(page).to have_selector('.katex .mord.mathnormal', text: 'b') + expect(page).to have_selector('.katex-display .mord.mathnormal', text: 'b') + expect(page).to have_selector('.katex-display .mtable .col-align-l .mord.mathnormal', text: 'f') end it 'only renders non XSS links' do @@ -35,7 +45,9 @@ RSpec.describe 'Math rendering', :js do visit project_issue_path(project, issue) page.within '.description > .md' do - expect(page).to have_selector('.katex-error') + # unfortunately there is no class selector for KaTeX's "unsupported command" + # formatting so we must match the style attribute + expect(page).to have_selector('.katex-html .mord[style*="color:"][style*="#cc0000"]', text: '\href') expect(page).to have_selector('.katex-html a', text: 'Gitlab') end end diff --git a/spec/frontend/jobs/components/table/cells.vue/duration_cell_spec.js b/spec/frontend/jobs/components/table/cells.vue/duration_cell_spec.js new file mode 100644 index 00000000000..763a4b0eaa2 --- /dev/null +++ b/spec/frontend/jobs/components/table/cells.vue/duration_cell_spec.js @@ -0,0 +1,81 @@ +import { shallowMount } from '@vue/test-utils'; +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import DurationCell from '~/jobs/components/table/cells/duration_cell.vue'; + +describe('Duration Cell', () => { + let wrapper; + + const findJobDuration = () => wrapper.findByTestId('job-duration'); + const findJobFinishedTime = () => wrapper.findByTestId('job-finished-time'); + const findDurationIcon = () => wrapper.findByTestId('duration-icon'); + const findFinishedTimeIcon = () => wrapper.findByTestId('finished-time-icon'); + + const createComponent = (props) => { + wrapper = extendedWrapper( + shallowMount(DurationCell, { + propsData: { + job: { + ...props, + }, + }, + }), + ); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + it('does not display duration or finished time when no properties are present', () => { + createComponent(); + + expect(findJobDuration().exists()).toBe(false); + expect(findJobFinishedTime().exists()).toBe(false); + }); + + it('displays duration and finished time when both properties are present', () => { + const props = { + duration: 7, + finishedAt: '2021-04-26T13:37:52Z', + }; + + createComponent(props); + + expect(findJobDuration().exists()).toBe(true); + expect(findJobFinishedTime().exists()).toBe(true); + }); + + it('displays only the duration of the job when the duration property is present', () => { + const props = { + duration: 7, + }; + + createComponent(props); + + expect(findJobDuration().exists()).toBe(true); + expect(findJobFinishedTime().exists()).toBe(false); + }); + + it('displays only the finished time of the job when the finshedAt property is present', () => { + const props = { + finishedAt: '2021-04-26T13:37:52Z', + }; + + createComponent(props); + + expect(findJobFinishedTime().exists()).toBe(true); + expect(findJobDuration().exists()).toBe(false); + }); + + it('displays icons for finished time and duration', () => { + const props = { + duration: 7, + finishedAt: '2021-04-26T13:37:52Z', + }; + + createComponent(props); + + expect(findFinishedTimeIcon().props('name')).toBe('calendar'); + expect(findDurationIcon().props('name')).toBe('timer'); + }); +}); diff --git a/spec/frontend/jobs/components/table/cells.vue/job_cell_spec.js b/spec/frontend/jobs/components/table/cells.vue/job_cell_spec.js new file mode 100644 index 00000000000..cfe6e45332e --- /dev/null +++ b/spec/frontend/jobs/components/table/cells.vue/job_cell_spec.js @@ -0,0 +1,112 @@ +import { shallowMount } from '@vue/test-utils'; +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; +import JobCell from '~/jobs/components/table/cells/job_cell.vue'; +import { mockJobsInTable } from '../../../mock_data'; + +const mockJob = mockJobsInTable[0]; +const mockJobCreatedByTag = mockJobsInTable[1]; + +describe('Job Cell', () => { + let wrapper; + + const findJobId = () => wrapper.findByTestId('job-id'); + const findJobRef = () => wrapper.findByTestId('job-ref'); + const findJobSha = () => wrapper.findByTestId('job-sha'); + const findLabelIcon = () => wrapper.findByTestId('label-icon'); + const findForkIcon = () => wrapper.findByTestId('fork-icon'); + const findAllTagBadges = () => wrapper.findAllByTestId('job-tag-badge'); + + const findBadgeById = (id) => wrapper.findByTestId(id); + + const createComponent = (jobData = mockJob) => { + wrapper = extendedWrapper( + shallowMount(JobCell, { + propsData: { + job: jobData, + }, + }), + ); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('Job Id', () => { + beforeEach(() => { + createComponent(); + }); + + it('displays the job id and links to the job', () => { + const expectedJobId = `#${getIdFromGraphQLId(mockJob.id)}`; + + expect(findJobId().text()).toBe(expectedJobId); + expect(findJobId().attributes('href')).toBe(mockJob.detailedStatus.detailsPath); + }); + }); + + describe('Ref of the job', () => { + it('displays the ref name and links to the ref', () => { + createComponent(); + + expect(findJobRef().text()).toBe(mockJob.refName); + expect(findJobRef().attributes('href')).toBe(mockJob.refPath); + }); + + it('displays fork icon when job is not created by tag', () => { + createComponent(); + + expect(findForkIcon().exists()).toBe(true); + expect(findLabelIcon().exists()).toBe(false); + }); + + it('displays label icon when job is created by a tag', () => { + createComponent(mockJobCreatedByTag); + + expect(findLabelIcon().exists()).toBe(true); + expect(findForkIcon().exists()).toBe(false); + }); + }); + + describe('Commit of the job', () => { + beforeEach(() => { + createComponent(); + }); + + it('displays the sha and links to the commit', () => { + expect(findJobSha().text()).toBe(mockJob.shortSha); + expect(findJobSha().attributes('href')).toBe(mockJob.commitPath); + }); + }); + + describe('Job badges', () => { + it('displays tags of the job', () => { + const mockJobWithTags = { + tags: ['tag-1', 'tag-2', 'tag-3'], + }; + + createComponent(mockJobWithTags); + + expect(findAllTagBadges()).toHaveLength(mockJobWithTags.tags.length); + }); + + it.each` + testId | text + ${'manual-job-badge'} | ${'manual'} + ${'triggered-job-badge'} | ${'triggered'} + ${'fail-job-badge'} | ${'allowed to fail'} + ${'delayed-job-badge'} | ${'delayed'} + `('displays the static $text badge', ({ testId, text }) => { + createComponent({ + manualJob: true, + triggered: true, + allowFailure: true, + scheduledAt: '2021-03-09T14:58:50+00:00', + }); + + expect(findBadgeById(testId).exists()).toBe(true); + expect(findBadgeById(testId).text()).toBe(text); + }); + }); +}); diff --git a/spec/frontend/jobs/components/table/cells.vue/pipeline_cell_spec.js b/spec/frontend/jobs/components/table/cells.vue/pipeline_cell_spec.js new file mode 100644 index 00000000000..1f5e0a7aa21 --- /dev/null +++ b/spec/frontend/jobs/components/table/cells.vue/pipeline_cell_spec.js @@ -0,0 +1,82 @@ +import { GlAvatar } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; +import PipelineCell from '~/jobs/components/table/cells/pipeline_cell.vue'; + +const mockJobWithoutUser = { + id: 'gid://gitlab/Ci::Build/2264', + pipeline: { + id: 'gid://gitlab/Ci::Pipeline/460', + path: '/root/ci-project/-/pipelines/460', + }, +}; + +const mockJobWithUser = { + id: 'gid://gitlab/Ci::Build/2264', + pipeline: { + id: 'gid://gitlab/Ci::Pipeline/460', + path: '/root/ci-project/-/pipelines/460', + user: { + avatarUrl: + 'https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon', + webPath: '/root', + }, + }, +}; + +describe('Pipeline Cell', () => { + let wrapper; + + const findPipelineId = () => wrapper.findByTestId('pipeline-id'); + const findPipelineUserLink = () => wrapper.findByTestId('pipeline-user-link'); + const findUserAvatar = () => wrapper.findComponent(GlAvatar); + + const createComponent = (props = mockJobWithUser) => { + wrapper = extendedWrapper( + shallowMount(PipelineCell, { + propsData: { + job: props, + }, + }), + ); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + describe('Pipeline Id', () => { + beforeEach(() => { + createComponent(); + }); + + it('displays the pipeline id and links to the pipeline', () => { + const expectedPipelineId = `#${getIdFromGraphQLId(mockJobWithUser.pipeline.id)}`; + + expect(findPipelineId().text()).toBe(expectedPipelineId); + expect(findPipelineId().attributes('href')).toBe(mockJobWithUser.pipeline.path); + }); + }); + + describe('Pipeline created by', () => { + const apiWrapperText = 'API'; + + it('shows and links to the pipeline user', () => { + createComponent(); + + expect(findPipelineUserLink().exists()).toBe(true); + expect(findPipelineUserLink().attributes('href')).toBe(mockJobWithUser.pipeline.user.webPath); + expect(findUserAvatar().attributes('src')).toBe(mockJobWithUser.pipeline.user.avatarUrl); + expect(wrapper.text()).not.toContain(apiWrapperText); + }); + + it('shows pipeline was created by the API', () => { + createComponent(mockJobWithoutUser); + + expect(findPipelineUserLink().exists()).toBe(false); + expect(findUserAvatar().exists()).toBe(false); + expect(wrapper.text()).toContain(apiWrapperText); + }); + }); +}); diff --git a/spec/frontend/jobs/components/table/jobs_table_spec.js b/spec/frontend/jobs/components/table/jobs_table_spec.js index db057efbfb4..ac8bef675f8 100644 --- a/spec/frontend/jobs/components/table/jobs_table_spec.js +++ b/spec/frontend/jobs/components/table/jobs_table_spec.js @@ -1,20 +1,29 @@ import { GlTable } from '@gitlab/ui'; -import { shallowMount } from '@vue/test-utils'; +import { mount } from '@vue/test-utils'; +import { extendedWrapper } from 'helpers/vue_test_utils_helper'; import JobsTable from '~/jobs/components/table/jobs_table.vue'; +import CiBadge from '~/vue_shared/components/ci_badge_link.vue'; import { mockJobsInTable } from '../../mock_data'; describe('Jobs Table', () => { let wrapper; const findTable = () => wrapper.findComponent(GlTable); + const findStatusBadge = () => wrapper.findComponent(CiBadge); + const findTableRows = () => wrapper.findAllByTestId('jobs-table-row'); + const findJobStage = () => wrapper.findByTestId('job-stage-name'); + const findJobName = () => wrapper.findByTestId('job-name'); + const findAllCoverageJobs = () => wrapper.findAllByTestId('job-coverage'); const createComponent = (props = {}) => { - wrapper = shallowMount(JobsTable, { - propsData: { - jobs: mockJobsInTable, - ...props, - }, - }); + wrapper = extendedWrapper( + mount(JobsTable, { + propsData: { + jobs: mockJobsInTable, + ...props, + }, + }), + ); }; beforeEach(() => { @@ -25,7 +34,31 @@ describe('Jobs Table', () => { wrapper.destroy(); }); - it('displays a table', () => { + it('displays the jobs table', () => { expect(findTable().exists()).toBe(true); }); + + it('displays correct number of job rows', () => { + expect(findTableRows()).toHaveLength(mockJobsInTable.length); + }); + + it('displays job status', () => { + expect(findStatusBadge().exists()).toBe(true); + }); + + it('displays the job stage and name', () => { + const firstJob = mockJobsInTable[0]; + + expect(findJobStage().text()).toBe(firstJob.stage.name); + expect(findJobName().text()).toBe(firstJob.name); + }); + + it('displays the coverage for only jobs that have coverage', () => { + const jobsThatHaveCoverage = mockJobsInTable.filter((job) => job.coverage !== null); + + jobsThatHaveCoverage.forEach((job, index) => { + expect(findAllCoverageJobs().at(index).text()).toBe(`${job.coverage}%`); + }); + expect(findAllCoverageJobs()).toHaveLength(jobsThatHaveCoverage.length); + }); }); diff --git a/spec/frontend/jobs/mock_data.js b/spec/frontend/jobs/mock_data.js index 1432c6d7e9b..360d17052a3 100644 --- a/spec/frontend/jobs/mock_data.js +++ b/spec/frontend/jobs/mock_data.js @@ -1292,6 +1292,7 @@ export const mockJobsInTable = [ title: 'Play', __typename: 'StatusAction', }, + detailsPath: '/root/ci-project/-/jobs/2004', __typename: 'DetailedStatus', }, id: 'gid://gitlab/Ci::Build/2004', @@ -1316,6 +1317,7 @@ export const mockJobsInTable = [ duration: null, finishedAt: null, coverage: null, + createdByTag: false, retryable: false, playable: true, cancelable: false, @@ -1353,6 +1355,7 @@ export const mockJobsInTable = [ duration: null, finishedAt: null, coverage: null, + createdByTag: true, retryable: false, playable: false, cancelable: false, @@ -1396,7 +1399,8 @@ export const mockJobsInTable = [ name: 'artifact_job', duration: 2, finishedAt: '2021-04-01T17:36:18Z', - coverage: null, + coverage: 82.71, + createdByTag: false, retryable: true, playable: false, cancelable: false, diff --git a/spec/frontend/pipelines/pipeline_multi_actions_spec.js b/spec/frontend/pipelines/pipeline_multi_actions_spec.js index bb110a8924f..88b3ef2032a 100644 --- a/spec/frontend/pipelines/pipeline_multi_actions_spec.js +++ b/spec/frontend/pipelines/pipeline_multi_actions_spec.js @@ -1,32 +1,46 @@ -import { GlDropdown, GlSprintf } from '@gitlab/ui'; +import { GlAlert, GlDropdown, GlSprintf } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; +import MockAdapter from 'axios-mock-adapter'; import { extendedWrapper } from 'helpers/vue_test_utils_helper'; -import PipelineMultiActions from '~/pipelines/components/pipelines_list/pipeline_multi_actions.vue'; +import waitForPromises from 'helpers/wait_for_promises'; +import axios from '~/lib/utils/axios_utils'; +import PipelineMultiActions, { + i18n, +} from '~/pipelines/components/pipelines_list/pipeline_multi_actions.vue'; describe('Pipeline Multi Actions Dropdown', () => { let wrapper; + let mockAxios; + const artifacts = [ + { + name: 'job my-artifact', + path: '/download/path', + }, + { + name: 'job-2 my-artifact-2', + path: '/download/path-two', + }, + ]; const artifactItemTestId = 'artifact-item'; + const artifactsEndpointPlaceholder = ':pipeline_artifacts_id'; + const artifactsEndpoint = `endpoint/${artifactsEndpointPlaceholder}/artifacts.json`; + const pipelineId = 108; - const defaultProps = { - artifacts: [ - { - name: 'job my-artifact', - path: '/download/path', - }, - { - name: 'job-2 my-artifact-2', - path: '/download/path-two', - }, - ], - }; - - const createComponent = (props = defaultProps) => { + const createComponent = ({ mockData = {} } = {}) => { wrapper = extendedWrapper( shallowMount(PipelineMultiActions, { + provide: { + artifactsEndpoint, + artifactsEndpointPlaceholder, + }, propsData: { - ...defaultProps, - ...props, + pipelineId, + }, + data() { + return { + ...mockData, + }; }, stubs: { GlSprintf, @@ -35,33 +49,64 @@ describe('Pipeline Multi Actions Dropdown', () => { ); }; + const findAlert = () => wrapper.findComponent(GlAlert); const findDropdown = () => wrapper.findComponent(GlDropdown); const findAllArtifactItems = () => wrapper.findAllByTestId(artifactItemTestId); const findFirstArtifactItem = () => wrapper.findByTestId(artifactItemTestId); beforeEach(() => { - createComponent(); + mockAxios = new MockAdapter(axios); }); afterEach(() => { + mockAxios.restore(); + wrapper.destroy(); }); it('should render the dropdown', () => { + createComponent(); + expect(findDropdown().exists()).toBe(true); }); describe('Artifacts', () => { + it('should fetch artifacts on dropdown click', async () => { + const endpoint = artifactsEndpoint.replace(artifactsEndpointPlaceholder, pipelineId); + mockAxios.onGet(endpoint).replyOnce(200, { artifacts }); + createComponent(); + findDropdown().vm.$emit('show'); + await waitForPromises(); + + expect(mockAxios.history.get).toHaveLength(1); + expect(wrapper.vm.artifacts).toEqual(artifacts); + }); + it('should render all the provided artifacts', () => { - expect(findAllArtifactItems()).toHaveLength(defaultProps.artifacts.length); + createComponent({ mockData: { artifacts } }); + + expect(findAllArtifactItems()).toHaveLength(artifacts.length); }); it('should render the correct artifact name and path', () => { - expect(findFirstArtifactItem().attributes('href')).toBe(defaultProps.artifacts[0].path); + createComponent({ mockData: { artifacts } }); - expect(findFirstArtifactItem().text()).toBe( - `Download ${defaultProps.artifacts[0].name} artifact`, - ); + expect(findFirstArtifactItem().attributes('href')).toBe(artifacts[0].path); + expect(findFirstArtifactItem().text()).toBe(`Download ${artifacts[0].name} artifact`); + }); + + describe('with a failing request', () => { + it('should render an error message', async () => { + const endpoint = artifactsEndpoint.replace(artifactsEndpointPlaceholder, pipelineId); + mockAxios.onGet(endpoint).replyOnce(500); + createComponent(); + findDropdown().vm.$emit('show'); + await waitForPromises(); + + const error = findAlert(); + expect(error.exists()).toBe(true); + expect(error.text()).toBe(i18n.artifactsFetchErrorMessage); + }); }); }); }); diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 817c3769b80..033fecf5113 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -568,6 +568,7 @@ project: - debian_distributions - merge_request_metrics - security_orchestration_policy_configuration +- timelogs award_emoji: - awardable - user diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index b159d0cfc76..825aa30e594 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -640,6 +640,7 @@ Timelog: - time_spent - merge_request_id - user_id +- project_id - spent_at - created_at - updated_at diff --git a/spec/lib/gitlab/pagination/keyset/iterator_spec.rb b/spec/lib/gitlab/pagination/keyset/iterator_spec.rb new file mode 100644 index 00000000000..656ae73945e --- /dev/null +++ b/spec/lib/gitlab/pagination/keyset/iterator_spec.rb @@ -0,0 +1,127 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Pagination::Keyset::Iterator do + let_it_be(:project) { create(:project) } + let_it_be(:issue_list_with_same_pos) { create_list(:issue, 3, project: project, relative_position: 100, updated_at: 1.day.ago) } + let_it_be(:issue_list_with_null_pos) { create_list(:issue, 3, project: project, relative_position: nil, updated_at: 1.day.ago) } + let_it_be(:issue_list_with_asc_pos) { create_list(:issue, 3, :with_asc_relative_position, project: project, updated_at: 1.day.ago) } + + let(:klass) { Issue } + let(:column) { 'relative_position' } + let(:direction) { :asc } + let(:reverse_direction) { ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::REVERSED_ORDER_DIRECTIONS[direction] } + let(:nulls_position) { :nulls_last } + let(:reverse_nulls_position) { ::Gitlab::Pagination::Keyset::ColumnOrderDefinition::REVERSED_NULL_POSITIONS[nulls_position] } + let(:custom_reorder) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: column, + column_expression: klass.arel_table[column], + order_expression: ::Gitlab::Database.nulls_order(column, direction, nulls_position), + reversed_order_expression: ::Gitlab::Database.nulls_order(column, reverse_direction, reverse_nulls_position), + order_direction: direction, + nullable: nulls_position, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + order_expression: klass.arel_table[:id].send(direction), + add_to_projections: true + ) + ]) + end + + let(:scope) { project.issues.reorder(custom_reorder) } + + subject { described_class.new(scope: scope) } + + describe '.each_batch' do + it 'yields an ActiveRecord::Relation when a block is given' do + subject.each_batch(of: 1) do |relation| + expect(relation).to be_a_kind_of(ActiveRecord::Relation) + end + end + + it 'accepts a custom batch size' do + count = 0 + + subject.each_batch(of: 2) { |relation| count += relation.count(:all) } + + expect(count).to eq(9) + end + + it 'allows updating of the yielded relations' do + time = Time.current + + subject.each_batch(of: 2) do |relation| + relation.update_all(updated_at: time) + end + + expect(Issue.where(updated_at: time).count).to eq(9) + end + + context 'with ordering direction' do + context 'when ordering asc' do + it 'orders ascending by default, including secondary order column' do + positions = [] + + subject.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } + + expect(positions).to eq(project.issues.order_relative_position_asc.order(id: :asc).pluck(:relative_position, :id)) + end + end + + context 'when reversing asc order' do + let(:scope) { project.issues.order(custom_reorder.reversed_order) } + + it 'orders in reverse of ascending' do + positions = [] + + subject.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } + + expect(positions).to eq(project.issues.order_relative_position_desc.order(id: :desc).pluck(:relative_position, :id)) + end + end + + context 'when asc order, with nulls first' do + let(:nulls_position) { :nulls_first } + + it 'orders ascending with nulls first' do + positions = [] + + subject.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } + + expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_first_order('relative_position', 'ASC')).order(id: :asc).pluck(:relative_position, :id)) + end + end + + context 'when ordering desc' do + let(:direction) { :desc } + let(:nulls_position) { :nulls_last } + + it 'orders descending' do + positions = [] + + subject.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:relative_position, :id)) } + + expect(positions).to eq(project.issues.reorder(::Gitlab::Database.nulls_last_order('relative_position', 'DESC')).order(id: :desc).pluck(:relative_position, :id)) + end + end + + context 'when ordering by columns are repeated twice' do + let(:direction) { :desc } + let(:column) { :id } + + it 'orders descending' do + positions = [] + + subject.each_batch(of: 2) { |rel| positions.concat(rel.pluck(:id)) } + + expect(positions).to eq(project.issues.reorder(id: :desc).pluck(:id)) + end + end + end + end +end diff --git a/spec/lib/gitlab/pagination/keyset/order_spec.rb b/spec/lib/gitlab/pagination/keyset/order_spec.rb index 06a8aee1048..26f52745b54 100644 --- a/spec/lib/gitlab/pagination/keyset/order_spec.rb +++ b/spec/lib/gitlab/pagination/keyset/order_spec.rb @@ -3,76 +3,77 @@ require 'spec_helper' RSpec.describe Gitlab::Pagination::Keyset::Order do - let(:table) { Arel::Table.new(:my_table) } - let(:order) { nil } + describe 'paginate over items correctly' do + let(:table) { Arel::Table.new(:my_table) } + let(:order) { nil } - def run_query(query) - ActiveRecord::Base.connection.execute(query).to_a - end + def run_query(query) + ActiveRecord::Base.connection.execute(query).to_a + end - def build_query(order:, where_conditions: nil, limit: nil) - <<-SQL + def build_query(order:, where_conditions: nil, limit: nil) + <<-SQL SELECT id, year, month FROM (#{table_data}) my_table (id, year, month) WHERE #{where_conditions || '1=1'} ORDER BY #{order} LIMIT #{limit || 999}; - SQL - end - - def iterate_and_collect(order:, page_size:, where_conditions: nil) - all_items = [] - - loop do - paginated_items = run_query(build_query(order: order, where_conditions: where_conditions, limit: page_size)) - break if paginated_items.empty? - - all_items.concat(paginated_items) - last_item = paginated_items.last - cursor_attributes = order.cursor_attributes_for_node(last_item) - where_conditions = order.build_where_values(cursor_attributes).to_sql + SQL end - all_items - end + def iterate_and_collect(order:, page_size:, where_conditions: nil) + all_items = [] - subject do - run_query(build_query(order: order)) - end + loop do + paginated_items = run_query(build_query(order: order, where_conditions: where_conditions, limit: page_size)) + break if paginated_items.empty? - shared_examples 'order examples' do - it { expect(subject).to eq(expected) } + all_items.concat(paginated_items) + last_item = paginated_items.last + cursor_attributes = order.cursor_attributes_for_node(last_item) + where_conditions = order.where_values_with_or_query(cursor_attributes).to_sql + end - context 'when paginating forwards' do - subject { iterate_and_collect(order: order, page_size: 2) } + all_items + end + subject do + run_query(build_query(order: order)) + end + + shared_examples 'order examples' do it { expect(subject).to eq(expected) } - context 'with different page size' do - subject { iterate_and_collect(order: order, page_size: 5) } + context 'when paginating forwards' do + subject { iterate_and_collect(order: order, page_size: 2) } it { expect(subject).to eq(expected) } + + context 'with different page size' do + subject { iterate_and_collect(order: order, page_size: 5) } + + it { expect(subject).to eq(expected) } + end + end + + context 'when paginating backwards' do + subject do + last_item = expected.last + cursor_attributes = order.cursor_attributes_for_node(last_item) + where_conditions = order.reversed_order.where_values_with_or_query(cursor_attributes) + + iterate_and_collect(order: order.reversed_order, page_size: 2, where_conditions: where_conditions.to_sql) + end + + it do + expect(subject).to eq(expected.reverse[1..-1]) # removing one item because we used it to calculate cursor data for the "last" page in subject + end end end - context 'when paginating backwards' do - subject do - last_item = expected.last - cursor_attributes = order.cursor_attributes_for_node(last_item) - where_conditions = order.reversed_order.build_where_values(cursor_attributes) - - iterate_and_collect(order: order.reversed_order, page_size: 2, where_conditions: where_conditions.to_sql) - end - - it do - expect(subject).to eq(expected.reverse[1..-1]) # removing one item because we used it to calculate cursor data for the "last" page in subject - end - end - end - - context 'when ordering by a distinct column' do - let(:table_data) do - <<-SQL + context 'when ordering by a distinct column' do + let(:table_data) do + <<-SQL VALUES (1, 0, 0), (2, 0, 0), (3, 0, 0), @@ -82,41 +83,41 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do (7, 0, 0), (8, 0, 0), (9, 0, 0) - SQL + SQL + end + + let(:order) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + column_expression: table['id'], + order_expression: table['id'].desc, + nullable: :not_nullable, + distinct: true + ) + ]) + end + + let(:expected) do + [ + { "id" => 9, "year" => 0, "month" => 0 }, + { "id" => 8, "year" => 0, "month" => 0 }, + { "id" => 7, "year" => 0, "month" => 0 }, + { "id" => 6, "year" => 0, "month" => 0 }, + { "id" => 5, "year" => 0, "month" => 0 }, + { "id" => 4, "year" => 0, "month" => 0 }, + { "id" => 3, "year" => 0, "month" => 0 }, + { "id" => 2, "year" => 0, "month" => 0 }, + { "id" => 1, "year" => 0, "month" => 0 } + ] + end + + it_behaves_like 'order examples' end - let(:order) do - Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - column_expression: table['id'], - order_expression: table['id'].desc, - nullable: :not_nullable, - distinct: true - ) - ]) - end - - let(:expected) do - [ - { "id" => 9, "year" => 0, "month" => 0 }, - { "id" => 8, "year" => 0, "month" => 0 }, - { "id" => 7, "year" => 0, "month" => 0 }, - { "id" => 6, "year" => 0, "month" => 0 }, - { "id" => 5, "year" => 0, "month" => 0 }, - { "id" => 4, "year" => 0, "month" => 0 }, - { "id" => 3, "year" => 0, "month" => 0 }, - { "id" => 2, "year" => 0, "month" => 0 }, - { "id" => 1, "year" => 0, "month" => 0 } - ] - end - - it_behaves_like 'order examples' - end - - context 'when ordering by two non-nullable columns and a distinct column' do - let(:table_data) do - <<-SQL + context 'when ordering by two non-nullable columns and a distinct column' do + let(:table_data) do + <<-SQL VALUES (1, 2010, 2), (2, 2011, 1), (3, 2009, 2), @@ -126,55 +127,55 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do (7, 2010, 3), (8, 2012, 4), (9, 2013, 5) - SQL + SQL + end + + let(:order) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'year', + column_expression: table['year'], + order_expression: table['year'].asc, + nullable: :not_nullable, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'month', + column_expression: table['month'], + order_expression: table['month'].asc, + nullable: :not_nullable, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + column_expression: table['id'], + order_expression: table['id'].asc, + nullable: :not_nullable, + distinct: true + ) + ]) + end + + let(:expected) do + [ + { 'year' => 2009, 'month' => 2, 'id' => 3 }, + { 'year' => 2009, 'month' => 2, 'id' => 6 }, + { 'year' => 2010, 'month' => 2, 'id' => 1 }, + { 'year' => 2010, 'month' => 3, 'id' => 7 }, + { 'year' => 2011, 'month' => 1, 'id' => 2 }, + { 'year' => 2011, 'month' => 1, 'id' => 4 }, + { 'year' => 2011, 'month' => 1, 'id' => 5 }, + { 'year' => 2012, 'month' => 4, 'id' => 8 }, + { 'year' => 2013, 'month' => 5, 'id' => 9 } + ] + end + + it_behaves_like 'order examples' end - let(:order) do - Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'year', - column_expression: table['year'], - order_expression: table['year'].asc, - nullable: :not_nullable, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'month', - column_expression: table['month'], - order_expression: table['month'].asc, - nullable: :not_nullable, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - column_expression: table['id'], - order_expression: table['id'].asc, - nullable: :not_nullable, - distinct: true - ) - ]) - end - - let(:expected) do - [ - { 'year' => 2009, 'month' => 2, 'id' => 3 }, - { 'year' => 2009, 'month' => 2, 'id' => 6 }, - { 'year' => 2010, 'month' => 2, 'id' => 1 }, - { 'year' => 2010, 'month' => 3, 'id' => 7 }, - { 'year' => 2011, 'month' => 1, 'id' => 2 }, - { 'year' => 2011, 'month' => 1, 'id' => 4 }, - { 'year' => 2011, 'month' => 1, 'id' => 5 }, - { 'year' => 2012, 'month' => 4, 'id' => 8 }, - { 'year' => 2013, 'month' => 5, 'id' => 9 } - ] - end - - it_behaves_like 'order examples' - end - - context 'when ordering by nullable columns and a distinct column' do - let(:table_data) do - <<-SQL + context 'when ordering by nullable columns and a distinct column' do + let(:table_data) do + <<-SQL VALUES (1, 2010, null), (2, 2011, 2), (3, null, null), @@ -186,61 +187,61 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do (9, null, 2), (10, null, null), (11, 2010, 2) - SQL + SQL + end + + let(:order) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'year', + column_expression: table['year'], + order_expression: Gitlab::Database.nulls_last_order('year', :asc), + reversed_order_expression: Gitlab::Database.nulls_first_order('year', :desc), + order_direction: :asc, + nullable: :nulls_last, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'month', + column_expression: table['month'], + order_expression: Gitlab::Database.nulls_last_order('month', :asc), + reversed_order_expression: Gitlab::Database.nulls_first_order('month', :desc), + order_direction: :asc, + nullable: :nulls_last, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + column_expression: table['id'], + order_expression: table['id'].asc, + nullable: :not_nullable, + distinct: true + ) + ]) + end + + let(:expected) do + [ + { "id" => 7, "year" => 2010, "month" => 2 }, + { "id" => 11, "year" => 2010, "month" => 2 }, + { "id" => 1, "year" => 2010, "month" => nil }, + { "id" => 5, "year" => 2010, "month" => nil }, + { "id" => 2, "year" => 2011, "month" => 2 }, + { "id" => 6, "year" => 2011, "month" => 2 }, + { "id" => 8, "year" => 2012, "month" => 2 }, + { "id" => 9, "year" => nil, "month" => 2 }, + { "id" => 4, "year" => nil, "month" => 5 }, + { "id" => 3, "year" => nil, "month" => nil }, + { "id" => 10, "year" => nil, "month" => nil } + ] + end + + it_behaves_like 'order examples' end - let(:order) do - Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'year', - column_expression: table['year'], - order_expression: Gitlab::Database.nulls_last_order('year', :asc), - reversed_order_expression: Gitlab::Database.nulls_first_order('year', :desc), - order_direction: :asc, - nullable: :nulls_last, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'month', - column_expression: table['month'], - order_expression: Gitlab::Database.nulls_last_order('month', :asc), - reversed_order_expression: Gitlab::Database.nulls_first_order('month', :desc), - order_direction: :asc, - nullable: :nulls_last, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - column_expression: table['id'], - order_expression: table['id'].asc, - nullable: :not_nullable, - distinct: true - ) - ]) - end - - let(:expected) do - [ - { "id" => 7, "year" => 2010, "month" => 2 }, - { "id" => 11, "year" => 2010, "month" => 2 }, - { "id" => 1, "year" => 2010, "month" => nil }, - { "id" => 5, "year" => 2010, "month" => nil }, - { "id" => 2, "year" => 2011, "month" => 2 }, - { "id" => 6, "year" => 2011, "month" => 2 }, - { "id" => 8, "year" => 2012, "month" => 2 }, - { "id" => 9, "year" => nil, "month" => 2 }, - { "id" => 4, "year" => nil, "month" => 5 }, - { "id" => 3, "year" => nil, "month" => nil }, - { "id" => 10, "year" => nil, "month" => nil } - ] - end - - it_behaves_like 'order examples' - end - - context 'when ordering by nullable columns with nulls first ordering and a distinct column' do - let(:table_data) do - <<-SQL + context 'when ordering by nullable columns with nulls first ordering and a distinct column' do + let(:table_data) do + <<-SQL VALUES (1, 2010, null), (2, 2011, 2), (3, null, null), @@ -252,61 +253,61 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do (9, null, 2), (10, null, null), (11, 2010, 2) - SQL + SQL + end + + let(:order) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'year', + column_expression: table['year'], + order_expression: Gitlab::Database.nulls_first_order('year', :asc), + reversed_order_expression: Gitlab::Database.nulls_last_order('year', :desc), + order_direction: :asc, + nullable: :nulls_first, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'month', + column_expression: table['month'], + order_expression: Gitlab::Database.nulls_first_order('month', :asc), + order_direction: :asc, + reversed_order_expression: Gitlab::Database.nulls_last_order('month', :desc), + nullable: :nulls_first, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + column_expression: table['id'], + order_expression: table['id'].asc, + nullable: :not_nullable, + distinct: true + ) + ]) + end + + let(:expected) do + [ + { "id" => 3, "year" => nil, "month" => nil }, + { "id" => 10, "year" => nil, "month" => nil }, + { "id" => 9, "year" => nil, "month" => 2 }, + { "id" => 4, "year" => nil, "month" => 5 }, + { "id" => 1, "year" => 2010, "month" => nil }, + { "id" => 5, "year" => 2010, "month" => nil }, + { "id" => 7, "year" => 2010, "month" => 2 }, + { "id" => 11, "year" => 2010, "month" => 2 }, + { "id" => 2, "year" => 2011, "month" => 2 }, + { "id" => 6, "year" => 2011, "month" => 2 }, + { "id" => 8, "year" => 2012, "month" => 2 } + ] + end + + it_behaves_like 'order examples' end - let(:order) do - Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'year', - column_expression: table['year'], - order_expression: Gitlab::Database.nulls_first_order('year', :asc), - reversed_order_expression: Gitlab::Database.nulls_last_order('year', :desc), - order_direction: :asc, - nullable: :nulls_first, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'month', - column_expression: table['month'], - order_expression: Gitlab::Database.nulls_first_order('month', :asc), - order_direction: :asc, - reversed_order_expression: Gitlab::Database.nulls_last_order('month', :desc), - nullable: :nulls_first, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - column_expression: table['id'], - order_expression: table['id'].asc, - nullable: :not_nullable, - distinct: true - ) - ]) - end - - let(:expected) do - [ - { "id" => 3, "year" => nil, "month" => nil }, - { "id" => 10, "year" => nil, "month" => nil }, - { "id" => 9, "year" => nil, "month" => 2 }, - { "id" => 4, "year" => nil, "month" => 5 }, - { "id" => 1, "year" => 2010, "month" => nil }, - { "id" => 5, "year" => 2010, "month" => nil }, - { "id" => 7, "year" => 2010, "month" => 2 }, - { "id" => 11, "year" => 2010, "month" => 2 }, - { "id" => 2, "year" => 2011, "month" => 2 }, - { "id" => 6, "year" => 2011, "month" => 2 }, - { "id" => 8, "year" => 2012, "month" => 2 } - ] - end - - it_behaves_like 'order examples' - end - - context 'when ordering by non-nullable columns with mixed directions and a distinct column' do - let(:table_data) do - <<-SQL + context 'when ordering by non-nullable columns with mixed directions and a distinct column' do + let(:table_data) do + <<-SQL VALUES (1, 2010, 0), (2, 2011, 0), (3, 2010, 0), @@ -318,158 +319,216 @@ RSpec.describe Gitlab::Pagination::Keyset::Order do (9, 2013, 0), (10, 2014, 0), (11, 2013, 0) - SQL - end - - let(:order) do - Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'year', - column_expression: table['year'], - order_expression: table['year'].asc, - nullable: :not_nullable, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - column_expression: table['id'], - order_expression: table['id'].desc, - nullable: :not_nullable, - distinct: true - ) - ]) - end - - let(:expected) do - [ - { "id" => 7, "year" => 2010, "month" => 0 }, - { "id" => 4, "year" => 2010, "month" => 0 }, - { "id" => 3, "year" => 2010, "month" => 0 }, - { "id" => 1, "year" => 2010, "month" => 0 }, - { "id" => 8, "year" => 2011, "month" => 0 }, - { "id" => 2, "year" => 2011, "month" => 0 }, - { "id" => 6, "year" => 2012, "month" => 0 }, - { "id" => 5, "year" => 2012, "month" => 0 }, - { "id" => 11, "year" => 2013, "month" => 0 }, - { "id" => 9, "year" => 2013, "month" => 0 }, - { "id" => 10, "year" => 2014, "month" => 0 } - ] - end - - it 'takes out a slice between two cursors' do - after_cursor = { "id" => 8, "year" => 2011 } - before_cursor = { "id" => 5, "year" => 2012 } - - after_conditions = order.build_where_values(after_cursor) - reversed = order.reversed_order - before_conditions = reversed.build_where_values(before_cursor) - - query = build_query(order: order, where_conditions: "(#{after_conditions.to_sql}) AND (#{before_conditions.to_sql})", limit: 100) - - expect(run_query(query)).to eq([ - { "id" => 2, "year" => 2011, "month" => 0 }, - { "id" => 6, "year" => 2012, "month" => 0 } - ]) - end - end - - context 'when the passed cursor values do not match with the order definition' do - let(:order) do - Gitlab::Pagination::Keyset::Order.build([ - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'year', - column_expression: table['year'], - order_expression: table['year'].asc, - nullable: :not_nullable, - distinct: false - ), - Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: 'id', - column_expression: table['id'], - order_expression: table['id'].desc, - nullable: :not_nullable, - distinct: true - ) - ]) - end - - context 'when values are missing' do - it 'raises error' do - expect { order.build_where_values(id: 1) }.to raise_error(/Missing items: year/) - end - end - - context 'when extra values are present' do - it 'raises error' do - expect { order.build_where_values(id: 1, year: 2, foo: 3) }.to raise_error(/Extra items: foo/) - end - end - - context 'when values are missing and extra values are present' do - it 'raises error' do - expect { order.build_where_values(year: 2, foo: 3) }.to raise_error(/Extra items: foo\. Missing items: id/) - end - end - - context 'when no values are passed' do - it 'returns nil' do - expect(order.build_where_values({})).to eq(nil) - end - end - end - - context 'extract and apply cursor attributes' do - let(:model) { Project.new(id: 100) } - let(:scope) { Project.all } - - shared_examples 'cursor attribute examples' do - describe '#cursor_attributes_for_node' do - it { expect(order.cursor_attributes_for_node(model)).to eq({ id: '100' }.with_indifferent_access) } + SQL end - describe '#apply_cursor_conditions' do - context 'when params with string keys are passed' do - subject(:sql) { order.apply_cursor_conditions(scope, { 'id' => '100' }).to_sql } - - it { is_expected.to include('"projects"."id" < 100)') } - end - - context 'when params with symbol keys are passed' do - subject(:sql) { order.apply_cursor_conditions(scope, { id: '100' }).to_sql } - - it { is_expected.to include('"projects"."id" < 100)') } - end - end - end - - context 'when string attribute name is given' do let(:order) do Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'year', + column_expression: table['year'], + order_expression: table['year'].asc, + nullable: :not_nullable, + distinct: false + ), Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( attribute_name: 'id', - order_expression: Project.arel_table['id'].desc, + column_expression: table['id'], + order_expression: table['id'].desc, nullable: :not_nullable, distinct: true ) ]) end - it_behaves_like 'cursor attribute examples' + let(:expected) do + [ + { "id" => 7, "year" => 2010, "month" => 0 }, + { "id" => 4, "year" => 2010, "month" => 0 }, + { "id" => 3, "year" => 2010, "month" => 0 }, + { "id" => 1, "year" => 2010, "month" => 0 }, + { "id" => 8, "year" => 2011, "month" => 0 }, + { "id" => 2, "year" => 2011, "month" => 0 }, + { "id" => 6, "year" => 2012, "month" => 0 }, + { "id" => 5, "year" => 2012, "month" => 0 }, + { "id" => 11, "year" => 2013, "month" => 0 }, + { "id" => 9, "year" => 2013, "month" => 0 }, + { "id" => 10, "year" => 2014, "month" => 0 } + ] + end + + it 'takes out a slice between two cursors' do + after_cursor = { "id" => 8, "year" => 2011 } + before_cursor = { "id" => 5, "year" => 2012 } + + after_conditions = order.where_values_with_or_query(after_cursor) + reversed = order.reversed_order + before_conditions = reversed.where_values_with_or_query(before_cursor) + + query = build_query(order: order, where_conditions: "(#{after_conditions.to_sql}) AND (#{before_conditions.to_sql})", limit: 100) + + expect(run_query(query)).to eq([ + { "id" => 2, "year" => 2011, "month" => 0 }, + { "id" => 6, "year" => 2012, "month" => 0 } + ]) + end end - context 'when symbol attribute name is given' do + context 'when the passed cursor values do not match with the order definition' do let(:order) do Gitlab::Pagination::Keyset::Order.build([ Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( - attribute_name: :id, - order_expression: Project.arel_table['id'].desc, + attribute_name: 'year', + column_expression: table['year'], + order_expression: table['year'].asc, + nullable: :not_nullable, + distinct: false + ), + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + column_expression: table['id'], + order_expression: table['id'].desc, nullable: :not_nullable, distinct: true ) ]) end - it_behaves_like 'cursor attribute examples' + context 'when values are missing' do + it 'raises error' do + expect { order.build_where_values(id: 1) }.to raise_error(/Missing items: year/) + end + end + + context 'when extra values are present' do + it 'raises error' do + expect { order.build_where_values(id: 1, year: 2, foo: 3) }.to raise_error(/Extra items: foo/) + end + end + + context 'when values are missing and extra values are present' do + it 'raises error' do + expect { order.build_where_values(year: 2, foo: 3) }.to raise_error(/Extra items: foo\. Missing items: id/) + end + end + + context 'when no values are passed' do + it 'returns empty array' do + expect(order.build_where_values({})).to eq([]) + end + end + end + + context 'extract and apply cursor attributes' do + let(:model) { Project.new(id: 100) } + let(:scope) { Project.all } + + shared_examples 'cursor attribute examples' do + describe '#cursor_attributes_for_node' do + it { expect(order.cursor_attributes_for_node(model)).to eq({ id: '100' }.with_indifferent_access) } + end + + describe '#apply_cursor_conditions' do + context 'when params with string keys are passed' do + subject(:sql) { order.apply_cursor_conditions(scope, { 'id' => '100' }).to_sql } + + it { is_expected.to include('"projects"."id" < 100)') } + end + + context 'when params with symbol keys are passed' do + subject(:sql) { order.apply_cursor_conditions(scope, { id: '100' }).to_sql } + + it { is_expected.to include('"projects"."id" < 100)') } + end + end + end + + context 'when string attribute name is given' do + let(:order) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: 'id', + order_expression: Project.arel_table['id'].desc, + nullable: :not_nullable, + distinct: true + ) + ]) + end + + it_behaves_like 'cursor attribute examples' + end + + context 'when symbol attribute name is given' do + let(:order) do + Gitlab::Pagination::Keyset::Order.build([ + Gitlab::Pagination::Keyset::ColumnOrderDefinition.new( + attribute_name: :id, + order_expression: Project.arel_table['id'].desc, + nullable: :not_nullable, + distinct: true + ) + ]) + end + + it_behaves_like 'cursor attribute examples' + end + end + end + + describe 'UNION optimization' do + let_it_be(:five_months_ago) { 5.months.ago } + + let_it_be(:user_1) { create(:user, created_at: five_months_ago) } + let_it_be(:user_2) { create(:user, created_at: five_months_ago) } + let_it_be(:user_3) { create(:user, created_at: 1.month.ago) } + let_it_be(:user_4) { create(:user, created_at: 2.months.ago) } + + let(:expected_results) { [user_3, user_4, user_2, user_1] } + let(:scope) { User.order(created_at: :desc, id: :desc) } + let(:keyset_aware_scope) { Gitlab::Pagination::Keyset::SimpleOrderBuilder.build(scope).first } + let(:iterator_options) { { scope: keyset_aware_scope } } + + subject(:items) do + [].tap do |collector| + Gitlab::Pagination::Keyset::Iterator.new(**iterator_options).each_batch(of: 2) do |models| + collector.concat(models) + end + end + end + + context 'when UNION optimization is off' do + it 'returns items in the correct order' do + iterator_options[:use_union_optimization] = false + + expect(items).to eq(expected_results) + end + end + + context 'when UNION optimization is on' do + before do + iterator_options[:use_union_optimization] = true + end + + it 'returns items in the correct order' do + expect(items).to eq(expected_results) + end + + it 'calls Gitlab::SQL::Union' do + expect_next_instances_of(Gitlab::SQL::Union, 2) do |instance| + expect(instance.send(:remove_order)).to eq(false) # Do not remove order from the queries + expect(instance.send(:remove_duplicates)).to eq(false) # Do not deduplicate the results + end + + items + end + + it 'builds UNION query' do + cursor_attributes = { created_at: five_months_ago, id: user_2.id } + order = Gitlab::Pagination::Keyset::Order.extract_keyset_order_object(keyset_aware_scope) + + query = order.apply_cursor_conditions(scope, cursor_attributes, use_union_optimization: true).to_sql + expect(query).to include('UNION ALL') + end end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index dc1c418bea9..008af34573d 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -130,6 +130,7 @@ RSpec.describe Project, factory_default: :keep do it { is_expected.to have_many(:debian_distributions).class_name('Packages::Debian::ProjectDistribution').dependent(:destroy) } it { is_expected.to have_many(:pipeline_artifacts) } it { is_expected.to have_many(:terraform_states).class_name('Terraform::State').inverse_of(:project) } + it { is_expected.to have_many(:timelogs) } # GitLab Pages it { is_expected.to have_many(:pages_domains) } diff --git a/spec/models/timelog_spec.rb b/spec/models/timelog_spec.rb index 6a252b444f9..31eeb36d1ee 100644 --- a/spec/models/timelog_spec.rb +++ b/spec/models/timelog_spec.rb @@ -3,11 +3,12 @@ require 'spec_helper' RSpec.describe Timelog do - subject { build(:timelog) } + subject { create(:timelog) } let(:issue) { create(:issue) } let(:merge_request) { create(:merge_request) } + it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:issue).touch(true) } it { is_expected.to belong_to(:merge_request).touch(true) } @@ -16,6 +17,8 @@ RSpec.describe Timelog do it { is_expected.to validate_presence_of(:time_spent) } it { is_expected.to validate_presence_of(:user) } + it { expect(subject.project_id).not_to be_nil } + describe 'Issuable validation' do it 'is invalid if issue_id and merge_request_id are missing' do subject.attributes = { issue: nil, merge_request: nil } diff --git a/yarn.lock b/yarn.lock index ac0b4ce2c1c..93920f1bcaf 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3132,7 +3132,7 @@ commander@2, commander@^2.10.0, commander@^2.18.0, commander@^2.19.0, commander@ resolved "https://registry.yarnpkg.com/commander/-/commander-2.20.0.tgz#d58bb2b5c1ee8f87b0d340027e9e94e222c5a422" integrity sha512-7j2y+40w61zy6YC2iRNpUe/NwhNyoXrYpHMrSunaMG64nRnaf96zO/KMQR4OyN/UnE5KLyEBnKHd4aG3rskjpQ== -commander@^6.2.0: +commander@^6.0.0, commander@^6.2.0: version "6.2.1" resolved "https://registry.yarnpkg.com/commander/-/commander-6.2.1.tgz#0792eb682dfbc325999bb2b84fddddba110ac73c" integrity sha512-U7VdrJFnJgo4xjrHpTzu0yrHPGImdsmD95ZlgYSEajAn2JKzDhDTPG9kBTefmObL2w/ngeZnilk+OV9CG3d7UA== @@ -7459,12 +7459,12 @@ karma@^4.2.0: tmp "0.0.33" useragent "2.3.0" -katex@^0.10.0: - version "0.10.2" - resolved "https://registry.yarnpkg.com/katex/-/katex-0.10.2.tgz#39973edbb65eda5b6f9e7f41648781e557dd4932" - integrity sha512-cQOmyIRoMloCoSIOZ1+gEwsksdJZ1EW4SWm3QzxSza/QsnZr6D4U1V9S4q+B/OLm2OQ8TCBecQ8MaIfnScI7cw== +katex@^0.13.2: + version "0.13.2" + resolved "https://registry.yarnpkg.com/katex/-/katex-0.13.2.tgz#4075b9144e6af992ec9a4b772fa3754763be5f26" + integrity sha512-u/KhjFDhyPr+70aiBn9SL/9w/QlLagIXBi2NZSbNnBUp2tR8dCjQplyEMkEzniem5gOeSCBjlBUg4VaiWs1JJg== dependencies: - commander "^2.19.0" + commander "^6.0.0" keyv@^3.0.0: version "3.1.0"