From 0771a79bf0ff2192c690385005aaf8d6c26c6b58 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 27 Jun 2025 01:23:21 +0800 Subject: [PATCH] Use standalone function to update repository cols (#34811) Extract `UpdateRepository` Follow up #34762 --------- Co-authored-by: wxiaoguang --- models/repo/update.go | 18 ++++++------------ modules/repository/branch.go | 9 +++++---- routers/private/hook_post_receive.go | 26 ++++++++++---------------- routers/web/repo/setting/setting.go | 27 ++++++++++----------------- 4 files changed, 31 insertions(+), 49 deletions(-) diff --git a/models/repo/update.go b/models/repo/update.go index f82ff7c76c..64065f11c4 100644 --- a/models/repo/update.go +++ b/models/repo/update.go @@ -40,21 +40,15 @@ func UpdateRepositoryUpdatedTime(ctx context.Context, repoID int64, updateTime t return err } -// 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) +// UpdateRepositoryColsWithAutoTime updates repository's columns and the timestamp fields automatically +func UpdateRepositoryColsWithAutoTime(ctx context.Context, repo *Repository, colName string, moreColNames ...string) error { + _, err := db.GetEngine(ctx).ID(repo.ID).Cols(append([]string{colName}, moreColNames...)...).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) +// UpdateRepositoryColsNoAutoTime updates repository's columns, doesn't change timestamp field automatically +func UpdateRepositoryColsNoAutoTime(ctx context.Context, repo *Repository, colName string, moreColNames ...string) error { + _, err := db.GetEngine(ctx).ID(repo.ID).Cols(append([]string{colName}, moreColNames...)...).NoAutoTime().Update(repo) return err } diff --git a/modules/repository/branch.go b/modules/repository/branch.go index 2bf9930f19..30aa0a6e85 100644 --- a/modules/repository/branch.go +++ b/modules/repository/branch.go @@ -41,11 +41,12 @@ func SyncRepoBranchesWithRepo(ctx context.Context, repo *repo_model.Repository, if err != nil { return 0, fmt.Errorf("GetObjectFormat: %w", err) } - _, err = db.GetEngine(ctx).ID(repo.ID).Update(&repo_model.Repository{ObjectFormatName: objFmt.Name()}) - if err != nil { - return 0, fmt.Errorf("UpdateRepository: %w", err) + if objFmt.Name() != repo.ObjectFormatName { + repo.ObjectFormatName = objFmt.Name() + if err = repo_model.UpdateRepositoryColsWithAutoTime(ctx, repo, "object_format_name"); err != nil { + return 0, fmt.Errorf("UpdateRepositoryColsWithAutoTime: %w", err) + } } - repo.ObjectFormatName = objFmt.Name() // keep consistent with db allBranches := container.Set[string]{} { diff --git a/routers/private/hook_post_receive.go b/routers/private/hook_post_receive.go index a391e572b3..e8bef7d6c1 100644 --- a/routers/private/hook_post_receive.go +++ b/routers/private/hook_post_receive.go @@ -207,25 +207,19 @@ func HookPostReceive(ctx *gitea_context.PrivateContext) { return } - cols := make([]string, 0, 2) - - if isPrivate.Has() { + // FIXME: these options are not quite right, for example: changing visibility should do more works than just setting the is_private flag + // These options should only be used for "push-to-create" + if isPrivate.Has() && repo.IsPrivate != isPrivate.Value() { + // TODO: it needs to do more work repo.IsPrivate = isPrivate.Value() - cols = append(cols, "is_private") + if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_private"); err != nil { + ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Failed to change visibility"}) + } } - - if isTemplate.Has() { + if isTemplate.Has() && repo.IsTemplate != isTemplate.Value() { repo.IsTemplate = isTemplate.Value() - cols = append(cols, "is_template") - } - - if len(cols) > 0 { - if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, cols...); err != nil { - log.Error("Failed to Update: %s/%s Error: %v", ownerName, repoName, err) - ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{ - Err: fmt.Sprintf("Failed to Update: %s/%s Error: %v", ownerName, repoName, err), - }) - return + if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_template"); err != nil { + ctx.JSON(http.StatusInternalServerError, private.HookPostReceiveResult{Err: "Failed to change template status"}) } } } diff --git a/routers/web/repo/setting/setting.go b/routers/web/repo/setting/setting.go index 6602685e94..6e16ead183 100644 --- a/routers/web/repo/setting/setting.go +++ b/routers/web/repo/setting/setting.go @@ -663,44 +663,37 @@ func handleSettingsPostAdvanced(ctx *context.Context) { func handleSettingsPostSigning(ctx *context.Context) { form := web.GetForm(ctx).(*forms.RepoSettingForm) repo := ctx.Repo.Repository - changed := false trustModel := repo_model.ToTrustModel(form.TrustModel) if trustModel != repo.TrustModel { repo.TrustModel = trustModel - changed = true - } - - if changed { - if err := repo_service.UpdateRepository(ctx, repo, false); err != nil { - ctx.ServerError("UpdateRepository", err) + if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "trust_model"); err != nil { + ctx.ServerError("UpdateRepositoryColsNoAutoTime", err) return } + log.Trace("Repository signing settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name) } - log.Trace("Repository signing settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name) ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success")) ctx.Redirect(ctx.Repo.RepoLink + "/settings") } func handleSettingsPostAdmin(ctx *context.Context) { - form := web.GetForm(ctx).(*forms.RepoSettingForm) - repo := ctx.Repo.Repository if !ctx.Doer.IsAdmin { ctx.HTTPError(http.StatusForbidden) return } + repo := ctx.Repo.Repository + form := web.GetForm(ctx).(*forms.RepoSettingForm) if repo.IsFsckEnabled != form.EnableHealthCheck { repo.IsFsckEnabled = form.EnableHealthCheck + if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_fsck_enabled"); err != nil { + ctx.ServerError("UpdateRepositoryColsNoAutoTime", err) + return + } + log.Trace("Repository admin settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name) } - if err := repo_service.UpdateRepository(ctx, repo, false); err != nil { - ctx.ServerError("UpdateRepository", err) - return - } - - log.Trace("Repository admin settings updated: %s/%s", ctx.Repo.Owner.Name, repo.Name) - ctx.Flash.Success(ctx.Tr("repo.settings.update_settings_success")) ctx.Redirect(ctx.Repo.RepoLink + "/settings") }