Place project_id FK in project_repositories tabletc-repo-full-path-in-db

Instead of having the foreign key `repository_id` in the `projects`
table, have a FK in the `project_repositories` table. This will:

- Reduce the size of `projects` table, which is huge already.
- Make the foreign key constraint `on_delete: :cascade` useful,
  because the `project_repositories` row will be deleted when
  `projects` row is deleted.
- Since the `project_repositories` row can only be created **after**
  the `projects` row (because hashed storage needs a `projects.id`),
  it does not require updating the `projects` row after creating the
  `project_repositories`.
- Make it easier in the future to have a N-to-M relation.

6 files changed, 9 insertions, 19 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 227c670efcd..cbb5dc5c99f 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -125,7 +125,6 @@ class Project < ActiveRecord::Base
   alias_attribute :title, :name
 
   # Relations
-  belongs_to :project_repository, foreign_key: :repository_id
   belongs_to :pool_repository
   belongs_to :creator, class_name: 'User'
   belongs_to :group, -> { where(type: 'Group') }, foreign_key: 'namespace_id'
@@ -183,6 +182,7 @@ class Project < ActiveRecord::Base
 
   has_one :import_state, autosave: true, class_name: 'ProjectImportState', inverse_of: :project
   has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
+  has_one :project_repository
 
   # Merge Requests for target project should be removed with it
   has_many :merge_requests, foreign_key: 'target_project_id', inverse_of: :target_project
@@ -1323,13 +1323,7 @@ class Project < ActiveRecord::Base
     return unless hashed_storage?(:repository)
 
     project_repo = project_repository || build_project_repository
-    project_repo.assign_attributes(shard_name: repository_storage, disk_path: disk_path)
-
-    if project_repo.persisted?
-      project_repo.save
-    else
-      update(project_repository: project_repo)
-    end
+    project_repo.update(shard_name: repository_storage, disk_path: disk_path)
   end
 
   def create_repository(force: false)
diff --git a/app/models/project_repository.rb b/app/models/project_repository.rb
index 17e6f79c1e4..d0202b97ce4 100644
--- a/app/models/project_repository.rb
+++ b/app/models/project_repository.rb
@@ -3,7 +3,7 @@
 class ProjectRepository < ActiveRecord::Base
   include RepositoryOnShard
 
-  has_one :project, foreign_key: :repository_id
+  belongs_to :project
 
   class << self
     def find_project(disk_path)
diff --git a/db/migrate/20181122160027_create_project_repositories.rb b/db/migrate/20181122160027_create_project_repositories.rb
index 3f934a13882..8475c8b3840 100644
--- a/db/migrate/20181122160027_create_project_repositories.rb
+++ b/db/migrate/20181122160027_create_project_repositories.rb
@@ -12,11 +12,7 @@ class CreateProjectRepositories < ActiveRecord::Migration[5.0]
     create_table :project_repositories, id: :bigserial do |t|
       t.references :shard, null: false, index: true, foreign_key: { on_delete: :restrict }
       t.string :disk_path, null: false, index: { unique: true }
+      t.references :project, null: false, index: true, foreign_key: { on_delete: :cascade }
     end
-
-    add_reference :projects, :repository,
-                  index: { where: 'repository_id IS NOT NULL' },
-                  type: :bigint,
-                  foreign_key: { to_table: :project_repositories }
   end
 end
diff --git a/db/schema.rb b/db/schema.rb
index e1e0f65496f..480566bd6d8 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -1585,7 +1585,9 @@ ActiveRecord::Schema.define(version: 20181122160027) do
   create_table "project_repositories", id: :bigserial, force: :cascade do |t|
     t.integer "shard_id", null: false
     t.string "disk_path", null: false
+    t.integer "project_id", null: false
     t.index ["disk_path"], name: "index_project_repositories_on_disk_path", unique: true, using: :btree
+    t.index ["project_id"], name: "index_project_repositories_on_project_id", using: :btree
     t.index ["shard_id"], name: "index_project_repositories_on_shard_id", using: :btree
   end
 
