Commit 43fbf8fd authored by GitLab Bot's avatar GitLab Bot

Automatic merge of gitlab-org/gitlab-ce master

parents 24492a99 f3b3d8b1
...@@ -434,7 +434,7 @@ group :ed25519 do ...@@ -434,7 +434,7 @@ group :ed25519 do
end end
# Gitaly GRPC client # Gitaly GRPC client
gem 'gitaly-proto', '~> 1.19.0', require: 'gitaly' gem 'gitaly-proto', '~> 1.22.0', require: 'gitaly'
gem 'grpc', '~> 1.19.0' gem 'grpc', '~> 1.19.0'
......
...@@ -305,7 +305,7 @@ GEM ...@@ -305,7 +305,7 @@ GEM
gettext_i18n_rails (>= 0.7.1) gettext_i18n_rails (>= 0.7.1)
po_to_json (>= 1.0.0) po_to_json (>= 1.0.0)
rails (>= 3.2.0) rails (>= 3.2.0)
gitaly-proto (1.19.0) gitaly-proto (1.22.0)
grpc (~> 1.0) grpc (~> 1.0)
github-markup (1.7.0) github-markup (1.7.0)
gitlab-default_value_for (3.1.1) gitlab-default_value_for (3.1.1)
...@@ -1090,7 +1090,7 @@ DEPENDENCIES ...@@ -1090,7 +1090,7 @@ DEPENDENCIES
gettext (~> 3.2.2) gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3) gettext_i18n_rails_js (~> 1.3)
gitaly-proto (~> 1.19.0) gitaly-proto (~> 1.22.0)
github-markup (~> 1.7.0) github-markup (~> 1.7.0)
gitlab-default_value_for (~> 3.1.1) gitlab-default_value_for (~> 3.1.1)
gitlab-labkit (~> 0.1.2) gitlab-labkit (~> 0.1.2)
......
...@@ -17,7 +17,7 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -17,7 +17,7 @@ class Projects::WikisController < Projects::ApplicationController
def pages def pages
@wiki_pages = Kaminari.paginate_array( @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]) ).page(params[:page])
@wiki_entries = WikiPage.group_by_directory(@wiki_pages) @wiki_entries = WikiPage.group_by_directory(@wiki_pages)
...@@ -118,7 +118,7 @@ class Projects::WikisController < Projects::ApplicationController ...@@ -118,7 +118,7 @@ class Projects::WikisController < Projects::ApplicationController
@sidebar_page = @project_wiki.find_sidebar(params[:version_id]) @sidebar_page = @project_wiki.find_sidebar(params[:version_id])
unless @sidebar_page # Fallback to default sidebar 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 end
rescue ProjectWiki::CouldNotCreateWikiError rescue ProjectWiki::CouldNotCreateWikiError
flash[:notice] = _("Could not create Wiki Repository at this time. Please try again later.") flash[:notice] = _("Could not create Wiki Repository at this time. Please try again later.")
......
...@@ -82,17 +82,25 @@ class ProjectWiki ...@@ -82,17 +82,25 @@ class ProjectWiki
end end
def empty? def empty?
pages(limit: 1).empty? list_pages(limit: 1).empty?
end 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 # Returns an Array of GitLab WikiPage instances or an
# empty Array if this Wiki has no pages. # empty Array if this Wiki has no pages.
def pages(limit: 0, sort: nil, direction: DIRECTION_ASC) def list_pages(limit: 0, sort: nil, direction: DIRECTION_ASC, load_content: false)
sort ||= TITLE_ORDER wiki.list_pages(
direction_desc = direction == DIRECTION_DESC limit: limit,
sort: sort,
wiki.pages( direction_desc: direction == DIRECTION_DESC,
limit: limit, sort: sort, direction_desc: direction_desc load_content: load_content
).map do |page| ).map do |page|
WikiPage.new(self, page, true) WikiPage.new(self, page, true)
end end
......
...@@ -56,7 +56,7 @@ module TestHooks ...@@ -56,7 +56,7 @@ module TestHooks
end end
def wiki_page_events_data def wiki_page_events_data
page = project.wiki.pages.first page = project.wiki.list_pages(limit: 1).first
if !project.wiki_enabled? || page.blank? if !project.wiki_enabled? || page.blank?
throw(:validation_error, s_('TestHooks|Ensure the wiki is enabled and has pages.')) throw(:validation_error, s_('TestHooks|Ensure the wiki is enabled and has pages.'))
end end
......
---
title: Added list_pages method to avoid loading all wiki pages content
merge_request: 22801
author:
type: performance
...@@ -33,7 +33,8 @@ module API ...@@ -33,7 +33,8 @@ module API
authorize! :read_wiki, user_project authorize! :read_wiki, user_project
entity = params[:with_content] ? Entities::WikiPage : Entities::WikiPageBasic 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 end
desc 'Get a wiki page' do desc 'Get a wiki page' do
......
...@@ -86,9 +86,14 @@ module Gitlab ...@@ -86,9 +86,14 @@ module Gitlab
end end
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 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
end end
...@@ -168,10 +173,17 @@ module Gitlab ...@@ -168,10 +173,17 @@ module Gitlab
Gitlab::Git::WikiFile.new(wiki_file) Gitlab::Git::WikiFile.new(wiki_file)
end end
def gitaly_get_all_pages(limit: 0, sort: nil, direction_desc: false) def gitaly_list_pages(limit: 0, sort: nil, direction_desc: false, load_content: false)
gitaly_wiki_client.get_all_pages( params = { limit: limit, sort: sort, direction_desc: direction_desc }
limit: limit, sort: sort, direction_desc: direction_desc
).map do |wiki_page, version| 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) Gitlab::Git::WikiPage.new(wiki_page, version)
end end
end end
......
...@@ -87,7 +87,27 @@ module Gitlab ...@@ -87,7 +87,27 @@ module Gitlab
wiki_page_from_iterator(response) wiki_page_from_iterator(response)
end 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) sort_value = Gitaly::WikiGetAllPagesRequest::SortBy.resolve(sort.to_s.upcase.to_sym)
params = { repository: @gitaly_repo, limit: limit, direction_desc: direction_desc } params = { repository: @gitaly_repo, limit: limit, direction_desc: direction_desc }
...@@ -95,6 +115,7 @@ module Gitlab ...@@ -95,6 +115,7 @@ module Gitlab
request = Gitaly::WikiGetAllPagesRequest.new(params) request = Gitaly::WikiGetAllPagesRequest.new(params)
response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_all_pages, request, timeout: GitalyClient.medium_timeout) response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_all_pages, request, timeout: GitalyClient.medium_timeout)
pages = [] pages = []
loop do loop do
......
...@@ -19,6 +19,18 @@ describe Projects::WikisController do ...@@ -19,6 +19,18 @@ describe Projects::WikisController do
destroy_page(wiki_title) destroy_page(wiki_title)
end 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 describe 'GET #show' do
render_views render_views
...@@ -28,9 +40,9 @@ describe Projects::WikisController do ...@@ -28,9 +40,9 @@ describe Projects::WikisController do
expect(controller).to receive(:load_wiki).and_return(project_wiki) expect(controller).to receive(:load_wiki).and_return(project_wiki)
# empty? call # 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 # 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 subject
...@@ -104,7 +116,7 @@ describe Projects::WikisController do ...@@ -104,7 +116,7 @@ describe Projects::WikisController do
subject 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
end end
...@@ -138,7 +150,7 @@ describe Projects::WikisController do ...@@ -138,7 +150,7 @@ describe Projects::WikisController do
allow(controller).to receive(:valid_encoding?).and_return(false) allow(controller).to receive(:valid_encoding?).and_return(false)
subject 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
end end
...@@ -148,7 +160,7 @@ describe Projects::WikisController do ...@@ -148,7 +160,7 @@ describe Projects::WikisController do
it 'updates the page' do it 'updates the page' do
subject 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.title).to eq new_title
expect(wiki_page.content).to eq new_content expect(wiki_page.content).to eq new_content
......
...@@ -21,13 +21,13 @@ describe Gitlab::Git::Wiki do ...@@ -21,13 +21,13 @@ describe Gitlab::Git::Wiki do
end end
it 'returns all the pages' do it 'returns all the pages' do
expect(subject.pages.count).to eq(2) expect(subject.list_pages.count).to eq(2)
expect(subject.pages.first.title).to eq 'page1' expect(subject.list_pages.first.title).to eq 'page1'
expect(subject.pages.last.title).to eq 'page2' expect(subject.list_pages.last.title).to eq 'page2'
end end
it 'returns only one page' do 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.count).to eq(1)
expect(pages.first.title).to eq 'page1' expect(pages.first.title).to eq 'page1'
...@@ -62,8 +62,8 @@ describe Gitlab::Git::Wiki do ...@@ -62,8 +62,8 @@ describe Gitlab::Git::Wiki do
subject.delete_page('*', commit_details('whatever')) subject.delete_page('*', commit_details('whatever'))
expect(subject.pages.count).to eq 1 expect(subject.list_pages.count).to eq 1
expect(subject.pages.first.title).to eq 'page1' expect(subject.list_pages.first.title).to eq 'page1'
end end
end end
...@@ -87,7 +87,7 @@ describe Gitlab::Git::Wiki do ...@@ -87,7 +87,7 @@ describe Gitlab::Git::Wiki do
with_them do with_them do
subject { wiki.preview_slug(title, format) } 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) } it { is_expected.to eq(expected_slug) }
...@@ -96,7 +96,7 @@ describe Gitlab::Git::Wiki do ...@@ -96,7 +96,7 @@ describe Gitlab::Git::Wiki do
create_page(title, 'content', format: format) 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) is_expected.to eq(gitaly_slug)
end end
......
...@@ -46,7 +46,7 @@ describe Gitlab::GitalyClient::WikiService do ...@@ -46,7 +46,7 @@ describe Gitlab::GitalyClient::WikiService do
end end
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(:page_2_info) { { title: 'My Page 2', raw_data: 'c', version: page_version } }
let(:response) do let(:response) do
[ [
...@@ -63,7 +63,7 @@ describe Gitlab::GitalyClient::WikiService do ...@@ -63,7 +63,7 @@ describe Gitlab::GitalyClient::WikiService do
let(:wiki_page_2) { subject[1].first } let(:wiki_page_2) { subject[1].first }
let(:wiki_page_2_version) { subject[1].last } 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 it 'sends a wiki_get_all_pages message' do
expect_any_instance_of(Gitaly::WikiService::Stub) expect_any_instance_of(Gitaly::WikiService::Stub)
...@@ -99,7 +99,7 @@ describe Gitlab::GitalyClient::WikiService do ...@@ -99,7 +99,7 @@ describe Gitlab::GitalyClient::WikiService do
end end
context 'with limits' do 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 it 'sends a request with the limit' do
expect_any_instance_of(Gitaly::WikiService::Stub) expect_any_instance_of(Gitaly::WikiService::Stub)
......
...@@ -109,8 +109,7 @@ describe ProjectWiki do ...@@ -109,8 +109,7 @@ describe ProjectWiki do
subject { super().empty? } subject { super().empty? }
it { is_expected.to be_falsey } it { is_expected.to be_falsey }
# Re-enable this when https://gitlab.com/gitlab-org/gitaly/issues/1204 is fixed it 'only instantiates a Wiki page once' do
xit 'only instantiates a Wiki page once' do
expect(WikiPage).to receive(:new).once.and_call_original expect(WikiPage).to receive(:new).once.and_call_original
subject subject
...@@ -119,22 +118,65 @@ describe ProjectWiki do ...@@ -119,22 +118,65 @@ describe ProjectWiki do
end end
end end
describe "#pages" do describe "#list_pages" do
let(:wiki_pages) { subject.list_pages }
before do before do
create_page("index", "This is an awesome new Gollum Wiki") create_page("index", "This is an index")
@pages = subject.pages create_page("index2", "This is an index2")
create_page("an index3", "This is an index3")
end end
after do after do
destroy_page(@pages.first.page) wiki_pages.each do |wiki_page|
destroy_page(wiki_page.page)
end
end end
it "returns an array of WikiPage instances" do 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 end
it "returns the correct number of pages" do context "with sorting options" do
expect(@pages.count).to eq(1) 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
end end
...@@ -144,7 +186,7 @@ describe ProjectWiki do ...@@ -144,7 +186,7 @@ describe ProjectWiki do
end end
after do after do
subject.pages.each { |page| destroy_page(page.page) } subject.list_pages.each { |page| destroy_page(page.page) }
end end
it "returns the latest version of the page if it exists" do it "returns the latest version of the page if it exists" do
...@@ -195,7 +237,7 @@ describe ProjectWiki do ...@@ -195,7 +237,7 @@ describe ProjectWiki do
end end
after do after do
subject.pages.each { |page| destroy_page(page.page) } subject.list_pages.each { |page| destroy_page(page.page) }
end end
it 'finds the page defined as _sidebar' do it 'finds the page defined as _sidebar' do
...@@ -242,12 +284,12 @@ describe ProjectWiki do ...@@ -242,12 +284,12 @@ describe ProjectWiki do
describe "#create_page" do describe "#create_page" do
after do after do
destroy_page(subject.pages.first.page) destroy_page(subject.list_pages.first.page)
end end
it "creates a new wiki page" do it "creates a new wiki page" do
expect(subject.create_page("test page", "this is content")).not_to eq(false) 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 end
it "returns false when a duplicate page exists" do it "returns false when a duplicate page exists" do
...@@ -262,7 +304,7 @@ describe ProjectWiki do ...@@ -262,7 +304,7 @@ describe ProjectWiki do
it "sets the correct commit message" do it "sets the correct commit message" do
subject.create_page("test page", "some content", :markdown, "commit message") 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 end
it 'sets the correct commit email' do it 'sets the correct commit email' do
...@@ -293,7 +335,7 @@ describe ProjectWiki do ...@@ -293,7 +335,7 @@ describe ProjectWiki do
format: :markdown, format: :markdown,
message: "updated page" message: "updated page"
) )
@page = subject.pages.first.page @page = subject.list_pages(load_content: true).first.page
end end
after do after do
...@@ -337,7 +379,7 @@ describe ProjectWiki do ...@@ -337,7 +379,7 @@ describe ProjectWiki do
it "deletes the page" do it "deletes the page" do
subject.delete_page(@page) subject.delete_page(@page)
expect(subject.pages.count).to eq(0) expect(subject.list_pages.count).to eq(0)
end end
it 'sets the correct commit email' do it 'sets the correct commit email' do
......
...@@ -44,47 +44,49 @@ describe WikiPage do ...@@ -44,47 +44,49 @@ describe WikiPage do
WikiDirectory.new('dir_2', pages) WikiDirectory.new('dir_2', pages)
end end
context 'sort by title' do context "#list_pages" do
let(:grouped_entries) { described_class.group_by_directory(wiki.pages) } context 'sort by title' do
let(:expected_grouped_entries) { [dir_1_1, dir_1, page_dir_2, dir_2, page_1, page_6] } 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| it 'returns an array with pages and directories' do
expected_page_or_dir = expected_grouped_entries[i] grouped_entries.each_with_index do |page_or_dir, i|
expected_slugs = get_slugs(expected_page_or_dir) expected_page_or_dir = expected_grouped_entries[i]
slugs = get_slugs(page_or_dir) 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 end
end
context 'sort by created_at' do context 'sort by created_at' do
let(:grouped_entries) { described_class.group_by_directory(wiki.pages(sort: 'created_at')) } 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] } 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 it 'returns an array with pages and directories' do
grouped_entries.each_with_index do |page_or_dir, i| grouped_entries.each_with_index do |page_or_dir, i|
expected_page_or_dir = expected_grouped_entries[i] expected_page_or_dir = expected_grouped_entries[i]
expected_slugs = get_slugs(expected_page_or_dir) expected_slugs = get_slugs(expected_page_or_dir)
slugs = get_slugs(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 end
end
it 'returns an array with retained order with directories at the top' do 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'] 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 = actual_order =
grouped_entries.map do |page_or_dir| grouped_entries.map do |page_or_dir|
get_slugs(page_or_dir) get_slugs(page_or_dir)
end end
.flatten .flatten
expect(actual_order).to eq(expected_order) expect(actual_order).to eq(expected_order)
end
end end
end end
end end
...@@ -386,7 +388,7 @@ describe WikiPage do ...@@ -386,7 +388,7 @@ describe WikiPage do
it "deletes the page" do it "deletes the page" do
@page.delete @page.delete
expect(wiki.pages).to be_empty expect(wiki.list_pages).to be_empty
end end
it "returns true" do it "returns true" do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment