Merge branch 'bvl-remove-forked-project-link' into 'master'

Remove ForkedProjectLink model

Closes #38883

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

11 files changed, 63 insertions, 95 deletions

diff --git a/app/controllers/admin/dashboard_controller.rb b/app/controllers/admin/dashboard_controller.rb
index b5fb5511638..23cc9ee247a 100644
--- a/app/controllers/admin/dashboard_controller.rb
+++ b/app/controllers/admin/dashboard_controller.rb
@@ -3,8 +3,8 @@
 class Admin::DashboardController < Admin::ApplicationController
   include CountHelper
 
-  COUNTED_ITEMS = [Project, User, Group, ForkedProjectLink, Issue, MergeRequest,
-                   Note, Snippet, Key, Milestone].freeze
+  COUNTED_ITEMS = [Project, User, Group, ForkNetworkMember, ForkNetwork, Issue,
+                   MergeRequest, Note, Snippet, Key, Milestone].freeze
 
   # rubocop: disable CodeReuse/ActiveRecord
   def index
diff --git a/app/helpers/count_helper.rb b/app/helpers/count_helper.rb
index e16223a82c9..13839474e1f 100644
--- a/app/helpers/count_helper.rb
+++ b/app/helpers/count_helper.rb
@@ -8,4 +8,18 @@ module CountHelper
 
     number_with_delimiter(count)
   end
+
+  # This will approximate the fork count by checking all counting all fork network
+  # memberships, and deducting 1 for each root of the fork network.
+  # This might be inacurate as the root of the fork network might have been deleted.
+  #
+  # This makes querying this information a lot more effecient and it should be
+  # accurate enough for the instance wide statistics
+  def approximate_fork_count_with_delimiters(count_data)
+    fork_network_count = count_data[ForkNetwork]
+    fork_network_member_count = count_data[ForkNetworkMember]
+    approximate_fork_count = fork_network_member_count - fork_network_count
+
+    number_with_delimiter(approximate_fork_count)
+  end
 end
diff --git a/app/models/forked_project_link.rb b/app/models/forked_project_link.rb
deleted file mode 100644
index 0f7067238cd..00000000000
--- a/app/models/forked_project_link.rb
+++ /dev/null
@@ -1,6 +0,0 @@
-# frozen_string_literal: true
-
-class ForkedProjectLink < ActiveRecord::Base
-  belongs_to :forked_to_project, -> { where.not(pending_delete: true) }, class_name: 'Project'
-  belongs_to :forked_from_project, -> { where.not(pending_delete: true) }, class_name: 'Project'
-end
diff --git a/app/models/project.rb b/app/models/project.rb
index b80e41e4a96..086f256174f 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -167,20 +167,15 @@ class Project < ActiveRecord::Base
   has_one :packagist_service
   has_one :hangouts_chat_service
 
-  # TODO: replace these relations with the fork network versions
-  has_one  :forked_project_link,  foreign_key: "forked_to_project_id"
-  has_one  :forked_from_project,  through:   :forked_project_link
-
-  has_many :forked_project_links, foreign_key: "forked_from_project_id"
-  has_many :forks,                through:     :forked_project_links, source: :forked_to_project
-  # TODO: replace these relations with the fork network versions
-
   has_one :root_of_fork_network,
           foreign_key: 'root_project_id',
           inverse_of: :root_project,
           class_name: 'ForkNetwork'
   has_one :fork_network_member
   has_one :fork_network, through: :fork_network_member
+  has_one :forked_from_project, through: :fork_network_member
+  has_many :forked_to_members, class_name: 'ForkNetworkMember', foreign_key: 'forked_from_project_id'
+  has_many :forks, through: :forked_to_members, source: :project, inverse_of: :forked_from_project
 
   has_one :import_state, autosave: true, class_name: 'ProjectImportState', inverse_of: :project
   has_one :import_export_upload, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
@@ -1250,12 +1245,7 @@ class Project < ActiveRecord::Base
   end
 
   def forked?
-    return true if fork_network && fork_network.root_project != self
-
-    # TODO: Use only the above conditional using the `fork_network`
-    # This is the old conditional that looks at the `forked_project_link`, we
-    # fall back to this while we're migrating the new models
-    !(forked_project_link.nil? || forked_project_link.forked_from_project.nil?)
+    fork_network && fork_network.root_project != self
   end
 
   def fork_source
@@ -1546,9 +1536,7 @@ class Project < ActiveRecord::Base
   def visibility_level_allowed_as_fork?(level = self.visibility_level)
     return true unless forked?
 
-    # self.forked_from_project will be nil before the project is saved, so
-    # we need to go through the relation
-    original_project = forked_project_link&.forked_from_project
+    original_project = fork_source
     return true unless original_project
 
     level <= original_project.visibility_level
diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb
index 0e6a7e8da54..20bfe5af7a1 100644
--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -13,8 +13,8 @@ module Projects
         return ::Projects::CreateFromTemplateService.new(current_user, params).execute
       end
 
-      forked_from_project_id = params.delete(:forked_from_project_id)
       import_data = params.delete(:import_data)
+      relations_block = params.delete(:relations_block)
 
       @project = Project.new(params)
 
@@ -24,11 +24,6 @@ module Projects
         return @project
       end
 
-      unless allowed_fork?(forked_from_project_id)
-        @project.errors.add(:forked_from_project_id, 'is forbidden')
-        return @project
-      end
-
       set_project_name_from_path
 
       # get namespace id
@@ -47,6 +42,7 @@ module Projects
         @project.namespace_id = current_user.namespace_id
       end
 
+      relations_block&.call(@project)
       yield(@project) if block_given?
 
       # If the block added errors, don't try to save the project
@@ -54,10 +50,6 @@ module Projects
 
       @project.creator = current_user
 
-      if forked_from_project_id
-        @project.build_forked_project_link(forked_from_project_id: forked_from_project_id)
-      end
-
       save_project_and_import_data(import_data)
 
       after_create_actions if @project.persisted?
@@ -80,15 +72,6 @@ module Projects
     end
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def allowed_fork?(source_project_id)
-      return true if source_project_id.nil?
-
-      source_project = Project.find_by(id: source_project_id)
-      current_user.can?(:fork_project, source_project)
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    # rubocop: disable CodeReuse/ActiveRecord
     def allowed_namespace?(user, namespace_id)
       namespace = Namespace.find_by(id: namespace_id)
       current_user.can?(:create_projects, namespace)
diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb
index cbbb88a9410..8dc0e044875 100644
--- a/app/services/projects/fork_service.rb
+++ b/app/services/projects/fork_service.rb
@@ -12,31 +12,42 @@ module Projects
 
     private
 
+    def allowed_fork?
+      current_user.can?(:fork_project, @project)
+    end
+
     def link_existing_project(fork_to_project)
       return if fork_to_project.forked?
 
-      link_fork_network(fork_to_project)
+      build_fork_network_member(fork_to_project)
 
-      # A forked project stores its LFS objects in the `forked_from_project`.
-      # So the LFS objects become inaccessible, and therefore delete them from
-      # the database so they'll get cleaned up.
-      #
-      # TODO: refactor this to get the correct lfs objects when implementing
-      #       https://gitlab.com/gitlab-org/gitlab-ce/issues/39769
-      fork_to_project.lfs_objects_projects.delete_all
+      if link_fork_network(fork_to_project)
+        # A forked project stores its LFS objects in the `forked_from_project`.
+        # So the LFS objects become inaccessible, and therefore delete them from
+        # the database so they'll get cleaned up.
+        #
+        # TODO: refactor this to get the correct lfs objects when implementing
+        #       https://gitlab.com/gitlab-org/gitlab-ce/issues/39769
+        fork_to_project.lfs_objects_projects.delete_all
 
-      fork_to_project
+        fork_to_project
+      end
     end
 
     def fork_new_project
       new_params = {
-        forked_from_project_id: @project.id,
         visibility_level:       allowed_visibility_level,
         description:            @project.description,
         name:                   @project.name,
         path:                   @project.path,
         shared_runners_enabled: @project.shared_runners_enabled,
-        namespace_id:           target_namespace.id
+        namespace_id:           target_namespace.id,
+        fork_network:           fork_network,
+        # We need to assign the fork network membership after the project has
+        # been instantiated to avoid ActiveRecord trying to create it when
+        # initializing the project, as that would cause a foreign key constraint
+        # exception.
+        relations_block:        -> (project) { build_fork_network_member(project) }
       }
 
       if @project.avatar.present? && @project.avatar.image?
@@ -46,38 +57,35 @@ module Projects
       new_project = CreateService.new(current_user, new_params).execute
       return new_project unless new_project.persisted?
 
+      # Set the forked_from_project relation after saving to avoid having to
+      # reload the project to reset the association information and cause an
+      # extra query.
+      new_project.forked_from_project = @project
+
       builds_access_level = @project.project_feature.builds_access_level
       new_project.project_feature.update(builds_access_level: builds_access_level)
 
-      link_fork_network(new_project)
-
       new_project
     end
 
     def fork_network
