Merge branch 'fj-208904-add-snippet-backfilling-repository-migration' into 'master'

Add Snippet repository backfiling background migration

See merge request gitlab-org/gitlab!29927

changelogs/unreleased/fj-208904-add-snippet-backfilling-repository-migration.yml

+---
+title: Add snippet repository backfilling migration
+merge_request: 29927
+author:
+type: other

db/post_migrate/20200420094444_backfill_snippet_repositories.rb

+# frozen_string_literal: true
+
+class BackfillSnippetRepositories < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  INTERVAL = 3.minutes
+  BATCH_SIZE = 100
+  MIGRATION = 'BackfillSnippetRepositories'
+
+  disable_ddl_transaction!
+
+  class Snippet < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'snippets'
+    self.inheritance_column = :_type_disabled
+  end
+
+  def up
+    queue_background_migration_jobs_by_range_at_intervals(Snippet,
+                                                          MIGRATION,
+                                                          INTERVAL,
+                                                          batch_size: BATCH_SIZE)
+  end
+
+  def down
+    # no-op
+  end
+end

db/structure.sql

@@ -13545,6 +13545,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200417044453
 20200417145946
 20200420092011
+20200420094444
 20200420104303
 20200420104323
 20200420162730

lib/gitlab/background_migration/backfill_snippet_repositories.rb

@@ -44,16 +44,19 @@ module Gitlab
         create_commit(snippet)
       end
 
-      def destroy_snippet_repository(snippet)
       # Removing the db record
-        snippet.snippet_repository&.destroy
+      def destroy_snippet_repository(snippet)
+        snippet.snippet_repository&.delete
       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?
+      def delete_repository(snippet)
+        return unless snippet.repository_exists?
+
+        snippet.repository.remove
+        snippet.repository.expire_exists_cache
       rescue => e
         logger.error(message: "Snippet Migration: error deleting repository. Reason: #{e.message}", snippet: snippet.id)
       end
@@ -82,7 +85,24 @@ module Gitlab
       end
 
       def create_commit(snippet)
-        snippet.snippet_repository.multi_files_action(snippet.author, snippet_action(snippet), commit_attrs)
+        snippet.snippet_repository.multi_files_action(commit_author(snippet), snippet_action(snippet), commit_attrs)
+      end
+
+      # If the user is not allowed to access git or update the snippet
+      # because it is blocked, internal, ghost, ... we cannot commit
+      # files because these users are not allowed to, but we need to
+      # migrate their snippets as well.
+      # In this scenario an admin user will be the one that will commit the files.
+      def commit_author(snippet)
+        if Gitlab::UserAccessSnippet.new(snippet.author, snippet: snippet).can_do_action?(:update_snippet)
+          snippet.author
+        else
+          admin_user
+        end
+      end
+
+      def admin_user
+        @admin_user ||= User.admins.active.first
       end
     end
   end

spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb

@@ -2,13 +2,30 @@
 
 require 'spec_helper'
 
-describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, schema: 2020_02_26_162723 do
+describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, schema: 2020_04_20_094444 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(:user_state) { 'active' }
+  let(:ghost) { false }
+  let(:user_type) { nil }
+
+  let!(:user) do
+    users.create(id: 1,
+                 email: 'user@example.com',
+                 projects_limit: 10,
+                 username: 'test',
+                 name: 'Test',
+                 state: user_state,
+                 ghost: ghost,
+                 last_activity_on: 1.minute.ago,
+                 user_type: user_type,
+                 confirmed_at: 1.day.ago)
+  end
+
+  let!(:admin) { users.create(id: 2, email: 'admin@example.com', projects_limit: 10, username: 'admin', name: 'Admin', admin: true, state: 'active') }
   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) }
@@ -54,14 +71,51 @@ describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, s
     end
 
     shared_examples 'commits the file to the repository' do
-      it do
+      context 'when author can update snippet and use git' do
+        it 'creates the repository and commit the file' do
           subject
 
           blob = blob_at(snippet, file_name)
+          last_commit = raw_repository(snippet).commit
 
           aggregate_failures do
             expect(blob).to be
             expect(blob.data).to eq content
+            expect(last_commit.author_name).to eq user.name
+            expect(last_commit.author_email).to eq user.email
+          end
+        end
+      end
+
+      context 'when author cannot update snippet or use git' do
+        shared_examples 'admin user commits files' do
+          it do
+            subject
+
+            last_commit = raw_repository(snippet).commit
+
+            expect(last_commit.author_name).to eq admin.name
+            expect(last_commit.author_email).to eq admin.email
+          end
+        end
+
+        context 'when user is blocked' do
+          let(:user_state) { 'blocked' }
+
+          it_behaves_like 'admin user commits files'
+        end
+
+        context 'when user is deactivated' do
+          let(:user_state) { 'deactivated' }
+
+          it_behaves_like 'admin user commits files'
+        end
+
+        context 'when user is a ghost' do
+          let(:ghost) { true }
+          let(:user_type) { 'ghost' }
+
+          it_behaves_like 'admin user commits files'
         end
       end
     end

spec/migrations/backfill_snippet_repositories_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20200420094444_backfill_snippet_repositories.rb')
+
+describe BackfillSnippetRepositories do
+  let(:users) { table(:users) }
+  let(:snippets) { table(:snippets) }
+  let(:user) { users.create(id: 1, email: 'user@example.com', projects_limit: 10, username: 'test', name: 'Test', state: 'active') }
+
+  def create_snippet(id)
+    params = {
+      id: id,
+      type: 'PersonalSnippet',
+      author_id: user.id,
+      file_name: 'foo',
+      content: 'bar'
+    }
+
+    snippets.create!(params)
+  end
+
+  it 'correctly schedules background migrations' do
+    create_snippet(1)
+    create_snippet(2)
+    create_snippet(3)
+
+    stub_const("#{described_class.name}::BATCH_SIZE", 2)
+
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(described_class::MIGRATION)
+          .to be_scheduled_delayed_migration(3.minutes, 1, 2)
+
+        expect(described_class::MIGRATION)
+          .to be_scheduled_delayed_migration(6.minutes, 3, 3)
+
+        expect(BackgroundMigrationWorker.jobs.size).to eq(2)
+      end
+    end
+  end
+end