Cut Google Maps spend: reverse-geocode cache, Details mask trim, confirm skip, closure persistence

config/config.yml

@@ -1,12 +1,14 @@
 server:
   google_maps:
+    # Every field is billed per Details call, tiered by field group — see
+    # TestDetailedSearchFieldsMask for why name and user_ratings_total are
+    # deliberately absent (nothing reads them from a Details response; both
+    # arrive free with every Nearby/Text Search result).
     detailed_search_fields:
-      - name
       - opening_hours
       - formatted_address
       - adr_address
       - url
-      - user_ratings_total
       - editorial_summary
       - photos
   plan_solver:

config_yaml_test.go

@@ -0,0 +1,46 @@
+package main
+
+import (
+	"os"
+	"testing"
+
+	"gopkg.in/yaml.v3"
+)
+
+// Pins the Place Details field mask in config/config.yml. Every field here is billed per Details
+// call, tiered by field group (Basic / Contact / Atmosphere on the legacy API), so an unused
+// field is pure spend:
+//   - name and user_ratings_total must stay OUT: neither is read from a Details response on any
+//     path (both already arrive free with every Nearby/Text Search result), and
+//     user_ratings_total alone pulls the call into the Atmosphere tier. The AddUserRatingsTotal
+//     admin migration passes its own single-field list and is unaffected.
+//   - the remaining fields are load-bearing: opening_hours (open-now filtering),
+//     formatted_address/adr_address (display + address parsing), url (also the Details-freshness
+//     signal — see iowrappers/data_migrations.go detailsSourcedFields), editorial_summary (trip
+//     planner), photos (place photos + confirm gap-fill).
+func TestDetailedSearchFieldsMask(t *testing.T) {
+	raw, err := os.ReadFile("config/config.yml")
+	if err != nil {
+		t.Fatalf("reading config/config.yml: %v", err)
+	}
+	var configs Configurations
+	if err := yaml.Unmarshal(raw, &configs); err != nil {
+		t.Fatalf("unmarshal config.yml: %v", err)
+	}
+
+	fields := make(map[string]bool)
+	for _, f := range configs.Server.GoogleMaps.DetailedSearchFields {
+		fields[f] = true
+	}
+
+	for _, banned := range []string{"name", "user_ratings_total"} {
+		if fields[banned] {
+			t.Errorf("detailed_search_fields contains %q, which no Details consumer reads — it only adds billing tier", banned)
+		}
+	}
+	for _, required := range []string{"opening_hours", "formatted_address", "adr_address", "url", "editorial_summary", "photos"} {
+		if !fields[required] {
+			t.Errorf("detailed_search_fields is missing load-bearing field %q", required)
+		}
+	}
+}

iowrappers/add_searched_place_currency_test.go

