Skip to content

Commit 224aa64

Browse files
lunnylunny
authored
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`.
1 parent 1e644e3 commit 224aa64

File tree

11 files changed

+121
-57
lines changed

11 files changed

+121
-57
lines changed

‎models/issues/pull.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -649,12 +649,6 @@ func GetAllUnmergedAgitPullRequestByPoster(ctx context.Context, uid int64) ([]*P
649649
return pulls, err
650650
}
651651

652-
// Update updates all fields of pull request.
653-
func (pr *PullRequest) Update(ctx context.Context) error {
654-
_, err := db.GetEngine(ctx).ID(pr.ID).AllCols().Update(pr)
655-
return err
656-
}
657-
658652
// UpdateCols updates specific fields of pull request.
659653
func (pr *PullRequest) UpdateCols(ctx context.Context, cols ...string) error {
660654
_, err := db.GetEngine(ctx).ID(pr.ID).Cols(cols...).Update(pr)

‎models/issues/pull_test.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -248,19 +248,6 @@ func TestGetPullRequestByIssueID(t *testing.T) {
248248
assert.True(t, issues_model.IsErrPullRequestNotExist(err))
249249
}
250250

251-
func TestPullRequest_Update(t *testing.T) {
252-
assert.NoError(t, unittest.PrepareTestDatabase())
253-
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: 1})
254-
pr.BaseBranch = "baseBranch"
255-
pr.HeadBranch = "headBranch"
256-
pr.Update(db.DefaultContext)
257-
258-
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{ID: pr.ID})
259-
assert.Equal(t, "baseBranch", pr.BaseBranch)
260-
assert.Equal(t, "headBranch", pr.HeadBranch)
261-
unittest.CheckConsistencyFor(t, pr)
262-
}
263-
264251
func TestPullRequest_UpdateCols(t *testing.T) {
265252
assert.NoError(t, unittest.PrepareTestDatabase())
266253
pr := &issues_model.PullRequest{

‎models/repo/release.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ func AddReleaseAttachments(ctx context.Context, releaseID int64, attachmentUUIDs
180180
}
181181
attachments[i].ReleaseID = releaseID
182182
// No assign value could be 0, so ignore AllCols().
183-
if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Update(attachments[i]); err != nil {
183+
if _, err = db.GetEngine(ctx).ID(attachments[i].ID).Cols("release_id").Update(attachments[i]); err != nil {
184184
return fmt.Errorf("update attachment [%d]: %w", attachments[i].ID, err)
185185
}
186186
}

‎models/repo/update.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,18 @@ func UpdateRepositoryUpdatedTime(ctx context.Context, repoID int64, updateTime t
4242

4343
// UpdateRepositoryColsWithAutoTime updates repository's columns
4444
func UpdateRepositoryColsWithAutoTime(ctx context.Context, repo *Repository, cols ...string) error {
45+
if len(cols) == 0 {
46+
return nil
47+
}
4548
_, err := db.GetEngine(ctx).ID(repo.ID).Cols(cols...).Update(repo)
4649
return err
4750
}
4851

4952
// UpdateRepositoryColsNoAutoTime updates repository's columns and but applies time change automatically
5053
func UpdateRepositoryColsNoAutoTime(ctx context.Context, repo *Repository, cols ...string) error {
54+
if len(cols) == 0 {
55+
return nil
56+
}
5157
_, err := db.GetEngine(ctx).ID(repo.ID).Cols(cols...).NoAutoTime().Update(repo)
5258
return err
5359
}

‎routers/web/repo/wiki_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,12 @@ func TestDefaultWikiBranch(t *testing.T) {
245245
assert.NoError(t, wiki_service.ChangeDefaultWikiBranch(db.DefaultContext, repoWithNoWiki, "main"))
246246

247247
// repo with wiki
248-
assert.NoError(t, repo_model.UpdateRepositoryColsNoAutoTime(db.DefaultContext, &repo_model.Repository{ID: 1, DefaultWikiBranch: "wrong-branch"}))
248+
assert.NoError(t, repo_model.UpdateRepositoryColsNoAutoTime(
249+
db.DefaultContext,
250+
&repo_model.Repository{ID: 1, DefaultWikiBranch: "wrong-branch"},
251+
"default_wiki_branch",
252+
),
253+
)
249254

250255
ctx, _ := contexttest.MockContext(t, "user2/repo1/wiki")
251256
ctx.SetPathParam("*", "Home")

‎services/repository/adopt.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,8 +196,13 @@ func adoptRepository(ctx context.Context, repo *repo_model.Repository, defaultBr
196196
return fmt.Errorf("setDefaultBranch: %w", err)
197197
}
198198
}
199-
if err = updateRepository(ctx, repo, false); err != nil {
200-
return fmt.Errorf("updateRepository: %w", err)
199+
200+
if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_empty", "default_branch"); err != nil {
201+
return fmt.Errorf("UpdateRepositoryCols: %w", err)
202+
}
203+
204+
if err = repo_module.UpdateRepoSize(ctx, repo); err != nil {
205+
log.Error("Failed to update size for repository: %v", err)
201206
}
202207

203208
return nil

‎services/repository/create.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,14 @@ func initRepository(ctx context.Context, u *user_model.User, repo *repo_model.Re
191191
}
192192
}
193193

194-
if err = UpdateRepository(ctx, repo, false); err != nil {
194+
if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_empty", "default_branch", "default_wiki_branch"); err != nil {
195195
return fmt.Errorf("updateRepository: %w", err)
196196
}
197197

198+
if err = repo_module.UpdateRepoSize(ctx, repo); err != nil {
199+
log.Error("Failed to update size for repository: %v", err)
200+
}
201+
198202
return nil
199203
}
200204

‎services/repository/fork.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func ForkRepository(ctx context.Context, doer, owner *user_model.User, opts Fork
209209

210210
// ConvertForkToNormalRepository convert the provided repo from a forked repo to normal repo
211211
func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Repository) error {
212-
err := db.WithTx(ctx, func(ctx context.Context) error {
212+
return db.WithTx(ctx, func(ctx context.Context) error {
213213
repo, err := repo_model.GetRepositoryByID(ctx, repo.ID)
214214
if err != nil {
215215
return err
@@ -226,16 +226,8 @@ func ConvertForkToNormalRepository(ctx context.Context, repo *repo_model.Reposit
226226

227227
repo.IsFork = false
228228
repo.ForkID = 0
229-
230-
if err := updateRepository(ctx, repo, false); err != nil {
231-
log.Error("Unable to update repository %-v whilst converting from fork. Error: %v", repo, err)
232-
return err
233-
}
234-
235-
return nil
229+
return repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_fork", "fork_id")
236230
})
237-
238-
return err
239231
}
240232

241233
type findForksOptions struct {

‎services/repository/generate.go

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -253,43 +253,35 @@ func generateRepoCommit(ctx context.Context, repo, templateRepo, generateRepo *r
253253
return initRepoCommit(ctx, tmpDir, repo, repo.Owner, defaultBranch)
254254
}
255255

256-
func generateGitContent(ctx context.Context, repo, templateRepo, generateRepo *repo_model.Repository) (err error) {
257-
tmpDir, cleanup, err := setting.AppDataTempDir("git-repo-content").MkdirTempRandom("gitea-" + repo.Name)
256+
// GenerateGitContent generates git content from a template repository
257+
func GenerateGitContent(ctx context.Context, templateRepo, generateRepo *repo_model.Repository) (err error) {
258+
tmpDir, cleanup, err := setting.AppDataTempDir("git-repo-content").MkdirTempRandom("gitea-" + generateRepo.Name)
258259
if err != nil {
259-
return fmt.Errorf("failed to create temp dir for repository %s: %w", repo.FullName(), err)
260+
return fmt.Errorf("failed to create temp dir for repository %s: %w", generateRepo.FullName(), err)
260261
}
261262
defer cleanup()
262263

263-
if err = generateRepoCommit(ctx, repo, templateRepo, generateRepo, tmpDir); err != nil {
264+
if err = generateRepoCommit(ctx, generateRepo, templateRepo, generateRepo, tmpDir); err != nil {
264265
return fmt.Errorf("generateRepoCommit: %w", err)
265266
}
266267

267268
// re-fetch repo
268-
if repo, err = repo_model.GetRepositoryByID(ctx, repo.ID); err != nil {
269+
if generateRepo, err = repo_model.GetRepositoryByID(ctx, generateRepo.ID); err != nil {
269270
return fmt.Errorf("getRepositoryByID: %w", err)
270271
}
271272

272273
// if there was no default branch supplied when generating the repo, use the default one from the template
273-
if strings.TrimSpace(repo.DefaultBranch) == "" {
274-
repo.DefaultBranch = templateRepo.DefaultBranch
274+
if strings.TrimSpace(generateRepo.DefaultBranch) == "" {
275+
generateRepo.DefaultBranch = templateRepo.DefaultBranch
275276
}
276277

277-
if err = gitrepo.SetDefaultBranch(ctx, repo, repo.DefaultBranch); err != nil {
278+
if err = gitrepo.SetDefaultBranch(ctx, generateRepo, generateRepo.DefaultBranch); err != nil {
278279
return fmt.Errorf("setDefaultBranch: %w", err)
279280
}
280-
if err = UpdateRepository(ctx, repo, false); err != nil {
281+
if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, generateRepo, "default_branch"); err != nil {
281282
return fmt.Errorf("updateRepository: %w", err)
282283
}
283284

284-
return nil
285-
}
286-
287-
// GenerateGitContent generates git content from a template repository
288-
func GenerateGitContent(ctx context.Context, templateRepo, generateRepo *repo_model.Repository) error {
289-
if err := generateGitContent(ctx, generateRepo, templateRepo, generateRepo); err != nil {
290-
return err
291-
}
292-
293285
if err := repo_module.UpdateRepoSize(ctx, generateRepo); err != nil {
294286
return fmt.Errorf("failed to update size for repository: %w", err)
295287
}

‎services/repository/migrate.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,10 +220,14 @@ func MigrateRepositoryGitData(ctx context.Context, u *user_model.User,
220220
}
221221

222222
repo.IsMirror = true
223-
if err = UpdateRepository(ctx, repo, false); err != nil {
223+
if err = repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "num_watches", "is_empty", "default_branch", "default_wiki_branch", "is_mirror"); err != nil {
224224
return nil, err
225225
}
226226

227+
if err = repo_module.UpdateRepoSize(ctx, repo); err != nil {
228+
log.Error("Failed to update size for repository: %v", err)
229+
}
230+
227231
// this is necessary for sync local tags from remote
228232
configName := fmt.Sprintf("remote.%s.fetch", mirrorModel.GetRemoteName())
229233
if stdout, _, err := git.NewCommand("config").

‎services/repository/repository.go

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,19 +127,94 @@ func UpdateRepository(ctx context.Context, repo *repo_model.Repository, visibili
127127
func MakeRepoPublic(ctx context.Context, repo *repo_model.Repository) (err error) {
128128
return db.WithTx(ctx, func(ctx context.Context) error {
129129
repo.IsPrivate = false
130-
if err = updateRepository(ctx, repo, true); err != nil {
131-
return fmt.Errorf("MakeRepoPublic: %w", err)
130+
if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_private"); err != nil {
131+
return err
132+
}
133+
134+
if err = repo.LoadOwner(ctx); err != nil {
135+
return fmt.Errorf("LoadOwner: %w", err)
132136
}
137+
if repo.Owner.IsOrganization() {
138+
// Organization repository need to recalculate access table when visibility is changed.
139+
if err = access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil {
140+
return fmt.Errorf("recalculateTeamAccesses: %w", err)
141+
}
142+
}
143+
144+
// Create/Remove git-daemon-export-ok for git-daemon...
145+
if err := checkDaemonExportOK(ctx, repo); err != nil {
146+
return err
147+
}
148+
149+
forkRepos, err := repo_model.GetRepositoriesByForkID(ctx, repo.ID)
150+
if err != nil {
151+
return fmt.Errorf("getRepositoriesByForkID: %w", err)
152+
}
153+
154+
if repo.Owner.Visibility != structs.VisibleTypePrivate {
155+
for i := range forkRepos {
156+
if err = MakeRepoPublic(ctx, forkRepos[i]); err != nil {
157+
return fmt.Errorf("MakeRepoPublic[%d]: %w", forkRepos[i].ID, err)
158+
}
159+
}
160+
}
161+
162+
// If visibility is changed, we need to update the issue indexer.
163+
// Since the data in the issue indexer have field to indicate if the repo is public or not.
164+
issue_indexer.UpdateRepoIndexer(ctx, repo.ID)
165+
133166
return nil
134167
})
135168
}
136169

137170
func MakeRepoPrivate(ctx context.Context, repo *repo_model.Repository) (err error) {
138171
return db.WithTx(ctx, func(ctx context.Context) error {
139172
repo.IsPrivate = true
140-
if err = updateRepository(ctx, repo, true); err != nil {
141-
return fmt.Errorf("MakeRepoPrivate: %w", err)
173+
if err := repo_model.UpdateRepositoryColsNoAutoTime(ctx, repo, "is_private"); err != nil {
174+
return err
175+
}
176+
177+
if err = repo.LoadOwner(ctx); err != nil {
178+
return fmt.Errorf("LoadOwner: %w", err)
142179
}
180+
if repo.Owner.IsOrganization() {
181+
// Organization repository need to recalculate access table when visibility is changed.
182+
if err = access_model.RecalculateTeamAccesses(ctx, repo, 0); err != nil {
183+
return fmt.Errorf("recalculateTeamAccesses: %w", err)
184+
}
185+
}
186+
187+
// If repo has become private, we need to set its actions to private.
188+
_, err = db.GetEngine(ctx).Where("repo_id = ?", repo.ID).Cols("is_private").Update(&activities_model.Action{
189+
IsPrivate: true,
190+
})
191+
if err != nil {
192+
return err
193+
}
194+
195+
if err = repo_model.ClearRepoStars(ctx, repo.ID); err != nil {
196+
return err
197+
}
198+
199+
// Create/Remove git-daemon-export-ok for git-daemon...
200+
if err := checkDaemonExportOK(ctx, repo); err != nil {
201+
return err
202+
}
203+
204+
forkRepos, err := repo_model.GetRepositoriesByForkID(ctx, repo.ID)
205+
if err != nil {
206+
return fmt.Errorf("getRepositoriesByForkID: %w", err)
207+
}
208+
for i := range forkRepos {
209+
if err = MakeRepoPrivate(ctx, forkRepos[i]); err != nil {
210+
return fmt.Errorf("MakeRepoPrivate[%d]: %w", forkRepos[i].ID, err)
211+
}
212+
}
213+
214+
// If visibility is changed, we need to update the issue indexer.
215+
// Since the data in the issue indexer have field to indicate if the repo is public or not.
216+
issue_indexer.UpdateRepoIndexer(ctx, repo.ID)
217+
143218
return nil
144219
})
145220
}

0 commit comments

Comments
 (0)