Refactor all compare routers

models/repo/fork.go

@@ -21,15 +21,17 @@ func GetRepositoriesByForkID(ctx context.Context, forkID int64) ([]*Repository,
 }
 
 // GetForkedRepo checks if given user has already forked a repository with given ID.
-func GetForkedRepo(ctx context.Context, ownerID, repoID int64) *Repository {
+func GetForkedRepo(ctx context.Context, ownerID, repoID int64) (*Repository, error) {
 	repo := new(Repository)
-	has, _ := db.GetEngine(ctx).
+	has, err := db.GetEngine(ctx).
 		Where("owner_id=? AND fork_id=?", ownerID, repoID).
 		Get(repo)
-	if has {
+	if err != nil {
-		return repo
+		return nil, err
+	} else if !has {
+		return nil, ErrRepoNotExist{ID: repoID}
 	}
-	return nil
+	return repo, nil
 }
 
 // HasForkedRepo checks if given user has already forked a repository with given ID.
@@ -41,19 +43,6 @@ func HasForkedRepo(ctx context.Context, ownerID, repoID int64) bool {
 	return has
 }
 
-// GetUserFork return user forked repository from this repository, if not forked return nil
-func GetUserFork(ctx context.Context, repoID, userID int64) (*Repository, error) {
-	var forkedRepo Repository
-	has, err := db.GetEngine(ctx).Where("fork_id = ?", repoID).And("owner_id = ?", userID).Get(&forkedRepo)
-	if err != nil {
-		return nil, err
-	}
-	if !has {
-		return nil, nil
-	}
-	return &forkedRepo, nil
-}
-
 // IncrementRepoForkNum increment repository fork number
 func IncrementRepoForkNum(ctx context.Context, repoID int64) error {
 	_, err := db.GetEngine(ctx).Exec("UPDATE `repository` SET num_forks=num_forks+1 WHERE id=?", repoID)
@@ -87,8 +76,8 @@ func GetForksByUserAndOrgs(ctx context.Context, user *user_model.User, repo *Rep
 	if user == nil {
 		return repoList, nil
 	}
-	forkedRepo, err := GetUserFork(ctx, repo.ID, user.ID)
+	forkedRepo, err := GetForkedRepo(ctx, repo.ID, user.ID)
-	if err != nil {
+	if err != nil && !IsErrRepoNotExist(err) {
 		return repoList, err
 	}
 	if forkedRepo != nil {

models/repo/fork_test.go

@@ -20,14 +20,14 @@ func TestGetUserFork(t *testing.T) {
 	repo, err := repo_model.GetRepositoryByID(db.DefaultContext, 10)
 	assert.NoError(t, err)
 	assert.NotNil(t, repo)
-	repo, err = repo_model.GetUserFork(db.DefaultContext, repo.ID, 13)
+	repo, err = repo_model.GetForkedRepo(db.DefaultContext, repo.ID, 13)
 	assert.NoError(t, err)
 	assert.NotNil(t, repo)
 
 	repo, err = repo_model.GetRepositoryByID(db.DefaultContext, 9)
 	assert.NoError(t, err)
 	assert.NotNil(t, repo)
-	repo, err = repo_model.GetUserFork(db.DefaultContext, repo.ID, 13)
+	repo, err = repo_model.GetForkedRepo(db.DefaultContext, repo.ID, 13)
 	assert.NoError(t, err)
 	assert.Nil(t, repo)
 }

routers/api/v1/repo/compare.go

@@ -105,25 +105,6 @@ func CompareDiff(ctx *context.APIContext) {
 	ctx.Repo.PullRequest.SameRepo = ci.IsSameRepo()
 	log.Trace("Repo path: %q, base branch: %q, head branch: %q", ctx.Repo.GitRepo.Path, ci.BaseOriRef, ci.HeadOriRef)
 
-	// Check if current user has fork of repository or in the same repository.
-	/*headRepo := repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, ctx.Repo.Repository.ID)
-	if headRepo == nil && !ci.IsSameRepo() {
-		err := ctx.Repo.Repository.GetBaseRepo(ctx)
-		if err != nil {
-			ctx.Error(http.StatusInternalServerError, "GetBaseRepo", err)
-			return nil, 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.NotFound("GetBaseRepo")
-			return nil, nil, nil, "", ""
-		}
-		// Assign headRepo so it can be used below.
-		headRepo = baseRepo.BaseRepo
-	}*/
-
 	ci.CompareInfo, err = ci.HeadGitRepo.GetCompareInfo(repo_model.RepoPath(baseRepo.Owner.Name, baseRepo.Name), ci.BaseOriRef, ci.HeadOriRef, false, false)
 	if err != nil {
 		ctx.Error(http.StatusInternalServerError, "GetCompareInfo", err)

routers/common/compare.go

@@ -53,7 +53,7 @@ func parseHead(head string) (string, string, string) {
 	}
 	ownerRepo := strings.SplitN(paths[0], "/", 2)
 	if len(ownerRepo) == 1 {
-		return "", paths[0], paths[1]
+		return paths[0], "", paths[1]
 	}
 	return ownerRepo[0], ownerRepo[1], paths[1]
 }
@@ -73,6 +73,7 @@ func parseCompareRouter(router string) (*CompareRouter, error) {
 				HeadOriRef:    headRef,
 				HeadOwnerName: headOwnerName,
 				HeadRepoName:  headRepoName,
+				DotTimes:      dotTimes,
 			}, nil
 		} else if len(parts) > 2 {
 			return nil, util.NewSilentWrapErrorf(util.ErrInvalidArgument, "invalid compare router: %s", router)
@@ -120,10 +121,10 @@ func (ci *CompareInfo) Close() {
 
 func detectFullRef(ctx context.Context, repoID int64, gitRepo *git.Repository, oriRef string) (git.RefName, bool, error) {
 	b, err := git_model.GetBranch(ctx, repoID, oriRef)
-	if err != nil {
+	if err != nil && !git_model.IsErrBranchNotExist(err) {
 		return "", false, err
 	}
-	if !b.IsDeleted {
+	if b != nil && !b.IsDeleted {
 		return git.RefNameFromBranch(oriRef), false, nil
 	}
 
@@ -142,6 +143,78 @@ func detectFullRef(ctx context.Context, repoID int64, gitRepo *git.Repository, o
 	return git.RefName(commitObjectID.String()), true, nil
 }
 
+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, 3)
+			}
+			curRepo = curRepo.BaseRepo
+		}
+		return curRepo, nil
+	}
+
+	return findHeadRepoFromRootBase(ctx, baseRepo, headUserID, 3)
+}
+
+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.GetForkedRepo(ctx, headUserID, baseRepo.ID)
+	if err == nil {
+		return repo, nil
+	}
+	if !repo_model.IsErrRepoNotExist(err) {
+		return nil, err
+	}
+
+	firstLevelForkedRepo, err := repo_model.GetRepositoriesByForkID(ctx, baseRepo.ID)
+	if err != nil {
+		return nil, err
+	}
+	for _, repo := range firstLevelForkedRepo {
+		forked, err := findHeadRepoFromRootBase(ctx, repo, headUserID, traverseLevel-1)
+		if err != nil {
+			return nil, err
+		}
+		if forked != nil {
+			return forked, nil
+		}
+	}
+	return nil, nil
+}
+
+// ParseComparePathParams Get compare 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 branch>...[<head repo>:]<head branch>
+// base<-head: master...head:feature
+// same repo: master...feature
 func ParseComparePathParams(ctx context.Context, pathParam string, baseRepo *repo_model.Repository, baseGitRepo *git.Repository) (*CompareInfo, error) {
 	ci := &CompareInfo{}
 	var err error
@@ -159,12 +232,18 @@ func ParseComparePathParams(ctx context.Context, pathParam string, baseRepo *rep
 	}
 
 	if ci.IsSameRepo() {
+		ci.HeadOwnerName = baseRepo.Owner.Name
+		ci.HeadRepoName = baseRepo.Name
 		ci.HeadUser = baseRepo.Owner
 		ci.HeadRepo = baseRepo
 		ci.HeadGitRepo = baseGitRepo
 	} else {
 		if ci.HeadOwnerName == baseRepo.Owner.Name {
 			ci.HeadUser = baseRepo.Owner
+			if ci.HeadRepoName == "" {
+				ci.HeadRepoName = baseRepo.Name
+				ci.HeadRepo = baseRepo
+			}
 		} else {
 			ci.HeadUser, err = user_model.GetUserByName(ctx, ci.HeadOwnerName)
 			if err != nil {
@@ -172,9 +251,15 @@ func ParseComparePathParams(ctx context.Context, pathParam string, baseRepo *rep
 			}
 		}
 
-		ci.HeadRepo, err = repo_model.GetRepositoryByOwnerAndName(ctx, ci.HeadOwnerName, ci.HeadRepoName)
+		if ci.HeadRepo == nil {
-		if err != nil {
+			if ci.HeadRepoName != "" {
-			return nil, err
+				ci.HeadRepo, err = repo_model.GetRepositoryByOwnerAndName(ctx, ci.HeadOwnerName, ci.HeadRepoName)
+			} else {
+				ci.HeadRepo, err = findHeadRepo(ctx, baseRepo, ci.HeadUser.ID)
+			}
+			if err != nil {
+				return nil, err
+			}
 		}
 		ci.HeadRepo.Owner = ci.HeadUser
 		ci.HeadGitRepo, err = gitrepo.OpenRepository(ctx, ci.HeadRepo)

routers/common/compare_test.go

@@ -52,6 +52,7 @@ func TestCompareRouters(t *testing.T) {
 			router: "develop",
 			compareRouter: &CompareRouter{
 				HeadOriRef: "develop",
+				DotTimes:   3,
 			},
 		},
 		{
@@ -60,6 +61,7 @@ func TestCompareRouters(t *testing.T) {
 				HeadOwnerName: "lunny",
 				HeadRepoName:  "forked_repo",
 				HeadOriRef:    "develop",
+				DotTimes:      3,
 			},
 		},
 		{
@@ -101,6 +103,22 @@ func TestCompareRouters(t *testing.T) {
 				DotTimes:   3,
 			},
 		},
+		{
+			router: "teabot-patch-1...v0.0.1",
+			compareRouter: &CompareRouter{
+				BaseOriRef: "teabot-patch-1",
+				HeadOriRef: "v0.0.1",
+				DotTimes:   3,
+			},
+		},
+		{
+			router: "teabot:feature1",
+			compareRouter: &CompareRouter{
+				HeadOwnerName: "teabot",
+				HeadOriRef:    "feature1",
+				DotTimes:      3,
+			},
+		},
 		{
 			router: "8eb19a5ae19abae15c0666d4ab98906139a7f439...283c030497b455ecfa759d4649f9f8b45158742e",
 			compareRouter: &CompareRouter{

routers/web/repo/compare.go

@@ -189,32 +189,6 @@ func setCsvCompareContext(ctx *context.Context) {
 // ParseCompareInfo parse compare info between two commit for preparing comparing references
 // Permission check for base repository's code read should be checked before invoking this function
 func ParseCompareInfo(ctx *context.Context) *common.CompareInfo {
-	// 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 branch>...[<head repo>:]<head branch>
-	// base<-head: master...head:feature
-	// same repo: master...feature
-
 	fileOnly := ctx.FormBool("file-only")
 	pathParam := ctx.PathParam("*")
 	baseRepo := ctx.Repo.Repository
@@ -250,90 +224,6 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo {
 		return nil
 	}
 
-	ctx.Data["PageIsComparePull"] = ci.IsPull() && ctx.Repo.CanReadIssuesOrPulls(true)
-	ctx.Data["BaseName"] = baseRepo.OwnerName
-	ctx.Data["BaseBranch"] = ci.BaseOriRef
-	ctx.Data["HeadUser"] = ci.HeadUser
-	ctx.Data["HeadBranch"] = ci.HeadOriRef
-	ctx.Repo.PullRequest.SameRepo = ci.IsSameRepo()
-
-	ctx.Data["BaseIsCommit"] = ci.IsBaseCommit
-	ctx.Data["BaseIsBranch"] = ci.BaseFullRef.IsBranch()
-	ctx.Data["BaseIsTag"] = ci.BaseFullRef.IsTag()
-	ctx.Data["IsPull"] = true
-
-	// Now we have the repository that represents the base
-
-	// The current base and head repositories and branches may not
-	// actually be the intended branches that the user wants to
-	// create a pull-request from - but also determining the head
-	// repo is difficult.
-
-	// We will want therefore to offer a few repositories to set as
-	// our base and head
-
-	// 1. First if the baseRepo is a fork get the "RootRepo" it was
-	// forked from
-	var rootRepo *repo_model.Repository
-	if baseRepo.IsFork {
-		err = baseRepo.GetBaseRepo(ctx)
-		if err != nil {
-			if !repo_model.IsErrRepoNotExist(err) {
-				ctx.ServerError("Unable to find root repo", err)
-				return nil
-			}
-		} else {
-			rootRepo = baseRepo.BaseRepo
-		}
-	}
-
-	// 2. Now if the current user is not the owner of the baseRepo,
-	// check if they have a fork of the base repo and offer that as
-	// "OwnForkRepo"
-	var ownForkRepo *repo_model.Repository
-	if ctx.Doer != nil && baseRepo.OwnerID != ctx.Doer.ID {
-		repo := repo_model.GetForkedRepo(ctx, ctx.Doer.ID, baseRepo.ID)
-		if repo != nil {
-			ownForkRepo = repo
-			ctx.Data["OwnForkRepo"] = ownForkRepo
-		}
-	}
-
-	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 {
-		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 {
-		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)
-		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)
-		has = ci.HeadRepo != nil
-	}
-
-	// 7. Otherwise if we're not the same repo and haven't found a repo give up
-	if !ci.IsSameRepo() && !has {
-		ctx.Data["PageIsComparePull"] = false
-	}
-
-	ctx.Data["HeadRepo"] = ci.HeadRepo
-	ctx.Data["BaseCompareRepo"] = ctx.Repo.Repository
-
 	// If we're not merging from the same repo:
 	if !ci.IsSameRepo() {
 		// Assert ctx.Doer has permission to read headRepo's codes
@@ -355,53 +245,23 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo {
 		ctx.Data["CanWriteToHeadRepo"] = permHead.CanWrite(unit.TypeCode)
 	}
 
-	// If we have a rootRepo and it's different from:
+	// TODO: prepareRepos, branches and tags for dropdowns
-	// 1. the computed base
-	// 2. the computed head
-	// then get the branches of it
-	if rootRepo != nil &&
-		rootRepo.ID != ci.HeadRepo.ID &&
-		rootRepo.ID != baseRepo.ID {
-		canRead := access_model.CheckRepoUnitUser(ctx, rootRepo, ctx.Doer, unit.TypeCode)
-		if canRead {
-			ctx.Data["RootRepo"] = rootRepo
-			if !fileOnly {
-				branches, tags, err := getBranchesAndTagsForRepo(ctx, rootRepo)
-				if err != nil {
-					ctx.ServerError("GetBranchesForRepo", err)
-					return nil
-				}
 
-				ctx.Data["RootRepoBranches"] = branches
+	ctx.Data["PageIsComparePull"] = ci.IsPull() && ctx.Repo.CanReadIssuesOrPulls(true)
-				ctx.Data["RootRepoTags"] = tags
+	ctx.Data["BaseName"] = baseRepo.OwnerName
-			}
+	ctx.Data["BaseBranch"] = ci.BaseOriRef
-		}
+	ctx.Data["HeadUser"] = ci.HeadUser
-	}
+	ctx.Data["HeadBranch"] = ci.HeadOriRef
+	ctx.Repo.PullRequest.SameRepo = ci.IsSameRepo()
 
-	// If we have a ownForkRepo and it's different from:
+	ctx.Data["BaseIsCommit"] = ci.IsBaseCommit
-	// 1. The computed base
+	ctx.Data["BaseIsBranch"] = ci.BaseFullRef.IsBranch()
-	// 2. The computed head
+	ctx.Data["BaseIsTag"] = ci.BaseFullRef.IsTag()
-	// 3. The rootRepo (if we have one)
+	ctx.Data["IsPull"] = true
-	// then get the branches from it.
+	// ctx.Data["OwnForkRepo"] = ownForkRepo FIXME: This is not used
-	if ownForkRepo != nil &&
-		ownForkRepo.ID != ci.HeadRepo.ID &&
-		ownForkRepo.ID != baseRepo.ID &&
-		(rootRepo == nil || ownForkRepo.ID != rootRepo.ID) {
-		canRead := access_model.CheckRepoUnitUser(ctx, ownForkRepo, ctx.Doer, unit.TypeCode)
-		if canRead {
-			ctx.Data["OwnForkRepo"] = ownForkRepo
-			if !fileOnly {
-				branches, tags, err := getBranchesAndTagsForRepo(ctx, ownForkRepo)
-				if err != nil {
-					ctx.ServerError("GetBranchesForRepo", err)
-					return nil
-				}
-				ctx.Data["OwnForkRepoBranches"] = branches
-				ctx.Data["OwnForkRepoTags"] = tags
-			}
-		}
-	}
 
+	ctx.Data["HeadRepo"] = ci.HeadRepo
+	ctx.Data["BaseCompareRepo"] = ctx.Repo.Repository
 	ctx.Data["HeadIsCommit"] = ci.IsHeadCommit
 	ctx.Data["HeadIsBranch"] = ci.HeadFullRef.IsBranch()
 	ctx.Data["HeadIsTag"] = ci.HeadFullRef.IsTag()

routers/web/repo/fork.go

@@ -166,7 +166,11 @@ func ForkPost(ctx *context.Context) {
 			ctx.RenderWithErr(ctx.Tr("repo.settings.new_owner_has_same_repo"), tplFork, &form)
 			return
 		}
-		repo := repo_model.GetForkedRepo(ctx, ctxUser.ID, traverseParentRepo.ID)
+		repo, err := repo_model.GetForkedRepo(ctx, ctxUser.ID, traverseParentRepo.ID)
+		if err != nil && !repo_model.IsErrRepoNotExist(err) {
+			ctx.ServerError("GetForkedRepo", err)
+			return
+		}
 		if repo != nil {
 			ctx.Redirect(ctxUser.HomeLink() + "/" + url.PathEscape(repo.Name))
 			return

services/repository/fork.go

@@ -71,8 +71,8 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
 		}
 	}
 
-	forkedRepo, err := repo_model.GetUserFork(ctx, opts.BaseRepo.ID, owner.ID)
+	forkedRepo, err := repo_model.GetForkedRepo(ctx, opts.BaseRepo.ID, owner.ID)
-	if err != nil {
+	if err != nil && !repo_model.IsErrRepoNotExist(err) {
 		return nil, err
 	}
 	if forkedRepo != nil {