@@ -0,0 +1,110 @@
+package iowrappers
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/weihesdlegend/Vacation-planner/POI"
+	"googlemaps.github.io/maps"
+)
+
+// These tests pin when a confirm buys a Place Details call. A Details call is the single most
+// expensive Google request the service makes (max field tier), and a re-confirm of a place whose
+// stored record already carries current Details-sourced fields holds all the data the confirm
+// needs — the same placeDetailsAreCurrent rule the nearby-search path already trusts.
+
+func countingEnricher(calls *int) placeDetailsEnricher {
+	return func(ctx context.Context, placeID string) (maps.PlaceDetailsResult, error) {
+		*calls++
+		return maps.PlaceDetailsResult{}, errors.New("no network in this test")
+	}
+}
+
+func TestConfirmSkipsDetailsWhenCachedRecordIsCurrent(t *testing.T) {
+	s, ctx := newAddSearchedPlaceFixture(t)
+
+	placeID := "museum-current"
+	stored := POI.Place{
+		ID:            placeID,
+		Name:          "City History Museum",
+		LocationType:  POI.LocationType("museum"),
+		Location:      POI.Location{Latitude: 37.4, Longitude: -122.1},
+		URL:           "https://maps.google.com/?cid=42", // proof a Details call landed (detailsSourcedFields)
+		LastUpdatedAt: time.Now().Format(time.RFC3339),
+	}
+	s.redisClient.SetPlacesAddGeoLocations(ctx, []POI.Place{stored})
+
+	stashCandidate(t, s, ctx, POI.Place{
+		ID:       placeID,
+		Name:     "City History Museum",
+		Types:    []string{"museum", "point_of_interest", "establishment"},
+		Location: POI.Location{Latitude: 37.4, Longitude: -122.1},
+	})
+
+	calls := 0
+	result, err := s.addSearchedPlaceToCache(ctx, placeID, countingEnricher(&calls))
+	if err != nil {
+		t.Fatalf("addSearchedPlaceToCache: %v", err)
+	}
+	if calls != 0 {
+		t.Errorf("enricher called %d times for a current cached record, want 0 — this is a billed Place Details call", calls)
+	}
+	if !result.AlreadyCached {
+		t.Error("AlreadyCached = false, want true")
+	}
+	if result.Place.URL != stored.URL {
+		t.Errorf("Place.URL = %q, want the cached record's %q restored", result.Place.URL, stored.URL)
+	}
+}
+
+func TestConfirmBuysDetailsWhenCachedRecordIsStale(t *testing.T) {
+	s, ctx := newAddSearchedPlaceFixture(t)
+
+	placeID := "museum-stale"
+	stale := time.Now().Add(-(PlaceDetailsRefreshDuration + 24*time.Hour))
+	s.redisClient.SetPlacesAddGeoLocations(ctx, []POI.Place{{
+		ID:            placeID,
+		Name:          "Old Museum",
+		LocationType:  POI.LocationType("museum"),
+		Location:      POI.Location{Latitude: 37.4, Longitude: -122.1},
+		URL:           "https://maps.google.com/?cid=43",
+		LastUpdatedAt: stale.Format(time.RFC3339),
+	}})
+
+	stashCandidate(t, s, ctx, POI.Place{
+		ID:       placeID,
+		Name:     "Old Museum",
+		Types:    []string{"museum", "point_of_interest", "establishment"},
+		Location: POI.Location{Latitude: 37.4, Longitude: -122.1},
+	})
+
+	calls := 0
+	if _, err := s.addSearchedPlaceToCache(ctx, placeID, countingEnricher(&calls)); err != nil {
+		t.Fatalf("addSearchedPlaceToCache: %v", err)
+	}
+	if calls != 1 {
+		t.Errorf("enricher called %d times for a stale cached record, want 1", calls)
+	}
+}
+
+func TestConfirmBuysDetailsWhenNotCached(t *testing.T) {
+	s, ctx := newAddSearchedPlaceFixture(t)
+
+	placeID := "museum-uncached"
+	stashCandidate(t, s, ctx, POI.Place{
+		ID:       placeID,
+		Name:     "Brand New Museum",
+		Types:    []string{"museum", "point_of_interest", "establishment"},
+		Location: POI.Location{Latitude: 37.4, Longitude: -122.1},
+	})
+
+	calls := 0
+	if _, err := s.addSearchedPlaceToCache(ctx, placeID, countingEnricher(&calls)); err != nil {
+		t.Fatalf("addSearchedPlaceToCache: %v", err)
+	}
+	if calls != 1 {
+		t.Errorf("enricher called %d times for an uncached place, want 1", calls)
+	}
+}

iowrappers/closure_persistence_test.go

