From 8ee689add4e4abbd065c95eb62f50d4800bdc8ae Mon Sep 17 00:00:00 2001 From: kolaente Date: Wed, 4 Nov 2026 13:34:55 +0100 Subject: [PATCH] fix(backgrounds): clear backgrounds that point at files used elsewhere A migration clears the background of projects whose background file is also a task attachment, an avatar, a data export, a pending import upload or another project's background. Of projects sharing a file, the oldest keeps it. The Unsplash and upload providers now remove the old background through DeleteBackgroundFileIfExists. --- pkg/migration/20261105115436.go | 70 ++++++++++++ pkg/migration/20261105115436_test.go | 115 ++++++++++++++++++++ pkg/modules/background/unsplash/unsplash.go | 14 +-- pkg/modules/background/upload/upload.go | 9 +- 4 files changed, 189 insertions(+), 19 deletions(-) create mode 100644 pkg/migration/20261105115436.go create mode 100644 pkg/migration/20261105115436_test.go diff --git a/pkg/migration/20261105115436.go b/pkg/migration/20261105115436.go new file mode 100644 index 000000000..3482b4ff6 --- /dev/null +++ b/pkg/migration/20261105115436.go @@ -0,0 +1,70 @@ +// 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 . + +package migration + +import ( + "code.vikunja.io/api/pkg/log" + + "src.techknowlogick.com/xormigrate" + "xorm.io/builder" + "xorm.io/xorm" +) + +// Of projects sharing a background file, the oldest keeps it. +var sharedBackgroundCond20261105115436 = builder.And( + builder.Gt{"background_file_id": 0}, + builder.Or( + builder.In("background_file_id", builder.Select("file_id").From("task_attachments")), + builder.In("background_file_id", builder.Select("avatar_file_id").From("users")), + builder.In("background_file_id", builder.Select("export_file_id").From("users")), + builder.In("background_file_id", builder.Select("upload_file_id").From("migration_status")), + builder.Expr("EXISTS (SELECT 1 FROM projects other WHERE other.background_file_id = projects.background_file_id AND other.id < projects.id)"), + ), +) + +func init() { + migrations = append(migrations, &xormigrate.Migration{ + ID: "20261105115436", + Description: "Clear project backgrounds that point at files used elsewhere", + Migrate: func(tx *xorm.Engine) error { + ids := []int64{} + if err := tx.Table("projects").Cols("id").Where(sharedBackgroundCond20261105115436).Find(&ids); err != nil { + return err + } + if len(ids) == 0 { + return nil + } + + // Usually none; any hit means the background was set to someone else's file. + log.Warningf("Clearing the background of %d projects whose background file is used elsewhere: %v", len(ids), ids) + for start := 0; start < len(ids); start += 500 { + chunk := ids[start:min(start+500, len(ids))] + _, err := tx.Table("projects").In("id", chunk).Update(map[string]any{ + "background_file_id": 0, + "background_blur_hash": "", + }) + if err != nil { + return err + } + } + return nil + }, + Rollback: func(tx *xorm.Engine) error { + return nil + }, + }) +} diff --git a/pkg/migration/20261105115436_test.go b/pkg/migration/20261105115436_test.go new file mode 100644 index 000000000..fab5925b0 --- /dev/null +++ b/pkg/migration/20261105115436_test.go @@ -0,0 +1,115 @@ +// 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 . + +package migration + +import ( + "slices" + "testing" + + "code.vikunja.io/api/pkg/db" + + "github.com/stretchr/testify/require" + "src.techknowlogick.com/xormigrate" +) + +type project20261105115436 struct { + ID int64 `xorm:"bigint autoincr not null unique pk"` + BackgroundFileID int64 `xorm:"null"` + BackgroundBlurHash string `xorm:"varchar(50) null"` +} + +func (project20261105115436) TableName() string { return "projects" } + +type taskAttachment20261105115436 struct { + ID int64 `xorm:"bigint autoincr not null unique pk"` + FileID int64 `xorm:"bigint not null"` +} + +func (taskAttachment20261105115436) TableName() string { return "task_attachments" } + +type user20261105115436 struct { + ID int64 `xorm:"bigint autoincr not null unique pk"` + AvatarFileID int64 `xorm:"null"` + ExportFileID int64 `xorm:"bigint null"` +} + +func (user20261105115436) TableName() string { return "users" } + +type migrationStatus20261105115436 struct { + ID int64 `xorm:"bigint autoincr not null unique pk"` + UploadFileID *int64 `xorm:"bigint null"` +} + +func (migrationStatus20261105115436) TableName() string { return "migration_status" } + +func TestClearSharedBackgrounds20261105115436(t *testing.T) { + x, err := db.CreateTestEngine() + require.NoError(t, err) + + tables := []any{ + project20261105115436{}, + taskAttachment20261105115436{}, + user20261105115436{}, + migrationStatus20261105115436{}, + } + t.Cleanup(func() { + require.NoError(t, x.DropTables(tables...)) + }) + require.NoError(t, x.DropTables(tables...)) + require.NoError(t, x.Sync2(tables...)) + + upload := int64(14) + _, err = x.Insert( + &taskAttachment20261105115436{ID: 1, FileID: 11}, + &user20261105115436{ID: 1, AvatarFileID: 12, ExportFileID: 13}, + &migrationStatus20261105115436{ID: 1, UploadFileID: &upload}, + &migrationStatus20261105115436{ID: 2}, + &project20261105115436{ID: 1, BackgroundFileID: 11, BackgroundBlurHash: "a"}, + &project20261105115436{ID: 2, BackgroundFileID: 12, BackgroundBlurHash: "b"}, + &project20261105115436{ID: 3, BackgroundFileID: 13, BackgroundBlurHash: "c"}, + &project20261105115436{ID: 4, BackgroundFileID: 14, BackgroundBlurHash: "d"}, + &project20261105115436{ID: 5, BackgroundFileID: 15, BackgroundBlurHash: "e"}, + &project20261105115436{ID: 6, BackgroundFileID: 15, BackgroundBlurHash: "f"}, + &project20261105115436{ID: 7, BackgroundFileID: 16, BackgroundBlurHash: "g"}, + &project20261105115436{ID: 8}, + ) + require.NoError(t, err) + + idx := slices.IndexFunc(migrations, func(m *xormigrate.Migration) bool { return m.ID == "20261105115436" }) + require.NotEqual(t, -1, idx) + require.NoError(t, migrations[idx].Migrate(x)) + + projects := []*project20261105115436{} + require.NoError(t, x.OrderBy("id").Find(&projects)) + got := map[int64]int64{} + for _, p := range projects { + got[p.ID] = p.BackgroundFileID + if p.BackgroundFileID == 0 { + require.Emptyf(t, p.BackgroundBlurHash, "project %d kept its blur hash", p.ID) + } + } + require.Equal(t, map[int64]int64{ + 1: 0, // task attachment + 2: 0, // avatar + 3: 0, // data export + 4: 0, // pending import upload + 5: 15, // oldest of two projects sharing a file + 6: 0, + 7: 16, // own file + 8: 0, + }, got) +} diff --git a/pkg/modules/background/unsplash/unsplash.go b/pkg/modules/background/unsplash/unsplash.go index 64222fb1d..421dfbdc6 100644 --- a/pkg/modules/background/unsplash/unsplash.go +++ b/pkg/modules/background/unsplash/unsplash.go @@ -333,18 +333,8 @@ func (p *Provider) Set(s *xorm.Session, image *background.Image, project *models return } - // Remove the old background if one exists - if project.BackgroundFileID != 0 { - file := files.File{ID: project.BackgroundFileID} - err = file.Delete(s) - if err != nil && !files.IsErrFileDoesNotExist(err) { - return err - } - - err = models.RemoveUnsplashPhoto(s, project.BackgroundFileID) - if err != nil && !files.IsErrFileDoesNotExist(err) { - return err - } + if err := project.DeleteBackgroundFileIfExists(s); err != nil { + return err } // Save the relation that we got it from unsplash diff --git a/pkg/modules/background/upload/upload.go b/pkg/modules/background/upload/upload.go index 2cb471874..9f490d8a2 100644 --- a/pkg/modules/background/upload/upload.go +++ b/pkg/modules/background/upload/upload.go @@ -54,13 +54,8 @@ func (p *Provider) Search(_ *xorm.Session, _ string, _ int64) (result []*backgro // @Failure 500 {object} models.Message "Internal error" // @Router /projects/{id}/backgrounds/upload [put] func (p *Provider) Set(s *xorm.Session, img *background.Image, project *models.Project, _ web.Auth) (err error) { - // Remove the old background if one exists - if project.BackgroundFileID != 0 { - file := files.File{ID: project.BackgroundFileID} - err := file.Delete(s) - if err != nil && !files.IsErrFileDoesNotExist(err) { - return err - } + if err := project.DeleteBackgroundFileIfExists(s); err != nil { + return err } file := &files.File{}