Metadata: Remove estimate when setting a new country #1018

go.mod

@@ -1,7 +1,7 @@
 module github.com/photoprism/photoprism
 
 require (
-	github.com/araddon/dateparse v0.0.0-20210204225525-33e44430e129
+	github.com/araddon/dateparse v0.0.0-20210207001429-0eec95c9db7e
 	github.com/cpuguy83/go-md2man/v2 v2.0.0 // indirect
 	github.com/disintegration/imaging v1.6.2
 	github.com/djherbis/times v1.2.0

go.sum

@@ -20,8 +20,8 @@ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03
 github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo=
 github.com/PuerkitoBio/goquery v1.5.1/go.mod h1:GsLWisAFVj4WgDibEWF4pvYnkVQBpKBKeU+7zCJoLcc=
 github.com/andybalholm/cascadia v1.1.0/go.mod h1:GsXiBklL0woXo1j/WYWtSYYC4ouU9PqHO0sqidkEA4Y=
-github.com/araddon/dateparse v0.0.0-20210204225525-33e44430e129 h1:TsUg64/STzjr2oObSMkQEUhSWiyTdbAQf+WjO93De6E=
+github.com/araddon/dateparse v0.0.0-20210207001429-0eec95c9db7e h1:OjdSMCht0ZVX7IH0nTdf00xEustvbtUGRgMh3gbdmOg=
-github.com/araddon/dateparse v0.0.0-20210204225525-33e44430e129/go.mod h1:hMAUZFIkk4B1FouGxqlogyMyU6BwY/UiVmmbbzz9Up8=
+github.com/araddon/dateparse v0.0.0-20210207001429-0eec95c9db7e/go.mod h1:DCaWoUhZrYW9p1lxo/cm8EmUOOzAPSEZNGF2DK1dJgw=
 github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU=
 github.com/chzyer/logex v1.1.10 h1:Swpa1K6QvQznwJRcfTfQJmTE72DqScAa40E+fbHEXEE=
 github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWRnGsAI=
@@ -229,7 +229,7 @@ github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope
 github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
 github.com/mattn/go-isatty v0.0.12 h1:wuysRhFDzyxgEmMf5xjvJ2M9dZoWAXNNr5LSBS7uHXY=
 github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU=
-github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
+github.com/mattn/go-runewidth v0.0.10/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk=
 github.com/mattn/go-sqlite3 v1.14.0/go.mod h1:JIl7NbARA7phWnGvh0LKTyg7S9BA+6gx71ShQilpsus=
 github.com/mattn/go-sqlite3 v2.0.1+incompatible h1:xQ15muvnzGBHpIpdrNi1DA5x0+TcBZzsIDwmw9uTHzw=
 github.com/mattn/go-sqlite3 v2.0.1+incompatible/go.mod h1:FPy6KqzDD04eiIsT53CuJW3U88zkxoIYsOqkbpncsNc=
@@ -250,6 +250,7 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
 github.com/rainycape/unidecode v0.0.0-20150907023854-cb7f23ec59be h1:ta7tUOvsPHVHGom5hKW5VXNc2xZIkfCKP8iaqOyYtUQ=
 github.com/rainycape/unidecode v0.0.0-20150907023854-cb7f23ec59be/go.mod h1:MIDFMn7db1kT65GmV94GzpX9Qdi7N/pQlwb+AN8wh+Q=
+github.com/rivo/uniseg v0.1.0/go.mod h1:J6wj4VEh+S6ZtnVlnTBMWIodfgj8LQOQFoIToxlJtxc=
 github.com/rogpeppe/go-internal v1.3.0/go.mod h1:M8bDsm7K2OlrFYOpmOWEs/qY81heoFRclV5y23lUDJ4=
 github.com/russross/blackfriday/v2 v2.0.1 h1:lPqVAte+HuHNfhJ/0LC98ESWRz8afy9tM/0RK8m9o+Q=
 github.com/russross/blackfriday/v2 v2.0.1/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=

internal/entity/photo.go

@@ -478,22 +478,22 @@ func (m *Photo) PreloadMany() {
 	m.PreloadAlbums()
 }
 
-// HasID checks if the photo has a database id and uid.
+// HasID tests if the photo has a database id and uid.
 func (m *Photo) HasID() bool {
 	return m.ID > 0 && m.PhotoUID != ""
 }
 
-// UnknownLocation checks if the photo has an unknown location.
+// UnknownLocation tests if the photo has an unknown location.
 func (m *Photo) UnknownLocation() bool {
-	return m.CellID == "" || m.CellID == UnknownLocation.ID
+	return m.CellID == "" || m.CellID == UnknownLocation.ID || m.NoLatLng()
 }
 
-// HasLocation checks if the photo has a known location.
+// HasLocation tests if the photo has a known location.
 func (m *Photo) HasLocation() bool {
 	return !m.UnknownLocation()
 }
 
-// LocationLoaded checks if the photo has a known location that is currently loaded.
+// LocationLoaded tests if the photo has a known location that is currently loaded.
 func (m *Photo) LocationLoaded() bool {
 	if m.Cell == nil {
 		return false

internal/entity/photo_estimate.go

@@ -10,7 +10,7 @@ import (
 
 // EstimateCountry updates the photo with an estimated country if possible.
 func (m *Photo) EstimateCountry() {
-	if m.HasLatLng() || m.HasLocation() || m.HasPlace() || m.HasCountry() && m.PlaceSrc != SrcAuto && m.PlaceSrc != SrcEstimate {
+	if m.HasLocation() || m.HasPlace() || m.HasCountry() && m.PlaceSrc != SrcAuto && m.PlaceSrc != SrcEstimate {
 		// Do nothing.
 		return
 	}
@@ -45,7 +45,7 @@ func (m *Photo) EstimateCountry() {
 
 // EstimatePlace updates the photo with an estimated place and country if possible.
 func (m *Photo) EstimatePlace() {
-	if m.HasLatLng() || m.HasLocation() || m.HasPlace() && m.PlaceSrc != SrcAuto && m.PlaceSrc != SrcEstimate {
+	if m.HasLocation() || m.HasPlace() && m.PlaceSrc != SrcAuto && m.PlaceSrc != SrcEstimate {
 		// Do nothing.
 		return
 	}

internal/entity/photo_estimate_test.go

@@ -43,7 +43,15 @@ func TestPhoto_EstimateCountry(t *testing.T) {
 		assert.Equal(t, "Canada", m.CountryName())
 	})
 	t.Run("photo has latlng", func(t *testing.T) {
-		m := Photo{PhotoTitle: "Port Lands / Gardiner Expressway / Toronto", PhotoLat: 13.333, PhotoLng: 40.998, PhotoName: "20120910_231851_CA06E1AD", OriginalName: "demo/Toronto/port-lands--gardiner-expressway--toronto_7999515645_o.jpg"}
+		m := Photo{
+			PhotoTitle:   "Port Lands / Gardiner Expressway / Toronto",
+			PhotoLat:     13.333,
+			PhotoLng:     40.998,
+			PhotoCountry: "zz",
+			CellID:       "161437aab90c",
+			PhotoName:    "20120910_231851_CA06E1AD",
+			OriginalName: "demo/Toronto/port-lands--gardiner-expressway--toronto_7999515645_o.jpg",
+		}
 		m.EstimateCountry()
 		assert.Equal(t, "zz", m.CountryCode())
 		assert.Equal(t, "Unknown", m.CountryName())

internal/entity/photo_fixtures.go

@@ -442,8 +442,8 @@ var PhotoFixtures = PhotoMap{
 		PhotoFavorite:    false,
 		PhotoPrivate:     false,
 		PhotoType:        "image",
-		PhotoLat:         0,
+		PhotoLat:         19.681944,
-		PhotoLng:         0,
+		PhotoLng:         -98.84659,
 		PhotoAltitude:    0,
 		PhotoIso:         0,
 		PhotoFocalLength: 0,

internal/entity/photo_location.go

@@ -115,11 +115,17 @@ func (m *Photo) UpdateLocation() (keywords []string, labels classify.Labels) {
 	if m.UnknownLocation() {
 		m.Cell = &UnknownLocation
 		m.CellID = UnknownLocation.ID
+
+		// Remove place estimate if better data is available.
+		if SrcPriority[m.PlaceSrc] > SrcPriority[SrcEstimate] {
+			m.Place = &UnknownPlace
+			m.PlaceID = UnknownPlace.ID
+		}
 	} else if err := m.LoadLocation(); err == nil {
 		m.Place = m.Cell.Place
 		m.PlaceID = m.Cell.PlaceID
 	} else {
-		log.Warn(err)
+		log.Warnf("photo: location %s not found in %s", m.CellID, m.PhotoName)
 	}
 
 	if m.UnknownPlace() {
@@ -128,7 +134,7 @@ func (m *Photo) UpdateLocation() (keywords []string, labels classify.Labels) {
 	} else if err := m.LoadPlace(); err == nil {
 		m.PhotoCountry = m.Place.CountryCode()
 	} else {
-		log.Warn(err)
+		log.Warnf("photo: place %s not found in %s", m.PlaceID, m.PhotoName)
 	}
 
 	if m.UnknownCountry() {

internal/entity/photo_location_test.go

@@ -54,3 +54,49 @@ func TestPhoto_CountryName(t *testing.T) {
 		assert.Equal(t, "Germany", m.CountryName())
 	})
 }
+
+func TestUpdateLocation(t *testing.T) {
+	t.Run("estimate", func(t *testing.T) {
+		m := Photo{
+			PhotoName:    "test_photo_2",
+			PhotoCountry: "zz",
+			PhotoLat:     0.0,
+			PhotoLng:     0.0,
+			PlaceID:      "s2:85d1ea7d3278",
+			PlaceSrc:     SrcEstimate,
+		}
+
+		assert.Equal(t, "Unknown", m.CountryName())
+
+		m.UpdateLocation()
+
+		assert.Equal(t, "Mexico", m.CountryName())
+		assert.Equal(t, "mx", m.PhotoCountry)
+		assert.Equal(t, float32(0.0), m.PhotoLat)
+		assert.Equal(t, float32(0.0), m.PhotoLng)
+		assert.Equal(t, "s2:85d1ea7d3278", m.PlaceID)
+		assert.Equal(t, SrcEstimate, m.PlaceSrc)
+	})
+
+	t.Run("change_estimate", func(t *testing.T) {
+		m := Photo{
+			PhotoName:    "test_photo_1",
+			PhotoCountry: "de",
+			PhotoLat:     0.0,
+			PhotoLng:     0.0,
+			PlaceID:      "s2:85d1ea7d3278",
+			PlaceSrc:     SrcManual,
+		}
+
+		assert.Equal(t, "Germany", m.CountryName())
+
+		m.UpdateLocation()
+
+		assert.Equal(t, "Germany", m.CountryName())
+		assert.Equal(t, "de", m.PhotoCountry)
+		assert.Equal(t, float32(0.0), m.PhotoLat)
+		assert.Equal(t, float32(0.0), m.PhotoLng)
+		assert.Equal(t, "zz", m.PlaceID)
+		assert.Equal(t, SrcManual, m.PlaceSrc)
+	})
+}

internal/entity/photo_merge.go

@@ -27,7 +27,7 @@ func (m *Photo) Identical(includeMeta, includeUuid bool) (identical Photos, err
 	}
 
 	switch {
-	case includeMeta && includeUuid && m.HasLocation() && m.HasLatLng() && m.TakenSrc == SrcMeta && rnd.IsUUID(m.UUID):
+	case includeMeta && includeUuid && m.HasLocation() && m.TakenSrc == SrcMeta && rnd.IsUUID(m.UUID):
 		if err := Db().
 			Where("(taken_at = ? AND taken_src = 'meta' AND photo_stack > -1 AND cell_id = ? AND camera_serial = ? AND camera_id = ?) "+
 				"OR (uuid = ? AND photo_stack > -1)"+
@@ -36,7 +36,7 @@ func (m *Photo) Identical(includeMeta, includeUuid bool) (identical Photos, err
 			Order("photo_quality DESC, id ASC").Find(&identical).Error; err != nil {
 			return identical, err
 		}
-	case includeMeta && m.HasLocation() && m.HasLatLng() && m.TakenSrc == SrcMeta:
+	case includeMeta && m.HasLocation() && m.TakenSrc == SrcMeta:
 		if err := Db().
 			Where("(taken_at = ? AND taken_src = 'meta' AND photo_stack > -1 AND cell_id = ? AND camera_serial = ? AND camera_id = ?) "+
 				"OR (photo_path = ? AND photo_name = ?)",

internal/entity/photo_test.go

@@ -183,12 +183,22 @@ func TestPhoto_PreloadMany(t *testing.T) {
 	})
 }
 
-func TestPhoto_NoLocation(t *testing.T) {
+func TestPhoto_UnknownLocation(t *testing.T) {
-	t.Run("true", func(t *testing.T) {
+	t.Run("no_location", func(t *testing.T) {
 		m := PhotoFixtures.Get("Photo01")
 		assert.True(t, m.UnknownLocation())
 	})
-	t.Run("false", func(t *testing.T) {
+
+	t.Run("no_lat_lng", func(t *testing.T) {
+		m := PhotoFixtures.Get("Photo08")
+		m.PhotoLat = 0.0
+		m.PhotoLng = 0.0
+		// t.Logf("MODEL: %+v", m)
+		assert.False(t, m.HasLocation())
+		assert.True(t, m.UnknownLocation())
+	})
+
+	t.Run("lat_lng_cell_id", func(t *testing.T) {
 		m := PhotoFixtures.Get("Photo08")
 		// t.Logf("MODEL: %+v", m)
 		assert.True(t, m.HasLocation())
@@ -213,7 +223,7 @@ func TestPhoto_HasLatLng(t *testing.T) {
 		assert.True(t, m.HasLatLng())
 	})
 	t.Run("false", func(t *testing.T) {
-		m := PhotoFixtures.Get("Photo08")
+		m := PhotoFixtures.Get("Photo09")
 		assert.False(t, m.HasLatLng())
 	})
 }
@@ -224,7 +234,7 @@ func TestPhoto_NoLatLng(t *testing.T) {
 		assert.False(t, m.NoLatLng())
 	})
 	t.Run("true", func(t *testing.T) {
-		m := PhotoFixtures.Get("Photo08")
+		m := PhotoFixtures.Get("Photo09")
 		assert.True(t, m.NoLatLng())
 	})
 }