From cf549500e063cfc340c4bf60b7abec54ced135d2 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Sat, 20 Mar 2021 06:49:29 +0800
Subject: [PATCH] Fix bug when upload on web (#15042)

* Fix bug when upload on web

* move into own function

Co-authored-by: 6543 <6543@obermui.de>
Co-authored-by: zeripath <art27@cantab.net>
---
 modules/repofiles/upload.go | 50 +++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 22 deletions(-)

diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go
index 2846e6c44..13c89f55b 100644
--- a/modules/repofiles/upload.go
+++ b/modules/repofiles/upload.go
@@ -125,7 +125,6 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep
 				return err
 			}
 			infos[i] = uploadInfo
-
 		} else if objectHash, err = t.HashObject(file); err != nil {
 			return err
 		}
@@ -133,7 +132,6 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep
 		// Add the object to the index
 		if err := t.AddObjectToIndex("100644", objectHash, path.Join(opts.TreePath, uploadInfo.upload.Name)); err != nil {
 			return err
-
 		}
 	}
 
@@ -170,28 +168,10 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep
 	// OK now we can insert the data into the store - there's no way to clean up the store
 	// once it's in there, it's in there.
 	contentStore := &lfs.ContentStore{ObjectStorage: storage.LFS}
-	for _, uploadInfo := range infos {
-		if uploadInfo.lfsMetaObject == nil {
-			continue
-		}
-		exist, err := contentStore.Exists(uploadInfo.lfsMetaObject)
-		if err != nil {
+	for _, info := range infos {
+		if err := uploadToLFSContentStore(info, contentStore); err != nil {
 			return cleanUpAfterFailure(&infos, t, err)
 		}
-		if !exist {
-			file, err := os.Open(uploadInfo.upload.LocalPath())
-			if err != nil {
-				return cleanUpAfterFailure(&infos, t, err)
-			}
-			defer file.Close()
-			// FIXME: Put regenerates the hash and copies the file over.
-			// I guess this strictly ensures the soundness of the store but this is inefficient.
-			if err := contentStore.Put(uploadInfo.lfsMetaObject, file); err != nil {
-				// OK Now we need to cleanup
-				// Can't clean up the store, once uploaded there they're there.
-				return cleanUpAfterFailure(&infos, t, err)
-			}
-		}
 	}
 
 	// Then push this tree to NewBranch
@@ -201,3 +181,29 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep
 
 	return models.DeleteUploads(uploads...)
 }
+
+func uploadToLFSContentStore(info uploadInfo, contentStore *lfs.ContentStore) error {
+	if info.lfsMetaObject == nil {
+		return nil
+	}
+	exist, err := contentStore.Exists(info.lfsMetaObject)
+	if err != nil {
+		return err
+	}
+	if !exist {
+		file, err := os.Open(info.upload.LocalPath())
+		if err != nil {
+			return err
+		}
+
+		defer file.Close()
+		// FIXME: Put regenerates the hash and copies the file over.
+		// I guess this strictly ensures the soundness of the store but this is inefficient.
+		if err := contentStore.Put(info.lfsMetaObject, file); err != nil {
+			// OK Now we need to cleanup
+			// Can't clean up the store, once uploaded there they're there.
+			return err
+		}
+	}
+	return nil
+}