Merge branch 'sh-use-wiki-limit-parameter-gitaly' into 'master'

Use limit parameter to retrieve Wikis from Gitaly

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

Gemfile

@@ -422,7 +422,7 @@ group :ed25519 do
 end
 
 # Gitaly GRPC client
-gem 'gitaly-proto', '~> 0.106.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 0.109.0', require: 'gitaly'
 gem 'grpc', '~> 1.11.0'
 
 # Locked until https://github.com/google/protobuf/issues/4210 is closed

Gemfile.lock

@@ -284,7 +284,7 @@ GEM
       gettext_i18n_rails (>= 0.7.1)
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
-    gitaly-proto (0.106.0)
+    gitaly-proto (0.109.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.10)
     github-linguist (5.3.3)
@@ -1042,7 +1042,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly-proto (~> 0.106.0)
+  gitaly-proto (~> 0.109.0)
   github-linguist (~> 5.3.3)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-gollum-lib (~> 4.2)

Gemfile.rails5.lock

@@ -287,7 +287,7 @@ GEM
       gettext_i18n_rails (>= 0.7.1)
       po_to_json (>= 1.0.0)
       rails (>= 3.2.0)
-    gitaly-proto (0.106.0)
+    gitaly-proto (0.109.0)
       google-protobuf (~> 3.1)
       grpc (~> 1.10)
     github-linguist (5.3.3)
@@ -1052,7 +1052,7 @@ DEPENDENCIES
   gettext (~> 3.2.2)
   gettext_i18n_rails (~> 1.8.0)
   gettext_i18n_rails_js (~> 1.3)
-  gitaly-proto (~> 0.106.0)
+  gitaly-proto (~> 0.109.0)
   github-linguist (~> 5.3.3)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-gollum-lib (~> 4.2)

app/controllers/projects/wikis_controller.rb

@@ -112,7 +112,7 @@ class Projects::WikisController < Projects::ApplicationController
   private
 
   def load_project_wiki
-    @project_wiki = ProjectWiki.new(@project, current_user)
+    @project_wiki = load_wiki
 
     # Call #wiki to make sure the Wiki Repo is initialized
     @project_wiki.wiki
@@ -128,6 +128,10 @@ class Projects::WikisController < Projects::ApplicationController
     false
   end
 
+  def load_wiki
+    ProjectWiki.new(@project, current_user)
+  end
+
   def wiki_params
     params.require(:wiki).permit(:title, :content, :format, :message, :last_commit_sha)
   end

app/models/project_wiki.rb

@@ -82,7 +82,7 @@ class ProjectWiki
 
   # Returns an Array of Gitlab WikiPage instances or an
   # empty Array if this Wiki has no pages.
-  def pages(limit: nil)
+  def pages(limit: 0)
     wiki.pages(limit: limit).map { |page| WikiPage.new(self, page, true) }
   end
 

changelogs/unreleased/sh-use-wiki-limit-parameter-gitaly.yml

+---
+title: Use limit parameter to retrieve Wikis from Gitaly
+merge_request: 20764
+author:
+type: performance

lib/gitlab/git/wiki.rb

@@ -44,9 +44,9 @@ module Gitlab
         end
       end
 
-      def pages(limit: nil)
+      def pages(limit: 0)
         @repository.wrapped_gitaly_errors do
-          gitaly_get_all_pages
+          gitaly_get_all_pages(limit: limit)
         end
       end
 
@@ -158,8 +158,8 @@ module Gitlab
         Gitlab::Git::WikiFile.new(wiki_file)
       end
 
-      def gitaly_get_all_pages
-        gitaly_wiki_client.get_all_pages.map do |wiki_page, version|
+      def gitaly_get_all_pages(limit: 0)
+        gitaly_wiki_client.get_all_pages(limit: limit).map do |wiki_page, version|
           Gitlab::Git::WikiPage.new(wiki_page, version)
         end
       end

lib/gitlab/gitaly_client/wiki_service.rb

@@ -85,8 +85,8 @@ module Gitlab
         wiki_page_from_iterator(response)
       end
 
