fix: address code review findings
This commit is contained in:
@@ -205,6 +205,18 @@ func RouteSearch(hc *HandlerContext, w http.ResponseWriter, r *http.Request) {
|
||||
RankingMode: rankingMode,
|
||||
}
|
||||
|
||||
// Determine if this is a far-term search (date is more than 7 days in the future)
|
||||
if req.Date != "" {
|
||||
requestDate, err := time.Parse("2006-01-02", req.Date)
|
||||
if err == nil {
|
||||
now := time.Now()
|
||||
daysDiff := int(requestDate.Sub(now).Hours() / 24)
|
||||
if daysDiff >= 7 {
|
||||
opts.FarTerm = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Detect closed stations and get neighbors for fallback
|
||||
closedStationsMap := make(map[string]bool)
|
||||
neighborsMap := make(map[string][]storage.StationNeighbor)
|
||||
@@ -431,7 +443,7 @@ func StationStatus(hc *HandlerContext, w http.ResponseWriter, r *http.Request) {
|
||||
func adminAuth(hc *HandlerContext, w http.ResponseWriter, r *http.Request) bool {
|
||||
expectedAPIKey := os.Getenv("TRIP_PLANNER_ADMIN_API_KEY")
|
||||
if expectedAPIKey == "" {
|
||||
http.Error(w, "unauthorized", http.StatusUnauthorized)
|
||||
http.Error(w, "server configuration error: TRIP_PLANNER_ADMIN_API_KEY is not set", http.StatusInternalServerError)
|
||||
return false
|
||||
}
|
||||
providedAPIKey := r.Header.Get("X-Admin-Api-Key")
|
||||
|
||||
@@ -131,12 +131,17 @@ func (sm *StationMonitor) updateStationStatus(ctx context.Context, tripCount int
|
||||
zeroSinceData, err := sm.Cache.Get(ctx, zeroSinceKey)
|
||||
var zeroSince time.Time
|
||||
if err == nil && zeroSinceData != nil {
|
||||
var zeroSinceUnix int64
|
||||
_, parseErr := fmt.Sscanf(string(zeroSinceData), "%d", &zeroSinceUnix)
|
||||
if parseErr == nil {
|
||||
zeroSince = time.Unix(zeroSinceUnix, 0)
|
||||
} else {
|
||||
// Handle "0" marker for time.Time{} (no zero-since)
|
||||
if string(zeroSinceData) == "0" {
|
||||
zeroSince = time.Time{}
|
||||
} else {
|
||||
var zeroSinceUnix int64
|
||||
_, parseErr := fmt.Sscanf(string(zeroSinceData), "%d", &zeroSinceUnix)
|
||||
if parseErr == nil {
|
||||
zeroSince = time.Unix(zeroSinceUnix, 0)
|
||||
} else {
|
||||
zeroSince = time.Time{}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -184,7 +189,12 @@ func (sm *StationMonitor) updateStationStatus(ctx context.Context, tripCount int
|
||||
}
|
||||
|
||||
// Write updated zero-since timestamp to cache with 24h TTL
|
||||
if err := sm.Cache.Set(ctx, zeroSinceKey, []byte(fmt.Sprintf("%d", zeroSince.Unix())), 24*time.Hour); err != nil {
|
||||
// Use "0" marker for time.Time{} to indicate no zero-since
|
||||
zeroSinceStr := "0"
|
||||
if !zeroSince.IsZero() {
|
||||
zeroSinceStr = fmt.Sprintf("%d", zeroSince.Unix())
|
||||
}
|
||||
if err := sm.Cache.Set(ctx, zeroSinceKey, []byte(zeroSinceStr), 24*time.Hour); err != nil {
|
||||
return newStatus, fmt.Errorf("cache set zero since: %w", err)
|
||||
}
|
||||
|
||||
|
||||
17
internal/cache/preferences.go
vendored
17
internal/cache/preferences.go
vendored
@@ -107,15 +107,6 @@ func (p *Preferences) AddSavedCity(ctx context.Context, userID, cityCode, cityNa
|
||||
|
||||
// RemoveSavedCity removes a city from the user's saved cities.
|
||||
func (p *Preferences) RemoveSavedCity(ctx context.Context, userID, cityCode string) error {
|
||||
key := &CacheKey{
|
||||
Kind: "prefs:saved_city:" + userID,
|
||||
Code: userID,
|
||||
From: "",
|
||||
To: "",
|
||||
Date: "",
|
||||
Request: "",
|
||||
}
|
||||
|
||||
// Load existing cities
|
||||
cities, err := p.GetSavedCities(ctx, userID)
|
||||
if err != nil {
|
||||
@@ -132,6 +123,10 @@ func (p *Preferences) RemoveSavedCity(ctx context.Context, userID, cityCode stri
|
||||
|
||||
if len(result) == 0 {
|
||||
// If no cities left, delete the key
|
||||
key := &CacheKey{
|
||||
Kind: "prefs:saved_city:" + userID,
|
||||
Code: userID,
|
||||
}
|
||||
return p.store.Delete(ctx, key)
|
||||
}
|
||||
|
||||
@@ -141,6 +136,10 @@ func (p *Preferences) RemoveSavedCity(ctx context.Context, userID, cityCode stri
|
||||
return err
|
||||
}
|
||||
|
||||
key := &CacheKey{
|
||||
Kind: "prefs:saved_city:" + userID,
|
||||
Code: userID,
|
||||
}
|
||||
return p.store.Set(ctx, key, data, PreferenceTTL)
|
||||
}
|
||||
|
||||
|
||||
5
internal/cache/store.go
vendored
5
internal/cache/store.go
vendored
@@ -167,6 +167,11 @@ func GetSearchKey(from, to, date string) *CacheKey {
|
||||
return &CacheKey{Kind: "search", From: from, To: to, Date: date}
|
||||
}
|
||||
|
||||
// GetSearchKeyWithFarTerm returns the cache key for a search query with far-term flag.
|
||||
func GetSearchKeyWithFarTerm(from, to, date, farTermFlag string) *CacheKey {
|
||||
return &CacheKey{Kind: "search", From: from, To: to, Date: date, Request: farTermFlag}
|
||||
}
|
||||
|
||||
// CacheAside represents the cache-aside pattern implementation.
|
||||
// It follows the pattern: Redis → miss → Postgres/API → write-back to Redis.
|
||||
type CacheAside struct {
|
||||
|
||||
@@ -2,6 +2,7 @@ package routing
|
||||
|
||||
import (
|
||||
"context"
|
||||
"log"
|
||||
"sort"
|
||||
"time"
|
||||
"trip-planner/internal/storage"
|
||||
@@ -626,9 +627,10 @@ func (g *Graph) FindRoute(originID, destID string, opts SearchOptions, closedSta
|
||||
"date": time.Now().Format("2006-01-02"),
|
||||
}
|
||||
|
||||
resp, err := yandexClient.Do(context.Background(), "GET", "/v3.0/search/", query)
|
||||
resp, err := yandexClient.Do(context.TODO(), "GET", "/v3.0/search/", query)
|
||||
if err != nil {
|
||||
// If API call fails, return nil (no route found)
|
||||
// If API call fails, log the error and return nil (no route found)
|
||||
log.Printf("WARNING: yandex search failed for route expansion: %v", err)
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -1106,9 +1108,9 @@ func (g *Graph) checkRouteForChanges(itinerary *Itinerary) bool {
|
||||
|
||||
// rescheduleRoute performs a re-search for the route with updated graph data.
|
||||
// This is called when significant changes are detected in the route legs.
|
||||
func (g *Graph) rescheduleRoute(originID, destID string, opts SearchOptions, closedStations map[string]bool, neighbors map[string][]storage.StationNeighbor) *Itinerary {
|
||||
func (g *Graph) rescheduleRoute(originID, destID string, opts SearchOptions, closedStations map[string]bool, neighbors map[string][]storage.StationNeighbor, yclient ...*yandex.Client) *Itinerary {
|
||||
// Re-run the search with the same options to get an updated route
|
||||
result := g.FindRoute(originID, destID, opts, closedStations, neighbors)
|
||||
result := g.FindRoute(originID, destID, opts, closedStations, neighbors, yclient...)
|
||||
if result != nil {
|
||||
result.LastChecked = time.Now().Unix()
|
||||
result.NeedsReSearch = false
|
||||
@@ -1119,9 +1121,9 @@ func (g *Graph) rescheduleRoute(originID, destID string, opts SearchOptions, clo
|
||||
|
||||
// CheckAndRescheduleRoute checks a route for changes and returns an updated route if needed.
|
||||
// This is the main entry point for flight change notification logic.
|
||||
func (g *Graph) CheckAndRescheduleRoute(itinerary *Itinerary, originID, destID string, opts SearchOptions, closedStations map[string]bool, neighbors map[string][]storage.StationNeighbor) *Itinerary {
|
||||
func (g *Graph) CheckAndRescheduleRoute(itinerary *Itinerary, originID, destID string, opts SearchOptions, closedStations map[string]bool, neighbors map[string][]storage.StationNeighbor, yclient ...*yandex.Client) *Itinerary {
|
||||
if g.checkRouteForChanges(itinerary) {
|
||||
return g.rescheduleRoute(originID, destID, opts, closedStations, neighbors)
|
||||
return g.rescheduleRoute(originID, destID, opts, closedStations, neighbors, yclient...)
|
||||
}
|
||||
return itinerary
|
||||
}
|
||||
|
||||
@@ -29,8 +29,12 @@ func NewSearchCacheService(cacheStore cache.Cache, yclient *yandex.Client, m *me
|
||||
// SearchWithCache performs a route search with caching support.
|
||||
// It uses the cache-aside pattern: try cache first, then Yandex API, then write back to cache.
|
||||
func (s *SearchCacheService) SearchWithCache(ctx context.Context, from, to, date string, opts SearchOptions) (*yandex.Response, error) {
|
||||
// Generate cache key
|
||||
searchKey := cache.GetSearchKey(from, to, date)
|
||||
// Generate cache key including far-term flag to distinguish near-term vs far-term searches
|
||||
farTermFlag := "near"
|
||||
if opts.FarTerm {
|
||||
farTermFlag = "far"
|
||||
}
|
||||
searchKey := cache.GetSearchKeyWithFarTerm(from, to, date, farTermFlag)
|
||||
|
||||
// Try to get from cache first
|
||||
fetchFunc := func() ([]byte, error) {
|
||||
|
||||
Reference in New Issue
Block a user