Browse Source

release: improve page load performance

Previously, we load all releases of a repository which could hurt
performance when the repository has a lot of releases.

Now we're able to only load releases in current page view we need
to show by matching with 'tag_name'.
Unknwon 8 years ago
parent
commit
451aef7a1c

+ 1 - 1
gogs.go

@@ -16,7 +16,7 @@ import (
 	"github.com/gogits/gogs/modules/setting"
 )
 
-const APP_VER = "0.10.12.0309"
+const APP_VER = "0.10.12.0310"
 
 func init() {
 	setting.AppVer = APP_VER

+ 67 - 32
models/release.go

@@ -51,6 +51,26 @@ func (r *Release) AfterSet(colName string, _ xorm.Cell) {
 	}
 }
 
+func (r *Release) loadAttributes(e Engine) (err error) {
+	if r.Publisher == nil {
+		r.Publisher, err = getUserByID(e, r.PublisherID)
+		if err != nil {
+			if IsErrUserNotExist(err) {
+				r.PublisherID = -1
+				r.Publisher = NewGhostUser()
+			} else {
+				return fmt.Errorf("getUserByID.(Publisher) [%d]: %v", r.PublisherID, err)
+			}
+		}
+	}
+
+	return nil
+}
+
+func (r *Release) LoadAttributes() error {
+	return r.loadAttributes(x)
+}
+
 // IsReleaseExist returns true if release with given tag name already exists.
 func IsReleaseExist(repoID int64, tagName string) (bool, error) {
 	if len(tagName) == 0 {
@@ -60,31 +80,31 @@ func IsReleaseExist(repoID int64, tagName string) (bool, error) {
 	return x.Get(&Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)})
 }
 
