From 459b1b20fe18eff4df5de7c0ec0cf3ed390b75f2 Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Fri, 29 Mar 2024 00:14:30 +0800 Subject: [PATCH] Fix migration v292 (#30153) Fix https://github.com/go-gitea/gitea/pull/29874#discussion_r1542227686 - 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` (cherry picked from commit 40cdc84b368cce8328b4b49ea5ecf1c5fa040300) --- models/migrations/migrations.go | 2 + models/migrations/v1_22/v292.go | 84 +------------- models/migrations/v1_22/v293.go | 108 ++++++++++++++++++ .../v1_22/{v292_test.go => v293_test.go} | 6 +- models/project/board.go | 92 +++++++-------- models/project/board_test.go | 6 +- 6 files changed, 163 insertions(+), 135 deletions(-) create mode 100644 models/migrations/v1_22/v293.go rename models/migrations/v1_22/{v292_test.go => v293_test.go} (87%) diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 89ad456a3..0fa46a8d0 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -571,6 +571,8 @@ var migrations = []Migration{ // v291 -> v292 NewMigration("Add Index to attachment.comment_id", v1_22.AddCommentIDIndexofAttachment), // v292 -> v293 + NewMigration("Ensure every project has exactly one default column - No Op", noopMigration), + // v293 -> v294 NewMigration("Ensure every project has exactly one default column", v1_22.CheckProjectColumnsConsistency), } diff --git a/models/migrations/v1_22/v292.go b/models/migrations/v1_22/v292.go index 7c051a2b7..beca556ae 100644 --- a/models/migrations/v1_22/v292.go +++ b/models/migrations/v1_22/v292.go @@ -3,83 +3,7 @@ package v1_22 //nolint -import ( - "code.gitea.io/gitea/models/project" - "code.gitea.io/gitea/modules/setting" - - "xorm.io/builder" - "xorm.io/xorm" -) - -// CheckProjectColumnsConsistency ensures there is exactly one default board per project present -func CheckProjectColumnsConsistency(x *xorm.Engine) error { - sess := x.NewSession() - defer sess.Close() - - if err := sess.Begin(); err != nil { - return err - } - - limit := setting.Database.IterateBufferSize - if limit <= 0 { - limit = 50 - } - - start := 0 - - for { - var projects []project.Project - 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). - Limit(limit, start). - Find(&projects); err != nil { - return err - } - - if len(projects) == 0 { - break - } - start += len(projects) - - for _, p := range projects { - var boards []project.Board - if err := sess.Where("project_id=? AND `default` = ?", p.ID, true).OrderBy("sorting").Find(&boards); err != nil { - return err - } - - if len(boards) == 0 { - if _, err := sess.Insert(project.Board{ - ProjectID: p.ID, - Default: true, - Title: "Uncategorized", - CreatorID: p.CreatorID, - }); err != nil { - return err - } - continue - } - - var boardsToUpdate []int64 - for id, b := range boards { - if id > 0 { - boardsToUpdate = append(boardsToUpdate, b.ID) - } - } - - if _, err := sess.Where(builder.Eq{"project_id": p.ID}.And(builder.In("id", boardsToUpdate))). - Cols("`default`").Update(&project.Board{Default: false}); err != nil { - return err - } - } - - if start%1000 == 0 { - if err := sess.Commit(); err != nil { - return err - } - if err := sess.Begin(); err != nil { - return err - } - } - } - - return sess.Commit() -} +// NOTE: noop the original migration has bug which some projects will be skip, so +// these projects will have no default board. +// So that this migration will be skipped and go to v293.go +// This file is a placeholder so that readers can know what happened diff --git a/models/migrations/v1_22/v293.go b/models/migrations/v1_22/v293.go new file mode 100644 index 000000000..53cc71929 --- /dev/null +++ b/models/migrations/v1_22/v293.go @@ -0,0 +1,108 @@ +// Copyright 2024 The Gitea Authors. All rights reserved. +// SPDX-License-Identifier: MIT + +package v1_22 //nolint + +import ( + "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/timeutil" + + "xorm.io/xorm" +) + +// CheckProjectColumnsConsistency ensures there is exactly one default board per project present +func CheckProjectColumnsConsistency(x *xorm.Engine) error { + sess := x.NewSession() + defer sess.Close() + + limit := setting.Database.IterateBufferSize + if limit <= 0 { + limit = 50 + } + + type Project struct { + ID int64 + CreatorID int64 + BoardID int64 + } + + type ProjectBoard struct { + ID int64 `xorm:"pk autoincr"` + Title string + Default bool `xorm:"NOT NULL DEFAULT false"` // issues not assigned to a specific board will be assigned to this board + Sorting int8 `xorm:"NOT NULL DEFAULT 0"` + Color string `xorm:"VARCHAR(7)"` + + ProjectID int64 `xorm:"INDEX NOT NULL"` + CreatorID int64 `xorm:"NOT NULL"` + + CreatedUnix timeutil.TimeStamp `xorm:"INDEX created"` + UpdatedUnix timeutil.TimeStamp `xorm:"INDEX updated"` + } + + for { + if err := sess.Begin(); err != nil { + return err + } + + // all these projects without defaults will be fixed in the same loop, so + // we just need to always get projects without defaults until no such project + var projects []*Project + if err := sess.Select("project.id as id, project.creator_id, project_board.id as board_id"). + Join("LEFT", "project_board", "project_board.project_id = project.id AND project_board.`default`=?", true). + Where("project_board.id is NULL OR project_board.id = 0"). + Limit(limit). + Find(&projects); err != nil { + return err + } + + for _, p := range projects { + if _, err := sess.Insert(ProjectBoard{ + ProjectID: p.ID, + Default: true, + Title: "Uncategorized", + CreatorID: p.CreatorID, + }); err != nil { + return err + } + } + if err := sess.Commit(); err != nil { + return err + } + + if len(projects) == 0 { + break + } + } + sess.Close() + + return removeDuplicatedBoardDefault(x) +} + +func removeDuplicatedBoardDefault(x *xorm.Engine) error { + type ProjectInfo struct { + ProjectID int64 + DefaultNum int + } + var projects []ProjectInfo + if err := x.Select("project_id, count(*) AS default_num"). + Table("project_board"). + Where("`default` = ?", true). + GroupBy("project_id"). + Having("count(*) > 1"). + Find(&projects); err != nil { + return err + } + + for _, project := range projects { + if _, err := x.Where("project_id=?", project.ProjectID). + Table("project_board"). + Limit(project.DefaultNum - 1). + Update(map[string]bool{ + "`default`": false, + }); err != nil { + return err + } + } + return nil +} diff --git a/models/migrations/v1_22/v292_test.go b/models/migrations/v1_22/v293_test.go similarity index 87% rename from models/migrations/v1_22/v292_test.go rename to models/migrations/v1_22/v293_test.go index 5e32e0220..ccc92f39a 100644 --- a/models/migrations/v1_22/v292_test.go +++ b/models/migrations/v1_22/v293_test.go @@ -31,14 +31,14 @@ func Test_CheckProjectColumnsConsistency(t *testing.T) { assert.Equal(t, int64(1), defaultBoard.ProjectID) assert.True(t, defaultBoard.Default) - // check if multiple defaults were removed + // check if multiple defaults, previous were removed and last will be kept expectDefaultBoard, err := project.GetBoard(db.DefaultContext, 2) assert.NoError(t, err) assert.Equal(t, int64(2), expectDefaultBoard.ProjectID) - assert.True(t, expectDefaultBoard.Default) + assert.False(t, expectDefaultBoard.Default) expectNonDefaultBoard, err := project.GetBoard(db.DefaultContext, 3) assert.NoError(t, err) assert.Equal(t, int64(2), expectNonDefaultBoard.ProjectID) - assert.False(t, expectNonDefaultBoard.Default) + assert.True(t, expectNonDefaultBoard.Default) } diff --git a/models/project/board.go b/models/project/board.go index 5605f259b..5f142a356 100644 --- a/models/project/board.go +++ b/models/project/board.go @@ -209,7 +209,6 @@ func deleteBoardByProjectID(ctx context.Context, projectID int64) error { // GetBoard fetches the current board of a project func GetBoard(ctx context.Context, boardID int64) (*Board, error) { board := new(Board) - has, err := db.GetEngine(ctx).ID(boardID).Get(board) if err != nil { return nil, err @@ -260,71 +259,62 @@ func (p *Project) GetBoards(ctx context.Context) (BoardList, error) { // getDefaultBoard return default board and ensure only one exists func (p *Project) getDefaultBoard(ctx context.Context) (*Board, error) { - var boards []Board - if err := db.GetEngine(ctx).Where("project_id=? AND `default` = ?", p.ID, true).OrderBy("sorting").Find(&boards); err != nil { + var board Board + has, err := db.GetEngine(ctx). + Where("project_id=? AND `default` = ?", p.ID, true). + Desc("id").Get(&board) + if err != nil { return nil, err } - // create a default board if none is found - if len(boards) == 0 { - board := Board{ - ProjectID: p.ID, - Default: true, - Title: "Uncategorized", - CreatorID: p.CreatorID, - } - if _, err := db.GetEngine(ctx).Insert(); err != nil { - return nil, err - } + if has { return &board, nil } - // unset default boards where too many default boards exist - if len(boards) > 1 { - var boardsToUpdate []int64 - for id, b := range boards { - if id > 0 { - boardsToUpdate = append(boardsToUpdate, b.ID) - } - } - - if _, err := db.GetEngine(ctx).Where(builder.Eq{"project_id": p.ID}.And(builder.In("id", boardsToUpdate))). - Cols("`default`").Update(&Board{Default: false}); err != nil { - return nil, err - } + // create a default board if none is found + board = Board{ + ProjectID: p.ID, + Default: true, + Title: "Uncategorized", + CreatorID: p.CreatorID, } - - return &boards[0], nil + if _, err := db.GetEngine(ctx).Insert(&board); err != nil { + return nil, err + } + return &board, nil } // SetDefaultBoard represents a board for issues not assigned to one func SetDefaultBoard(ctx context.Context, projectID, boardID int64) error { - if _, err := GetBoard(ctx, boardID); err != nil { + return db.WithTx(ctx, func(ctx context.Context) error { + if _, err := GetBoard(ctx, boardID); err != nil { + return err + } + + if _, err := db.GetEngine(ctx).Where(builder.Eq{ + "project_id": projectID, + "`default`": true, + }).Cols("`default`").Update(&Board{Default: false}); err != nil { + return err + } + + _, err := db.GetEngine(ctx).ID(boardID). + Where(builder.Eq{"project_id": projectID}). + Cols("`default`").Update(&Board{Default: true}) return err - } - - if _, err := db.GetEngine(ctx).Where(builder.Eq{ - "project_id": projectID, - "`default`": true, - }).Cols("`default`").Update(&Board{Default: false}); err != nil { - return err - } - - _, err := db.GetEngine(ctx).ID(boardID).Where(builder.Eq{"project_id": projectID}). - Cols("`default`").Update(&Board{Default: true}) - - return err + }) } // UpdateBoardSorting update project board sorting func UpdateBoardSorting(ctx context.Context, bs BoardList) error { - for i := range bs { - _, err := db.GetEngine(ctx).ID(bs[i].ID).Cols( - "sorting", - ).Update(bs[i]) - if err != nil { - return err + return db.WithTx(ctx, func(ctx context.Context) error { + for i := range bs { + if _, err := db.GetEngine(ctx).ID(bs[i].ID).Cols( + "sorting", + ).Update(bs[i]); err != nil { + return err + } } - } - return nil + return nil + }) } diff --git a/models/project/board_test.go b/models/project/board_test.go index c1c6f0180..71ba29a58 100644 --- a/models/project/board_test.go +++ b/models/project/board_test.go @@ -31,8 +31,12 @@ func TestGetDefaultBoard(t *testing.T) { board, err = projectWithMultipleDefaults.getDefaultBoard(db.DefaultContext) assert.NoError(t, err) assert.Equal(t, int64(6), board.ProjectID) - assert.Equal(t, int64(8), board.ID) + assert.Equal(t, int64(9), board.ID) + // set 8 as default board + assert.NoError(t, SetDefaultBoard(db.DefaultContext, board.ProjectID, 8)) + + // then 9 will become a non-default board board, err = GetBoard(db.DefaultContext, 9) assert.NoError(t, err) assert.Equal(t, int64(6), board.ProjectID)