Skip to content

Commit 1d191f9

Browse files
lunnyzeripath
andauthored
some refactor about code comments(#20821) (#22707)
fix #22691 backport #20821 Co-authored-by: zeripath <[email protected]>
1 parent 2e1afd5 commit 1d191f9

File tree

7 files changed

+260
-188
lines changed

7 files changed

+260
-188
lines changed

models/db/list_options.go

+49-1
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
package db
66

77
import (
8+
"context"
9+
810
"code.gitea.io/gitea/modules/setting"
911

12+
"xorm.io/builder"
1013
"xorm.io/xorm"
1114
)
1215

@@ -19,6 +22,7 @@ const (
1922
type Paginator interface {
2023
GetSkipTake() (skip, take int)
2124
GetStartEnd() (start, end int)
25+
IsListAll() bool
2226
}
2327

2428
// GetPaginatedSession creates a paginated database session
@@ -45,9 +49,12 @@ func SetEnginePagination(e Engine, p Paginator) Engine {
4549
// ListOptions options to paginate results
4650
type ListOptions struct {
4751
PageSize int
48-
Page int // start from 1
52+
Page int // start from 1
53+
ListAll bool // if true, then PageSize and Page will not be taken
4954
}
5055

56+
var _ Paginator = &ListOptions{}
57+
5158
// GetSkipTake returns the skip and take values
5259
func (opts *ListOptions) GetSkipTake() (skip, take int) {
5360
opts.SetDefaultValues()
@@ -61,6 +68,11 @@ func (opts *ListOptions) GetStartEnd() (start, end int) {
6168
return start, end
6269
}
6370

71+
// IsListAll indicates PageSize and Page will be ignored
72+
func (opts *ListOptions) IsListAll() bool {
73+
return opts.ListAll
74+
}
75+
6476
// SetDefaultValues sets default values
6577
func (opts *ListOptions) SetDefaultValues() {
6678
if opts.PageSize <= 0 {
@@ -80,6 +92,8 @@ type AbsoluteListOptions struct {
8092
take int
8193
}
8294

95+
var _ Paginator = &AbsoluteListOptions{}
96+
8397
// NewAbsoluteListOptions creates a list option with applied limits
8498
func NewAbsoluteListOptions(skip, take int) *AbsoluteListOptions {
8599
if skip < 0 {
@@ -94,6 +108,11 @@ func NewAbsoluteListOptions(skip, take int) *AbsoluteListOptions {
94108
return &AbsoluteListOptions{skip, take}
95109
}
96110

111+
// IsListAll will always return false
112+
func (opts *AbsoluteListOptions) IsListAll() bool {
113+
return false
114+
}
115+
97116
// GetSkipTake returns the skip and take values
98117
func (opts *AbsoluteListOptions) GetSkipTake() (skip, take int) {
99118
return opts.skip, opts.take
@@ -103,3 +122,32 @@ func (opts *AbsoluteListOptions) GetSkipTake() (skip, take int) {
103122
func (opts *AbsoluteListOptions) GetStartEnd() (start, end int) {
104123
return opts.skip, opts.skip + opts.take
105124
}
125+
126+
// FindOptions represents a find options
127+
type FindOptions interface {
128+
Paginator
129+
ToConds() builder.Cond
130+
}
131+
132+
// Find represents a common find function which accept an options interface
133+
func Find[T any](ctx context.Context, opts FindOptions, objects *[]T) error {
134+
sess := GetEngine(ctx).Where(opts.ToConds())
135+
if !opts.IsListAll() {
136+
sess.Limit(opts.GetSkipTake())
137+
}
138+
return sess.Find(&objects)
139+
}
140+
141+
// Count represents a common count function which accept an options interface
142+
func Count[T any](ctx context.Context, opts FindOptions, object T) (int64, error) {
143+
return GetEngine(ctx).Where(opts.ToConds()).Count(object)
144+
}
145+
146+
// FindAndCount represents a common findandcount function which accept an options interface
147+
func FindAndCount[T any](ctx context.Context, opts FindOptions, objects *[]T) (int64, error) {
148+
sess := GetEngine(ctx).Where(opts.ToConds())
149+
if !opts.IsListAll() {
150+
sess.Limit(opts.GetSkipTake())
151+
}
152+
return sess.FindAndCount(&objects)
153+
}

models/issues/comment.go

+27-156
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,7 @@ package issues
99
import (
1010
"context"
1111
"fmt"
12-
"regexp"
1312
"strconv"
14-
"strings"
1513
"unicode/utf8"
1614

1715
"code.gitea.io/gitea/models/db"
@@ -23,8 +21,6 @@ import (
2321
"code.gitea.io/gitea/modules/git"
2422
"code.gitea.io/gitea/modules/json"
2523
"code.gitea.io/gitea/modules/log"
26-
"code.gitea.io/gitea/modules/markup"
27-
"code.gitea.io/gitea/modules/markup/markdown"
2824
"code.gitea.io/gitea/modules/references"
2925
"code.gitea.io/gitea/modules/structs"
3026
"code.gitea.io/gitea/modules/timeutil"
@@ -697,31 +693,6 @@ func (c *Comment) LoadReview() error {
697693
return c.loadReview(db.DefaultContext)
698694
}
699695

700-
var notEnoughLines = regexp.MustCompile(`fatal: file .* has only \d+ lines?`)
701-
702-
func (c *Comment) checkInvalidation(doer *user_model.User, repo *git.Repository, branch string) error {
703-
// FIXME differentiate between previous and proposed line
704-
commit, err := repo.LineBlame(branch, repo.Path, c.TreePath, uint(c.UnsignedLine()))
705-
if err != nil && (strings.Contains(err.Error(), "fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
706-
c.Invalidated = true
707-
return UpdateComment(c, doer)
708-
}
709-
if err != nil {
710-
return err
711-
}
712-
if c.CommitSHA != "" && c.CommitSHA != commit.ID.String() {
713-
c.Invalidated = true
714-
return UpdateComment(c, doer)
715-
}
716-
return nil
717-
}
718-
719-
// CheckInvalidation checks if the line of code comment got changed by another commit.
720-
// If the line got changed the comment is going to be invalidated.
721-
func (c *Comment) CheckInvalidation(repo *git.Repository, doer *user_model.User, branch string) error {
722-
return c.checkInvalidation(doer, repo, branch)
723-
}
724-
725696
// DiffSide returns "previous" if Comment.Line is a LOC of the previous changes and "proposed" if it is a LOC of the proposed changes.
726697
func (c *Comment) DiffSide() string {
727698
if c.Line < 0 {
@@ -1065,23 +1036,28 @@ func GetCommentByID(ctx context.Context, id int64) (*Comment, error) {
10651036
// FindCommentsOptions describes the conditions to Find comments
10661037
type FindCommentsOptions struct {
10671038
db.ListOptions
1068-
RepoID int64
1069-
IssueID int64
1070-
ReviewID int64
1071-
Since int64
1072-
Before int64
1073-
Line int64
1074-
TreePath string
1075-
Type CommentType
1076-
}
1077-
1078-
func (opts *FindCommentsOptions) toConds() builder.Cond {
1039+
RepoID int64
1040+
IssueID int64
1041+
ReviewID int64
1042+
Since int64
1043+
Before int64
1044+
Line int64
1045+
TreePath string
1046+
Type CommentType
1047+
IssueIDs []int64
1048+
Invalidated util.OptionalBool
1049+
}
1050+
1051+
// ToConds implements FindOptions interface
1052+
func (opts *FindCommentsOptions) ToConds() builder.Cond {
10791053
cond := builder.NewCond()
10801054
if opts.RepoID > 0 {
10811055
cond = cond.And(builder.Eq{"issue.repo_id": opts.RepoID})
10821056
}
10831057
if opts.IssueID > 0 {
10841058
cond = cond.And(builder.Eq{"comment.issue_id": opts.IssueID})
1059+
} else if len(opts.IssueIDs) > 0 {
1060+
cond = cond.And(builder.In("comment.issue_id", opts.IssueIDs))
10851061
}
10861062
if opts.ReviewID > 0 {
10871063
cond = cond.And(builder.Eq{"comment.review_id": opts.ReviewID})
@@ -1101,13 +1077,16 @@ func (opts *FindCommentsOptions) toConds() builder.Cond {
11011077
if len(opts.TreePath) > 0 {
11021078
cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath})
11031079
}
1080+
if !opts.Invalidated.IsNone() {
1081+
cond = cond.And(builder.Eq{"comment.invalidated": opts.Invalidated.IsTrue()})
1082+
}
11041083
return cond
11051084
}
11061085

11071086
// FindComments returns all comments according options
11081087
func FindComments(ctx context.Context, opts *FindCommentsOptions) ([]*Comment, error) {
11091088
comments := make([]*Comment, 0, 10)
1110-
sess := db.GetEngine(ctx).Where(opts.toConds())
1089+
sess := db.GetEngine(ctx).Where(opts.ToConds())
11111090
if opts.RepoID > 0 {
11121091
sess.Join("INNER", "issue", "issue.id = comment.issue_id")
11131092
}
@@ -1126,13 +1105,19 @@ func FindComments(ctx context.Context, opts *FindCommentsOptions) ([]*Comment, e
11261105

11271106
// CountComments count all comments according options by ignoring pagination
11281107
func CountComments(opts *FindCommentsOptions) (int64, error) {
1129-
sess := db.GetEngine(db.DefaultContext).Where(opts.toConds())
1108+
sess := db.GetEngine(db.DefaultContext).Where(opts.ToConds())
11301109
if opts.RepoID > 0 {
11311110
sess.Join("INNER", "issue", "issue.id = comment.issue_id")
11321111
}
11331112
return sess.Count(&Comment{})
11341113
}
11351114

1115+
// UpdateCommentInvalidate updates comment invalidated column
1116+
func UpdateCommentInvalidate(ctx context.Context, c *Comment) error {
1117+
_, err := db.GetEngine(ctx).ID(c.ID).Cols("invalidated").Update(c)
1118+
return err
1119+
}
1120+
11361121
// UpdateComment updates information of comment.
11371122
func UpdateComment(c *Comment, doer *user_model.User) error {
11381123
ctx, committer, err := db.TxContext()
@@ -1191,120 +1176,6 @@ func DeleteComment(ctx context.Context, comment *Comment) error {
11911176
return DeleteReaction(ctx, &ReactionOptions{CommentID: comment.ID})
11921177
}
11931178

1194-
// CodeComments represents comments on code by using this structure: FILENAME -> LINE (+ == proposed; - == previous) -> COMMENTS
1195-
type CodeComments map[string]map[int64][]*Comment
1196-
1197-
// FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line
1198-
func FetchCodeComments(ctx context.Context, issue *Issue, currentUser *user_model.User) (CodeComments, error) {
1199-
return fetchCodeCommentsByReview(ctx, issue, currentUser, nil)
1200-
}
1201-
1202-
func fetchCodeCommentsByReview(ctx context.Context, issue *Issue, currentUser *user_model.User, review *Review) (CodeComments, error) {
1203-
pathToLineToComment := make(CodeComments)
1204-
if review == nil {
1205-
review = &Review{ID: 0}
1206-
}
1207-
opts := FindCommentsOptions{
1208-
Type: CommentTypeCode,
1209-
IssueID: issue.ID,
1210-
ReviewID: review.ID,
1211-
}
1212-
1213-
comments, err := findCodeComments(ctx, opts, issue, currentUser, review)
1214-
if err != nil {
1215-
return nil, err
1216-
}
1217-
1218-
for _, comment := range comments {
1219-
if pathToLineToComment[comment.TreePath] == nil {
1220-
pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment)
1221-
}
1222-
pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment)
1223-
}
1224-
return pathToLineToComment, nil
1225-
}
1226-
1227-
func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issue, currentUser *user_model.User, review *Review) ([]*Comment, error) {
1228-
var comments []*Comment
1229-
if review == nil {
1230-
review = &Review{ID: 0}
1231-
}
1232-
conds := opts.toConds()
1233-
if review.ID == 0 {
1234-
conds = conds.And(builder.Eq{"invalidated": false})
1235-
}
1236-
e := db.GetEngine(ctx)
1237-
if err := e.Where(conds).
1238-
Asc("comment.created_unix").
1239-
Asc("comment.id").
1240-
Find(&comments); err != nil {
1241-
return nil, err
1242-
}
1243-
1244-
if err := issue.LoadRepo(ctx); err != nil {
1245-
return nil, err
1246-
}
1247-
1248-
if err := CommentList(comments).loadPosters(ctx); err != nil {
1249-
return nil, err
1250-
}
1251-
1252-
// Find all reviews by ReviewID
1253-
reviews := make(map[int64]*Review)
1254-
ids := make([]int64, 0, len(comments))
1255-
for _, comment := range comments {
1256-
if comment.ReviewID != 0 {
1257-
ids = append(ids, comment.ReviewID)
1258-
}
1259-
}
1260-
if err := e.In("id", ids).Find(&reviews); err != nil {
1261-
return nil, err
1262-
}
1263-
1264-
n := 0
1265-
for _, comment := range comments {
1266-
if re, ok := reviews[comment.ReviewID]; ok && re != nil {
1267-
// If the review is pending only the author can see the comments (except if the review is set)
1268-
if review.ID == 0 && re.Type == ReviewTypePending &&
1269-
(currentUser == nil || currentUser.ID != re.ReviewerID) {
1270-
continue
1271-
}
1272-
comment.Review = re
1273-
}
1274-
comments[n] = comment
1275-
n++
1276-
1277-
if err := comment.LoadResolveDoer(); err != nil {
1278-
return nil, err
1279-
}
1280-
1281-
if err := comment.LoadReactions(issue.Repo); err != nil {
1282-
return nil, err
1283-
}
1284-
1285-
var err error
1286-
if comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{
1287-
Ctx: ctx,
1288-
URLPrefix: issue.Repo.Link(),
1289-
Metas: issue.Repo.ComposeMetas(),
1290-
}, comment.Content); err != nil {
1291-
return nil, err
1292-
}
1293-
}
1294-
return comments[:n], nil
1295-
}
1296-
1297-
// FetchCodeCommentsByLine fetches the code comments for a given treePath and line number
1298-
func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64) ([]*Comment, error) {
1299-
opts := FindCommentsOptions{
1300-
Type: CommentTypeCode,
1301-
IssueID: issue.ID,
1302-
TreePath: treePath,
1303-
Line: line,
1304-
}
1305-
return findCodeComments(ctx, opts, issue, currentUser, nil)
1306-
}
1307-
13081179
// UpdateCommentsMigrationsByType updates comments' migrations information via given git service type and original id and poster id
13091180
func UpdateCommentsMigrationsByType(tp structs.GitServiceType, originalAuthorID string, posterID int64) error {
13101181
_, err := db.GetEngine(db.DefaultContext).Table("comment").

0 commit comments

Comments
 (0)