From a19a376bf35b2009566e86b8190662c21ed7e2ba Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Mon, 2 Dec 2019 21:06:51 +0000 Subject: Add latest changes from gitlab-org/gitlab@master --- .gitlab-ci.yml | 2 +- .gitlab/ci/rails.gitlab-ci.yml | 26 ++++- .../javascripts/deploy_keys/components/app.vue | 2 +- .../list/components/collapsible_container.vue | 9 +- .../registry/list/components/table_registry.vue | 5 +- app/helpers/issuables_helper.rb | 4 + app/models/active_session.rb | 2 +- app/models/project.rb | 6 +- .../unreleased/hly-search-by-project-full-path.yml | 5 + doc/development/fe_guide/style/javascript.md | 74 +++++-------- doc/development/pipelines.md | 4 +- doc/user/project/img/service_desk_disabled.png | Bin 11708 -> 25013 bytes doc/user/project/img/service_desk_enabled.png | Bin 21514 -> 59684 bytes doc/user/project/service_desk.md | 5 +- lib/api/helpers/pagination.rb | 16 +-- lib/api/projects.rb | 6 +- lib/gitlab/pagination/keyset.rb | 21 ---- lib/gitlab/pagination/keyset/page.rb | 44 -------- lib/gitlab/pagination/keyset/pager.rb | 56 ---------- lib/gitlab/pagination/keyset/request_context.rb | 89 ---------------- lib/gitlab/sql/pattern.rb | 10 +- lib/quality/test_level.rb | 8 ++ locale/gitlab.pot | 18 ++++ qa/qa/page/mattermost/main.rb | 5 - qa/qa/page/project/settings/deploy_keys.rb | 2 +- scripts/review_apps/base-config.yaml | 16 +-- .../list/components/collapsible_container_spec.js | 28 ++--- .../list/components/table_registry_spec.js | 48 ++++----- spec/lib/api/helpers/pagination_spec.rb | 44 +------- spec/lib/gitlab/pagination/keyset/page_spec.rb | 66 ------------ spec/lib/gitlab/pagination/keyset/pager_spec.rb | 68 ------------ .../pagination/keyset/request_context_spec.rb | 115 --------------------- spec/lib/gitlab/pagination/keyset_spec.rb | 61 ----------- spec/lib/gitlab/sql/pattern_spec.rb | 10 ++ spec/lib/quality/test_level_spec.rb | 26 +++++ spec/models/active_session_spec.rb | 22 ++++ spec/models/project_spec.rb | 35 ++++++- spec/requests/api/projects_spec.rb | 81 --------------- spec/support/database_cleaner.rb | 33 ++++++ .../tasks/gitlab/import_export/import_rake_spec.rb | 8 +- 40 files changed, 281 insertions(+), 799 deletions(-) create mode 100644 changelogs/unreleased/hly-search-by-project-full-path.yml delete mode 100644 lib/gitlab/pagination/keyset.rb delete mode 100644 lib/gitlab/pagination/keyset/page.rb delete mode 100644 lib/gitlab/pagination/keyset/pager.rb delete mode 100644 lib/gitlab/pagination/keyset/request_context.rb delete mode 100644 spec/lib/gitlab/pagination/keyset/page_spec.rb delete mode 100644 spec/lib/gitlab/pagination/keyset/pager_spec.rb delete mode 100644 spec/lib/gitlab/pagination/keyset/request_context_spec.rb delete mode 100644 spec/lib/gitlab/pagination/keyset_spec.rb diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c12c64c55f6..8143c9e554a 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ -image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6.3-golang-1.11-git-2.22-chrome-73.0-node-12.x-yarn-1.16-postgresql-9.6-graphicsmagick-1.3.33" +image: "registry.gitlab.com/gitlab-org/gitlab-build-images:ruby-2.6.3-golang-1.12-git-2.22-chrome-73.0-node-12.x-yarn-1.16-postgresql-9.6-graphicsmagick-1.3.33" stages: - sync diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 78645f48b6f..4ac187e1670 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -92,13 +92,21 @@ setup-test-env: - .use-pg10 - .only-master +rspec migration pg9: + extends: .rspec-base-pg9 + parallel: 4 + +rspec migration pg9-foss: + extends: .rspec-base-pg9-foss + parallel: 4 + rspec unit pg9: extends: .rspec-base-pg9 - parallel: 24 + parallel: 20 rspec unit pg9-foss: extends: .rspec-base-pg9-foss - parallel: 24 + parallel: 20 rspec integration pg9: extends: .rspec-base-pg9 @@ -140,9 +148,13 @@ rspec system pg10: - .only-ee - .use-pg10-ee +rspec-ee migration pg9: + extends: .rspec-ee-base-pg9 + parallel: 2 + rspec-ee unit pg9: extends: .rspec-ee-base-pg9 - parallel: 7 + parallel: 5 rspec-ee integration pg9: extends: .rspec-ee-base-pg9 @@ -152,11 +164,17 @@ rspec-ee system pg9: extends: .rspec-ee-base-pg9 parallel: 5 +rspec-ee migration pg10: + extends: + - .rspec-ee-base-pg10 + - .only-master + parallel: 2 + rspec-ee unit pg10: extends: - .rspec-ee-base-pg10 - .only-master - parallel: 7 + parallel: 5 rspec-ee integration pg10: extends: diff --git a/app/assets/javascripts/deploy_keys/components/app.vue b/app/assets/javascripts/deploy_keys/components/app.vue index 922c907bb36..fd45e098758 100644 --- a/app/assets/javascripts/deploy_keys/components/app.vue +++ b/app/assets/javascripts/deploy_keys/components/app.vue @@ -133,7 +133,7 @@ export default { :keys="keys[currentTab]" :store="store" :endpoint="endpoint" - class="qa-project-deploy-keys" + data-qa-selector="project_deploy_keys" /> diff --git a/app/assets/javascripts/registry/list/components/collapsible_container.vue b/app/assets/javascripts/registry/list/components/collapsible_container.vue index 5a6f9370564..86bb2d8092e 100644 --- a/app/assets/javascripts/registry/list/components/collapsible_container.vue +++ b/app/assets/javascripts/registry/list/components/collapsible_container.vue @@ -31,7 +31,7 @@ export default { GlTooltip: GlTooltipDirective, GlModal: GlModalDirective, }, - mixins: [Tracking.mixin({})], + mixins: [Tracking.mixin()], props: { repo: { type: Object, @@ -43,7 +43,6 @@ export default { isOpen: false, modalId: `confirm-repo-deletion-modal-${this.repo.id}`, tracking: { - category: document.body.dataset.page, label: 'registry_repository_delete', }, }; @@ -67,7 +66,7 @@ export default { } }, handleDeleteRepository() { - this.track('confirm_delete', {}); + this.track('confirm_delete'); return this.deleteItem(this.repo) .then(() => { createFlash(__('This container registry has been scheduled for deletion.'), 'notice'); @@ -103,7 +102,7 @@ export default { :aria-label="s__('ContainerRegistry|Remove repository')" class="js-remove-repo btn-inverted" variant="danger" - @click="track('click_button', {})" + @click="track('click_button')" > @@ -132,7 +131,7 @@ export default { :modal-id="modalId" ok-variant="danger" @ok="handleDeleteRepository" - @cancel="track('cancel_delete', {})" + @cancel="track('cancel_delete')" >

