diff --git a/models/repo/fork.go b/models/repo/fork.go index 1c75e86458..b13b7b02ef 100644 --- a/models/repo/fork.go +++ b/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 { - return repo + if err != nil { + 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) - if err != nil { + forkedRepo, err := GetForkedRepo(ctx, user.ID, repo.ID) + if err != nil && !IsErrRepoNotExist(err) { return repoList, err } if forkedRepo != nil { diff --git a/models/repo/fork_test.go b/models/repo/fork_test.go index e8dca204cc..4004c977a2 100644 --- a/models/repo/fork_test.go +++ b/models/repo/fork_test.go @@ -13,21 +13,22 @@ import ( "github.com/stretchr/testify/assert" ) -func TestGetUserFork(t *testing.T) { +func Test_GetForkedRepo(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) // User13 has repo 11 forked from repo10 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, 13, repo.ID) 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) - assert.NoError(t, err) + repo, err = repo_model.GetForkedRepo(db.DefaultContext, 13, repo.ID) + assert.Error(t, err) + assert.True(t, repo_model.IsErrRepoNotExist(err)) assert.Nil(t, repo) } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index d0c3459b63..d212f7ff3e 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1113,7 +1113,11 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) 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) + headRepo, err := repo_model.GetForkedRepo(ctx, headUser.ID, baseRepo.ID) + if err != nil && !repo_model.IsErrRepoNotExist(err) { + ctx.Error(http.StatusInternalServerError, "GetForkedRepo", err) + return nil, nil + } if headRepo == nil && !isSameRepo { err = baseRepo.GetBaseRepo(ctx) if err != nil { diff --git a/routers/web/repo/compare.go b/routers/web/repo/compare.go index b3c1eb7cb0..b037c438e9 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -357,7 +357,11 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { // "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) + repo, err := repo_model.GetForkedRepo(ctx, ctx.Doer.ID, baseRepo.ID) + if err != nil && !repo_model.IsErrRepoNotExist(err) { + ctx.ServerError("GetForkedRepo", err) + return nil + } if repo != nil { ownForkRepo = repo ctx.Data["OwnForkRepo"] = ownForkRepo @@ -381,13 +385,21 @@ func ParseCompareInfo(ctx *context.Context) *common.CompareInfo { // 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, err = repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, baseRepo.ID) + if err != nil && !repo_model.IsErrRepoNotExist(err) { + ctx.ServerError("GetForkedRepo", err) + return nil + } 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, err = repo_model.GetForkedRepo(ctx, ci.HeadUser.ID, baseRepo.ForkID) + if err != nil && !repo_model.IsErrRepoNotExist(err) { + ctx.ServerError("GetForkedRepo", err) + return nil + } has = ci.HeadRepo != nil } diff --git a/routers/web/repo/fork.go b/routers/web/repo/fork.go index 786b5d7e43..e4f8ea517a 100644 --- a/routers/web/repo/fork.go +++ b/routers/web/repo/fork.go @@ -167,7 +167,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 diff --git a/services/repository/fork.go b/services/repository/fork.go index cff0b1a403..53fe036628 100644 --- a/services/repository/fork.go +++ b/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) - if err != nil { + forkedRepo, err := repo_model.GetForkedRepo(ctx, owner.ID, opts.BaseRepo.ID) + if err != nil && !repo_model.IsErrRepoNotExist(err) { return nil, err } if forkedRepo != nil {