vikunja
/
vikunja
6
43
Fork 71
Code Issues 152 Pull Requests 16 Packages Releases 32 Activity

Add session handling for task attachments

This commit is contained in:
kolaente 2020-12-22 22:44:38 +01:00
parent 2742530766
commit 7fdc5fc293
Signed by: konrad
GPG Key ID: F40E70337AB24C9B
7 changed files with 50 additions and 44 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
pkg/models/list_duplicate.go
View File

@ -137,7 +137,7 @@ func (ld *ListDuplicate) Create(s *xorm.Session, a web.Auth) (err error) {
// Save all attachments
// We also duplicate all underlying files since they could be modified in one list which would result in
// file changes in the other list which is not something we want.
attachments, err := getTaskAttachmentsByTaskIDs(oldTaskIDs)
attachments, err := getTaskAttachmentsByTaskIDs(s, oldTaskIDs)
if err != nil {
return err
}
@ -163,7 +163,7 @@ func (ld *ListDuplicate) Create(s *xorm.Session, a web.Auth) (err error) {
return err
}
err := attachment.NewAttachment(attachment.File.File, attachment.File.Name, attachment.File.Size, a)
err := attachment.NewAttachment(s, attachment.File.File, attachment.File.Name, attachment.File.Size, a)
if err != nil {
return err
}

35
pkg/models/task_attachment.go
View File

@ -19,6 +19,7 @@ package models
import (
"io"
"time"
"xorm.io/xorm"
"code.vikunja.io/api/pkg/files"
"code.vikunja.io/api/pkg/user"
@ -49,7 +50,7 @@ func (TaskAttachment) TableName() string {
// NewAttachment creates a new task attachment
// Note: I'm not sure if only accepting an io.ReadCloser and not an afero.File or os.File instead is a good way of doing things.
func (ta *TaskAttachment) NewAttachment(f io.ReadCloser, realname string, realsize uint64, a web.Auth) error {
func (ta *TaskAttachment) NewAttachment(s *xorm.Session, f io.ReadCloser, realname string, realsize uint64, a web.Auth) error {
// Store the file
file, err := files.Create(f, realname, realsize, a)
@ -64,7 +65,7 @@ func (ta *TaskAttachment) NewAttachment(f io.ReadCloser, realname string, realsi
// Add an entry to the db
ta.FileID = file.ID
ta.CreatedByID = a.GetID()
_, err = x.Insert(ta)
_, err = s.Insert(ta)
if err != nil {
// remove the uploaded file if adding it to the db fails
if err2 := file.Delete(); err2 != nil {
@ -77,8 +78,8 @@ func (ta *TaskAttachment) NewAttachment(f io.ReadCloser, realname string, realsi
}
// ReadOne returns a task attachment
func (ta *TaskAttachment) ReadOne() (err error) {
exists, err := x.Where("id = ?", ta.ID).Get(ta)
func (ta *TaskAttachment) ReadOne(s *xorm.Session) (err error) {
exists, err := s.Where("id = ?", ta.ID).Get(ta)
if err != nil {
return
}
@ -110,12 +111,12 @@ func (ta *TaskAttachment) ReadOne() (err error) {
// @Failure 404 {object} models.Message "The task does not exist."
// @Failure 500 {object} models.Message "Internal error"
// @Router /tasks/{id}/attachments [get]
func (ta *TaskAttachment) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) {
func (ta *TaskAttachment) ReadAll(s *xorm.Session, a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) {
attachments := []*TaskAttachment{}
limit, start := getLimitFromPageIndex(page, perPage)
query := x.
query := s.
Where("task_id = ?", ta.TaskID)
if limit > 0 {
query = query.Limit(limit, start)
@ -133,13 +134,13 @@ func (ta *TaskAttachment) ReadAll(a web.Auth, search string, page int, perPage i
}
fs := make(map[int64]*files.File)
err = x.In("id", fileIDs).Find(&fs)
err = s.In("id", fileIDs).Find(&fs)
if err != nil {
return nil, 0, 0, err
}
us := make(map[int64]*user.User)
err = x.In("id", userIDs).Find(&us)
err = s.In("id", userIDs).Find(&us)
if err != nil {
return nil, 0, 0, err
}
@ -153,7 +154,7 @@ func (ta *TaskAttachment) ReadAll(a web.Auth, search string, page int, perPage i
r.CreatedBy = us[r.CreatedByID]
}
numberOfTotalItems, err = x.
numberOfTotalItems, err = s.
Where("task_id = ?", ta.TaskID).
Count(&TaskAttachment{})
return attachments, len(attachments), numberOfTotalItems, err
@ -173,15 +174,17 @@ func (ta *TaskAttachment) ReadAll(a web.Auth, search string, page int, perPage i
// @Failure 404 {object} models.Message "The task does not exist."
// @Failure 500 {object} models.Message "Internal error"
// @Router /tasks/{id}/attachments/{attachmentID} [delete]
func (ta *TaskAttachment) Delete() error {
func (ta *TaskAttachment) Delete(s *xorm.Session) error {
// Load the attachment
err := ta.ReadOne()
err := ta.ReadOne(s)
if err != nil && !files.IsErrFileDoesNotExist(err) {
return err
}
// Delete it
_, err = x.Where("task_id = ? AND id = ?", ta.TaskID, ta.ID).Delete(ta)
_, err = s.
Where("task_id = ? AND id = ?", ta.TaskID, ta.ID).
Delete(ta)
if err != nil {
return err
}
@ -195,9 +198,9 @@ func (ta *TaskAttachment) Delete() error {
return err
}
func getTaskAttachmentsByTaskIDs(taskIDs []int64) (attachments []*TaskAttachment, err error) {
func getTaskAttachmentsByTaskIDs(s *xorm.Session, taskIDs []int64) (attachments []*TaskAttachment, err error) {
attachments = []*TaskAttachment{}
err = x.
err = s.
In("task_id", taskIDs).
Find(&attachments)
if err != nil {
@ -213,13 +216,13 @@ func getTaskAttachmentsByTaskIDs(taskIDs []int64) (attachments []*TaskAttachment
// Get all files
fs := make(map[int64]*files.File)
err = x.In("id", fileIDs).Find(&fs)
err = s.In("id", fileIDs).Find(&fs)
if err != nil {
return
}
users := make(map[int64]*user.User)
err = x.In("id", userIDs).Find(&users)
err = s.In("id", userIDs).Find(&users)
if err != nil {
return
}

11
pkg/models/task_attachment_rights.go
View File

@ -16,22 +16,25 @@
package models
import "code.vikunja.io/web"
import (
"code.vikunja.io/web"
"xorm.io/xorm"
)
// CanRead checks if the user can see an attachment
func (ta *TaskAttachment) CanRead(a web.Auth) (bool, int, error) {
func (ta *TaskAttachment) CanRead(s *xorm.Session, a web.Auth) (bool, int, error) {
t := &Task{ID: ta.TaskID}
return t.CanRead(a)
}
// CanDelete checks if the user can delete an attachment
func (ta *TaskAttachment) CanDelete(a web.Auth) (bool, error) {
func (ta *TaskAttachment) CanDelete(s *xorm.Session, a web.Auth) (bool, error) {
t := &Task{ID: ta.TaskID}
return t.CanWrite(a)
}
// CanCreate checks if the user can create an attachment
func (ta *TaskAttachment) CanCreate(a web.Auth) (bool, error) {
func (ta *TaskAttachment) CanCreate(s *xorm.Session, a web.Auth) (bool, error) {
t, err := GetTaskByIDSimple(ta.TaskID)
if err != nil {
return false, err

32
pkg/models/task_attachment_test.go
View File

@ -37,7 +37,7 @@ func TestTaskAttachment_ReadOne(t *testing.T) {
ta := &TaskAttachment{
ID: 1,
}
err := ta.ReadOne()
err := ta.ReadOne(s)
assert.NoError(t, err)
assert.NotNil(t, ta.File)
assert.True(t, ta.File.ID == ta.FileID && ta.FileID != 0)
@ -58,7 +58,7 @@ func TestTaskAttachment_ReadOne(t *testing.T) {
ta := &TaskAttachment{
ID: 9999,
}
err := ta.ReadOne()
err := ta.ReadOne(s)
assert.Error(t, err)
assert.True(t, IsErrTaskAttachmentDoesNotExist(err))
})
@ -68,7 +68,7 @@ func TestTaskAttachment_ReadOne(t *testing.T) {
ta := &TaskAttachment{
ID: 2,
}
err := ta.ReadOne()
err := ta.ReadOne(s)
assert.Error(t, err)
assert.EqualError(t, err, "file 9999 does not exist")
})
@ -104,7 +104,7 @@ func TestTaskAttachment_NewAttachment(t *testing.T) {
}
testuser := &user.User{ID: 1}
err := ta.NewAttachment(tf, "testfile", 100, testuser)
err := ta.NewAttachment(s, tf, "testfile", 100, testuser)
assert.NoError(t, err)
assert.NotEqual(t, 0, ta.FileID)
_, err = files.FileStat("files/" + strconv.FormatInt(ta.FileID, 10))
@ -127,7 +127,7 @@ func TestTaskAttachment_ReadAll(t *testing.T) {
db.LoadAndAssertFixtures(t)
files.InitTestFileFixtures(t)
ta := &TaskAttachment{TaskID: 1}
as, _, _, err := ta.ReadAll(&user.User{ID: 1}, "", 0, 50)
as, _, _, err := ta.ReadAll(s, &user.User{ID: 1}, "", 0, 50)
attachments, _ := as.([]*TaskAttachment)
assert.NoError(t, err)
assert.Len(t, attachments, 2)
@ -139,7 +139,7 @@ func TestTaskAttachment_Delete(t *testing.T) {
files.InitTestFileFixtures(t)
t.Run("Normal", func(t *testing.T) {
ta := &TaskAttachment{ID: 1}
err := ta.Delete()
err := ta.Delete(s)
assert.NoError(t, err)
// Check if the file itself was deleted
_, err = files.FileStat("/1") // The new file has the id 2 since it's the second attachment
@ -148,14 +148,14 @@ func TestTaskAttachment_Delete(t *testing.T) {
t.Run("Nonexisting", func(t *testing.T) {
files.InitTestFileFixtures(t)
ta := &TaskAttachment{ID: 9999}
err := ta.Delete()
err := ta.Delete(s)
assert.Error(t, err)
assert.True(t, IsErrTaskAttachmentDoesNotExist(err))
})
t.Run("Existing attachment, nonexisting file", func(t *testing.T) {
files.InitTestFileFixtures(t)
ta := &TaskAttachment{ID: 2}
err := ta.Delete()
err := ta.Delete(s)
assert.NoError(t, err)
})
}
@ -166,14 +166,14 @@ func TestTaskAttachment_Rights(t *testing.T) {
t.Run("Allowed", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 1}
can, _, err := ta.CanRead(u)
can, _, err := ta.CanRead(s, u)
assert.NoError(t, err)
assert.True(t, can)
})
t.Run("Forbidden", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 14}
can, _, err := ta.CanRead(u)
can, _, err := ta.CanRead(s, u)
assert.NoError(t, err)
assert.False(t, can)
})
@ -182,21 +182,21 @@ func TestTaskAttachment_Rights(t *testing.T) {
t.Run("Allowed", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 1}
can, err := ta.CanDelete(u)
can, err := ta.CanDelete(s, u)
assert.NoError(t, err)
assert.True(t, can)
})
t.Run("Forbidden, no access", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 14}
can, err := ta.CanDelete(u)
can, err := ta.CanDelete(s, u)
assert.NoError(t, err)
assert.False(t, can)
})
t.Run("Forbidden, shared read only", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 15}
can, err := ta.CanDelete(u)
can, err := ta.CanDelete(s, u)
assert.NoError(t, err)
assert.False(t, can)
})
@ -205,21 +205,21 @@ func TestTaskAttachment_Rights(t *testing.T) {
t.Run("Allowed", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 1}
can, err := ta.CanCreate(u)
can, err := ta.CanCreate(s, u)
assert.NoError(t, err)
assert.True(t, can)
})
t.Run("Forbidden, no access", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 14}
can, err := ta.CanCreate(u)
can, err := ta.CanCreate(s, u)
assert.NoError(t, err)
assert.False(t, can)
})
t.Run("Forbidden, shared read only", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
ta := &TaskAttachment{TaskID: 15}
can, err := ta.CanCreate(u)
can, err := ta.CanCreate(s, u)
assert.NoError(t, err)
assert.False(t, can)
})

2
pkg/models/tasks.go
View File

@ -557,7 +557,7 @@ func addLabelsToTasks(taskIDs []int64, taskMap map[int64]*Task) (err error) {
// Get task attachments
func addAttachmentsToTasks(taskIDs []int64, taskMap map[int64]*Task) (err error) {
attachments, err := getTaskAttachmentsByTaskIDs(taskIDs)
attachments, err := getTaskAttachmentsByTaskIDs(s, taskIDs)
if err != nil {
return
}

2
pkg/modules/migration/create_from_structure.go
View File

@ -164,7 +164,7 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
if len(a.File.FileContent) > 0 {
a.TaskID = t.ID
fr := ioutil.NopCloser(bytes.NewReader(a.File.FileContent))
err = a.NewAttachment(fr, a.File.Name, a.File.Size, user)
err = a.NewAttachment(s, fr, a.File.Name, a.File.Size, user)
if err != nil {
return
}

8
pkg/routes/api/v1/task_attachment.go
View File

@ -52,7 +52,7 @@ func UploadTaskAttachment(c echo.Context) error {
return handler.HandleHTTPError(err, c)
}
can, err := taskAttachment.CanCreate(auth)
can, err := taskAttachment.CanCreate(s, auth)
if err != nil {
return handler.HandleHTTPError(err, c)
}
@ -85,7 +85,7 @@ func UploadTaskAttachment(c echo.Context) error {
}
defer f.Close()
err = ta.NewAttachment(f, file.Filename, uint64(file.Size), auth)
err = ta.NewAttachment(s, f, file.Filename, uint64(file.Size), auth)
if err != nil {
r.Errors = append(r.Errors, handler.HandleHTTPError(err, c))
continue
@ -121,7 +121,7 @@ func GetTaskAttachment(c echo.Context) error {
if err != nil {
return handler.HandleHTTPError(err, c)
}
can, _, err := taskAttachment.CanRead(auth)
can, _, err := taskAttachment.CanRead(s, auth)
if err != nil {
return handler.HandleHTTPError(err, c)
}
@ -130,7 +130,7 @@ func GetTaskAttachment(c echo.Context) error {
}
// Get the attachment incl file
err = taskAttachment.ReadOne()
err = taskAttachment.ReadOne(s)
if err != nil {
return handler.HandleHTTPError(err, c)
}