From f913a7709d370f68fb2681958b5f81f73c1947b0 Mon Sep 17 00:00:00 2001
From: mao chao <chaomao@gitlab.cn>
Date: Mon, 19 Feb 2024 16:02:46 +0800
Subject: [PATCH] Fix pull lfs files issue

1. Raise error when lfs update failed
2. do lfs file update when reop not changed

EE: true
Changelog: changed
---
 .../projects/update_mirror_service.rb         | 12 ++++----
 .../projects/update_mirror_service_spec.rb    | 29 +++++++++----------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/ee/app/services/projects/update_mirror_service.rb b/ee/app/services/projects/update_mirror_service.rb
index 8a0a447edc322f..7d25c06470073c 100644
--- a/ee/app/services/projects/update_mirror_service.rb
+++ b/ee/app/services/projects/update_mirror_service.rb
@@ -35,17 +35,16 @@ def execute
         return error("The mirror user is not allowed to push code to all branches on this project.")
       end
 
-      checksum_before = project.repository.checksum
-
       update_tags do
         project.fetch_mirror(forced: true, check_tags_changed: true)
       end
 
       update_branches
 
-      # Updating LFS objects is expensive since it requires scanning for blobs with pointers.
-      # Let's skip this if the repository hasn't changed.
-      update_lfs_objects if project.repository.checksum != checksum_before
+      # We know updating LFS objects is expensive since it requires scanning for blobs with pointers.
+      # However we do NOT want to skip this because when some LFS file failed in last update mirror
+      # user may manually trigger mirror again to retry update LFS file even repo not changed.
+      update_lfs_objects
 
       # Running git fetch in the repository creates loose objects in the same
       # way running git push *to* the repository does, so ensure we run regular
@@ -161,8 +160,7 @@ def update_lfs_objects
 
       if result[:status] == :error
         log_error(result[:message])
-        # Uncomment once https://gitlab.com/gitlab-org/gitlab-foss/issues/61834 is closed
-        # raise UpdateError, result[:message]
+        raise UpdateError, result[:message]
       end
     end
 
diff --git a/ee/spec/services/projects/update_mirror_service_spec.rb b/ee/spec/services/projects/update_mirror_service_spec.rb
index a9c8e803a94127..3b604360214c3e 100644
--- a/ee/spec/services/projects/update_mirror_service_spec.rb
+++ b/ee/spec/services/projects/update_mirror_service_spec.rb
@@ -10,6 +10,10 @@
   subject(:service) { described_class.new(project, project.first_owner) }
 
   describe "#execute" do
+    before do
+      allow(project).to receive(:lfs_enabled?).and_return(false)
+    end
+
     context 'unlicensed' do
       before do
         stub_licensed_features(repository_mirrors: false)
@@ -435,10 +439,15 @@ def create_file(repository)
       context 'when repository does not change' do
         before do
           allow(project).to receive(:lfs_enabled?).and_return(true)
+          stub_fetch_mirror(project, repository: project.repository, tags_changed: false)
+          allow(project.repository).to receive(:checksum).and_return('some checksum')
         end
 
-        it 'does not attempt to update LFS objects' do
-          expect(Projects::LfsPointers::LfsImportService).not_to receive(:new)
+        it 'update LFS objects' do
+          expect(Projects::LfsPointers::LfsImportService).to receive(:new).and_call_original
+          expect_next_instance_of(Projects::LfsPointers::LfsObjectDownloadListService) do |instance|
+            expect(instance).to receive(:each_list_item)
+          end
 
           service.execute
         end
@@ -481,21 +490,11 @@ def create_file(repository)
               end
             end
 
-            # Uncomment once https://gitlab.com/gitlab-org/gitlab-foss/issues/61834 is closed
-            # it 'fails mirror operation' do
-            #   expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: 'error message')
-
-            #   result = subject.execute
-
-            #   expect(result[:status]).to eq :error
-            #   expect(result[:message]).to eq 'error message'
-            # end
-
-            # Remove once https://gitlab.com/gitlab-org/gitlab-foss/issues/61834 is closed
-            it 'does not fail mirror operation' do
+            it 'fails mirror operation' do
               result = subject.execute
 
-              expect(result[:status]).to eq :success
+              expect(result[:status]).to eq :error
+              expect(result[:message]).to eq error_message
             end
 
             it 'logs the error' do
-- 
GitLab