Skip to content

Commit 7fe0840

Browse files
lunnylunny
committed
Refactor compare router param parse
1 parent 98ef79d commit 7fe0840

File tree

4 files changed

+228
-145
lines changed

4 files changed

+228
-145
lines changed

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

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ package repo
55

66
import (
77
"net/http"
8-
"strings"
98

109
user_model "code.gitea.io/gitea/models/user"
1110
"code.gitea.io/gitea/modules/gitrepo"
@@ -52,18 +51,7 @@ func CompareDiff(ctx *context.APIContext) {
5251
}
5352
}
5453

55-
infoPath := ctx.PathParam("*")
56-
infos := []string{ctx.Repo.Repository.DefaultBranch, ctx.Repo.Repository.DefaultBranch}
57-
if infoPath != "" {
58-
infos = strings.SplitN(infoPath, "...", 2)
59-
if len(infos) != 2 {
60-
if infos = strings.SplitN(infoPath, "..", 2); len(infos) != 2 {
61-
infos = []string{ctx.Repo.Repository.DefaultBranch, infoPath}
62-
}
63-
}
64-
}
65-
66-
compareResult, closer := parseCompareInfo(ctx, api.CreatePullRequestOption{Base: infos[0], Head: infos[1]})
54+
compareResult, closer := parseCompareInfo(ctx, ctx.PathParam("*"))
6755
if ctx.Written() {
6856
return
6957
}

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

Lines changed: 63 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"code.gitea.io/gitea/modules/timeutil"
3131
"code.gitea.io/gitea/modules/web"
3232
"code.gitea.io/gitea/routers/api/v1/utils"
33+
"code.gitea.io/gitea/routers/common"
3334
asymkey_service "code.gitea.io/gitea/services/asymkey"
3435
"code.gitea.io/gitea/services/automerge"
3536
"code.gitea.io/gitea/services/context"
@@ -413,7 +414,7 @@ func CreatePullRequest(ctx *context.APIContext) {
413414
)
414415

415416
// Get repo/branch information
416-
compareResult, closer := parseCompareInfo(ctx, form)
417+
compareResult, closer := parseCompareInfo(ctx, form.Base+".."+form.Head)
417418
if ctx.Written() {
418419
return
419420
}
@@ -1065,61 +1066,76 @@ type parseCompareInfoResult struct {
10651066
}
10661067

