Merge branch 'zj-gitaly-migrate-old-migrations' into 'master'

Gitaly migrate old DB migrations

Closes gitaly#1106

See merge request gitlab-org/gitlab-ce!20708

4 files changed, 52 insertions, 84 deletions

diff --git a/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb b/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb
index dc16d5c5169..1eb6a8fa5df 100644
--- a/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb
+++ b/db/migrate/20161124141322_migrate_process_commit_worker_jobs.rb
@@ -1,9 +1,19 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
 class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
 
+  class Repository
+    attr_reader :storage
+
+    def initialize(storage, relative_path)
+      @storage = storage
+      @relative_path = relative_path
+    end
+
+    def gitaly_repository
+      Gitaly::Repository.new(storage_name: @storage, relative_path: @relative_path)
+    end
+  end
+
   class Project < ActiveRecord::Base
     def self.find_including_path(id)
       select("projects.*, CONCAT(namespaces.path, '/', projects.path) AS path_with_namespace")
@@ -11,19 +21,12 @@ class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration
         .find_by(id: id)
     end
 
-    def repository_storage_path
-      Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-        Gitlab.config.repositories.storages[repository_storage].legacy_disk_path
-      end
-    end
-
-    def repository_path
-      # TODO: review if the change from Legacy storage needs to reflect here as well.
-      File.join(repository_storage_path, read_attribute(:path_with_namespace) + '.git')
+    def commit(rev)
+      Gitlab::GitalyClient::CommitService.new(repository).find_commit(rev)
     end
 
     def repository
-      @repository ||= Rugged::Repository.new(repository_path)
+      @repository ||= Repository.new(repository_storage, read_attribute(:path_with_namespace) + '.git')
     end
   end
 
@@ -42,22 +45,19 @@ class MigrateProcessCommitWorkerJobs < ActiveRecord::Migration
 
         next unless project
 
-        begin
-          commit = project.repository.lookup(payload['args'][2])
-        rescue Rugged::OdbError
-          next
-        end
+        commit = project.commit(payload['args'][2])
+        next unless commit
 
         hash = {
-          id: commit.oid,
-          message: encode(commit.message),
-          parent_ids: commit.parent_ids,
-          authored_date: commit.author[:time],
-          author_name: encode(commit.author[:name]),
-          author_email: encode(commit.author[:email]),
-          committed_date: commit.committer[:time],
-          committer_email: encode(commit.committer[:email]),
-          committer_name: encode(commit.committer[:name])
+          id: commit.id,
+          message: encode(commit.body),
+          parent_ids: commit.parent_ids.to_a,
+          authored_date: Time.at(commit.author.date.seconds).utc,
+          author_name: encode(commit.author.name),
+          author_email: encode(commit.author.email),
+          committed_date: Time.at(commit.committer.date.seconds).utc,
+          committer_email: encode(commit.committer.email),
+          committer_name: encode(commit.committer.name)
         }
 
         payload['args'][2] = hash
diff --git a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb
index 133435523e1..db10426b483 100644
--- a/db/migrate/20161226122833_remove_dot_git_from_usernames.rb
+++ b/db/migrate/20161226122833_remove_dot_git_from_usernames.rb
@@ -1,11 +1,7 @@
-# See http://doc.gitlab.com/ce/development/migration_style_guide.html
-# for more information on how to write migrations for GitLab.
-
 class RemoveDotGitFromUsernames < ActiveRecord::Migration
   include Gitlab::Database::MigrationHelpers
   include Gitlab::ShellAdapter
 
-  # Set this constant to true if this migration requires downtime.
   DOWNTIME = false
 
   def up
@@ -64,16 +60,14 @@ class RemoveDotGitFromUsernames < ActiveRecord::Migration
     # we rename suffix instead of removing it
     path = path.sub(/\.git\z/, '_git')
 
-    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-      check_routes(path.dup, 0, path)
-    end
+    check_routes(path.dup, 0, path)
   end
 
   def check_routes(base, counter, path)
     route_exists = route_exists?(path)
 
-    Gitlab.config.repositories.storages.each do |shard, storage|
-      if route_exists || path_exists?(shard, storage.legacy_disk_path)
+    Gitlab.config.repositories.storages.each do |shard, _storage|
+      if route_exists || path_exists?(shard, path)
         counter += 1
         path = "#{base}#{counter}"
 
diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb
index f23ae1519ef..5b955753a92 100644
--- a/lib/gitlab/git/repository.rb
+++ b/lib/gitlab/git/repository.rb
@@ -1,4 +1,3 @@
-# Gitlab::Git::Repository is a wrapper around native Rugged::Repository object
 require 'tempfile'
 require 'forwardable'
 require "rubygems/package"
diff --git a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb b/spec/migrations/migrate_process_commit_worker_jobs_spec.rb
index a30e6c23ac9..6219a67c900 100644
--- a/spec/migrations/migrate_process_commit_worker_jobs_spec.rb
+++ b/spec/migrations/migrate_process_commit_worker_jobs_spec.rb
@@ -4,14 +4,11 @@ require 'spec_helper'
 require Rails.root.join('db', 'migrate', '20161124141322_migrate_process_commit_worker_jobs.rb')
 
 describe MigrateProcessCommitWorkerJobs do
