Merge branch 'fj-39202-snippet-migration' into 'master'

Version Control for Snippets: Development of the Migration Plan (MVP)

See merge request gitlab-org/gitlab!25905

app/models/snippet_repository.rb

@@ -18,12 +18,6 @@ class SnippetRepository < ApplicationRecord
     end
   end
 
-  def create_file(user, path, content, **options)
-    options[:actions] = transform_file_entries([{ file_path: path, content: content }])
-
-    capture_git_error { repository.multi_action(user, **options) }
-  end
-
   def multi_files_action(user, files = [], **options)
     return if files.nil? || files.empty?
 

lib/gitlab/background_migration/backfill_snippet_repositories.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module BackgroundMigration
+    # Class that will fill the project_repositories table for projects that
+    # are on hashed storage and an entry is missing in this table.
+    class BackfillSnippetRepositories
+      MAX_RETRIES = 2
+
+      def perform(start_id, stop_id)
+        Snippet.includes(:author, snippet_repository: :shard).where(id: start_id..stop_id).find_each do |snippet|
+          # We need to expire the exists? value for the cached method in case it was cached
+          snippet.repository.expire_exists_cache
+
+          next if repository_present?(snippet)
+
+          retry_index = 0
+
+          begin
+            create_repository_and_files(snippet)
+
+            logger.info(message: 'Snippet Migration: repository created and migrated', snippet: snippet.id)
+          rescue => e
+            retry_index += 1
+
+            retry if retry_index < MAX_RETRIES
+
+            logger.error(message: "Snippet Migration: error migrating snippet. Reason: #{e.message}", snippet: snippet.id)
+
+            destroy_snippet_repository(snippet)
+            delete_repository(snippet)
+          end
+        end
+      end
+
+      private
+
+      def repository_present?(snippet)
+        snippet.snippet_repository && !snippet.empty_repo?
+      end
+
+      def create_repository_and_files(snippet)
+        snippet.create_repository
+        create_commit(snippet)
+      end
+
+      def destroy_snippet_repository(snippet)
+        # Removing the db record
+        snippet.snippet_repository&.destroy
+      rescue => e
+        logger.error(message: "Snippet Migration: error destroying snippet repository. Reason: #{e.message}", snippet: snippet.id)
+      end
+
+      def delete_repository(snippet)
+        # Removing the repository in disk
+        snippet.repository.remove if snippet.repository_exists?
+      rescue => e
+        logger.error(message: "Snippet Migration: error deleting repository. Reason: #{e.message}", snippet: snippet.id)
+      end
+
+      def logger
+        @logger ||= Gitlab::BackgroundMigration::Logger.build
+      end
+
+      def snippet_action(snippet)
+        # We don't need the previous_path param
+        # Because we're not updating any existing file
+        [{ file_path: filename(snippet),
+           content: snippet.content }]
+      end
+
+      def filename(snippet)
+        snippet.file_name.presence || empty_file_name
+      end
+
+      def empty_file_name
+        @empty_file_name ||= "#{SnippetRepository::DEFAULT_EMPTY_FILE_NAME}1.txt"
+      end
+
+      def commit_attrs
+        @commit_attrs ||= { branch_name: 'master', message: 'Initial commit' }
+      end
+
+      def create_commit(snippet)
+        snippet.snippet_repository.multi_files_action(snippet.author, snippet_action(snippet), commit_attrs)
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, schema: 2020_02_26_162723 do
+  let(:gitlab_shell) { Gitlab::Shell.new }
+  let(:users) { table(:users) }
+  let(:snippets) { table(:snippets) }
+  let(:snippet_repositories) { table(:snippet_repositories) }
+
+  let(:user) { users.create(id: 1, email: 'user@example.com', projects_limit: 10, username: 'test', name: 'Test') }
+  let!(:snippet_with_repo) { snippets.create(id: 1, type: 'PersonalSnippet', author_id: user.id, file_name: file_name, content: content) }
+  let!(:snippet_with_empty_repo) { snippets.create(id: 2, type: 'PersonalSnippet', author_id: user.id, file_name: file_name, content: content) }
+  let!(:snippet_without_repo) { snippets.create(id: 3, type: 'PersonalSnippet', author_id: user.id, file_name: file_name, content: content) }
+
+  let(:file_name) { 'file_name.rb' }
+  let(:content) { 'content' }
+  let(:ids) { snippets.pluck('MIN(id)', 'MAX(id)').first }
+  let(:service) { described_class.new }
+
+  subject { service.perform(*ids) }
+
+  before do
+    allow(snippet_with_repo).to receive(:disk_path).and_return(disk_path(snippet_with_repo))
+
+    TestEnv.copy_repo(snippet_with_repo,
+                      bare_repo: TestEnv.factory_repo_path_bare,
+                      refs: TestEnv::BRANCH_SHA)
+
+    raw_repository(snippet_with_empty_repo).create_repository
+  end
+
+  after do
+    raw_repository(snippet_with_repo).remove
+    raw_repository(snippet_without_repo).remove
+    raw_repository(snippet_with_empty_repo).remove
+  end
+
+  describe '#perform' do
+    it 'logs successful migrated snippets' do
+      expect_next_instance_of(Gitlab::BackgroundMigration::Logger) do |instance|
+        expect(instance).to receive(:info).exactly(3).times
+      end
+
+      subject
+    end
+
+    context 'when snippet has a non empty repository' do
+      it 'does not perform any action' do
+        expect(service).not_to receive(:create_repository_and_files).with(snippet_with_repo)
+
+        subject
+      end
+    end
+
+    shared_examples 'commits the file to the repository' do
+      it do
+        subject
+
+        blob = blob_at(snippet, file_name)
+
+        aggregate_failures do
+          expect(blob).to be
+          expect(blob.data).to eq content
+        end
+      end
+    end
+
+    context 'when snippet has an empty repo' do
+      before do
+        expect(repository_exists?(snippet_with_empty_repo)).to be_truthy
+      end
+
+      it_behaves_like 'commits the file to the repository' do
+        let(:snippet) { snippet_with_empty_repo }
+      end
+    end
+
+    context 'when snippet does not have a repository' do
+      it 'creates the repository' do
+        expect { subject }.to change { repository_exists?(snippet_without_repo) }.from(false).to(true)
+      end
+
+      it_behaves_like 'commits the file to the repository' do
+        let(:snippet) { snippet_without_repo }
+      end
+    end
+
+    context 'when an error is raised' do
+      before do
+        allow(service).to receive(:create_commit).and_raise(StandardError)
+      end
+
+      it 'logs errors' do
+        expect_next_instance_of(Gitlab::BackgroundMigration::Logger) do |instance|
+          expect(instance).to receive(:error).exactly(3).times
+        end
+
+        subject
+      end
+
+      it "retries #{described_class::MAX_RETRIES} times the operation if it fails" do
+        expect(service).to receive(:create_commit).exactly(snippets.count * described_class::MAX_RETRIES).times
+
+        subject
+      end
+
+      it 'destroys the snippet repository' do
+        expect(service).to receive(:destroy_snippet_repository).exactly(3).times.and_call_original
+
+        subject
+
+        expect(snippet_repositories.count).to eq 0
+      end
+
+      it 'deletes the repository on disk' do
+        subject
+
+        aggregate_failures do
+          expect(repository_exists?(snippet_with_repo)).to be_falsey
+          expect(repository_exists?(snippet_without_repo)).to be_falsey
+          expect(repository_exists?(snippet_with_empty_repo)).to be_falsey
+        end
+      end
+    end
+  end
+
+  def blob_at(snippet, path)
+    raw_repository(snippet).blob_at('master', path)
+  end
+
+  def repository_exists?(snippet)
+    gitlab_shell.repository_exists?('default', "#{disk_path(snippet)}.git")
+  end
+
+  def raw_repository(snippet)
+    Gitlab::Git::Repository.new('default',
+                                "#{disk_path(snippet)}.git",
+                                Gitlab::GlRepository::SNIPPET.identifier_for_container(snippet),
+                                "@snippets/#{snippet.id}")
+  end
+
+  def hashed_repository(snippet)
+    Storage::Hashed.new(snippet, prefix: '@snippets')
+  end
+
+  def disk_path(snippet)
+    hashed_repository(snippet).disk_path
+  end
+
+  def ls_files(snippet)
+    raw_repository(snippet).ls_files(nil)
+  end
+end

