From a12d036a6856846f8ab52dd8acf3ed21f5a77036 Mon Sep 17 00:00:00 2001 From: Giteabot Date: Wed, 26 Jul 2023 19:53:15 +0800 Subject: [PATCH] Fix bugs in LFS meta garbage collection (#26122) (#26157) Backport #26122 by @Zettat123 This PR - Fix #26093. Replace `time.Time` with `timeutil.TimeStamp` - Fix #26135. Add missing `xorm:"extends"` to `CountLFSMetaObject` for LFS meta object query - Add a unit test for LFS meta object garbage collection Co-authored-by: Zettat123 --- models/git/lfs.go | 9 +++-- services/repository/lfs.go | 5 +-- services/repository/lfs_test.go | 64 +++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 services/repository/lfs_test.go diff --git a/models/git/lfs.go b/models/git/lfs.go index 7d3da72a94..e8192f92c5 100644 --- a/models/git/lfs.go +++ b/models/git/lfs.go @@ -6,7 +6,6 @@ package git import ( "context" "fmt" - "time" "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/perm" @@ -370,8 +369,8 @@ func IterateRepositoryIDsWithLFSMetaObjects(ctx context.Context, f func(ctx cont // IterateLFSMetaObjectsForRepoOptions provides options for IterateLFSMetaObjectsForRepo type IterateLFSMetaObjectsForRepoOptions struct { - OlderThan time.Time - UpdatedLessRecentlyThan time.Time + OlderThan timeutil.TimeStamp + UpdatedLessRecentlyThan timeutil.TimeStamp OrderByUpdated bool LoopFunctionAlwaysUpdates bool } @@ -382,8 +381,8 @@ func IterateLFSMetaObjectsForRepo(ctx context.Context, repoID int64, f func(cont batchSize := setting.Database.IterateBufferSize engine := db.GetEngine(ctx) type CountLFSMetaObject struct { - Count int64 - LFSMetaObject + Count int64 + LFSMetaObject `xorm:"extends"` } id := int64(0) diff --git a/services/repository/lfs.go b/services/repository/lfs.go index aeb808a72f..deb86988cf 100644 --- a/services/repository/lfs.go +++ b/services/repository/lfs.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" ) // GarbageCollectLFSMetaObjectsOptions provides options for GarbageCollectLFSMetaObjects function @@ -122,8 +123,8 @@ func GarbageCollectLFSMetaObjectsForRepo(ctx context.Context, repo *repo_model.R // // It is likely that a week is potentially excessive but it should definitely be enough that any // unassociated LFS object is genuinely unassociated. - OlderThan: opts.OlderThan, - UpdatedLessRecentlyThan: opts.UpdatedLessRecentlyThan, + OlderThan: timeutil.TimeStamp(opts.OlderThan.Unix()), + UpdatedLessRecentlyThan: timeutil.TimeStamp(opts.UpdatedLessRecentlyThan.Unix()), OrderByUpdated: true, LoopFunctionAlwaysUpdates: true, }) diff --git a/services/repository/lfs_test.go b/services/repository/lfs_test.go new file mode 100644 index 0000000000..e88befdfef --- /dev/null +++ b/services/repository/lfs_test.go @@ -0,0 +1,64 @@ +// Copyright 2023 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package repository + +import ( + "bytes" + "context" + "testing" + "time" + + "code.gitea.io/gitea/models/db" + git_model "code.gitea.io/gitea/models/git" + repo_model "code.gitea.io/gitea/models/repo" + "code.gitea.io/gitea/models/unittest" + "code.gitea.io/gitea/modules/lfs" + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/storage" + + "github.com/stretchr/testify/assert" +) + +func TestGarbageCollectLFSMetaObjects(t *testing.T) { + assert.NoError(t, unittest.PrepareTestDatabase()) + + setting.LFS.StartServer = true + err := storage.Init() + assert.NoError(t, err) + + repo, err := repo_model.GetRepositoryByOwnerAndName(db.DefaultContext, "user2", "repo1") + assert.NoError(t, err) + + // add lfs object + lfsContent := []byte("gitea1") + lfsOid := storeObjectInRepo(t, repo.ID, &lfsContent) + + // gc + err = GarbageCollectLFSMetaObjects(context.Background(), GarbageCollectLFSMetaObjectsOptions{ + AutoFix: true, + OlderThan: time.Now().Add(7 * 24 * time.Hour).Add(5 * 24 * time.Hour), + UpdatedLessRecentlyThan: time.Now().Add(7 * 24 * time.Hour).Add(3 * 24 * time.Hour), + }) + assert.NoError(t, err) + + // lfs meta has been deleted + _, err = git_model.GetLFSMetaObjectByOid(db.DefaultContext, repo.ID, lfsOid) + assert.ErrorIs(t, err, git_model.ErrLFSObjectNotExist) +} + +func storeObjectInRepo(t *testing.T, repositoryID int64, content *[]byte) string { + pointer, err := lfs.GeneratePointer(bytes.NewReader(*content)) + assert.NoError(t, err) + + _, err = git_model.NewLFSMetaObject(db.DefaultContext, &git_model.LFSMetaObject{Pointer: pointer, RepositoryID: repositoryID}) + assert.NoError(t, err) + contentStore := lfs.NewContentStore() + exist, err := contentStore.Exists(pointer) + assert.NoError(t, err) + if !exist { + err := contentStore.Put(pointer, bytes.NewReader(*content)) + assert.NoError(t, err) + } + return pointer.Oid +}