Skip to content

Commit 3c51f38

Browse files
lunnylunny
committed
Refactor compare router param parse
1 parent 7fe0840 commit 3c51f38

File tree

4 files changed

+163
-21
lines changed

4 files changed

+163
-21
lines changed

‎routers/api/v1/repo/pull.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"code.gitea.io/gitea/modules/setting"
2929
api "code.gitea.io/gitea/modules/structs"
3030
"code.gitea.io/gitea/modules/timeutil"
31+
"code.gitea.io/gitea/modules/util"
3132
"code.gitea.io/gitea/modules/web"
3233
"code.gitea.io/gitea/routers/api/v1/utils"
3334
"code.gitea.io/gitea/routers/common"
@@ -1068,7 +1069,7 @@ type parseCompareInfoResult struct {
10681069
// parseCompareInfo returns non-nil if it succeeds, it always writes to the context and returns nil if it fails
10691070
func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *parseCompareInfoResult, closer func()) {
10701071
baseRepo := ctx.Repo.Repository
1071-
compareReq, err := common.ParseCompareRouterParam(baseRepo, compareParam)
1072+
compareReq, err := common.ParseCompareRouterParam(compareParam)
10721073
if err != nil {
10731074
ctx.APIErrorNotFound()
10741075
return nil, nil
@@ -1178,8 +1179,8 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *par
11781179
return nil, nil
11791180
}
11801181

1181-
baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(compareReq.BaseOriRef)
1182-
headRef := headGitRepo.UnstableGuessRefByShortName(compareReq.HeadOriRef)
1182+
baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(util.Iif(compareReq.BaseOriRef == "", baseRepo.DefaultBranch, compareReq.BaseOriRef))
1183+
headRef := headGitRepo.UnstableGuessRefByShortName(util.Iif(compareReq.HeadOriRef == "", headRepo.DefaultBranch, compareReq.HeadOriRef))
11831184

11841185
log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), compareReq.BaseOriRef, baseRef, compareReq.HeadOriRef, headRef)
11851186

@@ -1191,7 +1192,7 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *par
11911192
return nil, nil
11921193
}
11931194

1194-
compareInfo, err := pull_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef.ShortName(), headRef.ShortName(), false, false)
1195+
compareInfo, err := pull_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef.ShortName(), headRef.ShortName(), compareReq.DirectComparison(), false)
11951196
if err != nil {
11961197
ctx.APIErrorInternal(err)
11971198
return nil, nil

‎routers/common/compare.go‎

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,7 @@ type CompareRouterReq struct {
3434
}
3535

3636
func (cr *CompareRouterReq) DirectComparison() bool {
37-
return cr.DotTimes == 2
38-
}
39-
40-
func (cr *CompareRouterReq) CompareDots() string {
41-
return strings.Repeat(".", cr.DotTimes)
37+
return cr.DotTimes == 2 || cr.CaretTimes == 0
4238
}
4339

4440
func parseBase(base string) (string, int) {
@@ -86,15 +82,9 @@ func parseHead(head string) (string, string, string) {
8682
// format: <base branch>...[<head repo>:]<head branch>
8783
// base<-head: master...head:feature
8884
// same repo: master...feature
89-
func ParseCompareRouterParam(baseRepo *repo_model.Repository, routerParam string) (*CompareRouterReq, error) {
85+
func ParseCompareRouterParam(routerParam string) (*CompareRouterReq, error) {
9086
if routerParam == "" {
91-
return &CompareRouterReq{
92-
BaseOriRef: baseRepo.DefaultBranch,
93-
HeadOwner: baseRepo.Owner.Name,
94-
HeadRepoName: baseRepo.Name,
95-
HeadOriRef: baseRepo.DefaultBranch,
96-
DotTimes: 2,
97-
}, nil
87+
return &CompareRouterReq{}, nil
9888
}
9989

10090
var basePart, headPart string
@@ -108,7 +98,6 @@ func ParseCompareRouterParam(baseRepo *repo_model.Repository, routerParam string
10898
if len(parts) == 1 {
10999
headOwnerName, headRepoName, headRef := parseHead(routerParam)
110100
return &CompareRouterReq{
111-
BaseOriRef: baseRepo.DefaultBranch,
112101
HeadOriRef: headRef,
113102
HeadOwner: headOwnerName,
114103
HeadRepoName: headRepoName,

‎routers/common/compare_test.go‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package common
5+
6+
import (
7+
"testing"
8+
9+
"code.gitea.io/gitea/models/unittest"
10+
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
func TestCompareRouterReq(t *testing.T) {
15+
unittest.PrepareTestEnv(t)
16+
17+
kases := []struct {
18+
router string
19+
CompareRouterReq *CompareRouterReq
20+
}{
21+
{
22+
router: "",
23+
CompareRouterReq: &CompareRouterReq{
24+
BaseOriRef: "",
25+
HeadOriRef: "",
26+
DotTimes: 0,
27+
},
28+
},
29+
{
30+
router: "main...develop",
31+
CompareRouterReq: &CompareRouterReq{
32+
BaseOriRef: "main",
33+
HeadOriRef: "develop",
34+
DotTimes: 3,
35+
},
36+
},
37+
{
38+
router: "main..develop",
39+
CompareRouterReq: &CompareRouterReq{
40+
BaseOriRef: "main",
41+
HeadOriRef: "develop",
42+
DotTimes: 2,
43+
},
44+
},
45+
{
46+
router: "main^...develop",
47+
CompareRouterReq: &CompareRouterReq{
48+
BaseOriRef: "main",
49+
HeadOriRef: "develop",
50+
CaretTimes: 1,
51+
DotTimes: 3,
52+
},
53+
},
54+
{
55+
router: "main^^^^^...develop",
56+
CompareRouterReq: &CompareRouterReq{
57+
BaseOriRef: "main",
58+
HeadOriRef: "develop",
59+
CaretTimes: 5,
60+
DotTimes: 3,
61+
},
62+
},
63+
{
64+
router: "develop",
65+
CompareRouterReq: &CompareRouterReq{
66+
HeadOriRef: "develop",
67+
DotTimes: 3,
68+
},
69+
},
70+
{
71+
router: "lunny/forked_repo:develop",
72+
CompareRouterReq: &CompareRouterReq{
73+
HeadOwner: "lunny",
74+
HeadRepoName: "forked_repo",
75+
HeadOriRef: "develop",
76+
DotTimes: 3,
77+
},
78+
},
79+
{
80+
router: "main...lunny/forked_repo:develop",
81+
CompareRouterReq: &CompareRouterReq{
82+
BaseOriRef: "main",
83+
HeadOwner: "lunny",
84+
HeadRepoName: "forked_repo",
85+
HeadOriRef: "develop",
86+
DotTimes: 3,
87+
},
88+
},
89+
{
90+
router: "main...lunny/forked_repo:develop",
91+
CompareRouterReq: &CompareRouterReq{
92+
BaseOriRef: "main",
93+
HeadOwner: "lunny",
94+
HeadRepoName: "forked_repo",
95+
HeadOriRef: "develop",
96+
DotTimes: 3,
97+
},
98+
},
99+
{
100+
router: "main^...lunny/forked_repo:develop",
101+
CompareRouterReq: &CompareRouterReq{
102+
BaseOriRef: "main",
103+
HeadOwner: "lunny",
104+
HeadRepoName: "forked_repo",
105+
HeadOriRef: "develop",
106+
DotTimes: 3,
107+
CaretTimes: 1,
108+
},
109+
},
110+
{
111+
router: "v1.0...v1.1",
112+
CompareRouterReq: &CompareRouterReq{
113+
BaseOriRef: "v1.0",
114+
HeadOriRef: "v1.1",
115+
DotTimes: 3,
116+
},
117+
},
118+
{
119+
router: "teabot-patch-1...v0.0.1",
120+
CompareRouterReq: &CompareRouterReq{
121+
BaseOriRef: "teabot-patch-1",
122+
HeadOriRef: "v0.0.1",
123+
DotTimes: 3,
124+
},
125+
},
126+
{
127+
router: "teabot:feature1",
128+
CompareRouterReq: &CompareRouterReq{
129+
HeadOwner: "teabot",
130+
HeadOriRef: "feature1",
131+
DotTimes: 3,
132+
},
133+
},
134+
{
135+
router: "8eb19a5ae19abae15c0666d4ab98906139a7f439...283c030497b455ecfa759d4649f9f8b45158742e",
136+
CompareRouterReq: &CompareRouterReq{
137+
BaseOriRef: "8eb19a5ae19abae15c0666d4ab98906139a7f439",
138+
HeadOriRef: "283c030497b455ecfa759d4649f9f8b45158742e",
139+
DotTimes: 3,
140+
},
141+
},
142+
}
143+
144+
for _, kase := range kases {
145+
t.Run(kase.router, func(t *testing.T) {
146+
r, err := ParseCompareRouterParam(kase.router)
147+
assert.NoError(t, err)
148+
assert.Equal(t, kase.CompareRouterReq, r)
149+
})
150+
}
151+
}

‎routers/web/repo/compare.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo {
197197
ci := &common.CompareInfo{}
198198
fileOnly := ctx.FormBool("file-only")
199199

200-
compareReq, err := common.ParseCompareRouterParam(baseRepo, ctx.PathParam("*"))
200+
compareReq, err := common.ParseCompareRouterParam(ctx.PathParam("*"))
201201
if err != nil {
202202
ctx.ServerError("GetUserByName", err)
203203
return nil
@@ -259,8 +259,9 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo {
259259
}
260260
}
261261
}
262-
ci.BaseBranch = compareReq.BaseOriRef
263-
ci.HeadBranch = compareReq.HeadOriRef
262+
ci.BaseBranch = util.Iif(compareReq.BaseOriRef == "", baseRepo.DefaultBranch, compareReq.BaseOriRef)
263+
ci.HeadBranch = util.Iif(compareReq.HeadOriRef == "", ci.HeadRepo.DefaultBranch, compareReq.HeadOriRef)
264+
ci.DirectComparison = compareReq.DirectComparison()
264265
isSameRepo := baseRepo.ID == ci.HeadRepo.ID
265266

266267
ctx.Data["BaseName"] = baseRepo.OwnerName

0 commit comments

Comments
 (0)