Commit 36df521d authored by Douwe Maan's avatar Douwe Maan

Merge branch 'fj-42354-custom-hooks-not-triggered-by-UI-wiki-edit' into 'master'

Fix Custom hooks are not triggered by UI wiki edit

Closes #42354

See merge request gitlab-org/gitlab-ce!18251
parents 6a5de6dd e8a27a67
......@@ -82,7 +82,7 @@ gem 'net-ldap'
# Git Wiki
# Required manually in config/initializers/gollum.rb to control load order
gem 'gitlab-gollum-lib', '~> 4.2'
gem 'gitlab-gollum-lib', '~> 4.2', require: false
gem 'gitlab-gollum-rugged_adapter', '~> 0.4.4', require: false
......@@ -415,7 +415,7 @@ group :ed25519 do
end
# Gitaly GRPC client
gem 'gitaly-proto', '~> 0.94.0', require: 'gitaly'
gem 'gitaly-proto', '~> 0.97.0', require: 'gitaly'
gem 'grpc', '~> 1.10.0'
# Locked until https://github.com/google/protobuf/issues/4210 is closed
......
......@@ -290,9 +290,9 @@ GEM
po_to_json (>= 1.0.0)
rails (>= 3.2.0)
gherkin-ruby (0.3.2)
gitaly-proto (0.94.0)
gitaly-proto (0.97.0)
google-protobuf (~> 3.1)
grpc (~> 1.0)
grpc (~> 1.10)
github-linguist (5.3.3)
charlock_holmes (~> 0.7.5)
escape_utils (~> 1.1.0)
......@@ -1064,7 +1064,7 @@ DEPENDENCIES
gettext (~> 3.2.2)
gettext_i18n_rails (~> 1.8.0)
gettext_i18n_rails_js (~> 1.3)
gitaly-proto (~> 0.94.0)
gitaly-proto (~> 0.97.0)
github-linguist (~> 5.3.3)
gitlab-flowdock-git-hook (~> 1.0.1)
gitlab-gollum-lib (~> 4.2)
......
......@@ -29,8 +29,7 @@ class Projects::WikisController < Projects::ApplicationController
else
return render('empty') unless can?(current_user, :create_wiki, @project)
@page = WikiPage.new(@project_wiki)
@page.title = params[:id]
@page = build_page(title: params[:id])
render 'edit'
end
......@@ -54,7 +53,7 @@ class Projects::WikisController < Projects::ApplicationController
else
render 'edit'
end
rescue WikiPage::PageChangedError, WikiPage::PageRenameError => e
rescue WikiPage::PageChangedError, WikiPage::PageRenameError, Gitlab::Git::Wiki::OperationError => e
@error = e
render 'edit'
end
......@@ -70,6 +69,11 @@ class Projects::WikisController < Projects::ApplicationController
else
render action: "edit"
end
rescue Gitlab::Git::Wiki::OperationError => e
@page = build_page(wiki_params)
@error = e
render 'edit'
end
def history
......@@ -94,6 +98,9 @@ class Projects::WikisController < Projects::ApplicationController
redirect_to project_wiki_path(@project, :home),
status: 302,
notice: "Page was successfully deleted"
rescue Gitlab::Git::Wiki::OperationError => e
@error = e
render 'edit'
end
def git_access
......@@ -116,4 +123,10 @@ class Projects::WikisController < Projects::ApplicationController
def wiki_params
params.require(:wiki).permit(:title, :content, :format, :message, :last_commit_sha)
end
def build_page(args)
WikiPage.new(@project_wiki).tap do |page|
page.update_attributes(args)
end
end
end
......@@ -28,7 +28,7 @@ module NavHelper
end
elsif current_path?('jobs#show')
%w[page-gutter build-sidebar right-sidebar-expanded]
elsif current_controller?('wikis') && current_action?('show', 'create', 'edit', 'update', 'history', 'git_access')
elsif current_controller?('wikis') && current_action?('show', 'create', 'edit', 'update', 'history', 'git_access', 'destroy')
%w[page-gutter wiki-sidebar right-sidebar-expanded]
else
[]
......
......@@ -179,7 +179,11 @@ class ProjectWiki
def commit_details(action, message = nil, title = nil)
commit_message = message || default_message(action, title)
Gitlab::Git::Wiki::CommitDetails.new(@user.name, @user.email, commit_message)
Gitlab::Git::Wiki::CommitDetails.new(@user.id,
@user.username,
@user.name,
@user.email,
commit_message)
end
def default_message(action, title)
......
......@@ -265,6 +265,15 @@ class WikiPage
title.present? && self.class.unhyphenize(@page.url_path) != title
end
# Updates the current @attributes hash by merging a hash of params
def update_attributes(attrs)
attrs[:title] = process_title(attrs[:title]) if attrs[:title].present?
attrs.slice!(:content, :format, :message, :title)
@attributes.merge!(attrs)
end
private
# Process and format the title based on the user input.
......@@ -290,15 +299,6 @@ class WikiPage
File.join(components)
end
# Updates the current @attributes hash by merging a hash of params
def update_attributes(attrs)
attrs[:title] = process_title(attrs[:title]) if attrs[:title].present?
attrs.slice!(:content, :format, :message, :title)
@attributes.merge!(attrs)
end
def set_attributes
attributes[:slug] = @page.url_path
attributes[:title] = @page.title
......
---
title: Triggering custom hooks by Wiki UI edit
merge_request: 18251
author:
type: fixed
......@@ -2,10 +2,11 @@ module Gitlab
module Git
class Wiki
DuplicatePageError = Class.new(StandardError)
OperationError = Class.new(StandardError)
CommitDetails = Struct.new(:name, :email, :message) do
CommitDetails = Struct.new(:user_id, :username, :name, :email, :message) do
def to_h
{ name: name, email: email, message: message }
{ user_id: user_id, username: username, name: name, email: email, message: message }
end
end
PageBlob = Struct.new(:name)
......@@ -140,6 +141,10 @@ module Gitlab
end
end
def gollum_wiki
@gollum_wiki ||= Gollum::Wiki.new(@repository.path)
end
private
# options:
......@@ -158,10 +163,6 @@ module Gitlab
offset: options[:offset])
end
def gollum_wiki
@gollum_wiki ||= Gollum::Wiki.new(@repository.path)
end
def gollum_page_by_path(page_path)
page_name = Gollum::Page.canonicalize_filename(page_path)
page_dir = File.split(page_path).first
......@@ -201,12 +202,12 @@ module Gitlab
assert_type!(format, Symbol)
assert_type!(commit_details, CommitDetails)
filename = File.basename(name)
dir = (tmp_dir = File.dirname(name)) == '.' ? '' : tmp_dir
gollum_wiki.write_page(filename, format, content, commit_details.to_h, dir)
with_committer_with_hooks(commit_details) do |committer|
filename = File.basename(name)
dir = (tmp_dir = File.dirname(name)) == '.' ? '' : tmp_dir
nil
gollum_wiki.write_page(filename, format, content, { committer: committer }, dir)
end
rescue Gollum::DuplicatePageError => e
raise Gitlab::Git::Wiki::DuplicatePageError, e.message
end
......@@ -214,24 +215,23 @@ module Gitlab
def gollum_delete_page(page_path, commit_details)
assert_type!(commit_details, CommitDetails)
gollum_wiki.delete_page(gollum_page_by_path(page_path), commit_details.to_h)
nil
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)
page = gollum_page_by_path(page_path)
committer = Gollum::Committer.new(page.wiki, commit_details.to_h)
# 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)
committer.commit
nil
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)
......@@ -288,6 +288,20 @@ module Gitlab
Gitlab::Git::WikiPage.new(wiki_page, version)
end
end
def committer_with_hooks(commit_details)
Gitlab::Wiki::CommitterWithHooks.new(self, commit_details.to_h)
end
def with_committer_with_hooks(commit_details, &block)
committer = committer_with_hooks(commit_details)
yield committer
committer.commit
nil
end
end
end
end
......@@ -200,6 +200,8 @@ module Gitlab
def gitaly_commit_details(commit_details)
Gitaly::WikiCommitDetails.new(
user_id: commit_details.user_id,
user_name: encode_binary(commit_details.username),
name: encode_binary(commit_details.name),
email: encode_binary(commit_details.email),
message: encode_binary(commit_details.message)
......
......@@ -2,10 +2,14 @@ module Gitlab
module GlId
def self.gl_id(user)
if user.present?
"user-#{user.id}"
gl_id_from_id_value(user.id)
else
""
''
end
end
def self.gl_id_from_id_value(id)
"user-#{id}"
end
end
end
module Gitlab
module Wiki
class CommitterWithHooks < Gollum::Committer
attr_reader :gl_wiki
def initialize(gl_wiki, options = {})
@gl_wiki = gl_wiki
super(gl_wiki.gollum_wiki, options)
end
def commit
result = Gitlab::Git::OperationService.new(git_user, gl_wiki.repository).with_branch(
@wiki.ref,
start_branch_name: @wiki.ref
) do |start_commit|
super(false)
end
result[:newrev]
rescue Gitlab::Git::HooksService::PreReceiveError => e
message = "Custom Hook failed: #{e.message}"
raise Gitlab::Git::Wiki::OperationError, message
end
private
def git_user
@git_user ||= Gitlab::Git::User.new(@options[:username],
@options[:name],
@options[:email],
gitlab_id)
end
def gitlab_id
Gitlab::GlId.gl_id_from_id_value(@options[:user_id])
end
end
end
end
......@@ -30,7 +30,7 @@ describe Gitlab::Git::Wiki do
end
def commit_details(name)
Gitlab::Git::Wiki::CommitDetails.new(user.name, user.email, "created page #{name}")
Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.email, "created page #{name}")
end
def destroy_page(title, dir = '')
......
require 'spec_helper'
describe Gitlab::Wiki::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) }
before do
project_wiki.create_page('home', 'test content')
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 'raises exception' do
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
end
it 'does not create a new commit inside the repository' do
current_rev = find_current_rev
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
expect(current_rev).to eq find_current_rev
end
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 'raises exception' do
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
end
it 'does not create a new commit inside the repository' do
current_rev = find_current_rev
expect { subject.commit }.to raise_error(Gitlab::Git::Wiki::OperationError)
expect(current_rev).to eq find_current_rev
end
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
subject.commit
expect(current_rev).not_to eq find_current_rev
end
end
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])
subject.commit
end
it 'creates the commit' do
current_rev = find_current_rev
subject.commit
expect(current_rev).not_to eq find_current_rev
end
end
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
context 'when updating a page' do
before do
project_wiki.update_page(find_page('home'), content: 'some other content', format: :markdown)
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
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_page(name)
wiki.page(title: name)
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'
end
end
......@@ -377,7 +377,7 @@ describe ProjectWiki do
end
def commit_details
Gitlab::Git::Wiki::CommitDetails.new(user.name, user.email, "test commit")
Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.email, "test commit")
end
def create_page(name, content)
......
......@@ -561,7 +561,7 @@ describe WikiPage do
end
def commit_details
Gitlab::Git::Wiki::CommitDetails.new(user.name, user.email, "test commit")
Gitlab::Git::Wiki::CommitDetails.new(user.id, user.username, user.name, user.email, "test commit")
end
def create_page(name, content)
......
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