Merge branch 'wiki-gitlab-git' into 'master'

Hide Gollum inside Gitlab::Git::Wiki

See merge request gitlab-org/gitlab-ce!14447

app/controllers/projects/wikis_controller.rb

@@ -18,16 +18,12 @@ class Projects::WikisController < Projects::ApplicationController
       response.headers['Content-Security-Policy'] = "default-src 'none'"
       response.headers['X-Content-Security-Policy'] = "default-src 'none'"
 
-      if file.on_disk?
-        send_file file.on_disk_path, disposition: 'inline'
-      else
       send_data(
         file.raw_data,
         type: file.mime_type,
         disposition: 'inline',
         filename: file.name
       )
-      end
     else
       return render('empty') unless can?(current_user, :create_wiki, @project)
       @page = WikiPage.new(@project_wiki)

app/models/project_wiki.rb

@@ -54,12 +54,15 @@ class ProjectWiki
     [Gitlab.config.gitlab.relative_url_root, '/', @project.full_path, '/wikis'].join('')
   end
 
-  # Returns the Gollum::Wiki object.
+  # Returns the Gitlab::Git::Wiki object.
   def wiki
     @wiki ||= begin
-      Gollum::Wiki.new(path_to_repo)
-    rescue Rugged::OSError
-      create_repo!
+      gl_repository = Gitlab::GlRepository.gl_repository(project, true)
+      raw_repository = Gitlab::Git::Repository.new(project.repository_storage, disk_path + '.git', gl_repository)
+
+      create_repo!(raw_repository) unless raw_repository.exists?
+
+      Gitlab::Git::Wiki.new(raw_repository)
     end
   end
 
@@ -86,20 +89,14 @@ class ProjectWiki
   # Returns an initialized WikiPage instance or nil
   def find_page(title, version = nil)
     page_title, page_dir = page_title_and_dir(title)
-    if page = wiki.page(page_title, version, page_dir)
+
+    if page = wiki.page(title: page_title, version: version, dir: page_dir)
       WikiPage.new(self, page, true)
-    else
-      nil
     end
   end
 
-  def find_file(name, version = nil, try_on_disk = true)
-    version = wiki.ref if version.nil? # Gollum::Wiki#file ?
-    if wiki_file = wiki.file(name, version, try_on_disk)
-      wiki_file
-    else
-      nil
-    end
+  def find_file(name, version = nil)
+    wiki.file(name, version)
   end
 
   def create_page(title, content, format = :markdown, message = nil)
@@ -108,7 +105,7 @@ class ProjectWiki
     wiki.write_page(title, format.to_sym, content, commit)
 
     update_project_activity
-  rescue Gollum::DuplicatePageError => e
+  rescue Gitlab::Git::Wiki::DuplicatePageError => e
     @error_message = "Duplicate page: #{e.message}"
     return false
   end
@@ -116,13 +113,13 @@ class ProjectWiki
   def update_page(page, content:, title: nil, format: :markdown, message: nil)
     commit = commit_details(:updated, message, page.title)
 
-    wiki.update_page(page, title || page.name, format.to_sym, content, commit)
+    wiki.update_page(page.path, title || page.name, format.to_sym, content, commit)
 
     update_project_activity
   end
 
   def delete_page(page, message = nil)
-    wiki.delete_page(page, commit_details(:deleted, message, page.title))
+    wiki.delete_page(page.path, commit_details(:deleted, message, page.title))
 
     update_project_activity
   end
@@ -145,20 +142,8 @@ class ProjectWiki
     wiki.class.default_ref
   end
 
-  def create_repo!
-    if init_repo(disk_path)
-      wiki = Gollum::Wiki.new(path_to_repo)
-    else
-      raise CouldNotCreateWikiError
-    end
-
-    repository.after_create
-
-    wiki
-  end
-
   def ensure_repository
-    create_repo! unless repository_exists?
+    raise CouldNotCreateWikiError unless wiki.repository_exists?
   end
 
   def hook_attrs
@@ -173,24 +158,24 @@ class ProjectWiki
 
   private
 
-  def init_repo(disk_path)
+  def create_repo!(raw_repository)
     gitlab_shell.add_repository(project.repository_storage, disk_path)
+
+    raise CouldNotCreateWikiError unless raw_repository.exists?
+
+    repository.after_create
   end
 
   def commit_details(action, message = nil, title = nil)
     commit_message = message || default_message(action, title)
 
