Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix unclear IsRepositoryExist logic #24374

Merged
merged 4 commits into from
Apr 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions models/repo/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -726,19 +726,23 @@ func GetRepositoriesMapByIDs(ids []int64) (map[int64]*Repository, error) {
return repos, db.GetEngine(db.DefaultContext).In("id", ids).Find(&repos)
}

// IsRepositoryExist returns true if the repository with given name under user has already existed.
func IsRepositoryExist(ctx context.Context, u *user_model.User, repoName string) (bool, error) {
has, err := db.GetEngine(ctx).Get(&Repository{
OwnerID: u.ID,
LowerName: strings.ToLower(repoName),
})
// IsRepositoryModelOrDirExist returns true if the repository with given name under user has already existed.
func IsRepositoryModelOrDirExist(ctx context.Context, u *user_model.User, repoName string) (bool, error) {
has, err := IsRepositoryModelExist(ctx, u, repoName)
if err != nil {
return false, err
}
isDir, err := util.IsDir(RepoPath(u.Name, repoName))
return has || isDir, err
}

func IsRepositoryModelExist(ctx context.Context, u *user_model.User, repoName string) (bool, error) {
return db.GetEngine(ctx).Get(&Repository{
OwnerID: u.ID,
LowerName: strings.ToLower(repoName),
})
}

// GetTemplateRepo populates repo.TemplateRepo for a generated repository and
// returns an error on failure (NOTE: no error is returned for
// non-generated repositories, and TemplateRepo will be left untouched)
Expand Down
4 changes: 2 additions & 2 deletions models/repo/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func CheckCreateRepository(doer, u *user_model.User, name string, overwriteOrAdo
return err
}

has, err := IsRepositoryExist(db.DefaultContext, u, name)
has, err := IsRepositoryModelOrDirExist(db.DefaultContext, u, name)
if err != nil {
return fmt.Errorf("IsRepositoryExist: %w", err)
} else if has {
Expand Down Expand Up @@ -147,7 +147,7 @@ func ChangeRepositoryName(doer *user_model.User, repo *Repository, newRepoName s
return err
}

has, err := IsRepositoryExist(db.DefaultContext, repo.Owner, newRepoName)
has, err := IsRepositoryModelOrDirExist(db.DefaultContext, repo.Owner, newRepoName)
if err != nil {
return fmt.Errorf("IsRepositoryExist: %w", err)
} else if has {
Expand Down
4 changes: 2 additions & 2 deletions models/repo_transfer.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func CreatePendingRepositoryTransfer(ctx context.Context, doer, newOwner *user_m
}

// Check if new owner has repository with same name.
if has, err := repo_model.IsRepositoryExist(ctx, newOwner, repo.Name); err != nil {
if has, err := repo_model.IsRepositoryModelExist(ctx, newOwner, repo.Name); err != nil {
return fmt.Errorf("IsRepositoryExist: %w", err)
} else if has {
return repo_model.ErrRepoAlreadyExist{
Expand Down Expand Up @@ -249,7 +249,7 @@ func TransferOwnership(doer *user_model.User, newOwnerName string, repo *repo_mo
newOwnerName = newOwner.Name // ensure capitalisation matches

// Check if new owner has repository with same name.
if has, err := repo_model.IsRepositoryExist(ctx, newOwner, repo.Name); err != nil {
if has, err := repo_model.IsRepositoryModelOrDirExist(ctx, newOwner, repo.Name); err != nil {
return fmt.Errorf("IsRepositoryExist: %w", err)
} else if has {
return repo_model.ErrRepoAlreadyExist{
Expand Down
2 changes: 1 addition & 1 deletion modules/repository/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func CreateRepositoryByExample(ctx context.Context, doer, u *user_model.User, re
return err
}

has, err := repo_model.IsRepositoryExist(ctx, u, repo.Name)
has, err := repo_model.IsRepositoryModelExist(ctx, u, repo.Name)
if err != nil {
return fmt.Errorf("IsRepositoryExist: %w", err)
} else if has {
Expand Down
4 changes: 2 additions & 2 deletions routers/api/v1/admin/adopt.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func AdoptRepository(ctx *context.APIContext) {
}

// check not a repo
has, err := repo_model.IsRepositoryExist(ctx, ctxUser, repoName)
has, err := repo_model.IsRepositoryModelExist(ctx, ctxUser, repoName)
if err != nil {
ctx.InternalServerError(err)
return
Expand Down Expand Up @@ -157,7 +157,7 @@ func DeleteUnadoptedRepository(ctx *context.APIContext) {
}

// check not a repo
has, err := repo_model.IsRepositoryExist(ctx, ctxUser, repoName)
has, err := repo_model.IsRepositoryModelExist(ctx, ctxUser, repoName)
if err != nil {
ctx.InternalServerError(err)
return
Expand Down
2 changes: 1 addition & 1 deletion routers/web/admin/repos.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ func AdoptOrDeleteRepository(ctx *context.Context) {
repoName := dirSplit[1]

// check not a repo
has, err := repo_model.IsRepositoryExist(ctx, ctxUser, repoName)
has, err := repo_model.IsRepositoryModelExist(ctx, ctxUser, repoName)
if err != nil {
ctx.ServerError("IsRepositoryExist", err)
return
Expand Down
2 changes: 1 addition & 1 deletion routers/web/user/setting/adopt.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ func AdoptOrDeleteRepository(ctx *context.Context) {
root := user_model.UserPath(ctxUser.LowerName)

// check not a repo
has, err := repo_model.IsRepositoryExist(ctx, ctxUser, dir)
has, err := repo_model.IsRepositoryModelExist(ctx, ctxUser, dir)
if err != nil {
ctx.ServerError("IsRepositoryExist", err)
return
Expand Down
2 changes: 1 addition & 1 deletion services/repository/adopt.go
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ func DeleteUnadoptedRepository(ctx context.Context, doer, u *user_model.User, re
}
}

if exist, err := repo_model.IsRepositoryExist(ctx, u, repoName); err != nil {
if exist, err := repo_model.IsRepositoryModelExist(ctx, u, repoName); err != nil {
return err
} else if exist {
return repo_model.ErrRepoAlreadyExist{
Expand Down