Mark existing LFS object for upload for forks

When source project already has the LFS object to be uploaded, it
should still be linked to the fork project. That way, forks has
access to their own LFS objects.

app/controllers/concerns/lfs_request.rb

@@ -116,6 +116,10 @@ module LfsRequest
     @objects ||= (params[:objects] || []).to_a
   end
 
+  def objects_oids
+    objects.map { |o| o['oid'].to_s }
+  end
+
   def has_authentication_ability?(capability)
     (authentication_abilities || []).include?(capability)
   end

app/controllers/repositories/lfs_api_controller.rb

@@ -45,15 +45,9 @@ module Repositories
       params[:operation] == 'upload'
     end
 
-    # rubocop: disable CodeReuse/ActiveRecord
-    def existing_oids
-      @existing_oids ||= begin
-        project.all_lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid)
-      end
-    end
-    # rubocop: enable CodeReuse/ActiveRecord
-
     def download_objects!
+      existing_oids = project.all_lfs_objects_oids(oids: objects_oids)
+
       objects.each do |object|
         if existing_oids.include?(object[:oid])
           object[:actions] = download_actions(object)
@@ -68,13 +62,17 @@ module Repositories
           }
         end
       end
+
       objects
     end
 
     def upload_objects!
+      existing_oids = project.lfs_objects_oids(oids: objects_oids)
+
       objects.each do |object|
         object[:actions] = upload_actions(object) unless existing_oids.include?(object[:oid])
       end
+
       objects
     end
 

app/models/project.rb

@@ -1398,12 +1398,17 @@ class Project < ApplicationRecord
       .where(lfs_objects_projects: { project_id: [self, lfs_storage_project] })
   end
 
-  # TODO: Call `#lfs_objects` instead once all LfsObjectsProject records are
-  # backfilled. At that point, projects can look at their own `lfs_objects`.
+  # TODO: Remove this method once all LfsObjectsProject records are backfilled
+  # for forks. At that point, projects can look at their own `lfs_objects` so
+  # `lfs_objects_oids` can be used instead.
   #
   # See https://gitlab.com/gitlab-org/gitlab/issues/122002 for more info.
-  def lfs_objects_oids
-    all_lfs_objects.pluck(:oid)
+  def all_lfs_objects_oids(oids: [])
+    oids(all_lfs_objects, oids: oids)
+  end
+
+  def lfs_objects_oids(oids: [])
+    oids(lfs_objects, oids: oids)
   end
 
   def personal?
@@ -2515,6 +2520,12 @@ class Project < ApplicationRecord
     reset
     retry
   end
+
+  def oids(objects, oids: [])
+    collection = oids.any? ? objects.where(oid: oids) : objects
+
+    collection.pluck(:oid)
+  end
 end
 
 Project.prepend_if_ee('EE::Project')

app/workers/repository_fork_worker.rb

@@ -52,7 +52,7 @@ class RepositoryForkWorker # rubocop:disable Scalability/IdempotentWorker
   def link_lfs_objects(source_project, target_project)
     Projects::LfsPointers::LfsLinkService
         .new(target_project)
-        .execute(source_project.lfs_objects_oids)
+        .execute(source_project.all_lfs_objects_oids)
   rescue Projects::LfsPointers::LfsLinkService::TooManyOidsError
     raise_fork_failure(
       source_project,

changelogs/unreleased/207216-lfs-batch-upload-fix.yml

+---
+title: Mark existing LFS object for upload for forks
+merge_request: 26344
+author:
+type: fixed

spec/models/project_spec.rb

@@ -5660,6 +5660,53 @@ describe Project do
     end
   end
 
+  describe '#all_lfs_objects_oids' do
+    let(:project) { create(:project) }
+    let(:lfs_object) { create(:lfs_object) }
+    let(:another_lfs_object) { create(:lfs_object) }
+
+    subject { project.all_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
+
+      context 'and there are specified oids' do
+        subject { project.all_lfs_objects_oids(oids: [lfs_object.oid]) }
+
+        it 'returns OIDs of LFS objects that match specified oids' do
+          expect(subject).to eq([lfs_object.oid])
+        end
+      end
+    end
+
+    context 'when fork has associated LFS objects to itself and source' do
+      let(:source) { create(:project) }
+      let(:project) { fork_project(source) }
+
+      before do
+        create(:lfs_objects_project, lfs_object: lfs_object, project: source)
+        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
+
   describe '#lfs_objects_oids' do
     let(:project) { create(:project) }
     let(:lfs_object) { create(:lfs_object) }
@@ -5676,6 +5723,14 @@ describe Project do
       it 'returns OIDs of LFS objects' do
         expect(subject).to match_array([lfs_object.oid, another_lfs_object.oid])
       end
+
+      context 'and there are specified oids' do
+        subject { project.lfs_objects_oids(oids: [lfs_object.oid]) }
+
+        it 'returns OIDs of LFS objects that match specified oids' do
+          expect(subject).to eq([lfs_object.oid])
+        end
+      end
     end
 
     context 'when project has no associated LFS objects' do

spec/requests/lfs_http_spec.rb

@@ -690,22 +690,34 @@ describe 'Git LFS API and storage' do
           end
 
           context 'when pushing an LFS object that already exists' do
+            shared_examples_for 'batch upload with existing LFS object' do
+              it_behaves_like 'LFS http 200 response'
+
+              it 'responds with links the object to the project' do
+                expect(json_response['objects']).to be_kind_of(Array)
+                expect(json_response['objects'].first).to include(sample_object)
+                expect(lfs_object.projects.pluck(:id)).not_to include(project.id)
+                expect(lfs_object.projects.pluck(:id)).to include(other_project.id)
+                expect(json_response['objects'].first['actions']['upload']['href']).to eq(objects_url(project, sample_oid, sample_size))
+                expect(json_response['objects'].first['actions']['upload']['header']).to include('Content-Type' => 'application/octet-stream')
+              end
+
+              it_behaves_like 'process authorization header', renew_authorization: true
+            end
+
             let(:update_lfs_permissions) do
               other_project.lfs_objects << lfs_object
             end
 
-            it_behaves_like 'LFS http 200 response'
-
-            it 'responds with links the object to the project' do
-              expect(json_response['objects']).to be_kind_of(Array)
-              expect(json_response['objects'].first).to include(sample_object)
-              expect(lfs_object.projects.pluck(:id)).not_to include(project.id)
-              expect(lfs_object.projects.pluck(:id)).to include(other_project.id)
-              expect(json_response['objects'].first['actions']['upload']['href']).to eq(objects_url(project, sample_oid, sample_size))
-              expect(json_response['objects'].first['actions']['upload']['header']).to include('Content-Type' => 'application/octet-stream')
+            context 'in another project' do
+              it_behaves_like 'batch upload with existing LFS object'
             end
 
-            it_behaves_like 'process authorization header', renew_authorization: true
+            context 'in source of fork project' do
+              let(:project) { fork_project(other_project) }
+
+              it_behaves_like 'batch upload with existing LFS object'
+            end
           end
 
           context 'when pushing a LFS object that does not exist' do

spec/workers/repository_fork_worker_spec.rb

@@ -83,7 +83,7 @@ describe RepositoryForkWorker do
       it 'calls Projects::LfsPointers::LfsLinkService#execute with OIDs of source project LFS objects' do
         expect_fork_repository.and_return(true)
         expect_next_instance_of(Projects::LfsPointers::LfsLinkService) do |service|
-          expect(service).to receive(:execute).with(project.lfs_objects_oids)
+          expect(service).to receive(:execute).with(project.all_lfs_objects_oids)
         end
 
         perform!