Skip to content
Snippets Groups Projects
Commit 14000a56 authored by Jacob Vosmaer (GitLab)'s avatar Jacob Vosmaer (GitLab) Committed by Sean McGivern
Browse files

Make Gitaly wiki RPC's mandatory

parent aca47a5b
No related branches found
No related tags found
1 merge request!10495Merge Requests - Assignee
Hide whitespace changes
Inline Side-by-side
lib/gitlab/git/wiki.rb
+ 22
− 148
View file @ 14000a56
@@ -27,63 +27,38 @@ module Gitlab
end
 
def write_page(name, format, content, commit_details)
@repository.gitaly_migrate(:wiki_write_page) do |is_enabled|
if is_enabled
gitaly_write_page(name, format, content, commit_details)
else
gollum_write_page(name, format, content, commit_details)
end
@repository.wrapped_gitaly_errors do
gitaly_write_page(name, format, content, commit_details)
end
end
 
def delete_page(page_path, commit_details)
@repository.gitaly_migrate(:wiki_delete_page) do |is_enabled|
if is_enabled
gitaly_delete_page(page_path, commit_details)
else
gollum_delete_page(page_path, commit_details)
end
@repository.wrapped_gitaly_errors do
gitaly_delete_page(page_path, commit_details)
end
end
 
def update_page(page_path, title, format, content, commit_details)
@repository.gitaly_migrate(:wiki_update_page) do |is_enabled|
if is_enabled
gitaly_update_page(page_path, title, format, content, commit_details)
else
gollum_update_page(page_path, title, format, content, commit_details)
end
@repository.wrapped_gitaly_errors do
gitaly_update_page(page_path, title, format, content, commit_details)
end
end
 
def pages(limit: nil)
@repository.gitaly_migrate(:wiki_get_all_pages) do |is_enabled|
if is_enabled
gitaly_get_all_pages
else
gollum_get_all_pages(limit: limit)
end
@repository.wrapped_gitaly_errors do
gitaly_get_all_pages
end
end
 