-      def get_all_pages
-        request = Gitaly::WikiGetAllPagesRequest.new(repository: @gitaly_repo)
+      def get_all_pages(limit: 0)
+        request = Gitaly::WikiGetAllPagesRequest.new(repository: @gitaly_repo, limit: limit)
         response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_all_pages, request, timeout: GitalyClient.medium_timeout)
         pages = []
 

spec/controllers/projects/wikis_controller_spec.rb

 require 'spec_helper'
 
 describe Projects::WikisController do
-  let(:project) { create(:project_empty_repo, :public) }
+  let(:project) { create(:project, :public, :repository) }
   let(:user) { create(:user) }
+  let(:wiki) { ProjectWiki.new(project, user) }
+
+  describe 'GET #show' do
+    let(:wiki_title) { 'page-title-test' }
+
+    render_views
+
+    before do
+      create_page(wiki_title, 'hello world')
+    end
+
+    it 'limits the retrieved pages for the sidebar' do
+      sign_in(user)
+
+      expect(controller).to receive(:load_wiki).and_return(wiki)
+
+      # empty? call
+      expect(wiki).to receive(:pages).with(limit: 1).and_call_original
+      # Sidebar entries
+      expect(wiki).to receive(:pages).with(limit: 15).and_call_original
+
+      get :show, namespace_id: project.namespace, project_id: project, id: wiki_title
+
+      expect(response).to have_http_status(:ok)
+      expect(response.body).to include(wiki_title)
+    end
+  end
 
   describe 'POST #preview_markdown' do
     it 'renders json in a correct format' do
@@ -13,4 +40,12 @@ describe Projects::WikisController do
       expect(JSON.parse(response.body).keys).to match_array(%w(body references))
     end
   end
+
+  def create_page(name, content)
+    project.wiki.wiki.write_page(name, :markdown, content, commit_details(name))
+  end
+
+  def commit_details(name)
+    Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.email, "created page #{name}")
+  end
 end

spec/lib/gitlab/git/wiki_spec.rb

@@ -6,6 +6,31 @@ describe Gitlab::Git::Wiki do
   let(:project_wiki) { ProjectWiki.new(project, user) }
   subject { project_wiki.wiki }
 
+  describe '#pages' do
+    before do
+      create_page('page1', 'content')
+      create_page('page2', 'content2')
+    end
+
+    after do
+      destroy_page('page1')
+      destroy_page('page2')
+    end
+
+    it 'returns all the pages' do
+      expect(subject.pages.count).to eq(2)
+      expect(subject.pages.first.title).to eq 'page1'
+      expect(subject.pages.last.title).to eq 'page2'
+    end
+
+    it 'returns only one page' do
+      pages = subject.pages(limit: 1)
+
+      expect(pages.count).to eq(1)
+      expect(pages.first.title).to eq 'page1'
+    end
+  end
+
   describe '#page' do
     before do
       create_page('page1', 'content')

spec/lib/gitlab/gitaly_client/wiki_service_spec.rb

@@ -70,6 +70,15 @@ describe Gitlab::GitalyClient::WikiService do
       subject
     end
 
+    it 'sends a limit of 0 to wiki_get_all_pages' do
+      expect_any_instance_of(Gitaly::WikiService::Stub)
+        .to receive(:wiki_get_all_pages)
+        .with(gitaly_request_with_params(limit: 0), kind_of(Hash))
+        .and_return([].each)
+
+      subject
+    end
+
     it 'concatenates the raw data and returns a pair of WikiPage and WikiPageVersion for each page' do
       expect_any_instance_of(Gitaly::WikiService::Stub)
         .to receive(:wiki_get_all_pages)
@@ -84,5 +93,18 @@ describe Gitlab::GitalyClient::WikiService do
       expect(wiki_page_2.raw_data).to eq('cd')
       expect(wiki_page_2_version.format).to eq('markdown')
     end
+
+    context 'with limits' do
+      subject { client.get_all_pages(limit: 1) }
+
+      it 'sends a request with the limit' do
+        expect_any_instance_of(Gitaly::WikiService::Stub)
+        .to receive(:wiki_get_all_pages)
+        .with(gitaly_request_with_params(limit: 1), kind_of(Hash))
+        .and_return([].each)
+
+        subject
+      end
+    end
   end
 end