From 7fe084000861e6b19fd3dfc5f29205cd1b83cad7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 7 Dec 2025 22:14:02 -0800 Subject: [PATCH 1/2] Refactor compare router param parse --- routers/api/v1/repo/compare.go | 14 +-- routers/api/v1/repo/pull.go | 110 ++++++++++++---------- routers/common/compare.go | 113 ++++++++++++++++++++++ routers/web/repo/compare.go | 166 +++++++++++++-------------------- 4 files changed, 243 insertions(+), 160 deletions(-) diff --git a/routers/api/v1/repo/compare.go b/routers/api/v1/repo/compare.go index 6d427c8073..9472bb7d3f 100644 --- a/routers/api/v1/repo/compare.go +++ b/routers/api/v1/repo/compare.go @@ -5,7 +5,6 @@ package repo import ( "net/http" - "strings" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/gitrepo" @@ -52,18 +51,7 @@ func CompareDiff(ctx *context.APIContext) { } } - infoPath := ctx.PathParam("*") - infos := []string{ctx.Repo.Repository.DefaultBranch, ctx.Repo.Repository.DefaultBranch} - if infoPath != "" { - infos = strings.SplitN(infoPath, "...", 2) - if len(infos) != 2 { - if infos = strings.SplitN(infoPath, "..", 2); len(infos) != 2 { - infos = []string{ctx.Repo.Repository.DefaultBranch, infoPath} - } - } - } - - compareResult, closer := parseCompareInfo(ctx, api.CreatePullRequestOption{Base: infos[0], Head: infos[1]}) + compareResult, closer := parseCompareInfo(ctx, ctx.PathParam("*")) if ctx.Written() { return } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 073c784242..87bcdd2658 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -30,6 +30,7 @@ import ( "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" + "code.gitea.io/gitea/routers/common" asymkey_service "code.gitea.io/gitea/services/asymkey" "code.gitea.io/gitea/services/automerge" "code.gitea.io/gitea/services/context" @@ -413,7 +414,7 @@ func CreatePullRequest(ctx *context.APIContext) { ) // Get repo/branch information - compareResult, closer := parseCompareInfo(ctx, form) + compareResult, closer := parseCompareInfo(ctx, form.Base+".."+form.Head) if ctx.Written() { return } @@ -1065,61 +1066,76 @@ type parseCompareInfoResult struct { } // parseCompareInfo returns non-nil if it succeeds, it always writes to the context and returns nil if it fails -func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) (result *parseCompareInfoResult, closer func()) { - var err error - // Get compared branches information - // format: ...[:] - // base<-head: master...head:feature - // same repo: master...feature +func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *parseCompareInfoResult, closer func()) { baseRepo := ctx.Repo.Repository - baseRefToGuess := form.Base - - headUser := ctx.Repo.Owner - headRefToGuess := form.Head - if headInfos := strings.Split(form.Head, ":"); len(headInfos) == 1 { - // If there is no head repository, it means pull request between same repository. - // Do nothing here because the head variables have been assigned above. - } else if len(headInfos) == 2 { - // There is a head repository (the head repository could also be the same base repo) - headRefToGuess = headInfos[1] - headUser, err = user_model.GetUserByName(ctx, headInfos[0]) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.APIErrorNotFound("GetUserByName") - } else { - ctx.APIErrorInternal(err) - } - return nil, nil - } - } else { + compareReq, err := common.ParseCompareRouterParam(baseRepo, compareParam) + if err != nil { ctx.APIErrorNotFound() return nil, nil } - isSameRepo := ctx.Repo.Owner.ID == headUser.ID - - // Check if current user has fork of repository or in the same repository. - headRepo := repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID) - if headRepo == nil && !isSameRepo { - err = baseRepo.GetBaseRepo(ctx) - if err != nil { - ctx.APIErrorInternal(err) + var headRepo *repo_model.Repository + if compareReq.HeadOwner == "" { + if compareReq.HeadRepoName == "" { + headRepo = ctx.Repo.Repository + } else { + ctx.APIErrorNotFound() return nil, nil } - - // Check if baseRepo's base repository is the same as headUser's repository. - if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID { - log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID) - ctx.APIErrorNotFound("GetBaseRepo") - return nil, nil + } else { + var headUser *user_model.User + if compareReq.HeadOwner == ctx.Repo.Owner.Name { + headUser = ctx.Repo.Owner + } else { + headUser, err = user_model.GetUserByName(ctx, compareReq.HeadOwner) + if err != nil { + if user_model.IsErrUserNotExist(err) { + ctx.APIErrorNotFound("GetUserByName") + } else { + ctx.APIErrorInternal(err) + } + return nil, nil + } + } + if compareReq.HeadRepoName == "" { + headRepo = repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID) + if headRepo == nil && headUser.ID != baseRepo.OwnerID { + err = baseRepo.GetBaseRepo(ctx) + if err != nil { + ctx.APIErrorInternal(err) + return nil, nil + } + + // Check if baseRepo's base repository is the same as headUser's repository. + if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID { + log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID) + ctx.APIErrorNotFound("GetBaseRepo") + return nil, nil + } + // Assign headRepo so it can be used below. + headRepo = baseRepo.BaseRepo + } + } else { + if compareReq.HeadOwner == ctx.Repo.Owner.Name && compareReq.HeadRepoName == ctx.Repo.Repository.Name { + headRepo = ctx.Repo.Repository + } else { + headRepo, err = repo_model.GetRepositoryByName(ctx, headUser.ID, compareReq.HeadRepoName) + if err != nil { + if repo_model.IsErrRepoNotExist(err) { + ctx.APIErrorNotFound("GetRepositoryByName") + } else { + ctx.APIErrorInternal(err) + } + return nil, nil + } + } } - // Assign headRepo so it can be used below. - headRepo = baseRepo.BaseRepo } + isSameRepo := baseRepo.ID == headRepo.ID + var headGitRepo *git.Repository if isSameRepo { - headRepo = ctx.Repo.Repository headGitRepo = ctx.Repo.GitRepo closer = func() {} // no need to close the head repo because it shares the base repo } else { @@ -1162,10 +1178,10 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) return nil, nil } - baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(baseRefToGuess) - headRef := headGitRepo.UnstableGuessRefByShortName(headRefToGuess) + baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(compareReq.BaseOriRef) + headRef := headGitRepo.UnstableGuessRefByShortName(compareReq.HeadOriRef) - log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), baseRefToGuess, baseRef, headRefToGuess, headRef) + log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), compareReq.BaseOriRef, baseRef, compareReq.HeadOriRef, headRef) baseRefValid := baseRef.IsBranch() || baseRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(ctx.Repo.Repository.ObjectFormatName), baseRef.ShortName()) headRefValid := headRef.IsBranch() || headRef.IsTag() || git.IsStringLikelyCommitID(git.ObjectFormatFromName(headRepo.ObjectFormatName), headRef.ShortName()) diff --git a/routers/common/compare.go b/routers/common/compare.go index fda31a07ba..386d54b8b9 100644 --- a/routers/common/compare.go +++ b/routers/common/compare.go @@ -4,9 +4,12 @@ package common import ( + "strings" + repo_model "code.gitea.io/gitea/models/repo" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/git" + "code.gitea.io/gitea/modules/util" pull_service "code.gitea.io/gitea/services/pull" ) @@ -20,3 +23,113 @@ type CompareInfo struct { HeadBranch string DirectComparison bool } + +type CompareRouterReq struct { + BaseOriRef string + HeadOwner string + HeadRepoName string + HeadOriRef string + CaretTimes int // ^ times after base ref + DotTimes int +} + +func (cr *CompareRouterReq) DirectComparison() bool { + return cr.DotTimes == 2 +} + +func (cr *CompareRouterReq) CompareDots() string { + return strings.Repeat(".", cr.DotTimes) +} + +func parseBase(base string) (string, int) { + parts := strings.SplitN(base, "^", 2) + if len(parts) == 1 { + return base, 0 + } + return parts[0], len(parts[1]) + 1 +} + +func parseHead(head string) (string, string, string) { + paths := strings.SplitN(head, ":", 2) + if len(paths) == 1 { + return "", "", paths[0] + } + ownerRepo := strings.SplitN(paths[0], "/", 2) + if len(ownerRepo) == 1 { + return paths[0], "", paths[1] + } + return ownerRepo[0], ownerRepo[1], paths[1] +} + +// ParseCompareRouterParam Get compare information from the router parameter. +// A full compare url is of the form: +// +// 1. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headBranch} +// 2. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}:{:headBranch} +// 3. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}/{:headRepoName}:{:headBranch} +// 4. /{:baseOwner}/{:baseRepoName}/compare/{:headBranch} +// 5. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}:{:headBranch} +// 6. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}/{:headRepoName}:{:headBranch} +// +// Here we obtain the infoPath "{:baseBranch}...[{:headOwner}/{:headRepoName}:]{:headBranch}" as ctx.PathParam("*") +// with the :baseRepo in ctx.Repo. +// +// Note: Generally :headRepoName is not provided here - we are only passed :headOwner. +// +// How do we determine the :headRepo? +// +// 1. If :headOwner is not set then the :headRepo = :baseRepo +// 2. If :headOwner is set - then look for the fork of :baseRepo owned by :headOwner +// 3. But... :baseRepo could be a fork of :headOwner's repo - so check that +// 4. Now, :baseRepo and :headRepos could be forks of the same repo - so check that +// +// format: ...[:] +// base<-head: master...head:feature +// same repo: master...feature +func ParseCompareRouterParam(baseRepo *repo_model.Repository, routerParam string) (*CompareRouterReq, error) { + if routerParam == "" { + return &CompareRouterReq{ + BaseOriRef: baseRepo.DefaultBranch, + HeadOwner: baseRepo.Owner.Name, + HeadRepoName: baseRepo.Name, + HeadOriRef: baseRepo.DefaultBranch, + DotTimes: 2, + }, nil + } + + var basePart, headPart string + dotTimes := 3 + parts := strings.Split(routerParam, "...") + if len(parts) > 2 { + return nil, util.NewInvalidArgumentErrorf("invalid compare router: %s", routerParam) + } + if len(parts) != 2 { + parts = strings.Split(routerParam, "..") + if len(parts) == 1 { + headOwnerName, headRepoName, headRef := parseHead(routerParam) + return &CompareRouterReq{ + BaseOriRef: baseRepo.DefaultBranch, + HeadOriRef: headRef, + HeadOwner: headOwnerName, + HeadRepoName: headRepoName, + DotTimes: dotTimes, + }, nil + } else if len(parts) > 2 { + return nil, util.NewInvalidArgumentErrorf("invalid compare router: %s", routerParam) + } + dotTimes = 2 + } + basePart, headPart = parts[0], parts[1] + + baseRef, caretTimes := parseBase(basePart) + headOwnerName, headRepoName, headRef := parseHead(headPart) + + return &CompareRouterReq{ + BaseOriRef: baseRef, + HeadOriRef: headRef, + HeadOwner: headOwnerName, + HeadRepoName: headRepoName, + CaretTimes: caretTimes, + DotTimes: dotTimes, + }, nil +} diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 7750278a8d..4f3e5206ee 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -195,110 +195,76 @@ func setCsvCompareContext(ctx *context.Context) { func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { baseRepo := ctx.Repo.Repository ci := &common.CompareInfo{} - fileOnly := ctx.FormBool("file-only") - // Get compared branches information - // A full compare url is of the form: - // - // 1. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headBranch} - // 2. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}:{:headBranch} - // 3. /{:baseOwner}/{:baseRepoName}/compare/{:baseBranch}...{:headOwner}/{:headRepoName}:{:headBranch} - // 4. /{:baseOwner}/{:baseRepoName}/compare/{:headBranch} - // 5. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}:{:headBranch} - // 6. /{:baseOwner}/{:baseRepoName}/compare/{:headOwner}/{:headRepoName}:{:headBranch} - // - // Here we obtain the infoPath "{:baseBranch}...[{:headOwner}/{:headRepoName}:]{:headBranch}" as ctx.PathParam("*") - // with the :baseRepo in ctx.Repo. - // - // Note: Generally :headRepoName is not provided here - we are only passed :headOwner. - // - // How do we determine the :headRepo? - // - // 1. If :headOwner is not set then the :headRepo = :baseRepo - // 2. If :headOwner is set - then look for the fork of :baseRepo owned by :headOwner - // 3. But... :baseRepo could be a fork of :headOwner's repo - so check that - // 4. Now, :baseRepo and :headRepos could be forks of the same repo - so check that - // - // format: ...[:] - // base<-head: master...head:feature - // same repo: master...feature - - var ( - isSameRepo bool - infoPath string - err error - ) - - infoPath = ctx.PathParam("*") - var infos []string - if infoPath == "" { - infos = []string{baseRepo.DefaultBranch, baseRepo.DefaultBranch} - } else { - infos = strings.SplitN(infoPath, "...", 2) - if len(infos) != 2 { - if infos = strings.SplitN(infoPath, "..", 2); len(infos) == 2 { - ci.DirectComparison = true - ctx.Data["PageIsComparePull"] = false - } else { - infos = []string{baseRepo.DefaultBranch, infoPath} - } - } - } - - ctx.Data["BaseName"] = baseRepo.OwnerName - ci.BaseBranch = infos[0] - ctx.Data["BaseBranch"] = ci.BaseBranch - - // If there is no head repository, it means compare between same repository. - headInfos := strings.Split(infos[1], ":") - if len(headInfos) == 1 { - isSameRepo = true - ci.HeadUser = ctx.Repo.Owner - ci.HeadBranch = headInfos[0] - } else if len(headInfos) == 2 { - headInfosSplit := strings.Split(headInfos[0], "/") - if len(headInfosSplit) == 1 { - ci.HeadUser, err = user_model.GetUserByName(ctx, headInfos[0]) - if err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.NotFound(nil) - } else { - ctx.ServerError("GetUserByName", err) - } - return nil - } - ci.HeadBranch = headInfos[1] - isSameRepo = ci.HeadUser.ID == ctx.Repo.Owner.ID - if isSameRepo { - ci.HeadRepo = baseRepo - } - } else { - ci.HeadRepo, err = repo_model.GetRepositoryByOwnerAndName(ctx, headInfosSplit[0], headInfosSplit[1]) - if err != nil { - if repo_model.IsErrRepoNotExist(err) { - ctx.NotFound(nil) - } else { - ctx.ServerError("GetRepositoryByOwnerAndName", err) - } - return nil - } - if err := ci.HeadRepo.LoadOwner(ctx); err != nil { - if user_model.IsErrUserNotExist(err) { - ctx.NotFound(nil) - } else { - ctx.ServerError("GetUserByName", err) - } - return nil - } - ci.HeadBranch = headInfos[1] - ci.HeadUser = ci.HeadRepo.Owner - isSameRepo = ci.HeadRepo.ID == ctx.Repo.Repository.ID - } - } else { - ctx.NotFound(nil) + compareReq, err := common.ParseCompareRouterParam(baseRepo, ctx.PathParam("*")) + if err != nil { + ctx.ServerError("GetUserByName", err) return nil } + + if compareReq.HeadOwner == "" { + if compareReq.HeadRepoName == "" { + ci.HeadRepo = baseRepo + } else { + ctx.NotFound(nil) + return nil + } + } else { + var headUser *user_model.User + if compareReq.HeadOwner == ctx.Repo.Owner.Name { + headUser = ctx.Repo.Owner + } else { + headUser, err = user_model.GetUserByName(ctx, compareReq.HeadOwner) + if err != nil { + if user_model.IsErrUserNotExist(err) { + ctx.NotFound(nil) + } else { + ctx.ServerError("GetUserByName", err) + } + return nil + } + } + if compareReq.HeadRepoName == "" { + ci.HeadRepo = repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID) + if ci.HeadRepo == nil && headUser.ID != baseRepo.OwnerID { + err = baseRepo.GetBaseRepo(ctx) + if err != nil { + ctx.ServerError("GetBaseRepo", err) + return nil + } + + // Check if baseRepo's base repository is the same as headUser's repository. + if baseRepo.BaseRepo == nil || baseRepo.BaseRepo.OwnerID != headUser.ID { + log.Trace("parseCompareInfo[%d]: does not have fork or in same repository", baseRepo.ID) + ctx.NotFound(nil) + return nil + } + // Assign headRepo so it can be used below. + ci.HeadRepo = baseRepo.BaseRepo + } + } else { + if compareReq.HeadOwner == ctx.Repo.Owner.Name && compareReq.HeadRepoName == ctx.Repo.Repository.Name { + ci.HeadRepo = ctx.Repo.Repository + } else { + ci.HeadRepo, err = repo_model.GetRepositoryByName(ctx, headUser.ID, compareReq.HeadRepoName) + if err != nil { + if repo_model.IsErrRepoNotExist(err) { + ctx.NotFound(nil) + } else { + ctx.ServerError("GetRepositoryByName", err) + } + return nil + } + } + } + } + ci.BaseBranch = compareReq.BaseOriRef + ci.HeadBranch = compareReq.HeadOriRef + isSameRepo := baseRepo.ID == ci.HeadRepo.ID + + ctx.Data["BaseName"] = baseRepo.OwnerName + ctx.Data["BaseBranch"] = ci.BaseBranch ctx.Data["HeadUser"] = ci.HeadUser ctx.Data["HeadBranch"] = ci.HeadBranch ctx.Repo.PullRequest.SameRepo = isSameRepo From 3c51f3825365b571c9262616de2c4c2687e71176 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 7 Dec 2025 22:55:36 -0800 Subject: [PATCH 2/2] Refactor compare router param parse --- routers/api/v1/repo/pull.go | 9 +- routers/common/compare.go | 17 +--- routers/common/compare_test.go | 151 +++++++++++++++++++++++++++++++++ routers/web/repo/compare.go | 7 +- 4 files changed, 163 insertions(+), 21 deletions(-) create mode 100644 routers/common/compare_test.go diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 87bcdd2658..815540957b 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -28,6 +28,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/utils" "code.gitea.io/gitea/routers/common" @@ -1068,7 +1069,7 @@ type parseCompareInfoResult struct { // parseCompareInfo returns non-nil if it succeeds, it always writes to the context and returns nil if it fails func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *parseCompareInfoResult, closer func()) { baseRepo := ctx.Repo.Repository - compareReq, err := common.ParseCompareRouterParam(baseRepo, compareParam) + compareReq, err := common.ParseCompareRouterParam(compareParam) if err != nil { ctx.APIErrorNotFound() return nil, nil @@ -1178,8 +1179,8 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *par return nil, nil } - baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(compareReq.BaseOriRef) - headRef := headGitRepo.UnstableGuessRefByShortName(compareReq.HeadOriRef) + baseRef := ctx.Repo.GitRepo.UnstableGuessRefByShortName(util.Iif(compareReq.BaseOriRef == "", baseRepo.DefaultBranch, compareReq.BaseOriRef)) + headRef := headGitRepo.UnstableGuessRefByShortName(util.Iif(compareReq.HeadOriRef == "", headRepo.DefaultBranch, compareReq.HeadOriRef)) log.Trace("Repo path: %q, base ref: %q->%q, head ref: %q->%q", ctx.Repo.Repository.RelativePath(), compareReq.BaseOriRef, baseRef, compareReq.HeadOriRef, headRef) @@ -1191,7 +1192,7 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *par return nil, nil } - compareInfo, err := pull_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef.ShortName(), headRef.ShortName(), false, false) + compareInfo, err := pull_service.GetCompareInfo(ctx, baseRepo, headRepo, headGitRepo, baseRef.ShortName(), headRef.ShortName(), compareReq.DirectComparison(), false) if err != nil { ctx.APIErrorInternal(err) return nil, nil diff --git a/routers/common/compare.go b/routers/common/compare.go index 386d54b8b9..94c0474b94 100644 --- a/routers/common/compare.go +++ b/routers/common/compare.go @@ -34,11 +34,7 @@ type CompareRouterReq struct { } func (cr *CompareRouterReq) DirectComparison() bool { - return cr.DotTimes == 2 -} - -func (cr *CompareRouterReq) CompareDots() string { - return strings.Repeat(".", cr.DotTimes) + return cr.DotTimes == 2 || cr.CaretTimes == 0 } func parseBase(base string) (string, int) { @@ -86,15 +82,9 @@ func parseHead(head string) (string, string, string) { // format: ...[:] // base<-head: master...head:feature // same repo: master...feature -func ParseCompareRouterParam(baseRepo *repo_model.Repository, routerParam string) (*CompareRouterReq, error) { +func ParseCompareRouterParam(routerParam string) (*CompareRouterReq, error) { if routerParam == "" { - return &CompareRouterReq{ - BaseOriRef: baseRepo.DefaultBranch, - HeadOwner: baseRepo.Owner.Name, - HeadRepoName: baseRepo.Name, - HeadOriRef: baseRepo.DefaultBranch, - DotTimes: 2, - }, nil + return &CompareRouterReq{}, nil } var basePart, headPart string @@ -108,7 +98,6 @@ func ParseCompareRouterParam(baseRepo *repo_model.Repository, routerParam string if len(parts) == 1 { headOwnerName, headRepoName, headRef := parseHead(routerParam) return &CompareRouterReq{ - BaseOriRef: baseRepo.DefaultBranch, HeadOriRef: headRef, HeadOwner: headOwnerName, HeadRepoName: headRepoName, diff --git a/routers/common/compare_test.go b/routers/common/compare_test.go new file mode 100644 index 0000000000..cb041ae85a --- /dev/null +++ b/routers/common/compare_test.go @@ -0,0 +1,151 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package common + +import ( + "testing" + + "code.gitea.io/gitea/models/unittest" + + "github.com/stretchr/testify/assert" +) + +func TestCompareRouterReq(t *testing.T) { + unittest.PrepareTestEnv(t) + + kases := []struct { + router string + CompareRouterReq *CompareRouterReq + }{ + { + router: "", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "", + HeadOriRef: "", + DotTimes: 0, + }, + }, + { + router: "main...develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "main..develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOriRef: "develop", + DotTimes: 2, + }, + }, + { + router: "main^...develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOriRef: "develop", + CaretTimes: 1, + DotTimes: 3, + }, + }, + { + router: "main^^^^^...develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOriRef: "develop", + CaretTimes: 5, + DotTimes: 3, + }, + }, + { + router: "develop", + CompareRouterReq: &CompareRouterReq{ + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "lunny/forked_repo:develop", + CompareRouterReq: &CompareRouterReq{ + HeadOwner: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "main...lunny/forked_repo:develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOwner: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "main...lunny/forked_repo:develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOwner: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + }, + }, + { + router: "main^...lunny/forked_repo:develop", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "main", + HeadOwner: "lunny", + HeadRepoName: "forked_repo", + HeadOriRef: "develop", + DotTimes: 3, + CaretTimes: 1, + }, + }, + { + router: "v1.0...v1.1", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "v1.0", + HeadOriRef: "v1.1", + DotTimes: 3, + }, + }, + { + router: "teabot-patch-1...v0.0.1", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "teabot-patch-1", + HeadOriRef: "v0.0.1", + DotTimes: 3, + }, + }, + { + router: "teabot:feature1", + CompareRouterReq: &CompareRouterReq{ + HeadOwner: "teabot", + HeadOriRef: "feature1", + DotTimes: 3, + }, + }, + { + router: "8eb19a5ae19abae15c0666d4ab98906139a7f439...283c030497b455ecfa759d4649f9f8b45158742e", + CompareRouterReq: &CompareRouterReq{ + BaseOriRef: "8eb19a5ae19abae15c0666d4ab98906139a7f439", + HeadOriRef: "283c030497b455ecfa759d4649f9f8b45158742e", + DotTimes: 3, + }, + }, + } + + for _, kase := range kases { + t.Run(kase.router, func(t *testing.T) { + r, err := ParseCompareRouterParam(kase.router) + assert.NoError(t, err) + assert.Equal(t, kase.CompareRouterReq, r) + }) + } +} diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index 4f3e5206ee..c84eb09c44 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -197,7 +197,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { ci := &common.CompareInfo{} fileOnly := ctx.FormBool("file-only") - compareReq, err := common.ParseCompareRouterParam(baseRepo, ctx.PathParam("*")) + compareReq, err := common.ParseCompareRouterParam(ctx.PathParam("*")) if err != nil { ctx.ServerError("GetUserByName", err) return nil @@ -259,8 +259,9 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { } } } - ci.BaseBranch = compareReq.BaseOriRef - ci.HeadBranch = compareReq.HeadOriRef + ci.BaseBranch = util.Iif(compareReq.BaseOriRef == "", baseRepo.DefaultBranch, compareReq.BaseOriRef) + ci.HeadBranch = util.Iif(compareReq.HeadOriRef == "", ci.HeadRepo.DefaultBranch, compareReq.HeadOriRef) + ci.DirectComparison = compareReq.DirectComparison() isSameRepo := baseRepo.ID == ci.HeadRepo.ID ctx.Data["BaseName"] = baseRepo.OwnerName