From 542e5c853262dac4a47455f552e3588e6be318da Mon Sep 17 00:00:00 2001 From: viktorstrate Date: Mon, 26 Apr 2021 20:37:29 +0200 Subject: [PATCH] Improve scanner_album NewRootPath tests --- api/database/database.go | 11 ++++++---- api/graphql/models/album.go | 2 +- api/graphql/models/user.go | 21 ++++++------------- api/graphql/resolvers/user.go | 35 ------------------------------- api/scanner/scanner_album.go | 28 ++++++++++++++++++++++++- api/scanner/scanner_album_test.go | 34 ++++++++++++++++++++++++++++-- api/test_utils/test_db_manager.go | 8 ++++++- 7 files changed, 80 insertions(+), 59 deletions(-) diff --git a/api/database/database.go b/api/database/database.go index cc429368..ac94e125 100644 --- a/api/database/database.go +++ b/api/database/database.go @@ -159,6 +159,7 @@ var database_models []interface{} = []interface{}{ &models.VideoMetadata{}, &models.ShareToken{}, &models.UserMediaData{}, + &models.UserAlbums{}, &models.UserPreferences{}, // Face detection @@ -167,9 +168,12 @@ var database_models []interface{} = []interface{}{ } func MigrateDatabase(db *gorm.DB) error { - err := db.AutoMigrate(database_models...) - if err != nil { + if err := db.SetupJoinTable(&models.User{}, "Albums", &models.UserAlbums{}); err != nil { + log.Printf("Setup UserAlbums join table failed: %v\n", err) + } + + if err := db.AutoMigrate(database_models...); err != nil { log.Printf("Auto migration failed: %v\n", err) } @@ -180,8 +184,7 @@ func MigrateDatabase(db *gorm.DB) error { // v2.3.0 - Changed type of MediaEXIF.Exposure and MediaEXIF.Flash // from string values to decimal and int respectively - err = migrate_exif_fields(db) - if err != nil { + if err := migrate_exif_fields(db); err != nil { log.Printf("Failed to run exif fields migration: %v\n", err) } diff --git a/api/graphql/models/album.go b/api/graphql/models/album.go index 9be3833c..79294e84 100644 --- a/api/graphql/models/album.go +++ b/api/graphql/models/album.go @@ -14,7 +14,7 @@ type Album struct { ParentAlbum *Album `gorm:"constraint:OnDelete:SET NULL;"` // OwnerID int `gorm:"not null"` // Owner User - Owners []User `gorm:"many2many:user_albums"` + Owners []User `gorm:"many2many:user_albums;constraint:OnDelete:CASCADE;"` Path string `gorm:"not null"` PathHash string `gorm:"unique"` } diff --git a/api/graphql/models/user.go b/api/graphql/models/user.go index 777e7068..8abdb81f 100644 --- a/api/graphql/models/user.go +++ b/api/graphql/models/user.go @@ -3,8 +3,6 @@ package models import ( "crypto/rand" "fmt" - "log" - "os" "time" "github.com/pkg/errors" @@ -17,7 +15,7 @@ type User struct { Username string `gorm:"unique;size:128"` Password *string `gorm:"size:256"` // RootPath string `gorm:"size:512` - Albums []Album `gorm:"many2many:user_albums"` + Albums []Album `gorm:"many2many:user_albums;constraint:OnDelete:CASCADE;"` Admin bool `gorm:"default:false"` } @@ -28,6 +26,11 @@ type UserMediaData struct { Favorite bool `gorm:"not null;default:false"` } +type UserAlbums struct { + UserID int `gorm:"primaryKey;autoIncrement:false;constraint:OnDelete:CASCADE;"` + AlbumID int `gorm:"primaryKey;autoIncrement:false;constraint:OnDelete:CASCADE;"` +} + type AccessToken struct { Model UserID int `gorm:"not null;index"` @@ -95,18 +98,6 @@ func AuthorizeUser(db *gorm.DB, username string, password string) (*User, error) return &user, nil } -var ErrorInvalidRootPath = errors.New("invalid root path") - -func ValidRootPath(rootPath string) bool { - _, err := os.Stat(rootPath) - if err != nil { - log.Printf("Warn: invalid root path: '%s'\n%s\n", rootPath, err) - return false - } - - return true -} - func RegisterUser(db *gorm.DB, username string, password *string, admin bool) (*User, error) { user := User{ Username: username, diff --git a/api/graphql/resolvers/user.go b/api/graphql/resolvers/user.go index 15547ffc..578baef7 100644 --- a/api/graphql/resolvers/user.go +++ b/api/graphql/resolvers/user.go @@ -2,11 +2,9 @@ package resolvers import ( "context" - "fmt" "os" "path" "strconv" - "strings" api "github.com/photoview/photoview/api/graphql" "github.com/photoview/photoview/api/graphql/auth" @@ -314,39 +312,6 @@ func (r *mutationResolver) UserAddRootPath(ctx context.Context, id int, rootPath return nil, err } - if !models.ValidRootPath(rootPath) { - return nil, errors.New("invalid root path") - } - - upperPaths := make([]string, 1) - upperPath := rootPath - upperPaths[0] = upperPath - for { - - substrIndex := strings.LastIndex(upperPath, "/") - if substrIndex == -1 { - break - } - - if substrIndex == 0 { - upperPaths = append(upperPaths, "/") - break - } - - upperPath = upperPath[0:substrIndex] - upperPaths = append(upperPaths, upperPath) - } - - var upperAlbums []models.Album - if err := r.Database.Model(&user).Association("Albums").Find(&upperAlbums, "albums.path IN (?)", upperPaths); err != nil { - // if err := r.Database.Model(models.Album{}).Where("path IN (?)", upperPaths).Find(&upperAlbums).Error; err != nil { - return nil, err - } - - if len(upperAlbums) > 0 { - return nil, errors.New(fmt.Sprintf("user already owns a path containing this path: %s", upperAlbums[0].Path)) - } - newAlbum, err := scanner.NewRootAlbum(r.Database, rootPath, &user) if err != nil { return nil, err diff --git a/api/scanner/scanner_album.go b/api/scanner/scanner_album.go index d998c981..ebd4a2ac 100644 --- a/api/scanner/scanner_album.go +++ b/api/scanner/scanner_album.go @@ -4,6 +4,7 @@ import ( "fmt" "io/ioutil" "log" + "os" "path" "time" @@ -18,6 +19,19 @@ import ( func NewRootAlbum(db *gorm.DB, rootPath string, owner *models.User) (*models.Album, error) { + if !ValidRootPath(rootPath) { + return nil, ErrorInvalidRootPath + } + + if !path.IsAbs(rootPath) { + wd, err := os.Getwd() + if err != nil { + return nil, err + } + + rootPath = path.Join(wd, rootPath) + } + owners := []models.User{ *owner, } @@ -36,7 +50,7 @@ func NewRootAlbum(db *gorm.DB, rootPath string, owner *models.User) (*models.Alb } if matchedUserAlbumCount > 0 { - return nil, errors.New(fmt.Sprintf("user already owns path (%s)", rootPath)) + return nil, errors.New(fmt.Sprintf("user already owns a path containing this path: %s", rootPath)) } if err := db.Model(&owner).Association("Albums").Append(&album); err != nil { @@ -59,6 +73,18 @@ func NewRootAlbum(db *gorm.DB, rootPath string, owner *models.User) (*models.Alb } } +var ErrorInvalidRootPath = errors.New("invalid root path") + +func ValidRootPath(rootPath string) bool { + _, err := os.Stat(rootPath) + if err != nil { + log.Printf("Warn: invalid root path: '%s'\n%s\n", rootPath, err) + return false + } + + return true +} + func scanAlbum(album *models.Album, cache *AlbumScannerCache, db *gorm.DB) { album_notify_key := utils.GenerateToken() diff --git a/api/scanner/scanner_album_test.go b/api/scanner/scanner_album_test.go index f234354c..dc3f7175 100644 --- a/api/scanner/scanner_album_test.go +++ b/api/scanner/scanner_album_test.go @@ -27,7 +27,7 @@ func TestNewRootPath(t *testing.T) { } assert.NotNil(t, album) - assert.Equal(t, "./test_data", album.Path) + assert.Contains(t, album.Path, "/api/scanner/test_data") assert.NotEmpty(t, album.Owners) }) @@ -36,7 +36,37 @@ func TestNewRootPath(t *testing.T) { _, err := scanner.NewRootAlbum(db, "./test_data", &user) assert.Error(t, err) - assert.Equal(t, err.Error(), "user already owns path (./test_data)") + assert.Contains(t, err.Error(), "user already owns a path containing this path:") + }) + + t.Run("Insert invalid root album", func(t *testing.T) { + + _, err := scanner.NewRootAlbum(db, "./invalid_path", &user) + + assert.Error(t, err) + assert.Equal(t, err.Error(), "invalid root path") + }) + + t.Run("Add existing root album to new user", func(t *testing.T) { + + user2 := models.User{ + Username: "user2", + } + + if !assert.NoError(t, db.Save(&user2).Error) { + return + } + + album, err := scanner.NewRootAlbum(db, "./test_data", &user2) + if !assert.NoError(t, err) { + return + } + + assert.NotNil(t, album) + assert.Contains(t, album.Path, "/api/scanner/test_data") + + owner_count := db.Model(&album).Association("Owners").Count() + assert.EqualValues(t, 2, owner_count) }) } diff --git a/api/test_utils/test_db_manager.go b/api/test_utils/test_db_manager.go index 58806941..ff108285 100644 --- a/api/test_utils/test_db_manager.go +++ b/api/test_utils/test_db_manager.go @@ -23,6 +23,10 @@ func (dbm *TestDBManager) Close() error { return nil } + if err := dbm.reset(); err != nil { + return err + } + sqlDB, err := dbm.DB.DB() if err != nil { return errors.Wrap(err, "get db instance when closing test database") @@ -47,7 +51,9 @@ func (dbm *TestDBManager) setup() error { dbm.DB = db - dbm.reset() + if err := dbm.reset(); err != nil { + return err + } return nil }