def page(title:, version: nil, dir: nil)
@repository.gitaly_migrate(:wiki_find_page,
status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
if is_enabled
gitaly_find_page(title: title, version: version, dir: dir)
else
gollum_find_page(title: title, version: version, dir: dir)
end
@repository.wrapped_gitaly_errors do
gitaly_find_page(title: title, version: version, dir: dir)
end
end
 
def file(name, version)
@repository.gitaly_migrate(:wiki_find_file) do |is_enabled|
if is_enabled
gitaly_find_file(name, version)
else
gollum_find_file(name, version)
end
@repository.wrapped_gitaly_errors do
gitaly_find_file(name, version)
end
end
 
@@ -92,24 +67,15 @@ module Gitlab
# :per_page - The number of items per page.
# :limit - Total number of items to return.
def page_versions(page_path, options = {})
@repository.gitaly_migrate(:wiki_page_versions) do |is_enabled|
if is_enabled
versions = gitaly_wiki_client.page_versions(page_path, options)
# Gitaly uses gollum-lib to get the versions. Gollum defaults to 20
# per page, but also fetches 20 if `limit` or `per_page` < 20.
# Slicing returns an array with the expected number of items.
slice_bound = options[:limit] || options[:per_page] || Gollum::Page.per_page
versions[0..slice_bound]
else
current_page = gollum_page_by_path(page_path)
commits_from_page(current_page, options).map do |gitlab_git_commit|
gollum_page = gollum_wiki.page(current_page.title, gitlab_git_commit.id)
Gitlab::Git::WikiPageVersion.new(gitlab_git_commit, gollum_page&.format)
end
end
versions = @repository.wrapped_gitaly_errors do
gitaly_wiki_client.page_versions(page_path, options)
end
# Gitaly uses gollum-lib to get the versions. Gollum defaults to 20
# per page, but also fetches 20 if `limit` or `per_page` < 20.
# Slicing returns an array with the expected number of items.
slice_bound = options[:limit] || options[:per_page] || Gollum::Page.per_page
versions[0..slice_bound]
end
 
def count_page_versions(page_path)
@@ -131,46 +97,13 @@ module Gitlab
def page_formatted_data(title:, dir: nil, version: nil)
version = version&.id
 
@repository.gitaly_migrate(:wiki_page_formatted_data, status: Gitlab::GitalyClient::MigrationStatus::OPT_OUT) do |is_enabled|
if is_enabled
gitaly_wiki_client.get_formatted_data(title: title, dir: dir, version: version)
else
# We don't use #page because if wiki_find_page feature is enabled, we would
# get a page without formatted_data.
gollum_find_page(title: title, dir: dir, version: version)&.formatted_data
end
@repository.wrapped_gitaly_errors do
gitaly_wiki_client.get_formatted_data(title: title, dir: dir, version: version)
end
end
 
def gollum_wiki
@gollum_wiki ||= Gollum::Wiki.new(@repository.path)
end
private
 
# options:
# :page - The Integer page number.
# :per_page - The number of items per page.
# :limit - Total number of items to return.
def commits_from_page(gollum_page, options = {})
unless options[:limit]
options[:offset] = ([1, options.delete(:page).to_i].max - 1) * Gollum::Page.per_page
options[:limit] = (options.delete(:per_page) || Gollum::Page.per_page).to_i
end
@repository.log(ref: gollum_page.last_version.id,
path: gollum_page.path,
limit: options[:limit],
offset: options[:offset])
end
def gollum_page_by_path(page_path)
page_name = Gollum::Page.canonicalize_filename(page_path)
page_dir = File.split(page_path).first
gollum_wiki.paged(page_name, page_dir)
end
def new_page(gollum_page)
Gitlab::Git::WikiPage.new(gollum_page, new_version(gollum_page, gollum_page.version.id))
end
@@ -199,65 +132,6 @@ module Gitlab
@gitaly_wiki_client ||= Gitlab::GitalyClient::WikiService.new(@repository)
end
 
def gollum_write_page(name, format, content, commit_details)
assert_type!(format, Symbol)
assert_type!(commit_details, CommitDetails)
with_committer_with_hooks(commit_details) do |committer|
filename = File.basename(name)
dir = (tmp_dir = File.dirname(name)) == '.' ? '' : tmp_dir
gollum_wiki.write_page(filename, format, content, { committer: committer }, dir)
end
rescue Gollum::DuplicatePageError => e
raise Gitlab::Git::Wiki::DuplicatePageError, e.message
end
def gollum_delete_page(page_path, commit_details)
assert_type!(commit_details, CommitDetails)
with_committer_with_hooks(commit_details) do |committer|
gollum_wiki.delete_page(gollum_page_by_path(page_path), committer: committer)
end
end
def gollum_update_page(page_path, title, format, content, commit_details)
assert_type!(format, Symbol)
assert_type!(commit_details, CommitDetails)
with_committer_with_hooks(commit_details) do |committer|
page = gollum_page_by_path(page_path)
# Instead of performing two renames if the title has changed,
# the update_page will only update the format and content and
# the rename_page will do anything related to moving/renaming
gollum_wiki.update_page(page, page.name, format, content, committer: committer)
gollum_wiki.rename_page(page, title, committer: committer)
end
end
def gollum_find_page(title:, version: nil, dir: nil)
if version
version = Gitlab::Git::Commit.find(@repository, version).id
end
gollum_page = gollum_wiki.page(title, version, dir)
return unless gollum_page
new_page(gollum_page)
end
def gollum_find_file(name, version)
version ||= self.class.default_ref
gollum_file = gollum_wiki.file(name, version)
return unless gollum_file
Gitlab::Git::WikiFile.new(gollum_file)
end
def gollum_get_all_pages(limit: nil)
gollum_wiki.pages(limit: limit).map { |gollum_page| new_page(gollum_page) }
end
def gitaly_write_page(name, format, content, commit_details)
gitaly_wiki_client.write_page(name, format, content, commit_details)
end
spec/lib/gitlab/git/committer_with_hooks_spec.rb
+ 109
− 107
View file @ 14000a56
require 'spec_helper'
 
describe Gitlab::Git::CommitterWithHooks, seed_helper: true do
shared_examples 'calling wiki hooks' do
let(:project) { create(:project) }
let(:user) { project.owner }
let(:project_wiki) { ProjectWiki.new(project, user) }
let(:wiki) { project_wiki.wiki }
let(:options) do
{
id: user.id,
username: user.username,
name: user.name,
email: user.email,
message: 'commit message'
}
end
subject { described_class.new(wiki, options) }
# TODO https://gitlab.com/gitlab-org/gitaly/issues/1234
skip 'needs to be moved to gitaly-ruby test suite' do
shared_examples 'calling wiki hooks' do
let(:project) { create(:project) }
let(:user) { project.owner }
let(:project_wiki) { ProjectWiki.new(project, user) }
let(:wiki) { project_wiki.wiki }
let(:options) do
{
id: user.id,
username: user.username,
name: user.name,
email: user.email,
message: 'commit message'
}
end
 
before do
project_wiki.create_page('home', 'test content')
end
subject { described_class.new(wiki, options) }
 
shared_examples 'failing pre-receive hook' do
before do
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([false, ''])
expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('update')
expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive')
project_wiki.create_page('home', 'test content')
end
 
it 'raises exception' do
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
end
shared_examples 'failing pre-receive hook' do
before do
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([false, ''])
expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('update')
expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive')
end
 
it 'does not create a new commit inside the repository' do
current_rev = find_current_rev
it 'raises exception' do
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
end
 
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
it 'does not create a new commit inside the repository' do
current_rev = find_current_rev
 
expect(current_rev).to eq find_current_rev
end
end
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
 
shared_examples 'failing update hook' do
before do
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, ''])
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([false, ''])
expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive')
expect(current_rev).to eq find_current_rev
end
end
 
