Use standalone function to update repository cols (#34811)

Extract `UpdateRepository`
Follow up #34762

---------

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

Author: lunny

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
 }
 

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]{}
 	{

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"})
 			}
 		}
 	}

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")
 }