10671068
// parseCompareInfo returns non-nil if it succeeds, it always writes to the context and returns nil if it fails
1068-
func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) (result *parseCompareInfoResult, closer func()) {
1069-
var err error
1070-
// Get compared branches information
1071-
// format: <base branch>...[<head repo>:]<head branch>
1072-
// base<-head: master...head:feature
1073-
// same repo: master...feature
1069+
func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *parseCompareInfoResult, closer func()) {
10741070
baseRepo := ctx.Repo.Repository
1075-
baseRefToGuess := form.Base
1076-
1077-
headUser := ctx.Repo.Owner
1078-
headRefToGuess := form.Head
1079-
if headInfos := strings.Split(form.Head, ":"); len(headInfos) == 1 {
1080-
// If there is no head repository, it means pull request between same repository.
1081-
// Do nothing here because the head variables have been assigned above.
1082-
} else if len(headInfos) == 2 {
1083-
// There is a head repository (the head repository could also be the same base repo)
1084-
headRefToGuess = headInfos[1]
1085-
headUser, err = user_model.GetUserByName(ctx, headInfos[0])
1086-
if err != nil {
1087-
if user_model.IsErrUserNotExist(err) {
1088-
ctx.APIErrorNotFound("GetUserByName")
1089-
} else {
1090-
ctx.APIErrorInternal(err)
1091-
}
1092-
return nil, nil
1093-
}
1094-
} else {
1071+
compareReq, err := common.ParseCompareRouterParam(baseRepo, compareParam)
1072+
if err != nil {
10951073
ctx.APIErrorNotFound()
10961074
return nil, nil
10971075
}
10981076

1099-
isSameRepo := ctx.Repo.Owner.ID == headUser.ID
1100-
1101-
// Check if current user has fork of repository or in the same repository.
1102-
headRepo := repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID)
1103-
if headRepo == nil && !isSameRepo {
1104-
err = baseRepo.GetBaseRepo(ctx)
1105-
if err != nil {
1106-
ctx.APIErrorInternal(err)
1077+
var headRepo *repo_model.Repository
1078+
if compareReq.HeadOwner == "" {
1079+
if compareReq.HeadRepoName == "" {
1080+
headRepo = ctx.Repo.Repository
1081+
} else {
1082+
ctx.APIErrorNotFound()
11071083
return nil, nil
11081084
}
1109-
1110-
// Check if baseRepo's base repository is the same as headUser's repository.
1111-
if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID {
1112-
log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID)
1113-
ctx.APIErrorNotFound("GetBaseRepo")
1114-
return nil, nil
1085+
} else {
1086+
var headUser *user_model.User
1087+
if compareReq.HeadOwner == ctx.Repo.Owner.Name {
1088+
headUser = ctx.Repo.Owner
1089+
} else {
1090+
headUser, err = user_model.GetUserByName(ctx, compareReq.HeadOwner)
1091+
if err != nil {
1092+
if user_model.IsErrUserNotExist(err) {
1093+
ctx.APIErrorNotFound("GetUserByName")
1094+
} else {
1095+
ctx.APIErrorInternal(err)
1096+
}
1097+
return nil, nil
1098+
}
1099+
}
1100+
if compareReq.HeadRepoName == "" {
1101+
headRepo = repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID)
1102+
if headRepo == nil && headUser.ID != baseRepo.OwnerID {
1103+
err = baseRepo.GetBaseRepo(ctx)
1104+
if err != nil {
1105+
ctx.APIErrorInternal(err)
1106+
return nil, nil
1107+
}
1108+
1109+
// Check if baseRepo's base repository is the same as headUser's repository.
1110+
if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID {
1111+
log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID)
1112+
ctx.APIErrorNotFound("GetBaseRepo")
1113+
return nil, nil
1114+
}
1115+
// Assign headRepo so it can be used below.
1116+
headRepo = baseRepo.BaseRepo
1117+
}
1118+
} else {
1119+
if compareReq.HeadOwner == ctx.Repo.Owner.Name && compareReq.HeadRepoName == ctx.Repo.Repository.Name {
1120+
headRepo = ctx.Repo.Repository
1121+
} else {
1122+
headRepo, err = repo_model.GetRepositoryByName(ctx, headUser.ID, compareReq.HeadRepoName)
1123+
if err != nil {
1124+
if repo_model.IsErrRepoNotExist(err) {
1125+
ctx.APIErrorNotFound("GetRepositoryByName")
1126+
} else {
1127+
ctx.APIErrorInternal(err)
1128+
}
1129+
return nil, nil
1130+
}
1131+
}
11151132
}
1116-
// Assign headRepo so it can be used below.
1117-
headRepo = baseRepo.BaseRepo
11181133
}
11191134

1135+
isSameRepo := baseRepo.ID == headRepo.ID
1136+
11201137
var headGitRepo *git.Repository
11211138
if isSameRepo {
1122-
headRepo = ctx.Repo.Repository
11231139
headGitRepo = ctx.Repo.GitRepo
11241140
closer = func() {} // no need to close the head repo because it shares the base repo
11251141
} else {
@@ -1162,10 +1178,10 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption)
11621178
return nil, nil
11631179
}
11641180

1165-
baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefToGuess)
1166-
headRef := headGitRepo.UnstableGuessRefByShortName(headRefToGuess)
1181+
baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(compareReq.BaseOriRef)
1182+
headRef := headGitRepo.UnstableGuessRefByShortName(compareReq.HeadOriRef)
11671183

1168-
log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), baseRefToGuess, baseRef, headRefToGuess, headRef)
1184+
log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), compareReq.BaseOriRef, baseRef, compareReq.HeadOriRef, headRef)
11691185

11701186
baseRefValid := baseRef.IsBranch() || baseRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName), baseRef.ShortName())
11711187
headRefValid := headRef.IsBranch() || headRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(headRepo.ObjectFormatName), headRef.ShortName())

‎routers/common/compare.go‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,12 @@
44
package common
55

66
import (
7+
"strings"
8+
79
repo_model "code.gitea.io/gitea/models/repo"
810
user_model "code.gitea.io/gitea/models/user"
911
"code.gitea.io/gitea/modules/git"
12+
"code.gitea.io/gitea/modules/util"
1013
pull_service "code.gitea.io/gitea/services/pull"
1114
)
1215

