Merge branch 'zj/gitlab-ce-zj-wiki-page-versions'

4 files changed, 75 insertions, 19 deletions

diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb
index fde96cbd35d..c4930d3d18d 100644
--- a/app/controllers/projects/wikis_controller.rb
+++ b/app/controllers/projects/wikis_controller.rb
@@ -76,9 +76,9 @@ class Projects::WikisController < Projects::ApplicationController
     @page = @project_wiki.find_page(params[:id])
 
     if @page
-      @page_versions = Kaminari.paginate_array(@page.versions(page: params[:page]),
+      @page_versions = Kaminari.paginate_array(@page.versions(page: params[:page].to_i),
                                                total_count: @page.count_versions)
-                               .page(params[:page])
+        .page(params[:page])
     else
       redirect_to(
         project_wiki_path(@project, :home),
diff --git a/lib/gitlab/git/wiki.rb b/lib/gitlab/git/wiki.rb
index daa17fc72cf..39040d56971 100644
--- a/lib/gitlab/git/wiki.rb
+++ b/lib/gitlab/git/wiki.rb
@@ -96,11 +96,23 @@ module Gitlab
       #  :per_page - The number of items per page.
       #  :limit    - Total number of items to return.
       def page_versions(page_path, options = {})
-        current_page = gollum_page_by_path(page_path)
+        @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)
+            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
         end
       end
 
diff --git a/lib/gitlab/gitaly_client/wiki_service.rb b/lib/gitlab/gitaly_client/wiki_service.rb
index 8e87a8cc36f..0d8dd5cb8f4 100644
--- a/lib/gitlab/gitaly_client/wiki_service.rb
+++ b/lib/gitlab/gitaly_client/wiki_service.rb
@@ -101,6 +101,30 @@ module Gitlab
         pages
       end
 
+      # options:
+      #  :page     - The Integer page number.
+      #  :per_page - The number of items per page.
+      #  :limit    - Total number of items to return.
+      def page_versions(page_path, options)
+        request = Gitaly::WikiGetPageVersionsRequest.new(
+          repository: @gitaly_repo,
+          page_path: encode_binary(page_path),
+          page: options[:page] || 1,
+          per_page: options[:per_page] || Gollum::Page.per_page
+        )
+
+        stream = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_page_versions, request)
+
+        versions = []
+        stream.each do |message|
+          message.versions.each do |version|
+            versions << new_wiki_page_version(version)
+          end
+        end
+
+        versions
+      end
+
       def find_file(name, revision)
         request = Gitaly::WikiFindFileRequest.new(
           repository: @gitaly_repo,
@@ -141,7 +165,7 @@ module Gitlab
 
       private
 
-      # If a block is given and the yielded value is true, iteration will be
+      # If a block is given and the yielded value is truthy, iteration will be
       # stopped early at that point; else the iterator is consumed entirely.
       # The iterator is traversed with `next` to allow resuming the iteration.
       def wiki_page_from_iterator(iterator)
@@ -158,10 +182,7 @@ module Gitlab
           else
             wiki_page = GitalyClient::WikiPage.new(page.to_h)
 
-            version = Gitlab::Git::WikiPageVersion.new(
-              Gitlab::Git::Commit.decorate(@repository, page.version.commit),
-              page.version.format
-            )
+            version = new_wiki_page_version(page.version)
           end
         end
 
@@ -170,6 +191,13 @@ module Gitlab
         [wiki_page, version]
       end
 
+      def new_wiki_page_version(version)
+        Gitlab::Git::WikiPageVersion.new(
+          Gitlab::Git::Commit.decorate(@repository, version.commit),
+          version.format
+        )
+      end
+
       def gitaly_commit_details(commit_details)
         Gitaly::WikiCommitDetails.new(
           name: encode_binary(commit_details.name),
diff --git a/spec/models/wiki_page_spec.rb b/spec/models/wiki_page_spec.rb
index 4a1d12cd448..d53ba497ed1 100644
--- a/spec/models/wiki_page_spec.rb
+++ b/spec/models/wiki_page_spec.rb
@@ -364,18 +364,34 @@ describe WikiPage do
   end
 
   describe "#versions" do
-    before do
-      create_page("Update", "content")
-      @page = wiki.find_page("Update")
+    shared_examples 'wiki page versions' do
+      let(:page) { wiki.find_page("Update") }
+
+      before do
+        create_page("Update", "content")
+      end
+
+      after do
+        destroy_page("Update")
+      end
+
+      it "returns an array of all commits for the page" do
+        3.times { |i| page.update(content: "content #{i}") }
+
+        expect(page.versions.count).to eq(4)
+      end
+
+      it 'returns instances of WikiPageVersion' do
+        expect(page.versions).to all( be_a(Gitlab::Git::WikiPageVersion) )
+      end
     end
 
-    after do
-      destroy_page("Update")
+    context 'when Gitaly is enabled' do
+      it_behaves_like 'wiki page versions'
     end
 
-    it "returns an array of all commits for the page" do
-      3.times { |i| @page.update(content: "content #{i}") }
-      expect(@page.versions.count).to eq(4)
+    context 'when Gitaly is disabled', :disable_gitaly do
+      it_behaves_like 'wiki page versions'
     end
   end