it 'raises exception' do
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
end
shared_examples 'failing update hook' do
before do
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, ''])
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([false, ''])
expect_any_instance_of(Gitlab::Git::HooksService).not_to receive(:run_hook).with('post-receive')
end
 
it 'does not create a new commit inside the repository' do
current_rev = find_current_rev
it 'raises exception' do
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
end
 
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
it 'does not create a new commit inside the repository' do
current_rev = find_current_rev
 
expect(current_rev).to eq find_current_rev
end
end
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
 
shared_examples 'failing post-receive hook' do
before do
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, ''])
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([true, ''])
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('post-receive').and_return([false, ''])
expect(current_rev).to eq find_current_rev
end
end
 
it 'does not raise exception' do
expect { subject.commit }.not_to raise_error
end
shared_examples 'failing post-receive hook' do
before do
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('pre-receive').and_return([true, ''])
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('update').and_return([true, ''])
expect_any_instance_of(Gitlab::Git::HooksService).to receive(:run_hook).with('post-receive').and_return([false, ''])
end
it 'does not raise exception' do
expect { subject.commit }.not_to raise_error
end
 
it 'creates the commit' do
current_rev = find_current_rev
it 'creates the commit' do
current_rev = find_current_rev
 
subject.commit
subject.commit
 
expect(current_rev).not_to eq find_current_rev
expect(current_rev).not_to eq find_current_rev
end
end
end
 
shared_examples 'when hooks call succceeds' do
let(:hook) { double(:hook) }
shared_examples 'when hooks call succceeds' do
let(:hook) { double(:hook) }
 
