Merge branch '65988-optimize-snippet-listings' into 'master'

Optimize queries for snippet listings

See merge request gitlab-org/gitlab-ce!32576

8 files changed, 125 insertions, 89 deletions

diff --git a/spec/controllers/dashboard/snippets_controller_spec.rb b/spec/controllers/dashboard/snippets_controller_spec.rb
new file mode 100644
index 00000000000..2d839094d34
--- /dev/null
+++ b/spec/controllers/dashboard/snippets_controller_spec.rb
@@ -0,0 +1,21 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Dashboard::SnippetsController do
+  let(:user) { create(:user) }
+
+  before do
+    sign_in(user)
+  end
+
+  describe 'GET #index' do
+    it_behaves_like 'paginated collection' do
+      let(:collection) { Snippet.all }
+
+      before do
+        create(:personal_snippet, :public, author: user)
+      end
+    end
+  end
+end
diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb
index 9a3fbfaac51..c5af04f72ee 100644
--- a/spec/controllers/dashboard/todos_controller_spec.rb
+++ b/spec/controllers/dashboard/todos_controller_spec.rb
@@ -82,35 +82,15 @@ describe Dashboard::TodosController do
       end
     end
 
-    context 'when using pagination' do
-      let(:last_page) { user.todos.page.total_pages }
+    it_behaves_like 'paginated collection' do
       let!(:issues) { create_list(:issue, 3, project: project, assignees: [user]) }
+      let(:collection) { user.todos }
 
       before do
         issues.each { |issue| todo_service.new_issue(issue, user) }
         allow(Kaminari.config).to receive(:default_per_page).and_return(2)
       end
 
-      it 'redirects to last_page if page number is larger than number of pages' do
-        get :index, params: { page: (last_page + 1).to_param }
-
-        expect(response).to redirect_to(dashboard_todos_path(page: last_page))
-      end
-
-      it 'goes to the correct page' do
-        get :index, params: { page: last_page }
-
-        expect(assigns(:todos).current_page).to eq(last_page)
-        expect(response).to have_gitlab_http_status(200)
-      end
-
-      it 'does not redirect to external sites when provided a host field' do
-        external_host = "www.example.com"
-        get :index, params: { page: (last_page + 1).to_param, host: external_host }
-
-        expect(response).to redirect_to(dashboard_todos_path(page: last_page))
-      end
-
       context 'when providing no filters' do
         it 'does not perform a query to get the page count, but gets that from the user' do
           allow(controller).to receive(:current_user).and_return(user)
diff --git a/spec/controllers/explore/snippets_controller_spec.rb b/spec/controllers/explore/snippets_controller_spec.rb
new file mode 100644
index 00000000000..fa659c6df7f
--- /dev/null
+++ b/spec/controllers/explore/snippets_controller_spec.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Explore::SnippetsController do
+  describe 'GET #index' do
+    it_behaves_like 'paginated collection' do
+      let(:collection) { Snippet.all }
+
+      before do
+        create(:personal_snippet, :public)
+      end
+    end
+  end
+end
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 608131dcbc8..367bd641f5d 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -71,9 +71,16 @@ describe Projects::IssuesController do
       end
     end
 
-    context 'with page param' do
-      let(:last_page) { project.issues.page.total_pages }
+    it_behaves_like 'paginated collection' do
       let!(:issue_list) { create_list(:issue, 2, project: project) }
+      let(:collection) { project.issues }
+      let(:params) do
+        {
+          namespace_id: project.namespace.to_param,
+          project_id: project,
+          state: 'opened'
+        }
+      end
 
       before do
         sign_in(user)
@@ -81,51 +88,10 @@ describe Projects::IssuesController do
         allow(Kaminari.config).to receive(:default_per_page).and_return(1)
       end
 
-      it 'redirects to last_page if page number is larger than number of pages' do
-        get :index,
-          params: {
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            page: (last_page + 1).to_param
-          }
-
-        expect(response).to redirect_to(namespace_project_issues_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope]))
-      end
-
-      it 'redirects to specified page' do
-        get :index,
-          params: {
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            page: last_page.to_param
-          }
-
-        expect(assigns(:issues).current_page).to eq(last_page)
-        expect(response).to have_gitlab_http_status(200)
-      end
-
-      it 'does not redirect to external sites when provided a host field' do
-        external_host = "www.example.com"
-        get :index,
-          params: {
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            page: (last_page + 1).to_param,
-            host: external_host
-          }
-
-        expect(response).to redirect_to(namespace_project_issues_path(page: last_page, state: controller.params[:state], scope: controller.params[:scope]))
-      end
-
       it 'does not use pagination if disabled' do
         allow(controller).to receive(:pagination_disabled?).and_return(true)
 
