Merge branch 'zj-pool-repository-creation' into 'master'

Allow public forks to be deduplicated

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

9 files changed, 322 insertions, 6 deletions

diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 7849bec4762..576191a5788 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -279,7 +279,7 @@ describe ProjectsController do
         expected_query = /#{public_project.fork_network.find_forks_in(other_user.namespace).to_sql}/
 
         expect { get(:show, namespace_id: public_project.namespace, id: public_project) }
-          .not_to exceed_query_limit(1).for_query(expected_query)
+          .not_to exceed_query_limit(2).for_query(expected_query)
       end
     end
   end
diff --git a/spec/factories/pool_repositories.rb b/spec/factories/pool_repositories.rb
index 2ed0844ed47..265a4643f46 100644
--- a/spec/factories/pool_repositories.rb
+++ b/spec/factories/pool_repositories.rb
@@ -1,5 +1,26 @@
 FactoryBot.define do
   factory :pool_repository do
-    shard
+    shard { Shard.by_name("default") }
+    state :none
+
+    before(:create) do |pool|
+      pool.source_project = create(:project, :repository)
+    end
+
+    trait :scheduled do
+      state :scheduled
+    end
+
+    trait :failed do
+      state :failed
+    end
+
+    trait :ready do
+      state :ready
+
+      after(:create) do |pool|
+        pool.create_object_pool
+      end
+    end
   end
 end
diff --git a/spec/lib/gitlab/git/object_pool_spec.rb b/spec/lib/gitlab/git/object_pool_spec.rb
new file mode 100644
index 00000000000..363c2aa67af
--- /dev/null
+++ b/spec/lib/gitlab/git/object_pool_spec.rb
@@ -0,0 +1,89 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Git::ObjectPool do
+  let(:pool_repository) { create(:pool_repository) }
+  let(:source_repository) { pool_repository.source_project.repository }
+
+  subject { pool_repository.object_pool }
+
+  describe '#storage' do
+    it "equals the pool repository's shard name" do
+      expect(subject.storage).not_to be_nil
+      expect(subject.storage).to eq(pool_repository.shard_name)
+    end
+  end
+
+  describe '#create' do
+    before do
+      subject.create
+    end
+
+    context "when the pool doesn't exist yet" do
+      it 'creates the pool' do
+        expect(subject.exists?).to be(true)
+      end
+    end
+
+    context 'when the pool already exists' do
+      it 'raises an FailedPrecondition' do
+        expect do
+          subject.create
+        end.to raise_error(GRPC::FailedPrecondition)
+      end
+    end
+  end
+
+  describe '#exists?' do
+    context "when the object pool doesn't exist" do
+      it 'returns false' do
+        expect(subject.exists?).to be(false)
+      end
+    end
+
+    context 'when the object pool exists' do
+      let(:pool) { create(:pool_repository, :ready) }
+
+      subject { pool.object_pool }
+
+      it 'returns true' do
+        expect(subject.exists?).to be(true)
+      end
+    end
+  end
+
+  describe '#link' do
+    let!(:pool_repository) { create(:pool_repository, :ready) }
+
+    context 'when no remotes are set' do
+      it 'sets a remote' do
+        subject.link(source_repository)
+
+        repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+          Rugged::Repository.new(subject.repository.path)
+        end
+
+        expect(repo.remotes.count).to be(1)
+        expect(repo.remotes.first.name).to eq(source_repository.object_pool_remote_name)
+      end
+    end
+
+    context 'when the remote is already set' do
+      before do
+        subject.link(source_repository)
+      end
+
+      it "doesn't raise an error" do
+        subject.link(source_repository)
+
+        repo = Gitlab::GitalyClient::StorageSettings.allow_disk_access do
+          Rugged::Repository.new(subject.repository.path)
+        end
+
+        expect(repo.remotes.count).to be(1)
+        expect(repo.remotes.first.name).to eq(source_repository.object_pool_remote_name)
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb b/spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb
new file mode 100644
index 00000000000..149b7ec5bb0
--- /dev/null
+++ b/spec/lib/gitlab/gitaly_client/object_pool_service_spec.rb
@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::GitalyClient::ObjectPoolService do
+  let(:pool_repository) { create(:pool_repository) }
+  let(:project) { create(:project, :repository) }
+  let(:raw_repository) { project.repository.raw }
+  let(:object_pool) { pool_repository.object_pool }
+
+  subject { described_class.new(object_pool) }
+
+  before do
+    subject.create(raw_repository)
+  end
+
+  describe '#create' do
+    it 'exists on disk' do
+      expect(object_pool.repository.exists?).to be(true)
+    end
+
+    context 'when the pool already exists' do
+      it 'returns an error' do
+        expect do
+          subject.create(raw_repository)
+        end.to raise_error(GRPC::FailedPrecondition)
+      end
+    end
+  end
+
+  describe '#delete' do
+    it 'removes the repository from disk' do
+      subject.delete
+
+      expect(object_pool.repository.exists?).to be(false)
+    end
+
+    context 'when called twice' do
+      it "doesn't raise an error" do
+        subject.delete
+
+        expect { object_pool.delete }.not_to raise_error
+      end
+    end
+  end
+end
diff --git a/spec/models/pool_repository_spec.rb b/spec/models/pool_repository_spec.rb
index 541e78507e5..3d3878b8c39 100644
--- a/spec/models/pool_repository_spec.rb
+++ b/spec/models/pool_repository_spec.rb
@@ -5,6 +5,7 @@ require 'spec_helper'
 describe PoolRepository do
   describe 'associations' do
     it { is_expected.to belong_to(:shard) }
