Merge branch '3354-add-unique-index-on-project-id-to-geo-project-registry-table' into 'master'

Add a unique index on project_id to the Geo project_registry table

Closes #3354

See merge request !2850

app/models/geo/project_registry.rb

 class Geo::ProjectRegistry < Geo::BaseRegistry
   belongs_to :project
 
-  validates :project, presence: true
+  validates :project, presence: true, uniqueness: true
 
   scope :dirty, -> { where(arel_table[:resync_repository].eq(true).or(arel_table[:resync_wiki].eq(true))) }
 

changelogs/unreleased-ee/3354-add-unique-index-on-project-id-to-geo-project-registry-table.yml

+---
+title: Geo - Add a unique index on project_id to the Geo project_registry table
+merge_request: 2850
+author:
+type: fixed

db/geo/migrate/20170906174622_remove_duplicates_from_project_registry.rb

+class RemoveDuplicatesFromProjectRegistry < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    execute <<-SQL
+      WITH good_rows AS (
+        SELECT project_id, MAX(id) as max_id
+        FROM project_registry
+        GROUP BY project_id
+        HAVING COUNT(*) > 1
+      )
+      DELETE FROM project_registry duplicated_rows
+      USING good_rows
+      WHERE good_rows.project_id = duplicated_rows.project_id
+        AND good_rows.max_id <> duplicated_rows.id;
+    SQL
+  end
+
+  def down
+  end
+end

db/geo/migrate/20170906182752_add_unique_index_to_project_id_on_project_registry.rb

+class AddUniqueIndexToProjectIdOnProjectRegistry < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    remove_concurrent_index :project_registry, :project_id if index_exists? :project_registry, :project_id
+    add_concurrent_index :project_registry, :project_id, unique: true
+  end
+
+  def down
+    remove_concurrent_index :project_registry, :project_id if index_exists? :project_registry, :project_id
+    add_concurrent_index :project_registry, :project_id
+  end
+end

db/geo/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170627195211) do
+ActiveRecord::Schema.define(version: 20170906182752) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -43,7 +43,7 @@ ActiveRecord::Schema.define(version: 20170627195211) do
 
   add_index "project_registry", ["last_repository_successful_sync_at"], name: "index_project_registry_on_last_repository_successful_sync_at", using: :btree
   add_index "project_registry", ["last_repository_synced_at"], name: "index_project_registry_on_last_repository_synced_at", using: :btree
-  add_index "project_registry", ["project_id"], name: "index_project_registry_on_project_id", using: :btree
+  add_index "project_registry", ["project_id"], name: "index_project_registry_on_project_id", unique: true, using: :btree
   add_index "project_registry", ["resync_repository"], name: "index_project_registry_on_resync_repository", using: :btree
   add_index "project_registry", ["resync_wiki"], name: "index_project_registry_on_resync_wiki", using: :btree
 

spec/models/geo/project_registry_spec.rb

 require 'spec_helper'
 
 describe Geo::ProjectRegistry do
+  subject { create(:geo_project_registry) }
+
   describe 'relationships' do
     it { is_expected.to belong_to(:project) }
   end
 
   describe 'validations' do
     it { is_expected.to validate_presence_of(:project) }
+    it { is_expected.to validate_uniqueness_of(:project) }
   end
 
   describe '.failed' do
     it 'returns projects where last attempt to sync failed' do
-      project = create(:project)
-      create(:geo_project_registry, :synced, project: project)
-      create(:geo_project_registry, :synced, :dirty, project: project)
-      repository_sync_failed = create(:geo_project_registry, :repository_sync_failed, project: project)
-      wiki_sync_failed = create(:geo_project_registry, :wiki_sync_failed, project: project)
+      create(:geo_project_registry, :synced)
+      create(:geo_project_registry, :synced, :dirty)
+      repository_sync_failed = create(:geo_project_registry, :repository_sync_failed)
+      wiki_sync_failed = create(:geo_project_registry, :wiki_sync_failed)
 
       expect(described_class.failed).to match_array([repository_sync_failed, wiki_sync_failed])
     end
@@ -23,10 +25,9 @@ describe Geo::ProjectRegistry do
 
   describe '.synced' do
     it 'returns synced projects' do
-      project = create(:project)
-      create(:geo_project_registry, :synced, :dirty, project: project)
-      create(:geo_project_registry, :sync_failed, project: project)
-      synced_project = create(:geo_project_registry, :synced, project: project)
+      create(:geo_project_registry, :synced, :dirty)
+      create(:geo_project_registry, :sync_failed)
+      synced_project = create(:geo_project_registry, :synced)
 
       expect(described_class.synced).to match_array([synced_project])
     end