diff options
Diffstat (limited to 'spec/requests')
60 files changed, 2175 insertions, 413 deletions
diff --git a/spec/requests/api/admin/plan_limits_spec.rb b/spec/requests/api/admin/plan_limits_spec.rb new file mode 100644 index 00000000000..6bc133f67c0 --- /dev/null +++ b/spec/requests/api/admin/plan_limits_spec.rb @@ -0,0 +1,177 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Admin::PlanLimits, 'PlanLimits' do + let_it_be(:user) { create(:user) } + let_it_be(:admin) { create(:admin) } + let_it_be(:plan) { create(:plan, name: 'default') } + + describe 'GET /application/plan_limits' do + context 'as a non-admin user' do + it 'returns 403' do + get api('/application/plan_limits', user) + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'as an admin user' do + context 'no params' do + it 'returns plan limits' do + get api('/application/plan_limits', admin) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Hash + expect(json_response['conan_max_file_size']).to eq(Plan.default.actual_limits.conan_max_file_size) + expect(json_response['generic_packages_max_file_size']).to eq(Plan.default.actual_limits.generic_packages_max_file_size) + expect(json_response['maven_max_file_size']).to eq(Plan.default.actual_limits.maven_max_file_size) + expect(json_response['npm_max_file_size']).to eq(Plan.default.actual_limits.npm_max_file_size) + expect(json_response['nuget_max_file_size']).to eq(Plan.default.actual_limits.nuget_max_file_size) + expect(json_response['pypi_max_file_size']).to eq(Plan.default.actual_limits.pypi_max_file_size) + end + end + + context 'correct plan name in params' do + before do + @params = { plan_name: 'default' } + end + + it 'returns plan limits' do + get api('/application/plan_limits', admin), params: @params + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Hash + expect(json_response['conan_max_file_size']).to eq(Plan.default.actual_limits.conan_max_file_size) + expect(json_response['generic_packages_max_file_size']).to eq(Plan.default.actual_limits.generic_packages_max_file_size) + expect(json_response['maven_max_file_size']).to eq(Plan.default.actual_limits.maven_max_file_size) + expect(json_response['npm_max_file_size']).to eq(Plan.default.actual_limits.npm_max_file_size) + expect(json_response['nuget_max_file_size']).to eq(Plan.default.actual_limits.nuget_max_file_size) + expect(json_response['pypi_max_file_size']).to eq(Plan.default.actual_limits.pypi_max_file_size) + end + end + + context 'invalid plan name in params' do + before do + @params = { plan_name: 'my-plan' } + end + + it 'returns validation error' do + get api('/application/plan_limits', admin), params: @params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to eq('plan_name does not have a valid value') + end + end + end + end + + describe 'PUT /application/plan_limits' do + context 'as a non-admin user' do + it 'returns 403' do + put api('/application/plan_limits', user), params: { plan_name: 'default' } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'as an admin user' do + context 'correct params' do + it 'updates multiple plan limits' do + put api('/application/plan_limits', admin), params: { + 'plan_name': 'default', + 'conan_max_file_size': 10, + 'generic_packages_max_file_size': 20, + 'maven_max_file_size': 30, + 'npm_max_file_size': 40, + 'nuget_max_file_size': 50, + 'pypi_max_file_size': 60 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Hash + expect(json_response['conan_max_file_size']).to eq(10) + expect(json_response['generic_packages_max_file_size']).to eq(20) + expect(json_response['maven_max_file_size']).to eq(30) + expect(json_response['npm_max_file_size']).to eq(40) + expect(json_response['nuget_max_file_size']).to eq(50) + expect(json_response['pypi_max_file_size']).to eq(60) + end + + it 'updates single plan limits' do + put api('/application/plan_limits', admin), params: { + 'plan_name': 'default', + 'maven_max_file_size': 100 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Hash + expect(json_response['maven_max_file_size']).to eq(100) + end + end + + context 'empty params' do + it 'fails to update plan limits' do + put api('/application/plan_limits', admin), params: {} + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to match('plan_name is missing') + end + end + + context 'params with wrong type' do + it 'fails to update plan limits' do + put api('/application/plan_limits', admin), params: { + 'plan_name': 'default', + 'conan_max_file_size': 'a', + 'generic_packages_max_file_size': 'b', + 'maven_max_file_size': 'c', + 'npm_max_file_size': 'd', + 'nuget_max_file_size': 'e', + 'pypi_max_file_size': 'f' + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to include( + 'conan_max_file_size is invalid', + 'generic_packages_max_file_size is invalid', + 'maven_max_file_size is invalid', + 'generic_packages_max_file_size is invalid', + 'npm_max_file_size is invalid', + 'nuget_max_file_size is invalid', + 'pypi_max_file_size is invalid' + ) + end + end + + context 'missing plan_name in params' do + it 'fails to update plan limits' do + put api('/application/plan_limits', admin), params: { 'conan_max_file_size': 0 } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to match('plan_name is missing') + end + end + + context 'additional undeclared params' do + before do + Plan.default.actual_limits.update!({ 'golang_max_file_size': 1000 }) + end + + it 'updates only declared plan limits' do + put api('/application/plan_limits', admin), params: { + 'plan_name': 'default', + 'pypi_max_file_size': 200, + 'golang_max_file_size': 999 + } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Hash + expect(json_response['pypi_max_file_size']).to eq(200) + expect(json_response['golang_max_file_size']).to be_nil + expect(Plan.default.actual_limits.golang_max_file_size).to eq(1000) + end + end + end + end +end diff --git a/spec/requests/api/api_spec.rb b/spec/requests/api/api_spec.rb index 8bd6049e6fa..522030652bd 100644 --- a/spec/requests/api/api_spec.rb +++ b/spec/requests/api/api_spec.rb @@ -112,6 +112,7 @@ RSpec.describe API::API do 'meta.project' => project.full_path, 'meta.root_namespace' => project.namespace.full_path, 'meta.user' => user.username, + 'meta.client_id' => an_instance_of(String), 'meta.feature_category' => 'issue_tracking') end end @@ -125,6 +126,7 @@ RSpec.describe API::API do expect(log_context).to match('correlation_id' => an_instance_of(String), 'meta.caller_id' => '/api/:version/users', 'meta.remote_ip' => an_instance_of(String), + 'meta.client_id' => an_instance_of(String), 'meta.feature_category' => 'users') end end @@ -133,6 +135,28 @@ RSpec.describe API::API do end end + describe 'Marginalia comments' do + context 'GET /user/:id' do + let_it_be(:user) { create(:user) } + let(:component_map) do + { + "application" => "test", + "endpoint_id" => "/api/:version/users/:id" + } + end + + subject { ActiveRecord::QueryRecorder.new { get api("/users/#{user.id}", user) } } + + it 'generates a query that includes the expected annotations' do + expect(subject.log.last).to match(/correlation_id:.*/) + + component_map.each do |component, value| + expect(subject.log.last).to include("#{component}:#{value}") + end + end + end + end + describe 'supported content-types' do context 'GET /user/:id.txt' do let_it_be(:user) { create(:user) } diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index a9afbd8bd72..d0c2b383013 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -34,7 +34,7 @@ RSpec.describe API::Ci::Pipelines do expect(json_response.first['sha']).to match(/\A\h{40}\z/) expect(json_response.first['id']).to eq pipeline.id expect(json_response.first['web_url']).to be_present - expect(json_response.first.keys).to contain_exactly(*%w[id sha ref status web_url created_at updated_at]) + expect(json_response.first.keys).to contain_exactly(*%w[id project_id sha ref status web_url created_at updated_at]) end context 'when parameter is passed' do @@ -350,6 +350,7 @@ RSpec.describe API::Ci::Pipelines do expect(json_job['pipeline']).not_to be_empty expect(json_job['pipeline']['id']).to eq job.pipeline.id + expect(json_job['pipeline']['project_id']).to eq job.pipeline.project_id expect(json_job['pipeline']['ref']).to eq job.pipeline.ref expect(json_job['pipeline']['sha']).to eq job.pipeline.sha expect(json_job['pipeline']['status']).to eq job.pipeline.status @@ -512,6 +513,7 @@ RSpec.describe API::Ci::Pipelines do expect(json_bridge['pipeline']).not_to be_empty expect(json_bridge['pipeline']['id']).to eq bridge.pipeline.id + expect(json_bridge['pipeline']['project_id']).to eq bridge.pipeline.project_id expect(json_bridge['pipeline']['ref']).to eq bridge.pipeline.ref expect(json_bridge['pipeline']['sha']).to eq bridge.pipeline.sha expect(json_bridge['pipeline']['status']).to eq bridge.pipeline.status @@ -522,6 +524,7 @@ RSpec.describe API::Ci::Pipelines do expect(json_bridge['downstream_pipeline']).not_to be_empty expect(json_bridge['downstream_pipeline']['id']).to eq downstream_pipeline.id + expect(json_bridge['downstream_pipeline']['project_id']).to eq downstream_pipeline.project_id expect(json_bridge['downstream_pipeline']['ref']).to eq downstream_pipeline.ref expect(json_bridge['downstream_pipeline']['sha']).to eq downstream_pipeline.sha expect(json_bridge['downstream_pipeline']['status']).to eq downstream_pipeline.status diff --git a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb index 4d8da50f8f0..9369b6aa464 100644 --- a/spec/requests/api/ci/runner/jobs_artifacts_spec.rb +++ b/spec/requests/api/ci/runner/jobs_artifacts_spec.rb @@ -17,9 +17,9 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end describe '/api/v4/jobs' do - let(:root_namespace) { create(:namespace) } - let(:namespace) { create(:namespace, parent: root_namespace) } - let(:project) { create(:project, namespace: namespace, shared_runners_enabled: false) } + let(:parent_group) { create(:group) } + let(:group) { create(:group, parent: parent_group) } + let(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:user) { create(:user) } @@ -78,7 +78,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do before do stub_application_setting(max_artifacts_size: application_max_size) - root_namespace.update!(max_artifacts_size: sample_max_size) + parent_group.update!(max_artifacts_size: sample_max_size) end it_behaves_like 'failed request' @@ -90,8 +90,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do before do stub_application_setting(max_artifacts_size: application_max_size) - root_namespace.update!(max_artifacts_size: root_namespace_max_size) - namespace.update!(max_artifacts_size: sample_max_size) + parent_group.update!(max_artifacts_size: root_namespace_max_size) + group.update!(max_artifacts_size: sample_max_size) end it_behaves_like 'failed request' @@ -104,8 +104,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do before do stub_application_setting(max_artifacts_size: application_max_size) - root_namespace.update!(max_artifacts_size: root_namespace_max_size) - namespace.update!(max_artifacts_size: child_namespace_max_size) + parent_group.update!(max_artifacts_size: root_namespace_max_size) + group.update!(max_artifacts_size: child_namespace_max_size) project.update!(max_artifacts_size: sample_max_size) end diff --git a/spec/requests/api/ci/runner/jobs_put_spec.rb b/spec/requests/api/ci/runner/jobs_put_spec.rb index f4c99307b1a..b5d2c4608c5 100644 --- a/spec/requests/api/ci/runner/jobs_put_spec.rb +++ b/spec/requests/api/ci/runner/jobs_put_spec.rb @@ -17,9 +17,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end describe '/api/v4/jobs' do - let(:root_namespace) { create(:namespace) } - let(:namespace) { create(:namespace, parent: root_namespace) } - let(:project) { create(:project, namespace: namespace, shared_runners_enabled: false) } + let(:group) { create(:group, :nested) } + let(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:user) { create(:user) } 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 74d8e3f7ae8..aced094e219 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -17,9 +17,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end describe '/api/v4/jobs' do - let(:root_namespace) { create(:namespace) } - let(:namespace) { create(:namespace, parent: root_namespace) } - let(:project) { create(:project, namespace: namespace, shared_runners_enabled: false) } + let(:group) { create(:group, :nested) } + let(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:user) { create(:user) } @@ -198,7 +197,12 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do 'when' => 'on_success' }] end - let(:expected_features) { { 'trace_sections' => true } } + let(:expected_features) do + { + 'trace_sections' => true, + 'failure_reasons' => include('script_failure') + } + end it 'picks a job' do request_job info: { platform: :darwin } @@ -220,7 +224,7 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(json_response['artifacts']).to eq(expected_artifacts) expect(json_response['cache']).to eq(expected_cache) expect(json_response['variables']).to include(*expected_variables) - expect(json_response['features']).to eq(expected_features) + expect(json_response['features']).to match(expected_features) end it 'creates persistent ref' do @@ -793,6 +797,50 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end end + describe 'setting the application context' do + subject { request_job } + + context 'when triggered by a user' do + let(:job) { create(:ci_build, user: user, project: project) } + + subject { request_job(id: job.id) } + + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { user: user.username, project: project.full_path, client_id: "user/#{user.id}" } } + end + + it_behaves_like 'not executing any extra queries for the application context', 3 do + # Extra queries: User, Project, Route + let(:subject_proc) { proc { request_job(id: job.id) } } + end + end + + context 'when the runner is of project type' do + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { project: project.full_path, client_id: "runner/#{runner.id}" } } + end + + it_behaves_like 'not executing any extra queries for the application context', 2 do + # Extra queries: Project, Route + let(:subject_proc) { proc { request_job } } + end + end + + context 'when the runner is of group type' do + let(:group) { create(:group) } + let(:runner) { create(:ci_runner, :group, groups: [group]) } + + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { root_namespace: group.full_path_components.first, client_id: "runner/#{runner.id}" } } + end + + it_behaves_like 'not executing any extra queries for the application context', 2 do + # Extra queries: Group, Route + let(:subject_proc) { proc { request_job } } + end + end + end + def request_job(token = runner.token, **params) new_params = params.merge(token: token, last_update: last_update) post api('/jobs/request'), params: new_params.to_json, headers: { 'User-Agent' => user_agent, 'Content-Type': 'application/json' } diff --git a/spec/requests/api/ci/runner/jobs_trace_spec.rb b/spec/requests/api/ci/runner/jobs_trace_spec.rb index 5b7a33d23d8..659cf055023 100644 --- a/spec/requests/api/ci/runner/jobs_trace_spec.rb +++ b/spec/requests/api/ci/runner/jobs_trace_spec.rb @@ -17,9 +17,8 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end describe '/api/v4/jobs' do - let(:root_namespace) { create(:namespace) } - let(:namespace) { create(:namespace, parent: root_namespace) } - let(:project) { create(:project, namespace: namespace, shared_runners_enabled: false) } + let(:group) { create(:group, :nested) } + let(:project) { create(:project, namespace: group, shared_runners_enabled: false) } let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') } let(:runner) { create(:ci_runner, :project, projects: [project]) } let(:user) { create(:user) } diff --git a/spec/requests/api/ci/runner/runners_delete_spec.rb b/spec/requests/api/ci/runner/runners_delete_spec.rb index 75960a1a1c0..6c6c465f161 100644 --- a/spec/requests/api/ci/runner/runners_delete_spec.rb +++ b/spec/requests/api/ci/runner/runners_delete_spec.rb @@ -37,8 +37,10 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do context 'when valid token is provided' do let(:runner) { create(:ci_runner) } + subject { delete api('/runners'), params: { token: runner.token } } + it 'deletes Runner' do - delete api('/runners'), params: { token: runner.token } + subject expect(response).to have_gitlab_http_status(:no_content) expect(::Ci::Runner.count).to eq(0) @@ -48,6 +50,10 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do let(:request) { api('/runners') } let(:params) { { token: runner.token } } end + + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { client_id: "runner/#{runner.id}" } } + end end end end diff --git a/spec/requests/api/ci/runner/runners_post_spec.rb b/spec/requests/api/ci/runner/runners_post_spec.rb index 7c362fae7d2..7984b1d4ca8 100644 --- a/spec/requests/api/ci/runner/runners_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_post_spec.rb @@ -35,25 +35,44 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end context 'when valid token is provided' do - it 'creates runner with default values' do - post api('/runners'), params: { token: registration_token } + def request + post api('/runners'), params: { token: token } + end - runner = ::Ci::Runner.first + context 'with a registration token' do + let(:token) { registration_token } - expect(response).to have_gitlab_http_status(:created) - expect(json_response['id']).to eq(runner.id) - expect(json_response['token']).to eq(runner.token) - expect(runner.run_untagged).to be true - expect(runner.active).to be true - expect(runner.token).not_to eq(registration_token) - expect(runner).to be_instance_type + it 'creates runner with default values' do + request + + runner = ::Ci::Runner.first + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['id']).to eq(runner.id) + expect(json_response['token']).to eq(runner.token) + expect(runner.run_untagged).to be true + expect(runner.active).to be true + expect(runner.token).not_to eq(registration_token) + expect(runner).to be_instance_type + end + + it_behaves_like 'storing arguments in the application context' do + subject { request } + + let(:expected_params) { { client_id: "runner/#{::Ci::Runner.first.id}" } } + end + + it_behaves_like 'not executing any extra queries for the application context' do + let(:subject_proc) { proc { request } } + end end context 'when project token is used' do let(:project) { create(:project) } + let(:token) { project.runners_token } it 'creates project runner' do - post api('/runners'), params: { token: project.runners_token } + request expect(response).to have_gitlab_http_status(:created) expect(project.runners.size).to eq(1) @@ -62,13 +81,24 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(runner.token).not_to eq(project.runners_token) expect(runner).to be_project_type end + + it_behaves_like 'storing arguments in the application context' do + subject { request } + + let(:expected_params) { { project: project.full_path, client_id: "runner/#{::Ci::Runner.first.id}" } } + end + + it_behaves_like 'not executing any extra queries for the application context' do + let(:subject_proc) { proc { request } } + end end context 'when group token is used' do let(:group) { create(:group) } + let(:token) { group.runners_token } it 'creates a group runner' do - post api('/runners'), params: { token: group.runners_token } + request expect(response).to have_gitlab_http_status(:created) expect(group.runners.reload.size).to eq(1) @@ -77,6 +107,16 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(runner.token).not_to eq(group.runners_token) expect(runner).to be_group_type end + + it_behaves_like 'storing arguments in the application context' do + subject { request } + + let(:expected_params) { { root_namespace: group.full_path_components.first, client_id: "runner/#{::Ci::Runner.first.id}" } } + end + + it_behaves_like 'not executing any extra queries for the application context' do + let(:subject_proc) { proc { request } } + end end end diff --git a/spec/requests/api/ci/runner/runners_verify_post_spec.rb b/spec/requests/api/ci/runner/runners_verify_post_spec.rb index e2f5f9b2d68..c2e97446738 100644 --- a/spec/requests/api/ci/runner/runners_verify_post_spec.rb +++ b/spec/requests/api/ci/runner/runners_verify_post_spec.rb @@ -37,11 +37,17 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do end context 'when valid token is provided' do + subject { post api('/runners/verify'), params: { token: runner.token } } + it 'verifies Runner credentials' do - post api('/runners/verify'), params: { token: runner.token } + subject expect(response).to have_gitlab_http_status(:ok) end + + it_behaves_like 'storing arguments in the application context' do + let(:expected_params) { { client_id: "runner/#{runner.id}" } } + end end end end diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb index bec15b788c3..10fa15d468f 100644 --- a/spec/requests/api/commit_statuses_spec.rb +++ b/spec/requests/api/commit_statuses_spec.rb @@ -291,7 +291,7 @@ RSpec.describe API::CommitStatuses do end context 'when retrying a commit status' do - before do + subject(:post_request) do post api(post_url, developer), params: { state: 'failed', name: 'test', ref: 'master' } @@ -300,15 +300,45 @@ RSpec.describe API::CommitStatuses do end it 'correctly posts a new commit status' do + post_request + expect(response).to have_gitlab_http_status(:created) expect(json_response['sha']).to eq(commit.id) expect(json_response['status']).to eq('success') end - it 'retries a commit status', :sidekiq_might_not_need_inline do - expect(CommitStatus.count).to eq 2 - expect(CommitStatus.first).to be_retried - expect(CommitStatus.last.pipeline).to be_success + context 'feature flags' do + using RSpec::Parameterized::TableSyntax + + where(:ci_fix_commit_status_retried, :ci_remove_update_retried_from_process_pipeline, :previous_statuses_retried) do + true | true | true + true | false | true + false | true | false + false | false | true + end + + with_them do + before do + stub_feature_flags( + ci_fix_commit_status_retried: ci_fix_commit_status_retried, + ci_remove_update_retried_from_process_pipeline: ci_remove_update_retried_from_process_pipeline + ) + end + + it 'retries a commit status', :sidekiq_might_not_need_inline do + post_request + + expect(CommitStatus.count).to eq 2 + + if previous_statuses_retried + expect(CommitStatus.first).to be_retried + expect(CommitStatus.last.pipeline).to be_success + else + expect(CommitStatus.first).not_to be_retried + expect(CommitStatus.last.pipeline).to be_failed + end + end + end end end diff --git a/spec/requests/api/composer_packages_spec.rb b/spec/requests/api/composer_packages_spec.rb index 06d4a2c6017..30a831d24fd 100644 --- a/spec/requests/api/composer_packages_spec.rb +++ b/spec/requests/api/composer_packages_spec.rb @@ -222,6 +222,52 @@ RSpec.describe API::ComposerPackages do it_behaves_like 'rejects Composer access with unknown group id' end + describe 'GET /api/v4/group/:id/-/packages/composer/p2/*package_name.json' do + let(:package_name) { 'foobar' } + let(:url) { "/group/#{group.id}/-/packages/composer/p2/#{package_name}.json" } + + subject { get api(url), headers: headers } + + context 'with no packages' do + include_context 'Composer user type', :developer, true do + it_behaves_like 'returning response status', :not_found + end + end + + context 'with valid project' do + let!(:package) { create(:composer_package, :with_metadatum, name: package_name, project: project) } + + where(:project_visibility_level, :user_role, :member, :user_token, :shared_examples_name, :expected_status) do + 'PUBLIC' | :developer | true | true | 'Composer package api request' | :success + 'PUBLIC' | :developer | true | false | 'process Composer api request' | :unauthorized + 'PUBLIC' | :developer | false | true | 'Composer package api request' | :success + 'PUBLIC' | :developer | false | false | 'process Composer api request' | :unauthorized + 'PUBLIC' | :guest | true | true | 'Composer package api request' | :success + 'PUBLIC' | :guest | true | false | 'process Composer api request' | :unauthorized + 'PUBLIC' | :guest | false | true | 'Composer package api request' | :success + 'PUBLIC' | :guest | false | false | 'process Composer api request' | :unauthorized + 'PUBLIC' | :anonymous | false | true | 'Composer package api request' | :success + 'PRIVATE' | :developer | true | true | 'Composer package api request' | :success + 'PRIVATE' | :developer | true | false | 'process Composer api request' | :unauthorized + 'PRIVATE' | :developer | false | true | 'process Composer api request' | :not_found + 'PRIVATE' | :developer | false | false | 'process Composer api request' | :unauthorized + 'PRIVATE' | :guest | true | true | 'process Composer api request' | :not_found + 'PRIVATE' | :guest | true | false | 'process Composer api request' | :unauthorized + 'PRIVATE' | :guest | false | true | 'process Composer api request' | :not_found + 'PRIVATE' | :guest | false | false | 'process Composer api request' | :unauthorized + 'PRIVATE' | :anonymous | false | true | 'process Composer api request' | :not_found + end + + with_them do + include_context 'Composer api group access', params[:project_visibility_level], params[:user_role], params[:user_token] do + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + end + end + end + + it_behaves_like 'rejects Composer access with unknown group id' + end + describe 'POST /api/v4/projects/:id/packages/composer' do let(:url) { "/projects/#{project.id}/packages/composer" } let(:params) { {} } diff --git a/spec/requests/api/discussions_spec.rb b/spec/requests/api/discussions_spec.rb index bdfc1589c9e..258bd26c05a 100644 --- a/spec/requests/api/discussions_spec.rb +++ b/spec/requests/api/discussions_spec.rb @@ -68,18 +68,11 @@ RSpec.describe API::Discussions do mr_commit = '0b4bc9a49b562e85de7cc9e834518ea6828729b9' parent_commit = 'ae73cb07c9eeaf35924a10f713b364d32b2dd34f' file = "files/ruby/feature.rb" - line_range = { - "start_line_code" => Gitlab::Git.diff_line_code(file, 1, 1), - "end_line_code" => Gitlab::Git.diff_line_code(file, 1, 1), - "start_line_type" => "text", - "end_line_type" => "text" - } position = build( :text_diff_position, :added, file: file, new_line: 1, - line_range: line_range, base_sha: parent_commit, head_sha: mr_commit, start_sha: parent_commit diff --git a/spec/requests/api/environments_spec.rb b/spec/requests/api/environments_spec.rb index b1ac8f9eeec..303e510883d 100644 --- a/spec/requests/api/environments_spec.rb +++ b/spec/requests/api/environments_spec.rb @@ -265,4 +265,76 @@ RSpec.describe API::Environments do end end end + + describe "DELETE /projects/:id/environments/review_apps" do + shared_examples "delete stopped review environments" do + around do |example| + freeze_time { example.run } + end + + it "deletes the old stopped review apps" do + old_stopped_review_env = create(:environment, :with_review_app, :stopped, created_at: 31.days.ago, project: project) + new_stopped_review_env = create(:environment, :with_review_app, :stopped, project: project) + old_active_review_env = create(:environment, :with_review_app, :available, created_at: 31.days.ago, project: project) + old_stopped_other_env = create(:environment, :stopped, created_at: 31.days.ago, project: project) + new_stopped_other_env = create(:environment, :stopped, project: project) + old_active_other_env = create(:environment, :available, created_at: 31.days.ago, project: project) + + delete api("/projects/#{project.id}/environments/review_apps", current_user), params: { dry_run: false } + project.environments.reload + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response["scheduled_entries"].size).to eq(1) + expect(json_response["scheduled_entries"].first["id"]).to eq(old_stopped_review_env.id) + expect(json_response["unprocessable_entries"].size).to eq(0) + + expect(old_stopped_review_env.reload.auto_delete_at).to eq(1.week.from_now) + expect(new_stopped_review_env.reload.auto_delete_at).to be_nil + expect(old_active_review_env.reload.auto_delete_at).to be_nil + expect(old_stopped_other_env.reload.auto_delete_at).to be_nil + expect(new_stopped_other_env.reload.auto_delete_at).to be_nil + expect(old_active_other_env.reload.auto_delete_at).to be_nil + end + end + + context "as a maintainer" do + it_behaves_like "delete stopped review environments" do + let(:current_user) { user } + end + end + + context "as a developer" do + let(:developer) { create(:user) } + + before do + project.add_developer(developer) + end + + it_behaves_like "delete stopped review environments" do + let(:current_user) { developer } + end + end + + context "as a reporter" do + let(:reporter) { create(:user) } + + before do + project.add_reporter(reporter) + end + + it "rejects the request" do + delete api("/projects/#{project.id}/environments/review_apps", reporter) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context "as a non member" do + it "rejects the request" do + delete api("/projects/#{project.id}/environments/review_apps", non_member) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end end diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index a47be1ead9c..16d56b6cfbe 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -444,17 +444,17 @@ RSpec.describe API::GenericPackages do 'PUBLIC' | :guest | true | :user_basic_auth | :success 'PUBLIC' | :developer | true | :invalid_personal_access_token | :unauthorized 'PUBLIC' | :guest | true | :invalid_personal_access_token | :unauthorized - 'PUBLIC' | :developer | true | :invalid_user_basic_auth | :unauthorized - 'PUBLIC' | :guest | true | :invalid_user_basic_auth | :unauthorized + 'PUBLIC' | :developer | true | :invalid_user_basic_auth | :success + 'PUBLIC' | :guest | true | :invalid_user_basic_auth | :success 'PUBLIC' | :developer | false | :personal_access_token | :success 'PUBLIC' | :guest | false | :personal_access_token | :success 'PUBLIC' | :developer | false | :user_basic_auth | :success 'PUBLIC' | :guest | false | :user_basic_auth | :success 'PUBLIC' | :developer | false | :invalid_personal_access_token | :unauthorized 'PUBLIC' | :guest | false | :invalid_personal_access_token | :unauthorized - 'PUBLIC' | :developer | false | :invalid_user_basic_auth | :unauthorized - 'PUBLIC' | :guest | false | :invalid_user_basic_auth | :unauthorized - 'PUBLIC' | :anonymous | false | :none | :unauthorized + 'PUBLIC' | :developer | false | :invalid_user_basic_auth | :success + 'PUBLIC' | :guest | false | :invalid_user_basic_auth | :success + 'PUBLIC' | :anonymous | false | :none | :success 'PRIVATE' | :developer | true | :personal_access_token | :success 'PRIVATE' | :guest | true | :personal_access_token | :forbidden 'PRIVATE' | :developer | true | :user_basic_auth | :success 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 44f924d8ae5..356e1e11def 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 @@ -13,7 +13,7 @@ RSpec.describe 'container repository details' do graphql_query_for( 'containerRepository', { id: container_repository_global_id }, - all_graphql_fields_for('ContainerRepositoryDetails') + all_graphql_fields_for('ContainerRepositoryDetails', excluded: ['pipeline']) ) end diff --git a/spec/requests/api/graphql/group/container_repositories_spec.rb b/spec/requests/api/graphql/group/container_repositories_spec.rb index 4aa775eba0f..939d7791d92 100644 --- a/spec/requests/api/graphql/group/container_repositories_spec.rb +++ b/spec/requests/api/graphql/group/container_repositories_spec.rb @@ -18,7 +18,7 @@ RSpec.describe 'getting container repositories in a group' do <<~GQL edges { node { - #{all_graphql_fields_for('container_repositories'.classify)} + #{all_graphql_fields_for('container_repositories'.classify, max_depth: 1)} } } GQL diff --git a/spec/requests/api/graphql/group/packages_spec.rb b/spec/requests/api/graphql/group/packages_spec.rb new file mode 100644 index 00000000000..85775598b2e --- /dev/null +++ b/spec/requests/api/graphql/group/packages_spec.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting a package list for a group' do + include GraphqlHelpers + + let_it_be(:resource) { create(:group, :private) } + let_it_be(:group_two) { create(:group, :private) } + let_it_be(:project) { create(:project, :repository, group: resource) } + let_it_be(:another_project) { create(:project, :repository, group: resource) } + let_it_be(:group_two_project) { create(:project, :repository, group: group_two) } + let_it_be(:current_user) { create(:user) } + + let_it_be(:package) { create(:package, project: project) } + let_it_be(:npm_package) { create(:npm_package, project: group_two_project) } + let_it_be(:maven_package) { create(:maven_package, project: project) } + let_it_be(:debian_package) { create(:debian_package, project: another_project) } + let_it_be(:composer_package) { create(:composer_package, project: another_project) } + let_it_be(:composer_metadatum) do + create(:composer_metadatum, package: composer_package, + target_sha: 'afdeh', + composer_json: { name: 'x', type: 'y', license: 'z', version: 1 }) + end + + let(:package_names) { graphql_data_at(:group, :packages, :nodes, :name) } + let(:target_shas) { graphql_data_at(:group, :packages, :nodes, :metadata, :target_sha) } + let(:packages) { graphql_data_at(:group, :packages, :nodes) } + + let(:fields) do + <<~QUERY + nodes { + #{all_graphql_fields_for('packages'.classify, excluded: ['project'])} + metadata { #{query_graphql_fragment('ComposerMetadata')} } + } + QUERY + end + + let(:query) do + graphql_query_for( + 'group', + { 'fullPath' => resource.full_path }, + query_graphql_field('packages', {}, fields) + ) + end + + it_behaves_like 'group and project packages query' + + context 'with a batched query' do + let(:batch_query) do + <<~QUERY + { + a: group(fullPath: "#{resource.full_path}") { packages { nodes { name } } } + b: group(fullPath: "#{group_two.full_path}") { packages { nodes { name } } } + } + QUERY + end + + let(:a_packages_names) { graphql_data_at(:a, :packages, :nodes, :name) } + + before do + resource.add_reporter(current_user) + group_two.add_reporter(current_user) + post_graphql(batch_query, current_user: current_user) + end + + it 'returns an error for the second group and data for the first' do + expect(a_packages_names).to contain_exactly( + package.name, + maven_package.name, + debian_package.name, + composer_package.name + ) + expect_graphql_errors_to_include [/Packages can be requested only for one group at a time/] + expect(graphql_data_at(:b, :packages)).to be(nil) + end + end +end diff --git a/spec/requests/api/graphql/issue/issue_spec.rb b/spec/requests/api/graphql/issue/issue_spec.rb index 09e89f65882..e8b8caf6c2d 100644 --- a/spec/requests/api/graphql/issue/issue_spec.rb +++ b/spec/requests/api/graphql/issue/issue_spec.rb @@ -8,10 +8,9 @@ RSpec.describe 'Query.issue(id)' do let_it_be(:project) { create(:project) } let_it_be(:issue) { create(:issue, project: project) } let_it_be(:current_user) { create(:user) } + let_it_be(:issue_params) { { 'id' => issue.to_global_id.to_s } } let(:issue_data) { graphql_data['issue'] } - - let_it_be(:issue_params) { { 'id' => issue.to_global_id.to_s } } let(:issue_fields) { all_graphql_fields_for('Issue'.classify) } let(:query) do @@ -62,7 +61,7 @@ RSpec.describe 'Query.issue(id)' do ) end - context 'selecting any single field' do + context 'when selecting any single field' do where(:field) do scalar_fields_of('Issue').map { |name| [name] } end @@ -84,13 +83,13 @@ RSpec.describe 'Query.issue(id)' do end end - context 'selecting multiple fields' do + context 'when selecting multiple fields' do let(:issue_fields) { ['title', 'description', 'updatedBy { username }'] } it 'returns the Issue with the specified fields' do post_graphql(query, current_user: current_user) - expect(issue_data.keys).to eq( %w(title description updatedBy) ) + expect(issue_data.keys).to eq %w[title description updatedBy] expect(issue_data['title']).to eq(issue.title) expect(issue_data['description']).to eq(issue.description) expect(issue_data['updatedBy']['username']).to eq(issue.author.username) @@ -110,14 +109,14 @@ RSpec.describe 'Query.issue(id)' do it 'returns correct attributes' do post_graphql(query, current_user: current_user) - expect(issue_data.keys).to eq( %w(moved movedTo) ) + expect(issue_data.keys).to eq %w[moved movedTo] expect(issue_data['moved']).to eq(true) expect(issue_data['movedTo']['title']).to eq(new_issue.title) end end context 'when passed a non-Issue gid' do - let(:mr) {create(:merge_request)} + let(:mr) { create(:merge_request) } it 'returns an error' do gid = mr.to_global_id.to_s diff --git a/spec/requests/api/graphql/mutations/alert_management/alerts/create_alert_issue_spec.rb b/spec/requests/api/graphql/mutations/alert_management/alerts/create_alert_issue_spec.rb index 6141a172253..f637ca98353 100644 --- a/spec/requests/api/graphql/mutations/alert_management/alerts/create_alert_issue_spec.rb +++ b/spec/requests/api/graphql/mutations/alert_management/alerts/create_alert_issue_spec.rb @@ -20,7 +20,9 @@ RSpec.describe 'Create an alert issue from an alert' do errors alert { iid - issueIid + issue { + iid + } } issue { iid @@ -46,7 +48,7 @@ RSpec.describe 'Create an alert issue from an alert' do expect(mutation_response.slice('alert', 'issue')).to eq( 'alert' => { 'iid' => alert.iid.to_s, - 'issueIid' => new_issue.iid.to_s + 'issue' => { 'iid' => new_issue.iid.to_s } }, 'issue' => { 'iid' => new_issue.iid.to_s, diff --git a/spec/requests/api/graphql/mutations/merge_requests/accept_spec.rb b/spec/requests/api/graphql/mutations/merge_requests/accept_spec.rb new file mode 100644 index 00000000000..2725b33d528 --- /dev/null +++ b/spec/requests/api/graphql/mutations/merge_requests/accept_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'accepting a merge request', :request_store do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:project) { create(:project, :public, :repository) } + let!(:merge_request) { create(:merge_request, source_project: project) } + let(:input) do + { + project_path: project.full_path, + iid: merge_request.iid.to_s, + sha: merge_request.diff_head_sha + } + end + + let(:mutation) { graphql_mutation(:merge_request_accept, input, 'mergeRequest { state }') } + let(:mutation_response) { graphql_mutation_response(:merge_request_accept) } + + context 'when the user is not allowed to accept a merge request' do + before do + project.add_reporter(current_user) + end + + it_behaves_like 'a mutation that returns a top-level access error' + end + + context 'when user has permissions to create a merge request' do + before do + project.add_maintainer(current_user) + end + + it 'merges the merge request' do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['mergeRequest']).to include( + 'state' => 'merged' + ) + end + end +end diff --git a/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb index 7dd897f6466..b5aaf304812 100644 --- a/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb +++ b/spec/requests/api/graphql/mutations/notes/create/diff_note_spec.rb @@ -9,8 +9,9 @@ RSpec.describe 'Adding a DiffNote' do let(:noteable) { create(:merge_request, source_project: project, target_project: project) } let(:project) { create(:project, :repository) } let(:diff_refs) { noteable.diff_refs } - let(:mutation) do - variables = { + + let(:base_variables) do + { noteable_id: GitlabSchema.id_from_object(noteable).to_s, body: 'Body text', position: { @@ -18,16 +19,16 @@ RSpec.describe 'Adding a DiffNote' do old_path: 'files/ruby/popen.rb', new_path: 'files/ruby/popen2.rb' }, - new_line: 14, base_sha: diff_refs.base_sha, head_sha: diff_refs.head_sha, start_sha: diff_refs.start_sha } } - - graphql_mutation(:create_diff_note, variables) end + let(:variables) { base_variables.deep_merge({ position: { new_line: 14 } }) } + let(:mutation) { graphql_mutation(:create_diff_note, variables) } + def mutation_response graphql_mutation_response(:create_diff_note) end @@ -41,6 +42,18 @@ RSpec.describe 'Adding a DiffNote' do it_behaves_like 'a Note mutation that creates a Note' + context 'add comment to old line' do + let(:variables) { base_variables.deep_merge({ position: { old_line: 14 } }) } + + it_behaves_like 'a Note mutation that creates a Note' + end + + context 'add a comment with a position without lines' do + let(:variables) { base_variables } + + it_behaves_like 'a Note mutation that does not create a Note' + end + it_behaves_like 'a Note mutation when there are active record validation errors', model: DiffNote it_behaves_like 'a Note mutation when there are rate limit validation errors' diff --git a/spec/requests/api/graphql/mutations/release_asset_links/create_spec.rb b/spec/requests/api/graphql/mutations/release_asset_links/create_spec.rb new file mode 100644 index 00000000000..c7a4cb1ebce --- /dev/null +++ b/spec/requests/api/graphql/mutations/release_asset_links/create_spec.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Creation of a new release asset link' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :private, :repository) } + let_it_be(:release) { create(:release, project: project, tag: 'v13.10') } + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + + let(:current_user) { developer } + + let(:mutation_name) { :release_asset_link_create } + + let(:mutation_arguments) do + { + projectPath: project.full_path, + tagName: release.tag, + name: 'awesome-app.dmg', + url: 'https://example.com/download/awesome-app.dmg', + directAssetPath: '/binaries/awesome-app.dmg', + linkType: 'PACKAGE' + } + end + + let(:mutation) do + graphql_mutation(mutation_name, mutation_arguments, <<~FIELDS) + link { + id + name + url + linkType + directAssetUrl + external + } + errors + FIELDS + end + + let(:create_link) { post_graphql_mutation(mutation, current_user: current_user) } + let(:mutation_response) { graphql_mutation_response(mutation_name)&.with_indifferent_access } + + it 'creates and returns a new asset link associated to the provided release', :aggregate_failures do + create_link + + expected_response = { + id: start_with("gid://gitlab/Releases::Link/"), + name: mutation_arguments[:name], + url: mutation_arguments[:url], + linkType: mutation_arguments[:linkType], + directAssetUrl: end_with(mutation_arguments[:directAssetPath]), + external: true + }.with_indifferent_access + + expect(mutation_response[:link]).to include(expected_response) + expect(mutation_response[:errors]).to eq([]) + end +end diff --git a/spec/requests/api/graphql/mutations/release_asset_links/update_spec.rb b/spec/requests/api/graphql/mutations/release_asset_links/update_spec.rb new file mode 100644 index 00000000000..92b558d4be3 --- /dev/null +++ b/spec/requests/api/graphql/mutations/release_asset_links/update_spec.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Updating an existing release asset link' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :private, :repository) } + let_it_be(:release) { create(:release, project: project) } + let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + + let_it_be(:release_link) do + create(:release_link, + release: release, + name: 'link name', + url: 'https://example.com/url', + filepath: '/permanent/path', + link_type: 'package') + end + + let(:current_user) { developer } + + let(:mutation_name) { :release_asset_link_update } + + let(:mutation_arguments) do + { + id: release_link.to_global_id.to_s, + name: 'updated name', + url: 'https://example.com/updated', + directAssetPath: '/updated/path', + linkType: 'IMAGE' + } + end + + let(:mutation) do + graphql_mutation(mutation_name, mutation_arguments, <<~FIELDS) + link { + id + name + url + linkType + directAssetUrl + external + } + errors + FIELDS + end + + let(:update_link) { post_graphql_mutation(mutation, current_user: current_user) } + let(:mutation_response) { graphql_mutation_response(mutation_name)&.with_indifferent_access } + + it 'updates and existing release asset link and returns the updated link', :aggregate_failures do + update_link + + expected_response = { + id: mutation_arguments[:id], + name: mutation_arguments[:name], + url: mutation_arguments[:url], + linkType: mutation_arguments[:linkType], + directAssetUrl: end_with(mutation_arguments[:directAssetPath]), + external: true + }.with_indifferent_access + + expect(mutation_response[:link]).to include(expected_response) + expect(mutation_response[:errors]).to eq([]) + end +end diff --git a/spec/requests/api/graphql/mutations/user_callouts/create_spec.rb b/spec/requests/api/graphql/mutations/user_callouts/create_spec.rb new file mode 100644 index 00000000000..cb67a60ebe4 --- /dev/null +++ b/spec/requests/api/graphql/mutations/user_callouts/create_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Create a user callout' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let(:feature_name) { ::UserCallout.feature_names.each_key.first } + + let(:input) do + { + 'featureName' => feature_name + } + end + + let(:mutation) { graphql_mutation(:userCalloutCreate, input) } + let(:mutation_response) { graphql_mutation_response(:userCalloutCreate) } + + it 'creates user callout' do + freeze_time do + post_graphql_mutation(mutation, current_user: current_user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['userCallout']['featureName']).to eq(feature_name.upcase) + expect(mutation_response['userCallout']['dismissedAt']).to eq(Time.current.iso8601) + end + end +end diff --git a/spec/requests/api/graphql/namespace/projects_spec.rb b/spec/requests/api/graphql/namespace/projects_spec.rb index 3e68503b7fb..414847c9c93 100644 --- a/spec/requests/api/graphql/namespace/projects_spec.rb +++ b/spec/requests/api/graphql/namespace/projects_spec.rb @@ -23,7 +23,7 @@ RSpec.describe 'getting projects' do projects(includeSubgroups: #{include_subgroups}) { edges { node { - #{all_graphql_fields_for('Project')} + #{all_graphql_fields_for('Project', max_depth: 1)} } } } diff --git a/spec/requests/api/graphql/packages/package_spec.rb b/spec/requests/api/graphql/packages/package_spec.rb index bb3ceb81f16..654215041cb 100644 --- a/spec/requests/api/graphql/packages/package_spec.rb +++ b/spec/requests/api/graphql/packages/package_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'package details' do end let(:depth) { 3 } - let(:excluded) { %w[metadata apiFuzzingCiConfiguration] } + let(:excluded) { %w[metadata apiFuzzingCiConfiguration pipeline] } let(:query) do graphql_query_for(:package, { id: package_global_id }, <<~FIELDS) diff --git a/spec/requests/api/graphql/project/alert_management/alert/issue_spec.rb b/spec/requests/api/graphql/project/alert_management/alert/issue_spec.rb new file mode 100644 index 00000000000..9724de4fedb --- /dev/null +++ b/spec/requests/api/graphql/project/alert_management/alert/issue_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting Alert Management Alert Issue' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user) } + let(:payload) { {} } + let(:query) { 'avg(metric) > 1.0' } + + let(:fields) do + <<~QUERY + nodes { + iid + issue { + iid + state + } + } + QUERY + end + + let(:graphql_query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('alertManagementAlerts', {}, fields) + ) + end + + let(:alerts) { graphql_data.dig('project', 'alertManagementAlerts', 'nodes') } + let(:first_alert) { alerts.first } + + before do + project.add_developer(current_user) + end + + context 'with gitlab alert' do + before do + create(:alert_management_alert, :with_issue, project: project, payload: payload) + end + + it 'includes the correct alert issue payload data' do + post_graphql(graphql_query, current_user: current_user) + + expect(first_alert).to include('issue' => { "iid" => "1", "state" => "opened" }) + end + end + + describe 'performance' do + let(:first_n) { var('Int') } + let(:params) { { first: first_n } } + let(:limited_query) { with_signature([first_n], query) } + + context 'with gitlab alert' do + before do + create(:alert_management_alert, :with_issue, project: project, payload: payload) + end + + it 'avoids N+1 queries' do + base_count = ActiveRecord::QueryRecorder.new do + post_graphql(limited_query, current_user: current_user, variables: first_n.with(1)) + end + + expect { post_graphql(limited_query, current_user: current_user) }.not_to exceed_query_limit(base_count) + end + end + end +end diff --git a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb index 8deed75a466..fe77d9dc86d 100644 --- a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'getting Alert Management Alerts' do let_it_be(:payload) { { 'custom' => { 'alert' => 'payload' }, 'runbook' => 'runbook' } } let_it_be(:project) { create(:project, :repository) } let_it_be(:current_user) { create(:user) } - let_it_be(:resolved_alert) { create(:alert_management_alert, :all_fields, :resolved, project: project, issue: nil, severity: :low).present } + let_it_be(:resolved_alert) { create(:alert_management_alert, :all_fields, :resolved, project: project, severity: :low).present } let_it_be(:triggered_alert) { create(:alert_management_alert, :all_fields, project: project, severity: :critical, payload: payload).present } let_it_be(:other_project_alert) { create(:alert_management_alert, :all_fields).present } @@ -60,7 +60,6 @@ RSpec.describe 'getting Alert Management Alerts' do it 'returns the correct properties of the alerts' do expect(first_alert).to include( 'iid' => triggered_alert.iid.to_s, - 'issueIid' => triggered_alert.issue_iid.to_s, 'title' => triggered_alert.title, 'description' => triggered_alert.description, 'severity' => triggered_alert.severity.upcase, @@ -82,7 +81,6 @@ RSpec.describe 'getting Alert Management Alerts' do expect(second_alert).to include( 'iid' => resolved_alert.iid.to_s, - 'issueIid' => resolved_alert.issue_iid.to_s, 'status' => 'RESOLVED', 'endedAt' => resolved_alert.ended_at.strftime('%Y-%m-%dT%H:%M:%SZ') ) diff --git a/spec/requests/api/graphql/project/container_repositories_spec.rb b/spec/requests/api/graphql/project/container_repositories_spec.rb index 2087d8c2cc3..5ffd48a7bc4 100644 --- a/spec/requests/api/graphql/project/container_repositories_spec.rb +++ b/spec/requests/api/graphql/project/container_repositories_spec.rb @@ -16,7 +16,7 @@ RSpec.describe 'getting container repositories in a project' do <<~GQL edges { node { - #{all_graphql_fields_for('container_repositories'.classify)} + #{all_graphql_fields_for('container_repositories'.classify, excluded: ['pipeline'])} } } GQL diff --git a/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb b/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb index dd16b052e0e..b1ecb32b365 100644 --- a/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb +++ b/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb @@ -34,7 +34,7 @@ RSpec.describe 'getting notes for a merge request' do notes { edges { node { - #{all_graphql_fields_for('Note')} + #{all_graphql_fields_for('Note', excluded: ['pipeline'])} } } } diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index a4e8d0bc35e..e32899c600e 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'getting merge request information nested in a project' do let(:current_user) { create(:user) } let(:merge_request_graphql_data) { graphql_data['project']['mergeRequest'] } let!(:merge_request) { create(:merge_request, source_project: project) } - let(:mr_fields) { all_graphql_fields_for('MergeRequest') } + let(:mr_fields) { all_graphql_fields_for('MergeRequest', excluded: ['pipeline']) } let(:query) do graphql_query_for( diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index d684be91dc9..d97a0ed9399 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -7,13 +7,27 @@ RSpec.describe 'getting merge request listings nested in a project' do let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:current_user) { create(:user) } - let_it_be(:label) { create(:label, project: project) } - let_it_be(:merge_request_a) { create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label]) } - let_it_be(:merge_request_b) { create(:merge_request, :closed, :unique_branches, source_project: project) } - let_it_be(:merge_request_c) { create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label]) } - let_it_be(:merge_request_d) { create(:merge_request, :locked, :unique_branches, source_project: project) } - let_it_be(:merge_request_e) { create(:merge_request, :unique_branches, source_project: project) } + + let_it_be(:merge_request_a) do + create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label]) + end + + let_it_be(:merge_request_b) do + create(:merge_request, :closed, :unique_branches, source_project: project) + end + + let_it_be(:merge_request_c) do + create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label]) + end + + let_it_be(:merge_request_d) do + create(:merge_request, :locked, :unique_branches, source_project: project) + end + + let_it_be(:merge_request_e) do + create(:merge_request, :unique_branches, source_project: project) + end let(:results) { graphql_data.dig('project', 'mergeRequests', 'nodes') } @@ -27,32 +41,38 @@ RSpec.describe 'getting merge request listings nested in a project' do ) end - let(:query) do - query_merge_requests(all_graphql_fields_for('MergeRequest', max_depth: 1)) - end - it_behaves_like 'a working graphql query' do + let(:query) do + query_merge_requests(all_graphql_fields_for('MergeRequest', max_depth: 2)) + end + before do - post_graphql(query, current_user: current_user) + # We cannot call the whitelist here, since the transaction does not + # begin until we enter the controller. + headers = { + 'X-GITLAB-QUERY-WHITELIST-ISSUE' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/322979' + } + + post_graphql(query, current_user: current_user, headers: headers) end end # The following tests are needed to guarantee that we have correctly annotated # all the gitaly calls. Selecting combinations of fields may mask this due to # memoization. - context 'requesting a single field' do + context 'when requesting a single field' do let_it_be(:fresh_mr) { create(:merge_request, :unique_branches, source_project: project) } + let(:search_params) { { iids: [fresh_mr.iid.to_s] } } + let(:graphql_data) do + GitlabSchema.execute(query, context: { current_user: current_user }).to_h['data'] + end before do project.repository.expire_branches_cache end - let(:graphql_data) do - GitlabSchema.execute(query, context: { current_user: current_user }).to_h['data'] - end - - context 'selecting any single scalar field' do + context 'when selecting any single scalar field' do where(:field) do scalar_fields_of('MergeRequest').map { |name| [name] } end @@ -68,7 +88,7 @@ RSpec.describe 'getting merge request listings nested in a project' do end end - context 'selecting any single nested field' do + context 'when selecting any single nested field' do where(:field, :subfield, :is_connection) do nested_fields_of('MergeRequest').flat_map do |name, field| type = field_type(field) @@ -95,7 +115,11 @@ RSpec.describe 'getting merge request listings nested in a project' do end end - shared_examples 'searching with parameters' do + shared_examples 'when searching with parameters' do + let(:query) do + query_merge_requests('iid title') + end + let(:expected) do mrs.map { |mr| a_hash_including('iid' => mr.iid.to_s, 'title' => mr.title) } end @@ -107,60 +131,60 @@ RSpec.describe 'getting merge request listings nested in a project' do end end - context 'there are no search params' do + context 'when there are no search params' do let(:search_params) { nil } let(:mrs) { [merge_request_a, merge_request_b, merge_request_c, merge_request_d, merge_request_e] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'the search params do not match anything' do - let(:search_params) { { iids: %w(foo bar baz) } } + context 'when the search params do not match anything' do + let(:search_params) { { iids: %w[foo bar baz] } } let(:mrs) { [] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by iids' do + context 'when searching by iids' do let(:search_params) { { iids: mrs.map(&:iid).map(&:to_s) } } let(:mrs) { [merge_request_a, merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by state' do + context 'when searching by state' do let(:search_params) { { state: :closed } } let(:mrs) { [merge_request_b, merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by source_branch' do + context 'when searching by source_branch' do let(:search_params) { { source_branches: mrs.map(&:source_branch) } } let(:mrs) { [merge_request_b, merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by target_branch' do + context 'when searching by target_branch' do let(:search_params) { { target_branches: mrs.map(&:target_branch) } } let(:mrs) { [merge_request_a, merge_request_d] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by label' do + context 'when searching by label' do let(:search_params) { { labels: [label.title] } } let(:mrs) { [merge_request_a, merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by combination' do + context 'when searching by combination' do let(:search_params) { { state: :closed, labels: [label.title] } } let(:mrs) { [merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end context 'when requesting `approved_by`' do @@ -203,10 +227,10 @@ RSpec.describe 'getting merge request listings nested in a project' do it 'exposes `commit_count`' do execute_query - expect(results).to match_array([ + expect(results).to match_array [ { "iid" => merge_request_a.iid.to_s, "commitCount" => 0 }, { "iid" => merge_request_with_commits.iid.to_s, "commitCount" => 29 } - ]) + ] end end @@ -216,8 +240,8 @@ RSpec.describe 'getting merge request listings nested in a project' do before do # make the MRs "merged" [merge_request_a, merge_request_b, merge_request_c].each do |mr| - mr.update_column(:state_id, MergeRequest.available_states[:merged]) - mr.metrics.update_column(:merged_at, Time.now) + mr.update!(state_id: MergeRequest.available_states[:merged]) + mr.metrics.update!(merged_at: Time.now) end end @@ -256,13 +280,12 @@ RSpec.describe 'getting merge request listings nested in a project' do end it 'returns the reviewers' do + nodes = merge_request_a.reviewers.map { |r| { 'username' => r.username } } + reviewers = { 'nodes' => match_array(nodes) } + execute_query - expect(results).to include a_hash_including('reviewers' => { - 'nodes' => match_array(merge_request_a.reviewers.map do |r| - a_hash_including('username' => r.username) - end) - }) + expect(results).to include a_hash_including('reviewers' => match(reviewers)) end include_examples 'N+1 query check' @@ -309,12 +332,14 @@ RSpec.describe 'getting merge request listings nested in a project' do allow(Gitlab::Database).to receive(:read_only?).and_return(false) end + def query_context + { current_user: current_user } + end + def run_query(number) # Ensure that we have a fresh request store and batch-context between runs - result = run_with_clean_state(query, - context: { current_user: current_user }, - variables: { first: number } - ) + vars = { first: number } + result = run_with_clean_state(query, context: query_context, variables: vars) graphql_dig_at(result.to_h, :data, :project, :merge_requests, :nodes) end @@ -348,39 +373,49 @@ RSpec.describe 'getting merge request listings nested in a project' do let(:data_path) { [:project, :mergeRequests] } def pagination_query(params) - graphql_query_for(:project, { full_path: project.full_path }, - <<~QUERY + graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY) mergeRequests(#{params}) { #{page_info} nodes { id } } - QUERY - ) + QUERY end context 'when sorting by merged_at DESC' do - it_behaves_like 'sorted paginated query' do - let(:sort_param) { :MERGED_AT_DESC } - let(:first_param) { 2 } + let(:sort_param) { :MERGED_AT_DESC } + let(:expected_results) do + [ + merge_request_b, + merge_request_d, + merge_request_c, + merge_request_e, + merge_request_a + ].map { |mr| global_id_of(mr) } + end - let(:expected_results) do - [ - merge_request_b, - merge_request_d, - merge_request_c, - merge_request_e, - merge_request_a - ].map { |mr| global_id_of(mr) } - end + before do + five_days_ago = 5.days.ago - before do - five_days_ago = 5.days.ago + merge_request_d.metrics.update!(merged_at: five_days_ago) + + # same merged_at, the second order column will decide (merge_request.id) + merge_request_c.metrics.update!(merged_at: five_days_ago) + + merge_request_b.metrics.update!(merged_at: 1.day.ago) + end + + it_behaves_like 'sorted paginated query' do + let(:first_param) { 2 } + end - merge_request_d.metrics.update!(merged_at: five_days_ago) + context 'when last parameter is given' do + let(:params) { graphql_args(sort: sort_param, last: 2) } + let(:page_info) { nil } - # same merged_at, the second order column will decide (merge_request.id) - merge_request_c.metrics.update!(merged_at: five_days_ago) + it 'takes the last 2 records' do + query = pagination_query(params) + post_graphql(query, current_user: current_user) - merge_request_b.metrics.update!(merged_at: 1.day.ago) + expect(results.map { |item| item["id"] }).to eq(expected_results.last(2)) end end end @@ -396,75 +431,45 @@ RSpec.describe 'getting merge request listings nested in a project' do let(:query) do # Note: __typename meta field is always requested by the FE - graphql_query_for(:project, { full_path: project.full_path }, - <<~QUERY + graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY) mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0, sourceBranches: null, labels: null) { count __typename } QUERY - ) end - shared_examples 'count examples' do - it 'returns the correct count' do - post_graphql(query, current_user: current_user) + it 'does not query the merge requests table for the count' do + query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - count = graphql_data.dig('project', 'mergeRequests', 'count') - expect(count).to eq(1) - end + queries = query_recorder.data.each_value.first[:occurrences] + expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) + expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) end - context 'when "optimized_merge_request_count_with_merged_at_filter" feature flag is enabled' do - before do - stub_feature_flags(optimized_merge_request_count_with_merged_at_filter: true) + context 'when total_time_to_merge and count is queried' do + let(:query) do + graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY) + mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0) { + totalTimeToMerge + count + } + QUERY end - it 'does not query the merge requests table for the count' do + it 'does not query the merge requests table for the total_time_to_merge' do query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } queries = query_recorder.data.each_value.first[:occurrences] - expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) - expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) + expect(queries).to include(match(/SELECT.+SUM.+FROM "merge_request_metrics" WHERE/)) end + end - context 'when total_time_to_merge and count is queried' do - let(:query) do - graphql_query_for(:project, { full_path: project.full_path }, - <<~QUERY - mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0) { - totalTimeToMerge - count - } - QUERY - ) - end - - it 'does not query the merge requests table for the total_time_to_merge' do - query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - - queries = query_recorder.data.each_value.first[:occurrences] - expect(queries).to include(match(/SELECT.+SUM.+FROM "merge_request_metrics" WHERE/)) - end - end - - it_behaves_like 'count examples' - - context 'when "optimized_merge_request_count_with_merged_at_filter" feature flag is disabled' do - before do - stub_feature_flags(optimized_merge_request_count_with_merged_at_filter: false) - end - - it 'queries the merge requests table for the count' do - query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - - queries = query_recorder.data.each_value.first[:occurrences] - expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) - expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) - end + it 'returns the correct count' do + post_graphql(query, current_user: current_user) - it_behaves_like 'count examples' - end + count = graphql_data.dig('project', 'mergeRequests', 'count') + expect(count).to eq(1) end end end diff --git a/spec/requests/api/graphql/project/packages_spec.rb b/spec/requests/api/graphql/project/packages_spec.rb index b20c96d54c8..3c04e0caf61 100644 --- a/spec/requests/api/graphql/project/packages_spec.rb +++ b/spec/requests/api/graphql/project/packages_spec.rb @@ -5,28 +5,28 @@ require 'spec_helper' RSpec.describe 'getting a package list for a project' do include GraphqlHelpers - let_it_be(:project) { create(:project, :repository) } + let_it_be(:resource) { create(:project, :repository) } let_it_be(:current_user) { create(:user) } - let_it_be(:package) { create(:package, project: project) } - let_it_be(:maven_package) { create(:maven_package, project: project) } - let_it_be(:debian_package) { create(:debian_package, project: project) } - let_it_be(:composer_package) { create(:composer_package, project: project) } + let_it_be(:package) { create(:package, project: resource) } + let_it_be(:maven_package) { create(:maven_package, project: resource) } + let_it_be(:debian_package) { create(:debian_package, project: resource) } + let_it_be(:composer_package) { create(:composer_package, project: resource) } let_it_be(:composer_metadatum) do create(:composer_metadatum, package: composer_package, target_sha: 'afdeh', composer_json: { name: 'x', type: 'y', license: 'z', version: 1 }) end - let(:package_names) { graphql_data_at(:project, :packages, :edges, :node, :name) } + let(:package_names) { graphql_data_at(:project, :packages, :nodes, :name) } + let(:target_shas) { graphql_data_at(:project, :packages, :nodes, :metadata, :target_sha) } + let(:packages) { graphql_data_at(:project, :packages, :nodes) } let(:fields) do <<~QUERY - edges { - node { - #{all_graphql_fields_for('packages'.classify, excluded: ['project'])} - metadata { #{query_graphql_fragment('ComposerMetadata')} } - } + nodes { + #{all_graphql_fields_for('packages'.classify, excluded: ['project'])} + metadata { #{query_graphql_fragment('ComposerMetadata')} } } QUERY end @@ -34,55 +34,10 @@ RSpec.describe 'getting a package list for a project' do let(:query) do graphql_query_for( 'project', - { 'fullPath' => project.full_path }, + { 'fullPath' => resource.full_path }, query_graphql_field('packages', {}, fields) ) end - context 'when user has access to the project' do - before do - project.add_reporter(current_user) - post_graphql(query, current_user: current_user) - end - - it_behaves_like 'a working graphql query' - - it 'returns packages successfully' do - expect(package_names).to contain_exactly( - package.name, - maven_package.name, - debian_package.name, - composer_package.name - ) - end - - it 'deals with metadata' do - target_shas = graphql_data_at(:project, :packages, :edges, :node, :metadata, :target_sha) - expect(target_shas).to contain_exactly(composer_metadatum.target_sha) - end - end - - context 'when the user does not have access to the project/packages' do - before do - post_graphql(query, current_user: current_user) - end - - it_behaves_like 'a working graphql query' - - it 'returns nil' do - expect(graphql_data['project']).to be_nil - end - end - - context 'when the user is not authenticated' do - before do - post_graphql(query) - end - - it_behaves_like 'a working graphql query' - - it 'returns nil' do - expect(graphql_data['project']).to be_nil - end - end + it_behaves_like 'group and project packages query' end diff --git a/spec/requests/api/graphql/project/pipeline_spec.rb b/spec/requests/api/graphql/project/pipeline_spec.rb index 6179b43629b..cc028ff2ff9 100644 --- a/spec/requests/api/graphql/project/pipeline_spec.rb +++ b/spec/requests/api/graphql/project/pipeline_spec.rb @@ -11,10 +11,14 @@ RSpec.describe 'getting pipeline information nested in a project' do let(:pipeline_graphql_data) { graphql_data['project']['pipeline'] } let!(:query) do - graphql_query_for( - 'project', - { 'fullPath' => project.full_path }, - query_graphql_field('pipeline', iid: pipeline.iid.to_s) + %( + query { + project(fullPath: "#{project.full_path}") { + pipeline(iid: "#{pipeline.iid}") { + configSource + } + } + } ) end diff --git a/spec/requests/api/graphql/snippets_spec.rb b/spec/requests/api/graphql/snippets_spec.rb new file mode 100644 index 00000000000..9edd805678a --- /dev/null +++ b/spec/requests/api/graphql/snippets_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'snippets' do + include GraphqlHelpers + + let_it_be(:current_user) { create(:user) } + let_it_be(:snippets) { create_list(:personal_snippet, 3, :repository, author: current_user) } + + describe 'querying for all fields' do + let(:query) do + graphql_query_for(:snippets, { ids: [global_id_of(snippets.first)] }, <<~SELECT) + nodes { #{all_graphql_fields_for('Snippet')} } + SELECT + end + + it 'can successfully query for snippets and their blobs' do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:snippets, :nodes)).to be_one + expect(graphql_data_at(:snippets, :nodes, :blobs, :nodes)).to be_present + end + end +end diff --git a/spec/requests/api/graphql/instance_statistics_measurements_spec.rb b/spec/requests/api/graphql/usage_trends_measurements_spec.rb index eb73dc59253..69a3ed7e09c 100644 --- a/spec/requests/api/graphql/instance_statistics_measurements_spec.rb +++ b/spec/requests/api/graphql/usage_trends_measurements_spec.rb @@ -2,22 +2,22 @@ require 'spec_helper' -RSpec.describe 'InstanceStatisticsMeasurements' do +RSpec.describe 'UsageTrendsMeasurements' do include GraphqlHelpers let(:current_user) { create(:user, :admin) } - let!(:instance_statistics_measurement_1) { create(:instance_statistics_measurement, :project_count, recorded_at: 20.days.ago, count: 5) } - let!(:instance_statistics_measurement_2) { create(:instance_statistics_measurement, :project_count, recorded_at: 10.days.ago, count: 10) } + let!(:usage_trends_measurement_1) { create(:usage_trends_measurement, :project_count, recorded_at: 20.days.ago, count: 5) } + let!(:usage_trends_measurement_2) { create(:usage_trends_measurement, :project_count, recorded_at: 10.days.ago, count: 10) } let(:arguments) { 'identifier: PROJECTS' } - let(:query) { graphql_query_for(:instanceStatisticsMeasurements, arguments, 'nodes { count identifier }') } + let(:query) { graphql_query_for(:UsageTrendsMeasurements, arguments, 'nodes { count identifier }') } before do post_graphql(query, current_user: current_user) end it 'returns measurement objects' do - expect(graphql_data.dig('instanceStatisticsMeasurements', 'nodes')).to eq([ + expect(graphql_data.dig('usageTrendsMeasurements', 'nodes')).to eq([ { "count" => 10, 'identifier' => 'PROJECTS' }, { "count" => 5, 'identifier' => 'PROJECTS' } ]) @@ -27,7 +27,7 @@ RSpec.describe 'InstanceStatisticsMeasurements' do let(:arguments) { %(identifier: PROJECTS, recordedAfter: "#{15.days.ago.to_date}", recordedBefore: "#{5.days.ago.to_date}") } it 'returns filtered measurement objects' do - expect(graphql_data.dig('instanceStatisticsMeasurements', 'nodes')).to eq([ + expect(graphql_data.dig('usageTrendsMeasurements', 'nodes')).to eq([ { "count" => 10, 'identifier' => 'PROJECTS' } ]) end diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index 91d10791541..8160a94aef2 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -314,14 +314,13 @@ RSpec.describe API::Helpers do expect(Gitlab::ErrorTracking).to receive(:sentry_dsn).and_return(Gitlab.config.sentry.dsn) Gitlab::ErrorTracking.configure - Raven.client.configuration.encoding = 'json' end it 'does not report a MethodNotAllowed exception to Sentry' do exception = Grape::Exceptions::MethodNotAllowed.new({ 'X-GitLab-Test' => '1' }) allow(exception).to receive(:backtrace).and_return(caller) - expect(Raven).not_to receive(:capture_exception).with(exception) + expect(Gitlab::ErrorTracking).not_to receive(:track_exception).with(exception) handle_api_exception(exception) end @@ -330,8 +329,7 @@ RSpec.describe API::Helpers do exception = RuntimeError.new('test error') allow(exception).to receive(:backtrace).and_return(caller) - expect(Raven).to receive(:capture_exception).with(exception, tags: - a_hash_including(correlation_id: 'new-correlation-id'), extra: {}) + expect(Gitlab::ErrorTracking).to receive(:track_exception).with(exception) Labkit::Correlation::CorrelationId.use_id('new-correlation-id') do handle_api_exception(exception) @@ -357,20 +355,6 @@ RSpec.describe API::Helpers do expect(json_response['message']).to start_with("\nRuntimeError (Runtime Error!):") end end - - context 'extra information' do - # Sentry events are an array of the form [auth_header, data, options] - let(:event_data) { Raven.client.transport.events.first[1] } - - it 'sends the params, excluding confidential values' do - expect(ProjectsFinder).to receive(:new).and_raise('Runtime Error!') - - get api('/projects', user), params: { password: 'dont_send_this', other_param: 'send_this' } - - expect(event_data).to include('other_param=send_this') - expect(event_data).to include('password=********') - end - end end describe '.authenticate_non_get!' do diff --git a/spec/requests/api/invitations_spec.rb b/spec/requests/api/invitations_spec.rb index 2ea237469b1..98a7aa63b16 100644 --- a/spec/requests/api/invitations_spec.rb +++ b/spec/requests/api/invitations_spec.rb @@ -3,14 +3,14 @@ require 'spec_helper' RSpec.describe API::Invitations do - let(:maintainer) { create(:user, username: 'maintainer_user') } - let(:developer) { create(:user) } - let(:access_requester) { create(:user) } - let(:stranger) { create(:user) } + let_it_be(:maintainer) { create(:user, username: 'maintainer_user') } + let_it_be(:developer) { create(:user) } + let_it_be(:access_requester) { create(:user) } + let_it_be(:stranger) { create(:user) } let(:email) { 'email1@example.com' } let(:email2) { 'email2@example.com' } - let(:project) do + let_it_be(:project) do create(:project, :public, creator_id: maintainer.id, namespace: maintainer.namespace) do |project| project.add_developer(developer) project.add_maintainer(maintainer) @@ -18,7 +18,7 @@ RSpec.describe API::Invitations do end end - let!(:group) do + let_it_be(:group, reload: true) do create(:group, :public) do |group| group.add_developer(developer) group.add_owner(maintainer) @@ -374,4 +374,104 @@ RSpec.describe API::Invitations do let(:source) { group } end end + + shared_examples 'PUT /:source_type/:id/invitations/:email' do |source_type| + def update_api(source, user, email) + api("/#{source.model_name.plural}/#{source.id}/invitations/#{email}", user) + end + + context "with :source_type == #{source_type.pluralize}" do + let!(:invite) { invite_member_by_email(source, source_type, developer.email, maintainer) } + + it_behaves_like 'a 404 response when source is private' do + let(:route) do + put update_api(source, stranger, invite.invite_email), params: { access_level: Member::MAINTAINER } + end + end + + context 'when authenticated as a non-member or member with insufficient rights' do + %i[access_requester stranger].each do |type| + context "as a #{type}" do + it 'returns 403' do + user = public_send(type) + + put update_api(source, user, invite.invite_email), params: { access_level: Member::MAINTAINER } + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + end + end + + context 'when authenticated as a maintainer/owner' do + context 'updating access level' do + it 'updates the invitation' do + put update_api(source, maintainer, invite.invite_email), params: { access_level: Member::MAINTAINER } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['access_level']).to eq(Member::MAINTAINER) + expect(invite.reload.access_level).to eq(Member::MAINTAINER) + end + end + + it 'returns 409 if member does not exist' do + put update_api(source, maintainer, non_existing_record_id), params: { access_level: Member::MAINTAINER } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it 'returns 400 when access_level is not given and there are no other params' do + put update_api(source, maintainer, invite.invite_email) + + expect(response).to have_gitlab_http_status(:bad_request) + end + + it 'returns 400 when access level is not valid' do + put update_api(source, maintainer, invite.invite_email), params: { access_level: non_existing_record_access_level } + + expect(response).to have_gitlab_http_status(:bad_request) + end + end + + context 'updating access expiry date' do + subject do + put update_api(source, maintainer, invite.invite_email), params: { expires_at: expires_at } + end + + context 'when set to a date in the past' do + let(:expires_at) { 2.days.ago.to_date } + + it 'does not update the member' do + subject + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq({ 'expires_at' => ['cannot be a date in the past'] }) + end + end + + context 'when set to a date in the future' do + let(:expires_at) { 2.days.from_now.to_date } + + it 'updates the member' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['expires_at']).to eq(expires_at.to_s) + end + end + end + end + end + + describe 'PUT /projects/:id/invitations' do + it_behaves_like 'PUT /:source_type/:id/invitations/:email', 'project' do + let(:source) { project } + end + end + + describe 'PUT /groups/:id/invitations' do + it_behaves_like 'PUT /:source_type/:id/invitations/:email', 'group' do + let(:source) { group } + end + end end diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb index 1c43ef25f14..fe00b654d3b 100644 --- a/spec/requests/api/jobs_spec.rb +++ b/spec/requests/api/jobs_spec.rb @@ -3,6 +3,9 @@ require 'spec_helper' RSpec.describe API::Jobs do + include HttpBasicAuthHelpers + include DependencyProxyHelpers + using RSpec::Parameterized::TableSyntax include HttpIOHelpers @@ -16,20 +19,151 @@ RSpec.describe API::Jobs do ref: project.default_branch) end - let!(:job) do - create(:ci_build, :success, :tags, pipeline: pipeline, - artifacts_expire_at: 1.day.since) - end - let(:user) { create(:user) } let(:api_user) { user } let(:reporter) { create(:project_member, :reporter, project: project).user } let(:guest) { create(:project_member, :guest, project: project).user } + let(:running_job) do + create(:ci_build, :running, project: project, + user: user, + pipeline: pipeline, + artifacts_expire_at: 1.day.since) + end + + let!(:job) do + create(:ci_build, :success, :tags, pipeline: pipeline, + artifacts_expire_at: 1.day.since) + end + before do project.add_developer(user) end + shared_examples 'returns common pipeline data' do + it 'returns common pipeline data' do + expect(json_response['pipeline']).not_to be_empty + expect(json_response['pipeline']['id']).to eq jobx.pipeline.id + expect(json_response['pipeline']['project_id']).to eq jobx.pipeline.project_id + expect(json_response['pipeline']['ref']).to eq jobx.pipeline.ref + expect(json_response['pipeline']['sha']).to eq jobx.pipeline.sha + expect(json_response['pipeline']['status']).to eq jobx.pipeline.status + end + end + + shared_examples 'returns common job data' do + it 'returns common job data' do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['id']).to eq(jobx.id) + expect(json_response['status']).to eq(jobx.status) + expect(json_response['stage']).to eq(jobx.stage) + expect(json_response['name']).to eq(jobx.name) + expect(json_response['ref']).to eq(jobx.ref) + expect(json_response['tag']).to eq(jobx.tag) + expect(json_response['coverage']).to eq(jobx.coverage) + expect(json_response['allow_failure']).to eq(jobx.allow_failure) + expect(Time.parse(json_response['created_at'])).to be_like_time(jobx.created_at) + expect(Time.parse(json_response['started_at'])).to be_like_time(jobx.started_at) + expect(Time.parse(json_response['artifacts_expire_at'])).to be_like_time(jobx.artifacts_expire_at) + expect(json_response['artifacts_file']).to be_nil + expect(json_response['artifacts']).to be_an Array + expect(json_response['artifacts']).to be_empty + expect(json_response['web_url']).to be_present + end + end + + shared_examples 'returns unauthorized' do + it 'returns unauthorized' do + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + describe 'GET /job' do + shared_context 'with auth headers' do + let(:headers_with_token) { header } + let(:params_with_token) { {} } + end + + shared_context 'with auth params' do + let(:headers_with_token) { {} } + let(:params_with_token) { param } + end + + shared_context 'without auth' do + let(:headers_with_token) { {} } + let(:params_with_token) { {} } + end + + before do |example| + unless example.metadata[:skip_before_request] + get api('/job'), headers: headers_with_token, params: params_with_token + end + end + + context 'with job token authentication header' do + include_context 'with auth headers' do + let(:header) { { API::Helpers::Runner::JOB_TOKEN_HEADER => running_job.token } } + end + + it_behaves_like 'returns common job data' do + let(:jobx) { running_job } + end + + it 'returns specific job data' do + expect(json_response['finished_at']).to be_nil + end + + it_behaves_like 'returns common pipeline data' do + let(:jobx) { running_job } + end + end + + context 'with job token authentication params' do + include_context 'with auth params' do + let(:param) { { job_token: running_job.token } } + end + + it_behaves_like 'returns common job data' do + let(:jobx) { running_job } + end + + it 'returns specific job data' do + expect(json_response['finished_at']).to be_nil + end + + it_behaves_like 'returns common pipeline data' do + let(:jobx) { running_job } + end + end + + context 'with non running job' do + include_context 'with auth headers' do + let(:header) { { API::Helpers::Runner::JOB_TOKEN_HEADER => job.token } } + end + + it_behaves_like 'returns unauthorized' + end + + context 'with basic auth header' do + let(:personal_access_token) { create(:personal_access_token, user: user) } + let(:token) { personal_access_token.token} + + include_context 'with auth headers' do + let(:header) { { Gitlab::Auth::AuthFinders::PRIVATE_TOKEN_HEADER => token } } + end + + it 'does not return a job' do + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'without authentication' do + include_context 'without auth' + + it_behaves_like 'returns unauthorized' + end + end + describe 'GET /projects/:id/jobs' do let(:query) { {} } @@ -150,39 +284,21 @@ RSpec.describe API::Jobs do end context 'authorized user' do + it_behaves_like 'returns common job data' do + let(:jobx) { job } + end + it 'returns specific job data' do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['id']).to eq(job.id) - expect(json_response['status']).to eq(job.status) - expect(json_response['stage']).to eq(job.stage) - expect(json_response['name']).to eq(job.name) - expect(json_response['ref']).to eq(job.ref) - expect(json_response['tag']).to eq(job.tag) - expect(json_response['coverage']).to eq(job.coverage) - expect(json_response['allow_failure']).to eq(job.allow_failure) - expect(Time.parse(json_response['created_at'])).to be_like_time(job.created_at) - expect(Time.parse(json_response['started_at'])).to be_like_time(job.started_at) expect(Time.parse(json_response['finished_at'])).to be_like_time(job.finished_at) - expect(Time.parse(json_response['artifacts_expire_at'])).to be_like_time(job.artifacts_expire_at) - expect(json_response['artifacts_file']).to be_nil - expect(json_response['artifacts']).to be_an Array - expect(json_response['artifacts']).to be_empty expect(json_response['duration']).to eq(job.duration) - expect(json_response['web_url']).to be_present end it_behaves_like 'a job with artifacts and trace', result_is_array: false do let(:api_endpoint) { "/projects/#{project.id}/jobs/#{second_job.id}" } end - it 'returns pipeline data' do - json_job = json_response - - expect(json_job['pipeline']).not_to be_empty - expect(json_job['pipeline']['id']).to eq job.pipeline.id - expect(json_job['pipeline']['ref']).to eq job.pipeline.ref - expect(json_job['pipeline']['sha']).to eq job.pipeline.sha - expect(json_job['pipeline']['status']).to eq job.pipeline.status + it_behaves_like 'returns common pipeline data' do + let(:jobx) { job } end end @@ -329,6 +445,17 @@ RSpec.describe API::Jobs do .to include('Content-Type' => 'application/json', 'Gitlab-Workhorse-Send-Data' => /artifacts-entry/) end + + context 'when artifacts are locked' do + it 'allows access to expired artifact' do + pipeline.artifacts_locked! + job.update!(artifacts_expire_at: Time.now - 7.days) + + get_artifact_file(artifact) + + expect(response).to have_gitlab_http_status(:ok) + end + end end end diff --git a/spec/requests/api/lint_spec.rb b/spec/requests/api/lint_spec.rb index b5bf697e9e3..cf8cac773f5 100644 --- a/spec/requests/api/lint_spec.rb +++ b/spec/requests/api/lint_spec.rb @@ -406,6 +406,24 @@ RSpec.describe API::Lint do end end + context 'with an empty repository' do + let_it_be(:empty_project) { create(:project_empty_repo) } + let_it_be(:yaml_content) do + File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml')) + end + + before do + empty_project.add_developer(api_user) + end + + it 'passes validation without errors' do + post api("/projects/#{empty_project.id}/ci/lint", api_user), params: { content: yaml_content } + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['valid']).to eq(true) + expect(json_response['errors']).to eq([]) + end + end + context 'when unauthenticated' do let_it_be(:api_user) { nil } diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index ad8e21bf4c1..09177dd1710 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -2537,7 +2537,7 @@ RSpec.describe API::MergeRequests do end end - describe "the should_remove_source_branch param" do + describe "the should_remove_source_branch param", :sidekiq_inline do let(:source_repository) { merge_request.source_project.repository } let(:source_branch) { merge_request.source_branch } @@ -2552,7 +2552,7 @@ RSpec.describe API::MergeRequests do end end - context "with a merge request that has force_remove_source_branch enabled" do + context "with a merge request that has force_remove_source_branch enabled", :sidekiq_inline do let(:source_repository) { merge_request.source_project.repository } let(:source_branch) { merge_request.source_branch } diff --git a/spec/requests/api/npm_instance_packages_spec.rb b/spec/requests/api/npm_instance_packages_spec.rb index 70c76067a6e..698885ddcf4 100644 --- a/spec/requests/api/npm_instance_packages_spec.rb +++ b/spec/requests/api/npm_instance_packages_spec.rb @@ -3,6 +3,11 @@ require 'spec_helper' RSpec.describe API::NpmInstancePackages do + # We need to create a subgroup with the same name as the hosting group. + # It has to be created first to exhibit this bug: https://gitlab.com/gitlab-org/gitlab/-/issues/321958 + let_it_be(:another_namespace) { create(:group, :public) } + let_it_be(:similarly_named_group) { create(:group, :public, parent: another_namespace, name: 'test-group') } + include_context 'npm api setup' describe 'GET /api/v4/packages/npm/*package_name' do diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index 7ea238c0607..e64b5ddc374 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -30,7 +30,7 @@ RSpec.describe API::NpmProjectPackages do end describe 'GET /api/v4/projects/:id/packages/npm/*package_name/-/*file_name' do - let_it_be(:package_file) { package.package_files.first } + let(:package_file) { package.package_files.first } let(:headers) { {} } let(:url) { api("/projects/#{project.id}/packages/npm/#{package.name}/-/#{package_file.file_name}") } @@ -127,24 +127,6 @@ RSpec.describe API::NpmProjectPackages do context 'when params are correct' do context 'invalid package record' do - context 'unscoped package' do - let(:package_name) { 'my_unscoped_package' } - let(:params) { upload_params(package_name: package_name) } - - it_behaves_like 'handling invalid record with 400 error' - - context 'with empty versions' do - let(:params) { upload_params(package_name: package_name).merge!(versions: {}) } - - it 'throws a 400 error' do - expect { upload_package_with_token(package_name, params) } - .not_to change { project.packages.count } - - expect(response).to have_gitlab_http_status(:bad_request) - end - end - end - context 'invalid package name' do let(:package_name) { "@#{group.path}/my_inv@@lid_package_name" } let(:params) { upload_params(package_name: package_name) } @@ -175,52 +157,71 @@ RSpec.describe API::NpmProjectPackages do end end - context 'scoped package' do - let(:package_name) { "@#{group.path}/my_package_name" } + context 'valid package record' do let(:params) { upload_params(package_name: package_name) } - context 'with access token' do - subject { upload_package_with_token(package_name, params) } + shared_examples 'handling upload with different authentications' do + context 'with access token' do + subject { upload_package_with_token(package_name, params) } + + it_behaves_like 'a package tracking event', 'API::NpmPackages', 'push_package' + + it 'creates npm package with file' do + expect { subject } + .to change { project.packages.count }.by(1) + .and change { Packages::PackageFile.count }.by(1) + .and change { Packages::Tag.count }.by(1) - it_behaves_like 'a package tracking event', 'API::NpmPackages', 'push_package' + expect(response).to have_gitlab_http_status(:ok) + end + end - it 'creates npm package with file' do - expect { subject } + it 'creates npm package with file with job token' do + expect { upload_package_with_job_token(package_name, params) } .to change { project.packages.count }.by(1) .and change { Packages::PackageFile.count }.by(1) - .and change { Packages::Tag.count }.by(1) expect(response).to have_gitlab_http_status(:ok) end - end - it 'creates npm package with file with job token' do - expect { upload_package_with_job_token(package_name, params) } - .to change { project.packages.count }.by(1) - .and change { Packages::PackageFile.count }.by(1) + context 'with an authenticated job token' do + let!(:job) { create(:ci_build, user: user) } - expect(response).to have_gitlab_http_status(:ok) - end + before do + Grape::Endpoint.before_each do |endpoint| + expect(endpoint).to receive(:current_authenticated_job) { job } + end + end - context 'with an authenticated job token' do - let!(:job) { create(:ci_build, user: user) } + after do + Grape::Endpoint.before_each nil + end - before do - Grape::Endpoint.before_each do |endpoint| - expect(endpoint).to receive(:current_authenticated_job) { job } + it 'creates the package metadata' do + upload_package_with_token(package_name, params) + + expect(response).to have_gitlab_http_status(:ok) + expect(project.reload.packages.find(json_response['id']).original_build_info.pipeline).to eq job.pipeline end end + end - after do - Grape::Endpoint.before_each nil - end + context 'with a scoped name' do + let(:package_name) { "@#{group.path}/my_package_name" } - it 'creates the package metadata' do - upload_package_with_token(package_name, params) + it_behaves_like 'handling upload with different authentications' + end - expect(response).to have_gitlab_http_status(:ok) - expect(project.reload.packages.find(json_response['id']).original_build_info.pipeline).to eq job.pipeline - end + context 'with any scoped name' do + let(:package_name) { "@any_scope/my_package_name" } + + it_behaves_like 'handling upload with different authentications' + end + + context 'with an unscoped name' do + let(:package_name) { "my_unscoped_package_name" } + + it_behaves_like 'handling upload with different authentications' end end diff --git a/spec/requests/api/oauth_tokens_spec.rb b/spec/requests/api/oauth_tokens_spec.rb index 52c7408545f..edadfbc3d0c 100644 --- a/spec/requests/api/oauth_tokens_spec.rb +++ b/spec/requests/api/oauth_tokens_spec.rb @@ -27,13 +27,13 @@ RSpec.describe 'OAuth tokens' do context 'when user does not have 2FA enabled' do context 'when no client credentials provided' do - it 'does not create an access token' do + it 'creates an access token' do user = create(:user) request_oauth_token(user) - expect(response).to have_gitlab_http_status(:unauthorized) - expect(json_response['access_token']).to be_nil + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['access_token']).to be_present end end @@ -51,6 +51,8 @@ RSpec.describe 'OAuth tokens' do context 'with invalid credentials' do it 'does not create an access token' do + pending 'Enable this example after https://github.com/doorkeeper-gem/doorkeeper/pull/1488 is merged and released' + user = create(:user) request_oauth_token(user, basic_auth_header(client.uid, 'invalid secret')) diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 181fcafd577..6c9a845b217 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -12,7 +12,6 @@ itself: # project - import_source - import_type - import_url - - issues_template - jobs_cache_index - last_repository_check_at - last_repository_check_failed @@ -24,7 +23,6 @@ itself: # project - merge_requests_author_approval - merge_requests_disable_committers_approval - merge_requests_rebase_enabled - - merge_requests_template - mirror_last_successful_update_at - mirror_last_update_at - mirror_overwrites_diverged_branches @@ -56,6 +54,7 @@ itself: # project - can_create_merge_request_in - compliance_frameworks - container_expiration_policy + - container_registry_image_prefix - default_branch - empty_repo - forks_count @@ -117,6 +116,7 @@ project_feature: - project_id - requirements_access_level - security_and_compliance_access_level + - container_registry_access_level - updated_at computed_attributes: - issues_enabled @@ -139,6 +139,7 @@ project_setting: - show_default_award_emojis - squash_option - updated_at + - cve_id_request_enabled build_service_desk_setting: # service_desk_setting unexposed_attributes: diff --git a/spec/requests/api/project_packages_spec.rb b/spec/requests/api/project_packages_spec.rb index 1f3887cab8a..97414b3b18a 100644 --- a/spec/requests/api/project_packages_spec.rb +++ b/spec/requests/api/project_packages_spec.rb @@ -257,6 +257,10 @@ RSpec.describe API::ProjectPackages do context 'project is private' do let(:project) { create(:project, :private) } + before do + expect(::Packages::Maven::Metadata::SyncWorker).not_to receive(:perform_async) + end + it 'returns 404 for non authenticated user' do delete api(package_url) @@ -301,6 +305,19 @@ RSpec.describe API::ProjectPackages do expect(response).to have_gitlab_http_status(:no_content) end end + + context 'with a maven package' do + let_it_be(:package1) { create(:maven_package, project: project) } + + it 'enqueues a sync worker job' do + project.add_maintainer(user) + + expect(::Packages::Maven::Metadata::SyncWorker) + .to receive(:perform_async).with(user.id, project.id, package1.name) + + delete api(package_url, user) + end + end end end end diff --git a/spec/requests/api/project_repository_storage_moves_spec.rb b/spec/requests/api/project_repository_storage_moves_spec.rb index 5e200312d1f..b40645ba2de 100644 --- a/spec/requests/api/project_repository_storage_moves_spec.rb +++ b/spec/requests/api/project_repository_storage_moves_spec.rb @@ -7,6 +7,6 @@ RSpec.describe API::ProjectRepositoryStorageMoves do let_it_be(:container) { create(:project, :repository).tap { |project| project.track_project_repository } } let_it_be(:storage_move) { create(:project_repository_storage_move, :scheduled, container: container) } let(:repository_storage_move_factory) { :project_repository_storage_move } - let(:bulk_worker_klass) { ProjectScheduleBulkRepositoryShardMovesWorker } + let(:bulk_worker_klass) { Projects::ScheduleBulkRepositoryShardMovesWorker } end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index ad36777184a..d2a33e32b30 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -1478,6 +1478,120 @@ RSpec.describe API::Projects do end end + describe "GET /projects/:id/groups" do + let_it_be(:root_group) { create(:group, :public, name: 'root group') } + let_it_be(:project_group) { create(:group, :public, parent: root_group, name: 'project group') } + let_it_be(:shared_group_with_dev_access) { create(:group, :private, parent: root_group, name: 'shared group') } + let_it_be(:shared_group_with_reporter_access) { create(:group, :private) } + let_it_be(:private_project) { create(:project, :private, group: project_group) } + let_it_be(:public_project) { create(:project, :public, group: project_group) } + + before_all do + create(:project_group_link, :developer, group: shared_group_with_dev_access, project: private_project) + create(:project_group_link, :reporter, group: shared_group_with_reporter_access, project: private_project) + end + + shared_examples_for 'successful groups response' do + it 'returns an array of groups' do + request + + aggregate_failures do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.map { |g| g['name'] }).to match_array(expected_groups.map(&:name)) + end + end + end + + context 'when unauthenticated' do + it 'does not return groups for private projects' do + get api("/projects/#{private_project.id}/groups") + + expect(response).to have_gitlab_http_status(:not_found) + end + + context 'for public projects' do + let(:request) { get api("/projects/#{public_project.id}/groups") } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group] } + end + end + end + + context 'when authenticated as user' do + context 'when user does not have access to the project' do + it 'does not return groups' do + get api("/projects/#{private_project.id}/groups", user) + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'when user has access to the project' do + let(:request) { get api("/projects/#{private_project.id}/groups", user), params: params } + let(:params) { {} } + + before do + private_project.add_developer(user) + end + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group] } + end + + context 'when search by root group name' do + let(:params) { { search: 'root' } } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group] } + end + end + + context 'with_shared option is on' do + let(:params) { { with_shared: true } } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group, shared_group_with_dev_access, shared_group_with_reporter_access] } + end + + context 'when shared_min_access_level is set' do + let(:params) { super().merge(shared_min_access_level: Gitlab::Access::DEVELOPER) } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group, shared_group_with_dev_access] } + end + end + + context 'when search by shared group name' do + let(:params) { super().merge(search: 'shared') } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [shared_group_with_dev_access] } + end + end + + context 'when skip_groups is set' do + let(:params) { super().merge(skip_groups: [shared_group_with_dev_access.id, root_group.id]) } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [shared_group_with_reporter_access, project_group] } + end + end + end + end + end + + context 'when authenticated as admin' do + let(:request) { get api("/projects/#{private_project.id}/groups", admin) } + + it_behaves_like 'successful groups response' do + let(:expected_groups) { [root_group, project_group] } + end + end + end + describe 'GET /projects/:id' do context 'when unauthenticated' do it 'does not return private projects' do @@ -1540,6 +1654,10 @@ RSpec.describe API::Projects do end context 'when authenticated as an admin' do + before do + stub_container_registry_config(enabled: true, host_port: 'registry.example.org:5000') + end + let(:project_attributes_file) { 'spec/requests/api/project_attributes.yml' } let(:project_attributes) { YAML.load_file(project_attributes_file) } @@ -1563,13 +1681,15 @@ RSpec.describe API::Projects do mirror requirements_enabled security_and_compliance_enabled + issues_template + merge_requests_template ] end keys end - it 'returns a project by id' do + it 'returns a project by id', :aggregate_failures do project project_member group = create(:group) @@ -1587,6 +1707,7 @@ RSpec.describe API::Projects do expect(json_response['ssh_url_to_repo']).to be_present expect(json_response['http_url_to_repo']).to be_present expect(json_response['web_url']).to be_present + expect(json_response['container_registry_image_prefix']).to eq("registry.example.org:5000/#{project.full_path}") expect(json_response['owner']).to be_a Hash expect(json_response['name']).to eq(project.name) expect(json_response['path']).to be_present @@ -1644,9 +1765,10 @@ RSpec.describe API::Projects do before do project project_member + stub_container_registry_config(enabled: true, host_port: 'registry.example.org:5000') end - it 'returns a project by id' do + it 'returns a project by id', :aggregate_failures do group = create(:group) link = create(:project_group_link, project: project, group: group) @@ -1662,6 +1784,7 @@ RSpec.describe API::Projects do expect(json_response['ssh_url_to_repo']).to be_present expect(json_response['http_url_to_repo']).to be_present expect(json_response['web_url']).to be_present + expect(json_response['container_registry_image_prefix']).to eq("registry.example.org:5000/#{project.full_path}") expect(json_response['owner']).to be_a Hash expect(json_response['name']).to eq(project.name) expect(json_response['path']).to be_present @@ -2818,7 +2941,7 @@ RSpec.describe API::Projects do Sidekiq::Testing.fake! do put(api("/projects/#{new_project.id}", user), params: { repository_storage: unknown_storage, issues_enabled: false }) end - end.not_to change(ProjectUpdateRepositoryStorageWorker.jobs, :size) + end.not_to change(Projects::UpdateRepositoryStorageWorker.jobs, :size) expect(response).to have_gitlab_http_status(:ok) expect(json_response['issues_enabled']).to eq(false) @@ -2845,7 +2968,7 @@ RSpec.describe API::Projects do Sidekiq::Testing.fake! do put(api("/projects/#{new_project.id}", admin), params: { repository_storage: 'test_second_storage' }) end - end.to change(ProjectUpdateRepositoryStorageWorker.jobs, :size).by(1) + end.to change(Projects::UpdateRepositoryStorageWorker.jobs, :size).by(1) expect(response).to have_gitlab_http_status(:ok) end diff --git a/spec/requests/api/protected_branches_spec.rb b/spec/requests/api/protected_branches_spec.rb index 8bcd493eb1f..6b1aa576167 100644 --- a/spec/requests/api/protected_branches_spec.rb +++ b/spec/requests/api/protected_branches_spec.rb @@ -68,6 +68,7 @@ RSpec.describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(branch_name) + expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(::Gitlab::Access::MAINTAINER) expect(json_response['merge_access_levels'][0]['access_level']).to eq(::Gitlab::Access::MAINTAINER) end @@ -132,6 +133,7 @@ RSpec.describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(branch_name) + expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) end @@ -141,6 +143,7 @@ RSpec.describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(branch_name) + expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER) expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) end @@ -150,6 +153,7 @@ RSpec.describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(branch_name) + expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER) end @@ -159,6 +163,7 @@ RSpec.describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(branch_name) + expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER) expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::DEVELOPER) end @@ -168,6 +173,7 @@ RSpec.describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(branch_name) + expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS) expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) end @@ -177,6 +183,7 @@ RSpec.describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(branch_name) + expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS) end @@ -186,10 +193,21 @@ RSpec.describe API::ProtectedBranches do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq(branch_name) + expect(json_response['allow_force_push']).to eq(false) expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS) expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::NO_ACCESS) end + it 'protects a single branch and allows force pushes' do + post post_endpoint, params: { name: branch_name, allow_force_push: true } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response['name']).to eq(branch_name) + expect(json_response['allow_force_push']).to eq(true) + expect(json_response['push_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) + expect(json_response['merge_access_levels'][0]['access_level']).to eq(Gitlab::Access::MAINTAINER) + end + it 'returns a 409 error if the same branch is protected twice' do post post_endpoint, params: { name: protected_name } diff --git a/spec/requests/api/repositories_spec.rb b/spec/requests/api/repositories_spec.rb index ace73e49c7c..31f0d7cec2a 100644 --- a/spec/requests/api/repositories_spec.rb +++ b/spec/requests/api/repositories_spec.rb @@ -650,6 +650,40 @@ RSpec.describe API::Repositories do expect(response).to have_gitlab_http_status(:ok) end + it 'supports leaving out the from and to attribute' do + spy = instance_spy(Repositories::ChangelogService) + + allow(Repositories::ChangelogService) + .to receive(:new) + .with( + project, + user, + version: '1.0.0', + date: DateTime.new(2020, 1, 1), + branch: 'kittens', + trailer: 'Foo', + file: 'FOO.md', + message: 'Commit message' + ) + .and_return(spy) + + expect(spy).to receive(:execute) + + post( + api("/projects/#{project.id}/repository/changelog", user), + params: { + version: '1.0.0', + date: '2020-01-01', + branch: 'kittens', + trailer: 'Foo', + file: 'FOO.md', + message: 'Commit message' + } + ) + + expect(response).to have_gitlab_http_status(:ok) + end + it 'produces an error when generating the changelog fails' do spy = instance_spy(Repositories::ChangelogService) diff --git a/spec/requests/api/resource_access_tokens_spec.rb b/spec/requests/api/resource_access_tokens_spec.rb index 9fd7eb2177d..79549bfc5e0 100644 --- a/spec/requests/api/resource_access_tokens_spec.rb +++ b/spec/requests/api/resource_access_tokens_spec.rb @@ -30,6 +30,18 @@ RSpec.describe API::ResourceAccessTokens do expect(token_ids).to match_array(access_tokens.pluck(:id)) end + it "exposes the correct token information", :aggregate_failures do + get_tokens + + token = access_tokens.last + api_get_token = json_response.last + + expect(api_get_token["name"]).to eq(token.name) + expect(api_get_token["scopes"]).to eq(token.scopes) + expect(api_get_token["expires_at"]).to eq(token.expires_at.to_date.iso8601) + expect(api_get_token).not_to have_key('token') + end + context "when using a project access token to GET other project access tokens" do let_it_be(:token) { access_tokens.first } @@ -182,13 +194,13 @@ RSpec.describe API::ResourceAccessTokens do end describe "POST projects/:id/access_tokens" do - let_it_be(:params) { { name: "test", scopes: ["api"], expires_at: Date.today + 1.month } } + let(:params) { { name: "test", scopes: ["api"], expires_at: expires_at } } + let(:expires_at) { 1.month.from_now } subject(:create_token) { post api("/projects/#{project_id}/access_tokens", user), params: params } context "when the user has maintainer permissions" do let_it_be(:project_id) { project.id } - let_it_be(:expires_at) { 1.month.from_now } before do project.add_maintainer(user) @@ -203,11 +215,12 @@ RSpec.describe API::ResourceAccessTokens do expect(json_response["name"]).to eq("test") expect(json_response["scopes"]).to eq(["api"]) expect(json_response["expires_at"]).to eq(expires_at.to_date.iso8601) + expect(json_response["token"]).to be_present end end context "when 'expires_at' is not set" do - let_it_be(:params) { { name: "test", scopes: ["api"] } } + let(:expires_at) { nil } it "creates a project access token with the params", :aggregate_failures do create_token diff --git a/spec/requests/api/rubygem_packages_spec.rb b/spec/requests/api/rubygem_packages_spec.rb index 5dd68bf9b10..d6ad8186063 100644 --- a/spec/requests/api/rubygem_packages_spec.rb +++ b/spec/requests/api/rubygem_packages_spec.rb @@ -3,9 +3,11 @@ require 'spec_helper' RSpec.describe API::RubygemPackages do + include PackagesManagerApiSpecHelpers + include WorkhorseHelpers using RSpec::Parameterized::TableSyntax - let_it_be(:project) { create(:project) } + let_it_be_with_reload(:project) { create(:project) } let_it_be(:personal_access_token) { create(:personal_access_token) } let_it_be(:user) { personal_access_token.user } let_it_be(:job) { create(:ci_build, :running, user: user) } @@ -13,6 +15,14 @@ RSpec.describe API::RubygemPackages do let_it_be(:project_deploy_token) { create(:project_deploy_token, deploy_token: deploy_token, project: project) } let_it_be(:headers) { {} } + let(:tokens) do + { + personal_access_token: personal_access_token.token, + deploy_token: deploy_token.token, + job_token: job.token + } + end + shared_examples 'when feature flag is disabled' do let(:headers) do { 'HTTP_AUTHORIZATION' => personal_access_token.token } @@ -34,7 +44,7 @@ RSpec.describe API::RubygemPackages do end shared_examples 'without authentication' do - it_behaves_like 'returning response status', :unauthorized + it_behaves_like 'returning response status', :not_found end shared_examples 'with authentication' do @@ -42,14 +52,6 @@ RSpec.describe API::RubygemPackages do { 'HTTP_AUTHORIZATION' => token } end - let(:tokens) do - { - personal_access_token: personal_access_token.token, - deploy_token: deploy_token.token, - job_token: job.token - } - end - where(:user_role, :token_type, :valid_token, :status) do :guest | :personal_access_token | true | :not_found :guest | :personal_access_token | false | :unauthorized @@ -106,34 +108,290 @@ RSpec.describe API::RubygemPackages do end describe 'GET /api/v4/projects/:project_id/packages/rubygems/gems/:file_name' do - let(:url) { api("/projects/#{project.id}/packages/rubygems/gems/my_gem-1.0.0.gem") } + let_it_be(:package_name) { 'package' } + let_it_be(:version) { '0.0.1' } + let_it_be(:package) { create(:rubygems_package, project: project, name: package_name, version: version) } + let_it_be(:file_name) { "#{package_name}-#{version}.gem" } + + let(:url) { api("/projects/#{project.id}/packages/rubygems/gems/#{file_name}") } subject { get(url, headers: headers) } - it_behaves_like 'an unimplemented route' + context 'with valid project' do + where(:visibility, :user_role, :member, :token_type, :valid_token, :shared_examples_name, :expected_status) do + :public | :developer | true | :personal_access_token | true | 'Rubygems gem download' | :success + :public | :guest | true | :personal_access_token | true | 'Rubygems gem download' | :success + :public | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :personal_access_token | true | 'Rubygems gem download' | :success + :public | :guest | false | :personal_access_token | true | 'Rubygems gem download' | :success + :public | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :anonymous | false | :personal_access_token | true | 'Rubygems gem download' | :success + :private | :developer | true | :personal_access_token | true | 'Rubygems gem download' | :success + :private | :guest | true | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :anonymous | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :public | :developer | true | :job_token | true | 'Rubygems gem download' | :success + :public | :guest | true | :job_token | true | 'Rubygems gem download' | :success + :public | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :job_token | true | 'Rubygems gem download' | :success + :public | :guest | false | :job_token | true | 'Rubygems gem download' | :success + :public | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :job_token | true | 'Rubygems gem download' | :success + :private | :guest | true | :job_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | true | :deploy_token | true | 'Rubygems gem download' | :success + :public | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :deploy_token | true | 'Rubygems gem download' | :success + :private | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + end + + with_them do + let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } + let(:headers) { user_role == :anonymous ? {} : { 'HTTP_AUTHORIZATION' => token } } + + before do + project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility.to_s)) + end + + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + end + end end describe 'POST /api/v4/projects/:project_id/packages/rubygems/api/v1/gems/authorize' do + include_context 'workhorse headers' + let(:url) { api("/projects/#{project.id}/packages/rubygems/api/v1/gems/authorize") } + let(:headers) { {} } subject { post(url, headers: headers) } - it_behaves_like 'an unimplemented route' + context 'with valid project' do + where(:visibility, :user_role, :member, :token_type, :valid_token, :shared_examples_name, :expected_status) do + :public | :developer | true | :personal_access_token | true | 'process rubygems workhorse authorization' | :success + :public | :guest | true | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :public | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :public | :guest | false | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :public | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :anonymous | false | :personal_access_token | true | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :personal_access_token | true | 'process rubygems workhorse authorization' | :success + :private | :guest | true | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :anonymous | false | :personal_access_token | true | 'rejects rubygems packages access' | :unauthorized + :public | :developer | true | :job_token | true | 'process rubygems workhorse authorization' | :success + :public | :guest | true | :job_token | true | 'rejects rubygems packages access' | :forbidden + :public | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :job_token | true | 'rejects rubygems packages access' | :forbidden + :public | :guest | false | :job_token | true | 'rejects rubygems packages access' | :forbidden + :public | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :job_token | true | 'process rubygems workhorse authorization' | :success + :private | :guest | true | :job_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | true | :deploy_token | true | 'process rubygems workhorse authorization' | :success + :public | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :deploy_token | true | 'process rubygems workhorse authorization' | :success + :private | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + end + + with_them do + let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } + let(:user_headers) { user_role == :anonymous ? {} : { 'HTTP_AUTHORIZATION' => token } } + let(:headers) { user_headers.merge(workhorse_headers) } + + before do + project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility.to_s)) + end + + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + end + end end describe 'POST /api/v4/projects/:project_id/packages/rubygems/api/v1/gems' do - let(:url) { api("/projects/#{project.id}/packages/rubygems/api/v1/gems") } + include_context 'workhorse headers' + + let(:url) { "/projects/#{project.id}/packages/rubygems/api/v1/gems" } + + let_it_be(:file_name) { 'package.gem' } + let(:headers) { {} } + let(:params) { { file: temp_file(file_name) } } + let(:file_key) { :file } + let(:send_rewritten_field) { true } + + subject do + workhorse_finalize( + api(url), + method: :post, + file_key: file_key, + params: params, + headers: headers, + send_rewritten_field: send_rewritten_field + ) + end - subject { post(url, headers: headers) } + context 'with valid project' do + where(:visibility, :user_role, :member, :token_type, :valid_token, :shared_examples_name, :expected_status) do + :public | :developer | true | :personal_access_token | true | 'process rubygems upload' | :created + :public | :guest | true | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :public | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :public | :guest | false | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :public | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :anonymous | false | :personal_access_token | true | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :personal_access_token | true | 'process rubygems upload' | :created + :private | :guest | true | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :anonymous | false | :personal_access_token | true | 'rejects rubygems packages access' | :unauthorized + :public | :developer | true | :job_token | true | 'process rubygems upload' | :created + :public | :guest | true | :job_token | true | 'rejects rubygems packages access' | :forbidden + :public | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :job_token | true | 'rejects rubygems packages access' | :forbidden + :public | :guest | false | :job_token | true | 'rejects rubygems packages access' | :forbidden + :public | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :job_token | true | 'process rubygems upload' | :created + :private | :guest | true | :job_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | true | :deploy_token | true | 'process rubygems upload' | :created + :public | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :deploy_token | true | 'process rubygems upload' | :created + :private | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + end - it_behaves_like 'an unimplemented route' + with_them do + let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } + let(:user_headers) { user_role == :anonymous ? {} : { 'HTTP_AUTHORIZATION' => token } } + let(:headers) { user_headers.merge(workhorse_headers) } + + before do + project.update_column(:visibility_level, Gitlab::VisibilityLevel.level_value(visibility.to_s)) + end + + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + end + + context 'failed package file save' do + let(:user_headers) { { 'HTTP_AUTHORIZATION' => personal_access_token.token } } + let(:headers) { user_headers.merge(workhorse_headers) } + + 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) + + expect { subject } + .to change { project.packages.count }.by(0) + .and change { Packages::PackageFile.count }.by(0) + expect(response).to have_gitlab_http_status(:error) + end + end + end end describe 'GET /api/v4/projects/:project_id/packages/rubygems/api/v1/dependencies' do + let_it_be(:package) { create(:rubygems_package, project: project) } + let(:url) { api("/projects/#{project.id}/packages/rubygems/api/v1/dependencies") } - subject { get(url, headers: headers) } + subject { get(url, headers: headers, params: params) } + + context 'with valid project' do + where(:visibility, :user_role, :member, :token_type, :valid_token, :shared_examples_name, :expected_status) do + :public | :developer | true | :personal_access_token | true | 'dependency endpoint success' | :success + :public | :guest | true | :personal_access_token | true | 'dependency endpoint success' | :success + :public | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :personal_access_token | true | 'dependency endpoint success' | :success + :public | :guest | false | :personal_access_token | true | 'dependency endpoint success' | :success + :public | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :anonymous | false | :personal_access_token | true | 'dependency endpoint success' | :success + :private | :developer | true | :personal_access_token | true | 'dependency endpoint success' | :success + :private | :guest | true | :personal_access_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :personal_access_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :anonymous | false | :personal_access_token | true | 'rejects rubygems packages access' | :not_found + :public | :developer | true | :job_token | true | 'dependency endpoint success' | :success + :public | :guest | true | :job_token | true | 'dependency endpoint success' | :success + :public | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | false | :job_token | true | 'dependency endpoint success' | :success + :public | :guest | false | :job_token | true | 'dependency endpoint success' | :success + :public | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :job_token | true | 'dependency endpoint success' | :success + :private | :guest | true | :job_token | true | 'rejects rubygems packages access' | :forbidden + :private | :developer | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | true | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :guest | false | :job_token | true | 'rejects rubygems packages access' | :not_found + :private | :developer | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :guest | false | :job_token | false | 'rejects rubygems packages access' | :unauthorized + :public | :developer | true | :deploy_token | true | 'dependency endpoint success' | :success + :public | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + :private | :developer | true | :deploy_token | true | 'dependency endpoint success' | :success + :private | :developer | true | :deploy_token | false | 'rejects rubygems packages access' | :unauthorized + end - it_behaves_like 'an unimplemented route' + with_them do + let(:token) { valid_token ? tokens[token_type] : 'invalid-token123' } + let(:headers) { user_role == :anonymous ? {} : { 'HTTP_AUTHORIZATION' => token } } + let(:params) { {} } + + before do + project.update!(visibility: visibility.to_s) + end + + it_behaves_like params[:shared_examples_name], params[:user_role], params[:expected_status], params[:member] + end + end end end diff --git a/spec/requests/api/snippet_repository_storage_moves_spec.rb b/spec/requests/api/snippet_repository_storage_moves_spec.rb index edb92569823..40d01500ac1 100644 --- a/spec/requests/api/snippet_repository_storage_moves_spec.rb +++ b/spec/requests/api/snippet_repository_storage_moves_spec.rb @@ -7,6 +7,6 @@ RSpec.describe API::SnippetRepositoryStorageMoves do let_it_be(:container) { create(:snippet, :repository).tap { |snippet| snippet.create_repository } } let_it_be(:storage_move) { create(:snippet_repository_storage_move, :scheduled, container: container) } let(:repository_storage_move_factory) { :snippet_repository_storage_move } - let(:bulk_worker_klass) { SnippetScheduleBulkRepositoryShardMovesWorker } + let(:bulk_worker_klass) { Snippets::ScheduleBulkRepositoryShardMovesWorker } end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index d70a8bd692d..2a7689eaddf 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -320,6 +320,18 @@ RSpec.describe API::Users do expect(json_response).to all(include('state' => /(blocked|ldap_blocked)/)) end + it "returns an array of external users" do + create(:user) + external_user = create(:user, external: true) + + get api("/users?external=true", user) + + expect(response).to match_response_schema('public_api/v4/user/basics') + expect(response).to include_pagination_headers + expect(json_response.size).to eq(1) + expect(json_response[0]['id']).to eq(external_user.id) + end + it "returns one user" do get api("/users?username=#{omniauth_user.username}", user) @@ -940,6 +952,18 @@ RSpec.describe API::Users do expect(new_user.private_profile?).to eq(true) end + it "creates user with view_diffs_file_by_file" do + post api('/users', admin), params: attributes_for(:user, view_diffs_file_by_file: true) + + expect(response).to have_gitlab_http_status(:created) + + user_id = json_response['id'] + new_user = User.find(user_id) + + expect(new_user).not_to eq(nil) + expect(new_user.user_preference.view_diffs_file_by_file?).to eq(true) + end + it "does not create user with invalid email" do post api('/users', admin), params: { @@ -1254,6 +1278,13 @@ RSpec.describe API::Users do expect(user.reload.private_profile).to eq(true) end + it "updates viewing diffs file by file" do + put api("/users/#{user.id}", admin), params: { view_diffs_file_by_file: true } + + expect(response).to have_gitlab_http_status(:ok) + expect(user.reload.user_preference.view_diffs_file_by_file?).to eq(true) + end + it "updates private profile to false when nil is given" do user.update!(private_profile: true) @@ -3044,18 +3075,6 @@ RSpec.describe API::Users do expect(response).to have_gitlab_http_status(:bad_request) end - - context 'when the clear_status_with_quick_options feature flag is disabled' do - before do - stub_feature_flags(clear_status_with_quick_options: false) - end - - it 'does not persist clear_status_at' do - put api('/user/status', user), params: { emoji: 'smirk', message: 'hello world', clear_status_after: '3_hours' } - - expect(user.status.reload.clear_status_at).to be_nil - end - end end end diff --git a/spec/requests/api/v3/github_spec.rb b/spec/requests/api/v3/github_spec.rb index e7d9ba99743..197c6cbb0eb 100644 --- a/spec/requests/api/v3/github_spec.rb +++ b/spec/requests/api/v3/github_spec.rb @@ -149,6 +149,8 @@ RSpec.describe API::V3::Github do end describe 'GET events' do + include ProjectForksHelper + let(:group) { create(:group) } let(:project) { create(:project, :empty_repo, path: 'project.with.dot', group: group) } let(:events_path) { "/repos/#{group.path}/#{project.path}/events" } @@ -174,6 +176,17 @@ RSpec.describe API::V3::Github do end end + it 'avoids N+1 queries' do + create(:merge_request, source_project: project) + source_project = fork_project(project, nil, repository: true) + + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) { jira_get v3_api(events_path, user) }.count + + create_list(:merge_request, 2, :unique_branches, source_project: source_project, target_project: project) + + expect { jira_get v3_api(events_path, user) }.not_to exceed_all_query_limit(control_count) + end + context 'if there are more merge requests' do let!(:merge_request) { create(:merge_request, id: 10000, source_project: project, target_project: project, author: user) } let!(:merge_request2) { create(:merge_request, id: 10001, source_project: project, source_branch: generate(:branch), target_project: project, author: user) } diff --git a/spec/requests/api/wikis_spec.rb b/spec/requests/api/wikis_spec.rb index f271f8aa853..d35aab40ca9 100644 --- a/spec/requests/api/wikis_spec.rb +++ b/spec/requests/api/wikis_spec.rb @@ -14,6 +14,7 @@ require 'spec_helper' RSpec.describe API::Wikis do include WorkhorseHelpers + include AfterNextHelpers let(:user) { create(:user) } let(:group) { create(:group).tap { |g| g.add_owner(user) } } @@ -578,6 +579,20 @@ RSpec.describe API::Wikis do include_examples 'wiki API 404 Wiki Page Not Found' end end + + context 'when there is an error deleting the page' do + it 'returns 422' do + project.add_maintainer(user) + + allow_next(WikiPages::DestroyService, current_user: user, container: project) + .to receive(:execute).and_return(ServiceResponse.error(message: 'foo')) + + delete(api(url, user)) + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(json_response['message']).to eq 'foo' + end + end end context 'when wiki belongs to a group project' do diff --git a/spec/requests/ide_controller_spec.rb b/spec/requests/ide_controller_spec.rb index 805c1f1d82b..4f127e07b6b 100644 --- a/spec/requests/ide_controller_spec.rb +++ b/spec/requests/ide_controller_spec.rb @@ -3,7 +3,11 @@ require 'spec_helper' RSpec.describe IdeController do - let(:user) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let_it_be(:creator) { project.creator } + let_it_be(:other_user) { create(:user) } + + let(:user) { creator } before do sign_in(user) @@ -14,4 +18,172 @@ RSpec.describe IdeController do get ide_url end + + describe '#index', :aggregate_failures do + subject { get route } + + shared_examples 'user cannot push code' do + include ProjectForksHelper + + let(:user) { other_user } + + context 'when user does not have fork' do + it 'does not instantiate forked_project instance var and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to eq project + expect(assigns(:forked_project)).to be_nil + end + end + + context 'when user has have fork' do + let!(:fork) { fork_project(project, user, repository: true) } + + it 'instantiates forked_project instance var and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to eq project + expect(assigns(:forked_project)).to eq fork + end + end + end + + context '/-/ide' do + let(:route) { '/-/ide' } + + it 'does not instantiate any instance var and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to be_nil + expect(assigns(:branch)).to be_nil + expect(assigns(:path)).to be_nil + expect(assigns(:merge_request)).to be_nil + expect(assigns(:forked_project)).to be_nil + end + end + + context '/-/ide/project' do + let(:route) { '/-/ide/project' } + + it 'does not instantiate any instance var and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to be_nil + expect(assigns(:branch)).to be_nil + expect(assigns(:path)).to be_nil + expect(assigns(:merge_request)).to be_nil + expect(assigns(:forked_project)).to be_nil + end + end + + context '/-/ide/project/:project' do + let(:route) { "/-/ide/project/#{project.full_path}" } + + it 'instantiates project instance var and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to eq project + expect(assigns(:branch)).to be_nil + expect(assigns(:path)).to be_nil + expect(assigns(:merge_request)).to be_nil + expect(assigns(:forked_project)).to be_nil + end + + it_behaves_like 'user cannot push code' + + %w(edit blob tree).each do |action| + context "/-/ide/project/:project/#{action}" do + let(:route) { "/-/ide/project/#{project.full_path}/#{action}" } + + it 'instantiates project instance var and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to eq project + expect(assigns(:branch)).to be_nil + expect(assigns(:path)).to be_nil + expect(assigns(:merge_request)).to be_nil + expect(assigns(:forked_project)).to be_nil + end + + it_behaves_like 'user cannot push code' + + context "/-/ide/project/:project/#{action}/:branch" do + let(:route) { "/-/ide/project/#{project.full_path}/#{action}/master" } + + it 'instantiates project and branch instance vars and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to eq project + expect(assigns(:branch)).to eq 'master' + expect(assigns(:path)).to be_nil + expect(assigns(:merge_request)).to be_nil + expect(assigns(:forked_project)).to be_nil + end + + it_behaves_like 'user cannot push code' + + context "/-/ide/project/:project/#{action}/:branch/-" do + let(:route) { "/-/ide/project/#{project.full_path}/#{action}/branch/slash/-" } + + it 'instantiates project and branch instance vars and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to eq project + expect(assigns(:branch)).to eq 'branch/slash' + expect(assigns(:path)).to be_nil + expect(assigns(:merge_request)).to be_nil + expect(assigns(:forked_project)).to be_nil + end + + it_behaves_like 'user cannot push code' + + context "/-/ide/project/:project/#{action}/:branch/-/:path" do + let(:route) { "/-/ide/project/#{project.full_path}/#{action}/master/-/foo/.bar" } + + it 'instantiates project, branch, and path instance vars and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to eq project + expect(assigns(:branch)).to eq 'master' + expect(assigns(:path)).to eq 'foo/.bar' + expect(assigns(:merge_request)).to be_nil + expect(assigns(:forked_project)).to be_nil + end + + it_behaves_like 'user cannot push code' + end + end + end + end + end + + context '/-/ide/project/:project/merge_requests/:merge_request_id' do + let!(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + + let(:route) { "/-/ide/project/#{project.full_path}/merge_requests/#{merge_request.id}" } + + it 'instantiates project and merge_request instance vars and return 200' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(assigns(:project)).to eq project + expect(assigns(:branch)).to be_nil + expect(assigns(:path)).to be_nil + expect(assigns(:merge_request)).to eq merge_request.id.to_s + expect(assigns(:forked_project)).to be_nil + end + + it_behaves_like 'user cannot push code' + end + end + end end diff --git a/spec/requests/projects/merge_requests/content_spec.rb b/spec/requests/projects/merge_requests/content_spec.rb new file mode 100644 index 00000000000..7e5ec6f64c4 --- /dev/null +++ b/spec/requests/projects/merge_requests/content_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'merge request content spec' do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let_it_be(:merge_request) { create(:merge_request, :with_head_pipeline, target_project: project, source_project: project) } + let_it_be(:ci_build) { create(:ci_build, :artifacts, pipeline: merge_request.head_pipeline) } + + before do + sign_in(user) + project.add_maintainer(user) + end + + shared_examples 'cached widget request' do + it 'avoids N+1 queries when multiple job artifacts are present' do + control = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get cached_widget_project_json_merge_request_path(project, merge_request, format: :json) + end + + create_list(:ci_build, 10, :artifacts, pipeline: merge_request.head_pipeline) + + expect do + get cached_widget_project_json_merge_request_path(project, merge_request, format: :json) + end.not_to exceed_query_limit(control) + end + end + + describe 'GET cached_widget' do + it_behaves_like 'cached widget request' + + context 'with non_public_artifacts disabled' do + before do + stub_feature_flags(non_public_artifacts: false) + end + + it_behaves_like 'cached widget request' + end + end +end diff --git a/spec/requests/projects/noteable_notes_spec.rb b/spec/requests/projects/noteable_notes_spec.rb index 5ae2aadaa84..2bf1ffb2edc 100644 --- a/spec/requests/projects/noteable_notes_spec.rb +++ b/spec/requests/projects/noteable_notes_spec.rb @@ -18,7 +18,9 @@ RSpec.describe 'Project noteable notes' do login_as(user) end - it 'does not set a Gitlab::EtagCaching ETag' do + it 'does not set a Gitlab::EtagCaching ETag if there is a note' do + create(:note_on_merge_request, noteable: merge_request, project: merge_request.project) + get notes_path expect(response).to have_gitlab_http_status(:ok) @@ -27,5 +29,12 @@ RSpec.describe 'Project noteable notes' do # interfere with notes pagination expect(response_etag).not_to eq(stored_etag) end + + it 'sets a Gitlab::EtagCaching ETag if there is no note' do + get notes_path + + expect(response).to have_gitlab_http_status(:ok) + expect(response_etag).to eq(stored_etag) + end end end |