diff options
30 files changed, 276 insertions, 48 deletions
diff --git a/app/finders/clusters/knative_services_finder.rb b/app/finders/clusters/knative_services_finder.rb index 71cebe4495e..af8c42f672f 100644 --- a/app/finders/clusters/knative_services_finder.rb +++ b/app/finders/clusters/knative_services_finder.rb @@ -11,6 +11,7 @@ module Clusters }.freeze self.reactive_cache_key = ->(finder) { finder.model_name } + self.reactive_cache_work_type = :external_dependency self.reactive_cache_worker_finder = ->(_id, *cache_args) { from_cache(*cache_args) } attr_reader :cluster, :environment diff --git a/app/finders/projects/serverless/functions_finder.rb b/app/finders/projects/serverless/functions_finder.rb index 3b4ecbb5387..13f84e0e3a5 100644 --- a/app/finders/projects/serverless/functions_finder.rb +++ b/app/finders/projects/serverless/functions_finder.rb @@ -9,6 +9,7 @@ module Projects attr_reader :project self.reactive_cache_key = ->(finder) { finder.cache_key } + self.reactive_cache_work_type = :external_dependency self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } MAX_CLUSTERS = 10 diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index 430a9b3c43e..78dd876020b 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -26,6 +26,8 @@ module Clusters KUBE_INGRESS_BASE_DOMAIN = 'KUBE_INGRESS_BASE_DOMAIN' APPLICATIONS_ASSOCIATIONS = APPLICATIONS.values.map(&:association_name).freeze + self.reactive_cache_work_type = :external_dependency + belongs_to :user belongs_to :management_project, class_name: '::Project', optional: true diff --git a/app/models/concerns/prometheus_adapter.rb b/app/models/concerns/prometheus_adapter.rb index abc41a1c476..761a151a474 100644 --- a/app/models/concerns/prometheus_adapter.rb +++ b/app/models/concerns/prometheus_adapter.rb @@ -9,6 +9,7 @@ module PrometheusAdapter self.reactive_cache_lease_timeout = 30.seconds self.reactive_cache_refresh_interval = 30.seconds self.reactive_cache_lifetime = 1.minute + self.reactive_cache_work_type = :external_dependency def prometheus_client raise NotImplementedError diff --git a/app/models/environment.rb b/app/models/environment.rb index b2391f33aca..248e2716b61 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -8,6 +8,7 @@ class Environment < ApplicationRecord self.reactive_cache_refresh_interval = 1.minute self.reactive_cache_lifetime = 55.seconds self.reactive_cache_hard_limit = 10.megabytes + self.reactive_cache_work_type = :external_dependency belongs_to :project, required: true diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 133850b6ab6..fa32c8a5450 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -22,6 +22,7 @@ module ErrorTracking }x.freeze self.reactive_cache_key = ->(setting) { [setting.class.model_name.singular, setting.project_id] } + self.reactive_cache_work_type = :external_dependency belongs_to :project diff --git a/app/models/ssh_host_key.rb b/app/models/ssh_host_key.rb index 9bd35d30845..72690ad7d04 100644 --- a/app/models/ssh_host_key.rb +++ b/app/models/ssh_host_key.rb @@ -24,6 +24,7 @@ class SshHostKey # This is achieved by making the lifetime shorter than the refresh interval. self.reactive_cache_refresh_interval = 15.minutes self.reactive_cache_lifetime = 10.minutes + self.reactive_cache_work_type = :external_dependency def self.find_by(opts = {}) opts = HashWithIndifferentAccess.new(opts) diff --git a/app/services/grafana/proxy_service.rb b/app/services/grafana/proxy_service.rb index 74fcdc750b0..ac4c3cc091c 100644 --- a/app/services/grafana/proxy_service.rb +++ b/app/services/grafana/proxy_service.rb @@ -12,6 +12,7 @@ module Grafana self.reactive_cache_key = ->(service) { service.cache_key } self.reactive_cache_lease_timeout = 30.seconds self.reactive_cache_refresh_interval = 30.seconds + self.reactive_cache_work_type = :external_dependency self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } attr_accessor :project, :datasource_id, :proxy_path, :query_params diff --git a/app/services/metrics/dashboard/grafana_metric_embed_service.rb b/app/services/metrics/dashboard/grafana_metric_embed_service.rb index d58b80162f5..5fd1e0dda89 100644 --- a/app/services/metrics/dashboard/grafana_metric_embed_service.rb +++ b/app/services/metrics/dashboard/grafana_metric_embed_service.rb @@ -18,6 +18,7 @@ module Metrics self.reactive_cache_lease_timeout = 30.seconds self.reactive_cache_refresh_interval = 30.minutes self.reactive_cache_lifetime = 30.minutes + self.reactive_cache_work_type = :external_dependency self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } class << self diff --git a/app/services/pod_logs/elasticsearch_service.rb b/app/services/pod_logs/elasticsearch_service.rb index 9a9b453c554..a9da477cbc1 100644 --- a/app/services/pod_logs/elasticsearch_service.rb +++ b/app/services/pod_logs/elasticsearch_service.rb @@ -11,6 +11,7 @@ module PodLogs :pod_logs, :filter_return_keys + self.reactive_cache_work_type = :external_dependency self.reactive_cache_worker_finder = ->(id, _cache_key, namespace, params) { new(::Clusters::Cluster.find(id), namespace, params: params) } private diff --git a/app/services/pod_logs/kubernetes_service.rb b/app/services/pod_logs/kubernetes_service.rb index 0a8072a9037..f59fc60113f 100644 --- a/app/services/pod_logs/kubernetes_service.rb +++ b/app/services/pod_logs/kubernetes_service.rb @@ -17,6 +17,7 @@ module PodLogs :split_logs, :filter_return_keys + self.reactive_cache_work_type = :external_dependency self.reactive_cache_worker_finder = ->(id, _cache_key, namespace, params) { new(::Clusters::Cluster.find(id), namespace, params: params) } private diff --git a/app/services/prometheus/proxy_service.rb b/app/services/prometheus/proxy_service.rb index 99c739a630b..085cfc76196 100644 --- a/app/services/prometheus/proxy_service.rb +++ b/app/services/prometheus/proxy_service.rb @@ -17,6 +17,7 @@ module Prometheus # is expected to change *and* be fetched again by the frontend self.reactive_cache_refresh_interval = 90.seconds self.reactive_cache_lifetime = 1.minute + self.reactive_cache_work_type = :external_dependency self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } attr_accessor :proxyable, :method, :path, :params diff --git a/changelogs/unreleased/214983-remove_additional_rows_in_application_settings.yml b/changelogs/unreleased/214983-remove_additional_rows_in_application_settings.yml new file mode 100644 index 00000000000..bea090b5039 --- /dev/null +++ b/changelogs/unreleased/214983-remove_additional_rows_in_application_settings.yml @@ -0,0 +1,5 @@ +--- +title: Delete orphaned rows in application_settings table +merge_request: 29981 +author: +type: performance diff --git a/db/post_migrate/20200420162730_remove_additional_application_settings_rows.rb b/db/post_migrate/20200420162730_remove_additional_application_settings_rows.rb new file mode 100644 index 00000000000..e4a0ec1eb4a --- /dev/null +++ b/db/post_migrate/20200420162730_remove_additional_application_settings_rows.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class RemoveAdditionalApplicationSettingsRows < ActiveRecord::Migration[6.0] + class ApplicationSetting < ActiveRecord::Base + self.table_name = 'application_settings' + end + + def up + return if ApplicationSetting.count == 1 + + execute "DELETE from application_settings WHERE id NOT IN (SELECT MAX(id) FROM application_settings);" + end + + def down + # no changes + end +end diff --git a/db/structure.sql b/db/structure.sql index 96211531462..01c4f8fefa1 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -13421,6 +13421,7 @@ COPY "schema_migrations" (version) FROM STDIN; 20200416120128 20200416120354 20200417044453 +20200420162730 20200420172113 20200420172752 20200420172927 diff --git a/doc/api/container_registry.md b/doc/api/container_registry.md index a41a71808ce..2e8656fef60 100644 --- a/doc/api/container_registry.md +++ b/doc/api/container_registry.md @@ -223,6 +223,9 @@ This action does not delete blobs. In order to delete them and recycle disk spac Delete registry repository tags in bulk based on given criteria. +<i class="fa fa-youtube-play youtube" aria-hidden="true"></i> +For an overview, see [Utilize the Container Registry API to delete all tags except *](https://youtu.be/Hi19bKe_xsg). + ```plaintext DELETE /projects/:id/registry/repositories/:repository_id/tags ``` diff --git a/doc/gitlab-basics/start-using-git.md b/doc/gitlab-basics/start-using-git.md index 2ff2af33b16..9ebcf258ee9 100644 --- a/doc/gitlab-basics/start-using-git.md +++ b/doc/gitlab-basics/start-using-git.md @@ -126,11 +126,11 @@ To start working locally on an existing remote repository, clone it with the com files to your local computer, automatically preserving the Git connection with the remote repository. -You can either clone it via HTTPS or [SSH](../ssh/README.md). If you chose to clone -it via HTTPS, you'll have to enter your credentials every time you pull and push. +You can either clone it via [HTTPS](#clone-via-https) or [SSH](#clone-via-ssh). If you chose to +clone it via HTTPS, you'll have to enter your credentials every time you pull and push. You can read more about credential storage in the [Git Credentials documentation](https://git-scm.com/book/en/v2/Git-Tools-Credential-Storage). -With SSH, you enter your credentials only once. +With [SSH](../ssh/README.md), you enter your credentials only once. You can find both paths (HTTPS and SSH) by navigating to your project's landing page and clicking **Clone**. GitLab will prompt you with both paths, from which you can copy @@ -142,24 +142,38 @@ As an example, consider this repository path: - SSH: `git@gitlab.com:gitlab-org/gitlab.git` To get started, open a terminal window in the directory you wish to clone the -repository files into, and run one of the following commands. +repository files into, and run one of the `git clone` commands as described below. -Clone via HTTPS: +Both commands will download a copy of the files in a folder named after the project's +name. You can then navigate to the new directory and start working on it locally. + +#### Clone via HTTPS + +To clone `https://gitlab.com/gitlab-org/gitlab.git` via HTTPS: ```shell git clone https://gitlab.com/gitlab-org/gitlab.git ``` -Clone via SSH: +You'll have to add your password every time you clone through HTTPS. If you have 2FA enabled +for your account, you'll have to use a [Personal Access Token](../user/profile/personal_access_tokens.md) +with **read_repository** or **write_repository** permissions instead of your account's password. + +If you don't have 2FA enabled, use your account's password. + +TIP: **Troubleshooting:** +On Windows, if you entered incorrect passwords multiple times and GitLab is responding `Access denied`, +you may have to add your namespace (user name or group name) to clone through HTTPS: +`git clone https://namespace@gitlab.com/gitlab-org/gitlab.git`. + +#### Clone via SSH + +To clone `git@gitlab.com:gitlab-org/gitlab.git` via SSH: ```shell git clone git@gitlab.com:gitlab-org/gitlab.git ``` -Both commands will download a copy of the files in a folder named after the project's -name. You can then navigate to the directory and start working -on it locally. - ### Switch to the master branch You are always in a branch when working with Git. The main branch is the master diff --git a/doc/telemetry/snowplow.md b/doc/telemetry/snowplow.md index 7b5b1667b23..02e161b8662 100644 --- a/doc/telemetry/snowplow.md +++ b/doc/telemetry/snowplow.md @@ -17,7 +17,7 @@ When working within HAML (or Vue templates) we can add `data-track-*` attributes Below is an example of `data-track-*` attributes assigned to a button: ```haml -%button.btn{ data: { track: { event: "click_button", label: "template_preview", property: "my-template" } } } +%button.btn{ data: { track_event: "click_button", track_label: "template_preview", track_property: "my-template" } } ``` ```html diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index e9fcd90a31a..00fe00cce6a 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -189,6 +189,9 @@ Once you've set up your identity provider to work with GitLab, you'll need to co  +NOTE: **Note:** +Please note that the certificate [fingerprint algorithm](#additional-setup-options) must be in SHA1. When configuring the identity provider, use a secure [signature algorithm](#additional-setup-options). + ## User access and management Once Group SSO is configured and enabled, users can access the GitLab.com group through the identity provider's dashboard. If [SCIM](scim_setup.md) is configured, please see the [user access and linking setup section on the SCIM page](scim_setup.md#user-access-and-linking-setup). diff --git a/doc/user/group/saml_sso/scim_setup.md b/doc/user/group/saml_sso/scim_setup.md index e333fd19c1b..d6c4952d03a 100644 --- a/doc/user/group/saml_sso/scim_setup.md +++ b/doc/user/group/saml_sso/scim_setup.md @@ -12,25 +12,28 @@ that group is synchronized between GitLab and the identity provider. GitLab's [SCIM API](../../../api/scim.md) implements part of [the RFC7644 protocol](https://tools.ietf.org/html/rfc7644). +## Features + Currently, the following actions are available: -- CREATE -- UPDATE -- DELETE (deprovisioning) +- Create users +- Update users (Azure only) +- Deactivate users The following identity providers are supported: - Azure +- Okta ## Requirements -- [Group SSO](index.md) must be configured. +- [Group Single Sign-On](index.md) must be configured. ## GitLab configuration -Once [Single sign-on](index.md) has been configured, we can: +Once [Group Single Sign-On](index.md) has been configured, we can: -1. Navigate to the group and click **Settings > SAML SSO**. +1. Navigate to the group and click **Administration > SAML SSO**. 1. Click on the **Generate a SCIM token** button. 1. Save the token and URL so they can be used in the next step. @@ -38,9 +41,12 @@ Once [Single sign-on](index.md) has been configured, we can: ## Identity Provider configuration -### Azure +- [Azure](#azure-configuration-steps) +- [Okta](#okta-configuration-steps) -The SAML application that was created during [Single sign-on](index.md) setup now needs to be set up for SCIM. +### Azure configuration steps + +The SAML application that was created during [Single sign-on](index.md) setup for [Azure](https://docs.microsoft.com/en-us/azure/active-directory/manage-apps/configure-single-sign-on-non-gallery-applications) now needs to be set up for SCIM. 1. Check the configuration for your GitLab SAML app and ensure that **Name identifier value** (NameID) points to `user.objectid` or another unique identifier. This will match the `extern_uid` used on GitLab. @@ -109,6 +115,38 @@ bottom of the **Provisioning** screen, together with a link to the audit logs. CAUTION: **Warning:** Once synchronized, changing the field mapped to `id` and `externalId` will likely cause provisioning errors, duplicate users, and prevent existing users from accessing the GitLab group. +### Okta configuration steps + +The SAML application that was created during [Single sign-on](index.md) setup for [Okta](The SAML application that was created during [Single sign-on](index.md) setup for [Okta](https://developer.okta.com/docs/guides/saml-application-setup/overview/) now needs to be set up for SCIM. + +1. Sign in to Okta. +1. If you see an **Admin** button in the top right, click the button. This will + ensure you are in the Admin area. + + TIP: **Tip:** If you're using the Developer Console, click **Developer Console** in the top + bar and select **Classic UI**. Otherwise, you may not see the buttons described + in the following steps: + +1. In the **Application** tab, click **Add Application**. +1. Search for **GitLab**, find and click on the 'GitLab' application. +1. On the GitLab application overview page, click **Add**. +1. Under **Application Visibility** select both check boxes. Currently the GitLab application does not support SAML authentication so the icon should not be shown to users. +1. Click **Done** to finish adding the application. +1. In the **Provisioning** tab, click **Configure API integration**. +1. Select **Enable API integration**. + - For **Base URL** enter the URL obtained from the GitLab SCIM configuration page + - For **API Token** enter the SCIM token obtained from the GitLab SCIM configuration page +1. Click 'Test API Credentials' to verify configuration. +1. Click **Save** to apply the settings. +1. Assign users in the **Assignments** tab. Assigned users will be created and + managed in your GitLab group. + +#### Okta Known Issues + +The Okta GitLab application currently only supports SCIM. Continue +using the separate Okta [SAML SSO](index.md) configuration along with the new SCIM +application described above. + ## User access and linking setup As long as [Group SAML](index.md) has been configured, prior to turning on sync, existing GitLab.com users can link to their accounts in one of the following ways, before synchronization is active: @@ -135,7 +173,9 @@ Upon the next sync, the user will be deprovisioned, which means that the user wi This section contains possible solutions for problems you might encounter. -### How do I verify my SCIM configuration is correct? +### Azure + +#### How do I verify my SCIM configuration is correct? Review the following: @@ -148,11 +188,11 @@ Review the following SCIM parameters for sensible values: - `displayName` - `emails[type eq "work"].value` -### Testing Azure connection: invalid credentials +#### Testing Azure connection: invalid credentials When testing the connection, you may encounter an error: **You appear to have entered invalid credentials. Please confirm you are using the correct information for an administrative account**. If `Tenant URL` and `secret token` are correct, check whether your group path contains characters that may be considered invalid JSON primitives (such as `.`). Removing such characters from the group path typically resolves the error. -### Azure: (Field) can't be blank sync error +#### Azure: (Field) can't be blank sync error When checking the Audit Logs for the Provisioning, you can sometimes see the error `Namespace can't be blank, Name can't be blank, and User can't be blank.` @@ -165,14 +205,7 @@ As a workaround, try an alternate mapping: 1. Delete the `name.formatted` target attribute entry. 1. Change the `displayName` source attribute to have `name.formatted` target attribute. -### Message: "SAML authentication failed: Email has already been taken" - -This message may be caused by the following: - -- Existing users have not yet signed into the new app. -- The identity provider attempts to create a new user account in GitLab with an email address that already exists in GitLab.com. - -### How do I diagnose why a user is unable to sign in +#### How do I diagnose why a user is unable to sign in The **Identity** (`extern_uid`) value stored by GitLab is updated by SCIM whenever `id` or `externalId` changes. Users won't be able to sign in unless the GitLab Identity (`extern_uid`) value matches the `NameId` sent by SAML. @@ -180,7 +213,7 @@ This value is also used by SCIM to match users on the `id`, and is updated by SC It is important that this SCIM `id` and SCIM `externalId` are configured to the same value as the SAML `NameId`. SAML responses can be traced using [debugging tools](./index.md#saml-debugging-tools), and any errors can be checked against our [SAML troubleshooting docs](./index.md#troubleshooting). -### How do I verify user's SAML NameId matches the SCIM externalId +#### How do I verify user's SAML NameId matches the SCIM externalId Group owners can see the list of users and the `externalId` stored for each user in the group SAML SSO Settings page. @@ -194,7 +227,7 @@ curl 'https://example.gitlab.com/api/scim/v2/groups/GROUP_NAME/Users?startIndex= To see how this compares to the value returned as the SAML NameId, you can have the user use a [SAML Tracer](index.md#saml-debugging-tools). -### Update or fix mismatched SCIM externalId and SAML NameId +#### Update or fix mismatched SCIM externalId and SAML NameId Whether the value was changed or you need to map to a different field, ensure `id`, `externalId`, and `NameId` all map to the same field. @@ -220,7 +253,7 @@ curl --verbose --request PATCH 'https://gitlab.com/api/scim/v2/groups/YOUR_GROUP It is important not to update these to incorrect values, since this will cause users to be unable to sign in. It is also important not to assign a value to the wrong user, as this would cause users to get signed into the wrong account. -### I need to change my SCIM app +#### I need to change my SCIM app Individual users can follow the instructions in the ["SAML authentication failed: User has already been taken"](./index.md#i-need-to-change-my-saml-app) section. diff --git a/package.json b/package.json index ca68bc0176e..ec77c17a82d 100644 --- a/package.json +++ b/package.json @@ -40,10 +40,10 @@ "@babel/preset-env": "^7.8.4", "@gitlab/at.js": "1.5.5", "@gitlab/svgs": "1.121.0", - "@gitlab/ui": "12.2.0", + "@gitlab/ui": "12.3.0", "@gitlab/visual-review-tools": "1.6.1", "@sentry/browser": "^5.10.2", - "@sourcegraph/code-host-integration": "0.0.36", + "@sourcegraph/code-host-integration": "0.0.37", "apollo-cache-inmemory": "^1.6.3", "apollo-client": "^2.6.4", "apollo-link": "^1.2.11", diff --git a/spec/controllers/projects/mirrors_controller_spec.rb b/spec/controllers/projects/mirrors_controller_spec.rb index faeade0d737..8cd940978c0 100644 --- a/spec/controllers/projects/mirrors_controller_spec.rb +++ b/spec/controllers/projects/mirrors_controller_spec.rb @@ -189,7 +189,7 @@ describe Projects::MirrorsController do context 'no data in cache' do it 'requests the cache to be filled and returns a 204 response' do - expect(ReactiveCachingWorker).to receive(:perform_async).with(cache.class, cache.id).at_least(:once) + expect(ExternalServiceReactiveCachingWorker).to receive(:perform_async).with(cache.class, cache.id).at_least(:once) do_get(project) diff --git a/spec/migrations/remove_additional_application_settings_rows_spec.rb b/spec/migrations/remove_additional_application_settings_rows_spec.rb new file mode 100644 index 00000000000..379fa385b8e --- /dev/null +++ b/spec/migrations/remove_additional_application_settings_rows_spec.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +require 'spec_helper' + +require Rails.root.join('db', 'post_migrate', '20200420162730_remove_additional_application_settings_rows.rb') + +describe RemoveAdditionalApplicationSettingsRows do + let(:application_settings) { table(:application_settings) } + + it 'removes additional rows from application settings' do + 3.times { application_settings.create! } + latest_settings = application_settings.create! + + disable_migrations_output { migrate! } + + expect(application_settings.count).to eq(1) + expect(application_settings.first).to eq(latest_settings) + end + + it 'leaves only row in application_settings' do + latest_settings = application_settings.create! + + disable_migrations_output { migrate! } + + expect(application_settings.first).to eq(latest_settings) + end +end diff --git a/spec/services/grafana/proxy_service_spec.rb b/spec/services/grafana/proxy_service_spec.rb index 694d531c9fc..8cb7210524a 100644 --- a/spec/services/grafana/proxy_service_spec.rb +++ b/spec/services/grafana/proxy_service_spec.rb @@ -66,7 +66,7 @@ describe Grafana::ProxyService do context 'with caching', :use_clean_rails_memory_store_caching do context 'when value not present in cache' do it 'returns nil' do - expect(ReactiveCachingWorker) + expect(ExternalServiceReactiveCachingWorker) .to receive(:perform_async) .with(service.class, service.id, *cache_params) diff --git a/spec/services/metrics/dashboard/grafana_metric_embed_service_spec.rb b/spec/services/metrics/dashboard/grafana_metric_embed_service_spec.rb index 034d6aba5d6..3eebbe98c9d 100644 --- a/spec/services/metrics/dashboard/grafana_metric_embed_service_spec.rb +++ b/spec/services/metrics/dashboard/grafana_metric_embed_service_spec.rb @@ -154,7 +154,7 @@ describe Metrics::Dashboard::GrafanaMetricEmbedService do context 'when value not present in cache' do it 'returns nil' do - expect(ReactiveCachingWorker) + expect(ExternalServiceReactiveCachingWorker) .to receive(:perform_async) .with(service.class, service.id, *cache_params) diff --git a/spec/services/prometheus/proxy_service_spec.rb b/spec/services/prometheus/proxy_service_spec.rb index 5a036194d01..656ccea10de 100644 --- a/spec/services/prometheus/proxy_service_spec.rb +++ b/spec/services/prometheus/proxy_service_spec.rb @@ -117,7 +117,7 @@ describe Prometheus::ProxyService do context 'when value not present in cache' do it 'returns nil' do - expect(ReactiveCachingWorker) + expect(ExternalServiceReactiveCachingWorker) .to receive(:perform_async) .with(subject.class, subject.id, *opts) diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index fc543186b08..b3d7f7bcece 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -246,12 +246,19 @@ module GraphqlHelpers # Raises an error if no data is found def graphql_data + # Note that `json_response` is defined as `let(:json_response)` and + # therefore, in a spec with multiple queries, will only contain data + # from the _first_ query, not subsequent ones json_response['data'] || (raise NoData, graphql_errors) end def graphql_data_at(*path) + graphql_dig_at(graphql_data, *path) + end + + def graphql_dig_at(data, *path) keys = path.map { |segment| GraphqlHelpers.fieldnamerize(segment) } - graphql_data.dig(*keys) + data.dig(*keys) end def graphql_errors diff --git a/spec/support/shared_examples/graphql/sorted_paginated_query_shared_examples.rb b/spec/support/shared_examples/graphql/sorted_paginated_query_shared_examples.rb new file mode 100644 index 00000000000..e530237b4e3 --- /dev/null +++ b/spec/support/shared_examples/graphql/sorted_paginated_query_shared_examples.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +# Use this for testing how a GraphQL query handles sorting and pagination. +# This is particularly important when using keyset pagination connection, +# which is the default for ActiveRecord relations, as certain sort keys +# might not be supportable. +# +# sort_param: the value to specify the sort +# data_path: the keys necessary to dig into the return GraphQL data to get the +# returned results +# first_param: number of items expected (like a page size) +# expected_results: array of comparison data of all items sorted correctly +# pagination_query: method that specifies the GraphQL query +# pagination_results_data: method that extracts the sorted data used to compare against +# the expected results +# +# Example: +# describe 'sorting and pagination' do +# let(:sort_project) { create(:project, :public) } +# let(:data_path) { [:project, :issues] } +# +# def pagination_query(params, page_info) +# graphql_query_for( +# 'project', +# { 'fullPath' => sort_project.full_path }, +# "issues(#{params}) { #{page_info} edges { node { iid weight } } }" +# ) +# end +# +# def pagination_results_data(data) +# data.map { |issue| issue.dig('node', 'iid').to_i } +# end +# +# context 'when sorting by weight' do +# ... +# context 'when ascending' do +# it_behaves_like 'sorted paginated query' do +# let(:sort_param) { 'WEIGHT_ASC' } +# let(:first_param) { 2 } +# let(:expected_results) { [weight_issue3.iid, weight_issue5.iid, weight_issue1.iid, weight_issue4.iid, weight_issue2.iid] } +# end +# end +# +RSpec.shared_examples 'sorted paginated query' do + it_behaves_like 'requires variables' do + let(:required_variables) { [:sort_param, :first_param, :expected_results, :data_path, :current_user] } + end + + describe do + let(:params) { "sort: #{sort_param}" } + let(:start_cursor) { graphql_data_at(*data_path, :pageInfo, :startCursor) } + let(:end_cursor) { graphql_data_at(*data_path, :pageInfo, :endCursor) } + let(:sorted_edges) { graphql_data_at(*data_path, :edges) } + let(:page_info) { "pageInfo { startCursor endCursor }" } + + def pagination_query(params, page_info) + raise('pagination_query(params, page_info) must be defined in the test, see example in comment') unless defined?(super) + + super + end + + def pagination_results_data(data) + raise('pagination_results_data(data) must be defined in the test, see example in comment') unless defined?(super) + + super(data) + end + + before do + post_graphql(pagination_query(params, page_info), current_user: current_user) + end + + context 'when sorting' do + it 'sorts correctly' do + expect(pagination_results_data(sorted_edges)).to eq expected_results + end + + context 'when paginating' do + let(:params) { "sort: #{sort_param}, first: #{first_param}" } + + it 'paginates correctly' do + expect(pagination_results_data(sorted_edges)).to eq expected_results.first(first_param) + + cursored_query = pagination_query("sort: #{sort_param}, after: \"#{end_cursor}\"", page_info) + post_graphql(cursored_query, current_user: current_user) + response_data = graphql_dig_at(JSON.parse(response.body), :data, *data_path, :edges) + + expect(pagination_results_data(response_data)).to eq expected_results.drop(first_param) + end + end + end + end +end diff --git a/spec/support/shared_examples/requires_variables_shared_example.rb b/spec/support/shared_examples/requires_variables_shared_example.rb new file mode 100644 index 00000000000..2921fccf87a --- /dev/null +++ b/spec/support/shared_examples/requires_variables_shared_example.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +RSpec.shared_examples 'requires variables' do + it 'shared example requires variables to be set', :aggregate_failures do + variables = Array.wrap(required_variables) + + variables.each do |variable_name| + expect { send(variable_name) }.not_to( + raise_error, "The following variable must be set to use this shared example: #{variable_name}" + ) + end + end +end diff --git a/yarn.lock b/yarn.lock index 1b92af7b407..a0298450219 100644 --- a/yarn.lock +++ b/yarn.lock @@ -786,10 +786,10 @@ resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.121.0.tgz#77083a68f72e9aa0e294da7715f378eef13b839e" integrity sha512-scz/6Y/eED7RMFLAlhT6PwXwe0Wj8ivnRsyulk9NXKoqUmAqZliNmBmzYsHy5bFf9NB6xVV/rOk1/92nbi/Yaw== -"@gitlab/ui@12.2.0": - version "12.2.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-12.2.0.tgz#2dc6103ef4eeb016cdff8a885bee173ed3eab5f3" - integrity sha512-Qkyzrcu28gfJy635kqgfXzIKndOMT6touZ32FlXYowyrJOBQOxQt1q4/Z+S6nDWkC0tbBuOsbGiPx6F37MjWjQ== +"@gitlab/ui@12.3.0": + version "12.3.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-12.3.0.tgz#9234205887675a6d13a51945ee62efc3c8b5e890" + integrity sha512-XrHC2pK7qlwy6K3OR/+iCP8TDewn3jaDIHCfHjt/KOwvD5LsEmam9RHjTiZ4epPZXLv4+JxCzbc4R+euEbIQ7g== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.3.0" @@ -1036,10 +1036,10 @@ "@sentry/types" "5.10.0" tslib "^1.9.3" -"@sourcegraph/code-host-integration@0.0.36": - version "0.0.36" - resolved "https://registry.yarnpkg.com/@sourcegraph/code-host-integration/-/code-host-integration-0.0.36.tgz#2f4d287840ac2944c78ef92f10f0db0ef8a077fa" - integrity sha512-Hpj1xiVhPxMsjLNre9MrYYAM1SPOWPE9yG9SPtz4dqYzc6/ycaPGyr+ljcaWEclS9hZCvkk4+qVC5WONpYVjyA== +"@sourcegraph/code-host-integration@0.0.37": + version "0.0.37" + resolved "https://registry.yarnpkg.com/@sourcegraph/code-host-integration/-/code-host-integration-0.0.37.tgz#87f9a602e2a60520b6038311a67face2ece86827" + integrity sha512-GQvNuPORLjsMhto57Ue1umeSV3cir+hMEaGxwCKmmq+cc9ZSZpuXa8RVBXuT5azN99K9/8zFps4woyPJ8wrjYA== "@types/anymatch@*": version "1.3.0" |