diff options
Diffstat (limited to 'spec/requests/api/graphql/project')
8 files changed, 223 insertions, 190 deletions
diff --git a/spec/requests/api/graphql/project/alert_management/alert/issue_spec.rb b/spec/requests/api/graphql/project/alert_management/alert/issue_spec.rb new file mode 100644 index 00000000000..9724de4fedb --- /dev/null +++ b/spec/requests/api/graphql/project/alert_management/alert/issue_spec.rb @@ -0,0 +1,71 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'getting Alert Management Alert Issue' do + include GraphqlHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:current_user) { create(:user) } + let(:payload) { {} } + let(:query) { 'avg(metric) > 1.0' } + + let(:fields) do + <<~QUERY + nodes { + iid + issue { + iid + state + } + } + QUERY + end + + let(:graphql_query) do + graphql_query_for( + 'project', + { 'fullPath' => project.full_path }, + query_graphql_field('alertManagementAlerts', {}, fields) + ) + end + + let(:alerts) { graphql_data.dig('project', 'alertManagementAlerts', 'nodes') } + let(:first_alert) { alerts.first } + + before do + project.add_developer(current_user) + end + + context 'with gitlab alert' do + before do + create(:alert_management_alert, :with_issue, project: project, payload: payload) + end + + it 'includes the correct alert issue payload data' do + post_graphql(graphql_query, current_user: current_user) + + expect(first_alert).to include('issue' => { "iid" => "1", "state" => "opened" }) + end + end + + describe 'performance' do + let(:first_n) { var('Int') } + let(:params) { { first: first_n } } + let(:limited_query) { with_signature([first_n], query) } + + context 'with gitlab alert' do + before do + create(:alert_management_alert, :with_issue, project: project, payload: payload) + end + + it 'avoids N+1 queries' do + base_count = ActiveRecord::QueryRecorder.new do + post_graphql(limited_query, current_user: current_user, variables: first_n.with(1)) + end + + expect { post_graphql(limited_query, current_user: current_user) }.not_to exceed_query_limit(base_count) + end + end + end +end diff --git a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb index 8deed75a466..fe77d9dc86d 100644 --- a/spec/requests/api/graphql/project/alert_management/alerts_spec.rb +++ b/spec/requests/api/graphql/project/alert_management/alerts_spec.rb @@ -7,7 +7,7 @@ RSpec.describe 'getting Alert Management Alerts' do let_it_be(:payload) { { 'custom' => { 'alert' => 'payload' }, 'runbook' => 'runbook' } } let_it_be(:project) { create(:project, :repository) } let_it_be(:current_user) { create(:user) } - let_it_be(:resolved_alert) { create(:alert_management_alert, :all_fields, :resolved, project: project, issue: nil, severity: :low).present } + let_it_be(:resolved_alert) { create(:alert_management_alert, :all_fields, :resolved, project: project, severity: :low).present } let_it_be(:triggered_alert) { create(:alert_management_alert, :all_fields, project: project, severity: :critical, payload: payload).present } let_it_be(:other_project_alert) { create(:alert_management_alert, :all_fields).present } @@ -60,7 +60,6 @@ RSpec.describe 'getting Alert Management Alerts' do it 'returns the correct properties of the alerts' do expect(first_alert).to include( 'iid' => triggered_alert.iid.to_s, - 'issueIid' => triggered_alert.issue_iid.to_s, 'title' => triggered_alert.title, 'description' => triggered_alert.description, 'severity' => triggered_alert.severity.upcase, @@ -82,7 +81,6 @@ RSpec.describe 'getting Alert Management Alerts' do expect(second_alert).to include( 'iid' => resolved_alert.iid.to_s, - 'issueIid' => resolved_alert.issue_iid.to_s, 'status' => 'RESOLVED', 'endedAt' => resolved_alert.ended_at.strftime('%Y-%m-%dT%H:%M:%SZ') ) diff --git a/spec/requests/api/graphql/project/container_repositories_spec.rb b/spec/requests/api/graphql/project/container_repositories_spec.rb index 2087d8c2cc3..5ffd48a7bc4 100644 --- a/spec/requests/api/graphql/project/container_repositories_spec.rb +++ b/spec/requests/api/graphql/project/container_repositories_spec.rb @@ -16,7 +16,7 @@ RSpec.describe 'getting container repositories in a project' do <<~GQL edges { node { - #{all_graphql_fields_for('container_repositories'.classify)} + #{all_graphql_fields_for('container_repositories'.classify, excluded: ['pipeline'])} } } GQL diff --git a/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb b/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb index dd16b052e0e..b1ecb32b365 100644 --- a/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb +++ b/spec/requests/api/graphql/project/merge_request/diff_notes_spec.rb @@ -34,7 +34,7 @@ RSpec.describe 'getting notes for a merge request' do notes { edges { node { - #{all_graphql_fields_for('Note')} + #{all_graphql_fields_for('Note', excluded: ['pipeline'])} } } } diff --git a/spec/requests/api/graphql/project/merge_request_spec.rb b/spec/requests/api/graphql/project/merge_request_spec.rb index a4e8d0bc35e..e32899c600e 100644 --- a/spec/requests/api/graphql/project/merge_request_spec.rb +++ b/spec/requests/api/graphql/project/merge_request_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'getting merge request information nested in a project' do let(:current_user) { create(:user) } let(:merge_request_graphql_data) { graphql_data['project']['mergeRequest'] } let!(:merge_request) { create(:merge_request, source_project: project) } - let(:mr_fields) { all_graphql_fields_for('MergeRequest') } + let(:mr_fields) { all_graphql_fields_for('MergeRequest', excluded: ['pipeline']) } let(:query) do graphql_query_for( diff --git a/spec/requests/api/graphql/project/merge_requests_spec.rb b/spec/requests/api/graphql/project/merge_requests_spec.rb index d684be91dc9..d97a0ed9399 100644 --- a/spec/requests/api/graphql/project/merge_requests_spec.rb +++ b/spec/requests/api/graphql/project/merge_requests_spec.rb @@ -7,13 +7,27 @@ RSpec.describe 'getting merge request listings nested in a project' do let_it_be(:project) { create(:project, :repository, :public) } let_it_be(:current_user) { create(:user) } - let_it_be(:label) { create(:label, project: project) } - let_it_be(:merge_request_a) { create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label]) } - let_it_be(:merge_request_b) { create(:merge_request, :closed, :unique_branches, source_project: project) } - let_it_be(:merge_request_c) { create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label]) } - let_it_be(:merge_request_d) { create(:merge_request, :locked, :unique_branches, source_project: project) } - let_it_be(:merge_request_e) { create(:merge_request, :unique_branches, source_project: project) } + + let_it_be(:merge_request_a) do + create(:labeled_merge_request, :unique_branches, source_project: project, labels: [label]) + end + + let_it_be(:merge_request_b) do + create(:merge_request, :closed, :unique_branches, source_project: project) + end + + let_it_be(:merge_request_c) do + create(:labeled_merge_request, :closed, :unique_branches, source_project: project, labels: [label]) + end + + let_it_be(:merge_request_d) do + create(:merge_request, :locked, :unique_branches, source_project: project) + end + + let_it_be(:merge_request_e) do + create(:merge_request, :unique_branches, source_project: project) + end let(:results) { graphql_data.dig('project', 'mergeRequests', 'nodes') } @@ -27,32 +41,38 @@ RSpec.describe 'getting merge request listings nested in a project' do ) end - let(:query) do - query_merge_requests(all_graphql_fields_for('MergeRequest', max_depth: 1)) - end - it_behaves_like 'a working graphql query' do + let(:query) do + query_merge_requests(all_graphql_fields_for('MergeRequest', max_depth: 2)) + end + before do - post_graphql(query, current_user: current_user) + # We cannot call the whitelist here, since the transaction does not + # begin until we enter the controller. + headers = { + 'X-GITLAB-QUERY-WHITELIST-ISSUE' => 'https://gitlab.com/gitlab-org/gitlab/-/issues/322979' + } + + post_graphql(query, current_user: current_user, headers: headers) end end # The following tests are needed to guarantee that we have correctly annotated # all the gitaly calls. Selecting combinations of fields may mask this due to # memoization. - context 'requesting a single field' do + context 'when requesting a single field' do let_it_be(:fresh_mr) { create(:merge_request, :unique_branches, source_project: project) } + let(:search_params) { { iids: [fresh_mr.iid.to_s] } } + let(:graphql_data) do + GitlabSchema.execute(query, context: { current_user: current_user }).to_h['data'] + end before do project.repository.expire_branches_cache end - let(:graphql_data) do - GitlabSchema.execute(query, context: { current_user: current_user }).to_h['data'] - end - - context 'selecting any single scalar field' do + context 'when selecting any single scalar field' do where(:field) do scalar_fields_of('MergeRequest').map { |name| [name] } end @@ -68,7 +88,7 @@ RSpec.describe 'getting merge request listings nested in a project' do end end - context 'selecting any single nested field' do + context 'when selecting any single nested field' do where(:field, :subfield, :is_connection) do nested_fields_of('MergeRequest').flat_map do |name, field| type = field_type(field) @@ -95,7 +115,11 @@ RSpec.describe 'getting merge request listings nested in a project' do end end - shared_examples 'searching with parameters' do + shared_examples 'when searching with parameters' do + let(:query) do + query_merge_requests('iid title') + end + let(:expected) do mrs.map { |mr| a_hash_including('iid' => mr.iid.to_s, 'title' => mr.title) } end @@ -107,60 +131,60 @@ RSpec.describe 'getting merge request listings nested in a project' do end end - context 'there are no search params' do + context 'when there are no search params' do let(:search_params) { nil } let(:mrs) { [merge_request_a, merge_request_b, merge_request_c, merge_request_d, merge_request_e] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'the search params do not match anything' do - let(:search_params) { { iids: %w(foo bar baz) } } + context 'when the search params do not match anything' do + let(:search_params) { { iids: %w[foo bar baz] } } let(:mrs) { [] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by iids' do + context 'when searching by iids' do let(:search_params) { { iids: mrs.map(&:iid).map(&:to_s) } } let(:mrs) { [merge_request_a, merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by state' do + context 'when searching by state' do let(:search_params) { { state: :closed } } let(:mrs) { [merge_request_b, merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by source_branch' do + context 'when searching by source_branch' do let(:search_params) { { source_branches: mrs.map(&:source_branch) } } let(:mrs) { [merge_request_b, merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by target_branch' do + context 'when searching by target_branch' do let(:search_params) { { target_branches: mrs.map(&:target_branch) } } let(:mrs) { [merge_request_a, merge_request_d] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by label' do + context 'when searching by label' do let(:search_params) { { labels: [label.title] } } let(:mrs) { [merge_request_a, merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end - context 'searching by combination' do + context 'when searching by combination' do let(:search_params) { { state: :closed, labels: [label.title] } } let(:mrs) { [merge_request_c] } - it_behaves_like 'searching with parameters' + it_behaves_like 'when searching with parameters' end context 'when requesting `approved_by`' do @@ -203,10 +227,10 @@ RSpec.describe 'getting merge request listings nested in a project' do it 'exposes `commit_count`' do execute_query - expect(results).to match_array([ + expect(results).to match_array [ { "iid" => merge_request_a.iid.to_s, "commitCount" => 0 }, { "iid" => merge_request_with_commits.iid.to_s, "commitCount" => 29 } - ]) + ] end end @@ -216,8 +240,8 @@ RSpec.describe 'getting merge request listings nested in a project' do before do # make the MRs "merged" [merge_request_a, merge_request_b, merge_request_c].each do |mr| - mr.update_column(:state_id, MergeRequest.available_states[:merged]) - mr.metrics.update_column(:merged_at, Time.now) + mr.update!(state_id: MergeRequest.available_states[:merged]) + mr.metrics.update!(merged_at: Time.now) end end @@ -256,13 +280,12 @@ RSpec.describe 'getting merge request listings nested in a project' do end it 'returns the reviewers' do + nodes = merge_request_a.reviewers.map { |r| { 'username' => r.username } } + reviewers = { 'nodes' => match_array(nodes) } + execute_query - expect(results).to include a_hash_including('reviewers' => { - 'nodes' => match_array(merge_request_a.reviewers.map do |r| - a_hash_including('username' => r.username) - end) - }) + expect(results).to include a_hash_including('reviewers' => match(reviewers)) end include_examples 'N+1 query check' @@ -309,12 +332,14 @@ RSpec.describe 'getting merge request listings nested in a project' do allow(Gitlab::Database).to receive(:read_only?).and_return(false) end + def query_context + { current_user: current_user } + end + def run_query(number) # Ensure that we have a fresh request store and batch-context between runs - result = run_with_clean_state(query, - context: { current_user: current_user }, - variables: { first: number } - ) + vars = { first: number } + result = run_with_clean_state(query, context: query_context, variables: vars) graphql_dig_at(result.to_h, :data, :project, :merge_requests, :nodes) end @@ -348,39 +373,49 @@ RSpec.describe 'getting merge request listings nested in a project' do let(:data_path) { [:project, :mergeRequests] } def pagination_query(params) - graphql_query_for(:project, { full_path: project.full_path }, - <<~QUERY + graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY) mergeRequests(#{params}) { #{page_info} nodes { id } } - QUERY - ) + QUERY end context 'when sorting by merged_at DESC' do - it_behaves_like 'sorted paginated query' do - let(:sort_param) { :MERGED_AT_DESC } - let(:first_param) { 2 } + let(:sort_param) { :MERGED_AT_DESC } + let(:expected_results) do + [ + merge_request_b, + merge_request_d, + merge_request_c, + merge_request_e, + merge_request_a + ].map { |mr| global_id_of(mr) } + end - let(:expected_results) do - [ - merge_request_b, - merge_request_d, - merge_request_c, - merge_request_e, - merge_request_a - ].map { |mr| global_id_of(mr) } - end + before do + five_days_ago = 5.days.ago - before do - five_days_ago = 5.days.ago + merge_request_d.metrics.update!(merged_at: five_days_ago) + + # same merged_at, the second order column will decide (merge_request.id) + merge_request_c.metrics.update!(merged_at: five_days_ago) + + merge_request_b.metrics.update!(merged_at: 1.day.ago) + end + + it_behaves_like 'sorted paginated query' do + let(:first_param) { 2 } + end - merge_request_d.metrics.update!(merged_at: five_days_ago) + context 'when last parameter is given' do + let(:params) { graphql_args(sort: sort_param, last: 2) } + let(:page_info) { nil } - # same merged_at, the second order column will decide (merge_request.id) - merge_request_c.metrics.update!(merged_at: five_days_ago) + it 'takes the last 2 records' do + query = pagination_query(params) + post_graphql(query, current_user: current_user) - merge_request_b.metrics.update!(merged_at: 1.day.ago) + expect(results.map { |item| item["id"] }).to eq(expected_results.last(2)) end end end @@ -396,75 +431,45 @@ RSpec.describe 'getting merge request listings nested in a project' do let(:query) do # Note: __typename meta field is always requested by the FE - graphql_query_for(:project, { full_path: project.full_path }, - <<~QUERY + graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY) mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0, sourceBranches: null, labels: null) { count __typename } QUERY - ) end - shared_examples 'count examples' do - it 'returns the correct count' do - post_graphql(query, current_user: current_user) + it 'does not query the merge requests table for the count' do + query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - count = graphql_data.dig('project', 'mergeRequests', 'count') - expect(count).to eq(1) - end + queries = query_recorder.data.each_value.first[:occurrences] + expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) + expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) end - context 'when "optimized_merge_request_count_with_merged_at_filter" feature flag is enabled' do - before do - stub_feature_flags(optimized_merge_request_count_with_merged_at_filter: true) + context 'when total_time_to_merge and count is queried' do + let(:query) do + graphql_query_for(:project, { full_path: project.full_path }, <<~QUERY) + mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0) { + totalTimeToMerge + count + } + QUERY end - it 'does not query the merge requests table for the count' do + it 'does not query the merge requests table for the total_time_to_merge' do query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } queries = query_recorder.data.each_value.first[:occurrences] - expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) - expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) + expect(queries).to include(match(/SELECT.+SUM.+FROM "merge_request_metrics" WHERE/)) end + end - context 'when total_time_to_merge and count is queried' do - let(:query) do - graphql_query_for(:project, { full_path: project.full_path }, - <<~QUERY - mergeRequests(mergedAfter: "2020-01-01", mergedBefore: "2020-01-05", first: 0) { - totalTimeToMerge - count - } - QUERY - ) - end - - it 'does not query the merge requests table for the total_time_to_merge' do - query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - - queries = query_recorder.data.each_value.first[:occurrences] - expect(queries).to include(match(/SELECT.+SUM.+FROM "merge_request_metrics" WHERE/)) - end - end - - it_behaves_like 'count examples' - - context 'when "optimized_merge_request_count_with_merged_at_filter" feature flag is disabled' do - before do - stub_feature_flags(optimized_merge_request_count_with_merged_at_filter: false) - end - - it 'queries the merge requests table for the count' do - query_recorder = ActiveRecord::QueryRecorder.new { post_graphql(query, current_user: current_user) } - - queries = query_recorder.data.each_value.first[:occurrences] - expect(queries).to include(match(/SELECT COUNT\(\*\) FROM "merge_requests"/)) - expect(queries).not_to include(match(/SELECT COUNT\(\*\) FROM "merge_request_metrics"/)) - end + it 'returns the correct count' do + post_graphql(query, current_user: current_user) - it_behaves_like 'count examples' - end + count = graphql_data.dig('project', 'mergeRequests', 'count') + expect(count).to eq(1) end end end diff --git a/spec/requests/api/graphql/project/packages_spec.rb b/spec/requests/api/graphql/project/packages_spec.rb index b20c96d54c8..3c04e0caf61 100644 --- a/spec/requests/api/graphql/project/packages_spec.rb +++ b/spec/requests/api/graphql/project/packages_spec.rb @@ -5,28 +5,28 @@ require 'spec_helper' RSpec.describe 'getting a package list for a project' do include GraphqlHelpers - let_it_be(:project) { create(:project, :repository) } + let_it_be(:resource) { create(:project, :repository) } let_it_be(:current_user) { create(:user) } - let_it_be(:package) { create(:package, project: project) } - let_it_be(:maven_package) { create(:maven_package, project: project) } - let_it_be(:debian_package) { create(:debian_package, project: project) } - let_it_be(:composer_package) { create(:composer_package, project: project) } + let_it_be(:package) { create(:package, project: resource) } + let_it_be(:maven_package) { create(:maven_package, project: resource) } + let_it_be(:debian_package) { create(:debian_package, project: resource) } + let_it_be(:composer_package) { create(:composer_package, project: resource) } let_it_be(:composer_metadatum) do create(:composer_metadatum, package: composer_package, target_sha: 'afdeh', composer_json: { name: 'x', type: 'y', license: 'z', version: 1 }) end - let(:package_names) { graphql_data_at(:project, :packages, :edges, :node, :name) } + let(:package_names) { graphql_data_at(:project, :packages, :nodes, :name) } + let(:target_shas) { graphql_data_at(:project, :packages, :nodes, :metadata, :target_sha) } + let(:packages) { graphql_data_at(:project, :packages, :nodes) } let(:fields) do <<~QUERY - edges { - node { - #{all_graphql_fields_for('packages'.classify, excluded: ['project'])} - metadata { #{query_graphql_fragment('ComposerMetadata')} } - } + nodes { + #{all_graphql_fields_for('packages'.classify, excluded: ['project'])} + metadata { #{query_graphql_fragment('ComposerMetadata')} } } QUERY end @@ -34,55 +34,10 @@ RSpec.describe 'getting a package list for a project' do let(:query) do graphql_query_for( 'project', - { 'fullPath' => project.full_path }, + { 'fullPath' => resource.full_path }, query_graphql_field('packages', {}, fields) ) end - context 'when user has access to the project' do - before do - project.add_reporter(current_user) - post_graphql(query, current_user: current_user) - end - - it_behaves_like 'a working graphql query' - - it 'returns packages successfully' do - expect(package_names).to contain_exactly( - package.name, - maven_package.name, - debian_package.name, - composer_package.name - ) - end - - it 'deals with metadata' do - target_shas = graphql_data_at(:project, :packages, :edges, :node, :metadata, :target_sha) - expect(target_shas).to contain_exactly(composer_metadatum.target_sha) - end - end - - context 'when the user does not have access to the project/packages' do - before do - post_graphql(query, current_user: current_user) - end - - it_behaves_like 'a working graphql query' - - it 'returns nil' do - expect(graphql_data['project']).to be_nil - end - end - - context 'when the user is not authenticated' do - before do - post_graphql(query) - end - - it_behaves_like 'a working graphql query' - - it 'returns nil' do - expect(graphql_data['project']).to be_nil - end - end + it_behaves_like 'group and project packages query' end diff --git a/spec/requests/api/graphql/project/pipeline_spec.rb b/spec/requests/api/graphql/project/pipeline_spec.rb index 6179b43629b..cc028ff2ff9 100644 --- a/spec/requests/api/graphql/project/pipeline_spec.rb +++ b/spec/requests/api/graphql/project/pipeline_spec.rb @@ -11,10 +11,14 @@ RSpec.describe 'getting pipeline information nested in a project' do let(:pipeline_graphql_data) { graphql_data['project']['pipeline'] } let!(:query) do - graphql_query_for( - 'project', - { 'fullPath' => project.full_path }, - query_graphql_field('pipeline', iid: pipeline.iid.to_s) + %( + query { + project(fullPath: "#{project.full_path}") { + pipeline(iid: "#{pipeline.iid}") { + configSource + } + } + } ) end |