Browse Source

#2966 code cleanup

Unknwon 3 years ago
parent
commit
6b98d58906
11 changed files with 138 additions and 127 deletions
  1. 1 1
      .gopmfile
  2. 1 1
      README.md
  3. 1 1
      glide.lock
  4. 1 1
      gogs.go
  5. 3 2
      models/error.go
  6. 72 49
      models/issue.go
  7. 37 9
      models/issue_comment.go
  8. 1 1
      routers/api/v1/repo/issue.go
  9. 20 60
      routers/api/v1/repo/issue_comment.go
  10. 0 1
      routers/repo/issue.go
  11. 1 1
      templates/.VERSION

+ 1 - 1
.gopmfile

@@ -19,7 +19,7 @@ github.com/go-xorm/xorm = commit:c6c7056
19 19
 github.com/gogits/chardet = commit:2404f77
20 20
 github.com/gogits/cron = commit:7f3990a
21 21
 github.com/gogits/git-module = commit:7b206b5
22
-github.com/gogits/go-gogs-client = commit:2ffd470
22
+github.com/gogits/go-gogs-client = commit:c52f7ee
23 23
 github.com/issue9/identicon = commit:d36b545
24 24
 github.com/jaytaylor/html2text = commit:52d9b78
25 25
 github.com/kardianos/minwinsvc = commit:cad6b2b

+ 1 - 1
README.md