-    { email: @user.email, name: @user.name, message: commit_message }
+    Gitlab::Git::Wiki::CommitDetails.new(@user.name, @user.email, commit_message)
   end
 
   def default_message(action, title)
     "#{@user.username} #{action} page: #{title}"
   end
 
-  def path_to_repo
-    @path_to_repo ||= File.join(project.repository_storage_path, "#{disk_path}.git")
-  end
-
   def update_project_activity
     @project.touch(:last_activity_at, :last_repository_updated_at)
   end

app/models/wiki_page.rb

@@ -50,7 +50,7 @@ class WikiPage
   # The Gitlab ProjectWiki instance.
   attr_reader :wiki
 
-  # The raw Gollum::Page instance.
+  # The raw Gitlab::Git::WikiPage instance.
   attr_reader :page
 
   # The attributes Hash used for storing and validating
@@ -75,7 +75,7 @@ class WikiPage
     if @attributes[:slug].present?
       @attributes[:slug]
     else
-      wiki.wiki.preview_page(title, '', format).url_path
+      wiki.wiki.preview_slug(title, format)
     end
   end
 
@@ -131,7 +131,7 @@ class WikiPage
   def versions
     return [] unless persisted?
 
-    @page.versions
+    wiki.wiki.page_versions(@page.path)
   end
 
   def commit
@@ -264,8 +264,8 @@ class WikiPage
     end
 
     page_title, page_dir = wiki.page_title_and_dir(page_details)
-    gollum_wiki = wiki.wiki
-    @page = gollum_wiki.paged(page_title, page_dir)
+    gitlab_git_wiki = wiki.wiki
+    @page = gitlab_git_wiki.page(title: page_title, dir: page_dir)
 
     set_attributes
     @persisted = errors.blank?

app/views/projects/wikis/history.html.haml

@@ -29,13 +29,13 @@
                                                      commit.id, index == 0) do
               = truncate_sha(commit.id)
           %td
-            = commit.author.name
+            = commit.author_name
           %td
             = commit.message
           %td
             #{time_ago_with_tooltip(version.authored_date)}
           %td
             %strong
-              = @page.page.wiki.page(@page.page.name, commit.id).try(:format)
+              = version.format
 
 = render 'sidebar'

app/views/projects/wikis/show.html.haml

@@ -11,7 +11,7 @@
   .nav-text
     %h2.wiki-page-title= @page.title.capitalize
     %span.wiki-last-edit-by
-      = (_("Last edited by %{name}") % { name: "<strong>#{@page.commit.author.name}</strong>" }).html_safe
+      = (_("Last edited by %{name}") % { name: "<strong>#{@page.commit.author_name}</strong>" }).html_safe
       #{time_ago_with_tooltip(@page.commit.authored_date)}
 
   .nav-controls

lib/gitlab/git/wiki.rb

+module Gitlab
+  module Git
+    class Wiki
+      DuplicatePageError = Class.new(StandardError)
+
+      CommitDetails = Struct.new(:name, :email, :message) do
+        def to_h
+          { name: name, email: email, message: message }
+        end
+      end
+
+      def self.default_ref
+        'master'
+      end
+
+      # Initialize with a Gitlab::Git::Repository instance
+      def initialize(repository)
+        @repository = repository
+      end
+
+      def repository_exists?
+        @repository.exists?
+      end
+
+      def write_page(name, format, content, commit_details)
+        assert_type!(format, Symbol)
+        assert_type!(commit_details, CommitDetails)
+
+        gollum_wiki.write_page(name, format, content, commit_details.to_h)
+
+        nil
+      rescue Gollum::DuplicatePageError => e
+        raise Gitlab::Git::Wiki::DuplicatePageError, e.message
+      end
+
+      def 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
+      end
+
+      def update_page(page_path, title, format, content, commit_details)
+        assert_type!(format, Symbol)
+        assert_type!(commit_details, CommitDetails)
+
+        gollum_wiki.update_page(gollum_page_by_path(page_path), title, format, content, commit_details.to_h)
+        nil
+      end
+
+      def pages
+        gollum_wiki.pages.map { |gollum_page| new_page(gollum_page) }
+      end
+
+      def 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 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 page_versions(page_path)
+        current_page = gollum_page_by_path(page_path)
+        current_page.versions.map do |gollum_git_commit|
+          gollum_page = gollum_wiki.page(current_page.title, gollum_git_commit.id)
+          new_version(gollum_page, gollum_git_commit.id)
+        end
+      end
+
+      def preview_slug(title, format)
+        gollum_wiki.preview_page(title, '', format).url_path
+      end
+
+      private
+
+      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
+
+        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
+
+      def new_version(gollum_page, commit_id)
+        commit = Gitlab::Git::Commit.find(@repository, commit_id)
+        Gitlab::Git::WikiPageVersion.new(commit, gollum_page&.format)
+      end
+
+      def assert_type!(object, klass)
+        unless object.is_a?(klass)
+          raise ArgumentError, "expected a #{klass}, got #{object.inspect}"
+        end
+      end
+    end
+  end
+end