-  let(:project) { create(:project, :legacy_storage, :repository) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
-  let(:user) { create(:user) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
-  let(:rugged) do
-    Gitlab::GitalyClient::StorageSettings.allow_disk_access do
-      project.repository.rugged
-    end
+  set(:project) { create(:project, :legacy_storage, :repository) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
+  set(:user) { create(:user) } # rubocop:disable RSpec/FactoriesInMigrationSpecs
+  let(:commit) do
+    Gitlab::Git::Commit.last(project.repository.raw)
   end
-  let(:commit) { rugged.rev_parse(project.commit.id) }
 
   describe 'Project' do
     describe 'find_including_path' do
@@ -29,32 +26,13 @@ describe MigrateProcessCommitWorkerJobs do
       end
     end
 
-    describe '#repository_storage_path' do
-      it 'returns the storage path for the repository' do
-        migration_project = described_class::Project
-          .find_including_path(project.id)
-
-        expect(File.directory?(migration_project.repository_storage_path))
-          .to eq(true)
-      end
-    end
-
-    describe '#repository_path' do
-      it 'returns the path to the repository' do
-        migration_project = described_class::Project
-          .find_including_path(project.id)
-
-        expect(File.directory?(migration_project.repository_path)).to eq(true)
-      end
-    end
-
     describe '#repository' do
-      it 'returns a Rugged::Repository' do
+      it 'returns a mock implemention of ::Repository' do
         migration_project = described_class::Project
           .find_including_path(project.id)
 
-        expect(migration_project.repository)
-          .to be_an_instance_of(Rugged::Repository)
+        expect(migration_project.repository).to respond_to(:storage)
+        expect(migration_project.repository).to respond_to(:gitaly_repository)
       end
     end
   end
@@ -72,7 +50,7 @@ describe MigrateProcessCommitWorkerJobs do
 
     before do
       Sidekiq.redis do |redis|
-        job = JSON.dump(args: [project.id, user.id, commit.oid])
+        job = JSON.dump(args: [project.id, user.id, commit.id])
         redis.lpush('queue:process_commit', job)
       end
     end
@@ -88,9 +66,10 @@ describe MigrateProcessCommitWorkerJobs do
     end
 
     it 'skips jobs using commits that no longer exist' do
-      allow_any_instance_of(Rugged::Repository).to receive(:lookup)
-        .with(commit.oid)
-        .and_raise(Rugged::OdbError)
+      allow_any_instance_of(Gitlab::GitalyClient::CommitService)
+        .to receive(:find_commit)
+        .with(commit.id)
+        .and_return(nil)
 
       migration.up
 
@@ -104,11 +83,7 @@ describe MigrateProcessCommitWorkerJobs do
     end
 
     it 'encodes data to UTF-8' do
-      allow_any_instance_of(Rugged::Repository).to receive(:lookup)
-        .with(commit.oid)
-        .and_return(commit)
-
-      allow(commit).to receive(:message)
+      allow(commit).to receive(:body)
         .and_return('김치'.force_encoding('BINARY'))
 
       migration.up
@@ -140,7 +115,7 @@ describe MigrateProcessCommitWorkerJobs do
       end
 
       it 'includes the commit ID' do
-        expect(commit_hash['id']).to eq(commit.oid)
+        expect(commit_hash['id']).to eq(commit.id)
       end
 
       it 'includes the commit message' do
@@ -152,27 +127,27 @@ describe MigrateProcessCommitWorkerJobs do
       end
 
       it 'includes the author date' do
-        expect(commit_hash['authored_date']).to eq(commit.author[:time].to_s)
+        expect(commit_hash['authored_date']).to eq(commit.authored_date.to_s)
       end
 
       it 'includes the author name' do
-        expect(commit_hash['author_name']).to eq(commit.author[:name])
+        expect(commit_hash['author_name']).to eq(commit.author_name)
       end
 
       it 'includes the author Email' do
-        expect(commit_hash['author_email']).to eq(commit.author[:email])
+        expect(commit_hash['author_email']).to eq(commit.author_email)
       end
 
       it 'includes the commit date' do
-        expect(commit_hash['committed_date']).to eq(commit.committer[:time].to_s)
+        expect(commit_hash['committed_date']).to eq(commit.committed_date.to_s)
       end
 
       it 'includes the committer name' do
-        expect(commit_hash['committer_name']).to eq(commit.committer[:name])
+        expect(commit_hash['committer_name']).to eq(commit.committer_name)
       end
 
       it 'includes the committer Email' do
-        expect(commit_hash['committer_email']).to eq(commit.committer[:email])
+        expect(commit_hash['committer_email']).to eq(commit.committer_email)
       end
     end
   end
@@ -186,7 +161,7 @@ describe MigrateProcessCommitWorkerJobs do
 
     before do
       Sidekiq.redis do |redis|
-        job = JSON.dump(args: [project.id, user.id, commit.oid])
+        job = JSON.dump(args: [project.id, user.id, commit.id])
         redis.lpush('queue:process_commit', job)
 
         migration.up
@@ -215,7 +190,7 @@ describe MigrateProcessCommitWorkerJobs do
       end
 
       it 'includes the commit SHA' do
-        expect(job['args'][2]).to eq(commit.oid)
+        expect(job['args'][2]).to eq(commit.id)
       end
     end
   end