@@ -3,7 +3,7 @@ Gogs - Go Git Service [![Build Status](https://travis-ci.org/gogits/gogs.svg?bra
3 3
 
4 4
 ![](https://github.com/gogits/gogs/blob/master/public/img/gogs-large-resize.png?raw=true)
5 5
 
6
-##### Current tip version: 0.9.85 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions)
6
+##### Current tip version: 0.9.86 (see [Releases](https://github.com/gogits/gogs/releases) for binary versions)
7 7
 
8 8
 | Web | UI  | Preview  |
9 9
 |:-------------:|:-------:|:-------:|

+ 1 - 1
glide.lock

@@ -43,7 +43,7 @@ imports:
43 43
 - name: github.com/gogits/git-module
44 44
   version: 7b206b529a09ae8cfa1df52a6c0cdd2612cfc6fc
45 45
 - name: github.com/gogits/go-gogs-client
46
-  version: 2ffd4704c6f37d7fb10110450fe035fa6df08db8
46
+  version: c52f7ee0cc58d3cd6e379025552873a8df6de322
47 47
 - name: github.com/issue9/identicon
48 48
   version: d36b54562f4cf70c83653e13dc95c220c79ef521
49 49
 - name: github.com/jaytaylor/html2text

+ 1 - 1
gogs.go

@@ -17,7 +17,7 @@ import (
17 17
 	"github.com/gogits/gogs/modules/setting"
18 18
 )
19 19
 
20
-const APP_VER = "0.9.85.0824"
20
+const APP_VER = "0.9.86.0826"
21 21
 
22 22
 func init() {
23 23
 	runtime.GOMAXPROCS(runtime.NumCPU())

+ 3 - 2
models/error.go

@@ -526,7 +526,8 @@ func (err ErrPullRequestNotExist) Error() string {
526 526
 //         \/             \/      \/     \/     \/
527 527
 
528 528
 type ErrCommentNotExist struct {
529
-	ID int64
529
+	ID      int64
530
+	IssueID int64
530 531
 }
531 532
 
532 533
 func IsErrCommentNotExist(err error) bool {
@@ -535,7 +536,7 @@ func IsErrCommentNotExist(err error) bool {
535 536
 }
536 537
 
537 538
 func (err ErrCommentNotExist) Error() string {
538
-	return fmt.Sprintf("comment does not exist [id: %d]", err.ID)
539
+	return fmt.Sprintf("comment does not exist [id: %d, issue_id: %d]", err.ID, err.IssueID)
539 540
 }
540 541
 
541 542
 // .____          ___.          .__

+ 72 - 49
models/issue.go

@@ -73,70 +73,55 @@ func (issue *Issue) BeforeUpdate() {
73 73
 }
74 74
 
75 75
 func (issue *Issue) AfterSet(colName string, _ xorm.Cell) {
76
-	var err error
77 76
 	switch colName {
78
-	case "id":
79
-		issue.Attachments, err = GetAttachmentsByIssueID(issue.ID)
80
-		if err != nil {
81
-			log.Error(3, "GetAttachmentsByIssueID[%d]: %v", issue.ID, err)
82
-		}
83
-
84
-		issue.Comments, err = GetCommentsByIssueID(issue.ID)
85
-		if err != nil {
86
-			log.Error(3, "GetCommentsByIssueID[%d]: %v", issue.ID, err)
87
-		}
77
+	case "deadline_unix":
78
+		issue.Deadline = time.Unix(issue.DeadlineUnix, 0).Local()
79
+	case "created_unix":
80
+		issue.Created = time.Unix(issue.CreatedUnix, 0).Local()
81
+	case "updated_unix":
82
+		issue.Updated = time.Unix(issue.UpdatedUnix, 0).Local()
83
+	}
84
+}
88 85
 
89
-		issue.Labels, err = GetLabelsByIssueID(issue.ID)
86
+func (issue *Issue) loadAttributes(e Engine) (err error) {
87
+	if issue.Repo == nil {
88
+		issue.Repo, err = getRepositoryByID(e, issue.RepoID)
90 89
 		if err != nil {
91
-			log.Error(3, "GetLabelsByIssueID[%d]: %v", issue.ID, err)
90
+			return fmt.Errorf("getRepositoryByID [%d]: %v", issue.RepoID, err)
92 91
 		}
92
+	}
93 93
 
94
-	case "poster_id":
95
-		issue.Poster, err = GetUserByID(issue.PosterID)
94
+	if issue.Poster == nil {
95
+		issue.Poster, err = getUserByID(e, issue.PosterID)
96 96
 		if err != nil {
97 97
 			if IsErrUserNotExist(err) {
98 98
 				issue.PosterID = -1
99 99
 				issue.Poster = NewGhostUser()
100 100
 			} else {
101
-				log.Error(3, "GetUserByID[%d]: %v", issue.ID, err)
101
+				return fmt.Errorf("getUserByID.(poster) [%d]: %v", issue.PosterID, err)
102 102
 			}
103 103
 			return
104 104
 		}
105
+	}
105 106
 
106
-	case "milestone_id":
107
-		if issue.MilestoneID == 0 {
108
-			return
109
-		}
110
-
111
-		issue.Milestone, err = GetMilestoneByRepoID(issue.RepoID, issue.MilestoneID)
107
+	if issue.Labels == nil {
108
+		issue.Labels, err = getLabelsByIssueID(e, issue.ID)
112 109
 		if err != nil {
113
-			log.Error(3, "GetMilestoneById[%d]: %v", issue.ID, err)
114
-		}
115
-
116
-	case "assignee_id":
117
-		if issue.AssigneeID == 0 {
118
-			return
110
+			return fmt.Errorf("getLabelsByIssueID [%d]: %v", issue.ID, err)
119 111
 		}
112
+	}
120 113
 
121
-		issue.Assignee, err = GetUserByID(issue.AssigneeID)
114
+	if issue.Milestone == nil && issue.MilestoneID > 0 {
115
+		issue.Milestone, err = getMilestoneByRepoID(e, issue.RepoID, issue.MilestoneID)
122 116
 		if err != nil {
123
-			log.Error(3, "GetUserByID[%d]: %v", issue.ID, err)
117
+			return fmt.Errorf("getMilestoneByRepoID [repo_id: %d, milestone_id: %d]: %v", issue.RepoID, issue.MilestoneID, err)
124 118
 		}
125
-
126
-	case "deadline_unix":
127
-		issue.Deadline = time.Unix(issue.DeadlineUnix, 0).Local()
128
-	case "created_unix":
129
-		issue.Created = time.Unix(issue.CreatedUnix, 0).Local()
130
-	case "updated_unix":
131
-		issue.Updated = time.Unix(issue.UpdatedUnix, 0).Local()
132 119
 	}
133
-}
134 120
 
135
-func (issue *Issue) loadAttributes(e Engine) (err error) {
136
-	if issue.Repo == nil {
137
-		issue.Repo, err = getRepositoryByID(e, issue.RepoID)
121
+	if issue.Assignee == nil && issue.AssigneeID > 0 {
122
+		issue.Assignee, err = getUserByID(e, issue.AssigneeID)
138 123
 		if err != nil {
139
-			return fmt.Errorf("getRepositoryByID [%d]: %v", issue.RepoID, err)
124
+			return fmt.Errorf("getUserByID.(assignee) [%d]: %v", issue.AssigneeID, err)
140 125
 		}
141 126
 	}
142 127
 
@@ -148,6 +133,20 @@ func (issue *Issue) loadAttributes(e Engine) (err error) {
148 133
 		}
149 134
 	}
150 135
 
136
+	if issue.Attachments == nil {
137
+		issue.Attachments, err = getAttachmentsByIssueID(e, issue.ID)
138
+		if err != nil {
139
+			return fmt.Errorf("getAttachmentsByIssueID [%d]: %v", issue.ID, err)
140
+		}
141
+	}
142
+
143
+	if issue.Comments == nil {
144
+		issue.Comments, err = getCommentsByIssueID(e, issue.ID)
145
+		if err != nil {
146
+			return fmt.Errorf("getCommentsByIssueID [%d]: %v", issue.ID, err)
147
+		}
148
+	}
149
+
151 150
 	return nil
152 151
 }
153 152
 
@@ -757,8 +756,8 @@ func GetIssueByRef(ref string) (*Issue, error) {
757 756
 	return issue, issue.LoadAttributes()
758 757
 }
759 758
 
760
-// GetIssueByIndex returns issue by given index in repository.
761
-func GetIssueByIndex(repoID, index int64) (*Issue, error) {
759
+// GetIssueByIndex returns raw issue without loading attributes by index in a repository.
760
+func GetRawIssueByIndex(repoID, index int64) (*Issue, error) {
762 761
 	issue := &Issue{
763 762
 		RepoID: repoID,
764 763
 		Index:  index,
@@ -769,6 +768,15 @@ func GetIssueByIndex(repoID, index int64) (*Issue, error) {
769 768
 	} else if !has {
770 769
 		return nil, ErrIssueNotExist{0, repoID, index}
771 770
 	}
771
+	return issue, nil
772
+}
773
+
774
+// GetIssueByIndex returns issue by index in a repository.
775
+func GetIssueByIndex(repoID, index int64) (*Issue, error) {
776
+	issue, err := GetRawIssueByIndex(repoID, index)
777
+	if err != nil {
778
+		return nil, err
779
+	}
772 780
 	return issue, issue.LoadAttributes()
773 781
 }
774 782
 
@@ -868,7 +876,18 @@ func Issues(opts *IssuesOptions) ([]*Issue, error) {
868 876
 	}
869 877
 
870 878
 	issues := make([]*Issue, 0, setting.UI.IssuePagingNum)
871
-	return issues, sess.Find(&issues)
879
+	if err := sess.Find(&issues); err != nil {
880
+		return nil, fmt.Errorf("Find: %v", err)
881
+	}
882
+
883
+	// FIXME: use IssueList to improve performance.
884
+	for i := range issues {
885
+		if err := issues[i].LoadAttributes(); err != nil {
886
+			return nil, fmt.Errorf("LoadAttributes [%d]: %v", issues[i].ID, err)
887
+		}
888
+	}
889
+
890
+	return issues, nil
872 891
 }
873 892
 
874 893
 // .___                             ____ ___
@@ -1412,7 +1431,7 @@ func getMilestoneByRepoID(e Engine, repoID, id int64) (*Milestone, error) {
1412 1431
 	return m, nil
1413 1432
 }
1414 1433
 
1415
-// GetWebhookByRepoID returns milestone of repository by given ID.
1434
+// GetWebhookByRepoID returns the milestone in a repository.
1416 1435
 func GetMilestoneByRepoID(repoID, id int64) (*Milestone, error) {
1417 1436
 	return getMilestoneByRepoID(x, repoID, id)
1418 1437
 }
@@ -1721,10 +1740,14 @@ func GetAttachmentByUUID(uuid string) (*Attachment, error) {
1721 1740
 	return getAttachmentByUUID(x, uuid)
1722 1741
 }
1723 1742
 
1724
-// GetAttachmentsByIssueID returns all attachments for given issue by ID.
1725
-func GetAttachmentsByIssueID(issueID int64) ([]*Attachment, error) {
1743
+func getAttachmentsByIssueID(e Engine, issueID int64) ([]*Attachment, error) {
1726 1744
 	attachments := make([]*Attachment, 0, 10)
1727
-	return attachments, x.Where("issue_id=? AND comment_id=0", issueID).Find(&attachments)
1745
+	return attachments, e.Where("issue_id = ? AND comment_id = 0", issueID).Find(&attachments)
1746
+}
1747
+
1748
+// GetAttachmentsByIssueID returns all attachments of an issue.
1749
+func GetAttachmentsByIssueID(issueID int64) ([]*Attachment, error) {
1750
+	return getAttachmentsByIssueID(x, issueID)
1728 1751
 }
1729 1752
 
1730 1753
 // GetAttachmentsByCommentID returns all attachments if comment by given ID.

+ 37 - 9
models/issue_comment.go

@@ -11,6 +11,7 @@ import (
11 11
 
12 12
 	"github.com/Unknwon/com"
13 13
 	"github.com/go-xorm/xorm"
14
+
14 15
 	api "github.com/gogits/go-gogs-client"
15 16
 
16 17
 	"github.com/gogits/gogs/modules/log"
@@ -59,6 +60,8 @@ type Comment struct {
59 60
 
60 61
 	Created     time.Time `xorm:"-"`
61 62
 	CreatedUnix int64
63
+	Updated     time.Time `xorm:"-"`
64
+	UpdatedUnix int64
62 65
 
63 66
 	// Reference issue in commit message
64 67
 	CommitSHA string `xorm:"VARCHAR(40)"`
@@ -71,6 +74,11 @@ type Comment struct {
71 74
 
72 75
 func (c *Comment) BeforeInsert() {
73 76
 	c.CreatedUnix = time.Now().Unix()
77
+	c.UpdatedUnix = c.CreatedUnix
78
+}
79
+
80
+func (c *Comment) BeforeUpdate() {
81
+	c.UpdatedUnix = time.Now().Unix()
74 82
 }
75 83
 
76 84
 func (c *Comment) AfterSet(colName string, _ xorm.Cell) {
@@ -94,6 +102,8 @@ func (c *Comment) AfterSet(colName string, _ xorm.Cell) {
94 102
 		}
95 103
 	case "created_unix":
96 104
 		c.Created = time.Unix(c.CreatedUnix, 0).Local()
105
+	case "updated_unix":
106
+		c.Updated = time.Unix(c.UpdatedUnix, 0).Local()
97 107
 	}
98 108
 }
99 109
 
@@ -105,16 +115,14 @@ func (c *Comment) AfterDelete() {
105 115
 	}
106 116
 }
107 117
 
108
-// APIFormat convert Comment struct to api.Comment struct
109 118
 func (c *Comment) APIFormat() *api.Comment {
110
-	apiComment := &api.Comment{
119
+	return &api.Comment{
111 120
 		ID:      c.ID,
112 121
 		Poster:  c.Poster.APIFormat(),
113 122
 		Body:    c.Content,
114 123
 		Created: c.Created,
124
+		Updated: c.Updated,
115 125
 	}
116
-
117
-	return apiComment
118 126
 }
119 127
 
120 128
 // HashTag returns unique hash tag for comment.
@@ -341,15 +349,32 @@ func GetCommentByID(id int64) (*Comment, error) {
341 349
 	if err != nil {
342 350
 		return nil, err
343 351
 	} else if !has {
344
-		return nil, ErrCommentNotExist{id}
352
+		return nil, ErrCommentNotExist{id, 0}
345 353
 	}
346 354
 	return c, nil
347 355
 }
348 356
 
349
-// GetCommentsByIssueID returns all comments of issue by given ID.
350
-func GetCommentsByIssueID(issueID int64) ([]*Comment, error) {
357
+func getCommentsByIssueIDSince(e Engine, issueID, since int64) ([]*Comment, error) {
351 358
 	comments := make([]*Comment, 0, 10)
352
-	return comments, x.Where("issue_id=?", issueID).Asc("created_unix").Find(&comments)
359
+	sess := e.Where("issue_id = ?", issueID).Asc("created_unix")
360
+	if since > 0 {
361
+		sess.And("created_unix >= ?", since)
362
+	}
363
+	return comments, sess.Find(&comments)
364
+}
365
+
366
+func getCommentsByIssueID(e Engine, issueID int64) ([]*Comment, error) {
367
+	return getCommentsByIssueIDSince(e, issueID, -1)
368
+}
369
+
370
+// GetCommentsByIssueID returns all comments of an issue.
371
+func GetCommentsByIssueID(issueID int64) ([]*Comment, error) {
372
+	return getCommentsByIssueID(x, issueID)
373
+}
374
+
375
+// GetCommentsByIssueID returns a list of comments of an issue since a given time point.
376
+func GetCommentsByIssueIDSince(issueID, since int64) ([]*Comment, error) {
377
+	return getCommentsByIssueIDSince(x, issueID, since)
353 378
 }
354 379
 
355 380
 // UpdateComment updates information of comment.
@@ -358,10 +383,13 @@ func UpdateComment(c *Comment) error {
358 383
 	return err
359 384
 }
360 385
 
361
-// DeleteCommentByID deletes a comment by given ID.
386
+// DeleteCommentByID deletes the comment by given ID.
362 387
 func DeleteCommentByID(id int64) error {
363 388
 	comment, err := GetCommentByID(id)
364 389
 	if err != nil {
390
+		if IsErrCommentNotExist(err) {
391
+			return nil
392
+		}
365 393
 		return err
366 394
 	}
367 395
 

+ 1 - 1
routers/api/v1/repo/issue.go

@@ -25,9 +25,9 @@ func ListIssues(ctx *context.APIContext) {
25 25
 		return
26 26
 	}
27 27
 
28
+	// FIXME: use IssueList to improve performance.
28 29
 	apiIssues := make([]*api.Issue, len(issues))
29 30
 	for i := range issues {
30
-		// FIXME: use IssueList to improve performance.
31 31
 		if err = issues[i].LoadAttributes(); err != nil {
32 32
 			ctx.Error(500, "LoadAttributes", err)
33 33
 			return

+ 20 - 60
routers/api/v1/repo/issue_comment.go

@@ -10,112 +10,72 @@ import (
10 10
 
11 11
 	"github.com/gogits/gogs/models"
12 12
 	"github.com/gogits/gogs/modules/context"
13
-	"github.com/gogits/gogs/modules/log"
14 13
 )
15 14
 
16
-const (
17
-	ISO8601Format = "2006-01-02T15:04:05Z"
18
-)
19
-
20
-// ListIssueComments list comments on an issue
21 15
 func ListIssueComments(ctx *context.APIContext) {
22 16
 	var since time.Time
23
-	var withSince bool
17
+	if len(ctx.Query("since")) > 0 {
18
+		since, _ = time.Parse(time.RFC3339, ctx.Query("since"))
19
+	}
24 20
 
25
-	// we get the issue instead of comments directly
26
-	// because to get comments we need to gets issue first,
27
-	// and there is already comments in the issue
28
-	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
21
+	// comments,err:=models.GetCommentsByIssueIDSince(, since)
22
+	issue, err := models.GetRawIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
29 23
 	if err != nil {
30
-		ctx.Error(500, "Comments", err)
24
+		ctx.Error(500, "GetRawIssueByIndex", err)
31 25
 		return
32 26
 	}
33 27
 
34
-	// parse `since`, by default we don't use `since`
35
-	if len(ctx.Query("since")) > 0 {
36
-		var err error
37
-		since, err = time.Parse(ISO8601Format, ctx.Query("since"))
38
-		if err == nil {
39
-			withSince = true
40
-		}
28
+	comments, err := models.GetCommentsByIssueIDSince(issue.ID, since.Unix())
29
+	if err != nil {
30
+		ctx.Error(500, "GetCommentsByIssueIDSince", err)
31
+		return
41 32
 	}
42 33
 
43
-	apiComments := []*api.Comment{}
44
-	for _, comment := range issue.Comments {
45
-		if withSince && !comment.Created.After(since) {
46
-			continue
47
-		}
48
-		apiComments = append(apiComments, comment.APIFormat())
34
+	apiComments := make([]*api.Comment, len(comments))
35
+	for i := range comments {
36
+		apiComments[i] = comments[i].APIFormat()
49 37
 	}
50
-
51 38
 	ctx.JSON(200, &apiComments)
52 39
 }
53 40
 
54
-// CreateIssueComment create comment on an issue
55 41
 func CreateIssueComment(ctx *context.APIContext, form api.CreateIssueCommentOption) {
56
-	// check issue
57 42
 	issue, err := models.GetIssueByIndex(ctx.Repo.Repository.ID, ctx.ParamsInt64(":index"))
58 43
 	if err != nil {
59
-		ctx.Error(500, "Comments", err)
44
+		ctx.Error(500, "GetIssueByIndex", err)
60 45
 		return
61 46
 	}
62 47
 
63
-	comment := &models.Comment{
64
-		Content: form.Body,
65
-	}
66
-
67
-	if len(form.Body) == 0 {
68
-		ctx.Handle(400, "CreateIssueComment:empty content", err)
69
-		return
70
-	}
71
-
72
-	// create comment
73
-	comment, err = models.CreateIssueComment(ctx.User, ctx.Repo.Repository, issue, form.Body, []string{})
48
+	comment, err := models.CreateIssueComment(ctx.User, ctx.Repo.Repository, issue, form.Body, nil)
74 49
 	if err != nil {
75
-		ctx.Handle(500, "CreateIssueComment", err)
50
+		ctx.Error(500, "CreateIssueComment", err)
76 51
 		return
77 52
 	}
78 53
 
79
-	log.Trace("Comment created: %d/%d/%d", ctx.Repo.Repository.ID, issue.ID, comment.ID)
80
-
81
-	// Refetch from database to assign some automatic values
82
-	comment, err = models.GetCommentByID(comment.ID)
83
-	if err != nil {
84
-		log.Info("Failed to refetch comment:%v", err)
85
-	}
86 54
 	ctx.JSON(201, comment.APIFormat())
87 55
 }
88 56
 
89
-// EditIssueComment edits an issue comment
90 57
 func EditIssueComment(ctx *context.APIContext, form api.EditIssueCommentOption) {
91 58
 	comment, err := models.GetCommentByID(ctx.ParamsInt64(":id"))
92 59
 	if err != nil {
93 60
 		if models.IsErrCommentNotExist(err) {
94 61
 			ctx.Error(404, "GetCommentByID", err)
95 62
 		} else {
96
-			ctx.Handle(500, "GetCommentByID", err)
63
+			ctx.Error(500, "GetCommentByID", err)
97 64
 		}
98 65
 		return
99 66
 	}
100 67
 
101 68
 	if !ctx.IsSigned || (ctx.User.ID != comment.PosterID && !ctx.Repo.IsAdmin()) {
102
-		ctx.Error(403, "edit comment", err)
69
+		ctx.Status(403)
103 70
 		return
104 71
 	} else if comment.Type != models.COMMENT_TYPE_COMMENT {
105
-		ctx.Error(204, "edit comment", err)
72
+		ctx.Status(204)
106 73
 		return
107 74
 	}
108 75
 
109 76
 	comment.Content = form.Body
110
-	if len(comment.Content) == 0 {
111
-		ctx.JSON(200, map[string]interface{}{
112
-			"content": "",
113
-		})
114
-		return
115
-	}
116
-
117 77
 	if err := models.UpdateComment(comment); err != nil {
118
-		ctx.Handle(500, "UpdateComment", err)
78
+		ctx.Error(500, "UpdateComment", err)
119 79
 		return
120 80
 	}
121 81
 	ctx.JSON(200, comment.APIFormat())

+ 0 - 1
routers/repo/issue.go

@@ -166,7 +166,6 @@ func Issues(ctx *context.Context) {
166 166
 	pager := paginater.New(total, setting.UI.IssuePagingNum, page, 5)
167 167
 	ctx.Data["Page"] = pager
168 168
 
169
-	// Get issues.
170 169
 	issues, err := models.Issues(&models.IssuesOptions{
171 170
 		UserID:      uid,
172 171
 		AssigneeID:  assigneeID,

+ 1 - 1
templates/.VERSION

@@ -1 +1 @@
1
-0.9.85.0824
1
+0.9.86.0826