lib/gitlab/git/wiki_file.rb

+module Gitlab
+  module Git
+    class WikiFile
+      attr_reader :mime_type, :raw_data, :name
+
+      # This class is meant to be serializable so that it can be constructed
+      # by Gitaly and sent over the network to GitLab.
+      #
+      # Because Gollum::File is not serializable we must get all the data from
+      # 'gollum_file' during initialization, and NOT store it in an instance
+      # variable.
+      def initialize(gollum_file)
+        @mime_type = gollum_file.mime_type
+        @raw_data = gollum_file.raw_data
+        @name = gollum_file.name
+      end
+    end
+  end
+end

lib/gitlab/git/wiki_page.rb

+module Gitlab
+  module Git
+    class WikiPage
+      attr_reader :url_path, :title, :format, :path, :version, :raw_data, :name, :text_data, :historical
+
+      # This class is meant to be serializable so that it can be constructed
+      # by Gitaly and sent over the network to GitLab.
+      #
+      # Because Gollum::Page is not serializable we must get all the data from
+      # 'gollum_page' during initialization, and NOT store it in an instance
+      # variable.
+      #
+      # Note that 'version' is a WikiPageVersion instance which it itself
+      # serializable. That means it's OK to store 'version' in an instance
+      # variable.
+      def initialize(gollum_page, version)
+        @url_path = gollum_page.url_path
+        @title = gollum_page.title
+        @format = gollum_page.format
+        @path = gollum_page.path
+        @raw_data = gollum_page.raw_data
+        @name = gollum_page.name
+        @historical = gollum_page.historical?
+
+        @version = version
+      end
+
+      def historical?
+        @historical
+      end
+
+      def text_data
+        return @text_data if defined?(@text_data)
+
+        @text_data = @raw_data && Gitlab::EncodingHelper.encode!(@raw_data.dup)
+      end
+    end
+  end
+end

lib/gitlab/git/wiki_page_version.rb

+module Gitlab
+  module Git
+    class WikiPageVersion
+      attr_reader :commit, :format
+
+      # This class is meant to be serializable so that it can be constructed
+      # by Gitaly and sent over the network to GitLab.
+      #
+      # Both 'commit' (a Gitlab::Git::Commit) and 'format' (a string) are
+      # serializable.
+      def initialize(commit, format)
+        @commit = commit
+        @format = format
+      end
+
+      delegate :message, :sha, :id, :author_name, :authored_date, to: :commit
+    end
+  end
+end

spec/features/projects/wiki/user_views_wiki_page_spec.rb

@@ -83,7 +83,7 @@ describe 'User views a wiki page' do
     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', true).and_return(file)
+      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')
 
       expect(page).to have_xpath('//img[@data-src="image.jpg"]')

spec/models/project_wiki_spec.rb

@@ -6,13 +6,10 @@ describe ProjectWiki do
   let(:user) { project.owner }
   let(:gitlab_shell) { Gitlab::Shell.new }
   let(:project_wiki) { described_class.new(project, user) }
+  let(:raw_repository) { Gitlab::Git::Repository.new(project.repository_storage, subject.disk_path + '.git', 'foo') }
 
   subject { project_wiki }
 
-  before do
-    project_wiki.wiki
-  end
-
   describe "#path_with_namespace" do
     it "returns the project path with namespace with the .wiki extension" do
       expect(subject.path_with_namespace).to eq(project.full_path + '.wiki')
@@ -61,8 +58,8 @@ describe ProjectWiki do
   end
 
   describe "#wiki" do
