diff --git a/models/user/user.go b/models/user/user.go index 925be83713..764890e26e 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -1461,3 +1461,14 @@ func GetUserOrOrgIDByName(ctx context.Context, name string) (int64, error) { } return id, nil } + +func GetUserOrOrgByName(ctx context.Context, name string) (*User, error) { + var u User + has, err := db.GetEngine(ctx).Table("user").Where("name = ?", name).Get(&u) + if err != nil { + return nil, err + } else if !has { + return nil, fmt.Errorf("user or org with name %s: %w", name, util.ErrNotExist) + } + return &u, nil +} diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index a90dba12c0..fe395dd587 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1075,6 +1075,12 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *par return nil, nil } + // remove the check when we support compare with carets + if compareReq.CaretTimes > 0 { + ctx.APIErrorNotFound("Unsupported compare syntax with carets") + return nil, nil + } + var headRepo *repo_model.Repository if compareReq.HeadOwner == "" { if compareReq.HeadRepoName != "" { // unsupported syntax @@ -1084,11 +1090,11 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *par headRepo = ctx.Repo.Repository } else { - var headUser *user_model.User + var headOwner *user_model.User if compareReq.HeadOwner == ctx.Repo.Owner.Name { - headUser = ctx.Repo.Owner + headOwner = ctx.Repo.Owner } else { - headUser, err = user_model.GetUserByName(ctx, compareReq.HeadOwner) + headOwner, err = user_model.GetUserOrOrgByName(ctx, compareReq.HeadOwner) if err != nil { if user_model.IsErrUserNotExist(err) { ctx.APIErrorNotFound("GetUserByName") @@ -1099,34 +1105,24 @@ func parseCompareInfo(ctx *context.APIContext, compareParam string) (result *par } } if compareReq.HeadRepoName == "" { - if headUser.ID == baseRepo.OwnerID { + if headOwner.ID == baseRepo.OwnerID { headRepo = baseRepo } else { - // TODO: forked's fork - headRepo = repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID) + headRepo, err = common.FindHeadRepo(ctx, baseRepo, headOwner.ID) + if err != nil { + ctx.APIErrorInternal(err) + return nil, nil + } if headRepo == nil { - // TODO: based's base? - 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 + ctx.HTTPError(http.StatusBadRequest, "The user "+headOwner.Name+" does not have a fork of the base repository") + return nil, nil } } } 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) + headRepo, err = repo_model.GetRepositoryByName(ctx, headOwner.ID, compareReq.HeadRepoName) if err != nil { if repo_model.IsErrRepoNotExist(err) { ctx.APIErrorNotFound("GetRepositoryByName") diff --git a/routers/common/compare.go b/routers/common/compare.go index f2def833e2..f2194095b4 100644 --- a/routers/common/compare.go +++ b/routers/common/compare.go @@ -4,6 +4,7 @@ package common import ( + "context" "strings" repo_model "code.gitea.io/gitea/models/repo" @@ -15,7 +16,7 @@ import ( // CompareInfo represents the collected results from ParseCompareInfo type CompareInfo struct { - HeadUser *user_model.User + HeadOwner *user_model.User HeadRepo *repo_model.Repository HeadGitRepo *git.Repository CompareInfo *pull_service.CompareInfo @@ -122,3 +123,54 @@ func ParseCompareRouterParam(routerParam string) (*CompareRouterReq, error) { DotTimes: dotTimes, }, nil } + +// maxForkTraverseLevel defines the maximum levels to traverse when searching for the head repository. +const maxForkTraverseLevel = 10 + +// FindHeadRepo tries to find the head repository based on the base repository and head user ID. +func FindHeadRepo(ctx context.Context, baseRepo *repo_model.Repository, headUserID int64) (*repo_model.Repository, error) { + if baseRepo.IsFork { + curRepo := baseRepo + for curRepo.OwnerID != headUserID { // We assume the fork deepth is not too deep. + if err := curRepo.GetBaseRepo(ctx); err != nil { + return nil, err + } + if curRepo.BaseRepo == nil { + return findHeadRepoFromRootBase(ctx, curRepo, headUserID, maxForkTraverseLevel) + } + curRepo = curRepo.BaseRepo + } + return curRepo, nil + } + + return findHeadRepoFromRootBase(ctx, baseRepo, headUserID, maxForkTraverseLevel) +} + +func findHeadRepoFromRootBase(ctx context.Context, baseRepo *repo_model.Repository, headUserID int64, traverseLevel int) (*repo_model.Repository, error) { + if traverseLevel == 0 { + return nil, nil + } + // test if we are lucky + repo, err := repo_model.GetUserFork(ctx, baseRepo.ID, headUserID) + if err != nil { + return nil, err + } + if repo != nil { + return repo, nil + } + + firstLevelForkedRepos, err := repo_model.GetRepositoriesByForkID(ctx, baseRepo.ID) + if err != nil { + return nil, err + } + for _, repo := range firstLevelForkedRepos { + forked, err := findHeadRepoFromRootBase(ctx, repo, headUserID, traverseLevel-1) + if err != nil { + return nil, err + } + if forked != nil { + return forked, nil + } + } + return nil, nil +} diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index b325b030a4..c7c4593566 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -202,6 +202,11 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { ctx.ServerError("GetUserByName", err) return nil } + // remove the check when we support compare with carets + if compareReq.CaretTimes > 0 { + ctx.HTTPError(http.StatusBadRequest, "Unsupported compare syntax with carets") + return nil + } if compareReq.HeadOwner == "" { if compareReq.HeadRepoName != "" { // unsupported syntax @@ -209,13 +214,13 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { return nil } - ci.HeadUser = baseRepo.Owner + ci.HeadOwner = baseRepo.Owner ci.HeadRepo = baseRepo } else { if compareReq.HeadOwner == ctx.Repo.Owner.Name { - ci.HeadUser = ctx.Repo.Owner + ci.HeadOwner = ctx.Repo.Owner } else { - ci.HeadUser, err = user_model.GetUserByName(ctx, compareReq.HeadOwner) + ci.HeadOwner, err = user_model.GetUserOrOrgByName(ctx, compareReq.HeadOwner) if err != nil { if user_model.IsErrUserNotExist(err) { ctx.NotFound(nil) @@ -226,34 +231,24 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { } } if compareReq.HeadRepoName == "" { - if ci.HeadUser.ID == baseRepo.OwnerID { + if ci.HeadOwner.ID == baseRepo.OwnerID { ci.HeadRepo = baseRepo } else { - // TODO: forked's fork - ci.HeadRepo = repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, baseRepo.ID) + ci.HeadRepo, err = common.FindHeadRepo(ctx, baseRepo, ci.HeadOwner.ID) + if err != nil { + ctx.ServerError("FindHeadRepo", err) + return nil + } if ci.HeadRepo == nil { - // TODO: based's base? - 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 != ci.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 + ctx.HTTPError(http.StatusBadRequest, "The user "+ci.HeadOwner.Name+" does not have a fork of the base repository") + return nil } } } 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, ci.HeadUser.ID, compareReq.HeadRepoName) + ci.HeadRepo, err = repo_model.GetRepositoryByName(ctx, ci.HeadOwner.ID, compareReq.HeadRepoName) if err != nil { if repo_model.IsErrRepoNotExist(err) { ctx.NotFound(nil) @@ -272,7 +267,7 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { ctx.Data["BaseName"] = baseRepo.OwnerName ctx.Data["BaseBranch"] = ci.BaseBranch - ctx.Data["HeadUser"] = ci.HeadUser + ctx.Data["HeadUser"] = ci.HeadOwner ctx.Data["HeadBranch"] = ci.HeadBranch ctx.Repo.PullRequest.SameRepo = isSameRepo @@ -344,27 +339,27 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { has := ci.HeadRepo != nil // 3. If the base is a forked from "RootRepo" and the owner of // the "RootRepo" is the :headUser - set headRepo to that - if !has && rootRepo != nil && rootRepo.OwnerID == ci.HeadUser.ID { + if !has && rootRepo != nil && rootRepo.OwnerID == ci.HeadOwner.ID { ci.HeadRepo = rootRepo has = true } // 4. If the ctx.Doer has their own fork of the baseRepo and the headUser is the ctx.Doer // set the headRepo to the ownFork - if !has && ownForkRepo != nil && ownForkRepo.OwnerID == ci.HeadUser.ID { + if !has && ownForkRepo != nil && ownForkRepo.OwnerID == ci.HeadOwner.ID { ci.HeadRepo = ownForkRepo has = true } // 5. If the headOwner has a fork of the baseRepo - use that if !has { - ci.HeadRepo = repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, baseRepo.ID) + ci.HeadRepo = repo_model.GetForkedRepo(ctx, ci.HeadOwner.ID, baseRepo.ID) has = ci.HeadRepo != nil } // 6. If the baseRepo is a fork and the headUser has a fork of that use that if !has && baseRepo.IsFork { - ci.HeadRepo = repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, baseRepo.ForkID) + ci.HeadRepo = repo_model.GetForkedRepo(ctx, ci.HeadOwner.ID, baseRepo.ForkID) has = ci.HeadRepo != nil } @@ -679,10 +674,10 @@ func PrepareCompareDiff( } ctx.Data["title"] = title - ctx.Data["Username"] = ci.HeadUser.Name + ctx.Data["Username"] = ci.HeadOwner.Name ctx.Data["Reponame"] = ci.HeadRepo.Name - setCompareContext(ctx, beforeCommit, headCommit, ci.HeadUser.Name, repo.Name) + setCompareContext(ctx, beforeCommit, headCommit, ci.HeadOwner.Name, repo.Name) return false } diff --git a/tests/integration/pull_create_test.go b/tests/integration/pull_create_test.go index ddafdf33b8..dc27f6a781 100644 --- a/tests/integration/pull_create_test.go +++ b/tests/integration/pull_create_test.go @@ -4,6 +4,7 @@ package integration import ( + "encoding/base64" "fmt" "net/http" "net/http/httptest" @@ -17,7 +18,9 @@ import ( repo_model "code.gitea.io/gitea/models/repo" "code.gitea.io/gitea/models/unittest" "code.gitea.io/gitea/modules/git/gitcmd" + api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/tests" "github.com/stretchr/testify/assert" @@ -295,6 +298,95 @@ func TestPullCreatePrFromBaseToFork(t *testing.T) { }) } +func TestCreatePullRequestFromNestedOrgForks(t *testing.T) { + onGiteaRun(t, func(t *testing.T, _ *url.URL) { + session := loginUser(t, "user1") + token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository, auth_model.AccessTokenScopeWriteOrganization) + + const ( + baseOrg = "test-fork-org1" + midForkOrg = "test-fork-org2" + leafForkOrg = "test-fork-org3" + repoName = "test-fork-repo" + patchBranch = "teabot-patch-1" + ) + + createOrg := func(name string) { + req := NewRequestWithJSON(t, "POST", "/api/v1/orgs", &api.CreateOrgOption{ + UserName: name, + Visibility: "public", + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + } + + createOrg(baseOrg) + createOrg(midForkOrg) + createOrg(leafForkOrg) + + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/orgs/%s/repos", baseOrg), &api.CreateRepoOption{ + Name: repoName, + AutoInit: true, + DefaultBranch: "main", + Private: false, + Readme: "Default", + }).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusCreated) + var baseRepo api.Repository + DecodeJSON(t, resp, &baseRepo) + assert.Equal(t, "main", baseRepo.DefaultBranch) + + forkIntoOrg := func(srcOrg, dstOrg string) api.Repository { + req := NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/forks", srcOrg, repoName), &api.CreateForkOption{ + Organization: util.ToPointer(dstOrg), + }).AddTokenAuth(token) + resp := MakeRequest(t, req, http.StatusAccepted) + var forkRepo api.Repository + DecodeJSON(t, resp, &forkRepo) + assert.NotNil(t, forkRepo.Owner) + if forkRepo.Owner != nil { + assert.Equal(t, dstOrg, forkRepo.Owner.UserName) + } + return forkRepo + } + + forkIntoOrg(baseOrg, midForkOrg) + forkIntoOrg(midForkOrg, leafForkOrg) + + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/contents/%s", leafForkOrg, repoName, "patch-from-org3.txt"), &api.CreateFileOptions{ + FileOptions: api.FileOptions{ + BranchName: "main", + NewBranchName: patchBranch, + Message: "create patch from org3", + }, + ContentBase64: base64.StdEncoding.EncodeToString([]byte("patch content")), + }).AddTokenAuth(token) + MakeRequest(t, req, http.StatusCreated) + + prPayload := map[string]string{ + "head": fmt.Sprintf("%s:%s", leafForkOrg, patchBranch), + "base": "main", + "title": "test creating pull from test-fork-org3 to test-fork-org1", + } + req = NewRequestWithJSON(t, "POST", fmt.Sprintf("/api/v1/repos/%s/%s/pulls", baseOrg, repoName), prPayload).AddTokenAuth(token) + resp = MakeRequest(t, req, http.StatusCreated) + var pr api.PullRequest + DecodeJSON(t, resp, &pr) + assert.Equal(t, prPayload["title"], pr.Title) + if assert.NotNil(t, pr.Head) { + assert.Equal(t, patchBranch, pr.Head.Ref) + if assert.NotNil(t, pr.Head.Repository) { + assert.Equal(t, fmt.Sprintf("%s/%s", leafForkOrg, repoName), pr.Head.Repository.FullName) + } + } + if assert.NotNil(t, pr.Base) { + assert.Equal(t, "main", pr.Base.Ref) + if assert.NotNil(t, pr.Base.Repository) { + assert.Equal(t, fmt.Sprintf("%s/%s", baseOrg, repoName), pr.Base.Repository.FullName) + } + } + }) +} + func TestPullCreateParallel(t *testing.T) { onGiteaRun(t, func(t *testing.T, u *url.URL) { sessionFork := loginUser(t, "user1")