Reduce queries in spec/requests/api/merge_requests_spec.rb

This reduces the queries from 97137 to 65136 and the query time from
02:04 to 01:27 in spec/requests/api/merge_requests.rb.

Signed-off-by: Rémy Coutable <remy@rymai.me>

1 files changed, 393 insertions, 14 deletions

diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index fee6312a9c7..4259fda7f04 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -4,32 +4,406 @@ describe API::MergeRequests do
   include ProjectForksHelper
 
   let(:base_time)   { Time.now }
-  let(:user)        { create(:user) }
-  let(:admin)       { create(:user, :admin) }
-  let(:non_member)  { create(:user) }
-  let!(:project)    { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
+  set(:user)        { create(:user) }
+  set(:admin)       { create(:user, :admin) }
+  let(:project)     { create(:project, :public, :repository, creator: user, namespace: user.namespace, only_allow_merge_if_pipeline_succeeds: false) }
   let(:milestone)   { create(:milestone, title: '1.0.0', project: project) }
-  let(:pipeline)    { create(:ci_empty_pipeline) }
-  let(:milestone1) { create(:milestone, title: '0.9', project: project) }
+  let(:milestone1)  { create(:milestone, title: '0.9', project: project) }
   let!(:merge_request) { create(:merge_request, :simple, milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Test", created_at: base_time) }
   let!(:merge_request_closed) { create(:merge_request, state: "closed", milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Closed test", created_at: base_time + 1.second) }
   let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test", created_at: base_time + 2.seconds, merge_commit_sha: '9999999999999999999999999999999999999999') }
   let!(:merge_request_locked) { create(:merge_request, state: "locked", milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: "Locked test", created_at: base_time + 1.second) }
   let!(:note)       { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") }
   let!(:note2)      { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") }
-  let!(:label) do
-    create(:label, title: 'label', color: '#FFAABB', project: project)
-  end
-  let!(:label2) { create(:label, title: 'a-test', color: '#FFFFFF', project: project) }
-  let!(:label_link) { create(:label_link, label: label, target: merge_request) }
-  let!(:label_link2) { create(:label_link, label: label2, target: merge_request) }
-  let!(:downvote) { create(:award_emoji, :downvote, awardable: merge_request) }
-  let!(:upvote) { create(:award_emoji, :upvote, awardable: merge_request) }
+  let(:label) { create(:label, title: 'label', color: '#FFAABB', project: project) }
+  let(:label2) { create(:label, title: 'a-test', color: '#FFFFFF', project: project) }
 
   before do
     project.add_reporter(user)
   end
 
+  shared_context 'with labels' do
+    before do
+      create(:label_link, label: label, target: merge_request)
+      create(:label_link, label: label2, target: merge_request)
+    end
+  end
+
+  shared_examples 'merge requests list' do
+    context 'when unauthenticated' do
+      it 'returns merge requests for public projects' do
+        get api(endpoint_path)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_an Array
+      end
+    end
+
+    context 'when authenticated' do
+      it 'avoids N+1 queries' do
+        control = ActiveRecord::QueryRecorder.new do
+          get api(endpoint_path, user)
+        end
+
+        create(:merge_request, state: 'closed', milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: 'Test', created_at: base_time)
+
+        merge_request = create(:merge_request, milestone: milestone1, author: user, assignee: user, source_project: project, target_project: project, title: 'Test', created_at: base_time)
+
+        merge_request.metrics.update!(merged_by: user,
+                                      latest_closed_by: user,
+                                      latest_closed_at: 1.hour.ago,
+                                      merged_at: 2.hours.ago)
+
+        expect do
+          get api(endpoint_path, user)
+        end.not_to exceed_query_limit(control)
+      end
+
+      context 'with labels' do
+        include_context 'with labels'
+
+        it 'returns an array of all merge_requests' do
+          get api(endpoint_path, user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to include_pagination_headers
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(4)
+          expect(json_response.last['title']).to eq(merge_request.title)
+          expect(json_response.last).to have_key('web_url')
+          expect(json_response.last['sha']).to eq(merge_request.diff_head_sha)
+          expect(json_response.last['merge_commit_sha']).to be_nil
+          expect(json_response.last['merge_commit_sha']).to eq(merge_request.merge_commit_sha)
+          expect(json_response.last['downvotes']).to eq(0)
+          expect(json_response.last['upvotes']).to eq(0)
+          expect(json_response.last['labels']).to eq([label2.title, label.title])
+          expect(json_response.first['title']).to eq(merge_request_merged.title)
+          expect(json_response.first['sha']).to eq(merge_request_merged.diff_head_sha)
+          expect(json_response.first['merge_commit_sha']).not_to be_nil
+          expect(json_response.first['merge_commit_sha']).to eq(merge_request_merged.merge_commit_sha)
+        end
+      end
+
+      it 'returns an array of all merge_requests using simple mode' do
+        path = endpoint_path + '?view=simple'
+
+        get api(path, user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        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).to be_an Array
+        expect(json_response.length).to eq(4)
+        expect(json_response.last['iid']).to eq(merge_request.iid)
+        expect(json_response.last['title']).to eq(merge_request.title)
+        expect(json_response.last).to have_key('web_url')
+        expect(json_response.first['iid']).to eq(merge_request_merged.iid)
+        expect(json_response.first['title']).to eq(merge_request_merged.title)
+        expect(json_response.first).to have_key('web_url')
+      end
+
+      it 'returns an array of all merge_requests' do
+        path = endpoint_path + '?state'
+
+        get api(path, user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(4)
+        expect(json_response.last['title']).to eq(merge_request.title)
+      end
+
+      it 'returns an array of open merge_requests' do
+        path = endpoint_path + '?state=opened'
+
+        get api(path, user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+        expect(json_response.last['title']).to eq(merge_request.title)
+      end
+
+      it 'returns an array of closed merge_requests' do
+        path = endpoint_path + '?state=closed'
+
+        get api(path, user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+        expect(json_response.first['title']).to eq(merge_request_closed.title)
+      end
+
+      it 'returns an array of merged merge_requests' do
+        path = endpoint_path + '?state=merged'
+
+        get api(path, user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to include_pagination_headers
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+        expect(json_response.first['title']).to eq(merge_request_merged.title)
+      end
+
+      it 'matches V4 response schema' do
+        get api(endpoint_path, user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(response).to match_response_schema('public_api/v4/merge_requests')
+      end
+
+      it 'returns an empty array if no issue matches milestone' do
+        get api(endpoint_path, user), params: { milestone: '1.0.0' }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(0)
+      end
+
+      it 'returns an empty array if milestone does not exist' do
+        get api(endpoint_path, user), params: { milestone: 'foo' }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(0)
+      end
+
+      it 'returns an array of merge requests in given milestone' do
+        get api(endpoint_path, user), params: { milestone: '0.9' }
+
+        closed_issues = json_response.select { |mr| mr['id'] == merge_request_closed.id }
+        expect(closed_issues.length).to eq(1)
+        expect(closed_issues.first['title']).to eq merge_request_closed.title
+      end
+
+      it 'returns an array of merge requests matching state in milestone' do
+        get api(endpoint_path, user), params: { milestone: '0.9', state: 'closed' }
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+        expect(json_response.first['id']).to eq(merge_request_closed.id)
+      end
+
+      context 'with labels' do
+        include_context 'with labels'
+
+        it 'returns an array of labeled merge requests' do
+          path = endpoint_path + "?labels=#{label.title}"
+
+          get api(path, user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(1)
+          expect(json_response.first['labels']).to eq([label2.title, label.title])
+        end
+
+        it 'returns an array of labeled merge requests where all labels match' do
+          path = endpoint_path + "?labels=#{label.title},foo,bar"
+
+          get api(path, user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(0)
+        end
+
+        it 'returns an empty array if no merge request matches labels' do
+          path = endpoint_path + '?labels=foo,bar'
+
+          get api(path, user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(0)
+        end
+
+        it 'returns an array of labeled merge requests where all labels match' do
+          path = endpoint_path + "?labels[]=#{label.title}&labels[]=#{label2.title}"
+
+          get api(path, user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(1)
+          expect(json_response.first['labels']).to eq([label2.title, label.title])
+        end
+
+        it 'returns an array of merge requests with any label when filtering by any label' do
+          get api(endpoint_path, user), params: { labels: [" #{label.title} ", " #{label2.title} "] }
+
+          expect_paginated_array_response
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(1)
+          expect(json_response.first['labels']).to eq([label2.title, label.title])
+          expect(json_response.first['id']).to eq(merge_request.id)
+        end
+
+        it 'returns an array of merge requests with any label when filtering by any label' do
+          get api(endpoint_path, user), params: { labels: ["#{label.title} , #{label2.title}"] }
+
+          expect_paginated_array_response
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(1)
+          expect(json_response.first['labels']).to eq([label2.title, label.title])
+          expect(json_response.first['id']).to eq(merge_request.id)
+        end
+
+        it 'returns an array of merge requests with any label when filtering by any label' do
+          get api(endpoint_path, user), params: { labels: IssuesFinder::FILTER_ANY }
+
+          expect_paginated_array_response
+          expect(json_response.length).to eq(1)
+          expect(json_response.first['id']).to eq(merge_request.id)
+        end
+
+        it 'returns an array of merge requests without a label when filtering by no label' do
+          get api(endpoint_path, user), params: { labels: IssuesFinder::FILTER_NONE }
+
+          response_ids = json_response.map { |merge_request| merge_request['id'] }
+
+          expect_paginated_array_response
+          expect(response_ids).to contain_exactly(merge_request_closed.id, merge_request_merged.id, merge_request_locked.id)
+        end
+      end
+
+      it 'returns an array of labeled merge requests that are merged for a milestone' do
+        bug_label = create(:label, title: 'bug', color: '#FFAABB', project: project)
+
+        mr1 = create(:merge_request, state: 'merged', source_project: project, target_project: project, milestone: milestone)
+        mr2 = create(:merge_request, state: 'merged', source_project: project, target_project: project, milestone: milestone1)
+        mr3 = create(:merge_request, state: 'closed', source_project: project, target_project: project, milestone: milestone1)
+        _mr = create(:merge_request, state: 'merged', source_project: project, target_project: project, milestone: milestone1)
+
+        create(:label_link, label: bug_label, target: mr1)
+        create(:label_link, label: bug_label, target: mr2)
+        create(:label_link, label: bug_label, target: mr3)
+
+        path = endpoint_path + "?labels=#{bug_label.title}&milestone=#{milestone1.title}&state=merged"
+
+        get api(path, user)
+
+        expect(response).to have_gitlab_http_status(200)
+        expect(json_response).to be_an Array
+        expect(json_response.length).to eq(1)
+        expect(json_response.first['id']).to eq(mr2.id)
+      end
+
+      context 'with ordering' do
+        before do
+          @mr_later = mr_with_later_created_and_updated_at_time
+          @mr_earlier = mr_with_earlier_created_and_updated_at_time
+        end
+
+        it 'returns an array of merge_requests in ascending order' do
+          path = endpoint_path + '?sort=asc'
+
+          get api(path, user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to include_pagination_headers
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(4)
+          response_dates = json_response.map { |merge_request| merge_request['created_at'] }
+          expect(response_dates).to eq(response_dates.sort)
+        end
+
+        it 'returns an array of merge_requests in descending order' do
+          path = endpoint_path + '?sort=desc'
+
+          get api(path, user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to include_pagination_headers
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(4)
+          response_dates = json_response.map { |merge_request| merge_request['created_at'] }
+          expect(response_dates).to eq(response_dates.sort.reverse)
+        end
+
+        context '2 merge requests with equal created_at' do
+          let!(:closed_mr2) do
+            create :merge_request,
+                   state: 'closed',
+                   milestone: milestone1,
+                   author: user,
+                   assignee: user,
+                   source_project: project,
+                   target_project: project,
+                   title: "Test",
+                   created_at: @mr_earlier.created_at
+          end
+
+          it 'page breaks first page correctly' do
+            get api("#{endpoint_path}?sort=desc&per_page=4", user)
+
+            response_ids = json_response.map { |merge_request| merge_request['id'] }
+
+            expect(response_ids).to include(closed_mr2.id)
+            expect(response_ids).not_to include(@mr_earlier.id)
+          end
+
+          it 'page breaks second page correctly' do
+            get api("#{endpoint_path}?sort=desc&per_page=4&page=2", user)
+
+            response_ids = json_response.map { |merge_request| merge_request['id'] }
+
+            expect(response_ids).not_to include(closed_mr2.id)
+            expect(response_ids).to include(@mr_earlier.id)
+          end
+        end
+
+        it 'returns an array of merge_requests ordered by updated_at' do
+          path = endpoint_path + '?order_by=updated_at'
+
+          get api(path, user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to include_pagination_headers
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(4)
+          response_dates = json_response.map { |merge_request| merge_request['updated_at'] }
+          expect(response_dates).to eq(response_dates.sort.reverse)
+        end
+
+        it 'returns an array of merge_requests ordered by created_at' do
+          path = endpoint_path + '?order_by=created_at&sort=asc'
+
+          get api(path, user)
+
+          expect(response).to have_gitlab_http_status(200)
+          expect(response).to include_pagination_headers
+          expect(json_response).to be_an Array
+          expect(json_response.length).to eq(4)
+          response_dates = json_response.map { |merge_request| merge_request['created_at'] }
+          expect(response_dates).to eq(response_dates.sort)
+        end
+      end
+
+      context 'source_branch param' do
+        it 'returns merge requests with the given source branch' do
+          get api(endpoint_path, user), params: { source_branch: merge_request_closed.source_branch, state: 'all' }
+
+          expect_response_contain_exactly(merge_request_closed, merge_request_merged, merge_request_locked)
+        end
+      end
+
+      context 'target_branch param' do
+        it 'returns merge requests with the given target branch' do
+          get api(endpoint_path, user), params: { target_branch: merge_request_closed.target_branch, state: 'all' }
+
+          expect_response_contain_exactly(merge_request_closed, merge_request_merged, merge_request_locked)
+        end
+      end
+    end
+  end
+
   describe 'route shadowing' do
     include GrapePathHelpers::NamedRouteMatcher
 
@@ -356,6 +730,9 @@ describe API::MergeRequests do
 
   describe "GET /projects/:id/merge_requests/:merge_request_iid" do
     it 'exposes known attributes' do
+      create(:award_emoji, :downvote, awardable: merge_request)
+      create(:award_emoji, :upvote, awardable: merge_request)
+
       get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
 
       expect(response).to have_gitlab_http_status(200)
@@ -405,6 +782,8 @@ describe API::MergeRequests do
     end
 
     context 'merge_request_metrics' do
+      let(:pipeline) { create(:ci_empty_pipeline) }
+
       before do
         merge_request.metrics.update!(merged_by: user,
                                       latest_closed_by: user,