spec/models/snippet_repository_spec.rb

@@ -26,44 +26,6 @@ describe SnippetRepository do
     end
   end
 
-  describe '#create_file' do
-    let(:snippet) { create(:personal_snippet, :empty_repo, author: user) }
-
-    it 'creates the file' do
-      snippet_repository.create_file(user, 'foo', 'bar', commit_opts)
-      blob = first_blob(snippet)
-
-      aggregate_failures do
-        expect(blob).not_to be_nil
-        expect(blob.path).to eq 'foo'
-        expect(blob.data).to eq 'bar'
-      end
-    end
-
-    it 'fills the file path if empty' do
-      snippet_repository.create_file(user, nil, 'bar', commit_opts)
-      blob = first_blob(snippet)
-
-      aggregate_failures do
-        expect(blob).not_to be_nil
-        expect(blob.path).to eq 'snippetfile1.txt'
-        expect(blob.data).to eq 'bar'
-      end
-    end
-
-    context 'when the file exists' do
-      let(:snippet) { create(:personal_snippet, :repository, author: user) }
-
-      it 'captures the git exception and raises a SnippetRepository::CommitError' do
-        existing_blob = first_blob(snippet)
-
-        expect do
-          snippet_repository.create_file(user, existing_blob.path, existing_blob.data, commit_opts)
-        end.to raise_error described_class::CommitError
-      end
-    end
-  end
-
   describe '#multi_files_action' do
     let(:new_file) { { file_path: 'new_file_test', content: 'bar' } }
     let(:move_file) { { previous_path: 'CHANGELOG', file_path: 'CHANGELOG_new', content: 'bar' } }