Merge branch 'bvl-delete-empty-fork-networks' into 'master'

Delete empty fork networks

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

app/models/fork_network_member.rb

@@ -4,4 +4,14 @@ class ForkNetworkMember < ActiveRecord::Base
   belongs_to :forked_from_project, class_name: 'Project'
 
   validates :fork_network, :project, presence: true
+
+  after_destroy :cleanup_fork_network
+
+  private
+
+  def cleanup_fork_network
+    # Explicitly using `#count` makes sure we have the correct number if the
+    # relation was loaded in the fork_network.
+    fork_network.destroy if fork_network.fork_network_members.count == 0
+  end
 end

changelogs/unreleased/bvl-delete-empty-fork-networks.yml

@@ -0,0 +1,5 @@
+---
+title: Clean up empty fork networks
+merge_request: 15373
+author:
+type: other

db/post_migrate/20171114104051_remove_empty_fork_networks.rb

@@ -0,0 +1,36 @@
+class RemoveEmptyForkNetworks < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 10_000
+
+  class MigrationForkNetwork < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'fork_networks'
+  end
+
+  class MigrationForkNetworkMembers < ActiveRecord::Base
+    self.table_name = 'fork_network_members'
+  end
+
+  disable_ddl_transaction!
+
+  def up
+    say 'Deleting empty ForkNetworks in batches'
+
+    has_members = MigrationForkNetworkMembers
+                    .where('fork_network_members.fork_network_id = fork_networks.id')
+                    .select(1)
+    MigrationForkNetwork.where('NOT EXISTS (?)', has_members)
+      .each_batch(of: BATCH_SIZE) do |networks|
+      deleted = networks.delete_all
+
+      say "Deleted #{deleted} rows in batch"
+    end
+  end
+
+  def down
+    # nothing
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171106180641) do
+ActiveRecord::Schema.define(version: 20171114104051) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

spec/factories/fork_network_members.rb

@@ -0,0 +1,8 @@
+FactoryGirl.define do
+  factory :fork_network_member do
+    association :project
+    association :fork_network
+
+    forked_from_project { fork_network.root_project }
+  end
+end

spec/migrations/remove_empty_fork_networks_spec.rb

@@ -0,0 +1,24 @@
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20171114104051_remove_empty_fork_networks.rb')
+
+describe RemoveEmptyForkNetworks, :migration do
+  let!(:fork_networks) { table(:fork_networks) }
+
+  let(:deleted_project) { create(:project) }
+  let!(:empty_network) { create(:fork_network, id: 1, root_project_id: deleted_project.id) }
+  let!(:other_network) { create(:fork_network, id: 2, root_project_id: create(:project).id) }
+
+  before do
+    deleted_project.destroy!
+  end
+
+  it 'deletes only the fork network without members' do
+    expect(fork_networks.count).to eq(2)
+
+    migrate!
+
+    expect(fork_networks.find_by(id: empty_network.id)).to be_nil
+    expect(fork_networks.find_by(id: other_network.id)).not_to be_nil
+    expect(fork_networks.count).to eq(1)
+  end
+end

spec/models/fork_network_member_spec.rb

@@ -5,4 +5,22 @@ describe ForkNetworkMember do
     it { is_expected.to validate_presence_of(:project) }
     it { is_expected.to validate_presence_of(:fork_network) }
   end
+
+  describe 'destroying a ForkNetworkMember' do
+    let(:fork_network_member) { create(:fork_network_member) }
+    let(:fork_network) { fork_network_member.fork_network }
+
+    it 'removes the fork network if it was the last member' do
+      fork_network.fork_network_members.destroy_all
+
+      expect(ForkNetwork.count).to eq(0)
+    end
+
+    it 'does not destroy the fork network if there are members left' do
+      fork_network_member.destroy!
+
+      # The root of the fork network is left
+      expect(ForkNetwork.count).to eq(1)
+    end
+  end
 end