From 56e53556c538236a040dcd0e499cc198d66e7cf6 Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Mon, 23 Oct 2017 22:12:11 +0200 Subject: Migrate Gitlab::Git::Wiki#file to Gitaly Closes gitaly#689 --- lib/gitlab/git/wiki.rb | 27 ++++++++-- lib/gitlab/gitaly_client/wiki_file.rb | 17 ++++++ lib/gitlab/gitaly_client/wiki_service.rb | 26 ++++++++++ .../projects/wiki/user_views_wiki_page_spec.rb | 13 +++-- spec/models/project_wiki_spec.rb | 47 +++++++++-------- spec/support/bare_repo_operations.rb | 60 ++++++++++++++++++++++ 6 files changed, 157 insertions(+), 33 deletions(-) create mode 100644 lib/gitlab/gitaly_client/wiki_file.rb create mode 100644 spec/support/bare_repo_operations.rb diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb index 549d22adde5..45362ac438b 100644 --- a/lib/gitlab/git/wiki.rb +++ b/lib/gitlab/git/wiki.rb @@ -68,11 +68,13 @@ module Gitlab end def 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) + @repository.gitaly_migrate(:wiki_find_file) do |is_enabled| + if is_enabled + gitaly_find_file(name, version) + else + gollum_find_file(name, version) + end + end end def page_versions(page_path) @@ -156,6 +158,14 @@ module Gitlab 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 gitaly_write_page(name, format, content, commit_details) gitaly_wiki_client.write_page(name, format, content, commit_details) end @@ -170,6 +180,13 @@ module Gitlab Gitlab::Git::WikiPage.new(wiki_page, version) end + + def gitaly_find_file(name, version) + wiki_file = gitaly_wiki_client.find_file(name, version) + return unless wiki_file + + Gitlab::Git::WikiFile.new(wiki_file) + end end end end diff --git a/lib/gitlab/gitaly_client/wiki_file.rb b/lib/gitlab/gitaly_client/wiki_file.rb new file mode 100644 index 00000000000..a2e415864e6 --- /dev/null +++ b/lib/gitlab/gitaly_client/wiki_file.rb @@ -0,0 +1,17 @@ +module Gitlab + module GitalyClient + class WikiFile + FIELDS = %i(name mime_type path raw_data).freeze + + attr_accessor(*FIELDS) + + def initialize(params) + params = params.with_indifferent_access + + FIELDS.each do |field| + instance_variable_set("@#{field}", params[field]) + end + end + end + end +end diff --git a/lib/gitlab/gitaly_client/wiki_service.rb b/lib/gitlab/gitaly_client/wiki_service.rb index 5d7930f5ff9..15f0f30d303 100644 --- a/lib/gitlab/gitaly_client/wiki_service.rb +++ b/lib/gitlab/gitaly_client/wiki_service.rb @@ -80,6 +80,32 @@ module Gitlab [wiki_page, version] end + def find_file(name, revision) + request = Gitaly::WikiFindFileRequest.new( + repository: @gitaly_repo, + name: GitalyClient.encode(name), + revision: GitalyClient.encode(revision) + ) + + response = GitalyClient.call(@repository.storage, :wiki_service, :wiki_find_file, request) + wiki_file = nil + + response.each do |message| + next unless message.name.present? + + if wiki_file + wiki_file.raw_data << message.raw_data + else + wiki_file = GitalyClient::WikiFile.new(message.to_h) + # All gRPC strings in a response are frozen, so we get + # an unfrozen version here so appending in the else clause below doesn't blow up. + wiki_file.raw_data = wiki_file.raw_data.dup + end + end + + wiki_file + end + private def gitaly_commit_details(commit_details) diff --git a/spec/features/projects/wiki/user_views_wiki_page_spec.rb b/spec/features/projects/wiki/user_views_wiki_page_spec.rb index 470391dc66b..89f6901eb01 100644 --- a/spec/features/projects/wiki/user_views_wiki_page_spec.rb +++ b/spec/features/projects/wiki/user_views_wiki_page_spec.rb @@ -81,10 +81,15 @@ describe 'User views a wiki page' do end it 'shows a file stored in a page' do - file = Gollum::File.new(project.wiki) - - allow_any_instance_of(Gollum::Wiki).to receive(:file).with('image.jpg', 'master').and_return(file) - allow_any_instance_of(Gollum::File).to receive(:mime_type).and_return('image/jpeg') + gollum_file_double = double('Gollum::File', + mime_type: 'image/jpeg', + name: 'images/image.jpg', + path: 'images/image.jpg', + raw_data: '') + wiki_file = Gitlab::Git::WikiFile.new(gollum_file_double) + + allow(wiki_file).to receive(:mime_type).and_return('image/jpeg') + allow_any_instance_of(ProjectWiki).to receive(:find_file).with('image.jpg', nil).and_return(wiki_file) expect(page).to have_xpath('//img[@data-src="image.jpg"]') expect(page).to have_link('image', href: "#{project.wiki.wiki_base_path}/image.jpg") diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 52a2b4b2850..3d46434fc27 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -156,36 +156,35 @@ describe ProjectWiki do end describe '#find_file' do - before do - file = Gollum::File.new(subject.wiki) - allow_any_instance_of(Gollum::Wiki) - .to receive(:file).with('image.jpg', 'master') - .and_return(file) - allow_any_instance_of(Gollum::File) - .to receive(:mime_type) - .and_return('image/jpeg') - allow_any_instance_of(Gollum::Wiki) - .to receive(:file).with('non-existant', 'master') - .and_return(nil) - end + shared_examples 'finding a wiki file' do + before do + file = File.open(Rails.root.join('spec', 'fixtures', 'dk.png')) + subject.wiki # Make sure the wiki repo exists - after do - allow_any_instance_of(Gollum::Wiki).to receive(:file).and_call_original - allow_any_instance_of(Gollum::File).to receive(:mime_type).and_call_original - end + BareRepoOperations.new(subject.repository.path_to_repo).commit_file(file, 'image.png') + end - it 'returns the latest version of the file if it exists' do - file = subject.find_file('image.jpg') - expect(file.mime_type).to eq('image/jpeg') + it 'returns the latest version of the file if it exists' do + file = subject.find_file('image.png') + expect(file.mime_type).to eq('image/png') + end + + it 'returns nil if the page does not exist' do + expect(subject.find_file('non-existant')).to eq(nil) + end + + it 'returns a Gitlab::Git::WikiFile instance' do + file = subject.find_file('image.png') + expect(file).to be_a Gitlab::Git::WikiFile + end end - it 'returns nil if the page does not exist' do - expect(subject.find_file('non-existant')).to eq(nil) + context 'when Gitaly wiki_find_file is enabled' do + it_behaves_like 'finding a wiki file' end - it 'returns a Gitlab::Git::WikiFile instance' do - file = subject.find_file('image.jpg') - expect(file).to be_a Gitlab::Git::WikiFile + context 'when Gitaly wiki_find_file is disabled', :skip_gitaly_mock do + it_behaves_like 'finding a wiki file' end end diff --git a/spec/support/bare_repo_operations.rb b/spec/support/bare_repo_operations.rb new file mode 100644 index 00000000000..38d11992dc2 --- /dev/null +++ b/spec/support/bare_repo_operations.rb @@ -0,0 +1,60 @@ +require 'zlib' + +class BareRepoOperations + # The ID of empty tree. + # See http://stackoverflow.com/a/40884093/1856239 and https://github.com/git/git/blob/3ad8b5bf26362ac67c9020bf8c30eee54a84f56d/cache.h#L1011-L1012 + EMPTY_TREE_ID = '4b825dc642cb6eb9a060e54bf8d69288fbee4904'.freeze + + include Gitlab::Popen + + def initialize(path_to_repo) + @path_to_repo = path_to_repo + end + + # Based on https://stackoverflow.com/a/25556917/1856239 + def commit_file(file, dst_path, branch = 'master') + head_id = execute(['show', '--format=format:%H', '--no-patch', branch], allow_failure: true)[0] || EMPTY_TREE_ID + + execute(['read-tree', '--empty']) + execute(['read-tree', head_id]) + + blob_id = execute(['hash-object', '--stdin', '-w']) do |stdin| + stdin.write(file.read) + end + + execute(['update-index', '--add', '--cacheinfo', '100644', blob_id[0], dst_path]) + + tree_id = execute(['write-tree']) + + commit_tree_args = ['commit-tree', tree_id[0], '-m', "Add #{dst_path}"] + commit_tree_args += ['-p', head_id] unless head_id == EMPTY_TREE_ID + commit_id = execute(commit_tree_args) + + execute(['update-ref', "refs/heads/#{branch}", commit_id[0]]) + end + + private + + def execute(args, allow_failure: false) + output, status = popen(base_args + args, nil) do |stdin| + yield stdin if block_given? + end + + unless status.zero? + if allow_failure + return [] + else + raise "Got a non-zero exit code while calling out `#{args.join(' ')}`: #{output}" + end + end + + output.split("\n") + end + + def base_args + [ + Gitlab.config.git.bin_path, + "--git-dir=#{@path_to_repo}" + ] + end +end -- cgit v1.2.1