From deccd53c246ea4b031cf35c12f6bed039a577236 Mon Sep 17 00:00:00 2001 From: wxiaoguang Date: Wed, 5 Aug 2026 00:17:07 +0800 Subject: [PATCH] fix: git cache (#38763) 1. always use "last commit cache" 2. correctly build the cache key for any input (SafeCacheKey) 3. fix the git note "last commit cache FIXME" and avoid OOM --- custom/conf/app.example.ini | 3 - models/repo/repo.go | 11 --- modules/cache/cache.go | 29 ++++++ modules/cache/cache_test.go | 8 ++ modules/git/commit.go | 5 +- modules/git/commit_count_cache.go | 27 +++++ modules/git/commit_info.go | 12 +-- modules/git/commit_info_nogogit_test.go | 4 +- modules/git/commit_info_test.go | 10 +- modules/git/git.go | 2 + modules/git/last_commit_cache.go | 74 ++++---------- modules/git/last_commit_cache_gogit.go | 3 - modules/git/last_commit_cache_nogogit.go | 3 - modules/git/notes.go | 120 +++++++++++------------ modules/git/notes_test.go | 59 +++++------ modules/git/repo.go | 6 ++ modules/setting/cache.go | 14 +-- routers/api/v1/repo/notes.go | 24 ++--- routers/web/repo/commit.go | 12 +-- services/context/repo.go | 17 +--- services/mirror/mirror_pull.go | 3 +- services/pull/merge.go | 4 +- services/repository/cache.go | 17 +--- services/repository/files/content.go | 20 ---- services/repository/push.go | 14 +-- tests/integration/repo_test.go | 10 +- 26 files changed, 213 insertions(+), 298 deletions(-) create mode 100644 modules/git/commit_count_cache.go diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 5e518a1be64..c546c10d2e3 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1973,9 +1973,6 @@ LEVEL = Info ;; Time to keep items in cache if not used, default is 8760 hours. ;; Setting it to -1 disables caching ;ITEM_TTL = 8760h -;; -;; Only enable the cache when repository's commits count great than -;COMMITS_COUNT = 1000 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/models/repo/repo.go b/models/repo/repo.go index f3ad15b3675..74c133baa05 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -365,17 +365,6 @@ func (repo *Repository) APIURL(ctxOpt ...context.Context) string { return httplib.MakeAbsoluteURL(ctx, setting.AppSubURL+"/api/v1/repos/"+url.PathEscape(repo.OwnerName)+"/"+url.PathEscape(repo.Name)) } -// GetCommitsCountCacheKey returns cache key used for commits count caching. -func (repo *Repository) GetCommitsCountCacheKey(contextName string, isRef bool) string { - var prefix string - if isRef { - prefix = "ref" - } else { - prefix = "commit" - } - return fmt.Sprintf("commits-count-%d-%s-%s", repo.ID, prefix, contextName) -} - // LoadUnits loads repo units into repo.Units func (repo *Repository) LoadUnits(ctx context.Context) (err error) { if repo.Units != nil { diff --git a/modules/cache/cache.go b/modules/cache/cache.go index 3be8f0fb0a3..77c2cafa4cd 100644 --- a/modules/cache/cache.go +++ b/modules/cache/cache.go @@ -4,6 +4,7 @@ package cache import ( + "crypto/sha256" "encoding/hex" "errors" "fmt" @@ -11,6 +12,7 @@ import ( "time" "gitea.dev/modules/setting" + "gitea.dev/modules/util" _ "gitea.com/go-chi/cache/memcache" //nolint:depguard // memcache plugin for cache, it is required for config "ADAPTER=memcache" ) @@ -117,3 +119,30 @@ func Remove(key string) { } _ = defaultCache.Delete(key) } + +// SafeCacheKey returns a cache-safe key for the input string +// Some caches like memcached have char & length limits. +// Caller must make sure the prefix is valid and well-designed. +// If prefix is already too long, the returned key will still exceed the limit, then just let the cache report an error. +func SafeCacheKey(prefix, input string) string { + // memcached has a limit 250 for key length, so we use 230 to leave some room for other prefixes and separators + return safeCacheKey(prefix, input, 230) +} + +func safeCacheKey(prefix, input string, limit int) string { + safeAsKey := len(prefix)+len(input)+3 <= limit + if safeAsKey { + for i := 0; i < len(input); i++ { + if c := input[i]; c <= ' ' || c >= 127 { + safeAsKey = false + break + } + } + } + sep, key := ":s-", input + if !safeAsKey { + hashBytes := sha256.Sum256(util.UnsafeStringToBytes(input)) + sep, key = ":h-", hex.EncodeToString(hashBytes[:]) + } + return prefix + sep + key +} diff --git a/modules/cache/cache_test.go b/modules/cache/cache_test.go index 97ccf0f6e51..1c2d7683dc6 100644 --- a/modules/cache/cache_test.go +++ b/modules/cache/cache_test.go @@ -124,3 +124,11 @@ func TestGetInt64(t *testing.T) { assert.EqualValues(t, 100, data) Remove("key") } + +func TestSafeCacheKey(t *testing.T) { + assert.Equal(t, "prefix:s-0~", safeCacheKey("prefix", "0~", 100)) + assert.Equal(t, "prefix:h-36a9e7f1c95b82ffb99743e0c5c4ce95d83c9a430aac59f84ef3cbfab6145068", safeCacheKey("prefix", " ", 100)) + + assert.Equal(t, "prefix:s-a", safeCacheKey("prefix", "a", 10)) + assert.Equal(t, "prefix:h-961b6dd3ede3cb8ecbaacbd68de040cd78eb2ed5889130cceb4c49268ea4d506", safeCacheKey("prefix", "aa", 10)) +} diff --git a/modules/git/commit.go b/modules/git/commit.go index bd19332b827..05397f11d8c 100644 --- a/modules/git/commit.go +++ b/modules/git/commit.go @@ -66,10 +66,7 @@ func (c *Commit) ParentCount() int { // GetCommitByPath return the commit of relative path object. func (c *Commit) GetCommitByPath(ctx context.Context, gitRepo *Repository, relpath string) (*Commit, error) { - if gitRepo.LastCommitCache != nil { - return gitRepo.LastCommitCache.GetCommitByPath(ctx, c.ID.String(), relpath) - } - return gitRepo.getCommitByPathWithID(ctx, c.ID, relpath) + return gitRepo.LastCommitCache.GetCommitByPath(ctx, c.ID, relpath) } func (c *Commit) Tree() *Tree { diff --git a/modules/git/commit_count_cache.go b/modules/git/commit_count_cache.go new file mode 100644 index 00000000000..e0cdcd38647 --- /dev/null +++ b/modules/git/commit_count_cache.go @@ -0,0 +1,27 @@ +// Copyright 2026 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package git + +import ( + "context" + + "gitea.dev/modules/cache" +) + +func makeCommitsCountCacheKey(repo RepositoryFacade, ref RefName) string { + return cache.SafeCacheKey("git-commits-count:"+repo.GitRepoManagedID(), ref.String()) +} + +func RemoveCommitsCountCache(repo RepositoryFacade, ref RefName) { + cache.Remove(makeCommitsCountCacheKey(repo, ref)) +} + +func GetCommitsCountCache(ctx context.Context, repo RepositoryFacade, ref RefName, commit *Commit) (int64, error) { + if commit == nil { + return 0, nil + } + return cache.GetInt64(makeCommitsCountCacheKey(repo, ref), func() (int64, error) { + return CommitsCountOfCommit(ctx, repo, commit.ID.String()) + }) +} diff --git a/modules/git/commit_info.go b/modules/git/commit_info.go index 19ed12fb9d8..b34ed7716de 100644 --- a/modules/git/commit_info.go +++ b/modules/git/commit_info.go @@ -60,15 +60,9 @@ func (tes Entries) GetCommitsInfo(ctx context.Context, timeout time.Duration, re entryNames = append(entryNames, entry.Name()) } - var revs map[string]*Commit - var remainingEntryNames []string - if gitRepo.LastCommitCache != nil { - revs, remainingEntryNames, err = getLastCommitForPathsByCache(ctx, commit.ID.String(), treePath, entryNames, gitRepo.LastCommitCache) - if err != nil { - return nil, nil, err - } - } else { - revs, remainingEntryNames = map[string]*Commit{}, entryNames + revs, remainingEntryNames, err := getLastCommitForPathsByCache(ctx, commit.ID.String(), treePath, entryNames, gitRepo.LastCommitCache) + if err != nil { + return nil, nil, err } if len(remainingEntryNames) > 0 { diff --git a/modules/git/commit_info_nogogit_test.go b/modules/git/commit_info_nogogit_test.go index 4380e985e79..19bab4d4166 100644 --- a/modules/git/commit_info_nogogit_test.go +++ b/modules/git/commit_info_nogogit_test.go @@ -11,6 +11,7 @@ import ( "testing" "time" + "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/test" "gitea.dev/modules/util" @@ -19,7 +20,8 @@ import ( ) func TestEntries_GetCommitsInfo_ContextErr(t *testing.T) { - repo, err := OpenRepositoryLocal(t.Context(), filepath.Join(testReposDir, "repo1_bare")) + repoPath, _ := filepath.Abs(filepath.Join(testReposDir, "repo1_bare")) + repo, err := OpenRepository(t.Context(), gitrepo.RepositoryManaged("dummy", repoPath)) require.NoError(t, err) defer repo.Close() diff --git a/modules/git/commit_info_test.go b/modules/git/commit_info_test.go index 97caf7e977e..e29d689b8c3 100644 --- a/modules/git/commit_info_test.go +++ b/modules/git/commit_info_test.go @@ -8,6 +8,8 @@ import ( "testing" "time" + "gitea.dev/modules/git/gitrepo" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -126,9 +128,9 @@ func testGetCommitsInfo(t *testing.T, repo1 *Repository) { } func TestEntries_GetCommitsInfo(t *testing.T) { - bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") - bareRepo1, err := OpenRepositoryLocal(t.Context(), bareRepo1Path) - assert.NoError(t, err) + bareRepo1Path, _ := filepath.Abs(filepath.Join(testReposDir, "repo1_bare")) + bareRepo1, err := OpenRepository(t.Context(), gitrepo.RepositoryManaged("repo1_bare", bareRepo1Path)) + require.NoError(t, err) defer bareRepo1.Close() testGetCommitsInfo(t, bareRepo1) @@ -137,7 +139,7 @@ func TestEntries_GetCommitsInfo(t *testing.T) { if err != nil { assert.NoError(t, err) } - clonedRepo1, err := OpenRepositoryLocal(t.Context(), clonedPath) + clonedRepo1, err := OpenRepository(t.Context(), gitrepo.RepositoryManaged("repo1_bare-clone", clonedPath)) if err != nil { assert.NoError(t, err) } diff --git a/modules/git/git.go b/modules/git/git.go index 09d87c55fdb..ec25c8c504d 100644 --- a/modules/git/git.go +++ b/modules/git/git.go @@ -13,6 +13,7 @@ import ( "runtime" "strings" + "gitea.dev/modules/cache" "gitea.dev/modules/git/gitcmd" "gitea.dev/modules/globallock" "gitea.dev/modules/log" @@ -191,6 +192,7 @@ func RunGitTests(m interface{ Run() int }) { } func runGitTests(m interface{ Run() int }) int { + _ = cache.Init() gitHomePath, cleanup, err := tempdir.OsTempDir("gitea-test").MkdirTempRandom("git-home") if err != nil { return testlogger.MainErrorf("unable to create temp dir: %v", err) diff --git a/modules/git/last_commit_cache.go b/modules/git/last_commit_cache.go index 2de40cfd0a9..2b78b728997 100644 --- a/modules/git/last_commit_cache.go +++ b/modules/git/last_commit_cache.go @@ -5,103 +5,69 @@ package git import ( "context" - "crypto/sha256" "fmt" "gitea.dev/modules/cache" "gitea.dev/modules/log" - "gitea.dev/modules/setting" ) -func getCacheKey(repoPath, commitID, entryPath string) string { - hashBytes := sha256.Sum256(fmt.Appendf(nil, "%s:%s:%s", repoPath, commitID, entryPath)) - return fmt.Sprintf("last_commit:%x", hashBytes) +func getCacheKey(repo RepositoryFacade, commitID, entryPath string) string { + return cache.SafeCacheKey(fmt.Sprintf("git-last-commit:%s:%s", repo.GitRepoManagedID(), commitID), entryPath) } // LastCommitCache represents a cache to store last commit type LastCommitCache struct { - repoPath string - ttl func() int64 + ttlFn func() int64 repo *Repository commitCache map[string]*Commit cache cache.StringCache } -// NewLastCommitCache creates a new last commit cache for repo -func NewLastCommitCache(count int64, repoPath string, gitRepo *Repository, cache cache.StringCache) *LastCommitCache { - if cache == nil { - return nil - } - if count < setting.CacheService.LastCommit.CommitsCount { - return nil - } - - return &LastCommitCache{ - repoPath: repoPath, - repo: gitRepo, - ttl: setting.LastCommitCacheTTLSeconds, - cache: cache, - } -} - -// Put put the last commit id with commit and entry path +// Put puts the last commit id with commit and entry path func (c *LastCommitCache) Put(ref, entryPath, commitID string) error { - if c == nil || c.cache == nil { - return nil - } log.Debug("LastCommitCache save: [%s:%s:%s]", ref, entryPath, commitID) - return c.cache.Put(getCacheKey(c.repoPath, ref, entryPath), commitID, c.ttl()) + return c.cache.Put(getCacheKey(c.repo, ref, entryPath), commitID, c.ttlFn()) } // Get gets the last commit information by commit id and entry path func (c *LastCommitCache) Get(ctx context.Context, ref, entryPath string) (*Commit, error) { - if c == nil || c.cache == nil { - return nil, nil //nolint:nilnil // return nil when cache is not available - } - - commitID, ok := c.cache.Get(getCacheKey(c.repoPath, ref, entryPath)) - if !ok || commitID == "" { + lastCommitID, ok := c.cache.Get(getCacheKey(c.repo, ref, entryPath)) + if !ok || lastCommitID == "" { return nil, nil //nolint:nilnil // return nil when cache miss } - log.Debug("LastCommitCache hit level 1: [%s:%s:%s]", ref, entryPath, commitID) - if c.commitCache != nil { - if commit, ok := c.commitCache[commitID]; ok { - log.Debug("LastCommitCache hit level 2: [%s:%s:%s]", ref, entryPath, commitID) - return commit, nil - } + log.Debug("LastCommitCache hit level 1: [%s:%s:%s]", ref, entryPath, lastCommitID) + if lastCommit, ok := c.commitCache[lastCommitID]; ok { + log.Debug("LastCommitCache hit level 2: [%s:%s:%s]", ref, entryPath, lastCommitID) + return lastCommit, nil } - commit, err := c.repo.GetCommit(ctx, commitID) + lastCommit, err := c.repo.GetCommit(ctx, lastCommitID) if err != nil { return nil, err } if c.commitCache == nil { c.commitCache = make(map[string]*Commit) } - c.commitCache[commitID] = commit - return commit, nil + c.commitCache[lastCommitID] = lastCommit + return lastCommit, nil } // GetCommitByPath gets the last commit for the entry in the provided commit -func (c *LastCommitCache) GetCommitByPath(ctx context.Context, commitID, entryPath string) (*Commit, error) { - sha, err := NewIDFromString(commitID) - if err != nil { - return nil, err - } - - lastCommit, err := c.Get(ctx, sha.String(), entryPath) +func (c *LastCommitCache) GetCommitByPath(ctx context.Context, entryCommitID ObjectID, entryPath string) (*Commit, error) { + entryCommitIDStr := entryCommitID.String() + lastCommit, err := c.Get(ctx, entryCommitIDStr, entryPath) if err != nil || lastCommit != nil { return lastCommit, err } - lastCommit, err = c.repo.getCommitByPathWithID(ctx, sha, entryPath) + lastCommit, err = c.repo.getCommitByPathWithID(ctx, entryCommitID, entryPath) if err != nil { return nil, err } - if err := c.Put(commitID, entryPath, lastCommit.ID.String()); err != nil { - log.Error("Unable to cache %s as the last commit for %q in %s %s. Error %v", lastCommit.ID.String(), entryPath, commitID, c.repoPath, err) + if err := c.Put(entryCommitIDStr, entryPath, lastCommit.ID.String()); err != nil { + log.Error("Unable to cache %s as the last commit for %q in %s %s. Error %v", lastCommit.ID.String(), entryPath, entryCommitID, c.repo.LogString(), err) } return lastCommit, nil diff --git a/modules/git/last_commit_cache_gogit.go b/modules/git/last_commit_cache_gogit.go index eec9545785a..063bd9a8f7f 100644 --- a/modules/git/last_commit_cache_gogit.go +++ b/modules/git/last_commit_cache_gogit.go @@ -14,9 +14,6 @@ import ( // CacheCommit will cache the commit from the gitRepository func (c *Commit) CacheCommit(ctx context.Context, gitRepo *Repository) error { - if gitRepo.LastCommitCache == nil { - return nil - } commitNodeIndex, closer := gitRepo.CommitNodeIndex() defer closer() diff --git a/modules/git/last_commit_cache_nogogit.go b/modules/git/last_commit_cache_nogogit.go index 4956cc698e4..68d80690dcf 100644 --- a/modules/git/last_commit_cache_nogogit.go +++ b/modules/git/last_commit_cache_nogogit.go @@ -11,9 +11,6 @@ import ( // CacheCommit will cache the commit from the gitRepository func (c *Commit) CacheCommit(ctx context.Context, gitRepo *Repository) error { - if gitRepo.LastCommitCache == nil { - return nil - } return c.recursiveCache(ctx, gitRepo, c.Tree(), "", 1) } diff --git a/modules/git/notes.go b/modules/git/notes.go index 3603b760176..f92a466808e 100644 --- a/modules/git/notes.go +++ b/modules/git/notes.go @@ -5,10 +5,10 @@ package git import ( "context" - "io" + "path" "strings" - "gitea.dev/modules/log" + "gitea.dev/modules/setting" ) // NotesRef is the git ref where Gitea will look for git-notes data. @@ -17,83 +17,73 @@ const NotesRef = "refs/notes/commits" // Note stores information about a note created using git-notes. type Note struct { - Message []byte - Commit *Commit + refCommit *Commit + + BlobMessage CommitMessage // if the blob is too large, the message will be truncated + BlobSize int64 + TreePath string } // GetNote retrieves the git-notes data for a given commit. -// FIXME: Add LastCommitCache support -func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note) error { - log.Trace("Searching for git note corresponding to the commit %q in the repository %q", commitID, repo.LogString()) - notes, err := repo.GetCommit(ctx, NotesRef) +func GetNote(ctx context.Context, repo *Repository, commitID string) (*Note, error) { + noteCommit, err := repo.GetCommit(ctx, NotesRef) if err != nil { - if IsErrNotExist(err) { - return err - } - log.Error("Unable to get commit from ref %q. Error: %v", NotesRef, err) - return err + return nil, err } - path := "" - - tree := notes.Tree() - log.Trace("Found tree with ID %q while searching for git note corresponding to the commit %q", tree.ID, commitID) - + // A note for a commit is stored in a blob in the notes commit tree, with the path being the commit ID. + // The path can be "FullCommitID" or a fanout path like "ab/cdef...." or "ab/cd/ef.....". + tree := noteCommit.Tree() + entryName := commitID var entry *TreeEntry - originalCommitID := commitID - for len(commitID) > 2 { - entry, err = tree.GetTreeEntryByPath(ctx, repo, commitID) + var treePathBuf strings.Builder + for len(entryName) > 2 { + entry, err = tree.GetTreeEntryByPath(ctx, repo, entryName) if err == nil { - path += commitID + treePathBuf.WriteString(entryName) break - } - if IsErrNotExist(err) { - tree, err = tree.SubTree(ctx, repo, commitID[0:2]) - path += commitID[0:2] + "/" - commitID = commitID[2:] - } - if err != nil { - // Err may have been updated by the SubTree we need to recheck if it's again an ErrNotExist - if !IsErrNotExist(err) { - log.Error("Unable to find git note corresponding to the commit %q. Error: %v", originalCommitID, err) + } else if IsErrNotExist(err) { + fanoutDir, fanoutName := entryName[0:2], entryName[2:] + tree, err = tree.SubTree(ctx, repo, fanoutDir) + if err != nil { + return nil, err } - return err + treePathBuf.WriteString(fanoutDir) + treePathBuf.WriteByte('/') + entryName = fanoutName + } else { + return nil, err } } + if entry == nil { + return nil, ErrNotExist{ID: commitID} + } + treePath := treePathBuf.String() blob := entry.Blob(repo) - dataRc, err := blob.DataAsync(ctx) + note := &Note{TreePath: treePath, refCommit: noteCommit} + note.BlobMessage.MessageRaw, err = blob.GetBlobContent(ctx, setting.UI.MaxDisplayFileSize) if err != nil { - log.Error("Unable to read blob with ID %q. Error: %v", blob.ID, err) - return err + return nil, err } - closed := false - defer func() { - if !closed { - _ = dataRc.Close() - } - }() - d, err := io.ReadAll(dataRc) - if err != nil { - log.Error("Unable to read blob with ID %q. Error: %v", blob.ID, err) - return err - } - _ = dataRc.Close() - closed = true - note.Message = d - - treePath := "" - if idx := strings.LastIndex(path, "/"); idx > -1 { - treePath = path[:idx] - path = path[idx+1:] - } - - lastCommits, err := GetLastCommitForPaths(ctx, repo, notes, treePath, []string{path}) - if err != nil { - log.Error("Unable to get the commit for the path %q. Error: %v", treePath, err) - return err - } - note.Commit = lastCommits[path] - - return nil + note.BlobSize = blob.Size(ctx) // it should be called after the get blob content, then the "size" is cached + return note, nil +} + +func GetNoteWithLastCommit(ctx context.Context, repo *Repository, commitID string) (*Note, *Commit, error) { + note, err := GetNote(ctx, repo, commitID) + if err != nil { + return nil, nil, err + } + parentPath, entryName := path.Split(note.TreePath) + parentPath = strings.Trim(parentPath, "/") + lastCommits, err := GetLastCommitForPaths(ctx, repo, note.refCommit, parentPath, []string{entryName}) + if err != nil { + return nil, nil, err + } + lastCommit := lastCommits[entryName] + if lastCommit == nil { + return nil, nil, ErrNotExist{ID: commitID} + } + return note, lastCommit, nil } diff --git a/modules/git/notes_test.go b/modules/git/notes_test.go index dc319f57c4c..3f552897694 100644 --- a/modules/git/notes_test.go +++ b/modules/git/notes_test.go @@ -7,45 +7,40 @@ import ( "path/filepath" "testing" + "gitea.dev/modules/git/gitrepo" + "github.com/stretchr/testify/assert" ) -func TestGetNotes(t *testing.T) { - bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") - bareRepo1, err := OpenRepositoryLocal(t.Context(), bareRepo1Path) - assert.NoError(t, err) - defer bareRepo1.Close() - - note := Note{} - err = GetNote(t.Context(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653", ¬e) - assert.NoError(t, err) - assert.Equal(t, []byte("Note contents\n"), note.Message) - assert.Equal(t, "Vladimir Panteleev", note.Commit.Author.Name) -} - -func TestGetNestedNotes(t *testing.T) { - repoPath := filepath.Join(testReposDir, "repo3_notes") - repo, err := OpenRepositoryLocal(t.Context(), repoPath) +func TestGetNote(t *testing.T) { + repo, err := OpenRepositoryLocal(t.Context(), filepath.Join(testReposDir, "repo1_bare")) assert.NoError(t, err) defer repo.Close() - note := Note{} - err = GetNote(t.Context(), repo, "3e668dbfac39cbc80a9ff9c61eb565d944453ba4", ¬e) + note, err := GetNote(t.Context(), repo, "95bb4d39648ee7e325106df01a621c530863a653") assert.NoError(t, err) - assert.Equal(t, []byte("Note 2"), note.Message) - err = GetNote(t.Context(), repo, "ba0a96fa63532d6c5087ecef070b0250ed72fa47", ¬e) - assert.NoError(t, err) - assert.Equal(t, []byte("Note 1"), note.Message) -} + assert.Equal(t, "Note contents\n", note.BlobMessage.MessageUTF8()) + assert.EqualValues(t, len(note.BlobMessage.MessageRaw), note.BlobSize) -func TestGetNonExistentNotes(t *testing.T) { - bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") - bareRepo1, err := OpenRepositoryLocal(t.Context(), bareRepo1Path) - assert.NoError(t, err) - defer bareRepo1.Close() - - note := Note{} - err = GetNote(t.Context(), bareRepo1, "non_existent_sha", ¬e) - assert.Error(t, err) + _, err = GetNote(t.Context(), repo, "non_existent_sha") assert.ErrorAs(t, err, &ErrNotExist{}) } + +func TestGetNoteNestedWithCache(t *testing.T) { + repoPath, _ := filepath.Abs(filepath.Join(testReposDir, "repo3_notes")) + repo, err := OpenRepository(t.Context(), gitrepo.RepositoryManaged("repo3_notes", repoPath)) + assert.NoError(t, err) + defer repo.Close() + + note, lastCommit, err := GetNoteWithLastCommit(t.Context(), repo, "ba0a96fa63532d6c5087ecef070b0250ed72fa47") + assert.NoError(t, err) + assert.Equal(t, "Note 1", note.BlobMessage.MessageUTF8()) + assert.Equal(t, "ba0a96fa63532d6c5087ecef070b0250ed72fa47", note.TreePath) + assert.Equal(t, "Filip Navara", lastCommit.Author.Name) + + note, lastCommit, err = GetNoteWithLastCommit(t.Context(), repo, "3e668dbfac39cbc80a9ff9c61eb565d944453ba4") + assert.NoError(t, err) + assert.Equal(t, "Note 2", note.BlobMessage.MessageUTF8()) + assert.Equal(t, "3e/66/8dbfac39cbc80a9ff9c61eb565d944453ba4", note.TreePath) + assert.Equal(t, "Filip Navara", lastCommit.Author.Name) +} diff --git a/modules/git/repo.go b/modules/git/repo.go index 520b09ec649..10e04f4b274 100644 --- a/modules/git/repo.go +++ b/modules/git/repo.go @@ -16,6 +16,7 @@ import ( "sync" "time" + "gitea.dev/modules/cache" "gitea.dev/modules/git/gitcmd" "gitea.dev/modules/git/gitrepo" "gitea.dev/modules/proxy" @@ -71,6 +72,11 @@ func OpenRepository(catFileBatchCtx context.Context, repo RepositoryFacade) (*Re gitRepo := &Repository{ RepositoryBase: RepositoryBase{tagCache: newObjectCache[*Tag](), repoFacade: repo, catFileBatchCtx: catFileBatchCtx}, } + gitRepo.RepositoryBase.LastCommitCache = &LastCommitCache{ + repo: gitRepo, + ttlFn: setting.LastCommitCacheTTLSeconds, + cache: cache.GetCache(), + } if err = openRepositoryInternal(gitRepo); err != nil { return nil, err } diff --git a/modules/setting/cache.go b/modules/setting/cache.go index 5f4db322f20..115c324678a 100644 --- a/modules/setting/cache.go +++ b/modules/setting/cache.go @@ -12,7 +12,7 @@ import ( // Cache represents cache settings type Cache struct { Adapter string - Interval int + Interval int // GC Conn string `ini:"-"` TTL time.Duration `ini:"ITEM_TTL"` } @@ -22,8 +22,7 @@ var CacheService = struct { Cache `ini:"cache"` LastCommit struct { - TTL time.Duration `ini:"ITEM_TTL"` - CommitsCount int64 + TTL time.Duration `ini:"ITEM_TTL"` } `ini:"cache.last_commit"` }{ Cache: Cache{ @@ -32,11 +31,9 @@ var CacheService = struct { TTL: 16 * time.Hour, }, LastCommit: struct { - TTL time.Duration `ini:"ITEM_TTL"` - CommitsCount int64 + TTL time.Duration `ini:"ITEM_TTL"` }{ - TTL: 8760 * time.Hour, - CommitsCount: 1000, + TTL: 8760 * time.Hour, }, } @@ -61,9 +58,6 @@ func loadCacheFrom(rootCfg ConfigProvider) { default: log.Fatal("Unknown cache adapter: %s", CacheService.Adapter) } - - sec = rootCfg.Section("cache.last_commit") - CacheService.LastCommit.CommitsCount = sec.Key("COMMITS_COUNT").MustInt64(1000) } // TTLSeconds returns the TTLSeconds or unix timestamp for memcache diff --git a/routers/api/v1/repo/notes.go b/routers/api/v1/repo/notes.go index 8ec33e28f09..80e17b784b7 100644 --- a/routers/api/v1/repo/notes.go +++ b/routers/api/v1/repo/notes.go @@ -4,7 +4,6 @@ package repo import ( - "errors" "net/http" "gitea.dev/modules/git" @@ -60,32 +59,23 @@ func GetNote(ctx *context.APIContext) { getNote(ctx, sha) } -func getNote(ctx *context.APIContext, identifier string) { - if ctx.Repo.GitRepo == nil { - ctx.APIErrorInternal(errors.New("no open git repo")) - return - } - - commitID, err := ctx.Repo.GitRepo.ConvertToGitID(ctx, identifier) +func getNote(ctx *context.APIContext, ref string) { + commit, err := ctx.Repo.GitRepo.GetCommit(ctx, ref) if err != nil { ctx.APIErrorAuto(err) return } - var note git.Note - if err := git.GetNote(ctx, ctx.Repo.GitRepo, commitID.String(), ¬e); err != nil { - if git.IsErrNotExist(err) { - ctx.APIErrorNotFound("commit doesn't exist: " + identifier) - return - } - ctx.APIErrorInternal(err) + note, lastCommit, err := git.GetNoteWithLastCommit(ctx, ctx.Repo.GitRepo, commit.ID.String()) + if err != nil { + ctx.APIErrorAuto(err) return } verification := ctx.FormString("verification") == "" || ctx.FormBool("verification") files := ctx.FormString("files") == "" || ctx.FormBool("files") - cmt, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, note.Commit, nil, + cmt, err := convert.ToCommit(ctx, ctx.Repo.Repository, ctx.Repo.GitRepo, lastCommit, nil, convert.ToCommitOptions{ Stat: true, Verification: verification, @@ -95,6 +85,6 @@ func getNote(ctx *context.APIContext, identifier string) { ctx.APIErrorInternal(err) return } - apiNote := api.Note{Message: string(note.Message), Commit: cmt} + apiNote := api.Note{Message: note.BlobMessage.MessageUTF8(), Commit: cmt} ctx.JSON(http.StatusOK, apiNote) } diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go index f948c763d55..1bff2a96e3f 100644 --- a/routers/web/repo/commit.go +++ b/routers/web/repo/commit.go @@ -7,7 +7,6 @@ package repo import ( "errors" "fmt" - "html/template" "net/http" "strings" @@ -21,9 +20,9 @@ import ( unit_model "gitea.dev/models/unit" user_model "gitea.dev/models/user" "gitea.dev/modules/base" - "gitea.dev/modules/charset" "gitea.dev/modules/fileicon" "gitea.dev/modules/git" + "gitea.dev/modules/htmlutil" "gitea.dev/modules/log" "gitea.dev/modules/markup" "gitea.dev/modules/setting" @@ -406,13 +405,12 @@ func Diff(ctx *context.Context) { return } - note := &git.Note{} - err = git.GetNote(ctx, gitRepo, commitID, note) + note, noteLastCommit, err := git.GetNoteWithLastCommit(ctx, gitRepo, commitID) if err == nil { - ctx.Data["NoteCommit"] = note.Commit - ctx.Data["NoteAuthor"] = user_model.GetUserByGitAuthor(ctx, note.Commit) + ctx.Data["NoteCommit"] = noteLastCommit + ctx.Data["NoteAuthor"] = user_model.GetUserByGitAuthor(ctx, noteLastCommit) rctx := renderhelper.NewRenderContextRepoComment(ctx, ctx.Repo.Repository, renderhelper.RepoCommentOptions{CurrentRefSubURL: "commit/" + util.PathEscapeSegments(commitID)}) - htmlMessage := template.HTML(template.HTMLEscapeString(string(charset.ToUTF8WithFallback(note.Message, charset.ConvertOpts{})))) + htmlMessage := htmlutil.EscapeString(note.BlobMessage.MessageUTF8()) ctx.Data["NoteRendered"] = markup.PostProcessCommitMessage(rctx, htmlMessage) } else if !git.IsErrNotExist(err) { log.Error("GetNote: %v", err) diff --git a/services/context/repo.go b/services/context/repo.go index 45978494d4a..abf7c963414 100644 --- a/services/context/repo.go +++ b/services/context/repo.go @@ -251,18 +251,6 @@ func (r *Repository) CanCreateIssueDependencies(ctx context.Context, user *user_ return r.Repository.IsDependenciesEnabled(ctx) && r.Permission.CanWriteIssuesOrPulls(isPull) } -// GetCommitsCount returns cached commit count for current view -func (r *Repository) GetCommitsCount(ctx context.Context) (int64, error) { - if r.Commit == nil { - return 0, nil - } - contextName := r.RefFullName.ShortName() - isRef := r.RefFullName.IsBranch() || r.RefFullName.IsTag() - return cache.GetInt64(r.Repository.GetCommitsCountCacheKey(contextName, isRef), func() (int64, error) { - return git.CommitsCountOfCommit(ctx, r.Repository, r.Commit.ID.String()) - }) -} - // GetCommitGraphsCount returns cached commit count for current view func (r *Repository) GetCommitGraphsCount(ctx context.Context, hidePRRefs bool, branches, files []string) (int64, error) { cacheKey := fmt.Sprintf("commits-count-%d-graph-%t-%s-%s", r.Repository.ID, hidePRRefs, branches, files) @@ -906,7 +894,7 @@ func RepoRefByDefaultBranch() func(*Context) { ctx.Repo.RefFullName = git.RefNameFromBranch(ctx.Repo.Repository.DefaultBranch) ctx.Repo.BranchName = ctx.Repo.Repository.DefaultBranch ctx.Repo.Commit, _ = ctx.Repo.GitRepo.GetBranchCommit(ctx, ctx.Repo.BranchName) - ctx.Repo.CommitsCount, _ = ctx.Repo.GetCommitsCount(ctx) + ctx.Repo.CommitsCount, _ = git.GetCommitsCountCache(ctx, ctx.Repo.Repository, ctx.Repo.RefFullName, ctx.Repo.Commit) ctx.Data["RefFullName"] = ctx.Repo.RefFullName ctx.Data["BranchName"] = ctx.Repo.BranchName ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount @@ -1053,7 +1041,7 @@ func RepoRefByType(detectRefType git.RefType) func(*Context) { ctx.Data["CanCreateBranch"] = ctx.Repo.CanCreateBranch() // only used by the branch selector dropdown: AllowCreateNewRef - ctx.Repo.CommitsCount, err = ctx.Repo.GetCommitsCount(ctx) + ctx.Repo.CommitsCount, err = git.GetCommitsCountCache(ctx, ctx.Repo.Repository, ctx.Repo.RefFullName, ctx.Repo.Commit) if err != nil { ctx.ServerError("GetCommitsCount", err) return @@ -1068,7 +1056,6 @@ func RepoRefByType(detectRefType git.RefType) func(*Context) { } } ctx.Data["CommitsCount"] = ctx.Repo.CommitsCount - ctx.Repo.GitRepo.LastCommitCache = git.NewLastCommitCache(ctx.Repo.CommitsCount, ctx.Repo.Repository.FullName(), ctx.Repo.GitRepo, cache.GetCache()) } } diff --git a/services/mirror/mirror_pull.go b/services/mirror/mirror_pull.go index a774eb361ce..f026bf19fa4 100644 --- a/services/mirror/mirror_pull.go +++ b/services/mirror/mirror_pull.go @@ -11,7 +11,6 @@ import ( repo_model "gitea.dev/models/repo" system_model "gitea.dev/models/system" - "gitea.dev/modules/cache" "gitea.dev/modules/git" "gitea.dev/modules/git/gitcmd" giturl "gitea.dev/modules/git/url" @@ -266,7 +265,7 @@ func runSync(ctx context.Context, m *repo_model.Mirror) ([]*repo_module.SyncResu } for _, branch := range branches { - cache.Remove(m.Repo.GetCommitsCountCacheKey(branch, true)) + git.RemoveCommitsCountCache(m.Repo, git.RefNameFromBranch(branch)) } m.UpdatedUnix = timeutil.TimeStampNow() diff --git a/services/pull/merge.go b/services/pull/merge.go index 9a52eb1954c..144d3905c5c 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -24,7 +24,6 @@ import ( repo_model "gitea.dev/models/repo" "gitea.dev/models/unit" user_model "gitea.dev/models/user" - "gitea.dev/modules/cache" "gitea.dev/modules/git" "gitea.dev/modules/git/gitcmd" "gitea.dev/modules/globallock" @@ -294,8 +293,7 @@ func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.U } // Reset cached commit count - cache.Remove(pr.Issue.Repo.GetCommitsCountCacheKey(pr.BaseBranch, true)) - + git.RemoveCommitsCountCache(pr.Issue.Repo, git.RefNameFromBranch(pr.BaseBranch)) return handleCloseCrossReferences(ctx, pr, doer) } diff --git a/services/repository/cache.go b/services/repository/cache.go index 84b232d5808..aaf998c7856 100644 --- a/services/repository/cache.go +++ b/services/repository/cache.go @@ -6,27 +6,14 @@ package repository import ( "context" - repo_model "gitea.dev/models/repo" - "gitea.dev/modules/cache" "gitea.dev/modules/git" ) -// CacheRef cachhe last commit information of the branch or the tag -func CacheRef(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, fullRefName git.RefName) error { +// CacheRef caches last commit information of the branch or the tag +func CacheRef(ctx context.Context, gitRepo *git.Repository, fullRefName git.RefName) error { commit, err := gitRepo.GetCommit(ctx, fullRefName.String()) if err != nil { return err } - - if gitRepo.LastCommitCache == nil { - commitsCount, err := cache.GetInt64(repo.GetCommitsCountCacheKey(fullRefName.ShortName(), true), func() (int64, error) { - return git.CommitsCountOfCommit(ctx, repo, commit.ID.String()) - }) - if err != nil { - return err - } - gitRepo.LastCommitCache = git.NewLastCommitCache(commitsCount, repo.FullName(), gitRepo, cache.GetCache()) - } - return commit.CacheCommit(ctx, gitRepo) } diff --git a/services/repository/files/content.go b/services/repository/files/content.go index 93252f59d18..652d5f794e8 100644 --- a/services/repository/files/content.go +++ b/services/repository/files/content.go @@ -11,7 +11,6 @@ import ( "strings" repo_model "gitea.dev/models/repo" - "gitea.dev/modules/cache" "gitea.dev/modules/git" "gitea.dev/modules/lfs" "gitea.dev/modules/setting" @@ -121,21 +120,7 @@ func GetFileContents(ctx context.Context, repo *repo_model.Repository, gitRepo * return getFileContentsByEntryInternal(ctx, repo, gitRepo, refCommit, entry, opts) } -func addLastCommitCache(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, cacheKey, fullName, sha string) error { - if gitRepo.LastCommitCache == nil { - commitsCount, err := cache.GetInt64(cacheKey, func() (int64, error) { - return git.CommitsCountOfCommit(ctx, repo, sha) - }) - if err != nil { - return err - } - gitRepo.LastCommitCache = git.NewLastCommitCache(commitsCount, fullName, gitRepo, cache.GetCache()) - } - return nil -} - func getFileContentsByEntryInternal(ctx context.Context, repo *repo_model.Repository, gitRepo *git.Repository, refCommit *utils.RefCommit, entry *git.TreeEntry, opts GetContentsOrListOptions) (*api.ContentsResponse, error) { - refType := refCommit.RefName.RefType() commit := refCommit.Commit selfURL, err := url.Parse(repo.APIURL() + "/contents/" + util.PathEscapeSegments(opts.TreePath) + "?ref=" + url.QueryEscape(refCommit.InputRef)) if err != nil { @@ -157,11 +142,6 @@ func getFileContentsByEntryInternal(ctx context.Context, repo *repo_model.Reposi } if opts.IncludeCommitMetadata || opts.IncludeCommitMessage { - err = addLastCommitCache(ctx, repo, gitRepo, repo.GetCommitsCountCacheKey(refCommit.InputRef, refType != git.RefTypeCommit), repo.FullName(), refCommit.CommitID) - if err != nil { - return nil, err - } - lastCommit, err := refCommit.Commit.GetCommitByPath(ctx, gitRepo, opts.TreePath) if err != nil { return nil, err diff --git a/services/repository/push.go b/services/repository/push.go index a08288163cf..017a152b6db 100644 --- a/services/repository/push.go +++ b/services/repository/push.go @@ -13,7 +13,6 @@ import ( "gitea.dev/models/db" repo_model "gitea.dev/models/repo" user_model "gitea.dev/models/user" - "gitea.dev/modules/cache" "gitea.dev/modules/git" "gitea.dev/modules/graceful" "gitea.dev/modules/log" @@ -205,7 +204,7 @@ func pushQueueHandleUpdates(optsList []*repo_module.PushUpdateOptions) error { notify_service.PushCommits(ctx, pusher, repo, opts, commits) // Cache for big repository - if err := CacheRef(graceful.GetManager().HammerContext(), repo, gitRepo, opts.RefFullName); err != nil { + if err := CacheRef(graceful.GetManager().HammerContext(), gitRepo, opts.RefFullName); err != nil { log.Error("repo_module.CacheRef %s/%s failed: %v", repo.ID, branch, err) } } else { @@ -309,16 +308,7 @@ func pushUpdateBranch(ctx context.Context, repo *repo_model.Repository, gitRepo OldCommitID: opts.OldCommitID, NewCommitID: opts.NewCommitID, }) - - if isForcePush { - log.Trace("Push %s is a force push", opts.NewCommitID) - - cache.Remove(repo.GetCommitsCountCacheKey(opts.RefName(), true)) - } else { - // TODO: increment update the commit count cache but not remove - cache.Remove(repo.GetCommitsCountCacheKey(opts.RefName(), true)) - } - + git.RemoveCommitsCountCache(repo, opts.RefFullName) return l, nil } diff --git a/tests/integration/repo_test.go b/tests/integration/repo_test.go index 508e9db8a92..57fea96e3d8 100644 --- a/tests/integration/repo_test.go +++ b/tests/integration/repo_test.go @@ -132,14 +132,8 @@ func testViewRepoWithCache(t *testing.T) { } // FIXME: these test don't seem quite right, no enough assert - // no last commit cache - testView(t) - // enable last commit cache for all repositories - defer test.MockVariableValue(&setting.CacheService.LastCommit.CommitsCount, 0)() - // first view will not hit the cache - testView(t) - // second view will hit the cache - testView(t) + testView(t) // first view will not hit the cache, need execute git operations + testView(t) // second view will hit the cache } func testViewRepoPrivate(t *testing.T) {