Merge pull request 'chore(refactor): split repo_service.ForkRepository in two' (#4879) from earl-warren/forgejo:wip-fork-split into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/4879
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
This commit is contained in:
Earl Warren 2024-08-11 15:50:52 +00:00
commit 44002a6399
9 changed files with 25 additions and 15 deletions

View file

@ -148,16 +148,16 @@ func CreateFork(ctx *context.APIContext) {
name = *form.Name name = *form.Name
} }
fork, err := repo_service.ForkRepository(ctx, ctx.Doer, forker, repo_service.ForkRepoOptions{ fork, err := repo_service.ForkRepositoryAndUpdates(ctx, ctx.Doer, forker, repo_service.ForkRepoOptions{
BaseRepo: repo, BaseRepo: repo,
Name: name, Name: name,
Description: repo.Description, Description: repo.Description,
}) })
if err != nil { if err != nil {
if errors.Is(err, util.ErrAlreadyExist) || repo_model.IsErrReachLimitOfRepo(err) { if errors.Is(err, util.ErrAlreadyExist) || repo_model.IsErrReachLimitOfRepo(err) {
ctx.Error(http.StatusConflict, "ForkRepository", err) ctx.Error(http.StatusConflict, "ForkRepositoryAndUpdates", err)
} else { } else {
ctx.Error(http.StatusInternalServerError, "ForkRepository", err) ctx.Error(http.StatusInternalServerError, "ForkRepositoryAndUpdates", err)
} }
return return
} }

View file

