1
0

Use db sessions everywere (#750)

Fix lint

Fix lint

Fix loading tasks with search

Fix loading lists

Fix loading task

Fix loading lists and namespaces

Fix tests

Fix user commands

Fix upload

Fix migration handlers

Fix all manual root handlers

Fix session in avatar

Fix session in list duplication & routes

Use sessions in migration code

Make sure the openid stuff uses a session

Add alias for db type in db package

Use sessions for file

Use a session for everything in users

Use a session for everything in users

Make sure to use a session everywhere in models

Create new session from db

Add session handling for user list

Add session handling for unsplash

Add session handling for teams and related

Add session handling for tasks and related entities

Add session handling for task reminders

Add session handling for task relations

Add session handling for task comments

Add session handling for task collections

Add session handling for task attachments

Add session handling for task assignees

Add session handling for saved filters

Add session handling for namespace and related types

Add session handling for namespace and related types

Add session handling for list users

Add session handling for list tests

Add session handling to list teams and related entities

Add session handling for link shares and related entities

Add session handling for labels and related entities

Add session handling for kanban and related entities

Add session handling for bulk task and related entities

Add session handling for lists and related entities

Add session configuration for web handler

Update web handler

Co-authored-by: kolaente <k@knt.li>
Reviewed-on: https://kolaente.dev/vikunja/api/pulls/750
Co-Authored-By: konrad <konrad@kola-entertainments.de>
Co-Committed-By: konrad <konrad@kola-entertainments.de>
This commit is contained in:
konrad
2020-12-23 15:32:28 +00:00
parent fa68e89c04
commit 8d1a09b5a2
107 changed files with 2428 additions and 1279 deletions

View File

@ -23,6 +23,9 @@ import (
"net/http"
"time"
"code.vikunja.io/api/pkg/db"
"xorm.io/xorm"
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/api/pkg/modules/auth"
@ -130,8 +133,17 @@ func HandleCallback(c echo.Context) error {
return err
}
s := db.NewSession()
defer s.Close()
// Check if we have seen this user before
u, err := getOrCreateUser(cl, idToken.Issuer, idToken.Subject)
u, err := getOrCreateUser(s, cl, idToken.Issuer, idToken.Subject)
if err != nil {
_ = s.Rollback()
return err
}
err = s.Commit()
if err != nil {
return err
}
@ -140,9 +152,9 @@ func HandleCallback(c echo.Context) error {
return auth.NewUserAuthTokenResponse(u, c)
}
func getOrCreateUser(cl *claims, issuer, subject string) (u *user.User, err error) {
func getOrCreateUser(s *xorm.Session, cl *claims, issuer, subject string) (u *user.User, err error) {
// Check if the user exists for that issuer and subject
u, err = user.GetUserWithEmail(&user.User{
u, err = user.GetUserWithEmail(s, &user.User{
Issuer: issuer,
Subject: subject,
})
@ -165,7 +177,7 @@ func getOrCreateUser(cl *claims, issuer, subject string) (u *user.User, err erro
uu.Username = petname.Generate(3, "-")
}
u, err = user.CreateUser(uu)
u, err = user.CreateUser(s, uu)
if err != nil && !user.IsErrUsernameExists(err) {
return nil, err
}
@ -173,14 +185,14 @@ func getOrCreateUser(cl *claims, issuer, subject string) (u *user.User, err erro
// If their preferred username is already taken, create some random one from the email and subject
if user.IsErrUsernameExists(err) {
uu.Username = petname.Generate(3, "-")
u, err = user.CreateUser(uu)
u, err = user.CreateUser(s, uu)
if err != nil {
return nil, err
}
}
// And create its namespace
err = models.CreateNewNamespaceForUser(u)
err = models.CreateNewNamespaceForUser(s, u)
if err != nil {
return nil, err
}
@ -196,7 +208,7 @@ func getOrCreateUser(cl *claims, issuer, subject string) (u *user.User, err erro
if cl.Name != u.Name {
u.Name = cl.Name
}
u, err = user.UpdateUser(&user.User{
u, err = user.UpdateUser(s, &user.User{
ID: u.ID,
Email: u.Email,
Name: u.Name,

View File

@ -26,12 +26,18 @@ import (
func TestGetOrCreateUser(t *testing.T) {
t.Run("new user", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
cl := &claims{
Email: "test@example.com",
PreferredUsername: "someUserWhoDoesNotExistYet",
}
u, err := getOrCreateUser(cl, "https://some.issuer", "12345")
u, err := getOrCreateUser(s, cl, "https://some.issuer", "12345")
assert.NoError(t, err)
err = s.Commit()
assert.NoError(t, err)
db.AssertExists(t, "users", map[string]interface{}{
"id": u.ID,
"email": cl.Email,
@ -40,13 +46,19 @@ func TestGetOrCreateUser(t *testing.T) {
})
t.Run("new user, no username provided", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
cl := &claims{
Email: "test@example.com",
PreferredUsername: "",
}
u, err := getOrCreateUser(cl, "https://some.issuer", "12345")
u, err := getOrCreateUser(s, cl, "https://some.issuer", "12345")
assert.NoError(t, err)
assert.NotEmpty(t, u.Username)
err = s.Commit()
assert.NoError(t, err)
db.AssertExists(t, "users", map[string]interface{}{
"id": u.ID,
"email": cl.Email,
@ -54,19 +66,28 @@ func TestGetOrCreateUser(t *testing.T) {
})
t.Run("new user, no email address", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
cl := &claims{
Email: "",
}
_, err := getOrCreateUser(cl, "https://some.issuer", "12345")
_, err := getOrCreateUser(s, cl, "https://some.issuer", "12345")
assert.Error(t, err)
})
t.Run("existing user, different email address", func(t *testing.T) {
db.LoadAndAssertFixtures(t)
s := db.NewSession()
defer s.Close()
cl := &claims{
Email: "other-email-address@some.service.com",
}
u, err := getOrCreateUser(cl, "https://some.service.com", "12345")
u, err := getOrCreateUser(s, cl, "https://some.service.com", "12345")
assert.NoError(t, err)
err = s.Commit()
assert.NoError(t, err)
db.AssertExists(t, "users", map[string]interface{}{
"id": u.ID,
"email": cl.Email,

View File

@ -19,6 +19,7 @@ package background
import (
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/web"
"xorm.io/xorm"
)
// Image represents an image which can be used as a list background
@ -33,7 +34,7 @@ type Image struct {
// Provider represents something that is able to get a list of images and set one of them as background
type Provider interface {
// Search is used to either return a pre-defined list of Image or let the user search for an image
Search(search string, page int64) (result []*Image, err error)
Search(s *xorm.Session, search string, page int64) (result []*Image, err error)
// Set sets an image which was most likely previously obtained by Search as list background
Set(image *Image, list *models.List, auth web.Auth) (err error)
Set(s *xorm.Session, image *Image, list *models.List, auth web.Auth) (err error)
}

View File

@ -22,6 +22,9 @@ import (
"strconv"
"strings"
"code.vikunja.io/api/pkg/db"
"xorm.io/xorm"
"code.vikunja.io/api/pkg/files"
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/api/pkg/models"
@ -59,8 +62,17 @@ func (bp *BackgroundProvider) SearchBackgrounds(c echo.Context) error {
}
}
result, err := p.Search(search, page)
s := db.NewSession()
defer s.Close()
result, err := p.Search(s, search, page)
if err != nil {
_ = s.Rollback()
return echo.NewHTTPError(http.StatusBadRequest, "An error occurred: "+err.Error())
}
if err := s.Commit(); err != nil {
_ = s.Rollback()
return echo.NewHTTPError(http.StatusBadRequest, "An error occurred: "+err.Error())
}
@ -68,7 +80,7 @@ func (bp *BackgroundProvider) SearchBackgrounds(c echo.Context) error {
}
// This function does all kinds of preparations for setting and uploading a background
func (bp *BackgroundProvider) setBackgroundPreparations(c echo.Context) (list *models.List, auth web.Auth, err error) {
func (bp *BackgroundProvider) setBackgroundPreparations(s *xorm.Session, c echo.Context) (list *models.List, auth web.Auth, err error) {
auth, err = auth2.GetAuthFromClaims(c)
if err != nil {
return nil, nil, echo.NewHTTPError(http.StatusBadRequest, "Invalid auth token: "+err.Error())
@ -81,7 +93,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,14 +102,18 @@ 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
}
// SetBackground sets an Image as list background
func (bp *BackgroundProvider) SetBackground(c echo.Context) error {
list, auth, err := bp.setBackgroundPreparations(c)
s := db.NewSession()
defer s.Close()
list, auth, err := bp.setBackgroundPreparations(s, c)
if err != nil {
_ = s.Rollback()
return handler.HandleHTTPError(err, c)
}
@ -106,11 +122,13 @@ func (bp *BackgroundProvider) SetBackground(c echo.Context) error {
image := &background.Image{}
err = c.Bind(image)
if err != nil {
_ = s.Rollback()
return echo.NewHTTPError(http.StatusBadRequest, "No or invalid model provided: "+err.Error())
}
err = p.Set(image, list, auth)
err = p.Set(s, image, list, auth)
if err != nil {
_ = s.Rollback()
return handler.HandleHTTPError(err, c)
}
return c.JSON(http.StatusOK, list)
@ -118,8 +136,12 @@ func (bp *BackgroundProvider) SetBackground(c echo.Context) error {
// UploadBackground uploads a background and passes the id of the uploaded file as an Image to the Set function of the BackgroundProvider.
func (bp *BackgroundProvider) UploadBackground(c echo.Context) error {
list, auth, err := bp.setBackgroundPreparations(c)
s := db.NewSession()
defer s.Close()
list, auth, err := bp.setBackgroundPreparations(s, c)
if err != nil {
_ = s.Rollback()
return handler.HandleHTTPError(err, c)
}
@ -128,10 +150,12 @@ func (bp *BackgroundProvider) UploadBackground(c echo.Context) error {
// Get + upload the image
file, err := c.FormFile("background")
if err != nil {
_ = s.Rollback()
return err
}
src, err := file.Open()
if err != nil {
_ = s.Rollback()
return err
}
defer src.Close()
@ -139,9 +163,11 @@ func (bp *BackgroundProvider) UploadBackground(c echo.Context) error {
// Validate we're dealing with an image
mime, err := mimetype.DetectReader(src)
if err != nil {
_ = s.Rollback()
return handler.HandleHTTPError(err, c)
}
if !strings.HasPrefix(mime.String(), "image") {
_ = s.Rollback()
return c.JSON(http.StatusBadRequest, models.Message{Message: "Uploaded file is no image."})
}
_, _ = src.Seek(0, io.SeekStart)
@ -149,6 +175,7 @@ func (bp *BackgroundProvider) UploadBackground(c echo.Context) error {
// Save the file
f, err := files.CreateWithMime(src, file.Filename, uint64(file.Size), auth, mime.String())
if err != nil {
_ = s.Rollback()
if files.IsErrFileIsTooLarge(err) {
return echo.ErrBadRequest
}
@ -158,10 +185,17 @@ func (bp *BackgroundProvider) UploadBackground(c echo.Context) error {
image := &background.Image{ID: strconv.FormatInt(f.ID, 10)}
err = p.Set(image, list, auth)
err = p.Set(s, image, list, auth)
if err != nil {
_ = s.Rollback()
return handler.HandleHTTPError(err, c)
}
if err := s.Commit(); err != nil {
_ = s.Rollback()
return handler.HandleHTTPError(err, c)
}
return c.JSON(http.StatusOK, list)
}
@ -190,17 +224,23 @@ func GetListBackground(c echo.Context) error {
return echo.NewHTTPError(http.StatusBadRequest, "Invalid list ID: "+err.Error())
}
s := db.NewSession()
defer s.Close()
// Check if a background for this list exists + Rights
list := &models.List{ID: listID}
can, _, err := list.CanRead(auth)
can, _, err := list.CanRead(s, auth)
if err != nil {
_ = s.Rollback()
return handler.HandleHTTPError(err, c)
}
if !can {
_ = s.Rollback()
log.Infof("Tried to get list background of list %d while not having the rights for it (User: %v)", listID, auth)
return echo.NewHTTPError(http.StatusForbidden)
}
if list.BackgroundFileID == 0 {
_ = s.Rollback()
return echo.NotFoundHandler(c)
}
@ -209,13 +249,19 @@ func GetListBackground(c echo.Context) error {
ID: list.BackgroundFileID,
}
if err := bgFile.LoadFileByID(); err != nil {
_ = s.Rollback()
return handler.HandleHTTPError(err, c)
}
// Unsplash requires pingbacks as per their api usage guidelines.
// To do this in a privacy-preserving manner, we do the ping from inside of Vikunja to not expose any user details.
// FIXME: This should use an event once we have events
unsplash.Pingback(bgFile)
unsplash.Pingback(s, bgFile)
if err := s.Commit(); err != nil {
_ = s.Rollback()
return handler.HandleHTTPError(err, c)
}
// Serve the file
return c.Stream(http.StatusOK, "image/jpg", bgFile.File)

View File

@ -26,6 +26,8 @@ import (
"strings"
"time"
"xorm.io/xorm"
"code.vikunja.io/api/pkg/config"
"code.vikunja.io/api/pkg/files"
"code.vikunja.io/api/pkg/log"
@ -150,7 +152,7 @@ func getUnsplashPhotoInfoByID(photoID string) (photo *Photo, err error) {
// @Success 200 {array} background.Image "An array with photos"
// @Failure 500 {object} models.Message "Internal error"
// @Router /backgrounds/unsplash/search [get]
func (p *Provider) Search(search string, page int64) (result []*background.Image, err error) {
func (p *Provider) Search(s *xorm.Session, search string, page int64) (result []*background.Image, err error) {
// If we don't have a search query, return results from the unsplash featured collection
if search == "" {
@ -243,7 +245,7 @@ func (p *Provider) Search(search string, page int64) (result []*background.Image
// @Failure 403 {object} web.HTTPError "The user does not have access to the list"
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{id}/backgrounds/unsplash [post]
func (p *Provider) Set(image *background.Image, list *models.List, auth web.Auth) (err error) {
func (p *Provider) Set(s *xorm.Session, image *background.Image, list *models.List, auth web.Auth) (err error) {
// Find the photo
photo, err := getUnsplashPhotoInfoByID(image.ID)
@ -292,7 +294,7 @@ func (p *Provider) Set(image *background.Image, list *models.List, auth web.Auth
return err
}
if err := models.RemoveUnsplashPhoto(list.BackgroundFileID); err != nil {
if err := models.RemoveUnsplashPhoto(s, list.BackgroundFileID); err != nil {
return err
}
}
@ -304,7 +306,7 @@ func (p *Provider) Set(image *background.Image, list *models.List, auth web.Auth
Author: photo.User.Username,
AuthorName: photo.User.Name,
}
err = unsplashPhoto.Save()
err = unsplashPhoto.Save(s)
if err != nil {
return
}
@ -315,13 +317,13 @@ func (p *Provider) Set(image *background.Image, list *models.List, auth web.Auth
list.BackgroundInformation = unsplashPhoto
// Set it as the list background
return models.SetListBackground(list.ID, file)
return models.SetListBackground(s, list.ID, file)
}
// Pingback pings the unsplash api if an unsplash photo has been accessed.
func Pingback(f *files.File) {
func Pingback(s *xorm.Session, f *files.File) {
// Check if the file is actually downloaded from unsplash
unsplashPhoto, err := models.GetUnsplashPhotoByFileID(f.ID)
unsplashPhoto, err := models.GetUnsplashPhotoByFileID(s, f.ID)
if err != nil {
if files.IsErrFileIsNotUnsplashFile(err) {
return

View File

@ -19,6 +19,8 @@ package upload
import (
"strconv"
"xorm.io/xorm"
"code.vikunja.io/api/pkg/files"
"code.vikunja.io/api/pkg/models"
"code.vikunja.io/api/pkg/modules/background"
@ -30,7 +32,7 @@ type Provider struct {
}
// Search is only used to implement the interface
func (p *Provider) Search(search string, page int64) (result []*background.Image, err error) {
func (p *Provider) Search(s *xorm.Session, search string, page int64) (result []*background.Image, err error) {
return
}
@ -50,7 +52,7 @@ func (p *Provider) Search(search string, page int64) (result []*background.Image
// @Failure 404 {object} models.Message "The list does not exist."
// @Failure 500 {object} models.Message "Internal error"
// @Router /lists/{id}/backgrounds/upload [put]
func (p *Provider) Set(image *background.Image, list *models.List, auth web.Auth) (err error) {
func (p *Provider) Set(s *xorm.Session, image *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}
@ -67,5 +69,5 @@ func (p *Provider) Set(image *background.Image, list *models.List, auth web.Auth
list.BackgroundInformation = &models.ListBackgroundType{Type: models.ListBackgroundUpload}
return models.SetListBackground(list.ID, file)
return models.SetListBackground(s, list.ID, file)
}

View File

@ -20,6 +20,8 @@ import (
"bytes"
"io/ioutil"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/files"
"code.vikunja.io/api/pkg/log"
"code.vikunja.io/api/pkg/models"
@ -34,10 +36,14 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
labels := make(map[string]*models.Label)
s := db.NewSession()
defer s.Close()
// Create all namespaces
for _, n := range str {
err = n.Create(user)
err = n.Create(s, user)
if err != nil {
_ = s.Rollback()
return
}
@ -54,8 +60,9 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
needsDefaultBucket := false
l.NamespaceID = n.ID
err = l.Create(user)
err = l.Create(s, user)
if err != nil {
_ = s.Rollback()
return
}
@ -67,11 +74,13 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
file, err := files.Create(backgroundFile, "", uint64(backgroundFile.Len()), user)
if err != nil {
_ = s.Rollback()
return err
}
err = models.SetListBackground(l.ID, file)
err = models.SetListBackground(s, l.ID, file)
if err != nil {
_ = s.Rollback()
return err
}
@ -87,8 +96,9 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
oldID := bucket.ID
bucket.ID = 0 // We want a new id
bucket.ListID = l.ID
err = bucket.Create(user)
err = bucket.Create(s, user)
if err != nil {
_ = s.Rollback()
return
}
buckets[oldID] = bucket
@ -111,8 +121,9 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
}
t.ListID = l.ID
err = t.Create(user)
err = t.Create(s, user)
if err != nil {
_ = s.Rollback()
return
}
@ -132,8 +143,9 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
// First create the related tasks if they do not exist
if rt.ID == 0 {
rt.ListID = t.ListID
err = rt.Create(user)
err = rt.Create(s, user)
if err != nil {
_ = s.Rollback()
return
}
log.Debugf("[creating structure] Created related task %d", rt.ID)
@ -145,8 +157,9 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
OtherTaskID: rt.ID,
RelationKind: kind,
}
err = taskRel.Create(user)
err = taskRel.Create(s, user)
if err != nil {
_ = s.Rollback()
return
}
@ -164,8 +177,9 @@ 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 {
_ = s.Rollback()
return
}
log.Debugf("[creating structure] Created new attachment %d", a.ID)
@ -180,8 +194,9 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
var exists bool
lb, exists = labels[label.Title+label.HexColor]
if !exists {
err = label.Create(user)
err = label.Create(s, user)
if err != nil {
_ = s.Rollback()
return err
}
log.Debugf("[creating structure] Created new label %d", label.ID)
@ -193,8 +208,9 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
LabelID: lb.ID,
TaskID: t.ID,
}
err = lt.Create(user)
err = lt.Create(s, user)
if err != nil {
_ = s.Rollback()
return err
}
log.Debugf("[creating structure] Associated task %d with label %d", t.ID, lb.ID)
@ -204,13 +220,15 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
// All tasks brought their own bucket with them, therefore the newly created default bucket is just extra space
if !needsDefaultBucket {
b := &models.Bucket{ListID: l.ID}
bucketsIn, _, _, err := b.ReadAll(user, "", 1, 1)
bucketsIn, _, _, err := b.ReadAll(s, user, "", 1, 1)
if err != nil {
_ = s.Rollback()
return err
}
buckets := bucketsIn.([]*models.Bucket)
err = buckets[0].Delete()
err = buckets[0].Delete(s)
if err != nil {
_ = s.Rollback()
return err
}
}
@ -222,5 +240,5 @@ func InsertFromStructure(str []*models.NamespaceWithLists, user *user.User) (err
log.Debugf("[creating structure] Done inserting new task structure")
return nil
return s.Commit()
}

View File

@ -19,20 +19,10 @@ package migration
import (
"code.vikunja.io/api/pkg/config"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/log"
"xorm.io/xorm"
)
var x *xorm.Engine
// InitDB sets up the database connection to use in this module
func InitDB() (err error) {
x, err = db.CreateDBEngine()
if err != nil {
log.Criticalf("Could not connect to db: %v", err.Error())
return
}
// Cache
if config.CacheEnabled.GetBool() && config.CacheType.GetString() == "redis" {
db.RegisterTableStructsForCache(GetTables())

View File

@ -19,6 +19,7 @@ package migration
import (
"time"
"code.vikunja.io/api/pkg/db"
"code.vikunja.io/api/pkg/user"
)
@ -37,17 +38,26 @@ func (s *Status) TableName() string {
// SetMigrationStatus sets the migration status for a user
func SetMigrationStatus(m Migrator, u *user.User) (err error) {
s := db.NewSession()
defer s.Close()
status := &Status{
UserID: u.ID,
MigratorName: m.Name(),
}
_, err = x.Insert(status)
_, err = s.Insert(status)
return
}
// GetMigrationStatus returns the migration status for a migration and a user
func GetMigrationStatus(m Migrator, u *user.User) (status *Status, err error) {
s := db.NewSession()
defer s.Close()
status = &Status{}
_, err = x.Where("user_id = ? and migrator_name = ?", u.ID, m.Name()).Desc("id").Get(status)
_, err = s.
Where("user_id = ? and migrator_name = ?", u.ID, m.Name()).
Desc("id").
Get(status)
return
}