summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@vanlanduyt.co>2018-08-31 19:16:34 +0200
committerBob Van Landuyt <bob@vanlanduyt.co>2018-10-19 11:57:20 +0200
commitf3fba178b931cd2ba67f81ee667893cb84e7d42f (patch)
tree24c5a688880d5edd75642a7f16623124bde9fc22
parent4321d70d24a6a7dc99f80fa4211ba6250504d99b (diff)
downloadgitlab-ce-f3fba178b931cd2ba67f81ee667893cb84e7d42f.tar.gz
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.
-rw-r--r--app/controllers/admin/dashboard_controller.rb4
-rw-r--r--app/helpers/count_helper.rb14
-rw-r--r--app/models/forked_project_link.rb6
-rw-r--r--app/models/project.rb22
-rw-r--r--app/services/projects/create_service.rb21
-rw-r--r--app/services/projects/fork_service.rb68
-rw-r--r--app/services/projects/forks_count_service.rb5
-rw-r--r--app/services/projects/move_forks_service.rb13
-rw-r--r--app/services/projects/unlink_fork_service.rb1
-rw-r--r--app/views/admin/dashboard/index.html.haml2
-rw-r--r--app/workers/namespaceless_project_destroy_worker.rb2
-rw-r--r--lib/api/entities.rb2
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb10
-rw-r--r--spec/factories/forked_project_links.rb15
-rw-r--r--spec/features/admin/dashboard_spec.rb28
-rw-r--r--spec/features/merge_request/user_edits_mr_spec.rb4
-rw-r--r--spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb8
-rw-r--r--spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb8
-rw-r--r--spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb8
-rw-r--r--spec/finders/fork_projects_finder_spec.rb15
-rw-r--r--spec/helpers/visibility_level_helper_spec.rb12
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml3
-rw-r--r--spec/models/forked_project_link_spec.rb68
-rw-r--r--spec/models/project_spec.rb23
-rw-r--r--spec/requests/api/projects_spec.rb2
-rw-r--r--spec/serializers/merge_request_widget_entity_spec.rb8
-rw-r--r--spec/services/projects/create_service_spec.rb11
-rw-r--r--spec/services/projects/destroy_service_spec.rb1
-rw-r--r--spec/services/projects/fork_service_spec.rb25
-rw-r--r--spec/services/projects/unlink_fork_service_spec.rb9
-rw-r--r--spec/support/helpers/project_forks_helper.rb2
-rw-r--r--spec/workers/namespaceless_project_destroy_worker_spec.rb4
-rw-r--r--spec/workers/rebase_worker_spec.rb12
-rw-r--r--spec/workers/repository_fork_worker_spec.rb32
34 files changed, 217 insertions, 251 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 c7ca322853f..7a447592568 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -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
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
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 5a4b85f98cf..65855e8aac4 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -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
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 73b62dc1151..78581dc37a4 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -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
diff --git a/spec/factories/forked_project_links.rb b/spec/factories/forked_project_links.rb
deleted file mode 100644
index bc59fea81ec..00000000000
--- a/spec/factories/forked_project_links.rb
+++ /dev/null
@@ -1,15 +0,0 @@
-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
diff --git a/spec/features/admin/dashboard_spec.rb b/spec/features/admin/dashboard_spec.rb
new file mode 100644
index 00000000000..a6ca0803469
--- /dev/null
+++ b/spec/features/admin/dashboard_spec.rb
@@ -0,0 +1,28 @@
+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
diff --git a/spec/features/merge_request/user_edits_mr_spec.rb b/spec/features/merge_request/user_edits_mr_spec.rb
index 8c9e782aa76..3152707136c 100644
--- a/spec/features/merge_request/user_edits_mr_spec.rb
+++ b/spec/features/merge_request/user_edits_mr_spec.rb
@@ -1,11 +1,13 @@
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
diff --git a/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb b/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb
index 029b66b5e8e..fc4a188d4a7 100644
--- a/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb
+++ b/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb
@@ -1,18 +1,20 @@
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
diff --git a/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb b/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb
index d4ad0b0a377..a6118453540 100644
--- a/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb
+++ b/spec/features/merge_request/user_sees_notes_from_forked_project_spec.rb
@@ -1,18 +1,20 @@
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
diff --git a/spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb b/spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb
index d30dcefc6aa..97093bbc2f6 100644
--- a/spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb
+++ b/spec/features/merge_request/user_sees_pipelines_from_forked_project_spec.rb
@@ -1,17 +1,19 @@
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
diff --git a/spec/finders/fork_projects_finder_spec.rb b/spec/finders/fork_projects_finder_spec.rb
index f0cef7ea406..b3fdffc3331 100644
--- a/spec/finders/fork_projects_finder_spec.rb
+++ b/spec/finders/fork_projects_finder_spec.rb
@@ -1,20 +1,21 @@
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
diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb
index a3be222b7bd..e565ac8c530 100644
--- a/spec/helpers/visibility_level_helper_spec.rb
+++ b/spec/helpers/visibility_level_helper_spec.rb
@@ -1,6 +1,8 @@
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
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index fe167033941..a63f34b5536 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -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
diff --git a/spec/models/forked_project_link_spec.rb b/spec/models/forked_project_link_spec.rb
deleted file mode 100644
index 32e33e8f42f..00000000000
--- a/spec/models/forked_project_link_spec.rb
+++ /dev/null
@@ -1,68 +0,0 @@
-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
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 3fecddefff2..ecfa7771b02 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -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
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index c8e98e6024c..22e5a7c7174 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -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
expect(project_fork_target).to be_forked
end
diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
index 0ba2539a717..5bf8aa7f23f 100644
--- a/spec/serializers/merge_request_widget_entity_spec.rb
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe MergeRequestWidgetEntity do
+ include ProjectForksHelper
+
let(:project) { create :project, :repository }
let(:resource) { create(:merge_request, source_project: project, target_project: project) }
let(:user) { create(:user) }
@@ -206,12 +208,12 @@ describe MergeRequestWidgetEntity do
describe 'when source project is deleted' do
let(:project) { create(:project, :repository) }
- let(:fork_project) { create(:project, :repository, forked_from_project: project) }
- let(:merge_request) { create(:merge_request, source_project: fork_project, target_project: project) }
+ let(:forked_project) { fork_project(project) }
+ let(:merge_request) { create(:merge_request, source_project: forked_project, target_project: project) }
it 'returns a blank rebase_path' do
allow(merge_request).to receive(:should_be_rebased?).and_return(true)
- fork_project.destroy
+ forked_project.destroy
merge_request.reload
entity = described_class.new(merge_request, request: request).as_json
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index a80c8a7fe51..08de27ca44a 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -295,6 +295,17 @@ describe Projects::CreateService, '#execute' do
end
end
+ it 'calls the passed block' do
+ fake_block = double('block')
+ opts[:relations_block] = fake_block
+
+ expect_next_instance_of(Project) do |project|
+ expect(fake_block).to receive(:call).with(project)
+ end
+
+ create_project(user, opts)
+ end
+
it 'writes project full path to .git/config' do
project = create_project(user, opts)
rugged = rugged_repo(project.repository)
diff --git a/spec/services/projects/destroy_service_spec.rb b/spec/services/projects/destroy_service_spec.rb
index 1d31d26f418..12ddf8447bd 100644
--- a/spec/services/projects/destroy_service_spec.rb
+++ b/spec/services/projects/destroy_service_spec.rb
@@ -259,7 +259,6 @@ describe Projects::DestroyService do
before do
project.lfs_objects << create(:lfs_object)
- forked_project.forked_project_link.destroy
forked_project.reload
end
diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb
index 947cb61038d..a3d24ae312a 100644
--- a/spec/services/projects/fork_service_spec.rb
+++ b/spec/services/projects/fork_service_spec.rb
@@ -31,6 +31,10 @@ describe Projects::ForkService do
it { is_expected.not_to be_persisted }
it { expect(subject.errors[:forked_from_project_id]).to eq(['is forbidden']) }
+
+ it 'does not create a fork network' do
+ expect { subject }.not_to change { @from_project.reload.fork_network }
+ end
end
describe "successfully creates project in the user namespace" do
@@ -70,6 +74,12 @@ describe Projects::ForkService do
expect(fork_network.root_project).to eq(@from_project)
expect(fork_network.projects).to contain_exactly(@from_project, to_project)
end
+
+ it 'imports the repository of the forked project' do
+ to_project = fork_project(@from_project, @to_user, repository: true)
+
+ expect(to_project.empty_repo?).to be_falsy
+ end
end
context 'creating a fork of a fork' do
@@ -247,11 +257,13 @@ describe Projects::ForkService do
context 'if project is not forked' do
it 'creates fork relation' do
- expect(fork_to_project.forked?).to be false
+ expect(fork_to_project.forked?).to be_falsy
expect(forked_from_project(fork_to_project)).to be_nil
subject.execute(fork_to_project)
+ fork_to_project.reload
+
expect(fork_to_project.forked?).to be true
expect(forked_from_project(fork_to_project)).to eq fork_from_project
expect(fork_to_project.forked_from_project).to eq fork_from_project
@@ -272,6 +284,17 @@ describe Projects::ForkService do
.to change { fork_to_project.lfs_objects_projects.count }
.to(0)
end
+
+ context 'if the fork is not allowed' do
+ let(:fork_from_project) { create(:project, :private) }
+
+ it 'does not delete the LFS objects' do
+ create(:lfs_objects_project, project: fork_to_project)
+
+ expect { subject.execute(fork_to_project) }
+ .not_to change { fork_to_project.lfs_objects_projects.size }
+ end
+ end
end
end
end
diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb
index 3ec6139bfa6..014aab44281 100644
--- a/spec/services/projects/unlink_fork_service_spec.rb
+++ b/spec/services/projects/unlink_fork_service_spec.rb
@@ -5,13 +5,12 @@ describe Projects::UnlinkForkService do
subject { described_class.new(forked_project, user) }
- let(:fork_link) { forked_project.forked_project_link }
let(:project) { create(:project, :public) }
let(:forked_project) { fork_project(project, user) }
let(:user) { create(:user) }
context 'with opened merge request on the source project' do
- let(:merge_request) { create(:merge_request, source_project: forked_project, target_project: fork_link.forked_from_project) }
+ let(:merge_request) { create(:merge_request, source_project: forked_project, target_project: forked_project.forked_from_project) }
let(:merge_request2) { create(:merge_request, source_project: forked_project, target_project: fork_project(project)) }
let(:merge_request_in_fork) { create(:merge_request, source_project: forked_project, target_project: forked_project) }
@@ -35,12 +34,6 @@ describe Projects::UnlinkForkService do
end
end
- it 'remove fork relation' do
- expect(forked_project.forked_project_link).to receive(:destroy)
-
- subject.execute
- end
-
it 'removes the link to the fork network' do
expect(forked_project.fork_network_member).to be_present
expect(forked_project.fork_network).to be_present
diff --git a/spec/support/helpers/project_forks_helper.rb b/spec/support/helpers/project_forks_helper.rb
index 2c501a2a27c..6a7132c3093 100644
--- a/spec/support/helpers/project_forks_helper.rb
+++ b/spec/support/helpers/project_forks_helper.rb
@@ -24,7 +24,7 @@ module ProjectForksHelper
allow(service).to receive(:gitlab_shell).and_return(shell)
end
- forked_project = service.execute
+ forked_project = service.execute(params[:target_project])
# Reload the both projects so they know about their newly created fork_network
if forked_project.persisted?
diff --git a/spec/workers/namespaceless_project_destroy_worker_spec.rb b/spec/workers/namespaceless_project_destroy_worker_spec.rb
index eec110dfbfb..2f21a1321e1 100644
--- a/spec/workers/namespaceless_project_destroy_worker_spec.rb
+++ b/spec/workers/namespaceless_project_destroy_worker_spec.rb
@@ -71,10 +71,10 @@ describe NamespacelessProjectDestroyWorker do
expect(merge_request.reload).to be_closed
end
- it 'destroys the link' do
+ it 'destroys fork network members' do
subject.perform(project.id)
- expect(parent_project.forked_project_links).to be_empty
+ expect(parent_project.forked_to_members).to be_empty
end
end
end
diff --git a/spec/workers/rebase_worker_spec.rb b/spec/workers/rebase_worker_spec.rb
index 20aff020dbb..936b9deaecc 100644
--- a/spec/workers/rebase_worker_spec.rb
+++ b/spec/workers/rebase_worker_spec.rb
@@ -1,25 +1,23 @@
require 'spec_helper'
describe RebaseWorker, '#perform' do
+ include ProjectForksHelper
+
context 'when rebasing an MR from a fork where upstream has protected branches' do
let(:upstream_project) { create(:project, :repository) }
- let(:fork_project) { create(:project, :repository) }
+ let(:forked_project) { fork_project(upstream_project, nil, repository: true) }
let(:merge_request) do
create(:merge_request,
- source_project: fork_project,
+ source_project: forked_project,
source_branch: 'feature_conflict',
target_project: upstream_project,
target_branch: 'master')
end
- before do
- create(:forked_project_link, forked_to_project: fork_project, forked_from_project: upstream_project)
- end
-
it 'sets the correct project for running hooks' do
expect(MergeRequests::RebaseService)
- .to receive(:new).with(fork_project, merge_request.author).and_call_original
+ .to receive(:new).with(forked_project, merge_request.author).and_call_original
subject.perform(merge_request, merge_request.author)
end
diff --git a/spec/workers/repository_fork_worker_spec.rb b/spec/workers/repository_fork_worker_spec.rb
index ac8716ecfb1..781f91ac9ca 100644
--- a/spec/workers/repository_fork_worker_spec.rb
+++ b/spec/workers/repository_fork_worker_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe RepositoryForkWorker do
+ include ProjectForksHelper
+
describe 'modules' do
it 'includes ProjectImportOptions' do
expect(described_class).to include_module(ProjectImportOptions)
@@ -8,9 +10,13 @@ describe RepositoryForkWorker do
end
describe "#perform" do
- let(:project) { create(:project, :repository) }
+ let(:project) { create(:project, :public, :repository) }
let(:shell) { Gitlab::Shell.new }
- let(:fork_project) { create(:project, :repository, :import_scheduled, forked_from_project: project) }
+ let(:forked_project) { create(:project, :repository, :import_scheduled) }
+
+ before do
+ fork_project(project, forked_project.creator, target_project: forked_project, repository: true)
+ end
shared_examples 'RepositoryForkWorker performing' do
before do
@@ -21,8 +27,8 @@ describe RepositoryForkWorker do
expect(shell).to receive(:fork_repository).with(
'default',
project.disk_path,
- fork_project.repository_storage,
- fork_project.disk_path
+ forked_project.repository_storage,
+ forked_project.disk_path
)
end
@@ -49,28 +55,28 @@ describe RepositoryForkWorker do
perform!
- expect(fork_project.protected_branches.first.name).to eq(fork_project.default_branch)
+ expect(forked_project.protected_branches.first.name).to eq(forked_project.default_branch)
end
it 'flushes various caches' do
expect_fork_repository.and_return(true)
# Works around https://github.com/rspec/rspec-mocks/issues/910
- expect(Project).to receive(:find).with(fork_project.id).and_return(fork_project)
- expect(fork_project.repository).to receive(:expire_emptiness_caches)
+ expect(Project).to receive(:find).with(forked_project.id).and_return(forked_project)
+ expect(forked_project.repository).to receive(:expire_emptiness_caches)
.and_call_original
- expect(fork_project.repository).to receive(:expire_exists_cache)
+ expect(forked_project.repository).to receive(:expire_exists_cache)
.and_call_original
- expect(fork_project.wiki.repository).to receive(:expire_emptiness_caches)
+ expect(forked_project.wiki.repository).to receive(:expire_emptiness_caches)
.and_call_original
- expect(fork_project.wiki.repository).to receive(:expire_exists_cache)
+ expect(forked_project.wiki.repository).to receive(:expire_exists_cache)
.and_call_original
perform!
end
it "handles bad fork" do
- error_message = "Unable to fork project #{fork_project.id} for repository #{project.disk_path} -> #{fork_project.disk_path}"
+ error_message = "Unable to fork project #{forked_project.id} for repository #{project.disk_path} -> #{forked_project.disk_path}"
expect_fork_repository.and_return(false)
@@ -80,7 +86,7 @@ describe RepositoryForkWorker do
context 'only project ID passed' do
def perform!
- subject.perform(fork_project.id)
+ subject.perform(forked_project.id)
end
it_behaves_like 'RepositoryForkWorker performing'
@@ -88,7 +94,7 @@ describe RepositoryForkWorker do
context 'project ID, storage and repo paths passed' do
def perform!
- subject.perform(fork_project.id, TestEnv.repos_path, project.disk_path)
+ subject.perform(forked_project.id, TestEnv.repos_path, project.disk_path)
end
it_behaves_like 'RepositoryForkWorker performing'