Refactor editor (#34780)

A complete rewrite

services/repository/files/content.go

@@ -42,7 +42,7 @@ func GetContentsOrList(ctx context.Context, repo *repo_model.Repository, refComm
 	}
 
 	// Check that the path given in opts.treePath is valid (not a git path)
-	cleanTreePath := CleanUploadFileName(treePath)
+	cleanTreePath := CleanGitTreePath(treePath)
 	if cleanTreePath == "" && treePath != "" {
 		return nil, ErrFilenameInvalid{
 			Path: treePath,
@@ -103,7 +103,7 @@ func GetObjectTypeFromTreeEntry(entry *git.TreeEntry) ContentType {
 // GetContents gets the metadata on a file's contents. Ref can be a branch, commit or tag
 func GetContents(ctx context.Context, repo *repo_model.Repository, refCommit *utils.RefCommit, treePath string, forList bool) (*api.ContentsResponse, error) {
 	// Check that the path given in opts.treePath is valid (not a git path)
-	cleanTreePath := CleanUploadFileName(treePath)
+	cleanTreePath := CleanGitTreePath(treePath)
 	if cleanTreePath == "" && treePath != "" {
 		return nil, ErrFilenameInvalid{
 			Path: treePath,

services/repository/files/file.go

@@ -134,9 +134,8 @@ func (err ErrFilenameInvalid) Unwrap() error {
 	return util.ErrInvalidArgument
 }
 
-// CleanUploadFileName Trims a filename and returns empty string if it is a .git directory
-func CleanUploadFileName(name string) string {
-	// Rebase the filename
+// CleanGitTreePath cleans a tree path for git, it returns an empty string the path is invalid (e.g.: contains ".git" part)
+func CleanGitTreePath(name string) string {
 	name = util.PathJoinRel(name)
 	// Git disallows any filenames to have a .git directory in them.
 	for part := range strings.SplitSeq(name, "/") {
@@ -144,5 +143,8 @@ func CleanUploadFileName(name string) string {
 			return ""
 		}
 	}
+	if name == "." {
+		name = ""
+	}
 	return name
 }

services/repository/files/file_test.go

@@ -10,17 +10,9 @@ import (
 )
 
 func TestCleanUploadFileName(t *testing.T) {
-	t.Run("Clean regular file", func(t *testing.T) {
-		name := "this/is/test"
-		cleanName := CleanUploadFileName(name)
-		expectedCleanName := name
-		assert.Equal(t, expectedCleanName, cleanName)
-	})
-
-	t.Run("Clean a .git path", func(t *testing.T) {
-		name := "this/is/test/.git"
-		cleanName := CleanUploadFileName(name)
-		expectedCleanName := ""
-		assert.Equal(t, expectedCleanName, cleanName)
-	})
+	assert.Equal(t, "", CleanGitTreePath(""))  //nolint
+	assert.Equal(t, "", CleanGitTreePath(".")) //nolint
+	assert.Equal(t, "a/b", CleanGitTreePath("a/b"))
+	assert.Equal(t, "", CleanGitTreePath(".git/b")) //nolint
+	assert.Equal(t, "", CleanGitTreePath("a/.git")) //nolint
 }

services/repository/files/update.go

@@ -88,8 +88,26 @@ func (err ErrRepoFileDoesNotExist) Unwrap() error {
 	return util.ErrNotExist
 }
 
+type LazyReadSeeker interface {
+	io.ReadSeeker
+	io.Closer
+	OpenLazyReader() error
+}
+
 // ChangeRepoFiles adds, updates or removes multiple files in the given repository
-func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, opts *ChangeRepoFilesOptions) (*structs.FilesResponse, error) {
+func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *user_model.User, opts *ChangeRepoFilesOptions) (_ *structs.FilesResponse, errRet error) {
+	var addedLfsPointers []lfs.Pointer
+	defer func() {
+		if errRet != nil {
+			for _, lfsPointer := range addedLfsPointers {
+				_, err := git_model.RemoveLFSMetaObjectByOid(ctx, repo.ID, lfsPointer.Oid)
+				if err != nil {
+					log.Error("ChangeRepoFiles: RemoveLFSMetaObjectByOid failed: %v", err)
+				}
+			}
+		}
+	}()
+
 	err := repo.MustNotBeArchived()
 	if err != nil {
 		return nil, err
@@ -127,14 +145,14 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
 		}
 
 		// Check that the path given in opts.treePath is valid (not a git path)
-		treePath := CleanUploadFileName(file.TreePath)
+		treePath := CleanGitTreePath(file.TreePath)
 		if treePath == "" {
 			return nil, ErrFilenameInvalid{
 				Path: file.TreePath,
 			}
 		}
 		// If there is a fromTreePath (we are copying it), also clean it up
-		fromTreePath := CleanUploadFileName(file.FromTreePath)
+		fromTreePath := CleanGitTreePath(file.FromTreePath)
 		if fromTreePath == "" && file.FromTreePath != "" {
 			return nil, ErrFilenameInvalid{
 				Path: file.FromTreePath,
@@ -241,10 +259,14 @@ func ChangeRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
 	lfsContentStore := lfs.NewContentStore()
 	for _, file := range opts.Files {
 		switch file.Operation {
-		case "create", "update", "rename":
-			if err = CreateUpdateRenameFile(ctx, t, file, lfsContentStore, repo.ID, hasOldBranch); err != nil {
+		case "create", "update", "rename", "upload":
+			addedLfsPointer, err := modifyFile(ctx, t, file, lfsContentStore, repo.ID)
+			if err != nil {
 				return nil, err
 			}
+			if addedLfsPointer != nil {
+				addedLfsPointers = append(addedLfsPointers, *addedLfsPointer)
+			}
 		case "delete":
 			if err = t.RemoveFilesFromIndex(ctx, file.TreePath); err != nil {
 				return nil, err
@@ -366,18 +388,29 @@ func (err ErrSHAOrCommitIDNotProvided) Error() string {
 
 // handles the check for various issues for ChangeRepoFiles
 func handleCheckErrors(file *ChangeRepoFile, commit *git.Commit, opts *ChangeRepoFilesOptions) error {
-	if file.Operation == "update" || file.Operation == "delete" || file.Operation == "rename" {
-		fromEntry, err := commit.GetTreeEntryByPath(file.Options.fromTreePath)
-		if err != nil {
-			return err
+	// check old entry (fromTreePath/fromEntry)
+	if file.Operation == "update" || file.Operation == "upload" || file.Operation == "delete" || file.Operation == "rename" {
+		var fromEntryIDString string
+		{
+			fromEntry, err := commit.GetTreeEntryByPath(file.Options.fromTreePath)
+			if file.Operation == "upload" && git.IsErrNotExist(err) {
+				fromEntry = nil
+			} else if err != nil {
+				return err
+			}
+			if fromEntry != nil {
+				fromEntryIDString = fromEntry.ID.String()
+				file.Options.executable = fromEntry.IsExecutable() // FIXME: legacy hacky approach, it shouldn't prepare the "Options" in the "check" function
+			}
 		}
+
 		if file.SHA != "" {
 			// If the SHA given doesn't match the SHA of the fromTreePath, throw error
-			if file.SHA != fromEntry.ID.String() {
+			if file.SHA != fromEntryIDString {
 				return pull_service.ErrSHADoesNotMatch{
 					Path:       file.Options.treePath,
 					GivenSHA:   file.SHA,
-					CurrentSHA: fromEntry.ID.String(),
+					CurrentSHA: fromEntryIDString,
 				}
 			}
 		} else if opts.LastCommitID != "" {
@@ -399,11 +432,10 @@ func handleCheckErrors(file *ChangeRepoFile, commit *git.Commit, opts *ChangeRep
 			// haven't been made. We throw an error if one wasn't provided.
 			return ErrSHAOrCommitIDNotProvided{}
 		}
-		// FIXME: legacy hacky approach, it shouldn't prepare the "Options" in the "check" function
-		file.Options.executable = fromEntry.IsExecutable()
 	}
 
-	if file.Operation == "create" || file.Operation == "update" || file.Operation == "rename" {
+	// check new entry (treePath/treeEntry)
+	if file.Operation == "create" || file.Operation == "update" || file.Operation == "upload" || file.Operation == "rename" {
 		// For operation's target path, we need to make sure no parts of the path are existing files or links
 		// except for the last item in the path (which is the file name).
 		// And that shouldn't exist IF it is a new file OR is being moved to a new path.
@@ -454,18 +486,23 @@ func handleCheckErrors(file *ChangeRepoFile, commit *git.Commit, opts *ChangeRep
 	return nil
 }
 
-func CreateUpdateRenameFile(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile, contentStore *lfs.ContentStore, repoID int64, hasOldBranch bool) error {
+func modifyFile(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile, contentStore *lfs.ContentStore, repoID int64) (addedLfsPointer *lfs.Pointer, _ error) {
+	if rd, ok := file.ContentReader.(LazyReadSeeker); ok {
+		if err := rd.OpenLazyReader(); err != nil {
+			return nil, fmt.Errorf("OpenLazyReader: %w", err)
+		}
+		defer rd.Close()
+	}
+
 	// Get the two paths (might be the same if not moving) from the index if they exist
 	filesInIndex, err := t.LsFiles(ctx, file.TreePath, file.FromTreePath)
 	if err != nil {
-		return fmt.Errorf("UpdateRepoFile: %w", err)
+		return nil, fmt.Errorf("LsFiles: %w", err)
 	}
 	// If is a new file (not updating) then the given path shouldn't exist
 	if file.Operation == "create" {
 		if slices.Contains(filesInIndex, file.TreePath) {
-			return ErrRepoFileAlreadyExists{
-				Path: file.TreePath,
-			}
+			return nil, ErrRepoFileAlreadyExists{Path: file.TreePath}
 		}
 	}
 
@@ -474,7 +511,7 @@ func CreateUpdateRenameFile(ctx context.Context, t *TemporaryUploadRepository, f
 		for _, indexFile := range filesInIndex {
 			if indexFile == file.Options.fromTreePath {
 				if err = t.RemoveFilesFromIndex(ctx, file.FromTreePath); err != nil {
-					return err
+					return nil, err
 				}
 			}
 		}
@@ -482,45 +519,46 @@ func CreateUpdateRenameFile(ctx context.Context, t *TemporaryUploadRepository, f
 
 	var writeObjectRet *writeRepoObjectRet
 	switch file.Operation {
-	case "create", "update":
-		writeObjectRet, err = writeRepoObjectForCreateOrUpdate(ctx, t, file)
+	case "create", "update", "upload":
+		writeObjectRet, err = writeRepoObjectForModify(ctx, t, file)
 	case "rename":
 		writeObjectRet, err = writeRepoObjectForRename(ctx, t, file)
 	default:
-		return util.NewInvalidArgumentErrorf("unknown file modification operation: '%s'", file.Operation)
+		return nil, util.NewInvalidArgumentErrorf("unknown file modification operation: '%s'", file.Operation)
 	}
 	if err != nil {
-		return err
+		return nil, err
 	}
 
 	// Add the object to the index, the "file.Options.executable" is set in handleCheckErrors by the caller (legacy hacky approach)
 	if err = t.AddObjectToIndex(ctx, util.Iif(file.Options.executable, "100755", "100644"), writeObjectRet.ObjectHash, file.Options.treePath); err != nil {
-		return err
+		return nil, err
 	}
 
 	if writeObjectRet.LfsContent == nil {
-		return nil // No LFS pointer, so nothing to do
+		return nil, nil // No LFS pointer, so nothing to do
 	}
 	defer writeObjectRet.LfsContent.Close()
 
 	// Now we must store the content into an LFS object
 	lfsMetaObject, err := git_model.NewLFSMetaObject(ctx, repoID, writeObjectRet.LfsPointer)
 	if err != nil {
-		return err
+		return nil, err
 	}
-	if exist, err := contentStore.Exists(lfsMetaObject.Pointer); err != nil {
-		return err
-	} else if exist {
-		return nil
-	}
-
-	err = contentStore.Put(lfsMetaObject.Pointer, writeObjectRet.LfsContent)
+	exist, err := contentStore.Exists(lfsMetaObject.Pointer)
 	if err != nil {
-		if _, errRemove := git_model.RemoveLFSMetaObjectByOid(ctx, repoID, lfsMetaObject.Oid); errRemove != nil {
-			return fmt.Errorf("unable to remove failed inserted LFS object %s: %v (Prev Error: %w)", lfsMetaObject.Oid, errRemove, err)
+		return nil, err
+	}
+	if !exist {
+		err = contentStore.Put(lfsMetaObject.Pointer, writeObjectRet.LfsContent)
+		if err != nil {
+			if _, errRemove := git_model.RemoveLFSMetaObjectByOid(ctx, repoID, lfsMetaObject.Oid); errRemove != nil {
+				return nil, fmt.Errorf("unable to remove failed inserted LFS object %s: %v (Prev Error: %w)", lfsMetaObject.Oid, errRemove, err)
+			}
+			return nil, err
 		}
 	}
-	return err
+	return &lfsMetaObject.Pointer, nil
 }
 
 func checkIsLfsFileInGitAttributes(ctx context.Context, t *TemporaryUploadRepository, paths []string) (ret []bool, err error) {
@@ -544,8 +582,8 @@ type writeRepoObjectRet struct {
 	LfsPointer lfs.Pointer
 }
 
-// writeRepoObjectForCreateOrUpdate hashes the git object for create or update operations
-func writeRepoObjectForCreateOrUpdate(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile) (ret *writeRepoObjectRet, err error) {
+// writeRepoObjectForModify hashes the git object for create or update operations
+func writeRepoObjectForModify(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile) (ret *writeRepoObjectRet, err error) {
 	ret = &writeRepoObjectRet{}
 	treeObjectContentReader := file.ContentReader
 	if setting.LFS.StartServer {
@@ -574,7 +612,7 @@ func writeRepoObjectForCreateOrUpdate(ctx context.Context, t *TemporaryUploadRep
 	return ret, nil
 }
 
-// writeRepoObjectForRename the same as writeRepoObjectForCreateOrUpdate buf for "rename"
+// writeRepoObjectForRename the same as writeRepoObjectForModify buf for "rename"
 func writeRepoObjectForRename(ctx context.Context, t *TemporaryUploadRepository, file *ChangeRepoFile) (ret *writeRepoObjectRet, err error) {
 	lastCommitID, err := t.GetLastCommit(ctx)
 	if err != nil {

services/repository/files/upload.go

@@ -8,15 +8,11 @@ import (
 	"fmt"
 	"os"
 	"path"
-	"strings"
+	"sync"
 
-	git_model "code.gitea.io/gitea/models/git"
 	repo_model "code.gitea.io/gitea/models/repo"
 	user_model "code.gitea.io/gitea/models/user"
-	"code.gitea.io/gitea/modules/git"
-	"code.gitea.io/gitea/modules/git/attribute"
-	"code.gitea.io/gitea/modules/lfs"
-	"code.gitea.io/gitea/modules/setting"
+	"code.gitea.io/gitea/modules/log"
 )
 
 // UploadRepoFileOptions contains the uploaded repository file options
@@ -32,23 +28,48 @@ type UploadRepoFileOptions struct {
 	Committer    *IdentityOptions
 }
 
-type uploadInfo struct {
-	upload        *repo_model.Upload
-	lfsMetaObject *git_model.LFSMetaObject
+type lazyLocalFileReader struct {
+	*os.File
+	localFilename string
+	counter       int
+	mu            sync.Mutex
 }
 
-func cleanUpAfterFailure(ctx context.Context, infos *[]uploadInfo, t *TemporaryUploadRepository, original error) error {
-	for _, info := range *infos {
-		if info.lfsMetaObject == nil {
-			continue
-		}
-		if !info.lfsMetaObject.Existing {
-			if _, err := git_model.RemoveLFSMetaObjectByOid(ctx, t.repo.ID, info.lfsMetaObject.Oid); err != nil {
-				original = fmt.Errorf("%w, %v", original, err) // We wrap the original error - as this is the underlying error that required the fallback
+var _ LazyReadSeeker = (*lazyLocalFileReader)(nil)
+
+func (l *lazyLocalFileReader) Close() error {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
+	if l.counter > 0 {
+		l.counter--
+		if l.counter == 0 {
+			if err := l.File.Close(); err != nil {
+				return fmt.Errorf("close file %s: %w", l.localFilename, err)
 			}
+			l.File = nil
 		}
+		return nil
 	}
-	return original
+	return fmt.Errorf("file %s already closed", l.localFilename)
+}
+
+func (l *lazyLocalFileReader) OpenLazyReader() error {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+
+	if l.File != nil {
+		l.counter++
+		return nil
+	}
+
+	file, err := os.Open(l.localFilename)
+	if err != nil {
+		return err
+	}
+	l.File = file
+	l.counter = 1
+	return nil
 }
 
 // UploadRepoFiles uploads files to the given repository
@@ -62,178 +83,29 @@ func UploadRepoFiles(ctx context.Context, repo *repo_model.Repository, doer *use
 		return fmt.Errorf("GetUploadsByUUIDs [uuids: %v]: %w", opts.Files, err)
 	}
 
-	names := make([]string, len(uploads))
-	infos := make([]uploadInfo, len(uploads))
-	for i, upload := range uploads {
-		// Check file is not lfs locked, will return nil if lock setting not enabled
-		filepath := path.Join(opts.TreePath, upload.Name)
-		lfsLock, err := git_model.GetTreePathLock(ctx, repo.ID, filepath)
-		if err != nil {
-			return err
-		}
-		if lfsLock != nil && lfsLock.OwnerID != doer.ID {
-			u, err := user_model.GetUserByID(ctx, lfsLock.OwnerID)
-			if err != nil {
-				return err
-			}
-			return git_model.ErrLFSFileLocked{RepoID: repo.ID, Path: filepath, UserName: u.Name}
-		}
-
-		names[i] = upload.Name
-		infos[i] = uploadInfo{upload: upload}
+	changeOpts := &ChangeRepoFilesOptions{
+		LastCommitID: opts.LastCommitID,
+		OldBranch:    opts.OldBranch,
+		NewBranch:    opts.NewBranch,
+		Message:      opts.Message,
+		Signoff:      opts.Signoff,
+		Author:       opts.Author,
+		Committer:    opts.Committer,
 	}
-
-	t, err := NewTemporaryUploadRepository(repo)
-	if err != nil {
-		return err
-	}
-	defer t.Close()
-
-	hasOldBranch := true
-	if err = t.Clone(ctx, opts.OldBranch, true); err != nil {
-		if !git.IsErrBranchNotExist(err) || !repo.IsEmpty {
-			return err
-		}
-		if err = t.Init(ctx, repo.ObjectFormatName); err != nil {
-			return err
-		}
-		hasOldBranch = false
-		opts.LastCommitID = ""
-	}
-	if hasOldBranch {
-		if err = t.SetDefaultIndex(ctx); err != nil {
-			return err
-		}
-	}
-
-	var attributesMap map[string]*attribute.Attributes
-	// when uploading to an empty repo, the old branch doesn't exist, but some "global gitattributes" or "info/attributes" may exist
-	if setting.LFS.StartServer {
-		attributesMap, err = attribute.CheckAttributes(ctx, t.gitRepo, "" /* use temp repo's working dir */, attribute.CheckAttributeOpts{
-			Attributes: []string{attribute.Filter},
-			Filenames:  names,
+	for _, upload := range uploads {
+		changeOpts.Files = append(changeOpts.Files, &ChangeRepoFile{
+			Operation:     "upload",
+			TreePath:      path.Join(opts.TreePath, upload.Name),
+			ContentReader: &lazyLocalFileReader{localFilename: upload.LocalPath()},
 		})
-		if err != nil {
-			return err
-		}
 	}
 
-	// Copy uploaded files into repository.
-	// TODO: there is a small problem: when uploading LFS files with ".gitattributes", the "check-attr" runs before this loop,
-	// so LFS files are not able to be added as LFS objects. Ideally we need to do in 3 steps in the future:
-	// 1. Add ".gitattributes" to git index
-	// 2. Run "check-attr" (the previous attribute.CheckAttributes call)
-	// 3. Add files to git index (this loop)
-	// This problem is trivial so maybe no need to spend too much time on it at the moment.
-	for i := range infos {
-		if err := copyUploadedLFSFileIntoRepository(ctx, &infos[i], attributesMap, t, opts.TreePath); err != nil {
-			return err
-		}
-	}
-
-	// Now write the tree
-	treeHash, err := t.WriteTree(ctx)
+	_, err = ChangeRepoFiles(ctx, repo, doer, changeOpts)
 	if err != nil {
 		return err
 	}
-
-	// Now commit the tree
-	commitOpts := &CommitTreeUserOptions{
-		ParentCommitID:    opts.LastCommitID,
-		TreeHash:          treeHash,
-		CommitMessage:     opts.Message,
-		SignOff:           opts.Signoff,
-		DoerUser:          doer,
-		AuthorIdentity:    opts.Author,
-		CommitterIdentity: opts.Committer,
-	}
-	commitHash, err := t.CommitTree(ctx, commitOpts)
-	if err != nil {
-		return err
-	}
-
-	// Now deal with LFS objects
-	for i := range infos {
-		if infos[i].lfsMetaObject == nil {
-			continue
-		}
-		infos[i].lfsMetaObject, err = git_model.NewLFSMetaObject(ctx, infos[i].lfsMetaObject.RepositoryID, infos[i].lfsMetaObject.Pointer)
-		if err != nil {
-			// OK Now we need to cleanup
-			return cleanUpAfterFailure(ctx, &infos, t, err)
-		}
-		// Don't move the files yet - we need to ensure that
-		// everything can be inserted first
-	}
-
-	// 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.NewContentStore()
-	for _, info := range infos {
-		if err := uploadToLFSContentStore(info, contentStore); err != nil {
-			return cleanUpAfterFailure(ctx, &infos, t, err)
-		}
-	}
-
-	// Then push this tree to NewBranch
-	if err := t.Push(ctx, doer, commitHash, opts.NewBranch); err != nil {
-		return err
-	}
-
-	return repo_model.DeleteUploads(ctx, uploads...)
-}
-
-func copyUploadedLFSFileIntoRepository(ctx context.Context, info *uploadInfo, attributesMap map[string]*attribute.Attributes, t *TemporaryUploadRepository, treePath string) error {
-	file, err := os.Open(info.upload.LocalPath())
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-
-	var objectHash string
-	if setting.LFS.StartServer && attributesMap[info.upload.Name] != nil && attributesMap[info.upload.Name].Get(attribute.Filter).ToString().Value() == "lfs" {
-		// Handle LFS
-		// FIXME: Inefficient! this should probably happen in models.Upload
-		pointer, err := lfs.GeneratePointer(file)
-		if err != nil {
-			return err
-		}
-
-		info.lfsMetaObject = &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: t.repo.ID}
-
-		if objectHash, err = t.HashObjectAndWrite(ctx, strings.NewReader(pointer.StringContent())); err != nil {
-			return err
-		}
-	} else if objectHash, err = t.HashObjectAndWrite(ctx, file); err != nil {
-		return err
-	}
-
-	// Add the object to the index
-	return t.AddObjectToIndex(ctx, "100644", objectHash, path.Join(treePath, info.upload.Name))
-}
-
-func uploadToLFSContentStore(info uploadInfo, contentStore *lfs.ContentStore) error {
-	if info.lfsMetaObject == nil {
-		return nil
-	}
-	exist, err := contentStore.Exists(info.lfsMetaObject.Pointer)
-	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.Pointer, file); err != nil {
-			// OK Now we need to cleanup
-			// Can't clean up the store, once uploaded there they're there.
-			return err
-		}
+	if err := repo_model.DeleteUploads(ctx, uploads...); err != nil {
+		log.Error("DeleteUploads: %v", err)
 	}
 	return nil
 }