@@ -0,0 +1,119 @@
+package iowrappers
+
+import (
+	"testing"
+
+	"github.com/weihesdlegend/Vacation-planner/POI"
+)
+
+// Pins that a cold search PERSISTS permanently-closed places while still excluding them from the
+// response. The previous behavior filtered non-Operational results BEFORE the cache write, which
+// discarded the closure signal entirely: the stale record kept Status OPERATIONAL forever and the
+// place kept being served from cache. Persisting the closure lets the read-side Operational
+// filters (RedisClient.NearbySearch) retire the place everywhere, at zero extra API cost.
+//
+// Filtering is the DEFAULT: now that closed places genuinely live in the cache, a caller that
+// forgets to ask for filtering must never receive them — planners consume these results and a
+// zero-value request has to be safe. IncludeClosedPlaces is the explicit opt-in for the rare
+// caller that wants everything.
+func TestColdSearchPersistsClosuresButExcludesThemFromResponse(t *testing.T) {
+	s, ctx := newAddSearchedPlaceFixture(t)
+
+	open := POI.Place{
+		ID:           "open-1",
+		Name:         "Open Diner",
+		LocationType: POI.LocationType("restaurant"),
+		Status:       POI.Operational,
+		Location:     POI.Location{Latitude: 37.4, Longitude: -122.1},
+	}
+	closed := POI.Place{
+		ID:           "closed-1",
+		Name:         "Shuttered Grill",
+		LocationType: POI.LocationType("restaurant"),
+		Status:       POI.ClosedPermanently,
+		Location:     POI.Location{Latitude: 37.401, Longitude: -122.101},
+	}
+	// Deliberately a zero-value request apart from the search identity: the default must filter.
+	req := &PlaceSearchRequest{
+		PlaceCat: POI.PlaceCategoryEatery,
+		Location: POI.Location{Latitude: 37.4, Longitude: -122.1},
+	}
+
+	got := s.persistAndFilterSearchResults(ctx, req, []POI.Place{open, closed})
+
+	if len(got) != 1 || got[0].ID != "open-1" {
+		t.Fatalf("response = %+v, want only open-1 (closed places excluded by default)", got)
+	}
+
+	cached, err := s.redisClient.CachedPlaces(ctx, []string{"open-1", "closed-1"})
+	if err != nil {
+		t.Fatalf("CachedPlaces: %v", err)
+	}
+	if _, ok := cached["open-1"]; !ok {
+		t.Error("open-1 was not persisted")
+	}
+	stored, ok := cached["closed-1"]
+	if !ok {
+		t.Fatal("closed-1 was not persisted — the closure signal was discarded, the pre-write-filter bug")
+	}
+	if stored.Status != POI.ClosedPermanently {
+		t.Errorf("closed-1 Status = %q, want %q recorded", stored.Status, POI.ClosedPermanently)
+	}
+
+	// The read path must retire it BY DEFAULT: a zero-value cache read excludes the closed place.
+	readReq := &PlaceSearchRequest{
+		PlaceCat:      POI.PlaceCategoryEatery,
+		Location:      POI.Location{Latitude: 37.4, Longitude: -122.1},
+		Radius:        1000,
+		MinNumResults: 1,
+	}
+	fromCache, err := s.redisClient.NearbySearch(ctx, readReq)
+	if err != nil {
+		t.Fatalf("RedisClient.NearbySearch: %v", err)
+	}
+	for _, p := range fromCache {
+		if p.ID == "closed-1" {
+			t.Error("closed-1 served from a default cache read — planners would receive a closed place")
+		}
+	}
+}
+
+func TestIncludeClosedPlacesOptsIntoUnfilteredResults(t *testing.T) {
+	s, ctx := newAddSearchedPlaceFixture(t)
+
+	places := []POI.Place{
+		{ID: "a", Name: "A", LocationType: POI.LocationType("restaurant"), Status: POI.Operational, Location: POI.Location{Latitude: 37.4, Longitude: -122.1}},
+		{ID: "b", Name: "B", LocationType: POI.LocationType("restaurant"), Status: POI.ClosedPermanently, Location: POI.Location{Latitude: 37.401, Longitude: -122.101}},
+	}
+	req := &PlaceSearchRequest{
+		PlaceCat:            POI.PlaceCategoryEatery,
+		Location:            POI.Location{Latitude: 37.4, Longitude: -122.1},
+		IncludeClosedPlaces: true,
+	}
+
+	got := s.persistAndFilterSearchResults(ctx, req, places)
+	if len(got) != 2 {
+		t.Fatalf("response has %d places, want 2 — IncludeClosedPlaces opted into everything", len(got))
+	}
+
+	readReq := &PlaceSearchRequest{
+		PlaceCat:            POI.PlaceCategoryEatery,
+		Location:            POI.Location{Latitude: 37.4, Longitude: -122.1},
+		Radius:              1000,
+		MinNumResults:       1,
+		IncludeClosedPlaces: true,
+	}
+	fromCache, err := s.redisClient.NearbySearch(ctx, readReq)
+	if err != nil {
+		t.Fatalf("RedisClient.NearbySearch: %v", err)
+	}
+	foundClosed := false
+	for _, p := range fromCache {
+		if p.ID == "b" {
+			foundClosed = true
+		}
+	}
+	if !foundClosed {
+		t.Error("IncludeClosedPlaces read did not return the closed place")
+	}
+}

