From 025697f286d644ea5e4227f801c07659f61ba1cb Mon Sep 17 00:00:00 2001 From: kolaente Date: Tue, 22 Dec 2020 16:45:55 +0100 Subject: [PATCH] Add session handling for lists and related entities --- pkg/models/bulk_task.go | 2 +- pkg/models/kanban_rights.go | 4 +- pkg/models/label_task.go | 2 +- pkg/models/link_sharing.go | 8 +- pkg/models/link_sharing_rights.go | 20 +-- pkg/models/list.go | 143 ++++++++----------- pkg/models/list_duplicate.go | 4 +- pkg/models/list_rights.go | 47 +++--- pkg/models/list_team.go | 12 +- pkg/models/list_team_rights.go | 2 +- pkg/models/list_users.go | 12 +- pkg/models/list_users_rights.go | 2 +- pkg/models/namespace.go | 34 ++--- pkg/models/saved_filters.go | 9 +- pkg/models/task_assignees.go | 11 +- pkg/models/task_assignees_rights.go | 2 +- pkg/models/task_collection.go | 5 +- pkg/models/tasks.go | 14 +- pkg/models/tasks_rights.go | 8 +- pkg/models/unsplash.go | 9 +- pkg/modules/background/handler/background.go | 6 +- pkg/routes/caldav/listStorageProvider.go | 2 +- 22 files changed, 166 insertions(+), 192 deletions(-) diff --git a/pkg/models/bulk_task.go b/pkg/models/bulk_task.go index d5541ff90..9ec3b10ac 100644 --- a/pkg/models/bulk_task.go +++ b/pkg/models/bulk_task.go @@ -61,7 +61,7 @@ func (bt *BulkTask) CanUpdate(a web.Auth) (bool, error) { // A user can update an task if he has write acces to its list l := &List{ID: bt.Tasks[0].ListID} - return l.CanWrite(a) + return l.CanWrite(nil, a) } // Update updates a bunch of tasks at once diff --git a/pkg/models/kanban_rights.go b/pkg/models/kanban_rights.go index 83881f1e6..cd0987863 100644 --- a/pkg/models/kanban_rights.go +++ b/pkg/models/kanban_rights.go @@ -21,7 +21,7 @@ import "code.vikunja.io/web" // CanCreate checks if a user can create a new bucket func (b *Bucket) CanCreate(a web.Auth) (bool, error) { l := &List{ID: b.ListID} - return l.CanWrite(a) + return l.CanWrite(nil, a) } // CanUpdate checks if a user can update an existing bucket @@ -41,5 +41,5 @@ func (b *Bucket) canDoBucket(a web.Auth) (bool, error) { return false, err } l := &List{ID: bb.ListID} - return l.CanWrite(a) + return l.CanWrite(nil, a) } diff --git a/pkg/models/label_task.go b/pkg/models/label_task.go index b631ed83e..103710e05 100644 --- a/pkg/models/label_task.go +++ b/pkg/models/label_task.go @@ -329,7 +329,7 @@ func (t *Task) updateTaskLabels(creator web.Auth, labels []*Label) (err error) { t.Labels = append(t.Labels, label) } - err = updateListLastUpdated(&List{ID: t.ListID}) + err = updateListLastUpdated(s, &List{ID: t.ListID}) return } diff --git a/pkg/models/link_sharing.go b/pkg/models/link_sharing.go index 608240fb2..5389a8f7a 100644 --- a/pkg/models/link_sharing.go +++ b/pkg/models/link_sharing.go @@ -19,6 +19,7 @@ package models import ( "time" + "xorm.io/xorm" "code.vikunja.io/api/pkg/user" "code.vikunja.io/api/pkg/utils" @@ -154,7 +155,7 @@ func (share *LinkSharing) ReadOne() (err error) { // @Router /lists/{list}/shares [get] func (share *LinkSharing) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, totalItems int64, err error) { list := &List{ID: share.ListID} - can, _, err := list.CanRead(a) + can, _, err := list.CanRead(nil, a) if err != nil { return nil, 0, 0, err } @@ -235,13 +236,12 @@ func GetLinkShareByHash(hash string) (share *LinkSharing, err error) { } // GetListByShareHash returns a link share by its hash -func GetListByShareHash(hash string) (list *List, err error) { +func GetListByShareHash(s *xorm.Session, hash string) (list *List, err error) { share, err := GetLinkShareByHash(hash) if err != nil { return } - list = &List{ID: share.ListID} - err = list.GetSimpleByID() + list, err = GetListSimpleByID(s, share.ListID) return } diff --git a/pkg/models/link_sharing_rights.go b/pkg/models/link_sharing_rights.go index 5ffdcf6cb..69737dc53 100644 --- a/pkg/models/link_sharing_rights.go +++ b/pkg/models/link_sharing_rights.go @@ -16,20 +16,23 @@ package models -import "code.vikunja.io/web" +import ( + "code.vikunja.io/web" + "xorm.io/xorm" +) // CanRead implements the read right check for a link share -func (share *LinkSharing) CanRead(a web.Auth) (bool, int, error) { +func (share *LinkSharing) CanRead(s *xorm.Session, a web.Auth) (bool, int, error) { // Don't allow creating link shares if the user itself authenticated with a link share if _, is := a.(*LinkSharing); is { return false, 0, nil } - l, err := GetListByShareHash(share.Hash) + l, err := GetListByShareHash(s, share.Hash) if err != nil { return false, 0, err } - return l.CanRead(a) + return l.CanRead(nil, a) } // CanDelete implements the delete right check for a link share @@ -47,22 +50,21 @@ func (share *LinkSharing) CanCreate(a web.Auth) (bool, error) { return share.canDoLinkShare(a) } -func (share *LinkSharing) canDoLinkShare(a web.Auth) (bool, error) { +func (share *LinkSharing) canDoLinkShare(s *xorm.Session, a web.Auth) (bool, error) { // Don't allow creating link shares if the user itself authenticated with a link share if _, is := a.(*LinkSharing); is { return false, nil } - l := &List{ID: share.ListID} - err := l.GetSimpleByID() + l, err := GetListSimpleByID(s, share.ListID) if err != nil { return false, err } // Check if the user is admin when the link right is admin if share.Right == RightAdmin { - return l.IsAdmin(a) + return l.IsAdmin(s, a) } - return l.CanWrite(a) + return l.CanWrite(nil, a) } diff --git a/pkg/models/list.go b/pkg/models/list.go index 21ffa9f08..ea6cf15c8 100644 --- a/pkg/models/list.go +++ b/pkg/models/list.go @@ -96,9 +96,9 @@ var FavoritesPseudoList = List{ } // GetListsByNamespaceID gets all lists in a namespace -func GetListsByNamespaceID(nID int64, doer *user.User) (lists []*List, err error) { +func GetListsByNamespaceID(s *xorm.Session, nID int64, doer *user.User) (lists []*List, err error) { if nID == -1 { - err = x.Select("l.*"). + err = s.Select("l.*"). Table("list"). Join("LEFT", []string{"team_list", "tl"}, "l.id = tl.list_id"). Join("LEFT", []string{"team_members", "tm"}, "tm.team_id = tl.team_id"). @@ -111,7 +111,7 @@ func GetListsByNamespaceID(nID int64, doer *user.User) (lists []*List, err error GroupBy("l.id"). Find(&lists) } else { - err = x.Select("l.*"). + err = s.Select("l.*"). Alias("l"). Join("LEFT", []string{"namespaces", "n"}, "l.namespace_id = n.id"). Where("l.is_archived = false"). @@ -124,7 +124,7 @@ func GetListsByNamespaceID(nID int64, doer *user.User) (lists []*List, err error } // get more list details - err = AddListDetails(lists) + err = addListDetails(s, lists) return lists, err } @@ -143,33 +143,34 @@ func GetListsByNamespaceID(nID int64, doer *user.User) (lists []*List, err error // @Failure 403 {object} web.HTTPError "The user does not have access to the list" // @Failure 500 {object} models.Message "Internal error" // @Router /lists [get] -func (l *List) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, totalItems int64, err error) { +func (l *List) ReadAll(s *xorm.Session, a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, totalItems int64, err error) { // Check if we're dealing with a share auth shareAuth, ok := a.(*LinkSharing) if ok { - list := &List{ID: shareAuth.ListID} - err := list.GetSimpleByID() + list, err := GetListSimpleByID(s, shareAuth.ListID) if err != nil { return nil, 0, 0, err } lists := []*List{list} - err = AddListDetails(lists) + err = addListDetails(s, lists) return lists, 0, 0, err } - lists, resultCount, totalItems, err := getRawListsForUser(&listOptions{ - search: search, - user: &user.User{ID: a.GetID()}, - page: page, - perPage: perPage, - isArchived: l.IsArchived, - }) + lists, resultCount, totalItems, err := getRawListsForUser( + s, + &listOptions{ + search: search, + user: &user.User{ID: a.GetID()}, + page: page, + perPage: perPage, + isArchived: l.IsArchived, + }) if err != nil { return nil, 0, 0, err } // Add more list details - err = AddListDetails(lists) + err = addListDetails(s, lists) return lists, resultCount, totalItems, err } @@ -185,7 +186,7 @@ func (l *List) ReadAll(a web.Auth, search string, page int, perPage int) (result // @Failure 403 {object} web.HTTPError "The user does not have access to the list" // @Failure 500 {object} models.Message "Internal error" // @Router /lists/{id} [get] -func (l *List) ReadOne() (err error) { +func (l *List) ReadOne(s *xorm.Session) (err error) { if l.ID == FavoritesPseudoList.ID { // Already "built" the list in CanRead @@ -194,7 +195,7 @@ func (l *List) ReadOne() (err error) { // Check for saved filters if getSavedFilterIDFromListID(l.ID) > 0 { - sf, err := getSavedFilterSimpleByID(getSavedFilterIDFromListID(l.ID)) + sf, err := getSavedFilterSimpleByID(s, getSavedFilterIDFromListID(l.ID)) if err != nil { return err } @@ -206,13 +207,13 @@ func (l *List) ReadOne() (err error) { } // Get list owner - l.Owner, err = user.GetUserByID(l.OwnerID) + l.Owner, err = user.GetUserByIDSess(s, l.OwnerID) if err != nil { return err } // Check if the namespace is archived and set the namespace to archived if it is not already archived individually. if !l.IsArchived { - err = l.CheckIsArchived() + err = l.CheckIsArchived(s) if err != nil { if !IsErrNamespaceIsArchived(err) && !IsErrListIsArchived(err) { return @@ -224,7 +225,7 @@ func (l *List) ReadOne() (err error) { // Get any background information if there is one set if l.BackgroundFileID != 0 { // Unsplash image - l.BackgroundInformation, err = GetUnsplashPhotoByFileID(l.BackgroundFileID) + l.BackgroundInformation, err = GetUnsplashPhotoByFileID(s, l.BackgroundFileID) if err != nil && !files.IsErrFileIsNotUnsplashFile(err) { return } @@ -237,44 +238,30 @@ func (l *List) ReadOne() (err error) { return nil } -// GetSimpleByID gets a list with only the basic items, aka no tasks or user objects. Returns an error if the list does not exist. -func (l *List) GetSimpleByID() (err error) { - s := x.NewSession() - err = l.getSimpleByID(s) - if err != nil { - _ = s.Rollback() - return err - } - return nil -} - -func (l *List) getSimpleByID(s *xorm.Session) (err error) { - if l.ID < 1 { - return ErrListDoesNotExist{ID: l.ID} +// GetListSimpleByID gets a list with only the basic items, aka no tasks or user objects. Returns an error if the list does not exist. +func GetListSimpleByID(s *xorm.Session, listID int64) (list *List, err error) { + if listID < 1 { + return nil, ErrListDoesNotExist{ID: listID} } - // We need to re-init our list object, because otherwise xorm creates a "where for every item in that list object, - // leading to not finding anything if the id is good, but for example the title is different. - id := l.ID - *l = List{} - exists, err := s.Where("id = ?", id).Get(l) + exists, err := s.Where("id = ?", listID).Get(list) if err != nil { return } if !exists { - return ErrListDoesNotExist{ID: l.ID} + return nil, ErrListDoesNotExist{ID: listID} } return } // GetListSimplByTaskID gets a list by a task id -func GetListSimplByTaskID(taskID int64) (l *List, err error) { +func GetListSimplByTaskID(s *xorm.Session, taskID int64) (l *List, err error) { // We need to re-init our list object, because otherwise xorm creates a "where for every item in that list object, // leading to not finding anything if the id is good, but for example the title is different. var list List - exists, err := x. + exists, err := s. Select("list.*"). Table(List{}). Join("INNER", "tasks", "list.id = tasks.list_id"). @@ -292,9 +279,9 @@ func GetListSimplByTaskID(taskID int64) (l *List, err error) { } // GetListsByIDs returns a map of lists from a slice with list ids -func GetListsByIDs(listIDs []int64) (lists map[int64]*List, err error) { +func GetListsByIDs(s *xorm.Session, listIDs []int64) (lists map[int64]*List, err error) { lists = make(map[int64]*List, len(listIDs)) - err = x.In("id", listIDs).Find(&lists) + err = s.In("id", listIDs).Find(&lists) return } @@ -307,8 +294,8 @@ type listOptions struct { } // Gets the lists only, without any tasks or so -func getRawListsForUser(opts *listOptions) (lists []*List, resultCount int, totalItems int64, err error) { - fullUser, err := user.GetUserByID(opts.user.ID) +func getRawListsForUser(s *xorm.Session, opts *listOptions) (lists []*List, resultCount int, totalItems int64, err error) { + fullUser, err := user.GetUserByIDSess(s, opts.user.ID) if err != nil { return nil, 0, 0, err } @@ -344,7 +331,7 @@ func getRawListsForUser(opts *listOptions) (lists []*List, resultCount int, tota // Gets all Lists where the user is either owner or in a team which has access to the list // Or in a team which has namespace read access - query := x.Select("l.*"). + query := s.Select("l.*"). Table("list"). Alias("l"). Join("INNER", []string{"namespaces", "n"}, "l.namespace_id = n.id"). @@ -372,7 +359,7 @@ func getRawListsForUser(opts *listOptions) (lists []*List, resultCount int, tota return nil, 0, 0, err } - totalItems, err = x. + totalItems, err = s. Table("list"). Alias("l"). Join("INNER", []string{"namespaces", "n"}, "l.namespace_id = n.id"). @@ -396,8 +383,8 @@ func getRawListsForUser(opts *listOptions) (lists []*List, resultCount int, tota return lists, len(lists), totalItems, err } -// AddListDetails adds owner user objects and list tasks to all lists in the slice -func AddListDetails(lists []*List) (err error) { +// addListDetails adds owner user objects and list tasks to all lists in the slice +func addListDetails(s *xorm.Session, lists []*List) (err error) { var ownerIDs []int64 for _, l := range lists { ownerIDs = append(ownerIDs, l.OwnerID) @@ -405,7 +392,7 @@ func AddListDetails(lists []*List) (err error) { // Get all list owners owners := map[int64]*user.User{} - err = x.In("id", ownerIDs).Find(&owners) + err = s.In("id", ownerIDs).Find(&owners) if err != nil { return } @@ -423,7 +410,7 @@ func AddListDetails(lists []*List) (err error) { // Unsplash background file info us := []*UnsplashPhoto{} - err = x.In("file_id", fileIDs).Find(&us) + err = s.In("file_id", fileIDs).Find(&us) if err != nil { return } @@ -450,7 +437,7 @@ type NamespaceList struct { } // CheckIsArchived returns an ErrListIsArchived or ErrNamespaceIsArchived if the list or its namespace is archived. -func (l *List) CheckIsArchived() (err error) { +func (l *List) CheckIsArchived(s *xorm.Session) (err error) { // When creating a new list, we check if the namespace is archived if l.ID == 0 { n := &Namespace{ID: l.NamespaceID} @@ -458,7 +445,7 @@ func (l *List) CheckIsArchived() (err error) { } nl := &NamespaceList{} - exists, err := x. + exists, err := s. Table("list"). Join("LEFT", "namespaces", "list.namespace_id = namespaces.id"). Where("list.id = ? AND (list.is_archived = true OR namespaces.is_archived = true)", l.ID). @@ -476,11 +463,11 @@ func (l *List) CheckIsArchived() (err error) { } // CreateOrUpdateList updates a list or creates it if it doesn't exist -func CreateOrUpdateList(list *List) (err error) { +func CreateOrUpdateList(s *xorm.Session, list *List) (err error) { // Check if the namespace exists if list.NamespaceID != 0 && list.NamespaceID != FavoritesPseudoNamespace.ID { - _, err = GetNamespaceByID(list.NamespaceID) + _, err = GetNamespaceByID(s, list.NamespaceID) if err != nil { return err } @@ -488,7 +475,7 @@ func CreateOrUpdateList(list *List) (err error) { // Check if the identifier is unique and not empty if list.Identifier != "" { - exists, err := x. + exists, err := s. Where("identifier = ?", list.Identifier). And("id != ?", list.ID). Exist(&List{}) @@ -501,7 +488,7 @@ func CreateOrUpdateList(list *List) (err error) { } if list.ID == 0 { - _, err = x.Insert(list) + _, err = s.Insert(list) metrics.UpdateCount(1, metrics.ListCountKey) } else { // We need to specify the cols we want to update here to be able to un-archive lists @@ -516,7 +503,7 @@ func CreateOrUpdateList(list *List) (err error) { colsToUpdate = append(colsToUpdate, "description") } - _, err = x. + _, err = s. ID(list.ID). Cols(colsToUpdate...). Update(list) @@ -526,12 +513,12 @@ func CreateOrUpdateList(list *List) (err error) { return } - err = list.GetSimpleByID() + list, err = GetListSimpleByID(s, list.ID) if err != nil { return } - err = list.ReadOne() + err = list.ReadOne(s) return } @@ -550,33 +537,23 @@ func CreateOrUpdateList(list *List) (err error) { // @Failure 403 {object} web.HTTPError "The user does not have access to the list" // @Failure 500 {object} models.Message "Internal error" // @Router /lists/{id} [post] -func (l *List) Update() (err error) { - return CreateOrUpdateList(l) +func (l *List) Update(s *xorm.Session) (err error) { + return CreateOrUpdateList(s, l) } -func updateListLastUpdated(list *List) (err error) { - s := x.NewSession() - err = updateListLastUpdatedS(s, list) - if err != nil { - _ = s.Rollback() - return err - } - return nil -} - -func updateListLastUpdatedS(s *xorm.Session, list *List) error { +func updateListLastUpdated(s *xorm.Session, list *List) error { _, err := s.ID(list.ID).Cols("updated").Update(list) return err } -func updateListByTaskID(taskID int64) (err error) { +func updateListByTaskID(s *xorm.Session, taskID int64) (err error) { // need to get the task to update the list last updated timestamp task, err := GetTaskByIDSimple(taskID) if err != nil { return err } - return updateListLastUpdated(&List{ID: task.ListID}) + return updateListLastUpdated(s, &List{ID: task.ListID}) } // Create implements the create method of CRUDable @@ -593,7 +570,7 @@ func updateListByTaskID(taskID int64) (err error) { // @Failure 403 {object} web.HTTPError "The user does not have access to the list" // @Failure 500 {object} models.Message "Internal error" // @Router /namespaces/{namespaceID}/lists [put] -func (l *List) Create(a web.Auth) (err error) { +func (l *List) Create(s *xorm.Session, a web.Auth) (err error) { err = l.CheckIsArchived() if err != nil { return err @@ -633,27 +610,27 @@ func (l *List) Create(a web.Auth) (err error) { // @Failure 403 {object} web.HTTPError "The user does not have access to the list" // @Failure 500 {object} models.Message "Internal error" // @Router /lists/{id} [delete] -func (l *List) Delete() (err error) { +func (l *List) Delete(s *xorm.Session) (err error) { // Delete the list - _, err = x.ID(l.ID).Delete(&List{}) + _, err = s.ID(l.ID).Delete(&List{}) if err != nil { return } metrics.UpdateCount(-1, metrics.ListCountKey) // Delete all todotasks on that list - _, err = x.Where("list_id = ?", l.ID).Delete(&Task{}) + _, err = s.Where("list_id = ?", l.ID).Delete(&Task{}) return } // SetListBackground sets a background file as list background in the db -func SetListBackground(listID int64, background *files.File) (err error) { +func SetListBackground(s *xorm.Session, listID int64, background *files.File) (err error) { l := &List{ ID: listID, BackgroundFileID: background.ID, } - _, err = x. + _, err = s. Where("id = ?", l.ID). Cols("background_file_id"). Update(l) diff --git a/pkg/models/list_duplicate.go b/pkg/models/list_duplicate.go index e723f3339..5bc3f2b8f 100644 --- a/pkg/models/list_duplicate.go +++ b/pkg/models/list_duplicate.go @@ -41,14 +41,14 @@ type ListDuplicate struct { func (ld *ListDuplicate) CanCreate(a web.Auth) (canCreate bool, err error) { // List Exists + user has read access to list ld.List = &List{ID: ld.ListID} - canRead, _, err := ld.List.CanRead(a) + canRead, _, err := ld.List.CanRead(nil, a) if err != nil || !canRead { return canRead, err } // Namespace exists + user has write access to is (-> can create new lists) ld.List.NamespaceID = ld.NamespaceID - return ld.List.CanCreate(a) + return ld.List.CanCreate(s, a) } // Create duplicates a list diff --git a/pkg/models/list_rights.go b/pkg/models/list_rights.go index fddcbf8de..170a32d95 100644 --- a/pkg/models/list_rights.go +++ b/pkg/models/list_rights.go @@ -20,10 +20,11 @@ import ( "code.vikunja.io/api/pkg/user" "code.vikunja.io/web" "xorm.io/builder" + "xorm.io/xorm" ) // CanWrite return whether the user can write on that list or not -func (l *List) CanWrite(a web.Auth) (bool, error) { +func (l *List) CanWrite(s *xorm.Session, a web.Auth) (bool, error) { // The favorite list can't be edited if l.ID == FavoritesPseudoList.ID { @@ -31,15 +32,14 @@ func (l *List) CanWrite(a web.Auth) (bool, error) { } // Get the list and check the right - originalList := &List{ID: l.ID} - err := originalList.GetSimpleByID() + originalList, err := GetListSimpleByID(s, l.ID) if err != nil { return false, err } // We put the result of the is archived check in a separate variable to be able to return it later without // needing to recheck it again - errIsArchived := originalList.CheckIsArchived() + errIsArchived := originalList.CheckIsArchived(s) var canWrite bool @@ -59,7 +59,7 @@ func (l *List) CanWrite(a web.Auth) (bool, error) { return canWrite, errIsArchived } - canWrite, _, err = originalList.checkRight(a, RightWrite, RightAdmin) + canWrite, _, err = originalList.checkRight(s, a, RightWrite, RightAdmin) if err != nil { return false, err } @@ -67,7 +67,7 @@ func (l *List) CanWrite(a web.Auth) (bool, error) { } // CanRead checks if a user has read access to a list -func (l *List) CanRead(a web.Auth) (bool, int, error) { +func (l *List) CanRead(s *xorm.Session, a web.Auth) (bool, int, error) { // The favorite list needs a special treatment if l.ID == FavoritesPseudoList.ID { @@ -84,34 +84,36 @@ func (l *List) CanRead(a web.Auth) (bool, int, error) { // Saved Filter Lists need a special case if getSavedFilterIDFromListID(l.ID) > 0 { sf := &SavedFilter{ID: getSavedFilterIDFromListID(l.ID)} - return sf.CanRead(a) + return sf.CanRead(s, a) } // Check if the user is either owner or can read - if err := l.GetSimpleByID(); err != nil { + var err error + originalList, err := GetListSimpleByID(s, l.ID) + if err != nil { return false, 0, err } // Check if we're dealing with a share auth shareAuth, ok := a.(*LinkSharing) if ok { - return l.ID == shareAuth.ListID && + return originalList.ID == shareAuth.ListID && (shareAuth.Right == RightRead || shareAuth.Right == RightWrite || shareAuth.Right == RightAdmin), int(shareAuth.Right), nil } - if l.isOwner(&user.User{ID: a.GetID()}) { + if originalList.isOwner(&user.User{ID: a.GetID()}) { return true, int(RightAdmin), nil } - return l.checkRight(a, RightRead, RightWrite, RightAdmin) + return originalList.checkRight(s, a, RightRead, RightWrite, RightAdmin) } // CanUpdate checks if the user can update a list -func (l *List) CanUpdate(a web.Auth) (canUpdate bool, err error) { +func (l *List) CanUpdate(s *xorm.Session, a web.Auth) (canUpdate bool, err error) { // The favorite list can't be edited if l.ID == FavoritesPseudoList.ID { return false, nil } - canUpdate, err = l.CanWrite(a) + canUpdate, err = l.CanWrite(s, a) // If the list is archived and the user tries to un-archive it, let the request through if IsErrListIsArchived(err) && !l.IsArchived { err = nil @@ -120,26 +122,25 @@ func (l *List) CanUpdate(a web.Auth) (canUpdate bool, err error) { } // CanDelete checks if the user can delete a list -func (l *List) CanDelete(a web.Auth) (bool, error) { - return l.IsAdmin(a) +func (l *List) CanDelete(s *xorm.Session, a web.Auth) (bool, error) { + return l.IsAdmin(s, a) } // CanCreate checks if the user can create a list -func (l *List) CanCreate(a web.Auth) (bool, error) { +func (l *List) CanCreate(s *xorm.Session, a web.Auth) (bool, error) { // A user can create a list if they have write access to the namespace n := &Namespace{ID: l.NamespaceID} - return n.CanWrite(a) + return n.CanWrite(s, a) } // IsAdmin returns whether the user has admin rights on the list or not -func (l *List) IsAdmin(a web.Auth) (bool, error) { +func (l *List) IsAdmin(s *xorm.Session, a web.Auth) (bool, error) { // The favorite list can't be edited if l.ID == FavoritesPseudoList.ID { return false, nil } - originalList := &List{ID: l.ID} - err := originalList.GetSimpleByID() + originalList, err := GetListSimpleByID(s, l.ID) if err != nil { return false, err } @@ -156,7 +157,7 @@ func (l *List) IsAdmin(a web.Auth) (bool, error) { if originalList.isOwner(&user.User{ID: a.GetID()}) { return true, nil } - is, _, err := originalList.checkRight(a, RightAdmin) + is, _, err := originalList.checkRight(s, a, RightAdmin) return is, err } @@ -166,7 +167,7 @@ func (l *List) isOwner(u *user.User) bool { } // Checks n different rights for any given user -func (l *List) checkRight(a web.Auth, rights ...Right) (bool, int, error) { +func (l *List) checkRight(s *xorm.Session, a web.Auth, rights ...Right) (bool, int, error) { /* The following loop creates an sql condition like this one: @@ -218,7 +219,7 @@ func (l *List) checkRight(a web.Auth, rights ...Right) (bool, int, error) { r := &allListRights{} var maxRight = 0 - exists, err := x. + exists, err := s. Table("list"). Alias("l"). // User stuff diff --git a/pkg/models/list_team.go b/pkg/models/list_team.go index 371e267ca..1d0b1c4d7 100644 --- a/pkg/models/list_team.go +++ b/pkg/models/list_team.go @@ -82,8 +82,8 @@ func (tl *TeamList) Create(a web.Auth) (err error) { } // Check if the list exists - l := &List{ID: tl.ListID} - if err := l.GetSimpleByID(); err != nil { + l, err := GetListSimpleByID(s, tl.ListID) + if err != nil { return err } @@ -104,7 +104,7 @@ func (tl *TeamList) Create(a web.Auth) (err error) { return err } - err = updateListLastUpdated(l) + err = updateListLastUpdated(s, l) return } @@ -147,7 +147,7 @@ func (tl *TeamList) Delete() (err error) { return err } - err = updateListLastUpdated(&List{ID: tl.ListID}) + err = updateListLastUpdated(s, &List{ID: tl.ListID}) return } @@ -169,7 +169,7 @@ func (tl *TeamList) Delete() (err error) { func (tl *TeamList) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, totalItems int64, err error) { // Check if the user can read the namespace l := &List{ID: tl.ListID} - canRead, _, err := l.CanRead(a) + canRead, _, err := l.CanRead(nil, a) if err != nil { return nil, 0, 0, err } @@ -247,6 +247,6 @@ func (tl *TeamList) Update() (err error) { return err } - err = updateListLastUpdated(&List{ID: tl.ListID}) + err = updateListLastUpdated(s, &List{ID: tl.ListID}) return } diff --git a/pkg/models/list_team_rights.go b/pkg/models/list_team_rights.go index 923c8757d..23fdbb9c3 100644 --- a/pkg/models/list_team_rights.go +++ b/pkg/models/list_team_rights.go @@ -42,5 +42,5 @@ func (tl *TeamList) canDoTeamList(a web.Auth) (bool, error) { } l := List{ID: tl.ListID} - return l.IsAdmin(a) + return l.IsAdmin(s, a) } diff --git a/pkg/models/list_users.go b/pkg/models/list_users.go index b9ada17f8..3c8e5cb67 100644 --- a/pkg/models/list_users.go +++ b/pkg/models/list_users.go @@ -79,8 +79,8 @@ func (lu *ListUser) Create(a web.Auth) (err error) { } // Check if the list exists - l := &List{ID: lu.ListID} - if err = l.GetSimpleByID(); err != nil { + l, err := GetListSimpleByID(s, lu.ListID) + if err != nil { return } @@ -111,7 +111,7 @@ func (lu *ListUser) Create(a web.Auth) (err error) { return err } - err = updateListLastUpdated(l) + err = updateListLastUpdated(s, l) return } @@ -153,7 +153,7 @@ func (lu *ListUser) Delete() (err error) { return err } - err = updateListLastUpdated(&List{ID: lu.ListID}) + err = updateListLastUpdated(s, &List{ID: lu.ListID}) return } @@ -175,7 +175,7 @@ func (lu *ListUser) Delete() (err error) { func (lu *ListUser) ReadAll(a web.Auth, search string, page int, perPage int) (result interface{}, resultCount int, numberOfTotalItems int64, err error) { // Check if the user has access to the list l := &List{ID: lu.ListID} - canRead, _, err := l.CanRead(a) + canRead, _, err := l.CanRead(nil, a) if err != nil { return nil, 0, 0, err } @@ -250,6 +250,6 @@ func (lu *ListUser) Update() (err error) { return err } - err = updateListLastUpdated(&List{ID: lu.ListID}) + err = updateListLastUpdated(s, &List{ID: lu.ListID}) return } diff --git a/pkg/models/list_users_rights.go b/pkg/models/list_users_rights.go index b80fd1d9d..a5b99c7a3 100644 --- a/pkg/models/list_users_rights.go +++ b/pkg/models/list_users_rights.go @@ -43,5 +43,5 @@ func (lu *ListUser) canDoListUser(a web.Auth) (bool, error) { // Get the list and check if the user has write access on it l := List{ID: lu.ListID} - return l.IsAdmin(a) + return l.IsAdmin(s, a) } diff --git a/pkg/models/namespace.go b/pkg/models/namespace.go index 4d4fb472a..b8ff55807 100644 --- a/pkg/models/namespace.go +++ b/pkg/models/namespace.go @@ -21,6 +21,7 @@ import ( "strconv" "strings" "time" + "xorm.io/xorm" "code.vikunja.io/api/pkg/log" @@ -95,43 +96,34 @@ func (Namespace) TableName() string { } // GetSimpleByID gets a namespace without things like the owner, it more or less only checks if it exists. -func (n *Namespace) GetSimpleByID() (err error) { - if n.ID == 0 { - return ErrNamespaceDoesNotExist{ID: n.ID} +func getNamespaceSimpleByID(s *xorm.Session, id int64) (namespace *Namespace, err error) { + if id == 0 { + return nil, ErrNamespaceDoesNotExist{ID: id} } // Get the namesapce with shared lists - if n.ID == -1 { - *n = SharedListsPseudoNamespace - return + if id == -1 { + return &SharedListsPseudoNamespace, nil } - if n.ID == FavoritesPseudoNamespace.ID { - *n = FavoritesPseudoNamespace - return + if id == FavoritesPseudoNamespace.ID { + return &FavoritesPseudoNamespace, nil } - namespaceFromDB := &Namespace{} - exists, err := x.Where("id = ?", n.ID).Get(namespaceFromDB) + exists, err := s.Where("id = ?", id).Get(namespace) if err != nil { return } if !exists { - return ErrNamespaceDoesNotExist{ID: n.ID} + return nil, ErrNamespaceDoesNotExist{ID: id} } - // We don't want to override the provided user struct because this would break updating, so we have to merge it - if err := mergo.Merge(namespaceFromDB, n, mergo.WithOverride); err != nil { - return err - } - *n = *namespaceFromDB return } // GetNamespaceByID returns a namespace object by its ID -func GetNamespaceByID(id int64) (namespace Namespace, err error) { - namespace = Namespace{ID: id} - err = namespace.GetSimpleByID() +func GetNamespaceByID(s *xorm.Session, id int64) (namespace *Namespace, err error) { + namespace, err = getNamespaceSimpleByID(s, id) if err != nil { return } @@ -360,7 +352,7 @@ func (n *Namespace) ReadAll(a web.Auth, search string, page int, perPage int) (r } // More details for the lists - err = AddListDetails(lists) + err = addListDetails(lists) if err != nil { return nil, 0, 0, err } diff --git a/pkg/models/saved_filters.go b/pkg/models/saved_filters.go index 97ab4f85e..621dec943 100644 --- a/pkg/models/saved_filters.go +++ b/pkg/models/saved_filters.go @@ -18,6 +18,7 @@ package models import ( "time" + "xorm.io/xorm" "code.vikunja.io/api/pkg/user" "code.vikunja.io/web" @@ -106,11 +107,11 @@ func (s *SavedFilter) Create(auth web.Auth) error { return err } -func getSavedFilterSimpleByID(id int64) (s *SavedFilter, err error) { - s = &SavedFilter{} - exists, err := x. +func getSavedFilterSimpleByID(s *xorm.Session, id int64) (sf *SavedFilter, err error) { + sf = &SavedFilter{} + exists, err := s. Where("id = ?", id). - Get(s) + Get(sf) if err != nil { return nil, err } diff --git a/pkg/models/task_assignees.go b/pkg/models/task_assignees.go index cbf0f7fea..148c9c781 100644 --- a/pkg/models/task_assignees.go +++ b/pkg/models/task_assignees.go @@ -118,8 +118,7 @@ func (t *Task) updateTaskAssignees(s *xorm.Session, assignees []*user.User) (err } // Get the list to perform later checks - list := List{ID: t.ListID} - err = list.GetSimpleByID() + list, err := GetListSimpleByID(s, t.ListID) if err != nil { return } @@ -141,7 +140,7 @@ func (t *Task) updateTaskAssignees(s *xorm.Session, assignees []*user.User) (err t.setTaskAssignees(assignees) - err = updateListLastUpdated(&List{ID: t.ListID}) + err = updateListLastUpdated(s, &List{ID: t.ListID}) return } @@ -208,7 +207,7 @@ func (t *Task) addNewAssigneeByID(newAssigneeID int64, list *List) (err error) { if err != nil { return err } - canRead, _, err := list.CanRead(newAssignee) + canRead, _, err := list.CanRead(nil, newAssignee) if err != nil { return err } @@ -224,7 +223,7 @@ func (t *Task) addNewAssigneeByID(newAssigneeID int64, list *List) (err error) { return err } - err = updateListLastUpdated(&List{ID: t.ListID}) + err = updateListLastUpdated(s, &List{ID: t.ListID}) return } @@ -248,7 +247,7 @@ func (la *TaskAssginee) ReadAll(a web.Auth, search string, page int, perPage int return nil, 0, 0, err } - can, _, err := task.CanRead(a) + can, _, err := task.CanRead(nil, a) if err != nil { return nil, 0, 0, err } diff --git a/pkg/models/task_assignees_rights.go b/pkg/models/task_assignees_rights.go index ea8cf13f7..0cd1eb606 100644 --- a/pkg/models/task_assignees_rights.go +++ b/pkg/models/task_assignees_rights.go @@ -41,5 +41,5 @@ func canDoTaskAssingee(taskID int64, a web.Auth) (bool, error) { if err != nil { return false, err } - return list.CanUpdate(a) + return list.CanUpdate(s, a) } diff --git a/pkg/models/task_collection.go b/pkg/models/task_collection.go index a0c56a15f..385b6978a 100644 --- a/pkg/models/task_collection.go +++ b/pkg/models/task_collection.go @@ -156,8 +156,7 @@ func (tf *TaskCollection) ReadAll(a web.Auth, search string, page int, perPage i shareAuth, is := a.(*LinkSharing) if is { - list := &List{ID: shareAuth.ListID} - err := list.GetSimpleByID() + list, err := GetListSimpleByID(s, shareAuth.ListID) if err != nil { return nil, 0, 0, err } @@ -177,7 +176,7 @@ func (tf *TaskCollection) ReadAll(a web.Auth, search string, page int, perPage i } else { // Check the list exists and the user has acess on it list := &List{ID: tf.ListID} - canRead, _, err := list.CanRead(a) + canRead, _, err := list.CanRead(nil, a) if err != nil { return nil, 0, 0, err } diff --git a/pkg/models/tasks.go b/pkg/models/tasks.go index dab48ac8a..b3f90887f 100644 --- a/pkg/models/tasks.go +++ b/pkg/models/tasks.go @@ -759,9 +759,9 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err } // Check if the list exists - l := &List{ID: t.ListID} - if err = l.getSimpleByID(s); err != nil { - return + l, err := GetListSimpleByID(s, t.ListID) + if err != nil { + return err } if _, is := a.(*LinkSharing); is { @@ -834,7 +834,7 @@ func createTask(s *xorm.Session, t *Task, a web.Auth, updateAssignees bool) (err t.setIdentifier(l) - err = updateListLastUpdatedS(s, &List{ID: t.ListID}) + err = updateListLastUpdated(s, &List{ID: t.ListID}) return } @@ -1047,7 +1047,7 @@ func (t *Task) Update() (err error) { } t.Updated = nt.Updated - err = updateListLastUpdatedS(s, &List{ID: t.ListID}) + err = updateListLastUpdated(s, &List{ID: t.ListID}) if err != nil { _ = s.Rollback() return err @@ -1174,7 +1174,7 @@ func (t *Task) updateReminders(s *xorm.Session, reminders []time.Time) (err erro t.Reminders = nil } - err = updateListLastUpdatedS(s, &List{ID: t.ListID}) + err = updateListLastUpdated(s, &List{ID: t.ListID}) return } @@ -1203,7 +1203,7 @@ func (t *Task) Delete() (err error) { metrics.UpdateCount(-1, metrics.TaskCountKey) - err = updateListLastUpdated(&List{ID: t.ListID}) + err = updateListLastUpdated(s, &List{ID: t.ListID}) return } diff --git a/pkg/models/tasks_rights.go b/pkg/models/tasks_rights.go index efc0f4b8a..f3707197b 100644 --- a/pkg/models/tasks_rights.go +++ b/pkg/models/tasks_rights.go @@ -34,7 +34,7 @@ func (t *Task) CanUpdate(a web.Auth) (bool, error) { func (t *Task) CanCreate(a web.Auth) (bool, error) { // A user can do a task if he has write acces to its list l := &List{ID: t.ListID} - return l.CanWrite(a) + return l.CanWrite(nil, a) } // CanRead determines if a user can read a task @@ -47,7 +47,7 @@ func (t *Task) CanRead(a web.Auth) (canRead bool, maxRight int, err error) { // A user can read a task if it has access to the list l := &List{ID: t.ListID} - return l.CanRead(a) + return l.CanRead(nil, a) } // CanWrite checks if a user has write access to a task @@ -66,7 +66,7 @@ func (t *Task) canDoTask(a web.Auth) (bool, error) { // Check if we're moving the task into a different list to check if the user has sufficient rights for that on the new list if t.ListID != 0 && t.ListID != ot.ListID { newList := &List{ID: t.ListID} - can, err := newList.CanWrite(a) + can, err := newList.CanWrite(nil, a) if err != nil { return false, err } @@ -77,5 +77,5 @@ func (t *Task) canDoTask(a web.Auth) (bool, error) { // A user can do a task if it has write acces to its list l := &List{ID: ot.ListID} - return l.CanWrite(a) + return l.CanWrite(nil, a) } diff --git a/pkg/models/unsplash.go b/pkg/models/unsplash.go index 8b0847480..6c9e85296 100644 --- a/pkg/models/unsplash.go +++ b/pkg/models/unsplash.go @@ -16,7 +16,10 @@ package models -import "code.vikunja.io/api/pkg/files" +import ( + "code.vikunja.io/api/pkg/files" + "xorm.io/xorm" +) // Unsplash requires us to do pingbacks to their site and also name the image author. // To do this properly, we need to save these information somewhere. @@ -42,9 +45,9 @@ func (u *UnsplashPhoto) Save() error { } // GetUnsplashPhotoByFileID returns an unsplash photo by its saved file id -func GetUnsplashPhotoByFileID(fileID int64) (u *UnsplashPhoto, err error) { +func GetUnsplashPhotoByFileID(s *xorm.Session, fileID int64) (u *UnsplashPhoto, err error) { u = &UnsplashPhoto{} - exists, err := x.Where("file_id = ?", fileID).Get(u) + exists, err := s.Where("file_id = ?", fileID).Get(u) if err != nil { return } diff --git a/pkg/modules/background/handler/background.go b/pkg/modules/background/handler/background.go index 2c1921c4e..2c2c4600e 100644 --- a/pkg/modules/background/handler/background.go +++ b/pkg/modules/background/handler/background.go @@ -81,7 +81,7 @@ func (bp *BackgroundProvider) setBackgroundPreparations(c echo.Context) (list *m // Check if the user has the right to change the list background list = &models.List{ID: listID} - can, err := list.CanUpdate(auth) + can, err := list.CanUpdate(s, auth) if err != nil { return } @@ -90,7 +90,7 @@ func (bp *BackgroundProvider) setBackgroundPreparations(c echo.Context) (list *m return list, auth, models.ErrGenericForbidden{} } // Load the list - err = list.GetSimpleByID() + list, err = models.GetListSimpleByID(s, list.ID) return } @@ -192,7 +192,7 @@ func GetListBackground(c echo.Context) error { // Check if a background for this list exists + Rights list := &models.List{ID: listID} - can, _, err := list.CanRead(auth) + can, _, err := list.CanRead(nil, auth) if err != nil { return handler.HandleHTTPError(err, c) } diff --git a/pkg/routes/caldav/listStorageProvider.go b/pkg/routes/caldav/listStorageProvider.go index 0892bb0b5..233d89b66 100644 --- a/pkg/routes/caldav/listStorageProvider.go +++ b/pkg/routes/caldav/listStorageProvider.go @@ -385,7 +385,7 @@ func (vlra *VikunjaListResourceAdapter) GetModTime() time.Time { } func (vcls *VikunjaCaldavListStorage) getListRessource(isCollection bool) (rr VikunjaListResourceAdapter, err error) { - can, _, err := vcls.list.CanRead(vcls.user) + can, _, err := vcls.list.CanRead(nil, vcls.user) if err != nil { return }