Commit 19cccbba authored by Zeger-Jan van de Weg's avatar Zeger-Jan van de Weg Committed by Bob Van Landuyt

Wiki: Use FindAllCommits RPC to list page versions

The wiki interface to the Git data leverages the Wiki Service in Gitaly.
This service uses LibGit2, and the Ruby sidecar. As such there's a
maintainance burden to the Wiki RPCs.

This change changes the Gitaly RPC that the `Wiki#versions` call uses to
FinddAllCommits. This RPC uses Git itself, and as such will allow the
remove of the PageVersions RPC on the Gitaly side in the next release.

Furthermore, it's expected that this change resolves other bugs too, for
example: https://gitlab.com/gitlab-org/gitlab/-/issues/330295#note_572087692
In this instance, `git-fsck(1)` can inflate a ZLib'ed Git object, where
Libgit2 for some reason fails. While I don't yet know why Libgit2 fails,
leveraging the same implementation as `git-fsck(1)` should resolve it.

Changelog: changed
parent d431623b
......@@ -141,8 +141,8 @@ module WikiActions
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def history
if page
@page_versions = Kaminari.paginate_array(page.versions(page: params[:page].to_i),
total_count: page.count_versions)
@commits = Kaminari.paginate_array(page.versions(page: params[:page].to_i),
total_count: page.count_versions)
.page(params[:page])
render 'shared/wikis/history'
......
......@@ -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
......
......@@ -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'
---
title: 'Wiki: Use FindAllCommits RPC to list page versions'
merge_request: 61459
author:
type: changed
......@@ -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
......
......@@ -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
......
......@@ -774,7 +774,7 @@ RSpec.describe WikiPage do
describe '#historical?' do
let!(:container) { create(:project) }
subject { create_wiki_page }
let(:wiki) { subject.wiki }
......
......@@ -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
......
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