@@ -1654,7 +1656,6 @@ ActiveRecord::Schema.define(version: 20181122160027) do
     t.boolean "pages_https_only", default: true
     t.boolean "remote_mirror_available_overridden"
     t.bigint "pool_repository_id"
-    t.bigint "repository_id"
     t.index ["ci_id"], name: "index_projects_on_ci_id", using: :btree
     t.index ["created_at"], name: "index_projects_on_created_at", using: :btree
     t.index ["creator_id"], name: "index_projects_on_creator_id", using: :btree
@@ -1670,7 +1671,6 @@ ActiveRecord::Schema.define(version: 20181122160027) do
     t.index ["path"], name: "index_projects_on_path_trigram", using: :gin, opclasses: {"path"=>"gin_trgm_ops"}
     t.index ["pending_delete"], name: "index_projects_on_pending_delete", using: :btree
     t.index ["pool_repository_id"], name: "index_projects_on_pool_repository_id", where: "(pool_repository_id IS NOT NULL)", using: :btree
-    t.index ["repository_id"], name: "index_projects_on_repository_id", where: "(repository_id IS NOT NULL)", using: :btree
     t.index ["repository_storage", "created_at"], name: "idx_project_repository_check_partial", where: "(last_repository_check_at IS NULL)", using: :btree
     t.index ["repository_storage"], name: "index_projects_on_repository_storage", using: :btree
     t.index ["runners_token"], name: "index_projects_on_runners_token", using: :btree
@@ -2384,10 +2384,10 @@ ActiveRecord::Schema.define(version: 20181122160027) do
   add_foreign_key "project_group_links", "projects", name: "fk_daa8cee94c", on_delete: :cascade
   add_foreign_key "project_import_data", "projects", name: "fk_ffb9ee3a10", on_delete: :cascade
   add_foreign_key "project_mirror_data", "projects", on_delete: :cascade
+  add_foreign_key "project_repositories", "projects", on_delete: :cascade
   add_foreign_key "project_repositories", "shards", on_delete: :restrict
   add_foreign_key "project_statistics", "projects", on_delete: :cascade
   add_foreign_key "projects", "repositories", column: "pool_repository_id", name: "fk_6e5c14658a", on_delete: :nullify
-  add_foreign_key "projects", "project_repositories", column: "repository_id"
   add_foreign_key "prometheus_metrics", "projects", on_delete: :cascade
   add_foreign_key "protected_branch_merge_access_levels", "protected_branches", name: "fk_8a3072ccb3", on_delete: :cascade
   add_foreign_key "protected_branch_push_access_levels", "protected_branches", name: "fk_9ffc86a3d9", on_delete: :cascade
diff --git a/spec/models/project_repository_spec.rb b/spec/models/project_repository_spec.rb
index b5e581bd56b..405d6810871 100644
--- a/spec/models/project_repository_spec.rb
+++ b/spec/models/project_repository_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
 describe ProjectRepository do
   describe 'associations' do
     it { is_expected.to belong_to(:shard) }
-    it { is_expected.to have_one(:project) }
+    it { is_expected.to belong_to(:project) }
   end
 
   describe '.find_project' do
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 286785502f5..73e68aae68d 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -8,7 +8,6 @@ describe Project do
     it { is_expected.to belong_to(:group) }
     it { is_expected.to belong_to(:namespace) }
     it { is_expected.to belong_to(:creator).class_name('User') }
-    it { is_expected.to belong_to(:project_repository) }
     it { is_expected.to belong_to(:pool_repository) }
     it { is_expected.to have_many(:users) }
     it { is_expected.to have_many(:services) }
@@ -55,6 +54,7 @@ describe Project do
     it { is_expected.to have_one(:gitlab_issue_tracker_service) }
     it { is_expected.to have_one(:external_wiki_service) }
     it { is_expected.to have_one(:project_feature) }
+    it { is_expected.to have_one(:project_repository) }
     it { is_expected.to have_one(:statistics).class_name('ProjectStatistics') }
     it { is_expected.to have_one(:import_data).class_name('ProjectImportData') }
     it { is_expected.to have_one(:last_event).class_name('Event') }