records_for_page.size - apply_headers(records_for_page.last, there_is_more) - - records_for_page - end - - private - - def apply_headers(last_record_in_page, there_is_more) - end_reached = last_record_in_page.nil? || !there_is_more - lower_bounds = last_record_in_page&.slice(page.order_by.keys) - - next_page = page.next(lower_bounds, end_reached) - - request.apply_headers(next_page) - end - - def page - @page ||= request.page - end - - def validate_order!(rel) - present_order = rel.order_values.map { |val| [val.expr.name.to_sym, val.direction] }.last(2).to_h - - unless page.order_by == present_order - raise ArgumentError, "Page's order_by does not match the relation's order: #{present_order} vs #{page.order_by}" - end - end - end - end - end -end diff --git a/lib/gitlab/pagination/keyset/request_context.rb b/lib/gitlab/pagination/keyset/request_context.rb deleted file mode 100644 index aeaed7587b3..00000000000 --- a/lib/gitlab/pagination/keyset/request_context.rb +++ /dev/null @@ -1,89 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - module Pagination - module Keyset - class RequestContext - attr_reader :request - - DEFAULT_SORT_DIRECTION = :desc - PRIMARY_KEY = :id - - # A tie breaker is added as an additional order-by column - # to establish a well-defined order. We use the primary key - # column here. - TIE_BREAKER = { PRIMARY_KEY => DEFAULT_SORT_DIRECTION }.freeze - - def initialize(request) - @request = request - end - - # extracts Paging information from request parameters - def page - @page ||= Page.new(order_by: order_by, per_page: params[:per_page]) - end - - def apply_headers(next_page) - request.header('Links', pagination_links(next_page)) - end - - private - - def order_by - return TIE_BREAKER.dup unless params[:order_by] - - order_by = { params[:order_by].to_sym => params[:sort]&.to_sym || DEFAULT_SORT_DIRECTION } - - # Order by an additional unique key, we use the primary key here - order_by = order_by.merge(TIE_BREAKER) unless order_by[PRIMARY_KEY] - - order_by - end - - def params - @params ||= request.params - end - - def lower_bounds_params(page) - page.lower_bounds.each_with_object({}) do |(column, value), params| - filter = filter_with_comparator(page, column) - params[filter] = value - end - end - - def filter_with_comparator(page, column) - direction = page.order_by[column] - - if direction&.to_sym == :desc - "#{column}_before" - else - "#{column}_after" - end - end - - def page_href(page) - base_request_uri.tap do |uri| - uri.query = query_params_for(page).to_query - end.to_s - end - - def pagination_links(next_page) - return if next_page.end_reached? - - %(<#{page_href(next_page)}>; rel="next") - end - - def base_request_uri - @base_request_uri ||= URI.parse(request.request.url).tap do |uri| - uri.host = Gitlab.config.gitlab.host - uri.port = Gitlab.config.gitlab.port - end - end - - def query_params_for(page) - request.params.merge(lower_bounds_params(page)) - end - end - end - end -end diff --git a/lib/gitlab/sql/pattern.rb b/lib/gitlab/sql/pattern.rb index f6edbfced7f..ca7ae429986 100644 --- a/lib/gitlab/sql/pattern.rb +++ b/lib/gitlab/sql/pattern.rb @@ -35,7 +35,7 @@ module Gitlab query.length >= min_chars_for_partial_matching end - # column - The column name to search in. + # column - The column name / Arel column to search in. # query - The text to search for. # lower_exact_match - When set to `true` we'll fall back to using # `LOWER(column) = query` instead of using `ILIKE`. @@ -43,19 +43,21 @@ module Gitlab query = query.squish return unless query.present? + arel_column = column.is_a?(Arel::Attributes::Attribute) ? column : arel_table[column] + words = select_fuzzy_words(query, use_minimum_char_limit: use_minimum_char_limit) if words.any? - words.map { |word| arel_table[column].matches(to_pattern(word, use_minimum_char_limit: use_minimum_char_limit)) }.reduce(:and) + words.map { |word| arel_column.matches(to_pattern(word, use_minimum_char_limit: use_minimum_char_limit)) }.reduce(:and) else # No words of at least 3 chars, but we can search for an exact # case insensitive match with the query as a whole if lower_exact_match Arel::Nodes::NamedFunction - .new('LOWER', [arel_table[column]]) + .new('LOWER', [arel_column]) .eq(query) else - arel_table[column].matches(sanitize_sql_like(query)) + arel_column.matches(sanitize_sql_like(query)) end end end diff --git a/lib/quality/test_level.rb b/lib/quality/test_level.rb index b7822adf6ed..90a8096cc2b 100644 --- a/lib/quality/test_level.rb +++ b/lib/quality/test_level.rb @@ -36,6 +36,10 @@ module Quality workers elastic_integration ], + migration: %w[ + migrations + lib/gitlab/background_migration + ], integration: %w[ controllers mailers @@ -62,6 +66,10 @@ module Quality def level_for(file_path) case file_path + # Detect migration first since some background migration tests are under + # spec/lib/gitlab/background_migration and tests under spec/lib are unit by default + when regexp(:migration) + :migration when regexp(:unit) :unit when regexp(:integration) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 34769f36c59..95a04eb18b0 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1702,6 +1702,9 @@ msgstr "" msgid "An error occurred while saving the approval settings" msgstr "" +msgid "An error occurred while saving the template. Please check if the template exists." +msgstr "" + msgid "An error occurred while subscribing to notifications." msgstr "" @@ -15054,6 +15057,9 @@ msgstr "" msgid "Save pipeline schedule" msgstr "" +msgid "Save template" +msgstr "" + msgid "Save variables" msgstr "" @@ -15715,6 +15721,12 @@ msgstr "" msgid "Service Desk is enabled but not yet active" msgstr "" +msgid "Service Desk is off" +msgstr "" + +msgid "Service Desk is on" +msgstr "" + msgid "Service Templates" msgstr "" @@ -17133,6 +17145,12 @@ msgstr "" msgid "Template" msgstr "" +msgid "Template to append to all Service Desk issues" +msgstr "" + +msgid "Template was successfully saved." +msgstr "" + msgid "Templates" msgstr "" diff --git a/qa/qa/page/mattermost/main.rb b/qa/qa/page/mattermost/main.rb index e531ace8529..eea5c4b527e 100644 --- a/qa/qa/page/mattermost/main.rb +++ b/qa/qa/page/mattermost/main.rb @@ -4,11 +4,6 @@ module QA module Page module Mattermost class Main < Page::Base - ## - # TODO, define all selectors required by this page object - # - # See gitlab-org/gitlab-qa#154 - # view 'app/views/projects/mattermosts/new.html.haml' def initialize diff --git a/qa/qa/page/project/settings/deploy_keys.rb b/qa/qa/page/project/settings/deploy_keys.rb index b8d961274a9..602bfc64710 100644 --- a/qa/qa/page/project/settings/deploy_keys.rb +++ b/qa/qa/page/project/settings/deploy_keys.rb @@ -12,7 +12,7 @@ module QA view 'app/assets/javascripts/deploy_keys/components/app.vue' do element :deploy_keys_section, /class=".*deploy\-keys.*"/ # rubocop:disable QA/ElementWithPattern - element :project_deploy_keys, 'class="qa-project-deploy-keys"' # rubocop:disable QA/ElementWithPattern + element :project_deploy_keys end view 'app/assets/javascripts/deploy_keys/components/key.vue' do diff --git a/scripts/review_apps/base-config.yaml b/scripts/review_apps/base-config.yaml index 407014858b4..1014bd9a89f 100644 --- a/scripts/review_apps/base-config.yaml +++ b/scripts/review_apps/base-config.yaml @@ -52,10 +52,10 @@ gitlab: resources: requests: cpu: 650m - memory: 880M + memory: 970M limits: cpu: 975m - memory: 1320M + memory: 1450M task-runner: resources: requests: @@ -68,10 +68,10 @@ gitlab: resources: requests: cpu: 500m - memory: 1540M + memory: 1630M limits: cpu: 750m - memory: 2310M + memory: 2450M deployment: readinessProbe: initialDelaySeconds: 5 # Default is 0 @@ -92,18 +92,18 @@ gitlab: gitlab-runner: resources: requests: - cpu: 450m + cpu: 675m memory: 100M limits: - cpu: 675m + cpu: 1015m memory: 150M minio: resources: requests: - cpu: 5m + cpu: 9m memory: 128M limits: - cpu: 10m + cpu: 15m memory: 280M nginx-ingress: controller: diff --git a/spec/frontend/registry/list/components/collapsible_container_spec.js b/spec/frontend/registry/list/components/collapsible_container_spec.js index 1f7d48e6e2c..cba49e72588 100644 --- a/spec/frontend/registry/list/components/collapsible_container_spec.js +++ b/spec/frontend/registry/list/components/collapsible_container_spec.js @@ -140,43 +140,33 @@ describe('collapsible registry container', () => { }); describe('tracking', () => { - const category = 'mock_page'; + const testTrackingCall = action => { + expect(Tracking.event).toHaveBeenCalledWith(undefined, action, { + label: 'registry_repository_delete', + }); + }; + beforeEach(() => { jest.spyOn(Tracking, 'event'); wrapper.vm.deleteItem = jest.fn().mockResolvedValue(); wrapper.vm.fetchRepos = jest.fn(); - wrapper.setData({ - tracking: { - ...wrapper.vm.tracking, - category, - }, - }); }); it('send an event when delete button is clicked', () => { const deleteBtn = findDeleteBtn(); deleteBtn.trigger('click'); - expect(Tracking.event).toHaveBeenCalledWith(category, 'click_button', { - label: 'registry_repository_delete', - category, - }); + testTrackingCall('click_button'); }); it('send an event when cancel is pressed on modal', () => { const deleteModal = findDeleteModal(); deleteModal.vm.$emit('cancel'); - expect(Tracking.event).toHaveBeenCalledWith(category, 'cancel_delete', { - label: 'registry_repository_delete', - category, - }); + testTrackingCall('cancel_delete'); }); it('send an event when confirm is clicked on modal', () => { const deleteModal = findDeleteModal(); deleteModal.vm.$emit('ok'); - expect(Tracking.event).toHaveBeenCalledWith(category, 'confirm_delete', { - label: 'registry_repository_delete', - category, - }); + testTrackingCall('confirm_delete'); }); }); }); diff --git a/spec/frontend/registry/list/components/table_registry_spec.js b/spec/frontend/registry/list/components/table_registry_spec.js index 51fd9612758..1d31381c85b 100644 --- a/spec/frontend/registry/list/components/table_registry_spec.js +++ b/spec/frontend/registry/list/components/table_registry_spec.js @@ -304,17 +304,14 @@ describe('table registry', () => { }); describe('event tracking', () => { - const mockPageName = 'mock_page'; + const testTrackingCall = (action, label = 'registry_tag_delete') => { + expect(Tracking.event).toHaveBeenCalledWith(undefined, action, { label, property: 'foo' }); + }; beforeEach(() => { jest.spyOn(Tracking, 'event'); wrapper.vm.handleSingleDelete = jest.fn(); wrapper.vm.handleMultipleDelete = jest.fn(); - document.body.dataset.page = mockPageName; - }); - - afterEach(() => { - document.body.dataset.page = null; }); describe('single tag delete', () => { @@ -325,29 +322,25 @@ describe('table registry', () => { it('send an event when delete button is clicked', () => { const deleteBtn = findDeleteButtonsRow(); deleteBtn.at(0).trigger('click'); - expect(Tracking.event).toHaveBeenCalledWith(mockPageName, 'click_button', { - label: 'registry_tag_delete', - property: 'foo', - }); + + testTrackingCall('click_button'); }); + it('send an event when cancel is pressed on modal', () => { const deleteModal = findDeleteModal(); deleteModal.vm.$emit('cancel'); - expect(Tracking.event).toHaveBeenCalledWith(mockPageName, 'cancel_delete', { - label: 'registry_tag_delete', - property: 'foo', - }); + + testTrackingCall('cancel_delete'); }); + it('send an event when confirm is clicked on modal', () => { const deleteModal = findDeleteModal(); deleteModal.vm.$emit('ok'); - expect(Tracking.event).toHaveBeenCalledWith(mockPageName, 'confirm_delete', { - label: 'registry_tag_delete', - property: 'foo', - }); + testTrackingCall('confirm_delete'); }); }); + describe('bulk tag delete', () => { beforeEach(() => { const items = [0, 1, 2]; @@ -357,27 +350,22 @@ describe('table registry', () => { it('send an event when delete button is clicked', () => { const deleteBtn = findDeleteButton(); deleteBtn.vm.$emit('click'); - expect(Tracking.event).toHaveBeenCalledWith(mockPageName, 'click_button', { - label: 'bulk_registry_tag_delete', - property: 'foo', - }); + + testTrackingCall('click_button', 'bulk_registry_tag_delete'); }); + it('send an event when cancel is pressed on modal', () => { const deleteModal = findDeleteModal(); deleteModal.vm.$emit('cancel'); - expect(Tracking.event).toHaveBeenCalledWith(mockPageName, 'cancel_delete', { - label: 'bulk_registry_tag_delete', - property: 'foo', - }); + + testTrackingCall('cancel_delete', 'bulk_registry_tag_delete'); }); + it('send an event when confirm is clicked on modal', () => { const deleteModal = findDeleteModal(); deleteModal.vm.$emit('ok'); - expect(Tracking.event).toHaveBeenCalledWith(mockPageName, 'confirm_delete', { - label: 'bulk_registry_tag_delete', - property: 'foo', - }); + testTrackingCall('confirm_delete', 'bulk_registry_tag_delete'); }); }); }); diff --git a/spec/lib/api/helpers/pagination_spec.rb b/spec/lib/api/helpers/pagination_spec.rb index d61341ed3e5..040ff1a8ebe 100644 --- a/spec/lib/api/helpers/pagination_spec.rb +++ b/spec/lib/api/helpers/pagination_spec.rb @@ -10,47 +10,13 @@ describe API::Helpers::Pagination do let(:offset_pagination) { double("offset pagination") } let(:expected_result) { double("result") } - before do - allow(subject).to receive(:params).and_return(params) - end - - context 'for offset pagination' do - let(:params) { {} } - - it 'delegates to OffsetPagination' do - expect(::Gitlab::Pagination::OffsetPagination).to receive(:new).with(subject).and_return(offset_pagination) - expect(offset_pagination).to receive(:paginate).with(relation).and_return(expected_result) - - result = subject.paginate(relation) - - expect(result).to eq(expected_result) - end - end - - context 'for keyset pagination' do - let(:params) { { pagination: 'keyset' } } - let(:request_context) { double('request context') } - - before do - allow(Gitlab::Pagination::Keyset::RequestContext).to receive(:new).with(subject).and_return(request_context) - allow(Gitlab::Pagination::Keyset).to receive(:available?).and_return(true) - end - - it 'delegates to KeysetPagination' do - expect(Gitlab::Pagination::Keyset).to receive(:paginate).with(request_context, relation).and_return(expected_result) - - result = subject.paginate(relation) - - expect(result).to eq(expected_result) - end + it 'delegates to OffsetPagination' do + expect(::Gitlab::Pagination::OffsetPagination).to receive(:new).with(subject).and_return(offset_pagination) + expect(offset_pagination).to receive(:paginate).with(relation).and_return(expected_result) - it 'renders a 501 error if keyset pagination isnt available yet' do - expect(Gitlab::Pagination::Keyset).to receive(:available?).with(request_context, relation).and_return(false) - expect(Gitlab::Pagination::Keyset).not_to receive(:paginate) - expect(subject).to receive(:error!).with(/not yet available/, 501) + result = subject.paginate(relation) - subject.paginate(relation) - end + expect(result).to eq(expected_result) end end end diff --git a/spec/lib/gitlab/pagination/keyset/page_spec.rb b/spec/lib/gitlab/pagination/keyset/page_spec.rb deleted file mode 100644 index bda9e6ecd13..00000000000 --- a/spec/lib/gitlab/pagination/keyset/page_spec.rb +++ /dev/null @@ -1,66 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Pagination::Keyset::Page do - describe '#per_page' do - it 'limits to a maximum of 20 records per page' do - per_page = described_class.new(per_page: 21).per_page - - expect(per_page).to eq(described_class::DEFAULT_PAGE_SIZE) - end - - it 'uses default value when given 0' do - per_page = described_class.new(per_page: 0).per_page - - expect(per_page).to eq(described_class::DEFAULT_PAGE_SIZE) - end - - it 'uses default value when given negative values' do - per_page = described_class.new(per_page: -1).per_page - - expect(per_page).to eq(described_class::DEFAULT_PAGE_SIZE) - end - - it 'uses the given value if it is within range' do - per_page = described_class.new(per_page: 10).per_page - - expect(per_page).to eq(10) - end - end - - describe '#next' do - let(:page) { described_class.new(order_by: order_by, lower_bounds: lower_bounds, per_page: per_page, end_reached: end_reached) } - subject { page.next(new_lower_bounds, new_end_reached) } - - let(:order_by) { { id: :desc } } - let(:lower_bounds) { { id: 42 } } - let(:per_page) { 10 } - let(:end_reached) { false } - - let(:new_lower_bounds) { { id: 21 } } - let(:new_end_reached) { true } - - it 'copies over order_by' do - expect(subject.order_by).to eq(page.order_by) - end - - it 'copies over per_page' do - expect(subject.per_page).to eq(page.per_page) - end - - it 'dups the instance' do - expect(subject).not_to eq(page) - end - - it 'sets lower_bounds only on new instance' do - expect(subject.lower_bounds).to eq(new_lower_bounds) - expect(page.lower_bounds).to eq(lower_bounds) - end - - it 'sets end_reached only on new instance' do - expect(subject.end_reached?).to eq(new_end_reached) - expect(page.end_reached?).to eq(end_reached) - end - end -end diff --git a/spec/lib/gitlab/pagination/keyset/pager_spec.rb b/spec/lib/gitlab/pagination/keyset/pager_spec.rb deleted file mode 100644 index 6d23fe2adcc..00000000000 --- a/spec/lib/gitlab/pagination/keyset/pager_spec.rb +++ /dev/null @@ -1,68 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Pagination::Keyset::Pager do - let(:relation) { Project.all.order(id: :asc) } - let(:request) { double('request', page: page, apply_headers: nil) } - let(:page) { Gitlab::Pagination::Keyset::Page.new(order_by: { id: :asc }, per_page: 3) } - let(:next_page) { double('next page') } - - before_all do - create_list(:project, 7) - end - - describe '#paginate' do - subject { described_class.new(request).paginate(relation) } - - it 'loads the result relation only once' do - expect do - subject - end.not_to exceed_query_limit(1) - end - - it 'passes information about next page to request' do - lower_bounds = relation.limit(page.per_page).last.slice(:id) - expect(page).to receive(:next).with(lower_bounds, false).and_return(next_page) - expect(request).to receive(:apply_headers).with(next_page) - - subject - end - - context 'when retrieving the last page' do - let(:relation) { Project.where('id > ?', Project.maximum(:id) - page.per_page).order(id: :asc) } - - it 'indicates this is the last page' do - expect(request).to receive(:apply_headers) do |next_page| - expect(next_page.end_reached?).to be_truthy - end - - subject - end - end - - context 'when retrieving an empty page' do - let(:relation) { Project.where('id > ?', Project.maximum(:id) + 1).order(id: :asc) } - - it 'indicates this is the last page' do - expect(request).to receive(:apply_headers) do |next_page| - expect(next_page.end_reached?).to be_truthy - end - - subject - end - end - - it 'returns an array with the loaded records' do - expect(subject).to eq(relation.limit(page.per_page).to_a) - end - - context 'validating the order clause' do - let(:page) { Gitlab::Pagination::Keyset::Page.new(order_by: { created_at: :asc }, per_page: 3) } - - it 'raises an error if has a different order clause than the page' do - expect { subject }.to raise_error(ArgumentError, /order_by does not match/) - end - end - end -end diff --git a/spec/lib/gitlab/pagination/keyset/request_context_spec.rb b/spec/lib/gitlab/pagination/keyset/request_context_spec.rb deleted file mode 100644 index 344ef90efa3..00000000000 --- a/spec/lib/gitlab/pagination/keyset/request_context_spec.rb +++ /dev/null @@ -1,115 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Pagination::Keyset::RequestContext do - let(:request) { double('request', params: params) } - - describe '#page' do - subject { described_class.new(request).page } - - context 'with only order_by given' do - let(:params) { { order_by: :id } } - - it 'extracts order_by/sorting information' do - page = subject - - expect(page.order_by).to eq(id: :desc) - end - end - - context 'with order_by and sort given' do - let(:params) { { order_by: :created_at, sort: :desc } } - - it 'extracts order_by/sorting information and adds tie breaker' do - page = subject - - expect(page.order_by).to eq(created_at: :desc, id: :desc) - end - end - - context 'with no order_by information given' do - let(:params) { {} } - - it 'defaults to tie breaker' do - page = subject - - expect(page.order_by).to eq({ id: :desc }) - end - end - - context 'with per_page params given' do - let(:params) { { per_page: 10 } } - - it 'extracts per_page information' do - page = subject - - expect(page.per_page).to eq(params[:per_page]) - end - end - end - - describe '#apply_headers' do - let(:request) { double('request', url: "http://#{Gitlab.config.gitlab.host}/api/v4/projects?foo=bar") } - let(:params) { { foo: 'bar' } } - let(:request_context) { double('request context', params: params, request: request) } - let(:next_page) { double('next page', order_by: { id: :asc }, lower_bounds: { id: 42 }, end_reached?: false) } - - subject { described_class.new(request_context).apply_headers(next_page) } - - it 'sets Links header with same host/path as the original request' do - orig_uri = URI.parse(request_context.request.url) - - expect(request_context).to receive(:header) do |name, header| - expect(name).to eq('Links') - - first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures - - uri = URI.parse(first_link) - - expect(uri.host).to eq(orig_uri.host) - expect(uri.path).to eq(orig_uri.path) - end - - subject - end - - it 'sets Links header with a link to the next page' do - orig_uri = URI.parse(request_context.request.url) - - expect(request_context).to receive(:header) do |name, header| - expect(name).to eq('Links') - - first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures - - query = CGI.parse(URI.parse(first_link).query) - - expect(query.except('id_after')).to eq(CGI.parse(orig_uri.query).except('id_after')) - expect(query['id_after']).to eq(['42']) - end - - subject - end - - context 'with descending order' do - let(:next_page) { double('next page', order_by: { id: :desc }, lower_bounds: { id: 42 }, end_reached?: false) } - - it 'sets Links header with a link to the next page' do - orig_uri = URI.parse(request_context.request.url) - - expect(request_context).to receive(:header) do |name, header| - expect(name).to eq('Links') - - first_link, _ = /<([^>]+)>; rel="next"/.match(header).captures - - query = CGI.parse(URI.parse(first_link).query) - - expect(query.except('id_before')).to eq(CGI.parse(orig_uri.query).except('id_before')) - expect(query['id_before']).to eq(['42']) - end - - subject - end - end - end -end diff --git a/spec/lib/gitlab/pagination/keyset_spec.rb b/spec/lib/gitlab/pagination/keyset_spec.rb deleted file mode 100644 index 755c422c46a..00000000000 --- a/spec/lib/gitlab/pagination/keyset_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Pagination::Keyset do - describe '.paginate' do - subject { described_class.paginate(request_context, relation) } - - let(:request_context) { double } - let(:relation) { double } - let(:pager) { double } - let(:result) { double } - - it 'uses Pager to paginate the relation' do - expect(Gitlab::Pagination::Keyset::Pager).to receive(:new).with(request_context).and_return(pager) - expect(pager).to receive(:paginate).with(relation).and_return(result) - - expect(subject).to eq(result) - end - end - - describe '.available?' do - subject { described_class } - - let(:request_context) { double("request context", page: page)} - let(:page) { double("page", order_by: order_by) } - - shared_examples_for 'keyset pagination is available' do - it 'returns true for Project' do - expect(subject.available?(request_context, Project.all)).to be_truthy - end - - it 'return false for other types of relations' do - expect(subject.available?(request_context, User.all)).to be_falsey - end - end - - context 'with order-by id asc' do - let(:order_by) { { id: :asc } } - - it_behaves_like 'keyset pagination is available' - end - - context 'with order-by id desc' do - let(:order_by) { { id: :desc } } - - it_behaves_like 'keyset pagination is available' - end - - context 'with other order-by columns' do - let(:order_by) { { created_at: :desc, id: :desc } } - it 'returns false for Project' do - expect(subject.available?(request_context, Project.all)).to be_falsey - end - - it 'return false for other types of relations' do - expect(subject.available?(request_context, User.all)).to be_falsey - end - end - end -end diff --git a/spec/lib/gitlab/sql/pattern_spec.rb b/spec/lib/gitlab/sql/pattern_spec.rb index 31944d51b3c..38b93913f6d 100644 --- a/spec/lib/gitlab/sql/pattern_spec.rb +++ b/spec/lib/gitlab/sql/pattern_spec.rb @@ -207,5 +207,15 @@ describe Gitlab::SQL::Pattern do expect(fuzzy_arel_match.to_sql).to match(/title.+I?LIKE '\%foo\%' AND .*title.*I?LIKE '\%baz\%' AND .*title.*I?LIKE '\%really bar\%'/) end end + + context 'when passing an Arel column' do + let(:query) { 'foo' } + + subject(:fuzzy_arel_match) { Project.fuzzy_arel_match(Route.arel_table[:path], query) } + + it 'returns a condition with the table and column name' do + expect(fuzzy_arel_match.to_sql).to match(/"routes"."path".*ILIKE '\%foo\%'/) + end + end end end diff --git a/spec/lib/quality/test_level_spec.rb b/spec/lib/quality/test_level_spec.rb index 4db188bd8f2..c85994402dd 100644 --- a/spec/lib/quality/test_level_spec.rb +++ b/spec/lib/quality/test_level_spec.rb @@ -25,6 +25,13 @@ RSpec.describe Quality::TestLevel do end end + context 'when level is migration' do + it 'returns a pattern' do + expect(subject.pattern(:migration)) + .to eq("spec/{migrations,lib/gitlab/background_migration}{,/**/}*_spec.rb") + end + end + context 'when level is integration' do it 'returns a pattern' do expect(subject.pattern(:integration)) @@ -79,6 +86,13 @@ RSpec.describe Quality::TestLevel do end end + context 'when level is migration' do + it 'returns a regexp' do + expect(subject.regexp(:migration)) + .to eq(%r{spec/(migrations|lib/gitlab/background_migration)}) + end + end + context 'when level is integration' do it 'returns a regexp' do expect(subject.regexp(:integration)) @@ -116,6 +130,18 @@ RSpec.describe Quality::TestLevel do expect(subject.level_for('spec/models/abuse_report_spec.rb')).to eq(:unit) end + it 'returns the correct level for a migration test' do + expect(subject.level_for('spec/migrations/add_default_and_free_plans_spec.rb')).to eq(:migration) + end + + it 'returns the correct level for a background_migration test' do + expect(subject.level_for('spec/lib/gitlab/background_migration/archive_legacy_traces_spec.rb')).to eq(:migration) + end + + it 'returns the correct level for a geo migration test' do + expect(described_class.new('ee/').level_for('ee/spec/migrations/geo/migrate_ci_job_artifacts_to_separate_registry_spec.rb')).to eq(:migration) + end + it 'returns the correct level for an integration test' do expect(subject.level_for('spec/mailers/abuse_report_mailer_spec.rb')).to eq(:integration) end diff --git a/spec/models/active_session_spec.rb b/spec/models/active_session_spec.rb index 07f716c4f38..c26675e75bf 100644 --- a/spec/models/active_session_spec.rb +++ b/spec/models/active_session_spec.rb @@ -307,6 +307,28 @@ RSpec.describe ActiveSession, :clean_gitlab_redis_shared_state do expect(lookup_entries).not_to include(max_number_of_sessions_plus_one.to_s, max_number_of_sessions_plus_two.to_s) end end + + it 'removes obsolete lookup entries even without active session' do + Gitlab::Redis::SharedState.with do |redis| + redis.sadd( + "session:lookup:user:gitlab:#{user.id}", + "#{max_number_of_sessions_plus_two + 1}" + ) + end + + ActiveSession.cleanup(user) + + Gitlab::Redis::SharedState.with do |redis| + lookup_entries = redis.smembers("session:lookup:user:gitlab:#{user.id}") + + expect(lookup_entries.count).to eq(ActiveSession::ALLOWED_NUMBER_OF_ACTIVE_SESSIONS) + expect(lookup_entries).not_to include( + max_number_of_sessions_plus_one.to_s, + max_number_of_sessions_plus_two.to_s, + (max_number_of_sessions_plus_two + 1).to_s + ) + end + end end end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 1ebac936341..b1f88c4530e 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1662,7 +1662,7 @@ describe Project do end describe '.search' do - let(:project) { create(:project, description: 'kitten mittens') } + let_it_be(:project) { create(:project, description: 'kitten mittens') } it 'returns projects with a matching name' do expect(described_class.search(project.name)).to eq([project]) @@ -1700,6 +1700,39 @@ describe Project do expect(described_class.search(project.path.upcase)).to eq([project]) end + context 'by full path' do + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, group: group) } + + context 'when feature is enabled' do + before do + stub_feature_flags(project_search_by_full_path: true) + end + + it 'returns projects that match the group path' do + expect(described_class.search(group.path)).to eq([project]) + end + + it 'returns projects that match the full path' do + expect(described_class.search(project.full_path)).to eq([project]) + end + end + + context 'when feature is disabled' do + before do + stub_feature_flags(project_search_by_full_path: false) + end + + it 'returns no results when searching by group path' do + expect(described_class.search(group.path)).to be_empty + end + + it 'returns no results when searching by full path' do + expect(described_class.search(project.full_path)).to be_empty + end + end + end + describe 'with pending_delete project' do let(:pending_delete_project) { create(:project, pending_delete: true) } diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 4c1db4f0d18..cda2dd7d2f4 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -570,87 +570,6 @@ describe API::Projects do let(:projects) { Project.all } end end - - context 'with keyset pagination' do - let(:current_user) { user } - let(:projects) { [public_project, project, project2, project3] } - - context 'headers and records' do - let(:params) { { pagination: 'keyset', order_by: :id, sort: :asc, per_page: 1 } } - - it 'includes a pagination header with link to the next page' do - get api('/projects', current_user), params: params - - expect(response.header).to include('Links') - expect(response.header['Links']).to include('pagination=keyset') - expect(response.header['Links']).to include("id_after=#{public_project.id}") - end - - it 'contains only the first project with per_page = 1' do - get api('/projects', current_user), params: params - - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array - expect(json_response.map { |p| p['id'] }).to contain_exactly(public_project.id) - end - - it 'does not include a link if the end has reached and there is no more data' do - get api('/projects', current_user), params: params.merge(id_after: project2.id) - - expect(response.header).not_to include('Links') - end - - it 'responds with 501 if order_by is different from id' do - get api('/projects', current_user), params: params.merge(order_by: :created_at) - - expect(response).to have_gitlab_http_status(501) - end - end - - context 'with descending sorting' do - let(:params) { { pagination: 'keyset', order_by: :id, sort: :desc, per_page: 1 } } - - it 'includes a pagination header with link to the next page' do - get api('/projects', current_user), params: params - - expect(response.header).to include('Links') - expect(response.header['Links']).to include('pagination=keyset') - expect(response.header['Links']).to include("id_before=#{project3.id}") - end - - it 'contains only the last project with per_page = 1' do - get api('/projects', current_user), params: params - - expect(response).to have_gitlab_http_status(200) - expect(json_response).to be_an Array - expect(json_response.map { |p| p['id'] }).to contain_exactly(project3.id) - end - end - - context 'retrieving the full relation' do - let(:params) { { pagination: 'keyset', order_by: :id, sort: :desc, per_page: 2 } } - - it 'returns all projects' do - url = '/projects' - requests = 0 - ids = [] - - while url && requests <= 5 # circuit breaker - requests += 1 - get api(url, current_user), params: params - - links = response.header['Links'] - url = links&.match(/<[^>]+(\/projects\?[^>]+)>; rel="next"/) do |match| - match[1] - end - - ids += JSON.parse(response.body).map { |p| p['id'] } - end - - expect(ids).to contain_exactly(*projects.map(&:id)) - end - end - end end describe 'POST /projects' do diff --git a/spec/support/database_cleaner.rb b/spec/support/database_cleaner.rb index 6f385d6e019..aaf408f6143 100644 --- a/spec/support/database_cleaner.rb +++ b/spec/support/database_cleaner.rb @@ -15,6 +15,39 @@ RSpec.configure do |config| delete_from_all_tables! end + config.append_after(:context, :migration) do + delete_from_all_tables! + + # Postgres maximum number of columns in a table is 1600 (https://github.com/postgres/postgres/blob/de41869b64d57160f58852eab20a27f248188135/src/include/access/htup_details.h#L23-L47). + # And since: + # "The DROP COLUMN form does not physically remove the column, but simply makes + # it invisible to SQL operations. Subsequent insert and update operations in the + # table will store a null value for the column. Thus, dropping a column is quick + # but it will not immediately reduce the on-disk size of your table, as the space + # occupied by the dropped column is not reclaimed. + # The space will be reclaimed over time as existing rows are updated." + # according to https://www.postgresql.org/docs/current/sql-altertable.html. + # We drop and recreate the database if any table has more than 1200 columns, just to be safe. + max_allowed_columns = 1200 + tables_with_more_than_allowed_columns = + ApplicationRecord.connection.execute("SELECT attrelid::regclass::text AS table, COUNT(*) AS column_count FROM pg_attribute GROUP BY attrelid HAVING COUNT(*) > #{max_allowed_columns}") + + if tables_with_more_than_allowed_columns.any? + tables_with_more_than_allowed_columns.each do |result| + puts "The #{result['table']} table has #{result['column_count']} columns." + end + puts "Recreating the database" + start = Gitlab::Metrics::System.monotonic_time + + ActiveRecord::Tasks::DatabaseTasks.drop_current + ActiveRecord::Tasks::DatabaseTasks.create_current + ActiveRecord::Tasks::DatabaseTasks.load_schema_current + ActiveRecord::Tasks::DatabaseTasks.migrate + + puts "Database re-creation done in #{Gitlab::Metrics::System.monotonic_time - start}" + end + end + config.around(:each, :delete) do |example| self.class.use_transactional_tests = false diff --git a/spec/tasks/gitlab/import_export/import_rake_spec.rb b/spec/tasks/gitlab/import_export/import_rake_spec.rb index e8507e63bf5..18b89912b9f 100644 --- a/spec/tasks/gitlab/import_export/import_rake_spec.rb +++ b/spec/tasks/gitlab/import_export/import_rake_spec.rb @@ -2,7 +2,7 @@ require 'rake_helper' -describe 'gitlab:import_export:import rake task' do +describe 'gitlab:import_export:import rake task', :sidekiq do let(:username) { 'root' } let(:namespace_path) { username } let!(:user) { create(:user, username: username) } @@ -12,6 +12,8 @@ describe 'gitlab:import_export:import rake task' do before do Rake.application.rake_require('tasks/gitlab/import_export/import') allow(Settings.uploads.object_store).to receive(:[]=).and_call_original + allow_any_instance_of(GitlabProjectImport).to receive(:exit) + .and_raise(RuntimeError, 'exit not handled') end around do |example| @@ -95,6 +97,10 @@ describe 'gitlab:import_export:import rake task' do end it 'fails project import with an error' do + # Catch exit call, and raise exception instead + expect_any_instance_of(GitlabProjectImport).to receive(:exit) + .with(1).and_raise(SystemExit) + expect { subject }.to raise_error(SystemExit).and output(error).to_stdout expect(project.merge_requests).to be_empty -- cgit v1.2.1