Commit f3fba178 authored by Bob Van Landuyt's avatar Bob Van Landuyt

Remove the `ForkedProjectLink` model

This removes the `ForkedProjectLink` model that has been replaced by
the `ForkNetworkMember` and `ForkNetwork` combination. All existing
relations have been adjusted to use these new models.

The `forked_project_link` table has been dropped.

The "Forks" count on the admin dashboard has been updated to count all
`ForkNetworkMember` rows and deduct the number of `ForkNetwork`
rows. This is because now the "root-project" of a fork network also
has a `ForkNetworkMember` row. This count could become inaccurate when
the root of a fork network is deleted.
parent 4321d70d
......@@ -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
......
......@@ -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
# 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
......@@ -164,20 +164,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
......@@ -1247,12 +1242,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
......@@ -1543,9 +1533,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
......
......@@ -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?
......@@ -79,15 +71,6 @@ module Projects
@project.errors.add(:namespace, "is not valid")
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)
......
......@@ -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
......
......@@ -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
......
......@@ -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
......@@ -17,18 +16,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)
......
......@@ -25,7 +25,6 @@ module Projects
end
@project.fork_network_member.destroy
@project.forked_project_link.destroy
end
# rubocop: enable CodeReuse/ActiveRecord
......
......@@ -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
......
......@@ -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
......@@ -260,7 +260,7 @@ module API
super(projects_relation).preload(:group)
.preload(project_group_links: :group,
fork_network: :root_project,
forked_project_link: :forked_from_project,
fork_network_member: :forked_from_project,
forked_from_project: [:route, :forks, :tags, namespace: :route])
end
# rubocop: enable CodeReuse/ActiveRecord
......
......@@ -838,12 +838,14 @@ describe Projects::MergeRequestsController do
end
context 'with a forked project' do
let(:fork_project) { create(:project, :repository, forked_from_project: project) }
let(:fork_owner) { fork_project.owner }
let(:forked_project) { fork_project(project, fork_owner, repository: true) }
let(:fork_owner) { create(:user) }
before do
merge_request.update!(source_project: fork_project)
fork_project.add_reporter(user)
project.add_developer(fork_owner)
merge_request.update!(source_project: forked_project)
forked_project.add_reporter(user)
end
context 'user cannot push to source branch' do
......
FactoryBot.define do
factory :forked_project_link do
association :forked_to_project, factory: [:project, :repository]
association :forked_from_project, factory: [:project, :repository]
after(:create) do |link|
link.forked_from_project.reload
link.forked_to_project.reload
end
trait :forked_to_empty_project do
association :forked_to_project, factory: [:project, :repository]
end
end
end
require 'spec_helper'
describe 'admin visits dashboard' do
include ProjectForksHelper
before do
sign_in(create(:admin))
end
context 'counting forks' do
it 'correctly counts 2 forks of a project' do
project = create(:project)
project_fork = fork_project(project)
fork_project(project_fork)
# Make sure the fork_networks & fork_networks reltuples have been updated
# to get a correct count on postgresql
if Gitlab::Database.postgresql?
ActiveRecord::Base.connection.execute('ANALYZE fork_networks')
ActiveRecord::Base.connection.execute('ANALYZE fork_network_members')
end
visit admin_root_path
expect(page).to have_content('Forks 2')
end
end
end
require 'rails_helper'
describe 'Merge request > User edits MR' do
include ProjectForksHelper
it_behaves_like 'an editable merge request'
context 'for a forked project' do
it_behaves_like 'an editable merge request' do
let(:source_project) { create(:project, :repository, forked_from_project: target_project) }
let(:source_project) { fork_project(target_project, nil, repository: true) }
end
end
end
require 'rails_helper'
describe 'Merge request > User sees MR from deleted forked project', :js do
include ProjectForksHelper
let(:project) { create(:project, :public, :repository) }
let(:user) { project.creator }
let(:fork_project) { create(:project, :public, :repository, forked_from_project: project) }
let(:forked_project) { fork_project(project, nil, repository: true) }
let!(:merge_request) do
create(:merge_request_with_diffs, source_project: fork_project,
create(:merge_request_with_diffs, source_project: forked_project,
target_project: project,
description: 'Test merge request')
end
before do
MergeRequests::MergeService.new(project, user).execute(merge_request)
fork_project.destroy!
forked_project.destroy!
sign_in(user)
visit project_merge_request_path(project, merge_request)
end
......
require 'rails_helper'
describe 'Merge request > User sees notes from forked project', :js do
include ProjectForksHelper
let(:project) { create(:project, :public, :repository) }
let(:user) { project.creator }
let(:fork_project) { create(:project, :public, :repository, forked_from_project: project) }
let(:forked_project) { fork_project(project, nil, repository: true) }
let!(:merge_request) do
create(:merge_request_with_diffs, source_project: fork_project,
create(:merge_request_with_diffs, source_project: forked_project,
target_project: project,
description: 'Test merge request')
end
before do
create(:note_on_commit, note: 'A commit comment',
project: fork_project,
project: forked_project,
commit_id: merge_request.commit_shas.first)
sign_in(user)
end
......
require 'rails_helper'
describe 'Merge request > User sees pipelines from forked project', :js do
include ProjectForksHelper
let(:target_project) { create(:project, :public, :repository) }
let(:user) { target_project.creator }
let(:fork_project) { create(:project, :repository, forked_from_project: target_project) }
let(:forked_project) { fork_project(target_project, nil, repository: true) }
let!(:merge_request) do
create(:merge_request_with_diffs, source_project: fork_project,
create(:merge_request_with_diffs, source_project: forked_project,
target_project: target_project,
description: 'Test merge request')
end
let(:pipeline) do
create(:ci_pipeline,
project: fork_project,
project: forked_project,
sha: merge_request.diff_head_sha,
ref: merge_request.source_branch)
end
......
require 'spec_helper'
describe ForkProjectsFinder do
let(:source_project) { create(:project, :empty_repo) }
let(:private_fork) { create(:project, :private, :empty_repo, name: 'A') }
let(:internal_fork) { create(:project, :internal, :empty_repo, name: 'B') }
let(:public_fork) { create(:project, :public, :empty_repo, name: 'C') }
include ProjectForksHelper
let(:source_project) { create(:project, :public, :empty_repo) }
let(:private_fork) { fork_project(source_project, nil, name: 'A') }
let(:internal_fork) { fork_project(source_project, nil, name: 'B') }
let(:public_fork) { fork_project(source_project, nil, name: 'C') }
let(:non_member) { create(:user) }
let(:private_fork_member) { create(:user) }
before do
private_fork.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
private_fork.add_developer(private_fork_member)
source_project.forks << private_fork
source_project.forks << internal_fork
source_project.forks << public_fork
internal_fork.update!(visibility_level: Gitlab::VisibilityLevel::INTERNAL)
end
describe '#execute' do
......
require 'spec_helper'
describe VisibilityLevelHelper do
include ProjectForksHelper
let(:project) { build(:project) }
let(:group) { build(:group) }
let(:personal_snippet) { build(:personal_snippet) }
......@@ -83,13 +85,13 @@ describe VisibilityLevelHelper do
describe "disallowed_visibility_level?" do
describe "forks" do
let(:project) { create(:project, :internal) }
let(:fork_project) { create(:project, forked_from_project: project) }
let(:project) { create(:project, :internal) }
let(:forked_project) { fork_project(project) }
it "disallows levels" do
expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy
expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey
expect(disallowed_visibility_level?(fork_project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey
expect(disallowed_visibility_level?(forked_project, Gitlab::VisibilityLevel::PUBLIC)).to be_truthy
expect(disallowed_visibility_level?(forked_project, Gitlab::VisibilityLevel::INTERNAL)).to be_falsey
expect(disallowed_visibility_level?(forked_project, Gitlab::VisibilityLevel::PRIVATE)).to be_falsey
end
end
......
......@@ -229,9 +229,8 @@ project:
- mock_ci_service
- mock_deployment_service
- mock_monitoring_service
- forked_project_link
- forked_to_members
- forked_from_project
- forked_project_links
- forks
- merge_requests
- fork_merge_requests
......
require 'spec_helper'
describe ForkedProjectLink, "add link on fork" do
include ProjectForksHelper
let(:project_from) { create(:project, :repository) }
let(:project_to) { fork_project(project_from, user) }
let(:user) { create(:user) }
before do
project_from.add_reporter(user)
end
it 'project_from knows its forks' do
_ = project_to
expect(project_from.forks.count).to eq(1)
end
it "project_to knows it is forked" do
expect(project_to.forked?).to be_truthy
end
it "project knows who it is forked from" do
expect(project_to.forked_from_project).to eq(project_from)
end
context 'project_to is pending_delete' do
before do
project_to.update!(pending_delete: true)
end
it { expect(project_from.forks.count).to eq(0) }
end
context 'project_from is pending_delete' do
before do
project_from.update!(pending_delete: true)
end
it { expect(project_to.forked_from_project).to be_nil }
end
describe '#forked?' do
let(:project_to) { create(:project, :repository, forked_project_link: forked_project_link) }
let(:forked_project_link) { create(:forked_project_link) }
before do
forked_project_link.forked_from_project = project_from
forked_project_link.forked_to_project = project_to
forked_project_link.save!
end
it "project_to knows it is forked" do
expect(project_to.forked?).to be_truthy
end
it "project_from is not forked" do
expect(project_from.forked?).to be_falsey
end
it "project_to.destroy destroys fork_link" do
project_to.destroy
expect(ForkedProjectLink.exists?(id: forked_project_link.id)).to eq(false)
end
end
end
......@@ -23,7 +23,6 @@ describe Project do
it { is_expected.to have_many(:deploy_keys) }
it { is_expected.to have_many(:hooks) }
it { is_expected.to have_many(:protected_branches) }
it { is_expected.to have_one(:forked_project_link) }
it { is_expected.to have_one(:slack_service) }
it { is_expected.to have_one(:microsoft_teams_service) }
it { is_expected.to have_one(:mattermost_service) }
......@@ -56,7 +55,7 @@ describe Project do
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') }
it { is_expected.to have_one(:forked_from_project).through(:forked_project_link) }
it { is_expected.to have_one(:forked_from_project).through(:fork_network_member) }
it { is_expected.to have_one(:auto_devops).class_name('ProjectAutoDevops') }
it { is_expected.to have_many(:commit_statuses) }
it { is_expected.to have_many(:pipelines) }
......@@ -77,7 +76,8 @@ describe Project do
it { is_expected.to have_many(:lfs_objects_projects) }
it { is_expected.to have_many(:project_group_links) }
it { is_expected.to have_many(:notification_settings).dependent(:delete_all) }
it { is_expected.to have_many(:forks).through(:forked_project_links) }
it { is_expected.to have_many(:forked_to_members).class_name('ForkNetworkMember') }
it { is_expected.to have_many(:forks).through(:forked_to_members) }
it { is_expected.to have_many(:uploads) }
it { is_expected.to have_many(:pipeline_schedules) }
it { is_expected.to have_many(:members_and_requesters) }
......@@ -1359,7 +1359,7 @@ describe Project do
context 'when checking on forked project' do
let(:project) { create(:project, :internal) }
let(:forked_project) { create(:project, forked_from_project: project) }
let(:forked_project) { fork_project(project) }
it { expect(forked_project.visibility_level_allowed?(Gitlab::VisibilityLevel::PRIVATE)).to be_truthy }
it { expect(forked_project.visibility_level_allowed?(Gitlab::VisibilityLevel::INTERNAL)).to be_truthy }
......@@ -1971,9 +1971,12 @@ describe Project do
let(:import_jid) { '123' }
context 'forked' do
let(:forked_project_link) { create(:forked_project_link, :forked_to_empty_project) }
let(:forked_from_project) { forked_project_link.forked_from_project }
let(:project) { forked_project_link.forked_to_project }
let(:forked_from_project) { create(:project, :repository) }
let(:project) { create(:project) }
before do
fork_project(forked_from_project, nil, target_project: project)
end
it 'schedules a RepositoryForkWorker job' do
expect(RepositoryForkWorker).to receive(:perform_async).with(project.id).and_return(import_jid)
......@@ -2260,6 +2263,12 @@ describe Project do
end
end
describe '#forks' do
it 'includes direct forks of the project' do
expect(project.forks).to contain_exactly(forked_project)
end
end
describe '#lfs_storage_project' do
it 'returns self for non-forks' do
expect(project.lfs_storage_project).to eq project
......
......@@ -1196,7 +1196,7 @@ describe API::Projects do
expect(response).to have_gitlab_http_status(201)
expect(project_fork_target.forked_from_project.id).to eq(project_fork_source.id)
expect(project_fork_target.forked_project_link).to be_present
expect(project_fork_target.fork_network_member).to be_present