From fbf675fbfbb92ed5bd9a69d624b57c3e9c431565 Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Fri, 5 Jun 2020 16:49:32 +0200 Subject: [PATCH] Add s2 prefix to all cell ids Fixes location search when using SQLite Signed-off-by: Michael Mayer --- frontend/src/pages/album/photos.vue | 4 +- frontend/src/pages/photos.vue | 4 +- internal/entity/location.go | 8 +- internal/entity/location_fixtures.go | 26 +++--- internal/entity/place_fixtures.go | 20 +++-- internal/entity/place_test.go | 11 +-- internal/maps/location.go | 5 ++ internal/maps/location_test.go | 8 ++ internal/maps/places/location_test.go | 13 ++- internal/photoprism/location_test.go | 5 +- internal/query/geo.go | 6 +- pkg/s2/prefix.go | 44 ++++++++++ pkg/s2/prefix_test.go | 111 ++++++++++++++++++++++++++ pkg/s2/s2.go | 6 +- 14 files changed, 230 insertions(+), 41 deletions(-) create mode 100644 pkg/s2/prefix.go create mode 100644 pkg/s2/prefix_test.go diff --git a/frontend/src/pages/album/photos.vue b/frontend/src/pages/album/photos.vue index 2d6dd29db..51e6c3574 100644 --- a/frontend/src/pages/album/photos.vue +++ b/frontend/src/pages/album/photos.vue @@ -126,9 +126,9 @@ const photo = this.results[index]; if (photo.LocationID) { - this.$router.push({name: "place", params: {q: "s2:" + photo.LocationID}}); + this.$router.push({name: "place", params: {q: photo.LocationID}}); } else if (photo.PlaceID.length > 3) { - this.$router.push({name: "place", params: {q: "s2:" + photo.PlaceID}}); + this.$router.push({name: "place", params: {q: photo.PlaceID}}); } }, editPhoto(index) { diff --git a/frontend/src/pages/photos.vue b/frontend/src/pages/photos.vue index d2653ddda..46691d1e7 100644 --- a/frontend/src/pages/photos.vue +++ b/frontend/src/pages/photos.vue @@ -166,9 +166,9 @@ const photo = this.results[index]; if (photo.LocationID) { - this.$router.push({name: "place", params: {q: "s2:" + photo.LocationID}}); + this.$router.push({name: "place", params: {q: photo.LocationID}}); } else if (photo.PlaceID.length > 3) { - this.$router.push({name: "place", params: {q: "s2:" + photo.PlaceID}}); + this.$router.push({name: "place", params: {q: photo.PlaceID}}); } }, editPhoto(index) { diff --git a/internal/entity/location.go b/internal/entity/location.go index 1829de1eb..bd47f06da 100644 --- a/internal/entity/location.go +++ b/internal/entity/location.go @@ -41,7 +41,7 @@ func CreateUnknownLocation() { func NewLocation(lat, lng float32) *Location { result := &Location{} - result.ID = s2.Token(float64(lat), float64(lng)) + result.ID = s2.PrefixedToken(float64(lat), float64(lng)) return result } @@ -57,7 +57,7 @@ func (m *Location) Find(api string) error { } l := &maps.Location{ - ID: m.ID, + ID: s2.NormalizeToken(m.ID), } if err := l.QueryApi(api); err != nil { @@ -65,11 +65,11 @@ func (m *Location) Find(api string) error { return err } - if place := FindPlace(l.S2Token(), l.Label()); place != nil { + if place := FindPlace(l.PrefixedToken(), l.Label()); place != nil { m.Place = place } else { place = &Place{ - ID: l.S2Token(), + ID: l.PrefixedToken(), LocLabel: l.Label(), LocCity: l.City(), LocState: l.State(), diff --git a/internal/entity/location_fixtures.go b/internal/entity/location_fixtures.go index 36c2ca738..fa2c2a869 100644 --- a/internal/entity/location_fixtures.go +++ b/internal/entity/location_fixtures.go @@ -1,6 +1,10 @@ package entity -import "time" +import ( + "time" + + "github.com/photoprism/photoprism/pkg/s2" +) type LocationMap map[string]Location @@ -22,7 +26,7 @@ func (m LocationMap) Pointer(name string) *Location { var LocationFixtures = LocationMap{ "mexico": { - ID: "85d1ea7d382c", + ID: s2.TokenPrefix+"85d1ea7d382c", PlaceID: PlaceFixtures.Get("mexico").ID, LocName: "Adosada Platform", LocCategory: "botanical garden", @@ -32,10 +36,10 @@ var LocationFixtures = LocationMap{ UpdatedAt: time.Now(), }, "caravan park": { - ID: "1ef75a71a36c", - PlaceID: "1ef75a71a36c", + ID: s2.TokenPrefix+"1ef75a71a36c", + PlaceID: s2.TokenPrefix+"1ef75a71a36c", Place: &Place{ - ID: "1ef75a71a36", + ID: "x1ef75a71a36", LocLabel: "Mandeni, KwaZulu-Natal, South Africa", LocCity: "Mandeni", LocState: "KwaZulu-Natal", @@ -50,7 +54,7 @@ var LocationFixtures = LocationMap{ UpdatedAt: time.Now(), }, "zinkwazi": { - ID: "1ef744d1e28c", + ID: s2.TokenPrefix+"1ef744d1e28c", PlaceID: PlaceFixtures.Get("zinkwazi").ID, Place: PlaceFixtures.Pointer("zinkwazi"), LocName: "Zinkwazi Beach", @@ -60,7 +64,7 @@ var LocationFixtures = LocationMap{ UpdatedAt: time.Now(), }, "hassloch": { - ID: "1ef744d1e280", + ID: s2.TokenPrefix+"1ef744d1e280", PlaceID: PlaceFixtures.Get("holidaypark").ID, Place: PlaceFixtures.Pointer("holidaypark"), LocName: "Holiday Park", @@ -70,7 +74,7 @@ var LocationFixtures = LocationMap{ UpdatedAt: time.Now(), }, "emptyNameLongCity": { - ID: "1ef744d1e281", + ID: s2.TokenPrefix+"1ef744d1e281", PlaceID: PlaceFixtures.Get("emptyNameLongCity").ID, Place: PlaceFixtures.Pointer("emptyNameLongCity"), LocName: "", @@ -80,7 +84,7 @@ var LocationFixtures = LocationMap{ UpdatedAt: time.Now(), }, "emptyNameShortCity": { - ID: "1ef744d1e282", + ID: s2.TokenPrefix+"1ef744d1e282", PlaceID: PlaceFixtures.Get("emptyNameShortCity").ID, Place: PlaceFixtures.Pointer("emptyNameShortCity"), LocName: "", @@ -90,7 +94,7 @@ var LocationFixtures = LocationMap{ UpdatedAt: time.Now(), }, "veryLongLocName": { - ID: "1ef744d1e283", + ID: s2.TokenPrefix+"1ef744d1e283", PlaceID: PlaceFixtures.Get("veryLongLocName").ID, Place: PlaceFixtures.Pointer("veryLongLocName"), LocName: "longlonglonglonglonglonglonglonglonglonglonglonglongName", @@ -100,7 +104,7 @@ var LocationFixtures = LocationMap{ UpdatedAt: time.Now(), }, "mediumLongLocName": { - ID: "1ef744d1e283", + ID: s2.TokenPrefix+"1ef744d1e283", PlaceID: PlaceFixtures.Get("mediumLongLocName").ID, Place: PlaceFixtures.Pointer("mediumLongLocName"), LocName: "longlonglonglonglonglongName", diff --git a/internal/entity/place_fixtures.go b/internal/entity/place_fixtures.go index 5a21a0645..de05b522e 100644 --- a/internal/entity/place_fixtures.go +++ b/internal/entity/place_fixtures.go @@ -1,6 +1,10 @@ package entity -import "time" +import ( + "time" + + "github.com/photoprism/photoprism/pkg/s2" +) type PlacesMap map[string]Place @@ -22,7 +26,7 @@ func (m PlacesMap) Pointer(name string) *Place { var PlaceFixtures = PlacesMap{ "mexico": { - ID: "85d1ea7d3278", + ID: s2.TokenPrefix+"85d1ea7d3278", LocLabel: "Teotihuacán, Mexico, Mexico", LocCity: "Teotihuacán", LocState: "State of Mexico", @@ -35,7 +39,7 @@ var PlaceFixtures = PlacesMap{ UpdatedAt: time.Now(), }, "zinkwazi": { - ID: "1ef744d1e279", + ID: s2.TokenPrefix+"1ef744d1e279", LocLabel: "KwaDukuza, KwaZulu-Natal, South Africa", LocCity: "KwaDukuza", LocState: "KwaZulu-Natal", @@ -48,7 +52,7 @@ var PlaceFixtures = PlacesMap{ UpdatedAt: time.Now(), }, "holidaypark": { - ID: "1ef744d1e280", + ID: s2.TokenPrefix+"1ef744d1e280", LocLabel: "Holiday Park, Amusement", LocCity: "", LocState: "Rheinland-Pfalz", @@ -61,7 +65,7 @@ var PlaceFixtures = PlacesMap{ UpdatedAt: time.Now(), }, "emptyNameLongCity": { - ID: "1ef744d1e281", + ID: s2.TokenPrefix+"1ef744d1e281", LocLabel: "labelEmptyNameLongCity", LocCity: "longlonglonglonglongcity", LocState: "Rheinland-Pfalz", @@ -74,7 +78,7 @@ var PlaceFixtures = PlacesMap{ UpdatedAt: time.Now(), }, "emptyNameShortCity": { - ID: "1ef744d1e282", + ID: s2.TokenPrefix+"1ef744d1e282", LocLabel: "labelEmptyNameShortCity", LocCity: "shortcity", LocState: "Rheinland-Pfalz", @@ -87,7 +91,7 @@ var PlaceFixtures = PlacesMap{ UpdatedAt: time.Now(), }, "veryLongLocName": { - ID: "1ef744d1e283", + ID: s2.TokenPrefix+"1ef744d1e283", LocLabel: "labelVeryLongLocName", LocCity: "Mainz", LocState: "Rheinland-Pfalz", @@ -100,7 +104,7 @@ var PlaceFixtures = PlacesMap{ UpdatedAt: time.Now(), }, "mediumLongLocName": { - ID: "1ef744d1e284", + ID: s2.TokenPrefix+"1ef744d1e284", LocLabel: "labelMediumLongLocName", LocCity: "New york", LocState: "New york", diff --git a/internal/entity/place_test.go b/internal/entity/place_test.go index 87a9dd003..2a4bf272c 100644 --- a/internal/entity/place_test.go +++ b/internal/entity/place_test.go @@ -4,6 +4,7 @@ import ( "testing" "time" + "github.com/photoprism/photoprism/pkg/s2" "github.com/stretchr/testify/assert" ) @@ -14,7 +15,7 @@ func TestCreateUnknownPlace(t *testing.T) { func TestFindPlaceByLabel(t *testing.T) { t.Run("find by id", func(t *testing.T) { - r := FindPlace("1ef744d1e280", "") + r := FindPlace(s2.TokenPrefix+"1ef744d1e280", "") if r == nil { t.Fatal("result should not be nil") @@ -23,7 +24,7 @@ func TestFindPlaceByLabel(t *testing.T) { assert.Equal(t, "de", r.LocCountry) }) t.Run("find by id", func(t *testing.T) { - r := FindPlace("85d1ea7d3278", "") + r := FindPlace(s2.TokenPrefix+"85d1ea7d3278", "") if r == nil { t.Fatal("result should not be nil") @@ -57,7 +58,7 @@ func TestPlace_Find(t *testing.T) { }) t.Run("record does not exist", func(t *testing.T) { place := &Place{ - ID: "1110", + ID: s2.TokenPrefix+"1110", LocLabel: "test", LocCity: "testCity", LocState: "", @@ -70,8 +71,8 @@ func TestPlace_Find(t *testing.T) { UpdatedAt: time.Now(), New: false, } - r := place.Find() - assert.Equal(t, "record not found", r.Error()) + err := place.Find() + assert.EqualError(t, err, "record not found") }) } diff --git a/internal/maps/location.go b/internal/maps/location.go index 193119294..0f9b7bcc1 100644 --- a/internal/maps/location.go +++ b/internal/maps/location.go @@ -6,6 +6,7 @@ import ( "github.com/photoprism/photoprism/internal/maps/osm" "github.com/photoprism/photoprism/internal/maps/places" + "github.com/photoprism/photoprism/pkg/s2" ) /* TODO @@ -156,6 +157,10 @@ func (l Location) S2Token() string { return l.ID } +func (l Location) PrefixedToken() string { + return s2.Prefix(l.ID) +} + func (l Location) Name() string { return l.LocName } diff --git a/internal/maps/location_test.go b/internal/maps/location_test.go index 46f3c78bd..b8b27628a 100644 --- a/internal/maps/location_test.go +++ b/internal/maps/location_test.go @@ -245,6 +245,14 @@ func TestLocation_S2Token(t *testing.T) { }) } +func TestLocation_PrefixedToken(t *testing.T) { + t.Run("123", func(t *testing.T) { + l := NewLocation("123", "Indian ocean", "", "", "Nürnberg", "Bayern", "de", "", []string{}) + + assert.Equal(t, s2.TokenPrefix+"123", l.PrefixedToken()) + }) +} + func TestLocation_Name(t *testing.T) { t.Run("Christkindlesmarkt", func(t *testing.T) { l := NewLocation("", "Christkindlesmarkt", "", "", "Nürnberg", "Bayern", "de", "", []string{}) diff --git a/internal/maps/places/location_test.go b/internal/maps/places/location_test.go index 461adf62d..ef7cabf7e 100644 --- a/internal/maps/places/location_test.go +++ b/internal/maps/places/location_test.go @@ -28,6 +28,11 @@ func TestFindLocation(t *testing.T) { assert.Error(t, err, "places: skipping lat 0.000000, lng 0.000000") t.Log(l) }) + t.Run("short id", func(t *testing.T) { + l, err := FindLocation("ab") + assert.Error(t, err, "places: skipping lat 0.000000, lng 0.000000") + t.Log(l) + }) t.Run("invalid id", func(t *testing.T) { l, err := FindLocation("") assert.Error(t, err, "places: invalid location id ") @@ -35,13 +40,13 @@ func TestFindLocation(t *testing.T) { }) t.Run("cached true", func(t *testing.T) { var p = NewPlace("1", "", "", "", "de", "") - location := NewLocation("54", 52.51961810676184, 13.40806264572578, "TestLocation", "test", p, true) + location := NewLocation("1e95998417cc", 52.51961810676184, 13.40806264572578, "TestLocation", "test", p, true) l, err := FindLocation(location.ID) if err != nil { t.Fatal(err) } assert.Equal(t, false, l.Cached) - l2, err2 := FindLocation("54") + l2, err2 := FindLocation("1e95998417cc") if err2 != nil { t.Fatal(err2) @@ -52,9 +57,9 @@ func TestFindLocation(t *testing.T) { func TestLocationGetters(t *testing.T) { var p = NewPlace("1", "testLabel", "berlin", "berlin", "de", "foobar") - location := NewLocation("54", 52.51961810676184, 13.40806264572578, "TestLocation", "test", p, true) + location := NewLocation("1e95998417cc", 52.51961810676184, 13.40806264572578, "TestLocation", "test", p, true) t.Run("wrong id", func(t *testing.T) { - assert.Equal(t, "54", location.CellID()) + assert.Equal(t, "1e95998417cc", location.CellID()) assert.Equal(t, "TestLocation", location.Name()) assert.Equal(t, "test", location.Category()) assert.Equal(t, "testLabel", location.Label()) diff --git a/internal/photoprism/location_test.go b/internal/photoprism/location_test.go index 6aebdbe40..95dc44748 100644 --- a/internal/photoprism/location_test.go +++ b/internal/photoprism/location_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/photoprism/photoprism/internal/config" + "github.com/photoprism/photoprism/pkg/s2" "github.com/stretchr/testify/assert" ) @@ -32,7 +33,7 @@ func TestMediaFile_Location(t *testing.T) { assert.Equal(t, "Hyogo Prefecture", location.State()) assert.Equal(t, "Japan", location.CountryName()) assert.Equal(t, "", location.Category()) - assert.True(t, strings.HasPrefix(location.ID, "3554df45")) + assert.True(t, strings.HasPrefix(location.ID, s2.TokenPrefix+"3554df45")) location2, err := mediaFile.Location() if err != nil { @@ -67,7 +68,7 @@ func TestMediaFile_Location(t *testing.T) { assert.Equal(t, "Tübingen", location.City()) assert.Equal(t, "de", location.CountryCode()) assert.Equal(t, "Germany", location.CountryName()) - assert.True(t, strings.HasPrefix(location.ID, "4799e4a5")) + assert.True(t, strings.HasPrefix(location.ID, s2.TokenPrefix+"4799e4a5")) }) t.Run("dog_orange.jpg", func(t *testing.T) { conf := config.TestConfig() diff --git a/internal/query/geo.go b/internal/query/geo.go index 979f95490..816d53d5d 100644 --- a/internal/query/geo.go +++ b/internal/query/geo.go @@ -26,6 +26,8 @@ func Geo(f form.GeoSearch) (results GeoResults, err error) { s := UnscopedDb() + // s.LogMode(true) + s = s.Table("photos"). Select(`photos.id, photos.photo_uid, photos.photo_type, photos.photo_lat, photos.photo_lng, photos.photo_title, photos.photo_description, photos.photo_favorite, photos.taken_at, files.file_hash, files.file_width, @@ -162,10 +164,10 @@ func Geo(f form.GeoSearch) (results GeoResults, err error) { } if f.S2 != "" { - s2Min, s2Max := s2.Range(f.S2, 7) + s2Min, s2Max := s2.PrefixedRange(f.S2, 7) s = s.Where("photos.location_id BETWEEN ? AND ?", s2Min, s2Max) } else if f.Olc != "" { - s2Min, s2Max := s2.Range(pluscode.S2(f.Olc), 7) + s2Min, s2Max := s2.PrefixedRange(pluscode.S2(f.Olc), 7) s = s.Where("photos.location_id BETWEEN ? AND ?", s2Min, s2Max) } else { // Inaccurate distance search, but probably 'good enough' for now diff --git a/pkg/s2/prefix.go b/pkg/s2/prefix.go new file mode 100644 index 000000000..c115fb4b9 --- /dev/null +++ b/pkg/s2/prefix.go @@ -0,0 +1,44 @@ +package s2 + +import ( + "strings" +) + +var TokenPrefix = "s2:" + +// NormalizeToken removes the prefix from a token and converts all characters to lower case. +func NormalizeToken(token string) string { + token = strings.ToLower(token) + token = strings.TrimSpace(token) + + if strings.HasPrefix(token, TokenPrefix) { + return token[len(TokenPrefix):] + } + + return token +} + +// Prefix adds a token prefix if not exists. +func Prefix(token string) string { + if len(token) < 3 { + return token + } + + if strings.HasPrefix(token, TokenPrefix) { + return token + } + + return TokenPrefix+token +} + +// PrefixedToken returns the prefixed S2 cell token for coordinates using the default level. +func PrefixedToken(lat, lng float64) string { + return Prefix(Token(lat, lng)) +} + +// Range returns a token range for finding nearby locations. +func PrefixedRange(token string, levelUp int) (min, max string) { + min, max = Range(token, levelUp) + + return Prefix(min), Prefix(max) +} diff --git a/pkg/s2/prefix_test.go b/pkg/s2/prefix_test.go new file mode 100644 index 000000000..252e0b909 --- /dev/null +++ b/pkg/s2/prefix_test.go @@ -0,0 +1,111 @@ +package s2 + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNormalizeToken(t *testing.T) { + t.Run(TokenPrefix+"1242342bac", func(t *testing.T) { + input := TokenPrefix+"1242342bac" + + output := NormalizeToken(input) + + assert.Equal(t, "1242342bac", output) + + }) + + t.Run("abc", func(t *testing.T) { + input := "abc" + + output := NormalizeToken(input) + + assert.Equal(t, "abc", output) + + }) +} + +func TestPrefix(t *testing.T) { + t.Run(TokenPrefix+"1242342bac", func(t *testing.T) { + input := TokenPrefix+"1242342bac" + + output := Prefix(input) + + assert.Equal(t, input, output) + + }) + + t.Run("abc", func(t *testing.T) { + input := "1242342bac" + + output := Prefix(input) + + assert.Equal(t, TokenPrefix+input, output) + + }) + + t.Run("empty string", func(t *testing.T) { + output := Prefix("") + + assert.Equal(t, "", output) + + }) +} + +func TestPrefixedToken(t *testing.T) { + t.Run("germany", func(t *testing.T) { + token := PrefixedToken(48.56344833333333, 8.996878333333333) + expected := TokenPrefix+"4799e370" + + assert.True(t, strings.HasPrefix(token, expected)) + }) + + t.Run("lat_overflow", func(t *testing.T) { + token := PrefixedToken(548.56344833333333, 8.996878333333333) + expected := "" + + assert.Equal(t, expected, token) + }) + + t.Run("lng_overflow", func(t *testing.T) { + token := PrefixedToken(48.56344833333333, 258.996878333333333) + expected := "" + + assert.Equal(t, expected, token) + }) +} + +func TestPrefixedRange(t *testing.T) { + t.Run("valid_1", func(t *testing.T) { + min, max := PrefixedRange("4799e370ca54c8b9", 1) + assert.Equal(t, TokenPrefix+"4799e370ca54c8b1", min) + assert.Equal(t, TokenPrefix+"4799e370ca54c8c1", max) + }) + t.Run("valid_2", func(t *testing.T) { + min, max := PrefixedRange(TokenPrefix+"4799e370ca54c8b9", 2) + assert.Equal(t, TokenPrefix+"4799e370ca54c881", min) + assert.Equal(t, TokenPrefix+"4799e370ca54c8c1", max) + }) + t.Run("valid_3", func(t *testing.T) { + min, max := PrefixedRange("4799e370ca54c8b9", 3) + assert.Equal(t, TokenPrefix+"4799e370ca54c801", min) + assert.Equal(t, TokenPrefix+"4799e370ca54c901", max) + }) + t.Run("valid_4", func(t *testing.T) { + min, max := PrefixedRange(TokenPrefix+"4799e370ca54c8b9", 4) + assert.Equal(t, TokenPrefix+"4799e370ca54c601", min) + assert.Equal(t, TokenPrefix+"4799e370ca54ca01", max) + }) + t.Run("valid_5", func(t *testing.T) { + min, max := PrefixedRange("4799e370ca54c8b9", 5) + assert.Equal(t, TokenPrefix+"4799e370ca54c001", min) + assert.Equal(t, TokenPrefix+"4799e370ca54d001", max) + }) + t.Run("invalid", func(t *testing.T) { + min, max := PrefixedRange("4799e370ca5q", 1) + assert.Equal(t, "", min) + assert.Equal(t, "", max) + }) +} diff --git a/pkg/s2/s2.go b/pkg/s2/s2.go index 89ce0b735..37203e34c 100644 --- a/pkg/s2/s2.go +++ b/pkg/s2/s2.go @@ -44,7 +44,9 @@ func TokenLevel(lat, lng float64, level int) string { // LatLng returns the coordinates for a S2 cell token. func LatLng(token string) (lat, lng float64) { - if token == "" || token == "-" { + token = NormalizeToken(token) + + if len(token) < 3 { return 0.0, 0.0 } @@ -65,6 +67,8 @@ func IsZero(lat, lng float64) bool { // Range returns a token range for finding nearby locations. func Range(token string, levelUp int) (min, max string) { + token = NormalizeToken(token) + c := gs2.CellIDFromToken(token) if !c.IsValid() {