+    it { is_expected.to have_one(:source_project) }
     it { is_expected.to have_many(:member_projects) }
   end
 
@@ -12,15 +13,14 @@ describe PoolRepository do
     let!(:pool_repository) { create(:pool_repository) }
 
     it { is_expected.to validate_presence_of(:shard) }
+    it { is_expected.to validate_presence_of(:source_project) }
   end
 
   describe '#disk_path' do
     it 'sets the hashed disk_path' do
       pool = create(:pool_repository)
 
-      elements = File.split(pool.disk_path)
-
-      expect(elements).to all( match(/\d{2,}/) )
+      expect(pool.disk_path).to match(%r{\A@pools/\h{2}/\h{2}/\h{64}})
     end
   end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 93c83fd21fd..1c85411dc3b 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -4092,6 +4092,44 @@ describe Project do
     end
   end
 
+  describe '#git_objects_poolable?' do
+    subject { project }
+
+    context 'when the feature flag is turned off' do
+      before do
+        stub_feature_flags(object_pools: false)
+      end
+
+      let(:project) { create(:project, :repository, :public) }
+
+      it { is_expected.not_to be_git_objects_poolable }
+    end
+
+    context 'when the feature flag is enabled' do
+      context 'when not using hashed storage' do
+        let(:project) { create(:project, :legacy_storage, :public, :repository) }
+
+        it { is_expected.not_to be_git_objects_poolable }
+      end
+
+      context 'when the project is not public' do
+        let(:project) { create(:project, :private) }
+
+        it { is_expected.not_to be_git_objects_poolable }
+      end
+
+      context 'when objects are poolable' do
+        let(:project) { create(:project, :repository, :public) }
+
+        before do
+          stub_application_setting(hashed_storage_enabled: true)
+        end
+
+        it { is_expected.to be_git_objects_poolable }
+      end
+    end
+  end
+
   def rugged_config
     rugged_repo(project.repository).config
   end
diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb
index a3d24ae312a..26e8d829345 100644
--- a/spec/services/projects/fork_service_spec.rb
+++ b/spec/services/projects/fork_service_spec.rb
@@ -2,7 +2,8 @@ require 'spec_helper'
 
 describe Projects::ForkService do
   include ProjectForksHelper
