Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

some refactor about code comments #20821

Merged
merged 9 commits into from
Jan 17, 2023
50 changes: 49 additions & 1 deletion models/db/list_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
package db

import (
"context"

"code.gitea.io/gitea/modules/setting"

"xorm.io/builder"
"xorm.io/xorm"
)

Expand All @@ -18,6 +21,7 @@ const (
type Paginator interface {
GetSkipTake() (skip, take int)
GetStartEnd() (start, end int)
IsListAll() bool
}

// GetPaginatedSession creates a paginated database session
Expand All @@ -44,9 +48,12 @@ func SetEnginePagination(e Engine, p Paginator) Engine {
// ListOptions options to paginate results
type ListOptions struct {
PageSize int
Page int // start from 1
Page int // start from 1
ListAll bool // if true, then PageSize and Page will not be taken
}

var _ Paginator = &ListOptions{}

// GetSkipTake returns the skip and take values
func (opts *ListOptions) GetSkipTake() (skip, take int) {
opts.SetDefaultValues()
Expand All @@ -60,6 +67,11 @@ func (opts *ListOptions) GetStartEnd() (start, end int) {
return start, end
}

// IsListAll indicates PageSize and Page will be ignored
func (opts *ListOptions) IsListAll() bool {
return opts.ListAll
}

// SetDefaultValues sets default values
func (opts *ListOptions) SetDefaultValues() {
if opts.PageSize <= 0 {
Expand All @@ -79,6 +91,8 @@ type AbsoluteListOptions struct {
take int
}

var _ Paginator = &AbsoluteListOptions{}

// NewAbsoluteListOptions creates a list option with applied limits
func NewAbsoluteListOptions(skip, take int) *AbsoluteListOptions {
if skip < 0 {
Expand All @@ -93,6 +107,11 @@ func NewAbsoluteListOptions(skip, take int) *AbsoluteListOptions {
return &AbsoluteListOptions{skip, take}
}

// IsListAll will always return false
func (opts *AbsoluteListOptions) IsListAll() bool {
return false
}

// GetSkipTake returns the skip and take values
func (opts *AbsoluteListOptions) GetSkipTake() (skip, take int) {
return opts.skip, opts.take
Expand All @@ -102,3 +121,32 @@ func (opts *AbsoluteListOptions) GetSkipTake() (skip, take int) {
func (opts *AbsoluteListOptions) GetStartEnd() (start, end int) {
return opts.skip, opts.skip + opts.take
}

// FindOptions represents a find options
type FindOptions interface {
Paginator
ToConds() builder.Cond
}

// Find represents a common find function which accept an options interface
func Find[T any](ctx context.Context, opts FindOptions, objects *[]T) error {
sess := GetEngine(ctx).Where(opts.ToConds())
if !opts.IsListAll() {
sess.Limit(opts.GetSkipTake())
}
return sess.Find(&objects)
}

// Count represents a common count function which accept an options interface
func Count[T any](ctx context.Context, opts FindOptions, object T) (int64, error) {
return GetEngine(ctx).Where(opts.ToConds()).Count(object)
}

// FindAndCount represents a common findandcount function which accept an options interface
func FindAndCount[T any](ctx context.Context, opts FindOptions, objects *[]T) (int64, error) {
sess := GetEngine(ctx).Where(opts.ToConds())
if !opts.IsListAll() {
sess.Limit(opts.GetSkipTake())
}
return sess.FindAndCount(&objects)
}
183 changes: 27 additions & 156 deletions models/issues/comment.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ package issues
import (
"context"
"fmt"
"regexp"
"strconv"
"strings"
"unicode/utf8"

"code.gitea.io/gitea/models/db"
Expand All @@ -22,8 +20,6 @@ import (
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/json"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/markup/markdown"
"code.gitea.io/gitea/modules/references"
"code.gitea.io/gitea/modules/structs"
"code.gitea.io/gitea/modules/timeutil"
Expand Down Expand Up @@ -687,31 +683,6 @@ func (c *Comment) LoadReview() error {
return c.loadReview(db.DefaultContext)
}

var notEnoughLines = regexp.MustCompile(`fatal: file .* has only \d+ lines?`)

func (c *Comment) checkInvalidation(doer *user_model.User, repo *git.Repository, branch string) error {
// FIXME differentiate between previous and proposed line
commit, err := repo.LineBlame(branch, repo.Path, c.TreePath, uint(c.UnsignedLine()))
if err != nil && (strings.Contains(err.Error(), "fatal: no such path") || notEnoughLines.MatchString(err.Error())) {
c.Invalidated = true
return UpdateComment(c, doer)
}
if err != nil {
return err
}
if c.CommitSHA != "" && c.CommitSHA != commit.ID.String() {
c.Invalidated = true
return UpdateComment(c, doer)
}
return nil
}

// CheckInvalidation checks if the line of code comment got changed by another commit.
// If the line got changed the comment is going to be invalidated.
func (c *Comment) CheckInvalidation(repo *git.Repository, doer *user_model.User, branch string) error {
return c.checkInvalidation(doer, repo, branch)
}

// DiffSide returns "previous" if Comment.Line is a LOC of the previous changes and "proposed" if it is a LOC of the proposed changes.
func (c *Comment) DiffSide() string {
if c.Line < 0 {
Expand Down Expand Up @@ -1008,23 +979,28 @@ func GetCommentByID(ctx context.Context, id int64) (*Comment, error) {
// FindCommentsOptions describes the conditions to Find comments
type FindCommentsOptions struct {
db.ListOptions
RepoID int64
IssueID int64
ReviewID int64
Since int64
Before int64
Line int64
TreePath string
Type CommentType
}

func (opts *FindCommentsOptions) toConds() builder.Cond {
RepoID int64
IssueID int64
ReviewID int64
Since int64
Before int64
Line int64
TreePath string
Type CommentType
IssueIDs []int64
Invalidated util.OptionalBool
}

// ToConds implements FindOptions interface
func (opts *FindCommentsOptions) ToConds() builder.Cond {
cond := builder.NewCond()
if opts.RepoID > 0 {
cond = cond.And(builder.Eq{"issue.repo_id": opts.RepoID})
}
if opts.IssueID > 0 {
cond = cond.And(builder.Eq{"comment.issue_id": opts.IssueID})
} else if len(opts.IssueIDs) > 0 {
cond = cond.And(builder.In("comment.issue_id", opts.IssueIDs))
}
if opts.ReviewID > 0 {
cond = cond.And(builder.Eq{"comment.review_id": opts.ReviewID})
Expand All @@ -1044,13 +1020,16 @@ func (opts *FindCommentsOptions) toConds() builder.Cond {
if len(opts.TreePath) > 0 {
cond = cond.And(builder.Eq{"comment.tree_path": opts.TreePath})
}
if !opts.Invalidated.IsNone() {
cond = cond.And(builder.Eq{"comment.invalidated": opts.Invalidated.IsTrue()})
}
return cond
}

// FindComments returns all comments according options
func FindComments(ctx context.Context, opts *FindCommentsOptions) ([]*Comment, error) {
comments := make([]*Comment, 0, 10)
sess := db.GetEngine(ctx).Where(opts.toConds())
sess := db.GetEngine(ctx).Where(opts.ToConds())
if opts.RepoID > 0 {
sess.Join("INNER", "issue", "issue.id = comment.issue_id")
}
Expand All @@ -1069,13 +1048,19 @@ func FindComments(ctx context.Context, opts *FindCommentsOptions) ([]*Comment, e

// CountComments count all comments according options by ignoring pagination
func CountComments(opts *FindCommentsOptions) (int64, error) {
sess := db.GetEngine(db.DefaultContext).Where(opts.toConds())
sess := db.GetEngine(db.DefaultContext).Where(opts.ToConds())
if opts.RepoID > 0 {
sess.Join("INNER", "issue", "issue.id = comment.issue_id")
}
return sess.Count(&Comment{})
}

// UpdateCommentInvalidate updates comment invalidated column
func UpdateCommentInvalidate(ctx context.Context, c *Comment) error {
_, err := db.GetEngine(ctx).ID(c.ID).Cols("invalidated").Update(c)
return err
}

// UpdateComment updates information of comment.
func UpdateComment(c *Comment, doer *user_model.User) error {
ctx, committer, err := db.TxContext(db.DefaultContext)
Expand Down Expand Up @@ -1134,120 +1119,6 @@ func DeleteComment(ctx context.Context, comment *Comment) error {
return DeleteReaction(ctx, &ReactionOptions{CommentID: comment.ID})
}

// CodeComments represents comments on code by using this structure: FILENAME -> LINE (+ == proposed; - == previous) -> COMMENTS
type CodeComments map[string]map[int64][]*Comment

// FetchCodeComments will return a 2d-map: ["Path"]["Line"] = Comments at line
func FetchCodeComments(ctx context.Context, issue *Issue, currentUser *user_model.User) (CodeComments, error) {
return fetchCodeCommentsByReview(ctx, issue, currentUser, nil)
}

func fetchCodeCommentsByReview(ctx context.Context, issue *Issue, currentUser *user_model.User, review *Review) (CodeComments, error) {
pathToLineToComment := make(CodeComments)
if review == nil {
review = &Review{ID: 0}
}
opts := FindCommentsOptions{
Type: CommentTypeCode,
IssueID: issue.ID,
ReviewID: review.ID,
}

comments, err := findCodeComments(ctx, opts, issue, currentUser, review)
if err != nil {
return nil, err
}

for _, comment := range comments {
if pathToLineToComment[comment.TreePath] == nil {
pathToLineToComment[comment.TreePath] = make(map[int64][]*Comment)
}
pathToLineToComment[comment.TreePath][comment.Line] = append(pathToLineToComment[comment.TreePath][comment.Line], comment)
}
return pathToLineToComment, nil
}

func findCodeComments(ctx context.Context, opts FindCommentsOptions, issue *Issue, currentUser *user_model.User, review *Review) ([]*Comment, error) {
var comments []*Comment
if review == nil {
review = &Review{ID: 0}
}
conds := opts.toConds()
if review.ID == 0 {
conds = conds.And(builder.Eq{"invalidated": false})
}
e := db.GetEngine(ctx)
if err := e.Where(conds).
Asc("comment.created_unix").
Asc("comment.id").
Find(&comments); err != nil {
return nil, err
}

if err := issue.LoadRepo(ctx); err != nil {
return nil, err
}

if err := CommentList(comments).LoadPosters(ctx); err != nil {
return nil, err
}

// Find all reviews by ReviewID
reviews := make(map[int64]*Review)
ids := make([]int64, 0, len(comments))
for _, comment := range comments {
if comment.ReviewID != 0 {
ids = append(ids, comment.ReviewID)
}
}
if err := e.In("id", ids).Find(&reviews); err != nil {
return nil, err
}

n := 0
for _, comment := range comments {
if re, ok := reviews[comment.ReviewID]; ok && re != nil {
// If the review is pending only the author can see the comments (except if the review is set)
if review.ID == 0 && re.Type == ReviewTypePending &&
(currentUser == nil || currentUser.ID != re.ReviewerID) {
continue
}
comment.Review = re
}
comments[n] = comment
n++

if err := comment.LoadResolveDoer(); err != nil {
return nil, err
}

if err := comment.LoadReactions(issue.Repo); err != nil {
return nil, err
}

var err error
if comment.RenderedContent, err = markdown.RenderString(&markup.RenderContext{
Ctx: ctx,
URLPrefix: issue.Repo.Link(),
Metas: issue.Repo.ComposeMetas(),
}, comment.Content); err != nil {
return nil, err
}
}
return comments[:n], nil
}

// FetchCodeCommentsByLine fetches the code comments for a given treePath and line number
func FetchCodeCommentsByLine(ctx context.Context, issue *Issue, currentUser *user_model.User, treePath string, line int64) ([]*Comment, error) {
opts := FindCommentsOptions{
Type: CommentTypeCode,
IssueID: issue.ID,
TreePath: treePath,
Line: line,
}
return findCodeComments(ctx, opts, issue, currentUser, nil)
}

// UpdateCommentsMigrationsByType updates comments' migrations information via given git service type and original id and poster id
func UpdateCommentsMigrationsByType(tp structs.GitServiceType, originalAuthorID string, posterID int64) error {
_, err := db.GetEngine(db.DefaultContext).Table("comment").
Expand Down
Loading