Merge branch 'zj-port-page-versions-wiki' into 'master'

Wiki: Use FindAllCommits RPC to list page versions

See merge request gitlab-org/gitlab!61459

app/controllers/concerns/wiki_actions.rb

@@ -141,7 +141,7 @@ module WikiActions
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def history
     if page
-      @page_versions = Kaminari.paginate_array(page.versions(page: params[:page].to_i),
+      @commits = Kaminari.paginate_array(page.versions(page: params[:page].to_i),
                                          total_count: page.count_versions)
         .page(params[:page])
 

app/models/wiki_page.rb

@@ -127,10 +127,21 @@ class WikiPage
     @path ||= @page.path
   end
 
+  # Returns a CommitCollection
+  #
+  # Queries the commits for current page's path, equivalent to
+  # `git log path/to/page`. Filters and options supported:
+  # https://gitlab.com/gitlab-org/gitaly/-/blob/master/proto/commit.proto#L322-344
   def versions(options = {})
     return [] unless persisted?
 
-    wiki.wiki.page_versions(page.path, options)
+    default_per_page = Kaminari.config.default_per_page
+    offset = [options[:page].to_i - 1, 0].max * options.fetch(:per_page, default_per_page)
+
+    wiki.repository.commits('HEAD',
+                            path: page.path,
+                            limit: options.fetch(:limit, default_per_page),
+                            offset: offset)
   end
 
   def count_versions

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

@@ -20,7 +20,7 @@
           %th= _('Changes')
           %th= _('Last updated')
       %tbody
-        - @page_versions.each do |commit|
+        - @commits.each do |commit|
           %tr
             %td
               = link_to wiki_page_path(@wiki, @page, version_id: commit.id) do
@@ -33,6 +33,6 @@
                   = commit.message
             %td
               = time_ago_with_tooltip(commit.authored_date)
-  = paginate @page_versions, theme: 'gitlab'
+  = paginate @commits, theme: 'gitlab'
 
 = render 'shared/wikis/sidebar'

changelogs/unreleased/zj-port-page-versions-wiki.yml

+---
+title: 'Wiki: Use FindAllCommits RPC to list page versions'
+merge_request: 61459
+author:
+type: changed

lib/gitlab/git/wiki.rb

@@ -96,22 +96,6 @@ module Gitlab
         end
       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 = {})
-        versions = 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] || DEFAULT_PAGINATION
-        versions[0..slice_bound]
-      end
-
       def count_page_versions(page_path)
         @repository.count_commits(ref: 'HEAD', path: page_path)
       end

lib/gitlab/gitaly_client/wiki_service.rb

@@ -119,30 +119,6 @@ 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] || Gitlab::Git::Wiki::DEFAULT_PAGINATION
-        )
-
-        stream = GitalyClient.call(@repository.storage, :wiki_service, :wiki_get_page_versions, request, timeout: GitalyClient.medium_timeout)
-
-        versions = []
-        stream.each do |message|
-          message.versions.each do |version|
-            versions << new_wiki_page_version(version)
-          end
-        end
-
-        versions
-      end
-
       private
 
       # If a block is given and the yielded value is truthy, iteration will be

spec/models/wiki_page_spec.rb

@@ -620,16 +620,12 @@ RSpec.describe WikiPage do
   end
 
   describe "#versions" do
-    include_context 'subject is persisted page'
+    let(:subject) { create_wiki_page }
 
     it "returns an array of all commits for the page" do
+      expect do
         3.times { |i| subject.update(content: "content #{i}") }
-
-      expect(subject.versions.count).to eq(4)
-    end
-
-    it 'returns instances of WikiPageVersion' do
-      expect(subject.versions).to all( be_a(Gitlab::Git::WikiPageVersion) )
+      end.to change { subject.versions.count }.by(3)
     end
   end
 
@@ -777,8 +773,11 @@ RSpec.describe WikiPage do
   end
 
   describe '#historical?' do
-    include_context 'subject is persisted page'
+    let!(:container) { create(:project) }
+
+    subject { create_wiki_page }
 
+    let(:wiki) { subject.wiki }
     let(:old_version) { subject.versions.last.id }
     let(:old_page) { wiki.find_page(subject.title, old_version) }
     let(:latest_version) { subject.versions.first.id }

spec/support/shared_examples/controllers/wiki_actions_shared_examples.rb

@@ -130,8 +130,8 @@ RSpec.shared_examples 'wiki controller actions' do
     it_behaves_like 'fetching history', :ok do
       let(:allow_read_wiki)   { true }
 
-      it 'assigns @page_versions' do
-        expect(assigns(:page_versions)).to be_present
+      it 'assigns @commits' do
+        expect(assigns(:commits)).to be_present
       end
     end