From 1418288734b430330023e4e17747175e4fea2f41 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Fri, 18 Sep 2020 14:09:26 +0200 Subject: [PATCH] Refactor: move Commit To APIFormat Code & Lot of StopWatch related things (#12729) * move GitCommit to APIFormat convertion into convert package * rename Commit convert functions * move stopwatch to api convertion into convert package & rm unused code & extend test * fix compare time * Gitea not Gogs ;) --- integrations/api_issue_stopwatch_test.go | 20 ++- models/issue_stopwatch.go | 68 +--------- modules/convert/convert.go | 59 +------- modules/convert/git_commit.go | 165 +++++++++++++++++++++++ modules/convert/issue.go | 40 ++++++ routers/api/v1/repo/commits.go | 101 +------------- routers/api/v1/repo/hook.go | 2 +- routers/api/v1/repo/issue_stopwatch.go | 3 +- 8 files changed, 230 insertions(+), 228 deletions(-) create mode 100644 modules/convert/git_commit.go diff --git a/integrations/api_issue_stopwatch_test.go b/integrations/api_issue_stopwatch_test.go index e0fe00c415..39b9b97411 100644 --- a/integrations/api_issue_stopwatch_test.go +++ b/integrations/api_issue_stopwatch_test.go @@ -7,6 +7,7 @@ package integrations import ( "net/http" "testing" + "time" "code.gitea.io/gitea/models" api "code.gitea.io/gitea/modules/structs" @@ -26,12 +27,19 @@ func TestAPIListStopWatches(t *testing.T) { resp := session.MakeRequest(t, req, http.StatusOK) var apiWatches []*api.StopWatch DecodeJSON(t, resp, &apiWatches) - expect := models.AssertExistsAndLoadBean(t, &models.Stopwatch{UserID: owner.ID}).(*models.Stopwatch) - expectAPI, _ := expect.APIFormat() - assert.Len(t, apiWatches, 1) - - assert.EqualValues(t, expectAPI.IssueIndex, apiWatches[0].IssueIndex) - assert.EqualValues(t, expectAPI.Created.Unix(), apiWatches[0].Created.Unix()) + stopwatch := models.AssertExistsAndLoadBean(t, &models.Stopwatch{UserID: owner.ID}).(*models.Stopwatch) + issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: stopwatch.IssueID}).(*models.Issue) + if assert.Len(t, apiWatches, 1) { + assert.EqualValues(t, stopwatch.CreatedUnix.AsTime().Unix(), apiWatches[0].Created.Unix()) + apiWatches[0].Created = time.Time{} + assert.EqualValues(t, api.StopWatch{ + Created: time.Time{}, + IssueIndex: issue.Index, + IssueTitle: issue.Title, + RepoName: repo.Name, + RepoOwnerName: repo.OwnerName, + }, *apiWatches[0]) + } } func TestAPIStopStopWatches(t *testing.T) { diff --git a/models/issue_stopwatch.go b/models/issue_stopwatch.go index 789f8ebdfc..4b2bf1505d 100644 --- a/models/issue_stopwatch.go +++ b/models/issue_stopwatch.go @@ -8,7 +8,6 @@ import ( "fmt" "time" - api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" ) @@ -20,9 +19,6 @@ type Stopwatch struct { CreatedUnix timeutil.TimeStamp `xorm:"created"` } -// Stopwatches is a List ful of Stopwatch -type Stopwatches []Stopwatch - func getStopwatch(e Engine, userID, issueID int64) (sw *Stopwatch, exists bool, err error) { sw = new(Stopwatch) exists, err = e. @@ -33,14 +29,14 @@ func getStopwatch(e Engine, userID, issueID int64) (sw *Stopwatch, exists bool, } // GetUserStopwatches return list of all stopwatches of a user -func GetUserStopwatches(userID int64, listOptions ListOptions) (*Stopwatches, error) { - sws := new(Stopwatches) +func GetUserStopwatches(userID int64, listOptions ListOptions) ([]*Stopwatch, error) { + sws := make([]*Stopwatch, 0, 8) sess := x.Where("stopwatch.user_id = ?", userID) if listOptions.Page != 0 { sess = listOptions.setSessionPagination(sess) } - err := sess.Find(sws) + err := sess.Find(&sws) if err != nil { return nil, err } @@ -194,61 +190,3 @@ func SecToTime(duration int64) string { return hrs } - -// APIFormat convert Stopwatch type to api.StopWatch type -func (sw *Stopwatch) APIFormat() (api.StopWatch, error) { - issue, err := getIssueByID(x, sw.IssueID) - if err != nil { - return api.StopWatch{}, err - } - if err := issue.LoadRepo(); err != nil { - return api.StopWatch{}, err - } - return api.StopWatch{ - Created: sw.CreatedUnix.AsTime(), - IssueIndex: issue.Index, - IssueTitle: issue.Title, - RepoOwnerName: issue.Repo.OwnerName, - RepoName: issue.Repo.Name, - }, nil -} - -// APIFormat convert Stopwatches type to api.StopWatches type -func (sws Stopwatches) APIFormat() (api.StopWatches, error) { - result := api.StopWatches(make([]api.StopWatch, 0, len(sws))) - - issueCache := make(map[int64]*Issue) - repoCache := make(map[int64]*Repository) - var ( - issue *Issue - repo *Repository - ok bool - err error - ) - - for _, sw := range sws { - issue, ok = issueCache[sw.IssueID] - if !ok { - issue, err = GetIssueByID(sw.IssueID) - if err != nil { - return nil, err - } - } - repo, ok = repoCache[issue.RepoID] - if !ok { - repo, err = GetRepositoryByID(issue.RepoID) - if err != nil { - return nil, err - } - } - - result = append(result, api.StopWatch{ - Created: sw.CreatedUnix.AsTime(), - IssueIndex: issue.Index, - IssueTitle: issue.Title, - RepoOwnerName: repo.OwnerName, - RepoName: repo.Name, - }) - } - return result, nil -} diff --git a/modules/convert/convert.go b/modules/convert/convert.go index 03a84115a5..ec676002b9 100644 --- a/modules/convert/convert.go +++ b/modules/convert/convert.go @@ -7,7 +7,6 @@ package convert import ( "fmt" - "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/git" @@ -43,7 +42,7 @@ func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models. return &api.Branch{ Name: b.Name, - Commit: ToCommit(repo, c), + Commit: ToPayloadCommit(repo, c), Protected: false, RequiredApprovals: 0, EnableStatusCheck: false, @@ -55,7 +54,7 @@ func ToBranch(repo *models.Repository, b *git.Branch, c *git.Commit, bp *models. branch := &api.Branch{ Name: b.Name, - Commit: ToCommit(repo, c), + Commit: ToPayloadCommit(repo, c), Protected: true, RequiredApprovals: bp.RequiredApprovals, EnableStatusCheck: bp.EnableStatusCheck, @@ -142,41 +141,6 @@ func ToTag(repo *models.Repository, t *git.Tag) *api.Tag { } } -// ToCommit convert a git.Commit to api.PayloadCommit -func ToCommit(repo *models.Repository, c *git.Commit) *api.PayloadCommit { - authorUsername := "" - if author, err := models.GetUserByEmail(c.Author.Email); err == nil { - authorUsername = author.Name - } else if !models.IsErrUserNotExist(err) { - log.Error("GetUserByEmail: %v", err) - } - - committerUsername := "" - if committer, err := models.GetUserByEmail(c.Committer.Email); err == nil { - committerUsername = committer.Name - } else if !models.IsErrUserNotExist(err) { - log.Error("GetUserByEmail: %v", err) - } - - return &api.PayloadCommit{ - ID: c.ID.String(), - Message: c.Message(), - URL: util.URLJoin(repo.HTMLURL(), "commit", c.ID.String()), - Author: &api.PayloadUser{ - Name: c.Author.Name, - Email: c.Author.Email, - UserName: authorUsername, - }, - Committer: &api.PayloadUser{ - Name: c.Committer.Name, - Email: c.Committer.Email, - UserName: committerUsername, - }, - Timestamp: c.Author.When, - Verification: ToVerification(c), - } -} - // ToVerification convert a git.Commit.Signature to an api.PayloadCommitVerification func ToVerification(c *git.Commit) *api.PayloadCommitVerification { verif := models.ParseCommitWithSignature(c) @@ -353,25 +317,6 @@ func ToAnnotatedTagObject(repo *models.Repository, commit *git.Commit) *api.Anno } } -// ToCommitUser convert a git.Signature to an api.CommitUser -func ToCommitUser(sig *git.Signature) *api.CommitUser { - return &api.CommitUser{ - Identity: api.Identity{ - Name: sig.Name, - Email: sig.Email, - }, - Date: sig.When.UTC().Format(time.RFC3339), - } -} - -// ToCommitMeta convert a git.Tag to an api.CommitMeta -func ToCommitMeta(repo *models.Repository, tag *git.Tag) *api.CommitMeta { - return &api.CommitMeta{ - SHA: tag.Object.String(), - URL: util.URLJoin(repo.APIURL(), "git/commits", tag.ID.String()), - } -} - // ToTopicResponse convert from models.Topic to api.TopicResponse func ToTopicResponse(topic *models.Topic) *api.TopicResponse { return &api.TopicResponse{ diff --git a/modules/convert/git_commit.go b/modules/convert/git_commit.go new file mode 100644 index 0000000000..55cbb3af80 --- /dev/null +++ b/modules/convert/git_commit.go @@ -0,0 +1,165 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package convert + +import ( + "time" + + "code.gitea.io/gitea/models" + "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/log" + api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" +) + +// ToCommitUser convert a git.Signature to an api.CommitUser +func ToCommitUser(sig *git.Signature) *api.CommitUser { + return &api.CommitUser{ + Identity: api.Identity{ + Name: sig.Name, + Email: sig.Email, + }, + Date: sig.When.UTC().Format(time.RFC3339), + } +} + +// ToCommitMeta convert a git.Tag to an api.CommitMeta +func ToCommitMeta(repo *models.Repository, tag *git.Tag) *api.CommitMeta { + return &api.CommitMeta{ + SHA: tag.Object.String(), + URL: util.URLJoin(repo.APIURL(), "git/commits", tag.ID.String()), + } +} + +// ToPayloadCommit convert a git.Commit to api.PayloadCommit +func ToPayloadCommit(repo *models.Repository, c *git.Commit) *api.PayloadCommit { + authorUsername := "" + if author, err := models.GetUserByEmail(c.Author.Email); err == nil { + authorUsername = author.Name + } else if !models.IsErrUserNotExist(err) { + log.Error("GetUserByEmail: %v", err) + } + + committerUsername := "" + if committer, err := models.GetUserByEmail(c.Committer.Email); err == nil { + committerUsername = committer.Name + } else if !models.IsErrUserNotExist(err) { + log.Error("GetUserByEmail: %v", err) + } + + return &api.PayloadCommit{ + ID: c.ID.String(), + Message: c.Message(), + URL: util.URLJoin(repo.HTMLURL(), "commit", c.ID.String()), + Author: &api.PayloadUser{ + Name: c.Author.Name, + Email: c.Author.Email, + UserName: authorUsername, + }, + Committer: &api.PayloadUser{ + Name: c.Committer.Name, + Email: c.Committer.Email, + UserName: committerUsername, + }, + Timestamp: c.Author.When, + Verification: ToVerification(c), + } +} + +// ToCommit convert a git.Commit to api.Commit +func ToCommit(repo *models.Repository, commit *git.Commit, userCache map[string]*models.User) (*api.Commit, error) { + + var apiAuthor, apiCommitter *api.User + + // Retrieve author and committer information + + var cacheAuthor *models.User + var ok bool + if userCache == nil { + cacheAuthor = (*models.User)(nil) + ok = false + } else { + cacheAuthor, ok = userCache[commit.Author.Email] + } + + if ok { + apiAuthor = cacheAuthor.APIFormat() + } else { + author, err := models.GetUserByEmail(commit.Author.Email) + if err != nil && !models.IsErrUserNotExist(err) { + return nil, err + } else if err == nil { + apiAuthor = author.APIFormat() + if userCache != nil { + userCache[commit.Author.Email] = author + } + } + } + + var cacheCommitter *models.User + if userCache == nil { + cacheCommitter = (*models.User)(nil) + ok = false + } else { + cacheCommitter, ok = userCache[commit.Committer.Email] + } + + if ok { + apiCommitter = cacheCommitter.APIFormat() + } else { + committer, err := models.GetUserByEmail(commit.Committer.Email) + if err != nil && !models.IsErrUserNotExist(err) { + return nil, err + } else if err == nil { + apiCommitter = committer.APIFormat() + if userCache != nil { + userCache[commit.Committer.Email] = committer + } + } + } + + // Retrieve parent(s) of the commit + apiParents := make([]*api.CommitMeta, commit.ParentCount()) + for i := 0; i < commit.ParentCount(); i++ { + sha, _ := commit.ParentID(i) + apiParents[i] = &api.CommitMeta{ + URL: repo.APIURL() + "/git/commits/" + sha.String(), + SHA: sha.String(), + } + } + + return &api.Commit{ + CommitMeta: &api.CommitMeta{ + URL: repo.APIURL() + "/git/commits/" + commit.ID.String(), + SHA: commit.ID.String(), + }, + HTMLURL: repo.HTMLURL() + "/commit/" + commit.ID.String(), + RepoCommit: &api.RepoCommit{ + URL: repo.APIURL() + "/git/commits/" + commit.ID.String(), + Author: &api.CommitUser{ + Identity: api.Identity{ + Name: commit.Committer.Name, + Email: commit.Committer.Email, + }, + Date: commit.Author.When.Format(time.RFC3339), + }, + Committer: &api.CommitUser{ + Identity: api.Identity{ + Name: commit.Committer.Name, + Email: commit.Committer.Email, + }, + Date: commit.Committer.When.Format(time.RFC3339), + }, + Message: commit.Message(), + Tree: &api.CommitMeta{ + URL: repo.APIURL() + "/git/trees/" + commit.ID.String(), + SHA: commit.ID.String(), + }, + }, + Author: apiAuthor, + Committer: apiCommitter, + Parents: apiParents, + }, nil +} diff --git a/modules/convert/issue.go b/modules/convert/issue.go index e89021cbcc..724ec8ffcf 100644 --- a/modules/convert/issue.go +++ b/modules/convert/issue.go @@ -115,6 +115,46 @@ func ToTrackedTime(t *models.TrackedTime) (apiT *api.TrackedTime) { return } +// ToStopWatches convert Stopwatch list to api.StopWatches +func ToStopWatches(sws []*models.Stopwatch) (api.StopWatches, error) { + result := api.StopWatches(make([]api.StopWatch, 0, len(sws))) + + issueCache := make(map[int64]*models.Issue) + repoCache := make(map[int64]*models.Repository) + var ( + issue *models.Issue + repo *models.Repository + ok bool + err error + ) + + for _, sw := range sws { + issue, ok = issueCache[sw.IssueID] + if !ok { + issue, err = models.GetIssueByID(sw.IssueID) + if err != nil { + return nil, err + } + } + repo, ok = repoCache[issue.RepoID] + if !ok { + repo, err = models.GetRepositoryByID(issue.RepoID) + if err != nil { + return nil, err + } + } + + result = append(result, api.StopWatch{ + Created: sw.CreatedUnix.AsTime(), + IssueIndex: issue.Index, + IssueTitle: issue.Title, + RepoOwnerName: repo.OwnerName, + RepoName: repo.Name, + }) + } + return result, nil +} + // ToTrackedTimeList converts TrackedTimeList to API format func ToTrackedTimeList(tl models.TrackedTimeList) api.TrackedTimeList { result := make([]*api.TrackedTime, 0, len(tl)) diff --git a/routers/api/v1/repo/commits.go b/routers/api/v1/repo/commits.go index 8b2dc28bdd..220bb0fd74 100644 --- a/routers/api/v1/repo/commits.go +++ b/routers/api/v1/repo/commits.go @@ -10,10 +10,10 @@ import ( "math" "net/http" "strconv" - "time" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/convert" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" @@ -73,7 +73,7 @@ func getCommit(ctx *context.APIContext, identifier string) { return } - json, err := toCommit(ctx, ctx.Repo.Repository, commit, nil) + json, err := convert.ToCommit(ctx.Repo.Repository, commit, nil) if err != nil { ctx.ServerError("toCommit", err) return @@ -193,7 +193,7 @@ func GetAllCommits(ctx *context.APIContext) { commit := commitPointer.Value.(*git.Commit) // Create json struct - apiCommits[i], err = toCommit(ctx, ctx.Repo.Repository, commit, userCache) + apiCommits[i], err = convert.ToCommit(ctx.Repo.Repository, commit, userCache) if err != nil { ctx.ServerError("toCommit", err) return @@ -215,98 +215,3 @@ func GetAllCommits(ctx *context.APIContext) { ctx.JSON(http.StatusOK, &apiCommits) } - -func toCommit(ctx *context.APIContext, repo *models.Repository, commit *git.Commit, userCache map[string]*models.User) (*api.Commit, error) { - - var apiAuthor, apiCommitter *api.User - - // Retrieve author and committer information - - var cacheAuthor *models.User - var ok bool - if userCache == nil { - cacheAuthor = ((*models.User)(nil)) - ok = false - } else { - cacheAuthor, ok = userCache[commit.Author.Email] - } - - if ok { - apiAuthor = cacheAuthor.APIFormat() - } else { - author, err := models.GetUserByEmail(commit.Author.Email) - if err != nil && !models.IsErrUserNotExist(err) { - return nil, err - } else if err == nil { - apiAuthor = author.APIFormat() - if userCache != nil { - userCache[commit.Author.Email] = author - } - } - } - - var cacheCommitter *models.User - if userCache == nil { - cacheCommitter = ((*models.User)(nil)) - ok = false - } else { - cacheCommitter, ok = userCache[commit.Committer.Email] - } - - if ok { - apiCommitter = cacheCommitter.APIFormat() - } else { - committer, err := models.GetUserByEmail(commit.Committer.Email) - if err != nil && !models.IsErrUserNotExist(err) { - return nil, err - } else if err == nil { - apiCommitter = committer.APIFormat() - if userCache != nil { - userCache[commit.Committer.Email] = committer - } - } - } - - // Retrieve parent(s) of the commit - apiParents := make([]*api.CommitMeta, commit.ParentCount()) - for i := 0; i < commit.ParentCount(); i++ { - sha, _ := commit.ParentID(i) - apiParents[i] = &api.CommitMeta{ - URL: repo.APIURL() + "/git/commits/" + sha.String(), - SHA: sha.String(), - } - } - - return &api.Commit{ - CommitMeta: &api.CommitMeta{ - URL: repo.APIURL() + "/git/commits/" + commit.ID.String(), - SHA: commit.ID.String(), - }, - HTMLURL: repo.HTMLURL() + "/commit/" + commit.ID.String(), - RepoCommit: &api.RepoCommit{ - URL: repo.APIURL() + "/git/commits/" + commit.ID.String(), - Author: &api.CommitUser{ - Identity: api.Identity{ - Name: commit.Committer.Name, - Email: commit.Committer.Email, - }, - Date: commit.Author.When.Format(time.RFC3339), - }, - Committer: &api.CommitUser{ - Identity: api.Identity{ - Name: commit.Committer.Name, - Email: commit.Committer.Email, - }, - Date: commit.Committer.When.Format(time.RFC3339), - }, - Message: commit.Message(), - Tree: &api.CommitMeta{ - URL: repo.APIURL() + "/git/trees/" + commit.ID.String(), - SHA: commit.ID.String(), - }, - }, - Author: apiAuthor, - Committer: apiCommitter, - Parents: apiParents, - }, nil -} diff --git a/routers/api/v1/repo/hook.go b/routers/api/v1/repo/hook.go index 3758493344..1a38fc3018 100644 --- a/routers/api/v1/repo/hook.go +++ b/routers/api/v1/repo/hook.go @@ -144,7 +144,7 @@ func TestHook(ctx *context.APIContext) { Before: ctx.Repo.Commit.ID.String(), After: ctx.Repo.Commit.ID.String(), Commits: []*api.PayloadCommit{ - convert.ToCommit(ctx.Repo.Repository, ctx.Repo.Commit), + convert.ToPayloadCommit(ctx.Repo.Repository, ctx.Repo.Commit), }, Repo: ctx.Repo.Repository.APIFormat(models.AccessModeNone), Pusher: convert.ToUser(ctx.User, ctx.IsSigned, false), diff --git a/routers/api/v1/repo/issue_stopwatch.go b/routers/api/v1/repo/issue_stopwatch.go index ca1593619e..a4a2261b9a 100644 --- a/routers/api/v1/repo/issue_stopwatch.go +++ b/routers/api/v1/repo/issue_stopwatch.go @@ -10,6 +10,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/context" + "code.gitea.io/gitea/modules/convert" "code.gitea.io/gitea/routers/api/v1/utils" ) @@ -224,7 +225,7 @@ func GetStopwatches(ctx *context.APIContext) { return } - apiSWs, err := sws.APIFormat() + apiSWs, err := convert.ToStopWatches(sws) if err != nil { ctx.Error(http.StatusInternalServerError, "APIFormat", err) return