From ee664acb356f8123f4f6b00b73c1e1cf0866c7fb Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 20 Oct 2022 09:40:42 +0000 Subject: Add latest changes from gitlab-org/gitlab@15-5-stable-ee --- .../admin/batched_background_migrations_spec.rb | 88 ++- spec/requests/api/branches_spec.rb | 48 +- spec/requests/api/bulk_imports_spec.rb | 12 + spec/requests/api/ci/job_artifacts_spec.rb | 43 ++ spec/requests/api/ci/jobs_spec.rb | 25 +- spec/requests/api/ci/resource_groups_spec.rb | 42 ++ .../api/ci/runner/jobs_request_post_spec.rb | 52 +- spec/requests/api/ci/runner/runners_reset_spec.rb | 1 - spec/requests/api/ci/runners_spec.rb | 40 +- spec/requests/api/deploy_tokens_spec.rb | 37 +- spec/requests/api/features_spec.rb | 772 +++++++++++---------- spec/requests/api/generic_packages_spec.rb | 15 + spec/requests/api/graphql/ci/ci_cd_setting_spec.rb | 2 + spec/requests/api/graphql/ci/config_spec.rb | 64 +- .../api/graphql/ci/config_variables_spec.rb | 9 + spec/requests/api/graphql/ci/jobs_spec.rb | 11 +- .../api/graphql/ci/pipeline_schedules_spec.rb | 88 +++ spec/requests/api/graphql/jobs_query_spec.rb | 55 ++ spec/requests/api/graphql/milestone_spec.rb | 199 +++--- .../mutations/ci/job/artifacts_destroy_spec.rb | 85 +++ .../ci/job_token_scope/add_project_spec.rb | 2 +- .../ci/job_token_scope/remove_project_spec.rb | 2 +- .../mutations/ci/pipeline_schedule_delete_spec.rb | 82 +++ .../ci/project_ci_cd_settings_update_spec.rb | 51 +- .../namespace/package_settings/update_spec.rb | 45 +- .../mutations/packages/bulk_destroy_spec.rb | 128 ++++ .../api/graphql/mutations/uploads/delete_spec.rb | 9 +- .../graphql/mutations/work_items/update_spec.rb | 72 ++ .../mutations/work_items/update_widgets_spec.rb | 61 -- .../api/graphql/project/branch_rules_spec.rb | 68 +- .../api/graphql/project/cluster_agents_spec.rb | 11 +- .../graphql/project/issue/designs/designs_spec.rb | 33 +- spec/requests/api/graphql/project/issues_spec.rb | 50 ++ .../api/graphql/project/merge_requests_spec.rb | 53 +- .../api/graphql/project/milestones_spec.rb | 7 +- .../api/graphql/project/work_items_spec.rb | 113 ++- spec/requests/api/graphql/todo_query_spec.rb | 9 +- .../api/graphql/usage_trends_measurements_spec.rb | 22 +- spec/requests/api/graphql/work_item_spec.rb | 16 +- spec/requests/api/groups_spec.rb | 34 +- spec/requests/api/helm_packages_spec.rb | 11 + spec/requests/api/import_github_spec.rb | 41 +- spec/requests/api/internal/base_spec.rb | 20 +- spec/requests/api/issues/issues_spec.rb | 15 + .../api/issues/post_projects_issues_spec.rb | 4 +- .../api/issues/put_projects_issues_spec.rb | 4 +- spec/requests/api/maven_packages_spec.rb | 52 +- spec/requests/api/merge_requests_spec.rb | 87 ++- spec/requests/api/metadata_spec.rb | 26 +- spec/requests/api/ml/mlflow_spec.rb | 330 ++++++--- spec/requests/api/pages_domains_spec.rb | 60 +- .../self_information_spec.rb | 102 +++ .../personal_access_tokens/self_revocation_spec.rb | 69 -- spec/requests/api/personal_access_tokens_spec.rb | 367 +++++++++- spec/requests/api/project_attributes.yml | 7 +- spec/requests/api/projects_spec.rb | 26 +- spec/requests/api/settings_spec.rb | 5 +- spec/requests/api/tags_spec.rb | 8 - spec/requests/api/users_spec.rb | 33 + spec/requests/api/version_spec.rb | 93 --- 60 files changed, 2666 insertions(+), 1250 deletions(-) create mode 100644 spec/requests/api/graphql/ci/pipeline_schedules_spec.rb create mode 100644 spec/requests/api/graphql/jobs_query_spec.rb create mode 100644 spec/requests/api/graphql/mutations/ci/job/artifacts_destroy_spec.rb create mode 100644 spec/requests/api/graphql/mutations/ci/pipeline_schedule_delete_spec.rb create mode 100644 spec/requests/api/graphql/mutations/packages/bulk_destroy_spec.rb delete mode 100644 spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb create mode 100644 spec/requests/api/personal_access_tokens/self_information_spec.rb delete mode 100644 spec/requests/api/personal_access_tokens/self_revocation_spec.rb delete mode 100644 spec/requests/api/version_spec.rb (limited to 'spec/requests/api') diff --git a/spec/requests/api/admin/batched_background_migrations_spec.rb b/spec/requests/api/admin/batched_background_migrations_spec.rb index c99b21c0c27..3b396a91d3e 100644 --- a/spec/requests/api/admin/batched_background_migrations_spec.rb +++ b/spec/requests/api/admin/batched_background_migrations_spec.rb @@ -9,6 +9,7 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do describe 'GET /admin/batched_background_migrations/:id' do let!(:migration) { create(:batched_background_migration, :paused) } let(:database) { :main } + let(:params) { { database: database } } subject(:show_migration) do get api("/admin/batched_background_migrations/#{migration.id}", admin), params: { database: database } @@ -27,10 +28,8 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do end context 'when the batched background migration does not exist' do - let(:params) { { database: database } } - it 'returns 404' do - put api("/admin/batched_background_migrations/#{non_existing_record_id}", admin), params: params + get api("/admin/batched_background_migrations/#{non_existing_record_id}", admin), params: params expect(response).to have_gitlab_http_status(:not_found) end @@ -58,6 +57,17 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do expect(response).to have_gitlab_http_status(:forbidden) end end + + context 'when the database name does not exist' do + let(:database) { :wrong_database } + + it 'returns bad request' do + get api("/admin/batched_background_migrations/#{migration.id}", admin), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include('database does not have a valid value') + end + end end describe 'GET /admin/batched_background_migrations' do @@ -82,6 +92,7 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do let(:database) { :ci } let(:schema) { :gitlab_ci } let(:ci_model) { Ci::ApplicationRecord } + let(:params) { { database: database } } context 'when CI database is provided' do let(:db_config) { instance_double(ActiveRecord::DatabaseConfigurations::HashConfig, name: 'fake_db') } @@ -94,7 +105,18 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(ci_model.connection).and_yield - get api('/admin/batched_background_migrations', admin), params: { database: :ci } + get api('/admin/batched_background_migrations', admin), params: params + end + + context 'when the database name does not exist' do + let(:database) { :wrong_database } + + it 'returns bad request' do + get api("/admin/batched_background_migrations", admin), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include('database does not have a valid value') + end end it 'returns CI database records' do @@ -105,7 +127,7 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do create(:batched_background_migration, :active, gitlab_schema: schema) end - get api('/admin/batched_background_migrations', admin), params: { database: :ci } + get api('/admin/batched_background_migrations', admin), params: params aggregate_failures "testing response" do expect(response).to have_gitlab_http_status(:ok) @@ -133,9 +155,10 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do describe 'PUT /admin/batched_background_migrations/:id/resume' do let!(:migration) { create(:batched_background_migration, :paused) } let(:database) { :main } + let(:params) { { database: database } } subject(:resume) do - put api("/admin/batched_background_migrations/#{migration.id}/resume", admin), params: { database: database } + put api("/admin/batched_background_migrations/#{migration.id}/resume", admin), params: params end it 'pauses the batched background migration' do @@ -149,8 +172,6 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do end context 'when the batched background migration does not exist' do - let(:params) { { database: database } } - it 'returns 404' do put api("/admin/batched_background_migrations/#{non_existing_record_id}/resume", admin), params: params @@ -158,6 +179,16 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do end end + context 'when the migration is not paused' do + let!(:migration) { create(:batched_background_migration, :failed) } + + it 'returns 422' do + put api("/admin/batched_background_migrations/#{migration.id}/resume", admin), params: params + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + context 'when multiple database is enabled' do let(:ci_model) { Ci::ApplicationRecord } let(:database) { :ci } @@ -171,6 +202,17 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do resume end + + context 'when the database name does not exist' do + let(:database) { :wrong_database } + + it 'returns bad request' do + put api("/admin/batched_background_migrations/#{migration.id}/resume", admin), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include('database does not have a valid value') + end + end end context 'when authenticated as a non-admin user' do @@ -184,9 +226,11 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do describe 'PUT /admin/batched_background_migrations/:id/pause' do let!(:migration) { create(:batched_background_migration, :active) } + let(:database) { :main } + let(:params) { { database: database } } it 'pauses the batched background migration' do - put api("/admin/batched_background_migrations/#{migration.id}/pause", admin), params: { database: :main } + put api("/admin/batched_background_migrations/#{migration.id}/pause", admin), params: params aggregate_failures "testing response" do expect(response).to have_gitlab_http_status(:ok) @@ -196,8 +240,6 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do end context 'when the batched background migration does not exist' do - let(:params) { { database: :main } } - it 'returns 404' do put api("/admin/batched_background_migrations/#{non_existing_record_id}/pause", admin), params: params @@ -205,8 +247,19 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do end end + context 'when the migration is not active' do + let!(:migration) { create(:batched_background_migration, :failed) } + + it 'returns 422' do + put api("/admin/batched_background_migrations/#{migration.id}/pause", admin), params: params + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + context 'when multiple database is enabled' do let(:ci_model) { Ci::ApplicationRecord } + let(:database) { :ci } before do skip_if_multiple_databases_not_setup @@ -215,7 +268,18 @@ RSpec.describe API::Admin::BatchedBackgroundMigrations do it 'uses the correct connection' do expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(ci_model.connection).and_yield - put api("/admin/batched_background_migrations/#{migration.id}/pause", admin), params: { database: :ci } + put api("/admin/batched_background_migrations/#{migration.id}/pause", admin), params: params + end + + context 'when the database name does not exist' do + let(:database) { :wrong_database } + + it 'returns bad request' do + put api("/admin/batched_background_migrations/#{migration.id}/pause", admin), params: params + + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.body).to include('database does not have a valid value') + end end end diff --git a/spec/requests/api/branches_spec.rb b/spec/requests/api/branches_spec.rb index f7539e13b80..750b9a39e15 100644 --- a/spec/requests/api/branches_spec.rb +++ b/spec/requests/api/branches_spec.rb @@ -221,55 +221,25 @@ RSpec.describe API::Branches do get api(route), params: { per_page: 1 } end - context 'when increase_branch_cache_expiry is enabled' do - it 'uses the cache up to 60 minutes' do - time_of_request = Time.current + it 'uses the cache up to 60 minutes' do + time_of_request = Time.current - get api(route), params: { per_page: 1 } - - travel_to time_of_request + 59.minutes do - expect(API::Entities::Branch).not_to receive(:represent) + get api(route), params: { per_page: 1 } - get api(route), params: { per_page: 1 } - end - end + travel_to time_of_request + 59.minutes do + expect(API::Entities::Branch).not_to receive(:represent) - it 'requests for new value after 60 minutes' do get api(route), params: { per_page: 1 } - - travel_to 61.minutes.from_now do - expect(API::Entities::Branch).to receive(:represent) - - get api(route), params: { per_page: 1 } - end end end - context 'when increase_branch_cache_expiry is disabled' do - before do - stub_feature_flags(increase_branch_cache_expiry: false) - end - - it 'uses the cache up to 10 minutes' do - time_of_request = Time.current - - get api(route), params: { per_page: 1 } + it 'requests for new value after 60 minutes' do + get api(route), params: { per_page: 1 } - travel_to time_of_request + 9.minutes do - expect(API::Entities::Branch).not_to receive(:represent) + travel_to 61.minutes.from_now do + expect(API::Entities::Branch).to receive(:represent) - get api(route), params: { per_page: 1 } - end - end - - it 'requests for new value after 10 minutes' do get api(route), params: { per_page: 1 } - - travel_to 11.minutes.from_now do - expect(API::Entities::Branch).to receive(:represent) - - get api(route), params: { per_page: 1 } - end end end end diff --git a/spec/requests/api/bulk_imports_spec.rb b/spec/requests/api/bulk_imports_spec.rb index 6a3d13567bd..ad57a370fc5 100644 --- a/spec/requests/api/bulk_imports_spec.rb +++ b/spec/requests/api/bulk_imports_spec.rb @@ -53,6 +53,18 @@ RSpec.describe API::BulkImports do end end + context 'when bulk_import feature flag is disabled' do + before do + stub_feature_flags(bulk_import: false) + end + + it 'returns 404' do + post api('/bulk_imports', user), params: {} + + expect(response).to have_gitlab_http_status(:not_found) + end + end + shared_examples 'starting a new migration' do it 'starts a new migration' do post api('/bulk_imports', user), params: { diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index 0fb11bf98d2..2bf242f06ed 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -389,6 +389,49 @@ RSpec.describe API::Ci::JobArtifacts do end end + context 'when Google CDN is enabled' do + let(:cdn_enabled) { true } + let(:cdn_config) do + { + 'provider' => 'Google', + 'url' => 'https://cdn.example.org', + 'key_name' => 'stanhu-key', + 'key' => Base64.urlsafe_encode64(SecureRandom.hex) + } + end + + before do + stub_feature_flags(ci_job_artifacts_cdn: cdn_enabled) + stub_object_storage_uploader(config: Gitlab.config.artifacts.object_store, + uploader: JobArtifactUploader, + proxy_download: proxy_download, + cdn: cdn_config) + allow(Gitlab::ApplicationContext).to receive(:push).and_call_original + end + + subject { get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user), env: { 'REMOTE_ADDR': '18.245.0.1' } } + + it 'returns CDN-signed URL' do + expect(Gitlab::ApplicationContext).to receive(:push).with(artifact_used_cdn: true).and_call_original + + subject + + expect(response.redirect_url).to start_with("https://cdn.example.org/#{artifact.file.path}") + end + + context 'when ci_job_artifacts_cdn feature flag is disabled' do + let(:cdn_enabled) { false } + + it 'returns the file remote URL' do + expect(Gitlab::ApplicationContext).to receive(:push).with(artifact_used_cdn: false).and_call_original + + subject + + expect(response).to redirect_to(artifact.file.url) + end + end + end + context 'authorized user' do it 'returns the file remote URL' do expect(response).to redirect_to(artifact.file.url) diff --git a/spec/requests/api/ci/jobs_spec.rb b/spec/requests/api/ci/jobs_spec.rb index b8983e9632e..0e17db516f4 100644 --- a/spec/requests/api/ci/jobs_spec.rb +++ b/spec/requests/api/ci/jobs_spec.rb @@ -226,18 +226,19 @@ RSpec.describe API::Ci::Jobs do expect(json_response.dig('user', 'username')).to eq(api_user.username) expect(json_response.dig('user', 'roles_in_project')).to match_array %w(guest reporter developer) expect(json_response).not_to include('environment') - expect(json_response['allowed_agents']).to match_array([ - { - 'id' => implicit_authorization.agent_id, - 'config_project' => hash_including('id' => implicit_authorization.agent.project_id), - 'configuration' => implicit_authorization.config - }, - { - 'id' => group_authorization.agent_id, - 'config_project' => hash_including('id' => group_authorization.agent.project_id), - 'configuration' => group_authorization.config - } - ]) + expect(json_response['allowed_agents']).to match_array( + [ + { + 'id' => implicit_authorization.agent_id, + 'config_project' => hash_including('id' => implicit_authorization.agent.project_id), + 'configuration' => implicit_authorization.config + }, + { + 'id' => group_authorization.agent_id, + 'config_project' => hash_including('id' => group_authorization.agent.project_id), + 'configuration' => group_authorization.config + } + ]) end end diff --git a/spec/requests/api/ci/resource_groups_spec.rb b/spec/requests/api/ci/resource_groups_spec.rb index 864c363e6d3..87df71f6096 100644 --- a/spec/requests/api/ci/resource_groups_spec.rb +++ b/spec/requests/api/ci/resource_groups_spec.rb @@ -77,6 +77,48 @@ RSpec.describe API::Ci::ResourceGroups do end end + describe 'GET /projects/:id/resource_groups/:key/upcoming_jobs' do + subject { get api("/projects/#{project.id}/resource_groups/#{key}/upcoming_jobs", user) } + + let_it_be(:resource_group) { create(:ci_resource_group, project: project) } + let_it_be(:processable) { create(:ci_processable, resource_group: resource_group) } + let_it_be(:upcoming_processable) { create(:ci_processable, :waiting_for_resource, resource_group: resource_group) } + + let(:key) { resource_group.key } + + it 'returns upcoming jobs of resource group', :aggregate_failures do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.length).to eq(1) + expect(json_response[0]['id']).to eq(upcoming_processable.id) + expect(json_response[0]['name']).to eq(upcoming_processable.name) + expect(json_response[0]['ref']).to eq(upcoming_processable.ref) + expect(json_response[0]['stage']).to eq(upcoming_processable.stage) + expect(json_response[0]['status']).to eq(upcoming_processable.status) + end + + context 'when user is reporter' do + let(:user) { reporter } + + it 'returns forbidden' do + subject + + expect(response).to have_gitlab_http_status(:forbidden) + end + end + + context 'when there is no corresponding resource group' do + let(:key) { 'unknown' } + + it 'returns not found' do + subject + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'PUT /projects/:id/resource_groups/:key' do subject { put api("/projects/#{project.id}/resource_groups/#{key}", user), params: params } 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 b33b97f90d7..d4f734e7bdd 100644 --- a/spec/requests/api/ci/runner/jobs_request_post_spec.rb +++ b/spec/requests/api/ci/runner/jobs_request_post_spec.rb @@ -220,14 +220,15 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(json_response['image']).to eq( { 'name' => 'image:1.0', 'entrypoint' => '/bin/sh', 'ports' => [], 'pull_policy' => nil } ) - expect(json_response['services']).to eq([ - { 'name' => 'postgres', 'entrypoint' => nil, 'alias' => nil, 'command' => nil, 'ports' => [], - 'variables' => nil, 'pull_policy' => nil }, - { 'name' => 'docker:stable-dind', 'entrypoint' => '/bin/sh', 'alias' => 'docker', 'command' => 'sleep 30', - 'ports' => [], 'variables' => [], 'pull_policy' => nil }, - { 'name' => 'mysql:latest', 'entrypoint' => nil, 'alias' => nil, 'command' => nil, 'ports' => [], - 'variables' => [{ 'key' => 'MYSQL_ROOT_PASSWORD', 'value' => 'root123.' }], 'pull_policy' => nil } - ]) + expect(json_response['services']).to eq( + [ + { 'name' => 'postgres', 'entrypoint' => nil, 'alias' => nil, 'command' => nil, 'ports' => [], + 'variables' => nil, 'pull_policy' => nil }, + { 'name' => 'docker:stable-dind', 'entrypoint' => '/bin/sh', 'alias' => 'docker', 'command' => 'sleep 30', + 'ports' => [], 'variables' => [], 'pull_policy' => nil }, + { 'name' => 'mysql:latest', 'entrypoint' => nil, 'alias' => nil, 'command' => nil, 'ports' => [], + 'variables' => [{ 'key' => 'MYSQL_ROOT_PASSWORD', 'value' => 'root123.' }], 'pull_policy' => nil } + ]) expect(json_response['steps']).to eq(expected_steps) expect(json_response['artifacts']).to eq(expected_artifacts) expect(json_response['cache']).to match(expected_cache) @@ -383,23 +384,24 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do expect(response).to have_gitlab_http_status(:created) expect(response.headers).not_to have_key('X-GitLab-Last-Update') - expect(json_response['steps']).to eq([ - { - "name" => "script", - "script" => ["make changelog | tee release_changelog.txt"], - "timeout" => 3600, - "when" => "on_success", - "allow_failure" => false - }, - { - "name" => "release", - "script" => - ["release-cli create --name \"Release $CI_COMMIT_SHA\" --description \"Created using the release-cli $EXTRA_DESCRIPTION\" --tag-name \"release-$CI_COMMIT_SHA\" --ref \"$CI_COMMIT_SHA\" --assets-link \"{\\\"url\\\":\\\"https://example.com/assets/1\\\",\\\"name\\\":\\\"asset1\\\"}\""], - "timeout" => 3600, - "when" => "on_success", - "allow_failure" => false - } - ]) + expect(json_response['steps']).to eq( + [ + { + "name" => "script", + "script" => ["make changelog | tee release_changelog.txt"], + "timeout" => 3600, + "when" => "on_success", + "allow_failure" => false + }, + { + "name" => "release", + "script" => + ["release-cli create --name \"Release $CI_COMMIT_SHA\" --description \"Created using the release-cli $EXTRA_DESCRIPTION\" --tag-name \"release-$CI_COMMIT_SHA\" --ref \"$CI_COMMIT_SHA\" --assets-link \"{\\\"url\\\":\\\"https://example.com/assets/1\\\",\\\"name\\\":\\\"asset1\\\"}\""], + "timeout" => 3600, + "when" => "on_success", + "allow_failure" => false + } + ]) end end diff --git a/spec/requests/api/ci/runner/runners_reset_spec.rb b/spec/requests/api/ci/runner/runners_reset_spec.rb index 8a61012ead1..02b66a89a0a 100644 --- a/spec/requests/api/ci/runner/runners_reset_spec.rb +++ b/spec/requests/api/ci/runner/runners_reset_spec.rb @@ -9,7 +9,6 @@ RSpec.describe API::Ci::Runner, :clean_gitlab_redis_shared_state do before do stub_feature_flags(ci_enable_live_trace: true) - stub_feature_flags(runner_registration_control: false) stub_gitlab_calls stub_application_setting(valid_runner_registrars: ApplicationSetting::VALID_RUNNER_REGISTRAR_TYPES) end diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index fa1f713e757..69f26d3f257 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -918,10 +918,11 @@ RSpec.describe API::Ci::Runners do create(:ci_build, :failed, runner: shared_runner, project: project_with_repo, pipeline: pipeline) expect_next_instance_of(Repository) do |repo| - expect(repo).to receive(:commits_by).with(oids: %w[ - 1a0b36b3cdad1d2ee32457c102a8c0b7056fa863 - c1c67abbaf91f624347bb3ae96eabe3a1b742478 - ]).once.and_call_original + expect(repo).to receive(:commits_by).with(oids: + %w[ + 1a0b36b3cdad1d2ee32457c102a8c0b7056fa863 + c1c67abbaf91f624347bb3ae96eabe3a1b742478 + ]).once.and_call_original end get api("/runners/#{shared_runner.id}/jobs", admin), params: { per_page: 2, order_by: 'id', sort: 'desc' } @@ -1124,30 +1125,27 @@ RSpec.describe API::Ci::Runners do it 'returns all runners' do get api("/groups/#{group.id}/runners", user) - expect(json_response).to match_array([ - a_hash_including('description' => 'Group runner A', 'active' => true, 'paused' => false), - a_hash_including('description' => 'Shared runner', 'active' => true, 'paused' => false) - ]) + expect(json_response).to match_array( + [ + a_hash_including('description' => 'Group runner A', 'active' => true, 'paused' => false), + a_hash_including('description' => 'Shared runner', 'active' => true, 'paused' => false) + ]) end context 'filter by type' do it 'returns record when valid and present' do get api("/groups/#{group.id}/runners?type=group_type", user) - expect(json_response).to match_array([ - a_hash_including('description' => 'Group runner A') - ]) + expect(json_response).to match_array([a_hash_including('description' => 'Group runner A')]) end it 'returns instance runners when instance_type is specified' do get api("/groups/#{group.id}/runners?type=instance_type", user) - expect(json_response).to match_array([ - a_hash_including('description' => 'Shared runner') - ]) + expect(json_response).to match_array([a_hash_including('description' => 'Shared runner')]) end - # TODO: Remove in %15.0 (https://gitlab.com/gitlab-org/gitlab/-/issues/351466) + # TODO: Remove when REST API v5 is implemented (https://gitlab.com/gitlab-org/gitlab/-/issues/351466) it 'returns empty result when type does not match' do get api("/groups/#{group.id}/runners?type=project_type", user) @@ -1167,18 +1165,14 @@ RSpec.describe API::Ci::Runners do it 'returns runners by paused state' do get api("/groups/#{group.id}/runners?paused=true", user) - expect(json_response).to match_array([ - a_hash_including('description' => 'Inactive group runner') - ]) + expect(json_response).to match_array([a_hash_including('description' => 'Inactive group runner')]) end context 'filter runners by status' do it 'returns runners by valid status' do get api("/groups/#{group.id}/runners?status=paused", user) - expect(json_response).to match_array([ - a_hash_including('description' => 'Inactive group runner') - ]) + expect(json_response).to match_array([a_hash_including('description' => 'Inactive group runner')]) end it 'does not filter by invalid status' do @@ -1195,9 +1189,7 @@ RSpec.describe API::Ci::Runners do get api("/groups/#{group.id}/runners?tag_list=tag1,tag2", user) - expect(json_response).to match_array([ - a_hash_including('description' => 'Runner tagged with tag1 and tag2') - ]) + expect(json_response).to match_array([a_hash_including('description' => 'Runner tagged with tag1 and tag2')]) end end diff --git a/spec/requests/api/deploy_tokens_spec.rb b/spec/requests/api/deploy_tokens_spec.rb index b5f8da1f327..e0296248a03 100644 --- a/spec/requests/api/deploy_tokens_spec.rb +++ b/spec/requests/api/deploy_tokens_spec.rb @@ -44,14 +44,15 @@ RSpec.describe API::DeployTokens do token_ids = json_response.map { |token| token['id'] } expect(response).to include_pagination_headers expect(response).to match_response_schema('public_api/v4/deploy_tokens') - expect(token_ids).to match_array([ - deploy_token.id, - revoked_deploy_token.id, - expired_deploy_token.id, - group_deploy_token.id, - revoked_group_deploy_token.id, - expired_group_deploy_token.id - ]) + expect(token_ids).to match_array( + [ + deploy_token.id, + revoked_deploy_token.id, + expired_deploy_token.id, + group_deploy_token.id, + revoked_group_deploy_token.id, + expired_group_deploy_token.id + ]) end context 'and active=true' do @@ -61,10 +62,11 @@ RSpec.describe API::DeployTokens do token_ids = json_response.map { |token| token['id'] } expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers - expect(token_ids).to match_array([ - deploy_token.id, - group_deploy_token.id - ]) + expect(token_ids).to match_array( + [ + deploy_token.id, + group_deploy_token.id + ]) end end end @@ -110,11 +112,12 @@ RSpec.describe API::DeployTokens do subject token_ids = json_response.map { |token| token['id'] } - expect(token_ids).to match_array([ - deploy_token.id, - expired_deploy_token.id, - revoked_deploy_token.id - ]) + expect(token_ids).to match_array( + [ + deploy_token.id, + expired_deploy_token.id, + revoked_deploy_token.id + ]) end context 'and active=true' do diff --git a/spec/requests/api/features_spec.rb b/spec/requests/api/features_spec.rb index b54be4f5258..d0334cf6dd2 100644 --- a/spec/requests/api/features_spec.rb +++ b/spec/requests/api/features_spec.rb @@ -92,402 +92,292 @@ RSpec.describe API::Features, stub_feature_flags: false do describe 'POST /feature' do let(:feature_name) { known_feature_flag.name } - context 'when the feature does not exist' do - it 'returns a 401 for anonymous users' do - post api("/features/#{feature_name}") + # TODO: remove this shared examples block when set_feature_flag_service feature flag + # is removed. Then remove also any duplicate specs covered by the service class. + shared_examples 'sets the feature flag status' do + context 'when the feature does not exist' do + it 'returns a 401 for anonymous users' do + post api("/features/#{feature_name}") - expect(response).to have_gitlab_http_status(:unauthorized) - end + expect(response).to have_gitlab_http_status(:unauthorized) + end - it 'returns a 403 for users' do - post api("/features/#{feature_name}", user) + it 'returns a 403 for users' do + post api("/features/#{feature_name}", user) - expect(response).to have_gitlab_http_status(:forbidden) - end + expect(response).to have_gitlab_http_status(:forbidden) + end - context 'when passed value=true' do - it 'creates an enabled feature' do - post api("/features/#{feature_name}", admin), params: { value: 'true' } + context 'when passed value=true' do + it 'creates an enabled feature' do + post api("/features/#{feature_name}", admin), params: { value: 'true' } - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'on', - 'gates' => [{ 'key' => 'boolean', 'value' => true }], - 'definition' => known_feature_flag_definition_hash - ) - end + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'on', + 'gates' => [{ 'key' => 'boolean', 'value' => true }], + 'definition' => known_feature_flag_definition_hash + ) + end - it 'logs the event' do - expect(Feature.logger).to receive(:info).once + it 'logs the event' do + expect(Feature.logger).to receive(:info).once - post api("/features/#{feature_name}", admin), params: { value: 'true' } - end + post api("/features/#{feature_name}", admin), params: { value: 'true' } + end - it 'creates an enabled feature for the given Flipper group when passed feature_group=perf_team' do - post api("/features/#{feature_name}", admin), params: { value: 'true', feature_group: 'perf_team' } + it 'creates an enabled feature for the given Flipper group when passed feature_group=perf_team' do + post api("/features/#{feature_name}", admin), params: { value: 'true', feature_group: 'perf_team' } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'groups', 'value' => ['perf_team'] } + ], + 'definition' => known_feature_flag_definition_hash + ) + end - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'groups', 'value' => ['perf_team'] } - ], - 'definition' => known_feature_flag_definition_hash - ) - end + it 'creates an enabled feature for the given user when passed user=username' do + post api("/features/#{feature_name}", admin), params: { value: 'true', user: user.username } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'actors', 'value' => ["User:#{user.id}"] } + ], + 'definition' => known_feature_flag_definition_hash + ) + end - it 'creates an enabled feature for the given user when passed user=username' do - post api("/features/#{feature_name}", admin), params: { value: 'true', user: user.username } + it 'creates an enabled feature for the given user and feature group when passed user=username and feature_group=perf_team' do + post api("/features/#{feature_name}", admin), params: { value: 'true', user: user.username, feature_group: 'perf_team' } - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ + expect(response).to have_gitlab_http_status(:created) + expect(json_response['name']).to eq(feature_name) + expect(json_response['state']).to eq('conditional') + expect(json_response['gates']).to contain_exactly( { 'key' => 'boolean', 'value' => false }, + { 'key' => 'groups', 'value' => ['perf_team'] }, { 'key' => 'actors', 'value' => ["User:#{user.id}"] } - ], - 'definition' => known_feature_flag_definition_hash - ) - end - - it 'creates an enabled feature for the given user and feature group when passed user=username and feature_group=perf_team' do - post api("/features/#{feature_name}", admin), params: { value: 'true', user: user.username, feature_group: 'perf_team' } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response['name']).to eq(feature_name) - expect(json_response['state']).to eq('conditional') - expect(json_response['gates']).to contain_exactly( - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'groups', 'value' => ['perf_team'] }, - { 'key' => 'actors', 'value' => ["User:#{user.id}"] } - ) + ) + end end - end - shared_examples 'does not enable the flag' do |actor_type| - let(:actor_path) { raise NotImplementedError } - let(:expected_inexistent_path) { actor_path } + shared_examples 'does not enable the flag' do |actor_type| + let(:actor_path) { raise NotImplementedError } + let(:expected_inexistent_path) { actor_path } - it 'returns the current state of the flag without changes' do - post api("/features/#{feature_name}", admin), params: { value: 'true', actor_type => actor_path } + it 'returns the current state of the flag without changes' do + post api("/features/#{feature_name}", admin), params: { value: 'true', actor_type => actor_path } - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']).to eq("400 Bad request - #{expected_inexistent_path} is not found!") + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq("400 Bad request - #{expected_inexistent_path} is not found!") + end end - end - shared_examples 'enables the flag for the actor' do |actor_type| - it 'sets the feature gate' do - post api("/features/#{feature_name}", admin), params: { value: 'true', actor_type => actor.full_path } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'actors', 'value' => ["#{actor.class}:#{actor.id}"] } - ], - 'definition' => known_feature_flag_definition_hash - ) + shared_examples 'enables the flag for the actor' do |actor_type| + it 'sets the feature gate' do + post api("/features/#{feature_name}", admin), params: { value: 'true', actor_type => actor.full_path } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'actors', 'value' => ["#{actor.class}:#{actor.id}"] } + ], + 'definition' => known_feature_flag_definition_hash + ) + end end - end - shared_examples 'creates an enabled feature for the specified entries' do - it do - post api("/features/#{feature_name}", admin), params: { value: 'true', **gate_params } + shared_examples 'creates an enabled feature for the specified entries' do + it do + post api("/features/#{feature_name}", admin), params: { value: 'true', **gate_params } - expect(response).to have_gitlab_http_status(:created) - expect(json_response['name']).to eq(feature_name) - expect(json_response['gates']).to contain_exactly( - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'actors', 'value' => array_including(expected_gate_params) } - ) + expect(response).to have_gitlab_http_status(:created) + expect(json_response['name']).to eq(feature_name) + expect(json_response['gates']).to contain_exactly( + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'actors', 'value' => array_including(expected_gate_params) } + ) + end end - end - context 'when enabling for a project by path' do - context 'when the project exists' do - it_behaves_like 'enables the flag for the actor', :project do - let(:actor) { create(:project) } + context 'when enabling for a project by path' do + context 'when the project exists' do + it_behaves_like 'enables the flag for the actor', :project do + let(:actor) { create(:project) } + end end - end - context 'when the project does not exist' do - it_behaves_like 'does not enable the flag', :project do - let(:actor_path) { 'mep/to/the/mep/mep' } + context 'when the project does not exist' do + it_behaves_like 'does not enable the flag', :project do + let(:actor_path) { 'mep/to/the/mep/mep' } + end end end - end - context 'when enabling for a group by path' do - context 'when the group exists' do - it_behaves_like 'enables the flag for the actor', :group do - let(:actor) { create(:group) } + context 'when enabling for a group by path' do + context 'when the group exists' do + it_behaves_like 'enables the flag for the actor', :group do + let(:actor) { create(:group) } + end end - end - context 'when the group does not exist' do - it_behaves_like 'does not enable the flag', :group do - let(:actor_path) { 'not/a/group' } + context 'when the group does not exist' do + it_behaves_like 'does not enable the flag', :group do + let(:actor_path) { 'not/a/group' } + end end end - end - context 'when enabling for a namespace by path' do - context 'when the user namespace exists' do - it_behaves_like 'enables the flag for the actor', :namespace do - let(:actor) { create(:namespace) } + context 'when enabling for a namespace by path' do + context 'when the user namespace exists' do + it_behaves_like 'enables the flag for the actor', :namespace do + let(:actor) { create(:namespace) } + end end - end - context 'when the group namespace exists' do - it_behaves_like 'enables the flag for the actor', :namespace do - let(:actor) { create(:group) } + context 'when the group namespace exists' do + it_behaves_like 'enables the flag for the actor', :namespace do + let(:actor) { create(:group) } + end end - end - context 'when the user namespace does not exist' do - it_behaves_like 'does not enable the flag', :namespace do - let(:actor_path) { 'not/a/group' } + context 'when the user namespace does not exist' do + it_behaves_like 'does not enable the flag', :namespace do + let(:actor_path) { 'not/a/group' } + end end - end - context 'when a project namespace exists' do - let(:project_namespace) { create(:project_namespace) } + context 'when a project namespace exists' do + let(:project_namespace) { create(:project_namespace) } - it_behaves_like 'does not enable the flag', :namespace do - let(:actor_path) { project_namespace.full_path } + it_behaves_like 'does not enable the flag', :namespace do + let(:actor_path) { project_namespace.full_path } + end end end - end - context 'with multiple users' do - let_it_be(:users) { create_list(:user, 3) } + context 'with multiple users' do + let_it_be(:users) { create_list(:user, 3) } - it_behaves_like 'creates an enabled feature for the specified entries' do - let(:gate_params) { { user: users.map(&:username).join(',') } } - let(:expected_gate_params) { users.map(&:flipper_id) } - end - - context 'when empty value exists between comma' do it_behaves_like 'creates an enabled feature for the specified entries' do - let(:gate_params) { { user: "#{users.first.username},,,," } } - let(:expected_gate_params) { users.first.flipper_id } + let(:gate_params) { { user: users.map(&:username).join(',') } } + let(:expected_gate_params) { users.map(&:flipper_id) } end - end - context 'when one of the users does not exist' do - it_behaves_like 'does not enable the flag', :user do - let(:actor_path) { "#{users.first.username},inexistent-entry" } - let(:expected_inexistent_path) { "inexistent-entry" } + context 'when empty value exists between comma' do + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { user: "#{users.first.username},,,," } } + let(:expected_gate_params) { users.first.flipper_id } + end end - end - end - context 'with multiple projects' do - let_it_be(:projects) { create_list(:project, 3) } - - it_behaves_like 'creates an enabled feature for the specified entries' do - let(:gate_params) { { project: projects.map(&:full_path).join(',') } } - let(:expected_gate_params) { projects.map(&:flipper_id) } - end - - context 'when empty value exists between comma' do - it_behaves_like 'creates an enabled feature for the specified entries' do - let(:gate_params) { { project: "#{projects.first.full_path},,,," } } - let(:expected_gate_params) { projects.first.flipper_id } + context 'when one of the users does not exist' do + it_behaves_like 'does not enable the flag', :user do + let(:actor_path) { "#{users.first.username},inexistent-entry" } + let(:expected_inexistent_path) { "inexistent-entry" } + end end end - context 'when one of the projects does not exist' do - it_behaves_like 'does not enable the flag', :project do - let(:actor_path) { "#{projects.first.full_path},inexistent-entry" } - let(:expected_inexistent_path) { "inexistent-entry" } - end - end - end - - context 'with multiple groups' do - let_it_be(:groups) { create_list(:group, 3) } + context 'with multiple projects' do + let_it_be(:projects) { create_list(:project, 3) } - it_behaves_like 'creates an enabled feature for the specified entries' do - let(:gate_params) { { group: groups.map(&:full_path).join(',') } } - let(:expected_gate_params) { groups.map(&:flipper_id) } - end - - context 'when empty value exists between comma' do it_behaves_like 'creates an enabled feature for the specified entries' do - let(:gate_params) { { group: "#{groups.first.full_path},,,," } } - let(:expected_gate_params) { groups.first.flipper_id } + let(:gate_params) { { project: projects.map(&:full_path).join(',') } } + let(:expected_gate_params) { projects.map(&:flipper_id) } end - end - context 'when one of the groups does not exist' do - it_behaves_like 'does not enable the flag', :group do - let(:actor_path) { "#{groups.first.full_path},inexistent-entry" } - let(:expected_inexistent_path) { "inexistent-entry" } + context 'when empty value exists between comma' do + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { project: "#{projects.first.full_path},,,," } } + let(:expected_gate_params) { projects.first.flipper_id } + end end - end - end - - context 'with multiple namespaces' do - let_it_be(:namespaces) { create_list(:namespace, 3) } - - it_behaves_like 'creates an enabled feature for the specified entries' do - let(:gate_params) { { namespace: namespaces.map(&:full_path).join(',') } } - let(:expected_gate_params) { namespaces.map(&:flipper_id) } - end - context 'when empty value exists between comma' do - it_behaves_like 'creates an enabled feature for the specified entries' do - let(:gate_params) { { namespace: "#{namespaces.first.full_path},,,," } } - let(:expected_gate_params) { namespaces.first.flipper_id } + context 'when one of the projects does not exist' do + it_behaves_like 'does not enable the flag', :project do + let(:actor_path) { "#{projects.first.full_path},inexistent-entry" } + let(:expected_inexistent_path) { "inexistent-entry" } + end end end - context 'when one of the namespaces does not exist' do - it_behaves_like 'does not enable the flag', :namespace do - let(:actor_path) { "#{namespaces.first.full_path},inexistent-entry" } - let(:expected_inexistent_path) { "inexistent-entry" } - end - end - end - - it 'creates a feature with the given percentage of time if passed an integer' do - post api("/features/#{feature_name}", admin), params: { value: '50' } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'percentage_of_time', 'value' => 50 } - ], - 'definition' => known_feature_flag_definition_hash - ) - end - - it 'creates a feature with the given percentage of time if passed a float' do - post api("/features/#{feature_name}", admin), params: { value: '0.01' } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'percentage_of_time', 'value' => 0.01 } - ], - 'definition' => known_feature_flag_definition_hash - ) - end - - it 'creates a feature with the given percentage of actors if passed an integer' do - post api("/features/#{feature_name}", admin), params: { value: '50', key: 'percentage_of_actors' } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'percentage_of_actors', 'value' => 50 } - ], - 'definition' => known_feature_flag_definition_hash - ) - end - - it 'creates a feature with the given percentage of actors if passed a float' do - post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors' } - - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'percentage_of_actors', 'value' => 0.01 } - ], - 'definition' => known_feature_flag_definition_hash - ) - end + context 'with multiple groups' do + let_it_be(:groups) { create_list(:group, 3) } - describe 'mutually exclusive parameters' do - shared_examples 'fails to set the feature flag' do - it 'returns an error' do - expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['error']).to match(/key, \w+ are mutually exclusive/) + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { group: groups.map(&:full_path).join(',') } } + let(:expected_gate_params) { groups.map(&:flipper_id) } end - end - context 'when key and feature_group are provided' do - before do - post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', feature_group: 'some-value' } + context 'when empty value exists between comma' do + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { group: "#{groups.first.full_path},,,," } } + let(:expected_gate_params) { groups.first.flipper_id } + end end - it_behaves_like 'fails to set the feature flag' - end - - context 'when key and user are provided' do - before do - post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', user: 'some-user' } + context 'when one of the groups does not exist' do + it_behaves_like 'does not enable the flag', :group do + let(:actor_path) { "#{groups.first.full_path},inexistent-entry" } + let(:expected_inexistent_path) { "inexistent-entry" } + end end - - it_behaves_like 'fails to set the feature flag' end - context 'when key and group are provided' do - before do - post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', group: 'somepath' } - end - - it_behaves_like 'fails to set the feature flag' - end + context 'with multiple namespaces' do + let_it_be(:namespaces) { create_list(:namespace, 3) } - context 'when key and namespace are provided' do - before do - post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', namespace: 'somepath' } + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { namespace: namespaces.map(&:full_path).join(',') } } + let(:expected_gate_params) { namespaces.map(&:flipper_id) } end - it_behaves_like 'fails to set the feature flag' - end - - context 'when key and project are provided' do - before do - post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', project: 'somepath' } + context 'when empty value exists between comma' do + it_behaves_like 'creates an enabled feature for the specified entries' do + let(:gate_params) { { namespace: "#{namespaces.first.full_path},,,," } } + let(:expected_gate_params) { namespaces.first.flipper_id } + end end - it_behaves_like 'fails to set the feature flag' + context 'when one of the namespaces does not exist' do + it_behaves_like 'does not enable the flag', :namespace do + let(:actor_path) { "#{namespaces.first.full_path},inexistent-entry" } + let(:expected_inexistent_path) { "inexistent-entry" } + end + end end - end - end - context 'when the feature exists' do - before do - Feature.disable(feature_name) # This also persists the feature on the DB - end - - context 'when passed value=true' do - it 'enables the feature' do - post api("/features/#{feature_name}", admin), params: { value: 'true' } + it 'creates a feature with the given percentage of time if passed an integer' do + post api("/features/#{feature_name}", admin), params: { value: '50' } expect(response).to have_gitlab_http_status(:created) expect(json_response).to match( 'name' => feature_name, - 'state' => 'on', - 'gates' => [{ 'key' => 'boolean', 'value' => true }], + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'percentage_of_time', 'value' => 50 } + ], 'definition' => known_feature_flag_definition_hash ) end - it 'enables the feature for the given Flipper group when passed feature_group=perf_team' do - post api("/features/#{feature_name}", admin), params: { value: 'true', feature_group: 'perf_team' } + it 'creates a feature with the given percentage of time if passed a float' do + post api("/features/#{feature_name}", admin), params: { value: '0.01' } expect(response).to have_gitlab_http_status(:created) expect(json_response).to match( @@ -495,14 +385,14 @@ RSpec.describe API::Features, stub_feature_flags: false do 'state' => 'conditional', 'gates' => [ { 'key' => 'boolean', 'value' => false }, - { 'key' => 'groups', 'value' => ['perf_team'] } + { 'key' => 'percentage_of_time', 'value' => 0.01 } ], 'definition' => known_feature_flag_definition_hash ) end - it 'enables the feature for the given user when passed user=username' do - post api("/features/#{feature_name}", admin), params: { value: 'true', user: user.username } + it 'creates a feature with the given percentage of actors if passed an integer' do + post api("/features/#{feature_name}", admin), params: { value: '50', key: 'percentage_of_actors' } expect(response).to have_gitlab_http_status(:created) expect(json_response).to match( @@ -510,102 +400,230 @@ RSpec.describe API::Features, stub_feature_flags: false do 'state' => 'conditional', 'gates' => [ { 'key' => 'boolean', 'value' => false }, - { 'key' => 'actors', 'value' => ["User:#{user.id}"] } + { 'key' => 'percentage_of_actors', 'value' => 50 } ], 'definition' => known_feature_flag_definition_hash ) end - end - - context 'when feature is enabled and value=false is passed' do - it 'disables the feature' do - Feature.enable(feature_name) - expect(Feature.enabled?(feature_name)).to eq(true) - post api("/features/#{feature_name}", admin), params: { value: 'false' } + it 'creates a feature with the given percentage of actors if passed a float' do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors' } expect(response).to have_gitlab_http_status(:created) expect(json_response).to match( 'name' => feature_name, - 'state' => 'off', - 'gates' => [{ 'key' => 'boolean', 'value' => false }], + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'percentage_of_actors', 'value' => 0.01 } + ], 'definition' => known_feature_flag_definition_hash ) end - it 'disables the feature for the given Flipper group when passed feature_group=perf_team' do - Feature.enable(feature_name, Feature.group(:perf_team)) - expect(Feature.enabled?(feature_name, admin)).to be_truthy + describe 'mutually exclusive parameters' do + shared_examples 'fails to set the feature flag' do + it 'returns an error' do + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['error']).to match(/key, \w+ are mutually exclusive/) + end + end - post api("/features/#{feature_name}", admin), params: { value: 'false', feature_group: 'perf_team' } + context 'when key and feature_group are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', feature_group: 'some-value' } + end - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'off', - 'gates' => [{ 'key' => 'boolean', 'value' => false }], - 'definition' => known_feature_flag_definition_hash - ) - end + it_behaves_like 'fails to set the feature flag' + end - it 'disables the feature for the given user when passed user=username' do - Feature.enable(feature_name, user) - expect(Feature.enabled?(feature_name, user)).to be_truthy + context 'when key and user are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', user: 'some-user' } + end - post api("/features/#{feature_name}", admin), params: { value: 'false', user: user.username } + it_behaves_like 'fails to set the feature flag' + end - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'off', - 'gates' => [{ 'key' => 'boolean', 'value' => false }], - 'definition' => known_feature_flag_definition_hash - ) + context 'when key and group are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', group: 'somepath' } + end + + it_behaves_like 'fails to set the feature flag' + end + + context 'when key and namespace are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', namespace: 'somepath' } + end + + it_behaves_like 'fails to set the feature flag' + end + + context 'when key and project are provided' do + before do + post api("/features/#{feature_name}", admin), params: { value: '0.01', key: 'percentage_of_actors', project: 'somepath' } + end + + it_behaves_like 'fails to set the feature flag' + end end end - context 'with a pre-existing percentage of time value' do + context 'when the feature exists' do before do - Feature.enable_percentage_of_time(feature_name, 50) + Feature.disable(feature_name) # This also persists the feature on the DB end - it 'updates the percentage of time if passed an integer' do - post api("/features/#{feature_name}", admin), params: { value: '30' } + context 'when passed value=true' do + it 'enables the feature' do + post api("/features/#{feature_name}", admin), params: { value: 'true' } - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'percentage_of_time', 'value' => 30 } - ], - 'definition' => known_feature_flag_definition_hash - ) + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'on', + 'gates' => [{ 'key' => 'boolean', 'value' => true }], + 'definition' => known_feature_flag_definition_hash + ) + end + + it 'enables the feature for the given Flipper group when passed feature_group=perf_team' do + post api("/features/#{feature_name}", admin), params: { value: 'true', feature_group: 'perf_team' } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'groups', 'value' => ['perf_team'] } + ], + 'definition' => known_feature_flag_definition_hash + ) + end + + it 'enables the feature for the given user when passed user=username' do + post api("/features/#{feature_name}", admin), params: { value: 'true', user: user.username } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'actors', 'value' => ["User:#{user.id}"] } + ], + 'definition' => known_feature_flag_definition_hash + ) + end end - end - context 'with a pre-existing percentage of actors value' do - before do - Feature.enable_percentage_of_actors(feature_name, 42) + context 'when feature is enabled and value=false is passed' do + it 'disables the feature' do + Feature.enable(feature_name) + expect(Feature.enabled?(feature_name)).to eq(true) + + post api("/features/#{feature_name}", admin), params: { value: 'false' } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'off', + 'gates' => [{ 'key' => 'boolean', 'value' => false }], + 'definition' => known_feature_flag_definition_hash + ) + end + + it 'disables the feature for the given Flipper group when passed feature_group=perf_team' do + Feature.enable(feature_name, Feature.group(:perf_team)) + expect(Feature.enabled?(feature_name, admin)).to be_truthy + + post api("/features/#{feature_name}", admin), params: { value: 'false', feature_group: 'perf_team' } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'off', + 'gates' => [{ 'key' => 'boolean', 'value' => false }], + 'definition' => known_feature_flag_definition_hash + ) + end + + it 'disables the feature for the given user when passed user=username' do + Feature.enable(feature_name, user) + expect(Feature.enabled?(feature_name, user)).to be_truthy + + post api("/features/#{feature_name}", admin), params: { value: 'false', user: user.username } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'off', + 'gates' => [{ 'key' => 'boolean', 'value' => false }], + 'definition' => known_feature_flag_definition_hash + ) + end end - it 'updates the percentage of actors if passed an integer' do - post api("/features/#{feature_name}", admin), params: { value: '74', key: 'percentage_of_actors' } + context 'with a pre-existing percentage of time value' do + before do + Feature.enable_percentage_of_time(feature_name, 50) + end - expect(response).to have_gitlab_http_status(:created) - expect(json_response).to match( - 'name' => feature_name, - 'state' => 'conditional', - 'gates' => [ - { 'key' => 'boolean', 'value' => false }, - { 'key' => 'percentage_of_actors', 'value' => 74 } - ], - 'definition' => known_feature_flag_definition_hash - ) + it 'updates the percentage of time if passed an integer' do + post api("/features/#{feature_name}", admin), params: { value: '30' } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'percentage_of_time', 'value' => 30 } + ], + 'definition' => known_feature_flag_definition_hash + ) + end + end + + context 'with a pre-existing percentage of actors value' do + before do + Feature.enable_percentage_of_actors(feature_name, 42) + end + + it 'updates the percentage of actors if passed an integer' do + post api("/features/#{feature_name}", admin), params: { value: '74', key: 'percentage_of_actors' } + + expect(response).to have_gitlab_http_status(:created) + expect(json_response).to match( + 'name' => feature_name, + 'state' => 'conditional', + 'gates' => [ + { 'key' => 'boolean', 'value' => false }, + { 'key' => 'percentage_of_actors', 'value' => 74 } + ], + 'definition' => known_feature_flag_definition_hash + ) + end end end end + + before do + stub_feature_flags(set_feature_flag_service: true) + end + + it_behaves_like 'sets the feature flag status' + + context 'when feature flag set_feature_flag_service is disabled' do + before do + stub_feature_flags(set_feature_flag_service: false) + end + + it_behaves_like 'sets the feature flag status' + end end describe 'DELETE /feature/:name' do diff --git a/spec/requests/api/generic_packages_spec.rb b/spec/requests/api/generic_packages_spec.rb index 823eafab734..0478e123086 100644 --- a/spec/requests/api/generic_packages_spec.rb +++ b/spec/requests/api/generic_packages_spec.rb @@ -602,6 +602,21 @@ RSpec.describe API::GenericPackages do end end + context 'with access to package registry for everyone' do + let_it_be(:user_role) { :anonymous } + + before do + project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + end + + it 'responds with success' do + download_file(auth_header) + + expect(response).to have_gitlab_http_status(:success) + end + end + context 'with package status' do where(:package_status, :expected_status) do :default | :success diff --git a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb index c19defa37e8..2dc7b9764fe 100644 --- a/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb +++ b/spec/requests/api/graphql/ci/ci_cd_setting_spec.rb @@ -48,6 +48,8 @@ RSpec.describe 'Getting Ci Cd Setting' do expect(settings_data['mergeTrainsEnabled']).to eql project.ci_cd_settings.merge_trains_enabled? expect(settings_data['keepLatestArtifact']).to eql project.keep_latest_artifacts_available? expect(settings_data['jobTokenScopeEnabled']).to eql project.ci_cd_settings.job_token_scope_enabled? + expect(settings_data['inboundJobTokenScopeEnabled']).to eql( + project.ci_cd_settings.inbound_job_token_scope_enabled?) end end end diff --git a/spec/requests/api/graphql/ci/config_spec.rb b/spec/requests/api/graphql/ci/config_spec.rb index 960fda80dd9..784019ee926 100644 --- a/spec/requests/api/graphql/ci/config_spec.rb +++ b/spec/requests/api/graphql/ci/config_spec.rb @@ -176,22 +176,22 @@ RSpec.describe 'Query.ciConfig' do "jobs" => { "nodes" => [ - { - "name" => "docker", - "groupName" => "docker", - "stage" => "test", - "script" => ["curl http://dockerhub/URL"], - "beforeScript" => ["bundle install", "bundle exec rake db:create"], - "afterScript" => ["echo 'run this after'"], - "allowFailure" => true, - "only" => { "refs" => %w[branches tags] }, - "when" => "manual", - "except" => { "refs" => ["branches"] }, - "environment" => nil, - "tags" => [], - "needs" => { "nodes" => [{ "name" => "spinach" }, { "name" => "rspec 0 1" }] } - } - ] + { + "name" => "docker", + "groupName" => "docker", + "stage" => "test", + "script" => ["curl http://dockerhub/URL"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => true, + "only" => { "refs" => %w[branches tags] }, + "when" => "manual", + "except" => { "refs" => ["branches"] }, + "environment" => nil, + "tags" => [], + "needs" => { "nodes" => [{ "name" => "spinach" }, { "name" => "rspec 0 1" }] } + } + ] } } ] @@ -209,22 +209,22 @@ RSpec.describe 'Query.ciConfig' do "jobs" => { "nodes" => [ - { - "name" => "deploy_job", - "groupName" => "deploy_job", - "stage" => "deploy", - "script" => ["echo 'done'"], - "beforeScript" => ["bundle install", "bundle exec rake db:create"], - "afterScript" => ["echo 'run this after'"], - "allowFailure" => false, - "only" => { "refs" => %w[branches tags] }, - "when" => "on_success", - "except" => nil, - "environment" => "production", - "tags" => [], - "needs" => { "nodes" => [] } - } - ] + { + "name" => "deploy_job", + "groupName" => "deploy_job", + "stage" => "deploy", + "script" => ["echo 'done'"], + "beforeScript" => ["bundle install", "bundle exec rake db:create"], + "afterScript" => ["echo 'run this after'"], + "allowFailure" => false, + "only" => { "refs" => %w[branches tags] }, + "when" => "on_success", + "except" => nil, + "environment" => "production", + "tags" => [], + "needs" => { "nodes" => [] } + } + ] } } ] diff --git a/spec/requests/api/graphql/ci/config_variables_spec.rb b/spec/requests/api/graphql/ci/config_variables_spec.rb index 2b5a5d0dc93..17133d7ea66 100644 --- a/spec/requests/api/graphql/ci/config_variables_spec.rb +++ b/spec/requests/api/graphql/ci/config_variables_spec.rb @@ -23,6 +23,7 @@ RSpec.describe 'Query.project(fullPath).ciConfigVariables(sha)' do ciConfigVariables(sha: "#{sha}") { key value + valueOptions description } } @@ -52,14 +53,22 @@ RSpec.describe 'Query.project(fullPath).ciConfigVariables(sha)' do post_graphql(query, current_user: user) expect(graphql_data.dig('project', 'ciConfigVariables')).to contain_exactly( + { + 'key' => 'KEY_VALUE_VAR', + 'value' => 'value x', + 'valueOptions' => nil, + 'description' => 'value of KEY_VALUE_VAR' + }, { 'key' => 'DB_NAME', 'value' => 'postgres', + 'valueOptions' => nil, 'description' => nil }, { 'key' => 'ENVIRONMENT_VAR', 'value' => 'env var value', + 'valueOptions' => ['env var value', 'env var value2'], 'description' => 'env var description' } ) diff --git a/spec/requests/api/graphql/ci/jobs_spec.rb b/spec/requests/api/graphql/ci/jobs_spec.rb index fa8fb1d54aa..47e3221c567 100644 --- a/spec/requests/api/graphql/ci/jobs_spec.rb +++ b/spec/requests/api/graphql/ci/jobs_spec.rb @@ -25,11 +25,12 @@ RSpec.describe 'Query.project.pipeline' do let(:first_n) { var('Int') } let(:query) do - with_signature([first_n], wrap_fields(query_graphql_path([ - [:project, { full_path: project.full_path }], - [:pipeline, { iid: pipeline.iid.to_s }], - [:stages, { first: first_n }] - ], stage_fields))) + with_signature([first_n], wrap_fields(query_graphql_path( + [ + [:project, { full_path: project.full_path }], + [:pipeline, { iid: pipeline.iid.to_s }], + [:stages, { first: first_n }] + ], stage_fields))) end let(:stage_fields) do diff --git a/spec/requests/api/graphql/ci/pipeline_schedules_spec.rb b/spec/requests/api/graphql/ci/pipeline_schedules_spec.rb new file mode 100644 index 00000000000..8b8ba09a95c --- /dev/null +++ b/spec/requests/api/graphql/ci/pipeline_schedules_spec.rb @@ -0,0 +1,88 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Query.project.pipelineSchedules' do + include GraphqlHelpers + + let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:user) { create(:user) } + let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + + let(:pipeline_schedule_graphql_data) { graphql_data_at(:project, :pipeline_schedules, :nodes, 0) } + + let(:params) { {} } + + let(:fields) do + <<~QUERY + nodes { + id + description + active + nextRunAt + realNextRun + lastPipeline { + id + } + refForDisplay + refPath + forTag + cron + cronTimezone + } + QUERY + end + + let(:query) do + %( + query { + project(fullPath: "#{project.full_path}") { + pipelineSchedules { + #{fields} + } + } + } + ) + end + + describe 'computed graphql fields' do + before do + pipeline_schedule.pipelines << build(:ci_pipeline, project: project) + + post_graphql(query, current_user: user) + end + + it_behaves_like 'a working graphql query' + + it 'returns calculated fields for a pipeline schedule' do + ref_for_display = pipeline_schedule_graphql_data['refForDisplay'] + + expect(ref_for_display).to eq('master') + expect(pipeline_schedule_graphql_data['refPath']).to eq("/#{project.full_path}/-/commits/#{ref_for_display}") + expect(pipeline_schedule_graphql_data['forTag']).to be(false) + end + end + + it 'avoids N+1 queries' do + create_pipeline_schedules(1) + + control = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: user) } + + create_pipeline_schedules(3) + + action = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: user) } + + expect(action).not_to exceed_query_limit(control) + end + + def create_pipeline_schedules(count) + create_list(:ci_pipeline_schedule, count, project: project) + .each do |pipeline_schedule| + create(:user).tap do |user| + project.add_developer(user) + pipeline_schedule.update!(owner: user) + end + pipeline_schedule.pipelines << build(:ci_pipeline, project: project) + end + end +end diff --git a/spec/requests/api/graphql/jobs_query_spec.rb b/spec/requests/api/graphql/jobs_query_spec.rb new file mode 100644 index 00000000000..5907566be7f --- /dev/null +++ b/spec/requests/api/graphql/jobs_query_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting job information' do + include GraphqlHelpers + + let_it_be(:job) { create(:ci_build, :success, name: 'job1') } + + let(:query) do + graphql_query_for(:jobs) + end + + context 'when user is admin' do + let_it_be(:current_user) { create(:admin) } + + it 'has full access to all jobs', :aggregate_failure do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:jobs, :count)).to eq(1) + expect(graphql_data_at(:jobs, :nodes)).to contain_exactly(a_graphql_entity_for(job)) + end + + context 'when filtered by status' do + let_it_be(:pending_job) { create(:ci_build, :pending) } + let_it_be(:failed_job) { create(:ci_build, :failed) } + + it 'gets pending jobs', :aggregate_failure do + post_graphql(graphql_query_for(:jobs, { statuses: :PENDING }), current_user: current_user) + + expect(graphql_data_at(:jobs, :count)).to eq(1) + expect(graphql_data_at(:jobs, :nodes)).to contain_exactly(a_graphql_entity_for(pending_job)) + end + + it 'gets pending and failed jobs', :aggregate_failure do + post_graphql(graphql_query_for(:jobs, { statuses: [:PENDING, :FAILED] }), current_user: current_user) + + expect(graphql_data_at(:jobs, :count)).to eq(2) + expect(graphql_data_at(:jobs, :nodes)).to match_array([a_graphql_entity_for(pending_job), + a_graphql_entity_for(failed_job)]) + end + end + end + + context 'if the user is not an admin' do + let_it_be(:current_user) { create(:user) } + + it 'has no access to the jobs', :aggregate_failure do + post_graphql(query, current_user: current_user) + + expect(graphql_data_at(:jobs, :count)).to eq(0) + expect(graphql_data_at(:jobs, :nodes)).to match_array([]) + end + end +end diff --git a/spec/requests/api/graphql/milestone_spec.rb b/spec/requests/api/graphql/milestone_spec.rb index f6835936418..78e7ec39ee3 100644 --- a/spec/requests/api/graphql/milestone_spec.rb +++ b/spec/requests/api/graphql/milestone_spec.rb @@ -5,8 +5,12 @@ require 'spec_helper' RSpec.describe 'Querying a Milestone' do include GraphqlHelpers + let_it_be(:group) { create(:group, :public) } + let_it_be(:project) { create(:project, group: group) } let_it_be(:guest) { create(:user) } - let_it_be(:project) { create(:project) } + let_it_be(:inherited_guest) { create(:user) } + let_it_be(:inherited_reporter) { create(:user) } + let_it_be(:inherited_developer) { create(:user) } let_it_be(:milestone) { create(:milestone, project: project) } let_it_be(:release_a) { create(:release, project: project) } let_it_be(:release_b) { create(:release, project: project) } @@ -14,116 +18,137 @@ RSpec.describe 'Querying a Milestone' do before_all do milestone.releases << [release_a, release_b] project.add_guest(guest) + group.add_guest(inherited_guest) + group.add_reporter(inherited_reporter) + group.add_developer(inherited_developer) end let(:expected_release_nodes) do contain_exactly(a_graphql_entity_for(release_a), a_graphql_entity_for(release_b)) end - context 'when we post the query' do - let(:current_user) { nil } - let(:query) do - graphql_query_for('milestone', { id: milestone.to_global_id.to_s }, all_graphql_fields_for('Milestone')) - end + shared_examples 'returns the milestone successfully' do + it_behaves_like 'a working graphql query' - subject { graphql_data['milestone'] } + it { is_expected.to include('title' => milestone.name) } - before do - post_graphql(query, current_user: current_user) + it 'contains release information' do + is_expected.to include('releases' => include('nodes' => expected_release_nodes)) end + end - context 'when the user has access to the milestone' do - let(:current_user) { guest } - - it_behaves_like 'a working graphql query' - - it { is_expected.to include('title' => milestone.name) } - - it 'contains release information' do - is_expected.to include('releases' => include('nodes' => expected_release_nodes)) + context 'when we post the query' do + context 'and the project is private' do + let(:query) do + graphql_query_for('milestone', { id: milestone.to_global_id.to_s }, all_graphql_fields_for('Milestone')) end - end - context 'when the user does not have access to the milestone' do - it_behaves_like 'a working graphql query' - - it { is_expected.to be_nil } - end + subject { graphql_data['milestone'] } - context 'when ID argument is missing' do - let(:query) do - graphql_query_for('milestone', {}, 'title') + before do + post_graphql(query, current_user: current_user) end - it 'raises an exception' do - expect(graphql_errors).to include(a_hash_including('message' => "Field 'milestone' is missing required arguments: id")) + context 'when the user is a direct project member' do + context 'and the user is a guest' do + let(:current_user) { guest } + + it_behaves_like 'returns the milestone successfully' + + context 'when there are two milestones' do + let_it_be(:milestone_b) { create(:milestone, project: project) } + + let(:milestone_fields) do + <<~GQL + fragment milestoneFields on Milestone { + #{all_graphql_fields_for('Milestone', max_depth: 1)} + releases { nodes { #{all_graphql_fields_for('Release', max_depth: 1)} } } + } + GQL + end + + let(:single_query) do + <<~GQL + query ($id_a: MilestoneID!) { + a: milestone(id: $id_a) { ...milestoneFields } + } + + #{milestone_fields} + GQL + end + + let(:multi_query) do + <<~GQL + query ($id_a: MilestoneID!, $id_b: MilestoneID!) { + a: milestone(id: $id_a) { ...milestoneFields } + b: milestone(id: $id_b) { ...milestoneFields } + } + #{milestone_fields} + GQL + end + + it 'returns the correct releases associated with each milestone' do + r = run_with_clean_state(multi_query, + context: { current_user: current_user }, + variables: { + id_a: global_id_of(milestone).to_s, + id_b: milestone_b.to_global_id.to_s + }) + + expect(r.to_h['errors']).to be_blank + expect(graphql_dig_at(r.to_h, :data, :a, :releases, :nodes)).to match expected_release_nodes + expect(graphql_dig_at(r.to_h, :data, :b, :releases, :nodes)).to be_empty + end + + it 'does not suffer from N+1 performance issues' do + baseline = ActiveRecord::QueryRecorder.new do + run_with_clean_state(single_query, + context: { current_user: current_user }, + variables: { id_a: milestone.to_global_id.to_s }) + end + + multi = ActiveRecord::QueryRecorder.new do + run_with_clean_state(multi_query, + context: { current_user: current_user }, + variables: { + id_a: milestone.to_global_id.to_s, + id_b: milestone_b.to_global_id.to_s + }) + end + + expect(multi).not_to exceed_query_limit(baseline) + end + end + end end - end - end - context 'when there are two milestones' do - let_it_be(:milestone_b) { create(:milestone, project: project) } - - let(:current_user) { guest } - let(:milestone_fields) do - <<~GQL - fragment milestoneFields on Milestone { - #{all_graphql_fields_for('Milestone', max_depth: 1)} - releases { nodes { #{all_graphql_fields_for('Release', max_depth: 1)} } } - } - GQL - end + context 'when the user is an inherited member from the group' do + where(:user) { [ref(:inherited_guest), ref(:inherited_reporter), ref(:inherited_developer)] } - let(:single_query) do - <<~GQL - query ($id_a: MilestoneID!) { - a: milestone(id: $id_a) { ...milestoneFields } - } + with_them do + let(:current_user) { user } - #{milestone_fields} - GQL - end + it_behaves_like 'returns the milestone successfully' + end + end - let(:multi_query) do - <<~GQL - query ($id_a: MilestoneID!, $id_b: MilestoneID!) { - a: milestone(id: $id_a) { ...milestoneFields } - b: milestone(id: $id_b) { ...milestoneFields } - } - #{milestone_fields} - GQL - end + context 'when unauthenticated' do + let(:current_user) { nil } - it 'produces correct results' do - r = run_with_clean_state(multi_query, - context: { current_user: current_user }, - variables: { - id_a: global_id_of(milestone).to_s, - id_b: milestone_b.to_global_id.to_s - }) - - expect(r.to_h['errors']).to be_blank - expect(graphql_dig_at(r.to_h, :data, :a, :releases, :nodes)).to match expected_release_nodes - expect(graphql_dig_at(r.to_h, :data, :b, :releases, :nodes)).to be_empty - end + it_behaves_like 'a working graphql query' - it 'does not suffer from N+1 performance issues' do - baseline = ActiveRecord::QueryRecorder.new do - run_with_clean_state(single_query, - context: { current_user: current_user }, - variables: { id_a: milestone.to_global_id.to_s }) - end + it { is_expected.to be_nil } - multi = ActiveRecord::QueryRecorder.new do - run_with_clean_state(multi_query, - context: { current_user: current_user }, - variables: { - id_a: milestone.to_global_id.to_s, - id_b: milestone_b.to_global_id.to_s - }) - end + context 'when ID argument is missing' do + let(:query) do + graphql_query_for('milestone', {}, 'title') + end - expect(multi).not_to exceed_query_limit(baseline) + it 'raises an exception' do + expect(graphql_errors).to include(a_hash_including('message' => "Field 'milestone' is missing required arguments: id")) + end + end + end end end end diff --git a/spec/requests/api/graphql/mutations/ci/job/artifacts_destroy_spec.rb b/spec/requests/api/graphql/mutations/ci/job/artifacts_destroy_spec.rb new file mode 100644 index 00000000000..bdad80995ea --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/job/artifacts_destroy_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'JobArtifactsDestroy' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:job) { create(:ci_build) } + + let(:mutation) do + variables = { + id: job.to_global_id.to_s + } + graphql_mutation(:job_artifacts_destroy, variables, <<~FIELDS) + job { + name + } + destroyedArtifactsCount + errors + FIELDS + end + + before do + create(:ci_job_artifact, :archive, job: job) + create(:ci_job_artifact, :junit, job: job) + end + + context 'when the user is not allowed to destroy the job artifacts' do + it 'returns an error' do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + expect(job.reload.job_artifacts.count).to be(2) + end + end + + context 'when the user is allowed to destroy the job artifacts' do + before do + job.project.add_maintainer(user) + end + + it 'destroys the job artifacts and returns the expected data' do + expected_data = { + 'jobArtifactsDestroy' => { + 'errors' => [], + 'destroyedArtifactsCount' => 2, + 'job' => { + 'name' => job.name + } + } + } + + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_data).to eq(expected_data) + expect(job.reload.job_artifacts.count).to be(0) + end + + context 'when the the project this job belongs to is undergoing stats refresh' do + it 'destroys no artifacts and returns the correct error' do + allow_next_found_instance_of(Project) do |project| + allow(project).to receive(:refreshing_build_artifacts_size?).and_return(true) + end + + expected_data = { + 'jobArtifactsDestroy' => { + 'errors' => ['Action temporarily disabled. The project this job belongs to is undergoing stats refresh.'], + 'destroyedArtifactsCount' => 0, + 'job' => { + 'name' => job.name + } + } + } + + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(graphql_data).to eq(expected_data) + expect(job.reload.job_artifacts.count).to be(2) + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb b/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb index 5269c60b50a..b2f84ab2869 100644 --- a/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_token_scope/add_project_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe 'CiJobTokenScopeAddProject' do include GraphqlHelpers - let_it_be(:project) { create(:project, ci_job_token_scope_enabled: true).tap(&:save!) } + let_it_be(:project) { create(:project, ci_outbound_job_token_scope_enabled: true).tap(&:save!) } let_it_be(:target_project) { create(:project) } let(:variables) do diff --git a/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb b/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb index b62291d1ebd..2b0adf89f40 100644 --- a/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/job_token_scope/remove_project_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe 'CiJobTokenScopeRemoveProject' do include GraphqlHelpers - let_it_be(:project) { create(:project, ci_job_token_scope_enabled: true).tap(&:save!) } + let_it_be(:project) { create(:project, ci_outbound_job_token_scope_enabled: true).tap(&:save!) } let_it_be(:target_project) { create(:project) } let_it_be(:link) do diff --git a/spec/requests/api/graphql/mutations/ci/pipeline_schedule_delete_spec.rb b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_delete_spec.rb new file mode 100644 index 00000000000..b197d223463 --- /dev/null +++ b/spec/requests/api/graphql/mutations/ci/pipeline_schedule_delete_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'PipelineScheduleDelete' do + include GraphqlHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) } + + let(:mutation) do + graphql_mutation( + :pipeline_schedule_delete, + { id: pipeline_schedule_id }, + <<-QL + errors + QL + ) + end + + let(:pipeline_schedule_id) { pipeline_schedule.to_global_id.to_s } + let(:mutation_response) { graphql_mutation_response(:pipeline_schedule_delete) } + + context 'when unauthorized' do + it 'returns an error' do + post_graphql_mutation(mutation, current_user: create(:user)) + + expect(graphql_errors).not_to be_empty + expect(graphql_errors[0]['message']) + .to eq( + "The resource that you are attempting to access does not exist " \ + "or you don't have permission to perform this action" + ) + end + end + + context 'when authorized' do + before do + project.add_maintainer(user) + end + + context 'when success' do + it do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + expect(mutation_response['errors']).to eq([]) + end + end + + context 'when failure' do + context 'when destroy fails' do + before do + allow_next_found_instance_of(Ci::PipelineSchedule) do |pipeline_schedule| + allow(pipeline_schedule).to receive(:destroy).and_return(false) + end + end + + it do + post_graphql_mutation(mutation, current_user: user) + + expect(response).to have_gitlab_http_status(:success) + + expect(mutation_response['errors']).to match_array(['Failed to remove the pipeline schedule']) + end + end + + context 'when pipeline schedule not found' do + let(:pipeline_schedule_id) { 'gid://gitlab/Ci::PipelineSchedule/0' } + + it do + post_graphql_mutation(mutation, current_user: user) + + expect(graphql_errors).not_to be_empty + expect(graphql_errors[0]['message']) + .to eq("Internal server error: Couldn't find Ci::PipelineSchedule with 'id'=0") + end + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb b/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb index 394d9ff53d1..c808cf5ede9 100644 --- a/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb +++ b/spec/requests/api/graphql/mutations/ci/project_ci_cd_settings_update_spec.rb @@ -6,15 +6,19 @@ RSpec.describe 'ProjectCiCdSettingsUpdate' do include GraphqlHelpers let_it_be(:project) do - create(:project, keep_latest_artifact: true, ci_job_token_scope_enabled: true) - .tap(&:save!) + create(:project, + keep_latest_artifact: true, + ci_outbound_job_token_scope_enabled: true, + ci_inbound_job_token_scope_enabled: true + ).tap(&:save!) end let(:variables) do { full_path: project.full_path, keep_latest_artifact: false, - job_token_scope_enabled: false + job_token_scope_enabled: false, + inbound_job_token_scope_enabled: false } end @@ -62,7 +66,7 @@ RSpec.describe 'ProjectCiCdSettingsUpdate' do project.reload expect(response).to have_gitlab_http_status(:success) - expect(project.ci_job_token_scope_enabled).to eq(false) + expect(project.ci_outbound_job_token_scope_enabled).to eq(false) end it 'does not update job_token_scope_enabled if not specified' do @@ -73,7 +77,44 @@ RSpec.describe 'ProjectCiCdSettingsUpdate' do project.reload expect(response).to have_gitlab_http_status(:success) - expect(project.ci_job_token_scope_enabled).to eq(true) + expect(project.ci_outbound_job_token_scope_enabled).to eq(true) + end + + describe 'inbound_job_token_scope_enabled' do + it 'updates inbound_job_token_scope_enabled' do + post_graphql_mutation(mutation, current_user: user) + + project.reload + + expect(response).to have_gitlab_http_status(:success) + expect(project.ci_inbound_job_token_scope_enabled).to eq(false) + end + + it 'does not update inbound_job_token_scope_enabled if not specified' do + variables.except!(:inbound_job_token_scope_enabled) + + post_graphql_mutation(mutation, current_user: user) + + project.reload + + expect(response).to have_gitlab_http_status(:success) + expect(project.ci_inbound_job_token_scope_enabled).to eq(true) + end + + context 'when ci_inbound_job_token_scope disabled' do + before do + stub_feature_flags(ci_inbound_job_token_scope: false) + end + + it 'does not update inbound_job_token_scope_enabled' do + post_graphql_mutation(mutation, current_user: user) + + project.reload + + expect(response).to have_gitlab_http_status(:success) + expect(project.ci_inbound_job_token_scope_enabled).to eq(true) + end + end end context 'when bad arguments are provided' do diff --git a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb index 194e42bf59d..567d8799d93 100644 --- a/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb +++ b/spec/requests/api/graphql/mutations/namespace/package_settings/update_spec.rb @@ -14,7 +14,13 @@ RSpec.describe 'Updating the package settings' do maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'foo-.*', generic_duplicates_allowed: false, - generic_duplicate_exception_regex: 'bar-.*' + generic_duplicate_exception_regex: 'bar-.*', + maven_package_requests_forwarding: true, + lock_maven_package_requests_forwarding: true, + npm_package_requests_forwarding: true, + lock_npm_package_requests_forwarding: true, + pypi_package_requests_forwarding: true, + lock_pypi_package_requests_forwarding: true } end @@ -26,6 +32,12 @@ RSpec.describe 'Updating the package settings' do mavenDuplicateExceptionRegex genericDuplicatesAllowed genericDuplicateExceptionRegex + mavenPackageRequestsForwarding + lockMavenPackageRequestsForwarding + npmPackageRequestsForwarding + lockNpmPackageRequestsForwarding + pypiPackageRequestsForwarding + lockPypiPackageRequestsForwarding } errors QL @@ -46,6 +58,12 @@ RSpec.describe 'Updating the package settings' do expect(package_settings_response['mavenDuplicateExceptionRegex']).to eq(params[:maven_duplicate_exception_regex]) expect(package_settings_response['genericDuplicatesAllowed']).to eq(params[:generic_duplicates_allowed]) expect(package_settings_response['genericDuplicateExceptionRegex']).to eq(params[:generic_duplicate_exception_regex]) + expect(package_settings_response['mavenPackageRequestsForwarding']).to eq(params[:maven_package_requests_forwarding]) + expect(package_settings_response['lockMavenPackageRequestsForwarding']).to eq(params[:lock_maven_package_requests_forwarding]) + expect(package_settings_response['pypiPackageRequestsForwarding']).to eq(params[:pypi_package_requests_forwarding]) + expect(package_settings_response['lockPypiPackageRequestsForwarding']).to eq(params[:lock_pypi_package_requests_forwarding]) + expect(package_settings_response['npmPackageRequestsForwarding']).to eq(params[:npm_package_requests_forwarding]) + expect(package_settings_response['lockNpmPackageRequestsForwarding']).to eq(params[:lock_npm_package_requests_forwarding]) end end @@ -75,8 +93,29 @@ RSpec.describe 'Updating the package settings' do RSpec.shared_examples 'accepting the mutation request updating the package settings' do it_behaves_like 'updating the namespace package setting attributes', - from: { maven_duplicates_allowed: true, maven_duplicate_exception_regex: 'SNAPSHOT', generic_duplicates_allowed: true, generic_duplicate_exception_regex: 'foo' }, - to: { maven_duplicates_allowed: false, maven_duplicate_exception_regex: 'foo-.*', generic_duplicates_allowed: false, generic_duplicate_exception_regex: 'bar-.*' } + from: { + maven_duplicates_allowed: true, + maven_duplicate_exception_regex: 'SNAPSHOT', + generic_duplicates_allowed: true, + generic_duplicate_exception_regex: 'foo', + maven_package_requests_forwarding: nil, + lock_maven_package_requests_forwarding: false, + npm_package_requests_forwarding: nil, + lock_npm_package_requests_forwarding: false, + pypi_package_requests_forwarding: nil, + lock_pypi_package_requests_forwarding: false + }, to: { + maven_duplicates_allowed: false, + maven_duplicate_exception_regex: 'foo-.*', + generic_duplicates_allowed: false, + generic_duplicate_exception_regex: 'bar-.*', + maven_package_requests_forwarding: true, + lock_maven_package_requests_forwarding: true, + npm_package_requests_forwarding: true, + lock_npm_package_requests_forwarding: true, + pypi_package_requests_forwarding: true, + lock_pypi_package_requests_forwarding: true + } it_behaves_like 'returning a success' it_behaves_like 'rejecting invalid regex' diff --git a/spec/requests/api/graphql/mutations/packages/bulk_destroy_spec.rb b/spec/requests/api/graphql/mutations/packages/bulk_destroy_spec.rb new file mode 100644 index 00000000000..1fe01af4f1c --- /dev/null +++ b/spec/requests/api/graphql/mutations/packages/bulk_destroy_spec.rb @@ -0,0 +1,128 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Destroying multiple packages' do + using RSpec::Parameterized::TableSyntax + + include GraphqlHelpers + + let_it_be(:project1) { create(:project) } + let_it_be(:project2) { create(:project) } + let_it_be(:user) { create(:user) } + let_it_be_with_reload(:packages1) { create_list(:package, 3, project: project1) } + let_it_be_with_reload(:packages2) { create_list(:package, 2, project: project2) } + + let(:ids) { packages1.append(packages2).flatten.map(&:to_global_id).map(&:to_s) } + + let(:query) do + <<~GQL + errors + GQL + end + + let(:params) do + { + ids: ids + } + end + + let(:mutation) { graphql_mutation(:destroy_packages, params, query) } + + describe 'post graphql mutation' do + subject(:mutation_request) { post_graphql_mutation(mutation, current_user: user) } + + shared_examples 'destroying the packages' do + it 'marks the packages as pending destruction' do + expect { mutation_request }.to change { ::Packages::Package.pending_destruction.count }.by(5) + end + + it_behaves_like 'returning response status', :success + end + + shared_examples 'denying the mutation request' do + |response = ::Gitlab::Graphql::Authorize::AuthorizeResource::RESOURCE_ACCESS_ERROR| + it 'does not mark the packages as pending destruction' do + expect { mutation_request }.not_to change { ::Packages::Package.pending_destruction.count } + expect_graphql_errors_to_include(response) + end + + it_behaves_like 'returning response status', :success + end + + context 'with valid params' do + where(:user_role, :shared_examples_name) do + :maintainer | 'destroying the packages' + :developer | 'denying the mutation request' + :reporter | 'denying the mutation request' + :guest | 'denying the mutation request' + :not_in_project | 'denying the mutation request' + end + + with_them do + before do + unless user_role == :not_in_project + project1.send("add_#{user_role}", user) + project2.send("add_#{user_role}", user) + end + end + + it_behaves_like params[:shared_examples_name] + end + + context 'for over the limit' do + before do + project1.add_maintainer(user) + project2.add_maintainer(user) + stub_const("Mutations::Packages::BulkDestroy::MAX_PACKAGES", 2) + end + + it_behaves_like 'denying the mutation request', ::Mutations::Packages::BulkDestroy::TOO_MANY_IDS_ERROR + end + + context 'with packages outside of the project' do + before do + project1.add_maintainer(user) + end + + it_behaves_like 'denying the mutation request' + end + end + + context 'with invalid params' do + let(:ids) { 'foo' } + + it_behaves_like 'denying the mutation request', 'invalid value for id' + end + + context 'with multi mutations' do + let(:package1) { packages1.first } + let(:package2) { packages2.first } + let(:query) do + <<~QUERY + mutation { + a: destroyPackages(input: { ids: ["#{package1.to_global_id}"]}) { + errors + } + b: destroyPackages(input: { ids: ["#{package2.to_global_id}"]}) { + errors + } + } + QUERY + end + + subject(:mutation_request) { post_graphql(query, current_user: user) } + + before do + project1.add_maintainer(user) + project2.add_maintainer(user) + end + + it 'executes the first mutation but not the second one' do + expect { mutation_request }.to change { package1.reload.status }.from('default').to('pending_destruction') + .and not_change { package2.reload.status } + expect_graphql_errors_to_include('"destroyPackages" field can be requested only for 1 Mutation(s) at a time.') + end + end + end +end diff --git a/spec/requests/api/graphql/mutations/uploads/delete_spec.rb b/spec/requests/api/graphql/mutations/uploads/delete_spec.rb index f44bf179397..2d1b33cc086 100644 --- a/spec/requests/api/graphql/mutations/uploads/delete_spec.rb +++ b/spec/requests/api/graphql/mutations/uploads/delete_spec.rb @@ -47,10 +47,11 @@ RSpec.describe 'Delete an upload' do expect(response).to have_gitlab_http_status(:success) expect(mutation_response['upload']).to be_nil - expect(mutation_response['errors']).to match_array([ - "The resource that you are attempting to access does not "\ - "exist or you don't have permission to perform this action." - ]) + expect(mutation_response['errors']).to match_array( + [ + "The resource that you are attempting to access does not "\ + "exist or you don't have permission to perform this action." + ]) end end end diff --git a/spec/requests/api/graphql/mutations/work_items/update_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_spec.rb index 909d6549fa5..6b0129c457f 100644 --- a/spec/requests/api/graphql/mutations/work_items/update_spec.rb +++ b/spec/requests/api/graphql/mutations/work_items/update_spec.rb @@ -144,6 +144,78 @@ RSpec.describe 'Update a work item' do end end + context 'with labels widget input' do + shared_examples 'mutation updating work item labels' do + it 'updates labels' do + expect do + post_graphql_mutation(mutation, current_user: current_user) + work_item.reload + end.to change { work_item.labels.count }.to(expected_labels.count) + + expect(work_item.labels).to match_array(expected_labels) + expect(mutation_response['workItem']['widgets']).to include( + 'labels' => { + 'nodes' => match_array(expected_labels.map { |l| { 'id' => l.to_gid.to_s } }) + }, + 'type' => 'LABELS' + ) + end + end + + let_it_be(:existing_label) { create(:label, project: project) } + let_it_be(:label1) { create(:label, project: project) } + let_it_be(:label2) { create(:label, project: project) } + + let(:fields) do + <<~FIELDS + workItem { + widgets { + type + ... on WorkItemWidgetLabels { + labels { + nodes { id } + } + } + } + } + errors + FIELDS + end + + let(:input) do + { 'labelsWidget' => { 'addLabelIds' => add_label_ids, 'removeLabelIds' => remove_label_ids } } + end + + let(:add_label_ids) { [] } + let(:remove_label_ids) { [] } + + before_all do + work_item.update!(labels: [existing_label]) + end + + context 'when only removing labels' do + let(:remove_label_ids) { [existing_label.to_gid.to_s] } + let(:expected_labels) { [] } + + it_behaves_like 'mutation updating work item labels' + end + + context 'when only adding labels' do + let(:add_label_ids) { [label1.to_gid.to_s, label2.to_gid.to_s] } + let(:expected_labels) { [label1, label2, existing_label] } + + it_behaves_like 'mutation updating work item labels' + end + + context 'when adding and removing labels' do + let(:remove_label_ids) { [existing_label.to_gid.to_s] } + let(:add_label_ids) { [label1.to_gid.to_s, label2.to_gid.to_s] } + let(:expected_labels) { [label1, label2] } + + it_behaves_like 'mutation updating work item labels' + end + end + context 'with due and start date widget input' do let(:start_date) { Date.today } let(:due_date) { 1.week.from_now.to_date } diff --git a/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb b/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb deleted file mode 100644 index 2a5cb937a2f..00000000000 --- a/spec/requests/api/graphql/mutations/work_items/update_widgets_spec.rb +++ /dev/null @@ -1,61 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe 'Update work item widgets' do - include GraphqlHelpers - - let_it_be(:project) { create(:project) } - let_it_be(:developer) { create(:user).tap { |user| project.add_developer(user) } } - let_it_be(:work_item, refind: true) { create(:work_item, project: project) } - - let(:input) { { 'descriptionWidget' => { 'description' => 'updated description' } } } - let(:mutation_response) { graphql_mutation_response(:work_item_update_widgets) } - let(:mutation) do - graphql_mutation(:workItemUpdateWidgets, input.merge('id' => work_item.to_global_id.to_s), <<~FIELDS) - errors - workItem { - description - widgets { - type - ... on WorkItemWidgetDescription { - description - } - } - } - FIELDS - end - - context 'the user is not allowed to update a work item' do - let(:current_user) { create(:user) } - - it_behaves_like 'a mutation that returns a top-level access error' - end - - context 'when user has permissions to update a work item', :aggregate_failures do - let(:current_user) { developer } - - it_behaves_like 'update work item description widget' do - let(:new_description) { 'updated description' } - end - - it_behaves_like 'has spam protection' do - let(:mutation_class) { ::Mutations::WorkItems::UpdateWidgets } - end - - context 'when the work_items feature flag is disabled' do - before do - stub_feature_flags(work_items: false) - end - - it 'does not update the work item and returns and error' do - expect do - post_graphql_mutation(mutation, current_user: current_user) - work_item.reload - end.to not_change(work_item, :description) - - expect(mutation_response['errors']).to contain_exactly('`work_items` feature flag disabled for this project') - end - end - end -end diff --git a/spec/requests/api/graphql/project/branch_rules_spec.rb b/spec/requests/api/graphql/project/branch_rules_spec.rb index 70fb37941e2..1aaf0e9edc7 100644 --- a/spec/requests/api/graphql/project/branch_rules_spec.rb +++ b/spec/requests/api/graphql/project/branch_rules_spec.rb @@ -21,27 +21,24 @@ RSpec.describe 'getting list of branch rules for a project' do let(:branch_rules_data) { graphql_data_at('project', 'branchRules', 'edges') } let(:variables) { { path: project.full_path } } - - let(:fields) do - <<~QUERY - pageInfo { - hasNextPage - hasPreviousPage - } - edges { - cursor - node { - #{all_graphql_fields_for('branch_rules'.classify)} - } - } - QUERY - end - + # fields must use let as the all_graphql_fields_for also configures some spies + let(:fields) { all_graphql_fields_for('BranchRule') } let(:query) do <<~GQL query($path: ID!, $n: Int, $cursor: String) { project(fullPath: $path) { - branchRules(first: $n, after: $cursor) { #{fields} } + branchRules(first: $n, after: $cursor) { + pageInfo { + hasNextPage + hasPreviousPage + } + edges { + cursor + node { + #{fields} + } + } + } } } GQL @@ -55,7 +52,9 @@ RSpec.describe 'getting list of branch rules for a project' do it_behaves_like 'a working graphql query' - it { expect(branch_rules_data).to be_empty } + it 'hides branch rules data' do + expect(branch_rules_data).to be_empty + end end context 'when the user does have read_protected_branch abilities' do @@ -66,12 +65,17 @@ RSpec.describe 'getting list of branch rules for a project' do it_behaves_like 'a working graphql query' - it 'includes a name' do + it 'returns branch rules data' do expect(branch_rules_data.dig(0, 'node', 'name')).to be_present - end - - it 'includes created_at and updated_at' do + expect(branch_rules_data.dig(0, 'node', 'isDefault')).to be(true).or be(false) + expect(branch_rules_data.dig(0, 'node', 'branchProtection')).to be_present expect(branch_rules_data.dig(0, 'node', 'createdAt')).to be_present + expect(branch_rules_data.dig(0, 'node', 'updatedAt')).to be_present + + expect(branch_rules_data.dig(1, 'node', 'name')).to be_present + expect(branch_rules_data.dig(1, 'node', 'isDefault')).to be(true).or be(false) + expect(branch_rules_data.dig(1, 'node', 'branchProtection')).to be_present + expect(branch_rules_data.dig(1, 'node', 'createdAt')).to be_present expect(branch_rules_data.dig(1, 'node', 'updatedAt')).to be_present end @@ -82,16 +86,16 @@ RSpec.describe 'getting list of branch rules for a project' do { path: project.full_path, n: branch_rule_limit, cursor: last_cursor } end - it_behaves_like 'a working graphql query' do - it 'only returns N branch_rules' do - expect(branch_rules_data.size).to eq(branch_rule_limit) - expect(has_next_page).to be_truthy - expect(has_prev_page).to be_falsey - post_graphql(query, current_user: current_user, variables: next_variables) - expect(branch_rules_data.size).to eq(branch_rule_limit) - expect(has_next_page).to be_falsey - expect(has_prev_page).to be_truthy - end + it_behaves_like 'a working graphql query' + + it 'returns pagination information' do + expect(branch_rules_data.size).to eq(branch_rule_limit) + expect(has_next_page).to be_truthy + expect(has_prev_page).to be_falsey + post_graphql(query, current_user: current_user, variables: next_variables) + expect(branch_rules_data.size).to eq(branch_rule_limit) + expect(has_next_page).to be_falsey + expect(has_prev_page).to be_truthy end context 'when no limit is provided' do diff --git a/spec/requests/api/graphql/project/cluster_agents_spec.rb b/spec/requests/api/graphql/project/cluster_agents_spec.rb index a34df0ee6f4..bb716cf2849 100644 --- a/spec/requests/api/graphql/project/cluster_agents_spec.rb +++ b/spec/requests/api/graphql/project/cluster_agents_spec.rb @@ -61,11 +61,12 @@ RSpec.describe 'Project.cluster_agents' do tokens = graphql_data_at(:project, :cluster_agents, :nodes, :tokens, :nodes) - expect(tokens).to match([ - a_graphql_entity_for(token_3), - a_graphql_entity_for(token_2), - a_graphql_entity_for(token_1) - ]) + expect(tokens).to match( + [ + a_graphql_entity_for(token_3), + a_graphql_entity_for(token_2), + a_graphql_entity_for(token_1) + ]) end it 'does not suffer from N+1 performance issues' do diff --git a/spec/requests/api/graphql/project/issue/designs/designs_spec.rb b/spec/requests/api/graphql/project/issue/designs/designs_spec.rb index 02bc9457c07..965534654ea 100644 --- a/spec/requests/api/graphql/project/issue/designs/designs_spec.rb +++ b/spec/requests/api/graphql/project/issue/designs/designs_spec.rb @@ -245,9 +245,10 @@ RSpec.describe 'Getting designs related to an issue' do end it 'only returns one version record for the design (the original version)' do - expect(version_nodes).to eq([ - [{ 'node' => { 'id' => global_id(version) } }] - ]) + expect(version_nodes).to eq( + [ + [{ 'node' => { 'id' => global_id(version) } }] + ]) end end @@ -289,10 +290,11 @@ RSpec.describe 'Getting designs related to an issue' do end it 'returns the correct versions records for both designs' do - expect(version_nodes).to eq([ - [{ 'node' => { 'id' => global_id(design.versions.first) } }], - [{ 'node' => { 'id' => global_id(second_design.versions.first) } }] - ]) + expect(version_nodes).to eq( + [ + [{ 'node' => { 'id' => global_id(design.versions.first) } }], + [{ 'node' => { 'id' => global_id(second_design.versions.first) } }] + ]) end end @@ -341,15 +343,16 @@ RSpec.describe 'Getting designs related to an issue' do end it 'returns all versions records for the designs' do - expect(version_nodes).to eq([ - [ - { 'node' => { 'id' => global_id(design.versions.first) } } - ], + expect(version_nodes).to eq( [ - { 'node' => { 'id' => global_id(second_design.versions.second) } }, - { 'node' => { 'id' => global_id(second_design.versions.first) } } - ] - ]) + [ + { 'node' => { 'id' => global_id(design.versions.first) } } + ], + [ + { 'node' => { 'id' => global_id(second_design.versions.second) } }, + { 'node' => { 'id' => global_id(second_design.versions.first) } } + ] + ]) end end end diff --git a/spec/requests/api/graphql/project/issues_spec.rb b/spec/requests/api/graphql/project/issues_spec.rb index 28282860416..3b8beb4f798 100644 --- a/spec/requests/api/graphql/project/issues_spec.rb +++ b/spec/requests/api/graphql/project/issues_spec.rb @@ -662,6 +662,7 @@ RSpec.describe 'getting an issue list for a project' do include_examples 'N+1 query check' end + context 'when requesting `closed_as_duplicate_of`' do let(:requested_fields) { 'closedAsDuplicateOf { id }' } let(:issue_a_dup) { create(:issue, project: project) } @@ -674,6 +675,55 @@ RSpec.describe 'getting an issue list for a project' do include_examples 'N+1 query check' end + + context 'when award emoji votes' do + let(:requested_fields) { [:upvotes, :downvotes] } + + before do + create_list(:award_emoji, 2, name: 'thumbsup', awardable: issue_a) + create_list(:award_emoji, 2, name: 'thumbsdown', awardable: issue_b) + end + + include_examples 'N+1 query check' + end + + context 'when requesting participants' do + let_it_be(:issue_c) { create(:issue, project: project) } + + let(:search_params) { { iids: [issue_a.iid.to_s, issue_c.iid.to_s] } } + let(:requested_fields) { 'participants { nodes { name } }' } + + before do + create(:award_emoji, :upvote, awardable: issue_a) + create(:award_emoji, :upvote, awardable: issue_b) + create(:award_emoji, :upvote, awardable: issue_c) + + note_with_emoji_a = create(:note_on_issue, noteable: issue_a, project: project) + note_with_emoji_b = create(:note_on_issue, noteable: issue_b, project: project) + note_with_emoji_c = create(:note_on_issue, noteable: issue_c, project: project) + + create(:award_emoji, :upvote, awardable: note_with_emoji_a) + create(:award_emoji, :upvote, awardable: note_with_emoji_b) + create(:award_emoji, :upvote, awardable: note_with_emoji_c) + end + + # Executes 3 extra queries to fetch participant_attrs + include_examples 'N+1 query check', threshold: 3 + end + + context 'when requesting labels' do + let(:requested_fields) { ['labels { nodes { id } }'] } + + before do + project_labels = create_list(:label, 2, project: project) + group_labels = create_list(:group_label, 2, group: group) + + issue_a.update!(labels: [project_labels.first, group_labels.first].flatten) + issue_b.update!(labels: [project_labels, group_labels].flatten) + end + + include_examples 'N+1 query check', skip_cached: false + end end def issues_ids diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index 5daec5543c0..2895737ae6f 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -5,12 +5,14 @@ require 'spec_helper' RSpec.describe 'getting merge request listings nested in a project' do include GraphqlHelpers - let_it_be(:project) { create(:project, :repository, :public) } + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :repository, :public, group: group) } let_it_be(:current_user) { create(:user) } let_it_be(:label) { create(:label, project: project) } + let_it_be(:group_label) { create(:group_label, group: group) } let_it_be_with_reload(:merge_request_a) do - create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label]) + create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label, group_label]) end let_it_be(:merge_request_b) do @@ -18,7 +20,7 @@ RSpec.describe 'getting merge request listings nested in a project' do end let_it_be(:merge_request_c) do - create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label]) + create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label, group_label]) end let_it_be(:merge_request_d) do @@ -327,6 +329,51 @@ RSpec.describe 'getting merge request listings nested in a project' do include_examples 'N+1 query check' end + + context 'when award emoji votes' do + let(:requested_fields) { [:upvotes, :downvotes] } + + before do + create_list(:award_emoji, 2, name: 'thumbsup', awardable: merge_request_a) + create_list(:award_emoji, 2, name: 'thumbsdown', awardable: merge_request_b) + end + + include_examples 'N+1 query check' + end + + context 'when requesting participants' do + let(:requested_fields) { 'participants { nodes { name } }' } + + before do + create(:award_emoji, :upvote, awardable: merge_request_a) + create(:award_emoji, :upvote, awardable: merge_request_b) + create(:award_emoji, :upvote, awardable: merge_request_c) + + note_with_emoji_a = create(:note_on_merge_request, noteable: merge_request_a, project: project) + note_with_emoji_b = create(:note_on_merge_request, noteable: merge_request_b, project: project) + note_with_emoji_c = create(:note_on_merge_request, noteable: merge_request_c, project: project) + + create(:award_emoji, :upvote, awardable: note_with_emoji_a) + create(:award_emoji, :upvote, awardable: note_with_emoji_b) + create(:award_emoji, :upvote, awardable: note_with_emoji_c) + end + + # Executes 3 extra queries to fetch participant_attrs + include_examples 'N+1 query check', threshold: 3 + end + + context 'when requesting labels' do + let(:requested_fields) { ['labels { nodes { id } }'] } + + before do + project_labels = create_list(:label, 2, project: project) + group_labels = create_list(:group_label, 2, group: group) + + merge_request_c.update!(labels: [project_labels, group_labels].flatten) + end + + include_examples 'N+1 query check', skip_cached: false + end end describe 'performance' do diff --git a/spec/requests/api/graphql/project/milestones_spec.rb b/spec/requests/api/graphql/project/milestones_spec.rb index d1ee157fc74..a577c367fe5 100644 --- a/spec/requests/api/graphql/project/milestones_spec.rb +++ b/spec/requests/api/graphql/project/milestones_spec.rb @@ -25,9 +25,10 @@ RSpec.describe 'getting milestone listings nested in a project' do graphql_query_for( :project, { full_path: project.full_path }, - query_graphql_field(:milestones, search_params, [ - query_graphql_field(:nodes, nil, %i[id title]) - ]) + query_graphql_field(:milestones, search_params, + [ + query_graphql_field(:nodes, nil, %i[id title]) + ]) ) end diff --git a/spec/requests/api/graphql/project/work_items_spec.rb b/spec/requests/api/graphql/project/work_items_spec.rb index 69f8d1cac74..e82f6ad24a2 100644 --- a/spec/requests/api/graphql/project/work_items_spec.rb +++ b/spec/requests/api/graphql/project/work_items_spec.rb @@ -2,16 +2,25 @@ require 'spec_helper' -RSpec.describe 'getting an work item list for a project' do +RSpec.describe 'getting a work item list for a project' do include GraphqlHelpers let_it_be(:group) { create(:group) } let_it_be(:project) { create(:project, :repository, :public, group: group) } let_it_be(:current_user) { create(:user) } + let_it_be(:label1) { create(:label, project: project) } + let_it_be(:label2) { create(:label, project: project) } - let_it_be(:item1) { create(:work_item, project: project, discussion_locked: true, title: 'item1') } + let_it_be(:item1) { create(:work_item, project: project, discussion_locked: true, title: 'item1', labels: [label1]) } let_it_be(:item2) do - create(:work_item, project: project, title: 'item2', last_edited_by: current_user, last_edited_at: 1.day.ago) + create( + :work_item, + project: project, + title: 'item2', + last_edited_by: current_user, + last_edited_at: 1.day.ago, + labels: [label2] + ) end let_it_be(:confidential_item) { create(:work_item, confidential: true, project: project, title: 'item3') } @@ -30,6 +39,70 @@ RSpec.describe 'getting an work item list for a project' do QUERY end + shared_examples 'work items resolver without N + 1 queries' do + it 'avoids N+1 queries' do + post_graphql(query, current_user: current_user) # warm-up + + control = ActiveRecord::QueryRecorder.new do + post_graphql(query, current_user: current_user) + end + + expect_graphql_errors_to_be_empty + + create_list( + :work_item, 3, + :task, + :last_edited_by_user, + last_edited_at: 1.week.ago, + project: project, + labels: [label1, label2] + ) + + expect_graphql_errors_to_be_empty + expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control) + end + end + + describe 'N + 1 queries' do + context 'when querying root fields' do + it_behaves_like 'work items resolver without N + 1 queries' + end + + # We need a separate example since all_graphql_fields_for will not fetch fields from types + # that implement the widget interface. Only `type` for the widgets field. + context 'when querying the widget interface' do + let(:fields) do + <<~GRAPHQL + nodes { + widgets { + type + ... on WorkItemWidgetDescription { + edited + lastEditedAt + lastEditedBy { + webPath + username + } + } + ... on WorkItemWidgetAssignees { + assignees { nodes { id } } + } + ... on WorkItemWidgetHierarchy { + parent { id } + } + ... on WorkItemWidgetLabels { + labels { nodes { id } } + allowsScopedLabels + } + } + } + GRAPHQL + end + + it_behaves_like 'work items resolver without N + 1 queries' + end + end + it_behaves_like 'a working graphql query' do before do post_graphql(query, current_user: current_user) @@ -78,40 +151,6 @@ RSpec.describe 'getting an work item list for a project' do end end - context 'when fetching description edit information' do - let(:fields) do - <<~GRAPHQL - nodes { - widgets { - type - ... on WorkItemWidgetDescription { - edited - lastEditedAt - lastEditedBy { - webPath - username - } - } - } - } - GRAPHQL - end - - it 'avoids N+1 queries' do - post_graphql(query, current_user: current_user) # warm-up - - control = ActiveRecord::QueryRecorder.new do - post_graphql(query, current_user: current_user) - end - expect_graphql_errors_to_be_empty - - create_list(:work_item, 3, :last_edited_by_user, last_edited_at: 1.week.ago, project: project) - - expect_graphql_errors_to_be_empty - expect { post_graphql(query, current_user: current_user) }.not_to exceed_query_limit(control) - end - end - context 'when filtering by search' do it_behaves_like 'query with a search term' do let(:issuable_data) { items_data } diff --git a/spec/requests/api/graphql/todo_query_spec.rb b/spec/requests/api/graphql/todo_query_spec.rb index be7242d95bd..7fe19448083 100644 --- a/spec/requests/api/graphql/todo_query_spec.rb +++ b/spec/requests/api/graphql/todo_query_spec.rb @@ -14,6 +14,11 @@ RSpec.describe 'Todo Query' do let_it_be(:todo) { create(:todo, user: todo_owner, target: issue) } let(:todo_subject) { todo } + + before do + project.add_developer(todo_owner) + end + let(:fields) do <<~GRAPHQL id @@ -31,10 +36,6 @@ RSpec.describe 'Todo Query' do graphql_query_for(:todo, { id: todo_subject.to_global_id.to_s }, fields) end - before do - project.add_developer(todo_owner) - end - subject(:graphql_response) do result = GitlabSchema.execute(query, context: { current_user: current_user }).to_h graphql_dig_at(result, :data, :todo) diff --git a/spec/requests/api/graphql/usage_trends_measurements_spec.rb b/spec/requests/api/graphql/usage_trends_measurements_spec.rb index 69a3ed7e09c..78a4321f522 100644 --- a/spec/requests/api/graphql/usage_trends_measurements_spec.rb +++ b/spec/requests/api/graphql/usage_trends_measurements_spec.rb @@ -17,19 +17,25 @@ RSpec.describe 'UsageTrendsMeasurements' do end it 'returns measurement objects' do - expect(graphql_data.dig('usageTrendsMeasurements', 'nodes')).to eq([ - { "count" => 10, 'identifier' => 'PROJECTS' }, - { "count" => 5, 'identifier' => 'PROJECTS' } - ]) + expect(graphql_data.dig('usageTrendsMeasurements', 'nodes')).to eq( + [ + { "count" => 10, 'identifier' => 'PROJECTS' }, + { "count" => 5, 'identifier' => 'PROJECTS' } + ]) end context 'with recorded_at filters' do - let(:arguments) { %(identifier: PROJECTS, recordedAfter: "#{15.days.ago.to_date}", recordedBefore: "#{5.days.ago.to_date}") } + let(:arguments) do + %(identifier: PROJECTS, + recordedAfter: "#{15.days.ago.to_date}", + recordedBefore: "#{5.days.ago.to_date}") + end it 'returns filtered measurement objects' do - expect(graphql_data.dig('usageTrendsMeasurements', 'nodes')).to eq([ - { "count" => 10, 'identifier' => 'PROJECTS' } - ]) + expect(graphql_data.dig('usageTrendsMeasurements', 'nodes')).to eq( + [ + { "count" => 10, 'identifier' => 'PROJECTS' } + ]) end end end diff --git a/spec/requests/api/graphql/work_item_spec.rb b/spec/requests/api/graphql/work_item_spec.rb index e4bb4109c76..2105e479ed2 100644 --- a/spec/requests/api/graphql/work_item_spec.rb +++ b/spec/requests/api/graphql/work_item_spec.rb @@ -128,10 +128,11 @@ RSpec.describe 'Query.work_item(id)' do hash_including( 'type' => 'HIERARCHY', 'parent' => nil, - 'children' => { 'nodes' => match_array([ - hash_including('id' => child_link1.work_item.to_gid.to_s), - hash_including('id' => child_link2.work_item.to_gid.to_s) - ]) } + 'children' => { 'nodes' => match_array( + [ + hash_including('id' => child_link1.work_item.to_gid.to_s), + hash_including('id' => child_link2.work_item.to_gid.to_s) + ]) } ) ) ) @@ -161,9 +162,10 @@ RSpec.describe 'Query.work_item(id)' do hash_including( 'type' => 'HIERARCHY', 'parent' => nil, - 'children' => { 'nodes' => match_array([ - hash_including('id' => child_link1.work_item.to_gid.to_s) - ]) } + 'children' => { 'nodes' => match_array( + [ + hash_including('id' => child_link1.work_item.to_gid.to_s) + ]) } ) ) ) diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 6169bc9b2a2..02d29601ceb 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -505,13 +505,35 @@ RSpec.describe API::Groups do group3.add_maintainer(user2) end - it 'returns an array of groups the user has at least master access' do - get api('/groups', user2), params: { min_access_level: 40 } + context 'with min_access_level parameter' do + it 'returns an array of groups the user has at least master access' do + get api('/groups', user2), params: { min_access_level: 40 } - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - expect(response_groups).to contain_exactly(group2.id, group3.id) + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(response_groups).to contain_exactly(group2.id, group3.id) + end + + context 'distinct count with present_groups_select_all feature flag' do + subject { get api('/groups', user2), params: { min_access_level: 40 } } + + it 'counts with *' do + count_sql = /#{Regexp.escape('SELECT count(*)')}/i + expect { subject }.to make_queries_matching count_sql + end + + context 'when present_groups_select_all feature flag is disabled' do + before do + stub_feature_flags(present_groups_select_all: false) + end + + it 'counts with count_column' do + count_sql = /#{Regexp.escape('SELECT count(count_column)')}/i + expect { subject }.to make_queries_matching count_sql + end + end + end end end diff --git a/spec/requests/api/helm_packages_spec.rb b/spec/requests/api/helm_packages_spec.rb index 5212e225351..6bd81f64913 100644 --- a/spec/requests/api/helm_packages_spec.rb +++ b/spec/requests/api/helm_packages_spec.rb @@ -73,6 +73,17 @@ RSpec.describe API::HelmPackages do end end + context 'with access to package registry for everyone' do + let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace } } + + before do + project.update!(visibility: Gitlab::VisibilityLevel::PRIVATE) + project.project_feature.update!(package_registry_access_level: ProjectFeature::PUBLIC) + end + + it_behaves_like 'process helm download content request', :anonymous, :success + end + context 'when an invalid token is passed' do let(:headers) { basic_auth_header(user.username, 'wrong') } diff --git a/spec/requests/api/import_github_spec.rb b/spec/requests/api/import_github_spec.rb index d2fa3dabe69..015a09d41ab 100644 --- a/spec/requests/api/import_github_spec.rb +++ b/spec/requests/api/import_github_spec.rb @@ -70,7 +70,8 @@ RSpec.describe API::ImportGithub do target_namespace: user.namespace_path, personal_access_token: token, repo_id: non_existing_record_id, - github_hostname: "https://github.somecompany.com/" + github_hostname: "https://github.somecompany.com/", + optional_stages: { attachments_import: true } } expect(response).to have_gitlab_http_status(:created) expect(json_response).to be_a Hash @@ -89,4 +90,42 @@ RSpec.describe API::ImportGithub do expect(response).to have_gitlab_http_status(:unprocessable_entity) end end + + describe "POST /import/github/cancel" do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :import_started, import_type: 'github', import_url: 'https://fake.url') } + + context 'when project import was canceled' do + before do + allow(Import::Github::CancelProjectImportService) + .to receive(:new).with(project, user) + .and_return(double(execute: { status: :success, project: project })) + end + + it 'returns success' do + post api("/import/github/cancel", user), params: { + project_id: project.id + } + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when project import was not canceled' do + before do + allow(Import::Github::CancelProjectImportService) + .to receive(:new).with(project, user) + .and_return(double(execute: { status: :error, message: 'The import cannot be canceled because it is finished', http_status: :bad_request })) + end + + it 'returns error' do + post api("/import/github/cancel", user), params: { + project_id: project.id + } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq('The import cannot be canceled because it is finished') + end + end + end end diff --git a/spec/requests/api/internal/base_spec.rb b/spec/requests/api/internal/base_spec.rb index 1f6c241b3f5..32cacfc713c 100644 --- a/spec/requests/api/internal/base_spec.rb +++ b/spec/requests/api/internal/base_spec.rb @@ -68,24 +68,6 @@ RSpec.describe API::Internal::Base do expect(response).to have_gitlab_http_status(:unauthorized) end - - context 'when gitlab_shell_jwt_token is disabled' do - before do - stub_feature_flags(gitlab_shell_jwt_token: false) - end - - it 'authenticates using a header' do - perform_request(headers: { API::Helpers::GITLAB_SHARED_SECRET_HEADER => Base64.encode64(secret_token) }) - - expect(response).to have_gitlab_http_status(:ok) - end - - it 'returns 401 when no credentials provided' do - get(api("/internal/check")) - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end end end @@ -1033,7 +1015,7 @@ RSpec.describe API::Internal::Base do context 'git push' do before do - stub_const('Gitlab::QueryLimiting::Transaction::THRESHOLD', 120) + allow(Gitlab::QueryLimiting::Transaction).to receive(:threshold).and_return(120) end subject { push_with_path(key, full_path: path, changes: '_any') } diff --git a/spec/requests/api/issues/issues_spec.rb b/spec/requests/api/issues/issues_spec.rb index dd7d32f3565..f5c73846173 100644 --- a/spec/requests/api/issues/issues_spec.rb +++ b/spec/requests/api/issues/issues_spec.rb @@ -1164,6 +1164,21 @@ RSpec.describe API::Issues do expect(json_response['title']).to eq('new issue') expect(json_response['issue_type']).to eq('issue') end + + context 'when issue create service returns an unrecoverable error' do + before do + allow_next_instance_of(Issues::CreateService) do |create_service| + allow(create_service).to receive(:execute).and_return(ServiceResponse.error(message: 'some error', http_status: 403)) + end + end + + it 'returns and error message and status code from the service' do + post api("/projects/#{project.id}/issues", user), params: { title: 'new issue' } + + expect(response).to have_gitlab_http_status(:forbidden) + expect(json_response['message']).to eq('some error') + end + end end describe 'PUT /projects/:id/issues/:issue_iid' do diff --git a/spec/requests/api/issues/post_projects_issues_spec.rb b/spec/requests/api/issues/post_projects_issues_spec.rb index 7c8994ad9ba..3883eb01391 100644 --- a/spec/requests/api/issues/post_projects_issues_spec.rb +++ b/spec/requests/api/issues/post_projects_issues_spec.rb @@ -274,9 +274,7 @@ RSpec.describe API::Issues do post api("/projects/#{project.id}/issues", user), params: { title: 'g' * 256 } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']['title']).to eq([ - 'is too long (maximum is 255 characters)' - ]) + expect(json_response['message']['title']).to eq(['is too long (maximum is 255 characters)']) end context 'resolving discussions' do diff --git a/spec/requests/api/issues/put_projects_issues_spec.rb b/spec/requests/api/issues/put_projects_issues_spec.rb index 6ea77cc6578..d6c57b460e0 100644 --- a/spec/requests/api/issues/put_projects_issues_spec.rb +++ b/spec/requests/api/issues/put_projects_issues_spec.rb @@ -381,9 +381,7 @@ RSpec.describe API::Issues do put api_for_user, params: { title: 'g' * 256 } expect(response).to have_gitlab_http_status(:bad_request) - expect(json_response['message']['title']).to eq([ - 'is too long (maximum is 255 characters)' - ]) + expect(json_response['message']['title']).to eq(['is too long (maximum is 255 characters)']) end end diff --git a/spec/requests/api/maven_packages_spec.rb b/spec/requests/api/maven_packages_spec.rb index d7cc6991ef4..d771c1e2dcc 100644 --- a/spec/requests/api/maven_packages_spec.rb +++ b/spec/requests/api/maven_packages_spec.rb @@ -254,7 +254,7 @@ RSpec.describe API::MavenPackages do let(:package_name) { package_in_project ? package_file.file_name : 'foo' } before do - allow_fetch_application_setting(attribute: 'maven_package_requests_forwarding', return_value: forward) + allow_fetch_cascade_application_setting(attribute: 'maven_package_requests_forwarding', return_value: forward) end it_behaves_like params[:shared_examples_name] @@ -273,7 +273,7 @@ RSpec.describe API::MavenPackages do before do stub_feature_flags(maven_central_request_forwarding: false) - allow_fetch_application_setting(attribute: 'maven_package_requests_forwarding', return_value: forward) + allow_fetch_cascade_application_setting(attribute: 'maven_package_requests_forwarding', return_value: forward) end it_behaves_like params[:shared_examples_name] @@ -438,21 +438,7 @@ RSpec.describe API::MavenPackages do it_behaves_like 'processing HEAD requests', instance_level: true end - context 'with check_maven_path_first enabled' do - before do - stub_feature_flags(check_maven_path_first: true) - end - - it_behaves_like 'handling groups, subgroups and user namespaces for', 'heading a file' - end - - context 'with check_maven_path_first disabled' do - before do - stub_feature_flags(check_maven_path_first: false) - end - - it_behaves_like 'handling groups, subgroups and user namespaces for', 'heading a file' - end + it_behaves_like 'handling groups, subgroups and user namespaces for', 'heading a file' end describe 'GET /api/v4/groups/:id/-/packages/maven/*path/:file_name' do @@ -668,21 +654,7 @@ RSpec.describe API::MavenPackages do let(:path) { package.maven_metadatum.path } let(:url) { "/groups/#{group.id}/-/packages/maven/#{path}/#{package_file.file_name}" } - context 'with check_maven_path_first enabled' do - before do - stub_feature_flags(check_maven_path_first: true) - end - - it_behaves_like 'handling groups and subgroups for', 'processing HEAD requests' - end - - context 'with check_maven_path_first disabled' do - before do - stub_feature_flags(check_maven_path_first: false) - end - - it_behaves_like 'handling groups and subgroups for', 'processing HEAD requests' - end + it_behaves_like 'handling groups and subgroups for', 'processing HEAD requests' end describe 'GET /api/v4/projects/:id/packages/maven/*path/:file_name' do @@ -774,21 +746,7 @@ RSpec.describe API::MavenPackages do let(:path) { package.maven_metadatum.path } let(:url) { "/projects/#{project.id}/packages/maven/#{path}/#{package_file.file_name}" } - context 'with check_maven_path_first enabled' do - before do - stub_feature_flags(check_maven_path_first: true) - end - - it_behaves_like 'processing HEAD requests' - end - - context 'with check_maven_path_first disabled' do - before do - stub_feature_flags(check_maven_path_first: false) - end - - it_behaves_like 'processing HEAD requests' - end + it_behaves_like 'processing HEAD requests' end describe 'PUT /api/v4/projects/:id/packages/maven/*path/:file_name/authorize' do diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 9d153286d14..d593e369d27 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -119,10 +119,13 @@ RSpec.describe API::MergeRequests do it 'returns an array of all merge_requests' do get api(endpoint_path, user) - expect_paginated_array_response([ - merge_request_merged.id, merge_request_locked.id, - merge_request_closed.id, merge_request.id - ]) + expect_paginated_array_response( + [ + merge_request_merged.id, + merge_request_locked.id, + merge_request_closed.id, + merge_request.id + ]) expect(json_response.last['title']).to eq(merge_request.title) expect(json_response.last).to have_key('web_url') @@ -172,10 +175,13 @@ RSpec.describe API::MergeRequests do get api(path, user) - expect_paginated_array_response([ - merge_request_merged.id, merge_request_locked.id, - merge_request_closed.id, merge_request.id - ]) + expect_paginated_array_response( + [ + merge_request_merged.id, + merge_request_locked.id, + merge_request_closed.id, + merge_request.id + ]) expect(json_response.last.keys).to match_array(%w(id iid title web_url created_at description project_id state updated_at)) expect(json_response.last['iid']).to eq(merge_request.iid) expect(json_response.last['title']).to eq(merge_request.title) @@ -190,10 +196,13 @@ RSpec.describe API::MergeRequests do get api(path, user) - expect_paginated_array_response([ - merge_request_merged.id, merge_request_locked.id, - merge_request_closed.id, merge_request.id - ]) + expect_paginated_array_response( + [ + merge_request_merged.id, + merge_request_locked.id, + merge_request_closed.id, + merge_request.id + ]) expect(json_response.last['title']).to eq(merge_request.title) end @@ -354,10 +363,13 @@ RSpec.describe API::MergeRequests do get api(path, user) - expect_paginated_array_response([ - merge_request.id, merge_request_closed.id, - merge_request_locked.id, merge_request_merged.id - ]) + expect_paginated_array_response( + [ + merge_request.id, + merge_request_closed.id, + merge_request_locked.id, + merge_request_merged.id + ]) response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end @@ -367,10 +379,13 @@ RSpec.describe API::MergeRequests do get api(path, user) - expect_paginated_array_response([ - merge_request_merged.id, merge_request_locked.id, - merge_request_closed.id, merge_request.id - ]) + expect_paginated_array_response( + [ + merge_request_merged.id, + merge_request_locked.id, + merge_request_closed.id, + merge_request.id + ]) response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -398,10 +413,13 @@ RSpec.describe API::MergeRequests do get api(path, user) - expect_paginated_array_response([ - merge_request.id, merge_request_locked.id, - merge_request_merged.id, merge_request_closed.id - ]) + expect_paginated_array_response( + [ + merge_request.id, + merge_request_locked.id, + merge_request_merged.id, + merge_request_closed.id + ]) response_dates = json_response.map { |merge_request| merge_request['updated_at'] } expect(response_dates).to eq(response_dates.sort.reverse) end @@ -411,10 +429,13 @@ RSpec.describe API::MergeRequests do get api(path, user) - expect_paginated_array_response([ - merge_request.id, merge_request_closed.id, - merge_request_locked.id, merge_request_merged.id - ]) + expect_paginated_array_response( + [ + merge_request.id, + merge_request_closed.id, + merge_request_locked.id, + merge_request_merged.id + ]) response_dates = json_response.map { |merge_request| merge_request['created_at'] } expect(response_dates).to eq(response_dates.sort) end @@ -1023,6 +1044,14 @@ RSpec.describe API::MergeRequests do it_behaves_like 'a non-cached MergeRequest api request', 1 end + context 'when the label changes' do + before do + merge_request.labels << create(:label, project: merge_request.project) + end + + it_behaves_like 'a non-cached MergeRequest api request', 1 + end + context 'when the assignees change' do before do merge_request.assignees << create(:user) @@ -3315,7 +3344,7 @@ RSpec.describe API::MergeRequests do end it 'handles external issues' do - jira_project = create(:jira_project, :public, :repository, name: 'JIR_EXT1') + jira_project = create(:project, :with_jira_integration, :public, :repository, name: 'JIR_EXT1') ext_issue = ExternalIssue.new("#{jira_project.name}-123", jira_project) issue = create(:issue, project: jira_project) description = "Closes #{ext_issue.to_reference(jira_project)}\ncloses #{issue.to_reference}" diff --git a/spec/requests/api/metadata_spec.rb b/spec/requests/api/metadata_spec.rb index dbca06b7f3e..5b6407c689b 100644 --- a/spec/requests/api/metadata_spec.rb +++ b/spec/requests/api/metadata_spec.rb @@ -6,7 +6,7 @@ RSpec.describe API::Metadata do shared_examples_for 'GET /metadata' do context 'when unauthenticated' do it 'returns authentication error' do - get api('/metadata') + get api(endpoint) expect(response).to have_gitlab_http_status(:unauthorized) end @@ -16,7 +16,7 @@ RSpec.describe API::Metadata do let(:user) { create(:user) } it 'returns the metadata information' do - get api('/metadata', user) + get api(endpoint, user) expect_metadata end @@ -29,13 +29,13 @@ RSpec.describe API::Metadata do let(:scopes) { %i(api) } it 'returns the metadata information' do - get api('/metadata', personal_access_token: personal_access_token) + get api(endpoint, personal_access_token: personal_access_token) expect_metadata end it 'returns "200" response on head requests' do - head api('/metadata', personal_access_token: personal_access_token) + head api(endpoint, personal_access_token: personal_access_token) expect(response).to have_gitlab_http_status(:ok) end @@ -45,13 +45,13 @@ RSpec.describe API::Metadata do let(:scopes) { %i(read_user) } it 'returns the metadata information' do - get api('/metadata', personal_access_token: personal_access_token) + get api(endpoint, personal_access_token: personal_access_token) expect_metadata end it 'returns "200" response on head requests' do - head api('/metadata', personal_access_token: personal_access_token) + head api(endpoint, personal_access_token: personal_access_token) expect(response).to have_gitlab_http_status(:ok) end @@ -61,7 +61,7 @@ RSpec.describe API::Metadata do let(:scopes) { %i(read_repository) } it 'returns authorization error' do - get api('/metadata', personal_access_token: personal_access_token) + get api(endpoint, personal_access_token: personal_access_token) expect(response).to have_gitlab_http_status(:forbidden) end @@ -76,18 +76,14 @@ RSpec.describe API::Metadata do end end - context 'with graphql enabled' do - before do - stub_feature_flags(graphql: true) - end + describe 'GET /metadata' do + let(:endpoint) { '/metadata' } include_examples 'GET /metadata' end - context 'with graphql disabled' do - before do - stub_feature_flags(graphql: false) - end + describe 'GET /version' do + let(:endpoint) { '/version' } include_examples 'GET /metadata' end diff --git a/spec/requests/api/ml/mlflow_spec.rb b/spec/requests/api/ml/mlflow_spec.rb index 4e7091a5b0f..09e9359c0b3 100644 --- a/spec/requests/api/ml/mlflow_spec.rb +++ b/spec/requests/api/ml/mlflow_spec.rb @@ -10,27 +10,35 @@ RSpec.describe API::Ml::Mlflow do let_it_be(:project) { create(:project, :private) } let_it_be(:developer) { create(:user).tap { |u| project.add_developer(u) } } + let_it_be(:another_project) { build(:project).tap { |p| p.add_developer(developer) } } let_it_be(:experiment) do create(:ml_experiments, user: project.creator, project: project) end let_it_be(:candidate) do - create(:ml_candidates, user: experiment.user, start_time: 1234, experiment: experiment) + create(:ml_candidates, + :with_metrics_and_params, user: experiment.user, start_time: 1234, experiment: experiment) end - let_it_be(:another_candidate) do - create(:ml_candidates, - experiment: create(:ml_experiments, project: create(:project))) + let_it_be(:tokens) do + { + write: create(:personal_access_token, scopes: %w[read_api api], user: developer), + read: create(:personal_access_token, scopes: %w[read_api], user: developer), + no_access: create(:personal_access_token, scopes: %w[read_user], user: developer), + different_user: create(:personal_access_token, scopes: %w[read_api api], user: build(:user)) + } end let(:current_user) { developer } let(:ff_value) { true } - let(:scopes) { %w[read_api api] } + let(:access_token) { tokens[:write] } let(:headers) do - { 'Authorization' => "Bearer #{create(:personal_access_token, scopes: scopes, user: current_user).token}" } + { 'Authorization' => "Bearer #{access_token.token}" } end - let(:params) { {} } + let(:project_id) { project.id } + let(:default_params) { {} } + let(:params) { default_params } let(:request) { get api(route), params: params, headers: headers } before do @@ -57,7 +65,7 @@ RSpec.describe API::Ml::Mlflow do shared_examples 'Requires api scope' do context 'when user has access but token has wrong scope' do - let(:scopes) { %w[read_api] } + let(:access_token) { tokens[:read] } it { expect(response).to have_gitlab_http_status(:forbidden) } end @@ -65,7 +73,7 @@ RSpec.describe API::Ml::Mlflow do shared_examples 'Requires read_api scope' do context 'when user has access but token has wrong scope' do - let(:scopes) { %w[read_user] } + let(:access_token) { tokens[:no_access] } it { expect(response).to have_gitlab_http_status(:forbidden) } end @@ -89,7 +97,7 @@ RSpec.describe API::Ml::Mlflow do end context 'when user does not have access' do - let(:current_user) { create(:user) } + let(:access_token) { tokens[:different_user] } it_behaves_like 'Not Found' end @@ -101,11 +109,41 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'GET /projects/:id/ml/mflow/api/2.0/mlflow/get' do + shared_examples 'run_id param error cases' do + context 'when run id is not passed' do + let(:params) { {} } + + it_behaves_like 'Bad Request' + end + + context 'when run_id is invalid' do + let(:params) { default_params.merge(run_id: non_existing_record_iid.to_s) } + + it_behaves_like 'Not Found - Resource Does Not Exist' + end + + context 'when run_id is not in in the project' do + let(:project_id) { another_project.id } + + it_behaves_like 'Not Found - Resource Does Not Exist' + end + end + + shared_examples 'Bad Request on missing required' do |keys| + keys.each do |key| + context "when \"#{key}\" is missing" do + let(:params) { default_params.tap { |p| p.delete(key) } } + + it_behaves_like 'Bad Request' + end + end + end + + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/get' do let(:experiment_iid) { experiment.iid.to_s } - let(:route) { "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/get?experiment_id=#{experiment_iid}" } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get?experiment_id=#{experiment_iid}" } - it 'returns the experiment' do + it 'returns the experiment', :aggregate_failures do expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('ml/get_experiment') expect(json_response).to include({ @@ -127,7 +165,7 @@ RSpec.describe API::Ml::Mlflow do end context 'and experiment_id is not passed' do - let(:route) { "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/get" } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get" } it_behaves_like 'Not Found - Resource Does Not Exist' end @@ -138,13 +176,43 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'GET /projects/:id/ml/mflow/api/2.0/mlflow/experiments/get-by-name' do + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/list' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/list" } + + it 'returns the experiments' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('ml/list_experiments') + expect(json_response).to include({ + 'experiments' => [ + 'experiment_id' => experiment.iid.to_s, + 'name' => experiment.name, + 'lifecycle_stage' => 'active', + 'artifact_location' => 'not_implemented' + ] + }) + end + + context 'when there are no experiments' do + let(:project_id) { another_project.id } + + it 'returns an empty list' do + expect(json_response).to include({ 'experiments' => [] }) + end + end + + describe 'Error States' do + it_behaves_like 'shared error cases' + it_behaves_like 'Requires read_api scope' + end + end + + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/get-by-name' do let(:experiment_name) { experiment.name } let(:route) do - "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/get-by-name?experiment_name=#{experiment_name}" + "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get-by-name?experiment_name=#{experiment_name}" end - it 'returns the experiment' do + it 'returns the experiment', :aggregate_failures do expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('ml/get_experiment') expect(json_response).to include({ @@ -165,7 +233,7 @@ RSpec.describe API::Ml::Mlflow do end context 'when has access but experiment_name is not passed' do - let(:route) { "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/get-by-name" } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/get-by-name" } it_behaves_like 'Not Found - Resource Does Not Exist' end @@ -175,16 +243,16 @@ RSpec.describe API::Ml::Mlflow do end end - describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/experiments/create' do + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/experiments/create' do let(:route) do - "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/experiments/create" + "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/experiments/create" end let(:params) { { name: 'new_experiment' } } let(:request) { post api(route), params: params, headers: headers } - it 'creates the experiment' do - expect(response).to have_gitlab_http_status(:created) + it 'creates the experiment', :aggregate_failures do + expect(response).to have_gitlab_http_status(:ok) expect(json_response).to include('experiment_id' ) end @@ -206,7 +274,7 @@ RSpec.describe API::Ml::Mlflow do end context 'when project does not exist' do - let(:route) { "/projects/#{non_existing_record_id}/ml/mflow/api/2.0/mlflow/experiments/create" } + let(:route) { "/projects/#{non_existing_record_id}/ml/mlflow/api/2.0/mlflow/experiments/create" } it_behaves_like 'Not Found', '404 Project Not Found' end @@ -217,15 +285,12 @@ RSpec.describe API::Ml::Mlflow do end describe 'Runs' do - describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/runs/create' do - let(:route) do - "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/runs/create" - end - + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/create' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/create" } let(:params) { { experiment_id: experiment.iid.to_s, start_time: Time.now.to_i } } let(:request) { post api(route), params: params, headers: headers } - it 'creates the run' do + it 'creates the run', :aggregate_failures do expected_properties = { 'experiment_id' => params[:experiment_id], 'user_id' => current_user.id.to_s, @@ -235,9 +300,10 @@ RSpec.describe API::Ml::Mlflow do 'lifecycle_stage' => "active" } - expect(response).to have_gitlab_http_status(:created) + expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('ml/run') - expect(json_response['run']).to include('info' => hash_including(**expected_properties), 'data' => {}) + expect(json_response['run']).to include('info' => hash_including(**expected_properties), + 'data' => { 'metrics' => [], 'params' => [] }) end describe 'Error States' do @@ -253,21 +319,22 @@ RSpec.describe API::Ml::Mlflow do it_behaves_like 'Not Found - Resource Does Not Exist' end + context 'when experiment exists but is not part of the project' do + let(:project_id) { another_project.id } + + it_behaves_like 'Not Found - Resource Does Not Exist' + end + it_behaves_like 'shared error cases' it_behaves_like 'Requires api scope' end end - describe 'GET /projects/:id/ml/mflow/api/2.0/mlflow/runs/get' do - let_it_be(:route) do - "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/runs/get" - end - - let_it_be(:candidate) { create(:ml_candidates, user: experiment.user, start_time: 1234, experiment: experiment) } - - let(:params) { { 'run_id' => candidate.iid } } + describe 'GET /projects/:id/ml/mlflow/api/2.0/mlflow/runs/get' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/get" } + let(:default_params) { { 'run_id' => candidate.iid } } - it 'gets the run' do + it 'gets the run', :aggregate_failures do expected_properties = { 'experiment_id' => candidate.experiment.iid.to_s, 'user_id' => candidate.user.id.to_s, @@ -277,90 +344,173 @@ RSpec.describe API::Ml::Mlflow do 'lifecycle_stage' => "active" } - expect(response).to have_gitlab_http_status(:success) + expect(response).to have_gitlab_http_status(:ok) expect(response).to match_response_schema('ml/run') - expect(json_response['run']).to include('info' => hash_including(**expected_properties), 'data' => {}) + expect(json_response['run']).to include( + 'info' => hash_including(**expected_properties), + 'data' => { + 'metrics' => [ + hash_including('key' => candidate.metrics[0].name), + hash_including('key' => candidate.metrics[1].name) + ], + 'params' => [ + { 'key' => candidate.params[0].name, 'value' => candidate.params[0].value }, + { 'key' => candidate.params[1].name, 'value' => candidate.params[1].value } + ] + }) end describe 'Error States' do - context 'when run id is not passed' do - let(:params) { {} } + it_behaves_like 'run_id param error cases' + it_behaves_like 'shared error cases' + it_behaves_like 'Requires read_api scope' + end + end - it_behaves_like 'Not Found - Resource Does Not Exist' - end + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/update' do + let(:default_params) { { run_id: candidate.iid.to_s, status: 'FAILED', end_time: Time.now.to_i } } + let(:request) { post api(route), params: params, headers: headers } + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/update" } - context 'when run id does not exist' do - let(:params) { { run_id: non_existing_record_iid.to_s } } + it 'updates the run', :aggregate_failures do + expected_properties = { + 'experiment_id' => candidate.experiment.iid.to_s, + 'user_id' => candidate.user.id.to_s, + 'start_time' => candidate.start_time, + 'end_time' => params[:end_time], + 'artifact_uri' => 'not_implemented', + 'status' => 'FAILED', + 'lifecycle_stage' => 'active' + } - it_behaves_like 'Not Found - Resource Does Not Exist' + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('ml/update_run') + expect(json_response).to include('run_info' => hash_including(**expected_properties)) + end + + describe 'Error States' do + context 'when status in invalid' do + let(:params) { default_params.merge(status: 'YOLO') } + + it_behaves_like 'Bad Request' end - context 'when run id exists but does not belong to project' do - let(:params) { { run_id: another_candidate.iid.to_s } } + context 'when end_time is invalid' do + let(:params) { default_params.merge(end_time: 's') } - it_behaves_like 'Not Found - Resource Does Not Exist' + it_behaves_like 'Bad Request' end it_behaves_like 'shared error cases' - it_behaves_like 'Requires read_api scope' + it_behaves_like 'Requires api scope' + it_behaves_like 'run_id param error cases' end end - end - describe 'POST /projects/:id/ml/mflow/api/2.0/mlflow/runs/update' do - let(:route) { "/projects/#{project.id}/ml/mflow/api/2.0/mlflow/runs/update" } - let(:params) { { run_id: candidate.iid.to_s, status: 'FAILED', end_time: Time.now.to_i } } - let(:request) { post api(route), params: params, headers: headers } + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-metric' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-metric" } + let(:default_params) { { run_id: candidate.iid.to_s, key: 'some_key', value: 10.0, timestamp: Time.now.to_i } } + let(:request) { post api(route), params: params, headers: headers } - it 'updates the run' do - expected_properties = { - 'experiment_id' => candidate.experiment.iid.to_s, - 'user_id' => candidate.user.id.to_s, - 'start_time' => candidate.start_time, - 'end_time' => params[:end_time], - 'artifact_uri' => 'not_implemented', - 'status' => 'FAILED', - 'lifecycle_stage' => 'active' - } - - expect(response).to have_gitlab_http_status(:success) - expect(response).to match_response_schema('ml/update_run') - expect(json_response).to include('run_info' => hash_including(**expected_properties)) + it 'logs the metric', :aggregate_failures do + candidate.metrics.reload + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + expect(candidate.metrics.length).to eq(3) + end + + describe 'Error Cases' do + it_behaves_like 'shared error cases' + it_behaves_like 'Requires api scope' + it_behaves_like 'run_id param error cases' + it_behaves_like 'Bad Request on missing required', [:key, :value, :timestamp] + end end - describe 'Error States' do - context 'when run id is not passed' do - let(:params) { {} } + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-parameter' do + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-parameter" } + let(:default_params) { { run_id: candidate.iid.to_s, key: 'some_key', value: 'value' } } + let(:request) { post api(route), params: params, headers: headers } - it_behaves_like 'Not Found - Resource Does Not Exist' + it 'logs the parameter', :aggregate_failures do + candidate.params.reload + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + expect(candidate.params.length).to eq(3) end - context 'when run id does not exist' do - let(:params) { { run_id: non_existing_record_iid.to_s } } + describe 'Error Cases' do + context 'when parameter was already logged' do + let(:params) { default_params.tap { |p| p[:key] = candidate.params[0].name } } - it_behaves_like 'Not Found - Resource Does Not Exist' + it_behaves_like 'Bad Request' + end + + it_behaves_like 'shared error cases' + it_behaves_like 'Requires api scope' + it_behaves_like 'run_id param error cases' + it_behaves_like 'Bad Request on missing required', [:key, :value] end + end - context 'when run id exists but does not belong to project' do - let(:params) { { run_id: another_candidate.iid.to_s } } + describe 'POST /projects/:id/ml/mlflow/api/2.0/mlflow/runs/log-batch' do + let(:candidate2) do + create(:ml_candidates, user: experiment.user, start_time: 1234, experiment: experiment) + end - it_behaves_like 'Not Found - Resource Does Not Exist' + let(:route) { "/projects/#{project_id}/ml/mlflow/api/2.0/mlflow/runs/log-batch" } + let(:default_params) do + { + run_id: candidate2.iid.to_s, + metrics: [ + { key: 'mae', value: 2.5, timestamp: 1552550804 }, + { key: 'rmse', value: 2.7, timestamp: 1552550804 } + ], + params: [{ key: 'model_class', value: 'LogisticRegression' }] + } end - context 'when run id exists but status in invalid' do - let(:params) { { run_id: candidate.iid.to_s, status: 'YOLO', end_time: Time.now.to_i } } + let(:request) { post api(route), params: params, headers: headers } - it_behaves_like 'Bad Request' + it 'logs parameters and metrics', :aggregate_failures do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_empty + expect(candidate2.params.size).to eq(1) + expect(candidate2.metrics.size).to eq(2) end - context 'when run id exists but end_time is invalid' do - let(:params) { { run_id: candidate.iid.to_s, status: 'FAILED', end_time: 's' } } + context 'when parameter was already logged' do + let(:params) do + default_params.tap { |p| p[:params] = [{ key: 'hello', value: 'a' }, { key: 'hello', value: 'b' }] } + end - it_behaves_like 'Bad Request' + it 'does not log', :aggregate_failures do + candidate.params.reload + + expect(response).to have_gitlab_http_status(:ok) + expect(candidate2.params.size).to eq(1) + end end - it_behaves_like 'shared error cases' - it_behaves_like 'Requires api scope' + describe 'Error Cases' do + context 'when required metric key is missing' do + let(:params) { default_params.tap { |p| p[:metrics] = [p[:metrics][0].delete(:key)] } } + + it_behaves_like 'Bad Request' + end + + context 'when required param key is missing' do + let(:params) { default_params.tap { |p| p[:params] = [p[:params][0].delete(:key)] } } + + it_behaves_like 'Bad Request' + end + + it_behaves_like 'shared error cases' + it_behaves_like 'Requires api scope' + it_behaves_like 'run_id param error cases' + end end end end diff --git a/spec/requests/api/pages_domains_spec.rb b/spec/requests/api/pages_domains_spec.rb index cd4e8b30d8f..8ef4e899193 100644 --- a/spec/requests/api/pages_domains_spec.rb +++ b/spec/requests/api/pages_domains_spec.rb @@ -259,7 +259,15 @@ RSpec.describe API::PagesDomains do shared_examples_for 'post pages domains' do it 'creates a new pages domain' do - post api(route, user), params: params + expect { post api(route, user), params: params } + .to publish_event(PagesDomains::PagesDomainCreatedEvent) + .with( + project_id: project.id, + namespace_id: project.namespace.id, + root_namespace_id: project.root_namespace.id, + domain: params[:domain] + ) + pages_domain = PagesDomain.find_by(domain: json_response['domain']) expect(response).to have_gitlab_http_status(:created) @@ -378,6 +386,17 @@ RSpec.describe API::PagesDomains do expect(pages_domain_secure.auto_ssl_enabled).to be false end + it 'publishes PagesDomainUpdatedEvent event' do + expect { put api(route_secure_domain, user), params: { certificate: nil, key: nil } } + .to publish_event(PagesDomains::PagesDomainUpdatedEvent) + .with( + project_id: project.id, + namespace_id: project.namespace.id, + root_namespace_id: project.root_namespace.id, + domain: pages_domain_secure.domain + ) + end + it 'updates pages domain adding certificate' do put api(route_domain, user), params: params_secure pages_domain.reload @@ -446,22 +465,29 @@ RSpec.describe API::PagesDomains do end.to change { pages_domain.reload.certificate_source }.from('gitlab_provided').to('user_provided') end - it 'fails to update pages domain adding certificate without key' do - put api(route_domain, user), params: params_secure_nokey + context 'with invalid params' do + it 'fails to update pages domain adding certificate without key' do + put api(route_domain, user), params: params_secure_nokey - expect(response).to have_gitlab_http_status(:bad_request) - end + expect(response).to have_gitlab_http_status(:bad_request) + end - it 'fails to update pages domain adding certificate with missing chain' do - put api(route_domain, user), params: pages_domain_secure_missing_chain_params.slice(:certificate) + it 'does not publish PagesDomainUpdatedEvent event' do + expect { put api(route_domain, user), params: params_secure_nokey } + .not_to publish_event(PagesDomains::PagesDomainUpdatedEvent) + end - expect(response).to have_gitlab_http_status(:bad_request) - end + it 'fails to update pages domain adding certificate with missing chain' do + put api(route_domain, user), params: pages_domain_secure_missing_chain_params.slice(:certificate) + + expect(response).to have_gitlab_http_status(:bad_request) + end - it 'fails to update pages domain with key missmatch' do - put api(route_secure_domain, user), params: pages_domain_secure_key_missmatch_params.slice(:certificate, :key) + it 'fails to update pages domain with key missmatch' do + put api(route_secure_domain, user), params: pages_domain_secure_key_missmatch_params.slice(:certificate, :key) - expect(response).to have_gitlab_http_status(:bad_request) + expect(response).to have_gitlab_http_status(:bad_request) + end end end @@ -523,7 +549,15 @@ RSpec.describe API::PagesDomains do describe 'DELETE /projects/:project_id/pages/domains/:domain' do shared_examples_for 'delete pages domain' do it 'deletes a pages domain' do - delete api(route_domain, user) + expect { delete api(route_domain, user) } + .to change(PagesDomain, :count).by(-1) + .and publish_event(PagesDomains::PagesDomainDeletedEvent) + .with( + project_id: project.id, + namespace_id: project.namespace.id, + root_namespace_id: project.root_namespace.id, + domain: pages_domain.domain + ) expect(response).to have_gitlab_http_status(:no_content) end diff --git a/spec/requests/api/personal_access_tokens/self_information_spec.rb b/spec/requests/api/personal_access_tokens/self_information_spec.rb new file mode 100644 index 00000000000..bdfac3ed14f --- /dev/null +++ b/spec/requests/api/personal_access_tokens/self_information_spec.rb @@ -0,0 +1,102 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::PersonalAccessTokens::SelfInformation do + let(:path) { '/personal_access_tokens/self' } + let(:token) { create(:personal_access_token, user: current_user) } + + let_it_be(:current_user) { create(:user) } + + describe 'DELETE /personal_access_tokens/self' do + subject(:delete_token) { delete api(path, personal_access_token: token) } + + shared_examples 'revoking token succeeds' do + it 'revokes token' do + delete_token + + expect(response).to have_gitlab_http_status(:no_content) + expect(token.reload).to be_revoked + end + end + + shared_examples 'revoking token denied' do |status| + it 'cannot revoke token' do + delete_token + + expect(response).to have_gitlab_http_status(status) + end + end + + context 'when current_user is an administrator', :enable_admin_mode do + let(:current_user) { create(:admin) } + + it_behaves_like 'revoking token succeeds' + + context 'with impersonated token' do + let(:token) { create(:personal_access_token, :impersonation, user: current_user) } + + it_behaves_like 'revoking token succeeds' + end + end + + context 'when current_user is not an administrator' do + let(:current_user) { create(:user) } + + it_behaves_like 'revoking token succeeds' + + context 'with impersonated token' do + let(:token) { create(:personal_access_token, :impersonation, user: current_user) } + + it_behaves_like 'revoking token denied', :bad_request + end + + context 'with already revoked token' do + let(:token) { create(:personal_access_token, :revoked, user: current_user) } + + it_behaves_like 'revoking token denied', :unauthorized + end + end + + Gitlab::Auth.all_available_scopes.each do |scope| + context "with a '#{scope}' scoped token" do + let(:token) { create(:personal_access_token, scopes: [scope], user: current_user) } + + it_behaves_like 'revoking token succeeds' + end + end + end + + describe 'GET /personal_access_tokens/self' do + Gitlab::Auth.all_available_scopes.each do |scope| + context "with a '#{scope}' scoped token" do + let(:token) { create(:personal_access_token, scopes: [scope], user: current_user) } + + it 'shows token info' do + get api(path, personal_access_token: token) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['scopes']).to match_array([scope.to_s]) + end + end + end + + context 'when token is invalid' do + it 'returns 401' do + get api(path, personal_access_token: instance_double(PersonalAccessToken, token: 'invalidtoken')) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + + context 'when token is expired' do + it 'returns 401' do + token = create(:personal_access_token, expires_at: 1.day.ago, user: current_user) + + get api(path, personal_access_token: token) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end + end +end diff --git a/spec/requests/api/personal_access_tokens/self_revocation_spec.rb b/spec/requests/api/personal_access_tokens/self_revocation_spec.rb deleted file mode 100644 index f829b39cc1e..00000000000 --- a/spec/requests/api/personal_access_tokens/self_revocation_spec.rb +++ /dev/null @@ -1,69 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe API::PersonalAccessTokens::SelfRevocation do - let_it_be(:current_user) { create(:user) } - - describe 'DELETE /personal_access_tokens/self' do - let(:path) { '/personal_access_tokens/self' } - let(:token) { create(:personal_access_token, user: current_user) } - - subject(:delete_token) { delete api(path, personal_access_token: token) } - - shared_examples 'revoking token succeeds' do - it 'revokes token' do - delete_token - - expect(response).to have_gitlab_http_status(:no_content) - expect(token.reload).to be_revoked - end - end - - shared_examples 'revoking token denied' do |status| - it 'cannot revoke token' do - delete_token - - expect(response).to have_gitlab_http_status(status) - end - end - - context 'when current_user is an administrator', :enable_admin_mode do - let(:current_user) { create(:admin) } - - it_behaves_like 'revoking token succeeds' - - context 'with impersonated token' do - let(:token) { create(:personal_access_token, :impersonation, user: current_user) } - - it_behaves_like 'revoking token succeeds' - end - end - - context 'when current_user is not an administrator' do - let(:current_user) { create(:user) } - - it_behaves_like 'revoking token succeeds' - - context 'with impersonated token' do - let(:token) { create(:personal_access_token, :impersonation, user: current_user) } - - it_behaves_like 'revoking token denied', :bad_request - end - - context 'with already revoked token' do - let(:token) { create(:personal_access_token, :revoked, user: current_user) } - - it_behaves_like 'revoking token denied', :unauthorized - end - end - - Gitlab::Auth.all_available_scopes.each do |scope| - context "with a '#{scope}' scoped token" do - let(:token) { create(:personal_access_token, scopes: [scope], user: current_user) } - - it_behaves_like 'revoking token succeeds' - end - end - end -end diff --git a/spec/requests/api/personal_access_tokens_spec.rb b/spec/requests/api/personal_access_tokens_spec.rb index 37b5a594f2a..31c4e8803e3 100644 --- a/spec/requests/api/personal_access_tokens_spec.rb +++ b/spec/requests/api/personal_access_tokens_spec.rb @@ -4,14 +4,34 @@ require 'spec_helper' RSpec.describe API::PersonalAccessTokens do let_it_be(:path) { '/personal_access_tokens' } - let_it_be(:token1) { create(:personal_access_token) } - let_it_be(:token2) { create(:personal_access_token) } - let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: token1.user) } - let_it_be(:current_user) { create(:user) } describe 'GET /personal_access_tokens' do + using RSpec::Parameterized::TableSyntax + + def map_id(json_resonse) + json_response.map { |pat| pat['id'] } + end + + shared_examples 'response as expected' do |params| + subject { get api(path, personal_access_token: current_users_token), params: params } + + it "status, count and result as expected" do + subject + + if status == :bad_request + expect(json_response).to eq(result) + elsif status == :ok + expect(map_id(json_response)).to a_collection_containing_exactly(*result) + end + + expect(response).to have_gitlab_http_status(status) + expect(json_response.count).to eq(result_count) + end + end + context 'logged in as an Administrator' do let_it_be(:current_user) { create(:admin) } + let_it_be(:current_users_token) { create(:personal_access_token, user: current_user) } it 'returns all PATs by default' do get api(path, current_user) @@ -21,60 +41,348 @@ RSpec.describe API::PersonalAccessTokens do end context 'filtered with user_id parameter' do + let_it_be(:token) { create(:personal_access_token) } + let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: token.user) } + it 'returns only PATs belonging to that user' do - get api(path, current_user), params: { user_id: token1.user.id } + get api(path, current_user), params: { user_id: token.user.id } expect(response).to have_gitlab_http_status(:ok) expect(json_response.count).to eq(2) - expect(json_response.first['user_id']).to eq(token1.user.id) + expect(json_response.first['user_id']).to eq(token.user.id) expect(json_response.last['id']).to eq(token_impersonated.id) end end - context 'logged in as a non-Administrator' do - let_it_be(:current_user) { create(:user) } + context 'filter with revoked parameter' do + let_it_be(:revoked_token) { create(:personal_access_token, revoked: true) } + let_it_be(:not_revoked_token1) { create(:personal_access_token, revoked: false) } + let_it_be(:not_revoked_token2) { create(:personal_access_token, revoked: false) } + + where(:revoked, :status, :result_count, :result) do + true | :ok | 1 | lazy { [revoked_token.id] } + false | :ok | 3 | lazy { [not_revoked_token1.id, not_revoked_token2.id, current_users_token.id] } + 'asdf' | :bad_request | 1 | { "error" => "revoked is invalid" } + end + + with_them do + it_behaves_like 'response as expected', revoked: params[:revoked] + end + end + + context 'filter with active parameter' do + let_it_be(:inactive_token1) { create(:personal_access_token, revoked: true) } + let_it_be(:inactive_token2) { create(:personal_access_token, expires_at: Time.new(2022, 01, 01, 00, 00, 00)) } + let_it_be(:active_token) { create(:personal_access_token) } + + where(:state, :status, :result_count, :result) do + 'inactive' | :ok | 2 | lazy { [inactive_token1.id, inactive_token2.id] } + 'active' | :ok | 2 | lazy { [active_token.id, current_users_token.id] } + 'asdf' | :bad_request | 1 | { "error" => "state does not have a valid value" } + end + + with_them do + it_behaves_like 'response as expected', state: params[:state] + end + end + + context 'filter with created parameter' do + let_it_be(:token1) { create(:personal_access_token, created_at: DateTime.new(2022, 01, 01, 12, 30, 25) ) } + + context 'test created_before' do + where(:created_at, :status, :result_count, :result) do + '2022-01-02' | :ok | 1 | lazy { [token1.id] } + '2022-01-01' | :ok | 0 | lazy { [] } + '2022-01-01T12:30:24' | :ok | 0 | lazy { [] } + '2022-01-01T12:30:25' | :ok | 1 | lazy { [token1.id] } + '2022-01-01T:12:30:26' | :ok | 1 | lazy { [token1.id] } + 'asdf' | :bad_request | 1 | { "error" => "created_before is invalid" } + end + + with_them do + it_behaves_like 'response as expected', created_before: params[:created_at] + end + end + + context 'test created_after' do + where(:created_at, :status, :result_count, :result) do + '2022-01-03' | :ok | 1 | lazy { [current_users_token.id] } + '2022-01-01' | :ok | 2 | lazy { [token1.id, current_users_token.id] } + '2022-01-01T12:30:25' | :ok | 2 | lazy { [token1.id, current_users_token.id] } + '2022-01-01T12:30:26' | :ok | 1 | lazy { [current_users_token.id] } + (DateTime.now + 1).to_s | :ok | 0 | lazy { [] } + 'asdf' | :bad_request | 1 | { "error" => "created_after is invalid" } + end + + with_them do + it_behaves_like 'response as expected', created_after: params[:created_at] + end + end + end + + context 'filter with last_used parameter' do + let_it_be(:token1) { create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 01, 12, 30, 25) ) } + let_it_be(:never_used_token) { create(:personal_access_token) } + + context 'test last_used_before' do + where(:last_used_at, :status, :result_count, :result) do + '2022-01-02' | :ok | 1 | lazy { [token1.id] } + '2022-01-01' | :ok | 0 | lazy { [] } + '2022-01-01T12:30:24' | :ok | 0 | lazy { [] } + '2022-01-01T12:30:25' | :ok | 1 | lazy { [token1.id] } + '2022-01-01T12:30:26' | :ok | 1 | lazy { [token1.id] } + 'asdf' | :bad_request | 1 | { "error" => "last_used_before is invalid" } + end + + with_them do + it_behaves_like 'response as expected', last_used_before: params[:last_used_at] + end + end + + context 'test last_used_after' do + where(:last_used_at, :status, :result_count, :result) do + '2022-01-03' | :ok | 1 | lazy { [current_users_token.id] } + '2022-01-01' | :ok | 2 | lazy { [token1.id, current_users_token.id] } + '2022-01-01T12:30:26' | :ok | 1 | lazy { [current_users_token.id] } + '2022-01-01T12:30:25' | :ok | 2 | lazy { [token1.id, current_users_token.id] } + (DateTime.now + 1).to_s | :ok | 0 | lazy { [] } + 'asdf' | :bad_request | 1 | { "error" => "last_used_after is invalid" } + end + + with_them do + it_behaves_like 'response as expected', last_used_after: params[:last_used_at] + end + end + end + + context 'filter with search parameter' do + let_it_be(:token1) { create(:personal_access_token, name: 'test_1') } + let_it_be(:token2) { create(:personal_access_token, name: 'test_2') } + let_it_be(:token3) { create(:personal_access_token, name: '') } + + where(:pattern, :status, :result_count, :result) do + 'test' | :ok | 2 | lazy { [token1.id, token2.id] } + '' | :ok | 4 | lazy { [token1.id, token2.id, token3.id, current_users_token.id] } + 'test_1' | :ok | 1 | lazy { [token1.id] } + 'asdf' | :ok | 0 | lazy { [] } + end + + with_them do + it_behaves_like 'response as expected', search: params[:pattern] + end + end + + context 'filter created_before/created_after combined with last_used_before/last_used_after' do + let_it_be(:date) { DateTime.new(2022, 01, 02) } + let_it_be(:token1) { create(:personal_access_token, created_at: date, last_used_at: date) } + + where(:date_before, :date_after, :status, :result_count, :result) do + '2022-01-03' | '2022-01-01' | :ok | 1 | lazy { [token1.id] } + '2022-01-01' | '2022-01-03' | :ok | 0 | lazy { [] } + '2022-01-03' | nil | :ok | 1 | lazy { [token1.id] } + nil | '2022-01-01' | :ok | 2 | lazy { [token1.id, current_users_token.id] } + end + + with_them do + it_behaves_like 'response as expected', { created_before: params[:date_before], + created_after: params[:date_after] } + it_behaves_like 'response as expected', { last_used_before: params[:date_before], + last_used_after: params[:date_after] } + end + end + + context 'filter created_before and created_after combined is valid' do + let_it_be(:token1) { create(:personal_access_token, created_at: DateTime.new(2022, 01, 02)) } + + where(:created_before, :created_after, :status, :result) do + '2022-01-02' | '2022-01-02' | :ok | lazy { [token1.id] } + '2022-01-03' | '2022-01-01' | :ok | lazy { [token1.id] } + '2022-01-01' | '2022-01-03' | :ok | lazy { [] } + '2022-01-03' | nil | :ok | lazy { [token1.id] } + nil | '2022-01-01' | :ok | lazy { [token1.id] } + end + + with_them do + it "returns all valid tokens" do + get api(path, personal_access_token: current_users_token), + params: { created_before: created_before, created_after: created_after } + + expect(response).to have_gitlab_http_status(status) + + expect(json_response.map { |pat| pat['id'] } ).to include(*result) if status == :ok + end + end + end + + context 'filter last_used_before and last_used_after combined is valid' do + let_it_be(:token1) { create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 02) ) } + + where(:last_used_before, :last_used_after, :status, :result) do + '2022-01-02' | '2022-01-02' | :ok | lazy { [token1.id] } + '2022-01-03' | '2022-01-01' | :ok | lazy { [token1.id] } + '2022-01-01' | '2022-01-03' | :ok | lazy { [] } + '2022-01-03' | nil | :ok | lazy { [token1.id] } + nil | '2022-01-01' | :ok | lazy { [token1.id] } + end + + with_them do + it "returns all valid tokens" do + get api(path, personal_access_token: current_users_token), + params: { last_used_before: last_used_before, last_used_after: last_used_after } + + expect(response).to have_gitlab_http_status(status) + + expect(json_response.map { |pat| pat['id'] } ).to include(*result) if status == :ok + end + end + end + end + + context 'logged in as a non-Administrator' do + let_it_be(:current_user) { create(:user) } + let_it_be(:current_users_token) { create(:personal_access_token, user: current_user) } + + it 'returns all PATs belonging to the signed-in user' do + get api(path, personal_access_token: current_users_token) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response.count).to eq(1) + expect(json_response.map { |r| r['id'] }.uniq).to contain_exactly(current_users_token.id) + expect(json_response.map { |r| r['user_id'] }.uniq).to contain_exactly(current_user.id) + end + + context 'filtered with user_id parameter' do let_it_be(:user) { create(:user) } - let_it_be(:token) { create(:personal_access_token, user: current_user) } - let_it_be(:other_token) { create(:personal_access_token, user: user) } - let_it_be(:token_impersonated) { create(:personal_access_token, impersonation: true, user: current_user) } - it 'returns all PATs belonging to the signed-in user' do - get api(path, current_user, personal_access_token: token) + it 'returns PATs belonging to the specific user' do + get api(path, current_user, personal_access_token: current_users_token), params: { user_id: current_user.id } expect(response).to have_gitlab_http_status(:ok) expect(json_response.count).to eq(1) + expect(json_response.map { |r| r['id'] }.uniq).to contain_exactly(current_users_token.id) expect(json_response.map { |r| r['user_id'] }.uniq).to contain_exactly(current_user.id) end - context 'filtered with user_id parameter' do - it 'returns PATs belonging to the specific user' do - get api(path, current_user, personal_access_token: token), params: { user_id: current_user.id } + it 'is unauthorized if filtered by a user other than current_user' do + get api(path, current_user, personal_access_token: current_users_token), params: { user_id: user.id } - expect(response).to have_gitlab_http_status(:ok) - expect(json_response.count).to eq(1) - expect(json_response.map { |r| r['user_id'] }.uniq).to contain_exactly(current_user.id) - end + expect(response).to have_gitlab_http_status(:unauthorized) + end + end - it 'is unauthorized if filtered by a user other than current_user' do - get api(path, current_user, personal_access_token: token), params: { user_id: user.id } + context 'filter with revoked parameter' do + let_it_be(:users_revoked_token) { create(:personal_access_token, revoked: true, user: current_user) } + let_it_be(:not_revoked_token) { create(:personal_access_token, revoked: false) } + let_it_be(:oter_revoked_token) { create(:personal_access_token, revoked: true) } - expect(response).to have_gitlab_http_status(:unauthorized) - end + where(:revoked, :status, :result_count, :result) do + true | :ok | 1 | lazy { [users_revoked_token.id] } + false | :ok | 1 | lazy { [current_users_token.id] } + end + + with_them do + it_behaves_like 'response as expected', revoked: params[:revoked] end end - context 'not authenticated' do - it 'is forbidden' do - get api(path) + context 'filter with active parameter' do + let_it_be(:users_inactive_token) { create(:personal_access_token, revoked: true, user: current_user) } + let_it_be(:inactive_token) { create(:personal_access_token, expires_at: Time.new(2022, 01, 01, 00, 00, 00)) } + let_it_be(:other_active_token) { create(:personal_access_token) } - expect(response).to have_gitlab_http_status(:unauthorized) + where(:state, :status, :result_count, :result) do + 'inactive' | :ok | 1 | lazy { [users_inactive_token.id] } + 'active' | :ok | 1 | lazy { [current_users_token.id] } + end + + with_them do + it_behaves_like 'response as expected', state: params[:state] + end + end + + # The created_before filter has been extensively tested in the 'logged in as administrator' section. + # Here it is only tested whether PATs to which the user has no access right are excluded from the filter function. + context 'filter with created parameter' do + let_it_be(:token1) do + create(:personal_access_token, created_at: DateTime.new(2022, 01, 02, 12, 30, 25), user: current_user ) + end + + let_it_be(:token2) { create(:personal_access_token, created_at: DateTime.new(2022, 01, 02, 12, 30, 25)) } + let_it_be(:status) { :ok } + + context 'created_before' do + let_it_be(:result_count) { 1 } + let_it_be(:result) { [token1.id] } + + it_behaves_like 'response as expected', created_before: '2022-01-03' + end + + context 'created_after' do + let_it_be(:result_count) { 2 } + let_it_be(:result) { [token1.id, current_users_token.id] } + + it_behaves_like 'response as expected', created_after: '2022-01-01' + end + end + + # The last_used_before filter has been extensively tested in the 'logged in as administrator' section. + # Here it is only tested whether PATs to which the user has no access right are excluded from the filter function. + context 'filter with last_used' do + let_it_be(:token1) do + create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 01, 12, 30, 25), user: current_user) + end + + let_it_be(:token2) { create(:personal_access_token, last_used_at: DateTime.new(2022, 01, 01, 12, 30, 25) ) } + let_it_be(:never_used_token) { create(:personal_access_token) } + let_it_be(:status) { :ok } + + context 'last_used_before' do + let_it_be(:result_count) { 1 } + let_it_be(:result) { [token1.id] } + + it_behaves_like 'response as expected', last_used_before: '2022-01-02' + end + + context 'last_used_after' do + let_it_be(:result_count) { 2 } + let_it_be(:result) { [token1.id, current_users_token.id] } + + it_behaves_like 'response as expected', last_used_after: '2022-01-01' + end + end + + # The search filter has been extensively tested in the 'logged in as administrator' section. + # Here it is only tested whether PATs to which the user has no access right are excluded from the filter function. + context 'filter with search parameter' do + let_it_be(:token1) { create(:personal_access_token, name: 'test_1', user: current_user) } + let_it_be(:token2) { create(:personal_access_token, name: 'test_1') } + let_it_be(:token3) { create(:personal_access_token, name: '') } + + where(:pattern, :status, :result_count, :result) do + 'test' | :ok | 1 | lazy { [token1.id] } + '' | :ok | 2 | lazy { [token1.id, current_users_token.id] } + 'test_1' | :ok | 1 | lazy { [token1.id] } + end + + with_them do + it_behaves_like 'response as expected', search: params[:pattern] end end end + + context 'not authenticated' do + it 'is forbidden' do + get api(path) + + expect(response).to have_gitlab_http_status(:unauthorized) + end + end end describe 'GET /personal_access_tokens/:id' do + let_it_be(:current_user) { create(:user) } let_it_be(:user_token) { create(:personal_access_token, user: current_user) } + let_it_be(:token1) { create(:personal_access_token) } let_it_be(:user_read_only_token) { create(:personal_access_token, scopes: ['read_repository'], user: current_user) } let_it_be(:user_token_path) { "/personal_access_tokens/#{user_token.id}" } let_it_be(:invalid_path) { "/personal_access_tokens/#{non_existing_record_id}" } @@ -136,6 +444,9 @@ RSpec.describe API::PersonalAccessTokens do end describe 'DELETE /personal_access_tokens/:id' do + let_it_be(:current_user) { create(:user) } + let_it_be(:token1) { create(:personal_access_token) } + let(:path) { "/personal_access_tokens/#{token1.id}" } context 'when current_user is an administrator', :enable_admin_mode do diff --git a/spec/requests/api/project_attributes.yml b/spec/requests/api/project_attributes.yml index 1335fa02aaf..0b4a96896d6 100644 --- a/spec/requests/api/project_attributes.yml +++ b/spec/requests/api/project_attributes.yml @@ -90,11 +90,10 @@ ci_cd_settings: - id - project_id - group_runners_enabled - - merge_pipelines_enabled - merge_trains_enabled - merge_pipelines_enabled - - merge_trains_enabled - auto_rollback_enabled + - inbound_job_token_scope_enabled remapped_attributes: default_git_depth: ci_default_git_depth forward_deployment_enabled: ci_forward_deployment_enabled @@ -129,7 +128,6 @@ project_feature: - infrastructure_access_level - feature_flags_access_level - environments_access_level - - releases_access_level - project_id - updated_at computed_attributes: @@ -149,6 +147,7 @@ project_setting: - has_vulnerabilities - legacy_open_source_license_available - prevent_merge_without_jira_issue + - only_allow_merge_if_all_status_checks_passed - warn_about_potentially_unwanted_characters - previous_default_branch - project_id @@ -161,6 +160,8 @@ project_setting: - target_platforms - selective_code_owner_removals - show_diff_preview_in_email + - suggested_reviewers_enabled + - jitsu_key build_service_desk_setting: # service_desk_setting unexposed_attributes: diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 7ad1ce0ede9..38f7d6e3eba 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -221,6 +221,16 @@ RSpec.describe API::Projects do expect(project_response['container_registry_enabled']).to eq(false) end + it 'includes releases_access_level', :aggregate_failures do + project.project_feature.update!(releases_access_level: ProjectFeature::DISABLED) + + get api('/projects', user) + project_response = json_response.find { |p| p['id'] == project.id } + + expect(response).to have_gitlab_http_status(:ok) + expect(project_response['releases_access_level']).to eq('disabled') + end + context 'when some projects are in a group' do before do create(:project, :public, group: create(:group)) @@ -1171,6 +1181,7 @@ RSpec.describe API::Projects do attrs[:analytics_access_level] = 'disabled' attrs[:container_registry_access_level] = 'private' attrs[:security_and_compliance_access_level] = 'private' + attrs[:releases_access_level] = 'disabled' end post api('/projects', user), params: project @@ -1180,7 +1191,7 @@ RSpec.describe API::Projects do project.each_pair do |k, v| next if %i[ has_external_issue_tracker has_external_wiki issues_enabled merge_requests_enabled wiki_enabled storage_version - container_registry_access_level + container_registry_access_level releases_access_level ].include?(k) expect(json_response[k.to_s]).to eq(v) @@ -1195,6 +1206,7 @@ RSpec.describe API::Projects do expect(project.project_feature.analytics_access_level).to eq(ProjectFeature::DISABLED) expect(project.project_feature.container_registry_access_level).to eq(ProjectFeature::PRIVATE) expect(project.project_feature.security_and_compliance_access_level).to eq(ProjectFeature::PRIVATE) + expect(project.project_feature.releases_access_level).to eq(ProjectFeature::DISABLED) end it 'assigns container_registry_enabled to project', :aggregate_failures do @@ -2333,6 +2345,7 @@ RSpec.describe API::Projects do expect(json_response['only_allow_merge_if_all_discussions_are_resolved']).to eq(project.only_allow_merge_if_all_discussions_are_resolved) expect(json_response['operations_access_level']).to be_present expect(json_response['security_and_compliance_access_level']).to be_present + expect(json_response['releases_access_level']).to be_present end it 'exposes all necessary attributes' do @@ -2402,6 +2415,7 @@ RSpec.describe API::Projects do expect(json_response['builds_access_level']).to be_present expect(json_response['operations_access_level']).to be_present expect(json_response['security_and_compliance_access_level']).to be_present + expect(json_response['releases_access_level']).to be_present expect(json_response).to have_key('emails_disabled') expect(json_response['resolve_outdated_diff_discussions']).to eq(project.resolve_outdated_diff_discussions) expect(json_response['remove_source_branch_after_merge']).to be_truthy @@ -2516,7 +2530,7 @@ RSpec.describe API::Projects do 'name' => project.repository.license.name, 'nickname' => project.repository.license.nickname, 'html_url' => project.repository.license.url, - 'source_url' => project.repository.license.meta['source'] + 'source_url' => nil }) end @@ -3386,6 +3400,14 @@ RSpec.describe API::Projects do expect(Project.find_by(path: project[:path]).analytics_access_level).to eq(ProjectFeature::PRIVATE) end + it 'sets releases_access_level', :aggregate_failures do + put api("/projects/#{project.id}", user), params: { releases_access_level: 'private' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['releases_access_level']).to eq('private') + expect(Project.find_by(path: project[:path]).releases_access_level).to eq(ProjectFeature::PRIVATE) + end + it 'returns 400 when nothing sent' do project_param = {} diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb index 315c76c8ac3..3a9b2d02af5 100644 --- a/spec/requests/api/settings_spec.rb +++ b/spec/requests/api/settings_spec.rb @@ -60,6 +60,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['inactive_projects_delete_after_months']).to eq(2) expect(json_response['inactive_projects_min_size_mb']).to eq(0) expect(json_response['inactive_projects_send_warning_email_after_months']).to eq(1) + expect(json_response['can_create_group']).to eq(true) end end @@ -156,7 +157,8 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do delete_inactive_projects: true, inactive_projects_delete_after_months: 24, inactive_projects_min_size_mb: 10, - inactive_projects_send_warning_email_after_months: 12 + inactive_projects_send_warning_email_after_months: 12, + can_create_group: false } expect(response).to have_gitlab_http_status(:ok) @@ -217,6 +219,7 @@ RSpec.describe API::Settings, 'Settings', :do_not_mock_admin_mode_setting do expect(json_response['inactive_projects_delete_after_months']).to eq(24) expect(json_response['inactive_projects_min_size_mb']).to eq(10) expect(json_response['inactive_projects_send_warning_email_after_months']).to eq(12) + expect(json_response['can_create_group']).to eq(false) end end diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index b62fbaead6f..c635d73efe3 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -418,14 +418,6 @@ RSpec.describe API::Tags do context 'annotated tag' do it 'creates a new annotated tag' do - # Identity must be set in .gitconfig to create annotated tag. - repo_path = Gitlab::GitalyClient::StorageSettings.allow_disk_access do - project.repository.path_to_repo - end - - system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.name #{user.name})) - system(*%W(#{Gitlab.config.git.bin_path} --git-dir=#{repo_path} config user.email #{user.email})) - post api(route, current_user), params: { tag_name: 'v7.1.0', ref: 'master', message: 'Release 7.1.0' } expect(response).to have_gitlab_http_status(:created) diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index 96e23337411..1b0a27e78e3 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -165,6 +165,7 @@ RSpec.describe API::Users do expect(json_response.first).not_to have_key('note') expect(json_response.first).not_to have_key('namespace_id') + expect(json_response.first).not_to have_key('created_by') end end @@ -175,6 +176,7 @@ RSpec.describe API::Users do expect(json_response.first).not_to have_key('note') expect(json_response.first).not_to have_key('namespace_id') + expect(json_response.first).not_to have_key('created_by') end end @@ -186,6 +188,26 @@ RSpec.describe API::Users do expect(json_response.first).to have_key('note') expect(json_response.first['note']).to eq '2018-11-05 | 2FA removed | user requested | www.gitlab.com' end + + context 'with `created_by` details' do + it 'has created_by as nil with a self-registered account' do + get api("/users", admin), params: { username: user.username } + + expect(response).to have_gitlab_http_status(:success) + expect(json_response.first).to have_key('created_by') + expect(json_response.first['created_by']).to eq(nil) + end + + it 'is created_by a user and has those details' do + created = create(:user, created_by_id: user.id) + + get api("/users", admin), params: { username: created.username } + + expect(response).to have_gitlab_http_status(:success) + expect(json_response.first['created_by'].symbolize_keys) + .to eq(API::Entities::UserBasic.new(user).as_json) + end + end end context 'N+1 queries' do @@ -940,6 +962,17 @@ RSpec.describe API::Users do expect(user.followees).to contain_exactly(followee) expect(response).to have_gitlab_http_status(:created) end + + it 'alerts and not follow when over followee limit' do + stub_const('Users::UserFollowUser::MAX_FOLLOWEE_LIMIT', 2) + Users::UserFollowUser::MAX_FOLLOWEE_LIMIT.times { user.follow(create(:user)) } + + post api("/users/#{followee.id}/follow", user) + expect(response).to have_gitlab_http_status(:bad_request) + expected_message = format(_("You can't follow more than %{limit} users. To follow more users, unfollow some others."), limit: Users::UserFollowUser::MAX_FOLLOWEE_LIMIT) + expect(json_response['message']).to eq(expected_message) + expect(user.following?(followee)).to be_falsey + end end context 'on a followed user' do diff --git a/spec/requests/api/version_spec.rb b/spec/requests/api/version_spec.rb deleted file mode 100644 index 7abbaf4f9ec..00000000000 --- a/spec/requests/api/version_spec.rb +++ /dev/null @@ -1,93 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe API::Version do - shared_examples_for 'GET /version' do - context 'when unauthenticated' do - it 'returns authentication error' do - get api('/version') - - expect(response).to have_gitlab_http_status(:unauthorized) - end - end - - context 'when authenticated as user' do - let(:user) { create(:user) } - - it 'returns the version information' do - get api('/version', user) - - expect_version - end - end - - context 'when authenticated with token' do - let(:personal_access_token) { create(:personal_access_token, scopes: scopes) } - - context 'with api scope' do - let(:scopes) { %i(api) } - - it 'returns the version information' do - get api('/version', personal_access_token: personal_access_token) - - expect_version - end - - it 'returns "200" response on head requests' do - head api('/version', personal_access_token: personal_access_token) - - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'with read_user scope' do - let(:scopes) { %i(read_user) } - - it 'returns the version information' do - get api('/version', personal_access_token: personal_access_token) - - expect_version - end - - it 'returns "200" response on head requests' do - head api('/version', personal_access_token: personal_access_token) - - expect(response).to have_gitlab_http_status(:ok) - end - end - - context 'with neither api nor read_user scope' do - let(:scopes) { %i(read_repository) } - - it 'returns authorization error' do - get api('/version', personal_access_token: personal_access_token) - - expect(response).to have_gitlab_http_status(:forbidden) - end - end - end - - def expect_version - expect(response).to have_gitlab_http_status(:ok) - expect(json_response['version']).to eq(Gitlab::VERSION) - expect(json_response['revision']).to eq(Gitlab.revision) - end - end - - context 'with graphql enabled' do - before do - stub_feature_flags(graphql: true) - end - - include_examples 'GET /version' - end - - context 'with graphql disabled' do - before do - stub_feature_flags(graphql: false) - end - - include_examples 'GET /version' - end -end -- cgit v1.2.3