From 7fe084000861e6b19fd3dfc5f29205cd1b83cad7 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sun, 7 Dec 2025 22:14:02 -0800 Subject: [PATCH] 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