Merge branch 'fj-replace-delete-page-rpc-call' into 'master'

Replace wiki_delete_page RPC [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!56495

app/models/wiki.rb

@@ -196,11 +196,21 @@ class Wiki
   def delete_page(page, message = nil)
     return unless page
 
+    if Feature.enabled?(:gitaly_replace_wiki_delete_page, user, default_enabled: :yaml)
+      capture_git_error(:deleted) do
+        repository.delete_file(user, page.path, **multi_commit_options(:deleted, message, page.title))
+
+        after_wiki_activity
+
+        true
+      end
+    else
       wiki.delete_page(page.path, commit_details(:deleted, message, page.title))
       after_wiki_activity
 
       true
     end
+  end
 
   def page_title_and_dir(title)
     return unless title
@@ -276,8 +286,20 @@ class Wiki
 
   private
 
+  def multi_commit_options(action, message = nil, title = nil)
+    commit_message = build_commit_message(action, message, title)
+    git_user = Gitlab::Git::User.from_gitlab(user)
+
+    {
+      branch_name: repository.root_ref,
+      message: commit_message,
+      author_email: git_user.email,
+      author_name: git_user.name
+    }
+  end
+
   def commit_details(action, message = nil, title = nil)
-    commit_message = message.presence || default_message(action, title)
+    commit_message = build_commit_message(action, message, title)
     git_user = Gitlab::Git::User.from_gitlab(user)
 
     Gitlab::Git::Wiki::CommitDetails.new(user.id,
@@ -287,9 +309,26 @@ class Wiki
                                          commit_message)
   end
 
+  def build_commit_message(action, message, title)
+    message.presence || default_message(action, title)
+  end
+
   def default_message(action, title)
     "#{user.username} #{action} page: #{title}"
   end
+
+  def capture_git_error(action, &block)
+    yield block
+  rescue Gitlab::Git::Index::IndexError,
+         Gitlab::Git::CommitError,
+         Gitlab::Git::PreReceiveError,
+         Gitlab::Git::CommandError,
+         ArgumentError => error
+
+    Gitlab::ErrorTracking.log_exception(error, action: action, wiki_id: id)
+
+    false
+  end
 end
 
 Wiki.prepend_if_ee('EE::Wiki')

config/feature_flags/development/gitaly_replace_wiki_delete_page.yml

+---
+name: gitaly_replace_wiki_delete_page
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/56495
+rollout_issue_url:
+milestone: '13.10'
+type: development
+group: group::editor
+default_enabled: false

spec/support/shared_examples/models/wiki_shared_examples.rb

@@ -481,6 +481,7 @@ RSpec.shared_examples 'wiki model' do
   end
 
   describe '#delete_page' do
+    shared_examples 'delete_page operations' do
       let(:page) { create(:wiki_page, wiki: wiki) }
 
       it 'deletes the page' do
@@ -506,6 +507,30 @@ RSpec.shared_examples 'wiki model' do
       end
     end
 
+    it_behaves_like 'delete_page operations'
+
+    context 'when an error is raised' do
+      it 'logs the error and returns false' do
+        page = build(:wiki_page, wiki: wiki)
+        exception = Gitlab::Git::Index::IndexError.new('foo')
+
+        allow(subject.repository).to receive(:delete_file).and_raise(exception)
+
+        expect(Gitlab::ErrorTracking).to receive(:log_exception).with(exception, action: :deleted, wiki_id: wiki.id)
+
+        expect(subject.delete_page(page)).to be_falsey
+      end
+    end
+
+    context 'when feature flag :gitaly_replace_wiki_delete_page is disabled' do
+      before do
+        stub_feature_flags(gitaly_replace_wiki_delete_page: false)
+      end
+
+      it_behaves_like 'delete_page operations'
+    end
+  end
+
   describe '#ensure_repository' do
     context 'if the repository exists' do
       it 'does not create the repository' do