summaryrefslogtreecommitdiffstats
path: root/routers
diff options
context:
space:
mode:
authoroliverpool <3864879+oliverpool@users.noreply.github.com>2023-05-10 05:43:55 +0200
committerGitHub <noreply@github.com>2023-05-10 11:43:55 +0800
commit8030614386b5d3fa02dc294446a344d274b04a26 (patch)
tree33868012ca12609feafbb3e938e0fcb5efeb5130 /routers
parent5930ab5fdf7a970fcca3cd50b44cf1cacb615a54 (diff)
downloadgitea-8030614386b5d3fa02dc294446a344d274b04a26.tar.gz
gitea-8030614386b5d3fa02dc294446a344d274b04a26.zip
fix: release page for empty or non-existing target (#24470)
Fixes #24145 To solve the bug, I added a "computed" `TargetBehind` field to the `Release` model, which indicates the target branch of a release. This is particularly useful if the target branch was deleted in the meantime (or is empty). I also did a micro-optimization in `calReleaseNumCommitsBehind`. Instead of checking that a branch exists and then call `GetBranchCommit`, I immediately call `GetBranchCommit` and handle the `git.ErrNotExist` error. This optimization is covered by the added unit test.
Diffstat (limited to 'routers')
-rw-r--r--routers/web/repo/release.go34
-rw-r--r--routers/web/repo/release_test.go47
2 files changed, 69 insertions, 12 deletions
diff --git a/routers/web/repo/release.go b/routers/web/repo/release.go
index 689994c146..afba1f18bf 100644
--- a/routers/web/repo/release.go
+++ b/routers/web/repo/release.go
@@ -5,6 +5,7 @@
package repo
import (
+ "errors"
"fmt"
"net/http"
"strings"
@@ -16,6 +17,7 @@ import (
user_model "code.gitea.io/gitea/models/user"
"code.gitea.io/gitea/modules/base"
"code.gitea.io/gitea/modules/context"
+ "code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/markup"
"code.gitea.io/gitea/modules/markup/markdown"
@@ -36,24 +38,32 @@ const (
// calReleaseNumCommitsBehind calculates given release has how many commits behind release target.
func calReleaseNumCommitsBehind(repoCtx *context.Repository, release *repo_model.Release, countCache map[string]int64) error {
- // Get count if not exists
- if _, ok := countCache[release.Target]; !ok {
- // short-circuit for the default branch
- if repoCtx.Repository.DefaultBranch == release.Target || repoCtx.GitRepo.IsBranchExist(release.Target) {
- commit, err := repoCtx.GitRepo.GetBranchCommit(release.Target)
- if err != nil {
+ target := release.Target
+ if target == "" {
+ target = repoCtx.Repository.DefaultBranch
+ }
+ // Get count if not cached
+ if _, ok := countCache[target]; !ok {
+ commit, err := repoCtx.GitRepo.GetBranchCommit(target)
+ if err != nil {
+ var errNotExist git.ErrNotExist
+ if target == repoCtx.Repository.DefaultBranch || !errors.As(err, &errNotExist) {
return fmt.Errorf("GetBranchCommit: %w", err)
}
- countCache[release.Target], err = commit.CommitsCount()
+ // fallback to default branch
+ target = repoCtx.Repository.DefaultBranch
+ commit, err = repoCtx.GitRepo.GetBranchCommit(target)
if err != nil {
- return fmt.Errorf("CommitsCount: %w", err)
+ return fmt.Errorf("GetBranchCommit(DefaultBranch): %w", err)
}
- } else {
- // Use NumCommits of the newest release on that target
- countCache[release.Target] = release.NumCommits
+ }
+ countCache[target], err = commit.CommitsCount()
+ if err != nil {
+ return fmt.Errorf("CommitsCount: %w", err)
}
}
- release.NumCommitsBehind = countCache[release.Target] - release.NumCommits
+ release.NumCommitsBehind = countCache[target] - release.NumCommits
+ release.TargetBehind = target
return nil
}
diff --git a/routers/web/repo/release_test.go b/routers/web/repo/release_test.go
index 81ae58178f..9ec1b4d349 100644
--- a/routers/web/repo/release_test.go
+++ b/routers/web/repo/release_test.go
@@ -11,6 +11,8 @@ import (
"code.gitea.io/gitea/modules/test"
"code.gitea.io/gitea/modules/web"
"code.gitea.io/gitea/services/forms"
+
+ "github.com/stretchr/testify/assert"
)
func TestNewReleasePost(t *testing.T) {
@@ -62,3 +64,48 @@ func TestNewReleasePost(t *testing.T) {
ctx.Repo.GitRepo.Close()
}
}
+
+func TestNewReleasesList(t *testing.T) {
+ unittest.PrepareTestEnv(t)
+ ctx := test.MockContext(t, "user2/repo-release/releases")
+ test.LoadUser(t, ctx, 2)
+ test.LoadRepo(t, ctx, 57)
+ test.LoadGitRepo(t, ctx)
+ t.Cleanup(func() { ctx.Repo.GitRepo.Close() })
+
+ Releases(ctx)
+ releases := ctx.Data["Releases"].([]*repo_model.Release)
+ type computedFields struct {
+ NumCommitsBehind int64
+ TargetBehind string
+ }
+ expectedComputation := map[string]computedFields{
+ "v1.0": {
+ NumCommitsBehind: 3,
+ TargetBehind: "main",
+ },
+ "v1.1": {
+ NumCommitsBehind: 1,
+ TargetBehind: "main",
+ },
+ "v2.0": {
+ NumCommitsBehind: 0,
+ TargetBehind: "main",
+ },
+ "non-existing-target-branch": {
+ NumCommitsBehind: 1,
+ TargetBehind: "main",
+ },
+ "empty-target-branch": {
+ NumCommitsBehind: 1,
+ TargetBehind: "main",
+ },
+ }
+ for _, r := range releases {
+ actual := computedFields{
+ NumCommitsBehind: r.NumCommitsBehind,
+ TargetBehind: r.TargetBehind,
+ }
+ assert.Equal(t, expectedComputation[r.TagName], actual, "wrong computed fields for %s: %#v", r.TagName, r)
+ }
+}