From c88e71c1d2d1af0ac9de73ae43484fa9063cb8d5 Mon Sep 17 00:00:00 2001
From: Lunny Xiao <xiaolunwen@gmail.com>
Date: Sat, 15 Mar 2025 20:14:56 -0700
Subject: [PATCH] Refactor functions to reduce repopath expose (#33892)
---
models/git/commit_status.go | 7 +++----
modules/indexer/stats/db.go | 10 +++++-----
modules/repository/commits.go | 13 +++++++------
modules/repository/commits_test.go | 10 +++++-----
services/actions/notifier.go | 4 ++--
services/webhook/notifier.go | 6 +++---
6 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/models/git/commit_status.go b/models/git/commit_status.go
index 5432eea55e..d1c94aa023 100644
--- a/models/git/commit_status.go
+++ b/models/git/commit_status.go
@@ -453,9 +453,8 @@ func NewCommitStatus(ctx context.Context, opts NewCommitStatusOptions) error {
return fmt.Errorf("NewCommitStatus[nil, %s]: no repository specified", opts.SHA)
}
- repoPath := opts.Repo.RepoPath()
if opts.Creator == nil {
- return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", repoPath, opts.SHA)
+ return fmt.Errorf("NewCommitStatus[%s, %s]: no user specified", opts.Repo.FullName(), opts.SHA)
}
ctx, committer, err := db.TxContext(ctx)
@@ -477,13 +476,13 @@ func NewCommitStatus(ctx context.Context, opts NewCommitStatusOptions) error {
opts.CommitStatus.CreatorID = opts.Creator.ID
opts.CommitStatus.RepoID = opts.Repo.ID
opts.CommitStatus.Index = idx
- log.Debug("NewCommitStatus[%s, %s]: %d", repoPath, opts.SHA, opts.CommitStatus.Index)
+ log.Debug("NewCommitStatus[%s, %s]: %d", opts.Repo.FullName(), opts.SHA, opts.CommitStatus.Index)
opts.CommitStatus.ContextHash = hashCommitStatusContext(opts.CommitStatus.Context)
// Insert new CommitStatus
if _, err = db.GetEngine(ctx).Insert(opts.CommitStatus); err != nil {
- return fmt.Errorf("insert CommitStatus[%s, %s]: %w", repoPath, opts.SHA, err)
+ return fmt.Errorf("insert CommitStatus[%s, %s]: %w", opts.Repo.FullName(), opts.SHA, err)
}
return committer.Commit()
diff --git a/modules/indexer/stats/db.go b/modules/indexer/stats/db.go
index 98a977c700..067a6f609b 100644
--- a/modules/indexer/stats/db.go
+++ b/modules/indexer/stats/db.go
@@ -49,10 +49,10 @@ func (db *DBIndexer) Index(id int64) error {
commitID, err := gitRepo.GetBranchCommitID(repo.DefaultBranch)
if err != nil {
if git.IsErrBranchNotExist(err) || git.IsErrNotExist(err) || setting.IsInTesting {
- log.Debug("Unable to get commit ID for default branch %s in %s ... skipping this repository", repo.DefaultBranch, repo.RepoPath())
+ log.Debug("Unable to get commit ID for default branch %s in %s ... skipping this repository", repo.DefaultBranch, repo.FullName())
return nil
}
- log.Error("Unable to get commit ID for default branch %s in %s. Error: %v", repo.DefaultBranch, repo.RepoPath(), err)
+ log.Error("Unable to get commit ID for default branch %s in %s. Error: %v", repo.DefaultBranch, repo.FullName(), err)
return err
}
@@ -65,17 +65,17 @@ func (db *DBIndexer) Index(id int64) error {
stats, err := gitRepo.GetLanguageStats(commitID)
if err != nil {
if !setting.IsInTesting {
- log.Error("Unable to get language stats for ID %s for default branch %s in %s. Error: %v", commitID, repo.DefaultBranch, repo.RepoPath(), err)
+ log.Error("Unable to get language stats for ID %s for default branch %s in %s. Error: %v", commitID, repo.DefaultBranch, repo.FullName(), err)
}
return err
}
err = repo_model.UpdateLanguageStats(ctx, repo, commitID, stats)
if err != nil {
- log.Error("Unable to update language stats for ID %s for default branch %s in %s. Error: %v", commitID, repo.DefaultBranch, repo.RepoPath(), err)
+ log.Error("Unable to update language stats for ID %s for default branch %s in %s. Error: %v", commitID, repo.DefaultBranch, repo.FullName(), err)
return err
}
- log.Debug("DBIndexer completed language stats for ID %s for default branch %s in %s. stats count: %d", commitID, repo.DefaultBranch, repo.RepoPath(), len(stats))
+ log.Debug("DBIndexer completed language stats for ID %s for default branch %s in %s. stats count: %d", commitID, repo.DefaultBranch, repo.FullName(), len(stats))
return nil
}
diff --git a/modules/repository/commits.go b/modules/repository/commits.go
index 6e4b75d5ca..16520fb28a 100644
--- a/modules/repository/commits.go
+++ b/modules/repository/commits.go
@@ -10,6 +10,7 @@ import (
"time"
"code.gitea.io/gitea/models/avatars"
+ repo_model "code.gitea.io/gitea/models/repo"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/cache"
"code.gitea.io/gitea/modules/git"
@@ -43,7 +44,7 @@ func NewPushCommits() *PushCommits {
}
// ToAPIPayloadCommit converts a single PushCommit to an api.PayloadCommit object.
-func ToAPIPayloadCommit(ctx context.Context, emailUsers map[string]*user_model.User, repoPath, repoLink string, commit *PushCommit) (*api.PayloadCommit, error) {
+func ToAPIPayloadCommit(ctx context.Context, emailUsers map[string]*user_model.User, repo *repo_model.Repository, commit *PushCommit) (*api.PayloadCommit, error) {
var err error
authorUsername := ""
author, ok := emailUsers[commit.AuthorEmail]
@@ -70,7 +71,7 @@ func ToAPIPayloadCommit(ctx context.Context, emailUsers map[string]*user_model.U
committerUsername = committer.Name
}
- fileStatus, err := git.GetCommitFileStatus(ctx, repoPath, commit.Sha1)
+ fileStatus, err := git.GetCommitFileStatus(ctx, repo.RepoPath(), commit.Sha1)
if err != nil {
return nil, fmt.Errorf("FileStatus [commit_sha1: %s]: %w", commit.Sha1, err)
}
@@ -78,7 +79,7 @@ func ToAPIPayloadCommit(ctx context.Context, emailUsers map[string]*user_model.U
return &api.PayloadCommit{
ID: commit.Sha1,
Message: commit.Message,
- URL: fmt.Sprintf("%s/commit/%s", repoLink, url.PathEscape(commit.Sha1)),
+ URL: fmt.Sprintf("%s/commit/%s", repo.HTMLURL(), url.PathEscape(commit.Sha1)),
Author: &api.PayloadUser{
Name: commit.AuthorName,
Email: commit.AuthorEmail,
@@ -98,14 +99,14 @@ func ToAPIPayloadCommit(ctx context.Context, emailUsers map[string]*user_model.U
// ToAPIPayloadCommits converts a PushCommits object to api.PayloadCommit format.
// It returns all converted commits and, if provided, the head commit or an error otherwise.
-func (pc *PushCommits) ToAPIPayloadCommits(ctx context.Context, repoPath, repoLink string) ([]*api.PayloadCommit, *api.PayloadCommit, error) {
+func (pc *PushCommits) ToAPIPayloadCommits(ctx context.Context, repo *repo_model.Repository) ([]*api.PayloadCommit, *api.PayloadCommit, error) {
commits := make([]*api.PayloadCommit, len(pc.Commits))
var headCommit *api.PayloadCommit
emailUsers := make(map[string]*user_model.User)
for i, commit := range pc.Commits {
- apiCommit, err := ToAPIPayloadCommit(ctx, emailUsers, repoPath, repoLink, commit)
+ apiCommit, err := ToAPIPayloadCommit(ctx, emailUsers, repo, commit)
if err != nil {
return nil, nil, err
}
@@ -117,7 +118,7 @@ func (pc *PushCommits) ToAPIPayloadCommits(ctx context.Context, repoPath, repoLi
}
if pc.HeadCommit != nil && headCommit == nil {
var err error
- headCommit, err = ToAPIPayloadCommit(ctx, emailUsers, repoPath, repoLink, pc.HeadCommit)
+ headCommit, err = ToAPIPayloadCommit(ctx, emailUsers, repo, pc.HeadCommit)
if err != nil {
return nil, nil, err
}
diff --git a/modules/repository/commits_test.go b/modules/repository/commits_test.go
index 3afc116e68..e8dbf0b4e9 100644
--- a/modules/repository/commits_test.go
+++ b/modules/repository/commits_test.go
@@ -50,14 +50,14 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) {
pushCommits.HeadCommit = &PushCommit{Sha1: "69554a6"}
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 16})
- payloadCommits, headCommit, err := pushCommits.ToAPIPayloadCommits(git.DefaultContext, repo.RepoPath(), "/user2/repo16")
+ payloadCommits, headCommit, err := pushCommits.ToAPIPayloadCommits(git.DefaultContext, repo)
assert.NoError(t, err)
assert.Len(t, payloadCommits, 3)
assert.NotNil(t, headCommit)
assert.Equal(t, "69554a6", payloadCommits[0].ID)
assert.Equal(t, "not signed commit", payloadCommits[0].Message)
- assert.Equal(t, "/user2/repo16/commit/69554a6", payloadCommits[0].URL)
+ assert.Equal(t, "https://try.gitea.io/user2/repo16/commit/69554a6", payloadCommits[0].URL)
assert.Equal(t, "User2", payloadCommits[0].Committer.Name)
assert.Equal(t, "user2", payloadCommits[0].Committer.UserName)
assert.Equal(t, "User2", payloadCommits[0].Author.Name)
@@ -68,7 +68,7 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) {
assert.Equal(t, "27566bd", payloadCommits[1].ID)
assert.Equal(t, "good signed commit (with not yet validated email)", payloadCommits[1].Message)
- assert.Equal(t, "/user2/repo16/commit/27566bd", payloadCommits[1].URL)
+ assert.Equal(t, "https://try.gitea.io/user2/repo16/commit/27566bd", payloadCommits[1].URL)
assert.Equal(t, "User2", payloadCommits[1].Committer.Name)
assert.Equal(t, "user2", payloadCommits[1].Committer.UserName)
assert.Equal(t, "User2", payloadCommits[1].Author.Name)
@@ -79,7 +79,7 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) {
assert.Equal(t, "5099b81", payloadCommits[2].ID)
assert.Equal(t, "good signed commit", payloadCommits[2].Message)
- assert.Equal(t, "/user2/repo16/commit/5099b81", payloadCommits[2].URL)
+ assert.Equal(t, "https://try.gitea.io/user2/repo16/commit/5099b81", payloadCommits[2].URL)
assert.Equal(t, "User2", payloadCommits[2].Committer.Name)
assert.Equal(t, "user2", payloadCommits[2].Committer.UserName)
assert.Equal(t, "User2", payloadCommits[2].Author.Name)
@@ -90,7 +90,7 @@ func TestPushCommits_ToAPIPayloadCommits(t *testing.T) {
assert.Equal(t, "69554a6", headCommit.ID)
assert.Equal(t, "not signed commit", headCommit.Message)
- assert.Equal(t, "/user2/repo16/commit/69554a6", headCommit.URL)
+ assert.Equal(t, "https://try.gitea.io/user2/repo16/commit/69554a6", headCommit.URL)
assert.Equal(t, "User2", headCommit.Committer.Name)
assert.Equal(t, "user2", headCommit.Committer.UserName)
assert.Equal(t, "User2", headCommit.Author.Name)
diff --git a/services/actions/notifier.go b/services/actions/notifier.go
index 1a23b4e0c5..831cde3523 100644
--- a/services/actions/notifier.go
+++ b/services/actions/notifier.go
@@ -532,7 +532,7 @@ func (n *actionsNotifier) PushCommits(ctx context.Context, pusher *user_model.Us
ctx = withMethod(ctx, "PushCommits")
apiPusher := convert.ToUser(ctx, pusher, nil)
- apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(ctx, repo.RepoPath(), repo.HTMLURL())
+ apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(ctx, repo)
if err != nil {
log.Error("commits.ToAPIPayloadCommits failed: %v", err)
return
@@ -593,7 +593,7 @@ func (n *actionsNotifier) SyncPushCommits(ctx context.Context, pusher *user_mode
ctx = withMethod(ctx, "SyncPushCommits")
apiPusher := convert.ToUser(ctx, pusher, nil)
- apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(ctx, repo.RepoPath(), repo.HTMLURL())
+ apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(ctx, repo)
if err != nil {
log.Error("commits.ToAPIPayloadCommits failed: %v", err)
return
diff --git a/services/webhook/notifier.go b/services/webhook/notifier.go
index 7d779cd527..9e3f21de29 100644
--- a/services/webhook/notifier.go
+++ b/services/webhook/notifier.go
@@ -606,7 +606,7 @@ func (m *webhookNotifier) IssueChangeMilestone(ctx context.Context, doer *user_m
func (m *webhookNotifier) PushCommits(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) {
apiPusher := convert.ToUser(ctx, pusher, nil)
- apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(ctx, repo.RepoPath(), repo.HTMLURL())
+ apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(ctx, repo)
if err != nil {
log.Error("commits.ToAPIPayloadCommits failed: %v", err)
return
@@ -845,7 +845,7 @@ func (m *webhookNotifier) DeleteRelease(ctx context.Context, doer *user_model.Us
func (m *webhookNotifier) SyncPushCommits(ctx context.Context, pusher *user_model.User, repo *repo_model.Repository, opts *repository.PushUpdateOptions, commits *repository.PushCommits) {
apiPusher := convert.ToUser(ctx, pusher, nil)
- apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(ctx, repo.RepoPath(), repo.HTMLURL())
+ apiCommits, apiHeadCommit, err := commits.ToAPIPayloadCommits(ctx, repo)
if err != nil {
log.Error("commits.ToAPIPayloadCommits failed: %v", err)
return
@@ -869,7 +869,7 @@ func (m *webhookNotifier) SyncPushCommits(ctx context.Context, pusher *user_mode
func (m *webhookNotifier) CreateCommitStatus(ctx context.Context, repo *repo_model.Repository, commit *repository.PushCommit, sender *user_model.User, status *git_model.CommitStatus) {
apiSender := convert.ToUser(ctx, sender, nil)
- apiCommit, err := repository.ToAPIPayloadCommit(ctx, map[string]*user_model.User{}, repo.RepoPath(), repo.HTMLURL(), commit)
+ apiCommit, err := repository.ToAPIPayloadCommit(ctx, map[string]*user_model.User{}, repo, commit)
if err != nil {
log.Error("commits.ToAPIPayloadCommits failed: %v", err)
return