From 7ee15fdf03e19ad34dd7354260e9382a450f73ab Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Thu, 29 May 2025 23:18:47 -0700 Subject: [PATCH 1/5] replace update repository function in some places --- models/issues/pull.go | 6 --- models/issues/pull_test.go | 13 ----- models/repo/release.go | 2 +- models/repo/repo.go | 8 +++ models/repo/update.go | 6 +++ services/repository/adopt.go | 5 +- services/repository/create.go | 2 +- services/repository/fork.go | 11 ++-- services/repository/generate.go | 2 +- services/repository/migrate.go | 2 +- services/repository/repository.go | 83 +++++++++++++++++++++++++++++-- 11 files changed, 104 insertions(+), 36 deletions(-) diff --git a/models/issues/pull.go b/models/issues/pull.go index e65b214dabfb3..0ff32e247337a 100644 --- a/models/issues/pull.go +++ b/models/issues/pull.go @@ -649,12 +649,6 @@ func GetAllUnmergedAgitPullRequestByPoster(ctx context.Context, uid int64) ([]*P return pulls, err } -// Update updates all fields of pull request. -func (pr *PullRequest) Update(ctx context.Context) error { - _, err := db.GetEngine(ctx).ID(pr.ID).AllCols().Update(pr) - return err -} - // UpdateCols updates specific fields of pull request. func (pr *PullRequest) UpdateCols(ctx context.Context, cols ...string) error { _, err := db.GetEngine(ctx).ID(pr.ID).Cols(cols...).Update(pr) diff --git a/models/issues/pull_test.go b/models/issues/pull_test.go index 53898cb42eb39..39efaa5792ffe 100644 --- a/models/issues/pull_test.go +++ b/models/issues/pull_test.go @@ -248,19 +248,6 @@ func TestGetPullRequestByIssueID(t *testing.T) { assert.True(t, issues_model.IsErrPullRequestNotExist(err)) } -func TestPullRequest_Update(t *testing.T) { - assert.NoError(t, unittest.PrepareTestDatabase()) - pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1}) - pr.BaseBranch = "baseBranch" - pr.HeadBranch = "headBranch" - pr.Update(db.DefaultContext) - - pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID}) - assert.Equal(t, "baseBranch", pr.BaseBranch) - assert.Equal(t, "headBranch", pr.HeadBranch) - unittest.CheckConsistencyFor(t, pr) -} - func TestPullRequest_UpdateCols(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) pr := &issues_model.PullRequest{ diff --git a/models/repo/release.go b/models/repo/release.go index 06cfa3734288c..59f4caf5aa9e0 100644 --- a/models/repo/release.go +++ b/models/repo/release.go @@ -180,7 +180,7 @@ func AddReleaseAttachments(ctx context.Context, releaseID int64, attachmentUUIDs } attachments[i].ReleaseID = releaseID // No assign value could be 0, so ignore AllCols(). - if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil { + if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Cols("release_id").Update(attachments[i]); err != nil { return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err) } } diff --git a/models/repo/repo.go b/models/repo/repo.go index 5aae02c6d8059..1a1ede2456d21 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -992,3 +992,11 @@ func UpdateRepositoryOwnerName(ctx context.Context, oldUserName, newUserName str } return nil } + +func UpdateRepositoryForksNum(ctx context.Context, repoID int64) error { + // Update the forks count of the repository + if _, err := db.GetEngine(ctx).Exec("UPDATE `repository` SET num_forks = (SELECT COUNT(*) FROM `repository` WHERE fork_id = ?) WHERE id = ?", repoID, repoID); err != nil { + return fmt.Errorf("update repository forks num: %w", err) + } + return nil +} diff --git a/models/repo/update.go b/models/repo/update.go index 8a15477a80235..f82ff7c76c222 100644 --- a/models/repo/update.go +++ b/models/repo/update.go @@ -42,12 +42,18 @@ func UpdateRepositoryUpdatedTime(ctx context.Context, repoID int64, updateTime t // UpdateRepositoryColsWithAutoTime updates repository's columns func UpdateRepositoryColsWithAutoTime(ctx context.Context, repo *Repository, cols ...string) error { + if len(cols) == 0 { + return nil + } _, err := db.GetEngine(ctx).ID(repo.ID).Cols(cols...).Update(repo) return err } // UpdateRepositoryColsNoAutoTime updates repository's columns and but applies time change automatically func UpdateRepositoryColsNoAutoTime(ctx context.Context, repo *Repository, cols ...string) error { + if len(cols) == 0 { + return nil + } _, err := db.GetEngine(ctx).ID(repo.ID).Cols(cols...).NoAutoTime().Update(repo) return err } diff --git a/services/repository/adopt.go b/services/repository/adopt.go index 97ba22ace50d2..68a8754279193 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -196,8 +196,9 @@ func adoptRepository(ctx context.Context, repo *repo_model.Repository, defaultBr return fmt.Errorf("setDefaultBranch: %w", err) } } - if err = updateRepository(ctx, repo, false); err != nil { - return fmt.Errorf("updateRepository: %w", err) + + if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_empty", "default_branch"); err != nil { + return fmt.Errorf("UpdateRepositoryCols: %w", err) } return nil diff --git a/services/repository/create.go b/services/repository/create.go index 83d7d84c08d44..f90f2c83f628a 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -191,7 +191,7 @@ func initRepository(ctx context.Context, u *user_model.User, repo *repo_model.Re } } - if err = UpdateRepository(ctx, repo, false); err != nil { + if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_empty", "default_branch", "default_wiki_branch"); err != nil { return fmt.Errorf("updateRepository: %w", err) } diff --git a/services/repository/fork.go b/services/repository/fork.go index bd1554f163e3d..244f72167d38d 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -209,7 +209,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork // ConvertForkToNormalRepository convert the provided repo from a forked repo to normal repo func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Repository) error { - err := db.WithTx(ctx, func(ctx context.Context) error { + return db.WithTx(ctx, func(ctx context.Context) error { repo, err := repo_model.GetRepositoryByID(ctx, repo.ID) if err != nil { return err @@ -224,18 +224,15 @@ func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Reposit return err } + parentRepoID := repo.ForkID repo.IsFork = false repo.ForkID = 0 - - if err := updateRepository(ctx, repo, false); err != nil { - log.Error("Unable to update repository %-v whilst converting from fork. Error: %v", repo, err) + if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_fork", "fork_id"); err != nil { return err } - return nil + return repo_model.UpdateRepositoryForksNum(ctx, parentRepoID) }) - - return err } type findForksOptions struct { diff --git a/services/repository/generate.go b/services/repository/generate.go index b02f7c9482076..658299ac84362 100644 --- a/services/repository/generate.go +++ b/services/repository/generate.go @@ -279,7 +279,7 @@ func generateGitContent(ctx context.Context, repo, templateRepo, generateRepo *r if err = gitrepo.SetDefaultBranch(ctx, repo, repo.DefaultBranch); err != nil { return fmt.Errorf("setDefaultBranch: %w", err) } - if err = UpdateRepository(ctx, repo, false); err != nil { + if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "default_branch"); err != nil { return fmt.Errorf("updateRepository: %w", err) } diff --git a/services/repository/migrate.go b/services/repository/migrate.go index 0859158b89915..553afbc4cfa25 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -220,7 +220,7 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, } repo.IsMirror = true - if err = UpdateRepository(ctx, repo, false); err != nil { + if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "num_watches", "is_empty", "default_branch", "default_wiki_branch", "is_mirror"); err != nil { return nil, err } diff --git a/services/repository/repository.go b/services/repository/repository.go index 739ef1ec384b9..0cdce336d4939 100644 --- a/services/repository/repository.go +++ b/services/repository/repository.go @@ -127,9 +127,42 @@ func UpdateRepository(ctx context.Context, repo *repo_model.Repository, visibili func MakeRepoPublic(ctx context.Context, repo *repo_model.Repository) (err error) { return db.WithTx(ctx, func(ctx context.Context) error { repo.IsPrivate = false - if err = updateRepository(ctx, repo, true); err != nil { - return fmt.Errorf("MakeRepoPublic: %w", err) + if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_private"); err != nil { + return err + } + + if err = repo.LoadOwner(ctx); err != nil { + return fmt.Errorf("LoadOwner: %w", err) } + if repo.Owner.IsOrganization() { + // Organization repository need to recalculate access table when visibility is changed. + if err = access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil { + return fmt.Errorf("recalculateTeamAccesses: %w", err) + } + } + + // Create/Remove git-daemon-export-ok for git-daemon... + if err := checkDaemonExportOK(ctx, repo); err != nil { + return err + } + + forkRepos, err := repo_model.GetRepositoriesByForkID(ctx, repo.ID) + if err != nil { + return fmt.Errorf("getRepositoriesByForkID: %w", err) + } + + if repo.Owner.Visibility != structs.VisibleTypePrivate { + for i := range forkRepos { + if err = MakeRepoPublic(ctx, forkRepos[i]); err != nil { + return fmt.Errorf("MakeRepoPublic[%d]: %w", forkRepos[i].ID, err) + } + } + } + + // If visibility is changed, we need to update the issue indexer. + // Since the data in the issue indexer have field to indicate if the repo is public or not. + issue_indexer.UpdateRepoIndexer(ctx, repo.ID) + return nil }) } @@ -137,9 +170,51 @@ func MakeRepoPublic(ctx context.Context, repo *repo_model.Repository) (err error func MakeRepoPrivate(ctx context.Context, repo *repo_model.Repository) (err error) { return db.WithTx(ctx, func(ctx context.Context) error { repo.IsPrivate = true - if err = updateRepository(ctx, repo, true); err != nil { - return fmt.Errorf("MakeRepoPrivate: %w", err) + if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_private"); err != nil { + return err + } + + if err = repo.LoadOwner(ctx); err != nil { + return fmt.Errorf("LoadOwner: %w", err) } + if repo.Owner.IsOrganization() { + // Organization repository need to recalculate access table when visibility is changed. + if err = access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil { + return fmt.Errorf("recalculateTeamAccesses: %w", err) + } + } + + // If repo has become private, we need to set its actions to private. + _, err = db.GetEngine(ctx).Where("repo_id = ?", repo.ID).Cols("is_private").Update(&activities_model.Action{ + IsPrivate: true, + }) + if err != nil { + return err + } + + if err = repo_model.ClearRepoStars(ctx, repo.ID); err != nil { + return err + } + + // Create/Remove git-daemon-export-ok for git-daemon... + if err := checkDaemonExportOK(ctx, repo); err != nil { + return err + } + + forkRepos, err := repo_model.GetRepositoriesByForkID(ctx, repo.ID) + if err != nil { + return fmt.Errorf("getRepositoriesByForkID: %w", err) + } + for i := range forkRepos { + if err = MakeRepoPrivate(ctx, forkRepos[i]); err != nil { + return fmt.Errorf("MakeRepoPrivate[%d]: %w", forkRepos[i].ID, err) + } + } + + // If visibility is changed, we need to update the issue indexer. + // Since the data in the issue indexer have field to indicate if the repo is public or not. + issue_indexer.UpdateRepoIndexer(ctx, repo.ID) + return nil }) } From 40f4b7ed5ebabdc3c49f6ecf16bd1e7160f0065a Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 30 May 2025 11:33:05 -0700 Subject: [PATCH 2/5] Fix test --- routers/web/repo/wiki_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/wiki_test.go b/routers/web/repo/wiki_test.go index d0139f6613520..ca74e6583e85e 100644 --- a/routers/web/repo/wiki_test.go +++ b/routers/web/repo/wiki_test.go @@ -245,7 +245,11 @@ func TestDefaultWikiBranch(t *testing.T) { assert.NoError(t, wiki_service.ChangeDefaultWikiBranch(db.DefaultContext, repoWithNoWiki, "main")) // repo with wiki - assert.NoError(t, repo_model.UpdateRepositoryColsNoAutoTime(db.DefaultContext, &repo_model.Repository{ID: 1, DefaultWikiBranch: "wrong-branch"})) + assert.NoError(t, repo_model.UpdateRepositoryColsNoAutoTime( + db.DefaultContext, + &repo_model.Repository{ID: 1, DefaultWikiBranch: "wrong-branch"}), + "default_wiki_branch", + ) ctx, _ := contexttest.MockContext(t, "user2/repo1/wiki") ctx.SetPathParam("*", "Home") From 34afc8e0d94dddb003ac6549c2bd7d6c67750b46 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 30 May 2025 13:03:43 -0700 Subject: [PATCH 3/5] Fix test --- routers/web/repo/wiki_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/routers/web/repo/wiki_test.go b/routers/web/repo/wiki_test.go index ca74e6583e85e..73f9970a072d9 100644 --- a/routers/web/repo/wiki_test.go +++ b/routers/web/repo/wiki_test.go @@ -247,8 +247,9 @@ func TestDefaultWikiBranch(t *testing.T) { // repo with wiki assert.NoError(t, repo_model.UpdateRepositoryColsNoAutoTime( db.DefaultContext, - &repo_model.Repository{ID: 1, DefaultWikiBranch: "wrong-branch"}), + &repo_model.Repository{ID: 1, DefaultWikiBranch: "wrong-branch"}, "default_wiki_branch", + ), ) ctx, _ := contexttest.MockContext(t, "user2/repo1/wiki") From 31cd865baad0bb171a1a94fc23196e18cd2d722c Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 30 May 2025 14:00:16 -0700 Subject: [PATCH 4/5] Improvements --- models/repo/repo.go | 8 -------- services/repository/adopt.go | 4 ++++ services/repository/create.go | 4 ++++ services/repository/fork.go | 7 +++++-- services/repository/generate.go | 28 ++++++++++------------------ services/repository/migrate.go | 4 ++++ 6 files changed, 27 insertions(+), 28 deletions(-) diff --git a/models/repo/repo.go b/models/repo/repo.go index 1a1ede2456d21..5aae02c6d8059 100644 --- a/models/repo/repo.go +++ b/models/repo/repo.go @@ -992,11 +992,3 @@ func UpdateRepositoryOwnerName(ctx context.Context, oldUserName, newUserName str } return nil } - -func UpdateRepositoryForksNum(ctx context.Context, repoID int64) error { - // Update the forks count of the repository - if _, err := db.GetEngine(ctx).Exec("UPDATE `repository` SET num_forks = (SELECT COUNT(*) FROM `repository` WHERE fork_id = ?) WHERE id = ?", repoID, repoID); err != nil { - return fmt.Errorf("update repository forks num: %w", err) - } - return nil -} diff --git a/services/repository/adopt.go b/services/repository/adopt.go index 68a8754279193..82b3817cd189b 100644 --- a/services/repository/adopt.go +++ b/services/repository/adopt.go @@ -201,6 +201,10 @@ func adoptRepository(ctx context.Context, repo *repo_model.Repository, defaultBr return fmt.Errorf("UpdateRepositoryCols: %w", err) } + if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { + log.Error("Failed to update size for repository: %v", err) + } + return nil } diff --git a/services/repository/create.go b/services/repository/create.go index f90f2c83f628a..6f918b2d24bfc 100644 --- a/services/repository/create.go +++ b/services/repository/create.go @@ -195,6 +195,10 @@ func initRepository(ctx context.Context, u *user_model.User, repo *repo_model.Re return fmt.Errorf("updateRepository: %w", err) } + if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { + log.Error("Failed to update size for repository: %v", err) + } + return nil } diff --git a/services/repository/fork.go b/services/repository/fork.go index 244f72167d38d..0ba3e9679a985 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -224,14 +224,17 @@ func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Reposit return err } - parentRepoID := repo.ForkID repo.IsFork = false repo.ForkID = 0 if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_fork", "fork_id"); err != nil { return err } - return repo_model.UpdateRepositoryForksNum(ctx, parentRepoID) + if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { + log.Error("Failed to update size for repository: %v", err) + } + + return nil }) } diff --git a/services/repository/generate.go b/services/repository/generate.go index 658299ac84362..586e60e2ec910 100644 --- a/services/repository/generate.go +++ b/services/repository/generate.go @@ -255,43 +255,35 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r return initRepoCommit(ctx, tmpDir, repo, repo.Owner, defaultBranch) } -func generateGitContent(ctx context.Context, repo, templateRepo, generateRepo *repo_model.Repository) (err error) { - tmpDir, cleanup, err := setting.AppDataTempDir("git-repo-content").MkdirTempRandom("gitea-" + repo.Name) +// GenerateGitContent generates git content from a template repository +func GenerateGitContent(ctx context.Context, templateRepo, generateRepo *repo_model.Repository) (err error) { + tmpDir, cleanup, err := setting.AppDataTempDir("git-repo-content").MkdirTempRandom("gitea-" + generateRepo.Name) if err != nil { - return fmt.Errorf("failed to create temp dir for repository %s: %w", repo.FullName(), err) + return fmt.Errorf("failed to create temp dir for repository %s: %w", generateRepo.FullName(), err) } defer cleanup() - if err = generateRepoCommit(ctx, repo, templateRepo, generateRepo, tmpDir); err != nil { + if err = generateRepoCommit(ctx, generateRepo, templateRepo, generateRepo, tmpDir); err != nil { return fmt.Errorf("generateRepoCommit: %w", err) } // re-fetch repo - if repo, err = repo_model.GetRepositoryByID(ctx, repo.ID); err != nil { + if generateRepo, err = repo_model.GetRepositoryByID(ctx, generateRepo.ID); err != nil { return fmt.Errorf("getRepositoryByID: %w", err) } // if there was no default branch supplied when generating the repo, use the default one from the template - if strings.TrimSpace(repo.DefaultBranch) == "" { - repo.DefaultBranch = templateRepo.DefaultBranch + if strings.TrimSpace(generateRepo.DefaultBranch) == "" { + generateRepo.DefaultBranch = templateRepo.DefaultBranch } - if err = gitrepo.SetDefaultBranch(ctx, repo, repo.DefaultBranch); err != nil { + if err = gitrepo.SetDefaultBranch(ctx, generateRepo, generateRepo.DefaultBranch); err != nil { return fmt.Errorf("setDefaultBranch: %w", err) } - if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "default_branch"); err != nil { + if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, generateRepo, "default_branch"); err != nil { return fmt.Errorf("updateRepository: %w", err) } - return nil -} - -// GenerateGitContent generates git content from a template repository -func GenerateGitContent(ctx context.Context, templateRepo, generateRepo *repo_model.Repository) error { - if err := generateGitContent(ctx, generateRepo, templateRepo, generateRepo); err != nil { - return err - } - if err := repo_module.UpdateRepoSize(ctx, generateRepo); err != nil { return fmt.Errorf("failed to update size for repository: %w", err) } diff --git a/services/repository/migrate.go b/services/repository/migrate.go index 553afbc4cfa25..0a3dc45339fd8 100644 --- a/services/repository/migrate.go +++ b/services/repository/migrate.go @@ -224,6 +224,10 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User, return nil, err } + if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { + log.Error("Failed to update size for repository: %v", err) + } + // this is necessary for sync local tags from remote configName := fmt.Sprintf("remote.%s.fetch", mirrorModel.GetRemoteName()) if stdout, _, err := git.NewCommand("config"). From 25fe804dd3f4b01cae8a248be29fb49d4350ef23 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 30 May 2025 14:03:07 -0700 Subject: [PATCH 5/5] Fix lint --- services/repository/fork.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/services/repository/fork.go b/services/repository/fork.go index 0ba3e9679a985..d0568e6072e46 100644 --- a/services/repository/fork.go +++ b/services/repository/fork.go @@ -226,15 +226,7 @@ func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Reposit repo.IsFork = false repo.ForkID = 0 - if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_fork", "fork_id"); err != nil { - return err - } - - if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { - log.Error("Failed to update size for repository: %v", err) - } - - return nil + return repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_fork", "fork_id") }) }