Skip to content

Commit

Permalink
some refactor about code comments(#20821) (#22707)
Browse files Browse the repository at this point in the history 
fix #22691
backport #20821

Co-authored-by: zeripath <art27@cantab.net>
  • Loading branch information
@lunny @zeripath
lunny and zeripath authored Feb 16, 2023
1 parent 2e1afd5 commit 1d191f9
Show file tree
Hide file tree
Showing 7 changed files with 260 additions and 188 deletions.
50 changes: 49 additions & 1 deletion models/db/list_options.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
package db

import (
"context"

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

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

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

// GetPaginatedSession creates a paginated database session
Expand All @@ -45,9 +49,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 @@ -61,6 +68,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 @@ -80,6 +92,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 @@ -94,6 +108,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 @@ -103,3 +122,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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,7 @@ package issues
import (
"context"
"fmt"
"regexp"
"strconv"
"strings"
"unicode/utf8"

"code.gitea.io/gitea/models/db"
Expand All @@ -23,8 +21,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 @@ -697,31 +693,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 @@ -1065,23 +1036,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 @@ -1101,13 +1077,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 @@ -1126,13 +1105,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()
Expand Down Expand Up @@ -1191,120 +1176,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

0 comments on commit 1d191f9

Please sign in to comment.