-    it "contains a Gollum::Wiki instance" do
-      expect(subject.wiki).to be_a Gollum::Wiki
+    it "contains a Gitlab::Git::Wiki instance" do
+      expect(subject.wiki).to be_a Gitlab::Git::Wiki
     end
 
     it "creates a new wiki repo if one does not yet exist" do
@@ -70,20 +67,18 @@ describe ProjectWiki do
     end
 
     it "raises CouldNotCreateWikiError if it can't create the wiki repository" do
-      allow(project_wiki).to receive(:init_repo).and_return(false)
-      expect { project_wiki.send(:create_repo!) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError)
+      # Create a fresh project which will not have a wiki
+      project_wiki = described_class.new(create(:project), user)
+      gitlab_shell = double(:gitlab_shell)
+      allow(gitlab_shell).to receive(:add_repository)
+      allow(project_wiki).to receive(:gitlab_shell).and_return(gitlab_shell)
+
+      expect { project_wiki.send(:wiki) }.to raise_exception(ProjectWiki::CouldNotCreateWikiError)
     end
   end
 
   describe "#empty?" do
     context "when the wiki repository is empty" do
-      before do
-        allow_any_instance_of(Gitlab::Shell).to receive(:add_repository) do
-          create_temp_repo("#{Rails.root}/tmp/test-git-base-path/non-existant.wiki.git")
-        end
-        allow(project).to receive(:full_path).and_return("non-existant")
-      end
-
       describe '#empty?' do
         subject { super().empty? }
         it { is_expected.to be_truthy }
@@ -154,13 +149,13 @@ describe ProjectWiki do
     before do
       file = Gollum::File.new(subject.wiki)
       allow_any_instance_of(Gollum::Wiki)
-                   .to receive(:file).with('image.jpg', 'master', true)
+                   .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', true)
+                   .to receive(:file).with('non-existant', 'master')
                    .and_return(nil)
     end
 
@@ -178,9 +173,9 @@ describe ProjectWiki do
       expect(subject.find_file('non-existant')).to eq(nil)
     end
 
-    it 'returns a Gollum::File instance' do
+    it 'returns a Gitlab::Git::WikiFile instance' do
       file = subject.find_file('image.jpg')
-      expect(file).to be_a Gollum::File
+      expect(file).to be_a Gitlab::Git::WikiFile
     end
   end
 
@@ -222,9 +217,9 @@ describe ProjectWiki do
   describe "#update_page" do
     before do
       create_page("update-page", "some content")
