diff --git a/services/search/pkg/content/extractor.go b/services/search/pkg/content/extractor.go index a91be6cb4e..3d4e457288 100644 --- a/services/search/pkg/content/extractor.go +++ b/services/search/pkg/content/extractor.go @@ -23,7 +23,7 @@ func getFirstValue(m map[string][]string, key string) (string, error) { return "", fmt.Errorf("unknown key: %v", key) } - if len(m) == 0 { + if len(v) == 0 { return "", fmt.Errorf("no values for: %v", key) } diff --git a/services/search/pkg/content/tika_location.go b/services/search/pkg/content/tika_location.go index d7b489daab..936ba9b911 100644 --- a/services/search/pkg/content/tika_location.go +++ b/services/search/pkg/content/tika_location.go @@ -1,6 +1,8 @@ package content import ( + "fmt" + "math" "strconv" libregraph "github.com/opencloud-eu/libre-graph-api-go" @@ -10,34 +12,40 @@ import ( const metresToFeet = 3.280839895 func (t Tika) getLocation(meta map[string][]string) *libregraph.GeoCoordinates { - var location *libregraph.GeoCoordinates - initLocation := func() { - if location == nil { - location = libregraph.NewGeoCoordinates() - } + // the facet needs a sane coordinate pair, an altitude alone is useless + lat, latErr := parseCoordinate(meta, "geo:lat", 90) + long, longErr := parseCoordinate(meta, "geo:long", 180) + if latErr != nil || longErr != nil { + return nil } - if v, err := getFirstValue(meta, "geo:lat"); err == nil { - if i, err := strconv.ParseFloat(v, 64); err == nil { - initLocation() - location.SetLatitude(i) - } - } - - if v, err := getFirstValue(meta, "geo:long"); err == nil { - if i, err := strconv.ParseFloat(v, 64); err == nil { - initLocation() - location.SetLongitude(i) - } - } + location := libregraph.NewGeoCoordinates() + location.SetLatitude(lat) + location.SetLongitude(long) // tika emits metres (negative below sea level), graph wants feet if v, err := getFirstValue(meta, "geo:alt"); err == nil { if metres, err := strconv.ParseFloat(v, 64); err == nil { - initLocation() location.SetAltitude(metres * metresToFeet) } } return location } + +func parseCoordinate(meta map[string][]string, key string, limit float64) (float64, error) { + v, err := getFirstValue(meta, key) + if err != nil { + return 0, err + } + + f, err := strconv.ParseFloat(v, 64) + if err != nil { + return 0, err + } + if math.Abs(f) > limit { + return 0, fmt.Errorf("%s out of range: %v", key, f) + } + + return f, nil +} diff --git a/services/search/pkg/content/tika_location_test.go b/services/search/pkg/content/tika_location_test.go index 067d06e4f8..8b79810de5 100644 --- a/services/search/pkg/content/tika_location_test.go +++ b/services/search/pkg/content/tika_location_test.go @@ -22,11 +22,25 @@ var _ = Describe("getLocation", func() { It("keeps below-sea-level altitude negative", func() { metres := -227.4 - location := Tika{}.getLocation(map[string][]string{"geo:alt": {"-227.4"}}) + location := Tika{}.getLocation(map[string][]string{ + "geo:lat": {"31.5"}, + "geo:long": {"35.47"}, + "geo:alt": {"-227.4"}, + }) Expect(location).ToNot(BeNil()) Expect(location.Altitude).To(Equal(libregraph.PtrFloat64(metres * metresToFeet))) }) + It("returns nil for an incomplete coordinate pair", func() { + Expect(Tika{}.getLocation(map[string][]string{"geo:lat": {"49.48"}})).To(BeNil()) + Expect(Tika{}.getLocation(map[string][]string{"geo:alt": {"227.4"}})).To(BeNil()) + }) + + It("returns nil for out-of-range coordinates", func() { + Expect(Tika{}.getLocation(map[string][]string{"geo:lat": {"91"}, "geo:long": {"11.1"}})).To(BeNil()) + Expect(Tika{}.getLocation(map[string][]string{"geo:lat": {"49.48"}, "geo:long": {"-180.5"}})).To(BeNil()) + }) + It("returns nil when no location metadata is present", func() { Expect(Tika{}.getLocation(map[string][]string{})).To(BeNil()) }) diff --git a/services/search/pkg/content/tika_photo.go b/services/search/pkg/content/tika_photo.go index a833df3b1d..2ee961a300 100644 --- a/services/search/pkg/content/tika_photo.go +++ b/services/search/pkg/content/tika_photo.go @@ -63,7 +63,7 @@ func (t Tika) getPhoto(meta map[string][]string) *libregraph.Photo { } if v, err := getFirstValue(meta, "exif:ExposureTime"); err == nil { - if i, err := strconv.ParseFloat(v, 64); err == nil { + if i, err := strconv.ParseFloat(v, 64); err == nil && i > 0 { initPhoto() photo.SetExposureNumerator(1) photo.SetExposureDenominator(math.Round(1 / i))