-  let(:gitlab_shell) { Gitlab::Shell.new }
+  include Gitlab::ShellAdapter
+
   context 'when forking a new project' do
     describe 'fork by user' do
       before do
@@ -235,6 +236,33 @@ describe Projects::ForkService do
     end
   end
 
+  context 'when forking with object pools' do
+    let(:fork_from_project) { create(:project, :public) }
+    let(:forker) { create(:user) }
+
+    before do
+      stub_feature_flags(object_pools: true)
+    end
+
+    context 'when no pool exists' do
+      it 'creates a new object pool' do
+        forked_project = fork_project(fork_from_project, forker)
+
+        expect(forked_project.pool_repository).to eq(fork_from_project.pool_repository)
+      end
+    end
+
+    context 'when a pool already exists' do
+      let!(:pool_repository) { create(:pool_repository, source_project: fork_from_project) }
+
+      it 'joins the object pool' do
+        forked_project = fork_project(fork_from_project, forker)
+
+        expect(forked_project.pool_repository).to eq(fork_from_project.pool_repository)
+      end
+    end
+  end
+
   context 'when linking fork to an existing project' do
     let(:fork_from_project) { create(:project, :public) }
     let(:fork_to_project) { create(:project, :public) }
diff --git a/spec/workers/object_pool/create_worker_spec.rb b/spec/workers/object_pool/create_worker_spec.rb
new file mode 100644
index 00000000000..06416489472
--- /dev/null
+++ b/spec/workers/object_pool/create_worker_spec.rb
@@ -0,0 +1,59 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ObjectPool::CreateWorker do
+  let(:pool) { create(:pool_repository, :scheduled) }
+
+  subject { described_class.new }
+
+  describe '#perform' do
+    context 'when the pool creation is successful' do
+      it 'marks the pool as ready' do
+        subject.perform(pool.id)
+
+        expect(pool.reload).to be_ready
+      end
+    end
+
+    context 'when a the pool already exists' do
+      before do
+        pool.create_object_pool
+      end
+
+      it 'cleans up the pool' do
+        expect do
+          subject.perform(pool.id)
+        end.to raise_error(GRPC::FailedPrecondition)
+
+        expect(pool.reload.failed?).to be(true)
+      end
+    end
+
+    context 'when the server raises an unknown error' do
+      before do
+        allow_any_instance_of(PoolRepository).to receive(:create_object_pool).and_raise(GRPC::Internal)
+      end
+
+      it 'marks the pool as failed' do
+        expect do
+          subject.perform(pool.id)
+        end.to raise_error(GRPC::Internal)
+
+        expect(pool.reload.failed?).to be(true)
+      end
+    end
+
+    context 'when the pool creation failed before' do
+      let(:pool) { create(:pool_repository, :failed) }
+
+      it 'deletes the pool first' do
+        expect_any_instance_of(PoolRepository).to receive(:delete_object_pool)
+
+        subject.perform(pool.id)
+
+        expect(pool.reload).to be_ready
+      end
+    end
+  end
+end
diff --git a/spec/workers/object_pool/join_worker_spec.rb b/spec/workers/object_pool/join_worker_spec.rb
new file mode 100644
index 00000000000..906bc22c8d2
--- /dev/null
+++ b/spec/workers/object_pool/join_worker_spec.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ObjectPool::JoinWorker do
+  let(:pool) { create(:pool_repository, :ready) }
+  let(:project) { pool.source_project }
+  let(:repository) { project.repository }
+
+  subject { described_class.new }
+
+  describe '#perform' do
+    context "when the pool is not joinable" do
+      let(:pool) { create(:pool_repository, :scheduled) }
+
+      it "doesn't raise an error" do
+        expect do
+          subject.perform(pool.id, project.id)
+        end.not_to raise_error
+      end
+    end
+
+    context 'when the pool has been joined before' do
+      before do
+        pool.link_repository(repository)
+      end
+
+      it 'succeeds in joining' do
+        expect do
+          subject.perform(pool.id, project.id)
+        end.not_to raise_error
+      end
+    end
+  end
+end