ai cooking
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

do real caching of locations for walmart (#277)

* do real caching of locations for walmart

* build passing

* in memory cache

* missing error check

* lazy storing

---------

Co-authored-by: paul miller <paul.miller>

authored by

Paul Miller
paul miller
and committed by
GitHub
9f74ef12 1a8a12d9

+278 -58
+1 -1
cmd/careme/web.go
··· 49 49 return fmt.Errorf("failed to create recipe generator: %w", err) 50 50 } 51 51 52 - locationStorage, err := locations.New(cfg) 52 + locationStorage, err := locations.New(cfg, cache) 53 53 if err != nil { 54 54 return fmt.Errorf("failed to create location server: %w", err) 55 55 }
+1 -1
cmd/careme/web_e2e_test.go
··· 130 130 if err != nil { 131 131 t.Fatalf("failed to create generator: %v", err) 132 132 } 133 - locationStorage, err := locations.New(cfg) 133 + locationStorage, err := locations.New(cfg, cacheStore) 134 134 if err != nil { 135 135 t.Fatalf("failed to create location server: %v", err) 136 136 }
+8 -2
cmd/zipstorecount/main.go
··· 1 1 package main 2 2 3 3 import ( 4 + "careme/internal/cache" 4 5 "careme/internal/config" 5 6 "careme/internal/locations" 6 7 "context" ··· 48 49 log.Fatalf("failed to load configuration: %v", err) 49 50 } 50 51 51 - client, err := locations.New(cfg) // warm up location client 52 + cacheStore, err := cache.MakeCache() 53 + if err != nil { 54 + log.Fatalf("failed to create cache: %v", err) 55 + } 56 + 57 + client, err := locations.New(cfg, cacheStore) 52 58 if err != nil { 53 - log.Fatalf("failed to create location stor age: %v", err) 59 + log.Fatalf("failed to create location storage: %v", err) 54 60 } 55 61 wg := sync.WaitGroup{} 56 62 resultsChan := make(chan zipStoreCount, len(metroZipCodes))
+69
internal/cache/memory.go
··· 1 + package cache 2 + 3 + import ( 4 + "context" 5 + "io" 6 + "sort" 7 + "strings" 8 + "sync" 9 + ) 10 + 11 + // InMemoryCache stores cache entries in process memory. 12 + type InMemoryCache struct { 13 + mu sync.RWMutex 14 + data map[string]string 15 + } 16 + 17 + var _ Cache = (*InMemoryCache)(nil) 18 + var _ ListCache = (*InMemoryCache)(nil) 19 + 20 + func NewInMemoryCache() *InMemoryCache { 21 + return &InMemoryCache{ 22 + data: make(map[string]string), 23 + } 24 + } 25 + 26 + func (c *InMemoryCache) Get(_ context.Context, key string) (io.ReadCloser, error) { 27 + c.mu.RLock() 28 + value, ok := c.data[key] 29 + c.mu.RUnlock() 30 + if !ok { 31 + return nil, ErrNotFound 32 + } 33 + return io.NopCloser(strings.NewReader(value)), nil 34 + } 35 + 36 + func (c *InMemoryCache) Exists(_ context.Context, key string) (bool, error) { 37 + c.mu.RLock() 38 + _, ok := c.data[key] 39 + c.mu.RUnlock() 40 + return ok, nil 41 + } 42 + 43 + func (c *InMemoryCache) Put(_ context.Context, key, value string, opts PutOptions) error { 44 + c.mu.Lock() 45 + defer c.mu.Unlock() 46 + 47 + if opts.Condition == PutIfNoneMatch { 48 + if _, exists := c.data[key]; exists { 49 + return ErrAlreadyExists 50 + } 51 + } 52 + 53 + c.data[key] = value 54 + return nil 55 + } 56 + 57 + func (c *InMemoryCache) List(_ context.Context, prefix string, _ string) ([]string, error) { 58 + c.mu.RLock() 59 + keys := make([]string, 0) 60 + for key := range c.data { 61 + if strings.HasPrefix(key, prefix) { 62 + keys = append(keys, strings.TrimPrefix(key, prefix)) 63 + } 64 + } 65 + c.mu.RUnlock() 66 + 67 + sort.Strings(keys) 68 + return keys, nil 69 + }
+74 -21
internal/locations/locations.go
··· 2 2 3 3 import ( 4 4 "careme/internal/auth" 5 + "careme/internal/cache" 5 6 "careme/internal/config" 6 7 "careme/internal/kroger" 7 8 locationtypes "careme/internal/locations/types" ··· 10 11 utypes "careme/internal/users/types" 11 12 "careme/internal/walmart" 12 13 "context" 14 + "encoding/json" 13 15 "errors" 14 16 "fmt" 15 17 "html/template" 18 + "io" 16 19 "log/slog" 17 20 "math" 18 21 "net/http" 19 22 "sort" 20 23 "sync" 24 + "time" 21 25 ) 22 26 23 27 type userLookup interface { ··· 25 29 } 26 30 27 31 type locationStorage struct { 28 - locationCache map[string]Location 29 - cacheLock sync.Mutex // to protect locationMap 30 - client []locationBackend 31 - zipCentroids map[string]ZipCentroid 32 + client []locationBackend 33 + zipCentroids map[string]ZipCentroid 34 + cache cache.Cache 32 35 } 33 36 34 37 // bad for rural areas if zip code is huge? 35 38 const maxLocationDistanceMiles = 20.0 39 + const locationCachePrefix = "location/" 36 40 37 41 type locationServer struct { 38 42 storage locationGetter ··· 52 56 // Location is kept as an alias for compatibility with existing imports. 53 57 type Location = locationtypes.Location 54 58 55 - func New(cfg *config.Config) (locationGetter, error) { 59 + func New(cfg *config.Config, c cache.Cache) (locationGetter, error) { 60 + if c == nil { 61 + return nil, fmt.Errorf("cache is required") 62 + } 56 63 if cfg.Mocks.Enable { 57 64 return mock{}, nil 58 65 } ··· 77 84 return nil, fmt.Errorf("failed to load zip centroids: %w", err) 78 85 } 79 86 return &locationStorage{ 80 - locationCache: make(map[string]Location), 81 - cacheLock: sync.Mutex{}, 82 - client: backends, 83 - zipCentroids: zipCentroids, 87 + client: backends, 88 + zipCentroids: zipCentroids, 89 + cache: c, 84 90 }, nil 85 91 86 92 } ··· 93 99 } 94 100 95 101 func (l *locationStorage) GetLocationByID(ctx context.Context, locationID string) (*Location, error) { 96 - l.cacheLock.Lock() 97 - 98 - if loc, exists := l.locationCache[locationID]; exists { 99 - l.cacheLock.Unlock() 100 - return &loc, nil 102 + if cachedLoc, ok := l.cachedLocationByID(ctx, locationID); ok { 103 + return &cachedLoc, nil 101 104 } 102 - l.cacheLock.Unlock() 103 105 104 106 for _, backend := range l.client { 105 107 if !backend.IsID(locationID) { ··· 111 113 return nil, err 112 114 } 113 115 114 - l.cacheLock.Lock() 115 - l.locationCache[locationID] = *loc 116 - l.cacheLock.Unlock() 116 + go func() { 117 + if err := l.storeLocationIfMissing(*loc); err != nil { 118 + slog.WarnContext(ctx, "failed to store location in cache", "location_id", loc.ID, "error", err) 119 + } 120 + }() 117 121 return loc, nil 118 122 } 119 123 return nil, fmt.Errorf("location ID %s not supported by any backend", locationID) ··· 170 174 allLocations = filtered 171 175 sortLocationsByDistanceFromCentroid(allLocations, requestedCentroid, l.zipCentroids) 172 176 173 - l.cacheLock.Lock() 174 - defer l.cacheLock.Unlock() 175 177 for _, loc := range allLocations { 176 - l.locationCache[loc.ID] = loc 178 + go func(loc Location) { 179 + if err := l.storeLocationIfMissing(loc); err != nil { 180 + slog.WarnContext(ctx, "failed to store location in cache", "location_id", loc.ID, "error", err) 181 + } 182 + }(loc) 177 183 } 178 184 return allLocations, nil 185 + } 186 + 187 + func (l *locationStorage) cachedLocationByID(ctx context.Context, locationID string) (Location, bool) { 188 + blob, err := l.cache.Get(ctx, locationCachePrefix+locationID) 189 + if err != nil { 190 + return Location{}, false 191 + } 192 + defer func() { 193 + _ = blob.Close() 194 + }() 195 + 196 + raw, err := io.ReadAll(blob) 197 + if err != nil { 198 + slog.WarnContext(ctx, "failed to read cached location blob", "location_id", locationID, "error", err) 199 + return Location{}, false 200 + } 201 + var loc Location 202 + if err := json.Unmarshal(raw, &loc); err != nil { 203 + slog.WarnContext(ctx, "failed to parse cached location blob", "location_id", locationID, "error", err) 204 + return Location{}, false 205 + } 206 + return loc, true 207 + } 208 + 209 + func (l *locationStorage) storeLocationIfMissing(loc Location) error { 210 + //itentionally giving its own context so its not canceled 211 + ctx, cancel := context.WithTimeout(context.Background(), time.Second*5) 212 + defer cancel() 213 + loc.CachedAt = time.Now().UTC() 214 + id := locationCachePrefix + loc.ID 215 + found, err := l.cache.Exists(ctx, id) 216 + if err != nil { 217 + return fmt.Errorf("failed to check location cache: %w", err) 218 + } 219 + if found { 220 + return nil 221 + } 222 + 223 + locationJSON, err := json.Marshal(loc) 224 + if err != nil { 225 + return fmt.Errorf("failed to marshal location for cache: %w", err) 226 + } 227 + //TODO clean out old ones? 228 + if err := l.cache.Put(ctx, id, string(locationJSON), cache.IfNoneMatch()); err != nil && !errors.Is(err, cache.ErrAlreadyExists) { 229 + return err 230 + } 231 + return nil 179 232 } 180 233 181 234 func sortLocationsByDistanceFromCentroid(locations []Location, requestedCentroid ZipCentroid, zipCentroids map[string]ZipCentroid) {
+114 -25
internal/locations/locations_test.go
··· 1 1 package locations 2 2 3 3 import ( 4 + cachepkg "careme/internal/cache" 4 5 "context" 6 + "encoding/json" 5 7 "fmt" 8 + "io" 6 9 "testing" 10 + "time" 7 11 ) 8 12 9 13 func TestGetLocationByIDUsesCache(t *testing.T) { 10 14 client := newFakeLocationClient() 15 + fc := cachepkg.NewInMemoryCache() 11 16 client.setDetailResponse("12345", Location{ 12 17 ID: "12345", 13 18 Name: "Friendly Market", ··· 15 20 ZipCode: "10001", 16 21 }) 17 22 18 - server := newTestLocationServer(client) 23 + server := newTestLocationServerWithBackendsAndCache([]locationBackend{client}, fc) 19 24 20 25 ctx := context.Background() 21 26 got, err := server.GetLocationByID(ctx, "12345") ··· 28 33 if got.ZipCode != "10001" { 29 34 t.Fatalf("unexpected zip code: %q", got.ZipCode) 30 35 } 31 - 36 + requireEventuallyCached(t, fc, locationCachePrefix+"12345") 37 + // Remove backend value to prove the second read comes from persistent cache. 38 + delete(client.details, "12345") 32 39 _, err = server.GetLocationByID(ctx, "12345") 33 40 if err != nil { 34 41 t.Fatalf("GetLocationByID second call returned error: %v", err) 35 42 } 36 - 37 - server.cacheLock.Lock() 38 - _, cached := server.locationCache["12345"] 39 - server.cacheLock.Unlock() 40 - if !cached { 41 - t.Fatalf("location 12345 not stored in cache") 42 - } 43 + requireEventuallyCached(t, fc, locationCachePrefix+"12345") 43 44 } 44 45 45 46 func TestGetLocationsByZipCachesLocations(t *testing.T) { 46 47 client := newFakeLocationClient() 48 + fc := cachepkg.NewInMemoryCache() 47 49 lat1 := 18.18060 48 50 lon1 := -66.74990 49 51 lat2 := 18.22000 ··· 69 71 }, 70 72 }) 71 73 72 - server := newTestLocationServer(client) 74 + server := newTestLocationServerWithBackendsAndCache([]locationBackend{client}, fc) 73 75 74 76 ctx := context.Background() 75 77 locs, err := server.GetLocationsByZip(ctx, "00601") ··· 92 94 t.Fatalf("unexpected second location zip code: %+v", locs[1]) 93 95 } 94 96 95 - server.cacheLock.Lock() 96 - _, okFirst := server.locationCache["111"] 97 - _, okSecond := server.locationCache["222"] 98 - server.cacheLock.Unlock() 99 - if !okFirst || !okSecond { 100 - t.Fatalf("expected both locations cached, got cache=%v", server.locationCache) 101 - } 97 + requireEventuallyCached(t, fc, locationCachePrefix+"111") 98 + requireEventuallyCached(t, fc, locationCachePrefix+"222") 102 99 } 103 100 104 101 func TestGetLocationsByZipSortsByCentroidDistance(t *testing.T) { ··· 174 171 if err == nil { 175 172 t.Fatalf("expected error when no location data returned") 176 173 } 174 + } 177 175 178 - server.cacheLock.Lock() 179 - _, cached := server.locationCache["999"] 180 - server.cacheLock.Unlock() 181 - if cached { 182 - t.Fatalf("location 999 should not be cached on error") 176 + func TestGetLocationByIDLoadsFromPersistentCache(t *testing.T) { 177 + client := newFakeLocationClient() 178 + fc := cachepkg.NewInMemoryCache() 179 + cachedAt := mustParseTime(t, "2026-01-01T00:00:00Z") 180 + preloaded := Location{ 181 + ID: "12345", 182 + Name: "Cached Store", 183 + Address: "1 Cache Way", 184 + ZipCode: "00601", 185 + CachedAt: cachedAt, 186 + } 187 + mustPutJSONInCache(t, fc, locationCachePrefix+"12345", preloaded) 188 + 189 + server := newTestLocationServerWithBackendsAndCache([]locationBackend{client}, fc) 190 + got, err := server.GetLocationByID(context.Background(), "12345") 191 + if err != nil { 192 + t.Fatalf("GetLocationByID returned error: %v", err) 193 + } 194 + if got.Name != "Cached Store" { 195 + t.Fatalf("expected cached location name, got %q", got.Name) 196 + } 197 + } 198 + 199 + func TestGetLocationsByZipStoresToPersistentCacheIfMissing(t *testing.T) { 200 + client := newFakeLocationClient() 201 + lat := 18.18060 202 + lon := -66.74990 203 + client.setListResponse("00601", []Location{ 204 + {ID: "111", Name: "Store 111", ZipCode: "00601", Lat: &lat, Lon: &lon}, 205 + }) 206 + 207 + fc := cachepkg.NewInMemoryCache() 208 + server := newTestLocationServerWithBackendsAndCache([]locationBackend{client}, fc) 209 + locs, err := server.GetLocationsByZip(context.Background(), "00601") 210 + if err != nil { 211 + t.Fatalf("GetLocationsByZip returned error: %v", err) 212 + } 213 + if len(locs) != 1 { 214 + t.Fatalf("expected 1 location, got %d", len(locs)) 215 + } 216 + 217 + storedRaw := requireEventuallyCached(t, fc, locationCachePrefix+"111") 218 + var stored Location 219 + if err := json.Unmarshal([]byte(storedRaw), &stored); err != nil { 220 + t.Fatalf("failed to decode stored location: %v", err) 221 + } 222 + if stored.CachedAt.IsZero() { 223 + t.Fatalf("expected cached_at to be set when persisted") 183 224 } 184 225 } 185 226 ··· 268 309 } 269 310 270 311 func newTestLocationServerWithBackends(backends []locationBackend) *locationStorage { 312 + return newTestLocationServerWithBackendsAndCache(backends, cachepkg.NewInMemoryCache()) 313 + } 314 + 315 + func newTestLocationServerWithBackendsAndCache(backends []locationBackend, c cachepkg.Cache) *locationStorage { 271 316 zipCentroids, err := loadEmbeddedZipCentroids() 272 317 if err != nil { 273 318 panic(err) 274 319 } 275 320 return &locationStorage{ 276 - locationCache: make(map[string]Location), 277 - client: backends, 278 - zipCentroids: zipCentroids, 321 + client: backends, 322 + zipCentroids: zipCentroids, 323 + cache: c, 324 + } 325 + } 326 + 327 + func mustPutJSONInCache(t *testing.T, c cachepkg.Cache, key string, value any) { 328 + t.Helper() 329 + raw, err := json.Marshal(value) 330 + if err != nil { 331 + t.Fatalf("failed to marshal test cache value: %v", err) 332 + } 333 + if err := c.Put(context.Background(), key, string(raw), cachepkg.Unconditional()); err != nil { 334 + t.Fatalf("failed to preload cache key %q: %v", key, err) 279 335 } 280 336 } 337 + 338 + func requireEventuallyCached(t *testing.T, c cachepkg.Cache, key string) string { 339 + t.Helper() 340 + deadline := time.Now().Add(2 * time.Second) 341 + for time.Now().Before(deadline) { 342 + raw, err := c.Get(context.Background(), key) 343 + if err == nil { 344 + defer func() { 345 + _ = raw.Close() 346 + }() 347 + body, readErr := io.ReadAll(raw) 348 + if readErr != nil { 349 + t.Fatalf("failed to read cached value for key %q: %v", key, readErr) 350 + } 351 + return string(body) 352 + } 353 + if err != cachepkg.ErrNotFound { 354 + t.Fatalf("failed checking cache for key %q: %v", key, err) 355 + } 356 + time.Sleep(10 * time.Millisecond) 357 + } 358 + t.Fatalf("expected cache entry %q to be persisted within timeout", key) 359 + return "" 360 + } 361 + 362 + func mustParseTime(t *testing.T, value string) time.Time { 363 + t.Helper() 364 + ts, err := time.Parse(time.RFC3339, value) 365 + if err != nil { 366 + t.Fatalf("failed to parse time %q: %v", value, err) 367 + } 368 + return ts 369 + }
+10 -7
internal/locations/types/location.go
··· 1 1 package types 2 2 3 + import "time" 4 + 3 5 type Location struct { 4 - ID string `json:"id"` 5 - Name string `json:"name"` 6 - Address string `json:"address"` 7 - State string `json:"state"` 8 - ZipCode string `json:"zip_code"` 9 - Lat *float64 `json:"lat,omitempty"` 10 - Lon *float64 `json:"lon,omitempty"` 6 + ID string `json:"id"` 7 + Name string `json:"name"` 8 + Address string `json:"address"` 9 + State string `json:"state"` 10 + ZipCode string `json:"zip_code"` 11 + Lat *float64 `json:"lat,omitempty"` 12 + Lon *float64 `json:"lon,omitempty"` 13 + CachedAt time.Time `json:"cached_at,omitempty"` 11 14 }
+1 -1
internal/mail/mail.go
··· 63 63 return nil, fmt.Errorf("failed to create recipe generator: %w", err) 64 64 } 65 65 66 - locationserver, err := locations.New(cfg) 66 + locationserver, err := locations.New(cfg, cache) 67 67 if err != nil { 68 68 return nil, fmt.Errorf("failed to create location server: %w", err) 69 69 }