Refactoring part 2.3: Long func refactoring in graphql/models (#1075)

* Split long functions and optimize code

* 2 more small files

---------

Co-authored-by: Konstantin Koval

api/graphql/models/actions/album_actions.go

@@ -6,7 +6,9 @@ import (
 	"gorm.io/gorm"
 )
 
-func MyAlbums(db *gorm.DB, user *models.User, order *models.Ordering, paginate *models.Pagination, onlyRoot *bool, showEmpty *bool, onlyWithFavorites *bool) ([]*models.Album, error) {
+func MyAlbums(db *gorm.DB, user *models.User, order *models.Ordering, paginate *models.Pagination,
+	onlyRoot *bool, showEmpty *bool, onlyWithFavorites *bool) ([]*models.Album, error) {
+
 	if err := user.FillAlbums(db); err != nil {
 		return nil, err
 	}
@@ -24,17 +26,7 @@ func MyAlbums(db *gorm.DB, user *models.User, order *models.Ordering, paginate *
 
 	if onlyRoot != nil && *onlyRoot {
 
-		var singleRootAlbumID int = -1
+		singleRootAlbumID := getSingleRootAlbumID(user)
-		for _, album := range user.Albums {
-			if album.ParentAlbumID == nil {
-				if singleRootAlbumID == -1 {
-					singleRootAlbumID = album.ID
-				} else {
-					singleRootAlbumID = -1
-					break
-				}
-			}
-		}
 
 		if singleRootAlbumID != -1 && len(user.Albums) > 1 {
 			query = query.Where("parent_album_id = ?", singleRootAlbumID)
@@ -43,6 +35,34 @@ func MyAlbums(db *gorm.DB, user *models.User, order *models.Ordering, paginate *
 		}
 	}
 
+	query = favoritesQuery(showEmpty, db, onlyWithFavorites, user, query)
+
+	query = models.FormatSQL(query, order, paginate)
+
+	var albums []*models.Album
+	if err := query.Find(&albums).Error; err != nil {
+		return nil, err
+	}
+
+	return albums, nil
+}
+
+func getSingleRootAlbumID(user *models.User) int {
+	var singleRootAlbumID int = -1
+	for _, album := range user.Albums {
+		if album.ParentAlbumID == nil {
+			if singleRootAlbumID == -1 {
+				singleRootAlbumID = album.ID
+			} else {
+				singleRootAlbumID = -1
+				break
+			}
+		}
+	}
+	return singleRootAlbumID
+}
+
+func favoritesQuery(showEmpty *bool, db *gorm.DB, onlyWithFavorites *bool, user *models.User, query *gorm.DB) *gorm.DB {
 	if showEmpty == nil || !*showEmpty {
 		subQuery := db.Model(&models.Media{}).Where("album_id = albums.id")
 
@@ -57,15 +77,7 @@ func MyAlbums(db *gorm.DB, user *models.User, order *models.Ordering, paginate *
 
 		query = query.Where("EXISTS (?)", subQuery)
 	}
-
+	return query
-	query = models.FormatSQL(query, order, paginate)
-
-	var albums []*models.Album
-	if err := query.Find(&albums).Error; err != nil {
-		return nil, err
-	}
-
-	return albums, nil
 }
 
 func Album(db *gorm.DB, user *models.User, id int) (*models.Album, error) {

api/graphql/models/actions/media_actions.go

@@ -10,7 +10,8 @@ func MyMedia(db *gorm.DB, user *models.User, order *models.Ordering, paginate *m
 		return nil, err
 	}
 
-	query := db.Where("media.album_id IN (SELECT user_albums.album_id FROM user_albums WHERE user_albums.user_id = ?)", user.ID)
+	query := db.Where("media.album_id IN (SELECT user_albums.album_id FROM user_albums WHERE user_albums.user_id = ?)",
+		user.ID)
 	query = models.FormatSQL(query, order, paginate)
 
 	var media []*models.Media

api/graphql/models/actions/search_actions.go

@@ -38,7 +38,7 @@ func Search(db *gorm.DB, query string, userID int, limitMedia *int, limitAlbums
 		Where("LOWER(media.title) LIKE ? OR LOWER(media.path) LIKE ?", wildQuery, wildQuery).
 		Clauses(clause.OrderBy{
 			Expression: clause.Expr{
-				SQL:                "(CASE WHEN LOWER(media.title) LIKE ? THEN 2 WHEN LOWER(media.path) LIKE ? THEN 1 END) DESC",
+				SQL:    "(CASE WHEN LOWER(media.title) LIKE ? THEN 2 WHEN LOWER(media.path) LIKE ? THEN 1 END) DESC",
 				Vars:               []interface{}{wildQuery, wildQuery},
 				WithoutParentheses: true},
 		}).

api/graphql/models/actions/share_token_actions.go

@@ -12,7 +12,9 @@ import (
 	"gorm.io/gorm"
 )
 
-func AddMediaShare(db *gorm.DB, user *models.User, mediaID int, expire *time.Time, password *string) (*models.ShareToken, error) {
+func AddMediaShare(db *gorm.DB, user *models.User, mediaID int, expire *time.Time, password *string) (*models.ShareToken,
+	error) {
+
 	var media models.Media
 
 	var query string
@@ -56,11 +58,14 @@ func AddMediaShare(db *gorm.DB, user *models.User, mediaID int, expire *time.Tim
 	return &shareToken, nil
 }
 
-func AddAlbumShare(db *gorm.DB, user *models.User, albumID int, expire *time.Time, password *string) (*models.ShareToken, error) {
+func AddAlbumShare(db *gorm.DB, user *models.User, albumID int, expire *time.Time, password *string) (*models.ShareToken,
+	error) {
+
 	var count int64
 	err := db.
 		Model(&models.Album{}).
-		Where("EXISTS (SELECT * FROM user_albums WHERE user_albums.album_id = albums.id AND user_albums.user_id = ?)", user.ID).
+		Where("EXISTS (SELECT * FROM user_albums WHERE user_albums.album_id = albums.id AND user_albums.user_id = ?)",
+			user.ID).
 		Count(&count).Error
 
 	if err != nil {

api/graphql/models/actions/timeline_actions.go

@@ -8,7 +8,9 @@ import (
 	"gorm.io/gorm"
 )
 
-func MyTimeline(db *gorm.DB, user *models.User, paginate *models.Pagination, onlyFavorites *bool, fromDate *time.Time) ([]*models.Media, error) {
+func MyTimeline(db *gorm.DB, user *models.User, paginate *models.Pagination, onlyFavorites *bool,
+	fromDate *time.Time) ([]*models.Media, error) {
+
 	const albumsTitleASC = "albums.title ASC"
 
 	query := db.
@@ -42,7 +44,11 @@ func MyTimeline(db *gorm.DB, user *models.User, paginate *models.Pagination, onl
 	}
 
 	if onlyFavorites != nil && *onlyFavorites {
-		query = query.Where("media.id IN (?)", db.Table("user_media_data").Select("user_media_data.media_id").Where("user_media_data.user_id = ?", user.ID).Where("user_media_data.favorite"))
+		query = query.
+			Where("media.id IN (?)", db.Table("user_media_data").
+				Select("user_media_data.media_id").
+				Where("user_media_data.user_id = ?", user.ID).
+				Where("user_media_data.favorite"))
 	}
 
 	query = models.FormatSQL(query, nil, paginate)

api/graphql/models/actions/user_actions.go

@@ -23,32 +23,27 @@ func DeleteUser(db *gorm.DB, userID int) (*models.User, error) {
 	var user models.User
 	deletedAlbumIDs := make([]int, 0)
 
-	err := db.Transaction(func(tx *gorm.DB) error {
+	var err error
-		if err := tx.First(&user, userID).Error; err != nil {
+	err = db.Transaction(func(tx *gorm.DB) error {
+		if err = tx.First(&user, userID).Error; err != nil {
 			return err
 		}
 
 		userAlbums := user.Albums
-		if err := tx.Model(&user).Association("Albums").Find(&userAlbums); err != nil {
+		if err = tx.Model(&user).Association("Albums").Find(&userAlbums); err != nil {
 			return err
 		}
 
-		if err := tx.Model(&user).Association("Albums").Clear(); err != nil {
+		if err = tx.Model(&user).Association("Albums").Clear(); err != nil {
 			return err
 		}
 
-		for _, album := range userAlbums {
+		deletedAlbumIDs, err = deleteNotOwnedAlbums(userAlbums, tx, deletedAlbumIDs)
-			var associatedUsers = tx.Model(album).Association("Owners").Count()
+		if err != nil {
-
+			return err
-			if associatedUsers == 0 {
-				deletedAlbumIDs = append(deletedAlbumIDs, album.ID)
-				if err := tx.Delete(album).Error; err != nil {
-					return err
-				}
-			}
 		}
 
-		if err := tx.Delete(&user).Error; err != nil {
+		if err = tx.Delete(&user).Error; err != nil {
 			return err
 		}
 
@@ -60,11 +55,30 @@ func DeleteUser(db *gorm.DB, userID int) (*models.User, error) {
 	}
 
 	// If there is only one associated user, clean up the cache folder and delete the album row
+	return &user, cleanup(deletedAlbumIDs)
+}
+
+func cleanup(deletedAlbumIDs []int) error {
+	var err error
 	for _, deletedAlbumID := range deletedAlbumIDs {
 		cachePath := path.Join(utils.MediaCachePath(), strconv.Itoa(int(deletedAlbumID)))
-		if err := os.RemoveAll(cachePath); err != nil {
+		if err = os.RemoveAll(cachePath); err != nil {
-			return &user, err
+			return err
 		}
 	}
-	return &user, nil
+	return err
+}
+
+func deleteNotOwnedAlbums(userAlbums []models.Album, tx *gorm.DB, deletedAlbumIDs []int) ([]int, error) {
+	for _, album := range userAlbums {
+		var associatedUsers = tx.Model(album).Association("Owners").Count()
+
+		if associatedUsers == 0 {
+			deletedAlbumIDs = append(deletedAlbumIDs, album.ID)
+			if err := tx.Delete(album).Error; err != nil {
+				return nil, err
+			}
+		}
+	}
+	return deletedAlbumIDs, nil
 }

api/graphql/models/media.go

@@ -135,7 +135,8 @@ func (p *MediaURL) CachedPath() (string, error) {
 	}
 
 	if p.Purpose == PhotoThumbnail || p.Purpose == PhotoHighRes || p.Purpose == VideoThumbnail || p.Purpose == VideoWeb {
-		cachedPath = path.Join(utils.MediaCachePath(), strconv.Itoa(int(p.Media.AlbumID)), strconv.Itoa(int(p.MediaID)), p.MediaName)
+		cachedPath = path.Join(utils.MediaCachePath(), strconv.Itoa(int(p.Media.AlbumID)), strconv.Itoa(int(p.MediaID)),
+			p.MediaName)
 	} else if p.Purpose == MediaOriginal {
 		cachedPath = p.Media.Path
 	} else {

api/graphql/models/user.go

@@ -166,7 +166,9 @@ func (user *User) FillAlbums(db *gorm.DB) error {
 
 func (user *User) OwnsAlbum(db *gorm.DB, album *Album) (bool, error) {
 	filter := func(query *gorm.DB) *gorm.DB {
-		return query.Where("EXISTS (SELECT 1 FROM user_albums WHERE user_albums.user_id = ? AND user_albums.album_id = id LIMIT 1)", user.ID)
+		return query.Where(
+			"EXISTS (SELECT 1 FROM user_albums WHERE user_albums.user_id = ? AND user_albums.album_id = id LIMIT 1)",
+			user.ID)
 	}
 
 	ownedParents, err := album.GetParents(db, filter)