-func createTag(gitRepo *git.Repository, rel *Release) error {
+func createTag(gitRepo *git.Repository, r *Release) error {
 	// Only actual create when publish.
-	if !rel.IsDraft {
-		if !gitRepo.IsTagExist(rel.TagName) {
-			commit, err := gitRepo.GetBranchCommit(rel.Target)
+	if !r.IsDraft {
+		if !gitRepo.IsTagExist(r.TagName) {
+			commit, err := gitRepo.GetBranchCommit(r.Target)
 			if err != nil {
 				return fmt.Errorf("GetBranchCommit: %v", err)
 			}
 
 			// Trim '--' prefix to prevent command line argument vulnerability.
-			rel.TagName = strings.TrimPrefix(rel.TagName, "--")
-			if err = gitRepo.CreateTag(rel.TagName, commit.ID.String()); err != nil {
+			r.TagName = strings.TrimPrefix(r.TagName, "--")
+			if err = gitRepo.CreateTag(r.TagName, commit.ID.String()); err != nil {
 				if strings.Contains(err.Error(), "is not a valid tag name") {
-					return ErrInvalidTagName{rel.TagName}
+					return ErrInvalidTagName{r.TagName}
 				}
 				return err
 			}
 		} else {
-			commit, err := gitRepo.GetTagCommit(rel.TagName)
+			commit, err := gitRepo.GetTagCommit(r.TagName)
 			if err != nil {
 				return fmt.Errorf("GetTagCommit: %v", err)
 			}
 
-			rel.Sha1 = commit.ID.String()
-			rel.NumCommits, err = commit.CommitsCount()
+			r.Sha1 = commit.ID.String()
+			r.NumCommits, err = commit.CommitsCount()
 			if err != nil {
 				return fmt.Errorf("CommitsCount: %v", err)
 			}
@@ -94,19 +114,19 @@ func createTag(gitRepo *git.Repository, rel *Release) error {
 }
 
 // CreateRelease creates a new release of repository.
-func CreateRelease(gitRepo *git.Repository, rel *Release) error {
-	isExist, err := IsReleaseExist(rel.RepoID, rel.TagName)
+func CreateRelease(gitRepo *git.Repository, r *Release) error {
+	isExist, err := IsReleaseExist(r.RepoID, r.TagName)
 	if err != nil {
 		return err
 	} else if isExist {
-		return ErrReleaseAlreadyExist{rel.TagName}
+		return ErrReleaseAlreadyExist{r.TagName}
 	}
 
-	if err = createTag(gitRepo, rel); err != nil {
+	if err = createTag(gitRepo, r); err != nil {
 		return err
 	}
-	rel.LowerTagName = strings.ToLower(rel.TagName)
-	_, err = x.InsertOne(rel)
+	r.LowerTagName = strings.ToLower(r.TagName)
+	_, err = x.InsertOne(r)
 	return err
 }
 
@@ -119,53 +139,68 @@ func GetRelease(repoID int64, tagName string) (*Release, error) {
 		return nil, ErrReleaseNotExist{0, tagName}
 	}
 
-	rel := &Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)}
-	_, err = x.Get(rel)
-	return rel, err
+	r := &Release{RepoID: repoID, LowerTagName: strings.ToLower(tagName)}
+	if _, err = x.Get(r); err != nil {
+		return nil, fmt.Errorf("Get: %v", err)
+	}
+
+	return r, r.LoadAttributes()
 }
 
 // GetReleaseByID returns release with given ID.
 func GetReleaseByID(id int64) (*Release, error) {
-	rel := new(Release)
-	has, err := x.Id(id).Get(rel)
+	r := new(Release)
+	has, err := x.Id(id).Get(r)
 	if err != nil {
 		return nil, err
 	} else if !has {
 		return nil, ErrReleaseNotExist{id, ""}
 	}
 
-	return rel, nil
+	return r, r.LoadAttributes()
+}
+
+// GetPublishedReleasesByRepoID returns a list of published releases of repository.
+// If matches is not empty, only published releases in matches will be returned.
+// In any case, drafts won't be returned by this function.
+func GetPublishedReleasesByRepoID(repoID int64, matches ...string) ([]*Release, error) {
+	sess := x.Where("repo_id = ?", repoID).And("is_draft = ?", false).Desc("created_unix")
+	if len(matches) > 0 {
+		sess.In("tag_name", matches)
+	}
+	releases := make([]*Release, 0, 5)
+	return releases, sess.Find(&releases, new(Release))
 }
 
-// GetReleasesByRepoID returns a list of releases of repository.
-func GetReleasesByRepoID(repoID int64) (rels []*Release, err error) {
-	err = x.Desc("created_unix").Find(&rels, Release{RepoID: repoID})
-	return rels, err
+// GetDraftReleasesByRepoID returns all draft releases of repository.
+func GetDraftReleasesByRepoID(repoID int64) ([]*Release, error) {
+	releases := make([]*Release, 0)
+	return releases, x.Where("repo_id = ?", repoID).And("is_draft = ?", true).Find(&releases)
 }
 
 type ReleaseSorter struct {
-	rels []*Release
+	releases []*Release
 }
 
 func (rs *ReleaseSorter) Len() int {
-	return len(rs.rels)
+	return len(rs.releases)
 }
 
 func (rs *ReleaseSorter) Less(i, j int) bool {
-	diffNum := rs.rels[i].NumCommits - rs.rels[j].NumCommits
+	diffNum := rs.releases[i].NumCommits - rs.releases[j].NumCommits
 	if diffNum != 0 {
 		return diffNum > 0
 	}
-	return rs.rels[i].Created.After(rs.rels[j].Created)
+	return rs.releases[i].Created.After(rs.releases[j].Created)
 }
 
 func (rs *ReleaseSorter) Swap(i, j int) {
-	rs.rels[i], rs.rels[j] = rs.rels[j], rs.rels[i]
+	rs.releases[i], rs.releases[j] = rs.releases[j], rs.releases[i]
 }
 
 // SortReleases sorts releases by number of commits and created time.
 func SortReleases(rels []*Release) {
-	sorter := &ReleaseSorter{rels: rels}
+	sorter := &ReleaseSorter{releases: rels}
 	sort.Sort(sorter)
 }
 

+ 44 - 34
routers/repo/release.go

@@ -59,35 +59,26 @@ func Releases(ctx *context.Context) {
 		return
 	}
 
-	// FIXME: should only get releases match tags result and drafts.
-	releases, err := models.GetReleasesByRepoID(ctx.Repo.Repository.ID)
+	releases, err := models.GetPublishedReleasesByRepoID(ctx.Repo.Repository.ID, tagsResult.Tags...)
 	if err != nil {
-		ctx.Handle(500, "GetReleasesByRepoID", err)
+		ctx.Handle(500, "GetPublishedReleasesByRepoID", err)
 		return
 	}
 
 	// Temproray cache commits count of used branches to speed up.
 	countCache := make(map[string]int64)
 
-	drafts := make([]*models.Release, 0, 1)
-	tags := make([]*models.Release, len(tagsResult.Tags))
+	results := make([]*models.Release, len(tagsResult.Tags))
 	for i, rawTag := range tagsResult.Tags {
 		for j, r := range releases {
-			if r == nil ||
-				(r.IsDraft && !ctx.Repo.IsOwner()) ||
-				(!r.IsDraft && r.TagName != rawTag) {
+			if r == nil || r.TagName != rawTag {
 				continue
 			}
 			releases[j] = nil // Mark as used.
 
-			r.Publisher, err = models.GetUserByID(r.PublisherID)
-			if err != nil {
-				if models.IsErrUserNotExist(err) {
-					r.Publisher = models.NewGhostUser()
-				} else {
-					ctx.Handle(500, "GetUserByID", err)
-					return
-				}
+			if err = r.LoadAttributes(); err != nil {
+				ctx.Handle(500, "LoadAttributes", err)
+				return
 			}
 
 			if err := calReleaseNumCommitsBehind(ctx.Repo, r, countCache); err != nil {
@@ -96,49 +87,68 @@ func Releases(ctx *context.Context) {
 			}
 
 			r.Note = markdown.RenderString(r.Note, ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas())
-			if r.TagName == rawTag {
-				tags[i] = r
-				break
-			}
-
-			if r.IsDraft {
-				drafts = append(drafts, r)
-			}
+			results[i] = r
+			break
 		}
 
-		if tags[i] == nil {
+		// No published release matches this tag
+		if results[i] == nil {
 			commit, err := ctx.Repo.GitRepo.GetTagCommit(rawTag)
 			if err != nil {
 				ctx.Handle(500, "GetTagCommit", err)
 				return
 			}
 
-			tags[i] = &models.Release{
+			results[i] = &models.Release{
 				Title:   rawTag,
 				TagName: rawTag,
 				Sha1:    commit.ID.String(),
 			}
 
-			tags[i].NumCommits, err = commit.CommitsCount()
+			results[i].NumCommits, err = commit.CommitsCount()
 			if err != nil {
 				ctx.Handle(500, "CommitsCount", err)
 				return
 			}
-			tags[i].NumCommitsBehind = ctx.Repo.CommitsCount - tags[i].NumCommits
+			results[i].NumCommitsBehind = ctx.Repo.CommitsCount - results[i].NumCommits
 		}
 	}
+	models.SortReleases(results)
+
+	// Only show drafts if user is viewing the latest page
+	var drafts []*models.Release
+	if tagsResult.HasLatest {
+		drafts, err = models.GetDraftReleasesByRepoID(ctx.Repo.Repository.ID)
+		if err != nil {
+			ctx.Handle(500, "GetDraftReleasesByRepoID", err)
+			return
+		}
+
+		for _, r := range drafts {
+			if err = r.LoadAttributes(); err != nil {
+				ctx.Handle(500, "LoadAttributes", err)
+				return
+			}
 
-	models.SortReleases(tags)
-	if len(drafts) > 0 && tagsResult.HasLatest {
-		tags = append(drafts, tags...)
+			if err := calReleaseNumCommitsBehind(ctx.Repo, r, countCache); err != nil {
+				ctx.Handle(500, "calReleaseNumCommitsBehind", err)
+				return
+			}
+
+			r.Note = markdown.RenderString(r.Note, ctx.Repo.RepoLink, ctx.Repo.Repository.ComposeMetas())
+		}
+
+		if len(drafts) > 0 {
+			results = append(drafts, results...)
+		}
 	}
 
-	ctx.Data["Releases"] = tags
+	ctx.Data["Releases"] = results
 	ctx.Data["HasPrevious"] = !tagsResult.HasLatest
 	ctx.Data["ReachEnd"] = tagsResult.ReachEnd
 	ctx.Data["PreviousAfter"] = tagsResult.PreviousAfter
-	if len(tags) > 0 {
-		ctx.Data["NextAfter"] = tags[len(tags)-1].TagName
+	if len(results) > 0 {
+		ctx.Data["NextAfter"] = results[len(results)-1].TagName
 	}
 	ctx.HTML(200, RELEASES)
 }

+ 1 - 1
templates/.VERSION

@@ -1 +1 @@
-0.10.12.0309
+0.10.12.0310

+ 1 - 1
vendor/github.com/gogits/git-module/git.go

@@ -10,7 +10,7 @@ import (
 	"time"
 )
 
-const _VERSION = "0.4.12"
+const _VERSION = "0.4.13"
 
 func Version() string {
 	return _VERSION

+ 1 - 1
vendor/github.com/gogits/git-module/repo_tag.go

@@ -171,7 +171,7 @@ func (repo *Repository) GetTagsAfter(after string, limit int) (*TagsResult, erro
 		}
 		if allTags[i] == after {
 			hasMatch = true
-			if limit > 0 && i-limit > 0 {
+			if limit > 0 && i-limit >= 0 {
 				previousAfter = allTags[i-limit]
 			}
 			continue

+ 3 - 3
vendor/vendor.json

@@ -159,10 +159,10 @@
 			"revisionTime": "2016-08-10T03:50:02Z"
 		},
 		{
-			"checksumSHA1": "RnWB5UDcTpSGepvWnwic1SyRZrc=",
+			"checksumSHA1": "sygMoTVpNmOTbsnZbgLR904p5GE=",
 			"path": "github.com/gogits/git-module",
-			"revision": "1b9552bab7499e5cbd96f6b550aaa0038faf6b67",
-			"revisionTime": "2017-02-19T18:16:29Z"
+			"revision": "2c083b82191752340c76271876671e18130ad662",
+			"revisionTime": "2017-03-10T19:06:55Z"
 		},
 		{
 			"checksumSHA1": "Rvj0LCHGhFQyIM7MzBPt1iRP89c=",