@@ -20,3 +23,113 @@ type CompareInfo struct {
2023
HeadBranch string
2124
DirectComparison bool
2225
}
26+
27+
type CompareRouterReq struct {
28+
BaseOriRef string
29+
HeadOwner string
30+
HeadRepoName string
31+
HeadOriRef string
32+
CaretTimes int // ^ times after base ref
33+
DotTimes int
34+
}
35+
36+
func (cr *CompareRouterReq) DirectComparison() bool {
37+
return cr.DotTimes == 2
38+
}
39+
40+
func (cr *CompareRouterReq) CompareDots() string {
41+
return strings.Repeat(".", cr.DotTimes)
42+
}
43+
44+
func parseBase(base string) (string, int) {
45+
parts := strings.SplitN(base, "^", 2)
46+
if len(parts) == 1 {
47+
return base, 0
48+
}
49+
return parts[0], len(parts[1]) + 1
50+
}
51+
52+
func parseHead(head string) (string, string, string) {
53+
paths := strings.SplitN(head, ":", 2)
54+
if len(paths) == 1 {
55+
return "", "", paths[0]
56+
}
57+
ownerRepo := strings.SplitN(paths[0], "/", 2)
58+
if len(ownerRepo) == 1 {
59+
return paths[0], "", paths[1]
60+
}
61+
return ownerRepo[0], ownerRepo[1], paths[1]
62+
}
63+
64+
// ParseCompareRouterParam Get compare information from the router parameter.
65+
// A full compare url is of the form:
66+
//
67+
// 1. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headBranch}
68+
// 2. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}:{:headBranch}
69+
// 3. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}/{:headRepoName}:{:headBranch}
70+
// 4. /{:baseOwner}/{:baseRepoName}/compare/{:headBranch}
71+
// 5. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}:{:headBranch}
72+
// 6. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}/{:headRepoName}:{:headBranch}
73+
//
74+
// Here we obtain the infoPath "{:baseBranch}...[{:headOwner}/{:headRepoName}:]{:headBranch}" as ctx.PathParam("*")
75+
// with the :baseRepo in ctx.Repo.
76+
//
77+
// Note: Generally :headRepoName is not provided here - we are only passed :headOwner.
78+
//
79+
// How do we determine the :headRepo?
80+
//
81+
// 1. If :headOwner is not set then the :headRepo = :baseRepo
82+
// 2. If :headOwner is set - then look for the fork of :baseRepo owned by :headOwner
83+
// 3. But... :baseRepo could be a fork of :headOwner's repo - so check that
84+
// 4. Now, :baseRepo and :headRepos could be forks of the same repo - so check that
85+
//
86+
// format: <base branch>...[<head repo>:]<head branch>
87+
// base<-head: master...head:feature
88+
// same repo: master...feature
89+
func ParseCompareRouterParam(baseRepo *repo_model.Repository, routerParam string) (*CompareRouterReq, error) {
90+
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
98+
}
99+
100+
var basePart, headPart string
101+
dotTimes := 3
102+
parts := strings.Split(routerParam, "...")
103+
if len(parts) > 2 {
104+
return nil, util.NewInvalidArgumentErrorf("invalid compare router: %s", routerParam)
105+
}
106+
if len(parts) != 2 {
107+
parts = strings.Split(routerParam, "..")
108+
if len(parts) == 1 {
109+
headOwnerName, headRepoName, headRef := parseHead(routerParam)
110+
return &CompareRouterReq{
111+
BaseOriRef: baseRepo.DefaultBranch,
112+
HeadOriRef: headRef,
113+
HeadOwner: headOwnerName,
114+
HeadRepoName: headRepoName,
115+
DotTimes: dotTimes,
116+
}, nil
117+
} else if len(parts) > 2 {
118+
return nil, util.NewInvalidArgumentErrorf("invalid compare router: %s", routerParam)
119+
}
120+
dotTimes = 2
121+
}
122+
basePart, headPart = parts[0], parts[1]
123+
124+
baseRef, caretTimes := parseBase(basePart)
125+
headOwnerName, headRepoName, headRef := parseHead(headPart)
126+
127+
return &CompareRouterReq{
128+
BaseOriRef: baseRef,
129+
HeadOriRef: headRef,
130+
HeadOwner: headOwnerName,
131+
HeadRepoName: headRepoName,
132+
CaretTimes: caretTimes,
133+
DotTimes: dotTimes,
134+
}, nil
135+
}

0 commit comments

Comments
 (0)