summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock4
-rw-r--r--app/controllers/projects/wikis_controller.rb4
-rw-r--r--app/models/project_wiki.rb22
-rw-r--r--app/services/test_hooks/project_service.rb2
-rw-r--r--changelogs/unreleased/fj-53523-add-option-avoid-loading-wiki-page-content.yml5
-rw-r--r--lib/api/wikis.rb3
-rw-r--r--lib/gitlab/git/wiki.rb24
-rw-r--r--lib/gitlab/gitaly_client/wiki_service.rb23
-rw-r--r--spec/controllers/projects/wikis_controller_spec.rb22
-rw-r--r--spec/lib/gitlab/git/wiki_spec.rb16
-rw-r--r--spec/lib/gitlab/gitaly_client/wiki_service_spec.rb6
-rw-r--r--spec/models/project_wiki_spec.rb74
-rw-r--r--spec/models/wiki_page_spec.rb66
14 files changed, 188 insertions, 85 deletions
diff --git a/Gemfile b/Gemfile
index 65ba7137892..a350f194f62 100644
--- a/Gemfile
+++ b/Gemfile
@@ -416,7 +416,7 @@ group :ed25519 do
end
# Gitaly GRPC client
-gem 'gitaly-proto', '~> 1.19.0', require: 'gitaly'
+gem 'gitaly-proto', '~> 1.22.0', require: 'gitaly'
gem 'grpc', '~> 1.19.0'
diff --git a/Gemfile.lock b/Gemfile.lock
index da8f8db9528..64f2f78a4f8 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -280,7 +280,7 @@ GEM
gettext_i18n_rails (>= 0.7.1)
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
- gitaly-proto (1.19.0)
+ gitaly-proto (1.22.0)
grpc (~> 1.0)
github-markup (1.7.0)
gitlab-default_value_for (3.1.1)
@@ -1052,7 +1052,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3)
- gitaly-proto (~> 1.19.0)
+ gitaly-proto (~> 1.22.0)
github-markup (~> 1.7.0)
gitlab-default_value_for (~> 3.1.1)
gitlab-labkit (~> 0.1.2)
diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb
index 88910c91763..fa5bdbc7d49 100644
--- a/app/controllers/projects/wikis_controller.rb
+++ b/app/controllers/projects/wikis_controller.rb
@@ -17,7 +17,7 @@ class Projects::WikisController < Projects::ApplicationController
def pages
@wiki_pages = Kaminari.paginate_array(
- @project_wiki.pages(sort: params[:sort], direction: params[:direction])
+ @project_wiki.list_pages(sort: params[:sort], direction: params[:direction])
).page(params[:page])
@wiki_entries = WikiPage.group_by_directory(@wiki_pages)
@@ -118,7 +118,7 @@ class Projects::WikisController < Projects::ApplicationController
@sidebar_page = @project_wiki.find_sidebar(params[:version_id])
unless @sidebar_page # Fallback to default sidebar
- @sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.pages(limit: 15))
+ @sidebar_wiki_entries = WikiPage.group_by_directory(@project_wiki.list_pages(limit: 15))
end
rescue ProjectWiki::CouldNotCreateWikiError
flash[:notice] = _("Could not create Wiki Repository at this time. Please try again later.")
diff --git a/app/models/project_wiki.rb b/app/models/project_wiki.rb
index 23ddd708396..c91add6439f 100644
--- a/app/models/project_wiki.rb
+++ b/app/models/project_wiki.rb
@@ -82,17 +82,25 @@ class ProjectWiki
end
def empty?
- pages(limit: 1).empty?
+ list_pages(limit: 1).empty?
end
+ # Lists wiki pages of the repository.
+ #
+ # limit - max number of pages returned by the method.
+ # sort - criterion by which the pages are sorted.
+ # direction - order of the sorted pages.
+ # load_content - option, which specifies whether the content inside the page
+ # will be loaded.
+ #
# Returns an Array of GitLab WikiPage instances or an
# empty Array if this Wiki has no pages.
- def pages(limit: 0, sort: nil, direction: DIRECTION_ASC)
- sort ||= TITLE_ORDER
- direction_desc = direction == DIRECTION_DESC
-
- wiki.pages(
- limit: limit, sort: sort, direction_desc: direction_desc
+ def list_pages(limit: 0, sort: nil, direction: DIRECTION_ASC, load_content: false)
+ wiki.list_pages(
+ limit: limit,
+ sort: sort,
+ direction_desc: direction == DIRECTION_DESC,
+ load_content: load_content
).map do |page|
WikiPage.new(self, page, true)
end
diff --git a/app/services/test_hooks/project_service.rb b/app/services/test_hooks/project_service.rb
index 6607f5b2418..a71278e8b8b 100644
--- a/app/services/test_hooks/project_service.rb
+++ b/app/services/test_hooks/project_service.rb
@@ -56,7 +56,7 @@ module TestHooks
end
def wiki_page_events_data
- page = project.wiki.pages.first
+ page = project.wiki.list_pages(limit: 1).first
if !project.wiki_enabled? || page.blank?
throw(:validation_error, s_('TestHooks|Ensure the wiki is enabled and has pages.'))
end
diff --git a/changelogs/unreleased/fj-53523-add-option-avoid-loading-wiki-page-content.yml b/changelogs/unreleased/fj-53523-add-option-avoid-loading-wiki-page-content.yml
new file mode 100644
index 00000000000..49eaff52e5a
--- /dev/null
+++ b/changelogs/unreleased/fj-53523-add-option-avoid-loading-wiki-page-content.yml
@@ -0,0 +1,5 @@
+---
+title: Added list_pages method to avoid loading all wiki pages content
+merge_request: 22801
+author:
+type: performance
diff --git a/lib/api/wikis.rb b/lib/api/wikis.rb
index 994074ddc67..5724adb2c40 100644
--- a/lib/api/wikis.rb
+++ b/lib/api/wikis.rb
@@ -33,7 +33,8 @@ module API
authorize! :read_wiki, user_project
entity = params[:with_content] ? Entities::WikiPage : Entities::WikiPageBasic
- present user_project.wiki.pages, with: entity
+
+ present user_project.wiki.list_pages(load_content: params[:with_content]), with: entity
end
desc 'Get a wiki page' do
diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb
index a0dd4a24363..c1bcd8e934a 100644
--- a/lib/gitlab/git/wiki.rb
+++ b/lib/gitlab/git/wiki.rb
@@ -86,9 +86,14 @@ module Gitlab
end
end
- def pages(limit: 0, sort: nil, direction_desc: false)
+ def list_pages(limit: 0, sort: nil, direction_desc: false, load_content: false)
wrapped_gitaly_errors do
- gitaly_get_all_pages(limit: limit, sort: sort, direction_desc: direction_desc)
+ gitaly_list_pages(
+ limit: limit,
+ sort: sort,
+ direction_desc: direction_desc,
+ load_content: load_content
+ )
end
end
@@ -168,10 +173,17 @@ module Gitlab
Gitlab::Git::WikiFile.new(wiki_file)
end
- def gitaly_get_all_pages(limit: 0, sort: nil, direction_desc: false)
- gitaly_wiki_client.get_all_pages(
- limit: limit, sort: sort, direction_desc: direction_desc
- ).map do |wiki_page, version|
+ def gitaly_list_pages(limit: 0, sort: nil, direction_desc: false, load_content: false)
+ params = { limit: limit, sort: sort, direction_desc: direction_desc }
+
+ gitaly_pages =
+ if load_content
+ gitaly_wiki_client.load_all_pages(params)
+ else
+ gitaly_wiki_client.list_all_pages(params)
+ end
+
+ gitaly_pages.map do |wiki_page, version|
Gitlab::Git::WikiPage.new(wiki_page, version)
end
end
diff --git a/lib/gitlab/gitaly_client/wiki_service.rb b/lib/gitlab/gitaly_client/wiki_service.rb
index e036cdcd800..ce9faad825c 100644
--- a/lib/gitlab/gitaly_client/wiki_service.rb
+++ b/lib/gitlab/gitaly_client/wiki_service.rb
@@ -87,7 +87,27 @@ module Gitlab
wiki_page_from_iterator(response)
end
- def get_all_pages(limit: 0, sort: nil, direction_desc: false)
+ def list_all_pages(limit: 0, sort: nil, direction_desc: false)
+ sort_value = Gitaly::WikiListPagesRequest::SortBy.resolve(sort.to_s.upcase.to_sym)
+
+ params = { repository: @gitaly_repo, limit: limit, direction_desc: direction_desc }
+ params[:sort] = sort_value if sort_value
+
+ request = Gitaly::WikiListPagesRequest.new(params)
+ stream = GitalyClient.call(@repository.storage, :wiki_service, :wiki_list_pages, request, timeout: GitalyClient.medium_timeout)
+ stream.each_with_object([]) do |message, pages|
+ page = message.page
+
+ next unless page
+
+ wiki_page = GitalyClient::WikiPage.new(page.to_h)
+ version = new_wiki_page_version(page.version)
+
+ pages << [wiki_page, version]
+ end
+ end
+
+ def load_all_pages(limit: 0, sort: nil, direction_desc: false)
sort_value = Gitaly::WikiGetAllPagesRequest::SortBy.resolve(sort.to_s.upcase.to_sym)
params = { repository: @gitaly_repo, limit: limit, direction_desc: direction_desc }
@@ -95,6 +115,7 @@ module Gitlab
request = Gitaly::WikiGetAllPagesRequest.new(params)
response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_all_pages, request, timeout: GitalyClient.medium_timeout)
+
pages = []
loop do
diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb
index e0a6fc52ee9..f2e0b5e5c1d 100644
--- a/spec/controllers/projects/wikis_controller_spec.rb
+++ b/spec/controllers/projects/wikis_controller_spec.rb
@@ -19,6 +19,18 @@ describe Projects::WikisController do
destroy_page(wiki_title)
end
+ describe 'GET #pages' do
+ subject { get :pages, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } }
+
+ it 'does not load the pages content' do
+ expect(controller).to receive(:load_wiki).and_return(project_wiki)
+
+ expect(project_wiki).to receive(:list_pages).twice.and_call_original
+
+ subject
+ end
+ end
+
describe 'GET #show' do
render_views
@@ -28,9 +40,9 @@ describe Projects::WikisController do
expect(controller).to receive(:load_wiki).and_return(project_wiki)
# empty? call
- expect(project_wiki).to receive(:pages).with(limit: 1).and_call_original
+ expect(project_wiki).to receive(:list_pages).with(limit: 1).and_call_original
# Sidebar entries
- expect(project_wiki).to receive(:pages).with(limit: 15).and_call_original
+ expect(project_wiki).to receive(:list_pages).with(limit: 15).and_call_original
subject
@@ -104,7 +116,7 @@ describe Projects::WikisController do
subject
- expect(response).to redirect_to(project_wiki_path(project, project_wiki.pages.first))
+ expect(response).to redirect_to(project_wiki_path(project, project_wiki.list_pages.first))
end
end
@@ -138,7 +150,7 @@ describe Projects::WikisController do
allow(controller).to receive(:valid_encoding?).and_return(false)
subject
- expect(response).to redirect_to(project_wiki_path(project, project_wiki.pages.first))
+ expect(response).to redirect_to(project_wiki_path(project, project_wiki.list_pages.first))
end
end
@@ -148,7 +160,7 @@ describe Projects::WikisController do
it 'updates the page' do
subject
- wiki_page = project_wiki.pages.first
+ wiki_page = project_wiki.list_pages(load_content: true).first
expect(wiki_page.title).to eq new_title
expect(wiki_page.content).to eq new_content
diff --git a/spec/lib/gitlab/git/wiki_spec.rb b/spec/lib/gitlab/git/wiki_spec.rb
index ded5d7576df..1e577392949 100644
--- a/spec/lib/gitlab/git/wiki_spec.rb
+++ b/spec/lib/gitlab/git/wiki_spec.rb
@@ -21,13 +21,13 @@ describe Gitlab::Git::Wiki do
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'
+ expect(subject.list_pages.count).to eq(2)
+ expect(subject.list_pages.first.title).to eq 'page1'
+ expect(subject.list_pages.last.title).to eq 'page2'
end
it 'returns only one page' do
- pages = subject.pages(limit: 1)
+ pages = subject.list_pages(limit: 1)
expect(pages.count).to eq(1)
expect(pages.first.title).to eq 'page1'
@@ -62,8 +62,8 @@ describe Gitlab::Git::Wiki do
subject.delete_page('*', commit_details('whatever'))
- expect(subject.pages.count).to eq 1
- expect(subject.pages.first.title).to eq 'page1'
+ expect(subject.list_pages.count).to eq 1
+ expect(subject.list_pages.first.title).to eq 'page1'
end
end
@@ -87,7 +87,7 @@ describe Gitlab::Git::Wiki do
with_them do
subject { wiki.preview_slug(title, format) }
- let(:gitaly_slug) { wiki.pages.first }
+ let(:gitaly_slug) { wiki.list_pages.first }
it { is_expected.to eq(expected_slug) }
@@ -96,7 +96,7 @@ describe Gitlab::Git::Wiki do
create_page(title, 'content', format: format)
- gitaly_slug = wiki.pages.first.url_path
+ gitaly_slug = wiki.list_pages.first.url_path
is_expected.to eq(gitaly_slug)
end
diff --git a/spec/lib/gitlab/gitaly_client/wiki_service_spec.rb b/spec/lib/gitlab/gitaly_client/wiki_service_spec.rb
index d82c9c28da0..4fa8e97aca0 100644
--- a/spec/lib/gitlab/gitaly_client/wiki_service_spec.rb
+++ b/spec/lib/gitlab/gitaly_client/wiki_service_spec.rb
@@ -46,7 +46,7 @@ describe Gitlab::GitalyClient::WikiService do
end
end
- describe '#get_all_pages' do
+ describe '#load_all_pages' do
let(:page_2_info) { { title: 'My Page 2', raw_data: 'c', version: page_version } }
let(:response) do
[
@@ -63,7 +63,7 @@ describe Gitlab::GitalyClient::WikiService do
let(:wiki_page_2) { subject[1].first }
let(:wiki_page_2_version) { subject[1].last }
- subject { client.get_all_pages }
+ subject { client.load_all_pages }
it 'sends a wiki_get_all_pages message' do
expect_any_instance_of(Gitaly::WikiService::Stub)
@@ -99,7 +99,7 @@ describe Gitlab::GitalyClient::WikiService do
end
context 'with limits' do
- subject { client.get_all_pages(limit: 1) }
+ subject { client.load_all_pages(limit: 1) }
it 'sends a request with the limit' do
expect_any_instance_of(Gitaly::WikiService::Stub)
diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb
index 2525a6aebe0..d12dd97bb9e 100644
--- a/spec/models/project_wiki_spec.rb
+++ b/spec/models/project_wiki_spec.rb
@@ -109,8 +109,7 @@ describe ProjectWiki do
subject { super().empty? }
it { is_expected.to be_falsey }
- # Re-enable this when https://gitlab.com/gitlab-org/gitaly/issues/1204 is fixed
- xit 'only instantiates a Wiki page once' do
+ it 'only instantiates a Wiki page once' do
expect(WikiPage).to receive(:new).once.and_call_original
subject
@@ -119,22 +118,65 @@ describe ProjectWiki do
end
end
- describe "#pages" do
+ describe "#list_pages" do
+ let(:wiki_pages) { subject.list_pages }
+
before do
- create_page("index", "This is an awesome new Gollum Wiki")
- @pages = subject.pages
+ create_page("index", "This is an index")
+ create_page("index2", "This is an index2")
+ create_page("an index3", "This is an index3")
end
after do
- destroy_page(@pages.first.page)
+ wiki_pages.each do |wiki_page|
+ destroy_page(wiki_page.page)
+ end
end
it "returns an array of WikiPage instances" do
- expect(@pages.first).to be_a WikiPage
+ expect(wiki_pages.first).to be_a WikiPage
+ end
+
+ it 'does not load WikiPage content by default' do
+ wiki_pages.each do |page|
+ expect(page.content).to be_empty
+ end
+ end
+
+ it 'returns all pages by default' do
+ expect(wiki_pages.count).to eq(3)
+ end
+
+ context "with limit option" do
+ it 'returns limited set of pages' do
+ expect(subject.list_pages(limit: 1).count).to eq(1)
+ end
end
- it "returns the correct number of pages" do
- expect(@pages.count).to eq(1)
+ context "with sorting options" do
+ it 'returns pages sorted by title by default' do
+ pages = ['an index3', 'index', 'index2']
+
+ expect(subject.list_pages.map(&:title)).to eq(pages)
+ expect(subject.list_pages(direction: "desc").map(&:title)).to eq(pages.reverse)
+ end
+
+ it 'returns pages sorted by created_at' do
+ pages = ['index', 'index2', 'an index3']
+
+ expect(subject.list_pages(sort: 'created_at').map(&:title)).to eq(pages)
+ expect(subject.list_pages(sort: 'created_at', direction: "desc").map(&:title)).to eq(pages.reverse)
+ end
+ end
+
+ context "with load_content option" do
+ let(:pages) { subject.list_pages(load_content: true) }
+
+ it 'loads WikiPage content' do
+ expect(pages.first.content).to eq("This is an index3")
+ expect(pages.second.content).to eq("This is an index")
+ expect(pages.third.content).to eq("This is an index2")
+ end
end
end
@@ -144,7 +186,7 @@ describe ProjectWiki do
end
after do
- subject.pages.each { |page| destroy_page(page.page) }
+ subject.list_pages.each { |page| destroy_page(page.page) }
end
it "returns the latest version of the page if it exists" do
@@ -195,7 +237,7 @@ describe ProjectWiki do
end
after do
- subject.pages.each { |page| destroy_page(page.page) }
+ subject.list_pages.each { |page| destroy_page(page.page) }
end
it 'finds the page defined as _sidebar' do
@@ -242,12 +284,12 @@ describe ProjectWiki do
describe "#create_page" do
after do
- destroy_page(subject.pages.first.page)
+ destroy_page(subject.list_pages.first.page)
end
it "creates a new wiki page" do
expect(subject.create_page("test page", "this is content")).not_to eq(false)
- expect(subject.pages.count).to eq(1)
+ expect(subject.list_pages.count).to eq(1)
end
it "returns false when a duplicate page exists" do
@@ -262,7 +304,7 @@ describe ProjectWiki do
it "sets the correct commit message" do
subject.create_page("test page", "some content", :markdown, "commit message")
- expect(subject.pages.first.page.version.message).to eq("commit message")
+ expect(subject.list_pages.first.page.version.message).to eq("commit message")
end
it 'sets the correct commit email' do
@@ -293,7 +335,7 @@ describe ProjectWiki do
format: :markdown,
message: "updated page"
)
- @page = subject.pages.first.page
+ @page = subject.list_pages(load_content: true).first.page
end
after do
@@ -337,7 +379,7 @@ describe ProjectWiki do
it "deletes the page" do
subject.delete_page(@page)
- expect(subject.pages.count).to eq(0)
+ expect(subject.list_pages.count).to eq(0)
end
it 'sets the correct commit email' do
diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb
index d5c85c11195..520a06e138e 100644
--- a/spec/models/wiki_page_spec.rb
+++ b/spec/models/wiki_page_spec.rb
@@ -44,47 +44,49 @@ describe WikiPage do
WikiDirectory.new('dir_2', pages)
end
- context 'sort by title' do
- let(:grouped_entries) { described_class.group_by_directory(wiki.pages) }
- let(:expected_grouped_entries) { [dir_1_1, dir_1, page_dir_2, dir_2, page_1, page_6] }
-
- it 'returns an array with pages and directories' do
- grouped_entries.each_with_index do |page_or_dir, i|
- expected_page_or_dir = expected_grouped_entries[i]
- expected_slugs = get_slugs(expected_page_or_dir)
- slugs = get_slugs(page_or_dir)
-
- expect(slugs).to match_array(expected_slugs)
+ context "#list_pages" do
+ context 'sort by title' do
+ let(:grouped_entries) { described_class.group_by_directory(wiki.list_pages) }
+ let(:expected_grouped_entries) { [dir_1_1, dir_1, page_dir_2, dir_2, page_1, page_6] }
+
+ it 'returns an array with pages and directories' do
+ grouped_entries.each_with_index do |page_or_dir, i|
+ expected_page_or_dir = expected_grouped_entries[i]
+ expected_slugs = get_slugs(expected_page_or_dir)
+ slugs = get_slugs(page_or_dir)
+
+ expect(slugs).to match_array(expected_slugs)
+ end
end
end
- end
- context 'sort by created_at' do
- let(:grouped_entries) { described_class.group_by_directory(wiki.pages(sort: 'created_at')) }
- let(:expected_grouped_entries) { [dir_1_1, page_1, dir_1, page_dir_2, dir_2, page_6] }
+ context 'sort by created_at' do
+ let(:grouped_entries) { described_class.group_by_directory(wiki.list_pages(sort: 'created_at')) }
+ let(:expected_grouped_entries) { [dir_1_1, page_1, dir_1, page_dir_2, dir_2, page_6] }
- it 'returns an array with pages and directories' do
- grouped_entries.each_with_index do |page_or_dir, i|
- expected_page_or_dir = expected_grouped_entries[i]
- expected_slugs = get_slugs(expected_page_or_dir)
- slugs = get_slugs(page_or_dir)
+ it 'returns an array with pages and directories' do
+ grouped_entries.each_with_index do |page_or_dir, i|
+ expected_page_or_dir = expected_grouped_entries[i]
+ expected_slugs = get_slugs(expected_page_or_dir)
+ slugs = get_slugs(page_or_dir)
- expect(slugs).to match_array(expected_slugs)
+ expect(slugs).to match_array(expected_slugs)
+ end
end
end
- end
- it 'returns an array with retained order with directories at the top' do
- expected_order = ['dir_1/dir_1_1/page_3', 'dir_1/page_2', 'dir_2', 'dir_2/page_4', 'dir_2/page_5', 'page_1', 'page_6']
+ it 'returns an array with retained order with directories at the top' do
+ expected_order = ['dir_1/dir_1_1/page_3', 'dir_1/page_2', 'dir_2', 'dir_2/page_4', 'dir_2/page_5', 'page_1', 'page_6']
- grouped_entries = described_class.group_by_directory(wiki.pages)
+ grouped_entries = described_class.group_by_directory(wiki.list_pages)
- actual_order =
- grouped_entries.map do |page_or_dir|
- get_slugs(page_or_dir)
- end
- .flatten
- expect(actual_order).to eq(expected_order)
+ actual_order =
+ grouped_entries.map do |page_or_dir|
+ get_slugs(page_or_dir)
+ end
+ .flatten
+ expect(actual_order).to eq(expected_order)
+ end
end
end
end
@@ -386,7 +388,7 @@ describe WikiPage do
it "deletes the page" do
@page.delete
- expect(wiki.pages).to be_empty
+ expect(wiki.list_pages).to be_empty
end
it "returns true" do