Commit b6a64df9 authored by Patrick Bajao's avatar Patrick Bajao

Fail forking when source has lots of LFS objects

Also use `pluck` instead of loading all objects in memory.
parent a9fcc790
...@@ -1388,6 +1388,14 @@ class Project < ApplicationRecord ...@@ -1388,6 +1388,14 @@ class Project < ApplicationRecord
.where(lfs_objects_projects: { project_id: [self, lfs_storage_project] }) .where(lfs_objects_projects: { project_id: [self, lfs_storage_project] })
end end
# TODO: Call `#lfs_objects` instead once all LfsObjectsProject records are
# backfilled. At that point, projects can look at their own `lfs_objects`.
#
# See https://gitlab.com/gitlab-org/gitlab/issues/122002 for more info.
def lfs_objects_oids
all_lfs_objects.pluck(:oid)
end
def personal? def personal?
!group !group
end end
......
...@@ -30,11 +30,13 @@ class RepositoryForkWorker ...@@ -30,11 +30,13 @@ class RepositoryForkWorker
result = gitlab_shell.fork_repository(source_project, target_project) result = gitlab_shell.fork_repository(source_project, target_project)
if result if result
Projects::LfsPointers::LfsLinkService link_lfs_objects(source_project, target_project)
.new(target_project)
.execute(lfs_pointers(source_project))
else else
raise "Unable to fork project #{target_project.id} for repository #{source_project.disk_path} -> #{target_project.disk_path}" raise_fork_failure(
source_project,
target_project,
'Failed to create fork repository'
)
end end
target_project.after_import target_project.after_import
...@@ -47,7 +49,19 @@ class RepositoryForkWorker ...@@ -47,7 +49,19 @@ class RepositoryForkWorker
false false
end end
def lfs_pointers(project) def link_lfs_objects(source_project, target_project)
project.lfs_objects.map(&:oid) Projects::LfsPointers::LfsLinkService
.new(target_project)
.execute(source_project.lfs_objects_oids)
rescue Projects::LfsPointers::LfsLinkService::TooManyOidsError
raise_fork_failure(
source_project,
target_project,
'Source project has too many LFS objects'
)
end
def raise_fork_failure(source_project, target_project, reason)
raise "Unable to fork project #{target_project.id} for repository #{source_project.disk_path} -> #{target_project.disk_path}: #{reason}"
end end
end end
...@@ -5547,6 +5547,31 @@ describe Project do ...@@ -5547,6 +5547,31 @@ describe Project do
end end
end end
describe '#lfs_objects_oids' do
let(:project) { create(:project) }
let(:lfs_object) { create(:lfs_object) }
let(:another_lfs_object) { create(:lfs_object) }
subject { project.lfs_objects_oids }
context 'when project has associated LFS objects' do
before do
create(:lfs_objects_project, lfs_object: lfs_object, project: project)
create(:lfs_objects_project, lfs_object: another_lfs_object, project: project)
end
it 'returns OIDs of LFS objects' do
expect(subject).to match_array([lfs_object.oid, another_lfs_object.oid])
end
end
context 'when project has no associated LFS objects' do
it 'returns empty array' do
expect(subject).to be_empty
end
end
end
def rugged_config def rugged_config
rugged_repo(project.repository).config rugged_repo(project.repository).config
end end
......
...@@ -72,8 +72,8 @@ describe RepositoryForkWorker do ...@@ -72,8 +72,8 @@ describe RepositoryForkWorker do
perform! perform!
end end
it "handles bad fork" do it 'handles bad fork' do
error_message = "Unable to fork project #{forked_project.id} for repository #{project.disk_path} -> #{forked_project.disk_path}" error_message = "Unable to fork project #{forked_project.id} for repository #{project.disk_path} -> #{forked_project.disk_path}: Failed to create fork repository"
expect_fork_repository.and_return(false) expect_fork_repository.and_return(false)
...@@ -83,11 +83,22 @@ describe RepositoryForkWorker do ...@@ -83,11 +83,22 @@ describe RepositoryForkWorker do
it 'calls Projects::LfsPointers::LfsLinkService#execute with OIDs of source project LFS objects' do it 'calls Projects::LfsPointers::LfsLinkService#execute with OIDs of source project LFS objects' do
expect_fork_repository.and_return(true) expect_fork_repository.and_return(true)
expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service| expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service|
expect(service).to receive(:execute).with(project.lfs_objects.map(&:oid)) expect(service).to receive(:execute).with(project.lfs_objects_oids)
end end
perform! perform!
end end
it "handles LFS objects link failure" do
error_message = "Unable to fork project #{forked_project.id} for repository #{project.disk_path} -> #{forked_project.disk_path}: Source project has too many LFS objects"
expect_fork_repository.and_return(true)
expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service|
expect(service).to receive(:execute).and_raise(Projects::LfsPointers::LfsLinkService::TooManyOidsError)
end
expect { perform! }.to raise_error(StandardError, error_message)
end
end end
context 'only project ID passed' do context 'only project ID passed' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment