Refactoring part 2.1: Long func refactoring in database (#1073)

* Split long functions and optimize code

* Replace `errors.Wrap()` with the `fmt.Errorf()`; rename `process*()` functions to `parse*()`

---------

Co-authored-by: Konstantin Koval

api/database/database.go

@@ -11,7 +11,6 @@ import (
 	"github.com/photoview/photoview/api/database/migrations"
 	"github.com/photoview/photoview/api/graphql/models"
 	"github.com/photoview/photoview/api/utils"
-	"github.com/pkg/errors"
 
 	"github.com/go-sql-driver/mysql"
 	gorm_mysql "gorm.io/driver/mysql"
@@ -23,12 +22,12 @@ import (
 
 func GetMysqlAddress(addressString string) (string, error) {
 	if addressString == "" {
-		return "", errors.New(fmt.Sprintf("Environment variable %s missing, exiting", utils.EnvMysqlURL.GetName()))
+		return "", fmt.Errorf("Environment variable %s missing, exiting", utils.EnvMysqlURL.GetName())
 	}
 
 	config, err := mysql.ParseDSN(addressString)
 	if err != nil {
-		return "", errors.Wrap(err, "Could not parse mysql url")
+		return "", fmt.Errorf("could not parse mysql url: %w", err)
 	}
 
 	config.MultiStatements = true
@@ -39,12 +38,12 @@ func GetMysqlAddress(addressString string) (string, error) {
 
 func GetPostgresAddress(addressString string) (*url.URL, error) {
 	if addressString == "" {
-		return nil, errors.New(fmt.Sprintf("Environment variable %s missing, exiting", utils.EnvPostgresURL.GetName()))
+		return nil, fmt.Errorf("Environment variable %s missing, exiting", utils.EnvPostgresURL.GetName())
 	}
 
 	address, err := url.Parse(addressString)
 	if err != nil {
-		return nil, errors.Wrap(err, "Could not parse postgres url")
+		return nil, fmt.Errorf("could not parse postgres url: %w", err)
 	}
 
 	return address, nil
@@ -57,7 +56,7 @@ func GetSqliteAddress(path string) (*url.URL, error) {
 
 	address, err := url.Parse(path)
 	if err != nil {
-		return nil, errors.Wrapf(err, "Could not parse sqlite url (%s)", path)
+		return nil, fmt.Errorf("could not parse sqlite url (%s): %w", path, err)
 	}
 
 	queryValues := address.Query()
@@ -204,7 +203,7 @@ func MigrateDatabase(db *gorm.DB) error {
 }
 
 func ClearDatabase(db *gorm.DB) error {
-	err := db.Transaction(func(tx *gorm.DB) error {
+	return db.Transaction(func(tx *gorm.DB) error {
 
 		dbDriver := drivers.DatabaseDriverFromEnv()
 
@@ -214,6 +213,21 @@ func ClearDatabase(db *gorm.DB) error {
 			}
 		}
 
+		if err := clearTables(tx, dbDriver); err != nil {
+			return err
+		}
+
+		if dbDriver == drivers.MYSQL {
+			if err := tx.Exec("SET FOREIGN_KEY_CHECKS = 1;").Error; err != nil {
+				return err
+			}
+		}
+
+		return nil
+	})
+}
+
+func clearTables(tx *gorm.DB, dbDriver drivers.DatabaseDriverType) error {
 	dryRun := tx.Session(&gorm.Session{DryRun: true})
 	for _, model := range database_models {
 		// get table name of model structure
@@ -233,21 +247,6 @@ func ClearDatabase(db *gorm.DB) error {
 				return err
 			}
 		}
-
 	}
-
-		if dbDriver == drivers.MYSQL {
-			if err := tx.Exec("SET FOREIGN_KEY_CHECKS = 1;").Error; err != nil {
-				return err
-			}
-		}
-
-		return nil
-	})
-
-	if err != nil {
-		return err
-	}
-
 	return nil
 }

api/database/migration_exif.go

@@ -7,135 +7,14 @@ import (
 	"strings"
 
 	"github.com/photoview/photoview/api/graphql/models"
-	"github.com/pkg/errors"
 	"gorm.io/gorm"
 )
 
-// Migrate MediaExif fields "exposure" and "flash" from strings to integers
-func migrateExifFields(db *gorm.DB) error {
-	mediaExifColumns, err := db.Migrator().ColumnTypes(&models.MediaEXIF{})
-	if err != nil {
-		return err
-	}
-
-	err = db.Transaction(func(tx *gorm.DB) error {
-		for _, exifCol := range mediaExifColumns {
-			if exifCol.Name() == "exposure" {
-				switch exifCol.DatabaseTypeName() {
-				case "double", "numeric", "real", "bigint", "integer":
-					// correct type, do nothing
-				default:
-					// do migration
-					if err := migrateExifFieldsExposure(db); err != nil {
-						return err
-					}
-				}
-			}
-
-			if exifCol.Name() == "flash" {
-				switch exifCol.DatabaseTypeName() {
-				case "double", "numeric", "real", "bigint", "integer":
-					// correct type, do nothing
-				default:
-					// do migration
-					if err := migrateExifFieldsFlash(db); err != nil {
-						return err
-					}
-				}
-			}
-		}
-
-		if err := db.AutoMigrate(&models.MediaEXIF{}); err != nil {
-			return errors.Wrap(err, "failed to auto migrate media_exif after exposure conversion")
-		}
-
-		return nil
-	})
-
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func migrateExifFieldsExposure(db *gorm.DB) error {
-	log.Println("Migrating `media_exif.exposure` from string to double")
-
-	err := db.Transaction(func(tx *gorm.DB) error {
-
-		if err := tx.Exec("UPDATE media_exif SET exposure = NULL WHERE exposure = ''").Error; err != nil {
-			return errors.Wrapf(err, "convert flash attribute empty values to NULL")
-		}
-
 type exifModel struct {
 	ID       int `gorm:"primarykey"`
 	Exposure *string
-		}
-		var results []exifModel
-
-		return tx.Model(&exifModel{}).Table("media_exif").Where("exposure LIKE '%/%'").FindInBatches(&results, 100, func(tx *gorm.DB, batch int) error {
-			for _, result := range results {
-
-				if result.Exposure == nil {
-					continue
-				}
-
-				frac := strings.Split(*result.Exposure, "/")
-				if len(frac) != 2 {
-					return errors.Errorf("failed to convert exposure value (%s) expected format x/y", frac)
-				}
-
-				numerator, err := strconv.ParseFloat(frac[0], 64)
-				if err != nil {
-					return err
-				}
-
-				denominator, err := strconv.ParseFloat(frac[1], 64)
-				if err != nil {
-					return err
-				}
-
-				decimalValue := numerator / denominator
-				*result.Exposure = fmt.Sprintf("%f", decimalValue)
-			}
-
-			tx.Save(&results)
-
-			return nil
-		}).Error
-	})
-
-	if err != nil {
-		return errors.Wrap(err, "migrating `media_exif.exposure` failed")
-	}
-
-	return nil
-}
-
-func migrateExifFieldsFlash(db *gorm.DB) error {
-	log.Println("Migrating `media_exif.flash` from string to int")
-
-	err := db.Transaction(func(tx *gorm.DB) error {
-
-		var dataType string
-		if err := tx.Raw("SELECT data_type FROM information_schema.columns WHERE table_name = 'media_exif' AND column_name = 'flash';").Find(&dataType).Error; err != nil {
-			return errors.Wrapf(err, "read data_type of column media_exif.flash")
-		}
-
-		if dataType == "bigint" {
-			return nil
-		}
-
-		if err := tx.Exec("UPDATE media_exif SET flash = NULL WHERE flash = ''").Error; err != nil {
-			return errors.Wrapf(err, "convert flash attribute empty values to NULL")
-		}
-
-		type exifModel struct {
-			ID    int `gorm:"primarykey"`
 	Flash    *string
 }
-		var results []exifModel
 
 var flashDescriptions = map[int]string{
 	0x0:  "No Flash",
@@ -167,7 +46,153 @@ func migrateExifFieldsFlash(db *gorm.DB) error {
 	0x5F: "Auto, Fired, Red-eye reduction, Return detected",
 }
 
-		return tx.Model(&exifModel{}).Table("media_exif").Where("flash IS NOT NULL").FindInBatches(&results, 100, func(tx *gorm.DB, batch int) error {
+// Migrate MediaExif fields "exposure" and "flash" from strings to integers
+func migrateExifFields(db *gorm.DB) error {
+	mediaExifColumns, err := db.Migrator().ColumnTypes(&models.MediaEXIF{})
+	if err != nil {
+		return err
+	}
+
+	return db.Transaction(func(tx *gorm.DB) error {
+		for _, exifCol := range mediaExifColumns {
+			if err := parseExposure(exifCol, db); err != nil {
+				return err
+			}
+
+			if err := parseFlash(exifCol, db); err != nil {
+				return err
+			}
+		}
+
+		if err := db.AutoMigrate(&models.MediaEXIF{}); err != nil {
+			return fmt.Errorf("failed to auto migrate media_exif after exposure conversion: %w", err)
+		}
+
+		return nil
+	})
+}
+
+func parseFlash(exifCol gorm.ColumnType, db *gorm.DB) error {
+	if exifCol.Name() == "flash" {
+		switch exifCol.DatabaseTypeName() {
+		case "double", "numeric", "real", "bigint", "integer":
+			// correct type, do nothing
+		default:
+			// do migration
+			if err := migrateExifFieldsFlash(db); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func parseExposure(exifCol gorm.ColumnType, db *gorm.DB) error {
+	if exifCol.Name() == "exposure" {
+		switch exifCol.DatabaseTypeName() {
+		case "double", "numeric", "real", "bigint", "integer":
+			// correct type, do nothing
+		default:
+			// do migration
+			if err := migrateExifFieldsExposure(db); err != nil {
+				return err
+			}
+		}
+	}
+	return nil
+}
+
+func migrateExifFieldsExposure(db *gorm.DB) error {
+	log.Println("Migrating `media_exif.exposure` from string to double")
+
+	err := db.Transaction(func(tx *gorm.DB) error {
+
+		if err := tx.Exec("UPDATE media_exif SET exposure = NULL WHERE exposure = ''").Error; err != nil {
+			return fmt.Errorf("convert flash attribute empty values to NULL: %w", err)
+		}
+
+		var results []exifModel
+
+		return calculateExposure(tx, results)
+	})
+
+	if err != nil {
+		return fmt.Errorf("migrating `media_exif.exposure` failed: %w", err)
+	}
+
+	return nil
+}
+
+func calculateExposure(tx *gorm.DB, results []exifModel) error {
+	return tx.Model(&exifModel{}).Table("media_exif").Where("exposure LIKE '%/%'").FindInBatches(
+		&results, 100, func(tx *gorm.DB, batch int) error {
+			for _, result := range results {
+
+				if result.Exposure == nil {
+					continue
+				}
+
+				frac := strings.Split(*result.Exposure, "/")
+				if len(frac) != 2 {
+					return fmt.Errorf("failed to convert exposure value (%s) expected format x/y", frac)
+				}
+
+				numerator, err := strconv.ParseFloat(frac[0], 64)
+				if err != nil {
+					return err
+				}
+
+				denominator, err := strconv.ParseFloat(frac[1], 64)
+				if err != nil {
+					return err
+				}
+
+				decimalValue := numerator / denominator
+				*result.Exposure = fmt.Sprintf("%f", decimalValue)
+			}
+
+			tx.Save(&results)
+
+			return nil
+		}).Error
+}
+
+func migrateExifFieldsFlash(db *gorm.DB) error {
+	log.Println("Migrating `media_exif.flash` from string to int")
+
+	err := db.Transaction(func(tx *gorm.DB) error {
+
+		var dataType string
+		if err := tx.Raw(
+			"SELECT data_type FROM information_schema.columns WHERE table_name = 'media_exif' AND column_name = 'flash';").
+			Find(&dataType).Error; err != nil {
+
+			return fmt.Errorf("read data_type of column media_exif.flash: %w", err)
+		}
+
+		if dataType == "bigint" {
+			return nil
+		}
+
+		if err := tx.Exec("UPDATE media_exif SET flash = NULL WHERE flash = ''").Error; err != nil {
+			return fmt.Errorf("convert flash attribute empty values to NULL: %w", err)
+		}
+
+		var results []exifModel
+
+		return replaceFlashValues(tx, results)
+	})
+
+	if err != nil {
+		return fmt.Errorf("migrating `media_exif.flash` failed: %w", err)
+	}
+
+	return nil
+}
+
+func replaceFlashValues(tx *gorm.DB, results []exifModel) error {
+	return tx.Model(&exifModel{}).Table("media_exif").Where("flash IS NOT NULL").FindInBatches(
+		&results, 100, func(tx *gorm.DB, batch int) error {
 			for _, result := range results {
 
 				if result.Flash == nil {
@@ -186,11 +211,4 @@ func migrateExifFieldsFlash(db *gorm.DB) error {
 
 			return nil
 		}).Error
-	})
-
-	if err != nil {
-		return errors.Wrap(err, "migrating `media_exif.flash` failed")
-	}
-
-	return nil
 }

api/database/migrations/exif_invalid_gps.go

@@ -1,8 +1,9 @@
 package migrations
 
 import (
+	"fmt"
+
 	"github.com/photoview/photoview/api/graphql/models"
-	"github.com/pkg/errors"
 	"gorm.io/gorm"
 )
 
@@ -16,7 +17,7 @@ func MigrateForExifGPSCorrection(db *gorm.DB) error {
 				"gps_latitude":  nil,
 				"gps_longitude": nil,
 			}).Error; err != nil {
-			return errors.Wrap(err, "failed to remove invalid GPS data from media_exif table")
+			return fmt.Errorf("failed to remove invalid GPS data from media_exif table: %w", err)
 		}
 		return nil
 	})