Refactoring part 1: Fix some annoying linter warnings in API code (#1064)

* names, constants of reusable strings, removed unneeded `if`, replaced deprecated imports

* more deprecated imports replaced

* What is this?))

* Fix a path and quote a var in Dockerfile

* Addressing review comments

---------

Co-authored-by: Konstantin Koval <kkb@ukr.net>

api/graphql/models/actions/album_actions.go

@@ -90,7 +90,7 @@ func Album(db *gorm.DB, user *models.User, id int) (*models.Album, error) {
 }
 
 func AlbumPath(db *gorm.DB, user *models.User, album *models.Album) ([]*models.Album, error) {
-	var album_path []*models.Album
+	var albumPath []*models.Album
 
 	err := db.Raw(`
 		WITH recursive path_albums AS (
@@ -99,11 +99,11 @@ func AlbumPath(db *gorm.DB, user *models.User, album *models.Album) ([]*models.A
 			SELECT parent.* FROM path_albums child JOIN albums parent ON parent.id = child.parent_album_id
 		)
 		SELECT * FROM path_albums WHERE id != ?
-	`, album.ID, album.ID).Scan(&album_path).Error
+	`, album.ID, album.ID).Scan(&albumPath).Error
 
 	// Make sure to only return albums this user owns
-	for i := len(album_path) - 1; i >= 0; i-- {
-		album := album_path[i]
+	for i := len(albumPath) - 1; i >= 0; i-- {
+		album := albumPath[i]
 
 		owns, err := user.OwnsAlbum(db, album)
 		if err != nil {
@@ -111,7 +111,7 @@ func AlbumPath(db *gorm.DB, user *models.User, album *models.Album) ([]*models.A
 		}
 
 		if !owns {
-			album_path = album_path[i+1:]
+			albumPath = albumPath[i+1:]
 			break
 		}
 
@@ -121,7 +121,7 @@ func AlbumPath(db *gorm.DB, user *models.User, album *models.Album) ([]*models.A
 		return nil, err
 	}
 
-	return album_path, nil
+	return albumPath, nil
 }
 
 func SetAlbumCover(db *gorm.DB, user *models.User, mediaID int) (*models.Album, error) {

api/graphql/models/actions/album_actions_test.go

@@ -140,8 +140,8 @@ func TestAlbumCover(t *testing.T) {
 		return
 	}
 
-	user_pass := "password"
-	regularUser, err := models.RegisterUser(db, "user1", &user_pass, false)
+	userPass := "password"
+	regularUser, err := models.RegisterUser(db, "user1", &userPass, false)
 	if !assert.NoError(t, err) {
 		return
 	}

api/graphql/models/actions/search_actions.go

@@ -10,16 +10,16 @@ import (
 	"gorm.io/gorm/clause"
 )
 
-func Search(db *gorm.DB, query string, userID int, _limitMedia *int, _limitAlbums *int) (*models.SearchResult, error) {
-	limitMedia := 10
-	limitAlbums := 10
+func Search(db *gorm.DB, query string, userID int, limitMedia *int, limitAlbums *int) (*models.SearchResult, error) {
+	limitMediaInternal := 10
+	limitAlbumsInternal := 10
 
-	if _limitMedia != nil {
-		limitMedia = *_limitMedia
+	if limitMedia != nil {
+		limitMediaInternal = *limitMedia
 	}
 
-	if _limitAlbums != nil {
-		limitAlbums = *_limitAlbums
+	if limitAlbums != nil {
+		limitAlbumsInternal = *limitAlbums
 	}
 
 	wildQuery := "%" + strings.ToLower(query) + "%"
@@ -42,7 +42,7 @@ func Search(db *gorm.DB, query string, userID int, _limitMedia *int, _limitAlbum
 				Vars:               []interface{}{wildQuery, wildQuery},
 				WithoutParentheses: true},
 		}).
-		Limit(limitMedia).Find(&media).Error
+		Limit(limitMediaInternal).Find(&media).Error
 
 	if err != nil {
 		return nil, errors.Wrapf(err, "searching media")
@@ -59,7 +59,7 @@ func Search(db *gorm.DB, query string, userID int, _limitMedia *int, _limitAlbum
 				Vars:               []interface{}{wildQuery, wildQuery},
 				WithoutParentheses: true},
 		}).
-		Limit(limitAlbums).
+		Limit(limitAlbumsInternal).
 		Find(&albums).Error
 
 	if err != nil {

api/graphql/models/actions/timeline_actions.go

@@ -9,6 +9,7 @@ import (
 )
 
 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.
 		Joins("JOIN albums ON media.album_id = albums.id").
@@ -20,19 +21,19 @@ func MyTimeline(db *gorm.DB, user *models.User, paginate *models.Pagination, onl
 			Order("DATE_TRUNC('year', date_shot) DESC").
 			Order("DATE_TRUNC('month', date_shot) DESC").
 			Order("DATE_TRUNC('day', date_shot) DESC").
-			Order("albums.title ASC").
+			Order(albumsTitleASC).
 			Order("media.date_shot DESC")
 	case drivers.SQLITE:
 		query = query.
 			Order("strftime('%Y-%m-%d', media.date_shot) DESC"). // convert to YYYY-MM-DD
-			Order("albums.title ASC").
+			Order(albumsTitleASC).
 			Order("TIME(media.date_shot) DESC")
 	default:
 		query = query.
 			Order("YEAR(media.date_shot) DESC").
 			Order("MONTH(media.date_shot) DESC").
 			Order("DAY(media.date_shot) DESC").
-			Order("albums.title ASC").
+			Order(albumsTitleASC).
 			Order("TIME(media.date_shot) DESC")
 	}
 

api/graphql/models/album_test.go

@@ -9,11 +9,14 @@ import (
 )
 
 func TestAlbumGetChildrenAndParents(t *testing.T) {
+	const photosPath = "/photos"
+	const photosChild1Path = "/photos/child1"
+	const photosChild1SubchildPath = "/photos/child1/subchild"
 	db := test_utils.DatabaseTest(t)
 
 	rootAlbum := models.Album{
 		Title: "root",
-		Path:  "/photos",
+		Path:  photosPath,
 	}
 
 	if !assert.NoError(t, db.Save(&rootAlbum).Error) {
@@ -23,7 +26,7 @@ func TestAlbumGetChildrenAndParents(t *testing.T) {
 	children := []models.Album{
 		{
 			Title:         "child1",
-			Path:          "/photos/child1",
+			Path:          photosChild1Path,
 			ParentAlbumID: &rootAlbum.ID,
 		},
 		{
@@ -41,47 +44,47 @@ func TestAlbumGetChildrenAndParents(t *testing.T) {
 		return
 	}
 
-	sub_child := models.Album{
+	subChild := models.Album{
 		Title:         "subchild",
-		Path:          "/photos/child1/subchild",
+		Path:          photosChild1SubchildPath,
 		ParentAlbumID: &children[0].ID,
 	}
 
-	if !assert.NoError(t, db.Save(&sub_child).Error) {
+	if !assert.NoError(t, db.Save(&subChild).Error) {
 		return
 	}
 
-	verifyResult := func(t *testing.T, expected_albums []*models.Album, result []*models.Album) {
-		assert.Equal(t, len(expected_albums), len(result))
+	verifyResult := func(t *testing.T, expectedAlbums []*models.Album, result []*models.Album) {
+		assert.Equal(t, len(expectedAlbums), len(result))
 
-		for _, expected := range expected_albums {
-			found_expected := false
+		for _, expected := range expectedAlbums {
+			foundExpected := false
 			for _, item := range result {
 				if item.Title == expected.Title && item.Path == expected.Path {
-					found_expected = true
+					foundExpected = true
 					break
 				}
 			}
-			if !found_expected {
+			if !foundExpected {
 				assert.Failf(t, "albums did not match", "expected to find item: %v", expected)
 			}
 		}
 	}
 
 	t.Run("Album get children", func(t *testing.T) {
-		root_children, err := rootAlbum.GetChildren(db, nil)
+		rootChildren, err := rootAlbum.GetChildren(db, nil)
 		if !assert.NoError(t, err) {
 			return
 		}
 
-		expected_children := []*models.Album{
+		expectedChildren := []*models.Album{
 			{
 				Title: "root",
-				Path:  "/photos",
+				Path:  photosPath,
 			},
 			{
 				Title: "child1",
-				Path:  "/photos/child1",
+				Path:  photosChild1Path,
 			},
 			{
 				Title: "child2",
@@ -89,35 +92,35 @@ func TestAlbumGetChildrenAndParents(t *testing.T) {
 			},
 			{
 				Title: "subchild",
-				Path:  "/photos/child1/subchild",
+				Path:  photosChild1SubchildPath,
 			},
 		}
 
-		verifyResult(t, expected_children, root_children)
+		verifyResult(t, expectedChildren, rootChildren)
 	})
 
 	t.Run("Album get parents", func(t *testing.T) {
-		parents, err := sub_child.GetParents(db, nil)
+		parents, err := subChild.GetParents(db, nil)
 		if !assert.NoError(t, err) {
 			return
 		}
 
-		expected_parents := []*models.Album{
+		expectedParents := []*models.Album{
 			{
 				Title: "root",
-				Path:  "/photos",
+				Path:  photosPath,
 			},
 			{
 				Title: "child1",
-				Path:  "/photos/child1",
+				Path:  photosChild1Path,
 			},
 			{
 				Title: "subchild",
-				Path:  "/photos/child1/subchild",
+				Path:  photosChild1SubchildPath,
 			},
 		}
 
-		verifyResult(t, expected_parents, parents)
+		verifyResult(t, expectedParents, parents)
 	})
 
 }

api/graphql/models/face_detection.go

@@ -111,7 +111,7 @@ func (fr *FaceRectangle) Scan(value interface{}) error {
 	slices := strings.Split(stringArray, ":")
 
 	if len(slices) != 4 {
-		return fmt.Errorf("Invalid face rectangle format, expected 4 values, got %d", len(slices))
+		return fmt.Errorf("invalid face rectangle format, expected 4 values, got %d", len(slices))
 	}
 
 	var err error

api/graphql/models/media_test.go

@@ -8,6 +8,8 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
+const mimeJpeg = "image/jpeg"
+
 func TestSanitizeMediaName(t *testing.T) {
 	tests := [][2]string{
 		{"filename.png", "filename_png"},
@@ -53,7 +55,7 @@ func TestMediaURLCachePath(t *testing.T) {
 func TestMediaURLGetURL(t *testing.T) {
 	photo := models.MediaURL{
 		MediaName:   "photo.jpg",
-		ContentType: "image/jpeg",
+		ContentType: mimeJpeg,
 		Purpose:     models.PhotoHighRes,
 	}
 
@@ -76,12 +78,12 @@ func TestMediaGetThumbnail(t *testing.T) {
 		MediaURL: []models.MediaURL{
 			{
 				MediaName:   "photo.jpg",
-				ContentType: "image/jpeg",
+				ContentType: mimeJpeg,
 				Purpose:     models.PhotoHighRes,
 			},
 			{
 				MediaName:   "thumbnail.jpg",
-				ContentType: "image/jpeg",
+				ContentType: mimeJpeg,
 				Purpose:     models.PhotoThumbnail,
 			},
 			{

api/graphql/models/user.go

@@ -54,16 +54,16 @@ func (u *UserPreferences) BeforeSave(tx *gorm.DB) error {
 	}
 
 	if u.Language != nil {
-		lang_str := string(*u.Language)
-		found_match := false
+		langStr := string(*u.Language)
+		foundMatch := false
 		for _, lang := range AllLanguageTranslation {
-			if string(lang) == lang_str {
-				found_match = true
+			if string(lang) == langStr {
+				foundMatch = true
 				break
 			}
 		}
 
-		if !found_match {
+		if !foundMatch {
 			return errors.New("invalid language value")
 		}
 	}
@@ -133,12 +133,12 @@ func (user *User) GenerateAccessToken(db *gorm.DB) (*AccessToken, error) {
 		bytes[i] = CHARACTERS[b%byte(len(CHARACTERS))]
 	}
 
-	token_value := string(bytes)
+	tokenValue := string(bytes)
 	expire := time.Now().Add(14 * 24 * time.Hour)
 
 	token := AccessToken{
 		UserID: user.ID,
-		Value:  token_value,
+		Value:  tokenValue,
 		Expire: expire,
 	}
 

api/graphql/models/user_test.go

@@ -137,7 +137,7 @@ func TestUserOwnsAlbum(t *testing.T) {
 		return
 	}
 
-	sub_albums := []models.Album{
+	subAlbums := []models.Album{
 		{
 			Title:         "subalbum1",
 			Path:          "/photos/album2/subalbum1",
@@ -155,7 +155,7 @@ func TestUserOwnsAlbum(t *testing.T) {
 		},
 	}
 
-	if !assert.NoError(t, db.Model(&user).Association("Albums").Append(&sub_albums)) {
+	if !assert.NoError(t, db.Model(&user).Association("Albums").Append(&subAlbums)) {
 		return
 	}
 
@@ -165,22 +165,22 @@ func TestUserOwnsAlbum(t *testing.T) {
 		assert.True(t, owns)
 	}
 
-	for _, album := range sub_albums {
+	for _, album := range subAlbums {
 		owns, err := user.OwnsAlbum(db, &album)
 		assert.NoError(t, err)
 		assert.True(t, owns)
 	}
 
-	separate_album := models.Album{
+	separateAlbum := models.Album{
 		Title: "separate_album",
 		Path:  "/my_media/album123",
 	}
 
-	if !assert.NoError(t, db.Save(&separate_album).Error) {
+	if !assert.NoError(t, db.Save(&separateAlbum).Error) {
 		return
 	}
 
-	owns, err := user.OwnsAlbum(db, &separate_album)
+	owns, err := user.OwnsAlbum(db, &separateAlbum)
 	assert.NoError(t, err)
 	assert.False(t, owns)
 }

api/graphql/resolvers/faces.go

@@ -11,6 +11,12 @@ import (
 	"gorm.io/gorm"
 )
 
+const faceGroupIDisQuestion = "face_group_id = ?"
+const mediaAlbumIDinQuestion = "media.album_id IN (?)"
+const imageFacesIDinQuestion = "image_faces.id IN (?)"
+
+var ErrFaceDetectorNotInitialized = errors.New("face detector not initialized")
+
 type imageFaceResolver struct {
 	*Resolver
 }
@@ -33,7 +39,7 @@ func (r imageFaceResolver) FaceGroup(ctx context.Context, obj *models.ImageFace)
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return nil, errors.New("face detector not initialized")
+		return nil, ErrFaceDetectorNotInitialized
 	}
 
 	var faceGroup models.FaceGroup
@@ -62,7 +68,7 @@ func (r faceGroupResolver) ImageFaces(ctx context.Context, obj *models.FaceGroup
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return nil, errors.New("face detector not initialized")
+		return nil, ErrFaceDetectorNotInitialized
 	}
 
 	if err := user.FillAlbums(db); err != nil {
@@ -76,7 +82,7 @@ func (r faceGroupResolver) ImageFaces(ctx context.Context, obj *models.FaceGroup
 
 	query := db.
 		Joins("Media").
-		Where("face_group_id = ?", obj.ID).
+		Where(faceGroupIDisQuestion, obj.ID).
 		Where("album_id IN (?)", userAlbumIDs)
 
 	query = models.FormatSQL(query, nil, paginate)
@@ -97,7 +103,7 @@ func (r faceGroupResolver) ImageFaceCount(ctx context.Context, obj *models.FaceG
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return -1, errors.New("face detector not initialized")
+		return -1, ErrFaceDetectorNotInitialized
 	}
 
 	if err := user.FillAlbums(db); err != nil {
@@ -112,7 +118,7 @@ func (r faceGroupResolver) ImageFaceCount(ctx context.Context, obj *models.FaceG
 	query := db.
 		Model(&models.ImageFace{}).
 		Joins("Media").
-		Where("face_group_id = ?", obj.ID).
+		Where(faceGroupIDisQuestion, obj.ID).
 		Where("album_id IN (?)", userAlbumIDs)
 
 	var count int64
@@ -131,7 +137,7 @@ func (r *queryResolver) FaceGroup(ctx context.Context, id int) (*models.FaceGrou
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return nil, errors.New("face detector not initialized")
+		return nil, ErrFaceDetectorNotInitialized
 	}
 
 	if err := user.FillAlbums(db); err != nil {
@@ -147,7 +153,7 @@ func (r *queryResolver) FaceGroup(ctx context.Context, id int) (*models.FaceGrou
 		Joins("LEFT JOIN image_faces ON image_faces.face_group_id = face_groups.id").
 		Joins("LEFT JOIN media ON image_faces.media_id = media.id").
 		Where("face_groups.id = ?", id).
-		Where("media.album_id IN (?)", userAlbumIDs)
+		Where(mediaAlbumIDinQuestion, userAlbumIDs)
 
 	var faceGroup models.FaceGroup
 	if err := faceGroupQuery.Find(&faceGroup).Error; err != nil {
@@ -165,7 +171,7 @@ func (r *queryResolver) MyFaceGroups(ctx context.Context, paginate *models.Pagin
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return nil, errors.New("face detector not initialized")
+		return nil, ErrFaceDetectorNotInitialized
 	}
 
 	if err := user.FillAlbums(db); err != nil {
@@ -179,7 +185,7 @@ func (r *queryResolver) MyFaceGroups(ctx context.Context, paginate *models.Pagin
 
 	faceGroupQuery := db.
 		Joins("JOIN image_faces ON image_faces.face_group_id = face_groups.id").
-		Where("image_faces.media_id IN (?)", db.Select("media.id").Table("media").Where("media.album_id IN (?)", userAlbumIDs)).
+		Where("image_faces.media_id IN (?)", db.Select("media.id").Table("media").Where(mediaAlbumIDinQuestion, userAlbumIDs)).
 		Group("image_faces.face_group_id").
 		Group("face_groups.id").
 		Order("CASE WHEN label IS NULL THEN 1 ELSE 0 END").
@@ -203,7 +209,7 @@ func (r *mutationResolver) SetFaceGroupLabel(ctx context.Context, faceGroupID in
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return nil, errors.New("face detector not initialized")
+		return nil, ErrFaceDetectorNotInitialized
 	}
 
 	faceGroup, err := userOwnedFaceGroup(db, user, faceGroupID)
@@ -226,7 +232,7 @@ func (r *mutationResolver) CombineFaceGroups(ctx context.Context, destinationFac
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return nil, errors.New("face detector not initialized")
+		return nil, ErrFaceDetectorNotInitialized
 	}
 
 	destinationFaceGroup, err := userOwnedFaceGroup(db, user, destinationFaceGroupID)
@@ -240,7 +246,7 @@ func (r *mutationResolver) CombineFaceGroups(ctx context.Context, destinationFac
 	}
 
 	updateError := db.Transaction(func(tx *gorm.DB) error {
-		if err := tx.Model(&models.ImageFace{}).Where("face_group_id = ?", sourceFaceGroup.ID).Update("face_group_id", destinationFaceGroup.ID).Error; err != nil {
+		if err := tx.Model(&models.ImageFace{}).Where(faceGroupIDisQuestion, sourceFaceGroup.ID).Update("face_group_id", destinationFaceGroup.ID).Error; err != nil {
 			return err
 		}
 
@@ -268,7 +274,7 @@ func (r *mutationResolver) MoveImageFaces(ctx context.Context, imageFaceIDs []in
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return nil, errors.New("face detector not initialized")
+		return nil, ErrFaceDetectorNotInitialized
 	}
 
 	userOwnedImageFaceIDs := make([]int, 0)
@@ -294,7 +300,7 @@ func (r *mutationResolver) MoveImageFaces(ctx context.Context, imageFaceIDs []in
 		var sourceFaceGroups []*models.FaceGroup
 		if err := tx.
 			Joins("LEFT JOIN image_faces ON image_faces.face_group_id = face_groups.id").
-			Where("image_faces.id IN (?)", userOwnedImageFaceIDs).
+			Where(imageFacesIDinQuestion, userOwnedImageFaceIDs).
 			Find(&sourceFaceGroups).Error; err != nil {
 			return err
 		}
@@ -309,7 +315,7 @@ func (r *mutationResolver) MoveImageFaces(ctx context.Context, imageFaceIDs []in
 		// delete face groups if they have become empty
 		for _, faceGroup := range sourceFaceGroups {
 			var count int64
-			if err := tx.Model(&models.ImageFace{}).Where("face_group_id = ?", faceGroup.ID).Count(&count).Error; err != nil {
+			if err := tx.Model(&models.ImageFace{}).Where(faceGroupIDisQuestion, faceGroup.ID).Count(&count).Error; err != nil {
 				return err
 			}
 
@@ -340,7 +346,7 @@ func (r *mutationResolver) RecognizeUnlabeledFaces(ctx context.Context) ([]*mode
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return nil, errors.New("face detector not initialized")
+		return nil, ErrFaceDetectorNotInitialized
 	}
 
 	var updatedImageFaces []*models.ImageFace
@@ -367,7 +373,7 @@ func (r *mutationResolver) DetachImageFaces(ctx context.Context, imageFaceIDs []
 	}
 
 	if face_detection.GlobalFaceDetector == nil {
-		return nil, errors.New("face detector not initialized")
+		return nil, ErrFaceDetectorNotInitialized
 	}
 
 	userOwnedImageFaceIDs := make([]int, 0)
@@ -431,13 +437,13 @@ func userOwnedFaceGroup(db *gorm.DB, user *models.User, faceGroupID int) (*model
 		Select("image_faces.id").
 		Table("image_faces").
 		Joins("JOIN media ON media.id = image_faces.media_id").
-		Where("media.album_id IN (?)", userAlbumIDs)
+		Where(mediaAlbumIDinQuestion, userAlbumIDs)
 
 	faceGroupQuery := db.
 		Model(&models.FaceGroup{}).
 		Joins("JOIN image_faces ON face_groups.id = image_faces.face_group_id").
 		Where("face_groups.id = ?", faceGroupID).
-		Where("image_faces.id IN (?)", imageFaceQuery)
+		Where(imageFacesIDinQuestion, imageFaceQuery)
 
 	var faceGroup models.FaceGroup
 	if err := faceGroupQuery.Find(&faceGroup).Error; err != nil {
@@ -463,8 +469,8 @@ func getUserOwnedImageFaces(tx *gorm.DB, user *models.User, imageFaceIDs []int)
 	var userOwnedImageFaces []*models.ImageFace
 	if err := tx.
 		Joins("JOIN media ON media.id = image_faces.media_id").
-		Where("media.album_id IN (?)", userAlbumIDs).
-		Where("image_faces.id IN (?)", imageFaceIDs).
+		Where(mediaAlbumIDinQuestion, userAlbumIDs).
+		Where(imageFacesIDinQuestion, imageFaceIDs).
 		Find(&userOwnedImageFaces).Error; err != nil {
 		return nil, err
 	}