-      if @project.fork_network
-        @project.fork_network
-      elsif forked_from_project = @project.forked_from_project
-        # TODO: remove this case when all background migrations have completed
-        # this only happens when a project had a `forked_project_link` that was
-        # not migrated to the `fork_network` relation
-        forked_from_project.fork_network || forked_from_project.create_root_of_fork_network
+      @fork_network ||= @project.fork_network || @project.build_root_of_fork_network
+    end
+
+    def build_fork_network_member(fork_to_project)
+      if allowed_fork?
+        fork_to_project.build_fork_network_member(forked_from_project: @project,
+                                                  fork_network: fork_network)
       else
-        @project.create_root_of_fork_network
+        fork_to_project.errors.add(:forked_from_project_id, 'is forbidden')
       end
     end
 
     def link_fork_network(fork_to_project)
-      fork_network.fork_network_members.create(project: fork_to_project,
-                                               forked_from_project: @project)
-
-      # TODO: remove this when ForkedProjectLink model is removed
-      unless fork_to_project.forked_project_link
-        fork_to_project.create_forked_project_link(forked_to_project: fork_to_project,
-                                                   forked_from_project: @project)
-      end
+      return if fork_to_project.errors.any?
 
-      refresh_forks_count
+      fork_to_project.fork_network_member.save &&
+        refresh_forks_count
     end
 
     def refresh_forks_count
diff --git a/app/services/projects/forks_count_service.rb b/app/services/projects/forks_count_service.rb
index 00e73148358..ca85e2dc281 100644
--- a/app/services/projects/forks_count_service.rb
+++ b/app/services/projects/forks_count_service.rb
@@ -9,10 +9,7 @@ module Projects
 
     # rubocop: disable CodeReuse/ActiveRecord
     def self.query(project_ids)
-      # We can't directly change ForkedProjectLink to ForkNetworkMember here
-      # Nowadays, when a call using v3 to projects/:id/fork is made,
-      # the relationship to ForkNetworkMember is not updated
-      ForkedProjectLink.where(forked_from_project: project_ids)
+      ForkNetworkMember.where(forked_from_project: project_ids)
     end
     # rubocop: enable CodeReuse/ActiveRecord
   end
diff --git a/app/services/projects/move_forks_service.rb b/app/services/projects/move_forks_service.rb
index 2948555a17c..33f0bab12c9 100644
--- a/app/services/projects/move_forks_service.rb
+++ b/app/services/projects/move_forks_service.rb
@@ -6,7 +6,6 @@ module Projects
       return unless super && source_project.fork_network
 
       Project.transaction(requires_new: true) do
-        move_forked_project_links
         move_fork_network_members
         update_root_project
         refresh_forks_count
@@ -18,18 +17,6 @@ module Projects
     private
 
     # rubocop: disable CodeReuse/ActiveRecord
-    def move_forked_project_links
-      # Update ancestor
-      ForkedProjectLink.where(forked_to_project: source_project)
-                       .update_all(forked_to_project_id: @project.id)
-
-      # Update the descendants
-      ForkedProjectLink.where(forked_from_project: source_project)
-                       .update_all(forked_from_project_id: @project.id)
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
-    # rubocop: disable CodeReuse/ActiveRecord
     def move_fork_network_members
       ForkNetworkMember.where(project: source_project).update_all(project_id: @project.id)
       ForkNetworkMember.where(forked_from_project: source_project).update_all(forked_from_project_id: @project.id)
diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb
index a8b7c7f136a..1b8a920268f 100644
--- a/app/services/projects/unlink_fork_service.rb
+++ b/app/services/projects/unlink_fork_service.rb
@@ -25,7 +25,6 @@ module Projects
       end
 
       @project.fork_network_member.destroy
-      @project.forked_project_link.destroy
     end
     # rubocop: enable CodeReuse/ActiveRecord
 
diff --git a/app/views/admin/dashboard/index.html.haml b/app/views/admin/dashboard/index.html.haml
index 85c04f8a01d..7ac79cc77f5 100644
--- a/app/views/admin/dashboard/index.html.haml
+++ b/app/views/admin/dashboard/index.html.haml
@@ -42,7 +42,7 @@
             %p
               Forks
               %span.light.float-right
-                = approximate_count_with_delimiters(@counts, ForkedProjectLink)
+                = approximate_fork_count_with_delimiters(@counts)
             %p
               Issues
               %span.light.float-right
diff --git a/app/workers/namespaceless_project_destroy_worker.rb b/app/workers/namespaceless_project_destroy_worker.rb
index d9df42c9e17..2965f3b1150 100644
--- a/app/workers/namespaceless_project_destroy_worker.rb
+++ b/app/workers/namespaceless_project_destroy_worker.rb
@@ -32,7 +32,5 @@ class NamespacelessProjectDestroyWorker
     merge_requests = project.forked_from_project.merge_requests.opened.from_project(project)
 
     merge_requests.update_all(state: 'closed')
-
-    project.forked_project_link.destroy
   end
 end