@ -294,7 +294,7 @@ func ForkPost(ctx *context.Context) {
} }
} }
repo, err := repo_service.ForkRepository(ctx, ctx.Doer, ctxUser, repo_service.ForkRepoOptions{ repo, err := repo_service.ForkRepositoryAndUpdates(ctx, ctx.Doer, ctxUser, repo_service.ForkRepoOptions{
BaseRepo: forkRepo, BaseRepo: forkRepo,
Name: form.RepoName, Name: form.RepoName,
Description: form.Description, Description: form.Description,

View file

@ -155,7 +155,7 @@ func (o *project) Put(ctx context.Context) generic.NodeID {
panic(fmt.Errorf("LoadOwner %v %w", o.forgejoProject.BaseRepo, err)) panic(fmt.Errorf("LoadOwner %v %w", o.forgejoProject.BaseRepo, err))
} }
repo, err := repo_service.ForkRepository(ctx, doer, owner, repo_service.ForkRepoOptions{ repo, err := repo_service.ForkRepositoryIfNotExists(ctx, doer, owner, repo_service.ForkRepoOptions{
BaseRepo: o.forgejoProject.BaseRepo, BaseRepo: o.forgejoProject.BaseRepo,
Name: o.forgejoProject.Name, Name: o.forgejoProject.Name,
Description: o.forgejoProject.Description, Description: o.forgejoProject.Description,

View file

@ -51,8 +51,8 @@ type ForkRepoOptions struct {
SingleBranch string SingleBranch string
} }
// ForkRepository forks a repository // ForkRepositoryIfNotExists creates a fork of a repository if it does not already exists and fails otherwise
func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts ForkRepoOptions) (*repo_model.Repository, error) { func ForkRepositoryIfNotExists(ctx context.Context, doer, owner *user_model.User, opts ForkRepoOptions) (*repo_model.Repository, error) {
// Fork is prohibited, if user has reached maximum limit of repositories // Fork is prohibited, if user has reached maximum limit of repositories
if !doer.IsAdmin && !owner.CanForkRepo() { if !doer.IsAdmin && !owner.CanForkRepo() {
return nil, repo_model.ErrReachLimitOfRepo{ return nil, repo_model.ErrReachLimitOfRepo{
@ -147,7 +147,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
} }
repoPath := repo_model.RepoPath(owner.Name, repo.Name) repoPath := repo_model.RepoPath(owner.Name, repo.Name)
if stdout, _, err := cloneCmd.AddDynamicArguments(oldRepoPath, repoPath). if stdout, _, err := cloneCmd.AddDynamicArguments(oldRepoPath, repoPath).
SetDescription(fmt.Sprintf("ForkRepository(git clone): %s to %s", opts.BaseRepo.FullName(), repo.FullName())). SetDescription(fmt.Sprintf("ForkRepositoryIfNotExists(git clone): %s to %s", opts.BaseRepo.FullName(), repo.FullName())).
RunStdBytes(&git.RunOpts{Timeout: 10 * time.Minute}); err != nil { RunStdBytes(&git.RunOpts{Timeout: 10 * time.Minute}); err != nil {
log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, opts.BaseRepo, stdout, err) log.Error("Fork Repository (git clone) Failed for %v (from %v):\nStdout: %s\nError: %v", repo, opts.BaseRepo, stdout, err)
return fmt.Errorf("git clone: %w", err) return fmt.Errorf("git clone: %w", err)
@ -158,7 +158,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
} }
if stdout, _, err := git.NewCommand(txCtx, "update-server-info"). if stdout, _, err := git.NewCommand(txCtx, "update-server-info").
SetDescription(fmt.Sprintf("ForkRepository(git update-server-info): %s", repo.FullName())). SetDescription(fmt.Sprintf("ForkRepositoryIfNotExists(git update-server-info): %s", repo.FullName())).
RunStdString(&git.RunOpts{Dir: repoPath}); err != nil { RunStdString(&git.RunOpts{Dir: repoPath}); err != nil {
log.Error("Fork Repository (git update-server-info) failed for %v:\nStdout: %s\nError: %v", repo, stdout, err) log.Error("Fork Repository (git update-server-info) failed for %v:\nStdout: %s\nError: %v", repo, stdout, err)
return fmt.Errorf("git update-server-info: %w", err) return fmt.Errorf("git update-server-info: %w", err)
@ -183,6 +183,16 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
return nil, err return nil, err
} }
return repo, nil
}
// ForkRepositoryAndUpdates forks a repository. On success it updates metadata (size, stats, etc.) and send a notification.
func ForkRepositoryAndUpdates(ctx context.Context, doer, owner *user_model.User, opts ForkRepoOptions) (*repo_model.Repository, error) {
repo, err := ForkRepositoryIfNotExists(ctx, doer, owner, opts)
if err != nil {
return nil, err
}
// even if below operations failed, it could be ignored. And they will be retried // even if below operations failed, it could be ignored. And they will be retried
if err := repo_module.UpdateRepoSize(ctx, repo); err != nil { if err := repo_module.UpdateRepoSize(ctx, repo); err != nil {
log.Error("Failed to update size for repository: %v", err) log.Error("Failed to update size for repository: %v", err)

View file

@ -23,7 +23,7 @@ func TestForkRepository(t *testing.T) {
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 13}) user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 13})
repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10}) repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 10})
fork, err := ForkRepository(git.DefaultContext, user, user, ForkRepoOptions{ fork, err := ForkRepositoryAndUpdates(git.DefaultContext, user, user, ForkRepoOptions{
BaseRepo: repo, BaseRepo: repo,
Name: "test", Name: "test",
Description: "test", Description: "test",
@ -39,7 +39,7 @@ func TestForkRepository(t *testing.T) {
setting.Repository.AllowForkWithoutMaximumLimit = false setting.Repository.AllowForkWithoutMaximumLimit = false
// user has reached maximum limit of repositories // user has reached maximum limit of repositories
user.MaxRepoCreation = 0 user.MaxRepoCreation = 0
fork2, err := ForkRepository(git.DefaultContext, user, user, ForkRepoOptions{ fork2, err := ForkRepositoryAndUpdates(git.DefaultContext, user, user, ForkRepoOptions{
BaseRepo: repo, BaseRepo: repo,
Name: "test", Name: "test",
Description: "test", Description: "test",

View file

@ -46,7 +46,7 @@ func TestPullRequestTargetEvent(t *testing.T) {
defer f() defer f()
// create the forked repo // create the forked repo
forkedRepo, err := repo_service.ForkRepository(git.DefaultContext, user2, org3, repo_service.ForkRepoOptions{ forkedRepo, err := repo_service.ForkRepositoryAndUpdates(git.DefaultContext, user2, org3, repo_service.ForkRepoOptions{
BaseRepo: baseRepo, BaseRepo: baseRepo,
Name: "forked-repo-pull-request-target", Name: "forked-repo-pull-request-target",
Description: "test pull-request-target event", Description: "test pull-request-target event",

View file

@ -70,7 +70,7 @@ func TestPullrequestReopen(t *testing.T) {
require.NoError(t, err) require.NoError(t, err)
// Create an head repository. // Create an head repository.
headRepo, err := repo_service.ForkRepository(git.DefaultContext, user2, org26, repo_service.ForkRepoOptions{ headRepo, err := repo_service.ForkRepositoryAndUpdates(git.DefaultContext, user2, org26, repo_service.ForkRepoOptions{
BaseRepo: baseRepo, BaseRepo: baseRepo,
Name: "reopen-head", Name: "reopen-head",
}) })

View file

@ -376,7 +376,7 @@ func TestPullView_CodeOwner(t *testing.T) {
t.Run("Forked Repo Pull Request", func(t *testing.T) { t.Run("Forked Repo Pull Request", func(t *testing.T) {
user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5}) user5 := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 5})
forkedRepo, err := repo_service.ForkRepository(db.DefaultContext, user2, user5, repo_service.ForkRepoOptions{ forkedRepo, err := repo_service.ForkRepositoryAndUpdates(db.DefaultContext, user2, user5, repo_service.ForkRepoOptions{
BaseRepo: repo, BaseRepo: repo,
Name: "test_codeowner_fork", Name: "test_codeowner_fork",
}) })

View file

@ -85,7 +85,7 @@ func TestAPIPullUpdateByRebase(t *testing.T) {
func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_model.PullRequest { func createOutdatedPR(t *testing.T, actor, forkOrg *user_model.User) *issues_model.PullRequest {
baseRepo, _, _ := CreateDeclarativeRepo(t, actor, "repo-pr-update", nil, nil, nil) baseRepo, _, _ := CreateDeclarativeRepo(t, actor, "repo-pr-update", nil, nil, nil)
headRepo, err := repo_service.ForkRepository(git.DefaultContext, actor, forkOrg, repo_service.ForkRepoOptions{ headRepo, err := repo_service.ForkRepositoryAndUpdates(git.DefaultContext, actor, forkOrg, repo_service.ForkRepoOptions{
BaseRepo: baseRepo, BaseRepo: baseRepo,
Name: "repo-pr-update", Name: "repo-pr-update",
Description: "desc", Description: "desc",