]> source.dussan.org Git - gitea.git/commitdiff
Fix migration v292 (#30153) v1.22.0-rc0
authorLunny Xiao <xiaolunwen@gmail.com>
Thu, 28 Mar 2024 16:14:30 +0000 (00:14 +0800)
committerGitHub <noreply@github.com>
Thu, 28 Mar 2024 16:14:30 +0000 (16:14 +0000)
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`

models/migrations/migrations.go
models/migrations/v1_22/v292.go
models/migrations/v1_22/v292_test.go [deleted file]
models/migrations/v1_22/v293.go [new file with mode: 0644]
models/migrations/v1_22/v293_test.go [new file with mode: 0644]
models/project/board.go
models/project/board_test.go

index 77895fba61899e3c203e0027f9323f56836d6cf8..0daa799ff675447b4f7fa6b68fa50258c2e2a014 100644 (file)
@@ -569,6 +569,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),
 }
 
index 7c051a2b753de1293a3c8d2fc6020b4e836763c8..beca556aee298aa2a8bb4d37aa1da9259b0b4d4e 100644 (file)
@@ -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/v292_test.go b/models/migrations/v1_22/v292_test.go
deleted file mode 100644 (file)
index 5e32e02..0000000
+++ /dev/null
@@ -1,44 +0,0 @@
-// Copyright 2024 The Gitea Authors. All rights reserved.
-// SPDX-License-Identifier: MIT
-
-package v1_22 //nolint
-
-import (
-       "testing"
-
-       "code.gitea.io/gitea/models/db"
-       "code.gitea.io/gitea/models/migrations/base"
-       "code.gitea.io/gitea/models/project"
-
-       "github.com/stretchr/testify/assert"
-)
-
-func Test_CheckProjectColumnsConsistency(t *testing.T) {
-       // Prepare and load the testing database
-       x, deferable := base.PrepareTestEnv(t, 0, new(project.Project), new(project.Board))
-       defer deferable()
-       if x == nil || t.Failed() {
-               return
-       }
-
-       assert.NoError(t, CheckProjectColumnsConsistency(x))
-
-       // check if default board was added
-       var defaultBoard project.Board
-       has, err := x.Where("project_id=? AND `default` = ?", 1, true).Get(&defaultBoard)
-       assert.NoError(t, err)
-       assert.True(t, has)
-       assert.Equal(t, int64(1), defaultBoard.ProjectID)
-       assert.True(t, defaultBoard.Default)
-
-       // check if multiple defaults were removed
-       expectDefaultBoard, err := project.GetBoard(db.DefaultContext, 2)
-       assert.NoError(t, err)
-       assert.Equal(t, int64(2), expectDefaultBoard.ProjectID)
-       assert.True(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)
-}
diff --git a/models/migrations/v1_22/v293.go b/models/migrations/v1_22/v293.go
new file mode 100644 (file)
index 0000000..53cc719
--- /dev/null
@@ -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/v293_test.go b/models/migrations/v1_22/v293_test.go
new file mode 100644 (file)
index 0000000..ccc92f3
--- /dev/null
@@ -0,0 +1,44 @@
+// Copyright 2024 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package v1_22 //nolint
+
+import (
+       "testing"
+
+       "code.gitea.io/gitea/models/db"
+       "code.gitea.io/gitea/models/migrations/base"
+       "code.gitea.io/gitea/models/project"
+
+       "github.com/stretchr/testify/assert"
+)
+
+func Test_CheckProjectColumnsConsistency(t *testing.T) {
+       // Prepare and load the testing database
+       x, deferable := base.PrepareTestEnv(t, 0, new(project.Project), new(project.Board))
+       defer deferable()
+       if x == nil || t.Failed() {
+               return
+       }
+
+       assert.NoError(t, CheckProjectColumnsConsistency(x))
+
+       // check if default board was added
+       var defaultBoard project.Board
+       has, err := x.Where("project_id=? AND `default` = ?", 1, true).Get(&defaultBoard)
+       assert.NoError(t, err)
+       assert.True(t, has)
+       assert.Equal(t, int64(1), defaultBoard.ProjectID)
+       assert.True(t, defaultBoard.Default)
+
+       // 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.False(t, expectDefaultBoard.Default)
+
+       expectNonDefaultBoard, err := project.GetBoard(db.DefaultContext, 3)
+       assert.NoError(t, err)
+       assert.Equal(t, int64(2), expectNonDefaultBoard.ProjectID)
+       assert.True(t, expectNonDefaultBoard.Default)
+}
index 5605f259b5005073fc9d3890d6da448880c96b00..5f142a356c68d916c8a32fc73f59ca644c5704b5 100644 (file)
@@ -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 err
-       }
-
-       if _, err := db.GetEngine(ctx).Where(builder.Eq{
-               "project_id": projectID,
-               "`default`":  true,
-       }).Cols("`default`").Update(&Board{Default: false}); err != nil {
-               return err
-       }
+       return db.WithTx(ctx, func(ctx context.Context) error {
+               if _, err := GetBoard(ctx, boardID); err != nil {
+                       return err
+               }
 
-       _, err := db.GetEngine(ctx).ID(boardID).Where(builder.Eq{"project_id": projectID}).
-               Cols("`default`").Update(&Board{Default: true})
+               if _, err := db.GetEngine(ctx).Where(builder.Eq{
+                       "project_id": projectID,
+                       "`default`":  true,
+               }).Cols("`default`").Update(&Board{Default: false}); err != nil {
+                       return err
+               }
 
-       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
+       })
 }
index c1c6f0180bd520f0c0504783c45eecfb6ce2767f..71ba29a5896dcbbd978726268fb8db5381cbf4f4 100644 (file)
@@ -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)