Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions iowrappers/nearby_search.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,9 @@ func restoreCachedDetails(places []POI.Place, cached map[string]POI.Place) {
if places[i].Address == (POI.Address{}) {
places[i].Address = stored.Address
}
if places[i].LocationType == POI.LocationTypeAny {
places[i].LocationType = stored.LocationType
}
if !places[i].HasRealOpeningHours() && stored.HasRealOpeningHours() {
places[i].Hours = stored.Hours
}
Expand Down Expand Up @@ -501,6 +504,14 @@ func parsePlacesSearchResponse(resp maps.PlacesSearchResponse, locationType POI.
// Preserve Google's actual feature types so callers can classify the place by
// its primary function, not the type this search happened to query for.
place.Types = res.Types
// Keyword (brand) searches query with LocationTypeAny, so the searched-type
// stamp above is blank — and the blind-upsert write path would cache (and
// overwrite typed records with) LocationType "". Derive the real type from
// the place's primary Google feature type instead. Typed searches keep their
// stamp: re-tagging those is ReclassifyForCategory's job on the category paths.
if locationType == POI.LocationTypeAny {
place.LocationType = POI.PrimaryLocationType(place.Types)
}
places = append(places, place)
}
return
Expand Down
72 changes: 72 additions & 0 deletions iowrappers/nearby_search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,78 @@ func TestSelectPlacesForDetailsSkipsCachedPlaces(t *testing.T) {
})
}

// TestParsePlacesSearchResponseKeywordDerivesLocationType covers the keyword (brand) search
// path: it queries Google with LocationTypeAny, so stamping results with the searched type
// caches LocationType "" — and the blind-upsert write path then wipes the type off any
// previously typed record (a McDonald's cached by an Eatery search served with no type).
// The parser must fall back to the place's primary Google feature type instead.
func TestParsePlacesSearchResponseKeywordDerivesLocationType(t *testing.T) {
resp := maps.PlacesSearchResponse{
Results: []maps.PlacesSearchResult{
{
Name: "McDonald's",
PlaceID: "mcd",
Geometry: maps.AddressGeometry{Location: maps.LatLng{Lat: 37.32, Lng: -122.03}},
Types: []string{"meal_takeaway", "food", "point_of_interest", "establishment"},
UserRatingsTotal: 100,
},
{ // umbrella types only: no meaningful primary type to derive
Name: "Mystery Venue",
PlaceID: "untyped",
Geometry: maps.AddressGeometry{Location: maps.LatLng{Lat: 37.33, Lng: -122.04}},
Types: []string{"point_of_interest", "establishment"},
UserRatingsTotal: 5,
},
},
}

t.Run("a keyword search derives the type from the place's primary Google type", func(t *testing.T) {
places := parsePlacesSearchResponse(resp, POI.LocationTypeAny, nil, map[string]bool{}, nil, nil)
if len(places) != 2 {
t.Fatalf("expect 2 places parsed, got %d", len(places))
}
if places[0].LocationType != POI.LocationTypeMealTakeaway {
t.Errorf("LocationType = %q, want %q derived from Types", places[0].LocationType, POI.LocationTypeMealTakeaway)
}
if places[1].LocationType != POI.LocationTypeAny {
t.Errorf("LocationType = %q, want empty when Types has no meaningful primary", places[1].LocationType)
}
})

t.Run("a typed search keeps its searched-type stamp", func(t *testing.T) {
// Re-tagging typed searches is ReclassifyForCategory's job on the category
// paths; the parser must not start second-guessing it here.
places := parsePlacesSearchResponse(resp, POI.LocationTypeRestaurant, nil, map[string]bool{}, nil, nil)
if places[0].LocationType != POI.LocationTypeRestaurant {
t.Errorf("LocationType = %q, want the searched type %q", places[0].LocationType, POI.LocationTypeRestaurant)
}
})
}

// TestRestoreCachedDetailsPreservesLocationType covers the other half of the same bug: a
// keyword result that still ends up untyped (no meaningful Types) must not blank out the
// LocationType a category search already stored — restore only ever fills a gap.
func TestRestoreCachedDetailsPreservesLocationType(t *testing.T) {
stored := storedPlace("near", time.Now())
stored.SetType(POI.LocationTypeRestaurant)

t.Run("an untyped rebuilt place recovers the stored type", func(t *testing.T) {
places := []POI.Place{{ID: "near"}}
restoreCachedDetails(places, map[string]POI.Place{"near": stored})
if places[0].LocationType != POI.LocationTypeRestaurant {
t.Errorf("LocationType = %q, want stored %q", places[0].LocationType, POI.LocationTypeRestaurant)
}
})

t.Run("a typed rebuilt place keeps its fresh type", func(t *testing.T) {
places := []POI.Place{{ID: "near", LocationType: POI.LocationTypeMealTakeaway}}
restoreCachedDetails(places, map[string]POI.Place{"near": stored})
if places[0].LocationType != POI.LocationTypeMealTakeaway {
t.Errorf("LocationType = %q, want fresh %q kept", places[0].LocationType, POI.LocationTypeMealTakeaway)
}
})
}

// TestRestoreCachedDetails pins the half of the optimisation that protects the data: the write
// path is a blind upsert, so a place whose Details call was skipped must not be written back
// stripped of the fields it was skipped because of.
Expand Down