fix(background): when list background is removed, delete file from file system and DB
continuous-integration/drone/pr Build is passing
Details
continuous-integration/drone/pr Build is passing
Details
Currently, if a list background is removed, the background file stays in disk and in the DB forever.
Fix this by deleting the file during background removal, similar to how it's done when a new background is uploaded: [source](726a517bec/pkg/modules/background/upload/upload.go (L56-L62)
)
Test Plan:
A) Before these changes
A.1) Create a new list
A.2) Set a new background to the list
A.3) Check file is in disk and DB:
```
$ ls files/
1
$ sqlite3 vikunja.db
SQLite version 3.40.1 2022-12-28 14:03:47
Enter ".help" for usage hints.
sqlite> select * from files;
1|20220410_180833.jpg||2822966|2023-01-25 21:25:28|1
sqlite> select * from lists;
1|Teste||||1|1|0|1|LeGlCmIqR*of_4R+WCoe4;s.oLjZ|65536.0|2023-01-25 21:25:21|2023-01-25 21:25:28
```
A.4) Remove background from list
A.5) Check file is still in disk and DB:
```
$ ls files/
1
$ sqlite3 vikunja.db
SQLite version 3.40.1 2022-12-28 14:03:47
Enter ".help" for usage hints.
sqlite> select * from files;
1|20220410_180833.jpg||2822966|2023-01-25 21:25:28|1
sqlite> select * from lists;
1|Teste||||1|1|0|0||65536.0|2023-01-25 21:25:21|2023-01-25 21:32:39
```
B) After these changes
B.1) Set background again
B.2) Check files in disk and DB:
```
$ ls files/
1 2
$ sqlite3 vikunja.db
SQLite version 3.40.1 2022-12-28 14:03:47
Enter ".help" for usage hints.
sqlite> select * from files;
1|20220410_180833.jpg||2822966|2023-01-25 21:25:28|1
2|20220410_180833.jpg||2822966|2023-01-25 21:34:33|1
sqlite> select * from lists;
1|Teste||||1|1|0|2|LeGlCmIqR*of_4R+WCoe4;s.oLjZ|65536.0|2023-01-25 21:25:21|2023-01-25 21:34:33
```
B.3) Remove background image
B.4) Check files and DB again, verify image 2 has been deleted:
```
$ ls files/
1
$ sqlite3 vikunja.db
SQLite version 3.40.1 2022-12-28 14:03:47
Enter ".help" for usage hints.
sqlite> select * from files;
1|20220410_180833.jpg||2822966|2023-01-25 21:25:28|1
sqlite> select * from lists;
1|Teste||||1|1|0|0||65536.0|2023-01-25 21:25:21|2023-01-25 21:36:13
```
This commit is contained in:
parent
75baba32a6
commit
d74fdb35d1
|
@ -242,3 +242,14 @@
|
|||
position: 7
|
||||
updated: 2018-12-02 15:13:12
|
||||
created: 2018-12-01 15:13:12
|
||||
-
|
||||
id: 25
|
||||
title: Test25 with background
|
||||
description: Lorem Ipsum
|
||||
identifier: test6
|
||||
owner_id: 6
|
||||
namespace_id: 6
|
||||
background_file_id: 1
|
||||
position: 8
|
||||
updated: 2018-12-02 15:13:12
|
||||
created: 2018-12-01 15:13:12
|
||||
|
|
|
@ -140,7 +140,7 @@ func (f *File) Delete() (err error) {
|
|||
var perr *os.PathError
|
||||
if errors.As(err, &perr) {
|
||||
// Don't fail when removing the file failed
|
||||
log.Errorf("Error deleting file %d: %w", err)
|
||||
log.Errorf("Error deleting file %d: %w", f.ID, err)
|
||||
return s.Commit()
|
||||
}
|
||||
|
||||
|
|
|
@ -795,6 +795,11 @@ func (l *List) Create(s *xorm.Session, a web.Auth) (err error) {
|
|||
// @Router /lists/{id} [delete]
|
||||
func (l *List) Delete(s *xorm.Session, a web.Auth) (err error) {
|
||||
|
||||
fullList, err := GetListSimpleByID(s, l.ID)
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
// Delete the list
|
||||
_, err = s.ID(l.ID).Delete(&List{})
|
||||
if err != nil {
|
||||
|
@ -815,12 +820,28 @@ func (l *List) Delete(s *xorm.Session, a web.Auth) (err error) {
|
|||
}
|
||||
}
|
||||
|
||||
err = fullList.DeleteBackgroundFileIfExists()
|
||||
if err != nil {
|
||||
return
|
||||
}
|
||||
|
||||
return events.Dispatch(&ListDeletedEvent{
|
||||
List: l,
|
||||
Doer: a,
|
||||
})
|
||||
}
|
||||
|
||||
// DeleteBackgroundFileIfExists deletes the list's background file from the db and the filesystem,
|
||||
// if one exists
|
||||
func (l *List) DeleteBackgroundFileIfExists() (err error) {
|
||||
if l.BackgroundFileID == 0 {
|
||||
return
|
||||
}
|
||||
|
||||
file := files.File{ID: l.BackgroundFileID}
|
||||
return file.Delete()
|
||||
}
|
||||
|
||||
// SetListBackground sets a background file as list background in the db
|
||||
func SetListBackground(s *xorm.Session, listID int64, background *files.File, blurHash string) (err error) {
|
||||
l := &List{
|
||||
|
|
|
@ -21,6 +21,7 @@ import (
|
|||
"testing"
|
||||
|
||||
"code.vikunja.io/api/pkg/db"
|
||||
"code.vikunja.io/api/pkg/files"
|
||||
"code.vikunja.io/api/pkg/user"
|
||||
"github.com/stretchr/testify/assert"
|
||||
)
|
||||
|
@ -246,17 +247,76 @@ func TestList_CreateOrUpdate(t *testing.T) {
|
|||
}
|
||||
|
||||
func TestList_Delete(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
list := List{
|
||||
ID: 1,
|
||||
}
|
||||
err := list.Delete(s, &user.User{ID: 1})
|
||||
assert.NoError(t, err)
|
||||
err = s.Commit()
|
||||
assert.NoError(t, err)
|
||||
db.AssertMissing(t, "lists", map[string]interface{}{
|
||||
"id": 1,
|
||||
t.Run("normal", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
s := db.NewSession()
|
||||
list := List{
|
||||
ID: 1,
|
||||
}
|
||||
err := list.Delete(s, &user.User{ID: 1})
|
||||
assert.NoError(t, err)
|
||||
err = s.Commit()
|
||||
assert.NoError(t, err)
|
||||
db.AssertMissing(t, "lists", map[string]interface{}{
|
||||
"id": 1,
|
||||
})
|
||||
})
|
||||
t.Run("with background", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
files.InitTestFileFixtures(t)
|
||||
s := db.NewSession()
|
||||
list := List{
|
||||
ID: 25,
|
||||
}
|
||||
err := list.Delete(s, &user.User{ID: 6})
|
||||
assert.NoError(t, err)
|
||||
err = s.Commit()
|
||||
assert.NoError(t, err)
|
||||
db.AssertMissing(t, "lists", map[string]interface{}{
|
||||
"id": 25,
|
||||
})
|
||||
db.AssertMissing(t, "files", map[string]interface{}{
|
||||
"id": 1,
|
||||
})
|
||||
})
|
||||
}
|
||||
|
||||
func TestList_DeleteBackgroundFileIfExists(t *testing.T) {
|
||||
t.Run("list with background", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
files.InitTestFileFixtures(t)
|
||||
s := db.NewSession()
|
||||
file := &files.File{ID: 1}
|
||||
list := List{
|
||||
ID: 1,
|
||||
BackgroundFileID: file.ID,
|
||||
}
|
||||
err := SetListBackground(s, list.ID, file, "")
|
||||
assert.NoError(t, err)
|
||||
err = list.DeleteBackgroundFileIfExists()
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
t.Run("list with invalid background", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
files.InitTestFileFixtures(t)
|
||||
s := db.NewSession()
|
||||
file := &files.File{ID: 9999}
|
||||
list := List{
|
||||
ID: 1,
|
||||
BackgroundFileID: file.ID,
|
||||
}
|
||||
err := SetListBackground(s, list.ID, file, "")
|
||||
assert.NoError(t, err)
|
||||
err = list.DeleteBackgroundFileIfExists()
|
||||
assert.Error(t, err)
|
||||
assert.True(t, files.IsErrFileDoesNotExist(err))
|
||||
})
|
||||
t.Run("list without background", func(t *testing.T) {
|
||||
db.LoadAndAssertFixtures(t)
|
||||
files.InitTestFileFixtures(t)
|
||||
list := List{ID: 1}
|
||||
err := list.DeleteBackgroundFileIfExists()
|
||||
assert.NoError(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
|
|
|
@ -341,6 +341,11 @@ func RemoveListBackground(c echo.Context) error {
|
|||
return err
|
||||
}
|
||||
|
||||
err = list.DeleteBackgroundFileIfExists()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
list.BackgroundFileID = 0
|
||||
list.BackgroundInformation = nil
|
||||
list.BackgroundBlurHash = ""
|
||||
|
|
|
@ -54,11 +54,9 @@ func (p *Provider) Search(s *xorm.Session, search string, page int64) (result []
|
|||
// @Router /lists/{id}/backgrounds/upload [put]
|
||||
func (p *Provider) Set(s *xorm.Session, img *background.Image, list *models.List, auth web.Auth) (err error) {
|
||||
// Remove the old background if one exists
|
||||
if list.BackgroundFileID != 0 {
|
||||
file := files.File{ID: list.BackgroundFileID}
|
||||
if err := file.Delete(); err != nil {
|
||||
return err
|
||||
}
|
||||
err = list.DeleteBackgroundFileIfExists()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
file := &files.File{}
|
||||
|
|
Loading…
Reference in New Issue