diff --git a/app/assets/javascripts/packages_and_registries/dependency_proxy/app.vue b/app/assets/javascripts/packages_and_registries/dependency_proxy/app.vue index 9ecd0dd70fe..6c64fd4ccb1 100644 --- a/app/assets/javascripts/packages_and_registries/dependency_proxy/app.vue +++ b/app/assets/javascripts/packages_and_registries/dependency_proxy/app.vue @@ -122,6 +122,9 @@ export default { showDeleteDropdown() { return this.group.dependencyProxyManifests?.nodes.length > 0; }, + showDependencyProxyImagePrefix() { + return this.group.dependencyProxyImagePrefix?.length > 0; + }, }, methods: { fetchNextPage() { @@ -186,6 +189,31 @@ export default { + + + + + + + +
- - - - - - - a { + font-weight: 600; + line-height: 16px; + color: $gl-text-color; + } + } + > a { font-size: 12px; color: currentColor; @@ -383,17 +391,6 @@ margin-left: auto; } -.breadcrumbs-sub-title { - margin: 0; - font-size: 12px; - font-weight: 600; - line-height: 16px; - - a { - color: $gl-text-color; - } -} - .btn-sign-in { background-color: $indigo-100; color: $indigo-900; diff --git a/app/graphql/types/current_user_todos.rb b/app/graphql/types/current_user_todos.rb index 2551db875b0..4c4cb516979 100644 --- a/app/graphql/types/current_user_todos.rb +++ b/app/graphql/types/current_user_todos.rb @@ -17,9 +17,24 @@ module Types def current_user_todos(state: nil) state ||= %i[done pending] # TodosFinder treats a `nil` state param as `pending` - klass = unpresented.class + key = [state, unpresented.class.name] - TodosFinder.new(current_user, state: state, type: klass.name, target_id: object.id).execute + BatchLoader::GraphQL.for(unpresented).batch(default_value: [], key: key) do |targets, loader, args| + state, klass_name = args[:key] + + targets_by_id = targets.index_by(&:id) + ids = targets_by_id.keys + + results = TodosFinder.new(current_user, state: state, type: klass_name, target_id: ids).execute + + by_target_id = results.group_by(&:target_id) + + by_target_id.each do |target_id, todos| + target = targets_by_id[target_id] + todos.each { _1.target = target } # prevent extra loads + loader.call(target, todos) + end + end end end end diff --git a/app/views/layouts/nav/_breadcrumbs.html.haml b/app/views/layouts/nav/_breadcrumbs.html.haml index 3c52c430868..fde4e74fb7a 100644 --- a/app/views/layouts/nav/_breadcrumbs.html.haml +++ b/app/views/layouts/nav/_breadcrumbs.html.haml @@ -18,9 +18,8 @@ = breadcrumb_list_item link_to(extra[:text], extra[:link]) = render "layouts/nav/breadcrumbs/collapsed_inline_list", location: :after - unless @skip_current_level_breadcrumb - %li - %h2.breadcrumbs-sub-title{ data: { qa_selector: 'breadcrumb_sub_title_content' } } - = link_to @breadcrumb_title, breadcrumb_title_link + %li{ data: { testid: 'breadcrumb-current-link', qa_selector: 'breadcrumb_current_link' } } + = link_to @breadcrumb_title, breadcrumb_title_link -# haml-lint:disable InlineJavaScript %script{ type: 'application/ld+json' } :plain diff --git a/doc/administration/postgresql/replication_and_failover.md b/doc/administration/postgresql/replication_and_failover.md index 8c7151606a5..84122149cb8 100644 --- a/doc/administration/postgresql/replication_and_failover.md +++ b/doc/administration/postgresql/replication_and_failover.md @@ -1123,25 +1123,36 @@ postgresql['trust_auth_cidr_addresses'] = %w(123.123.123.123/32 ) ### Reinitialize a replica -If replication is not occurring, it may be necessary to reinitialize a replica. +If a replica cannot start or rejoin the cluster, or when it lags behind and can not catch up, it might be necessary to reinitialize the replica: -1. On any server in the cluster, determine the Cluster and Member names, - and check the replication lag by running `gitlab-ctl patroni members`. Here is an example: +1. [Check the replication status](#check-replication-status) to confirm which server + needs to be reinitialized. For example: ```plaintext - + Cluster: postgresql-ha (6970678148837286213) ------+---------+---------+----+-----------+ - | Member | Host | Role | State | TL | Lag in MB | - +-------------------------------------+--------------+---------+---------+----+-----------+ - | gitlab-database-1.example.com | 172.18.0.111 | Replica | running | 5 | 0 | - | gitlab-database-2.example.com | 172.18.0.112 | Replica | running | 5 | 100 | - | gitlab-database-3.example.com | 172.18.0.113 | Leader | running | 5 | | - +-------------------------------------+--------------+---------+---------+----+-----------+ + + Cluster: postgresql-ha (6970678148837286213) ------+---------+--------------+----+-----------+ + | Member | Host | Role | State | TL | Lag in MB | + +-------------------------------------+--------------+---------+--------------+----+-----------+ + | gitlab-database-1.example.com | 172.18.0.111 | Replica | running | 55 | 0 | + | gitlab-database-2.example.com | 172.18.0.112 | Replica | start failed | | unknown | + | gitlab-database-3.example.com | 172.18.0.113 | Leader | running | 55 | | + +-------------------------------------+--------------+---------+--------------+----+-----------+ ``` -1. Reinitialize the affected replica server: +1. Sign in to the broken server and reinitialize the database and replication. Patroni will shut + down PostgreSQL on that server, remove the data directory, and reinitialize it from scratch: - ```plaintext - gitlab-ctl patroni reinitialize-replica postgresql-ha gitlab-database-2.example.com + ```shell + sudo gitlab-ctl patroni reinitialize-replica --member gitlab-database-2.example.com + ``` + + This can be run on any Patroni node, but be aware that `sudo gitlab-ctl patroni + reinitialize-replica` without `--member` will reinitialize the server it is run on. + It is recommended to run it locally on the broken server to reduce the risk of + unintended data loss. +1. Monitor the logs: + + ```shell + sudo gitlab-ctl tail patroni ``` ### Reset the Patroni state in Consul diff --git a/doc/administration/sidekiq.md b/doc/administration/sidekiq.md index b6c1c6a704e..162e83d1ab3 100644 --- a/doc/administration/sidekiq.md +++ b/doc/administration/sidekiq.md @@ -192,12 +192,8 @@ To configure the metrics server: ## Configure health checks -If you use health check probes to observe Sidekiq, -you can set a separate port for health checks. -Configuring health checks is only necessary if there is something that actually probes them. -For more information about health checks, see the [Sidekiq health check page](sidekiq_health_check.md). - -To enable health checks for Sidekiq: +If you use health check probes to observe Sidekiq, enable the Sidekiq health check server. +To make health checks available from `localhost:8092`: 1. Edit `/etc/gitlab/gitlab.rb`: @@ -207,16 +203,14 @@ To enable health checks for Sidekiq: sidekiq['health_checks_listen_port'] = "8092" ``` - NOTE: - If health check settings are not set, they default to the metrics exporter settings. - This default is deprecated and is set to be removed in [GitLab 15.0](https://gitlab.com/gitlab-org/gitlab/-/issues/347509). - 1. Reconfigure GitLab: ```shell sudo gitlab-ctl reconfigure ``` +For more information about health checks, see the [Sidekiq health check page](sidekiq_health_check.md). + ## Configure LDAP and user or group synchronization If you use LDAP for user and group management, you must add the LDAP configuration to your Sidekiq node as well as the LDAP diff --git a/doc/administration/sidekiq_health_check.md b/doc/administration/sidekiq_health_check.md index ae02711f46f..3ff17d3682b 100644 --- a/doc/administration/sidekiq_health_check.md +++ b/doc/administration/sidekiq_health_check.md @@ -21,8 +21,7 @@ The readiness probe checks whether the Sidekiq workers are ready to process jobs GET /readiness ``` -If you set Sidekiq's address as `localhost` and port as `8092`, -here's an example request: +If the server is bound to `localhost:8092`, the process cluster can be probed for readiness as follows: ```shell curl "http://localhost:8092/readiness" @@ -44,8 +43,7 @@ Checks whether the Sidekiq cluster is running. GET /liveness ``` -If you set Sidekiq's address as `localhost` and port as `8092`, -here's an example request: +If the server is bound to `localhost:8092`, the process cluster can be probed for liveness as follows: ```shell curl "http://localhost:8092/liveness" diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 951f9ac3c31..3263b4a0920 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -601,6 +601,7 @@ Input type: `AdminSidekiqQueuesDeleteJobsInput` | Name | Type | Description | | ---- | ---- | ----------- | +| `artifactSize` | [`String`](#string) | Delete jobs matching artifact_size in the context metadata. | | `callerId` | [`String`](#string) | Delete jobs matching caller_id in the context metadata. | | `clientId` | [`String`](#string) | Delete jobs matching client_id in the context metadata. | | `clientMutationId` | [`String`](#string) | A unique identifier for the client performing the mutation. | diff --git a/doc/development/sidekiq/idempotent_jobs.md b/doc/development/sidekiq/idempotent_jobs.md index 38db22f8467..a5ae8737ad1 100644 --- a/doc/development/sidekiq/idempotent_jobs.md +++ b/doc/development/sidekiq/idempotent_jobs.md @@ -135,7 +135,7 @@ happened. See [this issue](https://gitlab.com/gitlab-org/gitlab/-/issues/342123) GitLab doesn't skip jobs scheduled in the future, as we assume that the state has changed by the time the job is scheduled to -execute. Deduplication of jobs scheduled in the feature is possible +execute. Deduplication of jobs scheduled in the future is possible for both `until_executed` and `until_executing` strategies. If you do want to deduplicate jobs scheduled in the future, diff --git a/lib/api/ci/helpers/runner.rb b/lib/api/ci/helpers/runner.rb index 173cfc9a59a..44f9ebcf6c6 100644 --- a/lib/api/ci/helpers/runner.rb +++ b/lib/api/ci/helpers/runner.rb @@ -111,6 +111,10 @@ module API # noop: overridden in EE end + def log_artifact_size(artifact) + Gitlab::ApplicationContext.push(artifact: artifact) + end + private def get_runner_config_from_request diff --git a/lib/api/ci/runner.rb b/lib/api/ci/runner.rb index c6c238c73a0..60e476d8780 100644 --- a/lib/api/ci/runner.rb +++ b/lib/api/ci/runner.rb @@ -305,6 +305,7 @@ module API result = ::Ci::JobArtifacts::CreateService.new(job).execute(artifacts, params, metadata_file: metadata) if result[:status] == :success + log_artifact_size(artifacts) status :created body "201" else diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 33feadc1be4..7baa164ef8b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -565,6 +565,8 @@ module API def present_carrierwave_file!(file, supports_direct_download: true) return not_found! unless file&.exists? + log_artifact_size(file) if file.is_a?(JobArtifactUploader) + if file.file_storage? present_disk_file!(file.path, file.filename) elsif supports_direct_download && file.class.direct_download_enabled? @@ -718,6 +720,7 @@ module API # Deprecated. Use `send_artifacts_entry` instead. def legacy_send_artifacts_entry(file, entry) header(*Gitlab::Workhorse.send_artifacts_entry(file, entry)) + log_artifact_size(file) body '' end @@ -725,10 +728,15 @@ module API def send_artifacts_entry(file, entry) header(*Gitlab::Workhorse.send_artifacts_entry(file, entry)) header(*Gitlab::Workhorse.detect_content_type) + log_artifact_size(file) body '' end + def log_artifact_size(file) + Gitlab::ApplicationContext.push(artifact: file.model) + end + # The Grape Error Middleware only has access to `env` but not `params` nor # `request`. We workaround this by defining methods that returns the right # values. diff --git a/lib/gitlab/application_context.rb b/lib/gitlab/application_context.rb index b10330914ca..6ef5a1e2cd8 100644 --- a/lib/gitlab/application_context.rb +++ b/lib/gitlab/application_context.rb @@ -19,7 +19,8 @@ module Gitlab :job_id, :pipeline_id, :related_class, - :feature_category + :feature_category, + :artifact_size ].freeze private_constant :KNOWN_KEYS @@ -32,7 +33,8 @@ module Gitlab Attribute.new(:remote_ip, String), Attribute.new(:job, ::Ci::Build), Attribute.new(:related_class, String), - Attribute.new(:feature_category, String) + Attribute.new(:feature_category, String), + Attribute.new(:artifact, ::Ci::JobArtifact) ].freeze def self.known_keys @@ -74,6 +76,8 @@ module Gitlab assign_attributes(args) end + # rubocop: disable Metrics/CyclomaticComplexity + # rubocop: disable Metrics/PerceivedComplexity def to_lazy_hash {}.tap do |hash| hash[:user] = -> { username } if include_user? @@ -86,8 +90,11 @@ module Gitlab hash[:feature_category] = feature_category if set_values.include?(:feature_category) hash[:pipeline_id] = -> { job&.pipeline_id } if set_values.include?(:job) hash[:job_id] = -> { job&.id } if set_values.include?(:job) + hash[:artifact_size] = -> { artifact&.size } if set_values.include?(:artifact) end end + # rubocop: enable Metrics/CyclomaticComplexity + # rubocop: enable Metrics/PerceivedComplexity def use Labkit::Context.with_context(to_lazy_hash) { yield } diff --git a/lib/gitlab/graphql/queries.rb b/lib/gitlab/graphql/queries.rb index 5d3a9245427..cf06a2729d9 100644 --- a/lib/gitlab/graphql/queries.rb +++ b/lib/gitlab/graphql/queries.rb @@ -240,6 +240,9 @@ module Gitlab end end + # TODO: some queries live under app/graphql/queries - we should look there if/when we add fragments there + # See: https://gitlab.com/gitlab-org/gitlab/-/issues/361079 + # for fragments too. class Fragments def initialize(root, dir = 'app/assets/javascripts') @root = root @@ -278,7 +281,7 @@ module Gitlab def self.all ['.', 'ee'].flat_map do |prefix| - find(Rails.root / prefix / 'app/assets/javascripts') + find(Rails.root / prefix / 'app/assets/javascripts') + find(Rails.root / prefix / 'app/graphql/queries') end end diff --git a/package.json b/package.json index c71fd440708..f212e2634be 100644 --- a/package.json +++ b/package.json @@ -109,7 +109,7 @@ "codesandbox-api": "0.0.23", "compression-webpack-plugin": "^5.0.2", "copy-webpack-plugin": "^6.4.1", - "core-js": "^3.22.2", + "core-js": "^3.22.3", "cron-validator": "^1.1.1", "cronstrue": "^1.122.0", "cropper": "^2.3.0", diff --git a/qa/qa/page/component/snippet.rb b/qa/qa/page/component/snippet.rb index a8ae706858e..47d32ae8225 100644 --- a/qa/qa/page/component/snippet.rb +++ b/qa/qa/page/component/snippet.rb @@ -82,7 +82,7 @@ module QA base.view 'app/views/layouts/nav/_breadcrumbs.html.haml' do element :breadcrumb_links_content - element :breadcrumb_sub_title_content + element :breadcrumb_current_link end end @@ -257,7 +257,7 @@ module QA def snippet_id within_element(:breadcrumb_links_content) do - find_element(:breadcrumb_sub_title_content).text.delete_prefix('$') + find_element(:breadcrumb_current_link).text.delete_prefix('$') end end end diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 144cd6e70fc..67c5968545c 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -468,7 +468,7 @@ RSpec.describe "Admin Runners" do describe 'runner show page breadcrumbs' do it 'contains the current runner id and token' do page.within '[data-testid="breadcrumb-links"]' do - expect(page.find('h2')).to have_link("##{runner.id} (#{runner.short_sha})") + expect(page.find('[data-testid="breadcrumb-current-link"]')).to have_link("##{runner.id} (#{runner.short_sha})") end end end @@ -519,7 +519,7 @@ RSpec.describe "Admin Runners" do it 'contains the current runner id and token' do page.within '[data-testid="breadcrumb-links"]' do expect(page).to have_link("##{runner.id} (#{runner.short_sha})") - expect(page.find('h2')).to have_content("Edit") + expect(page.find('[data-testid="breadcrumb-current-link"]')).to have_content("Edit") end end end diff --git a/spec/features/admin/users/users_spec.rb b/spec/features/admin/users/users_spec.rb index 4d9a7f31911..a05e1531949 100644 --- a/spec/features/admin/users/users_spec.rb +++ b/spec/features/admin/users/users_spec.rb @@ -548,7 +548,7 @@ RSpec.describe 'Admin::Users' do end def check_breadcrumb(content) - expect(find('.breadcrumbs-sub-title')).to have_content(content) + expect(find('[data-testid="breadcrumb-current-link"]')).to have_content(content) end end diff --git a/spec/features/issues/user_sees_breadcrumb_links_spec.rb b/spec/features/issues/user_sees_breadcrumb_links_spec.rb index 44fa062480a..1577d7d5ce8 100644 --- a/spec/features/issues/user_sees_breadcrumb_links_spec.rb +++ b/spec/features/issues/user_sees_breadcrumb_links_spec.rb @@ -26,7 +26,7 @@ RSpec.describe 'New issue breadcrumb' do visit project_issue_path(project, issue) - expect(find('.breadcrumbs-sub-title a')[:href]).to end_with(issue_path(issue)) + expect(find('[data-testid="breadcrumb-current-link"] a')[:href]).to end_with(issue_path(issue)) end it 'excludes award_emoji from comment count', :js do diff --git a/spec/features/profiles/keys_spec.rb b/spec/features/profiles/keys_spec.rb index fde85a731a1..65944f5a537 100644 --- a/spec/features/profiles/keys_spec.rb +++ b/spec/features/profiles/keys_spec.rb @@ -29,7 +29,7 @@ RSpec.describe 'Profile > SSH Keys' do expect(page).to have_content("Title: #{attrs[:title]}") expect(page).to have_content(attrs[:key]) - expect(find('.breadcrumbs-sub-title')).to have_link(attrs[:title]) + expect(find('[data-testid="breadcrumb-current-link"]')).to have_link(attrs[:title]) end it 'shows a confirmable warning if the key begins with an algorithm name that is unsupported' do diff --git a/spec/features/profiles/oauth_applications_spec.rb b/spec/features/profiles/oauth_applications_spec.rb index 6827dff5434..9d79041dc9d 100644 --- a/spec/features/profiles/oauth_applications_spec.rb +++ b/spec/features/profiles/oauth_applications_spec.rb @@ -16,7 +16,7 @@ RSpec.describe 'Profile > Applications' do visit oauth_application_path(application) expect(page).to have_content("Application: #{application.name}") - expect(find('.breadcrumbs-sub-title')).to have_link(application.name) + expect(find('[data-testid="breadcrumb-current-link"]')).to have_link(application.name) end it 'deletes an application' do diff --git a/spec/features/projects/issues/design_management/user_uploads_designs_spec.rb b/spec/features/projects/issues/design_management/user_uploads_designs_spec.rb index 48ae70d3ec9..27d0be23aec 100644 --- a/spec/features/projects/issues/design_management/user_uploads_designs_spec.rb +++ b/spec/features/projects/issues/design_management/user_uploads_designs_spec.rb @@ -10,9 +10,6 @@ RSpec.describe 'User uploads new design', :js do let(:issue) { create(:issue, project: project) } before do - # Cause of raising query limiting threshold https://gitlab.com/gitlab-org/gitlab/-/issues/358845 - stub_const("Gitlab::QueryLimiting::Transaction::THRESHOLD", 106) - sign_in(user) enable_design_management(feature_enabled) visit project_issue_path(project, issue) diff --git a/spec/frontend/packages_and_registries/dependency_proxy/app_spec.js b/spec/frontend/packages_and_registries/dependency_proxy/app_spec.js index dbe9793fb8c..59acd7fe727 100644 --- a/spec/frontend/packages_and_registries/dependency_proxy/app_spec.js +++ b/spec/frontend/packages_and_registries/dependency_proxy/app_spec.js @@ -9,7 +9,7 @@ import { GlSprintf, GlEmptyState, } from '@gitlab/ui'; -import Vue from 'vue'; +import Vue, { nextTick } from 'vue'; import VueApollo from 'vue-apollo'; import MockAdapter from 'axios-mock-adapter'; import createMockApollo from 'helpers/mock_apollo_helper'; @@ -140,21 +140,23 @@ describe('DependencyProxyApp', () => { describe('when the dependency proxy is available', () => { describe('when is loading', () => { - it('renders the skeleton loader', () => { + beforeEach(() => { createComponent(); + }); + it('renders the skeleton loader', () => { expect(findSkeletonLoader().exists()).toBe(true); }); - it('does not show the main section', () => { - createComponent(); + it('does not render a form group with label', () => { + expect(findFormGroup().exists()).toBe(false); + }); + it('does not show the main section', () => { expect(findMainArea().exists()).toBe(false); }); it('does not render the info alert', () => { - createComponent(); - expect(findProxyNotAvailableAlert().exists()).toBe(false); }); }); @@ -193,7 +195,7 @@ describe('DependencyProxyApp', () => { }); }); - it('from group has a description with proxy count', () => { + it('form group has a description with proxy count', () => { expect(findProxyCountText().text()).toBe('Contains 2 blobs of images (1024 Bytes)'); }); @@ -257,6 +259,28 @@ describe('DependencyProxyApp', () => { }); }); + describe('triggering page event on list', () => { + beforeEach(async () => { + findManifestList().vm.$emit('next-page'); + + await nextTick(); + }); + + it('re-renders the skeleton loader', () => { + expect(findSkeletonLoader().exists()).toBe(true); + }); + + it('renders form group with label', () => { + expect(findFormGroup().attributes('label')).toEqual( + expect.stringMatching(DependencyProxyApp.i18n.proxyImagePrefix), + ); + }); + + it('does not show the main section', () => { + expect(findMainArea().exists()).toBe(false); + }); + }); + it('shows the clear cache dropdown list', () => { expect(findClearCacheDropdownList().exists()).toBe(true); diff --git a/spec/graphql/types/current_user_todos_type_spec.rb b/spec/graphql/types/current_user_todos_type_spec.rb index a0015e96788..91628077c19 100644 --- a/spec/graphql/types/current_user_todos_type_spec.rb +++ b/spec/graphql/types/current_user_todos_type_spec.rb @@ -3,7 +3,214 @@ require 'spec_helper' RSpec.describe GitlabSchema.types['CurrentUserTodos'] do + include GraphqlHelpers + specify { expect(described_class.graphql_name).to eq('CurrentUserTodos') } specify { expect(described_class).to have_graphql_fields(:current_user_todos).only } + + # Request store is necessary to prevent duplicate max-member-access lookups + describe '.current_user_todos', :request_store, :aggregate_failures do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :repository) } + let_it_be(:issue_a) { create(:issue, project: project) } + let_it_be(:issue_b) { create(:issue, project: project) } + + let_it_be(:todo_a) { create(:todo, :pending, user: user, project: project, target: issue_a) } + let_it_be(:todo_b) { create(:todo, :done, user: user, project: project, target: issue_a) } + let_it_be(:todo_c) { create(:todo, :pending, user: user, project: project, target: issue_b) } + let_it_be(:todo_d) { create(:todo, :done, user: user, project: project, target: issue_b) } + + let_it_be(:merge_request) { create(:merge_request, source_project: project) } + let_it_be(:todo_e) { create(:todo, :pending, user: user, project: project, target: merge_request) } + + let(:object_type) do + fresh_object_type('HasTodos').tap { _1.implements(Types::CurrentUserTodos) } + end + + let(:id_enum) do + Class.new(Types::BaseEnum) do + graphql_name 'AorB' + + value 'A' + value 'B' + end + end + + let(:query_type) do + i_a = issue_a + i_b = issue_b + issue_id = id_enum + mr = merge_request + + q = fresh_object_type('Query') + + q.field :issue, null: false, type: object_type do + argument :id, type: issue_id, required: true + end + + q.field :mr, null: false, type: object_type + + q.define_method(:issue) do |id:| + case id + when 'A' + i_a + when 'B' + i_b + end + end + + q.define_method(:mr) { mr } + + q + end + + let(:todo_fragment) do + <<-GQL + fragment todos on HasTodos { + todos: currentUserTodos { + nodes { id } + } + } + GQL + end + + let(:base_query) do + <<-GQL + query { + issue(id: A) { ... todos } + } + #{todo_fragment} + GQL + end + + let(:query_without_state_arguments) do + <<-GQL + query { + a: issue(id: A) { + ... todos + } + b: issue(id: B) { + ... todos + } + c: mr { + ... todos + } + d: mr { + ... todos + } + e: issue(id: A) { + ... todos + } + } + + #{todo_fragment} + GQL + end + + let(:with_state_arguments) do + <<-GQL + query { + a: issue(id: A) { + todos: currentUserTodos(state: pending) { nodes { id } } + } + b: issue(id: B) { + todos: currentUserTodos(state: done) { nodes { id } } + } + c: mr { + ... todos + } + } + + #{todo_fragment} + GQL + end + + before_all do + project.add_developer(user) + end + + it 'batches todo lookups, linear in the number of target types/state arguments' do + # The baseline is 4 queries: + # + # When we batch queries, we see the following three groups of queries: + # # user authorization + # 1. SELECT "users".* FROM "users" + # INNER JOIN "project_authorizations" + # ON "users"."id" = "project_authorizations"."user_id" + # WHERE "project_authorizations"."project_id" = project_id + # AND "project_authorizations"."access_level" = 50 + # 2. SELECT MAX("project_authorizations"."access_level") AS maximum_access_level, + # "project_authorizations"."user_id" AS project_authorizations_user_id + # FROM "project_authorizations" + # WHERE "project_authorizations"."project_id" = project_id + # AND "project_authorizations"."user_id" = user_id + # GROUP BY "project_authorizations"."user_id" + # + # # find todos for issues + # 1. SELECT "todos".* FROM "todos" + # WHERE "todos"."user_id" = user_id + # AND ("todos"."state" IN ('done','pending')) + # AND "todos"."target_id" IN (issue_a, issue_b) + # AND "todos"."target_type" = 'Issue' ORDER BY "todos"."id" DESC + # + # # find todos for merge_requests + # 1. SELECT "todos".* FROM "todos" WHERE "todos"."user_id" = user_id + # AND ("todos"."state" IN ('done','pending')) + # AND "todos"."target_id" = merge_request + # AND "todos"."target_type" = 'MergeRequest' ORDER BY "todos"."id" DESC + baseline = ActiveRecord::QueryRecorder.new do + execute_query(query_type, graphql: base_query) + end + + expect do + execute_query(query_type, graphql: query_without_state_arguments) + end.not_to exceed_query_limit(baseline) # at present this is 3 + + expect do + execute_query(query_type, graphql: with_state_arguments) + end.not_to exceed_query_limit(baseline.count + 1) + end + + it 'returns correct data' do + result = execute_query(query_type, + graphql: query_without_state_arguments, + raise_on_error: true).to_h + + expect(result.dig('data', 'a', 'todos', 'nodes')).to contain_exactly( + { "id" => global_id_of(todo_a) }, + { "id" => global_id_of(todo_b) } + ) + expect(result.dig('data', 'b', 'todos', 'nodes')).to contain_exactly( + { "id" => global_id_of(todo_c) }, + { "id" => global_id_of(todo_d) } + ) + expect(result.dig('data', 'c', 'todos', 'nodes')).to contain_exactly( + { "id" => global_id_of(todo_e) } + ) + expect(result.dig('data', 'd', 'todos', 'nodes')).to contain_exactly( + { "id" => global_id_of(todo_e) } + ) + expect(result.dig('data', 'e', 'todos', 'nodes')).to contain_exactly( + { "id" => global_id_of(todo_a) }, + { "id" => global_id_of(todo_b) } + ) + end + + it 'returns correct data, when state arguments are supplied' do + result = execute_query(query_type, + raise_on_error: true, + graphql: with_state_arguments).to_h + + expect(result.dig('data', 'a', 'todos', 'nodes')).to contain_exactly( + include("id" => global_id_of(todo_a)) + ) + expect(result.dig('data', 'b', 'todos', 'nodes')).to contain_exactly( + include("id" => global_id_of(todo_d)) + ) + expect(result.dig('data', 'c', 'todos', 'nodes')).to contain_exactly( + include("id" => global_id_of(todo_e)) + ) + end + end end diff --git a/spec/lib/api/ci/helpers/runner_helpers_spec.rb b/spec/lib/api/ci/helpers/runner_helpers_spec.rb index c4d740f0adc..c6cdc1732f5 100644 --- a/spec/lib/api/ci/helpers/runner_helpers_spec.rb +++ b/spec/lib/api/ci/helpers/runner_helpers_spec.rb @@ -70,5 +70,17 @@ RSpec.describe API::Ci::Helpers::Runner do expect(details['ip_address']).to eq(ip_address) end end + + describe '#log_artifact_size' do + subject { runner_helper.log_artifact_size(artifact) } + + let(:runner_params) { {} } + let(:artifact) { create(:ci_job_artifact, size: 42) } + let(:expected_params) { { artifact_size: artifact.size } } + let(:subject_proc) { proc { subject } } + + it_behaves_like 'storing arguments in the application context' + it_behaves_like 'not executing any extra queries for the application context' + end end end diff --git a/spec/lib/gitlab/application_context_spec.rb b/spec/lib/gitlab/application_context_spec.rb index f9e18a65af4..7ea0f324dfa 100644 --- a/spec/lib/gitlab/application_context_spec.rb +++ b/spec/lib/gitlab/application_context_spec.rb @@ -91,6 +91,7 @@ RSpec.describe Gitlab::ApplicationContext do let_it_be(:project) { create(:project) } let_it_be(:namespace) { create(:group) } let_it_be(:subgroup) { create(:group, parent: namespace) } + let_it_be(:artifact) { create(:ci_job_artifact, size: 42) } def result(context) context.to_lazy_hash.transform_values { |v| v.respond_to?(:call) ? v.call : v } diff --git a/spec/lib/gitlab/graphql/queries_spec.rb b/spec/lib/gitlab/graphql/queries_spec.rb index ad1aaac712e..2c2ec821385 100644 --- a/spec/lib/gitlab/graphql/queries_spec.rb +++ b/spec/lib/gitlab/graphql/queries_spec.rb @@ -85,11 +85,15 @@ RSpec.describe Gitlab::Graphql::Queries do describe '.all' do it 'is the combination of finding queries in CE and EE' do expect(described_class) - .to receive(:find).with(Rails.root / 'app/assets/javascripts').and_return([:ce]) + .to receive(:find).with(Rails.root / 'app/assets/javascripts').and_return([:ce_assets]) expect(described_class) - .to receive(:find).with(Rails.root / 'ee/app/assets/javascripts').and_return([:ee]) + .to receive(:find).with(Rails.root / 'ee/app/assets/javascripts').and_return([:ee_assets]) + expect(described_class) + .to receive(:find).with(Rails.root / 'app/graphql/queries').and_return([:ce_gql]) + expect(described_class) + .to receive(:find).with(Rails.root / 'ee/app/graphql/queries').and_return([:ee_gql]) - expect(described_class.all).to eq([:ce, :ee]) + expect(described_class.all).to contain_exactly(:ce_assets, :ee_assets, :ce_gql, :ee_gql) end end diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index 5abff85af9c..1dd1ca4e115 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -263,6 +263,9 @@ RSpec.describe API::Ci::JobArtifacts do 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } end + let(:expected_params) { { artifact_size: job.artifacts_file.size } } + let(:subject_proc) { proc { subject } } + it 'returns specific job artifacts' do subject @@ -270,6 +273,9 @@ RSpec.describe API::Ci::JobArtifacts do expect(response.headers.to_h).to include(download_headers) expect(response.body).to match_file(job.artifacts_file.file.file) end + + it_behaves_like 'storing arguments in the application context' + it_behaves_like 'not executing any extra queries for the application context' end context 'normal authentication' do diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index 3b6a71c03a4..652cc51fcc0 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -687,21 +687,28 @@ module GraphqlHelpers end end - # assumes query_string to be let-bound in the current context - def execute_query(query_type, schema: empty_schema, graphql: query_string) + # assumes query_string and user to be let-bound in the current context + def execute_query(query_type, schema: empty_schema, graphql: query_string, raise_on_error: false) schema.query(query_type) - schema.execute( + r = schema.execute( graphql, context: { current_user: user }, variables: {} ) + + if raise_on_error && r.to_h['errors'].present? + raise NoData, r.to_h['errors'] + end + + r end def empty_schema Class.new(GraphQL::Schema) do use GraphQL::Pagination::Connections use Gitlab::Graphql::Pagination::Connections + use BatchLoader::GraphQL lazy_resolve ::Gitlab::Graphql::Lazy, :force end diff --git a/yarn.lock b/yarn.lock index 29bee6f5711..d17d62253d1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3900,10 +3900,10 @@ core-js-pure@^3.0.0: resolved "https://registry.yarnpkg.com/core-js-pure/-/core-js-pure-3.6.5.tgz#c79e75f5e38dbc85a662d91eea52b8256d53b813" integrity sha512-lacdXOimsiD0QyNf9BC/mxivNJ/ybBGJXQFKzRekp1WTHoVUWsUHEn+2T8GJAzzIhyOuXA+gOxCVN3l+5PLPUA== -core-js@^3.22.2: - version "3.22.2" - resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.22.2.tgz#3ea0a245b0895fa39d1faa15fe75d91ade504a01" - integrity sha512-Z5I2vzDnEIqO2YhELVMFcL1An2CIsFe9Q7byZhs8c/QxummxZlAHw33TUHbIte987LkisOgL0LwQ1P9D6VISnA== +core-js@^3.22.3: + version "3.22.3" + resolved "https://registry.yarnpkg.com/core-js/-/core-js-3.22.3.tgz#498c41d997654cb00e81c7a54b44f0ab21ab01d5" + integrity sha512-1t+2a/d2lppW1gkLXx3pKPVGbBdxXAkqztvWb1EJ8oF8O2gIGiytzflNiFEehYwVK/t2ryUsGBoOFFvNx95mbg== core-js@~2.3.0: version "2.3.0"