Skip to content

Commit 40cdc84

Browse files
lunnylunny
authored
Fix migration v292 (#30153)
Fix #29874 (comment) - The migration of v292 will miss many projects. These projects will have no default board. This PR introduced a new migration number and removed v292 migration. - This PR also added the missed transactions on project-related operations. - Only `SetDefaultBoard` will remove duplicated defaults but not in `GetDefaultBoard`
1 parent 9585e19 commit 40cdc84

File tree

6 files changed

+162
-134
lines changed

6 files changed

+162
-134
lines changed

‎models/migrations/migrations.go

+2
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,8 @@ var migrations = []Migration{
569569
// v291 -> v292
570570
NewMigration("Add Index to attachment.comment_id", v1_22.AddCommentIDIndexofAttachment),
571571
// v292 -> v293
572+
NewMigration("Ensure every project has exactly one default column - No Op", noopMigration),
573+
// v293 -> v294
572574
NewMigration("Ensure every project has exactly one default column", v1_22.CheckProjectColumnsConsistency),
573575
}
574576

‎models/migrations/v1_22/v292.go

+4-80
Original file line numberDiff line numberDiff line change
@@ -3,83 +3,7 @@
33

44
package v1_22 //nolint
55

6-
import (
7-
"code.gitea.io/gitea/models/project"
8-
"code.gitea.io/gitea/modules/setting"
9-
10-
"xorm.io/builder"
11-
"xorm.io/xorm"
12-
)
13-
14-
// CheckProjectColumnsConsistency ensures there is exactly one default board per project present
15-
func CheckProjectColumnsConsistency(x *xorm.Engine) error {
16-
sess := x.NewSession()
17-
defer sess.Close()
18-
19-
if err := sess.Begin(); err != nil {
20-
return err
21-
}
22-
23-
limit := setting.Database.IterateBufferSize
24-
if limit <= 0 {
25-
limit = 50
26-
}
27-
28-
start := 0
29-
30-
for {
31-
var projects []project.Project
32-
if err := sess.SQL("SELECT DISTINCT `p`.`id`, `p`.`creator_id` FROM `project` `p` WHERE (SELECT COUNT(*) FROM `project_board` `pb` WHERE `pb`.`project_id` = `p`.`id` AND `pb`.`default` = ?) != 1", true).
33-
Limit(limit, start).
34-
Find(&projects); err != nil {
35-
return err
36-
}
37-
38-
if len(projects) == 0 {
39-
break
40-
}
41-
start += len(projects)
42-
43-
for _, p := range projects {
44-
var boards []project.Board
45-
if err := sess.Where("project_id=? AND `default` = ?", p.ID, true).OrderBy("sorting").Find(&boards); err != nil {
46-
return err
47-
}
48-
49-
if len(boards) == 0 {
50-
if _, err := sess.Insert(project.Board{
51-
ProjectID: p.ID,
52-
Default: true,
53-
Title: "Uncategorized",
54-
CreatorID: p.CreatorID,
55-
}); err != nil {
56-
return err
57-
}
58-
continue
59-
}
60-
61-
var boardsToUpdate []int64
62-
for id, b := range boards {
63-
if id > 0 {
64-
boardsToUpdate = append(boardsToUpdate, b.ID)
65-
}
66-
}
67-
68-
if _, err := sess.Where(builder.Eq{"project_id": p.ID}.And(builder.In("id", boardsToUpdate))).
69-
Cols("`default`").Update(&project.Board{Default: false}); err != nil {
70-
return err
71-
}
72-
}
73-
74-
if start%1000 == 0 {
75-
if err := sess.Commit(); err != nil {
76-
return err
77-
}
78-
if err := sess.Begin(); err != nil {
79-
return err
80-
}
81-
}
82-
}
83-
84-
return sess.Commit()
85-
}
6+
// NOTE: noop the original migration has bug which some projects will be skip, so
7+
// these projects will have no default board.
8+
// So that this migration will be skipped and go to v293.go
9+
// This file is a placeholder so that readers can know what happened

‎models/migrations/v1_22/v293.go

+108
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
// Copyright 2024 The Gitea Authors. All rights reserved.
2+
// SPDX-License-Identifier: MIT
3+
4+
package v1_22 //nolint
5+
6+
import (
7+
"code.gitea.io/gitea/modules/setting"
8+
"code.gitea.io/gitea/modules/timeutil"
9+
10+
"xorm.io/xorm"
11+
)
12+
13+
// CheckProjectColumnsConsistency ensures there is exactly one default board per project present
14+
func CheckProjectColumnsConsistency(x *xorm.Engine) error {
15+
sess := x.NewSession()
16+
defer sess.Close()
17+
18+
limit := setting.Database.IterateBufferSize
19+
if limit <= 0 {
20+
limit = 50
21+
}
22+
23+
type Project struct {
24+
ID int64
25+
CreatorID int64
26+
BoardID int64
27+
}
28+
29+
type ProjectBoard struct {
30+
ID int64 `xorm:"pk autoincr"`
31+
Title string
32+
Default bool `xorm:"NOT NULL DEFAULT false"` // issues not assigned to a specific board will be assigned to this board
33+
Sorting int8 `xorm:"NOT NULL DEFAULT 0"`
34+
Color string `xorm:"VARCHAR(7)"`
35+
36+
ProjectID int64 `xorm:"INDEX NOT NULL"`
37+
CreatorID int64 `xorm:"NOT NULL"`
38+
39+
CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"`
40+
UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"`
41+
}
42+
43+
for {
44+
if err := sess.Begin(); err != nil {
45+
return err
46+
}
47+
48+
// all these projects without defaults will be fixed in the same loop, so
49+
// we just need to always get projects without defaults until no such project
50+
var projects []*Project
51+
if err := sess.Select("project.id as id, project.creator_id, project_board.id as board_id").
52+
Join("LEFT", "project_board", "project_board.project_id = project.id AND project_board.`default`=?", true).
53+
Where("project_board.id is NULL OR project_board.id = 0").
54+
Limit(limit).
55+
Find(&projects); err != nil {
56+
return err
57+
}
58+
59+
for _, p := range projects {
60+
if _, err := sess.Insert(ProjectBoard{
61+
ProjectID: p.ID,
62+
Default: true,
63+
Title: "Uncategorized",
64+
CreatorID: p.CreatorID,
65+
}); err != nil {
66+
return err
67+
}
68+
}
69+
if err := sess.Commit(); err != nil {
70+
return err
71+
}
72+
73+
if len(projects) == 0 {
74+
break
75+
}
76+
}
77+
sess.Close()
78+
79+
return removeDuplicatedBoardDefault(x)
80+
}
81+
82+
func removeDuplicatedBoardDefault(x *xorm.Engine) error {
83+
type ProjectInfo struct {
84+
ProjectID int64
85+
DefaultNum int
86+
}
87+
var projects []ProjectInfo
88+
if err := x.Select("project_id, count(*) AS default_num").
89+
Table("project_board").
90+
Where("`default` = ?", true).
91+
GroupBy("project_id").
92+
Having("count(*) > 1").
93+
Find(&projects); err != nil {
94+
return err
95+
}
96+
97+
for _, project := range projects {
98+
if _, err := x.Where("project_id=?", project.ProjectID).
99+
Table("project_board").
100+
Limit(project.DefaultNum - 1).
101+
Update(map[string]bool{
102+
"`default`": false,
103+
}); err != nil {
104+
return err
105+
}
106+
}
107+
return nil
108+
}

‎models/migrations/v1_22/v292_test.go renamed to ‎models/migrations/v1_22/v293_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,14 @@ func Test_CheckProjectColumnsConsistency(t *testing.T) {
3131
assert.Equal(t, int64(1), defaultBoard.ProjectID)
3232
assert.True(t, defaultBoard.Default)
3333

34-
// check if multiple defaults were removed
34+
// check if multiple defaults, previous were removed and last will be kept
3535
expectDefaultBoard, err := project.GetBoard(db.DefaultContext, 2)
3636
assert.NoError(t, err)
3737
assert.Equal(t, int64(2), expectDefaultBoard.ProjectID)
38-
assert.True(t, expectDefaultBoard.Default)
38+
assert.False(t, expectDefaultBoard.Default)
3939

4040
expectNonDefaultBoard, err := project.GetBoard(db.DefaultContext, 3)
4141
assert.NoError(t, err)
4242
assert.Equal(t, int64(2), expectNonDefaultBoard.ProjectID)
43-
assert.False(t, expectNonDefaultBoard.Default)
43+
assert.True(t, expectNonDefaultBoard.Default)
4444
}

‎models/project/board.go

+40-50
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,6 @@ func deleteBoardByProjectID(ctx context.Context, projectID int64) error {
209209
// GetBoard fetches the current board of a project
210210
func GetBoard(ctx context.Context, boardID int64) (*Board, error) {
211211
board := new(Board)
212-
213212
has, err := db.GetEngine(ctx).ID(boardID).Get(board)
214213
if err != nil {
215214
return nil, err
@@ -260,71 +259,62 @@ func (p *Project) GetBoards(ctx context.Context) (BoardList, error) {
260259

261260
// getDefaultBoard return default board and ensure only one exists
262261
func (p *Project) getDefaultBoard(ctx context.Context) (*Board, error) {
263-
var boards []Board
264-
if err := db.GetEngine(ctx).Where("project_id=? AND `default` = ?", p.ID, true).OrderBy("sorting").Find(&boards); err != nil {
262+
var board Board
263+
has, err := db.GetEngine(ctx).
264+
Where("project_id=? AND `default` = ?", p.ID, true).
265+
Desc("id").Get(&board)
266+
if err != nil {
265267
return nil, err
266268
}
267269

268-
// create a default board if none is found
269-
if len(boards) == 0 {
270-
board := Board{
271-
ProjectID: p.ID,
272-
Default: true,
273-
Title: "Uncategorized",
274-
CreatorID: p.CreatorID,
275-
}
276-
if _, err := db.GetEngine(ctx).Insert(); err != nil {
277-
return nil, err
278-
}
270+
if has {
279271
return &board, nil
280272
}
281273

282-
// unset default boards where too many default boards exist
283-
if len(boards) > 1 {
284-
var boardsToUpdate []int64
285-
for id, b := range boards {
286-
if id > 0 {
287-
boardsToUpdate = append(boardsToUpdate, b.ID)
288-
}
289-
}
290-
291-
if _, err := db.GetEngine(ctx).Where(builder.Eq{"project_id": p.ID}.And(builder.In("id", boardsToUpdate))).
292-
Cols("`default`").Update(&Board{Default: false}); err != nil {
293-
return nil, err
294-
}
274+
// create a default board if none is found
275+
board = Board{
276+
ProjectID: p.ID,
277+
Default: true,
278+
Title: "Uncategorized",
279+
CreatorID: p.CreatorID,
295280
}
296-
297-
return &boards[0], nil
281+
if _, err := db.GetEngine(ctx).Insert(&board); err != nil {
282+
return nil, err
283+
}
284+
return &board, nil
298285
}
299286

300287
// SetDefaultBoard represents a board for issues not assigned to one
301288
func SetDefaultBoard(ctx context.Context, projectID, boardID int64) error {
302-
if _, err := GetBoard(ctx, boardID); err != nil {
303-
return err
304-
}
305-
306-
if _, err := db.GetEngine(ctx).Where(builder.Eq{
307-
"project_id": projectID,
308-
"`default`": true,
309-
}).Cols("`default`").Update(&Board{Default: false}); err != nil {
310-
return err
311-
}
289+
return db.WithTx(ctx, func(ctx context.Context) error {
290+
if _, err := GetBoard(ctx, boardID); err != nil {
291+
return err
292+
}
312293

313-
_, err := db.GetEngine(ctx).ID(boardID).Where(builder.Eq{"project_id": projectID}).
314-
Cols("`default`").Update(&Board{Default: true})
294+
if _, err := db.GetEngine(ctx).Where(builder.Eq{
295+
"project_id": projectID,
296+
"`default`": true,
297+
}).Cols("`default`").Update(&Board{Default: false}); err != nil {
298+
return err
299+
}
315300

316-
return err
301+
_, err := db.GetEngine(ctx).ID(boardID).
302+
Where(builder.Eq{"project_id": projectID}).
303+
Cols("`default`").Update(&Board{Default: true})
304+
return err
305+
})
317306
}
318307

319308
// UpdateBoardSorting update project board sorting
320309
func UpdateBoardSorting(ctx context.Context, bs BoardList) error {
321-
for i := range bs {
322-
_, err := db.GetEngine(ctx).ID(bs[i].ID).Cols(
323-
"sorting",
324-
).Update(bs[i])
325-
if err != nil {
326-
return err
310+
return db.WithTx(ctx, func(ctx context.Context) error {
311+
for i := range bs {
312+
if _, err := db.GetEngine(ctx).ID(bs[i].ID).Cols(
313+
"sorting",
314+
).Update(bs[i]); err != nil {
315+
return err
316+
}
327317
}
328-
}
329-
return nil
318+
return nil
319+
})
330320
}

‎models/project/board_test.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,12 @@ func TestGetDefaultBoard(t *testing.T) {
3131
board, err = projectWithMultipleDefaults.getDefaultBoard(db.DefaultContext)
3232
assert.NoError(t, err)
3333
assert.Equal(t, int64(6), board.ProjectID)
34-
assert.Equal(t, int64(8), board.ID)
34+
assert.Equal(t, int64(9), board.ID)
3535

36+
// set 8 as default board
37+
assert.NoError(t, SetDefaultBoard(db.DefaultContext, board.ProjectID, 8))
38+
39+
// then 9 will become a non-default board
3640
board, err = GetBoard(db.DefaultContext, 9)
3741
assert.NoError(t, err)
3842
assert.Equal(t, int64(6), board.ProjectID)

0 commit comments

Comments
 (0)