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

@@ -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)
+	stopwatch := models.AssertExistsAndLoadBean(t, &models.Stopwatch{UserID: owner.ID}).(*models.Stopwatch)
-	expectAPI, _ := expect.APIFormat()
+	issue := models.AssertExistsAndLoadBean(t, &models.Issue{ID: stopwatch.IssueID}).(*models.Issue)
-	assert.Len(t, apiWatches, 1)
+	if assert.Len(t, apiWatches, 1) {
-
+		assert.EqualValues(t, stopwatch.CreatedUnix.AsTime().Unix(), apiWatches[0].Created.Unix())
-	assert.EqualValues(t, expectAPI.IssueIndex, apiWatches[0].IssueIndex)
+		apiWatches[0].Created = time.Time{}
-	assert.EqualValues(t, expectAPI.Created.Unix(), apiWatches[0].Created.Unix())
+		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) {

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) {
+func GetUserStopwatches(userID int64, listOptions ListOptions) ([]*Stopwatch, error) {
-	sws := new(Stopwatches)
+	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
-}

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{

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
+}

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))

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
-}

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),

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