Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions pkg/db/test.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func AssertExists(t *testing.T, table string, values map[string]interface{}, cus
sql = sql[:len(sql)-5]
exists, err = x.SQL(sql).Get(&v)
} else {
exists, err = x.Table(table).Where(values).Get(&v)
exists, err = x.Table(table).Where(condFromValues(values)).Get(&v)
}
require.NoError(t, err, "Failed to assert entries exist in db")
if !exists {
Expand All @@ -106,10 +106,25 @@ func AssertExists(t *testing.T, table string, values map[string]interface{}, cus
}
}

// A nil value asserts the column is NULL — builder.Eq would render col = NULL, which never matches.
func condFromValues(values map[string]interface{}) builder.Cond {
cond := builder.NewCond()
for col, val := range values {
// builder does not quote identifiers, so reserved words like "limit" need it here.
quoted := x.Quote(col)
if val == nil {
cond = cond.And(builder.IsNull{quoted})
continue
}
cond = cond.And(builder.Eq{quoted: val})
}
return cond
}

// AssertMissing checks and asserts the nonexistence of certain entries in the db
func AssertMissing(t *testing.T, table string, values map[string]interface{}) {
all := []map[string]interface{}{}
err := x.Table(table).Where(values).Find(&all)
err := x.Table(table).Where(condFromValues(values)).Find(&all)
require.NoErrorf(t, err, "Failed to assert entries don't exist in db, error was: %s", err)

if len(all) > 0 {
Expand Down
73 changes: 73 additions & 0 deletions pkg/migration/20260903104500.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
// Vikunja is a to-do list application to facilitate your life.
// Copyright 2018-present Vikunja and contributors. All rights reserved.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

package migration

import (
"fmt"

"code.vikunja.io/api/pkg/db"

"src.techknowlogick.com/xormigrate"
"xorm.io/builder"
"xorm.io/xorm"
"xorm.io/xorm/schemas"
)

type projects20260903104500 struct {
ParentProjectID *int64 `xorm:"bigint INDEX null"`
}

func (projects20260903104500) TableName() string {
return "projects"
}

func rootProjectsParentToNull20260903104500(tx *xorm.Engine) error {
_, err := tx.
Where(builder.Eq{"parent_project_id": 0}).
Cols("parent_project_id").
Nullable("parent_project_id").
Update(&projects20260903104500{})
if err != nil {
return fmt.Errorf("could not set the parent of top-level projects to null: %w", err)
}

// MySQL has no partial indexes; it keeps using IDX_projects_parent_project_id.
if db.Type() == schemas.MYSQL {
return nil
}

// The recursive access CTE joins parent_project_id against a project id, which implies
// NOT NULL, so the planner can walk this index over the real children only instead of
// the full one where the root rows dominate.
_, err = tx.Exec("CREATE INDEX IF NOT EXISTS IDX_projects_parent_project_id_children ON projects (parent_project_id) WHERE parent_project_id IS NOT NULL")
if err != nil {
return fmt.Errorf("could not create the partial index on projects.parent_project_id: %w", err)
}

return nil
}

func init() {
migrations = append(migrations, &xormigrate.Migration{
ID: "20260903104500",
Description: "Store the parent of top-level projects as null and index only real children",
Migrate: rootProjectsParentToNull20260903104500,
Rollback: func(_ *xorm.Engine) error {
return nil
},
})
}
66 changes: 66 additions & 0 deletions pkg/migration/20260903104500_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Vikunja is a to-do list application to facilitate your life.
// Copyright 2018-present Vikunja and contributors. All rights reserved.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

package migration

import (
"testing"

"code.vikunja.io/api/pkg/db"

"github.com/stretchr/testify/require"
)

type projectsBefore20260903104500 struct {
ID int64 `xorm:"bigint autoincr not null unique pk"`
Title string `xorm:"varchar(250) not null"`
ParentProjectID *int64 `xorm:"bigint INDEX null"`
}

func (projectsBefore20260903104500) TableName() string {
return "projects"
}

func TestRootProjectsParentToNull20260903104500(t *testing.T) {
x, err := db.CreateTestEngine()
require.NoError(t, err)

table := projectsBefore20260903104500{}
t.Cleanup(func() {
require.NoError(t, x.DropTables(table))
})
require.NoError(t, x.DropTables(table))
require.NoError(t, x.Sync2(table)) //nolint:forbidigo // test-local table

root := int64(0)
child := int64(1)
_, err = x.Insert([]*projectsBefore20260903104500{
{ID: 1, Title: "root stored as 0", ParentProjectID: &root},
{ID: 2, Title: "root stored as null"},
{ID: 3, Title: "child", ParentProjectID: &child},
})
require.NoError(t, err)

require.NoError(t, rootProjectsParentToNull20260903104500(x))

projects := []*projectsBefore20260903104500{}
require.NoError(t, x.OrderBy("id").Find(&projects))
require.Len(t, projects, 3)
require.Nil(t, projects[0].ParentProjectID)
require.Nil(t, projects[1].ParentProjectID)
require.NotNil(t, projects[2].ParentProjectID)
require.Equal(t, child, *projects[2].ParentProjectID)
}
15 changes: 8 additions & 7 deletions pkg/models/project.go
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ func noParentProjectID() *int64 {
return Ptr(int64(0))
}

// AfterLoad normalizes a NULL parent_project_id — the column is nullable and rows
// predating it were never backfilled.
// AfterLoad normalizes a NULL parent_project_id — top-level projects are stored as
// NULL so the index on the column only covers real children.
func (p *Project) AfterLoad() {
if p.ParentProjectID == nil {
p.ParentProjectID = noParentProjectID()
Expand Down Expand Up @@ -957,13 +957,13 @@ func CreateProject(s *xorm.Session, project *Project, auth web.Auth, createBackl

project.HexColor = utils.NormalizeHex(project.HexColor)

// Persist top-level projects with an explicit 0 (not NULL) so the stored
// value and the serialized parent_project_id stay a plain number.
if project.ParentProjectID == nil {
project.ParentProjectID = Ptr(int64(0))
project.ParentProjectID = noParentProjectID()
}

_, err = s.Insert(project)
// Nullable maps the 0 sentinel to a stored NULL, keeping the index on
// parent_project_id to real children; the struct keeps the plain 0 the API returns.
_, err = s.Nullable("parent_project_id").Insert(project)
if err != nil {
return
}
Expand All @@ -981,7 +981,7 @@ func CreateProject(s *xorm.Session, project *Project, auth web.Auth, createBackl
}

project.Position = calculateDefaultPosition(project.ID, project.Position)
_, err = s.Where("id = ?", project.ID).Update(project)
_, err = s.Where("id = ?", project.ID).Nullable("parent_project_id").Update(project)
if err != nil {
return
}
Expand Down Expand Up @@ -1193,6 +1193,7 @@ func UpdateProject(s *xorm.Session, project *Project, auth web.Auth, updateProje
_, err = s.
ID(project.ID).
Cols(colsToUpdate...).
Nullable("parent_project_id").
Update(project)
if err != nil {
return err
Expand Down
2 changes: 2 additions & 0 deletions pkg/models/project_access.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ func (pa *projectAccess) permission(projectID int64) (Permission, bool) {
// can raise an inherited permission, never lower it. Binds the user id three times.
// tree uses UNION, not UNION ALL: deduplicating (id, permission) terminates on a
// parent_project_id cycle and caps the row count at three per project.
// The recursive step's join implies parent_project_id IS NOT NULL, which is why root
// projects store NULL: the partial index then covers real children only.
const projectAccessCTE = `
WITH RECURSIVE grants (project_id, permission) AS (
SELECT project_id, MAX(permission)
Expand Down
6 changes: 3 additions & 3 deletions pkg/models/project_repair.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ type RepairOrphanedProjectsResult struct {
}

// RepairOrphanedProjects finds projects whose parent_project_id references a
// project that no longer exists and sets their parent_project_id to 0,
// making them top-level projects.
// project that no longer exists and makes them top-level projects.
// If dryRun is true, it reports what would be fixed without making changes.
func RepairOrphanedProjects(s *xorm.Session, dryRun bool) (*RepairOrphanedProjectsResult, error) {
result := &RepairOrphanedProjectsResult{}
Expand Down Expand Up @@ -59,7 +58,8 @@ func RepairOrphanedProjects(s *xorm.Session, dryRun bool) (*RepairOrphanedProjec
p.ID, p.Title, p.parentID())
_, err = s.Where("id = ?", p.ID).
Cols("parent_project_id").
Update(&Project{ParentProjectID: Ptr(int64(0))})
Nullable("parent_project_id").
Update(&Project{ParentProjectID: noParentProjectID()})
if err != nil {
return result, err
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/models/project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestProject_CreateOrUpdate(t *testing.T) {
"id": project.ID,
"title": project.Title,
"description": project.Description,
"parent_project_id": 0,
"parent_project_id": nil,
}, false)
db.AssertExists(t, "project_views", map[string]interface{}{
"project_id": project.ID,
Expand Down Expand Up @@ -531,7 +531,7 @@ func TestProject_CreateOrUpdate(t *testing.T) {
require.NoError(t, err)
require.NoError(t, s.Commit())

db.AssertExists(t, "projects", map[string]interface{}{"id": 40, "parent_project_id": 0, "is_archived": true}, false)
db.AssertExists(t, "projects", map[string]interface{}{"id": 40, "parent_project_id": nil, "is_archived": true}, false)
})
t.Run("unarchive child under archived parent is rejected", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
Expand Down Expand Up @@ -589,7 +589,7 @@ func TestProject_CreateOrUpdate(t *testing.T) {
require.NoError(t, err)
require.NoError(t, s.Commit())

db.AssertExists(t, "projects", map[string]interface{}{"id": 21, "is_archived": false, "parent_project_id": 0}, false)
db.AssertExists(t, "projects", map[string]interface{}{"id": 21, "is_archived": false, "parent_project_id": nil}, false)
})
t.Run("unarchive child and move under unarchived parent", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
Expand Down
Loading