Merge branch 'jv-dedup-activerecord' into 'master'

Fix wrong use of ActiveRecord in PoolRepository

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

app/models/pool_repository.rb

@@ -7,7 +7,7 @@ class PoolRepository < ApplicationRecord
   include Shardable
   include AfterCommitQueue
 
-  has_one :source_project, class_name: 'Project'
+  belongs_to :source_project, class_name: 'Project'
   validates :source_project, presence: true
 
   has_many :member_projects, class_name: 'Project'
@@ -99,7 +99,8 @@ class PoolRepository < ApplicationRecord
   end
 
   def inspect
-    "#<#{self.class.name} id:#{id} state:#{state} disk_path:#{disk_path} source_project: #{source_project.full_path}>"
+    source = source_project ? source_project.full_path : 'nil'
+    "#<#{self.class.name} id:#{id} state:#{state} disk_path:#{disk_path} source_project: #{source}>"
   end
 
   private

app/models/project.rb

@@ -2131,13 +2131,11 @@ class Project < ApplicationRecord
   end
 
   def create_new_pool_repository
-    pool = begin
-             create_pool_repository!(shard: Shard.by_name(repository_storage), source_project: self)
-           rescue ActiveRecord::RecordNotUnique
-             pool_repository(true)
-           end
+    pool = PoolRepository.safe_find_or_create_by!(shard: Shard.by_name(repository_storage), source_project: self)
+    update!(pool_repository: pool)
 
     pool.schedule unless pool.scheduled?
+
     pool
   end
 

changelogs/unreleased/jv-dedup-activerecord.yml

+---
+title: Fix wrong use of ActiveRecord in PoolRepository
+merge_request: 27464
+author:
+type: fixed
+

spec/factories/pool_repositories.rb

@@ -5,6 +5,7 @@ FactoryBot.define do
 
     before(:create) do |pool|
       pool.source_project = create(:project, :repository)
+      pool.source_project.update!(pool_repository: pool)
     end
 
     trait :scheduled do

spec/models/pool_repository_spec.rb

@@ -5,7 +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 belong_to(:source_project) }
     it { is_expected.to have_many(:member_projects) }
   end
 

spec/models/project_spec.rb

@@ -4702,6 +4702,8 @@ describe Project do
       it 'returns that pool repository' do
         expect(subject).not_to be_empty
         expect(subject[:pool_repository]).to be_persisted
+
+        expect(project.reload.pool_repository).to eq(subject[:pool_repository])
       end
     end
   end