From f8b9590aa97da1413461f8a3239b2c54d715b4b1 Mon Sep 17 00:00:00 2001 From: Kostiantyn <32730812+kkovaletp@users.noreply.github.com> Date: Fri, 4 Oct 2024 16:49:41 +0300 Subject: [PATCH] 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 | 59 +++---- api/database/migration_exif.go | 184 +++++++++++--------- api/database/migrations/exif_invalid_gps.go | 5 +- 3 files changed, 133 insertions(+), 115 deletions(-) diff --git a/api/database/database.go b/api/database/database.go index fb452f8c..2b71bda7 100644 --- a/api/database/database.go +++ b/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,26 +213,8 @@ func ClearDatabase(db *gorm.DB) error { } } - dryRun := tx.Session(&gorm.Session{DryRun: true}) - for _, model := range database_models { - // get table name of model structure - table := dryRun.Find(model).Statement.Table - - switch dbDriver { - case drivers.POSTGRES: - if err := tx.Exec(fmt.Sprintf("TRUNCATE TABLE %s CASCADE", table)).Error; err != nil { - return err - } - case drivers.MYSQL: - if err := tx.Exec(fmt.Sprintf("TRUNCATE TABLE %s", table)).Error; err != nil { - return err - } - case drivers.SQLITE: - if err := tx.Exec(fmt.Sprintf("DELETE FROM %s", table)).Error; err != nil { - return err - } - } - + if err := clearTables(tx, dbDriver); err != nil { + return err } if dbDriver == drivers.MYSQL { @@ -244,10 +225,28 @@ func ClearDatabase(db *gorm.DB) error { return nil }) +} - if err != nil { - return err +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 + table := dryRun.Find(model).Statement.Table + + switch dbDriver { + case drivers.POSTGRES: + if err := tx.Exec(fmt.Sprintf("TRUNCATE TABLE %s CASCADE", table)).Error; err != nil { + return err + } + case drivers.MYSQL: + if err := tx.Exec(fmt.Sprintf("TRUNCATE TABLE %s", table)).Error; err != nil { + return err + } + case drivers.SQLITE: + if err := tx.Exec(fmt.Sprintf("DELETE FROM %s", table)).Error; err != nil { + return err + } + } } - return nil } diff --git a/api/database/migration_exif.go b/api/database/migration_exif.go index d6f8a40a..73effbd1 100644 --- a/api/database/migration_exif.go +++ b/api/database/migration_exif.go @@ -7,10 +7,45 @@ import ( "strings" "github.com/photoview/photoview/api/graphql/models" - "github.com/pkg/errors" "gorm.io/gorm" ) +type exifModel struct { + ID int `gorm:"primarykey"` + Exposure *string + Flash *string +} + +var flashDescriptions = map[int]string{ + 0x0: "No Flash", + 0x1: "Fired", + 0x5: "Fired, Return not detected", + 0x7: "Fired, Return detected", + 0x8: "On, Did not fire", + 0x9: "On, Fired", + 0xD: "On, Return not detected", + 0xF: "On, Return detected", + 0x10: "Off, Did not fire", + 0x14: "Off, Did not fire, Return not detected", + 0x18: "Auto, Did not fire", + 0x19: "Auto, Fired", + 0x1D: "Auto, Fired, Return not detected", + 0x1F: "Auto, Fired, Return detected", + 0x20: "No flash function", + 0x30: "Off, No flash function", + 0x41: "Fired, Red-eye reduction", + 0x45: "Fired, Red-eye reduction, Return not detected", + 0x47: "Fired, Red-eye reduction, Return detected", + 0x49: "On, Red-eye reduction", + 0x4D: "On, Red-eye reduction, Return not detected", + 0x4F: "On, Red-eye reduction, Return detected", + 0x50: "Off, Red-eye reduction", + 0x58: "Auto, Did not fire, Red-eye reduction", + 0x59: "Auto, Fired, Red-eye reduction", + 0x5D: "Auto, Fired, Red-eye reduction, Return not detected", + 0x5F: "Auto, Fired, Red-eye reduction, Return detected", +} + // Migrate MediaExif fields "exposure" and "flash" from strings to integers func migrateExifFields(db *gorm.DB) error { mediaExifColumns, err := db.Migrator().ColumnTypes(&models.MediaEXIF{}) @@ -18,44 +53,52 @@ func migrateExifFields(db *gorm.DB) error { return err } - err = db.Transaction(func(tx *gorm.DB) error { + return 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 err := parseExposure(exifCol, 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 := parseFlash(exifCol, 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 fmt.Errorf("failed to auto migrate media_exif after exposure conversion: %w", err) } return nil }) +} - if err != nil { - return err +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 } @@ -65,16 +108,24 @@ func migrateExifFieldsExposure(db *gorm.DB) error { 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") + return fmt.Errorf("convert flash attribute empty values to NULL: %w", err) } - 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 { + 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 { @@ -83,7 +134,7 @@ func migrateExifFieldsExposure(db *gorm.DB) error { frac := strings.Split(*result.Exposure, "/") if len(frac) != 2 { - return errors.Errorf("failed to convert exposure value (%s) expected format x/y", frac) + return fmt.Errorf("failed to convert exposure value (%s) expected format x/y", frac) } numerator, err := strconv.ParseFloat(frac[0], 64) @@ -104,13 +155,6 @@ func migrateExifFieldsExposure(db *gorm.DB) error { return nil }).Error - }) - - if err != nil { - return errors.Wrap(err, "migrating `media_exif.exposure` failed") - } - - return nil } func migrateExifFieldsFlash(db *gorm.DB) error { @@ -119,8 +163,11 @@ func migrateExifFieldsFlash(db *gorm.DB) error { 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 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" { @@ -128,46 +175,24 @@ func migrateExifFieldsFlash(db *gorm.DB) error { } 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") + return fmt.Errorf("convert flash attribute empty values to NULL: %w", err) } - type exifModel struct { - ID int `gorm:"primarykey"` - Flash *string - } var results []exifModel - var flashDescriptions = map[int]string{ - 0x0: "No Flash", - 0x1: "Fired", - 0x5: "Fired, Return not detected", - 0x7: "Fired, Return detected", - 0x8: "On, Did not fire", - 0x9: "On, Fired", - 0xD: "On, Return not detected", - 0xF: "On, Return detected", - 0x10: "Off, Did not fire", - 0x14: "Off, Did not fire, Return not detected", - 0x18: "Auto, Did not fire", - 0x19: "Auto, Fired", - 0x1D: "Auto, Fired, Return not detected", - 0x1F: "Auto, Fired, Return detected", - 0x20: "No flash function", - 0x30: "Off, No flash function", - 0x41: "Fired, Red-eye reduction", - 0x45: "Fired, Red-eye reduction, Return not detected", - 0x47: "Fired, Red-eye reduction, Return detected", - 0x49: "On, Red-eye reduction", - 0x4D: "On, Red-eye reduction, Return not detected", - 0x4F: "On, Red-eye reduction, Return detected", - 0x50: "Off, Red-eye reduction", - 0x58: "Auto, Did not fire, Red-eye reduction", - 0x59: "Auto, Fired, Red-eye reduction", - 0x5D: "Auto, Fired, Red-eye reduction, Return not detected", - 0x5F: "Auto, Fired, Red-eye reduction, Return detected", - } + return replaceFlashValues(tx, results) + }) - return tx.Model(&exifModel{}).Table("media_exif").Where("flash IS NOT NULL").FindInBatches(&results, 100, func(tx *gorm.DB, batch int) error { + 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 } diff --git a/api/database/migrations/exif_invalid_gps.go b/api/database/migrations/exif_invalid_gps.go index c7c33c88..9d0a6377 100644 --- a/api/database/migrations/exif_invalid_gps.go +++ b/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 })