-        get :index,
-          params: {
-            namespace_id: project.namespace.to_param,
-            project_id: project,
-            page: (last_page + 1).to_param
-          }
+        get :index, params: params.merge(page: last_page + 1)
 
         expect(response).to have_gitlab_http_status(200)
         expect(assigns(:issues).size).to eq(2)
diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb
index 9b5d7317c11..b13534b9088 100644
--- a/spec/controllers/projects/snippets_controller_spec.rb
+++ b/spec/controllers/projects/snippets_controller_spec.rb
@@ -13,31 +13,17 @@ describe Projects::SnippetsController do
   end
 
   describe 'GET #index' do
-    context 'when page param' do
-      let(:last_page) { project.snippets.page.total_pages }
-      let!(:project_snippet) { create(:project_snippet, :public, project: project, author: user) }
-
-      it 'redirects to last_page if page number is larger than number of pages' do
-        get :index,
-          params: {
-            namespace_id: project.namespace,
-            project_id: project,
-            page: (last_page + 1).to_param
-          }
-
-        expect(response).to redirect_to(namespace_project_snippets_path(page: last_page))
+    it_behaves_like 'paginated collection' do
+      let(:collection) { project.snippets }
+      let(:params) do
+        {
+          namespace_id: project.namespace,
+          project_id: project
+        }
       end
 
-      it 'redirects to specified page' do
-        get :index,
-          params: {
-            namespace_id: project.namespace,
-            project_id: project,
-            page: last_page.to_param
-          }
-
-        expect(assigns(:snippets).current_page).to eq(last_page)
-        expect(response).to have_gitlab_http_status(200)
+      before do
+        create(:project_snippet, :public, project: project, author: user)
       end
     end
 
diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb
index b0092bc8994..1b3a8965342 100644
--- a/spec/controllers/snippets_controller_spec.rb
+++ b/spec/controllers/snippets_controller_spec.rb
@@ -9,6 +9,15 @@ describe SnippetsController do
     let(:user) { create(:user) }
 
     context 'when username parameter is present' do
+      it_behaves_like 'paginated collection' do
+        let(:collection) { Snippet.all }
+        let(:params) { { username: user.username } }
+
+        before do
+          create(:personal_snippet, :public, author: user)
+        end
+      end
+
       it 'renders snippets of a user when username is present' do
         get :index, params: { username: user.username }
 
diff --git a/spec/lib/gitlab/noteable_metadata_spec.rb b/spec/lib/gitlab/noteable_metadata_spec.rb
new file mode 100644
index 00000000000..b12a1825f04
--- /dev/null
+++ b/spec/lib/gitlab/noteable_metadata_spec.rb
@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::NoteableMetadata do
+  subject { Class.new { include Gitlab::NoteableMetadata }.new }
+
+  it 'returns an empty Hash if an empty collection is provided' do
+    expect(subject.noteable_meta_data(Snippet.none, 'Snippet')).to eq({})
+  end
+
+  it 'raises an error when given a collection with no limit' do
+    expect { subject.noteable_meta_data(Snippet.all, 'Snippet') }.to raise_error(/must have a limit/)
+  end
+
+  context 'snippets' do
+    let!(:snippet) { create(:personal_snippet) }
+    let!(:other_snippet) { create(:personal_snippet) }
+    let!(:note) { create(:note, noteable: snippet) }
+
+    it 'aggregates stats on snippets' do
+      data = subject.noteable_meta_data(Snippet.all.limit(10), 'Snippet')
+
+      expect(data.count).to eq(2)
+      expect(data[snippet.id].user_notes_count).to eq(1)
+      expect(data[other_snippet.id].user_notes_count).to eq(0)
+    end
+  end
+end
diff --git a/spec/support/shared_examples/controllers/paginated_collection_shared_examples.rb b/spec/support/shared_examples/controllers/paginated_collection_shared_examples.rb
new file mode 100644
index 00000000000..bd84bd1093f
--- /dev/null
+++ b/spec/support/shared_examples/controllers/paginated_collection_shared_examples.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+shared_examples 'paginated collection' do
+  let(:collection) { nil }
+  let(:last_page) { collection.page.total_pages }
+  let(:action) { :index }
+  let(:params) { {} }
+
+  it 'renders a page number that is not ouf of range' do
+    get action, params: params.merge(page: last_page)
+
+    expect(response).to have_gitlab_http_status(200)
+  end
+
+  it 'redirects to last_page if page number is larger than number of pages' do
+    get action, params: params.merge(page: last_page + 1)
+
+    expect(response).to redirect_to(params.merge(page: last_page))
+  end
+
+  it 'does not redirect to external sites when provided a host field' do
+    external_host = 'www.example.com'
+
+    get action, params: params.merge(page: last_page + 1, host: external_host)
+
+    expect(response).to redirect_to(params.merge(page: last_page))
+  end
+end