From 686a8ab9b49ef22fb14a2b4b9c7981c36c84b481 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Tue, 29 Mar 2022 00:21:50 +0200 Subject: [PATCH] Search: Refactor photo search, fix test data and unit tests #1994 --- docker-compose.ci.yml | 1 - docker-compose.latest.yml | 10 +- docker-compose.mariadb.yml | 17 +++ docker-compose.postgres.yml | 1 - docker-compose.yml | 1 - internal/auto/auto_test.go | 4 - internal/config/flags.go | 4 +- internal/config/options.go | 12 +- internal/config/test.go | 36 +++-- internal/entity/album_fixtures.go | 50 +++---- internal/entity/const.go | 2 + internal/entity/db.go | 8 +- internal/entity/db_migrate.go | 20 ++- internal/entity/entity_test.go | 10 +- internal/query/query_test.go | 10 +- internal/search/photos.go | 132 +++++++++++------- internal/search/photos_filter_album_test.go | 7 +- internal/search/photos_filter_albums_test.go | 39 +++--- .../search/photos_filter_favorite_test.go | 5 +- internal/search/photos_results.go | 36 ++--- internal/search/photos_results_test.go | 2 +- internal/search/search_test.go | 7 +- 22 files changed, 242 insertions(+), 172 deletions(-) diff --git a/docker-compose.ci.yml b/docker-compose.ci.yml index 1965fb690..f31736fb4 100644 --- a/docker-compose.ci.yml +++ b/docker-compose.ci.yml @@ -37,7 +37,6 @@ services: PHOTOPRISM_DATABASE_USER: "root" PHOTOPRISM_DATABASE_PASSWORD: "photoprism" PHOTOPRISM_TEST_DRIVER: "sqlite" - PHOTOPRISM_TEST_DSN: ".test.db" PHOTOPRISM_ADMIN_PASSWORD: "photoprism" # the initial admin password (min 4 characters) PHOTOPRISM_ASSETS_PATH: "/go/src/github.com/photoprism/photoprism/assets" PHOTOPRISM_STORAGE_PATH: "/go/src/github.com/photoprism/photoprism/storage" diff --git a/docker-compose.latest.yml b/docker-compose.latest.yml index 7d2301e46..7d4bf1881 100644 --- a/docker-compose.latest.yml +++ b/docker-compose.latest.yml @@ -39,9 +39,9 @@ services: PHOTOPRISM_HTTP_COMPRESSION: "gzip" # improves transfer speed and bandwidth utilization (none or gzip) PHOTOPRISM_DATABASE_DRIVER: "mysql" PHOTOPRISM_DATABASE_SERVER: "mariadb:4001" - PHOTOPRISM_DATABASE_NAME: "latest" - PHOTOPRISM_DATABASE_USER: "root" - PHOTOPRISM_DATABASE_PASSWORD: "photoprism" + PHOTOPRISM_DATABASE_NAME: "photoprism_latest" + PHOTOPRISM_DATABASE_USER: "photoprism_latest" + PHOTOPRISM_DATABASE_PASSWORD: "photoprism_latest" PHOTOPRISM_DISABLE_CHOWN: "false" # disables storage permission updates on startup PHOTOPRISM_DISABLE_BACKUPS: "false" # don't backup photo and album metadata to YAML files PHOTOPRISM_DISABLE_WEBDAV: "false" # disables built-in WebDAV server @@ -62,8 +62,8 @@ services: TF_CPP_MIN_LOG_LEVEL: 0 # show TensorFlow log messages for development working_dir: "/photoprism" volumes: - - "./storage/latest:/photoprism/storage" - - "./storage/originals:/photoprism/originals" + - "/photoprism/storage" + - "/photoprism/originals" ## Join shared "photoprism-develop" network networks: diff --git a/docker-compose.mariadb.yml b/docker-compose.mariadb.yml index 7efd93058..87b5c697a 100644 --- a/docker-compose.mariadb.yml +++ b/docker-compose.mariadb.yml @@ -2,6 +2,23 @@ version: '3.5' ## MariaDB Server Versions for Development & Testing services: + ## MariaDB 10.8 Database Server + ## Docs: https://mariadb.com/kb/en/release-notes-mariadb-108-series/ + mariadb-10-8: + image: mariadb:10.8-rc + command: mysqld --port=4001 --transaction-isolation=READ-COMMITTED --character-set-server=utf8mb4 --collation-server=utf8mb4_unicode_ci --max-connections=512 --innodb-rollback-on-timeout=OFF --innodb-lock-wait-timeout=120 + expose: + - "4001" + ports: + - "4002:4001" # database port (host:container) + volumes: + - "./scripts/sql/mariadb-init.sql:/docker-entrypoint-initdb.d/init.sql" + environment: + MYSQL_ROOT_PASSWORD: photoprism + MYSQL_USER: photoprism + MYSQL_PASSWORD: photoprism + MYSQL_DATABASE: photoprism + ## MariaDB 10.7 Database Server mariadb-10-7: image: mariadb:10.7 diff --git a/docker-compose.postgres.yml b/docker-compose.postgres.yml index 079f225fa..490bb7552 100644 --- a/docker-compose.postgres.yml +++ b/docker-compose.postgres.yml @@ -48,7 +48,6 @@ services: PHOTOPRISM_DATABASE_USER: "photoprism" PHOTOPRISM_DATABASE_PASSWORD: "photoprism" PHOTOPRISM_TEST_DRIVER: "sqlite" - PHOTOPRISM_TEST_DSN: ".test.db" PHOTOPRISM_ADMIN_PASSWORD: "photoprism" # the initial admin password (min 4 characters) PHOTOPRISM_ASSETS_PATH: "/go/src/github.com/photoprism/photoprism/assets" PHOTOPRISM_STORAGE_PATH: "/go/src/github.com/photoprism/photoprism/storage" diff --git a/docker-compose.yml b/docker-compose.yml index 74a3db908..4cfabd0fc 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -79,7 +79,6 @@ services: PHOTOPRISM_DATABASE_USER: "root" PHOTOPRISM_DATABASE_PASSWORD: "photoprism" PHOTOPRISM_TEST_DRIVER: "sqlite" - PHOTOPRISM_TEST_DSN: ".test.db" # PHOTOPRISM_TEST_DSN_MYSQL8: "root:photoprism@tcp(mysql:4001)/photoprism?charset=utf8mb4,utf8&collation=utf8mb4_unicode_ci&parseTime=true" PHOTOPRISM_ASSETS_PATH: "/go/src/github.com/photoprism/photoprism/assets" PHOTOPRISM_STORAGE_PATH: "/go/src/github.com/photoprism/photoprism/storage" diff --git a/internal/auto/auto_test.go b/internal/auto/auto_test.go index 73a75298b..16c6ae8aa 100644 --- a/internal/auto/auto_test.go +++ b/internal/auto/auto_test.go @@ -12,10 +12,6 @@ func TestMain(m *testing.M) { log = logrus.StandardLogger() log.SetLevel(logrus.TraceLevel) - if err := os.Remove(".test.db"); err == nil { - log.Debugln("removed .test.db") - } - c := config.TestConfig() code := m.Run() diff --git a/internal/config/flags.go b/internal/config/flags.go index 8cb00403a..76838b761 100644 --- a/internal/config/flags.go +++ b/internal/config/flags.go @@ -342,12 +342,12 @@ var GlobalFlags = []cli.Flag{ EnvVar: "PHOTOPRISM_DATABASE_DRIVER", }, cli.StringFlag{ - Name: "database-dsn", + Name: "database-dsn, dsn", Usage: "database connection `DSN` (sqlite filename, optional for mysql)", EnvVar: "PHOTOPRISM_DATABASE_DSN", }, cli.StringFlag{ - Name: "database-server", + Name: "database-server, db", Usage: "database `HOST` incl. port e.g. \"mariadb:3306\" (or socket path)", EnvVar: "PHOTOPRISM_DATABASE_SERVER", }, diff --git a/internal/config/options.go b/internal/config/options.go index 528fac577..8aa368853 100644 --- a/internal/config/options.go +++ b/internal/config/options.go @@ -15,12 +15,14 @@ import ( "github.com/photoprism/photoprism/pkg/fs" ) -// Database drivers (sql dialects). +// SQL Databases. const ( - MySQL = "mysql" - MariaDB = "mariadb" - SQLite3 = "sqlite3" - Postgres = "postgres" // TODO: Requires GORM 2.0 for generic column data types + MySQL = "mysql" + MariaDB = "mariadb" + Postgres = "postgres" // TODO: Requires GORM 2.0 for generic column data types + SQLite3 = "sqlite3" + SQLiteTestDB = ".test.db" + SQLiteMemoryDSN = ":memory:" ) // Options provides a struct in which application configuration is stored. diff --git a/internal/config/test.go b/internal/config/test.go index 40cd98496..e808d1324 100644 --- a/internal/config/test.go +++ b/internal/config/test.go @@ -9,13 +9,16 @@ import ( "testing" "time" + "github.com/urfave/cli" + _ "github.com/jinzhu/gorm/dialects/mysql" _ "github.com/jinzhu/gorm/dialects/sqlite" + "github.com/photoprism/photoprism/internal/thumb" "github.com/photoprism/photoprism/pkg/capture" "github.com/photoprism/photoprism/pkg/fs" "github.com/photoprism/photoprism/pkg/rnd" - "github.com/urfave/cli" + "github.com/photoprism/photoprism/pkg/sanitize" ) // Download URL and ZIP hash for test files. @@ -40,18 +43,31 @@ func NewTestOptions() *Options { storagePath := fs.Abs("../../storage") testDataPath := filepath.Join(storagePath, "testdata") - dbDriver := os.Getenv("PHOTOPRISM_TEST_DRIVER") - dbDsn := os.Getenv("PHOTOPRISM_TEST_DSN") + driver := os.Getenv("PHOTOPRISM_TEST_DRIVER") + dsn := os.Getenv("PHOTOPRISM_TEST_DSN") // Config example for MySQL / MariaDB: - // dbDriver = MySQL, - // dbDsn = "photoprism:photoprism@tcp(mariadb:4001)/photoprism?parseTime=true", + // driver = MySQL, + // dsn = "photoprism:photoprism@tcp(mariadb:4001)/photoprism?parseTime=true", - if dbDriver == "test" || dbDriver == "sqlite" || dbDriver == "" || dbDsn == "" { - dbDriver = SQLite3 - dbDsn = ".test.db" + // Set default test database driver. + if driver == "test" || driver == "sqlite" || driver == "" || dsn == "" { + driver = SQLite3 } + // Set default database DSN. + if driver == SQLite3 { + if dsn == "" { + dsn = SQLiteMemoryDSN + } else if dsn != SQLiteTestDB { + // Continue. + + } else if err := os.Remove(dsn); err == nil { + log.Debugf("sqlite: test file %s removed", sanitize.Log(dsn)) + } + } + + // Test config options. c := &Options{ Name: "PhotoPrism", Version: "0.0.0", @@ -74,8 +90,8 @@ func NewTestOptions() *Options { TempPath: testDataPath + "/temp", ConfigPath: testDataPath + "/config", SidecarPath: testDataPath + "/sidecar", - DatabaseDriver: dbDriver, - DatabaseDsn: dbDsn, + DatabaseDriver: driver, + DatabaseDsn: dsn, AdminPassword: "photoprism", } diff --git a/internal/entity/album_fixtures.go b/internal/entity/album_fixtures.go index d74e2ffb9..016fd2031 100644 --- a/internal/entity/album_fixtures.go +++ b/internal/entity/album_fixtures.go @@ -618,31 +618,6 @@ var AlbumFixtures = AlbumMap{ UpdatedAt: time.Date(2020, 2, 1, 0, 0, 0, 0, time.UTC), DeletedAt: nil, }, - "red|green": { - ID: 1000023, - AlbumUID: "at1lxuqipotaab32", - AlbumSlug: "red|green", - AlbumPath: "", - AlbumType: AlbumDefault, - AlbumTitle: "Red|Green", - AlbumLocation: "", - AlbumCategory: "", - AlbumCaption: "", - AlbumDescription: "", - AlbumNotes: "", - AlbumFilter: "", - AlbumOrder: "name", - AlbumTemplate: "", - AlbumCountry: "zz", - AlbumYear: 0, - AlbumMonth: 0, - AlbumDay: 0, - AlbumFavorite: false, - AlbumPrivate: false, - CreatedAt: time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), - UpdatedAt: time.Date(2020, 2, 1, 0, 0, 0, 0, time.UTC), - DeletedAt: nil, - }, "blue|": { ID: 1000024, AlbumUID: "at1lxuqipotaab33", @@ -743,6 +718,31 @@ var AlbumFixtures = AlbumMap{ UpdatedAt: time.Date(2020, 2, 1, 0, 0, 0, 0, time.UTC), DeletedAt: nil, }, + "red|green": { + ID: 1000028, + AlbumUID: "at1lxuqipotaab32", + AlbumSlug: "red|green", + AlbumPath: "", + AlbumType: AlbumDefault, + AlbumTitle: "Red|Green", + AlbumLocation: "", + AlbumCategory: "", + AlbumCaption: "", + AlbumDescription: "", + AlbumNotes: "", + AlbumFilter: "", + AlbumOrder: "name", + AlbumTemplate: "", + AlbumCountry: "zz", + AlbumYear: 0, + AlbumMonth: 0, + AlbumDay: 0, + AlbumFavorite: false, + AlbumPrivate: false, + CreatedAt: time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC), + UpdatedAt: time.Date(2020, 2, 1, 0, 0, 0, 0, time.UTC), + DeletedAt: nil, + }, } // CreateAlbumFixtures inserts known entities into the database for testing. diff --git a/internal/entity/const.go b/internal/entity/const.go index 3701f28d7..5219c46ae 100644 --- a/internal/entity/const.go +++ b/internal/entity/const.go @@ -64,9 +64,11 @@ const ( // Sort Orders const ( + SortOrderDefault = "" SortOrderRelevance = "relevance" SortOrderCount = "count" SortOrderAdded = "added" + SortOrderImported = "imported" SortOrderEdited = "edited" SortOrderNewest = "newest" SortOrderOldest = "oldest" diff --git a/internal/entity/db.go b/internal/entity/db.go index 0dbf4ea94..4d566ed10 100644 --- a/internal/entity/db.go +++ b/internal/entity/db.go @@ -10,10 +10,12 @@ import ( _ "github.com/jinzhu/gorm/dialects/sqlite" ) -// Database drivers (sql dialects). +// SQL Databases. const ( - MySQL = "mysql" - SQLite3 = "sqlite3" + MySQL = "mysql" + SQLite3 = "sqlite3" + SQLiteTestDB = ".test.db" + SQLiteMemoryDSN = ":memory:" ) var dbProvider DbProvider diff --git a/internal/entity/db_migrate.go b/internal/entity/db_migrate.go index 80a228b4a..98859c047 100644 --- a/internal/entity/db_migrate.go +++ b/internal/entity/db_migrate.go @@ -1,7 +1,10 @@ package entity import ( + "os" "time" + + "github.com/photoprism/photoprism/pkg/sanitize" ) // MigrateDb creates database tables and inserts default fixtures as needed. @@ -26,18 +29,31 @@ func InitTestDb(driver, dsn string) *Gorm { return nil } + // Set default test database driver. if driver == "test" || driver == "sqlite" || driver == "" || dsn == "" { - driver = "sqlite3" - dsn = ".test.db" + driver = SQLite3 + } + + // Set default database DSN. + if driver == SQLite3 { + if dsn == "" { + dsn = SQLiteMemoryDSN + } else if dsn != SQLiteTestDB { + // Continue. + } else if err := os.Remove(dsn); err == nil { + log.Debugf("sqlite: test file %s removed", sanitize.Log(dsn)) + } } log.Infof("initializing %s test db in %s", driver, dsn) + // Create ORM instance. db := &Gorm{ Driver: driver, Dsn: dsn, } + // Insert test fixtures. SetDbProvider(db) ResetTestFixtures() diff --git a/internal/entity/entity_test.go b/internal/entity/entity_test.go index 7301aefff..1a979d03f 100644 --- a/internal/entity/entity_test.go +++ b/internal/entity/entity_test.go @@ -4,20 +4,18 @@ import ( "os" "testing" - "github.com/stretchr/testify/assert" - "github.com/sirupsen/logrus" + "github.com/stretchr/testify/assert" ) func TestMain(m *testing.M) { log = logrus.StandardLogger() log.SetLevel(logrus.TraceLevel) - if err := os.Remove(".test.db"); err == nil { - log.Debugln("removed .test.db") - } + db := InitTestDb( + os.Getenv("PHOTOPRISM_TEST_DRIVER"), + os.Getenv("PHOTOPRISM_TEST_DSN")) - db := InitTestDb(os.Getenv("PHOTOPRISM_TEST_DRIVER"), os.Getenv("PHOTOPRISM_TEST_DSN")) defer db.Close() code := m.Run() diff --git a/internal/query/query_test.go b/internal/query/query_test.go index 592755a6a..aca54044e 100644 --- a/internal/query/query_test.go +++ b/internal/query/query_test.go @@ -4,19 +4,19 @@ import ( "os" "testing" - "github.com/photoprism/photoprism/internal/entity" "github.com/sirupsen/logrus" + + "github.com/photoprism/photoprism/internal/entity" ) func TestMain(m *testing.M) { log = logrus.StandardLogger() log.SetLevel(logrus.TraceLevel) - if err := os.Remove(".test.db"); err == nil { - log.Debugln("removed .test.db") - } + db := entity.InitTestDb( + os.Getenv("PHOTOPRISM_TEST_DRIVER"), + os.Getenv("PHOTOPRISM_TEST_DSN")) - db := entity.InitTestDb(os.Getenv("PHOTOPRISM_TEST_DRIVER"), os.Getenv("PHOTOPRISM_TEST_DSN")) defer db.Close() code := m.Run() diff --git a/internal/search/photos.go b/internal/search/photos.go index e8fd9e8b7..89cd39e4d 100644 --- a/internal/search/photos.go +++ b/internal/search/photos.go @@ -27,55 +27,87 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { s := UnscopedDb() // s = s.LogMode(true) - // Base query. - s = s.Table("photos"). - Select(`photos.*, photos.id AS composite_id, - files.id AS file_id, files.file_uid, files.instance_id, files.file_primary, files.file_sidecar, - files.file_portrait,files.file_video, files.file_missing, files.file_name, files.file_root, files.file_hash, - files.file_codec, files.file_type, files.file_mime, files.file_width, files.file_height, - files.file_aspect_ratio, files.file_orientation, files.file_main_color, files.file_colors, files.file_luminance, - files.file_chroma, files.file_projection, files.file_diff, files.file_duration, files.file_size, - cameras.camera_make, cameras.camera_model, - lenses.lens_make, lenses.lens_model, - places.place_label, places.place_city, places.place_state, places.place_country`). - Joins("JOIN files ON photos.id = files.photo_id AND files.file_missing = 0 AND files.deleted_at IS NULL"). + // Select columns. + cols := []string{ + "photos.*", + "files.file_uid", + "files.id AS file_id", + "files.photo_id AS composite_id", + "files.instance_id", + "files.file_primary", + "files.file_sidecar", + "files.file_portrait", + "files.file_video", + "files.file_missing", + "files.file_name", + "files.file_root", + "files.file_hash", + "files.file_codec", + "files.file_type", + "files.file_mime", + "files.file_width", + "files.file_height", + "files.file_aspect_ratio", + "files.file_orientation", + "files.file_main_color", + "files.file_colors", + "files.file_luminance", + "files.file_chroma", + "files.file_projection", + "files.file_diff", + "files.file_duration", + "files.file_size", + "cameras.camera_make", + "cameras.camera_model", + "lenses.lens_make", + "lenses.lens_model", + "places.place_label", + "places.place_city", + "places.place_state", + "places.place_country", + } + + // Database tables. + s = s.Table("files").Select(strings.Join(cols, ", ")). + Joins("JOIN photos ON photos.id = files.photo_id"). Joins("LEFT JOIN cameras ON photos.camera_id = cameras.id"). Joins("LEFT JOIN lenses ON photos.lens_id = lenses.id"). - Joins("LEFT JOIN places ON photos.place_id = places.id") + Joins("LEFT JOIN places ON photos.place_id = places.id"). + Where("files.deleted_at IS NULL AND files.file_missing = 0") - // Limit result count. + // Offset and count. if f.Count > 0 && f.Count <= MaxResults { s = s.Limit(f.Count).Offset(f.Offset) } else { s = s.Limit(MaxResults).Offset(f.Offset) } - // Set sort order. + // Sort order. switch f.Order { case entity.SortOrderEdited: - s = s.Where("edited_at IS NOT NULL").Order("edited_at DESC, photos.photo_uid, files.file_primary DESC") + s = s.Where("photos.edited_at IS NOT NULL").Order("photos.edited_at DESC, files.photo_id DESC, files.file_primary DESC, files.id") case entity.SortOrderRelevance: if f.Label != "" { - s = s.Order("photo_quality DESC, photos_labels.uncertainty ASC, taken_at DESC, files.file_primary DESC") + s = s.Order("photos.photo_quality DESC, photos_labels.uncertainty ASC, photos.taken_at DESC, files.photo_id DESC, files.file_primary DESC, files.id") } else { - s = s.Order("photo_quality DESC, taken_at DESC, files.file_primary DESC") + s = s.Order("photos.photo_quality DESC, photos.taken_at DESC, files.photo_id DESC, files.file_primary DESC, files.id") } case entity.SortOrderNewest: - s = s.Order("taken_at DESC, photos.photo_uid, files.file_primary DESC") + s = s.Order("photos.taken_at DESC, files.photo_id DESC, files.file_primary DESC, files.id") case entity.SortOrderOldest: - s = s.Order("taken_at, photos.photo_uid, files.file_primary DESC") - case entity.SortOrderAdded: - s = s.Order("photos.id DESC, files.file_primary DESC") + s = s.Order("photos.taken_at, files.photo_id DESC, files.file_primary DESC, files.id") case entity.SortOrderSimilar: s = s.Where("files.file_diff > 0") - s = s.Order("photos.photo_color, photos.cell_id, files.file_diff, taken_at DESC, files.file_primary DESC") + s = s.Order("photos.photo_color, photos.cell_id, files.file_diff, photos.taken_at DESC, files.photo_id DESC, files.file_primary DESC, files.id") case entity.SortOrderName: - s = s.Order("photos.photo_path, photos.photo_name, files.file_primary DESC") + s = s.Order("photos.photo_path, photos.photo_name, files.photo_id DESC, files.file_primary DESC, files.id") + case entity.SortOrderDefault, entity.SortOrderImported, entity.SortOrderAdded: + s = s.Order("files.photo_id DESC, files.file_primary DESC, files.id") default: - s = s.Order("taken_at DESC, photos.photo_uid, files.file_primary DESC") + return PhotoResults{}, 0, fmt.Errorf("invalid sort order") } - // Include hidden files? + // Show hidden files? if !f.Hidden { s = s.Where("files.file_type = 'jpg' OR files.file_video = 1") @@ -86,7 +118,7 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { } } - // Return primary files only. + // Primary files only? if f.Primary { s = s.Where("files.file_primary = 1") } @@ -96,7 +128,7 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { // Take shortcut? if f.Album == "" && f.Query == "" { - s = s.Order("files.file_primary DESC") + s = s.Order("files.photo_id DESC, files.file_primary DESC, files.id") if result := s.Scan(&results); result.Error != nil { return results, 0, result.Error @@ -105,7 +137,7 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { log.Debugf("photos: found %s for %s [%s]", english.Plural(len(results), "result", "results"), f.SerializeAll(), time.Since(start)) if f.Merged { - return results.Merged() + return results.Merge() } return results, len(results), nil @@ -134,7 +166,7 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { } } - s = s.Joins("JOIN photos_labels ON photos_labels.photo_id = photos.id AND photos_labels.uncertainty < 100 AND photos_labels.label_id IN (?)", labelIds). + s = s.Joins("JOIN photos_labels ON photos_labels.photo_id = files.photo_id AND photos_labels.uncertainty < 100 AND photos_labels.label_id IN (?)", labelIds). Group("photos.id, files.id") } } @@ -191,14 +223,14 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { s = s.Where("photos.cell_id <> 'zz'") for _, where := range LikeAnyKeyword("k.keyword", f.Query) { - s = s.Where("photos.id IN (SELECT pk.photo_id FROM keywords k JOIN photos_keywords pk ON k.id = pk.keyword_id WHERE (?))", gorm.Expr(where)) + s = s.Where("files.photo_id IN (SELECT pk.photo_id FROM keywords k JOIN photos_keywords pk ON k.id = pk.keyword_id WHERE (?))", gorm.Expr(where)) } } else if f.Query != "" { if err := Db().Where(AnySlug("custom_slug", f.Query, " ")).Find(&labels).Error; len(labels) == 0 || err != nil { log.Debugf("search: label %s not found, using fuzzy search", txt.LogParamLower(f.Query)) for _, where := range LikeAnyKeyword("k.keyword", f.Query) { - s = s.Where("photos.id IN (SELECT pk.photo_id FROM keywords k JOIN photos_keywords pk ON k.id = pk.keyword_id WHERE (?))", gorm.Expr(where)) + s = s.Where("files.photo_id IN (SELECT pk.photo_id FROM keywords k JOIN photos_keywords pk ON k.id = pk.keyword_id WHERE (?))", gorm.Expr(where)) } } else { for _, l := range labels { @@ -215,11 +247,11 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { if wheres := LikeAnyKeyword("k.keyword", f.Query); len(wheres) > 0 { for _, where := range wheres { - s = s.Where("photos.id IN (SELECT pk.photo_id FROM keywords k JOIN photos_keywords pk ON k.id = pk.keyword_id WHERE (?)) OR "+ - "photos.id IN (SELECT pl.photo_id FROM photos_labels pl WHERE pl.uncertainty < 100 AND pl.label_id IN (?))", gorm.Expr(where), labelIds) + s = s.Where("files.photo_id IN (SELECT pk.photo_id FROM keywords k JOIN photos_keywords pk ON k.id = pk.keyword_id WHERE (?)) OR "+ + "files.photo_id IN (SELECT pl.photo_id FROM photos_labels pl WHERE pl.uncertainty < 100 AND pl.label_id IN (?))", gorm.Expr(where), labelIds) } } else { - s = s.Where("photos.id IN (SELECT pl.photo_id FROM photos_labels pl WHERE pl.uncertainty < 100 AND pl.label_id IN (?))", labelIds) + s = s.Where("files.photo_id IN (SELECT pl.photo_id FROM photos_labels pl WHERE pl.uncertainty < 100 AND pl.label_id IN (?))", labelIds) } } } @@ -227,7 +259,7 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { // Search for one or more keywords? if txt.NotEmpty(f.Keywords) { for _, where := range LikeAnyWord("k.keyword", f.Keywords) { - s = s.Where("photos.id IN (SELECT pk.photo_id FROM keywords k JOIN photos_keywords pk ON k.id = pk.keyword_id WHERE (?))", gorm.Expr(where)) + s = s.Where("files.photo_id IN (SELECT pk.photo_id FROM keywords k JOIN photos_keywords pk ON k.id = pk.keyword_id WHERE (?))", gorm.Expr(where)) } } @@ -246,17 +278,17 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { // Filter for specific face clusters? Example: PLJ7A3G4MBGZJRMVDIUCBLC46IAP4N7O if len(f.Face) >= 32 { for _, f := range strings.Split(strings.ToUpper(f.Face), txt.And) { - s = s.Where(fmt.Sprintf("photos.id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 WHERE face_id IN (?))", + s = s.Where(fmt.Sprintf("files.photo_id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 WHERE face_id IN (?))", entity.Marker{}.TableName()), strings.Split(f, txt.Or)) } } else if txt.New(f.Face) { - s = s.Where(fmt.Sprintf("photos.id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 AND m.marker_type = ? WHERE subj_uid IS NULL OR subj_uid = '')", + s = s.Where(fmt.Sprintf("files.photo_id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 AND m.marker_type = ? WHERE subj_uid IS NULL OR subj_uid = '')", entity.Marker{}.TableName()), entity.MarkerFace) } else if txt.No(f.Face) { - s = s.Where(fmt.Sprintf("photos.id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 AND m.marker_type = ? WHERE face_id IS NULL OR face_id = '')", + s = s.Where(fmt.Sprintf("files.photo_id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 AND m.marker_type = ? WHERE face_id IS NULL OR face_id = '')", entity.Marker{}.TableName()), entity.MarkerFace) } else if txt.Yes(f.Face) { - s = s.Where(fmt.Sprintf("photos.id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 AND m.marker_type = ? WHERE face_id IS NOT NULL AND face_id <> '')", + s = s.Where(fmt.Sprintf("files.photo_id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 AND m.marker_type = ? WHERE face_id IS NOT NULL AND face_id <> '')", entity.Marker{}.TableName()), entity.MarkerFace) } @@ -264,16 +296,16 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { if txt.NotEmpty(f.Subject) { for _, subj := range strings.Split(strings.ToLower(f.Subject), txt.And) { if subjects := strings.Split(subj, txt.Or); rnd.ContainsUIDs(subjects, 'j') { - s = s.Where(fmt.Sprintf("photos.id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 WHERE subj_uid IN (?))", + s = s.Where(fmt.Sprintf("files.photo_id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 WHERE subj_uid IN (?))", entity.Marker{}.TableName()), subjects) } else { - s = s.Where(fmt.Sprintf("photos.id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 JOIN %s s ON s.subj_uid = m.subj_uid WHERE (?))", + s = s.Where(fmt.Sprintf("files.photo_id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 JOIN %s s ON s.subj_uid = m.subj_uid WHERE (?))", entity.Marker{}.TableName(), entity.Subject{}.TableName()), gorm.Expr(AnySlug("s.subj_slug", subj, txt.Or))) } } } else if txt.NotEmpty(f.Subjects) { for _, where := range LikeAllNames(Cols{"subj_name", "subj_alias"}, f.Subjects) { - s = s.Where(fmt.Sprintf("photos.id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 JOIN %s s ON s.subj_uid = m.subj_uid WHERE (?))", + s = s.Where(fmt.Sprintf("files.photo_id IN (SELECT photo_id FROM files f JOIN %s m ON f.file_uid = m.file_uid AND m.marker_invalid = 0 JOIN %s s ON s.subj_uid = m.subj_uid WHERE (?))", entity.Marker{}.TableName(), entity.Subject{}.TableName()), gorm.Expr(where)) } } @@ -491,25 +523,25 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { // Find stacks only? if f.Stack { - s = s.Where("photos.id IN (SELECT a.photo_id FROM files a JOIN files b ON a.id != b.id AND a.photo_id = b.photo_id AND a.file_type = b.file_type WHERE a.file_type='jpg')") + s = s.Where("files.photo_id IN (SELECT a.photo_id FROM files a JOIN files b ON a.id != b.id AND a.photo_id = b.photo_id AND a.file_type = b.file_type WHERE a.file_type='jpg')") } // Filter by album? if rnd.IsPPID(f.Album, 'a') { if f.Filter != "" { - s = s.Where("photos.photo_uid NOT IN (SELECT photo_uid FROM photos_albums pa WHERE pa.hidden = 1 AND pa.album_uid = ?)", f.Album) + s = s.Where("files.photo_uid NOT IN (SELECT photo_uid FROM photos_albums pa WHERE pa.hidden = 1 AND pa.album_uid = ?)", f.Album) } else { - s = s.Joins("JOIN photos_albums ON photos_albums.photo_uid = photos.photo_uid"). + s = s.Joins("JOIN photos_albums ON photos_albums.photo_uid = files.photo_uid"). Where("photos_albums.hidden = 0 AND photos_albums.album_uid = ?", f.Album) } } else if f.Unsorted && f.Filter == "" { - s = s.Where("photos.photo_uid NOT IN (SELECT photo_uid FROM photos_albums pa WHERE pa.hidden = 0)") + s = s.Where("files.photo_uid NOT IN (SELECT photo_uid FROM photos_albums pa WHERE pa.hidden = 0)") } else if txt.NotEmpty(f.Album) { v := strings.Trim(f.Album, "*%") + "%" - s = s.Where("photos.photo_uid IN (SELECT pa.photo_uid FROM photos_albums pa JOIN albums a ON a.album_uid = pa.album_uid AND pa.hidden = 0 WHERE (a.album_title LIKE ? OR a.album_slug LIKE ?))", v, v) + s = s.Where("files.photo_uid IN (SELECT pa.photo_uid FROM photos_albums pa JOIN albums a ON a.album_uid = pa.album_uid AND pa.hidden = 0 WHERE (a.album_title LIKE ? OR a.album_slug LIKE ?))", v, v) } else if txt.NotEmpty(f.Albums) { for _, where := range LikeAnyWord("a.album_title", f.Albums) { - s = s.Where("photos.photo_uid IN (SELECT pa.photo_uid FROM photos_albums pa JOIN albums a ON a.album_uid = pa.album_uid AND pa.hidden = 0 WHERE (?))", gorm.Expr(where)) + s = s.Where("files.photo_uid IN (SELECT pa.photo_uid FROM photos_albums pa JOIN albums a ON a.album_uid = pa.album_uid AND pa.hidden = 0 WHERE (?))", gorm.Expr(where)) } } @@ -520,7 +552,7 @@ func Photos(f form.SearchPhotos) (results PhotoResults, count int, err error) { log.Debugf("photos: found %s for %s [%s]", english.Plural(len(results), "result", "results"), f.SerializeAll(), time.Since(start)) if f.Merged { - return results.Merged() + return results.Merge() } return results, len(results), nil diff --git a/internal/search/photos_filter_album_test.go b/internal/search/photos_filter_album_test.go index 196959d2b..71e0f205c 100644 --- a/internal/search/photos_filter_album_test.go +++ b/internal/search/photos_filter_album_test.go @@ -63,7 +63,6 @@ func TestPhotosFilterAlbum(t *testing.T) { t.Fatal(err) } - // TODO: Needs review, variable number of results. assert.GreaterOrEqual(t, len(photos), 0) }) t.Run("album middle &", func(t *testing.T) { @@ -181,7 +180,6 @@ func TestPhotosFilterAlbum(t *testing.T) { if err != nil { t.Fatal(err) } - // TODO: Needs review, variable number of results. assert.GreaterOrEqual(t, len(photos), 0) }) @@ -196,7 +194,6 @@ func TestPhotosFilterAlbum(t *testing.T) { if err != nil { t.Fatal(err) } - // TODO: Needs review, variable number of results. assert.GreaterOrEqual(t, len(photos), 0) }) @@ -306,7 +303,6 @@ func TestPhotosQueryAlbum(t *testing.T) { t.Fatal(err) } - // TODO: Needs review, variable number of results. assert.GreaterOrEqual(t, len(photos), 0) }) t.Run("album middle &", func(t *testing.T) { @@ -442,9 +438,8 @@ func TestPhotosQueryAlbum(t *testing.T) { if err != nil { t.Fatal(err) } - // TODO: Needs review, variable number of results. - assert.GreaterOrEqual(t, len(photos), 0) + assert.Greater(t, 2, len(photos)) }) t.Run("album end |", func(t *testing.T) { var f form.SearchPhotos diff --git a/internal/search/photos_filter_albums_test.go b/internal/search/photos_filter_albums_test.go index 4ac453ec6..b0cee8cf3 100644 --- a/internal/search/photos_filter_albums_test.go +++ b/internal/search/photos_filter_albums_test.go @@ -3,8 +3,9 @@ package search import ( "testing" - "github.com/photoprism/photoprism/internal/form" "github.com/stretchr/testify/assert" + + "github.com/photoprism/photoprism/internal/form" ) func TestPhotosFilterAlbums(t *testing.T) { @@ -182,22 +183,20 @@ func TestPhotosFilterAlbums(t *testing.T) { var f form.SearchPhotos f.Albums = "Red|Green" - f.Merged = false + f.Merged = true + + photos, count, err := Photos(f) - UnscopedDb().LogMode(true) - photos, _, err := Photos(f) - UnscopedDb().LogMode(false) if err != nil { t.Fatal(err) } - // TODO: Needs review, variable number of results. - if len(photos) > 0 { - // UID: pt9jtdre2lvl0yh0 - t.Logf("Search Result: %#v", photos) + if len(photos) != 1 { + t.Logf("excactly one result expected, but %d photos with %d files found", len(photos), count) + t.Logf("query results: %#v", photos) } - assert.Equal(t, 0, len(photos)) + assert.Equal(t, 1, len(photos)) }) t.Run("EndsWithPipe", func(t *testing.T) { var f form.SearchPhotos @@ -210,6 +209,7 @@ func TestPhotosFilterAlbums(t *testing.T) { if err != nil { t.Fatal(err) } + assert.Greater(t, len(photos), 0) }) t.Run("StartsWithNumber", func(t *testing.T) { @@ -428,23 +428,20 @@ func TestPhotosQueryAlbums(t *testing.T) { var f form.SearchPhotos f.Query = "albums:\"Red|Green\"" - f.Merged = false - UnscopedDb().LogMode(true) - photos, _, err := Photos(f) - UnscopedDb().LogMode(false) + f.Merged = true + + photos, count, err := Photos(f) + if err != nil { t.Fatal(err) } - // TODO: Needs review, variable number of results. - if len(photos) > 0 { - // UID pt9jtdre2lvl0yh0 - for _, p := range photos { - t.Logf("%#v", p) - } + if len(photos) != 1 { + t.Logf("excactly one result expected, but %d photos with %d files found", len(photos), count) + t.Logf("query results: %#v", photos) } - assert.Equal(t, 0, len(photos)) + assert.Equal(t, 1, len(photos)) }) t.Run("EndsWithPipe", func(t *testing.T) { var f form.SearchPhotos diff --git a/internal/search/photos_filter_favorite_test.go b/internal/search/photos_filter_favorite_test.go index bee6da933..f78647542 100644 --- a/internal/search/photos_filter_favorite_test.go +++ b/internal/search/photos_filter_favorite_test.go @@ -116,7 +116,8 @@ func TestPhotosQueryFavorite(t *testing.T) { t.Run("CenterSingleQuote", func(t *testing.T) { var f form.SearchPhotos - f.Query = "favorite:\"Father's Day\"" + // Note: If the string in favorite starts with f/F, the txt package will assume it means false, + f.Query = "favorite:\"Mother's Day\"" f.Merged = true photos, _, err := Photos(f) @@ -124,7 +125,7 @@ func TestPhotosQueryFavorite(t *testing.T) { if err != nil { t.Fatal(err) } - //TODO should not fail + assert.Equal(t, len(photos), len(photos0)) }) t.Run("EndsWithSingleQuote", func(t *testing.T) { diff --git a/internal/search/photos_results.go b/internal/search/photos_results.go index 098cb3fb3..5c282afcf 100644 --- a/internal/search/photos_results.go +++ b/internal/search/photos_results.go @@ -111,38 +111,38 @@ func (m PhotoResults) UIDs() []string { return result } -func (m PhotoResults) Merged() (PhotoResults, int, error) { - count := len(m) - merged := make([]Photo, 0, count) +// Merge consecutive file results that belong to the same photo. +func (m PhotoResults) Merge() (photos PhotoResults, count int, err error) { + count = len(m) + photos = make(PhotoResults, 0, count) - var lastId uint var i int + var photoId uint - for _, res := range m { + for _, photo := range m { file := entity.File{} - if err := deepcopier.Copy(&file).From(res); err != nil { - return merged, count, err + if err = deepcopier.Copy(&file).From(photo); err != nil { + return photos, count, err } - file.ID = res.FileID - res.CompositeID = fmt.Sprintf("%d-%d", res.ID, res.FileID) + file.ID = photo.FileID - if lastId == res.ID && i > 0 { - merged[i-1].Files = append(merged[i-1].Files, file) - merged[i-1].Merged = true + if photoId == photo.ID && i > 0 { + photos[i-1].Files = append(photos[i-1].Files, file) + photos[i-1].Merged = true continue } - res.Files = append(res.Files, file) - - merged = append(merged, res) - - lastId = res.ID i++ + photoId = photo.ID + photo.CompositeID = fmt.Sprintf("%d-%d", photoId, file.ID) + photo.Files = append(photo.Files, file) + + photos = append(photos, photo) } - return merged, count, nil + return photos, count, nil } // ShareBase returns a meaningful file name for sharing. diff --git a/internal/search/photos_results_test.go b/internal/search/photos_results_test.go index 95793e84e..f05d103de 100644 --- a/internal/search/photos_results_test.go +++ b/internal/search/photos_results_test.go @@ -128,7 +128,7 @@ func TestPhotosResults_Merged(t *testing.T) { results := PhotoResults{result1, result2} - merged, count, err := results.Merged() + merged, count, err := results.Merge() if err != nil { t.Fatal(err) diff --git a/internal/search/search_test.go b/internal/search/search_test.go index eb87fc875..3dd609a50 100644 --- a/internal/search/search_test.go +++ b/internal/search/search_test.go @@ -12,11 +12,10 @@ func TestMain(m *testing.M) { log = logrus.StandardLogger() log.SetLevel(logrus.TraceLevel) - if err := os.Remove(".test.db"); err == nil { - log.Debugln("removed .test.db") - } + db := entity.InitTestDb( + os.Getenv("PHOTOPRISM_TEST_DRIVER"), + os.Getenv("PHOTOPRISM_TEST_DSN")) - db := entity.InitTestDb(os.Getenv("PHOTOPRISM_TEST_DRIVER"), os.Getenv("PHOTOPRISM_TEST_DSN")) defer db.Close() code := m.Run()