fix: address code review findings
This commit is contained in:
@@ -1,90 +0,0 @@
|
||||
package routing
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
|
||||
"trip-planner/internal/cache"
|
||||
"trip-planner/internal/metrics"
|
||||
"trip-planner/internal/yandex"
|
||||
)
|
||||
|
||||
// SearchCacheService handles caching and on-demand Yandex /search calls.
|
||||
type SearchCacheService struct {
|
||||
cache *cache.CacheAside
|
||||
yclient *yandex.Client
|
||||
metrics *metrics.Metrics
|
||||
}
|
||||
|
||||
// NewSearchCacheService creates a new search cache service.
|
||||
func NewSearchCacheService(cacheStore cache.Cache, yclient *yandex.Client, m *metrics.Metrics) *SearchCacheService {
|
||||
return &SearchCacheService{
|
||||
cache: cache.NewCacheAside(cacheStore, m),
|
||||
yclient: yclient,
|
||||
metrics: m,
|
||||
}
|
||||
}
|
||||
|
||||
// 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 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) {
|
||||
// If we reach here, it's a cache miss - perform on-demand Yandex /search call
|
||||
return s.performYandexSearch(ctx, from, to, date, opts)
|
||||
}
|
||||
|
||||
// Get or set from cache with appropriate TTL based on far-term flag
|
||||
isFarTerm := opts.FarTerm
|
||||
data, err := s.cache.GetSearch(ctx, searchKey, fetchFunc, isFarTerm)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("search cache get/set: %w", err)
|
||||
}
|
||||
|
||||
// Parse the yandex.Response from cached data
|
||||
var result yandex.Response
|
||||
if err := json.Unmarshal(data, &result); err != nil {
|
||||
return nil, fmt.Errorf("failed to parse yandex response from cache: %w", err)
|
||||
}
|
||||
|
||||
return &result, nil
|
||||
}
|
||||
|
||||
// performYandexSearch makes the actual Yandex /search API call.
|
||||
func (s *SearchCacheService) performYandexSearch(ctx context.Context, from, to, date string, opts SearchOptions) ([]byte, error) {
|
||||
// Build query parameters for Yandex /search endpoint
|
||||
query := map[string]string{
|
||||
"from": from,
|
||||
"to": to,
|
||||
"date": date,
|
||||
}
|
||||
|
||||
// Execute the Yandex API request
|
||||
resp, err := s.yclient.Do(ctx, "GET", "/v3.0/search/", query)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("yandex search failed: %w", err)
|
||||
}
|
||||
|
||||
// Convert response to bytes for caching
|
||||
return convertResponseToBytes(resp)
|
||||
}
|
||||
|
||||
// convertResponseToBytes converts Yandex API response to bytes for caching.
|
||||
func convertResponseToBytes(resp *yandex.Response) ([]byte, error) {
|
||||
if resp == nil {
|
||||
return nil, fmt.Errorf("nil response")
|
||||
}
|
||||
data, err := json.Marshal(resp)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("failed to marshal response: %w", err)
|
||||
}
|
||||
return data, nil
|
||||
}
|
||||
@@ -1,90 +0,0 @@
|
||||
package routing
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"trip-planner/internal/cache"
|
||||
"trip-planner/internal/metrics"
|
||||
"trip-planner/internal/yandex"
|
||||
)
|
||||
|
||||
// mockCacheStoreForSearch is a mock implementation of Cache for testing search cache
|
||||
type mockCacheStoreForSearch struct {
|
||||
data map[string][]byte
|
||||
}
|
||||
|
||||
func (m *mockCacheStoreForSearch) Get(ctx context.Context, key *cache.CacheKey) ([]byte, error) {
|
||||
keyStr := key.Kind + ":" + key.Code
|
||||
if data, ok := m.data[keyStr]; ok {
|
||||
return data, nil
|
||||
}
|
||||
return nil, nil
|
||||
}
|
||||
|
||||
func (m *mockCacheStoreForSearch) Set(ctx context.Context, key *cache.CacheKey, value []byte, ttl time.Duration) error {
|
||||
keyStr := key.Kind + ":" + key.Code
|
||||
m.data[keyStr] = value
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *mockCacheStoreForSearch) Exists(ctx context.Context, key *cache.CacheKey) (bool, error) {
|
||||
keyStr := key.Kind + ":" + key.Code
|
||||
_, ok := m.data[keyStr]
|
||||
return ok, nil
|
||||
}
|
||||
|
||||
func (m *mockCacheStoreForSearch) Delete(ctx context.Context, key *cache.CacheKey) error {
|
||||
keyStr := key.Kind + ":" + key.Code
|
||||
delete(m.data, keyStr)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (m *mockCacheStoreForSearch) Increment(ctx context.Context, key *cache.CacheKey) (int64, error) {
|
||||
return 0, nil
|
||||
}
|
||||
|
||||
func (m *mockCacheStoreForSearch) Decrement(ctx context.Context, key *cache.CacheKey) (int64, error) {
|
||||
return 0, nil
|
||||
}
|
||||
|
||||
func TestNewSearchCacheService(t *testing.T) {
|
||||
mockStore := &mockCacheStoreForSearch{data: make(map[string][]byte)}
|
||||
metrics := metrics.New()
|
||||
yclient := yandex.NewClient("test-key")
|
||||
|
||||
svc := NewSearchCacheService(mockStore, yclient, metrics)
|
||||
if svc == nil {
|
||||
t.Error("expected SearchCacheService to be created")
|
||||
}
|
||||
if svc.cache == nil {
|
||||
t.Error("expected cache to be initialized")
|
||||
}
|
||||
if svc.yclient == nil {
|
||||
t.Error("expected yclient to be initialized")
|
||||
}
|
||||
if svc.metrics == nil {
|
||||
t.Error("expected metrics to be initialized")
|
||||
}
|
||||
}
|
||||
|
||||
func TestConvertResponseToBytes(t *testing.T) {
|
||||
// Test with nil response
|
||||
_, err := convertResponseToBytes(nil)
|
||||
if err == nil {
|
||||
t.Error("expected error for nil response")
|
||||
}
|
||||
|
||||
// Test with valid response
|
||||
resp := &yandex.Response{
|
||||
Segments: []yandex.Segment{},
|
||||
}
|
||||
data, err := convertResponseToBytes(resp)
|
||||
if err != nil {
|
||||
t.Errorf("expected no error, got %v", err)
|
||||
}
|
||||
if data == nil {
|
||||
t.Error("expected non-nil data")
|
||||
}
|
||||
}
|
||||
@@ -277,10 +277,10 @@ func isRetryableError(err error) bool {
|
||||
if err == nil {
|
||||
return false
|
||||
}
|
||||
// Check for HTTP status codes that are retryable (5xx errors)
|
||||
// Check for HTTP status codes that are retryable (5xx errors and 429)
|
||||
apiErr, ok := err.(*APIError)
|
||||
if ok {
|
||||
return apiErr.Code >= 500 && apiErr.Code < 600
|
||||
return (apiErr.Code >= 500 && apiErr.Code < 600) || apiErr.Code == 429
|
||||
}
|
||||
// Check for network errors
|
||||
errStr := err.Error()
|
||||
|
||||
Reference in New Issue
Block a user