Add missing X-Total-Count and fix some related bugs (#17968)

qwerty287 · KN4CK3R · 6543 · web-flow · commit 9d943bf374e5 · 2021-12-15T13:39:34.000+08:00
* Add missing `X-Total-Count` and fix some related bugs Adds `X-Total-Count` header to APIs that return a list but doesn't have it yet. Fixed bugs: * not returned after reporting error (https://github.com/qwerty287/gitea/blob/39eb82446c6fe5da3d79124e1f701f3795625b69/routers/api/v1/user/star.go#L70) * crash with index out of bounds, API issue/issueSubscriptions I also found various endpoints that return lists but do not apply/support pagination yet: ``` /repos/{owner}/{repo}/issues/{index}/labels /repos/{owner}/{repo}/issues/comments/{id}/reactions /repos/{owner}/{repo}/branch_protections /repos/{owner}/{repo}/contents /repos/{owner}/{repo}/hooks/git /repos/{owner}/{repo}/issue_templates /repos/{owner}/{repo}/releases/{id}/assets /repos/{owner}/{repo}/reviewers /repos/{owner}/{repo}/teams /user/emails /users/{username}/heatmap ``` If this is not expected, an new issue should be opened. Closes #13043 * fmt * Update routers/api/v1/repo/issue_subscription.go Co-authored-by: KN4CK3R <admin@oldschoolhack.me> * Use FindAndCount Co-authored-by: KN4CK3R <admin@oldschoolhack.me> Co-authored-by: 6543 <6543@obermui.de>
diff --git a/models/commit_status.go b/models/commit_status.go
@@ -231,11 +231,11 @@ type CommitStatusIndex struct {
 }
 
 // GetLatestCommitStatus returns all statuses with a unique context for a given commit.
-func GetLatestCommitStatus(repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, error) {
+func GetLatestCommitStatus(repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
 	return getLatestCommitStatus(db.GetEngine(db.DefaultContext), repoID, sha, listOptions)
 }
 
-func getLatestCommitStatus(e db.Engine, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, error) {
+func getLatestCommitStatus(e db.Engine, repoID int64, sha string, listOptions db.ListOptions) ([]*CommitStatus, int64, error) {
 	ids := make([]int64, 0, 10)
 	sess := e.Table(&CommitStatus{}).
 		Where("repo_id = ?", repoID).And("sha = ?", sha).
@@ -244,15 +244,15 @@ func getLatestCommitStatus(e db.Engine, repoID int64, sha string, listOptions db
 
 	sess = db.SetSessionPagination(sess, &listOptions)
 
-	err := sess.Find(&ids)
+	count, err := sess.FindAndCount(&ids)
 	if err != nil {
-		return nil, err
+		return nil, count, err
 	}
 	statuses := make([]*CommitStatus, 0, len(ids))
 	if len(ids) == 0 {
-		return statuses, nil
+		return statuses, count, nil
 	}
-	return statuses, e.In("id", ids).Find(&statuses)
+	return statuses, count, e.In("id", ids).Find(&statuses)
 }
 
 // FindRepoRecentCommitStatusContexts returns repository's recent commit status contexts
@@ -340,7 +340,7 @@ func ParseCommitsWithStatus(oldCommits []*asymkey_model.SignCommit, repo *repo_m
 		commit := &SignCommitWithStatuses{
 			SignCommit: c,
 		}
-		statuses, err := GetLatestCommitStatus(repo.ID, commit.ID.String(), db.ListOptions{})
+		statuses, _, err := GetLatestCommitStatus(repo.ID, commit.ID.String(), db.ListOptions{})
 		if err != nil {
 			log.Error("GetLatestCommitStatus: %v", err)
 		} else {
diff --git a/models/issue.go b/models/issue.go
@@ -238,7 +238,7 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) {
 		return nil
 	}
 	e := db.GetEngine(ctx)
-	reactions, err := findReactions(e, FindReactionsOptions{
+	reactions, _, err := findReactions(e, FindReactionsOptions{
 		IssueID: issue.ID,
 	})
 	if err != nil {
diff --git a/models/issue_comment.go b/models/issue_comment.go
@@ -593,7 +593,7 @@ func (c *Comment) loadReactions(e db.Engine, repo *repo_model.Repository) (err e
 	if c.Reactions != nil {
 		return nil
 	}
-	c.Reactions, err = findReactions(e, FindReactionsOptions{
+	c.Reactions, _, err = findReactions(e, FindReactionsOptions{
 		IssueID:   c.IssueID,
 		CommentID: c.ID,
 	})
diff --git a/models/issue_reaction.go b/models/issue_reaction.go
@@ -71,36 +71,38 @@ func (opts *FindReactionsOptions) toConds() builder.Cond {
 }
 
 // FindCommentReactions returns a ReactionList of all reactions from an comment
-func FindCommentReactions(comment *Comment) (ReactionList, error) {
+func FindCommentReactions(comment *Comment) (ReactionList, int64, error) {
 	return findReactions(db.GetEngine(db.DefaultContext), FindReactionsOptions{
 		IssueID:   comment.IssueID,
 		CommentID: comment.ID,
 	})
 }
 
 // FindIssueReactions returns a ReactionList of all reactions from an issue
-func FindIssueReactions(issue *Issue, listOptions db.ListOptions) (ReactionList, error) {
+func FindIssueReactions(issue *Issue, listOptions db.ListOptions) (ReactionList, int64, error) {
 	return findReactions(db.GetEngine(db.DefaultContext), FindReactionsOptions{
 		ListOptions: listOptions,
 		IssueID:     issue.ID,
 		CommentID:   -1,
 	})
 }
 
-func findReactions(e db.Engine, opts FindReactionsOptions) ([]*Reaction, error) {
-	e = e.
+func findReactions(e db.Engine, opts FindReactionsOptions) ([]*Reaction, int64, error) {
+	sess := e.
 		Where(opts.toConds()).
 		In("reaction.`type`", setting.UI.Reactions).
 		Asc("reaction.issue_id", "reaction.comment_id", "reaction.created_unix", "reaction.id")
 	if opts.Page != 0 {
-		e = db.SetEnginePagination(e, &opts)
+		sess = db.SetSessionPagination(sess, &opts)
 
 		reactions := make([]*Reaction, 0, opts.PageSize)
-		return reactions, e.Find(&reactions)
+		count, err := sess.FindAndCount(&reactions)
+		return reactions, count, err
 	}
 
 	reactions := make([]*Reaction, 0, 10)
-	return reactions, e.Find(&reactions)
+	count, err := sess.FindAndCount(&reactions)
+	return reactions, count, err
 }
 
 func createReaction(e db.Engine, opts *ReactionOptions) (*Reaction, error) {
@@ -120,7 +122,7 @@ func createReaction(e db.Engine, opts *ReactionOptions) (*Reaction, error) {
 		findOpts.CommentID = opts.Comment.ID
 	}
 
-	existingR, err := findReactions(e, findOpts)
+	existingR, _, err := findReactions(e, findOpts)
 	if err != nil {
 		return nil, err
 	}
diff --git a/models/issue_watch.go b/models/issue_watch.go
@@ -126,6 +126,20 @@ func getIssueWatchers(e db.Engine, issueID int64, listOptions db.ListOptions) (I
 	return watches, sess.Find(&watches)
 }
 
+// CountIssueWatchers count watchers/unwatchers of a given issue
+func CountIssueWatchers(issueID int64) (int64, error) {
+	return countIssueWatchers(db.GetEngine(db.DefaultContext), issueID)
+}
+
+func countIssueWatchers(e db.Engine, issueID int64) (int64, error) {
+	return e.
+		Where("`issue_watch`.issue_id = ?", issueID).
+		And("`issue_watch`.is_watching = ?", true).
+		And("`user`.is_active = ?", true).
+		And("`user`.prohibit_login = ?", false).
+		Join("INNER", "`user`", "`user`.id = `issue_watch`.user_id").Count(new(IssueWatch))
+}
+
 func removeIssueWatchersByRepoID(e db.Engine, userID, repoID int64) error {
 	_, err := e.
 		Join("INNER", "issue", "`issue`.id = `issue_watch`.issue_id AND `issue`.repo_id = ?", repoID).
diff --git a/modules/gitgraph/graph_models.go b/modules/gitgraph/graph_models.go
@@ -120,7 +120,7 @@ func (graph *Graph) LoadAndProcessCommits(repository *repo_model.Repository, git
 			return models.IsUserRepoAdmin(repository, user)
 		}, &keyMap)
 
-		statuses, err := models.GetLatestCommitStatus(repository.ID, c.Commit.ID.String(), db.ListOptions{})
+		statuses, _, err := models.GetLatestCommitStatus(repository.ID, c.Commit.ID.String(), db.ListOptions{})
 		if err != nil {
 			log.Error("GetLatestCommitStatus: %v", err)
 		} else {
diff --git a/modules/indexer/code/indexer_test.go b/modules/indexer/code/indexer_test.go
@@ -8,9 +8,10 @@ import (
 	"path/filepath"
 	"testing"
 
-	_ "code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/models/unittest"
 
+	_ "code.gitea.io/gitea/models"
+
 	"github.com/stretchr/testify/assert"
 )
 
diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go
@@ -11,11 +11,12 @@ import (
 	"testing"
 	"time"
 
-	_ "code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/setting"
 	"code.gitea.io/gitea/modules/util"
 
+	_ "code.gitea.io/gitea/models"
+
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/ini.v1"
 )
diff --git a/modules/indexer/stats/indexer_test.go b/modules/indexer/stats/indexer_test.go
@@ -9,11 +9,12 @@ import (
 	"testing"
 	"time"
 
-	_ "code.gitea.io/gitea/models"
 	repo_model "code.gitea.io/gitea/models/repo"
 	"code.gitea.io/gitea/models/unittest"
 	"code.gitea.io/gitea/modules/setting"
 
+	_ "code.gitea.io/gitea/models"
+
 	"github.com/stretchr/testify/assert"
 	"gopkg.in/ini.v1"
 )
diff --git a/routers/api/v1/org/team.go b/routers/api/v1/org/team.go
@@ -509,6 +509,7 @@ func GetTeamRepos(ctx *context.APIContext) {
 		}
 		repos[i] = convert.ToRepo(repo, access)
 	}
+	ctx.SetTotalCountHeader(int64(team.NumRepos))
 	ctx.JSON(http.StatusOK, repos)
 }
 
diff --git a/routers/api/v1/repo/issue_reaction.go b/routers/api/v1/repo/issue_reaction.go
@@ -67,9 +67,9 @@ func GetIssueCommentReactions(ctx *context.APIContext) {
 		return
 	}
 
-	reactions, err := models.FindCommentReactions(comment)
+	reactions, _, err := models.FindCommentReactions(comment)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "FindIssueReactions", err)
+		ctx.Error(http.StatusInternalServerError, "FindCommentReactions", err)
 		return
 	}
 	_, err = reactions.LoadUsers(ctx.Repo.Repository)
@@ -285,7 +285,7 @@ func GetIssueReactions(ctx *context.APIContext) {
 		return
 	}
 
-	reactions, err := models.FindIssueReactions(issue, utils.GetListOptions(ctx))
+	reactions, count, err := models.FindIssueReactions(issue, utils.GetListOptions(ctx))
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "FindIssueReactions", err)
 		return
@@ -305,6 +305,7 @@ func GetIssueReactions(ctx *context.APIContext) {
 		})
 	}
 
+	ctx.SetTotalCountHeader(count)
 	ctx.JSON(http.StatusOK, result)
 }
 
diff --git a/routers/api/v1/repo/issue_subscription.go b/routers/api/v1/repo/issue_subscription.go
@@ -279,9 +279,16 @@ func GetIssueSubscribers(ctx *context.APIContext) {
 		return
 	}
 	apiUsers := make([]*api.User, 0, len(users))
-	for i := range users {
-		apiUsers[i] = convert.ToUser(users[i], ctx.User)
+	for _, v := range users {
+		apiUsers = append(apiUsers, convert.ToUser(v, ctx.User))
 	}
 
+	count, err := models.CountIssueWatchers(issue.ID)
+	if err != nil {
+		ctx.Error(http.StatusInternalServerError, "CountIssueWatchers", err)
+		return
+	}
+
+	ctx.SetTotalCountHeader(count)
 	ctx.JSON(http.StatusOK, apiUsers)
 }
diff --git a/routers/api/v1/repo/status.go b/routers/api/v1/repo/status.go
@@ -253,7 +253,7 @@ func GetCombinedCommitStatusByRef(ctx *context.APIContext) {
 
 	repo := ctx.Repo.Repository
 
-	statuses, err := models.GetLatestCommitStatus(repo.ID, sha, utils.GetListOptions(ctx))
+	statuses, count, err := models.GetLatestCommitStatus(repo.ID, sha, utils.GetListOptions(ctx))
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetLatestCommitStatus", fmt.Errorf("GetLatestCommitStatus[%s, %s]: %v", repo.FullName(), sha, err))
 		return
@@ -266,6 +266,6 @@ func GetCombinedCommitStatusByRef(ctx *context.APIContext) {
 
 	combiStatus := convert.ToCombinedStatus(statuses, convert.ToRepo(repo, ctx.Repo.AccessMode))
 
-	// TODO: ctx.SetTotalCountHeader(count)
+	ctx.SetTotalCountHeader(count)
 	ctx.JSON(http.StatusOK, combiStatus)
 }
diff --git a/routers/api/v1/repo/tree.go b/routers/api/v1/repo/tree.go
@@ -63,6 +63,7 @@ func GetTree(ctx *context.APIContext) {
 	if tree, err := files_service.GetTreeBySHA(ctx.Repo.Repository, sha, ctx.FormInt("page"), ctx.FormInt("per_page"), ctx.FormBool("recursive")); err != nil {
 		ctx.Error(http.StatusBadRequest, "", err.Error())
 	} else {
+		ctx.SetTotalCountHeader(int64(tree.TotalCount))
 		ctx.JSON(http.StatusOK, tree)
 	}
 }
diff --git a/routers/api/v1/repo/wiki.go b/routers/api/v1/repo/wiki.go
@@ -323,6 +323,7 @@ func ListWikiPages(ctx *context.APIContext) {
 		pages = append(pages, convert.ToWikiPageMetaData(wikiName, c, ctx.Repo.Repository))
 	}
 
+	ctx.SetTotalCountHeader(int64(len(entries)))
 	ctx.JSON(http.StatusOK, pages)
 }
 
@@ -432,6 +433,7 @@ func ListPageRevisions(ctx *context.APIContext) {
 		return
 	}
 
+	ctx.SetTotalCountHeader(commitsCount)
 	ctx.JSON(http.StatusOK, convert.ToWikiCommitList(commitsHistory, commitsCount))
 }
 
diff --git a/routers/api/v1/user/star.go b/routers/api/v1/user/star.go
@@ -67,7 +67,10 @@ func GetStarredRepos(ctx *context.APIContext) {
 	repos, err := getStarredRepos(user, private, utils.GetListOptions(ctx))
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "getStarredRepos", err)
+		return
 	}
+
+	ctx.SetTotalCountHeader(int64(user.NumStars))
 	ctx.JSON(http.StatusOK, &repos)
 }
 
diff --git a/routers/web/repo/commit.go b/routers/web/repo/commit.go
@@ -337,7 +337,7 @@ func Diff(ctx *context.Context) {
 	ctx.Data["Commit"] = commit
 	ctx.Data["Diff"] = diff
 
-	statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, commitID, db.ListOptions{})
+	statuses, _, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, commitID, db.ListOptions{})
 	if err != nil {
 		log.Error("GetLatestCommitStatus: %v", err)
 	}
diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
@@ -339,7 +339,7 @@ func PrepareMergedViewPullInfo(ctx *context.Context, issue *models.Issue) *git.C
 
 	if len(compareInfo.Commits) != 0 {
 		sha := compareInfo.Commits[0].ID.String()
-		commitStatuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, sha, db.ListOptions{})
+		commitStatuses, _, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, sha, db.ListOptions{})
 		if err != nil {
 			ctx.ServerError("GetLatestCommitStatus", err)
 			return nil
@@ -393,7 +393,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 			ctx.ServerError(fmt.Sprintf("GetRefCommitID(%s)", pull.GetGitRefName()), err)
 			return nil
 		}
-		commitStatuses, err := models.GetLatestCommitStatus(repo.ID, sha, db.ListOptions{})
+		commitStatuses, _, err := models.GetLatestCommitStatus(repo.ID, sha, db.ListOptions{})
 		if err != nil {
 			ctx.ServerError("GetLatestCommitStatus", err)
 			return nil
@@ -482,7 +482,7 @@ func PrepareViewPullInfo(ctx *context.Context, issue *models.Issue) *git.Compare
 		return nil
 	}
 
-	commitStatuses, err := models.GetLatestCommitStatus(repo.ID, sha, db.ListOptions{})
+	commitStatuses, _, err := models.GetLatestCommitStatus(repo.ID, sha, db.ListOptions{})
 	if err != nil {
 		ctx.ServerError("GetLatestCommitStatus", err)
 		return nil
diff --git a/routers/web/repo/view.go b/routers/web/repo/view.go
@@ -790,7 +790,7 @@ func renderDirectoryFiles(ctx *context.Context, timeout time.Duration) git.Entri
 		ctx.Data["LatestCommitUser"] = user_model.ValidateCommitWithEmail(latestCommit)
 	}
 
-	statuses, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, ctx.Repo.Commit.ID.String(), db.ListOptions{})
+	statuses, _, err := models.GetLatestCommitStatus(ctx.Repo.Repository.ID, ctx.Repo.Commit.ID.String(), db.ListOptions{})
 	if err != nil {
 		log.Error("GetLatestCommitStatus: %v", err)
 	}
diff --git a/services/asymkey/ssh_key_test.go b/services/asymkey/ssh_key_test.go
@@ -11,6 +11,7 @@ import (
 	"code.gitea.io/gitea/models/login"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
+
 	"github.com/stretchr/testify/assert"
 )
 
diff --git a/services/mailer/mailer_test.go b/services/mailer/mailer_test.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"code.gitea.io/gitea/modules/setting"
+
 	"github.com/stretchr/testify/assert"
 )
 
diff --git a/services/pull/commit_status.go b/services/pull/commit_status.go
@@ -130,7 +130,7 @@ func GetPullRequestCommitStatusState(pr *models.PullRequest) (structs.CommitStat
 		return "", errors.Wrap(err, "LoadBaseRepo")
 	}
 
-	commitStatuses, err := models.GetLatestCommitStatus(pr.BaseRepo.ID, sha, db.ListOptions{})
+	commitStatuses, _, err := models.GetLatestCommitStatus(pr.BaseRepo.ID, sha, db.ListOptions{})
 	if err != nil {
 		return "", errors.Wrap(err, "GetLatestCommitStatus")
 	}
diff --git a/services/pull/pull.go b/services/pull/pull.go
@@ -782,7 +782,7 @@ func getLastCommitStatus(gitRepo *git.Repository, pr *models.PullRequest) (statu
 		return nil, err
 	}
 
-	statusList, err := models.GetLatestCommitStatus(pr.BaseRepo.ID, sha, db.ListOptions{})
+	statusList, _, err := models.GetLatestCommitStatus(pr.BaseRepo.ID, sha, db.ListOptions{})
 	if err != nil {
 		return nil, err
 	}
diff --git a/services/webhook/main_test.go b/services/webhook/main_test.go
@@ -8,8 +8,9 @@ import (
 	"path/filepath"
 	"testing"
 
-	_ "code.gitea.io/gitea/models"
 	"code.gitea.io/gitea/models/unittest"
+
+	_ "code.gitea.io/gitea/models"
 )
 
 func TestMain(m *testing.M) {

Original file line number	Diff line number	Diff line change
`@@ -238,7 +238,7 @@ func (issue *Issue) loadReactions(ctx context.Context) (err error) {`
`238`	`238`	`return nil`
`239`	`239`	`}`
`240`	`240`	`e := db.GetEngine(ctx)`
`241`		`- reactions, err := findReactions(e, FindReactionsOptions{`
	`241`	`+ reactions, _, err := findReactions(e, FindReactionsOptions{`
`242`	`242`	`IssueID: issue.ID,`
`243`	`243`	`})`
`244`	`244`	`if err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -593,7 +593,7 @@ func (c Comment) loadReactions(e db.Engine, repo repo_model.Repository) (err e`
`593`	`593`	`if c.Reactions != nil {`
`594`	`594`	`return nil`
`595`	`595`	`}`
`596`		`- c.Reactions, err = findReactions(e, FindReactionsOptions{`
	`596`	`+ c.Reactions, _, err = findReactions(e, FindReactionsOptions{`
`597`	`597`	`IssueID: c.IssueID,`
`598`	`598`	`CommentID: c.ID,`
`599`	`599`	`})`
Original file line number	Diff line number	Diff line change
`@@ -8,9 +8,10 @@ import (`
`8`	`8`	`"path/filepath"`
`9`	`9`	`"testing"`
`10`	`10`
`11`		`- _ "code.gitea.io/gitea/models"`
`12`	`11`	`"code.gitea.io/gitea/models/unittest"`
`13`	`12`
	`13`	`+ _ "code.gitea.io/gitea/models"`
	`14`	`+`
`14`	`15`	`"github.com/stretchr/testify/assert"`
`15`	`16`	`)`
`16`	`17`
Original file line number	Diff line number	Diff line change
`@@ -509,6 +509,7 @@ func GetTeamRepos(ctx *context.APIContext) {`
`509`	`509`	`}`
`510`	`510`	`repos[i] = convert.ToRepo(repo, access)`
`511`	`511`	`}`
	`512`	`+ ctx.SetTotalCountHeader(int64(team.NumRepos))`
`512`	`513`	`ctx.JSON(http.StatusOK, repos)`
`513`	`514`	`}`
`514`	`515`