From 82ca99db2dd557465c86918af01f52777a69066f Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Wed, 11 Dec 2024 22:15:47 -0800 Subject: [PATCH] Merge two functions with the same content --- models/repo/fork.go | 29 +++++++++-------------------- models/repo/fork_test.go | 4 ++-- routers/api/v1/repo/pull.go | 6 +++++- routers/web/repo/compare.go | 18 +++++++++++++++--- routers/web/repo/fork.go | 6 +++++- services/repository/fork.go | 4 ++-- 6 files changed, 38 insertions(+), 29 deletions(-) diff --git a/models/repo/fork.go b/models/repo/fork.go index 1c75e86458b..73797199628 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, repo.ID, user.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 e8dca204cc4..7a15874329e 100644 --- a/models/repo/fork_test.go +++ b/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) } diff --git a/routers/api/v1/repo/pull.go b/routers/api/v1/repo/pull.go index 86b204f51e2..bad078414e6 100644 --- a/routers/api/v1/repo/pull.go +++ b/routers/api/v1/repo/pull.go @@ -1132,7 +1132,11 @@ func parseCompareInfo(ctx *context.APIContext, form api.CreatePullRequestOption) } // 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, 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 278974bec3e..a500dd6f1e4 100644 --- a/routers/web/repo/compare.go +++ b/routers/web/repo/compare.go @@ -356,7 +356,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 @@ -380,13 +384,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 27e42a8f98e..c0d823c079c 100644 --- a/routers/web/repo/fork.go +++ b/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 diff --git a/services/repository/fork.go b/services/repository/fork.go index bc4fdf85627..26057266c5f 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, opts.BaseRepo.ID, owner.ID) + if err != nil && !repo_model.IsErrRepoNotExist(err) { return nil, err } if forkedRepo != nil {