From a5f948d68a204d744daf8ce775557815e3ad014d Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Sat, 2 Apr 2022 22:23:38 +0200 Subject: [PATCH] Search: Improve filter value parsing and update tests #1994 --- internal/search/geojson.go | 14 ++-- internal/search/like.go | 35 +++++++++ internal/search/like_test.go | 76 +++++++++++++++++++ internal/search/photos.go | 22 +++--- .../search/photos_filter_category_test.go | 14 ++-- internal/search/photos_filter_country_test.go | 40 ++-------- internal/search/photos_filter_state_test.go | 22 +++--- 7 files changed, 154 insertions(+), 69 deletions(-) diff --git a/internal/search/geojson.go b/internal/search/geojson.go index 597521331..74ea411d3 100644 --- a/internal/search/geojson.go +++ b/internal/search/geojson.go @@ -151,9 +151,9 @@ func Geo(f form.SearchGeo) (results GeoResults, 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) { + for _, f := range SplitAnd(strings.ToUpper(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 WHERE face_id IN (?))", - entity.Marker{}.TableName()), strings.Split(f, txt.Or)) + entity.Marker{}.TableName()), SplitOr(f)) } } 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 = '')", @@ -168,8 +168,8 @@ func Geo(f form.SearchGeo) (results GeoResults, err error) { // Filter for one or more subjects? if f.Subject != "" { - for _, subj := range strings.Split(strings.ToLower(f.Subject), txt.And) { - if subjects := strings.Split(subj, txt.Or); rnd.ContainsUIDs(subjects, 'j') { + for _, subj := range SplitAnd(strings.ToLower(f.Subject)) { + if subjects := SplitOr(subj); 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 (?))", entity.Marker{}.TableName()), subjects) } else { @@ -230,7 +230,7 @@ func Geo(f form.SearchGeo) (results GeoResults, err error) { // Filter by main color? if f.Color != "" { - s = s.Where("files.file_main_color IN (?)", strings.Split(strings.ToLower(f.Color), txt.Or)) + s = s.Where("files.file_main_color IN (?)", SplitOr(strings.ToLower(f.Color))) } // Find favorites only? @@ -250,12 +250,12 @@ func Geo(f form.SearchGeo) (results GeoResults, err error) { // Filter by location country? if f.Country != "" { - s = s.Where("photos.photo_country IN (?)", strings.Split(strings.ToLower(f.Country), txt.Or)) + s = s.Where("photos.photo_country IN (?)", SplitOr(strings.ToLower(f.Country))) } // Filter by media type? if f.Type != "" { - s = s.Where("photos.photo_type IN (?)", strings.Split(strings.ToLower(f.Type), txt.Or)) + s = s.Where("photos.photo_type IN (?)", SplitOr(strings.ToLower(f.Type))) } else if f.Video { s = s.Where("photos.photo_type = 'video'") } else if f.Photo { diff --git a/internal/search/like.go b/internal/search/like.go index d7c1194a1..6853e8726 100644 --- a/internal/search/like.go +++ b/internal/search/like.go @@ -261,3 +261,38 @@ func OrLike(col, s string) (where string, values []interface{}) { return where, values } + +// Split splits a search string into separate values and trims whitespace. +func Split(s string, sep string) (result []string) { + if s == "" { + return []string{} + } + + // Trim separator and split. + s = strings.Trim(s, sep) + v := strings.Split(s, sep) + + if len(v) <= 1 { + return v + } + + result = make([]string, 0, len(v)) + + for i := range v { + if t := strings.TrimSpace(v[i]); t != "" { + result = append(result, t) + } + } + + return result +} + +// SplitOr splits a search string into separate OR values for an IN condition. +func SplitOr(s string) (values []string) { + return Split(s, txt.Or) +} + +// SplitAnd splits a search string into separate AND values. +func SplitAnd(s string) (values []string) { + return Split(s, txt.And) +} diff --git a/internal/search/like_test.go b/internal/search/like_test.go index aa3ff2b47..cb54c7c04 100644 --- a/internal/search/like_test.go +++ b/internal/search/like_test.go @@ -358,3 +358,79 @@ func TestOrLike(t *testing.T) { assert.Equal(t, []interface{}{"1990%", "2790/07/27900704_070228_D6D51B6C.jpg"}, values) }) } + +func TestSplit(t *testing.T) { + t.Run("Empty", func(t *testing.T) { + values := Split("", "") + + assert.Equal(t, []string{}, values) + }) + t.Run("FooBar", func(t *testing.T) { + values := Split(" foo | Bar ", "&") + + assert.Equal(t, []string{" foo | Bar "}, values) + }) + t.Run("FooAndBar", func(t *testing.T) { + values := Split(" foo & Bar ", "o") + + assert.Equal(t, []string{"f", "& Bar"}, values) + }) + t.Run("FooAndBarAndBaz", func(t *testing.T) { + values := Split(" foo & Bar&BAZ ", "&") + + assert.Equal(t, []string{"foo", "Bar", "BAZ"}, values) + }) +} +func TestSplitOr(t *testing.T) { + t.Run("Empty", func(t *testing.T) { + values := SplitOr("") + + assert.Equal(t, []string{}, values) + }) + t.Run("FooBar", func(t *testing.T) { + values := SplitOr(" foo | Bar ") + + assert.Equal(t, []string{"foo", "Bar"}, values) + }) + t.Run("FooBarTrim", func(t *testing.T) { + values := SplitOr(" foo | Bar |") + + assert.Equal(t, []string{"foo", "Bar"}, values) + }) + t.Run("FooAndBar", func(t *testing.T) { + values := SplitOr(" foo & Bar ") + + assert.Equal(t, []string{" foo & Bar "}, values) + }) + t.Run("FooAndBarAndBaz", func(t *testing.T) { + values := SplitOr(" foo & Bar&BAZ ") + + assert.Equal(t, []string{" foo & Bar&BAZ "}, values) + }) +} + +func TestSplitAnd(t *testing.T) { + t.Run("Empty", func(t *testing.T) { + values := SplitAnd("") + + assert.Equal(t, []string{}, values) + }) + + t.Run("FooOrBar", func(t *testing.T) { + values := SplitAnd(" foo | Bar ") + + assert.Equal(t, []string{" foo | Bar "}, values) + }) + + t.Run("FooAndBar", func(t *testing.T) { + values := SplitAnd(" foo & Bar ") + + assert.Equal(t, []string{"foo", "Bar"}, values) + }) + + t.Run("FooAndBarAndBaz", func(t *testing.T) { + values := SplitAnd(" foo & Bar&BAZ ") + + assert.Equal(t, []string{"foo", "Bar", "BAZ"}, values) + }) +} diff --git a/internal/search/photos.go b/internal/search/photos.go index 91c98d349..b48f3b5fb 100644 --- a/internal/search/photos.go +++ b/internal/search/photos.go @@ -104,7 +104,7 @@ func searchPhotos(f form.SearchPhotos, resultCols string) (results PhotoResults, } if txt.NotEmpty(f.UID) { - s = s.Where("photos.photo_uid IN (?)", strings.Split(strings.ToLower(f.UID), txt.Or)) + s = s.Where("photos.photo_uid IN (?)", SplitOr(strings.ToLower(f.UID))) // Take shortcut? if f.Album == "" && f.Query == "" { @@ -256,9 +256,9 @@ func searchPhotos(f form.SearchPhotos, resultCols string) (results PhotoResults, // Filter for specific face clusters? Example: PLJ7A3G4MBGZJRMVDIUCBLC46IAP4N7O if len(f.Face) >= 32 { - for _, f := range strings.Split(strings.ToUpper(f.Face), txt.And) { + for _, f := range SplitAnd(strings.ToUpper(f.Face)) { 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)) + entity.Marker{}.TableName()), SplitOr(f)) } } else if txt.New(f.Face) { 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 = '')", @@ -273,8 +273,8 @@ func searchPhotos(f form.SearchPhotos, resultCols string) (results PhotoResults, // Filter for one or more subjects? 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') { + for _, subj := range SplitAnd(strings.ToLower(f.Subject)) { + if subjects := SplitOr(subj); rnd.ContainsUIDs(subjects, 'j') { 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 { @@ -345,7 +345,7 @@ func searchPhotos(f form.SearchPhotos, resultCols string) (results PhotoResults, // Filter by main color? if f.Color != "" { - s = s.Where("files.file_main_color IN (?)", strings.Split(strings.ToLower(f.Color), txt.Or)) + s = s.Where("files.file_main_color IN (?)", SplitOr(strings.ToLower(f.Color))) } // Find favorites only? @@ -376,23 +376,23 @@ func searchPhotos(f form.SearchPhotos, resultCols string) (results PhotoResults, // Filter by location country? if txt.NotEmpty(f.Country) { - s = s.Where("photos.photo_country IN (?)", strings.Split(strings.ToLower(f.Country), txt.Or)) + s = s.Where("photos.photo_country IN (?)", SplitOr(strings.ToLower(f.Country))) } // Filter by location state? if txt.NotEmpty(f.State) { - s = s.Where("places.place_state IN (?)", strings.Split(f.State, txt.Or)) + s = s.Where("places.place_state IN (?)", SplitOr(f.State)) } // Filter by location category? if txt.NotEmpty(f.Category) { s = s.Joins("JOIN cells ON photos.cell_id = cells.id"). - Where("cells.cell_category IN (?)", strings.Split(strings.ToLower(f.Category), txt.Or)) + Where("cells.cell_category IN (?)", SplitOr(strings.ToLower(f.Category))) } // Filter by media type? if txt.NotEmpty(f.Type) { - s = s.Where("photos.photo_type IN (?)", strings.Split(strings.ToLower(f.Type), txt.Or)) + s = s.Where("photos.photo_type IN (?)", SplitOr(strings.ToLower(f.Type))) } else if f.Video { s = s.Where("photos.photo_type = 'video'") } else if f.Photo { @@ -451,7 +451,7 @@ func searchPhotos(f form.SearchPhotos, resultCols string) (results PhotoResults, // Filter by file hash? if txt.NotEmpty(f.Hash) { - s = s.Where("files.file_hash IN (?)", strings.Split(strings.ToLower(f.Hash), txt.Or)) + s = s.Where("files.file_hash IN (?)", SplitOr(strings.ToLower(f.Hash))) } if f.Mono { diff --git a/internal/search/photos_filter_category_test.go b/internal/search/photos_filter_category_test.go index f3f584d07..461140976 100644 --- a/internal/search/photos_filter_category_test.go +++ b/internal/search/photos_filter_category_test.go @@ -143,7 +143,7 @@ func TestPhotosFilterCategory(t *testing.T) { t.Run("CenterAsterisk", func(t *testing.T) { var f form.SearchPhotos - f.Category = "My*Kids" + f.Category = "My*Camping" f.Merged = true photos, _, err := Photos(f) @@ -156,7 +156,7 @@ func TestPhotosFilterCategory(t *testing.T) { t.Run("EndsWithAsterisk", func(t *testing.T) { var f form.SearchPhotos - f.Category = "Yoga***" + f.Category = "Camping***" f.Merged = true photos, _, err := Photos(f) @@ -169,7 +169,7 @@ func TestPhotosFilterCategory(t *testing.T) { t.Run("StartsWithPipe", func(t *testing.T) { var f form.SearchPhotos - f.Category = "|Banana" + f.Category = "|Camping" f.Merged = true photos, _, err := Photos(f) @@ -183,7 +183,7 @@ func TestPhotosFilterCategory(t *testing.T) { t.Run("CenterPipe", func(t *testing.T) { var f form.SearchPhotos - f.Category = "Red|Green" + f.Category = "Camping|botanical garden" f.Merged = true photos, _, err := Photos(f) @@ -192,12 +192,14 @@ func TestPhotosFilterCategory(t *testing.T) { t.Fatal(err) } - assert.Equal(t, len(photos), 0) + assert.GreaterOrEqual(t, len(photos), 30) }) t.Run("EndsWithPipe", func(t *testing.T) { var f form.SearchPhotos - f.Category = "Blue|" + // Db().LogMode(true) + + f.Category = "Camping|" f.Merged = true photos, _, err := Photos(f) diff --git a/internal/search/photos_filter_country_test.go b/internal/search/photos_filter_country_test.go index 84c23928c..7c5f9ea02 100644 --- a/internal/search/photos_filter_country_test.go +++ b/internal/search/photos_filter_country_test.go @@ -48,8 +48,7 @@ func TestPhotosFilterCountry(t *testing.T) { } assert.GreaterOrEqual(t, len(photos), 13) }) - //TODO - /*t.Run("mx whitespace pipe whitespace de", func(t *testing.T) { + t.Run("mx whitespace pipe whitespace de", func(t *testing.T) { var f form.SearchPhotos f.Country = "mx | de" @@ -62,32 +61,6 @@ func TestPhotosFilterCountry(t *testing.T) { } assert.GreaterOrEqual(t, len(photos), 13) }) - t.Run("mx or de", func(t *testing.T) { - var f form.SearchPhotos - - f.Country = "mx or de" - f.Merged = true - - photos, _, err := Photos(f) - - if err != nil { - t.Fatal(err) - } - assert.GreaterOrEqual(t, len(photos), 13) - }) - t.Run("mx OR de", func(t *testing.T) { - var f form.SearchPhotos - - f.Country = "mx OR de" - f.Merged = true - - photos, _, err := Photos(f) - - if err != nil { - t.Fatal(err) - } - assert.GreaterOrEqual(t, len(photos), 13) - })*/ t.Run("StartsWithPercent", func(t *testing.T) { var f form.SearchPhotos @@ -284,7 +257,7 @@ func TestPhotosFilterCountry(t *testing.T) { t.Fatal(err) } - assert.Equal(t, len(photos), 0) + assert.Equal(t, 0, len(photos)) }) t.Run("StartsWithNumber", func(t *testing.T) { var f form.SearchPhotos @@ -367,8 +340,7 @@ func TestPhotosQueryCountry(t *testing.T) { } assert.GreaterOrEqual(t, len(photos), 13) }) - //TODO - /*t.Run("mx whitespace pipe whitespace de", func(t *testing.T) { + t.Run("mx whitespace pipe whitespace de", func(t *testing.T) { var f form.SearchPhotos f.Query = "country:\"mx | de\"" @@ -393,7 +365,7 @@ func TestPhotosQueryCountry(t *testing.T) { if err != nil { t.Fatal(err) } - assert.GreaterOrEqual(t, len(photos), 13) + assert.Equal(t, 0, len(photos)) }) t.Run("mx OR de", func(t *testing.T) { var f form.SearchPhotos @@ -406,8 +378,8 @@ func TestPhotosQueryCountry(t *testing.T) { if err != nil { t.Fatal(err) } - assert.GreaterOrEqual(t, len(photos), 13) - })*/ + assert.Equal(t, 0, len(photos)) + }) t.Run("StartsWithPercent", func(t *testing.T) { var f form.SearchPhotos diff --git a/internal/search/photos_filter_state_test.go b/internal/search/photos_filter_state_test.go index 31d894a53..c9595245f 100644 --- a/internal/search/photos_filter_state_test.go +++ b/internal/search/photos_filter_state_test.go @@ -138,7 +138,7 @@ func TestPhotosFilterState(t *testing.T) { if err != nil { t.Fatal(err) } - assert.Equal(t, len(photos), 0) + assert.Equal(t, 0, len(photos)) }) t.Run("CenterAmpersand", func(t *testing.T) { var f form.SearchPhotos @@ -248,7 +248,7 @@ func TestPhotosFilterState(t *testing.T) { t.Run("StartsWithPipe", func(t *testing.T) { var f form.SearchPhotos - f.State = "|Banana" + f.State = "|New york" f.Merged = true photos, _, err := Photos(f) @@ -256,11 +256,11 @@ func TestPhotosFilterState(t *testing.T) { if err != nil { t.Fatal(err) } - t.Log(photos[0].PlaceLabel) - t.Log(photos[1].PlaceLabel) + // t.Log(photos[0].PlaceLabel) + // t.Log(photos[1].PlaceLabel) //TODO shows photos where places set but state empty - assert.Equal(t, len(photos), 2) + assert.Equal(t, 1, len(photos)) }) t.Run("CenterPipe", func(t *testing.T) { var f form.SearchPhotos @@ -274,12 +274,12 @@ func TestPhotosFilterState(t *testing.T) { t.Fatal(err) } - assert.Equal(t, len(photos), 0) + assert.Equal(t, 0, len(photos)) }) t.Run("EndsWithPipe", func(t *testing.T) { var f form.SearchPhotos - f.State = "Blue|" + f.State = "Rheinland-Pfalz|" f.Merged = true photos, _, err := Photos(f) @@ -287,10 +287,10 @@ func TestPhotosFilterState(t *testing.T) { if err != nil { t.Fatal(err) } - t.Log(photos[0].PlaceLabel) - t.Log(photos[1].PlaceLabel) + // t.Log(photos[0].PlaceLabel) + // t.Log(photos[1].PlaceLabel) //TODO shows photos where places set but state empty - assert.Equal(t, len(photos), 2) + assert.Equal(t, 4, len(photos)) }) t.Run("StartsWithNumber", func(t *testing.T) { var f form.SearchPhotos @@ -303,7 +303,7 @@ func TestPhotosFilterState(t *testing.T) { if err != nil { t.Fatal(err) } - assert.Equal(t, len(photos), 0) + assert.Equal(t, 0, len(photos)) }) t.Run("CenterNumber", func(t *testing.T) { var f form.SearchPhotos