iowrappers/data_migrations_test.go

@@ -72,7 +72,8 @@ func TestRemovePlaces(t *testing.T) {
 	var err error
 	redisClient.SetPlacesAddGeoLocations(ctx, []POI.Place{placeA, placeB, placeC})
 	places, _ = redisClient.NearbySearch(ctx, &PlaceSearchRequest{
-		PlaceCat: POI.PlaceCategoryVisit,
+		PlaceCat:            POI.PlaceCategoryVisit,
+		IncludeClosedPlaces: true, // migration test reads raw bucket contents; fixtures carry no Status
 		Location: POI.Location{
 			Latitude:  12.5636,
 			Longitude: 14.7813,
@@ -92,7 +93,8 @@ func TestRemovePlaces(t *testing.T) {
 	}
 
 	places, _ = redisClient.NearbySearch(ctx, &PlaceSearchRequest{
-		PlaceCat: POI.PlaceCategoryVisit,
+		PlaceCat:            POI.PlaceCategoryVisit,
+		IncludeClosedPlaces: true, // migration test reads raw bucket contents; fixtures carry no Status
 		Location: POI.Location{
 			Latitude:  12.5636,
 			Longitude: 14.7813,
@@ -106,6 +108,7 @@ func TestRemovePlaces(t *testing.T) {
 	}
 
 	places, _ = redisClient.NearbySearch(ctx, &PlaceSearchRequest{PlaceCat: POI.PlaceCategoryEatery,
+		IncludeClosedPlaces: true, // migration test reads raw bucket contents; fixtures carry no Status
 		Location: POI.Location{
 			Latitude:  12.5636,
 			Longitude: 14.7813,

iowrappers/nearby_search.go

@@ -35,7 +35,13 @@ type PlaceSearchRequest struct {
 	// suppose a location has more places established over time, this field would help trigger new searches to get those new establishments.
 	MinNumResults uint
 
-	BusinessStatus POI.BusinessStatus
+	// IncludeClosedPlaces opts a caller INTO receiving non-Operational places. The zero value
+	// filters them from both cache reads and cold-search responses: closed places are persisted
+	// in the cache (their closure is the signal that retires them), so a request that forgot to
+	// ask for filtering must never serve one — planners consume these results directly. This
+	// replaces the old BusinessStatus field, whose zero value was the UNFILTERED behavior and
+	// therefore one forgotten assignment away from leaking closures.
+	IncludeClosedPlaces bool
 	// true if using precise geolocation instead of using a grander administrative area
 	UsePreciseLocation bool
 

iowrappers/photos_client.go

@@ -139,7 +139,9 @@ func (c *MapsPhotoClient) GetPhotoURL(ctx context.Context, photoRef string, plac
 
 			// Acquire semaphore for API rate limiting
 			c.mapsClient.apiSemaphore <- struct{}{}
-			r, err = c.mapsClient.PlaceDetailedSearch(ctx, placeId, c.mapsClient.DetailedSearchFields)
+			// Only the photo reference is needed here — the full DetailedSearchFields mask
+			// would bill the Contact/Atmosphere field tiers for data this path throws away.
+			r, err = c.mapsClient.PlaceDetailedSearch(ctx, placeId, []string{"photos"})
 			<-c.mapsClient.apiSemaphore // Release semaphore
 			if err != nil {
 				return "", err