kgess-mirror/forgejo
1
0
Fork 0
mirror of https://codeberg.org/forgejo/forgejo.git synced 2026-01-16 23:12:07 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 19

chore: simplify GetNote (#9985)

Browse source 
Return the Note object (avoid C-style functions).

Motivation to refactor this function is to avoid the function that uses last commit cache for git-notes, because it is not needed at the scale of git-notes. In the worst case it can be considered to make a patch to git to get the message and commitID, because git seems to have efficient code to do this (for getting messages, but does not expose the commit id).

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9985
Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>
This commit is contained in:
Gusted 2025-11-06 13:23:35 +01:00 • committed by Gusted
parent 54b3066e45
commit f9a6460cec
5 changed files with 29 additions and 41 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

34
modules/git/notes.go
View file

@ -1,4 +1,5 @@
// Copyright 2019 The Gitea Authors. All rights reserved.
// Copyright 2025 The Forgejo Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package git
@ -7,7 +8,6 @@ import (
"context"
"io"
"os"
"strings"
"forgejo.org/modules/log"
)
@ -23,16 +23,14 @@ type Note struct {
}
// 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 {
func GetNote(ctx context.Context, repo *Repository, commitID string) (*Note, error) {
log.Trace("Searching for git note corresponding to the commit %q in the repository %q", commitID, repo.Path)
notes, err := repo.GetCommit(NotesRef)
if err != nil {
if IsErrNotExist(err) {
return err
if !IsErrNotExist(err) {
log.Error("Unable to get commit from ref %q. Error: %v", NotesRef, err)
}
log.Error("Unable to get commit from ref %q. Error: %v", NotesRef, err)
return err
return nil, err
}
path := ""
@ -58,7 +56,7 @@ func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note)
if !IsErrNotExist(err) {
log.Error("Unable to find git note corresponding to the commit %q. Error: %v", originalCommitID, err)
}
return err
return nil, err
}
}
@ -66,7 +64,7 @@ func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note)
dataRc, err := blob.DataAsync()
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() {
@ -77,26 +75,18 @@ func GetNote(ctx context.Context, repo *Repository, commitID string, note *Note)
d, err := io.ReadAll(dataRc)
if err != nil {
log.Error("Unable to read blob with ID %q. Error: %v", blob.ID, err)
return err
return nil, 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, notes, treePath, []string{path})
lastCommit, err := repo.getCommitByPathWithID(notes.ID, path)
if err != nil {
log.Error("Unable to get the commit for the path %q. Error: %v", treePath, err)
return err
log.Error("Unable to get the commit for the path %q. Error: %v", path, err)
return nil, err
}
note.Commit = lastCommits[path]
return nil
return &Note{Message: d, Commit: lastCommit}, nil
}
func SetNote(ctx context.Context, repo *Repository, commitID, notes, doerName, doerEmail string) error {

26
modules/git/notes_test.go
View file

@ -29,11 +29,11 @@ func TestGetNotes(t *testing.T) {
require.NoError(t, err)
defer bareRepo1.Close()
note := git.Note{}
err = git.GetNote(t.Context(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653", &note)
note, err := git.GetNote(t.Context(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653")
require.NoError(t, err)
assert.Equal(t, []byte("Note contents\n"), note.Message)
assert.Equal(t, "Vladimir Panteleev", note.Commit.Author.Name)
assert.Equal(t, "ca6b5ddf303169a72d2a2971acde4f6eea194e5c", note.Commit.ID.String())
}
func TestGetNestedNotes(t *testing.T) {
@ -42,13 +42,14 @@ func TestGetNestedNotes(t *testing.T) {
require.NoError(t, err)
defer repo.Close()
note := git.Note{}
err = git.GetNote(t.Context(), repo, "3e668dbfac39cbc80a9ff9c61eb565d944453ba4", &note)
note, err := git.GetNote(t.Context(), repo, "3e668dbfac39cbc80a9ff9c61eb565d944453ba4")
require.NoError(t, err)
assert.Equal(t, []byte("Note 2"), note.Message)
err = git.GetNote(t.Context(), repo, "ba0a96fa63532d6c5087ecef070b0250ed72fa47", &note)
assert.Equal(t, "654c8b6b63c08bf37f638d3f521626b7fbbd4d37", note.Commit.ID.String())
note, err = git.GetNote(t.Context(), repo, "ba0a96fa63532d6c5087ecef070b0250ed72fa47")
require.NoError(t, err)
assert.Equal(t, []byte("Note 1"), note.Message)
assert.Equal(t, "654c8b6b63c08bf37f638d3f521626b7fbbd4d37", note.Commit.ID.String())
}
func TestGetNonExistentNotes(t *testing.T) {
@ -57,10 +58,10 @@ func TestGetNonExistentNotes(t *testing.T) {
require.NoError(t, err)
defer bareRepo1.Close()
note := git.Note{}
err = git.GetNote(t.Context(), bareRepo1, "non_existent_sha", &note)
note, err := git.GetNote(t.Context(), bareRepo1, "non_existent_sha")
require.Error(t, err)
assert.IsType(t, git.ErrNotExist{}, err)
assert.True(t, git.IsErrNotExist(err))
assert.Nil(t, note)
}
func TestSetNote(t *testing.T) {
@ -75,8 +76,7 @@ func TestSetNote(t *testing.T) {
require.NoError(t, git.SetNote(t.Context(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653", "This is a new note", "Test", "test@test.com"))
note := git.Note{}
err = git.GetNote(t.Context(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653", &note)
note, err := git.GetNote(t.Context(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653")
require.NoError(t, err)
assert.Equal(t, []byte("This is a new note\n"), note.Message)
assert.Equal(t, "Test", note.Commit.Author.Name)
@ -95,8 +95,8 @@ func TestRemoveNote(t *testing.T) {
require.NoError(t, git.RemoveNote(t.Context(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653"))
note := git.Note{}
err = git.GetNote(t.Context(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653", &note)
note, err := git.GetNote(t.Context(), bareRepo1, "95bb4d39648ee7e325106df01a621c530863a653")
require.Error(t, err)
assert.IsType(t, git.ErrNotExist{}, err)
assert.True(t, git.IsErrNotExist(err))
assert.Nil(t, note)
}

4
routers/api/v1/repo/notes.go
View file

@ -78,8 +78,8 @@ func getNote(ctx *context.APIContext, identifier string) {
return
}
var note git.Note
if err := git.GetNote(ctx, ctx.Repo.GitRepo, commitID.String(), &note); err != nil {
note, err := git.GetNote(ctx, ctx.Repo.GitRepo, commitID.String())
if err != nil {
if git.IsErrNotExist(err) {
ctx.NotFound(identifier)
return

3
routers/web/repo/commit.go
View file

@ -402,8 +402,7 @@ func Diff(ctx *context.Context) {
return
}
note := &git.Note{}
err = git.GetNote(ctx, ctx.Repo.GitRepo, commitID, note)
note, err := git.GetNote(ctx, ctx.Repo.GitRepo, commitID)
if err == nil {
ctx.Data["NoteCommit"] = note.Commit
ctx.Data["NoteAuthor"] = user_model.ValidateCommitWithEmail(ctx, note.Commit)

3
routers/web/repo/pull.go
View file

@ -997,8 +997,7 @@ func viewPullFiles(ctx *context.Context, specifiedStartCommit, specifiedEndCommi
return
}
note := &git.Note{}
err = git.GetNote(ctx, ctx.Repo.GitRepo, specifiedEndCommit, note)
note, err := git.GetNote(ctx, ctx.Repo.GitRepo, specifiedEndCommit)
if err == nil {
ctx.Data["NoteCommit"] = note.Commit
ctx.Data["NoteAuthor"] = user_model.ValidateCommitWithEmail(ctx, note.Commit)