-      @gollum_page = subject.wiki.paged("update-page")
+      @gitlab_git_wiki_page = subject.wiki.page(title: "update-page")
       subject.update_page(
-        @gollum_page,
+        @gitlab_git_wiki_page,
         content: "some other content",
         format: :markdown,
         message: "updated page"
@@ -246,7 +241,7 @@ describe ProjectWiki do
 
     it 'updates project activity' do
       subject.update_page(
-        @gollum_page,
+        @gitlab_git_wiki_page,
         content: 'Yet more content',
         format: :markdown,
         message: 'Updated page again'
@@ -262,7 +257,7 @@ describe ProjectWiki do
   describe "#delete_page" do
     before do
       create_page("index", "some content")
-      @page = subject.wiki.paged("index")
+      @page = subject.wiki.page(title: "index")
     end
 
     it "deletes the page" do
@@ -282,27 +277,28 @@ describe ProjectWiki do
 
   describe '#create_repo!' do
     it 'creates a repository' do
-      expect(subject).to receive(:init_repo)
-        .with(subject.full_path)
-        .and_return(true)
-
+      expect(raw_repository.exists?).to eq(false)
       expect(subject.repository).to receive(:after_create)
 
-      expect(subject.create_repo!).to be_an_instance_of(Gollum::Wiki)
+      subject.send(:create_repo!, raw_repository)
+
+      expect(raw_repository.exists?).to eq(true)
     end
   end
 
   describe '#ensure_repository' do
     it 'creates the repository if it not exist' do
-      allow(subject).to receive(:repository_exists?).and_return(false)
-
-      expect(subject).to receive(:create_repo!)
+      expect(raw_repository.exists?).to eq(false)
 
+      expect(subject).to receive(:create_repo!).and_call_original
       subject.ensure_repository
+
+      expect(raw_repository.exists?).to eq(true)
     end
 
     it 'does not create the repository if it exists' do
-      allow(subject).to receive(:repository_exists?).and_return(true)
+      subject.wiki
+      expect(raw_repository.exists?).to eq(true)
 
       expect(subject).not_to receive(:create_repo!)
 
@@ -329,7 +325,7 @@ describe ProjectWiki do
   end
 
   def commit_details
-    { name: user.name, email: user.email, message: "test commit" }
+    Gitlab::Git::Wiki::CommitDetails.new(user.name, user.email, "test commit")
   end
 
   def create_page(name, content)
@@ -337,6 +333,6 @@ describe ProjectWiki do
   end
 
   def destroy_page(page)
-    subject.wiki.delete_page(page, commit_details)
+    subject.delete_page(page, commit_details)
   end
 end

spec/models/wiki_page_spec.rb

@@ -80,7 +80,7 @@ describe WikiPage do
     context "when initialized with an existing gollum page" do
       before do
         create_page("test page", "test content")
-        @page = wiki.wiki.paged("test page")
+        @page = wiki.wiki.page(title: "test page")
         @wiki_page = described_class.new(wiki, @page, true)
       end
 
@@ -105,7 +105,7 @@ describe WikiPage do
       end
 
       it "sets the version attribute" do
-        expect(@wiki_page.version).to be_a Gollum::Git::Commit
+        expect(@wiki_page.version).to be_a Gitlab::Git::WikiPageVersion
       end
     end
   end
@@ -321,14 +321,14 @@ describe WikiPage do
     end
 
     it 'returns true when requesting an old version' do
-      old_version = @page.versions.last.to_s
+      old_version = @page.versions.last.id
       old_page = wiki.find_page('Update', old_version)
 
       expect(old_page.historical?).to eq true
     end
 
     it 'returns false when requesting latest version' do
-      latest_version = @page.versions.first.to_s
+      latest_version = @page.versions.first.id
       latest_page = wiki.find_page('Update', latest_version)
 
       expect(latest_page.historical?).to eq false
@@ -393,7 +393,7 @@ describe WikiPage do
   end
 
   def commit_details
-    { name: user.name, email: user.email, message: "test commit" }
+    Gitlab::Git::Wiki::CommitDetails.new(user.name, user.email, "test commit")
   end
 
   def create_page(name, content)
@@ -401,8 +401,8 @@ describe WikiPage do
   end
 
   def destroy_page(title)
-    page = wiki.wiki.paged(title)
-    wiki.wiki.delete_page(page, commit_details)
+    page = wiki.wiki.page(title: title)
+    wiki.delete_page(page, commit_details)
   end
 
   def get_slugs(page_or_dir)

spec/services/projects/create_service_spec.rb

@@ -76,9 +76,8 @@ describe Projects::CreateService, '#execute' do
     context 'wiki_enabled true creates wiki repository directory' do
       it do
         project = create_project(user, opts)
-        path = ProjectWiki.new(project, user).send(:path_to_repo)
 
-        expect(File.exist?(path)).to be_truthy
+        expect(wiki_repo(project).exists?).to be_truthy
       end
     end
 
@@ -86,11 +85,15 @@ describe Projects::CreateService, '#execute' do
       it do
         opts[:wiki_enabled] = false
         project = create_project(user, opts)
-        path = ProjectWiki.new(project, user).send(:path_to_repo)
 
-        expect(File.exist?(path)).to be_falsey
+        expect(wiki_repo(project).exists?).to be_falsey
       end
     end
+
+    def wiki_repo(project)
+      relative_path = ProjectWiki.new(project).disk_path + '.git'
+      Gitlab::Git::Repository.new(project.repository_storage, relative_path, 'foobar')
+    end
   end
 
   context 'builds_enabled global setting' do

spec/workers/repository_check/single_repository_worker_spec.rb

@@ -69,7 +69,12 @@ describe RepositoryCheck::SingleRepositoryWorker do
   end
 
   def break_wiki(project)
-    FileUtils.rm_rf(wiki_path(project) + '/objects')
+    objects_dir = wiki_path(project) + '/objects'
+
+    # Replace the /objects directory with a file so that the repo is
+    # invalid, _and_ 'git init' cannot fix it.
+    FileUtils.rm_rf(objects_dir)
+    FileUtils.touch(objects_dir) if File.directory?(wiki_path(project))
   end
 
   def wiki_path(project)