wanderer/gitea
1
0
Fork 0
forked from mirror/gitea
Code Issues Pull Requests Releases Activity

some refactor about code comments (#20821)

Browse Source
This commit is contained in:
Lunny Xiao 2023-01-18 05:03:44 +08:00 committed by GitHub
parent 60c4725cc2
commit db2286bbb6
Signed by: GitHub
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 258 additions and 186 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

50
models/db/list_options.go
View File

@ -4,8 +4,11 @@
package db
import (
"context"
"code.gitea.io/gitea/modules/setting"
"xorm.io/builder"
"xorm.io/xorm"
)
@ -18,6 +21,7 @@ const (
type Paginator interface {
GetSkipTake() (skip, take int)
GetStartEnd() (start, end int)
IsListAll() bool
}
// GetPaginatedSession creates a paginated database session
@ -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()
@ -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 {
@ -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 {
@ -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
@ -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)
}

179
models/issues/comment.go
View File

@ -8,9 +8,7 @@ package issues
import (
"context"
"fmt"
"regexp"
"strconv"
"strings"
"unicode/utf8"
"code.gitea.io/gitea/models/db"
@ -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"
@ -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 {
@ -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
RepoID int64
IssueID int64
ReviewID int64
Since int64
Before int64
Line int64
TreePath string
Type CommentType
IssueIDs []int64
Invalidated util.OptionalBool
}
func (opts *FindCommentsOptions) toConds() builder.Cond {
// 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})
@ -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")
}
@ -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)
@ -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").

129
models/issues/comment_code.go Normal file
View File

@ -0,0 +1,129 @@
// Copyright 2022 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package issues
import (
"context"
"code.gitea.io/gitea/models/db"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/markup/markdown"
"xorm.io/builder"
)
// 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)
}

27
models/issues/pull_list.go
View File

@ -12,7 +12,6 @@ import (
"code.gitea.io/gitea/models/unit"
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"xorm.io/xorm"
@ -161,7 +160,7 @@ func (prs PullRequestList) loadAttributes(ctx context.Context) error {
}
// Load issues.
issueIDs := prs.getIssueIDs()
issueIDs := prs.GetIssueIDs()
issues := make([]*Issue, 0, len(issueIDs))
if err := db.GetEngine(ctx).
Where("id > 0").
@ -180,7 +179,8 @@ func (prs PullRequestList) loadAttributes(ctx context.Context) error {
return nil
}
func (prs PullRequestList) getIssueIDs() []int64 {
// GetIssueIDs returns all issue ids
func (prs PullRequestList) GetIssueIDs() []int64 {
issueIDs := make([]int64, 0, len(prs))
for i := range prs {
issueIDs = append(issueIDs, prs[i].IssueID)
@ -192,24 +192,3 @@ func (prs PullRequestList) getIssueIDs() []int64 {
func (prs PullRequestList) LoadAttributes() error {
return prs.loadAttributes(db.DefaultContext)
}
// InvalidateCodeComments will lookup the prs for code comments which got invalidated by change
func (prs PullRequestList) InvalidateCodeComments(ctx context.Context, doer *user_model.User, repo *git.Repository, branch string) error {
if len(prs) == 0 {
return nil
}
issueIDs := prs.getIssueIDs()
var codeComments []*Comment
if err := db.GetEngine(ctx).
Where("type = ? and invalidated = ?", CommentTypeCode, false).
In("issue_id", issueIDs).
Find(&codeComments); err != nil {
return fmt.Errorf("find code comments: %w", err)
}
for _, comment := range codeComments {
if err := comment.CheckInvalidation(repo, doer, branch); err != nil {
return err
}
}
return nil
}

8
models/issues/review.go
View File

@ -972,7 +972,7 @@ func DeleteReview(r *Review) error {
ReviewID: r.ID,
}
if _, err := sess.Where(opts.toConds()).Delete(new(Comment)); err != nil {
if _, err := sess.Where(opts.ToConds()).Delete(new(Comment)); err != nil {
return err
}
@ -982,7 +982,7 @@ func DeleteReview(r *Review) error {
ReviewID: r.ID,
}
if _, err := sess.Where(opts.toConds()).Delete(new(Comment)); err != nil {
if _, err := sess.Where(opts.ToConds()).Delete(new(Comment)); err != nil {
return err
}
@ -1006,7 +1006,7 @@ func (r *Review) GetCodeCommentsCount() int {
IssueID: r.IssueID,
ReviewID: r.ID,
}
conds := opts.toConds()
conds := opts.ToConds()
if r.ID == 0 {
conds = conds.And(builder.Eq{"invalidated": false})
}
@ -1026,7 +1026,7 @@ func (r *Review) HTMLURL() string {
ReviewID: r.ID,
}
comment := new(Comment)
has, err := db.GetEngine(db.DefaultContext).Where(opts.toConds()).Get(comment)
has, err := db.GetEngine(db.DefaultContext).Where(opts.ToConds()).Get(comment)
if err != nil || !has {
return ""
}

2
services/pull/pull.go
View File

@ -240,7 +240,7 @@ func checkForInvalidation(ctx context.Context, requests issues_model.PullRequest
}
go func() {
// FIXME: graceful: We need to tell the manager we're doing something...
err := requests.InvalidateCodeComments(ctx, doer, gitRepo, branch)
err := InvalidateCodeComments(ctx, requests, doer, gitRepo, branch)
if err != nil {
log.Error("PullRequestList.InvalidateCodeComments: %v", err)
}

49
services/pull/review.go
View File

@ -23,6 +23,53 @@ import (
issue_service "code.gitea.io/gitea/services/issue"
)
var notEnoughLines = regexp.MustCompile(`fatal: file .* has only \d+ lines?`)
// 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 checkInvalidation(ctx context.Context, c *issues_model.Comment, 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 issues_model.UpdateCommentInvalidate(ctx, c)
}
if err != nil {
return err
}
if c.CommitSHA != "" && c.CommitSHA != commit.ID.String() {
c.Invalidated = true
return issues_model.UpdateCommentInvalidate(ctx, c)
}
return nil
}
// InvalidateCodeComments will lookup the prs for code comments which got invalidated by change
func InvalidateCodeComments(ctx context.Context, prs issues_model.PullRequestList, doer *user_model.User, repo *git.Repository, branch string) error {
if len(prs) == 0 {
return nil
}
issueIDs := prs.GetIssueIDs()
var codeComments []*issues_model.Comment
if err := db.Find(ctx, &issues_model.FindCommentsOptions{
ListOptions: db.ListOptions{
ListAll: true,
},
Type: issues_model.CommentTypeCode,
Invalidated: util.OptionalBoolFalse,
IssueIDs: issueIDs,
}, &codeComments); err != nil {
return fmt.Errorf("find code comments: %v", err)
}
for _, comment := range codeComments {
if err := checkInvalidation(ctx, comment, doer, repo, branch); err != nil {
return err
}
}
return nil
}
// CreateCodeComment creates a comment on the code line
func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.Repository, issue *issues_model.Issue, line int64, content, treePath string, isReview bool, replyReviewID int64, latestCommitID string) (*issues_model.Comment, error) {
var (
@ -114,8 +161,6 @@ func CreateCodeComment(ctx context.Context, doer *user_model.User, gitRepo *git.
return comment, nil
}
var notEnoughLines = regexp.MustCompile(`exit status 128 - fatal: file .* has only \d+ lines?`)
// createCodeComment creates a plain code comment at the specified line / path
func createCodeComment(ctx context.Context, doer *user_model.User, repo *repo_model.Repository, issue *issues_model.Issue, content, treePath string, line, reviewID int64) (*issues_model.Comment, error) {
var commitID, patch string