diff options
Diffstat (limited to 'spec/requests')
79 files changed, 1884 insertions, 650 deletions
diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index 382aabd45a1..9e1203bc720 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -70,6 +70,7 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do expect(json_response['artifacts']).to be_an Array expect(json_response['artifacts']).to be_empty expect(json_response['web_url']).to be_present + expect(json_response['archived']).to eq(jobx.archived?) end end @@ -132,12 +133,12 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do end it 'avoids N+1 queries', :skip_before_request do - control_count = ActiveRecord::QueryRecorder.new { perform_request }.count + control = ActiveRecord::QueryRecorder.new { perform_request } running_job = create(:ci_build, :running, project: project, user: user, pipeline: pipeline, artifacts_expire_at: 1.day.since) running_job.save! - expect { perform_request }.not_to exceed_query_limit(control_count) + expect { perform_request }.not_to exceed_query_limit(control) end it_behaves_like 'returns common pipeline data' do @@ -431,7 +432,7 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do first_build.user = create(:user) first_build.save! - control_count = ActiveRecord::QueryRecorder.new { go }.count + control = ActiveRecord::QueryRecorder.new { go } second_pipeline = create(:ci_empty_pipeline, project: project, sha: project.commit.id, ref: project.default_branch) second_build = create(:ci_build, :trace_artifact, :artifacts, :test_reports, pipeline: second_pipeline) @@ -439,7 +440,7 @@ RSpec.describe API::Ci::Jobs, feature_category: :continuous_integration do second_build.user = create(:user) second_build.save! - expect { go }.not_to exceed_query_limit(control_count) + expect { go }.not_to exceed_query_limit(control) end context 'filter project with one scope element' do diff --git a/spec/requests/api/ci/pipeline_schedules_spec.rb b/spec/requests/api/ci/pipeline_schedules_spec.rb index f534b093b7c..588991096b5 100644 --- a/spec/requests/api/ci/pipeline_schedules_spec.rb +++ b/spec/requests/api/ci/pipeline_schedules_spec.rb @@ -42,15 +42,15 @@ RSpec.describe API::Ci::PipelineSchedules, feature_category: :continuous_integra # We need at least two users to trigger a preload for that relation. create_pipeline_schedules(1) - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do get api("/projects/#{project.id}/pipeline_schedules", developer) - end.count + end create_pipeline_schedules(5) expect do get api("/projects/#{project.id}/pipeline_schedules", developer) - end.not_to exceed_query_limit(control_count) + end.not_to exceed_query_limit(control) end %w[active inactive].each do |target| diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index eef125e1bc3..ef169dbe872 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -471,15 +471,15 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query - end.count + end create_list(:ci_build, 3, :trace_artifact, :artifacts, :test_reports, pipeline: pipeline) expect do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end context 'pipeline has retried jobs' do @@ -671,15 +671,15 @@ RSpec.describe API::Ci::Pipelines, feature_category: :continuous_integration do end it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - end.count + end 3.times { create_bridge(pipeline) } expect do get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end end diff --git a/spec/requests/api/ci/runner/jobs_request_post_spec.rb b/spec/requests/api/ci/runner/jobs_request_post_spec.rb index 3d6d86335eb..e118ef9a384 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -932,7 +932,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego name: 'ruby', executor_opts: { docker: { - platform: 'amd64' + platform: 'amd64', + user: 'dave' } } } @@ -948,7 +949,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state, feature_catego 'image' => { 'name' => 'ruby', 'executor_opts' => { 'docker' => { - 'platform' => 'amd64' + 'platform' => 'amd64', + 'user' => 'dave' } }, 'pull_policy' => nil, diff --git a/spec/requests/api/ci/runner/yamls/image-executor_opts-user.yml b/spec/requests/api/ci/runner/yamls/image-executor_opts-user.yml new file mode 100644 index 00000000000..9fb56b941c9 --- /dev/null +++ b/spec/requests/api/ci/runner/yamls/image-executor_opts-user.yml @@ -0,0 +1,25 @@ +gitlab_ci: + rspec: + image: + name: alpine:latest + docker: + user: dave + script: echo Hello World + +request_response: + image: + name: alpine:latest + entrypoint: null + executor_opts: + docker: + user: dave + ports: [] + pull_policy: null + steps: + - name: script + script: ["echo Hello World"] + timeout: 3600 + when: on_success + allow_failure: false + services: [] + diff --git a/spec/requests/api/ci/runner/yamls/service-executor_opts-user.yml b/spec/requests/api/ci/runner/yamls/service-executor_opts-user.yml new file mode 100644 index 00000000000..ea824110e63 --- /dev/null +++ b/spec/requests/api/ci/runner/yamls/service-executor_opts-user.yml @@ -0,0 +1,27 @@ +gitlab_ci: + rspec: + services: + - name: docker:dind + docker: + user: john + script: echo Hello World + +request_response: + image: null + steps: + - name: script + script: ["echo Hello World"] + timeout: 3600 + when: on_success + allow_failure: false + services: + - name: docker:dind + alias: null + command: null + entrypoint: null + executor_opts: + docker: + user: john + ports: [] + pull_policy: null + variables: [] diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index 187880e16a4..11d906249e4 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -836,166 +836,219 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v end describe 'GET /runners/:id/jobs' do - let_it_be(:job_1) { create(:ci_build) } - let_it_be(:job_2) { create(:ci_build, :running, runner: shared_runner, project: project) } - let_it_be(:job_3) { create(:ci_build, :failed, runner: shared_runner, project: project) } - let_it_be(:job_4) { create(:ci_build, :running, runner: project_runner, project: project) } - let_it_be(:job_5) { create(:ci_build, :failed, runner: project_runner, project: project) } - let(:path) { "/runners/#{project_runner.id}/jobs" } + subject(:request) { get api(path, user, **api_params) } + + let_it_be(:shared_runner_manager1) { create(:ci_runner_machine, runner: shared_runner, system_xid: 'id2') } + let_it_be(:jobs) do + project_runner_manager1 = create(:ci_runner_machine, runner: project_runner, system_xid: 'id1') + project_runner_manager2 = create(:ci_runner_machine, runner: two_projects_runner, system_xid: 'id1') + + [ + create(:ci_build), + create(:ci_build, :running, runner_manager: shared_runner_manager1, project: project), + create(:ci_build, :failed, runner_manager: shared_runner_manager1, project: project), + create(:ci_build, :running, runner_manager: project_runner_manager1, project: project), + create(:ci_build, :failed, runner_manager: project_runner_manager1, project: project), + create(:ci_build, :running, runner_manager: project_runner_manager2, project: project), + create(:ci_build, :running, runner_manager: project_runner_manager2, project: project2) + ] + end + + let(:api_params) { {} } + let(:runner_id) { project_runner.id } + let(:query_part) { query_params.merge(system_id_params).map { |param| param.join('=') }.join('&') } + let(:path) { "/runners/#{runner_id}/jobs?#{query_part}" } + let(:query_params) { {} } + let(:system_id_params) { {} } it_behaves_like 'GET request permissions for admin mode' context 'admin user' do + let(:user) { admin } + let(:api_params) { { admin_mode: true } } + context 'when runner exists' do context 'when runner is shared' do + let(:runner_id) { shared_runner.id } + let(:system_id) { 'id2' } + it 'return jobs' do - get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true) + request expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(2) + expect(json_response).to match([ + a_hash_including('id' => jobs[1].id), + a_hash_including('id' => jobs[2].id) + ]) + end + + it_behaves_like 'an endpoint with keyset pagination', invalid_order: nil do + let(:first_record) { jobs[2] } + let(:second_record) { jobs[1] } + let(:api_call) { api(path, user, **api_params) } end end context 'when runner is a project runner' do it 'return jobs' do - get api(path, admin, admin_mode: true) + request expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(2) + expect(json_response).to match([ + a_hash_including('id' => jobs[3].id), + a_hash_including('id' => jobs[4].id) + ]) end context 'when user does not have authorization to see all jobs' do - it 'shows only jobs it has permission to see' do - create(:ci_build, :running, runner: two_projects_runner, project: project) - create(:ci_build, :running, runner: two_projects_runner, project: project2) + let(:runner_id) { two_projects_runner.id } + let(:user) { user2 } + let(:api_params) { {} } + before_all do project.add_guest(user2) project2.add_maintainer(user2) - get api("/runners/#{two_projects_runner.id}/jobs", user2) + end + + it 'shows only jobs it has permission to see' do + request expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(1) + expect(json_response).to match([a_hash_including('id' => jobs[6].id)]) end end end context 'when valid status is provided' do + let(:query_params) { { status: :failed } } + it 'return filtered jobs' do - get api("/runners/#{project_runner.id}/jobs?status=failed", admin, admin_mode: true) + request expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(1) - expect(json_response.first).to include('id' => job_5.id) + expect(json_response).to match([a_hash_including('id' => jobs[4].id)]) end end context 'when valid order_by is provided' do + let(:query_params) { { order_by: :id } } + context 'when sort order is not specified' do it 'return jobs in descending order' do - get api("/runners/#{project_runner.id}/jobs?order_by=id", admin, admin_mode: true) + request expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(2) - expect(json_response.first).to include('id' => job_5.id) + expect(json_response).to match([ + a_hash_including('id' => jobs[4].id), + a_hash_including('id' => jobs[3].id) + ]) end end context 'when sort order is specified as asc' do + let(:query_params) { { order_by: :id, sort: :asc } } + it 'return jobs sorted in ascending order' do - get api("/runners/#{project_runner.id}/jobs?order_by=id&sort=asc", admin, admin_mode: true) + request expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(2) - expect(json_response.first).to include('id' => job_4.id) + expect(json_response).to match([ + a_hash_including('id' => jobs[3].id), + a_hash_including('id' => jobs[4].id) + ]) end end end context 'when invalid status is provided' do + let(:query_params) { { status: 'non-existing' } } + it 'return 400' do - get api("/runners/#{project_runner.id}/jobs?status=non-existing", admin, admin_mode: true) + request expect(response).to have_gitlab_http_status(:bad_request) end end context 'when invalid order_by is provided' do + let(:query_params) { { order_by: 'non-existing' } } + it 'return 400' do - get api("/runners/#{project_runner.id}/jobs?order_by=non-existing", admin, admin_mode: true) + request expect(response).to have_gitlab_http_status(:bad_request) end end context 'when invalid sort is provided' do + let(:query_params) { { sort: 'non-existing' } } + it 'return 400' do - get api("/runners/#{project_runner.id}/jobs?sort=non-existing", admin, admin_mode: true) + request expect(response).to have_gitlab_http_status(:bad_request) end end end - it 'avoids N+1 DB queries' do - get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true) + describe 'eager loading' do + let(:runner_id) { shared_runner.id } - control = ActiveRecord::QueryRecorder.new do - get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true) - end + it 'avoids N+1 DB queries' do + get api(path, user, **api_params) - create(:ci_build, :failed, runner: shared_runner, project: project) + control = ActiveRecord::QueryRecorder.new do + get api(path, user, **api_params) + end - expect do - get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true) - end.not_to exceed_query_limit(control.count) - end + create(:ci_build, :failed, runner: shared_runner, project: project) - it 'batches loading of commits' do - shared_runner = create(:ci_runner, :instance, description: 'Shared runner') + expect do + get api(path, user, **api_params) + end.not_to exceed_query_limit(control.count) + end - project_with_repo = create(:project, :repository) + it 'batches loading of commits' do + project_with_repo = create(:project, :repository) + shared_runner_manager1 = create(:ci_runner_machine, runner: shared_runner, system_xid: 'id1') - pipeline = create(:ci_pipeline, project: project_with_repo, sha: 'ddd0f15ae83993f5cb66a927a28673882e99100b') - create(:ci_build, :running, runner: shared_runner, project: project_with_repo, pipeline: pipeline) + pipeline = create(:ci_pipeline, project: project_with_repo, sha: 'ddd0f15ae83993f5cb66a927a28673882e99100b') + create(:ci_build, :running, runner_manager: shared_runner_manager1, project: project_with_repo, pipeline: pipeline) - pipeline = create(:ci_pipeline, project: project_with_repo, sha: 'c1c67abbaf91f624347bb3ae96eabe3a1b742478') - create(:ci_build, :failed, runner: shared_runner, project: project_with_repo, pipeline: pipeline) + pipeline = create(:ci_pipeline, project: project_with_repo, sha: 'c1c67abbaf91f624347bb3ae96eabe3a1b742478') + create(:ci_build, :failed, runner_manager: shared_runner_manager1, project: project_with_repo, pipeline: pipeline) - pipeline = create(:ci_pipeline, project: project_with_repo, sha: '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') - create(:ci_build, :failed, runner: shared_runner, project: project_with_repo, pipeline: pipeline) + pipeline = create(:ci_pipeline, project: project_with_repo, sha: '1a0b36b3cdad1d2ee32457c102a8c0b7056fa863') + create(:ci_build, :failed, runner_manager: shared_runner_manager1, project: project_with_repo, pipeline: pipeline) - expect_next_instance_of(Repository) do |repo| - expect(repo).to receive(:commits_by).with(oids: - %w[ - 1a0b36b3cdad1d2ee32457c102a8c0b7056fa863 - c1c67abbaf91f624347bb3ae96eabe3a1b742478 - ]).once.and_call_original - end + expect_next_instance_of(Repository) do |repo| + expect(repo).to receive(:commits_by).with(oids: + %w[ + 1a0b36b3cdad1d2ee32457c102a8c0b7056fa863 + c1c67abbaf91f624347bb3ae96eabe3a1b742478 + ]).once.and_call_original + end - get api("/runners/#{shared_runner.id}/jobs", admin, admin_mode: true), params: { per_page: 2, order_by: 'id', sort: 'desc' } + get api(path, admin, admin_mode: true), params: { per_page: 2, order_by: 'id', sort: 'desc' } + end end context "when runner doesn't exist" do + let(:runner_id) { non_existing_record_id } + it 'returns 404' do - get api('/runners/0/jobs', admin, admin_mode: true) + request expect(response).to have_gitlab_http_status(:not_found) end @@ -1004,70 +1057,118 @@ RSpec.describe API::Ci::Runners, :aggregate_failures, feature_category: :fleet_v context "runner project's administrative user" do context 'when runner exists' do + let(:runner_id) { shared_runner.id } + context 'when runner is shared' do it 'returns 403' do - get api("/runners/#{shared_runner.id}/jobs", user) + request expect(response).to have_gitlab_http_status(:forbidden) end end context 'when runner is a project runner' do + let(:runner_id) { project_runner.id } + it 'return jobs' do - get api(path, user) + request expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(2) + expect(json_response).to match([ + a_hash_including('id' => jobs[3].id), + a_hash_including('id' => jobs[4].id) + ]) end - end - context 'when valid status is provided' do - it 'return filtered jobs' do - get api("/runners/#{project_runner.id}/jobs?status=failed", user) + context 'when valid status is provided' do + let(:query_params) { { status: :failed } } - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers + it 'return filtered jobs' do + request + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers - expect(json_response).to be_an(Array) - expect(json_response.length).to eq(1) - expect(json_response.first).to include('id' => job_5.id) + expect(json_response).to match([ + a_hash_including('id' => jobs[4].id) + ]) + end end - end - context 'when invalid status is provided' do - it 'return 400' do - get api("/runners/#{project_runner.id}/jobs?status=non-existing", user) + context 'when invalid status is provided' do + let(:query_params) { { status: 'non-existing' } } - expect(response).to have_gitlab_http_status(:bad_request) + it 'return 400' do + request + + expect(response).to have_gitlab_http_status(:bad_request) + end end end end context "when runner doesn't exist" do + let(:runner_id) { non_existing_record_id } + it 'returns 404' do - get api('/runners/0/jobs', user) + request expect(response).to have_gitlab_http_status(:not_found) end end - end - context 'other authorized user' do - it 'does not return jobs' do - get api(path, user2) + context 'other authorized user' do + let(:user) { user2 } - expect(response).to have_gitlab_http_status(:forbidden) + it 'does not return jobs' do + request + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'unauthorized user' do + let(:user) { nil } + + it 'does not return jobs' do + request + + expect(response).to have_gitlab_http_status(:unauthorized) + end end end - context 'unauthorized user' do - it 'does not return jobs' do - get api(path) + context 'with system_id param' do + let(:system_id_params) { { system_id: system_id } } + let(:system_id) { 'id1' } + let(:user) { admin } + let(:api_params) { { admin_mode: true } } - expect(response).to have_gitlab_http_status(:unauthorized) + it 'returns jobs from the runner manager' do + request + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_limited_pagination_headers + expect(response.headers).not_to include('X-Total', 'X-Total-Pages') + + expect(json_response).to match([ + a_hash_including('id' => jobs[3].id), + a_hash_including('id' => jobs[4].id) + ]) + end + + context 'when system_id does not match runner' do + let(:runner_id) { shared_runner.id } + + it 'does not return jobs' do + request + + expect(response).to have_gitlab_http_status(:ok) + + expect(json_response).to be_empty + end end end end diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb index 4ec5d195ff8..06ef68f190f 100644 --- a/spec/requests/api/commits_spec.rb +++ b/spec/requests/api/commits_spec.rb @@ -1842,11 +1842,11 @@ RSpec.describe API::Commits, feature_category: :source_code_management do it 'are returned without N + 1' do get api(route, current_user) # warm up the cache - control_count = ActiveRecord::QueryRecorder.new { get api(route, current_user) }.count + control = ActiveRecord::QueryRecorder.new { get api(route, current_user) } create(:diff_note_on_commit, project: project, author: create(:user)) - expect { get api(route, current_user) }.not_to exceed_query_limit(control_count) + expect { get api(route, current_user) }.not_to exceed_query_limit(control) end end end @@ -2386,11 +2386,11 @@ RSpec.describe API::Commits, feature_category: :source_code_management do it 'returns multiple merge requests without N + 1' do perform_request(user) - control_count = ActiveRecord::QueryRecorder.new { perform_request(user) }.count + control = ActiveRecord::QueryRecorder.new { perform_request(user) } create(:merge_request, :closed, source_project: project, source_branch: 'master', target_branch: 'feature') - expect { perform_request(user) }.not_to exceed_query_limit(control_count) + expect { perform_request(user) }.not_to exceed_query_limit(control) end end @@ -2457,6 +2457,8 @@ RSpec.describe API::Commits, feature_category: :source_code_management do end context 'with ssh signed commit' do + let_it_be(:project) { create(:project, :repository, :public, :in_group) } + let(:commit_id) { '7b5160f9bb23a3d58a0accdbe89da13b96b1ece9' } let!(:commit) { project.commit(commit_id) } diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index 30c345ef458..ca19a97ae49 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -135,11 +135,11 @@ RSpec.describe API::DeployKeys, :aggregate_failures, feature_category: :continuo it 'returns multiple deploy keys without N + 1' do perform_request - control_count = ActiveRecord::QueryRecorder.new { perform_request }.count + control = ActiveRecord::QueryRecorder.new { perform_request } create(:deploy_key, public: true, projects: [project], user: maintainer) - expect { perform_request }.not_to exceed_query_limit(control_count) + expect { perform_request }.not_to exceed_query_limit(control) end end diff --git a/spec/requests/api/deployments_spec.rb b/spec/requests/api/deployments_spec.rb index 5a8e1649e75..f68307df779 100644 --- a/spec/requests/api/deployments_spec.rb +++ b/spec/requests/api/deployments_spec.rb @@ -143,11 +143,11 @@ RSpec.describe API::Deployments, feature_category: :continuous_delivery do it 'returns multiple deployments without N + 1' do perform_request # warm up the cache - control_count = ActiveRecord::QueryRecorder.new { perform_request }.count + control = ActiveRecord::QueryRecorder.new { perform_request } create(:deployment, :success, project: project, deployable: build, iid: 21, ref: 'master') - expect { perform_request }.not_to exceed_query_limit(control_count) + expect { perform_request }.not_to exceed_query_limit(control) end end diff --git a/spec/requests/api/draft_notes_spec.rb b/spec/requests/api/draft_notes_spec.rb index f15ed6e2d5f..fc465fa7c42 100644 --- a/spec/requests/api/draft_notes_spec.rb +++ b/spec/requests/api/draft_notes_spec.rb @@ -87,6 +87,10 @@ RSpec.describe API::DraftNotes, feature_category: :code_review_workflow do let!(:deleted_draft_note_id) { draft_note_by_current_user.id } before do + allow_next_instance_of(DraftNotes::DestroyService) do |service| + allow(service).to receive(:unfolded_drafts?).and_return(true) + end + delete api( "#{base_url}/#{draft_note_by_current_user.id}", user diff --git a/spec/requests/api/feature_flags_spec.rb b/spec/requests/api/feature_flags_spec.rb index 4fb0dfbb070..2e513194627 100644 --- a/spec/requests/api/feature_flags_spec.rb +++ b/spec/requests/api/feature_flags_spec.rb @@ -67,12 +67,12 @@ RSpec.describe API::FeatureFlags, feature_category: :feature_flags do end it 'does not have N+1 problem' do - control_count = ActiveRecord::QueryRecorder.new { subject } + control = ActiveRecord::QueryRecorder.new { subject } create_list(:operations_feature_flag, 3, project: project) expect { get api("/projects/#{project.id}/feature_flags", user) } - .not_to exceed_query_limit(control_count) + .not_to exceed_query_limit(control) end it_behaves_like 'check user permission' diff --git a/spec/requests/api/graphql/achievements/user_achievements_query_spec.rb b/spec/requests/api/graphql/achievements/user_achievements_query_spec.rb index 32048ea1432..94678bd18da 100644 --- a/spec/requests/api/graphql/achievements/user_achievements_query_spec.rb +++ b/spec/requests/api/graphql/achievements/user_achievements_query_spec.rb @@ -89,14 +89,14 @@ RSpec.describe 'UserAchievements', feature_category: :user_profile do end it 'can lookahead to eliminate N+1 queries', :use_clean_rails_memory_store_caching do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do post_graphql(query, current_user: user) - end.count + end user2 = create(:user) create(:user_achievement, achievement: achievement, user: user2) - expect { post_graphql(query, current_user: user) }.not_to exceed_all_query_limit(control_count) + expect { post_graphql(query, current_user: user) }.not_to exceed_all_query_limit(control) end context 'when the achievements feature flag is disabled' do diff --git a/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb b/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb index 86e2b288890..312700b1dcf 100644 --- a/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb +++ b/spec/requests/api/graphql/boards/board_list_issues_query_spec.rb @@ -115,8 +115,8 @@ RSpec.describe 'get board lists', feature_category: :team_planning do let(:issue_params) { { filters: { or: { assignee_usernames: [user.username, another_user.username] } } } } it 'returns correctly filtered issues' do - issue1.assignee_ids = user.id - issue2.assignee_ids = another_user.id + IssueAssignee.create!(issue_id: issue1.id, user_id: user.id) + IssueAssignee.create!(issue_id: issue2.id, user_id: another_user.id) subject diff --git a/spec/requests/api/graphql/ci/catalog/resource_spec.rb b/spec/requests/api/graphql/ci/catalog/resource_spec.rb index 9fe73e7ba45..836e52197a3 100644 --- a/spec/requests/api/graphql/ci/catalog/resource_spec.rb +++ b/spec/requests/api/graphql/ci/catalog/resource_spec.rb @@ -22,7 +22,7 @@ RSpec.describe 'Query.ciCatalogResource', feature_category: :pipeline_compositio ) end - let_it_be(:resource) { create(:ci_catalog_resource, :published, project: project) } + let_it_be_with_reload(:resource) { create(:ci_catalog_resource, :published, project: project) } let(:query) do <<~GQL @@ -81,7 +81,7 @@ RSpec.describe 'Query.ciCatalogResource', feature_category: :pipeline_compositio nodes { id name - path + includePath inputs { name default @@ -126,7 +126,7 @@ RSpec.describe 'Query.ciCatalogResource', feature_category: :pipeline_compositio a_graphql_entity_for( components.first, name: components.first.name, - path: components.first.path, + include_path: components.first.path, inputs: [ a_graphql_entity_for( name: 'tags', @@ -148,48 +148,68 @@ RSpec.describe 'Query.ciCatalogResource', feature_category: :pipeline_compositio a_graphql_entity_for( components.last, name: components.last.name, - path: components.last.path + include_path: components.last.path ) ) end end end - describe 'versions' do - let(:query) do - <<~GQL - query { - ciCatalogResource(id: "#{resource.to_global_id}") { - id - versions { - nodes { - id - tagName - tagPath - releasedAt - author { + describe 'version fields' do + before_all do + # To test the readme_html field, we need to create versions with real commit shas + project.repository.create_branch('branch_v2', project.default_branch) + project.repository.update_file( + user, 'README.md', 'Readme v2', message: 'Update readme', branch_name: 'branch_v2') + + project.repository.add_tag(user, 'v1', project.default_branch) + project.repository.add_tag(user, 'v2', 'branch_v2') + end + + let_it_be(:author) { create(:user, name: 'author') } + + let_it_be(:version1) do + create(:release, :with_catalog_resource_version, + project: project, + tag: 'v1', + sha: project.commit('v1').sha, + released_at: '2023-01-01T00:00:00Z', + author: author + ).catalog_resource_version + end + + let_it_be(:version2) do + create(:release, :with_catalog_resource_version, + project: project, + tag: 'v2', + sha: project.commit('v2').sha, + released_at: '2023-02-01T00:00:00Z', + author: author + ).catalog_resource_version + end + + describe 'versions' do + let(:query) do + <<~GQL + query { + ciCatalogResource(id: "#{resource.to_global_id}") { + id + versions { + nodes { id name - webUrl + path + releasedAt + author { + id + name + webUrl + } } } } } - } - GQL - end - - context 'when the resource has versions' do - let_it_be(:author) { create(:user, name: 'author') } - - let_it_be(:version1) do - create(:release, :with_catalog_resource_version, project: project, released_at: '2023-01-01T00:00:00Z', - author: author).catalog_resource_version - end - - let_it_be(:version2) do - create(:release, :with_catalog_resource_version, project: project, released_at: '2023-02-01T00:00:00Z', - author: author).catalog_resource_version + GQL end it 'returns the resource with the versions data' do @@ -202,67 +222,124 @@ RSpec.describe 'Query.ciCatalogResource', feature_category: :pipeline_compositio expect(graphql_data_at(:ciCatalogResource, :versions, :nodes)).to contain_exactly( a_graphql_entity_for( version1, - tagName: version1.name, - tagPath: project_tag_path(project, version1.name), + name: version1.name, + path: project_tag_path(project, version1.name), releasedAt: version1.released_at, author: a_graphql_entity_for(author, :name) ), a_graphql_entity_for( version2, - tagName: version2.name, - tagPath: project_tag_path(project, version2.name), + name: version2.name, + path: project_tag_path(project, version2.name), releasedAt: version2.released_at, author: a_graphql_entity_for(author, :name) ) ) end - end - context 'when the resource does not have a version' do - it 'returns versions as an empty array' do - post_query + context 'when the readmeHtml field is requested on more than one version' do + let(:query) do + <<~GQL + query { + ciCatalogResource(fullPath: "#{resource.project.full_path}") { + versions { + nodes { + readmeHtml + } + } + } + } + GQL + end - expect(graphql_data_at(:ciCatalogResource)).to match( - a_graphql_entity_for(resource, versions: { 'nodes' => [] }) - ) + it 'limits the request to 1 version at a time' do + post_query + + expect_graphql_errors_to_include \ + [/"readmeHtml" field can be requested only for 1 CiCatalogResourceVersion\(s\) at a time./] + end + end + + context 'when the name argument is provided' do + let(:name) { 'v1' } + + let(:query) do + <<~GQL + query { + ciCatalogResource(fullPath: "#{resource.project.full_path}") { + versions(name: "#{name}") { + nodes { + id + name + path + releasedAt + readmeHtml + } + } + } + } + GQL + end + + it 'returns the version that matches the name' do + post_query + + expect(graphql_data_at(:ciCatalogResource, :versions, :nodes)).to contain_exactly( + a_graphql_entity_for( + version1, + name: version1.name, + path: project_tag_path(project, version1.name), + releasedAt: version1.released_at, + readmeHtml: a_string_including( + "#{project.full_path}/-/blob/#{project.default_branch}/README.md" + ) + ) + ) + end + + context 'when no version matches the name' do + let(:name) { 'does_not_exist' } + + it 'returns an empty array' do + post_query + + expect(graphql_data_at(:ciCatalogResource, :versions, :nodes)).to eq([]) + end + end + end + + context 'when the resource does not have a version' do + it 'returns an empty array' do + resource.versions.delete_all(:delete_all) + + post_query + + expect(graphql_data_at(:ciCatalogResource, :versions, :nodes)).to eq([]) + end end end - end - describe 'latestVersion' do - let(:query) do - <<~GQL - query { - ciCatalogResource(id: "#{resource.to_global_id}") { - id - latestVersion { + describe 'latestVersion' do + let(:query) do + <<~GQL + query { + ciCatalogResource(id: "#{resource.to_global_id}") { id - tagName - tagPath - releasedAt - author { + latestVersion { id name - webUrl + path + releasedAt + readmeHtml + author { + id + name + webUrl + } } } } - } - GQL - end - - context 'when the resource has versions' do - let_it_be(:author) { create(:user, name: 'author') } - - let_it_be(:latest_version) do - create(:release, :with_catalog_resource_version, project: project, released_at: '2023-02-01T00:00:00Z', - author: author).catalog_resource_version - end - - before_all do - # Previous version of the catalog resource - create(:release, :with_catalog_resource_version, project: project, released_at: '2023-01-01T00:00:00Z', - author: author) + GQL end it 'returns the resource with the latest version data' do @@ -272,24 +349,27 @@ RSpec.describe 'Query.ciCatalogResource', feature_category: :pipeline_compositio a_graphql_entity_for( resource, latestVersion: a_graphql_entity_for( - latest_version, - tagName: latest_version.name, - tagPath: project_tag_path(project, latest_version.name), - releasedAt: latest_version.released_at, + version2, + name: version2.name, + path: project_tag_path(project, version2.name), + releasedAt: version2.released_at, + readmeHtml: a_string_including('Readme v2'), author: a_graphql_entity_for(author, :name) ) ) ) end - end - context 'when the resource does not have a version' do - it 'returns nil' do - post_query + context 'when the resource does not have a version' do + it 'returns nil' do + resource.versions.delete_all(:delete_all) - expect(graphql_data_at(:ciCatalogResource)).to match( - a_graphql_entity_for(resource, latestVersion: nil) - ) + post_query + + expect(graphql_data_at(:ciCatalogResource)).to match( + a_graphql_entity_for(resource, latestVersion: nil) + ) + end end end end diff --git a/spec/requests/api/graphql/ci/catalog/resources_spec.rb b/spec/requests/api/graphql/ci/catalog/resources_spec.rb index 49a3f3be1d7..150507fc442 100644 --- a/spec/requests/api/graphql/ci/catalog/resources_spec.rb +++ b/spec/requests/api/graphql/ci/catalog/resources_spec.rb @@ -106,7 +106,7 @@ RSpec.describe 'Query.ciCatalogResources', feature_category: :pipeline_compositi versions { nodes { id - tagName + name releasedAt author { id @@ -153,7 +153,7 @@ RSpec.describe 'Query.ciCatalogResources', feature_category: :pipeline_compositi id latestVersion { id - tagName + name releasedAt author { id @@ -185,7 +185,7 @@ RSpec.describe 'Query.ciCatalogResources', feature_category: :pipeline_compositi resource1, latestVersion: a_graphql_entity_for( latest_version1, - tagName: latest_version1.name, + name: latest_version1.name, releasedAt: latest_version1.released_at, author: a_graphql_entity_for(author1, :name) ) @@ -194,7 +194,7 @@ RSpec.describe 'Query.ciCatalogResources', feature_category: :pipeline_compositi public_resource, latestVersion: a_graphql_entity_for( latest_version2, - tagName: latest_version2.name, + name: latest_version2.name, releasedAt: latest_version2.released_at, author: a_graphql_entity_for(author2, :name) ) diff --git a/spec/requests/api/graphql/ci/instance_variables_spec.rb b/spec/requests/api/graphql/ci/instance_variables_spec.rb index a612b4c91b6..6731631a075 100644 --- a/spec/requests/api/graphql/ci/instance_variables_spec.rb +++ b/spec/requests/api/graphql/ci/instance_variables_spec.rb @@ -12,6 +12,7 @@ RSpec.describe 'Query.ciVariables', feature_category: :secrets_management do nodes { id key + description value variableType protected @@ -36,6 +37,7 @@ RSpec.describe 'Query.ciVariables', feature_category: :secrets_management do expect(graphql_data.dig('ciVariables', 'nodes')).to contain_exactly({ 'id' => variable.to_global_id.to_s, 'key' => 'TEST_VAR', + 'description' => nil, 'value' => 'test', 'variableType' => 'ENV_VAR', 'masked' => false, diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 8262640b283..1b6948d0380 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -876,107 +876,95 @@ RSpec.describe 'Query.runner(id)', :freeze_time, feature_category: :fleet_visibi end describe 'Query limits' do - def runner_query(runner) - <<~SINGLE - runner(id: "#{runner.to_global_id}") { - #{all_graphql_fields_for('CiRunner', excluded: excluded_fields)} - createdBy { - id - username - webPath - webUrl - } - groups { - nodes { - id - path - fullPath - webUrl - } - } - projects { - nodes { - id - path - fullPath - webUrl - } - } - ownerProject { - id - path - fullPath - webUrl - } + let_it_be(:user2) { another_admin } + let_it_be(:user3) { create(:user) } + let_it_be(:tag_list) { %w[n_plus_1_test some_tag] } + let_it_be(:args) do + { current_user: user, token: { personal_access_token: create(:personal_access_token, user: user) } } + end + + let_it_be(:runner1) { create(:ci_runner, tag_list: tag_list, creator: user) } + let_it_be(:runner2) do + create(:ci_runner, :group, groups: [group], tag_list: tag_list, creator: user) + end + + let_it_be(:runner3) do + create(:ci_runner, :project, projects: [project1], tag_list: tag_list, creator: user) + end + + let(:single_discrete_runners_query) do + multiple_discrete_runners_query([]) + end + + let(:runner_fragment) do + <<~QUERY + #{all_graphql_fields_for('CiRunner', excluded: excluded_fields)} + createdBy { + id + username + webPath + webUrl } - SINGLE + QUERY end - let(:active_project_runner2) { create(:ci_runner, :project) } - let(:active_group_runner2) { create(:ci_runner, :group) } + # Exclude fields that are already hardcoded above (or tested separately), + # and also some fields from deeper objects which are problematic: + # - createdBy: Known N+1 issues, but only on exotic fields which we don't normally use + # - ownerProject.pipeline: Needs arguments (iid or sha) + # - project.productAnalyticsState: Can be requested only for 1 Project(s) at a time. + let(:excluded_fields) { %w[createdBy jobs pipeline productAnalyticsState] } + + it 'avoids N+1 queries', :use_sql_query_cache do + discrete_runners_control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + post_graphql(single_discrete_runners_query, **args) + end + + additional_runners = setup_additional_records + + expect do + post_graphql(multiple_discrete_runners_query(additional_runners), **args) - # Exclude fields that are already hardcoded above - let(:excluded_fields) { %w[createdBy jobs groups projects ownerProject] } + raise StandardError, flattened_errors if graphql_errors # Ensure any error in query causes test to fail + end.not_to exceed_query_limit(discrete_runners_control) + end - let(:single_query) do + def runner_query(runner, nr) <<~QUERY - { - instance_runner1: #{runner_query(active_instance_runner)} - group_runner1: #{runner_query(active_group_runner)} - project_runner1: #{runner_query(active_project_runner)} + runner#{nr}: runner(id: "#{runner.to_global_id}") { + #{runner_fragment} } QUERY end - let(:double_query) do + def multiple_discrete_runners_query(additional_runners) <<~QUERY { - instance_runner1: #{runner_query(active_instance_runner)} - instance_runner2: #{runner_query(inactive_instance_runner)} - group_runner1: #{runner_query(active_group_runner)} - group_runner2: #{runner_query(active_group_runner2)} - project_runner1: #{runner_query(active_project_runner)} - project_runner2: #{runner_query(active_project_runner2)} + #{runner_query(runner1, 1)} + #{runner_query(runner2, 2)} + #{runner_query(runner3, 3)} + #{additional_runners.each_with_index.map { |r, i| runner_query(r, 4 + i) }.join("\n")} } QUERY end - it 'does not execute more queries per runner', :aggregate_failures, quarantine: "https://gitlab.com/gitlab-org/gitlab/-/issues/391442" do - # warm-up license cache and so on: - personal_access_token = create(:personal_access_token, user: user) - args = { current_user: user, token: { personal_access_token: personal_access_token } } - post_graphql(double_query, **args) - - control = ActiveRecord::QueryRecorder.new { post_graphql(single_query, **args) } - - personal_access_token = create(:personal_access_token, user: another_admin) - args = { current_user: another_admin, token: { personal_access_token: personal_access_token } } - expect { post_graphql(double_query, **args) }.not_to exceed_query_limit(control) - - expect(graphql_data.count).to eq 6 - expect(graphql_data).to match( - a_hash_including( - 'instance_runner1' => a_graphql_entity_for(active_instance_runner), - 'instance_runner2' => a_graphql_entity_for(inactive_instance_runner), - 'group_runner1' => a_graphql_entity_for( - active_group_runner, - groups: { 'nodes' => contain_exactly(a_graphql_entity_for(group)) } - ), - 'group_runner2' => a_graphql_entity_for( - active_group_runner2, - groups: { 'nodes' => active_group_runner2.groups.map { |g| a_graphql_entity_for(g) } } - ), - 'project_runner1' => a_graphql_entity_for( - active_project_runner, - projects: { 'nodes' => active_project_runner.projects.map { |p| a_graphql_entity_for(p) } }, - owner_project: a_graphql_entity_for(active_project_runner.projects[0]) - ), - 'project_runner2' => a_graphql_entity_for( - active_project_runner2, - projects: { 'nodes' => active_project_runner2.projects.map { |p| a_graphql_entity_for(p) } }, - owner_project: a_graphql_entity_for(active_project_runner2.projects[0]) - ) - )) + def setup_additional_records + # Add more runners (including owned by other users) + runner4 = create(:ci_runner, tag_list: tag_list + %w[tag1 tag2], creator: user2) + runner5 = create(:ci_runner, :group, groups: [create(:group)], tag_list: tag_list + %w[tag2 tag3], creator: user3) + # Add one more project to runner + runner3.assign_to(create(:project)) + + # Add more runner managers (including to existing runners) + runner_manager1 = create(:ci_runner_machine, runner: runner1) + create(:ci_runner_machine, runner: runner1) + create(:ci_runner_machine, runner: runner2, system_xid: runner_manager1.system_xid) + create(:ci_runner_machine, runner: runner3) + create(:ci_runner_machine, runner: runner4, version: '16.4.1') + create(:ci_runner_machine, runner: runner5, version: '16.4.0', system_xid: runner_manager1.system_xid) + create(:ci_runner_machine, runner: runner3) + + [runner4, runner5] end end diff --git a/spec/requests/api/graphql/ci/runners_spec.rb b/spec/requests/api/graphql/ci/runners_spec.rb index 0fe14bef778..bfe5282cbaa 100644 --- a/spec/requests/api/graphql/ci/runners_spec.rb +++ b/spec/requests/api/graphql/ci/runners_spec.rb @@ -18,22 +18,34 @@ RSpec.describe 'Query.runners', feature_category: :fleet_visibility do let(:fields) do <<~QUERY nodes { - #{all_graphql_fields_for('CiRunner', excluded: %w[createdBy ownerProject])} - createdBy { - username - webPath - webUrl - } - ownerProject { - id - path - fullPath - webUrl - } + #{all_graphql_fields_for('CiRunner', excluded: excluded_fields)} } QUERY end + let(:query) do + %( + query { + runners { + #{fields} + } + } + ) + end + + # Exclude fields from deeper objects which are problematic: + # - ownerProject.pipeline: Needs arguments (iid or sha) + # - project.productAnalyticsState: Can be requested only for 1 Project(s) at a time. + let(:excluded_fields) { %w[pipeline productAnalyticsState] } + + it 'returns expected runners' do + post_graphql(query, current_user: current_user) + + expect(runners_graphql_data['nodes']).to contain_exactly( + *Ci::Runner.all.map { |expected_runner| a_graphql_entity_for(expected_runner) } + ) + end + context 'with filters' do shared_examples 'a working graphql query returning expected runners' do it_behaves_like 'a working graphql query' do @@ -49,31 +61,6 @@ RSpec.describe 'Query.runners', feature_category: :fleet_visibility do *Array(expected_runners).map { |expected_runner| a_graphql_entity_for(expected_runner) } ) end - - it 'does not execute more queries per runner', :aggregate_failures do - # warm-up license cache and so on: - personal_access_token = create(:personal_access_token, user: current_user) - args = { current_user: current_user, token: { personal_access_token: personal_access_token } } - post_graphql(query, **args) - expect(graphql_data_at(:runners, :nodes)).not_to be_empty - - admin2 = create(:admin) - personal_access_token = create(:personal_access_token, user: admin2) - args = { current_user: admin2, token: { personal_access_token: personal_access_token } } - control = ActiveRecord::QueryRecorder.new { post_graphql(query, **args) } - - runner2 = create(:ci_runner, :instance, version: '14.0.0', tag_list: %w[tag5 tag6], creator: admin2) - runner3 = create(:ci_runner, :project, version: '14.0.1', projects: [project], tag_list: %w[tag3 tag8], - creator: current_user) - - create(:ci_build, :failed, runner: runner2) - create(:ci_runner_machine, runner: runner2, version: '16.4.1') - - create(:ci_build, :failed, runner: runner3) - create(:ci_runner_machine, runner: runner3, version: '16.4.0') - - expect { post_graphql(query, **args) }.not_to exceed_query_limit(control) - end end context 'when filtered on type and status' do @@ -178,56 +165,129 @@ RSpec.describe 'Query.runners', feature_category: :fleet_visibility do end end end - end - context 'without filters' do - context 'with managers requested for multiple runners' do - let(:fields) do - <<~QUERY - nodes { - managers { - nodes { - #{all_graphql_fields_for('CiRunnerManager', max_depth: 1)} - } - } - } - QUERY - end + context 'when filtered by creator' do + let_it_be(:user) { create(:user) } + let_it_be(:runner_created_by_user) { create(:ci_runner, :project, creator: user) } let(:query) do %( query { - runners { + runners(creatorId: "#{creator.to_global_id}") { #{fields} } } ) end - it 'does not execute more queries per runner', :aggregate_failures do - # warm-up license cache and so on: - personal_access_token = create(:personal_access_token, user: current_user) - args = { current_user: current_user, token: { personal_access_token: personal_access_token } } - post_graphql(query, **args) - expect(graphql_data_at(:runners, :nodes)).not_to be_empty - - admin2 = create(:admin) - personal_access_token = create(:personal_access_token, user: admin2) - args = { current_user: admin2, token: { personal_access_token: personal_access_token } } - control = ActiveRecord::QueryRecorder.new { post_graphql(query, **args) } - - create(:ci_runner, :instance, :with_runner_manager, version: '14.0.0', tag_list: %w[tag5 tag6], - creator: admin2) - create(:ci_runner, :project, :with_runner_manager, version: '14.0.1', projects: [project], - tag_list: %w[tag3 tag8], - creator: current_user) - - expect { post_graphql(query, **args) }.not_to exceed_query_limit(control) + context 'when existing user id given' do + let(:creator) { user } + + before do + create(:ci_runner, :project, creator: create(:user)) # Should not be returned + end + + it_behaves_like 'a working graphql query returning expected runners' do + let(:expected_runners) { runner_created_by_user } + end + end + + context 'when non existent user id given' do + let(:creator) { User.new(id: non_existing_record_id) } + + it 'does not return any runners' do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:runners, :nodes)).to be_empty + end end end end end + describe 'Runner query limits' do + let_it_be(:user) { create(:user, :admin) } + let_it_be(:user2) { create(:user) } + let_it_be(:user3) { create(:user) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project) } + let_it_be(:tag_list) { %w[n_plus_1_test some_tag] } + let_it_be(:args) do + { current_user: user, token: { personal_access_token: create(:personal_access_token, user: user) } } + end + + let_it_be(:runner1) { create(:ci_runner, tag_list: tag_list, creator: user) } + let_it_be(:runner2) do + create(:ci_runner, :group, groups: [group], tag_list: tag_list, creator: user) + end + + let_it_be(:runner3) do + create(:ci_runner, :project, projects: [project], tag_list: tag_list, creator: user) + end + + let(:runner_fragment) do + <<~QUERY + #{all_graphql_fields_for('CiRunner', excluded: excluded_fields)} + createdBy { + id + username + webPath + webUrl + } + QUERY + end + + # Exclude fields that are already hardcoded above (or tested separately), + # and also some fields from deeper objects which are problematic: + # - createdBy: Known N+1 issues, but only on exotic fields which we don't normally use + # - ownerProject.pipeline: Needs arguments (iid or sha) + # - project.productAnalyticsState: Can be requested only for 1 Project(s) at a time. + let(:excluded_fields) { %w[createdBy jobs pipeline productAnalyticsState] } + + let(:runners_query) do + <<~QUERY + { + runners { + nodes { #{runner_fragment} } + } + } + QUERY + end + + it 'avoids N+1 queries', :use_sql_query_cache do + personal_access_token = create(:personal_access_token, user: user) + args = { current_user: user, token: { personal_access_token: personal_access_token } } + + runners_control = ActiveRecord::QueryRecorder.new(skip_cached: false) { post_graphql(runners_query, **args) } + + setup_additional_records + + expect { post_graphql(runners_query, **args) }.not_to exceed_query_limit(runners_control) + end + + def setup_additional_records + # Add more runners (including owned by other users) + runner4 = create(:ci_runner, tag_list: tag_list + %w[tag1 tag2], creator: user2) + runner5 = create(:ci_runner, :group, groups: [create(:group)], tag_list: tag_list + %w[tag2 tag3], creator: user3) + # Add one more project to runner + runner3.assign_to(create(:project)) + + # Add more runner managers (including to existing runners) + runner_manager1 = create(:ci_runner_machine, runner: runner1) + create(:ci_runner_machine, runner: runner1) + create(:ci_runner_machine, runner: runner2, system_xid: runner_manager1.system_xid) + create(:ci_runner_machine, runner: runner3) + create(:ci_runner_machine, runner: runner4, version: '16.4.1') + create(:ci_runner_machine, runner: runner5, version: '16.4.0', system_xid: runner_manager1.system_xid) + create(:ci_runner_machine, runner: runner3) + + create(:ci_build, :failed, runner: runner4) + create(:ci_build, :failed, runner: runner5) + + [runner4, runner5] + end + end + describe 'pagination' do let(:data_path) { [:runners] } diff --git a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb index 2acdd509355..46563aba992 100644 --- a/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb +++ b/spec/requests/api/graphql/container_repository/container_repository_details_spec.rb @@ -430,6 +430,85 @@ RSpec.describe 'container repository details', feature_category: :container_regi it_behaves_like 'returning an invalid value error' end + + context 'with referrers' do + let(:tags_response) { container_repository_details_response.dig('tags', 'edges') } + let(:raw_tags_response) do + [ + { + name: 'latest', + digest: 'sha256:1234567892', + config_digest: 'sha256:3332132331', + media_type: 'application/vnd.oci.image.manifest.v1+json', + size_bytes: 1234509876, + created_at: 10.minutes.ago, + updated_at: 10.minutes.ago, + referrers: [ + { + artifactType: 'application/vnd.example+type', + digest: 'sha256:57d3be92c2f857566ecc7f9306a80021c0a7fa631e0ef5146957235aea859961' + }, + { + artifactType: 'application/vnd.example+type+2', + digest: 'sha256:01db72e42d61b8d2183d53475814cce2bfb9c8a254e97539a852441979cd5c90' + } + ] + }, + { + name: 'latest', + digest: 'sha256:1234567893', + config_digest: 'sha256:3332132331', + media_type: 'application/vnd.oci.image.manifest.v1+json', + size_bytes: 1234509877, + created_at: 9.minutes.ago, + updated_at: 9.minutes.ago + } + ] + end + + let(:query) do + <<~GQL + query($id: ContainerRepositoryID!, $n: String) { + containerRepository(id: $id) { + tags(name: $n, referrers: true) { + edges { + node { + #{all_graphql_fields_for('ContainerRepositoryTag')} + } + } + } + } + } + GQL + end + + let(:url) { URI('/gitlab/v1/repositories/group1/proj1/tags/list/?before=tag1&referrers=true') } + + let(:response_body) do + { + pagination: { previous: { uri: url }, next: { uri: url } }, + response_body: ::Gitlab::Json.parse(raw_tags_response.to_json) + } + end + + it 'includes referrers in response' do + subject + + refs = tags_response.map { |tag| tag.dig('node', 'referrers') } + + expect(refs.first.size).to eq(2) + expect(refs.first.first).to include({ + 'artifactType' => 'application/vnd.example+type', + 'digest' => 'sha256:57d3be92c2f857566ecc7f9306a80021c0a7fa631e0ef5146957235aea859961' + }) + expect(refs.first.second).to include({ + 'artifactType' => 'application/vnd.example+type+2', + 'digest' => 'sha256:01db72e42d61b8d2183d53475814cce2bfb9c8a254e97539a852441979cd5c90' + }) + + expect(refs.second).to be_empty + end + end end it_behaves_like 'handling graphql network errors with the container registry' diff --git a/spec/requests/api/graphql/mutations/branch_rules/create_spec.rb b/spec/requests/api/graphql/mutations/branch_rules/create_spec.rb new file mode 100644 index 00000000000..85ba3d58ee5 --- /dev/null +++ b/spec/requests/api/graphql/mutations/branch_rules/create_spec.rb @@ -0,0 +1,68 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'BranchRuleCreate', feature_category: :source_code_management do + include GraphqlHelpers + let_it_be(:project) { create(:project, :public) } + let_it_be(:current_user, reload: true) { create(:user) } + + let(:params) do + { + project_path: project.full_path, + name: branch_name + } + end + + let(:branch_name) { 'branch_name/*' } + let(:mutation) { graphql_mutation(:branch_rule_create, params) } + let(:mutation_response) { graphql_mutation_response(:branch_rule_create) } + let(:mutation_errors) { mutation_response['errors'] } + + subject(:post_mutation) { post_graphql_mutation(mutation, current_user: current_user) } + + context 'when the user does not have permission' do + before_all do + project.add_developer(current_user) + end + + it_behaves_like 'a mutation that returns a top-level access error' + + it 'does not create the board' do + expect { post_mutation }.not_to change { ProtectedBranch.count } + end + end + + context 'when the user can create a branch rules' do + before_all do + project.add_maintainer(current_user) + end + + it 'creates the protected branch' do + expect { post_mutation }.to change { ProtectedBranch.count }.by(1) + end + + it 'returns the created branch rule' do + post_mutation + + expect(mutation_response).to have_key('branchRule') + expect(mutation_response['branchRule']['name']).to eq(branch_name) + expect(mutation_errors).to be_empty + end + + context 'when the branch rule already exist' do + let!(:existing_rule) { create :protected_branch, name: branch_name, project: project } + + it 'does not create the protected branch' do + expect { post_mutation }.not_to change { ProtectedBranch.count } + end + + it 'return an error message' do + post_mutation + + expect(mutation_errors).to include 'Name has already been taken' + expect(mutation_response['branchRule']).to be_nil + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/ml/models/create_spec.rb b/spec/requests/api/graphql/mutations/ml/models/create_spec.rb new file mode 100644 index 00000000000..0daabeab0d1 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ml/models/create_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Creation of a machine learning model', feature_category: :mlops do + include GraphqlHelpers + + let_it_be(:model) { create(:ml_models) } + let_it_be(:project) { model.project } + let_it_be(:current_user) { project.owner } + + let(:input) { { project_path: project.full_path, name: name, description: description } } + let(:name) { 'some_name' } + let(:description) { 'A description' } + + let(:mutation) { graphql_mutation(:ml_model_create, input) } + let(:mutation_response) { graphql_mutation_response(:ml_model_create) } + + context 'when user is not allowed write changes' do + before do + allow(Ability).to receive(:allowed?).and_call_original + allow(Ability).to receive(:allowed?) + .with(current_user, :write_model_registry, project) + .and_return(false) + end + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user is allowed write changes' do + it 'creates a models' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['model']).to include( + 'name' => name, + 'description' => description + ) + end + + context 'when name already exists' do + err_msg = "Name has already been taken" + let(:name) { model.name } + + it_behaves_like 'a mutation that returns errors in the response', errors: [err_msg] + end + end +end diff --git a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb index 05c1a2d96d9..7c5d86b9f5c 100644 --- a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb +++ b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb @@ -23,7 +23,9 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis lock_npm_package_requests_forwarding: true, pypi_package_requests_forwarding: true, lock_pypi_package_requests_forwarding: true, - nuget_symbol_server_enabled: true + nuget_symbol_server_enabled: true, + terraform_module_duplicates_allowed: true, + terraform_module_duplicate_exception_regex: 'foo-.*' } end @@ -44,6 +46,8 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis pypiPackageRequestsForwarding lockPypiPackageRequestsForwarding nugetSymbolServerEnabled + terraformModuleDuplicatesAllowed + terraformModuleDuplicateExceptionRegex } errors QL @@ -73,6 +77,8 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis expect(package_settings_response['npmPackageRequestsForwarding']).to eq(params[:npm_package_requests_forwarding]) expect(package_settings_response['lockNpmPackageRequestsForwarding']).to eq(params[:lock_npm_package_requests_forwarding]) expect(package_settings_response['nugetSymbolServerEnabled']).to eq(params[:nuget_symbol_server_enabled]) + expect(package_settings_response['terraformModuleDuplicatesAllowed']).to eq(params[:terraform_module_duplicates_allowed]) + expect(package_settings_response['terraformModuleDuplicateExceptionRegex']).to eq(params[:terraform_module_duplicate_exception_regex]) end end @@ -115,7 +121,9 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis lock_npm_package_requests_forwarding: false, pypi_package_requests_forwarding: nil, lock_pypi_package_requests_forwarding: false, - nuget_symbol_server_enabled: false + nuget_symbol_server_enabled: false, + terraform_module_duplicates_allowed: false, + terraform_module_duplicate_exception_regex: 'foo' }, to: { maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'foo-.*', @@ -129,7 +137,9 @@ RSpec.describe 'Updating the package settings', feature_category: :package_regis lock_npm_package_requests_forwarding: true, pypi_package_requests_forwarding: true, lock_pypi_package_requests_forwarding: true, - nuget_symbol_server_enabled: true + nuget_symbol_server_enabled: true, + terraform_module_duplicates_allowed: true, + terraform_module_duplicate_exception_regex: 'foo-.*' } it_behaves_like 'returning a success' diff --git a/spec/requests/api/graphql/mutations/organizations/update_spec.rb b/spec/requests/api/graphql/mutations/organizations/update_spec.rb index 4e819c280d0..33890ae4592 100644 --- a/spec/requests/api/graphql/mutations/organizations/update_spec.rb +++ b/spec/requests/api/graphql/mutations/organizations/update_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Mutations::Organizations::Update, feature_category: :cell do let_it_be(:user) { create(:user) } let_it_be_with_reload(:organization) do - create(:organization) { |org| create(:organization_user, organization: org, user: user) } + create(:organization) { |org| create(:organization_user, :owner, organization: org, user: user) } end let(:mutation) { graphql_mutation(:organization_update, params) } diff --git a/spec/requests/api/graphql/mutations/work_items/create_spec.rb b/spec/requests/api/graphql/mutations/work_items/create_spec.rb index 78b93c3210b..2c2cd5f2acc 100644 --- a/spec/requests/api/graphql/mutations/work_items/create_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/create_spec.rb @@ -281,6 +281,18 @@ RSpec.describe 'Create a work item', feature_category: :team_planning do it_behaves_like 'creates work item' + # This is a temporary measure just to ensure the internal id migration doesn't get conflicts + # More info in https://gitlab.com/gitlab-org/gitlab/-/merge_requests/139367 + context 'when making the request in a production environment' do + before do + stub_rails_env('production') + end + + it_behaves_like 'a mutation that returns top-level errors', errors: [ + 'Group level work items are disabled. Only project paths allowed in `namespacePath`.' + ] + end + context 'when the namespace_level_work_items feature flag is disabled' do before do stub_feature_flags(namespace_level_work_items: false) diff --git a/spec/requests/api/graphql/namespace/projects_spec.rb b/spec/requests/api/graphql/namespace/projects_spec.rb index a4bc94798be..107fcd8dcdd 100644 --- a/spec/requests/api/graphql/namespace/projects_spec.rb +++ b/spec/requests/api/graphql/namespace/projects_spec.rb @@ -20,6 +20,7 @@ RSpec.describe 'getting projects', feature_category: :groups_and_projects do 'namespace', { 'fullPath' => subject.full_path }, <<~QUERY + id projects(includeSubgroups: #{include_subgroups}) { edges { node { @@ -53,24 +54,30 @@ RSpec.describe 'getting projects', feature_category: :groups_and_projects do expect(graphql_data['namespace']['projects']['edges'].size).to eq(count) end + end - context 'with no user' do - it 'finds only public projects' do - post_graphql(query, current_user: nil) + it_behaves_like 'a graphql namespace' - expect(graphql_data['namespace']).to be_nil - end + context 'when no user is given' do + it 'finds only public projects' do + post_graphql(query, current_user: nil) + + expect(graphql_data_at(:namespace, :projects, :edges).size).to eq(1) end end - it_behaves_like 'a graphql namespace' - context 'when the namespace is a user' do subject { user.namespace } let(:include_subgroups) { false } it_behaves_like 'a graphql namespace' + + it 'does not show namespace entity for anonymous user' do + post_graphql(query, current_user: nil) + + expect(graphql_data['namespace']).to be_nil + end end context 'when not including subgroups' do diff --git a/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb b/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb index c8819f1e38f..273b6b8c25b 100644 --- a/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb +++ b/spec/requests/api/graphql/namespace/root_storage_statistics_spec.rb @@ -63,7 +63,7 @@ RSpec.describe 'rendering namespace statistics', feature_category: :metrics do it 'hides statistics for unauthenticated requests' do post_graphql(query, current_user: nil) - expect(graphql_data['namespace']).to be_blank + expect(graphql_data_at(:namespace, :root_storage_statistics)).to be_blank end end end diff --git a/spec/requests/api/graphql/namespace_query_spec.rb b/spec/requests/api/graphql/namespace_query_spec.rb index c0c7c5fee2b..86808915564 100644 --- a/spec/requests/api/graphql/namespace_query_spec.rb +++ b/spec/requests/api/graphql/namespace_query_spec.rb @@ -8,7 +8,8 @@ RSpec.describe 'Query', feature_category: :groups_and_projects do let_it_be(:user) { create(:user) } let_it_be(:other_user) { create(:user) } - let_it_be(:group_namespace) { create(:group) } + let_it_be(:group_namespace) { create(:group, :private) } + let_it_be(:public_group_namespace) { create(:group, :public) } let_it_be(:user_namespace) { create(:user_namespace, owner: user) } let_it_be(:project_namespace) { create(:project_namespace, parent: group_namespace) } @@ -60,6 +61,51 @@ RSpec.describe 'Query', feature_category: :groups_and_projects do end end + context 'when used with a public group' do + let(:target_namespace) { public_group_namespace } + + before do + subject + end + + it_behaves_like 'a working graphql query' + + context 'when user is a member' do + before do + public_group_namespace.add_developer(user) + end + + it 'fetches the expected data' do + expect(query_result).to include( + 'fullPath' => target_namespace.full_path, + 'name' => target_namespace.name + ) + end + end + + context 'when user is anonymous' do + let(:current_user) { nil } + + it 'fetches the expected data' do + expect(query_result).to include( + 'fullPath' => target_namespace.full_path, + 'name' => target_namespace.name + ) + end + end + + context 'when user is not a member' do + let(:current_user) { other_user } + + it 'fetches the expected data' do + expect(query_result).to include( + 'fullPath' => target_namespace.full_path, + 'name' => target_namespace.name + ) + end + end + end + it_behaves_like 'retrieving a namespace' do let(:target_namespace) { group_namespace } diff --git a/spec/requests/api/graphql/organizations/organization_query_spec.rb b/spec/requests/api/graphql/organizations/organization_query_spec.rb index c485e3b170d..14becd52e93 100644 --- a/spec/requests/api/graphql/organizations/organization_query_spec.rb +++ b/spec/requests/api/graphql/organizations/organization_query_spec.rb @@ -7,7 +7,6 @@ RSpec.describe 'getting organization information', feature_category: :cell do let(:query) { graphql_query_for(:organization, { id: organization.to_global_id }, organization_fields) } let(:current_user) { user } - let(:groups) { graphql_data_at(:organization, :groups, :nodes) } let(:organization_fields) do <<~FIELDS id @@ -23,24 +22,9 @@ RSpec.describe 'getting organization information', feature_category: :cell do let_it_be(:organization_user) { create(:organization_user) } let_it_be(:organization) { organization_user.organization } let_it_be(:user) { organization_user.user } - let_it_be(:parent_group) { create(:group, name: 'parent-group', organization: organization) } - let_it_be(:public_group) { create(:group, name: 'public-group', parent: parent_group, organization: organization) } - let_it_be(:other_group) { create(:group, name: 'other-group', organization: organization) } - let_it_be(:outside_organization_group) { create(:group) } - - let_it_be(:private_group) do - create(:group, :private, name: 'private-group', organization: organization) - end - - let_it_be(:no_access_group_in_org) do - create(:group, :private, name: 'no-access', organization: organization) - end - - before_all do - private_group.add_developer(user) - public_group.add_developer(user) - other_group.add_developer(user) - outside_organization_group.add_developer(user) + let_it_be(:project) { create(:project, organization: organization) { |p| p.add_developer(user) } } + let_it_be(:other_group) do + create(:group, name: 'other-group', organization: organization) { |g| g.add_developer(user) } end subject(:request_organization) { post_graphql(query, current_user: current_user) } @@ -62,25 +46,6 @@ RSpec.describe 'getting organization information', feature_category: :cell do end end - context 'when resolve_organization_groups feature flag is disabled' do - before do - stub_feature_flags(resolve_organization_groups: false) - end - - it 'returns no groups' do - request_organization - - expect(graphql_data_at(:organization)).not_to be_nil - expect(graphql_data_at(:organization, :groups, :nodes)).to be_empty - end - end - - it 'does not return ancestors of authorized groups' do - request_organization - - expect(groups.pluck('id')).not_to include(parent_group.to_global_id.to_s) - end - context 'when requesting organization user' do let(:organization_fields) do <<~FIELDS @@ -102,13 +67,13 @@ RSpec.describe 'getting organization information', feature_category: :cell do it 'returns correct organization user fields' do request_organization - organization_user_node = graphql_data_at(:organization, :organizationUsers, :nodes).first + organization_user_nodes = graphql_data_at(:organization, :organizationUsers, :nodes) expected_attributes = { "badges" => [{ "text" => "It's you!", "variant" => 'muted' }], "id" => organization_user.to_global_id.to_s, "user" => { "id" => user.to_global_id.to_s } } - expect(organization_user_node).to match(expected_attributes) + expect(organization_user_nodes).to include(expected_attributes) end it 'avoids N+1 queries for all the fields' do @@ -116,6 +81,8 @@ RSpec.describe 'getting organization information', feature_category: :cell do organization_user_2 = create(:organization_user, organization: organization) other_group.add_developer(organization_user_2.user) + organization_user_from_project = create(:organization_user, organization: organization) + project.add_developer(organization_user_from_project.user) expect { run_query }.not_to exceed_query_limit(base_query_count) end @@ -127,62 +94,144 @@ RSpec.describe 'getting organization information', feature_category: :cell do end end - context 'with `search` argument' do - let(:search) { 'oth' } - let(:organization_fields) do - <<~FIELDS - id - path - groups(search: "#{search}") { - nodes { - id - name - } - } - FIELDS + context 'when requesting groups' do + let(:groups) { graphql_data_at(:organization, :groups, :nodes) } + let_it_be(:parent_group) { create(:group, name: 'parent-group', organization: organization) } + let_it_be(:public_group) do + create(:group, name: 'public-group', parent: parent_group, organization: organization) end - it 'filters groups by name' do - request_organization + let_it_be(:private_group) do + create(:group, :private, name: 'private-group', organization: organization) + end - expect(groups).to contain_exactly(a_graphql_entity_for(other_group)) + before_all do + create(:group, :private, name: 'no-access', organization: organization) + private_group.add_developer(user) + public_group.add_developer(user) + create(:group) { |g| g.add_developer(user) } # outside organization end - end - context 'with `sort` argument' do - using RSpec::Parameterized::TableSyntax + context 'when resolve_organization_groups feature flag is disabled' do + before do + stub_feature_flags(resolve_organization_groups: false) + end + + it 'returns no groups' do + request_organization + + expect(graphql_data_at(:organization)).not_to be_nil + expect(graphql_data_at(:organization, :groups, :nodes)).to be_empty + end + end - let(:authorized_groups) { [public_group, private_group, other_group] } + it 'does not return ancestors of authorized groups' do + request_organization - where(:field, :direction, :sorted_groups) do - 'id' | 'asc' | lazy { authorized_groups.sort_by(&:id) } - 'id' | 'desc' | lazy { authorized_groups.sort_by(&:id).reverse } - 'name' | 'asc' | lazy { authorized_groups.sort_by(&:name) } - 'name' | 'desc' | lazy { authorized_groups.sort_by(&:name).reverse } - 'path' | 'asc' | lazy { authorized_groups.sort_by(&:path) } - 'path' | 'desc' | lazy { authorized_groups.sort_by(&:path).reverse } + expect(groups.pluck('id')).not_to include(parent_group.to_global_id.to_s) end - with_them do - let(:sort) { "#{field}_#{direction}".upcase } + context 'with `search` argument' do + let(:search) { 'oth' } let(:organization_fields) do <<~FIELDS id path - groups(sort: #{sort}) { + groups(search: "#{search}") { nodes { id + name } } FIELDS end - it 'sorts the groups' do + it 'filters groups by name' do request_organization - expect(groups.pluck('id')).to eq(sorted_groups.map(&:to_global_id).map(&:to_s)) + expect(groups).to contain_exactly(a_graphql_entity_for(other_group)) end end + + context 'with `sort` argument' do + using RSpec::Parameterized::TableSyntax + + let(:authorized_groups) { [public_group, private_group, other_group] } + + where(:field, :direction, :sorted_groups) do + 'id' | 'asc' | lazy { authorized_groups.sort_by(&:id) } + 'id' | 'desc' | lazy { authorized_groups.sort_by(&:id).reverse } + 'name' | 'asc' | lazy { authorized_groups.sort_by(&:name) } + 'name' | 'desc' | lazy { authorized_groups.sort_by(&:name).reverse } + 'path' | 'asc' | lazy { authorized_groups.sort_by(&:path) } + 'path' | 'desc' | lazy { authorized_groups.sort_by(&:path).reverse } + end + + with_them do + let(:sort) { "#{field}_#{direction}".upcase } + let(:organization_fields) do + <<~FIELDS + id + path + groups(sort: #{sort}) { + nodes { + id + } + } + FIELDS + end + + it 'sorts the groups' do + request_organization + + expect(groups.pluck('id')).to eq(sorted_groups.map(&:to_global_id).map(&:to_s)) + end + end + end + end + + context 'when requesting projects' do + let(:projects) { graphql_data_at(:organization, :projects, :nodes) } + let(:organization_fields) do + <<~FIELDS + projects { + nodes { + id + } + } + FIELDS + end + + before_all do + create(:project) { |p| p.add_developer(user) } # some other project that shouldn't show up in our results + end + + before do + request_organization + end + + it_behaves_like 'a working graphql query' + + it 'returns projects' do + expect(projects).to contain_exactly(a_graphql_entity_for(project)) + end + + it_behaves_like 'sorted paginated query' do + include_context 'no sort argument' + + let_it_be(:another_project) { create(:project, organization: organization) { |p| p.add_developer(user) } } + let_it_be(:another_project2) { create(:project, organization: organization) { |p| p.add_developer(user) } } + let(:first_param) { 2 } + let(:data_path) { [:organization, :projects] } + let(:all_records) { [another_project2, another_project, project].map { |p| global_id_of(p).to_s } } + end + + def pagination_query(params) + graphql_query_for( + :organization, { id: organization.to_global_id }, + query_nodes(:projects, :id, include_pagination_info: true, args: params) + ) + end end end end diff --git a/spec/requests/api/graphql/organizations/organizations_query_spec.rb b/spec/requests/api/graphql/organizations/organizations_query_spec.rb new file mode 100644 index 00000000000..12d81ed7412 --- /dev/null +++ b/spec/requests/api/graphql/organizations/organizations_query_spec.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting organizations information', feature_category: :cell do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + + let(:query) { graphql_query_for(:organizations, organizations_fields) } + let(:organizations) { graphql_data_at(:organizations, :nodes) } + let(:organizations_fields) do + <<~FIELDS + nodes { + id + path + } + FIELDS + end + + before_all { create_list(:organization, 3) } + + subject(:request_organization) { post_graphql(query, current_user: current_user) } + + context 'without authenticated user' do + let(:current_user) { nil } + + it_behaves_like 'a working graphql query' do + before do + request_organization + end + end + end + + context 'with authenticated user' do + let(:current_user) { user } + + it_behaves_like 'a working graphql query' do + before do + request_organization + end + end + + it_behaves_like 'sorted paginated query' do + include_context 'no sort argument' + + let(:first_param) { 2 } + let(:data_path) { [:organizations] } + let(:all_records) { Organizations::Organization.order(id: :desc).map { |o| global_id_of(o).to_s } } + end + + def pagination_query(params) + graphql_query_for(:organizations, params, "#{page_info} nodes { id }") + end + end +end diff --git a/spec/requests/api/graphql/project/container_repositories_spec.rb b/spec/requests/api/graphql/project/container_repositories_spec.rb index c86d3bdd14c..2307409c383 100644 --- a/spec/requests/api/graphql/project/container_repositories_spec.rb +++ b/spec/requests/api/graphql/project/container_repositories_spec.rb @@ -12,7 +12,7 @@ RSpec.describe 'getting container repositories in a project', feature_category: let_it_be(:container_repositories) { [container_repository, container_repositories_delete_scheduled, container_repositories_delete_failed].flatten } let_it_be(:container_expiration_policy) { project.container_expiration_policy } - let(:excluded_fields) { %w[pipeline jobs productAnalyticsState] } + let(:excluded_fields) { %w[pipeline jobs productAnalyticsState mlModels] } let(:container_repositories_fields) do <<~GQL edges { @@ -155,7 +155,7 @@ RSpec.describe 'getting container repositories in a project', feature_category: it_behaves_like 'handling graphql network errors with the container registry' it_behaves_like 'not hitting graphql network errors with the container registry' do - let(:excluded_fields) { %w[pipeline jobs tags tagsCount productAnalyticsState] } + let(:excluded_fields) { %w[pipeline jobs tags tagsCount productAnalyticsState mlModels] } end it 'returns the total count of container repositories' do diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index 23be9fa5286..96933505838 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -24,7 +24,7 @@ RSpec.describe 'getting merge request information nested in a project', feature_ # we exclude Project.pipeline because it needs arguments, # codequalityReportsComparer because it is behind a feature flag # and runners because the user is not an admin and therefore has no access - let(:excluded) { %w[jobs pipeline runners codequalityReportsComparer] } + let(:excluded) { %w[jobs pipeline runners codequalityReportsComparer mlModels] } let(:mr_fields) { all_graphql_fields_for('MergeRequest', excluded: excluded) } before do diff --git a/spec/requests/api/graphql/project/tree/tree_spec.rb b/spec/requests/api/graphql/project/tree/tree_spec.rb index 77b72bf39a1..d71908d6458 100644 --- a/spec/requests/api/graphql/project/tree/tree_spec.rb +++ b/spec/requests/api/graphql/project/tree/tree_spec.rb @@ -167,6 +167,8 @@ RSpec.describe 'getting a tree in a project', feature_category: :source_code_man end context 'when the ref points to a SSH-signed commit' do + let_it_be(:project) { create(:project, :repository, :in_group) } + let_it_be(:ref) { 'ssh-signed-commit' } let_it_be(:commit) { project.commit(ref) } let_it_be(:current_user) { create(:user, email: commit.committer_email).tap { |user| project.add_owner(user) } } diff --git a/spec/requests/api/graphql/projects/projects_spec.rb b/spec/requests/api/graphql/projects/projects_spec.rb index 84b8c2285f0..dfebcb7c42c 100644 --- a/spec/requests/api/graphql/projects/projects_spec.rb +++ b/spec/requests/api/graphql/projects/projects_spec.rb @@ -45,14 +45,14 @@ RSpec.describe 'getting a collection of projects', feature_category: :source_cod it 'avoids N+1 queries', :use_sql_query_cache, :clean_gitlab_redis_cache do post_graphql(single_project_query, current_user: current_user) - query_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do post_graphql(single_project_query, current_user: current_user) - end.count + end # There is an N+1 query for max_member_access_for_user_ids expect do post_graphql(query, current_user: current_user) - end.not_to exceed_all_query_limit(query_count + 5) + end.not_to exceed_all_query_limit(control).with_threshold(5) end it 'returns the expected projects' do diff --git a/spec/requests/api/graphql/user/user_achievements_query_spec.rb b/spec/requests/api/graphql/user/user_achievements_query_spec.rb index 2e6c3dcba61..ccff5bdf919 100644 --- a/spec/requests/api/graphql/user/user_achievements_query_spec.rb +++ b/spec/requests/api/graphql/user/user_achievements_query_spec.rb @@ -60,14 +60,14 @@ RSpec.describe 'UserAchievements', feature_category: :user_profile do end it 'can lookahead to eliminate N+1 queries', :use_clean_rails_memory_store_caching do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do post_graphql(query, current_user: user) - end.count + end achievement2 = create(:achievement, namespace: group) create_list(:user_achievement, 2, achievement: achievement2, user: user) - expect { post_graphql(query, current_user: user) }.not_to exceed_all_query_limit(control_count) + expect { post_graphql(query, current_user: user) }.not_to exceed_all_query_limit(control) end context 'when the achievements feature flag is disabled for a namespace' do diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index fe77b7ae736..c6d44b057a7 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -199,7 +199,7 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do it 'avoids N+1 queries' do post_graphql(query, current_user: current_user) # warm up - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do post_graphql(query, current_user: current_user) end @@ -207,7 +207,7 @@ RSpec.describe 'Query.work_item(id)', feature_category: :team_planning do expect do post_graphql(query, current_user: current_user) - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end context 'when user is guest' do diff --git a/spec/requests/api/group_milestones_spec.rb b/spec/requests/api/group_milestones_spec.rb index 82a4311f7d0..7b4075b3aeb 100644 --- a/spec/requests/api/group_milestones_spec.rb +++ b/spec/requests/api/group_milestones_spec.rb @@ -141,11 +141,11 @@ RSpec.describe API::GroupMilestones, feature_category: :team_planning do it 'returns multiple issues without performing N + 1' do perform_request - control_count = ActiveRecord::QueryRecorder.new { perform_request }.count + control = ActiveRecord::QueryRecorder.new { perform_request } create(:issue, project: project, milestone: milestone) - expect { perform_request }.not_to exceed_query_limit(control_count) + expect { perform_request }.not_to exceed_query_limit(control) end end diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 327dfd0a76b..6b949962e53 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -660,24 +660,24 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do get api("/groups/#{group1.id}", user1) expect(response).to have_gitlab_http_status(:ok) - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do get api("/groups/#{group1.id}", user1) - end.count + end create(:project, namespace: group1) expect do get api("/groups/#{group1.id}", user1) - end.not_to exceed_query_limit(control_count) + end.not_to exceed_query_limit(control) end it 'avoids N+1 queries with shared group links' do # setup at least 1 shared group, so that we record the queries that preload the nested associations too. create(:group_group_link, shared_group: group1, shared_with_group: create(:group)) - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do get api("/groups/#{group1.id}", user1) - end.count + end # setup "n" more shared groups create(:group_group_link, shared_group: group1, shared_with_group: create(:group)) @@ -686,7 +686,7 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do # test that no of queries for 1 shared group is same as for n shared groups expect do get api("/groups/#{group1.id}", user1) - end.not_to exceed_query_limit(control_count) + end.not_to exceed_query_limit(control) end end @@ -1364,15 +1364,15 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do get api("/groups/#{group1.id}/projects", user1) expect(response).to have_gitlab_http_status(:ok) - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do get api("/groups/#{group1.id}/projects", user1) - end.count + end create(:project, namespace: group1) expect do get api("/groups/#{group1.id}/projects", user1) - end.not_to exceed_query_limit(control_count) + end.not_to exceed_query_limit(control) end end @@ -1563,15 +1563,15 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do subject expect(response).to have_gitlab_http_status(:ok) - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do subject - end.count + end create(:project_group_link, project: create(:project), group: group1) expect do subject - end.not_to exceed_query_limit(control_count) + end.not_to exceed_query_limit(control) end end @@ -1937,6 +1937,59 @@ RSpec.describe API::Groups, feature_category: :groups_and_projects do end end + context 'when group is within a provided organization' do + let_it_be(:organization) { create(:organization) } + + context 'when user is an organization user' do + before_all do + create(:organization_user, user: user3, organization: organization) + end + + it 'creates group within organization' do + post api('/groups', user3), params: attributes_for_group_api(organization_id: organization.id) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['organization_id']).to eq(organization.id) + end + + context 'when parent_group is not part of the organization' do + it 'does not create the group with not_found' do + post( + api('/groups', user3), + params: attributes_for_group_api(parent_id: group2.id, organization_id: organization.id) + ) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + + context 'when organization does not exist' do + it 'does not create the group with not_found' do + post api('/groups', user3), params: attributes_for_group_api(organization_id: non_existing_record_id) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user is not an organization user' do + it 'does not create the group' do + post api('/groups', user3), params: attributes_for_group_api(organization_id: organization.id) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when user is an admin' do + it 'creates group within organization' do + post api('/groups', admin, admin_mode: true), params: attributes_for_group_api(organization_id: organization.id) + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['organization_id']).to eq(organization.id) + end + end + end + context "when authenticated as user with group permissions" do it "creates group", :aggregate_failures do group = attributes_for_group_api request_access_enabled: false diff --git a/spec/requests/api/import_bitbucket_server_spec.rb b/spec/requests/api/import_bitbucket_server_spec.rb index 9a9ccc867a3..4f838be1c81 100644 --- a/spec/requests/api/import_bitbucket_server_spec.rb +++ b/spec/requests/api/import_bitbucket_server_spec.rb @@ -128,7 +128,7 @@ RSpec.describe API::ImportBitbucketServer, feature_category: :importers do .to receive(:new).with(project_key, repo_slug, anything, project.name, user.namespace, user, anything, timeout_strategy) .and_return(double(execute: project)) - allow(Gitlab::UrlBlocker) + allow(Gitlab::HTTP_V2::UrlBlocker) .to receive(:blocked_url?) .and_return(true) post api("/import/bitbucket_server", user), params: { diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb index f555f39ff74..532492c9c2c 100644 --- a/spec/requests/api/import_github_spec.rb +++ b/spec/requests/api/import_github_spec.rb @@ -20,11 +20,18 @@ RSpec.describe API::ImportGithub, feature_category: :importers do } end + let(:headers) do + { + 'x-oauth-scopes' => 'read:org' + } + end + let(:client) { double('client', user: provider_user, repository: provider_repo) } before do Grape::Endpoint.before_each do |endpoint| allow(endpoint).to receive(:client).and_return(client) + allow(client).to receive_message_chain(:octokit, :last_response, :headers).and_return(headers) end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index e59633b6d35..87f3ee640f3 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -1437,7 +1437,7 @@ RSpec.describe API::Internal::Base, feature_category: :system_access do end let(:changes) do - "#{Gitlab::Git::BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{branch_name}" + "#{Gitlab::Git::SHA1_BLANK_SHA} 570e7b2abdd848b95f2f578043fc23bd6f6fd24d refs/heads/#{branch_name}" end subject { post api('/internal/post_receive'), params: valid_params, headers: gitlab_shell_internal_api_request_header } diff --git a/spec/requests/api/internal/kubernetes_spec.rb b/spec/requests/api/internal/kubernetes_spec.rb index 5ef041881b9..7934fa4a358 100644 --- a/spec/requests/api/internal/kubernetes_spec.rb +++ b/spec/requests/api/internal/kubernetes_spec.rb @@ -80,7 +80,7 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme it 'returns no_content for valid events' do counters = { gitops_sync: 10, k8s_api_proxy_request: 5 } - unique_counters = { agent_users_using_ci_tunnel: [10] } + unique_counters = { k8s_api_proxy_requests_unique_users_via_ci_access: [10] } send_request(params: { counters: counters, unique_counters: unique_counters }) @@ -89,7 +89,7 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme it 'returns no_content for counts of zero' do counters = { gitops_sync: 0, k8s_api_proxy_request: 0 } - unique_counters = { agent_users_using_ci_tunnel: [] } + unique_counters = { k8s_api_proxy_requests_unique_users_via_ci_access: [] } send_request(params: { counters: counters, unique_counters: unique_counters }) @@ -105,7 +105,7 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme end it 'returns 400 for non unique_counter set' do - unique_counters = { agent_users_using_ci_tunnel: 1 } + unique_counters = { k8s_api_proxy_requests_unique_users_via_ci_access: 1 } send_request(params: { unique_counters: unique_counters }) @@ -125,7 +125,6 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme users = create_list(:user, 3) user_ids = users.map(&:id) << users[0].id unique_counters = { - agent_users_using_ci_tunnel: user_ids, k8s_api_proxy_requests_unique_users_via_ci_access: user_ids, k8s_api_proxy_requests_unique_agents_via_ci_access: user_ids, k8s_api_proxy_requests_unique_users_via_user_access: user_ids, @@ -191,6 +190,7 @@ RSpec.describe API::Internal::Kubernetes, feature_category: :deployment_manageme end it 'tracks events and returns no_content', :aggregate_failures do + events[:agent_users_using_ci_tunnel] = events.values.flatten events.each do |event_name, event_list| event_list.each do |event| expect(Gitlab::InternalEvents).to receive(:track_event) diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index dc02e830027..60f3c4780eb 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -412,7 +412,7 @@ RSpec.describe API::Invitations, feature_category: :user_profile do expect do post invitations_url(project, maintainer), params: { email: emails, access_level: Member::DEVELOPER } - end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end.not_to exceed_all_query_limit(control).with_threshold(unresolved_n_plus_ones) end it 'does not exceed expected queries count for user_ids', :request_store, :use_sql_query_cache do @@ -430,7 +430,7 @@ RSpec.describe API::Invitations, feature_category: :user_profile do expect do post invitations_url(project, maintainer), params: { user_id: users.map(&:id).join(','), access_level: Member::DEVELOPER } - end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end.not_to exceed_all_query_limit(control).with_threshold(unresolved_n_plus_ones) end it 'does not exceed expected queries count with secondary emails', :request_store, :use_sql_query_cache do @@ -453,7 +453,7 @@ RSpec.describe API::Invitations, feature_category: :user_profile do expect do post invitations_url(project, maintainer), params: { email: emails, access_level: Member::DEVELOPER } - end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end.not_to exceed_all_query_limit(control).with_threshold(unresolved_n_plus_ones) end end @@ -491,7 +491,7 @@ RSpec.describe API::Invitations, feature_category: :user_profile do expect do post invitations_url(group, maintainer), params: { email: emails, access_level: Member::DEVELOPER } - end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end.not_to exceed_all_query_limit(control).with_threshold(unresolved_n_plus_ones) end it 'does not exceed expected queries count for secondary emails', :request_store, :use_sql_query_cache do @@ -514,7 +514,7 @@ RSpec.describe API::Invitations, feature_category: :user_profile do expect do post invitations_url(group, maintainer), params: { email: emails, access_level: Member::DEVELOPER } - end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end.not_to exceed_all_query_limit(control).with_threshold(unresolved_n_plus_ones) end end diff --git a/spec/requests/api/issue_links_spec.rb b/spec/requests/api/issue_links_spec.rb index fcb199a91a4..a4a9eca92b9 100644 --- a/spec/requests/api/issue_links_spec.rb +++ b/spec/requests/api/issue_links_spec.rb @@ -40,11 +40,11 @@ RSpec.describe API::IssueLinks, feature_category: :team_planning do it 'returns multiple links without N + 1' do perform_request(user) - control_count = ActiveRecord::QueryRecorder.new { perform_request(user) }.count + control = ActiveRecord::QueryRecorder.new { perform_request(user) } create(:issue_link, source: issue, target: create(:issue, project: project)) - expect { perform_request(user) }.not_to exceed_query_limit(control_count) + expect { perform_request(user) }.not_to exceed_query_limit(control) end end end diff --git a/spec/requests/api/issues/get_project_issues_spec.rb b/spec/requests/api/issues/get_project_issues_spec.rb index 9e54ec08486..6719297f54f 100644 --- a/spec/requests/api/issues/get_project_issues_spec.rb +++ b/spec/requests/api/issues/get_project_issues_spec.rb @@ -233,9 +233,9 @@ RSpec.describe API::Issues, feature_category: :team_planning do issues = create_list(:issue, 3, project: project, closed_by: user) - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/projects/#{project.id}/issues", user) - end.count + end milestone = create(:milestone, project: project) create(:issue, project: project, milestone: milestone, closed_by: create(:user)) @@ -245,7 +245,7 @@ RSpec.describe API::Issues, feature_category: :team_planning do expect do get api("/projects/#{project.id}/issues", user) - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end it 'returns 404 when project does not exist' do @@ -361,9 +361,9 @@ RSpec.describe API::Issues, feature_category: :team_planning do let(:label_c) { create(:label, title: 'bar', project: project) } it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/projects/#{project.id}/issues?with_labels_details=true", user) - end.count + end new_issue = create(:issue, project: project) create(:label_link, label: label, target: new_issue) @@ -372,7 +372,7 @@ RSpec.describe API::Issues, feature_category: :team_planning do expect do get api("/projects/#{project.id}/issues?with_labels_details=true", user) - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end end diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index 2110e4a077d..5e432cfca74 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -1054,10 +1054,10 @@ RSpec.describe API::MavenPackages, feature_category: :package_registry do context 'FIPS mode', :fips_mode do it_behaves_like 'package workhorse uploads' - it 'rejects the request for md5 file' do + it 'returns 200 for the request for md5 file' do upload_file_with_token(params: params, file_extension: 'jar.md5') - expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response).to have_gitlab_http_status(:ok) end end @@ -1276,10 +1276,13 @@ RSpec.describe API::MavenPackages, feature_category: :package_registry do end context 'with FIPS mode enabled', :fips_mode do - it 'rejects the request' do + it 'returns an empty body' do + expect_use_primary + subject - expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.body).to eq('') + expect(response).to have_gitlab_http_status(:ok) end end end diff --git a/spec/requests/api/members_spec.rb b/spec/requests/api/members_spec.rb index feb24a4e73f..7fc58140fb6 100644 --- a/spec/requests/api/members_spec.rb +++ b/spec/requests/api/members_spec.rb @@ -717,7 +717,7 @@ RSpec.describe API::Members, feature_category: :groups_and_projects do end.to change { source.members.count }.by(-1) end - it_behaves_like 'rate limited endpoint', rate_limit_key: :member_delete do + it_behaves_like 'rate limited endpoint', rate_limit_key: :members_delete do let(:current_user) { maintainer } let(:another_member) { create(:user) } diff --git a/spec/requests/api/merge_request_approvals_spec.rb b/spec/requests/api/merge_request_approvals_spec.rb index 2de59750273..886fc70edf2 100644 --- a/spec/requests/api/merge_request_approvals_spec.rb +++ b/spec/requests/api/merge_request_approvals_spec.rb @@ -117,6 +117,18 @@ RSpec.describe API::MergeRequestApprovals, feature_category: :source_code_manage end context 'for a bot user' do + context 'when the MR is merged' do + let(:merge_request) { create(:merge_request, :merged, :simple, author: user, source_project: project) } + + it 'returns 401' do + put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/reset_approvals", bot) + + merge_request.reload + expect(response).to have_gitlab_http_status(:unauthorized) + expect(merge_request.approvals.pluck(:user_id)).to contain_exactly(user2.id) + end + end + it 'clears approvals of the merge_request' do put api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/reset_approvals", bot) diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 6000fa29dc4..6ba51080bf0 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -193,7 +193,7 @@ RSpec.describe API::MergeRequests, :aggregate_failures, feature_category: :sourc control = ActiveRecord::QueryRecorder.new do get api(path, user) - end.count + end mr = create(:merge_request) create(:label_link, label: label, target: mr) @@ -1232,7 +1232,7 @@ RSpec.describe API::MergeRequests, :aggregate_failures, feature_category: :sourc it 'avoids N+1 queries', quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/330335' do control = ActiveRecord::QueryRecorder.new do get api("/projects/#{project.id}/merge_requests", user) - end.count + end create(:merge_request, author: user, assignees: [user], source_project: project, target_project: project, created_at: base_time) diff --git a/spec/requests/api/ml/mlflow/experiments_spec.rb b/spec/requests/api/ml/mlflow/experiments_spec.rb index 409b4529699..ac2d5539408 100644 --- a/spec/requests/api/ml/mlflow/experiments_spec.rb +++ b/spec/requests/api/ml/mlflow/experiments_spec.rb @@ -207,4 +207,81 @@ RSpec.describe API::Ml::Mlflow::Experiments, feature_category: :mlops do it_behaves_like 'MLflow|Bad Request on missing required', [:key, :value] end end + + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/search' do + let_it_be(:experiment_b) do + create(:ml_experiments, project: project, name: "#{experiment.name}_2") + end + + let_it_be(:experiment_c) do + create(:ml_experiments, project: project, name: "#{experiment.name}_1") + end + + let(:order_by) { nil } + let(:default_params) do + { + 'max_results' => 2, + 'order_by' => order_by + } + end + + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/search" } + let(:request) { post api(route), params: default_params.merge(**params), headers: headers } + + it 'returns all the models', :aggregate_failures do + is_expected.to have_gitlab_http_status(:ok) + is_expected.to match_response_schema('ml/search_experiments') + expect(json_response["experiments"].count).to be(2) + end + + describe 'pagination and ordering' do + RSpec.shared_examples 'a paginated search experiments request with order' do + it 'paginates respecting the provided order by' do + first_page_experiments = json_response['experiments'] + expect(first_page_experiments.size).to eq(2) + + expect(first_page_experiments[0]['experiment_id'].to_i).to eq(expected_order[0].iid) + expect(first_page_experiments[1]['experiment_id'].to_i).to eq(expected_order[1].iid) + + params = default_params.merge(page_token: json_response['next_page_token']) + + post api(route), params: params, headers: headers + + second_page_response = Gitlab::Json.parse(response.body) + second_page_experiments = second_page_response['experiments'] + + expect(second_page_response['next_page_token']).to be_nil + expect(second_page_experiments.size).to eq(1) + expect(second_page_experiments[0]['experiment_id'].to_i).to eq(expected_order[2].iid) + end + end + + let(:default_order) { [experiment_c, experiment_b, experiment] } + + context 'when ordering is not provided' do + let(:expected_order) { default_order } + + it_behaves_like 'a paginated search experiments request with order' + end + + context 'when order by column is provided', 'and column exists' do + let(:order_by) { 'name ASC' } + let(:expected_order) { [experiment, experiment_c, experiment_b] } + + it_behaves_like 'a paginated search experiments request with order' + end + + context 'when order by column is provided', 'and column does not exist' do + let(:order_by) { 'something DESC' } + let(:expected_order) { default_order } + + it_behaves_like 'a paginated search experiments request with order' + end + end + + describe 'Error States' do + it_behaves_like 'MLflow|shared error cases' + it_behaves_like 'MLflow|Requires api scope and write permission' + end + end end diff --git a/spec/requests/api/ml/mlflow/model_versions_spec.rb b/spec/requests/api/ml/mlflow/model_versions_spec.rb index e62bccf1507..812044651af 100644 --- a/spec/requests/api/ml/mlflow/model_versions_spec.rb +++ b/spec/requests/api/ml/mlflow/model_versions_spec.rb @@ -157,6 +157,23 @@ RSpec.describe API::Ml::Mlflow::ModelVersions, feature_category: :mlops do expect(json_response["model_version"]["version"]).to eq('2.0.0') end + describe 'user assigned version' do + let(:params) do + { + 'name' => model_name, + 'description' => 'description-text', + 'tags' => [{ 'key' => 'gitlab.version', 'value' => '1.2.3' }] + } + end + + it 'assigns the supplied version string via the gitlab tag' do + is_expected.to have_gitlab_http_status(:ok) + expect(json_response["model_version"]["version"]).to eq('1.2.3') + expect(json_response["model_version"]["tags"]).to match_array([{ "key" => 'gitlab.version', + "value" => '1.2.3' }]) + end + end + describe 'Error States' do context 'when has access' do context 'and model does not exist' do @@ -164,6 +181,30 @@ RSpec.describe API::Ml::Mlflow::ModelVersions, feature_category: :mlops do it_behaves_like 'MLflow|Not Found - Resource Does Not Exist' end + + # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731 + context 'when a duplicate tag name is supplied' do + let(:params) do + { name: model_name, tags: [{ key: 'key1', value: 'value1' }, { key: 'key1', value: 'value2' }] } + end + + it "returns a validation error", :aggregate_failures do + expect(json_response).to include({ 'error_code' => 'INVALID_PARAMETER_VALUE' }) + expect(model.metadata.count).to be 0 + end + end + + # TODO: Ensure consisted error responses https://gitlab.com/gitlab-org/gitlab/-/issues/429731 + context 'when an empty tag name is supplied' do + let(:params) do + { name: model_name, tags: [{ key: '', value: 'value1' }, { key: 'key1', value: 'value2' }] } + end + + it "returns a validation error", :aggregate_failures do + expect(json_response).to include({ 'error_code' => 'INVALID_PARAMETER_VALUE' }) + expect(model.metadata.count).to be 0 + end + end end it_behaves_like 'MLflow|an authenticated resource' diff --git a/spec/requests/api/namespaces_spec.rb b/spec/requests/api/namespaces_spec.rb index 5fd41013b25..2320b3be0c1 100644 --- a/spec/requests/api/namespaces_spec.rb +++ b/spec/requests/api/namespaces_spec.rb @@ -109,6 +109,19 @@ RSpec.describe API::Namespaces, :aggregate_failures, feature_category: :groups_a expect(json_response.map { |resource| resource['id'] }).to match_array([user.namespace_id, group2.id]) end end + + context 'with top_level_only param' do + it 'returns only top level groups' do + group1.add_owner(user) + group2.add_owner(user) + + get api("/namespaces?top_level_only=true", user) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response.map { |resource| resource['id'] }).to match_array([user.namespace_id, group1.id]) + end + end end end diff --git a/spec/requests/api/pages_domains_spec.rb b/spec/requests/api/pages_domains_spec.rb index 42d83ff8139..27d69f1aa03 100644 --- a/spec/requests/api/pages_domains_spec.rb +++ b/spec/requests/api/pages_domains_spec.rb @@ -31,7 +31,7 @@ RSpec.describe API::PagesDomains, feature_category: :pages do let(:route_letsencrypt_domain) { "/projects/#{project.id}/pages/domains/#{pages_domain_with_letsencrypt.domain}" } before do - allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + stub_pages_setting(enabled: true) end describe 'GET /pages/domains' do diff --git a/spec/requests/api/pages_spec.rb b/spec/requests/api/pages_spec.rb new file mode 100644 index 00000000000..23ffeb143cb --- /dev/null +++ b/spec/requests/api/pages_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe API::Pages, feature_category: :pages do + let_it_be(:project) { create(:project) } + let_it_be(:admin) { create(:admin) } + + let(:user) { create(:user) } + + before do + stub_pages_setting(enabled: true) + + create( + :project_setting, + project: project, + pages_unique_domain_enabled: true, + pages_unique_domain: 'unique-domain') + end + + context "when get pages setting endpoint" do + let(:user) { create(:user) } + + it "returns the :ok for project maintainers (and above)" do + project.add_maintainer(user) + + get api("/projects/#{project.id}/pages", user) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "returns the :forbidden for project developers (and below)" do + project.add_developer(user) + + get api("/projects/#{project.id}/pages", user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + + context "when the pages feature is disabled" do + it "returns the :not_found when user is not in the project" do + project.project_feature.update!(pages_access_level: 0) + + get api("/projects/#{project.id}/pages", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context "when the project has pages deployments", :time_freeze, :aggregate_failures do + let_it_be(:created_at) { Time.now.utc } + + before_all do + create(:pages_deployment, path_prefix: '/foo', project: project, created_at: created_at) + create(:pages_deployment, project: project, created_at: created_at) + + # this one is here to ensure the endpoint don't return "inactive" deployments + create( + :pages_deployment, + path_prefix: '/bar', + project: project, + created_at: created_at, + deleted_at: 5.minutes.from_now) + end + + it "return the right data" do + project.add_owner(user) + + get api("/projects/#{project.id}/pages", user) + + expect(json_response["force_https"]).to eq(false) + expect(json_response["is_unique_domain_enabled"]).to eq(true) + expect(json_response["url"]).to eq("http://unique-domain.example.com") + expect(json_response["deployments"]).to match_array([ + { + "created_at" => created_at.strftime('%Y-%m-%dT%H:%M:%S.%3LZ'), + "path_prefix" => "/foo", + "root_directory" => "public", + "url" => "http://unique-domain.example.com/foo" + }, + { + "created_at" => created_at.strftime('%Y-%m-%dT%H:%M:%S.%3LZ'), + "path_prefix" => nil, + "root_directory" => "public", + "url" => "http://unique-domain.example.com/" + } + ]) + end + end + end +end diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index 7797e8e9402..c9bba26524c 100644 --- a/spec/requests/api/project_container_repositories_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -65,7 +65,6 @@ RSpec.describe API::ProjectContainerRepositories, feature_category: :container_r shared_context 'using job token' do before do stub_exclusive_lease - stub_feature_flags(ci_job_token_scope: true) end subject { public_send(method, api(url), params: params.merge({ job_token: job.token })) } @@ -74,29 +73,15 @@ RSpec.describe API::ProjectContainerRepositories, feature_category: :container_r shared_context 'using job token from another project' do before do stub_exclusive_lease - stub_feature_flags(ci_job_token_scope: true) end subject { public_send(method, api(url), params: { job_token: job2.token }) } end - shared_context 'using job token while ci_job_token_scope feature flag is disabled' do - before do - stub_exclusive_lease - stub_feature_flags(ci_job_token_scope: false) - end - - subject { public_send(method, api(url), params: params.merge({ job_token: job.token })) } - end - shared_examples 'rejected job token scopes' do include_context 'using job token from another project' do it_behaves_like 'rejected container repository access', :maintainer, :forbidden end - - include_context 'using job token while ci_job_token_scope feature flag is disabled' do - it_behaves_like 'rejected container repository access', :maintainer, :forbidden - end end describe 'GET /projects/:id/registry/repositories' do diff --git a/spec/requests/api/project_import_spec.rb b/spec/requests/api/project_import_spec.rb index 49471b98eba..a73f3366dcb 100644 --- a/spec/requests/api/project_import_spec.rb +++ b/spec/requests/api/project_import_spec.rb @@ -62,9 +62,9 @@ RSpec.describe API::ProjectImport, :aggregate_failures, feature_category: :impor it_behaves_like 'requires import source to be enabled' it 'executes a limited number of queries', :use_clean_rails_redis_caching do - control_count = ActiveRecord::QueryRecorder.new { subject }.count + control = ActiveRecord::QueryRecorder.new { subject } - expect(control_count).to be <= 111 + expect(control.count).to be <= 111 end it 'schedules an import using a namespace' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index b8e029385e3..cf6152a9b67 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1152,7 +1152,7 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect do request - end.not_to exceed_all_query_limit(control.count) + end.not_to exceed_all_query_limit(control) end end @@ -3799,7 +3799,7 @@ RSpec.describe API::Projects, :aggregate_failures, feature_category: :groups_and expect do post api("/projects/#{project.id}/import_project_members/#{measure_project.id}", user) - end.not_to exceed_all_query_limit(control.count).with_threshold(unresolved_n_plus_ones) + end.not_to exceed_all_query_limit(control).with_threshold(unresolved_n_plus_ones) end it 'returns 200 when it successfully imports members from another project' do diff --git a/spec/requests/api/releases_spec.rb b/spec/requests/api/releases_spec.rb index 493dc4e72c6..0c811a21fb0 100644 --- a/spec/requests/api/releases_spec.rb +++ b/spec/requests/api/releases_spec.rb @@ -156,9 +156,9 @@ RSpec.describe API::Releases, :aggregate_failures, feature_category: :release_or create(:release, :with_evidence, project: project, tag: 'v0.1', author: maintainer) create(:release_link, release: project.releases.first) - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api("/projects/#{project.id}/releases", maintainer) - end.count + end create_list(:release, 2, :with_evidence, project: project, author: maintainer) create_list(:release, 2, project: project) @@ -167,7 +167,7 @@ RSpec.describe API::Releases, :aggregate_failures, feature_category: :release_or expect do get api("/projects/#{project.id}/releases", maintainer) - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end it 'serializes releases for the first time and read cached data from the second time' do @@ -1715,9 +1715,9 @@ RSpec.describe API::Releases, :aggregate_failures, feature_category: :release_or subject expect(response).to have_gitlab_http_status(:ok) - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do subject - end.count + end subgroups = create_list(:group, 10, parent: group1) projects = create_list(:project, 10, namespace: subgroups[0]) @@ -1725,7 +1725,7 @@ RSpec.describe API::Releases, :aggregate_failures, feature_category: :release_or expect do subject - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end end end diff --git a/spec/requests/api/terraform/modules/v1/packages_spec.rb b/spec/requests/api/terraform/modules/v1/namespace_packages_spec.rb index 949acdb17e1..d655085a30f 100644 --- a/spec/requests/api/terraform/modules/v1/packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/namespace_packages_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe API::Terraform::Modules::V1::Packages, feature_category: :package_registry do +RSpec.describe API::Terraform::Modules::V1::NamespacePackages, feature_category: :package_registry do include_context 'for terraform modules api setup' using RSpec::Parameterized::TableSyntax @@ -10,17 +10,19 @@ RSpec.describe API::Terraform::Modules::V1::Packages, feature_category: :package let(:url) { api("/packages/terraform/modules/v1/#{group.path}/#{package.name}/versions") } let(:headers) { { 'Authorization' => "Bearer #{tokens[:job_token]}" } } - subject { get(url, headers: headers) } + subject(:get_versions) { get(url, headers: headers) } context 'with a conflicting package name' do - let!(:conflicting_package) { create(:terraform_module_package, project: project, name: "conflict-#{package.name}", version: '2.0.0') } + let!(:conflicting_package) do + create(:terraform_module_package, project: project, name: "conflict-#{package.name}", version: '2.0.0') + end before do group.add_developer(user) end it 'returns only one version' do - subject + get_versions expect(json_response['modules'][0]['versions'].size).to eq(1) expect(json_response['modules'][0]['versions'][0]['version']).to eq('1.0.0') @@ -77,14 +79,14 @@ RSpec.describe API::Terraform::Modules::V1::Packages, feature_category: :package end describe 'GET /api/v4/packages/terraform/modules/v1/:module_namespace/:module_name/:module_system/download' do - context 'empty registry' do + context 'with empty registry' do let(:url) { api("/packages/terraform/modules/v1/#{group.path}/module-2/system/download") } let(:headers) { {} } - subject { get(url, headers: headers) } + subject(:get_download) { get(url, headers: headers) } it 'returns not found when there is no module' do - subject + get_download expect(response).to have_gitlab_http_status(:not_found) end @@ -150,14 +152,14 @@ RSpec.describe API::Terraform::Modules::V1::Packages, feature_category: :package end describe 'GET /api/v4/packages/terraform/modules/v1/:module_namespace/:module_name/:module_system' do - context 'empty registry' do + context 'with empty registry' do let(:url) { api("/packages/terraform/modules/v1/#{group.path}/non-existent/system") } let(:headers) { { 'Authorization' => "Bearer #{tokens[:personal_access_token]}" } } - subject { get(url, headers: headers) } + subject(:get_module) { get(url, headers: headers) } it 'returns not found when there is no module' do - subject + get_module expect(response).to have_gitlab_http_status(:not_found) end @@ -221,16 +223,16 @@ RSpec.describe API::Terraform::Modules::V1::Packages, feature_category: :package let(:url) { api("/packages/terraform/modules/v1/#{group.path}/#{package.name}/#{package.version}") } let(:headers) { {} } - subject { get(url, headers: headers) } + subject(:get_module_version) { get(url, headers: headers) } - context 'not found' do + context 'when not found' do let(:url) { api("/packages/terraform/modules/v1/#{group.path}/#{package.name}/2.0.0") } let(:headers) { { 'Authorization' => "Bearer #{tokens[:job_token]}" } } subject { get(url, headers: headers) } it 'returns not found when the specified version is not present in the registry' do - subject + get_module_version expect(response).to have_gitlab_http_status(:not_found) end @@ -343,7 +345,10 @@ RSpec.describe API::Terraform::Modules::V1::Packages, feature_category: :package end describe 'GET /api/v4/packages/terraform/modules/v1/:module_namespace/:module_name/:module_system/:module_version/file' do - let(:url) { api("/packages/terraform/modules/v1/#{group.path}/#{package.name}/#{package.version}/file?token=#{token}") } + let(:url) do + api("/packages/terraform/modules/v1/#{group.path}/#{package.name}/#{package.version}/file?token=#{token}") + end + let(:tokens) do { personal_access_token: ::Gitlab::JWTToken.new.tap { |jwt| jwt['token'] = personal_access_token.id }.encoded, @@ -352,7 +357,7 @@ RSpec.describe API::Terraform::Modules::V1::Packages, feature_category: :package } end - subject { get(url, headers: headers) } + subject(:get_file) { get(url, headers: headers) } context 'with valid namespace' do where(:visibility, :user_role, :member, :token_type, :shared_examples_name, :expected_status) do @@ -414,8 +419,15 @@ RSpec.describe API::Terraform::Modules::V1::Packages, feature_category: :package end context 'with package file pending destruction' do - let_it_be(:package) { create(:package, package_type: :terraform_module, project: project, name: "module-555/pending-destruction", version: '1.0.0') } - let_it_be(:package_file_pending_destruction) { create(:package_file, :pending_destruction, :xml, package: package) } + let_it_be(:package) do + create(:package, package_type: :terraform_module, project: project, name: "module-555/pending-destruction", + version: '1.0.0') + end + + let_it_be(:package_file_pending_destruction) do + create(:package_file, :pending_destruction, :xml, package: package) + end + let_it_be(:package_file) { create(:package_file, :terraform_module, package: package) } let(:token) { tokens[:personal_access_token] } @@ -426,7 +438,7 @@ RSpec.describe API::Terraform::Modules::V1::Packages, feature_category: :package end it 'does not return them' do - subject + get_file expect(response).to have_gitlab_http_status(:ok) expect(response.body).not_to eq(package_file_pending_destruction.file.file.read) diff --git a/spec/requests/api/terraform/modules/v1/project_packages_spec.rb b/spec/requests/api/terraform/modules/v1/project_packages_spec.rb index 1f3b2283d59..70f7ec64d40 100644 --- a/spec/requests/api/terraform/modules/v1/project_packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/project_packages_spec.rb @@ -6,6 +6,18 @@ RSpec.describe API::Terraform::Modules::V1::ProjectPackages, feature_category: : include_context 'for terraform modules api setup' using RSpec::Parameterized::TableSyntax + describe 'GET /api/v4/projects/:project_id/packages/terraform/modules/:module_name/:module_system' do + it_behaves_like 'handling project level terraform module download requests' do + let(:module_version) { nil } + end + end + + describe 'GET /api/v4/projects/:project_id/packages/terraform/modules/:module_name/:module_system/:module_version' do + it_behaves_like 'handling project level terraform module download requests' do + let(:module_version) { package.version } + end + end + describe 'PUT /api/v4/projects/:project_id/packages/terraform/modules/:module_name/:module_system/:module_version/file/authorize' do include_context 'workhorse headers' @@ -91,7 +103,28 @@ RSpec.describe API::Terraform::Modules::V1::ProjectPackages, feature_category: : ) end + shared_examples 'creating a package' do + it 'creates a package' do + expect { api_request } + .to change { project.packages.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) + expect(response).to have_gitlab_http_status(:created) + end + end + + shared_examples 'not creating a package' do |expected_status| + it 'does not create a package' do + expect { api_request } + .to change { project.packages.count }.by(0) + .and change { Packages::PackageFile.count }.by(0) + expect(response).to have_gitlab_http_status(expected_status) + end + end + context 'with valid project' do + let(:user_headers) { { 'PRIVATE-TOKEN' => personal_access_token.token } } + let(:headers) { user_headers.merge(workhorse_headers) } + where(:visibility, :user_role, :member, :token_header, :token_type, :shared_examples_name, :expected_status) do :public | :developer | true | 'PRIVATE-TOKEN' | :personal_access_token | 'process terraform module upload' | :created :public | :guest | true | 'PRIVATE-TOKEN' | :personal_access_token | 'rejects terraform module packages access' | :forbidden @@ -135,7 +168,6 @@ RSpec.describe API::Terraform::Modules::V1::ProjectPackages, feature_category: : with_them do let(:user_headers) { user_role == :anonymous ? {} : { token_header => token } } - let(:headers) { user_headers.merge(workhorse_headers) } let(:snowplow_gitlab_standard_context) do { project: project, namespace: project.namespace, user: snowplow_user, property: 'i_package_terraform_module_user' } @@ -160,43 +192,73 @@ RSpec.describe API::Terraform::Modules::V1::ProjectPackages, feature_category: : end context 'when failed package file save' do - let(:user_headers) { { 'PRIVATE-TOKEN' => personal_access_token.token } } - let(:headers) { user_headers.merge(workhorse_headers) } + before do + project.add_developer(user) + allow(Packages::CreatePackageFileService).to receive(:new).and_raise(StandardError) + end + + it_behaves_like 'not creating a package', :error + end + + context 'with an existing package in the same project' do + let_it_be_with_reload(:existing_package) do + create(:terraform_module_package, name: 'mymodule/mysystem', version: '1.0.0', project: project) + end before do project.add_developer(user) end - it 'does not create package record', :aggregate_failures do - allow(Packages::CreatePackageFileService).to receive(:new).and_raise(StandardError) + it_behaves_like 'not creating a package', :forbidden + + context 'when marked as pending_destruction' do + before do + existing_package.pending_destruction! + end - expect { api_request } - .to change { project.packages.count }.by(0) - .and change { Packages::PackageFile.count }.by(0) - expect(response).to have_gitlab_http_status(:error) + it_behaves_like 'creating a package' end + end + + context 'with existing package in another project' do + let_it_be(:package_settings) { create(:namespace_package_setting, namespace: group) } + let_it_be(:project2) { create(:project, namespace: group) } + let!(:existing_package) { create(:terraform_module_package, name: 'mymodule/mysystem', project: project2) } - context 'with an existing package' do - let_it_be_with_reload(:existing_package) do - create(:terraform_module_package, name: 'mymodule/mysystem', version: '1.0.0', project: project) + before do + project.add_developer(user) + end + + context 'when duplicates not allowed' do + it_behaves_like 'not creating a package', :forbidden + end + + context 'when duplicates allowed' do + before do + package_settings.update_column(:terraform_module_duplicates_allowed, true) end - it 'does not create a new package' do - expect { api_request } - .to change { project.packages.count }.by(0) - .and change { Packages::PackageFile.count }.by(0) - expect(response).to have_gitlab_http_status(:forbidden) + it_behaves_like 'creating a package' + end + + context 'with duplicate regex exception' do + before do + package_settings.update_columns( + terraform_module_duplicates_allowed: false, + terraform_module_duplicate_exception_regex: regex + ) + end + + context 'when regex matches' do + let(:regex) { ".*#{existing_package.name.last(3)}.*" } + + it_behaves_like 'creating a package' end - context 'when marked as pending_destruction' do - it 'does create a new package' do - existing_package.pending_destruction! + context 'when regex does not match' do + let(:regex) { '.*non-matching-regex.*' } - expect { api_request } - .to change { project.packages.count }.by(1) - .and change { Packages::PackageFile.count }.by(1) - expect(response).to have_gitlab_http_status(:created) - end + it_behaves_like 'not creating a package', :forbidden end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 86c4e04ef71..de3460208b7 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -265,9 +265,9 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile end it 'avoids N+1 queries when requested by admin' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api(path, admin) - end.count + end create_list(:user, 3) @@ -277,19 +277,19 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile expect do get api(path, admin) - end.not_to exceed_all_query_limit(control_count + 3) + end.not_to exceed_all_query_limit(control).with_threshold(3) end it 'avoids N+1 queries when requested by a regular user' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get api(path, user) - end.count + end create_list(:user, 3) expect do get api(path, user) - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end end @@ -2272,16 +2272,16 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile it 'avoids N+1 queries' do second_project.add_maintainer(user) - control_count = ActiveRecord::QueryRecorder.new do + control = ActiveRecord::QueryRecorder.new do get api(path, user) - end.count + end deploy_key = create(:deploy_key, user: second_user) create(:deploy_keys_project, project: second_project, deploy_key_id: deploy_key.id) expect do get api(path, user) - end.not_to exceed_query_limit(control_count) + end.not_to exceed_query_limit(control) end end end @@ -2328,15 +2328,15 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile end it 'avoids N+1 queries', :request_store do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do request - end.count + end create_list(:key, 2, user: user) expect do request - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end end end @@ -3044,15 +3044,15 @@ RSpec.describe API::Users, :aggregate_failures, feature_category: :user_profile end it 'avoids N+1 queries', :request_store do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do request - end.count + end create_list(:key, 2, user: user) expect do request - end.not_to exceed_all_query_limit(control_count) + end.not_to exceed_all_query_limit(control) end end diff --git a/spec/requests/application_controller_spec.rb b/spec/requests/application_controller_spec.rb index 52fdf6bc69e..969fdbe9838 100644 --- a/spec/requests/application_controller_spec.rb +++ b/spec/requests/application_controller_spec.rb @@ -5,11 +5,33 @@ require 'spec_helper' RSpec.describe ApplicationController, type: :request, feature_category: :shared do let_it_be(:user) { create(:user) } - before do - sign_in(user) - end - it_behaves_like 'Base action controller' do + before do + sign_in(user) + end + subject(:request) { get root_path } end + + describe 'session expiration' do + context 'when user is authenticated' do + it 'does not set the expire_after option' do + sign_in(user) + + get root_path + + expect(request.env['rack.session.options'][:expire_after]).to be_nil + end + end + + context 'when user is unauthenticated' do + it 'sets the expire_after option' do + get root_path + + expect(request.env['rack.session.options'][:expire_after]).to eq( + Settings.gitlab['unauthenticated_session_expire_delay'] + ) + end + end + end end diff --git a/spec/requests/explore/catalog_controller_spec.rb b/spec/requests/explore/catalog_controller_spec.rb index e75b0bba5a6..59a5dcffd3c 100644 --- a/spec/requests/explore/catalog_controller_spec.rb +++ b/spec/requests/explore/catalog_controller_spec.rb @@ -13,10 +13,6 @@ RSpec.describe Explore::CatalogController, feature_category: :pipeline_compositi catalog_resource.project.add_reporter(user) end - before do - sign_in(user) - end - shared_examples 'basic get requests' do |action| let(:path) do if action == :index @@ -34,6 +30,10 @@ RSpec.describe Explore::CatalogController, feature_category: :pipeline_compositi end describe 'GET #show' do + before do + sign_in(user) + end + it_behaves_like 'basic get requests', :show context 'when rendering a draft catalog resource' do @@ -56,14 +56,30 @@ RSpec.describe Explore::CatalogController, feature_category: :pipeline_compositi end describe 'GET #index' do - let(:subject) { get explore_catalog_index_path } + subject(:visit_explore_catalog) { get explore_catalog_index_path } + + context 'with an authenticated user' do + before do + sign_in(user) + end - it_behaves_like 'basic get requests', :index + it_behaves_like 'basic get requests', :index - it_behaves_like 'internal event tracking' do - let(:namespace) { user.namespace } - let(:project) { nil } - let(:event) { 'unique_users_visiting_ci_catalog' } + it_behaves_like 'internal event tracking' do + let(:namespace) { user.namespace } + let(:project) { nil } + let(:event) { 'unique_users_visiting_ci_catalog' } + end + end + + context 'with an anonymous user' do + it_behaves_like 'basic get requests', :index + + it 'does not track the event' do + expect(Gitlab::InternalEvents).not_to receive(:track_event) + + visit_explore_catalog + end end end end diff --git a/spec/requests/groups/milestones_controller_spec.rb b/spec/requests/groups/milestones_controller_spec.rb index 54a25333c02..ed24ad6489f 100644 --- a/spec/requests/groups/milestones_controller_spec.rb +++ b/spec/requests/groups/milestones_controller_spec.rb @@ -18,14 +18,16 @@ RSpec.describe Groups::MilestonesController, feature_category: :team_planning do public_project = create(:project, :public, :merge_requests_enabled, :issues_enabled, group: public_group) create(:milestone, project: public_project) - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { get group_milestones_path(public_group, format: :json) }.count + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get group_milestones_path(public_group, format: :json) + end projects = create_list(:project, 2, :public, :merge_requests_enabled, :issues_enabled, group: public_group) projects.each do |project| create(:milestone, project: project) end - expect { get group_milestones_path(public_group, format: :json) }.not_to exceed_all_query_limit(control_count) + expect { get group_milestones_path(public_group, format: :json) }.not_to exceed_all_query_limit(control) expect(response).to have_gitlab_http_status(:ok) milestones = json_response @@ -66,11 +68,11 @@ RSpec.describe Groups::MilestonesController, feature_category: :team_planning do it 'avoids N+1 database queries' do perform_request # warm up the cache - control_count = ActiveRecord::QueryRecorder.new { perform_request }.count + control = ActiveRecord::QueryRecorder.new { perform_request } create(:merge_request, milestone: milestone, source_project: project, source_branch: 'fix') - expect { perform_request }.not_to exceed_query_limit(control_count) + expect { perform_request }.not_to exceed_query_limit(control) end end end diff --git a/spec/requests/groups/registry/repositories_controller_spec.rb b/spec/requests/groups/registry/repositories_controller_spec.rb index f54acf118bb..e4818676f81 100644 --- a/spec/requests/groups/registry/repositories_controller_spec.rb +++ b/spec/requests/groups/registry/repositories_controller_spec.rb @@ -20,13 +20,13 @@ RSpec.describe Groups::Registry::RepositoriesController, feature_category: :cont create(:container_repository, project: project) endpoint = group_container_registries_path(group, format: :json) - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { get(endpoint) }.count + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { get(endpoint) } create_list(:project, 2, group: group).each do |project| create_list(:container_repository, 2, project: project) end - expect { get(endpoint) }.not_to exceed_all_query_limit(control_count) + expect { get(endpoint) }.not_to exceed_all_query_limit(control) # sanity check that response is 200 expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/requests/import/gitlab_projects_controller_spec.rb b/spec/requests/import/gitlab_projects_controller_spec.rb index 732851c7828..21f1f6598d7 100644 --- a/spec/requests/import/gitlab_projects_controller_spec.rb +++ b/spec/requests/import/gitlab_projects_controller_spec.rb @@ -28,6 +28,8 @@ RSpec.describe Import::GitlabProjectsController, feature_category: :importers do end context 'with a valid path' do + let(:experiment) { instance_double(ApplicationExperiment) } + it 'schedules an import and redirects to the new project path' do stub_import(namespace) @@ -36,6 +38,38 @@ RSpec.describe Import::GitlabProjectsController, feature_category: :importers do expect(flash[:notice]).to include('is being imported') expect(response).to have_gitlab_http_status(:found) end + + it 'tracks default_to_import_tab experiment' do + stub_import(namespace) + + allow_next_instance_of(described_class) do |controller| + allow(controller) + .to receive(:experiment) + .with(:default_to_import_tab, actor: user) + .and_return(experiment) + end + + expect(experiment).to receive(:track).with(:successfully_imported, property: 'gitlab_export') + + subject + end + + it 'does not track default_to_import_tab experiment when project was not imported' do + allow_next_instance_of(::Projects::GitlabProjectsImportService) do |service| + allow(service).to receive(:execute).and_return(build(:project)) + end + + allow_next_instance_of(described_class) do |controller| + allow(controller) + .to receive(:experiment) + .with(:default_to_import_tab, actor: user) + .and_return(experiment) + end + + expect(experiment).not_to receive(:track) + + subject + end end context 'with an invalid path' do diff --git a/spec/requests/jwks_controller_spec.rb b/spec/requests/jwks_controller_spec.rb index 3dc3ed68311..0cc30ff41fb 100644 --- a/spec/requests/jwks_controller_spec.rb +++ b/spec/requests/jwks_controller_spec.rb @@ -62,19 +62,5 @@ RSpec.describe JwksController, feature_category: :system_access do expect(response).to have_gitlab_http_status(:ok) expect(response.headers['Cache-Control']).to include('max-age=86400', 'public', 'must-revalidate', 'no-transform') end - - context 'when cache_control_headers_for_openid_jwks feature flag is disabled' do - before do - stub_feature_flags(cache_control_headers_for_openid_jwks: false) - end - - it 'does not have cache control header' do - get jwks_url - - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['Cache-Control']).not_to include('max-age=86400', 'public', - 'no-transform') - end - end end end diff --git a/spec/requests/oauth/authorizations_controller_spec.rb b/spec/requests/oauth/authorizations_controller_spec.rb index 7887bf52542..6ef8970a142 100644 --- a/spec/requests/oauth/authorizations_controller_spec.rb +++ b/spec/requests/oauth/authorizations_controller_spec.rb @@ -5,16 +5,19 @@ require 'spec_helper' RSpec.describe Oauth::AuthorizationsController, feature_category: :system_access do let_it_be(:user) { create(:user) } let_it_be(:application) { create(:oauth_application, redirect_uri: 'custom://test') } - let_it_be(:oauth_authorization_path) do - Gitlab::Routing.url_helpers.oauth_authorization_url( + + let(:params) do + { client_id: application.uid, response_type: 'code', scope: application.scopes, redirect_uri: application.redirect_uri, state: SecureRandom.hex - ) + } end + let(:oauth_authorization_path) { Gitlab::Routing.url_helpers.oauth_authorization_url(params) } + before do sign_in(user) end @@ -76,5 +79,23 @@ RSpec.describe Oauth::AuthorizationsController, feature_category: :system_access end end end + + context 'when the user is not signed in' do + before do + sign_out(user) + end + + it 'sets a lower session expiry and redirects to the sign in page' do + get oauth_authorization_path + + expect(request.env['rack.session.options'][:expire_after]).to eq( + Settings.gitlab['unauthenticated_session_expire_delay'] + ) + + expect(request.session['user_return_to']).to eq("/oauth/authorize?#{params.to_query}") + expect(response).to have_gitlab_http_status(:found) + expect(response).to redirect_to(new_user_session_path) + end + end end end diff --git a/spec/requests/openid_connect_spec.rb b/spec/requests/openid_connect_spec.rb index 6573fe570db..168008b0eb9 100644 --- a/spec/requests/openid_connect_spec.rb +++ b/spec/requests/openid_connect_spec.rb @@ -273,7 +273,7 @@ RSpec.describe 'OpenID Connect requests', feature_category: :system_access do let(:expected_scopes) do %w[ admin_mode api read_user read_api read_repository write_repository sudo openid profile email - read_observability write_observability create_runner k8s_proxy ai_features + read_observability write_observability create_runner k8s_proxy ai_features read_service_ping ] end diff --git a/spec/requests/organizations/organizations_controller_spec.rb b/spec/requests/organizations/organizations_controller_spec.rb index bfd0603eb3d..33eafaf8ebd 100644 --- a/spec/requests/organizations/organizations_controller_spec.rb +++ b/spec/requests/organizations/organizations_controller_spec.rb @@ -119,4 +119,29 @@ RSpec.describe Organizations::OrganizationsController, feature_category: :cell d it_behaves_like 'controller action that requires authentication by any user' end + + describe 'POST #preview_markdown' do + subject(:gitlab_request) { post preview_markdown_organizations_path, params: { text: '### Foo \n **bar**' } } + + it_behaves_like 'controller action that requires authentication by any user' + + context 'when the user is signed in' do + let_it_be(:user) { create(:user) } + + before do + sign_in(user) + end + + it 'returns html from markdown' do + stub_commonmark_sourcepos_disabled + sign_in(user) + gitlab_request + + body = Gitlab::Json.parse(response.body)['body'] + + expect(body).not_to include('Foo</h3>') + expect(body).to include('<strong>bar</strong>') + end + end + end end diff --git a/spec/requests/organizations/settings_controller_spec.rb b/spec/requests/organizations/settings_controller_spec.rb index 1d98e598159..0177187e3a3 100644 --- a/spec/requests/organizations/settings_controller_spec.rb +++ b/spec/requests/organizations/settings_controller_spec.rb @@ -21,13 +21,13 @@ RSpec.describe Organizations::SettingsController, feature_category: :cell do end context 'when the user is signed in' do + let_it_be(:user) { create(:user) } + before do sign_in(user) end context 'with no association to an organization' do - let_it_be(:user) { create(:user) } - it_behaves_like 'organization - not found response' it_behaves_like 'organization - action disabled by `ui_for_organizations` feature flag' end @@ -39,11 +39,18 @@ RSpec.describe Organizations::SettingsController, feature_category: :cell do it_behaves_like 'organization - action disabled by `ui_for_organizations` feature flag' end - context 'as an organization user' do - let_it_be(:user) { create :user } + context 'as a default organization user' do + before_all do + create(:organization_user, organization: organization, user: user) + end - before do - create :organization_user, organization: organization, user: user + it_behaves_like 'organization - not found response' + it_behaves_like 'organization - action disabled by `ui_for_organizations` feature flag' + end + + context 'as an owner of an organization' do + before_all do + create(:organization_user, :owner, organization: organization, user: user) end it_behaves_like 'organization - successful response' diff --git a/spec/requests/projects/gcp/artifact_registry/docker_images_controller_spec.rb b/spec/requests/projects/gcp/artifact_registry/docker_images_controller_spec.rb index d571999feb0..f1c87243516 100644 --- a/spec/requests/projects/gcp/artifact_registry/docker_images_controller_spec.rb +++ b/spec/requests/projects/gcp/artifact_registry/docker_images_controller_spec.rb @@ -14,7 +14,7 @@ RSpec.describe Projects::Gcp::ArtifactRegistry::DockerImagesController, feature_ describe '#index' do let(:service_response) { ServiceResponse.success(payload: dummy_client_payload) } let(:service_double) do - instance_double('Integrations::GoogleCloudPlatform::ArtifactRegistry::ListDockerImagesService') + instance_double('GoogleCloudPlatform::ArtifactRegistry::ListDockerImagesService') end subject(:get_index_page) do @@ -30,7 +30,7 @@ RSpec.describe Projects::Gcp::ArtifactRegistry::DockerImagesController, feature_ end before do - allow_next_instance_of(Integrations::GoogleCloudPlatform::ArtifactRegistry::ListDockerImagesService) do |service| + allow_next_instance_of(GoogleCloudPlatform::ArtifactRegistry::ListDockerImagesService) do |service| allow(service).to receive(:execute).and_return(service_response) end end @@ -58,6 +58,7 @@ RSpec.describe Projects::Gcp::ArtifactRegistry::DockerImagesController, feature_ expect(response.body).to include('tag2') expect(response.body).to include('Prev') expect(response.body).to include('Next') + expect(response.body).to include('https://location.pkg.dev/project/repo/image@sha256:6a') end context 'when the service returns an error response' do diff --git a/spec/requests/projects/google_cloud/databases_controller_spec.rb b/spec/requests/projects/google_cloud/databases_controller_spec.rb index fa978a3921f..46f6d27644c 100644 --- a/spec/requests/projects/google_cloud/databases_controller_spec.rb +++ b/spec/requests/projects/google_cloud/databases_controller_spec.rb @@ -96,7 +96,7 @@ RSpec.describe Projects::GoogleCloud::DatabasesController, :snowplow, feature_ca context 'when EnableCloudsqlService fails' do before do - allow_next_instance_of(::GoogleCloud::EnableCloudsqlService) do |service| + allow_next_instance_of(::CloudSeed::GoogleCloud::EnableCloudsqlService) do |service| allow(service).to receive(:execute) .and_return({ status: :error, message: 'error' }) end @@ -125,7 +125,7 @@ RSpec.describe Projects::GoogleCloud::DatabasesController, :snowplow, feature_ca context 'when EnableCloudsqlService is successful' do before do - allow_next_instance_of(::GoogleCloud::EnableCloudsqlService) do |service| + allow_next_instance_of(::CloudSeed::GoogleCloud::EnableCloudsqlService) do |service| allow(service).to receive(:execute) .and_return({ status: :success, message: 'success' }) end @@ -133,7 +133,7 @@ RSpec.describe Projects::GoogleCloud::DatabasesController, :snowplow, feature_ca context 'when CreateCloudsqlInstanceService fails' do before do - allow_next_instance_of(::GoogleCloud::CreateCloudsqlInstanceService) do |service| + allow_next_instance_of(::CloudSeed::GoogleCloud::CreateCloudsqlInstanceService) do |service| allow(service).to receive(:execute) .and_return({ status: :error, message: 'error' }) end @@ -162,7 +162,7 @@ RSpec.describe Projects::GoogleCloud::DatabasesController, :snowplow, feature_ca context 'when CreateCloudsqlInstanceService is successful' do before do - allow_next_instance_of(::GoogleCloud::CreateCloudsqlInstanceService) do |service| + allow_next_instance_of(::CloudSeed::GoogleCloud::CreateCloudsqlInstanceService) do |service| allow(service).to receive(:execute) .and_return({ status: :success, message: 'success' }) end diff --git a/spec/requests/projects/google_cloud/deployments_controller_spec.rb b/spec/requests/projects/google_cloud/deployments_controller_spec.rb index e9eac1e7ecd..1a6482477ef 100644 --- a/spec/requests/projects/google_cloud/deployments_controller_spec.rb +++ b/spec/requests/projects/google_cloud/deployments_controller_spec.rb @@ -110,7 +110,7 @@ RSpec.describe Projects::GoogleCloud::DeploymentsController, feature_category: : context 'when enable service fails' do before do - allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |service| + allow_next_instance_of(CloudSeed::GoogleCloud::EnableCloudRunService) do |service| allow(service) .to receive(:execute) .and_return( @@ -146,7 +146,7 @@ RSpec.describe Projects::GoogleCloud::DeploymentsController, feature_category: : before do mock_gcp_error = Google::Apis::ClientError.new('some_error') - allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |service| + allow_next_instance_of(CloudSeed::GoogleCloud::EnableCloudRunService) do |service| allow(service).to receive(:execute).and_raise(mock_gcp_error) end end @@ -173,14 +173,14 @@ RSpec.describe Projects::GoogleCloud::DeploymentsController, feature_category: : context 'GCP_PROJECT_IDs are defined' do before do - allow_next_instance_of(GoogleCloud::EnableCloudRunService) do |enable_cloud_run_service| + allow_next_instance_of(CloudSeed::GoogleCloud::EnableCloudRunService) do |enable_cloud_run_service| allow(enable_cloud_run_service).to receive(:execute).and_return({ status: :success }) end end context 'when generate pipeline service fails' do before do - allow_next_instance_of(GoogleCloud::GeneratePipelineService) do |generate_pipeline_service| + allow_next_instance_of(CloudSeed::GoogleCloud::GeneratePipelineService) do |generate_pipeline_service| allow(generate_pipeline_service).to receive(:execute).and_return({ status: :error }) end end @@ -206,7 +206,7 @@ RSpec.describe Projects::GoogleCloud::DeploymentsController, feature_category: : end it 'redirects to create merge request form' do - allow_next_instance_of(GoogleCloud::GeneratePipelineService) do |service| + allow_next_instance_of(CloudSeed::GoogleCloud::GeneratePipelineService) do |service| allow(service).to receive(:execute).and_return({ status: :success }) end diff --git a/spec/requests/projects/ml/models_controller_spec.rb b/spec/requests/projects/ml/models_controller_spec.rb index cda3f777a72..e469ee837bc 100644 --- a/spec/requests/projects/ml/models_controller_spec.rb +++ b/spec/requests/projects/ml/models_controller_spec.rb @@ -152,6 +152,27 @@ RSpec.describe Projects::Ml::ModelsController, feature_category: :mlops do end end + describe 'GET new' do + subject(:create_model_request) do + new_model + response + end + + before do + create_model_request + end + + it 'renders the template' do + is_expected.to render_template('projects/ml/models/new') + end + + context 'when user does not have access' do + let(:write_model_registry) { false } + + it { is_expected.to have_gitlab_http_status(:not_found) } + end + end + describe 'destroy' do let(:model_for_deletion) do create(:ml_models, project: project) @@ -198,4 +219,8 @@ RSpec.describe Projects::Ml::ModelsController, feature_category: :mlops do def delete_model delete project_ml_model_path(project, model_id) end + + def new_model + get new_project_ml_model_path(project) + end end diff --git a/spec/requests/projects/pipelines_controller_spec.rb b/spec/requests/projects/pipelines_controller_spec.rb index aa3fefdef14..8be4fecea04 100644 --- a/spec/requests/projects/pipelines_controller_spec.rb +++ b/spec/requests/projects/pipelines_controller_spec.rb @@ -25,14 +25,14 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte create_pipelines - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do get_pipelines_index - end.count + end create_pipelines # There appears to be one extra query for Pipelines#has_warnings? for some reason - expect { get_pipelines_index }.not_to exceed_all_query_limit(control_count + 1) + expect { get_pipelines_index }.not_to exceed_all_query_limit(control).with_threshold(1) expect(response).to have_gitlab_http_status(:ok) expect(json_response['pipelines'].count).to eq(11) end @@ -56,9 +56,9 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte it 'does not execute N+1 queries' do request_build_stage - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do request_build_stage - end.count + end create(:ci_build, pipeline: pipeline, stage: 'build') @@ -70,7 +70,7 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte status: :failed) end - expect { request_build_stage }.not_to exceed_all_query_limit(control_count) + expect { request_build_stage }.not_to exceed_all_query_limit(control) expect(response).to have_gitlab_http_status(:ok) end @@ -134,14 +134,14 @@ RSpec.describe Projects::PipelinesController, feature_category: :continuous_inte request_build_stage(retried: true) - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do request_build_stage(retried: true) - end.count + end create(:ci_build, :retried, :failed, pipeline: pipeline, stage: 'build') create(:ci_build, :failed, pipeline: pipeline, stage: 'build') - expect { request_build_stage(retried: true) }.not_to exceed_all_query_limit(control_count) + expect { request_build_stage(retried: true) }.not_to exceed_all_query_limit(control) expect(response).to have_gitlab_http_status(:ok) end diff --git a/spec/requests/projects/settings/packages_and_registries_controller_spec.rb b/spec/requests/projects/settings/packages_and_registries_controller_spec.rb index 2806beadd4e..c660be0f3bf 100644 --- a/spec/requests/projects/settings/packages_and_registries_controller_spec.rb +++ b/spec/requests/projects/settings/packages_and_registries_controller_spec.rb @@ -16,6 +16,36 @@ RSpec.describe Projects::Settings::PackagesAndRegistriesController, feature_cate stub_container_registry_config(enabled: container_registry_enabled) end + describe 'GET #show' do + context 'when user is authorized' do + let(:user) { project.creator } + + subject { get namespace_project_settings_packages_and_registries_path(user.namespace, project) } + + before do + sign_in(user) + end + + it 'pushes the feature flag "packages_protected_packages" to the view' do + subject + + expect(response.body).to have_pushed_frontend_feature_flags(packagesProtectedPackages: true) + end + + context 'when feature flag "packages_protected_packages" is disabled' do + before do + stub_feature_flags(packages_protected_packages: false) + end + + it 'does not push the feature flag "packages_protected_packages" to the view' do + subject + + expect(response.body).not_to have_pushed_frontend_feature_flags(packagesProtectedPackages: true) + end + end + end + end + describe 'GET #cleanup_tags' do subject { get cleanup_image_tags_namespace_project_settings_packages_and_registries_path(user.namespace, project) } diff --git a/spec/requests/search_controller_spec.rb b/spec/requests/search_controller_spec.rb index 365b20ad4aa..eaf11653256 100644 --- a/spec/requests/search_controller_spec.rb +++ b/spec/requests/search_controller_spec.rb @@ -112,7 +112,7 @@ RSpec.describe SearchController, type: :request, feature_category: :global_searc control = ActiveRecord::QueryRecorder.new { send_search_request(params_for_one) } expect(response.body).to include('search-results') # Confirm search results to prevent false positives - expect { send_search_request(params_for_many) }.not_to exceed_query_limit(control.count) + expect { send_search_request(params_for_many) }.not_to exceed_query_limit(control) expect(response.body).to include('search-results') # Confirm search results to prevent false positives end end @@ -125,7 +125,7 @@ RSpec.describe SearchController, type: :request, feature_category: :global_searc control = ActiveRecord::QueryRecorder.new { send_search_request(params_for_one) } expect(response.body).to include('search-results') # Confirm search results to prevent false positives - expect { send_search_request(params_for_many) }.not_to exceed_query_limit(control.count) + expect { send_search_request(params_for_many) }.not_to exceed_query_limit(control) expect(response.body).to include('search-results') # Confirm search results to prevent false positives end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index da111831c15..3b2d3a6f7a5 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -799,6 +799,46 @@ RSpec.describe UsersController, feature_category: :user_management do end end + describe 'GET #groups' do + before do + sign_in(user) + end + + context 'format html' do + it 'renders groups page' do + get user_groups_url user.username + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template('show') + end + end + + context 'format json' do + it 'response with groups data' do + get user_groups_url user.username, format: :json + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to have_key('html') + end + + context 'pagination' do + let!(:per_page_limit) { 3 } + + before do + allow(Kaminari.config).to receive(:default_per_page).and_return(per_page_limit) + create_list(:group, per_page_limit + 2).each { |group| group.add_owner(user) } + end + + it 'paginates without count' do + get user_groups_url user.username, format: :json + + expect(assigns(:groups).size).to eq(per_page_limit) + expect(assigns(:groups)).to be_a(Kaminari::PaginatableWithoutCount) + end + end + end + end + describe '#ensure_canonical_path' do before do sign_in(user) |