it 'calls the three hooks' do
expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
expect(hook).to receive(:trigger).exactly(3).times.and_return([true, nil])
it 'calls the three hooks' do
expect(Gitlab::Git::Hook).to receive(:new).exactly(3).times.and_return(hook)
expect(hook).to receive(:trigger).exactly(3).times.and_return([true, nil])
 
subject.commit
end
subject.commit
end
 
it 'creates the commit' do
current_rev = find_current_rev
it 'creates the commit' do
current_rev = find_current_rev
 
subject.commit
subject.commit
 
expect(current_rev).not_to eq find_current_rev
expect(current_rev).not_to eq find_current_rev
end
end
end
 
context 'when creating a page' do
before do
project_wiki.create_page('index', 'test content')
context 'when creating a page' do
before do
project_wiki.create_page('index', 'test content')
end
it_behaves_like 'failing pre-receive hook'
it_behaves_like 'failing update hook'
it_behaves_like 'failing post-receive hook'
it_behaves_like 'when hooks call succceeds'
end
 
it_behaves_like 'failing pre-receive hook'
it_behaves_like 'failing update hook'
it_behaves_like 'failing post-receive hook'
it_behaves_like 'when hooks call succceeds'
end
context 'when updating a page' do
before do
project_wiki.update_page(find_page('home'), content: 'some other content', format: :markdown)
end
 
context 'when updating a page' do
before do
project_wiki.update_page(find_page('home'), content: 'some other content', format: :markdown)
it_behaves_like 'failing pre-receive hook'
it_behaves_like 'failing update hook'
it_behaves_like 'failing post-receive hook'
it_behaves_like 'when hooks call succceeds'
end
 
it_behaves_like 'failing pre-receive hook'
it_behaves_like 'failing update hook'
it_behaves_like 'failing post-receive hook'
it_behaves_like 'when hooks call succceeds'
end
context 'when deleting a page' do
before do
project_wiki.delete_page(find_page('home'))
end
 
context 'when deleting a page' do
before do
project_wiki.delete_page(find_page('home'))
it_behaves_like 'failing pre-receive hook'
it_behaves_like 'failing update hook'
it_behaves_like 'failing post-receive hook'
it_behaves_like 'when hooks call succceeds'
end
 
it_behaves_like 'failing pre-receive hook'
it_behaves_like 'failing update hook'
it_behaves_like 'failing post-receive hook'
it_behaves_like 'when hooks call succceeds'
end
def find_current_rev
wiki.gollum_wiki.repo.commits.first&.sha
end
 
def find_current_rev
wiki.gollum_wiki.repo.commits.first&.sha
def find_page(name)
wiki.page(title: name)
end
end
 
def find_page(name)
wiki.page(title: name)
context 'when Gitaly is enabled' do
it_behaves_like 'calling wiki hooks'
end
end
# TODO: Uncomment once Gitaly updates the ruby vendor code
# context 'when Gitaly is enabled' do
# it_behaves_like 'calling wiki hooks'
# end
 
context 'when Gitaly is disabled', :skip_gitaly_mock do
it_behaves_like 'calling wiki hooks'
context 'when Gitaly is disabled', :disable_gitaly do
it_behaves_like 'calling wiki hooks'
end
end
end
spec/lib/gitlab/git/wiki_spec.rb
+ 2
− 4
View file @ 14000a56
@@ -6,9 +6,7 @@ describe Gitlab::Git::Wiki do
let(:project_wiki) { ProjectWiki.new(project, user) }
subject { project_wiki.wiki }
 
# Remove skip_gitaly_mock flag when gitaly_find_page when
# https://gitlab.com/gitlab-org/gitlab-ce/issues/42039 is solved
describe '#page', :skip_gitaly_mock do
describe '#page' do
before do
create_page('page1', 'content')
create_page('foo/page1', 'content foo/page1')
@@ -25,7 +23,7 @@ describe Gitlab::Git::Wiki do
end
end
 
describe '#delete_page', :skip_gitaly_mock do
describe '#delete_page' do
after do
destroy_page('page1')
end
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