diff --git a/app/finders/ci/jobs_finder.rb b/app/finders/ci/jobs_finder.rb index 40c610f82096ce03640e312a7422d3dfbefa9ddc..78791d737da8bbb9f197fc158fb634cf5351fdff 100644 --- a/app/finders/ci/jobs_finder.rb +++ b/app/finders/ci/jobs_finder.rb @@ -25,11 +25,7 @@ def execute attr_reader :current_user, :pipeline, :project, :params, :type def init_collection - if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true) - pipeline_jobs || project_jobs || all_jobs - else - project ? project_builds : all_jobs - end + pipeline_jobs || project_jobs || all_jobs end def all_jobs @@ -38,12 +34,6 @@ def all_jobs type.all end - def project_builds - raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_build, project) - - project.builds.relevant - end - def project_jobs return unless project raise Gitlab::Access::AccessDeniedError unless can?(current_user, :read_build, project) @@ -59,9 +49,7 @@ def pipeline_jobs end def filter_by_scope(builds) - if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true) - return filter_by_statuses!(params[:scope], builds) if params[:scope].is_a?(Array) - end + return filter_by_statuses!(params[:scope], builds) if params[:scope].is_a?(Array) case params[:scope] when 'pending' diff --git a/config/feature_flags/development/ci_jobs_finder_refactor.yml b/config/feature_flags/development/ci_jobs_finder_refactor.yml deleted file mode 100644 index f43db747e0ac9e055a206fc47bf91114bb4b556e..0000000000000000000000000000000000000000 --- a/config/feature_flags/development/ci_jobs_finder_refactor.yml +++ /dev/null @@ -1,7 +0,0 @@ ---- -name: ci_jobs_finder_refactor -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36622 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/245183 -group: group::continuous integration -type: development -default_enabled: true diff --git a/lib/api/ci/pipelines.rb b/lib/api/ci/pipelines.rb index 61e03ed1a95157ae044bde1ef141497da5ebb68e..dcbdfcd76f4dcf24c81a6e86966e872d31f0cd1b 100644 --- a/lib/api/ci/pipelines.rb +++ b/lib/api/ci/pipelines.rb @@ -128,15 +128,9 @@ class Pipelines < ::API::Base pipeline = user_project.all_pipelines.find(params[:pipeline_id]) - if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true) - builds = ::Ci::JobsFinder - .new(current_user: current_user, pipeline: pipeline, params: params) - .execute - else - authorize!(:read_build, pipeline) - builds = pipeline.builds - builds = filter_builds(builds, params[:scope]) - end + builds = ::Ci::JobsFinder + .new(current_user: current_user, pipeline: pipeline, params: params) + .execute builds = builds.with_preloads @@ -157,16 +151,9 @@ class Pipelines < ::API::Base pipeline = user_project.all_pipelines.find(params[:pipeline_id]) - if Feature.enabled?(:ci_jobs_finder_refactor, default_enabled: true) - bridges = ::Ci::JobsFinder - .new(current_user: current_user, pipeline: pipeline, params: params, type: ::Ci::Bridge) - .execute - else - authorize!(:read_pipeline, pipeline) - bridges = pipeline.bridges - bridges = filter_builds(bridges, params[:scope]) - end - + bridges = ::Ci::JobsFinder + .new(current_user: current_user, pipeline: pipeline, params: params, type: ::Ci::Bridge) + .execute bridges = bridges.with_preloads present paginate(bridges), with: Entities::Ci::Bridge @@ -246,21 +233,6 @@ class Pipelines < ::API::Base end helpers do - # NOTE: This method should be removed once the ci_jobs_finder_refactor FF is - # removed. https://gitlab.com/gitlab-org/gitlab/-/issues/245183 - # rubocop: disable CodeReuse/ActiveRecord - def filter_builds(builds, scope) - return builds if scope.nil? || scope.empty? - - available_statuses = ::CommitStatus::AVAILABLE_STATUSES - - unknown = scope - available_statuses - render_api_error!('Scope contains invalid value(s)', 400) unless unknown.empty? - - builds.where(status: scope) - end - # rubocop: enable CodeReuse/ActiveRecord - def pipeline strong_memoize(:pipeline) do user_project.all_pipelines.find(params[:pipeline_id]) diff --git a/spec/finders/ci/jobs_finder_spec.rb b/spec/finders/ci/jobs_finder_spec.rb index a6a41c3648950b9f4562fa0a1b43425d7f9d876c..4a6585e3f2bc343aecd83ec10d3e8af570341ffa 100644 --- a/spec/finders/ci/jobs_finder_spec.rb +++ b/spec/finders/ci/jobs_finder_spec.rb @@ -36,135 +36,62 @@ end end - context 'with ci_jobs_finder_refactor ff enabled' do - before do - stub_feature_flags(ci_jobs_finder_refactor: true) - end - - context 'scope is present' do - let(:jobs) { [job_1, job_2, job_3] } - - where(:scope, :index) do - [ - ['pending', 0], - ['running', 1], - ['finished', 2] - ] - end - - with_them do - let(:params) { { scope: scope } } - - it { expect(subject).to match_array([jobs[index]]) } - end + context 'scope is present' do + let(:jobs) { [job_1, job_2, job_3] } + + where(:scope, :index) do + [ + ['pending', 0], + ['running', 1], + ['finished', 2] + ] end - context 'scope is an array' do - let(:jobs) { [job_1, job_2, job_3] } - let(:params) {{ scope: ['running'] }} + with_them do + let(:params) { { scope: scope } } - it 'filters by the job statuses in the scope' do - expect(subject).to match_array([job_2]) - end + it { expect(subject).to match_array([jobs[index]]) } end end - context 'with ci_jobs_finder_refactor ff disabled' do - before do - stub_feature_flags(ci_jobs_finder_refactor: false) - end - - context 'scope is present' do - let(:jobs) { [job_1, job_2, job_3] } - - where(:scope, :index) do - [ - ['pending', 0], - ['running', 1], - ['finished', 2] - ] - end + context 'scope is an array' do + let(:jobs) { [job_1, job_2, job_3] } + let(:params) {{ scope: ['running'] }} - with_them do - let(:params) { { scope: scope } } - - it { expect(subject).to match_array([jobs[index]]) } - end + it 'filters by the job statuses in the scope' do + expect(subject).to match_array([job_2]) end end end - context 'with ci_jobs_finder_refactor ff enabled' do - before do - stub_feature_flags(ci_jobs_finder_refactor: true) - end - - context 'a project is present' do - subject { described_class.new(current_user: user, project: project, params: params).execute } - - context 'user has access to the project' do - before do - project.add_maintainer(user) - end - - it 'returns jobs for the specified project' do - expect(subject).to match_array([job_3]) - end - end + context 'a project is present' do + subject { described_class.new(current_user: user, project: project, params: params).execute } - context 'user has no access to project builds' do - before do - project.add_guest(user) - end - - it 'returns no jobs' do - expect(subject).to be_empty - end + context 'user has access to the project' do + before do + project.add_maintainer(user) end - context 'without user' do - let(:user) { nil } - - it 'returns no jobs' do - expect(subject).to be_empty - end + it 'returns jobs for the specified project' do + expect(subject).to match_array([job_3]) end end - end - - context 'with ci_jobs_finder_refactor ff disabled' do - before do - stub_feature_flags(ci_jobs_finder_refactor: false) - end - context 'a project is present' do - subject { described_class.new(current_user: user, project: project, params: params).execute } - context 'user has access to the project' do - before do - project.add_maintainer(user) - end - - it 'returns jobs for the specified project' do - expect(subject).to match_array([job_3]) - end + context 'user has no access to project builds' do + before do + project.add_guest(user) end - context 'user has no access to project builds' do - before do - project.add_guest(user) - end - - it 'returns no jobs' do - expect(subject).to be_empty - end + it 'returns no jobs' do + expect(subject).to be_empty end + end - context 'without user' do - let(:user) { nil } + context 'without user' do + let(:user) { nil } - it 'returns no jobs' do - expect(subject).to be_empty - end + it 'returns no jobs' do + expect(subject).to be_empty end end end diff --git a/spec/requests/api/ci/pipelines_spec.rb b/spec/requests/api/ci/pipelines_spec.rb index 577b43e6e428abe4f842cf0dd06e0a83dcc56973..767b570485157c75f0c979b93f3807252e62c9c7 100644 --- a/spec/requests/api/ci/pipelines_spec.rb +++ b/spec/requests/api/ci/pipelines_spec.rb @@ -325,236 +325,113 @@ end end - context 'with ci_jobs_finder_refactor ff enabled' do - before do - stub_feature_flags(ci_jobs_finder_refactor: true) + context 'authorized user' do + it 'returns pipeline jobs' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array end - context 'authorized user' do - it 'returns pipeline jobs' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - end - - it 'returns correct values' do - expect(json_response).not_to be_empty - expect(json_response.first['commit']['id']).to eq project.commit.id - expect(Time.parse(json_response.first['artifacts_expire_at'])).to be_like_time(job.artifacts_expire_at) - expect(json_response.first['artifacts_file']).to be_nil - expect(json_response.first['artifacts']).to be_an Array - expect(json_response.first['artifacts']).to be_empty - end - - it_behaves_like 'a job with artifacts and trace' do - let(:api_endpoint) { "/projects/#{project.id}/pipelines/#{pipeline.id}/jobs" } - end - - it 'returns pipeline data' do - json_job = json_response.first - - expect(json_job['pipeline']).not_to be_empty - expect(json_job['pipeline']['id']).to eq job.pipeline.id - expect(json_job['pipeline']['ref']).to eq job.pipeline.ref - expect(json_job['pipeline']['sha']).to eq job.pipeline.sha - expect(json_job['pipeline']['status']).to eq job.pipeline.status - end - - context 'filter jobs with one scope element' do - let(:query) { { 'scope' => 'pending' } } - - it do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - end - end - - context 'filter jobs with hash' do - let(:query) { { scope: { hello: 'pending', world: 'running' } } } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end - - context 'filter jobs with array of scope elements' do - let(:query) { { scope: %w(pending running) } } - - it do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - end - end - - context 'respond 400 when scope contains invalid state' do - let(:query) { { scope: %w(unknown running) } } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end - - context 'jobs in different pipelines' do - let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } - let!(:job2) { create(:ci_build, pipeline: pipeline2) } - - it 'excludes jobs from other pipelines' do - json_response.each { |job| expect(job['pipeline']['id']).to eq(pipeline.id) } - end - end - - it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query - end.count - - create_list(:ci_build, 3, :trace_artifact, :artifacts, :test_reports, pipeline: pipeline) - - expect do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query - end.not_to exceed_all_query_limit(control_count) - end + it 'returns correct values' do + expect(json_response).not_to be_empty + expect(json_response.first['commit']['id']).to eq project.commit.id + expect(Time.parse(json_response.first['artifacts_expire_at'])).to be_like_time(job.artifacts_expire_at) + expect(json_response.first['artifacts_file']).to be_nil + expect(json_response.first['artifacts']).to be_an Array + expect(json_response.first['artifacts']).to be_empty end - context 'no pipeline is found' do - it 'does not return jobs' do - get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/jobs", user) - - expect(json_response['message']).to eq '404 Project Not Found' - expect(response).to have_gitlab_http_status(:not_found) - end + it_behaves_like 'a job with artifacts and trace' do + let(:api_endpoint) { "/projects/#{project.id}/pipelines/#{pipeline.id}/jobs" } end - context 'unauthorized user' do - context 'when user is not logged in' do - let(:api_user) { nil } + it 'returns pipeline data' do + json_job = json_response.first - it 'does not return jobs' do - expect(json_response['message']).to eq '404 Project Not Found' - expect(response).to have_gitlab_http_status(:not_found) - end - end - - context 'when user is guest' do - let(:guest) { create(:project_member, :guest, project: project).user } - let(:api_user) { guest } - - it 'does not return jobs' do - expect(response).to have_gitlab_http_status(:forbidden) - end - end + expect(json_job['pipeline']).not_to be_empty + expect(json_job['pipeline']['id']).to eq job.pipeline.id + expect(json_job['pipeline']['ref']).to eq job.pipeline.ref + expect(json_job['pipeline']['sha']).to eq job.pipeline.sha + expect(json_job['pipeline']['status']).to eq job.pipeline.status end - end - context 'with ci_jobs_finder ff disabled' do - before do - stub_feature_flags(ci_jobs_finder_refactor: false) - end + context 'filter jobs with one scope element' do + let(:query) { { 'scope' => 'pending' } } - context 'authorized user' do - it 'returns pipeline jobs' do + it do expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers expect(json_response).to be_an Array end + end - it 'returns correct values' do - expect(json_response).not_to be_empty - expect(json_response.first['commit']['id']).to eq project.commit.id - expect(Time.parse(json_response.first['artifacts_expire_at'])).to be_like_time(job.artifacts_expire_at) - expect(json_response.first['artifacts_file']).to be_nil - expect(json_response.first['artifacts']).to be_an Array - expect(json_response.first['artifacts']).to be_empty - end - - it_behaves_like 'a job with artifacts and trace' do - let(:api_endpoint) { "/projects/#{project.id}/pipelines/#{pipeline.id}/jobs" } - end - - it 'returns pipeline data' do - json_job = json_response.first + context 'filter jobs with hash' do + let(:query) { { scope: { hello: 'pending', world: 'running' } } } - expect(json_job['pipeline']).not_to be_empty - expect(json_job['pipeline']['id']).to eq job.pipeline.id - expect(json_job['pipeline']['ref']).to eq job.pipeline.ref - expect(json_job['pipeline']['sha']).to eq job.pipeline.sha - expect(json_job['pipeline']['status']).to eq job.pipeline.status - end + it { expect(response).to have_gitlab_http_status(:bad_request) } + end - context 'filter jobs with one scope element' do - let(:query) { { 'scope' => 'pending' } } + context 'filter jobs with array of scope elements' do + let(:query) { { scope: %w(pending running) } } - it do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - end + it do + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array end + end - context 'filter jobs with hash' do - let(:query) { { scope: { hello: 'pending', world: 'running' } } } + context 'respond 400 when scope contains invalid state' do + let(:query) { { scope: %w(unknown running) } } - it { expect(response).to have_gitlab_http_status(:bad_request) } - end + it { expect(response).to have_gitlab_http_status(:bad_request) } + end - context 'filter jobs with array of scope elements' do - let(:query) { { scope: %w(pending running) } } + context 'jobs in different pipelines' do + let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } + let!(:job2) { create(:ci_build, pipeline: pipeline2) } - it do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - end + it 'excludes jobs from other pipelines' do + json_response.each { |job| expect(job['pipeline']['id']).to eq(pipeline.id) } end + end - context 'respond 400 when scope contains invalid state' do - let(:query) { { scope: %w(unknown running) } } + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query + end.count - it { expect(response).to have_gitlab_http_status(:bad_request) } - end + create_list(:ci_build, 3, :trace_artifact, :artifacts, :test_reports, pipeline: pipeline) - context 'jobs in different pipelines' do - let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } - let!(:job2) { create(:ci_build, pipeline: pipeline2) } - - it 'excludes jobs from other pipelines' do - json_response.each { |job| expect(job['pipeline']['id']).to eq(pipeline.id) } - end - end - - it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query - end.count + expect do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query + end.not_to exceed_all_query_limit(control_count) + end + end - create_list(:ci_build, 3, :trace_artifact, :artifacts, :test_reports, pipeline: pipeline) + context 'no pipeline is found' do + it 'does not return jobs' do + get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/jobs", user) - expect do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/jobs", api_user), params: query - end.not_to exceed_all_query_limit(control_count) - end + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) end + end - context 'no pipeline is found' do - it 'does not return jobs' do - get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/jobs", user) + context 'unauthorized user' do + context 'when user is not logged in' do + let(:api_user) { nil } + it 'does not return jobs' do expect(json_response['message']).to eq '404 Project Not Found' expect(response).to have_gitlab_http_status(:not_found) end end - context 'unauthorized user' do - context 'when user is not logged in' do - let(:api_user) { nil } - - it 'does not return jobs' do - expect(json_response['message']).to eq '404 Project Not Found' - expect(response).to have_gitlab_http_status(:not_found) - end - end + context 'when user is guest' do + let(:guest) { create(:project_member, :guest, project: project).user } + let(:api_user) { guest } - context 'when user is guest' do - let(:guest) { create(:project_member, :guest, project: project).user } - let(:api_user) { guest } - - it 'does not return jobs' do - expect(response).to have_gitlab_http_status(:forbidden) - end + it 'does not return jobs' do + expect(response).to have_gitlab_http_status(:forbidden) end end end @@ -583,314 +460,152 @@ end end - context 'with ci_jobs_finder_refactor ff enabled' do - before do - stub_feature_flags(ci_jobs_finder_refactor: true) + context 'authorized user' do + it 'returns pipeline bridges' do + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array end - context 'authorized user' do - it 'returns pipeline bridges' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - end + it 'returns correct values' do + expect(json_response).not_to be_empty + expect(json_response.first['commit']['id']).to eq project.commit.id + expect(json_response.first['id']).to eq bridge.id + expect(json_response.first['name']).to eq bridge.name + expect(json_response.first['stage']).to eq bridge.stage + end - it 'returns correct values' do - expect(json_response).not_to be_empty - expect(json_response.first['commit']['id']).to eq project.commit.id - expect(json_response.first['id']).to eq bridge.id - expect(json_response.first['name']).to eq bridge.name - expect(json_response.first['stage']).to eq bridge.stage - end + it 'returns pipeline data' do + json_bridge = json_response.first - it 'returns pipeline data' do - json_bridge = json_response.first + expect(json_bridge['pipeline']).not_to be_empty + expect(json_bridge['pipeline']['id']).to eq bridge.pipeline.id + expect(json_bridge['pipeline']['ref']).to eq bridge.pipeline.ref + expect(json_bridge['pipeline']['sha']).to eq bridge.pipeline.sha + expect(json_bridge['pipeline']['status']).to eq bridge.pipeline.status + end - expect(json_bridge['pipeline']).not_to be_empty - expect(json_bridge['pipeline']['id']).to eq bridge.pipeline.id - expect(json_bridge['pipeline']['ref']).to eq bridge.pipeline.ref - expect(json_bridge['pipeline']['sha']).to eq bridge.pipeline.sha - expect(json_bridge['pipeline']['status']).to eq bridge.pipeline.status - end + it 'returns downstream pipeline data' do + json_bridge = json_response.first - it 'returns downstream pipeline data' do - json_bridge = json_response.first + expect(json_bridge['downstream_pipeline']).not_to be_empty + expect(json_bridge['downstream_pipeline']['id']).to eq downstream_pipeline.id + expect(json_bridge['downstream_pipeline']['ref']).to eq downstream_pipeline.ref + expect(json_bridge['downstream_pipeline']['sha']).to eq downstream_pipeline.sha + expect(json_bridge['downstream_pipeline']['status']).to eq downstream_pipeline.status + end - expect(json_bridge['downstream_pipeline']).not_to be_empty - expect(json_bridge['downstream_pipeline']['id']).to eq downstream_pipeline.id - expect(json_bridge['downstream_pipeline']['ref']).to eq downstream_pipeline.ref - expect(json_bridge['downstream_pipeline']['sha']).to eq downstream_pipeline.sha - expect(json_bridge['downstream_pipeline']['status']).to eq downstream_pipeline.status + context 'filter bridges' do + before_all do + create_bridge(pipeline, :pending) + create_bridge(pipeline, :running) end - context 'filter bridges' do - before_all do - create_bridge(pipeline, :pending) - create_bridge(pipeline, :running) - end - - context 'with one scope element' do - let(:query) { { 'scope' => 'pending' } } - - it :skip_before_request do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - expect(json_response.count).to eq 1 - expect(json_response.first["status"]).to eq "pending" - end - end - - context 'with array of scope elements' do - let(:query) { { scope: %w(pending running) } } + context 'with one scope element' do + let(:query) { { 'scope' => 'pending' } } - it :skip_before_request do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + it :skip_before_request do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - expect(json_response.count).to eq 2 - json_response.each { |r| expect(%w(pending running).include?(r['status'])).to be true } - end + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.count).to eq 1 + expect(json_response.first["status"]).to eq "pending" end end - context 'respond 400 when scope contains invalid state' do - context 'in an array' do - let(:query) { { scope: %w(unknown running) } } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end - - context 'in a hash' do - let(:query) { { scope: { unknown: true } } } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end + context 'with array of scope elements' do + let(:query) { { scope: %w(pending running) } } - context 'in a string' do - let(:query) { { scope: "unknown" } } + it :skip_before_request do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - it { expect(response).to have_gitlab_http_status(:bad_request) } + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to be_an Array + expect(json_response.count).to eq 2 + json_response.each { |r| expect(%w(pending running).include?(r['status'])).to be true } end end + end - context 'bridges in different pipelines' do - let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } - let!(:bridge2) { create(:ci_bridge, pipeline: pipeline2) } + context 'respond 400 when scope contains invalid state' do + context 'in an array' do + let(:query) { { scope: %w(unknown running) } } - it 'excludes bridges from other pipelines' do - json_response.each { |bridge| expect(bridge['pipeline']['id']).to eq(pipeline.id) } - end + it { expect(response).to have_gitlab_http_status(:bad_request) } end - it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - end.count - - 3.times { create_bridge(pipeline) } + context 'in a hash' do + let(:query) { { scope: { unknown: true } } } - expect do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - end.not_to exceed_all_query_limit(control_count) + it { expect(response).to have_gitlab_http_status(:bad_request) } end - end - context 'no pipeline is found' do - it 'does not return bridges' do - get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/bridges", user) + context 'in a string' do + let(:query) { { scope: "unknown" } } - expect(json_response['message']).to eq '404 Project Not Found' - expect(response).to have_gitlab_http_status(:not_found) + it { expect(response).to have_gitlab_http_status(:bad_request) } end end - context 'unauthorized user' do - context 'when user is not logged in' do - let(:api_user) { nil } + context 'bridges in different pipelines' do + let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } + let!(:bridge2) { create(:ci_bridge, pipeline: pipeline2) } - it 'does not return bridges' do - expect(json_response['message']).to eq '404 Project Not Found' - expect(response).to have_gitlab_http_status(:not_found) - end + it 'excludes bridges from other pipelines' do + json_response.each { |bridge| expect(bridge['pipeline']['id']).to eq(pipeline.id) } end + end - context 'when user is guest' do - let(:api_user) { guest } - let(:guest) { create(:project_member, :guest, project: project).user } + it 'avoids N+1 queries' do + control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end.count - it 'does not return bridges' do - expect(response).to have_gitlab_http_status(:forbidden) - end - end + 3.times { create_bridge(pipeline) } - context 'when user has no read_build access for project' do - before do - project.add_guest(api_user) - end - - it 'does not return bridges' do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user) - expect(response).to have_gitlab_http_status(:forbidden) - end - end + expect do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query + end.not_to exceed_all_query_limit(control_count) end end - context 'with ci_jobs_finder_refactor ff disabled' do - before do - stub_feature_flags(ci_jobs_finder_refactor: false) - end - - context 'authorized user' do - it 'returns pipeline bridges' do - expect(response).to have_gitlab_http_status(:ok) - expect(response).to include_pagination_headers - expect(json_response).to be_an Array - end - - it 'returns correct values' do - expect(json_response).not_to be_empty - expect(json_response.first['commit']['id']).to eq project.commit.id - expect(json_response.first['id']).to eq bridge.id - expect(json_response.first['name']).to eq bridge.name - expect(json_response.first['stage']).to eq bridge.stage - end - - it 'returns pipeline data' do - json_bridge = json_response.first + context 'no pipeline is found' do + it 'does not return bridges' do + get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/bridges", user) - expect(json_bridge['pipeline']).not_to be_empty - expect(json_bridge['pipeline']['id']).to eq bridge.pipeline.id - expect(json_bridge['pipeline']['ref']).to eq bridge.pipeline.ref - expect(json_bridge['pipeline']['sha']).to eq bridge.pipeline.sha - expect(json_bridge['pipeline']['status']).to eq bridge.pipeline.status - end - - it 'returns downstream pipeline data' do - json_bridge = json_response.first - - expect(json_bridge['downstream_pipeline']).not_to be_empty - expect(json_bridge['downstream_pipeline']['id']).to eq downstream_pipeline.id - expect(json_bridge['downstream_pipeline']['ref']).to eq downstream_pipeline.ref - expect(json_bridge['downstream_pipeline']['sha']).to eq downstream_pipeline.sha - expect(json_bridge['downstream_pipeline']['status']).to eq downstream_pipeline.status - end - - context 'filter bridges' do - before_all do - create_bridge(pipeline, :pending) - create_bridge(pipeline, :running) - end - - context 'with one scope element' do - let(:query) { { 'scope' => 'pending' } } - - it :skip_before_request do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - expect(json_response.count).to eq 1 - expect(json_response.first["status"]).to eq "pending" - end - end - - context 'with array of scope elements' do - let(:query) { { scope: %w(pending running) } } - - it :skip_before_request do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to be_an Array - expect(json_response.count).to eq 2 - json_response.each { |r| expect(%w(pending running).include?(r['status'])).to be true } - end - end - end - - context 'respond 400 when scope contains invalid state' do - context 'in an array' do - let(:query) { { scope: %w(unknown running) } } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end - - context 'in a hash' do - let(:query) { { scope: { unknown: true } } } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end - - context 'in a string' do - let(:query) { { scope: "unknown" } } - - it { expect(response).to have_gitlab_http_status(:bad_request) } - end - end - - context 'bridges in different pipelines' do - let!(:pipeline2) { create(:ci_empty_pipeline, project: project) } - let!(:bridge2) { create(:ci_bridge, pipeline: pipeline2) } - - it 'excludes bridges from other pipelines' do - json_response.each { |bridge| expect(bridge['pipeline']['id']).to eq(pipeline.id) } - end - end - - it 'avoids N+1 queries' do - control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - end.count - - 3.times { create_bridge(pipeline) } - - expect do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user), params: query - end.not_to exceed_all_query_limit(control_count) - end + expect(json_response['message']).to eq '404 Project Not Found' + expect(response).to have_gitlab_http_status(:not_found) end + end - context 'no pipeline is found' do - it 'does not return bridges' do - get api("/projects/#{project2.id}/pipelines/#{pipeline.id}/bridges", user) + context 'unauthorized user' do + context 'when user is not logged in' do + let(:api_user) { nil } + it 'does not return bridges' do expect(json_response['message']).to eq '404 Project Not Found' expect(response).to have_gitlab_http_status(:not_found) end end - context 'unauthorized user' do - context 'when user is not logged in' do - let(:api_user) { nil } + context 'when user is guest' do + let(:api_user) { guest } + let(:guest) { create(:project_member, :guest, project: project).user } - it 'does not return bridges' do - expect(json_response['message']).to eq '404 Project Not Found' - expect(response).to have_gitlab_http_status(:not_found) - end + it 'does not return bridges' do + expect(response).to have_gitlab_http_status(:forbidden) end + end - context 'when user is guest' do - let(:api_user) { guest } - let(:guest) { create(:project_member, :guest, project: project).user } - - it 'does not return bridges' do - expect(response).to have_gitlab_http_status(:forbidden) - end + context 'when user has no read_build access for project' do + before do + project.add_guest(api_user) end - context 'when user has no read_build access for project' do - before do - project.add_guest(api_user) - end - - it 'does not return bridges' do - get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user) - expect(response).to have_gitlab_http_status(:forbidden) - end + it 'does not return bridges' do + get api("/projects/#{project.id}/pipelines/#{pipeline.id}/bridges", api_user) + expect(response).to have_gitlab_http_status(:forbidden) end end end