diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-29 15:09:14 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-29 15:09:14 +0300 |
commit | 1838e244070041ed8ebb6344c628b580d78b578f (patch) | |
tree | a6c863a58c74747c2241ff6df35593dfecfa83aa | |
parent | 290b2ab01becf60d9eaf1094b620f1fb32fdccb3 (diff) |
Add latest changes from gitlab-org/gitlab@master
53 files changed, 983 insertions, 72 deletions
@@ -399,8 +399,7 @@ group :development, :test do end group :development, :test, :danger do - gem 'danger-gitlab', '~> 8.0', require: false - gem 'gitlab-dangerfiles', '~> 0.8.0', require: false + gem 'gitlab-dangerfiles', '~> 1.1.0', require: false end group :development, :test, :coverage do diff --git a/Gemfile.lock b/Gemfile.lock index eff3f2c1cd1..75dced890e5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -441,8 +441,8 @@ GEM terminal-table (~> 1.5, >= 1.5.1) gitlab-chronic (0.10.5) numerizer (~> 0.2) - gitlab-dangerfiles (0.8.0) - danger + gitlab-dangerfiles (1.1.0) + danger-gitlab gitlab-experiment (0.5.1) activesupport (>= 3.0) scientist (~> 1.6, >= 1.6.0) @@ -1377,7 +1377,6 @@ DEPENDENCIES countries (~> 3.0) creole (~> 0.5.0) crystalball (~> 0.7.0) - danger-gitlab (~> 8.0) database_cleaner (~> 1.7.0) deckar01-task_list (= 2.3.1) default_value_for (~> 3.4.0) @@ -1423,7 +1422,7 @@ DEPENDENCIES gitaly (~> 13.9.0.pre.rc1) github-markup (~> 1.7.0) gitlab-chronic (~> 0.10.5) - gitlab-dangerfiles (~> 0.8.0) + gitlab-dangerfiles (~> 1.1.0) gitlab-experiment (~> 0.5.1) gitlab-fog-azure-rm (~> 1.0.1) gitlab-fog-google (~> 1.13) diff --git a/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue b/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue index bf92da66b27..c0e13d665c3 100644 --- a/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue +++ b/app/assets/javascripts/alerts_settings/components/alerts_settings_form.vue @@ -505,7 +505,7 @@ export default { <gl-form-textarea id="sample-payload" - v-model.trim="samplePayload.json" + v-model="samplePayload.json" :disabled="canEditPayload" :state="isSampePayloadValid" :placeholder="$options.i18n.integrationFormSteps.mapFields.placeholder" @@ -669,7 +669,7 @@ export default { <gl-form-textarea id="test-payload" - v-model.trim="testPayload.json" + v-model="testPayload.json" :state="isTestPayloadValid" :placeholder="$options.i18n.integrationFormSteps.testPayload.placeholder" class="gl-my-3" diff --git a/app/assets/javascripts/packages/list/components/packages_list_app.vue b/app/assets/javascripts/packages/list/components/packages_list_app.vue index a609dfebedf..3b236764088 100644 --- a/app/assets/javascripts/packages/list/components/packages_list_app.vue +++ b/app/assets/javascripts/packages/list/components/packages_list_app.vue @@ -5,6 +5,7 @@ import createFlash from '~/flash'; import { historyReplaceState } from '~/lib/utils/common_utils'; import { s__ } from '~/locale'; import { SHOW_DELETE_SUCCESS_ALERT } from '~/packages/shared/constants'; +import { FILTERED_SEARCH_TERM } from '~/packages_and_registries/shared/constants'; import { DELETE_PACKAGE_SUCCESS_MESSAGE } from '../constants'; import PackageSearch from './package_search.vue'; import PackageTitle from './package_title.vue'; @@ -30,7 +31,7 @@ export default { }), emptySearch() { return ( - this.filter.filter((f) => f.type !== 'filtered-search-term' || f.value?.data).length === 0 + this.filter.filter((f) => f.type !== FILTERED_SEARCH_TERM || f.value?.data).length === 0 ); }, diff --git a/app/assets/javascripts/packages_and_registries/shared/constants.js b/app/assets/javascripts/packages_and_registries/shared/constants.js new file mode 100644 index 00000000000..55b5816cc5a --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/shared/constants.js @@ -0,0 +1 @@ +export const FILTERED_SEARCH_TERM = 'filtered-search-term'; diff --git a/app/assets/javascripts/packages_and_registries/shared/utils.js b/app/assets/javascripts/packages_and_registries/shared/utils.js new file mode 100644 index 00000000000..663c0b2762a --- /dev/null +++ b/app/assets/javascripts/packages_and_registries/shared/utils.js @@ -0,0 +1,9 @@ +import { queryToObject } from '~/lib/utils/url_utility'; +import { FILTERED_SEARCH_TERM } from './constants'; + +export const getQueryParams = (query) => queryToObject(query, { gatherArrays: true }); + +export const keyValueToFilterToken = (type, data) => ({ type, value: { data } }); + +export const searchArrayToFilterTokens = (search) => + search.map((s) => keyValueToFilterToken(FILTERED_SEARCH_TERM, s)); diff --git a/app/assets/javascripts/registry/explorer/pages/list.vue b/app/assets/javascripts/registry/explorer/pages/list.vue index 625d491db6a..46b3ebbc633 100644 --- a/app/assets/javascripts/registry/explorer/pages/list.vue +++ b/app/assets/javascripts/registry/explorer/pages/list.vue @@ -11,6 +11,7 @@ import { import { get } from 'lodash'; import getContainerRepositoriesQuery from 'shared_queries/container_registry/get_container_repositories.query.graphql'; import createFlash from '~/flash'; +import { FILTERED_SEARCH_TERM } from '~/packages_and_registries/shared/constants'; import Tracking from '~/tracking'; import RegistrySearch from '~/vue_shared/components/registry/registry_search.vue'; import DeleteImage from '../components/delete_image.vue'; @@ -241,7 +242,7 @@ export default { }; }, doFilter() { - const search = this.filter.find((i) => i.type === 'filtered-search-term'); + const search = this.filter.find((i) => i.type === FILTERED_SEARCH_TERM); this.name = search?.value?.data; }, }, diff --git a/app/assets/javascripts/repository/components/blob_content_viewer.vue b/app/assets/javascripts/repository/components/blob_content_viewer.vue new file mode 100644 index 00000000000..a77c1a41787 --- /dev/null +++ b/app/assets/javascripts/repository/components/blob_content_viewer.vue @@ -0,0 +1,93 @@ +<script> +import { GlLoadingIcon } from '@gitlab/ui'; +import { uniqueId } from 'lodash'; +import BlobContent from '~/blob/components/blob_content.vue'; +import BlobHeader from '~/blob/components/blob_header.vue'; +import createFlash from '~/flash'; +import { __ } from '~/locale'; +import blobInfoQuery from '../queries/blob_info.query.graphql'; +import projectPathQuery from '../queries/project_path.query.graphql'; + +export default { + components: { + BlobHeader, + BlobContent, + GlLoadingIcon, + }, + apollo: { + projectPath: { + query: projectPathQuery, + }, + blobInfo: { + query: blobInfoQuery, + variables() { + return { + projectPath: this.projectPath, + filePath: this.path, + }; + }, + error() { + createFlash({ message: __('An error occurred while loading the file. Please try again.') }); + }, + }, + }, + provide() { + return { + blobHash: uniqueId(), + }; + }, + data() { + return { + projectPath: '', + blobInfo: { + name: '', + size: '', + rawBlob: '', + type: '', + fileType: '', + tooLarge: false, + path: '', + editBlobPath: '', + ideEditPath: '', + storedExternally: false, + rawPath: '', + externalStorageUrl: '', + replacePath: '', + deletePath: '', + canLock: false, + isLocked: false, + lockLink: '', + canModifyBlob: true, + forkPath: '', + simpleViewer: '', + richViewer: '', + }, + }; + }, + computed: { + isLoading() { + return this.$apollo.queries.blobInfo.loading; + }, + viewer() { + const { fileType, tooLarge, type } = this.blobInfo; + + return { fileType, tooLarge, type }; + }, + }, +}; +</script> + +<template> + <div> + <gl-loading-icon v-if="isLoading" /> + <div v-if="blobInfo && !isLoading"> + <blob-header :blob="blobInfo" /> + <blob-content + :blob="blobInfo" + :content="blobInfo.rawBlob" + :active-viewer="viewer" + :loading="false" + /> + </div> + </div> +</template> diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index 70918dd55e4..8ea5fce92fa 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -12,6 +12,7 @@ import { escapeRegExp } from 'lodash'; import { escapeFileUrl } from '~/lib/utils/url_utility'; import FileIcon from '~/vue_shared/components/file_icon.vue'; import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; +import glFeatureFlagMixin from '~/vue_shared/mixins/gl_feature_flags_mixin'; import getRefMixin from '../../mixins/get_ref'; import commitQuery from '../../queries/commit.query.graphql'; @@ -41,7 +42,7 @@ export default { }, }, }, - mixins: [getRefMixin], + mixins: [getRefMixin, glFeatureFlagMixin()], props: { id: { type: String, @@ -103,10 +104,21 @@ export default { }; }, computed: { + refactorBlobViewerEnabled() { + return this.glFeatures.refactorBlobViewer; + }, routerLinkTo() { - return this.isFolder - ? { path: `/-/tree/${this.escapedRef}/${escapeFileUrl(this.path)}` } - : null; + const blobRouteConfig = { path: `/-/blob/${this.escapedRef}/${escapeFileUrl(this.path)}` }; + const treeRouteConfig = { path: `/-/tree/${this.escapedRef}/${escapeFileUrl(this.path)}` }; + + if (this.refactorBlobViewerEnabled && this.isBlob) { + return blobRouteConfig; + } + + return this.isFolder ? treeRouteConfig : null; + }, + isBlob() { + return this.type === 'blob'; }, isFolder() { return this.type === 'tree'; @@ -115,7 +127,7 @@ export default { return this.type === 'commit'; }, linkComponent() { - return this.isFolder ? 'router-link' : 'a'; + return this.isFolder || (this.refactorBlobViewerEnabled && this.isBlob) ? 'router-link' : 'a'; }, fullPath() { return this.path.replace(new RegExp(`^${escapeRegExp(this.currentPath)}/`), ''); diff --git a/app/assets/javascripts/repository/pages/blob.vue b/app/assets/javascripts/repository/pages/blob.vue new file mode 100644 index 00000000000..c492f966364 --- /dev/null +++ b/app/assets/javascripts/repository/pages/blob.vue @@ -0,0 +1,17 @@ +<script> +// This file is in progress and behind a feature flag, please see the following issue for more: +// https://gitlab.com/gitlab-org/gitlab/-/issues/323200 + +// TODO (follow-up MR): import BlobContentViewer from '../components/blob_content_viewer.vue'; + +export default { + components: { + // TODO (follow-up MR): BlobContentViewer, + }, +}; +</script> + +<template> + <div></div> + <!-- TODO (follow-up MR): <blob-content-viewer/> --> +</template> diff --git a/app/assets/javascripts/repository/queries/blob_info.query.graphql b/app/assets/javascripts/repository/queries/blob_info.query.graphql new file mode 100644 index 00000000000..e0bbf12f3eb --- /dev/null +++ b/app/assets/javascripts/repository/queries/blob_info.query.graphql @@ -0,0 +1,30 @@ +query getBlobInfo($projectPath: ID!, $filePath: String!) { + project(fullPath: $projectPath) { + id + repository { + blobs(path: $filePath) { + name + size + rawBlob + type + fileType + tooLarge + path + editBlobPath + ideEditPath + storedExternally + rawPath + externalStorageUrl + replacePath + deletePath + canLock + isLocked + lockLink + canModifyBlob + forkPath + simpleViewer + richViewer + } + } + } +} diff --git a/app/assets/javascripts/repository/router.js b/app/assets/javascripts/repository/router.js index ad6e32d7055..c7f7451fb55 100644 --- a/app/assets/javascripts/repository/router.js +++ b/app/assets/javascripts/repository/router.js @@ -2,6 +2,7 @@ import { escapeRegExp } from 'lodash'; import Vue from 'vue'; import VueRouter from 'vue-router'; import { joinPaths } from '../lib/utils/url_utility'; +import BlobPage from './pages/blob.vue'; import IndexPage from './pages/index.vue'; import TreePage from './pages/tree.vue'; @@ -15,6 +16,13 @@ export default function createRouter(base, baseRef) { }), }; + const blobPathRoute = { + component: BlobPage, + props: (route) => ({ + path: route.params.path, + }), + }; + return new VueRouter({ mode: 'history', base: joinPaths(gon.relative_url_root || '', base), @@ -32,6 +40,18 @@ export default function createRouter(base, baseRef) { ...treePathRoute, }, { + name: 'blobPathDecoded', + // Sometimes the ref needs decoding depending on how the backend sends it to us + path: `(/-)?/blob/${decodeURI(baseRef)}/:path*`, + ...blobPathRoute, + }, + { + name: 'blobPath', + // Support without decoding as well just in case the ref doesn't need to be decoded + path: `(/-)?/blob/${escapeRegExp(baseRef)}/:path*`, + ...blobPathRoute, + }, + { path: '/', name: 'projectRoot', component: IndexPage, diff --git a/app/assets/javascripts/vue_shared/components/registry/registry_search.vue b/app/assets/javascripts/vue_shared/components/registry/registry_search.vue index 62453a25f62..0825c3a76ea 100644 --- a/app/assets/javascripts/vue_shared/components/registry/registry_search.vue +++ b/app/assets/javascripts/vue_shared/components/registry/registry_search.vue @@ -1,5 +1,6 @@ <script> import { GlSorting, GlSortingItem, GlFilteredSearch } from '@gitlab/ui'; +import { FILTERED_SEARCH_TERM } from '~/packages_and_registries/shared/constants'; const ASCENDING_ORDER = 'asc'; const DESCENDING_ORDER = 'desc'; @@ -45,18 +46,60 @@ export default { isSortAscending() { return this.sorting.sort === ASCENDING_ORDER; }, + baselineQueryStringFilters() { + return this.tokens.reduce((acc, curr) => { + acc[curr.type] = ''; + return acc; + }, {}); + }, }, methods: { + generateQueryData({ sorting = {}, filter = [] } = {}) { + // Ensure that we clean up the query when we remove a token from the search + const result = { ...this.baselineQueryStringFilters, ...sorting, search: [] }; + + filter.forEach((f) => { + if (f.type === FILTERED_SEARCH_TERM) { + result.search.push(f.value.data); + } else { + result[f.type] = f.value.data; + } + }); + return result; + }, onDirectionChange() { const sort = this.isSortAscending ? DESCENDING_ORDER : ASCENDING_ORDER; + const newQueryString = this.generateQueryData({ + sorting: { ...this.sorting, sort }, + filter: this.filter, + }); this.$emit('sorting:changed', { sort }); + this.$emit('query:changed', newQueryString); }, onSortItemClick(item) { + const newQueryString = this.generateQueryData({ + sorting: { ...this.sorting, orderBy: item }, + filter: this.filter, + }); this.$emit('sorting:changed', { orderBy: item }); + this.$emit('query:changed', newQueryString); + }, + submitSearch() { + const newQueryString = this.generateQueryData({ + sorting: this.sorting, + filter: this.filter, + }); + this.$emit('filter:submit'); + this.$emit('query:changed', newQueryString); }, clearSearch() { + const newQueryString = this.generateQueryData({ + sorting: this.sorting, + }); + this.$emit('filter:changed', []); this.$emit('filter:submit'); + this.$emit('query:changed', newQueryString); }, }, }; @@ -69,7 +112,7 @@ export default { class="gl-mr-4 gl-flex-fill-1" :placeholder="__('Filter results')" :available-tokens="tokens" - @submit="$emit('filter:submit')" + @submit="submitSearch" @clear="clearSearch" /> <gl-sorting diff --git a/app/assets/javascripts/vue_shared/components/url_sync.vue b/app/assets/javascripts/vue_shared/components/url_sync.vue index 2844d9e9e94..925c6008836 100644 --- a/app/assets/javascripts/vue_shared/components/url_sync.vue +++ b/app/assets/javascripts/vue_shared/components/url_sync.vue @@ -2,11 +2,18 @@ import { historyPushState } from '~/lib/utils/common_utils'; import { mergeUrlParams } from '~/lib/utils/url_utility'; +/** + * Renderless component to update the query string, + * the update is done by updating the query property or + * by using updateQuery method in the scoped slot. + * note: do not use both prop and updateQuery method. + */ export default { props: { query: { type: Object, - required: true, + required: false, + default: null, }, }, watch: { @@ -14,12 +21,19 @@ export default { immediate: true, deep: true, handler(newQuery) { - historyPushState(mergeUrlParams(newQuery, window.location.href, { spreadArrays: true })); + if (newQuery) { + this.updateQuery(newQuery); + } }, }, }, + methods: { + updateQuery(newQuery) { + historyPushState(mergeUrlParams(newQuery, window.location.href, { spreadArrays: true })); + }, + }, render() { - return this.$slots.default; + return this.$scopedSlots.default?.({ updateQuery: this.updateQuery }); }, }; </script> diff --git a/app/assets/stylesheets/page_bundles/ci_status.scss b/app/assets/stylesheets/page_bundles/ci_status.scss index 232d363b7f1..6b976106cc9 100644 --- a/app/assets/stylesheets/page_bundles/ci_status.scss +++ b/app/assets/stylesheets/page_bundles/ci_status.scss @@ -80,17 +80,3 @@ } } } - -.d-block.d-sm-none-inline { - .ci-status-link { - position: relative; - top: 2px; - left: 5px; - } -} - -.ci-status-link { - svg { - overflow: visible; - } -} diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 00390d90e8d..4f28207564a 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -35,6 +35,10 @@ class ProjectsController < Projects::ApplicationController push_frontend_feature_flag(:allow_editing_commit_messages, @project) end + before_action do + push_frontend_feature_flag(:refactor_blob_viewer, @project, default_enabled: :yaml) + end + layout :determine_layout feature_category :projects, [ diff --git a/app/models/deployment.rb b/app/models/deployment.rb index 5b3671923cc..423914139ba 100644 --- a/app/models/deployment.rb +++ b/app/models/deployment.rb @@ -45,6 +45,7 @@ class Deployment < ApplicationRecord scope :active, -> { where(status: %i[created running]) } scope :older_than, -> (deployment) { where('deployments.id < ?', deployment.id) } scope :with_deployable, -> { joins('INNER JOIN ci_builds ON ci_builds.id = deployments.deployable_id').preload(:deployable) } + scope :with_api_entity_associations, -> { preload({ deployable: { runner: [], tags: [], user: [], job_artifacts_archive: [] } }) } scope :finished_after, ->(date) { where('finished_at >= ?', date) } scope :finished_before, ->(date) { where('finished_at < ?', date) } diff --git a/changelogs/unreleased/229076-fix-access-denied-on-ca.yml b/changelogs/unreleased/229076-fix-access-denied-on-ca.yml new file mode 100644 index 00000000000..23fc9f1b31f --- /dev/null +++ b/changelogs/unreleased/229076-fix-access-denied-on-ca.yml @@ -0,0 +1,5 @@ +--- +title: Show the Contribution Analytics promotion page for users without permission +merge_request: 57222 +author: +type: changed diff --git a/changelogs/unreleased/325883-do-not-trim-whitespaces.yml b/changelogs/unreleased/325883-do-not-trim-whitespaces.yml new file mode 100644 index 00000000000..efd498fc597 --- /dev/null +++ b/changelogs/unreleased/325883-do-not-trim-whitespaces.yml @@ -0,0 +1,5 @@ +--- +title: Do not trim input for sample & test payload on alerts integration form +merge_request: 57617 +author: +type: changed diff --git a/changelogs/unreleased/id-cache-cache-merge-request-versions-version-api.yml b/changelogs/unreleased/id-cache-cache-merge-request-versions-version-api.yml new file mode 100644 index 00000000000..2b868d741bf --- /dev/null +++ b/changelogs/unreleased/id-cache-cache-merge-request-versions-version-api.yml @@ -0,0 +1,5 @@ +--- +title: Cache merge request diff version API +merge_request: 57568 +author: +type: performance diff --git a/changelogs/unreleased/id-n-1-for-api-deployments.yml b/changelogs/unreleased/id-n-1-for-api-deployments.yml new file mode 100644 index 00000000000..b6b97a71c24 --- /dev/null +++ b/changelogs/unreleased/id-n-1-for-api-deployments.yml @@ -0,0 +1,5 @@ +--- +title: Resolve N + 1 for deployments API +merge_request: 57558 +author: +type: performance diff --git a/changelogs/unreleased/kp-remove-roadmap-buffered-rendering-ff.yml b/changelogs/unreleased/kp-remove-roadmap-buffered-rendering-ff.yml new file mode 100644 index 00000000000..d4d3c34a5b8 --- /dev/null +++ b/changelogs/unreleased/kp-remove-roadmap-buffered-rendering-ff.yml @@ -0,0 +1,5 @@ +--- +title: Remove unused feature flag ':roadmap_buffered_rendering' +merge_request: 57486 +author: +type: removed diff --git a/config/feature_flags/development/cached_api_merge_request_version.yml b/config/feature_flags/development/cached_api_merge_request_version.yml new file mode 100644 index 00000000000..8c00ffa20e9 --- /dev/null +++ b/config/feature_flags/development/cached_api_merge_request_version.yml @@ -0,0 +1,8 @@ +--- +name: cached_api_merge_request_version +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57568 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/326093 +milestone: '13.11' +type: development +group: group::source code +default_enabled: true diff --git a/config/feature_flags/development/loose_index_scan_for_distinct_values.yml b/config/feature_flags/development/loose_index_scan_for_distinct_values.yml new file mode 100644 index 00000000000..84f693d9247 --- /dev/null +++ b/config/feature_flags/development/loose_index_scan_for_distinct_values.yml @@ -0,0 +1,8 @@ +--- +name: loose_index_scan_for_distinct_values +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/55985 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/324210 +milestone: '13.10' +type: development +group: group::optimize +default_enabled: false diff --git a/config/feature_flags/development/refactor_blob_viewer.yml b/config/feature_flags/development/refactor_blob_viewer.yml new file mode 100644 index 00000000000..231e2684023 --- /dev/null +++ b/config/feature_flags/development/refactor_blob_viewer.yml @@ -0,0 +1,8 @@ +--- +name: refactor_blob_viewer +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/57326 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/324351 +milestone: '13.11' +type: development +group: group::source code +default_enabled: false diff --git a/config/known_invalid_graphql_queries.yml b/config/known_invalid_graphql_queries.yml index 2989b3a4262..26188d6068f 100644 --- a/config/known_invalid_graphql_queries.yml +++ b/config/known_invalid_graphql_queries.yml @@ -4,3 +4,4 @@ filenames: - ee/app/assets/javascripts/security_configuration/api_fuzzing/graphql/api_fuzzing_ci_configuration.query.graphql - ee/app/assets/javascripts/security_configuration/api_fuzzing/graphql/create_api_fuzzing_configuration.mutation.graphql - ee/app/assets/javascripts/security_configuration/dast_profiles/graphql/dast_failed_site_validations.query.graphql + - app/assets/javascripts/repository/queries/blob_info.query.graphql diff --git a/doc/api/groups.md b/doc/api/groups.md index cedfba4689f..6c01b2cf2a6 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -975,6 +975,9 @@ Parameters: The response is `202 Accepted` if the user has authorization. +NOTE: +A GitLab.com group can't be removed if it is linked to a subscription. To remove such a group, first [link the subscription](../subscriptions/index.md#change-the-linked-namespace) with a different group. + ## Restore group marked for deletion **(PREMIUM)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/33257) in GitLab 12.8. diff --git a/doc/development/database/not_null_constraints.md b/doc/development/database/not_null_constraints.md index 01d5b7af7f1..743ad87ee87 100644 --- a/doc/development/database/not_null_constraints.md +++ b/doc/development/database/not_null_constraints.md @@ -207,7 +207,7 @@ end ## `NOT NULL` constraints on large tables -If you have to clean up a text column for a really [large table](https://gitlab.com/gitlab-org/gitlab/-/blob/master/rubocop/migration_helpers.rb#L12) +If you have to clean up a text column for a [high-traffic table](../migration_style_guide.md#high-traffic-tables) (for example, the `artifacts` in `ci_builds`), your background migration will go on for a while and it will need an additional [background migration cleaning up](../background_migrations.md#cleaning-up) in the release after adding the data migration. diff --git a/doc/install/postgresql_extensions.md b/doc/install/postgresql_extensions.md index 8d5a2a79b72..663ec547733 100644 --- a/doc/install/postgresql_extensions.md +++ b/doc/install/postgresql_extensions.md @@ -15,6 +15,7 @@ The following extensions must be loaded into the GitLab database: |--------------|------------------------| | `pg_trgm` | 8.6 | | `btree_gist` | 13.1 | +| `plpgsql` | 11.7 | In order to install extensions, PostgreSQL requires the user to have superuser privileges. Typically, the GitLab database user is not a superuser. Therefore, regular database migrations diff --git a/lib/api/deployments.rb b/lib/api/deployments.rb index d0c842bb19d..0a6ecf2919c 100644 --- a/lib/api/deployments.rb +++ b/lib/api/deployments.rb @@ -36,7 +36,9 @@ module API get ':id/deployments' do authorize! :read_deployment, user_project - deployments = DeploymentsFinder.new(params.merge(project: user_project)).execute + deployments = + DeploymentsFinder.new(params.merge(project: user_project)) + .execute.with_api_entity_associations present paginate(deployments), with: Entities::Deployment end diff --git a/lib/api/entities/user.rb b/lib/api/entities/user.rb index 248a86751d2..3ce6d03e236 100644 --- a/lib/api/entities/user.rb +++ b/lib/api/entities/user.rb @@ -11,10 +11,10 @@ module API work_information(user) end expose :followers, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) } do |user| - user.followers.count + user.followers.size end expose :following, if: ->(user, opts) { Ability.allowed?(opts[:current_user], :read_user_profile, user) } do |user| - user.followees.count + user.followees.size end end end diff --git a/lib/api/groups.rb b/lib/api/groups.rb index ec097e3e3d9..6247895a2dd 100644 --- a/lib/api/groups.rb +++ b/lib/api/groups.rb @@ -141,6 +141,10 @@ module API def authorize_group_creation! authorize! :create_group end + + def check_subscription!(group) + render_api_error!("This group can't be removed because it is linked to a subscription.", :bad_request) if group.paid? + end end resource :groups do @@ -239,6 +243,7 @@ module API delete ":id" do group = find_group!(params[:id]) authorize! :admin_group, group + check_subscription! group delete_group(group) end diff --git a/lib/api/merge_request_diffs.rb b/lib/api/merge_request_diffs.rb index 97a6c7075b3..79303945598 100644 --- a/lib/api/merge_request_diffs.rb +++ b/lib/api/merge_request_diffs.rb @@ -45,7 +45,11 @@ module API merge_request = find_merge_request_with_access(params[:merge_request_iid]) - present merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull + if Feature.enabled?(:cached_api_merge_request_version, default_enabled: :yaml) + present_cached merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull, cache_context: nil + else + present merge_request.merge_request_diffs.find(params[:version_id]), with: Entities::MergeRequestDiffFull + end end end end diff --git a/lib/gitlab/database/batch_count.rb b/lib/gitlab/database/batch_count.rb index 5a506da0d05..9002d39e1ee 100644 --- a/lib/gitlab/database/batch_count.rb +++ b/lib/gitlab/database/batch_count.rb @@ -88,11 +88,16 @@ module Gitlab batch_start = start while batch_start < finish - batch_end = [batch_start + batch_size, finish].min - batch_relation = build_relation_batch(batch_start, batch_end, mode) - begin - results = merge_results(results, batch_relation.send(@operation, *@operation_args)) # rubocop:disable GitlabSecurity/PublicSend + batch_end = [batch_start + batch_size, finish].min + batch_relation = build_relation_batch(batch_start, batch_end, mode) + + op_args = @operation_args + if @operation == :count && @operation_args.blank? && use_loose_index_scan_for_distinct_values?(mode) + op_args = [Gitlab::Database::LooseIndexScanDistinctCount::COLUMN_ALIAS] + end + + results = merge_results(results, batch_relation.send(@operation, *op_args)) # rubocop:disable GitlabSecurity/PublicSend batch_start = batch_end rescue ActiveRecord::QueryCanceled => error # retry with a safe batch size & warmer cache @@ -102,6 +107,18 @@ module Gitlab log_canceled_batch_fetch(batch_start, mode, batch_relation.to_sql, error) return FALLBACK end + rescue Gitlab::Database::LooseIndexScanDistinctCount::ColumnConfigurationError => error + Gitlab::AppJsonLogger + .error( + event: 'batch_count', + relation: @relation.table_name, + operation: @operation, + operation_args: @operation_args, + mode: mode, + message: "LooseIndexScanDistinctCount column error: #{error.message}" + ) + + return FALLBACK end sleep(SLEEP_TIME_IN_SECONDS) @@ -123,7 +140,11 @@ module Gitlab private def build_relation_batch(start, finish, mode) - @relation.select(@column).public_send(mode).where(between_condition(start, finish)) # rubocop:disable GitlabSecurity/PublicSend + if use_loose_index_scan_for_distinct_values?(mode) + Gitlab::Database::LooseIndexScanDistinctCount.new(@relation, @column).build_query(from: start, to: finish) + else + @relation.select(@column).public_send(mode).where(between_condition(start, finish)) # rubocop:disable GitlabSecurity/PublicSend + end end def batch_size_for_mode_and_operation(mode, operation) @@ -165,6 +186,14 @@ module Gitlab message: "Query has been canceled with message: #{error.message}" ) end + + def use_loose_index_scan_for_distinct_values?(mode) + Feature.enabled?(:loose_index_scan_for_distinct_values) && not_group_by_query? && mode == :distinct + end + + def not_group_by_query? + !@relation.is_a?(ActiveRecord::Relation) || @relation.group_values.blank? + end end end end diff --git a/lib/gitlab/database/loose_index_scan_distinct_count.rb b/lib/gitlab/database/loose_index_scan_distinct_count.rb new file mode 100644 index 00000000000..884f4d47ff8 --- /dev/null +++ b/lib/gitlab/database/loose_index_scan_distinct_count.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +module Gitlab + module Database + # This class builds efficient batched distinct query by using loose index scan. + # Consider the following example: + # > Issue.distinct(:project_id).where(project_id: (1...100)).count + # + # Note: there is an index on project_id + # + # This query will read each element in the index matching the project_id filter. + # If for a project_id has 100_000 issues, all 100_000 elements will be read. + # + # A loose index scan will read only one entry from the index for each project_id to reduce the number of disk reads. + # + # Usage: + # + # Gitlab::Database::LooseIndexScanDisctinctCount.new(Issue, :project_id).count(from: 1, to: 100) + # + # The query will return the number of distinct projects_ids between 1 and 100 + # + # Getting the Arel query: + # + # Gitlab::Database::LooseIndexScanDisctinctCount.new(Issue, :project_id).build_query(from: 1, to: 100) + class LooseIndexScanDistinctCount + COLUMN_ALIAS = 'distinct_count_column' + + ColumnConfigurationError = Class.new(StandardError) + + def initialize(scope, column) + if scope.is_a?(ActiveRecord::Relation) + @scope = scope + @model = scope.model + else + @scope = scope.where({}) + @model = scope + end + + @column = transform_column(column) + end + + def count(from:, to:) + build_query(from: from, to: to).count(COLUMN_ALIAS) + end + + def build_query(from:, to:) # rubocop:disable Metrics/AbcSize + cte = Gitlab::SQL::RecursiveCTE.new(:counter_cte, union_args: { remove_order: false }) + table = model.arel_table + + cte << @scope + .dup + .select(column.as(COLUMN_ALIAS)) + .where(column.gteq(from)) + .where(column.lt(to)) + .order(column) + .limit(1) + + inner_query = @scope + .dup + .where(column.gt(cte.table[COLUMN_ALIAS])) + .where(column.lt(to)) + .select(column.as(COLUMN_ALIAS)) + .order(column) + .limit(1) + + cte << cte.table + .project(Arel::Nodes::Grouping.new(Arel.sql(inner_query.to_sql)).as(COLUMN_ALIAS)) + .where(cte.table[COLUMN_ALIAS].lt(to)) + + model + .with + .recursive(cte.to_arel) + .from(cte.alias_to(table)) + .unscope(where: :source_type) + .unscope(where: model.inheritance_column) # Remove STI query, not needed here + end + + private + + attr_reader :column, :model + + # Transforms the column so it can be used in Arel expressions + # + # 'table.column' => 'table.column' + # 'column' => 'table_name.column' + # :column => 'table_name.column' + # Arel::Attributes::Attribute => name of the column + def transform_column(column) + if column.is_a?(String) || column.is_a?(Symbol) + column_as_string = column.to_s + column_as_string = "#{model.table_name}.#{column_as_string}" unless column_as_string.include?('.') + + Arel.sql(column_as_string) + elsif column.is_a?(Arel::Attributes::Attribute) + column + else + raise ColumnConfigurationError.new("Cannot transform the column: #{column.inspect}, please provide the column name as string") + end + end + end + end +end diff --git a/lib/gitlab/sql/recursive_cte.rb b/lib/gitlab/sql/recursive_cte.rb index e45ac5d4765..607ce10d778 100644 --- a/lib/gitlab/sql/recursive_cte.rb +++ b/lib/gitlab/sql/recursive_cte.rb @@ -23,9 +23,11 @@ module Gitlab attr_reader :table # name - The name of the CTE as a String or Symbol. - def initialize(name) + # union_args - The arguments supplied to Gitlab::SQL::Union class when building inner recursive query + def initialize(name, union_args: {}) @table = Arel::Table.new(name) @queries = [] + @union_args = union_args end # Adds a query to the body of the CTE. @@ -37,7 +39,7 @@ module Gitlab # Returns the Arel relation for this CTE. def to_arel - sql = Arel::Nodes::SqlLiteral.new(Union.new(@queries).to_sql) + sql = Arel::Nodes::SqlLiteral.new(Union.new(@queries, **@union_args).to_sql) Arel::Nodes::As.new(table, Arel::Nodes::Grouping.new(sql)) end diff --git a/lib/gitlab/sql/set_operator.rb b/lib/gitlab/sql/set_operator.rb index d58a1415493..59a808eafa9 100644 --- a/lib/gitlab/sql/set_operator.rb +++ b/lib/gitlab/sql/set_operator.rb @@ -8,6 +8,9 @@ module Gitlab # ORDER BYs are dropped from the relations as the final sort order is not # guaranteed any way. # + # remove_order: false option can be used in special cases where the + # ORDER BY is necessary for the query. + # # Example usage: # # union = Gitlab::SQL::Union.new([user.personal_projects, user.projects]) @@ -15,9 +18,10 @@ module Gitlab # # Project.where("id IN (#{sql})") class SetOperator - def initialize(relations, remove_duplicates: true) + def initialize(relations, remove_duplicates: true, remove_order: true) @relations = relations @remove_duplicates = remove_duplicates + @remove_order = remove_order end def self.operator_keyword @@ -30,7 +34,9 @@ module Gitlab # By using "unprepared_statements" we remove the usage of placeholders # (thus fixing this problem), at a slight performance cost. fragments = ActiveRecord::Base.connection.unprepared_statement do - relations.map { |rel| rel.reorder(nil).to_sql }.reject(&:blank?) + relations.map do |rel| + remove_order ? rel.reorder(nil).to_sql : rel.to_sql + end.reject(&:blank?) end if fragments.any? @@ -47,7 +53,7 @@ module Gitlab private - attr_reader :relations, :remove_duplicates + attr_reader :relations, :remove_duplicates, :remove_order end end end diff --git a/lib/gitlab/sql/union.rb b/lib/gitlab/sql/union.rb index 7fb3487a5e5..c4e95284c50 100644 --- a/lib/gitlab/sql/union.rb +++ b/lib/gitlab/sql/union.rb @@ -4,8 +4,8 @@ module Gitlab module SQL # Class for building SQL UNION statements. # - # ORDER BYs are dropped from the relations as the final sort order is not - # guaranteed any way. + # By default ORDER BYs are dropped from the relations as the final sort + # order is not guaranteed any way. # # Example usage: # diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 4e0c512c234..80765e8540e 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3533,6 +3533,9 @@ msgstr "" msgid "An error occurred while loading the file. Please try again later." msgstr "" +msgid "An error occurred while loading the file. Please try again." +msgstr "" + msgid "An error occurred while loading the merge request changes." msgstr "" diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index 4730679feb8..34601cab24f 100644 --- a/spec/features/projects_spec.rb +++ b/spec/features/projects_spec.rb @@ -290,6 +290,7 @@ RSpec.describe 'Project' do let(:project) { create(:forked_project_with_submodules) } before do + stub_feature_flags(refactor_blob_viewer: false) project.add_maintainer(user) sign_in user visit project_path(project) diff --git a/spec/frontend/packages_and_registries/shared/utils_spec.js b/spec/frontend/packages_and_registries/shared/utils_spec.js new file mode 100644 index 00000000000..73ea52d8153 --- /dev/null +++ b/spec/frontend/packages_and_registries/shared/utils_spec.js @@ -0,0 +1,35 @@ +import { + getQueryParams, + keyValueToFilterToken, + searchArrayToFilterTokens, +} from '~/packages_and_registries/shared/utils'; + +describe('Packages And Registries shared utils', () => { + describe('getQueryParams', () => { + it('returns an object from a query string, with arrays', () => { + const queryString = 'foo=bar&baz[]=1&baz[]=2'; + + expect(getQueryParams(queryString)).toStrictEqual({ foo: 'bar', baz: ['1', '2'] }); + }); + }); + + describe('keyValueToFilterToken', () => { + it('returns an object in the correct form', () => { + const type = 'myType'; + const data = 1; + + expect(keyValueToFilterToken(type, data)).toStrictEqual({ type, value: { data } }); + }); + }); + + describe('searchArrayToFilterTokens', () => { + it('returns an array of objects in the correct form', () => { + const search = ['one', 'two']; + + expect(searchArrayToFilterTokens(search)).toStrictEqual([ + { type: 'filtered-search-term', value: { data: 'one' } }, + { type: 'filtered-search-term', value: { data: 'two' } }, + ]); + }); + }); +}); diff --git a/spec/frontend/registry/explorer/pages/list_spec.js b/spec/frontend/registry/explorer/pages/list_spec.js index f7f207cc183..901c649692b 100644 --- a/spec/frontend/registry/explorer/pages/list_spec.js +++ b/spec/frontend/registry/explorer/pages/list_spec.js @@ -4,6 +4,7 @@ import VueApollo from 'vue-apollo'; import createMockApollo from 'helpers/mock_apollo_helper'; import waitForPromises from 'helpers/wait_for_promises'; import getContainerRepositoriesQuery from 'shared_queries/container_registry/get_container_repositories.query.graphql'; +import { FILTERED_SEARCH_TERM } from '~/packages_and_registries/shared/constants'; import DeleteImage from '~/registry/explorer/components/delete_image.vue'; import CliCommands from '~/registry/explorer/components/list_page/cli_commands.vue'; import GroupEmptyState from '~/registry/explorer/components/list_page/group_empty_state.vue'; @@ -343,7 +344,7 @@ describe('List Page', () => { const doSearch = async () => { await waitForApolloRequestRender(); findRegistrySearch().vm.$emit('filter:changed', [ - { type: 'filtered-search-term', value: { data: 'centos6' } }, + { type: FILTERED_SEARCH_TERM, value: { data: 'centos6' } }, ]); findRegistrySearch().vm.$emit('filter:submit'); diff --git a/spec/frontend/repository/components/blob_content_viewer_spec.js b/spec/frontend/repository/components/blob_content_viewer_spec.js new file mode 100644 index 00000000000..a0d00ff9a7d --- /dev/null +++ b/spec/frontend/repository/components/blob_content_viewer_spec.js @@ -0,0 +1,85 @@ +import { GlLoadingIcon } from '@gitlab/ui'; +import { shallowMount } from '@vue/test-utils'; +import BlobContent from '~/blob/components/blob_content.vue'; +import BlobHeader from '~/blob/components/blob_header.vue'; +import BlobContentViewer from '~/repository/components/blob_content_viewer.vue'; + +let wrapper; +const mockData = { + name: 'some_file.js', + size: 123, + rawBlob: 'raw content', + type: 'text', + fileType: 'text', + tooLarge: false, + path: 'some_file.js', + editBlobPath: 'some_file.js/edit', + ideEditPath: 'some_file.js/ide/edit', + storedExternally: false, + rawPath: 'some_file.js', + externalStorageUrl: 'some_file.js', + replacePath: 'some_file.js/replace', + deletePath: 'some_file.js/delete', + canLock: true, + isLocked: false, + lockLink: 'some_file.js/lock', + canModifyBlob: true, + forkPath: 'some_file.js/fork', + simpleViewer: {}, + richViewer: {}, +}; + +function factory(path, loading = false) { + wrapper = shallowMount(BlobContentViewer, { + propsData: { + path, + }, + mocks: { + $apollo: { + queries: { + blobInfo: { + loading, + }, + }, + }, + }, + }); + + wrapper.setData({ blobInfo: mockData }); +} + +describe('Blob content viewer component', () => { + const findLoadingIcon = () => wrapper.find(GlLoadingIcon); + const findBlobHeader = () => wrapper.find(BlobHeader); + const findBlobContent = () => wrapper.find(BlobContent); + + afterEach(() => { + wrapper.destroy(); + }); + + beforeEach(() => { + factory('some_file.js'); + }); + + it('renders a GlLoadingIcon component', () => { + factory('some_file.js', true); + + expect(findLoadingIcon().exists()).toBe(true); + }); + + it('renders a BlobHeader component', () => { + expect(findBlobHeader().exists()).toBe(true); + }); + + it('renders a BlobContent component', () => { + expect(findBlobContent().exists()).toBe(true); + + expect(findBlobContent().props('loading')).toEqual(false); + expect(findBlobContent().props('content')).toEqual('raw content'); + expect(findBlobContent().props('activeViewer')).toEqual({ + fileType: 'text', + tooLarge: false, + type: 'text', + }); + }); +}); diff --git a/spec/frontend/repository/components/table/row_spec.js b/spec/frontend/repository/components/table/row_spec.js index 69cb69de5df..3ebffbedcdb 100644 --- a/spec/frontend/repository/components/table/row_spec.js +++ b/spec/frontend/repository/components/table/row_spec.js @@ -19,6 +19,9 @@ function factory(propsData = {}) { projectPath: 'gitlab-org/gitlab-ce', url: `https://test.com`, }, + provide: { + glFeatures: { refactorBlobViewer: true }, + }, mocks: { $router, }, @@ -81,7 +84,7 @@ describe('Repository table row component', () => { it.each` type | component | componentName ${'tree'} | ${RouterLinkStub} | ${'RouterLink'} - ${'file'} | ${'a'} | ${'hyperlink'} + ${'blob'} | ${RouterLinkStub} | ${'RouterLink'} ${'commit'} | ${'a'} | ${'hyperlink'} `('renders a $componentName for type $type', ({ type, component }) => { factory({ diff --git a/spec/frontend/repository/router_spec.js b/spec/frontend/repository/router_spec.js index 3c7dda05ca3..3354b2315fc 100644 --- a/spec/frontend/repository/router_spec.js +++ b/spec/frontend/repository/router_spec.js @@ -1,3 +1,4 @@ +import BlobPage from '~/repository/pages/blob.vue'; import IndexPage from '~/repository/pages/index.vue'; import TreePage from '~/repository/pages/tree.vue'; import createRouter from '~/repository/router'; @@ -11,6 +12,7 @@ describe('Repository router spec', () => { ${'/-/tree/master'} | ${'master'} | ${TreePage} | ${'TreePage'} ${'/-/tree/master/app/assets'} | ${'master'} | ${TreePage} | ${'TreePage'} ${'/-/tree/123/app/assets'} | ${'master'} | ${null} | ${'null'} + ${'/-/blob/master/file.md'} | ${'master'} | ${BlobPage} | ${'BlobPage'} `('sets component as $componentName for path "$path"', ({ path, component, branch }) => { const router = createRouter('', branch); diff --git a/spec/frontend/vue_shared/components/registry/registry_search_spec.js b/spec/frontend/vue_shared/components/registry/registry_search_spec.js index 28bdb275756..f5ef5b3d443 100644 --- a/spec/frontend/vue_shared/components/registry/registry_search_spec.js +++ b/spec/frontend/vue_shared/components/registry/registry_search_spec.js @@ -1,5 +1,6 @@ import { GlSorting, GlSortingItem, GlFilteredSearch } from '@gitlab/ui'; import { shallowMount } from '@vue/test-utils'; +import { FILTERED_SEARCH_TERM } from '~/packages_and_registries/shared/constants'; import component from '~/vue_shared/components/registry/registry_search.vue'; describe('Registry Search', () => { @@ -12,8 +13,18 @@ describe('Registry Search', () => { const defaultProps = { filter: [], sorting: { sort: 'asc', orderBy: 'name' }, - tokens: ['foo'], - sortableFields: [{ label: 'name', orderBy: 'name' }, { label: 'baz' }], + tokens: [{ type: 'foo' }], + sortableFields: [ + { label: 'name', orderBy: 'name' }, + { label: 'baz', orderBy: 'bar' }, + ], + }; + + const defaultQueryChangedPayload = { + foo: '', + orderBy: 'name', + search: [], + sort: 'asc', }; const mountComponent = (propsData = defaultProps) => { @@ -55,20 +66,22 @@ describe('Registry Search', () => { expect(wrapper.emitted('filter:changed')).toEqual([['foo']]); }); - it('emits filter:submit on submit event', () => { + it('emits filter:submit and query:changed on submit event', () => { mountComponent(); findFilteredSearch().vm.$emit('submit'); expect(wrapper.emitted('filter:submit')).toEqual([[]]); + expect(wrapper.emitted('query:changed')).toEqual([[defaultQueryChangedPayload]]); }); - it('emits filter:changed and filter:submit on clear event', () => { + it('emits filter:changed, filter:submit and query:changed on clear event', () => { mountComponent(); findFilteredSearch().vm.$emit('clear'); expect(wrapper.emitted('filter:changed')).toEqual([[[]]]); expect(wrapper.emitted('filter:submit')).toEqual([[]]); + expect(wrapper.emitted('query:changed')).toEqual([[defaultQueryChangedPayload]]); }); it('binds tokens prop', () => { @@ -90,15 +103,47 @@ describe('Registry Search', () => { findPackageListSorting().vm.$emit('sortDirectionChange'); expect(wrapper.emitted('sorting:changed')).toEqual([[{ sort: 'desc' }]]); + expect(wrapper.emitted('query:changed')).toEqual([ + [{ ...defaultQueryChangedPayload, sort: 'desc' }], + ]); }); it('on sort item click emits sorting:changed event ', () => { mountComponent(); - findSortingItems().at(0).vm.$emit('click'); + findSortingItems().at(1).vm.$emit('click'); expect(wrapper.emitted('sorting:changed')).toEqual([ - [{ orderBy: defaultProps.sortableFields[0].orderBy }], + [{ orderBy: defaultProps.sortableFields[1].orderBy }], + ]); + expect(wrapper.emitted('query:changed')).toEqual([ + [{ ...defaultQueryChangedPayload, orderBy: 'bar' }], + ]); + }); + }); + + describe('query string calculation', () => { + const filter = [ + { type: FILTERED_SEARCH_TERM, value: { data: 'one' } }, + { type: FILTERED_SEARCH_TERM, value: { data: 'two' } }, + { type: 'typeOne', value: { data: 'value_one' } }, + { type: 'typeTwo', value: { data: 'value_two' } }, + ]; + + it('aggregates the filter in the correct object', () => { + mountComponent({ ...defaultProps, filter }); + + findFilteredSearch().vm.$emit('submit'); + + expect(wrapper.emitted('query:changed')).toEqual([ + [ + { + ...defaultQueryChangedPayload, + search: ['one', 'two'], + typeOne: 'value_one', + typeTwo: 'value_two', + }, + ], ]); }); }); diff --git a/spec/frontend/vue_shared/components/url_sync_spec.js b/spec/frontend/vue_shared/components/url_sync_spec.js new file mode 100644 index 00000000000..86bbc146c5f --- /dev/null +++ b/spec/frontend/vue_shared/components/url_sync_spec.js @@ -0,0 +1,97 @@ +import { shallowMount } from '@vue/test-utils'; +import setWindowLocation from 'helpers/set_window_location_helper'; +import { historyPushState } from '~/lib/utils/common_utils'; +import { mergeUrlParams } from '~/lib/utils/url_utility'; +import UrlSyncComponent from '~/vue_shared/components/url_sync.vue'; + +jest.mock('~/lib/utils/url_utility', () => ({ + mergeUrlParams: jest.fn((query, url) => `urlParams: ${query} ${url}`), +})); + +jest.mock('~/lib/utils/common_utils', () => ({ + historyPushState: jest.fn(), +})); + +describe('url sync component', () => { + let wrapper; + const mockQuery = { group_id: '5014437163714', project_ids: ['5014437608314'] }; + const TEST_HOST = 'http://testhost/'; + + setWindowLocation(TEST_HOST); + + const findButton = () => wrapper.find('button'); + + const createComponent = ({ query = mockQuery, scopedSlots, slots } = {}) => { + wrapper = shallowMount(UrlSyncComponent, { + propsData: { query }, + scopedSlots, + slots, + }); + }; + + afterEach(() => { + wrapper.destroy(); + }); + + const expectUrlSync = (query, times, mergeUrlParamsReturnValue) => { + expect(mergeUrlParams).toHaveBeenCalledTimes(times); + expect(mergeUrlParams).toHaveBeenCalledWith(query, TEST_HOST, { spreadArrays: true }); + + expect(historyPushState).toHaveBeenCalledTimes(times); + expect(historyPushState).toHaveBeenCalledWith(mergeUrlParamsReturnValue); + }; + + describe('with query as a props', () => { + it('immediately syncs the query to the URL', () => { + createComponent(); + + expectUrlSync(mockQuery, 1, mergeUrlParams.mock.results[0].value); + }); + + describe('when the query is modified', () => { + const newQuery = { foo: true }; + + it('updates the URL with the new query', async () => { + createComponent(); + // using setProps to test the watcher + await wrapper.setProps({ query: newQuery }); + + expectUrlSync(mockQuery, 2, mergeUrlParams.mock.results[1].value); + }); + }); + }); + + describe('with scoped slot', () => { + const scopedSlots = { + default: ` + <button @click="props.updateQuery({bar: 'baz'})">Update Query </button> + `, + }; + + it('renders the scoped slot', () => { + createComponent({ query: null, scopedSlots }); + + expect(findButton().exists()).toBe(true); + }); + + it('syncs the url with the scoped slots function', () => { + createComponent({ query: null, scopedSlots }); + + findButton().trigger('click'); + + expectUrlSync({ bar: 'baz' }, 1, mergeUrlParams.mock.results[0].value); + }); + }); + + describe('with slot', () => { + const slots = { + default: '<button>Normal Slot</button>', + }; + + it('renders the default slot', () => { + createComponent({ query: null, slots }); + + expect(findButton().exists()).toBe(true); + }); + }); +}); diff --git a/spec/lib/gitlab/database/batch_count_spec.rb b/spec/lib/gitlab/database/batch_count_spec.rb index 29688b18e94..da13bc425d1 100644 --- a/spec/lib/gitlab/database/batch_count_spec.rb +++ b/spec/lib/gitlab/database/batch_count_spec.rb @@ -270,6 +270,8 @@ RSpec.describe Gitlab::Database::BatchCount do end it "defaults the batch size to #{Gitlab::Database::BatchCounter::DEFAULT_DISTINCT_BATCH_SIZE}" do + stub_feature_flags(loose_index_scan_for_distinct_values: false) + min_id = model.minimum(:id) relation = instance_double(ActiveRecord::Relation) allow(model).to receive_message_chain(:select, public_send: relation) @@ -315,13 +317,85 @@ RSpec.describe Gitlab::Database::BatchCount do end end - it_behaves_like 'when batch fetch query is canceled' do + context 'when the loose_index_scan_for_distinct_values feature flag is off' do + it_behaves_like 'when batch fetch query is canceled' do + let(:mode) { :distinct } + let(:operation) { :count } + let(:operation_args) { nil } + let(:column) { nil } + + subject { described_class.method(:batch_distinct_count) } + + before do + stub_feature_flags(loose_index_scan_for_distinct_values: false) + end + end + end + + context 'when the loose_index_scan_for_distinct_values feature flag is on' do let(:mode) { :distinct } let(:operation) { :count } let(:operation_args) { nil } let(:column) { nil } + let(:batch_size) { 10_000 } + subject { described_class.method(:batch_distinct_count) } + + before do + stub_feature_flags(loose_index_scan_for_distinct_values: true) + end + + it 'reduces batch size by half and retry fetch' do + too_big_batch_relation_mock = instance_double(ActiveRecord::Relation) + + count_method = double(send: 1) + + allow(too_big_batch_relation_mock).to receive(:send).and_raise(ActiveRecord::QueryCanceled) + allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive_message_chain(:new, :build_query).with(from: 0, to: batch_size).and_return(too_big_batch_relation_mock) + allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive_message_chain(:new, :build_query).with(from: 0, to: batch_size / 2).and_return(count_method) + allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive_message_chain(:new, :build_query).with(from: batch_size / 2, to: batch_size).and_return(count_method) + + subject.call(model, column, batch_size: batch_size, start: 0, finish: batch_size - 1) + end + + context 'when all retries fail' do + let(:batch_count_query) { 'SELECT COUNT(id) FROM relation WHERE id BETWEEN 0 and 1' } + + before do + relation = instance_double(ActiveRecord::Relation) + allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive_message_chain(:new, :build_query).and_return(relation) + allow(relation).to receive(:send).and_raise(ActiveRecord::QueryCanceled.new('query timed out')) + allow(relation).to receive(:to_sql).and_return(batch_count_query) + end + + it 'logs failing query' do + expect(Gitlab::AppJsonLogger).to receive(:error).with( + event: 'batch_count', + relation: model.table_name, + operation: operation, + operation_args: operation_args, + start: 0, + mode: mode, + query: batch_count_query, + message: 'Query has been canceled with message: query timed out' + ) + expect(subject.call(model, column, batch_size: batch_size, start: 0)).to eq(-1) + end + end + + context 'when LooseIndexScanDistinctCount raises error' do + let(:column) { :creator_id } + let(:error_class) { Gitlab::Database::LooseIndexScanDistinctCount::ColumnConfigurationError } + + it 'rescues ColumnConfigurationError' do + allow(Gitlab::Database::LooseIndexScanDistinctCount).to receive(:new).and_raise(error_class.new('error message')) + + expect(Gitlab::AppJsonLogger).to receive(:error).with(a_hash_including(message: 'LooseIndexScanDistinctCount column error: error message')) + + expect(subject.call(Project, column, batch_size: 10_000, start: 0)).to eq(-1) + end + end end end diff --git a/spec/lib/gitlab/database/loose_index_scan_distinct_count_spec.rb b/spec/lib/gitlab/database/loose_index_scan_distinct_count_spec.rb new file mode 100644 index 00000000000..e0eac26e4d9 --- /dev/null +++ b/spec/lib/gitlab/database/loose_index_scan_distinct_count_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Database::LooseIndexScanDistinctCount do + context 'counting distinct users' do + let_it_be(:user) { create(:user) } + let_it_be(:other_user) { create(:user) } + + let(:column) { :creator_id } + + before_all do + create_list(:project, 3, creator: user) + create_list(:project, 1, creator: other_user) + end + + subject(:count) { described_class.new(Project, :creator_id).count(from: Project.minimum(:creator_id), to: Project.maximum(:creator_id) + 1) } + + it { is_expected.to eq(2) } + + context 'when STI model is queried' do + it 'does not raise error' do + expect { described_class.new(Group, :owner_id).count(from: 0, to: 1) }.not_to raise_error + end + end + + context 'when model with default_scope is queried' do + it 'does not raise error' do + expect { described_class.new(GroupMember, :id).count(from: 0, to: 1) }.not_to raise_error + end + end + + context 'when the fully qualified column is given' do + let(:column) { 'projects.creator_id' } + + it { is_expected.to eq(2) } + end + + context 'when AR attribute is given' do + let(:column) { Project.arel_table[:creator_id] } + + it { is_expected.to eq(2) } + end + + context 'when invalid value is given for the column' do + let(:column) { Class.new } + + it { expect { described_class.new(Group, column) }.to raise_error(Gitlab::Database::LooseIndexScanDistinctCount::ColumnConfigurationError) } + end + + context 'when null values are present' do + before do + create_list(:project, 2).each { |p| p.update_column(:creator_id, nil) } + end + + it { is_expected.to eq(2) } + end + end + + context 'counting STI models' do + let!(:groups) { create_list(:group, 3) } + let!(:namespaces) { create_list(:namespace, 2) } + + let(:max_id) { Namespace.maximum(:id) + 1 } + + it 'counts groups' do + count = described_class.new(Group, :id).count(from: 0, to: max_id) + expect(count).to eq(3) + end + end +end diff --git a/spec/requests/api/conan_project_packages_spec.rb b/spec/requests/api/conan_project_packages_spec.rb index fefaf9790b1..da054ed2e96 100644 --- a/spec/requests/api/conan_project_packages_spec.rb +++ b/spec/requests/api/conan_project_packages_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'spec_helper' -RSpec.describe API::ConanProjectPackages do +RSpec.describe API::ConanProjectPackages, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/326194' do include_context 'conan api setup' let(:project_id) { project.id } diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index 8113de96ac4..d6a0423e83c 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -3,22 +3,26 @@ require 'spec_helper' RSpec.describe API::Deployments do - let(:user) { create(:user) } - let(:non_member) { create(:user) } + let_it_be(:user) { create(:user) } + let_it_be(:non_member) { create(:user) } before do project.add_maintainer(user) end describe 'GET /projects/:id/deployments' do - let(:project) { create(:project, :repository) } - let!(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: Time.now, updated_at: Time.now) } - let!(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'master', created_at: 1.day.ago, updated_at: 2.hours.ago) } - let!(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'master', created_at: 2.days.ago, updated_at: 1.hour.ago) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:deployment_1) { create(:deployment, :success, project: project, iid: 11, ref: 'master', created_at: Time.now, updated_at: Time.now) } + let_it_be(:deployment_2) { create(:deployment, :success, project: project, iid: 12, ref: 'master', created_at: 1.day.ago, updated_at: 2.hours.ago) } + let_it_be(:deployment_3) { create(:deployment, :success, project: project, iid: 8, ref: 'master', created_at: 2.days.ago, updated_at: 1.hour.ago) } + + def perform_request(params = {}) + get api("/projects/#{project.id}/deployments", user), params: params + end context 'as member of the project' do it 'returns projects deployments sorted by id asc' do - get api("/projects/#{project.id}/deployments", user) + perform_request expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -32,7 +36,7 @@ RSpec.describe API::Deployments do context 'with updated_at filters specified' do it 'returns projects deployments with last update in specified datetime range' do - get api("/projects/#{project.id}/deployments", user), params: { updated_before: 30.minutes.ago, updated_after: 90.minutes.ago } + perform_request({ updated_before: 30.minutes.ago, updated_after: 90.minutes.ago }) expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers @@ -42,10 +46,7 @@ RSpec.describe API::Deployments do context 'with the environment filter specifed' do it 'returns deployments for the environment' do - get( - api("/projects/#{project.id}/deployments", user), - params: { environment: deployment_1.environment.name } - ) + perform_request({ environment: deployment_1.environment.name }) expect(json_response.size).to eq(1) expect(json_response.first['iid']).to eq(deployment_1.iid) @@ -86,6 +87,16 @@ RSpec.describe API::Deployments do end end end + + it 'returns multiple deployments without N + 1' do + perform_request # warm up the cache + + control_count = ActiveRecord::QueryRecorder.new { perform_request }.count + + create(:deployment, :success, project: project, iid: 21, ref: 'master') + + expect { perform_request }.not_to exceed_query_limit(control_count) + end end context 'as non member' do diff --git a/spec/requests/api/merge_request_diffs_spec.rb b/spec/requests/api/merge_request_diffs_spec.rb index 971fb5e991c..540ceab877f 100644 --- a/spec/requests/api/merge_request_diffs_spec.rb +++ b/spec/requests/api/merge_request_diffs_spec.rb @@ -75,5 +75,25 @@ RSpec.describe API::MergeRequestDiffs, 'MergeRequestDiffs' do let(:url) { "/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/#{merge_request_diff.id}" } end end + + context 'caching merge request diffs', :use_clean_rails_redis_caching do + it 'is performed' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/#{merge_request_diff.id}", user) + + expect(Rails.cache.fetch(merge_request_diff.cache_key)).to be_present + end + + context 'when cached_api_merge_request_version is disabled' do + before do + stub_feature_flags(cached_api_merge_request_version: false) + end + + it 'is not performed' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/versions/#{merge_request_diff.id}", user) + + expect(Rails.cache.fetch(merge_request_diff.cache_key)).to be_nil + end + end + end end end diff --git a/spec/support/shared_examples/lib/gitlab/sql/set_operator_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/sql/set_operator_shared_examples.rb index 73beef06855..aa6a51c3646 100644 --- a/spec/support/shared_examples/lib/gitlab/sql/set_operator_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/sql/set_operator_shared_examples.rb @@ -43,4 +43,33 @@ RSpec.shared_examples 'SQL set operator' do |operator_keyword| expect(set_operator.to_sql).to eq('NULL') end end + + describe 'remove_order parameter' do + let(:scopes) do + [ + User.where(id: 1).order(id: :desc).limit(1), + User.where(id: 2).order(id: :asc).limit(1) + ] + end + + subject(:union_query) { described_class.new(scopes, remove_order: remove_order).to_sql } + + context 'when remove_order: true' do + let(:remove_order) { true } + + it 'removes the ORDER BY from the query' do + expect(union_query).not_to include('ORDER BY "users"."id" DESC') + expect(union_query).not_to include('ORDER BY "users"."id" ASC') + end + end + + context 'when remove_order: false' do + let(:remove_order) { false } + + it 'does not remove the ORDER BY from the query' do + expect(union_query).to include('ORDER BY "users"."id" DESC') + expect(union_query).to include('ORDER BY "users"."id" ASC') + end + end + end end |