mirror of
				https://github.com/go-gitea/gitea.git
				synced 2025-10-30 02:16:20 +01:00 
			
		
		
		
	Replace update repository function in some places (#34566)
`UpdateAllCols` is dangerous, the columns should be updated when necessary. This PR replaces some `updateRepository` invokes to reduce possible problems and wrongly updated time. Some parts have been fixed in #34388, but some are hidden in the function `updateRepository`. Alternatively, using `UpdateRepositoryColsNoAutoTime` to update the changed columns. Some `UpdateRepoSize` invokes are duplicated, so they will be removed when extracting from `updateRepository`.
This commit is contained in:
		| @@ -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) | ||||
|   | ||||
| @@ -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{ | ||||
|   | ||||
| @@ -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) | ||||
| 		} | ||||
| 	} | ||||
|   | ||||
| @@ -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 | ||||
| } | ||||
|   | ||||
| @@ -245,7 +245,12 @@ 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") | ||||
|   | ||||
| @@ -196,8 +196,13 @@ 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) | ||||
| 	} | ||||
|  | ||||
| 	if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { | ||||
| 		log.Error("Failed to update size for repository: %v", err) | ||||
| 	} | ||||
|  | ||||
| 	return nil | ||||
|   | ||||
| @@ -191,10 +191,14 @@ 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) | ||||
| 	} | ||||
|  | ||||
| 	if err = repo_module.UpdateRepoSize(ctx, repo); err != nil { | ||||
| 		log.Error("Failed to update size for repository: %v", err) | ||||
| 	} | ||||
|  | ||||
| 	return nil | ||||
| } | ||||
|  | ||||
|   | ||||
| @@ -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 | ||||
| @@ -226,16 +226,8 @@ func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Reposit | ||||
|  | ||||
| 		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) | ||||
| 			return err | ||||
| 		} | ||||
|  | ||||
| 		return nil | ||||
| 		return repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_fork", "fork_id") | ||||
| 	}) | ||||
|  | ||||
| 	return err | ||||
| } | ||||
|  | ||||
| type findForksOptions struct { | ||||
|   | ||||
| @@ -253,43 +253,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 = UpdateRepository(ctx, repo, false); 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) | ||||
| 	} | ||||
|   | ||||
| @@ -220,10 +220,14 @@ 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 | ||||
| 		} | ||||
|  | ||||
| 		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"). | ||||
|   | ||||
| @@ -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 | ||||
| 	}) | ||||
| } | ||||
|   | ||||
		Reference in New Issue
	
	Block a user