From cad70599a65894651d91ad230b57857a443a4d26 Mon Sep 17 00:00:00 2001
From: Kyle Evans <kevans91@users.noreply.github.com>
Date: Sat, 28 Aug 2021 03:37:14 -0500
Subject: [PATCH] Refactor the fork service slightly to take ForkRepoOptions
(#16744)
* Refactor the fork service slightly to take ForkRepoOptions
This reduces the number of places we need to change if we want to add other
options during fork time.
Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
* Fix integrations and tests after ForkRepository refactor
Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
* Update OldRepo -> BaseRepo
Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
* gofmt pass
Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
---
integrations/pull_update_test.go | 6 +++++-
models/repo.go | 7 +++++++
modules/repository/fork.go | 32 +++++++++++++++----------------
modules/repository/fork_test.go | 6 +++++-
routers/api/v1/repo/fork.go | 6 +++++-
routers/web/repo/pull.go | 6 +++++-
services/repository/repository.go | 6 +++---
7 files changed, 46 insertions(+), 23 deletions(-)
diff --git a/integrations/pull_update_test.go b/integrations/pull_update_test.go
index 2dc966316e..46a1e04b25 100644
--- a/integrations/pull_update_test.go
+++ b/integrations/pull_update_test.go
@@ -60,7 +60,11 @@ func createOutdatedPR(t *testing.T, actor, forkOrg *models.User) *models.PullReq
assert.NoError(t, err)
assert.NotEmpty(t, baseRepo)
- headRepo, err := repo_module.ForkRepository(actor, forkOrg, baseRepo, "repo-pr-update", "desc")
+ headRepo, err := repo_module.ForkRepository(actor, forkOrg, models.ForkRepoOptions{
+ BaseRepo: baseRepo,
+ Name: "repo-pr-update",
+ Description: "desc",
+ })
assert.NoError(t, err)
assert.NotEmpty(t, headRepo)
diff --git a/models/repo.go b/models/repo.go
index f2e3f5b58b..c3f93f3562 100644
--- a/models/repo.go
+++ b/models/repo.go
@@ -1004,6 +1004,13 @@ type CreateRepoOptions struct {
MirrorInterval string
}
+// ForkRepoOptions contains the fork repository options
+type ForkRepoOptions struct {
+ BaseRepo *Repository
+ Name string
+ Description string
+}
+
// GetRepoInitFile returns repository init files
func GetRepoInitFile(tp, name string) ([]byte, error) {
cleanedName := strings.TrimLeft(path.Clean("/"+name), "/")
diff --git a/modules/repository/fork.go b/modules/repository/fork.go
index f8cb74bcb4..892c1494df 100644
--- a/modules/repository/fork.go
+++ b/modules/repository/fork.go
@@ -16,15 +16,15 @@ import (
)
// ForkRepository forks a repository
-func ForkRepository(doer, owner *models.User, oldRepo *models.Repository, name, desc string) (_ *models.Repository, err error) {
- forkedRepo, err := oldRepo.GetUserFork(owner.ID)
+func ForkRepository(doer, owner *models.User, opts models.ForkRepoOptions) (_ *models.Repository, err error) {
+ forkedRepo, err := opts.BaseRepo.GetUserFork(owner.ID)
if err != nil {
return nil, err
}
if forkedRepo != nil {
return nil, models.ErrForkAlreadyExist{
Uname: owner.Name,
- RepoName: oldRepo.FullName(),
+ RepoName: opts.BaseRepo.FullName(),
ForkName: forkedRepo.FullName(),
}
}
@@ -33,17 +33,17 @@ func ForkRepository(doer, owner *models.User, oldRepo *models.Repository, name,
OwnerID: owner.ID,
Owner: owner,
OwnerName: owner.Name,
- Name: name,
- LowerName: strings.ToLower(name),
- Description: desc,
- DefaultBranch: oldRepo.DefaultBranch,
- IsPrivate: oldRepo.IsPrivate || oldRepo.Owner.Visibility == structs.VisibleTypePrivate,
- IsEmpty: oldRepo.IsEmpty,
+ Name: opts.Name,
+ LowerName: strings.ToLower(opts.Name),
+ Description: opts.Description,
+ DefaultBranch: opts.BaseRepo.DefaultBranch,
+ IsPrivate: opts.BaseRepo.IsPrivate || opts.BaseRepo.Owner.Visibility == structs.VisibleTypePrivate,
+ IsEmpty: opts.BaseRepo.IsEmpty,
IsFork: true,
- ForkID: oldRepo.ID,
+ ForkID: opts.BaseRepo.ID,
}
- oldRepoPath := oldRepo.RepoPath()
+ oldRepoPath := opts.BaseRepo.RepoPath()
err = models.WithTx(func(ctx models.DBContext) error {
if err = models.CreateRepository(ctx, doer, owner, repo, false); err != nil {
@@ -59,13 +59,13 @@ func ForkRepository(doer, owner *models.User, oldRepo *models.Repository, name,
}
}
- if err = models.IncrementRepoForkNum(ctx, oldRepo.ID); err != nil {
+ if err = models.IncrementRepoForkNum(ctx, opts.BaseRepo.ID); err != nil {
rollbackRemoveFn()
return err
}
// copy lfs files failure should not be ignored
- if err := models.CopyLFS(ctx, repo, oldRepo); err != nil {
+ if err := models.CopyLFS(ctx, repo, opts.BaseRepo); err != nil {
rollbackRemoveFn()
return err
}
@@ -73,9 +73,9 @@ func ForkRepository(doer, owner *models.User, oldRepo *models.Repository, name,
repoPath := models.RepoPath(owner.Name, repo.Name)
if stdout, err := git.NewCommand(
"clone", "--bare", oldRepoPath, repoPath).
- SetDescription(fmt.Sprintf("ForkRepository(git clone): %s to %s", oldRepo.FullName(), repo.FullName())).
+ SetDescription(fmt.Sprintf("ForkRepository(git clone): %s to %s", opts.BaseRepo.FullName(), repo.FullName())).
RunInDirTimeout(10*time.Minute, ""); err != nil {
- log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, oldRepo, stdout, err)
+ log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, opts.BaseRepo, stdout, err)
rollbackRemoveFn()
return fmt.Errorf("git clone: %v", err)
}
@@ -103,7 +103,7 @@ func ForkRepository(doer, owner *models.User, oldRepo *models.Repository, name,
if err = repo.UpdateSize(ctx); err != nil {
log.Error("Failed to update size for repository: %v", err)
}
- if err := models.CopyLanguageStat(oldRepo, repo); err != nil {
+ if err := models.CopyLanguageStat(opts.BaseRepo, repo); err != nil {
log.Error("Copy language stat from oldRepo failed")
}
diff --git a/modules/repository/fork_test.go b/modules/repository/fork_test.go
index cb3526bccf..fc4a724660 100644
--- a/modules/repository/fork_test.go
+++ b/modules/repository/fork_test.go
@@ -18,7 +18,11 @@ func TestForkRepository(t *testing.T) {
user := models.AssertExistsAndLoadBean(t, &models.User{ID: 13}).(*models.User)
repo := models.AssertExistsAndLoadBean(t, &models.Repository{ID: 10}).(*models.Repository)
- fork, err := ForkRepository(user, user, repo, "test", "test")
+ fork, err := ForkRepository(user, user, models.ForkRepoOptions{
+ BaseRepo: repo,
+ Name: "test",
+ Description: "test",
+ })
assert.Nil(t, fork)
assert.Error(t, err)
assert.True(t, models.IsErrForkAlreadyExist(err))
diff --git a/routers/api/v1/repo/fork.go b/routers/api/v1/repo/fork.go
index a3f9aa14f9..b3504faad9 100644
--- a/routers/api/v1/repo/fork.go
+++ b/routers/api/v1/repo/fork.go
@@ -123,7 +123,11 @@ func CreateFork(ctx *context.APIContext) {
forker = org
}
- fork, err := repo_service.ForkRepository(ctx.User, forker, repo, repo.Name, repo.Description)
+ fork, err := repo_service.ForkRepository(ctx.User, forker, models.ForkRepoOptions{
+ BaseRepo: repo,
+ Name: repo.Name,
+ Description: repo.Description,
+ })
if err != nil {
ctx.Error(http.StatusInternalServerError, "ForkRepository", err)
return
diff --git a/routers/web/repo/pull.go b/routers/web/repo/pull.go
index 2fa5ab9186..a41c9eb2b4 100644
--- a/routers/web/repo/pull.go
+++ b/routers/web/repo/pull.go
@@ -225,7 +225,11 @@ func ForkPost(ctx *context.Context) {
}
}
- repo, err := repo_service.ForkRepository(ctx.User, ctxUser, forkRepo, form.RepoName, form.Description)
+ repo, err := repo_service.ForkRepository(ctx.User, ctxUser, models.ForkRepoOptions{
+ BaseRepo: forkRepo,
+ Name: form.RepoName,
+ Description: form.Description,
+ })
if err != nil {
ctx.Data["Err_RepoName"] = true
switch {
diff --git a/services/repository/repository.go b/services/repository/repository.go
index 20d630424c..cde4af2afa 100644
--- a/services/repository/repository.go
+++ b/services/repository/repository.go
@@ -47,13 +47,13 @@ func DeleteUnadoptedRepository(doer, owner *models.User, name string) error {
}
// ForkRepository forks a repository
-func ForkRepository(doer, u *models.User, oldRepo *models.Repository, name, desc string) (*models.Repository, error) {
- repo, err := repo_module.ForkRepository(doer, u, oldRepo, name, desc)
+func ForkRepository(doer, u *models.User, opts models.ForkRepoOptions) (*models.Repository, error) {
+ repo, err := repo_module.ForkRepository(doer, u, opts)
if err != nil {
return nil, err
}
- notification.NotifyForkRepository(doer, oldRepo, repo)
+ notification.NotifyForkRepository(doer, opts.BaseRepo, repo)
return repo, nil
}